sis-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ama...@apache.org
Subject [sis] branch refactor/strict_storage_connector updated: fix(Storage): Fix closing behovior and add unit tests on strict version of storage connector.
Date Fri, 01 May 2020 18:02:47 GMT
This is an automated email from the ASF dual-hosted git repository.

amanin pushed a commit to branch refactor/strict_storage_connector
in repository https://gitbox.apache.org/repos/asf/sis.git


The following commit(s) were added to refs/heads/refactor/strict_storage_connector by this
push:
     new 7cdffc0  fix(Storage): Fix closing behovior and add unit tests on strict version
of storage connector.
7cdffc0 is described below

commit 7cdffc0181f875c839d911d1d164a831b1a129ce
Author: Alexis Manin <amanin@apache.org>
AuthorDate: Fri May 1 20:02:28 2020 +0200

    fix(Storage): Fix closing behovior and add unit tests on strict version of storage connector.
---
 .../apache/sis/storage/StrictStorageConnector.java |  27 +++--
 .../sis/storage/StrictStorageConnectorTest.java    | 112 +++++++++++++++++----
 2 files changed, 113 insertions(+), 26 deletions(-)

diff --git a/storage/sis-storage/src/main/java/org/apache/sis/storage/StrictStorageConnector.java
b/storage/sis-storage/src/main/java/org/apache/sis/storage/StrictStorageConnector.java
index 0ba6ed5..1e2010f 100644
--- a/storage/sis-storage/src/main/java/org/apache/sis/storage/StrictStorageConnector.java
+++ b/storage/sis-storage/src/main/java/org/apache/sis/storage/StrictStorageConnector.java
@@ -48,20 +48,24 @@ import org.apache.sis.util.collection.BackingStoreException;
  *
  * Example:
  * <pre>
