Skip to content

Commit

Permalink
Revert "Makes route prefix setting routeNamePrefix optional (#868)"
Browse files Browse the repository at this point in the history
This reverts commit 9e31418.
  • Loading branch information
owaiskazi19 authored Jul 11, 2023
1 parent 37bdb5a commit 3eae594
Show file tree
Hide file tree
Showing 6 changed files with 14 additions and 55 deletions.
3 changes: 0 additions & 3 deletions DEVELOPER_GUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
20 changes: 0 additions & 20 deletions src/main/java/org/opensearch/sdk/ExtensionSettings.java
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ public class ExtensionSettings {
private String hostPort;
private String opensearchAddress;
private String opensearchPort;
private String routeNamePrefix;
private Map<String, String> securitySettings;

/**
Expand Down Expand Up @@ -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.
Expand All @@ -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(
Expand All @@ -130,11 +127,9 @@ 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 @@ -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.
Expand Down Expand Up @@ -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) {
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).setRouteNamePrefix(extensionSettings.getRoutePrefix());
((BaseExtensionRestHandler) extensionRestHandler).setExtensionName(extensionSettings.getExtensionName());
}
extensionRestPathRegistry.registerHandler(extensionRestHandler);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -81,30 +76,17 @@ protected List<ReplacedRouteHandler> 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
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(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()
);
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(addRouteNamePrefix("remote_greet_with_name"))
.uniqueName(routePrefix("remote_greet_with_name"))
.legacyActionNames(Collections.emptySet())
.build()
);
Expand Down

0 comments on commit 3eae594

Please sign in to comment.