Library enhancement proposal for copying data from/to Reader/Writer InputStream/OutputStream

[Pavel Rappo]

Patrick, as soon as I get feedback from Alan and Paul I'll merge these changes into the current version.

1. I wonder if it makes sense to provide an additional method where client specifies CharBuffer to be used?
Moreover if the first argument is CharSequence then we don't even need a buffer:

/**
 * Reads all characters from a readable and appends them to an appendable.
 *
 * @param source       the readable to read from ({@code source != null})
 * @param target       the appendable to append to ({@code target != null})
 *
 * @return             the number of characters copied
 *
 * @throws IOException if an I/O error occurs when reading or appending
 */
public static long copy(Readable source, Appendable target)
        throws IOException {
    Objects.requireNonNull(source, "source");
    Objects.requireNonNull(target, "target");

    if (source instanceof CharSequence)
        return copy1((CharSequence) source, target);

    return copy0(source, target, CharBuffer.allocate(BUFFER_SIZE));
}

/**
 * Reads all characters from a readable and appends them to an appendable.
 *
 * @param source       the readable to read from ({@code source != null})
 * @param target       the appendable to append to ({@code target != null})
 * @param buffer       the intermediate buffer to use
 *                     ({@code buffer != null && buffer.hasRemaining()
 *                             && !buffer.isReadOnly()})
 * @return             the number of characters copied
 * @throws IOException if an I/O error occurs when reading or appending
 */
public static long copy(Readable source, Appendable target,
                        CharBuffer buffer) throws IOException {
    Objects.requireNonNull(source, "source");
    Objects.requireNonNull(target, "target");
    Objects.requireNonNull(buffer, "buffer");
    if (!buffer.hasRemaining())
        throw new IllegalArgumentException("Insufficient buffer size");
    if (buffer.isReadOnly())
        throw new IllegalArgumentException("buffer is read only");
    return copy0(source, target, buffer);
}

private static long copy1(CharSequence source, Appendable target)
        throws IOException {
    long copied = source.length();
    target.append(source);
    return copied;
}

