Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -36,19 +36,27 @@
* manipulated without changing Elasticsearch's behavior.
*/
public final class RequestOptions {
/**
* Default request options.
*/
public static final RequestOptions DEFAULT = new Builder(
Collections.<Header>emptyList(), HeapBufferedResponseConsumerFactory.DEFAULT).build();
Collections.<Header>emptyList(), HeapBufferedResponseConsumerFactory.DEFAULT, null).build();

private final List<Header> headers;
private final HttpAsyncResponseConsumerFactory httpAsyncResponseConsumerFactory;
private final Boolean overrideStrictDeprecationMode;

private RequestOptions(Builder builder) {
this.headers = Collections.unmodifiableList(new ArrayList<>(builder.headers));
this.httpAsyncResponseConsumerFactory = builder.httpAsyncResponseConsumerFactory;
this.overrideStrictDeprecationMode = builder.overrideStrictDeprecationMode;
}

/**
* Create a builder that contains these options but can be modified.
*/
public Builder toBuilder() {
return new Builder(headers, httpAsyncResponseConsumerFactory);
return new Builder(headers, httpAsyncResponseConsumerFactory, overrideStrictDeprecationMode);
}

/**
Expand All @@ -68,6 +76,15 @@ public HttpAsyncResponseConsumerFactory getHttpAsyncResponseConsumerFactory() {
return httpAsyncResponseConsumerFactory;
}

/**
* Override for the client's default for
* {@link RestClientBuilder#setStrictDeprecationMode strict deprecation}
* . Null means accept the client's default.
*/
public Boolean getOverrideStrictDeprecationMode() {
return overrideStrictDeprecationMode;
}

