From 549a5d3e733a3037066e0de6d2d66191139e5220 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Wed, 3 Aug 2016 00:07:26 -0700 Subject: [PATCH] Remove interfaces for notification services We have 4 types of notification services, and all of them have an interface with a single implementation class. They also all unnecessarily are lifecycle componenets, but the only thing start does is read the settings. This change converts all 4 notification services to classes, and makes them regular components instead of lifecycle services. Original commit: elastic/x-pack-elasticsearch@897115ae655d0b0abcd8dc97a4161e1395bb8027 --- .../org/elasticsearch/xpack/XPackPlugin.java | 7 -- .../xpack/notification/Notification.java | 48 ++++------- .../notification/NotificationModule.java | 23 +---- .../notification/email/EmailService.java | 63 ++++++++++++-- .../email/InternalEmailService.java | 85 ------------------- .../notification/hipchat/HipChatService.java | 36 ++++++-- .../hipchat/InternalHipChatService.java | 58 ------------- .../pagerduty/InternalPagerDutyService.java | 59 ------------- .../pagerduty/PagerDutyService.java | 37 ++++++-- .../slack/InternalSlackService.java | 58 ------------- .../notification/slack/SlackService.java | 35 ++++++-- ...rviceTests.java => EmailServiceTests.java} | 17 +--- .../email/ManualPublicSmtpServersTester.java | 49 +++++------ .../notification/email/ProfileTests.java | 2 +- ...iceTests.java => HipChatServiceTests.java} | 33 +++---- .../AbstractWatcherIntegrationTestCase.java | 15 +--- 16 files changed, 211 insertions(+), 414 deletions(-) delete mode 100644 elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/notification/email/InternalEmailService.java delete mode 100644 elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/notification/hipchat/InternalHipChatService.java delete mode 100644 elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/notification/pagerduty/InternalPagerDutyService.java delete mode 100644 elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/notification/slack/InternalSlackService.java rename elasticsearch/x-pack/src/test/java/org/elasticsearch/xpack/notification/email/{InternalEmailServiceTests.java => EmailServiceTests.java} (85%) rename elasticsearch/x-pack/src/test/java/org/elasticsearch/xpack/notification/hipchat/{InternalHipChatServiceTests.java => HipChatServiceTests.java} (91%) diff --git a/elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/XPackPlugin.java b/elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/XPackPlugin.java index 0531f810f93..f379054b171 100644 --- a/elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/XPackPlugin.java +++ b/elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/XPackPlugin.java @@ -180,13 +180,6 @@ public class XPackPlugin extends Plugin implements ScriptPlugin, ActionPlugin, I return modules; } - @Override - public Collection> getGuiceServiceClasses() { - ArrayList> services = new ArrayList<>(); - services.addAll(notification.nodeServices()); - return services; - } - @Override public Collection createComponents(Client client, ClusterService clusterService, ThreadPool threadPool, ResourceWatcherService resourceWatcherService) { diff --git a/elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/notification/Notification.java b/elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/notification/Notification.java index f564ea66e33..50be8264663 100644 --- a/elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/notification/Notification.java +++ b/elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/notification/Notification.java @@ -5,29 +5,23 @@ */ package org.elasticsearch.xpack.notification; -import org.elasticsearch.client.Client; -import org.elasticsearch.common.component.LifecycleComponent; -import org.elasticsearch.common.inject.Module; -import org.elasticsearch.common.settings.Setting; -import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.common.settings.SettingsModule; -import org.elasticsearch.xpack.common.http.HttpClient; -import org.elasticsearch.xpack.notification.email.EmailService; -import org.elasticsearch.xpack.notification.email.InternalEmailService; -import org.elasticsearch.xpack.notification.hipchat.HipChatService; -import org.elasticsearch.xpack.notification.hipchat.InternalHipChatService; -import org.elasticsearch.xpack.notification.pagerduty.InternalPagerDutyService; -import org.elasticsearch.xpack.notification.pagerduty.PagerDutyAccount; -import org.elasticsearch.xpack.notification.pagerduty.PagerDutyService; -import org.elasticsearch.xpack.notification.slack.InternalSlackService; -import org.elasticsearch.xpack.notification.slack.SlackService; - import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.List; +import org.elasticsearch.client.Client; +import org.elasticsearch.common.component.LifecycleComponent; +import org.elasticsearch.common.inject.Module; +import org.elasticsearch.common.settings.Setting; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.xpack.notification.email.EmailService; +import org.elasticsearch.xpack.notification.hipchat.HipChatService; +import org.elasticsearch.xpack.notification.pagerduty.PagerDutyAccount; +import org.elasticsearch.xpack.notification.pagerduty.PagerDutyService; +import org.elasticsearch.xpack.notification.slack.SlackService; + public class Notification { private final boolean transportClient; @@ -37,10 +31,10 @@ public class Notification { } public List> getSettings() { - return Arrays.asList(InternalSlackService.SLACK_ACCOUNT_SETTING, - InternalEmailService.EMAIL_ACCOUNT_SETTING, - InternalHipChatService.HIPCHAT_ACCOUNT_SETTING, - InternalPagerDutyService.PAGERDUTY_ACCOUNT_SETTING); + return Arrays.asList(SlackService.SLACK_ACCOUNT_SETTING, + EmailService.EMAIL_ACCOUNT_SETTING, + HipChatService.HIPCHAT_ACCOUNT_SETTING, + PagerDutyService.PAGERDUTY_ACCOUNT_SETTING); } public List getSettingsFilter() { @@ -54,18 +48,6 @@ public class Notification { return settingsFilter; } - public Collection> nodeServices() { - if (transportClient) { - return Collections.emptyList(); - } - return Arrays.>asList( - EmailService.class, - HipChatService.class, - SlackService.class, - PagerDutyService.class - ); - } - public Collection nodeModules() { if (transportClient) { return Collections.emptyList(); diff --git a/elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/notification/NotificationModule.java b/elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/notification/NotificationModule.java index b8777192ffc..156b2e867b5 100644 --- a/elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/notification/NotificationModule.java +++ b/elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/notification/NotificationModule.java @@ -8,17 +8,12 @@ package org.elasticsearch.xpack.notification; import org.elasticsearch.common.inject.AbstractModule; import org.elasticsearch.common.inject.multibindings.MapBinder; import org.elasticsearch.xpack.notification.email.EmailService; -import org.elasticsearch.xpack.notification.email.HtmlSanitizer; -import org.elasticsearch.xpack.notification.email.InternalEmailService; import org.elasticsearch.xpack.notification.email.attachment.DataAttachmentParser; import org.elasticsearch.xpack.notification.email.attachment.EmailAttachmentParser; import org.elasticsearch.xpack.notification.email.attachment.EmailAttachmentsParser; import org.elasticsearch.xpack.notification.email.attachment.HttpEmailAttachementParser; import org.elasticsearch.xpack.notification.hipchat.HipChatService; -import org.elasticsearch.xpack.notification.hipchat.InternalHipChatService; -import org.elasticsearch.xpack.notification.pagerduty.InternalPagerDutyService; import org.elasticsearch.xpack.notification.pagerduty.PagerDutyService; -import org.elasticsearch.xpack.notification.slack.InternalSlackService; import org.elasticsearch.xpack.notification.slack.SlackService; import java.util.HashMap; @@ -40,9 +35,7 @@ public class NotificationModule extends AbstractModule { @Override protected void configure() { - // email - bind(InternalEmailService.class).asEagerSingleton(); - bind(EmailService.class).to(InternalEmailService.class).asEagerSingleton(); + bind(EmailService.class).asEagerSingleton(); MapBinder emailParsersBinder = MapBinder.newMapBinder(binder(), String.class, EmailAttachmentParser.class); @@ -51,16 +44,8 @@ public class NotificationModule extends AbstractModule { } bind(EmailAttachmentsParser.class).asEagerSingleton(); - // hipchat - bind(InternalHipChatService.class).asEagerSingleton(); - bind(HipChatService.class).to(InternalHipChatService.class); - - // slack - bind(InternalSlackService.class).asEagerSingleton(); - bind(SlackService.class).to(InternalSlackService.class); - - // pager duty - bind(InternalPagerDutyService.class).asEagerSingleton(); - bind(PagerDutyService.class).to(InternalPagerDutyService.class); + bind(HipChatService.class).asEagerSingleton(); + bind(SlackService.class).asEagerSingleton(); + bind(PagerDutyService.class).asEagerSingleton(); } } diff --git a/elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/notification/email/EmailService.java b/elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/notification/email/EmailService.java index 08172e068ab..81cb7bc0583 100644 --- a/elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/notification/email/EmailService.java +++ b/elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/notification/email/EmailService.java @@ -5,20 +5,69 @@ */ package org.elasticsearch.xpack.notification.email; -import org.elasticsearch.common.component.LifecycleComponent; - import javax.mail.MessagingException; +import org.elasticsearch.common.Nullable; +import org.elasticsearch.common.component.AbstractComponent; +import org.elasticsearch.common.inject.Inject; +import org.elasticsearch.common.logging.ESLogger; +import org.elasticsearch.common.settings.ClusterSettings; +import org.elasticsearch.common.settings.Setting; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.xpack.security.crypto.CryptoService; + /** - * + * A component to store email credentials and handle sending email notifications. */ -public interface EmailService extends LifecycleComponent{ +public class EmailService extends AbstractComponent { - EmailSent send(Email email, Authentication auth, Profile profile) throws MessagingException; + private final CryptoService cryptoService; + public static final Setting EMAIL_ACCOUNT_SETTING = + Setting.groupSetting("xpack.notification.email.", Setting.Property.Dynamic, Setting.Property.NodeScope); - EmailSent send(Email email, Authentication auth, Profile profile, String accountName) throws MessagingException; + private volatile Accounts accounts; - class EmailSent { + @Inject + public EmailService(Settings settings, @Nullable CryptoService cryptoService, ClusterSettings clusterSettings) { + super(settings); + this.cryptoService = cryptoService; + clusterSettings.addSettingsUpdateConsumer(EMAIL_ACCOUNT_SETTING, this::setEmailAccountSettings); + setEmailAccountSettings(EMAIL_ACCOUNT_SETTING.get(settings)); + } + + private void setEmailAccountSettings(Settings settings) { + this.accounts = createAccounts(settings, logger); + } + + public EmailSent send(Email email, Authentication auth, Profile profile) throws MessagingException { + return send(email, auth, profile, (String) null); + } + + public EmailSent send(Email email, Authentication auth, Profile profile, String accountName) throws MessagingException { + Account account = accounts.account(accountName); + if (account == null) { + throw new IllegalArgumentException("failed to send email with subject [" + email.subject() + "] via account [" + accountName + + "]. account does not exist"); + } + return send(email, auth, profile, account); + } + + EmailSent send(Email email, Authentication auth, Profile profile, Account account) throws MessagingException { + assert account != null; + try { + email = account.send(email, auth, profile); + } catch (MessagingException me) { + throw new MessagingException("failed to send email with subject [" + email.subject() + "] via account [" + account.name() + + "]", me); + } + return new EmailSent(account.name(), email); + } + + protected Accounts createAccounts(Settings settings, ESLogger logger) { + return new Accounts(settings, cryptoService, logger); + } + + public static class EmailSent { private final String account; private final Email email; diff --git a/elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/notification/email/InternalEmailService.java b/elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/notification/email/InternalEmailService.java deleted file mode 100644 index 8fdbf9adc59..00000000000 --- a/elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/notification/email/InternalEmailService.java +++ /dev/null @@ -1,85 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * 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.email; - -import javax.mail.MessagingException; - -import org.elasticsearch.ElasticsearchException; -import org.elasticsearch.common.Nullable; -import org.elasticsearch.common.component.AbstractLifecycleComponent; -import org.elasticsearch.common.inject.Inject; -import org.elasticsearch.common.logging.ESLogger; -import org.elasticsearch.common.settings.ClusterSettings; -import org.elasticsearch.common.settings.Setting; -import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.xpack.security.crypto.CryptoService; - -/** - * - */ -public class InternalEmailService extends AbstractLifecycleComponent implements EmailService { - - private final CryptoService cryptoService; - public static final Setting EMAIL_ACCOUNT_SETTING = - Setting.groupSetting("xpack.notification.email.", Setting.Property.Dynamic, Setting.Property.NodeScope); - - private volatile Accounts accounts; - - @Inject - public InternalEmailService(Settings settings, @Nullable CryptoService cryptoService, ClusterSettings clusterSettings) { - super(settings); - this.cryptoService = cryptoService; - clusterSettings.addSettingsUpdateConsumer(EMAIL_ACCOUNT_SETTING, this::setEmailAccountSettings); - setEmailAccountSettings(EMAIL_ACCOUNT_SETTING.get(settings)); - } - - private void setEmailAccountSettings(Settings settings) { - this.accounts = createAccounts(settings, logger); - } - - @Override - protected void doStart() throws ElasticsearchException { - } - - @Override - protected void doStop() throws ElasticsearchException { - } - - @Override - protected void doClose() throws ElasticsearchException { - } - - @Override - public EmailSent send(Email email, Authentication auth, Profile profile) throws MessagingException { - return send(email, auth, profile, (String) null); - } - - @Override - public EmailSent send(Email email, Authentication auth, Profile profile, String accountName) throws MessagingException { - Account account = accounts.account(accountName); - if (account == null) { - throw new IllegalArgumentException("failed to send email with subject [" + email.subject() + "] via account [" + accountName - + "]. account does not exist"); - } - return send(email, auth, profile, account); - } - - EmailSent send(Email email, Authentication auth, Profile profile, Account account) throws MessagingException { - assert account != null; - try { - email = account.send(email, auth, profile); - } catch (MessagingException me) { - throw new MessagingException("failed to send email with subject [" + email.subject() + "] via account [" + account.name() + - "]", me); - } - return new EmailSent(account.name(), email); - } - - protected Accounts createAccounts(Settings settings, ESLogger logger) { - return new Accounts(settings, cryptoService, logger); - } - -} diff --git a/elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/notification/hipchat/HipChatService.java b/elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/notification/hipchat/HipChatService.java index eaad3872d9b..510f8c4d568 100644 --- a/elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/notification/hipchat/HipChatService.java +++ b/elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/notification/hipchat/HipChatService.java @@ -5,22 +5,48 @@ */ package org.elasticsearch.xpack.notification.hipchat; -import org.elasticsearch.common.component.LifecycleComponent; +import org.elasticsearch.common.component.AbstractComponent; +import org.elasticsearch.common.inject.Inject; +import org.elasticsearch.common.settings.ClusterSettings; +import org.elasticsearch.common.settings.Setting; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.xpack.common.http.HttpClient; /** - * + * A component to store hipchat credentials. */ -public interface HipChatService extends LifecycleComponent { +public class HipChatService extends AbstractComponent { + + private final HttpClient httpClient; + private volatile HipChatAccounts accounts; + public static final Setting HIPCHAT_ACCOUNT_SETTING = + Setting.groupSetting("xpack.notification.hipchat.", Setting.Property.Dynamic, Setting.Property.NodeScope); + + @Inject + public HipChatService(Settings settings, HttpClient httpClient, ClusterSettings clusterSettings) { + super(settings); + this.httpClient = httpClient; + clusterSettings.addSettingsUpdateConsumer(HIPCHAT_ACCOUNT_SETTING, this::setHipchatAccountSetting); + setHipchatAccountSetting(HIPCHAT_ACCOUNT_SETTING.get(settings)); + } + + private void setHipchatAccountSetting(Settings setting) { + accounts = new HipChatAccounts(setting, httpClient, logger); + } /** * @return The default hipchat account. */ - HipChatAccount getDefaultAccount(); + public HipChatAccount getDefaultAccount() { + return accounts.account(null); + } /** * @return The account identified by the given name. If the given name is {@code null} the default * account will be returned. */ - HipChatAccount getAccount(String accountName); + public HipChatAccount getAccount(String name) { + return accounts.account(name); + } } diff --git a/elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/notification/hipchat/InternalHipChatService.java b/elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/notification/hipchat/InternalHipChatService.java deleted file mode 100644 index 9d9400f7c04..00000000000 --- a/elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/notification/hipchat/InternalHipChatService.java +++ /dev/null @@ -1,58 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * 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.hipchat; - -import org.elasticsearch.common.component.AbstractLifecycleComponent; -import org.elasticsearch.common.inject.Inject; -import org.elasticsearch.common.settings.ClusterSettings; -import org.elasticsearch.common.settings.Setting; -import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.xpack.common.http.HttpClient; - -/** - * - */ -public class InternalHipChatService extends AbstractLifecycleComponent implements HipChatService { - - private final HttpClient httpClient; - private volatile HipChatAccounts accounts; - public static final Setting HIPCHAT_ACCOUNT_SETTING = - Setting.groupSetting("xpack.notification.hipchat.", Setting.Property.Dynamic, Setting.Property.NodeScope); - - @Inject - public InternalHipChatService(Settings settings, HttpClient httpClient, ClusterSettings clusterSettings) { - super(settings); - this.httpClient = httpClient; - clusterSettings.addSettingsUpdateConsumer(HIPCHAT_ACCOUNT_SETTING, this::setHipchatAccountSetting); - } - - @Override - protected void doStart() { - setHipchatAccountSetting(HIPCHAT_ACCOUNT_SETTING.get(settings)); - } - - @Override - protected void doStop() { - } - - @Override - protected void doClose() { - } - - private void setHipchatAccountSetting(Settings setting) { - accounts = new HipChatAccounts(setting, httpClient, logger); - } - - @Override - public HipChatAccount getDefaultAccount() { - return accounts.account(null); - } - - @Override - public HipChatAccount getAccount(String name) { - return accounts.account(name); - } -} diff --git a/elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/notification/pagerduty/InternalPagerDutyService.java b/elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/notification/pagerduty/InternalPagerDutyService.java deleted file mode 100644 index 5c3e16d2ba8..00000000000 --- a/elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/notification/pagerduty/InternalPagerDutyService.java +++ /dev/null @@ -1,59 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * 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.pagerduty; - -import org.elasticsearch.common.component.AbstractLifecycleComponent; -import org.elasticsearch.common.inject.Inject; -import org.elasticsearch.common.settings.ClusterSettings; -import org.elasticsearch.common.settings.Setting; -import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.xpack.common.http.HttpClient; - -/** - * - */ -public class InternalPagerDutyService extends AbstractLifecycleComponent implements PagerDutyService { - - public static final Setting PAGERDUTY_ACCOUNT_SETTING = - Setting.groupSetting("xpack.notification.pagerduty.", Setting.Property.Dynamic, Setting.Property.NodeScope); - - private final HttpClient httpClient; - private volatile PagerDutyAccounts accounts; - - @Inject - public InternalPagerDutyService(Settings settings, HttpClient httpClient, ClusterSettings clusterSettings) { - super(settings); - this.httpClient = httpClient; - clusterSettings.addSettingsUpdateConsumer(PAGERDUTY_ACCOUNT_SETTING, this::setPagerDutyAccountSetting); - } - - @Override - protected void doStart() { - setPagerDutyAccountSetting(PAGERDUTY_ACCOUNT_SETTING.get(settings)); - } - - @Override - protected void doStop() { - } - - @Override - protected void doClose() { - } - - private void setPagerDutyAccountSetting(Settings settings) { - accounts = new PagerDutyAccounts(settings, httpClient, logger); - } - - @Override - public PagerDutyAccount getDefaultAccount() { - return accounts.account(null); - } - - @Override - public PagerDutyAccount getAccount(String name) { - return accounts.account(name); - } -} diff --git a/elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/notification/pagerduty/PagerDutyService.java b/elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/notification/pagerduty/PagerDutyService.java index 73154b18bae..3a2e6728a10 100644 --- a/elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/notification/pagerduty/PagerDutyService.java +++ b/elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/notification/pagerduty/PagerDutyService.java @@ -5,14 +5,41 @@ */ package org.elasticsearch.xpack.notification.pagerduty; -import org.elasticsearch.common.component.LifecycleComponent; +import org.elasticsearch.common.component.AbstractComponent; +import org.elasticsearch.common.inject.Inject; +import org.elasticsearch.common.settings.ClusterSettings; +import org.elasticsearch.common.settings.Setting; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.xpack.common.http.HttpClient; /** - * + * A component to store pagerduty credentials. */ -public interface PagerDutyService extends LifecycleComponent { +public class PagerDutyService extends AbstractComponent { - PagerDutyAccount getDefaultAccount(); + public static final Setting PAGERDUTY_ACCOUNT_SETTING = + Setting.groupSetting("xpack.notification.pagerduty.", Setting.Property.Dynamic, Setting.Property.NodeScope); - PagerDutyAccount getAccount(String accountName); + private final HttpClient httpClient; + private volatile PagerDutyAccounts accounts; + + @Inject + public PagerDutyService(Settings settings, HttpClient httpClient, ClusterSettings clusterSettings) { + super(settings); + this.httpClient = httpClient; + clusterSettings.addSettingsUpdateConsumer(PAGERDUTY_ACCOUNT_SETTING, this::setPagerDutyAccountSetting); + setPagerDutyAccountSetting(PAGERDUTY_ACCOUNT_SETTING.get(settings)); + } + + private void setPagerDutyAccountSetting(Settings settings) { + accounts = new PagerDutyAccounts(settings, httpClient, logger); + } + + public PagerDutyAccount getDefaultAccount() { + return accounts.account(null); + } + + public PagerDutyAccount getAccount(String name) { + return accounts.account(name); + } } diff --git a/elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/notification/slack/InternalSlackService.java b/elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/notification/slack/InternalSlackService.java deleted file mode 100644 index fe4e1d0658f..00000000000 --- a/elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/notification/slack/InternalSlackService.java +++ /dev/null @@ -1,58 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * 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.slack; - -import org.elasticsearch.common.component.AbstractLifecycleComponent; -import org.elasticsearch.common.inject.Inject; -import org.elasticsearch.common.settings.ClusterSettings; -import org.elasticsearch.common.settings.Setting; -import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.xpack.common.http.HttpClient; - -/** - * - */ -public class InternalSlackService extends AbstractLifecycleComponent implements SlackService { - - private final HttpClient httpClient; - public static final Setting SLACK_ACCOUNT_SETTING = - Setting.groupSetting("xpack.notification.slack.", Setting.Property.Dynamic, Setting.Property.NodeScope); - private volatile SlackAccounts accounts; - - @Inject - public InternalSlackService(Settings settings, HttpClient httpClient, ClusterSettings clusterSettings) { - super(settings); - this.httpClient = httpClient; - clusterSettings.addSettingsUpdateConsumer(SLACK_ACCOUNT_SETTING, this::setSlackAccountSetting); - } - - @Override - protected void doStart() { - setSlackAccountSetting(SLACK_ACCOUNT_SETTING.get(settings)); - } - - @Override - protected void doStop() { - } - - @Override - protected void doClose() { - } - - @Override - public SlackAccount getDefaultAccount() { - return accounts.account(null); - } - - private void setSlackAccountSetting(Settings setting) { - accounts = new SlackAccounts(setting, httpClient, logger); - } - - @Override - public SlackAccount getAccount(String name) { - return accounts.account(name); - } -} diff --git a/elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/notification/slack/SlackService.java b/elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/notification/slack/SlackService.java index f77006f3837..f93e91243f7 100644 --- a/elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/notification/slack/SlackService.java +++ b/elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/notification/slack/SlackService.java @@ -5,22 +5,47 @@ */ package org.elasticsearch.xpack.notification.slack; -import org.elasticsearch.common.component.LifecycleComponent; +import org.elasticsearch.common.component.AbstractComponent; +import org.elasticsearch.common.inject.Inject; +import org.elasticsearch.common.settings.ClusterSettings; +import org.elasticsearch.common.settings.Setting; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.xpack.common.http.HttpClient; /** - * + * A component to store slack credentials. */ -public interface SlackService extends LifecycleComponent { +public class SlackService extends AbstractComponent { + private final HttpClient httpClient; + public static final Setting SLACK_ACCOUNT_SETTING = + Setting.groupSetting("xpack.notification.slack.", Setting.Property.Dynamic, Setting.Property.NodeScope); + private volatile SlackAccounts accounts; + + @Inject + public SlackService(Settings settings, HttpClient httpClient, ClusterSettings clusterSettings) { + super(settings); + this.httpClient = httpClient; + clusterSettings.addSettingsUpdateConsumer(SLACK_ACCOUNT_SETTING, this::setSlackAccountSetting); + setSlackAccountSetting(SLACK_ACCOUNT_SETTING.get(settings)); + } /** * @return The default slack account. */ - SlackAccount getDefaultAccount(); + public SlackAccount getDefaultAccount() { + return accounts.account(null); + } + + private void setSlackAccountSetting(Settings setting) { + accounts = new SlackAccounts(setting, httpClient, logger); + } /** * @return The account identified by the given name. If the given name is {@code null} the default * account will be returned. */ - SlackAccount getAccount(String accountName); + public SlackAccount getAccount(String name) { + return accounts.account(name); + } } diff --git a/elasticsearch/x-pack/src/test/java/org/elasticsearch/xpack/notification/email/InternalEmailServiceTests.java b/elasticsearch/x-pack/src/test/java/org/elasticsearch/xpack/notification/email/EmailServiceTests.java similarity index 85% rename from elasticsearch/x-pack/src/test/java/org/elasticsearch/xpack/notification/email/InternalEmailServiceTests.java rename to elasticsearch/x-pack/src/test/java/org/elasticsearch/xpack/notification/email/EmailServiceTests.java index 952d5d7c301..b85ee604e9c 100644 --- a/elasticsearch/x-pack/src/test/java/org/elasticsearch/xpack/notification/email/InternalEmailServiceTests.java +++ b/elasticsearch/x-pack/src/test/java/org/elasticsearch/xpack/notification/email/EmailServiceTests.java @@ -22,29 +22,20 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; -/** - * - */ -public class InternalEmailServiceTests extends ESTestCase { - private InternalEmailService service; +public class EmailServiceTests extends ESTestCase { + private EmailService service; private Accounts accounts; @Before public void init() throws Exception { accounts = mock(Accounts.class); - service = new InternalEmailService(Settings.EMPTY, null, - new ClusterSettings(Settings.EMPTY, Collections.singleton(InternalEmailService.EMAIL_ACCOUNT_SETTING))) { + service = new EmailService(Settings.EMPTY, null, + new ClusterSettings(Settings.EMPTY, Collections.singleton(EmailService.EMAIL_ACCOUNT_SETTING))) { @Override protected Accounts createAccounts(Settings settings, ESLogger logger) { return accounts; } }; - service.start(); - } - - @After - public void cleanup() throws Exception { - service.stop(); } public void testSend() throws Exception { diff --git a/elasticsearch/x-pack/src/test/java/org/elasticsearch/xpack/notification/email/ManualPublicSmtpServersTester.java b/elasticsearch/x-pack/src/test/java/org/elasticsearch/xpack/notification/email/ManualPublicSmtpServersTester.java index 7df1aa0f019..312c1beb628 100644 --- a/elasticsearch/x-pack/src/test/java/org/elasticsearch/xpack/notification/email/ManualPublicSmtpServersTester.java +++ b/elasticsearch/x-pack/src/test/java/org/elasticsearch/xpack/notification/email/ManualPublicSmtpServersTester.java @@ -89,42 +89,35 @@ public class ManualPublicSmtpServersTester { static void test(Profile profile, Settings.Builder settingsBuilder) throws Exception { String path = "/org/elasticsearch/xpack/watcher/actions/email/service/logo.png"; - if (InternalEmailServiceTests.class.getResourceAsStream(path) == null) { + if (EmailServiceTests.class.getResourceAsStream(path) == null) { throw new ElasticsearchException("Could not find logo at path {}", path); } - InternalEmailService service = startEmailService(settingsBuilder); - try { + EmailService service = startEmailService(settingsBuilder); + ToXContent content = (xContentBuilder, params) -> xContentBuilder.startObject() + .field("key1", "value1") + .field("key2", "value2") + .field("key3", "value3") + .endObject(); - ToXContent content = (xContentBuilder, params) -> xContentBuilder.startObject() - .field("key1", "value1") - .field("key2", "value2") - .field("key3", "value3") - .endObject(); + Email email = Email.builder() + .id("_id") + .subject("_subject") + .textBody("_text_body") + .htmlBody("html body

") + .attach(new Attachment.XContent.Yaml("test.yml", content)) + .attach(new Attachment.Stream("logo.png", "logo.png", true, + () -> EmailServiceTests.class.getResourceAsStream(path))) + .build(); - Email email = Email.builder() - .id("_id") - .subject("_subject") - .textBody("_text_body") - .htmlBody("html body

") - .attach(new Attachment.XContent.Yaml("test.yml", content)) - .attach(new Attachment.Stream("logo.png", "logo.png", true, - () -> InternalEmailServiceTests.class.getResourceAsStream(path))) - .build(); + EmailService.EmailSent sent = service.send(email, null, profile); - EmailService.EmailSent sent = service.send(email, null, profile); - - terminal.println(String.format(Locale.ROOT, "email sent via account [%s]", sent.account())); - } finally { - service.stop(); - } + terminal.println(String.format(Locale.ROOT, "email sent via account [%s]", sent.account())); } - static InternalEmailService startEmailService(Settings.Builder builder) { + static EmailService startEmailService(Settings.Builder builder) { Settings settings = builder.build(); - InternalEmailService service = new InternalEmailService(settings, null, - new ClusterSettings(settings, Collections.singleton(InternalEmailService.EMAIL_ACCOUNT_SETTING))); - service.start(); - return service; + return new EmailService(settings, null, + new ClusterSettings(settings, Collections.singleton(EmailService.EMAIL_ACCOUNT_SETTING))); } } diff --git a/elasticsearch/x-pack/src/test/java/org/elasticsearch/xpack/notification/email/ProfileTests.java b/elasticsearch/x-pack/src/test/java/org/elasticsearch/xpack/notification/email/ProfileTests.java index 304de681427..c2b0897d854 100644 --- a/elasticsearch/x-pack/src/test/java/org/elasticsearch/xpack/notification/email/ProfileTests.java +++ b/elasticsearch/x-pack/src/test/java/org/elasticsearch/xpack/notification/email/ProfileTests.java @@ -22,7 +22,7 @@ public class ProfileTests extends ESTestCase { public void testThatInlineAttachmentsAreCreated() throws Exception { String path = "/org/elasticsearch/xpack/watcher/actions/email/service/logo.png"; Attachment attachment = new Attachment.Stream("inline.png", "inline.png", true, - () -> InternalEmailServiceTests.class.getResourceAsStream(path)); + () -> EmailServiceTests.class.getResourceAsStream(path)); Email email = Email.builder() .id("foo") diff --git a/elasticsearch/x-pack/src/test/java/org/elasticsearch/xpack/notification/hipchat/InternalHipChatServiceTests.java b/elasticsearch/x-pack/src/test/java/org/elasticsearch/xpack/notification/hipchat/HipChatServiceTests.java similarity index 91% rename from elasticsearch/x-pack/src/test/java/org/elasticsearch/xpack/notification/hipchat/InternalHipChatServiceTests.java rename to elasticsearch/x-pack/src/test/java/org/elasticsearch/xpack/notification/hipchat/HipChatServiceTests.java index bea6d4fece1..04452d58bb3 100644 --- a/elasticsearch/x-pack/src/test/java/org/elasticsearch/xpack/notification/hipchat/InternalHipChatServiceTests.java +++ b/elasticsearch/x-pack/src/test/java/org/elasticsearch/xpack/notification/hipchat/HipChatServiceTests.java @@ -26,7 +26,7 @@ import static org.mockito.Mockito.mock; /** * */ -public class InternalHipChatServiceTests extends ESTestCase { +public class HipChatServiceTests extends ESTestCase { private HttpClient httpClient; @Before @@ -53,9 +53,8 @@ public class InternalHipChatServiceTests extends ESTestCase { settingsBuilder.put("xpack.notification.hipchat.account." + accountName + ".port", port); } buildMessageDefaults(accountName, settingsBuilder, defaultRoom, null, defaultFrom, defaultColor, defaultFormat, defaultNotify); - InternalHipChatService service = new InternalHipChatService(settingsBuilder.build(), httpClient, - new ClusterSettings(settingsBuilder.build(), Collections.singleton(InternalHipChatService.HIPCHAT_ACCOUNT_SETTING))); - service.start(); + HipChatService service = new HipChatService(settingsBuilder.build(), httpClient, + new ClusterSettings(settingsBuilder.build(), Collections.singleton(HipChatService.HIPCHAT_ACCOUNT_SETTING))); HipChatAccount account = service.getAccount(accountName); assertThat(account, notNullValue()); @@ -102,9 +101,8 @@ public class InternalHipChatServiceTests extends ESTestCase { settingsBuilder.put("xpack.notification.hipchat.account." + accountName + ".port", port); } buildMessageDefaults(accountName, settingsBuilder, null, null, defaultFrom, defaultColor, defaultFormat, defaultNotify); - InternalHipChatService service = new InternalHipChatService(settingsBuilder.build(), httpClient, - new ClusterSettings(settingsBuilder.build(), Collections.singleton(InternalHipChatService.HIPCHAT_ACCOUNT_SETTING))); - service.start(); + HipChatService service = new HipChatService(settingsBuilder.build(), httpClient, + new ClusterSettings(settingsBuilder.build(), Collections.singleton(HipChatService.HIPCHAT_ACCOUNT_SETTING))); HipChatAccount account = service.getAccount(accountName); assertThat(account, notNullValue()); @@ -131,13 +129,10 @@ public class InternalHipChatServiceTests extends ESTestCase { .put("xpack.notification.hipchat.account." + accountName + ".profile", HipChatAccount.Profile.INTEGRATION.value()) .put("xpack.notification.hipchat.account." + accountName + ".auth_token", "_token"); - try (InternalHipChatService service = new InternalHipChatService(settingsBuilder.build(), httpClient, - new ClusterSettings(settingsBuilder.build(), Collections.singleton(InternalHipChatService.HIPCHAT_ACCOUNT_SETTING)))) { - service.start(); - fail("Expected SettingsException"); - } catch (SettingsException e) { - assertThat(e.getMessage(), containsString("missing required [room] setting for [integration] account profile")); - } + SettingsException e = expectThrows(SettingsException.class, () -> + new HipChatService(settingsBuilder.build(), httpClient, + new ClusterSettings(settingsBuilder.build(), Collections.singleton(HipChatService.HIPCHAT_ACCOUNT_SETTING)))); + assertThat(e.getMessage(), containsString("missing required [room] setting for [integration] account profile")); } public void testSingleAccountUser() throws Exception { @@ -159,9 +154,8 @@ public class InternalHipChatServiceTests extends ESTestCase { settingsBuilder.put("xpack.notification.hipchat.account." + accountName + ".port", port); } buildMessageDefaults(accountName, settingsBuilder, defaultRoom, defaultUser, null, defaultColor, defaultFormat, defaultNotify); - InternalHipChatService service = new InternalHipChatService(settingsBuilder.build(), httpClient, - new ClusterSettings(settingsBuilder.build(), Collections.singleton(InternalHipChatService.HIPCHAT_ACCOUNT_SETTING))); - service.start(); + HipChatService service = new HipChatService(settingsBuilder.build(), httpClient, + new ClusterSettings(settingsBuilder.build(), Collections.singleton(HipChatService.HIPCHAT_ACCOUNT_SETTING))); HipChatAccount account = service.getAccount(accountName); assertThat(account, notNullValue()); @@ -221,9 +215,8 @@ public class InternalHipChatServiceTests extends ESTestCase { buildMessageDefaults(name, settingsBuilder, null, null, null, defaultColor, defaultFormat, defaultNotify); } - InternalHipChatService service = new InternalHipChatService(settingsBuilder.build(), httpClient, - new ClusterSettings(settingsBuilder.build(), Collections.singleton(InternalHipChatService.HIPCHAT_ACCOUNT_SETTING))); - service.start(); + HipChatService service = new HipChatService(settingsBuilder.build(), httpClient, + new ClusterSettings(settingsBuilder.build(), Collections.singleton(HipChatService.HIPCHAT_ACCOUNT_SETTING))); for (int i = 0; i < 5; i++) { String name = "_a" + i; diff --git a/elasticsearch/x-pack/watcher/src/test/java/org/elasticsearch/xpack/watcher/test/AbstractWatcherIntegrationTestCase.java b/elasticsearch/x-pack/watcher/src/test/java/org/elasticsearch/xpack/watcher/test/AbstractWatcherIntegrationTestCase.java index 817f1c4f305..bd16518063c 100644 --- a/elasticsearch/x-pack/watcher/src/test/java/org/elasticsearch/xpack/watcher/test/AbstractWatcherIntegrationTestCase.java +++ b/elasticsearch/x-pack/watcher/src/test/java/org/elasticsearch/xpack/watcher/test/AbstractWatcherIntegrationTestCase.java @@ -17,6 +17,7 @@ import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; import org.elasticsearch.cluster.routing.IndexRoutingTable; import org.elasticsearch.common.component.AbstractLifecycleComponent; import org.elasticsearch.common.io.Streams; +import org.elasticsearch.common.settings.ClusterSettings; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.Callback; import org.elasticsearch.common.xcontent.XContentHelper; @@ -588,10 +589,11 @@ public abstract class AbstractWatcherIntegrationTestCase extends ESIntegTestCase assertThat("watcher should only run on the elected master node, but it is running on [" + running + "] nodes", running, equalTo(1)); } - public static class NoopEmailService extends AbstractLifecycleComponent implements EmailService { + public static class NoopEmailService extends EmailService { public NoopEmailService() { - super(Settings.EMPTY); + super(Settings.EMPTY, null, + new ClusterSettings(Settings.EMPTY, Collections.singleton(EmailService.EMAIL_ACCOUNT_SETTING))); } @Override @@ -603,15 +605,6 @@ public abstract class AbstractWatcherIntegrationTestCase extends ESIntegTestCase public EmailSent send(Email email, Authentication auth, Profile profile, String accountName) { return new EmailSent(accountName, email); } - - @Override - protected void doStart() {} - - @Override - protected void doStop() {} - - @Override - protected void doClose() {} } protected static class TimeWarp {