private static long copy0(Readable source, Appendable target,
                          CharBuffer buffer) throws IOException {

2. I think guts of the copy method could be changed to something like this:

    long copied = 0L;
+   int oldLim = buffer.limit();

    for (int read; (read = source.read(buffer)) > -1; copied += read) {
	buffer.flip();
        target.append(buffer, 0, read);
+       buffer.limit(oldLim);
    }
    return copied;

Otherwise on each iteration you're potentially shrinking available buffer storage based on
amount of data read into it (if I'm not mistaken). So each time you should restore the limit.

3. We should think of revisiting and including the only method from sun.misc.IOUtils. Removing
the class and reparenting its dependencies. 



-Pavel

> diff -r 194458fe7339 src/java.base/share/classes/java/io/IOUtil.java
> --- a/src/java.base/share/classes/java/io/IOUtil.java	Thu Nov 27 13:50:36 2014 +0100
> +++ b/src/java.base/share/classes/java/io/IOUtil.java	Thu Nov 27 16:08:09 2014 +0100
> @@ -47,10 +47,28 @@
>     /**
>      * Reads all bytes from an input stream and writes them to an output stream.
>      *
> -     * @param source       the input stream to read from
> -     * @param target       the output stream to write to
> -     * @return             the number of bytes successfully copied
> -     * @throws IOException if an I/O error occurs when reading or writing
> +     * <p>  If an I/O error occurs reading from the input stream or writing to
> +     * the output stream, then it may do so after the some bytes have been read 
> +     * or written. Consequently the input stream may not be at end of stream and 
> +     * may be in an inconsistent state. It is strongly recommended that the input
> +     * and output stream be promptly closed if an I/O error occurs.
> +     *
> +     * <p> This method may block indefinitely reading from the input stream (or
> +     * writing to the output stream). The behavior for the case that the input 
> +     * and output stream is <i>asynchronously closed</i> or the thread interrupted
> +     * during the copy is highly input and output stream system provider specific 
> +     * and therefore not specified.
> +     *
> +     * @param   source
> +     *          the input stream to read from
> +     * @param   target
> +     *          the output stream to write to
> +     *
> +     * @return  the number of bytes successfully copied
> +     * @throws  IOException
> +     *          if an I/O error occurs when reading or writing. In particular,
> +     *          an <code>IOException</code> may be thrown if the output stream 
> +     *          has been closed.
>      */
>     public static long copy(InputStream source, OutputStream target)
>             throws IOException {
> @@ -62,16 +80,34 @@
>     /**
>      * Reads all bytes from an input stream and writes them to an output stream.
>      *
> -     * @param source       the input stream to read from (not {@code null})
> -     * @param target       the output stream to write to (not {@code null})
> -     * @param buffer       the intermediate buffer to use (not {@code null},
> -     *                     and {@code buffer.length >= 1})
> -     * @return             the number of bytes successfully copied
> -     * @throws IOException if an I/O error occurs when reading or writing
> +     * <p>  If an I/O error occurs reading from the input stream or writing to
> +     * the output stream, then it may do so after the some bytes have been read 
> +     * or written. Consequently the input stream may not be at end of stream and 
> +     * may be in an inconsistent state. It is strongly recommended that the input
> +     * and output stream be promptly closed if an I/O error occurs.
> +     *
> +     * <p> This method may block indefinitely reading from the input stream (or
> +     * writing to the output stream). The behavior for the case that the input 
> +     * and output stream is <i>asynchronously closed</i> or the thread interrupted
> +     * during the copy is highly input and output stream system provider specific 
> +     * and therefore not specified.
> +     *
> +     * @param   source
> +     *          the input stream to read from (not {@code null})
> +     * @param   target
> +     *          the output stream to write to (not {@code null})
> +     * @param   buffer
> +     *          the intermediate buffer to use (not {@code null},
> +     *          and {@code buffer.length >= 1})
> +     *
> +     * @return  the number of bytes successfully copied
> +     * @throws  IOException
> +     *          if an I/O error occurs when reading or writing. In particular,
> +     *          an <code>IOException</code> may be thrown if the output stream 
> +     *          has been closed.
>      */
>     public static long copy(InputStream source, OutputStream target,
>                             byte[] buffer) throws IOException {
> -
>         Objects.requireNonNull(source, "source");
>         Objects.requireNonNull(target, "target");
>         Objects.requireNonNull(buffer, "buffer");
> @@ -92,23 +128,39 @@
>     /**
>      * Reads all characters from an readable and writes them to an appendable.
>      *
> -     * @param source the readable to read from
> -     * @param target the appendable to write to
> +     * <p>  If an I/O error occurs reading from the readable or writing to the 
> +     * appendable, then it may do so after the some characters have been read 
> +     * or written. Consequently the readable may not be at end of data and may
> +     * be in an inconsistent state. It is strongly recommended that the readable
> +     * and writable be promptly closed (in case of closables) if an I/O error
> +     * occurs.
>      *
> -     * @return the number of characters successfully read and written
> +     * <p> This method may block indefinitely reading from the readable (or
> +     * writing to the appendable). The behavior for the case that the input and
> +     * appendable is <i>asynchronously closed</i> or the thread interrupted
> +     * during the copy is highly readable and appendable system provider specific 
> +     * and therefore not specified.
>      *
> -     * @throws IOException if an I/O error occurs when reading or writing
> +     * @param   source
> +     *          the readable to read from
> +     * @param   target
> +     *          the appendable to write to
> +     *
> +     * @return  the number of characters successfully copied
> +     *
> +     * @throws  IOException
> +     *          if an I/O error occurs when reading or writing.
>      */
>     public static long copy(Readable source, Appendable target) throws IOException {
> -        long totalRead = 0L;
> +        Objects.requireNonNull(source, "source");
> +        Objects.requireNonNull(target, "target");
> +        long copied = 0L;
>         CharBuffer buffer = CharBuffer.allocate(BUFFER_SIZE);
> -        int read;
> -        while ((read = source.read(buffer)) > -1) {
> +        for (int read; (read = source.read(buffer)) > -1; copied += read) {
> 	    buffer.flip();
>             target.append(buffer, 0, read);
> -            totalRead += read;
>         }
> -        return totalRead;
> +        return copied;
>     }
> }
> 
>