From 618b4dd9edb62edae57119781e301d27ca2370bb Mon Sep 17 00:00:00 2001 From: Oliver Lockwood Date: Wed, 22 May 2024 11:36:50 +0100 Subject: [PATCH] #13776: allow adding query parameters to RequestOptions Signed-off-by: Oliver Lockwood --- .../org/opensearch/client/RequestOptions.java | 41 ++++++++++++++++- .../org/opensearch/client/RestClient.java | 1 + .../client/RequestOptionsTests.java | 44 +++++++++++++++++++ 3 files changed, 84 insertions(+), 2 deletions(-) diff --git a/client/rest/src/main/java/org/opensearch/client/RequestOptions.java b/client/rest/src/main/java/org/opensearch/client/RequestOptions.java index 189d785faaf45..227114dbb6786 100644 --- a/client/rest/src/main/java/org/opensearch/client/RequestOptions.java +++ b/client/rest/src/main/java/org/opensearch/client/RequestOptions.java @@ -40,8 +40,11 @@ import java.util.ArrayList; import java.util.Collections; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.Objects; +import java.util.stream.Collectors; /** * The portion of an HTTP request to OpenSearch that can be @@ -53,18 +56,21 @@ public final class RequestOptions { */ public static final RequestOptions DEFAULT = new Builder( Collections.emptyList(), + Collections.emptyMap(), HeapBufferedResponseConsumerFactory.DEFAULT, null, null ).build(); private final List
headers; + private final Map queryParams; private final HttpAsyncResponseConsumerFactory httpAsyncResponseConsumerFactory; private final WarningsHandler warningsHandler; private final RequestConfig requestConfig; private RequestOptions(Builder builder) { this.headers = Collections.unmodifiableList(new ArrayList<>(builder.headers)); + this.queryParams = Collections.unmodifiableMap(new HashMap<>(builder.queryParams)); this.httpAsyncResponseConsumerFactory = builder.httpAsyncResponseConsumerFactory; this.warningsHandler = builder.warningsHandler; this.requestConfig = builder.requestConfig; @@ -74,7 +80,7 @@ private RequestOptions(Builder builder) { * Create a builder that contains these options but can be modified. */ public Builder toBuilder() { - return new Builder(headers, httpAsyncResponseConsumerFactory, warningsHandler, requestConfig); + return new Builder(headers, queryParams, httpAsyncResponseConsumerFactory, warningsHandler, requestConfig); } /** @@ -84,6 +90,13 @@ public List
getHeaders() { return headers; } + /** + * Query parameters to attach to the request. + */ + public Map getQueryParams() { + return queryParams; + } + /** * The {@link HttpAsyncResponseConsumerFactory} used to create one * {@link AsyncResponseConsumer} callback per retry. Controls how the @@ -142,6 +155,12 @@ public String toString() { b.append(headers.get(h).toString()); } } + if (queryParams.size() > 0) { + if (comma) b.append(", "); + comma = true; + b.append("queryParams="); + b.append(queryParams.entrySet().stream().map(e -> e.getKey() + "=" + e.getValue()).collect(Collectors.joining(","))); + } if (httpAsyncResponseConsumerFactory != HttpAsyncResponseConsumerFactory.DEFAULT) { if (comma) b.append(", "); comma = true; @@ -170,6 +189,7 @@ public boolean equals(Object obj) { RequestOptions other = (RequestOptions) obj; return headers.equals(other.headers) + && queryParams.equals(other.queryParams) && httpAsyncResponseConsumerFactory.equals(other.httpAsyncResponseConsumerFactory) && Objects.equals(warningsHandler, other.warningsHandler); } @@ -179,7 +199,7 @@ public boolean equals(Object obj) { */ @Override public int hashCode() { - return Objects.hash(headers, httpAsyncResponseConsumerFactory, warningsHandler); + return Objects.hash(headers, queryParams, httpAsyncResponseConsumerFactory, warningsHandler); } /** @@ -189,17 +209,20 @@ public int hashCode() { */ public static class Builder { private final List
headers; + private final Map queryParams; private HttpAsyncResponseConsumerFactory httpAsyncResponseConsumerFactory; private WarningsHandler warningsHandler; private RequestConfig requestConfig; private Builder( List
headers, + Map queryParams, HttpAsyncResponseConsumerFactory httpAsyncResponseConsumerFactory, WarningsHandler warningsHandler, RequestConfig requestConfig ) { this.headers = new ArrayList<>(headers); + this.queryParams = new HashMap<>(); this.httpAsyncResponseConsumerFactory = httpAsyncResponseConsumerFactory; this.warningsHandler = warningsHandler; this.requestConfig = requestConfig; @@ -226,6 +249,20 @@ public Builder addHeader(String name, String value) { return this; } + /** + * Add the provided query parameter to the request. + * + * @param name the query parameter name + * @param value the query parameter value + * @throws NullPointerException if {@code name} or {@code value} is null. + */ + public Builder addQueryParam(String name, String value) { + Objects.requireNonNull(name, "query parameter name cannot be null"); + Objects.requireNonNull(value, "query parameter value cannot be null"); + this.queryParams.put(name, value); + return this; + } + /** * Set the {@link HttpAsyncResponseConsumerFactory} used to create one * {@link AsyncResponseConsumer} callback per retry. Controls how the diff --git a/client/rest/src/main/java/org/opensearch/client/RestClient.java b/client/rest/src/main/java/org/opensearch/client/RestClient.java index 15905add76c4f..1083117f988fb 100644 --- a/client/rest/src/main/java/org/opensearch/client/RestClient.java +++ b/client/rest/src/main/java/org/opensearch/client/RestClient.java @@ -832,6 +832,7 @@ private class InternalRequest { InternalRequest(Request request) { this.request = request; Map params = new HashMap<>(request.getParameters()); + params.putAll(request.getOptions().getQueryParams()); // ignore is a special parameter supported by the clients, shouldn't be sent to es String ignoreString = params.remove("ignore"); this.ignoreErrorCodes = getIgnoreErrorCodes(ignoreString, request.getMethod()); diff --git a/client/rest/src/test/java/org/opensearch/client/RequestOptionsTests.java b/client/rest/src/test/java/org/opensearch/client/RequestOptionsTests.java index a7f9a48c73393..5b2013d898200 100644 --- a/client/rest/src/test/java/org/opensearch/client/RequestOptionsTests.java +++ b/client/rest/src/test/java/org/opensearch/client/RequestOptionsTests.java @@ -39,7 +39,9 @@ import java.util.ArrayList; import java.util.Collections; +import java.util.HashMap; import java.util.List; +import java.util.Map; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotEquals; @@ -90,6 +92,41 @@ public void testAddHeader() { } } + public void testAddQueryParam() { + try { + randomBuilder().addQueryParam(null, randomAsciiLettersOfLengthBetween(3, 10)); + fail("expected failure"); + } catch (NullPointerException e) { + assertEquals("query parameter name cannot be null", e.getMessage()); + } + + try { + randomBuilder().addQueryParam(randomAsciiLettersOfLengthBetween(3, 10), null); + fail("expected failure"); + } catch (NullPointerException e) { + assertEquals("query parameter value cannot be null", e.getMessage()); + } + + RequestOptions.Builder builder = RequestOptions.DEFAULT.toBuilder(); + int numQueryParams = between(0, 5); + Map queryParams = new HashMap<>(); + for (int i = 0; i < numQueryParams; i++) { + String name = randomAsciiAlphanumOfLengthBetween(5, 10); + String value = randomAsciiAlphanumOfLength(3); + queryParams.put(name, value); + builder.addQueryParam(name, value); + } + RequestOptions options = builder.build(); + assertEquals(queryParams, options.getQueryParams()); + + try { + options.getQueryParams().put(randomAsciiAlphanumOfLengthBetween(5, 10), randomAsciiAlphanumOfLength(3)); + fail("expected failure"); + } catch (UnsupportedOperationException e) { + assertNull(e.getMessage()); + } + } + public void testSetHttpAsyncResponseConsumerFactory() { try { RequestOptions.DEFAULT.toBuilder().setHttpAsyncResponseConsumerFactory(null); @@ -145,6 +182,13 @@ static RequestOptions.Builder randomBuilder() { } } + if (randomBoolean()) { + int queryParamCount = between(1, 5); + for (int i = 0; i < queryParamCount; i++) { + builder.addQueryParam(randomAsciiAlphanumOfLength(3), randomAsciiAlphanumOfLength(3)); + } + } + if (randomBoolean()) { builder.setHttpAsyncResponseConsumerFactory(new HeapBufferedResponseConsumerFactory(1)); }