sis-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From desruisse...@apache.org
Subject svn commit: r1509666 - in /sis/branches/JDK7/storage/sis-storage/src/main/java/org/apache/sis: internal/storage/IOUtilities.java storage/DataStore.java storage/DataStoreProvider.java storage/DataStores.java storage/ProbeResult.java
Date Fri, 02 Aug 2013 12:43:52 GMT
Author: desruisseaux
Date: Fri Aug  2 12:43:52 2013
New Revision: 1509666

URL: http://svn.apache.org/r1509666
Log:
More conservative approach about exceptions and allowed OpenOptions in internal IOUtilities.
Documentation clarifications.

Modified:
    sis/branches/JDK7/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/IOUtilities.java
    sis/branches/JDK7/storage/sis-storage/src/main/java/org/apache/sis/storage/DataStore.java
    sis/branches/JDK7/storage/sis-storage/src/main/java/org/apache/sis/storage/DataStoreProvider.java
    sis/branches/JDK7/storage/sis-storage/src/main/java/org/apache/sis/storage/DataStores.java
    sis/branches/JDK7/storage/sis-storage/src/main/java/org/apache/sis/storage/ProbeResult.java

Modified: sis/branches/JDK7/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/IOUtilities.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK7/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/IOUtilities.java?rev=1509666&r1=1509665&r2=1509666&view=diff
==============================================================================
--- sis/branches/JDK7/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/IOUtilities.java
[UTF-8] (original)
+++ sis/branches/JDK7/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/IOUtilities.java
[UTF-8] Fri Aug  2 12:43:52 2013
@@ -16,6 +16,11 @@
  */
 package org.apache.sis.internal.storage;
 
+import java.util.Set;
+import java.util.EnumSet;
+import java.util.HashSet;
+import java.util.Collections;
+import java.util.Arrays;
 import java.util.Locale;
 import java.io.File;
 import java.io.FileInputStream;
@@ -41,6 +46,7 @@ import java.nio.file.Files;
 import java.nio.file.InvalidPathException;
 import java.nio.file.FileSystemNotFoundException;
 import java.nio.file.OpenOption;
+import java.nio.file.StandardOpenOption;
 import java.nio.charset.StandardCharsets;
 
 
@@ -61,6 +67,12 @@ import java.nio.charset.StandardCharsets
  */
 public final class IOUtilities extends Static {
     /**
+     * Options to be rejected by {@link #open(Object, String, OpenOption[])} for safety reasons.
+     */
+    private static final Set<StandardOpenOption> ILLEGAL_OPTIONS = EnumSet.of(
+            StandardOpenOption.APPEND, StandardOpenOption.TRUNCATE_EXISTING, StandardOpenOption.DELETE_ON_CLOSE);
+
+    /**
      * Do not allow instantiation of this class.
      */
     private IOUtilities() {
@@ -285,14 +297,12 @@ public final class IOUtilities extends S
             return new File(uri);
         } catch (IllegalArgumentException cause) {
             /*
-             * Typically happen when the URI contains fragment that can not be represented
-             * in a File (e.g. a Query part), so it could be considered as if the URI with
-             * the fragment part can not represent an existing file.
+             * Typically happen when the URI scheme is not "file". But may also happen if
the
+             * URI contains fragment that can not be represented in a File (e.g. a Query
part).
+             * The IllegalArgumentException does not allow us to distinguish those cases.
              */
-            final MalformedURLException e = new MalformedURLException(Exceptions.formatChainedMessages(
-                    null, Errors.format(Errors.Keys.IllegalArgumentValue_2, "URL", url),
cause));
-            e.initCause(cause);
-            throw e;
+            throw new IOException(Exceptions.formatChainedMessages(null,
+                    Errors.format(Errors.Keys.IllegalArgumentValue_2, "URL", url), cause),
cause);
         }
     }
 
