From e196ede6278bd217c62b23e342afc19272d3a591 Mon Sep 17 00:00:00 2001 From: HARPER Jon Date: Thu, 19 Sep 2024 09:06:40 +0200 Subject: [PATCH 1/7] CgmesImporter, allow importing with wrong dataextension when the mainfile doesn't exist This happens when filenames have the 2 following properties: - the filename contains a dot: - the filename does not contain the dataextension For example this filename triggers the bug: "microgrid.v2.complete.zip" but these work: "microgrid_v2_complete.zip" (because empty dataextension) or "microgrid.v2.complete.xml.zip" (because ".xml" dataextension) The problem is that the main extension is derived as ".v2.complete" but the cgmes importer only accepts an empty dataextension or ".xml" (it checks the dataextension to avoid importing as cgmes from a datasource that was meant by the user to be used as another format). But for now no other importer works without a main file, so when the mainfile doesn't exist so we shouldn't block the import just like when the dataextension is not specified NOTE: this happens because we want to allow archive filenames to be very flexible (with or without the dataextension, and for cgmes even with a totally different basename as what is inside). NOTE: if we ever have another importer that works like the cgmes importer, this means we will not be able to handle the following case: archive/network_EQ.xml archive/network_TP.xml # cgmes files archive/network_foo.other archive/network_bar.other # some new importer format and be able to import from a single directory containing both networks by specifying the dataextension. But this should be a rare case, and moving the data to separate directories or using separate basenames works around the problem. Signed-off-by: HARPER Jon --- .../com/powsybl/cgmes/model/CgmesOnDataSource.java | 2 +- .../powsybl/cgmes/model/CgmesOnDataSourceTest.java | 14 +++++++++++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/cgmes/cgmes-model/src/main/java/com/powsybl/cgmes/model/CgmesOnDataSource.java b/cgmes/cgmes-model/src/main/java/com/powsybl/cgmes/model/CgmesOnDataSource.java index e0d2b7ce7c1..9b29aa6068d 100644 --- a/cgmes/cgmes-model/src/main/java/com/powsybl/cgmes/model/CgmesOnDataSource.java +++ b/cgmes/cgmes-model/src/main/java/com/powsybl/cgmes/model/CgmesOnDataSource.java @@ -35,7 +35,7 @@ public ReadOnlyDataSource dataSource() { } private boolean checkIfMainFileNotWithCgmesData(boolean isCim14) throws IOException { - if (dataSource.getDataExtension() == null || dataSource.getDataExtension().isEmpty()) { + if (dataSource.getDataExtension() == null || dataSource.getDataExtension().isEmpty() || !dataSource.exists(null, dataSource.getDataExtension())) { return false; } else if (EXTENSION.equals(dataSource.getDataExtension()) && dataSource.exists(null, EXTENSION)) { try (InputStream is = dataSource.newInputStream(null, EXTENSION)) { diff --git a/cgmes/cgmes-model/src/test/java/com/powsybl/cgmes/model/CgmesOnDataSourceTest.java b/cgmes/cgmes-model/src/test/java/com/powsybl/cgmes/model/CgmesOnDataSourceTest.java index 4474e0c55ff..8cb51b8865c 100644 --- a/cgmes/cgmes-model/src/test/java/com/powsybl/cgmes/model/CgmesOnDataSourceTest.java +++ b/cgmes/cgmes-model/src/test/java/com/powsybl/cgmes/model/CgmesOnDataSourceTest.java @@ -28,6 +28,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; /** * @author Jon Harper {@literal } @@ -71,13 +72,24 @@ void testFileDoesNotExist() throws IOException { byte[] data = "Test String".getBytes(); out.write(data, 0, data.length); out.closeEntry(); + e = new ZipEntry("foo.xml"); + out.putNextEntry(e); + data = getClass().getResourceAsStream("/empty_cim16_EQ.xml").readAllBytes(); + out.write(data, 0, data.length); + out.closeEntry(); } catch (IOException ex) { throw new RuntimeException(ex); } } ReadOnlyDataSource dataSource = new ZipArchiveDataSource(testDir, "foo.iidm.zip", "test", "xml", null); CgmesOnDataSource cgmesOnDataSource = new CgmesOnDataSource(dataSource); - assertFalse(cgmesOnDataSource.exists()); + assertTrue(cgmesOnDataSource.exists()); + ReadOnlyDataSource dataSource2 = new ZipArchiveDataSource(testDir, "foo.iidm.zip", "test", "iidm", null); + CgmesOnDataSource cgmesOnDataSource2 = new CgmesOnDataSource(dataSource2); + assertTrue(cgmesOnDataSource2.exists()); + ReadOnlyDataSource dataSource3 = new ZipArchiveDataSource(testDir, "foo.iidm.zip", "foo", "bar", null); + CgmesOnDataSource cgmesOnDataSource3 = new CgmesOnDataSource(dataSource3); + assertFalse(cgmesOnDataSource3.exists()); } } } From 1d6a4e7f35df7839386fba175e396373b0cad229 Mon Sep 17 00:00:00 2001 From: HARPER Jon Date: Thu, 19 Sep 2024 14:33:46 +0200 Subject: [PATCH 2/7] more testing Signed-off-by: HARPER Jon --- .../cgmes/model/CgmesOnDataSourceTest.java | 124 +++++++++++++++--- 1 file changed, 107 insertions(+), 17 deletions(-) diff --git a/cgmes/cgmes-model/src/test/java/com/powsybl/cgmes/model/CgmesOnDataSourceTest.java b/cgmes/cgmes-model/src/test/java/com/powsybl/cgmes/model/CgmesOnDataSourceTest.java index 8cb51b8865c..5e89e9cf2b7 100644 --- a/cgmes/cgmes-model/src/test/java/com/powsybl/cgmes/model/CgmesOnDataSourceTest.java +++ b/cgmes/cgmes-model/src/test/java/com/powsybl/cgmes/model/CgmesOnDataSourceTest.java @@ -13,7 +13,6 @@ import com.powsybl.commons.datasource.ResourceDataSource; import com.powsybl.commons.datasource.ResourceSet; import com.powsybl.commons.datasource.ZipArchiveDataSource; -import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; @@ -28,13 +27,14 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertTrue; /** * @author Jon Harper {@literal } */ class CgmesOnDataSourceTest { + private static String XIIDM_XML_NOT_CGMES = ""; + static Stream provideArguments() { return Stream.of( Arguments.of("EQ cim14", "empty_cim14_EQ.xml", "14", true), @@ -59,37 +59,127 @@ void testExists(String testName, String filename, String cimVersion, boolean exp assertEquals(expectedExists, exists); } - @Test - void testFileDoesNotExist() throws IOException { + static Stream provideArgumentsFortestXmlMainFileXiidmZip() { + return Stream.of( + Arguments.of("foo", "xml"), + Arguments.of("foo", null), + Arguments.of("foo", ""), + Arguments.of("foo", "notexists"), + Arguments.of("bar", "xml"), + Arguments.of("bar", null), + Arguments.of("bar", ""), + Arguments.of("bar", "notexists") + ); + } + + @ParameterizedTest + @MethodSource("provideArgumentsFortestXmlMainFileXiidmZip") + void testXmlMainFileXiidmZip(String basename, String dataextension) throws IOException { Path testDir; try (FileSystem fileSystem = Jimfs.newFileSystem(Configuration.unix())) { testDir = fileSystem.getPath("/tmp"); Files.createDirectories(testDir); - try (ZipOutputStream out = new ZipOutputStream(Files.newOutputStream(testDir.resolve("foo.iidm.zip")))) { + try (ZipOutputStream out = new ZipOutputStream(Files.newOutputStream(testDir.resolve("my.zip")))) { try { - ZipEntry e = new ZipEntry("foo.bar"); + ZipEntry e = new ZipEntry("foo.xml"); out.putNextEntry(e); - byte[] data = "Test String".getBytes(); + byte[] data = XIIDM_XML_NOT_CGMES.getBytes(); out.write(data, 0, data.length); out.closeEntry(); - e = new ZipEntry("foo.xml"); + } catch (IOException ex) { + throw new RuntimeException(ex); + } + } + + ReadOnlyDataSource dataSourceGoodBasenameGoodDataExtension = new ZipArchiveDataSource( + testDir, "my.zip", basename, dataextension, null); + CgmesOnDataSource cgmesOnDataSourceGoodBasenameGoodDataExtension = new CgmesOnDataSource( + dataSourceGoodBasenameGoodDataExtension); + assertFalse(cgmesOnDataSourceGoodBasenameGoodDataExtension.exists()); + + } + } + + static Stream provideArgumentsForTestXmlMainFileCgmesZip() { + return Stream.of( + Arguments.of("foo", "xml", true), + Arguments.of("foo", "xiidm", false), + Arguments.of("foo", "notexists", true), + Arguments.of("foo", "", true), + Arguments.of("foo", null, true), + Arguments.of("bar", "xml", false), + Arguments.of("bar", "xiidm", false), + Arguments.of("bar", "notexists", true), + Arguments.of("bar", "", true), + Arguments.of("bar", null, true), + Arguments.of("kop", "xml", true), + Arguments.of("kop", "xiidm", false), + Arguments.of("kop", "notexists", true), + Arguments.of("kop", "", true), + Arguments.of("kop", null, true), + Arguments.of("notexist", "xml", true), + Arguments.of("notexist", "xiidm", true), + Arguments.of("notexist", "notexists", true), + Arguments.of("notexist", "", true), + Arguments.of("notexist", null, true) + ); + } + + @ParameterizedTest + @MethodSource("provideArgumentsForTestXmlMainFileCgmesZip") + void testXmlMainFileCgmesZip(String basename, String dataextension, boolean expected) throws IOException { + Path testDir; + try (FileSystem fileSystem = Jimfs.newFileSystem(Configuration.unix())) { + testDir = fileSystem.getPath("/tmp"); + Files.createDirectories(testDir); + try (ZipOutputStream out = new ZipOutputStream(Files.newOutputStream(testDir.resolve("my.zip")))) { + try { + // The cgmes file that justifies importing this as CGMES + ZipEntry e = new ZipEntry("foo.xml"); + out.putNextEntry(e); + byte[] data = getClass().getResourceAsStream("/empty_cim16_EQ.xml").readAllBytes(); + out.write(data, 0, data.length); + out.closeEntry(); + + // random other files, depending on the datasource basename and dataextension + // the cmgmes importer will refuse to import to allow other importers to import + e = new ZipEntry("foo.xiidm"); + out.putNextEntry(e); + data = "same basename as the tested cgmes file foo.xml".getBytes(); + out.write(data, 0, data.length); + out.closeEntry(); + // Note: and no need to test prefix matching file names like "fooooo.xiidm" or + // "fooooo.xml" + // because the prefixing of the basename is not used for exists() + + // different basename but xml may still be cgmes + e = new ZipEntry("bar.xml"); out.putNextEntry(e); - data = getClass().getResourceAsStream("/empty_cim16_EQ.xml").readAllBytes(); + data = XIIDM_XML_NOT_CGMES.getBytes(); out.write(data, 0, data.length); out.closeEntry(); + + e = new ZipEntry("bar.xiidm"); + out.putNextEntry(e); + data = "different basename different extension".getBytes(); + out.write(data, 0, data.length); + out.closeEntry(); + + e = new ZipEntry("kop.xiidm"); + out.putNextEntry(e); + data = "nothing in common, there is no other file with the same basename and .xml extension" + .getBytes(); + out.write(data, 0, data.length); + out.closeEntry(); + } catch (IOException ex) { throw new RuntimeException(ex); } } - ReadOnlyDataSource dataSource = new ZipArchiveDataSource(testDir, "foo.iidm.zip", "test", "xml", null); + ReadOnlyDataSource dataSource = new ZipArchiveDataSource(testDir, "my.zip", basename, dataextension, null); CgmesOnDataSource cgmesOnDataSource = new CgmesOnDataSource(dataSource); - assertTrue(cgmesOnDataSource.exists()); - ReadOnlyDataSource dataSource2 = new ZipArchiveDataSource(testDir, "foo.iidm.zip", "test", "iidm", null); - CgmesOnDataSource cgmesOnDataSource2 = new CgmesOnDataSource(dataSource2); - assertTrue(cgmesOnDataSource2.exists()); - ReadOnlyDataSource dataSource3 = new ZipArchiveDataSource(testDir, "foo.iidm.zip", "foo", "bar", null); - CgmesOnDataSource cgmesOnDataSource3 = new CgmesOnDataSource(dataSource3); - assertFalse(cgmesOnDataSource3.exists()); + assertEquals(expected, cgmesOnDataSource.exists()); } } + } From 83506145f842553901359d775c341f0741a6a11c Mon Sep 17 00:00:00 2001 From: Jon Harper Date: Thu, 19 Sep 2024 16:09:32 +0200 Subject: [PATCH 3/7] Update cgmes/cgmes-model/src/test/java/com/powsybl/cgmes/model/CgmesOnDataSourceTest.java Co-authored-by: Florian Dupuy <66690739+flo-dup@users.noreply.github.com> Signed-off-by: Jon Harper --- .../java/com/powsybl/cgmes/model/CgmesOnDataSourceTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cgmes/cgmes-model/src/test/java/com/powsybl/cgmes/model/CgmesOnDataSourceTest.java b/cgmes/cgmes-model/src/test/java/com/powsybl/cgmes/model/CgmesOnDataSourceTest.java index 5e89e9cf2b7..90a59931db8 100644 --- a/cgmes/cgmes-model/src/test/java/com/powsybl/cgmes/model/CgmesOnDataSourceTest.java +++ b/cgmes/cgmes-model/src/test/java/com/powsybl/cgmes/model/CgmesOnDataSourceTest.java @@ -59,7 +59,7 @@ void testExists(String testName, String filename, String cimVersion, boolean exp assertEquals(expectedExists, exists); } - static Stream provideArgumentsFortestXmlMainFileXiidmZip() { + static Stream provideArgumentsForTestXmlMainFileXiidmZip() { return Stream.of( Arguments.of("foo", "xml"), Arguments.of("foo", null), From a629e8e4a3e53d4ff10b0853e7d59e70cb70cf2a Mon Sep 17 00:00:00 2001 From: Jon Harper Date: Thu, 19 Sep 2024 16:09:53 +0200 Subject: [PATCH 4/7] Update cgmes/cgmes-model/src/test/java/com/powsybl/cgmes/model/CgmesOnDataSourceTest.java Co-authored-by: Florian Dupuy <66690739+flo-dup@users.noreply.github.com> Signed-off-by: Jon Harper --- .../java/com/powsybl/cgmes/model/CgmesOnDataSourceTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cgmes/cgmes-model/src/test/java/com/powsybl/cgmes/model/CgmesOnDataSourceTest.java b/cgmes/cgmes-model/src/test/java/com/powsybl/cgmes/model/CgmesOnDataSourceTest.java index 90a59931db8..cb901448fbf 100644 --- a/cgmes/cgmes-model/src/test/java/com/powsybl/cgmes/model/CgmesOnDataSourceTest.java +++ b/cgmes/cgmes-model/src/test/java/com/powsybl/cgmes/model/CgmesOnDataSourceTest.java @@ -33,7 +33,7 @@ */ class CgmesOnDataSourceTest { - private static String XIIDM_XML_NOT_CGMES = ""; + private final static String XIIDM_XML_NOT_CGMES = ""; static Stream provideArguments() { return Stream.of( From 2906e964120c51e227bddd12d59c7f81e3fdf672 Mon Sep 17 00:00:00 2001 From: HARPER Jon Date: Thu, 19 Sep 2024 16:28:17 +0200 Subject: [PATCH 5/7] checkstyle Signed-off-by: HARPER Jon --- .../java/com/powsybl/cgmes/model/CgmesOnDataSourceTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cgmes/cgmes-model/src/test/java/com/powsybl/cgmes/model/CgmesOnDataSourceTest.java b/cgmes/cgmes-model/src/test/java/com/powsybl/cgmes/model/CgmesOnDataSourceTest.java index cb901448fbf..5ecaa843c04 100644 --- a/cgmes/cgmes-model/src/test/java/com/powsybl/cgmes/model/CgmesOnDataSourceTest.java +++ b/cgmes/cgmes-model/src/test/java/com/powsybl/cgmes/model/CgmesOnDataSourceTest.java @@ -33,7 +33,7 @@ */ class CgmesOnDataSourceTest { - private final static String XIIDM_XML_NOT_CGMES = ""; + private static final String XIIDM_XML_NOT_CGMES = ""; static Stream provideArguments() { return Stream.of( From a73c18d6c03011791461509b077d50e6d47eceff Mon Sep 17 00:00:00 2001 From: HARPER Jon Date: Thu, 19 Sep 2024 17:00:47 +0200 Subject: [PATCH 6/7] fix after review Signed-off-by: HARPER Jon --- .../java/com/powsybl/cgmes/model/CgmesOnDataSourceTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cgmes/cgmes-model/src/test/java/com/powsybl/cgmes/model/CgmesOnDataSourceTest.java b/cgmes/cgmes-model/src/test/java/com/powsybl/cgmes/model/CgmesOnDataSourceTest.java index 5ecaa843c04..a4bd3a0c720 100644 --- a/cgmes/cgmes-model/src/test/java/com/powsybl/cgmes/model/CgmesOnDataSourceTest.java +++ b/cgmes/cgmes-model/src/test/java/com/powsybl/cgmes/model/CgmesOnDataSourceTest.java @@ -73,7 +73,7 @@ static Stream provideArgumentsForTestXmlMainFileXiidmZip() { } @ParameterizedTest - @MethodSource("provideArgumentsFortestXmlMainFileXiidmZip") + @MethodSource("provideArgumentsForTestXmlMainFileXiidmZip") void testXmlMainFileXiidmZip(String basename, String dataextension) throws IOException { Path testDir; try (FileSystem fileSystem = Jimfs.newFileSystem(Configuration.unix())) { From 6d13331f7391ef77ba1c433c258f71ed6df0582c Mon Sep 17 00:00:00 2001 From: HARPER Jon Date: Fri, 20 Sep 2024 10:18:06 +0200 Subject: [PATCH 7/7] remove unneed exists, it's already checked Signed-off-by: HARPER Jon --- .../main/java/com/powsybl/cgmes/model/CgmesOnDataSource.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cgmes/cgmes-model/src/main/java/com/powsybl/cgmes/model/CgmesOnDataSource.java b/cgmes/cgmes-model/src/main/java/com/powsybl/cgmes/model/CgmesOnDataSource.java index 9b29aa6068d..b1eaf3d9044 100644 --- a/cgmes/cgmes-model/src/main/java/com/powsybl/cgmes/model/CgmesOnDataSource.java +++ b/cgmes/cgmes-model/src/main/java/com/powsybl/cgmes/model/CgmesOnDataSource.java @@ -37,7 +37,7 @@ public ReadOnlyDataSource dataSource() { private boolean checkIfMainFileNotWithCgmesData(boolean isCim14) throws IOException { if (dataSource.getDataExtension() == null || dataSource.getDataExtension().isEmpty() || !dataSource.exists(null, dataSource.getDataExtension())) { return false; - } else if (EXTENSION.equals(dataSource.getDataExtension()) && dataSource.exists(null, EXTENSION)) { + } else if (EXTENSION.equals(dataSource.getDataExtension())) { try (InputStream is = dataSource.newInputStream(null, EXTENSION)) { return isCim14 ? !existsNamespacesCim14(NamespaceReader.namespaces(is)) : !existsNamespaces(NamespaceReader.namespaces(is)); }