Skip to content

Commit da19ebb

Browse files
jaeoptjaeopt
authored
[FSSDK-10041] fix to inject common httpclient to projectConfigManager, eventHandler and odpManager (#542)
OptimizelyFactory method for injecting customHttpClient is fixed to share the customHttpClient for all modules using httpClient - HttpProjectConfigManager - AsyncEventHander - ODPManager
1 parent a77eaa8 commit da19ebb

14 files changed

+149
-63
lines changed

‎core-api/src/main/java/com/optimizely/ab/Optimizely.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,6 @@ public class Optimizely implements AutoCloseable {
7878
private static final Logger logger = LoggerFactory.getLogger(Optimizely.class);
7979

8080
final DecisionService decisionService;
81-
@VisibleForTesting
8281
@Deprecated
8382
final EventHandler eventHandler;
8483
@VisibleForTesting
@@ -88,7 +87,8 @@ public class Optimizely implements AutoCloseable {
8887

8988
public final List<OptimizelyDecideOption> defaultDecideOptions;
9089

91-
private final ProjectConfigManager projectConfigManager;
90+
@VisibleForTesting
91+
final ProjectConfigManager projectConfigManager;
9292

9393
@Nullable
9494
private final OptimizelyConfigManager optimizelyConfigManager;

‎core-api/src/main/java/com/optimizely/ab/event/BatchEventProcessor.java

+3-1
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
*/
1717
package com.optimizely.ab.event;
1818

19+
import com.optimizely.ab.annotations.VisibleForTesting;
1920
import com.optimizely.ab.config.ProjectConfig;
2021
import com.optimizely.ab.event.internal.EventFactory;
2122
import com.optimizely.ab.event.internal.UserEvent;
@@ -58,7 +59,8 @@ public class BatchEventProcessor implements EventProcessor, AutoCloseable {
5859
private static final Object FLUSH_SIGNAL = new Object();
5960

6061
private final BlockingQueue<Object> eventQueue;
61-
private final EventHandler eventHandler;
62+
@VisibleForTesting
63+
public final EventHandler eventHandler;
6264

6365
final int batchSize;
6466
final long flushInterval;

‎core-api/src/main/java/com/optimizely/ab/odp/ODPEventManager.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,8 @@ public class ODPEventManager {
5353
// needs to see the change immediately.
5454
private volatile ODPConfig odpConfig;
5555
private EventDispatcherThread eventDispatcherThread;
56-
57-
private final ODPApiManager apiManager;
56+
@VisibleForTesting
57+
public final ODPApiManager apiManager;
5858

5959
// The eventQueue needs to be thread safe. We are not doing anything extra for thread safety here
6060
// because `LinkedBlockingQueue` itself is thread safe.

‎core-api/src/main/java/com/optimizely/ab/odp/ODPManager.java

-1
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
import org.slf4j.LoggerFactory;
2222

2323
import javax.annotation.Nonnull;
24-
import java.util.Collections;
2524
import java.util.List;
2625
import java.util.Map;
2726
import java.util.Set;

‎core-api/src/main/java/com/optimizely/ab/odp/ODPSegmentManager.java

+3-2
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
*/
1717
package com.optimizely.ab.odp;
1818

19+
import com.optimizely.ab.annotations.VisibleForTesting;
1920
import com.optimizely.ab.internal.Cache;
2021
import com.optimizely.ab.internal.DefaultLRUCache;
2122
import com.optimizely.ab.odp.parser.ResponseJsonParser;
@@ -31,8 +32,8 @@ public class ODPSegmentManager {
3132
private static final Logger logger = LoggerFactory.getLogger(ODPSegmentManager.class);
3233

3334
private static final String SEGMENT_URL_PATH = "/v3/graphql";
34-
35-
private final ODPApiManager apiManager;
35+
@VisibleForTesting
36+
public final ODPApiManager apiManager;
3637

3738
private volatile ODPConfig odpConfig;
3839

‎core-httpclient-impl/src/main/java/com/optimizely/ab/OptimizelyFactory.java

+35-9
Original file line numberDiff line numberDiff line change
@@ -279,23 +279,37 @@ public static Optimizely newDefaultInstance(String sdkKey, String fallback, Stri
279279
* @param customHttpClient Customizable CloseableHttpClient to build OptimizelyHttpClient.
280280
* @return A new Optimizely instance
281281
*/
282-
public static Optimizely newDefaultInstance(String sdkKey, String fallback, String datafileAccessToken, CloseableHttpClient customHttpClient) {
282+
public static Optimizely newDefaultInstance(
283+
String sdkKey,
284+
String fallback,
285+
String datafileAccessToken,
286+
CloseableHttpClient customHttpClient
287+
) {
288+
OptimizelyHttpClient optimizelyHttpClient = customHttpClient == null ? null : new OptimizelyHttpClient(customHttpClient);
289+
283290
NotificationCenter notificationCenter = new NotificationCenter();
284-
OptimizelyHttpClient optimizelyHttpClient = new OptimizelyHttpClient(customHttpClient);
285-
HttpProjectConfigManager.Builder builder;
286-
builder = HttpProjectConfigManager.builder()
291+
292+
HttpProjectConfigManager.Builder builder = HttpProjectConfigManager.builder()
287293
.withDatafile(fallback)
288294
.withNotificationCenter(notificationCenter)
289-
.withOptimizelyHttpClient(customHttpClient == null ? null : optimizelyHttpClient)
295+
.withOptimizelyHttpClient(optimizelyHttpClient)
290296
.withSdkKey(sdkKey);
291297

292298
if (datafileAccessToken != null) {
293299
builder.withDatafileAccessToken(datafileAccessToken);
294300
}
295301

296-
return newDefaultInstance(builder.build(), notificationCenter);
297-
}
302+
ProjectConfigManager configManager = builder.build();
303+
304+
EventHandler eventHandler = AsyncEventHandler.builder()
305+
.withOptimizelyHttpClient(optimizelyHttpClient)
306+
.build();
298307

308+
ODPApiManager odpApiManager = new DefaultODPApiManager(optimizelyHttpClient);
309+
310+
return newDefaultInstance(configManager, notificationCenter, eventHandler, odpApiManager);
311+
}
312+
299313
/**
300314
* Returns a new Optimizely instance based on preset configuration.
301315
* EventHandler - {@link AsyncEventHandler}
@@ -329,6 +343,19 @@ public static Optimizely newDefaultInstance(ProjectConfigManager configManager,
329343
* @return A new Optimizely instance
330344
* */
331345
public static Optimizely newDefaultInstance(ProjectConfigManager configManager, NotificationCenter notificationCenter, EventHandler eventHandler) {
346+
return newDefaultInstance(configManager, notificationCenter, eventHandler, null);
347+
}
348+
349+
/**
350+
* Returns a new Optimizely instance based on preset configuration.
351+
*
352+
* @param configManager The {@link ProjectConfigManager} supplied to Optimizely instance.
353+
* @param notificationCenter The {@link ProjectConfigManager} supplied to Optimizely instance.
354+
* @param eventHandler The {@link EventHandler} supplied to Optimizely instance.
355+
* @param odpApiManager The {@link ODPApiManager} supplied to Optimizely instance.
356+
* @return A new Optimizely instance
357+
* */
358+
public static Optimizely newDefaultInstance(ProjectConfigManager configManager, NotificationCenter notificationCenter, EventHandler eventHandler, ODPApiManager odpApiManager) {
332359
if (notificationCenter == null) {
333360
notificationCenter = new NotificationCenter();
334361
}
@@ -338,9 +365,8 @@ public static Optimizely newDefaultInstance(ProjectConfigManager configManager,
338365
.withNotificationCenter(notificationCenter)
339366
.build();
340367

341-
ODPApiManager defaultODPApiManager = new DefaultODPApiManager();
342368
ODPManager odpManager = ODPManager.builder()
343-
.withApiManager(defaultODPApiManager)
369+
.withApiManager(odpApiManager != null ? odpApiManager : new DefaultODPApiManager())
344370
.build();
345371

346372
return Optimizely.builder()

‎core-httpclient-impl/src/main/java/com/optimizely/ab/OptimizelyHttpClient.java

-1
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@
4141
public class OptimizelyHttpClient implements Closeable {
4242

4343
private static final Logger logger = LoggerFactory.getLogger(OptimizelyHttpClient.class);
44-
4544
private final CloseableHttpClient httpClient;
4645

4746
OptimizelyHttpClient(CloseableHttpClient httpClient) {

‎core-httpclient-impl/src/main/java/com/optimizely/ab/config/HttpProjectConfigManager.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,8 @@ public class HttpProjectConfigManager extends PollingProjectConfigManager {
6161

6262
private static final Logger logger = LoggerFactory.getLogger(HttpProjectConfigManager.class);
6363

64-
private final OptimizelyHttpClient httpClient;
64+
@VisibleForTesting
65+
public final OptimizelyHttpClient httpClient;
6566
private final URI uri;
6667
private final String datafileAccessToken;
6768
private String datafileLastModified;

‎core-httpclient-impl/src/main/java/com/optimizely/ab/event/AsyncEventHandler.java

+36-15
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
import java.util.concurrent.TimeUnit;
4646

4747
import javax.annotation.CheckForNull;
48+
import javax.annotation.Nullable;
4849

4950
/**
5051
* {@link EventHandler} implementation that queues events and has a separate pool of threads responsible
@@ -67,7 +68,8 @@ public class AsyncEventHandler implements EventHandler, AutoCloseable {
6768
private static final Logger logger = LoggerFactory.getLogger(AsyncEventHandler.class);
6869
private static final ProjectConfigResponseHandler EVENT_RESPONSE_HANDLER = new ProjectConfigResponseHandler();
6970

70-
private final OptimizelyHttpClient httpClient;
71+
@VisibleForTesting
72+
public final OptimizelyHttpClient httpClient;
7173
private final ExecutorService workerExecutor;
7274

7375
private final long closeTimeout;
@@ -110,7 +112,15 @@ public AsyncEventHandler(int queueCapacity,
110112
int validateAfter,
111113
long closeTimeout,
112114
TimeUnit closeTimeoutUnit) {
113-
this(queueCapacity, numWorkers, maxConnections, connectionsPerRoute, validateAfter, closeTimeout, closeTimeoutUnit, null);
115+
this(queueCapacity,
116+
numWorkers,
117+
maxConnections,
118+
connectionsPerRoute,
119+
validateAfter,
120+
closeTimeout,
121+
closeTimeoutUnit,
122+
null,
123+
null);
114124
}
115125

116126
public AsyncEventHandler(int queueCapacity,
@@ -120,24 +130,27 @@ public AsyncEventHandler(int queueCapacity,
120130
int validateAfter,
121131
long closeTimeout,
122132
TimeUnit closeTimeoutUnit,
133+
@Nullable OptimizelyHttpClient httpClient,
123134
@Nullable ThreadFactory threadFactory) {
135+
if (httpClient != null) {
136+
this.httpClient = httpClient;
137+
} else {
138+
maxConnections = validateInput("maxConnections", maxConnections, DEFAULT_MAX_CONNECTIONS);
139+
connectionsPerRoute = validateInput("connectionsPerRoute", connectionsPerRoute, DEFAULT_MAX_PER_ROUTE);
140+
validateAfter = validateInput("validateAfter", validateAfter, DEFAULT_VALIDATE_AFTER_INACTIVITY);
141+
this.httpClient = OptimizelyHttpClient.builder()
142+
.withMaxTotalConnections(maxConnections)
143+
.withMaxPerRoute(connectionsPerRoute)
144+
.withValidateAfterInactivity(validateAfter)
145+
// infrequent event discards observed. staled connections force-closed after a long idle time.
146+
.withEvictIdleConnections(1L, TimeUnit.MINUTES)
147+
.build();
148+
}
124149

125150
queueCapacity = validateInput("queueCapacity", queueCapacity, DEFAULT_QUEUE_CAPACITY);
126151
numWorkers = validateInput("numWorkers", numWorkers, DEFAULT_NUM_WORKERS);
127-
maxConnections = validateInput("maxConnections", maxConnections, DEFAULT_MAX_CONNECTIONS);
128-
connectionsPerRoute = validateInput("connectionsPerRoute", connectionsPerRoute, DEFAULT_MAX_PER_ROUTE);
129-
validateAfter = validateInput("validateAfter", validateAfter, DEFAULT_VALIDATE_AFTER_INACTIVITY);
130-
131-
this.httpClient = OptimizelyHttpClient.builder()
132-
.withMaxTotalConnections(maxConnections)
133-
.withMaxPerRoute(connectionsPerRoute)
134-
.withValidateAfterInactivity(validateAfter)
135-
// infrequent event discards observed. staled connections force-closed after a long idle time.
136-
.withEvictIdleConnections(1L, TimeUnit.MINUTES)
137-
.build();
138152

139153
NamedThreadFactory namedThreadFactory = new NamedThreadFactory("optimizely-event-dispatcher-thread-%s", true, threadFactory);
140-
141154
this.workerExecutor = new ThreadPoolExecutor(numWorkers, numWorkers,
142155
0L, TimeUnit.MILLISECONDS,
143156
new ArrayBlockingQueue<>(queueCapacity),
@@ -302,6 +315,7 @@ public static class Builder {
302315
int validateAfterInactivity = PropertyUtils.getInteger(CONFIG_VALIDATE_AFTER_INACTIVITY, DEFAULT_VALIDATE_AFTER_INACTIVITY);
303316
private long closeTimeout = Long.MAX_VALUE;
304317
private TimeUnit closeTimeoutUnit = TimeUnit.MILLISECONDS;
318+
private OptimizelyHttpClient httpClient;
305319

306320
public Builder withQueueCapacity(int queueCapacity) {
307321
if (queueCapacity <= 0) {
@@ -344,6 +358,11 @@ public Builder withCloseTimeout(long closeTimeout, TimeUnit unit) {
344358
return this;
345359
}
346360

361+
public Builder withOptimizelyHttpClient(OptimizelyHttpClient httpClient) {
362+
this.httpClient = httpClient;
363+
return this;
364+
}
365+
347366
public AsyncEventHandler build() {
348367
return new AsyncEventHandler(
349368
queueCapacity,
@@ -352,7 +371,9 @@ public AsyncEventHandler build() {
352371
maxPerRoute,
353372
validateAfterInactivity,
354373
closeTimeout,
355-
closeTimeoutUnit
374+
closeTimeoutUnit,
375+
httpClient,
376+
null
356377
);
357378
}
358379
}

‎core-httpclient-impl/src/main/java/com/optimizely/ab/odp/DefaultODPApiManager.java

+11-5
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import org.slf4j.Logger;
2828
import org.slf4j.LoggerFactory;
2929

30+
import javax.annotation.Nullable;
3031
import java.io.IOException;
3132
import java.io.UnsupportedEncodingException;
3233
import java.util.Iterator;
@@ -36,11 +37,13 @@
3637
public class DefaultODPApiManager implements ODPApiManager {
3738
private static final Logger logger = LoggerFactory.getLogger(DefaultODPApiManager.class);
3839

39-
private final OptimizelyHttpClient httpClientSegments;
40-
private final OptimizelyHttpClient httpClientEvents;
40+
@VisibleForTesting
41+
public final OptimizelyHttpClient httpClientSegments;
42+
@VisibleForTesting
43+
public final OptimizelyHttpClient httpClientEvents;
4144

4245
public DefaultODPApiManager() {
43-
this(OptimizelyHttpClient.builder().build());
46+
this(null);
4447
}
4548

4649
public DefaultODPApiManager(int segmentFetchTimeoutMillis, int eventDispatchTimeoutMillis) {
@@ -53,8 +56,11 @@ public DefaultODPApiManager(int segmentFetchTimeoutMillis, int eventDispatchTime
5356
}
5457
}
5558

56-
@VisibleForTesting
57-
DefaultODPApiManager(OptimizelyHttpClient httpClient) {
59+
public DefaultODPApiManager(@Nullable OptimizelyHttpClient customHttpClient) {
60+
OptimizelyHttpClient httpClient = customHttpClient;
61+
if (httpClient == null) {
62+
httpClient = OptimizelyHttpClient.builder().build();
63+
}
5864
this.httpClientSegments = httpClient;
5965
this.httpClientEvents = httpClient;
6066
}

‎core-httpclient-impl/src/test/java/com/optimizely/ab/OptimizelyFactoryTest.java

+31-15
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,10 @@
2525
import com.optimizely.ab.event.BatchEventProcessor;
2626
import com.optimizely.ab.internal.PropertyUtils;
2727
import com.optimizely.ab.notification.NotificationCenter;
28-
import org.apache.http.HttpHost;
29-
import org.apache.http.conn.routing.HttpRoutePlanner;
28+
import com.optimizely.ab.odp.DefaultODPApiManager;
29+
import com.optimizely.ab.odp.ODPManager;
3030
import org.apache.http.impl.client.CloseableHttpClient;
31-
import org.apache.http.impl.client.HttpClientBuilder;
3231
import org.apache.http.impl.client.HttpClients;
33-
import org.apache.http.impl.conn.DefaultProxyRoutePlanner;
3432
import org.junit.After;
3533
import org.junit.Before;
3634
import org.junit.Test;
@@ -247,21 +245,39 @@ public void newDefaultInstanceWithDatafileAccessToken() throws Exception {
247245

248246
@Test
249247
public void newDefaultInstanceWithDatafileAccessTokenAndCustomHttpClient() throws Exception {
250-
// Add custom Proxy and Port here
251-
int port = 443;
252-
String proxyHostName = "someProxy.com";
253-
HttpHost proxyHost = new HttpHost(proxyHostName, port);
248+
CloseableHttpClient customHttpClient = HttpClients.custom().build();
254249

255-
HttpRoutePlanner routePlanner = new DefaultProxyRoutePlanner(proxyHost);
256-
257-
HttpClientBuilder clientBuilder = HttpClients.custom();
258-
clientBuilder = clientBuilder.setRoutePlanner(routePlanner);
259-
260-
CloseableHttpClient httpClient = clientBuilder.build();
261250
String datafileString = Resources.toString(Resources.getResource("valid-project-config-v4.json"), Charsets.UTF_8);
262-
optimizely = OptimizelyFactory.newDefaultInstance("sdk-key", datafileString, "auth-token", httpClient);
251+
optimizely = OptimizelyFactory.newDefaultInstance("sdk-key", datafileString, "auth-token", customHttpClient);
263252
assertTrue(optimizely.isValid());
253+
254+
// HttpProjectConfigManager should be using the customHttpClient
255+
256+
HttpProjectConfigManager projectConfigManager = (HttpProjectConfigManager) optimizely.projectConfigManager;
257+
assert(doesUseCustomHttpClient(projectConfigManager.httpClient, customHttpClient));
258+
259+
// AsyncEventHandler should be using the customHttpClient
260+
261+
BatchEventProcessor eventProcessor = (BatchEventProcessor) optimizely.eventProcessor;
262+
AsyncEventHandler eventHandler = (AsyncEventHandler)eventProcessor.eventHandler;
263+
assert(doesUseCustomHttpClient(eventHandler.httpClient, customHttpClient));
264+
265+
// ODPManager should be using the customHttpClient
266+
267+
ODPManager odpManager = optimizely.getODPManager();
268+
assert odpManager != null;
269+
DefaultODPApiManager odpApiManager = (DefaultODPApiManager) odpManager.getEventManager().apiManager;
270+
assert(doesUseCustomHttpClient(odpApiManager.httpClientSegments, customHttpClient));
271+
assert(doesUseCustomHttpClient(odpApiManager.httpClientEvents, customHttpClient));
272+
}
273+
274+
boolean doesUseCustomHttpClient(OptimizelyHttpClient optimizelyHttpClient, CloseableHttpClient customHttpClient) {
275+
if (optimizelyHttpClient == null) {
276+
return false;
277+
}
278+
return optimizelyHttpClient.getHttpClient() == customHttpClient;
264279
}
280+
265281
public ProjectConfigManager projectConfigManagerReturningNull = new ProjectConfigManager() {
266282
@Override
267283
public ProjectConfig getConfig() {

‎core-httpclient-impl/src/test/java/com/optimizely/ab/config/HttpProjectConfigManagerTest.java

-2
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
import com.google.common.io.Resources;
2121
import com.optimizely.ab.OptimizelyHttpClient;
2222
import org.apache.http.HttpHeaders;
23-
import org.apache.http.HttpResponse;
2423
import org.apache.http.ProtocolVersion;
2524
import org.apache.http.StatusLine;
2625
import org.apache.http.client.ClientProtocolException;
@@ -44,7 +43,6 @@
4443
import static java.util.concurrent.TimeUnit.SECONDS;
4544
import static java.util.concurrent.TimeUnit.MINUTES;
4645
import static org.junit.Assert.*;
47-
import static org.mockito.Matchers.any;
4846
import static org.mockito.Mockito.*;
4947

5048
@RunWith(MockitoJUnitRunner.class)

0 commit comments

Comments
 (0)