@@ -323,9 +333,20 @@ public final class IOUtilities extends S
         try {
             return Paths.get(uri);
         } catch (IllegalArgumentException | FileSystemNotFoundException cause) {
-            final MalformedURLException e = new MalformedURLException(Exceptions.formatChainedMessages(
-                    null, Errors.format(Errors.Keys.IllegalArgumentValue_2, "URL", url),
cause));
-            e.initCause(cause);
+            final String message = Exceptions.formatChainedMessages(null,
+                    Errors.format(Errors.Keys.IllegalArgumentValue_2, "URL", url), cause);
+            /*
+             * If the exception is IllegalArgumentException, then the URI scheme has been
recognized
+             * but the URI syntax is illegal for that file system. So we can consider that
the URL is
+             * malformed in regard to the rules of that particular file system.
+             */
+            final IOException e;
+            if (cause instanceof IllegalArgumentException) {
+                e = new MalformedURLException(message);
+                e.initCause(cause);
+            } else {
+                e = new IOException(message, cause);
+            }
             throw e;
         }
     }
@@ -352,7 +373,10 @@ public final class IOUtilities extends S
         if (path == null) {
             return null;
         }
-        // Check if the path seems to be an ordinary file.
+        /*
+         * Check if the path seems to be a local file. Those paths are assumed never encoded.
+         * The heuristic rules applied here may change in any future SIS version.
+         */
         if (path.indexOf('?') < 0 && path.indexOf('#') < 0) {
             final int s = path.indexOf(':');
             /*
@@ -366,11 +390,14 @@ public final class IOUtilities extends S
         }
         final URL url = new URL(path);
         final String scheme = url.getProtocol();
-        if (scheme != null) {
-            if (scheme.equalsIgnoreCase("file")) {
-                return toFile(url, encoding);
-            }
+        if (scheme != null && scheme.equalsIgnoreCase("file")) {
+            return toFile(url, encoding);
         }
+        /*
+         * Leave the URL in its original encoding on the assumption that this is the encoding
expected by
+         * the server. This is different than the policy for URI, because the later are always
in UTF-8.
+         * If a URI is needed, callers should use toURI(url, encoding).
+         */
         return url;
     }
 
@@ -389,26 +416,49 @@ public final class IOUtilities extends S
      * 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
      * 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.
