Skip to content

[Extensions] Renames AD extension routes to NamedRoutes #942

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.

Sign up for GitHub

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

Closed
DarshitChanpura wants to merge 3 commits into from
Closed

[Extensions] Renames AD extension routes to NamedRoutes #942

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
45c30fd
Renames AD extension routes to NamedRoutes and modifies replaced hand…
DarshitChanpura Jul 6, 2023
713b2af
Renames route names to conform to uris and avoids star imports
DarshitChanpura Jul 10, 2023
47bdc95
Changes the routePrefix method call to reflect correct name
DarshitChanpura Jul 11, 2023
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
27 changes: 12 additions & 15 deletions src/main/java/org/opensearch/ad/rest/AbstractSearchAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
import static org.opensearch.common.xcontent.XContentFactory.jsonBuilder;
import static org.opensearch.common.xcontent.XContentParserUtils.ensureExpectedToken;
import static org.opensearch.core.xcontent.ToXContent.EMPTY_PARAMS;
import static org.opensearch.rest.RestRequest.Method.GET;
import static org.opensearch.rest.RestRequest.Method.POST;

import java.io.IOException;
import java.util.ArrayList;
Expand Down Expand Up @@ -45,8 +47,10 @@
import org.opensearch.core.xcontent.XContentParser;
import org.opensearch.extensions.rest.ExtensionRestResponse;
import org.opensearch.rest.BytesRestResponse;
import org.opensearch.rest.NamedRoute;
import org.opensearch.rest.RestChannel;
import org.opensearch.rest.RestRequest;
import org.opensearch.rest.RestResponse;
import org.opensearch.rest.RestStatus;
import org.opensearch.sdk.ExtensionsRunner;
import org.opensearch.sdk.SDKClient.SDKRestClient;
Expand Down Expand Up @@ -88,7 +92,7 @@ public AbstractSearchAction(
this.extensionsRunner = extensionsRunner;
}

