-
Notifications
You must be signed in to change notification settings - Fork 40
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
Datasources - Part 2: main extension + existStrict + builder #3102
Conversation
Signed-off-by: Nicolas Rol <[email protected]>
Signed-off-by: Nicolas Rol <[email protected]>
Signed-off-by: Nicolas Rol <[email protected]>
Signed-off-by: Nicolas Rol <[email protected]>
Signed-off-by: Nicolas Rol <[email protected]>
Signed-off-by: Nicolas Rol <[email protected]>
Signed-off-by: Nicolas Rol <[email protected]>
Signed-off-by: Nicolas Rol <[email protected]>
Signed-off-by: Nicolas Rol <[email protected]>
Signed-off-by: Nicolas Rol <[email protected]>
Signed-off-by: Nicolas Rol <[email protected]>
Signed-off-by: Nicolas Rol <[email protected]>
Signed-off-by: Nicolas Rol <[email protected]>
Signed-off-by: Nicolas Rol <[email protected]>
Signed-off-by: Nicolas Rol <[email protected]>
…ataSource Signed-off-by: Nicolas Rol <[email protected]>
Signed-off-by: Nicolas Rol <[email protected]>
Signed-off-by: Nicolas Rol <[email protected]>
…asources_2_main_extension # Conflicts: # commons/src/main/java/com/powsybl/commons/datasource/ReadOnlyDataSource.java
Signed-off-by: Nicolas Rol <[email protected]>
c76c4eb
to
4fd2dba
Compare
@@ -59,7 +62,8 @@ protected OutputStream getCompressedOutputStream(OutputStream os) throws IOExcep | |||
|
|||
private Path getPath(String fileName) { | |||
Objects.requireNonNull(fileName); | |||
return directory.resolve(fileName + getCompressionExt()); | |||
FileInformation fileInformation = new FileInformation(fileName, false); | |||
return directory.resolve(fileInformation.getCompressionFormat() == null ? fileName + getCompressionExtension() : fileName); |
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.
mention this fix/change or put in a separate PR ?
This changes compressed directory data source to allow newinputstream newoutputstream and exists to work with "foo.xiidm" when the file is "foo.xiidm.gz"
so it introduces the concept of "filenameOrUncompressedFilename" ?
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.
added info about this in the PR comment + in the breaking change section
} | ||
|
||
@Test | ||
void testConversionZip() throws IOException { |
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.
very nice that you added a test !
since everything can be implemented only with getBaseExtension ( see #3085 ) , do we still want to add an API existsStrict ? Maybe yes but i'm not sure |
Signed-off-by: Nicolas Rol <[email protected]>
Signed-off-by: Nicolas Rol <[email protected]>
I find it better to do the check on the datasource side and not on the importer side. However, after discussion with @flo-dup, I changed |
Signed-off-by: Nicolas Rol <[email protected]>
# Conflicts: # commons/src/main/java/com/powsybl/commons/datasource/AbstractArchiveDataSource.java # commons/src/main/java/com/powsybl/commons/datasource/AbstractFileSystemDataSource.java # commons/src/main/java/com/powsybl/commons/datasource/Bzip2DirectoryDataSource.java # commons/src/main/java/com/powsybl/commons/datasource/DataSourceUtil.java # commons/src/main/java/com/powsybl/commons/datasource/DirectoryDataSource.java # commons/src/main/java/com/powsybl/commons/datasource/GenericReadOnlyDataSource.java # commons/src/main/java/com/powsybl/commons/datasource/GzDirectoryDataSource.java # commons/src/main/java/com/powsybl/commons/datasource/ReadOnlyDataSource.java # commons/src/main/java/com/powsybl/commons/datasource/XZDirectoryDataSource.java # commons/src/main/java/com/powsybl/commons/datasource/ZipArchiveDataSource.java # commons/src/main/java/com/powsybl/commons/datasource/ZstdDirectoryDataSource.java # commons/src/test/java/com/powsybl/commons/datasource/AbstractArchiveDataSourceTest.java # commons/src/test/java/com/powsybl/commons/datasource/AbstractFileSystemDataSourceTest.java # commons/src/test/java/com/powsybl/commons/datasource/Bzip2DirectoryDataSourceTest.java # commons/src/test/java/com/powsybl/commons/datasource/DirectoryDataSourceTest.java # commons/src/test/java/com/powsybl/commons/datasource/GzDirectoryDataSourceTest.java # commons/src/test/java/com/powsybl/commons/datasource/MultipleReadOnlyDataSourceTest.java # commons/src/test/java/com/powsybl/commons/datasource/XZDirectoryDataSourceTest.java # commons/src/test/java/com/powsybl/commons/datasource/ZipArchiveDataSourceTest.java # commons/src/test/java/com/powsybl/commons/datasource/ZstdDirectoryDataSourceTest.java
Signed-off-by: Nicolas Rol <[email protected]>
Signed-off-by: Nicolas Rol <[email protected]>
Signed-off-by: Nicolas Rol <[email protected]>
commons/src/main/java/com/powsybl/commons/datasource/FileInformation.java
Outdated
Show resolved
Hide resolved
commons/src/main/java/com/powsybl/commons/datasource/ReadOnlyDataSource.java
Show resolved
Hide resolved
commons/src/main/java/com/powsybl/commons/datasource/FileInformation.java
Show resolved
Hide resolved
commons/src/main/java/com/powsybl/commons/datasource/FileInformation.java
Outdated
Show resolved
Hide resolved
commons/src/main/java/com/powsybl/commons/datasource/FileInformation.java
Outdated
Show resolved
Hide resolved
commons/src/main/java/com/powsybl/commons/datasource/ReadOnlyDataSource.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Nicolas Rol <[email protected]>
cgmes/cgmes-model/src/main/java/com/powsybl/cgmes/model/CgmesOnDataSource.java
Outdated
Show resolved
Hide resolved
cgmes/cgmes-model/src/main/java/com/powsybl/cgmes/model/CgmesOnDataSource.java
Outdated
Show resolved
Hide resolved
cgmes/cgmes-model/src/main/java/com/powsybl/cgmes/model/CgmesOnDataSource.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Nicolas Rol <[email protected]>
Signed-off-by: Nicolas Rol <[email protected]>
Signed-off-by: Nicolas Rol <[email protected]>
Signed-off-by: Nicolas Rol <[email protected]>
Quality Gate passedIssues Measures |
if (dataSource.getDataExtension() == null || dataSource.getDataExtension().isEmpty()) { | ||
return false; | ||
} else if (EXTENSIONS.equals(dataSource.getDataExtension())) { | ||
try (InputStream is = dataSource.newInputStream(null, EXTENSIONS)) { |
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.
Does this do an NPE if the user specified a data extension but the file doesn't actually exist ?
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.
It would throw an exception but not a NPE. It will be fixed in #3114
* reorganize datasource classes * add tests * add dataExtension parameter to datasource * add builder and file information * add a conversion test * move boolean exists(String suffix, String ext) to AbstractFileSystemDataSource Signed-off-by: Nicolas Rol <[email protected]>
Please check if the PR fulfills these requirements
Does this PR already have an issue describing the problem?
What kind of change does this PR introduce?
Feature + bug fix
What is the current behavior?
What is the new behavior (if this is a feature change)?
Features:
dataExtension
corresponds to the extension of the files the users want to consider in their datasources. For example, ifdataExtension == "xiidm"
, only files ending with".xiidm"
will be considered. The value can be anything. If the parameter is empty, it won't be considered in the file listing..iidm.xml
,.iidm.json
,.iidm.bin
) are not anymore considered by the importers.DataSourceBuilder
is now provided in addition to usual datasources creation methods.Bug fixes:
DirectoryDataSource
method (exists
,newOutputStream
, etc.), the datasource compression format extension is added to the filename only if the filename doesn't have one already.Does this PR introduce a breaking change or deprecate an API?
If yes, please check if the following requirements are fulfilled
What changes might users need to make in their application due to this PR? (migration steps)
Some datasource constructors have changed or have been deleted. Change accordingly:
Bzip2DirectoryDataSource(Path directory, String baseName, DataSourceObserver observer)
orBzip2DirectoryDataSource(Path directory, String baseName)
Bzip2DirectoryDataSource(Path directory, String baseName, String dataExtension, DataSourceObserver observer)
GzDirectoryDataSource(Path directory, String baseName, DataSourceObserver observer)
orGzDirectoryDataSource(Path directory, String baseName)
GzDirectoryDataSource(Path directory, String baseName, String dataExtension, DataSourceObserver observer)
XZDirectoryDataSource(Path directory, String baseName, DataSourceObserver observer)
orXZDirectoryDataSource(Path directory, String baseName)
XZDirectoryDataSource(Path directory, String baseName, String dataExtension, DataSourceObserver observer)
ZstdDirectoryDataSource(Path directory, String baseName, DataSourceObserver observer)
orZstdDirectoryDataSource(Path directory, String baseName)
ZstdDirectoryDataSource(Path directory, String baseName, String dataExtension, DataSourceObserver observer)
XZDirectoryDataSource(Path directory, String baseName, DataSourceObserver observer)
orXzDirectoryDataSource(Path directory, String baseName)
XzDirectoryDataSource(Path directory, String baseName, String dataExtension, DataSourceObserver observer)
ZipArchiveDataSource(Path directory, String zipFileName, String baseName, DataSourceObserver observer)
ZipArchiveDataSource(Path directory, String zipFileName, String baseName, String dataExtension, DataSourceObserver observer)
ZipArchiveDataSource(Path directory, String zipFileName, String baseName)
ZipArchiveDataSource(Path directory, String zipFileName, String baseName, String dataExtension)
"Double" extensions (
.iidm.xml
,.iidm.json
,.iidm.bin
) are not anymore considered by the importers. If you used them, please use those instead:.iidm.xml
.xiidm
.iidm.json
.jiidm
.iidm.bin
.biidm
Previously, when providing a filename to a datasource method (
exists
,newOutputStream
, etc.), the datasource compression format extension was always added to the filename. Now it is added to the filename only if the filename doesn't have one already.Other information: