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 <[email protected]>
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);
+ }
}
}
