From e4f24472e880dc06649bf98b8fd573ecd52f1d4d Mon Sep 17 00:00:00 2001 From: Evgeny Sabirov Date: Sat, 9 Nov 2019 02:21:54 +0300 Subject: [PATCH] Redesign OkHttpClientFactory: - 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 --- .../client/api/client/BaseApiClient.java | 4 +- .../client/factory/OkHttpClientFactory.java | 82 ++++++++++++------- .../factory/OkHttpClientFactoryTest.java | 4 +- 3 files changed, 55 insertions(+), 35 deletions(-) diff --git a/sdk/java-v2/src/main/java/org/arvados/client/api/client/BaseApiClient.java b/sdk/java-v2/src/main/java/org/arvados/client/api/client/BaseApiClient.java index 7e8a2979be..a8d1a08cb0 100644 --- a/sdk/java-v2/src/main/java/org/arvados/client/api/client/BaseApiClient.java +++ b/sdk/java-v2/src/main/java/org/arvados/client/api/client/BaseApiClient.java @@ -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() { diff --git a/sdk/java-v2/src/main/java/org/arvados/client/api/client/factory/OkHttpClientFactory.java b/sdk/java-v2/src/main/java/org/arvados/client/api/client/factory/OkHttpClientFactory.java index 0e95e661e7..f9041c9281 100644 --- a/sdk/java-v2/src/main/java/org/arvados/client/api/client/factory/OkHttpClientFactory.java +++ b/sdk/java-v2/src/main/java/org/arvados/client/api/client/factory/OkHttpClientFactory.java @@ -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 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()"; - } - } } diff --git a/sdk/java-v2/src/test/java/org/arvados/client/api/client/factory/OkHttpClientFactoryTest.java b/sdk/java-v2/src/test/java/org/arvados/client/api/client/factory/OkHttpClientFactoryTest.java index f7e1813294..f485d3bb02 100644 --- a/sdk/java-v2/src/test/java/org/arvados/client/api/client/factory/OkHttpClientFactoryTest.java +++ b/sdk/java-v2/src/test/java/org/arvados/client/api/client/factory/OkHttpClientFactoryTest.java @@ -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); -- 2.30.2