Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: allow user-specified default OkHttpClient instance #174

Merged
merged 1 commit into from
Apr 4, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 20 additions & 2 deletions src/main/java/com/ibm/cloud/sdk/core/http/HttpClientSingleton.java
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,25 @@ protected HttpClientSingleton() {
}

/**
* Configures the HTTP client.
* Returns the current {@link OkHttpClient} instance held by this singleton.
* This is the client instance that is used as a default configuration from which other client instances are built.
* @return the current OkHttpClient instance
*/
public OkHttpClient getHttpClient() {
return this.okHttpClient;
}

/**
* Sets the current {@link OkHttpClient} instance held by this singleton.
* This is the client instance that is used as a default configuration from which other client instances are built.
* @param client the new OkHttpClient instance to use as a default client configuration
*/
public void setHttpClient(OkHttpClient client) {
this.okHttpClient = client;
}

/**
* Configures a new HTTP client instance.
*
* @return the HTTP client
*/
Expand Down Expand Up @@ -290,7 +308,7 @@ private OkHttpClient setLoggingLevel(OkHttpClient client, LoggingLevel loggingLe
*
* @param builder the {@link OkHttpClient} builder.
*/
private void setupTLSProtocol(final OkHttpClient.Builder builder) {
public static void setupTLSProtocol(final OkHttpClient.Builder builder) {
try {
TrustManagerFactory trustManagerFactory =
TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm());
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* (C) Copyright IBM Corp. 2019, 2021.
* (C) Copyright IBM Corp. 2019, 2022.
*
* Licensed 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
Expand Down Expand Up @@ -40,16 +40,20 @@
* <ul>
* <li>disableSSLVerification - a flag that indicates whether or not client-side SSL verification should be disabled.
* <li>headers - a Map of keys/values that will be set as HTTP headers on requests sent to the token service.
* <li>proxy - a java.net.Proxy instance that will be set on the Client object used to interact with the token service.
* <li>proxyAuthenticator - an okhttp3.Authenticator instance to be set on the Client object used to interact wth
* the token service.
* <li>proxy - a java.net.Proxy instance that will be set on the OkHttpClient instance used to
* interact with the token service.
* <li>proxyAuthenticator - an okhttp3.Authenticator instance to be set on the OkHttpClient instance used to
* interact with the token service.
* <li>client - a fully-configured OkHttpClient instance to be used to interact with the token service.
* </ul>
*/
public abstract class TokenRequestBasedAuthenticator<T extends AbstractToken, R extends TokenServerResponse>
extends AuthenticatorBase implements Authenticator {

private static final Logger logger = Logger.getLogger(TokenRequestBasedAuthenticator.class.getName());

protected OkHttpClient client;

// Configuration properties that are common to all subclasses.
private boolean disableSSLVerification;
private Map<String, String> headers;
Expand All @@ -67,6 +71,47 @@ private void setTokenData(T tokenData) {
this.tokenData = tokenData;
}

/**
* Sets the OkHttpClient instance to be used when interacting with the token service.
* @param client the OkHttpClient instance to use
*/
public void setClient(OkHttpClient client) {
this.client = client;
}

/**
* Returns the OkHttpClient instance to be used when interacting with the token service.
* @return the client instance or null if a client insance has not yet been set
*/
public OkHttpClient getClient() {
return this.client;
}

/**
* Returns a properly-configured OkHttpClient instance to use when interacting with the token service.
* This function is different from "getClient()" in that it will configure and save
* a client instance if one has not yet been setup for "this".
* @return a non-null, configured OkHttpClient instance
*/
protected synchronized OkHttpClient getConfiguredClient() {
if (this.client == null) {
OkHttpClient defaultClient = HttpClientSingleton.getInstance().getHttpClient();

HttpConfigOptions.Builder clientOptions = new HttpConfigOptions.Builder()
.disableSslVerification(this.disableSSLVerification)
.proxy(this.proxy)
.proxyAuthenticator(this.proxyAuthenticator);

if (logger.isLoggable(Level.FINE)) {
clientOptions.loggingLevel(LoggingLevel.BODY);
}

this.client = HttpClientSingleton.getInstance().configureClient(defaultClient, clientOptions.build());
}

return this.client;
}

/**
* Validates the configuration properties associated with the Authenticator.
* Each concrete subclass must implement this method.
Expand Down Expand Up @@ -136,7 +181,7 @@ public Proxy getProxy() {

/**
* Sets a Proxy object on this Authenticator.
* @param proxy the proxy object to be associated with the Client used to interact wth the token service.
* @param proxy the proxy object to be associated with the Client used to interact with the token service.
*/
public void setProxy(Proxy proxy) {
this.proxy = proxy;
Expand Down Expand Up @@ -256,18 +301,7 @@ protected R invokeRequest(final RequestBuilder requestBuilder, final Class<? ext
// Allocate the response.
final Object[] responseObj = new Object[1];

// Set up the Client we'll use to invoke the request.
final HttpConfigOptions.Builder clientOptions = new HttpConfigOptions.Builder()
.disableSslVerification(this.disableSSLVerification)
.proxy(this.proxy)
.proxyAuthenticator(this.proxyAuthenticator);

// Enable request/response logging.
if (logger.isLoggable(Level.FINE)) {
clientOptions.loggingLevel(LoggingLevel.BODY);
}

final OkHttpClient client = HttpClientSingleton.getInstance().configureClient(clientOptions.build());
final OkHttpClient client = getConfiguredClient();

final Request request = requestBuilder.build();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import static org.testng.Assert.assertTrue;
import static org.testng.Assert.fail;

import java.util.Arrays;
import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.TimeUnit;
Expand All @@ -38,7 +39,9 @@
import com.ibm.cloud.sdk.core.test.BaseServiceUnitTest;
import com.ibm.cloud.sdk.core.util.Clock;

import okhttp3.ConnectionSpec;
import okhttp3.Headers;
import okhttp3.OkHttpClient;
import okhttp3.Request;
import okhttp3.mockwebserver.RecordedRequest;

Expand Down Expand Up @@ -404,6 +407,19 @@ public void testAuthenticateNewAndCachedToken() throws Throwable {
.url(url)
.build();

// Create a custom client and set it on the authenticator.
ConnectionSpec spec = new ConnectionSpec.Builder(ConnectionSpec.MODERN_TLS)
.allEnabledCipherSuites()
.build();
OkHttpClient client = new OkHttpClient.Builder()
.connectTimeout(30, TimeUnit.SECONDS)
.writeTimeout(120, TimeUnit.SECONDS)
.readTimeout(120, TimeUnit.SECONDS)
.connectionSpecs(Arrays.asList(spec, ConnectionSpec.CLEARTEXT))
.build();
authenticator.setClient(client);
assertEquals(authenticator.getClient(), client);

Request.Builder requestBuilder = new Request.Builder().url("https://test.com");

// Set mock server responses.
Expand All @@ -422,6 +438,9 @@ public void testAuthenticateNewAndCachedToken() throws Throwable {
requestBuilder = new Request.Builder().url("https://test.com");
authenticator.authenticate(requestBuilder);
verifyAuthHeader(requestBuilder, "Bearer " + vpcIamAccessTokenResponse1.getAccessToken());

// Verify that the authenticator is still using the same client instance that we set before.
assertEquals(authenticator.getClient(), client);
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,16 @@
package com.ibm.cloud.sdk.core.test.http;

import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertNotEquals;
import static org.testng.Assert.assertNotNull;
import static org.testng.Assert.assertTrue;

import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Iterator;
import java.util.List;
import java.util.concurrent.TimeUnit;

import javax.net.ssl.SSLSocket;

Expand Down Expand Up @@ -126,4 +129,45 @@ public void testSetRetryStrategy() {
assertEquals(testRetryInterceptors, 1);
assertEquals(otherRetryInterceptors, 0);
}

@Test
public void testSetClient() {

// Create a custom client and set it on the HttpClientSingleton
// as the default for configuring other clients.
ConnectionSpec spec = new ConnectionSpec.Builder(ConnectionSpec.MODERN_TLS)
.allEnabledCipherSuites()
.build();
OkHttpClient client = new OkHttpClient.Builder()
.connectTimeout(30, TimeUnit.SECONDS)
.writeTimeout(120, TimeUnit.SECONDS)
.readTimeout(120, TimeUnit.SECONDS)
.connectionSpecs(Arrays.asList(spec, ConnectionSpec.CLEARTEXT))
.build();
HttpClientSingleton cs = HttpClientSingleton.getInstance();
assertNotNull(cs.getHttpClient());

// Verify set/get.
cs.setHttpClient(client);
assertEquals(cs.getHttpClient(), client);

// Verify that "client" is used to configure other clients.
// To verify this, we'll just configure a new client with SSL verification disabled,
// and its timeout properties should be the same as what we set above on our original client.
HttpConfigOptions options = new HttpConfigOptions.Builder()
.disableSslVerification(true)
.build();
OkHttpClient client2 = cs.configureClient(options);

// Verify that the "configureClient()" call above set a new default client instance in the singleton.
assertNotEquals(cs.getHttpClient(), client);

// Verify that the new client is a different instance than our original client.
assertNotEquals(client2, client);

// Verify that the new client "inherits" the config from our original client.
assertEquals(client2.connectTimeoutMillis(), 30 * 1000);
assertEquals(client2.writeTimeoutMillis(), 120 * 1000);
assertEquals(client2.readTimeoutMillis(), 120 * 1000);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import static org.testng.Assert.fail;

import java.nio.file.NoSuchFileException;
import java.util.Arrays;
import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.TimeUnit;
Expand All @@ -45,7 +46,9 @@
import com.ibm.cloud.sdk.core.test.BaseServiceUnitTest;
import com.ibm.cloud.sdk.core.util.Clock;

import okhttp3.ConnectionSpec;
import okhttp3.Headers;
import okhttp3.OkHttpClient;
import okhttp3.Request;
import okhttp3.mockwebserver.RecordedRequest;

Expand Down Expand Up @@ -249,6 +252,19 @@ public void testAuthenticateNewAndStoredToken() throws Throwable {
.url(url)
.build();

// Create a custom client and set it on the authenticator.
ConnectionSpec spec = new ConnectionSpec.Builder(ConnectionSpec.MODERN_TLS)
.allEnabledCipherSuites()
.build();
OkHttpClient client = new OkHttpClient.Builder()
.connectTimeout(30, TimeUnit.SECONDS)
.writeTimeout(120, TimeUnit.SECONDS)
.readTimeout(120, TimeUnit.SECONDS)
.connectionSpecs(Arrays.asList(spec, ConnectionSpec.CLEARTEXT))
.build();
authenticator.setClient(client);
assertEquals(authenticator.getClient(), client);

Request.Builder requestBuilder = new Request.Builder().url("https://test.com");

// Set mock server response.
Expand Down Expand Up @@ -280,6 +296,9 @@ public void testAuthenticateNewAndStoredToken() throws Throwable {
requestBuilder = new Request.Builder().url("https://test.com");
authenticator.authenticate(requestBuilder);
verifyAuthHeader(requestBuilder, "Bearer " + tokenData1.getAccessToken());

// Verify that the authenticator is still using the same client instance that we set before.
assertEquals(authenticator.getClient(), client);
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import static org.testng.Assert.assertTrue;
import static org.testng.Assert.fail;

import java.util.Arrays;
import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.TimeUnit;
Expand All @@ -39,7 +40,9 @@
import com.ibm.cloud.sdk.core.test.BaseServiceUnitTest;
import com.ibm.cloud.sdk.core.util.Clock;

import okhttp3.ConnectionSpec;
import okhttp3.Headers;
import okhttp3.OkHttpClient;
import okhttp3.Request;
import okhttp3.mockwebserver.RecordedRequest;

Expand Down Expand Up @@ -312,17 +315,32 @@ public void testAuthenticateNewAndStoredToken() {
.password(testPassword)
.disableSSLVerification(true)
.build();
Request.Builder requestBuilder;

// Create a custom client and set it on the authenticator.
ConnectionSpec spec = new ConnectionSpec.Builder(ConnectionSpec.MODERN_TLS)
.allEnabledCipherSuites()
.build();
OkHttpClient client = new OkHttpClient.Builder()
.connectTimeout(30, TimeUnit.SECONDS)
.writeTimeout(120, TimeUnit.SECONDS)
.readTimeout(120, TimeUnit.SECONDS)
.connectionSpecs(Arrays.asList(spec, ConnectionSpec.CLEARTEXT))
.build();
authenticator.setClient(client);
assertEquals(authenticator.getClient(), client);

// Authenticator should request new, valid token.
requestBuilder = new Request.Builder().url("https://test.com");
Request.Builder requestBuilder = new Request.Builder().url("https://test.com");
authenticator.authenticate(requestBuilder);
verifyAuthHeader(requestBuilder, "Bearer " + tokenData.getToken());

// Authenticator should just return the same token this time since we have a valid one stored.
requestBuilder = new Request.Builder().url("https://test.com");
authenticator.authenticate(requestBuilder);
verifyAuthHeader(requestBuilder, "Bearer " + tokenData.getToken());

// Verify that the authenticator is still using the same client instance that we set before.
assertEquals(authenticator.getClient(), client);
}

@Test
Expand Down
Loading