Skip to content

Commit

Permalink
Makes route prefix setting routeNamePrefix optional (opensearch-pro…
Browse files Browse the repository at this point in the history
…ject#868)

* Renames demo extension to `helloWorld`

Signed-off-by: Darshit Chanpura <[email protected]>

* Updates uniqueId to not reflect the change

Signed-off-by: Darshit Chanpura <[email protected]>

* Adds a validation check for setting extension name

Signed-off-by: Darshit Chanpura <[email protected]>

* Fixes checkstyle errors

Signed-off-by: Darshit Chanpura <[email protected]>

* Adds routePrefix as an optional setting in .yml

Signed-off-by: Darshit Chanpura <[email protected]>

* Updates dev guide to add documentation for the optional setting `routePrefix`

Signed-off-by: Darshit Chanpura <[email protected]>

* Renames `routePrefix` to `routeNamePrefix`

Signed-off-by: Darshit Chanpura <[email protected]>

* Renames erroneous instance of `hello-world`

Signed-off-by: Darshit Chanpura <[email protected]>

* Addresses PR feedback

Signed-off-by: Darshit Chanpura <[email protected]>
(cherry picked from commit 0103ada30250693c7b986a9667fd0060b414a66e)

* Changes allowed route name prefixes

Signed-off-by: Darshit Chanpura <[email protected]>

* Fixes error message and removes hyphen from allowed characters in route name prefix

Signed-off-by: Darshit Chanpura <[email protected]>

---------

Signed-off-by: Darshit Chanpura <[email protected]>
  • Loading branch information
DarshitChanpura authored Jul 11, 2023
1 parent a898727 commit 9e31418
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 14 deletions.
3 changes: 3 additions & 0 deletions DEVELOPER_GUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,9 @@ 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
Expand Down
20 changes: 20 additions & 0 deletions src/main/java/org/opensearch/sdk/ExtensionSettings.java
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ public class ExtensionSettings {
private String hostPort;
private String opensearchAddress;
private String opensearchPort;
private String routeNamePrefix;
private Map<String, String> securitySettings;

/**
Expand Down Expand Up @@ -96,6 +97,7 @@ 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.
Expand All @@ -119,6 +121,7 @@ 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(
Expand All @@ -127,9 +130,11 @@ public ExtensionSettings(
String hostPort,
String opensearchAddress,
String opensearchPort,
String routeNamePrefix,
Map<String, String> securitySettings
) {
this(extensionName, hostAddress, hostPort, opensearchAddress, opensearchPort);
this.routeNamePrefix = routeNamePrefix;
this.securitySettings = securitySettings;
}

Expand Down Expand Up @@ -189,6 +194,14 @@ 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.
Expand Down Expand Up @@ -239,12 +252,19 @@ 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) {
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/opensearch/sdk/ExtensionsRunner.java
Original file line number Diff line number Diff line change
Expand Up @@ -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).setExtensionName(extensionSettings.getExtensionName());
((BaseExtensionRestHandler) extensionRestHandler).setRouteNamePrefix(extensionSettings.getRoutePrefix());
}
extensionRestPathRegistry.registerHandler(extensionRestHandler);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,11 @@
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;
Expand All @@ -41,7 +44,9 @@
*/
public abstract class BaseExtensionRestHandler implements ExtensionRestHandler {

private static String extensionName;
private static final String VALID_ROUTE_PREFIX_PATTERN = "^[a-zA-Z0-9_]*$";

private String routeNamePrefix;

/**
* Constant for JSON content type
Expand Down Expand Up @@ -76,17 +81,30 @@ protected List<ReplacedRouteHandler> replacedRouteHandlers() {
return Collections.emptyList();
}

public void setExtensionName(String extensionName) {
BaseExtensionRestHandler.extensionName = extensionName;
/**
* 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 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
* 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 static String routePrefix(String name) {
return extensionName + ":" + name;
protected String addRouteNamePrefix(String routeName) {
if (Strings.isNullOrEmpty(routeNamePrefix)) {
return routeName;
}
return routeNamePrefix + ":" + routeName;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,25 +66,25 @@ public List<NamedRoute> routes() {
new NamedRoute.Builder().method(GET)
.path("/hello")
.handler(handleGetRequest)
.uniqueName(routePrefix("greet"))
.uniqueName(addRouteNamePrefix("greet"))
.legacyActionNames(Set.of("cluster:admin/opensearch/hw/greet"))
.build(),
new NamedRoute.Builder().method(POST)
.path("/hello")
.handler(handlePostRequest)
.uniqueName(routePrefix("greet_with_adjective"))
.uniqueName(addRouteNamePrefix("greet_with_adjective"))
.legacyActionNames(Collections.emptySet())
.build(),
new NamedRoute.Builder().method(PUT)
.path("/hello/{name}")
.handler(handlePutRequest)
.uniqueName(routePrefix("greet_with_name"))
.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(routePrefix("goodbye"))
.uniqueName(addRouteNamePrefix("goodbye"))
.legacyActionNames(Collections.emptySet())
.build()
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public List<NamedRoute> routes() {
new NamedRoute.Builder().method(GET)
.path("/hello/{name}")
.handler(handleRemoteGetRequest)
.uniqueName(routePrefix("remote_greet_with_name"))
.uniqueName(addRouteNamePrefix("remote_greet_with_name"))
.legacyActionNames(Collections.emptySet())
.build()
);
Expand Down

0 comments on commit 9e31418

Please sign in to comment.