# HG changeset patch
# User Severin Gehwolf <sgehwolf@redhat.com>
# Date 1364994767 -7200
# Node ID f2bea882daf45eb7ac6cf32b11dd91fe3da600c1
# Parent  f82c6b24dcbd97c271596d99a0ec6a199b4e42c2
Avoid stack trace on agent shutdown.
Reviewed-by: omajid
Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2013-April/006254.html

diff -r f82c6b24dcbd -r f2bea882daf4 storage/core/src/main/java/com/redhat/thermostat/storage/core/QueuedStorage.java
--- a/storage/core/src/main/java/com/redhat/thermostat/storage/core/QueuedStorage.java	Thu Apr 04 11:24:33 2013 +0200
+++ b/storage/core/src/main/java/com/redhat/thermostat/storage/core/QueuedStorage.java	Wed Apr 03 15:12:47 2013 +0200
@@ -277,7 +277,11 @@
 
     @Override
     public void shutdown() {
-        delegate.shutdown();
+        /*
+         * First shut down executors. This may trigger some pushes to the
+         * storage implementation (a.k.a. delegate). Hence, this should get
+         * shut down last as this closes the connection etc.
+         */
         try {
             executor.shutdown();
             executor.awaitTermination(SHUTDOWN_TIMEOUT_SECONDS, TimeUnit.SECONDS);
@@ -290,6 +294,7 @@
         } catch (InterruptedException ex) {
             // Fall through. 
         }
+        delegate.shutdown();
     }
 
 }
diff -r f82c6b24dcbd -r f2bea882daf4 storage/core/src/test/java/com/redhat/thermostat/storage/core/QueuedStorageTest.java
--- a/storage/core/src/test/java/com/redhat/thermostat/storage/core/QueuedStorageTest.java	Thu Apr 04 11:24:33 2013 +0200
+++ b/storage/core/src/test/java/com/redhat/thermostat/storage/core/QueuedStorageTest.java	Wed Apr 03 15:12:47 2013 +0200
@@ -177,6 +177,22 @@
         }
 
     }
+    
+    private static class TestShutdownExecutor extends TestExecutor {
+        long executorShutDownTime = -1;
+        
+        @Override
+        public void shutdown() {
+            super.shutdown();
+            executorShutDownTime = System.currentTimeMillis();
+            // delay shutdown just a little
+            try {
+                Thread.sleep(10);
+            } catch (InterruptedException e) {
+                Thread.currentThread().interrupt();
+            }
+        }
+    }
 
     private static class TestPojo implements Pojo {
         
@@ -429,5 +445,124 @@
         assertEquals(3, fileExecutor.getAwaitTerminationTimeout());
         assertEquals(TimeUnit.SECONDS, fileExecutor.getAwaitTerminationTimeUnit());
     }
+    
+    @Test
+    public void executorsShutdownBeforeDelegate() {
+        StubStorage delegate = new StubStorage();
+        TestShutdownExecutor executor = new TestShutdownExecutor();
+        TestShutdownExecutor fileExecutor = new TestShutdownExecutor();
+        queuedStorage = new QueuedStorage(delegate, executor, fileExecutor);
+        queuedStorage.shutdown();
+        
+        // all shutdown methods should have been called
+        assertTrue(-1 != delegate.shutDownTime);
+        assertTrue(-1 != executor.executorShutDownTime);
+        assertTrue(-1 != fileExecutor.executorShutDownTime);
+        // delegate should have shut down last
+        assertTrue(delegate.shutDownTime > executor.executorShutDownTime);
+        assertTrue(delegate.shutDownTime > fileExecutor.executorShutDownTime);
+    }
+    
+    private static class StubStorage implements Storage {
+
+        long shutDownTime = -1;
+
+        @Override
+        public void setAgentId(UUID id) {
+            // not implemented
+            throw new AssertionError();
+        }
+
+        @Override
+        public String getAgentId() {
+            // not implementes
+            throw new AssertionError();
+        }
+
+        @Override
+        public void registerCategory(Category<?> category) {
+            // not implemented
+            throw new AssertionError();
+        }
+
+        @Override
+        public Connection getConnection() {
+            // not implemented
+            throw new AssertionError();
+        }
+
+        @Override
+        public Add createAdd(Category<?> category) {
+            // not implemented
+            throw new AssertionError();
+        }
+
+        @Override
+        public Replace createReplace(Category<?> category) {
+            // not implemented
+            throw new AssertionError();
+        }
+
+        @Override
+        public void removePojo(Remove remove) {
+            // not implemented
+            throw new AssertionError();
+        }
+
+        @Override
+        public void purge(String agentId) {
+            // not implemented
+            throw new AssertionError();
+        }
+
+        @Override
+        public long getCount(Category<?> category) {
+            // not implemented
+            throw new AssertionError();
+        }
+
+        @Override
+        public void saveFile(String filename, InputStream data) {
+            // not implemented
+            throw new AssertionError();
+
+        }
+
+        @Override
+        public InputStream loadFile(String filename) {
+            // not implemented
+            throw new AssertionError();
+        }
+
+        @Override
+        public <T extends Pojo> Query<T> createQuery(Category<T> category) {
+            // not implemented
+            throw new AssertionError();
+        }
+
+        @Override
+        public Update createUpdate(Category<?> category) {
+            // not implemented
+            throw new AssertionError();
+        }
+
+        @Override
+        public Remove createRemove() {
+            // not implemented
+            throw new AssertionError();
+        }
+
+        @Override
+        public void shutdown() {
+            shutDownTime = System.currentTimeMillis();
+            // delay shutdown just a little
+            try {
+                Thread.sleep(10);
+            } catch (InterruptedException e) {
+                Thread.currentThread().interrupt();
+            }
+        }
+        
+    }
 }