From 7418d2f2fbc340cf9149f7827e736abc46a89dfd Mon Sep 17 00:00:00 2001 From: Daniel Radeau Date: Thu, 10 Sep 2026 11:59:10 +0200 Subject: [PATCH 1/2] =?UTF-8?q?bugs=20#16842=20#16843=20#16845=20fix:=20im?= =?UTF-8?q?port=20interminable,=20une=20attente=20d'1ms=20par=20objet=20im?= =?UTF-8?q?port=C3=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit doProgressLog terminait par un Thread.sleep(1) placé hors du test de niveau de log, ce dernier étant fait à l'intérieur de doProgressLogWithoutInterruption. L'attente s'exécutait donc à chaque appel, y compris quand le message était filtré et n'était affiché nulle part. Or ce doProgressLog est appelé au niveau OBJECTS dans les boucles chaudes de l'import, alors que ImportThread règle le niveau à OBJECTS_GROUP en mode normal: une attente par fichier extrait du zip, par DataObjectGroup, par BinaryDataObject, par PhysicalDataObject et par ArchiveUnit. Sur un SIP de 50 000 AU et 50 000 binaires cela fait environ 200 000 attentes, soit 4 mn sous Linux et jusqu'à 50 mn sous Windows où un sleep d'1ms dure réellement 15,6ms. Pendant tout ce temps les menus Traiter et Export restent grisés, puisque setContextLoaded n'est appelé que depuis ImportThread.done(), d'où l'impossibilité de vérifier la conformité SEDA, de vérifier la conformité à un profil et d'exporter le SIP dans la demi-heure suivant l'ouverture. - SEDALibProgressLogger: les trois Thread.sleep(1) sont remplacés par checkInterruption(), qui lève InterruptedException si le drapeau d'interruption est posé. La sémantique d'annulation est identique, Thread.sleep ne faisant rien d'autre sur interruption, mais le coût est nul. - SEDALibProgressLoggerTest: non-régression sur la durée des appels filtrés (20 000 appels sous 2s, contre 23s avant correctif) et sur la propagation de l'annulation par doProgressLog et doProgressLogIfStep. Ce correctif couvre les trois tickets ouverts sur le même symptôme: #16842 pour la vérification de conformité au SEDA 2.1, #16843 pour la vérification de conformité au profil RNG des AN et #16845 pour l'export du SIP. --- .../sedalib/utils/SEDALibProgressLogger.java | 22 +++- .../utils/SEDALibProgressLoggerTest.java | 115 ++++++++++++++++++ 2 files changed, 133 insertions(+), 4 deletions(-) create mode 100644 sedalib/src/test/java/fr/gouv/vitam/tools/sedalib/utils/SEDALibProgressLoggerTest.java 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/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) + ); + } +} From edef06655e12cc97496c1e9c459b38cb58a71745 Mon Sep 17 00:00:00 2001 From: Daniel Radeau Date: Thu, 10 Sep 2026 12:39:49 +0200 Subject: [PATCH 2/2] =?UTF-8?q?bug=20#16846=20fix:=20binaires=20manquants?= =?UTF-8?q?=20=C3=A0=20l'export=20alors=20que=20les=20m=C3=A9tadonn=C3=A9e?= =?UTF-8?q?s=20sont=20l=C3=A0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Un BinaryDataObject porte d'un côté ses métadonnées et de l'autre le chemin du fichier qui contient son contenu, et rien ne garantissait que les deux soient d'accord. Le désaccord n'était constaté qu'à l'export, sur le premier fichier introuvable, avec un message ne nommant que le SIP en cours d'écriture. Cause identifiée, et reproduite par les tests sur le SIP d'exemple: le chemin sur disque était construit à partir de l'Uri brute du manifest, alors que les importeurs SIP et DIP extraient tout dans un répertoire "content" en minuscules. Un manifest déclarant "Content/ID13.txt" produisait donc un BinaryDataObject pointant sur un répertoire inexistant, et tous ses binaires devenaient inatteignables tandis que leurs métadonnées étaient bien présentes. Le défaut reste invisible sous Windows, dont le système de fichiers est insensible à la casse, et se manifeste partout ailleurs. - BinaryDataObject.normalizePackageUri: la normalisation du répertoire de paquet est mise en commun, et appliquée là où le chemin sur disque est construit (DataObjectPackage, DataObjectGroup) comme à la décompression (importeurs SIP et DIP), pour que les deux ne puissent plus diverger. - DataObjectPackage.getUnreadableBinaryDataObjectDescriptions et verifyBinaryDataObjectFilesAreReadable: recensement des BinaryDataObjects sans fichier associé, ou dont le fichier est absent ou illisible. - ArchiveTransferToSIPExporter, DataObjectPackageToDiskExporter: contrôle avant d'écrire quoi que ce soit. Tous les fichiers manquants sont nommés en une seule passe, au lieu d'un export à refaire autant de fois qu'il en manque, et rien n'est écrit plutôt que de laisser un SIP tronqué. - SIPToArchiveTransferImporter: l'import signale les binaires déclarés au manifest et absents du paquet, dans le journal et dans le récapitulatif, pour que l'écart soit rattaché au paquet source et non découvert à l'export. - DiskToDataObjectPackageImporter, DiskToArchiveTransferImporter: un fichier écarté par un motif d'exclusion était ignoré sans une ligne de journal; il est désormais tracé et compté dans le récapitulatif. - Tests: SIP complet sans anomalie, binaire supprimé détecté et nommé, export refusé sans rien écrire, Uri à casse variable, et comptage des fichiers exclus. --- .../tools/sedalib/core/BinaryDataObject.java | 23 +++ .../tools/sedalib/core/DataObjectGroup.java | 4 +- .../tools/sedalib/core/DataObjectPackage.java | 78 ++++++++- .../ArchiveTransferToSIPExporter.java | 20 +++ .../DataObjectPackageToDiskExporter.java | 8 + ...ToArchiveDeliveryRequestReplyImporter.java | 7 +- .../DiskToArchiveTransferImporter.java | 3 + .../DiskToDataObjectPackageImporter.java | 24 +++ .../SIPToArchiveTransferImporter.java | 47 +++++- .../sedalib/core/BinaryDataObjectUriTest.java | 68 ++++++++ .../inout/UnreadableBinaryDataObjectTest.java | 159 ++++++++++++++++++ 11 files changed, 433 insertions(+), 8 deletions(-) create mode 100644 sedalib/src/test/java/fr/gouv/vitam/tools/sedalib/core/BinaryDataObjectUriTest.java create mode 100644 sedalib/src/test/java/fr/gouv/vitam/tools/sedalib/inout/UnreadableBinaryDataObjectTest.java diff --git a/sedalib/src/main/java/fr/gouv/vitam/tools/sedalib/core/BinaryDataObject.java b/sedalib/src/main/java/fr/gouv/vitam/tools/sedalib/core/BinaryDataObject.java index 87347a69..655b792e 100644 --- a/sedalib/src/main/java/fr/gouv/vitam/tools/sedalib/core/BinaryDataObject.java +++ b/sedalib/src/main/java/fr/gouv/vitam/tools/sedalib/core/BinaryDataObject.java @@ -370,6 +370,29 @@ private String undefined(String a) { // SEDA XML exporter + /** + * The length of the "content" package directory name. + */ + private static final int CONTENT_PREFIX_LENGTH = 7; + + /** + * Normalizes the package directory of an Uri declared in a manifest. + *

