From fd297637a21790517a245321d8f6e55075f07cc6 Mon Sep 17 00:00:00 2001 From: javanna Date: Fri, 8 Jul 2016 15:35:14 +0200 Subject: [PATCH] Rest Client: add short performRequest method variants without params and/or body Users wanting to send a request by providing only its method and endpoint, effectively the only two required arguments, shouldn't need to pass in an empty map and a null entity for the body. While at it we can also add a variant to send requests by specifying only method, endpoint and params, but not body. Headers remain a vararg as last argument, so they can always optionally be provided. Closes #19312 --- .../org/elasticsearch/client/RestClient.java | 33 +++++++++++ .../client/RestClientMultipleHostsTests.java | 15 ++--- .../client/RestClientSingleHostTests.java | 55 +++++++++++++------ .../client/sniff/HostsSniffer.java | 2 +- 4 files changed, 79 insertions(+), 26 deletions(-) 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 a8f46ee6b0e..e3bb1b3c507 100644 --- a/client/rest/src/main/java/org/elasticsearch/client/RestClient.java +++ b/client/rest/src/main/java/org/elasticsearch/client/RestClient.java @@ -117,6 +117,39 @@ public final class RestClient implements Closeable { this.blacklist.clear(); } + /** + * Sends a request to the elasticsearch cluster that the current client points to. + * Shortcut to {@link #performRequest(String, String, Map, HttpEntity, Header...)} but without parameters and request body. + * + * @param method the http method + * @param endpoint the path of the request (without host and port) + * @param headers the optional request headers + * @return the response returned by elasticsearch + * @throws IOException in case of a problem or the connection was aborted + * @throws ClientProtocolException in case of an http protocol error + * @throws ResponseException in case elasticsearch responded with a status code that indicated an error + */ + public Response performRequest(String method, String endpoint, Header... headers) throws IOException { + return performRequest(method, endpoint, Collections.emptyMap(), null, headers); + } + + /** + * Sends a request to the elasticsearch cluster that the current client points to. + * Shortcut to {@link #performRequest(String, String, Map, HttpEntity, Header...)} but without request body. + * + * @param method the http method + * @param endpoint the path of the request (without host and port) + * @param params the query_string parameters + * @param headers the optional request headers + * @return the response returned by elasticsearch + * @throws IOException in case of a problem or the connection was aborted + * @throws ClientProtocolException in case of an http protocol error + * @throws ResponseException in case elasticsearch responded with a status code that indicated an error + */ + public Response performRequest(String method, String endpoint, Map params, Header... headers) throws IOException { + return performRequest(method, endpoint, params, null, headers); + } + /** * Sends a request to the elasticsearch cluster that the current client points to. * Selects a host out of the provided ones in a round-robin fashion. Failing hosts are marked dead and retried after a certain 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 9d082466123..b32f848581f 100644 --- a/client/rest/src/test/java/org/elasticsearch/client/RestClientMultipleHostsTests.java +++ b/client/rest/src/test/java/org/elasticsearch/client/RestClientMultipleHostsTests.java @@ -19,7 +19,6 @@ package org.elasticsearch.client; -import com.carrotsearch.randomizedtesting.RandomizedTest; import com.carrotsearch.randomizedtesting.generators.RandomInts; import org.apache.http.HttpHost; import org.apache.http.HttpRequest; @@ -103,7 +102,7 @@ public class RestClientMultipleHostsTests extends RestClientTestCase { for (int j = 0; j < httpHosts.length; j++) { int statusCode = randomOkStatusCode(getRandom()); try (Response response = restClient.performRequest(randomHttpMethod(getRandom()), "/" + statusCode, - Collections.emptyMap(), null)) { + Collections.emptyMap())) { assertThat(response.getStatusLine().getStatusCode(), equalTo(statusCode)); assertTrue("host not found: " + response.getHost(), hostsSet.remove(response.getHost())); } @@ -121,8 +120,7 @@ public class RestClientMultipleHostsTests extends RestClientTestCase { for (int j = 0; j < httpHosts.length; j++) { String method = randomHttpMethod(getRandom()); int statusCode = randomErrorNoRetryStatusCode(getRandom()); - try (Response response = restClient.performRequest(method, "/" + statusCode, - Collections.emptyMap(), null)) { + try (Response response = restClient.performRequest(method, "/" + statusCode, Collections.emptyMap())) { if (method.equals("HEAD") && statusCode == 404) { //no exception gets thrown although we got a 404 assertThat(response.getStatusLine().getStatusCode(), equalTo(404)); @@ -149,7 +147,7 @@ public class RestClientMultipleHostsTests extends RestClientTestCase { public void testRoundRobinRetryErrors() throws Exception { String retryEndpoint = randomErrorRetryEndpoint(); try { - restClient.performRequest(randomHttpMethod(getRandom()), retryEndpoint, Collections.emptyMap(), null); + restClient.performRequest(randomHttpMethod(getRandom()), retryEndpoint, Collections.emptyMap()); fail("request should have failed"); } catch(ResponseException e) { Set hostsSet = new HashSet<>(); @@ -199,7 +197,7 @@ public class RestClientMultipleHostsTests extends RestClientTestCase { for (int j = 0; j < httpHosts.length; j++) { retryEndpoint = randomErrorRetryEndpoint(); try { - restClient.performRequest(randomHttpMethod(getRandom()), retryEndpoint, Collections.emptyMap(), null); + restClient.performRequest(randomHttpMethod(getRandom()), retryEndpoint, Collections.emptyMap()); fail("request should have failed"); } catch(ResponseException e) { Response response = e.getResponse(); @@ -226,7 +224,7 @@ public class RestClientMultipleHostsTests extends RestClientTestCase { int statusCode = randomErrorNoRetryStatusCode(getRandom()); Response response; try (Response esResponse = restClient.performRequest(randomHttpMethod(getRandom()), "/" + statusCode, - Collections.emptyMap(), null)) { + Collections.emptyMap())) { response = esResponse; } catch(ResponseException e) { @@ -245,8 +243,7 @@ public class RestClientMultipleHostsTests extends RestClientTestCase { for (int y = 0; y < i + 1; y++) { retryEndpoint = randomErrorRetryEndpoint(); try { - restClient.performRequest(randomHttpMethod(getRandom()), retryEndpoint, - Collections.emptyMap(), null); + restClient.performRequest(randomHttpMethod(getRandom()), retryEndpoint, Collections.emptyMap()); fail("request should have failed"); } catch(ResponseException e) { Response response = e.getResponse(); 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 59e88b0ed63..b250614b91b 100644 --- a/client/rest/src/test/java/org/elasticsearch/client/RestClientSingleHostTests.java +++ b/client/rest/src/test/java/org/elasticsearch/client/RestClientSingleHostTests.java @@ -19,7 +19,6 @@ package org.elasticsearch.client; -import com.carrotsearch.randomizedtesting.RandomizedTest; import com.carrotsearch.randomizedtesting.generators.RandomInts; import com.carrotsearch.randomizedtesting.generators.RandomStrings; import org.apache.http.Header; @@ -156,7 +155,7 @@ public class RestClientSingleHostTests extends RestClientTestCase { } } - public void testSetNodes() throws IOException { + public void testSetHosts() throws IOException { try { restClient.setHosts((HttpHost[]) null); fail("setHosts should have failed"); @@ -189,8 +188,7 @@ public class RestClientSingleHostTests extends RestClientTestCase { public void testOkStatusCodes() throws Exception { for (String method : getHttpMethods()) { for (int okStatusCode : getOkStatusCodes()) { - Response response = restClient.performRequest(method, "/" + okStatusCode, - Collections.emptyMap(), null); + Response response = performRequest(method, "/" + okStatusCode); assertThat(response.getStatusLine().getStatusCode(), equalTo(okStatusCode)); } } @@ -204,8 +202,7 @@ public class RestClientSingleHostTests extends RestClientTestCase { for (String method : getHttpMethods()) { //error status codes should cause an exception to be thrown for (int errorStatusCode : getAllErrorStatusCodes()) { - try (Response response = restClient.performRequest(method, "/" + errorStatusCode, - Collections.emptyMap(), null)) { + 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)); @@ -231,14 +228,14 @@ public class RestClientSingleHostTests extends RestClientTestCase { for (String method : getHttpMethods()) { //IOExceptions should be let bubble up try { - restClient.performRequest(method, "/coe", Collections.emptyMap(), null); + performRequest(method, "/coe"); fail("request should have failed"); } catch(IOException e) { assertThat(e, instanceOf(ConnectTimeoutException.class)); } failureListener.assertCalled(httpHost); try { - restClient.performRequest(method, "/soe", Collections.emptyMap(), null); + performRequest(method, "/soe"); fail("request should have failed"); } catch(IOException e) { assertThat(e, instanceOf(SocketTimeoutException.class)); @@ -275,8 +272,7 @@ public class RestClientSingleHostTests extends RestClientTestCase { } for (String method : Arrays.asList("HEAD", "OPTIONS", "TRACE")) { try { - restClient.performRequest(method, "/" + randomStatusCode(getRandom()), - Collections.emptyMap(), entity); + restClient.performRequest(method, "/" + randomStatusCode(getRandom()), Collections.emptyMap(), entity); fail("request should have failed"); } catch(UnsupportedOperationException e) { assertThat(e.getMessage(), equalTo(method + " with body is not supported")); @@ -288,13 +284,13 @@ public class RestClientSingleHostTests extends RestClientTestCase { String method = randomHttpMethod(getRandom()); int statusCode = randomStatusCode(getRandom()); try { - restClient.performRequest(method, "/" + statusCode, Collections.emptyMap(), null, (Header[])null); + performRequest(method, "/" + statusCode, (Header[])null); fail("request should have failed"); } catch(NullPointerException e) { assertEquals("request headers must not be null", e.getMessage()); } try { - restClient.performRequest(method, "/" + statusCode, Collections.emptyMap(), null, (Header)null); + performRequest(method, "/" + statusCode, (Header)null); fail("request should have failed"); } catch(NullPointerException e) { assertEquals("request header must not be null", e.getMessage()); @@ -305,7 +301,13 @@ public class RestClientSingleHostTests extends RestClientTestCase { String method = randomHttpMethod(getRandom()); int statusCode = randomStatusCode(getRandom()); try { - restClient.performRequest(method, "/" + statusCode, null, null); + restClient.performRequest(method, "/" + statusCode, (Map)null); + fail("request should have failed"); + } catch(NullPointerException e) { + assertEquals("params must not be null", e.getMessage()); + } + try { + restClient.performRequest(method, "/" + statusCode, null, (HttpEntity)null); fail("request should have failed"); } catch(NullPointerException e) { assertEquals("params must not be null", e.getMessage()); @@ -352,7 +354,8 @@ public class RestClientSingleHostTests extends RestClientTestCase { String uriAsString = "/" + randomStatusCode(getRandom()); URIBuilder uriBuilder = new URIBuilder(uriAsString); Map params = Collections.emptyMap(); - if (getRandom().nextBoolean()) { + boolean hasParams = randomBoolean(); + if (hasParams) { int numParams = RandomInts.randomIntBetween(getRandom(), 1, 3); params = new HashMap<>(numParams); for (int i = 0; i < numParams; i++) { @@ -395,7 +398,8 @@ public class RestClientSingleHostTests extends RestClientTestCase { } HttpEntity entity = null; - if (request instanceof HttpEntityEnclosingRequest && getRandom().nextBoolean()) { + boolean hasBody = request instanceof HttpEntityEnclosingRequest && getRandom().nextBoolean(); + if (hasBody) { entity = new StringEntity(RandomStrings.randomAsciiOfLengthBetween(getRandom(), 10, 100)); ((HttpEntityEnclosingRequest) request).setEntity(entity); } @@ -418,10 +422,29 @@ public class RestClientSingleHostTests extends RestClientTestCase { } try { - restClient.performRequest(method, uriAsString, params, entity, headers); + if (hasParams == false && hasBody == false && randomBoolean()) { + restClient.performRequest(method, uriAsString, headers); + } else if (hasBody == false && randomBoolean()) { + restClient.performRequest(method, uriAsString, params, headers); + } else { + restClient.performRequest(method, uriAsString, params, entity, headers); + } } catch(ResponseException e) { //all good } return request; } + + private Response performRequest(String method, String endpoint, Header... headers) throws IOException { + switch(randomIntBetween(0, 2)) { + case 0: + return restClient.performRequest(method, endpoint, headers); + case 1: + return restClient.performRequest(method, endpoint, Collections.emptyMap(), headers); + case 2: + return restClient.performRequest(method, endpoint, Collections.emptyMap(), null, headers); + default: + throw new UnsupportedOperationException(); + } + } } diff --git a/client/sniffer/src/main/java/org/elasticsearch/client/sniff/HostsSniffer.java b/client/sniffer/src/main/java/org/elasticsearch/client/sniff/HostsSniffer.java index f06436c6175..bfe21f5e7d1 100644 --- a/client/sniffer/src/main/java/org/elasticsearch/client/sniff/HostsSniffer.java +++ b/client/sniffer/src/main/java/org/elasticsearch/client/sniff/HostsSniffer.java @@ -62,7 +62,7 @@ public class HostsSniffer { * Calls the elasticsearch nodes info api, parses the response and returns all the found http hosts */ public List sniffHosts() throws IOException { - try (Response response = restClient.performRequest("get", "/_nodes/http", sniffRequestParams, null)) { + try (Response response = restClient.performRequest("get", "/_nodes/http", sniffRequestParams)) { return readHosts(response.getEntity()); } }