RestClient: simplify ssl configuration and make http config callback functional friendly

client/rest/src/main/java/org/elasticsearch/client/RestClient.java

@@ -395,6 +395,7 @@ public final class RestClient implements Closeable {
         private Header[] defaultHeaders = EMPTY_HEADERS;
         private FailureListener failureListener;
         private HttpClientConfigCallback httpClientConfigCallback;
+        private RequestConfigCallback requestConfigCallback;
 
         /**
          * Creates a new builder instance and sets the hosts that the client will send requests to.
@@ -450,6 +451,15 @@ public final class RestClient implements Closeable {
             return this;
         }
 
+        /**
+         * Sets the {@link RequestConfigCallback} to be used to customize http client configuration
+         */
+        public Builder setRequestConfigCallback(RequestConfigCallback requestConfigCallback) {
+            Objects.requireNonNull(requestConfigCallback, "requestConfigCallback must not be null");
+            this.requestConfigCallback = requestConfigCallback;
+            return this;
+        }
+
         /**
          * Creates a new {@link RestClient} based on the provided configuration.
          */
@@ -467,8 +477,8 @@ public final class RestClient implements Closeable {
                     .setSocketTimeout(DEFAULT_SOCKET_TIMEOUT_MILLIS)
                     .setConnectionRequestTimeout(DEFAULT_CONNECTION_REQUEST_TIMEOUT_MILLIS);
 
-            if (httpClientConfigCallback != null) {
+            if (requestConfigCallback != null) {
-                httpClientConfigCallback.customizeDefaultRequestConfig(requestConfigBuilder);
+                requestConfigCallback.customizeRequestConfig(requestConfigBuilder);
             }
             RequestConfig requestConfig = requestConfigBuilder.build();
 
@@ -487,24 +497,30 @@ public final class RestClient implements Closeable {
         }
     }
 
+    /**
+     * Callback used the default {@link RequestConfig} being set to the {@link CloseableHttpClient}
+     * @see HttpClientBuilder#setDefaultRequestConfig
+     */
+    public interface RequestConfigCallback {
+        /**
+         * Allows to customize the {@link RequestConfig} that will be used with each request.
+         * It is common to customize the different timeout values through this method without losing any other useful default
+         * value that the {@link RestClient.Builder} internally sets.
+         */
+        void customizeRequestConfig(RequestConfig.Builder requestConfigBuilder);
+    }
+
     /**
      * Callback used to customize the {@link CloseableHttpClient} instance used by a {@link RestClient} instance.
      * Allows to customize default {@link RequestConfig} being set to the client and any parameter that
      * can be set through {@link HttpClientBuilder}
      */
     public interface HttpClientConfigCallback {
-        /**
-         * Allows to customize the {@link RequestConfig} that will be used with each request.
-         * It is common to customize the different timeout values through this method without losing any other useful default
-         * value that the {@link RestClient.Builder} sets internally.
-         */
-        void customizeDefaultRequestConfig(RequestConfig.Builder requestConfigBuilder);
-
         /**
          * Allows to customize the {@link CloseableHttpClient} being created and used by the {@link RestClient}.
          * It is common to customzie the default {@link org.apache.http.client.CredentialsProvider} through this method,
-         * or the {@link org.apache.http.conn.HttpClientConnectionManager} being used without losing any other useful default
+         * without losing any other useful default value that the {@link RestClient.Builder} internally sets.
-         * value that the {@link RestClient.Builder} sets internally.
+         * Also useful to setup ssl through {@link SSLSocketFactoryHttpConfigCallback}.
          */
         void customizeHttpClient(HttpClientBuilder httpClientBuilder);
     }

client/rest/src/main/java/org/elasticsearch/client/SSLSocketFactoryHttpConfigCallback.java

@@ -0,0 +1,53 @@
+/*
+ * 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;
+
+import org.apache.http.config.Registry;
+import org.apache.http.config.RegistryBuilder;
+import org.apache.http.conn.socket.ConnectionSocketFactory;
+import org.apache.http.conn.socket.PlainConnectionSocketFactory;
+import org.apache.http.conn.ssl.SSLConnectionSocketFactory;
+import org.apache.http.impl.client.HttpClientBuilder;
+import org.apache.http.impl.conn.PoolingHttpClientConnectionManager;
+
+/**
+ * Helps configuring the http client when needing to communicate over ssl. It effectively replaces the connection manager
+ * with one that has ssl properly configured thanks to the provided {@link SSLConnectionSocketFactory}.
+ */
+public class SSLSocketFactoryHttpConfigCallback implements RestClient.HttpClientConfigCallback {
+
+    private final SSLConnectionSocketFactory sslSocketFactory;
+
+    public SSLSocketFactoryHttpConfigCallback(SSLConnectionSocketFactory sslSocketFactory) {
+        this.sslSocketFactory = sslSocketFactory;
+    }
+
+    @Override
+    public void customizeHttpClient(HttpClientBuilder httpClientBuilder) {
+        Registry<ConnectionSocketFactory> socketFactoryRegistry = RegistryBuilder.<ConnectionSocketFactory>create()
+                .register("http", PlainConnectionSocketFactory.getSocketFactory())
+                .register("https", sslSocketFactory).build();
+        PoolingHttpClientConnectionManager connectionManager = new PoolingHttpClientConnectionManager(socketFactoryRegistry);
+        //default settings may be too constraining
+        connectionManager.setDefaultMaxPerRoute(10);
+        connectionManager.setMaxTotal(30);
+        httpClientBuilder.setConnectionManager(connectionManager);
+    }
+}

client/rest/src/test/java/org/elasticsearch/client/RestClientBuilderTests.java

@@ -92,6 +92,13 @@ public class RestClientBuilderTests extends RestClientTestCase {
             assertEquals("httpClientConfigCallback must not be null", e.getMessage());
         }
 
+        try {
+            RestClient.builder(new HttpHost("localhost", 9200)).setRequestConfigCallback(null);
+            fail("should have failed");
+        } catch(NullPointerException e) {
+            assertEquals("requestConfigCallback must not be null", e.getMessage());
+        }
+
         int numNodes = RandomInts.randomIntBetween(getRandom(), 1, 5);
         HttpHost[] hosts = new HttpHost[numNodes];
         for (int i = 0; i < numNodes; i++) {
@@ -100,14 +107,15 @@ public class RestClientBuilderTests extends RestClientTestCase {
         RestClient.Builder builder = RestClient.builder(hosts);
         if (getRandom().nextBoolean()) {
             builder.setHttpClientConfigCallback(new RestClient.HttpClientConfigCallback() {
-                @Override
-                public void customizeDefaultRequestConfig(RequestConfig.Builder requestConfigBuilder) {
-
-                }
-
                 @Override
                 public void customizeHttpClient(HttpClientBuilder httpClientBuilder) {
-
+                }
+            });
+        }
+        if (getRandom().nextBoolean()) {
+            builder.setRequestConfigCallback(new RestClient.RequestConfigCallback() {
+                @Override
+                public void customizeRequestConfig(RequestConfig.Builder requestConfigBuilder) {
                 }
             });
         }

qa/smoke-test-http/src/test/java/org/elasticsearch/http/HttpCompressionIT.java

@@ -22,7 +22,6 @@ import org.apache.http.Header;
 import org.apache.http.HttpException;
 import org.apache.http.HttpHeaders;
 import org.apache.http.HttpResponseInterceptor;
-import org.apache.http.client.config.RequestConfig;
 import org.apache.http.entity.StringEntity;
 import org.apache.http.impl.client.HttpClientBuilder;
 import org.apache.http.message.BasicHeader;
@@ -138,10 +137,6 @@ public class HttpCompressionIT extends ESIntegTestCase {
             this.contentEncodingHeaderExtractor = contentEncodingHeaderExtractor;
         }
 
-        @Override
-        public void customizeDefaultRequestConfig(RequestConfig.Builder requestConfigBuilder) {
-        }
-
         @Override
         public void customizeHttpClient(HttpClientBuilder httpClientBuilder) {
             httpClientBuilder.addInterceptorFirst(contentEncodingHeaderExtractor);

test/framework/src/main/java/org/elasticsearch/test/rest/client/RestTestClient.java

@@ -22,14 +22,8 @@ import com.carrotsearch.randomizedtesting.RandomizedTest;
 import org.apache.http.Header;
 import org.apache.http.HttpEntity;
 import org.apache.http.HttpHost;
-import org.apache.http.client.config.RequestConfig;
-import org.apache.http.config.Registry;
-import org.apache.http.config.RegistryBuilder;
-import org.apache.http.conn.socket.ConnectionSocketFactory;
 import org.apache.http.conn.ssl.SSLConnectionSocketFactory;
 import org.apache.http.entity.StringEntity;
-import org.apache.http.impl.client.HttpClientBuilder;
-import org.apache.http.impl.conn.PoolingHttpClientConnectionManager;
 import org.apache.http.message.BasicHeader;
 import org.apache.http.ssl.SSLContexts;
 import org.apache.lucene.util.IOUtils;
@@ -37,6 +31,7 @@ import org.elasticsearch.Version;
 import org.elasticsearch.client.Response;
 import org.elasticsearch.client.ResponseException;
 import org.elasticsearch.client.RestClient;
+import org.elasticsearch.client.SSLSocketFactoryHttpConfigCallback;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.io.PathUtils;
 import org.elasticsearch.common.logging.ESLogger;
@@ -267,7 +262,15 @@ public class RestTestClient implements Closeable {
     }
 
     private static RestClient createRestClient(URL[] urls, Settings settings) throws IOException {
-        PoolingHttpClientConnectionManager connectionManager;
+        String protocol = settings.get(PROTOCOL, "http");
+        HttpHost[] hosts = new HttpHost[urls.length];
+        for (int i = 0; i < hosts.length; i++) {
+            URL url = urls[i];
+            hosts[i] = new HttpHost(url.getHost(), url.getPort(), protocol);
+        }
+        RestClient.Builder builder = RestClient.builder(hosts).setMaxRetryTimeoutMillis(30000)
+                .setRequestConfigCallback(requestConfigBuilder -> requestConfigBuilder.setSocketTimeout(30000));
+
         String keystorePath = settings.get(TRUSTSTORE_PATH);
         if (keystorePath != null) {
             final String keystorePass = settings.get(TRUSTSTORE_PASSWORD);
@@ -284,38 +287,13 @@ public class RestTestClient implements Closeable {
                     keyStore.load(is, keystorePass.toCharArray());
                 }
                 SSLContext sslcontext = SSLContexts.custom().loadTrustMaterial(keyStore, null).build();
-                Registry<ConnectionSocketFactory> socketFactoryRegistry = RegistryBuilder.<ConnectionSocketFactory>create()
+                SSLConnectionSocketFactory sslConnectionSocketFactory = new SSLConnectionSocketFactory(sslcontext);
-                        .register("https", new SSLConnectionSocketFactory(sslcontext)).build();
+                builder.setHttpClientConfigCallback(new SSLSocketFactoryHttpConfigCallback(sslConnectionSocketFactory));
-                connectionManager = new PoolingHttpClientConnectionManager(socketFactoryRegistry);
             } catch (KeyStoreException|NoSuchAlgorithmException|KeyManagementException|CertificateException e) {
                 throw new RuntimeException(e);
             }
-        } else {
-            connectionManager = new PoolingHttpClientConnectionManager();
-        }
-        //default settings may be too constraining
-        connectionManager.setDefaultMaxPerRoute(10);
-        connectionManager.setMaxTotal(30);
-
-        String protocol = settings.get(PROTOCOL, "http");
-        HttpHost[] hosts = new HttpHost[urls.length];
-        for (int i = 0; i < hosts.length; i++) {
-            URL url = urls[i];
-            hosts[i] = new HttpHost(url.getHost(), url.getPort(), protocol);
         }
 
-        RestClient.Builder builder = RestClient.builder(hosts).setMaxRetryTimeoutMillis(30000)
-                .setHttpClientConfigCallback(new RestClient.HttpClientConfigCallback() {
-            @Override
-            public void customizeDefaultRequestConfig(RequestConfig.Builder requestConfigBuilder) {
-                requestConfigBuilder.setSocketTimeout(30000);
-            }
-
-            @Override
-            public void customizeHttpClient(HttpClientBuilder httpClientBuilder) {
-                httpClientBuilder.setConnectionManager(connectionManager);
-            }
-        });
         try (ThreadContext threadContext = new ThreadContext(settings)) {
             Header[] defaultHeaders = new Header[threadContext.getHeaders().size()];
             int i = 0;