Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FSSDK-10041] fix to inject common httpclient to projectConfigManager, eventHandler and odpManager #542

Merged
merged 7 commits into from
Apr 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions core-api/src/main/java/com/optimizely/ab/Optimizely.java
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
jaeopt marked this conversation as resolved.
Show resolved Hide resolved

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}
Expand Down Expand Up @@ -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();
}
Expand All @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
Expand Down Expand Up @@ -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,
Expand All @@ -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),
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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,
Expand All @@ -352,7 +371,9 @@ public AsyncEventHandler build() {
maxPerRoute,
validateAfterInactivity,
closeTimeout,
closeTimeoutUnit
closeTimeoutUnit,
httpClient,
null
);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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) {
Expand All @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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)
Expand Down
Loading
Loading