From a6cff75983785269478336c3bde0defc311f0e22 Mon Sep 17 00:00:00 2001 From: "opensearch-trigger-bot[bot]" <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com> Date: Mon, 6 Mar 2023 16:58:19 -0800 Subject: [PATCH] [Extensions] Only store names of extension transport actions (#6545) (#6556) * Only store names of extension transport actions (cherry picked from commit 84cec8ee4200373061a7b404e7dbbdc14a317b3d) Signed-off-by: Daniel Widdis Signed-off-by: github-actions[bot] Co-authored-by: github-actions[bot] --- .../RegisterTransportActionsRequest.java | 43 +++++-------------- .../ExtensionTransportActionsHandler.java | 2 +- .../RegisterTransportActionsRequestTests.java | 11 ++--- ...ExtensionTransportActionsHandlerTests.java | 9 ++-- 4 files changed, 17 insertions(+), 48 deletions(-) diff --git a/server/src/main/java/org/opensearch/extensions/RegisterTransportActionsRequest.java b/server/src/main/java/org/opensearch/extensions/RegisterTransportActionsRequest.java index 47061f94dee83..94b15e2192722 100644 --- a/server/src/main/java/org/opensearch/extensions/RegisterTransportActionsRequest.java +++ b/server/src/main/java/org/opensearch/extensions/RegisterTransportActionsRequest.java @@ -8,18 +8,14 @@ package org.opensearch.extensions; -import org.opensearch.action.ActionRequest; -import org.opensearch.action.ActionResponse; -import org.opensearch.action.support.TransportAction; import org.opensearch.common.io.stream.StreamInput; import org.opensearch.common.io.stream.StreamOutput; import org.opensearch.transport.TransportRequest; import java.io.IOException; -import java.util.HashMap; -import java.util.Map; +import java.util.HashSet; import java.util.Objects; -import java.util.Map.Entry; +import java.util.Set; /** * Request to register extension Transport actions @@ -27,41 +23,27 @@ * @opensearch.internal */ public class RegisterTransportActionsRequest extends TransportRequest { + // The uniqueId defining the extension which runs this action private String uniqueId; - private Map>> transportActions; + // The action names to register + private Set transportActions; - public RegisterTransportActionsRequest( - String uniqueId, - Map>> transportActions - ) { + public RegisterTransportActionsRequest(String uniqueId, Set transportActions) { this.uniqueId = uniqueId; - this.transportActions = new HashMap<>(transportActions); + this.transportActions = transportActions; } public RegisterTransportActionsRequest(StreamInput in) throws IOException { super(in); this.uniqueId = in.readString(); - Map>> actions = new HashMap<>(); - int actionCount = in.readVInt(); - for (int i = 0; i < actionCount; i++) { - try { - String actionName = in.readString(); - @SuppressWarnings("unchecked") - Class> transportAction = (Class< - ? extends TransportAction>) Class.forName(in.readString()); - actions.put(actionName, transportAction); - } catch (ClassNotFoundException e) { - throw new IllegalArgumentException("Could not read transport action"); - } - } - this.transportActions = actions; + this.transportActions = new HashSet<>(in.readStringList()); } public String getUniqueId() { return uniqueId; } - public Map>> getTransportActions() { + public Set getTransportActions() { return transportActions; } @@ -69,12 +51,7 @@ public String getUniqueId() { public void writeTo(StreamOutput out) throws IOException { super.writeTo(out); out.writeString(uniqueId); - out.writeVInt(this.transportActions.size()); - for (Entry>> action : transportActions - .entrySet()) { - out.writeString(action.getKey()); - out.writeString(action.getValue().getName()); - } + out.writeStringCollection(transportActions); } @Override diff --git a/server/src/main/java/org/opensearch/extensions/action/ExtensionTransportActionsHandler.java b/server/src/main/java/org/opensearch/extensions/action/ExtensionTransportActionsHandler.java index f76fe794b2f84..1f2b58c2bd524 100644 --- a/server/src/main/java/org/opensearch/extensions/action/ExtensionTransportActionsHandler.java +++ b/server/src/main/java/org/opensearch/extensions/action/ExtensionTransportActionsHandler.java @@ -93,7 +93,7 @@ public TransportResponse handleRegisterTransportActionsRequest(RegisterTransport logger.debug("Register Transport Actions request recieved {}", transportActionsRequest); DiscoveryExtensionNode extension = extensionIdMap.get(transportActionsRequest.getUniqueId()); try { - for (String action : transportActionsRequest.getTransportActions().keySet()) { + for (String action : transportActionsRequest.getTransportActions()) { registerAction(action, extension); } } catch (Exception e) { diff --git a/server/src/test/java/org/opensearch/extensions/RegisterTransportActionsRequestTests.java b/server/src/test/java/org/opensearch/extensions/RegisterTransportActionsRequestTests.java index eb59c80ac6461..27f1597e5779f 100644 --- a/server/src/test/java/org/opensearch/extensions/RegisterTransportActionsRequestTests.java +++ b/server/src/test/java/org/opensearch/extensions/RegisterTransportActionsRequestTests.java @@ -9,20 +9,19 @@ package org.opensearch.extensions; import org.junit.Before; -import org.opensearch.action.admin.indices.create.AutoCreateAction.TransportAction; -import org.opensearch.common.collect.Map; import org.opensearch.common.io.stream.BytesStreamOutput; import org.opensearch.common.io.stream.StreamInput; import org.opensearch.test.OpenSearchTestCase; import java.io.IOException; +import java.util.Set; public class RegisterTransportActionsRequestTests extends OpenSearchTestCase { private RegisterTransportActionsRequest originalRequest; @Before public void setup() { - this.originalRequest = new RegisterTransportActionsRequest("extension-uniqueId", Map.of("testAction", TransportAction.class)); + this.originalRequest = new RegisterTransportActionsRequest("extension-uniqueId", Set.of("testAction")); } public void testRegisterTransportActionsRequest() throws IOException { @@ -31,16 +30,12 @@ public void testRegisterTransportActionsRequest() throws IOException { StreamInput input = output.bytes().streamInput(); RegisterTransportActionsRequest parsedRequest = new RegisterTransportActionsRequest(input); assertEquals(parsedRequest.getTransportActions(), originalRequest.getTransportActions()); - assertEquals(parsedRequest.getTransportActions().get("testAction"), originalRequest.getTransportActions().get("testAction")); assertEquals(parsedRequest.getTransportActions().size(), originalRequest.getTransportActions().size()); assertEquals(parsedRequest.hashCode(), originalRequest.hashCode()); assertTrue(originalRequest.equals(parsedRequest)); } public void testToString() { - assertEquals( - originalRequest.toString(), - "TransportActionsRequest{uniqueId=extension-uniqueId, actions={testAction=class org.opensearch.action.admin.indices.create.AutoCreateAction$TransportAction}}" - ); + assertEquals(originalRequest.toString(), "TransportActionsRequest{uniqueId=extension-uniqueId, actions=[testAction]}"); } } diff --git a/server/src/test/java/org/opensearch/extensions/action/ExtensionTransportActionsHandlerTests.java b/server/src/test/java/org/opensearch/extensions/action/ExtensionTransportActionsHandlerTests.java index 6f29cd780aaa8..2d0821a0fb7dd 100644 --- a/server/src/test/java/org/opensearch/extensions/action/ExtensionTransportActionsHandlerTests.java +++ b/server/src/test/java/org/opensearch/extensions/action/ExtensionTransportActionsHandlerTests.java @@ -11,7 +11,6 @@ import org.junit.After; import org.junit.Before; import org.opensearch.Version; -import org.opensearch.action.admin.indices.create.AutoCreateAction.TransportAction; import org.opensearch.client.node.NodeClient; import org.opensearch.cluster.node.DiscoveryNode; import org.opensearch.common.io.stream.NamedWriteableRegistry; @@ -39,6 +38,7 @@ import java.util.Collections; import java.util.HashMap; import java.util.Map; +import java.util.Set; import java.util.concurrent.TimeUnit; import static java.util.Collections.emptyMap; @@ -118,7 +118,7 @@ public void testRegisterAction() { public void testRegisterTransportActionsRequest() { String action = "test-action"; - RegisterTransportActionsRequest request = new RegisterTransportActionsRequest("uniqueid1", Map.of(action, TransportAction.class)); + RegisterTransportActionsRequest request = new RegisterTransportActionsRequest("uniqueid1", Set.of(action)); AcknowledgedResponse response = (AcknowledgedResponse) extensionTransportActionsHandler.handleRegisterTransportActionsRequest( request ); @@ -150,10 +150,7 @@ public void testSendTransportRequestToExtension() throws InterruptedException { ); // Register Action - RegisterTransportActionsRequest registerRequest = new RegisterTransportActionsRequest( - "uniqueid1", - Map.of(action, TransportAction.class) - ); + RegisterTransportActionsRequest registerRequest = new RegisterTransportActionsRequest("uniqueid1", Set.of(action)); AcknowledgedResponse response = (AcknowledgedResponse) extensionTransportActionsHandler.handleRegisterTransportActionsRequest( registerRequest );