BoundedInputStream can count its bytes without wrapping a

garydgregory · garydgregory · commit 812d8a4b53d9 · 2024-02-18T17:44:08.000-05:00
CountingInputStream

- Deprecate CountingInputStream in favor of BoundedInputStream
- Add a BoundedInputStream builder
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
@@ -130,6 +130,8 @@ The <action> type attribute can be add,update,fix,remove.
       <action dev="ggregory" type="add"                due-to="Gary Gregory">Add AbstractStreamBuilder.getReader().</action>
       <action dev="ggregory" type="add"                due-to="Gary Gregory">Add Maven property project.build.outputTimestamp for build reproducibility.</action>
       <action dev="ggregory" type="add"                due-to="Gary Gregory">Add ProxyInputStream.unwrap().</action>
+      <action dev="ggregory" type="add"                due-to="Gary Gregory">Add a running count and builder to BoundedInputStream.</action>
+      <action dev="ggregory" type="add"                due-to="Gary Gregory">Add a builder to CountingInputStream.</action>
       <!--  UPDATE -->
       <action dev="ggregory" type="update"             due-to="Gary Gregory">Bump commons.bytebuddy.version from 1.14.10 to 1.14.11 #534.</action>
       <action dev="ggregory" type="update"             due-to="Gary Gregory">Bump org.apache.commons:commons-parent from 65 to 66.</action>
diff --git a/src/main/java/org/apache/commons/io/input/BoundedInputStream.java b/src/main/java/org/apache/commons/io/input/BoundedInputStream.java
@@ -24,28 +24,153 @@
 import org.apache.commons.io.IOUtils;
 import org.apache.commons.io.build.AbstractStreamBuilder;
 
+//@formatter:off
 /**
- * Reads bytes up to a maximum length, if its count goes above that, it stops.
+ * Reads bytes up to a maximum count and stops once reached.
  * <p>
- * This is useful to wrap {@code ServletInputStream}s. The {@code ServletInputStream} will block if you try to read content from it that isn't there, because it
- * doesn't know whether the content hasn't arrived yet or whether the content has finished. So, one of these, initialized with the {@code Content-Length} sent
- * in the {@code ServletInputStream}'s header, will stop it blocking, providing it's been sent with a correct content length.
+ * To build an instance, see {@link AbstractBuilder}.
  * </p>
  * <p>
- * To build an instance, use {@link Builder}.
+ * By default, a {@link BoundedInputStream} is <em>unbound</em>; so make sure to call {@link AbstractBuilder#setMaxCount(long)}.
  * </p>
- *
+ * <p>
+ * You can find out how many bytes this stream has seen so far by calling {@link BoundedInputStream#getCount()}. This value reflects bytes read and skipped.
+ * </p>
+ * <h2>Using a ServletInputStream</h2>
+ * <p>
+ * A {@code ServletInputStream} can block if you try to read content that isn't there
+ * because it doesn't know whether the content hasn't arrived yet or whether the content has finished. Initialize an {@link BoundedInputStream} with the
+ * {@code Content-Length} sent in the {@code ServletInputStream}'s header, this stop it from blocking, providing it's been sent with a correct content
+ * length in the first place.
+ * </p>
+ * <h2>Using NIO</h2>
+ * <pre>{@code
+ * BoundedInputStream s = BoundedInputStream.builder()
+ *   .setPath(Paths.get("MyFile.xml"))
+ *   .setMaxCount(1024)
+ *   .setPropagateClose(false)
+ *   .get();
+ * }
+ * </pre>
+ * <h2>Using IO</h2>
+ * <pre>{@code
+ * BoundedInputStream s = BoundedInputStream.builder()
+ *   .setFile(new File("MyFile.xml"))
+ *   .setMaxCount(1024)
+ *   .setPropagateClose(false)
+ *   .get();
+ * }
+ * </pre>
+ * <h2>Counting Bytes</h2>
+ * <p>You can set the running count when building, which is most useful when starting from another stream:
+ * <pre>{@code
+ * InputStream in = ...;
+ * BoundedInputStream s = BoundedInputStream.builder()
+ *   .setInputStream(in)
+ *   .setCount(12)
+ *   .setMaxCount(1024)
+ *   .setPropagateClose(false)
+ *   .get();
+ * }
+ * </pre>
  * @see Builder
  * @since 2.0
  */
