From 3eae59410a1c4adf6f2cdbde61b98f825d8f298f Mon Sep 17 00:00:00 2001 From: Owais Kazi Date: Tue, 11 Jul 2023 16:29:04 -0700 Subject: [PATCH] Revert "Makes route prefix setting `routeNamePrefix` optional (#868)" This reverts commit 9e31418e5642cd4772db8641d8baed2392480a16. --- DEVELOPER_GUIDE.md | 3 -- .../org/opensearch/sdk/ExtensionSettings.java | 20 ----------- .../org/opensearch/sdk/ExtensionsRunner.java | 2 +- .../sdk/rest/BaseExtensionRestHandler.java | 34 +++++-------------- .../helloworld/rest/RestHelloAction.java | 8 ++--- .../rest/RestRemoteHelloAction.java | 2 +- 6 files changed, 14 insertions(+), 55 deletions(-) diff --git a/DEVELOPER_GUIDE.md b/DEVELOPER_GUIDE.md index 43a3eefe..e9957f47 100644 --- a/DEVELOPER_GUIDE.md +++ b/DEVELOPER_GUIDE.md @@ -303,9 +303,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/src/main/java/org/opensearch/sdk/ExtensionSettings.java b/src/main/java/org/opensearch/sdk/ExtensionSettings.java index 52f08ce6..53ae7668 100644 --- a/src/main/java/org/opensearch/sdk/ExtensionSettings.java +++ b/src/main/java/org/opensearch/sdk/ExtensionSettings.java @@ -53,7 +53,6 @@ public class ExtensionSettings { private String hostPort; private String opensearchAddress; private String opensearchPort; - private String routeNamePrefix; private Map securitySettings; /** @@ -97,7 +96,6 @@ private ExtensionSettings() { * Instantiate this class using the specified parameters. * * @param extensionName The extension name. Provided to OpenSearch as a response to initialization query. Must match the defined extension name in OpenSearch. - * * @param hostAddress The IP Address to bind this extension to. * @param hostPort The port to bind this extension to. * @param opensearchAddress The IP Address on which OpenSearch is running. @@ -121,7 +119,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( @@ -130,11 +127,9 @@ public ExtensionSettings( String hostPort, String opensearchAddress, String opensearchPort, - String routeNamePrefix, Map securitySettings ) { this(extensionName, hostAddress, hostPort, opensearchAddress, opensearchPort); - this.routeNamePrefix = routeNamePrefix; this.securitySettings = securitySettings; } @@ -194,14 +189,6 @@ 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. @@ -252,19 +239,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 4c6f281f..68f60930 100644 --- a/src/main/java/org/opensearch/sdk/ExtensionsRunner.java +++ b/src/main/java/org/opensearch/sdk/ExtensionsRunner.java @@ -235,7 +235,7 @@ protected ExtensionsRunner(Extension extension) throws IOException { // store REST handlers in the registry for (ExtensionRestHandler extensionRestHandler : ((ActionExtension) extension).getExtensionRestHandlers()) { if (extensionRestHandler instanceof BaseExtensionRestHandler) { - ((BaseExtensionRestHandler) extensionRestHandler).setRouteNamePrefix(extensionSettings.getRoutePrefix()); + ((BaseExtensionRestHandler) extensionRestHandler).setExtensionName(extensionSettings.getExtensionName()); } 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 d4067011..b6fc7c8a 100644 --- a/src/main/java/org/opensearch/sdk/rest/BaseExtensionRestHandler.java +++ b/src/main/java/org/opensearch/sdk/rest/BaseExtensionRestHandler.java @@ -23,11 +23,8 @@ import java.util.function.Function; import static org.apache.hc.core5.http.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; @@ -44,9 +41,7 @@ */ public abstract class BaseExtensionRestHandler implements ExtensionRestHandler { - private static final String VALID_ROUTE_PREFIX_PATTERN = "^[a-zA-Z0-9_]*$"; - - private String routeNamePrefix; + private static String extensionName; /** * Constant for JSON content type @@ -81,30 +76,17 @@ 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; + public void setExtensionName(String extensionName) { + BaseExtensionRestHandler.extensionName = extensionName; } /** - * 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 + * Generates a name for the handler prepended with the extension's name + * @param name The human-readable name for a route registered by this extension + * @return Returns a name prepended with the extension's name */ - protected String addRouteNamePrefix(String routeName) { - if (Strings.isNullOrEmpty(routeNamePrefix)) { - return routeName; - } - return routeNamePrefix + ":" + routeName; + protected static String routePrefix(String name) { + return extensionName + ":" + name; } @Override 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..17a36c04 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 @@ -66,25 +66,25 @@ public List routes() { new NamedRoute.Builder().method(GET) .path("/hello") .handler(handleGetRequest) - .uniqueName(addRouteNamePrefix("greet")) + .uniqueName(routePrefix("greet")) .legacyActionNames(Set.of("cluster:admin/opensearch/hw/greet")) .build(), new NamedRoute.Builder().method(POST) .path("/hello") .handler(handlePostRequest) - .uniqueName(addRouteNamePrefix("greet_with_adjective")) + .uniqueName(routePrefix("greet_with_adjective")) .legacyActionNames(Collections.emptySet()) .build(), new NamedRoute.Builder().method(PUT) .path("/hello/{name}") .handler(handlePutRequest) - .uniqueName(addRouteNamePrefix("greet_with_name")) + .uniqueName(routePrefix("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")) + .uniqueName(routePrefix("goodbye")) .legacyActionNames(Collections.emptySet()) .build() ); 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..d3c12e42 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 @@ -58,7 +58,7 @@ public List routes() { new NamedRoute.Builder().method(GET) .path("/hello/{name}") .handler(handleRemoteGetRequest) - .uniqueName(addRouteNamePrefix("remote_greet_with_name")) + .uniqueName(routePrefix("remote_greet_with_name")) .legacyActionNames(Collections.emptySet()) .build() );