From 36105231c35e62ea198df5296456aa7e55823562 Mon Sep 17 00:00:00 2001 From: Alexander Reelsen Date: Wed, 15 Nov 2017 11:51:23 +0100 Subject: [PATCH] Watcher: Return useful error message when no accounts are found (elastic/x-pack-elasticsearch#2897) When there were no accounts configured, watcher returned a cryptic error message containing 'null' in the description. This fix returns a more clear error message. On top a dedicated NotificationServiceTests class was added to remove redundant test cases in the hipchat/jira/slack unit tests, that all basically tested NotificationService capabilties. relates elastic/x-pack-elasticsearch#2666 Original commit: elastic/x-pack-elasticsearch@45d0d1df318cf99ca27c2e3dca9762f0778c2b5f --- .../notification/NotificationService.java | 8 +- .../notification/email/EmailService.java | 2 +- .../notification/hipchat/HipChatService.java | 2 +- .../notification/jira/JiraService.java | 2 +- .../pagerduty/PagerDutyService.java | 3 +- .../notification/slack/SlackService.java | 6 +- .../NotificationServiceTests.java | 93 +++++++++++ .../actions/jira/JiraActionFactoryTests.java | 21 --- .../hipchat/HipChatAccountsTests.java | 110 +------------ .../notification/jira/JiraAccountTests.java | 86 ----------- .../slack/SlackAccountsTests.java | 144 ------------------ 11 files changed, 108 insertions(+), 369 deletions(-) create mode 100644 plugin/src/test/java/org/elasticsearch/xpack/notification/NotificationServiceTests.java delete mode 100644 plugin/src/test/java/org/elasticsearch/xpack/watcher/notification/slack/SlackAccountsTests.java diff --git a/plugin/src/main/java/org/elasticsearch/xpack/watcher/notification/NotificationService.java b/plugin/src/main/java/org/elasticsearch/xpack/watcher/notification/NotificationService.java index 5e2d06a8510..60ae0240d52 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/watcher/notification/NotificationService.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/watcher/notification/NotificationService.java @@ -20,12 +20,14 @@ import java.util.function.BiFunction; */ public abstract class NotificationService extends AbstractComponent { + private final String type; // both are guarded by this private Map accounts; protected Account defaultAccount; - public NotificationService(Settings settings) { + public NotificationService(Settings settings, String type) { super(settings); + this.type = type; } protected synchronized void setAccountSetting(Settings settings) { @@ -46,6 +48,10 @@ public abstract class NotificationService extends AbstractComponent { defaultAccount = this.defaultAccount; } Account theAccount = accounts.getOrDefault(name, defaultAccount); + if (theAccount == null && name == null) { + throw new IllegalArgumentException("no accounts of type [" + type + "] configured. " + + "Please set up an account using the [xpack.notification." + type +"] settings"); + } if (theAccount == null) { throw new IllegalArgumentException("no account found for name: [" + name + "]"); } diff --git a/plugin/src/main/java/org/elasticsearch/xpack/watcher/notification/email/EmailService.java b/plugin/src/main/java/org/elasticsearch/xpack/watcher/notification/email/EmailService.java index b0bb870c89f..6a655be7162 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/watcher/notification/email/EmailService.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/watcher/notification/email/EmailService.java @@ -24,7 +24,7 @@ public class EmailService extends NotificationService { Setting.groupSetting("xpack.notification.email.", Setting.Property.Dynamic, Setting.Property.NodeScope); public EmailService(Settings settings, @Nullable CryptoService cryptoService, ClusterSettings clusterSettings) { - super(settings); + super(settings, "email"); this.cryptoService = cryptoService; clusterSettings.addSettingsUpdateConsumer(EMAIL_ACCOUNT_SETTING, this::setAccountSetting); setAccountSetting(EMAIL_ACCOUNT_SETTING.get(settings)); diff --git a/plugin/src/main/java/org/elasticsearch/xpack/watcher/notification/hipchat/HipChatService.java b/plugin/src/main/java/org/elasticsearch/xpack/watcher/notification/hipchat/HipChatService.java index aacb2d9d2ca..11631018358 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/watcher/notification/hipchat/HipChatService.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/watcher/notification/hipchat/HipChatService.java @@ -23,7 +23,7 @@ public class HipChatService extends NotificationService { private HipChatServer defaultServer; public HipChatService(Settings settings, HttpClient httpClient, ClusterSettings clusterSettings) { - super(settings); + super(settings, "hipchat"); this.httpClient = httpClient; clusterSettings.addSettingsUpdateConsumer(HIPCHAT_ACCOUNT_SETTING, this::setAccountSetting); setAccountSetting(HIPCHAT_ACCOUNT_SETTING.get(settings)); diff --git a/plugin/src/main/java/org/elasticsearch/xpack/watcher/notification/jira/JiraService.java b/plugin/src/main/java/org/elasticsearch/xpack/watcher/notification/jira/JiraService.java index cb1a9d77107..318bbb1db01 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/watcher/notification/jira/JiraService.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/watcher/notification/jira/JiraService.java @@ -24,7 +24,7 @@ public class JiraService extends NotificationService { private final HttpClient httpClient; public JiraService(Settings settings, HttpClient httpClient, ClusterSettings clusterSettings) { - super(settings); + super(settings, "jira"); this.httpClient = httpClient; clusterSettings.addSettingsUpdateConsumer(JIRA_ACCOUNT_SETTING, this::setAccountSetting); setAccountSetting(JIRA_ACCOUNT_SETTING.get(settings)); diff --git a/plugin/src/main/java/org/elasticsearch/xpack/watcher/notification/pagerduty/PagerDutyService.java b/plugin/src/main/java/org/elasticsearch/xpack/watcher/notification/pagerduty/PagerDutyService.java index a38e83a80ba..10d20b458c8 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/watcher/notification/pagerduty/PagerDutyService.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/watcher/notification/pagerduty/PagerDutyService.java @@ -22,7 +22,7 @@ public class PagerDutyService extends NotificationService { private final HttpClient httpClient; public PagerDutyService(Settings settings, HttpClient httpClient, ClusterSettings clusterSettings) { - super(settings); + super(settings, "pagerduty"); this.httpClient = httpClient; clusterSettings.addSettingsUpdateConsumer(PAGERDUTY_ACCOUNT_SETTING, this::setAccountSetting); setAccountSetting(PAGERDUTY_ACCOUNT_SETTING.get(settings)); @@ -32,5 +32,4 @@ public class PagerDutyService extends NotificationService { protected PagerDutyAccount createAccount(String name, Settings accountSettings) { return new PagerDutyAccount(name, accountSettings, accountSettings, httpClient, logger); } - } diff --git a/plugin/src/main/java/org/elasticsearch/xpack/watcher/notification/slack/SlackService.java b/plugin/src/main/java/org/elasticsearch/xpack/watcher/notification/slack/SlackService.java index 9bb3f3dc50a..fb8dc4c86c6 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/watcher/notification/slack/SlackService.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/watcher/notification/slack/SlackService.java @@ -16,12 +16,13 @@ import org.elasticsearch.xpack.watcher.notification.NotificationService; */ public class SlackService extends NotificationService { - private final HttpClient httpClient; public static final Setting SLACK_ACCOUNT_SETTING = Setting.groupSetting("xpack.notification.slack.", Setting.Property.Dynamic, Setting.Property.NodeScope); + private final HttpClient httpClient; + public SlackService(Settings settings, HttpClient httpClient, ClusterSettings clusterSettings) { - super(settings); + super(settings, "slack"); this.httpClient = httpClient; clusterSettings.addSettingsUpdateConsumer(SLACK_ACCOUNT_SETTING, this::setAccountSetting); setAccountSetting(SLACK_ACCOUNT_SETTING.get(settings)); @@ -31,5 +32,4 @@ public class SlackService extends NotificationService { protected SlackAccount createAccount(String name, Settings accountSettings) { return new SlackAccount(name, accountSettings, accountSettings, httpClient, logger); } - } diff --git a/plugin/src/test/java/org/elasticsearch/xpack/notification/NotificationServiceTests.java b/plugin/src/test/java/org/elasticsearch/xpack/notification/NotificationServiceTests.java new file mode 100644 index 00000000000..143e8454211 --- /dev/null +++ b/plugin/src/test/java/org/elasticsearch/xpack/notification/NotificationServiceTests.java @@ -0,0 +1,93 @@ +/* + * 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; + +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.settings.SettingsException; +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.xpack.watcher.notification.NotificationService; + +import static org.hamcrest.Matchers.anyOf; +import static org.hamcrest.Matchers.is; + +public class NotificationServiceTests extends ESTestCase { + + public void testSingleAccount() throws Exception { + String accountName = randomAlphaOfLength(10); + Settings settings = Settings.builder().put("account." + accountName, "bar").build(); + + TestNotificationService service = new TestNotificationService(settings); + assertThat(service.getAccount(accountName), is(accountName)); + // single account, this will also be the default + assertThat(service.getAccount("non-existing"), is(accountName)); + } + + public void testMultipleAccountsWithExistingDefault() throws Exception { + String accountName = randomAlphaOfLength(10); + Settings settings = Settings.builder() + .put("account." + accountName, "bar") + .put("account.second", "bar") + .put("default_account", accountName) + .build(); + + TestNotificationService service = new TestNotificationService(settings); + assertThat(service.getAccount(accountName), is(accountName)); + assertThat(service.getAccount("second"), is("second")); + assertThat(service.getAccount("non-existing"), is(accountName)); + } + + public void testMultipleAccountsWithNoDefault() throws Exception { + String accountName = randomAlphaOfLength(10); + Settings settings = Settings.builder() + .put("account." + accountName, "bar") + .put("account.second", "bar") + .put("account.third", "bar") + .build(); + + TestNotificationService service = new TestNotificationService(settings); + assertThat(service.getAccount(null), anyOf(is(accountName), is("second"), is("third"))); + } + + public void testMultipleAccountsUnknownDefault() throws Exception { + String accountName = randomAlphaOfLength(10); + Settings settings = Settings.builder() + .put("account." + accountName, "bar") + .put("account.second", "bar") + .put("default_account", "non-existing") + .build(); + + SettingsException e = expectThrows(SettingsException.class, () -> new TestNotificationService(settings)); + assertThat(e.getMessage(), is("could not find default account [non-existing]")); + } + + public void testNoSpecifiedDefaultAccount() throws Exception { + String accountName = randomAlphaOfLength(10); + Settings settings = Settings.builder().put("account." + accountName, "bar").build(); + + TestNotificationService service = new TestNotificationService(settings); + assertThat(service.getAccount(null), is(accountName)); + } + + public void testAccountDoesNotExist() throws Exception{ + TestNotificationService service = new TestNotificationService(Settings.EMPTY); + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> service.getAccount(null)); + assertThat(e.getMessage(), + is("no accounts of type [test] configured. Please set up an account using the [xpack.notification.test] settings")); + } + + private static class TestNotificationService extends NotificationService { + + TestNotificationService(Settings settings) { + super(settings, "test"); + setAccountSetting(settings); + } + + @Override + protected String createAccount(String name, Settings accountSettings) { + return name; + } + } +} \ No newline at end of file diff --git a/plugin/src/test/java/org/elasticsearch/xpack/watcher/actions/jira/JiraActionFactoryTests.java b/plugin/src/test/java/org/elasticsearch/xpack/watcher/actions/jira/JiraActionFactoryTests.java index ea821b67810..49fe4090e67 100644 --- a/plugin/src/test/java/org/elasticsearch/xpack/watcher/actions/jira/JiraActionFactoryTests.java +++ b/plugin/src/test/java/org/elasticsearch/xpack/watcher/actions/jira/JiraActionFactoryTests.java @@ -5,34 +5,27 @@ */ package org.elasticsearch.xpack.watcher.actions.jira; -import org.elasticsearch.common.settings.ClusterSettings; -import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.test.ESTestCase; -import org.elasticsearch.xpack.watcher.common.text.TextTemplateEngine; import org.elasticsearch.xpack.watcher.notification.jira.JiraAccount; import org.elasticsearch.xpack.watcher.notification.jira.JiraService; import org.junit.Before; -import static java.util.Collections.singleton; import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; import static org.elasticsearch.xpack.watcher.notification.jira.JiraAccountTests.randomIssueDefaults; import static org.elasticsearch.xpack.watcher.actions.ActionBuilders.jiraAction; -import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; public class JiraActionFactoryTests extends ESTestCase { - private JiraActionFactory factory; private JiraService service; @Before public void init() throws Exception { service = mock(JiraService.class); - factory = new JiraActionFactory(Settings.EMPTY, mock(TextTemplateEngine.class), service); } public void testParseAction() throws Exception { @@ -47,18 +40,4 @@ public class JiraActionFactoryTests extends ESTestCase { JiraAction parsedAction = JiraAction.parse("_w1", "_a1", parser); assertThat(parsedAction, equalTo(action)); } - - public void testParseActionUnknownAccount() throws Exception { - ClusterSettings clusterSettings = new ClusterSettings(Settings.EMPTY, singleton(JiraService.JIRA_ACCOUNT_SETTING)); - JiraService service = new JiraService(Settings.EMPTY, null, clusterSettings); - factory = new JiraActionFactory(Settings.EMPTY, mock(TextTemplateEngine.class), service); - - JiraAction action = jiraAction("_unknown", randomIssueDefaults()).build(); - XContentBuilder jsonBuilder = jsonBuilder().value(action); - XContentParser parser = createParser(jsonBuilder); - parser.nextToken(); - - IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> factory.parseExecutable("_w1", "_a1", parser)); - assertThat(e.getMessage(), containsString("no account found for name: [_unknown]")); - } } diff --git a/plugin/src/test/java/org/elasticsearch/xpack/watcher/notification/hipchat/HipChatAccountsTests.java b/plugin/src/test/java/org/elasticsearch/xpack/watcher/notification/hipchat/HipChatAccountsTests.java index b624c37d02e..8635d4982c7 100644 --- a/plugin/src/test/java/org/elasticsearch/xpack/watcher/notification/hipchat/HipChatAccountsTests.java +++ b/plugin/src/test/java/org/elasticsearch/xpack/watcher/notification/hipchat/HipChatAccountsTests.java @@ -7,7 +7,6 @@ package org.elasticsearch.xpack.watcher.notification.hipchat; import org.elasticsearch.common.settings.ClusterSettings; import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.common.settings.SettingsException; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xpack.watcher.common.http.HttpClient; import org.elasticsearch.xpack.watcher.common.http.HttpProxy; @@ -21,10 +20,7 @@ import org.mockito.ArgumentCaptor; import java.util.Collections; import java.util.HashMap; -import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; -import static org.hamcrest.Matchers.isOneOf; -import static org.hamcrest.Matchers.notNullValue; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -36,110 +32,6 @@ public class HipChatAccountsTests extends ESTestCase { httpClient = mock(HttpClient.class); } - public void testSingleAccount() throws Exception { - Settings.Builder builder = Settings.builder() - .put("xpack.notification.hipchat.default_account", "account1"); - addAccountSettings("account1", builder); - HipChatService service = new HipChatService(builder.build(), httpClient, new ClusterSettings(Settings.EMPTY, - Collections.singleton(HipChatService.HIPCHAT_ACCOUNT_SETTING))); - HipChatAccount account = service.getAccount("account1"); - assertThat(account, notNullValue()); - assertThat(account.name, equalTo("account1")); - account = service.getAccount(null); // falling back on the default - assertThat(account, notNullValue()); - assertThat(account.name, equalTo("account1")); - } - - public void testSingleAccountNoExplicitDefault() throws Exception { - Settings.Builder builder = Settings.builder(); - addAccountSettings("account1", builder); - - HipChatService service = new HipChatService(builder.build(), httpClient, new ClusterSettings(Settings.EMPTY, - Collections.singleton(HipChatService.HIPCHAT_ACCOUNT_SETTING))); - HipChatAccount account = service.getAccount("account1"); - assertThat(account, notNullValue()); - assertThat(account.name, equalTo("account1")); - account = service.getAccount(null); // falling back on the default - assertThat(account, notNullValue()); - assertThat(account.name, equalTo("account1")); - } - - public void testMultipleAccounts() throws Exception { - Settings.Builder builder = Settings.builder() - .put("xpack.notification.hipchat.default_account", "account1"); - addAccountSettings("account1", builder); - addAccountSettings("account2", builder); - - HipChatService service = new HipChatService(builder.build(), httpClient, new ClusterSettings(Settings.EMPTY, - Collections.singleton(HipChatService.HIPCHAT_ACCOUNT_SETTING))); - HipChatAccount account = service.getAccount("account1"); - assertThat(account, notNullValue()); - assertThat(account.name, equalTo("account1")); - account = service.getAccount("account2"); - assertThat(account, notNullValue()); - assertThat(account.name, equalTo("account2")); - account = service.getAccount(null); // falling back on the default - assertThat(account, notNullValue()); - assertThat(account.name, equalTo("account1")); - } - - public void testMultipleAccountsNoExplicitDefault() throws Exception { - Settings.Builder builder = Settings.builder() - .put("xpack.notification.hipchat.default_account", "account1"); - addAccountSettings("account1", builder); - addAccountSettings("account2", builder); - - HipChatService service = new HipChatService(builder.build(), httpClient, new ClusterSettings(Settings.EMPTY, - Collections.singleton(HipChatService.HIPCHAT_ACCOUNT_SETTING))); - HipChatAccount account = service.getAccount("account1"); - assertThat(account, notNullValue()); - assertThat(account.name, equalTo("account1")); - account = service.getAccount("account2"); - assertThat(account, notNullValue()); - assertThat(account.name, equalTo("account2")); - account = service.getAccount(null); - assertThat(account, notNullValue()); - assertThat(account.name, isOneOf("account1", "account2")); - } - - public void testMultipleAccountsUnknownDefault() throws Exception { - Settings.Builder builder = Settings.builder() - .put("xpack.notification.hipchat.default_account", "unknown"); - addAccountSettings("account1", builder); - addAccountSettings("account2", builder); - try { - new HipChatService(builder.build(), httpClient, new ClusterSettings(Settings.EMPTY, - Collections.singleton(HipChatService.HIPCHAT_ACCOUNT_SETTING))); - fail("Expected SettingsException"); - } catch (SettingsException e) { - assertThat(e.getMessage(), is("could not find default account [unknown]")); - } - } - - public void testNoAccount() throws Exception { - Settings.Builder builder = Settings.builder(); - HipChatService service = new HipChatService(builder.build(), httpClient, new ClusterSettings(Settings.EMPTY, - Collections.singleton(HipChatService.HIPCHAT_ACCOUNT_SETTING))); - try { - service.getAccount(null); - fail("no accounts are configured so trying to get the default account should throw an IllegalStateException"); - } catch (IllegalArgumentException e) { - assertThat(e.getMessage(), is("no account found for name: [null]")); - } - } - - public void testNoAccountWithDefaultAccount() throws Exception { - Settings.Builder builder = Settings.builder() - .put("xpack.notification.hipchat.default_account", "unknown"); - try { - new HipChatService(builder.build(), httpClient, new ClusterSettings(Settings.EMPTY, - Collections.singleton(HipChatService.HIPCHAT_ACCOUNT_SETTING))); - fail("Expected SettingsException"); - } catch (SettingsException e) { - assertThat(e.getMessage(), is("could not find default account [unknown]")); - } - } - public void testProxy() throws Exception { Settings.Builder builder = Settings.builder() .put("xpack.notification.hipchat.default_account", "account1"); @@ -152,7 +44,7 @@ public class HipChatAccountsTests extends ESTestCase { .addRooms(new TextTemplate("room")) .setFrom("from") .build(); - HipChatMessage hipChatMessage = template.render(new MockTextTemplateEngine(), new HashMap()); + HipChatMessage hipChatMessage = template.render(new MockTextTemplateEngine(), new HashMap<>()); ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(HttpRequest.class); when(httpClient.execute(argumentCaptor.capture())).thenReturn(new HttpResponse(200)); diff --git a/plugin/src/test/java/org/elasticsearch/xpack/watcher/notification/jira/JiraAccountTests.java b/plugin/src/test/java/org/elasticsearch/xpack/watcher/notification/jira/JiraAccountTests.java index 7b765563e3c..f6b45784d3a 100644 --- a/plugin/src/test/java/org/elasticsearch/xpack/watcher/notification/jira/JiraAccountTests.java +++ b/plugin/src/test/java/org/elasticsearch/xpack/watcher/notification/jira/JiraAccountTests.java @@ -32,7 +32,6 @@ import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.is; -import static org.hamcrest.Matchers.isOneOf; import static org.hamcrest.Matchers.notNullValue; import static org.mockito.Matchers.any; import static org.mockito.Mockito.mock; @@ -74,91 +73,6 @@ public class JiraAccountTests extends ESTestCase { assertThat(e.getMessage(), containsString("invalid jira [test] account settings. missing required [password] setting")); } - public void testSingleAccount() throws Exception { - Settings.Builder builder = Settings.builder().put("xpack.notification.jira.default_account", "account1"); - addAccountSettings("account1", builder); - - JiraService service = new JiraService(builder.build(), httpClient, clusterSettings); - JiraAccount account = service.getAccount("account1"); - assertThat(account, notNullValue()); - assertThat(account.getName(), equalTo("account1")); - account = service.getAccount(null); // falling back on the default - assertThat(account, notNullValue()); - assertThat(account.getName(), equalTo("account1")); - } - - public void testSingleAccountNoExplicitDefault() throws Exception { - Settings.Builder builder = Settings.builder(); - addAccountSettings("account1", builder); - - JiraService service = new JiraService(builder.build(), httpClient, clusterSettings); - JiraAccount account = service.getAccount("account1"); - assertThat(account, notNullValue()); - assertThat(account.getName(), equalTo("account1")); - account = service.getAccount(null); // falling back on the default - assertThat(account, notNullValue()); - assertThat(account.getName(), equalTo("account1")); - } - - public void testMultipleAccounts() throws Exception { - Settings.Builder builder = Settings.builder().put("xpack.notification.jira.default_account", "account1"); - addAccountSettings("account1", builder); - addAccountSettings("account2", builder); - - JiraService service = new JiraService(builder.build(), httpClient, clusterSettings); - JiraAccount account = service.getAccount("account1"); - assertThat(account, notNullValue()); - assertThat(account.getName(), equalTo("account1")); - account = service.getAccount("account2"); - assertThat(account, notNullValue()); - assertThat(account.getName(), equalTo("account2")); - account = service.getAccount(null); // falling back on the default - assertThat(account, notNullValue()); - assertThat(account.getName(), equalTo("account1")); - } - - public void testMultipleAccountsNoExplicitDefault() throws Exception { - Settings.Builder builder = Settings.builder().put("xpack.notification.jira.default_account", "account1"); - addAccountSettings("account1", builder); - addAccountSettings("account2", builder); - - JiraService service = new JiraService(builder.build(), httpClient, clusterSettings); - JiraAccount account = service.getAccount("account1"); - assertThat(account, notNullValue()); - assertThat(account.getName(), equalTo("account1")); - account = service.getAccount("account2"); - assertThat(account, notNullValue()); - assertThat(account.getName(), equalTo("account2")); - account = service.getAccount(null); - assertThat(account, notNullValue()); - assertThat(account.getName(), isOneOf("account1", "account2")); - } - - public void testMultipleAccountsUnknownDefault() throws Exception { - Settings.Builder builder = Settings.builder().put("xpack.notification.jira.default_account", "unknown"); - addAccountSettings("account1", builder); - addAccountSettings("account2", builder); - SettingsException e = expectThrows(SettingsException.class, () -> new JiraService(builder.build(), httpClient, clusterSettings) - ); - assertThat(e.getMessage(), is("could not find default account [unknown]")); - } - - public void testNoAccount() throws Exception { - Settings.Builder builder = Settings.builder(); - JiraService service = new JiraService(builder.build(), httpClient, clusterSettings); - - IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> service.getAccount(null)); - assertThat(e.getMessage(), is("no account found for name: [null]")); - } - - public void testNoAccountWithDefaultAccount() throws Exception { - Settings.Builder builder = Settings.builder().put("xpack.notification.jira.default_account", "unknown"); - - SettingsException e = expectThrows(SettingsException.class, () -> new JiraService(builder.build(), httpClient, clusterSettings) - ); - assertThat(e.getMessage(), is("could not find default account [unknown]")); - } - public void testUnsecureAccountUrl() throws Exception { Settings settings = Settings.builder().put("url", "http://localhost").put("user", "foo").put("password", "bar").build(); SettingsException e = expectThrows(SettingsException.class, () -> new JiraAccount("test", settings, null)); diff --git a/plugin/src/test/java/org/elasticsearch/xpack/watcher/notification/slack/SlackAccountsTests.java b/plugin/src/test/java/org/elasticsearch/xpack/watcher/notification/slack/SlackAccountsTests.java deleted file mode 100644 index 8320d3f242c..00000000000 --- a/plugin/src/test/java/org/elasticsearch/xpack/watcher/notification/slack/SlackAccountsTests.java +++ /dev/null @@ -1,144 +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.watcher.notification.slack; - -import org.elasticsearch.common.settings.ClusterSettings; -import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.common.settings.SettingsException; -import org.elasticsearch.test.ESTestCase; -import org.elasticsearch.xpack.watcher.common.http.HttpClient; -import org.elasticsearch.xpack.watcher.notification.slack.message.SlackMessageDefaultsTests; -import org.junit.Before; - -import java.util.Collections; - -import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.is; -import static org.hamcrest.Matchers.isOneOf; -import static org.hamcrest.Matchers.notNullValue; -import static org.mockito.Mockito.mock; - -public class SlackAccountsTests extends ESTestCase { - private HttpClient httpClient; - - @Before - public void init() throws Exception { - httpClient = mock(HttpClient.class); - } - - public void testSingleAccount() throws Exception { - Settings.Builder builder = Settings.builder() - .put("xpack.notification.slack.default_account", "account1"); - addAccountSettings("account1", builder); - - SlackService service = new SlackService(builder.build(), httpClient, new ClusterSettings(Settings.EMPTY, - Collections.singleton(SlackService.SLACK_ACCOUNT_SETTING))); - SlackAccount account = service.getAccount("account1"); - assertThat(account, notNullValue()); - assertThat(account.name, equalTo("account1")); - account = service.getAccount(null); // falling back on the default - assertThat(account, notNullValue()); - assertThat(account.name, equalTo("account1")); - } - - public void testSingleAccountNoExplicitDefault() throws Exception { - Settings.Builder builder = Settings.builder(); - addAccountSettings("account1", builder); - - SlackService service = new SlackService(builder.build(), httpClient, new ClusterSettings(Settings.EMPTY, - Collections.singleton(SlackService.SLACK_ACCOUNT_SETTING))); - SlackAccount account = service.getAccount("account1"); - assertThat(account, notNullValue()); - assertThat(account.name, equalTo("account1")); - account = service.getAccount(null); // falling back on the default - assertThat(account, notNullValue()); - assertThat(account.name, equalTo("account1")); - } - - public void testMultipleAccounts() throws Exception { - Settings.Builder builder = Settings.builder() - .put("xpack.notification.slack.default_account", "account1"); - addAccountSettings("account1", builder); - addAccountSettings("account2", builder); - - SlackService service = new SlackService(builder.build(), httpClient, new ClusterSettings(Settings.EMPTY, - Collections.singleton(SlackService.SLACK_ACCOUNT_SETTING))); - SlackAccount account = service.getAccount("account1"); - assertThat(account, notNullValue()); - assertThat(account.name, equalTo("account1")); - account = service.getAccount("account2"); - assertThat(account, notNullValue()); - assertThat(account.name, equalTo("account2")); - account = service.getAccount(null); // falling back on the default - assertThat(account, notNullValue()); - assertThat(account.name, equalTo("account1")); - } - - public void testMultipleAccountsNoExplicitDefault() throws Exception { - Settings.Builder builder = Settings.builder() - .put("xpack.notification.slack.default_account", "account1"); - addAccountSettings("account1", builder); - addAccountSettings("account2", builder); - - SlackService service = new SlackService(builder.build(), httpClient, new ClusterSettings(Settings.EMPTY, - Collections.singleton(SlackService.SLACK_ACCOUNT_SETTING))); - SlackAccount account = service.getAccount("account1"); - assertThat(account, notNullValue()); - assertThat(account.name, equalTo("account1")); - account = service.getAccount("account2"); - assertThat(account, notNullValue()); - assertThat(account.name, equalTo("account2")); - account = service.getAccount(null); - assertThat(account, notNullValue()); - assertThat(account.name, isOneOf("account1", "account2")); - } - - public void testMultipleAccountsUnknownDefault() throws Exception { - Settings.Builder builder = Settings.builder() - .put("xpack.notification.slack.default_account", "unknown"); - addAccountSettings("account1", builder); - addAccountSettings("account2", builder); - try { - new SlackService(builder.build(), httpClient, new ClusterSettings(Settings.EMPTY, - Collections.singleton(SlackService.SLACK_ACCOUNT_SETTING))); - fail("Expected SettingsException"); - } catch (SettingsException e) { - assertThat(e.getMessage(), is("could not find default account [unknown]")); - } - } - - public void testNoAccount() throws Exception { - Settings.Builder builder = Settings.builder(); - SlackService service = new SlackService(builder.build(), httpClient, new ClusterSettings(Settings.EMPTY, - Collections.singleton(SlackService.SLACK_ACCOUNT_SETTING))); - try { - service.getAccount(null); - fail("no accounts are configured so trying to get the default account should throw an IllegalStateException"); - } catch (IllegalArgumentException e) { - assertThat(e.getMessage(), is("no account found for name: [null]")); - } - } - - public void testNoAccountWithDefaultAccount() throws Exception { - Settings.Builder builder = Settings.builder() - .put("xpack.notification.slack.default_account", "unknown"); - try { - new SlackService(builder.build(), httpClient, new ClusterSettings(Settings.EMPTY, - Collections.singleton(SlackService.SLACK_ACCOUNT_SETTING))); - fail("Expected SettingsException"); - } catch (SettingsException e) { - assertThat(e.getMessage(), is("could not find default account [unknown]")); - } - } - - private void addAccountSettings(String name, Settings.Builder builder) { - builder.put("xpack.notification.slack.account." + name + ".url", "https://hooks.slack.com/services/" + randomAlphaOfLength(50)); - Settings defaults = SlackMessageDefaultsTests.randomSettings(); - for (String setting : defaults.keySet()) { - builder.copy("xpack.notification.slack.message_defaults." + setting, setting, defaults); - } - } -}