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 9640679acd6867a0a2b9b0d69e86ef7a63d5f22c Author: Martin Desruisseaux <[email protected]> AuthorDate: Tue Jun 11 12:38:17 2019 +0200 StorageConnector.getStorageAs(…) should not try to open a channel on directory. This is a workaround for https://bugs.openjdk.java.net/browse/JDK-8080629 --- .../main/java/org/apache/sis/xml/NilReason.java | 2 +- .../apache/sis/internal/storage/folder/Store.java | 3 --- .../sis/internal/storage/io/ChannelFactory.java | 31 +++++++++++----------- .../org/apache/sis/storage/StorageConnector.java | 2 +- .../sis/storage/UnsupportedStorageException.java | 25 ++++++++++++++--- 5 files changed, 39 insertions(+), 24 deletions(-) diff --git a/core/sis-metadata/src/main/java/org/apache/sis/xml/NilReason.java b/core/sis-metadata/src/main/java/org/apache/sis/xml/NilReason.java index 91864b4..5f511ca 100644 --- a/core/sis-metadata/src/main/java/org/apache/sis/xml/NilReason.java +++ b/core/sis-metadata/src/main/java/org/apache/sis/xml/NilReason.java @@ -460,7 +460,7 @@ public final class NilReason implements Serializable { /** * Returns {@code true} if the given object may be one of the sentinel values * created by {@link #createNilPrimitive(Class)}. This method only checks the value. - * If this method returns {@code true}, then the caller still need to check the actual instance using the + * If this method returns {@code true}, then the caller still needs to check the actual instance using the * {@link PrimitiveTypeProperties} class. The purpose of this method is to filter the values that can not * be sentinel values, in order to avoid the synchronization done by {@code PrimitiveTypeProperties}. */ diff --git a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/folder/Store.java b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/folder/Store.java index 1bc6157..61e1842 100644 --- a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/folder/Store.java +++ b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/folder/Store.java @@ -32,7 +32,6 @@ import java.nio.file.DirectoryStream; import java.nio.file.DirectoryIteratorException; import java.io.IOException; import java.io.UncheckedIOException; -import java.nio.file.OpenOption; import org.opengis.util.GenericName; import org.opengis.util.NameFactory; import org.opengis.util.NameSpace; @@ -54,7 +53,6 @@ import org.apache.sis.internal.system.DefaultFactories; import org.apache.sis.internal.storage.MetadataBuilder; import org.apache.sis.internal.storage.StoreUtilities; import org.apache.sis.internal.storage.StoreResource; -import org.apache.sis.internal.storage.io.ChannelFactory; import org.apache.sis.internal.storage.Resources; import org.apache.sis.storage.event.ChangeEvent; import org.apache.sis.storage.event.ChangeListener; @@ -312,7 +310,6 @@ class Store extends DataStore implements StoreResource, Aggregate, DirectoryStre connector.setOption(OptionKey.LOCALE, locale); connector.setOption(OptionKey.TIMEZONE, timezone); connector.setOption(OptionKey.ENCODING, encoding); - connector.setOption(OptionKey.OPEN_OPTIONS, new OpenOption[] {ChannelFactory.REQUIRE_REGULAR_FILE}); try { if (componentProvider == null) { next = DataStores.open(connector); // May throw UnsupportedStorageException. diff --git a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/io/ChannelFactory.java b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/io/ChannelFactory.java index a01fe08..41c05e4 100644 --- a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/io/ChannelFactory.java +++ b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/io/ChannelFactory.java @@ -80,17 +80,6 @@ public abstract class ChannelFactory { StandardOpenOption.APPEND, StandardOpenOption.TRUNCATE_EXISTING, StandardOpenOption.DELETE_ON_CLOSE); /** - * A customized option for instructing {@link #prepare(Object, String, boolean, OpenOption...)} - * to not try to open directories or non-existent files. By default, {@link Path} to non-regular - * files will cause an {@link IOException} to be thrown. But if this option is provided, then we - * will rather behave as if {@link Path} was an unknown type. This will cause store providers to - * not try to open that file, which gives the caller a chance to fallback on its own process. - */ - public static final OpenOption REQUIRE_REGULAR_FILE = new OpenOption() { - @Override public String toString() {return "REQUIRE_REGULAR_FILE";} - }; - - /** * For subclass constructors. */ ChannelFactory() { @@ -98,7 +87,7 @@ public abstract class ChannelFactory { /** * Returns a byte channel factory from the given input or output, - * or {@code null} if the given input/output is of unknown type. + * or {@code null} if the given input/output is unsupported. * More specifically: * * <ul> @@ -109,11 +98,12 @@ public abstract class ChannelFactory { * and the {@code allowWriteOnly} argument is {@code true}, * then the factory will return that output directly or indirectly as a wrapper.</li> * <li>If the given storage if a {@link Path}, {@link File}, {@link URL}, {@link URI} - * or {@link CharSequence}, then the factory will open new channels on demand.</li> + * or {@link CharSequence} and the file is not a directory, then the factory will + * open new channels on demand.</li> * </ul> * * The given options are used for opening the channel on a <em>best effort basis</em>. - * In particular, even if the caller provided the {@code WRITE} option, he still needs + * In particular, even if the caller provided the {@code WRITE} option, (s)he still needs * to verify if the returned channel implements {@link java.nio.channels.WritableByteChannel}. * This is because the channel may be opened by {@link URL#openStream()}, in which case the * options are ignored. @@ -123,7 +113,7 @@ public abstract class ChannelFactory { * {@code APPEND}, {@code TRUNCATE_EXISTING}, {@code DELETE_ON_CLOSE}. We reject those options * because this method is primarily designed for readable channels, with optional data edition. * Since the write option is not guaranteed to be honored, we have to reject the options that - * would alter significatively the channel behavior depending on whether we have been able to + * would alter significantly the channel behavior depending on whether we have been able to * honor the options or not.</p> * * @param storage the stream or the file to open, or {@code null}. @@ -249,9 +239,18 @@ public abstract class ChannelFactory { } }; } + /* + * Handle path to directory as an unsupported input. Attempts to read bytes from that channel would cause an + * IOException to be thrown. On Java 10, that IOException does not occur at channel openning time but rather + * at reading time. See https://bugs.openjdk.java.net/browse/JDK-8080629 for more information. + * + * If the file does not exist, we let NoSuchFileException to be thrown by the code below because non-existant + * file is probably an error. This is not the same situation than a directory, which can not be opened by this + * class but may be opened by some specialized DataStore implementations. + */ if (storage instanceof Path) { final Path path = (Path) storage; - if (!optionSet.remove(REQUIRE_REGULAR_FILE) || Files.isRegularFile(path)) { + if (!Files.isDirectory(path)) { return new ChannelFactory() { @Override public ReadableByteChannel readable(String filename, WarningListeners<DataStore> listeners) throws IOException { return Files.newByteChannel(path, optionSet); 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 bd72e7c..655af4e 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 @@ -889,7 +889,7 @@ public class StorageConnector implements Serializable { * @throws DataStoreException if the storage can not be reseted. */ private void reset() throws DataStoreException { - if (views != null && !reset(views.get(null))) { + if (views != null && !views.isEmpty() && !reset(views.get(null))) { throw new ForwardOnlyStorageException(Resources.format( Resources.Keys.StreamIsReadOnce_1, getStorageName())); } diff --git a/storage/sis-storage/src/main/java/org/apache/sis/storage/UnsupportedStorageException.java b/storage/sis-storage/src/main/java/org/apache/sis/storage/UnsupportedStorageException.java index 5180098..e8edb6a 100644 --- a/storage/sis-storage/src/main/java/org/apache/sis/storage/UnsupportedStorageException.java +++ b/storage/sis-storage/src/main/java/org/apache/sis/storage/UnsupportedStorageException.java @@ -17,8 +17,12 @@ package org.apache.sis.storage; import java.util.Locale; +import java.io.File; +import java.nio.file.Path; +import java.nio.file.Files; import java.nio.file.OpenOption; import org.apache.sis.util.Classes; +import org.apache.sis.util.Workaround; import org.apache.sis.internal.storage.Resources; import org.apache.sis.internal.storage.io.IOUtilities; @@ -29,7 +33,7 @@ import org.apache.sis.internal.storage.io.IOUtilities; * can not handle the given input or output object. * * @author Martin Desruisseaux (Geomatys) - * @version 0.8 + * @version 1.0 * @since 0.4 * @module */ @@ -100,7 +104,22 @@ public class UnsupportedStorageException extends IllegalOpenParameterException { */ public UnsupportedStorageException(final Locale locale, final String format, final Object storage, final OpenOption... options) { super(locale, IOUtilities.isWrite(options) ? Resources.Keys.IllegalOutputTypeForWriter_2 - : Resources.Keys.IllegalInputTypeForReader_2, - format, Classes.getClass(storage)); + : Resources.Keys.IllegalInputTypeForReader_2, format, type(storage)); + } + + /** + * Returns the type of the given storage, with a special case for files or paths to directories. + * This is a work around for RFE #4093999 in Sun's bug database + * ("Relax constraint on placement of this()/super() call in constructors"). + */ + @Workaround(library="JDK", version="10") + private static Object type(final Object storage) { + if ((storage instanceof File && ((File) storage).isDirectory()) || + (storage instanceof Path && Files.isDirectory((Path) storage))) + { + return "Directory"; + } else { + return Classes.getClass(storage); + } } }
