Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generate RestRequest to pass to Rest Handlers #623

Merged
merged 8 commits into from
Mar 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions PLUGIN_MIGRATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
19 changes: 7 additions & 12 deletions src/main/java/org/opensearch/sdk/BaseExtensionRestHandler.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand All @@ -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()))
Expand All @@ -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
*/
Expand Down Expand Up @@ -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() + "]"
);
}

Expand All @@ -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() + "]");
}

Expand All @@ -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,
Expand Down
5 changes: 2 additions & 3 deletions src/main/java/org/opensearch/sdk/ExtensionRestHandler.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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.
Expand Down
6 changes: 4 additions & 2 deletions src/main/java/org/opensearch/sdk/ExtensionsRunner.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);

Expand Down
8 changes: 4 additions & 4 deletions src/main/java/org/opensearch/sdk/RouteHandler.java
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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;
}
Expand All @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
}

Expand All @@ -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(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand All @@ -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");
Expand Down Expand Up @@ -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);
Expand All @@ -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.");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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");
Expand Down
Loading