Merge branch 'master' into netty4_transport

* master: (25 commits)
  docs: remove unused file and add link to invalid role examples
  Remove interfaces for notification services
  Redirect to URL specified by next parameter, if it is set
  Fix fixture and tests so they pass again
  Update error message to be more actionable
  Switch to NamedWriteable pull based extension in xpack
  Fixing issue with infinite redirect loop
  Toggle display of security nav controls more dynamically
  Pass in xpackMainPlugin instead of xpackMainPlugin.info
  Wrap the return in a Promise
  Only unset the cookie if it is currently set
  Clarifying intent of code
  Updating tests fixtures + adding assertion for client cookie deletion
  If security is disabled, do not attempt to call the authenticate ES API
  Disambiguate between resolve function names
  Revert to not using xpackMainPlugin.info until the xpackMainPlugin is ready
  Redirect /login => / if security is disabled in ES
  Register/deregister security management items depending on whether there's an auth'd user
  Show/hide the username + logout button depending on whether there is an auth'd user
  If security is disabled, continue without auth + delete client cookie
  ...

Original commit: elastic/x-pack-elasticsearch@16b92a1a59

elasticsearch/x-pack/graph/src/main/java/org/elasticsearch/xpack/graph/GraphFeatureSet.java

@@ -5,16 +5,15 @@
  */
 package org.elasticsearch.xpack.graph;
 
+import java.io.IOException;
+
 import org.elasticsearch.common.Nullable;
 import org.elasticsearch.common.inject.Inject;
-import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.license.XPackLicenseState;
 import org.elasticsearch.xpack.XPackFeatureSet;
 
-import java.io.IOException;
-
 /**
  *
  */
