Skip to content

Commit

Permalink
13776: allow adding query parameters to RequestOptions
Browse files Browse the repository at this point in the history
Signed-off-by: Oliver Lockwood <[email protected]>
  • Loading branch information
oliverlockwood committed May 22, 2024
1 parent faa592a commit 9384163
Show file tree
Hide file tree
Showing 4 changed files with 85 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,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))
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 final class RequestOptions {
*/
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> 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;
Expand All @@ -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);
}

/**
Expand All @@ -84,6 +90,13 @@ public List<Header> getHeaders() {
return headers;
}

/**
* Query parameters to attach to the request.
*/
public Map<String, String> getQueryParams() {
return queryParams;
}

/**
* The {@link HttpAsyncResponseConsumerFactory} used to create one
* {@link HttpAsyncResponseConsumer} callback per retry. Controls how the
Expand Down Expand Up @@ -139,6 +152,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;
Expand All @@ -163,13 +182,14 @@ 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);
}

@Override
public int hashCode() {
return Objects.hash(headers, httpAsyncResponseConsumerFactory, warningsHandler);
return Objects.hash(headers, queryParams, httpAsyncResponseConsumerFactory, warningsHandler);
}

/**
Expand All @@ -179,17 +199,20 @@ public int hashCode() {
*/
public static class Builder {
private final List<Header> headers;
private final Map<String, String> queryParams;
private HttpAsyncResponseConsumerFactory httpAsyncResponseConsumerFactory;
private WarningsHandler warningsHandler;
private RequestConfig requestConfig;

private Builder(
List<Header> headers,
Map<String, String> 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;
Expand All @@ -216,6 +239,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 HttpAsyncResponseConsumer} callback per retry. Controls how the
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -810,6 +810,7 @@ private class InternalRequest {
InternalRequest(Request request) {
this.request = request;
Map<String, String> 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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,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;
Expand Down Expand Up @@ -89,6 +91,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<String, String> 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);
Expand Down Expand Up @@ -144,6 +181,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));
}
Expand Down

0 comments on commit 9384163

Please sign in to comment.