From 525ad995e35179e5cc517677eef374bf102ce12e Mon Sep 17 00:00:00 2001 From: zashraf1985 Date: Thu, 27 Oct 2022 10:25:04 -0700 Subject: [PATCH 01/11] Integrated ODPManager with UserContext and Optimizely --- .../java/com/optimizely/ab/Optimizely.java | 62 ++++++++--- .../optimizely/ab/OptimizelyUserContext.java | 26 ++++- .../ab/config/DatafileProjectConfig.java | 1 + .../optimizely/ab/config/ProjectConfig.java | 3 + .../com/optimizely/ab/odp/ODPApiManager.java | 4 +- .../java/com/optimizely/ab/odp/ODPConfig.java | 12 +- .../optimizely/ab/odp/ODPEventManager.java | 23 ++-- .../com/optimizely/ab/odp/ODPManager.java | 104 +++++++++++++++++- .../optimizely/ab/odp/ODPSegmentManager.java | 20 ++-- .../ab/odp/ODPEventManagerTest.java | 27 +++-- .../com/optimizely/ab/odp/ODPManagerTest.java | 34 +++--- .../ab/odp/ODPSegmentManagerTest.java | 52 +++++---- .../com/optimizely/ab/OptimizelyFactory.java | 9 ++ .../ab/odp/DefaultODPApiManager.java | 6 +- 14 files changed, 288 insertions(+), 95 deletions(-) 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 51335ec4f..f16d0759b 100644 --- a/core-api/src/main/java/com/optimizely/ab/Optimizely.java +++ b/core-api/src/main/java/com/optimizely/ab/Optimizely.java @@ -28,6 +28,8 @@ import com.optimizely.ab.event.internal.*; import com.optimizely.ab.event.internal.payload.EventBatch; import com.optimizely.ab.notification.*; +import com.optimizely.ab.odp.ODPEvent; +import com.optimizely.ab.odp.ODPManager; import com.optimizely.ab.optimizelyconfig.OptimizelyConfig; import com.optimizely.ab.optimizelyconfig.OptimizelyConfigManager; import com.optimizely.ab.optimizelyconfig.OptimizelyConfigService; @@ -96,6 +98,9 @@ public class Optimizely implements AutoCloseable { @Nullable private final UserProfileService userProfileService; + @Nullable + private final ODPManager odpManager; + private Optimizely(@Nonnull EventHandler eventHandler, @Nonnull EventProcessor eventProcessor, @Nonnull ErrorHandler errorHandler, @@ -104,7 +109,8 @@ private Optimizely(@Nonnull EventHandler eventHandler, @Nonnull ProjectConfigManager projectConfigManager, @Nullable OptimizelyConfigManager optimizelyConfigManager, @Nonnull NotificationCenter notificationCenter, - @Nonnull List defaultDecideOptions + @Nonnull List defaultDecideOptions, + @Nullable ODPManager odpManager ) { this.eventHandler = eventHandler; this.eventProcessor = eventProcessor; @@ -115,6 +121,15 @@ private Optimizely(@Nonnull EventHandler eventHandler, this.optimizelyConfigManager = optimizelyConfigManager; this.notificationCenter = notificationCenter; this.defaultDecideOptions = defaultDecideOptions; + this.odpManager = odpManager; + + if (odpManager != null) { + odpManager.getEventManager().start(); + if (getProjectConfig() != null) { + updateODPSettings(); + } + addUpdateConfigNotificationHandler(configNotification -> { updateODPSettings(); }); + } } /** @@ -128,8 +143,6 @@ public boolean isValid() { return getProjectConfig() != null; } - - /** * Checks if eventHandler {@link EventHandler} and projectConfigManager {@link ProjectConfigManager} * are Closeable {@link Closeable} and calls close on them. @@ -674,9 +687,9 @@ public OptimizelyJSON getFeatureVariableJSON(@Nonnull String featureKey, */ @Nullable public OptimizelyJSON getFeatureVariableJSON(@Nonnull String featureKey, - @Nonnull String variableKey, - @Nonnull String userId, - @Nonnull Map attributes) { + @Nonnull String variableKey, + @Nonnull String userId, + @Nonnull Map attributes) { return getFeatureVariableValueForType( featureKey, @@ -688,10 +701,10 @@ public OptimizelyJSON getFeatureVariableJSON(@Nonnull String featureKey, @VisibleForTesting T getFeatureVariableValueForType(@Nonnull String featureKey, - @Nonnull String variableKey, - @Nonnull String userId, - @Nonnull Map attributes, - @Nonnull String variableType) { + @Nonnull String variableKey, + @Nonnull String userId, + @Nonnull Map attributes, + @Nonnull String variableType) { if (featureKey == null) { logger.warn("The featureKey parameter must be nonnull."); return null; @@ -878,7 +891,7 @@ public OptimizelyJSON getAllFeatureVariables(@Nonnull String featureKey, } } else { logger.info("User \"{}\" was not bucketed into any variation for feature flag \"{}\". " + - "The default values are being returned.", userId, featureKey); + "The default values are being returned.", userId, featureKey); } Map valuesMap = new HashMap(); @@ -1142,7 +1155,7 @@ public OptimizelyConfig getOptimizelyConfig() { * @param userId The user ID to be used for bucketing. * @param attributes: A map of attribute names to current user attribute values. * @return An OptimizelyUserContext associated with this OptimizelyClient. - */ + */ public OptimizelyUserContext createUserContext(@Nonnull String userId, @Nonnull Map attributes) { if (userId == null) { @@ -1413,6 +1426,23 @@ public int addNotificationHandler(Class clazz, NotificationHandler han return notificationCenter.addNotificationHandler(clazz, handler); } + public ODPManager getODPManager() { + return odpManager; + } + + public void sendODPEvent(ODPEvent event) { + if (odpManager != null) { + odpManager.getEventManager().sendEvent(event); + } + } + + private void updateODPSettings() { + if (odpManager != null && getProjectConfig() != null) { + ProjectConfig projectConfig = getProjectConfig(); + odpManager.updateSettings(projectConfig.getHostForODP(), projectConfig.getPublicKeyForODP(), projectConfig.getAllSegments()); + } + } + //======== Builder ========// /** @@ -1467,6 +1497,7 @@ public static class Builder { private UserProfileService userProfileService; private NotificationCenter notificationCenter; private List defaultDecideOptions; + private ODPManager odpManager; // For backwards compatibility private AtomicProjectConfigManager fallbackConfigManager = new AtomicProjectConfigManager(); @@ -1562,6 +1593,11 @@ public Builder withDefaultDecideOptions(List defaultDeci return this; } + public Builder withODPManager(ODPManager odpManager) { + this.odpManager = odpManager; + return this; + } + // Helper functions for making testing easier protected Builder withBucketing(Bucketer bucketer) { this.bucketer = bucketer; @@ -1636,7 +1672,7 @@ public Optimizely build() { defaultDecideOptions = Collections.emptyList(); } - return new Optimizely(eventHandler, eventProcessor, errorHandler, decisionService, userProfileService, projectConfigManager, optimizelyConfigManager, notificationCenter, defaultDecideOptions); + return new Optimizely(eventHandler, eventProcessor, errorHandler, decisionService, userProfileService, projectConfigManager, optimizelyConfigManager, notificationCenter, defaultDecideOptions, odpManager); } } } diff --git a/core-api/src/main/java/com/optimizely/ab/OptimizelyUserContext.java b/core-api/src/main/java/com/optimizely/ab/OptimizelyUserContext.java index e59c4f3aa..6b0a596dc 100644 --- a/core-api/src/main/java/com/optimizely/ab/OptimizelyUserContext.java +++ b/core-api/src/main/java/com/optimizely/ab/OptimizelyUserContext.java @@ -17,6 +17,8 @@ package com.optimizely.ab; import com.optimizely.ab.config.Variation; +import com.optimizely.ab.odp.ODPManager; +import com.optimizely.ab.odp.ODPSegmentOption; import com.optimizely.ab.optimizelydecision.*; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -66,6 +68,11 @@ public OptimizelyUserContext(@Nonnull Optimizely optimizely, } this.qualifiedSegments = Collections.synchronizedList( qualifiedSegments == null ? new LinkedList<>(): qualifiedSegments); + + ODPManager odpManager = optimizely.getODPManager(); + if (odpManager != null) { + odpManager.getEventManager().identifyUser(userId); + } } public OptimizelyUserContext(@Nonnull Optimizely optimizely, @Nonnull String userId) { @@ -282,6 +289,24 @@ public void setQualifiedSegments(List qualifiedSegments) { this.qualifiedSegments.addAll(qualifiedSegments); } + public void fetchQualifiedSegments() { + fetchQualifiedSegments(false, false); + } + + public void fetchQualifiedSegments(@Nonnull Boolean ignoreCache, @Nonnull Boolean resetCache) { + ODPManager odpManager = optimizely.getODPManager(); + if (odpManager != null) { + List segmentOptions = new ArrayList<>(); + if (ignoreCache) { + segmentOptions.add(ODPSegmentOption.IGNORE_CACHE); + } + if (resetCache) { + segmentOptions.add(ODPSegmentOption.RESET_CACHE); + } + setQualifiedSegments(odpManager.getSegmentManager().getQualifiedSegments(userId, segmentOptions)); + } + } + // Utils @Override @@ -309,5 +334,4 @@ public String toString() { ", attributes='" + attributes + '\'' + '}'; } - } diff --git a/core-api/src/main/java/com/optimizely/ab/config/DatafileProjectConfig.java b/core-api/src/main/java/com/optimizely/ab/config/DatafileProjectConfig.java index 94341c717..28ad519a5 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/DatafileProjectConfig.java +++ b/core-api/src/main/java/com/optimizely/ab/config/DatafileProjectConfig.java @@ -434,6 +434,7 @@ public List getExperiments() { return experiments; } + @Override public Set getAllSegments() { return this.allSegments; } diff --git a/core-api/src/main/java/com/optimizely/ab/config/ProjectConfig.java b/core-api/src/main/java/com/optimizely/ab/config/ProjectConfig.java index be512bd04..2073be9ef 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/ProjectConfig.java +++ b/core-api/src/main/java/com/optimizely/ab/config/ProjectConfig.java @@ -24,6 +24,7 @@ import javax.annotation.Nullable; import java.util.List; import java.util.Map; +import java.util.Set; /** * ProjectConfig is an interface capturing the experiment, variation and feature definitions. @@ -69,6 +70,8 @@ Experiment getExperimentForKey(@Nonnull String experimentKey, List getExperiments(); + Set getAllSegments(); + List getExperimentsForEventKey(String eventKey); List getFeatureFlags(); diff --git a/core-api/src/main/java/com/optimizely/ab/odp/ODPApiManager.java b/core-api/src/main/java/com/optimizely/ab/odp/ODPApiManager.java index dee9413dd..6385d2b7b 100644 --- a/core-api/src/main/java/com/optimizely/ab/odp/ODPApiManager.java +++ b/core-api/src/main/java/com/optimizely/ab/odp/ODPApiManager.java @@ -15,10 +15,10 @@ */ package com.optimizely.ab.odp; -import java.util.List; +import java.util.Set; public interface ODPApiManager { - String fetchQualifiedSegments(String apiKey, String apiEndpoint, String userKey, String userValue, List segmentsToCheck); + String fetchQualifiedSegments(String apiKey, String apiEndpoint, String userKey, String userValue, Set segmentsToCheck); Integer sendEvents(String apiKey, String apiEndpoint, String eventPayload); } diff --git a/core-api/src/main/java/com/optimizely/ab/odp/ODPConfig.java b/core-api/src/main/java/com/optimizely/ab/odp/ODPConfig.java index 25402b172..eb055e63f 100644 --- a/core-api/src/main/java/com/optimizely/ab/odp/ODPConfig.java +++ b/core-api/src/main/java/com/optimizely/ab/odp/ODPConfig.java @@ -17,7 +17,7 @@ package com.optimizely.ab.odp; import java.util.Collections; -import java.util.List; +import java.util.Set; public class ODPConfig { @@ -25,16 +25,16 @@ public class ODPConfig { private String apiHost; - private List allSegments; + private Set allSegments; - public ODPConfig(String apiKey, String apiHost, List allSegments) { + public ODPConfig(String apiKey, String apiHost, Set allSegments) { this.apiKey = apiKey; this.apiHost = apiHost; this.allSegments = allSegments; } public ODPConfig(String apiKey, String apiHost) { - this(apiKey, apiHost, Collections.emptyList()); + this(apiKey, apiHost, Collections.emptySet()); } public synchronized Boolean isReady() { @@ -64,11 +64,11 @@ public synchronized String getApiHost() { return apiHost; } - public synchronized List getAllSegments() { + public synchronized Set getAllSegments() { return allSegments; } - public synchronized void setAllSegments(List allSegments) { + public synchronized void setAllSegments(Set allSegments) { this.allSegments = allSegments; } 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 ab4ce301e..c753aeaa8 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 @@ -52,12 +52,11 @@ public class ODPEventManager { // because `LinkedBlockingQueue` itself is thread safe. private final BlockingQueue eventQueue = new LinkedBlockingQueue<>(); - public ODPEventManager(@Nonnull ODPConfig odpConfig, @Nonnull ODPApiManager apiManager) { - this(odpConfig, apiManager, null, null, null); + public ODPEventManager(@Nonnull ODPApiManager apiManager) { + this(apiManager, null, null, null); } - public ODPEventManager(@Nonnull ODPConfig odpConfig, @Nonnull ODPApiManager apiManager, @Nullable Integer batchSize, @Nullable Integer queueSize, @Nullable Integer flushInterval) { - this.odpConfig = odpConfig; + public ODPEventManager(@Nonnull ODPApiManager apiManager, @Nullable Integer batchSize, @Nullable Integer queueSize, @Nullable Integer flushInterval) { this.apiManager = apiManager; this.batchSize = (batchSize != null && batchSize > 1) ? batchSize : DEFAULT_BATCH_SIZE; this.queueSize = queueSize != null ? queueSize : DEFAULT_QUEUE_SIZE; @@ -70,12 +69,16 @@ public void start() { eventDispatcherThread.start(); } - public void updateSettings(ODPConfig odpConfig) { - if (!this.odpConfig.equals(odpConfig) && eventQueue.offer(new FlushEvent(this.odpConfig))) { - this.odpConfig = odpConfig; + public void updateSettings(ODPConfig newConfig) { + if (odpConfig == null || (!odpConfig.equals(newConfig) && eventQueue.offer(new FlushEvent(odpConfig)))) { + odpConfig = newConfig; } } + public void identifyUser(String userId) { + identifyUser(null, userId); + } + public void identifyUser(@Nullable String vuid, String userId) { Map identifiers = new HashMap<>(); if (vuid != null) { @@ -111,7 +114,7 @@ private void processEvent(ODPEvent event) { return; } - if (!odpConfig.isReady()) { + if (odpConfig == null || !odpConfig.isReady()) { logger.debug("Unable to Process ODP Event. ODPConfig is not ready."); return; } @@ -188,6 +191,10 @@ public void run() { } private void flush(ODPConfig odpConfig) { + if (currentBatch.size() == 0) { + return; + } + if (odpConfig.isReady()) { String payload = ODPJsonSerializerFactory.getSerializer().serializeEvents(currentBatch); String endpoint = odpConfig.getApiHost() + EVENT_URL_PATH; 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 cb7e04f99..0f24d5616 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 @@ -16,20 +16,26 @@ */ package com.optimizely.ab.odp; +import com.optimizely.ab.internal.Cache; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import javax.annotation.Nonnull; import java.util.List; +import java.util.Set; public class ODPManager { + private static final Logger logger = LoggerFactory.getLogger(ODPManager.class); + private volatile ODPConfig odpConfig; private final ODPSegmentManager segmentManager; private final ODPEventManager eventManager; - public ODPManager(@Nonnull ODPConfig odpConfig, @Nonnull ODPApiManager apiManager) { - this(odpConfig, new ODPSegmentManager(odpConfig, apiManager), new ODPEventManager(odpConfig, apiManager)); + private ODPManager(@Nonnull ODPApiManager apiManager) { + this(new ODPSegmentManager(apiManager), new ODPEventManager(apiManager)); } - public ODPManager(@Nonnull ODPConfig odpConfig, @Nonnull ODPSegmentManager segmentManager, @Nonnull ODPEventManager eventManager) { - this.odpConfig = odpConfig; + private ODPManager(@Nonnull ODPSegmentManager segmentManager, @Nonnull ODPEventManager eventManager) { this.segmentManager = segmentManager; this.eventManager = eventManager; this.eventManager.start(); @@ -43,9 +49,10 @@ public ODPEventManager getEventManager() { return eventManager; } - public Boolean updateSettings(String apiHost, String apiKey, List allSegments) { + public Boolean updateSettings(String apiHost, String apiKey, Set allSegments) { ODPConfig newConfig = new ODPConfig(apiKey, apiHost, allSegments); - if (!odpConfig.equals(newConfig)) { + if (odpConfig == null || !odpConfig.equals(newConfig)) { + logger.debug("Updating ODP Config"); odpConfig = newConfig; eventManager.updateSettings(odpConfig); segmentManager.resetCache(); @@ -58,4 +65,89 @@ public Boolean updateSettings(String apiHost, String apiKey, List allSeg public void close() { eventManager.stop(); } + + public static Builder builder() { + return new Builder(); + } + + public static class Builder { + private ODPSegmentManager segmentManager; + private ODPEventManager eventManager; + private ODPApiManager apiManager; + private Integer cacheSize; + private Integer cacheTimeoutSeconds; + private Integer batchSize; + private Integer flushIntervalMillis; + private Cache> cacheImpl; + + public Builder withApiManager(ODPApiManager apiManager) { + this.apiManager = apiManager; + return this; + } + + public Builder withSegmentManager(ODPSegmentManager segmentManager) { + this.segmentManager = segmentManager; + return this; + } + + public Builder withEventManager(ODPEventManager eventManager) { + this.eventManager = eventManager; + return this; + } + + public Builder withSegmentCacheSize(Integer cacheSize) { + this.cacheSize = cacheSize; + return this; + } + + public Builder withSegmentCacheTimeout(Integer cacheTimeoutSeconds) { + this.cacheTimeoutSeconds = cacheTimeoutSeconds; + return this; + } + + public Builder withSegmentCache(Cache> cacheImpl) { + this.cacheImpl = cacheImpl; + return this; + } + + public Builder withEventBatchSize(Integer batchSize) { + this.batchSize = batchSize; + return this; + } + + public Builder withEventFlushInterval(Integer flushIntervalMillis) { + this.flushIntervalMillis = flushIntervalMillis; + return this; + } + + public ODPManager build() { + if ((segmentManager == null || eventManager == null) && apiManager == null) { + logger.warn("ApiManager instance is needed when using default EventManager or SegmentManager"); + return null; + } + + if (segmentManager == null) { + if (cacheImpl != null) { + segmentManager = new ODPSegmentManager(apiManager, cacheImpl); + } else if (cacheSize != null || cacheTimeoutSeconds != null) { + // Converting null to -1 so that DefaultCache uses the default values; + if (cacheSize == null) { + cacheSize = -1; + } + if (cacheTimeoutSeconds == null) { + cacheTimeoutSeconds = -1; + } + segmentManager = new ODPSegmentManager(apiManager, cacheSize, cacheTimeoutSeconds); + } else { + segmentManager = new ODPSegmentManager(apiManager); + } + } + + if (eventManager == null) { + eventManager = new ODPEventManager(apiManager, batchSize, null, flushIntervalMillis); + } + + return new ODPManager(segmentManager, eventManager); + } + } } 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 352c4ec8f..a6aa5a54d 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 @@ -38,19 +38,17 @@ public class ODPSegmentManager { private final Cache> segmentsCache; - public ODPSegmentManager(ODPConfig odpConfig, ODPApiManager apiManager) { - this(odpConfig, apiManager, Cache.DEFAULT_MAX_SIZE, Cache.DEFAULT_TIMEOUT_SECONDS); + public ODPSegmentManager(ODPApiManager apiManager) { + this(apiManager, Cache.DEFAULT_MAX_SIZE, Cache.DEFAULT_TIMEOUT_SECONDS); } - public ODPSegmentManager(ODPConfig odpConfig, ODPApiManager apiManager, Cache> cache) { + public ODPSegmentManager(ODPApiManager apiManager, Cache> cache) { this.apiManager = apiManager; - this.odpConfig = odpConfig; this.segmentsCache = cache; } - public ODPSegmentManager(ODPConfig odpConfig, ODPApiManager apiManager, Integer cacheSize, Integer cacheTimeoutSeconds) { + public ODPSegmentManager(ODPApiManager apiManager, Integer cacheSize, Integer cacheTimeoutSeconds) { this.apiManager = apiManager; - this.odpConfig = odpConfig; this.segmentsCache = new DefaultLRUCache<>(cacheSize, cacheTimeoutSeconds); } @@ -66,7 +64,7 @@ public List getQualifiedSegments(ODPUserKey userKey, String userValue) { } public List getQualifiedSegments(ODPUserKey userKey, String userValue, List options) { - if (!odpConfig.isReady()) { + if (odpConfig == null || !odpConfig.isReady()) { logger.error("Audience segments fetch failed (ODP is not enabled)"); return Collections.emptyList(); } @@ -93,7 +91,13 @@ public List getQualifiedSegments(ODPUserKey userKey, String userValue, L ResponseJsonParser parser = ResponseJsonParserFactory.getParser(); String qualifiedSegmentsResponse = apiManager.fetchQualifiedSegments(odpConfig.getApiKey(), odpConfig.getApiHost() + SEGMENT_URL_PATH, userKey.getKeyString(), userValue, odpConfig.getAllSegments()); - qualifiedSegments = parser.parseQualifiedSegments(qualifiedSegmentsResponse); + try { + qualifiedSegments = parser.parseQualifiedSegments(qualifiedSegmentsResponse); + } catch (Exception e) { + logger.debug("Audience segments fetch failed (Error Parsing Response)"); + logger.debug(e.getMessage()); + qualifiedSegments = Collections.emptyList(); + } if (qualifiedSegments != null && !options.contains(ODPSegmentOption.IGNORE_CACHE)) { segmentsCache.save(cacheKey, qualifiedSegments); diff --git a/core-api/src/test/java/com/optimizely/ab/odp/ODPEventManagerTest.java b/core-api/src/test/java/com/optimizely/ab/odp/ODPEventManagerTest.java index fd4287e0f..d6941671f 100644 --- a/core-api/src/test/java/com/optimizely/ab/odp/ODPEventManagerTest.java +++ b/core-api/src/test/java/com/optimizely/ab/odp/ODPEventManagerTest.java @@ -59,7 +59,8 @@ public void setup() { @Test public void logAndDiscardEventWhenEventManagerIsNotRunning() { ODPConfig odpConfig = new ODPConfig("key", "host", null); - ODPEventManager eventManager = new ODPEventManager(odpConfig, mockApiManager); + ODPEventManager eventManager = new ODPEventManager(mockApiManager); + eventManager.updateSettings(odpConfig); ODPEvent event = new ODPEvent("test-type", "test-action", Collections.emptyMap(), Collections.emptyMap()); eventManager.sendEvent(event); logbackVerifier.expectMessage(Level.WARN, "Failed to Process ODP Event. ODPEventManager is not running"); @@ -68,7 +69,8 @@ public void logAndDiscardEventWhenEventManagerIsNotRunning() { @Test public void logAndDiscardEventWhenODPConfigNotReady() { ODPConfig odpConfig = new ODPConfig(null, null, null); - ODPEventManager eventManager = new ODPEventManager(odpConfig, mockApiManager); + ODPEventManager eventManager = new ODPEventManager(mockApiManager); + eventManager.updateSettings(odpConfig); eventManager.start(); ODPEvent event = new ODPEvent("test-type", "test-action", Collections.emptyMap(), Collections.emptyMap()); eventManager.sendEvent(event); @@ -80,7 +82,8 @@ public void dispatchEventsInCorrectNumberOfBatches() throws InterruptedException Mockito.reset(mockApiManager); Mockito.when(mockApiManager.sendEvents(any(), any(), any())).thenReturn(202); ODPConfig odpConfig = new ODPConfig("key", "http://www.odp-host.com", null); - ODPEventManager eventManager = new ODPEventManager(odpConfig, mockApiManager); + ODPEventManager eventManager = new ODPEventManager(mockApiManager); + eventManager.updateSettings(odpConfig); eventManager.start(); for (int i = 0; i < 25; i++) { eventManager.sendEvent(getEvent(i)); @@ -95,7 +98,8 @@ public void dispatchEventsWithCorrectPayload() throws InterruptedException { Mockito.when(mockApiManager.sendEvents(any(), any(), any())).thenReturn(202); int batchSize = 2; ODPConfig odpConfig = new ODPConfig("key", "http://www.odp-host.com", null); - ODPEventManager eventManager = new ODPEventManager(odpConfig, mockApiManager, batchSize, null, null); + ODPEventManager eventManager = new ODPEventManager(mockApiManager, batchSize, null, null); + eventManager.updateSettings(odpConfig); eventManager.start(); for (int i = 0; i < 6; i++) { eventManager.sendEvent(getEvent(i)); @@ -126,7 +130,8 @@ public void dispatchEventsWithCorrectFlushInterval() throws InterruptedException Mockito.reset(mockApiManager); Mockito.when(mockApiManager.sendEvents(any(), any(), any())).thenReturn(202); ODPConfig odpConfig = new ODPConfig("key", "http://www.odp-host.com", null); - ODPEventManager eventManager = new ODPEventManager(odpConfig, mockApiManager); + ODPEventManager eventManager = new ODPEventManager(mockApiManager); + eventManager.updateSettings(odpConfig); eventManager.start(); for (int i = 0; i < 25; i++) { eventManager.sendEvent(getEvent(i)); @@ -144,7 +149,8 @@ public void retryFailedEvents() throws InterruptedException { Mockito.reset(mockApiManager); Mockito.when(mockApiManager.sendEvents(any(), any(), any())).thenReturn(500); ODPConfig odpConfig = new ODPConfig("key", "http://www.odp-host.com", null); - ODPEventManager eventManager = new ODPEventManager(odpConfig, mockApiManager); + ODPEventManager eventManager = new ODPEventManager(mockApiManager); + eventManager.updateSettings(odpConfig); eventManager.start(); for (int i = 0; i < 25; i++) { eventManager.sendEvent(getEvent(i)); @@ -164,7 +170,8 @@ public void shouldFlushAllScheduledEventsBeforeStopping() throws InterruptedExce Mockito.reset(mockApiManager); Mockito.when(mockApiManager.sendEvents(any(), any(), any())).thenReturn(202); ODPConfig odpConfig = new ODPConfig("key", "http://www.odp-host.com", null); - ODPEventManager eventManager = new ODPEventManager(odpConfig, mockApiManager); + ODPEventManager eventManager = new ODPEventManager(mockApiManager); + eventManager.updateSettings(odpConfig); eventManager.start(); for (int i = 0; i < 25; i++) { eventManager.sendEvent(getEvent(i)); @@ -181,7 +188,8 @@ public void prepareCorrectPayloadForIdentifyUser() throws InterruptedException { Mockito.when(mockApiManager.sendEvents(any(), any(), any())).thenReturn(202); int batchSize = 2; ODPConfig odpConfig = new ODPConfig("key", "http://www.odp-host.com", null); - ODPEventManager eventManager = new ODPEventManager(odpConfig, mockApiManager, batchSize, null, null); + ODPEventManager eventManager = new ODPEventManager(mockApiManager, batchSize, null, null); + eventManager.updateSettings(odpConfig); eventManager.start(); for (int i = 0; i < 2; i++) { eventManager.identifyUser("the-vuid-" + i, "the-fs-user-id-" + i); @@ -208,7 +216,8 @@ public void applyUpdatedODPConfigWhenAvailable() throws InterruptedException { Mockito.reset(mockApiManager); Mockito.when(mockApiManager.sendEvents(any(), any(), any())).thenReturn(202); ODPConfig odpConfig = new ODPConfig("key", "http://www.odp-host.com", null); - ODPEventManager eventManager = new ODPEventManager(odpConfig, mockApiManager); + ODPEventManager eventManager = new ODPEventManager(mockApiManager); + eventManager.updateSettings(odpConfig); eventManager.start(); for (int i = 0; i < 25; i++) { eventManager.sendEvent(getEvent(i)); diff --git a/core-api/src/test/java/com/optimizely/ab/odp/ODPManagerTest.java b/core-api/src/test/java/com/optimizely/ab/odp/ODPManagerTest.java index 924c88836..02abe88a1 100644 --- a/core-api/src/test/java/com/optimizely/ab/odp/ODPManagerTest.java +++ b/core-api/src/test/java/com/optimizely/ab/odp/ODPManagerTest.java @@ -22,6 +22,8 @@ import org.mockito.Mockito; import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; import static org.mockito.Matchers.*; import static org.mockito.Mockito.*; @@ -48,15 +50,15 @@ public void setup() { @Test public void shouldStartEventManagerWhenODPManagerIsInitialized() { - ODPConfig config = new ODPConfig("test-key", "test-host"); - new ODPManager(config, mockSegmentManager, mockEventManager); + ODPManager.builder().withSegmentManager(mockSegmentManager).withEventManager(mockEventManager).build(); + verify(mockEventManager, times(1)).start(); } @Test public void shouldStopEventManagerWhenCloseIsCalled() { - ODPConfig config = new ODPConfig("test-key", "test-host"); - ODPManager odpManager = new ODPManager(config, mockSegmentManager, mockEventManager); + ODPManager odpManager = ODPManager.builder().withSegmentManager(mockSegmentManager).withEventManager(mockEventManager).build(); + odpManager.updateSettings("test-key", "test-host", Collections.emptySet()); // Stop is not called in the default flow. verify(mockEventManager, times(0)).stop(); @@ -69,15 +71,15 @@ public void shouldStopEventManagerWhenCloseIsCalled() { @Test public void shouldUseNewSettingsInEventManagerWhenODPConfigIsUpdated() throws InterruptedException { Mockito.when(mockApiManager.sendEvents(any(), any(), any())).thenReturn(200); - ODPConfig config = new ODPConfig("test-key", "test-host", Arrays.asList("segment1", "segment2")); - ODPManager odpManager = new ODPManager(config, mockApiManager); + ODPManager odpManager = ODPManager.builder().withApiManager(mockApiManager).build(); + odpManager.updateSettings("test-host", "test-key", new HashSet<>(Arrays.asList("segment1", "segment2"))); odpManager.getEventManager().identifyUser("vuid", "fsuid"); Thread.sleep(2000); verify(mockApiManager, times(1)) .sendEvents(eq("test-key"), eq("test-host/v3/events"), any()); - odpManager.updateSettings("test-host-updated", "test-key-updated", Arrays.asList("segment1")); + odpManager.updateSettings("test-host-updated", "test-key-updated", new HashSet<>(Arrays.asList("segment1"))); odpManager.getEventManager().identifyUser("vuid", "fsuid"); Thread.sleep(1200); verify(mockApiManager, times(1)) @@ -86,16 +88,16 @@ public void shouldUseNewSettingsInEventManagerWhenODPConfigIsUpdated() throws In @Test public void shouldUseNewSettingsInSegmentManagerWhenODPConfigIsUpdated() { - Mockito.when(mockApiManager.fetchQualifiedSegments(anyString(), anyString(), anyString(), anyString(), anyList())) + Mockito.when(mockApiManager.fetchQualifiedSegments(anyString(), anyString(), anyString(), anyString(), anySet())) .thenReturn(API_RESPONSE); - ODPConfig config = new ODPConfig("test-key", "test-host", Arrays.asList("segment1", "segment2")); - ODPManager odpManager = new ODPManager(config, mockApiManager); + ODPManager odpManager = ODPManager.builder().withApiManager(mockApiManager).build(); + odpManager.updateSettings("test-host", "test-key", new HashSet<>(Arrays.asList("segment1", "segment2"))); odpManager.getSegmentManager().getQualifiedSegments("test-id"); verify(mockApiManager, times(1)) .fetchQualifiedSegments(eq("test-key"), eq("test-host/v3/graphql"), any(), any(), any()); - odpManager.updateSettings("test-host-updated", "test-key-updated", Arrays.asList("segment1")); + odpManager.updateSettings("test-host-updated", "test-key-updated", new HashSet<>(Arrays.asList("segment1"))); odpManager.getSegmentManager().getQualifiedSegments("test-id"); verify(mockApiManager, times(1)) .fetchQualifiedSegments(eq("test-key-updated"), eq("test-host-updated/v3/graphql"), any(), any(), any()); @@ -103,21 +105,19 @@ public void shouldUseNewSettingsInSegmentManagerWhenODPConfigIsUpdated() { @Test public void shouldGetEventManager() { - ODPConfig config = new ODPConfig("test-key", "test-host"); - ODPManager odpManager = new ODPManager(config, mockSegmentManager, mockEventManager); + ODPManager odpManager = ODPManager.builder().withSegmentManager(mockSegmentManager).withEventManager(mockEventManager).build(); assertNotNull(odpManager.getEventManager()); - odpManager = new ODPManager(config, mockApiManager); + odpManager = ODPManager.builder().withApiManager(mockApiManager).build(); assertNotNull(odpManager.getEventManager()); } @Test public void shouldGetSegmentManager() { - ODPConfig config = new ODPConfig("test-key", "test-host"); - ODPManager odpManager = new ODPManager(config, mockSegmentManager, mockEventManager); + ODPManager odpManager = ODPManager.builder().withSegmentManager(mockSegmentManager).withEventManager(mockEventManager).build(); assertNotNull(odpManager.getSegmentManager()); - odpManager = new ODPManager(config, mockApiManager); + odpManager = ODPManager.builder().withApiManager(mockApiManager).build(); assertNotNull(odpManager.getSegmentManager()); } } diff --git a/core-api/src/test/java/com/optimizely/ab/odp/ODPSegmentManagerTest.java b/core-api/src/test/java/com/optimizely/ab/odp/ODPSegmentManagerTest.java index f784d53d0..9e4f12942 100644 --- a/core-api/src/test/java/com/optimizely/ab/odp/ODPSegmentManagerTest.java +++ b/core-api/src/test/java/com/optimizely/ab/odp/ODPSegmentManagerTest.java @@ -31,6 +31,7 @@ import java.util.Arrays; import java.util.Collections; +import java.util.HashSet; import java.util.List; public class ODPSegmentManagerTest { @@ -56,8 +57,9 @@ public void setup() { public void cacheHit() { Mockito.when(mockCache.lookup(any())).thenReturn(Arrays.asList("segment1-cached", "segment2-cached")); - ODPConfig odpConfig = new ODPConfig("testKey", "testHost", Arrays.asList("segment1", "segment2")); - ODPSegmentManager segmentManager = new ODPSegmentManager(odpConfig, mockApiManager, mockCache); + ODPConfig odpConfig = new ODPConfig("testKey", "testHost", new HashSet<>(Arrays.asList("segment1", "segment2"))); + ODPSegmentManager segmentManager = new ODPSegmentManager(mockApiManager, mockCache); + segmentManager.updateSettings(odpConfig); List segments = segmentManager.getQualifiedSegments(ODPUserKey.FS_USER_ID, "testId"); // Cache lookup called with correct key @@ -76,11 +78,12 @@ public void cacheHit() { @Test public void cacheMiss() { Mockito.when(mockCache.lookup(any())).thenReturn(null); - Mockito.when(mockApiManager.fetchQualifiedSegments(anyString(), anyString(), anyString(), anyString(), anyList())) + Mockito.when(mockApiManager.fetchQualifiedSegments(anyString(), anyString(), anyString(), anyString(), anySet())) .thenReturn(API_RESPONSE); - ODPConfig odpConfig = new ODPConfig("testKey", "testHost", Arrays.asList("segment1", "segment2")); - ODPSegmentManager segmentManager = new ODPSegmentManager(odpConfig, mockApiManager, mockCache); + ODPConfig odpConfig = new ODPConfig("testKey", "testHost", new HashSet<>(Arrays.asList("segment1", "segment2"))); + ODPSegmentManager segmentManager = new ODPSegmentManager(mockApiManager, mockCache); + segmentManager.updateSettings(odpConfig); List segments = segmentManager.getQualifiedSegments(ODPUserKey.VUID, "testId"); // Cache lookup called with correct key @@ -88,7 +91,7 @@ public void cacheMiss() { // Cache miss! Make api call and save to cache verify(mockApiManager, times(1)) - .fetchQualifiedSegments(odpConfig.getApiKey(), odpConfig.getApiHost() + "/v3/graphql", "vuid", "testId", Arrays.asList("segment1", "segment2")); + .fetchQualifiedSegments(odpConfig.getApiKey(), odpConfig.getApiHost() + "/v3/graphql", "vuid", "testId", new HashSet<>(Arrays.asList("segment1", "segment2"))); verify(mockCache, times(1)).save("vuid-$-testId", Arrays.asList("segment1", "segment2")); verify(mockCache, times(0)).reset(); @@ -100,11 +103,12 @@ public void cacheMiss() { @Test public void ignoreCache() { Mockito.when(mockCache.lookup(any())).thenReturn(Arrays.asList("segment1-cached", "segment2-cached")); - Mockito.when(mockApiManager.fetchQualifiedSegments(anyString(), anyString(), anyString(), anyString(), anyList())) + Mockito.when(mockApiManager.fetchQualifiedSegments(anyString(), anyString(), anyString(), anyString(), anySet())) .thenReturn(API_RESPONSE); - ODPConfig odpConfig = new ODPConfig("testKey", "testHost", Arrays.asList("segment1", "segment2")); - ODPSegmentManager segmentManager = new ODPSegmentManager(odpConfig, mockApiManager, mockCache); + ODPConfig odpConfig = new ODPConfig("testKey", "testHost", new HashSet<>(Arrays.asList("segment1", "segment2"))); + ODPSegmentManager segmentManager = new ODPSegmentManager(mockApiManager, mockCache); + segmentManager.updateSettings(odpConfig); List segments = segmentManager.getQualifiedSegments(ODPUserKey.FS_USER_ID, "testId", Collections.singletonList(ODPSegmentOption.IGNORE_CACHE)); // Cache Ignored! lookup should not be called @@ -112,7 +116,7 @@ public void ignoreCache() { // Cache Ignored! Make API Call but do NOT save because of cacheIgnore verify(mockApiManager, times(1)) - .fetchQualifiedSegments(odpConfig.getApiKey(), odpConfig.getApiHost() + "/v3/graphql", "fs_user_id", "testId", Arrays.asList("segment1", "segment2")); + .fetchQualifiedSegments(odpConfig.getApiKey(), odpConfig.getApiHost() + "/v3/graphql", "fs_user_id", "testId", new HashSet<>(Arrays.asList("segment1", "segment2"))); verify(mockCache, times(0)).save(any(), any()); verify(mockCache, times(0)).reset(); @@ -122,22 +126,23 @@ public void ignoreCache() { @Test public void resetCache() { Mockito.when(mockCache.lookup(any())).thenReturn(Arrays.asList("segment1-cached", "segment2-cached")); - Mockito.when(mockApiManager.fetchQualifiedSegments(anyString(), anyString(), anyString(), anyString(), anyList())) + Mockito.when(mockApiManager.fetchQualifiedSegments(anyString(), anyString(), anyString(), anyString(), anySet())) .thenReturn(API_RESPONSE); - ODPConfig odpConfig = new ODPConfig("testKey", "testHost", Arrays.asList("segment1", "segment2")); - ODPSegmentManager segmentManager = new ODPSegmentManager(odpConfig, mockApiManager, mockCache); + ODPConfig odpConfig = new ODPConfig("testKey", "testHost", new HashSet<>(Arrays.asList("segment1", "segment2"))); + ODPSegmentManager segmentManager = new ODPSegmentManager(mockApiManager, mockCache); + segmentManager.updateSettings(odpConfig); List segments = segmentManager.getQualifiedSegments(ODPUserKey.FS_USER_ID, "testId", Collections.singletonList(ODPSegmentOption.RESET_CACHE)); // Call reset verify(mockCache, times(1)).reset(); - // Cache Reset! lookup should not be called becaues cache would be empty. + // Cache Reset! lookup should not be called because cache would be empty. verify(mockCache, times(0)).lookup(any()); // Cache reset but not Ignored! Make API Call and save to cache verify(mockApiManager, times(1)) - .fetchQualifiedSegments(odpConfig.getApiKey(), odpConfig.getApiHost() + "/v3/graphql", "fs_user_id", "testId", Arrays.asList("segment1", "segment2")); + .fetchQualifiedSegments(odpConfig.getApiKey(), odpConfig.getApiHost() + "/v3/graphql", "fs_user_id", "testId", new HashSet<>(Arrays.asList("segment1", "segment2"))); verify(mockCache, times(1)).save("fs_user_id-$-testId", Arrays.asList("segment1", "segment2")); assertEquals(Arrays.asList("segment1", "segment2"), segments); @@ -146,11 +151,12 @@ public void resetCache() { @Test public void resetAndIgnoreCache() { Mockito.when(mockCache.lookup(any())).thenReturn(Arrays.asList("segment1-cached", "segment2-cached")); - Mockito.when(mockApiManager.fetchQualifiedSegments(anyString(), anyString(), anyString(), anyString(), anyList())) + Mockito.when(mockApiManager.fetchQualifiedSegments(anyString(), anyString(), anyString(), anyString(), anySet())) .thenReturn(API_RESPONSE); - ODPConfig odpConfig = new ODPConfig("testKey", "testHost", Arrays.asList("segment1", "segment2")); - ODPSegmentManager segmentManager = new ODPSegmentManager(odpConfig, mockApiManager, mockCache); + ODPConfig odpConfig = new ODPConfig("testKey", "testHost", new HashSet<>(Arrays.asList("segment1", "segment2"))); + ODPSegmentManager segmentManager = new ODPSegmentManager(mockApiManager, mockCache); + segmentManager.updateSettings(odpConfig); List segments = segmentManager .getQualifiedSegments(ODPUserKey.FS_USER_ID, "testId", Arrays.asList(ODPSegmentOption.RESET_CACHE, ODPSegmentOption.IGNORE_CACHE)); @@ -161,7 +167,7 @@ public void resetAndIgnoreCache() { // Cache is also Ignored! Make API Call but do not save verify(mockApiManager, times(1)) - .fetchQualifiedSegments(odpConfig.getApiKey(), odpConfig.getApiHost() + "/v3/graphql", "fs_user_id", "testId", Arrays.asList("segment1", "segment2")); + .fetchQualifiedSegments(odpConfig.getApiKey(), odpConfig.getApiHost() + "/v3/graphql", "fs_user_id", "testId", new HashSet<>(Arrays.asList("segment1", "segment2"))); verify(mockCache, times(0)).save(any(), any()); assertEquals(Arrays.asList("segment1", "segment2"), segments); @@ -171,8 +177,9 @@ public void resetAndIgnoreCache() { public void odpConfigNotReady() { Mockito.when(mockCache.lookup(any())).thenReturn(Arrays.asList("segment1-cached", "segment2-cached")); - ODPConfig odpConfig = new ODPConfig(null, null, Arrays.asList("segment1", "segment2")); - ODPSegmentManager segmentManager = new ODPSegmentManager(odpConfig, mockApiManager, mockCache); + ODPConfig odpConfig = new ODPConfig(null, null, new HashSet<>(Arrays.asList("segment1", "segment2"))); + ODPSegmentManager segmentManager = new ODPSegmentManager(mockApiManager, mockCache); + segmentManager.updateSettings(odpConfig); List segments = segmentManager.getQualifiedSegments(ODPUserKey.FS_USER_ID, "testId"); // No further methods should be called. @@ -191,7 +198,8 @@ public void noSegmentsInProject() { Mockito.when(mockCache.lookup(any())).thenReturn(Arrays.asList("segment1-cached", "segment2-cached")); ODPConfig odpConfig = new ODPConfig("testKey", "testHost", null); - ODPSegmentManager segmentManager = new ODPSegmentManager(odpConfig, mockApiManager, mockCache); + ODPSegmentManager segmentManager = new ODPSegmentManager(mockApiManager, mockCache); + segmentManager.updateSettings(odpConfig); List segments = segmentManager.getQualifiedSegments(ODPUserKey.FS_USER_ID, "testId"); // No further methods should be called. 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 2e888e9bb..37d56da03 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 @@ -23,6 +23,9 @@ import com.optimizely.ab.event.EventHandler; import com.optimizely.ab.internal.PropertyUtils; import com.optimizely.ab.notification.NotificationCenter; +import com.optimizely.ab.odp.DefaultODPApiManager; +import com.optimizely.ab.odp.ODPApiManager; +import com.optimizely.ab.odp.ODPManager; import org.apache.http.impl.client.CloseableHttpClient; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -319,10 +322,16 @@ public static Optimizely newDefaultInstance(ProjectConfigManager configManager, .withNotificationCenter(notificationCenter) .build(); + ODPApiManager defaultODPApiManager = new DefaultODPApiManager(); + ODPManager odpManager = ODPManager.builder() + .withApiManager(defaultODPApiManager) + .build(); + return Optimizely.builder() .withEventProcessor(eventProcessor) .withConfigManager(configManager) .withNotificationCenter(notificationCenter) + .withODPManager(odpManager) .build(); } } 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 07ee1b3eb..edfc673bb 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,7 +27,7 @@ import java.io.IOException; import java.io.UnsupportedEncodingException; -import java.util.List; +import java.util.Set; public class DefaultODPApiManager implements ODPApiManager { private static final Logger logger = LoggerFactory.getLogger(DefaultODPApiManager.class); @@ -44,7 +44,7 @@ public DefaultODPApiManager() { } @VisibleForTesting - String getSegmentsStringForRequest(List segmentsList) { + String getSegmentsStringForRequest(Set segmentsList) { StringBuilder segmentsString = new StringBuilder(); for (int i = 0; i < segmentsList.size(); i++) { if (i > 0) { @@ -129,7 +129,7 @@ String getSegmentsStringForRequest(List segmentsList) { } */ @Override - public String fetchQualifiedSegments(String apiKey, String apiEndpoint, String userKey, String userValue, List segmentsToCheck) { + public String fetchQualifiedSegments(String apiKey, String apiEndpoint, String userKey, String userValue, Set segmentsToCheck) { HttpPost request = new HttpPost(apiEndpoint); String segmentsString = getSegmentsStringForRequest(segmentsToCheck); String requestPayload = String.format("{\"query\": \"query {customer(%s: \\\"%s\\\") {audiences(subset: [%s]) {edges {node {name state}}}}}\"}", userKey, userValue, segmentsString); From 5d3cb213e7a8acd4fd5b27077cc9dcb281be3ab2 Mon Sep 17 00:00:00 2001 From: zashraf1985 Date: Thu, 27 Oct 2022 10:51:11 -0700 Subject: [PATCH 02/11] fixed an issue in DefaultODPApiManager --- .../optimizely/ab/odp/DefaultODPApiManager.java | 5 ++++- .../ab/odp/DefaultODPApiManagerTest.java | 15 ++++++++------- 2 files changed, 12 insertions(+), 8 deletions(-) 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 edfc673bb..3ee3efbfc 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 java.io.IOException; import java.io.UnsupportedEncodingException; +import java.util.Iterator; import java.util.Set; public class DefaultODPApiManager implements ODPApiManager { @@ -45,12 +46,14 @@ public DefaultODPApiManager() { @VisibleForTesting String getSegmentsStringForRequest(Set segmentsList) { + StringBuilder segmentsString = new StringBuilder(); + Iterator segmentsListIterator = segmentsList.iterator(); for (int i = 0; i < segmentsList.size(); i++) { if (i > 0) { segmentsString.append(", "); } - segmentsString.append("\\\"").append(segmentsList.get(i)).append("\\\""); + segmentsString.append("\\\"").append(segmentsListIterator.next()).append("\\\""); } return segmentsString.toString(); } 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 77440f804..2574de661 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 @@ -30,6 +30,7 @@ import java.util.ArrayList; import java.util.Arrays; +import java.util.HashSet; import static org.junit.Assert.*; import static org.mockito.Matchers.any; @@ -65,29 +66,29 @@ private void setupHttpClient(int statusCode) throws Exception { public void generateCorrectSegmentsStringWhenListHasOneItem() { DefaultODPApiManager apiManager = new DefaultODPApiManager(); String expected = "\\\"only_segment\\\""; - String actual = apiManager.getSegmentsStringForRequest(Arrays.asList("only_segment")); + String actual = apiManager.getSegmentsStringForRequest(new HashSet<>(Arrays.asList("only_segment"))); assertEquals(expected, actual); } @Test public void generateCorrectSegmentsStringWhenListHasMultipleItems() { DefaultODPApiManager apiManager = new DefaultODPApiManager(); - String expected = "\\\"segment_1\\\", \\\"segment_2\\\", \\\"segment_3\\\""; - String actual = apiManager.getSegmentsStringForRequest(Arrays.asList("segment_1", "segment_2", "segment_3")); + String expected = "\\\"segment_1\\\", \\\"segment_3\\\", \\\"segment_2\\\""; + String actual = apiManager.getSegmentsStringForRequest(new HashSet<>(Arrays.asList("segment_1", "segment_2", "segment_3"))); assertEquals(expected, actual); } @Test public void generateEmptyStringWhenGivenListIsEmpty() { DefaultODPApiManager apiManager = new DefaultODPApiManager(); - String actual = apiManager.getSegmentsStringForRequest(new ArrayList<>()); + String actual = apiManager.getSegmentsStringForRequest(new HashSet<>()); assertEquals("", actual); } @Test public void generateCorrectRequestBody() throws Exception { ODPApiManager apiManager = new DefaultODPApiManager(mockHttpClient); - apiManager.fetchQualifiedSegments("key", "endPoint", "fs_user_id", "test_user", Arrays.asList("segment_1", "segment_2")); + apiManager.fetchQualifiedSegments("key", "endPoint", "fs_user_id", "test_user", new HashSet<>(Arrays.asList("segment_1", "segment_2"))); verify(mockHttpClient, times(1)).execute(any(HttpPost.class)); String expectedResponse = "{\"query\": \"query {customer(fs_user_id: \\\"test_user\\\") {audiences(subset: [\\\"segment_1\\\", \\\"segment_2\\\"]) {edges {node {name state}}}}}\"}"; @@ -99,7 +100,7 @@ public void generateCorrectRequestBody() throws Exception { @Test public void returnResponseStringWhenStatusIs200() throws Exception { ODPApiManager apiManager = new DefaultODPApiManager(mockHttpClient); - String responseString = apiManager.fetchQualifiedSegments("key", "endPoint", "fs_user_id", "test_user", Arrays.asList("segment_1", "segment_2")); + String responseString = apiManager.fetchQualifiedSegments("key", "endPoint", "fs_user_id", "test_user", new HashSet<>(Arrays.asList("segment_1", "segment_2"))); verify(mockHttpClient, times(1)).execute(any(HttpPost.class)); assertEquals(validResponse, responseString); } @@ -108,7 +109,7 @@ public void returnResponseStringWhenStatusIs200() throws Exception { public void returnNullWhenStatusIsNot200AndLogError() throws Exception { setupHttpClient(500); ODPApiManager apiManager = new DefaultODPApiManager(mockHttpClient); - String responseString = apiManager.fetchQualifiedSegments("key", "endPoint", "fs_user_id", "test_user", Arrays.asList("segment_1", "segment_2")); + String responseString = apiManager.fetchQualifiedSegments("key", "endPoint", "fs_user_id", "test_user", new HashSet<>(Arrays.asList("segment_1", "segment_2"))); verify(mockHttpClient, times(1)).execute(any(HttpPost.class)); logbackVerifier.expectMessage(Level.ERROR, "Unexpected response from ODP server, Response code: 500, null"); assertNull(responseString); From 0a1bf21b21951149492fe1eb08dd86cfdf864edc Mon Sep 17 00:00:00 2001 From: zashraf1985 Date: Thu, 27 Oct 2022 19:06:59 -0700 Subject: [PATCH 03/11] Calling close on ODPEventManager when Optimizely closes --- core-api/src/main/java/com/optimizely/ab/Optimizely.java | 3 +++ .../main/java/com/optimizely/ab/odp/ODPEventManager.java | 9 +++++++-- .../src/main/java/com/optimizely/ab/odp/ODPManager.java | 2 +- 3 files changed, 11 insertions(+), 3 deletions(-) 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 f16d0759b..94135ecfe 100644 --- a/core-api/src/main/java/com/optimizely/ab/Optimizely.java +++ b/core-api/src/main/java/com/optimizely/ab/Optimizely.java @@ -154,6 +154,9 @@ public void close() { tryClose(eventProcessor); tryClose(eventHandler); tryClose(projectConfigManager); + if (odpManager != null) { + tryClose(odpManager); + } } //======== activate calls ========// 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 c753aeaa8..b5f409850 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 @@ -64,9 +64,13 @@ public ODPEventManager(@Nonnull ODPApiManager apiManager, @Nullable Integer batc } public void start() { + if (eventDispatcherThread == null) { + eventDispatcherThread = new EventDispatcherThread(); + } + if (!isRunning) { + eventDispatcherThread.start(); + } isRunning = true; - eventDispatcherThread = new EventDispatcherThread(); - eventDispatcherThread.start(); } public void updateSettings(ODPConfig newConfig) { @@ -187,6 +191,7 @@ public void run() { } } + isRunning = false; logger.debug("Exiting ODP Event Dispatcher Thread."); } 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 0f24d5616..b7f8c98ac 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 @@ -24,7 +24,7 @@ import java.util.List; import java.util.Set; -public class ODPManager { +public class ODPManager implements AutoCloseable { private static final Logger logger = LoggerFactory.getLogger(ODPManager.class); private volatile ODPConfig odpConfig; From 0c74a18bfcd4470fc14c3a2852a52e779c8e022a Mon Sep 17 00:00:00 2001 From: zashraf1985 Date: Wed, 2 Nov 2022 09:44:23 -0700 Subject: [PATCH 04/11] Incorporated review feedback. --- .../java/com/optimizely/ab/Optimizely.java | 20 +++++- .../optimizely/ab/OptimizelyUserContext.java | 44 +++++++++---- .../com/optimizely/ab/odp/ODPManager.java | 62 +++++++++++++++---- .../optimizely/ab/odp/ODPSegmentManager.java | 6 +- .../ab/odp/ODPSegmentManagerTest.java | 2 +- 5 files changed, 104 insertions(+), 30 deletions(-) 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 94135ecfe..710c83412 100644 --- a/core-api/src/main/java/com/optimizely/ab/Optimizely.java +++ b/core-api/src/main/java/com/optimizely/ab/Optimizely.java @@ -1429,13 +1429,31 @@ public int addNotificationHandler(Class clazz, NotificationHandler han return notificationCenter.addNotificationHandler(clazz, handler); } + @Nullable public ODPManager getODPManager() { return odpManager; } - public void sendODPEvent(ODPEvent event) { + public void sendODPEvent(@Nonnull String type, @Nonnull String action, @Nullable Map identifiers, @Nullable Map data) { if (odpManager != null) { + ODPEvent event = new ODPEvent(type, action, identifiers, data); odpManager.getEventManager().sendEvent(event); + } else { + logger.error("ODP event send failed (ODP is not enabled)"); + } + } + + public void identifyUser(String userId) { + ODPManager odpManager = getODPManager(); + if (odpManager != null) { + odpManager.getEventManager().identifyUser(userId); + } + } + + public void identifyUser(@Nullable String vuid, String userId) { + ODPManager odpManager = getODPManager(); + if (odpManager != null) { + odpManager.getEventManager().identifyUser(vuid, userId); } } diff --git a/core-api/src/main/java/com/optimizely/ab/OptimizelyUserContext.java b/core-api/src/main/java/com/optimizely/ab/OptimizelyUserContext.java index 6b0a596dc..63472740f 100644 --- a/core-api/src/main/java/com/optimizely/ab/OptimizelyUserContext.java +++ b/core-api/src/main/java/com/optimizely/ab/OptimizelyUserContext.java @@ -56,6 +56,15 @@ public OptimizelyUserContext(@Nonnull Optimizely optimizely, @Nonnull Map attributes, @Nullable Map forcedDecisionsMap, @Nullable List qualifiedSegments) { + this(optimizely, userId, attributes, forcedDecisionsMap, qualifiedSegments, false); + } + + public OptimizelyUserContext(@Nonnull Optimizely optimizely, + @Nonnull String userId, + @Nonnull Map attributes, + @Nullable Map forcedDecisionsMap, + @Nullable List qualifiedSegments, + @Nullable Boolean shouldIdentifyUser) { this.optimizely = optimizely; this.userId = userId; if (attributes != null) { @@ -69,9 +78,8 @@ public OptimizelyUserContext(@Nonnull Optimizely optimizely, this.qualifiedSegments = Collections.synchronizedList( qualifiedSegments == null ? new LinkedList<>(): qualifiedSegments); - ODPManager odpManager = optimizely.getODPManager(); - if (odpManager != null) { - odpManager.getEventManager().identifyUser(userId); + if (shouldIdentifyUser == null || shouldIdentifyUser) { + optimizely.identifyUser(userId); } } @@ -289,21 +297,33 @@ public void setQualifiedSegments(List qualifiedSegments) { this.qualifiedSegments.addAll(qualifiedSegments); } + /** + * Fetch all qualified segments for the user context. + * + * The segments fetched will be saved and can be accessed at any time by calling {@link #getQualifiedSegments()}. + */ public void fetchQualifiedSegments() { - fetchQualifiedSegments(false, false); + ODPManager odpManager = optimizely.getODPManager(); + if (odpManager != null) { + synchronized (odpManager) { + setQualifiedSegments(odpManager.getSegmentManager().getQualifiedSegments(userId)); + } + } } - public void fetchQualifiedSegments(@Nonnull Boolean ignoreCache, @Nonnull Boolean resetCache) { + /** + * Fetch all qualified segments for the user context. + * + * The segments fetched will be saved and can be accessed at any time by calling {@link #getQualifiedSegments()}. + * + * @param segmentOptions A set of options for fetching qualified segments. + */ + public void fetchQualifiedSegments(@Nonnull List segmentOptions) { ODPManager odpManager = optimizely.getODPManager(); if (odpManager != null) { - List segmentOptions = new ArrayList<>(); - if (ignoreCache) { - segmentOptions.add(ODPSegmentOption.IGNORE_CACHE); - } - if (resetCache) { - segmentOptions.add(ODPSegmentOption.RESET_CACHE); + synchronized (odpManager) { + setQualifiedSegments(odpManager.getSegmentManager().getQualifiedSegments(userId, segmentOptions)); } - setQualifiedSegments(odpManager.getSegmentManager().getQualifiedSegments(userId, segmentOptions)); } } 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 b7f8c98ac..11e7b2fec 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 @@ -76,50 +76,86 @@ public static class Builder { private ODPApiManager apiManager; private Integer cacheSize; private Integer cacheTimeoutSeconds; - private Integer batchSize; - private Integer flushIntervalMillis; private Cache> cacheImpl; + /** + * Provide an custom {@link ODPManager} instance which makes http calls to fetch segments and send events. + * + * A Default ODPApiManager is available in `core-httpclient-impl` package. + * + * @param apiManager The implementation of {@link ODPManager} + * @return ODPManager builder + */ public Builder withApiManager(ODPApiManager apiManager) { this.apiManager = apiManager; return this; } + /** + * Provide an optional custom {@link ODPSegmentManager} instance. + * + * A Default {@link ODPSegmentManager} implementation is automatically used if none provided. + * + * @param segmentManager The implementation of {@link ODPSegmentManager} + * @return ODPManager builder + */ public Builder withSegmentManager(ODPSegmentManager segmentManager) { this.segmentManager = segmentManager; return this; } + /** + * Provide an optional custom {@link ODPEventManager} instance. + * + * A Default {@link ODPEventManager} implementation is automatically used if none provided. + * + * @param eventManager The implementation of {@link ODPEventManager} + * @return ODPManager builder + */ public Builder withEventManager(ODPEventManager eventManager) { this.eventManager = eventManager; return this; } + /** + * Provide an optional custom cache size + * + * A Default cache size is automatically used if none provided. + * + * @param cacheSize Custom cache size to be used. + * @return ODPManager builder + */ public Builder withSegmentCacheSize(Integer cacheSize) { this.cacheSize = cacheSize; return this; } + /** + * Provide an optional custom cache timeout. + * + * A Default cache timeout is automatically used if none provided. + * + * @param cacheTimeoutSeconds Custom cache timeout in seconds. + * @return ODPManager builder + */ public Builder withSegmentCacheTimeout(Integer cacheTimeoutSeconds) { this.cacheTimeoutSeconds = cacheTimeoutSeconds; return this; } + /** + * Provide an optional custom Segment Cache implementation. + * + * A Default LRU Cache implementation is automatically used if none provided. + * + * @param cacheImpl Customer Cache Implementation. + * @return ODPManager builder + */ public Builder withSegmentCache(Cache> cacheImpl) { this.cacheImpl = cacheImpl; return this; } - public Builder withEventBatchSize(Integer batchSize) { - this.batchSize = batchSize; - return this; - } - - public Builder withEventFlushInterval(Integer flushIntervalMillis) { - this.flushIntervalMillis = flushIntervalMillis; - return this; - } - public ODPManager build() { if ((segmentManager == null || eventManager == null) && apiManager == null) { logger.warn("ApiManager instance is needed when using default EventManager or SegmentManager"); @@ -144,7 +180,7 @@ public ODPManager build() { } if (eventManager == null) { - eventManager = new ODPEventManager(apiManager, batchSize, null, flushIntervalMillis); + eventManager = new ODPEventManager(apiManager); } return new ODPManager(segmentManager, eventManager); 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 a6aa5a54d..4a51b1375 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 @@ -66,7 +66,7 @@ public List getQualifiedSegments(ODPUserKey userKey, String userValue) { public List getQualifiedSegments(ODPUserKey userKey, String userValue, List options) { if (odpConfig == null || !odpConfig.isReady()) { logger.error("Audience segments fetch failed (ODP is not enabled)"); - return Collections.emptyList(); + return null; } if (!odpConfig.hasSegments()) { @@ -94,9 +94,9 @@ public List getQualifiedSegments(ODPUserKey userKey, String userValue, L try { qualifiedSegments = parser.parseQualifiedSegments(qualifiedSegmentsResponse); } catch (Exception e) { - logger.debug("Audience segments fetch failed (Error Parsing Response)"); + logger.error("Audience segments fetch failed (Error Parsing Response)"); logger.debug(e.getMessage()); - qualifiedSegments = Collections.emptyList(); + qualifiedSegments = null; } if (qualifiedSegments != null && !options.contains(ODPSegmentOption.IGNORE_CACHE)) { diff --git a/core-api/src/test/java/com/optimizely/ab/odp/ODPSegmentManagerTest.java b/core-api/src/test/java/com/optimizely/ab/odp/ODPSegmentManagerTest.java index 9e4f12942..42303e7af 100644 --- a/core-api/src/test/java/com/optimizely/ab/odp/ODPSegmentManagerTest.java +++ b/core-api/src/test/java/com/optimizely/ab/odp/ODPSegmentManagerTest.java @@ -190,7 +190,7 @@ public void odpConfigNotReady() { logbackVerifier.expectMessage(Level.ERROR, "Audience segments fetch failed (ODP is not enabled)"); - assertEquals(Collections.emptyList(), segments); + assertNull(segments); } @Test From 42b2425f8777e65effcc3db8476dfb20f992f063 Mon Sep 17 00:00:00 2001 From: zashraf1985 Date: Wed, 2 Nov 2022 16:10:08 -0700 Subject: [PATCH 05/11] Changed GraphQl query to use variables for better sanitization. --- .../java/com/optimizely/ab/odp/DefaultODPApiManager.java | 8 ++++++-- .../com/optimizely/ab/odp/DefaultODPApiManagerTest.java | 6 +++--- 2 files changed, 9 insertions(+), 5 deletions(-) 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 3ee3efbfc..576701066 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 @@ -53,7 +53,7 @@ String getSegmentsStringForRequest(Set segmentsList) { if (i > 0) { segmentsString.append(", "); } - segmentsString.append("\\\"").append(segmentsListIterator.next()).append("\\\""); + segmentsString.append("\"").append(segmentsListIterator.next()).append("\""); } return segmentsString.toString(); } @@ -135,7 +135,11 @@ String getSegmentsStringForRequest(Set segmentsList) { public String fetchQualifiedSegments(String apiKey, String apiEndpoint, String userKey, String userValue, Set segmentsToCheck) { HttpPost request = new HttpPost(apiEndpoint); String segmentsString = getSegmentsStringForRequest(segmentsToCheck); - String requestPayload = String.format("{\"query\": \"query {customer(%s: \\\"%s\\\") {audiences(subset: [%s]) {edges {node {name state}}}}}\"}", userKey, userValue, segmentsString); + + String query = String.format("query($userId: String, $audiences: [String]) {customer(%s: $userId) {audiences(subset: $audiences) {edges {node {name state}}}}}", userKey); + String variables = String.format("{\"userId\": \"%s\", \"audiences\": [%s]}", userValue, segmentsString); + String requestPayload = String.format("{\"query\": \"%s\", \"variables\": %s}", query, variables); + try { request.setEntity(new StringEntity(requestPayload)); } catch (UnsupportedEncodingException e) { 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 2574de661..638a3e1ee 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 @@ -65,7 +65,7 @@ private void setupHttpClient(int statusCode) throws Exception { @Test public void generateCorrectSegmentsStringWhenListHasOneItem() { DefaultODPApiManager apiManager = new DefaultODPApiManager(); - String expected = "\\\"only_segment\\\""; + String expected = "\"only_segment\""; String actual = apiManager.getSegmentsStringForRequest(new HashSet<>(Arrays.asList("only_segment"))); assertEquals(expected, actual); } @@ -73,7 +73,7 @@ public void generateCorrectSegmentsStringWhenListHasOneItem() { @Test public void generateCorrectSegmentsStringWhenListHasMultipleItems() { DefaultODPApiManager apiManager = new DefaultODPApiManager(); - String expected = "\\\"segment_1\\\", \\\"segment_3\\\", \\\"segment_2\\\""; + String expected = "\"segment_1\", \"segment_3\", \"segment_2\""; String actual = apiManager.getSegmentsStringForRequest(new HashSet<>(Arrays.asList("segment_1", "segment_2", "segment_3"))); assertEquals(expected, actual); } @@ -91,7 +91,7 @@ public void generateCorrectRequestBody() throws Exception { apiManager.fetchQualifiedSegments("key", "endPoint", "fs_user_id", "test_user", new HashSet<>(Arrays.asList("segment_1", "segment_2"))); verify(mockHttpClient, times(1)).execute(any(HttpPost.class)); - String expectedResponse = "{\"query\": \"query {customer(fs_user_id: \\\"test_user\\\") {audiences(subset: [\\\"segment_1\\\", \\\"segment_2\\\"]) {edges {node {name state}}}}}\"}"; + String expectedResponse = "{\"query\": \"query($userId: String, $audiences: [String]) {customer(fs_user_id: $userId) {audiences(subset: $audiences) {edges {node {name state}}}}}\", \"variables\": {\"userId\": \"test_user\", \"audiences\": [\"segment_1\", \"segment_2\"]}}"; ArgumentCaptor request = ArgumentCaptor.forClass(HttpPost.class); verify(mockHttpClient).execute(request.capture()); assertEquals(expectedResponse, EntityUtils.toString(request.getValue().getEntity())); From 79263fcdb925d4d2be772806d947ac3f2cc79907 Mon Sep 17 00:00:00 2001 From: zashraf1985 Date: Fri, 4 Nov 2022 12:41:59 -0700 Subject: [PATCH 06/11] Incorporated more review feedback. --- .../src/main/java/com/optimizely/ab/Optimizely.java | 6 +++--- .../java/com/optimizely/ab/OptimizelyUserContext.java | 11 +++-------- .../src/main/java/com/optimizely/ab/odp/ODPEvent.java | 6 ++++-- .../java/com/optimizely/ab/odp/ODPEventManager.java | 6 ++++-- .../main/java/com/optimizely/ab/odp/ODPManager.java | 2 +- 5 files changed, 15 insertions(+), 16 deletions(-) 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 710c83412..6e32bf556 100644 --- a/core-api/src/main/java/com/optimizely/ab/Optimizely.java +++ b/core-api/src/main/java/com/optimizely/ab/Optimizely.java @@ -1434,7 +1434,7 @@ public ODPManager getODPManager() { return odpManager; } - public void sendODPEvent(@Nonnull String type, @Nonnull String action, @Nullable Map identifiers, @Nullable Map data) { + public void sendODPEvent(@Nullable String type, @Nonnull String action, @Nullable Map identifiers, @Nullable Map data) { if (odpManager != null) { ODPEvent event = new ODPEvent(type, action, identifiers, data); odpManager.getEventManager().sendEvent(event); @@ -1443,14 +1443,14 @@ public void sendODPEvent(@Nonnull String type, @Nonnull String action, @Nullable } } - public void identifyUser(String userId) { + public void identifyUser(@Nonnull String userId) { ODPManager odpManager = getODPManager(); if (odpManager != null) { odpManager.getEventManager().identifyUser(userId); } } - public void identifyUser(@Nullable String vuid, String userId) { + public void identifyUser(@Nullable String vuid, @Nullable String userId) { ODPManager odpManager = getODPManager(); if (odpManager != null) { odpManager.getEventManager().identifyUser(vuid, userId); diff --git a/core-api/src/main/java/com/optimizely/ab/OptimizelyUserContext.java b/core-api/src/main/java/com/optimizely/ab/OptimizelyUserContext.java index 63472740f..bace41d73 100644 --- a/core-api/src/main/java/com/optimizely/ab/OptimizelyUserContext.java +++ b/core-api/src/main/java/com/optimizely/ab/OptimizelyUserContext.java @@ -56,7 +56,7 @@ public OptimizelyUserContext(@Nonnull Optimizely optimizely, @Nonnull Map attributes, @Nullable Map forcedDecisionsMap, @Nullable List qualifiedSegments) { - this(optimizely, userId, attributes, forcedDecisionsMap, qualifiedSegments, false); + this(optimizely, userId, attributes, forcedDecisionsMap, qualifiedSegments, true); } public OptimizelyUserContext(@Nonnull Optimizely optimizely, @@ -100,7 +100,7 @@ public Optimizely getOptimizely() { } public OptimizelyUserContext copy() { - return new OptimizelyUserContext(optimizely, userId, attributes, forcedDecisionsMap, qualifiedSegments); + return new OptimizelyUserContext(optimizely, userId, attributes, forcedDecisionsMap, qualifiedSegments, false); } /** @@ -303,12 +303,7 @@ public void setQualifiedSegments(List qualifiedSegments) { * The segments fetched will be saved and can be accessed at any time by calling {@link #getQualifiedSegments()}. */ public void fetchQualifiedSegments() { - ODPManager odpManager = optimizely.getODPManager(); - if (odpManager != null) { - synchronized (odpManager) { - setQualifiedSegments(odpManager.getSegmentManager().getQualifiedSegments(userId)); - } - } + fetchQualifiedSegments(Collections.emptyList()); } /** diff --git a/core-api/src/main/java/com/optimizely/ab/odp/ODPEvent.java b/core-api/src/main/java/com/optimizely/ab/odp/ODPEvent.java index de7001ca8..a4dd37f2b 100644 --- a/core-api/src/main/java/com/optimizely/ab/odp/ODPEvent.java +++ b/core-api/src/main/java/com/optimizely/ab/odp/ODPEvent.java @@ -22,13 +22,15 @@ import java.util.Map; public class ODPEvent { + public static final String EVENT_TYPE_FULLSTACK = "fullstack"; + private String type; private String action; private Map identifiers; private Map data; - public ODPEvent(@Nonnull String type, @Nonnull String action, @Nullable Map identifiers, @Nullable Map data) { - this.type = type; + public ODPEvent(@Nullable String type, @Nonnull String action, @Nullable Map identifiers, @Nullable Map data) { + this.type = type == null ? EVENT_TYPE_FULLSTACK : type; this.action = action; this.identifiers = identifiers != null ? identifiers : Collections.emptyMap(); this.data = data != null ? data : Collections.emptyMap(); 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 b5f409850..b2a9a658b 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 @@ -83,12 +83,14 @@ public void identifyUser(String userId) { identifyUser(null, userId); } - public void identifyUser(@Nullable String vuid, String userId) { + public void identifyUser(@Nullable String vuid, @Nullable String userId) { Map identifiers = new HashMap<>(); if (vuid != null) { identifiers.put(ODPUserKey.VUID.getKeyString(), vuid); } - identifiers.put(ODPUserKey.FS_USER_ID.getKeyString(), userId); + if (userId != null) { + identifiers.put(ODPUserKey.FS_USER_ID.getKeyString(), userId); + } ODPEvent event = new ODPEvent("fullstack", "client_initialized", identifiers, null); sendEvent(event); } 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 11e7b2fec..3e198a209 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 @@ -79,7 +79,7 @@ public static class Builder { private Cache> cacheImpl; /** - * Provide an custom {@link ODPManager} instance which makes http calls to fetch segments and send events. + * Provide a custom {@link ODPManager} instance which makes http calls to fetch segments and send events. * * A Default ODPApiManager is available in `core-httpclient-impl` package. * From e573a6ac81254961226ded321a8fe6b605939c15 Mon Sep 17 00:00:00 2001 From: zashraf1985 Date: Tue, 8 Nov 2022 10:11:56 -0800 Subject: [PATCH 07/11] Added tests for `Optimizely` --- .../com/optimizely/ab/OptimizelyTest.java | 137 +++++++++++++++++- 1 file changed, 136 insertions(+), 1 deletion(-) diff --git a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java b/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java index 2cab4a01e..e16ffdb02 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java @@ -34,6 +34,9 @@ import com.optimizely.ab.internal.ControlAttribute; import com.optimizely.ab.internal.LogbackVerifier; import com.optimizely.ab.notification.*; +import com.optimizely.ab.odp.ODPEvent; +import com.optimizely.ab.odp.ODPEventManager; +import com.optimizely.ab.odp.ODPManager; import com.optimizely.ab.optimizelydecision.DecisionResponse; import com.optimizely.ab.optimizelyjson.OptimizelyJSON; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; @@ -44,7 +47,9 @@ import org.junit.rules.RuleChain; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; +import org.mockito.ArgumentCaptor; import org.mockito.Mock; +import org.mockito.Mockito; import org.mockito.junit.MockitoJUnit; import org.mockito.junit.MockitoRule; @@ -179,10 +184,21 @@ public void testClose() throws Exception { withSettings().extraInterfaces(AutoCloseable.class) ); + ODPEventManager mockODPEventManager = mock(ODPEventManager.class); + Mockito.doNothing().when(mockODPEventManager).sendEvent(any()); + + ODPManager mockODPManager = mock( + ODPManager.class, + withSettings().extraInterfaces(AutoCloseable.class) + ); + + Mockito.when(mockODPManager.getEventManager()).thenReturn(mockODPEventManager); + Optimizely optimizely = Optimizely.builder() .withEventHandler(mockEventHandler) .withEventProcessor(mockEventProcessor) .withConfigManager(mockProjectConfigManager) + .withODPManager(mockODPManager) .build(); optimizely.close(); @@ -190,7 +206,7 @@ public void testClose() throws Exception { verify((AutoCloseable) mockEventHandler).close(); verify((AutoCloseable) mockProjectConfigManager).close(); verify((AutoCloseable) mockEventProcessor).close(); - + verify((AutoCloseable) mockODPManager).close(); } //======== activate tests ========// @@ -4666,4 +4682,123 @@ public void getFlagVariationByKey() throws IOException { assertEquals(variationKey, variation.getKey()); } + @Test + public void initODPManagerWithoutProjectConfig() { + ProjectConfigManager mockProjectConfigManager = mock(ProjectConfigManager.class); + ODPEventManager mockODPEventManager = mock(ODPEventManager.class); + ODPManager mockODPManager = mock(ODPManager.class); + + Mockito.when(mockODPManager.getEventManager()).thenReturn(mockODPEventManager); + Optimizely.builder() + .withConfigManager(mockProjectConfigManager) + .withODPManager(mockODPManager) + .build(); + + verify(mockODPEventManager).start(); + verify(mockODPManager, never()).updateSettings(any(), any(), any()); + } + + @Test + public void initODPManagerWithProjectConfig() throws IOException { + ODPEventManager mockODPEventManager = mock(ODPEventManager.class); + ODPManager mockODPManager = mock(ODPManager.class); + + Mockito.when(mockODPManager.getEventManager()).thenReturn(mockODPEventManager); + Optimizely.builder() + .withDatafile(validConfigJsonV4()) + .withODPManager(mockODPManager) + .build(); + + verify(mockODPEventManager).start(); + verify(mockODPManager, times(1)).updateSettings(any(), any(), any()); + } + + @Test + public void updateODPManagerWhenConfigUpdates() throws IOException { + ODPEventManager mockODPEventManager = mock(ODPEventManager.class); + ODPManager mockODPManager = mock(ODPManager.class); + NotificationCenter mockNotificationCenter = mock(NotificationCenter.class); + + Mockito.when(mockODPManager.getEventManager()).thenReturn(mockODPEventManager); + Optimizely.builder() + .withDatafile(validConfigJsonV4()) + .withNotificationCenter(mockNotificationCenter) + .withODPManager(mockODPManager) + .build(); + + verify(mockODPManager, times(1)).updateSettings(any(), any(), any()); + + Mockito.verify(mockNotificationCenter, times(1)).addNotificationHandler(any(), any()); + } + + @Test + public void sendODPEvent() { + ProjectConfigManager mockProjectConfigManager = mock(ProjectConfigManager.class); + ODPEventManager mockODPEventManager = mock(ODPEventManager.class); + ODPManager mockODPManager = mock(ODPManager.class); + + Mockito.when(mockODPManager.getEventManager()).thenReturn(mockODPEventManager); + Optimizely optimizely = Optimizely.builder() + .withConfigManager(mockProjectConfigManager) + .withODPManager(mockODPManager) + .build(); + + verify(mockODPEventManager).start(); + + Map identifiers = new HashMap<>(); + identifiers.put("id1", "value1"); + identifiers.put("id2", "value2"); + + Map data = new HashMap<>(); + data.put("sdk", "java"); + data.put("revision", 52); + + optimizely.sendODPEvent("fullstack", "identify", identifiers, data); + ArgumentCaptor eventArgument = ArgumentCaptor.forClass(ODPEvent.class); + verify(mockODPEventManager).sendEvent(eventArgument.capture()); + + assertEquals("fullstack", eventArgument.getValue().getType()); + assertEquals("identify", eventArgument.getValue().getAction()); + assertEquals(identifiers, eventArgument.getValue().getIdentifiers()); + assertEquals(data, eventArgument.getValue().getData()); + } + + @Test + public void sendODPEventError() { + ProjectConfigManager mockProjectConfigManager = mock(ProjectConfigManager.class); + + Optimizely optimizely = Optimizely.builder() + .withConfigManager(mockProjectConfigManager) + .build(); + + Map identifiers = new HashMap<>(); + identifiers.put("id1", "value1"); + identifiers.put("id2", "value2"); + + Map data = new HashMap<>(); + data.put("sdk", "java"); + data.put("revision", 52); + + optimizely.sendODPEvent("fullstack", "identify", identifiers, data); + logbackVerifier.expectMessage(Level.ERROR, "ODP event send failed (ODP is not enabled)"); + } + + @Test + public void identifyUser() { + ProjectConfigManager mockProjectConfigManager = mock(ProjectConfigManager.class); + ODPEventManager mockODPEventManager = mock(ODPEventManager.class); + ODPManager mockODPManager = mock(ODPManager.class); + + Mockito.when(mockODPManager.getEventManager()).thenReturn(mockODPEventManager); + Optimizely optimizely = Optimizely.builder() + .withConfigManager(mockProjectConfigManager) + .withODPManager(mockODPManager) + .build(); + + optimizely.identifyUser("the-user"); + Mockito.verify(mockODPEventManager, times(1)).identifyUser("the-user"); + + optimizely.identifyUser("the-vuid", "the-user"); + Mockito.verify(mockODPEventManager, times(1)).identifyUser("the-vuid", "the-user"); + } } From 6d88b86f77538586d876c2b9c5f409134f9aa306 Mon Sep 17 00:00:00 2001 From: zashraf1985 Date: Tue, 8 Nov 2022 16:18:26 -0800 Subject: [PATCH 08/11] Completed the unit tests. --- .../com/optimizely/ab/internal/Cache.java | 2 + .../ab/internal/DefaultLRUCache.java | 10 ++ .../optimizely/ab/odp/ODPSegmentManager.java | 8 ++ .../optimizely/ab/OptimizelyBuilderTest.java | 18 +++- .../ab/OptimizelyUserContextTest.java | 54 +++++++++++ .../ab/odp/ODPManagerBuilderTest.java | 96 +++++++++++++++++++ 6 files changed, 184 insertions(+), 4 deletions(-) create mode 100644 core-api/src/test/java/com/optimizely/ab/odp/ODPManagerBuilderTest.java diff --git a/core-api/src/main/java/com/optimizely/ab/internal/Cache.java b/core-api/src/main/java/com/optimizely/ab/internal/Cache.java index ba667ebd2..7aa279d68 100644 --- a/core-api/src/main/java/com/optimizely/ab/internal/Cache.java +++ b/core-api/src/main/java/com/optimizely/ab/internal/Cache.java @@ -22,4 +22,6 @@ public interface Cache { void save(String key, T value); T lookup(String key); void reset(); + Integer getMaxSize(); + Integer getCacheTimeoutSeconds(); } diff --git a/core-api/src/main/java/com/optimizely/ab/internal/DefaultLRUCache.java b/core-api/src/main/java/com/optimizely/ab/internal/DefaultLRUCache.java index a531c5c83..e085b2040 100644 --- a/core-api/src/main/java/com/optimizely/ab/internal/DefaultLRUCache.java +++ b/core-api/src/main/java/com/optimizely/ab/internal/DefaultLRUCache.java @@ -84,6 +84,16 @@ public void reset() { } } + @Override + public Integer getMaxSize() { + return maxSize; + } + + @Override + public Integer getCacheTimeoutSeconds() { + return (int) (timeoutMillis / 1000); + } + private class CacheEntity { public T value; public Long timestamp; 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 4a51b1375..b605999c4 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 @@ -110,6 +110,14 @@ private String getCacheKey(String userKey, String userValue) { return userKey + "-$-" + userValue; } + public Integer getCacheMaxSize() { + return segmentsCache.getMaxSize(); + } + + public Integer getCacheTimeoutSeconds() { + return segmentsCache.getCacheTimeoutSeconds(); + } + public void updateSettings(ODPConfig odpConfig) { this.odpConfig = odpConfig; } diff --git a/core-api/src/test/java/com/optimizely/ab/OptimizelyBuilderTest.java b/core-api/src/test/java/com/optimizely/ab/OptimizelyBuilderTest.java index 91bb19e18..79382a5b7 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyBuilderTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyBuilderTest.java @@ -20,11 +20,12 @@ import com.optimizely.ab.config.*; import com.optimizely.ab.error.ErrorHandler; import com.optimizely.ab.error.NoOpErrorHandler; -import com.optimizely.ab.event.BatchEventProcessor; import com.optimizely.ab.event.EventHandler; import com.optimizely.ab.event.LogEvent; import com.optimizely.ab.event.internal.BuildVersionInfo; import com.optimizely.ab.event.internal.payload.EventBatch; +import com.optimizely.ab.odp.ODPEventManager; +import com.optimizely.ab.odp.ODPManager; import com.optimizely.ab.optimizelydecision.OptimizelyDecideOption; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import org.junit.Rule; @@ -32,6 +33,7 @@ import org.junit.rules.ExpectedException; import org.mockito.ArgumentCaptor; import org.mockito.Mock; +import org.mockito.Mockito; import org.mockito.junit.MockitoJUnit; import org.mockito.junit.MockitoRule; @@ -43,10 +45,7 @@ import static org.hamcrest.CoreMatchers.instanceOf; import static org.hamcrest.CoreMatchers.is; import static org.junit.Assert.*; -import static org.mockito.Matchers.any; -import static org.mockito.Matchers.anyString; import static org.mockito.Mockito.*; -import static org.mockito.Mockito.never; /** * Tests for {@link Optimizely#builder(String, EventHandler)}. @@ -244,4 +243,15 @@ public void withClientInfo() throws Exception { assertEquals(argument.getValue().getEventBatch().getClientVersion(), "1.2.3"); } + @Test + public void withODPManager() { + ODPEventManager mockODPEventManager = mock(ODPEventManager.class); + ODPManager mockODPManager = mock(ODPManager.class); + + Mockito.when(mockODPManager.getEventManager()).thenReturn(mockODPEventManager); + Optimizely optimizely = Optimizely.builder() + .withODPManager(mockODPManager) + .build(); + assertEquals(mockODPManager, optimizely.getODPManager()); + } } diff --git a/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java b/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java index c196938c4..019e19fb0 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java @@ -23,8 +23,13 @@ import com.optimizely.ab.config.*; import com.optimizely.ab.config.parser.ConfigParseException; import com.optimizely.ab.event.ForwardingEventProcessor; +import com.optimizely.ab.event.internal.UserContext; import com.optimizely.ab.event.internal.payload.DecisionMetadata; import com.optimizely.ab.notification.NotificationCenter; +import com.optimizely.ab.odp.ODPEventManager; +import com.optimizely.ab.odp.ODPManager; +import com.optimizely.ab.odp.ODPSegmentManager; +import com.optimizely.ab.odp.ODPSegmentOption; import com.optimizely.ab.optimizelydecision.DecisionMessage; import com.optimizely.ab.optimizelydecision.DecisionResponse; import com.optimizely.ab.optimizelydecision.OptimizelyDecideOption; @@ -35,6 +40,7 @@ import org.junit.Before; import org.junit.Rule; import org.junit.Test; +import org.mockito.Mockito; import java.util.*; @@ -1610,6 +1616,54 @@ public void setForcedDecisionsAndCallDecideDeliveryRuleToDecision() { )); } /********************************************[END DECIDE TESTS WITH FDs]******************************************/ + + @Test + public void fetchQualifiedSegments() { + ODPEventManager mockODPEventManager = mock(ODPEventManager.class); + ODPSegmentManager mockODPSegmentManager = mock(ODPSegmentManager.class); + ODPManager mockODPManager = mock(ODPManager.class); + + Mockito.when(mockODPManager.getEventManager()).thenReturn(mockODPEventManager); + Mockito.when(mockODPManager.getSegmentManager()).thenReturn(mockODPSegmentManager); + + Optimizely optimizely = Optimizely.builder() + .withODPManager(mockODPManager) + .build(); + + OptimizelyUserContext userContext = optimizely.createUserContext("test-user"); + + userContext.fetchQualifiedSegments(); + verify(mockODPSegmentManager).getQualifiedSegments("test-user", Collections.emptyList()); + + userContext.fetchQualifiedSegments(Collections.singletonList(ODPSegmentOption.RESET_CACHE)); + verify(mockODPSegmentManager).getQualifiedSegments("test-user", Collections.singletonList(ODPSegmentOption.RESET_CACHE)); + } + + @Test + public void identifyUser() { + ODPEventManager mockODPEventManager = mock(ODPEventManager.class); + ODPSegmentManager mockODPSegmentManager = mock(ODPSegmentManager.class); + ODPManager mockODPManager = mock(ODPManager.class); + + Mockito.when(mockODPManager.getEventManager()).thenReturn(mockODPEventManager); + Mockito.when(mockODPManager.getSegmentManager()).thenReturn(mockODPSegmentManager); + + Optimizely optimizely = Optimizely.builder() + .withODPManager(mockODPManager) + .build(); + + OptimizelyUserContext userContext = optimizely.createUserContext("test-user"); + verify(mockODPEventManager).identifyUser("test-user"); + + Mockito.reset(mockODPEventManager); + OptimizelyUserContext userContextClone = userContext.copy(); + + // identifyUser should not be called the new userContext is created through copy + verify(mockODPEventManager, never()).identifyUser("test-user"); + + assertNotSame(userContextClone, userContext); + } + // utils Map createUserProfileMap(String experimentId, String variationId) { diff --git a/core-api/src/test/java/com/optimizely/ab/odp/ODPManagerBuilderTest.java b/core-api/src/test/java/com/optimizely/ab/odp/ODPManagerBuilderTest.java new file mode 100644 index 000000000..730e55d95 --- /dev/null +++ b/core-api/src/test/java/com/optimizely/ab/odp/ODPManagerBuilderTest.java @@ -0,0 +1,96 @@ +/** + * + * Copyright 2022, Optimizely + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.optimizely.ab.odp; + +import com.optimizely.ab.internal.Cache; +import org.junit.Test; + +import java.util.Arrays; +import java.util.HashSet; +import java.util.List; + +import static org.mockito.Matchers.*; +import static org.mockito.Mockito.*; +import static org.junit.Assert.*; + +public class ODPManagerBuilderTest { + + @Test + public void withApiManager() { + ODPApiManager mockApiManager = mock(ODPApiManager.class); + ODPManager odpManager = ODPManager.builder().withApiManager(mockApiManager).build(); + odpManager.updateSettings("test-host", "test-key", new HashSet<>(Arrays.asList("Segment-1", "Segment-2"))); + odpManager.getSegmentManager().getQualifiedSegments("test-user"); + verify(mockApiManager).fetchQualifiedSegments(any(), any(), any(), any(), any()); + } + + @Test + public void withSegmentManager() { + ODPSegmentManager mockSegmentManager = mock(ODPSegmentManager.class); + ODPEventManager mockEventManager = mock(ODPEventManager.class); + ODPManager odpManager = ODPManager.builder() + .withSegmentManager(mockSegmentManager) + .withEventManager(mockEventManager) + .build(); + assertSame(mockSegmentManager, odpManager.getSegmentManager()); + } + + @Test + public void withEventManager() { + ODPSegmentManager mockSegmentManager = mock(ODPSegmentManager.class); + ODPEventManager mockEventManager = mock(ODPEventManager.class); + ODPManager odpManager = ODPManager.builder() + .withSegmentManager(mockSegmentManager) + .withEventManager(mockEventManager) + .build(); + assertSame(mockEventManager, odpManager.getEventManager()); + } + + @Test + public void withSegmentCache() { + Cache> mockCache = mock(Cache.class); + ODPApiManager mockApiManager = mock(ODPApiManager.class); + ODPManager odpManager = ODPManager.builder() + .withApiManager(mockApiManager) + .withSegmentCache(mockCache) + .build(); + + odpManager.updateSettings("test-host", "test-key", new HashSet<>(Arrays.asList("Segment-1", "Segment-2"))); + odpManager.getSegmentManager().getQualifiedSegments("test-user"); + verify(mockCache).lookup("fs_user_id-$-test-user"); + } + + @Test + public void withSegmentCacheSize() { + ODPApiManager mockApiManager = mock(ODPApiManager.class); + ODPManager odpManager = ODPManager.builder() + .withApiManager(mockApiManager) + .withSegmentCacheSize(500) + .build(); + assertEquals(Integer.valueOf(500), odpManager.getSegmentManager().getCacheMaxSize()); + } + + @Test + public void withSegmentCacheTimeout() { + ODPApiManager mockApiManager = mock(ODPApiManager.class); + ODPManager odpManager = ODPManager.builder() + .withApiManager(mockApiManager) + .withSegmentCacheTimeout(53) + .build(); + assertEquals(Integer.valueOf(53), odpManager.getSegmentManager().getCacheTimeoutSeconds()); + } +} From 68aa5ffa5c7ee2fddeff77173c0d4e366a0fa5d0 Mon Sep 17 00:00:00 2001 From: zashraf1985 Date: Fri, 11 Nov 2022 12:19:44 -0800 Subject: [PATCH 09/11] Added Non blocking implementation for fetching segments. --- .../optimizely/ab/OptimizelyUserContext.java | 43 +++- .../optimizely/ab/odp/ODPSegmentCallback.java | 22 ++ .../optimizely/ab/odp/ODPSegmentManager.java | 43 ++++ .../ab/OptimizelyUserContextTest.java | 47 ++++- .../ab/odp/ODPSegmentManagerTest.java | 194 +++++++++++++++++- 5 files changed, 336 insertions(+), 13 deletions(-) create mode 100644 core-api/src/main/java/com/optimizely/ab/odp/ODPSegmentCallback.java diff --git a/core-api/src/main/java/com/optimizely/ab/OptimizelyUserContext.java b/core-api/src/main/java/com/optimizely/ab/OptimizelyUserContext.java index bace41d73..475882759 100644 --- a/core-api/src/main/java/com/optimizely/ab/OptimizelyUserContext.java +++ b/core-api/src/main/java/com/optimizely/ab/OptimizelyUserContext.java @@ -16,8 +16,9 @@ */ package com.optimizely.ab; -import com.optimizely.ab.config.Variation; import com.optimizely.ab.odp.ODPManager; +import com.optimizely.ab.odp.ODPSegmentCallback; +import com.optimizely.ab.odp.ODPSegmentManager; import com.optimizely.ab.odp.ODPSegmentOption; import com.optimizely.ab.optimizelydecision.*; import org.slf4j.Logger; @@ -299,7 +300,7 @@ public void setQualifiedSegments(List qualifiedSegments) { /** * Fetch all qualified segments for the user context. - * + *

* The segments fetched will be saved and can be accessed at any time by calling {@link #getQualifiedSegments()}. */ public void fetchQualifiedSegments() { @@ -308,7 +309,7 @@ public void fetchQualifiedSegments() { /** * Fetch all qualified segments for the user context. - * + *

* The segments fetched will be saved and can be accessed at any time by calling {@link #getQualifiedSegments()}. * * @param segmentOptions A set of options for fetching qualified segments. @@ -322,6 +323,42 @@ public void fetchQualifiedSegments(@Nonnull List segmentOption } } + /** + * Fetch all qualified segments for the user context in a non-blocking manner. This method will fetch segments + * in a separate thread and invoke the provided callback when results are available. + *

+ * The segments fetched will be saved and can be accessed at any time by calling {@link #getQualifiedSegments()}. + * + * @param callback A callback to invoke when results are available. + * @param segmentOptions A set of options for fetching qualified segments. + */ + public void fetchQualifiedSegments(ODPSegmentCallback callback, List segmentOptions) { + ODPManager odpManager = optimizely.getODPManager(); + if (odpManager == null) { + logger.error("Audience segments fetch failed (ODP is not enabled"); + callback.invokeCallback(); + } else { + odpManager.getSegmentManager().getQualifiedSegments(userId, segments -> { + if (segments != null) { + setQualifiedSegments(segments); + } + callback.invokeCallback(); + }, segmentOptions); + } + } + + /** + * Fetch all qualified segments for the user context in a non-blocking manner. This method will fetch segments + * in a separate thread and invoke the provided callback when results are available. + *

+ * The segments fetched will be saved and can be accessed at any time by calling {@link #getQualifiedSegments()}. + * + * @param callback A callback to invoke when results are available. + */ + public void fetchQualifiedSegments(ODPSegmentCallback callback) { + fetchQualifiedSegments(callback, Collections.emptyList()); + } + // Utils @Override diff --git a/core-api/src/main/java/com/optimizely/ab/odp/ODPSegmentCallback.java b/core-api/src/main/java/com/optimizely/ab/odp/ODPSegmentCallback.java new file mode 100644 index 000000000..0011094d1 --- /dev/null +++ b/core-api/src/main/java/com/optimizely/ab/odp/ODPSegmentCallback.java @@ -0,0 +1,22 @@ +/** + * + * Copyright 2022, Optimizely + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.optimizely.ab.odp; + +@FunctionalInterface +public interface ODPSegmentCallback { + void invokeCallback(); +} 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 b605999c4..c64c8b6a2 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 @@ -106,6 +106,23 @@ public List getQualifiedSegments(ODPUserKey userKey, String userValue, L return qualifiedSegments; } + public void getQualifiedSegments(ODPUserKey userKey, String userValue, ODPSegmentFetchCallback callback, List options) { + AsyncSegmentFetcher segmentFetcher = new AsyncSegmentFetcher(userKey, userValue, options, callback); + segmentFetcher.start(); + } + + public void getQualifiedSegments(ODPUserKey userKey, String userValue, ODPSegmentFetchCallback callback) { + getQualifiedSegments(userKey, userValue, callback, Collections.emptyList()); + } + + public void getQualifiedSegments(String fsUserId, ODPSegmentFetchCallback callback, List segmentOptions) { + getQualifiedSegments(ODPUserKey.FS_USER_ID, fsUserId, callback, segmentOptions); + } + + public void getQualifiedSegments(String fsUserId, ODPSegmentFetchCallback callback) { + getQualifiedSegments(ODPUserKey.FS_USER_ID, fsUserId, callback, Collections.emptyList()); + } + private String getCacheKey(String userKey, String userValue) { return userKey + "-$-" + userValue; } @@ -125,4 +142,30 @@ public void updateSettings(ODPConfig odpConfig) { public void resetCache() { segmentsCache.reset(); } + + @FunctionalInterface + public interface ODPSegmentFetchCallback { + void invokeCallback(List segments); + } + + private class AsyncSegmentFetcher extends Thread { + + private final ODPUserKey userKey; + private final String userValue; + private final List segmentOptions; + private final ODPSegmentFetchCallback callback; + + public AsyncSegmentFetcher(ODPUserKey userKey, String userValue, List segmentOptions, ODPSegmentFetchCallback callback) { + this.userKey = userKey; + this.userValue = userValue; + this.segmentOptions = segmentOptions; + this.callback = callback; + } + + @Override + public void run() { + List segments = getQualifiedSegments(userKey, userValue, segmentOptions); + callback.invokeCallback(segments); + } + } } diff --git a/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java b/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java index 019e19fb0..255cd01a2 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java @@ -23,15 +23,10 @@ import com.optimizely.ab.config.*; import com.optimizely.ab.config.parser.ConfigParseException; import com.optimizely.ab.event.ForwardingEventProcessor; -import com.optimizely.ab.event.internal.UserContext; import com.optimizely.ab.event.internal.payload.DecisionMetadata; import com.optimizely.ab.notification.NotificationCenter; -import com.optimizely.ab.odp.ODPEventManager; -import com.optimizely.ab.odp.ODPManager; -import com.optimizely.ab.odp.ODPSegmentManager; -import com.optimizely.ab.odp.ODPSegmentOption; +import com.optimizely.ab.odp.*; import com.optimizely.ab.optimizelydecision.DecisionMessage; -import com.optimizely.ab.optimizelydecision.DecisionResponse; import com.optimizely.ab.optimizelydecision.OptimizelyDecideOption; import com.optimizely.ab.optimizelydecision.OptimizelyDecision; import com.optimizely.ab.optimizelyjson.OptimizelyJSON; @@ -43,6 +38,7 @@ import org.mockito.Mockito; import java.util.*; +import java.util.concurrent.CountDownLatch; import static com.optimizely.ab.config.ValidProjectConfigV4.ATTRIBUTE_HOUSE_KEY; import static com.optimizely.ab.config.ValidProjectConfigV4.AUDIENCE_GRYFFINDOR_VALUE; @@ -1639,6 +1635,45 @@ public void fetchQualifiedSegments() { verify(mockODPSegmentManager).getQualifiedSegments("test-user", Collections.singletonList(ODPSegmentOption.RESET_CACHE)); } + @Test + public void fetchQualifiedSegmentsAsync() throws InterruptedException { + ODPEventManager mockODPEventManager = mock(ODPEventManager.class); + ODPSegmentManager mockODPSegmentManager = mock(ODPSegmentManager.class); + ODPManager mockODPManager = mock(ODPManager.class); + + doAnswer( + invocation -> { + ODPSegmentManager.ODPSegmentFetchCallback callback = invocation.getArgumentAt(1, ODPSegmentManager.ODPSegmentFetchCallback.class); + callback.invokeCallback(Arrays.asList("segment1", "segment2")); + return null; + } + ).when(mockODPSegmentManager).getQualifiedSegments(any(), (ODPSegmentManager.ODPSegmentFetchCallback) any(), any()); + Mockito.when(mockODPManager.getEventManager()).thenReturn(mockODPEventManager); + Mockito.when(mockODPManager.getSegmentManager()).thenReturn(mockODPSegmentManager); + + Optimizely optimizely = Optimizely.builder() + .withODPManager(mockODPManager) + .build(); + + OptimizelyUserContext userContext = optimizely.createUserContext("test-user"); + + CountDownLatch countDownLatch = new CountDownLatch(1); + userContext.fetchQualifiedSegments(countDownLatch::countDown); + + countDownLatch.await(); + verify(mockODPSegmentManager).getQualifiedSegments(eq("test-user"), any(ODPSegmentManager.ODPSegmentFetchCallback.class), eq(Collections.emptyList())); + assertEquals(Arrays.asList("segment1", "segment2"), userContext.getQualifiedSegments()); + + // reset qualified segments + userContext.setQualifiedSegments(Collections.emptyList()); + CountDownLatch countDownLatch2 = new CountDownLatch(1); + userContext.fetchQualifiedSegments(countDownLatch2::countDown, Collections.singletonList(ODPSegmentOption.RESET_CACHE)); + + countDownLatch2.await(); + verify(mockODPSegmentManager).getQualifiedSegments(eq("test-user"), any(ODPSegmentManager.ODPSegmentFetchCallback.class), eq(Collections.singletonList(ODPSegmentOption.RESET_CACHE))); + assertEquals(Arrays.asList("segment1", "segment2"), userContext.getQualifiedSegments()); + } + @Test public void identifyUser() { ODPEventManager mockODPEventManager = mock(ODPEventManager.class); diff --git a/core-api/src/test/java/com/optimizely/ab/odp/ODPSegmentManagerTest.java b/core-api/src/test/java/com/optimizely/ab/odp/ODPSegmentManagerTest.java index 42303e7af..4d34d49b9 100644 --- a/core-api/src/test/java/com/optimizely/ab/odp/ODPSegmentManagerTest.java +++ b/core-api/src/test/java/com/optimizely/ab/odp/ODPSegmentManagerTest.java @@ -29,10 +29,8 @@ import static org.mockito.Mockito.*; import static org.junit.Assert.*; -import java.util.Arrays; -import java.util.Collections; -import java.util.HashSet; -import java.util.List; +import java.util.*; +import java.util.concurrent.CountDownLatch; public class ODPSegmentManagerTest { @@ -212,4 +210,192 @@ public void noSegmentsInProject() { assertEquals(Collections.emptyList(), segments); } + + // Tests for Async version + + @Test + public void cacheHitAsync() throws InterruptedException { + CountDownLatch countDownLatch = new CountDownLatch(1); + Mockito.when(mockCache.lookup(any())).thenReturn(Arrays.asList("segment1-cached", "segment2-cached")); + + ODPConfig odpConfig = new ODPConfig("testKey", "testHost", new HashSet<>(Arrays.asList("segment1", "segment2"))); + ODPSegmentManager segmentManager = new ODPSegmentManager(mockApiManager, mockCache); + segmentManager.updateSettings(odpConfig); + segmentManager.getQualifiedSegments(ODPUserKey.FS_USER_ID, "testId", (segments) -> { + assertEquals(Arrays.asList("segment1-cached", "segment2-cached"), segments); + countDownLatch.countDown(); + }); + + countDownLatch.await(); + + // Cache lookup called with correct key + verify(mockCache, times(1)).lookup("fs_user_id-$-testId"); + + // Cache hit! No api call was made to the server. + verify(mockApiManager, times(0)).fetchQualifiedSegments(any(), any(), any(), any(), any()); + verify(mockCache, times(0)).save(any(), any()); + verify(mockCache, times(0)).reset(); + + logbackVerifier.expectMessage(Level.DEBUG, "ODP Cache Hit. Returning segments from Cache."); + } + + @Test + public void cacheMissAsync() throws InterruptedException { + CountDownLatch countDownLatch = new CountDownLatch(1); + Mockito.when(mockCache.lookup(any())).thenReturn(null); + Mockito.when(mockApiManager.fetchQualifiedSegments(anyString(), anyString(), anyString(), anyString(), anySet())) + .thenReturn(API_RESPONSE); + + ODPConfig odpConfig = new ODPConfig("testKey", "testHost", new HashSet<>(Arrays.asList("segment1", "segment2"))); + ODPSegmentManager segmentManager = new ODPSegmentManager(mockApiManager, mockCache); + segmentManager.updateSettings(odpConfig); + segmentManager.getQualifiedSegments(ODPUserKey.VUID, "testId", (segments) -> { + assertEquals(Arrays.asList("segment1", "segment2"), segments); + countDownLatch.countDown(); + }); + + countDownLatch.await(); + + // Cache lookup called with correct key + verify(mockCache, times(1)).lookup("vuid-$-testId"); + + // Cache miss! Make api call and save to cache + verify(mockApiManager, times(1)) + .fetchQualifiedSegments(odpConfig.getApiKey(), odpConfig.getApiHost() + "/v3/graphql", "vuid", "testId", new HashSet<>(Arrays.asList("segment1", "segment2"))); + verify(mockCache, times(1)).save("vuid-$-testId", Arrays.asList("segment1", "segment2")); + verify(mockCache, times(0)).reset(); + + logbackVerifier.expectMessage(Level.DEBUG, "ODP Cache Miss. Making a call to ODP Server."); + } + + @Test + public void ignoreCacheAsync() throws InterruptedException { + CountDownLatch countDownLatch = new CountDownLatch(1); + Mockito.when(mockCache.lookup(any())).thenReturn(Arrays.asList("segment1-cached", "segment2-cached")); + Mockito.when(mockApiManager.fetchQualifiedSegments(anyString(), anyString(), anyString(), anyString(), anySet())) + .thenReturn(API_RESPONSE); + + ODPConfig odpConfig = new ODPConfig("testKey", "testHost", new HashSet<>(Arrays.asList("segment1", "segment2"))); + ODPSegmentManager segmentManager = new ODPSegmentManager(mockApiManager, mockCache); + segmentManager.updateSettings(odpConfig); + segmentManager.getQualifiedSegments(ODPUserKey.FS_USER_ID, "testId", segments -> { + assertEquals(Arrays.asList("segment1", "segment2"), segments); + countDownLatch.countDown(); + }, Collections.singletonList(ODPSegmentOption.IGNORE_CACHE)); + + countDownLatch.await(); + + // Cache Ignored! lookup should not be called + verify(mockCache, times(0)).lookup(any()); + + // Cache Ignored! Make API Call but do NOT save because of cacheIgnore + verify(mockApiManager, times(1)) + .fetchQualifiedSegments(odpConfig.getApiKey(), odpConfig.getApiHost() + "/v3/graphql", "fs_user_id", "testId", new HashSet<>(Arrays.asList("segment1", "segment2"))); + verify(mockCache, times(0)).save(any(), any()); + verify(mockCache, times(0)).reset(); + } + + @Test + public void resetCacheAsync() throws InterruptedException { + CountDownLatch countDownLatch = new CountDownLatch(1); + Mockito.when(mockCache.lookup(any())).thenReturn(Arrays.asList("segment1-cached", "segment2-cached")); + Mockito.when(mockApiManager.fetchQualifiedSegments(anyString(), anyString(), anyString(), anyString(), anySet())) + .thenReturn(API_RESPONSE); + + ODPConfig odpConfig = new ODPConfig("testKey", "testHost", new HashSet<>(Arrays.asList("segment1", "segment2"))); + ODPSegmentManager segmentManager = new ODPSegmentManager(mockApiManager, mockCache); + segmentManager.updateSettings(odpConfig); + segmentManager.getQualifiedSegments(ODPUserKey.FS_USER_ID, "testId", segments -> { + assertEquals(Arrays.asList("segment1", "segment2"), segments); + countDownLatch.countDown(); + }, Collections.singletonList(ODPSegmentOption.RESET_CACHE)); + + countDownLatch.await(); + + // Call reset + verify(mockCache, times(1)).reset(); + + // Cache Reset! lookup should not be called because cache would be empty. + verify(mockCache, times(0)).lookup(any()); + + // Cache reset but not Ignored! Make API Call and save to cache + verify(mockApiManager, times(1)) + .fetchQualifiedSegments(odpConfig.getApiKey(), odpConfig.getApiHost() + "/v3/graphql", "fs_user_id", "testId", new HashSet<>(Arrays.asList("segment1", "segment2"))); + verify(mockCache, times(1)).save("fs_user_id-$-testId", Arrays.asList("segment1", "segment2")); + } + + @Test + public void resetAndIgnoreCacheAsync() throws InterruptedException { + CountDownLatch countDownLatch = new CountDownLatch(1); + Mockito.when(mockCache.lookup(any())).thenReturn(Arrays.asList("segment1-cached", "segment2-cached")); + Mockito.when(mockApiManager.fetchQualifiedSegments(anyString(), anyString(), anyString(), anyString(), anySet())) + .thenReturn(API_RESPONSE); + + ODPConfig odpConfig = new ODPConfig("testKey", "testHost", new HashSet<>(Arrays.asList("segment1", "segment2"))); + ODPSegmentManager segmentManager = new ODPSegmentManager(mockApiManager, mockCache); + segmentManager.updateSettings(odpConfig); + segmentManager.getQualifiedSegments(ODPUserKey.FS_USER_ID, "testId", segments -> { + assertEquals(Arrays.asList("segment1", "segment2"), segments); + countDownLatch.countDown(); + }, Arrays.asList(ODPSegmentOption.RESET_CACHE, ODPSegmentOption.IGNORE_CACHE)); + + countDownLatch.await(); + + // Call reset + verify(mockCache, times(1)).reset(); + + verify(mockCache, times(0)).lookup(any()); + + // Cache is also Ignored! Make API Call but do not save + verify(mockApiManager, times(1)) + .fetchQualifiedSegments(odpConfig.getApiKey(), odpConfig.getApiHost() + "/v3/graphql", "fs_user_id", "testId", new HashSet<>(Arrays.asList("segment1", "segment2"))); + verify(mockCache, times(0)).save(any(), any()); + } + + @Test + public void odpConfigNotReadyAsync() throws InterruptedException { + CountDownLatch countDownLatch = new CountDownLatch(1); + Mockito.when(mockCache.lookup(any())).thenReturn(Arrays.asList("segment1-cached", "segment2-cached")); + + ODPConfig odpConfig = new ODPConfig(null, null, new HashSet<>(Arrays.asList("segment1", "segment2"))); + ODPSegmentManager segmentManager = new ODPSegmentManager(mockApiManager, mockCache); + segmentManager.updateSettings(odpConfig); + segmentManager.getQualifiedSegments(ODPUserKey.FS_USER_ID, "testId", segments -> { + assertNull(segments); + countDownLatch.countDown(); + }); + + countDownLatch.await(); + // No further methods should be called. + verify(mockCache, times(0)).lookup("fs_user_id-$-testId"); + verify(mockApiManager, times(0)).fetchQualifiedSegments(any(), any(), any(), any(), any()); + verify(mockCache, times(0)).save(any(), any()); + verify(mockCache, times(0)).reset(); + + logbackVerifier.expectMessage(Level.ERROR, "Audience segments fetch failed (ODP is not enabled)"); + } + + @Test + public void noSegmentsInProjectAsync() throws InterruptedException { + CountDownLatch countDownLatch = new CountDownLatch(1); + Mockito.when(mockCache.lookup(any())).thenReturn(Arrays.asList("segment1-cached", "segment2-cached")); + + ODPConfig odpConfig = new ODPConfig("testKey", "testHost", null); + ODPSegmentManager segmentManager = new ODPSegmentManager(mockApiManager, mockCache); + segmentManager.updateSettings(odpConfig); + segmentManager.getQualifiedSegments(ODPUserKey.FS_USER_ID, "testId", segments -> { + assertEquals(Collections.emptyList(), segments); + countDownLatch.countDown(); + }); + + countDownLatch.await(); + + // No further methods should be called. + verify(mockCache, times(0)).lookup("fs_user_id-$-testId"); + verify(mockApiManager, times(0)).fetchQualifiedSegments(any(), any(), any(), any(), any()); + verify(mockCache, times(0)).save(any(), any()); + verify(mockCache, times(0)).reset(); + + logbackVerifier.expectMessage(Level.DEBUG, "No Segments are used in the project, Not Fetching segments. Returning empty list"); + } } From 29f2b096e5695d4ad0e03c783fe6202c335283d2 Mon Sep 17 00:00:00 2001 From: zashraf1985 Date: Wed, 16 Nov 2022 16:25:32 -0800 Subject: [PATCH 10/11] Incorporated review feedback --- .../java/com/optimizely/ab/Optimizely.java | 29 +++++++---- .../optimizely/ab/OptimizelyUserContext.java | 34 +++++-------- .../optimizely/ab/odp/ODPSegmentCallback.java | 2 +- .../optimizely/ab/odp/ODPSegmentManager.java | 4 +- .../com/optimizely/ab/OptimizelyTest.java | 3 -- .../ab/OptimizelyUserContextTest.java | 49 +++++++++++++++++-- 6 files changed, 80 insertions(+), 41 deletions(-) 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 6e32bf556..0fbacaa3b 100644 --- a/core-api/src/main/java/com/optimizely/ab/Optimizely.java +++ b/core-api/src/main/java/com/optimizely/ab/Optimizely.java @@ -28,8 +28,7 @@ import com.optimizely.ab.event.internal.*; import com.optimizely.ab.event.internal.payload.EventBatch; import com.optimizely.ab.notification.*; -import com.optimizely.ab.odp.ODPEvent; -import com.optimizely.ab.odp.ODPManager; +import com.optimizely.ab.odp.*; import com.optimizely.ab.optimizelyconfig.OptimizelyConfig; import com.optimizely.ab.optimizelyconfig.OptimizelyConfigManager; import com.optimizely.ab.optimizelyconfig.OptimizelyConfigService; @@ -1429,6 +1428,25 @@ public int addNotificationHandler(Class clazz, NotificationHandler han return notificationCenter.addNotificationHandler(clazz, handler); } + public List fetchQualifiedSegments(String userId, @Nonnull List segmentOptions) { + if (odpManager != null) { + synchronized (odpManager) { + return odpManager.getSegmentManager().getQualifiedSegments(userId, segmentOptions); + } + } + logger.error("Audience segments fetch failed (ODP is not enabled)."); + return null; + } + + public void fetchQualifiedSegments(String userId, ODPSegmentManager.ODPSegmentFetchCallback callback, List segmentOptions) { + if (odpManager == null) { + logger.error("Audience segments fetch failed (ODP is not enabled)."); + callback.onCompleted(null); + } else { + odpManager.getSegmentManager().getQualifiedSegments(userId, callback, segmentOptions); + } + } + @Nullable public ODPManager getODPManager() { return odpManager; @@ -1450,13 +1468,6 @@ public void identifyUser(@Nonnull String userId) { } } - public void identifyUser(@Nullable String vuid, @Nullable String userId) { - ODPManager odpManager = getODPManager(); - if (odpManager != null) { - odpManager.getEventManager().identifyUser(vuid, userId); - } - } - private void updateODPSettings() { if (odpManager != null && getProjectConfig() != null) { ProjectConfig projectConfig = getProjectConfig(); diff --git a/core-api/src/main/java/com/optimizely/ab/OptimizelyUserContext.java b/core-api/src/main/java/com/optimizely/ab/OptimizelyUserContext.java index 475882759..5ba15b5b4 100644 --- a/core-api/src/main/java/com/optimizely/ab/OptimizelyUserContext.java +++ b/core-api/src/main/java/com/optimizely/ab/OptimizelyUserContext.java @@ -18,7 +18,6 @@ import com.optimizely.ab.odp.ODPManager; import com.optimizely.ab.odp.ODPSegmentCallback; -import com.optimizely.ab.odp.ODPSegmentManager; import com.optimizely.ab.odp.ODPSegmentOption; import com.optimizely.ab.optimizelydecision.*; import org.slf4j.Logger; @@ -303,8 +302,8 @@ public void setQualifiedSegments(List qualifiedSegments) { *

* The segments fetched will be saved and can be accessed at any time by calling {@link #getQualifiedSegments()}. */ - public void fetchQualifiedSegments() { - fetchQualifiedSegments(Collections.emptyList()); + public Boolean fetchQualifiedSegments() { + return fetchQualifiedSegments(Collections.emptyList()); } /** @@ -314,13 +313,12 @@ public void fetchQualifiedSegments() { * * @param segmentOptions A set of options for fetching qualified segments. */ - public void fetchQualifiedSegments(@Nonnull List segmentOptions) { - ODPManager odpManager = optimizely.getODPManager(); - if (odpManager != null) { - synchronized (odpManager) { - setQualifiedSegments(odpManager.getSegmentManager().getQualifiedSegments(userId, segmentOptions)); - } + public Boolean fetchQualifiedSegments(@Nonnull List segmentOptions) { + List segments = optimizely.fetchQualifiedSegments(userId, segmentOptions); + if (segments != null) { + setQualifiedSegments(segments); } + return segments != null; } /** @@ -333,18 +331,12 @@ public void fetchQualifiedSegments(@Nonnull List segmentOption * @param segmentOptions A set of options for fetching qualified segments. */ public void fetchQualifiedSegments(ODPSegmentCallback callback, List segmentOptions) { - ODPManager odpManager = optimizely.getODPManager(); - if (odpManager == null) { - logger.error("Audience segments fetch failed (ODP is not enabled"); - callback.invokeCallback(); - } else { - odpManager.getSegmentManager().getQualifiedSegments(userId, segments -> { - if (segments != null) { - setQualifiedSegments(segments); - } - callback.invokeCallback(); - }, segmentOptions); - } + optimizely.fetchQualifiedSegments(userId, segments -> { + if (segments != null) { + setQualifiedSegments(segments); + } + callback.onCompleted(segments != null); + }, segmentOptions); } /** diff --git a/core-api/src/main/java/com/optimizely/ab/odp/ODPSegmentCallback.java b/core-api/src/main/java/com/optimizely/ab/odp/ODPSegmentCallback.java index 0011094d1..57bc5097a 100644 --- a/core-api/src/main/java/com/optimizely/ab/odp/ODPSegmentCallback.java +++ b/core-api/src/main/java/com/optimizely/ab/odp/ODPSegmentCallback.java @@ -18,5 +18,5 @@ @FunctionalInterface public interface ODPSegmentCallback { - void invokeCallback(); + void onCompleted(Boolean isFetchSuccessful); } 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 c64c8b6a2..29bad87be 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 @@ -145,7 +145,7 @@ public void resetCache() { @FunctionalInterface public interface ODPSegmentFetchCallback { - void invokeCallback(List segments); + void onCompleted(List segments); } private class AsyncSegmentFetcher extends Thread { @@ -165,7 +165,7 @@ public AsyncSegmentFetcher(ODPUserKey userKey, String userValue, List segments = getQualifiedSegments(userKey, userValue, segmentOptions); - callback.invokeCallback(segments); + callback.onCompleted(segments); } } } diff --git a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java b/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java index e16ffdb02..9fd3dd675 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java @@ -4797,8 +4797,5 @@ public void identifyUser() { optimizely.identifyUser("the-user"); Mockito.verify(mockODPEventManager, times(1)).identifyUser("the-user"); - - optimizely.identifyUser("the-vuid", "the-user"); - Mockito.verify(mockODPEventManager, times(1)).identifyUser("the-vuid", "the-user"); } } diff --git a/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java b/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java index 255cd01a2..5dd47a8cf 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java @@ -16,6 +16,7 @@ */ package com.optimizely.ab; +import ch.qos.logback.classic.Level; import com.google.common.base.Charsets; import com.google.common.io.Resources; import com.optimizely.ab.bucketing.FeatureDecision; @@ -24,6 +25,7 @@ import com.optimizely.ab.config.parser.ConfigParseException; import com.optimizely.ab.event.ForwardingEventProcessor; import com.optimizely.ab.event.internal.payload.DecisionMetadata; +import com.optimizely.ab.internal.LogbackVerifier; import com.optimizely.ab.notification.NotificationCenter; import com.optimizely.ab.odp.*; import com.optimizely.ab.optimizelydecision.DecisionMessage; @@ -53,6 +55,9 @@ public class OptimizelyUserContextTest { @Rule public EventHandlerRule eventHandler = new EventHandlerRule(); + @Rule + public LogbackVerifier logbackVerifier = new LogbackVerifier(); + String userId = "tester"; boolean isListenerCalled = false; @@ -1628,13 +1633,23 @@ public void fetchQualifiedSegments() { OptimizelyUserContext userContext = optimizely.createUserContext("test-user"); - userContext.fetchQualifiedSegments(); + assertTrue(userContext.fetchQualifiedSegments()); verify(mockODPSegmentManager).getQualifiedSegments("test-user", Collections.emptyList()); - userContext.fetchQualifiedSegments(Collections.singletonList(ODPSegmentOption.RESET_CACHE)); + assertTrue(userContext.fetchQualifiedSegments(Collections.singletonList(ODPSegmentOption.RESET_CACHE))); verify(mockODPSegmentManager).getQualifiedSegments("test-user", Collections.singletonList(ODPSegmentOption.RESET_CACHE)); } + @Test + public void fetchQualifiedSegmentsError() { + Optimizely optimizely = Optimizely.builder() + .build(); + OptimizelyUserContext userContext = optimizely.createUserContext("test-user"); + + assertFalse(userContext.fetchQualifiedSegments()); + logbackVerifier.expectMessage(Level.ERROR, "Audience segments fetch failed (ODP is not enabled)."); + } + @Test public void fetchQualifiedSegmentsAsync() throws InterruptedException { ODPEventManager mockODPEventManager = mock(ODPEventManager.class); @@ -1644,7 +1659,7 @@ public void fetchQualifiedSegmentsAsync() throws InterruptedException { doAnswer( invocation -> { ODPSegmentManager.ODPSegmentFetchCallback callback = invocation.getArgumentAt(1, ODPSegmentManager.ODPSegmentFetchCallback.class); - callback.invokeCallback(Arrays.asList("segment1", "segment2")); + callback.onCompleted(Arrays.asList("segment1", "segment2")); return null; } ).when(mockODPSegmentManager).getQualifiedSegments(any(), (ODPSegmentManager.ODPSegmentFetchCallback) any(), any()); @@ -1658,7 +1673,10 @@ public void fetchQualifiedSegmentsAsync() throws InterruptedException { OptimizelyUserContext userContext = optimizely.createUserContext("test-user"); CountDownLatch countDownLatch = new CountDownLatch(1); - userContext.fetchQualifiedSegments(countDownLatch::countDown); + userContext.fetchQualifiedSegments((Boolean isFetchSuccessful) -> { + assertTrue(isFetchSuccessful); + countDownLatch.countDown(); + }); countDownLatch.await(); verify(mockODPSegmentManager).getQualifiedSegments(eq("test-user"), any(ODPSegmentManager.ODPSegmentFetchCallback.class), eq(Collections.emptyList())); @@ -1667,13 +1685,34 @@ public void fetchQualifiedSegmentsAsync() throws InterruptedException { // reset qualified segments userContext.setQualifiedSegments(Collections.emptyList()); CountDownLatch countDownLatch2 = new CountDownLatch(1); - userContext.fetchQualifiedSegments(countDownLatch2::countDown, Collections.singletonList(ODPSegmentOption.RESET_CACHE)); + userContext.fetchQualifiedSegments((Boolean isFetchSuccessful) -> { + assertTrue(isFetchSuccessful); + countDownLatch2.countDown(); + }, Collections.singletonList(ODPSegmentOption.RESET_CACHE)); countDownLatch2.await(); verify(mockODPSegmentManager).getQualifiedSegments(eq("test-user"), any(ODPSegmentManager.ODPSegmentFetchCallback.class), eq(Collections.singletonList(ODPSegmentOption.RESET_CACHE))); assertEquals(Arrays.asList("segment1", "segment2"), userContext.getQualifiedSegments()); } + @Test + public void fetchQualifiedSegmentsAsyncError() throws InterruptedException { + Optimizely optimizely = Optimizely.builder() + .build(); + + OptimizelyUserContext userContext = optimizely.createUserContext("test-user"); + + CountDownLatch countDownLatch = new CountDownLatch(1); + userContext.fetchQualifiedSegments((Boolean isFetchSuccessful) -> { + assertFalse(isFetchSuccessful); + countDownLatch.countDown(); + }); + + countDownLatch.await(); + assertEquals(Collections.emptyList(), userContext.getQualifiedSegments()); + logbackVerifier.expectMessage(Level.ERROR, "Audience segments fetch failed (ODP is not enabled)."); + } + @Test public void identifyUser() { ODPEventManager mockODPEventManager = mock(ODPEventManager.class); From 49a9686783fb851f2fb219a580a172d28f1837b6 Mon Sep 17 00:00:00 2001 From: zashraf1985 Date: Wed, 16 Nov 2022 16:32:21 -0800 Subject: [PATCH 11/11] removed redundant methods from cache --- .../com/optimizely/ab/internal/Cache.java | 2 -- .../ab/internal/DefaultLRUCache.java | 10 ---------- .../optimizely/ab/odp/ODPSegmentManager.java | 8 -------- .../ab/odp/ODPManagerBuilderTest.java | 20 ------------------- 4 files changed, 40 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/internal/Cache.java b/core-api/src/main/java/com/optimizely/ab/internal/Cache.java index 7aa279d68..ba667ebd2 100644 --- a/core-api/src/main/java/com/optimizely/ab/internal/Cache.java +++ b/core-api/src/main/java/com/optimizely/ab/internal/Cache.java @@ -22,6 +22,4 @@ public interface Cache { void save(String key, T value); T lookup(String key); void reset(); - Integer getMaxSize(); - Integer getCacheTimeoutSeconds(); } diff --git a/core-api/src/main/java/com/optimizely/ab/internal/DefaultLRUCache.java b/core-api/src/main/java/com/optimizely/ab/internal/DefaultLRUCache.java index e085b2040..a531c5c83 100644 --- a/core-api/src/main/java/com/optimizely/ab/internal/DefaultLRUCache.java +++ b/core-api/src/main/java/com/optimizely/ab/internal/DefaultLRUCache.java @@ -84,16 +84,6 @@ public void reset() { } } - @Override - public Integer getMaxSize() { - return maxSize; - } - - @Override - public Integer getCacheTimeoutSeconds() { - return (int) (timeoutMillis / 1000); - } - private class CacheEntity { public T value; public Long timestamp; 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 29bad87be..90a36fa5d 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 @@ -127,14 +127,6 @@ private String getCacheKey(String userKey, String userValue) { return userKey + "-$-" + userValue; } - public Integer getCacheMaxSize() { - return segmentsCache.getMaxSize(); - } - - public Integer getCacheTimeoutSeconds() { - return segmentsCache.getCacheTimeoutSeconds(); - } - public void updateSettings(ODPConfig odpConfig) { this.odpConfig = odpConfig; } diff --git a/core-api/src/test/java/com/optimizely/ab/odp/ODPManagerBuilderTest.java b/core-api/src/test/java/com/optimizely/ab/odp/ODPManagerBuilderTest.java index 730e55d95..0f7f59ae9 100644 --- a/core-api/src/test/java/com/optimizely/ab/odp/ODPManagerBuilderTest.java +++ b/core-api/src/test/java/com/optimizely/ab/odp/ODPManagerBuilderTest.java @@ -73,24 +73,4 @@ public void withSegmentCache() { odpManager.getSegmentManager().getQualifiedSegments("test-user"); verify(mockCache).lookup("fs_user_id-$-test-user"); } - - @Test - public void withSegmentCacheSize() { - ODPApiManager mockApiManager = mock(ODPApiManager.class); - ODPManager odpManager = ODPManager.builder() - .withApiManager(mockApiManager) - .withSegmentCacheSize(500) - .build(); - assertEquals(Integer.valueOf(500), odpManager.getSegmentManager().getCacheMaxSize()); - } - - @Test - public void withSegmentCacheTimeout() { - ODPApiManager mockApiManager = mock(ODPApiManager.class); - ODPManager odpManager = ODPManager.builder() - .withApiManager(mockApiManager) - .withSegmentCacheTimeout(53) - .build(); - assertEquals(Integer.valueOf(53), odpManager.getSegmentManager().getCacheTimeoutSeconds()); - } }