From 04217c72d4335c18b289ac28c1401bee2cd05e5a Mon Sep 17 00:00:00 2001 From: Yannick Welsch Date: Mon, 18 Sep 2017 13:33:16 +0200 Subject: [PATCH] Deguice ActionFilter Allows to instantiate TransportAction instances without Guice --- .../elasticsearch/action/ActionModule.java | 18 ++++++----- .../action/support/ActionFilters.java | 3 -- .../java/org/elasticsearch/node/Node.java | 10 +++--- .../elasticsearch/plugins/ActionPlugin.java | 3 +- .../cluster/ClusterInfoServiceIT.java | 17 +++++----- .../ReindexFromRemoteWithAuthTests.java | 28 ++++++++++++++--- .../http/ContextAndHeaderTransportIT.java | 31 ++++++++++--------- 7 files changed, 67 insertions(+), 43 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/action/ActionModule.java b/core/src/main/java/org/elasticsearch/action/ActionModule.java index 6cfd89d2d26bd..86582e9b8d046 100644 --- 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 actionPlugins; private final Map> actions; - private final List> actionFilters; + private final ActionFilters actionFilters; private final AutoCreateIndex autoCreateIndex; private final DestructiveOperations destructiveOperations; private final RestController restController; @@ -503,8 +504,9 @@ public void reg return unmodifiableMap(actions.getRegistry()); } - private List> setupActionFilters(List actionPlugins) { - return unmodifiableList(actionPlugins.stream().flatMap(p -> p.getActionFilters().stream()).collect(Collectors.toList())); + private ActionFilters setupActionFilters(List actionPlugins) { + return new ActionFilters( + Collections.unmodifiableSet(actionPlugins.stream().flatMap(p -> p.getActionFilters().stream()).collect(Collectors.toSet()))); } public void initRestHandlers(Supplier nodesInCluster) { @@ -649,11 +651,7 @@ public void initRestHandlers(Supplier nodesInCluster) { @Override protected void configure() { - Multibinder actionFilterMultibinder = Multibinder.newSetBinder(binder(), ActionFilter.class); - for (Class 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 index 426129a66d6ab..c66bac31a3dee 100644 --- 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 actionFilters) { this.filters = actionFilters.toArray(new ActionFilter[actionFilters.size()]); Arrays.sort(filters, new Comparator() { diff --git a/core/src/main/java/org/elasticsearch/node/Node.java b/core/src/main/java/org/elasticsearch/node/Node.java index cee85c9619912..5688dfc3f875e 100644 --- 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 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 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 index 346bf491d619b..377da56f6018b 100644 --- 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> getActionFilters() { + default List 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 index b5e7a1201f969..0f16b4d7f1169 100644 --- 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> getActionFilters() { - return singletonList(BlockingActionFilter.class); + public List getActionFilters() { + return singletonList(blockingActionFilter); } } public static class BlockingActionFilter extends org.elasticsearch.action.support.ActionFilter.Simple { private Set blockedActions = emptySet(); - @Inject public BlockingActionFilter(Settings settings) { super(settings); } diff --git a/modules/reindex/src/test/java/org/elasticsearch/index/reindex/ReindexFromRemoteWithAuthTests.java b/modules/reindex/src/test/java/org/elasticsearch/index/reindex/ReindexFromRemoteWithAuthTests.java index c4b5c26e5c4ef..31077c405d8e1 100644 --- a/modules/reindex/src/test/java/org/elasticsearch/index/reindex/ReindexFromRemoteWithAuthTests.java +++ b/modules/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 testFilter = new SetOnce<>(); + + @Override + public Collection 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> getActionFilters() { - return singletonList(ReindexFromRemoteWithAuthTests.TestFilter.class); + public List 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 index 28b5825513f8f..749c03598a378 100644 --- 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 createGuiceModules() { - return Collections.singletonList(new ActionLoggingModule()); - } + private final SetOnce loggingFilter = new SetOnce<>(); @Override - public List> getActionFilters() { - return singletonList(LoggingFilter.class); + public Collection 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 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;