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
The following commit(s) were added to refs/heads/geoapi-4.0 by this push: new 1aa1be663e Use the image input/output stream provided by the storage connector in World File data store. Fix a "channel closed" exception when using the image output stream. 1aa1be663e is described below commit 1aa1be663e26724b9ffea63c4afeec88560c9f62 Author: Martin Desruisseaux <martin.desruisse...@geomatys.com> AuthorDate: Fri Mar 14 14:55:15 2025 +0100 Use the image input/output stream provided by the storage connector in World File data store. Fix a "channel closed" exception when using the image output stream. --- .../org/apache/sis/storage/StorageConnector.java | 28 +++++---- .../org/apache/sis/storage/image/FormatFilter.java | 4 +- .../org/apache/sis/storage/image/FormatFinder.java | 67 ++++++---------------- .../apache/sis/storage/image/MultiImageStore.java | 2 +- .../apache/sis/storage/image/SingleImageStore.java | 2 +- .../apache/sis/storage/image/WorldFileStore.java | 27 ++------- .../apache/sis/storage/image/WritableStore.java | 2 +- .../sis/storage/image/SelfConsistencyTest.java | 2 +- .../sis/storage/image/WorldFileStoreTest.java | 13 ++--- 9 files changed, 48 insertions(+), 99 deletions(-) diff --git a/endorsed/src/org.apache.sis.storage/main/org/apache/sis/storage/StorageConnector.java b/endorsed/src/org.apache.sis.storage/main/org/apache/sis/storage/StorageConnector.java index da17522954..dc53518cbf 100644 --- a/endorsed/src/org.apache.sis.storage/main/org/apache/sis/storage/StorageConnector.java +++ b/endorsed/src/org.apache.sis.storage/main/org/apache/sis/storage/StorageConnector.java @@ -18,7 +18,6 @@ package org.apache.sis.storage; import java.util.Map; import java.util.HashMap; -import java.util.Iterator; import java.util.IdentityHashMap; import java.util.Objects; import java.util.function.UnaryOperator; @@ -178,8 +177,8 @@ public class StorageConnector implements Serializable { /** * A flag for <code>{@linkplain #addView(Class, Object, Class, byte) addView}(…, view, source, flags)</code> - * telling that before resetting the {@code view}, we need to reset the {@code source} first. This flag should - * can be unset if any change in the position of {@code view} is immediately reflected in the position of + * telling that before resetting the {@code view}, we need to reset the {@code source} first. This flag can + * be unset if any change in the position of {@code view} is immediately reflected in the position of * {@code source}, and vice-versa. * * @see Coupled#cascadeOnReset() @@ -188,8 +187,8 @@ public class StorageConnector implements Serializable { /** * A flag for <code>{@linkplain #addView(Class, Object, Class, byte) addView}(…, view, source, flags)</code> - * telling that {@code view} cannot be reset, so it should be set to {@code null} instead. This implies - * that a new view of the same type will be recreated next time it will be requested. + * telling that {@code view} cannot be reset, so it should be set to {@code null} instead. This implies that + * a new view of the same type will be recreated next time it will be requested. * * <p>When this flag is set, the {@link #CASCADE_ON_RESET} should usually be set at the same time.</p> */ @@ -1257,7 +1256,7 @@ public class StorageConnector implements Serializable { final ImageInputStream in = getStorageAs(ImageInputStream.class); if (in != null) { in.mark(); - final byte[] buffer = new byte[MINIMAL_BUFFER_SIZE]; + final var buffer = new byte[MINIMAL_BUFFER_SIZE]; final int n = in.read(buffer); in.reset(); if (n >= 1) { @@ -1464,7 +1463,7 @@ public class StorageConnector implements Serializable { private DataSource createDataSource() throws DataStoreException { final URI uri = getStorageAs(URI.class); if (uri != null && Constants.JDBC.equalsIgnoreCase(uri.getScheme())) { - final var source = new URLDataSource(uri); + final DataSource source = new URLDataSource(uri); addView(DataSource.class, source, null, (byte) 0); return source; } @@ -1638,13 +1637,16 @@ public class StorageConnector implements Serializable { Coupled c = views.get(DataOutput.class); views.put(ImageOutputStream.class, c); // Share the same `Coupled` instance. } else { + final byte cascade; final ChannelDataOutput c = getStorageAs(ChannelDataOutput.class); if (c != null && c.channel instanceof ReadableByteChannel) { asDataOutput = new ChannelImageOutputStream(c); + cascade = CASCADE_ON_RESET; } else { asDataOutput = null; // Remember that there is no view. + cascade = 0; } - addView(ImageOutputStream.class, asDataOutput, ChannelDataOutput.class, (byte) 0); + addView(ImageOutputStream.class, asDataOutput, ChannelDataOutput.class, cascade); } return asDataOutput; } @@ -1796,7 +1798,7 @@ public class StorageConnector implements Serializable { * Create a list of all views to close. The boolean value is TRUE if the view should be closed, or FALSE * if the view should be protected (not closed). FALSE values shall have precedence over TRUE values. */ - final Map<AutoCloseable,Boolean> toClose = new IdentityHashMap<>(views.size()); + final var toClose = new IdentityHashMap<AutoCloseable,Boolean>(views.size()); for (Coupled c : views.values()) { Object v = c.view; if (v != view) { @@ -1822,11 +1824,7 @@ public class StorageConnector implements Serializable { /* * Trim the map in order to keep only the views to close. */ - for (final Iterator<Boolean> it = toClose.values().iterator(); it.hasNext();) { - if (Boolean.FALSE.equals(it.next())) { - it.remove(); - } - } + toClose.values().removeIf((c) -> Boolean.FALSE.equals(c)); /* * The "AutoCloseable.close() is not indempotent" problem * ------------------------------------------------------ @@ -1917,7 +1915,7 @@ public class StorageConnector implements Serializable { if (isClosed()) { return Resources.format(Resources.Keys.ClosedStorageConnector); } - final TreeTable table = new DefaultTreeTable(TableColumn.NAME, TableColumn.VALUE); + final var table = new DefaultTreeTable(TableColumn.NAME, TableColumn.VALUE); final TreeTable.Node root = table.getRoot(); root.setValue(TableColumn.NAME, Classes.getShortClassName(this)); root.setValue(TableColumn.VALUE, getStorageName()); diff --git a/endorsed/src/org.apache.sis.storage/main/org/apache/sis/storage/image/FormatFilter.java b/endorsed/src/org.apache.sis.storage/main/org/apache/sis/storage/image/FormatFilter.java index 05dc15d2e2..bc42a84438 100644 --- a/endorsed/src/org.apache.sis.storage/main/org/apache/sis/storage/image/FormatFilter.java +++ b/endorsed/src/org.apache.sis.storage/main/org/apache/sis/storage/image/FormatFilter.java @@ -74,7 +74,7 @@ enum FormatFilter { /** * Types of image inputs that are accepted by {@link StorageConnector}. An input type is accepted if * it is equal to one of those types. We do not use {@link Class#isAssignableFrom(Class)} because if - * an image reader requests a sub-type, we can probably not provide it ourselves. + * an image reader requests a sub-type, we can probably not create instances of that class ourselves. */ private static final Class<?>[] VALID_INPUTS = { ImageInputStream.class, DataInput.class, InputStream.class, File.class, Path.class, URL.class, URI.class @@ -83,7 +83,7 @@ enum FormatFilter { /** * Types of image outputs that are accepted by {@link StorageConnector}. An output type is accepted * if it is equal to one of those types. We do not use {@link Class#isAssignableFrom(Class)} because - * if an image reader requests a sub-type, we can probably not provide it ourselves. + * an image writer requests a sub-type, we can probably not create instances of that class ourselves. */ private static final Class<?>[] VALID_OUTPUTS = { // `ImageOutputStream` intentionally excluded because not handled by `StorageConnector`. diff --git a/endorsed/src/org.apache.sis.storage/main/org/apache/sis/storage/image/FormatFinder.java b/endorsed/src/org.apache.sis.storage/main/org/apache/sis/storage/image/FormatFinder.java index 949657a429..be05f079cc 100644 --- a/endorsed/src/org.apache.sis.storage/main/org/apache/sis/storage/image/FormatFinder.java +++ b/endorsed/src/org.apache.sis.storage/main/org/apache/sis/storage/image/FormatFinder.java @@ -19,25 +19,21 @@ package org.apache.sis.storage.image; import java.util.Map; import java.util.LinkedHashMap; import java.io.DataOutput; -import java.io.File; import java.io.IOException; import java.nio.file.Path; import java.nio.file.Files; import javax.imageio.ImageIO; import javax.imageio.ImageReader; import javax.imageio.ImageWriter; -import javax.imageio.IIOException; import javax.imageio.spi.ImageReaderSpi; import javax.imageio.spi.ImageWriterSpi; import javax.imageio.stream.ImageInputStream; import javax.imageio.stream.ImageOutputStream; -import javax.imageio.stream.FileImageOutputStream; import java.awt.image.RenderedImage; import org.apache.sis.storage.StorageConnector; import org.apache.sis.storage.DataStoreException; import org.apache.sis.io.stream.InternalOptionKey; import org.apache.sis.io.stream.IOUtilities; -import org.apache.sis.util.Workaround; /** @@ -128,6 +124,8 @@ final class FormatFinder implements AutoCloseable { { this.provider = provider; this.connector = connector; + + @SuppressWarnings("LocalVariableHidesMemberVariable") Object storage = connector.getStorage(); if (storage instanceof ImageReader) { reader = (ImageReader) storage; @@ -141,7 +139,7 @@ final class FormatFinder implements AutoCloseable { this.storage = storage; this.suffix = IOUtilities.extension(storage); final var filter = connector.getOption(InternalOptionKey.PREFERRED_PROVIDERS); - preferredFormat = filter instanceof DataStoreFilter ? ((DataStoreFilter) filter).preferred : null; + preferredFormat = (filter instanceof DataStoreFilter) ? ((DataStoreFilter) filter).preferred : null; /* * Detect if the image can be opened in read/write mode. * If not, it will be opened in read-only mode. @@ -171,11 +169,14 @@ final class FormatFinder implements AutoCloseable { /** * Returns the name of the format. + * This method may open the file for determining its format. + * It may result in the creation of a new, initially empty, file. * * @return name of the format, or {@code null} if unknown. */ final String[] getFormatName() throws DataStoreException, IOException { if (openAsWriter) { + @SuppressWarnings("LocalVariableHidesMemberVariable") final ImageWriter writer = getOrCreateWriter(null); if (writer != null) { final ImageWriterSpi spi = writer.getOriginatingProvider(); @@ -184,6 +185,7 @@ final class FormatFinder implements AutoCloseable { } } } else { + @SuppressWarnings("LocalVariableHidesMemberVariable") final ImageReader reader = getOrCreateReader(); if (reader != null) { final ImageReaderSpi spi = reader.getOriginatingProvider(); @@ -196,7 +198,7 @@ final class FormatFinder implements AutoCloseable { } /** - * Returns the user-specified reader or searches for a reader that claim to be able to read the storage input. + * Returns the user-specified reader or searches for a reader that claims to be able to read the storage input. * This method tries first the readers associated to the file suffix. If no reader is found, then this method * tries all other readers. * @@ -205,7 +207,7 @@ final class FormatFinder implements AutoCloseable { final ImageReader getOrCreateReader() throws DataStoreException, IOException { if (!readerLookupDone) { readerLookupDone = true; - final Map<ImageReaderSpi,Boolean> deferred = new LinkedHashMap<>(); + final var deferred = new LinkedHashMap<ImageReaderSpi,Boolean>(); if (preferredFormat != null) { reader = FormatFilter.NAME.createReader(preferredFormat, this, deferred); } @@ -223,23 +225,21 @@ final class FormatFinder implements AutoCloseable { ImageInputStream stream = null; for (final Map.Entry<ImageReaderSpi,Boolean> entry : deferred.entrySet()) { if (entry.getValue()) { - if (stream == null) try { + if (stream == null) { if (isWritable) { // ImageOutputStream is both read and write. - stream = ImageIO.createImageOutputStream(storage); + stream = connector.getStorageAs(ImageOutputStream.class); } if (stream == null) { - stream = ImageIO.createImageInputStream(storage); + stream = connector.getStorageAs(ImageInputStream.class); if (stream == null) break; } - } catch (IIOException e) { - throw unwrap(e); } final ImageReaderSpi p = entry.getKey(); if (p.canDecodeInput(stream)) { reader = p.createReaderInstance(); reader.setInput(stream); - keepOpen = storage; + keepOpen = stream; break; } } @@ -259,7 +259,7 @@ final class FormatFinder implements AutoCloseable { final ImageWriter getOrCreateWriter(final RenderedImage image) throws DataStoreException, IOException { if (!writerLookupDone) { writerLookupDone = true; - final Map<ImageWriterSpi,Boolean> deferred = new LinkedHashMap<>(); + final var deferred = new LinkedHashMap<ImageWriterSpi,Boolean>(); if (preferredFormat != null) { writer = FormatFilter.NAME.createWriter(preferredFormat, this, image, deferred); } @@ -273,20 +273,13 @@ final class FormatFinder implements AutoCloseable { for (final Map.Entry<ImageWriterSpi,Boolean> entry : deferred.entrySet()) { if (entry.getValue()) { if (stream == null) { - final File file = connector.getStorageAs(File.class); - if (file != null) { - stream = new FileImageOutputStream(file); - } else try { - stream = ImageIO.createImageOutputStream(storage); - if (stream == null) break; - } catch (IIOException e) { - throw unwrap(e); - } + stream = connector.getStorageAs(ImageOutputStream.class); + if (stream == null) break; } final ImageWriterSpi p = entry.getKey(); writer = p.createWriterInstance(); writer.setOutput(stream); - keepOpen = storage; + keepOpen = stream; break; } } @@ -297,30 +290,8 @@ final class FormatFinder implements AutoCloseable { } /** - * Returns the cause of given exception if it exists, or the exception itself otherwise. - * This method is invoked in the {@code catch} block of a {@code try} block invoking - * {@link ImageIO#createImageInputStream(Object)} or - * {@link ImageIO#createImageOutputStream(Object)}. - * - * <h4>Rational</h4> - * As of Java 18, above-cited methods systematically catch all {@link IOException}s and wrap - * them in an {@link IIOException} with <q>Cannot create cache file!</q> error message. - * This is conform to Image I/O specification but misleading if the stream provider throws an - * {@link IOException} for another reason. Even when the failure is really caused by a problem - * with cache file, we want to propagate the original exception to user because its message - * may tell that there is no space left on device or no write permission. - * - * @see org.apache.sis.storage.StorageConnector#unwrap(IIOException) - */ - @Workaround(library = "JDK", version = "18") - private static IOException unwrap(final IIOException e) { - final Throwable cause = e.getCause(); - return (cause instanceof IOException) ? (IOException) cause : e; - } - - /** - * Closes all unused resources. Keep open only the find of objects needed by the image reader or writer. - * This method must be invoked after by {@link WorldFileStore} construction. + * Closes all unused resources, keeping open only the object needed by the image reader or writer. + * This method should be invoked after {@link WorldFileStore} construction in a {@code try} block. */ @Override public final void close() throws DataStoreException { diff --git a/endorsed/src/org.apache.sis.storage/main/org/apache/sis/storage/image/MultiImageStore.java b/endorsed/src/org.apache.sis.storage/main/org/apache/sis/storage/image/MultiImageStore.java index 8049603a53..72b7b43390 100644 --- a/endorsed/src/org.apache.sis.storage/main/org/apache/sis/storage/image/MultiImageStore.java +++ b/endorsed/src/org.apache.sis.storage/main/org/apache/sis/storage/image/MultiImageStore.java @@ -38,7 +38,7 @@ final class MultiImageStore extends WorldFileStore implements Aggregate { * @throws IOException if an error occurred while creating the image reader instance. */ MultiImageStore(final FormatFinder format) throws DataStoreException, IOException { - super(format, false); + super(format); } /** diff --git a/endorsed/src/org.apache.sis.storage/main/org/apache/sis/storage/image/SingleImageStore.java b/endorsed/src/org.apache.sis.storage/main/org/apache/sis/storage/image/SingleImageStore.java index cd8f1ddf69..b27693f864 100644 --- a/endorsed/src/org.apache.sis.storage/main/org/apache/sis/storage/image/SingleImageStore.java +++ b/endorsed/src/org.apache.sis.storage/main/org/apache/sis/storage/image/SingleImageStore.java @@ -54,7 +54,7 @@ final class SingleImageStore extends WorldFileStore implements GridCoverageResou * @throws IOException if an error occurred while creating the image reader instance. */ SingleImageStore(final FormatFinder format) throws DataStoreException, IOException { - super(format, true); + super(format); } /** diff --git a/endorsed/src/org.apache.sis.storage/main/org/apache/sis/storage/image/WorldFileStore.java b/endorsed/src/org.apache.sis.storage/main/org/apache/sis/storage/image/WorldFileStore.java index e58db7265f..49a8f0eb12 100644 --- a/endorsed/src/org.apache.sis.storage/main/org/apache/sis/storage/image/WorldFileStore.java +++ b/endorsed/src/org.apache.sis.storage/main/org/apache/sis/storage/image/WorldFileStore.java @@ -232,27 +232,11 @@ public class WorldFileStore extends PRJDataStore { /** * Creates a new store from the given file, URL or stream. * - * @param provider the factory that created this {@code DataStore} instance, or {@code null} if unspecified. - * @param connector information about the storage (URL, stream, <i>etc</i>). + * @param format information about the storage (URL, stream, <i>etc</i>) and the reader/writer to use. * @throws DataStoreException if an error occurred while opening the stream. * @throws IOException if an error occurred while creating the image reader instance. */ - public WorldFileStore(final WorldFileStoreProvider provider, final StorageConnector connector) - throws DataStoreException, IOException - { - this(new FormatFinder(provider, connector), true); - } - - /** - * Creates a new store from the given file, URL or stream. - * - * @param format information about the storage (URL, stream, <i>etc</i>) and the reader/writer to use. - * @param readOnly {@code true} if the store should be open in read-only mode, ignoring {@code format}. - * This is a workaround while waiting for JEP 447: Statements before super(…). - * @throws DataStoreException if an error occurred while opening the stream. - * @throws IOException if an error occurred while creating the image reader instance. - */ - WorldFileStore(final FormatFinder format, final boolean readOnly) throws DataStoreException, IOException { + WorldFileStore(final FormatFinder format) throws DataStoreException, IOException { super(format.provider, format.connector); listeners.useReadOnlyEvents(); identifiers = new HashMap<>(); @@ -260,16 +244,13 @@ public class WorldFileStore extends PRJDataStore { if (format.storage instanceof Closeable) { toClose = (Closeable) format.storage; } - if (readOnly || !format.openAsWriter) { + if (!format.openAsWriter) { reader = format.getOrCreateReader(); if (reader == null) { throw new UnsupportedStorageException(super.getLocale(), WorldFileStoreProvider.NAME, format.storage, format.connector.getOption(OptionKey.OPEN_OPTIONS)); } configureReader(); - if (readOnly) { - format.close(); - } /* * Do not invoke any method that may cause the image reader to start reading the stream, * because the `WritableStore` subclass will want to save the initial stream position. @@ -815,7 +796,7 @@ loop: for (int convention=0;; convention++) { final ImageReader codec = reader; if (codec != null) codec.abort(); synchronized (this) { - final Closeable stream = toClose; + final Closeable stream = toClose; reader = null; toClose = null; metadata = null; diff --git a/endorsed/src/org.apache.sis.storage/main/org/apache/sis/storage/image/WritableStore.java b/endorsed/src/org.apache.sis.storage/main/org/apache/sis/storage/image/WritableStore.java index aa97853d4f..f6faac8564 100644 --- a/endorsed/src/org.apache.sis.storage/main/org/apache/sis/storage/image/WritableStore.java +++ b/endorsed/src/org.apache.sis.storage/main/org/apache/sis/storage/image/WritableStore.java @@ -107,7 +107,7 @@ class WritableStore extends WorldFileStore { * @throws IOException if an error occurred while creating the image reader instance. */ WritableStore(final FormatFinder format) throws DataStoreException, IOException { - super(format, false); + super(format); if (getCurrentReader() != null) { numImages = -1; } else { diff --git a/endorsed/src/org.apache.sis.storage/test/org/apache/sis/storage/image/SelfConsistencyTest.java b/endorsed/src/org.apache.sis.storage/test/org/apache/sis/storage/image/SelfConsistencyTest.java index a96c7020b3..7bb47e5b1d 100644 --- a/endorsed/src/org.apache.sis.storage/test/org/apache/sis/storage/image/SelfConsistencyTest.java +++ b/endorsed/src/org.apache.sis.storage/test/org/apache/sis/storage/image/SelfConsistencyTest.java @@ -56,7 +56,7 @@ public final class SelfConsistencyTest extends CoverageReadConsistency<WorldFile private static WorldFileStore openFile() throws IOException, DataStoreException { final URL url = WorldFileStoreTest.class.getResource("gradient.png"); assertNotNull(url, "Test file not found."); - return new WorldFileStore(null, new StorageConnector(url)); + return new WorldFileStore(new FormatFinder(null, new StorageConnector(url))); } /** diff --git a/endorsed/src/org.apache.sis.storage/test/org/apache/sis/storage/image/WorldFileStoreTest.java b/endorsed/src/org.apache.sis.storage/test/org/apache/sis/storage/image/WorldFileStoreTest.java index 9027bd7ec4..0524750b93 100644 --- a/endorsed/src/org.apache.sis.storage/test/org/apache/sis/storage/image/WorldFileStoreTest.java +++ b/endorsed/src/org.apache.sis.storage/test/org/apache/sis/storage/image/WorldFileStoreTest.java @@ -69,7 +69,7 @@ public final class WorldFileStoreTest extends TestCase { */ @Test public void testProbeContent() throws DataStoreException { - final WorldFileStoreProvider provider = new WorldFileStoreProvider(); + final var provider = new WorldFileStoreProvider(); final ProbeResult result = provider.probeContent(testData()); assertTrue(result.isSupported()); assertEquals("image/png", result.getMimeType()); @@ -82,7 +82,7 @@ public final class WorldFileStoreTest extends TestCase { */ @Test public void testMetadata() throws DataStoreException { - final WorldFileStoreProvider provider = new WorldFileStoreProvider(); + final var provider = new WorldFileStoreProvider(); try (WorldFileStore store = provider.open(testData())) { /* * Opportunistic check of store type. Should be read-only, @@ -104,8 +104,7 @@ public final class WorldFileStoreTest extends TestCase { final String format = getSingleton(id.getResourceFormats()).getFormatSpecificationCitation().getTitle().toString(); assertTrue(format.contains("PNG"), format); assertEquals("WGS 84", getSingleton(metadata.getReferenceSystemInfo()).getName().getCode()); - final GeographicBoundingBox bbox = (GeographicBoundingBox) - getSingleton(getSingleton(id.getExtents()).getGeographicElements()); + final var bbox = (GeographicBoundingBox) getSingleton(getSingleton(id.getExtents()).getGeographicElements()); assertEquals( -90, bbox.getSouthBoundLatitude()); assertEquals( +90, bbox.getNorthBoundLatitude()); assertEquals(-180, bbox.getWestBoundLongitude()); @@ -128,7 +127,7 @@ public final class WorldFileStoreTest extends TestCase { */ @Test public void testReadWrite(@TempDir final Path directory) throws DataStoreException, IOException { - final WorldFileStoreProvider provider = new WorldFileStoreProvider(false); + final var provider = new WorldFileStoreProvider(false); try (WorldFileStore source = provider.open(testData())) { assertFalse(source instanceof WritableStore); final GridCoverageResource resource = getSingleton(source.components()); @@ -138,13 +137,13 @@ public final class WorldFileStoreTest extends TestCase { * Write the resource in a new file using a different format. */ final Path targetPath = directory.resolve("copy.jpg"); - final StorageConnector connector = new StorageConnector(targetPath); + final var connector = new StorageConnector(targetPath); connector.setOption(OptionKey.OPEN_OPTIONS, new StandardOpenOption[] { StandardOpenOption.CREATE_NEW, StandardOpenOption.WRITE }); try (WritableStore target = (WritableStore) provider.open(connector)) { assertEquals(0, target.isMultiImages()); - final WritableResource copy = (WritableResource) target.add(resource); + final var copy = (WritableResource) target.add(resource); assertEquals(1, target.isMultiImages()); assertNotSame(resource, copy); assertEquals (resource.getGridGeometry(), copy.getGridGeometry());