You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by gg...@apache.org on 2024/07/08 19:38:16 UTC

(commons-io) branch master updated (c553b0b40 -> 99673666d)

This is an automated email from the ASF dual-hosted git repository.

ggregory pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/commons-io.git


    from c553b0b40 Add MarkShieldInputStream tests
     new 86b5da5ca Our convention is public tests.
     new fae5ae925 Normalize internal method name
     new 99673666d MemoryMappedFileInputStream.available() should return 0 after the stream is closed

The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 src/changes/changes.xml                            |   2 +
 .../io/input/MemoryMappedFileInputStream.java      |  23 +++--
 .../io/input/MemoryMappedFileInputStreamTest.java  | 115 +++++++++++++--------
 3 files changed, 88 insertions(+), 52 deletions(-)


(commons-io) 01/03: Our convention is public tests.

Posted by gg...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

ggregory pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-io.git

commit 86b5da5ca37345b624066e878ed5f9e2f1f26bea
Author: Gary Gregory <ga...@gmail.com>
AuthorDate: Mon Jul 8 14:35:44 2024 -0400

    Our convention is public tests.
---
 .../io/input/MemoryMappedFileInputStreamTest.java  | 34 +++++++++++-----------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/src/test/java/org/apache/commons/io/input/MemoryMappedFileInputStreamTest.java b/src/test/java/org/apache/commons/io/input/MemoryMappedFileInputStreamTest.java
