Skip to content

Commit

Permalink
LLREST: Introduce a strict mode (#33708)
Browse files Browse the repository at this point in the history
Introduces `RestClientBuilder#setStrictDeprecationMode` which defaults
to false but when set to true, causes a rest request to fail if a
deprecation warning header comes back in the response from Elasticsearch.
This should be valueable to Elasticsearch's tests, especially those of the
High Level REST Client where they will help catch divergence between the
client and the server.
  • Loading branch information
lipsill authored and nik9000 committed Sep 28, 2018
1 parent d6d965a commit c8ad76b
Show file tree
Hide file tree
Showing 7 changed files with 72 additions and 8 deletions.
44 changes: 44 additions & 0 deletions client/rest/src/main/java/org/elasticsearch/client/Response.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,11 @@
import org.apache.http.RequestLine;
import org.apache.http.StatusLine;

import java.util.ArrayList;
import java.util.List;
import java.util.Objects;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

/**
* Holds an elasticsearch response. It wraps the {@link HttpResponse} returned and associates it with
Expand Down Expand Up @@ -96,6 +100,46 @@ public HttpEntity getEntity() {
return response.getEntity();
}

private static final Pattern WARNING_HEADER_PATTERN = Pattern.compile(
"299 " + // warn code
"Elasticsearch-\\d+\\.\\d+\\.\\d+(?:-(?:alpha|beta|rc)\\d+)?(?:-SNAPSHOT)?-(?:[a-f0-9]{7}|Unknown) " + // warn agent
"\"((?:\t| |!|[\\x23-\\x5B]|[\\x5D-\\x7E]|[\\x80-\\xFF]|\\\\|\\\\\")*)\" " + // quoted warning value, captured
// quoted RFC 1123 date format
"\"" + // opening quote
"(?:Mon|Tue|Wed|Thu|Fri|Sat|Sun), " + // weekday
"\\d{2} " + // 2-digit day
"(?:Jan|Feb|Mar|Apr|May|Jun|Jul|Aug|Sep|Oct|Nov|Dec) " + // month
"\\d{4} " + // 4-digit year
"\\d{2}:\\d{2}:\\d{2} " + // (two-digit hour):(two-digit minute):(two-digit second)
"GMT" + // GMT
"\""); // closing quote

/**
* Returns a list of all warning headers returned in the response.
*/
public List<String> getWarnings() {
List<String> warnings = new ArrayList<>();
for (Header header : response.getHeaders("Warning")) {
String warning = header.getValue();
final Matcher matcher = WARNING_HEADER_PATTERN.matcher(warning);
if (matcher.matches()) {
warnings.add(matcher.group(1));
continue;
}
warnings.add(warning);
}
return warnings;
}

/**
* Returns true if there is at least one warning header returned in the
* response.
*/
public boolean hasWarnings() {
Header[] warnings = response.getHeaders("Warning");
return warnings != null && warnings.length > 0;
}

HttpResponse getHttpResponse() {
return response;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@ private static String buildMessage(Response response) throws IOException {
response.getStatusLine().toString()
);

if (response.hasWarnings()) {
message += "\n" + "Warnings: " + response.getWarnings();
}

HttpEntity entity = response.getEntity();
if (entity != null) {
if (entity.isRepeatable() == false) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,15 +110,17 @@ public class RestClient implements Closeable {
private final FailureListener failureListener;
private final NodeSelector nodeSelector;
private volatile NodeTuple<List<Node>> nodeTuple;
private final boolean strictDeprecationMode;

RestClient(CloseableHttpAsyncClient client, long maxRetryTimeoutMillis, Header[] defaultHeaders,
List<Node> nodes, String pathPrefix, FailureListener failureListener, NodeSelector nodeSelector) {
RestClient(CloseableHttpAsyncClient client, long maxRetryTimeoutMillis, Header[] defaultHeaders, List<Node> nodes, String pathPrefix,
FailureListener failureListener, NodeSelector nodeSelector, boolean strictDeprecationMode) {
this.client = client;
this.maxRetryTimeoutMillis = maxRetryTimeoutMillis;
this.defaultHeaders = Collections.unmodifiableList(Arrays.asList(defaultHeaders));
this.failureListener = failureListener;
this.pathPrefix = pathPrefix;
this.nodeSelector = nodeSelector;
this.strictDeprecationMode = strictDeprecationMode;
setNodes(nodes);
}

Expand Down Expand Up @@ -535,7 +537,11 @@ public void completed(HttpResponse httpResponse) {
Response response = new Response(request.getRequestLine(), node.getHost(), httpResponse);
if (isSuccessfulResponse(statusCode) || ignoreErrorCodes.contains(response.getStatusLine().getStatusCode())) {
onResponse(node);
listener.onSuccess(response);
if (strictDeprecationMode && response.hasWarnings()) {
listener.onDefinitiveFailure(new ResponseException(response));
} else {
listener.onSuccess(response);
}
} else {
ResponseException responseException = new ResponseException(response);
if (isRetryStatus(statusCode)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ public final class RestClientBuilder {
private RequestConfigCallback requestConfigCallback;
private String pathPrefix;
private NodeSelector nodeSelector = NodeSelector.ANY;
private boolean strictDeprecationMode = false;

/**
* Creates a new builder instance and sets the hosts that the client will send requests to.
Expand Down Expand Up @@ -185,6 +186,15 @@ public RestClientBuilder setNodeSelector(NodeSelector nodeSelector) {
return this;
}

/**
* Whether the REST client should return any response containing at least
* one warning header as a failure.
*/
public RestClientBuilder setStrictDeprecationMode(boolean strictDeprecationMode) {
this.strictDeprecationMode = strictDeprecationMode;
return this;
}

/**
* Creates a new {@link RestClient} based on the provided configuration.
*/
Expand All @@ -199,7 +209,7 @@ public CloseableHttpAsyncClient run() {
}
});
RestClient restClient = new RestClient(httpClient, maxRetryTimeout, defaultHeaders, nodes,
pathPrefix, failureListener, nodeSelector);
pathPrefix, failureListener, nodeSelector, strictDeprecationMode);
httpClient.start();
return restClient;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ public void run() {
}
nodes = Collections.unmodifiableList(nodes);
failureListener = new HostsTrackingFailureListener();
return new RestClient(httpClient, 10000, new Header[0], nodes, null, failureListener, nodeSelector);
return new RestClient(httpClient, 10000, new Header[0], nodes, null, failureListener, nodeSelector, false);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ public void run() {
node = new Node(new HttpHost("localhost", 9200));
failureListener = new HostsTrackingFailureListener();
restClient = new RestClient(httpClient, 10000, defaultHeaders,
singletonList(node), null, failureListener, NodeSelector.ANY);
singletonList(node), null, failureListener, NodeSelector.ANY, false);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public class RestClientTests extends RestClientTestCase {
public void testCloseIsIdempotent() throws IOException {
List<Node> nodes = singletonList(new Node(new HttpHost("localhost", 9200)));
CloseableHttpAsyncClient closeableHttpAsyncClient = mock(CloseableHttpAsyncClient.class);
RestClient restClient = new RestClient(closeableHttpAsyncClient, 1_000, new Header[0], nodes, null, null, null);
RestClient restClient = new RestClient(closeableHttpAsyncClient, 1_000, new Header[0], nodes, null, null, null, false);
restClient.close();
verify(closeableHttpAsyncClient, times(1)).close();
restClient.close();
Expand Down Expand Up @@ -500,7 +500,7 @@ private static String assertSelectAllRejected( NodeTuple<List<Node>> nodeTuple,
private static RestClient createRestClient() {
List<Node> nodes = Collections.singletonList(new Node(new HttpHost("localhost", 9200)));
return new RestClient(mock(CloseableHttpAsyncClient.class), randomLongBetween(1_000, 30_000),
new Header[] {}, nodes, null, null, null);
new Header[] {}, nodes, null, null, null, false);
}

public void testRoundRobin() throws IOException {
Expand Down

0 comments on commit c8ad76b

Please sign in to comment.