This is an automated email from the ASF dual-hosted git repository. bodewig pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/commons-compress.git
The following commit(s) were added to refs/heads/master by this push: new 5ea938c make MultiReadOnlySeekableByteChannel a bit more spec-compliant 5ea938c is described below commit 5ea938cb476513f6465612312a0cda1d60f5ac56 Author: Stefan Bodewig <bode...@apache.org> AuthorDate: Sat Jan 4 15:50:03 2020 +0100 make MultiReadOnlySeekableByteChannel a bit more spec-compliant --- .../utils/MultiReadOnlySeekableByteChannel.java | 13 +++ .../MultiReadOnlySeekableByteChannelTest.java | 93 ++++++++++++++++++++++ .../utils/SeekableInMemoryByteChannelTest.java | 32 ++++---- 3 files changed, 122 insertions(+), 16 deletions(-) diff --git a/src/main/java/org/apache/commons/compress/utils/MultiReadOnlySeekableByteChannel.java b/src/main/java/org/apache/commons/compress/utils/MultiReadOnlySeekableByteChannel.java index 21f25ec..f46e771 100644 --- a/src/main/java/org/apache/commons/compress/utils/MultiReadOnlySeekableByteChannel.java +++ b/src/main/java/org/apache/commons/compress/utils/MultiReadOnlySeekableByteChannel.java @@ -117,6 +117,13 @@ public class MultiReadOnlySeekableByteChannel implements SeekableByteChannel { return true; } + /** + * Returns this channel's position. + * + * <p>This method violates the contract of {@link SeekableByteChannel#position()} as it will not throw any exception + * when invoked on a closed channel. Instead it will return the position the channel had when close has been + * called.</p> + */ @Override public long position() { return globalPosition; @@ -131,6 +138,9 @@ public class MultiReadOnlySeekableByteChannel implements SeekableByteChannel { * @throws IOException */ public synchronized SeekableByteChannel position(long channelNumber, long relativeOffset) throws IOException { + if (!isOpen()) { + throw new ClosedChannelException(); + } long globalPosition = relativeOffset; for (int i = 0; i < channelNumber; i++) { globalPosition += channels.get(i).size(); @@ -141,6 +151,9 @@ public class MultiReadOnlySeekableByteChannel implements SeekableByteChannel { @Override public long size() throws IOException { + if (!isOpen()) { + throw new ClosedChannelException(); + } long acc = 0; for (SeekableByteChannel ch : channels) { acc += ch.size(); diff --git a/src/test/java/org/apache/commons/compress/utils/MultiReadOnlySeekableByteChannelTest.java b/src/test/java/org/apache/commons/compress/utils/MultiReadOnlySeekableByteChannelTest.java index 94a218e..6d15e9b 100644 --- a/src/test/java/org/apache/commons/compress/utils/MultiReadOnlySeekableByteChannelTest.java +++ b/src/test/java/org/apache/commons/compress/utils/MultiReadOnlySeekableByteChannelTest.java @@ -29,6 +29,7 @@ import java.util.Arrays; import java.util.List; import org.junit.Assert; +import org.junit.Ignore; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; @@ -291,4 +292,96 @@ public class MultiReadOnlySeekableByteChannelTest { return this; } } + + // Contract Tests added in response to https://issues.apache.org/jira/browse/COMPRESS-499 + + private SeekableByteChannel testChannel() { + return MultiReadOnlySeekableByteChannel + .forSeekableByteChannels(makeEmpty(), makeEmpty()); + } + + // https://docs.oracle.com/javase/7/docs/api/java/io/Closeable.html#close() + + /* + * <q>If the stream is already closed then invoking this method has no effect.</q> + */ + @Test + public void closeIsIdempotent() throws Exception { + try (SeekableByteChannel c = testChannel()) { + c.close(); + Assert.assertFalse(c.isOpen()); + c.close(); + Assert.assertFalse(c.isOpen()); + } + } + + // https://docs.oracle.com/javase/7/docs/api/java/nio/channels/SeekableByteChannel.html#position() + + /* + * <q>ClosedChannelException - If this channel is closed</q> + */ + @Test + @Ignore("we deliberately violate the spec") + public void throwsClosedChannelExceptionWhenPositionIsReadOnClosedChannel() throws Exception { + thrown.expect(ClosedChannelException.class); + try (SeekableByteChannel c = testChannel()) { + c.close(); + c.position(); + } + } + + // https://docs.oracle.com/javase/7/docs/api/java/nio/channels/SeekableByteChannel.html#size() + + /* + * <q>ClosedChannelException - If this channel is closed</q> + */ + @Test + public void throwsClosedChannelExceptionWhenSizeIsReadOnClosedChannel() throws Exception { + thrown.expect(ClosedChannelException.class); + try (SeekableByteChannel c = testChannel()) { + c.close(); + c.size(); + } + } + + // https://docs.oracle.com/javase/7/docs/api/java/nio/channels/SeekableByteChannel.html#position(long) + + /* + * <q>ClosedChannelException - If this channel is closed</q> + */ + @Test + public void throwsClosedChannelExceptionWhenPositionIsSetOnClosedChannel() throws Exception { + thrown.expect(ClosedChannelException.class); + try (SeekableByteChannel c = testChannel()) { + c.close(); + c.position(0); + } + } + + /* + * <q>Setting the position to a value that is greater than the current size is legal but does not change the size of + * the entity. A later attempt to read bytes at such a position will immediately return an end-of-file + * indication</q> + */ + @Test + public void readingFromAPositionAfterEndReturnsEOF() throws Exception { + try (SeekableByteChannel c = testChannel()) { + c.position(2); + Assert.assertEquals(2, c.position()); + ByteBuffer readBuffer = ByteBuffer.allocate(5); + Assert.assertEquals(-1, c.read(readBuffer)); + } + } + + /* + * <q>IllegalArgumentException - If the new position is negative</q> + */ + @Test + public void throwsIllegalArgumentExceptionWhenPositionIsSetToANegativeValue() throws Exception { + thrown.expect(IllegalArgumentException.class); + try (SeekableByteChannel c = testChannel()) { + c.position(-1); + } + } + } diff --git a/src/test/java/org/apache/commons/compress/utils/SeekableInMemoryByteChannelTest.java b/src/test/java/org/apache/commons/compress/utils/SeekableInMemoryByteChannelTest.java index df2fc83..e9824b4 100644 --- a/src/test/java/org/apache/commons/compress/utils/SeekableInMemoryByteChannelTest.java +++ b/src/test/java/org/apache/commons/compress/utils/SeekableInMemoryByteChannelTest.java @@ -211,8 +211,8 @@ public class SeekableInMemoryByteChannelTest { * <q>If the stream is already closed then invoking this method has no effect.</q> */ @Test - public void closeIsIdempotent() { - try (SeekableInMemoryByteChannel c = new SeekableInMemoryByteChannel()) { + public void closeIsIdempotent() throws Exception { + try (SeekableByteChannel c = new SeekableInMemoryByteChannel()) { c.close(); assertFalse(c.isOpen()); c.close(); @@ -228,7 +228,7 @@ public class SeekableInMemoryByteChannelTest { @Test(expected = ClosedChannelException.class) @Ignore("we deliberately violate the spec") public void throwsClosedChannelExceptionWhenPositionIsReadOnClosedChannel() throws Exception { - try (SeekableInMemoryByteChannel c = new SeekableInMemoryByteChannel()) { + try (SeekableByteChannel c = new SeekableInMemoryByteChannel()) { c.close(); c.position(); } @@ -242,7 +242,7 @@ public class SeekableInMemoryByteChannelTest { @Test(expected = ClosedChannelException.class) @Ignore("we deliberately violate the spec") public void throwsClosedChannelExceptionWhenSizeIsReadOnClosedChannel() throws Exception { - try (SeekableInMemoryByteChannel c = new SeekableInMemoryByteChannel()) { + try (SeekableByteChannel c = new SeekableInMemoryByteChannel()) { c.close(); c.size(); } @@ -255,7 +255,7 @@ public class SeekableInMemoryByteChannelTest { */ @Test(expected = ClosedChannelException.class) public void throwsClosedChannelExceptionWhenPositionIsSetOnClosedChannel() throws Exception { - try (SeekableInMemoryByteChannel c = new SeekableInMemoryByteChannel()) { + try (SeekableByteChannel c = new SeekableInMemoryByteChannel()) { c.close(); c.position(0); } @@ -268,7 +268,7 @@ public class SeekableInMemoryByteChannelTest { */ @Test public void readingFromAPositionAfterEndReturnsEOF() throws Exception { - try (SeekableInMemoryByteChannel c = new SeekableInMemoryByteChannel()) { + try (SeekableByteChannel c = new SeekableInMemoryByteChannel()) { c.position(2); assertEquals(2, c.position()); ByteBuffer readBuffer = ByteBuffer.allocate(5); @@ -283,7 +283,7 @@ public class SeekableInMemoryByteChannelTest { * unspecified.</q> */ public void writingToAPositionAfterEndGrowsChannel() throws Exception { - try (SeekableInMemoryByteChannel c = new SeekableInMemoryByteChannel()) { + try (SeekableByteChannel c = new SeekableInMemoryByteChannel()) { c.position(2); assertEquals(2, c.position()); ByteBuffer inData = ByteBuffer.wrap(testData); @@ -302,7 +302,7 @@ public class SeekableInMemoryByteChannelTest { */ @Test(expected = IllegalArgumentException.class) public void throwsIllegalArgumentExceptionWhenPositionIsSetToANegativeValue() throws Exception { - try (SeekableInMemoryByteChannel c = new SeekableInMemoryByteChannel()) { + try (SeekableByteChannel c = new SeekableInMemoryByteChannel()) { c.position(-1); } } @@ -314,7 +314,7 @@ public class SeekableInMemoryByteChannelTest { */ @Test public void truncateToCurrentSizeDoesntChangeAnything() throws Exception { - try (SeekableInMemoryByteChannel c = new SeekableInMemoryByteChannel(testData)) { + try (SeekableByteChannel c = new SeekableInMemoryByteChannel(testData)) { assertEquals(testData.length, c.size()); c.truncate(testData.length); assertEquals(testData.length, c.size()); @@ -329,7 +329,7 @@ public class SeekableInMemoryByteChannelTest { */ @Test public void truncateToBiggerSizeDoesntChangeAnything() throws Exception { - try (SeekableInMemoryByteChannel c = new SeekableInMemoryByteChannel(testData)) { + try (SeekableByteChannel c = new SeekableInMemoryByteChannel(testData)) { assertEquals(testData.length, c.size()); c.truncate(testData.length + 1); assertEquals(testData.length, c.size()); @@ -344,7 +344,7 @@ public class SeekableInMemoryByteChannelTest { */ @Test public void truncateDoesntChangeSmallPosition() throws Exception { - try (SeekableInMemoryByteChannel c = new SeekableInMemoryByteChannel(testData)) { + try (SeekableByteChannel c = new SeekableInMemoryByteChannel(testData)) { c.position(1); c.truncate(testData.length - 1); assertEquals(testData.length - 1, c.size()); @@ -357,7 +357,7 @@ public class SeekableInMemoryByteChannelTest { */ @Test public void truncateMovesPositionWhenShrinkingBeyondPosition() throws Exception { - try (SeekableInMemoryByteChannel c = new SeekableInMemoryByteChannel(testData)) { + try (SeekableByteChannel c = new SeekableInMemoryByteChannel(testData)) { c.position(4); c.truncate(3); assertEquals(3, c.size()); @@ -370,7 +370,7 @@ public class SeekableInMemoryByteChannelTest { */ @Test public void truncateMovesPositionWhenNotResizingButPositionBiggerThanSize() throws Exception { - try (SeekableInMemoryByteChannel c = new SeekableInMemoryByteChannel(testData)) { + try (SeekableByteChannel c = new SeekableInMemoryByteChannel(testData)) { c.position(2 * testData.length); c.truncate(testData.length); assertEquals(testData.length, c.size()); @@ -383,7 +383,7 @@ public class SeekableInMemoryByteChannelTest { */ @Test public void truncateMovesPositionWhenNewSizeIsBiggerThanSizeAndPositionIsEvenBigger() throws Exception { - try (SeekableInMemoryByteChannel c = new SeekableInMemoryByteChannel(testData)) { + try (SeekableByteChannel c = new SeekableInMemoryByteChannel(testData)) { c.position(2 * testData.length); c.truncate(testData.length + 1); assertEquals(testData.length, c.size()); @@ -396,7 +396,7 @@ public class SeekableInMemoryByteChannelTest { */ @Test(expected = IllegalArgumentException.class) public void throwsIllegalArgumentExceptionWhenTruncatingToANegativeSize() throws Exception { - try (SeekableInMemoryByteChannel c = new SeekableInMemoryByteChannel()) { + try (SeekableByteChannel c = new SeekableInMemoryByteChannel()) { c.truncate(-1); } } @@ -407,7 +407,7 @@ public class SeekableInMemoryByteChannelTest { @Test(expected = ClosedChannelException.class) @Ignore("we deliberately violate the spec") public void throwsClosedChannelExceptionWhenTruncateIsCalledOnClosedChannel() throws Exception { - try (SeekableInMemoryByteChannel c = new SeekableInMemoryByteChannel()) { + try (SeekableByteChannel c = new SeekableInMemoryByteChannel()) { c.close(); c.truncate(0); }