@@ -24,10 +23,9 @@ public class GraphFeatureSet implements XPackFeatureSet {
     private final XPackLicenseState licenseState;
 
     @Inject
-    public GraphFeatureSet(Settings settings, @Nullable XPackLicenseState licenseState, NamedWriteableRegistry namedWriteableRegistry) {
+    public GraphFeatureSet(Settings settings, @Nullable XPackLicenseState licenseState) {
         this.enabled = Graph.enabled(settings);
         this.licenseState = licenseState;
-        namedWriteableRegistry.register(Usage.class, Usage.writeableName(Graph.NAME), Usage::new);
     }
 
     @Override
@@ -55,7 +53,7 @@ public class GraphFeatureSet implements XPackFeatureSet {
         return new Usage(available(), enabled());
     }
 
-    static class Usage extends XPackFeatureSet.Usage {
+    public static class Usage extends XPackFeatureSet.Usage {
 
         public Usage(StreamInput input) throws IOException {
             super(input);

elasticsearch/x-pack/graph/src/test/java/org/elasticsearch/xpack/graph/GraphFeatureSetTests.java

@@ -5,17 +5,13 @@
  */
 package org.elasticsearch.xpack.graph;
 
-import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.license.XPackLicenseState;
 import org.elasticsearch.test.ESTestCase;
 import org.junit.Before;
 
 import static org.hamcrest.core.Is.is;
-import static org.mockito.Matchers.anyObject;
-import static org.mockito.Matchers.eq;
 import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 /**
@@ -24,21 +20,14 @@ import static org.mockito.Mockito.when;
 public class GraphFeatureSetTests extends ESTestCase {
 
     private XPackLicenseState licenseState;
-    private NamedWriteableRegistry namedWriteableRegistry;
 
     @Before
     public void init() throws Exception {
         licenseState = mock(XPackLicenseState.class);
-        namedWriteableRegistry = mock(NamedWriteableRegistry.class);
-    }
-
-    public void testWritableRegistration() throws Exception {
-        new GraphFeatureSet(Settings.EMPTY, licenseState, namedWriteableRegistry);
-        verify(namedWriteableRegistry).register(eq(GraphFeatureSet.Usage.class), eq("xpack.usage.graph"), anyObject());
     }
 
     public void testAvailable() throws Exception {
-        GraphFeatureSet featureSet = new GraphFeatureSet(Settings.EMPTY, licenseState, namedWriteableRegistry);
+        GraphFeatureSet featureSet = new GraphFeatureSet(Settings.EMPTY, licenseState);
         boolean available = randomBoolean();
         when(licenseState.isGraphAllowed()).thenReturn(available);
         assertThat(featureSet.available(), is(available));
@@ -54,7 +43,7 @@ public class GraphFeatureSetTests extends ESTestCase {
         } else {
             settings.put("xpack.graph.enabled", enabled);
         }
-        GraphFeatureSet featureSet = new GraphFeatureSet(settings.build(), licenseState, namedWriteableRegistry);
+        GraphFeatureSet featureSet = new GraphFeatureSet(settings.build(), licenseState);
         assertThat(featureSet.enabled(), is(enabled));
     }
 

elasticsearch/x-pack/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/MonitoringFeatureSet.java

@@ -5,9 +5,12 @@
  */
 package org.elasticsearch.xpack.monitoring;
 
+import java.io.IOException;
+import java.util.HashMap;
+import java.util.Map;
+
 import org.elasticsearch.common.Nullable;
 import org.elasticsearch.common.inject.Inject;
-import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.settings.Settings;
@@ -17,10 +20,6 @@ import org.elasticsearch.xpack.XPackFeatureSet;
 import org.elasticsearch.xpack.monitoring.agent.exporter.Exporter;
 import org.elasticsearch.xpack.monitoring.agent.exporter.Exporters;
 
-import java.io.IOException;
-import java.util.HashMap;
-import java.util.Map;
-
 /**
  *
  */
@@ -31,12 +30,10 @@ public class MonitoringFeatureSet implements XPackFeatureSet {
     private final Exporters exporters;
 
     @Inject
-    public MonitoringFeatureSet(Settings settings, @Nullable XPackLicenseState licenseState, @Nullable Exporters exporters,
+    public MonitoringFeatureSet(Settings settings, @Nullable XPackLicenseState licenseState, @Nullable Exporters exporters) {
-                                NamedWriteableRegistry namedWriteableRegistry) {
         this.enabled = MonitoringSettings.ENABLED.get(settings);
         this.licenseState = licenseState;
         this.exporters = exporters;
-        namedWriteableRegistry.register(Usage.class, Usage.writeableName(Monitoring.NAME), Usage::new);
     }
 
     @Override
@@ -79,7 +76,7 @@ public class MonitoringFeatureSet implements XPackFeatureSet {
         return usage;
     }
 
-    static class Usage extends XPackFeatureSet.Usage {
+    public static class Usage extends XPackFeatureSet.Usage {
 
         private static final String ENABLED_EXPORTERS_XFIELD = "enabled_exporters";
 

elasticsearch/x-pack/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/MonitoringFeatureSetTests.java

@@ -5,7 +5,9 @@
  */
 package org.elasticsearch.xpack.monitoring;
 
-import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
+import java.util.ArrayList;
+import java.util.List;
+
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.license.XPackLicenseState;
 import org.elasticsearch.test.ESTestCase;
@@ -17,16 +19,10 @@ import org.elasticsearch.xpack.monitoring.agent.exporter.local.LocalExporter;
 import org.elasticsearch.xpack.watcher.support.xcontent.XContentSource;
 import org.junit.Before;
 
-import java.util.ArrayList;
-import java.util.List;
-
 import static org.hamcrest.CoreMatchers.nullValue;
 import static org.hamcrest.Matchers.notNullValue;
 import static org.hamcrest.core.Is.is;
-import static org.mockito.Matchers.anyObject;
-import static org.mockito.Matchers.eq;
 import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 /**
@@ -35,23 +31,16 @@ import static org.mockito.Mockito.when;
 public class MonitoringFeatureSetTests extends ESTestCase {
 
     private XPackLicenseState licenseState;
-    private NamedWriteableRegistry namedWriteableRegistry;
     private Exporters exporters;
 
     @Before
     public void init() throws Exception {
         licenseState = mock(XPackLicenseState.class);
         exporters = mock(Exporters.class);
-        namedWriteableRegistry = mock(NamedWriteableRegistry.class);
-    }
-
-    public void testWritableRegistration() throws Exception {
-        new MonitoringFeatureSet(Settings.EMPTY, licenseState, exporters, namedWriteableRegistry);
-        verify(namedWriteableRegistry).register(eq(MonitoringFeatureSet.Usage.class), eq("xpack.usage.monitoring"), anyObject());
     }
 
     public void testAvailable() throws Exception {
-        MonitoringFeatureSet featureSet = new MonitoringFeatureSet(Settings.EMPTY, licenseState, exporters, namedWriteableRegistry);
+        MonitoringFeatureSet featureSet = new MonitoringFeatureSet(Settings.EMPTY, licenseState, exporters);
         boolean available = randomBoolean();
         when(licenseState.isMonitoringAllowed()).thenReturn(available);
         assertThat(featureSet.available(), is(available));
@@ -61,12 +50,12 @@ public class MonitoringFeatureSetTests extends ESTestCase {
         boolean enabled = randomBoolean();
         Settings.Builder settings = Settings.builder();
         settings.put("xpack.monitoring.enabled", enabled);
-        MonitoringFeatureSet featureSet = new MonitoringFeatureSet(settings.build(), licenseState, exporters, namedWriteableRegistry);
+        MonitoringFeatureSet featureSet = new MonitoringFeatureSet(settings.build(), licenseState, exporters);
         assertThat(featureSet.enabled(), is(enabled));
     }
 
     public void testEnabledDefault() throws Exception {
-        MonitoringFeatureSet featureSet = new MonitoringFeatureSet(Settings.EMPTY, licenseState, exporters, namedWriteableRegistry);
+        MonitoringFeatureSet featureSet = new MonitoringFeatureSet(Settings.EMPTY, licenseState, exporters);
         assertThat(featureSet.enabled(), is(true));
     }
 
@@ -103,7 +92,7 @@ public class MonitoringFeatureSetTests extends ESTestCase {
         }
         when(exporters.iterator()).thenReturn(exporterList.iterator());
 
-        MonitoringFeatureSet featureSet = new MonitoringFeatureSet(Settings.EMPTY, licenseState, exporters, namedWriteableRegistry);
+        MonitoringFeatureSet featureSet = new MonitoringFeatureSet(Settings.EMPTY, licenseState, exporters);
         XPackFeatureSet.Usage usage = featureSet.usage();
         assertThat(usage.name(), is(featureSet.name()));
         assertThat(usage.enabled(), is(featureSet.enabled()));

elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/SecurityFeatureSet.java

@@ -52,9 +52,8 @@ public class SecurityFeatureSet implements XPackFeatureSet {
 
     @Inject
     public SecurityFeatureSet(Settings settings, @Nullable XPackLicenseState licenseState, @Nullable Realms realms,
-                              NamedWriteableRegistry namedWriteableRegistry, @Nullable CompositeRolesStore rolesStore,
+                              @Nullable CompositeRolesStore rolesStore, @Nullable IPFilter ipFilter,
-                              @Nullable IPFilter ipFilter, @Nullable AuditTrailService auditTrailService,
+                              @Nullable AuditTrailService auditTrailService, @Nullable CryptoService cryptoService) {
-                              @Nullable CryptoService cryptoService) {
         this.enabled = Security.enabled(settings);
         this.licenseState = licenseState;
         this.realms = realms;
@@ -63,7 +62,6 @@ public class SecurityFeatureSet implements XPackFeatureSet {
         this.ipFilter = ipFilter;
         this.auditTrailService = auditTrailService;
         this.cryptoService = cryptoService;
-        namedWriteableRegistry.register(Usage.class, Usage.writeableName(Security.NAME), Usage::new);
     }
 
     @Override
@@ -139,7 +137,7 @@ public class SecurityFeatureSet implements XPackFeatureSet {
         return Collections.singletonMap("enabled", cryptoService != null && cryptoService.isEncryptionEnabled());
     }
 
-    static class Usage extends XPackFeatureSet.Usage {
+    public static class Usage extends XPackFeatureSet.Usage {
 
         private static final String REALMS_XFIELD = "realms";
         private static final String ROLES_XFIELD = "roles";

elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/SecurityFeatureSetTests.java

@@ -5,8 +5,12 @@
  */
 package org.elasticsearch.xpack.security;
 
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+
 import org.elasticsearch.common.collect.MapBuilder;
-import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.license.XPackLicenseState;
 import org.elasticsearch.test.ESTestCase;
@@ -23,20 +27,12 @@ import org.elasticsearch.xpack.watcher.support.xcontent.XContentSource;
 import org.junit.After;
 import org.junit.Before;
 
-import java.util.Arrays;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.Map;
-
 import static org.hamcrest.CoreMatchers.nullValue;
 import static org.hamcrest.Matchers.contains;
 import static org.hamcrest.Matchers.hasEntry;
 import static org.hamcrest.Matchers.notNullValue;
 import static org.hamcrest.core.Is.is;
-import static org.mockito.Matchers.anyObject;
-import static org.mockito.Matchers.eq;
 import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 public class SecurityFeatureSetTests extends ESTestCase {
@@ -44,7 +40,6 @@ public class SecurityFeatureSetTests extends ESTestCase {
     private Settings settings;
     private XPackLicenseState licenseState;
     private Realms realms;
-    private NamedWriteableRegistry namedWriteableRegistry;
     private IPFilter ipFilter;
     private CompositeRolesStore rolesStore;
     private AuditTrailService auditTrail;
@@ -55,7 +50,6 @@ public class SecurityFeatureSetTests extends ESTestCase {
         settings = Settings.builder().put("path.home", createTempDir()).build();
         licenseState = mock(XPackLicenseState.class);
         realms = mock(Realms.class);
-        namedWriteableRegistry = mock(NamedWriteableRegistry.class);
         ipFilter = mock(IPFilter.class);
         rolesStore = mock(CompositeRolesStore.class);
         auditTrail = mock(AuditTrailService.class);
@@ -67,13 +61,8 @@ public class SecurityFeatureSetTests extends ESTestCase {
         AnonymousUser.initialize(Settings.EMPTY);
     }
 
-    public void testWritableRegistration() throws Exception {
-        new SecurityFeatureSet(settings, licenseState, realms, namedWriteableRegistry, rolesStore, ipFilter, auditTrail, cryptoService);
-        verify(namedWriteableRegistry).register(eq(SecurityFeatureSet.Usage.class), eq("xpack.usage.security"), anyObject());
-    }
-
     public void testAvailable() throws Exception {
-        SecurityFeatureSet featureSet = new SecurityFeatureSet(settings, licenseState, realms, namedWriteableRegistry, rolesStore,
+        SecurityFeatureSet featureSet = new SecurityFeatureSet(settings, licenseState, realms, rolesStore,
                 ipFilter, auditTrail, cryptoService);
         boolean available = randomBoolean();
         when(licenseState.isAuthAllowed()).thenReturn(available);
@@ -86,13 +75,13 @@ public class SecurityFeatureSetTests extends ESTestCase {
                 .put(this.settings)
                 .put("xpack.security.enabled", enabled)
                 .build();
-        SecurityFeatureSet featureSet = new SecurityFeatureSet(settings, licenseState, realms, namedWriteableRegistry, rolesStore,
+        SecurityFeatureSet featureSet = new SecurityFeatureSet(settings, licenseState, realms, rolesStore,
                 ipFilter, auditTrail, cryptoService);
         assertThat(featureSet.enabled(), is(enabled));
     }
 
     public void testEnabledDefault() throws Exception {
-        SecurityFeatureSet featureSet = new SecurityFeatureSet(settings, licenseState, realms, namedWriteableRegistry, rolesStore,
+        SecurityFeatureSet featureSet = new SecurityFeatureSet(settings, licenseState, realms, rolesStore,
                         ipFilter, auditTrail, cryptoService);
         assertThat(featureSet.enabled(), is(true));
     }
@@ -164,7 +153,7 @@ public class SecurityFeatureSetTests extends ESTestCase {
             AnonymousUser.initialize(Settings.builder().put(AnonymousUser.ROLES_SETTING.getKey(), "foo").build());
         }
 
-        SecurityFeatureSet featureSet = new SecurityFeatureSet(settings.build(), licenseState, realms, namedWriteableRegistry, rolesStore,
+        SecurityFeatureSet featureSet = new SecurityFeatureSet(settings.build(), licenseState, realms, rolesStore,
                 ipFilter, auditTrail, cryptoService);
         XPackFeatureSet.Usage usage = featureSet.usage();
         assertThat(usage, is(notNullValue()));

elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/authz/accesscontrol/SecurityIndexSearcherWrapperUnitTests.java

@@ -113,7 +113,7 @@ public class SecurityIndexSearcherWrapperUnitTests extends ESTestCase {
                 Collections.emptyMap(), Collections.emptyMap());
         SimilarityService similarityService = new SimilarityService(indexSettings, Collections.emptyMap());
         mapperService = new MapperService(indexSettings, analysisService, similarityService,
-                new IndicesModule(new NamedWriteableRegistry(), emptyList()).getMapperRegistry(), () -> null);
+                new IndicesModule(emptyList()).getMapperRegistry(), () -> null);
 
         ShardId shardId = new ShardId(index, 0);
         licenseState = mock(XPackLicenseState.class);

elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/XPackFeatureSet.java

@@ -61,7 +61,7 @@ public interface XPackFeatureSet {
 
         @Override
         public String getWriteableName() {
-            return writeableName(name);
+            return name;
         }
 
         @Override
@@ -81,10 +81,6 @@ public interface XPackFeatureSet {
             builder.field(AVAILABLE_XFIELD, available);
             builder.field(ENABLED_XFIELD, enabled);
         }
-
-        public static String writeableName(String featureName) {
-            return "xpack.usage." + featureName;
-        }
     }
 
 }

elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/XPackPlugin.java

@@ -10,6 +10,7 @@ import java.nio.file.Path;
 import java.security.AccessController;
 import java.security.PrivilegedAction;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
@@ -31,6 +32,7 @@ import org.elasticsearch.common.inject.Binder;
 import org.elasticsearch.common.inject.Module;
 import org.elasticsearch.common.inject.multibindings.Multibinder;
 import org.elasticsearch.common.inject.util.Providers;
+import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
 import org.elasticsearch.common.network.NetworkModule;
 import org.elasticsearch.common.settings.Setting;
 import org.elasticsearch.common.settings.Settings;
@@ -63,7 +65,9 @@ import org.elasticsearch.xpack.common.text.TextTemplateModule;
 import org.elasticsearch.xpack.extensions.XPackExtension;
 import org.elasticsearch.xpack.extensions.XPackExtensionsService;
 import org.elasticsearch.xpack.graph.Graph;
+import org.elasticsearch.xpack.graph.GraphFeatureSet;
 import org.elasticsearch.xpack.monitoring.Monitoring;
+import org.elasticsearch.xpack.monitoring.MonitoringFeatureSet;
 import org.elasticsearch.xpack.monitoring.MonitoringSettings;
 import org.elasticsearch.xpack.notification.Notification;
 import org.elasticsearch.xpack.notification.email.Account;
@@ -72,11 +76,13 @@ import org.elasticsearch.xpack.rest.action.RestXPackInfoAction;
 import org.elasticsearch.xpack.rest.action.RestXPackUsageAction;
 import org.elasticsearch.xpack.security.InternalClient;
 import org.elasticsearch.xpack.security.Security;
+import org.elasticsearch.xpack.security.SecurityFeatureSet;
 import org.elasticsearch.xpack.security.authc.AuthenticationService;
 import org.elasticsearch.xpack.security.authc.support.UsernamePasswordToken;
 import org.elasticsearch.xpack.support.clock.Clock;
 import org.elasticsearch.xpack.support.clock.SystemClock;
 import org.elasticsearch.xpack.watcher.Watcher;
+import org.elasticsearch.xpack.watcher.WatcherFeatureSet;
 import org.elasticsearch.xpack.watcher.support.WatcherScript;
 
 public class XPackPlugin extends Plugin implements ScriptPlugin, ActionPlugin, IngestPlugin {
@@ -180,13 +186,6 @@ public class XPackPlugin extends Plugin implements ScriptPlugin, ActionPlugin, I
         return modules;
     }
 
-    @Override
-    public Collection<Class<? extends LifecycleComponent>> getGuiceServiceClasses() {
-        ArrayList<Class<? extends LifecycleComponent>> services = new ArrayList<>();
-        services.addAll(notification.nodeServices());
-        return services;
-    }
-
     @Override
     public Collection<Object> createComponents(Client client, ClusterService clusterService, ThreadPool threadPool,
                                                ResourceWatcherService resourceWatcherService) {
@@ -321,6 +320,16 @@ public class XPackPlugin extends Plugin implements ScriptPlugin, ActionPlugin, I
         return security.getProcessors(parameters);
     }
 
+    @Override
+    public List<NamedWriteableRegistry.Entry> getNamedWriteables() {
+        return Arrays.asList(
+            new NamedWriteableRegistry.Entry(XPackFeatureSet.Usage.class, Security.NAME, SecurityFeatureSet.Usage::new),
+            new NamedWriteableRegistry.Entry(XPackFeatureSet.Usage.class, Watcher.NAME, WatcherFeatureSet.Usage::new),
+            new NamedWriteableRegistry.Entry(XPackFeatureSet.Usage.class, Monitoring.NAME, MonitoringFeatureSet.Usage::new),
+            new NamedWriteableRegistry.Entry(XPackFeatureSet.Usage.class, Graph.NAME, GraphFeatureSet.Usage::new)
+        );
+    }
+
     public void onIndexModule(IndexModule module) {
         security.onIndexModule(module);
     }

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<Setting<?>> getSettings() {
-        return Arrays.asList(InternalSlackService.SLACK_ACCOUNT_SETTING,
+        return Arrays.asList(SlackService.SLACK_ACCOUNT_SETTING,
-        InternalEmailService.EMAIL_ACCOUNT_SETTING,
+        EmailService.EMAIL_ACCOUNT_SETTING,
-        InternalHipChatService.HIPCHAT_ACCOUNT_SETTING,
+        HipChatService.HIPCHAT_ACCOUNT_SETTING,
-        InternalPagerDutyService.PAGERDUTY_ACCOUNT_SETTING);
+        PagerDutyService.PAGERDUTY_ACCOUNT_SETTING);
     }
 
     public List<String> getSettingsFilter() {
@@ -54,18 +48,6 @@ public class Notification {
         return settingsFilter;
     }
 
-    public Collection<Class<? extends LifecycleComponent>> nodeServices() {
-        if (transportClient) {
-            return Collections.emptyList();
-        }
-        return Arrays.<Class<? extends LifecycleComponent>>asList(
-                EmailService.class,
-                HipChatService.class,
-                SlackService.class,
-                PagerDutyService.class
-        );
-    }
-
     public Collection<? extends Module> nodeModules() {
         if (transportClient) {
             return Collections.emptyList();

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(EmailService.class).asEagerSingleton();
-        bind(InternalEmailService.class).asEagerSingleton();
-        bind(EmailService.class).to(InternalEmailService.class).asEagerSingleton();
 
         MapBinder<String, EmailAttachmentParser> emailParsersBinder = MapBinder.newMapBinder(binder(), String.class,
                 EmailAttachmentParser.class);
@@ -51,16 +44,8 @@ public class NotificationModule extends AbstractModule {
         }
         bind(EmailAttachmentsParser.class).asEagerSingleton();
 
-        // hipchat
+        bind(HipChatService.class).asEagerSingleton();
-        bind(InternalHipChatService.class).asEagerSingleton();
+        bind(SlackService.class).asEagerSingleton();
-        bind(HipChatService.class).to(InternalHipChatService.class);
+        bind(PagerDutyService.class).asEagerSingleton();
-
-        // slack
-        bind(InternalSlackService.class).asEagerSingleton();
-        bind(SlackService.class).to(InternalSlackService.class);
-
-        // pager duty
-        bind(InternalPagerDutyService.class).asEagerSingleton();
-        bind(PagerDutyService.class).to(InternalPagerDutyService.class);
     }
 }

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<Settings> 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;

elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/notification/email/InternalEmailService.java

@@ -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<Settings> 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);
-    }
-
-}

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<Settings> 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);
+    }
 
 }

elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/notification/hipchat/InternalHipChatService.java

@@ -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<Settings> 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);
-    }
-}

elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/notification/pagerduty/InternalPagerDutyService.java

@@ -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<Settings> 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);
-    }
-}

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<Settings> 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);
+    }
 }

elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/notification/slack/InternalSlackService.java

@@ -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<Settings> 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);
-    }
-}

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<Settings> 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);
+    }
 }

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 EmailServiceTests extends ESTestCase {
- *
+    private EmailService service;
- */
-public class InternalEmailServiceTests extends ESTestCase {
-    private InternalEmailService service;
     private Accounts accounts;
 
     @Before
     public void init() throws Exception {
         accounts = mock(Accounts.class);
-        service = new InternalEmailService(Settings.EMPTY, null,
+        service = new EmailService(Settings.EMPTY, null,
-                new ClusterSettings(Settings.EMPTY, Collections.singleton(InternalEmailService.EMAIL_ACCOUNT_SETTING))) {
+                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 {

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);
+        EmailService service = startEmailService(settingsBuilder);
-        try {
+        ToXContent content = (xContentBuilder, params) -> xContentBuilder.startObject()
+                .field("key1", "value1")
+                .field("key2", "value2")
+                .field("key3", "value3")
+                .endObject();
 
-            ToXContent content = (xContentBuilder, params) -> xContentBuilder.startObject()
+        Email email = Email.builder()
-                    .field("key1", "value1")
+                .id("_id")
-                    .field("key2", "value2")
+                .subject("_subject")
-                    .field("key3", "value3")
+                .textBody("_text_body")
-                    .endObject();
+                .htmlBody("<b>html body</b><p/><p/><img src=\"cid:logo.png\"/>")
+                .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()
+        EmailService.EmailSent sent = service.send(email, null, profile);
-                    .id("_id")
-                    .subject("_subject")
-                    .textBody("_text_body")
-                    .htmlBody("<b>html body</b><p/><p/><img src=\"cid:logo.png\"/>")
-                    .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);
+        terminal.println(String.format(Locale.ROOT, "email sent via account [%s]", sent.account()));
-
-            terminal.println(String.format(Locale.ROOT, "email sent via account [%s]", sent.account()));
-        } finally {
-            service.stop();
-        }
     }
 
-    static InternalEmailService startEmailService(Settings.Builder builder) {
+    static EmailService startEmailService(Settings.Builder builder) {
         Settings settings = builder.build();
-        InternalEmailService service = new InternalEmailService(settings, null,
+        return new EmailService(settings, null,
-                new ClusterSettings(settings, Collections.singleton(InternalEmailService.EMAIL_ACCOUNT_SETTING)));
+                new ClusterSettings(settings, Collections.singleton(EmailService.EMAIL_ACCOUNT_SETTING)));
-        service.start();
-        return service;
     }
 }

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")

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,
+        HipChatService service = new HipChatService(settingsBuilder.build(), httpClient,
-                new ClusterSettings(settingsBuilder.build(), Collections.singleton(InternalHipChatService.HIPCHAT_ACCOUNT_SETTING)));
+                new ClusterSettings(settingsBuilder.build(), Collections.singleton(HipChatService.HIPCHAT_ACCOUNT_SETTING)));
-        service.start();
 
         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,
+        HipChatService service = new HipChatService(settingsBuilder.build(), httpClient,
-                new ClusterSettings(settingsBuilder.build(), Collections.singleton(InternalHipChatService.HIPCHAT_ACCOUNT_SETTING)));
+                new ClusterSettings(settingsBuilder.build(), Collections.singleton(HipChatService.HIPCHAT_ACCOUNT_SETTING)));
-        service.start();
 
         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,
+        SettingsException e = expectThrows(SettingsException.class, () ->
-                new ClusterSettings(settingsBuilder.build(), Collections.singleton(InternalHipChatService.HIPCHAT_ACCOUNT_SETTING)))) {
+            new HipChatService(settingsBuilder.build(), httpClient,
-            service.start();
+                new ClusterSettings(settingsBuilder.build(), Collections.singleton(HipChatService.HIPCHAT_ACCOUNT_SETTING))));
-            fail("Expected SettingsException");
+        assertThat(e.getMessage(), containsString("missing required [room] setting for [integration] account profile"));
-        } catch (SettingsException e) {
-            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,
+        HipChatService service = new HipChatService(settingsBuilder.build(), httpClient,
-                new ClusterSettings(settingsBuilder.build(), Collections.singleton(InternalHipChatService.HIPCHAT_ACCOUNT_SETTING)));
+                new ClusterSettings(settingsBuilder.build(), Collections.singleton(HipChatService.HIPCHAT_ACCOUNT_SETTING)));
-        service.start();
 
         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,