- * final StrictStorageConnector c = new StrictStorageConnector(Paths.get("path/to/file");
- * // Use connector automatically reset buffering to check support
- * Boolean isSupported = c.useAsBuffer((buffer) -%gt; buffer.get() == SEARCHED_KEY);
- * // Once support is validated, acquire real storage connection. At this point,
- * // storage life cycle becomes the responsability of the caller
- * if (supported) {
+ * try ( StrictStorageConnector c = new StrictStorageConnector(Paths.get("path/to/file"))
{
+ *
+ *   // Use connector automatically reset buffering to check support
+ *   Boolean isSupported = c.useAsBuffer((buffer) -%gt; buffer.get() == SEARCHED_KEY);
+ *
+ *   // Once support is validated, acquire real storage connection. At this point,
+ *   // storage life cycle becomes the responsability of the caller
+ *   if (supported) {
  *     try ( InputStream stream = c.commit( InputStream.class ) ) {
  *         // read all needed data from aquired stream
  *     }
- * } else c.closeAllExcept(null); // not acceptable input, completely close component.
+ *   }
+ * }
  * </pre>
  */
-public class StrictStorageConnector extends StorageConnector {
+public class StrictStorageConnector extends StorageConnector implements AutoCloseable {
 
+    private Object committedStorage;
     private volatile int concurrentFlag;
 
     public StrictStorageConnector(Object storage) {
@@ -70,6 +74,8 @@ public class StrictStorageConnector extends StorageConnector {
 
     @Override
     public void closeAllExcept(Object view) throws DataStoreException {
+        // Closing multiple times is OK. However, if the view is not null, we will let control
raise an error.
+        if (concurrentFlag < 0 && view == null) return;
         try {
             doUnderControl(() -> {
                 concurrentFlag = -1;
@@ -190,6 +196,11 @@ public class StrictStorageConnector extends StorageConnector {
         }
     }
 
+    @Override
+    public void close() throws IOException, DataStoreException {
+        super.closeAllExcept(committedStorage);
+    }
+
     private interface StorageCallable<V> extends Callable<V> {
         @Override
         V call() throws IOException, DataStoreException;
diff --git a/storage/sis-storage/src/test/java/org/apache/sis/storage/StrictStorageConnectorTest.java
b/storage/sis-storage/src/test/java/org/apache/sis/storage/StrictStorageConnectorTest.java
index 02cdb7e..2cc74c2 100644
--- a/storage/sis-storage/src/test/java/org/apache/sis/storage/StrictStorageConnectorTest.java
+++ b/storage/sis-storage/src/test/java/org/apache/sis/storage/StrictStorageConnectorTest.java
@@ -1,14 +1,15 @@
 package org.apache.sis.storage;
 
 import java.io.IOException;
+import java.io.InputStream;
 import java.net.URISyntaxException;
+import java.nio.ByteBuffer;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import org.apache.sis.setup.OptionKey;
 import org.apache.sis.test.DependsOn;
-import org.junit.Ignore;
 import org.junit.Test;
 
 import static org.junit.Assert.assertArrayEquals;
@@ -16,6 +17,7 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 @DependsOn(org.apache.sis.storage.StorageConnectorTest.class)
 public class StrictStorageConnectorTest {
@@ -64,37 +66,111 @@ public class StrictStorageConnectorTest {
     @Test
     public void byte_buffer_is_rewind_after_use() throws Exception {
         final byte[] ctrl = getFileBytes();
-        final StrictStorageConnector connector = create(false);
-        // Mess with internal buffer
-        connector.useAsBuffer(buffer -> {
-            // mess with it
-            return buffer.get(new byte[10]);
-        });
-        // ensure it has been properly rewind
-        connector.useAsBuffer(buffer -> {
-            assertEquals(0, buffer.position());
-            byte[] readValue = new byte[buffer.remaining()];
-            buffer.get(readValue);
-            assertArrayEquals(ctrl, readValue);
-            return null;
-        });
+        try (final StrictStorageConnector connector = create(false)) {
+            // Mess with internal buffer
+            connector.useAsBuffer(buffer -> {
+                // mess with it
+                return buffer.get(new byte[10]);
+            });
+            // ensure it has been properly rewind
+            connector.useAsBuffer(buffer -> {
+                assertEquals(0, buffer.position());
+                byte[] readValue = new byte[buffer.remaining()];
+                buffer.get(readValue);
+                assertArrayEquals(ctrl, readValue);
+                return null;
+            });
+        }
+    }
+
+    @Test
+    public void fail_fast_when_user_corrupts_stream_mark() throws IOException, DataStoreException
{
+        try (final StrictStorageConnector c = create(false)) {
+            try {
+                c.useAsImageInputStream(stream -> {
+                    stream.skipBytes(1);
+                    stream.mark();
+                    return 0;
+                });
+                fail("We should have detected something has gone wrong");
+            } catch (DataStoreException e) {
+                // Expected behavior: connector has detected that rewind did not work properly.
+            }
+        }
     }
 
     @Test
-    @Ignore("To implement")
     public void no_concurrency_allowed() throws Exception {
+        try (final StrictStorageConnector c = create(false)) {
+            synchronized (c) {
+            new Thread(() -> {
+                try {
+                    c.useAsBuffer(buffer -> {
+                        synchronized (c) {
+                            try {
+                                c.notifyAll();
+                                c.wait(1000);
+                            } catch (InterruptedException e) {
+                                // Do not matter here.
+                            }
+                        }
+                        return null;
+                    });
 
+                } catch (IOException | DataStoreException e) {
+                    // Do not matter here.
+                }
+            }).start();
+
+            // Ensure above operation is
+                c.wait(100);
+            }
+
+            try {
+                c.useAsBuffer(buffer -> null);
+                fail("Concurrency error should have been raised.");
+            } catch (ConcurrentReadException e) {
+                // Expected behavior: fail-fast to prevent concurrency.
+            }
+            synchronized (c) {
+                c.notifyAll();
+            }
+        }
     }
 
     @Test
-    @Ignore("To implement")
     public void commit_close_all_resources_but_chosen() throws Exception {
+        final InputStream is;
+        try (final StrictStorageConnector c = create(false)) {
+
+            is = c.commit(InputStream.class);
+
+            try {
+                c.getStorageAs(ByteBuffer.class);
+                fail("connector should be closed");
+            } catch (IllegalStateException e) {
+                // Expected behavior
+                try {
+                    is.read();
+                } catch (IOException bis) {
+                    fail("We queried for the input stream to stay open.");
+                }
+            }
+        }
 
+        try ( final InputStream close = is ) {
+            is.read();
+        } catch (IOException e) {
+            fail("Committed storage view should still be opened.");
+        }
     }
 
     @Test
-    @Ignore("To implement")
     public void closing_multiple_times_causes_no_error() throws Exception {
+        try ( StrictStorageConnector c = create(true) ) {
 
+            c.commit(InputStream.class);
+            c.closeAllExcept(null);
+        }
     }
 }


Mime
View raw message