@Override
public String toString() {
StringBuilder b = new StringBuilder();
Expand Down Expand Up @@ -106,13 +123,21 @@ public int hashCode() {
return Objects.hash(headers, httpAsyncResponseConsumerFactory);
}

/**
* Builds {@link RequestOptions}. Get one by calling
* {@link RequestOptions#toBuilder} on {@link RequestOptions#DEFAULT} or
* any other {@linkplain RequestOptions}.
*/
public static class Builder {
private final List<Header> headers;
private HttpAsyncResponseConsumerFactory httpAsyncResponseConsumerFactory;
private Boolean overrideStrictDeprecationMode;

private Builder(List<Header> headers, HttpAsyncResponseConsumerFactory httpAsyncResponseConsumerFactory) {
private Builder(List<Header> headers, HttpAsyncResponseConsumerFactory httpAsyncResponseConsumerFactory,
Boolean overrideStrictDeprecationMode) {
this.headers = new ArrayList<>(headers);
this.httpAsyncResponseConsumerFactory = httpAsyncResponseConsumerFactory;
this.overrideStrictDeprecationMode = overrideStrictDeprecationMode;
}

/**
Expand Down Expand Up @@ -141,6 +166,15 @@ public void setHttpAsyncResponseConsumerFactory(HttpAsyncResponseConsumerFactory
this.httpAsyncResponseConsumerFactory =
Objects.requireNonNull(httpAsyncResponseConsumerFactory, "httpAsyncResponseConsumerFactory cannot be null");
}

/**
* Override the client's default for
* {@link RestClientBuilder#setStrictDeprecationMode strict deprecation}
* . Null means accept the client's default
*/
public void setOverrideStrictDeprecationMode(Boolean overrideStrictDeprecationMode) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor comment... I think this name is a bit confusing. My first instinct was to set this true if I wanted to override the client's default strict mode (because I want it to be not-strict). But that's not quite how it works, since this A) overrides and B) sets to the boolean you pass.

Maybe just setStrictDeprecationMode(Boolean) is fine, and it's understood/documented that if you set something it overrides the default which is strict?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is the name I started with but I second guessed myself. I'll switch it back.

this.overrideStrictDeprecationMode = overrideStrictDeprecationMode;
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -274,12 +274,15 @@ void performRequestAsyncNoCatch(Request request, ResponseListener listener) thro
setHeaders(httpRequest, request.getOptions().getHeaders());
FailureTrackingResponseListener failureTrackingResponseListener = new FailureTrackingResponseListener(listener);
long startTime = System.nanoTime();
performRequestAsync(startTime, nextNode(), httpRequest, ignoreErrorCodes,
boolean thisRequestStrictDeprecationMode = request.getOptions().getOverrideStrictDeprecationMode() == null ?
strictDeprecationMode : request.getOptions().getOverrideStrictDeprecationMode();
performRequestAsync(startTime, nextNode(), httpRequest, ignoreErrorCodes, thisRequestStrictDeprecationMode,
request.getOptions().getHttpAsyncResponseConsumerFactory(), failureTrackingResponseListener);
}

private void performRequestAsync(final long startTime, final NodeTuple<Iterator<Node>> nodeTuple, final HttpRequestBase request,
final Set<Integer> ignoreErrorCodes,
final boolean thisRequestStrictDeprecationMode,
final HttpAsyncResponseConsumerFactory httpAsyncResponseConsumerFactory,
final FailureTrackingResponseListener listener) {
final Node node = nodeTuple.nodes.next();
Expand All @@ -298,7 +301,7 @@ public void completed(HttpResponse httpResponse) {
Response response = new Response(request.getRequestLine(), node.getHost(), httpResponse);
if (isSuccessfulResponse(statusCode) || ignoreErrorCodes.contains(response.getStatusLine().getStatusCode())) {
onResponse(node);
if (strictDeprecationMode && response.hasWarnings()) {
if (thisRequestStrictDeprecationMode && response.hasWarnings()) {
listener.onDefinitiveFailure(new ResponseException(response));
} else {
listener.onSuccess(response);
Expand Down Expand Up @@ -343,7 +346,8 @@ private void retryIfPossible(Exception exception) {
} else {
listener.trackFailure(exception);
request.reset();
performRequestAsync(startTime, nodeTuple, request, ignoreErrorCodes, httpAsyncResponseConsumerFactory, listener);
performRequestAsync(startTime, nodeTuple, request, ignoreErrorCodes,
thisRequestStrictDeprecationMode, httpAsyncResponseConsumerFactory, listener);
}
} else {
listener.onDefinitiveFailure(exception);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -357,10 +357,17 @@ private void assertDeprecationWarnings(List<String> warningHeaderTexts, List<Str
for (String warningHeaderText : warningHeaderTexts) {
options.addHeader("Warning", warningHeaderText);
}
boolean thisRequestStrictDeprecationMode;
if (randomBoolean()) {
thisRequestStrictDeprecationMode = randomBoolean();
options.setOverrideStrictDeprecationMode(thisRequestStrictDeprecationMode);
} else {
thisRequestStrictDeprecationMode = strictDeprecationMode;
}
request.setOptions(options);

Response response;
if (strictDeprecationMode) {
if (thisRequestStrictDeprecationMode) {
try {
restClient.performRequest(request);
fail("expected ResponseException because strict deprecation mode is enabled");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import org.elasticsearch.Version;
import org.elasticsearch.action.admin.cluster.node.tasks.list.ListTasksAction;
import org.elasticsearch.client.Request;
import org.elasticsearch.client.RequestOptions;
import org.elasticsearch.client.Response;
import org.elasticsearch.client.ResponseException;
import org.elasticsearch.client.RestClient;
Expand Down Expand Up @@ -618,7 +619,11 @@ protected RestClient buildClient(Settings settings, HttpHost[] hosts) throws IOE
/**
* Whether the used REST client should return any response containing at
* least one warning header as a failure.
* @deprecated always run in strict mode and use
* {@link RequestOptions.Builder#setOverrideStrictDeprecationMode} to
* enable perissive mode for individual requests
*/
@Deprecated
protected boolean getStrictDeprecationMode() {
return true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,8 @@ protected static void setOptions(Request request, Map<String, String> headers) {
logger.debug("Adding header {} with value {}", header.getKey(), header.getValue());
options.addHeader(header.getKey(), header.getValue());
}
// We check the warnings ourselves
options.setOverrideStrictDeprecationMode(false);
request.setOptions(options);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -432,9 +432,4 @@ protected final RestClientBuilder getClientBuilderWithSniffedHosts() throws IOEx
configureClient(builder, restClientSettings());
return builder;
}

@Override
protected boolean getStrictDeprecationMode() {
return false;
}
}