Skip to content

Commit 17b3ab3

Browse files
committed
cleanup, add test coverage
1 parent 62c8584 commit 17b3ab3

File tree

6 files changed

+396
-74
lines changed

6 files changed

+396
-74
lines changed

CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
44

55
## 3.1.3 (08/11/2021)
66
- [Issue-55](https://github.com/SourceLabOrg/kafka-connect-client/issues/55) Create new HttpContext for every request.
7-
-
7+
- []
88

99
## 3.1.2 (07/21/2021)
1010

src/main/java/org/sourcelab/kafka/connect/apiclient/rest/DefaultHttpClientConfigHooks.java

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,20 @@
1+
/**
2+
* Copyright 2018, 2019 SourceLab.org https://github.com/SourceLabOrg/kafka-connect-client
3+
*
4+
* Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated
5+
* documentation files (the "Software"), to deal in the Software without restriction, including without limitation the
6+
* rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to permit
7+
* persons to whom the Software is furnished to do so, subject to the following conditions:
8+
*
9+
* The above copyright notice and this permission notice shall be included in all copies or substantial portions of the
10+
* Software.
11+
*
12+
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE
13+
* WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
14+
* COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
15+
* OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
16+
*/
17+
118
package org.sourcelab.kafka.connect.apiclient.rest;
219

320
/**

src/main/java/org/sourcelab/kafka/connect/apiclient/rest/HttpClientConfigHooks.java

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,20 @@
1+
/**
2+
* Copyright 2018, 2019 SourceLab.org https://github.com/SourceLabOrg/kafka-connect-client
3+
*
4+
* Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated
5+
* documentation files (the "Software"), to deal in the Software without restriction, including without limitation the
6+
* rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to permit
7+
* persons to whom the Software is furnished to do so, subject to the following conditions:
8+
*
9+
* The above copyright notice and this permission notice shall be included in all copies or substantial portions of the
10+
* Software.
11+
*
12+
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE
13+
* WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
14+
* COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
15+
* OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
16+
*/
17+
118
package org.sourcelab.kafka.connect.apiclient.rest;
219

320
import org.apache.http.client.AuthCache;
@@ -13,6 +30,18 @@
1330
* HttpClient configuration hooks.
1431
*
1532
* Provides an interface for modifying how the underlying HttpClient instance is created.
33+
*
34+
* Usage of this would look like:
35+
*
36+
* final RestClient restClient = new HttpClientRestClient(new HttpClientConfigHooks {
37+
* // Override methods as needed to modify behavior.
38+
* });
39+
*
40+
* // Create client, passing configuration and RestClient implementation
41+
* final KafkaConnectClient client = new KafkaConnectClient(configuration, restClient);
42+
*
43+
* // Use client as normal...
44+
*
1645
*/
1746
public interface HttpClientConfigHooks {
1847
/**

src/main/java/org/sourcelab/kafka/connect/apiclient/rest/HttpClientRestClient.java

Lines changed: 33 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@
6060
import java.util.Collection;
6161
import java.util.Collections;
6262
import java.util.Map;
63+
import java.util.Objects;
6364
import java.util.concurrent.TimeUnit;
6465

6566
/**
@@ -130,7 +131,10 @@ public void init(final Configuration configuration) {
130131
final HttpsContextBuilder httpsContextBuilder = configHooks.createHttpsContextBuilder(configuration);
131132

132133
// Create and setup client builder
133-
HttpClientBuilder clientBuilder = configHooks.createHttpClientBuilder(configuration);
134+
HttpClientBuilder clientBuilder = Objects.requireNonNull(
135+
configHooks.createHttpClientBuilder(configuration),
136+
"HttpClientConfigHook::createHttpClientBuilder() must return non-null instance."
137+
);
134138
clientBuilder
135139
// Define timeout
136140
.setConnectionTimeToLive(configuration.getConnectionTimeToLiveInSeconds(), TimeUnit.SECONDS)
@@ -139,15 +143,24 @@ public void init(final Configuration configuration) {
139143
.setSSLSocketFactory(httpsContextBuilder.createSslSocketFactory());
140144

141145
// Define our RequestConfigBuilder
142-
RequestConfig.Builder requestConfigBuilder = configHooks.createRequestConfigBuilder(configuration);
146+
RequestConfig.Builder requestConfigBuilder = Objects.requireNonNull(
147+
configHooks.createRequestConfigBuilder(configuration),
148+
"HttpClientConfigHook::createRequestConfigBuilder() must return non-null instance."
149+
);
143150

144151
requestConfigBuilder.setConnectTimeout(configuration.getRequestTimeoutInSeconds() * 1_000);
145152

146153
// Define our Credentials Provider
147-
credsProvider = configHooks.createCredentialsProvider(configuration);
154+
credsProvider = Objects.requireNonNull(
155+
configHooks.createCredentialsProvider(configuration),
156+
"HttpClientConfigHook::createCredentialsProvider() must return non-null instance."
157+
);
148158

149159
// Define our auth cache
150-
authCache = configHooks.createAuthCache(configuration);
160+
authCache = Objects.requireNonNull(
161+
configHooks.createAuthCache(configuration),
162+
"HttpClientConfigHook::createAuthCache() must return non-null instance."
163+
);
151164

152165
// If we have a configured proxy host
153166
if (configuration.getProxyHost() != null) {
@@ -201,9 +214,18 @@ public void init(final Configuration configuration) {
201214
}
202215

203216
// Call Modify hooks
204-
authCache = configHooks.modifyAuthCache(configuration, authCache);
205-
credsProvider = configHooks.modifyCredentialsProvider(configuration, credsProvider);
206-
requestConfigBuilder = configHooks.modifyRequestConfig(configuration, requestConfigBuilder);
217+
authCache = Objects.requireNonNull(
218+
configHooks.modifyAuthCache(configuration, authCache),
219+
"HttpClientConfigHook::modifyAuthCache() must return non-null instance."
220+
);
221+
credsProvider = Objects.requireNonNull(
222+
configHooks.modifyCredentialsProvider(configuration, credsProvider),
223+
"HttpClientConfigHook::modifyCredentialsProvider() must return non-null instance."
224+
);
225+
requestConfigBuilder = Objects.requireNonNull(
226+
configHooks.modifyRequestConfig(configuration, requestConfigBuilder),
227+
"HttpClientConfigHook::modifyRequestConfig() must return non-null instance."
228+
);
207229

208230
// Attach Credentials provider to client builder.
209231
clientBuilder.setDefaultCredentialsProvider(credsProvider);
@@ -212,7 +234,10 @@ public void init(final Configuration configuration) {
212234
clientBuilder.setDefaultRequestConfig(requestConfigBuilder.build());
213235

214236
// build http client
215-
clientBuilder = configHooks.modifyHttpClientBuilder(configuration, clientBuilder);
237+
clientBuilder = Objects.requireNonNull(
238+
configHooks.modifyHttpClientBuilder(configuration, clientBuilder),
239+
"HttpClientConfigHook::modifyHttpClientBuilder() must return non-null instance."
240+
);
216241
httpClient = clientBuilder.build();
217242
}
218243

Lines changed: 211 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,211 @@
1+
/**
2+
* Copyright 2018, 2019 SourceLab.org https://github.com/SourceLabOrg/kafka-connect-client
3+
*
4+
* Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated
5+
* documentation files (the "Software"), to deal in the Software without restriction, including without limitation the
6+
* rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to permit
7+
* persons to whom the Software is furnished to do so, subject to the following conditions:
8+
*
9+
* The above copyright notice and this permission notice shall be included in all copies or substantial portions of the
10+
* Software.
11+
*
12+
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE
13+
* WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
14+
* COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
15+
* OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
16+
*/
17+
18+
package org.sourcelab.kafka.connect.apiclient.rest;
19+
20+
import org.apache.http.client.AuthCache;
21+
import org.apache.http.client.CredentialsProvider;
22+
import org.apache.http.client.config.RequestConfig;
23+
import org.apache.http.client.protocol.HttpClientContext;
24+
import org.apache.http.impl.client.BasicAuthCache;
25+
import org.apache.http.impl.client.BasicCredentialsProvider;
26+
import org.apache.http.impl.client.HttpClientBuilder;
27+
import org.junit.Test;
28+
import org.sourcelab.kafka.connect.apiclient.Configuration;
29+
30+
import static org.junit.Assert.assertNotSame;
31+
import static org.junit.Assert.assertSame;
32+
import static org.junit.Assert.assertTrue;
33+
import static org.mockito.Mockito.mock;
34+
import static org.mockito.Mockito.spy;
35+
import static org.mockito.Mockito.verifyZeroInteractions;
36+
37+
/**
38+
* Verifying behavior of DefaultHttpClientConfigHooks.
39+
*/
40+
public class DefaultHttpClientConfigHooksTest {
41+
42+
private final Configuration configuration = new Configuration("https://dummy.host");
43+
private final DefaultHttpClientConfigHooks configHooks = new DefaultHttpClientConfigHooks();
44+
45+
/**
46+
* Test that the default behavior to always create a new instance.
47+
*/
48+
@Test
49+
public void createHttpClientBuilder_alwaysCreatesNewInstance() {
50+
final HttpClientContext instance1 = configHooks.createHttpClientContext(configuration);
51+
final HttpClientContext instance2 = configHooks.createHttpClientContext(configuration);
52+
final HttpClientContext instance3 = configHooks.createHttpClientContext(configuration);
53+
54+
assertNotSame("Instances should be different", instance1, instance2);
55+
assertNotSame("Instances should be different", instance1, instance3);
56+
assertNotSame("Instances should be different", instance2, instance3);
57+
}
58+
59+
/**
60+
* Test that the default behavior to always create a new instance.
61+
*/
62+
@Test
63+
public void createHttpsContextBuilder_alwaysCreatesNewInstance() {
64+
final HttpsContextBuilder instance1 = configHooks.createHttpsContextBuilder(configuration);
65+
final HttpsContextBuilder instance2 = configHooks.createHttpsContextBuilder(configuration);
66+
final HttpsContextBuilder instance3 = configHooks.createHttpsContextBuilder(configuration);
67+
68+
assertNotSame("Instances should be different", instance1, instance2);
69+
assertNotSame("Instances should be different", instance1, instance3);
70+
assertNotSame("Instances should be different", instance2, instance3);
71+
}
72+
73+
/**
74+
* Test that the default behavior to always create a new instance.
75+
*/
76+
@Test
77+
public void createRequestConfigBuilder_alwaysCreatesNewInstance() {
78+
final RequestConfig.Builder instance1 = configHooks.createRequestConfigBuilder(configuration);
79+
final RequestConfig.Builder instance2 = configHooks.createRequestConfigBuilder(configuration);
80+
final RequestConfig.Builder instance3 = configHooks.createRequestConfigBuilder(configuration);
81+
82+
assertNotSame("Instances should be different", instance1, instance2);
83+
assertNotSame("Instances should be different", instance1, instance3);
84+
assertNotSame("Instances should be different", instance2, instance3);
85+
}
86+
87+
/**
88+
* Test that the default behavior to always create a new instance.
89+
*/
90+
@Test
91+
public void createAuthCache_alwaysCreatesNewInstance() {
92+
final AuthCache instance1 = configHooks.createAuthCache(configuration);
93+
final AuthCache instance2 = configHooks.createAuthCache(configuration);
94+
final AuthCache instance3 = configHooks.createAuthCache(configuration);
95+
96+
// Instances should differ
97+
assertNotSame("Instances should be different", instance1, instance2);
98+
assertNotSame("Instances should be different", instance1, instance3);
99+
assertNotSame("Instances should be different", instance2, instance3);
100+
101+
// Should be expected type
102+
assertTrue("Unexpected Type", instance1 instanceof BasicAuthCache);
103+
}
104+
105+
/**
106+
* Test that the default behavior to always create a new instance.
107+
*/
108+
@Test
109+
public void createCredentialsProvider_alwaysCreatesNewInstance() {
110+
final CredentialsProvider instance1 = configHooks.createCredentialsProvider(configuration);
111+
final CredentialsProvider instance2 = configHooks.createCredentialsProvider(configuration);
112+
final CredentialsProvider instance3 = configHooks.createCredentialsProvider(configuration);
113+
114+
// Instances should differ
115+
assertNotSame("Instances should be different", instance1, instance2);
116+
assertNotSame("Instances should be different", instance1, instance3);
117+
assertNotSame("Instances should be different", instance2, instance3);
118+
119+
// Should be expected type
120+
assertTrue("Unexpected Type", instance1 instanceof BasicCredentialsProvider);
121+
}
122+
123+
/**
124+
* Test that the default behavior is to always create a new HttpClientContext.
125+
*/
126+
@Test
127+
public void createHttpClientContext_alwaysCreatesNewInstance() {
128+
final HttpClientContext instance1 = configHooks.createHttpClientContext(configuration);
129+
final HttpClientContext instance2 = configHooks.createHttpClientContext(configuration);
130+
final HttpClientContext instance3 = configHooks.createHttpClientContext(configuration);
131+
132+
assertNotSame("Instances should be different", instance1, instance2);
133+
assertNotSame("Instances should be different", instance1, instance3);
134+
assertNotSame("Instances should be different", instance2, instance3);
135+
}
136+
137+
/**
138+
* Test that the default behavior is to always return the same instance unmodified.
139+
*/
140+
@Test
141+
public void modifyAuthCache_doesntModifyInstance() {
142+
final AuthCache mockInstance = spy(AuthCache.class);
143+
144+
// Call method under test
145+
final AuthCache result = configHooks.modifyAuthCache(configuration, mockInstance);
146+
147+
// Verify returned the same instance
148+
assertSame("Should be same instance", mockInstance, result);
149+
verifyZeroInteractions(mockInstance);
150+
}
151+
152+
/**
153+
* Test that the default behavior is to always return the same instance unmodified.
154+
*/
155+
@Test
156+
public void modifyCredentialsProvider_doesntModifyInstance() {
157+
final CredentialsProvider mockInstance = spy(CredentialsProvider.class);
158+
159+
// Call method under test
160+
final CredentialsProvider result = configHooks.modifyCredentialsProvider(configuration, mockInstance);
161+
162+
// Verify returned the same instance
163+
assertSame("Should be same instance", mockInstance, result);
164+
verifyZeroInteractions(mockInstance);
165+
}
166+
167+
/**
168+
* Test that the default behavior is to always return the same instance unmodified.
169+
*/
170+
@Test
171+
public void modifyRequestConfig_doesntModifyInstance() {
172+
final RequestConfig.Builder mockInstance = spy(RequestConfig.Builder.class);
173+
174+
// Call method under test
175+
final RequestConfig.Builder result = configHooks.modifyRequestConfig(configuration, mockInstance);
176+
177+
// Verify returned the same instance
178+
assertSame("Should be same instance", mockInstance, result);
179+
verifyZeroInteractions(mockInstance);
180+
}
181+
182+
/**
183+
* Test that the default behavior is to always return the same instance unmodified.
184+
*/
185+
@Test
186+
public void modifyHttpClientBuilder_doesntModifyInstance() {
187+
final HttpClientBuilder mockInstance = spy(HttpClientBuilder.class);
188+
189+
// Call method under test
190+
final HttpClientBuilder result = configHooks.modifyHttpClientBuilder(configuration, mockInstance);
191+
192+
// Verify returned the same instance
193+
assertSame("Should be same instance", mockInstance, result);
194+
verifyZeroInteractions(mockInstance);
195+
}
196+
197+
/**
198+
* Test that the default behavior is to always return the same instance unmodified.
199+
*/
200+
@Test
201+
public void modifyHttpClientContext_doesntModifyInstance() {
202+
final HttpClientContext mockInstance = spy(HttpClientContext.class);
203+
204+
// Call method under test
205+
final HttpClientContext result = configHooks.modifyHttpClientContext(configuration, mockInstance);
206+
207+
// Verify returned the same instance
208+
assertSame("Should be same instance", mockInstance, result);
209+
verifyZeroInteractions(mockInstance);
210+
}
211+
}

0 commit comments

Comments
 (0)