+        HipChatService service = new HipChatService(settingsBuilder.build(), httpClient,
-                new ClusterSettings(settingsBuilder.build(), Collections.singleton(InternalHipChatService.HIPCHAT_ACCOUNT_SETTING)));
+                new ClusterSettings(settingsBuilder.build(), Collections.singleton(HipChatService.HIPCHAT_ACCOUNT_SETTING)));
-        service.start();
 
         for (int i = 0; i < 5; i++) {
             String name = "_a" + i;

elasticsearch/x-pack/watcher/src/main/java/org/elasticsearch/xpack/watcher/WatcherFeatureSet.java

@@ -5,9 +5,12 @@
  */
 package org.elasticsearch.xpack.watcher;
 
+import java.io.IOException;
+import java.util.Collections;
+import java.util.Map;
+
 import org.elasticsearch.common.Nullable;
 import org.elasticsearch.common.inject.Inject;
-import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.settings.Settings;
@@ -15,10 +18,6 @@ import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.license.XPackLicenseState;
 import org.elasticsearch.xpack.XPackFeatureSet;
 
-import java.io.IOException;
-import java.util.Collections;
-import java.util.Map;
-
 /**
  *
  */
@@ -29,12 +28,10 @@ public class WatcherFeatureSet implements XPackFeatureSet {
     private final WatcherService watcherService;
 
     @Inject
-    public WatcherFeatureSet(Settings settings, @Nullable XPackLicenseState licenseState, NamedWriteableRegistry namedWriteableRegistry,
+    public WatcherFeatureSet(Settings settings, @Nullable XPackLicenseState licenseState, @Nullable WatcherService watcherService) {
-                             @Nullable WatcherService watcherService) {
         this.watcherService = watcherService;
         this.enabled = Watcher.enabled(settings);
         this.licenseState = licenseState;
-        namedWriteableRegistry.register(Usage.class, Usage.writeableName(Watcher.NAME), Usage::new);
     }
 
     @Override
@@ -62,7 +59,7 @@ public class WatcherFeatureSet implements XPackFeatureSet {
         return new Usage(available(), enabled(), watcherService != null ? watcherService.usageStats() : Collections.emptyMap());
     }
 
-    static class Usage extends XPackFeatureSet.Usage {
+    public static class Usage extends XPackFeatureSet.Usage {
 
         private final Map<String, Object> stats;
 

elasticsearch/x-pack/watcher/src/test/java/org/elasticsearch/xpack/watcher/WatcherFeatureSetTests.java

@@ -5,7 +5,9 @@
  */
 package org.elasticsearch.xpack.watcher;
 
-import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
+import java.util.HashMap;
+import java.util.Map;
+
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.xcontent.ToXContent;
 import org.elasticsearch.common.xcontent.XContentBuilder;
@@ -14,17 +16,11 @@ import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.xpack.watcher.support.xcontent.XContentSource;
 import org.junit.Before;
 
-import java.util.HashMap;
-import java.util.Map;
-
 import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
 import static org.hamcrest.Matchers.hasEntry;
 import static org.hamcrest.Matchers.instanceOf;
 import static org.hamcrest.core.Is.is;
-import static org.mockito.Matchers.anyObject;
-import static org.mockito.Matchers.eq;
 import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 /**
@@ -33,23 +29,16 @@ import static org.mockito.Mockito.when;
 public class WatcherFeatureSetTests extends ESTestCase {
 
     private XPackLicenseState licenseState;
-    private NamedWriteableRegistry namedWriteableRegistry;
     private WatcherService watcherService;
 
     @Before
     public void init() throws Exception {
         licenseState = mock(XPackLicenseState.class);
-        namedWriteableRegistry = mock(NamedWriteableRegistry.class);
         watcherService = mock(WatcherService.class);
     }
 
-    public void testWritableRegistration() throws Exception {
-        new WatcherFeatureSet(Settings.EMPTY, licenseState, namedWriteableRegistry, watcherService);
-        verify(namedWriteableRegistry).register(eq(WatcherFeatureSet.Usage.class), eq("xpack.usage.watcher"), anyObject());
-    }
-
     public void testAvailable() throws Exception {
-        WatcherFeatureSet featureSet = new WatcherFeatureSet(Settings.EMPTY, licenseState, namedWriteableRegistry, watcherService);
+        WatcherFeatureSet featureSet = new WatcherFeatureSet(Settings.EMPTY, licenseState, watcherService);
         boolean available = randomBoolean();
         when(licenseState.isWatcherAllowed()).thenReturn(available);
         assertThat(featureSet.available(), is(available));
@@ -65,7 +54,7 @@ public class WatcherFeatureSetTests extends ESTestCase {
         } else {
             settings.put("xpack.watcher.enabled", enabled);
         }
-        WatcherFeatureSet featureSet = new WatcherFeatureSet(settings.build(), licenseState, namedWriteableRegistry, watcherService);
+        WatcherFeatureSet featureSet = new WatcherFeatureSet(settings.build(), licenseState, watcherService);
         assertThat(featureSet.enabled(), is(enabled));
     }
 
@@ -74,7 +63,7 @@ public class WatcherFeatureSetTests extends ESTestCase {
         statsMap.put("foo", "bar");
         when(watcherService.usageStats()).thenReturn(statsMap);
 
-        WatcherFeatureSet featureSet = new WatcherFeatureSet(Settings.EMPTY, licenseState, namedWriteableRegistry, watcherService);
+        WatcherFeatureSet featureSet = new WatcherFeatureSet(Settings.EMPTY, licenseState, watcherService);
         XContentBuilder builder = jsonBuilder();
         featureSet.usage().toXContent(builder, ToXContent.EMPTY_PARAMS);
 

elasticsearch/x-pack/watcher/src/test/java/org/elasticsearch/xpack/watcher/test/AbstractWatcherIntegrationTestCase.java

@@ -15,9 +15,9 @@ import org.elasticsearch.client.node.NodeClient;
 import org.elasticsearch.cluster.ClusterState;
 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.network.NetworkModule;
+import org.elasticsearch.common.settings.ClusterSettings;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.util.Callback;
 import org.elasticsearch.common.xcontent.XContentHelper;
@@ -598,10 +598,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
@@ -613,15 +614,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 {