+//@formatter:on
 public class BoundedInputStream extends ProxyInputStream {
 
-    // TODO For 3.0, extend CountingInputStream. Or, add a max feature to CountingInputStream.
+    /**
+     * For subclassing builders from {@link BoundedInputStream} subclassses.
+     *
+     * @param <T> The subclass.
+     */
+    static abstract class AbstractBuilder<T extends AbstractBuilder<T>> extends AbstractStreamBuilder<BoundedInputStream, T> {
+
+        /** The current count of bytes counted. */
+        private long count;
+
+        /** The max count of bytes to read. */
+        private long maxCount = EOF;
+
+        /** Flag if {@link #close()} should be propagated, {@code true} by default. */
+        private boolean propagateClose = true;
+
+        long getCount() {
+            return count;
+        }
+
+        long getMaxCount() {
+            return maxCount;
+        }
+
+        boolean isPropagateClose() {
+            return propagateClose;
+        }
+
+        /**
+         * Sets the current number of bytes counted.
+         * <p>
+         * Useful when building from another stream to carry forward a read count.
+         * </p>
+         * <p>
+         * Default is {@code 0}, negative means 0.
+         * </p>
+         *
+         * @param count The current number of bytes counted.
+         * @return this.
+         */
+        public T setCount(final long count) {
+            this.count = Math.max(0, count);
+            return asThis();
+        }
+
+        /**
+         * Sets the maximum number of bytes to return.
+         * <p>
+         * Default is {@value IOUtils#EOF}, negative means unbound.
+         * </p>
+         *
+         * @param maxCount The maximum number of bytes to return.
+         * @return this.
+         */
+        public T setMaxCount(final long maxCount) {
+            this.maxCount = Math.max(EOF, maxCount);
+            return asThis();
+        }
+
+        /**
+         * Sets whether the {@link #close()} method should propagate to the underling {@link InputStream}.
+         * <p>
+         * Default is {@code true}.
+         * </p>
+         *
+         * @param propagateClose {@code true} if calling {@link #close()} propagates to the {@code close()} method of the underlying stream or {@code false} if
+         *                       it does not.
+         * @return this.
+         */
+        public T setPropagateClose(final boolean propagateClose) {
+            this.propagateClose = propagateClose;
+            return asThis();
+        }
+
+    }
 
     //@formatter:off
     /**
      * Builds a new {@link BoundedInputStream}.
-     *
+     * <p>
+     * By default, a {@link BoundedInputStream} is <em>unbound</em>; so make sure to call {@link AbstractBuilder#setMaxCount(long)}.
+     * </p>
+     * <p>
+     * You can find out how many bytes this stream has seen so far by calling {@link BoundedInputStream#getCount()}. This value reflects bytes read and skipped.
+     * </p>
+     * <h2>Using a ServletInputStream</h2>
+     * <p>
+     * A {@code ServletInputStream} can block if you try to read content that isn't there
+     * because it doesn't know whether the content hasn't arrived yet or whether the content has finished. Initialize an {@link BoundedInputStream} with the
+     * {@code Content-Length} sent in the {@code ServletInputStream}'s header, this stop it from blocking, providing it's been sent with a correct content
+     * length in the first place.
+     * </p>
      * <h2>Using NIO</h2>
      * <pre>{@code
      * BoundedInputStream s = BoundedInputStream.builder()
@@ -64,18 +189,24 @@ public class BoundedInputStream extends ProxyInputStream {
      *   .get();
      * }
      * </pre>
+     * <h2>Counting Bytes</h2>
+     * <p>You can set the running count when building, which is most useful when starting from another stream:
+     * <pre>{@code
+     * InputStream in = ...;
+     * BoundedInputStream s = BoundedInputStream.builder()
+     *   .setInputStream(in)
+     *   .setCount(12)
+     *   .setMaxCount(1024)
+     *   .setPropagateClose(false)
+     *   .get();
+     * }
+     * </pre>
      *
      * @see #get()
      * @since 2.16.0
      */
     //@formatter:on
-    public static class Builder extends AbstractStreamBuilder<BoundedInputStream, Builder> {
-
-        /** The max count of bytes to read. */
-        private long maxCount = EOF;
-
-        /** Flag if close should be propagated. */
-        private boolean propagateClose = true;
+    public static class Builder extends AbstractBuilder<Builder> {
 
         /**
          * Builds a new {@link BoundedInputStream}.
@@ -100,50 +231,24 @@ public static class Builder extends AbstractStreamBuilder<BoundedInputStream, Bu
         @SuppressWarnings("resource")
         @Override
         public BoundedInputStream get() throws IOException {
-            return new BoundedInputStream(getInputStream(), maxCount, propagateClose);
-        }
-
-        /**
-         * Sets the maximum number of bytes to return.
-         * <p>
-         * Default is {@value IOUtils#EOF}.
-         * </p>
-         *
-         * @param maxCount The maximum number of bytes to return.
-         * @return this.
-         */
-        public Builder setMaxCount(final long maxCount) {
-            this.maxCount = maxCount;
-            return this;
-        }
-
-        /**
-         * Sets whether the {@link #close()} method should propagate to the underling {@link InputStream}.
-         * <p>
-         * Default is true.
-         * </p>
-         *
-         * @param propagateClose {@code true} if calling {@link #close()} propagates to the {@code close()} method of the underlying stream or {@code false} if
-         *                       it does not.
-         * @return this.
-         */
-        public Builder setPropagateClose(final boolean propagateClose) {
-            this.propagateClose = propagateClose;
-            return this;
+            return new BoundedInputStream(getInputStream(), getCount(), getMaxCount(), isPropagateClose());
         }
 
     }
 
     /**
-     * Constructs a new {@link Builder}.
+     * Constructs a new {@link AbstractBuilder}.
      *
-     * @return a new {@link Builder}.
+     * @return a new {@link AbstractBuilder}.
      * @since 2.16.0
      */
     public static Builder builder() {
         return new Builder();
     }
 
+    /** The current count of bytes counted. */
+    private long count;
+
     /** The max count of bytes to read. */
     private final long maxCount;
 
@@ -158,7 +263,7 @@ public static Builder builder() {
      * Constructs a new {@link BoundedInputStream} that wraps the given input stream and is unlimited.
      *
      * @param in The wrapped input stream.
-     * @deprecated Use {@link Builder#get()}.
+     * @deprecated Use {@link AbstractBuilder#get()}.
      */
     @Deprecated
     public BoundedInputStream(final InputStream in) {
@@ -169,33 +274,49 @@ public BoundedInputStream(final InputStream in) {
      * Constructs a new {@link BoundedInputStream} that wraps the given input stream and limits it to a certain size.
      *
      * @param inputStream The wrapped input stream.
-     * @param maxCount   The maximum number of bytes to return.
-     * @deprecated Use {@link Builder#get()}.
+     * @param maxCount    The maximum number of bytes to return.
+     * @deprecated Use {@link AbstractBuilder#get()}.
      */
     @Deprecated
     public BoundedInputStream(final InputStream inputStream, final long maxCount) {
         // Some badly designed methods - e.g. the Servlet API - overload length
         // such that "-1" means stream finished
-        this(inputStream, maxCount, true);
+        this(inputStream, 0, maxCount, true);
     }
 
     /**
      * Constructs a new {@link BoundedInputStream} that wraps the given input stream and limits it to a certain size.
      *
      * @param inputStream    The wrapped input stream.
-     * @param maxCount      The maximum number of bytes to return.
+     * @param count          The current number of bytes read.
+     * @param maxCount       The maximum number of bytes to return.
      * @param propagateClose {@code true} if calling {@link #close()} propagates to the {@code close()} method of the underlying stream or {@code false} if it
      *                       does not.
      */
-    @SuppressWarnings("resource") // Caller closes.
-    private BoundedInputStream(final InputStream inputStream, final long maxCount, final boolean propagateClose) {
+    BoundedInputStream(final InputStream inputStream, final long count, final long maxCount, final boolean propagateClose) {
         // Some badly designed methods - e.g. the Servlet API - overload length
         // such that "-1" means stream finished
-        super(new CountingInputStream(inputStream));
+        // Can't throw because we start from an InputStream.
+        super(inputStream);
+        this.count = count;
         this.maxCount = maxCount;
         this.propagateClose = propagateClose;
     }
 
+    /**
+     * Adds the number of read bytes to the count.
+     *
+     * @param n number of bytes read, or -1 if no more bytes are available
+     * @throws IOException Not thrown here but subclasses may throw.
+     * @since 2.0
+     */
+    @Override
+    protected synchronized void afterRead(final int n) throws IOException {
+        if (n != EOF) {
+            count += n;
+        }
+    }
+
     /**
      * {@inheritDoc}
      */
@@ -226,13 +347,8 @@ public void close() throws IOException {
      * @return The count of bytes read.
      * @since 2.12.0
      */
-    @SuppressWarnings("resource") // no allocation
     public long getCount() {
-        return getCountingInputStream().getByteCount();
-    }
-
-    private CountingInputStream getCountingInputStream() {
-        return (CountingInputStream) in;
+        return count;
     }
 
     /**
@@ -264,7 +380,7 @@ public long getMaxLength() {
      * @since 2.16.0
      */
     public long getRemaining() {
-        return getMaxCount() - getCount();
+        return Math.max(0, getMaxCount() - getCount());
     }
 
     private boolean isMaxCount() {
@@ -373,7 +489,7 @@ public synchronized void reset() throws IOException {
      *
      * @param propagateClose {@code true} if calling {@link #close()} propagates to the {@code close()} method of the underlying stream or {@code false} if it
      *                       does not.
-     * @deprecated Use {@link Builder#setPropagateClose(boolean)}.
+     * @deprecated Use {@link AbstractBuilder#setPropagateClose(boolean)}.
      */
     @Deprecated
     public void setPropagateClose(final boolean propagateClose) {
@@ -389,7 +505,9 @@ public void setPropagateClose(final boolean propagateClose) {
      */
     @Override
     public long skip(final long n) throws IOException {
-        return super.skip(toReadLen(n));
+        final long skip = super.skip(toReadLen(n));
+        count += skip;
+        return skip;
     }
 
     private long toReadLen(final long len) {
diff --git a/src/main/java/org/apache/commons/io/input/CountingInputStream.java b/src/main/java/org/apache/commons/io/input/CountingInputStream.java
@@ -28,7 +28,9 @@
  * A typical use case would be during debugging, to ensure that data is being
  * read as expected.
  * </p>
+ * @deprecated Use {@link BoundedInputStream} (unbounded by default).
  */
+@Deprecated
 public class CountingInputStream extends ProxyInputStream {
 
     /** The count of bytes that have passed. */
diff --git a/src/test/java/org/apache/commons/io/input/BoundedInputStreamTest.java b/src/test/java/org/apache/commons/io/input/BoundedInputStreamTest.java
