From 0a5736fcb489568733056494025ce2cef9f82df0 Mon Sep 17 00:00:00 2001 From: Oliver Lockwood Date: Wed, 22 May 2024 11:36:50 +0100 Subject: [PATCH 1/6] 13776: allow adding query parameters to RequestOptions Signed-off-by: Oliver Lockwood --- CHANGELOG.md | 1 + .../org/opensearch/client/RequestOptions.java | 41 ++++++++++++++++- .../org/opensearch/client/RestClient.java | 1 + .../client/RequestOptionsTests.java | 44 +++++++++++++++++++ 4 files changed, 85 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a4e920f3a202d..fdc5f16276b5a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Add support for Azure Managed Identity in repository-azure ([#12423](https://github.com/opensearch-project/OpenSearch/issues/12423)) - Add useCompoundFile index setting ([#13478](https://github.com/opensearch-project/OpenSearch/pull/13478)) - Make outbound side of transport protocol dependent ([#13293](https://github.com/opensearch-project/OpenSearch/pull/13293)) +- Allow setting query parameters on requests ([#13776](https://github.com/opensearch-project/OpenSearch/issues/13776)) ### Dependencies - Bump `com.github.spullara.mustache.java:compiler` from 0.9.10 to 0.9.13 ([#13329](https://github.com/opensearch-project/OpenSearch/pull/13329), [#13559](https://github.com/opensearch-project/OpenSearch/pull/13559)) 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)); } From 1028b6b94c0d34beeb7e96a25207b3a785a36b23 Mon Sep 17 00:00:00 2001 From: Oliver Lockwood Date: Wed, 22 May 2024 13:45:19 +0100 Subject: [PATCH 2/6] Fix bug highlighted by unit testing Signed-off-by: Oliver Lockwood --- .../src/main/java/org/opensearch/client/RequestOptions.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 227114dbb6786..a69237e1d07c3 100644 --- a/client/rest/src/main/java/org/opensearch/client/RequestOptions.java +++ b/client/rest/src/main/java/org/opensearch/client/RequestOptions.java @@ -222,7 +222,7 @@ private Builder( RequestConfig requestConfig ) { this.headers = new ArrayList<>(headers); - this.queryParams = new HashMap<>(); + this.queryParams = new HashMap<>(queryParams); this.httpAsyncResponseConsumerFactory = httpAsyncResponseConsumerFactory; this.warningsHandler = warningsHandler; this.requestConfig = requestConfig; From 0931c423c50738bc7573dd5234f662d00509071a Mon Sep 17 00:00:00 2001 From: Oliver Lockwood Date: Thu, 23 May 2024 09:11:46 +0100 Subject: [PATCH 3/6] Address code review comments Signed-off-by: Oliver Lockwood --- .../org/opensearch/client/RequestOptions.java | 28 +++++++++---------- .../org/opensearch/client/RestClient.java | 2 +- .../client/RequestOptionsTests.java | 27 +++++++----------- 3 files changed, 25 insertions(+), 32 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 a69237e1d07c3..f860c25e4d6c5 100644 --- a/client/rest/src/main/java/org/opensearch/client/RequestOptions.java +++ b/client/rest/src/main/java/org/opensearch/client/RequestOptions.java @@ -63,14 +63,14 @@ public final class RequestOptions { ).build(); private final List
headers; - private final Map queryParams; + private final Map parameters; 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.parameters = Collections.unmodifiableMap(new HashMap<>(builder.parameters)); this.httpAsyncResponseConsumerFactory = builder.httpAsyncResponseConsumerFactory; this.warningsHandler = builder.warningsHandler; this.requestConfig = builder.requestConfig; @@ -80,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, queryParams, httpAsyncResponseConsumerFactory, warningsHandler, requestConfig); + return new Builder(headers, parameters, httpAsyncResponseConsumerFactory, warningsHandler, requestConfig); } /** @@ -93,8 +93,8 @@ public List
getHeaders() { /** * Query parameters to attach to the request. */ - public Map getQueryParams() { - return queryParams; + public Map getParameters() { + return parameters; } /** @@ -155,11 +155,11 @@ public String toString() { b.append(headers.get(h).toString()); } } - if (queryParams.size() > 0) { + if (parameters.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(","))); + b.append(parameters.entrySet().stream().map(e -> e.getKey() + "=" + e.getValue()).collect(Collectors.joining(","))); } if (httpAsyncResponseConsumerFactory != HttpAsyncResponseConsumerFactory.DEFAULT) { if (comma) b.append(", "); @@ -189,7 +189,7 @@ public boolean equals(Object obj) { RequestOptions other = (RequestOptions) obj; return headers.equals(other.headers) - && queryParams.equals(other.queryParams) + && parameters.equals(other.parameters) && httpAsyncResponseConsumerFactory.equals(other.httpAsyncResponseConsumerFactory) && Objects.equals(warningsHandler, other.warningsHandler); } @@ -199,7 +199,7 @@ public boolean equals(Object obj) { */ @Override public int hashCode() { - return Objects.hash(headers, queryParams, httpAsyncResponseConsumerFactory, warningsHandler); + return Objects.hash(headers, parameters, httpAsyncResponseConsumerFactory, warningsHandler); } /** @@ -209,20 +209,20 @@ public int hashCode() { */ public static class Builder { private final List
headers; - private final Map queryParams; + private final Map parameters; private HttpAsyncResponseConsumerFactory httpAsyncResponseConsumerFactory; private WarningsHandler warningsHandler; private RequestConfig requestConfig; private Builder( List
headers, - Map queryParams, + Map parameters, HttpAsyncResponseConsumerFactory httpAsyncResponseConsumerFactory, WarningsHandler warningsHandler, RequestConfig requestConfig ) { this.headers = new ArrayList<>(headers); - this.queryParams = new HashMap<>(queryParams); + this.parameters = new HashMap<>(parameters); this.httpAsyncResponseConsumerFactory = httpAsyncResponseConsumerFactory; this.warningsHandler = warningsHandler; this.requestConfig = requestConfig; @@ -256,10 +256,10 @@ public Builder addHeader(String name, String value) { * @param value the query parameter value * @throws NullPointerException if {@code name} or {@code value} is null. */ - public Builder addQueryParam(String name, String value) { + public Builder addParameter(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); + this.parameters.put(name, value); return this; } 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 1083117f988fb..10a5e05a4d2b5 100644 --- a/client/rest/src/main/java/org/opensearch/client/RestClient.java +++ b/client/rest/src/main/java/org/opensearch/client/RestClient.java @@ -832,7 +832,7 @@ private class InternalRequest { InternalRequest(Request request) { this.request = request; Map params = new HashMap<>(request.getParameters()); - params.putAll(request.getOptions().getQueryParams()); + params.putAll(request.getOptions().getParameters()); // 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 5b2013d898200..5c3d2da57760e 100644 --- a/client/rest/src/test/java/org/opensearch/client/RequestOptionsTests.java +++ b/client/rest/src/test/java/org/opensearch/client/RequestOptionsTests.java @@ -47,6 +47,7 @@ import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertThrows; import static org.junit.Assert.fail; import static org.mockito.Mockito.mock; @@ -92,20 +93,12 @@ 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()); - } + public void testAddParameter() { + assertThrows("query parameter name cannot be null", NullPointerException.class, + () -> randomBuilder().addParameter(null, randomAsciiLettersOfLengthBetween(3, 10))); - try { - randomBuilder().addQueryParam(randomAsciiLettersOfLengthBetween(3, 10), null); - fail("expected failure"); - } catch (NullPointerException e) { - assertEquals("query parameter value cannot be null", e.getMessage()); - } + assertThrows("query parameter value cannot be null", NullPointerException.class, + () -> randomBuilder().addParameter(randomAsciiLettersOfLengthBetween(3, 10), null)); RequestOptions.Builder builder = RequestOptions.DEFAULT.toBuilder(); int numQueryParams = between(0, 5); @@ -114,13 +107,13 @@ public void testAddQueryParam() { String name = randomAsciiAlphanumOfLengthBetween(5, 10); String value = randomAsciiAlphanumOfLength(3); queryParams.put(name, value); - builder.addQueryParam(name, value); + builder.addParameter(name, value); } RequestOptions options = builder.build(); - assertEquals(queryParams, options.getQueryParams()); + assertEquals(queryParams, options.getParameters()); try { - options.getQueryParams().put(randomAsciiAlphanumOfLengthBetween(5, 10), randomAsciiAlphanumOfLength(3)); + options.getParameters().put(randomAsciiAlphanumOfLengthBetween(5, 10), randomAsciiAlphanumOfLength(3)); fail("expected failure"); } catch (UnsupportedOperationException e) { assertNull(e.getMessage()); @@ -185,7 +178,7 @@ static RequestOptions.Builder randomBuilder() { if (randomBoolean()) { int queryParamCount = between(1, 5); for (int i = 0; i < queryParamCount; i++) { - builder.addQueryParam(randomAsciiAlphanumOfLength(3), randomAsciiAlphanumOfLength(3)); + builder.addParameter(randomAsciiAlphanumOfLength(3), randomAsciiAlphanumOfLength(3)); } } From ad51c821a570268899a05a5a6d3d8b54d492ddf2 Mon Sep 17 00:00:00 2001 From: Oliver Lockwood Date: Thu, 23 May 2024 09:19:50 +0100 Subject: [PATCH 4/6] Run spotlessApply to satisfy formatting rules Signed-off-by: Oliver Lockwood --- .../opensearch/client/RequestOptionsTests.java | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) 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 5c3d2da57760e..d32ef8de6d14d 100644 --- a/client/rest/src/test/java/org/opensearch/client/RequestOptionsTests.java +++ b/client/rest/src/test/java/org/opensearch/client/RequestOptionsTests.java @@ -94,11 +94,17 @@ public void testAddHeader() { } public void testAddParameter() { - assertThrows("query parameter name cannot be null", NullPointerException.class, - () -> randomBuilder().addParameter(null, randomAsciiLettersOfLengthBetween(3, 10))); - - assertThrows("query parameter value cannot be null", NullPointerException.class, - () -> randomBuilder().addParameter(randomAsciiLettersOfLengthBetween(3, 10), null)); + assertThrows( + "query parameter name cannot be null", + NullPointerException.class, + () -> randomBuilder().addParameter(null, randomAsciiLettersOfLengthBetween(3, 10)) + ); + + assertThrows( + "query parameter value cannot be null", + NullPointerException.class, + () -> randomBuilder().addParameter(randomAsciiLettersOfLengthBetween(3, 10), null) + ); RequestOptions.Builder builder = RequestOptions.DEFAULT.toBuilder(); int numQueryParams = between(0, 5); From d0420a3ed0c54b8594b53b4af8907e2409a0cb32 Mon Sep 17 00:00:00 2001 From: Oliver Lockwood Date: Thu, 23 May 2024 17:57:09 +0100 Subject: [PATCH 5/6] Fix more queryParams->parameters cases Signed-off-by: Oliver Lockwood --- .../java/org/opensearch/client/RequestOptions.java | 2 +- .../org/opensearch/client/RequestOptionsTests.java | 10 +++++----- 2 files changed, 6 insertions(+), 6 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 f860c25e4d6c5..4ecc6547cd54c 100644 --- a/client/rest/src/main/java/org/opensearch/client/RequestOptions.java +++ b/client/rest/src/main/java/org/opensearch/client/RequestOptions.java @@ -158,7 +158,7 @@ public String toString() { if (parameters.size() > 0) { if (comma) b.append(", "); comma = true; - b.append("queryParams="); + b.append("parameters="); b.append(parameters.entrySet().stream().map(e -> e.getKey() + "=" + e.getValue()).collect(Collectors.joining(","))); } if (httpAsyncResponseConsumerFactory != HttpAsyncResponseConsumerFactory.DEFAULT) { 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 d32ef8de6d14d..06fc92559c2d3 100644 --- a/client/rest/src/test/java/org/opensearch/client/RequestOptionsTests.java +++ b/client/rest/src/test/java/org/opensearch/client/RequestOptionsTests.java @@ -107,16 +107,16 @@ public void testAddParameter() { ); RequestOptions.Builder builder = RequestOptions.DEFAULT.toBuilder(); - int numQueryParams = between(0, 5); - Map queryParams = new HashMap<>(); - for (int i = 0; i < numQueryParams; i++) { + int numParameters = between(0, 5); + Map parameters = new HashMap<>(); + for (int i = 0; i < numParameters; i++) { String name = randomAsciiAlphanumOfLengthBetween(5, 10); String value = randomAsciiAlphanumOfLength(3); - queryParams.put(name, value); + parameters.put(name, value); builder.addParameter(name, value); } RequestOptions options = builder.build(); - assertEquals(queryParams, options.getParameters()); + assertEquals(parameters, options.getParameters()); try { options.getParameters().put(randomAsciiAlphanumOfLengthBetween(5, 10), randomAsciiAlphanumOfLength(3)); From 963f9291f9810962cf0e558773557e40c0ef0ec9 Mon Sep 17 00:00:00 2001 From: Oliver Lockwood Date: Tue, 28 May 2024 13:40:30 +0100 Subject: [PATCH 6/6] Apply code review feedback Signed-off-by: Oliver Lockwood --- .../rest/src/main/java/org/opensearch/client/Request.java | 8 +++++++- .../main/java/org/opensearch/client/RequestOptions.java | 6 ++++-- .../src/main/java/org/opensearch/client/RestClient.java | 1 - 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/client/rest/src/main/java/org/opensearch/client/Request.java b/client/rest/src/main/java/org/opensearch/client/Request.java index 441b01b0891ad..32fedee0c97bf 100644 --- a/client/rest/src/main/java/org/opensearch/client/Request.java +++ b/client/rest/src/main/java/org/opensearch/client/Request.java @@ -110,7 +110,13 @@ public void addParameters(Map paramSource) { * will change it. */ public Map getParameters() { - return unmodifiableMap(parameters); + if (options.getParameters().isEmpty()) { + return unmodifiableMap(parameters); + } else { + Map combinedParameters = new HashMap<>(parameters); + combinedParameters.putAll(options.getParameters()); + return unmodifiableMap(combinedParameters); + } } /** 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 4ecc6547cd54c..bbc1f8bc85fcb 100644 --- a/client/rest/src/main/java/org/opensearch/client/RequestOptions.java +++ b/client/rest/src/main/java/org/opensearch/client/RequestOptions.java @@ -91,7 +91,8 @@ public List
getHeaders() { } /** - * Query parameters to attach to the request. + * Query parameters to attach to the request. Any parameters present here + * will override matching parameters in the {@link Request}, if they exist. */ public Map getParameters() { return parameters; @@ -250,7 +251,8 @@ public Builder addHeader(String name, String value) { } /** - * Add the provided query parameter to the request. + * Add the provided query parameter to the request. Any parameters added here + * will override matching parameters in the {@link Request}, if they exist. * * @param name the query parameter name * @param value the query parameter value 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 10a5e05a4d2b5..15905add76c4f 100644 --- a/client/rest/src/main/java/org/opensearch/client/RestClient.java +++ b/client/rest/src/main/java/org/opensearch/client/RestClient.java @@ -832,7 +832,6 @@ private class InternalRequest { InternalRequest(Request request) { this.request = request; Map params = new HashMap<>(request.getParameters()); - params.putAll(request.getOptions().getParameters()); // 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());