bug #16844 fix: le contrôle de conformité ne remonte qu'une anomalie à la fois

sedalib/src/main/java/fr/gouv/vitam/tools/sedalib/utils/SEDALibProgressLogger.java

@@ -321,7 +321,7 @@ public static void doProgressLogIfDebug(SEDALibProgressLogger spl, String log, T
     }
 
     /**
-     * Do progress log, and log with exception detail if any, and wait 1ms to allow interruption
+     * Do progress log, and log with exception detail if any, and check for interruption
      *
      * @param spl   the SEDALib progress logger
      * @param level the level
@@ -333,10 +333,24 @@ public static void doProgressLog(SEDALibProgressLogger spl, int level, String lo
         throws InterruptedException {
         if (spl != null) {
             doProgressLogWithoutInterruption(spl, level, log, e);
-            Thread.sleep(1);
+            checkInterruption();
         }
     }
 
+    /**
+     * Check if the current thread has been interrupted, and if so throw the interruption.
+     * <p>
+     * This is the interruption point offered to the callers of the progress log methods. It has to
+     * stay allocation and syscall free, as it's called once per imported object (per unzipped file,
+     * per DataObjectGroup, per BinaryDataObject, per ArchiveUnit...), even when the message is
+     * filtered out by the log level and displayed nowhere.
+     *
+     * @throws InterruptedException if the current thread has been interrupted
+     */
+    private static void checkInterruption() throws InterruptedException {
+        if (Thread.interrupted()) throw new InterruptedException();
+    }
+
     /**
      * Do progress log if the counter is a step multiple.
      *
@@ -356,7 +370,7 @@ public static void doProgressLogIfStep(SEDALibProgressLogger spl, int level, int
                         (spl.progressLogFunc != null) && (level <= spl.progressFuncLogLevel)
                     ) spl.progressLogFunc.doProgressLog(count, (count % spl.progressFuncStep == 0 ? "" : " * ") + log);
                     spl.log(level, log);
-                    Thread.sleep(1);
+                    checkInterruption();
                     spl.previousStepEpochSeconds = nowEpochSeconds;
                     return;
                 }
@@ -365,7 +379,7 @@ public static void doProgressLogIfStep(SEDALibProgressLogger spl, int level, int
                 }
                 if ((spl.progressLogFunc != null) && (count % spl.progressFuncStep) == 0) {
                     spl.progressLogFunc.doProgressLog(count, log);
-                    Thread.sleep(1);
+                    checkInterruption();
                 }
             }
         }

sedalib/src/main/java/fr/gouv/vitam/tools/sedalib/xml/SEDAXMLValidator.java

@@ -40,6 +40,7 @@
 import fr.gouv.vitam.tools.sedalib.core.seda.SedaContext;
 import fr.gouv.vitam.tools.sedalib.utils.SEDALibException;
 import org.apache.xerces.util.XMLCatalogResolver;
+import org.xml.sax.ErrorHandler;
 import org.xml.sax.SAXException;
 import org.xml.sax.SAXParseException;
 
@@ -57,6 +58,8 @@
 import java.io.IOException;
 import java.net.URL;
 import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
+import java.util.List;
 import java.util.Scanner;
 
 public class SEDAXMLValidator {
@@ -171,6 +174,68 @@ private String getContextualErrorMessage(String manifest, SAXParseException e) {
         return result;
     }
 
+    /**
+     * The maximum number of anomalies detailed in the validation message.
+     */
+    private static final int MAX_LISTED_ANOMALIES = 50;
+
+    /**
+     * Collects every recoverable validation error instead of letting the validator throw on the
+     * first one.
+     * <p>
+     * Without an error handler the validator stops on the first anomaly, so a non conformant
+     * manifest gives one anomaly at a time and has to be checked as many times as it has problems.
+     * Worse, the check has to be replayed to see the next one, and the picture it gives changes from
+     * one run to the next, which is what the "anomalies remontées puis disparues" report describes.
+     */
+    private static class CollectingErrorHandler implements ErrorHandler {
+
+        private final List<SAXParseException> errors = new ArrayList<>();
+
+        @Override
+        public void warning(SAXParseException e) {
+            // a warning is not a conformity anomaly, the manifest stays valid
+        }
+
+        @Override
+        public void error(SAXParseException e) {
+            errors.add(e);
+        }
+
+        @Override
+        public void fatalError(SAXParseException e) throws SAXException {
+            errors.add(e);
+            // the document can't be parsed further, no point in going on
+            throw e;
+        }
+
+        private List<SAXParseException> getErrors() {
+            return errors;
+        }
+    }
+
+    private void throwIfAnomalies(String manifest, CollectingErrorHandler errorHandler) throws SEDALibException {
+        List<SAXParseException> errors = errorHandler.getErrors();
+        if (errors.isEmpty()) return;
+
+        StringBuilder message = new StringBuilder(
+            "Le flux XML n'est pas conforme, " + errors.size() + " anomalie(s) détectée(s)"
+        );
+        int count = 0;
+        for (SAXParseException error : errors) {
+            if (count == MAX_LISTED_ANOMALIES) {
+                message
+                    .append("\n\n-> ... et ")
+                    .append(errors.size() - MAX_LISTED_ANOMALIES)
+                    .append(" autre(s) anomalie(s)");
+                break;
+            }
+            message.append("\n\n-> ").append(getContextualErrorMessage(manifest, error));
+            count++;
+        }
+        throw new SEDALibException(message.toString());
+    }
+
     /**
      * Check with xsd schema.
      *
@@ -188,14 +253,19 @@ public boolean checkWithXSDSchema(String manifest, Schema xmlSchema) throws SEDA
             xmlStreamReader = xmlInputFactory.createXMLStreamReader(bais, "UTF-8");
 
             final Validator validator = xmlSchema.newValidator();
-            validator.validate(new StAXSource(xmlStreamReader));
+            CollectingErrorHandler errorHandler = new CollectingErrorHandler();
+            validator.setErrorHandler(errorHandler);
+            try {
+                validator.validate(new StAXSource(xmlStreamReader));
+            } catch (SAXParseException e) {
+                // a fatal error stops the parsing, it's already collected
+            }
+            throwIfAnomalies(manifest, errorHandler);
             return true;
         } catch (IOException e) {
             throw new SEDALibException("Erreur d'accès au flux XML", e);
         } catch (XMLStreamException e) {
             throw new SEDALibException("Impossible d'ouvrir le flux XML", e);
-        } catch (SAXParseException e) {
-            throw new SEDALibException("Le flux XML n'est pas conforme\n-> " + getContextualErrorMessage(manifest, e));
         } catch (SAXException e) {
             throw new SEDALibException("Le flux XML n'est pas conforme", e);
         } finally {
@@ -220,10 +290,15 @@ public boolean checkWithXSDSchema(String manifest, Schema xmlSchema) throws SEDA
     public boolean checkWithRNGSchema(String manifest, Schema rngSchema) throws SEDALibException {
         try (ByteArrayInputStream bais = new ByteArrayInputStream(manifest.getBytes(StandardCharsets.UTF_8))) {
             final Validator validator = rngSchema.newValidator();
-            validator.validate(new StreamSource(bais));
+            CollectingErrorHandler errorHandler = new CollectingErrorHandler();
+            validator.setErrorHandler(errorHandler);
+            try {
+                validator.validate(new StreamSource(bais));
+            } catch (SAXParseException e) {
+                // a fatal error stops the parsing, it's already collected
+            }
+            throwIfAnomalies(manifest, errorHandler);
             return true;
-        } catch (SAXParseException e) {
-            throw new SEDALibException("Le flux XML n'est pas conforme\n-> " + getContextualErrorMessage(manifest, e));
         } catch (SAXException e) {
             throw new SEDALibException("Le flux XML n'est pas conforme", e);
         } catch (IOException e) {

sedalib/src/test/java/fr/gouv/vitam/tools/sedalib/inout/SEDAValidationTest.java

@@ -58,16 +58,27 @@
 import java.io.IOException;
 import java.nio.charset.StandardCharsets;
 
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
-import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
+import static org.assertj.core.api.Assertions.catchThrowable;
 import static org.junit.jupiter.api.Assertions.assertAll;
 
 @ExtendWith(SedaContextExtension.class)
 class SEDAValidationTest implements UseTestFiles {
 
     @Test
     void testSedaXmlValidationOK() throws IllegalArgumentException, SEDALibException, InterruptedException {
+        DiskToArchiveTransferImporter di = importSampleWithCompleteGlobalMetadata();
+
+        // validation
+        assertAll(() -> di.getArchiveTransfer().sedaSchemaValidate(null));
+    }
+
+    @Test
+    void testSedaXmlArchiveTransferGenerationAndValidationForSedaVersion2()
+        throws IllegalArgumentException, SEDALibException, InterruptedException {
         // do import of test directory
+        SedaContext.setVersion(SedaVersion.V2_2);
         DiskToArchiveTransferImporter di = new DiskToArchiveTransferImporter(
             "src/test/resources/PacketSamples/SampleWithLinksModelV2",
             null
@@ -95,6 +106,7 @@ void testSedaXmlValidationOK() throws IllegalArgumentException, SEDALibException
         "<DisseminationRuleCodeListVersion>DisseminationRuleCodeListVersion0</DisseminationRuleCodeListVersion>\n" +
         "<ReuseRuleCodeListVersion>ReuseRuleCodeListVersion0</ReuseRuleCodeListVersion>\n" +
         "<ClassificationRuleCodeListVersion>ClassificationRuleCodeListVersion0</ClassificationRuleCodeListVersion>\n" +
+        "<HoldRuleCodeListVersion>HoldRuleCodeListVersion0</HoldRuleCodeListVersion>\n" +
         "<AuthorizationReasonCodeListVersion>AuthorizationReasonCodeListVersion0</AuthorizationReasonCodeListVersion>\n" +
         "<RelationshipCodeListVersion>RelationshipCodeListVersion0</RelationshipCodeListVersion>\n" +
         "  </CodeListVersions>";
@@ -112,15 +124,28 @@ void testSedaXmlValidationOK() throws IllegalArgumentException, SEDALibException
         di.getArchiveTransfer().getGlobalMetadata().archivalAgencyIdentifier = "Identifier4";
         di.getArchiveTransfer().getGlobalMetadata().transferringAgencyIdentifier = "Identifier5";
 
+        ArchiveUnit au = di.getArchiveTransfer().getDataObjectPackage().getArchiveUnitById("ID11");
+        BinaryDataObject bdo = (BinaryDataObject) au.getTheDataObjectGroup().getBinaryDataObjectList().get(0);
+        bdo.addMetadata(new StringType("DataObjectProfile", "Test"));
+
         // validation
         assertAll(() -> di.getArchiveTransfer().sedaSchemaValidate(null));
+        try (
+            ByteArrayOutputStream baos = new ByteArrayOutputStream();
+            SEDAXMLStreamWriter xmlWriter = new SEDAXMLStreamWriter(baos, 2)
+        ) {
+            di.getArchiveTransfer().toSedaXml(xmlWriter, true, null);
+            String atout = baos.toString(StandardCharsets.UTF_8);
+            assertThat(atout).contains("fr:gouv:culture:archivesdefrance:seda:v2.2");
+            assertThat(atout).contains("DataObjectProfile");
+        } catch (XMLStreamException | IOException e) {
+            throw new RuntimeException(e);
+        }
+        SedaContext.setVersion(SedaVersion.V2_1);
     }
 
-    @Test
-    void testSedaXmlArchiveTransferGenerationAndValidationForSedaVersion2()
-        throws IllegalArgumentException, SEDALibException, InterruptedException {
-        // do import of test directory
-        SedaContext.setVersion(SedaVersion.V2_2);
+    private static DiskToArchiveTransferImporter importSampleWithCompleteGlobalMetadata()
+        throws SEDALibException, InterruptedException {
         DiskToArchiveTransferImporter di = new DiskToArchiveTransferImporter(
             "src/test/resources/PacketSamples/SampleWithLinksModelV2",
             null
@@ -148,7 +173,6 @@ void testSedaXmlArchiveTransferGenerationAndValidationForSedaVersion2()
         "<DisseminationRuleCodeListVersion>DisseminationRuleCodeListVersion0</DisseminationRuleCodeListVersion>\n" +
         "<ReuseRuleCodeListVersion>ReuseRuleCodeListVersion0</ReuseRuleCodeListVersion>\n" +
         "<ClassificationRuleCodeListVersion>ClassificationRuleCodeListVersion0</ClassificationRuleCodeListVersion>\n" +
-        "<HoldRuleCodeListVersion>HoldRuleCodeListVersion0</HoldRuleCodeListVersion>\n" +
         "<AuthorizationReasonCodeListVersion>AuthorizationReasonCodeListVersion0</AuthorizationReasonCodeListVersion>\n" +
         "<RelationshipCodeListVersion>RelationshipCodeListVersion0</RelationshipCodeListVersion>\n" +
         "  </CodeListVersions>";
@@ -165,25 +189,7 @@ void testSedaXmlArchiveTransferGenerationAndValidationForSedaVersion2()
             );
         di.getArchiveTransfer().getGlobalMetadata().archivalAgencyIdentifier = "Identifier4";
         di.getArchiveTransfer().getGlobalMetadata().transferringAgencyIdentifier = "Identifier5";
-
-        ArchiveUnit au = di.getArchiveTransfer().getDataObjectPackage().getArchiveUnitById("ID11");
-        BinaryDataObject bdo = (BinaryDataObject) au.getTheDataObjectGroup().getBinaryDataObjectList().get(0);
-        bdo.addMetadata(new StringType("DataObjectProfile", "Test"));
-
-        // validation
-        assertAll(() -> di.getArchiveTransfer().sedaSchemaValidate(null));
-        try (
-            ByteArrayOutputStream baos = new ByteArrayOutputStream();
-            SEDAXMLStreamWriter xmlWriter = new SEDAXMLStreamWriter(baos, 2)
-        ) {
-            di.getArchiveTransfer().toSedaXml(xmlWriter, true, null);
-            String atout = baos.toString(StandardCharsets.UTF_8);
-            assertThat(atout).contains("fr:gouv:culture:archivesdefrance:seda:v2.2");
-            assertThat(atout).contains("DataObjectProfile");
-        } catch (XMLStreamException | IOException e) {
-            throw new RuntimeException(e);
-        }
-        SedaContext.setVersion(SedaVersion.V2_1);
+        return di;
     }
 
     @Test
@@ -275,5 +281,66 @@ void testSedaRNGProfileValidationKO() throws IllegalArgumentException, SEDALibEx
             () -> si.getArchiveTransfer().sedaProfileValidate("src/test/resources/PacketSamples/profile.rng", null)
         ).hasMessageContaining("\"Title\" invalid; must be equal to \"Versement de la matrice cadastrale numérique\"");
     }
-    // TODO testWithXSD
+
+    /**
+     * The validator had no error handler, so it threw on the first anomaly: a non conformant manifest
+     * gave one anomaly at a time and had to be checked as many times as it had problems. Every
+     * anomaly is now collected in one pass.
+     */
+    @Test
+    void shouldReportEveryAnomalyAndNotOnlyTheFirst() throws Exception {
+        DiskToArchiveTransferImporter di = importSampleWithCompleteGlobalMetadata();
+        int emptied = 0;
+        for (ArchiveUnit au : di.getArchiveTransfer().getDataObjectPackage().getAuInDataObjectPackageIdMap().values()) {
+            au.setContentXmlData("");
+            if (++emptied == 2) break;
+        }
+
+        Throwable thrown = catchThrowable(() -> di.getArchiveTransfer().sedaSchemaValidate(null));
+
+        assertThat(thrown).isInstanceOf(SEDALibException.class);
+        assertThat(thrown.getMessage()).contains("2 anomalie(s) détectée(s)");
+    }
+
+    /**
+     * The same check replayed on the same package has to give the same anomalies, which is what the
+     * "résultats non reproductibles du contrôle au profil RNG" report denies.
+     */
+    @Test
+    void shouldGiveTheSameAnomaliesWhenTheProfileCheckIsReplayed() throws Exception {
+        TestUtilities.eraseAll("target/tmpJunit/KO_468_replay.zip-tmpdir");
+        SIPToArchiveTransferImporter si = new SIPToArchiveTransferImporter(
+            "src/test/resources/PacketSamples/KO_468.zip",
+            "target/tmpJunit/KO_468_replay.zip-tmpdir",
+            null
+        );
+        si.doImport();
+
+        Throwable first = catchThrowable(
+            () -> si.getArchiveTransfer().sedaProfileValidate("src/test/resources/PacketSamples/profile.rng", null)
+        );
+        Throwable second = catchThrowable(
+            () -> si.getArchiveTransfer().sedaProfileValidate("src/test/resources/PacketSamples/profile.rng", null)
+        );
+
+        assertThat(first).isNotNull();
+        assertThat(second).isNotNull();
+        assertThat(second.getMessage()).isEqualTo(first.getMessage());
+    }
+
+    /**
+     * Same requirement for the SEDA schema check.
+     */
+    @Test
+    void shouldGiveTheSameAnomaliesWhenTheSchemaCheckIsReplayed() throws Exception {
+        DiskToArchiveTransferImporter di = importSampleWithCompleteGlobalMetadata();
+        di.getArchiveTransfer().getDataObjectPackage().getArchiveUnitById("ID38").setContentXmlData("");
+
+        Throwable first = catchThrowable(() -> di.getArchiveTransfer().sedaSchemaValidate(null));
+        Throwable second = catchThrowable(() -> di.getArchiveTransfer().sedaSchemaValidate(null));
+
+        assertThat(first).isNotNull();
+        assertThat(second).isNotNull();
+        assertThat(second.getMessage()).isEqualTo(first.getMessage());
+    }
 }