diff --git a/CREATE_YOUR_FIRST_EXTENSION.md b/CREATE_YOUR_FIRST_EXTENSION.md index 5fbc210d..5130b8c9 100644 --- a/CREATE_YOUR_FIRST_EXTENSION.md +++ b/CREATE_YOUR_FIRST_EXTENSION.md @@ -150,60 +150,43 @@ These classes must implement _`ExtensionRestHandler`_, which is a functional int The `BaseExtensionRestHandler` class provides many useful methods for exception handling in requests. -For the CRUD extension example, you'll implement one REST route for each option and delegate it to the appropriate handler function. Each route is an instance of `NamedRoute` and requires at least a method, path, and globally unique name. +For the CRUD extension example, you'll implement one REST route for each option and delegate it to the appropriate handler function. ```java import java.util.List; import java.util.function.Function; -import org.opensearch.rest.NamedRoute; -import org.opensearch.rest.RestRequest; +import org.opensearch.extensions.rest.ExtensionRestResponse; import org.opensearch.rest.RestRequest.Method; -import org.opensearch.rest.RestResponse; +import org.opensearch.rest.RestRequest; import org.opensearch.rest.RestStatus; import org.opensearch.sdk.rest.BaseExtensionRestHandler; public class CrudAction extends BaseExtensionRestHandler { @Override - public List routes() { + protected List routeHandlers() { return List.of( - new NamedRoute.Builder().method(Method.PUT) - .path("/sample") - .uniqueName("crud_extension:sample/create") - .handler(createHandler) - .build(), - new NamedRoute.Builder().method(Method.GET) - .path("/sample/{id}") - .uniqueName("crud_extension:sample/get") - .handler(readHandler) - .build(), - new NamedRoute.Builder().method(Method.POST) - .path("/sample/{id}") - .uniqueName("crud_extension:sample/post") - .handler(updateHandler) - .build(), - new NamedRoute.Builder().method(Method.DELETE) - .path("/sample/{id}") - .uniqueName("crud_extension:sample/delete") - .handler(deleteHandler) - .build() + new RouteHandler(Method.PUT, "/sample", createHandler), + new RouteHandler(Method.GET, "/sample/{id}", readHandler), + new RouteHandler(Method.POST, "/sample/{id}", updateHandler), + new RouteHandler(Method.DELETE, "/sample/{id}", deleteHandler) ); } - Function createHandler = (request) -> { + Function createHandler = (request) -> { return new ExtensionRestResponse(request, RestStatus.OK, "To be implemented"); }; - Function readHandler = (request) -> { + Function readHandler = (request) -> { return new ExtensionRestResponse(request, RestStatus.OK, "To be implemented"); }; - Function updateHandler = (request) -> { + Function updateHandler = (request) -> { return new ExtensionRestResponse(request, RestStatus.OK, "To be implemented"); }; - Function deleteHandler = (request) -> { + Function deleteHandler = (request) -> { return new ExtensionRestResponse(request, RestStatus.OK, "To be implemented"); }; } @@ -278,7 +261,7 @@ return createJsonResponse(request, RestStatus.OK, "_id", response.id()); Finally, you have the following code for the create handler method: ```java -Function createHandler = (request) -> { +Function createHandler = (request) -> { IndexResponse response; try { // Create index if it doesn't exist @@ -312,7 +295,7 @@ GetResponse response = client.get(new GetRequest.Builder().index("crud Adding exception handling, the following is the full handler method: ```java -Function readHandler = (request) -> { +Function readHandler = (request) -> { GetResponse response; // Parse ID from request String id = request.param("id"); @@ -333,7 +316,7 @@ Function readHandler = (request) -> { You will create a new document similar to the one you created in the create handler, parse the ID as you did in the read handler, and then update that document. With exception handling, the following is the update handler method: ```java -Function updateHandler = (request) -> { +Function updateHandler = (request) -> { UpdateResponse response; // Parse ID from request String id = request.param("id"); @@ -360,7 +343,7 @@ Function updateHandler = (request) -> { You only need the ID to delete a document, so the delete handler method is implemented as follows: ```java -Function deleteHandler = (request) -> { +Function deleteHandler = (request) -> { DeleteResponse response; // Parse ID from request String id = request.param("id"); diff --git a/DEVELOPER_GUIDE.md b/DEVELOPER_GUIDE.md index 43a3eefe..021ebc7a 100644 --- a/DEVELOPER_GUIDE.md +++ b/DEVELOPER_GUIDE.md @@ -146,14 +146,14 @@ To **run OpenSearch from a compiled binary**, follow these steps: - Start OpenSearch using `./bin/opensearch`. - Send the below sample REST API to initialize an extension ```bash -curl -XPOST "localhost:9200/_extensions/initialize" -H "Content-Type:application/json" --data '{ -"name":"hello-world", -"uniqueId":"hello-world", -"hostAddress":"127.0.0.1", -"port":"4532", -"version":"1.0", -"opensearchVersion":"3.0.0", -"minimumCompatibleVersion":"3.0.0", +curl -XPOST "localhost:9200/_extensions/initialize" -H "Content-Type:application/json" --data '{ \ +"name":"hello-world", \ +"uniqueId":"hello-world", \ +"hostAddress":"127.0.0.1", \ +"port":"4532", \ +"version":"1.0", \ +"opensearchVersion":"3.0.0", \ +"minimumCompatibleVersion":"3.0.0", \ "dependencies":[{"uniqueId":"test1","version":"2.0.0"},{"uniqueId":"test2","version":"3.0.0"}] \ }' ``` @@ -162,20 +162,18 @@ To **run OpenSearch from Gradle**, follow these steps: - Run `./gradlew run` to start OpenSearch. - Send the below sample REST API to initialize an extension ```bash -curl -XPOST "localhost:9200/_extensions/initialize" -H "Content-Type:application/json" --data '{ -"name":"hello-world", -"uniqueId":"hello-world", -"hostAddress":"127.0.0.1", -"port":"4532", -"version":"1.0", -"opensearchVersion":"3.0.0", -"minimumCompatibleVersion":"3.0.0", -"dependencies":[{"uniqueId":"test1","version":"2.0.0"},{"uniqueId":"test2","version":"3.0.0"}] +curl -XPOST "localhost:9200/_extensions/initialize" -H "Content-Type:application/json" --data '{ \ +"name":"hello-world", \ +"uniqueId":"hello-world", \ +"hostAddress":"127.0.0.1", \ +"port":"4532", \ +"version":"1.0", \ +"opensearchVersion":"3.0.0", \ +"minimumCompatibleVersion":"3.0.0", \ +"dependencies":[{"uniqueId":"test1","version":"2.0.0"},{"uniqueId":"test2","version":"3.0.0"}] \ }' ``` -Note: If the Security plugin is initialized in OpenSearch, use admin credentials to send extension initialization request. - In response to the REST `/initialize` request, `ExtensionsManager` discovers the extension listening on a predefined port and executes the TCP handshake protocol to establish a data transfer connection. Then OpenSearch sends a request to the OpenSearch SDK for Java and, upon acknowledgment, the extension responds with its name. This name is logged in the terminal where OpenSearch is running: ```bash @@ -303,9 +301,6 @@ The artifact will include extension settings for the sample Hello World extensio opensearchPort: 9200 ``` -You can optionally add `routeNamePrefix:` as a value to the yml. This setting allows you to prefix all your registered NamedRoute names. -The value must be alphanumeric and can contain `_` in the name. - Start the sample extension with `./bin/opensearch-sdk-java` ### Submitting changes diff --git a/PLUGIN_MIGRATION.md b/PLUGIN_MIGRATION.md index 5be87b61..bfb134c3 100644 --- a/PLUGIN_MIGRATION.md +++ b/PLUGIN_MIGRATION.md @@ -67,38 +67,14 @@ XContentParser parser = XContentType.JSON Other potential initialization values are: ```java this.environmentSettings = extensionsRunner.getEnvironmentSettings(); -this.transportService = extensionsRunner.getSdkTransportService().getTransportService(); +this.transportService = extensionsRunner.getExtensionTransportService(); this.restClient = anomalyDetectorExtension.getRestClient(); this.sdkClusterService = new SDKClusterService(extensionsRunner); ``` Many of these components are also available via Guice injection. -### Replace `Route` with `NamedRoute` -Change `routes()` to be NamedRoutes. Here is a sample of an existing route converted to a named route: -Before: -``` -public List routes() { - return ImmutableList.of( - new Route(GET, "/uri") - ); -} -``` -With new scheme: -``` -private Function uriHandler = () -> {}; -public List routes() { - return ImmutableList.of( - new NamedRoute.Builder().method(GET).path("/uri").uniqueName("extension:uri").handler(uriHandler).build() - ); -} -``` - -You can optionally also add `actionNames()` to this route. These should correspond to any current actions defined as permissions in roles. -`actionNames()` serve as a valuable tool for converting plugins into extensions while maintaining compatibility with pre-defined reserved roles. -Ensure that these name-to-route mappings are easily accessible to the cluster admins to allow granting access to these APIs. - -Change `prepareRequest()` to `handleRequest()`. +Optionally, change the `routes()` to `routeHandlers()`. Change `prepareRequest()` to `handleRequest()`. ### Replace `BytesRestResponse` with `ExtensionRestResponse` diff --git a/build.gradle b/build.gradle index a7122ac8..5d61109e 100644 --- a/build.gradle +++ b/build.gradle @@ -12,7 +12,6 @@ import java.nio.file.Files import org.gradle.api.internal.tasks.testing.junitplatform.JUnitPlatformTestFramework import org.opensearch.gradle.test.RestIntegTestTask -import org.opensearch.gradle.testclusters.ExtensionsProperties buildscript { ext { @@ -147,7 +146,6 @@ dependencies { def javaxVersion = "1" def guavaFailureAccessVersion = "1.0.1" def aopallianceVersion = "1.0" - def slf4jVersion = "1.7.36" api("org.opensearch:opensearch:${opensearchVersion}") implementation("org.apache.logging.log4j:log4j-api:${log4jVersion}") @@ -188,19 +186,16 @@ dependencies { testRuntimeOnly("org.junit.platform:junit-platform-launcher:${junitPlatform}") configurations.all { - resolutionStrategy { - force("jakarta.json:jakarta.json-api:${jakartaVersion}") - force("com.fasterxml.jackson.core:jackson-databind:${jacksonDatabindVersion}") - force("com.fasterxml.jackson.core:jackson-core:${jacksonDatabindVersion}") - force("com.fasterxml.jackson.dataformat:jackson-dataformat-yaml:${jacksonDatabindVersion}") - force("com.fasterxml.jackson.dataformat:jackson-dataformat-smile:${jacksonDatabindVersion}") - force("com.fasterxml.jackson.dataformat:jackson-dataformat-cbor:${jacksonDatabindVersion}") - force("org.apache.logging.log4j:log4j-api:${log4jVersion}") - force("org.apache.logging.log4j:log4j-core:${log4jVersion}") - force("org.apache.logging.log4j:log4j-jul:${log4jVersion}") - force("org.opensearch.client:opensearch-rest-client:${opensearchVersion}") - force("org.slf4j:slf4j-api:${slf4jVersion}") - } + resolutionStrategy.force("jakarta.json:jakarta.json-api:${jakartaVersion}") + resolutionStrategy.force("com.fasterxml.jackson.core:jackson-databind:${jacksonDatabindVersion}") + resolutionStrategy.force("com.fasterxml.jackson.core:jackson-core:${jacksonDatabindVersion}") + resolutionStrategy.force("com.fasterxml.jackson.dataformat:jackson-dataformat-yaml:${jacksonDatabindVersion}") + resolutionStrategy.force("com.fasterxml.jackson.dataformat:jackson-dataformat-smile:${jacksonDatabindVersion}") + resolutionStrategy.force("com.fasterxml.jackson.dataformat:jackson-dataformat-cbor:${jacksonDatabindVersion}") + resolutionStrategy.force("org.apache.logging.log4j:log4j-api:${log4jVersion}") + resolutionStrategy.force("org.apache.logging.log4j:log4j-core:${log4jVersion}") + resolutionStrategy.force("org.apache.logging.log4j:log4j-jul:${log4jVersion}") + resolutionStrategy.force("org.opensearch.client:opensearch-rest-client:${opensearchVersion}") } } diff --git a/config/certs/cert-gen.sh b/config/certs/cert-gen.sh deleted file mode 100755 index 2547511b..00000000 --- a/config/certs/cert-gen.sh +++ /dev/null @@ -1,34 +0,0 @@ -#! /bin/bash - -openssl genrsa -out root-ca-key.pem 2048 -openssl req -new -x509 -sha256 -key root-ca-key.pem -subj "/C=US/ST=NEW YORK/L=BROOKLYN/O=OPENSEARCH/OU=SECURITY/CN=ROOT" -out root-ca.pem -days 730 - -openssl genrsa -out extension-01-key-temp.pem 2048 -openssl pkcs8 -inform PEM -outform PEM -in extension-01-key-temp.pem -topk8 -nocrypt -v1 PBE-SHA1-3DES -out extension-01-key.pem -openssl req -new -key extension-01-key.pem -subj "/C=US/ST=NEW YORK/L=BROOKLYN/O=OPENSEARCH/OU=SECURITY/CN=extension-01" -out extension-01.csr -echo 'subjectAltName=DNS:extension-01' | tee -a extension-01.ext -echo 'subjectAltName=IP:172.20.0.11' | tee -a extension-01.ext -openssl x509 -req -in extension-01.csr -CA root-ca.pem -CAkey root-ca-key.pem -CAcreateserial -sha256 -out extension-01.pem -days 730 -extfile extension-01.ext - -rm extension-01-key-temp.pem -rm extension-01.csr -rm extension-01.ext -rm root-ca.srl - -openssl genrsa -out admin-key-temp.pem 2048 -openssl pkcs8 -inform PEM -outform PEM -in admin-key-temp.pem -topk8 -nocrypt -v1 PBE-SHA1-3DES -out admin-key.pem -openssl req -new -key admin-key.pem -subj "/C=US/ST=NEW YORK/L=BROOKLYN/O=OPENSEARCH/OU=SECURITY/CN=A" -out admin.csr -openssl x509 -req -in admin.csr -CA root-ca.pem -CAkey root-ca-key.pem -CAcreateserial -sha256 -out admin.pem -days 730 -openssl genrsa -out os-node-01-key-temp.pem 2048 -openssl pkcs8 -inform PEM -outform PEM -in os-node-01-key-temp.pem -topk8 -nocrypt -v1 PBE-SHA1-3DES -out os-node-01-key.pem -openssl req -new -key os-node-01-key.pem -subj "/C=US/ST=NEW YORK/L=BROOKLYN/O=OPENSEARCH/OU=SECURITY/CN=os-node-01" -out os-node-01.csr -echo 'subjectAltName=DNS:os-node-01' | tee -a os-node-01.ext -echo 'subjectAltName=IP:172.20.0.11' | tee -a os-node-01.ext -openssl x509 -req -in os-node-01.csr -CA root-ca.pem -CAkey root-ca-key.pem -CAcreateserial -sha256 -out os-node-01.pem -days 730 -extfile os-node-01.ext - -rm admin-key-temp.pem -rm admin.csr -rm os-node-01-key-temp.pem -rm os-node-01.csr -rm os-node-01.ext -rm root-ca.srl \ No newline at end of file diff --git a/src/main/java/org/opensearch/sdk/ExtensionSettings.java b/src/main/java/org/opensearch/sdk/ExtensionSettings.java index f366faed..50c0993b 100644 --- a/src/main/java/org/opensearch/sdk/ExtensionSettings.java +++ b/src/main/java/org/opensearch/sdk/ExtensionSettings.java @@ -53,13 +53,7 @@ public class ExtensionSettings { private String hostPort; private String opensearchAddress; private String opensearchPort; - private String routeNamePrefix; - private Map securitySettings; - /** - * A set of keys for security settings related to SSL transport, keystore and truststore files, and hostname verification. - * These settings are used in OpenSearch to secure network communication and ensure data privacy. - */ public static final Set SECURITY_SETTINGS_KEYS = Set.of( "path.home", // TODO Find the right place to put this setting SSL_TRANSPORT_CLIENT_PEMCERT_FILEPATH, @@ -85,6 +79,8 @@ public class ExtensionSettings { SSL_TRANSPORT_TRUSTSTORE_TYPE ); + private Map securitySettings; + /** * Jackson requires a no-arg constructor. */ @@ -120,7 +116,6 @@ public ExtensionSettings(String extensionName, String hostAddress, String hostPo * @param hostPort The port to bind this extension to. * @param opensearchAddress The IP Address on which OpenSearch is running. * @param opensearchPort The port on which OpenSearch is running. - * @param routeNamePrefix The prefix to be pre-pended to a NamedRoute being registered * @param securitySettings A generic map of any settings set in the config file that are not default setting keys */ public ExtensionSettings( @@ -129,83 +124,40 @@ public ExtensionSettings( String hostPort, String opensearchAddress, String opensearchPort, - String routeNamePrefix, Map securitySettings ) { this(extensionName, hostAddress, hostPort, opensearchAddress, opensearchPort); - this.routeNamePrefix = routeNamePrefix; this.securitySettings = securitySettings; } - /** - * Returns the name of the extension. - * @return A string representing the name of the extension. - */ public String getExtensionName() { return extensionName; } - /** - * Returns the host address associated with this object. - * @return The host address as a string. - */ public String getHostAddress() { return hostAddress; } - /** - * Returns the host and port number of the server. - * @return A string representation of the host and port number of the server. - */ public String getHostPort() { return hostPort; } - /** - * Sets the OpenSearch server address to use for connecting to OpenSearch. - * @param opensearchAddress the URL or IP address of the OpenSearch server. - */ public void setOpensearchAddress(String opensearchAddress) { this.opensearchAddress = opensearchAddress; } - /** - * Returns the address of the OpenSearch instance being used by the application. - * @return The address of the OpenSearch instance. - */ public String getOpensearchAddress() { return opensearchAddress; } - /** - * Sets the OpenSearch port number to be used for communication. - * @param opensearchPort The port number to set. - */ public void setOpensearchPort(String opensearchPort) { this.opensearchPort = opensearchPort; } - /** - * Returns the OpenSearch port number. - * @return The OpenSearch port number as a String. - */ public String getOpensearchPort() { return opensearchPort; } - /** - * Returns the route Prefix for all routes registered by this extension - * @return A string representing the route prefix of this extension - */ - public String getRoutePrefix() { - return routeNamePrefix; - } - - /** - * Returns the security settings as a map of key-value pairs. - * The keys represent the different security settings available, and the values represent the values set for each key. - * @return A map of security settings and their values. - */ public Map getSecuritySettings() { return securitySettings; } @@ -251,19 +203,12 @@ public static ExtensionSettings readSettingsFromYaml(String extensionSettingsPat securitySettings.put(settingKey, extensionMap.get(settingKey).toString()); } } - - // Making routeNamePrefix an optional setting - String routeNamePrefix = null; - if (extensionMap.containsKey("routeNamePrefix")) { - routeNamePrefix = extensionMap.get("routeNamePrefix").toString(); - } return new ExtensionSettings( extensionMap.get("extensionName").toString(), extensionMap.get("hostAddress").toString(), extensionMap.get("hostPort").toString(), extensionMap.get("opensearchAddress").toString(), extensionMap.get("opensearchPort").toString(), - routeNamePrefix, securitySettings ); } catch (URISyntaxException e) { diff --git a/src/main/java/org/opensearch/sdk/ExtensionsRunner.java b/src/main/java/org/opensearch/sdk/ExtensionsRunner.java index b884b79d..7cb1f6f2 100644 --- a/src/main/java/org/opensearch/sdk/ExtensionsRunner.java +++ b/src/main/java/org/opensearch/sdk/ExtensionsRunner.java @@ -36,7 +36,6 @@ import org.opensearch.sdk.handlers.ExtensionsInitRequestHandler; import org.opensearch.sdk.handlers.ExtensionsRestRequestHandler; import org.opensearch.sdk.handlers.UpdateSettingsRequestHandler; -import org.opensearch.sdk.rest.BaseExtensionRestHandler; import org.opensearch.sdk.rest.ExtensionRestHandler; import org.opensearch.sdk.rest.ExtensionRestPathRegistry; import org.opensearch.tasks.TaskManager; @@ -234,9 +233,6 @@ protected ExtensionsRunner(Extension extension) throws IOException { if (extension instanceof ActionExtension) { // store REST handlers in the registry for (ExtensionRestHandler extensionRestHandler : ((ActionExtension) extension).getExtensionRestHandlers()) { - if (extensionRestHandler instanceof BaseExtensionRestHandler) { - ((BaseExtensionRestHandler) extensionRestHandler).setRouteNamePrefix(extensionSettings.getRoutePrefix()); - } extensionRestPathRegistry.registerHandler(extensionRestHandler); } } diff --git a/src/main/java/org/opensearch/sdk/rest/BaseExtensionRestHandler.java b/src/main/java/org/opensearch/sdk/rest/BaseExtensionRestHandler.java index c72b8a21..524d3941 100644 --- a/src/main/java/org/opensearch/sdk/rest/BaseExtensionRestHandler.java +++ b/src/main/java/org/opensearch/sdk/rest/BaseExtensionRestHandler.java @@ -14,7 +14,7 @@ import java.util.List; import java.util.Optional; import java.util.Set; - +import org.opensearch.rest.BaseRestHandler; import static org.opensearch.rest.RestStatus.INTERNAL_SERVER_ERROR; import static org.opensearch.rest.RestStatus.NOT_FOUND; @@ -23,40 +23,40 @@ import java.util.function.Function; import static org.apache.http.entity.ContentType.APPLICATION_JSON; -import org.opensearch.OpenSearchException; import org.opensearch.common.logging.DeprecationLogger; import org.opensearch.common.xcontent.json.JsonXContent; -import org.opensearch.core.common.Strings; import org.opensearch.extensions.rest.ExtensionRestResponse; -import org.opensearch.rest.BaseRestHandler; -import org.opensearch.rest.DeprecationRestHandler; -import org.opensearch.rest.NamedRoute; import org.opensearch.rest.RestHandler.DeprecatedRoute; import org.opensearch.rest.RestHandler.ReplacedRoute; -import org.opensearch.rest.RestRequest; +import org.opensearch.rest.RestHandler.Route; import org.opensearch.rest.RestRequest.Method; -import org.opensearch.rest.RestResponse; +import org.opensearch.rest.DeprecationRestHandler; +import org.opensearch.rest.RestRequest; import org.opensearch.rest.RestStatus; /** * Provides convenience methods to reduce boilerplate code in an {@link ExtensionRestHandler} implementation. */ public abstract class BaseExtensionRestHandler implements ExtensionRestHandler { - - private static final String VALID_ROUTE_PREFIX_PATTERN = "^[a-zA-Z0-9_]*$"; - - private String routeNamePrefix; - /** * Constant for JSON content type */ public static final String JSON_CONTENT_TYPE = APPLICATION_JSON.withCharset(StandardCharsets.UTF_8).toString(); - @Override - public List routes() { + /** + * Defines a list of methods which handle each rest {@link Route}. Override this in a subclass to use the functional syntax. + * + * @return a list of {@link RouteHandler} with corresponding methods to each route. + */ + protected List routeHandlers() { return Collections.emptyList(); } + @Override + public List routes() { + return List.copyOf(routeHandlers()); + } + /** * Defines a list of methods which handle each rest {@link DeprecatedRoute}. Override this in a subclass to use the functional syntax. * @@ -80,32 +80,6 @@ protected List replacedRouteHandlers() { return Collections.emptyList(); } - /** - * Sets the route prefix that can be used to prepend route names - * @param prefix the prefix to be used - */ - public void setRouteNamePrefix(String prefix) { - // we by-pass null assignment as routeNamePrefixes are not mandatory - if (prefix != null && !prefix.matches(VALID_ROUTE_PREFIX_PATTERN)) { - throw new OpenSearchException( - "Invalid route name prefix specified. The prefix may include the following characters" + " 'a-z', 'A-Z', '0-9', '_'" - ); - } - routeNamePrefix = prefix; - } - - /** - * Generates a name for the handler prepended with the route prefix - * @param routeName The human-readable name for a route registered by this extension - * @return Returns a name conditionally prepended with the valid route prefix - */ - protected String addRouteNamePrefix(String routeName) { - if (Strings.isNullOrEmpty(routeNamePrefix)) { - return routeName; - } - return routeNamePrefix + ":" + routeName; - } - @Override public List replacedRoutes() { return List.copyOf(replacedRouteHandlers()); @@ -113,12 +87,12 @@ public List replacedRoutes() { @Override public ExtensionRestResponse handleRequest(RestRequest request) { - Optional route = routes().stream() + Optional handler = routeHandlers().stream() .filter(rh -> rh.getMethod().equals(request.method())) .filter(rh -> restPathMatches(request.path(), rh.getPath())) .findFirst(); - if (route.isPresent() && route.get().handler() != null) { - return (ExtensionRestResponse) route.get().handler().apply(request); + if (handler.isPresent()) { + return handler.get().handleRequest(request); } Optional deprecatedHandler = deprecatedRouteHandlers().stream() .filter(rh -> rh.getMethod().equals(request.method())) @@ -148,7 +122,7 @@ public ExtensionRestResponse handleRequest(RestRequest request) { * Determines if the request's path is a match for the configured handler path. * * @param requestPath The path from the {@link RestRequest} - * @param handlerPath The path from the {@link NamedRoute} or {@link DeprecatedRouteHandler} or {@link ReplacedRouteHandler} + * @param handlerPath The path from the {@link RouteHandler} or {@link DeprecatedRouteHandler} * @return true if the request path matches the route */ private boolean restPathMatches(String requestPath, String handlerPath) { @@ -249,12 +223,42 @@ protected ExtensionRestResponse createEmptyJsonResponse(RestRequest request, Res return new ExtensionRestResponse(request, status, JSON_CONTENT_TYPE, "{}"); } + /** + * A subclass of {@link Route} that includes a handler method for that route. + */ + public static class RouteHandler extends Route { + + private final Function responseHandler; + + /** + * Handle the method and path with the specified handler. + * + * @param method The {@link Method} to handle. + * @param path The path to handle. + * @param handler The method which handles the method and path. + */ + public RouteHandler(Method method, String path, Function handler) { + super(method, path); + this.responseHandler = handler; + } + + /** + * Executes the handler for this route. + * + * @param request The request to handle + * @return the {@link ExtensionRestResponse} result from the handler for this route. + */ + public ExtensionRestResponse handleRequest(RestRequest request) { + return responseHandler.apply(request); + } + } + /** * A subclass of {@link DeprecatedRoute} that includes a handler method for that route. */ public static class DeprecatedRouteHandler extends DeprecatedRoute { - private final Function responseHandler; + private final Function responseHandler; /** * Handle the method and path with the specified handler. @@ -264,7 +268,12 @@ public static class DeprecatedRouteHandler extends DeprecatedRoute { * @param deprecationMessage The message to log with the deprecation logger * @param handler The method which handles the method and path. */ - public DeprecatedRouteHandler(Method method, String path, String deprecationMessage, Function handler) { + public DeprecatedRouteHandler( + Method method, + String path, + String deprecationMessage, + Function handler + ) { super(method, path, deprecationMessage); this.responseHandler = handler; } @@ -276,7 +285,7 @@ public DeprecatedRouteHandler(Method method, String path, String deprecationMess * @return the {@link ExtensionRestResponse} result from the handler for this route. */ public ExtensionRestResponse handleRequest(RestRequest request) { - return (ExtensionRestResponse) responseHandler.apply(request); + return responseHandler.apply(request); } } diff --git a/src/main/java/org/opensearch/sdk/rest/ExtensionRestHandler.java b/src/main/java/org/opensearch/sdk/rest/ExtensionRestHandler.java index 5cb2e502..ed4d3c87 100644 --- a/src/main/java/org/opensearch/sdk/rest/ExtensionRestHandler.java +++ b/src/main/java/org/opensearch/sdk/rest/ExtensionRestHandler.java @@ -14,7 +14,6 @@ import org.opensearch.extensions.rest.ExtensionRestResponse; import org.opensearch.rest.BaseRestHandler; -import org.opensearch.rest.NamedRoute; import org.opensearch.rest.RestHandler; import org.opensearch.rest.RestHandler.DeprecatedRoute; import org.opensearch.rest.RestHandler.ReplacedRoute; @@ -43,7 +42,7 @@ public interface ExtensionRestHandler { * * @return The routes this handler will handle. */ - default List routes() { + default List routes() { return Collections.emptyList(); } diff --git a/src/main/java/org/opensearch/sdk/rest/ExtensionRestPathRegistry.java b/src/main/java/org/opensearch/sdk/rest/ExtensionRestPathRegistry.java index 0a8cc12c..3dd76950 100644 --- a/src/main/java/org/opensearch/sdk/rest/ExtensionRestPathRegistry.java +++ b/src/main/java/org/opensearch/sdk/rest/ExtensionRestPathRegistry.java @@ -11,12 +11,10 @@ import java.util.ArrayList; import java.util.List; -import java.util.Set; import org.opensearch.common.Nullable; import org.opensearch.common.logging.DeprecationLogger; import org.opensearch.common.path.PathTrie; - import org.opensearch.rest.RestRequest.Method; import org.opensearch.rest.RestUtils; import org.opensearch.sdk.rest.BaseExtensionRestHandler.ExtensionDeprecationRestHandler; @@ -40,18 +38,9 @@ public class ExtensionRestPathRegistry { * @param restHandler The RestHandler to register routes. */ public void registerHandler(ExtensionRestHandler restHandler) { - restHandler.routes().forEach(route -> { - String routeActionName = route.name(); - if (routeActionName == null) { - throw new IllegalArgumentException("Route handler must have a name associated with it."); - } - Set associatedActions = route.actionNames(); - registerHandler(route.getMethod(), route.getPath(), routeActionName, associatedActions, restHandler); - }); - + restHandler.routes().forEach(route -> registerHandler(route.getMethod(), route.getPath(), restHandler)); restHandler.deprecatedRoutes() .forEach(route -> registerAsDeprecatedHandler(route.getMethod(), route.getPath(), restHandler, route.getDeprecationMessage())); - restHandler.replacedRoutes() .forEach( route -> registerWithDeprecatedHandler( @@ -68,28 +57,20 @@ public void registerHandler(ExtensionRestHandler restHandler) { * Registers a REST handler to be executed when one of the provided methods and path match the request. * * @param path Path to handle (e.g., "/{index}/{type}/_bulk") - * @param extensionRestHandler The handler to actually execute - * @param name The name corresponding to this handler - * @param actionNames The set of actions to be registered with this handler + * @param handler The handler to actually execute * @param method GET, POST, etc. */ - public void registerHandler( - Method method, - String path, - String name, - Set actionNames, - ExtensionRestHandler extensionRestHandler - ) { + private void registerHandler(Method method, String path, ExtensionRestHandler extensionRestHandler) { pathTrie.insertOrUpdate( path, new SDKMethodHandlers(path, extensionRestHandler, method), (mHandlers, newMHandler) -> mHandlers.addMethods(extensionRestHandler, method) ); if (extensionRestHandler instanceof ExtensionDeprecationRestHandler) { - registeredDeprecatedPaths.add(restPathToString(method, path, name, actionNames)); + registeredDeprecatedPaths.add(restPathToString(method, path)); registeredDeprecatedPaths.add(((ExtensionDeprecationRestHandler) extensionRestHandler).getDeprecationMessage()); } else { - registeredPaths.add(restPathToString(method, path, name, actionNames)); + registeredPaths.add(restPathToString(method, path)); } } @@ -104,7 +85,7 @@ public void registerHandler( private void registerAsDeprecatedHandler(Method method, String path, ExtensionRestHandler handler, String deprecationMessage) { assert (handler instanceof ExtensionDeprecationRestHandler) == false; - registerHandler(method, path, null, null, new ExtensionDeprecationRestHandler(handler, deprecationMessage, deprecationLogger)); + registerHandler(method, path, new ExtensionDeprecationRestHandler(handler, deprecationMessage, deprecationLogger)); } /** @@ -148,7 +129,7 @@ private void registerWithDeprecatedHandler( + path + "] instead."; - registerHandler(method, path, null, null, handler); + registerHandler(method, path, handler); registerAsDeprecatedHandler(deprecatedMethod, deprecatedPath, handler, deprecationMessage); } @@ -190,18 +171,9 @@ public List getRegisteredDeprecatedPaths() { * * @param method the method. * @param path the path. - * @param name the name corresponding to this route. - * @param actionNames the set of actions registered with this route * @return A string appending the method and path. */ - public static String restPathToString(Method method, String path, String name, Set actionNames) { - StringBuilder sb = new StringBuilder(method.name() + " " + path + " "); - if (name != null && !name.isEmpty()) { - sb.append(name).append(" "); - } - if (actionNames != null) { - actionNames.forEach(act -> sb.append(act).append(",")); - } - return sb.toString(); + public static String restPathToString(Method method, String path) { + return method.name() + " " + path; } } diff --git a/src/main/java/org/opensearch/sdk/rest/ReplacedRouteHandler.java b/src/main/java/org/opensearch/sdk/rest/ReplacedRouteHandler.java index 1fdab92b..99d4c5ab 100644 --- a/src/main/java/org/opensearch/sdk/rest/ReplacedRouteHandler.java +++ b/src/main/java/org/opensearch/sdk/rest/ReplacedRouteHandler.java @@ -16,14 +16,13 @@ import org.opensearch.rest.RestHandler.Route; import org.opensearch.rest.RestRequest; import org.opensearch.rest.RestRequest.Method; -import org.opensearch.rest.RestResponse; /** * A subclass of {@link ReplacedRoute} that includes a handler method for that route. */ public class ReplacedRouteHandler extends ReplacedRoute { - private final Function responseHandler; + private final Function responseHandler; /** * Handle replaced routes using new and deprecated methods and new and deprecated paths. @@ -32,14 +31,14 @@ public class ReplacedRouteHandler extends ReplacedRoute { * @param path new route path * @param deprecatedMethod deprecated method * @param deprecatedPath deprecated path - * @param handler The method which handles the REST method and path. + * @param handler The method which handles the method and path. */ public ReplacedRouteHandler( Method method, String path, Method deprecatedMethod, String deprecatedPath, - Function handler + Function handler ) { super(method, path, deprecatedMethod, deprecatedPath); this.responseHandler = handler; @@ -52,9 +51,9 @@ public ReplacedRouteHandler( * @param method route method * @param path new route path * @param deprecatedPath deprecated path - * @param handler The method which handles the REST method and path. + * @param handler The method which handles the method and path. */ - public ReplacedRouteHandler(Method method, String path, String deprecatedPath, Function handler) { + public ReplacedRouteHandler(Method method, String path, String deprecatedPath, Function handler) { this(method, path, method, deprecatedPath, handler); } @@ -64,9 +63,9 @@ public ReplacedRouteHandler(Method method, String path, String deprecatedPath, F * @param route route * @param prefix new route prefix * @param deprecatedPrefix deprecated prefix - * @param handler The method which handles the REST method and path. + * @param handler The method which handles the method and path. */ - public ReplacedRouteHandler(Route route, String prefix, String deprecatedPrefix, Function handler) { + public ReplacedRouteHandler(Route route, String prefix, String deprecatedPrefix, Function handler) { this(route.getMethod(), prefix + route.getPath(), deprecatedPrefix + route.getPath(), handler); } @@ -77,6 +76,6 @@ public ReplacedRouteHandler(Route route, String prefix, String deprecatedPrefix, * @return the {@link ExtensionRestResponse} result from the handler for this route. */ public ExtensionRestResponse handleRequest(RestRequest request) { - return (ExtensionRestResponse) responseHandler.apply(request); + return responseHandler.apply(request); } } 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 b2d9df0b..8673d48c 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,21 +15,16 @@ import org.opensearch.common.xcontent.XContentType; import org.opensearch.common.xcontent.json.JsonXContent; import org.opensearch.extensions.rest.ExtensionRestResponse; -import org.opensearch.rest.NamedRoute; import org.opensearch.rest.RestRequest; -import org.opensearch.rest.RestResponse; import org.opensearch.sdk.rest.BaseExtensionRestHandler; import org.opensearch.sdk.rest.ExtensionRestHandler; - import java.io.IOException; import java.net.URLDecoder; import java.nio.ByteBuffer; import java.nio.charset.StandardCharsets; import java.util.ArrayList; -import java.util.Collections; import java.util.List; import java.util.Random; -import java.util.Set; import java.util.function.Function; import static org.opensearch.rest.RestRequest.Method.DELETE; @@ -55,49 +50,24 @@ public class RestHelloAction extends BaseExtensionRestHandler { private List worldAdjectives = new ArrayList<>(); private Random rand = new Random(); - /** - * Instantiate this action - */ - public RestHelloAction() {} - @Override - public List routes() { + public List routeHandlers() { return List.of( - new NamedRoute.Builder().method(GET) - .path("/hello") - .handler(handleGetRequest) - .uniqueName(addRouteNamePrefix("greet")) - .legacyActionNames(Set.of("cluster:admin/opensearch/hw/greet")) - .build(), - new NamedRoute.Builder().method(POST) - .path("/hello") - .handler(handlePostRequest) - .uniqueName(addRouteNamePrefix("greet_with_adjective")) - .legacyActionNames(Collections.emptySet()) - .build(), - new NamedRoute.Builder().method(PUT) - .path("/hello/{name}") - .handler(handlePutRequest) - .uniqueName(addRouteNamePrefix("greet_with_name")) - .legacyActionNames(Set.of("cluster:admin/opensearch/hw/greet_with_name")) - .build(), - new NamedRoute.Builder().method(DELETE) - .path("/goodbye") - .handler(handleDeleteRequest) - .uniqueName(addRouteNamePrefix("goodbye")) - .legacyActionNames(Collections.emptySet()) - .build() + new RouteHandler(GET, "/hello", handleGetRequest), + new RouteHandler(POST, "/hello", handlePostRequest), + new RouteHandler(PUT, "/hello/{name}", handlePutRequest), + new RouteHandler(DELETE, "/goodbye", handleDeleteRequest) ); } - private Function handleGetRequest = (request) -> { + private Function 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 handlePostRequest = (request) -> { + private Function handlePostRequest = (request) -> { if (request.hasContent()) { String adjective = ""; XContentType contentType = request.getXContentType(); @@ -145,7 +115,7 @@ public List routes() { return new ExtensionRestResponse(request, BAD_REQUEST, TEXT_CONTENT_TYPE, noContent); }; - private Function handlePutRequest = (request) -> { + private Function handlePutRequest = (request) -> { String name = request.param("name"); try { worldName = URLDecoder.decode(name, StandardCharsets.UTF_8); @@ -155,7 +125,7 @@ public List routes() { return new ExtensionRestResponse(request, OK, "Updated the world's name to " + worldName); }; - private Function handleDeleteRequest = (request) -> { + private Function handleDeleteRequest = (request) -> { this.worldName = DEFAULT_NAME; this.worldAdjectives.clear(); return new ExtensionRestResponse(request, OK, "Goodbye, cruel world! Restored default values."); diff --git a/src/main/java/org/opensearch/sdk/sample/helloworld/rest/RestRemoteHelloAction.java b/src/main/java/org/opensearch/sdk/sample/helloworld/rest/RestRemoteHelloAction.java index ac69e08f..cf4952f7 100644 --- a/src/main/java/org/opensearch/sdk/sample/helloworld/rest/RestRemoteHelloAction.java +++ b/src/main/java/org/opensearch/sdk/sample/helloworld/rest/RestRemoteHelloAction.java @@ -14,9 +14,7 @@ import org.opensearch.extensions.ExtensionsManager; import org.opensearch.extensions.action.RemoteExtensionActionResponse; import org.opensearch.extensions.rest.ExtensionRestResponse; -import org.opensearch.rest.NamedRoute; import org.opensearch.rest.RestRequest; -import org.opensearch.rest.RestResponse; import org.opensearch.sdk.ExtensionsRunner; import org.opensearch.sdk.SDKClient; import org.opensearch.sdk.action.RemoteExtensionAction; @@ -26,7 +24,6 @@ import org.opensearch.sdk.sample.helloworld.transport.SampleRequest; import org.opensearch.sdk.sample.helloworld.transport.SampleResponse; -import java.util.Collections; import java.util.List; import java.util.concurrent.CompletableFuture; import java.util.concurrent.TimeUnit; @@ -52,19 +49,11 @@ public RestRemoteHelloAction(ExtensionsRunner runner) { } @Override - public List routes() { - return List.of( - - new NamedRoute.Builder().method(GET) - .path("/hello/{name}") - .handler(handleRemoteGetRequest) - .uniqueName(addRouteNamePrefix("remote_greet_with_name")) - .legacyActionNames(Collections.emptySet()) - .build() - ); + public List routeHandlers() { + return List.of(new RouteHandler(GET, "/hello/{name}", handleRemoteGetRequest)); } - private Function handleRemoteGetRequest = (request) -> { + private Function handleRemoteGetRequest = (request) -> { SDKClient client = extensionsRunner.getSdkClient(); String name = request.param("name"); @@ -91,7 +80,7 @@ public List routes() { TimeUnit.SECONDS ).get(); if (!response.isSuccess()) { - return new ExtensionRestResponse(request, OK, "Remote extension response failed: " + response.getResponseBytesAsString()); + return new ExtensionRestResponse(request, OK, "Remote extension reponse failed: " + response.getResponseBytesAsString()); } // Parse out the expected response class from the bytes SampleResponse sampleResponse = new SampleResponse(StreamInput.wrap(response.getResponseBytes())); diff --git a/src/test/java/org/opensearch/sdk/rest/TestBaseExtensionRestHandler.java b/src/test/java/org/opensearch/sdk/rest/TestBaseExtensionRestHandler.java index 3c375331..7fa02c83 100644 --- a/src/test/java/org/opensearch/sdk/rest/TestBaseExtensionRestHandler.java +++ b/src/test/java/org/opensearch/sdk/rest/TestBaseExtensionRestHandler.java @@ -17,56 +17,35 @@ import org.junit.jupiter.api.Test; import org.opensearch.common.bytes.BytesArray; import org.opensearch.extensions.rest.ExtensionRestResponse; -import org.opensearch.rest.NamedRoute; import org.opensearch.rest.RestHandler.Route; import org.opensearch.rest.RestRequest; import org.opensearch.rest.RestRequest.Method; -import org.opensearch.rest.RestResponse; import org.opensearch.rest.RestStatus; import org.opensearch.test.OpenSearchTestCase; -import static org.opensearch.rest.RestRequest.Method.GET; - public class TestBaseExtensionRestHandler extends OpenSearchTestCase { private final BaseExtensionRestHandler handler = new BaseExtensionRestHandler() { @Override - public List routes() { - return List.of( - new NamedRoute.Builder().method(GET) - .path("/foo") - .handler(handleFoo) - .uniqueName("foo") - .legacyActionNames(Collections.emptySet()) - .build() - ); + public List routeHandlers() { + return List.of(new RouteHandler(Method.GET, "/foo", handleFoo)); } @Override public List deprecatedRouteHandlers() { - return List.of(new DeprecatedRouteHandler(GET, "/deprecated/foo", "It's deprecated", handleBar)); + return List.of(new DeprecatedRouteHandler(Method.GET, "/deprecated/foo", "It's deprecated", handleFoo)); } @Override public List replacedRouteHandlers() { return List.of( - new ReplacedRouteHandler(GET, "/new/foo", GET, "/old/foo", handleBar), - new ReplacedRouteHandler(Method.PUT, "/new/put/foo", "/old/put/foo", handleBar), - new ReplacedRouteHandler(new Route(Method.POST, "/foo"), "/new", "/old", handleBar) + new ReplacedRouteHandler(Method.GET, "/new/foo", Method.GET, "/old/foo", handleFoo), + new ReplacedRouteHandler(Method.PUT, "/new/put/foo", "/old/put/foo", handleFoo), + new ReplacedRouteHandler(new Route(Method.POST, "/foo"), "/new", "/old", handleFoo) ); } - private final Function handleFoo = (request) -> { - try { - if ("foo".equals(request.content().utf8ToString())) { - return createJsonResponse(request, RestStatus.OK, "success", "named foo"); - } - throw new IllegalArgumentException("no foo"); - } catch (Exception e) { - return exceptionalRequest(request, e); - } - }; - private final Function handleBar = (request) -> { + private final Function handleFoo = (request) -> { try { if ("bar".equals(request.content().utf8ToString())) { return createJsonResponse(request, RestStatus.OK, "success", "bar"); @@ -83,12 +62,13 @@ public void testHandlerDefaultRoutes() { BaseExtensionRestHandler defaultHandler = new BaseExtensionRestHandler() { }; assertTrue(defaultHandler.routes().isEmpty()); + assertTrue(defaultHandler.routeHandlers().isEmpty()); } @Test public void testJsonResponse() { RestRequest successfulRequest = TestSDKRestRequest.createTestRestRequest( - GET, + Method.GET, "/foo", "/foo", Collections.emptyMap(), @@ -106,7 +86,7 @@ public void testJsonResponse() { @Test public void testJsonDeprecatedResponse() { RestRequest successfulRequest = TestSDKRestRequest.createTestRestRequest( - GET, + Method.GET, "/deprecated/foo", "/deprecated/foo", Collections.emptyMap(), @@ -124,7 +104,7 @@ public void testJsonDeprecatedResponse() { @Test public void testJsonReplacedResponseGet() { RestRequest successfulRequestOld = TestSDKRestRequest.createTestRestRequest( - GET, + Method.GET, "/old/foo", "/old/foo", Collections.emptyMap(), @@ -135,7 +115,7 @@ public void testJsonReplacedResponseGet() { null ); RestRequest successfulRequestNew = TestSDKRestRequest.createTestRestRequest( - GET, + Method.GET, "/new/foo", "/new/foo", Collections.emptyMap(), @@ -223,7 +203,7 @@ public void testJsonReplacedResponsePost() { @Test public void testErrorResponseOnException() { RestRequest exceptionalRequest = TestSDKRestRequest.createTestRestRequest( - GET, + Method.GET, "/foo", "/foo", Collections.emptyMap(), @@ -263,7 +243,7 @@ public void testErrorResponseOnUnhandled() { ); RestRequest unhandledRequestPath = TestSDKRestRequest.createTestRestRequest( - GET, + Method.GET, "foobar", "foobar", Collections.emptyMap(), @@ -289,25 +269,18 @@ public void testErrorResponseOnUnhandled() { public void testCreateEmptyJsonResponse() { BaseExtensionRestHandler handlerWithEmptyJsonResponse = new BaseExtensionRestHandler() { @Override - public List routes() { - return List.of( - new NamedRoute.Builder().method(GET) - .path("/emptyJsonResponse") - .handler(handleEmptyJsonResponse) - .uniqueName("emptyresponse") - .legacyActionNames(Collections.emptySet()) - .build() - ); + public List routeHandlers() { + return List.of(new RouteHandler(Method.GET, "/emptyJsonResponse", handleEmptyJsonResponse)); } - private final Function handleEmptyJsonResponse = (request) -> createEmptyJsonResponse( + private final Function handleEmptyJsonResponse = (request) -> createEmptyJsonResponse( request, RestStatus.OK ); }; RestRequest emptyJsonResponseRequest = TestSDKRestRequest.createTestRestRequest( - GET, + Method.GET, "/emptyJsonResponse", "/emptyJsonResponse", Collections.emptyMap(), diff --git a/src/test/java/org/opensearch/sdk/rest/TestExtensionRestPathRegistry.java b/src/test/java/org/opensearch/sdk/rest/TestExtensionRestPathRegistry.java index 008214d0..8d794cb7 100644 --- a/src/test/java/org/opensearch/sdk/rest/TestExtensionRestPathRegistry.java +++ b/src/test/java/org/opensearch/sdk/rest/TestExtensionRestPathRegistry.java @@ -9,16 +9,14 @@ package org.opensearch.sdk.rest; -import java.util.Collections; import java.util.List; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.opensearch.extensions.rest.ExtensionRestResponse; -import org.opensearch.rest.NamedRoute; -import org.opensearch.rest.RestHandler.Route; import org.opensearch.rest.RestHandler.DeprecatedRoute; import org.opensearch.rest.RestHandler.ReplacedRoute; +import org.opensearch.rest.RestHandler.Route; import org.opensearch.rest.RestRequest; import org.opensearch.rest.RestRequest.Method; import org.opensearch.sdk.rest.BaseExtensionRestHandler.ExtensionDeprecationRestHandler; @@ -30,8 +28,8 @@ public class TestExtensionRestPathRegistry extends OpenSearchTestCase { private ExtensionRestHandler fooHandler = new ExtensionRestHandler() { @Override - public List routes() { - return List.of(new NamedRoute.Builder().method(Method.GET).path("/foo").uniqueName("foo").build()); + public List routes() { + return List.of(new Route(Method.GET, "/foo")); } @Override @@ -48,16 +46,18 @@ public ExtensionRestResponse handleRequest(RestRequest request) { @Override public List replacedRoutes() { return List.of( - new ReplacedRouteHandler(Method.GET, "/new/foo", Method.GET, "/old/foo", r -> null), - new ReplacedRouteHandler(Method.PUT, "/new/put/foo", "/old/put/foo", r -> null), - new ReplacedRouteHandler(new Route(Method.POST, "/foo"), "/new", "/old", r -> null) + new ReplacedRouteHandler(Method.GET, "/new/foo", Method.GET, "/old/foo", r -> { return null; }), + new ReplacedRouteHandler(Method.PUT, "/new/put/foo", "/old/put/foo", r -> { + return null; + }), + new ReplacedRouteHandler(new Route(Method.POST, "/foo"), "/new", "/old", r -> { return null; }) ); } }; private ExtensionRestHandler barHandler = new ExtensionRestHandler() { @Override - public List routes() { - return List.of(new NamedRoute.Builder().method(Method.PUT).path("/bar/{planet}").uniqueName("bar_planet").build()); + public List routes() { + return List.of(new Route(Method.PUT, "/bar/{planet}")); } @Override @@ -67,11 +67,8 @@ public ExtensionRestResponse handleRequest(RestRequest request) { }; private ExtensionRestHandler bazHandler = new ExtensionRestHandler() { @Override - public List routes() { - return List.of( - new NamedRoute.Builder().method(Method.POST).path("/baz/{moon}/qux").uniqueName("bar_qux_for_moon").build(), - new NamedRoute.Builder().method(Method.PUT).path("/bar/baz").uniqueName("bar_baz").build() - ); + public List routes() { + return List.of(new Route(Method.POST, "/baz/{moon}/qux"), new Route(Method.PUT, "/bar/baz")); } @Override @@ -95,8 +92,8 @@ public void testRegisterConflicts() { // Can't register same exact name ExtensionRestHandler duplicateFooHandler = new ExtensionRestHandler() { @Override - public List routes() { - return List.of(new NamedRoute.Builder().method(Method.GET).path("/foo").uniqueName("foo").build()); + public List routes() { + return List.of(new Route(Method.GET, "/foo")); } @Override @@ -108,8 +105,8 @@ public ExtensionRestResponse handleRequest(RestRequest request) { // Can't register conflicting named wildcards, even if method is different ExtensionRestHandler barNoneHandler = new ExtensionRestHandler() { @Override - public List routes() { - return List.of(new NamedRoute.Builder().method(Method.GET).path("/bar/{none}").uniqueName("bar_none").build()); + public List routes() { + return List.of(new Route(Method.GET, "/bar/{none}")); } @Override @@ -184,11 +181,6 @@ public void testGetRegisteredReplacedPaths() { @Test public void testRestPathToString() { - assertEquals("GET /foo", ExtensionRestPathRegistry.restPathToString(Method.GET, "/foo", "", Collections.emptySet())); - } - - @Test - public void testRestPathWithNameToString() { - assertEquals("GET /foo foo", ExtensionRestPathRegistry.restPathToString(Method.GET, "/foo", "foo", Collections.emptySet())); + assertEquals("GET /foo", ExtensionRestPathRegistry.restPathToString(Method.GET, "/foo")); } } diff --git a/src/test/java/org/opensearch/sdk/rest/TestNamedRouteHandler.java b/src/test/java/org/opensearch/sdk/rest/TestNamedRouteHandler.java deleted file mode 100644 index cf557d11..00000000 --- a/src/test/java/org/opensearch/sdk/rest/TestNamedRouteHandler.java +++ /dev/null @@ -1,45 +0,0 @@ -/* - * Copyright OpenSearch Contributors - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - */ - -package org.opensearch.sdk.rest; - -import org.opensearch.extensions.rest.ExtensionRestResponse; -import org.opensearch.rest.NamedRoute; -import org.opensearch.rest.RestStatus; -import org.opensearch.test.OpenSearchTestCase; - -import java.util.Collections; - -import static org.opensearch.rest.RestRequest.Method.GET; - -public class TestNamedRouteHandler extends OpenSearchTestCase { - public void testUnnamedRouteHandler() { - assertThrows( - IllegalArgumentException.class, - () -> new NamedRoute.Builder().method(GET) - .path("/foo/bar") - .handler(req -> new ExtensionRestResponse(req, RestStatus.OK, "content")) - .uniqueName("") - .legacyActionNames(Collections.emptySet()) - .build() - ); - } - - public void testNamedRouteHandler() { - NamedRoute nr = new NamedRoute.Builder().method(GET) - .path("/foo/bar") - .handler(req -> new ExtensionRestResponse(req, RestStatus.OK, "content")) - .uniqueName("") - .legacyActionNames(Collections.emptySet()) - .build(); - - assertEquals("foo", nr.name()); - assertNotNull(nr.handler()); - } -} 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 f45930f8..8c4ee0a2 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 @@ -16,7 +16,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.opensearch.rest.NamedRoute; +import org.opensearch.rest.RestHandler.Route; import org.opensearch.rest.RestRequest; import org.opensearch.rest.RestRequest.Method; import org.opensearch.common.bytes.BytesArray; @@ -47,7 +47,7 @@ public void setUp() throws Exception { @Test public void testRoutes() { - List routes = restHelloAction.routes(); + List routes = restHelloAction.routes(); assertEquals(4, routes.size()); assertEquals(Method.GET, routes.get(0).getMethod()); assertEquals("/hello", routes.get(0).getPath());