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 89c3309dbbdd..ce33224b7ecd 100644 --- a/client/rest/src/main/java/org/elasticsearch/client/RestClient.java +++ b/client/rest/src/main/java/org/elasticsearch/client/RestClient.java @@ -49,6 +49,7 @@ import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.Comparator; +import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; import java.util.List; @@ -282,15 +283,44 @@ public class RestClient implements Closeable { public void performRequestAsync(String method, String endpoint, Map params, HttpEntity entity, HttpAsyncResponseConsumerFactory httpAsyncResponseConsumerFactory, ResponseListener responseListener, Header... headers) { - URI uri = buildUri(pathPrefix, endpoint, params); + Objects.requireNonNull(params, "params must not be null"); + Map requestParams = new HashMap<>(params); + //ignore is a special parameter supported by the clients, shouldn't be sent to es + String ignoreString = requestParams.remove("ignore"); + Set ignoreErrorCodes; + if (ignoreString == null) { + if (HttpHead.METHOD_NAME.equals(method)) { + //404 never causes error if returned for a HEAD request + ignoreErrorCodes = Collections.singleton(404); + } else { + ignoreErrorCodes = Collections.emptySet(); + } + } else { + String[] ignoresArray = ignoreString.split(","); + ignoreErrorCodes = new HashSet<>(); + if (HttpHead.METHOD_NAME.equals(method)) { + //404 never causes error if returned for a HEAD request + ignoreErrorCodes.add(404); + } + for (String ignoreCode : ignoresArray) { + try { + ignoreErrorCodes.add(Integer.valueOf(ignoreCode)); + } catch (NumberFormatException e) { + throw new IllegalArgumentException("ignore value should be a number, found [" + ignoreString + "] instead", e); + } + } + } + URI uri = buildUri(pathPrefix, endpoint, requestParams); HttpRequestBase request = createHttpRequest(method, uri, entity); setHeaders(request, headers); FailureTrackingResponseListener failureTrackingResponseListener = new FailureTrackingResponseListener(responseListener); long startTime = System.nanoTime(); - performRequestAsync(startTime, nextHost().iterator(), request, httpAsyncResponseConsumerFactory, failureTrackingResponseListener); + performRequestAsync(startTime, nextHost().iterator(), request, ignoreErrorCodes, httpAsyncResponseConsumerFactory, + failureTrackingResponseListener); } private void performRequestAsync(final long startTime, final Iterator hosts, final HttpRequestBase request, + final Set ignoreErrorCodes, final HttpAsyncResponseConsumerFactory httpAsyncResponseConsumerFactory, final FailureTrackingResponseListener listener) { final HttpHost host = hosts.next(); @@ -304,7 +334,7 @@ public class RestClient implements Closeable { RequestLogger.logResponse(logger, request, host, httpResponse); int statusCode = httpResponse.getStatusLine().getStatusCode(); Response response = new Response(request.getRequestLine(), host, httpResponse); - if (isSuccessfulResponse(request.getMethod(), statusCode)) { + if (isSuccessfulResponse(statusCode) || ignoreErrorCodes.contains(response.getStatusLine().getStatusCode())) { onResponse(host); listener.onSuccess(response); } else { @@ -312,7 +342,7 @@ public class RestClient implements Closeable { if (isRetryStatus(statusCode)) { //mark host dead and retry against next one onFailure(host); - retryIfPossible(responseException, hosts, request); + retryIfPossible(responseException); } else { //mark host alive and don't retry, as the error should be a request problem onResponse(host); @@ -329,13 +359,13 @@ public class RestClient implements Closeable { try { RequestLogger.logFailedRequest(logger, request, host, failure); onFailure(host); - retryIfPossible(failure, hosts, request); + retryIfPossible(failure); } catch(Exception e) { listener.onDefinitiveFailure(e); } } - private void retryIfPossible(Exception exception, Iterator hosts, HttpRequestBase request) { + private void retryIfPossible(Exception exception) { if (hosts.hasNext()) { //in case we are retrying, check whether maxRetryTimeout has been reached long timeElapsedMillis = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - startTime); @@ -347,7 +377,7 @@ public class RestClient implements Closeable { } else { listener.trackFailure(exception); request.reset(); - performRequestAsync(startTime, hosts, request, httpAsyncResponseConsumerFactory, listener); + performRequestAsync(startTime, hosts, request, ignoreErrorCodes, httpAsyncResponseConsumerFactory, listener); } } else { listener.onDefinitiveFailure(exception); @@ -452,8 +482,8 @@ public class RestClient implements Closeable { client.close(); } - private static boolean isSuccessfulResponse(String method, int statusCode) { - return statusCode < 300 || (HttpHead.METHOD_NAME.equals(method) && statusCode == 404); + private static boolean isSuccessfulResponse(int statusCode) { + return statusCode < 300; } private static boolean isRetryStatus(int statusCode) { @@ -510,7 +540,6 @@ public class RestClient implements Closeable { } private static URI buildUri(String pathPrefix, String path, Map params) { - Objects.requireNonNull(params, "params must not be null"); Objects.requireNonNull(path, "path must not be null"); try { String fullPath; 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 865f9b1817af..2489eb717ffa 100644 --- a/client/rest/src/test/java/org/elasticsearch/client/RestClientSingleHostTests.java +++ b/client/rest/src/test/java/org/elasticsearch/client/RestClientSingleHostTests.java @@ -216,23 +216,45 @@ public class RestClientSingleHostTests extends RestClientTestCase { */ public void testErrorStatusCodes() throws IOException { for (String method : getHttpMethods()) { + Set expectedIgnores = new HashSet<>(); + String ignoreParam = ""; + if (HttpHead.METHOD_NAME.equals(method)) { + expectedIgnores.add(404); + } + if (randomBoolean()) { + int numIgnores = randomIntBetween(1, 3); + for (int i = 0; i < numIgnores; i++) { + Integer code = randomFrom(getAllErrorStatusCodes()); + expectedIgnores.add(code); + ignoreParam += code; + if (i < numIgnores - 1) { + ignoreParam += ","; + } + } + } //error status codes should cause an exception to be thrown for (int errorStatusCode : getAllErrorStatusCodes()) { try { - Response response = performRequest(method, "/" + errorStatusCode); - if (method.equals("HEAD") && errorStatusCode == 404) { - //no exception gets thrown although we got a 404 - assertThat(response.getStatusLine().getStatusCode(), equalTo(errorStatusCode)); + Map params; + if (ignoreParam.isEmpty()) { + params = Collections.emptyMap(); + } else { + params = Collections.singletonMap("ignore", ignoreParam); + } + Response response = performRequest(method, "/" + errorStatusCode, params); + if (expectedIgnores.contains(errorStatusCode)) { + //no exception gets thrown although we got an error status code, as it was configured to be ignored + assertEquals(errorStatusCode, response.getStatusLine().getStatusCode()); } else { fail("request should have failed"); } } catch(ResponseException e) { - if (method.equals("HEAD") && errorStatusCode == 404) { + if (expectedIgnores.contains(errorStatusCode)) { throw e; } - assertThat(e.getResponse().getStatusLine().getStatusCode(), equalTo(errorStatusCode)); + assertEquals(errorStatusCode, e.getResponse().getStatusLine().getStatusCode()); } - if (errorStatusCode <= 500) { + if (errorStatusCode <= 500 || expectedIgnores.contains(errorStatusCode)) { failureListener.assertNotCalled(); } else { failureListener.assertCalled(httpHost); @@ -351,11 +373,10 @@ public class RestClientSingleHostTests extends RestClientTestCase { private HttpUriRequest performRandomRequest(String method) throws Exception { String uriAsString = "/" + randomStatusCode(getRandom()); URIBuilder uriBuilder = new URIBuilder(uriAsString); - Map params = Collections.emptyMap(); + final Map params = new HashMap<>(); boolean hasParams = randomBoolean(); if (hasParams) { int numParams = randomIntBetween(1, 3); - params = new HashMap<>(numParams); for (int i = 0; i < numParams; i++) { String paramKey = "param-" + i; String paramValue = randomAsciiOfLengthBetween(3, 10); @@ -363,6 +384,14 @@ public class RestClientSingleHostTests extends RestClientTestCase { uriBuilder.addParameter(paramKey, paramValue); } } + if (randomBoolean()) { + //randomly add some ignore parameter, which doesn't get sent as part of the request + String ignore = Integer.toString(randomFrom(RestClientTestUtil.getAllErrorStatusCodes())); + if (randomBoolean()) { + ignore += "," + Integer.toString(randomFrom(RestClientTestUtil.getAllErrorStatusCodes())); + } + params.put("ignore", ignore); + } URI uri = uriBuilder.build(); HttpUriRequest request; @@ -433,16 +462,25 @@ public class RestClientSingleHostTests extends RestClientTestCase { } private Response performRequest(String method, String endpoint, Header... headers) throws IOException { - switch(randomIntBetween(0, 2)) { + return performRequest(method, endpoint, Collections.emptyMap(), headers); + } + + private Response performRequest(String method, String endpoint, Map params, Header... headers) throws IOException { + int methodSelector; + if (params.isEmpty()) { + methodSelector = randomIntBetween(0, 2); + } else { + methodSelector = randomIntBetween(1, 2); + } + switch(methodSelector) { case 0: return restClient.performRequest(method, endpoint, headers); case 1: - return restClient.performRequest(method, endpoint, Collections.emptyMap(), headers); + return restClient.performRequest(method, endpoint, params, headers); case 2: - return restClient.performRequest(method, endpoint, Collections.emptyMap(), (HttpEntity)null, headers); + return restClient.performRequest(method, endpoint, params, (HttpEntity)null, headers); default: throw new UnsupportedOperationException(); } } - } diff --git a/client/test/src/main/java/org/elasticsearch/client/RestClientTestUtil.java b/client/test/src/main/java/org/elasticsearch/client/RestClientTestUtil.java index dbf85578b199..a0a6641abbc5 100644 --- a/client/test/src/main/java/org/elasticsearch/client/RestClientTestUtil.java +++ b/client/test/src/main/java/org/elasticsearch/client/RestClientTestUtil.java @@ -59,7 +59,7 @@ final class RestClientTestUtil { } static int randomStatusCode(Random random) { - return RandomPicks.randomFrom(random, ALL_ERROR_STATUS_CODES); + return RandomPicks.randomFrom(random, ALL_STATUS_CODES); } static int randomOkStatusCode(Random random) { diff --git a/docs/java-rest/usage.asciidoc b/docs/java-rest/usage.asciidoc index b3097ef9d0bd..69eb16280ed0 100644 --- a/docs/java-rest/usage.asciidoc +++ b/docs/java-rest/usage.asciidoc @@ -206,7 +206,14 @@ access to the returned response. NOTE: A `ResponseException` is **not** thrown for `HEAD` requests that return a `404` status code because it is an expected `HEAD` response that simply denotes that the resource is not found. All other HTTP methods (e.g., `GET`) -throw a `ResponseException` for `404` responses. +throw a `ResponseException` for `404` responses unless the `ignore` parameter +contains `404`. `ignore` is a special client parameter that doesn't get sent +to Elasticsearch and contains a comma separated list of error status codes. +It allows to control whether some error status code should be treated as an +expected response rather than as an exception. This is useful for instance +with the get api as it can return `404` when the document is missing, in which +case the response body will not contain an error but rather the usual get api +response, just without the document as it was not found. === Example requests diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestClient.java b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestClient.java index 3b719b6d04f1..748a08384ca9 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestClient.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestClient.java @@ -40,8 +40,6 @@ import org.elasticsearch.test.rest.yaml.restspec.ClientYamlSuiteRestSpec; import java.io.IOException; import java.net.URI; import java.net.URISyntaxException; -import java.util.ArrayList; -import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -59,7 +57,7 @@ public class ClientYamlTestClient { * Query params that don't need to be declared in the spec, they are supported by default. */ private static final Set ALWAYS_ACCEPTED_QUERY_STRING_PARAMS = Sets.newHashSet( - "error_trace", "filter_path", "human", "pretty", "source"); + "ignore", "error_trace", "human", "filter_path", "pretty", "source"); private final ClientYamlSuiteRestSpec restSpec; private final RestClient restClient; @@ -101,31 +99,12 @@ public class ClientYamlTestClient { } } - List ignores = new ArrayList<>(); - Map requestParams; - if (params == null) { - requestParams = Collections.emptyMap(); - } else { - requestParams = new HashMap<>(params); - if (params.isEmpty() == false) { - //ignore is a special parameter supported by the clients, shouldn't be sent to es - String ignoreString = requestParams.remove("ignore"); - if (ignoreString != null) { - try { - ignores.add(Integer.valueOf(ignoreString)); - } catch (NumberFormatException e) { - throw new IllegalArgumentException("ignore value should be a number, found [" + ignoreString + "] instead"); - } - } - } - } - ClientYamlSuiteRestApi restApi = restApi(apiName); //divide params between ones that go within query string and ones that go within path Map pathParts = new HashMap<>(); Map queryStringParams = new HashMap<>(); - for (Map.Entry entry : requestParams.entrySet()) { + for (Map.Entry entry : params.entrySet()) { if (restApi.getPathParts().contains(entry.getKey())) { pathParts.put(entry.getKey(), entry.getValue()); } else { @@ -197,9 +176,6 @@ public class ClientYamlTestClient { Response response = restClient.performRequest(requestMethod, requestPath, queryStringParams, requestBody, requestHeaders); return new ClientYamlTestResponse(response); } catch(ResponseException e) { - if (ignores.contains(e.getResponse().getStatusLine().getStatusCode())) { - return new ClientYamlTestResponse(e.getResponse()); - } throw new ClientYamlTestResponseException(e); } }