-
Notifications
You must be signed in to change notification settings - Fork 49
Handle unsupported IIDM file versions with a better error message #3646
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 5 commits
0521a4d
f195735
31fdd51
11c6c77
56c0467
a316dfa
575ab56
de049f0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -58,32 +58,44 @@ public String getComment() { | |||||||||||
| } | ||||||||||||
|
|
||||||||||||
| protected boolean exists(ReadOnlyDataSource dataSource, String ext) throws IOException { | ||||||||||||
| try { | ||||||||||||
| if (ext != null) { | ||||||||||||
| try (InputStream is = dataSource.newInputStream(null, ext)) { | ||||||||||||
| // check the first root element is network and namespace is IIDM | ||||||||||||
| XMLStreamReader xmlsr = getXMLInputFactory().createXMLStreamReader(is); | ||||||||||||
| try { | ||||||||||||
| while (xmlsr.hasNext()) { | ||||||||||||
| int eventType = xmlsr.next(); | ||||||||||||
| if (eventType == XMLStreamConstants.START_ELEMENT) { | ||||||||||||
| String name = xmlsr.getLocalName(); | ||||||||||||
| String ns = xmlsr.getNamespaceURI(); | ||||||||||||
| return NetworkSerDe.NETWORK_ROOT_ELEMENT_NAME.equals(name) | ||||||||||||
| && (Stream.of(IidmVersion.values()).anyMatch(v -> v.getNamespaceURI().equals(ns)) | ||||||||||||
| || Stream.of(IidmVersion.values()).filter(v -> v.compareTo(IidmVersion.V_1_7) >= 0).anyMatch(v -> v.getNamespaceURI(false).equals(ns))); | ||||||||||||
| } | ||||||||||||
| } | ||||||||||||
| } finally { | ||||||||||||
| cleanClose(xmlsr); | ||||||||||||
| if (ext == null) { | ||||||||||||
| return false; | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| try (InputStream is = dataSource.newInputStream(null, ext)) { | ||||||||||||
| XMLStreamReader xmlsr = getXMLInputFactory().createXMLStreamReader(is); | ||||||||||||
| try { | ||||||||||||
| return isValidNetworkRoot(xmlsr); | ||||||||||||
| } finally { | ||||||||||||
| cleanClose(xmlsr); | ||||||||||||
| } | ||||||||||||
| } catch (XMLStreamException e) { | ||||||||||||
| return false; // not a valid XML file | ||||||||||||
| } | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| private boolean isValidNetworkRoot(XMLStreamReader xmlsr) throws XMLStreamException { | ||||||||||||
| while (xmlsr.hasNext()) { | ||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The comment that previously existed was useful:
Suggested change
|
||||||||||||
| if (xmlsr.next() == XMLStreamConstants.START_ELEMENT) { | ||||||||||||
| String name = xmlsr.getLocalName(); | ||||||||||||
| String ns = xmlsr.getNamespaceURI(); | ||||||||||||
|
|
||||||||||||
| if (!NetworkSerDe.NETWORK_ROOT_ELEMENT_NAME.equals(name)) { | ||||||||||||
| return false; | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| if (!ns.isEmpty()) { | ||||||||||||
| String version = ns.substring(ns.lastIndexOf('/') + 1); | ||||||||||||
| if (!version.isEmpty()) { | ||||||||||||
| IidmVersion.fromNamespaceURI(ns); | ||||||||||||
|
||||||||||||
| IidmVersion.fromNamespaceURI(ns); | |
| // Try to load an IidmVersion from the version number. | |
| // If it is not supported, this will throw an exception giving details on the encountered version | |
| // and the maximum supported version. | |
| IidmVersion.of(version, "_"); |
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another problem here. You could not throw a exception here without a minimum of checks.
When a file is imported, in Importer.find(...), all the importer present in the class loader will be tested to see if one of them support the file format. I think that you can reach this line each time you import an XML file whose extension is .xml and whose first root element is network.
If you have a custom network file format with these characteristics, you may encountered the unsupported IIDM version exception depending on the order the importers are tested.
One control that can be added is on the known namespace prefixes. I suggest that you loop on IidmVersion.values() to retrieve their namespace prefixes (by this, I mean the part before the last'/') and to check that the one of the current namespace is equal to one of them. If it is indeed, you can check the version and raise an exception if it is unknown. Else, you'll have to ignore the check.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,6 +8,7 @@ | |
| package com.powsybl.iidm.serde; | ||
|
|
||
| import com.google.common.io.ByteStreams; | ||
| import com.powsybl.commons.PowsyblException; | ||
| import com.powsybl.commons.datasource.*; | ||
| import com.powsybl.commons.report.PowsyblCoreReportResourceBundle; | ||
| import com.powsybl.commons.test.PowsyblTestReportResourceBundle; | ||
|
|
@@ -23,6 +24,7 @@ | |
| import java.io.StringWriter; | ||
| import java.nio.charset.StandardCharsets; | ||
| import java.nio.file.Files; | ||
| import java.nio.file.Path; | ||
| import java.util.Arrays; | ||
| import java.util.List; | ||
| import java.util.Properties; | ||
|
|
@@ -90,6 +92,20 @@ private void writeNetworkWithComment(String fileName) throws IOException { | |
| } | ||
| } | ||
|
|
||
| private void writeNetworkUnsupportedIidmVersion(String fileName) throws IOException { | ||
| writeNetwork(fileName, unsupportedIidmVersionNamespaceURI(), false); | ||
| } | ||
|
|
||
| private String unsupportedIidmVersionNamespaceURI() { | ||
| String currentIidmVersion = CURRENT_IIDM_VERSION.toString(); | ||
| String currentIidmNamespaceURI = CURRENT_IIDM_VERSION.getNamespaceURI(); | ||
| String[] currentIidmVersionSplit = currentIidmVersion.split("_"); | ||
| int currentVersionMajor = Integer.parseInt(currentIidmVersionSplit[1]); | ||
| int currentVersionMinor = Integer.parseInt(currentIidmVersionSplit[2]); | ||
| String unsupportedIidmVersion = String.format("%d_%d", currentVersionMajor, currentVersionMinor + 1); | ||
| return currentIidmNamespaceURI.substring(0, currentIidmNamespaceURI.lastIndexOf("/") + 1) + unsupportedIidmVersion; | ||
| } | ||
|
|
||
| @BeforeEach | ||
| public void setUp() throws IOException { | ||
| super.setUp(); | ||
|
|
@@ -116,6 +132,7 @@ public void setUp() throws IOException { | |
| } | ||
| writeNetworkWithComment("/test7.xiidm"); | ||
| writeNetworkWithExtension("/test8.xiidm", CURRENT_IIDM_VERSION.getNamespaceURI()); | ||
| writeNetworkUnsupportedIidmVersion("/test9.xiidm"); | ||
|
|
||
| importer = new XMLImporter(); | ||
| } | ||
|
|
@@ -154,6 +171,19 @@ void exists() { | |
| assertFalse(importer.exists(new DirectoryDataSource(fileSystem.getPath("/"), "testDummy"))); // namespace URI is not defined | ||
| } | ||
|
|
||
| @Test | ||
| void testUnsupportedIidmVersion() { | ||
| String[] currentIidmVersionSplit = CURRENT_IIDM_VERSION.toString().split("_"); | ||
| int unsupportedIidmVersionMajor = Integer.parseInt(currentIidmVersionSplit[1]); | ||
| int unsupportedIidmVersionMinor = Integer.parseInt(currentIidmVersionSplit[2]) + 1; | ||
| String unsupportedIidmVersion = String.format("%d.%d", unsupportedIidmVersionMajor, unsupportedIidmVersionMinor); | ||
|
||
| Path dir = fileSystem.getPath("/"); | ||
| ReadOnlyDataSource dataSource = new DirectoryDataSource(dir, "test9"); | ||
| String expectedError = "IIDM Version " + unsupportedIidmVersion + "is not supported. Max supported version: " | ||
| + CURRENT_IIDM_VERSION.toString("."); | ||
| assertThrows(PowsyblException.class, () -> importer.exists(dataSource), expectedError); | ||
| } | ||
|
|
||
| @Test | ||
| void copy() throws Exception { | ||
| importer.copy(new DirectoryDataSource(fileSystem.getPath("/"), "test0"), new DirectoryDataSource(fileSystem.getPath("/"), "test0_copy")); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.