2 files changed, 88 insertions, 14 deletions

diff --git a/infra/impl/src/main/java/io/fd/honeycomb/impl/ShutdownHandlerImpl.java b/infra/impl/src/main/java/io/fd/honeycomb/impl/ShutdownHandlerImpl.java
index c7cdb21bb..71145817f 100644
--- a/infra/impl/src/main/java/io/fd/honeycomb/impl/ShutdownHandlerImpl.java
+++ b/infra/impl/src/main/java/io/fd/honeycomb/impl/ShutdownHandlerImpl.java
@@ -16,8 +16,7 @@
 
 package io.fd.honeycomb.impl;
 
-import static java.lang.String.format;
-
+import com.google.common.annotations.VisibleForTesting;
 import io.fd.honeycomb.data.init.ShutdownHandler;
 import java.util.Deque;
 import java.util.LinkedList;
@@ -25,7 +24,7 @@ import javax.annotation.Nonnull;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-public class ShutdownHandlerImpl implements ShutdownHandler {
+public final class ShutdownHandlerImpl implements ShutdownHandler {
 
     private static final Logger LOG = LoggerFactory.getLogger(ShutdownHandlerImpl.class);
 
@@ -33,17 +32,7 @@ public class ShutdownHandlerImpl implements ShutdownHandler {
 
     public ShutdownHandlerImpl() {
         components = new LinkedList<>();
-        Runtime.getRuntime().addShutdownHook(new Thread(() -> {
-            // close components in reverse order that they were registered
-            components.descendingIterator().forEachRemaining(closeable -> {
-                LOG.info("Closing component {}", closeable.getName());
-                try {
-                    closeable.getComponent().close();
-                } catch (Exception e) {
-                    throw new IllegalStateException(format("Unable to close component %s", closeable.getName()), e);
-                }
-            });
-        }));
+        Runtime.getRuntime().addShutdownHook(new Thread((this::performShutdown)));
     }
 
     @Override
@@ -70,4 +59,18 @@ public class ShutdownHandlerImpl implements ShutdownHandler {
             return component;
         }
     }
+
+    @VisibleForTesting
+    void performShutdown() {
+        // close components in reverse order that they were registered
+        components.descendingIterator().forEachRemaining(closeable -> {
+            LOG.info("Closing component {}", closeable.getName());
+            try {
+                closeable.getComponent().close();
+            } catch (Exception e) {
+                // We can't do much here, so logging exception and moving to the next closable component
+                LOG.warn("Unable to close component {}", closeable.getName(), e);
+            }
+        });
+    }
 }
diff --git a/infra/minimal-distribution-test/src/test/java/io/fd/honeycomb/impl/ShutdownHandlerImplTest.java b/infra/minimal-distribution-test/src/test/java/io/fd/honeycomb/impl/ShutdownHandlerImplTest.java
new file mode 100644
index 000000000..4e8135488
--- /dev/null
+++ b/infra/minimal-distribution-test/src/test/java/io/fd/honeycomb/impl/ShutdownHandlerImplTest.java
@@ -0,0 +1,71 @@
+/*
+ * Copyright (c) 2017 Cisco and/or its affiliates.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package io.fd.honeycomb.impl;
+
+import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.mock;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.InOrder;
+import org.mockito.Mockito;
+
+public class ShutdownHandlerImplTest {
+
+    private ShutdownHandlerImpl shutdownHandler;
+
+    @Before
+    public void setUp() {
+        shutdownHandler = new ShutdownHandlerImpl();
+    }
+
+    @Test
+    public void testShutdownOrder() throws Exception {
+        AutoCloseable component0 = mock(AutoCloseable.class);
+        AutoCloseable component1 = mock(AutoCloseable.class);
+        AutoCloseable component2 = mock(AutoCloseable.class);
+        InOrder inOrder = Mockito.inOrder(component0, component1, component2);
+        shutdownHandler.register("component0", component0);
+        shutdownHandler.register("component1", component1);
+        shutdownHandler.register("component2", component2);
+        shutdownHandler.performShutdown();
+
+        // check the order was reversed
+        inOrder.verify(component2).close();
+        inOrder.verify(component1).close();
+        inOrder.verify(component0).close();
+    }
+
+    @Test
+    public void testShutdownFail() throws Exception {
+        AutoCloseable component0 = mock(AutoCloseable.class);
+        AutoCloseable component1 = mock(AutoCloseable.class);
+        doThrow(new IllegalStateException()).when(component1).close();
+        AutoCloseable component2 = mock(AutoCloseable.class);
+        InOrder inOrder = Mockito.inOrder(component0, component1, component2);
+        shutdownHandler.register("component0", component0);
+        shutdownHandler.register("component1", component1);
+        shutdownHandler.register("component2", component2);
+        shutdownHandler.performShutdown();
+
+        // shoutdown failed for component1, but other components should be closed
+        inOrder.verify(component2).close();
+        inOrder.verify(component0).close();
+        inOrder.verifyNoMoreInteractions();
+    }
+
+}