sis-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From desruisse...@apache.org
Subject svn commit: r1805991 - in /sis/branches/JDK8/storage/sis-storage/src/main/java/org/apache/sis: internal/storage/io/ storage/
Date Thu, 24 Aug 2017 02:51:13 GMT
Author: desruisseaux
Date: Thu Aug 24 02:51:12 2017
New Revision: 1805991

URL: http://svn.apache.org/viewvc?rev=1805991&view=rev
Log:
Make InputStreamAdapter more compliant to InputStream contract.

Modified:
    sis/branches/JDK8/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/io/ChannelData.java
    sis/branches/JDK8/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/io/ChannelFactory.java
    sis/branches/JDK8/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/io/InputStreamAdapter.java
    sis/branches/JDK8/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/io/Markable.java
    sis/branches/JDK8/storage/sis-storage/src/main/java/org/apache/sis/storage/StorageConnector.java

Modified: sis/branches/JDK8/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/io/ChannelData.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK8/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/io/ChannelData.java?rev=1805991&r1=1805990&r2=1805991&view=diff
==============================================================================
--- sis/branches/JDK8/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/io/ChannelData.java
[UTF-8] (original)
+++ sis/branches/JDK8/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/io/ChannelData.java
[UTF-8] Thu Aug 24 02:51:12 2017
@@ -272,13 +272,11 @@ public abstract class ChannelData implem
      * An {@code IOException} may be be thrown if the previous marked position lies in the
      * discarded portion of the stream.
      *
