You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sis.apache.org by de...@apache.org on 2021/09/01 16:30:15 UTC
[sis] 02/03: Add a `StoreMetadata.isLowPriority` boolean for
testing a provider last. This is used when a provider is too generic and we
want to allow more specialized providers to be tested first.
This is an automated email from the ASF dual-hosted git repository.
desruisseaux pushed a commit to branch geoapi-4.0
in repository https://gitbox.apache.org/repos/asf/sis.git
commit 311d7ea64a8f6eba63ad64f4917b53265ef3a6f3
Author: Martin Desruisseaux <ma...@geomatys.com>
AuthorDate: Wed Sep 1 18:09:36 2021 +0200
Add a `StoreMetadata.isLowPriority` boolean for testing a provider last.
This is used when a provider is too generic and we want to allow more specialized providers to be tested first.
---
.../apache/sis/internal/gui/ResourceLoader.java | 12 ---
.../sis/storage/netcdf/NetcdfStoreProvider.java | 3 +-
.../apache/sis/internal/storage/StoreMetadata.java | 12 +++
.../storage/folder/FolderStoreProvider.java | 16 ++--
.../sis/internal/storage/folder/package-info.java | 2 +-
.../org/apache/sis/storage/DataStoreRegistry.java | 85 +++++++++++++++++-----
.../org/apache/sis/storage/StorageConnector.java | 18 +++--
.../org.apache.sis.storage.DataStoreProvider | 1 +
.../sis/internal/storage/folder/StoreTest.java | 5 +-
9 files changed, 102 insertions(+), 52 deletions(-)
diff --git a/application/sis-javafx/src/main/java/org/apache/sis/internal/gui/ResourceLoader.java b/application/sis-javafx/src/main/java/org/apache/sis/internal/gui/ResourceLoader.java
index 988a820..d18916c 100644
--- a/application/sis-javafx/src/main/java/org/apache/sis/internal/gui/ResourceLoader.java
+++ b/application/sis-javafx/src/main/java/org/apache/sis/internal/gui/ResourceLoader.java
@@ -21,7 +21,6 @@ import java.net.URL;
import java.io.File;
import java.nio.file.Path;
import java.nio.file.Paths;
-import java.nio.file.Files;
import java.io.IOException;
import java.net.URISyntaxException;
import java.util.function.Consumer;
@@ -36,7 +35,6 @@ import org.apache.sis.storage.DataStoreException;
import org.apache.sis.storage.DataStores;
import org.apache.sis.util.collection.Cache;
import org.apache.sis.util.resources.Vocabulary;
-import org.apache.sis.internal.storage.folder.FolderStoreProvider;
import org.apache.sis.internal.storage.io.IOUtilities;
import org.apache.sis.storage.DataStore;
@@ -154,16 +152,6 @@ public final class ResourceLoader extends Task<Resource> {
* Loads the resource after we verified that it is not in the cache.
*/
private DataStore load() throws DataStoreException {
- Object input = source;
- if (input instanceof StorageConnector) {
- input = ((StorageConnector) input).getStorage();
- }
- if ((input instanceof File && ((File) input).isDirectory()) ||
- (input instanceof Path && Files.isDirectory((Path) input)))
- {
- return FolderStoreProvider.INSTANCE.open((source instanceof StorageConnector)
- ? (StorageConnector) source : new StorageConnector(input));
- }
return DataStores.open(source);
}
diff --git a/storage/sis-netcdf/src/main/java/org/apache/sis/storage/netcdf/NetcdfStoreProvider.java b/storage/sis-netcdf/src/main/java/org/apache/sis/storage/netcdf/NetcdfStoreProvider.java
index 9459c54..50e97a9 100644
--- a/storage/sis-netcdf/src/main/java/org/apache/sis/storage/netcdf/NetcdfStoreProvider.java
+++ b/storage/sis-netcdf/src/main/java/org/apache/sis/storage/netcdf/NetcdfStoreProvider.java
@@ -74,7 +74,8 @@ import org.apache.sis.util.Version;
@StoreMetadata(formatName = NetcdfStoreProvider.NAME,
fileSuffixes = "nc",
capabilities = Capability.READ,
- resourceTypes = {Aggregate.class, FeatureSet.class, GridCoverageResource.class})
+ resourceTypes = {Aggregate.class, FeatureSet.class, GridCoverageResource.class},
+ isLowPriority = true)
public class NetcdfStoreProvider extends DataStoreProvider {
/**
* The format name.
diff --git a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/StoreMetadata.java b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/StoreMetadata.java
index 292d9c8..7be37c6 100644
--- a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/StoreMetadata.java
+++ b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/StoreMetadata.java
@@ -92,4 +92,16 @@ public @interface StoreMetadata {
* @return information about the expected resource types which might be encounter with this format.
*/
Class<? extends Resource>[] resourceTypes() default {};
+
+ /**
+ * Returns {@code true} if the data store should be tested last when searching for a data store capable
+ * to open a given file. This method should return {@code true} if the data store claims to be able to
+ * open a wide variety of files, in order to allow specialized data stores to be tested before generic
+ * data stores.
+ *
+ * <p>If many data stores have low priority, the ordering between them is unspecified.</p>
+ *
+ * @return {@code true} if this data store should be tested after all "normal priority" data stores.
+ */
+ boolean isLowPriority() default false;
}
diff --git a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/folder/FolderStoreProvider.java b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/folder/FolderStoreProvider.java
index c630652..7ae0ade 100644
--- a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/folder/FolderStoreProvider.java
+++ b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/folder/FolderStoreProvider.java
@@ -52,9 +52,9 @@ import org.apache.sis.setup.OptionKey;
/**
- * The provider of {@link Store} instances. This provider is intentionally <strong>not</strong> registered
- * in {@code META-INF/services/org.apache.sis.storage.DataStoreProvider} because is will open any directory,
- * which may conflict with other providers opening only directory with some specific content.
+ * The provider of {@link Store} instances. This provider is intentionally registered with lowest priority
+ * because it will open any directory, which may conflict with other providers opening only directory with
+ * some specific content.
*
* @author Johann Sorel (Geomatys)
* @author Martin Desruisseaux (Geomatys)
@@ -64,7 +64,8 @@ import org.apache.sis.setup.OptionKey;
*/
@StoreMetadata(formatName = FolderStoreProvider.NAME,
capabilities = {Capability.READ, Capability.WRITE},
- resourceTypes = {Aggregate.class, FeatureSet.class, GridCoverageResource.class})
+ resourceTypes = {Aggregate.class, FeatureSet.class, GridCoverageResource.class},
+ isLowPriority = true)
public final class FolderStoreProvider extends DataStoreProvider {
/**
* A short name or abbreviation for the data format.
@@ -116,14 +117,9 @@ public final class FolderStoreProvider extends DataStoreProvider {
}
/**
- * The unique instance of this provider.
- */
- public static final FolderStoreProvider INSTANCE = new FolderStoreProvider();
-
- /**
* Creates a new provider.
*/
- private FolderStoreProvider() {
+ public FolderStoreProvider() {
}
/**
diff --git a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/folder/package-info.java b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/folder/package-info.java
index 9dfa98b..906c57f 100644
--- a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/folder/package-info.java
+++ b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/folder/package-info.java
@@ -19,7 +19,7 @@
* {@link org.apache.sis.storage.DataStore} implementation for a folder containing an arbitrary amount of data files.
*
* @author Johann Sorel (Geomatys)
- * @version 1.0
+ * @version 1.1
* @since 0.8
* @module
*/
diff --git a/storage/sis-storage/src/main/java/org/apache/sis/storage/DataStoreRegistry.java b/storage/sis-storage/src/main/java/org/apache/sis/storage/DataStoreRegistry.java
index 524880e..e2c8383 100644
--- a/storage/sis-storage/src/main/java/org/apache/sis/storage/DataStoreRegistry.java
+++ b/storage/sis-storage/src/main/java/org/apache/sis/storage/DataStoreRegistry.java
@@ -43,7 +43,7 @@ import org.apache.sis.util.ArraysExt;
* on the part of the caller.
*
* @author Martin Desruisseaux (Geomatys)
- * @version 1.0
+ * @version 1.1
* @since 0.4
* @module
*/
@@ -139,17 +139,54 @@ final class DataStoreRegistry {
}
/**
+ * The kind of providers to test. The provider are divided in 4 categories depending on whether
+ * the file suffix matches the suffix expected by the provider, and whether the provider should
+ * be tested last for giving a chance to specialized providers to open the file.
+ */
+ private enum Category {
+ /** The provider can be tested now and the file suffix matches. */
+ SUFFIX_MATCH(true, false),
+
+ /** The provider could be tested now but the file suffix does not match. */
+ IGNORE_SUFFIX(false, false),
+
+ /** The provider should be tested last, but the file suffix matches. */
+ DEFERRED(true, true),
+
+ /** The provider should be tested last because it is too generic. */
+ DEFERRED_IGNORE_SUFFIX(false, true);
+
+ /** Whether this category checks if suffix matches. */
+ final boolean useSuffix;
+
+ /** Whether this category is for providers to test last. */
+ final boolean isLowPriority;
+
+ /** Creates a new enumeration value. */
+ private Category(final boolean useSuffix, final boolean isLowPriority) {
+ this.useSuffix = useSuffix;
+ this.isLowPriority = isLowPriority;
+ }
+
+ /** Returns the categories, optionally ignoring file suffix. */
+ static Category[] values(final boolean useSuffix) {
+ return useSuffix ? values() : new Category[] {IGNORE_SUFFIX, DEFERRED_IGNORE_SUFFIX};
+ }
+ }
+
+ /**
* Implementation of {@link #probeContentType(Object)} and {@link #open(Object)}.
*
* @param storage the input/output object as a URL, file, image input stream, <i>etc.</i>.
* @param open {@code true} for creating a {@link DataStore}, or {@code false} if not needed.
+ * @return the result, or {@code null} if the format is not recognized and {@code open} is {@code false}.
* @throws UnsupportedStorageException if no {@link DataStoreProvider} is found for a given storage object.
* @throws DataStoreException if an error occurred while opening the storage.
*
* @todo Iterate on {@code ServiceLoader.Provider.type()} on JDK9.
*/
private ProbeProviderPair lookup(final Object storage, final boolean open) throws DataStoreException {
- StorageConnector connector;
+ StorageConnector connector; // Will be reset to `null` if it shall not be closed.
if (storage instanceof StorageConnector) {
connector = (StorageConnector) storage;
} else {
@@ -158,20 +195,23 @@ final class DataStoreRegistry {
/*
* If we can get a filename extension from the given storage (file, URL, etc.), then we may perform two iterations
* on the provider list. The first iteration will use only the providers which declare capability to read files of
- * that suffix (matchCondition = TRUE). Only if no provider has been able to read that file, we will do a second
- * iteration on other providers (matchCondition = FALSE). The intent is to avoid DataStoreProvider.probeContent(…)
+ * that suffix (Category.SUFFIX_MATCH). Only if no provider has been able to read that file, we will do a second
+ * iteration on other providers (Category.IGNORE_SUFFIX). The intent is to avoid DataStoreProvider.probeContent(…)
* invocations loading large dependencies.
*/
- final String extension = connector.getFileExtension();
- Boolean matchCondition = (extension != null && !extension.isEmpty()) ? Boolean.TRUE : null;
+ final String extension = connector.getFileExtension();
+ final boolean useSuffix = !(extension == null || extension.isEmpty());
+ final Category[] categories = Category.values(useSuffix);
+ ProbeProviderPair selected = null;
final List<ProbeProviderPair> needMoreBytes = new LinkedList<>();
- ProbeProviderPair selected = null;
try {
-search: do {
+search: for (int ci=0; ci < categories.length; ci++) {
+ final Category category = categories[ci];
/*
- * All usages of 'loader' and its 'providers' iterator must be protected in a synchronized block,
+ * All usages of `loader` and its `providers` iterator must be protected in a synchronized block,
* because ServiceLoader is not thread-safe. We try to keep the synhronization block as small as
- * possible for less contention. In particular, the probeContent(connector) method call may be costly.
+ * possible for less contention. In particular, the `probeContent(connector)` method call may be
+ * costly.
*/
final Iterator<DataStoreProvider> providers;
DataStoreProvider provider;
@@ -180,10 +220,20 @@ search: do {
provider = providers.hasNext() ? providers.next() : null;
}
while (provider != null) {
- boolean accept = true;
- if (matchCondition != null) {
- final StoreMetadata md = provider.getClass().getAnnotation(StoreMetadata.class);
- accept = (md != null && ArraysExt.containsIgnoreCase(md.fileSuffixes(), extension)) == matchCondition;
+ /*
+ * Check if the provider should be tested in current iteration.
+ * The criteria for testing a provider is determined by comparing
+ * provider metadata with the category tested in current iteration.
+ */
+ boolean accept;
+ final StoreMetadata md = provider.getClass().getAnnotation(StoreMetadata.class);
+ if (md == null) {
+ accept = (ci == 0); // If no metadata, test only in first iteration.
+ } else {
+ accept = (md.isLowPriority() == category.isLowPriority);
+ if (accept & useSuffix) {
+ accept = ArraysExt.containsIgnoreCase(md.fileSuffixes(), extension) == category.useSuffix;
+ }
}
if (accept) {
final ProbeResult probe = provider.probeContent(connector);
@@ -219,7 +269,7 @@ search: do {
}
}
/*
- * If any provider did not had enough bytes for answering the 'probeContent(…)' question,
+ * If any provider did not had enough bytes for answering the `probeContent(…)` question,
* get more bytes and try again. We try to prefetch more bytes only if we have no choice
* in order to avoid latency on network connection.
*/
@@ -242,10 +292,9 @@ search: do {
/*
* If we filtered providers by the file extension without finding a suitable provider,
* try again with all other providers (even if they are for another file extension).
- * We do that by changing 'matchCondition' from TRUE to FALSE. In all other cases,
- * we stop the search.
+ * We do that by changing moving to the next `Category`.
*/
- } while (matchCondition != null && (matchCondition = !matchCondition) == false);
+ }
/*
* If a provider has been found, or if a provider returned UNDETERMINED, use that one
* for opening a DataStore. Note that if more than one provider returned UNDETERMINED,
diff --git a/storage/sis-storage/src/main/java/org/apache/sis/storage/StorageConnector.java b/storage/sis-storage/src/main/java/org/apache/sis/storage/StorageConnector.java
index e04113f..b0b9e5f 100644
--- a/storage/sis-storage/src/main/java/org/apache/sis/storage/StorageConnector.java
+++ b/storage/sis-storage/src/main/java/org/apache/sis/storage/StorageConnector.java
@@ -1091,14 +1091,16 @@ public class StorageConnector implements Serializable {
c = getView(ByteBuffer.class);
if (reset(c)) { // reset(c) as a matter of principle, but (c != null) would have worked.
final ByteBuffer buffer = (ByteBuffer) c.view;
- final int p = buffer.limit();
- final long mark = input.getStreamPosition();
- input.seek(Math.addExact(mark, p));
- final int n = input.read(buffer.array(), p, buffer.capacity() - p);
- input.seek(mark);
- if (n > 0) {
- buffer.limit(p + n);
- return true;
+ if (buffer != null) {
+ final int p = buffer.limit();
+ final long mark = input.getStreamPosition();
+ input.seek(Math.addExact(mark, p));
+ final int n = input.read(buffer.array(), p, buffer.capacity() - p);
+ input.seek(mark);
+ if (n > 0) {
+ buffer.limit(p + n);
+ return true;
+ }
}
}
}
diff --git a/storage/sis-storage/src/main/resources/META-INF/services/org.apache.sis.storage.DataStoreProvider b/storage/sis-storage/src/main/resources/META-INF/services/org.apache.sis.storage.DataStoreProvider
index 62b5285..5c1a70f 100644
--- a/storage/sis-storage/src/main/resources/META-INF/services/org.apache.sis.storage.DataStoreProvider
+++ b/storage/sis-storage/src/main/resources/META-INF/services/org.apache.sis.storage.DataStoreProvider
@@ -1,3 +1,4 @@
org.apache.sis.internal.storage.xml.StoreProvider
org.apache.sis.internal.storage.wkt.StoreProvider
org.apache.sis.internal.storage.csv.StoreProvider
+org.apache.sis.internal.storage.folder.FolderStoreProvider
diff --git a/storage/sis-storage/src/test/java/org/apache/sis/internal/storage/folder/StoreTest.java b/storage/sis-storage/src/test/java/org/apache/sis/internal/storage/folder/StoreTest.java
index 236116a..1f04666 100644
--- a/storage/sis-storage/src/test/java/org/apache/sis/internal/storage/folder/StoreTest.java
+++ b/storage/sis-storage/src/test/java/org/apache/sis/internal/storage/folder/StoreTest.java
@@ -43,7 +43,7 @@ import static org.junit.Assume.assumeTrue;
*
* @author Johann Sorel (Geomatys)
* @author Martin Desruisseaux (Geomatys)
- * @version 1.0
+ * @version 1.1
* @since 0.8
* @module
*/
@@ -89,11 +89,12 @@ public final strictfp class StoreTest extends TestCase {
*/
@Test
public void testSearchProviderParameter() throws URISyntaxException, DataStoreException, IOException {
+ final FolderStoreProvider provider = new FolderStoreProvider();
final Set<String> identifiers = new HashSet<>(Arrays.asList("Sample 1", "Sample 2", "Sample 3", "data4"));
final ParameterValueGroup params = FolderStoreProvider.PARAMETERS.createValue();
params.parameter("location").setValue(testDirectory());
params.parameter("format").setValue("XML");
- try (Store store = (Store) FolderStoreProvider.INSTANCE.open(params)) {
+ try (Store store = (Store) provider.open(params)) {
assertEquals("Expected one less data store.", 3, store.components().size());
verifyContent(store, identifiers);
}