Expand exception handling, don't explicitly close Streams in IOUtils

Author: wodencafe

src/main/java/org/apache/commons/io/IOExceptionList.java

@@ -18,6 +18,7 @@
 package org.apache.commons.io;
 
 import java.io.IOException;
+import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
 import java.util.Objects;
@@ -35,6 +36,30 @@ public class IOExceptionList extends IOException {
 
     private static final long serialVersionUID = 1L;
 
+    /**
+     * Unwinds a {@code IOExceptionList} into a {@link List} of {@link Throwable}
+     * containing all of the underlying {@code Throwable} instances using
+     * {@link #getCauseList()}.
+     *
+     * Any instances of {@code IOExceptionList} encountered will be recursively
+     * unwound as well, and the contents of their {@code #getCauseList()} will
+     * be included in the returned {@code List}.
+     *
+     * @param ioExceptionList The {@code IOExceptionList} to recursively unwind,
+     * may be null, and {@code IOExceptionList#getCauseList()} may be null or empty.
+     * @return A {@code List} containing all of the {@code Throwable} instances
+     * inside the given {@code IOExceptionList} using {@code IOExceptionList#getCauseList()},
+     * this {@code List} will never contain instances of {@code IOExceptionList} itself.
+     * @since 2.12.0
+     */
+    public static List<Throwable> unwind(IOExceptionList ioExceptionList) {
+        if (ioExceptionList != null && !IOExceptionList.isEmpty(ioExceptionList.getCauseList())) {
+            return unwind(ioExceptionList.getCauseList());
+        } else {
+            return Collections.emptyList();
+        }
+    }
+
     /**
      * Throws this exception if the list is not null or empty.
      *
@@ -49,6 +74,20 @@ public static void checkEmpty(final List<? extends Throwable> causeList, final O
         }
     }
 
+    private static List<Throwable> unwind(final List<? extends Throwable> causeList) {
+        final List<Throwable> exceptions = new ArrayList<>();
+        if (Objects.nonNull(causeList)) {
+            for (Throwable t : causeList) {
+                if (t instanceof IOExceptionList) {
+                    exceptions.addAll(unwind((IOExceptionList)t));
+                } else {
+                    exceptions.add(t);
+                }
+            }
+        }
+        return exceptions;
+    }
+
     private static boolean isEmpty(final List<? extends Throwable> causeList) {
         return causeList == null || causeList.isEmpty();
     }

src/main/java/org/apache/commons/io/IOUtils.java

@@ -395,21 +395,16 @@ public static void close(final Closeable... closeables) throws IOException {
     }
 
     /**
-     * Closes the entries in the given {@link Stream} as null-safe operations,
-     * and closes the underlying {@code Stream}.
+     * Closes the entries in the given {@link Stream} as null-safe operations.
      *
      * @param <T> The element type.
-     * @param closeables The resource(s) to close, may be null.
+     * @param closeables The resource(s) to close, may be null or empty.
      * @throws IOExceptionList if an I/O error occurs.
      * @since 2.12.0
      */
     public static <T extends Closeable> void close(final Stream<T> closeables) throws IOExceptionList {
         if (closeables != null) {
-            try {
-                IOConsumer.forEachIndexed(closeables, IOUtils::close);
-            } finally {
-                closeables.close();
-            }
+            IOConsumer.forEachIndexed(closeables.filter(Objects::nonNull), IOUtils::close);
         }
     }
 
@@ -434,24 +429,19 @@ public static void close(final Closeable closeable, final IOConsumer<IOException
     }
 
     /**
-     * Closes the entries in the given {@link Stream} as null-safe operations,
-     * and closes the underlying {@code Stream}.
+     * Closes the entries in the given {@link Stream} as null-safe operations.
      *
      * @param <T> The element type.
      * @param consumer Consume the IOException thrown by {@link Closeable#close()}.
-     * @param closeables The resource(s) to close, may be null.
+     * @param closeables The resource(s) to close, may be null or empty.
      * @throws IOException if an I/O error occurs.
      */
     public static <T extends Closeable> void close(final IOConsumer<IOException> consumer, final Stream<T> closeables) throws IOException {
-        if (closeables != null) {
-            try {
-                close(closeables);
-            } catch (final IOException e) {
-                if (consumer != null) {
-                    consumer.accept(e);
-                }
-            } finally {
-                closeables.close();
+        try {
+            close(closeables);
+        } catch (final IOException e) {
+            if (consumer != null) {
+               consumer.accept(e);
             }
         }
     }
@@ -604,40 +594,30 @@ public static void closeQuietly(final Closeable closeable, final Consumer<IOExce
     }
 
     /**
-     * Closes the given {@link Stream} as a null-safe operation while consuming IOException by the given {@code consumer},
-     * and closes the underlying {@code Stream}.
+     * Closes the given {@link Stream} as a null-safe operation while consuming IOException by the given {@code consumer}.
      *
      * @param <T> The element type.
-     * @param closeables The resource(s) to close, may be null.
+     * @param closeables The resource(s) to close, may be null or empty.
      * @since 2.12.0
      */
     public static <T extends Closeable> void closeQuietly(final Stream<T> closeables) {
         closeQuietly(null, closeables);
     }
 
     /**
-     * Closes the given {@link Stream} as a null-safe operation while consuming IOException by the given {@code consumer},
-     * and closes the underlying {@code Stream}.
+     * Closes the given {@link Stream} as a null-safe operation while consuming IOException by the given {@code consumer}.
      *
      * @param <T> The element type.
-     * @param consumer Consume the IOException thrown by {@link Closeable#close()}.
-     * @param closeables The resource(s) to close, may be null.
+     * @param consumer Consume the IOException thrown by {@link Closeable#close()}, may be null.
+     * @param closeables The resource(s) to close, may be null or empty.
      * @since 2.12.0
      */
     public static <T extends Closeable> void closeQuietly(final Consumer<IOException> consumer, final Stream<T> closeables) {
-        if (closeables != null) {
-            try {
-                close(closeables);
-            } catch (final IOException e) {
-                if (consumer != null) {
-                    consumer.accept(e);
-                }
-            } finally {
-                try {
-                    closeables.close();
-                } catch (Exception e) {
-                    // Do nothing.
-                }
+        try {
+            close(closeables);
+        } catch (final IOException e) {
+            if (consumer != null) {
+                consumer.accept(e);
             }
         }
     }

src/main/java/org/apache/commons/io/function/IOConsumer.java

@@ -26,6 +26,7 @@
 
 import org.apache.commons.io.IOExceptionList;
 import org.apache.commons.io.IOIndexedException;
+import org.apache.commons.io.UncheckedIOExceptions;
 
 /**
  * Like {@link Consumer} but throws {@link IOException}.
@@ -51,14 +52,37 @@ public interface IOConsumer<T> {
      * @return a {@code Consumer} that wraps the given {@code IOConsumer}.
      * @since 2.12.0
      */
-    static <T> Consumer<T> wrap(IOConsumer<T> consumer) {
+    static <T> Consumer<T> asConsumer(IOConsumer<T> consumer) {
         return new Consumer<T>() {
             @Override
             public void accept(T t) {
                 try {
                     consumer.accept(t);
                 } catch (IOException e) {
-                    throw new UncheckedIOException(e);
+                    throw UncheckedIOExceptions.create(String.format("%s thrown from %s", e.getClass().getName(), String.valueOf(consumer)), e);
+                }
+            }
+        };
+    }
+
+    /**
+     * Wraps a {@link Consumer} inside of a {@code IOConsumer}
+     * that catches {@link UncheckedIOException}s that are thrown by the underlying
+     * {@code IOConsumer} and rethrows them as {@link IOException}
+     *
+     * @param <T> The element type.
+     * @param consumer The {@code Consumer} to wrap.
+     * @return a {@code IOConsumer} that wraps the given {@code Consumer}.
+     * @since 2.12.0
+     */
+    static <T> IOConsumer<T> wrap(Consumer<T> consumer) {
+        return new IOConsumer<T>() {
+            @Override
+            public void accept(T t) throws IOException {
+                try {
+                    consumer.accept(t);
+                } catch (UncheckedIOException e) {
+                    throw e.getCause() == null ? new IOException(e) : e.getCause();
                 }
             }
         };
@@ -150,18 +174,6 @@ static <T> IOConsumer<T> noop() {
      */
     void accept(T t) throws IOException;
 
-    /**
-     * Returns this {@code IOConsumer} wrapped inside of a {@link Consumer}
-     * that throws {@link UncheckedIOException} for any {@link IOException}s
-     * that are thrown by this {@code IOConsumer}.
-     *
-     * @return a {@code Consumer} that wraps this {@code IOConsumer}.
-     * @since 2.12.0
-     */
-    default Consumer<T> asConsumer() {
-        return wrap(this);
-    }
-
     /**
      * Returns a composed {@code IOConsumer} that performs, in sequence, this operation followed by the {@code after}
      * operation. If performing either operation throws an exception, it is relayed to the caller of the composed operation.

src/main/java/org/apache/commons/io/function/IOStreams.java

@@ -19,10 +19,13 @@
 
 import java.io.IOException;
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.List;
+import java.util.Objects;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.function.BiFunction;
+import java.util.function.Supplier;
 import java.util.stream.Stream;
 
 import org.apache.commons.io.IOExceptionList;
@@ -52,27 +55,50 @@ static <T> void forEach(final Stream<T> stream, final IOConsumer<T> action) thro
 
     static <T> void forEachIndexed(final Stream<T> stream, final IOConsumer<T> action, final BiFunction<Integer, IOException, IOException> exSupplier)
         throws IOExceptionList {
-        final AtomicReference<List<Throwable>> causeList = new AtomicReference<>();
+        final LazyAtomicReference<List<Throwable>> causeList = new LazyAtomicReference<>(() -> new ArrayList<>());
         final AtomicInteger index = new AtomicInteger();
         try {
             stream.forEach(e -> {
                 try {
                     action.accept(e);
-                } catch (final IOException ioex) {
-                    if (causeList.get() == null) {
-                        causeList.set(new ArrayList<>());
+                } catch (final IOExceptionList ioexl) {
+                    final Collection<Throwable> exceptions = IOExceptionList.unwind(ioexl);
+                    for (final Throwable t : exceptions) {
+                        if (t instanceof IOException) {
+                            causeList.getLazy().add(exSupplier.apply(index.get(), (IOException)t));
+                        } else {
+                            causeList.getLazy().add(exSupplier.apply(index.get(), new IOException(t)));
+                        }
                     }
-                    causeList.get().add(exSupplier.apply(index.get(), ioex));
+                } catch (final IOException ioex) {
+                    causeList.getLazy().add(exSupplier.apply(index.get(), ioex));
+                } catch (final Throwable t) {
+                    causeList.getLazy().add(exSupplier.apply(index.get(), new IOException(t)));
                 }
                 index.incrementAndGet();
             });
         }
         catch (Throwable t) {
-            if (causeList.get() == null) {
-                causeList.set(new ArrayList<>());
-            }
-            causeList.get().add(t);
+            causeList.getLazy().add(exSupplier.apply(index.get(), new IOException(t)));
         }
         IOExceptionList.checkEmpty(causeList.get(), "forEach");
     }
+
+    private static class LazyAtomicReference<T> extends AtomicReference<T> {
+
+        private static final long serialVersionUID = 1L;
+        private final Supplier<T> supplier;
+
+        public LazyAtomicReference(Supplier<T> supplier) {
+            Objects.requireNonNull(supplier);
+            this.supplier = supplier;
+        }
+
+        public final T getLazy() {
+            if (Objects.isNull(get())) {
+                set(supplier.get());
+            }
+            return get();
+        }
+    }
 }