Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Makes route prefix setting routeNamePrefix optional #868

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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.
DarshitChanpura marked this conversation as resolved.
Show resolved Hide resolved
The value must be alphanumeric and can contain `_` in the name.

DarshitChanpura marked this conversation as resolved.
Show resolved Hide resolved
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,
DarshitChanpura marked this conversation as resolved.
Show resolved Hide resolved
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