diff --git a/imageio/imageio-core/src/main/java/com/twelvemonkeys/imageio/stream/BufferedChannelImageInputStream.java b/imageio/imageio-core/src/main/java/com/twelvemonkeys/imageio/stream/BufferedChannelImageInputStream.java index b11ef3ea..83a08410 100644 --- a/imageio/imageio-core/src/main/java/com/twelvemonkeys/imageio/stream/BufferedChannelImageInputStream.java +++ b/imageio/imageio-core/src/main/java/com/twelvemonkeys/imageio/stream/BufferedChannelImageInputStream.java @@ -31,6 +31,7 @@ package com.twelvemonkeys.imageio.stream; import javax.imageio.stream.ImageInputStreamImpl; +import java.io.Closeable; import java.io.File; import java.io.FileInputStream; import java.io.IOException; @@ -52,6 +53,10 @@ import static java.lang.Math.max; * for shorter reads, like single byte or bit reads. */ final class BufferedChannelImageInputStream extends ImageInputStreamImpl { + private static final Closeable CLOSEABLE_STUB = new Closeable() { + @Override public void close() {} + }; + static final int DEFAULT_BUFFER_SIZE = 8192; private ByteBuffer byteBuffer = ByteBuffer.allocate(DEFAULT_BUFFER_SIZE); @@ -63,6 +68,7 @@ final class BufferedChannelImageInputStream extends ImageInputStreamImpl { private final byte[] integralCacheArray = integralCache.array(); private SeekableByteChannel channel; + private Closeable closeable; /** * Constructs a {@code BufferedChannelImageInputStream} that will read from a given {@code File}. @@ -86,49 +92,62 @@ final class BufferedChannelImageInputStream extends ImageInputStreamImpl { * @throws SecurityException if a security manager is installed, and it denies read access to the file. */ public BufferedChannelImageInputStream(final Path file) throws IOException { - this(FileChannel.open(notNull(file, "file"), StandardOpenOption.READ)); + this(FileChannel.open(notNull(file, "file"), StandardOpenOption.READ), true); } /** * Constructs a {@code BufferedChannelImageInputStream} that will read from a given {@code RandomAccessFile}. - *

- * Closing this stream will close the {@code RandomAccessFile}. - *

* * @param file a {@code RandomAccessFile} to read from. * @throws IllegalArgumentException if {@code file} is {@code null}. */ public BufferedChannelImageInputStream(final RandomAccessFile file) { - // Assumption: Closing the FileChannel will also close its backing RandomAccessFile - // (it does in the OpenJDK implementation, and it makes sense, although I can't see this is documented behaviour). - this(notNull(file, "file").getChannel()); + // Closing the RAF is inconsistent, but emulates the behavior of javax.imageio.stream.FileImageInputStream + this(notNull(file, "file").getChannel(), true); } /** * Constructs a {@code BufferedChannelImageInputStream} that will read from a given {@code FileInputStream}. *

- * Closing this stream will close the {@code FileInputStream}. + * Closing this stream will not close the {@code FileInputStream}. *

* * @param inputStream a {@code FileInputStream} to read from. * @throws IllegalArgumentException if {@code inputStream} is {@code null}. */ public BufferedChannelImageInputStream(final FileInputStream inputStream) { - // Assumption: Closing the FileChannel will also close its backing FileInputStream (it does in the OpenJDK implementation, although I can't see this is documented). - this(notNull(inputStream, "inputStream").getChannel()); + this(notNull(inputStream, "inputStream").getChannel(), false); } /** * Constructs a {@code BufferedChannelImageInputStream} that will read from a given {@code SeekableByteChannel}. *

- * Closing this stream will close the {@code SeekableByteChannel}. + * Closing this stream will not close the {@code SeekableByteChannel}. *

* * @param channel a {@code SeekableByteChannel} to read from. * @throws IllegalArgumentException if {@code channel} is {@code null}. */ public BufferedChannelImageInputStream(final SeekableByteChannel channel) { + this(notNull(channel, "channel"), false); + } + + /** + * Constructs a {@code BufferedChannelImageInputStream} that will read from a given {@code Cache}. + *

+ * Closing this stream will close the {@code Cache}. + *

+ * + * @param cache a {@code SeekableByteChannel} to read from. + * @throws IllegalArgumentException if {@code channel} is {@code null}. + */ + BufferedChannelImageInputStream(final Cache cache) { + this(notNull(cache, "cache"), true); + } + + private BufferedChannelImageInputStream(final SeekableByteChannel channel, boolean closeChannelOnClose) { this.channel = notNull(channel, "channel"); + this.closeable = closeChannelOnClose ? this.channel : CLOSEABLE_STUB; } @SuppressWarnings("BooleanMethodIsAlwaysInverted") @@ -246,8 +265,14 @@ final class BufferedChannelImageInputStream extends ImageInputStreamImpl { buffer = null; byteBuffer = null; - channel.close(); channel = null; + + try { + closeable.close(); + } + finally { + closeable = null; + } } // Need to override the readShort(), readInt() and readLong() methods, @@ -315,9 +340,9 @@ final class BufferedChannelImageInputStream extends ImageInputStreamImpl { public void flushBefore(final long pos) throws IOException { super.flushBefore(pos); - if (channel instanceof MemoryCache) { + if (channel instanceof Cache) { // In case of memory cache, free up memory - ((MemoryCache) channel).flushBefore(pos); + ((Cache) channel).flushBefore(pos); } } } diff --git a/imageio/imageio-core/src/main/java/com/twelvemonkeys/imageio/stream/BufferedInputStreamImageInputStreamSpi.java b/imageio/imageio-core/src/main/java/com/twelvemonkeys/imageio/stream/BufferedInputStreamImageInputStreamSpi.java index cf22875f..1f59e5ac 100644 --- a/imageio/imageio-core/src/main/java/com/twelvemonkeys/imageio/stream/BufferedInputStreamImageInputStreamSpi.java +++ b/imageio/imageio-core/src/main/java/com/twelvemonkeys/imageio/stream/BufferedInputStreamImageInputStreamSpi.java @@ -53,7 +53,7 @@ public final class BufferedInputStreamImageInputStreamSpi extends ImageInputStre } // Otherwise, create a cache for backwards seeking - return new BufferedChannelImageInputStream(useCacheFile ? new DiskCache(channel, cacheDir): new MemoryCache(channel)); + return new BufferedChannelImageInputStream(useCacheFile ? new FileCache(channel, cacheDir) : new MemoryCache(channel)); } throw new IllegalArgumentException("Expected input of type InputStream: " + input); diff --git a/imageio/imageio-core/src/main/java/com/twelvemonkeys/imageio/stream/Cache.java b/imageio/imageio-core/src/main/java/com/twelvemonkeys/imageio/stream/Cache.java new file mode 100644 index 00000000..995a6bb3 --- /dev/null +++ b/imageio/imageio-core/src/main/java/com/twelvemonkeys/imageio/stream/Cache.java @@ -0,0 +1,7 @@ +package com.twelvemonkeys.imageio.stream; + +import java.nio.channels.SeekableByteChannel; + +interface Cache extends SeekableByteChannel { + void flushBefore(long pos); +} diff --git a/imageio/imageio-core/src/main/java/com/twelvemonkeys/imageio/stream/DiskCache.java b/imageio/imageio-core/src/main/java/com/twelvemonkeys/imageio/stream/FileCache.java similarity index 92% rename from imageio/imageio-core/src/main/java/com/twelvemonkeys/imageio/stream/DiskCache.java rename to imageio/imageio-core/src/main/java/com/twelvemonkeys/imageio/stream/FileCache.java index 18af816b..ce12060a 100644 --- a/imageio/imageio-core/src/main/java/com/twelvemonkeys/imageio/stream/DiskCache.java +++ b/imageio/imageio-core/src/main/java/com/twelvemonkeys/imageio/stream/FileCache.java @@ -26,19 +26,19 @@ import static java.nio.file.StandardOpenOption.WRITE; // the usual {@link #read read} and {@link #write write} methods. From the // standpoint of performance it is generally only worth mapping relatively // large files into memory. -final class DiskCache implements SeekableByteChannel { +final class FileCache implements Cache { final static int BLOCK_SIZE = 1 << 13; private final FileChannel cache; private final ReadableByteChannel channel; // TODO: Perhaps skip this constructor? - DiskCache(InputStream stream, File cacheDir) throws IOException { + FileCache(InputStream stream, File cacheDir) throws IOException { // Stream will be closed with channel, documented behavior this(Channels.newChannel(notNull(stream, "stream")), cacheDir); } - public DiskCache(ReadableByteChannel channel, File cacheDir) throws IOException { + public FileCache(ReadableByteChannel channel, File cacheDir) throws IOException { this.channel = notNull(channel, "channel"); isTrue(cacheDir == null || cacheDir.isDirectory(), cacheDir, "%s is not a directory"); @@ -65,12 +65,7 @@ final class DiskCache implements SeekableByteChannel { @Override public void close() throws IOException { - try { - cache.close(); - } - finally { - channel.close(); - } + cache.close(); } @Override @@ -110,5 +105,8 @@ final class DiskCache implements SeekableByteChannel { public SeekableByteChannel truncate(long size) { throw new NonWritableChannelException(); } + + @Override public void flushBefore(long pos) { + } } diff --git a/imageio/imageio-core/src/main/java/com/twelvemonkeys/imageio/stream/MemoryCache.java b/imageio/imageio-core/src/main/java/com/twelvemonkeys/imageio/stream/MemoryCache.java index e0f86bee..9e8bbae6 100644 --- a/imageio/imageio-core/src/main/java/com/twelvemonkeys/imageio/stream/MemoryCache.java +++ b/imageio/imageio-core/src/main/java/com/twelvemonkeys/imageio/stream/MemoryCache.java @@ -13,7 +13,7 @@ import java.util.List; import static com.twelvemonkeys.lang.Validate.notNull; import static java.lang.Math.min; -public final class MemoryCache implements SeekableByteChannel { +final class MemoryCache implements Cache { final static int BLOCK_SIZE = 1 << 13; @@ -78,12 +78,7 @@ public final class MemoryCache implements SeekableByteChannel { @Override public void close() throws IOException { - try { - cache.clear(); - } - finally { - channel.close(); - } + cache.clear(); } @Override @@ -135,7 +130,8 @@ public final class MemoryCache implements SeekableByteChannel { throw new NonWritableChannelException(); } - void flushBefore(long pos) { + @Override + public void flushBefore(long pos) { if (pos < start) { throw new IndexOutOfBoundsException("pos < flushed position"); } diff --git a/imageio/imageio-core/src/main/java/com/twelvemonkeys/imageio/stream/URLImageInputStreamSpi.java b/imageio/imageio-core/src/main/java/com/twelvemonkeys/imageio/stream/URLImageInputStreamSpi.java index f59b9c30..924b189b 100644 --- a/imageio/imageio-core/src/main/java/com/twelvemonkeys/imageio/stream/URLImageInputStreamSpi.java +++ b/imageio/imageio-core/src/main/java/com/twelvemonkeys/imageio/stream/URLImageInputStreamSpi.java @@ -80,7 +80,7 @@ public final class URLImageInputStreamSpi extends ImageInputStreamSpi { // Otherwise revert to cached InputStream urlStream = url.openStream(); - return new BufferedChannelImageInputStream(useCacheFile ? new DiskCache(urlStream, cacheDir) : new MemoryCache(urlStream)); + return new BufferedChannelImageInputStream(useCacheFile ? new FileCache(urlStream, cacheDir) : new MemoryCache(urlStream)); } throw new IllegalArgumentException("Expected input of type URL: " + input); diff --git a/imageio/imageio-core/src/test/java/com/twelvemonkeys/imageio/stream/BufferedChannelImageInputStreamDiskCacheTest.java b/imageio/imageio-core/src/test/java/com/twelvemonkeys/imageio/stream/BufferedChannelImageInputStreamFileCacheTest.java similarity index 93% rename from imageio/imageio-core/src/test/java/com/twelvemonkeys/imageio/stream/BufferedChannelImageInputStreamDiskCacheTest.java rename to imageio/imageio-core/src/test/java/com/twelvemonkeys/imageio/stream/BufferedChannelImageInputStreamFileCacheTest.java index 8050476e..eefd3818 100755 --- a/imageio/imageio-core/src/test/java/com/twelvemonkeys/imageio/stream/BufferedChannelImageInputStreamDiskCacheTest.java +++ b/imageio/imageio-core/src/test/java/com/twelvemonkeys/imageio/stream/BufferedChannelImageInputStreamFileCacheTest.java @@ -57,7 +57,7 @@ import static org.mockito.Mockito.verify; * @version $Id: BufferedFileImageInputStreamTestCase.java,v 1.0 Apr 21, 2009 10:58:48 AM haraldk Exp$ */ // TODO: Remove this test, and instead test the disk cache directly! -public class BufferedChannelImageInputStreamDiskCacheTest { +public class BufferedChannelImageInputStreamFileCacheTest { private final Random random = new Random(170984354357234566L); private InputStream randomDataToInputStream(byte[] data) { @@ -68,7 +68,7 @@ public class BufferedChannelImageInputStreamDiskCacheTest { @Test public void testCreate() throws IOException { - try (BufferedChannelImageInputStream stream = new BufferedChannelImageInputStream(new DiskCache(new ByteArrayInputStream(new byte[0]), null))) { + try (BufferedChannelImageInputStream stream = new BufferedChannelImageInputStream(new FileCache(new ByteArrayInputStream(new byte[0]), null))) { assertEquals("Stream length should be unknown", -1, stream.length()); } } @@ -76,7 +76,7 @@ public class BufferedChannelImageInputStreamDiskCacheTest { @Test public void testCreateNullStream() throws IOException { try { - new DiskCache((InputStream) null, null); + new FileCache((InputStream) null, null); fail("Expected IllegalArgumentException"); } catch (IllegalArgumentException expected) { @@ -90,7 +90,7 @@ public class BufferedChannelImageInputStreamDiskCacheTest { @Test public void testCreateNullChannel() throws IOException { try { - new DiskCache((ReadableByteChannel) null, null); + new FileCache((ReadableByteChannel) null, null); fail("Expected IllegalArgumentException"); } catch (IllegalArgumentException expected) { @@ -106,7 +106,7 @@ public class BufferedChannelImageInputStreamDiskCacheTest { byte[] data = new byte[1024 * 1024]; InputStream input = randomDataToInputStream(data); - try (BufferedChannelImageInputStream stream = new BufferedChannelImageInputStream(new DiskCache(input, null))) { + try (BufferedChannelImageInputStream stream = new BufferedChannelImageInputStream(new FileCache(input, null))) { assertEquals("Stream length should be unknown", -1, stream.length()); for (byte value : data) { @@ -122,7 +122,7 @@ public class BufferedChannelImageInputStreamDiskCacheTest { byte[] data = new byte[1024 * 1024]; InputStream input = randomDataToInputStream(data); - try (BufferedChannelImageInputStream stream = new BufferedChannelImageInputStream(new DiskCache(input, null))) { + try (BufferedChannelImageInputStream stream = new BufferedChannelImageInputStream(new FileCache(input, null))) { assertEquals("Stream length should be unknown", -1, stream.length()); byte[] result = new byte[1024]; @@ -141,7 +141,7 @@ public class BufferedChannelImageInputStreamDiskCacheTest { byte[] data = new byte[1024 * 14]; InputStream input = randomDataToInputStream(data); - try (BufferedChannelImageInputStream stream = new BufferedChannelImageInputStream(new DiskCache(input, null))) { + try (BufferedChannelImageInputStream stream = new BufferedChannelImageInputStream(new FileCache(input, null))) { assertEquals("Stream length should be unknown", -1, stream.length()); byte[] result = new byte[7]; @@ -159,7 +159,7 @@ public class BufferedChannelImageInputStreamDiskCacheTest { byte[] data = new byte[1024 * 18]; InputStream input = randomDataToInputStream(data); - try (BufferedChannelImageInputStream stream = new BufferedChannelImageInputStream(new DiskCache(input, null))) { + try (BufferedChannelImageInputStream stream = new BufferedChannelImageInputStream(new FileCache(input, null))) { assertEquals("Stream length should be unknown", -1, stream.length()); byte[] result = new byte[9]; @@ -180,7 +180,7 @@ public class BufferedChannelImageInputStreamDiskCacheTest { byte[] data = new byte[256]; InputStream input = randomDataToInputStream(data); - try (BufferedChannelImageInputStream stream = new BufferedChannelImageInputStream(new DiskCache(input, null))) { + try (BufferedChannelImageInputStream stream = new BufferedChannelImageInputStream(new FileCache(input, null))) { assertEquals("Stream length should be unknown", -1, stream.length()); byte[] buffer = new byte[data.length * 2]; @@ -198,7 +198,7 @@ public class BufferedChannelImageInputStreamDiskCacheTest { long value = ByteBuffer.wrap(bytes).getLong(); // Create stream - try (ImageInputStream stream = new BufferedChannelImageInputStream(new DiskCache(input, null))) { + try (ImageInputStream stream = new BufferedChannelImageInputStream(new FileCache(input, null))) { for (int i = 1; i <= 64; i++) { assertEquals(String.format("bit %d differ", i), (value << (i - 1L)) >>> 63L, stream.readBit()); } @@ -212,7 +212,7 @@ public class BufferedChannelImageInputStreamDiskCacheTest { long value = ByteBuffer.wrap(bytes).getLong(); // Create stream - try (ImageInputStream stream = new BufferedChannelImageInputStream(new DiskCache(input, null))) { + try (ImageInputStream stream = new BufferedChannelImageInputStream(new FileCache(input, null))) { for (int i = 1; i <= 64; i++) { stream.seek(0); assertEquals(String.format("bit %d differ", i), value >>> (64L - i), stream.readBits(i)); @@ -228,7 +228,7 @@ public class BufferedChannelImageInputStreamDiskCacheTest { long value = ByteBuffer.wrap(bytes).getLong(); // Create stream - try (ImageInputStream stream = new BufferedChannelImageInputStream(new DiskCache(input, null))) { + try (ImageInputStream stream = new BufferedChannelImageInputStream(new FileCache(input, null))) { for (int i = 1; i <= 60; i++) { stream.seek(0); stream.setBitOffset(i % 8); @@ -244,7 +244,7 @@ public class BufferedChannelImageInputStreamDiskCacheTest { InputStream input = randomDataToInputStream(bytes); ByteBuffer buffer = ByteBuffer.wrap(bytes).order(ByteOrder.BIG_ENDIAN); - try (final ImageInputStream stream = new BufferedChannelImageInputStream(new DiskCache(input, null))) { + try (final ImageInputStream stream = new BufferedChannelImageInputStream(new FileCache(input, null))) { stream.setByteOrder(ByteOrder.BIG_ENDIAN); for (int i = 0; i < bytes.length / 2; i++) { @@ -282,7 +282,7 @@ public class BufferedChannelImageInputStreamDiskCacheTest { InputStream input = randomDataToInputStream(bytes); ByteBuffer buffer = ByteBuffer.wrap(bytes).order(ByteOrder.BIG_ENDIAN); - try (final ImageInputStream stream = new BufferedChannelImageInputStream(new DiskCache(input, null))) { + try (final ImageInputStream stream = new BufferedChannelImageInputStream(new FileCache(input, null))) { stream.setByteOrder(ByteOrder.BIG_ENDIAN); for (int i = 0; i < bytes.length / 4; i++) { @@ -320,7 +320,7 @@ public class BufferedChannelImageInputStreamDiskCacheTest { InputStream input = randomDataToInputStream(bytes); ByteBuffer buffer = ByteBuffer.wrap(bytes).order(ByteOrder.BIG_ENDIAN); - try (final ImageInputStream stream = new BufferedChannelImageInputStream(new DiskCache(input, null))) { + try (final ImageInputStream stream = new BufferedChannelImageInputStream(new FileCache(input, null))) { stream.setByteOrder(ByteOrder.BIG_ENDIAN); for (int i = 0; i < bytes.length / 8; i++) { @@ -357,7 +357,7 @@ public class BufferedChannelImageInputStreamDiskCacheTest { byte[] bytes = new byte[9]; InputStream input = randomDataToInputStream(bytes); - try (final ImageInputStream stream = new BufferedChannelImageInputStream(new DiskCache(input, null))) { + try (final ImageInputStream stream = new BufferedChannelImageInputStream(new FileCache(input, null))) { stream.seek(1000); assertEquals(-1, stream.read()); @@ -406,11 +406,11 @@ public class BufferedChannelImageInputStreamDiskCacheTest { @Test public void testClose() throws IOException { // Create wrapper stream - InputStream mock = mock(InputStream.class); - ImageInputStream stream = new BufferedChannelImageInputStream(new DiskCache(mock, null)); + Cache cache = mock(Cache.class); + ImageInputStream stream = new BufferedChannelImageInputStream(cache); stream.close(); - verify(mock, only()).close(); + verify(cache, only()).close(); } @Test @@ -422,7 +422,7 @@ public class BufferedChannelImageInputStreamDiskCacheTest { byte[] bytes = new byte[size]; InputStream input = randomDataToInputStream(bytes); - try (BufferedChannelImageInputStream stream = new BufferedChannelImageInputStream(new DiskCache(input, null))) { + try (BufferedChannelImageInputStream stream = new BufferedChannelImageInputStream(new FileCache(input, null))) { byte[] result = new byte[size]; int head = stream.read(result, 0, 12); // Provoke a buffered read int len = stream.read(result, 12, size - 12); // Rest of buffer + direct read diff --git a/imageio/imageio-core/src/test/java/com/twelvemonkeys/imageio/stream/BufferedChannelImageInputStreamMemoryCacheTest.java b/imageio/imageio-core/src/test/java/com/twelvemonkeys/imageio/stream/BufferedChannelImageInputStreamMemoryCacheTest.java index 00206a71..588aa729 100755 --- a/imageio/imageio-core/src/test/java/com/twelvemonkeys/imageio/stream/BufferedChannelImageInputStreamMemoryCacheTest.java +++ b/imageio/imageio-core/src/test/java/com/twelvemonkeys/imageio/stream/BufferedChannelImageInputStreamMemoryCacheTest.java @@ -406,11 +406,11 @@ public class BufferedChannelImageInputStreamMemoryCacheTest { @Test public void testClose() throws IOException { // Create wrapper stream - InputStream mock = mock(InputStream.class); - ImageInputStream stream = new BufferedChannelImageInputStream(new MemoryCache(mock)); + Cache cache = mock(Cache.class); + ImageInputStream stream = new BufferedChannelImageInputStream(cache); stream.close(); - verify(mock, only()).close(); + verify(cache, only()).close(); } @Test diff --git a/imageio/imageio-core/src/test/java/com/twelvemonkeys/imageio/stream/BufferedChannelImageInputStreamTest.java b/imageio/imageio-core/src/test/java/com/twelvemonkeys/imageio/stream/BufferedChannelImageInputStreamTest.java index 417e0ce4..3a63b1f9 100755 --- a/imageio/imageio-core/src/test/java/com/twelvemonkeys/imageio/stream/BufferedChannelImageInputStreamTest.java +++ b/imageio/imageio-core/src/test/java/com/twelvemonkeys/imageio/stream/BufferedChannelImageInputStreamTest.java @@ -47,7 +47,7 @@ import java.util.Random; import static com.twelvemonkeys.imageio.stream.BufferedImageInputStreamTest.rangeEquals; import static org.junit.Assert.*; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.only; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; /** @@ -403,14 +403,11 @@ public class BufferedChannelImageInputStreamTest { @Test public void testCloseChannel() throws IOException { - // NOTE: As the stream-based constructor is chained to the channel-based one, - // we'll rely on the fact that closing the channel will close the stream. - - SeekableByteChannel mock = mock(SeekableByteChannel.class); - ImageInputStream stream = new BufferedChannelImageInputStream(mock); + SeekableByteChannel channel = mock(SeekableByteChannel.class); + ImageInputStream stream = new BufferedChannelImageInputStream(channel); stream.close(); - verify(mock, only()).close(); + verify(channel, never()).close(); } @Test