+ * A manifest may name the binary directory in any case, "Content/ID13.txt" as well as + * "content/ID13.txt", and the SIP and DIP importers extract all of them into a lowercase + * "content" directory. The on disk path built from the Uri has to be normalized the same way, + * otherwise it points at a directory that doesn't exist on a case sensitive file system and the + * binary is unreachable while its metadata is there. + * + * @param uri the uri as declared in the manifest + * @return the uri with a lowercase content package directory + */ + public static String normalizePackageUri(String uri) { + if (uri == null) return null; + if (uri.toLowerCase().startsWith("content")) return "content" + uri.substring(CONTENT_PREFIX_LENGTH); + return uri; + } + private void finalizeUri() throws SEDALibException { FileInfo fileInfo = getMetadataFileInfo(); String tmpUri = "content/" + inDataPackageObjectId; diff --git a/sedalib/src/main/java/fr/gouv/vitam/tools/sedalib/core/DataObjectGroup.java b/sedalib/src/main/java/fr/gouv/vitam/tools/sedalib/core/DataObjectGroup.java index ee850f2f..c311ea3c 100644 --- a/sedalib/src/main/java/fr/gouv/vitam/tools/sedalib/core/DataObjectGroup.java +++ b/sedalib/src/main/java/fr/gouv/vitam/tools/sedalib/core/DataObjectGroup.java @@ -291,7 +291,9 @@ public static String idFromSedaXml( SedaContext.getVersion() ); StringType bdoUri = (StringType) bdo.getFirstNamedMetadata("Uri"); - bdo.setOnDiskPathFromString(rootDir + File.separator + bdoUri.getValue()); + bdo.setOnDiskPathFromString( + rootDir + File.separator + BinaryDataObject.normalizePackageUri(bdoUri.getValue()) + ); dog.addDataObject(bdo); break; case "PhysicalDataObject": diff --git a/sedalib/src/main/java/fr/gouv/vitam/tools/sedalib/core/DataObjectPackage.java b/sedalib/src/main/java/fr/gouv/vitam/tools/sedalib/core/DataObjectPackage.java index d2cf0c66..5f7dac77 100644 --- a/sedalib/src/main/java/fr/gouv/vitam/tools/sedalib/core/DataObjectPackage.java +++ b/sedalib/src/main/java/fr/gouv/vitam/tools/sedalib/core/DataObjectPackage.java @@ -38,6 +38,7 @@ package fr.gouv.vitam.tools.sedalib.core; import fr.gouv.vitam.tools.sedalib.metadata.content.Content; +import fr.gouv.vitam.tools.sedalib.metadata.data.FileInfo; import fr.gouv.vitam.tools.sedalib.metadata.namedtype.IntegerType; import fr.gouv.vitam.tools.sedalib.utils.SEDALibException; import fr.gouv.vitam.tools.sedalib.utils.SEDALibProgressLogger; @@ -46,6 +47,8 @@ import javax.xml.stream.XMLStreamException; import java.io.File; +import java.nio.file.Files; +import java.nio.file.Path; import java.text.DecimalFormat; import java.util.*; import java.util.Map.Entry; @@ -1216,7 +1219,11 @@ public static void importDataObjectPackageObjects( break; case "BinaryDataObject": bdo = BinaryDataObject.fromSedaXml(xmlReader, dataObjectPackage, sedaLibProgressLogger); - bdo.setOnDiskPathFromString(rootDir + File.separator + bdo.getMetadataUri().getValue()); + bdo.setOnDiskPathFromString( + rootDir + + File.separator + + BinaryDataObject.normalizePackageUri(bdo.getMetadataUri().getValue()) + ); doProgressLog( sedaLibProgressLogger, SEDALibProgressLogger.OBJECTS, @@ -1551,6 +1558,75 @@ public void setManagementMetadataXmlData(String managementMetadataXmlData) { this.managementMetadataXmlData = managementMetadataXmlData; } + /** + * Lists the BinaryDataObjects whose binary file can't be read on disk. + *

