From c32a4324b03d66508e125d34cc5bc1031ef25367 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Fri, 5 Aug 2016 14:11:01 +0200 Subject: [PATCH 1/3] Add NamedWriteables from plugins to TransportClient Plugins provide NamedWriteables that are added to the NamedWriteableRegistry. Those are added on Nodes already, the same mechanism is added to the setup for TransportClient. --- .../org/elasticsearch/client/transport/TransportClient.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/core/src/main/java/org/elasticsearch/client/transport/TransportClient.java b/core/src/main/java/org/elasticsearch/client/transport/TransportClient.java index e5538aa9917..aa11b389555 100644 --- a/core/src/main/java/org/elasticsearch/client/transport/TransportClient.java +++ b/core/src/main/java/org/elasticsearch/client/transport/TransportClient.java @@ -124,6 +124,9 @@ public abstract class TransportClient extends AbstractClient { List entries = new ArrayList<>(); entries.addAll(networkModule.getNamedWriteables()); entries.addAll(searchModule.getNamedWriteables()); + entries.addAll(pluginsService.filterPlugins(Plugin.class).stream() + .flatMap(p -> p.getNamedWriteables().stream()) + .collect(Collectors.toList())); NamedWriteableRegistry namedWriteableRegistry = new NamedWriteableRegistry(entries); ModulesBuilder modules = new ModulesBuilder(); From e1629356560b9d68dff22633a8c357cc3e6d79e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Fri, 5 Aug 2016 16:05:52 +0200 Subject: [PATCH 2/3] Add test to check that plugin NamedWriteables are registerd with TransportClient --- .../client/transport/TransportClientIT.java | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/core/src/test/java/org/elasticsearch/client/transport/TransportClientIT.java b/core/src/test/java/org/elasticsearch/client/transport/TransportClientIT.java index 761cc8cf0ae..59fbb94606d 100644 --- a/core/src/test/java/org/elasticsearch/client/transport/TransportClientIT.java +++ b/core/src/test/java/org/elasticsearch/client/transport/TransportClientIT.java @@ -22,11 +22,17 @@ package org.elasticsearch.client.transport; import org.elasticsearch.Version; import org.elasticsearch.client.Client; import org.elasticsearch.cluster.node.DiscoveryNode; +import org.elasticsearch.common.io.stream.NamedWriteable; +import org.elasticsearch.common.io.stream.NamedWriteableRegistry; +import org.elasticsearch.common.io.stream.NamedWriteableRegistry.Entry; +import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.network.NetworkModule; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.transport.TransportAddress; import org.elasticsearch.env.Environment; import org.elasticsearch.node.Node; +import org.elasticsearch.plugins.Plugin; import org.elasticsearch.test.ESIntegTestCase; import org.elasticsearch.test.ESIntegTestCase.ClusterScope; import org.elasticsearch.test.ESIntegTestCase.Scope; @@ -34,6 +40,8 @@ import org.elasticsearch.transport.MockTransportClient; import org.elasticsearch.transport.TransportService; import java.io.IOException; +import java.util.Arrays; +import java.util.List; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThanOrEqualTo; @@ -42,6 +50,7 @@ import static org.hamcrest.Matchers.startsWith; @ClusterScope(scope = Scope.TEST, numDataNodes = 0, transportClientRatio = 1.0) public class TransportClientIT extends ESIntegTestCase { + public void testPickingUpChangesInDiscoveryNode() { String nodeName = internalCluster().startNode(Settings.builder().put(Node.NODE_DATA_SETTING.getKey(), false)); @@ -97,4 +106,47 @@ public class TransportClientIT extends ESIntegTestCase { assertThat(Client.CLIENT_TYPE_SETTING_S.get(settings), is("transport")); } } + + /** + * test that when plugins are provided that want to register + * {@link NamedWriteable}, those are also made known to the + * {@link NamedWriteableRegistry} of the transport client + */ + public void testPluginNamedWriteablesRegistered() { + Settings baseSettings = Settings.builder() + .put(Environment.PATH_HOME_SETTING.getKey(), createTempDir()) + .build(); + try (TransportClient client = new MockTransportClient(baseSettings, pluginList(MockPlugin.class))) { + NamedWriteableRegistry registry = client.injector.getInstance(NamedWriteableRegistry.class); + assertNotNull(registry.getReader(MockPlugin.MockNamedWriteable.class, MockPlugin.MockNamedWriteable.NAME)); + } + } + + public static class MockPlugin extends Plugin { + + @Override + public List getNamedWriteables() { + return Arrays.asList(new Entry[]{ new Entry(MockNamedWriteable.class, MockNamedWriteable.NAME, MockNamedWriteable::new)}); + } + + public class MockNamedWriteable implements NamedWriteable { + + static final String NAME = "mockNamedWritable"; + + MockNamedWriteable(StreamInput in) { + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + } + + @Override + public String getWriteableName() { + return NAME; + } + + } + } + + } From 6ccb70e1ab8963e7742ce972ad5303646f2b3323 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Fri, 5 Aug 2016 18:11:50 +0200 Subject: [PATCH 3/3] Avoid using injector and more test to TransportClientTests --- .../client/transport/TransportClient.java | 9 +++- .../client/transport/TransportClientIT.java | 51 ------------------- .../transport/TransportClientTests.java | 50 ++++++++++++++++++ 3 files changed, 57 insertions(+), 53 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/client/transport/TransportClient.java b/core/src/main/java/org/elasticsearch/client/transport/TransportClient.java index aa11b389555..f7ce9f929bd 100644 --- a/core/src/main/java/org/elasticsearch/client/transport/TransportClient.java +++ b/core/src/main/java/org/elasticsearch/client/transport/TransportClient.java @@ -170,7 +170,7 @@ public abstract class TransportClient extends AbstractClient { transportService.start(); transportService.acceptIncomingRequests(); - ClientTemplate transportClient = new ClientTemplate(injector, pluginLifecycleComponents, nodesService, proxy); + ClientTemplate transportClient = new ClientTemplate(injector, pluginLifecycleComponents, nodesService, proxy, namedWriteableRegistry); resourcesToClose.clear(); return transportClient; } finally { @@ -183,12 +183,15 @@ public abstract class TransportClient extends AbstractClient { private final List pluginLifecycleComponents; private final TransportClientNodesService nodesService; private final TransportProxyClient proxy; + private final NamedWriteableRegistry namedWriteableRegistry; - private ClientTemplate(Injector injector, List pluginLifecycleComponents, TransportClientNodesService nodesService, TransportProxyClient proxy) { + private ClientTemplate(Injector injector, List pluginLifecycleComponents, + TransportClientNodesService nodesService, TransportProxyClient proxy, NamedWriteableRegistry namedWriteableRegistry) { this.injector = injector; this.pluginLifecycleComponents = pluginLifecycleComponents; this.nodesService = nodesService; this.proxy = proxy; + this.namedWriteableRegistry = namedWriteableRegistry; } Settings getSettings() { @@ -203,6 +206,7 @@ public abstract class TransportClient extends AbstractClient { public static final String CLIENT_TYPE = "transport"; final Injector injector; + final NamedWriteableRegistry namedWriteableRegistry; private final List pluginLifecycleComponents; private final TransportClientNodesService nodesService; @@ -231,6 +235,7 @@ public abstract class TransportClient extends AbstractClient { this.pluginLifecycleComponents = Collections.unmodifiableList(template.pluginLifecycleComponents); this.nodesService = template.nodesService; this.proxy = template.proxy; + this.namedWriteableRegistry = template.namedWriteableRegistry; } /** diff --git a/core/src/test/java/org/elasticsearch/client/transport/TransportClientIT.java b/core/src/test/java/org/elasticsearch/client/transport/TransportClientIT.java index 59fbb94606d..9b5b764b88e 100644 --- a/core/src/test/java/org/elasticsearch/client/transport/TransportClientIT.java +++ b/core/src/test/java/org/elasticsearch/client/transport/TransportClientIT.java @@ -22,17 +22,11 @@ package org.elasticsearch.client.transport; import org.elasticsearch.Version; import org.elasticsearch.client.Client; import org.elasticsearch.cluster.node.DiscoveryNode; -import org.elasticsearch.common.io.stream.NamedWriteable; -import org.elasticsearch.common.io.stream.NamedWriteableRegistry; -import org.elasticsearch.common.io.stream.NamedWriteableRegistry.Entry; -import org.elasticsearch.common.io.stream.StreamInput; -import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.network.NetworkModule; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.transport.TransportAddress; import org.elasticsearch.env.Environment; import org.elasticsearch.node.Node; -import org.elasticsearch.plugins.Plugin; import org.elasticsearch.test.ESIntegTestCase; import org.elasticsearch.test.ESIntegTestCase.ClusterScope; import org.elasticsearch.test.ESIntegTestCase.Scope; @@ -40,8 +34,6 @@ import org.elasticsearch.transport.MockTransportClient; import org.elasticsearch.transport.TransportService; import java.io.IOException; -import java.util.Arrays; -import java.util.List; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThanOrEqualTo; @@ -106,47 +98,4 @@ public class TransportClientIT extends ESIntegTestCase { assertThat(Client.CLIENT_TYPE_SETTING_S.get(settings), is("transport")); } } - - /** - * test that when plugins are provided that want to register - * {@link NamedWriteable}, those are also made known to the - * {@link NamedWriteableRegistry} of the transport client - */ - public void testPluginNamedWriteablesRegistered() { - Settings baseSettings = Settings.builder() - .put(Environment.PATH_HOME_SETTING.getKey(), createTempDir()) - .build(); - try (TransportClient client = new MockTransportClient(baseSettings, pluginList(MockPlugin.class))) { - NamedWriteableRegistry registry = client.injector.getInstance(NamedWriteableRegistry.class); - assertNotNull(registry.getReader(MockPlugin.MockNamedWriteable.class, MockPlugin.MockNamedWriteable.NAME)); - } - } - - public static class MockPlugin extends Plugin { - - @Override - public List getNamedWriteables() { - return Arrays.asList(new Entry[]{ new Entry(MockNamedWriteable.class, MockNamedWriteable.NAME, MockNamedWriteable::new)}); - } - - public class MockNamedWriteable implements NamedWriteable { - - static final String NAME = "mockNamedWritable"; - - MockNamedWriteable(StreamInput in) { - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - } - - @Override - public String getWriteableName() { - return NAME; - } - - } - } - - } diff --git a/core/src/test/java/org/elasticsearch/client/transport/TransportClientTests.java b/core/src/test/java/org/elasticsearch/client/transport/TransportClientTests.java index 2145f66b5e0..c97418bae37 100644 --- a/core/src/test/java/org/elasticsearch/client/transport/TransportClientTests.java +++ b/core/src/test/java/org/elasticsearch/client/transport/TransportClientTests.java @@ -20,10 +20,20 @@ package org.elasticsearch.client.transport; import org.elasticsearch.action.admin.cluster.health.ClusterHealthRequest; +import org.elasticsearch.common.io.stream.NamedWriteable; +import org.elasticsearch.common.io.stream.NamedWriteableRegistry; +import org.elasticsearch.common.io.stream.NamedWriteableRegistry.Entry; +import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.env.Environment; +import org.elasticsearch.plugins.Plugin; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.transport.MockTransportClient; +import java.io.IOException; +import java.util.Arrays; +import java.util.List; import java.util.concurrent.ExecutionException; import static org.hamcrest.CoreMatchers.containsString; @@ -38,4 +48,44 @@ public class TransportClientTests extends ESTestCase { expectThrows(IllegalStateException.class, () -> client.admin().cluster().health(new ClusterHealthRequest()).get()); assertThat(e, hasToString(containsString("transport client is closed"))); } + + /** + * test that when plugins are provided that want to register + * {@link NamedWriteable}, those are also made known to the + * {@link NamedWriteableRegistry} of the transport client + */ + public void testPluginNamedWriteablesRegistered() { + Settings baseSettings = Settings.builder() + .put(Environment.PATH_HOME_SETTING.getKey(), createTempDir()) + .build(); + try (TransportClient client = new MockTransportClient(baseSettings, Arrays.asList(MockPlugin.class))) { + assertNotNull(client.namedWriteableRegistry.getReader(MockPlugin.MockNamedWriteable.class, MockPlugin.MockNamedWriteable.NAME)); + } + } + + public static class MockPlugin extends Plugin { + + @Override + public List getNamedWriteables() { + return Arrays.asList(new Entry[]{ new Entry(MockNamedWriteable.class, MockNamedWriteable.NAME, MockNamedWriteable::new)}); + } + + public class MockNamedWriteable implements NamedWriteable { + + static final String NAME = "mockNamedWritable"; + + MockNamedWriteable(StreamInput in) { + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + } + + @Override + public String getWriteableName() { + return NAME; + } + + } + } }