Use a list instead of a method and clean test

elastic · Jul 3, 2018 · 7be7b49 · 7be7b49
1 parent 153a97b
commit 7be7b49
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 24 deletions.
diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java
@@ -229,6 +229,7 @@ public class Watcher extends Plugin implements ActionPlugin, ScriptPlugin, Reloa
     private SetOnce<JiraService> jiraService = new SetOnce<>();
     private SetOnce<SlackService> slackService = new SetOnce<>();
     private SetOnce<PagerDutyService> pagerDutyService = new SetOnce<>();
+    List<NotificationService> reloadableServices = new ArrayList<>();
 
     public Watcher(final Settings settings) {
         this.settings = settings;
@@ -278,10 +279,16 @@ public Collection<Object> createComponents(Client client, ClusterService cluster
 
         // notification
         emailService.set(new EmailService(settings, cryptoService, clusterService.getClusterSettings()));
+        reloadableServices.add(emailService.get());
         hipChatService.set(new HipChatService(settings, httpClient, clusterService.getClusterSettings()));
+        reloadableServices.add(hipChatService.get());
         jiraService.set(new JiraService(settings, httpClient, clusterService.getClusterSettings()));
+        reloadableServices.add(jiraService.get());
         slackService.set(new SlackService(settings, httpClient, clusterService.getClusterSettings()));
+        reloadableServices.add(slackService.get());
         pagerDutyService.set(new PagerDutyService(settings, httpClient, clusterService.getClusterSettings()));
+        reloadableServices.add(pagerDutyService.get());
+
 
         TextTemplateEngine templateEngine = new TextTemplateEngine(settings, scriptService);
         Map<String, EmailAttachmentParser> emailAttachmentParsers = new HashMap<>();
@@ -624,18 +631,6 @@ public void close() throws IOException {
         IOUtils.closeWhileHandlingException(httpClient);
     }
 
-    /**
-     * Used to detect which {@link NotificationService} get reloaded. Also useful for testing the reload in tests.
-     * @return
-     */
-    protected List<NotificationService> getReloadableServices() {
-        return Arrays.asList(
-            emailService.get(),
-            hipChatService.get(),
-            jiraService.get(),
-            slackService.get(),
-            pagerDutyService.get());
-    }
 
     /**
      * Reloads all reloadable services' settings.
@@ -648,7 +643,7 @@ protected List<NotificationService> getReloadableServices() {
      */
     @Override
     public void reload(Settings settings) throws Exception {
-        for (NotificationService service : getReloadableServices()) {
+        for (NotificationService service : reloadableServices) {
             service.reload(settings);
         }
     }

diff --git a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/WatcherPluginTests.java b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/WatcherPluginTests.java
@@ -25,6 +25,9 @@
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.hasSize;
 import static org.hamcrest.Matchers.is;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
 
 public class WatcherPluginTests extends ESTestCase {
 
@@ -126,17 +129,13 @@ public void testReload() throws Exception {
             .put("xpack.watcher.enabled", false)
             .put("path.home", createTempDir())
             .build();
-        TestNotificationService service = new TestNotificationService(settings, "test");
-        Watcher watcher = new Watcher(settings) {
-            @Override
-            protected List<NotificationService> getReloadableServices() {
-                return Collections.singletonList(service);
-            }
-        };
-
-        assertFalse(service.calledCreateAccount);
-        watcher.reload(settings);
-        assertTrue(service.calledCreateAccount);
+        NotificationService mockService = mock(NotificationService.class);
+        Watcher watcher = new Watcher(settings);
+        watcher.reloadableServices.clear();
+        watcher.reloadableServices.add(mockService);
 
+        verify(mockService, times(0)).reload(settings);
+        watcher.reload(settings);
+        verify(mockService, times(1)).reload(settings);
     }
 }