index a26b46a87..a6f5c9953 100644
--- a/src/test/java/org/apache/commons/io/input/MemoryMappedFileInputStreamTest.java
+++ b/src/test/java/org/apache/commons/io/input/MemoryMappedFileInputStreamTest.java
@@ -44,7 +44,7 @@ public class MemoryMappedFileInputStreamTest {
     Path tempDir;
 
     @AfterEach
-    void afterEach() {
+    public void afterEach() {
         // Ask to run the garbage collector to clean up memory mapped buffers,
         // otherwise the temporary files won't be able to be removed when running on
         // Windows. Calling gc() is just a hint to the VM.
@@ -71,7 +71,7 @@ public class MemoryMappedFileInputStreamTest {
     }
 
     @Test
-    void testAlternateBufferSize() throws IOException {
+    public void testAlternateBufferSize() throws IOException {
         // setup
         final Path file = createTestFile(1024 * 1024);
         final byte[] expectedData = Files.readAllBytes(file);
@@ -84,7 +84,7 @@ public class MemoryMappedFileInputStreamTest {
     }
 
     @Test
-    void testEmptyFile() throws IOException {
+    public void testEmptyFile() throws IOException {
         // setup
         final Path file = createTestFile(0);
         // test
@@ -95,7 +95,7 @@ public class MemoryMappedFileInputStreamTest {
     }
 
     @Test
-    void testLargerFile() throws IOException {
+    public void testLargerFile() throws IOException {
         // setup
         final Path file = createTestFile(1024 * 1024);
         final byte[] expectedData = Files.readAllBytes(file);
@@ -108,7 +108,7 @@ public class MemoryMappedFileInputStreamTest {
     }
 
     @Test
-    void testReadAfterClose() throws IOException {
+    public void testReadAfterClose() throws IOException {
         // setup
         final Path file = createTestFile(1 * 1024 * 1024);
 
@@ -121,7 +121,7 @@ public class MemoryMappedFileInputStreamTest {
     }
 
     @Test
-    void testReadSingleByte() throws IOException {
+    public void testReadSingleByte() throws IOException {
         // setup
         final Path file = createTestFile(2);
         final byte[] expectedData = Files.readAllBytes(file);
@@ -136,7 +136,7 @@ public class MemoryMappedFileInputStreamTest {
     }
 
     @Test
-    void testSkipAtStart() throws IOException {
+    public void testSkipAtStart() throws IOException {
         // setup
         final Path file = createTestFile(100);
         final byte[] expectedData = Files.readAllBytes(file);
@@ -151,7 +151,7 @@ public class MemoryMappedFileInputStreamTest {
     }
 
     @Test
-    void testSkipEmpty() throws IOException {
+    public void testSkipEmpty() throws IOException {
         // setup
         final Path file = createTestFile(0);
         // test
@@ -163,7 +163,7 @@ public class MemoryMappedFileInputStreamTest {
     }
 
     @Test
-    void testSkipInCurrentBuffer() throws IOException {
+    public void testSkipInCurrentBuffer() throws IOException {
         // setup
         final Path file = createTestFile(100);
         final byte[] expectedData = Files.readAllBytes(file);
@@ -180,7 +180,7 @@ public class MemoryMappedFileInputStreamTest {
 
     @ParameterizedTest
     @ValueSource(ints = {-5, -1, 0})
-    void testSkipNoop(final int amountToSkip) throws IOException {
+    public void testSkipNoop(final int amountToSkip) throws IOException {
         // setup
         final Path file = createTestFile(10);
         final byte[] expectedData = Files.readAllBytes(file);
@@ -193,7 +193,7 @@ public class MemoryMappedFileInputStreamTest {
     }
 
     @Test
-    void testSkipOutOfCurrentBuffer() throws IOException {
+    public void testSkipOutOfCurrentBuffer() throws IOException {
         // setup
         final Path file = createTestFile(100);
         final byte[] expectedData = Files.readAllBytes(file);
@@ -209,7 +209,7 @@ public class MemoryMappedFileInputStreamTest {
     }
 
     @Test
-    void testSkipPastEof() throws IOException {
+    public void testSkipPastEof() throws IOException {
         // setup
         final Path file = createTestFile(100);
 
@@ -223,7 +223,7 @@ public class MemoryMappedFileInputStreamTest {
     }
 
     @Test
-    void testSkipToEndOfCurrentBuffer() throws IOException {
+    public void testSkipToEndOfCurrentBuffer() throws IOException {
         // setup
         final Path file = createTestFile(100);
         final byte[] expectedData = Files.readAllBytes(file);
@@ -239,7 +239,7 @@ public class MemoryMappedFileInputStreamTest {
     }
 
     @Test
-    void testSkipToEndOfCurrentBufferBuilder() throws IOException {
+    public void testSkipToEndOfCurrentBufferBuilder() throws IOException {
         // setup
         final Path file = createTestFile(100);
         final byte[] expectedData = Files.readAllBytes(file);
@@ -256,7 +256,7 @@ public class MemoryMappedFileInputStreamTest {
     }
 
     @Test
-    void testSmallFileBuilder() throws IOException {
+    public void testSmallFileBuilder() throws IOException {
         // setup
         final Path file = createTestFile(100);
         final byte[] expectedData = Files.readAllBytes(file);
@@ -269,7 +269,7 @@ public class MemoryMappedFileInputStreamTest {
     }
 
     @Test
-    void testSmallPath() throws IOException {
+    public void testSmallPath() throws IOException {
         // setup
         final Path file = createTestFile(100);
         final byte[] expectedData = Files.readAllBytes(file);
@@ -282,7 +282,7 @@ public class MemoryMappedFileInputStreamTest {
     }
 
     @Test
-    void testSmallPathBuilder() throws IOException {
+    public void testSmallPathBuilder() throws IOException {
         // setup
         final Path file = createTestFile(100);
         final byte[] expectedData = Files.readAllBytes(file);


(commons-io) 03/03: MemoryMappedFileInputStream.available() should return 0 after the stream is closed

Posted by gg...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

ggregory pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-io.git

commit 99673666d9defb2c64df9bdcf6367710670ebdf7
Author: Gary Gregory <ga...@gmail.com>
AuthorDate: Mon Jul 8 15:37:34 2024 -0400

    MemoryMappedFileInputStream.available() should return 0 after the stream
    is closed
    
    MemoryMappedFileInputStream.read() should return -1 after the stream is
    closed
---
 src/changes/changes.xml                            |  2 +
 .../io/input/MemoryMappedFileInputStream.java      |  7 +-
 .../io/input/MemoryMappedFileInputStreamTest.java  | 81 +++++++++++++++-------
 3 files changed, 63 insertions(+), 27 deletions(-)

diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 664dec549..cfc22cf10 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -75,6 +75,8 @@ The <action> type attribute can be add,update,fix,remove.
       <action dev="ggregory" type="fix"                due-to="Gary Gregory">Deprecate NullInputStream.INSTANCE in favor of constructors.</action>
       <action dev="ggregory" type="fix"                due-to="Gary Gregory">NullInputStream.available() should return 0 after the stream is closed.</action>
       <action dev="ggregory" type="fix"                due-to="Gary Gregory">NullInputStream.read() should return -1 after the stream is closed.</action>
+      <action dev="ggregory" type="fix"                due-to="Gary Gregory">MemoryMappedFileInputStream.available() should return 0 after the stream is closed.</action>
+      <action dev="ggregory" type="fix"                due-to="Gary Gregory">MemoryMappedFileInputStream.read() should return -1 after the stream is closed.</action>
       <!-- UPDATE -->
       <action dev="ggregory" type="update"             due-to="Dependabot">Bump tests commons.bytebuddy.version from 1.14.13 to 1.14.17 #615, #621, #631, #635.</action>
       <action dev="ggregory" type="update"             due-to="Dependabot">Bump tests commons-codec:commons-codec from 1.16.1 to 1.17.0.</action>
diff --git a/src/main/java/org/apache/commons/io/input/MemoryMappedFileInputStream.java b/src/main/java/org/apache/commons/io/input/MemoryMappedFileInputStream.java
index e5d8a3e6a..248f94510 100644
--- a/src/main/java/org/apache/commons/io/input/MemoryMappedFileInputStream.java
+++ b/src/main/java/org/apache/commons/io/input/MemoryMappedFileInputStream.java
@@ -167,6 +167,7 @@ public final class MemoryMappedFileInputStream extends InputStream {
 
     @Override
     public int available() throws IOException {
+        //return buffer != null ? buffer.remaining(): 0;
         return buffer.remaining();
     }
 
@@ -186,7 +187,7 @@ public final class MemoryMappedFileInputStream extends InputStream {
     public void close() throws IOException {
         if (!closed) {
             cleanBuffer();
-            buffer = null;
+            buffer = EMPTY_BUFFER;
             channel.close();
             closed = true;
         }
@@ -210,7 +211,9 @@ public final class MemoryMappedFileInputStream extends InputStream {
 
     @Override
     public int read() throws IOException {
-        checkOpen();
+        if (closed) {
+            return EOF;
+        }
         if (!buffer.hasRemaining()) {
             nextBuffer();
             if (!buffer.hasRemaining()) {
diff --git a/src/test/java/org/apache/commons/io/input/MemoryMappedFileInputStreamTest.java b/src/test/java/org/apache/commons/io/input/MemoryMappedFileInputStreamTest.java
index a6f5c9953..690c56c7f 100644
--- a/src/test/java/org/apache/commons/io/input/MemoryMappedFileInputStreamTest.java
+++ b/src/test/java/org/apache/commons/io/input/MemoryMappedFileInputStreamTest.java
@@ -33,6 +33,7 @@ import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.io.TempDir;
 import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.MethodSource;
 import org.junit.jupiter.params.provider.ValueSource;
 
 /**
@@ -62,11 +63,11 @@ public class MemoryMappedFileInputStreamTest {
         return Files.write(Files.createTempFile(tempDir, null, null), RandomUtils.nextBytes(size));
     }
 
-    private MemoryMappedFileInputStream newStream(final Path file) throws IOException {
+    private MemoryMappedFileInputStream newInputStream(final Path file) throws IOException {
         return MemoryMappedFileInputStream.builder().setPath(file).get();
     }
 
-    private MemoryMappedFileInputStream newStream(final Path file, final int bufferSize) throws IOException {
+    private MemoryMappedFileInputStream newInputStream(final Path file, final int bufferSize) throws IOException {
         return MemoryMappedFileInputStream.builder().setPath(file).setBufferSize(bufferSize).get();
     }
 
@@ -75,20 +76,47 @@ public class MemoryMappedFileInputStreamTest {
         // setup
         final Path file = createTestFile(1024 * 1024);
         final byte[] expectedData = Files.readAllBytes(file);
-
         // test
-        try (InputStream inputStream = newStream(file, 1024)) {
+        try (InputStream inputStream = newInputStream(file, 1024)) {
             // verify
             assertArrayEquals(expectedData, IOUtils.toByteArray(inputStream));
         }
     }
 
+    @SuppressWarnings("resource")
+    @ParameterizedTest
+    @MethodSource(AbstractInputStreamTest.ARRAY_LENGTHS_NAME)
+    public void testAvailableAfterClose(final int len) throws Exception {
+        final Path file = createTestFile(len);
+        final InputStream shadow;
+        try (InputStream inputStream = newInputStream(file, 1024)) {
+            // verify
+            assertEquals(0, inputStream.available());
+            shadow = inputStream;
+        }
+        assertEquals(0, shadow.available());
+    }
+
+    @ParameterizedTest
+    @MethodSource(AbstractInputStreamTest.ARRAY_LENGTHS_NAME)
+    public void testAvailableAfterOpen(final int len) throws Exception {
+        final Path file = createTestFile(len);
+        try (InputStream inputStream = newInputStream(file, 1024)) {
+            // verify
+            assertEquals(0, inputStream.available());
+            inputStream.read();
+            assertEquals(Math.max(len - 1, 0), inputStream.available());
+            IOUtils.toByteArray(inputStream);
+            assertEquals(0, inputStream.available());
+        }
+    }
+
     @Test
     public void testEmptyFile() throws IOException {
         // setup
         final Path file = createTestFile(0);
         // test
-        try (InputStream inputStream = newStream(file)) {
+        try (InputStream inputStream = newInputStream(file)) {
             // verify
             assertArrayEquals(EMPTY_BYTE_ARRAY, IOUtils.toByteArray(inputStream));
         }
@@ -99,9 +127,8 @@ public class MemoryMappedFileInputStreamTest {
         // setup
         final Path file = createTestFile(1024 * 1024);
         final byte[] expectedData = Files.readAllBytes(file);
-
         // test
-        try (InputStream inputStream = newStream(file)) {
+        try (InputStream inputStream = newInputStream(file)) {
             // verify
             assertArrayEquals(expectedData, IOUtils.toByteArray(inputStream));
         }
@@ -111,22 +138,33 @@ public class MemoryMappedFileInputStreamTest {
     public void testReadAfterClose() throws IOException {
         // setup
         final Path file = createTestFile(1 * 1024 * 1024);
-
         // test
-        try (InputStream inputStream = newStream(file, 1024)) {
+        try (InputStream inputStream = newInputStream(file, 1024)) {
             inputStream.close();
             // verify
             Assertions.assertThrows(IOException.class, () -> IOUtils.toByteArray(inputStream));
         }
     }
 
+    @SuppressWarnings("resource")
+    @ParameterizedTest
+    @MethodSource(AbstractInputStreamTest.ARRAY_LENGTHS_NAME)
+    public void testReadAfterClose(final int len) throws Exception {
+        final Path file = createTestFile(len);
+        final InputStream shadow;
+        try (InputStream inputStream = newInputStream(file, 1024)) {
+            shadow = inputStream;
+        }
+        assertEquals(IOUtils.EOF, shadow.read());
+    }
+
     @Test
     public void testReadSingleByte() throws IOException {
         // setup
         final Path file = createTestFile(2);
         final byte[] expectedData = Files.readAllBytes(file);
         // test
-        try (InputStream inputStream = newStream(file, 1024)) {
+        try (InputStream inputStream = newInputStream(file, 1024)) {
             final int b1 = inputStream.read();
             final int b2 = inputStream.read();
             assertEquals(-1, inputStream.read());
@@ -140,9 +178,8 @@ public class MemoryMappedFileInputStreamTest {
         // setup
         final Path file = createTestFile(100);
         final byte[] expectedData = Files.readAllBytes(file);
-
         // test
-        try (InputStream inputStream = newStream(file, 10)) {
+        try (InputStream inputStream = newInputStream(file, 10)) {
             assertEquals(1, inputStream.skip(1));
             final byte[] data = IOUtils.toByteArray(inputStream);
             // verify
@@ -155,7 +192,7 @@ public class MemoryMappedFileInputStreamTest {
         // setup
         final Path file = createTestFile(0);
         // test
-        try (InputStream inputStream = newStream(file)) {
+        try (InputStream inputStream = newInputStream(file)) {
             assertEquals(0, inputStream.skip(5));
             // verify
             assertArrayEquals(EMPTY_BYTE_ARRAY, IOUtils.toByteArray(inputStream));
@@ -167,9 +204,8 @@ public class MemoryMappedFileInputStreamTest {
         // setup
         final Path file = createTestFile(100);
         final byte[] expectedData = Files.readAllBytes(file);
-
         // test
-        try (InputStream inputStream = newStream(file, 10)) {
+        try (InputStream inputStream = newInputStream(file, 10)) {
             IOUtils.toByteArray(inputStream, 5);
             assertEquals(3, inputStream.skip(3));
             final byte[] data = IOUtils.toByteArray(inputStream);
@@ -185,7 +221,7 @@ public class MemoryMappedFileInputStreamTest {
         final Path file = createTestFile(10);
         final byte[] expectedData = Files.readAllBytes(file);
         // test
-        try (InputStream inputStream = newStream(file)) {
+        try (InputStream inputStream = newInputStream(file)) {
             assertEquals(0, inputStream.skip(amountToSkip));
             // verify
             assertArrayEquals(expectedData, IOUtils.toByteArray(inputStream));
@@ -197,9 +233,8 @@ public class MemoryMappedFileInputStreamTest {
         // setup
         final Path file = createTestFile(100);
         final byte[] expectedData = Files.readAllBytes(file);
-
         // test
-        try (InputStream inputStream = newStream(file, 10)) {
+        try (InputStream inputStream = newInputStream(file, 10)) {
             IOUtils.toByteArray(inputStream, 5);
             assertEquals(6, inputStream.skip(6));
             final byte[] data = IOUtils.toByteArray(inputStream);
@@ -212,9 +247,8 @@ public class MemoryMappedFileInputStreamTest {
     public void testSkipPastEof() throws IOException {
         // setup
         final Path file = createTestFile(100);
-
         // test
-        try (InputStream inputStream = newStream(file, 10)) {
+        try (InputStream inputStream = newInputStream(file, 10)) {
             IOUtils.toByteArray(inputStream, 5);
             assertEquals(95, inputStream.skip(96));
             // verify
@@ -229,7 +263,7 @@ public class MemoryMappedFileInputStreamTest {
         final byte[] expectedData = Files.readAllBytes(file);
 
         // test
-        try (InputStream inputStream = newStream(file, 10)) {
+        try (InputStream inputStream = newInputStream(file, 10)) {
             IOUtils.toByteArray(inputStream, 5);
             assertEquals(5, inputStream.skip(5));
             final byte[] data = IOUtils.toByteArray(inputStream);
@@ -243,7 +277,6 @@ public class MemoryMappedFileInputStreamTest {
         // setup
         final Path file = createTestFile(100);
         final byte[] expectedData = Files.readAllBytes(file);
-
         // test
         try (MemoryMappedFileInputStream inputStream = MemoryMappedFileInputStream.builder().setPath(file).setBufferSize(10).get()) {
             assertEquals(10, inputStream.getBufferSize());
@@ -260,7 +293,6 @@ public class MemoryMappedFileInputStreamTest {
         // setup
         final Path file = createTestFile(100);
         final byte[] expectedData = Files.readAllBytes(file);
-
         // test
         try (InputStream inputStream = MemoryMappedFileInputStream.builder().setFile(file.toFile()).get()) {
             // verify
@@ -275,7 +307,7 @@ public class MemoryMappedFileInputStreamTest {
         final byte[] expectedData = Files.readAllBytes(file);
 
         // test
-        try (InputStream inputStream = newStream(file)) {
+        try (InputStream inputStream = newInputStream(file)) {
             // verify
             assertArrayEquals(expectedData, IOUtils.toByteArray(inputStream));
         }
@@ -286,7 +318,6 @@ public class MemoryMappedFileInputStreamTest {
         // setup
         final Path file = createTestFile(100);
         final byte[] expectedData = Files.readAllBytes(file);
-
         // test
         try (InputStream inputStream = MemoryMappedFileInputStream.builder().setPath(file).get()) {
             // verify


(commons-io) 02/03: Normalize internal method name

Posted by gg...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

ggregory pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-io.git

commit fae5ae925458595a5248029554a42e1d2973d97f
Author: Gary Gregory <ga...@gmail.com>
AuthorDate: Mon Jul 8 14:39:54 2024 -0400

    Normalize internal method name
---
 .../commons/io/input/MemoryMappedFileInputStream.java  | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/src/main/java/org/apache/commons/io/input/MemoryMappedFileInputStream.java b/src/main/java/org/apache/commons/io/input/MemoryMappedFileInputStream.java
index 04c385ffd..e5d8a3e6a 100644
--- a/src/main/java/org/apache/commons/io/input/MemoryMappedFileInputStream.java
+++ b/src/main/java/org/apache/commons/io/input/MemoryMappedFileInputStream.java
@@ -170,6 +170,12 @@ public final class MemoryMappedFileInputStream extends InputStream {
         return buffer.remaining();
     }
 
+    private void checkOpen() throws IOException {
+        if (closed) {
+            throw new IOException("Stream closed");
+        }
+    }
+
     private void cleanBuffer() {
         if (ByteBufferCleaner.isSupported() && buffer.isDirect()) {
             ByteBufferCleaner.clean(buffer);
@@ -186,12 +192,6 @@ public final class MemoryMappedFileInputStream extends InputStream {
         }
     }
 
-    private void ensureOpen() throws IOException {
-        if (closed) {
-            throw new IOException("Stream closed");
-        }
-    }
-
     int getBufferSize() {
         return bufferSize;
     }
@@ -210,7 +210,7 @@ public final class MemoryMappedFileInputStream extends InputStream {
 
     @Override
     public int read() throws IOException {
-        ensureOpen();
+        checkOpen();
         if (!buffer.hasRemaining()) {
             nextBuffer();
             if (!buffer.hasRemaining()) {
@@ -222,7 +222,7 @@ public final class MemoryMappedFileInputStream extends InputStream {
 
     @Override
     public int read(final byte[] b, final int off, final int len) throws IOException {
-        ensureOpen();
+        checkOpen();
         if (!buffer.hasRemaining()) {
             nextBuffer();
             if (!buffer.hasRemaining()) {
@@ -236,7 +236,7 @@ public final class MemoryMappedFileInputStream extends InputStream {
 
     @Override
     public long skip(final long n) throws IOException {
-        ensureOpen();
+        checkOpen();
         if (n <= 0) {
             return 0;
         }