+ * A BinaryDataObject carries its metadata and, apart from them, the path of the file holding its + * content. Nothing guarantees that both agree: a SIP import sets that path from the Uri declared + * in the manifest without verifying that the file was indeed in the package, and a work can be + * saved and reloaded long after the imported files have moved. The descriptive metadata is then + * complete while the binary is nowhere to be found, which is only discovered when the export + * tries to open it. + * + * @return one description per unreadable BinaryDataObject, empty if all of them can be read + */ + private static final int MAX_LISTED_UNREADABLE = 20; + + public List getUnreadableBinaryDataObjectDescriptions() { + List descriptions = new ArrayList<>(); + for (DataObjectGroup dog : dogInDataObjectPackageIdMap.values()) { + if (dog.getBinaryDataObjectList() == null) continue; + for (BinaryDataObject bdo : dog.getBinaryDataObjectList()) { + String problem = getUnreadableDescription(bdo); + if (problem != null) descriptions.add(problem); + } + } + return descriptions; + } + + /** + * Verifies that every BinaryDataObject file can be read, and tells which ones can't if any. + * + * @throws SEDALibException if at least one BinaryDataObject file is missing or unreadable, with + * all of them listed in the message + */ + public void verifyBinaryDataObjectFilesAreReadable() throws SEDALibException { + List problems = getUnreadableBinaryDataObjectDescriptions(); + if (problems.isEmpty()) return; + + StringBuilder message = new StringBuilder( + problems.size() + + " fichier(s) binaire(s) sont introuvables ou illisibles alors que leurs métadonnées sont présentes:" + ); + for (String problem : problems.subList(0, Math.min(problems.size(), MAX_LISTED_UNREADABLE))) { + message.append("\n - ").append(problem); + } + if (problems.size() > MAX_LISTED_UNREADABLE) { + message + .append("\n - ... et ") + .append(problems.size() - MAX_LISTED_UNREADABLE) + .append(" autre(s), voir le journal pour la liste complète"); + } + throw new SEDALibException(message.toString()); + } + + private static String getUnreadableDescription(BinaryDataObject bdo) { + String filename = null; + FileInfo fileInfo = bdo.getMetadataFileInfo(); + if (fileInfo != null) filename = fileInfo.getSimpleMetadata("Filename"); + String identification = + "BinaryDataObject [" + + bdo.getInDataObjectPackageId() + + "]" + + (filename == null ? "" : " [" + filename + "]"); + + Path path = bdo.getOnDiskPath(); + if (path == null) return identification + " n'a pas de fichier associé"; + if (!Files.exists(path)) return identification + " a pour fichier [" + path + "] qui n'existe pas"; + if (!Files.isReadable(path)) return identification + " a pour fichier [" + path + "] qui n'est pas lisible"; + return null; + } + /** * Gets export metadata list. * diff --git a/sedalib/src/main/java/fr/gouv/vitam/tools/sedalib/inout/exporter/ArchiveTransferToSIPExporter.java b/sedalib/src/main/java/fr/gouv/vitam/tools/sedalib/inout/exporter/ArchiveTransferToSIPExporter.java index cb70695f..e87e2add 100644 --- a/sedalib/src/main/java/fr/gouv/vitam/tools/sedalib/inout/exporter/ArchiveTransferToSIPExporter.java +++ b/sedalib/src/main/java/fr/gouv/vitam/tools/sedalib/inout/exporter/ArchiveTransferToSIPExporter.java @@ -191,6 +191,24 @@ public String getSEDAXMLManifest(boolean hierarchicalFlag, boolean indentedFlag) return result; } + /** + * Verifies that every BinaryDataObject file can be read before writing anything. + *

+ * Without it the export stops on the first unreadable file, leaving a truncated SIP behind and + * naming only that one file, so a package missing many binaries has to be exported as many times + * to discover them all. All of them are listed here in one pass, and the SIP is not started at + * all when one is missing. + * + * @throws SEDALibException if at least one BinaryDataObject file is missing or unreadable + */ + private void verifyAllBinaryDataObjectFilesAreReadable() throws SEDALibException { + try { + archiveTransfer.getDataObjectPackage().verifyBinaryDataObjectFilesAreReadable(); + } catch (SEDALibException e) { + throw new SEDALibException("Export du SIP impossible, " + e.getMessage()); + } + } + /** * Do export the ArchiveTransfer to SEDA Submission Information Packet (SIP). * @@ -215,6 +233,8 @@ public void doExportToSEDASIP(String fileName, boolean hierarchicalFlag, boolean this.indentedFlag = indentedFlag; this.manifestOnly = false; + verifyAllBinaryDataObjectFilesAreReadable(); + try { Files.createDirectories(Paths.get(fileName).toAbsolutePath().getParent()); } catch (IOException e1) { diff --git a/sedalib/src/main/java/fr/gouv/vitam/tools/sedalib/inout/exporter/DataObjectPackageToDiskExporter.java b/sedalib/src/main/java/fr/gouv/vitam/tools/sedalib/inout/exporter/DataObjectPackageToDiskExporter.java index 3259cc27..bf6821c0 100644 --- a/sedalib/src/main/java/fr/gouv/vitam/tools/sedalib/inout/exporter/DataObjectPackageToDiskExporter.java +++ b/sedalib/src/main/java/fr/gouv/vitam/tools/sedalib/inout/exporter/DataObjectPackageToDiskExporter.java @@ -596,6 +596,14 @@ private void exportArchiveUnit(ArchiveUnit au, Path containerPath) throws SEDALi * @throws InterruptedException if export process is interrupted */ public void doExport(String directoryName) throws SEDALibException, InterruptedException { + // all the unreadable binaries are named at once, and nothing is written when there is one, + // rather than stopping on the first one with a half written hierarchy left behind + try { + dataObjectPackage.verifyBinaryDataObjectFilesAreReadable(); + } catch (SEDALibException e) { + throw new SEDALibException("Export sur disque impossible, " + e.getMessage()); + } + Path exportPath = Paths.get(directoryName); try { Files.createDirectories(exportPath); diff --git a/sedalib/src/main/java/fr/gouv/vitam/tools/sedalib/inout/importer/DIPToArchiveDeliveryRequestReplyImporter.java b/sedalib/src/main/java/fr/gouv/vitam/tools/sedalib/inout/importer/DIPToArchiveDeliveryRequestReplyImporter.java index e6216859..ff2f6394 100644 --- a/sedalib/src/main/java/fr/gouv/vitam/tools/sedalib/inout/importer/DIPToArchiveDeliveryRequestReplyImporter.java +++ b/sedalib/src/main/java/fr/gouv/vitam/tools/sedalib/inout/importer/DIPToArchiveDeliveryRequestReplyImporter.java @@ -38,6 +38,7 @@ package fr.gouv.vitam.tools.sedalib.inout.importer; import fr.gouv.vitam.tools.sedalib.core.ArchiveDeliveryRequestReply; +import fr.gouv.vitam.tools.sedalib.core.BinaryDataObject; import fr.gouv.vitam.tools.sedalib.utils.SEDALibException; import fr.gouv.vitam.tools.sedalib.utils.SEDALibProgressLogger; import fr.gouv.vitam.tools.sedalib.xml.SEDAXMLEventReader; @@ -122,9 +123,9 @@ public String unZipDip(String zipFile, String outputFolder) throws SEDALibExcept ArchiveEntry ze; while ((ze = zais.getNextEntry()) != null) { String fileName = ze.getName().trim(); - // change any case ConTenT to lowercase content on import as in fromSEDA in - // BinaryDataObject - if (fileName.toLowerCase().startsWith("content")) fileName = "content" + fileName.substring(7); + // change any case ConTenT to lowercase content on import, the on disk path built + // from the manifest Uri is normalized the same way + fileName = BinaryDataObject.normalizePackageUri(fileName); Path newPath = Paths.get(outputFolder + File.separator + fileName); diff --git a/sedalib/src/main/java/fr/gouv/vitam/tools/sedalib/inout/importer/DiskToArchiveTransferImporter.java b/sedalib/src/main/java/fr/gouv/vitam/tools/sedalib/inout/importer/DiskToArchiveTransferImporter.java index 6361e24a..5b298c9f 100644 --- a/sedalib/src/main/java/fr/gouv/vitam/tools/sedalib/inout/importer/DiskToArchiveTransferImporter.java +++ b/sedalib/src/main/java/fr/gouv/vitam/tools/sedalib/inout/importer/DiskToArchiveTransferImporter.java @@ -346,6 +346,9 @@ public String getSummary() { result += "encodé selon un modèle hybride V1/V2 de la structure\n"; break; } + if (diskToDataObjectPackageImporter.getIgnoredFileCount() > 0) result += + diskToDataObjectPackageImporter.getIgnoredFileCount() + + " fichier(s) ignoré(s) car correspondant à un motif d'exclusion, voir le journal\n"; if ((start != null) && (end != null)) result += "chargé en " + Duration.between(start, end).toString().substring(2) + "\n"; return result; diff --git a/sedalib/src/main/java/fr/gouv/vitam/tools/sedalib/inout/importer/DiskToDataObjectPackageImporter.java b/sedalib/src/main/java/fr/gouv/vitam/tools/sedalib/inout/importer/DiskToDataObjectPackageImporter.java index ee57e205..3f54b734 100644 --- a/sedalib/src/main/java/fr/gouv/vitam/tools/sedalib/inout/importer/DiskToDataObjectPackageImporter.java +++ b/sedalib/src/main/java/fr/gouv/vitam/tools/sedalib/inout/importer/DiskToDataObjectPackageImporter.java @@ -202,6 +202,20 @@ public class DiskToDataObjectPackageImporter { */ private int inCounter; + /** + * The number of files skipped because they match one of the ignore patterns. + */ + private int ignoredFileCount; + + /** + * Gets the number of files skipped because they match one of the ignore patterns. + * + * @return the ignored file count + */ + public int getIgnoredFileCount() { + return ignoredFileCount; + } + /** * The start and end instants, for duration computation. */ @@ -245,6 +259,7 @@ private DiskToDataObjectPackageImporter( else this.extractTitleFromFileNameFunction = simpleCopy; this.inCounter = 0; + this.ignoredFileCount = 0; this.sedaLibProgressLogger = sedaLibProgressLogger; } @@ -698,6 +713,15 @@ private ArchiveUnit processDirectory(Path path) throws SEDALibException, Interru fileName = curPath.getFileName().toString(); if (!Files.isDirectory(curPath) && mustBeIgnored(fileName)) { + // an ignored file used to disappear without a word, so a package silently missing + // binaries gave the user nothing to go on + ignoredFileCount++; + doProgressLog( + sedaLibProgressLogger, + SEDALibProgressLogger.OBJECTS, + "sedalib: fichier [" + curPathString + "] ignoré, il correspond à un motif d'exclusion", + null + ); continue; } else if (analyzeLink(curPath)) { if (noLinkFlag) continue; diff --git a/sedalib/src/main/java/fr/gouv/vitam/tools/sedalib/inout/importer/SIPToArchiveTransferImporter.java b/sedalib/src/main/java/fr/gouv/vitam/tools/sedalib/inout/importer/SIPToArchiveTransferImporter.java index c71b1ecb..d5beead7 100644 --- a/sedalib/src/main/java/fr/gouv/vitam/tools/sedalib/inout/importer/SIPToArchiveTransferImporter.java +++ b/sedalib/src/main/java/fr/gouv/vitam/tools/sedalib/inout/importer/SIPToArchiveTransferImporter.java @@ -38,6 +38,7 @@ package fr.gouv.vitam.tools.sedalib.inout.importer; import fr.gouv.vitam.tools.sedalib.core.ArchiveTransfer; +import fr.gouv.vitam.tools.sedalib.core.BinaryDataObject; import fr.gouv.vitam.tools.sedalib.utils.SEDALibException; import fr.gouv.vitam.tools.sedalib.utils.SEDALibProgressLogger; import fr.gouv.vitam.tools.sedalib.xml.SEDAXMLEventReader; @@ -57,6 +58,7 @@ import java.time.Duration; import java.time.Instant; import java.util.Date; +import java.util.List; import static fr.gouv.vitam.tools.sedalib.utils.SEDALibProgressLogger.doProgressLog; import static fr.gouv.vitam.tools.sedalib.utils.SEDALibProgressLogger.doProgressLogIfStep; @@ -84,6 +86,11 @@ public class SIPToArchiveTransferImporter { */ private ArchiveTransfer archiveTransfer; + /** + * The number of BinaryDataObjects of the manifest whose file was not in the package. + */ + private int unreadableBinaryDataObjectCount; + /** * The end. */ @@ -121,9 +128,9 @@ public String unZipSip(String zipFile, String outputFolder) throws SEDALibExcept ArchiveEntry ze; while ((ze = zais.getNextEntry()) != null) { String fileName = ze.getName().trim(); - // change any case ConTenT to lowercase content on import as in fromSEDA in - // BinaryDataObject - if (fileName.toLowerCase().startsWith("content")) fileName = "content" + fileName.substring(7); + // change any case ConTenT to lowercase content on import, the on disk path built + // from the manifest Uri is normalized the same way + fileName = BinaryDataObject.normalizePackageUri(fileName); Path newPath = Paths.get(outputFolder + File.separator + fileName); @@ -244,10 +251,40 @@ public void doImport() throws SEDALibException, InterruptedException { throw new SEDALibException("Impossible d'importer le fichier [" + manifest + "] comme manifest du SIP", e); } + logUnreadableBinaryDataObjects(); + end = Instant.now(); doProgressLog(sedaLibProgressLogger, SEDALibProgressLogger.GLOBAL, "sedalib: import du SIP terminé", null); } + /** + * Warns about the BinaryDataObjects of the manifest whose file is not in the package. + *

+ * The on disk path of a BinaryDataObject is built from the Uri declared in the manifest, without + * checking that the file was really there. An incomplete SIP is thus imported without a word, and + * the missing binaries are only discovered much later, when the export fails on them. Saying it at + * import time is what lets the discrepancy be traced back to the source package. + */ + private void logUnreadableBinaryDataObjects() throws InterruptedException { + List problems = archiveTransfer.getDataObjectPackage().getUnreadableBinaryDataObjectDescriptions(); + if (problems.isEmpty()) return; + + unreadableBinaryDataObjectCount = problems.size(); + doProgressLog( + sedaLibProgressLogger, + SEDALibProgressLogger.GLOBAL, + "sedalib: attention, " + + problems.size() + + " fichier(s) binaire(s) déclaré(s) dans le manifest sont absents du SIP [" + + zipFile + + "], les métadonnées sont importées mais l'export échouera tant qu'ils manqueront", + null + ); + for (String problem : problems) { + doProgressLog(sedaLibProgressLogger, SEDALibProgressLogger.STEP, "sedalib: " + problem, null); + } + } + /** * Gets the archive transfer. * @@ -266,6 +303,10 @@ public String getSummary() { String result; result = archiveTransfer.getDescription() + "\n"; + if (unreadableBinaryDataObjectCount > 0) result += + "attention, " + + unreadableBinaryDataObjectCount + + " fichier(s) binaire(s) déclaré(s) dans le manifest sont absents du SIP, voir le journal\n"; if (start != null) result += "chargé en " + Duration.between(start, end).toString().substring(2) + "\n"; return result; } diff --git a/sedalib/src/test/java/fr/gouv/vitam/tools/sedalib/core/BinaryDataObjectUriTest.java b/sedalib/src/test/java/fr/gouv/vitam/tools/sedalib/core/BinaryDataObjectUriTest.java new file mode 100644 index 00000000..27e974f9 --- /dev/null +++ b/sedalib/src/test/java/fr/gouv/vitam/tools/sedalib/core/BinaryDataObjectUriTest.java @@ -0,0 +1,68 @@ +/** + * 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.core; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; + +class BinaryDataObjectUriTest { + + @Test + void shouldLowercaseTheContentPackageDirectoryWhateverItsCase() { + assertEquals("content/ID13.txt", BinaryDataObject.normalizePackageUri("Content/ID13.txt")); + assertEquals("content/ID13.txt", BinaryDataObject.normalizePackageUri("CONTENT/ID13.txt")); + assertEquals("content/ID13.txt", BinaryDataObject.normalizePackageUri("ConTenT/ID13.txt")); + } + + @Test + void shouldLeaveAnAlreadyLowercaseUriUntouched() { + assertEquals("content/ID13.txt", BinaryDataObject.normalizePackageUri("content/ID13.txt")); + } + + @Test + void shouldLeaveAnUriOutsideTheContentDirectoryUntouched() { + assertEquals("Binary/ID13.txt", BinaryDataObject.normalizePackageUri("Binary/ID13.txt")); + } + + @Test + void shouldHandleANullUri() { + assertNull(BinaryDataObject.normalizePackageUri(null)); + } +} diff --git a/sedalib/src/test/java/fr/gouv/vitam/tools/sedalib/inout/UnreadableBinaryDataObjectTest.java b/sedalib/src/test/java/fr/gouv/vitam/tools/sedalib/inout/UnreadableBinaryDataObjectTest.java new file mode 100644 index 00000000..4d5ca3d9 --- /dev/null +++ b/sedalib/src/test/java/fr/gouv/vitam/tools/sedalib/inout/UnreadableBinaryDataObjectTest.java @@ -0,0 +1,159 @@ +/** + * 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.inout; + +import fr.gouv.vitam.tools.sedalib.SedaContextExtension; +import fr.gouv.vitam.tools.sedalib.TestUtilities; +import fr.gouv.vitam.tools.sedalib.UseTestFiles; +import fr.gouv.vitam.tools.sedalib.core.BinaryDataObject; +import fr.gouv.vitam.tools.sedalib.core.DataObjectGroup; +import fr.gouv.vitam.tools.sedalib.inout.exporter.ArchiveTransferToSIPExporter; +import fr.gouv.vitam.tools.sedalib.inout.importer.DiskToArchiveTransferImporter; +import fr.gouv.vitam.tools.sedalib.inout.importer.SIPToArchiveTransferImporter; +import fr.gouv.vitam.tools.sedalib.utils.SEDALibException; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; + +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +/** + * A BinaryDataObject carries its metadata on one side and the path of its binary file on the other, + * and nothing used to guarantee that both agree. A SIP whose manifest declares a file absent from the + * package was imported without a word, and the discrepancy only surfaced at export time, as a failure + * on the first missing file naming nothing but the SIP being written. That is the "export incomplet, + * binaires manquants alors que les métadonnées descriptives sont présentes" report. + */ +@ExtendWith(SedaContextExtension.class) +class UnreadableBinaryDataObjectTest implements UseTestFiles { + + private static SIPToArchiveTransferImporter importSampleSip(String tmpDir) throws Exception { + TestUtilities.eraseAll(tmpDir); + SIPToArchiveTransferImporter si = new SIPToArchiveTransferImporter( + "src/test/resources/PacketSamples/SIP_OK.zip", + tmpDir, + null + ); + si.doImport(); + return si; + } + + private static BinaryDataObject firstBinaryDataObject(SIPToArchiveTransferImporter si) { + for (DataObjectGroup dog : si + .getArchiveTransfer() + .getDataObjectPackage() + .getDogInDataObjectPackageIdMap() + .values()) { + if ((dog.getBinaryDataObjectList() != null) && !dog.getBinaryDataObjectList().isEmpty()) { + return dog.getBinaryDataObjectList().get(0); + } + } + throw new IllegalStateException("le SIP de test n'a aucun BinaryDataObject"); + } + + @Test + void shouldReportNoUnreadableBinaryDataObjectOnACompleteSip() throws Exception { + SIPToArchiveTransferImporter si = importSampleSip("target/tmpJunit/UnreadableBDO-complete"); + + assertThat( + si.getArchiveTransfer().getDataObjectPackage().getUnreadableBinaryDataObjectDescriptions() + ).isEmpty(); + } + + /** + * SIP_OK.zip declares its binaries as "Content/ID13.txt" while the importer extracts them into a + * lowercase "content" directory. The on disk path was built from the raw Uri, so on a case + * sensitive file system every binary of such a SIP was unreachable, with its metadata present. + */ + @Test + void shouldFindTheBinaryFilesWhenTheManifestUriIsCapitalized() throws Exception { + SIPToArchiveTransferImporter si = importSampleSip("target/tmpJunit/UnreadableBDO-case"); + + BinaryDataObject bdo = firstBinaryDataObject(si); + assertThat(bdo.getOnDiskPath().getParent().getFileName().toString()).isEqualTo("content"); + assertThat(Files.exists(bdo.getOnDiskPath())).isTrue(); + } + + @Test + void shouldReportTheBinaryDataObjectWhoseFileIsGone() throws Exception { + SIPToArchiveTransferImporter si = importSampleSip("target/tmpJunit/UnreadableBDO-missing"); + BinaryDataObject bdo = firstBinaryDataObject(si); + Files.delete(bdo.getOnDiskPath()); + + assertThat(si.getArchiveTransfer().getDataObjectPackage().getUnreadableBinaryDataObjectDescriptions()) + .hasSize(1) + .allSatisfy( + description -> + assertThat(description).contains(bdo.getInDataObjectPackageId()).contains("qui n'existe pas") + ); + } + + @Test + void shouldRefuseTheSipExportAndNameEveryMissingFile() throws Exception { + SIPToArchiveTransferImporter si = importSampleSip("target/tmpJunit/UnreadableBDO-export"); + BinaryDataObject bdo = firstBinaryDataObject(si); + Files.delete(bdo.getOnDiskPath()); + Path exportedSip = Paths.get("target/tmpJunit/UnreadableBDO-export/exported.zip"); + Files.deleteIfExists(exportedSip); + ArchiveTransferToSIPExporter exporter = new ArchiveTransferToSIPExporter(si.getArchiveTransfer(), null); + + assertThatThrownBy(() -> exporter.doExportToSEDASIP(exportedSip.toString(), true, true)) + .isInstanceOf(SEDALibException.class) + .hasMessageContaining("Export du SIP impossible") + .hasMessageContaining(bdo.getInDataObjectPackageId()); + + // nothing has been written, rather than a truncated SIP silently missing its binaries + assertThat(Files.exists(exportedSip)).isFalse(); + } + + @Test + void shouldCountAndReportTheFilesIgnoredAtDiskImport() throws Exception { + DiskToArchiveTransferImporter di = new DiskToArchiveTransferImporter( + "src/test/resources/PacketSamples/SampleWithTitleDirectoryNameModelV2", + null + ); + di.addIgnorePattern(".*\\.jpg"); + di.doImport(); + + assertThat(di.getSummary()).contains("fichier(s) ignoré(s)"); + } +}