You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sis.apache.org by de...@apache.org on 2022/01/06 01:20:19 UTC
[sis] 06/07: Use the new safer `StorageConnector.commit(…)` and `DataStoreProvider.probeContent(…)` where they can easily be used.
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 c64bef93a7cc3ea62faf2d2e3dd7096271da9d0a
Author: Martin Desruisseaux <ma...@geomatys.com>
AuthorDate: Thu Jan 6 00:38:09 2022 +0100
Use the new safer `StorageConnector.commit(…)` and `DataStoreProvider.probeContent(…)` where they can easily be used.
---
.../apache/sis/storage/geotiff/GeoTiffStore.java | 8 +-----
.../sis/storage/geotiff/GeoTiffStoreProvider.java | 22 ++++++---------
.../sis/storage/netcdf/NetcdfStoreProvider.java | 10 ++-----
.../sis/internal/storage/folder/StoreProvider.java | 1 +
.../org/apache/sis/internal/storage/wkt/Store.java | 8 +-----
.../sis/internal/storage/xml/AbstractProvider.java | 24 ++++++++--------
.../org/apache/sis/storage/DataStoreProvider.java | 11 ++++----
.../apache/sis/storage/DataStoreProviderTest.java | 33 ++++++++++++++++++++++
8 files changed, 63 insertions(+), 54 deletions(-)
diff --git a/storage/sis-geotiff/src/main/java/org/apache/sis/storage/geotiff/GeoTiffStore.java b/storage/sis-geotiff/src/main/java/org/apache/sis/storage/geotiff/GeoTiffStore.java
index ce301a1..f0fdbc9 100644
--- a/storage/sis-geotiff/src/main/java/org/apache/sis/storage/geotiff/GeoTiffStore.java
+++ b/storage/sis-geotiff/src/main/java/org/apache/sis/storage/geotiff/GeoTiffStore.java
@@ -38,7 +38,6 @@ import org.apache.sis.storage.DataStore;
import org.apache.sis.storage.DataStoreProvider;
import org.apache.sis.storage.StorageConnector;
import org.apache.sis.storage.DataStoreException;
-import org.apache.sis.storage.UnsupportedStorageException;
import org.apache.sis.storage.DataStoreClosedException;
import org.apache.sis.storage.IllegalNameException;
import org.apache.sis.storage.event.StoreEvent;
@@ -201,14 +200,9 @@ public class GeoTiffStore extends DataStore implements Aggregate {
final Charset encoding = connector.getOption(OptionKey.ENCODING);
this.encoding = (encoding != null) ? encoding : StandardCharsets.US_ASCII;
- final ChannelDataInput input = connector.getStorageAs(ChannelDataInput.class);
- if (input == null) {
- throw new UnsupportedStorageException(super.getLocale(), Constants.GEOTIFF,
- connector.getStorage(), connector.getOption(OptionKey.OPEN_OPTIONS));
- }
location = connector.getStorageAs(URI.class);
path = connector.getStorageAs(Path.class);
- connector.closeAllExcept(input);
+ final ChannelDataInput input = connector.commit(ChannelDataInput.class, Constants.GEOTIFF);
try {
reader = new Reader(this, input);
} catch (IOException e) {
diff --git a/storage/sis-geotiff/src/main/java/org/apache/sis/storage/geotiff/GeoTiffStoreProvider.java b/storage/sis-geotiff/src/main/java/org/apache/sis/storage/geotiff/GeoTiffStoreProvider.java
index 4d1881b..a150790 100644
--- a/storage/sis-geotiff/src/main/java/org/apache/sis/storage/geotiff/GeoTiffStoreProvider.java
+++ b/storage/sis-geotiff/src/main/java/org/apache/sis/storage/geotiff/GeoTiffStoreProvider.java
@@ -105,29 +105,23 @@ public class GeoTiffStoreProvider extends DataStoreProvider {
* @throws DataStoreException if an I/O error occurred.
*/
@Override
+ @SuppressWarnings("fallthrough")
public ProbeResult probeContent(StorageConnector connector) throws DataStoreException {
- final ByteBuffer buffer = connector.getStorageAs(ByteBuffer.class);
- if (buffer != null) {
+ return probeContent(connector, ByteBuffer.class, (buffer) -> {
if (buffer.remaining() < 2 * Short.BYTES) {
return ProbeResult.INSUFFICIENT_BYTES;
}
- final int p = buffer.position();
- final short order = buffer.getShort(p);
- final boolean isBigEndian = (order == GeoTIFF.BIG_ENDIAN);
- if (isBigEndian || order == GeoTIFF.LITTLE_ENDIAN) {
- final ByteOrder old = buffer.order();
- try {
- buffer.order(isBigEndian ? ByteOrder.BIG_ENDIAN : ByteOrder.LITTLE_ENDIAN);
- switch (buffer.getShort(p + Short.BYTES)) {
+ switch (buffer.getShort()) {
+ case GeoTIFF.LITTLE_ENDIAN: buffer.order(ByteOrder.LITTLE_ENDIAN); // Fall through
+ case GeoTIFF.BIG_ENDIAN: { // Default buffer order is big endian.
+ switch (buffer.getShort()) {
case GeoTIFF.CLASSIC:
case GeoTIFF.BIG_TIFF: return new ProbeResult(true, MIME_TYPE, VERSION);
}
- } finally {
- buffer.order(old);
}
}
- }
- return ProbeResult.UNSUPPORTED_STORAGE;
+ return ProbeResult.UNSUPPORTED_STORAGE;
+ });
}
/**
diff --git a/storage/sis-netcdf/src/main/java/org/apache/sis/storage/netcdf/NetcdfStoreProvider.java b/storage/sis-netcdf/src/main/java/org/apache/sis/storage/netcdf/NetcdfStoreProvider.java
index 1058f22..7dd23ac 100644
--- a/storage/sis-netcdf/src/main/java/org/apache/sis/storage/netcdf/NetcdfStoreProvider.java
+++ b/storage/sis-netcdf/src/main/java/org/apache/sis/storage/netcdf/NetcdfStoreProvider.java
@@ -190,17 +190,13 @@ public class NetcdfStoreProvider extends DataStoreProvider {
int version = 0;
boolean hasVersion = false;
boolean isSupported = false;
- final ByteBuffer buffer = connector.getStorageAs(ByteBuffer.class);
+ ByteBuffer buffer = connector.getStorageAs(ByteBuffer.class);
if (buffer != null) {
- /*
- * This block is written as if `ByteBuffer` was an immutable object.
- * In particular we use the "absolute position" variants of `get(…)`.
- * Consequently we do not need to mark and reset buffer position.
- */
if (buffer.remaining() < Integer.BYTES) {
return ProbeResult.INSUFFICIENT_BYTES;
}
- final int header = buffer.getInt(buffer.position());
+ buffer = buffer.duplicate(); // Get a buffer with ByteOrder.BIG_ENDIAN.
+ final int header = buffer.getInt();
if ((header & 0xFFFFFF00) == ChannelDecoder.MAGIC_NUMBER) {
hasVersion = true;
version = header & 0xFF;
diff --git a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/folder/StoreProvider.java b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/folder/StoreProvider.java
index 9561f58..2f81fac 100644
--- a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/folder/StoreProvider.java
+++ b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/folder/StoreProvider.java
@@ -258,6 +258,7 @@ public final class StoreProvider extends DataStoreProvider {
throw new DataStoreException(Resources.format(errorKey,
(path != null) ? path : connector.getStorageName(), isDirectory), e);
}
+ connector.closeAllExcept(path);
return store;
}
diff --git a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/wkt/Store.java b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/wkt/Store.java
index b85681b..9fd77e3 100644
--- a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/wkt/Store.java
+++ b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/wkt/Store.java
@@ -29,7 +29,6 @@ import org.apache.sis.internal.storage.Resources;
import org.apache.sis.storage.StorageConnector;
import org.apache.sis.storage.DataStoreException;
import org.apache.sis.storage.DataStoreContentException;
-import org.apache.sis.storage.UnsupportedStorageException;
import org.apache.sis.internal.storage.MetadataBuilder;
import org.apache.sis.internal.storage.URIDataStore;
import org.apache.sis.referencing.IdentifiedObjects;
@@ -85,12 +84,7 @@ final class Store extends URIDataStore {
public Store(final StoreProvider provider, final StorageConnector connector) throws DataStoreException {
super(provider, connector);
objects = new ArrayList<>();
- source = connector.getStorageAs(Reader.class);
- connector.closeAllExcept(source);
- if (source == null) {
- throw new UnsupportedStorageException(super.getLocale(), StoreProvider.NAME,
- connector.getStorage(), connector.getOption(OptionKey.OPEN_OPTIONS));
- }
+ source = connector.commit(Reader.class, StoreProvider.NAME);
library = connector.getOption(OptionKey.GEOMETRY_LIBRARY);
}
diff --git a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/xml/AbstractProvider.java b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/xml/AbstractProvider.java
index c0d19b4..2908d59 100644
--- a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/xml/AbstractProvider.java
+++ b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/xml/AbstractProvider.java
@@ -23,7 +23,6 @@ import java.io.IOException;
import java.nio.ByteBuffer;
import org.apache.sis.storage.DataStore;
import org.apache.sis.storage.DataStoreException;
-import org.apache.sis.storage.CanNotProbeException;
import org.apache.sis.storage.StorageConnector;
import org.apache.sis.storage.ProbeResult;
import org.apache.sis.internal.storage.io.IOUtilities;
@@ -114,17 +113,23 @@ public abstract class AbstractProvider extends DocumentedStoreProvider {
*/
final ByteBuffer buffer = connector.getStorageAs(ByteBuffer.class);
if (buffer != null) {
+ /*
+ * We do not use the safer `probeContent(…)` method because we do not have a mechanism
+ * for telling if `UNSUPPORTED_STORAGE` was determined by this block or if we got that
+ * result because the buffer was null.
+ */
if (buffer.remaining() < HEADER.length) {
return ProbeResult.INSUFFICIENT_BYTES;
}
// Quick check for "<?xml " header.
+ final int p = buffer.position();
for (int i=0; i<HEADER.length; i++) {
- if (buffer.get(i) != HEADER[i]) {
+ if (buffer.get(p + i) != HEADER[i]) { // TODO: use ByteBuffer.mismatch(…) with JDK11.
return ProbeResult.UNSUPPORTED_STORAGE;
}
}
// Now check for a more accurate MIME type.
- buffer.position(HEADER.length);
+ buffer.position(p + HEADER.length);
final ProbeResult result = new MimeTypeDetector(mimeForNameSpaces, mimeForRootElements) {
@Override int read() {
if (buffer.hasRemaining()) {
@@ -134,20 +139,17 @@ public abstract class AbstractProvider extends DocumentedStoreProvider {
return -1;
}
}.probeContent();
- buffer.position(0);
+ buffer.position(p);
return result;
}
/*
* We should enter in this block only if the user gave us explicitly a Reader.
* A common case is a StringReader wrapping a String object.
*/
- final Reader reader = connector.getStorageAs(Reader.class);
- if (reader != null) try {
+ return probeContent(connector, Reader.class, (reader) -> {
// Quick check for "<?xml " header.
- reader.mark(HEADER.length + READ_AHEAD_LIMIT);
for (int i=0; i<HEADER.length; i++) {
if (reader.read() != HEADER[i]) {
- reader.reset();
return ProbeResult.UNSUPPORTED_STORAGE;
}
}
@@ -158,11 +160,7 @@ public abstract class AbstractProvider extends DocumentedStoreProvider {
return (--remaining >= 0) ? IOUtilities.readCodePoint(reader) : -1;
}
}.probeContent();
- reader.reset();
return result;
- } catch (IOException e) {
- throw new CanNotProbeException(this, connector, e);
- }
- return ProbeResult.UNSUPPORTED_STORAGE;
+ });
}
}
diff --git a/storage/sis-storage/src/main/java/org/apache/sis/storage/DataStoreProvider.java b/storage/sis-storage/src/main/java/org/apache/sis/storage/DataStoreProvider.java
index 235ca30..02bdeb3 100644
--- a/storage/sis-storage/src/main/java/org/apache/sis/storage/DataStoreProvider.java
+++ b/storage/sis-storage/src/main/java/org/apache/sis/storage/DataStoreProvider.java
@@ -274,7 +274,7 @@ public abstract class DataStoreProvider {
* Current implementation accepts the following types (this list may be expanded in future versions):
*
* <blockquote>
- * {@link java.nio.ByteBuffer},
+ * {@link java.nio.ByteBuffer} (default byte order fixed to {@link java.nio.ByteOrder#BIG_ENDIAN BIG_ENDIAN}),
* {@link java.io.InputStream},
* {@link java.io.DataInput},
* {@link javax.imageio.stream.ImageInputStream} and
@@ -363,10 +363,9 @@ public abstract class DataStoreProvider {
try {
if (input instanceof ByteBuffer) {
/*
- * No need to save buffer position because `asReadOnlyBuffer()`
- * creates an independent buffer with its own mark and position.
- * Byte order of the view is intentionally left to the default
- * because we expect the callers to set the order that they need.
+ * No need to save buffer position because `asReadOnlyBuffer()` creates an independent buffer
+ * with its own mark and position. Byte order of the view is intentionally fixed to BIG_ENDIAN
+ * (the default) regardless the byte order of the original buffer.
*/
final ByteBuffer buffer = (ByteBuffer) input;
result = prober.test(type.cast(buffer.asReadOnlyBuffer()));
@@ -451,7 +450,7 @@ public abstract class DataStoreProvider {
* @since 1.2
*/
@FunctionalInterface
- public interface Prober<S> {
+ protected interface Prober<S> {
/**
* Probes the given input and returns an indication about whether that input is supported.
* This method may return {@code SUPPORTED} if there is reasonable chance of success based
diff --git a/storage/sis-storage/src/test/java/org/apache/sis/storage/DataStoreProviderTest.java b/storage/sis-storage/src/test/java/org/apache/sis/storage/DataStoreProviderTest.java
index c311793..c423504 100644
--- a/storage/sis-storage/src/test/java/org/apache/sis/storage/DataStoreProviderTest.java
+++ b/storage/sis-storage/src/test/java/org/apache/sis/storage/DataStoreProviderTest.java
@@ -22,6 +22,7 @@ import java.io.InputStream;
import java.io.IOException;
import java.io.InputStreamReader;
import java.nio.ByteBuffer;
+import java.nio.ByteOrder;
import java.nio.charset.StandardCharsets;
import org.opengis.parameter.ParameterDescriptorGroup;
import org.apache.sis.test.DependsOn;
@@ -81,6 +82,38 @@ public final strictfp class DataStoreProviderTest extends TestCase {
}
/**
+ * Verifies that the {@link ByteBuffer} given to the {@code Prober} always have the default
+ * {@link ByteOrder#BIG_ENDIAN}. Some data store implementations rely on this default value.
+ *
+ * @throws DataStoreException if an error occurred while using the storage connector.
+ */
+ @Test
+ public void verifyByteOrder() throws DataStoreException {
+ /*
+ * Creates a byte buffer with an arbitrary position and byte order.
+ */
+ final ByteBuffer original = ByteBuffer.allocate(8).order(ByteOrder.LITTLE_ENDIAN);
+ original.position(3);
+ final StorageConnector connector = new StorageConnector(original);
+ /*
+ * Verify that the byte buffer given to the prober always have the big endian order,
+ * regardless the byte order of the original buffer. This is part of method contract.
+ */
+ assertEquals(provider.probeContent(connector, ByteBuffer.class, buffer -> {
+ assertEquals(ByteOrder.BIG_ENDIAN, buffer.order());
+ assertEquals(3, buffer.position());
+ assertEquals(8, buffer.limit());
+ buffer.position(5).mark();
+ return ProbeResult.UNDETERMINED;
+ }), ProbeResult.UNDETERMINED);
+ /*
+ * Verifies that the origial buffer has its byte order and position unchanged.
+ */
+ assertEquals(ByteOrder.LITTLE_ENDIAN, original.order());
+ assertEquals(3, original.position());
+ }
+
+ /**
* Verifies that {@link DataStoreProvider#probeContent(StorageConnector, Class, Prober)}
* with a {@link ByteBuffer} leaves the position of the original buffer unchanged.
*