[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

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<OptimizelyDecideOption> defaultDecideOptions;
 
-    private final ProjectConfigManager projectConfigManager;
+    @VisibleForTesting
+    final ProjectConfigManager projectConfigManager;
 
     @Nullable
     private final OptimizelyConfigManager optimizelyConfigManager;

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<Object> eventQueue;
-    private final EventHandler eventHandler;
+    @VisibleForTesting
+    public final EventHandler eventHandler;
 
     final int batchSize;
     final long flushInterval;

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.

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;

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;
 

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()

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) {

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;

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
             );
         }
     }

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;
     }

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() {

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)