Consolidate watcher setting update registration (#31762)

Previously the call to register a listener for settings updates was in each individual service, rather than in the notification service itself. This change ensures that each child of the notification service gets registered with the settings update consumer.
elastic · Jul 3, 2018 · e65115a · e65115a
1 parent dc869aa
commit e65115a
Show file tree

Hide file tree

Showing 7 changed files with 18 additions and 12 deletions.
diff --git a/...tcher/src/main/java/org/elasticsearch/xpack/watcher/notification/NotificationService.java b/...tcher/src/main/java/org/elasticsearch/xpack/watcher/notification/NotificationService.java
@@ -7,11 +7,14 @@
 
 import org.elasticsearch.common.collect.Tuple;
 import org.elasticsearch.common.component.AbstractComponent;
+import org.elasticsearch.common.settings.ClusterSettings;
+import org.elasticsearch.common.settings.Setting;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.settings.SettingsException;
 
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 import java.util.function.BiFunction;
 
@@ -25,7 +28,14 @@ public abstract class NotificationService<Account> extends AbstractComponent {
     private Map<String, Account> accounts;
     private Account defaultAccount;
 
-    public NotificationService(Settings settings, String type) {
+    public NotificationService(Settings settings, String type,
+                               ClusterSettings clusterSettings, List<Setting<?>> pluginSettings) {
+        this(settings, type);
+        clusterSettings.addSettingsUpdateConsumer(this::setAccountSetting, pluginSettings);
+    }
+
+    // Used for testing only
+    NotificationService(Settings settings, String type) {
         super(settings);
         this.type = type;
     }

diff --git a/...atcher/src/main/java/org/elasticsearch/xpack/watcher/notification/email/EmailService.java b/...atcher/src/main/java/org/elasticsearch/xpack/watcher/notification/email/EmailService.java
@@ -94,9 +94,8 @@ public class EmailService extends NotificationService<Account> {
     private final CryptoService cryptoService;
 
     public EmailService(Settings settings, @Nullable CryptoService cryptoService, ClusterSettings clusterSettings) {
-        super(settings, "email");
+        super(settings, "email", clusterSettings, EmailService.getSettings());
         this.cryptoService = cryptoService;
-        clusterSettings.addSettingsUpdateConsumer(this::setAccountSetting, getSettings());
         // ensure logging of setting changes
         clusterSettings.addSettingsUpdateConsumer(SETTING_DEFAULT_ACCOUNT, (s) -> {});
         clusterSettings.addAffixUpdateConsumer(SETTING_PROFILE, (s, o) -> {}, (s, o) -> {});

diff --git a/...er/src/main/java/org/elasticsearch/xpack/watcher/notification/hipchat/HipChatService.java b/...er/src/main/java/org/elasticsearch/xpack/watcher/notification/hipchat/HipChatService.java
@@ -65,9 +65,8 @@ public class HipChatService extends NotificationService<HipChatAccount> {
     private HipChatServer defaultServer;
 
     public HipChatService(Settings settings, HttpClient httpClient, ClusterSettings clusterSettings) {
-        super(settings, "hipchat");
+        super(settings, "hipchat", clusterSettings, HipChatService.getSettings());
         this.httpClient = httpClient;
-        clusterSettings.addSettingsUpdateConsumer(this::setAccountSetting, getSettings());
         // ensure logging of setting changes
         clusterSettings.addSettingsUpdateConsumer(SETTING_DEFAULT_ACCOUNT, (s) -> {});
         clusterSettings.addSettingsUpdateConsumer(SETTING_DEFAULT_HOST, (s) -> {});

diff --git a/.../watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/jira/JiraService.java b/.../watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/jira/JiraService.java
@@ -60,9 +60,8 @@ public class JiraService extends NotificationService<JiraAccount> {
     private final HttpClient httpClient;
 
     public JiraService(Settings settings, HttpClient httpClient, ClusterSettings clusterSettings) {
-        super(settings, "jira");
+        super(settings, "jira", clusterSettings, JiraService.getSettings());
         this.httpClient = httpClient;
-        clusterSettings.addSettingsUpdateConsumer(this::setAccountSetting, getSettings());
         // ensure logging of setting changes
         clusterSettings.addSettingsUpdateConsumer(SETTING_DEFAULT_ACCOUNT, (s) -> {});
         clusterSettings.addAffixUpdateConsumer(SETTING_ALLOW_HTTP, (s, o) -> {}, (s, o) -> {});

diff --git a/...rc/main/java/org/elasticsearch/xpack/watcher/notification/pagerduty/PagerDutyService.java b/...rc/main/java/org/elasticsearch/xpack/watcher/notification/pagerduty/PagerDutyService.java
@@ -39,7 +39,7 @@ public class PagerDutyService extends NotificationService<PagerDutyAccount> {
     private final HttpClient httpClient;
 
     public PagerDutyService(Settings settings, HttpClient httpClient, ClusterSettings clusterSettings) {
-        super(settings, "pagerduty");
+        super(settings, "pagerduty", clusterSettings, PagerDutyService.getSettings());
         this.httpClient = httpClient;
         clusterSettings.addSettingsUpdateConsumer(SETTING_DEFAULT_ACCOUNT, (s) -> {});
         clusterSettings.addAffixUpdateConsumer(SETTING_SERVICE_API_KEY, (s, o) -> {}, (s, o) -> {});

diff --git a/...atcher/src/main/java/org/elasticsearch/xpack/watcher/notification/slack/SlackService.java b/...atcher/src/main/java/org/elasticsearch/xpack/watcher/notification/slack/SlackService.java
@@ -39,9 +39,8 @@ public class SlackService extends NotificationService<SlackAccount> {
     private final HttpClient httpClient;
 
     public SlackService(Settings settings, HttpClient httpClient, ClusterSettings clusterSettings) {
-        super(settings, "slack");
+        super(settings, "slack", clusterSettings, SlackService.getSettings());
         this.httpClient = httpClient;
-        clusterSettings.addSettingsUpdateConsumer(this::setAccountSetting, getSettings());
         clusterSettings.addSettingsUpdateConsumer(SETTING_DEFAULT_ACCOUNT, (s) -> {});
         clusterSettings.addAffixUpdateConsumer(SETTING_URL, (s, o) -> {}, (s, o) -> {});
         clusterSettings.addAffixUpdateConsumer(SETTING_URL_SECURE, (s, o) -> {}, (s, o) -> {});

diff --git a/...otification/NotificationServiceTests.java → ...otification/NotificationServiceTests.java b/...otification/NotificationServiceTests.java → ...otification/NotificationServiceTests.java
@@ -3,7 +3,7 @@
  * or more contributor license agreements. Licensed under the Elastic License;
  * you may not use this file except in compliance with the Elastic License.
  */
-package org.elasticsearch.xpack.notification;
+package org.elasticsearch.xpack.watcher.notification;
 
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.settings.SettingsException;
@@ -90,4 +90,4 @@ protected String createAccount(String name, Settings accountSettings) {
             return name;
         }
     }
-}
+}