Redesign OkHttpClientFactory:
authorEvgeny Sabirov <evgeny.sabirov@genestack.com>
Fri, 8 Nov 2019 23:21:54 +0000 (02:21 +0300)
committerEvgeny Sabirov <evgeny.sabirov@genestack.com>
Fri, 8 Nov 2019 23:21:54 +0000 (02:21 +0300)
- Singleton factory implementation
- Predefined default OkHttpClient instances
- Build OkHttpClient instances with shared factories/connection pools to optimize resource usage

Arvados-DCO-1.1-Signed-off-by: Evgeny Sabirov <evgeny.sabirov@genestack.com>

sdk/java-v2/src/main/java/org/arvados/client/api/client/BaseApiClient.java
sdk/java-v2/src/main/java/org/arvados/client/api/client/factory/OkHttpClientFactory.java
sdk/java-v2/src/test/java/org/arvados/client/api/client/factory/OkHttpClientFactoryTest.java

index 7e8a2979befaee7af09007bd51e3d0fbc878d312..a8d1a08cb09643262bf657498aefc53b727f168c 100644 (file)
@@ -34,9 +34,7 @@ abstract class BaseApiClient {
 
     BaseApiClient(ConfigProvider config) {
         this.config = config;
-        client = OkHttpClientFactory.builder()
-                .build()
-                .create(config.isApiHostInsecure());
+        this.client = OkHttpClientFactory.INSTANCE.create(config.isApiHostInsecure());
     }
 
     Request.Builder getRequestBuilder() {
index 0e95e661e7fccd1b24f433e2ace298effa9064c1..f9041c9281a0f4026e6136741dba7da04901fc05 100644 (file)
@@ -7,6 +7,7 @@
 
 package org.arvados.client.api.client.factory;
 
+import com.google.common.base.Suppliers;
 import okhttp3.OkHttpClient;
 import org.arvados.client.exception.ArvadosClientException;
 import org.slf4j.Logger;
@@ -19,31 +20,60 @@ import java.security.KeyManagementException;
 import java.security.NoSuchAlgorithmException;
 import java.security.SecureRandom;
 import java.security.cert.X509Certificate;
+import java.util.function.Supplier;
 
-public class OkHttpClientFactory {
-
+/**
+ * {@link OkHttpClient} instance factory that builds and configures client instances sharing
+ * the common resource pool: this is the recommended approach to optimize resource usage.
+ */
+public final class OkHttpClientFactory {
+    public static final OkHttpClientFactory INSTANCE = new OkHttpClientFactory();
     private final Logger log = org.slf4j.LoggerFactory.getLogger(OkHttpClientFactory.class);
+    private final OkHttpClient clientSecure = new OkHttpClient();
+    private final Supplier<OkHttpClient> clientUnsecure =
+            Suppliers.memoize(this::getDefaultClientAcceptingAllCertificates);
+
+    private OkHttpClientFactory() { /* singleton */}
 
-    OkHttpClientFactory() {
+    public OkHttpClient create(boolean apiHostInsecure) {
+        return apiHostInsecure ? getDefaultUnsecureClient() : getDefaultClient();
     }
 
-    public static OkHttpClientFactoryBuilder builder() {
-        return new OkHttpClientFactoryBuilder();
+    /**
+     * @return default secure {@link OkHttpClient} with shared resource pool.
+     */
+    public OkHttpClient getDefaultClient() {
+        return clientSecure;
     }
 
-    public OkHttpClient create(boolean apiHostInsecure) {
-        OkHttpClient.Builder builder = new OkHttpClient.Builder();
-        if (apiHostInsecure) {
-            trustAllCertificates(builder);
-        }
-        return builder.build();
+    /**
+     * @return default {@link OkHttpClient} with shared resource pool
+     * that will accept all SSL certificates by default.
+     */
+    public OkHttpClient getDefaultUnsecureClient() {
+        return clientUnsecure.get();
+    }
+
+    /**
+     * @return default {@link OkHttpClient.Builder} with shared resource pool.
+     */
+    public OkHttpClient.Builder getDefaultClientBuilder() {
+        return clientSecure.newBuilder();
+    }
+
+    /**
+     * @return default {@link OkHttpClient.Builder} with shared resource pool
+     * that is preconfigured to accept all SSL certificates.
+     */
+    public OkHttpClient.Builder getDefaultUnsecureClientBuilder() {
+        return clientUnsecure.get().newBuilder();
     }
 
-    private void trustAllCertificates(OkHttpClient.Builder builder) {
+    private OkHttpClient getDefaultClientAcceptingAllCertificates() {
         log.warn("Creating unsafe OkHttpClient. All SSL certificates will be accepted.");
         try {
             // Create a trust manager that does not validate certificate chains
-            final TrustManager[] trustAllCerts = new TrustManager[] { createX509TrustManager() };
+            final TrustManager[] trustAllCerts = {createX509TrustManager()};
 
             // Install the all-trusting trust manager
             SSLContext sslContext = SSLContext.getInstance("SSL");
@@ -51,8 +81,11 @@ public class OkHttpClientFactory {
             // Create an ssl socket factory with our all-trusting manager
             final SSLSocketFactory sslSocketFactory = sslContext.getSocketFactory();
 
+            // Create the OkHttpClient.Builder with shared resource pool
+            final OkHttpClient.Builder builder = clientSecure.newBuilder();
             builder.sslSocketFactory(sslSocketFactory, (X509TrustManager) trustAllCerts[0]);
             builder.hostnameVerifier((hostname, session) -> true);
+            return builder.build();
         } catch (NoSuchAlgorithmException | KeyManagementException e) {
             throw new ArvadosClientException("Error establishing SSL context", e);
         }
@@ -60,30 +93,19 @@ public class OkHttpClientFactory {
 
     private static X509TrustManager createX509TrustManager() {
         return new X509TrustManager() {
-            
+
             @Override
-            public void checkClientTrusted(X509Certificate[] chain, String authType) {}
+            public void checkClientTrusted(X509Certificate[] chain, String authType) {
+            }
 
             @Override
-            public void checkServerTrusted(X509Certificate[] chain, String authType) {}
+            public void checkServerTrusted(X509Certificate[] chain, String authType) {
+            }
 
             @Override
             public X509Certificate[] getAcceptedIssuers() {
-                return new X509Certificate[] {};
+                return new X509Certificate[]{};
             }
         };
     }
-
-    public static class OkHttpClientFactoryBuilder {
-        OkHttpClientFactoryBuilder() {
-        }
-
-        public OkHttpClientFactory build() {
-            return new OkHttpClientFactory();
-        }
-
-        public String toString() {
-            return "OkHttpClientFactory.OkHttpClientFactoryBuilder()";
-        }
-    }
 }
index f7e18132941715a4340684e23183054730f3646c..f485d3bb02aff3e3faca87c77ef8af1f42c5e06d 100644 (file)
@@ -32,7 +32,7 @@ public class OkHttpClientFactoryTest extends ArvadosClientMockedWebServerTest {
     public void secureOkHttpClientIsCreated() throws Exception {
 
         // given
-        OkHttpClientFactory factory = OkHttpClientFactory.builder().build();
+        OkHttpClientFactory factory = OkHttpClientFactory.INSTANCE;
         // * configure HTTPS server
         SSLSocketFactory sf = getSSLSocketFactoryWithSelfSignedCertificate();
         server.useHttps(sf, false);
@@ -50,7 +50,7 @@ public class OkHttpClientFactoryTest extends ArvadosClientMockedWebServerTest {
     @Test
     public void insecureOkHttpClientIsCreated() throws Exception {
         // given
-        OkHttpClientFactory factory = OkHttpClientFactory.builder().build();
+        OkHttpClientFactory factory = OkHttpClientFactory.INSTANCE;
         // * configure HTTPS server
         SSLSocketFactory sf = getSSLSocketFactoryWithSelfSignedCertificate();
         server.useHttps(sf, false);