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 <martin.desruisse...@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. *