From c8ad76b34e1a1edcb27c5917a05913919ada4204 Mon Sep 17 00:00:00 2001 From: lipsill <39668292+lipsill@users.noreply.github.com> Date: Fri, 28 Sep 2018 15:25:19 +0200 Subject: [PATCH] LLREST: Introduce a strict mode (#33708) 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. --- .../org/elasticsearch/client/Response.java | 44 +++++++++++++++++++ .../client/ResponseException.java | 4 ++ .../org/elasticsearch/client/RestClient.java | 12 +++-- .../client/RestClientBuilder.java | 12 ++++- .../client/RestClientMultipleHostsTests.java | 2 +- .../client/RestClientSingleHostTests.java | 2 +- .../elasticsearch/client/RestClientTests.java | 4 +- 7 files changed, 72 insertions(+), 8 deletions(-) diff --git a/client/rest/src/main/java/org/elasticsearch/client/Response.java b/client/rest/src/main/java/org/elasticsearch/client/Response.java index 39bbf769713b2..ab61f01f66159 100644 --- a/client/rest/src/main/java/org/elasticsearch/client/Response.java +++ b/client/rest/src/main/java/org/elasticsearch/client/Response.java @@ -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 @@ -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 getWarnings() { + List 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; } diff --git a/client/rest/src/main/java/org/elasticsearch/client/ResponseException.java b/client/rest/src/main/java/org/elasticsearch/client/ResponseException.java index 5e646d975c89c..0957e25fb7033 100644 --- a/client/rest/src/main/java/org/elasticsearch/client/ResponseException.java +++ b/client/rest/src/main/java/org/elasticsearch/client/ResponseException.java @@ -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) { diff --git a/client/rest/src/main/java/org/elasticsearch/client/RestClient.java b/client/rest/src/main/java/org/elasticsearch/client/RestClient.java index 934b952608674..2d7940b4a893c 100644 --- a/client/rest/src/main/java/org/elasticsearch/client/RestClient.java +++ b/client/rest/src/main/java/org/elasticsearch/client/RestClient.java @@ -110,15 +110,17 @@ public class RestClient implements Closeable { private final FailureListener failureListener; private final NodeSelector nodeSelector; private volatile NodeTuple> nodeTuple; + private final boolean strictDeprecationMode; - RestClient(CloseableHttpAsyncClient client, long maxRetryTimeoutMillis, Header[] defaultHeaders, - List nodes, String pathPrefix, FailureListener failureListener, NodeSelector nodeSelector) { + RestClient(CloseableHttpAsyncClient client, long maxRetryTimeoutMillis, Header[] defaultHeaders, List 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); } @@ -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)) { diff --git a/client/rest/src/main/java/org/elasticsearch/client/RestClientBuilder.java b/client/rest/src/main/java/org/elasticsearch/client/RestClientBuilder.java index dd3f5ad5a7274..84cc3ee1667b1 100644 --- a/client/rest/src/main/java/org/elasticsearch/client/RestClientBuilder.java +++ b/client/rest/src/main/java/org/elasticsearch/client/RestClientBuilder.java @@ -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. @@ -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. */ @@ -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; } diff --git a/client/rest/src/test/java/org/elasticsearch/client/RestClientMultipleHostsTests.java b/client/rest/src/test/java/org/elasticsearch/client/RestClientMultipleHostsTests.java index e1062076a0dbf..7dd1c4d842bff 100644 --- a/client/rest/src/test/java/org/elasticsearch/client/RestClientMultipleHostsTests.java +++ b/client/rest/src/test/java/org/elasticsearch/client/RestClientMultipleHostsTests.java @@ -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); } /** diff --git a/client/rest/src/test/java/org/elasticsearch/client/RestClientSingleHostTests.java b/client/rest/src/test/java/org/elasticsearch/client/RestClientSingleHostTests.java index cb326f4a24c8d..48b9af9548d9c 100644 --- a/client/rest/src/test/java/org/elasticsearch/client/RestClientSingleHostTests.java +++ b/client/rest/src/test/java/org/elasticsearch/client/RestClientSingleHostTests.java @@ -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); } /** diff --git a/client/rest/src/test/java/org/elasticsearch/client/RestClientTests.java b/client/rest/src/test/java/org/elasticsearch/client/RestClientTests.java index ef94b70542f6c..10e58c50b9310 100644 --- a/client/rest/src/test/java/org/elasticsearch/client/RestClientTests.java +++ b/client/rest/src/test/java/org/elasticsearch/client/RestClientTests.java @@ -58,7 +58,7 @@ public class RestClientTests extends RestClientTestCase { public void testCloseIsIdempotent() throws IOException { List 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(); @@ -500,7 +500,7 @@ private static String assertSelectAllRejected( NodeTuple> nodeTuple, private static RestClient createRestClient() { List 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 {