diff --git a/resip/src/main/java/fr/gouv/vitam/tools/resip/frame/ExportContextDialog.java b/resip/src/main/java/fr/gouv/vitam/tools/resip/frame/ExportContextDialog.java index 3fe3d078..06cb3c42 100644 --- a/resip/src/main/java/fr/gouv/vitam/tools/resip/frame/ExportContextDialog.java +++ b/resip/src/main/java/fr/gouv/vitam/tools/resip/frame/ExportContextDialog.java @@ -38,6 +38,7 @@ package fr.gouv.vitam.tools.resip.frame; import fr.gouv.vitam.tools.resip.app.ResipGraphicApp; +import fr.gouv.vitam.tools.resip.frame.preferences.PreferencesDialog; import fr.gouv.vitam.tools.resip.parameters.ExportContext; import fr.gouv.vitam.tools.resip.parameters.Preferences; import fr.gouv.vitam.tools.sedalib.core.GlobalMetadata; @@ -89,6 +90,7 @@ public class ExportContextDialog extends JDialog { private JRadioButton allUsageButton; private JTextField nameMaxSizeTextField; private JCheckBox csvExtendedFormatChexBox; + private JComboBox csvExportCharsetCombobox; private JTextArea metadataFilterTextArea; private JCheckBox metadataFilterCheckBox; @@ -708,6 +710,28 @@ public ExportContextDialog(JFrame owner, ExportContext exportContext) { exportParametersPanel.add(csvExtendedFormatChexBox, gbc); csvExtendedFormatChexBox.setSelected(exportContext.isCsvExtendedFormat()); + JLabel csvExportCharsetLabel = new JLabel("Encodage du csv exporté :"); + gbc = new GridBagConstraints(); + gbc.anchor = GridBagConstraints.EAST; + gbc.insets = new Insets(0, 5, 5, 5); + gbc.gridx = 0; + gbc.gridy = 8; + exportParametersPanel.add(csvExportCharsetLabel, gbc); + + csvExportCharsetCombobox = new JComboBox<>(PreferencesDialog.CHARSET_STRINGS); + csvExportCharsetCombobox.setFont(MainWindow.LABEL_FONT); + csvExportCharsetCombobox.setToolTipText( + "Encodage des csv produits à l'export, indépendant de celui attendu à l'import. " + + "UTF-8 est écrit avec un BOM, ce qu'Excel sous Windows sait lire." + ); + gbc = new GridBagConstraints(); + gbc.insets = new Insets(0, 0, 5, 5); + gbc.fill = GridBagConstraints.HORIZONTAL; + gbc.gridx = 1; + gbc.gridy = 8; + exportParametersPanel.add(csvExportCharsetCombobox, gbc); + csvExportCharsetCombobox.setSelectedItem(exportContext.getCsvCharsetName()); + JLabel metadataFilterLabel = new JLabel("Filtrage des métadonnées"); metadataFilterLabel.setFont(MainWindow.BOLD_LABEL_FONT); gbc = new GridBagConstraints(); @@ -718,7 +742,7 @@ public ExportContextDialog(JFrame owner, ExportContext exportContext) { gbc.anchor = GridBagConstraints.NORTHWEST; gbc.fill = GridBagConstraints.BOTH; gbc.gridx = 0; - gbc.gridy = 8; + gbc.gridy = 9; exportParametersPanel.add(metadataFilterLabel, gbc); scrollPane = new JScrollPane(); @@ -729,7 +753,7 @@ public ExportContextDialog(JFrame owner, ExportContext exportContext) { gbc.fill = GridBagConstraints.BOTH; gbc.insets = new Insets(0, 5, 5, 5); gbc.gridx = 1; - gbc.gridy = 9; + gbc.gridy = 10; exportParametersPanel.add(scrollPane, gbc); metadataFilterTextArea = new JTextArea(); @@ -750,7 +774,7 @@ public ExportContextDialog(JFrame owner, ExportContext exportContext) { gbc.anchor = GridBagConstraints.EAST; gbc.insets = new Insets(0, 5, 5, 5); gbc.gridx = 0; - gbc.gridy = 9; + gbc.gridy = 10; gbc.weighty = 1.0; exportParametersPanel.add(metadataFilterCheckBox, gbc); metadataFilterCheckBox.addItemListener(this::metadataFilterEvent); @@ -834,6 +858,7 @@ public boolean extractFromDialog() { } gmc.setMaxNameSize(tmp); gmc.setCsvExtendedFormat(csvExtendedFormatChexBox.isSelected()); + gmc.setCsvCharsetName((String) csvExportCharsetCombobox.getSelectedItem()); gmc.setManagementMetadataXmlData(managementMetadataTextArea.getText()); gmc.setMetadataFilterFlag(metadataFilterCheckBox.isSelected()); gmc.setKeptMetadataList( diff --git a/resip/src/main/java/fr/gouv/vitam/tools/resip/frame/preferences/PreferencesDialog.java b/resip/src/main/java/fr/gouv/vitam/tools/resip/frame/preferences/PreferencesDialog.java index 3e2a4688..8f7d976b 100644 --- a/resip/src/main/java/fr/gouv/vitam/tools/resip/frame/preferences/PreferencesDialog.java +++ b/resip/src/main/java/fr/gouv/vitam/tools/resip/frame/preferences/PreferencesDialog.java @@ -113,6 +113,7 @@ public class PreferencesDialog extends JDialog { private final JRadioButton allUsageButton; private final JTextField nameMaxSizeTextField; private final JCheckBox csvExtendedFormatChexBox; + private final JComboBox csvExportCharsetCombobox; private final JRadioButton reindexYesRadioButton; private final JTextArea metadataFilterTextArea; private final JCheckBox metadataFilterCheckBox; @@ -172,9 +173,9 @@ public class PreferencesDialog extends JDialog { private int returnValue; /** - * The proposed charsets. + * The proposed charsets, shared with the export context dialog. */ - private static final String[] charsetStrings = { + public static final String[] CHARSET_STRINGS = { "windows-1252", "ISO-8859-1", "UTF-8", @@ -825,6 +826,28 @@ public PreferencesDialog(JFrame owner) { exportParametersPanel.add(csvExtendedFormatChexBox, gbc); csvExtendedFormatChexBox.setSelected(gmc.isCsvExtendedFormat()); + JLabel csvExportCharsetLabel = new JLabel("Encodage du csv exporté :"); + gbc = new GridBagConstraints(); + gbc.anchor = GridBagConstraints.EAST; + gbc.insets = new Insets(0, 5, 5, 5); + gbc.gridx = 0; + gbc.gridy = 10; + exportParametersPanel.add(csvExportCharsetLabel, gbc); + + csvExportCharsetCombobox = new JComboBox<>(CHARSET_STRINGS); + csvExportCharsetCombobox.setFont(MainWindow.LABEL_FONT); + csvExportCharsetCombobox.setToolTipText( + "Encodage des csv produits à l'export, indépendant de celui attendu à l'import. " + + "UTF-8 est écrit avec un BOM, ce qu'Excel sous Windows sait lire." + ); + gbc = new GridBagConstraints(); + gbc.insets = new Insets(0, 0, 5, 5); + gbc.fill = GridBagConstraints.HORIZONTAL; + gbc.gridx = 1; + gbc.gridy = 10; + exportParametersPanel.add(csvExportCharsetCombobox, gbc); + csvExportCharsetCombobox.setSelectedItem(gmc.getCsvCharsetName()); + JLabel metadataFilterLabel = new JLabel("Filtrage des métadonnées"); metadataFilterLabel.setFont(MainWindow.BOLD_LABEL_FONT); gbc = new GridBagConstraints(); @@ -834,7 +857,7 @@ public PreferencesDialog(JFrame owner) { gbc.weighty = 0.0; gbc.anchor = GridBagConstraints.NORTHWEST; gbc.gridx = 0; - gbc.gridy = 10; + gbc.gridy = 11; exportParametersPanel.add(metadataFilterLabel, gbc); scrollPane = new JScrollPane(); @@ -845,7 +868,7 @@ public PreferencesDialog(JFrame owner) { gbc.fill = GridBagConstraints.BOTH; gbc.insets = new Insets(0, 5, 5, 5); gbc.gridx = 1; - gbc.gridy = 11; + gbc.gridy = 12; exportParametersPanel.add(scrollPane, gbc); metadataFilterTextArea = new JTextArea(); @@ -866,7 +889,7 @@ public PreferencesDialog(JFrame owner) { gbc.anchor = GridBagConstraints.EAST; gbc.insets = new Insets(0, 5, 5, 5); gbc.gridx = 0; - gbc.gridy = 11; + gbc.gridy = 12; gbc.weighty = 1.0; gbc.fill = GridBagConstraints.BOTH; exportParametersPanel.add(metadataFilterCheckBox, gbc); @@ -904,7 +927,7 @@ public PreferencesDialog(JFrame owner) { gbc.gridy = 1; importParametersPanel.add(mailCharsetLabel, gbc); - defaultMailCharsetCombobox = new JComboBox<>(charsetStrings); + defaultMailCharsetCombobox = new JComboBox<>(CHARSET_STRINGS); defaultMailCharsetCombobox.setFont(MainWindow.LABEL_FONT); gbc = new GridBagConstraints(); gbc.insets = new Insets(0, 0, 5, 5); @@ -1057,7 +1080,7 @@ public PreferencesDialog(JFrame owner) { gbc.gridy = 9; importParametersPanel.add(csvCharsetLabel, gbc); - csvCharsetCombobox = new JComboBox<>(charsetStrings); + csvCharsetCombobox = new JComboBox<>(CHARSET_STRINGS); csvCharsetCombobox.setFont(MainWindow.LABEL_FONT); gbc = new GridBagConstraints(); gbc.insets = new Insets(0, 0, 5, 5); @@ -1646,6 +1669,7 @@ private boolean extractFromDialog() { } gmc.setMaxNameSize(tmp); gmc.setCsvExtendedFormat(csvExtendedFormatChexBox.isSelected()); + gmc.setCsvCharsetName((String) csvExportCharsetCombobox.getSelectedItem()); gmc.setManagementMetadataXmlData(managementMetadataTextArea.getText()); gmc.setMetadataFilterFlag(metadataFilterCheckBox.isSelected()); gmc.setKeptMetadataList( diff --git a/resip/src/main/java/fr/gouv/vitam/tools/resip/parameters/ExportContext.java b/resip/src/main/java/fr/gouv/vitam/tools/resip/parameters/ExportContext.java index 7a1998f3..dc55de24 100644 --- a/resip/src/main/java/fr/gouv/vitam/tools/resip/parameters/ExportContext.java +++ b/resip/src/main/java/fr/gouv/vitam/tools/resip/parameters/ExportContext.java @@ -81,6 +81,9 @@ public class ExportContext { /** The csv extended format flag. */ private boolean csvExtendedFormat; + /** The csv export charset name. */ + private String csvCharsetName; + /** The ManagementMetadata. */ private String managementMetadataXmlData; @@ -106,6 +109,7 @@ public ExportContext() { this.usageVersionSelectionMode = LAST_DATAOBJECT; this.maxNameSize = 32; this.csvExtendedFormat = true; + this.csvCharsetName = getDefaultCsvCharsetName(); this.globalMetadata = new GlobalMetadata(); this.managementMetadataXmlData = null; this.onDiskOutput = null; @@ -146,6 +150,7 @@ public ExportContext( this.usageVersionSelectionMode = usageVersionSelectionMode; this.maxNameSize = maxNameSize; this.csvExtendedFormat = csvExtendedFormat; + this.csvCharsetName = getDefaultCsvCharsetName(); this.globalMetadata = globalMetadata; this.managementMetadataXmlData = managementMetadataXmlData; this.setOnDiskOutput(onDiskOutput); @@ -173,6 +178,7 @@ public ExportContext(String sipExportFileName) throws ResipException { this.usageVersionSelectionMode = sec.usageVersionSelectionMode; this.maxNameSize = sec.maxNameSize; this.csvExtendedFormat = sec.csvExtendedFormat; + this.csvCharsetName = (sec.csvCharsetName == null ? getDefaultCsvCharsetName() : sec.csvCharsetName); this.globalMetadata = sec.globalMetadata; this.managementMetadataXmlData = sec.managementMetadataXmlData; this.metadataFilterFlag = sec.metadataFilterFlag; @@ -235,6 +241,8 @@ public ExportContext(Preferences preferences) { csvExtendedFormat = Boolean.parseBoolean( preferences.getPrefProperties().getProperty("exportContext.csvExport.csvExtendedFormat", "true") ); + csvCharsetName = preferences.getPrefProperties().getProperty("exportContext.csvExport.charsetName", ""); + if (csvCharsetName.isEmpty()) csvCharsetName = getDefaultCsvCharsetName(); managementMetadataXmlData = nullIfEmpty( preferences.getPrefProperties().getProperty("exportContext.general.managementMetadataXmlData", "") ); @@ -320,6 +328,12 @@ public void toPrefs(Preferences preferences) { preferences .getPrefProperties() .setProperty("exportContext.csvExport.csvExtendedFormat", Boolean.toString(csvExtendedFormat)); + preferences + .getPrefProperties() + .setProperty( + "exportContext.csvExport.charsetName", + (csvCharsetName == null ? getDefaultCsvCharsetName() : csvCharsetName) + ); preferences .getPrefProperties() .setProperty( @@ -417,6 +431,36 @@ public void toPrefs(Preferences preferences) { ); } + /** + * Gets the csv export charset name. + * + * @return the csv export charset name + */ + public String getCsvCharsetName() { + return csvCharsetName; + } + + /** + * Sets the csv export charset name. + * + * @param csvCharsetName the csv export charset name + */ + public void setCsvCharsetName(String csvCharsetName) { + this.csvCharsetName = csvCharsetName; + } + + /** + * Gets the platform default charset name for csv export, windows-1252 on Windows where Excel + * expects it, UTF-8 elsewhere. Same default as the csv import one, so that the export keeps the + * behaviour it had when it was wrongly driven by the import preference. + * + * @return the default csv export charset name + */ + public static String getDefaultCsvCharsetName() { + if (System.getProperty("os.name").toLowerCase().contains("win")) return "windows-1252"; + return "UTF-8"; + } + /** * Sets the default prefs. */ @@ -427,6 +471,7 @@ public void setDefaultPrefs() { this.usageVersionSelectionMode = LAST_DATAOBJECT; this.maxNameSize = 32; this.csvExtendedFormat = true; + this.csvCharsetName = getDefaultCsvCharsetName(); this.managementMetadataXmlData = " \n" + " Acquisition Information\n" + " Public Archive\n" + diff --git a/resip/src/main/java/fr/gouv/vitam/tools/resip/threads/ExportThread.java b/resip/src/main/java/fr/gouv/vitam/tools/resip/threads/ExportThread.java index cb379b06..db5700c0 100644 --- a/resip/src/main/java/fr/gouv/vitam/tools/resip/threads/ExportThread.java +++ b/resip/src/main/java/fr/gouv/vitam/tools/resip/threads/ExportThread.java @@ -226,7 +226,7 @@ public String doInBackground() { CSVImportContext cmic = new CSVImportContext(Preferences.getInstance()); DataObjectPackageToCSVMetadataExporter cme = new DataObjectPackageToCSVMetadataExporter( archiveTransfer.getDataObjectPackage(), - cmic.getCsvCharsetName(), + work.getExportContext().getCsvCharsetName(), cmic.getDelimiter(), work.getExportContext().getUsageVersionSelectionMode(), work.getExportContext().isCsvExtendedFormat(), @@ -245,7 +245,7 @@ public String doInBackground() { CSVImportContext cmicz = new CSVImportContext(Preferences.getInstance()); DataObjectPackageToCSVMetadataExporter cmez = new DataObjectPackageToCSVMetadataExporter( archiveTransfer.getDataObjectPackage(), - cmicz.getCsvCharsetName(), + work.getExportContext().getCsvCharsetName(), cmicz.getDelimiter(), work.getExportContext().getUsageVersionSelectionMode(), work.getExportContext().isCsvExtendedFormat(), @@ -264,7 +264,7 @@ public String doInBackground() { CSVImportContext cmicm = new CSVImportContext(Preferences.getInstance()); DataObjectPackageToCSVMetadataExporter cmem = new DataObjectPackageToCSVMetadataExporter( archiveTransfer.getDataObjectPackage(), - cmicm.getCsvCharsetName(), + work.getExportContext().getCsvCharsetName(), cmicm.getDelimiter(), work.getExportContext().getUsageVersionSelectionMode(), work.getExportContext().isCsvExtendedFormat(), diff --git a/resip/src/test/java/fr/gouv/vitam/tools/resip/parameters/ExportContextTest.java b/resip/src/test/java/fr/gouv/vitam/tools/resip/parameters/ExportContextTest.java index 3d414824..c7f009d2 100644 --- a/resip/src/test/java/fr/gouv/vitam/tools/resip/parameters/ExportContextTest.java +++ b/resip/src/test/java/fr/gouv/vitam/tools/resip/parameters/ExportContextTest.java @@ -71,6 +71,11 @@ void test() throws Exception { ExportContext reloadSipContext = mapper.readValue(ssc, ExportContext.class); String dssc = mapper.writeValueAsString(reloadSipContext); + // the csv export charset default depends on the platform, so it's asserted apart and dropped + // from the comparison with the reference file + assertThat(reloadSipContext.getCsvCharsetName()).isEqualTo(ExportContext.getDefaultCsvCharsetName()); + dssc = dssc.replaceAll("\\R? *\"csvCharsetName\" : \"[^\"]*\",?", ""); + String fromfile = new String( Files.readAllBytes(Paths.get("src/test/resources/PacketSamples/ExportContext.config")), StandardCharsets.UTF_8 @@ -78,4 +83,39 @@ void test() throws Exception { assertThat(TestUtilities.LineEndNormalize(dssc)).isEqualTo(TestUtilities.LineEndNormalize(fromfile)); } + + /** + * The csv export charset used to be the one of the csv import preference, so setting the import + * one to UTF-8 silently turned every csv export into an UTF-8 one, unreadable by Excel on Windows. + * It's now a setting of its own, that has to survive the serialization of a saved work. + */ + @Test + void csvExportCharsetSurvivesJsonRoundTrip() throws Exception { + ExportContext gmc = new ExportContext(); + gmc.setDefaultPrefs(); + gmc.setCsvCharsetName("UTF-8"); + ObjectMapper mapper = new ObjectMapper(); + + ExportContext reloaded = mapper.readValue(mapper.writeValueAsString(gmc), ExportContext.class); + + assertThat(reloaded.getCsvCharsetName()).isEqualTo("UTF-8"); + } + + /** + * A work saved before the export charset existed carries no value for it, and has to fall back on + * the platform default rather than on a null charset name, which would break the export. + */ + @Test + void csvExportCharsetFallsBackOnDefaultWhenAbsentFromJson() throws Exception { + ObjectMapper mapper = new ObjectMapper(); + ExportContext reference = new ExportContext(); + reference.setDefaultPrefs(); + String withoutCharset = mapper + .writeValueAsString(reference) + .replaceAll("\\R? *\"csvCharsetName\" : \"[^\"]*\",?", ""); + + ExportContext reloaded = mapper.readValue(withoutCharset, ExportContext.class); + + assertThat(reloaded.getCsvCharsetName()).isEqualTo(ExportContext.getDefaultCsvCharsetName()); + } } diff --git a/sedalib/src/main/java/fr/gouv/vitam/tools/sedalib/inout/exporter/DataObjectPackageToCSVMetadataExporter.java b/sedalib/src/main/java/fr/gouv/vitam/tools/sedalib/inout/exporter/DataObjectPackageToCSVMetadataExporter.java index 816d91e5..65dfaeed 100644 --- a/sedalib/src/main/java/fr/gouv/vitam/tools/sedalib/inout/exporter/DataObjectPackageToCSVMetadataExporter.java +++ b/sedalib/src/main/java/fr/gouv/vitam/tools/sedalib/inout/exporter/DataObjectPackageToCSVMetadataExporter.java @@ -48,6 +48,7 @@ import org.apache.commons.io.FileUtils; import java.io.*; +import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; @@ -843,7 +844,11 @@ private void createCsvBAOSPrintScream() throws SEDALibException { try { csvBAOS = new ByteArrayOutputStream(); csvPrintStream = new PrintStream(csvBAOS, true, encoding); - } catch (UnsupportedEncodingException e) { + // Excel on Windows reads a BOM less UTF-8 csv as windows-1252 and shows the accented + // characters as mojibake, so the byte order mark has to be there. The csv importers + // drop it back on reading. + if (StandardCharsets.UTF_8.equals(Charset.forName(encoding))) csvPrintStream.print('\uFEFF'); + } catch (UnsupportedEncodingException | IllegalArgumentException e) { throw new SEDALibException("Encodage [" + encoding + "] inconnu", e); } } diff --git a/sedalib/src/main/java/fr/gouv/vitam/tools/sedalib/inout/importer/CSVMetadataToDataObjectPackageImporter.java b/sedalib/src/main/java/fr/gouv/vitam/tools/sedalib/inout/importer/CSVMetadataToDataObjectPackageImporter.java index c8beb45f..539ca841 100644 --- a/sedalib/src/main/java/fr/gouv/vitam/tools/sedalib/inout/importer/CSVMetadataToDataObjectPackageImporter.java +++ b/sedalib/src/main/java/fr/gouv/vitam/tools/sedalib/inout/importer/CSVMetadataToDataObjectPackageImporter.java @@ -46,12 +46,14 @@ import fr.gouv.vitam.tools.sedalib.core.DataObjectGroup; import fr.gouv.vitam.tools.sedalib.core.DataObjectPackage; import fr.gouv.vitam.tools.sedalib.metadata.data.FileInfo; +import fr.gouv.vitam.tools.sedalib.utils.ByteOrderMarkUtil; import fr.gouv.vitam.tools.sedalib.utils.SEDALibException; import fr.gouv.vitam.tools.sedalib.utils.SEDALibProgressLogger; import java.io.FileInputStream; import java.io.IOException; import java.io.InputStreamReader; +import java.io.Reader; import java.nio.file.DirectoryStream; import java.nio.file.Files; import java.nio.file.Path; @@ -261,7 +263,9 @@ private boolean readCSVFile() throws SEDALibException, InterruptedException { CsvSchema schema = mapper.schemaFor(String[].class).withColumnSeparator(separator); mapper.enable(CsvParser.Feature.WRAP_AS_ARRAY); try ( - InputStreamReader isr = new InputStreamReader(new FileInputStream(csvMetadataFileName), encoding); + Reader isr = ByteOrderMarkUtil.skipByteOrderMark( + new InputStreamReader(new FileInputStream(csvMetadataFileName), encoding) + ); MappingIterator it = mapper.readerFor(String[].class).with(schema).readValues(isr) ) { while (it.hasNext()) { diff --git a/sedalib/src/main/java/fr/gouv/vitam/tools/sedalib/inout/importer/CSVTreeToDataObjectPackageImporter.java b/sedalib/src/main/java/fr/gouv/vitam/tools/sedalib/inout/importer/CSVTreeToDataObjectPackageImporter.java index 24d3ceb1..e2c44e0a 100644 --- a/sedalib/src/main/java/fr/gouv/vitam/tools/sedalib/inout/importer/CSVTreeToDataObjectPackageImporter.java +++ b/sedalib/src/main/java/fr/gouv/vitam/tools/sedalib/inout/importer/CSVTreeToDataObjectPackageImporter.java @@ -44,12 +44,14 @@ import fr.gouv.vitam.tools.sedalib.core.ArchiveUnit; import fr.gouv.vitam.tools.sedalib.core.DataObjectPackage; import fr.gouv.vitam.tools.sedalib.metadata.content.Content; +import fr.gouv.vitam.tools.sedalib.utils.ByteOrderMarkUtil; import fr.gouv.vitam.tools.sedalib.utils.SEDALibException; import fr.gouv.vitam.tools.sedalib.utils.SEDALibProgressLogger; import java.io.FileInputStream; import java.io.IOException; import java.io.InputStreamReader; +import java.io.Reader; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; @@ -180,7 +182,9 @@ private void readCSVFile() throws SEDALibException, InterruptedException { CsvSchema schema = mapper.schemaFor(String[].class).withColumnSeparator(separator); mapper.enable(CsvParser.Feature.WRAP_AS_ARRAY); try ( - InputStreamReader isr = new InputStreamReader(new FileInputStream(csvFileName), encoding); + Reader isr = ByteOrderMarkUtil.skipByteOrderMark( + new InputStreamReader(new FileInputStream(csvFileName), encoding) + ); MappingIterator it = mapper.readerFor(String[].class).with(schema).readValues(isr) ) { while (it.hasNext()) { diff --git a/sedalib/src/main/java/fr/gouv/vitam/tools/sedalib/utils/ByteOrderMarkUtil.java b/sedalib/src/main/java/fr/gouv/vitam/tools/sedalib/utils/ByteOrderMarkUtil.java new file mode 100644 index 00000000..9eeae805 --- /dev/null +++ b/sedalib/src/main/java/fr/gouv/vitam/tools/sedalib/utils/ByteOrderMarkUtil.java @@ -0,0 +1,75 @@ +/** + * 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 java.io.IOException; +import java.io.PushbackReader; +import java.io.Reader; + +/** + * The Class ByteOrderMarkUtil. + *

+ * Utility used to drop the byte order mark a reader may begin with. Csv files exported for Excel on + * Windows have to carry one when they are UTF-8 encoded, and it has to be dropped on reading, as it + * would otherwise be kept in the first header cell and make the column unrecognized. + */ +public class ByteOrderMarkUtil { + + /** + * The byte order mark character, as decoded by any unicode charset decoder. + */ + private static final char BYTE_ORDER_MARK = '\uFEFF'; + + private ByteOrderMarkUtil() { + // utility class + } + + /** + * Wraps the reader in one skipping the byte order mark it may begin with. + * + * @param reader the reader + * @return a reader positioned after the byte order mark if there was one, on the first character otherwise + * @throws IOException if the first character can't be read + */ + public static Reader skipByteOrderMark(Reader reader) throws IOException { + PushbackReader pushbackReader = new PushbackReader(reader, 1); + int firstChar = pushbackReader.read(); + if ((firstChar != -1) && (firstChar != BYTE_ORDER_MARK)) pushbackReader.unread(firstChar); + return pushbackReader; + } +} 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/inout/CSVMetadataExporterTest.java b/sedalib/src/test/java/fr/gouv/vitam/tools/sedalib/inout/CSVMetadataExporterTest.java index 3184b08b..e93917a6 100644 --- a/sedalib/src/test/java/fr/gouv/vitam/tools/sedalib/inout/CSVMetadataExporterTest.java +++ b/sedalib/src/test/java/fr/gouv/vitam/tools/sedalib/inout/CSVMetadataExporterTest.java @@ -52,9 +52,11 @@ import org.junit.jupiter.api.extension.ExtendWith; import java.io.*; +import java.nio.charset.Charset; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; +import java.util.Arrays; import java.util.Enumeration; import java.util.HashMap; import java.util.stream.Stream; @@ -269,11 +271,70 @@ void exportCSVOK() throws SEDALibException, InterruptedException, IOException { String generatedFileContent = TestUtilities.SlackNormalize( FileUtils.readFileToString(new File(TEMPORARY_FILE), "UTF8") ); + // an UTF-8 csv is exported with a BOM, so that Excel on Windows doesn't read it as windows-1252 + assertThat(generatedFileContent).startsWith("\uFEFF"); + generatedFileContent = generatedFileContent.substring(1); String expectedFileContent = ResourceUtils.getResourceAsString("ExpectedResults/ExportedMetadata.csv"); assertThat(generatedFileContent).isEqualToNormalizingNewlines(expectedFileContent); } + private static DataObjectPackage importSampleDirectory() throws SEDALibException, InterruptedException { + DiskToArchiveTransferImporter di = new DiskToArchiveTransferImporter( + "src/test/resources/PacketSamples/SampleWithTitleDirectoryNameModelV2", + null + ); + di.addIgnorePattern("Thumbs.db"); + di.addIgnorePattern("pagefile.sys"); + di.doImport(); + return di.getArchiveTransfer().getDataObjectPackage(); + } + + /** + * Excel on Windows reads a BOM less UTF-8 csv as windows-1252 and shows every accented character + * as mojibake, which is what the users reported on the exported metadata. + */ + @Test + void exportCSVInUTF8StartsWithByteOrderMark() throws SEDALibException, InterruptedException, IOException { + eraseAll("target/tmpJunit/CSVMetadataExporterCSV"); + DataObjectPackageToCSVMetadataExporter cme = new DataObjectPackageToCSVMetadataExporter( + importSampleDirectory(), + "UTF8", + ';', + ALL_DATAOBJECTS, + false, + 0, + null + ); + + cme.doExportToCSVMetadataFile(TEMPORARY_FILE); + + byte[] firstBytes = Arrays.copyOf(Files.readAllBytes(Paths.get(TEMPORARY_FILE)), 3); + assertThat(firstBytes).isEqualTo(new byte[] { (byte) 0xEF, (byte) 0xBB, (byte) 0xBF }); + } + + /** + * The BOM is an UTF-8 only matter, a windows-1252 csv has to stay byte for byte what it was. + */ + @Test + void exportCSVInWindows1252HasNoByteOrderMark() throws SEDALibException, InterruptedException, IOException { + eraseAll("target/tmpJunit/CSVMetadataExporterCSV"); + DataObjectPackageToCSVMetadataExporter cme = new DataObjectPackageToCSVMetadataExporter( + importSampleDirectory(), + "windows-1252", + ';', + ALL_DATAOBJECTS, + false, + 0, + null + ); + + cme.doExportToCSVMetadataFile(TEMPORARY_FILE); + + String content = new String(Files.readAllBytes(Paths.get(TEMPORARY_FILE)), Charset.forName("windows-1252")); + assertThat(content).doesNotStartWith("\uFEFF").startsWith("File;"); + } + @Test void exportCSVWithExtendedFormat() throws SEDALibException, InterruptedException, IOException { // do import of test directory @@ -306,6 +367,9 @@ void exportCSVWithExtendedFormat() throws SEDALibException, InterruptedException String generatedFileContent = TestUtilities.SlackNormalize( FileUtils.readFileToString(new File(TEMPORARY_FILE), "UTF8") ); + // an UTF-8 csv is exported with a BOM, so that Excel on Windows doesn't read it as windows-1252 + assertThat(generatedFileContent).startsWith("\uFEFF"); + generatedFileContent = generatedFileContent.substring(1); String expectedFileContent = ResourceUtils.getResourceAsString( "ExpectedResults/ExportedMetadataWithExtendedFormat.csv" ); diff --git a/sedalib/src/test/java/fr/gouv/vitam/tools/sedalib/inout/CSVMetadataToDataObjectPackageImporterTest.java b/sedalib/src/test/java/fr/gouv/vitam/tools/sedalib/inout/CSVMetadataToDataObjectPackageImporterTest.java index a7dcc79e..7e29b796 100644 --- a/sedalib/src/test/java/fr/gouv/vitam/tools/sedalib/inout/CSVMetadataToDataObjectPackageImporterTest.java +++ b/sedalib/src/test/java/fr/gouv/vitam/tools/sedalib/inout/CSVMetadataToDataObjectPackageImporterTest.java @@ -55,9 +55,11 @@ import java.io.FileNotFoundException; import java.io.IOException; +import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.Paths; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.assertj.core.api.AssertionsForClassTypes.assertThat; @@ -65,6 +67,36 @@ @ExtendWith(SedaContextExtension.class) class CSVMetadataToDataObjectPackageImporterTest { + /** + * Non regression on the csv round trip: an UTF-8 csv is now exported with a BOM, so that Excel on + * Windows doesn't show the accented characters as mojibake. That BOM has to be dropped on reading, + * as it would otherwise stay in the first header cell and make the first column unrecognized. + */ + @Test + void importOKCSVStartingWithByteOrderMark() throws SEDALibException, InterruptedException, IOException { + // Given a csv identical to the reference one, but UTF-8 encoded and starting with a BOM + Path bomCsv = Paths.get("target/tmpJunit/CSVMetadataImporterBOM/MetadataTestOK1col.csv"); + Files.createDirectories(bomCsv.getParent()); + String content = new String( + Files.readAllBytes(Paths.get("src/test/resources/PacketSamples/MetadataTestOK1col.csv")), + Charset.forName("windows-1252") + ); + Files.write(bomCsv, ("\uFEFF" + content).getBytes(StandardCharsets.UTF_8)); + + // When loaded + CSVMetadataToDataObjectPackageImporter cmi = new CSVMetadataToDataObjectPackageImporter( + bomCsv.toString(), + "UTF-8", + ';', + null + ); + cmi.doImport(); + + // Then the header line has been understood and the archive units are there + assertThat(cmi.getDataObjectPackage().getArchiveUnitById("ID10")).isNotNull(); + assertThat(cmi.getDataObjectPackage().getArchiveUnitById("ID17")).isNotNull(); + } + @Test void importOKCSV1column() throws SEDALibException, InterruptedException, JsonProcessingException { // Given diff --git a/sedalib/src/test/java/fr/gouv/vitam/tools/sedalib/utils/ByteOrderMarkUtilTest.java b/sedalib/src/test/java/fr/gouv/vitam/tools/sedalib/utils/ByteOrderMarkUtilTest.java new file mode 100644 index 00000000..6b2f51cb --- /dev/null +++ b/sedalib/src/test/java/fr/gouv/vitam/tools/sedalib/utils/ByteOrderMarkUtilTest.java @@ -0,0 +1,84 @@ +/** + * 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.Test; + +import java.io.IOException; +import java.io.Reader; +import java.io.StringReader; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +class ByteOrderMarkUtilTest { + + private static String readAll(Reader reader) throws IOException { + StringBuilder sb = new StringBuilder(); + int c; + while ((c = reader.read()) != -1) sb.append((char) c); + return sb.toString(); + } + + @Test + void shouldDropALeadingByteOrderMark() throws IOException { + Reader reader = ByteOrderMarkUtil.skipByteOrderMark(new StringReader("Id;Title\n\"ID1\";\"Été\"")); + + assertEquals("Id;Title\n\"ID1\";\"Été\"", readAll(reader)); + } + + @Test + void shouldKeepEveryCharacterWhenThereIsNoByteOrderMark() throws IOException { + Reader reader = ByteOrderMarkUtil.skipByteOrderMark(new StringReader("Id;Title\n\"ID1\";\"Été\"")); + + assertEquals("Id;Title\n\"ID1\";\"Été\"", readAll(reader)); + } + + @Test + void shouldDropOnlyTheFirstByteOrderMark() throws IOException { + Reader reader = ByteOrderMarkUtil.skipByteOrderMark(new StringReader("Id")); + + assertEquals("Id", readAll(reader)); + } + + @Test + void shouldHandleAnEmptyReader() throws IOException { + Reader reader = ByteOrderMarkUtil.skipByteOrderMark(new StringReader("")); + + assertEquals("", readAll(reader)); + } +} 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) + ); + } +}