From 982b6669f52a3354088dd1e9a0dc39297543144e Mon Sep 17 00:00:00 2001 From: Darshit Chanpura <35282393+DarshitChanpura@users.noreply.github.com> Date: Tue, 11 Jul 2023 19:32:39 -0400 Subject: [PATCH] Revert "[Backport 1.x] Register new routes via SDK as named routes (#827) and Makes route prefix setting routeNamePrefix optional #868 (#866)" This reverts commit 32d65675a84aa22f6c2cd62a315e37d938b1ea63. --- CREATE_YOUR_FIRST_EXTENSION.md | 20 ++-- DEVELOPER_GUIDE.md | 39 +++---- PLUGIN_MIGRATION.md | 28 +---- build.gradle | 26 ++--- config/certs/cert-gen.sh | 34 ------ .../org/opensearch/sdk/ExtensionSettings.java | 59 +--------- .../org/opensearch/sdk/ExtensionsRunner.java | 4 - .../sdk/rest/BaseExtensionRestHandler.java | 105 ++++++++++-------- .../sdk/rest/ExtensionRestHandler.java | 3 +- .../sdk/rest/ExtensionRestPathRegistry.java | 46 ++------ .../sdk/rest/ReplacedRouteHandler.java | 17 ++- .../helloworld/rest/RestHelloAction.java | 48 ++------ .../rest/RestRemoteHelloAction.java | 19 +--- .../rest/TestBaseExtensionRestHandler.java | 63 +++-------- .../rest/TestExtensionRestPathRegistry.java | 42 +++---- .../sdk/rest/TestNamedRouteHandler.java | 45 -------- .../helloworld/rest/TestRestHelloAction.java | 4 +- 17 files changed, 167 insertions(+), 435 deletions(-) delete mode 100755 config/certs/cert-gen.sh delete mode 100644 src/test/java/org/opensearch/sdk/rest/TestNamedRouteHandler.java diff --git a/CREATE_YOUR_FIRST_EXTENSION.md b/CREATE_YOUR_FIRST_EXTENSION.md index 643efc07d..53d49fc14 100644 --- a/CREATE_YOUR_FIRST_EXTENSION.md +++ b/CREATE_YOUR_FIRST_EXTENSION.md @@ -164,28 +164,28 @@ 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("extension1:sample/create").handler(createHandler).build(), - new NamedRoute.Builder().method(Method.GET).path("/sample/{id}").uniqueName("extension1:sample/get").handler(readHandler).build(), - new NamedRoute.Builder().method(Method.POST).path("/sample/{id}").uniqueName("extension1:sample/post").handler(updateHandler).build(), - new NamedRoute.Builder().method(Method.DELETE).path("/sample/{id}").uniqueName("extension1: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"); }; } @@ -248,7 +248,7 @@ return createJsonResponse(request, RestStatus.OK, "_id", response.id()); Finally, you have the following code: ```java -Function createHandler = (request) -> { +Function createHandler = (request) -> { IndexResponse response; try { BooleanResponse exists = client.indices().exists(new ExistsRequest.Builder().index("crudsample").build()); diff --git a/DEVELOPER_GUIDE.md b/DEVELOPER_GUIDE.md index 43a3eefe0..021ebc7a9 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 5be87b611..bfb134c39 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 a7122ac89..172b86e69 100644 --- a/build.gradle +++ b/build.gradle @@ -147,7 +147,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 +187,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}") } } @@ -324,7 +320,7 @@ task closeTestExtension (type: Exec) { tasks.named("integTest").configure { finalizedBy(closeTestExtension) } testClusters.integTest { - extension(true) + extension(new ExtensionsProperties("${testExtensionYml.name}", "${testExtensionYml.uniqueId}", "${testExtensionYml.hostAddress}", "${testExtensionYml.port}", "${testExtensionYml.version}", "${testExtensionYml.opensearchVersion}", "${testExtensionYml.minimumCompatibleVersion}")) testDistribution = "ARCHIVE" // Cluster shrink exception thrown if we try to set numberOfNodes to 1, so only apply if > 1 if (_numNodes > 1) numberOfNodes = _numNodes diff --git a/config/certs/cert-gen.sh b/config/certs/cert-gen.sh deleted file mode 100755 index 2547511b6..000000000 --- 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 f366faeda..50c0993bd 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 b884b79d0..7cb1f6f21 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 c72b8a21b..524d39410 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 5cb2e5021..ed4d3c873 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 0a8cc12ca..3dd76950e 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 1fdab92b2..99d4c5ab6 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 b2d9df0b0..8673d48c7 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 ac69e08f8..cf4952f7e 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 3c375331d..7fa02c83d 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 008214d03..8d794cb79 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 cf557d117..000000000 --- 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 f45930f82..8c4ee0a22 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());