From 62c8584e800f241a3515b57bb97fe7537f629b08 Mon Sep 17 00:00:00 2001 From: Crim Date: Wed, 11 Aug 2021 17:39:06 +0900 Subject: [PATCH 1/4] First pass adding config hooks interface --- CHANGELOG.md | 4 + .../rest/DefaultHttpClientConfigHooks.java | 7 ++ .../apiclient/rest/HttpClientConfigHooks.java | 116 ++++++++++++++++++ .../apiclient/rest/HttpClientRestClient.java | 36 ++++-- 4 files changed, 155 insertions(+), 8 deletions(-) create mode 100644 src/main/java/org/sourcelab/kafka/connect/apiclient/rest/DefaultHttpClientConfigHooks.java create mode 100644 src/main/java/org/sourcelab/kafka/connect/apiclient/rest/HttpClientConfigHooks.java diff --git a/CHANGELOG.md b/CHANGELOG.md index ab5bc6f..f08c48f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) and this project adheres to [Semantic Versioning](http://semver.org/). +## 3.1.3 (08/11/2021) +- [Issue-55](https://github.com/SourceLabOrg/kafka-connect-client/issues/55) Create new HttpContext for every request. +- + ## 3.1.2 (07/21/2021) - [Issue-54](https://github.com/SourceLabOrg/kafka-connect-client/issues/54) Resolution for issue-54 diff --git a/src/main/java/org/sourcelab/kafka/connect/apiclient/rest/DefaultHttpClientConfigHooks.java b/src/main/java/org/sourcelab/kafka/connect/apiclient/rest/DefaultHttpClientConfigHooks.java new file mode 100644 index 0000000..c137f6c --- /dev/null +++ b/src/main/java/org/sourcelab/kafka/connect/apiclient/rest/DefaultHttpClientConfigHooks.java @@ -0,0 +1,7 @@ +package org.sourcelab.kafka.connect.apiclient.rest; + +/** + * Default implementation makes no modifications. + */ +public class DefaultHttpClientConfigHooks implements HttpClientConfigHooks { +} diff --git a/src/main/java/org/sourcelab/kafka/connect/apiclient/rest/HttpClientConfigHooks.java b/src/main/java/org/sourcelab/kafka/connect/apiclient/rest/HttpClientConfigHooks.java new file mode 100644 index 0000000..a5c0100 --- /dev/null +++ b/src/main/java/org/sourcelab/kafka/connect/apiclient/rest/HttpClientConfigHooks.java @@ -0,0 +1,116 @@ +package org.sourcelab.kafka.connect.apiclient.rest; + +import org.apache.http.client.AuthCache; +import org.apache.http.client.CredentialsProvider; +import org.apache.http.client.config.RequestConfig; +import org.apache.http.client.protocol.HttpClientContext; +import org.apache.http.impl.client.BasicAuthCache; +import org.apache.http.impl.client.BasicCredentialsProvider; +import org.apache.http.impl.client.HttpClientBuilder; +import org.sourcelab.kafka.connect.apiclient.Configuration; + +/** + * HttpClient configuration hooks. + * + * Provides an interface for modifying how the underlying HttpClient instance is created. + */ +public interface HttpClientConfigHooks { + /** + * Create HttpClientBuilder instance. + * @param configuration KafkaConnectClient configuration. + * @return HttpClientBuilder instance. + */ + default HttpClientBuilder createHttpClientBuilder(final Configuration configuration) { + return HttpClientBuilder.create(); + } + + /** + * Create HttpsContextBuilder instance. + * @param configuration KafkaConnectClient configuration. + * @return HttpsContextBuilder instance. + */ + default HttpsContextBuilder createHttpsContextBuilder(final Configuration configuration) { + return new HttpsContextBuilder(configuration); + } + + /** + * Create RequestConfig.Builder instance. + * @param configuration KafkaConnectClient configuration. + * @return RequestConfig.Builder instance. + */ + default RequestConfig.Builder createRequestConfigBuilder(final Configuration configuration) { + return RequestConfig.custom(); + } + + /** + * Create AuthCache instance. + * @param configuration KafkaConnectClient configuration. + * @return AuthCache instance. + */ + default AuthCache createAuthCache(final Configuration configuration) { + return new BasicAuthCache(); + } + + /** + * Create CredentialsProvider instance. + * @param configuration KafkaConnectClient configuration. + * @return CredentialsProvider instance. + */ + default CredentialsProvider createCredentialsProvider(final Configuration configuration) { + return new BasicCredentialsProvider(); + } + + /** + * Create HttpClientContext instance. + * @param configuration KafkaConnectClient configuration. + * @return HttpClientContext instance. + */ + default HttpClientContext createHttpClientContext(final Configuration configuration) { + return HttpClientContext.create(); + } + + /** + * Ability to modify or replace the AuthCache instance after initial configuration has been performed on it. + * @param configuration KafkaConnectClient configuration. + * @return AuthCache instance. + */ + default AuthCache modifyAuthCache(final Configuration configuration, final AuthCache authCache) { + return authCache; + } + + /** + * Ability to modify or replace the CredentialsProvider instance after initial configuration has been performed on it. + * @param configuration KafkaConnectClient configuration. + * @return CredentialsProvider instance. + */ + default CredentialsProvider modifyCredentialsProvider(final Configuration configuration, final CredentialsProvider credentialsProvider) { + return credentialsProvider; + } + + /** + * Ability to modify or replace the RequestConfig.Builder instance after initial configuration has been performed on it. + * @param configuration KafkaConnectClient configuration. + * @return RequestConfig.Builder instance. + */ + default RequestConfig.Builder modifyRequestConfig(final Configuration configuration, final RequestConfig.Builder builder) { + return builder; + } + + /** + * Ability to modify or replace the HttpClientBuilder instance after initial configuration has been performed on it. + * @param configuration KafkaConnectClient configuration. + * @return HttpClientBuilder instance. + */ + default HttpClientBuilder modifyHttpClientBuilder(final Configuration configuration, final HttpClientBuilder builder) { + return builder; + } + + /** + * Ability to modify or replace the HttpClientContext instance after initial configuration has been performed on it. + * @param configuration KafkaConnectClient configuration. + * @return HttpClientContext instance. + */ + default HttpClientContext modifyHttpClientContext(final Configuration configuration, final HttpClientContext context) { + return context; + } +} diff --git a/src/main/java/org/sourcelab/kafka/connect/apiclient/rest/HttpClientRestClient.java b/src/main/java/org/sourcelab/kafka/connect/apiclient/rest/HttpClientRestClient.java index 7ddbbdc..e84c8f9 100644 --- a/src/main/java/org/sourcelab/kafka/connect/apiclient/rest/HttpClientRestClient.java +++ b/src/main/java/org/sourcelab/kafka/connect/apiclient/rest/HttpClientRestClient.java @@ -96,10 +96,24 @@ public class HttpClientRestClient implements RestClient { */ private CredentialsProvider credsProvider; + /** + * Provides an interface for modifying how the underlying HttpClient instance is created. + */ + private final HttpClientConfigHooks configHooks; + /** * Constructor. */ public HttpClientRestClient() { + this(new DefaultHttpClientConfigHooks()); + } + + /** + * Constructor allowing for injecting configuration hooks. + * @param configHooks For hooking/overriding into how the underlying HttpClient is configured. + */ + public HttpClientRestClient(final HttpClientConfigHooks configHooks) { + this.configHooks = configHooks; } /** @@ -113,10 +127,10 @@ public void init(final Configuration configuration) { this.configuration = configuration; // Create https context builder utility. - final HttpsContextBuilder httpsContextBuilder = new HttpsContextBuilder(configuration); + final HttpsContextBuilder httpsContextBuilder = configHooks.createHttpsContextBuilder(configuration); - // Setup client builder - final HttpClientBuilder clientBuilder = createHttpClientBuilder(); + // Create and setup client builder + HttpClientBuilder clientBuilder = configHooks.createHttpClientBuilder(configuration); clientBuilder // Define timeout .setConnectionTimeToLive(configuration.getConnectionTimeToLiveInSeconds(), TimeUnit.SECONDS) @@ -125,15 +139,15 @@ public void init(final Configuration configuration) { .setSSLSocketFactory(httpsContextBuilder.createSslSocketFactory()); // Define our RequestConfigBuilder - final RequestConfig.Builder requestConfigBuilder = RequestConfig.custom(); + RequestConfig.Builder requestConfigBuilder = configHooks.createRequestConfigBuilder(configuration); requestConfigBuilder.setConnectTimeout(configuration.getRequestTimeoutInSeconds() * 1_000); // Define our Credentials Provider - credsProvider = new BasicCredentialsProvider(); + credsProvider = configHooks.createCredentialsProvider(configuration); // Define our auth cache - authCache = new BasicAuthCache(); + authCache = configHooks.createAuthCache(configuration); // If we have a configured proxy host if (configuration.getProxyHost() != null) { @@ -186,6 +200,11 @@ public void init(final Configuration configuration) { } } + // Call Modify hooks + authCache = configHooks.modifyAuthCache(configuration, authCache); + credsProvider = configHooks.modifyCredentialsProvider(configuration, credsProvider); + requestConfigBuilder = configHooks.modifyRequestConfig(configuration, requestConfigBuilder); + // Attach Credentials provider to client builder. clientBuilder.setDefaultCredentialsProvider(credsProvider); @@ -193,6 +212,7 @@ public void init(final Configuration configuration) { clientBuilder.setDefaultRequestConfig(requestConfigBuilder.build()); // build http client + clientBuilder = configHooks.modifyHttpClientBuilder(configuration, clientBuilder); httpClient = clientBuilder.build(); } @@ -407,12 +427,12 @@ private String constructApiUrl(final String endPoint) { */ private HttpClientContext createHttpClientContext() { // Define our context - final HttpClientContext httpClientContext = HttpClientContext.create(); + final HttpClientContext httpClientContext = configHooks.createHttpClientContext(configuration); // Configure context. httpClientContext.setAuthCache(authCache); httpClientContext.setCredentialsProvider(credsProvider); - return httpClientContext; + return configHooks.modifyHttpClientContext(configuration, httpClientContext); } } From 17b3ab383647bca1657c598a083b3175d51834e7 Mon Sep 17 00:00:00 2001 From: Crim Date: Wed, 11 Aug 2021 18:17:11 +0900 Subject: [PATCH 2/4] cleanup, add test coverage --- CHANGELOG.md | 2 +- .../rest/DefaultHttpClientConfigHooks.java | 17 ++ .../apiclient/rest/HttpClientConfigHooks.java | 29 +++ .../apiclient/rest/HttpClientRestClient.java | 41 +++- .../DefaultHttpClientConfigHooksTest.java | 211 ++++++++++++++++++ .../rest/HttpClientRestClientTest.java | 170 ++++++++------ 6 files changed, 396 insertions(+), 74 deletions(-) create mode 100644 src/test/java/org/sourcelab/kafka/connect/apiclient/rest/DefaultHttpClientConfigHooksTest.java diff --git a/CHANGELOG.md b/CHANGELOG.md index f08c48f..aad5019 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## 3.1.3 (08/11/2021) - [Issue-55](https://github.com/SourceLabOrg/kafka-connect-client/issues/55) Create new HttpContext for every request. -- +- [] ## 3.1.2 (07/21/2021) diff --git a/src/main/java/org/sourcelab/kafka/connect/apiclient/rest/DefaultHttpClientConfigHooks.java b/src/main/java/org/sourcelab/kafka/connect/apiclient/rest/DefaultHttpClientConfigHooks.java index c137f6c..b34189f 100644 --- a/src/main/java/org/sourcelab/kafka/connect/apiclient/rest/DefaultHttpClientConfigHooks.java +++ b/src/main/java/org/sourcelab/kafka/connect/apiclient/rest/DefaultHttpClientConfigHooks.java @@ -1,3 +1,20 @@ +/** + * Copyright 2018, 2019 SourceLab.org https://github.com/SourceLabOrg/kafka-connect-client + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated + * documentation files (the "Software"), to deal in the Software without restriction, including without limitation the + * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to permit + * persons to whom the Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE + * WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR + * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ + package org.sourcelab.kafka.connect.apiclient.rest; /** diff --git a/src/main/java/org/sourcelab/kafka/connect/apiclient/rest/HttpClientConfigHooks.java b/src/main/java/org/sourcelab/kafka/connect/apiclient/rest/HttpClientConfigHooks.java index a5c0100..5e931ac 100644 --- a/src/main/java/org/sourcelab/kafka/connect/apiclient/rest/HttpClientConfigHooks.java +++ b/src/main/java/org/sourcelab/kafka/connect/apiclient/rest/HttpClientConfigHooks.java @@ -1,3 +1,20 @@ +/** + * Copyright 2018, 2019 SourceLab.org https://github.com/SourceLabOrg/kafka-connect-client + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated + * documentation files (the "Software"), to deal in the Software without restriction, including without limitation the + * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to permit + * persons to whom the Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE + * WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR + * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ + package org.sourcelab.kafka.connect.apiclient.rest; import org.apache.http.client.AuthCache; @@ -13,6 +30,18 @@ * HttpClient configuration hooks. * * Provides an interface for modifying how the underlying HttpClient instance is created. + * + * Usage of this would look like: + * + * final RestClient restClient = new HttpClientRestClient(new HttpClientConfigHooks { + * // Override methods as needed to modify behavior. + * }); + * + * // Create client, passing configuration and RestClient implementation + * final KafkaConnectClient client = new KafkaConnectClient(configuration, restClient); + * + * // Use client as normal... + * */ public interface HttpClientConfigHooks { /** diff --git a/src/main/java/org/sourcelab/kafka/connect/apiclient/rest/HttpClientRestClient.java b/src/main/java/org/sourcelab/kafka/connect/apiclient/rest/HttpClientRestClient.java index e84c8f9..c5270bf 100644 --- a/src/main/java/org/sourcelab/kafka/connect/apiclient/rest/HttpClientRestClient.java +++ b/src/main/java/org/sourcelab/kafka/connect/apiclient/rest/HttpClientRestClient.java @@ -60,6 +60,7 @@ import java.util.Collection; import java.util.Collections; import java.util.Map; +import java.util.Objects; import java.util.concurrent.TimeUnit; /** @@ -130,7 +131,10 @@ public void init(final Configuration configuration) { final HttpsContextBuilder httpsContextBuilder = configHooks.createHttpsContextBuilder(configuration); // Create and setup client builder - HttpClientBuilder clientBuilder = configHooks.createHttpClientBuilder(configuration); + HttpClientBuilder clientBuilder = Objects.requireNonNull( + configHooks.createHttpClientBuilder(configuration), + "HttpClientConfigHook::createHttpClientBuilder() must return non-null instance." + ); clientBuilder // Define timeout .setConnectionTimeToLive(configuration.getConnectionTimeToLiveInSeconds(), TimeUnit.SECONDS) @@ -139,15 +143,24 @@ public void init(final Configuration configuration) { .setSSLSocketFactory(httpsContextBuilder.createSslSocketFactory()); // Define our RequestConfigBuilder - RequestConfig.Builder requestConfigBuilder = configHooks.createRequestConfigBuilder(configuration); + RequestConfig.Builder requestConfigBuilder = Objects.requireNonNull( + configHooks.createRequestConfigBuilder(configuration), + "HttpClientConfigHook::createRequestConfigBuilder() must return non-null instance." + ); requestConfigBuilder.setConnectTimeout(configuration.getRequestTimeoutInSeconds() * 1_000); // Define our Credentials Provider - credsProvider = configHooks.createCredentialsProvider(configuration); + credsProvider = Objects.requireNonNull( + configHooks.createCredentialsProvider(configuration), + "HttpClientConfigHook::createCredentialsProvider() must return non-null instance." + ); // Define our auth cache - authCache = configHooks.createAuthCache(configuration); + authCache = Objects.requireNonNull( + configHooks.createAuthCache(configuration), + "HttpClientConfigHook::createAuthCache() must return non-null instance." + ); // If we have a configured proxy host if (configuration.getProxyHost() != null) { @@ -201,9 +214,18 @@ public void init(final Configuration configuration) { } // Call Modify hooks - authCache = configHooks.modifyAuthCache(configuration, authCache); - credsProvider = configHooks.modifyCredentialsProvider(configuration, credsProvider); - requestConfigBuilder = configHooks.modifyRequestConfig(configuration, requestConfigBuilder); + authCache = Objects.requireNonNull( + configHooks.modifyAuthCache(configuration, authCache), + "HttpClientConfigHook::modifyAuthCache() must return non-null instance." + ); + credsProvider = Objects.requireNonNull( + configHooks.modifyCredentialsProvider(configuration, credsProvider), + "HttpClientConfigHook::modifyCredentialsProvider() must return non-null instance." + ); + requestConfigBuilder = Objects.requireNonNull( + configHooks.modifyRequestConfig(configuration, requestConfigBuilder), + "HttpClientConfigHook::modifyRequestConfig() must return non-null instance." + ); // Attach Credentials provider to client builder. clientBuilder.setDefaultCredentialsProvider(credsProvider); @@ -212,7 +234,10 @@ public void init(final Configuration configuration) { clientBuilder.setDefaultRequestConfig(requestConfigBuilder.build()); // build http client - clientBuilder = configHooks.modifyHttpClientBuilder(configuration, clientBuilder); + clientBuilder = Objects.requireNonNull( + configHooks.modifyHttpClientBuilder(configuration, clientBuilder), + "HttpClientConfigHook::modifyHttpClientBuilder() must return non-null instance." + ); httpClient = clientBuilder.build(); } diff --git a/src/test/java/org/sourcelab/kafka/connect/apiclient/rest/DefaultHttpClientConfigHooksTest.java b/src/test/java/org/sourcelab/kafka/connect/apiclient/rest/DefaultHttpClientConfigHooksTest.java new file mode 100644 index 0000000..ab616f9 --- /dev/null +++ b/src/test/java/org/sourcelab/kafka/connect/apiclient/rest/DefaultHttpClientConfigHooksTest.java @@ -0,0 +1,211 @@ +/** + * Copyright 2018, 2019 SourceLab.org https://github.com/SourceLabOrg/kafka-connect-client + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated + * documentation files (the "Software"), to deal in the Software without restriction, including without limitation the + * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to permit + * persons to whom the Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE + * WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR + * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ + +package org.sourcelab.kafka.connect.apiclient.rest; + +import org.apache.http.client.AuthCache; +import org.apache.http.client.CredentialsProvider; +import org.apache.http.client.config.RequestConfig; +import org.apache.http.client.protocol.HttpClientContext; +import org.apache.http.impl.client.BasicAuthCache; +import org.apache.http.impl.client.BasicCredentialsProvider; +import org.apache.http.impl.client.HttpClientBuilder; +import org.junit.Test; +import org.sourcelab.kafka.connect.apiclient.Configuration; + +import static org.junit.Assert.assertNotSame; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verifyZeroInteractions; + +/** + * Verifying behavior of DefaultHttpClientConfigHooks. + */ +public class DefaultHttpClientConfigHooksTest { + + private final Configuration configuration = new Configuration("https://dummy.host"); + private final DefaultHttpClientConfigHooks configHooks = new DefaultHttpClientConfigHooks(); + + /** + * Test that the default behavior to always create a new instance. + */ + @Test + public void createHttpClientBuilder_alwaysCreatesNewInstance() { + final HttpClientContext instance1 = configHooks.createHttpClientContext(configuration); + final HttpClientContext instance2 = configHooks.createHttpClientContext(configuration); + final HttpClientContext instance3 = configHooks.createHttpClientContext(configuration); + + assertNotSame("Instances should be different", instance1, instance2); + assertNotSame("Instances should be different", instance1, instance3); + assertNotSame("Instances should be different", instance2, instance3); + } + + /** + * Test that the default behavior to always create a new instance. + */ + @Test + public void createHttpsContextBuilder_alwaysCreatesNewInstance() { + final HttpsContextBuilder instance1 = configHooks.createHttpsContextBuilder(configuration); + final HttpsContextBuilder instance2 = configHooks.createHttpsContextBuilder(configuration); + final HttpsContextBuilder instance3 = configHooks.createHttpsContextBuilder(configuration); + + assertNotSame("Instances should be different", instance1, instance2); + assertNotSame("Instances should be different", instance1, instance3); + assertNotSame("Instances should be different", instance2, instance3); + } + + /** + * Test that the default behavior to always create a new instance. + */ + @Test + public void createRequestConfigBuilder_alwaysCreatesNewInstance() { + final RequestConfig.Builder instance1 = configHooks.createRequestConfigBuilder(configuration); + final RequestConfig.Builder instance2 = configHooks.createRequestConfigBuilder(configuration); + final RequestConfig.Builder instance3 = configHooks.createRequestConfigBuilder(configuration); + + assertNotSame("Instances should be different", instance1, instance2); + assertNotSame("Instances should be different", instance1, instance3); + assertNotSame("Instances should be different", instance2, instance3); + } + + /** + * Test that the default behavior to always create a new instance. + */ + @Test + public void createAuthCache_alwaysCreatesNewInstance() { + final AuthCache instance1 = configHooks.createAuthCache(configuration); + final AuthCache instance2 = configHooks.createAuthCache(configuration); + final AuthCache instance3 = configHooks.createAuthCache(configuration); + + // Instances should differ + assertNotSame("Instances should be different", instance1, instance2); + assertNotSame("Instances should be different", instance1, instance3); + assertNotSame("Instances should be different", instance2, instance3); + + // Should be expected type + assertTrue("Unexpected Type", instance1 instanceof BasicAuthCache); + } + + /** + * Test that the default behavior to always create a new instance. + */ + @Test + public void createCredentialsProvider_alwaysCreatesNewInstance() { + final CredentialsProvider instance1 = configHooks.createCredentialsProvider(configuration); + final CredentialsProvider instance2 = configHooks.createCredentialsProvider(configuration); + final CredentialsProvider instance3 = configHooks.createCredentialsProvider(configuration); + + // Instances should differ + assertNotSame("Instances should be different", instance1, instance2); + assertNotSame("Instances should be different", instance1, instance3); + assertNotSame("Instances should be different", instance2, instance3); + + // Should be expected type + assertTrue("Unexpected Type", instance1 instanceof BasicCredentialsProvider); + } + + /** + * Test that the default behavior is to always create a new HttpClientContext. + */ + @Test + public void createHttpClientContext_alwaysCreatesNewInstance() { + final HttpClientContext instance1 = configHooks.createHttpClientContext(configuration); + final HttpClientContext instance2 = configHooks.createHttpClientContext(configuration); + final HttpClientContext instance3 = configHooks.createHttpClientContext(configuration); + + assertNotSame("Instances should be different", instance1, instance2); + assertNotSame("Instances should be different", instance1, instance3); + assertNotSame("Instances should be different", instance2, instance3); + } + + /** + * Test that the default behavior is to always return the same instance unmodified. + */ + @Test + public void modifyAuthCache_doesntModifyInstance() { + final AuthCache mockInstance = spy(AuthCache.class); + + // Call method under test + final AuthCache result = configHooks.modifyAuthCache(configuration, mockInstance); + + // Verify returned the same instance + assertSame("Should be same instance", mockInstance, result); + verifyZeroInteractions(mockInstance); + } + + /** + * Test that the default behavior is to always return the same instance unmodified. + */ + @Test + public void modifyCredentialsProvider_doesntModifyInstance() { + final CredentialsProvider mockInstance = spy(CredentialsProvider.class); + + // Call method under test + final CredentialsProvider result = configHooks.modifyCredentialsProvider(configuration, mockInstance); + + // Verify returned the same instance + assertSame("Should be same instance", mockInstance, result); + verifyZeroInteractions(mockInstance); + } + + /** + * Test that the default behavior is to always return the same instance unmodified. + */ + @Test + public void modifyRequestConfig_doesntModifyInstance() { + final RequestConfig.Builder mockInstance = spy(RequestConfig.Builder.class); + + // Call method under test + final RequestConfig.Builder result = configHooks.modifyRequestConfig(configuration, mockInstance); + + // Verify returned the same instance + assertSame("Should be same instance", mockInstance, result); + verifyZeroInteractions(mockInstance); + } + + /** + * Test that the default behavior is to always return the same instance unmodified. + */ + @Test + public void modifyHttpClientBuilder_doesntModifyInstance() { + final HttpClientBuilder mockInstance = spy(HttpClientBuilder.class); + + // Call method under test + final HttpClientBuilder result = configHooks.modifyHttpClientBuilder(configuration, mockInstance); + + // Verify returned the same instance + assertSame("Should be same instance", mockInstance, result); + verifyZeroInteractions(mockInstance); + } + + /** + * Test that the default behavior is to always return the same instance unmodified. + */ + @Test + public void modifyHttpClientContext_doesntModifyInstance() { + final HttpClientContext mockInstance = spy(HttpClientContext.class); + + // Call method under test + final HttpClientContext result = configHooks.modifyHttpClientContext(configuration, mockInstance); + + // Verify returned the same instance + assertSame("Should be same instance", mockInstance, result); + verifyZeroInteractions(mockInstance); + } +} \ No newline at end of file diff --git a/src/test/java/org/sourcelab/kafka/connect/apiclient/rest/HttpClientRestClientTest.java b/src/test/java/org/sourcelab/kafka/connect/apiclient/rest/HttpClientRestClientTest.java index 18c32b7..703e994 100644 --- a/src/test/java/org/sourcelab/kafka/connect/apiclient/rest/HttpClientRestClientTest.java +++ b/src/test/java/org/sourcelab/kafka/connect/apiclient/rest/HttpClientRestClientTest.java @@ -17,13 +17,18 @@ package org.sourcelab.kafka.connect.apiclient.rest; +import org.apache.http.client.AuthCache; +import org.apache.http.client.CredentialsProvider; import org.apache.http.client.ResponseHandler; +import org.apache.http.client.config.RequestConfig; import org.apache.http.client.methods.HttpUriRequest; import org.apache.http.client.protocol.HttpClientContext; import org.apache.http.impl.client.CloseableHttpClient; import org.apache.http.impl.client.HttpClientBuilder; import org.junit.BeforeClass; import org.junit.Test; +import org.mockito.Mock; +import org.mockito.Mockito; import org.sourcelab.kafka.connect.apiclient.Configuration; import org.sourcelab.kafka.connect.apiclient.request.Request; import org.sourcelab.kafka.connect.apiclient.request.RequestMethod; @@ -35,10 +40,13 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotSame; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.any; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; public class HttpClientRestClientTest { @@ -90,71 +98,6 @@ public void doHttpTest() throws Exception { } } - /** - * Test that the HttpClientRestClient actually uses the builder returned from the - * {@link HttpClientRestClient#createHttpClientBuilder()} method. - */ - @Test - public void doHttp_withCustomHttpClientBuilder() { - // Create a mock builder and a rest client that uses the mock builder - final HttpClientBuilder builderMock = mock(HttpClientBuilder.class); - HttpClientRestClient restClient = new HttpClientRestClient() { - @Override - protected HttpClientBuilder createHttpClientBuilder() { - return builderMock; - } - }; - - // Init the rest client - final Configuration configuration = new Configuration("http://localhost:" + HTTP_PORT); - restClient.init(configuration); - - // Verify the mock was used to create the HttpClient - verify(builderMock).build(); - } - - /** - * Test that the every request uses a new HttpClientContext. - */ - @Test - public void doHttp_verifyNewHttpContextOnEveryRequest() throws IOException { - AtomicReference firstContext = new AtomicReference<>(); - CloseableHttpClient httpClientMock = mock(CloseableHttpClient.class); - when(httpClientMock.execute(any(HttpUriRequest.class), any(ResponseHandler.class), any(HttpClientContext.class))) - .then(invocation -> { - // Store the context of first request - HttpClientContext context = invocation.getArgument(2); - firstContext.set(context); - return null; - }) - .then(invocation -> { - // Compare the context of second request with the first context - HttpClientContext context = invocation.getArgument(2); - assertNotSame(context, firstContext.get()); - return null; - }); - - // Create a mock builder and a rest client that uses the mock builder - final HttpClientBuilder builderMock = mock(HttpClientBuilder.class); - HttpClientRestClient restClient = new HttpClientRestClient() { - @Override - protected HttpClientBuilder createHttpClientBuilder() { - return builderMock; - } - }; - when(builderMock.build()).thenReturn(httpClientMock); - - // Init the rest client - final Configuration configuration = new Configuration("http://localhost:" + HTTP_PORT); - restClient.init(configuration); - - restClient.submitRequest(new DummyRequest()); - restClient.submitRequest(new DummyRequest()); - - verify(httpClientMock, times(2)) - .execute(any(HttpUriRequest.class), any(ResponseHandler.class), any(HttpClientContext.class)); - } - /** * Test against Https server. */ @@ -226,6 +169,103 @@ public void doHttps_withClientValidation_withCertificateValidation_Test() throws } } + /** + * Verifies that config hooks are called as expected during HttpClientRestClient::init(). + */ + @Test + public void validateConfigHooks() { + // Create mock + final HttpClientConfigHooks mockHooks = spy(DefaultHttpClientConfigHooks.class); + + // Define configuration + final Configuration configuration = new Configuration("http://localhost:" + HTTP_PORT); + + // Create rest client injecting hooks + final HttpClientRestClient restClient = new HttpClientRestClient(mockHooks); + + // Call init + restClient.init(configuration); + + // Verify spy was called as expected + + // HttpClient Builder + Mockito + .verify(mockHooks, times(1)) + .createHttpClientBuilder(configuration); + Mockito + .verify(mockHooks, times(1)) + .modifyHttpClientBuilder(eq(configuration), any(HttpClientBuilder.class)); + + // HttpsContext Builder + Mockito + .verify(mockHooks, times(1)) + .createHttpsContextBuilder(configuration); + + // Request Builder + Mockito + .verify(mockHooks, times(1)) + .createRequestConfigBuilder(configuration); + Mockito + .verify(mockHooks, times(1)) + .modifyRequestConfig(eq(configuration), any(RequestConfig.Builder.class)); + + // AuthCache + Mockito + .verify(mockHooks, times(1)) + .createAuthCache(configuration); + Mockito + .verify(mockHooks, times(1)) + .modifyAuthCache(eq(configuration), any(AuthCache.class)); + + // CredentialsProvider + Mockito + .verify(mockHooks, times(1)) + .createCredentialsProvider(configuration); + Mockito + .verify(mockHooks, times(1)) + .modifyCredentialsProvider(eq(configuration), any(CredentialsProvider.class)); + + // Verify we had no other odd interactions. + verifyNoMoreInteractions(mockHooks); + } + + /** + * Verifies that config hooks are called as expected during HttpClientRestClient::createHttpClientContext(). + */ + @Test + public void verifyHttpClientReturnedByHookIsUsed() throws Exception { + // Create mock + final HttpClientConfigHooks mockHooks = spy(DefaultHttpClientConfigHooks.class); + + // Define configuration + final Configuration configuration = new Configuration("http://localhost:" + HTTP_PORT); + + // Create rest client injecting hooks + final HttpClientRestClient restClient = new HttpClientRestClient(mockHooks); + + // Call init + restClient.init(configuration); + + try (final TestHttpServer httpServer = new TestHttpServer() + .withHttp(HTTP_PORT) + .withMockData(RESPONSE_DATA) + .start() + ) { + // Make 2 requests + RestResponse result = restClient.submitRequest(new DummyRequest()); + assertEquals(RESPONSE_DATA, result.getResponseStr()); + + result = restClient.submitRequest(new DummyRequest()); + assertEquals(RESPONSE_DATA, result.getResponseStr()); + } + + // Verify hooks on HttpClientContext + verify(mockHooks, times(2)) + .createHttpClientContext(configuration); + verify(mockHooks, times(2)) + .modifyHttpClientContext(eq(configuration), any(HttpClientContext.class)); + } + /** * Represents a dummy request. */ From 32d2fd7da5b045bd1470897c85bd208e00967c2e Mon Sep 17 00:00:00 2001 From: Crim Date: Wed, 11 Aug 2021 18:23:18 +0900 Subject: [PATCH 3/4] cleanup --- CHANGELOG.md | 16 +++++++++++++++- .../apiclient/rest/HttpClientRestClient.java | 2 -- .../rest/DefaultHttpClientConfigHooksTest.java | 1 - .../apiclient/rest/HttpClientRestClientTest.java | 9 --------- 4 files changed, 15 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index aad5019..189c02e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,21 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## 3.1.3 (08/11/2021) - [Issue-55](https://github.com/SourceLabOrg/kafka-connect-client/issues/55) Create new HttpContext for every request. -- [] +- [PR-59](https://github.com/SourceLabOrg/kafka-connect-client/pull/59) Adds supportted way to modify the underlying configuration of HttpClient via HttpClientConfigHooks interface. + +Usage of these hooks would look like: + +```java +// Directly create underlying RestClient and pass your HttpClientConfigHooks implementation. +final RestClient restClient = new HttpClientRestClient(new HttpClientConfigHooks { + // Override methods as needed to modify behavior. +}); + +// Create KafkaConnectClient, passing configuration and RestClient implementation +final KafkaConnectClient client = new KafkaConnectClient(configuration, restClient); + +// Use client as normal... +``` ## 3.1.2 (07/21/2021) diff --git a/src/main/java/org/sourcelab/kafka/connect/apiclient/rest/HttpClientRestClient.java b/src/main/java/org/sourcelab/kafka/connect/apiclient/rest/HttpClientRestClient.java index c5270bf..a9bb886 100644 --- a/src/main/java/org/sourcelab/kafka/connect/apiclient/rest/HttpClientRestClient.java +++ b/src/main/java/org/sourcelab/kafka/connect/apiclient/rest/HttpClientRestClient.java @@ -35,8 +35,6 @@ import org.apache.http.client.utils.URIBuilder; import org.apache.http.entity.StringEntity; import org.apache.http.impl.auth.BasicScheme; -import org.apache.http.impl.client.BasicAuthCache; -import org.apache.http.impl.client.BasicCredentialsProvider; import org.apache.http.impl.client.CloseableHttpClient; import org.apache.http.impl.client.HttpClientBuilder; import org.apache.http.message.BasicHeader; diff --git a/src/test/java/org/sourcelab/kafka/connect/apiclient/rest/DefaultHttpClientConfigHooksTest.java b/src/test/java/org/sourcelab/kafka/connect/apiclient/rest/DefaultHttpClientConfigHooksTest.java index ab616f9..c0ef130 100644 --- a/src/test/java/org/sourcelab/kafka/connect/apiclient/rest/DefaultHttpClientConfigHooksTest.java +++ b/src/test/java/org/sourcelab/kafka/connect/apiclient/rest/DefaultHttpClientConfigHooksTest.java @@ -30,7 +30,6 @@ import static org.junit.Assert.assertNotSame; import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; -import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verifyZeroInteractions; diff --git a/src/test/java/org/sourcelab/kafka/connect/apiclient/rest/HttpClientRestClientTest.java b/src/test/java/org/sourcelab/kafka/connect/apiclient/rest/HttpClientRestClientTest.java index 703e994..bb09211 100644 --- a/src/test/java/org/sourcelab/kafka/connect/apiclient/rest/HttpClientRestClientTest.java +++ b/src/test/java/org/sourcelab/kafka/connect/apiclient/rest/HttpClientRestClientTest.java @@ -19,15 +19,11 @@ import org.apache.http.client.AuthCache; import org.apache.http.client.CredentialsProvider; -import org.apache.http.client.ResponseHandler; import org.apache.http.client.config.RequestConfig; -import org.apache.http.client.methods.HttpUriRequest; import org.apache.http.client.protocol.HttpClientContext; -import org.apache.http.impl.client.CloseableHttpClient; import org.apache.http.impl.client.HttpClientBuilder; import org.junit.BeforeClass; import org.junit.Test; -import org.mockito.Mock; import org.mockito.Mockito; import org.sourcelab.kafka.connect.apiclient.Configuration; import org.sourcelab.kafka.connect.apiclient.request.Request; @@ -35,19 +31,14 @@ import testserver.TestHttpServer; import java.io.File; -import java.io.IOException; -import java.util.concurrent.atomic.AtomicReference; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotSame; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.any; -import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; -import static org.mockito.Mockito.when; public class HttpClientRestClientTest { From a0633df8cf9e09326c87cab625127f556ad1aea0 Mon Sep 17 00:00:00 2001 From: Crim Date: Wed, 11 Aug 2021 18:55:32 +0900 Subject: [PATCH 4/4] update license header --- .../connect/apiclient/rest/DefaultHttpClientConfigHooks.java | 2 +- .../kafka/connect/apiclient/rest/HttpClientConfigHooks.java | 2 +- .../apiclient/rest/DefaultHttpClientConfigHooksTest.java | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/sourcelab/kafka/connect/apiclient/rest/DefaultHttpClientConfigHooks.java b/src/main/java/org/sourcelab/kafka/connect/apiclient/rest/DefaultHttpClientConfigHooks.java index b34189f..7fd53d4 100644 --- a/src/main/java/org/sourcelab/kafka/connect/apiclient/rest/DefaultHttpClientConfigHooks.java +++ b/src/main/java/org/sourcelab/kafka/connect/apiclient/rest/DefaultHttpClientConfigHooks.java @@ -1,5 +1,5 @@ /** - * Copyright 2018, 2019 SourceLab.org https://github.com/SourceLabOrg/kafka-connect-client + * Copyright 2018, 2019, 2020, 2021 SourceLab.org https://github.com/SourceLabOrg/kafka-connect-client * * Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated * documentation files (the "Software"), to deal in the Software without restriction, including without limitation the diff --git a/src/main/java/org/sourcelab/kafka/connect/apiclient/rest/HttpClientConfigHooks.java b/src/main/java/org/sourcelab/kafka/connect/apiclient/rest/HttpClientConfigHooks.java index 5e931ac..09b46ab 100644 --- a/src/main/java/org/sourcelab/kafka/connect/apiclient/rest/HttpClientConfigHooks.java +++ b/src/main/java/org/sourcelab/kafka/connect/apiclient/rest/HttpClientConfigHooks.java @@ -1,5 +1,5 @@ /** - * Copyright 2018, 2019 SourceLab.org https://github.com/SourceLabOrg/kafka-connect-client + * Copyright 2018, 2019, 2020, 2021 SourceLab.org https://github.com/SourceLabOrg/kafka-connect-client * * Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated * documentation files (the "Software"), to deal in the Software without restriction, including without limitation the diff --git a/src/test/java/org/sourcelab/kafka/connect/apiclient/rest/DefaultHttpClientConfigHooksTest.java b/src/test/java/org/sourcelab/kafka/connect/apiclient/rest/DefaultHttpClientConfigHooksTest.java index c0ef130..5608736 100644 --- a/src/test/java/org/sourcelab/kafka/connect/apiclient/rest/DefaultHttpClientConfigHooksTest.java +++ b/src/test/java/org/sourcelab/kafka/connect/apiclient/rest/DefaultHttpClientConfigHooksTest.java @@ -1,5 +1,5 @@ /** - * Copyright 2018, 2019 SourceLab.org https://github.com/SourceLabOrg/kafka-connect-client + * Copyright 2018, 2019, 2020, 2021 SourceLab.org https://github.com/SourceLabOrg/kafka-connect-client * * Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated * documentation files (the "Software"), to deal in the Software without restriction, including without limitation the