Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

feat: Integrated ODPManager with UserContext and Optimizely client #490

Merged
merged 11 commits into from
Nov 21, 2022
Merged

feat: Integrated ODPManager with UserContext and Optimizely client #490

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
525ad99
Integrated ODPManager with UserContext and Optimizely
zashraf1985 Oct 27, 2022
5d3cb21
fixed an issue in DefaultODPApiManager
zashraf1985 Oct 27, 2022
0a1bf21
Calling close on ODPEventManager when Optimizely closes
zashraf1985 Oct 28, 2022
0c74a18
Incorporated review feedback.
zashraf1985 Nov 2, 2022
42b2425
Changed GraphQl query to use variables for better sanitization.
zashraf1985 Nov 2, 2022
79263fc
Incorporated more review feedback.
zashraf1985 Nov 4, 2022
e573a6a
Added tests for `Optimizely`
zashraf1985 Nov 8, 2022
6d88b86
Completed the unit tests.
zashraf1985 Nov 9, 2022
68aa5ff
Added Non blocking implementation for fetching segments.
zashraf1985 Nov 11, 2022
29f2b09
Incorporated review feedback
zashraf1985 Nov 17, 2022
49a9686
removed redundant methods from cache
zashraf1985 Nov 17, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 70 additions & 13 deletions core-api/src/main/java/com/optimizely/ab/Optimizely.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand All @@ -104,7 +109,8 @@ private Optimizely(@Nonnull EventHandler eventHandler,
@Nonnull ProjectConfigManager projectConfigManager,
@Nullable OptimizelyConfigManager optimizelyConfigManager,
@Nonnull NotificationCenter notificationCenter,
@Nonnull List<OptimizelyDecideOption> defaultDecideOptions
@Nonnull List<OptimizelyDecideOption> defaultDecideOptions,
@Nullable ODPManager odpManager
) {
this.eventHandler = eventHandler;
this.eventProcessor = eventProcessor;
Expand All @@ -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(); });
}
}

