From 04d620da742ef57d4bba08af54934b2f2d305de0 Mon Sep 17 00:00:00 2001 From: javanna Date: Wed, 8 Jun 2016 12:29:16 +0200 Subject: [PATCH] require hosts when creating RestClient.Builder Also fix order of arguments when using assertEquals --- .../client/sniff/HostsSnifferTests.java | 2 +- .../client/sniff/SnifferBuilderTests.java | 14 +++--- .../org/elasticsearch/client/RestClient.java | 30 +++++------- .../client/RestClientBuilderTests.java | 46 ++++++++----------- .../client/RestClientIntegTests.java | 4 +- .../client/RestClientMultipleHostsTests.java | 2 +- .../client/RestClientSingleHostTests.java | 2 +- .../elasticsearch/test/ESIntegTestCase.java | 2 +- .../test/rest/client/RestTestClient.java | 2 +- 9 files changed, 44 insertions(+), 60 deletions(-) diff --git a/client-sniffer/src/test/java/org/elasticsearch/client/sniff/HostsSnifferTests.java b/client-sniffer/src/test/java/org/elasticsearch/client/sniff/HostsSnifferTests.java index 57705c1b832..826d7fb35f8 100644 --- a/client-sniffer/src/test/java/org/elasticsearch/client/sniff/HostsSnifferTests.java +++ b/client-sniffer/src/test/java/org/elasticsearch/client/sniff/HostsSnifferTests.java @@ -82,7 +82,7 @@ public class HostsSnifferTests extends LuceneTestCase { public void testSniffNodes() throws IOException, URISyntaxException { HttpHost httpHost = new HttpHost(httpServer.getAddress().getHostName(), httpServer.getAddress().getPort()); - try (RestClient restClient = RestClient.builder().setHosts(httpHost).build()) { + try (RestClient restClient = RestClient.builder(httpHost).build()) { HostsSniffer sniffer = new HostsSniffer(restClient, sniffRequestTimeout, scheme); try { List sniffedHosts = sniffer.sniffHosts(); diff --git a/client-sniffer/src/test/java/org/elasticsearch/client/sniff/SnifferBuilderTests.java b/client-sniffer/src/test/java/org/elasticsearch/client/sniff/SnifferBuilderTests.java index 9bcd567ed2a..c0bb9def964 100644 --- a/client-sniffer/src/test/java/org/elasticsearch/client/sniff/SnifferBuilderTests.java +++ b/client-sniffer/src/test/java/org/elasticsearch/client/sniff/SnifferBuilderTests.java @@ -40,48 +40,48 @@ public class SnifferBuilderTests extends LuceneTestCase { for (int i = 0; i < numNodes; i++) { hosts[i] = new HttpHost("localhost", 9200 + i); } - try (RestClient client = RestClient.builder().setHosts(hosts).build()) { + try (RestClient client = RestClient.builder(hosts).build()) { try { Sniffer.builder(client).setScheme(null); fail("should have failed"); } catch(NullPointerException e) { - assertEquals(e.getMessage(), "scheme cannot be null"); + assertEquals("scheme cannot be null", e.getMessage()); } try { Sniffer.builder(client).setScheme("whatever"); fail("should have failed"); } catch(IllegalArgumentException e) { - assertEquals(e.getMessage(), "scheme must be either http or https"); + assertEquals("scheme must be either http or https", e.getMessage()); } try { Sniffer.builder(client).setSniffInterval(RandomInts.randomIntBetween(random(), Integer.MIN_VALUE, 0)); fail("should have failed"); } catch(IllegalArgumentException e) { - assertEquals(e.getMessage(), "sniffInterval must be greater than 0"); + assertEquals("sniffInterval must be greater than 0", e.getMessage()); } try { Sniffer.builder(client).setSniffRequestTimeout(RandomInts.randomIntBetween(random(), Integer.MIN_VALUE, 0)); fail("should have failed"); } catch(IllegalArgumentException e) { - assertEquals(e.getMessage(), "sniffRequestTimeout must be greater than 0"); + assertEquals("sniffRequestTimeout must be greater than 0", e.getMessage()); } try { Sniffer.builder(client).setSniffAfterFailureDelay(RandomInts.randomIntBetween(random(), Integer.MIN_VALUE, 0)); fail("should have failed"); } catch(IllegalArgumentException e) { - assertEquals(e.getMessage(), "sniffAfterFailureDelay must be greater than 0"); + assertEquals("sniffAfterFailureDelay must be greater than 0", e.getMessage()); } try { Sniffer.builder(null).build(); fail("should have failed"); } catch(NullPointerException e) { - assertEquals(e.getMessage(), "restClient cannot be null"); + assertEquals("restClient cannot be null", e.getMessage()); } try (Sniffer sniffer = Sniffer.builder(client).build()) { diff --git a/client/src/main/java/org/elasticsearch/client/RestClient.java b/client/src/main/java/org/elasticsearch/client/RestClient.java index 161abbd40a7..a01ad694b96 100644 --- a/client/src/main/java/org/elasticsearch/client/RestClient.java +++ b/client/src/main/java/org/elasticsearch/client/RestClient.java @@ -350,8 +350,8 @@ public final class RestClient implements Closeable { /** * Returns a new {@link Builder} to help with {@link RestClient} creation. */ - public static Builder builder() { - return new Builder(); + public static Builder builder(HttpHost... hosts) { + return new Builder(hosts); } /** @@ -365,13 +365,19 @@ public final class RestClient implements Closeable { private static final Header[] EMPTY_HEADERS = new Header[0]; + private final HttpHost[] hosts; private CloseableHttpClient httpClient; private int maxRetryTimeout = DEFAULT_MAX_RETRY_TIMEOUT; - private HttpHost[] hosts; private Header[] defaultHeaders = EMPTY_HEADERS; - private Builder() { - + /** + * Creates a new builder instance and sets the hosts that the client will send requests to. + */ + private Builder(HttpHost... hosts) { + if (hosts == null || hosts.length == 0) { + throw new IllegalArgumentException("no hosts provided"); + } + this.hosts = hosts; } /** @@ -399,17 +405,6 @@ public final class RestClient implements Closeable { return this; } - /** - * Sets the hosts that the client will send requests to. - */ - public Builder setHosts(HttpHost... hosts) { - if (hosts == null || hosts.length == 0) { - throw new IllegalArgumentException("no hosts provided"); - } - this.hosts = hosts; - return this; - } - /** * Sets the default request headers, to be used when creating the default http client instance. * In case the http client is set through {@link #setHttpClient(CloseableHttpClient)}, the default headers need to be @@ -431,9 +426,6 @@ public final class RestClient implements Closeable { if (httpClient == null) { httpClient = createDefaultHttpClient(null); } - if (hosts == null || hosts.length == 0) { - throw new IllegalArgumentException("no hosts provided"); - } return new RestClient(httpClient, maxRetryTimeout, defaultHeaders, hosts); } diff --git a/client/src/test/java/org/elasticsearch/client/RestClientBuilderTests.java b/client/src/test/java/org/elasticsearch/client/RestClientBuilderTests.java index 3ea0fe14ed2..75587ed8db1 100644 --- a/client/src/test/java/org/elasticsearch/client/RestClientBuilderTests.java +++ b/client/src/test/java/org/elasticsearch/client/RestClientBuilderTests.java @@ -32,61 +32,53 @@ public class RestClientBuilderTests extends LuceneTestCase { public void testBuild() throws IOException { try { - RestClient.builder().setMaxRetryTimeout(RandomInts.randomIntBetween(random(), Integer.MIN_VALUE, 0)); + RestClient.builder((HttpHost[])null); fail("should have failed"); } catch(IllegalArgumentException e) { - assertEquals(e.getMessage(), "maxRetryTimeout must be greater than 0"); + assertEquals("no hosts provided", e.getMessage()); } try { - RestClient.builder().setHosts((HttpHost[])null); + RestClient.builder(); fail("should have failed"); } catch(IllegalArgumentException e) { - assertEquals(e.getMessage(), "no hosts provided"); + assertEquals("no hosts provided", e.getMessage()); } try { - RestClient.builder().setHosts(); - fail("should have failed"); - } catch(IllegalArgumentException e) { - assertEquals(e.getMessage(), "no hosts provided"); - } - - try { - RestClient.builder().build(); - fail("should have failed"); - } catch(IllegalArgumentException e) { - assertEquals(e.getMessage(), "no hosts provided"); - } - - try { - RestClient.builder().setHosts(new HttpHost[]{new HttpHost("localhost", 9200), null}).build(); + RestClient.builder(new HttpHost[]{new HttpHost("localhost", 9200), null}).build(); fail("should have failed"); } catch(NullPointerException e) { - assertEquals(e.getMessage(), "host cannot be null"); + assertEquals("host cannot be null", e.getMessage()); } try { - RestClient.builder().setDefaultHeaders(null); + RestClient.builder(new HttpHost("localhost", 9200)).setMaxRetryTimeout(RandomInts.randomIntBetween(random(), Integer.MIN_VALUE, 0)); fail("should have failed"); - } catch(NullPointerException e) { - assertEquals(e.getMessage(), "default headers must not be null"); + } catch(IllegalArgumentException e) { + assertEquals("maxRetryTimeout must be greater than 0", e.getMessage()); } try { - RestClient.builder().setDefaultHeaders(new Header[]{null}); + RestClient.builder(new HttpHost("localhost", 9200)).setDefaultHeaders(null); fail("should have failed"); } catch(NullPointerException e) { - assertEquals(e.getMessage(), "default header must not be null"); + assertEquals("default headers must not be null", e.getMessage()); + } + + try { + RestClient.builder(new HttpHost("localhost", 9200)).setDefaultHeaders(new Header[]{null}); + fail("should have failed"); + } catch(NullPointerException e) { + assertEquals("default header must not be null", e.getMessage()); } - RestClient.Builder builder = RestClient.builder(); int numNodes = RandomInts.randomIntBetween(random(), 1, 5); HttpHost[] hosts = new HttpHost[numNodes]; for (int i = 0; i < numNodes; i++) { hosts[i] = new HttpHost("localhost", 9200 + i); } - builder.setHosts(hosts); + RestClient.Builder builder = RestClient.builder(hosts); if (random().nextBoolean()) { builder.setHttpClient(HttpClientBuilder.create().build()); } diff --git a/client/src/test/java/org/elasticsearch/client/RestClientIntegTests.java b/client/src/test/java/org/elasticsearch/client/RestClientIntegTests.java index 420ff6ed7f7..c5067dca63f 100644 --- a/client/src/test/java/org/elasticsearch/client/RestClientIntegTests.java +++ b/client/src/test/java/org/elasticsearch/client/RestClientIntegTests.java @@ -77,8 +77,8 @@ public class RestClientIntegTests extends LuceneTestCase { String headerValue = RandomStrings.randomAsciiOfLengthBetween(random(), 3, 10); defaultHeaders[i] = new BasicHeader(headerName, headerValue); } - restClient = RestClient.builder().setDefaultHeaders(defaultHeaders) - .setHosts(new HttpHost(httpServer.getAddress().getHostName(), httpServer.getAddress().getPort())).build(); + restClient = RestClient.builder(new HttpHost(httpServer.getAddress().getHostName(), httpServer.getAddress().getPort())) + .setDefaultHeaders(defaultHeaders).build(); } private static void createStatusCodeContext(HttpServer httpServer, final int statusCode) { diff --git a/client/src/test/java/org/elasticsearch/client/RestClientMultipleHostsTests.java b/client/src/test/java/org/elasticsearch/client/RestClientMultipleHostsTests.java index d78a03509d0..7ea24cef8e3 100644 --- a/client/src/test/java/org/elasticsearch/client/RestClientMultipleHostsTests.java +++ b/client/src/test/java/org/elasticsearch/client/RestClientMultipleHostsTests.java @@ -87,7 +87,7 @@ public class RestClientMultipleHostsTests extends LuceneTestCase { for (int i = 0; i < numHosts; i++) { httpHosts[i] = new HttpHost("localhost", 9200 + i); } - restClient = RestClient.builder().setHosts(httpHosts).setHttpClient(httpClient).build(); + restClient = RestClient.builder(httpHosts).setHttpClient(httpClient).build(); failureListener = new TrackingFailureListener(); restClient.setFailureListener(failureListener); } diff --git a/client/src/test/java/org/elasticsearch/client/RestClientSingleHostTests.java b/client/src/test/java/org/elasticsearch/client/RestClientSingleHostTests.java index c3c08bee628..1112f38bbee 100644 --- a/client/src/test/java/org/elasticsearch/client/RestClientSingleHostTests.java +++ b/client/src/test/java/org/elasticsearch/client/RestClientSingleHostTests.java @@ -122,7 +122,7 @@ public class RestClientSingleHostTests extends LuceneTestCase { defaultHeaders[i] = new BasicHeader(headerName, headerValue); } httpHost = new HttpHost("localhost", 9200); - restClient = RestClient.builder().setHosts(httpHost).setHttpClient(httpClient).setDefaultHeaders(defaultHeaders).build(); + restClient = RestClient.builder(httpHost).setHttpClient(httpClient).setDefaultHeaders(defaultHeaders).build(); failureListener = new TrackingFailureListener(); restClient.setFailureListener(failureListener); } diff --git a/test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java index ab25d25fac0..054da7664b5 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java @@ -2068,7 +2068,7 @@ public abstract class ESIntegTestCase extends ESTestCase { hosts.add(new HttpHost(NetworkAddress.format(address.getAddress()), address.getPort(), protocol)); } } - RestClient.Builder builder = RestClient.builder().setHosts(hosts.toArray(new HttpHost[hosts.size()])); + RestClient.Builder builder = RestClient.builder(hosts.toArray(new HttpHost[hosts.size()])); if (httpClient != null) { builder.setHttpClient(httpClient); } diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/client/RestTestClient.java b/test/framework/src/main/java/org/elasticsearch/test/rest/client/RestTestClient.java index 94f85422edf..d21e302168b 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/client/RestTestClient.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/client/RestTestClient.java @@ -305,7 +305,7 @@ public class RestTestClient implements Closeable { hosts[i] = new HttpHost(url.getHost(), url.getPort(), protocol); } - RestClient.Builder builder = RestClient.builder().setHttpClient(httpClient).setHosts(hosts); + RestClient.Builder builder = RestClient.builder(hosts).setHttpClient(httpClient); try (ThreadContext threadContext = new ThreadContext(settings)) { Header[] defaultHeaders = new Header[threadContext.getHeaders().size()]; int i = 0;