[GitHub] [druid] clintropolis commented on a change in pull request #10247: Remove CloseQuietly and migrate its usages to other methods.

[GitBox <gi...@apache.org>]

clintropolis commented on a change in pull request #10247:
URL: https://github.com/apache/druid/pull/10247#discussion_r595598037



##########
File path: core/src/main/java/org/apache/druid/utils/CloseableUtils.java
##########
@@ -34,15 +42,129 @@
    * first.close();
    * second.close();
    *
-   * to have safety of {@link org.apache.druid.java.util.common.io.Closer}, but without associated boilerplate code
+   * to have safety of {@link Closer}, but without associated boilerplate code
    * of creating a Closer and registering objects in it.
    */
-  public static void closeBoth(Closeable first, Closeable second) throws IOException
+  public static void closeAll(Closeable first, Closeable... others) throws IOException
+  {
+    final List<Closeable> closeables = new ArrayList<>(others.length + 1);
+    closeables.add(first);
+    closeables.addAll(Arrays.asList(others));
+    closeAll(closeables);
+  }
+
+  /**
+   * Close all the provided {@param closeables}, from first to last.
+   */
+  public static <T extends Closeable> void closeAll(Iterable<T> closeables) throws IOException
+  {
+    final Closer closer = Closer.create();
+
+    // Register in reverse order, so we close from first to last.
+    closer.registerAll(Lists.reverse(Lists.newArrayList(closeables)));
+    closer.close();
+  }
+
+  /**
+   * Like {@link Closeable#close()}, but guaranteed to throw {@param caught}. Will add any exceptions encountered
+   * during closing to {@param caught} using {@link Throwable#addSuppressed(Throwable)}.
+   *
+   * Should be used like {@code throw CloseableUtils.closeInCatch(e, closeable)}. (The "throw" is important for
+   * reachability detection.)
+   */
+  public static <E extends Throwable> RuntimeException closeInCatch(
+      final E caught,
+      @Nullable final Closeable closeable
+  ) throws E
+  {
+    if (caught == null) {
+      // Incorrect usage; throw an exception with an error message that may be useful to the programmer.
+      final RuntimeException e1 = new IllegalStateException("Must be called with non-null caught exception");
+
+      if (closeable != null) {
+        try {
+          closeable.close();
+        }
+        catch (Throwable e2) {
+          e1.addSuppressed(e2);
+        }
+      }
+
+      throw e1;
+    }
+
+    if (closeable != null) {
+      try {
+        closeable.close();
+      }
+      catch (Throwable e) {
+        caught.addSuppressed(e);
+      }
+    }
+
+    throw caught;
+  }
+
+  /**
+   * Like {@link #closeInCatch} but wraps {@param caught} in a {@link RuntimeException} if it is a checked exception.
+   */
+  public static <E extends Throwable> RuntimeException closeAndWrapInCatch(
+      final E caught,
+      @Nullable final Closeable closeable
+  )
+  {
+    try {
+      throw closeInCatch(caught, closeable);
+    }
+    catch (RuntimeException | Error e) {
+      // Unchecked exception.
+      throw e;
+    }
+    catch (Throwable e) {
+      // Checked exception; must wrap.
+      throw new RuntimeException(e);
+    }
+  }
+
+  /**
+   * Like {@link Closeable#close()} but wraps IOExceptions in RuntimeExceptions.
+   */
+  public static void closeAndWrapExceptions(@Nullable final Closeable closeable)
   {
-    //noinspection EmptyTryBlock
-    try (Closeable ignore1 = second;
-         Closeable ignore2 = first) {
-      // piggy-back try-with-resources semantics
+    if (closeable == null) {
+      return;
+    }
+
+    try {
+      closeable.close();
+    }
+    catch (IOException e) {
+      throw new RuntimeException(e);
+    }
+  }
+
+  /**
+   * Like {@link Closeable#close()} but sends any exceptions to the provided Consumer, and then throws them away.
+   *
+   * If the Consumer throws an exception, that exception is thrown by this method. So if your intent is to chomp
+   * exceptions, you should avoid writing a Consumer that might thrown an exception.

Review comment:
       ```suggestion
      * exceptions, you should avoid writing a Consumer that might throw an exception.
   ```




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org