Skip to content

Commit a32859b

Browse files
oliverlockwoodoliverlockwood
authored
13776: allow adding query parameters to RequestOptions (#13777)
* 13776: allow adding query parameters to RequestOptions Signed-off-by: Oliver Lockwood <oliver.lockwood@cantab.net> * Fix bug highlighted by unit testing Signed-off-by: Oliver Lockwood <oliver.lockwood@cantab.net> * Address code review comments Signed-off-by: Oliver Lockwood <oliver.lockwood@cantab.net> * Run spotlessApply to satisfy formatting rules Signed-off-by: Oliver Lockwood <oliver.lockwood@cantab.net> * Fix more queryParams->parameters cases Signed-off-by: Oliver Lockwood <oliver.lockwood@cantab.net> * Apply code review feedback Signed-off-by: Oliver Lockwood <oliver.lockwood@cantab.net> --------- Signed-off-by: Oliver Lockwood <oliver.lockwood@cantab.net>
1 parent 6c9603a commit a32859b

File tree

4 files changed

+92
-3
lines changed

4 files changed

+92
-3
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
1313
- [Remote Store] Upload translog checkpoint as object metadata to translog.tlog([#13637](https://github.com/opensearch-project/OpenSearch/pull/13637))
1414
- Add getMetadataFields to MapperService ([#13819](https://github.com/opensearch-project/OpenSearch/pull/13819))
1515
- [Remote State] Add async remote state deletion task running on an interval, configurable by a setting ([#13131](https://github.com/opensearch-project/OpenSearch/pull/13131))
16+
- Allow setting query parameters on requests ([#13776](https://github.com/opensearch-project/OpenSearch/issues/13776))
1617

1718
### Dependencies
1819
- 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))

‎client/rest/src/main/java/org/opensearch/client/Request.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,13 @@ public void addParameters(Map<String, String> paramSource) {
110110
* will change it.
111111
*/
112112
public Map<String, String> getParameters() {
113-
return unmodifiableMap(parameters);
113+
if (options.getParameters().isEmpty()) {
114+
return unmodifiableMap(parameters);
115+
} else {
116+
Map<String, String> combinedParameters = new HashMap<>(parameters);
117+
combinedParameters.putAll(options.getParameters());
118+
return unmodifiableMap(combinedParameters);
119+
}
114120
}
115121

116122
/**

‎client/rest/src/main/java/org/opensearch/client/RequestOptions.java

Lines changed: 41 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,11 @@
4040

4141
import java.util.ArrayList;
4242
import java.util.Collections;
43+
import java.util.HashMap;
4344
import java.util.List;
45+
import java.util.Map;
4446
import java.util.Objects;
47+
import java.util.stream.Collectors;
4548

4649
/**
4750
* The portion of an HTTP request to OpenSearch that can be
@@ -53,18 +56,21 @@ public final class RequestOptions {
5356
*/
5457
public static final RequestOptions DEFAULT = new Builder(
5558
Collections.emptyList(),
59+
Collections.emptyMap(),
5660
HeapBufferedResponseConsumerFactory.DEFAULT,
5761
null,
5862
null
5963
).build();
6064

6165
private final List<Header> headers;
66+
private final Map<String, String> parameters;
6267
private final HttpAsyncResponseConsumerFactory httpAsyncResponseConsumerFactory;
6368
private final WarningsHandler warningsHandler;
6469
private final RequestConfig requestConfig;
6570

6671
private RequestOptions(Builder builder) {
6772
this.headers = Collections.unmodifiableList(new ArrayList<>(builder.headers));
73+
this.parameters = Collections.unmodifiableMap(new HashMap<>(builder.parameters));
6874
this.httpAsyncResponseConsumerFactory = builder.httpAsyncResponseConsumerFactory;
6975
this.warningsHandler = builder.warningsHandler;
7076
this.requestConfig = builder.requestConfig;
@@ -74,7 +80,7 @@ private RequestOptions(Builder builder) {
7480
* Create a builder that contains these options but can be modified.
7581
*/
7682
public Builder toBuilder() {
77-
return new Builder(headers, httpAsyncResponseConsumerFactory, warningsHandler, requestConfig);
83+
return new Builder(headers, parameters, httpAsyncResponseConsumerFactory, warningsHandler, requestConfig);
7884
}
7985

8086
/**
@@ -84,6 +90,14 @@ public List<Header> getHeaders() {
8490
return headers;
8591
}
8692

93+
/**
94+
* Query parameters to attach to the request. Any parameters present here
95+
* will override matching parameters in the {@link Request}, if they exist.
96+
*/
97+
public Map<String, String> getParameters() {
98+
return parameters;
99+
}
100+
87101
/**
88102
* The {@link HttpAsyncResponseConsumerFactory} used to create one
89103
* {@link AsyncResponseConsumer} callback per retry. Controls how the
@@ -142,6 +156,12 @@ public String toString() {
142156
b.append(headers.get(h).toString());
143157
}
144158
}
159+
if (parameters.size() > 0) {
160+
if (comma) b.append(", ");
161+
comma = true;
162+
b.append("parameters=");
163+
b.append(parameters.entrySet().stream().map(e -> e.getKey() + "=" + e.getValue()).collect(Collectors.joining(",")));
164+
}
145165
if (httpAsyncResponseConsumerFactory != HttpAsyncResponseConsumerFactory.DEFAULT) {
146166
if (comma) b.append(", ");
147167
comma = true;
@@ -170,6 +190,7 @@ public boolean equals(Object obj) {
170190

171191
RequestOptions other = (RequestOptions) obj;
172192
return headers.equals(other.headers)
193+
&& parameters.equals(other.parameters)
173194
&& httpAsyncResponseConsumerFactory.equals(other.httpAsyncResponseConsumerFactory)
174195
&& Objects.equals(warningsHandler, other.warningsHandler);
175196
}
@@ -179,7 +200,7 @@ public boolean equals(Object obj) {
179200
*/
180201
@Override
181202
public int hashCode() {
182-
return Objects.hash(headers, httpAsyncResponseConsumerFactory, warningsHandler);
203+
return Objects.hash(headers, parameters, httpAsyncResponseConsumerFactory, warningsHandler);
183204
}
184205

185206
/**
@@ -189,17 +210,20 @@ public int hashCode() {
189210
*/
190211
public static class Builder {
191212
private final List<Header> headers;
213+
private final Map<String, String> parameters;
192214
private HttpAsyncResponseConsumerFactory httpAsyncResponseConsumerFactory;
193215
private WarningsHandler warningsHandler;
194216
private RequestConfig requestConfig;
195217

196218
private Builder(
197219
List<Header> headers,
220+
Map<String, String> parameters,
198221
HttpAsyncResponseConsumerFactory httpAsyncResponseConsumerFactory,
199222
WarningsHandler warningsHandler,
200223
RequestConfig requestConfig
201224
) {
202225
this.headers = new ArrayList<>(headers);
226+
this.parameters = new HashMap<>(parameters);
203227
this.httpAsyncResponseConsumerFactory = httpAsyncResponseConsumerFactory;
204228
this.warningsHandler = warningsHandler;
205229
this.requestConfig = requestConfig;
@@ -226,6 +250,21 @@ public Builder addHeader(String name, String value) {
226250
return this;
227251
}
228252

253+
/**
254+
* Add the provided query parameter to the request. Any parameters added here
255+
* will override matching parameters in the {@link Request}, if they exist.
256+
*
257+
* @param name the query parameter name
258+
* @param value the query parameter value
259+
* @throws NullPointerException if {@code name} or {@code value} is null.
260+
*/
261+
public Builder addParameter(String name, String value) {
262+
Objects.requireNonNull(name, "query parameter name cannot be null");
263+
Objects.requireNonNull(value, "query parameter value cannot be null");
264+
this.parameters.put(name, value);
265+
return this;
266+
}
267+
229268
/**
230269
* Set the {@link HttpAsyncResponseConsumerFactory} used to create one
231270
* {@link AsyncResponseConsumer} callback per retry. Controls how the

‎client/rest/src/test/java/org/opensearch/client/RequestOptionsTests.java

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,12 +39,15 @@
3939

4040
import java.util.ArrayList;
4141
import java.util.Collections;
42+
import java.util.HashMap;
4243
import java.util.List;
44+
import java.util.Map;
4345

4446
import static org.junit.Assert.assertEquals;
4547
import static org.junit.Assert.assertNotEquals;
4648
import static org.junit.Assert.assertNull;
4749
import static org.junit.Assert.assertSame;
50+
import static org.junit.Assert.assertThrows;
4851
import static org.junit.Assert.fail;
4952
import static org.mockito.Mockito.mock;
5053

@@ -90,6 +93,39 @@ public void testAddHeader() {
9093
}
9194
}
9295

96+
public void testAddParameter() {
97+
assertThrows(
98+
"query parameter name cannot be null",
99+
NullPointerException.class,
100+
() -> randomBuilder().addParameter(null, randomAsciiLettersOfLengthBetween(3, 10))
101+
);
102+
103+
assertThrows(
104+
"query parameter value cannot be null",
105+
NullPointerException.class,
106+
() -> randomBuilder().addParameter(randomAsciiLettersOfLengthBetween(3, 10), null)
107+
);
108+
109+
RequestOptions.Builder builder = RequestOptions.DEFAULT.toBuilder();
110+
int numParameters = between(0, 5);
111+
Map<String, String> parameters = new HashMap<>();
112+
for (int i = 0; i < numParameters; i++) {
113+
String name = randomAsciiAlphanumOfLengthBetween(5, 10);
114+
String value = randomAsciiAlphanumOfLength(3);
115+
parameters.put(name, value);
116+
builder.addParameter(name, value);
117+
}
118+
RequestOptions options = builder.build();
119+
assertEquals(parameters, options.getParameters());
120+
121+
try {
122+
options.getParameters().put(randomAsciiAlphanumOfLengthBetween(5, 10), randomAsciiAlphanumOfLength(3));
123+
fail("expected failure");
124+
} catch (UnsupportedOperationException e) {
125+
assertNull(e.getMessage());
126+
}
127+
}
128+
93129
public void testSetHttpAsyncResponseConsumerFactory() {
94130
try {
95131
RequestOptions.DEFAULT.toBuilder().setHttpAsyncResponseConsumerFactory(null);
@@ -145,6 +181,13 @@ static RequestOptions.Builder randomBuilder() {
145181
}
146182
}
147183

184+
if (randomBoolean()) {
185+
int queryParamCount = between(1, 5);
186+
for (int i = 0; i < queryParamCount; i++) {
187+
builder.addParameter(randomAsciiAlphanumOfLength(3), randomAsciiAlphanumOfLength(3));
188+
}
189+
}
190+
148191
if (randomBoolean()) {
149192
builder.setHttpAsyncResponseConsumerFactory(new HeapBufferedResponseConsumerFactory(1));
150193
}

0 commit comments

Comments
 (0)