Extract close quietly body execution callback (#1698)

Author: cronik

src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/ContainerStepExecution.java

@@ -12,13 +12,11 @@
 import hudson.slaves.NodeProperty;
 import hudson.slaves.NodePropertyDescriptor;
 import hudson.util.DescribableList;
-import java.io.Closeable;
 import java.util.Collections;
 import java.util.List;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import jenkins.model.Jenkins;
-import org.jenkinsci.plugins.workflow.steps.BodyExecutionCallback;
 import org.jenkinsci.plugins.workflow.steps.BodyInvoker;
 import org.jenkinsci.plugins.workflow.steps.EnvironmentExpander;
 import org.jenkinsci.plugins.workflow.steps.StepContext;
@@ -81,7 +79,7 @@ public boolean start() throws Exception {
                 .newBodyInvoker()
                 .withContexts(
                         BodyInvoker.mergeLauncherDecorators(getContext().get(LauncherDecorator.class), decorator), env)
-                .withCallback(new ContainerExecCallback(decorator))
+                .withCallback(closeQuietlyCallback(decorator))
                 .start();
         return false;
     }
@@ -91,21 +89,4 @@ public void stop(@NonNull Throwable cause) throws Exception {
         LOGGER.log(Level.FINE, "Stopping container step.");
         closeQuietly(getContext(), decorator);
     }
-
-    @SuppressFBWarnings("SE_BAD_FIELD")
-    private static class ContainerExecCallback extends BodyExecutionCallback.TailCall {
-
-        private static final long serialVersionUID = 6385838254761750483L;
-
-        private final Closeable[] closeables;
-
-        private ContainerExecCallback(Closeable... closeables) {
-            this.closeables = closeables;
-        }
-
-        @Override
-        public void finished(StepContext context) {
-            closeQuietly(context, closeables);
-        }
-    }
 }

src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/Resources.java

@@ -16,29 +16,78 @@
 
 package org.csanchez.jenkins.plugins.kubernetes.pipeline;
 
+import edu.umd.cs.findbugs.annotations.NonNull;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import hudson.model.TaskListener;
 import java.io.Closeable;
 import java.io.IOException;
 import java.util.logging.Level;
 import java.util.logging.Logger;
+import org.jenkinsci.plugins.workflow.steps.BodyExecutionCallback;
 import org.jenkinsci.plugins.workflow.steps.StepContext;
 
 public class Resources {
-    private static final transient Logger LOGGER = Logger.getLogger(ContainerStepExecution.class.getName());
 
-    static void closeQuietly(StepContext context, Closeable... closeables) {
+    private static final Logger LOGGER = Logger.getLogger(ContainerStepExecution.class.getName());
+
+    /**
+     * Close each {@link Closeable}, capturing and ignoring any {@link IOException} thrown.
+     * @param context step context
+     * @param closeables list of closable resources to close
+     */
+    public static void closeQuietly(@NonNull StepContext context, @NonNull Closeable... closeables) {
         for (Closeable c : closeables) {
             if (c != null) {
                 try {
                     c.close();
                 } catch (IOException e) {
                     try {
-                        context.get(TaskListener.class).error("Error while closing: [" + c + "]");
+                        TaskListener listener = context.get(TaskListener.class);
+                        if (listener != null) {
+                            listener.error("Error while closing: [" + c + "]");
+                        }
                     } catch (IOException | InterruptedException e1) {
                         LOGGER.log(Level.WARNING, "Error writing to task listener", e);
                     }
                 }
             }
         }
     }
+
+    /**
+     * Factory method for a {@link BodyExecutionCallback} that closes each {@link Closeable} when
+     * execution is finished.
+     * @param closeables list of closable resources
+     * @return new body execution callback, never null
+     */
+    @NonNull
+    public static BodyExecutionCallback closeQuietlyCallback(@NonNull Closeable... closeables) {
+        return new CloseQuietlyBodyExecutionCallback(closeables);
+    }
+
+    /**
+     * Pipeline body execution callback that quietly closes all referenced {@link Closeable} when
+     * {@link #finished(StepContext)} is called.
+     * @see #closeQuietly(StepContext, Closeable...)
+     */
+    @SuppressFBWarnings("SE_BAD_FIELD")
+    private static class CloseQuietlyBodyExecutionCallback extends BodyExecutionCallback.TailCall {
+
+        private static final long serialVersionUID = 6385838254761750483L;
+
+        private final Closeable[] closeables;
+
+        /**
+         * Create new body execution callback that closes the provided resources quietly.
+         * @param closeables list of closable resources
+         */
+        CloseQuietlyBodyExecutionCallback(@NonNull Closeable... closeables) {
+            this.closeables = closeables;
+        }
+
+        @Override
+        public void finished(StepContext context) {
+            closeQuietly(context, closeables);
+        }
+    }
 }

src/test/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/ResourcesTest.java

@@ -0,0 +1,75 @@
+package org.csanchez.jenkins.plugins.kubernetes.pipeline;
+
+import static org.mockito.Mockito.*;
+
+import hudson.AbortException;
+import hudson.model.TaskListener;
+import java.io.Closeable;
+import java.io.IOException;
+import org.jenkinsci.plugins.workflow.steps.BodyExecutionCallback;
+import org.jenkinsci.plugins.workflow.steps.StepContext;
+import org.junit.Test;
+
+public class ResourcesTest {
+
+    @Test
+    public void testCloseQuietly() throws Exception {
+        StepContext ctx = mock(StepContext.class);
+        TaskListener listener = mock(TaskListener.class);
+        when(ctx.get(TaskListener.class))
+                .thenReturn(listener)
+                .thenThrow(IOException.class)
+                .thenThrow(InterruptedException.class)
+                .thenReturn(null);
+        Closeable c1 = mock(Closeable.class);
+        doThrow(IOException.class).when(c1).close();
+        Closeable c2 = mock(Closeable.class);
+        doThrow(IOException.class).when(c2).close();
+        Closeable c3 = mock(Closeable.class);
+        doThrow(IOException.class).when(c3).close();
+        Closeable c4 = mock(Closeable.class);
+
+        // test
+        Resources.closeQuietly(ctx, c1, c2, null, c3, c4);
+
+        // verify
+        verify(c1).close();
+        verify(c2).close();
+        verify(c3).close();
+        verify(c4).close();
+        verify(ctx, times(3)).get(TaskListener.class);
+        verify(listener).error(any());
+    }
+
+    @Test
+    public void testCloseQuietlyCallbackOnSuccess() throws Exception {
+        StepContext ctx = mock(StepContext.class);
+        Closeable c1 = mock(Closeable.class);
+        doThrow(IOException.class).when(c1).close();
+        Closeable c2 = mock(Closeable.class);
+
+        // test
+        BodyExecutionCallback callback = Resources.closeQuietlyCallback(c1, c2);
+        callback.onSuccess(ctx, "done");
+
+        // verify
+        verify(c1).close();
+        verify(c2).close();
+    }
+
+    @Test
+    public void testCloseQuietlyCallbackOnFailure() throws Exception {
+        StepContext ctx = mock(StepContext.class);
+        Closeable c1 = mock(Closeable.class);
+        doThrow(IOException.class).when(c1).close();
+        Closeable c2 = mock(Closeable.class);
+
+        // test
+        BodyExecutionCallback callback = Resources.closeQuietlyCallback(c1, c2);
+        callback.onFailure(ctx, new AbortException());
+
+        // verify
+        verify(c1).close();
+        verify(c2).close();
+    }
+}