Simplify Sniffer initialization and automatically create the default HostsSniffer (#19599)

Simplify Sniffer initialization and automatically create the default HostsSniffer

Take Sniffer.Builder out to its own top level class. Remove HostsSniffer.Builder and let SnifferBuilder create the default HostsSniffer. This simplifies the Sniffer initialization as the HostsSniffer is not mandatory anymore. It can still be specified though in case the configuration needs to be changed or a different impl has to be used. Also make HostsSniffer an interface.

client/sniffer/src/main/java/org/elasticsearch/client/sniff/ElasticsearchHostsSniffer.java

@@ -0,0 +1,170 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.elasticsearch.client.sniff;
+
+import com.fasterxml.jackson.core.JsonFactory;
+import com.fasterxml.jackson.core.JsonParser;
+import com.fasterxml.jackson.core.JsonToken;
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+import org.apache.http.HttpEntity;
+import org.apache.http.HttpHost;
+import org.elasticsearch.client.Response;
+import org.elasticsearch.client.RestClient;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.URI;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * Class responsible for sniffing the http hosts from elasticsearch through the nodes info api and returning them back.
+ * Compatible with elasticsearch 5.x and 2.x.
+ */
+public final class ElasticsearchHostsSniffer implements HostsSniffer {
+
+    private static final Log logger = LogFactory.getLog(ElasticsearchHostsSniffer.class);
+
+    public static final long DEFAULT_SNIFF_REQUEST_TIMEOUT = TimeUnit.SECONDS.toMillis(1);
+
+    private final RestClient restClient;
+    private final Map<String, String> sniffRequestParams;
+    private final Scheme scheme;
+    private final JsonFactory jsonFactory = new JsonFactory();
+
+    /**
+     * Creates a new instance of the Elasticsearch sniffer. It will use the provided {@link RestClient} to fetch the hosts,
+     * through the nodes info api, the default sniff request timeout value {@link #DEFAULT_SNIFF_REQUEST_TIMEOUT} and http
+     * as the scheme for all the hosts.
+     * @param restClient client used to fetch the hosts from elasticsearch through nodes info api. Usually the same instance
+     *                   that is also provided to {@link Sniffer#builder(RestClient)}, so that the hosts are set to the same
+     *                   client that was used to fetch them.
+     */
+    public ElasticsearchHostsSniffer(RestClient restClient) {
+        this(restClient, DEFAULT_SNIFF_REQUEST_TIMEOUT, ElasticsearchHostsSniffer.Scheme.HTTP);
+    }
+
+    /**
+     * Creates a new instance of the Elasticsearch sniffer. It will use the provided {@link RestClient} to fetch the hosts
+     * through the nodes info api, the provided sniff request timeout value and scheme.
+     * @param restClient client used to fetch the hosts from elasticsearch through nodes info api. Usually the same instance
+     *                   that is also provided to {@link Sniffer#builder(RestClient)}, so that the hosts are set to the same
+     *                   client that was used to sniff them.
+     * @param sniffRequestTimeoutMillis the sniff request timeout (in milliseconds) to be passed in as a query string parameter
+     *                                  to elasticsearch. Allows to halt the request without any failure, as only the nodes
+     *                                  that have responded within this timeout will be returned.
+     * @param scheme the scheme to associate sniffed nodes with (as it is not returned by elasticsearch)
+     */
+    public ElasticsearchHostsSniffer(RestClient restClient, long sniffRequestTimeoutMillis, Scheme scheme) {
+        this.restClient = Objects.requireNonNull(restClient, "restClient cannot be null");
+        if (sniffRequestTimeoutMillis < 0) {
+            throw new IllegalArgumentException("sniffRequestTimeoutMillis must be greater than 0");
+        }
+        this.sniffRequestParams = Collections.<String, String>singletonMap("timeout", sniffRequestTimeoutMillis + "ms");
+        this.scheme = Objects.requireNonNull(scheme, "scheme cannot be null");
+    }
+
+    /**
+     * Calls the elasticsearch nodes info api, parses the response and returns all the found http hosts
+     */
+    public List<HttpHost> sniffHosts() throws IOException {
+        Response response = restClient.performRequest("get", "/_nodes/http", sniffRequestParams);
+        return readHosts(response.getEntity());
+    }
+
+    private List<HttpHost> readHosts(HttpEntity entity) throws IOException {
+        try (InputStream inputStream = entity.getContent()) {
+            JsonParser parser = jsonFactory.createParser(inputStream);
+            if (parser.nextToken() != JsonToken.START_OBJECT) {
+                throw new IOException("expected data to start with an object");
+            }
+            List<HttpHost> hosts = new ArrayList<>();
+            while (parser.nextToken() != JsonToken.END_OBJECT) {
+                if (parser.getCurrentToken() == JsonToken.START_OBJECT) {
+                    if ("nodes".equals(parser.getCurrentName())) {
+                        while (parser.nextToken() != JsonToken.END_OBJECT) {
+                            JsonToken token = parser.nextToken();
+                            assert token == JsonToken.START_OBJECT;
+                            String nodeId = parser.getCurrentName();
+                            HttpHost sniffedHost = readHost(nodeId, parser, this.scheme);
+                            if (sniffedHost != null) {
+                                logger.trace("adding node [" + nodeId + "]");
+                                hosts.add(sniffedHost);
+                            }
+                        }
+                    } else {
+                        parser.skipChildren();
+                    }
+                }
+            }
+            return hosts;
+        }
+    }
+
+    private static HttpHost readHost(String nodeId, JsonParser parser, Scheme scheme) throws IOException {
+        HttpHost httpHost = null;
+        String fieldName = null;
+        while (parser.nextToken() != JsonToken.END_OBJECT) {
+            if (parser.getCurrentToken() == JsonToken.FIELD_NAME) {
+                fieldName = parser.getCurrentName();
+            } else if (parser.getCurrentToken() == JsonToken.START_OBJECT) {
+                if ("http".equals(fieldName)) {
+                    while (parser.nextToken() != JsonToken.END_OBJECT) {
+                        if (parser.getCurrentToken() == JsonToken.VALUE_STRING && "publish_address".equals(parser.getCurrentName())) {
+                            URI boundAddressAsURI = URI.create(scheme + "://" + parser.getValueAsString());
+                            httpHost = new HttpHost(boundAddressAsURI.getHost(), boundAddressAsURI.getPort(),
+                                    boundAddressAsURI.getScheme());
+                        } else if (parser.getCurrentToken() == JsonToken.START_OBJECT) {
+                            parser.skipChildren();
+                        }
+                    }
+                } else {
+                    parser.skipChildren();
+                }
+            }
+        }
+        //http section is not present if http is not enabled on the node, ignore such nodes
+        if (httpHost == null) {
+            logger.debug("skipping node [" + nodeId + "] with http disabled");
+            return null;
+        }
+        return httpHost;
+    }
+
+    public enum Scheme {
+        HTTP("http"), HTTPS("https");
+
+        private final String name;
+
+        Scheme(String name) {
+            this.name = name;
+        }
+
+        @Override
+        public String toString() {
+            return name;
+        }
+    }
+}

client/sniffer/src/main/java/org/elasticsearch/client/sniff/HostsSniffer.java

@@ -19,175 +19,17 @@
 
 package org.elasticsearch.client.sniff;
 
-import com.fasterxml.jackson.core.JsonFactory;
-import com.fasterxml.jackson.core.JsonParser;
-import com.fasterxml.jackson.core.JsonToken;
-import org.apache.commons.logging.Log;
-import org.apache.commons.logging.LogFactory;
-import org.apache.http.HttpEntity;
 import org.apache.http.HttpHost;
-import org.elasticsearch.client.Response;
-import org.elasticsearch.client.RestClient;
 
 import java.io.IOException;
-import java.io.InputStream;
-import java.net.URI;
-import java.util.ArrayList;
-import java.util.Collections;
 import java.util.List;
-import java.util.Map;
-import java.util.Objects;
-import java.util.concurrent.TimeUnit;
 
 /**
- * Class responsible for sniffing the http hosts from elasticsearch through the nodes info api and returning them back.
- * Compatible with elasticsearch 5.x and 2.x.
+ * Responsible for sniffing the http hosts
  */
-public class HostsSniffer {
-
-    private static final Log logger = LogFactory.getLog(HostsSniffer.class);
-
-    private final RestClient restClient;
-    private final Map<String, String> sniffRequestParams;
-    private final Scheme scheme;
-    private final JsonFactory jsonFactory = new JsonFactory();
-
-    protected HostsSniffer(RestClient restClient, long sniffRequestTimeoutMillis, Scheme scheme) {
-        this.restClient = restClient;
-        this.sniffRequestParams = Collections.<String, String>singletonMap("timeout", sniffRequestTimeoutMillis + "ms");
-        this.scheme = scheme;
-    }
-
+public interface HostsSniffer {
     /**
-     * Calls the elasticsearch nodes info api, parses the response and returns all the found http hosts
+     * Returns the sniffed http hosts
      */
-    public List<HttpHost> sniffHosts() throws IOException {
-        Response response = restClient.performRequest("get", "/_nodes/http", sniffRequestParams);
-        return readHosts(response.getEntity());
-    }
-
-    private List<HttpHost> readHosts(HttpEntity entity) throws IOException {
-        try (InputStream inputStream = entity.getContent()) {
-            JsonParser parser = jsonFactory.createParser(inputStream);
-            if (parser.nextToken() != JsonToken.START_OBJECT) {
-                throw new IOException("expected data to start with an object");
-            }
-            List<HttpHost> hosts = new ArrayList<>();
-            while (parser.nextToken() != JsonToken.END_OBJECT) {
-                if (parser.getCurrentToken() == JsonToken.START_OBJECT) {
-                    if ("nodes".equals(parser.getCurrentName())) {
-                        while (parser.nextToken() != JsonToken.END_OBJECT) {
-                            JsonToken token = parser.nextToken();
-                            assert token == JsonToken.START_OBJECT;
-                            String nodeId = parser.getCurrentName();
-                            HttpHost sniffedHost = readHost(nodeId, parser, this.scheme);
-                            if (sniffedHost != null) {
-                                logger.trace("adding node [" + nodeId + "]");
-                                hosts.add(sniffedHost);
-                            }
-                        }
-                    } else {
-                        parser.skipChildren();
-                    }
-                }
-            }
-            return hosts;
-        }
-    }
-
-    private static HttpHost readHost(String nodeId, JsonParser parser, Scheme scheme) throws IOException {
-        HttpHost httpHost = null;
-        String fieldName = null;
-        while (parser.nextToken() != JsonToken.END_OBJECT) {
-            if (parser.getCurrentToken() == JsonToken.FIELD_NAME) {
-                fieldName = parser.getCurrentName();
-            } else if (parser.getCurrentToken() == JsonToken.START_OBJECT) {
-                if ("http".equals(fieldName)) {
-                    while (parser.nextToken() != JsonToken.END_OBJECT) {
-                        if (parser.getCurrentToken() == JsonToken.VALUE_STRING && "publish_address".equals(parser.getCurrentName())) {
-                            URI boundAddressAsURI = URI.create(scheme + "://" + parser.getValueAsString());
-                            httpHost = new HttpHost(boundAddressAsURI.getHost(), boundAddressAsURI.getPort(),
-                                    boundAddressAsURI.getScheme());
-                        } else if (parser.getCurrentToken() == JsonToken.START_OBJECT) {
-                            parser.skipChildren();
-                        }
-                    }
-                } else {
-                    parser.skipChildren();
-                }
-            }
-        }
-        //http section is not present if http is not enabled on the node, ignore such nodes
-        if (httpHost == null) {
-            logger.debug("skipping node [" + nodeId + "] with http disabled");
-            return null;
-        }
-        return httpHost;
-    }
-
-    /**
-     * Returns a new {@link Builder} to help with {@link HostsSniffer} creation.
-     */
-    public static Builder builder(RestClient restClient) {
-        return new Builder(restClient);
-    }
-
-    public enum Scheme {
-        HTTP("http"), HTTPS("https");
-
-        private final String name;
-
-        Scheme(String name) {
-            this.name = name;
-        }
-
-        @Override
-        public String toString() {
-            return name;
-        }
-    }
-
-    /**
-     * HostsSniffer builder. Helps creating a new {@link HostsSniffer}.
-     */
-    public static class Builder {
-        public static final long DEFAULT_SNIFF_REQUEST_TIMEOUT = TimeUnit.SECONDS.toMillis(1);
-
-        private final RestClient restClient;
-        private long sniffRequestTimeoutMillis = DEFAULT_SNIFF_REQUEST_TIMEOUT;
-        private Scheme scheme = Scheme.HTTP;
-
-        private Builder(RestClient restClient) {
-            Objects.requireNonNull(restClient, "restClient cannot be null");
-            this.restClient = restClient;
-        }
-
-        /**
-         * Sets the sniff request timeout (in milliseconds) to be passed in as a query string parameter to elasticsearch.
-         * Allows to halt the request without any failure, as only the nodes that have responded within this timeout will be returned.
-         */
-        public Builder setSniffRequestTimeoutMillis(int sniffRequestTimeoutMillis) {
-            if (sniffRequestTimeoutMillis <= 0) {
-                throw new IllegalArgumentException("sniffRequestTimeoutMillis must be greater than 0");
-            }
-            this.sniffRequestTimeoutMillis = sniffRequestTimeoutMillis;
-            return this;
-        }
-
-        /**
-         * Sets the scheme to associate sniffed nodes with (as it is not returned by elasticsearch)
-         */
-        public Builder setScheme(Scheme scheme) {
-            Objects.requireNonNull(scheme, "scheme cannot be null");
-            this.scheme = scheme;
-            return this;
-        }
-
-        /**
-         * Creates a new {@link HostsSniffer} instance given the provided configuration
-         */
-        public HostsSniffer build() {
-            return new HostsSniffer(restClient, sniffRequestTimeoutMillis, scheme);
-        }
-    }
+    List<HttpHost> sniffHosts() throws IOException;
 }

client/sniffer/src/main/java/org/elasticsearch/client/sniff/Sniffer.java

@@ -28,7 +28,6 @@ import org.elasticsearch.client.RestClientBuilder;
 import java.io.Closeable;
 import java.io.IOException;
 import java.util.List;
-import java.util.Objects;
 import java.util.concurrent.Executors;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.ScheduledFuture;
@@ -36,12 +35,12 @@ import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
 
 /**
- * Class responsible for sniffing nodes from an elasticsearch cluster and setting them to a provided instance of {@link RestClient}.
- * Must be created via {@link Builder}, which allows to set all of the different options or rely on defaults.
+ * Class responsible for sniffing nodes from some source (default is elasticsearch itself) and setting them to a provided instance of
+ * {@link RestClient}. Must be created via {@link SnifferBuilder}, which allows to set all of the different options or rely on defaults.
  * A background task fetches the nodes through the {@link HostsSniffer} and sets them to the {@link RestClient} instance.
  * It is possible to perform sniffing on failure by creating a {@link SniffOnFailureListener} and providing it as an argument to
- * {@link RestClientBuilder#setFailureListener(RestClient.FailureListener)}. The Sniffer implementation
- * needs to be lazily set to the previously created SniffOnFailureListener through {@link SniffOnFailureListener#setSniffer(Sniffer)}.
+ * {@link RestClientBuilder#setFailureListener(RestClient.FailureListener)}. The Sniffer implementation needs to be lazily set to the
+ * previously created SniffOnFailureListener through {@link SniffOnFailureListener#setSniffer(Sniffer)}.
  */
 public final class Sniffer implements Closeable {
 
@@ -49,7 +48,7 @@ public final class Sniffer implements Closeable {
 
     private final Task task;
 
-    private Sniffer(RestClient restClient, HostsSniffer hostsSniffer, long sniffInterval, long sniffAfterFailureDelay) {
+    Sniffer(RestClient restClient, HostsSniffer hostsSniffer, long sniffInterval, long sniffAfterFailureDelay) {
         this.task = new Task(hostsSniffer, restClient, sniffInterval, sniffAfterFailureDelay);
     }
 
@@ -144,64 +143,12 @@ public final class Sniffer implements Closeable {
     }
 
     /**
-     * Returns a new {@link Builder} to help with {@link Sniffer} creation.
+     * Returns a new {@link SnifferBuilder} to help with {@link Sniffer} creation.
+     *
+     * @param restClient the client that gets its hosts set (via {@link RestClient#setHosts(HttpHost...)}) once they are fetched
+     * @return a new instance of {@link SnifferBuilder}
      */
-    public static Builder builder(RestClient restClient, HostsSniffer hostsSniffer) {
-        return new Builder(restClient, hostsSniffer);
-    }
-
-    /**
-     * Sniffer builder. Helps creating a new {@link Sniffer}.
-     */
-    public static final class Builder {
-        public static final long DEFAULT_SNIFF_INTERVAL = TimeUnit.MINUTES.toMillis(5);
-        public static final long DEFAULT_SNIFF_AFTER_FAILURE_DELAY = TimeUnit.MINUTES.toMillis(1);
-
-        private final RestClient restClient;
-        private final HostsSniffer hostsSniffer;
-        private long sniffIntervalMillis = DEFAULT_SNIFF_INTERVAL;
-        private long sniffAfterFailureDelayMillis = DEFAULT_SNIFF_AFTER_FAILURE_DELAY;
-
-        /**
-         * Creates a new builder instance by providing the {@link RestClient} that will be used to communicate with elasticsearch,
-         * and the
-         */
-        private Builder(RestClient restClient, HostsSniffer hostsSniffer) {
-            Objects.requireNonNull(restClient, "restClient cannot be null");
-            this.restClient = restClient;
-            Objects.requireNonNull(hostsSniffer, "hostsSniffer cannot be null");
-            this.hostsSniffer = hostsSniffer;
-        }
-
-        /**
-         * Sets the interval between consecutive ordinary sniff executions in milliseconds. Will be honoured when
-         * sniffOnFailure is disabled or when there are no failures between consecutive sniff executions.
-         * @throws IllegalArgumentException if sniffIntervalMillis is not greater than 0
-         */
-        public Builder setSniffIntervalMillis(int sniffIntervalMillis) {
-            if (sniffIntervalMillis <= 0) {
-                throw new IllegalArgumentException("sniffIntervalMillis must be greater than 0");
-            }
-            this.sniffIntervalMillis = sniffIntervalMillis;
-            return this;
-        }
-
-        /**
-         * Sets the delay of a sniff execution scheduled after a failure (in milliseconds)
-         */
-        public Builder setSniffAfterFailureDelayMillis(int sniffAfterFailureDelayMillis) {
-            if (sniffAfterFailureDelayMillis <= 0) {
-                throw new IllegalArgumentException("sniffAfterFailureDelayMillis must be greater than 0");
-            }
-            this.sniffAfterFailureDelayMillis = sniffAfterFailureDelayMillis;
-            return this;
-        }
-
-        /**
-         * Creates the {@link Sniffer} based on the provided configuration.
-         */
-        public Sniffer build() {
-            return new Sniffer(restClient, hostsSniffer, sniffIntervalMillis, sniffAfterFailureDelayMillis);
-        }
+    public static SnifferBuilder builder(RestClient restClient) {
+        return new SnifferBuilder(restClient);
     }
 }

client/sniffer/src/main/java/org/elasticsearch/client/sniff/SnifferBuilder.java

@@ -0,0 +1,91 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.elasticsearch.client.sniff;
+
+import org.elasticsearch.client.RestClient;
+
+import java.util.Objects;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * Sniffer builder. Helps creating a new {@link Sniffer}.
+ */
+public final class SnifferBuilder {
+    public static final long DEFAULT_SNIFF_INTERVAL = TimeUnit.MINUTES.toMillis(5);
+    public static final long DEFAULT_SNIFF_AFTER_FAILURE_DELAY = TimeUnit.MINUTES.toMillis(1);
+
+    private final RestClient restClient;
+    private long sniffIntervalMillis = DEFAULT_SNIFF_INTERVAL;
+    private long sniffAfterFailureDelayMillis = DEFAULT_SNIFF_AFTER_FAILURE_DELAY;
+    private HostsSniffer hostsSniffer;
+
+    /**
+     * Creates a new builder instance by providing the {@link RestClient} that will be used to communicate with elasticsearch
+     */
+    SnifferBuilder(RestClient restClient) {
+        Objects.requireNonNull(restClient, "restClient cannot be null");
+        this.restClient = restClient;
+    }
+
+    /**
+     * Sets the interval between consecutive ordinary sniff executions in milliseconds. Will be honoured when
+     * sniffOnFailure is disabled or when there are no failures between consecutive sniff executions.
+     * @throws IllegalArgumentException if sniffIntervalMillis is not greater than 0
+     */
+    public SnifferBuilder setSniffIntervalMillis(int sniffIntervalMillis) {
+        if (sniffIntervalMillis <= 0) {
+            throw new IllegalArgumentException("sniffIntervalMillis must be greater than 0");
+        }
+        this.sniffIntervalMillis = sniffIntervalMillis;
+        return this;
+    }
+
+    /**
+     * Sets the delay of a sniff execution scheduled after a failure (in milliseconds)
+     */
+    public SnifferBuilder setSniffAfterFailureDelayMillis(int sniffAfterFailureDelayMillis) {
+        if (sniffAfterFailureDelayMillis <= 0) {
+            throw new IllegalArgumentException("sniffAfterFailureDelayMillis must be greater than 0");
+        }
+        this.sniffAfterFailureDelayMillis = sniffAfterFailureDelayMillis;
+        return this;
+    }
+
+    /**
+     * Sets the {@link HostsSniffer} to be used to read hosts. A default instance of {@link ElasticsearchHostsSniffer}
+     * is created when not provided. This method can be used to change the configuration of the {@link ElasticsearchHostsSniffer},
+     * or to provide a different implementation (e.g. in case hosts need to taken from a different source).
+     */
+    public SnifferBuilder setHostsSniffer(HostsSniffer hostsSniffer) {
+        Objects.requireNonNull(hostsSniffer, "hostsSniffer cannot be null");
+        this.hostsSniffer = hostsSniffer;
+        return this;
+    }
+
+    /**
+     * Creates the {@link Sniffer} based on the provided configuration.
+     */
+    public Sniffer build() {
+        if (hostsSniffer == null) {
+            this.hostsSniffer = new ElasticsearchHostsSniffer(restClient);
+        }
+        return new Sniffer(restClient, hostsSniffer, sniffIntervalMillis, sniffAfterFailureDelayMillis);
+    }
+}

client/sniffer/src/test/java/org/elasticsearch/client/sniff/HostsSnifferTests.java → client/sniffer/src/test/java/org/elasticsearch/client/sniff/ElasticsearchHostsSnifferTests.java

@@ -60,17 +60,17 @@ import static org.junit.Assert.fail;
 
 //animal-sniffer doesn't like our usage of com.sun.net.httpserver.* classes
 @IgnoreJRERequirement
-public class HostsSnifferTests extends RestClientTestCase {
+public class ElasticsearchHostsSnifferTests extends RestClientTestCase {
 
     private int sniffRequestTimeout;
-    private HostsSniffer.Scheme scheme;
+    private ElasticsearchHostsSniffer.Scheme scheme;
     private SniffResponse sniffResponse;
     private HttpServer httpServer;
 
     @Before
     public void startHttpServer() throws IOException {
         this.sniffRequestTimeout = RandomInts.randomIntBetween(getRandom(), 1000, 10000);
-        this.scheme = RandomPicks.randomFrom(getRandom(), HostsSniffer.Scheme.values());
+        this.scheme = RandomPicks.randomFrom(getRandom(), ElasticsearchHostsSniffer.Scheme.values());
         if (rarely()) {
             this.sniffResponse = SniffResponse.buildFailure();
         } else {
@@ -85,14 +85,35 @@ public class HostsSnifferTests extends RestClientTestCase {
         httpServer.stop(0);
     }
 
-    public void testSniffNodes() throws IOException {
+    public void testConstructorValidation() throws IOException {
+        try {
+            new ElasticsearchHostsSniffer(null, 1, ElasticsearchHostsSniffer.Scheme.HTTP);
+            fail("should have failed");
+        } catch(NullPointerException e) {
+            assertEquals("restClient cannot be null", e.getMessage());
+        }
         HttpHost httpHost = new HttpHost(httpServer.getAddress().getHostString(), httpServer.getAddress().getPort());
         try (RestClient restClient = RestClient.builder(httpHost).build()) {
-            HostsSniffer.Builder builder = HostsSniffer.builder(restClient).setSniffRequestTimeoutMillis(sniffRequestTimeout);
-            if (scheme != HostsSniffer.Scheme.HTTP || randomBoolean()) {
-                builder.setScheme(scheme);
+            try {
+                new ElasticsearchHostsSniffer(restClient, 1, null);
+                fail("should have failed");
+            } catch (NullPointerException e) {
+                assertEquals(e.getMessage(), "scheme cannot be null");
             }
-            HostsSniffer sniffer = builder.build();
+            try {
+                new ElasticsearchHostsSniffer(restClient, RandomInts.randomIntBetween(getRandom(), Integer.MIN_VALUE, 0),
+                        ElasticsearchHostsSniffer.Scheme.HTTP);
+                fail("should have failed");
+            } catch (IllegalArgumentException e) {
+                assertEquals(e.getMessage(), "sniffRequestTimeoutMillis must be greater than 0");
+            }
+        }
+    }
+
+    public void testSniffNodes() throws IOException {
+        HttpHost httpHost = new HttpHost(httpServer.getAddress().getHostString(), httpServer.getAddress().getPort());
+        try (RestClient restClient = RestClient.builder(httpHost).build()) {
+            ElasticsearchHostsSniffer sniffer = new ElasticsearchHostsSniffer(restClient, sniffRequestTimeout, scheme);
             try {
                 List<HttpHost> sniffedHosts = sniffer.sniffHosts();
                 if (sniffResponse.isFailure) {
@@ -153,7 +174,7 @@ public class HostsSnifferTests extends RestClientTestCase {
         }
     }
 
-    private static SniffResponse buildSniffResponse(HostsSniffer.Scheme scheme) throws IOException {
+    private static SniffResponse buildSniffResponse(ElasticsearchHostsSniffer.Scheme scheme) throws IOException {
         int numNodes = RandomInts.randomIntBetween(getRandom(), 1, 5);
         List<HttpHost> hosts = new ArrayList<>(numNodes);
         JsonFactory jsonFactory = new JsonFactory();

client/sniffer/src/test/java/org/elasticsearch/client/sniff/HostsSnifferBuilderTests.java

@@ -1,73 +0,0 @@
-/*
- * Licensed to Elasticsearch under one or more contributor
- * license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright
- * ownership. Elasticsearch licenses this file to you under
- * the Apache License, Version 2.0 (the "License"); you may
- * not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *    http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-
-package org.elasticsearch.client.sniff;
-
-import com.carrotsearch.randomizedtesting.generators.RandomInts;
-import com.carrotsearch.randomizedtesting.generators.RandomPicks;
-import org.apache.http.HttpHost;
-import org.elasticsearch.client.RestClient;
-import org.elasticsearch.client.RestClientTestCase;
-
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.fail;
-
-public class HostsSnifferBuilderTests extends RestClientTestCase {
-
-    public void testBuild() throws Exception {
-        try {
-            HostsSniffer.builder(null);
-            fail("should have failed");
-        } catch(NullPointerException e) {
-            assertEquals(e.getMessage(), "restClient cannot be null");
-        }
-
-        int numNodes = RandomInts.randomIntBetween(getRandom(), 1, 5);
-        HttpHost[] hosts = new HttpHost[numNodes];
-        for (int i = 0; i < numNodes; i++) {
-            hosts[i] = new HttpHost("localhost", 9200 + i);
-        }
-
-        try (RestClient client = RestClient.builder(hosts).build()) {
-            try {
-                HostsSniffer.builder(client).setScheme(null);
-                fail("should have failed");
-            } catch(NullPointerException e) {
-                assertEquals(e.getMessage(), "scheme cannot be null");
-            }
-
-            try {
-                HostsSniffer.builder(client).setSniffRequestTimeoutMillis(RandomInts.randomIntBetween(getRandom(), Integer.MIN_VALUE, 0));
-                fail("should have failed");
-            } catch(IllegalArgumentException e) {
-                assertEquals(e.getMessage(), "sniffRequestTimeoutMillis must be greater than 0");
-            }
-
-            HostsSniffer.Builder builder = HostsSniffer.builder(client);
-            if (getRandom().nextBoolean()) {
-                builder.setScheme(RandomPicks.randomFrom(getRandom(), HostsSniffer.Scheme.values()));
-            }
-            if (getRandom().nextBoolean()) {
-                builder.setSniffRequestTimeoutMillis(RandomInts.randomIntBetween(getRandom(), 1, Integer.MAX_VALUE));
-            }
-            assertNotNull(builder.build());
-        }
-    }
-}

client/sniffer/src/test/java/org/elasticsearch/client/sniff/MockHostsSniffer.java

@@ -22,18 +22,15 @@ package org.elasticsearch.client.sniff;
 import org.apache.http.HttpHost;
 
 import java.io.IOException;
-import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 
-class MockHostsSniffer extends HostsSniffer {
-    MockHostsSniffer() {
-        super(null, -1, null);
-    }
-
+/**
+ * Mock implementation of {@link HostsSniffer}. Useful to prevent any connection attempt while testing builders etc.
+ */
+class MockHostsSniffer implements HostsSniffer {
     @Override
     public List<HttpHost> sniffHosts() throws IOException {
-        List<HttpHost> hosts = new ArrayList<>();
-        hosts.add(new HttpHost("localhost", 9200));
-        return hosts;
+        return Collections.singletonList(new HttpHost("localhost", 9200));
     }
 }

client/sniffer/src/test/java/org/elasticsearch/client/sniff/SniffOnFailureListenerTests.java

@@ -46,7 +46,7 @@ public class SniffOnFailureListenerTests extends RestClientTestCase {
         }
 
         try (RestClient restClient = RestClient.builder(new HttpHost("localhost", 9200)).build()) {
-            try (Sniffer sniffer = Sniffer.builder(restClient, new MockHostsSniffer()).build()) {
+            try (Sniffer sniffer = Sniffer.builder(restClient).setHostsSniffer(new MockHostsSniffer()).build()) {
                 listener.setSniffer(sniffer);
                 try {
                     listener.setSniffer(sniffer);

client/sniffer/src/test/java/org/elasticsearch/client/sniff/SnifferBuilderTests.java

@@ -37,50 +37,52 @@ public class SnifferBuilderTests extends RestClientTestCase {
             hosts[i] = new HttpHost("localhost", 9200 + i);
         }
 
-        HostsSniffer hostsSniffer = new MockHostsSniffer();
-
         try (RestClient client = RestClient.builder(hosts).build()) {
             try {
-                Sniffer.builder(null, hostsSniffer).build();
+                Sniffer.builder(null).build();
                 fail("should have failed");
             } catch(NullPointerException e) {
                 assertEquals("restClient cannot be null", e.getMessage());
             }
 
             try {
-                Sniffer.builder(client, null).build();
+                Sniffer.builder(client).setSniffIntervalMillis(RandomInts.randomIntBetween(getRandom(), Integer.MIN_VALUE, 0));
                 fail("should have failed");
-            } catch(NullPointerException e) {
-                assertEquals("hostsSniffer cannot be null", e.getMessage());
+            } catch(IllegalArgumentException e) {
+                assertEquals("sniffIntervalMillis must be greater than 0", e.getMessage());
             }
 
             try {
-                Sniffer.builder(client, hostsSniffer)
-                        .setSniffIntervalMillis(RandomInts.randomIntBetween(getRandom(), Integer.MIN_VALUE, 0));
+                Sniffer.builder(client).setSniffAfterFailureDelayMillis(RandomInts.randomIntBetween(getRandom(), Integer.MIN_VALUE, 0));
                 fail("should have failed");
             } catch(IllegalArgumentException e) {
-                assertEquals("sniffIntervalMillis must be greater than 0", e.getMessage());
+                assertEquals("sniffAfterFailureDelayMillis must be greater than 0", e.getMessage());
             }
 
+
             try {
-                Sniffer.builder(client, hostsSniffer)
-                        .setSniffAfterFailureDelayMillis(RandomInts.randomIntBetween(getRandom(), Integer.MIN_VALUE, 0));
+                Sniffer.builder(client).setHostsSniffer(null);
                 fail("should have failed");
-            } catch(IllegalArgumentException e) {
-                assertEquals("sniffAfterFailureDelayMillis must be greater than 0", e.getMessage());
+            } catch(NullPointerException e) {
+                assertEquals("hostsSniffer cannot be null", e.getMessage());
             }
 
-            try (Sniffer sniffer = Sniffer.builder(client, hostsSniffer).build()) {
+
+            try (Sniffer sniffer = Sniffer.builder(client).build()) {
                 assertNotNull(sniffer);
             }
 
-            Sniffer.Builder builder = Sniffer.builder(client, hostsSniffer);
+            SnifferBuilder builder = Sniffer.builder(client);
             if (getRandom().nextBoolean()) {
                 builder.setSniffIntervalMillis(RandomInts.randomIntBetween(getRandom(), 1, Integer.MAX_VALUE));
             }
             if (getRandom().nextBoolean()) {
                 builder.setSniffAfterFailureDelayMillis(RandomInts.randomIntBetween(getRandom(), 1, Integer.MAX_VALUE));
             }
+            if (getRandom().nextBoolean()) {
+                builder.setHostsSniffer(new MockHostsSniffer());
+            }
+
             try (Sniffer sniffer = builder.build()) {
                 assertNotNull(sniffer);
             }