diff --git a/sedalib/src/main/java/fr/gouv/vitam/tools/sedalib/utils/SEDALibProgressLogger.java b/sedalib/src/main/java/fr/gouv/vitam/tools/sedalib/utils/SEDALibProgressLogger.java
index 78356c99..62d36213 100644
--- a/sedalib/src/main/java/fr/gouv/vitam/tools/sedalib/utils/SEDALibProgressLogger.java
+++ b/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.
+ *
+ * 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();
}
}
}
diff --git a/sedalib/src/main/java/fr/gouv/vitam/tools/sedalib/xml/SEDAXMLValidator.java b/sedalib/src/main/java/fr/gouv/vitam/tools/sedalib/xml/SEDAXMLValidator.java
index 8434bea4..53853dd2 100644
--- a/sedalib/src/main/java/fr/gouv/vitam/tools/sedalib/xml/SEDAXMLValidator.java
+++ b/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.
+ *
+ * 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 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 getErrors() {
+ return errors;
+ }
+ }
+
+ private void throwIfAnomalies(String manifest, CollectingErrorHandler errorHandler) throws SEDALibException {
+ List 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) {
diff --git a/sedalib/src/test/java/fr/gouv/vitam/tools/sedalib/inout/SEDAValidationTest.java b/sedalib/src/test/java/fr/gouv/vitam/tools/sedalib/inout/SEDAValidationTest.java
index 0b172063..b8de10a4 100644
--- a/sedalib/src/test/java/fr/gouv/vitam/tools/sedalib/inout/SEDAValidationTest.java
+++ b/sedalib/src/test/java/fr/gouv/vitam/tools/sedalib/inout/SEDAValidationTest.java
@@ -58,8 +58,9 @@
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)
@@ -67,7 +68,17 @@ 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
"DisseminationRuleCodeListVersion0\n" +
"ReuseRuleCodeListVersion0\n" +
"ClassificationRuleCodeListVersion0\n" +
+ "HoldRuleCodeListVersion0\n" +
"AuthorizationReasonCodeListVersion0\n" +
"RelationshipCodeListVersion0\n" +
" ";
@@ -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()
"DisseminationRuleCodeListVersion0\n" +
"ReuseRuleCodeListVersion0\n" +
"ClassificationRuleCodeListVersion0\n" +
- "HoldRuleCodeListVersion0\n" +
"AuthorizationReasonCodeListVersion0\n" +
"RelationshipCodeListVersion0\n" +
" ";
@@ -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());
+ }
}
diff --git a/sedalib/src/test/java/fr/gouv/vitam/tools/sedalib/utils/SEDALibProgressLoggerTest.java b/sedalib/src/test/java/fr/gouv/vitam/tools/sedalib/utils/SEDALibProgressLoggerTest.java
new file mode 100644
index 00000000..e7448553
--- /dev/null
+++ b/sedalib/src/test/java/fr/gouv/vitam/tools/sedalib/utils/SEDALibProgressLoggerTest.java
@@ -0,0 +1,115 @@
+/**
+ * Copyright French Prime minister Office/SGMAP/DINSIC/Vitam Program (2019-2022)
+ * and the signatories of the "VITAM - Accord du Contributeur" agreement.
+ *
+ * contact@programmevitam.fr
+ *
+ * This software is a computer program whose purpose is to provide
+ * tools for construction and manipulation of SIP (Submission
+ * Information Package) conform to the SEDA (Standard d’Échange
+ * de données pour l’Archivage) standard.
+ *
+ * This software is governed by the CeCILL-C license under French law and
+ * abiding by the rules of distribution of free software. You can use,
+ * modify and/ or redistribute the software under the terms of the CeCILL-C
+ * license as circulated by CEA, CNRS and INRIA at the following URL
+ * "http://www.cecill.info".
+ *
+ * As a counterpart to the access to the source code and rights to copy,
+ * modify and redistribute granted by the license, users are provided only
+ * with a limited warranty and the software's author, the holder of the
+ * economic rights, and the successive licensors have only limited
+ * liability.
+ *
+ * In this respect, the user's attention is drawn to the risks associated
+ * with loading, using, modifying and/or developing or reproducing the
+ * software by the user in light of its specific status of free software,
+ * that may mean that it is complicated to manipulate, and that also
+ * therefore means that it is reserved for developers and experienced
+ * professionals having in-depth computer knowledge. Users are therefore
+ * encouraged to load and test the software's suitability as regards their
+ * requirements in conditions enabling the security of their systems and/or
+ * data to be ensured and, more generally, to use and operate it in the
+ * same conditions as regards security.
+ *
+ * The fact that you are presently reading this means that you have had
+ * knowledge of the CeCILL-C license and that you accept its terms.
+ */
+package fr.gouv.vitam.tools.sedalib.utils;
+
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Test;
+
+import static fr.gouv.vitam.tools.sedalib.utils.SEDALibProgressLogger.OBJECTS;
+import static fr.gouv.vitam.tools.sedalib.utils.SEDALibProgressLogger.OBJECTS_GROUP;
+import static org.junit.jupiter.api.Assertions.*;
+
+class SEDALibProgressLoggerTest {
+
+ private static final int FILTERED_OUT_CALLS = 20000;
+
+ @AfterEach
+ void clearInterruptedFlag() {
+ Thread.interrupted();
+ }
+
+ /**
+ * Non regression on the import duration: doProgressLog used to end with a Thread.sleep(1) placed
+ * outside of the log level test, so every unzipped file, DataObjectGroup, BinaryDataObject and
+ * ArchiveUnit cost at least one sleep even when the message was filtered out and displayed
+ * nowhere. On Windows, where a 1ms sleep really lasts up to 15.6ms, that alone turned the import
+ * of a large SIP into a half hour of pure sleeping, during which the Traiter and Export menus
+ * stayed greyed out.
+ */
+ @Test
+ void shouldNotWaitOnMessagesFilteredOutByLogLevel() throws InterruptedException {
+ SEDALibProgressLogger spl = new SEDALibProgressLogger(null, OBJECTS_GROUP);
+
+ long start = System.nanoTime();
+ for (int i = 0; i < FILTERED_OUT_CALLS; i++) {
+ SEDALibProgressLogger.doProgressLog(spl, OBJECTS, "sedalib: objet [" + i + "] importé", null);
+ }
+ long durationMs = (System.nanoTime() - start) / 1_000_000;
+
+ // one sleep per call would be at least 20s here, and around 5mn on Windows
+ assertTrue(
+ durationMs < 2000,
+ FILTERED_OUT_CALLS + " appels filtrés ont pris " + durationMs + "ms, un délai est réapparu par appel"
+ );
+ }
+
+ @Test
+ void shouldThrowWhenThreadIsInterrupted() {
+ SEDALibProgressLogger spl = new SEDALibProgressLogger(null, OBJECTS_GROUP);
+
+ Thread.currentThread().interrupt();
+
+ assertThrows(
+ InterruptedException.class,
+ () -> SEDALibProgressLogger.doProgressLog(spl, OBJECTS, "sedalib: objet importé", null)
+ );
+ assertFalse(Thread.currentThread().isInterrupted(), "le drapeau d'interruption doit avoir été consommé");
+ }
+
+ @Test
+ void shouldThrowFromStepLogWhenThreadIsInterrupted() {
+ SEDALibProgressLogger spl = new SEDALibProgressLogger(null, OBJECTS_GROUP, (count, log) -> {}, 1);
+
+ Thread.currentThread().interrupt();
+
+ assertThrows(
+ InterruptedException.class,
+ () -> SEDALibProgressLogger.doProgressLogIfStep(spl, OBJECTS_GROUP, 1, "1 fichier extrait")
+ );
+ assertFalse(Thread.currentThread().isInterrupted(), "le drapeau d'interruption doit avoir été consommé");
+ }
+
+ @Test
+ void shouldNotThrowWhenThreadIsNotInterrupted() {
+ SEDALibProgressLogger spl = new SEDALibProgressLogger(null, OBJECTS_GROUP);
+
+ assertDoesNotThrow(
+ () -> SEDALibProgressLogger.doProgressLog(spl, OBJECTS_GROUP, "sedalib: import terminé", null)
+ );
+ }
+}