Deguice ActionFilter (#26691)

Allows to instantiate TransportAction instances without Guice.
elastic · Sep 20, 2017 · ff1e262 · ff1e262
1 parent 61849a1
commit ff1e262
Show file tree

Hide file tree

Showing 7 changed files with 67 additions and 43 deletions.
diff --git a/core/src/main/java/org/elasticsearch/action/ActionModule.java b/core/src/main/java/org/elasticsearch/action/ActionModule.java
@@ -315,6 +315,7 @@
 import org.elasticsearch.usage.UsageService;
 
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -341,7 +342,7 @@ public class ActionModule extends AbstractModule {
     private final SettingsFilter settingsFilter;
     private final List<ActionPlugin> actionPlugins;
     private final Map<String, ActionHandler<?, ?>> actions;
-    private final List<Class<? extends ActionFilter>> actionFilters;
+    private final ActionFilters actionFilters;
     private final AutoCreateIndex autoCreateIndex;
     private final DestructiveOperations destructiveOperations;
     private final RestController restController;
@@ -503,8 +504,9 @@ public <Request extends ActionRequest, Response extends ActionResponse> void reg
         return unmodifiableMap(actions.getRegistry());
     }
 
-    private List<Class<? extends ActionFilter>> setupActionFilters(List<ActionPlugin> actionPlugins) {
-        return unmodifiableList(actionPlugins.stream().flatMap(p -> p.getActionFilters().stream()).collect(Collectors.toList()));
+    private ActionFilters setupActionFilters(List<ActionPlugin> actionPlugins) {
+        return new ActionFilters(
+            Collections.unmodifiableSet(actionPlugins.stream().flatMap(p -> p.getActionFilters().stream()).collect(Collectors.toSet())));
     }
 
     public void initRestHandlers(Supplier<DiscoveryNodes> nodesInCluster) {
@@ -649,11 +651,7 @@ public void initRestHandlers(Supplier<DiscoveryNodes> nodesInCluster) {
 
     @Override
     protected void configure() {
-        Multibinder<ActionFilter> actionFilterMultibinder = Multibinder.newSetBinder(binder(), ActionFilter.class);
-        for (Class<? extends ActionFilter> actionFilter : actionFilters) {
-            actionFilterMultibinder.addBinding().to(actionFilter);
-        }
-        bind(ActionFilters.class).asEagerSingleton();
+        bind(ActionFilters.class).toInstance(actionFilters);
         bind(DestructiveOperations.class).toInstance(destructiveOperations);
 
         if (false == transportClient) {
@@ -676,6 +674,10 @@ protected void configure() {
         }
     }
 
+    public ActionFilters getActionFilters() {
+        return actionFilters;
+    }
+
     public RestController getRestController() {
         return restController;
     }

diff --git a/core/src/main/java/org/elasticsearch/action/support/ActionFilters.java b/core/src/main/java/org/elasticsearch/action/support/ActionFilters.java
@@ -19,8 +19,6 @@
 
 package org.elasticsearch.action.support;
 
-import org.elasticsearch.common.inject.Inject;
-
 import java.util.Arrays;
 import java.util.Comparator;
 import java.util.Set;
@@ -32,7 +30,6 @@ public class ActionFilters {
 
     private final ActionFilter[] filters;
 
-    @Inject
     public ActionFilters(Set<ActionFilter> actionFilters) {
         this.filters = actionFilters.toArray(new ActionFilter[actionFilters.size()]);
         Arrays.sort(filters, new Comparator<ActionFilter>() {

diff --git a/core/src/main/java/org/elasticsearch/node/Node.java b/core/src/main/java/org/elasticsearch/node/Node.java
@@ -361,10 +361,6 @@ protected Node(final Environment environment, Collection<Class<? extends Plugin>
             CircuitBreakerService circuitBreakerService = createCircuitBreakerService(settingsModule.getSettings(),
                 settingsModule.getClusterSettings());
             resourcesToClose.add(circuitBreakerService);
-            ActionModule actionModule = new ActionModule(false, settings, clusterModule.getIndexNameExpressionResolver(),
-                    settingsModule.getIndexScopedSettings(), settingsModule.getClusterSettings(), settingsModule.getSettingsFilter(),
-                    threadPool, pluginsService.filterPlugins(ActionPlugin.class), client, circuitBreakerService, usageService);
-            modules.add(actionModule);
             modules.add(new GatewayModule());
 
 
@@ -400,6 +396,12 @@ protected Node(final Environment environment, Collection<Class<? extends Plugin>
                                                  scriptModule.getScriptService(), xContentRegistry, environment, nodeEnvironment,
                                                  namedWriteableRegistry).stream())
                 .collect(Collectors.toList());
+
+            ActionModule actionModule = new ActionModule(false, settings, clusterModule.getIndexNameExpressionResolver(),
+                settingsModule.getIndexScopedSettings(), settingsModule.getClusterSettings(), settingsModule.getSettingsFilter(),
+                threadPool, pluginsService.filterPlugins(ActionPlugin.class), client, circuitBreakerService, usageService);
+            modules.add(actionModule);
+
             final RestController restController = actionModule.getRestController();
             final NetworkModule networkModule = new NetworkModule(settings, false, pluginsService.filterPlugins(NetworkPlugin.class),
                     threadPool, bigArrays, circuitBreakerService, namedWriteableRegistry, xContentRegistry, networkService, restController);

diff --git a/core/src/main/java/org/elasticsearch/plugins/ActionPlugin.java b/core/src/main/java/org/elasticsearch/plugins/ActionPlugin.java
@@ -33,7 +33,6 @@
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.settings.SettingsFilter;
 import org.elasticsearch.common.util.concurrent.ThreadContext;
-import org.elasticsearch.plugins.Plugin;
 import org.elasticsearch.rest.RestController;
 import org.elasticsearch.rest.RestHandler;
 
@@ -66,7 +65,7 @@ public interface ActionPlugin {
     /**
      * Action filters added by this plugin.
      */
-    default List<Class<? extends ActionFilter>> getActionFilters() {
+    default List<ActionFilter> getActionFilters() {
         return Collections.emptyList();
     }
     /**

diff --git a/core/src/test/java/org/elasticsearch/cluster/ClusterInfoServiceIT.java b/core/src/test/java/org/elasticsearch/cluster/ClusterInfoServiceIT.java
@@ -23,7 +23,6 @@
 import org.elasticsearch.ElasticsearchException;
 import org.elasticsearch.action.ActionListener;
 import org.elasticsearch.action.ActionRequest;
-import org.elasticsearch.action.ActionResponse;
 import org.elasticsearch.action.admin.cluster.node.stats.NodesStatsAction;
 import org.elasticsearch.action.admin.indices.stats.IndicesStatsAction;
 import org.elasticsearch.action.support.ActionFilter;
@@ -35,7 +34,6 @@
 import org.elasticsearch.cluster.service.ClusterService;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.collect.ImmutableOpenMap;
-import org.elasticsearch.common.inject.Inject;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.unit.TimeValue;
 import org.elasticsearch.env.NodeEnvironment;
@@ -48,9 +46,6 @@
 import org.elasticsearch.test.ESIntegTestCase;
 import org.elasticsearch.test.InternalTestCluster;
 import org.elasticsearch.test.transport.MockTransportService;
-import org.elasticsearch.transport.ConnectionProfile;
-import org.elasticsearch.transport.Transport;
-import org.elasticsearch.transport.TransportException;
 import org.elasticsearch.transport.TransportRequest;
 import org.elasticsearch.transport.TransportRequestOptions;
 import org.elasticsearch.transport.TransportService;
@@ -80,16 +75,22 @@
 public class ClusterInfoServiceIT extends ESIntegTestCase {
 
     public static class TestPlugin extends Plugin implements ActionPlugin {
+
+        private final BlockingActionFilter blockingActionFilter;
+
+        public TestPlugin(Settings settings) {
+            blockingActionFilter = new BlockingActionFilter(settings);
+        }
+
         @Override
-        public List<Class<? extends ActionFilter>> getActionFilters() {
-            return singletonList(BlockingActionFilter.class);
+        public List<ActionFilter> getActionFilters() {
+            return singletonList(blockingActionFilter);
         }
     }
 
     public static class BlockingActionFilter extends org.elasticsearch.action.support.ActionFilter.Simple {
         private Set<String> blockedActions = emptySet();
 
-        @Inject
         public BlockingActionFilter(Settings settings) {
             super(settings);
         }

diff --git a/...reindex/src/test/java/org/elasticsearch/index/reindex/ReindexFromRemoteWithAuthTests.java b/...reindex/src/test/java/org/elasticsearch/index/reindex/ReindexFromRemoteWithAuthTests.java
@@ -19,6 +19,7 @@
 
 package org.elasticsearch.index.reindex;
 
+import org.apache.lucene.util.SetOnce;
 import org.elasticsearch.ElasticsearchSecurityException;
 import org.elasticsearch.ElasticsearchStatusException;
 import org.elasticsearch.action.ActionListener;
@@ -29,23 +30,31 @@
 import org.elasticsearch.action.support.ActionFilter;
 import org.elasticsearch.action.support.ActionFilterChain;
 import org.elasticsearch.action.support.WriteRequest.RefreshPolicy;
+import org.elasticsearch.client.Client;
+import org.elasticsearch.cluster.service.ClusterService;
 import org.elasticsearch.common.bytes.BytesArray;
-import org.elasticsearch.common.inject.Inject;
+import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
 import org.elasticsearch.common.network.NetworkModule;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.transport.TransportAddress;
 import org.elasticsearch.common.util.concurrent.ThreadContext;
+import org.elasticsearch.common.xcontent.NamedXContentRegistry;
+import org.elasticsearch.env.Environment;
+import org.elasticsearch.env.NodeEnvironment;
 import org.elasticsearch.plugins.ActionPlugin;
 import org.elasticsearch.plugins.Plugin;
 import org.elasticsearch.rest.RestStatus;
+import org.elasticsearch.script.ScriptService;
 import org.elasticsearch.tasks.Task;
 import org.elasticsearch.test.ESSingleNodeTestCase;
 import org.elasticsearch.threadpool.ThreadPool;
 import org.elasticsearch.transport.Netty4Plugin;
+import org.elasticsearch.watcher.ResourceWatcherService;
 import org.junit.Before;
 
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 
@@ -129,9 +138,21 @@ public void testReindexWithBadAuthentication() throws Exception {
      * Plugin that demands authentication.
      */
     public static class TestPlugin extends Plugin implements ActionPlugin {
+
+        private final SetOnce<ReindexFromRemoteWithAuthTests.TestFilter> testFilter = new SetOnce<>();
+
+        @Override
+        public Collection<Object> createComponents(Client client, ClusterService clusterService, ThreadPool threadPool,
+                                                   ResourceWatcherService resourceWatcherService, ScriptService scriptService,
+                                                   NamedXContentRegistry xContentRegistry, Environment environment,
+                                                   NodeEnvironment nodeEnvironment, NamedWriteableRegistry namedWriteableRegistry) {
+            testFilter.set(new ReindexFromRemoteWithAuthTests.TestFilter(threadPool));
+            return Collections.emptyList();
+        }
+
         @Override
-        public List<Class<? extends ActionFilter>> getActionFilters() {
-            return singletonList(ReindexFromRemoteWithAuthTests.TestFilter.class);
+        public List<ActionFilter> getActionFilters() {
+            return singletonList(testFilter.get());
         }
 
         @Override
@@ -153,7 +174,6 @@ public static class TestFilter implements ActionFilter {
         private static final String EXAMPLE_HEADER = "Example-Header";
         private final ThreadContext context;
 
-        @Inject
         public TestFilter(ThreadPool threadPool) {
             context = threadPool.getThreadContext();
         }

diff --git a/qa/smoke-test-http/src/test/java/org/elasticsearch/http/ContextAndHeaderTransportIT.java b/qa/smoke-test-http/src/test/java/org/elasticsearch/http/ContextAndHeaderTransportIT.java
@@ -20,6 +20,7 @@
 package org.elasticsearch.http;
 
 import org.apache.http.message.BasicHeader;
+import org.apache.lucene.util.SetOnce;
 import org.elasticsearch.action.ActionListener;
 import org.elasticsearch.action.ActionRequest;
 import org.elasticsearch.action.admin.indices.refresh.RefreshRequest;
@@ -31,12 +32,14 @@
 import org.elasticsearch.action.termvectors.MultiTermVectorsRequest;
 import org.elasticsearch.client.Client;
 import org.elasticsearch.client.Response;
-import org.elasticsearch.common.inject.AbstractModule;
-import org.elasticsearch.common.inject.Inject;
-import org.elasticsearch.common.inject.Module;
+import org.elasticsearch.cluster.service.ClusterService;
+import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
 import org.elasticsearch.common.network.NetworkModule;
 import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.common.xcontent.NamedXContentRegistry;
 import org.elasticsearch.common.xcontent.XContentType;
+import org.elasticsearch.env.Environment;
+import org.elasticsearch.env.NodeEnvironment;
 import org.elasticsearch.index.query.BoolQueryBuilder;
 import org.elasticsearch.index.query.GeoShapeQueryBuilder;
 import org.elasticsearch.index.query.MoreLikeThisQueryBuilder;
@@ -46,8 +49,10 @@
 import org.elasticsearch.indices.TermsLookup;
 import org.elasticsearch.plugins.ActionPlugin;
 import org.elasticsearch.plugins.Plugin;
+import org.elasticsearch.script.ScriptService;
 import org.elasticsearch.test.ESIntegTestCase.ClusterScope;
 import org.elasticsearch.threadpool.ThreadPool;
+import org.elasticsearch.watcher.ResourceWatcherService;
 import org.junit.After;
 import org.junit.Before;
 
@@ -282,21 +287,20 @@ private Client transportClient() {
 
     public static class ActionLoggingPlugin extends Plugin implements ActionPlugin {
 
-        @Override
-        public Collection<Module> createGuiceModules() {
-            return Collections.<Module>singletonList(new ActionLoggingModule());
-        }
+        private final SetOnce<LoggingFilter> loggingFilter = new SetOnce<>();
 
         @Override
-        public List<Class<? extends ActionFilter>> getActionFilters() {
-            return singletonList(LoggingFilter.class);
+        public Collection<Object> createComponents(Client client, ClusterService clusterService, ThreadPool threadPool,
+                                                   ResourceWatcherService resourceWatcherService, ScriptService scriptService,
+                                                   NamedXContentRegistry xContentRegistry, Environment environment,
+                                                   NodeEnvironment nodeEnvironment, NamedWriteableRegistry namedWriteableRegistry) {
+            loggingFilter.set(new LoggingFilter(clusterService.getSettings(), threadPool));
+            return Collections.emptyList();
         }
-    }
 
-    public static class ActionLoggingModule extends AbstractModule {
         @Override
-        protected void configure() {
-            bind(LoggingFilter.class).asEagerSingleton();
+        public List<ActionFilter> getActionFilters() {
+            return singletonList(loggingFilter.get());
         }
 
     }
@@ -305,7 +309,6 @@ public static class LoggingFilter extends ActionFilter.Simple {
 
         private final ThreadPool threadPool;
 
-        @Inject
         public LoggingFilter(Settings settings, ThreadPool pool) {
             super(settings);
             this.threadPool = pool;