Generate RestRequest to pass to Rest Handlers (#623)

* Generate RestRequest to pass to Rest Handlers

Signed-off-by: Daniel Widdis <[email protected]>

* Update Response handling to use RestRequest (needs companion PR)

Signed-off-by: Daniel Widdis <[email protected]>

* Update Extension with new SDK signature (preview of AD extension PR)

Signed-off-by: Daniel Widdis <[email protected]>

* Fix tests

Signed-off-by: Daniel Widdis <[email protected]>

* Less diff on migration, yay!

Signed-off-by: Daniel Widdis <[email protected]>

* Remove stray reference to old type

Signed-off-by: Daniel Widdis <[email protected]>

* Typo fix

Signed-off-by: Daniel Widdis <[email protected]>

* CharSequence isEmpty() is a JDK 15+ feature

Signed-off-by: Daniel Widdis <[email protected]>

---------

Signed-off-by: Daniel Widdis <[email protected]>

PLUGIN_MIGRATION.md

@@ -66,11 +66,6 @@ Many of these components are also available via Guice injection.
 
 Optionally change the `routes()` to `routeHandlers()`.  Change `prepareRequest()` to `handleRequest()`.
 
-### Replace RestRequest with ExtensionRestRequest
-
- - Change `request.contentParser()` to `request.contentParser(this.namedXContentRegistry)`
- - Change `request.getHttpRequest().method()` to `request.method()`
-
 ### Replace BytesRestResponse with ExtensionRestResponse
 
  - Add the `request` as the first parameter, the remainder of the parameters should be the same.

src/main/java/org/opensearch/sdk/BaseExtensionRestHandler.java

@@ -18,9 +18,9 @@
 import java.util.Optional;
 
 import org.opensearch.common.xcontent.json.JsonXContent;
-import org.opensearch.extensions.rest.ExtensionRestRequest;
 import org.opensearch.extensions.rest.ExtensionRestResponse;
 import org.opensearch.rest.RestHandler.Route;
+import org.opensearch.rest.RestRequest;
 import org.opensearch.rest.RestStatus;
 
 /**
@@ -44,7 +44,7 @@ public List<Route> routes() {
     }
 
     @Override
-    public ExtensionRestResponse handleRequest(ExtensionRestRequest request) {
+    public ExtensionRestResponse handleRequest(RestRequest request) {
         Optional<RouteHandler> handler = routeHandlers().stream()
             .filter(rh -> rh.getMethod().equals(request.method()))
             .filter(rh -> restPathMatches(request.path(), rh.getPath()))
@@ -55,7 +55,7 @@ public ExtensionRestResponse handleRequest(ExtensionRestRequest request) {
     /**
      * Determines if the request's path is a match for the configured handler path.
      *
-     * @param requestPath The path from the {@link ExtensionRestRequest}
+     * @param requestPath The path from the {@link RestRequest}
      * @param handlerPath The path from the {@link RouteHandler}
      * @return true if the request path matches the route
      */
@@ -85,12 +85,12 @@ private boolean restPathMatches(String requestPath, String handlerPath) {
      * @param request The request that couldn't be handled.
      * @return an ExtensionRestResponse identifying the unhandled request.
      */
-    protected ExtensionRestResponse unhandledRequest(ExtensionRestRequest request) {
+    protected ExtensionRestResponse unhandledRequest(RestRequest request) {
         return createJsonResponse(
             request,
             NOT_FOUND,
             "error",
-            "Extension REST action improperly configured to handle: [" + request.toString() + "]"
+            "Extension REST action improperly configured to handle: [" + request.method() + " " + request.uri() + "]"
         );
     }
 
@@ -101,7 +101,7 @@ protected ExtensionRestResponse unhandledRequest(ExtensionRestRequest request) {
      * @param e The exception
      * @return an ExtensionRestResponse identifying the exception
      */
-    protected ExtensionRestResponse exceptionalRequest(ExtensionRestRequest request, Exception e) {
+    protected ExtensionRestResponse exceptionalRequest(RestRequest request, Exception e) {
         return createJsonResponse(request, INTERNAL_SERVER_ERROR, "error", "Request failed with exception: [" + e.getMessage() + "]");
     }
 
@@ -114,12 +114,7 @@ protected ExtensionRestResponse exceptionalRequest(ExtensionRestRequest request,
      * @param responseStr The string to include
      * @return an ExtensionRestResponse in JSON format including the specified string as the content of the specified field
      */
-    protected ExtensionRestResponse createJsonResponse(
-        ExtensionRestRequest request,
-        RestStatus status,
-        String fieldName,
-        String responseStr
-    ) {
+    protected ExtensionRestResponse createJsonResponse(RestRequest request, RestStatus status, String fieldName, String responseStr) {
         try {
             return new ExtensionRestResponse(
                 request,

src/main/java/org/opensearch/sdk/ExtensionRestHandler.java

@@ -12,7 +12,6 @@
 import java.util.Collections;
 import java.util.List;
 
-import org.opensearch.extensions.rest.ExtensionRestRequest;
 import org.opensearch.extensions.rest.ExtensionRestResponse;
 import org.opensearch.rest.BaseRestHandler;
 import org.opensearch.rest.RestHandler;
@@ -31,10 +30,10 @@ public interface ExtensionRestHandler {
      * Parameter contains components of the {@link RestRequest} received from a user.
      * This method corresponds to the {@link BaseRestHandler#prepareRequest} method.
      *
-     * @param request a REST request object for a request to be forwarded to extensions
+     * @param restRequest a REST request object for a request to be forwarded to extensions
      * @return An {@link ExtensionRestResponse} to the request.
      */
-    ExtensionRestResponse handleRequest(ExtensionRestRequest request);
+    ExtensionRestResponse handleRequest(RestRequest restRequest);
 
     /**
      * A list of {@link Route}s that this ExtensionRestHandler is responsible for handling.

src/main/java/org/opensearch/sdk/ExtensionsRunner.java

@@ -152,7 +152,7 @@ public class ExtensionsRunner {
     private final ExtensionsIndicesModuleRequestHandler extensionsIndicesModuleRequestHandler = new ExtensionsIndicesModuleRequestHandler();
     private final ExtensionsIndicesModuleNameRequestHandler extensionsIndicesModuleNameRequestHandler =
         new ExtensionsIndicesModuleNameRequestHandler();
-    private final ExtensionsRestRequestHandler extensionsRestRequestHandler = new ExtensionsRestRequestHandler(extensionRestPathRegistry);
+    private final ExtensionsRestRequestHandler extensionsRestRequestHandler;
     private final ExtensionActionRequestHandler extensionsActionRequestHandler;
     private final AtomicReference<RunnableTaskExecutionListener> runnableTaskListener;
     private final IndexNameExpressionResolver indexNameExpressionResolver;
@@ -196,8 +196,10 @@ protected ExtensionsRunner(Extension extension) throws IOException {
         this.customNamedXContent = extension.getNamedXContent();
         // save custom namedWriteable
         this.customNamedWriteables = extension.getNamedWriteables();
-        // initialize NamedXContent Registry. Must happen after getting extension namedXContent
+        // initialize NamedXContent Registry.
         this.sdkNamedXContentRegistry = new SDKNamedXContentRegistry(this);
+        // initialize RestRequest Handler. Must happen after instantiating SDKNamedXContentRegistry
+        this.extensionsRestRequestHandler = new ExtensionsRestRequestHandler(extensionRestPathRegistry, sdkNamedXContentRegistry);
         // initialize NamedWriteable Registry. Must happen after getting extension namedWriteable
         this.sdkNamedWriteableRegistry = new SDKNamedWriteableRegistry(this);
 

src/main/java/org/opensearch/sdk/RouteHandler.java

@@ -11,17 +11,17 @@
 
 import java.util.function.Function;
 
-import org.opensearch.extensions.rest.ExtensionRestRequest;
 import org.opensearch.extensions.rest.ExtensionRestResponse;
 import org.opensearch.rest.RestHandler.Route;
+import org.opensearch.rest.RestRequest;
 import org.opensearch.rest.RestRequest.Method;
 
 /**
  * A subclass of {@link Route} that includes a handler method for that route.
  */
 public class RouteHandler extends Route {
 
-    private final Function<ExtensionRestRequest, ExtensionRestResponse> responseHandler;
+    private final Function<RestRequest, ExtensionRestResponse> responseHandler;
 
     /**
      * Handle the method and path with the specified handler.
@@ -30,7 +30,7 @@ public class RouteHandler extends Route {
      * @param path The path to handle.
      * @param handler The method which handles the method and path.
      */
-    public RouteHandler(Method method, String path, Function<ExtensionRestRequest, ExtensionRestResponse> handler) {
+    public RouteHandler(Method method, String path, Function<RestRequest, ExtensionRestResponse> handler) {
         super(method, path);
         this.responseHandler = handler;
     }
@@ -41,7 +41,7 @@ public RouteHandler(Method method, String path, Function<ExtensionRestRequest, E
      * @param request The request to handle
      * @return the {@link ExtensionRestResponse} result from the handler for this route.
      */
-    public ExtensionRestResponse handleRequest(ExtensionRestRequest request) {
+    public ExtensionRestResponse handleRequest(RestRequest request) {
         return responseHandler.apply(request);
     }
 }

src/main/java/org/opensearch/sdk/handlers/ExtensionsRestRequestHandler.java

@@ -12,11 +12,23 @@
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.opensearch.common.bytes.BytesReference;
+import org.opensearch.common.xcontent.XContentType;
 import org.opensearch.extensions.rest.ExtensionRestRequest;
 import org.opensearch.extensions.rest.ExtensionRestResponse;
 import org.opensearch.extensions.rest.RestExecuteOnExtensionResponse;
+import org.opensearch.http.HttpRequest;
+import org.opensearch.http.HttpResponse;
+import org.opensearch.rest.RestRequest;
+import org.opensearch.rest.RestRequest.Method;
+import org.opensearch.rest.RestStatus;
 import org.opensearch.sdk.ExtensionRestHandler;
 import org.opensearch.sdk.ExtensionsRunner;
+import org.opensearch.sdk.SDKNamedXContentRegistry;
+
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+
 import org.opensearch.sdk.ExtensionRestPathRegistry;
 
 import static java.nio.charset.StandardCharsets.UTF_8;
@@ -32,13 +44,16 @@
 public class ExtensionsRestRequestHandler {
     private static final Logger logger = LogManager.getLogger(ExtensionsRestRequestHandler.class);
     private final ExtensionRestPathRegistry extensionRestPathRegistry;
+    private final SDKNamedXContentRegistry sdkNamedXContentRegistry;
 
     /**
      * Instantiate this class with an existing registry
      *
      * @param restPathRegistry The ExtensionsRunnerer's REST path registry
+     * @param sdkNamedXContentRegistry
      */
-    public ExtensionsRestRequestHandler(ExtensionRestPathRegistry restPathRegistry) {
+    public ExtensionsRestRequestHandler(ExtensionRestPathRegistry restPathRegistry, SDKNamedXContentRegistry sdkNamedXContentRegistry) {
+        this.sdkNamedXContentRegistry = sdkNamedXContentRegistry;
         this.extensionRestPathRegistry = restPathRegistry;
     }
 
@@ -62,8 +77,73 @@ public RestExecuteOnExtensionResponse handleRestExecuteOnExtensionRequest(Extens
             );
         }
 
+        // Temporary code to create a RestRequest from the ExtensionRestRequest before header code added
+        // Remove this and replace with SDKRestRequest being generated by this PR:
+        // https://github.com/opensearch-project/opensearch-sdk-java/pull/605
+        RestRequest restRequest = RestRequest.request(sdkNamedXContentRegistry.getRegistry(), new HttpRequest() {
+
+            @Override
+            public Method method() {
+                return request.method();
+            }
+
+            @Override
+            public String uri() {
+                // path strips query off uri but probably want to pass the whole uri
+                // this will make the request behave as expected (without query params)
+                return request.path();
+            }
+
+            @Override
+            public BytesReference content() {
+                return request.content();
+            }
+
+            @Override
+            public Map<String, List<String>> getHeaders() {
+                // This effectively recreates the only header we need right now
+                // PR replacing this will pass more headers
+                XContentType xContentType = request.getXContentType();
+                return xContentType == null ? Collections.emptyMap() : Map.of("Content-Type", List.of(xContentType.mediaType()));
+            }
+
+            @Override
+            public List<String> strictCookies() {
+                return Collections.emptyList();
+            }
+
+            @Override
+            public HttpVersion protocolVersion() {
+                return null;
+            }
+
+            @Override
+            public HttpRequest removeHeader(String header) {
+                // we don't use
+                return null;
+            }
+
+            @Override
+            public HttpResponse createResponse(RestStatus status, BytesReference content) {
+                return null;
+            }
+
+            @Override
+            public Exception getInboundException() {
+                return null;
+            }
+
+            @Override
+            public void release() {}
+
+            @Override
+            public HttpRequest releaseAndCopy() {
+                return null;
+            }
+        }, null);
+
         // Get response from extension
-        ExtensionRestResponse response = restHandler.handleRequest(request);
+        ExtensionRestResponse response = restHandler.handleRequest(restRequest);
         logger.info("Sending extension response to OpenSearch: " + response.status());
         return new RestExecuteOnExtensionResponse(
             response.status(),

src/main/java/org/opensearch/sdk/sample/helloworld/rest/RestHelloAction.java

@@ -11,12 +11,11 @@
 
 import org.opensearch.OpenSearchParseException;
 import org.opensearch.common.bytes.BytesReference;
-import org.opensearch.core.xcontent.NamedXContentRegistry;
 import org.opensearch.core.xcontent.XContentBuilder;
 import org.opensearch.common.xcontent.XContentType;
 import org.opensearch.common.xcontent.json.JsonXContent;
-import org.opensearch.extensions.rest.ExtensionRestRequest;
 import org.opensearch.extensions.rest.ExtensionRestResponse;
+import org.opensearch.rest.RestRequest;
 import org.opensearch.sdk.BaseExtensionRestHandler;
 import org.opensearch.sdk.ExtensionRestHandler;
 import org.opensearch.sdk.RouteHandler;
@@ -63,14 +62,14 @@ public List<RouteHandler> routeHandlers() {
         );
     }
 
-    private Function<ExtensionRestRequest, ExtensionRestResponse> handleGetRequest = (request) -> {
+    private Function<RestRequest, ExtensionRestResponse> handleGetRequest = (request) -> {
         String worldNameWithRandomAdjective = worldAdjectives.isEmpty()
             ? worldName
             : String.join(" ", worldAdjectives.get(rand.nextInt(worldAdjectives.size())), worldName);
         return new ExtensionRestResponse(request, OK, String.format(GREETING, worldNameWithRandomAdjective));
     };
 
-    private Function<ExtensionRestRequest, ExtensionRestResponse> handlePostRequest = (request) -> {
+    private Function<RestRequest, ExtensionRestResponse> handlePostRequest = (request) -> {
         if (request.hasContent()) {
             String adjective = "";
             XContentType contentType = request.getXContentType();
@@ -79,7 +78,7 @@ public List<RouteHandler> routeHandlers() {
                 adjective = request.content().utf8ToString();
             } else if (contentType.equals(XContentType.JSON)) {
                 try {
-                    adjective = request.contentParser(NamedXContentRegistry.EMPTY).mapStrings().get("adjective");
+                    adjective = request.contentParser().mapStrings().get("adjective");
                 } catch (IOException | OpenSearchParseException e) {
                     // Sample plain text response
                     return new ExtensionRestResponse(request, BAD_REQUEST, "Unable to parse adjective from JSON");
@@ -118,7 +117,7 @@ public List<RouteHandler> routeHandlers() {
         return new ExtensionRestResponse(request, BAD_REQUEST, TEXT_CONTENT_TYPE, noContent);
     };
 
-    private Function<ExtensionRestRequest, ExtensionRestResponse> handlePutRequest = (request) -> {
+    private Function<RestRequest, ExtensionRestResponse> handlePutRequest = (request) -> {
         String name = request.param("name");
         try {
             worldName = URLDecoder.decode(name, StandardCharsets.UTF_8);
@@ -128,7 +127,7 @@ public List<RouteHandler> routeHandlers() {
         return new ExtensionRestResponse(request, OK, "Updated the world's name to " + worldName);
     };
 
-    private Function<ExtensionRestRequest, ExtensionRestResponse> handleDeleteRequest = (request) -> {
+    private Function<RestRequest, ExtensionRestResponse> handleDeleteRequest = (request) -> {
         this.worldName = DEFAULT_NAME;
         this.worldAdjectives.clear();
         return new ExtensionRestResponse(request, OK, "Goodbye, cruel world! Restored default values.");

src/main/java/org/opensearch/sdk/sample/helloworld/rest/RestRemoteHelloAction.java

@@ -13,8 +13,8 @@
 import org.opensearch.common.io.stream.StreamInput;
 import org.opensearch.extensions.ExtensionsManager;
 import org.opensearch.extensions.action.RemoteExtensionActionResponse;
-import org.opensearch.extensions.rest.ExtensionRestRequest;
 import org.opensearch.extensions.rest.ExtensionRestResponse;
+import org.opensearch.rest.RestRequest;
 import org.opensearch.sdk.BaseExtensionRestHandler;
 import org.opensearch.sdk.ExtensionsRunner;
 import org.opensearch.sdk.RouteHandler;
@@ -54,7 +54,7 @@ public List<RouteHandler> routeHandlers() {
         return List.of(new RouteHandler(GET, "/hello/{name}", handleRemoteGetRequest));
     }
 
-    private Function<ExtensionRestRequest, ExtensionRestResponse> handleRemoteGetRequest = (request) -> {
+    private Function<RestRequest, ExtensionRestResponse> handleRemoteGetRequest = (request) -> {
         SDKClient client = extensionsRunner.getSdkClient();
 
         String name = request.param("name");