From b3218fef200cef5411101b2f0ce0785c87e69dcf 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 39bbf769713..ab61f01f661 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.HttpResponse; 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 class Response { 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 5e646d975c8..0957e25fb70 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 @@ public final class ResponseException extends 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 a7afbc8ffbd..d68e371f318 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); } @@ -296,7 +298,11 @@ public class RestClient implements Closeable { 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 dd3f5ad5a72..84cc3ee1667 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 final class RestClientBuilder { 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 final class RestClientBuilder { } }); 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 e1062076a0d..7dd1c4d842b 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 class RestClientMultipleHostsTests extends RestClientTestCase { } 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 0c589e6a40c..3aa10762676 100644 --- a/client/rest/src/test/java/org/elasticsearch/client/RestClientSingleHostTests.java +++ b/client/rest/src/test/java/org/elasticsearch/client/RestClientSingleHostTests.java @@ -148,7 +148,7 @@ public class RestClientSingleHostTests extends RestClientTestCase { 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 4a037b18404..69cdfeae85d 100644 --- a/client/rest/src/test/java/org/elasticsearch/client/RestClientTests.java +++ b/client/rest/src/test/java/org/elasticsearch/client/RestClientTests.java @@ -57,7 +57,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(); @@ -345,7 +345,7 @@ public class RestClientTests extends RestClientTestCase { 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 {