-     * <p>This method differs from the {@link javax.imageio.stream.ImageInputStream}
contract in two aspects:</p>
-     * <ul>
-     *   <li>This method may, under some conditions, be able to perform its work even
if the marked
-     *       position is before the flushed position.</li>
-     *   <li>If there is no mark, this method throws an {@link InvalidMarkException}
rather than
-     *       doing nothing. Doing nothing is considered a too high risk of error.</li>
-     * </ul>
+     * <div class="section">Departure from Image I/O specification</div>
+     * The {@link javax.imageio.stream.ImageInputStream#reset()} contract specifies that
if there is no matching mark,
+     * then this method shall do nothing. This method throws {@link InvalidMarkException}
instead; silently ignoring
+     * the mismatch is considered too dangerous. However we may revisit this policy in the
future if it appears to be
+     * a compatibility problem. Consequently, no code shall rely on {@code InvalidMarkException}
to be thrown.
      *
      * @throws IOException if an I/O error occurs.
      */

Modified: sis/branches/JDK8/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/io/ChannelFactory.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK8/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/io/ChannelFactory.java?rev=1805991&r1=1805990&r2=1805991&view=diff
==============================================================================
--- sis/branches/JDK8/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/io/ChannelFactory.java
[UTF-8] (original)
+++ sis/branches/JDK8/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/io/ChannelFactory.java
[UTF-8] Thu Aug 24 02:51:12 2017
@@ -357,7 +357,7 @@ public abstract class ChannelFactory {
      * A factory used as a fallback when we failed to convert a {@link File} to a {@link
Path}.
      * This is used only if the conversion attempt threw an {@link InvalidPathException}.
Such
      * failure is unlikely to happen, but if it happens anyway we try to open the channel
in a
-     * way less surprising for the user (closer to the object he has specified).
+     * less surprising way for the user (i.e. closer to the object (s)he has specified).
      */
     private static final class Fallback extends ChannelFactory {
         /**

Modified: sis/branches/JDK8/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/io/InputStreamAdapter.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK8/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/io/InputStreamAdapter.java?rev=1805991&r1=1805990&r2=1805991&view=diff
==============================================================================
--- sis/branches/JDK8/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/io/InputStreamAdapter.java
[UTF-8] (original)
+++ sis/branches/JDK8/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/io/InputStreamAdapter.java
[UTF-8] Thu Aug 24 02:51:12 2017
@@ -20,11 +20,14 @@ import java.io.InputStream;
 import java.io.IOException;
 import javax.imageio.stream.ImageInputStream;
 
+// Branch-dependent imports
+import java.io.UncheckedIOException;
+
 
 /**
  * Wraps an {@link ImageInputStream} as a standard {@link InputStream}.
  *
- * @author  Martin Desruisseaux (IRD)
+ * @author  Martin Desruisseaux (IRD, Geomatys)
  * @version 0.8
  *
  * @see OutputStreamAdapter
@@ -41,13 +44,27 @@ public final class InputStreamAdapter ex
     public final ImageInputStream input;
 
     /**
+     * Position of the last mark created by {@link #mark(int)}, or the file beginning if
there is no mark.
+     */
+    private long markPosition;
+
+    /**
+     * Count of marks created by {@link #mark()}, not counting the mark created by {@link
#mark(int)}.
+     * We have to keep this count ourselves because {@link ImageInputStream#reset()} does
nothing if
+     * there is no mark, and provides no API for letting us know if {@code reset()} worked.
+     */
+    private int nestedMarks;
+
+    /**
      * Constructs a new input stream.
      *
-     * @param input  the stream to wrap.
+     * @param  input  the stream to wrap.
+     * @throws IOException  if an error occurred while creating the adapter.
      */
-    public InputStreamAdapter(final ImageInputStream input) {
+    public InputStreamAdapter(final ImageInputStream input) throws IOException {
         assert !(input instanceof InputStream);
         this.input = input;
+        markPosition = input.getStreamPosition();
     }
 
     /**
@@ -105,31 +122,60 @@ public final class InputStreamAdapter ex
     }
 
     /**
-     * Marks the current position in this input stream.
+     * Discards all previous marks and marks the current position in this input stream.
+     * This method is part of {@link InputStream} API, where only one mark can be set and
multiple
+     * calls to {@code reset()} move to the same position until {@code mark(int)} is invoked
again.
      *
      * @param  readlimit  ignored.
+     * @throws UncheckedIOException if the mark can not be set.
      */
     @Override
     public void mark(final int readlimit) {
-        input.mark();
+        try {
+            markPosition = input.getStreamPosition();
+            input.flushBefore(markPosition);
+            nestedMarks = 0;
+        } catch (IOException e) {
+            throw new UncheckedIOException(e);      // InputStream.mark() does not allow
us to throw IOException.
+        }
     }
 
     /**
      * Marks the current position in this input stream.
+     * This method is part of {@link Markable} API, where marks can be nested.
+     * It is okay to invoke this method after {@link #mark(int)} (but not before).
      */
     @Override
     public void mark() {
         input.mark();
+        nestedMarks++;
     }
 
     /**
      * Repositions this stream to the position at the time the {@code mark} method was last
called.
+     * This method has to comply with both {@link InputStream#reset()} and {@link Markable#reset()}
+     * contracts. It does that by choosing the first option in following list:
+     *
+     * <ul>
+     *   <li>If there is nested {@link #mark()} calls, then this {@code reset()} method
sets the stream
+     *       position to the most recent unmatched call to {@code mark()}.</li>
+     *   <li>Otherwise if the {@link #mark(int)} method has been invoked, then this
method sets the stream
+     *       position to the mark created by the most recent call to {@code mark(int)}. The
{@code reset()}
+     *       method can be invoked many time; it will always set the position to the same
mark
+     *       (this behavior is required by {@link InputStream} contract).</li>
+     *   <li>Otherwise this method sets the stream position to the position it had
when this
+     *       {@code InputStreamAdapter} has been created.</li>
+     * </ul>
      *
      * @throws IOException if an I/O error occurs.
      */
     @Override
     public void reset() throws IOException {
-        input.reset();
+        if (--nestedMarks >= 0) {
+            input.reset();
+        } else {
+            input.seek(markPosition);
+        }
     }
 
     /**

Modified: sis/branches/JDK8/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/io/Markable.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK8/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/io/Markable.java?rev=1805991&r1=1805990&r2=1805991&view=diff
==============================================================================
--- sis/branches/JDK8/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/io/Markable.java
[UTF-8] (original)
+++ sis/branches/JDK8/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/io/Markable.java
[UTF-8] Thu Aug 24 02:51:12 2017
@@ -17,7 +17,6 @@
 package org.apache.sis.internal.storage.io;
 
 import java.io.IOException;
-import java.nio.InvalidMarkException;
 
 
 /**
@@ -56,6 +55,8 @@ public interface Markable {
      * Calls to {@code mark()} and {@code reset()} can be nested arbitrarily.
      *
      * @throws IOException if this stream can not mark the current position.
+     *
+     * @see javax.imageio.stream.ImageInputStream#mark()
      */
     void mark() throws IOException;
 
@@ -64,8 +65,14 @@ public interface Markable {
      * An {@code IOException} may be be thrown if the previous marked position lies in the
      * discarded portion of the stream.
      *
-     * @throws InvalidMarkException if there is no mark.
+     * <p>If there is no mark, then the behavior is undefined.
+     * {@link java.io.InputStream#reset()} specifies that we shall move to the file beginning.
+     * {@link javax.imageio.stream.ImageInputStream#reset()} specifies that we shall do nothing.
+     * {@link ChannelDataInput#reset()} throws {@link java.nio.InvalidMarkException}.</p>
+     *
      * @throws IOException if a mark was defined but this stream can not move to that position.
+     *
+     * @see javax.imageio.stream.ImageInputStream#reset()
      */
     void reset() throws IOException;
 }

Modified: sis/branches/JDK8/storage/sis-storage/src/main/java/org/apache/sis/storage/StorageConnector.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK8/storage/sis-storage/src/main/java/org/apache/sis/storage/StorageConnector.java?rev=1805991&r1=1805990&r2=1805991&view=diff
==============================================================================
--- sis/branches/JDK8/storage/sis-storage/src/main/java/org/apache/sis/storage/StorageConnector.java
[UTF-8] (original)
+++ sis/branches/JDK8/storage/sis-storage/src/main/java/org/apache/sis/storage/StorageConnector.java
[UTF-8] Thu Aug 24 02:51:12 2017
@@ -166,7 +166,7 @@ public class StorageConnector implements
      * @see #getOption(OptionKey)
      * @see #setOption(OptionKey, Object)
      */
-    private transient Map<OptionKey<?>, Object> options;
+    private Map<OptionKey<?>, Object> options;
 
     /**
      * Creates a new data store connection wrapping the given input/output object.
@@ -432,14 +432,29 @@ public class StorageConnector implements
     /**
      * Assuming that {@link #storage} is an instance of {@link InputStream}, resets its position.
This method
      * is the converse of the marks performed at the beginning of {@link #createChannelDataInput(boolean)}.
+     *
+     * <div class="note"><b>Rational:</b>
+     * {@link DataStoreProvider#probeContent(StorageConnector)} contract requires that implementors
reset the
+     * input stream themselves. However if the last {@code DataStoreProvider} instance that
we tried worked on
+     * {@code ChannelDataInput}, then the provider performed a call to {@link ChannelDataInput#reset()},
which
+     * did not reseted the underlying input stream. So we need to perform the missing {@link
InputStream#reset()}
+     * here, then synchronize the {@code ChannelDataInput} position accordingly.</div>
      */
     private void resetInputStream() throws IOException {
         final ChannelDataInput channel = getView(ChannelDataInput.class);
         if (channel != null) {
+            /*
+             * Note on InputStream.reset() behavior documented in java.io:
+             *
+             *  - It does not discard the mark, so it is okay if reset() is invoked twice.
+             *  - If mark is unsupported, may either throw IOException or reset the stream
+             *    to an implementation-dependent fixed state.
+             */
             ((InputStream) storage).reset();        // May throw an exception if mark is
unsupported.
             channel.buffer.limit(0);                // Must be after storage.reset().
             channel.setStreamPosition(0);           // Must be after buffer.limit(0).
         }
+        removeView(Reader.class);                   // Reader will need to be recreated from
scratch.
     }
 
     /**
@@ -451,7 +466,7 @@ public class StorageConnector implements
      */
     private void createChannelDataInput(final boolean asImageInputStream) throws IOException
{
         /*
-         * Before to try to open an InputStream, mark its position so we can rewind if the
user asks for
+         * Before to try to wrap an InputStream, mark its position so we can rewind if the
user asks for
          * the InputStream directly. We need to reset because ChannelDataInput may have read
some bytes.
          * Note that if mark is unsupported, the default InputStream.mark() implementation
does nothing.
          * See above 'resetInputStream()' method.
@@ -461,7 +476,7 @@ public class StorageConnector implements
         }
         /*
          * Following method call recognizes ReadableByteChannel, InputStream (with special
case for FileInputStream),
-         * URL, URI, File, Path or other types that may be added in future SIS versions.
+         * URL, URI, File, Path or other types that may be added in future Apache SIS versions.
          */
         final ChannelFactory factory = ChannelFactory.prepare(storage,
                 getOption(OptionKey.URL_ENCODING), false, getOption(OptionKey.OPEN_OPTIONS));
@@ -534,7 +549,7 @@ public class StorageConnector implements
     /**
      * Creates a {@link ByteBuffer} from the {@link ChannelDataInput} if possible, or from
the
      * {@link ImageInputStream} otherwise. The buffer will be initialized with an arbitrary
amount
-     * of bytes read from the input. This amount is not sufficient, it can be increased by
a call
+     * of bytes read from the input. If this amount is not sufficient, it can be increased
by a call
      * to {@link #prefetch()}.
      *
      * @throws IOException if an error occurred while opening a stream for the input.
@@ -624,7 +639,7 @@ public class StorageConnector implements
      * @throws IllegalArgumentException if the given {@code type} argument is not a supported
type.
      * @throws Exception if an error occurred while opening a stream or database connection.
      */
-    private Object createView(final Class<?> type) throws IllegalArgumentException,
Exception {
+    private Object createView(final Class<?> type) throws Exception {
         if (type == String.class) {
             return IOUtilities.toString(storage);
         }
@@ -680,7 +695,7 @@ public class StorageConnector implements
                  * 2) InputStreamReader does not support mark/reset, which is a desired limitation
for now.
                  *    This is because reseting the Reader would not reset the underlying
InputStream, which
                  *    would cause other DataStoreProvider.probeContent(…) methods to fail
if they try to use
-                 *    the InputStream. For now we let the InputStreamReader.mark() to throws
an IOException,
+                 *    the InputStream. For now we let the InputStreamReader.mark() to throw
an IOException,
                  *    but we may need to provide our own subclass of BufferedReader in a
future SIS version
                  *    if mark/reset support is needed here.
                  */
@@ -718,6 +733,18 @@ public class StorageConnector implements
     }
 
     /**
+     * Removes the given view from the cache.
+     * This method is invoked for forcing the view to be recreated if requested again.
+     *
+     * @param type  the view type to remove.
+     */
+    private void removeView(final Class<?> type) {
+        if (views.remove(type) != null) {
+            viewsToClose.remove(type);
+        }
+    }
+
+    /**
      * Declares that the given {@code input} will need to be closed by the {@link #closeAllExcept(Object)}
method.
      * The {@code input} argument is always a new instance wrapping, directly or indirectly,
the {@link #storage}.
      * Callers must specify the wrapped object in the {@code delegate} argument.
@@ -730,7 +757,8 @@ public class StorageConnector implements
             viewsToClose = new IdentityHashMap<>(4);
         }
         if (viewsToClose.put(input, delegate) != null) {
-            throw new AssertionError(input);
+            // Should never happen, unless someone used this StorageConnector in another
thread.
+            throw new ConcurrentModificationException();
         }
     }
 



Mime
View raw message