From 70a8c32338cad4038bf098c3f0bb46e5aac69440 Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Wed, 28 Sep 2022 20:40:38 -0700 Subject: [PATCH 1/8] Rename/merge duplicate ExtensionRestRequest implementations Signed-off-by: Daniel Widdis --- .../opensearch/sdk/ExtensionRestHandler.java | 7 +- .../org/opensearch/sdk/ExtensionsRunner.java | 237 +++++++++++++++--- .../ExtensionsRestRequestHandler.java | 15 +- .../helloworld/rest/RestHelloAction.java | 2 +- .../sdk/TestExtensionRestPathRegistry.java | 1 + .../sdk/TestExtensionRestRequest.java | 57 ----- .../opensearch/sdk/TestExtensionsRunner.java | 18 +- .../helloworld/rest/TestRestHelloAction.java | 15 +- 8 files changed, 231 insertions(+), 121 deletions(-) delete mode 100644 src/test/java/org/opensearch/sdk/TestExtensionRestRequest.java diff --git a/src/main/java/org/opensearch/sdk/ExtensionRestHandler.java b/src/main/java/org/opensearch/sdk/ExtensionRestHandler.java index c62095e8..05553338 100644 --- a/src/main/java/org/opensearch/sdk/ExtensionRestHandler.java +++ b/src/main/java/org/opensearch/sdk/ExtensionRestHandler.java @@ -9,6 +9,7 @@ import java.util.List; +import org.opensearch.extensions.rest.ExtensionRestRequest; import org.opensearch.rest.BaseRestHandler; import org.opensearch.rest.RestHandler; import org.opensearch.rest.RestHandler.Route; @@ -29,12 +30,12 @@ public interface ExtensionRestHandler { /** * Handles REST Requests forwarded from OpenSearch for a configured route on an extension. - * Parameters are components of the {@link RestRequest} received from a user. + * Parameter contains components of the {@link RestRequest} received from a user. * This method corresponds to the {@link BaseRestHandler#prepareRequest} method. * As in that method, consumed parameters must be tracked and returned in the response. * - * @param restRequest a REST request object for a request to be forwarded to extensions + * @param request a REST request object for a request to be forwarded to extensions * @return An {@link ExtensionRestResponse} to the request. */ - ExtensionRestResponse handleRequest(ExtensionRestRequest restRequest); + ExtensionRestResponse handleRequest(ExtensionRestRequest request); } diff --git a/src/main/java/org/opensearch/sdk/ExtensionsRunner.java b/src/main/java/org/opensearch/sdk/ExtensionsRunner.java index 6ad1d1e9..6853f38e 100644 --- a/src/main/java/org/opensearch/sdk/ExtensionsRunner.java +++ b/src/main/java/org/opensearch/sdk/ExtensionsRunner.java @@ -15,11 +15,13 @@ import org.opensearch.Version; import org.opensearch.cluster.ClusterModule; import org.opensearch.cluster.node.DiscoveryNode; +import org.opensearch.common.bytes.BytesReference; import org.opensearch.common.io.stream.NamedWriteableRegistry; import org.opensearch.common.io.stream.NamedWriteableRegistryParseRequest; import org.opensearch.extensions.OpenSearchRequest; +import org.opensearch.extensions.rest.ExtensionRestRequest; import org.opensearch.extensions.rest.RegisterRestActionsRequest; -import org.opensearch.extensions.rest.RestExecuteOnExtensionRequest; +import org.opensearch.extensions.rest.RestExecuteOnExtensionResponse; import org.opensearch.extensions.settings.RegisterCustomSettingsRequest; import org.opensearch.common.network.NetworkModule; import org.opensearch.common.network.NetworkService; @@ -27,7 +29,9 @@ import org.opensearch.common.settings.Settings; import org.opensearch.common.util.PageCacheRecycler; import org.opensearch.discovery.InitializeExtensionsRequest; +import org.opensearch.discovery.InitializeExtensionsResponse; import org.opensearch.extensions.ExtensionActionListenerOnFailureRequest; +import org.opensearch.extensions.ExtensionBooleanResponse; import org.opensearch.extensions.DiscoveryExtension; import org.opensearch.extensions.EnvironmentSettingsRequest; import org.opensearch.extensions.AddSettingsUpdateConsumerRequest; @@ -35,10 +39,13 @@ import org.opensearch.extensions.ExtensionRequest; import org.opensearch.extensions.ExtensionsOrchestrator; import org.opensearch.index.IndicesModuleRequest; +import org.opensearch.index.IndicesModuleResponse; import org.opensearch.indices.IndicesModule; import org.opensearch.indices.breaker.CircuitBreakerService; import org.opensearch.indices.breaker.NoneCircuitBreakerService; import org.opensearch.rest.RestHandler.Route; +import org.opensearch.rest.RestResponse; +import org.opensearch.rest.RestStatus; import org.opensearch.transport.netty4.Netty4Transport; import org.opensearch.transport.SharedGroupFactory; import org.opensearch.sdk.handlers.ActionListenerOnFailureResponseHandler; @@ -59,6 +66,7 @@ import org.opensearch.transport.ClusterConnectionManager; import org.opensearch.transport.ConnectionManager; import org.opensearch.transport.TransportInterceptor; +import org.opensearch.transport.TransportResponse; import org.opensearch.transport.TransportService; import org.opensearch.transport.TransportSettings; @@ -80,7 +88,8 @@ /** * The primary class to run an extension. *

- * This class Javadoc will eventually be expanded with a full description/tutorial for users. + * This class Javadoc will eventually be expanded with a full + * description/tutorial for users. */ public class ExtensionsRunner { @@ -136,7 +145,8 @@ public class ExtensionsRunner { /** * Instantiates a new Extensions Runner using test settings. * - * @throws IOException if the runner failed to read settings or API. + * @throws IOException + * if the runner failed to read settings or API. */ public ExtensionsRunner() throws IOException { ExtensionSettings extensionSettings = readExtensionSettings(); @@ -151,8 +161,10 @@ public ExtensionsRunner() throws IOException { /** * Instantiates a new Extensions Runner using the specified extension. * - * @param extension The settings with which to start the runner. - * @throws IOException if the runner failed to read settings or API. + * @param extension + * The settings with which to start the runner. + * @throws IOException + * if the runner failed to read settings or API. */ private ExtensionsRunner(Extension extension) throws IOException { ExtensionSettings extensionSettings = extension.getExtensionSettings(); @@ -214,10 +226,128 @@ DiscoveryNode getOpensearchNode() { } /** +<<<<<<< HEAD * Initializes a Netty4Transport object. This object will be wrapped in a {@link TransportService} object. +======= + * Handles a extension request from OpenSearch. This is the first request for + * the transport communication and will initialize the extension and will be a + * part of OpenSearch bootstrap. * - * @param settings The transport settings to configure. - * @param threadPool A thread pool to use. + * @param extensionInitRequest + * The request to handle. + * @return A response to OpenSearch validating that this is an extension. + */ + InitializeExtensionsResponse handleExtensionInitRequest(InitializeExtensionsRequest extensionInitRequest) { + logger.info("Registering Extension Request received from OpenSearch"); + opensearchNode = extensionInitRequest.getSourceNode(); + setUniqueId(extensionInitRequest.getExtension().getId()); + // Successfully initialized. Send the response. + try { + return new InitializeExtensionsResponse(settings.get(NODE_NAME_SETTING)); + } finally { + // After sending successful response to initialization, send the REST API and + // Settings + setOpensearchNode(opensearchNode); + setExtensionNode(extensionInitRequest.getExtension()); + extensionTransportService.connectToNode(opensearchNode); + sendRegisterRestActionsRequest(extensionTransportService); + sendRegisterCustomSettingsRequest(extensionTransportService); + transportActions.sendRegisterTransportActionsRequest(extensionTransportService, opensearchNode); + } + } + + /** + * Handles a request from OpenSearch and invokes the extension point API + * corresponding with the request type + * + * @param request + * The request to handle. + * @return A response to OpenSearch for the corresponding API + * @throws Exception + * if the corresponding handler for the request is not present + */ + TransportResponse handleOpenSearchRequest(OpenSearchRequest request) throws Exception { + // Read enum + switch (request.getRequestType()) { + case REQUEST_OPENSEARCH_NAMED_WRITEABLE_REGISTRY: + return namedWriteableRegistryApi.handleNamedWriteableRegistryRequest(request); + // Add additional request handlers here + default: + throw new Exception("Handler not present for the provided request"); + } + } + + /** + * Handles a request for extension point indices from OpenSearch. The + * {@link #handleExtensionInitRequest(InitializeExtensionsRequest)} method must + * have been called first to initialize the extension. + * + * @param indicesModuleRequest + * The request to handle. + * @param transportService + * The transport service communicating with OpenSearch. + * @return A response to OpenSearch with this extension's index and search + * listeners. + */ + IndicesModuleResponse handleIndicesModuleRequest(IndicesModuleRequest indicesModuleRequest, TransportService transportService) { + logger.info("Registering Indices Module Request received from OpenSearch"); + IndicesModuleResponse indicesModuleResponse = new IndicesModuleResponse(true, true, true); + return indicesModuleResponse; + } + + /** + * Handles a request for extension name from OpenSearch. The + * {@link #handleExtensionInitRequest(InitializeExtensionsRequest)} method must + * have been called first to initialize the extension. + * + * @param indicesModuleRequest + * The request to handle. + * @return A response acknowledging the request. + */ + ExtensionBooleanResponse handleIndicesModuleNameRequest(IndicesModuleRequest indicesModuleRequest) { + // Works as beforeIndexRemoved + logger.info("Registering Indices Module Name Request received from OpenSearch"); + ExtensionBooleanResponse indicesModuleNameResponse = new ExtensionBooleanResponse(true); + return indicesModuleNameResponse; + } + + /** + * Handles a request from OpenSearch to execute a REST request on the extension. + * + * @param request + * The REST request to execute. + * @return A response acknowledging the request. + */ + RestExecuteOnExtensionResponse handleRestExecuteOnExtensionRequest(ExtensionRestRequest request) { + + ExtensionRestHandler restHandler = extensionRestPathRegistry.getHandler(request.method(), request.uri()); + if (restHandler == null) { + return new RestExecuteOnExtensionResponse( + RestStatus.NOT_FOUND, + "No handler for " + ExtensionRestPathRegistry.restPathToString(request.method(), request.uri()) + ); + } + + // Get response from extension + RestResponse response = restHandler.handleRequest(request); + logger.info("Sending extension response to OpenSearch: " + response.status()); + return new RestExecuteOnExtensionResponse( + response.status(), + response.contentType(), + BytesReference.toBytes(response.content()), + response.getHeaders() + ); + } + + /** + * Initializes a Netty4Transport object. This object will be wrapped in a + * {@link TransportService} object. +>>>>>>> 60694f1 (Rename/merge duplicate ExtensionRestRequest implementations) + * + * @param settings + * The transport settings to configure. + * @param threadPool + * A thread pool to use. * @return The configured Netty4Transport object. */ public Netty4Transport getNetty4Transport(Settings settings, ThreadPool threadPool) { @@ -253,9 +383,11 @@ public Netty4Transport getNetty4Transport(Settings settings, ThreadPool threadPo } /** - * Initializes the TransportService object for this extension. This object will control communication between the extension and OpenSearch. + * Initializes the TransportService object for this extension. This object will + * control communication between the extension and OpenSearch. * - * @param settings The transport settings to configure. + * @param settings + * The transport settings to configure. * @return The initialized TransportService object. */ public TransportService initializeExtensionTransportService(Settings settings) { @@ -293,7 +425,8 @@ public TransportService initializeExtensionTransportService(Settings settings) { /** * Starts a TransportService. * - * @param transportService The TransportService to start. + * @param transportService + * The TransportService to start. */ public void startTransportService(TransportService transportService) { // start transport service and accept incoming requests @@ -301,7 +434,8 @@ public void startTransportService(TransportService transportService) { transportService.acceptIncomingRequests(); // Extension Request is the first request for the transport communication. - // This request will initialize the extension and will be a part of OpenSearch bootstrap + // This request will initialize the extension and will be a part of OpenSearch + // bootstrap transportService.registerRequestHandler( ExtensionsOrchestrator.REQUEST_EXTENSION_ACTION_NAME, ThreadPool.Names.GENERIC, @@ -357,7 +491,7 @@ public void startTransportService(TransportService transportService) { ThreadPool.Names.GENERIC, false, false, - RestExecuteOnExtensionRequest::new, + ExtensionRestRequest::new, ((request, channel, task) -> channel.sendResponse(extensionsRestRequestHandler.handleRestExecuteOnExtensionRequest(request))) ); @@ -375,7 +509,8 @@ public void startTransportService(TransportService transportService) { /** * Requests that OpenSearch register the REST Actions for this extension. * - * @param transportService The TransportService defining the connection to OpenSearch. + * @param transportService + * The TransportService defining the connection to OpenSearch. */ public void sendRegisterRestActionsRequest(TransportService transportService) { List extensionRestPaths = extensionRestPathRegistry.getRegisteredPaths(); @@ -396,7 +531,8 @@ public void sendRegisterRestActionsRequest(TransportService transportService) { /** * Requests that OpenSearch register the custom settings for this extension. * - * @param transportService The TransportService defining the connection to OpenSearch. + * @param transportService + * The TransportService defining the connection to OpenSearch. */ public void sendRegisterCustomSettingsRequest(TransportService transportService) { logger.info("Sending Settings request to OpenSearch"); @@ -414,9 +550,11 @@ public void sendRegisterCustomSettingsRequest(TransportService transportService) } /** - * Requests the cluster state from OpenSearch. The result will be handled by a {@link ClusterStateResponseHandler}. + * Requests the cluster state from OpenSearch. The result will be handled by a + * {@link ClusterStateResponseHandler}. * - * @param transportService The TransportService defining the connection to OpenSearch. + * @param transportService + * The TransportService defining the connection to OpenSearch. */ public void sendClusterStateRequest(TransportService transportService) { logger.info("Sending Cluster State request to OpenSearch"); @@ -434,9 +572,11 @@ public void sendClusterStateRequest(TransportService transportService) { } /** - * Requests the cluster settings from OpenSearch. The result will be handled by a {@link ClusterSettingsResponseHandler}. + * Requests the cluster settings from OpenSearch. The result will be handled by + * a {@link ClusterSettingsResponseHandler}. * - * @param transportService The TransportService defining the connection to OpenSearch. + * @param transportService + * The TransportService defining the connection to OpenSearch. */ public void sendClusterSettingsRequest(TransportService transportService) { logger.info("Sending Cluster Settings request to OpenSearch"); @@ -454,9 +594,11 @@ public void sendClusterSettingsRequest(TransportService transportService) { } /** - * Requests the local node from OpenSearch. The result will be handled by a {@link LocalNodeResponseHandler}. + * Requests the local node from OpenSearch. The result will be handled by a + * {@link LocalNodeResponseHandler}. * - * @param transportService The TransportService defining the connection to OpenSearch. + * @param transportService + * The TransportService defining the connection to OpenSearch. */ public void sendLocalNodeRequest(TransportService transportService) { logger.info("Sending Local Node request to OpenSearch"); @@ -474,10 +616,13 @@ public void sendLocalNodeRequest(TransportService transportService) { } /** - * Requests the ActionListener onFailure method to be run by OpenSearch. The result will be handled by a {@link ActionListenerOnFailureResponseHandler}. + * Requests the ActionListener onFailure method to be run by OpenSearch. The + * result will be handled by a {@link ActionListenerOnFailureResponseHandler}. * - * @param transportService The TransportService defining the connection to OpenSearch. - * @param failureException The exception to be sent to OpenSearch + * @param transportService + * The TransportService defining the connection to OpenSearch. + * @param failureException + * The exception to be sent to OpenSearch */ public void sendActionListenerOnFailureRequest(TransportService transportService, Exception failureException) { logger.info("Sending ActionListener onFailure request to OpenSearch"); @@ -495,10 +640,15 @@ public void sendActionListenerOnFailureRequest(TransportService transportService } /** - * Requests the environment setting values from OpenSearch for the corresponding component settings. The result will be handled by a {@link EnvironmentSettingsResponseHandler}. + * Requests the environment setting values from OpenSearch for the corresponding + * component settings. The result will be handled by a + * {@link EnvironmentSettingsResponseHandler}. * - * @param componentSettings The component setting that correspond to the values provided by the environment settings - * @param transportService The TransportService defining the connection to OpenSearch. + * @param componentSettings + * The component setting that correspond to the values provided by + * the environment settings + * @param transportService + * The TransportService defining the connection to OpenSearch. */ public void sendEnvironmentSettingsRequest(TransportService transportService, List> componentSettings) { logger.info("Sending Environment Settings request to OpenSearch"); @@ -516,12 +666,18 @@ public void sendEnvironmentSettingsRequest(TransportService transportService, Li } /** - * Registers settings and setting consumers with the {@link UpdateSettingsRequestHandler} and then sends a request to OpenSearch to register these Setting objects with a callback to this extension. - * The result will be handled by a {@link ExtensionBooleanResponseHandler}. + * Registers settings and setting consumers with the + * {@link UpdateSettingsRequestHandler} and then sends a request to OpenSearch + * to register these Setting objects with a callback to this extension. The + * result will be handled by a {@link ExtensionBooleanResponseHandler}. * - * @param transportService The TransportService defining the connection to OpenSearch. - * @param settingUpdateConsumers A map of setting objects and their corresponding consumers - * @throws Exception if there are no setting update consumers within the settingUpdateConsumers map + * @param transportService + * The TransportService defining the connection to OpenSearch. + * @param settingUpdateConsumers + * A map of setting objects and their corresponding consumers + * @throws Exception + * if there are no setting update consumers within the + * settingUpdateConsumers map */ public void sendAddSettingsUpdateConsumerRequest(TransportService transportService, Map, Consumer> settingUpdateConsumers) throws Exception { @@ -561,7 +717,9 @@ private Settings getSettings() { /** * Starts an ActionListener. * - * @param timeout The timeout for the listener in milliseconds. A timeout of 0 means no timeout. + * @param timeout + * The timeout for the listener in milliseconds. A timeout of 0 means + * no timeout. */ public void startActionListener(int timeout) { final ActionListener actionListener = new ActionListener(); @@ -571,8 +729,10 @@ public void startActionListener(int timeout) { /** * Runs the specified extension. * - * @param extension The extension to run. - * @throws IOException on failure to bind ports. + * @param extension + * The extension to run. + * @throws IOException + * on failure to bind ports. */ public static void run(Extension extension) throws IOException { logger.info("Starting extension " + extension.getExtensionSettings().getExtensionName()); @@ -581,10 +741,13 @@ public static void run(Extension extension) throws IOException { } /** - * Run the Extension. For internal/testing purposes only. Imports settings and sets up Transport Service listening for incoming connections. + * Run the Extension. For internal/testing purposes only. Imports settings and + * sets up Transport Service listening for incoming connections. * - * @param args Unused - * @throws IOException if the runner failed to connect to the OpenSearch cluster. + * @param args + * Unused + * @throws IOException + * if the runner failed to connect to the OpenSearch cluster. */ public static void main(String[] args) throws IOException { diff --git a/src/main/java/org/opensearch/sdk/handlers/ExtensionsRestRequestHandler.java b/src/main/java/org/opensearch/sdk/handlers/ExtensionsRestRequestHandler.java index 5dbe0792..fcc277ec 100644 --- a/src/main/java/org/opensearch/sdk/handlers/ExtensionsRestRequestHandler.java +++ b/src/main/java/org/opensearch/sdk/handlers/ExtensionsRestRequestHandler.java @@ -10,7 +10,6 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.opensearch.common.bytes.BytesReference; -import org.opensearch.extensions.rest.RestExecuteOnExtensionRequest; import org.opensearch.extensions.rest.RestExecuteOnExtensionResponse; import org.opensearch.rest.RestStatus; import org.opensearch.sdk.ExtensionRestHandler; @@ -33,24 +32,18 @@ public class ExtensionsRestRequestHandler { * @param request The REST request to execute. * @return A response acknowledging the request. */ - public RestExecuteOnExtensionResponse handleRestExecuteOnExtensionRequest(RestExecuteOnExtensionRequest request) { + public RestExecuteOnExtensionResponse handleRestExecuteOnExtensionRequest(ExtensionRestRequest request) { - ExtensionRestHandler restHandler = extensionRestPathRegistry.getHandler(request.getMethod(), request.getUri()); + ExtensionRestHandler restHandler = extensionRestPathRegistry.getHandler(request.method(), request.uri()); if (restHandler == null) { return new RestExecuteOnExtensionResponse( RestStatus.NOT_FOUND, - "No handler for " + ExtensionRestPathRegistry.restPathToString(request.getMethod(), request.getUri()) + "No handler for " + ExtensionRestPathRegistry.restPathToString(request.method(), request.uri()) ); } - // ExtensionRestRequest restRequest = new ExtensionRestRequest(request); - ExtensionRestRequest restRequest = new ExtensionRestRequest( - request.getMethod(), - request.getUri(), - request.getRequestIssuerIdentity() - ); // Get response from extension - ExtensionRestResponse response = restHandler.handleRequest(restRequest); + ExtensionRestResponse response = restHandler.handleRequest(request); logger.info("Sending extension response to OpenSearch: " + response.status()); return new RestExecuteOnExtensionResponse( response.status(), diff --git a/src/main/java/org/opensearch/sdk/sample/helloworld/rest/RestHelloAction.java b/src/main/java/org/opensearch/sdk/sample/helloworld/rest/RestHelloAction.java index 185fcff0..8d0d34f6 100644 --- a/src/main/java/org/opensearch/sdk/sample/helloworld/rest/RestHelloAction.java +++ b/src/main/java/org/opensearch/sdk/sample/helloworld/rest/RestHelloAction.java @@ -7,10 +7,10 @@ */ package org.opensearch.sdk.sample.helloworld.rest; +import org.opensearch.extensions.rest.ExtensionRestRequest; import org.opensearch.rest.RestHandler.Route; import org.opensearch.rest.RestRequest.Method; import org.opensearch.sdk.ExtensionRestHandler; -import org.opensearch.sdk.ExtensionRestRequest; import org.opensearch.sdk.ExtensionRestResponse; import java.net.URLDecoder; diff --git a/src/test/java/org/opensearch/sdk/TestExtensionRestPathRegistry.java b/src/test/java/org/opensearch/sdk/TestExtensionRestPathRegistry.java index 8a0a2cb7..d6f2ae38 100644 --- a/src/test/java/org/opensearch/sdk/TestExtensionRestPathRegistry.java +++ b/src/test/java/org/opensearch/sdk/TestExtensionRestPathRegistry.java @@ -4,6 +4,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.opensearch.extensions.rest.ExtensionRestRequest; import org.opensearch.rest.RestHandler.Route; import org.opensearch.rest.RestRequest.Method; import org.opensearch.test.OpenSearchTestCase; diff --git a/src/test/java/org/opensearch/sdk/TestExtensionRestRequest.java b/src/test/java/org/opensearch/sdk/TestExtensionRestRequest.java deleted file mode 100644 index 0187dd62..00000000 --- a/src/test/java/org/opensearch/sdk/TestExtensionRestRequest.java +++ /dev/null @@ -1,57 +0,0 @@ -package org.opensearch.sdk; - -import org.junit.jupiter.api.Test; -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.NamedWriteableAwareStreamInput; -import org.opensearch.common.io.stream.NamedWriteableRegistry; -import org.opensearch.identity.ExtensionTokenProcessor; -import org.opensearch.identity.PrincipalIdentifierToken; -import org.opensearch.rest.RestRequest; -import org.opensearch.test.OpenSearchTestCase; - -import java.security.Principal; - -public class TestExtensionRestRequest extends OpenSearchTestCase { - - @Test - public void testExtensionRestRequest() throws Exception { - RestRequest.Method expectedMethod = RestRequest.Method.GET; - String expectedUri = "/test/uri"; - String extensionUniqueId1 = "ext_1"; - Principal userPrincipal = () -> "user1"; - ExtensionTokenProcessor extensionTokenProcessor = new ExtensionTokenProcessor(extensionUniqueId1); - PrincipalIdentifierToken expectedRequestIssuerIdentity = extensionTokenProcessor.generateToken(userPrincipal); - NamedWriteableRegistry registry = new NamedWriteableRegistry( - org.opensearch.common.collect.List.of( - new NamedWriteableRegistry.Entry( - PrincipalIdentifierToken.class, - PrincipalIdentifierToken.NAME, - PrincipalIdentifierToken::new - ) - ) - ); - - ExtensionRestRequest request = new ExtensionRestRequest(expectedMethod, expectedUri, expectedRequestIssuerIdentity); - - assertEquals(expectedMethod, request.method()); - assertEquals(expectedUri, request.uri()); - assertEquals(expectedRequestIssuerIdentity, request.getRequestIssuerIdentity()); - - try (BytesStreamOutput out = new BytesStreamOutput()) { - request.writeTo(out); - out.flush(); - try (BytesStreamInput in = new BytesStreamInput(BytesReference.toBytes(out.bytes()))) { - try (NamedWriteableAwareStreamInput nameWritableAwareIn = new NamedWriteableAwareStreamInput(in, registry)) { - request = new ExtensionRestRequest(nameWritableAwareIn); - } - - assertEquals(expectedMethod, request.method()); - assertEquals(expectedUri, request.uri()); - assertEquals(expectedRequestIssuerIdentity, request.getRequestIssuerIdentity()); - } - } - } - -} diff --git a/src/test/java/org/opensearch/sdk/TestExtensionsRunner.java b/src/test/java/org/opensearch/sdk/TestExtensionsRunner.java index 54688a88..1b4f329f 100644 --- a/src/test/java/org/opensearch/sdk/TestExtensionsRunner.java +++ b/src/test/java/org/opensearch/sdk/TestExtensionsRunner.java @@ -35,6 +35,7 @@ import org.junit.jupiter.api.Test; import org.opensearch.Version; import org.opensearch.cluster.node.DiscoveryNode; +import org.opensearch.common.bytes.BytesArray; import org.opensearch.common.io.stream.NamedWriteableRegistryResponse; import org.opensearch.common.settings.Settings; import org.opensearch.common.settings.Setting.Property; @@ -45,7 +46,7 @@ import org.opensearch.extensions.ExtensionBooleanResponse; import org.opensearch.extensions.ExtensionsOrchestrator.OpenSearchRequestType; import org.opensearch.extensions.OpenSearchRequest; -import org.opensearch.extensions.rest.RestExecuteOnExtensionRequest; +import org.opensearch.extensions.rest.ExtensionRestRequest; import org.opensearch.extensions.rest.RestExecuteOnExtensionResponse; import org.opensearch.identity.ExtensionTokenProcessor; import org.opensearch.rest.BytesRestResponse; @@ -72,8 +73,6 @@ public class TestExtensionsRunner extends OpenSearchTestCase { private static final String EXTENSION_NAME = "sample-extension"; private ExtensionsInitRequestHandler extensionsInitRequestHandler = new ExtensionsInitRequestHandler(); private OpensearchRequestHandler opensearchRequestHandler = new OpensearchRequestHandler(); - private ExtensionsRestRequestHandler extensionsRestRequestHandler = new ExtensionsRestRequestHandler(); - private ExtensionsRunner extensionsRunner; private TransportService transportService; @@ -168,12 +167,19 @@ public void testHandleOpenSearchRequest() throws Exception { } @Test - public void testHandleRestExecuteOnExtensionRequest() throws Exception { + public void testHandleExtensionRestRequest() throws Exception { ExtensionTokenProcessor ext = new ExtensionTokenProcessor(EXTENSION_NAME); Principal userPrincipal = () -> "user1"; - RestExecuteOnExtensionRequest request = new RestExecuteOnExtensionRequest(Method.GET, "/foo", ext.generateToken(userPrincipal)); - RestExecuteOnExtensionResponse response = extensionsRestRequestHandler.handleRestExecuteOnExtensionRequest(request); + ExtensionRestRequest request = new ExtensionRestRequest( + Method.GET, + "/foo", + Collections.emptyMap(), + null, + new BytesArray(""), + ext.generateToken(userPrincipal) + ); + RestExecuteOnExtensionResponse response = extensionsRunner.handleRestExecuteOnExtensionRequest(request); // this will fail in test environment with no registered actions assertEquals(RestStatus.NOT_FOUND, response.getStatus()); assertEquals(BytesRestResponse.TEXT_CONTENT_TYPE, response.getContentType()); diff --git a/src/test/java/org/opensearch/sdk/sample/helloworld/rest/TestRestHelloAction.java b/src/test/java/org/opensearch/sdk/sample/helloworld/rest/TestRestHelloAction.java index 42a366b4..c7b0b151 100644 --- a/src/test/java/org/opensearch/sdk/sample/helloworld/rest/TestRestHelloAction.java +++ b/src/test/java/org/opensearch/sdk/sample/helloworld/rest/TestRestHelloAction.java @@ -9,7 +9,9 @@ import java.nio.charset.StandardCharsets; import java.security.Principal; +import java.util.Collections; import java.util.List; +import java.util.Map; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -18,11 +20,11 @@ import org.opensearch.rest.RestHandler.Route; import org.opensearch.rest.RestRequest.Method; import org.opensearch.common.bytes.BytesReference; +import org.opensearch.extensions.rest.ExtensionRestRequest; import org.opensearch.rest.BytesRestResponse; import org.opensearch.rest.RestResponse; import org.opensearch.rest.RestStatus; import org.opensearch.sdk.ExtensionRestHandler; -import org.opensearch.sdk.ExtensionRestRequest; import org.opensearch.test.OpenSearchTestCase; public class TestRestHelloAction extends OpenSearchTestCase { @@ -52,12 +54,13 @@ public void testHandleRequest() { Principal userPrincipal = () -> "user1"; ExtensionTokenProcessor extensionTokenProcessor = new ExtensionTokenProcessor(EXTENSION_NAME); PrincipalIdentifierToken token = extensionTokenProcessor.generateToken(userPrincipal); + Map params = Collections.emptyMap(); - ExtensionRestRequest getRequest = new ExtensionRestRequest(Method.GET, "/hello", token); - ExtensionRestRequest putRequest = new ExtensionRestRequest(Method.PUT, "/hello", token); - ExtensionRestRequest updateRequest = new ExtensionRestRequest(Method.PUT, "/hello/Passing+Test", token); - ExtensionRestRequest badRequest = new ExtensionRestRequest(Method.PUT, "/hello/Bad%Request", token); - ExtensionRestRequest unsuccessfulRequest = new ExtensionRestRequest(Method.GET, "/goodbye", token); + ExtensionRestRequest getRequest = new ExtensionRestRequest(Method.GET, "/hello", params, token); + ExtensionRestRequest putRequest = new ExtensionRestRequest(Method.PUT, "/hello", params, token); + ExtensionRestRequest updateRequest = new ExtensionRestRequest(Method.PUT, "/hello/Passing+Test", params, token); + ExtensionRestRequest badRequest = new ExtensionRestRequest(Method.PUT, "/hello/Bad%Request", params, token); + ExtensionRestRequest unsuccessfulRequest = new ExtensionRestRequest(Method.GET, "/goodbye", params, token); RestResponse response = restHelloAction.handleRequest(getRequest); assertEquals(RestStatus.OK, response.status()); From 16854bd067a466b59fe9015fa5454d9481bf383d Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Wed, 28 Sep 2022 21:12:11 -0700 Subject: [PATCH 2/8] Track consumed params inside request object Signed-off-by: Daniel Widdis --- .../opensearch/sdk/ExtensionRestResponse.java | 31 +++++++------ .../org/opensearch/sdk/ExtensionsRunner.java | 2 +- .../helloworld/rest/RestHelloAction.java | 46 +++++++++---------- .../sdk/TestExtensionRestResponse.java | 32 +++++++------ .../helloworld/rest/TestRestHelloAction.java | 23 +++------- 5 files changed, 66 insertions(+), 68 deletions(-) diff --git a/src/main/java/org/opensearch/sdk/ExtensionRestResponse.java b/src/main/java/org/opensearch/sdk/ExtensionRestResponse.java index c623c3ec..984f8270 100644 --- a/src/main/java/org/opensearch/sdk/ExtensionRestResponse.java +++ b/src/main/java/org/opensearch/sdk/ExtensionRestResponse.java @@ -11,6 +11,7 @@ import org.opensearch.common.bytes.BytesReference; import org.opensearch.common.xcontent.XContentBuilder; +import org.opensearch.extensions.rest.ExtensionRestRequest; import org.opensearch.rest.BytesRestResponse; import org.opensearch.rest.RestStatus; @@ -27,64 +28,64 @@ public class ExtensionRestResponse extends BytesRestResponse { /** * Creates a new response based on {@link XContentBuilder}. * + * @param request the REST request being responded to. * @param status The REST status. * @param builder The builder for the response. - * @param consumedParams Parameters consumed by the handler. */ - public ExtensionRestResponse(RestStatus status, XContentBuilder builder, List consumedParams) { + public ExtensionRestResponse(ExtensionRestRequest request, RestStatus status, XContentBuilder builder) { super(status, builder); - addConsumedParamHeader(consumedParams); + addConsumedParamHeader(request.consumedParams()); } /** * Creates a new plain text response. * + * @param request the REST request being responded to. * @param status The REST status. * @param content A plain text response string. - * @param consumedParams Parameters consumed by the handler. */ - public ExtensionRestResponse(RestStatus status, String content, List consumedParams) { + public ExtensionRestResponse(ExtensionRestRequest request, RestStatus status, String content) { super(status, content); - addConsumedParamHeader(consumedParams); + addConsumedParamHeader(request.consumedParams()); } /** * Creates a new plain text response. * + * @param request the REST request being responded to. * @param status The REST status. * @param contentType The content type of the response string. * @param content A response string. - * @param consumedParams Parameters consumed by the handler. */ - public ExtensionRestResponse(RestStatus status, String contentType, String content, List consumedParams) { + public ExtensionRestResponse(ExtensionRestRequest request, RestStatus status, String contentType, String content) { super(status, contentType, content); - addConsumedParamHeader(consumedParams); + addConsumedParamHeader(request.consumedParams()); } /** * Creates a binary response. * + * @param request the REST request being responded to. * @param status The REST status. * @param contentType The content type of the response bytes. * @param content Response bytes. - * @param consumedParams Parameters consumed by the handler. */ - public ExtensionRestResponse(RestStatus status, String contentType, byte[] content, List consumedParams) { + public ExtensionRestResponse(ExtensionRestRequest request, RestStatus status, String contentType, byte[] content) { super(status, contentType, content); - addConsumedParamHeader(consumedParams); + addConsumedParamHeader(request.consumedParams()); } /** * Creates a binary response. * + * @param request the REST request being responded to. * @param status The REST status. * @param contentType The content type of the response bytes. * @param content Response bytes. - * @param consumedParams Parameters consumed by the handler. */ - public ExtensionRestResponse(RestStatus status, String contentType, BytesReference content, List consumedParams) { + public ExtensionRestResponse(ExtensionRestRequest request, RestStatus status, String contentType, BytesReference content) { super(status, contentType, content); - addConsumedParamHeader(consumedParams); + addConsumedParamHeader(request.consumedParams()); } private void addConsumedParamHeader(List consumedParams) { diff --git a/src/main/java/org/opensearch/sdk/ExtensionsRunner.java b/src/main/java/org/opensearch/sdk/ExtensionsRunner.java index 6853f38e..e83d12c0 100644 --- a/src/main/java/org/opensearch/sdk/ExtensionsRunner.java +++ b/src/main/java/org/opensearch/sdk/ExtensionsRunner.java @@ -327,7 +327,7 @@ RestExecuteOnExtensionResponse handleRestExecuteOnExtensionRequest(ExtensionRest "No handler for " + ExtensionRestPathRegistry.restPathToString(request.method(), request.uri()) ); } - + logger.trace("Handling " + request.toString()); // Get response from extension RestResponse response = restHandler.handleRequest(request); logger.info("Sending extension response to OpenSearch: " + response.status()); diff --git a/src/main/java/org/opensearch/sdk/sample/helloworld/rest/RestHelloAction.java b/src/main/java/org/opensearch/sdk/sample/helloworld/rest/RestHelloAction.java index 8d0d34f6..4e5ce476 100644 --- a/src/main/java/org/opensearch/sdk/sample/helloworld/rest/RestHelloAction.java +++ b/src/main/java/org/opensearch/sdk/sample/helloworld/rest/RestHelloAction.java @@ -15,7 +15,6 @@ import java.net.URLDecoder; import java.nio.charset.StandardCharsets; -import java.util.ArrayList; import java.util.List; import static org.opensearch.rest.RestRequest.Method.GET; @@ -39,31 +38,32 @@ public List routes() { @Override public ExtensionRestResponse handleRequest(ExtensionRestRequest request) { - // We need to track which parameters are consumed to pass back to OpenSearch - List consumedParams = new ArrayList<>(); Method method = request.method(); - String uri = request.uri(); - if (Method.GET.equals(method) && "/hello".equals(uri)) { - return new ExtensionRestResponse(OK, String.format(GREETING, worldName), consumedParams); - } else if (Method.PUT.equals(method) && uri.startsWith("/hello/")) { - // Placeholder code here for parameters in named wildcard paths - // Full implementation based on params() will be implemented as part of - // https://github.com/opensearch-project/opensearch-sdk-java/issues/111 - String name = uri.substring("/hello/".length()); - consumedParams.add("name"); - try { - worldName = URLDecoder.decode(name, StandardCharsets.UTF_8); - } catch (IllegalArgumentException e) { - return new ExtensionRestResponse(BAD_REQUEST, e.getMessage(), consumedParams); - } - return new ExtensionRestResponse(OK, "Updated the world's name to " + worldName, consumedParams); + if (Method.GET.equals(method)) { + return handleGetRequest(request); + } else if (Method.PUT.equals(method)) { + return handlePutRequest(request); } - return new ExtensionRestResponse( - NOT_FOUND, - "Extension REST action improperly configured to handle " + method.name() + " " + uri, - consumedParams - ); + return handleBadRequest(request); + } + + private ExtensionRestResponse handleGetRequest(ExtensionRestRequest request) { + return new ExtensionRestResponse(request, OK, String.format(GREETING, worldName)); + } + + private ExtensionRestResponse handlePutRequest(ExtensionRestRequest request) { + String name = request.param("name"); + try { + worldName = URLDecoder.decode(name, StandardCharsets.UTF_8); + } catch (IllegalArgumentException e) { + return new ExtensionRestResponse(request, BAD_REQUEST, e.getMessage()); + } + return new ExtensionRestResponse(request, OK, "Updated the world's name to " + worldName); + } + + private ExtensionRestResponse handleBadRequest(ExtensionRestRequest request) { + return new ExtensionRestResponse(request, NOT_FOUND, "Extension REST action improperly configured to handle " + request.toString()); } } diff --git a/src/test/java/org/opensearch/sdk/TestExtensionRestResponse.java b/src/test/java/org/opensearch/sdk/TestExtensionRestResponse.java index e11aed5e..63ba6693 100644 --- a/src/test/java/org/opensearch/sdk/TestExtensionRestResponse.java +++ b/src/test/java/org/opensearch/sdk/TestExtensionRestResponse.java @@ -2,6 +2,7 @@ import java.io.IOException; import java.nio.ByteBuffer; +import java.util.Collections; import java.util.List; import org.junit.jupiter.api.BeforeEach; @@ -9,6 +10,8 @@ import org.opensearch.common.bytes.BytesReference; import org.opensearch.common.xcontent.XContentBuilder; import org.opensearch.common.xcontent.XContentType; +import org.opensearch.extensions.rest.ExtensionRestRequest; +import org.opensearch.rest.RestRequest.Method; import org.opensearch.test.OpenSearchTestCase; import static org.opensearch.rest.BytesRestResponse.TEXT_CONTENT_TYPE; @@ -23,7 +26,7 @@ public class TestExtensionRestResponse extends OpenSearchTestCase { private String testText; private byte[] testBytes; - private List testConsumedParams; + private ExtensionRestRequest request; @Override @BeforeEach @@ -31,7 +34,10 @@ public void setUp() throws Exception { super.setUp(); testText = "plain text"; testBytes = new byte[] { 1, 2 }; - testConsumedParams = List.of("foo", "bar"); + request = new ExtensionRestRequest(Method.GET, "/foo", Collections.emptyMap(), null); + // consume params "foo" and "bar" + request.param("foo"); + request.param("bar"); } @Test @@ -40,33 +46,33 @@ public void testConstructorWithBuilder() throws IOException { builder.startObject(); builder.field("status", ACCEPTED); builder.endObject(); - ExtensionRestResponse response = new ExtensionRestResponse(OK, builder, testConsumedParams); + ExtensionRestResponse response = new ExtensionRestResponse(request, OK, builder); assertEquals(OK, response.status()); assertEquals(JSON_CONTENT_TYPE, response.contentType()); assertEquals("{\"status\":\"ACCEPTED\"}", response.content().utf8ToString()); List consumedParams = response.getHeaders().get(CONSUMED_PARAMS_KEY); for (String param : consumedParams) { - assertTrue(testConsumedParams.contains(param)); + assertTrue(request.consumedParams().contains(param)); } } @Test public void testConstructorWithPlainText() { - ExtensionRestResponse response = new ExtensionRestResponse(OK, testText, testConsumedParams); + ExtensionRestResponse response = new ExtensionRestResponse(request, OK, testText); assertEquals(OK, response.status()); assertEquals(TEXT_CONTENT_TYPE, response.contentType()); assertEquals(testText, response.content().utf8ToString()); List consumedParams = response.getHeaders().get(CONSUMED_PARAMS_KEY); for (String param : consumedParams) { - assertTrue(testConsumedParams.contains(param)); + assertTrue(request.consumedParams().contains(param)); } } @Test public void testConstructorWithText() { - ExtensionRestResponse response = new ExtensionRestResponse(OK, TEXT_CONTENT_TYPE, testText, testConsumedParams); + ExtensionRestResponse response = new ExtensionRestResponse(request, OK, TEXT_CONTENT_TYPE, testText); assertEquals(OK, response.status()); assertEquals(TEXT_CONTENT_TYPE, response.contentType()); @@ -74,30 +80,30 @@ public void testConstructorWithText() { List consumedParams = response.getHeaders().get(CONSUMED_PARAMS_KEY); for (String param : consumedParams) { - assertTrue(testConsumedParams.contains(param)); + assertTrue(request.consumedParams().contains(param)); } } @Test public void testConstructorWithByteArray() { - ExtensionRestResponse response = new ExtensionRestResponse(OK, OCTET_CONTENT_TYPE, testBytes, testConsumedParams); + ExtensionRestResponse response = new ExtensionRestResponse(request, OK, OCTET_CONTENT_TYPE, testBytes); assertEquals(OK, response.status()); assertEquals(OCTET_CONTENT_TYPE, response.contentType()); assertArrayEquals(testBytes, BytesReference.toBytes(response.content())); List consumedParams = response.getHeaders().get(CONSUMED_PARAMS_KEY); for (String param : consumedParams) { - assertTrue(testConsumedParams.contains(param)); + assertTrue(request.consumedParams().contains(param)); } } @Test public void testConstructorWithBytesReference() { ExtensionRestResponse response = new ExtensionRestResponse( + request, OK, OCTET_CONTENT_TYPE, - BytesReference.fromByteBuffer(ByteBuffer.wrap(testBytes, 0, 2)), - testConsumedParams + BytesReference.fromByteBuffer(ByteBuffer.wrap(testBytes, 0, 2)) ); assertEquals(OK, response.status()); @@ -105,7 +111,7 @@ public void testConstructorWithBytesReference() { assertArrayEquals(testBytes, BytesReference.toBytes(response.content())); List consumedParams = response.getHeaders().get(CONSUMED_PARAMS_KEY); for (String param : consumedParams) { - assertTrue(testConsumedParams.contains(param)); + assertTrue(request.consumedParams().contains(param)); } } } diff --git a/src/test/java/org/opensearch/sdk/sample/helloworld/rest/TestRestHelloAction.java b/src/test/java/org/opensearch/sdk/sample/helloworld/rest/TestRestHelloAction.java index c7b0b151..1a2847db 100644 --- a/src/test/java/org/opensearch/sdk/sample/helloworld/rest/TestRestHelloAction.java +++ b/src/test/java/org/opensearch/sdk/sample/helloworld/rest/TestRestHelloAction.java @@ -57,10 +57,13 @@ public void testHandleRequest() { Map params = Collections.emptyMap(); ExtensionRestRequest getRequest = new ExtensionRestRequest(Method.GET, "/hello", params, token); - ExtensionRestRequest putRequest = new ExtensionRestRequest(Method.PUT, "/hello", params, token); - ExtensionRestRequest updateRequest = new ExtensionRestRequest(Method.PUT, "/hello/Passing+Test", params, token); - ExtensionRestRequest badRequest = new ExtensionRestRequest(Method.PUT, "/hello/Bad%Request", params, token); - ExtensionRestRequest unsuccessfulRequest = new ExtensionRestRequest(Method.GET, "/goodbye", params, token); + ExtensionRestRequest putRequest = new ExtensionRestRequest( + Method.PUT, + "/hello/Passing+Test", + Map.of("name", "Passing+Test"), + token + ); + ExtensionRestRequest badRequest = new ExtensionRestRequest(Method.PUT, "/hello/Bad%Request", Map.of("name", "Bad%Request"), token); RestResponse response = restHelloAction.handleRequest(getRequest); assertEquals(RestStatus.OK, response.status()); @@ -69,12 +72,6 @@ public void testHandleRequest() { assertEquals("Hello, World!", responseStr); response = restHelloAction.handleRequest(putRequest); - assertEquals(RestStatus.NOT_FOUND, response.status()); - assertEquals(BytesRestResponse.TEXT_CONTENT_TYPE, response.contentType()); - responseStr = new String(BytesReference.toBytes(response.content()), StandardCharsets.UTF_8); - assertTrue(responseStr.contains("PUT")); - - response = restHelloAction.handleRequest(updateRequest); assertEquals(RestStatus.OK, response.status()); assertEquals(BytesRestResponse.TEXT_CONTENT_TYPE, response.contentType()); responseStr = new String(BytesReference.toBytes(response.content()), StandardCharsets.UTF_8); @@ -91,12 +88,6 @@ public void testHandleRequest() { assertEquals(BytesRestResponse.TEXT_CONTENT_TYPE, response.contentType()); responseStr = new String(BytesReference.toBytes(response.content()), StandardCharsets.UTF_8); assertTrue(responseStr.contains("Illegal hex characters in escape (%) pattern")); - - response = restHelloAction.handleRequest(unsuccessfulRequest); - assertEquals(RestStatus.NOT_FOUND, response.status()); - assertEquals(BytesRestResponse.TEXT_CONTENT_TYPE, response.contentType()); - responseStr = new String(BytesReference.toBytes(response.content()), StandardCharsets.UTF_8); - assertTrue(responseStr.contains("/goodbye")); } } From 61efa08351bc7f0503fb0c6e0bcabde46c7f15bc Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Wed, 28 Sep 2022 23:08:52 -0700 Subject: [PATCH 3/8] Put back test for the "this will never happen" case Signed-off-by: Daniel Widdis --- .../sdk/sample/helloworld/rest/TestRestHelloAction.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/test/java/org/opensearch/sdk/sample/helloworld/rest/TestRestHelloAction.java b/src/test/java/org/opensearch/sdk/sample/helloworld/rest/TestRestHelloAction.java index 1a2847db..000544ae 100644 --- a/src/test/java/org/opensearch/sdk/sample/helloworld/rest/TestRestHelloAction.java +++ b/src/test/java/org/opensearch/sdk/sample/helloworld/rest/TestRestHelloAction.java @@ -64,6 +64,7 @@ public void testHandleRequest() { token ); ExtensionRestRequest badRequest = new ExtensionRestRequest(Method.PUT, "/hello/Bad%Request", Map.of("name", "Bad%Request"), token); + ExtensionRestRequest unsuccessfulRequest = new ExtensionRestRequest(Method.POST, "/goodbye", params, token); RestResponse response = restHelloAction.handleRequest(getRequest); assertEquals(RestStatus.OK, response.status()); @@ -88,6 +89,12 @@ public void testHandleRequest() { assertEquals(BytesRestResponse.TEXT_CONTENT_TYPE, response.contentType()); responseStr = new String(BytesReference.toBytes(response.content()), StandardCharsets.UTF_8); assertTrue(responseStr.contains("Illegal hex characters in escape (%) pattern")); + + response = restHelloAction.handleRequest(unsuccessfulRequest); + assertEquals(RestStatus.NOT_FOUND, response.status()); + assertEquals(BytesRestResponse.TEXT_CONTENT_TYPE, response.contentType()); + responseStr = new String(BytesReference.toBytes(response.content()), StandardCharsets.UTF_8); + assertTrue(responseStr.contains("/goodbye")); } } From f29c8930f5e2be4f8745dc293be1e42e9e961b28 Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Thu, 29 Sep 2022 19:31:41 -0700 Subject: [PATCH 4/8] Add content and expand param getters Signed-off-by: Daniel Widdis --- .../opensearch/sdk/ExtensionRestResponse.java | 17 +++++++++----- .../sdk/TestExtensionRestResponse.java | 10 +++++++- .../opensearch/sdk/TestExtensionsRunner.java | 2 +- .../helloworld/rest/TestRestHelloAction.java | 23 ++++++++++++++++--- 4 files changed, 41 insertions(+), 11 deletions(-) diff --git a/src/main/java/org/opensearch/sdk/ExtensionRestResponse.java b/src/main/java/org/opensearch/sdk/ExtensionRestResponse.java index 984f8270..2b915cf3 100644 --- a/src/main/java/org/opensearch/sdk/ExtensionRestResponse.java +++ b/src/main/java/org/opensearch/sdk/ExtensionRestResponse.java @@ -24,6 +24,10 @@ public class ExtensionRestResponse extends BytesRestResponse { * Key passed in {@link BytesRestResponse} headers to identify parameters consumed by the handler. For internal use. */ static final String CONSUMED_PARAMS_KEY = "extension.consumed.parameters"; + /** + * Key passed in {@link BytesRestResponse} headers to identify content consumed by the handler. For internal use. + */ + static final String CONSUMED_CONTENT_KEY = "extension.consumed.content"; /** * Creates a new response based on {@link XContentBuilder}. @@ -34,7 +38,7 @@ public class ExtensionRestResponse extends BytesRestResponse { */ public ExtensionRestResponse(ExtensionRestRequest request, RestStatus status, XContentBuilder builder) { super(status, builder); - addConsumedParamHeader(request.consumedParams()); + addConsumedHeaders(request.consumedParams(), request.isContentConsumed()); } /** @@ -46,7 +50,7 @@ public ExtensionRestResponse(ExtensionRestRequest request, RestStatus status, XC */ public ExtensionRestResponse(ExtensionRestRequest request, RestStatus status, String content) { super(status, content); - addConsumedParamHeader(request.consumedParams()); + addConsumedHeaders(request.consumedParams(), request.isContentConsumed()); } /** @@ -59,7 +63,7 @@ public ExtensionRestResponse(ExtensionRestRequest request, RestStatus status, St */ public ExtensionRestResponse(ExtensionRestRequest request, RestStatus status, String contentType, String content) { super(status, contentType, content); - addConsumedParamHeader(request.consumedParams()); + addConsumedHeaders(request.consumedParams(), request.isContentConsumed()); } /** @@ -72,7 +76,7 @@ public ExtensionRestResponse(ExtensionRestRequest request, RestStatus status, St */ public ExtensionRestResponse(ExtensionRestRequest request, RestStatus status, String contentType, byte[] content) { super(status, contentType, content); - addConsumedParamHeader(request.consumedParams()); + addConsumedHeaders(request.consumedParams(), request.isContentConsumed()); } /** @@ -85,10 +89,11 @@ public ExtensionRestResponse(ExtensionRestRequest request, RestStatus status, St */ public ExtensionRestResponse(ExtensionRestRequest request, RestStatus status, String contentType, BytesReference content) { super(status, contentType, content); - addConsumedParamHeader(request.consumedParams()); + addConsumedHeaders(request.consumedParams(), request.isContentConsumed()); } - private void addConsumedParamHeader(List consumedParams) { + private void addConsumedHeaders(List consumedParams, boolean contentConusmed) { consumedParams.stream().forEach(p -> addHeader(CONSUMED_PARAMS_KEY, p)); + addHeader(CONSUMED_CONTENT_KEY, Boolean.toString(contentConusmed)); } } diff --git a/src/test/java/org/opensearch/sdk/TestExtensionRestResponse.java b/src/test/java/org/opensearch/sdk/TestExtensionRestResponse.java index 63ba6693..4bd982da 100644 --- a/src/test/java/org/opensearch/sdk/TestExtensionRestResponse.java +++ b/src/test/java/org/opensearch/sdk/TestExtensionRestResponse.java @@ -7,6 +7,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.opensearch.common.bytes.BytesArray; import org.opensearch.common.bytes.BytesReference; import org.opensearch.common.xcontent.XContentBuilder; import org.opensearch.common.xcontent.XContentType; @@ -34,10 +35,12 @@ public void setUp() throws Exception { super.setUp(); testText = "plain text"; testBytes = new byte[] { 1, 2 }; - request = new ExtensionRestRequest(Method.GET, "/foo", Collections.emptyMap(), null); + request = new ExtensionRestRequest(Method.GET, "/foo", Collections.emptyMap(), null, new BytesArray("Text Content"), null); // consume params "foo" and "bar" request.param("foo"); request.param("bar"); + // consume content + request.content(); } @Test @@ -55,6 +58,7 @@ public void testConstructorWithBuilder() throws IOException { for (String param : consumedParams) { assertTrue(request.consumedParams().contains(param)); } + assertTrue(request.isContentConsumed()); } @Test @@ -68,6 +72,7 @@ public void testConstructorWithPlainText() { for (String param : consumedParams) { assertTrue(request.consumedParams().contains(param)); } + assertTrue(request.isContentConsumed()); } @Test @@ -82,6 +87,7 @@ public void testConstructorWithText() { for (String param : consumedParams) { assertTrue(request.consumedParams().contains(param)); } + assertTrue(request.isContentConsumed()); } @Test @@ -95,6 +101,7 @@ public void testConstructorWithByteArray() { for (String param : consumedParams) { assertTrue(request.consumedParams().contains(param)); } + assertTrue(request.isContentConsumed()); } @Test @@ -113,5 +120,6 @@ public void testConstructorWithBytesReference() { for (String param : consumedParams) { assertTrue(request.consumedParams().contains(param)); } + assertTrue(request.isContentConsumed()); } } diff --git a/src/test/java/org/opensearch/sdk/TestExtensionsRunner.java b/src/test/java/org/opensearch/sdk/TestExtensionsRunner.java index 1b4f329f..db5a9b9c 100644 --- a/src/test/java/org/opensearch/sdk/TestExtensionsRunner.java +++ b/src/test/java/org/opensearch/sdk/TestExtensionsRunner.java @@ -176,7 +176,7 @@ public void testHandleExtensionRestRequest() throws Exception { "/foo", Collections.emptyMap(), null, - new BytesArray(""), + new BytesArray("bar"), ext.generateToken(userPrincipal) ); RestExecuteOnExtensionResponse response = extensionsRunner.handleRestExecuteOnExtensionRequest(request); diff --git a/src/test/java/org/opensearch/sdk/sample/helloworld/rest/TestRestHelloAction.java b/src/test/java/org/opensearch/sdk/sample/helloworld/rest/TestRestHelloAction.java index 000544ae..cfe04a18 100644 --- a/src/test/java/org/opensearch/sdk/sample/helloworld/rest/TestRestHelloAction.java +++ b/src/test/java/org/opensearch/sdk/sample/helloworld/rest/TestRestHelloAction.java @@ -19,6 +19,7 @@ import org.opensearch.identity.PrincipalIdentifierToken; import org.opensearch.rest.RestHandler.Route; import org.opensearch.rest.RestRequest.Method; +import org.opensearch.common.bytes.BytesArray; import org.opensearch.common.bytes.BytesReference; import org.opensearch.extensions.rest.ExtensionRestRequest; import org.opensearch.rest.BytesRestResponse; @@ -56,15 +57,31 @@ public void testHandleRequest() { PrincipalIdentifierToken token = extensionTokenProcessor.generateToken(userPrincipal); Map params = Collections.emptyMap(); - ExtensionRestRequest getRequest = new ExtensionRestRequest(Method.GET, "/hello", params, token); + ExtensionRestRequest getRequest = new ExtensionRestRequest(Method.GET, "/hello", params, null, new BytesArray(""), token); ExtensionRestRequest putRequest = new ExtensionRestRequest( Method.PUT, "/hello/Passing+Test", Map.of("name", "Passing+Test"), + null, + new BytesArray(""), + token + ); + ExtensionRestRequest badRequest = new ExtensionRestRequest( + Method.PUT, + "/hello/Bad%Request", + Map.of("name", "Bad%Request"), + null, + new BytesArray(""), + token + ); + ExtensionRestRequest unsuccessfulRequest = new ExtensionRestRequest( + Method.POST, + "/goodbye", + params, + null, + new BytesArray(""), token ); - ExtensionRestRequest badRequest = new ExtensionRestRequest(Method.PUT, "/hello/Bad%Request", Map.of("name", "Bad%Request"), token); - ExtensionRestRequest unsuccessfulRequest = new ExtensionRestRequest(Method.POST, "/goodbye", params, token); RestResponse response = restHelloAction.handleRequest(getRequest); assertEquals(RestStatus.OK, response.status()); From 89eee75cd88054116011637006b4f16bb66f4d67 Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Thu, 29 Sep 2022 20:58:19 -0700 Subject: [PATCH 5/8] Rebase to main and update after conficts Signed-off-by: Daniel Widdis --- .../opensearch/sdk/ExtensionRestRequest.java | 121 ------------------ .../org/opensearch/sdk/ExtensionsRunner.java | 5 +- .../ExtensionsRestRequestHandler.java | 2 +- .../opensearch/sdk/TestExtensionsRunner.java | 1 - 4 files changed, 2 insertions(+), 127 deletions(-) delete mode 100644 src/main/java/org/opensearch/sdk/ExtensionRestRequest.java diff --git a/src/main/java/org/opensearch/sdk/ExtensionRestRequest.java b/src/main/java/org/opensearch/sdk/ExtensionRestRequest.java deleted file mode 100644 index 5f170a59..00000000 --- a/src/main/java/org/opensearch/sdk/ExtensionRestRequest.java +++ /dev/null @@ -1,121 +0,0 @@ -/* - * 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.opensearch.common.io.stream.StreamInput; -import org.opensearch.common.io.stream.StreamOutput; -import org.opensearch.extensions.rest.RestExecuteOnExtensionRequest; -import org.opensearch.identity.PrincipalIdentifierToken; -import org.opensearch.rest.RestRequest.Method; -import org.opensearch.transport.TransportRequest; - -import java.io.IOException; -import java.util.Objects; - -/** - * A subclass of {@link TransportRequest} which contains request relevant information - * to be utilised in ExtensionRestHandler implementation - */ -public class ExtensionRestRequest extends TransportRequest { - private Method method; - private String uri; - /** - * The owner of this request object - */ - private PrincipalIdentifierToken principalIdentifierToken; - - /** - * This object can be instantiated given method, uri and identifier - * @param method of type {@link Method} - * @param uri url string - * @param principalIdentifier the owner of this request - */ - public ExtensionRestRequest(Method method, String uri, PrincipalIdentifierToken principalIdentifier) { - this.method = method; - this.uri = uri; - this.principalIdentifierToken = principalIdentifier; - } - - /** - * The object to be created from rest request object incoming from OpenSearch - * @param request incoming object from OpenSearch - * @throws IllegalArgumentException when request is null - */ - protected ExtensionRestRequest(RestExecuteOnExtensionRequest request) throws IllegalArgumentException { - if (request == null) throw new IllegalArgumentException("Request object can't be null"); - this.method = request.getMethod(); - this.uri = request.getUri(); - this.principalIdentifierToken = request.getRequestIssuerIdentity(); - } - - /** - * Object generated from input stream - * @param in Input stream - * @throws IOException if there's an error in generating object from input stream - */ - public ExtensionRestRequest(StreamInput in) throws IOException { - super(in); - method = in.readEnum(Method.class); - uri = in.readString(); - principalIdentifierToken = in.readNamedWriteable(PrincipalIdentifierToken.class); - } - - /** - * Write this object to output stream - * @param out the writeable output stream - * @throws IOException if there's an error in generating object from output stream - */ - @Override - public void writeTo(StreamOutput out) throws IOException { - super.writeTo(out); - out.writeEnum(method); - out.writeString(uri); - out.writeNamedWriteable(principalIdentifierToken); - } - - /** - * @return This REST request {@link Method} type - */ - public Method method() { - return method; - } - - /** - * @return This REST request's uri - */ - public String uri() { - return uri; - } - - /** - * @return This REST request issuer's identity token - */ - public PrincipalIdentifierToken getRequestIssuerIdentity() { - return principalIdentifierToken; - } - - @Override - public String toString() { - return "ExtensionRestRequest{method=" + method + ", uri=" + uri + ", requester = " + principalIdentifierToken.getToken() + "}"; - } - - @Override - public boolean equals(Object obj) { - if (this == obj) return true; - if (obj == null || getClass() != obj.getClass()) return false; - ExtensionRestRequest that = (ExtensionRestRequest) obj; - return Objects.equals(method, that.method) - && Objects.equals(uri, that.uri) - && Objects.equals(principalIdentifierToken, that.principalIdentifierToken); - } - - @Override - public int hashCode() { - return Objects.hash(method, uri, principalIdentifierToken); - } -} diff --git a/src/main/java/org/opensearch/sdk/ExtensionsRunner.java b/src/main/java/org/opensearch/sdk/ExtensionsRunner.java index e83d12c0..8e4d2711 100644 --- a/src/main/java/org/opensearch/sdk/ExtensionsRunner.java +++ b/src/main/java/org/opensearch/sdk/ExtensionsRunner.java @@ -226,9 +226,6 @@ DiscoveryNode getOpensearchNode() { } /** -<<<<<<< HEAD - * Initializes a Netty4Transport object. This object will be wrapped in a {@link TransportService} object. -======= * Handles a extension request from OpenSearch. This is the first request for * the transport communication and will initialize the extension and will be a * part of OpenSearch bootstrap. @@ -342,7 +339,7 @@ RestExecuteOnExtensionResponse handleRestExecuteOnExtensionRequest(ExtensionRest /** * Initializes a Netty4Transport object. This object will be wrapped in a * {@link TransportService} object. ->>>>>>> 60694f1 (Rename/merge duplicate ExtensionRestRequest implementations) + >>>>>>> 60694f1 (Rename/merge duplicate ExtensionRestRequest implementations) * * @param settings * The transport settings to configure. diff --git a/src/main/java/org/opensearch/sdk/handlers/ExtensionsRestRequestHandler.java b/src/main/java/org/opensearch/sdk/handlers/ExtensionsRestRequestHandler.java index fcc277ec..351c8248 100644 --- a/src/main/java/org/opensearch/sdk/handlers/ExtensionsRestRequestHandler.java +++ b/src/main/java/org/opensearch/sdk/handlers/ExtensionsRestRequestHandler.java @@ -10,12 +10,12 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.opensearch.common.bytes.BytesReference; +import org.opensearch.extensions.rest.ExtensionRestRequest; import org.opensearch.extensions.rest.RestExecuteOnExtensionResponse; import org.opensearch.rest.RestStatus; import org.opensearch.sdk.ExtensionRestHandler; import org.opensearch.sdk.ExtensionsRunner; import org.opensearch.sdk.ExtensionRestPathRegistry; -import org.opensearch.sdk.ExtensionRestRequest; import org.opensearch.sdk.ExtensionRestResponse; /** diff --git a/src/test/java/org/opensearch/sdk/TestExtensionsRunner.java b/src/test/java/org/opensearch/sdk/TestExtensionsRunner.java index db5a9b9c..cfa9de14 100644 --- a/src/test/java/org/opensearch/sdk/TestExtensionsRunner.java +++ b/src/test/java/org/opensearch/sdk/TestExtensionsRunner.java @@ -59,7 +59,6 @@ import org.opensearch.sdk.handlers.ClusterStateResponseHandler; import org.opensearch.sdk.handlers.EnvironmentSettingsResponseHandler; import org.opensearch.sdk.handlers.ExtensionsInitRequestHandler; -import org.opensearch.sdk.handlers.ExtensionsRestRequestHandler; import org.opensearch.sdk.handlers.LocalNodeResponseHandler; import org.opensearch.sdk.handlers.ExtensionStringResponseHandler; import org.opensearch.sdk.handlers.OpensearchRequestHandler; From ffa6b38d7d79f21cb40bb2386f1f838180e68d91 Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Thu, 29 Sep 2022 21:15:04 -0700 Subject: [PATCH 6/8] More merge conflict cleanup Signed-off-by: Daniel Widdis --- .../org/opensearch/sdk/ExtensionsRunner.java | 189 ++---------------- .../opensearch/sdk/TestExtensionsRunner.java | 4 +- 2 files changed, 25 insertions(+), 168 deletions(-) diff --git a/src/main/java/org/opensearch/sdk/ExtensionsRunner.java b/src/main/java/org/opensearch/sdk/ExtensionsRunner.java index 8e4d2711..ec3fb01c 100644 --- a/src/main/java/org/opensearch/sdk/ExtensionsRunner.java +++ b/src/main/java/org/opensearch/sdk/ExtensionsRunner.java @@ -15,13 +15,11 @@ import org.opensearch.Version; import org.opensearch.cluster.ClusterModule; import org.opensearch.cluster.node.DiscoveryNode; -import org.opensearch.common.bytes.BytesReference; import org.opensearch.common.io.stream.NamedWriteableRegistry; import org.opensearch.common.io.stream.NamedWriteableRegistryParseRequest; import org.opensearch.extensions.OpenSearchRequest; import org.opensearch.extensions.rest.ExtensionRestRequest; import org.opensearch.extensions.rest.RegisterRestActionsRequest; -import org.opensearch.extensions.rest.RestExecuteOnExtensionResponse; import org.opensearch.extensions.settings.RegisterCustomSettingsRequest; import org.opensearch.common.network.NetworkModule; import org.opensearch.common.network.NetworkService; @@ -29,9 +27,7 @@ import org.opensearch.common.settings.Settings; import org.opensearch.common.util.PageCacheRecycler; import org.opensearch.discovery.InitializeExtensionsRequest; -import org.opensearch.discovery.InitializeExtensionsResponse; import org.opensearch.extensions.ExtensionActionListenerOnFailureRequest; -import org.opensearch.extensions.ExtensionBooleanResponse; import org.opensearch.extensions.DiscoveryExtension; import org.opensearch.extensions.EnvironmentSettingsRequest; import org.opensearch.extensions.AddSettingsUpdateConsumerRequest; @@ -39,13 +35,10 @@ import org.opensearch.extensions.ExtensionRequest; import org.opensearch.extensions.ExtensionsOrchestrator; import org.opensearch.index.IndicesModuleRequest; -import org.opensearch.index.IndicesModuleResponse; import org.opensearch.indices.IndicesModule; import org.opensearch.indices.breaker.CircuitBreakerService; import org.opensearch.indices.breaker.NoneCircuitBreakerService; import org.opensearch.rest.RestHandler.Route; -import org.opensearch.rest.RestResponse; -import org.opensearch.rest.RestStatus; import org.opensearch.transport.netty4.Netty4Transport; import org.opensearch.transport.SharedGroupFactory; import org.opensearch.sdk.handlers.ActionListenerOnFailureResponseHandler; @@ -66,7 +59,6 @@ import org.opensearch.transport.ClusterConnectionManager; import org.opensearch.transport.ConnectionManager; import org.opensearch.transport.TransportInterceptor; -import org.opensearch.transport.TransportResponse; import org.opensearch.transport.TransportService; import org.opensearch.transport.TransportSettings; @@ -145,8 +137,7 @@ public class ExtensionsRunner { /** * Instantiates a new Extensions Runner using test settings. * - * @throws IOException - * if the runner failed to read settings or API. + * @throws IOException if the runner failed to read settings or API. */ public ExtensionsRunner() throws IOException { ExtensionSettings extensionSettings = readExtensionSettings(); @@ -161,10 +152,8 @@ public ExtensionsRunner() throws IOException { /** * Instantiates a new Extensions Runner using the specified extension. * - * @param extension - * The settings with which to start the runner. - * @throws IOException - * if the runner failed to read settings or API. + * @param extension The settings with which to start the runner. + * @throws IOException if the runner failed to read settings or API. */ private ExtensionsRunner(Extension extension) throws IOException { ExtensionSettings extensionSettings = extension.getExtensionSettings(); @@ -225,121 +214,9 @@ DiscoveryNode getOpensearchNode() { return opensearchNode; } - /** - * Handles a extension request from OpenSearch. This is the first request for - * the transport communication and will initialize the extension and will be a - * part of OpenSearch bootstrap. - * - * @param extensionInitRequest - * The request to handle. - * @return A response to OpenSearch validating that this is an extension. - */ - InitializeExtensionsResponse handleExtensionInitRequest(InitializeExtensionsRequest extensionInitRequest) { - logger.info("Registering Extension Request received from OpenSearch"); - opensearchNode = extensionInitRequest.getSourceNode(); - setUniqueId(extensionInitRequest.getExtension().getId()); - // Successfully initialized. Send the response. - try { - return new InitializeExtensionsResponse(settings.get(NODE_NAME_SETTING)); - } finally { - // After sending successful response to initialization, send the REST API and - // Settings - setOpensearchNode(opensearchNode); - setExtensionNode(extensionInitRequest.getExtension()); - extensionTransportService.connectToNode(opensearchNode); - sendRegisterRestActionsRequest(extensionTransportService); - sendRegisterCustomSettingsRequest(extensionTransportService); - transportActions.sendRegisterTransportActionsRequest(extensionTransportService, opensearchNode); - } - } - - /** - * Handles a request from OpenSearch and invokes the extension point API - * corresponding with the request type - * - * @param request - * The request to handle. - * @return A response to OpenSearch for the corresponding API - * @throws Exception - * if the corresponding handler for the request is not present - */ - TransportResponse handleOpenSearchRequest(OpenSearchRequest request) throws Exception { - // Read enum - switch (request.getRequestType()) { - case REQUEST_OPENSEARCH_NAMED_WRITEABLE_REGISTRY: - return namedWriteableRegistryApi.handleNamedWriteableRegistryRequest(request); - // Add additional request handlers here - default: - throw new Exception("Handler not present for the provided request"); - } - } - - /** - * Handles a request for extension point indices from OpenSearch. The - * {@link #handleExtensionInitRequest(InitializeExtensionsRequest)} method must - * have been called first to initialize the extension. - * - * @param indicesModuleRequest - * The request to handle. - * @param transportService - * The transport service communicating with OpenSearch. - * @return A response to OpenSearch with this extension's index and search - * listeners. - */ - IndicesModuleResponse handleIndicesModuleRequest(IndicesModuleRequest indicesModuleRequest, TransportService transportService) { - logger.info("Registering Indices Module Request received from OpenSearch"); - IndicesModuleResponse indicesModuleResponse = new IndicesModuleResponse(true, true, true); - return indicesModuleResponse; - } - - /** - * Handles a request for extension name from OpenSearch. The - * {@link #handleExtensionInitRequest(InitializeExtensionsRequest)} method must - * have been called first to initialize the extension. - * - * @param indicesModuleRequest - * The request to handle. - * @return A response acknowledging the request. - */ - ExtensionBooleanResponse handleIndicesModuleNameRequest(IndicesModuleRequest indicesModuleRequest) { - // Works as beforeIndexRemoved - logger.info("Registering Indices Module Name Request received from OpenSearch"); - ExtensionBooleanResponse indicesModuleNameResponse = new ExtensionBooleanResponse(true); - return indicesModuleNameResponse; - } - - /** - * Handles a request from OpenSearch to execute a REST request on the extension. - * - * @param request - * The REST request to execute. - * @return A response acknowledging the request. - */ - RestExecuteOnExtensionResponse handleRestExecuteOnExtensionRequest(ExtensionRestRequest request) { - - ExtensionRestHandler restHandler = extensionRestPathRegistry.getHandler(request.method(), request.uri()); - if (restHandler == null) { - return new RestExecuteOnExtensionResponse( - RestStatus.NOT_FOUND, - "No handler for " + ExtensionRestPathRegistry.restPathToString(request.method(), request.uri()) - ); - } - logger.trace("Handling " + request.toString()); - // Get response from extension - RestResponse response = restHandler.handleRequest(request); - logger.info("Sending extension response to OpenSearch: " + response.status()); - return new RestExecuteOnExtensionResponse( - response.status(), - response.contentType(), - BytesReference.toBytes(response.content()), - response.getHeaders() - ); - } - /** * Initializes a Netty4Transport object. This object will be wrapped in a * {@link TransportService} object. - >>>>>>> 60694f1 (Rename/merge duplicate ExtensionRestRequest implementations) * * @param settings * The transport settings to configure. @@ -383,8 +260,7 @@ public Netty4Transport getNetty4Transport(Settings settings, ThreadPool threadPo * Initializes the TransportService object for this extension. This object will * control communication between the extension and OpenSearch. * - * @param settings - * The transport settings to configure. + * @param settings The transport settings to configure. * @return The initialized TransportService object. */ public TransportService initializeExtensionTransportService(Settings settings) { @@ -422,8 +298,7 @@ public TransportService initializeExtensionTransportService(Settings settings) { /** * Starts a TransportService. * - * @param transportService - * The TransportService to start. + * @param transportService The TransportService to start. */ public void startTransportService(TransportService transportService) { // start transport service and accept incoming requests @@ -506,8 +381,7 @@ public void startTransportService(TransportService transportService) { /** * Requests that OpenSearch register the REST Actions for this extension. * - * @param transportService - * The TransportService defining the connection to OpenSearch. + * @param transportService The TransportService defining the connection to OpenSearch. */ public void sendRegisterRestActionsRequest(TransportService transportService) { List extensionRestPaths = extensionRestPathRegistry.getRegisteredPaths(); @@ -528,8 +402,7 @@ public void sendRegisterRestActionsRequest(TransportService transportService) { /** * Requests that OpenSearch register the custom settings for this extension. * - * @param transportService - * The TransportService defining the connection to OpenSearch. + * @param transportService The TransportService defining the connection to OpenSearch. */ public void sendRegisterCustomSettingsRequest(TransportService transportService) { logger.info("Sending Settings request to OpenSearch"); @@ -550,8 +423,7 @@ public void sendRegisterCustomSettingsRequest(TransportService transportService) * Requests the cluster state from OpenSearch. The result will be handled by a * {@link ClusterStateResponseHandler}. * - * @param transportService - * The TransportService defining the connection to OpenSearch. + * @param transportService The TransportService defining the connection to OpenSearch. */ public void sendClusterStateRequest(TransportService transportService) { logger.info("Sending Cluster State request to OpenSearch"); @@ -572,8 +444,7 @@ public void sendClusterStateRequest(TransportService transportService) { * Requests the cluster settings from OpenSearch. The result will be handled by * a {@link ClusterSettingsResponseHandler}. * - * @param transportService - * The TransportService defining the connection to OpenSearch. + * @param transportService The TransportService defining the connection to OpenSearch. */ public void sendClusterSettingsRequest(TransportService transportService) { logger.info("Sending Cluster Settings request to OpenSearch"); @@ -594,8 +465,7 @@ public void sendClusterSettingsRequest(TransportService transportService) { * Requests the local node from OpenSearch. The result will be handled by a * {@link LocalNodeResponseHandler}. * - * @param transportService - * The TransportService defining the connection to OpenSearch. + * @param transportService The TransportService defining the connection to OpenSearch. */ public void sendLocalNodeRequest(TransportService transportService) { logger.info("Sending Local Node request to OpenSearch"); @@ -616,10 +486,8 @@ public void sendLocalNodeRequest(TransportService transportService) { * Requests the ActionListener onFailure method to be run by OpenSearch. The * result will be handled by a {@link ActionListenerOnFailureResponseHandler}. * - * @param transportService - * The TransportService defining the connection to OpenSearch. - * @param failureException - * The exception to be sent to OpenSearch + * @param transportService The TransportService defining the connection to OpenSearch. + * @param failureException The exception to be sent to OpenSearch */ public void sendActionListenerOnFailureRequest(TransportService transportService, Exception failureException) { logger.info("Sending ActionListener onFailure request to OpenSearch"); @@ -641,11 +509,8 @@ public void sendActionListenerOnFailureRequest(TransportService transportService * component settings. The result will be handled by a * {@link EnvironmentSettingsResponseHandler}. * - * @param componentSettings - * The component setting that correspond to the values provided by - * the environment settings - * @param transportService - * The TransportService defining the connection to OpenSearch. + * @param componentSettings The component setting that correspond to the values provided by the environment settings + * @param transportService The TransportService defining the connection to OpenSearch. */ public void sendEnvironmentSettingsRequest(TransportService transportService, List> componentSettings) { logger.info("Sending Environment Settings request to OpenSearch"); @@ -668,13 +533,9 @@ public void sendEnvironmentSettingsRequest(TransportService transportService, Li * to register these Setting objects with a callback to this extension. The * result will be handled by a {@link ExtensionBooleanResponseHandler}. * - * @param transportService - * The TransportService defining the connection to OpenSearch. - * @param settingUpdateConsumers - * A map of setting objects and their corresponding consumers - * @throws Exception - * if there are no setting update consumers within the - * settingUpdateConsumers map + * @param transportService The TransportService defining the connection to OpenSearch. + * @param settingUpdateConsumers A map of setting objects and their corresponding consumers + * @throws Exception if there are no setting update consumers within the settingUpdateConsumers map */ public void sendAddSettingsUpdateConsumerRequest(TransportService transportService, Map, Consumer> settingUpdateConsumers) throws Exception { @@ -714,9 +575,7 @@ private Settings getSettings() { /** * Starts an ActionListener. * - * @param timeout - * The timeout for the listener in milliseconds. A timeout of 0 means - * no timeout. + * @param timeout The timeout for the listener in milliseconds. A timeout of 0 means no timeout. */ public void startActionListener(int timeout) { final ActionListener actionListener = new ActionListener(); @@ -726,10 +585,8 @@ public void startActionListener(int timeout) { /** * Runs the specified extension. * - * @param extension - * The extension to run. - * @throws IOException - * on failure to bind ports. + * @param extension The extension to run. + * @throws IOException on failure to bind ports. */ public static void run(Extension extension) throws IOException { logger.info("Starting extension " + extension.getExtensionSettings().getExtensionName()); @@ -741,10 +598,8 @@ public static void run(Extension extension) throws IOException { * Run the Extension. For internal/testing purposes only. Imports settings and * sets up Transport Service listening for incoming connections. * - * @param args - * Unused - * @throws IOException - * if the runner failed to connect to the OpenSearch cluster. + * @param args Unused + * @throws IOException if the runner failed to connect to the OpenSearch cluster. */ public static void main(String[] args) throws IOException { diff --git a/src/test/java/org/opensearch/sdk/TestExtensionsRunner.java b/src/test/java/org/opensearch/sdk/TestExtensionsRunner.java index cfa9de14..30d0ab80 100644 --- a/src/test/java/org/opensearch/sdk/TestExtensionsRunner.java +++ b/src/test/java/org/opensearch/sdk/TestExtensionsRunner.java @@ -59,6 +59,7 @@ import org.opensearch.sdk.handlers.ClusterStateResponseHandler; import org.opensearch.sdk.handlers.EnvironmentSettingsResponseHandler; import org.opensearch.sdk.handlers.ExtensionsInitRequestHandler; +import org.opensearch.sdk.handlers.ExtensionsRestRequestHandler; import org.opensearch.sdk.handlers.LocalNodeResponseHandler; import org.opensearch.sdk.handlers.ExtensionStringResponseHandler; import org.opensearch.sdk.handlers.OpensearchRequestHandler; @@ -72,6 +73,7 @@ public class TestExtensionsRunner extends OpenSearchTestCase { private static final String EXTENSION_NAME = "sample-extension"; private ExtensionsInitRequestHandler extensionsInitRequestHandler = new ExtensionsInitRequestHandler(); private OpensearchRequestHandler opensearchRequestHandler = new OpensearchRequestHandler(); + private ExtensionsRestRequestHandler extensionsRestRequestHandler = new ExtensionsRestRequestHandler(); private ExtensionsRunner extensionsRunner; private TransportService transportService; @@ -178,7 +180,7 @@ public void testHandleExtensionRestRequest() throws Exception { new BytesArray("bar"), ext.generateToken(userPrincipal) ); - RestExecuteOnExtensionResponse response = extensionsRunner.handleRestExecuteOnExtensionRequest(request); + RestExecuteOnExtensionResponse response = extensionsRestRequestHandler.handleRestExecuteOnExtensionRequest(request); // this will fail in test environment with no registered actions assertEquals(RestStatus.NOT_FOUND, response.getStatus()); assertEquals(BytesRestResponse.TEXT_CONTENT_TYPE, response.getContentType()); From a9033cb9a0ef22aabd3ecb76d3b9a029e5031a4a Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Thu, 29 Sep 2022 21:23:16 -0700 Subject: [PATCH 7/8] Whitespace diff reduction Signed-off-by: Daniel Widdis --- .../org/opensearch/sdk/ExtensionsRunner.java | 73 ++++++++----------- 1 file changed, 29 insertions(+), 44 deletions(-) diff --git a/src/main/java/org/opensearch/sdk/ExtensionsRunner.java b/src/main/java/org/opensearch/sdk/ExtensionsRunner.java index ec3fb01c..062deef2 100644 --- a/src/main/java/org/opensearch/sdk/ExtensionsRunner.java +++ b/src/main/java/org/opensearch/sdk/ExtensionsRunner.java @@ -80,8 +80,7 @@ /** * The primary class to run an extension. *

- * This class Javadoc will eventually be expanded with a full - * description/tutorial for users. + * This class Javadoc will eventually be expanded with a full description/tutorial for users. */ public class ExtensionsRunner { @@ -152,7 +151,7 @@ public ExtensionsRunner() throws IOException { /** * Instantiates a new Extensions Runner using the specified extension. * - * @param extension The settings with which to start the runner. + * @param extension The settings with which to start the runner. * @throws IOException if the runner failed to read settings or API. */ private ExtensionsRunner(Extension extension) throws IOException { @@ -215,13 +214,10 @@ DiscoveryNode getOpensearchNode() { } /** - * Initializes a Netty4Transport object. This object will be wrapped in a - * {@link TransportService} object. + * Initializes a Netty4Transport object. This object will be wrapped in a {@link TransportService} object. * - * @param settings - * The transport settings to configure. - * @param threadPool - * A thread pool to use. + * @param settings The transport settings to configure. + * @param threadPool A thread pool to use. * @return The configured Netty4Transport object. */ public Netty4Transport getNetty4Transport(Settings settings, ThreadPool threadPool) { @@ -257,10 +253,9 @@ public Netty4Transport getNetty4Transport(Settings settings, ThreadPool threadPo } /** - * Initializes the TransportService object for this extension. This object will - * control communication between the extension and OpenSearch. + * Initializes the TransportService object for this extension. This object will control communication between the extension and OpenSearch. * - * @param settings The transport settings to configure. + * @param settings The transport settings to configure. * @return The initialized TransportService object. */ public TransportService initializeExtensionTransportService(Settings settings) { @@ -298,7 +293,7 @@ public TransportService initializeExtensionTransportService(Settings settings) { /** * Starts a TransportService. * - * @param transportService The TransportService to start. + * @param transportService The TransportService to start. */ public void startTransportService(TransportService transportService) { // start transport service and accept incoming requests @@ -306,8 +301,7 @@ public void startTransportService(TransportService transportService) { transportService.acceptIncomingRequests(); // Extension Request is the first request for the transport communication. - // This request will initialize the extension and will be a part of OpenSearch - // bootstrap + // This request will initialize the extension and will be a part of OpenSearch bootstrap transportService.registerRequestHandler( ExtensionsOrchestrator.REQUEST_EXTENSION_ACTION_NAME, ThreadPool.Names.GENERIC, @@ -381,7 +375,7 @@ public void startTransportService(TransportService transportService) { /** * Requests that OpenSearch register the REST Actions for this extension. * - * @param transportService The TransportService defining the connection to OpenSearch. + * @param transportService The TransportService defining the connection to OpenSearch. */ public void sendRegisterRestActionsRequest(TransportService transportService) { List extensionRestPaths = extensionRestPathRegistry.getRegisteredPaths(); @@ -402,7 +396,7 @@ public void sendRegisterRestActionsRequest(TransportService transportService) { /** * Requests that OpenSearch register the custom settings for this extension. * - * @param transportService The TransportService defining the connection to OpenSearch. + * @param transportService The TransportService defining the connection to OpenSearch. */ public void sendRegisterCustomSettingsRequest(TransportService transportService) { logger.info("Sending Settings request to OpenSearch"); @@ -420,10 +414,9 @@ public void sendRegisterCustomSettingsRequest(TransportService transportService) } /** - * Requests the cluster state from OpenSearch. The result will be handled by a - * {@link ClusterStateResponseHandler}. + * Requests the cluster state from OpenSearch. The result will be handled by a {@link ClusterStateResponseHandler}. * - * @param transportService The TransportService defining the connection to OpenSearch. + * @param transportService The TransportService defining the connection to OpenSearch. */ public void sendClusterStateRequest(TransportService transportService) { logger.info("Sending Cluster State request to OpenSearch"); @@ -441,10 +434,9 @@ public void sendClusterStateRequest(TransportService transportService) { } /** - * Requests the cluster settings from OpenSearch. The result will be handled by - * a {@link ClusterSettingsResponseHandler}. + * Requests the cluster settings from OpenSearch. The result will be handled by a {@link ClusterSettingsResponseHandler}. * - * @param transportService The TransportService defining the connection to OpenSearch. + * @param transportService The TransportService defining the connection to OpenSearch. */ public void sendClusterSettingsRequest(TransportService transportService) { logger.info("Sending Cluster Settings request to OpenSearch"); @@ -462,10 +454,9 @@ public void sendClusterSettingsRequest(TransportService transportService) { } /** - * Requests the local node from OpenSearch. The result will be handled by a - * {@link LocalNodeResponseHandler}. + * Requests the local node from OpenSearch. The result will be handled by a {@link LocalNodeResponseHandler}. * - * @param transportService The TransportService defining the connection to OpenSearch. + * @param transportService The TransportService defining the connection to OpenSearch. */ public void sendLocalNodeRequest(TransportService transportService) { logger.info("Sending Local Node request to OpenSearch"); @@ -483,11 +474,10 @@ public void sendLocalNodeRequest(TransportService transportService) { } /** - * Requests the ActionListener onFailure method to be run by OpenSearch. The - * result will be handled by a {@link ActionListenerOnFailureResponseHandler}. + * Requests the ActionListener onFailure method to be run by OpenSearch. The result will be handled by a {@link ActionListenerOnFailureResponseHandler}. * - * @param transportService The TransportService defining the connection to OpenSearch. - * @param failureException The exception to be sent to OpenSearch + * @param transportService The TransportService defining the connection to OpenSearch. + * @param failureException The exception to be sent to OpenSearch */ public void sendActionListenerOnFailureRequest(TransportService transportService, Exception failureException) { logger.info("Sending ActionListener onFailure request to OpenSearch"); @@ -505,12 +495,10 @@ public void sendActionListenerOnFailureRequest(TransportService transportService } /** - * Requests the environment setting values from OpenSearch for the corresponding - * component settings. The result will be handled by a - * {@link EnvironmentSettingsResponseHandler}. + * Requests the environment setting values from OpenSearch for the corresponding component settings. The result will be handled by a {@link EnvironmentSettingsResponseHandler}. * * @param componentSettings The component setting that correspond to the values provided by the environment settings - * @param transportService The TransportService defining the connection to OpenSearch. + * @param transportService The TransportService defining the connection to OpenSearch. */ public void sendEnvironmentSettingsRequest(TransportService transportService, List> componentSettings) { logger.info("Sending Environment Settings request to OpenSearch"); @@ -528,12 +516,10 @@ public void sendEnvironmentSettingsRequest(TransportService transportService, Li } /** - * Registers settings and setting consumers with the - * {@link UpdateSettingsRequestHandler} and then sends a request to OpenSearch - * to register these Setting objects with a callback to this extension. The - * result will be handled by a {@link ExtensionBooleanResponseHandler}. + * Registers settings and setting consumers with the {@link UpdateSettingsRequestHandler} and then sends a request to OpenSearch to register these Setting objects with a callback to this extension. + * The result will be handled by a {@link ExtensionBooleanResponseHandler}. * - * @param transportService The TransportService defining the connection to OpenSearch. + * @param transportService The TransportService defining the connection to OpenSearch. * @param settingUpdateConsumers A map of setting objects and their corresponding consumers * @throws Exception if there are no setting update consumers within the settingUpdateConsumers map */ @@ -575,7 +561,7 @@ private Settings getSettings() { /** * Starts an ActionListener. * - * @param timeout The timeout for the listener in milliseconds. A timeout of 0 means no timeout. + * @param timeout The timeout for the listener in milliseconds. A timeout of 0 means no timeout. */ public void startActionListener(int timeout) { final ActionListener actionListener = new ActionListener(); @@ -585,8 +571,8 @@ public void startActionListener(int timeout) { /** * Runs the specified extension. * - * @param extension The extension to run. - * @throws IOException on failure to bind ports. + * @param extension The extension to run. + * @throws IOException on failure to bind ports. */ public static void run(Extension extension) throws IOException { logger.info("Starting extension " + extension.getExtensionSettings().getExtensionName()); @@ -595,8 +581,7 @@ public static void run(Extension extension) throws IOException { } /** - * Run the Extension. For internal/testing purposes only. Imports settings and - * sets up Transport Service listening for incoming connections. + * Run the Extension. For internal/testing purposes only. Imports settings and sets up Transport Service listening for incoming connections. * * @param args Unused * @throws IOException if the runner failed to connect to the OpenSearch cluster. From ca7130259538c7a45708ca03bdf4df6ac8202986 Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Fri, 30 Sep 2022 09:36:11 -0700 Subject: [PATCH 8/8] Change uri to path in RestRequest handling Signed-off-by: Daniel Widdis --- .../opensearch/sdk/ExtensionRestHandler.java | 1 - .../sdk/ExtensionRestPathRegistry.java | 22 +++++++++---------- .../ExtensionsRestRequestHandler.java | 4 ++-- 3 files changed, 13 insertions(+), 14 deletions(-) diff --git a/src/main/java/org/opensearch/sdk/ExtensionRestHandler.java b/src/main/java/org/opensearch/sdk/ExtensionRestHandler.java index 05553338..4f96b980 100644 --- a/src/main/java/org/opensearch/sdk/ExtensionRestHandler.java +++ b/src/main/java/org/opensearch/sdk/ExtensionRestHandler.java @@ -32,7 +32,6 @@ public interface ExtensionRestHandler { * Handles REST Requests forwarded from OpenSearch for a configured route on an extension. * Parameter contains components of the {@link RestRequest} received from a user. * This method corresponds to the {@link BaseRestHandler#prepareRequest} method. - * As in that method, consumed parameters must be tracked and returned in the response. * * @param request a REST request object for a request to be forwarded to extensions * @return An {@link ExtensionRestResponse} to the request. diff --git a/src/main/java/org/opensearch/sdk/ExtensionRestPathRegistry.java b/src/main/java/org/opensearch/sdk/ExtensionRestPathRegistry.java index 26d0c556..56bd850d 100644 --- a/src/main/java/org/opensearch/sdk/ExtensionRestPathRegistry.java +++ b/src/main/java/org/opensearch/sdk/ExtensionRestPathRegistry.java @@ -28,11 +28,11 @@ public class ExtensionRestPathRegistry { * Register a REST handler to handle a method and route in this extension's path registry. * * @param method The method to register. - * @param uri The URI to register. May include named wildcards. + * @param path The path to register. May include named wildcards. * @param extensionRestHandler The RestHandler to handle this route */ - public void registerHandler(Method method, String uri, ExtensionRestHandler extensionRestHandler) { - String restPath = restPathToString(method, uri); + public void registerHandler(Method method, String path, ExtensionRestHandler extensionRestHandler) { + String restPath = restPathToString(method, path); pathTrie.insert(restPath, extensionRestHandler); registeredPaths.add(restPath); } @@ -41,11 +41,11 @@ public void registerHandler(Method method, String uri, ExtensionRestHandler exte * Get the registered REST handler for the specified method and URI. * * @param method the registered method. - * @param uri the registered URI. + * @param path the registered path. * @return The REST handler registered to handle this method and URI combination if found, null otherwise. */ - public ExtensionRestHandler getHandler(Method method, String uri) { - return pathTrie.retrieve(restPathToString(method, uri)); + public ExtensionRestHandler getHandler(Method method, String path) { + return pathTrie.retrieve(restPathToString(method, path)); } /** @@ -58,13 +58,13 @@ public List getRegisteredPaths() { } /** - * Converts a REST method and URI to a string. + * Converts a REST method and path to a space delimited string to be used as a map lookup key. * * @param method the method. - * @param uri the URI. - * @return A string appending the method and URI. + * @param path the path. + * @return A string appending the method and path. */ - public static String restPathToString(Method method, String uri) { - return method.name() + " " + uri; + public static String restPathToString(Method method, String path) { + return method.name() + " " + path; } } diff --git a/src/main/java/org/opensearch/sdk/handlers/ExtensionsRestRequestHandler.java b/src/main/java/org/opensearch/sdk/handlers/ExtensionsRestRequestHandler.java index 351c8248..f777347c 100644 --- a/src/main/java/org/opensearch/sdk/handlers/ExtensionsRestRequestHandler.java +++ b/src/main/java/org/opensearch/sdk/handlers/ExtensionsRestRequestHandler.java @@ -34,11 +34,11 @@ public class ExtensionsRestRequestHandler { */ public RestExecuteOnExtensionResponse handleRestExecuteOnExtensionRequest(ExtensionRestRequest request) { - ExtensionRestHandler restHandler = extensionRestPathRegistry.getHandler(request.method(), request.uri()); + ExtensionRestHandler restHandler = extensionRestPathRegistry.getHandler(request.method(), request.path()); if (restHandler == null) { return new RestExecuteOnExtensionResponse( RestStatus.NOT_FOUND, - "No handler for " + ExtensionRestPathRegistry.restPathToString(request.method(), request.uri()) + "No handler for " + ExtensionRestPathRegistry.restPathToString(request.method(), request.path()) ); }