diff --git a/core-api/src/main/java/com/optimizely/ab/Optimizely.java b/core-api/src/main/java/com/optimizely/ab/Optimizely.java index e64882ed6..fede007d5 100644 --- a/core-api/src/main/java/com/optimizely/ab/Optimizely.java +++ b/core-api/src/main/java/com/optimizely/ab/Optimizely.java @@ -78,7 +78,6 @@ public class Optimizely implements AutoCloseable { private static final Logger logger = LoggerFactory.getLogger(Optimizely.class); final DecisionService decisionService; - @VisibleForTesting @Deprecated final EventHandler eventHandler; @VisibleForTesting @@ -88,7 +87,8 @@ public class Optimizely implements AutoCloseable { public final List defaultDecideOptions; - private final ProjectConfigManager projectConfigManager; + @VisibleForTesting + final ProjectConfigManager projectConfigManager; @Nullable private final OptimizelyConfigManager optimizelyConfigManager; diff --git a/core-api/src/main/java/com/optimizely/ab/event/BatchEventProcessor.java b/core-api/src/main/java/com/optimizely/ab/event/BatchEventProcessor.java index 740cfb8c3..4f31b37e8 100644 --- a/core-api/src/main/java/com/optimizely/ab/event/BatchEventProcessor.java +++ b/core-api/src/main/java/com/optimizely/ab/event/BatchEventProcessor.java @@ -16,6 +16,7 @@ */ package com.optimizely.ab.event; +import com.optimizely.ab.annotations.VisibleForTesting; import com.optimizely.ab.config.ProjectConfig; import com.optimizely.ab.event.internal.EventFactory; import com.optimizely.ab.event.internal.UserEvent; @@ -58,7 +59,8 @@ public class BatchEventProcessor implements EventProcessor, AutoCloseable { private static final Object FLUSH_SIGNAL = new Object(); private final BlockingQueue eventQueue; - private final EventHandler eventHandler; + @VisibleForTesting + public final EventHandler eventHandler; final int batchSize; final long flushInterval; diff --git a/core-api/src/main/java/com/optimizely/ab/odp/ODPEventManager.java b/core-api/src/main/java/com/optimizely/ab/odp/ODPEventManager.java index b50c16045..43727b501 100644 --- a/core-api/src/main/java/com/optimizely/ab/odp/ODPEventManager.java +++ b/core-api/src/main/java/com/optimizely/ab/odp/ODPEventManager.java @@ -53,8 +53,8 @@ public class ODPEventManager { // needs to see the change immediately. private volatile ODPConfig odpConfig; private EventDispatcherThread eventDispatcherThread; - - private final ODPApiManager apiManager; + @VisibleForTesting + public final ODPApiManager apiManager; // The eventQueue needs to be thread safe. We are not doing anything extra for thread safety here // because `LinkedBlockingQueue` itself is thread safe. diff --git a/core-api/src/main/java/com/optimizely/ab/odp/ODPManager.java b/core-api/src/main/java/com/optimizely/ab/odp/ODPManager.java index 4f1ddc52d..3a47e3f04 100644 --- a/core-api/src/main/java/com/optimizely/ab/odp/ODPManager.java +++ b/core-api/src/main/java/com/optimizely/ab/odp/ODPManager.java @@ -21,7 +21,6 @@ import org.slf4j.LoggerFactory; import javax.annotation.Nonnull; -import java.util.Collections; import java.util.List; import java.util.Map; import java.util.Set; diff --git a/core-api/src/main/java/com/optimizely/ab/odp/ODPSegmentManager.java b/core-api/src/main/java/com/optimizely/ab/odp/ODPSegmentManager.java index 8cd917269..6caae29ca 100644 --- a/core-api/src/main/java/com/optimizely/ab/odp/ODPSegmentManager.java +++ b/core-api/src/main/java/com/optimizely/ab/odp/ODPSegmentManager.java @@ -16,6 +16,7 @@ */ package com.optimizely.ab.odp; +import com.optimizely.ab.annotations.VisibleForTesting; import com.optimizely.ab.internal.Cache; import com.optimizely.ab.internal.DefaultLRUCache; import com.optimizely.ab.odp.parser.ResponseJsonParser; @@ -31,8 +32,8 @@ public class ODPSegmentManager { private static final Logger logger = LoggerFactory.getLogger(ODPSegmentManager.class); private static final String SEGMENT_URL_PATH = "/v3/graphql"; - - private final ODPApiManager apiManager; + @VisibleForTesting + public final ODPApiManager apiManager; private volatile ODPConfig odpConfig; diff --git a/core-httpclient-impl/src/main/java/com/optimizely/ab/OptimizelyFactory.java b/core-httpclient-impl/src/main/java/com/optimizely/ab/OptimizelyFactory.java index 1c6ee2820..f26851375 100644 --- a/core-httpclient-impl/src/main/java/com/optimizely/ab/OptimizelyFactory.java +++ b/core-httpclient-impl/src/main/java/com/optimizely/ab/OptimizelyFactory.java @@ -279,23 +279,37 @@ public static Optimizely newDefaultInstance(String sdkKey, String fallback, Stri * @param customHttpClient Customizable CloseableHttpClient to build OptimizelyHttpClient. * @return A new Optimizely instance */ - public static Optimizely newDefaultInstance(String sdkKey, String fallback, String datafileAccessToken, CloseableHttpClient customHttpClient) { + public static Optimizely newDefaultInstance( + String sdkKey, + String fallback, + String datafileAccessToken, + CloseableHttpClient customHttpClient + ) { + OptimizelyHttpClient optimizelyHttpClient = customHttpClient == null ? null : new OptimizelyHttpClient(customHttpClient); + NotificationCenter notificationCenter = new NotificationCenter(); - OptimizelyHttpClient optimizelyHttpClient = new OptimizelyHttpClient(customHttpClient); - HttpProjectConfigManager.Builder builder; - builder = HttpProjectConfigManager.builder() + + HttpProjectConfigManager.Builder builder = HttpProjectConfigManager.builder() .withDatafile(fallback) .withNotificationCenter(notificationCenter) - .withOptimizelyHttpClient(customHttpClient == null ? null : optimizelyHttpClient) + .withOptimizelyHttpClient(optimizelyHttpClient) .withSdkKey(sdkKey); if (datafileAccessToken != null) { builder.withDatafileAccessToken(datafileAccessToken); } - return newDefaultInstance(builder.build(), notificationCenter); - } + ProjectConfigManager configManager = builder.build(); + + EventHandler eventHandler = AsyncEventHandler.builder() + .withOptimizelyHttpClient(optimizelyHttpClient) + .build(); + ODPApiManager odpApiManager = new DefaultODPApiManager(optimizelyHttpClient); + + return newDefaultInstance(configManager, notificationCenter, eventHandler, odpApiManager); + } + /** * Returns a new Optimizely instance based on preset configuration. * EventHandler - {@link AsyncEventHandler} @@ -329,6 +343,19 @@ public static Optimizely newDefaultInstance(ProjectConfigManager configManager, * @return A new Optimizely instance * */ public static Optimizely newDefaultInstance(ProjectConfigManager configManager, NotificationCenter notificationCenter, EventHandler eventHandler) { + return newDefaultInstance(configManager, notificationCenter, eventHandler, null); + } + + /** + * Returns a new Optimizely instance based on preset configuration. + * + * @param configManager The {@link ProjectConfigManager} supplied to Optimizely instance. + * @param notificationCenter The {@link ProjectConfigManager} supplied to Optimizely instance. + * @param eventHandler The {@link EventHandler} supplied to Optimizely instance. + * @param odpApiManager The {@link ODPApiManager} supplied to Optimizely instance. + * @return A new Optimizely instance + * */ + public static Optimizely newDefaultInstance(ProjectConfigManager configManager, NotificationCenter notificationCenter, EventHandler eventHandler, ODPApiManager odpApiManager) { if (notificationCenter == null) { notificationCenter = new NotificationCenter(); } @@ -338,9 +365,8 @@ public static Optimizely newDefaultInstance(ProjectConfigManager configManager, .withNotificationCenter(notificationCenter) .build(); - ODPApiManager defaultODPApiManager = new DefaultODPApiManager(); ODPManager odpManager = ODPManager.builder() - .withApiManager(defaultODPApiManager) + .withApiManager(odpApiManager != null ? odpApiManager : new DefaultODPApiManager()) .build(); return Optimizely.builder() diff --git a/core-httpclient-impl/src/main/java/com/optimizely/ab/OptimizelyHttpClient.java b/core-httpclient-impl/src/main/java/com/optimizely/ab/OptimizelyHttpClient.java index f4040276f..363bce59c 100644 --- a/core-httpclient-impl/src/main/java/com/optimizely/ab/OptimizelyHttpClient.java +++ b/core-httpclient-impl/src/main/java/com/optimizely/ab/OptimizelyHttpClient.java @@ -41,7 +41,6 @@ public class OptimizelyHttpClient implements Closeable { private static final Logger logger = LoggerFactory.getLogger(OptimizelyHttpClient.class); - private final CloseableHttpClient httpClient; OptimizelyHttpClient(CloseableHttpClient httpClient) { diff --git a/core-httpclient-impl/src/main/java/com/optimizely/ab/config/HttpProjectConfigManager.java b/core-httpclient-impl/src/main/java/com/optimizely/ab/config/HttpProjectConfigManager.java index a583eae98..095e32a67 100644 --- a/core-httpclient-impl/src/main/java/com/optimizely/ab/config/HttpProjectConfigManager.java +++ b/core-httpclient-impl/src/main/java/com/optimizely/ab/config/HttpProjectConfigManager.java @@ -61,7 +61,8 @@ public class HttpProjectConfigManager extends PollingProjectConfigManager { private static final Logger logger = LoggerFactory.getLogger(HttpProjectConfigManager.class); - private final OptimizelyHttpClient httpClient; + @VisibleForTesting + public final OptimizelyHttpClient httpClient; private final URI uri; private final String datafileAccessToken; private String datafileLastModified; diff --git a/core-httpclient-impl/src/main/java/com/optimizely/ab/event/AsyncEventHandler.java b/core-httpclient-impl/src/main/java/com/optimizely/ab/event/AsyncEventHandler.java index 539b8b642..434b29103 100644 --- a/core-httpclient-impl/src/main/java/com/optimizely/ab/event/AsyncEventHandler.java +++ b/core-httpclient-impl/src/main/java/com/optimizely/ab/event/AsyncEventHandler.java @@ -45,6 +45,7 @@ import java.util.concurrent.TimeUnit; import javax.annotation.CheckForNull; +import javax.annotation.Nullable; /** * {@link EventHandler} implementation that queues events and has a separate pool of threads responsible @@ -67,7 +68,8 @@ public class AsyncEventHandler implements EventHandler, AutoCloseable { private static final Logger logger = LoggerFactory.getLogger(AsyncEventHandler.class); private static final ProjectConfigResponseHandler EVENT_RESPONSE_HANDLER = new ProjectConfigResponseHandler(); - private final OptimizelyHttpClient httpClient; + @VisibleForTesting + public final OptimizelyHttpClient httpClient; private final ExecutorService workerExecutor; private final long closeTimeout; @@ -110,7 +112,15 @@ public AsyncEventHandler(int queueCapacity, int validateAfter, long closeTimeout, TimeUnit closeTimeoutUnit) { - this(queueCapacity, numWorkers, maxConnections, connectionsPerRoute, validateAfter, closeTimeout, closeTimeoutUnit, null); + this(queueCapacity, + numWorkers, + maxConnections, + connectionsPerRoute, + validateAfter, + closeTimeout, + closeTimeoutUnit, + null, + null); } public AsyncEventHandler(int queueCapacity, @@ -120,24 +130,27 @@ public AsyncEventHandler(int queueCapacity, int validateAfter, long closeTimeout, TimeUnit closeTimeoutUnit, + @Nullable OptimizelyHttpClient httpClient, @Nullable ThreadFactory threadFactory) { + if (httpClient != null) { + this.httpClient = httpClient; + } else { + maxConnections = validateInput("maxConnections", maxConnections, DEFAULT_MAX_CONNECTIONS); + connectionsPerRoute = validateInput("connectionsPerRoute", connectionsPerRoute, DEFAULT_MAX_PER_ROUTE); + validateAfter = validateInput("validateAfter", validateAfter, DEFAULT_VALIDATE_AFTER_INACTIVITY); + this.httpClient = OptimizelyHttpClient.builder() + .withMaxTotalConnections(maxConnections) + .withMaxPerRoute(connectionsPerRoute) + .withValidateAfterInactivity(validateAfter) + // infrequent event discards observed. staled connections force-closed after a long idle time. + .withEvictIdleConnections(1L, TimeUnit.MINUTES) + .build(); + } queueCapacity = validateInput("queueCapacity", queueCapacity, DEFAULT_QUEUE_CAPACITY); numWorkers = validateInput("numWorkers", numWorkers, DEFAULT_NUM_WORKERS); - maxConnections = validateInput("maxConnections", maxConnections, DEFAULT_MAX_CONNECTIONS); - connectionsPerRoute = validateInput("connectionsPerRoute", connectionsPerRoute, DEFAULT_MAX_PER_ROUTE); - validateAfter = validateInput("validateAfter", validateAfter, DEFAULT_VALIDATE_AFTER_INACTIVITY); - - this.httpClient = OptimizelyHttpClient.builder() - .withMaxTotalConnections(maxConnections) - .withMaxPerRoute(connectionsPerRoute) - .withValidateAfterInactivity(validateAfter) - // infrequent event discards observed. staled connections force-closed after a long idle time. - .withEvictIdleConnections(1L, TimeUnit.MINUTES) - .build(); NamedThreadFactory namedThreadFactory = new NamedThreadFactory("optimizely-event-dispatcher-thread-%s", true, threadFactory); - this.workerExecutor = new ThreadPoolExecutor(numWorkers, numWorkers, 0L, TimeUnit.MILLISECONDS, new ArrayBlockingQueue<>(queueCapacity), @@ -302,6 +315,7 @@ public static class Builder { int validateAfterInactivity = PropertyUtils.getInteger(CONFIG_VALIDATE_AFTER_INACTIVITY, DEFAULT_VALIDATE_AFTER_INACTIVITY); private long closeTimeout = Long.MAX_VALUE; private TimeUnit closeTimeoutUnit = TimeUnit.MILLISECONDS; + private OptimizelyHttpClient httpClient; public Builder withQueueCapacity(int queueCapacity) { if (queueCapacity <= 0) { @@ -344,6 +358,11 @@ public Builder withCloseTimeout(long closeTimeout, TimeUnit unit) { return this; } + public Builder withOptimizelyHttpClient(OptimizelyHttpClient httpClient) { + this.httpClient = httpClient; + return this; + } + public AsyncEventHandler build() { return new AsyncEventHandler( queueCapacity, @@ -352,7 +371,9 @@ public AsyncEventHandler build() { maxPerRoute, validateAfterInactivity, closeTimeout, - closeTimeoutUnit + closeTimeoutUnit, + httpClient, + null ); } } diff --git a/core-httpclient-impl/src/main/java/com/optimizely/ab/odp/DefaultODPApiManager.java b/core-httpclient-impl/src/main/java/com/optimizely/ab/odp/DefaultODPApiManager.java index 3a7ae3291..b733427de 100644 --- a/core-httpclient-impl/src/main/java/com/optimizely/ab/odp/DefaultODPApiManager.java +++ b/core-httpclient-impl/src/main/java/com/optimizely/ab/odp/DefaultODPApiManager.java @@ -27,6 +27,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import javax.annotation.Nullable; import java.io.IOException; import java.io.UnsupportedEncodingException; import java.util.Iterator; @@ -36,11 +37,13 @@ public class DefaultODPApiManager implements ODPApiManager { private static final Logger logger = LoggerFactory.getLogger(DefaultODPApiManager.class); - private final OptimizelyHttpClient httpClientSegments; - private final OptimizelyHttpClient httpClientEvents; + @VisibleForTesting + public final OptimizelyHttpClient httpClientSegments; + @VisibleForTesting + public final OptimizelyHttpClient httpClientEvents; public DefaultODPApiManager() { - this(OptimizelyHttpClient.builder().build()); + this(null); } public DefaultODPApiManager(int segmentFetchTimeoutMillis, int eventDispatchTimeoutMillis) { @@ -53,8 +56,11 @@ public DefaultODPApiManager(int segmentFetchTimeoutMillis, int eventDispatchTime } } - @VisibleForTesting - DefaultODPApiManager(OptimizelyHttpClient httpClient) { + public DefaultODPApiManager(@Nullable OptimizelyHttpClient customHttpClient) { + OptimizelyHttpClient httpClient = customHttpClient; + if (httpClient == null) { + httpClient = OptimizelyHttpClient.builder().build(); + } this.httpClientSegments = httpClient; this.httpClientEvents = httpClient; } diff --git a/core-httpclient-impl/src/test/java/com/optimizely/ab/OptimizelyFactoryTest.java b/core-httpclient-impl/src/test/java/com/optimizely/ab/OptimizelyFactoryTest.java index aaa3a67fa..a15085645 100644 --- a/core-httpclient-impl/src/test/java/com/optimizely/ab/OptimizelyFactoryTest.java +++ b/core-httpclient-impl/src/test/java/com/optimizely/ab/OptimizelyFactoryTest.java @@ -25,12 +25,10 @@ import com.optimizely.ab.event.BatchEventProcessor; import com.optimizely.ab.internal.PropertyUtils; import com.optimizely.ab.notification.NotificationCenter; -import org.apache.http.HttpHost; -import org.apache.http.conn.routing.HttpRoutePlanner; +import com.optimizely.ab.odp.DefaultODPApiManager; +import com.optimizely.ab.odp.ODPManager; import org.apache.http.impl.client.CloseableHttpClient; -import org.apache.http.impl.client.HttpClientBuilder; import org.apache.http.impl.client.HttpClients; -import org.apache.http.impl.conn.DefaultProxyRoutePlanner; import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -247,21 +245,39 @@ public void newDefaultInstanceWithDatafileAccessToken() throws Exception { @Test public void newDefaultInstanceWithDatafileAccessTokenAndCustomHttpClient() throws Exception { - // Add custom Proxy and Port here - int port = 443; - String proxyHostName = "someProxy.com"; - HttpHost proxyHost = new HttpHost(proxyHostName, port); + CloseableHttpClient customHttpClient = HttpClients.custom().build(); - HttpRoutePlanner routePlanner = new DefaultProxyRoutePlanner(proxyHost); - - HttpClientBuilder clientBuilder = HttpClients.custom(); - clientBuilder = clientBuilder.setRoutePlanner(routePlanner); - - CloseableHttpClient httpClient = clientBuilder.build(); String datafileString = Resources.toString(Resources.getResource("valid-project-config-v4.json"), Charsets.UTF_8); - optimizely = OptimizelyFactory.newDefaultInstance("sdk-key", datafileString, "auth-token", httpClient); + optimizely = OptimizelyFactory.newDefaultInstance("sdk-key", datafileString, "auth-token", customHttpClient); assertTrue(optimizely.isValid()); + + // HttpProjectConfigManager should be using the customHttpClient + + HttpProjectConfigManager projectConfigManager = (HttpProjectConfigManager) optimizely.projectConfigManager; + assert(doesUseCustomHttpClient(projectConfigManager.httpClient, customHttpClient)); + + // AsyncEventHandler should be using the customHttpClient + + BatchEventProcessor eventProcessor = (BatchEventProcessor) optimizely.eventProcessor; + AsyncEventHandler eventHandler = (AsyncEventHandler)eventProcessor.eventHandler; + assert(doesUseCustomHttpClient(eventHandler.httpClient, customHttpClient)); + + // ODPManager should be using the customHttpClient + + ODPManager odpManager = optimizely.getODPManager(); + assert odpManager != null; + DefaultODPApiManager odpApiManager = (DefaultODPApiManager) odpManager.getEventManager().apiManager; + assert(doesUseCustomHttpClient(odpApiManager.httpClientSegments, customHttpClient)); + assert(doesUseCustomHttpClient(odpApiManager.httpClientEvents, customHttpClient)); + } + + boolean doesUseCustomHttpClient(OptimizelyHttpClient optimizelyHttpClient, CloseableHttpClient customHttpClient) { + if (optimizelyHttpClient == null) { + return false; + } + return optimizelyHttpClient.getHttpClient() == customHttpClient; } + public ProjectConfigManager projectConfigManagerReturningNull = new ProjectConfigManager() { @Override public ProjectConfig getConfig() { diff --git a/core-httpclient-impl/src/test/java/com/optimizely/ab/config/HttpProjectConfigManagerTest.java b/core-httpclient-impl/src/test/java/com/optimizely/ab/config/HttpProjectConfigManagerTest.java index 9cbc0bb01..77960d518 100644 --- a/core-httpclient-impl/src/test/java/com/optimizely/ab/config/HttpProjectConfigManagerTest.java +++ b/core-httpclient-impl/src/test/java/com/optimizely/ab/config/HttpProjectConfigManagerTest.java @@ -20,7 +20,6 @@ import com.google.common.io.Resources; import com.optimizely.ab.OptimizelyHttpClient; import org.apache.http.HttpHeaders; -import org.apache.http.HttpResponse; import org.apache.http.ProtocolVersion; import org.apache.http.StatusLine; import org.apache.http.client.ClientProtocolException; @@ -44,7 +43,6 @@ import static java.util.concurrent.TimeUnit.SECONDS; import static java.util.concurrent.TimeUnit.MINUTES; import static org.junit.Assert.*; -import static org.mockito.Matchers.any; import static org.mockito.Mockito.*; @RunWith(MockitoJUnitRunner.class) diff --git a/core-httpclient-impl/src/test/java/com/optimizely/ab/event/AsyncEventHandlerTest.java b/core-httpclient-impl/src/test/java/com/optimizely/ab/event/AsyncEventHandlerTest.java index 79a4105a1..f87811b96 100644 --- a/core-httpclient-impl/src/test/java/com/optimizely/ab/event/AsyncEventHandlerTest.java +++ b/core-httpclient-impl/src/test/java/com/optimizely/ab/event/AsyncEventHandlerTest.java @@ -22,14 +22,9 @@ import com.optimizely.ab.event.internal.payload.EventBatch; import org.apache.http.client.ResponseHandler; import org.apache.http.client.methods.HttpGet; -import org.apache.http.impl.client.CloseableHttpClient; -import org.junit.Rule; import org.junit.Test; -import org.junit.rules.ExpectedException; import org.junit.runner.RunWith; import org.mockito.Mock; -import org.mockito.junit.MockitoJUnit; -import org.mockito.junit.MockitoRule; import java.io.IOException; import java.util.HashMap; @@ -38,7 +33,6 @@ import java.util.concurrent.RejectedExecutionException; import java.util.concurrent.TimeUnit; -import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import org.mockito.runners.MockitoJUnitRunner; import static com.optimizely.ab.event.AsyncEventHandler.builder; @@ -124,6 +118,30 @@ public void testShutdownAndForcedTermination() throws Exception { verify(mockHttpClient).close(); } + @Test + public void testBuilderWithCustomHttpClient() { + OptimizelyHttpClient customHttpClient = OptimizelyHttpClient.builder().build(); + + AsyncEventHandler eventHandler = builder() + .withOptimizelyHttpClient(customHttpClient) + .withMaxTotalConnections(1) + .withMaxPerRoute(2) + .withCloseTimeout(10, TimeUnit.SECONDS) + .build(); + + assert eventHandler.httpClient == customHttpClient; + } + + @Test + public void testBuilderWithDefaultHttpClient() { + AsyncEventHandler eventHandler = builder() + .withMaxTotalConnections(3) + .withMaxPerRoute(4) + .withCloseTimeout(10, TimeUnit.SECONDS) + .build(); + assert(eventHandler.httpClient != null); + } + @Test public void testInvalidQueueCapacity() { AsyncEventHandler.Builder builder = builder(); diff --git a/core-httpclient-impl/src/test/java/com/optimizely/ab/odp/DefaultODPApiManagerTest.java b/core-httpclient-impl/src/test/java/com/optimizely/ab/odp/DefaultODPApiManagerTest.java index a268cacc7..780831ff2 100644 --- a/core-httpclient-impl/src/test/java/com/optimizely/ab/odp/DefaultODPApiManagerTest.java +++ b/core-httpclient-impl/src/test/java/com/optimizely/ab/odp/DefaultODPApiManagerTest.java @@ -33,7 +33,6 @@ import java.util.List; import static org.junit.Assert.*; -import static org.mockito.Matchers.any; import static org.mockito.Mockito.*; public class DefaultODPApiManagerTest {