From 5141a004c5846f2727a7283a5276e6acd41feed2 Mon Sep 17 00:00:00 2001 From: Daulet Date: Tue, 4 Apr 2023 00:53:05 +0500 Subject: [PATCH 01/11] added uri and httpVersion to HttpRequest Signed-off-by: Daulet --- .../opensearch/sdk/handlers/ExtensionsRestRequestHandler.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/opensearch/sdk/handlers/ExtensionsRestRequestHandler.java b/src/main/java/org/opensearch/sdk/handlers/ExtensionsRestRequestHandler.java index 535910f5..2c5fb707 100644 --- a/src/main/java/org/opensearch/sdk/handlers/ExtensionsRestRequestHandler.java +++ b/src/main/java/org/opensearch/sdk/handlers/ExtensionsRestRequestHandler.java @@ -91,7 +91,7 @@ public Method method() { public String uri() { // path strips query off uri but probably want to pass the whole uri // this will make the request behave as expected (without query params) - return request.path(); + return request.uri(); } @Override @@ -114,7 +114,7 @@ public List strictCookies() { @Override public HttpVersion protocolVersion() { - return null; + return request.httpVersion(); } @Override From 6a9d86a95528149d83425dce1ed9d7839f459914 Mon Sep 17 00:00:00 2001 From: Daulet Date: Tue, 4 Apr 2023 02:29:43 +0500 Subject: [PATCH 02/11] added SDKHttpRequest, SDKRestRequest Signed-off-by: Daulet --- .../org/opensearch/sdk/SDKHttpRequest.java | 144 ++++++++++++++++++ .../org/opensearch/sdk/SDKRestRequest.java | 44 ++++++ .../ExtensionsRestRequestHandler.java | 79 ++-------- 3 files changed, 201 insertions(+), 66 deletions(-) create mode 100644 src/main/java/org/opensearch/sdk/SDKHttpRequest.java create mode 100644 src/main/java/org/opensearch/sdk/SDKRestRequest.java diff --git a/src/main/java/org/opensearch/sdk/SDKHttpRequest.java b/src/main/java/org/opensearch/sdk/SDKHttpRequest.java new file mode 100644 index 00000000..da666b75 --- /dev/null +++ b/src/main/java/org/opensearch/sdk/SDKHttpRequest.java @@ -0,0 +1,144 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.sdk; + +import org.opensearch.common.bytes.BytesReference; +import org.opensearch.extensions.rest.ExtensionRestRequest; +import org.opensearch.http.HttpRequest; +import org.opensearch.http.HttpResponse; +import org.opensearch.rest.RestRequest; +import org.opensearch.rest.RestStatus; + +import java.util.List; +import java.util.Map; + +/** + * This class helps to get instance of HttpRequest + */ +public class SDKHttpRequest implements HttpRequest { + private final RestRequest.Method method; + private final String uri; + private final BytesReference content; + private final Map> headers; + private final HttpVersion httpVersion; + + /** + * Instantiates this class with a copy of request + * + * @param request The request + */ + public SDKHttpRequest(ExtensionRestRequest request) { + this.method = request.method(); + this.uri = request.uri(); + this.content = request.content(); + this.headers = request.headers(); + this.httpVersion = request.httpVersion(); + } + + /** + * This method returns request method + * @return A method of request + */ + @Override + public RestRequest.Method method() { + return method; + } + + /** + * This method returns request uri + * @return URI of request + */ + @Override + public String uri() { + return uri; + } + + /** + * This method returns request content + * @return content of request + */ + @Override + public BytesReference content() { + return content; + } + + /** + * This method returns request headers + * @return map containing headers + */ + @Override + public Map> getHeaders() { + return headers; + } + + /** + * This method returns request cookies + * @return list containing cookies + */ + @Override + public List strictCookies() { + return null; + } + + /** + * This method returns request HTTP protocol version + * @return version of request HTTP protocol + */ + @Override + public HttpVersion protocolVersion() { + return httpVersion; + } + + /** + * This method removes headers from request + * @param s header + */ + @Override + public HttpRequest removeHeader(String s) { + return null; + } + + /** + * This method creates response + * @param restStatus response status + * @param bytesReference content + */ + @Override + public HttpResponse createResponse(RestStatus restStatus, BytesReference bytesReference) { + return null; + } + + /** + * This method returns inbound exception + * @return thrown exception + */ + @Override + public Exception getInboundException() { + return null; + } + + /** + * Release any resources associated with this request. + */ + @Override + public void release() { + + } + + /** + * If this instances uses any pooled resources, creates a copy of this instance that does not use any pooled resources and releases + * any resources associated with this instance. If the instance does not use any shared resources, returns itself. + * @return a safe unpooled http request + */ + @Override + public HttpRequest releaseAndCopy() { + return null; + } +} diff --git a/src/main/java/org/opensearch/sdk/SDKRestRequest.java b/src/main/java/org/opensearch/sdk/SDKRestRequest.java new file mode 100644 index 00000000..c6b49bbe --- /dev/null +++ b/src/main/java/org/opensearch/sdk/SDKRestRequest.java @@ -0,0 +1,44 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.sdk; + +import java.util.List; +import java.util.Map; + +import org.opensearch.core.xcontent.NamedXContentRegistry; +import org.opensearch.http.HttpChannel; +import org.opensearch.http.HttpRequest; +import org.opensearch.rest.RestRequest; + +/** + * This class helps to get instance of RestRequest + */ +public class SDKRestRequest extends RestRequest { + /** + * Instantiates this class with a copy of request + * + * @param xContentRegistry The request's content registry + * @param params The request's params + * @param path The request's path + * @param headers The request's headers + * @param httpRequest The request's httpRequest + * @param httpChannel The request's http channel + */ + public SDKRestRequest( + NamedXContentRegistry xContentRegistry, + Map params, + String path, + Map> headers, + HttpRequest httpRequest, + HttpChannel httpChannel + ) { + super(xContentRegistry, params, path, headers, httpRequest, httpChannel); + } +} diff --git a/src/main/java/org/opensearch/sdk/handlers/ExtensionsRestRequestHandler.java b/src/main/java/org/opensearch/sdk/handlers/ExtensionsRestRequestHandler.java index 2c5fb707..0013eba3 100644 --- a/src/main/java/org/opensearch/sdk/handlers/ExtensionsRestRequestHandler.java +++ b/src/main/java/org/opensearch/sdk/handlers/ExtensionsRestRequestHandler.java @@ -23,7 +23,9 @@ import org.opensearch.rest.RestStatus; import org.opensearch.sdk.ExtensionRestHandler; import org.opensearch.sdk.ExtensionsRunner; +import org.opensearch.sdk.SDKHttpRequest; import org.opensearch.sdk.SDKNamedXContentRegistry; +import org.opensearch.sdk.SDKRestRequest; import java.util.Collections; import java.util.List; @@ -77,73 +79,18 @@ public RestExecuteOnExtensionResponse handleRestExecuteOnExtensionRequest(Extens ); } - // Temporary code to create a RestRequest from the ExtensionRestRequest before header code added - // Remove this and replace with SDKRestRequest being generated by this PR: - // https://github.com/opensearch-project/opensearch-sdk-java/pull/605 - RestRequest restRequest = RestRequest.request(sdkNamedXContentRegistry.getRegistry(), new HttpRequest() { - - @Override - public Method method() { - return request.method(); - } - - @Override - public String uri() { - // path strips query off uri but probably want to pass the whole uri - // this will make the request behave as expected (without query params) - return request.uri(); - } - - @Override - public BytesReference content() { - return request.content(); - } - - @Override - public Map> getHeaders() { - // This effectively recreates the only header we need right now - // PR replacing this will pass more headers - XContentType xContentType = request.getXContentType(); - return xContentType == null ? Collections.emptyMap() : Map.of("Content-Type", List.of(xContentType.mediaType())); - } - - @Override - public List strictCookies() { - return Collections.emptyList(); - } - - @Override - public HttpVersion protocolVersion() { - return request.httpVersion(); - } - - @Override - public HttpRequest removeHeader(String header) { - // we don't use - return null; - } - - @Override - public HttpResponse createResponse(RestStatus status, BytesReference content) { - return null; - } - - @Override - public Exception getInboundException() { - return null; - } - - @Override - public void release() {} - - @Override - public HttpRequest releaseAndCopy() { - return null; - } - }, null); - + SDKHttpRequest sdkHttpRequest = new SDKHttpRequest(request); + SDKRestRequest sdkRestRequest = new SDKRestRequest( + sdkNamedXContentRegistry.getRegistry(), + request.params(), + request.path(), + request.headers(), + sdkHttpRequest, + null + ); + // Get response from extension - ExtensionRestResponse response = restHandler.handleRequest(restRequest); + ExtensionRestResponse response = restHandler.handleRequest(sdkRestRequest); logger.info("Sending extension response to OpenSearch: " + response.status()); return new RestExecuteOnExtensionResponse( response.status(), From 6988f043d7b73f309c8933088cdc2fc80fc16a17 Mon Sep 17 00:00:00 2001 From: Daulet Date: Tue, 4 Apr 2023 02:39:44 +0500 Subject: [PATCH 03/11] spotless applied Signed-off-by: Daulet --- .../sdk/handlers/ExtensionsRestRequestHandler.java | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/src/main/java/org/opensearch/sdk/handlers/ExtensionsRestRequestHandler.java b/src/main/java/org/opensearch/sdk/handlers/ExtensionsRestRequestHandler.java index 0013eba3..971dea2a 100644 --- a/src/main/java/org/opensearch/sdk/handlers/ExtensionsRestRequestHandler.java +++ b/src/main/java/org/opensearch/sdk/handlers/ExtensionsRestRequestHandler.java @@ -12,25 +12,15 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.opensearch.common.bytes.BytesReference; -import org.opensearch.common.xcontent.XContentType; import org.opensearch.extensions.rest.ExtensionRestRequest; import org.opensearch.extensions.rest.ExtensionRestResponse; import org.opensearch.extensions.rest.RestExecuteOnExtensionResponse; -import org.opensearch.http.HttpRequest; -import org.opensearch.http.HttpResponse; -import org.opensearch.rest.RestRequest; -import org.opensearch.rest.RestRequest.Method; -import org.opensearch.rest.RestStatus; import org.opensearch.sdk.ExtensionRestHandler; import org.opensearch.sdk.ExtensionsRunner; import org.opensearch.sdk.SDKHttpRequest; import org.opensearch.sdk.SDKNamedXContentRegistry; import org.opensearch.sdk.SDKRestRequest; -import java.util.Collections; -import java.util.List; -import java.util.Map; - import org.opensearch.sdk.ExtensionRestPathRegistry; import static java.nio.charset.StandardCharsets.UTF_8; @@ -88,7 +78,7 @@ public RestExecuteOnExtensionResponse handleRestExecuteOnExtensionRequest(Extens sdkHttpRequest, null ); - + // Get response from extension ExtensionRestResponse response = restHandler.handleRequest(sdkRestRequest); logger.info("Sending extension response to OpenSearch: " + response.status()); From 2a51e4b86f4fa8c6ad600b244bf242e589637658 Mon Sep 17 00:00:00 2001 From: Daulet Date: Tue, 4 Apr 2023 22:09:31 +0500 Subject: [PATCH 04/11] moved SDKHttpRequest and SDKRestRequest to org.opensearch.sdk.rest Signed-off-by: Daulet --- .../ExtensionsRestRequestHandler.java | 7 ++- .../sdk/{ => rest}/SDKHttpRequest.java | 46 ++++++------------- .../sdk/{ => rest}/SDKRestRequest.java | 4 +- 3 files changed, 18 insertions(+), 39 deletions(-) rename src/main/java/org/opensearch/sdk/{ => rest}/SDKHttpRequest.java (66%) rename src/main/java/org/opensearch/sdk/{ => rest}/SDKRestRequest.java (93%) diff --git a/src/main/java/org/opensearch/sdk/handlers/ExtensionsRestRequestHandler.java b/src/main/java/org/opensearch/sdk/handlers/ExtensionsRestRequestHandler.java index 971dea2a..e0e5a741 100644 --- a/src/main/java/org/opensearch/sdk/handlers/ExtensionsRestRequestHandler.java +++ b/src/main/java/org/opensearch/sdk/handlers/ExtensionsRestRequestHandler.java @@ -17,9 +17,9 @@ import org.opensearch.extensions.rest.RestExecuteOnExtensionResponse; import org.opensearch.sdk.ExtensionRestHandler; import org.opensearch.sdk.ExtensionsRunner; -import org.opensearch.sdk.SDKHttpRequest; import org.opensearch.sdk.SDKNamedXContentRegistry; -import org.opensearch.sdk.SDKRestRequest; +import org.opensearch.sdk.rest.SDKHttpRequest; +import org.opensearch.sdk.rest.SDKRestRequest; import org.opensearch.sdk.ExtensionRestPathRegistry; @@ -69,13 +69,12 @@ public RestExecuteOnExtensionResponse handleRestExecuteOnExtensionRequest(Extens ); } - SDKHttpRequest sdkHttpRequest = new SDKHttpRequest(request); SDKRestRequest sdkRestRequest = new SDKRestRequest( sdkNamedXContentRegistry.getRegistry(), request.params(), request.path(), request.headers(), - sdkHttpRequest, + new SDKHttpRequest(request), null ); diff --git a/src/main/java/org/opensearch/sdk/SDKHttpRequest.java b/src/main/java/org/opensearch/sdk/rest/SDKHttpRequest.java similarity index 66% rename from src/main/java/org/opensearch/sdk/SDKHttpRequest.java rename to src/main/java/org/opensearch/sdk/rest/SDKHttpRequest.java index da666b75..e01efa98 100644 --- a/src/main/java/org/opensearch/sdk/SDKHttpRequest.java +++ b/src/main/java/org/opensearch/sdk/rest/SDKHttpRequest.java @@ -7,7 +7,7 @@ * compatible open source license. */ -package org.opensearch.sdk; +package org.opensearch.sdk.rest; import org.opensearch.common.bytes.BytesReference; import org.opensearch.extensions.rest.ExtensionRestRequest; @@ -30,7 +30,7 @@ public class SDKHttpRequest implements HttpRequest { private final HttpVersion httpVersion; /** - * Instantiates this class with a copy of request + * Instantiates this class with a copy of {@link ExtensionRestRequest} * * @param request The request */ @@ -42,63 +42,43 @@ public SDKHttpRequest(ExtensionRestRequest request) { this.httpVersion = request.httpVersion(); } - /** - * This method returns request method - * @return A method of request - */ @Override public RestRequest.Method method() { return method; } - /** - * This method returns request uri - * @return URI of request - */ @Override public String uri() { return uri; } - /** - * This method returns request content - * @return content of request - */ @Override public BytesReference content() { return content; } - /** - * This method returns request headers - * @return map containing headers - */ @Override public Map> getHeaders() { return headers; } /** - * This method returns request cookies - * @return list containing cookies + * Not implemented. Does nothing. + * @return null */ @Override public List strictCookies() { return null; } - /** - * This method returns request HTTP protocol version - * @return version of request HTTP protocol - */ @Override public HttpVersion protocolVersion() { return httpVersion; } /** - * This method removes headers from request - * @param s header + * Not implemented. Does nothing. + * @return null */ @Override public HttpRequest removeHeader(String s) { @@ -106,9 +86,10 @@ public HttpRequest removeHeader(String s) { } /** - * This method creates response + * Not implemented. Does nothing. * @param restStatus response status * @param bytesReference content + * @return null */ @Override public HttpResponse createResponse(RestStatus restStatus, BytesReference bytesReference) { @@ -116,8 +97,8 @@ public HttpResponse createResponse(RestStatus restStatus, BytesReference bytesRe } /** - * This method returns inbound exception - * @return thrown exception + * Not implemented. Does nothing. + * @return null */ @Override public Exception getInboundException() { @@ -125,7 +106,7 @@ public Exception getInboundException() { } /** - * Release any resources associated with this request. + * Not implemented. Does nothing. */ @Override public void release() { @@ -133,9 +114,8 @@ public void release() { } /** - * If this instances uses any pooled resources, creates a copy of this instance that does not use any pooled resources and releases - * any resources associated with this instance. If the instance does not use any shared resources, returns itself. - * @return a safe unpooled http request + * Not implemented. Does nothing. + * @return null */ @Override public HttpRequest releaseAndCopy() { diff --git a/src/main/java/org/opensearch/sdk/SDKRestRequest.java b/src/main/java/org/opensearch/sdk/rest/SDKRestRequest.java similarity index 93% rename from src/main/java/org/opensearch/sdk/SDKRestRequest.java rename to src/main/java/org/opensearch/sdk/rest/SDKRestRequest.java index c6b49bbe..0d4187d8 100644 --- a/src/main/java/org/opensearch/sdk/SDKRestRequest.java +++ b/src/main/java/org/opensearch/sdk/rest/SDKRestRequest.java @@ -7,7 +7,7 @@ * compatible open source license. */ -package org.opensearch.sdk; +package org.opensearch.sdk.rest; import java.util.List; import java.util.Map; @@ -22,7 +22,7 @@ */ public class SDKRestRequest extends RestRequest { /** - * Instantiates this class with a copy of request + * Instantiates this class with request's params * * @param xContentRegistry The request's content registry * @param params The request's params From 3ca9c0bf4260a034a1e7bf85b5fb4d51f63b1b05 Mon Sep 17 00:00:00 2001 From: Daulet Date: Tue, 4 Apr 2023 22:40:17 +0500 Subject: [PATCH 05/11] tests changed for new ExtensionRestRequest Signed-off-by: Daulet --- .../sdk/TestBaseExtensionRestHandler.java | 94 +++++-------------- .../opensearch/sdk/TestExtensionsRunner.java | 7 +- 2 files changed, 32 insertions(+), 69 deletions(-) diff --git a/src/test/java/org/opensearch/sdk/TestBaseExtensionRestHandler.java b/src/test/java/org/opensearch/sdk/TestBaseExtensionRestHandler.java index eab8599d..9acac97c 100644 --- a/src/test/java/org/opensearch/sdk/TestBaseExtensionRestHandler.java +++ b/src/test/java/org/opensearch/sdk/TestBaseExtensionRestHandler.java @@ -13,18 +13,18 @@ import java.util.Collections; import java.util.List; import java.util.Map; -import java.util.Map.Entry; import java.util.function.Function; import org.junit.jupiter.api.Test; import org.opensearch.common.bytes.BytesArray; import org.opensearch.common.bytes.BytesReference; import org.opensearch.common.xcontent.XContentType; +import org.opensearch.extensions.rest.ExtensionRestRequest; import org.opensearch.extensions.rest.ExtensionRestResponse; import org.opensearch.http.HttpRequest; -import org.opensearch.http.HttpResponse; import org.opensearch.rest.RestRequest; import org.opensearch.rest.RestRequest.Method; +import org.opensearch.sdk.rest.SDKRestRequest; import org.opensearch.rest.RestStatus; import org.opensearch.test.OpenSearchTestCase; @@ -91,10 +91,13 @@ public void testErrorResponseOnUnhandled() { RestRequest unhandledRequestMethod = createTestRestRequest( Method.PUT, "foo", + "foo", + Collections.emptyMap(), Collections.emptyMap(), null, new BytesArray(new byte[0]), - "" + "", + null ); ExtensionRestResponse response = handler.handleRequest(unhandledRequestMethod); assertEquals(RestStatus.NOT_FOUND, response.status()); @@ -110,10 +113,13 @@ public void testErrorResponseOnUnhandled() { RestRequest unhandledRequestPath = createTestRestRequest( Method.GET, "foobar", + "foobar", + Collections.emptyMap(), Collections.emptyMap(), null, new BytesArray(new byte[0]), - "" + "", + null ); response = handler.handleRequest(unhandledRequestPath); assertEquals(RestStatus.NOT_FOUND, response.status()); @@ -129,74 +135,26 @@ public void testErrorResponseOnUnhandled() { public static RestRequest createTestRestRequest( final Method method, + final String uri, final String path, final Map params, + final Map headers, final XContentType xContentType, final BytesReference content, - final String principalIdentifier + final String principalIdentifier, + final HttpRequest.HttpVersion httpVersion ) { - // Temporary code to create a RestRequest from the ExtensionRestRequest before header code added - // Remove this and replace with SDKRestRequest being generated by this PR: - // https://github.com/opensearch-project/opensearch-sdk-java/pull/605 - return RestRequest.request(null, new HttpRequest() { - - @Override - public Method method() { - return method; - } - - @Override - public String uri() { - StringBuilder uri = new StringBuilder(); - for (Entry param : params.entrySet()) { - uri.append(uri.length() == 0 ? '?' : '&').append(param.getKey()).append('=').append(param.getValue()); - } - return path + uri.toString(); - } - - @Override - public BytesReference content() { - return content; - } - - @Override - public Map> getHeaders() { - return xContentType == null ? Collections.emptyMap() : Map.of("Content-Type", List.of(xContentType.mediaType())); - } - - @Override - public List strictCookies() { - return Collections.emptyList(); - } - - @Override - public HttpVersion protocolVersion() { - return null; - } - - @Override - public HttpRequest removeHeader(String header) { - // we don't use - return null; - } - - @Override - public HttpResponse createResponse(RestStatus status, BytesReference content) { - return null; - } - - @Override - public Exception getInboundException() { - return null; - } - - @Override - public void release() {} - - @Override - public HttpRequest releaseAndCopy() { - return null; - } - }, null); + ExtensionRestRequest request = new ExtensionRestRequest( + method, + uri, + path, + params, + headers, + xContentType, + content, + principalIdentifier, + httpVersion + ); + return new SDKRestRequest(null, request.params(), request.path(), request.headers(), new SDKHttpRequest(request), null); } } diff --git a/src/test/java/org/opensearch/sdk/TestExtensionsRunner.java b/src/test/java/org/opensearch/sdk/TestExtensionsRunner.java index 00f0439b..01f796a0 100644 --- a/src/test/java/org/opensearch/sdk/TestExtensionsRunner.java +++ b/src/test/java/org/opensearch/sdk/TestExtensionsRunner.java @@ -45,6 +45,7 @@ import org.opensearch.extensions.ExtensionDependency; import org.opensearch.extensions.rest.ExtensionRestRequest; import org.opensearch.extensions.rest.RestExecuteOnExtensionResponse; +import org.opensearch.http.HttpRequest; import org.opensearch.rest.BytesRestResponse; import org.opensearch.rest.RestRequest.Method; import org.opensearch.rest.RestStatus; @@ -142,13 +143,17 @@ public void testHandleExtensionRestRequest() throws Exception { String ext = "token_placeholder"; @SuppressWarnings("unused") // placeholder to test the token when identity features merged Principal userPrincipal = () -> "user1"; + HttpRequest.HttpVersion httpVersion = HttpRequest.HttpVersion.HTTP_1_1; ExtensionRestRequest request = new ExtensionRestRequest( Method.GET, "/foo", + "/foo", + Collections.emptyMap(), Collections.emptyMap(), null, new BytesArray("bar"), - ext + ext, + httpVersion ); RestExecuteOnExtensionResponse response = extensionsRestRequestHandler.handleRestExecuteOnExtensionRequest(request); // this will fail in test environment with no registered actions From 3b5cac9a593f92bda5a68b13479170560ac07c89 Mon Sep 17 00:00:00 2001 From: Daulet Date: Wed, 5 Apr 2023 22:41:02 +0500 Subject: [PATCH 06/11] TestSDKRestRequest created Signed-off-by: Daulet --- .../sdk/TestBaseExtensionRestHandler.java | 49 +++-------- .../opensearch/sdk/TestSDKRestRequest.java | 84 +++++++++++++++++++ 2 files changed, 96 insertions(+), 37 deletions(-) create mode 100644 src/test/java/org/opensearch/sdk/TestSDKRestRequest.java diff --git a/src/test/java/org/opensearch/sdk/TestBaseExtensionRestHandler.java b/src/test/java/org/opensearch/sdk/TestBaseExtensionRestHandler.java index 9acac97c..08b18e9d 100644 --- a/src/test/java/org/opensearch/sdk/TestBaseExtensionRestHandler.java +++ b/src/test/java/org/opensearch/sdk/TestBaseExtensionRestHandler.java @@ -12,19 +12,13 @@ import java.nio.charset.StandardCharsets; import java.util.Collections; import java.util.List; -import java.util.Map; import java.util.function.Function; import org.junit.jupiter.api.Test; import org.opensearch.common.bytes.BytesArray; -import org.opensearch.common.bytes.BytesReference; -import org.opensearch.common.xcontent.XContentType; -import org.opensearch.extensions.rest.ExtensionRestRequest; import org.opensearch.extensions.rest.ExtensionRestResponse; -import org.opensearch.http.HttpRequest; import org.opensearch.rest.RestRequest; import org.opensearch.rest.RestRequest.Method; -import org.opensearch.sdk.rest.SDKRestRequest; import org.opensearch.rest.RestStatus; import org.opensearch.test.OpenSearchTestCase; @@ -58,13 +52,16 @@ public void testHandlerDefaultRoutes() { @Test public void testJsonErrorResponse() { - RestRequest successfulRequest = createTestRestRequest( + RestRequest successfulRequest = TestSDKRestRequest.createTestRestRequest( Method.GET, "foo", + "foo", + Collections.emptyMap(), Collections.emptyMap(), null, new BytesArray("bar".getBytes(StandardCharsets.UTF_8)), - "" + "", + null ); ExtensionRestResponse response = handler.handleRequest(successfulRequest); assertEquals(RestStatus.OK, response.status()); @@ -73,13 +70,16 @@ public void testJsonErrorResponse() { @Test public void testErrorResponseOnException() { - RestRequest exceptionalRequest = createTestRestRequest( + RestRequest exceptionalRequest = TestSDKRestRequest.createTestRestRequest( Method.GET, "foo", + "foo", + Collections.emptyMap(), Collections.emptyMap(), null, new BytesArray("baz".getBytes(StandardCharsets.UTF_8)), - "" + "", + null ); ExtensionRestResponse response = handler.handleRequest(exceptionalRequest); assertEquals(RestStatus.INTERNAL_SERVER_ERROR, response.status()); @@ -88,7 +88,7 @@ public void testErrorResponseOnException() { @Test public void testErrorResponseOnUnhandled() { - RestRequest unhandledRequestMethod = createTestRestRequest( + RestRequest unhandledRequestMethod = TestSDKRestRequest.createTestRestRequest( Method.PUT, "foo", "foo", @@ -110,7 +110,7 @@ public void testErrorResponseOnUnhandled() { response.content().utf8ToString() ); - RestRequest unhandledRequestPath = createTestRestRequest( + RestRequest unhandledRequestPath = TestSDKRestRequest.createTestRestRequest( Method.GET, "foobar", "foobar", @@ -132,29 +132,4 @@ public void testErrorResponseOnUnhandled() { response.content().utf8ToString() ); } - - public static RestRequest createTestRestRequest( - final Method method, - final String uri, - final String path, - final Map params, - final Map headers, - final XContentType xContentType, - final BytesReference content, - final String principalIdentifier, - final HttpRequest.HttpVersion httpVersion - ) { - ExtensionRestRequest request = new ExtensionRestRequest( - method, - uri, - path, - params, - headers, - xContentType, - content, - principalIdentifier, - httpVersion - ); - return new SDKRestRequest(null, request.params(), request.path(), request.headers(), new SDKHttpRequest(request), null); - } } diff --git a/src/test/java/org/opensearch/sdk/TestSDKRestRequest.java b/src/test/java/org/opensearch/sdk/TestSDKRestRequest.java new file mode 100644 index 00000000..ad8f1299 --- /dev/null +++ b/src/test/java/org/opensearch/sdk/TestSDKRestRequest.java @@ -0,0 +1,84 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.sdk; + +import java.util.Arrays; +import java.util.List; +import java.util.Map; + +import org.junit.Test; +import org.opensearch.common.bytes.BytesArray; +import org.opensearch.common.bytes.BytesReference; +import org.opensearch.common.xcontent.XContentType; +import org.opensearch.extensions.rest.ExtensionRestRequest; +import org.opensearch.http.HttpRequest; +import org.opensearch.rest.RestRequest; +import org.opensearch.rest.RestRequest.Method; +import org.opensearch.sdk.rest.SDKHttpRequest; +import org.opensearch.sdk.rest.SDKRestRequest; +import org.opensearch.test.OpenSearchTestCase; + +import static java.util.Map.entry; + +public class TestSDKRestRequest extends OpenSearchTestCase { + @Test + public void TestSDKRestRequestMethods() { + RestRequest.Method expectedMethod = Method.GET; + String expectedUri = "foobar?foo=bar&baz=42"; + String expectedPath = "foo"; + Map expectedParams = Map.ofEntries(entry("foo", "bar"), entry("baz", "42")); + Map> expectedHeaders = Map.ofEntries(entry("foo", Arrays.asList("hello", "world"))); + XContentType exptectedXContentType = null; + BytesReference expectedContent = new BytesArray("bar"); + + SDKRestRequest sdkRestRequest = createTestRestRequest( + expectedMethod, + expectedUri, + expectedPath, + expectedParams, + expectedHeaders, + exptectedXContentType, + expectedContent, + "", + null + ); + assertEquals(expectedMethod, sdkRestRequest.method()); + assertEquals(expectedUri, sdkRestRequest.uri()); + assertEquals(expectedPath, sdkRestRequest.path()); + assertEquals(expectedParams, sdkRestRequest.params()); + assertEquals(expectedHeaders, sdkRestRequest.getHeaders()); + assertEquals(expectedContent, sdkRestRequest.content()); + } + + public static RestRequest createTestRestRequest( + final Method method, + final String uri, + final String path, + final Map params, + final Map headers, + final XContentType xContentType, + final BytesReference content, + final String principalIdentifier, + final HttpRequest.HttpVersion httpVersion + ) { + ExtensionRestRequest request = new ExtensionRestRequest( + method, + uri, + path, + params, + headers, + xContentType, + content, + principalIdentifier, + httpVersion + ); + return new SDKRestRequest(null, request.params(), request.path(), request.headers(), new SDKHttpRequest(request), null); + } +} From 54bbb6235c63687d4fc7af925db42c2a517880bf Mon Sep 17 00:00:00 2001 From: Daulet Date: Wed, 5 Apr 2023 23:40:29 +0500 Subject: [PATCH 07/11] corrected test for xContentType, added test for contentParser() Signed-off-by: Daulet --- .../java/org/opensearch/sdk/TestSDKRestRequest.java | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/test/java/org/opensearch/sdk/TestSDKRestRequest.java b/src/test/java/org/opensearch/sdk/TestSDKRestRequest.java index ad8f1299..0341af6e 100644 --- a/src/test/java/org/opensearch/sdk/TestSDKRestRequest.java +++ b/src/test/java/org/opensearch/sdk/TestSDKRestRequest.java @@ -34,8 +34,11 @@ public void TestSDKRestRequestMethods() { String expectedUri = "foobar?foo=bar&baz=42"; String expectedPath = "foo"; Map expectedParams = Map.ofEntries(entry("foo", "bar"), entry("baz", "42")); - Map> expectedHeaders = Map.ofEntries(entry("foo", Arrays.asList("hello", "world"))); - XContentType exptectedXContentType = null; + Map> expectedHeaders = Map.ofEntries( + entry("Content-Type", Arrays.asList("application/json")), + entry("foo", Arrays.asList("hello", "world")) + ); + XContentType exptectedXContentType = XContentType.JSON; BytesReference expectedContent = new BytesArray("bar"); SDKRestRequest sdkRestRequest = createTestRestRequest( @@ -54,7 +57,11 @@ public void TestSDKRestRequestMethods() { assertEquals(expectedPath, sdkRestRequest.path()); assertEquals(expectedParams, sdkRestRequest.params()); assertEquals(expectedHeaders, sdkRestRequest.getHeaders()); + assertEquals(exptectedXContentType, sdkRestRequest.getXContentType()); assertEquals(expectedContent, sdkRestRequest.content()); + + Map source = sdkRestRequest.contentParser().map(); + assertEquals("bar", (String) source.get("foo")); } public static RestRequest createTestRestRequest( @@ -68,6 +75,7 @@ public static RestRequest createTestRestRequest( final String principalIdentifier, final HttpRequest.HttpVersion httpVersion ) { + // xContentType is not used. It will be parsed from headers ExtensionRestRequest request = new ExtensionRestRequest( method, uri, From e7de8b57d8fe2ca4a3b603bb504e68ac5e70bc27 Mon Sep 17 00:00:00 2001 From: Daulet Date: Thu, 6 Apr 2023 01:18:30 +0500 Subject: [PATCH 08/11] replaced .map() with mapStrings() in contentParser() Signed-off-by: Daulet --- src/test/java/org/opensearch/sdk/TestSDKRestRequest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/java/org/opensearch/sdk/TestSDKRestRequest.java b/src/test/java/org/opensearch/sdk/TestSDKRestRequest.java index 0341af6e..1e91e491 100644 --- a/src/test/java/org/opensearch/sdk/TestSDKRestRequest.java +++ b/src/test/java/org/opensearch/sdk/TestSDKRestRequest.java @@ -60,8 +60,8 @@ public void TestSDKRestRequestMethods() { assertEquals(exptectedXContentType, sdkRestRequest.getXContentType()); assertEquals(expectedContent, sdkRestRequest.content()); - Map source = sdkRestRequest.contentParser().map(); - assertEquals("bar", (String) source.get("foo")); + Map source = sdkRestRequest.contentParser().mapStrings(); + assertEquals("bar", source.get("foo")); } public static RestRequest createTestRestRequest( From 0648f6e10f75f2bf708d455dede880b3dbfda831 Mon Sep 17 00:00:00 2001 From: Daulet Nassipkali Date: Fri, 7 Apr 2023 22:59:16 +0500 Subject: [PATCH 09/11] Update SDKHttpRequest.java Signed-off-by: Daulet Nassipkali --- src/main/java/org/opensearch/sdk/rest/SDKHttpRequest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/opensearch/sdk/rest/SDKHttpRequest.java b/src/main/java/org/opensearch/sdk/rest/SDKHttpRequest.java index e01efa98..46c8973f 100644 --- a/src/main/java/org/opensearch/sdk/rest/SDKHttpRequest.java +++ b/src/main/java/org/opensearch/sdk/rest/SDKHttpRequest.java @@ -39,7 +39,7 @@ public SDKHttpRequest(ExtensionRestRequest request) { this.uri = request.uri(); this.content = request.content(); this.headers = request.headers(); - this.httpVersion = request.httpVersion(); + this.httpVersion = request.protocolVersion(); } @Override From f925249f04f3b4d332103fb1310c60a74d9d5a5e Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Fri, 7 Apr 2023 12:07:54 -0700 Subject: [PATCH 10/11] Fix tests for new format Signed-off-by: Daniel Widdis --- .../opensearch/sdk/TestSDKRestRequest.java | 7 +- .../helloworld/rest/TestRestHelloAction.java | 89 +++++++++++++------ 2 files changed, 66 insertions(+), 30 deletions(-) diff --git a/src/test/java/org/opensearch/sdk/TestSDKRestRequest.java b/src/test/java/org/opensearch/sdk/TestSDKRestRequest.java index 1e91e491..4b40b3e4 100644 --- a/src/test/java/org/opensearch/sdk/TestSDKRestRequest.java +++ b/src/test/java/org/opensearch/sdk/TestSDKRestRequest.java @@ -9,6 +9,7 @@ package org.opensearch.sdk; +import java.io.IOException; import java.util.Arrays; import java.util.List; import java.util.Map; @@ -29,7 +30,7 @@ public class TestSDKRestRequest extends OpenSearchTestCase { @Test - public void TestSDKRestRequestMethods() { + public void TestSDKRestRequestMethods() throws IOException { RestRequest.Method expectedMethod = Method.GET; String expectedUri = "foobar?foo=bar&baz=42"; String expectedPath = "foo"; @@ -41,7 +42,7 @@ public void TestSDKRestRequestMethods() { XContentType exptectedXContentType = XContentType.JSON; BytesReference expectedContent = new BytesArray("bar"); - SDKRestRequest sdkRestRequest = createTestRestRequest( + RestRequest sdkRestRequest = createTestRestRequest( expectedMethod, expectedUri, expectedPath, @@ -69,7 +70,7 @@ public static RestRequest createTestRestRequest( final String uri, final String path, final Map params, - final Map headers, + final Map> headers, final XContentType xContentType, final BytesReference content, final String principalIdentifier, diff --git a/src/test/java/org/opensearch/sdk/sample/helloworld/rest/TestRestHelloAction.java b/src/test/java/org/opensearch/sdk/sample/helloworld/rest/TestRestHelloAction.java index 08a75ac3..78b62116 100644 --- a/src/test/java/org/opensearch/sdk/sample/helloworld/rest/TestRestHelloAction.java +++ b/src/test/java/org/opensearch/sdk/sample/helloworld/rest/TestRestHelloAction.java @@ -22,10 +22,11 @@ import org.opensearch.common.bytes.BytesArray; import org.opensearch.common.bytes.BytesReference; import org.opensearch.common.xcontent.XContentType; +import org.opensearch.http.HttpRequest.HttpVersion; import org.opensearch.rest.RestResponse; import org.opensearch.rest.RestStatus; import org.opensearch.sdk.ExtensionRestHandler; -import org.opensearch.sdk.TestBaseExtensionRestHandler; +import org.opensearch.sdk.TestSDKRestRequest; import org.opensearch.test.OpenSearchTestCase; public class TestRestHelloAction extends OpenSearchTestCase { @@ -66,85 +67,115 @@ public void testHandleRequest() { String token = "placeholder_token"; Map params = Collections.emptyMap(); - RestRequest getRequest = TestBaseExtensionRestHandler.createTestRestRequest( + RestRequest getRequest = TestSDKRestRequest.createTestRestRequest( Method.GET, "/hello", + "/hello", params, - null, + headers(XContentType.JSON), + XContentType.JSON, new BytesArray(""), - token + token, + HttpVersion.HTTP_1_1 ); - RestRequest putRequest = TestBaseExtensionRestHandler.createTestRestRequest( + RestRequest putRequest = TestSDKRestRequest.createTestRestRequest( Method.PUT, "/hello/Passing+Test", + "/hello/Passing+Test", Map.of("name", "Passing+Test"), - null, + headers(XContentType.JSON), + XContentType.JSON, new BytesArray(""), - token + token, + HttpVersion.HTTP_1_1 ); - RestRequest postRequest = TestBaseExtensionRestHandler.createTestRestRequest( + RestRequest postRequest = TestSDKRestRequest.createTestRestRequest( Method.POST, "/hello", + "/hello", params, + headers(XContentType.JSON), XContentType.JSON, new BytesArray("{\"adjective\":\"testable\"}"), - token + token, + HttpVersion.HTTP_1_1 ); - RestRequest badPostRequest = TestBaseExtensionRestHandler.createTestRestRequest( + RestRequest badPostRequest = TestSDKRestRequest.createTestRestRequest( Method.POST, "/hello", + "/hello", params, + headers(XContentType.JSON), XContentType.JSON, new BytesArray("{\"adjective\":\"\"}"), - token + token, + HttpVersion.HTTP_1_1 ); - RestRequest noContentPostRequest = TestBaseExtensionRestHandler.createTestRestRequest( + RestRequest noContentPostRequest = TestSDKRestRequest.createTestRestRequest( Method.POST, "/hello", + "/hello", params, + headers(null), null, new BytesArray(""), - token + token, + HttpVersion.HTTP_1_1 ); - RestRequest badContentTypePostRequest = TestBaseExtensionRestHandler.createTestRestRequest( + RestRequest badContentTypePostRequest = TestSDKRestRequest.createTestRestRequest( Method.POST, "/hello", + "/hello", params, + headers(XContentType.YAML), XContentType.YAML, new BytesArray("yaml:"), - token + token, + HttpVersion.HTTP_1_1 ); - RestRequest deleteRequest = TestBaseExtensionRestHandler.createTestRestRequest( + RestRequest deleteRequest = TestSDKRestRequest.createTestRestRequest( Method.DELETE, "/goodbye", + "/goodbye", params, - null, + headers(XContentType.JSON), + XContentType.JSON, new BytesArray(""), - token + token, + HttpVersion.HTTP_1_1 ); - RestRequest unhandledMethodRequest = TestBaseExtensionRestHandler.createTestRestRequest( + RestRequest unhandledMethodRequest = TestSDKRestRequest.createTestRestRequest( Method.HEAD, "/hi", + "/hi", params, - null, + headers(XContentType.JSON), + XContentType.JSON, new BytesArray(""), - token + token, + HttpVersion.HTTP_1_1 ); - RestRequest unhandledPathRequest = TestBaseExtensionRestHandler.createTestRestRequest( + RestRequest unhandledPathRequest = TestSDKRestRequest.createTestRestRequest( Method.GET, "/hi", + "/hi", params, - null, + headers(XContentType.JSON), + XContentType.JSON, new BytesArray(""), - token + token, + HttpVersion.HTTP_1_1 ); - RestRequest unhandledPathLengthRequest = TestBaseExtensionRestHandler.createTestRestRequest( + RestRequest unhandledPathLengthRequest = TestSDKRestRequest.createTestRestRequest( Method.DELETE, "/goodbye/cruel/world", + "/goodbye/cruel/world", params, - null, + headers(XContentType.JSON), + XContentType.JSON, new BytesArray(""), - token + token, + HttpVersion.HTTP_1_1 ); // Initial default response @@ -235,4 +266,8 @@ public void testHandleRequest() { responseStr = new String(BytesReference.toBytes(response.content()), StandardCharsets.UTF_8); assertTrue(responseStr.contains("/goodbye")); } + + private static Map> headers(XContentType type) { + return type == null ? Collections.emptyMap() : Map.of("Content-Type", List.of(type.mediaType())); + } } From f07c86cac1441fba953e8ddbbf1aad40b98bfac5 Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Fri, 7 Apr 2023 12:22:18 -0700 Subject: [PATCH 11/11] Use correct Test annotation so tests actually run Signed-off-by: Daniel Widdis --- .../opensearch/sdk/TestBaseExtensionRestHandler.java | 1 + .../opensearch/sdk/{ => rest}/TestSDKRestRequest.java | 10 ++++------ .../sample/helloworld/rest/TestRestHelloAction.java | 2 +- 3 files changed, 6 insertions(+), 7 deletions(-) rename src/test/java/org/opensearch/sdk/{ => rest}/TestSDKRestRequest.java (91%) diff --git a/src/test/java/org/opensearch/sdk/TestBaseExtensionRestHandler.java b/src/test/java/org/opensearch/sdk/TestBaseExtensionRestHandler.java index 08b18e9d..5184ecc1 100644 --- a/src/test/java/org/opensearch/sdk/TestBaseExtensionRestHandler.java +++ b/src/test/java/org/opensearch/sdk/TestBaseExtensionRestHandler.java @@ -19,6 +19,7 @@ import org.opensearch.extensions.rest.ExtensionRestResponse; import org.opensearch.rest.RestRequest; import org.opensearch.rest.RestRequest.Method; +import org.opensearch.sdk.rest.TestSDKRestRequest; import org.opensearch.rest.RestStatus; import org.opensearch.test.OpenSearchTestCase; diff --git a/src/test/java/org/opensearch/sdk/TestSDKRestRequest.java b/src/test/java/org/opensearch/sdk/rest/TestSDKRestRequest.java similarity index 91% rename from src/test/java/org/opensearch/sdk/TestSDKRestRequest.java rename to src/test/java/org/opensearch/sdk/rest/TestSDKRestRequest.java index 4b40b3e4..48b0b33e 100644 --- a/src/test/java/org/opensearch/sdk/TestSDKRestRequest.java +++ b/src/test/java/org/opensearch/sdk/rest/TestSDKRestRequest.java @@ -7,14 +7,14 @@ * compatible open source license. */ -package org.opensearch.sdk; +package org.opensearch.sdk.rest; import java.io.IOException; import java.util.Arrays; import java.util.List; import java.util.Map; -import org.junit.Test; +import org.junit.jupiter.api.Test; import org.opensearch.common.bytes.BytesArray; import org.opensearch.common.bytes.BytesReference; import org.opensearch.common.xcontent.XContentType; @@ -22,15 +22,13 @@ import org.opensearch.http.HttpRequest; import org.opensearch.rest.RestRequest; import org.opensearch.rest.RestRequest.Method; -import org.opensearch.sdk.rest.SDKHttpRequest; -import org.opensearch.sdk.rest.SDKRestRequest; import org.opensearch.test.OpenSearchTestCase; import static java.util.Map.entry; public class TestSDKRestRequest extends OpenSearchTestCase { @Test - public void TestSDKRestRequestMethods() throws IOException { + public void testSDKRestRequestMethods() throws IOException { RestRequest.Method expectedMethod = Method.GET; String expectedUri = "foobar?foo=bar&baz=42"; String expectedPath = "foo"; @@ -40,7 +38,7 @@ public void TestSDKRestRequestMethods() throws IOException { entry("foo", Arrays.asList("hello", "world")) ); XContentType exptectedXContentType = XContentType.JSON; - BytesReference expectedContent = new BytesArray("bar"); + BytesReference expectedContent = new BytesArray("{\"foo\":\"bar\"}"); RestRequest sdkRestRequest = createTestRestRequest( expectedMethod, diff --git a/src/test/java/org/opensearch/sdk/sample/helloworld/rest/TestRestHelloAction.java b/src/test/java/org/opensearch/sdk/sample/helloworld/rest/TestRestHelloAction.java index 78b62116..2fcfda5c 100644 --- a/src/test/java/org/opensearch/sdk/sample/helloworld/rest/TestRestHelloAction.java +++ b/src/test/java/org/opensearch/sdk/sample/helloworld/rest/TestRestHelloAction.java @@ -26,7 +26,7 @@ import org.opensearch.rest.RestResponse; import org.opensearch.rest.RestStatus; import org.opensearch.sdk.ExtensionRestHandler; -import org.opensearch.sdk.TestSDKRestRequest; +import org.opensearch.sdk.rest.TestSDKRestRequest; import org.opensearch.test.OpenSearchTestCase; public class TestRestHelloAction extends OpenSearchTestCase {