Skip to content

Commit 06dda4c

Browse files
dbwiddisdbwiddis
committed
Use Function rather than Supplier for better thread safety
Signed-off-by: Daniel Widdis <[email protected]>
1 parent 833e52a commit 06dda4c

File tree

3 files changed

+36
-98
lines changed

3 files changed

+36
-98
lines changed

‎src/main/java/org/opensearch/sdk/BaseExtensionRestHandler.java

+6-73
Original file line numberDiff line numberDiff line change
@@ -14,20 +14,15 @@
1414
import java.util.List;
1515
import java.util.Optional;
1616

17-
import org.opensearch.common.bytes.BytesReference;
18-
import org.opensearch.common.xcontent.XContentBuilder;
1917
import org.opensearch.extensions.rest.ExtensionRestRequest;
2018
import org.opensearch.extensions.rest.ExtensionRestResponse;
2119
import org.opensearch.rest.RestHandler.Route;
22-
import org.opensearch.rest.RestStatus;
2320

2421
/**
2522
* Provides convenience methods to reduce boilerplate code in an {@link ExtensionRestHandler} implementation.
2623
*/
2724
public abstract class BaseExtensionRestHandler implements ExtensionRestHandler {
2825

29-
private ExtensionRestRequest request;
30-
3126
/**
3227
* Defines a list of methods which handle each rest {@link Route}.
3328
*
@@ -42,12 +37,11 @@ public List<Route> routes() {
4237

4338
@Override
4439
public ExtensionRestResponse handleRequest(ExtensionRestRequest request) {
45-
this.request = request;
4640
Optional<RouteHandler> handler = routeHandlers().stream()
4741
.filter(rh -> rh.getMethod().equals(request.method()))
4842
.filter(rh -> restPathMatches(request.path(), rh.getPath()))
4943
.findFirst();
50-
return handler.isPresent() ? handler.get().getExtensionRestResponse() : unhandledRequest();
44+
return handler.isPresent() ? handler.get().getExtensionRestResponse(request) : unhandledRequest(request);
5145
}
5246

5347
/**
@@ -77,74 +71,13 @@ private boolean restPathMatches(String requestPath, String handlerPath) {
7771
}
7872

7973
/**
80-
* Returns a default response when a request does not match the handled methods or paths. This can occur if a
81-
* handler indicates routes that it handles but does not actually handle them.
74+
* Returns a default response when a request does not match the handled methods or paths.
75+
* This can occur if a handler indicates routes that it handles but does not actually handle them.
8276
*
77+
* @param request The request that couldn't be handled.
8378
* @return an ExtensionRestResponse identifying the unhandled request.
8479
*/
85-
protected ExtensionRestResponse unhandledRequest() {
86-
return createResponse(NOT_FOUND, "Extension REST action improperly configured to handle " + getRequest().toString());
87-
}
88-
89-
/**
90-
* Creates an {@link ExtensionRestResponse} with the given status and Xcontent.
91-
*
92-
* @param status The {@link RestStatus} for the response.
93-
* @param builder An {@link XContentBuilder} for the response.
94-
* @return the response.
95-
*/
96-
protected ExtensionRestResponse createResponse(RestStatus status, XContentBuilder builder) {
97-
return new ExtensionRestResponse(getRequest(), status, builder);
98-
}
99-
100-
/**
101-
* Creates an {@link ExtensionRestResponse} with the given status and text content.
102-
*
103-
* @param status The {@link RestStatus} for the response.
104-
* @param content The content.
105-
* @return the response.
106-
*/
107-
protected ExtensionRestResponse createResponse(RestStatus status, String content) {
108-
return new ExtensionRestResponse(getRequest(), status, content);
109-
}
110-
111-
/**
112-
* Creates an {@link ExtensionRestResponse} with the given status and content.
113-
*
114-
* @param status The {@link RestStatus} for the response.
115-
* @param contentType The type of the content.
116-
* @param content The content.
117-
* @return the response.
118-
*/
119-
protected ExtensionRestResponse createResponse(RestStatus status, String contentType, String content) {
120-
return new ExtensionRestResponse(getRequest(), status, contentType, content);
121-
}
122-
123-
/**
124-
* Creates an {@link ExtensionRestResponse} with the given status and binary content.
125-
*
126-
* @param status The {@link RestStatus} for the response.
127-
* @param contentType The type of the content.
128-
* @param content The content.
129-
* @return the response.
130-
*/
131-
protected ExtensionRestResponse createResponse(RestStatus status, String contentType, byte[] content) {
132-
return new ExtensionRestResponse(getRequest(), status, contentType, content);
133-
}
134-
135-
/**
136-
* Creates an {@link ExtensionRestResponse} with the given status and binary content.
137-
*
138-
* @param status The {@link RestStatus} for the response.
139-
* @param contentType The type of the content.
140-
* @param content The content.
141-
* @return the response.
142-
*/
143-
protected ExtensionRestResponse createResponse(RestStatus status, String contentType, BytesReference content) {
144-
return new ExtensionRestResponse(getRequest(), status, contentType, content);
145-
}
146-
147-
public ExtensionRestRequest getRequest() {
148-
return request;
80+
protected ExtensionRestResponse unhandledRequest(ExtensionRestRequest request) {
81+
return new ExtensionRestResponse(request, NOT_FOUND, "Extension REST action improperly configured to handle " + request.toString());
14982
}
15083
}

‎src/main/java/org/opensearch/sdk/RouteHandler.java

+7-5
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,9 @@
99

1010
package org.opensearch.sdk;
1111

12-
import java.util.function.Supplier;
12+
import java.util.function.Function;
1313

14+
import org.opensearch.extensions.rest.ExtensionRestRequest;
1415
import org.opensearch.extensions.rest.ExtensionRestResponse;
1516
import org.opensearch.rest.RestHandler.Route;
1617
import org.opensearch.rest.RestRequest.Method;
@@ -20,7 +21,7 @@
2021
*/
2122
public class RouteHandler extends Route {
2223

23-
private final Supplier<ExtensionRestResponse> responseHandler;
24+
private final Function<ExtensionRestRequest, ExtensionRestResponse> responseHandler;
2425

2526
/**
2627
* Handle the method and path with the specified handler.
@@ -29,17 +30,18 @@ public class RouteHandler extends Route {
2930
* @param path The path to handle.
3031
* @param handler The method which handles the method and path.
3132
*/
32-
public RouteHandler(Method method, String path, Supplier<ExtensionRestResponse> handler) {
33+
public RouteHandler(Method method, String path, Function<ExtensionRestRequest, ExtensionRestResponse> handler) {
3334
super(method, path);
3435
this.responseHandler = handler;
3536
}
3637

3738
/**
3839
* Executes the handler for this route.
3940
*
41+
* @param request The request to handle
4042
* @return the {@link ExtensionRestResponse} result from the handler for this route.
4143
*/
42-
public ExtensionRestResponse getExtensionRestResponse() {
43-
return responseHandler.get();
44+
public ExtensionRestResponse getExtensionRestResponse(ExtensionRestRequest request) {
45+
return responseHandler.apply(request);
4446
}
4547
}

‎src/main/java/org/opensearch/sdk/sample/helloworld/rest/RestHelloAction.java

+23-20
Original file line numberDiff line numberDiff line change
@@ -55,22 +55,21 @@ public class RestHelloAction extends BaseExtensionRestHandler {
5555
@Override
5656
public List<RouteHandler> routeHandlers() {
5757
return List.of(
58-
new RouteHandler(GET, "/hello", this::handleGetRequest),
59-
new RouteHandler(POST, "/hello", this::handlePostRequest),
60-
new RouteHandler(PUT, "/hello/{name}", this::handlePutRequest),
61-
new RouteHandler(DELETE, "/goodbye", this::handleDeleteRequest)
58+
new RouteHandler(GET, "/hello", r -> handleGetRequest(r)),
59+
new RouteHandler(POST, "/hello", r -> handlePostRequest(r)),
60+
new RouteHandler(PUT, "/hello/{name}", r -> handlePutRequest(r)),
61+
new RouteHandler(DELETE, "/goodbye", r -> handleDeleteRequest(r))
6262
);
6363
}
6464

65-
private ExtensionRestResponse handleGetRequest() {
65+
private ExtensionRestResponse handleGetRequest(ExtensionRestRequest request) {
6666
String worldNameWithRandomAdjective = worldAdjectives.isEmpty()
6767
? worldName
6868
: String.join(" ", worldAdjectives.get(rand.nextInt(worldAdjectives.size())), worldName);
69-
return createResponse(OK, String.format(GREETING, worldNameWithRandomAdjective));
69+
return new ExtensionRestResponse(request, OK, String.format(GREETING, worldNameWithRandomAdjective));
7070
}
7171

72-
private ExtensionRestResponse handlePostRequest() {
73-
ExtensionRestRequest request = getRequest();
72+
private ExtensionRestResponse handlePostRequest(ExtensionRestRequest request) {
7473
if (request.hasContent()) {
7574
String adjective = "";
7675
XContentType contentType = request.getXContentType();
@@ -82,11 +81,16 @@ private ExtensionRestResponse handlePostRequest() {
8281
adjective = request.contentParser(NamedXContentRegistry.EMPTY).mapStrings().get("adjective");
8382
} catch (IOException | OpenSearchParseException e) {
8483
// Sample plain text response
85-
return createResponse(BAD_REQUEST, "Unable to parse adjective from JSON");
84+
return new ExtensionRestResponse(request, BAD_REQUEST, "Unable to parse adjective from JSON");
8685
}
8786
} else {
8887
// Sample text response with content type
89-
return createResponse(NOT_ACCEPTABLE, TEXT_CONTENT_TYPE, "Only text and JSON content types are supported");
88+
return new ExtensionRestResponse(
89+
request,
90+
NOT_ACCEPTABLE,
91+
TEXT_CONTENT_TYPE,
92+
"Only text and JSON content types are supported"
93+
);
9094
}
9195
if (adjective != null && !adjective.isBlank()) {
9296
worldAdjectives.add(adjective.trim());
@@ -96,37 +100,36 @@ private ExtensionRestResponse handlePostRequest() {
96100
.startObject()
97101
.field("worldAdjectives", worldAdjectives)
98102
.endObject();
99-
return createResponse(OK, builder);
103+
return new ExtensionRestResponse(request, OK, builder);
100104
} catch (IOException e) {
101105
// Sample response for developer error
102-
return unhandledRequest();
106+
return unhandledRequest(request);
103107
}
104108
}
105109
byte[] noAdjective = "No adjective included with POST request".getBytes(StandardCharsets.UTF_8);
106110
// Sample binary response with content type
107-
return createResponse(BAD_REQUEST, TEXT_CONTENT_TYPE, noAdjective);
111+
return new ExtensionRestResponse(request, BAD_REQUEST, TEXT_CONTENT_TYPE, noAdjective);
108112
}
109113
// Sample bytes reference response with content type
110114
BytesReference noContent = BytesReference.fromByteBuffer(
111115
ByteBuffer.wrap("No content included with POST request".getBytes(StandardCharsets.UTF_8))
112116
);
113-
return createResponse(BAD_REQUEST, TEXT_CONTENT_TYPE, noContent);
117+
return new ExtensionRestResponse(request, BAD_REQUEST, TEXT_CONTENT_TYPE, noContent);
114118
}
115119

116-
private ExtensionRestResponse handlePutRequest() {
117-
ExtensionRestRequest request = getRequest();
120+
private ExtensionRestResponse handlePutRequest(ExtensionRestRequest request) {
118121
String name = request.param("name");
119122
try {
120123
worldName = URLDecoder.decode(name, StandardCharsets.UTF_8);
121124
} catch (IllegalArgumentException e) {
122-
return createResponse(BAD_REQUEST, e.getMessage());
125+
return new ExtensionRestResponse(request, BAD_REQUEST, e.getMessage());
123126
}
124-
return createResponse(OK, "Updated the world's name to " + worldName);
127+
return new ExtensionRestResponse(request, OK, "Updated the world's name to " + worldName);
125128
}
126129

127-
private ExtensionRestResponse handleDeleteRequest() {
130+
private ExtensionRestResponse handleDeleteRequest(ExtensionRestRequest request) {
128131
this.worldName = DEFAULT_NAME;
129132
this.worldAdjectives.clear();
130-
return createResponse(OK, "Goodbye, cruel world! Restored default values.");
133+
return new ExtensionRestResponse(request, OK, "Goodbye, cruel world! Restored default values.");
131134
}
132135
}

0 commit comments

Comments
 (0)