sis-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From desruisse...@apache.org
Subject [sis] 02/02: StorageConnector.getStorageAs(…) should not try to open a channel on directory. This is a workaround for https://bugs.openjdk.java.net/browse/JDK-8080629
Date Tue, 11 Jun 2019 10:42:00 GMT
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 <martin.desruisseaux@geomatys.com>
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);
+        }
     }
 }


Mime
View raw message