+     *
+     * <p>The following options are illegal and will cause an exception to be thrown
if provided:
+     * {@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
+     * honor the options or not.</p>
      *
      * @param  input The file to open, or {@code null}.
-     * @param  encoding If the URL is encoded in a {@code application/x-www-form-urlencoded}
-     *         MIME format, the character encoding (normally {@code "UTF-8"}). If the URL
is
-     *         not encoded, then {@code null}. This argument is ignored if the given path
does
-     *         not need to be converted from URL to {@code File}.
-     * @param  options The options to use for creating a new byte channel, or an empty set
for read-only.
-     * @return The input stream for the given file, or {@code null} if the given type is
unknown.
+     * @param  encoding If the input is an encoded URL, the character encoding (normally
{@code "UTF-8"}).
+     *         If the URL is not encoded, then {@code null}. This argument is ignored if
the given input
+     *         does not need to be converted from URL to {@code File}.
+     * @param  options The options to use for creating a new byte channel. Can be null or
empty for read-only.
+     * @return The channel for the given input, or {@code null} if the given input is of
unknown type.
      * @throws IOException If an error occurred while opening the given file.
      */
     public static ReadableByteChannel open(Object input, final String encoding, OpenOption...
options) throws IOException {
+        /*
+         * Unconditionally verify the options, even if we may not use them.
+         */
+        final Set<OpenOption> optionSet;
+        if (options == null || options.length == 0) {
+            optionSet = Collections.emptySet();
+        } else {
+            optionSet = new HashSet<>(Arrays.asList(options));
+            optionSet.add(StandardOpenOption.READ);
+            if (optionSet.removeAll(ILLEGAL_OPTIONS)) {
+                throw new IllegalArgumentException(Errors.format(Errors.Keys.IllegalArgumentValue_2,
+                        "options", Arrays.toString(options)));
+            }
+        }
+        /*
+         * Check for inputs that are already readable channels or input streams. We perform
an explicit check
+         * for FileInputStream even if  Channels.newChannel(InputStream)  does this check
implicitly, because
+         * Channels.newChannel(…) restricts itself to the exact FileInputStream class while
we want to invoke
+         * getChannel() for any subclasses.
+         */
         if (input instanceof ReadableByteChannel) {
             return (ReadableByteChannel) input;
         }
         if (input instanceof InputStream) {
-            /*
-             * Channels.newChannel(InputStream) checks for FileInputStream, but it requires
that exact class.
-             * Concequently we have to perform our own check if we want to allow any FileInputStream
subclass
-             * to have their 'getChannel()' method invoked.
-             */
             if (input instanceof FileInputStream) {
                 return ((FileInputStream) input).getChannel();
             }
@@ -418,40 +468,21 @@ public final class IOUtilities extends S
          * In the following cases, we will try hard to convert to Path objects before to
fallback
          * on File, URL or URI, because only Path instances allow us to use the given OpenOptions.
          */
-        if (input instanceof CharSequence) { // Needs to be before the check for File or
URL.
-            input = toFileOrURL(input.toString(), encoding);
-        }
-        /*
-         * If the input is a File or a CharSequence that we have been able to convert to
a File,
-         * try to convert to a Path in order to be able to use the OpenOptions. Only if we
fail
-         * to convert to a Path (which is unlikely), we will use directly the File.
-         */
-        if (input instanceof File) {
+        if (input instanceof URL) {
             try {
-                input = ((File) input).toPath();
-            } catch (InvalidPathException e) {
-                // Unlikely to happen. But if it happens anyway, try to open the channel
in a
-                // way less surprising for the user (closer to the object he has specified).
-                final ReadableByteChannel channel;
-                try {
-                    channel = new FileInputStream((File) input).getChannel();
-                } catch (IOException ioe) {
-                    ioe.addSuppressed(e);
-                    throw ioe;
-                }
-                // We have been able to create a channel, maybe not with the given OpenOptions.
-                // But the exception was nevertheless unexpected, so log its stack trace
in order
-                // to allow the developer to check if there is something wrong.
-                Logging.unexpectedException(Logging.getLogger("org.apache.sis.storage"),
IOUtilities.class, "open", e);
-                return channel;
+                input = toPath((URL) input, encoding);
+            } catch (IOException e) {
+                // This is normal if the URL uses HTTP or FTP protocol for instance.
+                // Log the exception at FINE level without stack trace. We will open
+                // the channel later using the URL instead than using the Path.
+                recoverableException(e);
             }
-        }
-        /*
-         * If the user gave us a URI, try again to convert to a Path for the same reasons
than the above File case.
-         * A failure here is much more likely than in the File case, because JDK7 does not
provide file systems for
-         * HTTP or FTP protocols by default.
-         */
-        if (input instanceof URI) { // Needs to be before the check for URL.
+        } else if (input instanceof URI) {
+            /*
+             * If the user gave us a URI, try to convert to a Path before to fallback to
URL, in order to be
+             * able to use the given OpenOptions.  Note that the conversion to Path is likely
to fail if the
+             * URL uses HTTP or FTP protocols, because JDK7 does not provide file systems
for them by default.
+             */
             final URI uri = (URI) input;
             try {
                 input = Paths.get(uri);
@@ -462,20 +493,60 @@ public final class IOUtilities extends S
                     ioe.addSuppressed(e);
                     throw ioe;
                 }
-                // We have been able to create a channel, maybe not with the given OpenOptions.
-                // Log the exception at a fine level and without stack trace, because it
was probably normal.
-                Logging.recoverableException(Logging.getLogger("org.apache.sis.storage"),
IOUtilities.class, "open", e);
+                // We have been able to convert to URL, but the given OpenOptions may not
be used.
+                // Log the exception at FINE level without stack trace, because the exception
is
+                // probably a normal behavior in this context.
+                recoverableException(e);
+            }
+        } else {
+            if (input instanceof CharSequence) { // Needs to be before the check for File
or URL.
+                input = toFileOrURL(input.toString(), encoding);
+            }
+            /*
+             * If the input is a File or a CharSequence that we have been able to convert
to a File,
+             * try to convert to a Path in order to be able to use the OpenOptions. Only
if we fail
+             * to convert to a Path (which is unlikely), we will use directly the File.
+             */
+            if (input instanceof File) {
+                try {
+                    input = ((File) input).toPath();
+                } catch (InvalidPathException e) {
+                    // Unlikely to happen. But if it happens anyway, try to open the channel
in a
+                    // way less surprising for the user (closer to the object he has specified).
+                    final ReadableByteChannel channel;
+                    try {
+                        channel = new FileInputStream((File) input).getChannel();
+                    } catch (IOException ioe) {
+                        ioe.addSuppressed(e);
+                        throw ioe;
+                    }
+                    // We have been able to create a channel, maybe not with the given OpenOptions.
+                    // But the exception was nevertheless unexpected, so log its stack trace
in order
+                    // to allow the developer to check if there is something wrong.
+                    Logging.unexpectedException(Logging.getLogger("org.apache.sis.storage"),
IOUtilities.class, "open", e);
+                    return channel;
+                }
             }
         }
+        /*
+         * One last check for URL. The URL may be either the given input if we have not been
able
+         * to convert it to a Path, or a URI, File or CharSequence input converted to URL.
Do not
+         * try to convert the URL to a Path, because this has already been tried before this
point.
+         */
         if (input instanceof URL) {
             return Channels.newChannel(((URL) input).openStream());
         }
         if (input instanceof Path) {
-            if (options == null) {
-                options = new OpenOption[0];
-            }
-            return Files.newByteChannel((Path) input, options);
+            return Files.newByteChannel((Path) input, optionSet);
         }
         return null;
     }
+
+    /**
+     * Invoked for reporting exceptions that may be normal behavior. This method logs
+     * the exception at {@link java.util.logging.Level#FINE} without stack trace.
+     */
+    private static void recoverableException(final Exception warning) {
+        Logging.recoverableException(Logging.getLogger("org.apache.sis.storage"), IOUtilities.class,
"open", warning);
+    }
 }

Modified: sis/branches/JDK7/storage/sis-storage/src/main/java/org/apache/sis/storage/DataStore.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK7/storage/sis-storage/src/main/java/org/apache/sis/storage/DataStore.java?rev=1509666&r1=1509665&r2=1509666&view=diff
==============================================================================
--- sis/branches/JDK7/storage/sis-storage/src/main/java/org/apache/sis/storage/DataStore.java
[UTF-8] (original)
+++ sis/branches/JDK7/storage/sis-storage/src/main/java/org/apache/sis/storage/DataStore.java
[UTF-8] Fri Aug  2 12:43:52 2013
@@ -38,6 +38,8 @@ import org.apache.sis.util.logging.Warni
  * @since   0.3
  * @version 0.3
  * @module
+ *
+ * @see DataStores#open(Object)
  */
 public abstract class DataStore implements Localized, AutoCloseable {
     /**

Modified: sis/branches/JDK7/storage/sis-storage/src/main/java/org/apache/sis/storage/DataStoreProvider.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK7/storage/sis-storage/src/main/java/org/apache/sis/storage/DataStoreProvider.java?rev=1509666&r1=1509665&r2=1509666&view=diff
==============================================================================
--- sis/branches/JDK7/storage/sis-storage/src/main/java/org/apache/sis/storage/DataStoreProvider.java
[UTF-8] (original)
+++ sis/branches/JDK7/storage/sis-storage/src/main/java/org/apache/sis/storage/DataStoreProvider.java
[UTF-8] Fri Aug  2 12:43:52 2013
@@ -75,10 +75,10 @@ public abstract class DataStoreProvider 
      * only that there appears to be a reasonable chance of success based on a brief inspection
of the
      * {@linkplain StorageConnector#getStorage() storage object} or contents.
      *
-     * Implementors are responsible for restoring the input to its original stream position
on return of this method.
+     * <p>Implementors are responsible for restoring the input to its original stream
position on return of this method.
      * Implementors can use a mark/reset pair for this purpose. Marks are available as
      * {@link java.nio.ByteBuffer#mark()}, {@link java.io.InputStream#mark(int)} and
-     * {@link javax.imageio.stream.ImageInputStream#mark()}.
+     * {@link javax.imageio.stream.ImageInputStream#mark()}.</p>
      *
      * {@section Implementation example}
      * Implementations will typically check the first bytes of the stream for a "magic number"
associated
@@ -127,7 +127,7 @@ public abstract class DataStoreProvider 
      * @throws IllegalArgumentException If the set contains an invalid combination of options.
      * @throws DataStoreException If an error occurred while creating the data store instance.
      *
-     * @see DataStores#open(Object, Set)
+     * @see DataStores#open(Object)
      */
     public abstract DataStore open(StorageConnector storage) throws DataStoreException;
 }

Modified: sis/branches/JDK7/storage/sis-storage/src/main/java/org/apache/sis/storage/DataStores.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK7/storage/sis-storage/src/main/java/org/apache/sis/storage/DataStores.java?rev=1509666&r1=1509665&r2=1509666&view=diff
==============================================================================
--- sis/branches/JDK7/storage/sis-storage/src/main/java/org/apache/sis/storage/DataStores.java
[UTF-8] (original)
+++ sis/branches/JDK7/storage/sis-storage/src/main/java/org/apache/sis/storage/DataStores.java
[UTF-8] Fri Aug  2 12:43:52 2013
@@ -22,7 +22,7 @@ import org.apache.sis.internal.system.Sy
 
 
 /**
- * Creates {@link DataStore} instances from a given storage object.
+ * Static convenience methods creating {@link DataStore} instances from a given storage object.
  * Storage objects are typically {@link java.io.File} or {@link javax.sql.DataSource} instances,
  * but can also be any other objects documented in the {@link StorageConnector} class.
  *

Modified: sis/branches/JDK7/storage/sis-storage/src/main/java/org/apache/sis/storage/ProbeResult.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK7/storage/sis-storage/src/main/java/org/apache/sis/storage/ProbeResult.java?rev=1509666&r1=1509665&r2=1509666&view=diff
==============================================================================
--- sis/branches/JDK7/storage/sis-storage/src/main/java/org/apache/sis/storage/ProbeResult.java
[UTF-8] (original)
+++ sis/branches/JDK7/storage/sis-storage/src/main/java/org/apache/sis/storage/ProbeResult.java
[UTF-8] Fri Aug  2 12:43:52 2013
@@ -18,7 +18,7 @@ package org.apache.sis.storage;
 
 
 /**
- * Tells whether a storage (file, database, <i>etc.</i>) appears to be supported
by a {@code DataStore}.
+ * Tells whether a storage (file, database) appears to be supported by a {@code DataStore}.
  * There is three categories of values in this enumeration:
  *
  * <ul>
@@ -27,7 +27,8 @@ package org.apache.sis.storage;
  *       whether the storage can be opened. SIS will try to use such provider last, if no
better suited
  *       provider is found.</li>
  *   <li>All other values indicate that the storage can not be opened. The actual enumeration
value gives
- *       the reason (e.g. unknown format, or unsupported version).</li>
+ *       the reason (e.g. {@linkplain #UNKNOWN_FORMAT unknown format}, or
+ *       {@linkplain #UNSUPPORTED_VERSION unsupported version}).</li>
  * </ul>
  *
  * When a {@link DataStores#open DataStores.open(…)} method is invoked, SIS will iterate
over the list of known



Mime
View raw message