Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

13776: allow adding query parameters to RequestOptions #13777

Merged
merged 9 commits into from
May 31, 2024
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
- [Remote Store] Add dynamic cluster settings to set timeout for segments upload to Remote Store ([#13679](https://github.com/opensearch-project/OpenSearch/pull/13679))
- [Remote Store] Upload translog checkpoint as object metadata to translog.tlog([#13637](https://github.com/opensearch-project/OpenSearch/pull/13637))
- Add getMetadataFields to MapperService ([#13819](https://github.com/opensearch-project/OpenSearch/pull/13819))
- 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))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,13 @@
* will change it.
*/
public Map<String, String> getParameters() {
return unmodifiableMap(parameters);
if (options.getParameters().isEmpty()) {
return unmodifiableMap(parameters);
} else {
Map<String, String> combinedParameters = new HashMap<>(parameters);
combinedParameters.putAll(options.getParameters());
return unmodifiableMap(combinedParameters);

Check warning on line 118 in client/rest/src/main/java/org/opensearch/client/Request.java

View check run for this annotation

Codecov / codecov/patch

client/rest/src/main/java/org/opensearch/client/Request.java#L116-L118

Added lines #L116 - L118 were not covered by tests
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -53,18 +56,21 @@
*/
public static final RequestOptions DEFAULT = new Builder(
Collections.emptyList(),
Collections.emptyMap(),
HeapBufferedResponseConsumerFactory.DEFAULT,
null,
null
).build();

private final List<Header> headers;
private final Map<String, String> 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.parameters = Collections.unmodifiableMap(new HashMap<>(builder.parameters));
this.httpAsyncResponseConsumerFactory = builder.httpAsyncResponseConsumerFactory;
this.warningsHandler = builder.warningsHandler;
this.requestConfig = builder.requestConfig;
Expand All @@ -74,7 +80,7 @@
* 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, parameters, httpAsyncResponseConsumerFactory, warningsHandler, requestConfig);
}

/**
Expand All @@ -84,6 +90,14 @@
return headers;
}

/**
* Query parameters to attach to the request. Any parameters present here
* will override matching parameters in the {@link Request}, if they exist.
*/
public Map<String, String> getParameters() {
return parameters;
}

/**
* The {@link HttpAsyncResponseConsumerFactory} used to create one
* {@link AsyncResponseConsumer} callback per retry. Controls how the
Expand Down Expand Up @@ -142,6 +156,12 @@
b.append(headers.get(h).toString());
}
}
if (parameters.size() > 0) {
if (comma) b.append(", ");
comma = true;
b.append("parameters=");
b.append(parameters.entrySet().stream().map(e -> e.getKey() + "=" + e.getValue()).collect(Collectors.joining(",")));

Check warning on line 163 in client/rest/src/main/java/org/opensearch/client/RequestOptions.java

View check run for this annotation

Codecov / codecov/patch

client/rest/src/main/java/org/opensearch/client/RequestOptions.java#L161-L163

Added lines #L161 - L163 were not covered by tests
}
if (httpAsyncResponseConsumerFactory != HttpAsyncResponseConsumerFactory.DEFAULT) {
if (comma) b.append(", ");
comma = true;
Expand Down Expand Up @@ -170,6 +190,7 @@

RequestOptions other = (RequestOptions) obj;
return headers.equals(other.headers)
&& parameters.equals(other.parameters)
&& httpAsyncResponseConsumerFactory.equals(other.httpAsyncResponseConsumerFactory)
&& Objects.equals(warningsHandler, other.warningsHandler);
}
Expand All @@ -179,7 +200,7 @@
*/
@Override
public int hashCode() {
return Objects.hash(headers, httpAsyncResponseConsumerFactory, warningsHandler);
return Objects.hash(headers, parameters, httpAsyncResponseConsumerFactory, warningsHandler);
}

/**
Expand All @@ -189,17 +210,20 @@
*/
public static class Builder {
private final List<Header> headers;
private final Map<String, String> parameters;
private HttpAsyncResponseConsumerFactory httpAsyncResponseConsumerFactory;
private WarningsHandler warningsHandler;
private RequestConfig requestConfig;

private Builder(
List<Header> headers,
Map<String, String> parameters,
HttpAsyncResponseConsumerFactory httpAsyncResponseConsumerFactory,
WarningsHandler warningsHandler,
RequestConfig requestConfig
) {
this.headers = new ArrayList<>(headers);
this.parameters = new HashMap<>(parameters);
this.httpAsyncResponseConsumerFactory = httpAsyncResponseConsumerFactory;
this.warningsHandler = warningsHandler;
this.requestConfig = requestConfig;
Expand All @@ -226,6 +250,21 @@
return this;
}

/**
* 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
* @throws NullPointerException if {@code name} or {@code value} is null.
*/
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.parameters.put(name, value);
return this;
}

/**
* Set the {@link HttpAsyncResponseConsumerFactory} used to create one
* {@link AsyncResponseConsumer} callback per retry. Controls how the
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,15 @@

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;
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;

Expand Down Expand Up @@ -90,6 +93,39 @@ 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)
);

RequestOptions.Builder builder = RequestOptions.DEFAULT.toBuilder();
int numParameters = between(0, 5);
Map<String, String> parameters = new HashMap<>();
for (int i = 0; i < numParameters; i++) {
String name = randomAsciiAlphanumOfLengthBetween(5, 10);
String value = randomAsciiAlphanumOfLength(3);
parameters.put(name, value);
builder.addParameter(name, value);
}
RequestOptions options = builder.build();
assertEquals(parameters, options.getParameters());

try {
options.getParameters().put(randomAsciiAlphanumOfLengthBetween(5, 10), randomAsciiAlphanumOfLength(3));
fail("expected failure");
} catch (UnsupportedOperationException e) {
assertNull(e.getMessage());
}
}

public void testSetHttpAsyncResponseConsumerFactory() {
try {
RequestOptions.DEFAULT.toBuilder().setHttpAsyncResponseConsumerFactory(null);
Expand Down Expand Up @@ -145,6 +181,13 @@ static RequestOptions.Builder randomBuilder() {
}
}

if (randomBoolean()) {
int queryParamCount = between(1, 5);
for (int i = 0; i < queryParamCount; i++) {
builder.addParameter(randomAsciiAlphanumOfLength(3), randomAsciiAlphanumOfLength(3));
}
}

if (randomBoolean()) {
builder.setHttpAsyncResponseConsumerFactory(new HeapBufferedResponseConsumerFactory(1));
}
Expand Down
Loading