From 5d52a199d7b4693ac9d38f82adf1f37d403e94e3 Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Sat, 18 Mar 2023 14:47:25 -0700 Subject: [PATCH 01/20] Add ProxyAction with TransportAction and handlers Signed-off-by: Daniel Widdis --- .../opensearch/sdk/action/ProxyAction.java | 32 +++++++ .../sdk/action/ProxyTransportAction.java | 48 +++++++++++ .../sdk/action/SDKActionModule.java | 86 +++++++++++++++---- ...ionResponseToExtensionResponseHandler.java | 80 +++++++++++++++++ .../sdk/action/TestSDKActionModule.java | 6 +- 5 files changed, 232 insertions(+), 20 deletions(-) create mode 100644 src/main/java/org/opensearch/sdk/action/ProxyAction.java create mode 100644 src/main/java/org/opensearch/sdk/action/ProxyTransportAction.java create mode 100644 src/main/java/org/opensearch/sdk/handlers/TransportActionResponseToExtensionResponseHandler.java diff --git a/src/main/java/org/opensearch/sdk/action/ProxyAction.java b/src/main/java/org/opensearch/sdk/action/ProxyAction.java new file mode 100644 index 00000000..94bcf2ba --- /dev/null +++ b/src/main/java/org/opensearch/sdk/action/ProxyAction.java @@ -0,0 +1,32 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.sdk.action; + +import org.opensearch.action.ActionType; +import org.opensearch.extensions.action.ExtensionActionResponse; + +/** + * The {@link ActionType} used as they key for the {@link ProxyTransportAction}. + */ +public class ProxyAction extends ActionType { + + /** + * The name to look up this action with + */ + public static final String NAME = "internal/proxyaction"; + /** + * The singleton instance of this class + */ + public static final ProxyAction INSTANCE = new ProxyAction(); + + private ProxyAction() { + super(NAME, ExtensionActionResponse::new); + } +} diff --git a/src/main/java/org/opensearch/sdk/action/ProxyTransportAction.java b/src/main/java/org/opensearch/sdk/action/ProxyTransportAction.java new file mode 100644 index 00000000..3269197d --- /dev/null +++ b/src/main/java/org/opensearch/sdk/action/ProxyTransportAction.java @@ -0,0 +1,48 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.sdk.action; + +import org.opensearch.action.ActionListener; +import org.opensearch.action.support.ActionFilters; +import org.opensearch.action.support.TransportAction; +import org.opensearch.extensions.action.ExtensionActionResponse; +import org.opensearch.extensions.action.TransportActionRequestFromExtension; +import org.opensearch.tasks.Task; +import org.opensearch.tasks.TaskManager; + +import com.google.inject.Inject; + +/** + * Sends a request to OpenSearch for a remote extension to execute an action. + */ +public class ProxyTransportAction extends TransportAction { + + /** + * Instantiate this action + * + * @param actionName The action name + * @param actionFilters Action filters + * @param taskManager The task manager + */ + @Inject + protected ProxyTransportAction(String actionName, ActionFilters actionFilters, TaskManager taskManager) { + super(actionName, actionFilters, taskManager); + } + + @Override + protected void doExecute(Task task, TransportActionRequestFromExtension request, ActionListener listener) { + // TODO: Invoke ActionModule.sendProxyAction and handle the response + if (request == null) { + listener.onFailure(new IllegalArgumentException("The request is null.")); + } else { + listener.onResponse(new ExtensionActionResponse(new byte[0])); + } + } +} diff --git a/src/main/java/org/opensearch/sdk/action/SDKActionModule.java b/src/main/java/org/opensearch/sdk/action/SDKActionModule.java index ef8f5868..ecaf4bff 100644 --- a/src/main/java/org/opensearch/sdk/action/SDKActionModule.java +++ b/src/main/java/org/opensearch/sdk/action/SDKActionModule.java @@ -11,6 +11,7 @@ import java.util.Collections; import java.util.Map; +import java.util.concurrent.TimeoutException; import java.util.stream.Collectors; import org.apache.logging.log4j.LogManager; @@ -21,10 +22,12 @@ import org.opensearch.cluster.node.DiscoveryNode; import org.opensearch.common.NamedRegistry; import org.opensearch.extensions.ExtensionsManager; -import org.opensearch.extensions.RegisterTransportActionsRequest; +import org.opensearch.extensions.action.RegisterTransportActionsRequest; +import org.opensearch.extensions.action.TransportActionRequestFromExtension; import org.opensearch.sdk.ActionExtension.ActionHandler; import org.opensearch.sdk.Extension; import org.opensearch.sdk.handlers.AcknowledgedResponseHandler; +import org.opensearch.sdk.handlers.TransportActionResponseToExtensionResponseHandler; import org.opensearch.transport.TransportService; import com.google.inject.AbstractModule; @@ -62,24 +65,33 @@ public ActionFilters getActionFilters() { } private static Map> setupActions(Extension extension) { - if (extension instanceof ActionExtension) { - // Subclass NamedRegistry for easy registration - class ActionRegistry extends NamedRegistry> { - ActionRegistry() { - super("action"); - } - - public void register(ActionHandler handler) { - register(handler.getAction().name(), handler); - } + /** + * Subclass of NamedRegistry permitting easier action registration + */ + class ActionRegistry extends NamedRegistry> { + ActionRegistry() { + super("sdkaction"); } - ActionRegistry actions = new ActionRegistry(); - // Register getActions in it - ((ActionExtension) extension).getActions().stream().forEach(actions::register); - return unmodifiableMap(actions.getRegistry()); + /** + * Register an action handler pairing an ActionType and TransportAction + * + * @param handler The ActionHandler to register + */ + public void register(ActionHandler handler) { + register(handler.getAction().name(), handler); + } } - return Collections.emptyMap(); + ActionRegistry actions = new ActionRegistry(); + + // Register SDK actions + actions.register(new ActionHandler<>(ProxyAction.INSTANCE, ProxyTransportAction.class)); + + // Register actions from getActions extension point + if (extension instanceof ActionExtension) { + ((ActionExtension) extension).getActions().stream().forEach(actions::register); + } + return unmodifiableMap(actions.getRegistry()); } private static ActionFilters setupActionFilters(Extension extension) { @@ -95,6 +107,8 @@ protected void configure() { // Bind action filters bind(ActionFilters.class).toInstance(actionFilters); + // Bind local actions + // bind ActionType -> transportAction Map used by Client @SuppressWarnings("rawtypes") MapBinder transportActionsBinder = MapBinder.newMapBinder( @@ -114,7 +128,7 @@ protected void configure() { * * @param transportService The TransportService defining the connection to OpenSearch. * @param opensearchNode The OpenSearch node where transport actions being registered. - * @param uniqueId The identity used to + * @param uniqueId The uniqueId defining the extension which runs this action */ public void sendRegisterTransportActionsRequest(TransportService transportService, DiscoveryNode opensearchNode, String uniqueId) { logger.info("Sending Register Transport Actions request to OpenSearch"); @@ -130,4 +144,42 @@ public void sendRegisterTransportActionsRequest(TransportService transportServic logger.info("Failed to send Register Transport Actions request to OpenSearch", e); } } + + /** + * Requests that OpenSearch execute a Transport Actions on another extension. + * + * @param transportService The TransportService defining the connection to OpenSearch. + * @param opensearchNode The OpenSearch node where transport actions being registered. + * @param action The fully qualified class name of the action to execute + * @param requestBytes A buffer containing serialized parameters to be understood by the remote action + * @param uniqueId The uniqueId defining the extension which is requesting this action + * @return A buffer serializing the response from the remote action if successful, otherwise empty + */ + public byte[] sendProxyActionRequest( + TransportService transportService, + DiscoveryNode opensearchNode, + String action, + byte[] requestBytes, + String uniqueId + ) { + logger.info("Sending ProxyAction request to OpenSearch for [" + action + "]"); + TransportActionResponseToExtensionResponseHandler handleTransportActionResponseHandler = + new TransportActionResponseToExtensionResponseHandler(); + try { + transportService.sendRequest( + opensearchNode, + ExtensionsManager.REQUEST_EXTENSION_HANDLE_TRANSPORT_ACTION, + new TransportActionRequestFromExtension(action, requestBytes, uniqueId), + handleTransportActionResponseHandler + ); + // Wait on response + handleTransportActionResponseHandler.awaitResponse(); + } catch (TimeoutException e) { + logger.info("Failed to receive ProxyAction response from OpenSearch", e); + } catch (Exception e) { + logger.info("Failed to send ProxyAction request to OpenSearch", e); + } + // At this point, response handler has read in the response bytes + return handleTransportActionResponseHandler.getResponseBytes(); + } } diff --git a/src/main/java/org/opensearch/sdk/handlers/TransportActionResponseToExtensionResponseHandler.java b/src/main/java/org/opensearch/sdk/handlers/TransportActionResponseToExtensionResponseHandler.java new file mode 100644 index 00000000..c8939fff --- /dev/null +++ b/src/main/java/org/opensearch/sdk/handlers/TransportActionResponseToExtensionResponseHandler.java @@ -0,0 +1,80 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.sdk.handlers; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.opensearch.extensions.ExtensionsManager; +import org.opensearch.extensions.action.TransportActionResponseToExtension; +import org.opensearch.common.io.stream.StreamInput; +import org.opensearch.sdk.action.SDKActionModule; +import org.opensearch.threadpool.ThreadPool; +import org.opensearch.transport.TransportException; +import org.opensearch.transport.TransportResponseHandler; + +import java.io.IOException; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.TimeUnit; + +/** + * This class handles the response from OpenSearch to a {@link SDKActionModule#sendProxyActionRequest()} call. + */ +public class TransportActionResponseToExtensionResponseHandler implements TransportResponseHandler { + + private static final Logger logger = LogManager.getLogger(TransportActionResponseToExtensionResponseHandler.class); + private final CompletableFuture inProgressFuture; + private byte[] responseBytes; + + /** + * Instantiates a new TransportActionResponseToExtensionHandler with a count down latch and an empty Settings object + */ + public TransportActionResponseToExtensionResponseHandler() { + this.inProgressFuture = new CompletableFuture<>(); + this.responseBytes = new byte[0]; + } + + @Override + public void handleResponse(TransportActionResponseToExtension response) { + logger.info("received {}", response); + + // Set TransportActionResponseToExtension from response + this.responseBytes = response.getResponseBytes(); + inProgressFuture.complete(response); + } + + @Override + public void handleException(TransportException exp) { + logger.info("TransportActionResponseToExtensionRequest failed", exp); + inProgressFuture.completeExceptionally(exp); + } + + @Override + public String executor() { + return ThreadPool.Names.GENERIC; + } + + @Override + public TransportActionResponseToExtension read(StreamInput in) throws IOException { + return new TransportActionResponseToExtension(in); + } + + /** + * Waits for the TransportActionResponseToExtensionHandler future to complete + * @throws Exception + * if the response times out + */ + public void awaitResponse() throws Exception { + inProgressFuture.orTimeout(ExtensionsManager.EXTENSION_REQUEST_WAIT_TIMEOUT, TimeUnit.SECONDS).get(); + } + + public byte[] getResponseBytes() { + return this.responseBytes; + } +} diff --git a/src/test/java/org/opensearch/sdk/action/TestSDKActionModule.java b/src/test/java/org/opensearch/sdk/action/TestSDKActionModule.java index 1c37989a..7832cfe5 100644 --- a/src/test/java/org/opensearch/sdk/action/TestSDKActionModule.java +++ b/src/test/java/org/opensearch/sdk/action/TestSDKActionModule.java @@ -20,7 +20,7 @@ import org.opensearch.common.settings.Settings; import org.opensearch.common.transport.TransportAddress; import org.opensearch.extensions.ExtensionsManager; -import org.opensearch.extensions.RegisterTransportActionsRequest; +import org.opensearch.extensions.action.RegisterTransportActionsRequest; import org.opensearch.sdk.ActionExtension; import org.opensearch.sdk.Extension; import org.opensearch.sdk.ExtensionSettings; @@ -33,7 +33,6 @@ import java.util.Arrays; import java.util.Collections; import java.util.List; -import java.util.Set; import static java.util.Collections.emptyMap; import static java.util.Collections.emptySet; @@ -108,6 +107,7 @@ public void testRegisterTransportAction() { any(AcknowledgedResponseHandler.class) ); assertEquals(TEST_UNIQUE_ID, registerTransportActionsRequestCaptor.getValue().getUniqueId()); - assertEquals(Set.of(TEST_ACTION_NAME), registerTransportActionsRequestCaptor.getValue().getTransportActions()); + assertTrue(registerTransportActionsRequestCaptor.getValue().getTransportActions().contains(ProxyAction.NAME)); + assertTrue(registerTransportActionsRequestCaptor.getValue().getTransportActions().contains(TEST_ACTION_NAME)); } } From c8888ce30dd3cf22d5cd8545736db6202ee27a7d Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Sat, 18 Mar 2023 15:44:12 -0700 Subject: [PATCH 02/20] Give SDKActionModule a copy of ExtensionsRunner to use with transport Signed-off-by: Daniel Widdis --- .../org/opensearch/sdk/ExtensionsRunner.java | 2 +- .../sdk/action/ProxyTransportAction.java | 2 +- .../sdk/action/SDKActionModule.java | 31 +++++++++---------- .../ExtensionsInitRequestHandler.java | 7 +---- .../sdk/action/TestSDKActionModule.java | 12 +++++-- .../helloworld/TestHelloWorldExtension.java | 5 ++- 6 files changed, 30 insertions(+), 29 deletions(-) diff --git a/src/main/java/org/opensearch/sdk/ExtensionsRunner.java b/src/main/java/org/opensearch/sdk/ExtensionsRunner.java index da3106a2..1d49777d 100644 --- a/src/main/java/org/opensearch/sdk/ExtensionsRunner.java +++ b/src/main/java/org/opensearch/sdk/ExtensionsRunner.java @@ -193,7 +193,7 @@ protected ExtensionsRunner(Extension extension) throws IOException { // Bind the return values from create components modules.add(this::injectComponents); // Bind actions from getActions - this.sdkActionModule = new SDKActionModule(extension); + this.sdkActionModule = new SDKActionModule(this, extension); modules.add(this.sdkActionModule); // Finally, perform the injection this.injector = Guice.createInjector(modules); diff --git a/src/main/java/org/opensearch/sdk/action/ProxyTransportAction.java b/src/main/java/org/opensearch/sdk/action/ProxyTransportAction.java index 3269197d..08482431 100644 --- a/src/main/java/org/opensearch/sdk/action/ProxyTransportAction.java +++ b/src/main/java/org/opensearch/sdk/action/ProxyTransportAction.java @@ -38,7 +38,7 @@ protected ProxyTransportAction(String actionName, ActionFilters actionFilters, T @Override protected void doExecute(Task task, TransportActionRequestFromExtension request, ActionListener listener) { - // TODO: Invoke ActionModule.sendProxyAction and handle the response + // SDKActionModule.sendProxyActionRequest(); if (request == null) { listener.onFailure(new IllegalArgumentException("The request is null.")); } else { diff --git a/src/main/java/org/opensearch/sdk/action/SDKActionModule.java b/src/main/java/org/opensearch/sdk/action/SDKActionModule.java index ecaf4bff..55c07c7e 100644 --- a/src/main/java/org/opensearch/sdk/action/SDKActionModule.java +++ b/src/main/java/org/opensearch/sdk/action/SDKActionModule.java @@ -26,6 +26,7 @@ import org.opensearch.extensions.action.TransportActionRequestFromExtension; import org.opensearch.sdk.ActionExtension.ActionHandler; import org.opensearch.sdk.Extension; +import org.opensearch.sdk.ExtensionsRunner; import org.opensearch.sdk.handlers.AcknowledgedResponseHandler; import org.opensearch.sdk.handlers.TransportActionResponseToExtensionResponseHandler; import org.opensearch.transport.TransportService; @@ -43,15 +44,18 @@ public class SDKActionModule extends AbstractModule { private final Logger logger = LogManager.getLogger(SDKActionModule.class); + private ExtensionsRunner extensionsRunner; private final Map> actions; private final ActionFilters actionFilters; /** * Instantiate this module * - * @param extension An instance of {@link ActionExtension}. + * @param extensionsRunner The ExtensionsRunner instance + * @param extension The extension */ - public SDKActionModule(Extension extension) { + public SDKActionModule(ExtensionsRunner extensionsRunner, Extension extension) { + this.extensionsRunner = extensionsRunner; this.actions = setupActions(extension); this.actionFilters = setupActionFilters(extension); } @@ -125,13 +129,12 @@ protected void configure() { /** * Requests that OpenSearch register the Transport Actions for this extension. - * - * @param transportService The TransportService defining the connection to OpenSearch. - * @param opensearchNode The OpenSearch node where transport actions being registered. - * @param uniqueId The uniqueId defining the extension which runs this action */ - public void sendRegisterTransportActionsRequest(TransportService transportService, DiscoveryNode opensearchNode, String uniqueId) { + public void sendRegisterTransportActionsRequest() { logger.info("Sending Register Transport Actions request to OpenSearch"); + TransportService transportService = extensionsRunner.getExtensionTransportService(); + DiscoveryNode opensearchNode = extensionsRunner.getOpensearchNode(); + String uniqueId = extensionsRunner.getUniqueId(); AcknowledgedResponseHandler registerTransportActionsResponseHandler = new AcknowledgedResponseHandler(); try { transportService.sendRequest( @@ -148,21 +151,15 @@ public void sendRegisterTransportActionsRequest(TransportService transportServic /** * Requests that OpenSearch execute a Transport Actions on another extension. * - * @param transportService The TransportService defining the connection to OpenSearch. - * @param opensearchNode The OpenSearch node where transport actions being registered. * @param action The fully qualified class name of the action to execute * @param requestBytes A buffer containing serialized parameters to be understood by the remote action - * @param uniqueId The uniqueId defining the extension which is requesting this action * @return A buffer serializing the response from the remote action if successful, otherwise empty */ - public byte[] sendProxyActionRequest( - TransportService transportService, - DiscoveryNode opensearchNode, - String action, - byte[] requestBytes, - String uniqueId - ) { + public byte[] sendProxyActionRequest(String action, byte[] requestBytes) { logger.info("Sending ProxyAction request to OpenSearch for [" + action + "]"); + TransportService transportService = extensionsRunner.getExtensionTransportService(); + DiscoveryNode opensearchNode = extensionsRunner.getOpensearchNode(); + String uniqueId = extensionsRunner.getUniqueId(); TransportActionResponseToExtensionResponseHandler handleTransportActionResponseHandler = new TransportActionResponseToExtensionResponseHandler(); try { diff --git a/src/main/java/org/opensearch/sdk/handlers/ExtensionsInitRequestHandler.java b/src/main/java/org/opensearch/sdk/handlers/ExtensionsInitRequestHandler.java index e995e63e..384769d0 100644 --- a/src/main/java/org/opensearch/sdk/handlers/ExtensionsInitRequestHandler.java +++ b/src/main/java/org/opensearch/sdk/handlers/ExtensionsInitRequestHandler.java @@ -62,12 +62,7 @@ public InitializeExtensionResponse handleExtensionInitRequest(InitializeExtensio extensionTransportService.connectToNode(extensionsRunner.opensearchNode); extensionsRunner.sendRegisterRestActionsRequest(extensionTransportService); extensionsRunner.sendRegisterCustomSettingsRequest(extensionTransportService); - extensionsRunner.getSdkActionModule() - .sendRegisterTransportActionsRequest( - extensionTransportService, - extensionsRunner.opensearchNode, - extensionsRunner.getUniqueId() - ); + extensionsRunner.getSdkActionModule().sendRegisterTransportActionsRequest(); // Get OpenSearch Settings and set values on ExtensionsRunner Settings settings = extensionsRunner.sendEnvironmentSettingsRequest(extensionTransportService); extensionsRunner.setEnvironmentSettings(settings); diff --git a/src/test/java/org/opensearch/sdk/action/TestSDKActionModule.java b/src/test/java/org/opensearch/sdk/action/TestSDKActionModule.java index 7832cfe5..d1d1e355 100644 --- a/src/test/java/org/opensearch/sdk/action/TestSDKActionModule.java +++ b/src/test/java/org/opensearch/sdk/action/TestSDKActionModule.java @@ -24,6 +24,7 @@ import org.opensearch.sdk.ActionExtension; import org.opensearch.sdk.Extension; import org.opensearch.sdk.ExtensionSettings; +import org.opensearch.sdk.ExtensionsRunner; import org.opensearch.sdk.handlers.AcknowledgedResponseHandler; import org.opensearch.test.OpenSearchTestCase; import org.opensearch.transport.Transport; @@ -49,8 +50,10 @@ public class TestSDKActionModule extends OpenSearchTestCase { private static final String TEST_UNIQUE_ID = "test-extension"; private static final String TEST_ACTION_NAME = "testAction"; + private ExtensionsRunner extensionsRunner; private TransportService transportService; private DiscoveryNode opensearchNode; + private SDKActionModule sdkActionModule; private static class TestActionExtension implements Extension, ActionExtension { @Override @@ -68,8 +71,6 @@ public ExtensionSettings getExtensionSettings() { } } - private SDKActionModule sdkActionModule = new SDKActionModule(new TestActionExtension()); - @Override @BeforeEach public void setUp() throws Exception { @@ -92,6 +93,11 @@ public void setUp() throws Exception { emptySet(), Version.CURRENT ); + extensionsRunner = mock(ExtensionsRunner.class); + when(extensionsRunner.getExtensionTransportService()).thenReturn(transportService); + when(extensionsRunner.getOpensearchNode()).thenReturn(opensearchNode); + when(extensionsRunner.getUniqueId()).thenReturn(TEST_UNIQUE_ID); + sdkActionModule = new SDKActionModule(extensionsRunner, new TestActionExtension()); } @Test @@ -99,7 +105,7 @@ public void testRegisterTransportAction() { ArgumentCaptor registerTransportActionsRequestCaptor = ArgumentCaptor.forClass( RegisterTransportActionsRequest.class ); - sdkActionModule.sendRegisterTransportActionsRequest(transportService, opensearchNode, TEST_UNIQUE_ID); + sdkActionModule.sendRegisterTransportActionsRequest(); verify(transportService, times(1)).sendRequest( any(), eq(ExtensionsManager.REQUEST_EXTENSION_REGISTER_TRANSPORT_ACTIONS), diff --git a/src/test/java/org/opensearch/sdk/sample/helloworld/TestHelloWorldExtension.java b/src/test/java/org/opensearch/sdk/sample/helloworld/TestHelloWorldExtension.java index 27d8ce70..2b95284c 100644 --- a/src/test/java/org/opensearch/sdk/sample/helloworld/TestHelloWorldExtension.java +++ b/src/test/java/org/opensearch/sdk/sample/helloworld/TestHelloWorldExtension.java @@ -44,10 +44,12 @@ import com.google.inject.Injector; import com.google.inject.Key; +import static org.mockito.Mockito.mock; import static org.opensearch.sdk.sample.helloworld.ExampleCustomSettingConfig.VALIDATED_SETTING; public class TestHelloWorldExtension extends OpenSearchTestCase { + private ExtensionsRunner extensionsRunner; private HelloWorldExtension extension; private Injector injector; private SDKClient sdkClient; @@ -67,6 +69,7 @@ private UnregisteredAction() { @BeforeEach public void setUp() throws Exception { super.setUp(); + this.extensionsRunner = mock(ExtensionsRunner.class); this.extension = new HelloWorldExtension(); // Do portions of Guice injection needed for this test @@ -74,7 +77,7 @@ public void setUp() throws Exception { ThreadPool threadPool = new ThreadPool(settings); TaskManager taskManager = new TaskManager(settings, threadPool, Collections.emptySet()); this.sdkClient = new SDKClient(extensionSettings); - this.injector = Guice.createInjector(new SDKActionModule(extension), b -> { + this.injector = Guice.createInjector(new SDKActionModule(extensionsRunner, extension), b -> { b.bind(ThreadPool.class).toInstance(threadPool); b.bind(TaskManager.class).toInstance(taskManager); b.bind(SDKClient.class).toInstance(sdkClient); From 7998524b11d271920783c045723f727380c2853c Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Sat, 18 Mar 2023 17:12:12 -0700 Subject: [PATCH 03/20] Add new ProxyActionRequest Signed-off-by: Daniel Widdis --- .../sdk/action/ProxyActionRequest.java | 94 +++++++++++++++++++ ...ionResponseToExtensionResponseHandler.java | 5 +- .../sdk/action/TestProxyActionRequest.java | 39 ++++++++ 3 files changed, 136 insertions(+), 2 deletions(-) create mode 100644 src/main/java/org/opensearch/sdk/action/ProxyActionRequest.java create mode 100644 src/test/java/org/opensearch/sdk/action/TestProxyActionRequest.java diff --git a/src/main/java/org/opensearch/sdk/action/ProxyActionRequest.java b/src/main/java/org/opensearch/sdk/action/ProxyActionRequest.java new file mode 100644 index 00000000..3c9ca6cc --- /dev/null +++ b/src/main/java/org/opensearch/sdk/action/ProxyActionRequest.java @@ -0,0 +1,94 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.sdk.action; + +import java.io.IOException; +import java.util.Objects; + +import org.opensearch.action.ActionRequest; +import org.opensearch.action.ActionRequestValidationException; +import org.opensearch.common.io.stream.StreamInput; +import org.opensearch.common.io.stream.StreamOutput; +import org.opensearch.extensions.action.ExtensionTransportActionsHandler; + +/** + * A request class to request an action be executed on another extension + */ +public class ProxyActionRequest extends ActionRequest { + /** + * action is the transport action intended to be invoked which is registered by an extension via {@link ExtensionTransportActionsHandler}. + */ + private final String action; + /** + * requestBytes is the raw bytes being transported between extensions. + */ + private final byte[] requestBytes; + + /** + * ProxyActionRequest constructor. + * + * @param action is the transport action intended to be invoked which is registered by an extension via {@link ExtensionTransportActionsHandler}. + * @param requestBytes is the raw bytes being transported between extensions. + */ + public ProxyActionRequest(String action, byte[] requestBytes) { + this.action = action; + this.requestBytes = requestBytes; + } + + /** + * ProxyActionRequest constructor from {@link StreamInput}. + * + * @param in bytes stream input used to de-serialize the message. + * @throws IOException when message de-serialization fails. + */ + public ProxyActionRequest(StreamInput in) throws IOException { + super(in); + this.action = in.readString(); + this.requestBytes = in.readByteArray(); + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + super.writeTo(out); + out.writeString(action); + out.writeByteArray(requestBytes); + } + + public String getAction() { + return this.action; + } + + public byte[] getRequestBytes() { + return this.requestBytes; + } + + @Override + public ActionRequestValidationException validate() { + return null; + } + + @Override + public String toString() { + return "ProxyActionRequest{action=" + action + ", requestBytes=" + requestBytes + "}"; + } + + @Override + public boolean equals(Object obj) { + if (this == obj) return true; + if (obj == null || getClass() != obj.getClass()) return false; + ProxyActionRequest that = (ProxyActionRequest) obj; + return Objects.equals(action, that.action) && Objects.equals(requestBytes, that.requestBytes); + } + + @Override + public int hashCode() { + return Objects.hash(action, requestBytes); + } +} diff --git a/src/main/java/org/opensearch/sdk/handlers/TransportActionResponseToExtensionResponseHandler.java b/src/main/java/org/opensearch/sdk/handlers/TransportActionResponseToExtensionResponseHandler.java index c8939fff..865d0009 100644 --- a/src/main/java/org/opensearch/sdk/handlers/TransportActionResponseToExtensionResponseHandler.java +++ b/src/main/java/org/opensearch/sdk/handlers/TransportActionResponseToExtensionResponseHandler.java @@ -13,6 +13,7 @@ import org.apache.logging.log4j.Logger; import org.opensearch.extensions.ExtensionsManager; import org.opensearch.extensions.action.TransportActionResponseToExtension; +import org.opensearch.common.Nullable; import org.opensearch.common.io.stream.StreamInput; import org.opensearch.sdk.action.SDKActionModule; import org.opensearch.threadpool.ThreadPool; @@ -30,14 +31,13 @@ public class TransportActionResponseToExtensionResponseHandler implements Transp private static final Logger logger = LogManager.getLogger(TransportActionResponseToExtensionResponseHandler.class); private final CompletableFuture inProgressFuture; - private byte[] responseBytes; + private byte[] responseBytes = null; /** * Instantiates a new TransportActionResponseToExtensionHandler with a count down latch and an empty Settings object */ public TransportActionResponseToExtensionResponseHandler() { this.inProgressFuture = new CompletableFuture<>(); - this.responseBytes = new byte[0]; } @Override @@ -74,6 +74,7 @@ public void awaitResponse() throws Exception { inProgressFuture.orTimeout(ExtensionsManager.EXTENSION_REQUEST_WAIT_TIMEOUT, TimeUnit.SECONDS).get(); } + @Nullable public byte[] getResponseBytes() { return this.responseBytes; } diff --git a/src/test/java/org/opensearch/sdk/action/TestProxyActionRequest.java b/src/test/java/org/opensearch/sdk/action/TestProxyActionRequest.java new file mode 100644 index 00000000..84d3bf9b --- /dev/null +++ b/src/test/java/org/opensearch/sdk/action/TestProxyActionRequest.java @@ -0,0 +1,39 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.sdk.action; + +import java.nio.charset.StandardCharsets; + +import org.opensearch.common.bytes.BytesReference; +import org.opensearch.common.io.stream.BytesStreamInput; +import org.opensearch.common.io.stream.BytesStreamOutput; +import org.opensearch.test.OpenSearchTestCase; + +public class TestProxyActionRequest extends OpenSearchTestCase { + public void testProxyActionRequest() throws Exception { + String expectedAction = "test-action"; + byte[] expectedRequestBytes = "request-bytes".getBytes(StandardCharsets.UTF_8); + ProxyActionRequest request = new ProxyActionRequest(expectedAction, expectedRequestBytes); + + assertEquals(expectedAction, request.getAction()); + assertEquals(expectedRequestBytes, request.getRequestBytes()); + + try (BytesStreamOutput out = new BytesStreamOutput()) { + request.writeTo(out); + out.flush(); + try (BytesStreamInput in = new BytesStreamInput(BytesReference.toBytes(out.bytes()))) { + request = new ProxyActionRequest(in); + + assertEquals(expectedAction, request.getAction()); + assertArrayEquals(expectedRequestBytes, request.getRequestBytes()); + } + } + } +} From 97529fe4839d9cd6a61883784b42bfd0fe3da303 Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Sat, 18 Mar 2023 18:13:47 -0700 Subject: [PATCH 04/20] Add SDKTransportService wrapper accessible to actions Signed-off-by: Daniel Widdis --- .../org/opensearch/sdk/ExtensionsRunner.java | 10 +++ .../opensearch/sdk/SDKTransportService.java | 87 +++++++++++++++++++ .../sdk/action/SDKActionModule.java | 35 -------- .../ExtensionsInitRequestHandler.java | 6 ++ 4 files changed, 103 insertions(+), 35 deletions(-) create mode 100644 src/main/java/org/opensearch/sdk/SDKTransportService.java diff --git a/src/main/java/org/opensearch/sdk/ExtensionsRunner.java b/src/main/java/org/opensearch/sdk/ExtensionsRunner.java index 1d49777d..bacc35ed 100644 --- a/src/main/java/org/opensearch/sdk/ExtensionsRunner.java +++ b/src/main/java/org/opensearch/sdk/ExtensionsRunner.java @@ -132,6 +132,7 @@ public class ExtensionsRunner { private final SDKNamedXContentRegistry sdkNamedXContentRegistry; private final SDKClient sdkClient; private final SDKClusterService sdkClusterService; + private final SDKTransportService sdkTransportService; private final SDKActionModule sdkActionModule; private ExtensionsInitRequestHandler extensionsInitRequestHandler = new ExtensionsInitRequestHandler(this); @@ -175,6 +176,8 @@ protected ExtensionsRunner(Extension extension) throws IOException { this.sdkClient = new SDKClient(extensionSettings); // initialize SDKClusterService. Must happen after extension field assigned this.sdkClusterService = new SDKClusterService(this); + // initialize SDKTransportService. Must happen after extension field assigned + this.sdkTransportService = new SDKTransportService(); // Create Guice modules for injection List modules = new ArrayList<>(); @@ -189,6 +192,7 @@ protected ExtensionsRunner(Extension extension) throws IOException { b.bind(SDKClient.class).toInstance(getSdkClient()); b.bind(SDKClusterService.class).toInstance(getSdkClusterService()); + b.bind(SDKTransportService.class).toInstance(getSdkTransportService()); }); // Bind the return values from create components modules.add(this::injectComponents); @@ -638,6 +642,10 @@ public TransportService getExtensionTransportService() { return extensionTransportService; } + public SDKTransportService getSdkTransportService() { + return sdkTransportService; + } + /** * Starts an ActionListener. * @@ -660,6 +668,8 @@ public static void run(Extension extension) throws IOException { // initialize the transport service NettyTransport nettyTransport = new NettyTransport(runner); runner.extensionTransportService = nettyTransport.initializeExtensionTransportService(runner.getSettings(), runner.getThreadPool()); + // TODO: merge above line with below line when refactoring out extensionTransportService + runner.getSdkTransportService().setTransportService(runner.extensionTransportService); runner.startActionListener(0); } diff --git a/src/main/java/org/opensearch/sdk/SDKTransportService.java b/src/main/java/org/opensearch/sdk/SDKTransportService.java new file mode 100644 index 00000000..87a35f15 --- /dev/null +++ b/src/main/java/org/opensearch/sdk/SDKTransportService.java @@ -0,0 +1,87 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.sdk; + +import java.util.concurrent.TimeoutException; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.opensearch.cluster.node.DiscoveryNode; +import org.opensearch.extensions.ExtensionsManager; +import org.opensearch.extensions.action.TransportActionRequestFromExtension; +import org.opensearch.sdk.handlers.TransportActionResponseToExtensionResponseHandler; +import org.opensearch.transport.TransportService; + +/** + * Wrapper class for {@link TranspoortService} and associated methods. + * + * TODO: Move all the sendFooRequest() methods here + * TODO: Replace usages of getExtensionTransportService with this class + */ +public class SDKTransportService { + private final Logger logger = LogManager.getLogger(SDKTransportService.class); + + private TransportService transportService; + private DiscoveryNode opensearchNode; + private String uniqueId; + + /** + * Requests that OpenSearch execute a Transport Actions on another extension. + * + * @param action The fully qualified class name of the action to execute + * @param requestBytes A buffer containing serialized parameters to be understood by the remote action + * @return A buffer serializing the response from the remote action if successful, otherwise empty + */ + public byte[] sendProxyActionRequest(String action, byte[] requestBytes) { + logger.info("Sending ProxyAction request to OpenSearch for [" + action + "]"); + TransportActionResponseToExtensionResponseHandler handleTransportActionResponseHandler = + new TransportActionResponseToExtensionResponseHandler(); + try { + transportService.sendRequest( + opensearchNode, + ExtensionsManager.REQUEST_EXTENSION_HANDLE_TRANSPORT_ACTION, + new TransportActionRequestFromExtension(action, requestBytes, uniqueId), + handleTransportActionResponseHandler + ); + // Wait on response + handleTransportActionResponseHandler.awaitResponse(); + } catch (TimeoutException e) { + logger.info("Failed to receive ProxyAction response from OpenSearch", e); + } catch (Exception e) { + logger.info("Failed to send ProxyAction request to OpenSearch", e); + } + // At this point, response handler has read in the response bytes + return handleTransportActionResponseHandler.getResponseBytes(); + } + + public TransportService getTransportService() { + return transportService; + } + + public DiscoveryNode getOpensearchNode() { + return opensearchNode; + } + + public String getUniqueId() { + return uniqueId; + } + + public void setTransportService(TransportService transportService) { + this.transportService = transportService; + } + + public void setOpensearchNode(DiscoveryNode opensearchNode) { + this.opensearchNode = opensearchNode; + } + + public void setUniqueId(String uniqueId) { + this.uniqueId = uniqueId; + } +} diff --git a/src/main/java/org/opensearch/sdk/action/SDKActionModule.java b/src/main/java/org/opensearch/sdk/action/SDKActionModule.java index 55c07c7e..695ada9b 100644 --- a/src/main/java/org/opensearch/sdk/action/SDKActionModule.java +++ b/src/main/java/org/opensearch/sdk/action/SDKActionModule.java @@ -11,7 +11,6 @@ import java.util.Collections; import java.util.Map; -import java.util.concurrent.TimeoutException; import java.util.stream.Collectors; import org.apache.logging.log4j.LogManager; @@ -23,12 +22,10 @@ import org.opensearch.common.NamedRegistry; import org.opensearch.extensions.ExtensionsManager; import org.opensearch.extensions.action.RegisterTransportActionsRequest; -import org.opensearch.extensions.action.TransportActionRequestFromExtension; import org.opensearch.sdk.ActionExtension.ActionHandler; import org.opensearch.sdk.Extension; import org.opensearch.sdk.ExtensionsRunner; import org.opensearch.sdk.handlers.AcknowledgedResponseHandler; -import org.opensearch.sdk.handlers.TransportActionResponseToExtensionResponseHandler; import org.opensearch.transport.TransportService; import com.google.inject.AbstractModule; @@ -147,36 +144,4 @@ public void sendRegisterTransportActionsRequest() { logger.info("Failed to send Register Transport Actions request to OpenSearch", e); } } - - /** - * Requests that OpenSearch execute a Transport Actions on another extension. - * - * @param action The fully qualified class name of the action to execute - * @param requestBytes A buffer containing serialized parameters to be understood by the remote action - * @return A buffer serializing the response from the remote action if successful, otherwise empty - */ - public byte[] sendProxyActionRequest(String action, byte[] requestBytes) { - logger.info("Sending ProxyAction request to OpenSearch for [" + action + "]"); - TransportService transportService = extensionsRunner.getExtensionTransportService(); - DiscoveryNode opensearchNode = extensionsRunner.getOpensearchNode(); - String uniqueId = extensionsRunner.getUniqueId(); - TransportActionResponseToExtensionResponseHandler handleTransportActionResponseHandler = - new TransportActionResponseToExtensionResponseHandler(); - try { - transportService.sendRequest( - opensearchNode, - ExtensionsManager.REQUEST_EXTENSION_HANDLE_TRANSPORT_ACTION, - new TransportActionRequestFromExtension(action, requestBytes, uniqueId), - handleTransportActionResponseHandler - ); - // Wait on response - handleTransportActionResponseHandler.awaitResponse(); - } catch (TimeoutException e) { - logger.info("Failed to receive ProxyAction response from OpenSearch", e); - } catch (Exception e) { - logger.info("Failed to send ProxyAction request to OpenSearch", e); - } - // At this point, response handler has read in the response bytes - return handleTransportActionResponseHandler.getResponseBytes(); - } } diff --git a/src/main/java/org/opensearch/sdk/handlers/ExtensionsInitRequestHandler.java b/src/main/java/org/opensearch/sdk/handlers/ExtensionsInitRequestHandler.java index 384769d0..e452a23b 100644 --- a/src/main/java/org/opensearch/sdk/handlers/ExtensionsInitRequestHandler.java +++ b/src/main/java/org/opensearch/sdk/handlers/ExtensionsInitRequestHandler.java @@ -16,6 +16,7 @@ import org.opensearch.discovery.InitializeExtensionRequest; import org.opensearch.discovery.InitializeExtensionResponse; import org.opensearch.sdk.ExtensionsRunner; +import org.opensearch.sdk.SDKTransportService; import org.opensearch.transport.TransportService; import static org.opensearch.sdk.ExtensionsRunner.NODE_NAME_SETTING; @@ -48,6 +49,10 @@ public InitializeExtensionResponse handleExtensionInitRequest(InitializeExtensio logger.info("Registering Extension Request received from OpenSearch"); extensionsRunner.opensearchNode = extensionInitRequest.getSourceNode(); extensionsRunner.setUniqueId(extensionInitRequest.getExtension().getId()); + // TODO: Remove above two lines in favor of the below when refactoring + SDKTransportService sdkTransportService = extensionsRunner.getSdkTransportService(); + sdkTransportService.setOpensearchNode(extensionInitRequest.getSourceNode()); + sdkTransportService.setUniqueId(extensionInitRequest.getExtension().getId()); // Successfully initialized. Send the response. try { return new InitializeExtensionResponse( @@ -58,6 +63,7 @@ public InitializeExtensionResponse handleExtensionInitRequest(InitializeExtensio // After sending successful response to initialization, send the REST API and Settings extensionsRunner.setOpensearchNode(extensionsRunner.opensearchNode); extensionsRunner.setExtensionNode(extensionInitRequest.getExtension()); + // TODO: replace with sdkTransportService.getTransportService() TransportService extensionTransportService = extensionsRunner.getExtensionTransportService(); extensionTransportService.connectToNode(extensionsRunner.opensearchNode); extensionsRunner.sendRegisterRestActionsRequest(extensionTransportService); From c96474c0f547fbe05b4b048d1f6450ead5425043 Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Sat, 18 Mar 2023 18:23:23 -0700 Subject: [PATCH 05/20] Implement ProxyTransportAction Signed-off-by: Daniel Widdis --- .../opensearch/sdk/SDKTransportService.java | 14 ++++++----- .../sdk/action/ProxyTransportAction.java | 25 +++++++++++++------ 2 files changed, 25 insertions(+), 14 deletions(-) diff --git a/src/main/java/org/opensearch/sdk/SDKTransportService.java b/src/main/java/org/opensearch/sdk/SDKTransportService.java index 87a35f15..2434ee1c 100644 --- a/src/main/java/org/opensearch/sdk/SDKTransportService.java +++ b/src/main/java/org/opensearch/sdk/SDKTransportService.java @@ -14,8 +14,10 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.opensearch.cluster.node.DiscoveryNode; +import org.opensearch.common.Nullable; import org.opensearch.extensions.ExtensionsManager; import org.opensearch.extensions.action.TransportActionRequestFromExtension; +import org.opensearch.sdk.action.ProxyActionRequest; import org.opensearch.sdk.handlers.TransportActionResponseToExtensionResponseHandler; import org.opensearch.transport.TransportService; @@ -35,19 +37,19 @@ public class SDKTransportService { /** * Requests that OpenSearch execute a Transport Actions on another extension. * - * @param action The fully qualified class name of the action to execute - * @param requestBytes A buffer containing serialized parameters to be understood by the remote action - * @return A buffer serializing the response from the remote action if successful, otherwise empty + * @param request The request to send + * @return A buffer serializing the response from the remote action if successful, otherwise null */ - public byte[] sendProxyActionRequest(String action, byte[] requestBytes) { - logger.info("Sending ProxyAction request to OpenSearch for [" + action + "]"); + @Nullable + public byte[] sendProxyActionRequest(ProxyActionRequest request) { + logger.info("Sending ProxyAction request to OpenSearch for [" + request.getAction() + "]"); TransportActionResponseToExtensionResponseHandler handleTransportActionResponseHandler = new TransportActionResponseToExtensionResponseHandler(); try { transportService.sendRequest( opensearchNode, ExtensionsManager.REQUEST_EXTENSION_HANDLE_TRANSPORT_ACTION, - new TransportActionRequestFromExtension(action, requestBytes, uniqueId), + new TransportActionRequestFromExtension(request.getAction(), request.getRequestBytes(), uniqueId), handleTransportActionResponseHandler ); // Wait on response diff --git a/src/main/java/org/opensearch/sdk/action/ProxyTransportAction.java b/src/main/java/org/opensearch/sdk/action/ProxyTransportAction.java index 08482431..daf79fa5 100644 --- a/src/main/java/org/opensearch/sdk/action/ProxyTransportAction.java +++ b/src/main/java/org/opensearch/sdk/action/ProxyTransportAction.java @@ -13,7 +13,7 @@ import org.opensearch.action.support.ActionFilters; import org.opensearch.action.support.TransportAction; import org.opensearch.extensions.action.ExtensionActionResponse; -import org.opensearch.extensions.action.TransportActionRequestFromExtension; +import org.opensearch.sdk.SDKTransportService; import org.opensearch.tasks.Task; import org.opensearch.tasks.TaskManager; @@ -22,7 +22,9 @@ /** * Sends a request to OpenSearch for a remote extension to execute an action. */ -public class ProxyTransportAction extends TransportAction { +public class ProxyTransportAction extends TransportAction { + + private SDKTransportService sdkTransportService; /** * Instantiate this action @@ -30,19 +32,26 @@ public class ProxyTransportAction extends TransportAction listener) { - // SDKActionModule.sendProxyActionRequest(); - if (request == null) { - listener.onFailure(new IllegalArgumentException("The request is null.")); + protected void doExecute(Task task, ProxyActionRequest request, ActionListener listener) { + byte[] responseBytes = sdkTransportService.sendProxyActionRequest(request); + if (responseBytes == null) { + listener.onFailure(new RuntimeException("No response received from remote extension.")); } else { - listener.onResponse(new ExtensionActionResponse(new byte[0])); + listener.onResponse(new ExtensionActionResponse(responseBytes)); } } } From 03a2cac51a49b6f2681e6c39b1878253ef6d8b4a Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Sat, 18 Mar 2023 19:45:52 -0700 Subject: [PATCH 06/20] Add test case to HelloWorldExtension Signed-off-by: Daniel Widdis --- .../org/opensearch/sdk/BaseExtension.java | 16 ++-- .../java/org/opensearch/sdk/Extension.java | 7 ++ .../org/opensearch/sdk/ExtensionsRunner.java | 3 + .../opensearch/sdk/SDKTransportService.java | 1 + .../sdk/action/ProxyActionRequest.java | 21 +++++ ...ionResponseToExtensionResponseHandler.java | 2 +- .../helloworld/HelloWorldExtension.java | 3 +- .../rest/RestRemoteHelloAction.java | 84 +++++++++++++++++++ .../sdk/TestExtensionInterfaces.java | 3 + .../sdk/action/TestProxyActionRequest.java | 34 ++++++++ .../sdk/action/TestSDKActionModule.java | 9 +- .../helloworld/TestHelloWorldExtension.java | 7 +- 12 files changed, 172 insertions(+), 18 deletions(-) create mode 100644 src/main/java/org/opensearch/sdk/sample/helloworld/rest/RestRemoteHelloAction.java diff --git a/src/main/java/org/opensearch/sdk/BaseExtension.java b/src/main/java/org/opensearch/sdk/BaseExtension.java index 0a109c8e..085f1e04 100644 --- a/src/main/java/org/opensearch/sdk/BaseExtension.java +++ b/src/main/java/org/opensearch/sdk/BaseExtension.java @@ -11,8 +11,6 @@ import java.io.IOException; -import com.google.inject.Inject; - /** * An abstract class that simplifies extension initialization and provides an instance of the runner. */ @@ -20,7 +18,6 @@ public abstract class BaseExtension implements Extension { /** * The {@link ExtensionsRunner} instance running this extension */ - @Inject private ExtensionsRunner extensionsRunner; /** @@ -56,11 +53,16 @@ public ExtensionSettings getExtensionSettings() { return this.settings; } + @Override + public void setExtensionsRunner(ExtensionsRunner runner) { + this.extensionsRunner = runner; + } + /** - * Gets the {@link ExtensionsRunner} of this extension. - * - * @return the extension runner. - */ + * Gets the {@link ExtensionsRunner} of this extension. + * + * @return the extension runner. + */ public ExtensionsRunner extensionsRunner() { return this.extensionsRunner; } diff --git a/src/main/java/org/opensearch/sdk/Extension.java b/src/main/java/org/opensearch/sdk/Extension.java index 07c0e73a..710a31b3 100644 --- a/src/main/java/org/opensearch/sdk/Extension.java +++ b/src/main/java/org/opensearch/sdk/Extension.java @@ -25,6 +25,13 @@ */ public interface Extension { + /** + * Set the instance of {@link ExtensionsRunner} for this extension. + * + * @param runner The ExtensionsRunner instance. + */ + public void setExtensionsRunner(ExtensionsRunner runner); + /** * Gets the {@link ExtensionSettings} of this extension. * diff --git a/src/main/java/org/opensearch/sdk/ExtensionsRunner.java b/src/main/java/org/opensearch/sdk/ExtensionsRunner.java index bacc35ed..49075087 100644 --- a/src/main/java/org/opensearch/sdk/ExtensionsRunner.java +++ b/src/main/java/org/opensearch/sdk/ExtensionsRunner.java @@ -153,7 +153,10 @@ public class ExtensionsRunner { * @throws IOException if the runner failed to read settings or API. */ protected ExtensionsRunner(Extension extension) throws IOException { + // Link these classes together this.extension = extension; + extension.setExtensionsRunner(this); + // Initialize concrete classes needed by extensions // These must have getters from this class to be accessible via createComponents // If they require later initialization, create a concrete wrapper class and update the internals diff --git a/src/main/java/org/opensearch/sdk/SDKTransportService.java b/src/main/java/org/opensearch/sdk/SDKTransportService.java index 2434ee1c..fc2851de 100644 --- a/src/main/java/org/opensearch/sdk/SDKTransportService.java +++ b/src/main/java/org/opensearch/sdk/SDKTransportService.java @@ -26,6 +26,7 @@ * * TODO: Move all the sendFooRequest() methods here * TODO: Replace usages of getExtensionTransportService with this class + * https://github.com/opensearch-project/opensearch-sdk-java/issues/585 */ public class SDKTransportService { private final Logger logger = LogManager.getLogger(SDKTransportService.class); diff --git a/src/main/java/org/opensearch/sdk/action/ProxyActionRequest.java b/src/main/java/org/opensearch/sdk/action/ProxyActionRequest.java index 3c9ca6cc..c694729c 100644 --- a/src/main/java/org/opensearch/sdk/action/ProxyActionRequest.java +++ b/src/main/java/org/opensearch/sdk/action/ProxyActionRequest.java @@ -14,6 +14,8 @@ import org.opensearch.action.ActionRequest; import org.opensearch.action.ActionRequestValidationException; +import org.opensearch.common.bytes.BytesReference; +import org.opensearch.common.io.stream.BytesStreamOutput; import org.opensearch.common.io.stream.StreamInput; import org.opensearch.common.io.stream.StreamOutput; import org.opensearch.extensions.action.ExtensionTransportActionsHandler; @@ -42,6 +44,25 @@ public ProxyActionRequest(String action, byte[] requestBytes) { this.requestBytes = requestBytes; } + /** + * ProxyAcctionRequest constructor with a request class + * + * @param request A class extending {@link ActionRequest} associated with an action to be executed on another extension. + */ + public ProxyActionRequest(ActionRequest request) { + this.action = request.getClass().getName(); + byte[] bytes = new byte[0]; + try (BytesStreamOutput out = new BytesStreamOutput()) { + request.writeTo(out); + out.flush(); + bytes = BytesReference.toBytes(out.bytes()); + } catch (IOException e) { + // This Should Never Happen (TM) + // Won't get an IOException locally + } + this.requestBytes = bytes; + } + /** * ProxyActionRequest constructor from {@link StreamInput}. * diff --git a/src/main/java/org/opensearch/sdk/handlers/TransportActionResponseToExtensionResponseHandler.java b/src/main/java/org/opensearch/sdk/handlers/TransportActionResponseToExtensionResponseHandler.java index 865d0009..22574a25 100644 --- a/src/main/java/org/opensearch/sdk/handlers/TransportActionResponseToExtensionResponseHandler.java +++ b/src/main/java/org/opensearch/sdk/handlers/TransportActionResponseToExtensionResponseHandler.java @@ -34,7 +34,7 @@ public class TransportActionResponseToExtensionResponseHandler implements Transp private byte[] responseBytes = null; /** - * Instantiates a new TransportActionResponseToExtensionHandler with a count down latch and an empty Settings object + * Instantiates a new TransportActionResponseToExtensionHandler */ public TransportActionResponseToExtensionResponseHandler() { this.inProgressFuture = new CompletableFuture<>(); diff --git a/src/main/java/org/opensearch/sdk/sample/helloworld/HelloWorldExtension.java b/src/main/java/org/opensearch/sdk/sample/helloworld/HelloWorldExtension.java index faf48e03..caee8766 100644 --- a/src/main/java/org/opensearch/sdk/sample/helloworld/HelloWorldExtension.java +++ b/src/main/java/org/opensearch/sdk/sample/helloworld/HelloWorldExtension.java @@ -24,6 +24,7 @@ import org.opensearch.sdk.ActionExtension; import org.opensearch.sdk.ActionExtension.ActionHandler; import org.opensearch.sdk.sample.helloworld.rest.RestHelloAction; +import org.opensearch.sdk.sample.helloworld.rest.RestRemoteHelloAction; import org.opensearch.sdk.sample.helloworld.transport.SampleAction; import org.opensearch.sdk.sample.helloworld.transport.SampleTransportAction; @@ -56,7 +57,7 @@ public HelloWorldExtension() { @Override public List getExtensionRestHandlers() { - return List.of(new RestHelloAction()); + return List.of(new RestHelloAction(), new RestRemoteHelloAction(extensionsRunner())); } @Override diff --git a/src/main/java/org/opensearch/sdk/sample/helloworld/rest/RestRemoteHelloAction.java b/src/main/java/org/opensearch/sdk/sample/helloworld/rest/RestRemoteHelloAction.java new file mode 100644 index 00000000..5fbe93ba --- /dev/null +++ b/src/main/java/org/opensearch/sdk/sample/helloworld/rest/RestRemoteHelloAction.java @@ -0,0 +1,84 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.sdk.sample.helloworld.rest; + +import org.opensearch.action.ActionListener; +import org.opensearch.extensions.ExtensionsManager; +import org.opensearch.extensions.action.ExtensionActionResponse; +import org.opensearch.extensions.rest.ExtensionRestRequest; +import org.opensearch.extensions.rest.ExtensionRestResponse; +import org.opensearch.sdk.BaseExtensionRestHandler; +import org.opensearch.sdk.ExtensionsRunner; +import org.opensearch.sdk.RouteHandler; +import org.opensearch.sdk.SDKClient; +import org.opensearch.sdk.action.ProxyAction; +import org.opensearch.sdk.action.ProxyActionRequest; +import org.opensearch.sdk.sample.helloworld.transport.SampleRequest; + +import java.util.List; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.TimeUnit; +import java.util.function.Function; + +import static org.opensearch.rest.RestRequest.Method.GET; +import static org.opensearch.rest.RestStatus.OK; + +/** + * Sample REST Handler demostrating proxy actions to another extension + */ +public class RestRemoteHelloAction extends BaseExtensionRestHandler { + + private ExtensionsRunner extensionsRunner; + + /** + * Instantiate this action + * + * @param runner The ExtensionsRunner instance + */ + public RestRemoteHelloAction(ExtensionsRunner runner) { + this.extensionsRunner = runner; + } + + @Override + public List routeHandlers() { + return List.of(new RouteHandler(GET, "/hello/{name}", handleRemoteGetRequest)); + } + + private Function handleRemoteGetRequest = (request) -> { + SDKClient client = extensionsRunner.getSdkClient(); + + String name = request.param("name"); + // Create a request using class on remote + // This class happens to be local for simplicity but this should be from a dependency + SampleRequest sampleRequest = new SampleRequest(name); + // Serialize this request in a proxy action request + ProxyActionRequest proxyActionRequest = new ProxyActionRequest(sampleRequest); + + // TODO: We need async client.execute to hide these action listener details and return the future directly + // https://github.com/opensearch-project/opensearch-sdk-java/issues/584 + CompletableFuture futureResponse = new CompletableFuture<>(); + client.execute( + ProxyAction.INSTANCE, + proxyActionRequest, + ActionListener.wrap(r -> futureResponse.complete(r), e -> futureResponse.completeExceptionally(e)) + ); + try { + return new ExtensionRestResponse( + request, + OK, + "Received remote extension reponse: " + + futureResponse.orTimeout(ExtensionsManager.EXTENSION_REQUEST_WAIT_TIMEOUT, TimeUnit.SECONDS).get().toString() + ); + } catch (Exception e) { + return exceptionalRequest(request, e); + } + }; + +} diff --git a/src/test/java/org/opensearch/sdk/TestExtensionInterfaces.java b/src/test/java/org/opensearch/sdk/TestExtensionInterfaces.java index 72964c09..c0bf043f 100644 --- a/src/test/java/org/opensearch/sdk/TestExtensionInterfaces.java +++ b/src/test/java/org/opensearch/sdk/TestExtensionInterfaces.java @@ -34,6 +34,9 @@ void testExtension() { public ExtensionSettings getExtensionSettings() { return null; } + + @Override + public void setExtensionsRunner(ExtensionsRunner runner) {} }; assertTrue(extension.getSettings().isEmpty()); diff --git a/src/test/java/org/opensearch/sdk/action/TestProxyActionRequest.java b/src/test/java/org/opensearch/sdk/action/TestProxyActionRequest.java index 84d3bf9b..b12e3f84 100644 --- a/src/test/java/org/opensearch/sdk/action/TestProxyActionRequest.java +++ b/src/test/java/org/opensearch/sdk/action/TestProxyActionRequest.java @@ -9,11 +9,15 @@ package org.opensearch.sdk.action; +import java.io.IOException; import java.nio.charset.StandardCharsets; +import org.opensearch.action.ActionRequest; +import org.opensearch.action.ActionRequestValidationException; import org.opensearch.common.bytes.BytesReference; import org.opensearch.common.io.stream.BytesStreamInput; import org.opensearch.common.io.stream.BytesStreamOutput; +import org.opensearch.common.io.stream.StreamOutput; import org.opensearch.test.OpenSearchTestCase; public class TestProxyActionRequest extends OpenSearchTestCase { @@ -36,4 +40,34 @@ public void testProxyActionRequest() throws Exception { } } } + + public void testProxyActionRequestWithClass() throws Exception { + class TestRequest extends ActionRequest { + + private String data; + + public TestRequest(String data) { + this.data = data; + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + super.writeTo(out); + out.writeString(data); + } + + @Override + public ActionRequestValidationException validate() { + return null; + } + } + + ProxyActionRequest request = new ProxyActionRequest(new TestRequest("test-action")); + + String expectedAction = request.getClass().getName(); + byte[] expectedRequestBytes = "test-action".getBytes(StandardCharsets.UTF_8); + + assertEquals(expectedAction, request.getAction()); + assertEquals(expectedRequestBytes, request.getRequestBytes()); + } } diff --git a/src/test/java/org/opensearch/sdk/action/TestSDKActionModule.java b/src/test/java/org/opensearch/sdk/action/TestSDKActionModule.java index d1d1e355..a3f0458f 100644 --- a/src/test/java/org/opensearch/sdk/action/TestSDKActionModule.java +++ b/src/test/java/org/opensearch/sdk/action/TestSDKActionModule.java @@ -22,7 +22,7 @@ import org.opensearch.extensions.ExtensionsManager; import org.opensearch.extensions.action.RegisterTransportActionsRequest; import org.opensearch.sdk.ActionExtension; -import org.opensearch.sdk.Extension; +import org.opensearch.sdk.BaseExtension; import org.opensearch.sdk.ExtensionSettings; import org.opensearch.sdk.ExtensionsRunner; import org.opensearch.sdk.handlers.AcknowledgedResponseHandler; @@ -55,10 +55,9 @@ public class TestSDKActionModule extends OpenSearchTestCase { private DiscoveryNode opensearchNode; private SDKActionModule sdkActionModule; - private static class TestActionExtension implements Extension, ActionExtension { - @Override - public ExtensionSettings getExtensionSettings() { - return null; + private static class TestActionExtension extends BaseExtension implements ActionExtension { + public TestActionExtension() { + super(mock(ExtensionSettings.class)); } @Override diff --git a/src/test/java/org/opensearch/sdk/sample/helloworld/TestHelloWorldExtension.java b/src/test/java/org/opensearch/sdk/sample/helloworld/TestHelloWorldExtension.java index 2b95284c..f39677fe 100644 --- a/src/test/java/org/opensearch/sdk/sample/helloworld/TestHelloWorldExtension.java +++ b/src/test/java/org/opensearch/sdk/sample/helloworld/TestHelloWorldExtension.java @@ -25,7 +25,6 @@ import org.opensearch.action.ActionType; import org.opensearch.action.support.TransportAction; import org.opensearch.common.settings.Settings; -import org.opensearch.rest.RestHandler.Route; import org.opensearch.sdk.ActionExtension.ActionHandler; import org.opensearch.sdk.sample.helloworld.transport.SampleAction; import org.opensearch.sdk.sample.helloworld.transport.SampleRequest; @@ -113,9 +112,9 @@ public void testExtensionSettings() { @Test public void testExtensionRestHandlers() { List extensionRestHandlers = extension.getExtensionRestHandlers(); - assertEquals(1, extensionRestHandlers.size()); - List routes = extensionRestHandlers.get(0).routes(); - assertEquals(4, routes.size()); + assertEquals(2, extensionRestHandlers.size()); + assertEquals(4, extensionRestHandlers.get(0).routes().size()); + assertEquals(1, extensionRestHandlers.get(1).routes().size()); } @Test From e3d26a3cea0beef5a8c63c9594591cb4f1d54ca3 Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Sat, 18 Mar 2023 21:48:58 -0700 Subject: [PATCH 07/20] Better naming of ExtensionActionResponse and correct action name Signed-off-by: Daniel Widdis --- .../org/opensearch/sdk/SDKTransportService.java | 13 ++++++------- ...ler.java => ExtensionActionResponseHandler.java} | 6 +++--- 2 files changed, 9 insertions(+), 10 deletions(-) rename src/main/java/org/opensearch/sdk/handlers/{TransportActionResponseToExtensionResponseHandler.java => ExtensionActionResponseHandler.java} (88%) diff --git a/src/main/java/org/opensearch/sdk/SDKTransportService.java b/src/main/java/org/opensearch/sdk/SDKTransportService.java index fc2851de..ab4f0bae 100644 --- a/src/main/java/org/opensearch/sdk/SDKTransportService.java +++ b/src/main/java/org/opensearch/sdk/SDKTransportService.java @@ -18,7 +18,7 @@ import org.opensearch.extensions.ExtensionsManager; import org.opensearch.extensions.action.TransportActionRequestFromExtension; import org.opensearch.sdk.action.ProxyActionRequest; -import org.opensearch.sdk.handlers.TransportActionResponseToExtensionResponseHandler; +import org.opensearch.sdk.handlers.ExtensionActionResponseHandler; import org.opensearch.transport.TransportService; /** @@ -44,24 +44,23 @@ public class SDKTransportService { @Nullable public byte[] sendProxyActionRequest(ProxyActionRequest request) { logger.info("Sending ProxyAction request to OpenSearch for [" + request.getAction() + "]"); - TransportActionResponseToExtensionResponseHandler handleTransportActionResponseHandler = - new TransportActionResponseToExtensionResponseHandler(); + ExtensionActionResponseHandler extensionActionResponseHandler = new ExtensionActionResponseHandler(); try { transportService.sendRequest( opensearchNode, - ExtensionsManager.REQUEST_EXTENSION_HANDLE_TRANSPORT_ACTION, + ExtensionsManager.TRANSPORT_ACTION_REQUEST_FROM_EXTENSION, new TransportActionRequestFromExtension(request.getAction(), request.getRequestBytes(), uniqueId), - handleTransportActionResponseHandler + extensionActionResponseHandler ); // Wait on response - handleTransportActionResponseHandler.awaitResponse(); + extensionActionResponseHandler.awaitResponse(); } catch (TimeoutException e) { logger.info("Failed to receive ProxyAction response from OpenSearch", e); } catch (Exception e) { logger.info("Failed to send ProxyAction request to OpenSearch", e); } // At this point, response handler has read in the response bytes - return handleTransportActionResponseHandler.getResponseBytes(); + return extensionActionResponseHandler.getResponseBytes(); } public TransportService getTransportService() { diff --git a/src/main/java/org/opensearch/sdk/handlers/TransportActionResponseToExtensionResponseHandler.java b/src/main/java/org/opensearch/sdk/handlers/ExtensionActionResponseHandler.java similarity index 88% rename from src/main/java/org/opensearch/sdk/handlers/TransportActionResponseToExtensionResponseHandler.java rename to src/main/java/org/opensearch/sdk/handlers/ExtensionActionResponseHandler.java index 22574a25..37595613 100644 --- a/src/main/java/org/opensearch/sdk/handlers/TransportActionResponseToExtensionResponseHandler.java +++ b/src/main/java/org/opensearch/sdk/handlers/ExtensionActionResponseHandler.java @@ -27,16 +27,16 @@ /** * This class handles the response from OpenSearch to a {@link SDKActionModule#sendProxyActionRequest()} call. */ -public class TransportActionResponseToExtensionResponseHandler implements TransportResponseHandler { +public class ExtensionActionResponseHandler implements TransportResponseHandler { - private static final Logger logger = LogManager.getLogger(TransportActionResponseToExtensionResponseHandler.class); + private static final Logger logger = LogManager.getLogger(ExtensionActionResponseHandler.class); private final CompletableFuture inProgressFuture; private byte[] responseBytes = null; /** * Instantiates a new TransportActionResponseToExtensionHandler */ - public TransportActionResponseToExtensionResponseHandler() { + public ExtensionActionResponseHandler() { this.inProgressFuture = new CompletableFuture<>(); } From 77e436b4fa90d54333edf1db52dbdd1185c116d9 Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Sun, 19 Mar 2023 16:59:56 -0700 Subject: [PATCH 08/20] Refactoring with TransportService and latest OpenSearch PR updates Signed-off-by: Daniel Widdis --- .../org/opensearch/sdk/ExtensionsRunner.java | 2 +- .../java/org/opensearch/sdk/SDKClient.java | 16 +++ .../opensearch/sdk/SDKTransportService.java | 28 ++++++ .../sdk/action/ProxyActionRequest.java | 11 ++- .../sdk/action/ProxyTransportAction.java | 2 +- .../sdk/action/SDKActionModule.java | 38 +------- .../ExtensionActionResponseHandler.java | 20 ++-- .../ExtensionsInitRequestHandler.java | 2 +- .../rest/RestRemoteHelloAction.java | 14 ++- .../sdk/TestExtensionInterfaces.java | 1 - .../sdk/TestSDKTransportService.java | 97 +++++++++++++++++++ .../sdk/action/TestProxyActionRequest.java | 61 ++++++++---- .../sdk/action/TestSDKActionModule.java | 70 ++----------- .../helloworld/TestHelloWorldExtension.java | 8 +- 14 files changed, 224 insertions(+), 146 deletions(-) create mode 100644 src/test/java/org/opensearch/sdk/TestSDKTransportService.java diff --git a/src/main/java/org/opensearch/sdk/ExtensionsRunner.java b/src/main/java/org/opensearch/sdk/ExtensionsRunner.java index 49075087..33694f07 100644 --- a/src/main/java/org/opensearch/sdk/ExtensionsRunner.java +++ b/src/main/java/org/opensearch/sdk/ExtensionsRunner.java @@ -200,7 +200,7 @@ protected ExtensionsRunner(Extension extension) throws IOException { // Bind the return values from create components modules.add(this::injectComponents); // Bind actions from getActions - this.sdkActionModule = new SDKActionModule(this, extension); + this.sdkActionModule = new SDKActionModule(extension); modules.add(this.sdkActionModule); // Finally, perform the injection this.injector = Guice.createInjector(modules); diff --git a/src/main/java/org/opensearch/sdk/SDKClient.java b/src/main/java/org/opensearch/sdk/SDKClient.java index a389ba01..bfb57405 100644 --- a/src/main/java/org/opensearch/sdk/SDKClient.java +++ b/src/main/java/org/opensearch/sdk/SDKClient.java @@ -13,6 +13,7 @@ import java.io.IOException; import java.util.Collections; import java.util.Map; +import java.util.stream.Collectors; import com.fasterxml.jackson.annotation.JsonTypeInfo; import com.fasterxml.jackson.databind.DeserializationFeature; @@ -100,6 +101,9 @@ public SDKClient(ExtensionSettings extensionSettings) { // Used by client.execute, populated by initialize method @SuppressWarnings("rawtypes") private Map actions = Collections.emptyMap(); + // Used by remote client execution where we get a string for the class name + @SuppressWarnings("rawtypes") + private Map actionClassToInstanceMap = Collections.emptyMap(); /** * Initialize this client. @@ -109,6 +113,7 @@ public SDKClient(ExtensionSettings extensionSettings) { @SuppressWarnings("rawtypes") public void initialize(Map actions) { this.actions = actions; + this.actionClassToInstanceMap = actions.keySet().stream().collect(Collectors.toMap(a -> a.getClass().getName(), a -> a)); } /** @@ -285,6 +290,17 @@ public void close() throws IOException { doCloseHighLevelClient(); } + /** + * Gets an instance of {@link ActionType} from its corresponding class name, suitable for using as the first parameter in {@link #execute(ActionType, ActionRequest, ActionListener)}. + * + * @param className The class name of the action type + * @return The instance corresponding to the class name + */ + @SuppressWarnings("unchecked") + public ActionType getActionFromClassName(String className) { + return actionClassToInstanceMap.get(className); + } + /** * Executes a generic action, denoted by an {@link ActionType}. * diff --git a/src/main/java/org/opensearch/sdk/SDKTransportService.java b/src/main/java/org/opensearch/sdk/SDKTransportService.java index ab4f0bae..a6ff572b 100644 --- a/src/main/java/org/opensearch/sdk/SDKTransportService.java +++ b/src/main/java/org/opensearch/sdk/SDKTransportService.java @@ -9,15 +9,22 @@ package org.opensearch.sdk; +import java.util.Map; +import java.util.Set; import java.util.concurrent.TimeoutException; +import java.util.stream.Collectors; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.opensearch.cluster.node.DiscoveryNode; import org.opensearch.common.Nullable; import org.opensearch.extensions.ExtensionsManager; +import org.opensearch.extensions.action.RegisterTransportActionsRequest; import org.opensearch.extensions.action.TransportActionRequestFromExtension; +import org.opensearch.sdk.ActionExtension.ActionHandler; import org.opensearch.sdk.action.ProxyActionRequest; +import org.opensearch.sdk.action.SDKActionModule; +import org.opensearch.sdk.handlers.AcknowledgedResponseHandler; import org.opensearch.sdk.handlers.ExtensionActionResponseHandler; import org.opensearch.transport.TransportService; @@ -35,6 +42,27 @@ public class SDKTransportService { private DiscoveryNode opensearchNode; private String uniqueId; + /** + * Requests that OpenSearch register the Transport Actions for this extension. + * + * @param actions The map of registered actions from {@link SDKActionModule#getActions()} + */ + public void sendRegisterTransportActionsRequest(Map> actions) { + logger.info("Sending Register Transport Actions request to OpenSearch"); + Set actionNameSet = actions.values().stream().map(h -> h.getAction().getClass().getName()).collect(Collectors.toSet()); + AcknowledgedResponseHandler registerTransportActionsResponseHandler = new AcknowledgedResponseHandler(); + try { + transportService.sendRequest( + opensearchNode, + ExtensionsManager.REQUEST_EXTENSION_REGISTER_TRANSPORT_ACTIONS, + new RegisterTransportActionsRequest(uniqueId, actionNameSet), + registerTransportActionsResponseHandler + ); + } catch (Exception e) { + logger.info("Failed to send Register Transport Actions request to OpenSearch", e); + } + } + /** * Requests that OpenSearch execute a Transport Actions on another extension. * diff --git a/src/main/java/org/opensearch/sdk/action/ProxyActionRequest.java b/src/main/java/org/opensearch/sdk/action/ProxyActionRequest.java index c694729c..c5e17c30 100644 --- a/src/main/java/org/opensearch/sdk/action/ProxyActionRequest.java +++ b/src/main/java/org/opensearch/sdk/action/ProxyActionRequest.java @@ -14,6 +14,8 @@ import org.opensearch.action.ActionRequest; import org.opensearch.action.ActionRequestValidationException; +import org.opensearch.action.ActionResponse; +import org.opensearch.action.ActionType; import org.opensearch.common.bytes.BytesReference; import org.opensearch.common.io.stream.BytesStreamOutput; import org.opensearch.common.io.stream.StreamInput; @@ -36,7 +38,7 @@ public class ProxyActionRequest extends ActionRequest { /** * ProxyActionRequest constructor. * - * @param action is the transport action intended to be invoked which is registered by an extension via {@link ExtensionTransportActionsHandler}. + * @param action is the ActionType class name intended to be invoked which is registered by an extension via {@link ExtensionTransportActionsHandler}. * @param requestBytes is the raw bytes being transported between extensions. */ public ProxyActionRequest(String action, byte[] requestBytes) { @@ -45,12 +47,13 @@ public ProxyActionRequest(String action, byte[] requestBytes) { } /** - * ProxyAcctionRequest constructor with a request class + * ProxyAcctionRequest constructor with an ActionType and Request class * + * @param instance An instance of {@link ActionType} registered with the remote extension's getActions registry * @param request A class extending {@link ActionRequest} associated with an action to be executed on another extension. */ - public ProxyActionRequest(ActionRequest request) { - this.action = request.getClass().getName(); + public ProxyActionRequest(ActionType instance, ActionRequest request) { + this.action = instance.getClass().getName(); byte[] bytes = new byte[0]; try (BytesStreamOutput out = new BytesStreamOutput()) { request.writeTo(out); diff --git a/src/main/java/org/opensearch/sdk/action/ProxyTransportAction.java b/src/main/java/org/opensearch/sdk/action/ProxyTransportAction.java index daf79fa5..1fd7cd18 100644 --- a/src/main/java/org/opensearch/sdk/action/ProxyTransportAction.java +++ b/src/main/java/org/opensearch/sdk/action/ProxyTransportAction.java @@ -51,7 +51,7 @@ protected void doExecute(Task task, ProxyActionRequest request, ActionListener> actions; private final ActionFilters actionFilters; /** * Instantiate this module * - * @param extensionsRunner The ExtensionsRunner instance * @param extension The extension */ - public SDKActionModule(ExtensionsRunner extensionsRunner, Extension extension) { - this.extensionsRunner = extensionsRunner; + public SDKActionModule(Extension extension) { this.actions = setupActions(extension); this.actionFilters = setupActionFilters(extension); } @@ -108,8 +95,6 @@ protected void configure() { // Bind action filters bind(ActionFilters.class).toInstance(actionFilters); - // Bind local actions - // bind ActionType -> transportAction Map used by Client @SuppressWarnings("rawtypes") MapBinder transportActionsBinder = MapBinder.newMapBinder( @@ -123,25 +108,4 @@ protected void configure() { transportActionsBinder.addBinding(action.getAction()).to(action.getTransportAction()).asEagerSingleton(); } } - - /** - * Requests that OpenSearch register the Transport Actions for this extension. - */ - public void sendRegisterTransportActionsRequest() { - logger.info("Sending Register Transport Actions request to OpenSearch"); - TransportService transportService = extensionsRunner.getExtensionTransportService(); - DiscoveryNode opensearchNode = extensionsRunner.getOpensearchNode(); - String uniqueId = extensionsRunner.getUniqueId(); - AcknowledgedResponseHandler registerTransportActionsResponseHandler = new AcknowledgedResponseHandler(); - try { - transportService.sendRequest( - opensearchNode, - ExtensionsManager.REQUEST_EXTENSION_REGISTER_TRANSPORT_ACTIONS, - new RegisterTransportActionsRequest(uniqueId, getActions().keySet()), - registerTransportActionsResponseHandler - ); - } catch (Exception e) { - logger.info("Failed to send Register Transport Actions request to OpenSearch", e); - } - } } diff --git a/src/main/java/org/opensearch/sdk/handlers/ExtensionActionResponseHandler.java b/src/main/java/org/opensearch/sdk/handlers/ExtensionActionResponseHandler.java index 37595613..518b547e 100644 --- a/src/main/java/org/opensearch/sdk/handlers/ExtensionActionResponseHandler.java +++ b/src/main/java/org/opensearch/sdk/handlers/ExtensionActionResponseHandler.java @@ -12,7 +12,7 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.opensearch.extensions.ExtensionsManager; -import org.opensearch.extensions.action.TransportActionResponseToExtension; +import org.opensearch.extensions.action.ExtensionActionResponse; import org.opensearch.common.Nullable; import org.opensearch.common.io.stream.StreamInput; import org.opensearch.sdk.action.SDKActionModule; @@ -27,31 +27,31 @@ /** * This class handles the response from OpenSearch to a {@link SDKActionModule#sendProxyActionRequest()} call. */ -public class ExtensionActionResponseHandler implements TransportResponseHandler { +public class ExtensionActionResponseHandler implements TransportResponseHandler { private static final Logger logger = LogManager.getLogger(ExtensionActionResponseHandler.class); - private final CompletableFuture inProgressFuture; + private final CompletableFuture inProgressFuture; private byte[] responseBytes = null; /** - * Instantiates a new TransportActionResponseToExtensionHandler + * Instantiates a new ExtensionActionResponseHandler */ public ExtensionActionResponseHandler() { this.inProgressFuture = new CompletableFuture<>(); } @Override - public void handleResponse(TransportActionResponseToExtension response) { + public void handleResponse(ExtensionActionResponse response) { logger.info("received {}", response); - // Set TransportActionResponseToExtension from response + // Set ExtensionActionResponse from response this.responseBytes = response.getResponseBytes(); inProgressFuture.complete(response); } @Override public void handleException(TransportException exp) { - logger.info("TransportActionResponseToExtensionRequest failed", exp); + logger.info("ExtensionActionResponseRequest failed", exp); inProgressFuture.completeExceptionally(exp); } @@ -61,12 +61,12 @@ public String executor() { } @Override - public TransportActionResponseToExtension read(StreamInput in) throws IOException { - return new TransportActionResponseToExtension(in); + public ExtensionActionResponse read(StreamInput in) throws IOException { + return new ExtensionActionResponse(in); } /** - * Waits for the TransportActionResponseToExtensionHandler future to complete + * Waits for the ExtensionActionResponseHandler future to complete * @throws Exception * if the response times out */ diff --git a/src/main/java/org/opensearch/sdk/handlers/ExtensionsInitRequestHandler.java b/src/main/java/org/opensearch/sdk/handlers/ExtensionsInitRequestHandler.java index e452a23b..90444878 100644 --- a/src/main/java/org/opensearch/sdk/handlers/ExtensionsInitRequestHandler.java +++ b/src/main/java/org/opensearch/sdk/handlers/ExtensionsInitRequestHandler.java @@ -68,7 +68,7 @@ public InitializeExtensionResponse handleExtensionInitRequest(InitializeExtensio extensionTransportService.connectToNode(extensionsRunner.opensearchNode); extensionsRunner.sendRegisterRestActionsRequest(extensionTransportService); extensionsRunner.sendRegisterCustomSettingsRequest(extensionTransportService); - extensionsRunner.getSdkActionModule().sendRegisterTransportActionsRequest(); + sdkTransportService.sendRegisterTransportActionsRequest(extensionsRunner.getSdkActionModule().getActions()); // Get OpenSearch Settings and set values on ExtensionsRunner Settings settings = extensionsRunner.sendEnvironmentSettingsRequest(extensionTransportService); extensionsRunner.setEnvironmentSettings(settings); diff --git a/src/main/java/org/opensearch/sdk/sample/helloworld/rest/RestRemoteHelloAction.java b/src/main/java/org/opensearch/sdk/sample/helloworld/rest/RestRemoteHelloAction.java index 5fbe93ba..1cd8ca1d 100644 --- a/src/main/java/org/opensearch/sdk/sample/helloworld/rest/RestRemoteHelloAction.java +++ b/src/main/java/org/opensearch/sdk/sample/helloworld/rest/RestRemoteHelloAction.java @@ -20,8 +20,10 @@ import org.opensearch.sdk.SDKClient; import org.opensearch.sdk.action.ProxyAction; import org.opensearch.sdk.action.ProxyActionRequest; +import org.opensearch.sdk.sample.helloworld.transport.SampleAction; import org.opensearch.sdk.sample.helloworld.transport.SampleRequest; +import java.nio.charset.StandardCharsets; import java.util.List; import java.util.concurrent.CompletableFuture; import java.util.concurrent.TimeUnit; @@ -59,7 +61,8 @@ public List routeHandlers() { // This class happens to be local for simplicity but this should be from a dependency SampleRequest sampleRequest = new SampleRequest(name); // Serialize this request in a proxy action request - ProxyActionRequest proxyActionRequest = new ProxyActionRequest(sampleRequest); + // This requires that the remote extension has a corresponding action registered + ProxyActionRequest proxyActionRequest = new ProxyActionRequest(SampleAction.INSTANCE, sampleRequest); // TODO: We need async client.execute to hide these action listener details and return the future directly // https://github.com/opensearch-project/opensearch-sdk-java/issues/584 @@ -70,11 +73,16 @@ public List routeHandlers() { ActionListener.wrap(r -> futureResponse.complete(r), e -> futureResponse.completeExceptionally(e)) ); try { + byte[] responseBytes = futureResponse.orTimeout(ExtensionsManager.EXTENSION_REQUEST_WAIT_TIMEOUT, TimeUnit.SECONDS) + .get() + .getResponseBytes(); + if (responseBytes == null) { + return new ExtensionRestResponse(request, OK, "Received null remote extension reponse: "); + } return new ExtensionRestResponse( request, OK, - "Received remote extension reponse: " - + futureResponse.orTimeout(ExtensionsManager.EXTENSION_REQUEST_WAIT_TIMEOUT, TimeUnit.SECONDS).get().toString() + "Received remote extension reponse: " + new String(responseBytes, StandardCharsets.UTF_8) ); } catch (Exception e) { return exceptionalRequest(request, e); diff --git a/src/test/java/org/opensearch/sdk/TestExtensionInterfaces.java b/src/test/java/org/opensearch/sdk/TestExtensionInterfaces.java index c0bf043f..3339ed33 100644 --- a/src/test/java/org/opensearch/sdk/TestExtensionInterfaces.java +++ b/src/test/java/org/opensearch/sdk/TestExtensionInterfaces.java @@ -22,7 +22,6 @@ import org.opensearch.ingest.Processor; import org.opensearch.test.OpenSearchTestCase; -import java.util.Map; import java.util.function.Predicate; public class TestExtensionInterfaces extends OpenSearchTestCase { diff --git a/src/test/java/org/opensearch/sdk/TestSDKTransportService.java b/src/test/java/org/opensearch/sdk/TestSDKTransportService.java new file mode 100644 index 00000000..3aafd329 --- /dev/null +++ b/src/test/java/org/opensearch/sdk/TestSDKTransportService.java @@ -0,0 +1,97 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.sdk; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; +import org.opensearch.Version; +import org.opensearch.cluster.node.DiscoveryNode; +import org.opensearch.common.settings.Settings; +import org.opensearch.common.transport.TransportAddress; +import org.opensearch.extensions.ExtensionsManager; +import org.opensearch.extensions.action.RegisterTransportActionsRequest; +import org.opensearch.sdk.action.ProxyAction; +import org.opensearch.sdk.action.SDKActionModule; +import org.opensearch.sdk.action.TestSDKActionModule; +import org.opensearch.sdk.handlers.AcknowledgedResponseHandler; +import org.opensearch.test.OpenSearchTestCase; +import org.opensearch.transport.Transport; +import org.opensearch.transport.TransportService; + +import java.net.InetAddress; +import java.util.Collections; + +import static java.util.Collections.emptyMap; +import static java.util.Collections.emptySet; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; + +public class TestSDKTransportService extends OpenSearchTestCase { + + private static final String TEST_UNIQUE_ID = "test-extension"; + private static final String TEST_ACTION_NAME = "testAction"; + + private TransportService transportService; + private DiscoveryNode opensearchNode; + private SDKActionModule sdkActionModule; + private SDKTransportService sdkTransportService; + + @Override + @BeforeEach + public void setUp() throws Exception { + super.setUp(); + this.transportService = spy( + new TransportService( + Settings.EMPTY, + mock(Transport.class), + null, + TransportService.NOOP_TRANSPORT_INTERCEPTOR, + x -> null, + null, + Collections.emptySet() + ) + ); + this.opensearchNode = new DiscoveryNode( + "test_node", + new TransportAddress(InetAddress.getByName("localhost"), 9876), + emptyMap(), + emptySet(), + Version.CURRENT + ); + sdkActionModule = new SDKActionModule(new TestSDKActionModule.TestActionExtension()); + + sdkTransportService = new SDKTransportService(); + sdkTransportService.setTransportService(transportService); + sdkTransportService.setOpensearchNode(opensearchNode); + sdkTransportService.setUniqueId(TEST_UNIQUE_ID); + } + + @Test + public void testRegisterTransportAction() { + ArgumentCaptor registerTransportActionsRequestCaptor = ArgumentCaptor.forClass( + RegisterTransportActionsRequest.class + ); + + sdkTransportService.sendRegisterTransportActionsRequest(sdkActionModule.getActions()); + verify(transportService, times(1)).sendRequest( + any(), + eq(ExtensionsManager.REQUEST_EXTENSION_REGISTER_TRANSPORT_ACTIONS), + registerTransportActionsRequestCaptor.capture(), + any(AcknowledgedResponseHandler.class) + ); + assertEquals(TEST_UNIQUE_ID, registerTransportActionsRequestCaptor.getValue().getUniqueId()); + assertTrue(registerTransportActionsRequestCaptor.getValue().getTransportActions().contains(ProxyAction.class.getName())); + } +} diff --git a/src/test/java/org/opensearch/sdk/action/TestProxyActionRequest.java b/src/test/java/org/opensearch/sdk/action/TestProxyActionRequest.java index b12e3f84..a2772fa0 100644 --- a/src/test/java/org/opensearch/sdk/action/TestProxyActionRequest.java +++ b/src/test/java/org/opensearch/sdk/action/TestProxyActionRequest.java @@ -14,9 +14,12 @@ import org.opensearch.action.ActionRequest; import org.opensearch.action.ActionRequestValidationException; +import org.opensearch.action.ActionResponse; +import org.opensearch.action.ActionType; import org.opensearch.common.bytes.BytesReference; import org.opensearch.common.io.stream.BytesStreamInput; import org.opensearch.common.io.stream.BytesStreamOutput; +import org.opensearch.common.io.stream.StreamInput; import org.opensearch.common.io.stream.StreamOutput; import org.opensearch.test.OpenSearchTestCase; @@ -42,32 +45,52 @@ public void testProxyActionRequest() throws Exception { } public void testProxyActionRequestWithClass() throws Exception { - class TestRequest extends ActionRequest { - private String data; + ProxyActionRequest request = new ProxyActionRequest(TestAction.INSTANCE, new TestRequest("test-action")); - public TestRequest(String data) { - this.data = data; - } + String expectedAction = request.getClass().getName(); + byte[] expectedRequestBytes = "test-action".getBytes(StandardCharsets.UTF_8); - @Override - public void writeTo(StreamOutput out) throws IOException { - super.writeTo(out); - out.writeString(data); - } + assertEquals(expectedAction, request.getAction()); + assertEquals(expectedRequestBytes, request.getRequestBytes()); + } - @Override - public ActionRequestValidationException validate() { - return null; - } + static class TestRequest extends ActionRequest { + + private String data; + + public TestRequest(String data) { + this.data = data; } - ProxyActionRequest request = new ProxyActionRequest(new TestRequest("test-action")); + @Override + public void writeTo(StreamOutput out) throws IOException { + super.writeTo(out); + out.writeString(data); + } - String expectedAction = request.getClass().getName(); - byte[] expectedRequestBytes = "test-action".getBytes(StandardCharsets.UTF_8); + @Override + public ActionRequestValidationException validate() { + return null; + } + } + + static class TestResponse extends ActionResponse { + public TestResponse(StreamInput in) {} + + @Override + public void writeTo(StreamOutput out) throws IOException {} + } + + static class TestAction extends ActionType { + + public static final String NAME = "helloworld/sample"; + public static final TestAction INSTANCE = new TestAction(); + + private TestAction() { + super(NAME, TestResponse::new); + } - assertEquals(expectedAction, request.getAction()); - assertEquals(expectedRequestBytes, request.getRequestBytes()); } + } diff --git a/src/test/java/org/opensearch/sdk/action/TestSDKActionModule.java b/src/test/java/org/opensearch/sdk/action/TestSDKActionModule.java index a3f0458f..12177842 100644 --- a/src/test/java/org/opensearch/sdk/action/TestSDKActionModule.java +++ b/src/test/java/org/opensearch/sdk/action/TestSDKActionModule.java @@ -11,51 +11,27 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.mockito.ArgumentCaptor; -import org.opensearch.Version; import org.opensearch.action.ActionRequest; import org.opensearch.action.ActionResponse; import org.opensearch.action.ActionType; -import org.opensearch.cluster.node.DiscoveryNode; -import org.opensearch.common.settings.Settings; -import org.opensearch.common.transport.TransportAddress; -import org.opensearch.extensions.ExtensionsManager; -import org.opensearch.extensions.action.RegisterTransportActionsRequest; import org.opensearch.sdk.ActionExtension; import org.opensearch.sdk.BaseExtension; import org.opensearch.sdk.ExtensionSettings; -import org.opensearch.sdk.ExtensionsRunner; -import org.opensearch.sdk.handlers.AcknowledgedResponseHandler; import org.opensearch.test.OpenSearchTestCase; -import org.opensearch.transport.Transport; -import org.opensearch.transport.TransportService; -import java.net.InetAddress; import java.util.Arrays; -import java.util.Collections; import java.util.List; -import static java.util.Collections.emptyMap; -import static java.util.Collections.emptySet; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.spy; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; public class TestSDKActionModule extends OpenSearchTestCase { - private static final String TEST_UNIQUE_ID = "test-extension"; - private static final String TEST_ACTION_NAME = "testAction"; + public static final String TEST_ACTION_NAME = "testAction"; - private ExtensionsRunner extensionsRunner; - private TransportService transportService; - private DiscoveryNode opensearchNode; private SDKActionModule sdkActionModule; - private static class TestActionExtension extends BaseExtension implements ActionExtension { + public static class TestActionExtension extends BaseExtension implements ActionExtension { public TestActionExtension() { super(mock(ExtensionSettings.class)); } @@ -74,45 +50,13 @@ public TestActionExtension() { @BeforeEach public void setUp() throws Exception { super.setUp(); - this.transportService = spy( - new TransportService( - Settings.EMPTY, - mock(Transport.class), - null, - TransportService.NOOP_TRANSPORT_INTERCEPTOR, - x -> null, - null, - Collections.emptySet() - ) - ); - this.opensearchNode = new DiscoveryNode( - "test_node", - new TransportAddress(InetAddress.getByName("localhost"), 9876), - emptyMap(), - emptySet(), - Version.CURRENT - ); - extensionsRunner = mock(ExtensionsRunner.class); - when(extensionsRunner.getExtensionTransportService()).thenReturn(transportService); - when(extensionsRunner.getOpensearchNode()).thenReturn(opensearchNode); - when(extensionsRunner.getUniqueId()).thenReturn(TEST_UNIQUE_ID); - sdkActionModule = new SDKActionModule(extensionsRunner, new TestActionExtension()); + sdkActionModule = new SDKActionModule(new TestActionExtension()); } @Test - public void testRegisterTransportAction() { - ArgumentCaptor registerTransportActionsRequestCaptor = ArgumentCaptor.forClass( - RegisterTransportActionsRequest.class - ); - sdkActionModule.sendRegisterTransportActionsRequest(); - verify(transportService, times(1)).sendRequest( - any(), - eq(ExtensionsManager.REQUEST_EXTENSION_REGISTER_TRANSPORT_ACTIONS), - registerTransportActionsRequestCaptor.capture(), - any(AcknowledgedResponseHandler.class) - ); - assertEquals(TEST_UNIQUE_ID, registerTransportActionsRequestCaptor.getValue().getUniqueId()); - assertTrue(registerTransportActionsRequestCaptor.getValue().getTransportActions().contains(ProxyAction.NAME)); - assertTrue(registerTransportActionsRequestCaptor.getValue().getTransportActions().contains(TEST_ACTION_NAME)); + public void testGetActions() { + assertEquals(2, sdkActionModule.getActions().size()); + assertTrue(sdkActionModule.getActions().containsKey(ProxyAction.NAME)); + assertTrue(sdkActionModule.getActions().containsKey(TEST_ACTION_NAME)); } } diff --git a/src/test/java/org/opensearch/sdk/sample/helloworld/TestHelloWorldExtension.java b/src/test/java/org/opensearch/sdk/sample/helloworld/TestHelloWorldExtension.java index f39677fe..e157e90c 100644 --- a/src/test/java/org/opensearch/sdk/sample/helloworld/TestHelloWorldExtension.java +++ b/src/test/java/org/opensearch/sdk/sample/helloworld/TestHelloWorldExtension.java @@ -43,12 +43,11 @@ import com.google.inject.Injector; import com.google.inject.Key; -import static org.mockito.Mockito.mock; import static org.opensearch.sdk.sample.helloworld.ExampleCustomSettingConfig.VALIDATED_SETTING; +@SuppressWarnings("deprecation") public class TestHelloWorldExtension extends OpenSearchTestCase { - private ExtensionsRunner extensionsRunner; private HelloWorldExtension extension; private Injector injector; private SDKClient sdkClient; @@ -68,7 +67,6 @@ private UnregisteredAction() { @BeforeEach public void setUp() throws Exception { super.setUp(); - this.extensionsRunner = mock(ExtensionsRunner.class); this.extension = new HelloWorldExtension(); // Do portions of Guice injection needed for this test @@ -76,7 +74,7 @@ public void setUp() throws Exception { ThreadPool threadPool = new ThreadPool(settings); TaskManager taskManager = new TaskManager(settings, threadPool, Collections.emptySet()); this.sdkClient = new SDKClient(extensionSettings); - this.injector = Guice.createInjector(new SDKActionModule(extensionsRunner, extension), b -> { + this.injector = Guice.createInjector(new SDKActionModule(extension), b -> { b.bind(ThreadPool.class).toInstance(threadPool); b.bind(TaskManager.class).toInstance(taskManager); b.bind(SDKClient.class).toInstance(sdkClient); @@ -146,7 +144,6 @@ public void onFailure(Exception e) { assertEquals(expectedGreeting, response.getGreeting()); } - @SuppressWarnings("deprecation") @Test public void testRestClientExecuteSampleActions() throws Exception { String expectedName = "world"; @@ -194,7 +191,6 @@ public void onFailure(Exception e) { assertEquals("The request name is blank.", cause.getMessage()); } - @SuppressWarnings("deprecation") @Test public void testExceptionalRestClientExecuteSampleActions() throws Exception { String expectedName = ""; From 33ef61d5861998e853a31b80e10a1d3fc2ff5bb3 Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Sun, 19 Mar 2023 20:14:21 -0700 Subject: [PATCH 09/20] Add ExtensionsActionRequestHandler Signed-off-by: Daniel Widdis --- .../org/opensearch/sdk/ExtensionsRunner.java | 13 +++ .../sdk/action/ProxyActionRequest.java | 1 - .../ExtensionActionResponseHandler.java | 4 +- .../ExtensionsActionRequestHandler.java | 110 ++++++++++++++++++ .../opensearch/sdk/TestExtensionsRunner.java | 2 +- 5 files changed, 126 insertions(+), 4 deletions(-) create mode 100644 src/main/java/org/opensearch/sdk/handlers/ExtensionsActionRequestHandler.java diff --git a/src/main/java/org/opensearch/sdk/ExtensionsRunner.java b/src/main/java/org/opensearch/sdk/ExtensionsRunner.java index 33694f07..25d6fdd8 100644 --- a/src/main/java/org/opensearch/sdk/ExtensionsRunner.java +++ b/src/main/java/org/opensearch/sdk/ExtensionsRunner.java @@ -25,6 +25,7 @@ import org.opensearch.extensions.DiscoveryExtensionNode; import org.opensearch.extensions.AddSettingsUpdateConsumerRequest; import org.opensearch.extensions.UpdateSettingsRequest; +import org.opensearch.extensions.action.ExtensionActionRequest; import org.opensearch.extensions.ExtensionsManager.RequestType; import org.opensearch.extensions.ExtensionRequest; import org.opensearch.extensions.ExtensionsManager; @@ -33,6 +34,7 @@ import org.opensearch.sdk.handlers.ClusterSettingsResponseHandler; import org.opensearch.sdk.handlers.ClusterStateResponseHandler; import org.opensearch.sdk.handlers.EnvironmentSettingsResponseHandler; +import org.opensearch.sdk.handlers.ExtensionsActionRequestHandler; import org.opensearch.sdk.action.SDKActionModule; import org.opensearch.sdk.handlers.AcknowledgedResponseHandler; import org.opensearch.sdk.handlers.ExtensionDependencyResponseHandler; @@ -140,6 +142,7 @@ public class ExtensionsRunner { private ExtensionsIndicesModuleNameRequestHandler extensionsIndicesModuleNameRequestHandler = new ExtensionsIndicesModuleNameRequestHandler(); private ExtensionsRestRequestHandler extensionsRestRequestHandler = new ExtensionsRestRequestHandler(extensionRestPathRegistry); + private ExtensionsActionRequestHandler extensionsActionRequestHandler; /** * Instantiates a new update settings request handler @@ -209,6 +212,8 @@ protected ExtensionsRunner(Extension extension) throws IOException { // initialize SDKClient action map initializeSdkClient(); + extensionsActionRequestHandler = new ExtensionsActionRequestHandler(getSdkClient(), getSdkActionModule()); + if (extension instanceof ActionExtension) { // store REST handlers in the registry for (ExtensionRestHandler extensionRestHandler : ((ActionExtension) extension).getExtensionRestHandlers()) { @@ -398,6 +403,14 @@ public void startTransportService(TransportService transportService) { ((request, channel, task) -> channel.sendResponse(updateSettingsRequestHandler.handleUpdateSettingsRequest(request))) ); + transportService.registerRequestHandler( + ExtensionsManager.REQUEST_EXTENSION_HANDLE_TRANSPORT_ACTION, + ThreadPool.Names.GENERIC, + false, + false, + ExtensionActionRequest::new, + ((request, channel, task) -> channel.sendResponse(extensionsActionRequestHandler.handleExtensionActionRequest(request))) + ); } /** diff --git a/src/main/java/org/opensearch/sdk/action/ProxyActionRequest.java b/src/main/java/org/opensearch/sdk/action/ProxyActionRequest.java index c5e17c30..a6d26252 100644 --- a/src/main/java/org/opensearch/sdk/action/ProxyActionRequest.java +++ b/src/main/java/org/opensearch/sdk/action/ProxyActionRequest.java @@ -57,7 +57,6 @@ public ProxyActionRequest(ActionType instance, ActionR byte[] bytes = new byte[0]; try (BytesStreamOutput out = new BytesStreamOutput()) { request.writeTo(out); - out.flush(); bytes = BytesReference.toBytes(out.bytes()); } catch (IOException e) { // This Should Never Happen (TM) diff --git a/src/main/java/org/opensearch/sdk/handlers/ExtensionActionResponseHandler.java b/src/main/java/org/opensearch/sdk/handlers/ExtensionActionResponseHandler.java index 518b547e..5439602b 100644 --- a/src/main/java/org/opensearch/sdk/handlers/ExtensionActionResponseHandler.java +++ b/src/main/java/org/opensearch/sdk/handlers/ExtensionActionResponseHandler.java @@ -13,9 +13,9 @@ import org.apache.logging.log4j.Logger; import org.opensearch.extensions.ExtensionsManager; import org.opensearch.extensions.action.ExtensionActionResponse; +import org.opensearch.sdk.SDKTransportService; import org.opensearch.common.Nullable; import org.opensearch.common.io.stream.StreamInput; -import org.opensearch.sdk.action.SDKActionModule; import org.opensearch.threadpool.ThreadPool; import org.opensearch.transport.TransportException; import org.opensearch.transport.TransportResponseHandler; @@ -25,7 +25,7 @@ import java.util.concurrent.TimeUnit; /** - * This class handles the response from OpenSearch to a {@link SDKActionModule#sendProxyActionRequest()} call. + * This class handles the response from OpenSearch to a {@link SDKTransportService#sendProxyActionRequest()} call. */ public class ExtensionActionResponseHandler implements TransportResponseHandler { diff --git a/src/main/java/org/opensearch/sdk/handlers/ExtensionsActionRequestHandler.java b/src/main/java/org/opensearch/sdk/handlers/ExtensionsActionRequestHandler.java new file mode 100644 index 00000000..61f07921 --- /dev/null +++ b/src/main/java/org/opensearch/sdk/handlers/ExtensionsActionRequestHandler.java @@ -0,0 +1,110 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.sdk.handlers; + +import java.io.IOException; +import java.util.Optional; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.TimeUnit; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.opensearch.action.ActionListener; +import org.opensearch.action.ActionResponse; +import org.opensearch.action.ActionType; +import org.opensearch.common.bytes.BytesReference; +import org.opensearch.common.io.stream.BytesStreamOutput; +import org.opensearch.extensions.ExtensionsManager; +import org.opensearch.extensions.action.ExtensionActionRequest; +import org.opensearch.extensions.action.ExtensionActionResponse; +import org.opensearch.sdk.SDKClient; +import org.opensearch.sdk.SDKTransportService; +import org.opensearch.sdk.action.SDKActionModule; + +/** + * This class handles a request from OpenSearch from another extension's {@link SDKTransportService#sendProxyActionRequest()} call. + */ +public class ExtensionsActionRequestHandler { + private static final Logger logger = LogManager.getLogger(ExtensionsActionRequestHandler.class); + + final SDKActionModule sdkActionModule; + final SDKClient client; + + /** + * Instantiate this handler + * + * @param sdkClient An initialized SDKClient with the registered actions + * @param sdkActionModule An initialized SDKActionModule with the registered actions + */ + public ExtensionsActionRequestHandler(SDKClient sdkClient, SDKActionModule sdkActionModule) { + this.sdkActionModule = sdkActionModule; + this.client = sdkClient; + } + + /** + * Handles a request from OpenSearch to execute a TransportAction on the extension. + * + * @param request The request to execute + * @return The response from the TransportAction + */ + public ExtensionActionResponse handleExtensionActionRequest(ExtensionActionRequest request) { + logger.info("Received request to execute action [" + request.getAction() + "]"); + final ExtensionActionResponse response = new ExtensionActionResponse(false, new byte[0]); + + // Find matching ActionType instance + Optional> optionalAction = sdkActionModule.getActions() + .values() + .stream() + .map(h -> h.getAction()) + .filter(a -> a.getClass().getName().equals(request.getAction())) + .findAny(); + if (optionalAction.isEmpty()) { + response.setResponseBytesAsString("No action [" + request.getAction() + "] is registered."); + return response; + } + + // Execute the action + logger.info("Executing action [" + request.getAction() + "]"); + ActionType action = optionalAction.get(); + // TODO: We need async client.execute to hide these action listener details and return the future directly + // https://github.com/opensearch-project/opensearch-sdk-java/issues/584 + CompletableFuture futureResponse = new CompletableFuture<>(); + client.execute(action, request, ActionListener.wrap(r -> { + byte[] bytes = new byte[0]; + try (BytesStreamOutput out = new BytesStreamOutput()) { + ((ActionResponse) r).writeTo(out); + bytes = BytesReference.toBytes(out.bytes()); + } catch (IOException e) { + // This Should Never Happen (TM) + // Won't get an IOException locally + } + response.setSuccess(true); + response.setResponseBytes(bytes); + futureResponse.complete(response); + }, e -> futureResponse.completeExceptionally(e))); + + logger.info("Waiting for response to action [" + request.getAction() + "]"); + try { + ExtensionActionResponse actionResponse = futureResponse.orTimeout( + ExtensionsManager.EXTENSION_REQUEST_WAIT_TIMEOUT, + TimeUnit.SECONDS + ).get(); + response.setSuccess(true); + response.setResponseBytes(actionResponse.getResponseBytes()); + logger.info("Response successful to [" + request.getAction() + "]"); + } catch (Exception e) { + response.setResponseBytesAsString("Action failed: " + e.getMessage()); + logger.info("Response failed to [" + request.getAction() + "]"); + } + logger.info("Sending action response to OpenSearch: " + response.getResponseBytesAsString()); + return response; + } + +} diff --git a/src/test/java/org/opensearch/sdk/TestExtensionsRunner.java b/src/test/java/org/opensearch/sdk/TestExtensionsRunner.java index 7088798a..69d70c55 100644 --- a/src/test/java/org/opensearch/sdk/TestExtensionsRunner.java +++ b/src/test/java/org/opensearch/sdk/TestExtensionsRunner.java @@ -100,7 +100,7 @@ public void testStartTransportService() { verify(transportService, times(1)).start(); // cannot verify acceptIncomingRequests as it is a final method // test registerRequestHandlers - verify(transportService, times(5)).registerRequestHandler(anyString(), anyString(), anyBoolean(), anyBoolean(), any(), any()); + verify(transportService, times(6)).registerRequestHandler(anyString(), anyString(), anyBoolean(), anyBoolean(), any(), any()); } @Test From 39c2d7d190c36acd324808adf7eaa6d493876d30 Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Sun, 19 Mar 2023 23:00:49 -0700 Subject: [PATCH 10/20] Instantiate Proxy Action Request Signed-off-by: Daniel Widdis --- .../opensearch/sdk/SDKTransportService.java | 11 ++++- .../sdk/action/ProxyActionRequest.java | 32 +++++++------ .../ExtensionsActionRequestHandler.java | 46 +++++++++++++++---- .../sdk/action/TestProxyActionRequest.java | 24 ++++------ 4 files changed, 74 insertions(+), 39 deletions(-) diff --git a/src/main/java/org/opensearch/sdk/SDKTransportService.java b/src/main/java/org/opensearch/sdk/SDKTransportService.java index a6ff572b..eee2bf8c 100644 --- a/src/main/java/org/opensearch/sdk/SDKTransportService.java +++ b/src/main/java/org/opensearch/sdk/SDKTransportService.java @@ -9,6 +9,8 @@ package org.opensearch.sdk; +import java.nio.ByteBuffer; +import java.nio.charset.StandardCharsets; import java.util.Map; import java.util.Set; import java.util.concurrent.TimeoutException; @@ -72,12 +74,19 @@ public void sendRegisterTransportActionsRequest(Map> @Nullable public byte[] sendProxyActionRequest(ProxyActionRequest request) { logger.info("Sending ProxyAction request to OpenSearch for [" + request.getAction() + "]"); + // Combine class name string and request bytes + byte[] requestClassBytes = request.getRequestClass().getBytes(StandardCharsets.UTF_8); + byte[] proxyRequestBytes = ByteBuffer.allocate(requestClassBytes.length + 1 + request.getRequestBytes().length) + .put(requestClassBytes) + .put((byte) 0) + .put(request.getRequestBytes()) + .array(); ExtensionActionResponseHandler extensionActionResponseHandler = new ExtensionActionResponseHandler(); try { transportService.sendRequest( opensearchNode, ExtensionsManager.TRANSPORT_ACTION_REQUEST_FROM_EXTENSION, - new TransportActionRequestFromExtension(request.getAction(), request.getRequestBytes(), uniqueId), + new TransportActionRequestFromExtension(request.getAction(), proxyRequestBytes, uniqueId), extensionActionResponseHandler ); // Wait on response diff --git a/src/main/java/org/opensearch/sdk/action/ProxyActionRequest.java b/src/main/java/org/opensearch/sdk/action/ProxyActionRequest.java index a6d26252..da04e1b5 100644 --- a/src/main/java/org/opensearch/sdk/action/ProxyActionRequest.java +++ b/src/main/java/org/opensearch/sdk/action/ProxyActionRequest.java @@ -27,24 +27,17 @@ */ public class ProxyActionRequest extends ActionRequest { /** - * action is the transport action intended to be invoked which is registered by an extension via {@link ExtensionTransportActionsHandler}. + * action is the TransportAction intended to be invoked which is registered by an extension via {@link ExtensionTransportActionsHandler}. */ private final String action; /** - * requestBytes is the raw bytes being transported between extensions. + * requestClass is the ActionRequest class associated with the TransportAction */ - private final byte[] requestBytes; - + private final String requestClass; /** - * ProxyActionRequest constructor. - * - * @param action is the ActionType class name intended to be invoked which is registered by an extension via {@link ExtensionTransportActionsHandler}. - * @param requestBytes is the raw bytes being transported between extensions. + * requestBytes is the raw bytes being transported between extensions. */ - public ProxyActionRequest(String action, byte[] requestBytes) { - this.action = action; - this.requestBytes = requestBytes; - } + private final byte[] requestBytes; /** * ProxyAcctionRequest constructor with an ActionType and Request class @@ -54,6 +47,7 @@ public ProxyActionRequest(String action, byte[] requestBytes) { */ public ProxyActionRequest(ActionType instance, ActionRequest request) { this.action = instance.getClass().getName(); + this.requestClass = request.getClass().getName(); byte[] bytes = new byte[0]; try (BytesStreamOutput out = new BytesStreamOutput()) { request.writeTo(out); @@ -74,6 +68,7 @@ public ProxyActionRequest(ActionType instance, ActionR public ProxyActionRequest(StreamInput in) throws IOException { super(in); this.action = in.readString(); + this.requestClass = in.readString(); this.requestBytes = in.readByteArray(); } @@ -81,6 +76,7 @@ public ProxyActionRequest(StreamInput in) throws IOException { public void writeTo(StreamOutput out) throws IOException { super.writeTo(out); out.writeString(action); + out.writeString(requestClass); out.writeByteArray(requestBytes); } @@ -88,6 +84,10 @@ public String getAction() { return this.action; } + public String getRequestClass() { + return this.requestClass; + } + public byte[] getRequestBytes() { return this.requestBytes; } @@ -99,7 +99,7 @@ public ActionRequestValidationException validate() { @Override public String toString() { - return "ProxyActionRequest{action=" + action + ", requestBytes=" + requestBytes + "}"; + return "ProxyActionRequest{action=" + action + ", requestClass=" + requestClass + ", requestBytes=" + requestBytes + "}"; } @Override @@ -107,11 +107,13 @@ public boolean equals(Object obj) { if (this == obj) return true; if (obj == null || getClass() != obj.getClass()) return false; ProxyActionRequest that = (ProxyActionRequest) obj; - return Objects.equals(action, that.action) && Objects.equals(requestBytes, that.requestBytes); + return Objects.equals(action, that.action) + && Objects.equals(requestClass, that.requestClass) + && Objects.equals(requestBytes, that.requestBytes); } @Override public int hashCode() { - return Objects.hash(action, requestBytes); + return Objects.hash(action, requestClass, requestBytes); } } diff --git a/src/main/java/org/opensearch/sdk/handlers/ExtensionsActionRequestHandler.java b/src/main/java/org/opensearch/sdk/handlers/ExtensionsActionRequestHandler.java index 61f07921..d58ac4c0 100644 --- a/src/main/java/org/opensearch/sdk/handlers/ExtensionsActionRequestHandler.java +++ b/src/main/java/org/opensearch/sdk/handlers/ExtensionsActionRequestHandler.java @@ -10,6 +10,9 @@ package org.opensearch.sdk.handlers; import java.io.IOException; +import java.lang.reflect.Constructor; +import java.nio.charset.StandardCharsets; +import java.util.Arrays; import java.util.Optional; import java.util.concurrent.CompletableFuture; import java.util.concurrent.TimeUnit; @@ -17,10 +20,12 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.opensearch.action.ActionListener; +import org.opensearch.action.ActionRequest; import org.opensearch.action.ActionResponse; import org.opensearch.action.ActionType; import org.opensearch.common.bytes.BytesReference; import org.opensearch.common.io.stream.BytesStreamOutput; +import org.opensearch.common.io.stream.StreamInput; import org.opensearch.extensions.ExtensionsManager; import org.opensearch.extensions.action.ExtensionActionRequest; import org.opensearch.extensions.action.ExtensionActionResponse; @@ -55,7 +60,7 @@ public ExtensionsActionRequestHandler(SDKClient sdkClient, SDKActionModule sdkAc * @return The response from the TransportAction */ public ExtensionActionResponse handleExtensionActionRequest(ExtensionActionRequest request) { - logger.info("Received request to execute action [" + request.getAction() + "]"); + logger.debug("Received request to execute action [" + request.getAction() + "]"); final ExtensionActionResponse response = new ExtensionActionResponse(false, new byte[0]); // Find matching ActionType instance @@ -70,13 +75,30 @@ public ExtensionActionResponse handleExtensionActionRequest(ExtensionActionReque return response; } - // Execute the action - logger.info("Executing action [" + request.getAction() + "]"); ActionType action = optionalAction.get(); + logger.debug("Found matching action [" + action.name() + "], an instance of [" + action.getClass().getName() + "]"); + + // Extract request class name from bytes and instantiate request + int nullPos = indexOf(request.getRequestBytes(), (byte) 0); + String requestClassName = new String(Arrays.copyOfRange(request.getRequestBytes(), 0, nullPos), StandardCharsets.UTF_8); + ActionRequest actionRequest = null; + try { + Class clazz = Class.forName(requestClassName); + Constructor constructor = clazz.getConstructor(StreamInput.class); + StreamInput requestByteStream = StreamInput.wrap( + Arrays.copyOfRange(request.getRequestBytes(), nullPos + 1, request.getRequestBytes().length - nullPos) + ); + actionRequest = (ActionRequest) constructor.newInstance(requestByteStream); + } catch (Exception e) { + response.setResponseBytesAsString("No request class [" + requestClassName + "] is available."); + return response; + } + + // Execute the action // TODO: We need async client.execute to hide these action listener details and return the future directly // https://github.com/opensearch-project/opensearch-sdk-java/issues/584 CompletableFuture futureResponse = new CompletableFuture<>(); - client.execute(action, request, ActionListener.wrap(r -> { + client.execute(action, actionRequest, ActionListener.wrap(r -> { byte[] bytes = new byte[0]; try (BytesStreamOutput out = new BytesStreamOutput()) { ((ActionResponse) r).writeTo(out); @@ -90,7 +112,7 @@ public ExtensionActionResponse handleExtensionActionRequest(ExtensionActionReque futureResponse.complete(response); }, e -> futureResponse.completeExceptionally(e))); - logger.info("Waiting for response to action [" + request.getAction() + "]"); + logger.debug("Waiting for response to action [" + request.getAction() + "]"); try { ExtensionActionResponse actionResponse = futureResponse.orTimeout( ExtensionsManager.EXTENSION_REQUEST_WAIT_TIMEOUT, @@ -98,13 +120,21 @@ public ExtensionActionResponse handleExtensionActionRequest(ExtensionActionReque ).get(); response.setSuccess(true); response.setResponseBytes(actionResponse.getResponseBytes()); - logger.info("Response successful to [" + request.getAction() + "]"); + logger.debug("Response successful to [" + request.getAction() + "]"); } catch (Exception e) { response.setResponseBytesAsString("Action failed: " + e.getMessage()); - logger.info("Response failed to [" + request.getAction() + "]"); + logger.debug("Response failed to [" + request.getAction() + "]"); } - logger.info("Sending action response to OpenSearch: " + response.getResponseBytesAsString()); + logger.debug("Sending action response to OpenSearch: " + response.getResponseBytesAsString()); return response; } + private static int indexOf(byte[] bytes, byte value) { + for (int offset = 0; offset < bytes.length; ++offset) { + if (bytes[offset] == value) { + return offset; + } + } + return -1; + } } diff --git a/src/test/java/org/opensearch/sdk/action/TestProxyActionRequest.java b/src/test/java/org/opensearch/sdk/action/TestProxyActionRequest.java index a2772fa0..b9887705 100644 --- a/src/test/java/org/opensearch/sdk/action/TestProxyActionRequest.java +++ b/src/test/java/org/opensearch/sdk/action/TestProxyActionRequest.java @@ -25,12 +25,16 @@ public class TestProxyActionRequest extends OpenSearchTestCase { public void testProxyActionRequest() throws Exception { - String expectedAction = "test-action"; - byte[] expectedRequestBytes = "request-bytes".getBytes(StandardCharsets.UTF_8); - ProxyActionRequest request = new ProxyActionRequest(expectedAction, expectedRequestBytes); + TestRequest testRequest = new TestRequest("test-action"); + ProxyActionRequest request = new ProxyActionRequest(TestAction.INSTANCE, testRequest); + + String expectedAction = request.getClass().getName(); + String expectedRequestClass = testRequest.getClass().getName(); + byte[] expectedRequestBytes = "test-action".getBytes(StandardCharsets.UTF_8); assertEquals(expectedAction, request.getAction()); - assertEquals(expectedRequestBytes, request.getRequestBytes()); + assertEquals(expectedRequestClass, request.getRequestClass()); + assertArrayEquals(expectedRequestBytes, request.getRequestBytes()); try (BytesStreamOutput out = new BytesStreamOutput()) { request.writeTo(out); @@ -39,22 +43,12 @@ public void testProxyActionRequest() throws Exception { request = new ProxyActionRequest(in); assertEquals(expectedAction, request.getAction()); + assertEquals(expectedRequestClass, request.getRequestClass()); assertArrayEquals(expectedRequestBytes, request.getRequestBytes()); } } } - public void testProxyActionRequestWithClass() throws Exception { - - ProxyActionRequest request = new ProxyActionRequest(TestAction.INSTANCE, new TestRequest("test-action")); - - String expectedAction = request.getClass().getName(); - byte[] expectedRequestBytes = "test-action".getBytes(StandardCharsets.UTF_8); - - assertEquals(expectedAction, request.getAction()); - assertEquals(expectedRequestBytes, request.getRequestBytes()); - } - static class TestRequest extends ActionRequest { private String data; From a881f5d5e5386652b7fc05091e48745b98bbab12 Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Sun, 19 Mar 2023 23:22:40 -0700 Subject: [PATCH 11/20] Working test case! Signed-off-by: Daniel Widdis --- .../org/opensearch/sdk/BaseExtension.java | 8 ++++---- .../org/opensearch/sdk/ExtensionsRunner.java | 6 +++--- ...ava => ExtensionActionRequestHandler.java} | 10 +++++----- .../rest/RestRemoteHelloAction.java | 20 ++++++++----------- 4 files changed, 20 insertions(+), 24 deletions(-) rename src/main/java/org/opensearch/sdk/handlers/{ExtensionsActionRequestHandler.java => ExtensionActionRequestHandler.java} (95%) diff --git a/src/main/java/org/opensearch/sdk/BaseExtension.java b/src/main/java/org/opensearch/sdk/BaseExtension.java index 085f1e04..fde742d5 100644 --- a/src/main/java/org/opensearch/sdk/BaseExtension.java +++ b/src/main/java/org/opensearch/sdk/BaseExtension.java @@ -59,10 +59,10 @@ public void setExtensionsRunner(ExtensionsRunner runner) { } /** - * Gets the {@link ExtensionsRunner} of this extension. - * - * @return the extension runner. - */ + * Gets the {@link ExtensionsRunner} of this extension. + * + * @return the extension runner. + */ public ExtensionsRunner extensionsRunner() { return this.extensionsRunner; } diff --git a/src/main/java/org/opensearch/sdk/ExtensionsRunner.java b/src/main/java/org/opensearch/sdk/ExtensionsRunner.java index 25d6fdd8..2401875e 100644 --- a/src/main/java/org/opensearch/sdk/ExtensionsRunner.java +++ b/src/main/java/org/opensearch/sdk/ExtensionsRunner.java @@ -34,7 +34,7 @@ import org.opensearch.sdk.handlers.ClusterSettingsResponseHandler; import org.opensearch.sdk.handlers.ClusterStateResponseHandler; import org.opensearch.sdk.handlers.EnvironmentSettingsResponseHandler; -import org.opensearch.sdk.handlers.ExtensionsActionRequestHandler; +import org.opensearch.sdk.handlers.ExtensionActionRequestHandler; import org.opensearch.sdk.action.SDKActionModule; import org.opensearch.sdk.handlers.AcknowledgedResponseHandler; import org.opensearch.sdk.handlers.ExtensionDependencyResponseHandler; @@ -142,7 +142,7 @@ public class ExtensionsRunner { private ExtensionsIndicesModuleNameRequestHandler extensionsIndicesModuleNameRequestHandler = new ExtensionsIndicesModuleNameRequestHandler(); private ExtensionsRestRequestHandler extensionsRestRequestHandler = new ExtensionsRestRequestHandler(extensionRestPathRegistry); - private ExtensionsActionRequestHandler extensionsActionRequestHandler; + private ExtensionActionRequestHandler extensionsActionRequestHandler; /** * Instantiates a new update settings request handler @@ -212,7 +212,7 @@ protected ExtensionsRunner(Extension extension) throws IOException { // initialize SDKClient action map initializeSdkClient(); - extensionsActionRequestHandler = new ExtensionsActionRequestHandler(getSdkClient(), getSdkActionModule()); + extensionsActionRequestHandler = new ExtensionActionRequestHandler(getSdkClient(), getSdkActionModule()); if (extension instanceof ActionExtension) { // store REST handlers in the registry diff --git a/src/main/java/org/opensearch/sdk/handlers/ExtensionsActionRequestHandler.java b/src/main/java/org/opensearch/sdk/handlers/ExtensionActionRequestHandler.java similarity index 95% rename from src/main/java/org/opensearch/sdk/handlers/ExtensionsActionRequestHandler.java rename to src/main/java/org/opensearch/sdk/handlers/ExtensionActionRequestHandler.java index d58ac4c0..0823be54 100644 --- a/src/main/java/org/opensearch/sdk/handlers/ExtensionsActionRequestHandler.java +++ b/src/main/java/org/opensearch/sdk/handlers/ExtensionActionRequestHandler.java @@ -36,8 +36,8 @@ /** * This class handles a request from OpenSearch from another extension's {@link SDKTransportService#sendProxyActionRequest()} call. */ -public class ExtensionsActionRequestHandler { - private static final Logger logger = LogManager.getLogger(ExtensionsActionRequestHandler.class); +public class ExtensionActionRequestHandler { + private static final Logger logger = LogManager.getLogger(ExtensionActionRequestHandler.class); final SDKActionModule sdkActionModule; final SDKClient client; @@ -48,7 +48,7 @@ public class ExtensionsActionRequestHandler { * @param sdkClient An initialized SDKClient with the registered actions * @param sdkActionModule An initialized SDKActionModule with the registered actions */ - public ExtensionsActionRequestHandler(SDKClient sdkClient, SDKActionModule sdkActionModule) { + public ExtensionActionRequestHandler(SDKClient sdkClient, SDKActionModule sdkActionModule) { this.sdkActionModule = sdkActionModule; this.client = sdkClient; } @@ -86,11 +86,11 @@ public ExtensionActionResponse handleExtensionActionRequest(ExtensionActionReque Class clazz = Class.forName(requestClassName); Constructor constructor = clazz.getConstructor(StreamInput.class); StreamInput requestByteStream = StreamInput.wrap( - Arrays.copyOfRange(request.getRequestBytes(), nullPos + 1, request.getRequestBytes().length - nullPos) + Arrays.copyOfRange(request.getRequestBytes(), nullPos + 1, request.getRequestBytes().length) ); actionRequest = (ActionRequest) constructor.newInstance(requestByteStream); } catch (Exception e) { - response.setResponseBytesAsString("No request class [" + requestClassName + "] is available."); + response.setResponseBytesAsString("No request class [" + requestClassName + "] is available: " + e.getMessage()); return response; } diff --git a/src/main/java/org/opensearch/sdk/sample/helloworld/rest/RestRemoteHelloAction.java b/src/main/java/org/opensearch/sdk/sample/helloworld/rest/RestRemoteHelloAction.java index 1cd8ca1d..e326a287 100644 --- a/src/main/java/org/opensearch/sdk/sample/helloworld/rest/RestRemoteHelloAction.java +++ b/src/main/java/org/opensearch/sdk/sample/helloworld/rest/RestRemoteHelloAction.java @@ -23,7 +23,6 @@ import org.opensearch.sdk.sample.helloworld.transport.SampleAction; import org.opensearch.sdk.sample.helloworld.transport.SampleRequest; -import java.nio.charset.StandardCharsets; import java.util.List; import java.util.concurrent.CompletableFuture; import java.util.concurrent.TimeUnit; @@ -61,7 +60,8 @@ public List routeHandlers() { // This class happens to be local for simplicity but this should be from a dependency SampleRequest sampleRequest = new SampleRequest(name); // Serialize this request in a proxy action request - // This requires that the remote extension has a corresponding action registered + // This requires that the remote extension has a corresponding transport action registered + // This Action class happens to be local for simplicity but this should be from a dependency ProxyActionRequest proxyActionRequest = new ProxyActionRequest(SampleAction.INSTANCE, sampleRequest); // TODO: We need async client.execute to hide these action listener details and return the future directly @@ -73,17 +73,13 @@ public List routeHandlers() { ActionListener.wrap(r -> futureResponse.complete(r), e -> futureResponse.completeExceptionally(e)) ); try { - byte[] responseBytes = futureResponse.orTimeout(ExtensionsManager.EXTENSION_REQUEST_WAIT_TIMEOUT, TimeUnit.SECONDS) - .get() - .getResponseBytes(); - if (responseBytes == null) { - return new ExtensionRestResponse(request, OK, "Received null remote extension reponse: "); + + ExtensionActionResponse response = futureResponse.orTimeout(ExtensionsManager.EXTENSION_REQUEST_WAIT_TIMEOUT, TimeUnit.SECONDS) + .get(); + if (!response.isSuccess()) { + return new ExtensionRestResponse(request, OK, "Remote extension reponse failed: " + response.getResponseBytesAsString()); } - return new ExtensionRestResponse( - request, - OK, - "Received remote extension reponse: " + new String(responseBytes, StandardCharsets.UTF_8) - ); + return new ExtensionRestResponse(request, OK, "Received remote extension reponse: " + response.getResponseBytesAsString()); } catch (Exception e) { return exceptionalRequest(request, e); } From e0149d0944b24dde7d5244677ac355f587a23801 Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Mon, 20 Mar 2023 08:58:28 -0700 Subject: [PATCH 12/20] Properly parse returned byte array into a response Signed-off-by: Daniel Widdis --- .../sdk/action/ProxyActionRequest.java | 15 ++++++++++++++- .../ExtensionActionRequestHandler.java | 2 +- .../ExtensionActionResponseHandler.java | 3 ++- .../helloworld/rest/RestRemoteHelloAction.java | 12 ++++++++---- .../sdk/action/TestProxyActionRequest.java | 18 +++++++++++++----- 5 files changed, 38 insertions(+), 12 deletions(-) diff --git a/src/main/java/org/opensearch/sdk/action/ProxyActionRequest.java b/src/main/java/org/opensearch/sdk/action/ProxyActionRequest.java index da04e1b5..d26a95d9 100644 --- a/src/main/java/org/opensearch/sdk/action/ProxyActionRequest.java +++ b/src/main/java/org/opensearch/sdk/action/ProxyActionRequest.java @@ -40,7 +40,7 @@ public class ProxyActionRequest extends ActionRequest { private final byte[] requestBytes; /** - * ProxyAcctionRequest constructor with an ActionType and Request class + * ProxyActionRequest constructor with an ActionType and Request class. Requires a dependency on the remote extension code. * * @param instance An instance of {@link ActionType} registered with the remote extension's getActions registry * @param request A class extending {@link ActionRequest} associated with an action to be executed on another extension. @@ -59,6 +59,19 @@ public ProxyActionRequest(ActionType instance, ActionR this.requestBytes = bytes; } + /** + * ProxyActionRequest constructor with class names and request bytes. Does not require a dependency on the remote extension code. + * + * @param action A string representing the fully qualified class name of the remote ActionType instance + * @param requestClass A string representing the fully qualified class name of the remote ActionRequest class + * @param requestBytes Bytes representing the serialized parameters to be used in the ActionRequest class StreamInput constructor + */ + public ProxyActionRequest(String action, String requestClass, byte[] requestBytes) { + this.action = action; + this.requestClass = requestClass; + this.requestBytes = requestBytes; + } + /** * ProxyActionRequest constructor from {@link StreamInput}. * diff --git a/src/main/java/org/opensearch/sdk/handlers/ExtensionActionRequestHandler.java b/src/main/java/org/opensearch/sdk/handlers/ExtensionActionRequestHandler.java index 0823be54..da268176 100644 --- a/src/main/java/org/opensearch/sdk/handlers/ExtensionActionRequestHandler.java +++ b/src/main/java/org/opensearch/sdk/handlers/ExtensionActionRequestHandler.java @@ -125,7 +125,7 @@ public ExtensionActionResponse handleExtensionActionRequest(ExtensionActionReque response.setResponseBytesAsString("Action failed: " + e.getMessage()); logger.debug("Response failed to [" + request.getAction() + "]"); } - logger.debug("Sending action response to OpenSearch: " + response.getResponseBytesAsString()); + logger.debug("Sending action response to OpenSearch: " + response.getResponseBytes().length + " bytes"); return response; } diff --git a/src/main/java/org/opensearch/sdk/handlers/ExtensionActionResponseHandler.java b/src/main/java/org/opensearch/sdk/handlers/ExtensionActionResponseHandler.java index 5439602b..c9b23813 100644 --- a/src/main/java/org/opensearch/sdk/handlers/ExtensionActionResponseHandler.java +++ b/src/main/java/org/opensearch/sdk/handlers/ExtensionActionResponseHandler.java @@ -42,8 +42,9 @@ public ExtensionActionResponseHandler() { @Override public void handleResponse(ExtensionActionResponse response) { - logger.info("received {}", response); + logger.info("Received response bytes: " + response.getResponseBytes().length + " bytes"); + logger.debug("Received response bytes: " + response.getResponseBytesAsString()); // Set ExtensionActionResponse from response this.responseBytes = response.getResponseBytes(); inProgressFuture.complete(response); diff --git a/src/main/java/org/opensearch/sdk/sample/helloworld/rest/RestRemoteHelloAction.java b/src/main/java/org/opensearch/sdk/sample/helloworld/rest/RestRemoteHelloAction.java index e326a287..1f3c004e 100644 --- a/src/main/java/org/opensearch/sdk/sample/helloworld/rest/RestRemoteHelloAction.java +++ b/src/main/java/org/opensearch/sdk/sample/helloworld/rest/RestRemoteHelloAction.java @@ -10,6 +10,7 @@ package org.opensearch.sdk.sample.helloworld.rest; import org.opensearch.action.ActionListener; +import org.opensearch.common.io.stream.StreamInput; import org.opensearch.extensions.ExtensionsManager; import org.opensearch.extensions.action.ExtensionActionResponse; import org.opensearch.extensions.rest.ExtensionRestRequest; @@ -22,6 +23,7 @@ import org.opensearch.sdk.action.ProxyActionRequest; import org.opensearch.sdk.sample.helloworld.transport.SampleAction; import org.opensearch.sdk.sample.helloworld.transport.SampleRequest; +import org.opensearch.sdk.sample.helloworld.transport.SampleResponse; import java.util.List; import java.util.concurrent.CompletableFuture; @@ -57,11 +59,12 @@ public List routeHandlers() { String name = request.param("name"); // Create a request using class on remote - // This class happens to be local for simplicity but this should be from a dependency + // This class happens to be local for simplicity but is a class on the remote extension SampleRequest sampleRequest = new SampleRequest(name); + // Serialize this request in a proxy action request // This requires that the remote extension has a corresponding transport action registered - // This Action class happens to be local for simplicity but this should be from a dependency + // This Action class happens to be local for simplicity but is a class on the remote extension ProxyActionRequest proxyActionRequest = new ProxyActionRequest(SampleAction.INSTANCE, sampleRequest); // TODO: We need async client.execute to hide these action listener details and return the future directly @@ -73,13 +76,14 @@ public List routeHandlers() { ActionListener.wrap(r -> futureResponse.complete(r), e -> futureResponse.completeExceptionally(e)) ); try { - ExtensionActionResponse response = futureResponse.orTimeout(ExtensionsManager.EXTENSION_REQUEST_WAIT_TIMEOUT, TimeUnit.SECONDS) .get(); if (!response.isSuccess()) { return new ExtensionRestResponse(request, OK, "Remote extension reponse failed: " + response.getResponseBytesAsString()); } - return new ExtensionRestResponse(request, OK, "Received remote extension reponse: " + response.getResponseBytesAsString()); + // Parse out the expected response class from the bytes + SampleResponse sampleResponse = new SampleResponse(StreamInput.wrap(response.getResponseBytes())); + return new ExtensionRestResponse(request, OK, "Received greeting from remote extension: " + sampleResponse.getGreeting()); } catch (Exception e) { return exceptionalRequest(request, e); } diff --git a/src/test/java/org/opensearch/sdk/action/TestProxyActionRequest.java b/src/test/java/org/opensearch/sdk/action/TestProxyActionRequest.java index b9887705..d8920a6c 100644 --- a/src/test/java/org/opensearch/sdk/action/TestProxyActionRequest.java +++ b/src/test/java/org/opensearch/sdk/action/TestProxyActionRequest.java @@ -10,8 +10,8 @@ package org.opensearch.sdk.action; import java.io.IOException; -import java.nio.charset.StandardCharsets; +import org.junit.jupiter.api.Test; import org.opensearch.action.ActionRequest; import org.opensearch.action.ActionRequestValidationException; import org.opensearch.action.ActionResponse; @@ -24,18 +24,26 @@ import org.opensearch.test.OpenSearchTestCase; public class TestProxyActionRequest extends OpenSearchTestCase { + + @Test public void testProxyActionRequest() throws Exception { TestRequest testRequest = new TestRequest("test-action"); - ProxyActionRequest request = new ProxyActionRequest(TestAction.INSTANCE, testRequest); - String expectedAction = request.getClass().getName(); + String expectedAction = TestAction.class.getName(); String expectedRequestClass = testRequest.getClass().getName(); - byte[] expectedRequestBytes = "test-action".getBytes(StandardCharsets.UTF_8); + byte[] expectedRequestBytes; + try (BytesStreamOutput out = new BytesStreamOutput()) { + testRequest.writeTo(out); + expectedRequestBytes = BytesReference.toBytes(out.bytes()); + } + ProxyActionRequest request = new ProxyActionRequest(TestAction.INSTANCE, testRequest); assertEquals(expectedAction, request.getAction()); assertEquals(expectedRequestClass, request.getRequestClass()); assertArrayEquals(expectedRequestBytes, request.getRequestBytes()); + request = new ProxyActionRequest(expectedAction, expectedRequestClass, expectedRequestBytes); + try (BytesStreamOutput out = new BytesStreamOutput()) { request.writeTo(out); out.flush(); @@ -78,7 +86,7 @@ public void writeTo(StreamOutput out) throws IOException {} static class TestAction extends ActionType { - public static final String NAME = "helloworld/sample"; + public static final String NAME = "test"; public static final TestAction INSTANCE = new TestAction(); private TestAction() { From f170fcee092f962920e8cb414d7cb921ce25cfdb Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Mon, 20 Mar 2023 13:01:46 -0700 Subject: [PATCH 13/20] Add sequence diagram to DESIGN.md Signed-off-by: Daniel Widdis --- DESIGN.md | 6 ++++++ Docs/RemoteActionExecution.svg | 1 + 2 files changed, 7 insertions(+) create mode 100644 Docs/RemoteActionExecution.svg diff --git a/DESIGN.md b/DESIGN.md index 130be287..8f9db10b 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -119,6 +119,12 @@ The `ExtensionsManager` reads a list of extensions present in `extensions.yml`. (27) The User receives the response. +#### Remote Action Execution on another Extension + +Extensions may invoke actions on other extensions using the `ProxyAction` and `ProxyActionRequest`. The code sequence is shown below. + +![](Docs/RemoteActionExecution.svg) + #### Extension Point Implementation Walk Through An example of a more complex extension point, `getNamedXContent()` is shown below. A similar pattern can be followed for most extension points. diff --git a/Docs/RemoteActionExecution.svg b/Docs/RemoteActionExecution.svg new file mode 100644 index 00000000..b7172e9d --- /dev/null +++ b/Docs/RemoteActionExecution.svg @@ -0,0 +1 @@ +title%20Remote%20Action%20Execution%0A%0Aparticipant%20%22Local%20Extension%22%20as%20a%0Aparticipant%20%22OpenSearch%22%20as%20os%0Aparticipant%20%22Remote%20Extension%22%20as%20b%0A%0Aos%3C-b%3AIterate%20getActions()%20and%20register%5Cnaction%20handlers%20in%20OpenSearch%20using%5CnExtensionTransportActionsHandler%5CnregisterAction()%20method%0Aos-%3Eos%3Aregister%20actions%20in%20ActionModule%5CnDynamicActionRegistry%20linking%5CnAction%20class%20name%20to%20extension%0A%0Aa-%3Ea%3ACreate%20ProxyActionRequest%20with%5CnRemote%20Extension%20ActionType%20class%2C%5CnActionRequest%20class%2C%20and%20bytes%20from%5CnActionRequest%20param%20serialization%0A%0Aa-%3Ea%3Aclient.execute()%20invokes%20doExecute()%5Cnon%20ProxyTransportAction%0A%0Aa-%3Eos%3AsendProxyActionRequest()%20method%20in%5CnSDKTransportService%20generates%5CnTransportActionRequestFromExtension%5Cnand%20sends%20to%20OpenSearch%5CnExtensionTransportActionsHandler%0A%0Aos-%3Eos%3AhandleTransportActionRequestFromExtension()%5Cnidentifies%20remote%20Extension%20node%20and%20calls%5Cnclient.execute()%20on%20Dynamic%20action%20from%5CnDynamicActionRegistry%0A%0Aos-%3Eb%3AsendTransportRequestToExtension()%5Cngenerates%20ExtensionActionRequest%5Cnand%20sends%20to%20the%20remote%20extension%0A%0Ab%3C-b%3AExtensionActionRequestHandler%5Cnreconstructs%20ActionType%20instance%5Cnand%20ActionRequest%20class%20and%5Cninvokes%20client.execute()%20on%20the%5Cndesired%20remote%20action%0A%0Aos%3C-b%3AExtensionActionResponse%20handled%5Cnby%20sendTransportRequestToExtension()%0A%0Aa%3C-os%3AExtensionActionResponse%20handled%5Cnby%20sendProxyActionRequest()%0A%0Aa-%3Ea%3AResponse%20bytes%20deserialized%20based%5Cnon%20remote%20extension's%20ActionResponse%5CnserializationRemote Action ExecutionLocal ExtensionOpenSearchRemote ExtensionIterate getActions() and registeraction handlers in OpenSearch usingExtensionTransportActionsHandlerregisterAction() methodregister actions in ActionModuleDynamicActionRegistry linkingAction class name to extensionCreate ProxyActionRequest withRemote Extension ActionType class,ActionRequest class, and bytes fromActionRequest param serializationclient.execute() invokes doExecute()on ProxyTransportActionsendProxyActionRequest() method inSDKTransportService generatesTransportActionRequestFromExtensionand sends to OpenSearchExtensionTransportActionsHandlerhandleTransportActionRequestFromExtension()identifies remote Extension node and callsclient.execute() on Dynamic action fromDynamicActionRegistrysendTransportRequestToExtension()generates ExtensionActionRequestand sends to the remote extensionExtensionActionRequestHandlerreconstructs ActionType instanceand ActionRequest class andinvokes client.execute() on thedesired remote actionExtensionActionResponse handledby sendTransportRequestToExtension()ExtensionActionResponse handledby sendProxyActionRequest()Response bytes deserialized basedon remote extension's ActionResponseserialization From 87803c64a6180759206cae53bb3b6892492dd0ef Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Tue, 21 Mar 2023 13:52:16 -0700 Subject: [PATCH 14/20] Typoo fix Signed-off-by: Daniel Widdis --- src/main/java/org/opensearch/sdk/SDKTransportService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/opensearch/sdk/SDKTransportService.java b/src/main/java/org/opensearch/sdk/SDKTransportService.java index eee2bf8c..0b8e6cc8 100644 --- a/src/main/java/org/opensearch/sdk/SDKTransportService.java +++ b/src/main/java/org/opensearch/sdk/SDKTransportService.java @@ -31,7 +31,7 @@ import org.opensearch.transport.TransportService; /** - * Wrapper class for {@link TranspoortService} and associated methods. + * Wrapper class for {@link TransportService} and associated methods. * * TODO: Move all the sendFooRequest() methods here * TODO: Replace usages of getExtensionTransportService with this class From 34b927e179bde3cf88f1db3f5965d064cf15df3c Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Wed, 22 Mar 2023 14:51:03 -0700 Subject: [PATCH 15/20] Update with latest changes on companion PR Signed-off-by: Daniel Widdis --- .../java/org/opensearch/sdk/ExtensionsRunner.java | 9 ++++++--- .../java/org/opensearch/sdk/action/ProxyAction.java | 5 +++-- .../opensearch/sdk/action/ProxyTransportAction.java | 7 ++++--- .../sdk/handlers/ExtensionActionRequestHandler.java | 9 +++++---- .../sdk/handlers/ExtensionActionResponseHandler.java | 11 ++++++----- .../sample/helloworld/rest/RestRemoteHelloAction.java | 5 +++-- 6 files changed, 27 insertions(+), 19 deletions(-) diff --git a/src/main/java/org/opensearch/sdk/ExtensionsRunner.java b/src/main/java/org/opensearch/sdk/ExtensionsRunner.java index 2401875e..ec6e130f 100644 --- a/src/main/java/org/opensearch/sdk/ExtensionsRunner.java +++ b/src/main/java/org/opensearch/sdk/ExtensionsRunner.java @@ -401,10 +401,13 @@ public void startTransportService(TransportService transportService) { false, UpdateSettingsRequest::new, ((request, channel, task) -> channel.sendResponse(updateSettingsRequestHandler.handleUpdateSettingsRequest(request))) - ); - + ); + + // TODO: This handles a remote extension request sending a RemoteExtensionActionResponse + // For actions sent from OpenSearch or a plugin using ProxyAction need to write a new request handler + // for ExtensionsManager.REQUEST_EXTENSION_HANDLE_TRANSPORT_ACTION that sends an ExtensionActionResponse transportService.registerRequestHandler( - ExtensionsManager.REQUEST_EXTENSION_HANDLE_TRANSPORT_ACTION, + ExtensionsManager.REQUEST_EXTENSION_HANDLE_REMOTE_TRANSPORT_ACTION, ThreadPool.Names.GENERIC, false, false, diff --git a/src/main/java/org/opensearch/sdk/action/ProxyAction.java b/src/main/java/org/opensearch/sdk/action/ProxyAction.java index 94bcf2ba..3cb52c2c 100644 --- a/src/main/java/org/opensearch/sdk/action/ProxyAction.java +++ b/src/main/java/org/opensearch/sdk/action/ProxyAction.java @@ -11,11 +11,12 @@ import org.opensearch.action.ActionType; import org.opensearch.extensions.action.ExtensionActionResponse; +import org.opensearch.extensions.action.RemoteExtensionActionResponse; /** * The {@link ActionType} used as they key for the {@link ProxyTransportAction}. */ -public class ProxyAction extends ActionType { +public class ProxyAction extends ActionType { /** * The name to look up this action with @@ -27,6 +28,6 @@ public class ProxyAction extends ActionType { public static final ProxyAction INSTANCE = new ProxyAction(); private ProxyAction() { - super(NAME, ExtensionActionResponse::new); + super(NAME, RemoteExtensionActionResponse::new); } } diff --git a/src/main/java/org/opensearch/sdk/action/ProxyTransportAction.java b/src/main/java/org/opensearch/sdk/action/ProxyTransportAction.java index 1fd7cd18..36f81e07 100644 --- a/src/main/java/org/opensearch/sdk/action/ProxyTransportAction.java +++ b/src/main/java/org/opensearch/sdk/action/ProxyTransportAction.java @@ -13,6 +13,7 @@ import org.opensearch.action.support.ActionFilters; import org.opensearch.action.support.TransportAction; import org.opensearch.extensions.action.ExtensionActionResponse; +import org.opensearch.extensions.action.RemoteExtensionActionResponse; import org.opensearch.sdk.SDKTransportService; import org.opensearch.tasks.Task; import org.opensearch.tasks.TaskManager; @@ -22,7 +23,7 @@ /** * Sends a request to OpenSearch for a remote extension to execute an action. */ -public class ProxyTransportAction extends TransportAction { +public class ProxyTransportAction extends TransportAction { private SDKTransportService sdkTransportService; @@ -46,12 +47,12 @@ protected ProxyTransportAction( } @Override - protected void doExecute(Task task, ProxyActionRequest request, ActionListener listener) { + protected void doExecute(Task task, ProxyActionRequest request, ActionListener listener) { byte[] responseBytes = sdkTransportService.sendProxyActionRequest(request); if (responseBytes == null) { listener.onFailure(new RuntimeException("No response received from remote extension.")); } else { - listener.onResponse(new ExtensionActionResponse(true, responseBytes)); + listener.onResponse(new RemoteExtensionActionResponse(true, responseBytes)); } } } diff --git a/src/main/java/org/opensearch/sdk/handlers/ExtensionActionRequestHandler.java b/src/main/java/org/opensearch/sdk/handlers/ExtensionActionRequestHandler.java index da268176..8c5b3515 100644 --- a/src/main/java/org/opensearch/sdk/handlers/ExtensionActionRequestHandler.java +++ b/src/main/java/org/opensearch/sdk/handlers/ExtensionActionRequestHandler.java @@ -29,6 +29,7 @@ import org.opensearch.extensions.ExtensionsManager; import org.opensearch.extensions.action.ExtensionActionRequest; import org.opensearch.extensions.action.ExtensionActionResponse; +import org.opensearch.extensions.action.RemoteExtensionActionResponse; import org.opensearch.sdk.SDKClient; import org.opensearch.sdk.SDKTransportService; import org.opensearch.sdk.action.SDKActionModule; @@ -59,9 +60,9 @@ public ExtensionActionRequestHandler(SDKClient sdkClient, SDKActionModule sdkAct * @param request The request to execute * @return The response from the TransportAction */ - public ExtensionActionResponse handleExtensionActionRequest(ExtensionActionRequest request) { + public RemoteExtensionActionResponse handleExtensionActionRequest(ExtensionActionRequest request) { logger.debug("Received request to execute action [" + request.getAction() + "]"); - final ExtensionActionResponse response = new ExtensionActionResponse(false, new byte[0]); + final RemoteExtensionActionResponse response = new RemoteExtensionActionResponse(false, new byte[0]); // Find matching ActionType instance Optional> optionalAction = sdkActionModule.getActions() @@ -97,7 +98,7 @@ public ExtensionActionResponse handleExtensionActionRequest(ExtensionActionReque // Execute the action // TODO: We need async client.execute to hide these action listener details and return the future directly // https://github.com/opensearch-project/opensearch-sdk-java/issues/584 - CompletableFuture futureResponse = new CompletableFuture<>(); + CompletableFuture futureResponse = new CompletableFuture<>(); client.execute(action, actionRequest, ActionListener.wrap(r -> { byte[] bytes = new byte[0]; try (BytesStreamOutput out = new BytesStreamOutput()) { @@ -114,7 +115,7 @@ public ExtensionActionResponse handleExtensionActionRequest(ExtensionActionReque logger.debug("Waiting for response to action [" + request.getAction() + "]"); try { - ExtensionActionResponse actionResponse = futureResponse.orTimeout( + RemoteExtensionActionResponse actionResponse = futureResponse.orTimeout( ExtensionsManager.EXTENSION_REQUEST_WAIT_TIMEOUT, TimeUnit.SECONDS ).get(); diff --git a/src/main/java/org/opensearch/sdk/handlers/ExtensionActionResponseHandler.java b/src/main/java/org/opensearch/sdk/handlers/ExtensionActionResponseHandler.java index c9b23813..4a953816 100644 --- a/src/main/java/org/opensearch/sdk/handlers/ExtensionActionResponseHandler.java +++ b/src/main/java/org/opensearch/sdk/handlers/ExtensionActionResponseHandler.java @@ -13,6 +13,7 @@ import org.apache.logging.log4j.Logger; import org.opensearch.extensions.ExtensionsManager; import org.opensearch.extensions.action.ExtensionActionResponse; +import org.opensearch.extensions.action.RemoteExtensionActionResponse; import org.opensearch.sdk.SDKTransportService; import org.opensearch.common.Nullable; import org.opensearch.common.io.stream.StreamInput; @@ -27,10 +28,10 @@ /** * This class handles the response from OpenSearch to a {@link SDKTransportService#sendProxyActionRequest()} call. */ -public class ExtensionActionResponseHandler implements TransportResponseHandler { +public class ExtensionActionResponseHandler implements TransportResponseHandler { private static final Logger logger = LogManager.getLogger(ExtensionActionResponseHandler.class); - private final CompletableFuture inProgressFuture; + private final CompletableFuture inProgressFuture; private byte[] responseBytes = null; /** @@ -41,7 +42,7 @@ public ExtensionActionResponseHandler() { } @Override - public void handleResponse(ExtensionActionResponse response) { + public void handleResponse(RemoteExtensionActionResponse response) { logger.info("Received response bytes: " + response.getResponseBytes().length + " bytes"); logger.debug("Received response bytes: " + response.getResponseBytesAsString()); @@ -62,8 +63,8 @@ public String executor() { } @Override - public ExtensionActionResponse read(StreamInput in) throws IOException { - return new ExtensionActionResponse(in); + public RemoteExtensionActionResponse read(StreamInput in) throws IOException { + return new RemoteExtensionActionResponse(in); } /** diff --git a/src/main/java/org/opensearch/sdk/sample/helloworld/rest/RestRemoteHelloAction.java b/src/main/java/org/opensearch/sdk/sample/helloworld/rest/RestRemoteHelloAction.java index 1f3c004e..18f948f8 100644 --- a/src/main/java/org/opensearch/sdk/sample/helloworld/rest/RestRemoteHelloAction.java +++ b/src/main/java/org/opensearch/sdk/sample/helloworld/rest/RestRemoteHelloAction.java @@ -13,6 +13,7 @@ import org.opensearch.common.io.stream.StreamInput; import org.opensearch.extensions.ExtensionsManager; import org.opensearch.extensions.action.ExtensionActionResponse; +import org.opensearch.extensions.action.RemoteExtensionActionResponse; import org.opensearch.extensions.rest.ExtensionRestRequest; import org.opensearch.extensions.rest.ExtensionRestResponse; import org.opensearch.sdk.BaseExtensionRestHandler; @@ -69,14 +70,14 @@ public List routeHandlers() { // TODO: We need async client.execute to hide these action listener details and return the future directly // https://github.com/opensearch-project/opensearch-sdk-java/issues/584 - CompletableFuture futureResponse = new CompletableFuture<>(); + CompletableFuture futureResponse = new CompletableFuture<>(); client.execute( ProxyAction.INSTANCE, proxyActionRequest, ActionListener.wrap(r -> futureResponse.complete(r), e -> futureResponse.completeExceptionally(e)) ); try { - ExtensionActionResponse response = futureResponse.orTimeout(ExtensionsManager.EXTENSION_REQUEST_WAIT_TIMEOUT, TimeUnit.SECONDS) + RemoteExtensionActionResponse response = futureResponse.orTimeout(ExtensionsManager.EXTENSION_REQUEST_WAIT_TIMEOUT, TimeUnit.SECONDS) .get(); if (!response.isSuccess()) { return new ExtensionRestResponse(request, OK, "Remote extension reponse failed: " + response.getResponseBytesAsString()); From 812f80db1857e90da6527fdee5d8b23c89ef54c6 Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Thu, 23 Mar 2023 07:49:57 -0700 Subject: [PATCH 16/20] Rename ProxyFoo to RemoteExtensionFoo Signed-off-by: Daniel Widdis --- .../java/org/opensearch/sdk/ExtensionsRunner.java | 4 ++-- .../org/opensearch/sdk/SDKTransportService.java | 10 +++++++--- ...roxyAction.java => RemoteExtensionAction.java} | 11 +++++------ ...est.java => RemoteExtensionActionRequest.java} | 10 +++++----- ...n.java => RemoteExtensionTransportAction.java} | 7 +++---- .../opensearch/sdk/action/SDKActionModule.java | 2 +- .../handlers/ExtensionActionRequestHandler.java | 1 - .../handlers/ExtensionActionResponseHandler.java | 1 - .../helloworld/rest/RestRemoteHelloAction.java | 15 ++++++++------- .../opensearch/sdk/TestSDKTransportService.java | 4 ++-- .../sdk/action/TestProxyActionRequest.java | 6 +++--- .../sdk/action/TestSDKActionModule.java | 2 +- 12 files changed, 37 insertions(+), 36 deletions(-) rename src/main/java/org/opensearch/sdk/action/{ProxyAction.java => RemoteExtensionAction.java} (60%) rename src/main/java/org/opensearch/sdk/action/{ProxyActionRequest.java => RemoteExtensionActionRequest.java} (91%) rename src/main/java/org/opensearch/sdk/action/{ProxyTransportAction.java => RemoteExtensionTransportAction.java} (83%) diff --git a/src/main/java/org/opensearch/sdk/ExtensionsRunner.java b/src/main/java/org/opensearch/sdk/ExtensionsRunner.java index ec6e130f..729e60e6 100644 --- a/src/main/java/org/opensearch/sdk/ExtensionsRunner.java +++ b/src/main/java/org/opensearch/sdk/ExtensionsRunner.java @@ -401,8 +401,8 @@ public void startTransportService(TransportService transportService) { false, UpdateSettingsRequest::new, ((request, channel, task) -> channel.sendResponse(updateSettingsRequestHandler.handleUpdateSettingsRequest(request))) - ); - + ); + // TODO: This handles a remote extension request sending a RemoteExtensionActionResponse // For actions sent from OpenSearch or a plugin using ProxyAction need to write a new request handler // for ExtensionsManager.REQUEST_EXTENSION_HANDLE_TRANSPORT_ACTION that sends an ExtensionActionResponse diff --git a/src/main/java/org/opensearch/sdk/SDKTransportService.java b/src/main/java/org/opensearch/sdk/SDKTransportService.java index 0b8e6cc8..afe6581e 100644 --- a/src/main/java/org/opensearch/sdk/SDKTransportService.java +++ b/src/main/java/org/opensearch/sdk/SDKTransportService.java @@ -24,7 +24,7 @@ import org.opensearch.extensions.action.RegisterTransportActionsRequest; import org.opensearch.extensions.action.TransportActionRequestFromExtension; import org.opensearch.sdk.ActionExtension.ActionHandler; -import org.opensearch.sdk.action.ProxyActionRequest; +import org.opensearch.sdk.action.RemoteExtensionActionRequest; import org.opensearch.sdk.action.SDKActionModule; import org.opensearch.sdk.handlers.AcknowledgedResponseHandler; import org.opensearch.sdk.handlers.ExtensionActionResponseHandler; @@ -51,7 +51,11 @@ public class SDKTransportService { */ public void sendRegisterTransportActionsRequest(Map> actions) { logger.info("Sending Register Transport Actions request to OpenSearch"); - Set actionNameSet = actions.values().stream().map(h -> h.getAction().getClass().getName()).collect(Collectors.toSet()); + Set actionNameSet = actions.values() + .stream() + .filter(h -> !h.getAction().name().startsWith("internal")) + .map(h -> h.getAction().getClass().getName()) + .collect(Collectors.toSet()); AcknowledgedResponseHandler registerTransportActionsResponseHandler = new AcknowledgedResponseHandler(); try { transportService.sendRequest( @@ -72,7 +76,7 @@ public void sendRegisterTransportActionsRequest(Map> * @return A buffer serializing the response from the remote action if successful, otherwise null */ @Nullable - public byte[] sendProxyActionRequest(ProxyActionRequest request) { + public byte[] sendProxyActionRequest(RemoteExtensionActionRequest request) { logger.info("Sending ProxyAction request to OpenSearch for [" + request.getAction() + "]"); // Combine class name string and request bytes byte[] requestClassBytes = request.getRequestClass().getBytes(StandardCharsets.UTF_8); diff --git a/src/main/java/org/opensearch/sdk/action/ProxyAction.java b/src/main/java/org/opensearch/sdk/action/RemoteExtensionAction.java similarity index 60% rename from src/main/java/org/opensearch/sdk/action/ProxyAction.java rename to src/main/java/org/opensearch/sdk/action/RemoteExtensionAction.java index 3cb52c2c..b18b8487 100644 --- a/src/main/java/org/opensearch/sdk/action/ProxyAction.java +++ b/src/main/java/org/opensearch/sdk/action/RemoteExtensionAction.java @@ -10,24 +10,23 @@ package org.opensearch.sdk.action; import org.opensearch.action.ActionType; -import org.opensearch.extensions.action.ExtensionActionResponse; import org.opensearch.extensions.action.RemoteExtensionActionResponse; /** - * The {@link ActionType} used as they key for the {@link ProxyTransportAction}. + * The {@link ActionType} used as they key for the {@link RemoteExtensionTransportAction}. */ -public class ProxyAction extends ActionType { +public class RemoteExtensionAction extends ActionType { /** * The name to look up this action with */ - public static final String NAME = "internal/proxyaction"; + public static final String NAME = "internal/extension-proxyaction"; /** * The singleton instance of this class */ - public static final ProxyAction INSTANCE = new ProxyAction(); + public static final RemoteExtensionAction INSTANCE = new RemoteExtensionAction(); - private ProxyAction() { + private RemoteExtensionAction() { super(NAME, RemoteExtensionActionResponse::new); } } diff --git a/src/main/java/org/opensearch/sdk/action/ProxyActionRequest.java b/src/main/java/org/opensearch/sdk/action/RemoteExtensionActionRequest.java similarity index 91% rename from src/main/java/org/opensearch/sdk/action/ProxyActionRequest.java rename to src/main/java/org/opensearch/sdk/action/RemoteExtensionActionRequest.java index d26a95d9..ecb95b48 100644 --- a/src/main/java/org/opensearch/sdk/action/ProxyActionRequest.java +++ b/src/main/java/org/opensearch/sdk/action/RemoteExtensionActionRequest.java @@ -25,7 +25,7 @@ /** * A request class to request an action be executed on another extension */ -public class ProxyActionRequest extends ActionRequest { +public class RemoteExtensionActionRequest extends ActionRequest { /** * action is the TransportAction intended to be invoked which is registered by an extension via {@link ExtensionTransportActionsHandler}. */ @@ -45,7 +45,7 @@ public class ProxyActionRequest extends ActionRequest { * @param instance An instance of {@link ActionType} registered with the remote extension's getActions registry * @param request A class extending {@link ActionRequest} associated with an action to be executed on another extension. */ - public ProxyActionRequest(ActionType instance, ActionRequest request) { + public RemoteExtensionActionRequest(ActionType instance, ActionRequest request) { this.action = instance.getClass().getName(); this.requestClass = request.getClass().getName(); byte[] bytes = new byte[0]; @@ -66,7 +66,7 @@ public ProxyActionRequest(ActionType instance, ActionR * @param requestClass A string representing the fully qualified class name of the remote ActionRequest class * @param requestBytes Bytes representing the serialized parameters to be used in the ActionRequest class StreamInput constructor */ - public ProxyActionRequest(String action, String requestClass, byte[] requestBytes) { + public RemoteExtensionActionRequest(String action, String requestClass, byte[] requestBytes) { this.action = action; this.requestClass = requestClass; this.requestBytes = requestBytes; @@ -78,7 +78,7 @@ public ProxyActionRequest(String action, String requestClass, byte[] requestByte * @param in bytes stream input used to de-serialize the message. * @throws IOException when message de-serialization fails. */ - public ProxyActionRequest(StreamInput in) throws IOException { + public RemoteExtensionActionRequest(StreamInput in) throws IOException { super(in); this.action = in.readString(); this.requestClass = in.readString(); @@ -119,7 +119,7 @@ public String toString() { public boolean equals(Object obj) { if (this == obj) return true; if (obj == null || getClass() != obj.getClass()) return false; - ProxyActionRequest that = (ProxyActionRequest) obj; + RemoteExtensionActionRequest that = (RemoteExtensionActionRequest) obj; return Objects.equals(action, that.action) && Objects.equals(requestClass, that.requestClass) && Objects.equals(requestBytes, that.requestBytes); diff --git a/src/main/java/org/opensearch/sdk/action/ProxyTransportAction.java b/src/main/java/org/opensearch/sdk/action/RemoteExtensionTransportAction.java similarity index 83% rename from src/main/java/org/opensearch/sdk/action/ProxyTransportAction.java rename to src/main/java/org/opensearch/sdk/action/RemoteExtensionTransportAction.java index 36f81e07..e52ea3eb 100644 --- a/src/main/java/org/opensearch/sdk/action/ProxyTransportAction.java +++ b/src/main/java/org/opensearch/sdk/action/RemoteExtensionTransportAction.java @@ -12,7 +12,6 @@ import org.opensearch.action.ActionListener; import org.opensearch.action.support.ActionFilters; import org.opensearch.action.support.TransportAction; -import org.opensearch.extensions.action.ExtensionActionResponse; import org.opensearch.extensions.action.RemoteExtensionActionResponse; import org.opensearch.sdk.SDKTransportService; import org.opensearch.tasks.Task; @@ -23,7 +22,7 @@ /** * Sends a request to OpenSearch for a remote extension to execute an action. */ -public class ProxyTransportAction extends TransportAction { +public class RemoteExtensionTransportAction extends TransportAction { private SDKTransportService sdkTransportService; @@ -36,7 +35,7 @@ public class ProxyTransportAction extends TransportAction listener) { + protected void doExecute(Task task, RemoteExtensionActionRequest request, ActionListener listener) { byte[] responseBytes = sdkTransportService.sendProxyActionRequest(request); if (responseBytes == null) { listener.onFailure(new RuntimeException("No response received from remote extension.")); diff --git a/src/main/java/org/opensearch/sdk/action/SDKActionModule.java b/src/main/java/org/opensearch/sdk/action/SDKActionModule.java index 41d7df16..9f77aadc 100644 --- a/src/main/java/org/opensearch/sdk/action/SDKActionModule.java +++ b/src/main/java/org/opensearch/sdk/action/SDKActionModule.java @@ -73,7 +73,7 @@ public void register(ActionHandler handler) { ActionRegistry actions = new ActionRegistry(); // Register SDK actions - actions.register(new ActionHandler<>(ProxyAction.INSTANCE, ProxyTransportAction.class)); + actions.register(new ActionHandler<>(RemoteExtensionAction.INSTANCE, RemoteExtensionTransportAction.class)); // Register actions from getActions extension point if (extension instanceof ActionExtension) { diff --git a/src/main/java/org/opensearch/sdk/handlers/ExtensionActionRequestHandler.java b/src/main/java/org/opensearch/sdk/handlers/ExtensionActionRequestHandler.java index 8c5b3515..2a42d215 100644 --- a/src/main/java/org/opensearch/sdk/handlers/ExtensionActionRequestHandler.java +++ b/src/main/java/org/opensearch/sdk/handlers/ExtensionActionRequestHandler.java @@ -28,7 +28,6 @@ import org.opensearch.common.io.stream.StreamInput; import org.opensearch.extensions.ExtensionsManager; import org.opensearch.extensions.action.ExtensionActionRequest; -import org.opensearch.extensions.action.ExtensionActionResponse; import org.opensearch.extensions.action.RemoteExtensionActionResponse; import org.opensearch.sdk.SDKClient; import org.opensearch.sdk.SDKTransportService; diff --git a/src/main/java/org/opensearch/sdk/handlers/ExtensionActionResponseHandler.java b/src/main/java/org/opensearch/sdk/handlers/ExtensionActionResponseHandler.java index 4a953816..b1b45782 100644 --- a/src/main/java/org/opensearch/sdk/handlers/ExtensionActionResponseHandler.java +++ b/src/main/java/org/opensearch/sdk/handlers/ExtensionActionResponseHandler.java @@ -12,7 +12,6 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.opensearch.extensions.ExtensionsManager; -import org.opensearch.extensions.action.ExtensionActionResponse; import org.opensearch.extensions.action.RemoteExtensionActionResponse; import org.opensearch.sdk.SDKTransportService; import org.opensearch.common.Nullable; diff --git a/src/main/java/org/opensearch/sdk/sample/helloworld/rest/RestRemoteHelloAction.java b/src/main/java/org/opensearch/sdk/sample/helloworld/rest/RestRemoteHelloAction.java index 18f948f8..9ce93adf 100644 --- a/src/main/java/org/opensearch/sdk/sample/helloworld/rest/RestRemoteHelloAction.java +++ b/src/main/java/org/opensearch/sdk/sample/helloworld/rest/RestRemoteHelloAction.java @@ -12,7 +12,6 @@ import org.opensearch.action.ActionListener; import org.opensearch.common.io.stream.StreamInput; import org.opensearch.extensions.ExtensionsManager; -import org.opensearch.extensions.action.ExtensionActionResponse; import org.opensearch.extensions.action.RemoteExtensionActionResponse; import org.opensearch.extensions.rest.ExtensionRestRequest; import org.opensearch.extensions.rest.ExtensionRestResponse; @@ -20,8 +19,8 @@ import org.opensearch.sdk.ExtensionsRunner; import org.opensearch.sdk.RouteHandler; import org.opensearch.sdk.SDKClient; -import org.opensearch.sdk.action.ProxyAction; -import org.opensearch.sdk.action.ProxyActionRequest; +import org.opensearch.sdk.action.RemoteExtensionAction; +import org.opensearch.sdk.action.RemoteExtensionActionRequest; import org.opensearch.sdk.sample.helloworld.transport.SampleAction; import org.opensearch.sdk.sample.helloworld.transport.SampleRequest; import org.opensearch.sdk.sample.helloworld.transport.SampleResponse; @@ -66,19 +65,21 @@ public List routeHandlers() { // Serialize this request in a proxy action request // This requires that the remote extension has a corresponding transport action registered // This Action class happens to be local for simplicity but is a class on the remote extension - ProxyActionRequest proxyActionRequest = new ProxyActionRequest(SampleAction.INSTANCE, sampleRequest); + RemoteExtensionActionRequest proxyActionRequest = new RemoteExtensionActionRequest(SampleAction.INSTANCE, sampleRequest); // TODO: We need async client.execute to hide these action listener details and return the future directly // https://github.com/opensearch-project/opensearch-sdk-java/issues/584 CompletableFuture futureResponse = new CompletableFuture<>(); client.execute( - ProxyAction.INSTANCE, + RemoteExtensionAction.INSTANCE, proxyActionRequest, ActionListener.wrap(r -> futureResponse.complete(r), e -> futureResponse.completeExceptionally(e)) ); try { - RemoteExtensionActionResponse response = futureResponse.orTimeout(ExtensionsManager.EXTENSION_REQUEST_WAIT_TIMEOUT, TimeUnit.SECONDS) - .get(); + RemoteExtensionActionResponse response = futureResponse.orTimeout( + ExtensionsManager.EXTENSION_REQUEST_WAIT_TIMEOUT, + TimeUnit.SECONDS + ).get(); if (!response.isSuccess()) { return new ExtensionRestResponse(request, OK, "Remote extension reponse failed: " + response.getResponseBytesAsString()); } diff --git a/src/test/java/org/opensearch/sdk/TestSDKTransportService.java b/src/test/java/org/opensearch/sdk/TestSDKTransportService.java index 3aafd329..9fba866e 100644 --- a/src/test/java/org/opensearch/sdk/TestSDKTransportService.java +++ b/src/test/java/org/opensearch/sdk/TestSDKTransportService.java @@ -18,7 +18,7 @@ import org.opensearch.common.transport.TransportAddress; import org.opensearch.extensions.ExtensionsManager; import org.opensearch.extensions.action.RegisterTransportActionsRequest; -import org.opensearch.sdk.action.ProxyAction; +import org.opensearch.sdk.action.RemoteExtensionAction; import org.opensearch.sdk.action.SDKActionModule; import org.opensearch.sdk.action.TestSDKActionModule; import org.opensearch.sdk.handlers.AcknowledgedResponseHandler; @@ -92,6 +92,6 @@ public void testRegisterTransportAction() { any(AcknowledgedResponseHandler.class) ); assertEquals(TEST_UNIQUE_ID, registerTransportActionsRequestCaptor.getValue().getUniqueId()); - assertTrue(registerTransportActionsRequestCaptor.getValue().getTransportActions().contains(ProxyAction.class.getName())); + assertTrue(registerTransportActionsRequestCaptor.getValue().getTransportActions().contains(RemoteExtensionAction.class.getName())); } } diff --git a/src/test/java/org/opensearch/sdk/action/TestProxyActionRequest.java b/src/test/java/org/opensearch/sdk/action/TestProxyActionRequest.java index d8920a6c..417fbdac 100644 --- a/src/test/java/org/opensearch/sdk/action/TestProxyActionRequest.java +++ b/src/test/java/org/opensearch/sdk/action/TestProxyActionRequest.java @@ -37,18 +37,18 @@ public void testProxyActionRequest() throws Exception { expectedRequestBytes = BytesReference.toBytes(out.bytes()); } - ProxyActionRequest request = new ProxyActionRequest(TestAction.INSTANCE, testRequest); + RemoteExtensionActionRequest request = new RemoteExtensionActionRequest(TestAction.INSTANCE, testRequest); assertEquals(expectedAction, request.getAction()); assertEquals(expectedRequestClass, request.getRequestClass()); assertArrayEquals(expectedRequestBytes, request.getRequestBytes()); - request = new ProxyActionRequest(expectedAction, expectedRequestClass, expectedRequestBytes); + request = new RemoteExtensionActionRequest(expectedAction, expectedRequestClass, expectedRequestBytes); try (BytesStreamOutput out = new BytesStreamOutput()) { request.writeTo(out); out.flush(); try (BytesStreamInput in = new BytesStreamInput(BytesReference.toBytes(out.bytes()))) { - request = new ProxyActionRequest(in); + request = new RemoteExtensionActionRequest(in); assertEquals(expectedAction, request.getAction()); assertEquals(expectedRequestClass, request.getRequestClass()); diff --git a/src/test/java/org/opensearch/sdk/action/TestSDKActionModule.java b/src/test/java/org/opensearch/sdk/action/TestSDKActionModule.java index 12177842..83e34fdf 100644 --- a/src/test/java/org/opensearch/sdk/action/TestSDKActionModule.java +++ b/src/test/java/org/opensearch/sdk/action/TestSDKActionModule.java @@ -56,7 +56,7 @@ public void setUp() throws Exception { @Test public void testGetActions() { assertEquals(2, sdkActionModule.getActions().size()); - assertTrue(sdkActionModule.getActions().containsKey(ProxyAction.NAME)); + assertTrue(sdkActionModule.getActions().containsKey(RemoteExtensionAction.NAME)); assertTrue(sdkActionModule.getActions().containsKey(TEST_ACTION_NAME)); } } From 79499a089eef38c4a641d374a2ffd90268522fa5 Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Thu, 23 Mar 2023 08:25:30 -0700 Subject: [PATCH 17/20] Better handling of response bytes Signed-off-by: Daniel Widdis --- .../org/opensearch/sdk/SDKTransportService.java | 16 +++++++++------- .../action/RemoteExtensionTransportAction.java | 8 ++++---- .../handlers/ExtensionActionResponseHandler.java | 12 +++++++----- 3 files changed, 20 insertions(+), 16 deletions(-) diff --git a/src/main/java/org/opensearch/sdk/SDKTransportService.java b/src/main/java/org/opensearch/sdk/SDKTransportService.java index afe6581e..596a64e2 100644 --- a/src/main/java/org/opensearch/sdk/SDKTransportService.java +++ b/src/main/java/org/opensearch/sdk/SDKTransportService.java @@ -19,9 +19,9 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.opensearch.cluster.node.DiscoveryNode; -import org.opensearch.common.Nullable; import org.opensearch.extensions.ExtensionsManager; import org.opensearch.extensions.action.RegisterTransportActionsRequest; +import org.opensearch.extensions.action.RemoteExtensionActionResponse; import org.opensearch.extensions.action.TransportActionRequestFromExtension; import org.opensearch.sdk.ActionExtension.ActionHandler; import org.opensearch.sdk.action.RemoteExtensionActionRequest; @@ -75,9 +75,8 @@ public void sendRegisterTransportActionsRequest(Map> * @param request The request to send * @return A buffer serializing the response from the remote action if successful, otherwise null */ - @Nullable - public byte[] sendProxyActionRequest(RemoteExtensionActionRequest request) { - logger.info("Sending ProxyAction request to OpenSearch for [" + request.getAction() + "]"); + public RemoteExtensionActionResponse sendRemoteExtensionActionRequest(RemoteExtensionActionRequest request) { + logger.info("Sending Remote Extension Action request to OpenSearch for [" + request.getAction() + "]"); // Combine class name string and request bytes byte[] requestClassBytes = request.getRequestClass().getBytes(StandardCharsets.UTF_8); byte[] proxyRequestBytes = ByteBuffer.allocate(requestClassBytes.length + 1 + request.getRequestBytes().length) @@ -96,12 +95,15 @@ public byte[] sendProxyActionRequest(RemoteExtensionActionRequest request) { // Wait on response extensionActionResponseHandler.awaitResponse(); } catch (TimeoutException e) { - logger.info("Failed to receive ProxyAction response from OpenSearch", e); + logger.info("Failed to receive Remote Extension Action response from OpenSearch", e); } catch (Exception e) { - logger.info("Failed to send ProxyAction request to OpenSearch", e); + logger.info("Failed to send Remote Extension Action request to OpenSearch", e); } // At this point, response handler has read in the response bytes - return extensionActionResponseHandler.getResponseBytes(); + return new RemoteExtensionActionResponse( + extensionActionResponseHandler.isSuccess(), + extensionActionResponseHandler.getResponseBytes() + ); } public TransportService getTransportService() { diff --git a/src/main/java/org/opensearch/sdk/action/RemoteExtensionTransportAction.java b/src/main/java/org/opensearch/sdk/action/RemoteExtensionTransportAction.java index e52ea3eb..fa9ce8ed 100644 --- a/src/main/java/org/opensearch/sdk/action/RemoteExtensionTransportAction.java +++ b/src/main/java/org/opensearch/sdk/action/RemoteExtensionTransportAction.java @@ -47,11 +47,11 @@ protected RemoteExtensionTransportAction( @Override protected void doExecute(Task task, RemoteExtensionActionRequest request, ActionListener listener) { - byte[] responseBytes = sdkTransportService.sendProxyActionRequest(request); - if (responseBytes == null) { - listener.onFailure(new RuntimeException("No response received from remote extension.")); + RemoteExtensionActionResponse response = sdkTransportService.sendRemoteExtensionActionRequest(request); + if (response.getResponseBytes().length > 0) { + listener.onResponse(response); } else { - listener.onResponse(new RemoteExtensionActionResponse(true, responseBytes)); + listener.onFailure(new RuntimeException("No response received from remote extension.")); } } } diff --git a/src/main/java/org/opensearch/sdk/handlers/ExtensionActionResponseHandler.java b/src/main/java/org/opensearch/sdk/handlers/ExtensionActionResponseHandler.java index b1b45782..21e67c01 100644 --- a/src/main/java/org/opensearch/sdk/handlers/ExtensionActionResponseHandler.java +++ b/src/main/java/org/opensearch/sdk/handlers/ExtensionActionResponseHandler.java @@ -14,7 +14,6 @@ import org.opensearch.extensions.ExtensionsManager; import org.opensearch.extensions.action.RemoteExtensionActionResponse; import org.opensearch.sdk.SDKTransportService; -import org.opensearch.common.Nullable; import org.opensearch.common.io.stream.StreamInput; import org.opensearch.threadpool.ThreadPool; import org.opensearch.transport.TransportException; @@ -31,7 +30,8 @@ public class ExtensionActionResponseHandler implements TransportResponseHandler< private static final Logger logger = LogManager.getLogger(ExtensionActionResponseHandler.class); private final CompletableFuture inProgressFuture; - private byte[] responseBytes = null; + private boolean success = false; + private byte[] responseBytes = new byte[0]; /** * Instantiates a new ExtensionActionResponseHandler @@ -43,9 +43,8 @@ public ExtensionActionResponseHandler() { @Override public void handleResponse(RemoteExtensionActionResponse response) { logger.info("Received response bytes: " + response.getResponseBytes().length + " bytes"); - - logger.debug("Received response bytes: " + response.getResponseBytesAsString()); // Set ExtensionActionResponse from response + this.success = response.isSuccess(); this.responseBytes = response.getResponseBytes(); inProgressFuture.complete(response); } @@ -75,7 +74,10 @@ public void awaitResponse() throws Exception { inProgressFuture.orTimeout(ExtensionsManager.EXTENSION_REQUEST_WAIT_TIMEOUT, TimeUnit.SECONDS).get(); } - @Nullable + public boolean isSuccess() { + return success; + } + public byte[] getResponseBytes() { return this.responseBytes; } From a3182af2515e65c832c75629784db6888d9df19c Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Thu, 23 Mar 2023 11:38:29 -0700 Subject: [PATCH 18/20] Handle plugin remote action requests Signed-off-by: Daniel Widdis --- .../org/opensearch/sdk/ExtensionsRunner.java | 16 ++++++++++++---- .../ExtensionActionRequestHandler.java | 19 +++++++++++++++++-- .../opensearch/sdk/TestExtensionsRunner.java | 2 +- .../sdk/TestSDKTransportService.java | 11 +++++++++-- 4 files changed, 39 insertions(+), 9 deletions(-) diff --git a/src/main/java/org/opensearch/sdk/ExtensionsRunner.java b/src/main/java/org/opensearch/sdk/ExtensionsRunner.java index 729e60e6..bfe51189 100644 --- a/src/main/java/org/opensearch/sdk/ExtensionsRunner.java +++ b/src/main/java/org/opensearch/sdk/ExtensionsRunner.java @@ -403,17 +403,25 @@ public void startTransportService(TransportService transportService) { ((request, channel, task) -> channel.sendResponse(updateSettingsRequestHandler.handleUpdateSettingsRequest(request))) ); - // TODO: This handles a remote extension request sending a RemoteExtensionActionResponse - // For actions sent from OpenSearch or a plugin using ProxyAction need to write a new request handler - // for ExtensionsManager.REQUEST_EXTENSION_HANDLE_TRANSPORT_ACTION that sends an ExtensionActionResponse + // This handles a remote extension request from OpenSearch or a plugin, sending an ExtensionActionResponse transportService.registerRequestHandler( - ExtensionsManager.REQUEST_EXTENSION_HANDLE_REMOTE_TRANSPORT_ACTION, + ExtensionsManager.REQUEST_EXTENSION_HANDLE_TRANSPORT_ACTION, ThreadPool.Names.GENERIC, false, false, ExtensionActionRequest::new, ((request, channel, task) -> channel.sendResponse(extensionsActionRequestHandler.handleExtensionActionRequest(request))) ); + + // This handles a remote extension request from another extension, sending a RemoteExtensionActionResponse + transportService.registerRequestHandler( + ExtensionsManager.REQUEST_EXTENSION_HANDLE_REMOTE_TRANSPORT_ACTION, + ThreadPool.Names.GENERIC, + false, + false, + ExtensionActionRequest::new, + ((request, channel, task) -> channel.sendResponse(extensionsActionRequestHandler.handleRemoteExtensionActionRequest(request))) + ); } /** diff --git a/src/main/java/org/opensearch/sdk/handlers/ExtensionActionRequestHandler.java b/src/main/java/org/opensearch/sdk/handlers/ExtensionActionRequestHandler.java index 2a42d215..95d2214b 100644 --- a/src/main/java/org/opensearch/sdk/handlers/ExtensionActionRequestHandler.java +++ b/src/main/java/org/opensearch/sdk/handlers/ExtensionActionRequestHandler.java @@ -28,6 +28,7 @@ import org.opensearch.common.io.stream.StreamInput; import org.opensearch.extensions.ExtensionsManager; import org.opensearch.extensions.action.ExtensionActionRequest; +import org.opensearch.extensions.action.ExtensionActionResponse; import org.opensearch.extensions.action.RemoteExtensionActionResponse; import org.opensearch.sdk.SDKClient; import org.opensearch.sdk.SDKTransportService; @@ -54,12 +55,26 @@ public ExtensionActionRequestHandler(SDKClient sdkClient, SDKActionModule sdkAct } /** - * Handles a request from OpenSearch to execute a TransportAction on the extension. + * Handles a request from OpenSearch to execute a TransportAction on the extension. These requests originated from OpenSearch or a plugin. * * @param request The request to execute * @return The response from the TransportAction */ - public RemoteExtensionActionResponse handleExtensionActionRequest(ExtensionActionRequest request) { + public ExtensionActionResponse handleExtensionActionRequest(ExtensionActionRequest request) { + // For now we just delegate to the remote actions. + // There is potential in the future for handling these requests differently + RemoteExtensionActionResponse response = handleRemoteExtensionActionRequest(request); + // Discard the success bit and just return the bytes + return new ExtensionActionResponse(response.getResponseBytes()); + } + + /** + * Handles a request from OpenSearch to execute a TransportAction on the extension. These requests originated from another extension. + * + * @param request The request to execute + * @return The response from the TransportAction + */ + public RemoteExtensionActionResponse handleRemoteExtensionActionRequest(ExtensionActionRequest request) { logger.debug("Received request to execute action [" + request.getAction() + "]"); final RemoteExtensionActionResponse response = new RemoteExtensionActionResponse(false, new byte[0]); diff --git a/src/test/java/org/opensearch/sdk/TestExtensionsRunner.java b/src/test/java/org/opensearch/sdk/TestExtensionsRunner.java index 69d70c55..5f6a5028 100644 --- a/src/test/java/org/opensearch/sdk/TestExtensionsRunner.java +++ b/src/test/java/org/opensearch/sdk/TestExtensionsRunner.java @@ -100,7 +100,7 @@ public void testStartTransportService() { verify(transportService, times(1)).start(); // cannot verify acceptIncomingRequests as it is a final method // test registerRequestHandlers - verify(transportService, times(6)).registerRequestHandler(anyString(), anyString(), anyBoolean(), anyBoolean(), any(), any()); + verify(transportService, times(7)).registerRequestHandler(anyString(), anyString(), anyBoolean(), anyBoolean(), any(), any()); } @Test diff --git a/src/test/java/org/opensearch/sdk/TestSDKTransportService.java b/src/test/java/org/opensearch/sdk/TestSDKTransportService.java index 9fba866e..60a1bc2a 100644 --- a/src/test/java/org/opensearch/sdk/TestSDKTransportService.java +++ b/src/test/java/org/opensearch/sdk/TestSDKTransportService.java @@ -41,7 +41,6 @@ public class TestSDKTransportService extends OpenSearchTestCase { private static final String TEST_UNIQUE_ID = "test-extension"; - private static final String TEST_ACTION_NAME = "testAction"; private TransportService transportService; private DiscoveryNode opensearchNode; @@ -92,6 +91,14 @@ public void testRegisterTransportAction() { any(AcknowledgedResponseHandler.class) ); assertEquals(TEST_UNIQUE_ID, registerTransportActionsRequestCaptor.getValue().getUniqueId()); - assertTrue(registerTransportActionsRequestCaptor.getValue().getTransportActions().contains(RemoteExtensionAction.class.getName())); + // Should contain the TestAction, but since it's mocked the name may change + assertTrue( + registerTransportActionsRequestCaptor.getValue() + .getTransportActions() + .stream() + .anyMatch(s -> s.startsWith("org.opensearch.action.ActionType$MockitoMock$")) + ); + // Internal action should be filtered out + assertFalse(registerTransportActionsRequestCaptor.getValue().getTransportActions().contains(RemoteExtensionAction.class.getName())); } } From 2f18fcbd9bfb7f95cb91050c9c17238142f0f56a Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Thu, 23 Mar 2023 13:43:01 -0700 Subject: [PATCH 19/20] Address code review comments Signed-off-by: Daniel Widdis --- .../org/opensearch/sdk/ExtensionsRunner.java | 12 +++---- .../opensearch/sdk/SDKTransportService.java | 8 ++--- .../sdk/action/RemoteExtensionAction.java | 2 +- .../action/RemoteExtensionActionRequest.java | 17 ++++++---- .../sdk/action/SDKActionModule.java | 2 +- .../ExtensionActionRequestHandler.java | 33 +++++++------------ .../ExtensionActionResponseHandler.java | 2 +- .../helloworld/HelloWorldExtension.java | 1 - .../helloworld/TestHelloWorldExtension.java | 6 ++++ 9 files changed, 41 insertions(+), 42 deletions(-) diff --git a/src/main/java/org/opensearch/sdk/ExtensionsRunner.java b/src/main/java/org/opensearch/sdk/ExtensionsRunner.java index bfe51189..ccc87db1 100644 --- a/src/main/java/org/opensearch/sdk/ExtensionsRunner.java +++ b/src/main/java/org/opensearch/sdk/ExtensionsRunner.java @@ -137,12 +137,12 @@ public class ExtensionsRunner { private final SDKTransportService sdkTransportService; private final SDKActionModule sdkActionModule; - private ExtensionsInitRequestHandler extensionsInitRequestHandler = new ExtensionsInitRequestHandler(this); - private ExtensionsIndicesModuleRequestHandler extensionsIndicesModuleRequestHandler = new ExtensionsIndicesModuleRequestHandler(); - private ExtensionsIndicesModuleNameRequestHandler extensionsIndicesModuleNameRequestHandler = + private final ExtensionsInitRequestHandler extensionsInitRequestHandler = new ExtensionsInitRequestHandler(this); + private final ExtensionsIndicesModuleRequestHandler extensionsIndicesModuleRequestHandler = new ExtensionsIndicesModuleRequestHandler(); + private final ExtensionsIndicesModuleNameRequestHandler extensionsIndicesModuleNameRequestHandler = new ExtensionsIndicesModuleNameRequestHandler(); - private ExtensionsRestRequestHandler extensionsRestRequestHandler = new ExtensionsRestRequestHandler(extensionRestPathRegistry); - private ExtensionActionRequestHandler extensionsActionRequestHandler; + private final ExtensionsRestRequestHandler extensionsRestRequestHandler = new ExtensionsRestRequestHandler(extensionRestPathRegistry); + private final ExtensionActionRequestHandler extensionsActionRequestHandler; /** * Instantiates a new update settings request handler @@ -212,7 +212,7 @@ protected ExtensionsRunner(Extension extension) throws IOException { // initialize SDKClient action map initializeSdkClient(); - extensionsActionRequestHandler = new ExtensionActionRequestHandler(getSdkClient(), getSdkActionModule()); + extensionsActionRequestHandler = new ExtensionActionRequestHandler(getSdkClient()); if (extension instanceof ActionExtension) { // store REST handlers in the registry diff --git a/src/main/java/org/opensearch/sdk/SDKTransportService.java b/src/main/java/org/opensearch/sdk/SDKTransportService.java index 596a64e2..20d8377a 100644 --- a/src/main/java/org/opensearch/sdk/SDKTransportService.java +++ b/src/main/java/org/opensearch/sdk/SDKTransportService.java @@ -65,7 +65,7 @@ public void sendRegisterTransportActionsRequest(Map> registerTransportActionsResponseHandler ); } catch (Exception e) { - logger.info("Failed to send Register Transport Actions request to OpenSearch", e); + logger.error("Failed to send Register Transport Actions request to OpenSearch", e); } } @@ -81,7 +81,7 @@ public RemoteExtensionActionResponse sendRemoteExtensionActionRequest(RemoteExte byte[] requestClassBytes = request.getRequestClass().getBytes(StandardCharsets.UTF_8); byte[] proxyRequestBytes = ByteBuffer.allocate(requestClassBytes.length + 1 + request.getRequestBytes().length) .put(requestClassBytes) - .put((byte) 0) + .put(RemoteExtensionActionRequest.UNIT_SEPARATOR) .put(request.getRequestBytes()) .array(); ExtensionActionResponseHandler extensionActionResponseHandler = new ExtensionActionResponseHandler(); @@ -95,9 +95,9 @@ public RemoteExtensionActionResponse sendRemoteExtensionActionRequest(RemoteExte // Wait on response extensionActionResponseHandler.awaitResponse(); } catch (TimeoutException e) { - logger.info("Failed to receive Remote Extension Action response from OpenSearch", e); + logger.error("Failed to receive Remote Extension Action response from OpenSearch", e); } catch (Exception e) { - logger.info("Failed to send Remote Extension Action request to OpenSearch", e); + logger.error("Failed to send Remote Extension Action request to OpenSearch", e); } // At this point, response handler has read in the response bytes return new RemoteExtensionActionResponse( diff --git a/src/main/java/org/opensearch/sdk/action/RemoteExtensionAction.java b/src/main/java/org/opensearch/sdk/action/RemoteExtensionAction.java index b18b8487..4cb8d00d 100644 --- a/src/main/java/org/opensearch/sdk/action/RemoteExtensionAction.java +++ b/src/main/java/org/opensearch/sdk/action/RemoteExtensionAction.java @@ -20,7 +20,7 @@ public class RemoteExtensionAction extends ActionType + * This array is the serialized bytes used to instantiate the {@link #requestClass} instance using its StreamInput constructor. */ private final byte[] requestBytes; /** - * ProxyActionRequest constructor with an ActionType and Request class. Requires a dependency on the remote extension code. + * RemoteExtensionActionRequest constructor with an ActionType and Request class. Requires a dependency on the remote extension code. * * @param instance An instance of {@link ActionType} registered with the remote extension's getActions registry * @param request A class extending {@link ActionRequest} associated with an action to be executed on another extension. @@ -53,14 +59,13 @@ public RemoteExtensionActionRequest(ActionType instanc request.writeTo(out); bytes = BytesReference.toBytes(out.bytes()); } catch (IOException e) { - // This Should Never Happen (TM) - // Won't get an IOException locally + throw new IllegalStateException("Writing an OutputStream to memory should never result in an IOException."); } this.requestBytes = bytes; } /** - * ProxyActionRequest constructor with class names and request bytes. Does not require a dependency on the remote extension code. + * RemoteExtensionActionRequest constructor with class names and request bytes. Does not require a dependency on the remote extension code. * * @param action A string representing the fully qualified class name of the remote ActionType instance * @param requestClass A string representing the fully qualified class name of the remote ActionRequest class @@ -73,7 +78,7 @@ public RemoteExtensionActionRequest(String action, String requestClass, byte[] r } /** - * ProxyActionRequest constructor from {@link StreamInput}. + * RemoteExtensionActionRequest constructor from {@link StreamInput}. * * @param in bytes stream input used to de-serialize the message. * @throws IOException when message de-serialization fails. @@ -112,7 +117,7 @@ public ActionRequestValidationException validate() { @Override public String toString() { - return "ProxyActionRequest{action=" + action + ", requestClass=" + requestClass + ", requestBytes=" + requestBytes + "}"; + return "RemoteExtensionActionRequest{action=" + action + ", requestClass=" + requestClass + ", requestBytes=" + requestBytes + "}"; } @Override diff --git a/src/main/java/org/opensearch/sdk/action/SDKActionModule.java b/src/main/java/org/opensearch/sdk/action/SDKActionModule.java index 9f77aadc..4b6cf228 100644 --- a/src/main/java/org/opensearch/sdk/action/SDKActionModule.java +++ b/src/main/java/org/opensearch/sdk/action/SDKActionModule.java @@ -58,7 +58,7 @@ public ActionFilters getActionFilters() { */ class ActionRegistry extends NamedRegistry> { ActionRegistry() { - super("sdkaction"); + super("action"); } /** diff --git a/src/main/java/org/opensearch/sdk/handlers/ExtensionActionRequestHandler.java b/src/main/java/org/opensearch/sdk/handlers/ExtensionActionRequestHandler.java index 95d2214b..7511cf9e 100644 --- a/src/main/java/org/opensearch/sdk/handlers/ExtensionActionRequestHandler.java +++ b/src/main/java/org/opensearch/sdk/handlers/ExtensionActionRequestHandler.java @@ -13,7 +13,6 @@ import java.lang.reflect.Constructor; import java.nio.charset.StandardCharsets; import java.util.Arrays; -import java.util.Optional; import java.util.concurrent.CompletableFuture; import java.util.concurrent.TimeUnit; @@ -32,7 +31,7 @@ import org.opensearch.extensions.action.RemoteExtensionActionResponse; import org.opensearch.sdk.SDKClient; import org.opensearch.sdk.SDKTransportService; -import org.opensearch.sdk.action.SDKActionModule; +import org.opensearch.sdk.action.RemoteExtensionActionRequest; /** * This class handles a request from OpenSearch from another extension's {@link SDKTransportService#sendProxyActionRequest()} call. @@ -40,18 +39,15 @@ public class ExtensionActionRequestHandler { private static final Logger logger = LogManager.getLogger(ExtensionActionRequestHandler.class); - final SDKActionModule sdkActionModule; - final SDKClient client; + private final SDKClient sdkClient; /** * Instantiate this handler * * @param sdkClient An initialized SDKClient with the registered actions - * @param sdkActionModule An initialized SDKActionModule with the registered actions */ - public ExtensionActionRequestHandler(SDKClient sdkClient, SDKActionModule sdkActionModule) { - this.sdkActionModule = sdkActionModule; - this.client = sdkClient; + public ExtensionActionRequestHandler(SDKClient sdkClient) { + this.sdkClient = sdkClient; } /** @@ -79,23 +75,17 @@ public RemoteExtensionActionResponse handleRemoteExtensionActionRequest(Extensio final RemoteExtensionActionResponse response = new RemoteExtensionActionResponse(false, new byte[0]); // Find matching ActionType instance - Optional> optionalAction = sdkActionModule.getActions() - .values() - .stream() - .map(h -> h.getAction()) - .filter(a -> a.getClass().getName().equals(request.getAction())) - .findAny(); - if (optionalAction.isEmpty()) { + ActionType action = sdkClient.getActionFromClassName(request.getAction()); + if (action == null) { response.setResponseBytesAsString("No action [" + request.getAction() + "] is registered."); return response; } - - ActionType action = optionalAction.get(); logger.debug("Found matching action [" + action.name() + "], an instance of [" + action.getClass().getName() + "]"); // Extract request class name from bytes and instantiate request - int nullPos = indexOf(request.getRequestBytes(), (byte) 0); - String requestClassName = new String(Arrays.copyOfRange(request.getRequestBytes(), 0, nullPos), StandardCharsets.UTF_8); + int nullPos = indexOf(request.getRequestBytes(), RemoteExtensionActionRequest.UNIT_SEPARATOR); + String requestClassName = new String(Arrays.copyOfRange(request.getRequestBytes(), 0, nullPos + 1), StandardCharsets.UTF_8) + .stripTrailing(); ActionRequest actionRequest = null; try { Class clazz = Class.forName(requestClassName); @@ -113,14 +103,13 @@ public RemoteExtensionActionResponse handleRemoteExtensionActionRequest(Extensio // TODO: We need async client.execute to hide these action listener details and return the future directly // https://github.com/opensearch-project/opensearch-sdk-java/issues/584 CompletableFuture futureResponse = new CompletableFuture<>(); - client.execute(action, actionRequest, ActionListener.wrap(r -> { + sdkClient.execute(action, actionRequest, ActionListener.wrap(r -> { byte[] bytes = new byte[0]; try (BytesStreamOutput out = new BytesStreamOutput()) { ((ActionResponse) r).writeTo(out); bytes = BytesReference.toBytes(out.bytes()); } catch (IOException e) { - // This Should Never Happen (TM) - // Won't get an IOException locally + throw new IllegalStateException("Writing an OutputStream to memory should never result in an IOException."); } response.setSuccess(true); response.setResponseBytes(bytes); diff --git a/src/main/java/org/opensearch/sdk/handlers/ExtensionActionResponseHandler.java b/src/main/java/org/opensearch/sdk/handlers/ExtensionActionResponseHandler.java index 21e67c01..caa82a50 100644 --- a/src/main/java/org/opensearch/sdk/handlers/ExtensionActionResponseHandler.java +++ b/src/main/java/org/opensearch/sdk/handlers/ExtensionActionResponseHandler.java @@ -51,7 +51,7 @@ public void handleResponse(RemoteExtensionActionResponse response) { @Override public void handleException(TransportException exp) { - logger.info("ExtensionActionResponseRequest failed", exp); + logger.error("ExtensionActionResponseRequest failed", exp); inProgressFuture.completeExceptionally(exp); } diff --git a/src/main/java/org/opensearch/sdk/sample/helloworld/HelloWorldExtension.java b/src/main/java/org/opensearch/sdk/sample/helloworld/HelloWorldExtension.java index caee8766..edbcc7e2 100644 --- a/src/main/java/org/opensearch/sdk/sample/helloworld/HelloWorldExtension.java +++ b/src/main/java/org/opensearch/sdk/sample/helloworld/HelloWorldExtension.java @@ -22,7 +22,6 @@ import org.opensearch.sdk.ExtensionSettings; import org.opensearch.sdk.ExtensionsRunner; import org.opensearch.sdk.ActionExtension; -import org.opensearch.sdk.ActionExtension.ActionHandler; import org.opensearch.sdk.sample.helloworld.rest.RestHelloAction; import org.opensearch.sdk.sample.helloworld.rest.RestRemoteHelloAction; import org.opensearch.sdk.sample.helloworld.transport.SampleAction; diff --git a/src/test/java/org/opensearch/sdk/sample/helloworld/TestHelloWorldExtension.java b/src/test/java/org/opensearch/sdk/sample/helloworld/TestHelloWorldExtension.java index e157e90c..564ac50e 100644 --- a/src/test/java/org/opensearch/sdk/sample/helloworld/TestHelloWorldExtension.java +++ b/src/test/java/org/opensearch/sdk/sample/helloworld/TestHelloWorldExtension.java @@ -121,6 +121,12 @@ public void testGetActions() { assertEquals(1, actions.size()); } + @Test + public void testClientGetActionFromClassName() { + ActionType action = SampleAction.INSTANCE; + assertEquals(action, sdkClient.getActionFromClassName(action.getClass().getName())); + } + @Test public void testClientExecuteSampleActions() throws Exception { String expectedName = "world"; From fe859e26b24386145f8d9d71d181a77057fb5cdf Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Thu, 23 Mar 2023 14:01:05 -0700 Subject: [PATCH 20/20] Update sequence diagram Signed-off-by: Daniel Widdis --- Docs/RemoteActionExecution.svg | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Docs/RemoteActionExecution.svg b/Docs/RemoteActionExecution.svg index b7172e9d..e7c9c3eb 100644 --- a/Docs/RemoteActionExecution.svg +++ b/Docs/RemoteActionExecution.svg @@ -1 +1 @@ -title%20Remote%20Action%20Execution%0A%0Aparticipant%20%22Local%20Extension%22%20as%20a%0Aparticipant%20%22OpenSearch%22%20as%20os%0Aparticipant%20%22Remote%20Extension%22%20as%20b%0A%0Aos%3C-b%3AIterate%20getActions()%20and%20register%5Cnaction%20handlers%20in%20OpenSearch%20using%5CnExtensionTransportActionsHandler%5CnregisterAction()%20method%0Aos-%3Eos%3Aregister%20actions%20in%20ActionModule%5CnDynamicActionRegistry%20linking%5CnAction%20class%20name%20to%20extension%0A%0Aa-%3Ea%3ACreate%20ProxyActionRequest%20with%5CnRemote%20Extension%20ActionType%20class%2C%5CnActionRequest%20class%2C%20and%20bytes%20from%5CnActionRequest%20param%20serialization%0A%0Aa-%3Ea%3Aclient.execute()%20invokes%20doExecute()%5Cnon%20ProxyTransportAction%0A%0Aa-%3Eos%3AsendProxyActionRequest()%20method%20in%5CnSDKTransportService%20generates%5CnTransportActionRequestFromExtension%5Cnand%20sends%20to%20OpenSearch%5CnExtensionTransportActionsHandler%0A%0Aos-%3Eos%3AhandleTransportActionRequestFromExtension()%5Cnidentifies%20remote%20Extension%20node%20and%20calls%5Cnclient.execute()%20on%20Dynamic%20action%20from%5CnDynamicActionRegistry%0A%0Aos-%3Eb%3AsendTransportRequestToExtension()%5Cngenerates%20ExtensionActionRequest%5Cnand%20sends%20to%20the%20remote%20extension%0A%0Ab%3C-b%3AExtensionActionRequestHandler%5Cnreconstructs%20ActionType%20instance%5Cnand%20ActionRequest%20class%20and%5Cninvokes%20client.execute()%20on%20the%5Cndesired%20remote%20action%0A%0Aos%3C-b%3AExtensionActionResponse%20handled%5Cnby%20sendTransportRequestToExtension()%0A%0Aa%3C-os%3AExtensionActionResponse%20handled%5Cnby%20sendProxyActionRequest()%0A%0Aa-%3Ea%3AResponse%20bytes%20deserialized%20based%5Cnon%20remote%20extension's%20ActionResponse%5CnserializationRemote Action ExecutionLocal ExtensionOpenSearchRemote ExtensionIterate getActions() and registeraction handlers in OpenSearch usingExtensionTransportActionsHandlerregisterAction() methodregister actions in ActionModuleDynamicActionRegistry linkingAction class name to extensionCreate ProxyActionRequest withRemote Extension ActionType class,ActionRequest class, and bytes fromActionRequest param serializationclient.execute() invokes doExecute()on ProxyTransportActionsendProxyActionRequest() method inSDKTransportService generatesTransportActionRequestFromExtensionand sends to OpenSearchExtensionTransportActionsHandlerhandleTransportActionRequestFromExtension()identifies remote Extension node and callsclient.execute() on Dynamic action fromDynamicActionRegistrysendTransportRequestToExtension()generates ExtensionActionRequestand sends to the remote extensionExtensionActionRequestHandlerreconstructs ActionType instanceand ActionRequest class andinvokes client.execute() on thedesired remote actionExtensionActionResponse handledby sendTransportRequestToExtension()ExtensionActionResponse handledby sendProxyActionRequest()Response bytes deserialized basedon remote extension's ActionResponseserialization +title%20Remote%20Action%20Execution%0A%0Aparticipant%20%22Local%20Extension%22%20as%20a%0Aparticipant%20%22OpenSearch%22%20as%20os%0Aparticipant%20%22Remote%20Extension%22%20as%20b%0A%0Aos%3C-b%3AIterate%20getActions()%20and%20register%5Cnaction%20handlers%20in%20OpenSearch%20using%5CnExtensionTransportActionsHandler%5CnregisterAction()%20method%0Aos-%3Eos%3Aregister%20actions%20in%20ActionModule%5CnDynamicActionRegistry%20linking%5CnAction%20class%20name%20to%20extension%0A%0Aa-%3Ea%3ACreate%20RemoteExtensionActionRequest%20with%5CnRemote%20Extension%20ActionType%20class%2C%5CnActionRequest%20class%2C%20and%20bytes%20from%5CnActionRequest%20param%20serialization%0A%0Aa-%3Ea%3Aclient.execute()%20invokes%20doExecute()%5Cnon%20RemoteExtensionTransportAction%0A%0Aa-%3Eos%3AsendRemoteExtensionActionRequest()%20method%20in%5CnSDKTransportService%20generates%5CnTransportActionRequestFromExtension%5Cnand%20sends%20to%20OpenSearch%5CnExtensionTransportActionsHandler%0A%0Aos-%3Eos%3AhandleTransportActionRequestFromExtension()%5Cnidentifies%20remote%20Extension%20node%20and%20calls%5Cnclient.execute()%20on%20Dynamic%20action%20from%5CnDynamicActionRegistry%0A%0Aos-%3Eb%3AsendTransportRequestToExtension()%5Cngenerates%20ExtensionActionRequest%5Cnand%20sends%20to%20the%20remote%20extension%0A%0Ab%3C-b%3AExtensionActionRequestHandler%5Cnreconstructs%20ActionType%20instance%5Cnand%20ActionRequest%20class%20and%5Cninvokes%20client.execute()%20on%20the%5Cndesired%20remote%20action%0A%0Aos%3C-b%3ARemoteExtensionActionResponse%20handled%5Cnby%20sendTransportRequestToExtension()%0A%0Aa%3C-os%3ARemoteExtensionActionResponse%20handled%5Cnby%20sendRemoteExtensionActionRequest()%0A%0Aa-%3Ea%3AResponse%20bytes%20deserialized%20based%20on%20remote%5Cnextension's%20ActionResponse%20serializationRemote Action ExecutionLocal ExtensionOpenSearchRemote ExtensionIterate getActions() and registeraction handlers in OpenSearch usingExtensionTransportActionsHandlerregisterAction() methodregister actions in ActionModuleDynamicActionRegistry linkingAction class name to extensionCreate RemoteExtensionActionRequest withRemote Extension ActionType class,ActionRequest class, and bytes fromActionRequest param serializationclient.execute() invokes doExecute()on RemoteExtensionTransportActionsendRemoteExtensionActionRequest() method inSDKTransportService generatesTransportActionRequestFromExtensionand sends to OpenSearchExtensionTransportActionsHandlerhandleTransportActionRequestFromExtension()identifies remote Extension node and callsclient.execute() on Dynamic action fromDynamicActionRegistrysendTransportRequestToExtension()generates ExtensionActionRequestand sends to the remote extensionExtensionActionRequestHandlerreconstructs ActionType instanceand ActionRequest class andinvokes client.execute() on thedesired remote actionRemoteExtensionActionResponse handledby sendTransportRequestToExtension()RemoteExtensionActionResponse handledby sendRemoteExtensionActionRequest()Response bytes deserialized based on remoteextension's ActionResponse serialization