ThresholdingOutputStream: a negative threshold should behave like a zero threshold and trigger the event on the first write (#609)
* add failing test for negative threshold
* revert [IO-405]
A negative threshold should behave like a threshold of 0.
Until the first write the threshold is not reached actually, then the callback is called.
* fix typo in comment
* if the event has not been triggered, thresholdExceeded should be false.
* a negative threshold should behave like a zero behavior.
revert other changes
* remove stale comment
---------
Co-authored-by: r.proserpio <r.proserpio>
diff --git a/src/main/java/org/apache/commons/io/output/ThresholdingOutputStream.java b/src/main/java/org/apache/commons/io/output/ThresholdingOutputStream.java
index fd7a861..c0bf601 100644
--- a/src/main/java/org/apache/commons/io/output/ThresholdingOutputStream.java
+++ b/src/main/java/org/apache/commons/io/output/ThresholdingOutputStream.java
@@ -81,6 +81,7 @@
/**
* Constructs an instance of this class which will trigger an event at the specified threshold.
+ * A negative threshold has no meaning and will be treated as 0
*
* @param threshold The number of bytes at which to trigger an event.
* @param thresholdConsumer Accepts reaching the threshold.
@@ -89,10 +90,9 @@
*/
public ThresholdingOutputStream(final int threshold, final IOConsumer<ThresholdingOutputStream> thresholdConsumer,
final IOFunction<ThresholdingOutputStream, OutputStream> outputStreamGetter) {
- this.threshold = threshold;
+ this.threshold = threshold < 0 ? 0 : threshold;
this.thresholdConsumer = thresholdConsumer == null ? IOConsumer.noop() : thresholdConsumer;
this.outputStreamGetter = outputStreamGetter == null ? NOOP_OS_GETTER : outputStreamGetter;
- this.thresholdExceeded = threshold < 0;
}
/**
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 08e38c1..d412845 100644
--- a/src/test/java/org/apache/commons/io/output/DeferredFileOutputStreamTest.java
+++ b/src/test/java/org/apache/commons/io/output/DeferredFileOutputStreamTest.java
@@ -105,6 +105,28 @@
}
/**
+ * Tests the case where the threshold is negative, and therefore the data is always written to disk. The actual data
+ * written to disk is verified, as is the file itself.
+ */
+ @ParameterizedTest(name = "initialBufferSize = {0}")
+ @MethodSource("data")
+ public void testThresholdNegative(final int initialBufferSize) throws IOException {
+ final File testFile = Files.createTempFile(tempDirPath, "testThresholdNegative", "dat").toFile();
+ try (DeferredFileOutputStream dfos = 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());
+ verifyResultFile(testFile);
+ }
+ }
+
+ /**
* Tests the case where the amount of data is exactly the same as the threshold. The behavior should be the same as
* that for the amount of data being below (i.e. not exceeding) the threshold.
*/
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 cc03f8d..4391643 100644
--- a/src/test/java/org/apache/commons/io/output/ThresholdingOutputStreamTest.java
+++ b/src/test/java/org/apache/commons/io/output/ThresholdingOutputStreamTest.java
@@ -32,13 +32,47 @@
*/
public class ThresholdingOutputStreamTest {
+ /**
+ * Tests the case where the threshold is negative.
+ * The threshold is not reached until something is written to the stream.
+ */
@Test
public void testThresholdLessThanZero() throws IOException {
- try (final ThresholdingOutputStream out = new ThresholdingOutputStream(-1)) {
+ final AtomicBoolean reached = new AtomicBoolean();
+ try (final ThresholdingOutputStream out = new ThresholdingOutputStream(-1) {
+ @Override
+ protected void thresholdReached() throws IOException {
+ reached.set(true);
+ }
+ }) {
+ assertFalse(reached.get());
+ out.write(89);
+ assertTrue(reached.get());
assertTrue(out.isThresholdExceeded());
}
}
+ /**
+ * Tests the case where no bytes are written.
+ * The threshold is not reached until something is written to the stream.
+ */
+ @Test
+ public void testThresholdZeroWrite() throws IOException {
+ final AtomicBoolean reached = new AtomicBoolean();
+ try (final ThresholdingOutputStream out = new ThresholdingOutputStream(7) {
+ @Override
+ protected void thresholdReached() throws IOException {
+ reached.set(true);
+ }
+ }) {
+ assertFalse(out.isThresholdExceeded());
+ assertFalse(reached.get());
+ out.write(new byte[0]);
+ assertFalse(out.isThresholdExceeded());
+ assertFalse(reached.get());
+ }
+ }
+
@Test
public void testThresholdZero() throws IOException {
final AtomicBoolean reached = new AtomicBoolean();
