From 58ef141840ea1f40136f1c104afe27c443f70550 Mon Sep 17 00:00:00 2001 From: "Gary D. Gregory" Date: Sat, 1 Feb 2025 09:10:52 -0500 Subject: [PATCH] [IO-867] Fix ThresholdingOutputStream#isThresholdExceeded Add more assertions for the subclass DeferredFileOutputStream --- .../output/DeferredFileOutputStreamTest.java | 237 ++++++++++-------- .../output/ThresholdingOutputStreamTest.java | 22 +- 2 files changed, 148 insertions(+), 111 deletions(-) diff --git a/src/test/java/org/apache/commons/io/output/DeferredFileOutputStreamTest.java b/src/test/java/org/apache/commons/io/output/DeferredFileOutputStreamTest.java index d89bd1a03b6..a7ba793f6c0 100644 --- a/src/test/java/org/apache/commons/io/output/DeferredFileOutputStreamTest.java +++ b/src/test/java/org/apache/commons/io/output/DeferredFileOutputStreamTest.java @@ -16,6 +16,7 @@ */ package org.apache.commons.io.output; +import static org.apache.commons.io.output.ThresholdingOutputStreamTest.assertThresholdingInitialState; import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; @@ -45,6 +46,10 @@ */ public class DeferredFileOutputStreamTest extends AbstractTempDirTest { + private static void assertDeferredInitialState(final DeferredFileOutputStream out) { + assertTrue(out.isInMemory()); + } + public static IntStream data() { return IntStream.of(1, 2, 4, 8, 16, 32, 64, 128, 256, 512, 1024, 2048, 4096); } @@ -62,21 +67,27 @@ public static IntStream data() { /** * Tests the case where the amount of data exceeds the threshold, and is therefore written to disk. The actual data * written to disk is verified, as is the file itself. + * + * @param initialBufferSize the initial buffer size. + * @throws IOException on a test failure. */ @ParameterizedTest(name = "initialBufferSize = {0}") @MethodSource("data") public void testAboveThreshold(final int initialBufferSize) throws IOException { final File testFile = Files.createTempFile(tempDirPath, "testAboveThreshold", "dat").toFile(); - try (DeferredFileOutputStream dfos = DeferredFileOutputStream.builder() - .setThreshold(testBytes.length - 5) + final int threshold = testBytes.length - 5; + try (DeferredFileOutputStream out = DeferredFileOutputStream.builder() + .setThreshold(threshold) .setBufferSize(initialBufferSize) .setOutputFile(testFile) .get()) { - dfos.write(testBytes, 0, testBytes.length); - dfos.close(); - assertFalse(dfos.isInMemory()); - assertNull(dfos.getData()); - assertEquals(testFile.length(), dfos.getByteCount()); + assertThresholdingInitialState(out, threshold, 0); + assertDeferredInitialState(out); + out.write(testBytes, 0, testBytes.length); + out.close(); + assertFalse(out.isInMemory()); + assertNull(out.getData()); + assertEquals(testFile.length(), out.getByteCount()); verifyResultFile(testFile); } } @@ -90,16 +101,19 @@ public void testAboveThreshold(final int initialBufferSize) throws IOException { @MethodSource("data") public void testAboveThresholdGetInputStream(final int initialBufferSize, final @TempDir Path tempDir) throws IOException { final File testFile = Files.createTempFile(tempDirPath, "testAboveThreshold", "dat").toFile(); - try (DeferredFileOutputStream dfos = DeferredFileOutputStream.builder() - .setThreshold(testBytes.length - 5) + final int threshold = testBytes.length - 5; + try (DeferredFileOutputStream out = DeferredFileOutputStream.builder() + .setThreshold(threshold) .setBufferSize(initialBufferSize) .setOutputFile(testFile) .get()) { - dfos.write(testBytes, 0, testBytes.length); - dfos.close(); - assertFalse(dfos.isInMemory()); - assertEquals(testFile.length(), dfos.getByteCount()); - try (InputStream is = dfos.toInputStream()) { + assertThresholdingInitialState(out, threshold, 0); + assertDeferredInitialState(out); + out.write(testBytes, 0, testBytes.length); + out.close(); + assertFalse(out.isInMemory()); + assertEquals(testFile.length(), out.getByteCount()); + try (InputStream is = out.toInputStream()) { assertArrayEquals(testBytes, IOUtils.toByteArray(is)); } verifyResultFile(testFile); @@ -113,17 +127,20 @@ public void testAboveThresholdGetInputStream(final int initialBufferSize, final @ParameterizedTest(name = "initialBufferSize = {0}") @MethodSource("data") public void testAtThreshold(final int initialBufferSize) throws IOException { - try (DeferredFileOutputStream dfos = DeferredFileOutputStream.builder() + final int threshold = testBytes.length; + try (DeferredFileOutputStream out = DeferredFileOutputStream.builder() // @formatter:off - .setThreshold(testBytes.length) + .setThreshold(threshold) .setBufferSize(initialBufferSize) .get()) { // @formatter:on - dfos.write(testBytes, 0, testBytes.length); - dfos.close(); - assertTrue(dfos.isInMemory()); - assertEquals(testBytes.length, dfos.getByteCount()); - final byte[] resultBytes = dfos.getData(); + assertThresholdingInitialState(out, threshold, 0); + assertDeferredInitialState(out); + out.write(testBytes, 0, testBytes.length); + out.close(); + assertTrue(out.isInMemory()); + assertEquals(testBytes.length, out.getByteCount()); + final byte[] resultBytes = out.getData(); assertEquals(testBytes.length, resultBytes.length); assertArrayEquals(resultBytes, testBytes); } @@ -136,17 +153,20 @@ public void testAtThreshold(final int initialBufferSize) throws IOException { @ParameterizedTest(name = "initialBufferSize = {0}") @MethodSource("data") public void testBelowThreshold(final int initialBufferSize) throws IOException { - try (DeferredFileOutputStream dfos = DeferredFileOutputStream.builder() + final int threshold = testBytes.length + 42; + try (DeferredFileOutputStream out = DeferredFileOutputStream.builder() // @formatter:off - .setThreshold(testBytes.length + 42) + .setThreshold(threshold) .setBufferSize(initialBufferSize) .get()) { // @formatter:on - dfos.write(testBytes, 0, testBytes.length); - dfos.close(); - assertTrue(dfos.isInMemory()); - assertEquals(testBytes.length, dfos.getByteCount()); - final byte[] resultBytes = dfos.getData(); + assertThresholdingInitialState(out, threshold, 0); + assertDeferredInitialState(out); + out.write(testBytes, 0, testBytes.length); + out.close(); + assertTrue(out.isInMemory()); + assertEquals(testBytes.length, out.getByteCount()); + final byte[] resultBytes = out.getData(); assertEquals(testBytes.length, resultBytes.length); assertArrayEquals(resultBytes, testBytes); } @@ -160,16 +180,19 @@ public void testBelowThreshold(final int initialBufferSize) throws IOException { @MethodSource("data") public void testBelowThresholdGetInputStream(final int initialBufferSize) throws IOException { // @formatter:off - try (DeferredFileOutputStream dfos = DeferredFileOutputStream.builder() - .setThreshold(testBytes.length + 42) + final int threshold = testBytes.length + 42; + try (DeferredFileOutputStream out = DeferredFileOutputStream.builder() + .setThreshold(threshold) .setBufferSize(initialBufferSize) .get()) { // @formatter:on - dfos.write(testBytes, 0, testBytes.length); - dfos.close(); - assertTrue(dfos.isInMemory()); - assertEquals(testBytes.length, dfos.getByteCount()); - try (InputStream is = dfos.toInputStream()) { + assertThresholdingInitialState(out, threshold, 0); + assertDeferredInitialState(out); + out.write(testBytes, 0, testBytes.length); + out.close(); + assertTrue(out.isInMemory()); + assertEquals(testBytes.length, out.getByteCount()); + try (InputStream is = out.toInputStream()) { assertArrayEquals(testBytes, IOUtils.toByteArray(is)); } } @@ -184,8 +207,9 @@ public void testTempFileAboveThreshold(final int initialBufferSize) throws IOExc final String prefix = "commons-io-test"; final String suffix = ".out"; // @formatter:off - try (DeferredFileOutputStream dfos = DeferredFileOutputStream.builder() - .setThreshold(testBytes.length - 5) + final int threshold = testBytes.length - 5; + try (DeferredFileOutputStream out = DeferredFileOutputStream.builder() + .setThreshold(threshold) .setBufferSize(initialBufferSize) .setPrefix(prefix) .setSuffix(suffix) @@ -193,19 +217,21 @@ public void testTempFileAboveThreshold(final int initialBufferSize) throws IOExc .setDirectory(tempDirPath.toFile()) .get()) { // @formatter:on - assertNull(dfos.getFile(), "Check File is null-A"); - assertNull(dfos.getPath(), "Check Path is null-A"); - dfos.write(testBytes, 0, testBytes.length); - dfos.close(); - assertFalse(dfos.isInMemory()); - assertEquals(testBytes.length, dfos.getByteCount()); - assertNull(dfos.getData()); - assertNotNull(dfos.getFile(), "Check file not null"); - assertTrue(dfos.getFile().exists(), "Check file exists"); - assertTrue(dfos.getFile().getName().startsWith(prefix), "Check prefix"); - assertTrue(dfos.getFile().getName().endsWith(suffix), "Check suffix"); - assertEquals(tempDirPath, dfos.getPath().getParent(), "Check dir"); - verifyResultFile(dfos.getFile()); + assertThresholdingInitialState(out, threshold, 0); + assertDeferredInitialState(out); + assertNull(out.getFile(), "Check File is null-A"); + assertNull(out.getPath(), "Check Path is null-A"); + out.write(testBytes, 0, testBytes.length); + out.close(); + assertFalse(out.isInMemory()); + assertEquals(testBytes.length, out.getByteCount()); + assertNull(out.getData()); + assertNotNull(out.getFile(), "Check file not null"); + assertTrue(out.getFile().exists(), "Check file exists"); + assertTrue(out.getFile().getName().startsWith(prefix), "Check prefix"); + assertTrue(out.getFile().getName().endsWith(suffix), "Check suffix"); + assertEquals(tempDirPath, out.getPath().getParent(), "Check dir"); + verifyResultFile(out.getFile()); } } @@ -218,9 +244,10 @@ public void testTempFileAboveThreshold(final int initialBufferSize) throws IOExc public void testTempFileAboveThresholdPrefixOnly(final int initialBufferSize) throws IOException { final String prefix = "commons-io-test"; final String suffix = null; - try (final DeferredFileOutputStream dfos = DeferredFileOutputStream.builder() + final int threshold = testBytes.length - 5; + try (final DeferredFileOutputStream out = DeferredFileOutputStream.builder() // @formatter:off - .setThreshold(testBytes.length - 5) + .setThreshold(threshold) .setBufferSize(initialBufferSize) .setPrefix(prefix) .setSuffix(suffix) @@ -228,21 +255,23 @@ public void testTempFileAboveThresholdPrefixOnly(final int initialBufferSize) th .get()) { // @formatter:on try { - assertNull(dfos.getFile(), "Check File is null-A"); - assertNull(dfos.getPath(), "Check Path is null-A"); - dfos.write(testBytes, 0, testBytes.length); - dfos.close(); - assertFalse(dfos.isInMemory()); - assertNull(dfos.getData()); - assertEquals(testBytes.length, dfos.getByteCount()); - assertNotNull(dfos.getFile(), "Check file not null"); - assertTrue(dfos.getFile().exists(), "Check file exists"); - assertTrue(dfos.getFile().getName().startsWith(prefix), "Check prefix"); - assertTrue(dfos.getFile().getName().endsWith(".tmp"), "Check suffix"); // ".tmp" is default - verifyResultFile(dfos.getFile()); + assertThresholdingInitialState(out, threshold, 0); + assertDeferredInitialState(out); + assertNull(out.getFile(), "Check File is null-A"); + assertNull(out.getPath(), "Check Path is null-A"); + out.write(testBytes, 0, testBytes.length); + out.close(); + assertFalse(out.isInMemory()); + assertNull(out.getData()); + assertEquals(testBytes.length, out.getByteCount()); + assertNotNull(out.getFile(), "Check file not null"); + assertTrue(out.getFile().exists(), "Check file exists"); + assertTrue(out.getFile().getName().startsWith(prefix), "Check prefix"); + assertTrue(out.getFile().getName().endsWith(".tmp"), "Check suffix"); // ".tmp" is default + verifyResultFile(out.getFile()); } finally { // Delete the temporary file. - dfos.getFile().delete(); + out.getFile().delete(); } } } @@ -256,14 +285,17 @@ public void testTempFileAboveThresholdPrefixOnly(final int initialBufferSize) th public void testTempFileBelowThreshold(final int initialBufferSize) throws IOException { final String prefix = "commons-io-test"; final String suffix = ".out"; - try (final DeferredFileOutputStream dfos = new DeferredFileOutputStream(testBytes.length + 42, initialBufferSize, prefix, suffix, tempDirFile)) { - assertNull(dfos.getFile(), "Check File is null-A"); - assertNull(dfos.getPath(), "Check Path is null-A"); - dfos.write(testBytes, 0, testBytes.length); - dfos.close(); - assertTrue(dfos.isInMemory()); - assertEquals(testBytes.length, dfos.getByteCount()); - assertNull(dfos.getFile(), "Check file is null-B"); + final int threshold = testBytes.length + 42; + try (final DeferredFileOutputStream out = new DeferredFileOutputStream(threshold, initialBufferSize, prefix, suffix, tempDirFile)) { + assertThresholdingInitialState(out, threshold, 0); + assertDeferredInitialState(out); + assertNull(out.getFile(), "Check File is null-A"); + assertNull(out.getPath(), "Check Path is null-A"); + out.write(testBytes, 0, testBytes.length); + out.close(); + assertTrue(out.isInMemory()); + assertEquals(testBytes.length, out.getByteCount()); + assertNull(out.getFile(), "Check file is null-B"); } } @@ -287,16 +319,18 @@ public void testTempFileError() throws Exception { @MethodSource("data") public void testThresholdNegative(final int initialBufferSize) throws IOException { final File testFile = Files.createTempFile(tempDirPath, "testThresholdNegative", "dat").toFile(); - try (DeferredFileOutputStream dfos = DeferredFileOutputStream.builder() + try (DeferredFileOutputStream out = DeferredFileOutputStream.builder() .setThreshold(-1) .setBufferSize(initialBufferSize) .setOutputFile(testFile) .get()) { - dfos.write(testBytes, 0, testBytes.length); - dfos.close(); - assertFalse(dfos.isInMemory()); - assertNull(dfos.getData()); - assertEquals(testFile.length(), dfos.getByteCount()); + assertThresholdingInitialState(out, 0, 0); + assertDeferredInitialState(out); + out.write(testBytes, 0, testBytes.length); + out.close(); + assertFalse(out.isInMemory()); + assertNull(out.getData()); + assertEquals(testFile.length(), out.getByteCount()); verifyResultFile(testFile); } } @@ -310,21 +344,24 @@ public void testThresholdNegative(final int initialBufferSize) throws IOExceptio @MethodSource("data") public void testThresholdReached(final int initialBufferSize) throws IOException { final File testFile = Files.createTempFile(tempDirPath, "testThresholdReached", "dat").toFile(); - try (final DeferredFileOutputStream dfos = DeferredFileOutputStream.builder() + final int threshold = testBytes.length /2; + try (final DeferredFileOutputStream out = DeferredFileOutputStream.builder() // @formatter:off - .setThreshold(testBytes.length /2) + .setThreshold(threshold) .setBufferSize(initialBufferSize) .setOutputFile(testFile) .get()) { // @formatter:on + assertThresholdingInitialState(out, threshold, 0); + assertDeferredInitialState(out); final int chunkSize = testBytes.length / 3; - dfos.write(testBytes, 0, chunkSize); - dfos.write(testBytes, chunkSize, chunkSize); - dfos.write(testBytes, chunkSize * 2, testBytes.length - chunkSize * 2); - dfos.close(); - assertFalse(dfos.isInMemory()); - assertNull(dfos.getData()); - assertEquals(testBytes.length, dfos.getByteCount()); + out.write(testBytes, 0, chunkSize); + out.write(testBytes, chunkSize, chunkSize); + out.write(testBytes, chunkSize * 2, testBytes.length - chunkSize * 2); + out.close(); + assertFalse(out.isInMemory()); + assertNull(out.getData()); + assertEquals(testBytes.length, out.getByteCount()); verifyResultFile(testFile); } } @@ -336,16 +373,16 @@ public void testThresholdReached(final int initialBufferSize) throws IOException @MethodSource("data") public void testWriteToLarge(final int initialBufferSize) throws IOException { final File testFile = Files.createTempFile(tempDirPath, "testWriteToFile", "dat").toFile(); + final int threshold = testBytes.length / 2; try (ByteArrayOutputStream baos = new ByteArrayOutputStream(initialBufferSize); - DeferredFileOutputStream dfos = DeferredFileOutputStream.builder().setThreshold(testBytes.length / 2).setOutputFile(testFile).get()) { + DeferredFileOutputStream dfos = DeferredFileOutputStream.builder().setThreshold(threshold).setOutputFile(testFile).get()) { + assertThresholdingInitialState(dfos, threshold, 0); + assertDeferredInitialState(dfos); dfos.write(testBytes); - assertTrue(testFile.exists()); assertFalse(dfos.isInMemory()); assertEquals(testBytes.length, dfos.getByteCount()); - assertThrows(IOException.class, () -> dfos.writeTo(baos)); - dfos.close(); dfos.writeTo(baos); final byte[] copiedBytes = baos.toByteArray(); @@ -361,16 +398,16 @@ public void testWriteToLarge(final int initialBufferSize) throws IOException { @MethodSource("data") public void testWriteToLargeCtor(final int initialBufferSize) throws IOException { final File testFile = Files.createTempFile(tempDirPath, "testWriteToFile", "dat").toFile(); + final int threshold = testBytes.length / 2; try (ByteArrayOutputStream baos = new ByteArrayOutputStream(initialBufferSize); - DeferredFileOutputStream dfos = new DeferredFileOutputStream(testBytes.length / 2, testFile)) { + DeferredFileOutputStream dfos = new DeferredFileOutputStream(threshold, testFile)) { + assertThresholdingInitialState(dfos, threshold, 0); + assertDeferredInitialState(dfos); dfos.write(testBytes); - assertTrue(testFile.exists()); assertFalse(dfos.isInMemory()); - assertThrows(IOException.class, () -> dfos.writeTo(baos)); assertEquals(testBytes.length, dfos.getByteCount()); - dfos.close(); dfos.writeTo(baos); final byte[] copiedBytes = baos.toByteArray(); @@ -387,15 +424,15 @@ public void testWriteToLargeCtor(final int initialBufferSize) throws IOException @MethodSource("data") public void testWriteToSmall(final int initialBufferSize) throws IOException { final File testFile = Files.createTempFile(tempDirPath, "testWriteToMem", "dat").toFile(); + final int threshold = testBytes.length * 2; try (ByteArrayOutputStream baos = new ByteArrayOutputStream(initialBufferSize); - final DeferredFileOutputStream dfos = new DeferredFileOutputStream(testBytes.length * 2, initialBufferSize, testFile)) { + final DeferredFileOutputStream dfos = new DeferredFileOutputStream(threshold, initialBufferSize, testFile)) { + assertThresholdingInitialState(dfos, threshold, 0); + assertDeferredInitialState(dfos); dfos.write(testBytes); - assertTrue(dfos.isInMemory()); - assertThrows(IOException.class, () -> dfos.writeTo(baos)); assertEquals(testBytes.length, dfos.getByteCount()); - dfos.close(); dfos.writeTo(baos); final byte[] copiedBytes = baos.toByteArray(); diff --git a/src/test/java/org/apache/commons/io/output/ThresholdingOutputStreamTest.java b/src/test/java/org/apache/commons/io/output/ThresholdingOutputStreamTest.java index 4f205cc507d..fd0c415a5a3 100644 --- a/src/test/java/org/apache/commons/io/output/ThresholdingOutputStreamTest.java +++ b/src/test/java/org/apache/commons/io/output/ThresholdingOutputStreamTest.java @@ -42,7 +42,7 @@ public class ThresholdingOutputStreamTest { * @param expectedThreshold the expected threshold. * @param expectedByeCount the expected byte count. */ - private static void assertInitialState(final ThresholdingOutputStream out, final int expectedThreshold, final int expectedByeCount) { + static void assertThresholdingInitialState(final ThresholdingOutputStream out, final int expectedThreshold, final int expectedByeCount) { assertFalse(out.isThresholdExceeded()); assertEquals(expectedThreshold, out.getThreshold()); assertEquals(expectedByeCount, out.getByteCount()); @@ -68,7 +68,7 @@ protected void thresholdReached() throws IOException { reached.set(true); } }) { - assertInitialState(out, threshold, initCount); + assertThresholdingInitialState(out, threshold, initCount); out.write('a'); assertFalse(reached.get()); out.write('a'); @@ -96,7 +96,7 @@ protected void thresholdReached() throws IOException { reached.set(true); } }) { - assertInitialState(out, threshold, initCount); + assertThresholdingInitialState(out, threshold, initCount); out.write('a'); assertFalse(reached.get()); out.write('a'); @@ -112,7 +112,7 @@ public void testThresholdIOConsumer() throws Exception { final int threshold = 1; try (ThresholdingOutputStream out = new ThresholdingOutputStream(threshold, null, os -> new ByteArrayOutputStream(4))) { - assertInitialState(out, threshold, 0); + assertThresholdingInitialState(out, threshold, 0); out.write('a'); assertFalse(reached.get()); out.write('a'); @@ -121,7 +121,7 @@ public void testThresholdIOConsumer() throws Exception { // Null output stream function reached.set(false); try (ThresholdingOutputStream out = new ThresholdingOutputStream(threshold, os -> reached.set(true), null)) { - assertInitialState(out, threshold, 0); + assertThresholdingInitialState(out, threshold, 0); out.write('a'); assertFalse(reached.get()); out.write('a'); @@ -131,7 +131,7 @@ public void testThresholdIOConsumer() throws Exception { reached.set(false); try (ThresholdingOutputStream out = new ThresholdingOutputStream(threshold, os -> reached.set(true), os -> new ByteArrayOutputStream(4))) { - assertInitialState(out, threshold, 0); + assertThresholdingInitialState(out, threshold, 0); out.write('a'); assertFalse(reached.get()); out.write('a'); @@ -145,7 +145,7 @@ public void testThresholdIOConsumerIOException() throws Exception { try (ThresholdingOutputStream out = new ThresholdingOutputStream(threshold, os -> { throw new IOException("Threshold reached."); }, os -> new ByteArrayOutputStream(4))) { - assertInitialState(out, threshold, 0); + assertThresholdingInitialState(out, threshold, 0); out.write('a'); assertThrows(IOException.class, () -> out.write('a')); } @@ -157,7 +157,7 @@ public void testThresholdIOConsumerUncheckedException() throws Exception { try (ThresholdingOutputStream out = new ThresholdingOutputStream(threshold, os -> { throw new IllegalStateException("Threshold reached."); }, os -> new ByteArrayOutputStream(4))) { - assertInitialState(out, threshold, 0); + assertThresholdingInitialState(out, threshold, 0); out.write('a'); assertThrows(IllegalStateException.class, () -> out.write('a')); } @@ -176,7 +176,7 @@ protected void thresholdReached() throws IOException { reached.set(true); } }) { - assertInitialState(out, 0, 0); + assertThresholdingInitialState(out, 0, 0); assertFalse(reached.get()); out.write(89); assertTrue(reached.get()); @@ -194,7 +194,7 @@ protected void thresholdReached() throws IOException { reached.set(true); } }) { - assertInitialState(out, threshold, 0); + assertThresholdingInitialState(out, threshold, 0); out.write(89); assertTrue(reached.get()); assertTrue(out.isThresholdExceeded()); @@ -215,7 +215,7 @@ protected void thresholdReached() throws IOException { reached.set(true); } }) { - assertInitialState(out, threshold, 0); + assertThresholdingInitialState(out, threshold, 0); assertFalse(reached.get()); out.write(new byte[0]); assertFalse(out.isThresholdExceeded());