private Function<RestRequest, ExtensionRestResponse> handleRequest = (request) -> {
private Function<RestRequest, RestResponse> handleRequest = (request) -> {
try {
return prepareRequest(request);
} catch (Exception e) {
Expand Down Expand Up @@ -170,11 +174,13 @@ protected ExtensionRestResponse search(RestRequest request, SearchResponse respo
return new ExtensionRestResponse(request, RestStatus.OK, response.toXContent(JsonXContent.contentBuilder(), EMPTY_PARAMS));
}

public List<RouteHandler> routeHandlers() {
List<RouteHandler> routes = new ArrayList<>();
@Override
public List<NamedRoute> routes() {
List<NamedRoute> routes = new ArrayList<>();
for (String path : urlPaths) {
routes.add(new RouteHandler(RestRequest.Method.POST, path, handleRequest));
routes.add(new RouteHandler(RestRequest.Method.GET, path, handleRequest));
routes
.add(new NamedRoute.Builder().method(POST).path(path).uniqueName(addRouteNamePrefix(path)).handler(handleRequest).build());
routes.add(new NamedRoute.Builder().method(GET).path(path).uniqueName(addRouteNamePrefix(path)).handler(handleRequest).build());
}
return routes;
}
Expand All @@ -183,16 +189,7 @@ public List<RouteHandler> routeHandlers() {
public List<ReplacedRouteHandler> replacedRouteHandlers() {
List<ReplacedRouteHandler> replacedRoutes = new ArrayList<>();
for (Pair<String, String> deprecatedPath : deprecatedPaths) {
replacedRoutes
.add(
new ReplacedRouteHandler(
RestRequest.Method.POST,
deprecatedPath.getKey(),
RestRequest.Method.POST,
deprecatedPath.getValue(),
handleRequest
)
);
replacedRoutes.add(new ReplacedRouteHandler(POST, deprecatedPath.getKey(), POST, deprecatedPath.getValue(), handleRequest));
replacedRoutes
.add(
new ReplacedRouteHandler(
Expand Down
3 changes: 2 additions & 1 deletion src/main/java/org/opensearch/ad/rest/RestAnomalyDetectorJobAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
import org.opensearch.jobscheduler.JobSchedulerPlugin;
import org.opensearch.jobscheduler.rest.request.GetJobDetailsRequest;
import org.opensearch.rest.RestRequest;
import org.opensearch.rest.RestResponse;
import org.opensearch.rest.RestStatus;
import org.opensearch.sdk.ExtensionsRunner;
import org.opensearch.sdk.SDKClient.SDKRestClient;
Expand Down Expand Up @@ -173,7 +174,7 @@ public List<ReplacedRouteHandler> replacedRouteHandlers() {
);
}

private Function<RestRequest, ExtensionRestResponse> handleRequest = (request) -> {
private Function<RestRequest, RestResponse> handleRequest = (request) -> {
try {
return prepareRequest(request);
} catch (Exception e) {
Expand Down
3 changes: 2 additions & 1 deletion src/main/java/org/opensearch/ad/rest/RestDeleteAnomalyDetectorAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import org.opensearch.core.xcontent.ToXContent;
import org.opensearch.extensions.rest.ExtensionRestResponse;
import org.opensearch.rest.RestRequest;
import org.opensearch.rest.RestResponse;
import org.opensearch.rest.RestStatus;
import org.opensearch.sdk.ExtensionsRunner;
import org.opensearch.sdk.SDKClient.SDKRestClient;
Expand Down Expand Up @@ -105,7 +106,7 @@ public List<ReplacedRouteHandler> replacedRouteHandlers() {
);
}

private Function<RestRequest, ExtensionRestResponse> handleRequest = (request) -> {
private Function<RestRequest, RestResponse> handleRequest = (request) -> {
Copy link
Copy Markdown
Member

@owaiskazi19 owaiskazi19 Jul 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trying to understand why are we changing the response type from ExtensionRestResponse to RestResponse?
If this is coming from NameRoute then we should change the return type of prepareRequest to RestResponse as well?

Copy link
Copy Markdown
Member Author

@DarshitChanpura DarshitChanpura Jul 11, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NamedRoute is available to both plugins and extensions, and hence RestResponse instead of ExtensionRestResponse (which inherits RestResponse transitively). prepareRequest should still return ExtensionRestResponse as it is one of the concrete implementation of RestResponse

Copy link
Copy Markdown
Member

@owaiskazi19 owaiskazi19 Jul 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On line 71

Copy link
Copy Markdown
Member Author

@DarshitChanpura DarshitChanpura Jul 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prepareRequest should still return ExtensionRestResponse as it is one of the concrete implementations of RestResponse and will be interpreted as such when send the response back to user. However, the only use case I see for ExtensionRestResponse is for communication between OS and extension to mark that the content was consumed and a list of consumed content.

@dbwiddis had some thoughts on whether we should keep ExtensionRestResponse or just use BytesRestReponse class

try {
return prepareRequest(request);
} catch (Exception e) {
Expand Down
17 changes: 14 additions & 3 deletions src/main/java/org/opensearch/ad/rest/RestDeleteAnomalyResultsAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
package org.opensearch.ad.rest;

import static org.opensearch.ad.indices.AnomalyDetectionIndices.ALL_AD_RESULTS_INDEX_PATTERN;
import static org.opensearch.rest.RestRequest.Method.DELETE;

import java.io.IOException;
import java.util.List;
Expand All @@ -34,7 +35,9 @@
import org.opensearch.extensions.rest.ExtensionRestResponse;
import org.opensearch.index.reindex.BulkByScrollResponse;
import org.opensearch.index.reindex.DeleteByQueryRequest;
import org.opensearch.rest.NamedRoute;
import org.opensearch.rest.RestRequest;
import org.opensearch.rest.RestResponse;
import org.opensearch.rest.RestStatus;
import org.opensearch.sdk.ExtensionsRunner;
import org.opensearch.sdk.SDKClient.SDKRestClient;
Expand Down Expand Up @@ -71,7 +74,7 @@ public String getName() {
return DELETE_AD_RESULTS_ACTION;
}

private Function<RestRequest, ExtensionRestResponse> handleRequest = (request) -> {
private Function<RestRequest, RestResponse> handleRequest = (request) -> {
try {
return prepareRequest(request);
} catch (Exception e) {
Expand Down Expand Up @@ -112,8 +115,16 @@ protected ExtensionRestResponse prepareRequest(RestRequest request) throws IOExc
}

@Override
public List<RouteHandler> routeHandlers() {
public List<NamedRoute> routes() {
String path = AnomalyDetectorExtension.AD_BASE_DETECTORS_URI + "/results";
return ImmutableList
.of(new RouteHandler(RestRequest.Method.DELETE, AnomalyDetectorExtension.AD_BASE_DETECTORS_URI + "/results", handleRequest));
.of(
new NamedRoute.Builder()
.method(DELETE)
.path(path)
.uniqueName(addRouteNamePrefix("detectors/results"))
.handler(handleRequest)
.build()
);
}
}
3 changes: 2 additions & 1 deletion src/main/java/org/opensearch/ad/rest/RestExecuteAnomalyDetectorAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import org.opensearch.core.xcontent.XContentParser;
import org.opensearch.extensions.rest.ExtensionRestResponse;
import org.opensearch.rest.RestRequest;
import org.opensearch.rest.RestResponse;
import org.opensearch.rest.RestStatus;
import org.opensearch.sdk.ExtensionsRunner;
import org.opensearch.sdk.SDKClient.SDKRestClient;
Expand Down Expand Up @@ -110,7 +111,7 @@ protected ExtensionRestResponse prepareRequest(RestRequest request) throws IOExc
);
}

private Function<RestRequest, ExtensionRestResponse> handleRequest = (request) -> {
private Function<RestRequest, RestResponse> handleRequest = (request) -> {
try {
return prepareRequest(request);
} catch (Exception e) {
Expand Down
30 changes: 17 additions & 13 deletions src/main/java/org/opensearch/ad/rest/RestGetAnomalyDetectorAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import static org.opensearch.ad.util.RestHandlerUtils.DETECTOR_ID;
import static org.opensearch.ad.util.RestHandlerUtils.PROFILE;
import static org.opensearch.ad.util.RestHandlerUtils.TYPE;
import static org.opensearch.rest.RestRequest.Method.POST;

import java.io.IOException;
import java.util.List;
Expand All @@ -40,7 +41,9 @@
import org.opensearch.core.common.Strings;
import org.opensearch.core.xcontent.ToXContent;
import org.opensearch.extensions.rest.ExtensionRestResponse;
import org.opensearch.rest.NamedRoute;
import org.opensearch.rest.RestRequest;
import org.opensearch.rest.RestResponse;
import org.opensearch.rest.RestStatus;
import org.opensearch.rest.action.RestActions;
import org.opensearch.sdk.ExtensionsRunner;
Expand Down Expand Up @@ -109,7 +112,7 @@ protected ExtensionRestResponse prepareRequest(RestRequest request) throws IOExc
return getAnomalyDetectorResponse(request, response);
}

private Function<RestRequest, ExtensionRestResponse> handleRequest = (request) -> {
private Function<RestRequest, RestResponse> handleRequest = (request) -> {
try {
return prepareRequest(request);
} catch (Exception e) {
Expand All @@ -119,21 +122,22 @@ protected ExtensionRestResponse prepareRequest(RestRequest request) throws IOExc
};

@Override
public List<RouteHandler> routeHandlers() {
public List<NamedRoute> routes() {
return ImmutableList
.of(
// Opensearch-only API. Considering users may provide entity in the search body, support POST as well.
new RouteHandler(
RestRequest.Method.POST,
String.format(Locale.ROOT, "%s/{%s}/%s", AnomalyDetectorExtension.AD_BASE_DETECTORS_URI, DETECTOR_ID, PROFILE),
handleRequest
),
new RouteHandler(
RestRequest.Method.POST,
String
.format(Locale.ROOT, "%s/{%s}/%s/{%s}", AnomalyDetectorExtension.AD_BASE_DETECTORS_URI, DETECTOR_ID, PROFILE, TYPE),
handleRequest
)
new NamedRoute.Builder()
.method(POST)
.path(String.format(Locale.ROOT, "%s/{%s}/%s", AnomalyDetectorExtension.AD_BASE_DETECTORS_URI, DETECTOR_ID, PROFILE))
.uniqueName(addRouteNamePrefix("detectors/profile"))
.handler(handleRequest)
.build(),
new NamedRoute.Builder()
.method(POST)
.path(String.format(Locale.ROOT, "%s/{%s}/%s", AnomalyDetectorExtension.AD_BASE_DETECTORS_URI, DETECTOR_ID, TYPE))
.uniqueName(addRouteNamePrefix("detectors/type"))
.handler(handleRequest)
.build()
);
}

Expand Down
3 changes: 2 additions & 1 deletion src/main/java/org/opensearch/ad/rest/RestIndexAnomalyDetectorAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import org.opensearch.extensions.rest.ExtensionRestResponse;
import org.opensearch.index.seqno.SequenceNumbers;
import org.opensearch.rest.RestRequest;
import org.opensearch.rest.RestResponse;
import org.opensearch.rest.RestStatus;
import org.opensearch.sdk.ExtensionsRunner;
import org.opensearch.sdk.SDKClient.SDKRestClient;
Expand Down Expand Up @@ -154,7 +155,7 @@ public List<ReplacedRouteHandler> replacedRouteHandlers() {
);
}

private Function<RestRequest, ExtensionRestResponse> handleRequest = (request) -> {
private Function<RestRequest, RestResponse> handleRequest = (request) -> {
try {
return prepareRequest(request);
} catch (Exception e) {
Expand Down
18 changes: 11 additions & 7 deletions src/main/java/org/opensearch/ad/rest/RestPreviewAnomalyDetectorAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

import static org.opensearch.ad.util.RestHandlerUtils.PREVIEW;
import static org.opensearch.common.xcontent.XContentParserUtils.ensureExpectedToken;
import static org.opensearch.rest.RestRequest.Method.POST;

import java.io.IOException;
import java.util.List;
Expand Down Expand Up @@ -40,7 +41,9 @@
import org.opensearch.core.xcontent.ToXContent;
import org.opensearch.core.xcontent.XContentParser;
import org.opensearch.extensions.rest.ExtensionRestResponse;
import org.opensearch.rest.NamedRoute;
import org.opensearch.rest.RestRequest;
import org.opensearch.rest.RestResponse;
import org.opensearch.rest.RestStatus;
import org.opensearch.sdk.ExtensionsRunner;
import org.opensearch.sdk.SDKClient.SDKRestClient;
Expand All @@ -67,7 +70,7 @@ public String getName() {
return PREVIEW_ANOMALY_DETECTOR_ACTION;
}

private Function<RestRequest, ExtensionRestResponse> handleRequest = (request) -> {
private Function<RestRequest, RestResponse> handleRequest = (request) -> {
try {
return prepareRequest(request);
} catch (Exception e) {
Expand Down Expand Up @@ -138,15 +141,16 @@ private String validateAdExecutionInput(AnomalyDetectorExecutionInput input) {
}

@Override
public List<RouteHandler> routeHandlers() {
public List<NamedRoute> routes() {
return ImmutableList
.of(
// preview detector
new RouteHandler(
RestRequest.Method.POST,
String.format(Locale.ROOT, "%s/%s", AnomalyDetectorExtension.AD_BASE_DETECTORS_URI, PREVIEW),
handleRequest
)
new NamedRoute.Builder()
.method(POST)
.path(String.format(String.format(Locale.ROOT, "%s/%s", AnomalyDetectorExtension.AD_BASE_DETECTORS_URI, PREVIEW)))
.uniqueName(addRouteNamePrefix("detectors/preview"))
.handler(handleRequest)
.build()
);
}

Expand Down
7 changes: 4 additions & 3 deletions src/main/java/org/opensearch/ad/rest/RestSearchAnomalyDetectorInfoAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,9 @@
import org.opensearch.common.xcontent.json.JsonXContent;
import org.opensearch.core.xcontent.ToXContent;
import org.opensearch.extensions.rest.ExtensionRestResponse;
import org.opensearch.rest.RestHandler;
import org.opensearch.rest.NamedRoute;
import org.opensearch.rest.RestRequest;
import org.opensearch.rest.RestResponse;
import org.opensearch.rest.RestStatus;
import org.opensearch.sdk.ExtensionsRunner;
import org.opensearch.sdk.SDKClient.SDKRestClient;
Expand All @@ -61,7 +62,7 @@ public String getName() {
return SEARCH_ANOMALY_DETECTOR_INFO_ACTION;
}

private Function<RestRequest, ExtensionRestResponse> handleRequest = (request) -> {
private Function<RestRequest, RestResponse> handleRequest = (request) -> {
try {
return prepareRequest(request);
} catch (Exception e) {
Expand Down Expand Up @@ -105,7 +106,7 @@ protected ExtensionRestResponse prepareRequest(RestRequest request) throws IOExc
}

@Override
public List<RestHandler.Route> routes() {
public List<NamedRoute> routes() {
return ImmutableList.of();
}

Expand Down
22 changes: 18 additions & 4 deletions src/main/java/org/opensearch/ad/rest/RestSearchTopAnomalyResultAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
package org.opensearch.ad.rest;

import static org.opensearch.common.xcontent.XContentParserUtils.ensureExpectedToken;
import static org.opensearch.rest.RestRequest.Method.GET;
import static org.opensearch.rest.RestRequest.Method.POST;

import java.io.IOException;
import java.util.List;
Expand All @@ -33,7 +35,9 @@
import org.opensearch.core.xcontent.ToXContent;
import org.opensearch.core.xcontent.XContentParser;
import org.opensearch.extensions.rest.ExtensionRestResponse;
import org.opensearch.rest.NamedRoute;
import org.opensearch.rest.RestRequest;
import org.opensearch.rest.RestResponse;
import org.opensearch.rest.RestStatus;
import org.opensearch.sdk.ExtensionsRunner;
import org.opensearch.sdk.SDKClient.SDKRestClient;
Expand Down Expand Up @@ -68,7 +72,7 @@ public String getName() {
return SEARCH_TOP_ANOMALY_DETECTOR_ACTION;
}

private Function<RestRequest, ExtensionRestResponse> handleRequest = (request) -> {
private Function<RestRequest, RestResponse> handleRequest = (request) -> {
try {
return prepareRequest(request);
} catch (Exception e) {
Expand Down Expand Up @@ -131,11 +135,21 @@ private SearchTopAnomalyResultRequest getSearchTopAnomalyResultRequest(RestReque
}

@Override
public List<RouteHandler> routeHandlers() {
public List<NamedRoute> routes() {
return ImmutableList
.of(
new RouteHandler(RestRequest.Method.POST, URL_PATH, handleRequest),
new RouteHandler(RestRequest.Method.GET, URL_PATH, handleRequest)
new NamedRoute.Builder()
.method(POST)
.path(URL_PATH)
.uniqueName(addRouteNamePrefix("search/post/topresults"))
.handler(handleRequest)
.build(),
new NamedRoute.Builder()
.method(GET)
.path(URL_PATH)
.uniqueName(addRouteNamePrefix("search/read/topresults"))
.handler(handleRequest)
.build()
);
}
}
3 changes: 2 additions & 1 deletion src/main/java/org/opensearch/ad/rest/RestStatsAnomalyDetectorAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
import org.opensearch.core.xcontent.XContentBuilder;
import org.opensearch.extensions.rest.ExtensionRestResponse;
import org.opensearch.rest.RestRequest;
import org.opensearch.rest.RestResponse;
import org.opensearch.rest.RestStatus;
import org.opensearch.sdk.ExtensionsRunner;
import org.opensearch.sdk.SDKClient.SDKRestClient;
Expand Down Expand Up @@ -115,7 +116,7 @@ public List<ReplacedRouteHandler> replacedRouteHandlers() {
);
}

private Function<RestRequest, ExtensionRestResponse> handleRequest = (request) -> {
private Function<RestRequest, RestResponse> handleRequest = (request) -> {
try {
return prepareRequest(request);
} catch (Exception e) {
Expand Down
Loading