You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by bo...@apache.org on 2020/01/04 14:50:25 UTC

[commons-compress] branch master updated: make MultiReadOnlySeekableByteChannel a bit more spec-compliant

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 <bo...@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);
         }