/**
Expand All @@ -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.
Expand All @@ -141,6 +154,9 @@ public void close() {
tryClose(eventProcessor);
tryClose(eventHandler);
tryClose(projectConfigManager);
if (odpManager != null) {
tryClose(odpManager);
}
}

//======== activate calls ========//
Expand Down Expand Up @@ -674,9 +690,9 @@ public OptimizelyJSON getFeatureVariableJSON(@Nonnull String featureKey,
*/
@Nullable
public OptimizelyJSON getFeatureVariableJSON(@Nonnull String featureKey,
@Nonnull String variableKey,
@Nonnull String userId,
@Nonnull Map<String, ?> attributes) {
@Nonnull String variableKey,
@Nonnull String userId,
@Nonnull Map<String, ?> attributes) {

return getFeatureVariableValueForType(
featureKey,
Expand All @@ -688,10 +704,10 @@ public OptimizelyJSON getFeatureVariableJSON(@Nonnull String featureKey,

@VisibleForTesting
<T> T getFeatureVariableValueForType(@Nonnull String featureKey,
@Nonnull String variableKey,
@Nonnull String userId,
@Nonnull Map<String, ?> attributes,
@Nonnull String variableType) {
@Nonnull String variableKey,
@Nonnull String userId,
@Nonnull Map<String, ?> attributes,
@Nonnull String variableType) {
if (featureKey == null) {
logger.warn("The featureKey parameter must be nonnull.");
return null;
Expand Down Expand Up @@ -878,7 +894,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<String, Object> valuesMap = new HashMap<String, Object>();
Expand Down Expand Up @@ -1142,7 +1158,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<String, ?> attributes) {
if (userId == null) {
Expand Down Expand Up @@ -1413,6 +1429,41 @@ public <T> int addNotificationHandler(Class<T> clazz, NotificationHandler<T> han
return notificationCenter.addNotificationHandler(clazz, handler);
}

@Nullable
zashraf1985 marked this conversation as resolved.
Show resolved Hide resolved
public ODPManager getODPManager() {
zashraf1985 marked this conversation as resolved.
Show resolved Hide resolved
return odpManager;
}

public void sendODPEvent(@Nonnull String type, @Nonnull String action, @Nullable Map<String, String> identifiers, @Nullable Map<String, Object> data) {
zashraf1985 marked this conversation as resolved.
Show resolved Hide resolved
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) {
zashraf1985 marked this conversation as resolved.
Show resolved Hide resolved
ODPManager odpManager = getODPManager();
if (odpManager != null) {
odpManager.getEventManager().identifyUser(userId);
}
}

public void identifyUser(@Nullable String vuid, String userId) {
zashraf1985 marked this conversation as resolved.
Show resolved Hide resolved
ODPManager odpManager = getODPManager();
if (odpManager != null) {
odpManager.getEventManager().identifyUser(vuid, userId);
}
zashraf1985 marked this conversation as resolved.
Show resolved Hide resolved
}

private void updateODPSettings() {
if (odpManager != null && getProjectConfig() != null) {
ProjectConfig projectConfig = getProjectConfig();
odpManager.updateSettings(projectConfig.getHostForODP(), projectConfig.getPublicKeyForODP(), projectConfig.getAllSegments());
}
}

//======== Builder ========//

/**
Expand Down Expand Up @@ -1467,6 +1518,7 @@ public static class Builder {
private UserProfileService userProfileService;
private NotificationCenter notificationCenter;
private List<OptimizelyDecideOption> defaultDecideOptions;
private ODPManager odpManager;

// For backwards compatibility
private AtomicProjectConfigManager fallbackConfigManager = new AtomicProjectConfigManager();
Expand Down Expand Up @@ -1562,6 +1614,11 @@ public Builder withDefaultDecideOptions(List<OptimizelyDecideOption> defaultDeci
return this;
}

public Builder withODPManager(ODPManager odpManager) {
zashraf1985 marked this conversation as resolved.
Show resolved Hide resolved
this.odpManager = odpManager;
return this;
}

// Helper functions for making testing easier
protected Builder withBucketing(Bucketer bucketer) {
this.bucketer = bucketer;
Expand Down Expand Up @@ -1636,7 +1693,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);
}
}
}
46 changes: 45 additions & 1 deletion core-api/src/main/java/com/optimizely/ab/OptimizelyUserContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -54,6 +56,15 @@ public OptimizelyUserContext(@Nonnull Optimizely optimizely,
@Nonnull Map<String, ?> attributes,
@Nullable Map<String, OptimizelyForcedDecision> forcedDecisionsMap,
@Nullable List<String> qualifiedSegments) {
this(optimizely, userId, attributes, forcedDecisionsMap, qualifiedSegments, false);
zashraf1985 marked this conversation as resolved.
Show resolved Hide resolved
}

public OptimizelyUserContext(@Nonnull Optimizely optimizely,
@Nonnull String userId,
@Nonnull Map<String, ?> attributes,
@Nullable Map<String, OptimizelyForcedDecision> forcedDecisionsMap,
@Nullable List<String> qualifiedSegments,
@Nullable Boolean shouldIdentifyUser) {
this.optimizely = optimizely;
this.userId = userId;
if (attributes != null) {
Expand All @@ -66,6 +77,10 @@ public OptimizelyUserContext(@Nonnull Optimizely optimizely,
}

this.qualifiedSegments = Collections.synchronizedList( qualifiedSegments == null ? new LinkedList<>(): qualifiedSegments);

if (shouldIdentifyUser == null || shouldIdentifyUser) {
optimizely.identifyUser(userId);
}
}

public OptimizelyUserContext(@Nonnull Optimizely optimizely, @Nonnull String userId) {
Expand Down Expand Up @@ -282,6 +297,36 @@ public void setQualifiedSegments(List<String> 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() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we return boolean (true/false for success/failure) for blocking fetchSegment calls. Clients can check the status before continuing with decide call?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

ODPManager odpManager = optimizely.getODPManager();
if (odpManager != null) {
synchronized (odpManager) {
setQualifiedSegments(odpManager.getSegmentManager().getQualifiedSegments(userId));
}
}
zashraf1985 marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* 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<ODPSegmentOption> segmentOptions) {
ODPManager odpManager = optimizely.getODPManager();
if (odpManager != null) {
synchronized (odpManager) {
setQualifiedSegments(odpManager.getSegmentManager().getQualifiedSegments(userId, segmentOptions));
}
}
}

// Utils

@Override
Expand Down Expand Up @@ -309,5 +354,4 @@ public String toString() {
", attributes='" + attributes + '\'' +
'}';
}

}
1 change: 1 addition & 0 deletions core-api/src/main/java/com/optimizely/ab/config/DatafileProjectConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,7 @@ public List<Experiment> getExperiments() {
return experiments;
}

@Override
public Set<String> getAllSegments() {
return this.allSegments;
}
Expand Down
3 changes: 3 additions & 0 deletions core-api/src/main/java/com/optimizely/ab/config/ProjectConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -69,6 +70,8 @@ Experiment getExperimentForKey(@Nonnull String experimentKey,

List<Experiment> getExperiments();

Set<String> getAllSegments();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wondering if this addition to the interface will require changes to all existing customization.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If users are using their own ProjectConfigManagers? yes! But this is the only way to add more functionality to ProjectConfig. I believe almost no one will be customizing the whole ProjectConfigManager. If there are a handful of customers who are even doing it, ATS will go out as a major version change anyway. We can mention this in the release notes as a breaking change. What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zashraf1985 We'll go for a major version change if we have to. We better keep all the breaking changes required.


List<Experiment> getExperimentsForEventKey(String eventKey);

List<FeatureFlag> getFeatureFlags();
Expand Down
4 changes: 2 additions & 2 deletions core-api/src/main/java/com/optimizely/ab/odp/ODPApiManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> segmentsToCheck);
String fetchQualifiedSegments(String apiKey, String apiEndpoint, String userKey, String userValue, Set<String> segmentsToCheck);

Integer sendEvents(String apiKey, String apiEndpoint, String eventPayload);
}
12 changes: 6 additions & 6 deletions core-api/src/main/java/com/optimizely/ab/odp/ODPConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,24 +17,24 @@
package com.optimizely.ab.odp;

import java.util.Collections;
import java.util.List;
import java.util.Set;

public class ODPConfig {

private String apiKey;

private String apiHost;

private List<String> allSegments;
private Set<String> allSegments;

public ODPConfig(String apiKey, String apiHost, List<String> allSegments) {
public ODPConfig(String apiKey, String apiHost, Set<String> 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() {
Expand Down Expand Up @@ -64,11 +64,11 @@ public synchronized String getApiHost() {
return apiHost;
}

public synchronized List<String> getAllSegments() {
public synchronized Set<String> getAllSegments() {
return allSegments;
}

public synchronized void setAllSegments(List<String> allSegments) {
public synchronized void setAllSegments(Set<String> allSegments) {
this.allSegments = allSegments;
}

Expand Down
Loading