From cdaa2e0abf7680fe0a63729d51cc8f05bf44087d Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Tue, 28 Sep 2021 12:22:53 -0400 Subject: [PATCH 01/39] Addition of ForcedDecisions --- .../java/com/optimizely/ab/Optimizely.java | 54 +++-- .../optimizely/ab/OptimizelyUserContext.java | 229 +++++++++++++++++- .../ab/bucketing/DecisionService.java | 104 +++++--- .../ab/bucketing/FeatureDecision.java | 2 +- .../ab/config/DatafileProjectConfig.java | 41 ++++ .../optimizely/ab/config/ProjectConfig.java | 2 + .../com/optimizely/ab/OptimizelyTest.java | 52 ++-- .../ab/OptimizelyUserContextTest.java | 164 +++++++++++++ .../ab/bucketing/DecisionServiceTest.java | 180 ++++++-------- 9 files changed, 633 insertions(+), 195 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 8a7034d0e..6459f383a 100644 --- a/core-api/src/main/java/com/optimizely/ab/Optimizely.java +++ b/core-api/src/main/java/com/optimizely/ab/Optimizely.java @@ -376,7 +376,7 @@ public void track(@Nonnull String eventName, @Nonnull public Boolean isFeatureEnabled(@Nonnull String featureKey, @Nonnull String userId) { - return isFeatureEnabled(featureKey, userId, Collections.emptyMap()); + return isFeatureEnabled(featureKey, userId, Collections.emptyMap()); } /** @@ -422,9 +422,9 @@ private Boolean isFeatureEnabled(@Nonnull ProjectConfig projectConfig, return false; } - Map copiedAttributes = copyAttributes(attributes); + Map copiedAttributes = copyAttributes(attributes); FeatureDecision.DecisionSource decisionSource = FeatureDecision.DecisionSource.ROLLOUT; - FeatureDecision featureDecision = decisionService.getVariationForFeature(featureFlag, userId, copiedAttributes, projectConfig).getResult(); + FeatureDecision featureDecision = decisionService.getVariationForFeature(featureFlag, createUserContext(userId, copiedAttributes), projectConfig).getResult(); Boolean featureEnabled = false; SourceInfo sourceInfo = new RolloutSourceInfo(); if (featureDecision.decisionSource != null) { @@ -732,8 +732,8 @@ T getFeatureVariableValueForType(@Nonnull String featureKey, } String variableValue = variable.getDefaultValue(); - Map copiedAttributes = copyAttributes(attributes); - FeatureDecision featureDecision = decisionService.getVariationForFeature(featureFlag, userId, copiedAttributes, projectConfig).getResult(); + Map copiedAttributes = copyAttributes(attributes); + FeatureDecision featureDecision = decisionService.getVariationForFeature(featureFlag, createUserContext(userId, copiedAttributes), projectConfig).getResult(); Boolean featureEnabled = false; if (featureDecision.variation != null) { if (featureDecision.variation.getFeatureEnabled()) { @@ -865,8 +865,8 @@ public OptimizelyJSON getAllFeatureVariables(@Nonnull String featureKey, return null; } - Map copiedAttributes = copyAttributes(attributes); - FeatureDecision featureDecision = decisionService.getVariationForFeature(featureFlag, userId, copiedAttributes, projectConfig).getResult(); + Map copiedAttributes = copyAttributes(attributes); + FeatureDecision featureDecision = decisionService.getVariationForFeature(featureFlag, createUserContext(userId, copiedAttributes), projectConfig, Collections.emptyList()).getResult(); Boolean featureEnabled = false; Variation variation = featureDecision.variation; @@ -924,7 +924,7 @@ public OptimizelyJSON getAllFeatureVariables(@Nonnull String featureKey, * return Empty List. */ public List getEnabledFeatures(@Nonnull String userId, @Nonnull Map attributes) { - List enabledFeaturesList = new ArrayList(); + List enabledFeaturesList = new ArrayList(); if (!validateUserId(userId)) { return enabledFeaturesList; } @@ -935,7 +935,7 @@ public List getEnabledFeatures(@Nonnull String userId, @Nonnull Map copiedAttributes = copyAttributes(attributes); + Map copiedAttributes = copyAttributes(attributes); for (FeatureFlag featureFlag : projectConfig.getFeatureFlags()) { String featureKey = featureFlag.getKey(); if (isFeatureEnabled(projectConfig, featureKey, userId, copiedAttributes)) @@ -966,9 +966,8 @@ private Variation getVariation(@Nonnull ProjectConfig projectConfig, @Nonnull Experiment experiment, @Nonnull String userId, @Nonnull Map attributes) throws UnknownExperimentException { - Map copiedAttributes = copyAttributes(attributes); - Variation variation = decisionService.getVariation(experiment, userId, copiedAttributes, projectConfig).getResult(); - + Map copiedAttributes = copyAttributes(attributes); + Variation variation = decisionService.getVariation(experiment, createUserContext(userId, copiedAttributes), projectConfig).getResult(); String notificationType = NotificationCenter.DecisionNotificationType.AB_TEST.toString(); if (projectConfig.getExperimentFeatureKeyMapping().get(experiment.getId()) != null) { @@ -1034,7 +1033,10 @@ public Variation getVariation(@Nonnull String experimentKey, * @param variationKey The variation key to force the user into. If the variation key is null * then the forcedVariation for that experiment is removed. * @return boolean A boolean value that indicates if the set completed successfully. + * + * @deprecated use {@link OptimizelyUserContext#setForcedDecision(String, String, String)} instead */ + @Deprecated public boolean setForcedVariation(@Nonnull String experimentKey, @Nonnull String userId, @Nullable String variationKey) { @@ -1065,7 +1067,10 @@ public boolean setForcedVariation(@Nonnull String experimentKey, * @param userId The user ID to be used for bucketing. * @return The variation the user was bucketed into. This value can be null if the * forced variation fails. + * + * @deprecated use {@link OptimizelyUserContext#getForcedDecision(String, String)} instead */ + @Deprecated @Nullable public Variation getForcedVariation(@Nonnull String experimentKey, @Nonnull String userId) { @@ -1181,8 +1186,7 @@ OptimizelyDecision decide(@Nonnull OptimizelyUserContext user, Map copiedAttributes = new HashMap<>(attributes); DecisionResponse decisionVariation = decisionService.getVariationForFeature( flag, - userId, - copiedAttributes, + user, projectConfig, allOptions); FeatureDecision flagDecision = decisionVariation.getResult(); @@ -1332,14 +1336,32 @@ private DecisionResponse> getDecisionVariableMap(@Nonnull Fe return new DecisionResponse(valuesMap, reasons); } + /** + * Gets a variation based on flagKey and variationKey + * + * @param flagKey The flag key for the variation + * @param variationKey The variation key for the variation + * @return Returns a variation based on flagKey and variationKey, otherwise null + */ + public Variation getFlagVariationByKey(String flagKey, String variationKey) { + Map> flagVariationsMap = getProjectConfig().getFlagVariationsMap(); + List variations = flagVariationsMap.get(flagKey); + for (Variation variation : variations) { + if (variation.getKey().equals(variationKey)) { + return variation; + } + } + return null; + } + /** * Helper method which makes separate copy of attributesMap variable and returns it * * @param attributes map to copy * @return copy of attributes */ - private Map copyAttributes(Map attributes) { - Map copiedAttributes = null; + private Map copyAttributes(Map attributes) { + Map copiedAttributes = null; if (attributes != null) { copiedAttributes = new HashMap<>(attributes); } 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 f9cff6f44..15f6fb0ab 100644 --- a/core-api/src/main/java/com/optimizely/ab/OptimizelyUserContext.java +++ b/core-api/src/main/java/com/optimizely/ab/OptimizelyUserContext.java @@ -16,19 +16,36 @@ */ package com.optimizely.ab; -import com.optimizely.ab.optimizelydecision.OptimizelyDecideOption; -import com.optimizely.ab.optimizelydecision.OptimizelyDecision; +import com.optimizely.ab.config.Variation; +import com.optimizely.ab.optimizelydecision.*; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import javax.annotation.Nonnull; import javax.annotation.Nullable; -import java.util.Collections; -import java.util.HashMap; -import java.util.List; -import java.util.Map; +import java.util.*; public class OptimizelyUserContext { + static class ForcedDecision { + private String flagKey; + private String ruleKey; + private String variationKey; + + ForcedDecision(@Nonnull String flagKey, String ruleKey, @Nonnull String variationKey) { + this.flagKey = flagKey; + this.ruleKey = ruleKey; + this.variationKey = variationKey; + } + + public String getFlagKey() { return flagKey; } + public String getRuleKey() { return ruleKey; } + public String getVariationKey() { return variationKey; } + } + + // flagKeys mapped to ruleKeys mapped to forcedDecisions + Map> forcedDecisionsMap = new HashMap<>(); + Map forcedDecisionsMapWithNoRuleKey = new HashMap<>(); + @Nonnull private final String userId; @@ -172,6 +189,206 @@ public void trackEvent(@Nonnull String eventName) throws UnknownEventTypeExcepti trackEvent(eventName, Collections.emptyMap()); } + /** + * + * @param flagKey The flag key for the forced decision + * @param variationKey The variation key for the forced decision + * @return Returns a boolean, True if successfully set, otherwise false + */ + public Boolean setForcedDecision(@Nonnull String flagKey, @Nonnull String variationKey) { + return setForcedDecision(flagKey, null, variationKey); + } + + /** + * Set a forced decision + * + * @param flagKey The flag key for the forced decision + * @param ruleKey The rule key for the forced decision + * @param variationKey The variation key for the forced decision + * @return Returns a boolean, Ture if successfully set, otherwise false + */ + public Boolean setForcedDecision(@Nonnull String flagKey, String ruleKey, @Nonnull String variationKey) { + if (optimizely.getOptimizelyConfig() == null) { + logger.error("Optimizely SDK not ready."); + return false; + } + + if (ruleKey == null) { + // If the ruleKey is null, we will populate/update the appropriate map + if (forcedDecisionsMapWithNoRuleKey.get(flagKey) != null) { + forcedDecisionsMapWithNoRuleKey.get(flagKey).variationKey = variationKey; + } else { + forcedDecisionsMapWithNoRuleKey.put(flagKey, new ForcedDecision(flagKey, null, variationKey)); + } + } else { + // If the flagKey and ruleKey are already present, set the updated variationKey + if (forcedDecisionsMap.containsKey(flagKey)) { + if (forcedDecisionsMap.get(flagKey).containsKey(ruleKey)) { + forcedDecisionsMap.get(flagKey).get(ruleKey).variationKey = variationKey; + } else { + forcedDecisionsMap.get(flagKey).put(ruleKey, new ForcedDecision(flagKey, ruleKey, variationKey)); + } + } else { + Map forcedDecision = new HashMap<>(); + forcedDecision.put(ruleKey, new ForcedDecision(flagKey, ruleKey, variationKey)); + forcedDecisionsMap.put(flagKey, forcedDecision); + } + } + + return true; + } + + /** + * + * @param flagKey The flag key for the forced decision + * @return Returns a variationKey for a given forced decision + */ + public String getForcedDecision(@Nonnull String flagKey) { + return getForcedDecision(flagKey, null); + } + + /** + * Get a forced decision + * + * @param flagKey The flag key for the forced decision + * @param ruleKey The rule key for the forced decision + * @return Returns a variationKey for a given forced decision + */ + public String getForcedDecision(@Nonnull String flagKey, String ruleKey) { + if (optimizely.getOptimizelyConfig() == null) { + logger.error("Optimizely SDK not ready."); + return null; + } + return findForcedDecision(flagKey, ruleKey); + } + + /** + * Finds a forced decision + * + * @param flagKey The flag key for the forced decision + * @param ruleKey The rule key for the forced decision + * @return Returns a variationKey relating to the found forced decision, otherwise null + */ + public String findForcedDecision(@Nonnull String flagKey, String ruleKey) { + String variationKey = null; + if (ruleKey != null) { + if (forcedDecisionsMap.size() > 0 && forcedDecisionsMap.containsKey(flagKey)) { + if (forcedDecisionsMap.get(flagKey).containsKey(ruleKey)) { + variationKey = forcedDecisionsMap.get(flagKey).get(ruleKey).getVariationKey(); + } + } + } else { + if (forcedDecisionsMapWithNoRuleKey.size() > 0 && forcedDecisionsMapWithNoRuleKey.containsKey(flagKey)) { + variationKey = forcedDecisionsMapWithNoRuleKey.get(flagKey).getVariationKey(); + } + } + return variationKey; + } + + /** + * + * @param flagKey The flag key in the forced decision + * @return Returns a boolean of true if successful, otherwise false + */ + public boolean removeForcedDecision(@Nonnull String flagKey) { + return removeForcedDecision(flagKey, null); + } + + /** + * Remove a forced decision + * + * @param flagKey The flag key for the forced decision + * @param ruleKey The rule key for the forced decision + * @return Returns a boolean, true if successfully removed, otherwise false + */ + public boolean removeForcedDecision(@Nonnull String flagKey, String ruleKey) { + if (optimizely.getOptimizelyConfig() == null) { + logger.error("Optimizely SDK not ready."); + return false; + } + if (ruleKey != null) { + try { + forcedDecisionsMap.get(flagKey).remove(ruleKey); + if (forcedDecisionsMap.get(flagKey).size() == 0) { + forcedDecisionsMap.remove(flagKey); + } + return true; + } catch (Exception e) { + logger.error("Forced Decision does not exist to remove - " + e); + } + } else { + try { + forcedDecisionsMapWithNoRuleKey.remove(flagKey); + return true; + } catch (Exception e) { + logger.error("Forced Decision does not exist to remove - " + e); + } + } + + return false; + } + + /** + * Remove all forced decisions + * + * @return Returns a boolean, True if successfully, otherwise false + */ + public boolean removeAllForcedDecisions() { + if (optimizely.getProjectConfig() == null) { + logger.error("Optimizely SDK not ready."); + return false; + } + // Clear both maps for with and without ruleKey + forcedDecisionsMap.clear(); + forcedDecisionsMapWithNoRuleKey.clear(); + return true; + } + + /** + * Find a validated forced decision + * + * @param flagKey The flag key for the forced decision + * @return Returns a DecisionResponse structure of type Variation, otherwise null with reasons + */ + public DecisionResponse findValidatedForcedDecision(@Nonnull String flagKey) { + return findValidatedForcedDecision(flagKey, null); + } + + /** + * Find a validated forced decision + * + * @param flagKey The flag key for a forced decision + * @param ruleKey The rule key for a forced decision + * @return Returns a DecisionResponse structure of type Variation, otherwise null result with reasons + */ + public DecisionResponse findValidatedForcedDecision(@Nonnull String flagKey, String ruleKey) { + DecisionReasons reasons = DefaultDecisionReasons.newInstance(); + // TODO - Move all info strings to a single class to be called rather than hardcoded in functions + String variationKey = findForcedDecision(flagKey, ruleKey); + if (variationKey != null) { + Variation variation = optimizely.getFlagVariationByKey(flagKey, variationKey); + String strRuleKey = ruleKey != null ? ruleKey : "null"; + if (variation != null) { + String info = "Variation " + variationKey + + " is mapped to flag: " + flagKey + + " and rule: " + strRuleKey + + " and user: " + userId + + " in the forced decision map."; + logger.debug(info); + reasons.addInfo(info); + return new DecisionResponse(variation, reasons); + } else { + String info = "Invalid variation is mapped to flag: " + flagKey + + " and rule: " + strRuleKey + + " and user: " + userId + + " forced decision map."; + logger.debug(info); + reasons.addInfo(info); + } + } + return new DecisionResponse<>(null, reasons); + } + // Utils @Override diff --git a/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java b/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java index c6a267f5b..42f31eb7f 100644 --- a/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java +++ b/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java @@ -16,6 +16,7 @@ package com.optimizely.ab.bucketing; import com.optimizely.ab.OptimizelyRuntimeException; +import com.optimizely.ab.OptimizelyUserContext; import com.optimizely.ab.config.*; import com.optimizely.ab.error.ErrorHandler; import com.optimizely.ab.internal.ControlAttribute; @@ -83,16 +84,14 @@ public DecisionService(@Nonnull Bucketer bucketer, * Get a {@link Variation} of an {@link Experiment} for a user to be allocated into. * * @param experiment The Experiment the user will be bucketed into. - * @param userId The userId of the user. - * @param filteredAttributes The user's attributes. This should be filtered to just attributes in the Datafile. + * @param user The current OptimizelyUserContext * @param projectConfig The current projectConfig * @param options An array of decision options * @return A {@link DecisionResponse} including the {@link Variation} that user is bucketed into (or null) and the decision reasons */ @Nonnull public DecisionResponse getVariation(@Nonnull Experiment experiment, - @Nonnull String userId, - @Nonnull Map filteredAttributes, + @Nonnull OptimizelyUserContext user, @Nonnull ProjectConfig projectConfig, @Nonnull List options) { DecisionReasons reasons = DefaultDecisionReasons.newInstance(); @@ -104,13 +103,13 @@ public DecisionResponse getVariation(@Nonnull Experiment experiment, } // look for forced bucketing first. - DecisionResponse decisionVariation = getForcedVariation(experiment, userId); + DecisionResponse decisionVariation = getForcedVariation(experiment, user.getUserId()); reasons.merge(decisionVariation.getReasons()); Variation variation = decisionVariation.getResult(); // check for whitelisting if (variation == null) { - decisionVariation = getWhitelistedVariation(experiment, userId); + decisionVariation = getWhitelistedVariation(experiment, user.getUserId()); reasons.merge(decisionVariation.getReasons()); variation = decisionVariation.getResult(); } @@ -125,7 +124,7 @@ public DecisionResponse getVariation(@Nonnull Experiment experiment, if (userProfileService != null && !ignoreUPS) { try { - Map userProfileMap = userProfileService.lookup(userId); + Map userProfileMap = userProfileService.lookup(user.getUserId()); if (userProfileMap == null) { String message = reasons.addInfo("We were unable to get a user profile map from the UserProfileService."); logger.info(message); @@ -151,14 +150,14 @@ public DecisionResponse getVariation(@Nonnull Experiment experiment, return new DecisionResponse(variation, reasons); } } else { // if we could not find a user profile, make a new one - userProfile = new UserProfile(userId, new HashMap()); + userProfile = new UserProfile(user.getUserId(), new HashMap()); } } - DecisionResponse decisionMeetAudience = ExperimentUtils.doesUserMeetAudienceConditions(projectConfig, experiment, filteredAttributes, EXPERIMENT, experiment.getKey()); + DecisionResponse decisionMeetAudience = ExperimentUtils.doesUserMeetAudienceConditions(projectConfig, experiment, user.getAttributes(), EXPERIMENT, experiment.getKey()); reasons.merge(decisionMeetAudience.getReasons()); if (decisionMeetAudience.getResult()) { - String bucketingId = getBucketingId(userId, filteredAttributes); + String bucketingId = getBucketingId(user.getUserId(), user.getAttributes()); decisionVariation = bucketer.bucket(experiment, bucketingId, projectConfig); reasons.merge(decisionVariation.getReasons()); @@ -175,33 +174,30 @@ public DecisionResponse getVariation(@Nonnull Experiment experiment, return new DecisionResponse(variation, reasons); } - String message = reasons.addInfo("User \"%s\" does not meet conditions to be in experiment \"%s\".", userId, experiment.getKey()); + String message = reasons.addInfo("User \"%s\" does not meet conditions to be in experiment \"%s\".", user.getUserId(), experiment.getKey()); logger.info(message); return new DecisionResponse(null, reasons); } @Nonnull public DecisionResponse getVariation(@Nonnull Experiment experiment, - @Nonnull String userId, - @Nonnull Map filteredAttributes, + @Nonnull OptimizelyUserContext user, @Nonnull ProjectConfig projectConfig) { - return getVariation(experiment, userId, filteredAttributes, projectConfig, Collections.emptyList()); + return getVariation(experiment, user, projectConfig, Collections.emptyList()); } /** * Get the variation the user is bucketed into for the FeatureFlag * * @param featureFlag The feature flag the user wants to access. - * @param userId User Identifier - * @param filteredAttributes A map of filtered attributes. + * @param user The current OptimizelyuserContext * @param projectConfig The current projectConfig * @param options An array of decision options * @return A {@link DecisionResponse} including a {@link FeatureDecision} and the decision reasons */ @Nonnull public DecisionResponse getVariationForFeature(@Nonnull FeatureFlag featureFlag, - @Nonnull String userId, - @Nonnull Map filteredAttributes, + @Nonnull OptimizelyUserContext user, @Nonnull ProjectConfig projectConfig, @Nonnull List options) { DecisionReasons reasons = DefaultDecisionReasons.newInstance(); @@ -210,7 +206,7 @@ public DecisionResponse getVariationForFeature(@Nonnull Feature for (String experimentId : featureFlag.getExperimentIds()) { Experiment experiment = projectConfig.getExperimentIdMapping().get(experimentId); - DecisionResponse decisionVariation = getVariation(experiment, userId, filteredAttributes, projectConfig, options); + DecisionResponse decisionVariation = getVariationFromExperimentRule(projectConfig, featureFlag.getKey(), experiment, user, options); reasons.merge(decisionVariation.getReasons()); Variation variation = decisionVariation.getResult(); @@ -225,28 +221,27 @@ public DecisionResponse getVariationForFeature(@Nonnull Feature logger.info(message); } - DecisionResponse decisionFeature = getVariationForFeatureInRollout(featureFlag, userId, filteredAttributes, projectConfig); + DecisionResponse decisionFeature = getVariationForFeatureInRollout(featureFlag, user, projectConfig); reasons.merge(decisionFeature.getReasons()); FeatureDecision featureDecision = decisionFeature.getResult(); + String message; if (featureDecision.variation == null) { - String message = reasons.addInfo("The user \"%s\" was not bucketed into a rollout for feature flag \"%s\".", - userId, featureFlag.getKey()); - logger.info(message); + message = reasons.addInfo("The user \"%s\" was not bucketed into a rollout for feature flag \"%s\".", + user.getUserId(), featureFlag.getKey()); } else { - String message = reasons.addInfo("The user \"%s\" was bucketed into a rollout for feature flag \"%s\".", - userId, featureFlag.getKey()); - logger.info(message); + message = reasons.addInfo("The user \"%s\" was bucketed into a rollout for feature flag \"%s\".", + user.getUserId(), featureFlag.getKey()); } + logger.info(message); return new DecisionResponse(featureDecision, reasons); } @Nonnull public DecisionResponse getVariationForFeature(@Nonnull FeatureFlag featureFlag, - @Nonnull String userId, - @Nonnull Map filteredAttributes, + @Nonnull OptimizelyUserContext user, @Nonnull ProjectConfig projectConfig) { - return getVariationForFeature(featureFlag, userId, filteredAttributes, projectConfig,Collections.emptyList()); + return getVariationForFeature(featureFlag, user, projectConfig, Collections.emptyList()); } /** @@ -255,15 +250,13 @@ public DecisionResponse getVariationForFeature(@Nonnull Feature * Fall back onto the everyone else rule if the user is ever excluded from a rule due to traffic allocation. * * @param featureFlag The feature flag the user wants to access. - * @param userId User Identifier - * @param filteredAttributes A map of filtered attributes. + * @param user The current OptimizelyUserContext * @param projectConfig The current projectConfig * @return A {@link DecisionResponse} including a {@link FeatureDecision} and the decision reasons */ @Nonnull DecisionResponse getVariationForFeatureInRollout(@Nonnull FeatureFlag featureFlag, - @Nonnull String userId, - @Nonnull Map filteredAttributes, + @Nonnull OptimizelyUserContext user, @Nonnull ProjectConfig projectConfig) { DecisionReasons reasons = DefaultDecisionReasons.newInstance(); @@ -286,7 +279,7 @@ DecisionResponse getVariationForFeatureInRollout(@Nonnull Featu if (rolloutRulesLength == 0) { return new DecisionResponse(new FeatureDecision(null, null, null), reasons); } - String bucketingId = getBucketingId(userId, filteredAttributes); + String bucketingId = getBucketingId(user.getUserId(), user.getAttributes()); Variation variation; DecisionResponse decisionMeetAudience; @@ -294,7 +287,7 @@ DecisionResponse getVariationForFeatureInRollout(@Nonnull Featu for (int i = 0; i < rolloutRulesLength - 1; i++) { Experiment rolloutRule = rollout.getExperiments().get(i); - decisionMeetAudience = ExperimentUtils.doesUserMeetAudienceConditions(projectConfig, rolloutRule, filteredAttributes, RULE, Integer.toString(i + 1)); + decisionMeetAudience = ExperimentUtils.doesUserMeetAudienceConditions(projectConfig, rolloutRule, user.getAttributes(), RULE, Integer.toString(i + 1)); reasons.merge(decisionMeetAudience.getReasons()); if (decisionMeetAudience.getResult()) { decisionVariation = bucketer.bucket(rolloutRule, bucketingId, projectConfig); @@ -308,7 +301,7 @@ DecisionResponse getVariationForFeatureInRollout(@Nonnull Featu new FeatureDecision(rolloutRule, variation, FeatureDecision.DecisionSource.ROLLOUT), reasons); } else { - String message = reasons.addInfo("User \"%s\" does not meet conditions for targeting rule \"%d\".", userId, i + 1); + String message = reasons.addInfo("User \"%s\" does not meet conditions for targeting rule \"%d\".", user.getUserId(), i + 1); logger.debug(message); } } @@ -316,7 +309,7 @@ DecisionResponse getVariationForFeatureInRollout(@Nonnull Featu // get last rule which is the fall back rule Experiment finalRule = rollout.getExperiments().get(rolloutRulesLength - 1); - decisionMeetAudience = ExperimentUtils.doesUserMeetAudienceConditions(projectConfig, finalRule, filteredAttributes, RULE, "Everyone Else"); + decisionMeetAudience = ExperimentUtils.doesUserMeetAudienceConditions(projectConfig, finalRule, user.getAttributes(), RULE, "Everyone Else"); reasons.merge(decisionMeetAudience.getReasons()); if (decisionMeetAudience.getResult()) { decisionVariation = bucketer.bucket(finalRule, bucketingId, projectConfig); @@ -324,7 +317,7 @@ DecisionResponse getVariationForFeatureInRollout(@Nonnull Featu reasons.merge(decisionVariation.getReasons()); if (variation != null) { - String message = reasons.addInfo("User \"%s\" meets conditions for targeting rule \"Everyone Else\".", userId); + String message = reasons.addInfo("User \"%s\" meets conditions for targeting rule \"Everyone Else\".", user.getUserId()); logger.debug(message); return new DecisionResponse( new FeatureDecision(finalRule, variation, FeatureDecision.DecisionSource.ROLLOUT), @@ -484,7 +477,10 @@ public ConcurrentHashMap> getForcedVar * @param variationKey The variation key to force the user into. If the variation key is null * then the forcedVariation for that experiment is removed. * @return boolean A boolean value that indicates if the set completed successfully. + * + * @deprecated use {@link OptimizelyUserContext#setForcedDecision(String, String, String)} instead */ + @Deprecated public boolean setForcedVariation(@Nonnull Experiment experiment, @Nonnull String userId, @Nullable String variationKey) { @@ -509,7 +505,7 @@ public boolean setForcedVariation(@Nonnull Experiment experiment, ConcurrentHashMap experimentToVariation; if (!forcedVariationMapping.containsKey(userId)) { - forcedVariationMapping.putIfAbsent(userId, new ConcurrentHashMap()); + forcedVariationMapping.putIfAbsent(userId, new ConcurrentHashMap()); } experimentToVariation = forcedVariationMapping.get(userId); @@ -551,7 +547,10 @@ public boolean setForcedVariation(@Nonnull Experiment experiment, * @param userId The user ID to be used for bucketing. * @return A {@link DecisionResponse} including the {@link Variation} that user is bucketed into (or null) * and the decision reasons. The variation can be null if the forced variation fails. + * + * @deprecated use {@link OptimizelyUserContext#getForcedDecision(String, String)} instead */ + @Deprecated @Nonnull public DecisionResponse getForcedVariation(@Nonnull Experiment experiment, @Nonnull String userId) { @@ -581,6 +580,33 @@ public DecisionResponse getForcedVariation(@Nonnull Experiment experi return new DecisionResponse(null, reasons); } + + public DecisionResponse getVariationFromExperimentRule(@Nonnull ProjectConfig projectConfig, + @Nonnull String flagKey, + @Nonnull Experiment rule, + @Nonnull OptimizelyUserContext user, + @Nonnull List options) { + DecisionReasons reasons = DefaultDecisionReasons.newInstance(); + + String ruleKey = rule != null ? rule.getKey() : null; + // Check Forced-Decision + DecisionResponse forcedDecisionResponse = user.findValidatedForcedDecision(flagKey, ruleKey); + + reasons.merge(forcedDecisionResponse.getReasons()); + + Variation variation = forcedDecisionResponse.getResult(); + if (variation != null) { + return new DecisionResponse(variation, reasons); + } + //regular decision + DecisionResponse decisionResponse = getVariation(rule, user, projectConfig, options); + reasons.merge(decisionResponse.getReasons()); + + variation = decisionResponse.getResult(); + + return new DecisionResponse(variation, reasons); + } + /** * Helper function to check that the provided userId is valid * diff --git a/core-api/src/main/java/com/optimizely/ab/bucketing/FeatureDecision.java b/core-api/src/main/java/com/optimizely/ab/bucketing/FeatureDecision.java index b0f0a11ed..6cb81509d 100644 --- a/core-api/src/main/java/com/optimizely/ab/bucketing/FeatureDecision.java +++ b/core-api/src/main/java/com/optimizely/ab/bucketing/FeatureDecision.java @@ -20,7 +20,7 @@ import javax.annotation.Nullable; -public class FeatureDecision { +public class FeatureDecision { /** * The {@link Experiment} the Feature is associated with. */ 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 0757b6d4e..643439ed7 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 @@ -80,6 +80,9 @@ public class DatafileProjectConfig implements ProjectConfig { private final Map experimentKeyMapping; private final Map featureKeyMapping; + // Key to Entity mappings for Forced Decisions + private final Map> flagVariationsMap; + // id to entity mappings private final Map audienceIdMapping; private final Map experimentIdMapping; @@ -209,6 +212,39 @@ public DatafileProjectConfig(String accountId, // Generate experiment to featureFlag list mapping to identify if experiment is AB-Test experiment or Feature-Test Experiment. this.experimentFeatureKeyMapping = ProjectConfigUtils.generateExperimentFeatureMapping(this.featureFlags); + + flagVariationsMap = new HashMap<>(); + if (featureFlags != null) { + for (FeatureFlag flag : featureFlags) { + Map variationsIdMap = new HashMap<>(); + List variations = new ArrayList<>(); + for (String experimentId : flag.getExperimentIds()) { + // We need to get each experiment by id and add to our list of experiments + Experiment exp = experimentIdMapping.get(experimentId); + variations.addAll(getVariationsFromExperiments(variationsIdMap, exp)); + + } + Rollout rollout = rolloutIdMapping.get(flag.getRolloutId()); + if (rollout != null) { + for (Experiment experiment : rollout.getExperiments()) { + variations.addAll(getVariationsFromExperiments(variationsIdMap, experiment)); + } + } + // Grab all the variations from the flag experiments and rollouts and add to flagVariationsMap + flagVariationsMap.put(flag.getKey(), variations); + } + } + } + + private List getVariationsFromExperiments(Map variationsIdMap, Experiment exp) { + List variations = new ArrayList<>(); + for (Variation variation : exp.getVariations()) { + if (!variationsIdMap.containsKey(variation.getId())) { + variationsIdMap.put(variation.getId(), variation); + variations.add(variation); + } + } + return variations; } /** @@ -463,6 +499,11 @@ public Map> getExperimentFeatureKeyMapping() { return experimentFeatureKeyMapping; } + @Override + public Map> getFlagVariationsMap() { + return flagVariationsMap; + } + @Override public String toString() { return "ProjectConfig{" + 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 a6222e8b2..9c3321708 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 @@ -103,6 +103,8 @@ Experiment getExperimentForKey(@Nonnull String experimentKey, Map> getExperimentFeatureKeyMapping(); + Map> getFlagVariationsMap(); + @Override String toString(); 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 a0c0541ac..3d1155b67 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java @@ -1716,8 +1716,7 @@ public void getEnabledFeaturesWithNoFeatureEnabled() throws Exception { FeatureDecision featureDecision = new FeatureDecision(null, null, FeatureDecision.DecisionSource.ROLLOUT); doReturn(DecisionResponse.responseNoReasons(featureDecision)).when(mockDecisionService).getVariationForFeature( any(FeatureFlag.class), - anyString(), - anyMapOf(String.class, String.class), + any(OptimizelyUserContext.class), any(ProjectConfig.class) ); int notificationId = optimizely.addDecisionNotificationHandler( decisionNotification -> { }); @@ -1833,8 +1832,7 @@ public void isFeatureEnabledWithListenerUserInExperimentFeatureOff() throws Exce FeatureDecision featureDecision = new FeatureDecision(activatedExperiment, variation, FeatureDecision.DecisionSource.FEATURE_TEST); doReturn(DecisionResponse.responseNoReasons(featureDecision)).when(mockDecisionService).getVariationForFeature( any(FeatureFlag.class), - anyString(), - anyMapOf(String.class, String.class), + any(OptimizelyUserContext.class), any(ProjectConfig.class) ); @@ -2710,7 +2708,7 @@ public void trackEventWithListenerNullAttributes() throws Exception { //======== Feature Accessor Tests ========// /** - * Verify {@link Optimizely#getFeatureVariableValueForType(String, String, String, Map, FeatureVariable.VariableType)} + * Verify {@link Optimizely#getFeatureVariableValueForType(String, String, String, Map, String)} * returns null and logs a message * when it is called with a feature key that has no corresponding feature in the datafile. */ @@ -2894,11 +2892,12 @@ public void getFeatureVariableValueReturnsDefaultValueWhenFeatureEnabledIsFalse( Optimizely optimizely = optimizelyBuilder.withDecisionService(mockDecisionService).build(); + Attribute attribute = new Attribute("Dummy","Dummy","Dummy"); + FeatureDecision featureDecision = new FeatureDecision(multivariateExperiment, VARIATION_MULTIVARIATE_EXPERIMENT_GRED, FeatureDecision.DecisionSource.FEATURE_TEST); doReturn(DecisionResponse.responseNoReasons(featureDecision)).when(mockDecisionService).getVariationForFeature( FEATURE_FLAG_MULTI_VARIATE_FEATURE, - genericUserId, - Collections.singletonMap(ATTRIBUTE_HOUSE_KEY, AUDIENCE_GRYFFINDOR_VALUE), + optimizely.createUserContext(genericUserId, Collections.singletonMap(ATTRIBUTE_HOUSE_KEY, AUDIENCE_GRYFFINDOR_VALUE)), validProjectConfig ); @@ -3110,8 +3109,7 @@ public void isFeatureEnabledReturnsFalseWhenFeatureKeyIsNull() throws Exception verify(mockDecisionService, never()).getVariationForFeature( any(FeatureFlag.class), - any(String.class), - anyMapOf(String.class, String.class), + any(OptimizelyUserContext.class), any(ProjectConfig.class) ); } @@ -3132,8 +3130,7 @@ public void isFeatureEnabledReturnsFalseWhenUserIdIsNull() throws Exception { verify(mockDecisionService, never()).getVariationForFeature( any(FeatureFlag.class), - any(String.class), - anyMapOf(String.class, String.class), + any(OptimizelyUserContext.class), any(ProjectConfig.class) ); } @@ -3156,8 +3153,7 @@ public void isFeatureEnabledReturnsFalseWhenFeatureFlagKeyIsInvalid() throws Exc verify(mockDecisionService, never()).getVariation( any(Experiment.class), - anyString(), - anyMapOf(String.class, String.class), + any(OptimizelyUserContext.class), any(ProjectConfig.class) ); } @@ -3179,8 +3175,7 @@ public void isFeatureEnabledReturnsFalseWhenUserIsNotBucketedIntoAnyVariation() FeatureDecision featureDecision = new FeatureDecision(null, null, null); doReturn(DecisionResponse.responseNoReasons(featureDecision)).when(mockDecisionService).getVariationForFeature( any(FeatureFlag.class), - anyString(), - anyMapOf(String.class, String.class), + any(OptimizelyUserContext.class), any(ProjectConfig.class) ); @@ -3195,8 +3190,7 @@ public void isFeatureEnabledReturnsFalseWhenUserIsNotBucketedIntoAnyVariation() verify(mockDecisionService).getVariationForFeature( eq(FEATURE_FLAG_MULTI_VARIATE_FEATURE), - eq(genericUserId), - eq(Collections.emptyMap()), + eq(optimizely.createUserContext(genericUserId, Collections.emptyMap())), eq(validProjectConfig) ); } @@ -3221,8 +3215,7 @@ public void isFeatureEnabledReturnsTrueButDoesNotSendWhenUserIsBucketedIntoVaria FeatureDecision featureDecision = new FeatureDecision(experiment, variation, FeatureDecision.DecisionSource.ROLLOUT); doReturn(DecisionResponse.responseNoReasons(featureDecision)).when(mockDecisionService).getVariationForFeature( eq(FEATURE_FLAG_MULTI_VARIATE_FEATURE), - eq(genericUserId), - eq(Collections.emptyMap()), + eq(optimizely.createUserContext(genericUserId, Collections.emptyMap())), eq(validProjectConfig) ); @@ -3242,8 +3235,7 @@ public void isFeatureEnabledReturnsTrueButDoesNotSendWhenUserIsBucketedIntoVaria verify(mockDecisionService).getVariationForFeature( eq(FEATURE_FLAG_MULTI_VARIATE_FEATURE), - eq(genericUserId), - eq(Collections.emptyMap()), + eq(optimizely.createUserContext(genericUserId, Collections.emptyMap())), eq(validProjectConfig) ); } @@ -3306,8 +3298,7 @@ public void isFeatureEnabledTrueWhenFeatureEnabledOfVariationIsTrue() throws Exc FeatureDecision featureDecision = new FeatureDecision(experiment, variation, FeatureDecision.DecisionSource.ROLLOUT); doReturn(DecisionResponse.responseNoReasons(featureDecision)).when(mockDecisionService).getVariationForFeature( eq(FEATURE_FLAG_MULTI_VARIATE_FEATURE), - eq(genericUserId), - eq(Collections.emptyMap()), + eq(optimizely.createUserContext(genericUserId, Collections.emptyMap())), eq(validProjectConfig) ); @@ -3336,8 +3327,7 @@ public void isFeatureEnabledFalseWhenFeatureEnabledOfVariationIsFalse() throws E FeatureDecision featureDecision = new FeatureDecision(experiment, variation, FeatureDecision.DecisionSource.ROLLOUT); doReturn(DecisionResponse.responseNoReasons(featureDecision)).when(mockDecisionService).getVariationForFeature( eq(FEATURE_FLAG_MULTI_VARIATE_FEATURE), - eq(genericUserId), - eq(Collections.emptyMap()), + eq(spyOptimizely.createUserContext(genericUserId, Collections.emptyMap())), eq(validProjectConfig) ); @@ -3366,8 +3356,7 @@ public void isFeatureEnabledReturnsFalseAndDispatchesWhenUserIsBucketedIntoAnExp FeatureDecision featureDecision = new FeatureDecision(activatedExperiment, variation, FeatureDecision.DecisionSource.FEATURE_TEST); doReturn(DecisionResponse.responseNoReasons(featureDecision)).when(mockDecisionService).getVariationForFeature( any(FeatureFlag.class), - anyString(), - anyMapOf(String.class, String.class), + any(OptimizelyUserContext.class), any(ProjectConfig.class) ); @@ -3422,8 +3411,7 @@ public void isFeatureEnabledWithInvalidDatafile() throws Exception { // make sure we didn't even attempt to bucket the user verify(mockDecisionService, never()).getVariationForFeature( any(FeatureFlag.class), - anyString(), - anyMap(), + any(OptimizelyUserContext.class), any(ProjectConfig.class) ); } @@ -3512,13 +3500,11 @@ public void getEnabledFeatureWithMockDecisionService() throws Exception { FeatureDecision featureDecision = new FeatureDecision(null, null, FeatureDecision.DecisionSource.ROLLOUT); doReturn(DecisionResponse.responseNoReasons(featureDecision)).when(mockDecisionService).getVariationForFeature( any(FeatureFlag.class), - anyString(), - anyMapOf(String.class, String.class), + any(OptimizelyUserContext.class), any(ProjectConfig.class) ); - List featureFlags = optimizely.getEnabledFeatures(genericUserId, - Collections.emptyMap()); + List featureFlags = optimizely.getEnabledFeatures(genericUserId, Collections.emptyMap()); assertTrue(featureFlags.isEmpty()); eventHandler.expectImpression(null, "", genericUserId); 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 0ac8beedb..7c0a28a73 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java @@ -26,6 +26,7 @@ import com.optimizely.ab.event.internal.payload.DecisionMetadata; import com.optimizely.ab.notification.NotificationCenter; 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; @@ -1190,6 +1191,169 @@ public void decideReasons_missingAttributeValue() { )); } + @Test + public void setForcedDecisionWithRuleKeyTest() { + String flagKey = "55555"; + String ruleKey = "77777"; + String variationKey = "33333"; + OptimizelyUserContext optimizelyUserContext = new OptimizelyUserContext( + optimizely, + userId, + Collections.emptyMap()); + + optimizelyUserContext.setForcedDecision(flagKey, ruleKey, variationKey); + OptimizelyUserContext.ForcedDecision forcedDecision = optimizelyUserContext.forcedDecisionsMap.get(flagKey).get(ruleKey); + assertEquals(forcedDecision.getFlagKey(), flagKey); + assertEquals(forcedDecision.getRuleKey(), ruleKey); + assertEquals(forcedDecision.getVariationKey(), variationKey); + } + + @Test + public void setForcedDecisionWithoutRuleKeyTest() { + String flagKey = "55555"; + String variationKey = "33333"; + OptimizelyUserContext optimizelyUserContext = new OptimizelyUserContext( + optimizely, + userId, + Collections.emptyMap()); + + optimizelyUserContext.setForcedDecision(flagKey, variationKey); + OptimizelyUserContext.ForcedDecision forcedDecision = optimizelyUserContext.forcedDecisionsMapWithNoRuleKey.get(flagKey); + assertEquals(forcedDecision.getFlagKey(), flagKey); + assertEquals(forcedDecision.getVariationKey(), variationKey); + } + + @Test + public void getForcedVariationWithRuleKey() { + String flagKey = "55555"; + String ruleKey = "77777"; + String variationKey = "33333"; + OptimizelyUserContext optimizelyUserContext = new OptimizelyUserContext( + optimizely, + userId, + Collections.emptyMap()); + + optimizelyUserContext.setForcedDecision(flagKey, ruleKey, variationKey); + assertEquals(variationKey, optimizelyUserContext.getForcedDecision(flagKey, ruleKey)); + } + + @Test + public void failedGetForcedDecisionWithRuleKey() { + String flagKey = "55555"; + String invalidFlagKey = "11"; + String ruleKey = "77777"; + String variationKey = "33333"; + OptimizelyUserContext optimizelyUserContext = new OptimizelyUserContext( + optimizely, + userId, + Collections.emptyMap()); + + optimizelyUserContext.setForcedDecision(flagKey, ruleKey, variationKey); + assertNull(optimizelyUserContext.getForcedDecision(invalidFlagKey, ruleKey)); + } + + @Test + public void getForcedVariationWithoutRuleKey() { + String flagKey = "55555"; + String variationKey = "33333"; + OptimizelyUserContext optimizelyUserContext = new OptimizelyUserContext( + optimizely, + userId, + Collections.emptyMap()); + + optimizelyUserContext.setForcedDecision(flagKey, variationKey); + assertEquals(variationKey, optimizelyUserContext.getForcedDecision(flagKey)); + } + + @Test + public void failedGetForcedDecisionWithoutRuleKey() { + String flagKey = "55555"; + String invalidFlagKey = "11"; + String variationKey = "33333"; + OptimizelyUserContext optimizelyUserContext = new OptimizelyUserContext( + optimizely, + userId, + Collections.emptyMap()); + + optimizelyUserContext.setForcedDecision(flagKey, variationKey); + assertNull(optimizelyUserContext.getForcedDecision(invalidFlagKey)); + } + + @Test + public void removeForcedDecisionWithRuleKey() { + String flagKey = "55555"; + String ruleKey = "77777"; + String variationKey = "33333"; + OptimizelyUserContext optimizelyUserContext = new OptimizelyUserContext( + optimizely, + userId, + Collections.emptyMap()); + + optimizelyUserContext.setForcedDecision(flagKey, ruleKey, variationKey); + assertEquals(true, optimizelyUserContext.removeForcedDecision(flagKey, ruleKey)); + } + + @Test + public void removeForcedDecisionWithoutRuleKey() { + String flagKey = "55555"; + String variationKey = "33333"; + OptimizelyUserContext optimizelyUserContext = new OptimizelyUserContext( + optimizely, + userId, + Collections.emptyMap()); + + optimizelyUserContext.setForcedDecision(flagKey, variationKey); + assertEquals(true, optimizelyUserContext.removeForcedDecision(flagKey)); + } + + @Test + public void removeAllForcedDecisions() { + String flagKey1 = "55555"; + String ruleKey1 = "77777"; + String variationKey1 = "33333"; + String flagKey2 = "11"; + String variationKey2 = "5"; + OptimizelyUserContext optimizelyUserContext = new OptimizelyUserContext( + optimizely, + userId, + Collections.emptyMap()); + + optimizelyUserContext.setForcedDecision(flagKey1, ruleKey1, variationKey1); + optimizelyUserContext.setForcedDecision(flagKey2, variationKey2); + assertEquals(true, optimizelyUserContext.removeAllForcedDecisions()); + } + + @Test + public void findValidatedForcedDecisionWithRuleKey() { + String ruleKey = "77777"; + String flagKey = "feature_2"; + String variationKey = "variation_no_traffic"; + OptimizelyUserContext optimizelyUserContext = new OptimizelyUserContext( + optimizely, + userId, + Collections.emptyMap()); + + optimizelyUserContext.setForcedDecision(flagKey, ruleKey, variationKey); + DecisionResponse response = optimizelyUserContext.findValidatedForcedDecision(flagKey, ruleKey); + Variation variation = response.getResult(); + assertEquals(variationKey, variation.getKey()); + } + + @Test + public void findValidatedForcedDecisionWithoutRuleKey() { + String flagKey = "feature_2"; + String variationKey = "variation_no_traffic"; + OptimizelyUserContext optimizelyUserContext = new OptimizelyUserContext( + optimizely, + userId, + Collections.emptyMap()); + + optimizelyUserContext.setForcedDecision(flagKey, variationKey); + DecisionResponse response = optimizelyUserContext.findValidatedForcedDecision(flagKey); + Variation variation = response.getResult(); + assertEquals(variationKey, variation.getKey()); + } + // utils Map createUserProfileMap(String experimentId, String variationId) { diff --git a/core-api/src/test/java/com/optimizely/ab/bucketing/DecisionServiceTest.java b/core-api/src/test/java/com/optimizely/ab/bucketing/DecisionServiceTest.java index 59dc47b22..55c5cfb67 100644 --- a/core-api/src/test/java/com/optimizely/ab/bucketing/DecisionServiceTest.java +++ b/core-api/src/test/java/com/optimizely/ab/bucketing/DecisionServiceTest.java @@ -16,6 +16,8 @@ package com.optimizely.ab.bucketing; import ch.qos.logback.classic.Level; +import com.optimizely.ab.Optimizely; +import com.optimizely.ab.OptimizelyUserContext; import com.optimizely.ab.config.*; import com.optimizely.ab.error.ErrorHandler; import com.optimizely.ab.internal.ControlAttribute; @@ -62,6 +64,8 @@ public class DecisionServiceTest { private Variation whitelistedVariation; private DecisionService decisionService; + private Optimizely optimizely; + @Rule public LogbackVerifier logbackVerifier = new LogbackVerifier(); @@ -74,13 +78,14 @@ public void setUp() throws Exception { whitelistedVariation = whitelistedExperiment.getVariationKeyToVariationMap().get("vtag1"); Bucketer bucketer = new Bucketer(); decisionService = spy(new DecisionService(bucketer, mockErrorHandler, null)); + this.optimizely = Optimizely.builder().build(); } //========= getVariation tests =========/ /** - * Verify that {@link DecisionService#getVariation(Experiment, String, Map, ProjectConfig)} + * Verify that {@link DecisionService#getVariation(Experiment, OptimizelyUserContext, ProjectConfig)} * gives precedence to forced variation bucketing over audience evaluation. */ @Test @@ -89,19 +94,24 @@ public void getVariationWhitelistedPrecedesAudienceEval() throws Exception { Variation expectedVariation = experiment.getVariations().get(0); // user excluded without audiences and whitelisting - assertNull(decisionService.getVariation(experiment, genericUserId, Collections.emptyMap(), validProjectConfig).getResult()); + assertNull(decisionService.getVariation( + experiment, + optimizely.createUserContext( + genericUserId, + null), + validProjectConfig).getResult()); logbackVerifier.expectMessage(Level.INFO, "User \"" + whitelistedUserId + "\" is forced in variation \"vtag1\"."); // no attributes provided for a experiment that has an audience - assertThat(decisionService.getVariation(experiment, whitelistedUserId, Collections.emptyMap(), validProjectConfig).getResult(), is(expectedVariation)); + assertThat(decisionService.getVariation(experiment, optimizely.createUserContext(whitelistedUserId, null), validProjectConfig).getResult(), is(expectedVariation)); verify(decisionService).getWhitelistedVariation(eq(experiment), eq(whitelistedUserId)); verify(decisionService, never()).getStoredVariation(eq(experiment), any(UserProfile.class), any(ProjectConfig.class)); } /** - * Verify that {@link DecisionService#getVariation(Experiment, String, Map, ProjectConfig)} + * Verify that {@link DecisionService#getVariation(Experiment, OptimizelyUserContext, ProjectConfig)} * gives precedence to forced variation bucketing over whitelisting. */ @Test @@ -111,23 +121,23 @@ public void getForcedVariationBeforeWhitelisting() throws Exception { Variation expectedVariation = experiment.getVariations().get(1); // user excluded without audiences and whitelisting - assertNull(decisionService.getVariation(experiment, genericUserId, Collections.emptyMap(), validProjectConfig).getResult()); + assertNull(decisionService.getVariation(experiment, optimizely.createUserContext(genericUserId, Collections.emptyMap()), validProjectConfig).getResult()); // set the runtimeForcedVariation decisionService.setForcedVariation(experiment, whitelistedUserId, expectedVariation.getKey()); // no attributes provided for a experiment that has an audience - assertThat(decisionService.getVariation(experiment, whitelistedUserId, Collections.emptyMap(), validProjectConfig).getResult(), is(expectedVariation)); + assertThat(decisionService.getVariation(experiment, optimizely.createUserContext(whitelistedUserId, Collections.emptyMap()), validProjectConfig).getResult(), is(expectedVariation)); //verify(decisionService).getForcedVariation(experiment.getKey(), whitelistedUserId); verify(decisionService, never()).getStoredVariation(eq(experiment), any(UserProfile.class), any(ProjectConfig.class)); assertEquals(decisionService.getWhitelistedVariation(experiment, whitelistedUserId).getResult(), whitelistVariation); assertTrue(decisionService.setForcedVariation(experiment, whitelistedUserId, null)); assertNull(decisionService.getForcedVariation(experiment, whitelistedUserId).getResult()); - assertThat(decisionService.getVariation(experiment, whitelistedUserId, Collections.emptyMap(), validProjectConfig).getResult(), is(whitelistVariation)); + assertThat(decisionService.getVariation(experiment, optimizely.createUserContext(whitelistedUserId, Collections.emptyMap()), validProjectConfig).getResult(), is(whitelistVariation)); } /** - * Verify that {@link DecisionService#getVariation(Experiment, String, Map, ProjectConfig)} + * Verify that {@link DecisionService#getVariation(Experiment, OptimizelyUserContext, ProjectConfig)} * gives precedence to forced variation bucketing over audience evaluation. */ @Test @@ -136,12 +146,12 @@ public void getVariationForcedPrecedesAudienceEval() throws Exception { Variation expectedVariation = experiment.getVariations().get(1); // user excluded without audiences and whitelisting - assertNull(decisionService.getVariation(experiment, genericUserId, Collections.emptyMap(), validProjectConfig).getResult()); + assertNull(decisionService.getVariation(experiment, optimizely.createUserContext(genericUserId, Collections.emptyMap()), validProjectConfig).getResult()); // set the runtimeForcedVariation decisionService.setForcedVariation(experiment, genericUserId, expectedVariation.getKey()); // no attributes provided for a experiment that has an audience - assertThat(decisionService.getVariation(experiment, genericUserId, Collections.emptyMap(), validProjectConfig).getResult(), is(expectedVariation)); + assertThat(decisionService.getVariation(experiment, optimizely.createUserContext(genericUserId, Collections.emptyMap()), validProjectConfig).getResult(), is(expectedVariation)); verify(decisionService, never()).getStoredVariation(eq(experiment), any(UserProfile.class), eq(validProjectConfig)); assertEquals(decisionService.setForcedVariation(experiment, genericUserId, null), true); @@ -149,7 +159,7 @@ public void getVariationForcedPrecedesAudienceEval() throws Exception { } /** - * Verify that {@link DecisionService#getVariation(Experiment, String, Map, ProjectConfig)} + * Verify that {@link DecisionService#getVariation(Experiment, OptimizelyUserContext, ProjectConfig)} * gives precedence to forced variation bucketing over user profile. */ @Test @@ -165,22 +175,22 @@ public void getVariationForcedBeforeUserProfile() throws Exception { DecisionService decisionService = spy(new DecisionService(new Bucketer(), mockErrorHandler, userProfileService)); // ensure that normal users still get excluded from the experiment when they fail audience evaluation - assertNull(decisionService.getVariation(experiment, genericUserId, Collections.emptyMap(), validProjectConfig).getResult()); + assertNull(decisionService.getVariation(experiment, optimizely.createUserContext(genericUserId, Collections.emptyMap()), validProjectConfig).getResult()); // ensure that a user with a saved user profile, sees the same variation regardless of audience evaluation assertEquals(variation, - decisionService.getVariation(experiment, userProfileId, Collections.emptyMap(), validProjectConfig).getResult()); + decisionService.getVariation(experiment, optimizely.createUserContext(userProfileId, Collections.emptyMap()), validProjectConfig).getResult()); Variation forcedVariation = experiment.getVariations().get(1); decisionService.setForcedVariation(experiment, userProfileId, forcedVariation.getKey()); assertEquals(forcedVariation, - decisionService.getVariation(experiment, userProfileId, Collections.emptyMap(), validProjectConfig).getResult()); + decisionService.getVariation(experiment, optimizely.createUserContext(userProfileId, Collections.emptyMap()), validProjectConfig).getResult()); assertTrue(decisionService.setForcedVariation(experiment, userProfileId, null)); assertNull(decisionService.getForcedVariation(experiment, userProfileId).getResult()); } /** - * Verify that {@link DecisionService#getVariation(Experiment, String, Map, ProjectConfig)} + * Verify that {@link DecisionService#getVariation(Experiment, OptimizelyUserContext, ProjectConfig)} * gives precedence to user profile over audience evaluation. */ @Test @@ -196,16 +206,16 @@ public void getVariationEvaluatesUserProfileBeforeAudienceTargeting() throws Exc DecisionService decisionService = spy(new DecisionService(new Bucketer(), mockErrorHandler, userProfileService)); // ensure that normal users still get excluded from the experiment when they fail audience evaluation - assertNull(decisionService.getVariation(experiment, genericUserId, Collections.emptyMap(), validProjectConfig).getResult()); + assertNull(decisionService.getVariation(experiment, optimizely.createUserContext(genericUserId, Collections.emptyMap()), validProjectConfig).getResult()); // ensure that a user with a saved user profile, sees the same variation regardless of audience evaluation assertEquals(variation, - decisionService.getVariation(experiment, userProfileId, Collections.emptyMap(), validProjectConfig).getResult()); + decisionService.getVariation(experiment, optimizely.createUserContext(userProfileId, Collections.emptyMap()), validProjectConfig).getResult()); } /** - * Verify that {@link DecisionService#getVariation(Experiment, String, Map, ProjectConfig)} + * Verify that {@link DecisionService#getVariation(Experiment, OptimizelyUserContext, ProjectConfig)} * gives a null variation on a Experiment that is not running. Set the forced variation. * And, test to make sure that after setting forced variation, the getVariation still returns * null. @@ -217,7 +227,7 @@ public void getVariationOnNonRunningExperimentWithForcedVariation() { Variation variation = experiment.getVariations().get(0); // ensure that the not running variation returns null with no forced variation set. - assertNull(decisionService.getVariation(experiment, "userId", Collections.emptyMap(), validProjectConfig).getResult()); + assertNull(decisionService.getVariation(experiment, optimizely.createUserContext("userId", Collections.emptyMap()), validProjectConfig).getResult()); // we call getVariation 3 times on an experiment that is not running. logbackVerifier.expectMessage(Level.INFO, @@ -228,12 +238,12 @@ public void getVariationOnNonRunningExperimentWithForcedVariation() { // ensure that a user with a forced variation set // still gets back a null variation if the variation is not running. - assertNull(decisionService.getVariation(experiment, "userId", Collections.emptyMap(), validProjectConfig).getResult()); + assertNull(decisionService.getVariation(experiment, optimizely.createUserContext("userId", Collections.emptyMap()), validProjectConfig).getResult()); // set the forced variation back to null assertTrue(decisionService.setForcedVariation(experiment, "userId", null)); // test one more time that the getVariation returns null for the experiment that is not running. - assertNull(decisionService.getVariation(experiment, "userId", Collections.emptyMap(), validProjectConfig).getResult()); + assertNull(decisionService.getVariation(experiment, optimizely.createUserContext("userid", Collections.emptyMap()), validProjectConfig).getResult()); } @@ -241,7 +251,7 @@ public void getVariationOnNonRunningExperimentWithForcedVariation() { //========== get Variation for Feature tests ==========// /** - * Verify that {@link DecisionService#getVariationForFeature(FeatureFlag, String, Map, ProjectConfig)} + * Verify that {@link DecisionService#getVariationForFeature(FeatureFlag, OptimizelyUserContext, ProjectConfig)} * returns null when the {@link FeatureFlag} is not used in any experiments or rollouts. */ @Test @@ -263,8 +273,7 @@ public void getVariationForFeatureReturnsNullWhenFeatureFlagExperimentIdsIsEmpty FeatureDecision featureDecision = decisionService.getVariationForFeature( emptyFeatureFlag, - genericUserId, - Collections.emptyMap(), + optimizely.createUserContext(genericUserId, Collections.emptyMap()), validProjectConfig).getResult(); assertNull(featureDecision.variation); assertNull(featureDecision.decisionSource); @@ -275,7 +284,7 @@ public void getVariationForFeatureReturnsNullWhenFeatureFlagExperimentIdsIsEmpty } /** - * Verify that {@link DecisionService#getVariationForFeature(FeatureFlag, String, Map, ProjectConfig)} + * Verify that {@link DecisionService#getVariationForFeature(FeatureFlag, OptimizelyUserContext, ProjectConfig)} * returns null when the user is not bucketed into any experiments or rollouts for the {@link FeatureFlag}. */ @Test @@ -286,24 +295,21 @@ public void getVariationForFeatureReturnsNullWhenItGetsNoVariationsForExperiment // do not bucket to any experiments doReturn(DecisionResponse.nullNoReasons()).when(decisionService).getVariation( any(Experiment.class), - anyString(), - anyMapOf(String.class, String.class), + any(OptimizelyUserContext.class), any(ProjectConfig.class), anyObject() ); // do not bucket to any rollouts doReturn(DecisionResponse.responseNoReasons(new FeatureDecision(null, null, null))).when(decisionService).getVariationForFeatureInRollout( any(FeatureFlag.class), - anyString(), - anyMapOf(String.class, String.class), + any(OptimizelyUserContext.class), any(ProjectConfig.class) ); // try to get a variation back from the decision service for the feature flag FeatureDecision featureDecision = decisionService.getVariationForFeature( spyFeatureFlag, - genericUserId, - Collections.emptyMap(), + optimizely.createUserContext(genericUserId, Collections.emptyMap()), validProjectConfig ).getResult(); assertNull(featureDecision.variation); @@ -314,11 +320,11 @@ public void getVariationForFeatureReturnsNullWhenItGetsNoVariationsForExperiment FEATURE_MULTI_VARIATE_FEATURE_KEY + "\"."); verify(spyFeatureFlag, times(2)).getExperimentIds(); - verify(spyFeatureFlag, times(1)).getKey(); + verify(spyFeatureFlag, times(2)).getKey(); } /** - * Verify that {@link DecisionService#getVariationForFeature(FeatureFlag, String, Map, ProjectConfig)} + * Verify that {@link DecisionService#getVariationForFeature(FeatureFlag, OptimizelyUserContext, ProjectConfig)} * returns the variation of the experiment a user gets bucketed into for an experiment. */ @Test @@ -328,36 +334,33 @@ public void getVariationForFeatureReturnsVariationReturnedFromGetVariation() { doReturn(DecisionResponse.nullNoReasons()).when(decisionService).getVariation( eq(ValidProjectConfigV4.EXPERIMENT_MUTEX_GROUP_EXPERIMENT_1), - anyString(), - anyMapOf(String.class, String.class), + any(OptimizelyUserContext.class), any(ProjectConfig.class), anyObject() ); doReturn(DecisionResponse.responseNoReasons(ValidProjectConfigV4.VARIATION_MUTEX_GROUP_EXP_2_VAR_1)).when(decisionService).getVariation( eq(ValidProjectConfigV4.EXPERIMENT_MUTEX_GROUP_EXPERIMENT_2), - anyString(), - anyMapOf(String.class, String.class), + any(OptimizelyUserContext.class), any(ProjectConfig.class), anyObject() ); FeatureDecision featureDecision = decisionService.getVariationForFeature( spyFeatureFlag, - genericUserId, - Collections.emptyMap(), + optimizely.createUserContext(genericUserId, Collections.emptyMap()), v4ProjectConfig ).getResult(); assertEquals(ValidProjectConfigV4.VARIATION_MUTEX_GROUP_EXP_2_VAR_1, featureDecision.variation); assertEquals(FeatureDecision.DecisionSource.FEATURE_TEST, featureDecision.decisionSource); verify(spyFeatureFlag, times(2)).getExperimentIds(); - verify(spyFeatureFlag, never()).getKey(); + verify(spyFeatureFlag, times(2)).getKey(); } /** * Verify that when getting a {@link Variation} for a {@link FeatureFlag} in - * {@link DecisionService#getVariationForFeature(FeatureFlag, String, Map, ProjectConfig)}, + * {@link DecisionService#getVariationForFeature(FeatureFlag, OptimizelyUserContext, ProjectConfig)}, * check first if the user is bucketed to an {@link Experiment} * then check if the user is not bucketed to an experiment, * check for a {@link Rollout}. @@ -376,8 +379,7 @@ public void getVariationForFeatureReturnsVariationFromExperimentBeforeRollout() doReturn(DecisionResponse.responseNoReasons(experimentVariation)) .when(decisionService).getVariation( eq(featureExperiment), - anyString(), - anyMapOf(String.class, String.class), + any(OptimizelyUserContext.class), any(ProjectConfig.class), anyObject() ); @@ -386,16 +388,14 @@ public void getVariationForFeatureReturnsVariationFromExperimentBeforeRollout() doReturn(DecisionResponse.responseNoReasons(new FeatureDecision(rolloutExperiment, rolloutVariation, FeatureDecision.DecisionSource.ROLLOUT))) .when(decisionService).getVariationForFeatureInRollout( eq(featureFlag), - anyString(), - anyMapOf(String.class, String.class), + any(OptimizelyUserContext.class), any(ProjectConfig.class) ); // make sure we get the right variation back FeatureDecision featureDecision = decisionService.getVariationForFeature( featureFlag, - genericUserId, - Collections.emptyMap(), + optimizely.createUserContext(genericUserId, Collections.emptyMap()), v4ProjectConfig ).getResult(); assertEquals(experimentVariation, featureDecision.variation); @@ -404,16 +404,14 @@ public void getVariationForFeatureReturnsVariationFromExperimentBeforeRollout() // make sure we do not even check for rollout bucketing verify(decisionService, never()).getVariationForFeatureInRollout( any(FeatureFlag.class), - anyString(), - anyMapOf(String.class, String.class), + any(OptimizelyUserContext.class), any(ProjectConfig.class) ); // make sure we ask for experiment bucketing once verify(decisionService, times(1)).getVariation( any(Experiment.class), - anyString(), - anyMapOf(String.class, String.class), + any(OptimizelyUserContext.class), any(ProjectConfig.class), anyObject() ); @@ -421,7 +419,7 @@ public void getVariationForFeatureReturnsVariationFromExperimentBeforeRollout() /** * Verify that when getting a {@link Variation} for a {@link FeatureFlag} in - * {@link DecisionService#getVariationForFeature(FeatureFlag, String, Map, ProjectConfig)}, + * {@link DecisionService#getVariationForFeature(FeatureFlag, OptimizelyUserContext, ProjectConfig)}, * check first if the user is bucketed to an {@link Rollout} * if the user is not bucketed to an experiment. */ @@ -438,8 +436,7 @@ public void getVariationForFeatureReturnsVariationFromRolloutWhenExperimentFails doReturn(DecisionResponse.nullNoReasons()) .when(decisionService).getVariation( eq(featureExperiment), - anyString(), - anyMapOf(String.class, String.class), + any(OptimizelyUserContext.class), any(ProjectConfig.class), anyObject() ); @@ -448,16 +445,14 @@ public void getVariationForFeatureReturnsVariationFromRolloutWhenExperimentFails doReturn(DecisionResponse.responseNoReasons(new FeatureDecision(rolloutExperiment, rolloutVariation, FeatureDecision.DecisionSource.ROLLOUT))) .when(decisionService).getVariationForFeatureInRollout( eq(featureFlag), - anyString(), - anyMapOf(String.class, String.class), + any(OptimizelyUserContext.class), any(ProjectConfig.class) ); // make sure we get the right variation back FeatureDecision featureDecision = decisionService.getVariationForFeature( featureFlag, - genericUserId, - Collections.emptyMap(), + optimizely.createUserContext(genericUserId, Collections.emptyMap()), v4ProjectConfig ).getResult(); assertEquals(rolloutVariation, featureDecision.variation); @@ -466,16 +461,14 @@ public void getVariationForFeatureReturnsVariationFromRolloutWhenExperimentFails // make sure we do not even check for rollout bucketing verify(decisionService, times(1)).getVariationForFeatureInRollout( any(FeatureFlag.class), - anyString(), - anyMapOf(String.class, String.class), + any(OptimizelyUserContext.class), any(ProjectConfig.class) ); // make sure we ask for experiment bucketing once verify(decisionService, times(1)).getVariation( any(Experiment.class), - anyString(), - anyMapOf(String.class, String.class), + any(OptimizelyUserContext.class), any(ProjectConfig.class), anyObject() ); @@ -490,7 +483,7 @@ public void getVariationForFeatureReturnsVariationFromRolloutWhenExperimentFails //========== getVariationForFeatureInRollout tests ==========// /** - * Verify that {@link DecisionService#getVariationForFeatureInRollout(FeatureFlag, String, Map, ProjectConfig)} + * Verify that {@link DecisionService#getVariationForFeatureInRollout(FeatureFlag, OptimizelyUserContext, ProjectConfig)} * returns null when trying to bucket a user into a {@link FeatureFlag} * that does not have a {@link Rollout} attached. */ @@ -503,8 +496,7 @@ public void getVariationForFeatureInRolloutReturnsNullWhenFeatureIsNotAttachedTo FeatureDecision featureDecision = decisionService.getVariationForFeatureInRollout( mockFeatureFlag, - genericUserId, - Collections.emptyMap(), + optimizely.createUserContext(genericUserId, Collections.emptyMap()), validProjectConfig ).getResult(); assertNull(featureDecision.variation); @@ -517,7 +509,7 @@ public void getVariationForFeatureInRolloutReturnsNullWhenFeatureIsNotAttachedTo } /** - * Verify that {@link DecisionService#getVariationForFeatureInRollout(FeatureFlag, String, Map, ProjectConfig)} + * Verify that {@link DecisionService#getVariationForFeatureInRollout(FeatureFlag, OptimizelyUserContext, ProjectConfig)} * return null when a user is excluded from every rule of a rollout due to traffic allocation. */ @Test @@ -533,10 +525,7 @@ public void getVariationForFeatureInRolloutReturnsNullWhenUserIsExcludedFromAllT FeatureDecision featureDecision = decisionService.getVariationForFeatureInRollout( FEATURE_FLAG_MULTI_VARIATE_FEATURE, - genericUserId, - Collections.singletonMap( - ATTRIBUTE_HOUSE_KEY, AUDIENCE_GRYFFINDOR_VALUE - ), + optimizely.createUserContext(genericUserId, Collections.singletonMap(ATTRIBUTE_HOUSE_KEY, AUDIENCE_GRYFFINDOR_VALUE)), v4ProjectConfig ).getResult(); assertNull(featureDecision.variation); @@ -549,7 +538,7 @@ public void getVariationForFeatureInRolloutReturnsNullWhenUserIsExcludedFromAllT } /** - * Verify that {@link DecisionService#getVariationForFeatureInRollout(FeatureFlag, String, Map, ProjectConfig)} + * Verify that {@link DecisionService#getVariationForFeatureInRollout(FeatureFlag, OptimizelyUserContext, ProjectConfig)} * returns null when a user is excluded from every rule of a rollout due to targeting * and also fails traffic allocation in the everyone else rollout. */ @@ -562,8 +551,7 @@ public void getVariationForFeatureInRolloutReturnsNullWhenUserFailsAllAudiencesA FeatureDecision featureDecision = decisionService.getVariationForFeatureInRollout( FEATURE_FLAG_MULTI_VARIATE_FEATURE, - genericUserId, - Collections.emptyMap(), + optimizely.createUserContext(genericUserId, Collections.emptyMap()), v4ProjectConfig ).getResult(); assertNull(featureDecision.variation); @@ -574,7 +562,7 @@ public void getVariationForFeatureInRolloutReturnsNullWhenUserFailsAllAudiencesA } /** - * Verify that {@link DecisionService#getVariationForFeatureInRollout(FeatureFlag, String, Map, ProjectConfig)} + * Verify that {@link DecisionService#getVariationForFeatureInRollout(FeatureFlag, OptimizelyUserContext, ProjectConfig)} * returns the variation of "Everyone Else" rule * when the user fails targeting for all rules, but is bucketed into the "Everyone Else" rule. */ @@ -594,8 +582,7 @@ public void getVariationForFeatureInRolloutReturnsVariationWhenUserFailsAllAudie FeatureDecision featureDecision = decisionService.getVariationForFeatureInRollout( FEATURE_FLAG_MULTI_VARIATE_FEATURE, - genericUserId, - Collections.emptyMap(), + optimizely.createUserContext(genericUserId, Collections.emptyMap()), v4ProjectConfig ).getResult(); logbackVerifier.expectMessage(Level.DEBUG, "Evaluating audiences for rule \"1\": [3468206642]."); @@ -614,7 +601,7 @@ public void getVariationForFeatureInRolloutReturnsVariationWhenUserFailsAllAudie } /** - * Verify that {@link DecisionService#getVariationForFeatureInRollout(FeatureFlag, String, Map, ProjectConfig)} + * Verify that {@link DecisionService#getVariationForFeatureInRollout(FeatureFlag, OptimizelyUserContext, ProjectConfig)} * returns the variation of "Everyone Else" rule * when the user passes targeting for a rule, but was failed the traffic allocation for that rule, * and is bucketed successfully into the "Everyone Else" rule. @@ -636,10 +623,7 @@ public void getVariationForFeatureInRolloutReturnsVariationWhenUserFailsTrafficI FeatureDecision featureDecision = decisionService.getVariationForFeatureInRollout( FEATURE_FLAG_MULTI_VARIATE_FEATURE, - genericUserId, - Collections.singletonMap( - ATTRIBUTE_HOUSE_KEY, AUDIENCE_GRYFFINDOR_VALUE - ), + optimizely.createUserContext(genericUserId, Collections.singletonMap(ATTRIBUTE_HOUSE_KEY, AUDIENCE_GRYFFINDOR_VALUE)), v4ProjectConfig ).getResult(); assertEquals(expectedVariation, featureDecision.variation); @@ -652,7 +636,7 @@ public void getVariationForFeatureInRolloutReturnsVariationWhenUserFailsTrafficI } /** - * Verify that {@link DecisionService#getVariationForFeatureInRollout(FeatureFlag, String, Map, ProjectConfig)} + * Verify that {@link DecisionService#getVariationForFeatureInRollout(FeatureFlag, OptimizelyUserContext, ProjectConfig)} * returns the variation of "Everyone Else" rule * when the user passes targeting for a rule, but was failed the traffic allocation for that rule, * and is bucketed successfully into the "Everyone Else" rule. @@ -679,15 +663,14 @@ public void getVariationForFeatureInRolloutReturnsVariationWhenUserFailsTrafficI FeatureDecision featureDecision = decisionService.getVariationForFeatureInRollout( FEATURE_FLAG_MULTI_VARIATE_FEATURE, - genericUserId, - DatafileProjectConfigTestUtils.createMapOfObjects( + optimizely.createUserContext(genericUserId, DatafileProjectConfigTestUtils.createMapOfObjects( DatafileProjectConfigTestUtils.createListOfObjects( ATTRIBUTE_HOUSE_KEY, ATTRIBUTE_NATIONALITY_KEY ), DatafileProjectConfigTestUtils.createListOfObjects( AUDIENCE_GRYFFINDOR_VALUE, AUDIENCE_ENGLISH_CITIZENS_VALUE ) - ), + )), v4ProjectConfig ).getResult(); assertEquals(expectedVariation, featureDecision.variation); @@ -698,7 +681,7 @@ public void getVariationForFeatureInRolloutReturnsVariationWhenUserFailsTrafficI } /** - * Verify that {@link DecisionService#getVariationForFeatureInRollout(FeatureFlag, String, Map, ProjectConfig)} + * Verify that {@link DecisionService#getVariationForFeatureInRollout(FeatureFlag, OptimizelyUserContext, ProjectConfig)} * returns the variation of "English Citizens" rule * when the user fails targeting for previous rules, but passes targeting and traffic for Rule 3. */ @@ -718,10 +701,7 @@ public void getVariationForFeatureInRolloutReturnsVariationWhenUserFailsTargetin FeatureDecision featureDecision = decisionService.getVariationForFeatureInRollout( FEATURE_FLAG_MULTI_VARIATE_FEATURE, - genericUserId, - Collections.singletonMap( - ATTRIBUTE_NATIONALITY_KEY, AUDIENCE_ENGLISH_CITIZENS_VALUE - ), + optimizely.createUserContext(genericUserId, Collections.singletonMap(ATTRIBUTE_NATIONALITY_KEY, AUDIENCE_ENGLISH_CITIZENS_VALUE)), v4ProjectConfig ).getResult(); assertEquals(englishCitizenVariation, featureDecision.variation); @@ -811,7 +791,7 @@ public void bucketReturnsVariationStoredInUserProfile() throws Exception { // ensure user with an entry in the user profile is bucketed into the corresponding stored variation assertEquals(variation, - decisionService.getVariation(experiment, userProfileId, Collections.emptyMap(), noAudienceProjectConfig).getResult()); + decisionService.getVariation(experiment, optimizely.createUserContext(userProfileId, Collections.emptyMap()), noAudienceProjectConfig).getResult()); verify(userProfileService).lookup(userProfileId); } @@ -867,7 +847,7 @@ public void getStoredVariationReturnsNullWhenVariationIsNoLongerInConfig() throw } /** - * Verify that {@link DecisionService#getVariation(Experiment, String, Map, ProjectConfig)} + * Verify that {@link DecisionService#getVariation(Experiment, OptimizelyUserContext, ProjectConfig)} * saves a {@link Variation}of an {@link Experiment} for a user when a {@link UserProfileService} is present. */ @SuppressFBWarnings @@ -890,7 +870,7 @@ public void getVariationSavesBucketedVariationIntoUserProfile() throws Exception DecisionService decisionService = new DecisionService(mockBucketer, mockErrorHandler, userProfileService); assertEquals(variation, decisionService.getVariation( - experiment, userProfileId, Collections.emptyMap(), noAudienceProjectConfig).getResult() + experiment, optimizely.createUserContext(userProfileId, Collections.emptyMap()), noAudienceProjectConfig).getResult() ); logbackVerifier.expectMessage(Level.INFO, String.format("Saved variation \"%s\" of experiment \"%s\" for user \"" + userProfileId + "\".", variation.getId(), @@ -900,7 +880,7 @@ public void getVariationSavesBucketedVariationIntoUserProfile() throws Exception } /** - * Verify that {@link DecisionService#getVariation(Experiment, String, Map, ProjectConfig)} logs correctly + * Verify that {@link DecisionService#getVariation(Experiment, OptimizelyUserContext, ProjectConfig)} logs correctly * when a {@link UserProfileService} is present but fails to save an activation. */ @Test @@ -950,7 +930,7 @@ public void getVariationSavesANewUserProfile() throws Exception { when(bucketer.bucket(eq(experiment), eq(userProfileId), eq(noAudienceProjectConfig))).thenReturn(DecisionResponse.responseNoReasons(variation)); when(userProfileService.lookup(userProfileId)).thenReturn(null); - assertEquals(variation, decisionService.getVariation(experiment, userProfileId, Collections.emptyMap(), noAudienceProjectConfig).getResult()); + assertEquals(variation, decisionService.getVariation(experiment, optimizely.createUserContext(userProfileId, Collections.emptyMap()), noAudienceProjectConfig).getResult()); verify(userProfileService).save(expectedUserProfile.toMap()); } @@ -963,15 +943,15 @@ public void getVariationBucketingId() throws Exception { when(bucketer.bucket(eq(experiment), eq("bucketId"), eq(validProjectConfig))).thenReturn(DecisionResponse.responseNoReasons(expectedVariation)); - Map attr = new HashMap(); + Map attr = new HashMap(); attr.put(ControlAttribute.BUCKETING_ATTRIBUTE.toString(), "bucketId"); // user excluded without audiences and whitelisting - assertThat(decisionService.getVariation(experiment, genericUserId, attr, validProjectConfig).getResult(), is(expectedVariation)); + assertThat(decisionService.getVariation(experiment, optimizely.createUserContext(genericUserId, attr), validProjectConfig).getResult(), is(expectedVariation)); } /** - * Verify that {@link DecisionService#getVariationForFeatureInRollout(FeatureFlag, String, Map, ProjectConfig)} + * Verify that {@link DecisionService#getVariationForFeatureInRollout(FeatureFlag, OptimizelyUserContext, ProjectConfig)} * uses bucketing ID to bucket the user into rollouts. */ @Test @@ -981,7 +961,7 @@ public void getVariationForRolloutWithBucketingId() { FeatureFlag featureFlag = FEATURE_FLAG_SINGLE_VARIABLE_INTEGER; String bucketingId = "user_bucketing_id"; String userId = "user_id"; - Map attributes = new HashMap(); + Map attributes = new HashMap(); attributes.put(ControlAttribute.BUCKETING_ATTRIBUTE.toString(), bucketingId); Bucketer bucketer = mock(Bucketer.class); @@ -999,7 +979,7 @@ public void getVariationForRolloutWithBucketingId() { rolloutVariation, FeatureDecision.DecisionSource.ROLLOUT); - FeatureDecision featureDecision = decisionService.getVariationForFeature(featureFlag, userId, attributes, v4ProjectConfig).getResult(); + FeatureDecision featureDecision = decisionService.getVariationForFeature(featureFlag, optimizely.createUserContext(userId, attributes), v4ProjectConfig).getResult(); assertEquals(expectedFeatureDecision, featureDecision); } From bd41dd13f84308a840592a2b9dcdb26bd04f6134 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Tue, 28 Sep 2021 12:31:03 -0400 Subject: [PATCH 02/39] Update constructor to use ? instead of object - reverting --- .../main/java/com/optimizely/ab/Optimizely.java | 16 ++++++++-------- .../com/optimizely/ab/OptimizelyUserContext.java | 2 +- 2 files changed, 9 insertions(+), 9 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 6459f383a..cf643becc 100644 --- a/core-api/src/main/java/com/optimizely/ab/Optimizely.java +++ b/core-api/src/main/java/com/optimizely/ab/Optimizely.java @@ -422,7 +422,7 @@ private Boolean isFeatureEnabled(@Nonnull ProjectConfig projectConfig, return false; } - Map copiedAttributes = copyAttributes(attributes); + Map copiedAttributes = copyAttributes(attributes); FeatureDecision.DecisionSource decisionSource = FeatureDecision.DecisionSource.ROLLOUT; FeatureDecision featureDecision = decisionService.getVariationForFeature(featureFlag, createUserContext(userId, copiedAttributes), projectConfig).getResult(); Boolean featureEnabled = false; @@ -732,7 +732,7 @@ T getFeatureVariableValueForType(@Nonnull String featureKey, } String variableValue = variable.getDefaultValue(); - Map copiedAttributes = copyAttributes(attributes); + Map copiedAttributes = copyAttributes(attributes); FeatureDecision featureDecision = decisionService.getVariationForFeature(featureFlag, createUserContext(userId, copiedAttributes), projectConfig).getResult(); Boolean featureEnabled = false; if (featureDecision.variation != null) { @@ -865,7 +865,7 @@ public OptimizelyJSON getAllFeatureVariables(@Nonnull String featureKey, return null; } - Map copiedAttributes = copyAttributes(attributes); + Map copiedAttributes = copyAttributes(attributes); FeatureDecision featureDecision = decisionService.getVariationForFeature(featureFlag, createUserContext(userId, copiedAttributes), projectConfig, Collections.emptyList()).getResult(); Boolean featureEnabled = false; Variation variation = featureDecision.variation; @@ -935,7 +935,7 @@ public List getEnabledFeatures(@Nonnull String userId, @Nonnull Map copiedAttributes = copyAttributes(attributes); + Map copiedAttributes = copyAttributes(attributes); for (FeatureFlag featureFlag : projectConfig.getFeatureFlags()) { String featureKey = featureFlag.getKey(); if (isFeatureEnabled(projectConfig, featureKey, userId, copiedAttributes)) @@ -951,7 +951,7 @@ public List getEnabledFeatures(@Nonnull String userId, @Nonnull MapemptyMap()); + return getVariation(experiment, userId, Collections.emptyMap()); } @Nullable @@ -966,7 +966,7 @@ private Variation getVariation(@Nonnull ProjectConfig projectConfig, @Nonnull Experiment experiment, @Nonnull String userId, @Nonnull Map attributes) throws UnknownExperimentException { - Map copiedAttributes = copyAttributes(attributes); + Map copiedAttributes = copyAttributes(attributes); Variation variation = decisionService.getVariation(experiment, createUserContext(userId, copiedAttributes), projectConfig).getResult(); String notificationType = NotificationCenter.DecisionNotificationType.AB_TEST.toString(); @@ -1150,7 +1150,7 @@ public OptimizelyConfig getOptimizelyConfig() { * @return An OptimizelyUserContext associated with this OptimizelyClient. */ public OptimizelyUserContext createUserContext(@Nonnull String userId, - @Nonnull Map attributes) { + @Nonnull Map attributes) { if (userId == null) { logger.warn("The userId parameter must be nonnull."); return null; @@ -1360,7 +1360,7 @@ public Variation getFlagVariationByKey(String flagKey, String variationKey) { * @param attributes map to copy * @return copy of attributes */ - private Map copyAttributes(Map attributes) { + private Map copyAttributes(Map attributes) { Map copiedAttributes = null; if (attributes != null) { copiedAttributes = new HashMap<>(attributes); 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 15f6fb0ab..ed4d2938f 100644 --- a/core-api/src/main/java/com/optimizely/ab/OptimizelyUserContext.java +++ b/core-api/src/main/java/com/optimizely/ab/OptimizelyUserContext.java @@ -59,7 +59,7 @@ static class ForcedDecision { public OptimizelyUserContext(@Nonnull Optimizely optimizely, @Nonnull String userId, - @Nonnull Map attributes) { + @Nonnull Map attributes) { this.optimizely = optimizely; this.userId = userId; if (attributes != null) { From 25dfa07d293818f30f429ad6914b2379c01ad8a9 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Tue, 28 Sep 2021 12:32:03 -0400 Subject: [PATCH 03/39] Update constructor to use ? instead of object - reverting --- core-api/src/main/java/com/optimizely/ab/Optimizely.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 cf643becc..43b5a8568 100644 --- a/core-api/src/main/java/com/optimizely/ab/Optimizely.java +++ b/core-api/src/main/java/com/optimizely/ab/Optimizely.java @@ -1361,7 +1361,7 @@ public Variation getFlagVariationByKey(String flagKey, String variationKey) { * @return copy of attributes */ private Map copyAttributes(Map attributes) { - Map copiedAttributes = null; + Map copiedAttributes = null; if (attributes != null) { copiedAttributes = new HashMap<>(attributes); } From 00fbc2103abdb55944ac1bba6f0fb36c400c9cc2 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Tue, 28 Sep 2021 13:26:19 -0400 Subject: [PATCH 04/39] Update dodgy tests. --- core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java | 2 -- .../java/com/optimizely/ab/bucketing/DecisionServiceTest.java | 4 ++-- 2 files changed, 2 insertions(+), 4 deletions(-) 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 3d1155b67..4cfc4159b 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java @@ -2892,8 +2892,6 @@ public void getFeatureVariableValueReturnsDefaultValueWhenFeatureEnabledIsFalse( Optimizely optimizely = optimizelyBuilder.withDecisionService(mockDecisionService).build(); - Attribute attribute = new Attribute("Dummy","Dummy","Dummy"); - FeatureDecision featureDecision = new FeatureDecision(multivariateExperiment, VARIATION_MULTIVARIATE_EXPERIMENT_GRED, FeatureDecision.DecisionSource.FEATURE_TEST); doReturn(DecisionResponse.responseNoReasons(featureDecision)).when(mockDecisionService).getVariationForFeature( FEATURE_FLAG_MULTI_VARIATE_FEATURE, diff --git a/core-api/src/test/java/com/optimizely/ab/bucketing/DecisionServiceTest.java b/core-api/src/test/java/com/optimizely/ab/bucketing/DecisionServiceTest.java index 55c5cfb67..4962d319f 100644 --- a/core-api/src/test/java/com/optimizely/ab/bucketing/DecisionServiceTest.java +++ b/core-api/src/test/java/com/optimizely/ab/bucketing/DecisionServiceTest.java @@ -98,13 +98,13 @@ public void getVariationWhitelistedPrecedesAudienceEval() throws Exception { experiment, optimizely.createUserContext( genericUserId, - null), + Collections.emptyMap()), validProjectConfig).getResult()); logbackVerifier.expectMessage(Level.INFO, "User \"" + whitelistedUserId + "\" is forced in variation \"vtag1\"."); // no attributes provided for a experiment that has an audience - assertThat(decisionService.getVariation(experiment, optimizely.createUserContext(whitelistedUserId, null), validProjectConfig).getResult(), is(expectedVariation)); + assertThat(decisionService.getVariation(experiment, optimizely.createUserContext(whitelistedUserId, Collections.emptyMap()), validProjectConfig).getResult(), is(expectedVariation)); verify(decisionService).getWhitelistedVariation(eq(experiment), eq(whitelistedUserId)); verify(decisionService, never()).getStoredVariation(eq(experiment), any(UserProfile.class), any(ProjectConfig.class)); From 9079018a1a8d7fc1b3453f403011d3310b6af849 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Tue, 28 Sep 2021 14:03:56 -0400 Subject: [PATCH 05/39] Add additional test to OptimizelyTest.java --- .../src/main/java/com/optimizely/ab/Optimizely.java | 10 ++++++---- .../test/java/com/optimizely/ab/OptimizelyTest.java | 11 +++++++++++ 2 files changed, 17 insertions(+), 4 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 43b5a8568..9d12145ac 100644 --- a/core-api/src/main/java/com/optimizely/ab/Optimizely.java +++ b/core-api/src/main/java/com/optimizely/ab/Optimizely.java @@ -1345,10 +1345,12 @@ private DecisionResponse> getDecisionVariableMap(@Nonnull Fe */ public Variation getFlagVariationByKey(String flagKey, String variationKey) { Map> flagVariationsMap = getProjectConfig().getFlagVariationsMap(); - List variations = flagVariationsMap.get(flagKey); - for (Variation variation : variations) { - if (variation.getKey().equals(variationKey)) { - return variation; + if (flagVariationsMap.containsKey(flagKey)) { + List variations = flagVariationsMap.get(flagKey); + for (Variation variation : variations) { + if (variation.getKey().equals(variationKey)) { + return variation; + } } } return null; 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 4cfc4159b..731ba8c92 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java @@ -4655,4 +4655,15 @@ public void createUserContext_multiple() { assertTrue(user2.getAttributes().isEmpty()); } + @Test + public void getFlagVariationByKey() throws IOException { + String flagKey = "double_single_variable_feature"; + String variationKey = "pi_variation"; + Optimizely optimizely = Optimizely.builder().withDatafile(validConfigJsonV4()).build(); + Variation variation = optimizely.getFlagVariationByKey(flagKey, variationKey); + + assertNotNull(variation); + assertEquals(variationKey, variation.getKey()); + } + } From f75c04a33c11312a7e7da3044bfae7fa9968c821 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Tue, 28 Sep 2021 14:20:24 -0400 Subject: [PATCH 06/39] Add tests to OptimizelyUserContextTest.java --- .../ab/OptimizelyUserContextTest.java | 65 ++++++++++++++++++- 1 file changed, 62 insertions(+), 3 deletions(-) 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 7c0a28a73..3f4b231ac 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java @@ -1223,6 +1223,19 @@ public void setForcedDecisionWithoutRuleKeyTest() { assertEquals(forcedDecision.getVariationKey(), variationKey); } + @Test + public void setForcedDecisionWithoutRuleKeyTestSdkNotReady() { + String flagKey = "55555"; + String variationKey = "33333"; + Optimizely optimizely = new Optimizely.Builder().build(); + OptimizelyUserContext optimizelyUserContext = new OptimizelyUserContext( + optimizely, + userId, + Collections.emptyMap()); + + assertFalse(optimizelyUserContext.setForcedDecision(flagKey, variationKey)); + } + @Test public void getForcedVariationWithRuleKey() { String flagKey = "55555"; @@ -1265,6 +1278,20 @@ public void getForcedVariationWithoutRuleKey() { assertEquals(variationKey, optimizelyUserContext.getForcedDecision(flagKey)); } + @Test + public void getForcedVariationWithoutRuleKeySdkNotReady() { + String flagKey = "55555"; + String variationKey = "33333"; + Optimizely optimizely = new Optimizely.Builder().build(); + OptimizelyUserContext optimizelyUserContext = new OptimizelyUserContext( + optimizely, + userId, + Collections.emptyMap()); + + optimizelyUserContext.setForcedDecision(flagKey, variationKey); + assertNull(optimizelyUserContext.getForcedDecision(flagKey)); + } + @Test public void failedGetForcedDecisionWithoutRuleKey() { String flagKey = "55555"; @@ -1290,7 +1317,7 @@ public void removeForcedDecisionWithRuleKey() { Collections.emptyMap()); optimizelyUserContext.setForcedDecision(flagKey, ruleKey, variationKey); - assertEquals(true, optimizelyUserContext.removeForcedDecision(flagKey, ruleKey)); + assertTrue(optimizelyUserContext.removeForcedDecision(flagKey, ruleKey)); } @Test @@ -1303,7 +1330,21 @@ public void removeForcedDecisionWithoutRuleKey() { Collections.emptyMap()); optimizelyUserContext.setForcedDecision(flagKey, variationKey); - assertEquals(true, optimizelyUserContext.removeForcedDecision(flagKey)); + assertTrue(optimizelyUserContext.removeForcedDecision(flagKey)); + } + + @Test + public void removeForcedDecisionWithoutRuleKeySdkNotReady() { + String flagKey = "55555"; + String variationKey = "33333"; + Optimizely optimizely = new Optimizely.Builder().build(); + OptimizelyUserContext optimizelyUserContext = new OptimizelyUserContext( + optimizely, + userId, + Collections.emptyMap()); + + optimizelyUserContext.setForcedDecision(flagKey, variationKey); + assertFalse(optimizelyUserContext.removeForcedDecision(flagKey)); } @Test @@ -1320,7 +1361,25 @@ public void removeAllForcedDecisions() { optimizelyUserContext.setForcedDecision(flagKey1, ruleKey1, variationKey1); optimizelyUserContext.setForcedDecision(flagKey2, variationKey2); - assertEquals(true, optimizelyUserContext.removeAllForcedDecisions()); + assertTrue(optimizelyUserContext.removeAllForcedDecisions()); + } + + @Test + public void removeAllForcedDecisionsSdkNotReady() { + String flagKey1 = "55555"; + String ruleKey1 = "77777"; + String variationKey1 = "33333"; + String flagKey2 = "11"; + String variationKey2 = "5"; + Optimizely optimizely = new Optimizely.Builder().build(); + OptimizelyUserContext optimizelyUserContext = new OptimizelyUserContext( + optimizely, + userId, + Collections.emptyMap()); + + optimizelyUserContext.setForcedDecision(flagKey1, ruleKey1, variationKey1); + optimizelyUserContext.setForcedDecision(flagKey2, variationKey2); + assertFalse(optimizelyUserContext.removeAllForcedDecisions()); } @Test From 225cd1e0b25d3a13fea24cc25cf2eb5fb7fc8a69 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Tue, 28 Sep 2021 14:43:39 -0400 Subject: [PATCH 07/39] Update tests for more coverage. --- .../ab/OptimizelyUserContextTest.java | 38 +++++++++++++++---- 1 file changed, 31 insertions(+), 7 deletions(-) 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 3f4b231ac..9d8bf9355 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java @@ -1202,27 +1202,51 @@ public void setForcedDecisionWithRuleKeyTest() { Collections.emptyMap()); optimizelyUserContext.setForcedDecision(flagKey, ruleKey, variationKey); - OptimizelyUserContext.ForcedDecision forcedDecision = optimizelyUserContext.forcedDecisionsMap.get(flagKey).get(ruleKey); - assertEquals(forcedDecision.getFlagKey(), flagKey); - assertEquals(forcedDecision.getRuleKey(), ruleKey); - assertEquals(forcedDecision.getVariationKey(), variationKey); + String foundVariationKey = optimizelyUserContext.getForcedDecision(flagKey, ruleKey); + assertEquals(variationKey, foundVariationKey); + } + + @Test + public void setForcedDecisionsWithRuleKeyTest() { + String flagKey = "55555"; + String ruleKey = "77777"; + String ruleKey2 = "88888"; + String variationKey = "33333"; + String variationKey2 = "44444"; + OptimizelyUserContext optimizelyUserContext = new OptimizelyUserContext( + optimizely, + userId, + Collections.emptyMap()); + + optimizelyUserContext.setForcedDecision(flagKey, ruleKey, variationKey); + optimizelyUserContext.setForcedDecision(flagKey, ruleKey2, variationKey2); + assertEquals(variationKey, optimizelyUserContext.getForcedDecision(flagKey, ruleKey)); + assertEquals(variationKey2, optimizelyUserContext.getForcedDecision(flagKey, ruleKey2)); + + // Update first forcedDecision + optimizelyUserContext.setForcedDecision(flagKey, ruleKey, variationKey2); + assertEquals(variationKey2, optimizelyUserContext.getForcedDecision(flagKey, ruleKey)); } @Test public void setForcedDecisionWithoutRuleKeyTest() { String flagKey = "55555"; String variationKey = "33333"; + String updatedVariationKey = "55555"; OptimizelyUserContext optimizelyUserContext = new OptimizelyUserContext( optimizely, userId, Collections.emptyMap()); optimizelyUserContext.setForcedDecision(flagKey, variationKey); - OptimizelyUserContext.ForcedDecision forcedDecision = optimizelyUserContext.forcedDecisionsMapWithNoRuleKey.get(flagKey); - assertEquals(forcedDecision.getFlagKey(), flagKey); - assertEquals(forcedDecision.getVariationKey(), variationKey); + assertEquals(variationKey, optimizelyUserContext.getForcedDecision(flagKey)); + + // Update forcedDecision + optimizelyUserContext.setForcedDecision(flagKey, updatedVariationKey); + assertEquals(updatedVariationKey, optimizelyUserContext.getForcedDecision(flagKey)); } + @Test public void setForcedDecisionWithoutRuleKeyTestSdkNotReady() { String flagKey = "55555"; From 2a47e23c97c56d94f1fe0ae2e32b0c56986f0a6b Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Tue, 28 Sep 2021 15:08:05 -0400 Subject: [PATCH 08/39] Revert javadoc comment test. --- core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java | 2 +- 1 file changed, 1 insertion(+), 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 731ba8c92..0a9c334f8 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java @@ -2708,7 +2708,7 @@ public void trackEventWithListenerNullAttributes() throws Exception { //======== Feature Accessor Tests ========// /** - * Verify {@link Optimizely#getFeatureVariableValueForType(String, String, String, Map, String)} + * Verify {@link Optimizely#getFeatureVariableValueForType(String, String, String, Map, FeatureVariable.VariableType)} * returns null and logs a message * when it is called with a feature key that has no corresponding feature in the datafile. */ From 881f8020e89675d9b2428a59ca51ad5a60f19694 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Tue, 28 Sep 2021 15:26:05 -0400 Subject: [PATCH 09/39] Update javadocs with titles. --- .../src/main/java/com/optimizely/ab/OptimizelyUserContext.java | 3 +++ 1 file changed, 3 insertions(+) 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 ed4d2938f..a039f503f 100644 --- a/core-api/src/main/java/com/optimizely/ab/OptimizelyUserContext.java +++ b/core-api/src/main/java/com/optimizely/ab/OptimizelyUserContext.java @@ -190,6 +190,7 @@ public void trackEvent(@Nonnull String eventName) throws UnknownEventTypeExcepti } /** + * Set a forced decision * * @param flagKey The flag key for the forced decision * @param variationKey The variation key for the forced decision @@ -239,6 +240,7 @@ public Boolean setForcedDecision(@Nonnull String flagKey, String ruleKey, @Nonnu } /** + * Get a forced decision * * @param flagKey The flag key for the forced decision * @return Returns a variationKey for a given forced decision @@ -286,6 +288,7 @@ public String findForcedDecision(@Nonnull String flagKey, String ruleKey) { } /** + * Remove a forced decision * * @param flagKey The flag key in the forced decision * @return Returns a boolean of true if successful, otherwise false From 5cec109b557b79045f072aac82fb3bc43b778cba Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Tue, 28 Sep 2021 15:55:51 -0400 Subject: [PATCH 10/39] Update deprecations. --- .../java/com/optimizely/ab/Optimizely.java | 53 ++++++++++++++++++- 1 file changed, 51 insertions(+), 2 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 9d12145ac..fcb49655c 100644 --- a/core-api/src/main/java/com/optimizely/ab/Optimizely.java +++ b/core-api/src/main/java/com/optimizely/ab/Optimizely.java @@ -372,7 +372,10 @@ public void track(@Nonnull String eventName, * @return True if the feature is enabled. * False if the feature is disabled. * False if the feature is not found. + * + * @deprecated use __decide__ API instead. Refer to [the migration guide] (https://docs.developers.optimizely.com/full-stack/v4.0/docs/migrate-from-older-versions-java) */ + @Deprecated @Nonnull public Boolean isFeatureEnabled(@Nonnull String featureKey, @Nonnull String userId) { @@ -389,7 +392,10 @@ public Boolean isFeatureEnabled(@Nonnull String featureKey, * @return True if the feature is enabled. * False if the feature is disabled. * False if the feature is not found. + * + * @deprecated use __decide__ API instead. Refer to [the migration guide] (https://docs.developers.optimizely.com/full-stack/v4.0/docs/migrate-from-older-versions-java) */ + @Deprecated @Nonnull public Boolean isFeatureEnabled(@Nonnull String featureKey, @Nonnull String userId, @@ -403,6 +409,11 @@ public Boolean isFeatureEnabled(@Nonnull String featureKey, return isFeatureEnabled(projectConfig, featureKey, userId, attributes); } + /** + * + * @deprecated use __decide__ API instead. Refer to [the migration guide] (https://docs.developers.optimizely.com/full-stack/v4.0/docs/migrate-from-older-versions-java) + */ + @Deprecated @Nonnull private Boolean isFeatureEnabled(@Nonnull ProjectConfig projectConfig, @Nonnull String featureKey, @@ -494,7 +505,12 @@ public Boolean getFeatureVariableBoolean(@Nonnull String featureKey, * @param attributes The user's attributes. * @return The Boolean value of the boolean single variable feature. * Null if the feature or variable could not be found. + * + * + * + * @deprecated use __decide__ API instead. Refer to [the migration guide] (https://docs.developers.optimizely.com/full-stack/v4.0/docs/migrate-from-older-versions-java) */ + @Deprecated @Nullable public Boolean getFeatureVariableBoolean(@Nonnull String featureKey, @Nonnull String variableKey, @@ -518,7 +534,10 @@ public Boolean getFeatureVariableBoolean(@Nonnull String featureKey, * @param userId The ID of the user. * @return The Double value of the double single variable feature. * Null if the feature or variable could not be found. + * + * @deprecated use __decide__ API instead. Refer to [the migration guide] (https://docs.developers.optimizely.com/full-stack/v4.0/docs/migrate-from-older-versions-java) */ + @Deprecated @Nullable public Double getFeatureVariableDouble(@Nonnull String featureKey, @Nonnull String variableKey, @@ -535,7 +554,10 @@ public Double getFeatureVariableDouble(@Nonnull String featureKey, * @param attributes The user's attributes. * @return The Double value of the double single variable feature. * Null if the feature or variable could not be found. + * + * @deprecated use __decide__ API instead. Refer to [the migration guide] (https://docs.developers.optimizely.com/full-stack/v4.0/docs/migrate-from-older-versions-java) */ + @Deprecated @Nullable public Double getFeatureVariableDouble(@Nonnull String featureKey, @Nonnull String variableKey, @@ -567,7 +589,10 @@ public Double getFeatureVariableDouble(@Nonnull String featureKey, * @param userId The ID of the user. * @return The Integer value of the integer single variable feature. * Null if the feature or variable could not be found. + * + * @deprecated use __decide__ API instead. Refer to [the migration guide] (https://docs.developers.optimizely.com/full-stack/v4.0/docs/migrate-from-older-versions-java) */ + @Deprecated @Nullable public Integer getFeatureVariableInteger(@Nonnull String featureKey, @Nonnull String variableKey, @@ -584,7 +609,10 @@ public Integer getFeatureVariableInteger(@Nonnull String featureKey, * @param attributes The user's attributes. * @return The Integer value of the integer single variable feature. * Null if the feature or variable could not be found. + * + * @deprecated use __decide__ API instead. Refer to [the migration guide] (https://docs.developers.optimizely.com/full-stack/v4.0/docs/migrate-from-older-versions-java) */ + @Deprecated @Nullable public Integer getFeatureVariableInteger(@Nonnull String featureKey, @Nonnull String variableKey, @@ -617,7 +645,10 @@ public Integer getFeatureVariableInteger(@Nonnull String featureKey, * @param userId The ID of the user. * @return The String value of the string single variable feature. * Null if the feature or variable could not be found. + * + * @deprecated use __decide__ API instead. Refer to [the migration guide] (https://docs.developers.optimizely.com/full-stack/v4.0/docs/migrate-from-older-versions-java) */ + @Deprecated @Nullable public String getFeatureVariableString(@Nonnull String featureKey, @Nonnull String variableKey, @@ -634,7 +665,10 @@ public String getFeatureVariableString(@Nonnull String featureKey, * @param attributes The user's attributes. * @return The String value of the string single variable feature. * Null if the feature or variable could not be found. + * + * @deprecated use __decide__ API instead. Refer to [the migration guide] (https://docs.developers.optimizely.com/full-stack/v4.0/docs/migrate-from-older-versions-java) */ + @Deprecated @Nullable public String getFeatureVariableString(@Nonnull String featureKey, @Nonnull String variableKey, @@ -657,7 +691,10 @@ public String getFeatureVariableString(@Nonnull String featureKey, * @param userId The ID of the user. * @return An OptimizelyJSON instance for the JSON variable value. * Null if the feature or variable could not be found. + * + * @deprecated use __decide__ API instead. Refer to [the migration guide] (https://docs.developers.optimizely.com/full-stack/v4.0/docs/migrate-from-older-versions-java) */ + @Deprecated @Nullable public OptimizelyJSON getFeatureVariableJSON(@Nonnull String featureKey, @Nonnull String variableKey, @@ -674,7 +711,10 @@ public OptimizelyJSON getFeatureVariableJSON(@Nonnull String featureKey, * @param attributes The user's attributes. * @return An OptimizelyJSON instance for the JSON variable value. * Null if the feature or variable could not be found. + * + * @deprecated use __decide__ API instead. Refer to [the migration guide] (https://docs.developers.optimizely.com/full-stack/v4.0/docs/migrate-from-older-versions-java) */ + @Deprecated @Nullable public OptimizelyJSON getFeatureVariableJSON(@Nonnull String featureKey, @Nonnull String variableKey, @@ -824,7 +864,10 @@ Object convertStringToType(String variableValue, String type) { * @param userId The ID of the user. * @return An OptimizelyJSON instance for all variable values. * Null if the feature could not be found. + * + * @deprecated use __decide__ API instead. Refer to [the migration guide] (https://docs.developers.optimizely.com/full-stack/v4.0/docs/migrate-from-older-versions-java) */ + @Deprecated @Nullable public OptimizelyJSON getAllFeatureVariables(@Nonnull String featureKey, @Nonnull String userId) { @@ -839,7 +882,10 @@ public OptimizelyJSON getAllFeatureVariables(@Nonnull String featureKey, * @param attributes The user's attributes. * @return An OptimizelyJSON instance for all variable values. * Null if the feature could not be found. + * + * @deprecated use __decide__ API instead. Refer to [the migration guide] (https://docs.developers.optimizely.com/full-stack/v4.0/docs/migrate-from-older-versions-java) */ + @Deprecated @Nullable public OptimizelyJSON getAllFeatureVariables(@Nonnull String featureKey, @Nonnull String userId, @@ -922,7 +968,10 @@ public OptimizelyJSON getAllFeatureVariables(@Nonnull String featureKey, * @param attributes The user's attributes. * @return List of the feature keys that are enabled for the user if the userId is empty it will * return Empty List. + * + * @deprecated use __decide__ API instead. Refer to [the migration guide] (https://docs.developers.optimizely.com/full-stack/v4.0/docs/migrate-from-older-versions-java) */ + @Deprecated public List getEnabledFeatures(@Nonnull String userId, @Nonnull Map attributes) { List enabledFeaturesList = new ArrayList(); if (!validateUserId(userId)) { @@ -1034,7 +1083,7 @@ public Variation getVariation(@Nonnull String experimentKey, * then the forcedVariation for that experiment is removed. * @return boolean A boolean value that indicates if the set completed successfully. * - * @deprecated use {@link OptimizelyUserContext#setForcedDecision(String, String, String)} instead + * @deprecated use __decide__ API instead. Refer to [the migration guide] (https://docs.developers.optimizely.com/full-stack/v4.0/docs/migrate-from-older-versions-java) */ @Deprecated public boolean setForcedVariation(@Nonnull String experimentKey, @@ -1068,7 +1117,7 @@ public boolean setForcedVariation(@Nonnull String experimentKey, * @return The variation the user was bucketed into. This value can be null if the * forced variation fails. * - * @deprecated use {@link OptimizelyUserContext#getForcedDecision(String, String)} instead + * @deprecated use __decide__ API instead. Refer to [the migration guide] (https://docs.developers.optimizely.com/full-stack/v4.0/docs/migrate-from-older-versions-java) */ @Deprecated @Nullable From 496485285d20bcd85a1c1618158744f55ae0695e Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Tue, 28 Sep 2021 15:58:29 -0400 Subject: [PATCH 11/39] Remove deprecations for set and get forcedVariations. --- core-api/src/main/java/com/optimizely/ab/Optimizely.java | 6 ------ .../java/com/optimizely/ab/bucketing/DecisionService.java | 6 ------ 2 files changed, 12 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 fcb49655c..869f87f9e 100644 --- a/core-api/src/main/java/com/optimizely/ab/Optimizely.java +++ b/core-api/src/main/java/com/optimizely/ab/Optimizely.java @@ -1082,10 +1082,7 @@ public Variation getVariation(@Nonnull String experimentKey, * @param variationKey The variation key to force the user into. If the variation key is null * then the forcedVariation for that experiment is removed. * @return boolean A boolean value that indicates if the set completed successfully. - * - * @deprecated use __decide__ API instead. Refer to [the migration guide] (https://docs.developers.optimizely.com/full-stack/v4.0/docs/migrate-from-older-versions-java) */ - @Deprecated public boolean setForcedVariation(@Nonnull String experimentKey, @Nonnull String userId, @Nullable String variationKey) { @@ -1116,10 +1113,7 @@ public boolean setForcedVariation(@Nonnull String experimentKey, * @param userId The user ID to be used for bucketing. * @return The variation the user was bucketed into. This value can be null if the * forced variation fails. - * - * @deprecated use __decide__ API instead. Refer to [the migration guide] (https://docs.developers.optimizely.com/full-stack/v4.0/docs/migrate-from-older-versions-java) */ - @Deprecated @Nullable public Variation getForcedVariation(@Nonnull String experimentKey, @Nonnull String userId) { diff --git a/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java b/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java index 42f31eb7f..88cb28797 100644 --- a/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java +++ b/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java @@ -477,10 +477,7 @@ public ConcurrentHashMap> getForcedVar * @param variationKey The variation key to force the user into. If the variation key is null * then the forcedVariation for that experiment is removed. * @return boolean A boolean value that indicates if the set completed successfully. - * - * @deprecated use {@link OptimizelyUserContext#setForcedDecision(String, String, String)} instead */ - @Deprecated public boolean setForcedVariation(@Nonnull Experiment experiment, @Nonnull String userId, @Nullable String variationKey) { @@ -547,10 +544,7 @@ public boolean setForcedVariation(@Nonnull Experiment experiment, * @param userId The user ID to be used for bucketing. * @return A {@link DecisionResponse} including the {@link Variation} that user is bucketed into (or null) * and the decision reasons. The variation can be null if the forced variation fails. - * - * @deprecated use {@link OptimizelyUserContext#getForcedDecision(String, String)} instead */ - @Deprecated @Nonnull public DecisionResponse getForcedVariation(@Nonnull Experiment experiment, @Nonnull String userId) { From 31de5d5fb0078a3961da7251f35662d4560d214c Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Fri, 1 Oct 2021 10:12:30 -0400 Subject: [PATCH 12/39] Simplify the way flagVariationsMap is built --- .../ab/config/DatafileProjectConfig.java | 39 ++++++++++--------- 1 file changed, 20 insertions(+), 19 deletions(-) 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 643439ed7..8ef8cca79 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 @@ -216,18 +216,12 @@ public DatafileProjectConfig(String accountId, flagVariationsMap = new HashMap<>(); if (featureFlags != null) { for (FeatureFlag flag : featureFlags) { - Map variationsIdMap = new HashMap<>(); List variations = new ArrayList<>(); - for (String experimentId : flag.getExperimentIds()) { - // We need to get each experiment by id and add to our list of experiments - Experiment exp = experimentIdMapping.get(experimentId); - variations.addAll(getVariationsFromExperiments(variationsIdMap, exp)); - - } - Rollout rollout = rolloutIdMapping.get(flag.getRolloutId()); - if (rollout != null) { - for (Experiment experiment : rollout.getExperiments()) { - variations.addAll(getVariationsFromExperiments(variationsIdMap, experiment)); + for (Experiment rule : getAllRulesForFlag(flag)) { + for (Variation variation : rule.getVariations()) { + if (!variations.contains(variation)) { + variations.add(variation); + } } } // Grab all the variations from the flag experiments and rollouts and add to flagVariationsMap @@ -236,17 +230,24 @@ public DatafileProjectConfig(String accountId, } } - private List getVariationsFromExperiments(Map variationsIdMap, Experiment exp) { - List variations = new ArrayList<>(); - for (Variation variation : exp.getVariations()) { - if (!variationsIdMap.containsKey(variation.getId())) { - variationsIdMap.put(variation.getId(), variation); - variations.add(variation); - } + /** + * Helper method to grab all rules for a flag + * @param flag The flag to grab all the rules from + * @return Returns a list of Experiments as rules + */ + private List getAllRulesForFlag(FeatureFlag flag) { + List rules = new ArrayList<>(); + Rollout rollout = rolloutIdMapping.get(flag.getRolloutId()); + for (String experimentId : flag.getExperimentIds()) { + rules.add(experimentIdMapping.get(experimentId)); + } + if (rollout != null) { + rules.addAll(rollout.getExperiments()); } - return variations; + return rules; } + /** * Helper method to retrieve the {@link Experiment} for the given experiment key. * If {@link RaiseExceptionErrorHandler} is provided, either an experiment is returned, From 34cc7668894a994a6aafa031a57b4d0d8d45f3eb Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Fri, 1 Oct 2021 10:30:28 -0400 Subject: [PATCH 13/39] Fix removeForcedDecision when no object in map --- .../com/optimizely/ab/OptimizelyUserContext.java | 6 ++++-- .../optimizely/ab/OptimizelyUserContextTest.java | 14 ++++++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) 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 a039f503f..6c12056df 100644 --- a/core-api/src/main/java/com/optimizely/ab/OptimizelyUserContext.java +++ b/core-api/src/main/java/com/optimizely/ab/OptimizelyUserContext.java @@ -321,8 +321,10 @@ public boolean removeForcedDecision(@Nonnull String flagKey, String ruleKey) { } } else { try { - forcedDecisionsMapWithNoRuleKey.remove(flagKey); - return true; + ForcedDecision result = forcedDecisionsMapWithNoRuleKey.remove(flagKey); + if (result != null) { + return true; + } } catch (Exception e) { logger.error("Forced Decision does not exist to remove - " + e); } 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 9d8bf9355..899c50a59 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java @@ -1357,6 +1357,20 @@ public void removeForcedDecisionWithoutRuleKey() { assertTrue(optimizelyUserContext.removeForcedDecision(flagKey)); } + @Test + public void removeForcedDecisionWithNullRuleKeyAfterAddingWithRuleKey() { + String flagKey = "flag2"; + String ruleKey = "default-rollout-3045-20390585493"; + String variationKey = "variation2"; + OptimizelyUserContext optimizelyUserContext = new OptimizelyUserContext( + optimizely, + userId, + Collections.emptyMap()); + + optimizelyUserContext.setForcedDecision(flagKey, ruleKey, variationKey); + assertFalse(optimizelyUserContext.removeForcedDecision(flagKey)); + } + @Test public void removeForcedDecisionWithoutRuleKeySdkNotReady() { String flagKey = "55555"; From 65fe3ad513b7d7cd20c94d7256dd38d59559d6b7 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Fri, 8 Oct 2021 09:32:53 -0400 Subject: [PATCH 14/39] cleanup and remove deprecation warnings. --- .../java/com/optimizely/ab/Optimizely.java | 33 ------------------- .../optimizely/ab/OptimizelyUserContext.java | 11 ++++--- 2 files changed, 6 insertions(+), 38 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 869f87f9e..d0b2d7647 100644 --- a/core-api/src/main/java/com/optimizely/ab/Optimizely.java +++ b/core-api/src/main/java/com/optimizely/ab/Optimizely.java @@ -373,9 +373,7 @@ public void track(@Nonnull String eventName, * False if the feature is disabled. * False if the feature is not found. * - * @deprecated use __decide__ API instead. Refer to [the migration guide] (https://docs.developers.optimizely.com/full-stack/v4.0/docs/migrate-from-older-versions-java) */ - @Deprecated @Nonnull public Boolean isFeatureEnabled(@Nonnull String featureKey, @Nonnull String userId) { @@ -393,9 +391,7 @@ public Boolean isFeatureEnabled(@Nonnull String featureKey, * False if the feature is disabled. * False if the feature is not found. * - * @deprecated use __decide__ API instead. Refer to [the migration guide] (https://docs.developers.optimizely.com/full-stack/v4.0/docs/migrate-from-older-versions-java) */ - @Deprecated @Nonnull public Boolean isFeatureEnabled(@Nonnull String featureKey, @Nonnull String userId, @@ -409,11 +405,6 @@ public Boolean isFeatureEnabled(@Nonnull String featureKey, return isFeatureEnabled(projectConfig, featureKey, userId, attributes); } - /** - * - * @deprecated use __decide__ API instead. Refer to [the migration guide] (https://docs.developers.optimizely.com/full-stack/v4.0/docs/migrate-from-older-versions-java) - */ - @Deprecated @Nonnull private Boolean isFeatureEnabled(@Nonnull ProjectConfig projectConfig, @Nonnull String featureKey, @@ -508,9 +499,7 @@ public Boolean getFeatureVariableBoolean(@Nonnull String featureKey, * * * - * @deprecated use __decide__ API instead. Refer to [the migration guide] (https://docs.developers.optimizely.com/full-stack/v4.0/docs/migrate-from-older-versions-java) */ - @Deprecated @Nullable public Boolean getFeatureVariableBoolean(@Nonnull String featureKey, @Nonnull String variableKey, @@ -535,9 +524,7 @@ public Boolean getFeatureVariableBoolean(@Nonnull String featureKey, * @return The Double value of the double single variable feature. * Null if the feature or variable could not be found. * - * @deprecated use __decide__ API instead. Refer to [the migration guide] (https://docs.developers.optimizely.com/full-stack/v4.0/docs/migrate-from-older-versions-java) */ - @Deprecated @Nullable public Double getFeatureVariableDouble(@Nonnull String featureKey, @Nonnull String variableKey, @@ -555,9 +542,7 @@ public Double getFeatureVariableDouble(@Nonnull String featureKey, * @return The Double value of the double single variable feature. * Null if the feature or variable could not be found. * - * @deprecated use __decide__ API instead. Refer to [the migration guide] (https://docs.developers.optimizely.com/full-stack/v4.0/docs/migrate-from-older-versions-java) */ - @Deprecated @Nullable public Double getFeatureVariableDouble(@Nonnull String featureKey, @Nonnull String variableKey, @@ -590,9 +575,7 @@ public Double getFeatureVariableDouble(@Nonnull String featureKey, * @return The Integer value of the integer single variable feature. * Null if the feature or variable could not be found. * - * @deprecated use __decide__ API instead. Refer to [the migration guide] (https://docs.developers.optimizely.com/full-stack/v4.0/docs/migrate-from-older-versions-java) */ - @Deprecated @Nullable public Integer getFeatureVariableInteger(@Nonnull String featureKey, @Nonnull String variableKey, @@ -610,9 +593,7 @@ public Integer getFeatureVariableInteger(@Nonnull String featureKey, * @return The Integer value of the integer single variable feature. * Null if the feature or variable could not be found. * - * @deprecated use __decide__ API instead. Refer to [the migration guide] (https://docs.developers.optimizely.com/full-stack/v4.0/docs/migrate-from-older-versions-java) */ - @Deprecated @Nullable public Integer getFeatureVariableInteger(@Nonnull String featureKey, @Nonnull String variableKey, @@ -646,9 +627,7 @@ public Integer getFeatureVariableInteger(@Nonnull String featureKey, * @return The String value of the string single variable feature. * Null if the feature or variable could not be found. * - * @deprecated use __decide__ API instead. Refer to [the migration guide] (https://docs.developers.optimizely.com/full-stack/v4.0/docs/migrate-from-older-versions-java) */ - @Deprecated @Nullable public String getFeatureVariableString(@Nonnull String featureKey, @Nonnull String variableKey, @@ -666,9 +645,7 @@ public String getFeatureVariableString(@Nonnull String featureKey, * @return The String value of the string single variable feature. * Null if the feature or variable could not be found. * - * @deprecated use __decide__ API instead. Refer to [the migration guide] (https://docs.developers.optimizely.com/full-stack/v4.0/docs/migrate-from-older-versions-java) */ - @Deprecated @Nullable public String getFeatureVariableString(@Nonnull String featureKey, @Nonnull String variableKey, @@ -692,9 +669,7 @@ public String getFeatureVariableString(@Nonnull String featureKey, * @return An OptimizelyJSON instance for the JSON variable value. * Null if the feature or variable could not be found. * - * @deprecated use __decide__ API instead. Refer to [the migration guide] (https://docs.developers.optimizely.com/full-stack/v4.0/docs/migrate-from-older-versions-java) */ - @Deprecated @Nullable public OptimizelyJSON getFeatureVariableJSON(@Nonnull String featureKey, @Nonnull String variableKey, @@ -712,9 +687,7 @@ public OptimizelyJSON getFeatureVariableJSON(@Nonnull String featureKey, * @return An OptimizelyJSON instance for the JSON variable value. * Null if the feature or variable could not be found. * - * @deprecated use __decide__ API instead. Refer to [the migration guide] (https://docs.developers.optimizely.com/full-stack/v4.0/docs/migrate-from-older-versions-java) */ - @Deprecated @Nullable public OptimizelyJSON getFeatureVariableJSON(@Nonnull String featureKey, @Nonnull String variableKey, @@ -865,9 +838,7 @@ Object convertStringToType(String variableValue, String type) { * @return An OptimizelyJSON instance for all variable values. * Null if the feature could not be found. * - * @deprecated use __decide__ API instead. Refer to [the migration guide] (https://docs.developers.optimizely.com/full-stack/v4.0/docs/migrate-from-older-versions-java) */ - @Deprecated @Nullable public OptimizelyJSON getAllFeatureVariables(@Nonnull String featureKey, @Nonnull String userId) { @@ -883,9 +854,7 @@ public OptimizelyJSON getAllFeatureVariables(@Nonnull String featureKey, * @return An OptimizelyJSON instance for all variable values. * Null if the feature could not be found. * - * @deprecated use __decide__ API instead. Refer to [the migration guide] (https://docs.developers.optimizely.com/full-stack/v4.0/docs/migrate-from-older-versions-java) */ - @Deprecated @Nullable public OptimizelyJSON getAllFeatureVariables(@Nonnull String featureKey, @Nonnull String userId, @@ -969,9 +938,7 @@ public OptimizelyJSON getAllFeatureVariables(@Nonnull String featureKey, * @return List of the feature keys that are enabled for the user if the userId is empty it will * return Empty List. * - * @deprecated use __decide__ API instead. Refer to [the migration guide] (https://docs.developers.optimizely.com/full-stack/v4.0/docs/migrate-from-older-versions-java) */ - @Deprecated public List getEnabledFeatures(@Nonnull String userId, @Nonnull Map attributes) { List enabledFeaturesList = new ArrayList(); if (!validateUserId(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 6c12056df..422957909 100644 --- a/core-api/src/main/java/com/optimizely/ab/OptimizelyUserContext.java +++ b/core-api/src/main/java/com/optimizely/ab/OptimizelyUserContext.java @@ -311,11 +311,13 @@ public boolean removeForcedDecision(@Nonnull String flagKey, String ruleKey) { } if (ruleKey != null) { try { - forcedDecisionsMap.get(flagKey).remove(ruleKey); - if (forcedDecisionsMap.get(flagKey).size() == 0) { - forcedDecisionsMap.remove(flagKey); + ForcedDecision result = forcedDecisionsMap.get(flagKey).remove(ruleKey); + if (result != null) { + if (forcedDecisionsMap.get(flagKey).size() == 0) { + forcedDecisionsMap.remove(flagKey); + } + return true; } - return true; } catch (Exception e) { logger.error("Forced Decision does not exist to remove - " + e); } @@ -368,7 +370,6 @@ public DecisionResponse findValidatedForcedDecision(@Nonnull String f */ public DecisionResponse findValidatedForcedDecision(@Nonnull String flagKey, String ruleKey) { DecisionReasons reasons = DefaultDecisionReasons.newInstance(); - // TODO - Move all info strings to a single class to be called rather than hardcoded in functions String variationKey = findForcedDecision(flagKey, ruleKey); if (variationKey != null) { Variation variation = optimizely.getFlagVariationByKey(flagKey, variationKey); From 8f03cf3bf8fd0af57bbbd8700775ed5ea5d3775a Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Fri, 8 Oct 2021 10:32:16 -0400 Subject: [PATCH 15/39] Change forcedDecisions to use a map of Strings to Strings based on a new class OptimizelyForcedDecisionsKey which uses the toString to build a key based on a flagKey and ruleKey, where rulekey, if null wil be the string equivelant and then all converted to its hashcode string equivelant. --- .../optimizely/ab/OptimizelyUserContext.java | 87 +++---------------- .../OptimizelyForcedDecisionKey.java | 24 +++++ 2 files changed, 38 insertions(+), 73 deletions(-) create mode 100644 core-api/src/main/java/com/optimizely/ab/bucketing/OptimizelyForcedDecisionKey.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 422957909..6e87c5508 100644 --- a/core-api/src/main/java/com/optimizely/ab/OptimizelyUserContext.java +++ b/core-api/src/main/java/com/optimizely/ab/OptimizelyUserContext.java @@ -16,6 +16,7 @@ */ package com.optimizely.ab; +import com.optimizely.ab.bucketing.OptimizelyForcedDecisionKey; import com.optimizely.ab.config.Variation; import com.optimizely.ab.optimizelydecision.*; import org.slf4j.Logger; @@ -26,25 +27,9 @@ import java.util.*; public class OptimizelyUserContext { - static class ForcedDecision { - private String flagKey; - private String ruleKey; - private String variationKey; - - ForcedDecision(@Nonnull String flagKey, String ruleKey, @Nonnull String variationKey) { - this.flagKey = flagKey; - this.ruleKey = ruleKey; - this.variationKey = variationKey; - } - - public String getFlagKey() { return flagKey; } - public String getRuleKey() { return ruleKey; } - public String getVariationKey() { return variationKey; } - } - // flagKeys mapped to ruleKeys mapped to forcedDecisions - Map> forcedDecisionsMap = new HashMap<>(); - Map forcedDecisionsMapWithNoRuleKey = new HashMap<>(); + // OptimizelyForcedDecisionsKey mapped to variationKeys + Map forcedDecisionsMap = new HashMap<>(); @Nonnull private final String userId; @@ -214,27 +199,8 @@ public Boolean setForcedDecision(@Nonnull String flagKey, String ruleKey, @Nonnu return false; } - if (ruleKey == null) { - // If the ruleKey is null, we will populate/update the appropriate map - if (forcedDecisionsMapWithNoRuleKey.get(flagKey) != null) { - forcedDecisionsMapWithNoRuleKey.get(flagKey).variationKey = variationKey; - } else { - forcedDecisionsMapWithNoRuleKey.put(flagKey, new ForcedDecision(flagKey, null, variationKey)); - } - } else { - // If the flagKey and ruleKey are already present, set the updated variationKey - if (forcedDecisionsMap.containsKey(flagKey)) { - if (forcedDecisionsMap.get(flagKey).containsKey(ruleKey)) { - forcedDecisionsMap.get(flagKey).get(ruleKey).variationKey = variationKey; - } else { - forcedDecisionsMap.get(flagKey).put(ruleKey, new ForcedDecision(flagKey, ruleKey, variationKey)); - } - } else { - Map forcedDecision = new HashMap<>(); - forcedDecision.put(ruleKey, new ForcedDecision(flagKey, ruleKey, variationKey)); - forcedDecisionsMap.put(flagKey, forcedDecision); - } - } + OptimizelyForcedDecisionKey key = new OptimizelyForcedDecisionKey(flagKey, ruleKey); + forcedDecisionsMap.put(key.toString(), variationKey); return true; } @@ -272,18 +238,9 @@ public String getForcedDecision(@Nonnull String flagKey, String ruleKey) { * @return Returns a variationKey relating to the found forced decision, otherwise null */ public String findForcedDecision(@Nonnull String flagKey, String ruleKey) { - String variationKey = null; - if (ruleKey != null) { - if (forcedDecisionsMap.size() > 0 && forcedDecisionsMap.containsKey(flagKey)) { - if (forcedDecisionsMap.get(flagKey).containsKey(ruleKey)) { - variationKey = forcedDecisionsMap.get(flagKey).get(ruleKey).getVariationKey(); - } - } - } else { - if (forcedDecisionsMapWithNoRuleKey.size() > 0 && forcedDecisionsMapWithNoRuleKey.containsKey(flagKey)) { - variationKey = forcedDecisionsMapWithNoRuleKey.get(flagKey).getVariationKey(); - } - } + String variationKey; + OptimizelyForcedDecisionKey lookupKey = new OptimizelyForcedDecisionKey(flagKey, ruleKey); + variationKey = forcedDecisionsMap.get(lookupKey.toString()); return variationKey; } @@ -309,27 +266,12 @@ public boolean removeForcedDecision(@Nonnull String flagKey, String ruleKey) { logger.error("Optimizely SDK not ready."); return false; } - if (ruleKey != null) { - try { - ForcedDecision result = forcedDecisionsMap.get(flagKey).remove(ruleKey); - if (result != null) { - if (forcedDecisionsMap.get(flagKey).size() == 0) { - forcedDecisionsMap.remove(flagKey); - } - return true; - } - } catch (Exception e) { - logger.error("Forced Decision does not exist to remove - " + e); - } - } else { - try { - ForcedDecision result = forcedDecisionsMapWithNoRuleKey.remove(flagKey); - if (result != null) { - return true; - } - } catch (Exception e) { - logger.error("Forced Decision does not exist to remove - " + e); - } + + OptimizelyForcedDecisionKey removeKey = new OptimizelyForcedDecisionKey(flagKey, ruleKey); + try { + return forcedDecisionsMap.remove(removeKey.toString()) != null ? true : false; + } catch (Exception e) { + logger.error("Forced Decision does not exist to remove - " + e); } return false; @@ -347,7 +289,6 @@ public boolean removeAllForcedDecisions() { } // Clear both maps for with and without ruleKey forcedDecisionsMap.clear(); - forcedDecisionsMapWithNoRuleKey.clear(); return true; } diff --git a/core-api/src/main/java/com/optimizely/ab/bucketing/OptimizelyForcedDecisionKey.java b/core-api/src/main/java/com/optimizely/ab/bucketing/OptimizelyForcedDecisionKey.java new file mode 100644 index 000000000..9cbb67f0c --- /dev/null +++ b/core-api/src/main/java/com/optimizely/ab/bucketing/OptimizelyForcedDecisionKey.java @@ -0,0 +1,24 @@ +package com.optimizely.ab.bucketing; + +import javax.annotation.Nonnull; + +public class OptimizelyForcedDecisionKey { + private String flagKey; + private String ruleKey; + + public OptimizelyForcedDecisionKey(@Nonnull String flagKey, String ruleKey) { + this.flagKey = flagKey; + this.ruleKey = ruleKey; + } + + public String getFlagKey() { return flagKey; } + + public String getRuleKey() { return ruleKey; } + + public String toString() { + StringBuilder keyString = new StringBuilder(); + keyString.append(getFlagKey().hashCode()); + keyString.append(getRuleKey() != null ? getRuleKey().hashCode() : "null"); + return keyString.toString(); + } +} From a968525e00c2524c7ae1d0e485ff61dbf4887cd1 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Fri, 8 Oct 2021 10:56:51 -0400 Subject: [PATCH 16/39] Add license header --- .../bucketing/OptimizelyForcedDecisionKey.java | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/core-api/src/main/java/com/optimizely/ab/bucketing/OptimizelyForcedDecisionKey.java b/core-api/src/main/java/com/optimizely/ab/bucketing/OptimizelyForcedDecisionKey.java index 9cbb67f0c..639f744c0 100644 --- a/core-api/src/main/java/com/optimizely/ab/bucketing/OptimizelyForcedDecisionKey.java +++ b/core-api/src/main/java/com/optimizely/ab/bucketing/OptimizelyForcedDecisionKey.java @@ -1,3 +1,19 @@ +/** + * + * Copyright 2021, Optimizely and contributors + * + * 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.bucketing; import javax.annotation.Nonnull; From 3e415ca761876636790b46d1f767dbb2e9362f69 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Fri, 8 Oct 2021 11:36:33 -0400 Subject: [PATCH 17/39] Dont append ruleKey to key object toString method if null. --- .../optimizely/ab/bucketing/OptimizelyForcedDecisionKey.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/core-api/src/main/java/com/optimizely/ab/bucketing/OptimizelyForcedDecisionKey.java b/core-api/src/main/java/com/optimizely/ab/bucketing/OptimizelyForcedDecisionKey.java index 639f744c0..77be19123 100644 --- a/core-api/src/main/java/com/optimizely/ab/bucketing/OptimizelyForcedDecisionKey.java +++ b/core-api/src/main/java/com/optimizely/ab/bucketing/OptimizelyForcedDecisionKey.java @@ -34,7 +34,9 @@ public OptimizelyForcedDecisionKey(@Nonnull String flagKey, String ruleKey) { public String toString() { StringBuilder keyString = new StringBuilder(); keyString.append(getFlagKey().hashCode()); - keyString.append(getRuleKey() != null ? getRuleKey().hashCode() : "null"); + if (getRuleKey() != null) { + keyString.append(getRuleKey().hashCode()); + } return keyString.toString(); } } From 06b12b0c8abd83dd11c22a53550a47b8e1cb6b3e Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Tue, 12 Oct 2021 13:54:05 -0400 Subject: [PATCH 18/39] Update logic in decide method. --- .../java/com/optimizely/ab/Optimizely.java | 24 +++++++++++++------ 1 file changed, 17 insertions(+), 7 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 d0b2d7647..cb2d36179 100644 --- a/core-api/src/main/java/com/optimizely/ab/Optimizely.java +++ b/core-api/src/main/java/com/optimizely/ab/Optimizely.java @@ -1194,13 +1194,23 @@ OptimizelyDecision decide(@Nonnull OptimizelyUserContext user, DecisionReasons decisionReasons = DefaultDecisionReasons.newInstance(allOptions); Map copiedAttributes = new HashMap<>(attributes); - DecisionResponse decisionVariation = decisionService.getVariationForFeature( - flag, - user, - projectConfig, - allOptions); - FeatureDecision flagDecision = decisionVariation.getResult(); - decisionReasons.merge(decisionVariation.getReasons()); + FeatureDecision flagDecision; + + // Check Forced Decision + DecisionResponse forcedDecisionVariation = user.findValidatedForcedDecision(flag.getKey(), null); + decisionReasons.merge(forcedDecisionVariation.getReasons()); + if (forcedDecisionVariation.getResult() != null) { + flagDecision = new FeatureDecision(null, forcedDecisionVariation.getResult(), FeatureDecision.DecisionSource.FEATURE_TEST); + } else { + // Regular decision + DecisionResponse decisionVariation = decisionService.getVariationForFeature( + flag, + user, + projectConfig, + allOptions); + flagDecision = decisionVariation.getResult(); + decisionReasons.merge(decisionVariation.getReasons()); + } Boolean flagEnabled = false; if (flagDecision.variation != null) { From 1d416c84d5b17b6164713e3cbe1633a8b7151588 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Tue, 12 Oct 2021 15:49:17 -0400 Subject: [PATCH 19/39] Change logic to decide and adjust logic for getVariationForFeature. --- .../ab/bucketing/DecisionService.java | 67 +++++++++++++------ 1 file changed, 47 insertions(+), 20 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java b/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java index 88cb28797..53dfb36a8 100644 --- a/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java +++ b/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java @@ -202,6 +202,52 @@ public DecisionResponse getVariationForFeature(@Nonnull Feature @Nonnull List options) { DecisionReasons reasons = DefaultDecisionReasons.newInstance(); + DecisionResponse decisionVariationResponse = getVariationFromExperiment(projectConfig, featureFlag, user, options); + reasons.merge(decisionVariationResponse.getReasons()); + + FeatureDecision decision = decisionVariationResponse.getResult(); + if (decision != null) { + return new DecisionResponse(decision, reasons); + } + + DecisionResponse decisionFeatureResponse = getVariationForFeatureInRollout(featureFlag, user, projectConfig); + reasons.merge(decisionFeatureResponse.getReasons()); + decision = decisionFeatureResponse.getResult(); + + String message; + if (decision.variation == null) { + message = reasons.addInfo("The user \"%s\" was not bucketed into a rollout for feature flag \"%s\".", + user.getUserId(), featureFlag.getKey()); + } else { + message = reasons.addInfo("The user \"%s\" was bucketed into a rollout for feature flag \"%s\".", + user.getUserId(), featureFlag.getKey()); + } + logger.info(message); + + return new DecisionResponse(decision, reasons); + } + + @Nonnull + public DecisionResponse getVariationForFeature(@Nonnull FeatureFlag featureFlag, + @Nonnull OptimizelyUserContext user, + @Nonnull ProjectConfig projectConfig) { + return getVariationForFeature(featureFlag, user, projectConfig, Collections.emptyList()); + } + + /** + * + * @param projectConfig The ProjectConfig. + * @param featureFlag The feature flag the user wants to access. + * @param user The current OptimizelyUserContext. + * @param options An array of decision options + * @return A {@link DecisionResponse} including a {@link FeatureDecision} and the decision reasons + */ + @Nonnull + DecisionResponse getVariationFromExperiment(@Nonnull ProjectConfig projectConfig, + @Nonnull FeatureFlag featureFlag, + @Nonnull OptimizelyUserContext user, + @Nonnull List options) { + DecisionReasons reasons = DefaultDecisionReasons.newInstance(); if (!featureFlag.getExperimentIds().isEmpty()) { for (String experimentId : featureFlag.getExperimentIds()) { Experiment experiment = projectConfig.getExperimentIdMapping().get(experimentId); @@ -221,27 +267,8 @@ public DecisionResponse getVariationForFeature(@Nonnull Feature logger.info(message); } - DecisionResponse decisionFeature = getVariationForFeatureInRollout(featureFlag, user, projectConfig); - reasons.merge(decisionFeature.getReasons()); - FeatureDecision featureDecision = decisionFeature.getResult(); - - String message; - if (featureDecision.variation == null) { - message = reasons.addInfo("The user \"%s\" was not bucketed into a rollout for feature flag \"%s\".", - user.getUserId(), featureFlag.getKey()); - } else { - message = reasons.addInfo("The user \"%s\" was bucketed into a rollout for feature flag \"%s\".", - user.getUserId(), featureFlag.getKey()); - } - logger.info(message); - return new DecisionResponse(featureDecision, reasons); - } + return new DecisionResponse(null, reasons); - @Nonnull - public DecisionResponse getVariationForFeature(@Nonnull FeatureFlag featureFlag, - @Nonnull OptimizelyUserContext user, - @Nonnull ProjectConfig projectConfig) { - return getVariationForFeature(featureFlag, user, projectConfig, Collections.emptyList()); } /** From 443830c42e3c623813bfc05b05c4fdb9863bf173 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Wed, 13 Oct 2021 16:45:12 -0400 Subject: [PATCH 20/39] Update logic for forcedDecisions, add in missing methods for DecisionService.java. --- .../optimizely/ab/OptimizelyUserContext.java | 38 ++++-- .../ab/bucketing/DecisionService.java | 125 ++++++++++++------ 2 files changed, 116 insertions(+), 47 deletions(-) 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 6e87c5508..f484f8bd3 100644 --- a/core-api/src/main/java/com/optimizely/ab/OptimizelyUserContext.java +++ b/core-api/src/main/java/com/optimizely/ab/OptimizelyUserContext.java @@ -28,8 +28,11 @@ public class OptimizelyUserContext { + // Constant used for null values when making keys for Forced Decisions + static String OPTI_NULL_STRING = "NULL"; + // OptimizelyForcedDecisionsKey mapped to variationKeys - Map forcedDecisionsMap = new HashMap<>(); + Map> forcedDecisionsMap = new HashMap<>(); @Nonnull private final String userId; @@ -199,8 +202,17 @@ public Boolean setForcedDecision(@Nonnull String flagKey, String ruleKey, @Nonnu return false; } - OptimizelyForcedDecisionKey key = new OptimizelyForcedDecisionKey(flagKey, ruleKey); - forcedDecisionsMap.put(key.toString(), variationKey); + ruleKey = ruleKey != null ? ruleKey : OPTI_NULL_STRING; + + if (forcedDecisionsMap.get(flagKey) != null) { + Map existingForcedDecisions = forcedDecisionsMap.get(flagKey); + existingForcedDecisions.put(ruleKey, variationKey); + } else { + Map forcedDecision = new HashMap<>(); + forcedDecision.put(ruleKey, variationKey); + forcedDecisionsMap.put(flagKey, forcedDecision); + } + return true; } @@ -222,6 +234,7 @@ public String getForcedDecision(@Nonnull String flagKey) { * @param ruleKey The rule key for the forced decision * @return Returns a variationKey for a given forced decision */ + @Nullable public String getForcedDecision(@Nonnull String flagKey, String ruleKey) { if (optimizely.getOptimizelyConfig() == null) { logger.error("Optimizely SDK not ready."); @@ -237,10 +250,12 @@ public String getForcedDecision(@Nonnull String flagKey, String ruleKey) { * @param ruleKey The rule key for the forced decision * @return Returns a variationKey relating to the found forced decision, otherwise null */ + @Nullable public String findForcedDecision(@Nonnull String flagKey, String ruleKey) { - String variationKey; - OptimizelyForcedDecisionKey lookupKey = new OptimizelyForcedDecisionKey(flagKey, ruleKey); - variationKey = forcedDecisionsMap.get(lookupKey.toString()); + String variationKey = null; + if (forcedDecisionsMap.get(flagKey) != null ) { + variationKey = forcedDecisionsMap.get(flagKey).get(ruleKey != null ? ruleKey : OPTI_NULL_STRING); + } return variationKey; } @@ -267,11 +282,16 @@ public boolean removeForcedDecision(@Nonnull String flagKey, String ruleKey) { return false; } - OptimizelyForcedDecisionKey removeKey = new OptimizelyForcedDecisionKey(flagKey, ruleKey); try { - return forcedDecisionsMap.remove(removeKey.toString()) != null ? true : false; + if (forcedDecisionsMap.get(flagKey) != null) { + ruleKey = ruleKey != null ? ruleKey : OPTI_NULL_STRING; + if (forcedDecisionsMap.get(flagKey).remove(ruleKey) != null) { + forcedDecisionsMap.remove(flagKey); + return true; + } + } } catch (Exception e) { - logger.error("Forced Decision does not exist to remove - " + e); + logger.error("Unable to remove forced-decision - " + e); } return false; diff --git a/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java b/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java index 53dfb36a8..09ee84555 100644 --- a/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java +++ b/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java @@ -25,6 +25,7 @@ import com.optimizely.ab.optimizelydecision.DecisionResponse; import com.optimizely.ab.optimizelydecision.DefaultDecisionReasons; import com.optimizely.ab.optimizelydecision.OptimizelyDecideOption; +import com.sun.org.apache.xpath.internal.operations.Bool; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -306,51 +307,33 @@ DecisionResponse getVariationForFeatureInRollout(@Nonnull Featu if (rolloutRulesLength == 0) { return new DecisionResponse(new FeatureDecision(null, null, null), reasons); } - String bucketingId = getBucketingId(user.getUserId(), user.getAttributes()); - Variation variation; - DecisionResponse decisionMeetAudience; - DecisionResponse decisionVariation; - for (int i = 0; i < rolloutRulesLength - 1; i++) { - Experiment rolloutRule = rollout.getExperiments().get(i); - decisionMeetAudience = ExperimentUtils.doesUserMeetAudienceConditions(projectConfig, rolloutRule, user.getAttributes(), RULE, Integer.toString(i + 1)); - reasons.merge(decisionMeetAudience.getReasons()); - if (decisionMeetAudience.getResult()) { - decisionVariation = bucketer.bucket(rolloutRule, bucketingId, projectConfig); - reasons.merge(decisionVariation.getReasons()); - variation = decisionVariation.getResult(); - - if (variation == null) { - break; - } - return new DecisionResponse( - new FeatureDecision(rolloutRule, variation, FeatureDecision.DecisionSource.ROLLOUT), - reasons); - } else { - String message = reasons.addInfo("User \"%s\" does not meet conditions for targeting rule \"%d\".", user.getUserId(), i + 1); - logger.debug(message); - } - } + int index = 0; + while (index < rolloutRulesLength) { - // get last rule which is the fall back rule - Experiment finalRule = rollout.getExperiments().get(rolloutRulesLength - 1); - - decisionMeetAudience = ExperimentUtils.doesUserMeetAudienceConditions(projectConfig, finalRule, user.getAttributes(), RULE, "Everyone Else"); - reasons.merge(decisionMeetAudience.getReasons()); - if (decisionMeetAudience.getResult()) { - decisionVariation = bucketer.bucket(finalRule, bucketingId, projectConfig); - variation = decisionVariation.getResult(); - reasons.merge(decisionVariation.getReasons()); + DecisionResponse decisionVariationResponse = getVariationFromDeliveryRule( + projectConfig, + featureFlag.getKey(), + rollout.getExperiments(), + index, + user + ); + reasons.merge(decisionVariationResponse.getReasons()); + Map response = decisionVariationResponse.getResult(); + Variation variation = (Variation)response.keySet().toArray()[0]; + Boolean skipToEveryoneElse = response.get(variation); if (variation != null) { - String message = reasons.addInfo("User \"%s\" meets conditions for targeting rule \"Everyone Else\".", user.getUserId()); - logger.debug(message); - return new DecisionResponse( - new FeatureDecision(finalRule, variation, FeatureDecision.DecisionSource.ROLLOUT), - reasons); + Experiment rule = rollout.getExperiments().get(index); + FeatureDecision featureDecision = new FeatureDecision(rule, variation, FeatureDecision.DecisionSource.ROLLOUT); + return new DecisionResponse(featureDecision, reasons); } + + // The last rule is special for "Everyone Else" + index = skipToEveryoneElse ? (rolloutRulesLength - 1) : (index + 1); } + return new DecisionResponse(new FeatureDecision(null, null, null), reasons); } @@ -638,4 +621,70 @@ private boolean validateUserId(String userId) { return (userId != null); } + DecisionResponse getVariationFromDeliveryRule(@Nonnull ProjectConfig projectConfig, + @Nonnull String flagKey, + @Nonnull List rules, + @Nonnull int ruleIndex, + @Nonnull OptimizelyUserContext user) { + DecisionReasons reasons = DefaultDecisionReasons.newInstance(); + Boolean skipToEveryoneElse = false; + Map variationToSkipToEveryoneElse = new HashMap<>(); + // Check forced-decisions first + Experiment rule = rules.get(ruleIndex); + DecisionResponse forcedDecisionResponse = user.findValidatedForcedDecision(flagKey, rule.getKey()); + reasons.merge(forcedDecisionResponse.getReasons()); + + Variation variation = forcedDecisionResponse.getResult(); + if (variation != null) { + variationToSkipToEveryoneElse.put(variation, false); + return new DecisionResponse(variationToSkipToEveryoneElse, reasons); + } + + // Handle a regular decision + String bucketingId = getBucketingId(user.getUserId(), user.getAttributes()); + Boolean everyoneElse = (ruleIndex == rules.size() - 1); + String loggingKey = everyoneElse ? "Everyone Else" : String.valueOf(ruleIndex + 1); + + Variation bucketedVariation = null; + + DecisionResponse audienceDecisionResponse = ExperimentUtils.doesUserMeetAudienceConditions( + projectConfig, + rule, + user.getAttributes(), + RULE, + String.valueOf(ruleIndex + 1) + ); + + reasons.merge(audienceDecisionResponse.getReasons()); + String message; + if (audienceDecisionResponse.getResult()) { + message = reasons.addInfo("User \"%s\" meets conditions for targeting rule \"%s\".", user.getUserId(), loggingKey); + reasons.addInfo(message); + logger.debug(message); + + DecisionResponse decisionResponse = bucketer.bucket(rule, bucketingId, projectConfig); + reasons.merge(decisionResponse.getReasons()); + bucketedVariation = decisionResponse.getResult(); + + if (bucketedVariation != null) { + message = reasons.addInfo("User \"%s\" bucketed for targeting rule \"%s\".", user.getUserId(), loggingKey); + logger.debug(message); + reasons.addInfo(message); + } else if (!everyoneElse) { + message = reasons.addInfo("User \"%s\" is not bucketed for targeting rule \"%s\".", user.getUserId(), loggingKey); + logger.debug(message); + reasons.addInfo(message); + // Skip the rest of rollout rules to the everyone-else rule if audience matches but not bucketed. + skipToEveryoneElse = true; + } + } else { + message = reasons.addInfo("User \"%s\" does not meet conditions for targeting rule \"%d\".", user.getUserId(), ruleIndex + 1); + reasons.addInfo(message); + logger.debug(message); + } + variationToSkipToEveryoneElse.clear(); + variationToSkipToEveryoneElse.put(bucketedVariation, skipToEveryoneElse); + return new DecisionResponse(variationToSkipToEveryoneElse, reasons); + } + } From 2161c00050b4214cc77dc9680193590b8aa6ca6a Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Thu, 14 Oct 2021 11:55:05 -0400 Subject: [PATCH 21/39] Update coverage and remove unused class --- .../OptimizelyForcedDecisionKey.java | 42 ---------------- .../ab/bucketing/DecisionServiceTest.java | 49 +++++++++++++++++++ 2 files changed, 49 insertions(+), 42 deletions(-) delete mode 100644 core-api/src/main/java/com/optimizely/ab/bucketing/OptimizelyForcedDecisionKey.java diff --git a/core-api/src/main/java/com/optimizely/ab/bucketing/OptimizelyForcedDecisionKey.java b/core-api/src/main/java/com/optimizely/ab/bucketing/OptimizelyForcedDecisionKey.java deleted file mode 100644 index 77be19123..000000000 --- a/core-api/src/main/java/com/optimizely/ab/bucketing/OptimizelyForcedDecisionKey.java +++ /dev/null @@ -1,42 +0,0 @@ -/** - * - * Copyright 2021, Optimizely and contributors - * - * 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.bucketing; - -import javax.annotation.Nonnull; - -public class OptimizelyForcedDecisionKey { - private String flagKey; - private String ruleKey; - - public OptimizelyForcedDecisionKey(@Nonnull String flagKey, String ruleKey) { - this.flagKey = flagKey; - this.ruleKey = ruleKey; - } - - public String getFlagKey() { return flagKey; } - - public String getRuleKey() { return ruleKey; } - - public String toString() { - StringBuilder keyString = new StringBuilder(); - keyString.append(getFlagKey().hashCode()); - if (getRuleKey() != null) { - keyString.append(getRuleKey().hashCode()); - } - return keyString.toString(); - } -} diff --git a/core-api/src/test/java/com/optimizely/ab/bucketing/DecisionServiceTest.java b/core-api/src/test/java/com/optimizely/ab/bucketing/DecisionServiceTest.java index 4962d319f..c1235e809 100644 --- a/core-api/src/test/java/com/optimizely/ab/bucketing/DecisionServiceTest.java +++ b/core-api/src/test/java/com/optimizely/ab/bucketing/DecisionServiceTest.java @@ -715,6 +715,55 @@ public void getVariationForFeatureInRolloutReturnsVariationWhenUserFailsTargetin verify(mockBucketer, times(1)).bucket(any(Experiment.class), anyString(), any(ProjectConfig.class)); } + @Test + public void getVariationFromDeliveryRuleTest() { + int index = 3; + List rules = ROLLOUT_2.getExperiments(); + Experiment experiment = ROLLOUT_2.getExperiments().get(index); + Variation expectedVariation = null; + for (Variation variation : experiment.getVariations()) { + if (variation.getKey().equals("3137445031")) { + expectedVariation = variation; + } + } + DecisionResponse decisionResponse = decisionService.getVariationFromDeliveryRule( + v4ProjectConfig, + FEATURE_FLAG_MULTI_VARIATE_FEATURE.getKey(), + rules, + index, + optimizely.createUserContext(genericUserId, Collections.singletonMap(ATTRIBUTE_NATIONALITY_KEY, AUDIENCE_ENGLISH_CITIZENS_VALUE)) + ); + + Variation variation = (Variation) decisionResponse.getResult().keySet().toArray()[0]; + Boolean skipToEveryoneElse = (Boolean) decisionResponse.getResult().get(variation); + assertNotNull(decisionResponse.getResult()); + assertNotNull(variation); + assertNotNull(expectedVariation); + assertEquals(expectedVariation, variation); + assertFalse(skipToEveryoneElse); + } + + @Test + public void getVariationFromExperimentRuleTest() { + int index = 3; + Experiment experiment = ROLLOUT_2.getExperiments().get(index); + Variation expectedVariation = null; + for (Variation variation : experiment.getVariations()) { + if (variation.getKey().equals("3137445031")) { + expectedVariation = variation; + } + } + DecisionResponse decisionResponse = decisionService.getVariationFromExperimentRule( + v4ProjectConfig, + FEATURE_FLAG_MULTI_VARIATE_FEATURE.getKey(), + experiment, + optimizely.createUserContext(genericUserId, Collections.singletonMap(ATTRIBUTE_NATIONALITY_KEY, AUDIENCE_ENGLISH_CITIZENS_VALUE)), + Collections.emptyList() + ); + + assertEquals(expectedVariation, decisionResponse.getResult()); + } + //========= white list tests ==========/ /** From b9f43384c5c6df21a79edf9296c0a9ef28e34770 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Thu, 14 Oct 2021 12:24:09 -0400 Subject: [PATCH 22/39] Add comment to method --- .../com/optimizely/ab/bucketing/DecisionService.java | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java b/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java index 09ee84555..c59812b26 100644 --- a/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java +++ b/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java @@ -621,6 +621,16 @@ private boolean validateUserId(String userId) { return (userId != null); } + /** + * + * @param projectConfig The Project config + * @param flagKey The flag key for the feature flag + * @param rules The experiments belonging to a rollout + * @param ruleIndex The index of the rule + * @param user The OptimizelyUserContext + * @return Returns a DecisionResponse Object containing a Map + * where the Variation is the result and the Boolean is the skipToEveryoneElse. + */ DecisionResponse getVariationFromDeliveryRule(@Nonnull ProjectConfig projectConfig, @Nonnull String flagKey, @Nonnull List rules, From 10b05c1207ee5ac0510d1a99a11d1fc9ade9cbc1 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Thu, 14 Oct 2021 12:32:26 -0400 Subject: [PATCH 23/39] Remove unused import - cleanup. --- .../src/main/java/com/optimizely/ab/OptimizelyUserContext.java | 1 - 1 file changed, 1 deletion(-) 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 f484f8bd3..e525cabd5 100644 --- a/core-api/src/main/java/com/optimizely/ab/OptimizelyUserContext.java +++ b/core-api/src/main/java/com/optimizely/ab/OptimizelyUserContext.java @@ -16,7 +16,6 @@ */ package com.optimizely.ab; -import com.optimizely.ab.bucketing.OptimizelyForcedDecisionKey; import com.optimizely.ab.config.Variation; import com.optimizely.ab.optimizelydecision.*; import org.slf4j.Logger; From 7b6e8696213c92e8f3cdb58274b830c8d3ce2c3a Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Wed, 20 Oct 2021 09:18:06 -0400 Subject: [PATCH 24/39] Remove null from findValidatedForcedDecisions call in decide method since ruleKey is never passed. --- core-api/src/main/java/com/optimizely/ab/Optimizely.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 cb2d36179..dc6449da1 100644 --- a/core-api/src/main/java/com/optimizely/ab/Optimizely.java +++ b/core-api/src/main/java/com/optimizely/ab/Optimizely.java @@ -1197,7 +1197,7 @@ OptimizelyDecision decide(@Nonnull OptimizelyUserContext user, FeatureDecision flagDecision; // Check Forced Decision - DecisionResponse forcedDecisionVariation = user.findValidatedForcedDecision(flag.getKey(), null); + DecisionResponse forcedDecisionVariation = user.findValidatedForcedDecision(flag.getKey()); decisionReasons.merge(forcedDecisionVariation.getReasons()); if (forcedDecisionVariation.getResult() != null) { flagDecision = new FeatureDecision(null, forcedDecisionVariation.getResult(), FeatureDecision.DecisionSource.FEATURE_TEST); From 2c79e6d7c1da39ae8a0fcc64d97ad0e56ae50b01 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Wed, 20 Oct 2021 09:57:26 -0400 Subject: [PATCH 25/39] Switch to String.format for info. --- .../com/optimizely/ab/OptimizelyUserContext.java | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) 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 e525cabd5..57bbb85bc 100644 --- a/core-api/src/main/java/com/optimizely/ab/OptimizelyUserContext.java +++ b/core-api/src/main/java/com/optimizely/ab/OptimizelyUserContext.java @@ -334,20 +334,14 @@ public DecisionResponse findValidatedForcedDecision(@Nonnull String f if (variationKey != null) { Variation variation = optimizely.getFlagVariationByKey(flagKey, variationKey); String strRuleKey = ruleKey != null ? ruleKey : "null"; + String info; if (variation != null) { - String info = "Variation " + variationKey - + " is mapped to flag: " + flagKey - + " and rule: " + strRuleKey - + " and user: " + userId - + " in the forced decision map."; + info = String.format("Variation %s is mapped to flag: %s and rule: %s and user: %s in the forced decisions map.", variationKey, flagKey, strRuleKey, userId); logger.debug(info); reasons.addInfo(info); return new DecisionResponse(variation, reasons); } else { - String info = "Invalid variation is mapped to flag: " + flagKey - + " and rule: " + strRuleKey - + " and user: " + userId - + " forced decision map."; + info = String.format("Invalid variation is mapped to flag: %s and rule: %s and user: %s in the forced decisions map.", flagKey, strRuleKey, userId); logger.debug(info); reasons.addInfo(info); } From 3d0f887b4487bd4d6c22091468a6a6b1254b52cd Mon Sep 17 00:00:00 2001 From: Jake Brown Date: Fri, 22 Oct 2021 14:01:22 -0400 Subject: [PATCH 26/39] Adding in new classes for forcedDecisionsAPI --- .../main/java/com/optimizely/ab/OptimizelyDecisionContext.java | 2 ++ .../main/java/com/optimizely/ab/OptimizelyForcedDecision.java | 2 ++ 2 files changed, 4 insertions(+) create mode 100644 core-api/src/main/java/com/optimizely/ab/OptimizelyDecisionContext.java create mode 100644 core-api/src/main/java/com/optimizely/ab/OptimizelyForcedDecision.java diff --git a/core-api/src/main/java/com/optimizely/ab/OptimizelyDecisionContext.java b/core-api/src/main/java/com/optimizely/ab/OptimizelyDecisionContext.java new file mode 100644 index 000000000..985b3e126 --- /dev/null +++ b/core-api/src/main/java/com/optimizely/ab/OptimizelyDecisionContext.java @@ -0,0 +1,2 @@ +package com.optimizely.ab;public class OptimizelyDecisionContext { +} diff --git a/core-api/src/main/java/com/optimizely/ab/OptimizelyForcedDecision.java b/core-api/src/main/java/com/optimizely/ab/OptimizelyForcedDecision.java new file mode 100644 index 000000000..08a096bb1 --- /dev/null +++ b/core-api/src/main/java/com/optimizely/ab/OptimizelyForcedDecision.java @@ -0,0 +1,2 @@ +package com.optimizely.ab;public class OptimizelyForcedDecision { +} From 55c07574e30394260d01e9b6e10c2db3252424be Mon Sep 17 00:00:00 2001 From: Jake Brown Date: Fri, 22 Oct 2021 14:04:23 -0400 Subject: [PATCH 27/39] Addressing changes and new API implementations. --- .../java/com/optimizely/ab/Optimizely.java | 3 +- .../ab/OptimizelyDecisionContext.java | 31 +++- .../ab/OptimizelyForcedDecision.java | 15 +- .../optimizely/ab/OptimizelyUserContext.java | 124 +++++---------- .../ab/bucketing/DecisionService.java | 29 ++-- .../ab/bucketing/FeatureDecision.java | 2 +- .../ab/OptimizelyUserContextTest.java | 147 ++++++++++++------ .../ab/bucketing/DecisionServiceTest.java | 7 +- 8 files changed, 200 insertions(+), 158 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 dc6449da1..347204802 100644 --- a/core-api/src/main/java/com/optimizely/ab/Optimizely.java +++ b/core-api/src/main/java/com/optimizely/ab/Optimizely.java @@ -1197,7 +1197,8 @@ OptimizelyDecision decide(@Nonnull OptimizelyUserContext user, FeatureDecision flagDecision; // Check Forced Decision - DecisionResponse forcedDecisionVariation = user.findValidatedForcedDecision(flag.getKey()); + OptimizelyDecisionContext optimizelyDecisionContext = new OptimizelyDecisionContext(flag.getKey(), null); + DecisionResponse forcedDecisionVariation = user.findValidatedForcedDecision(optimizelyDecisionContext); decisionReasons.merge(forcedDecisionVariation.getReasons()); if (forcedDecisionVariation.getResult() != null) { flagDecision = new FeatureDecision(null, forcedDecisionVariation.getResult(), FeatureDecision.DecisionSource.FEATURE_TEST); diff --git a/core-api/src/main/java/com/optimizely/ab/OptimizelyDecisionContext.java b/core-api/src/main/java/com/optimizely/ab/OptimizelyDecisionContext.java index 985b3e126..f92da3747 100644 --- a/core-api/src/main/java/com/optimizely/ab/OptimizelyDecisionContext.java +++ b/core-api/src/main/java/com/optimizely/ab/OptimizelyDecisionContext.java @@ -1,2 +1,31 @@ -package com.optimizely.ab;public class OptimizelyDecisionContext { +package com.optimizely.ab; + +import javax.annotation.Nonnull; +import javax.annotation.Nullable; + +public class OptimizelyDecisionContext { + public static final String OPTI_NULL_RULE_KEY = "$opt-null-rule-key"; + + private String flagKey; + private String ruleKey; + + public OptimizelyDecisionContext(@Nonnull String flagKey, @Nullable String ruleKey) { + this.flagKey = flagKey; + this.ruleKey = ruleKey; + } + + public String getFlagKey() { + return flagKey; + } + + public String getRuleKey() { + return ruleKey != null ? ruleKey : OPTI_NULL_RULE_KEY; + } + + public String getKey() { + StringBuilder keyBuilder = new StringBuilder(); + keyBuilder.append(flagKey); + keyBuilder.append(getRuleKey()); + return keyBuilder.toString(); + } } diff --git a/core-api/src/main/java/com/optimizely/ab/OptimizelyForcedDecision.java b/core-api/src/main/java/com/optimizely/ab/OptimizelyForcedDecision.java index 08a096bb1..d3cb2ea01 100644 --- a/core-api/src/main/java/com/optimizely/ab/OptimizelyForcedDecision.java +++ b/core-api/src/main/java/com/optimizely/ab/OptimizelyForcedDecision.java @@ -1,2 +1,15 @@ -package com.optimizely.ab;public class OptimizelyForcedDecision { +package com.optimizely.ab; + +import javax.annotation.Nonnull; + +public class OptimizelyForcedDecision { + private String variationKey; + + public OptimizelyForcedDecision(@Nonnull String variationKey) { + this.variationKey = variationKey; + } + + public String getVariationKey() { + return variationKey; + } } 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 57bbb85bc..f8ed5862a 100644 --- a/core-api/src/main/java/com/optimizely/ab/OptimizelyUserContext.java +++ b/core-api/src/main/java/com/optimizely/ab/OptimizelyUserContext.java @@ -24,14 +24,11 @@ import javax.annotation.Nonnull; import javax.annotation.Nullable; import java.util.*; +import java.util.concurrent.ConcurrentHashMap; public class OptimizelyUserContext { - - // Constant used for null values when making keys for Forced Decisions - static String OPTI_NULL_STRING = "NULL"; - // OptimizelyForcedDecisionsKey mapped to variationKeys - Map> forcedDecisionsMap = new HashMap<>(); + Map forcedDecisionsMap; @Nonnull private final String userId; @@ -179,113 +176,69 @@ public void trackEvent(@Nonnull String eventName) throws UnknownEventTypeExcepti /** * Set a forced decision * - * @param flagKey The flag key for the forced decision - * @param variationKey The variation key for the forced decision - * @return Returns a boolean, True if successfully set, otherwise false - */ - public Boolean setForcedDecision(@Nonnull String flagKey, @Nonnull String variationKey) { - return setForcedDecision(flagKey, null, variationKey); - } - - /** - * Set a forced decision - * - * @param flagKey The flag key for the forced decision - * @param ruleKey The rule key for the forced decision - * @param variationKey The variation key for the forced decision + * @param optimizelyDecisionContext The OptimizelyDecisionContext containing flagKey and ruleKey + * @param optimizelyForcedDecision The OptimizelyForcedDecision containing the variationKey * @return Returns a boolean, Ture if successfully set, otherwise false */ - public Boolean setForcedDecision(@Nonnull String flagKey, String ruleKey, @Nonnull String variationKey) { + public Boolean setForcedDecision(@Nonnull OptimizelyDecisionContext optimizelyDecisionContext, + @Nonnull OptimizelyForcedDecision optimizelyForcedDecision) { if (optimizely.getOptimizelyConfig() == null) { logger.error("Optimizely SDK not ready."); return false; } - - ruleKey = ruleKey != null ? ruleKey : OPTI_NULL_STRING; - - if (forcedDecisionsMap.get(flagKey) != null) { - Map existingForcedDecisions = forcedDecisionsMap.get(flagKey); - existingForcedDecisions.put(ruleKey, variationKey); - } else { - Map forcedDecision = new HashMap<>(); - forcedDecision.put(ruleKey, variationKey); - forcedDecisionsMap.put(flagKey, forcedDecision); + // Check if the forcedDecisionsMap has been initialized yet or not + if (forcedDecisionsMap == null ){ + // Thread-safe implementation of HashMap + forcedDecisionsMap = new ConcurrentHashMap<>(); } - - + forcedDecisionsMap.put(optimizelyDecisionContext.getKey(), optimizelyForcedDecision); return true; } /** * Get a forced decision * - * @param flagKey The flag key for the forced decision - * @return Returns a variationKey for a given forced decision - */ - public String getForcedDecision(@Nonnull String flagKey) { - return getForcedDecision(flagKey, null); - } - - /** - * Get a forced decision - * - * @param flagKey The flag key for the forced decision - * @param ruleKey The rule key for the forced decision + * @param optimizelyDecisionContext The OptimizelyDecisionContext containing flagKey and ruleKey * @return Returns a variationKey for a given forced decision */ @Nullable - public String getForcedDecision(@Nonnull String flagKey, String ruleKey) { + public OptimizelyForcedDecision getForcedDecision(@Nonnull OptimizelyDecisionContext optimizelyDecisionContext) { if (optimizely.getOptimizelyConfig() == null) { logger.error("Optimizely SDK not ready."); return null; } - return findForcedDecision(flagKey, ruleKey); + return findForcedDecision(optimizelyDecisionContext); } /** * Finds a forced decision * - * @param flagKey The flag key for the forced decision - * @param ruleKey The rule key for the forced decision + * @param optimizelyDecisionContext The OptimizelyDecisionContext containing flagKey and ruleKey * @return Returns a variationKey relating to the found forced decision, otherwise null */ @Nullable - public String findForcedDecision(@Nonnull String flagKey, String ruleKey) { - String variationKey = null; - if (forcedDecisionsMap.get(flagKey) != null ) { - variationKey = forcedDecisionsMap.get(flagKey).get(ruleKey != null ? ruleKey : OPTI_NULL_STRING); + public OptimizelyForcedDecision findForcedDecision(@Nonnull OptimizelyDecisionContext optimizelyDecisionContext) { + if (forcedDecisionsMap != null) { + return forcedDecisionsMap.get(optimizelyDecisionContext.getKey()); } - return variationKey; - } - - /** - * Remove a forced decision - * - * @param flagKey The flag key in the forced decision - * @return Returns a boolean of true if successful, otherwise false - */ - public boolean removeForcedDecision(@Nonnull String flagKey) { - return removeForcedDecision(flagKey, null); + return null; } /** * Remove a forced decision * - * @param flagKey The flag key for the forced decision - * @param ruleKey The rule key for the forced decision + * @param optimizelyDecisionContext The OptimizelyDecisionContext containing flagKey and ruleKey * @return Returns a boolean, true if successfully removed, otherwise false */ - public boolean removeForcedDecision(@Nonnull String flagKey, String ruleKey) { + public boolean removeForcedDecision(@Nonnull OptimizelyDecisionContext optimizelyDecisionContext) { if (optimizely.getOptimizelyConfig() == null) { logger.error("Optimizely SDK not ready."); return false; } try { - if (forcedDecisionsMap.get(flagKey) != null) { - ruleKey = ruleKey != null ? ruleKey : OPTI_NULL_STRING; - if (forcedDecisionsMap.get(flagKey).remove(ruleKey) != null) { - forcedDecisionsMap.remove(flagKey); + if (forcedDecisionsMap != null) { + if (forcedDecisionsMap.remove(optimizelyDecisionContext.getKey()) != null) { return true; } } @@ -307,41 +260,34 @@ public boolean removeAllForcedDecisions() { return false; } // Clear both maps for with and without ruleKey - forcedDecisionsMap.clear(); + if (forcedDecisionsMap != null) { + forcedDecisionsMap.clear(); + } return true; } /** * Find a validated forced decision * - * @param flagKey The flag key for the forced decision - * @return Returns a DecisionResponse structure of type Variation, otherwise null with reasons - */ - public DecisionResponse findValidatedForcedDecision(@Nonnull String flagKey) { - return findValidatedForcedDecision(flagKey, null); - } - - /** - * Find a validated forced decision - * - * @param flagKey The flag key for a forced decision - * @param ruleKey The rule key for a forced decision + * @param optimizelyDecisionContext The OptimizelyDecisionContext containing flagKey and ruleKey * @return Returns a DecisionResponse structure of type Variation, otherwise null result with reasons */ - public DecisionResponse findValidatedForcedDecision(@Nonnull String flagKey, String ruleKey) { + public DecisionResponse findValidatedForcedDecision(@Nonnull OptimizelyDecisionContext optimizelyDecisionContext) { DecisionReasons reasons = DefaultDecisionReasons.newInstance(); - String variationKey = findForcedDecision(flagKey, ruleKey); + OptimizelyForcedDecision optimizelyForcedDecision = findForcedDecision(optimizelyDecisionContext); + String variationKey = optimizelyForcedDecision != null ? optimizelyForcedDecision.getVariationKey() : null; if (variationKey != null) { - Variation variation = optimizely.getFlagVariationByKey(flagKey, variationKey); - String strRuleKey = ruleKey != null ? ruleKey : "null"; + Variation variation = optimizely.getFlagVariationByKey(optimizelyDecisionContext.getFlagKey(), variationKey); + String ruleKey = optimizelyDecisionContext.getRuleKey(); + String flagKey = optimizelyDecisionContext.getFlagKey(); String info; if (variation != null) { - info = String.format("Variation %s is mapped to flag: %s and rule: %s and user: %s in the forced decisions map.", variationKey, flagKey, strRuleKey, userId); + info = String.format("Variation %s is mapped to flag: %s and rule: %s and user: %s in the forced decisions map.", variationKey, flagKey, ruleKey, userId); logger.debug(info); reasons.addInfo(info); return new DecisionResponse(variation, reasons); } else { - info = String.format("Invalid variation is mapped to flag: %s and rule: %s and user: %s in the forced decisions map.", flagKey, strRuleKey, userId); + info = String.format("Invalid variation is mapped to flag: %s and rule: %s and user: %s in the forced decisions map.", flagKey, ruleKey, userId); logger.debug(info); reasons.addInfo(info); } diff --git a/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java b/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java index c59812b26..12b5659fa 100644 --- a/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java +++ b/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java @@ -15,6 +15,8 @@ ***************************************************************************/ package com.optimizely.ab.bucketing; +import com.optimizely.ab.OptimizelyDecisionContext; +import com.optimizely.ab.OptimizelyForcedDecision; import com.optimizely.ab.OptimizelyRuntimeException; import com.optimizely.ab.OptimizelyUserContext; import com.optimizely.ab.config.*; @@ -26,6 +28,7 @@ import com.optimizely.ab.optimizelydecision.DefaultDecisionReasons; import com.optimizely.ab.optimizelydecision.OptimizelyDecideOption; import com.sun.org.apache.xpath.internal.operations.Bool; +import javafx.util.Pair; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -312,7 +315,7 @@ DecisionResponse getVariationForFeatureInRollout(@Nonnull Featu int index = 0; while (index < rolloutRulesLength) { - DecisionResponse decisionVariationResponse = getVariationFromDeliveryRule( + DecisionResponse decisionVariationResponse = getVariationFromDeliveryRule( projectConfig, featureFlag.getKey(), rollout.getExperiments(), @@ -321,9 +324,9 @@ DecisionResponse getVariationForFeatureInRollout(@Nonnull Featu ); reasons.merge(decisionVariationResponse.getReasons()); - Map response = decisionVariationResponse.getResult(); - Variation variation = (Variation)response.keySet().toArray()[0]; - Boolean skipToEveryoneElse = response.get(variation); + Pair response = decisionVariationResponse.getResult(); + Variation variation = response.getKey(); + Boolean skipToEveryoneElse = response.getValue(); if (variation != null) { Experiment rule = rollout.getExperiments().get(index); FeatureDecision featureDecision = new FeatureDecision(rule, variation, FeatureDecision.DecisionSource.ROLLOUT); @@ -593,8 +596,9 @@ public DecisionResponse getVariationFromExperimentRule(@Nonnull Proje DecisionReasons reasons = DefaultDecisionReasons.newInstance(); String ruleKey = rule != null ? rule.getKey() : null; + OptimizelyDecisionContext optimizelyDecisionContext = new OptimizelyDecisionContext(flagKey, ruleKey); // Check Forced-Decision - DecisionResponse forcedDecisionResponse = user.findValidatedForcedDecision(flagKey, ruleKey); + DecisionResponse forcedDecisionResponse = user.findValidatedForcedDecision(optimizelyDecisionContext); reasons.merge(forcedDecisionResponse.getReasons()); @@ -628,25 +632,27 @@ private boolean validateUserId(String userId) { * @param rules The experiments belonging to a rollout * @param ruleIndex The index of the rule * @param user The OptimizelyUserContext - * @return Returns a DecisionResponse Object containing a Map + * @return Returns a DecisionResponse Object containing a Pair * where the Variation is the result and the Boolean is the skipToEveryoneElse. */ - DecisionResponse getVariationFromDeliveryRule(@Nonnull ProjectConfig projectConfig, + DecisionResponse getVariationFromDeliveryRule(@Nonnull ProjectConfig projectConfig, @Nonnull String flagKey, @Nonnull List rules, @Nonnull int ruleIndex, @Nonnull OptimizelyUserContext user) { DecisionReasons reasons = DefaultDecisionReasons.newInstance(); Boolean skipToEveryoneElse = false; - Map variationToSkipToEveryoneElse = new HashMap<>(); + Pair variationToSkipToEveryoneElse; + // Check forced-decisions first Experiment rule = rules.get(ruleIndex); - DecisionResponse forcedDecisionResponse = user.findValidatedForcedDecision(flagKey, rule.getKey()); + OptimizelyDecisionContext optimizelyDecisionContext = new OptimizelyDecisionContext(flagKey, rule.getKey()); + DecisionResponse forcedDecisionResponse = user.findValidatedForcedDecision(optimizelyDecisionContext); reasons.merge(forcedDecisionResponse.getReasons()); Variation variation = forcedDecisionResponse.getResult(); if (variation != null) { - variationToSkipToEveryoneElse.put(variation, false); + variationToSkipToEveryoneElse = new Pair<>(variation, false); return new DecisionResponse(variationToSkipToEveryoneElse, reasons); } @@ -692,8 +698,7 @@ DecisionResponse getVariationFromDeliveryRule(@Nonnull ProjectConfig projec reasons.addInfo(message); logger.debug(message); } - variationToSkipToEveryoneElse.clear(); - variationToSkipToEveryoneElse.put(bucketedVariation, skipToEveryoneElse); + variationToSkipToEveryoneElse = new Pair<>(bucketedVariation, skipToEveryoneElse); return new DecisionResponse(variationToSkipToEveryoneElse, reasons); } diff --git a/core-api/src/main/java/com/optimizely/ab/bucketing/FeatureDecision.java b/core-api/src/main/java/com/optimizely/ab/bucketing/FeatureDecision.java index 6cb81509d..b0f0a11ed 100644 --- a/core-api/src/main/java/com/optimizely/ab/bucketing/FeatureDecision.java +++ b/core-api/src/main/java/com/optimizely/ab/bucketing/FeatureDecision.java @@ -20,7 +20,7 @@ import javax.annotation.Nullable; -public class FeatureDecision { +public class FeatureDecision { /** * The {@link Experiment} the Feature is associated with. */ 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 899c50a59..5ce8ee2ce 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java @@ -1200,9 +1200,10 @@ public void setForcedDecisionWithRuleKeyTest() { optimizely, userId, Collections.emptyMap()); - - optimizelyUserContext.setForcedDecision(flagKey, ruleKey, variationKey); - String foundVariationKey = optimizelyUserContext.getForcedDecision(flagKey, ruleKey); + OptimizelyDecisionContext optimizelyDecisionContext = new OptimizelyDecisionContext(flagKey, ruleKey); + OptimizelyForcedDecision optimizelyForcedDecision = new OptimizelyForcedDecision(variationKey); + optimizelyUserContext.setForcedDecision(optimizelyDecisionContext, optimizelyForcedDecision); + String foundVariationKey = optimizelyUserContext.getForcedDecision(optimizelyDecisionContext).getVariationKey(); assertEquals(variationKey, foundVariationKey); } @@ -1218,14 +1219,18 @@ public void setForcedDecisionsWithRuleKeyTest() { userId, Collections.emptyMap()); - optimizelyUserContext.setForcedDecision(flagKey, ruleKey, variationKey); - optimizelyUserContext.setForcedDecision(flagKey, ruleKey2, variationKey2); - assertEquals(variationKey, optimizelyUserContext.getForcedDecision(flagKey, ruleKey)); - assertEquals(variationKey2, optimizelyUserContext.getForcedDecision(flagKey, ruleKey2)); + OptimizelyDecisionContext optimizelyDecisionContext = new OptimizelyDecisionContext(flagKey, ruleKey); + OptimizelyForcedDecision optimizelyForcedDecision = new OptimizelyForcedDecision(variationKey); + OptimizelyDecisionContext optimizelyDecisionContext2 = new OptimizelyDecisionContext(flagKey, ruleKey2); + OptimizelyForcedDecision optimizelyForcedDecision2 = new OptimizelyForcedDecision(variationKey2); + optimizelyUserContext.setForcedDecision(optimizelyDecisionContext, optimizelyForcedDecision); + optimizelyUserContext.setForcedDecision(optimizelyDecisionContext2, optimizelyForcedDecision2); + assertEquals(variationKey, optimizelyUserContext.getForcedDecision(optimizelyDecisionContext).getVariationKey()); + assertEquals(variationKey2, optimizelyUserContext.getForcedDecision(optimizelyDecisionContext2).getVariationKey()); // Update first forcedDecision - optimizelyUserContext.setForcedDecision(flagKey, ruleKey, variationKey2); - assertEquals(variationKey2, optimizelyUserContext.getForcedDecision(flagKey, ruleKey)); + optimizelyUserContext.setForcedDecision(optimizelyDecisionContext, optimizelyForcedDecision2); + assertEquals(variationKey2, optimizelyUserContext.getForcedDecision(optimizelyDecisionContext).getVariationKey()); } @Test @@ -1238,12 +1243,16 @@ public void setForcedDecisionWithoutRuleKeyTest() { userId, Collections.emptyMap()); - optimizelyUserContext.setForcedDecision(flagKey, variationKey); - assertEquals(variationKey, optimizelyUserContext.getForcedDecision(flagKey)); + OptimizelyDecisionContext optimizelyDecisionContext = new OptimizelyDecisionContext(flagKey, null); + OptimizelyForcedDecision optimizelyForcedDecision = new OptimizelyForcedDecision(variationKey); + OptimizelyForcedDecision updatedOptimizelyForcedDecision = new OptimizelyForcedDecision(updatedVariationKey); + + optimizelyUserContext.setForcedDecision(optimizelyDecisionContext, optimizelyForcedDecision); + assertEquals(variationKey, optimizelyUserContext.getForcedDecision(optimizelyDecisionContext).getVariationKey()); // Update forcedDecision - optimizelyUserContext.setForcedDecision(flagKey, updatedVariationKey); - assertEquals(updatedVariationKey, optimizelyUserContext.getForcedDecision(flagKey)); + optimizelyUserContext.setForcedDecision(optimizelyDecisionContext, updatedOptimizelyForcedDecision); + assertEquals(updatedVariationKey, optimizelyUserContext.getForcedDecision(optimizelyDecisionContext).getVariationKey()); } @@ -1257,7 +1266,9 @@ public void setForcedDecisionWithoutRuleKeyTestSdkNotReady() { userId, Collections.emptyMap()); - assertFalse(optimizelyUserContext.setForcedDecision(flagKey, variationKey)); + OptimizelyDecisionContext optimizelyDecisionContext = new OptimizelyDecisionContext(flagKey, null); + OptimizelyForcedDecision optimizelyForcedDecision = new OptimizelyForcedDecision(variationKey); + assertFalse(optimizelyUserContext.setForcedDecision(optimizelyDecisionContext, optimizelyForcedDecision)); } @Test @@ -1270,8 +1281,11 @@ public void getForcedVariationWithRuleKey() { userId, Collections.emptyMap()); - optimizelyUserContext.setForcedDecision(flagKey, ruleKey, variationKey); - assertEquals(variationKey, optimizelyUserContext.getForcedDecision(flagKey, ruleKey)); + OptimizelyDecisionContext optimizelyDecisionContext = new OptimizelyDecisionContext(flagKey, ruleKey); + OptimizelyForcedDecision optimizelyForcedDecision = new OptimizelyForcedDecision(variationKey); + + optimizelyUserContext.setForcedDecision(optimizelyDecisionContext, optimizelyForcedDecision); + assertEquals(variationKey, optimizelyUserContext.getForcedDecision(optimizelyDecisionContext).getVariationKey()); } @Test @@ -1285,8 +1299,12 @@ public void failedGetForcedDecisionWithRuleKey() { userId, Collections.emptyMap()); - optimizelyUserContext.setForcedDecision(flagKey, ruleKey, variationKey); - assertNull(optimizelyUserContext.getForcedDecision(invalidFlagKey, ruleKey)); + OptimizelyDecisionContext optimizelyDecisionContext = new OptimizelyDecisionContext(flagKey, ruleKey); + OptimizelyDecisionContext invalidOptimizelyDecisionContext = new OptimizelyDecisionContext(invalidFlagKey, ruleKey); + OptimizelyForcedDecision optimizelyForcedDecision = new OptimizelyForcedDecision(variationKey); + + optimizelyUserContext.setForcedDecision(optimizelyDecisionContext, optimizelyForcedDecision); + assertNull(optimizelyUserContext.getForcedDecision(invalidOptimizelyDecisionContext)); } @Test @@ -1298,8 +1316,11 @@ public void getForcedVariationWithoutRuleKey() { userId, Collections.emptyMap()); - optimizelyUserContext.setForcedDecision(flagKey, variationKey); - assertEquals(variationKey, optimizelyUserContext.getForcedDecision(flagKey)); + OptimizelyDecisionContext optimizelyDecisionContext = new OptimizelyDecisionContext(flagKey, null); + OptimizelyForcedDecision optimizelyForcedDecision = new OptimizelyForcedDecision(variationKey); + + optimizelyUserContext.setForcedDecision(optimizelyDecisionContext, optimizelyForcedDecision); + assertEquals(variationKey, optimizelyUserContext.getForcedDecision(optimizelyDecisionContext).getVariationKey()); } @Test @@ -1312,8 +1333,11 @@ public void getForcedVariationWithoutRuleKeySdkNotReady() { userId, Collections.emptyMap()); - optimizelyUserContext.setForcedDecision(flagKey, variationKey); - assertNull(optimizelyUserContext.getForcedDecision(flagKey)); + OptimizelyDecisionContext optimizelyDecisionContext = new OptimizelyDecisionContext(flagKey, null); + OptimizelyForcedDecision optimizelyForcedDecision = new OptimizelyForcedDecision(variationKey); + + optimizelyUserContext.setForcedDecision(optimizelyDecisionContext, optimizelyForcedDecision); + assertNull(optimizelyUserContext.getForcedDecision(optimizelyDecisionContext)); } @Test @@ -1326,8 +1350,12 @@ public void failedGetForcedDecisionWithoutRuleKey() { userId, Collections.emptyMap()); - optimizelyUserContext.setForcedDecision(flagKey, variationKey); - assertNull(optimizelyUserContext.getForcedDecision(invalidFlagKey)); + OptimizelyDecisionContext optimizelyDecisionContext = new OptimizelyDecisionContext(flagKey, null); + OptimizelyDecisionContext invalidOptimizelyDecisionContext = new OptimizelyDecisionContext(invalidFlagKey, null); + OptimizelyForcedDecision optimizelyForcedDecision = new OptimizelyForcedDecision(variationKey); + + optimizelyUserContext.setForcedDecision(optimizelyDecisionContext, optimizelyForcedDecision); + assertNull(optimizelyUserContext.getForcedDecision(invalidOptimizelyDecisionContext)); } @Test @@ -1340,8 +1368,11 @@ public void removeForcedDecisionWithRuleKey() { userId, Collections.emptyMap()); - optimizelyUserContext.setForcedDecision(flagKey, ruleKey, variationKey); - assertTrue(optimizelyUserContext.removeForcedDecision(flagKey, ruleKey)); + OptimizelyDecisionContext optimizelyDecisionContext = new OptimizelyDecisionContext(flagKey, ruleKey); + OptimizelyForcedDecision optimizelyForcedDecision = new OptimizelyForcedDecision(variationKey); + + optimizelyUserContext.setForcedDecision(optimizelyDecisionContext, optimizelyForcedDecision); + assertTrue(optimizelyUserContext.removeForcedDecision(optimizelyDecisionContext)); } @Test @@ -1353,8 +1384,11 @@ public void removeForcedDecisionWithoutRuleKey() { userId, Collections.emptyMap()); - optimizelyUserContext.setForcedDecision(flagKey, variationKey); - assertTrue(optimizelyUserContext.removeForcedDecision(flagKey)); + OptimizelyDecisionContext optimizelyDecisionContext = new OptimizelyDecisionContext(flagKey, null); + OptimizelyForcedDecision optimizelyForcedDecision = new OptimizelyForcedDecision(variationKey); + + optimizelyUserContext.setForcedDecision(optimizelyDecisionContext, optimizelyForcedDecision); + assertTrue(optimizelyUserContext.removeForcedDecision(optimizelyDecisionContext)); } @Test @@ -1367,8 +1401,12 @@ public void removeForcedDecisionWithNullRuleKeyAfterAddingWithRuleKey() { userId, Collections.emptyMap()); - optimizelyUserContext.setForcedDecision(flagKey, ruleKey, variationKey); - assertFalse(optimizelyUserContext.removeForcedDecision(flagKey)); + OptimizelyDecisionContext optimizelyDecisionContext = new OptimizelyDecisionContext(flagKey, null); + OptimizelyDecisionContext optimizelyDecisionContextNonNull = new OptimizelyDecisionContext(flagKey, ruleKey); + OptimizelyForcedDecision optimizelyForcedDecision = new OptimizelyForcedDecision(variationKey); + + optimizelyUserContext.setForcedDecision(optimizelyDecisionContext, optimizelyForcedDecision); + assertFalse(optimizelyUserContext.removeForcedDecision(optimizelyDecisionContextNonNull)); } @Test @@ -1381,42 +1419,45 @@ public void removeForcedDecisionWithoutRuleKeySdkNotReady() { userId, Collections.emptyMap()); - optimizelyUserContext.setForcedDecision(flagKey, variationKey); - assertFalse(optimizelyUserContext.removeForcedDecision(flagKey)); + OptimizelyDecisionContext optimizelyDecisionContext = new OptimizelyDecisionContext(flagKey, null); + OptimizelyForcedDecision optimizelyForcedDecision = new OptimizelyForcedDecision(variationKey); + + optimizelyUserContext.setForcedDecision(optimizelyDecisionContext, optimizelyForcedDecision); + assertFalse(optimizelyUserContext.removeForcedDecision(optimizelyDecisionContext)); } @Test public void removeAllForcedDecisions() { - String flagKey1 = "55555"; - String ruleKey1 = "77777"; - String variationKey1 = "33333"; - String flagKey2 = "11"; - String variationKey2 = "5"; + String flagKey = "55555"; + String ruleKey = "77777"; + String variationKey = "33333"; OptimizelyUserContext optimizelyUserContext = new OptimizelyUserContext( optimizely, userId, Collections.emptyMap()); - optimizelyUserContext.setForcedDecision(flagKey1, ruleKey1, variationKey1); - optimizelyUserContext.setForcedDecision(flagKey2, variationKey2); + OptimizelyDecisionContext optimizelyDecisionContext = new OptimizelyDecisionContext(flagKey, ruleKey); + OptimizelyForcedDecision optimizelyForcedDecision = new OptimizelyForcedDecision(variationKey); + + optimizelyUserContext.setForcedDecision(optimizelyDecisionContext, optimizelyForcedDecision); assertTrue(optimizelyUserContext.removeAllForcedDecisions()); } @Test public void removeAllForcedDecisionsSdkNotReady() { - String flagKey1 = "55555"; - String ruleKey1 = "77777"; - String variationKey1 = "33333"; - String flagKey2 = "11"; - String variationKey2 = "5"; + String flagKey = "55555"; + String ruleKey = "77777"; + String variationKey = "33333"; Optimizely optimizely = new Optimizely.Builder().build(); OptimizelyUserContext optimizelyUserContext = new OptimizelyUserContext( optimizely, userId, Collections.emptyMap()); - optimizelyUserContext.setForcedDecision(flagKey1, ruleKey1, variationKey1); - optimizelyUserContext.setForcedDecision(flagKey2, variationKey2); + OptimizelyDecisionContext optimizelyDecisionContext = new OptimizelyDecisionContext(flagKey, ruleKey); + OptimizelyForcedDecision optimizelyForcedDecision = new OptimizelyForcedDecision(variationKey); + + optimizelyUserContext.setForcedDecision(optimizelyDecisionContext, optimizelyForcedDecision); assertFalse(optimizelyUserContext.removeAllForcedDecisions()); } @@ -1430,8 +1471,11 @@ public void findValidatedForcedDecisionWithRuleKey() { userId, Collections.emptyMap()); - optimizelyUserContext.setForcedDecision(flagKey, ruleKey, variationKey); - DecisionResponse response = optimizelyUserContext.findValidatedForcedDecision(flagKey, ruleKey); + OptimizelyDecisionContext optimizelyDecisionContext = new OptimizelyDecisionContext(flagKey, ruleKey); + OptimizelyForcedDecision optimizelyForcedDecision = new OptimizelyForcedDecision(variationKey); + + optimizelyUserContext.setForcedDecision(optimizelyDecisionContext, optimizelyForcedDecision); + DecisionResponse response = optimizelyUserContext.findValidatedForcedDecision(optimizelyDecisionContext); Variation variation = response.getResult(); assertEquals(variationKey, variation.getKey()); } @@ -1445,8 +1489,11 @@ public void findValidatedForcedDecisionWithoutRuleKey() { userId, Collections.emptyMap()); - optimizelyUserContext.setForcedDecision(flagKey, variationKey); - DecisionResponse response = optimizelyUserContext.findValidatedForcedDecision(flagKey); + OptimizelyDecisionContext optimizelyDecisionContext = new OptimizelyDecisionContext(flagKey, null); + OptimizelyForcedDecision optimizelyForcedDecision = new OptimizelyForcedDecision(variationKey); + + optimizelyUserContext.setForcedDecision(optimizelyDecisionContext, optimizelyForcedDecision); + DecisionResponse response = optimizelyUserContext.findValidatedForcedDecision(optimizelyDecisionContext); Variation variation = response.getResult(); assertEquals(variationKey, variation.getKey()); } diff --git a/core-api/src/test/java/com/optimizely/ab/bucketing/DecisionServiceTest.java b/core-api/src/test/java/com/optimizely/ab/bucketing/DecisionServiceTest.java index c1235e809..a63270bfa 100644 --- a/core-api/src/test/java/com/optimizely/ab/bucketing/DecisionServiceTest.java +++ b/core-api/src/test/java/com/optimizely/ab/bucketing/DecisionServiceTest.java @@ -24,6 +24,7 @@ import com.optimizely.ab.internal.LogbackVerifier; import com.optimizely.ab.optimizelydecision.DecisionResponse; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; +import javafx.util.Pair; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -726,7 +727,7 @@ public void getVariationFromDeliveryRuleTest() { expectedVariation = variation; } } - DecisionResponse decisionResponse = decisionService.getVariationFromDeliveryRule( + DecisionResponse decisionResponse = decisionService.getVariationFromDeliveryRule( v4ProjectConfig, FEATURE_FLAG_MULTI_VARIATE_FEATURE.getKey(), rules, @@ -734,8 +735,8 @@ public void getVariationFromDeliveryRuleTest() { optimizely.createUserContext(genericUserId, Collections.singletonMap(ATTRIBUTE_NATIONALITY_KEY, AUDIENCE_ENGLISH_CITIZENS_VALUE)) ); - Variation variation = (Variation) decisionResponse.getResult().keySet().toArray()[0]; - Boolean skipToEveryoneElse = (Boolean) decisionResponse.getResult().get(variation); + Variation variation = (Variation) decisionResponse.getResult().getKey(); + Boolean skipToEveryoneElse = (Boolean) decisionResponse.getResult().getValue(); assertNotNull(decisionResponse.getResult()); assertNotNull(variation); assertNotNull(expectedVariation); From d1943f6219b81919382ae1522d1d594b31f635ae Mon Sep 17 00:00:00 2001 From: Jake Brown Date: Fri, 22 Oct 2021 14:10:08 -0400 Subject: [PATCH 28/39] Update to fix issue with java PAIR to test. --- .../java/com/optimizely/ab/bucketing/DecisionService.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java b/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java index 12b5659fa..a17edfbc2 100644 --- a/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java +++ b/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java @@ -16,7 +16,6 @@ package com.optimizely.ab.bucketing; import com.optimizely.ab.OptimizelyDecisionContext; -import com.optimizely.ab.OptimizelyForcedDecision; import com.optimizely.ab.OptimizelyRuntimeException; import com.optimizely.ab.OptimizelyUserContext; import com.optimizely.ab.config.*; @@ -27,7 +26,6 @@ import com.optimizely.ab.optimizelydecision.DecisionResponse; import com.optimizely.ab.optimizelydecision.DefaultDecisionReasons; import com.optimizely.ab.optimizelydecision.OptimizelyDecideOption; -import com.sun.org.apache.xpath.internal.operations.Bool; import javafx.util.Pair; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -596,8 +594,9 @@ public DecisionResponse getVariationFromExperimentRule(@Nonnull Proje DecisionReasons reasons = DefaultDecisionReasons.newInstance(); String ruleKey = rule != null ? rule.getKey() : null; - OptimizelyDecisionContext optimizelyDecisionContext = new OptimizelyDecisionContext(flagKey, ruleKey); // Check Forced-Decision + + OptimizelyDecisionContext optimizelyDecisionContext = new OptimizelyDecisionContext(flagKey, ruleKey); DecisionResponse forcedDecisionResponse = user.findValidatedForcedDecision(optimizelyDecisionContext); reasons.merge(forcedDecisionResponse.getReasons()); From d0aced2a464eba3c0d938187b7ced1a634e68a83 Mon Sep 17 00:00:00 2001 From: Jake Brown Date: Fri, 22 Oct 2021 14:30:45 -0400 Subject: [PATCH 29/39] Revert to Map for now. --- .../ab/bucketing/DecisionService.java | 19 ++++++++----------- .../ab/bucketing/DecisionServiceTest.java | 6 +++--- 2 files changed, 11 insertions(+), 14 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java b/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java index a17edfbc2..4337f335a 100644 --- a/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java +++ b/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java @@ -26,10 +26,8 @@ import com.optimizely.ab.optimizelydecision.DecisionResponse; import com.optimizely.ab.optimizelydecision.DefaultDecisionReasons; import com.optimizely.ab.optimizelydecision.OptimizelyDecideOption; -import javafx.util.Pair; import org.slf4j.Logger; import org.slf4j.LoggerFactory; - import javax.annotation.Nonnull; import javax.annotation.Nullable; import java.util.Collections; @@ -313,7 +311,7 @@ DecisionResponse getVariationForFeatureInRollout(@Nonnull Featu int index = 0; while (index < rolloutRulesLength) { - DecisionResponse decisionVariationResponse = getVariationFromDeliveryRule( + DecisionResponse decisionVariationResponse = getVariationFromDeliveryRule( projectConfig, featureFlag.getKey(), rollout.getExperiments(), @@ -322,9 +320,9 @@ DecisionResponse getVariationForFeatureInRollout(@Nonnull Featu ); reasons.merge(decisionVariationResponse.getReasons()); - Pair response = decisionVariationResponse.getResult(); - Variation variation = response.getKey(); - Boolean skipToEveryoneElse = response.getValue(); + Map response = decisionVariationResponse.getResult(); + Variation variation = (Variation)response.keySet().toArray()[0]; + Boolean skipToEveryoneElse = response.get(variation); if (variation != null) { Experiment rule = rollout.getExperiments().get(index); FeatureDecision featureDecision = new FeatureDecision(rule, variation, FeatureDecision.DecisionSource.ROLLOUT); @@ -595,7 +593,6 @@ public DecisionResponse getVariationFromExperimentRule(@Nonnull Proje String ruleKey = rule != null ? rule.getKey() : null; // Check Forced-Decision - OptimizelyDecisionContext optimizelyDecisionContext = new OptimizelyDecisionContext(flagKey, ruleKey); DecisionResponse forcedDecisionResponse = user.findValidatedForcedDecision(optimizelyDecisionContext); @@ -634,14 +631,14 @@ private boolean validateUserId(String userId) { * @return Returns a DecisionResponse Object containing a Pair * where the Variation is the result and the Boolean is the skipToEveryoneElse. */ - DecisionResponse getVariationFromDeliveryRule(@Nonnull ProjectConfig projectConfig, + DecisionResponse getVariationFromDeliveryRule(@Nonnull ProjectConfig projectConfig, @Nonnull String flagKey, @Nonnull List rules, @Nonnull int ruleIndex, @Nonnull OptimizelyUserContext user) { DecisionReasons reasons = DefaultDecisionReasons.newInstance(); Boolean skipToEveryoneElse = false; - Pair variationToSkipToEveryoneElse; + Map variationToSkipToEveryoneElse = new HashMap<>();; // Check forced-decisions first Experiment rule = rules.get(ruleIndex); @@ -651,7 +648,7 @@ DecisionResponse getVariationFromDeliveryRule(@Nonnull ProjectConfig proje Variation variation = forcedDecisionResponse.getResult(); if (variation != null) { - variationToSkipToEveryoneElse = new Pair<>(variation, false); + variationToSkipToEveryoneElse.put(variation, false); return new DecisionResponse(variationToSkipToEveryoneElse, reasons); } @@ -697,7 +694,7 @@ DecisionResponse getVariationFromDeliveryRule(@Nonnull ProjectConfig proje reasons.addInfo(message); logger.debug(message); } - variationToSkipToEveryoneElse = new Pair<>(bucketedVariation, skipToEveryoneElse); + variationToSkipToEveryoneElse.put(bucketedVariation, skipToEveryoneElse); return new DecisionResponse(variationToSkipToEveryoneElse, reasons); } diff --git a/core-api/src/test/java/com/optimizely/ab/bucketing/DecisionServiceTest.java b/core-api/src/test/java/com/optimizely/ab/bucketing/DecisionServiceTest.java index a63270bfa..abcacbafd 100644 --- a/core-api/src/test/java/com/optimizely/ab/bucketing/DecisionServiceTest.java +++ b/core-api/src/test/java/com/optimizely/ab/bucketing/DecisionServiceTest.java @@ -727,7 +727,7 @@ public void getVariationFromDeliveryRuleTest() { expectedVariation = variation; } } - DecisionResponse decisionResponse = decisionService.getVariationFromDeliveryRule( + DecisionResponse decisionResponse = decisionService.getVariationFromDeliveryRule( v4ProjectConfig, FEATURE_FLAG_MULTI_VARIATE_FEATURE.getKey(), rules, @@ -735,8 +735,8 @@ public void getVariationFromDeliveryRuleTest() { optimizely.createUserContext(genericUserId, Collections.singletonMap(ATTRIBUTE_NATIONALITY_KEY, AUDIENCE_ENGLISH_CITIZENS_VALUE)) ); - Variation variation = (Variation) decisionResponse.getResult().getKey(); - Boolean skipToEveryoneElse = (Boolean) decisionResponse.getResult().getValue(); + Variation variation = (Variation) decisionResponse.getResult().values().toArray()[0]; + Boolean skipToEveryoneElse = (Boolean) decisionResponse.getResult().get(variation); assertNotNull(decisionResponse.getResult()); assertNotNull(variation); assertNotNull(expectedVariation); From 8bf6faffc0be8af86d96d63b5349d307f080cce3 Mon Sep 17 00:00:00 2001 From: Jake Brown Date: Fri, 22 Oct 2021 14:49:38 -0400 Subject: [PATCH 30/39] Update OptimizelyUserContext to copy FD, update DecisionService to user SimpleEntry from AbstractMap instead of Pair since no longer supported in Java 11 --- .../optimizely/ab/OptimizelyUserContext.java | 16 +++++++++++- .../ab/bucketing/DecisionService.java | 26 ++++++++----------- .../ab/bucketing/DecisionServiceTest.java | 11 +++----- 3 files changed, 30 insertions(+), 23 deletions(-) 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 f8ed5862a..497887923 100644 --- a/core-api/src/main/java/com/optimizely/ab/OptimizelyUserContext.java +++ b/core-api/src/main/java/com/optimizely/ab/OptimizelyUserContext.java @@ -53,6 +53,20 @@ public OptimizelyUserContext(@Nonnull Optimizely optimizely, } } + public OptimizelyUserContext(@Nonnull Optimizely optimizely, + @Nonnull String userId, + @Nonnull Map attributes, + @Nullable Map forcedDecisionsMap) { + this.optimizely = optimizely; + this.userId = userId; + if (attributes != null) { + this.attributes = Collections.synchronizedMap(new HashMap<>(attributes)); + } else { + this.attributes = Collections.synchronizedMap(new HashMap<>()); + } + this.forcedDecisionsMap = forcedDecisionsMap; + } + public OptimizelyUserContext(@Nonnull Optimizely optimizely, @Nonnull String userId) { this(optimizely, userId, Collections.EMPTY_MAP); } @@ -70,7 +84,7 @@ public Optimizely getOptimizely() { } public OptimizelyUserContext copy() { - return new OptimizelyUserContext(optimizely, userId, attributes); + return new OptimizelyUserContext(optimizely, userId, attributes, forcedDecisionsMap); } /** diff --git a/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java b/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java index 4337f335a..3d2285ec6 100644 --- a/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java +++ b/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java @@ -30,10 +30,7 @@ import org.slf4j.LoggerFactory; import javax.annotation.Nonnull; import javax.annotation.Nullable; -import java.util.Collections; -import java.util.HashMap; -import java.util.List; -import java.util.Map; +import java.util.*; import java.util.concurrent.ConcurrentHashMap; import static com.optimizely.ab.internal.LoggingConstants.LoggingEntityType.EXPERIMENT; @@ -311,7 +308,7 @@ DecisionResponse getVariationForFeatureInRollout(@Nonnull Featu int index = 0; while (index < rolloutRulesLength) { - DecisionResponse decisionVariationResponse = getVariationFromDeliveryRule( + DecisionResponse decisionVariationResponse = getVariationFromDeliveryRule( projectConfig, featureFlag.getKey(), rollout.getExperiments(), @@ -320,9 +317,9 @@ DecisionResponse getVariationForFeatureInRollout(@Nonnull Featu ); reasons.merge(decisionVariationResponse.getReasons()); - Map response = decisionVariationResponse.getResult(); - Variation variation = (Variation)response.keySet().toArray()[0]; - Boolean skipToEveryoneElse = response.get(variation); + AbstractMap.SimpleEntry response = decisionVariationResponse.getResult(); + Variation variation = response.getKey(); + Boolean skipToEveryoneElse = response.getValue(); if (variation != null) { Experiment rule = rollout.getExperiments().get(index); FeatureDecision featureDecision = new FeatureDecision(rule, variation, FeatureDecision.DecisionSource.ROLLOUT); @@ -631,15 +628,14 @@ private boolean validateUserId(String userId) { * @return Returns a DecisionResponse Object containing a Pair * where the Variation is the result and the Boolean is the skipToEveryoneElse. */ - DecisionResponse getVariationFromDeliveryRule(@Nonnull ProjectConfig projectConfig, + DecisionResponse getVariationFromDeliveryRule(@Nonnull ProjectConfig projectConfig, @Nonnull String flagKey, @Nonnull List rules, @Nonnull int ruleIndex, @Nonnull OptimizelyUserContext user) { DecisionReasons reasons = DefaultDecisionReasons.newInstance(); Boolean skipToEveryoneElse = false; - Map variationToSkipToEveryoneElse = new HashMap<>();; - + AbstractMap.SimpleEntry variationToSkipToEveryoneElsePair; // Check forced-decisions first Experiment rule = rules.get(ruleIndex); OptimizelyDecisionContext optimizelyDecisionContext = new OptimizelyDecisionContext(flagKey, rule.getKey()); @@ -648,8 +644,8 @@ DecisionResponse getVariationFromDeliveryRule(@Nonnull ProjectConfig projec Variation variation = forcedDecisionResponse.getResult(); if (variation != null) { - variationToSkipToEveryoneElse.put(variation, false); - return new DecisionResponse(variationToSkipToEveryoneElse, reasons); + variationToSkipToEveryoneElsePair = new AbstractMap.SimpleEntry<>(variation, false); + return new DecisionResponse(variationToSkipToEveryoneElsePair, reasons); } // Handle a regular decision @@ -694,8 +690,8 @@ DecisionResponse getVariationFromDeliveryRule(@Nonnull ProjectConfig projec reasons.addInfo(message); logger.debug(message); } - variationToSkipToEveryoneElse.put(bucketedVariation, skipToEveryoneElse); - return new DecisionResponse(variationToSkipToEveryoneElse, reasons); + variationToSkipToEveryoneElsePair = new AbstractMap.SimpleEntry<>(bucketedVariation, skipToEveryoneElse); + return new DecisionResponse(variationToSkipToEveryoneElsePair, reasons); } } diff --git a/core-api/src/test/java/com/optimizely/ab/bucketing/DecisionServiceTest.java b/core-api/src/test/java/com/optimizely/ab/bucketing/DecisionServiceTest.java index abcacbafd..05962a808 100644 --- a/core-api/src/test/java/com/optimizely/ab/bucketing/DecisionServiceTest.java +++ b/core-api/src/test/java/com/optimizely/ab/bucketing/DecisionServiceTest.java @@ -32,10 +32,7 @@ import org.mockito.junit.MockitoJUnit; import org.mockito.junit.MockitoRule; -import java.util.Collections; -import java.util.HashMap; -import java.util.List; -import java.util.Map; +import java.util.*; import static com.optimizely.ab.config.DatafileProjectConfigTestUtils.*; import static com.optimizely.ab.config.ValidProjectConfigV4.*; @@ -727,7 +724,7 @@ public void getVariationFromDeliveryRuleTest() { expectedVariation = variation; } } - DecisionResponse decisionResponse = decisionService.getVariationFromDeliveryRule( + DecisionResponse decisionResponse = decisionService.getVariationFromDeliveryRule( v4ProjectConfig, FEATURE_FLAG_MULTI_VARIATE_FEATURE.getKey(), rules, @@ -735,8 +732,8 @@ public void getVariationFromDeliveryRuleTest() { optimizely.createUserContext(genericUserId, Collections.singletonMap(ATTRIBUTE_NATIONALITY_KEY, AUDIENCE_ENGLISH_CITIZENS_VALUE)) ); - Variation variation = (Variation) decisionResponse.getResult().values().toArray()[0]; - Boolean skipToEveryoneElse = (Boolean) decisionResponse.getResult().get(variation); + Variation variation = (Variation) decisionResponse.getResult().getKey(); + Boolean skipToEveryoneElse = (Boolean) decisionResponse.getResult().getValue(); assertNotNull(decisionResponse.getResult()); assertNotNull(variation); assertNotNull(expectedVariation); From 66cb22fad20d39726eeba2eec8bd18855dfbffba Mon Sep 17 00:00:00 2001 From: Jake Brown Date: Fri, 22 Oct 2021 15:07:20 -0400 Subject: [PATCH 31/39] Updated tests and license headers --- .../ab/OptimizelyDecisionContext.java | 16 +++++++ .../ab/OptimizelyForcedDecision.java | 16 +++++++ .../ab/OptimizelyDecisionContextTest.java | 44 +++++++++++++++++++ .../ab/OptimizelyForcedDecisionTest.java | 30 +++++++++++++ .../ab/bucketing/DecisionServiceTest.java | 1 - 5 files changed, 106 insertions(+), 1 deletion(-) create mode 100644 core-api/src/test/java/com/optimizely/ab/OptimizelyDecisionContextTest.java create mode 100644 core-api/src/test/java/com/optimizely/ab/OptimizelyForcedDecisionTest.java diff --git a/core-api/src/main/java/com/optimizely/ab/OptimizelyDecisionContext.java b/core-api/src/main/java/com/optimizely/ab/OptimizelyDecisionContext.java index f92da3747..c8f819c9b 100644 --- a/core-api/src/main/java/com/optimizely/ab/OptimizelyDecisionContext.java +++ b/core-api/src/main/java/com/optimizely/ab/OptimizelyDecisionContext.java @@ -1,3 +1,19 @@ +/** + * + * Copyright 2021, Optimizely and contributors + * + * 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; import javax.annotation.Nonnull; diff --git a/core-api/src/main/java/com/optimizely/ab/OptimizelyForcedDecision.java b/core-api/src/main/java/com/optimizely/ab/OptimizelyForcedDecision.java index d3cb2ea01..d73a86c83 100644 --- a/core-api/src/main/java/com/optimizely/ab/OptimizelyForcedDecision.java +++ b/core-api/src/main/java/com/optimizely/ab/OptimizelyForcedDecision.java @@ -1,3 +1,19 @@ +/** + * + * Copyright 2021, Optimizely and contributors + * + * 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; import javax.annotation.Nonnull; diff --git a/core-api/src/test/java/com/optimizely/ab/OptimizelyDecisionContextTest.java b/core-api/src/test/java/com/optimizely/ab/OptimizelyDecisionContextTest.java new file mode 100644 index 000000000..4b0e95fbc --- /dev/null +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyDecisionContextTest.java @@ -0,0 +1,44 @@ +/** + * + * Copyright 2021, Optimizely and contributors + * + * 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; + +import org.junit.Test; +import static junit.framework.TestCase.assertEquals; + +public class OptimizelyDecisionContextTest { + + @Test + public void initializeOptimizelyDecisionContextWithFlagKeyAndRuleKey() { + String flagKey = "test-flag-key"; + String ruleKey = "1029384756"; + String expectedKey = flagKey + ruleKey; + OptimizelyDecisionContext optimizelyDecisionContext = new OptimizelyDecisionContext(flagKey, ruleKey); + assertEquals(flagKey, optimizelyDecisionContext.getFlagKey()); + assertEquals(ruleKey, optimizelyDecisionContext.getRuleKey()); + assertEquals(expectedKey, optimizelyDecisionContext.getKey()); + } + + @Test + public void initializeOptimizelyDecisionContextWithFlagKey() { + String flagKey = "test-flag-key"; + String expectedKey = flagKey + OptimizelyDecisionContext.OPTI_NULL_RULE_KEY; + OptimizelyDecisionContext optimizelyDecisionContext = new OptimizelyDecisionContext(flagKey, null); + assertEquals(flagKey, optimizelyDecisionContext.getFlagKey()); + assertEquals(OptimizelyDecisionContext.OPTI_NULL_RULE_KEY, optimizelyDecisionContext.getRuleKey()); + assertEquals(expectedKey, optimizelyDecisionContext.getKey()); + } +} diff --git a/core-api/src/test/java/com/optimizely/ab/OptimizelyForcedDecisionTest.java b/core-api/src/test/java/com/optimizely/ab/OptimizelyForcedDecisionTest.java new file mode 100644 index 000000000..90c0f9e50 --- /dev/null +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyForcedDecisionTest.java @@ -0,0 +1,30 @@ +/** + * + * Copyright 2021, Optimizely and contributors + * + * 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; + +import org.junit.Test; +import static junit.framework.TestCase.assertEquals; + +public class OptimizelyForcedDecisionTest { + + @Test + public void initializeOptimizelyForcedDecision() { + String variationKey = "test-variation-key"; + OptimizelyForcedDecision optimizelyForcedDecision = new OptimizelyForcedDecision(variationKey); + assertEquals(variationKey, optimizelyForcedDecision.getVariationKey()); + } +} diff --git a/core-api/src/test/java/com/optimizely/ab/bucketing/DecisionServiceTest.java b/core-api/src/test/java/com/optimizely/ab/bucketing/DecisionServiceTest.java index 05962a808..eddbf0178 100644 --- a/core-api/src/test/java/com/optimizely/ab/bucketing/DecisionServiceTest.java +++ b/core-api/src/test/java/com/optimizely/ab/bucketing/DecisionServiceTest.java @@ -24,7 +24,6 @@ import com.optimizely.ab.internal.LogbackVerifier; import com.optimizely.ab.optimizelydecision.DecisionResponse; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; -import javafx.util.Pair; import org.junit.Before; import org.junit.Rule; import org.junit.Test; From 743f9131d5d9a0db926ec91a199014531db94a60 Mon Sep 17 00:00:00 2001 From: Jake Brown Date: Fri, 22 Oct 2021 15:36:20 -0400 Subject: [PATCH 32/39] Added test case to confirm no duplicate variations added to the falgs variations map per flag and changed implementation to use a Map using variation.id as the key to ensure. --- .../ab/config/DatafileProjectConfig.java | 10 ++++---- .../ab/config/DatafileProjectConfigTest.java | 23 ++++++++++++++++--- 2 files changed, 24 insertions(+), 9 deletions(-) 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 8ef8cca79..831563826 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 @@ -31,9 +31,7 @@ import javax.annotation.CheckForNull; import javax.annotation.Nonnull; import javax.annotation.Nullable; -import javax.annotation.concurrent.Immutable; import java.util.*; -import java.util.concurrent.ConcurrentHashMap; /** * DatafileProjectConfig is an implementation of ProjectConfig that is backed by a @@ -216,16 +214,16 @@ public DatafileProjectConfig(String accountId, flagVariationsMap = new HashMap<>(); if (featureFlags != null) { for (FeatureFlag flag : featureFlags) { - List variations = new ArrayList<>(); + Map variationIdToVariationsMap = new HashMap<>(); for (Experiment rule : getAllRulesForFlag(flag)) { for (Variation variation : rule.getVariations()) { - if (!variations.contains(variation)) { - variations.add(variation); + if(!variationIdToVariationsMap.containsKey(variation.getId())) { + variationIdToVariationsMap.put(variation.getId(), variation); } } } // Grab all the variations from the flag experiments and rollouts and add to flagVariationsMap - flagVariationsMap.put(flag.getKey(), variations); + flagVariationsMap.put(flag.getKey(), new ArrayList<>(variationIdToVariationsMap.values())); } } } diff --git a/core-api/src/test/java/com/optimizely/ab/config/DatafileProjectConfigTest.java b/core-api/src/test/java/com/optimizely/ab/config/DatafileProjectConfigTest.java index cab4face3..41b02ea91 100644 --- a/core-api/src/test/java/com/optimizely/ab/config/DatafileProjectConfigTest.java +++ b/core-api/src/test/java/com/optimizely/ab/config/DatafileProjectConfigTest.java @@ -17,15 +17,14 @@ package com.optimizely.ab.config; import ch.qos.logback.classic.Level; +import com.google.errorprone.annotations.Var; import com.optimizely.ab.config.audience.AndCondition; import com.optimizely.ab.config.audience.Condition; import com.optimizely.ab.config.audience.NotCondition; import com.optimizely.ab.config.audience.OrCondition; import com.optimizely.ab.config.audience.UserAttribute; -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; +import java.util.*; import static java.util.Arrays.asList; import static org.hamcrest.CoreMatchers.is; @@ -170,4 +169,22 @@ public void getAttributeIDWhenAttributeKeyPrefixIsMatched() { " has reserved prefix $opt_; using attribute ID instead of reserved attribute name."); } + @Test + public void confirmUniqueVariationsInFlagVariationsMapTest() { + // Test to confirm no duplicate variations are added for each flag + // This should never happen as a Map is used for each flag based on variation ID as the key + Map> flagVariationsMap = projectConfig.getFlagVariationsMap(); + for (List variationsList : flagVariationsMap.values()) { + Boolean duplicate = false; + Map variationIdToVariationsMap = new HashMap<>(); + for (Variation variation : variationsList) { + if (variationIdToVariationsMap.containsKey(variation.getId())) { + duplicate = true; + } + variationIdToVariationsMap.put(variation.getId(), variation); + } + assertFalse(duplicate); + } + } + } From 80b7fb0f4c85e832366201fd452bbeacbc750e5e Mon Sep 17 00:00:00 2001 From: Jake Brown Date: Fri, 22 Oct 2021 16:16:56 -0400 Subject: [PATCH 33/39] Update tests to cover decide with FD and confirm usage. --- .../src/main/java/com/optimizely/ab/Optimizely.java | 13 ------------- .../optimizely/ab/bucketing/DecisionService.java | 2 +- .../optimizely/ab/OptimizelyUserContextTest.java | 13 ++++++++++--- 3 files changed, 11 insertions(+), 17 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 347204802..c53095692 100644 --- a/core-api/src/main/java/com/optimizely/ab/Optimizely.java +++ b/core-api/src/main/java/com/optimizely/ab/Optimizely.java @@ -372,7 +372,6 @@ public void track(@Nonnull String eventName, * @return True if the feature is enabled. * False if the feature is disabled. * False if the feature is not found. - * */ @Nonnull public Boolean isFeatureEnabled(@Nonnull String featureKey, @@ -390,7 +389,6 @@ public Boolean isFeatureEnabled(@Nonnull String featureKey, * @return True if the feature is enabled. * False if the feature is disabled. * False if the feature is not found. - * */ @Nonnull public Boolean isFeatureEnabled(@Nonnull String featureKey, @@ -496,9 +494,6 @@ public Boolean getFeatureVariableBoolean(@Nonnull String featureKey, * @param attributes The user's attributes. * @return The Boolean value of the boolean single variable feature. * Null if the feature or variable could not be found. - * - * - * */ @Nullable public Boolean getFeatureVariableBoolean(@Nonnull String featureKey, @@ -523,7 +518,6 @@ public Boolean getFeatureVariableBoolean(@Nonnull String featureKey, * @param userId The ID of the user. * @return The Double value of the double single variable feature. * Null if the feature or variable could not be found. - * */ @Nullable public Double getFeatureVariableDouble(@Nonnull String featureKey, @@ -541,7 +535,6 @@ public Double getFeatureVariableDouble(@Nonnull String featureKey, * @param attributes The user's attributes. * @return The Double value of the double single variable feature. * Null if the feature or variable could not be found. - * */ @Nullable public Double getFeatureVariableDouble(@Nonnull String featureKey, @@ -574,7 +567,6 @@ public Double getFeatureVariableDouble(@Nonnull String featureKey, * @param userId The ID of the user. * @return The Integer value of the integer single variable feature. * Null if the feature or variable could not be found. - * */ @Nullable public Integer getFeatureVariableInteger(@Nonnull String featureKey, @@ -592,7 +584,6 @@ public Integer getFeatureVariableInteger(@Nonnull String featureKey, * @param attributes The user's attributes. * @return The Integer value of the integer single variable feature. * Null if the feature or variable could not be found. - * */ @Nullable public Integer getFeatureVariableInteger(@Nonnull String featureKey, @@ -626,7 +617,6 @@ public Integer getFeatureVariableInteger(@Nonnull String featureKey, * @param userId The ID of the user. * @return The String value of the string single variable feature. * Null if the feature or variable could not be found. - * */ @Nullable public String getFeatureVariableString(@Nonnull String featureKey, @@ -644,7 +634,6 @@ public String getFeatureVariableString(@Nonnull String featureKey, * @param attributes The user's attributes. * @return The String value of the string single variable feature. * Null if the feature or variable could not be found. - * */ @Nullable public String getFeatureVariableString(@Nonnull String featureKey, @@ -668,7 +657,6 @@ public String getFeatureVariableString(@Nonnull String featureKey, * @param userId The ID of the user. * @return An OptimizelyJSON instance for the JSON variable value. * Null if the feature or variable could not be found. - * */ @Nullable public OptimizelyJSON getFeatureVariableJSON(@Nonnull String featureKey, @@ -686,7 +674,6 @@ public OptimizelyJSON getFeatureVariableJSON(@Nonnull String featureKey, * @param attributes The user's attributes. * @return An OptimizelyJSON instance for the JSON variable value. * Null if the feature or variable could not be found. - * */ @Nullable public OptimizelyJSON getFeatureVariableJSON(@Nonnull String featureKey, diff --git a/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java b/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java index 3d2285ec6..d295d19cb 100644 --- a/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java +++ b/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java @@ -625,7 +625,7 @@ private boolean validateUserId(String userId) { * @param rules The experiments belonging to a rollout * @param ruleIndex The index of the rule * @param user The OptimizelyUserContext - * @return Returns a DecisionResponse Object containing a Pair + * @return Returns a DecisionResponse Object containing a AbstractMap.SimpleEntry * where the Variation is the result and the Boolean is the skipToEveryoneElse. */ DecisionResponse getVariationFromDeliveryRule(@Nonnull ProjectConfig projectConfig, 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 5ce8ee2ce..9fd25f7b8 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java @@ -1209,11 +1209,11 @@ public void setForcedDecisionWithRuleKeyTest() { @Test public void setForcedDecisionsWithRuleKeyTest() { - String flagKey = "55555"; - String ruleKey = "77777"; + String flagKey = "feature_2"; + String ruleKey = "exp_no_audience"; String ruleKey2 = "88888"; String variationKey = "33333"; - String variationKey2 = "44444"; + String variationKey2 = "variation_with_traffic"; OptimizelyUserContext optimizelyUserContext = new OptimizelyUserContext( optimizely, userId, @@ -1231,6 +1231,13 @@ public void setForcedDecisionsWithRuleKeyTest() { // Update first forcedDecision optimizelyUserContext.setForcedDecision(optimizelyDecisionContext, optimizelyForcedDecision2); assertEquals(variationKey2, optimizelyUserContext.getForcedDecision(optimizelyDecisionContext).getVariationKey()); + + // Test to confirm decide uses proper FD + OptimizelyDecision decision = optimizelyUserContext.decide(flagKey, Arrays.asList(OptimizelyDecideOption.INCLUDE_REASONS)); + + assertTrue(decision.getReasons().contains( + String.format("Variation %s is mapped to flag: %s and rule: %s and user: %s in the forced decisions map.", variationKey2, flagKey, ruleKey, userId) + )); } @Test From 81d1c54f841b9fd9adce9acd44121a64d34309b9 Mon Sep 17 00:00:00 2001 From: Jake Brown Date: Tue, 26 Oct 2021 09:18:48 -0400 Subject: [PATCH 34/39] Update tests and add logging to match FSC requirement for findValidatedForcedDecision... --- .../optimizely/ab/OptimizelyUserContext.java | 5 ++- .../ab/OptimizelyUserContextTest.java | 39 ++++++++++++++++++- 2 files changed, 41 insertions(+), 3 deletions(-) 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 497887923..45ab3dc89 100644 --- a/core-api/src/main/java/com/optimizely/ab/OptimizelyUserContext.java +++ b/core-api/src/main/java/com/optimizely/ab/OptimizelyUserContext.java @@ -295,13 +295,14 @@ public DecisionResponse findValidatedForcedDecision(@Nonnull Optimize String ruleKey = optimizelyDecisionContext.getRuleKey(); String flagKey = optimizelyDecisionContext.getFlagKey(); String info; + String target = ruleKey != OptimizelyDecisionContext.OPTI_NULL_RULE_KEY ? String.format("flag (%s), rule (%s)", flagKey, ruleKey) : String.format("flag (%s)", flagKey); if (variation != null) { - info = String.format("Variation %s is mapped to flag: %s and rule: %s and user: %s in the forced decisions map.", variationKey, flagKey, ruleKey, userId); + info = String.format("Variation (%s) is mapped to %s and user (%s) in the forced decision map.", variationKey, target, userId); logger.debug(info); reasons.addInfo(info); return new DecisionResponse(variation, reasons); } else { - info = String.format("Invalid variation is mapped to flag: %s and rule: %s and user: %s in the forced decisions map.", flagKey, ruleKey, userId); + info = String.format("Invalid variation is mapped to %s and user (%s) in the forced decision map.", target, userId); logger.debug(info); reasons.addInfo(info); } 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 9fd25f7b8..d62f9ea05 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java @@ -1417,8 +1417,26 @@ public void removeForcedDecisionWithNullRuleKeyAfterAddingWithRuleKey() { } @Test - public void removeForcedDecisionWithoutRuleKeySdkNotReady() { + public void removeForcedDecisionWithIncorrectFlagKey() { String flagKey = "55555"; + String variationKey = "variation2"; + String incorrectFlagKey = "flag1"; + OptimizelyUserContext optimizelyUserContext = new OptimizelyUserContext( + optimizely, + userId, + Collections.emptyMap()); + + OptimizelyDecisionContext optimizelyDecisionContext = new OptimizelyDecisionContext(flagKey, null); + OptimizelyDecisionContext incorrectOptimizelyDecisionContext = new OptimizelyDecisionContext(incorrectFlagKey, null); + OptimizelyForcedDecision optimizelyForcedDecision = new OptimizelyForcedDecision(variationKey); + + optimizelyUserContext.setForcedDecision(optimizelyDecisionContext, optimizelyForcedDecision); + assertFalse(optimizelyUserContext.removeForcedDecision(incorrectOptimizelyDecisionContext)); + } + + @Test + public void removeForcedDecisionWithoutRuleKeySdkNotReady() { + String flagKey = "flag2"; String variationKey = "33333"; Optimizely optimizely = new Optimizely.Builder().build(); OptimizelyUserContext optimizelyUserContext = new OptimizelyUserContext( @@ -1433,6 +1451,25 @@ public void removeForcedDecisionWithoutRuleKeySdkNotReady() { assertFalse(optimizelyUserContext.removeForcedDecision(optimizelyDecisionContext)); } + @Test + public void removeForcedDecisionWithIncorrectFlagKeyButSimilarRuleKey() { + String flagKey = "flag2"; + String incorrectFlagKey = "flag3"; + String ruleKey = "default-rollout-3045-20390585493"; + String variationKey = "variation2"; + OptimizelyUserContext optimizelyUserContext = new OptimizelyUserContext( + optimizely, + userId, + Collections.emptyMap()); + + OptimizelyDecisionContext optimizelyDecisionContext = new OptimizelyDecisionContext(flagKey, ruleKey); + OptimizelyDecisionContext similarOptimizelyDecisionContext = new OptimizelyDecisionContext(incorrectFlagKey, ruleKey); + OptimizelyForcedDecision optimizelyForcedDecision = new OptimizelyForcedDecision(variationKey); + + optimizelyUserContext.setForcedDecision(optimizelyDecisionContext, optimizelyForcedDecision); + assertFalse(optimizelyUserContext.removeForcedDecision(similarOptimizelyDecisionContext)); + } + @Test public void removeAllForcedDecisions() { String flagKey = "55555"; From 88470c57dbe5de9b94303a00200dbb3b479fbf3c Mon Sep 17 00:00:00 2001 From: Jake Brown Date: Wed, 27 Oct 2021 08:56:33 -0400 Subject: [PATCH 35/39] Updated Test cases --- .../ab/OptimizelyUserContextTest.java | 26 ++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) 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 d62f9ea05..cf5cf9563 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java @@ -1236,7 +1236,7 @@ public void setForcedDecisionsWithRuleKeyTest() { OptimizelyDecision decision = optimizelyUserContext.decide(flagKey, Arrays.asList(OptimizelyDecideOption.INCLUDE_REASONS)); assertTrue(decision.getReasons().contains( - String.format("Variation %s is mapped to flag: %s and rule: %s and user: %s in the forced decisions map.", variationKey2, flagKey, ruleKey, userId) + String.format("Variation (%s) is mapped to flag (%s), rule (%s) and user (%s) in the forced decision map.", variationKey2, flagKey, ruleKey, userId) )); } @@ -1542,6 +1542,30 @@ public void findValidatedForcedDecisionWithoutRuleKey() { assertEquals(variationKey, variation.getKey()); } + @Test + public void setForcedDecisionsAndCallDecide() { + String flagKey = "feature_2"; + String ruleKey = "exp_no_audience"; + String variationKey = "variation_with_traffic"; + OptimizelyUserContext optimizelyUserContext = new OptimizelyUserContext( + optimizely, + userId, + Collections.emptyMap()); + + OptimizelyDecisionContext optimizelyDecisionContext = new OptimizelyDecisionContext(flagKey, ruleKey); + OptimizelyForcedDecision optimizelyForcedDecision = new OptimizelyForcedDecision(variationKey); + optimizelyUserContext.setForcedDecision(optimizelyDecisionContext, optimizelyForcedDecision); + assertEquals(variationKey, optimizelyUserContext.getForcedDecision(optimizelyDecisionContext).getVariationKey()); + + // Test to confirm decide uses proper FD + OptimizelyDecision decision = optimizelyUserContext.decide(flagKey, Arrays.asList(OptimizelyDecideOption.INCLUDE_REASONS)); + + assertNotNull(decision); + assertTrue(decision.getReasons().contains( + String.format("Variation (%s) is mapped to flag (%s), rule (%s) and user (%s) in the forced decision map.", variationKey, flagKey, ruleKey, userId) + )); + } + // utils Map createUserProfileMap(String experimentId, String variationId) { From 37323c49f97240d071de1dbc9db2c3cfd97fbded Mon Sep 17 00:00:00 2001 From: Jake Brown Date: Tue, 2 Nov 2021 09:25:41 -0400 Subject: [PATCH 36/39] Address comments for OptimizelyDecisionContext and OptimizelyUserContext --- .../java/com/optimizely/ab/OptimizelyDecisionContext.java | 2 ++ .../main/java/com/optimizely/ab/OptimizelyUserContext.java | 6 +++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/OptimizelyDecisionContext.java b/core-api/src/main/java/com/optimizely/ab/OptimizelyDecisionContext.java index c8f819c9b..4c4159301 100644 --- a/core-api/src/main/java/com/optimizely/ab/OptimizelyDecisionContext.java +++ b/core-api/src/main/java/com/optimizely/ab/OptimizelyDecisionContext.java @@ -21,6 +21,7 @@ public class OptimizelyDecisionContext { public static final String OPTI_NULL_RULE_KEY = "$opt-null-rule-key"; + public static final String OPTI_KEY_DIVIDER = "-$opt$-"; private String flagKey; private String ruleKey; @@ -41,6 +42,7 @@ public String getRuleKey() { public String getKey() { StringBuilder keyBuilder = new StringBuilder(); keyBuilder.append(flagKey); + keyBuilder.append(OPTI_KEY_DIVIDER); keyBuilder.append(getRuleKey()); return keyBuilder.toString(); } 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 45ab3dc89..47c81948f 100644 --- a/core-api/src/main/java/com/optimizely/ab/OptimizelyUserContext.java +++ b/core-api/src/main/java/com/optimizely/ab/OptimizelyUserContext.java @@ -60,11 +60,11 @@ public OptimizelyUserContext(@Nonnull Optimizely optimizely, this.optimizely = optimizely; this.userId = userId; if (attributes != null) { - this.attributes = Collections.synchronizedMap(new HashMap<>(attributes)); + this.attributes = new ConcurrentHashMap<>(new HashMap<>(attributes)); } else { - this.attributes = Collections.synchronizedMap(new HashMap<>()); + this.attributes = new ConcurrentHashMap<>(new HashMap<>()); } - this.forcedDecisionsMap = forcedDecisionsMap; + this.forcedDecisionsMap = forcedDecisionsMap != null ? forcedDecisionsMap : new ConcurrentHashMap<>(); } public OptimizelyUserContext(@Nonnull Optimizely optimizely, @Nonnull String userId) { From ff7903187c092f60e484206c2995f23cfa529d14 Mon Sep 17 00:00:00 2001 From: Jake Brown Date: Tue, 2 Nov 2021 13:39:04 -0400 Subject: [PATCH 37/39] Added test cases and fixed Null case when experiment is null during UserEventFactory.createImpressionEvent. --- .../ab/bucketing/DecisionService.java | 1 + .../ab/event/internal/UserEventFactory.java | 6 +- .../ab/OptimizelyUserContextTest.java | 144 ++++++++++++++++++ 3 files changed, 148 insertions(+), 3 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java b/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java index d295d19cb..e386c360d 100644 --- a/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java +++ b/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java @@ -634,6 +634,7 @@ DecisionResponse getVariationFromDeliveryRule(@Nonnull @Nonnull int ruleIndex, @Nonnull OptimizelyUserContext user) { DecisionReasons reasons = DefaultDecisionReasons.newInstance(); + Boolean skipToEveryoneElse = false; AbstractMap.SimpleEntry variationToSkipToEveryoneElsePair; // Check forced-decisions first diff --git a/core-api/src/main/java/com/optimizely/ab/event/internal/UserEventFactory.java b/core-api/src/main/java/com/optimizely/ab/event/internal/UserEventFactory.java index 20d771033..9c44f455b 100644 --- a/core-api/src/main/java/com/optimizely/ab/event/internal/UserEventFactory.java +++ b/core-api/src/main/java/com/optimizely/ab/event/internal/UserEventFactory.java @@ -54,9 +54,9 @@ public static ImpressionEvent createImpressionEvent(@Nonnull ProjectConfig proje if (variation != null) { variationKey = variation.getKey(); variationID = variation.getId(); - layerID = activatedExperiment.getLayerId(); - experimentId = activatedExperiment.getId(); - experimentKey = activatedExperiment.getKey(); + layerID = activatedExperiment != null ? activatedExperiment.getLayerId() : ""; + experimentId = activatedExperiment != null ? activatedExperiment.getId() : ""; + experimentKey = activatedExperiment != null ? activatedExperiment.getKey() : ""; } UserContext userContext = new UserContext.Builder() 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 cf5cf9563..6197d878b 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java @@ -1565,7 +1565,151 @@ public void setForcedDecisionsAndCallDecide() { String.format("Variation (%s) is mapped to flag (%s), rule (%s) and user (%s) in the forced decision map.", variationKey, flagKey, ruleKey, userId) )); } + /******************************************[START DECIDE TESTS WITH FDs]******************************************/ + @Test + public void setForcedDecisionsAndCallDecideFlagToDecision() { + String flagKey = "feature_1"; + String variationKey = "a"; + + optimizely = new Optimizely.Builder() + .withDatafile(datafile) + .withEventProcessor(new ForwardingEventProcessor(eventHandler, null)) + .build(); + + OptimizelyUserContext optimizelyUserContext = new OptimizelyUserContext( + optimizely, + userId, + Collections.emptyMap()); + + OptimizelyDecisionContext optimizelyDecisionContext = new OptimizelyDecisionContext(flagKey, null); + OptimizelyForcedDecision optimizelyForcedDecision = new OptimizelyForcedDecision(variationKey); + optimizelyUserContext.setForcedDecision(optimizelyDecisionContext, optimizelyForcedDecision); + assertEquals(variationKey, optimizelyUserContext.getForcedDecision(optimizelyDecisionContext).getVariationKey()); + + optimizely.addDecisionNotificationHandler( + decisionNotification -> { + Assert.assertEquals(decisionNotification.getDecisionInfo().get(DECISION_EVENT_DISPATCHED), true); + isListenerCalled = true; + }); + + isListenerCalled = false; + + // Test to confirm decide uses proper FD + OptimizelyDecision decision = optimizelyUserContext.decide(flagKey, Arrays.asList(OptimizelyDecideOption.INCLUDE_REASONS)); + + assertTrue(isListenerCalled); + + String variationId = "10389729780"; + String experimentId = ""; + + + DecisionMetadata metadata = new DecisionMetadata.Builder() + .setFlagKey(flagKey) + .setRuleKey("") + .setRuleType("feature-test") + .setVariationKey(variationKey) + .setEnabled(true) + .build(); + + eventHandler.expectImpression(experimentId, variationId, userId, Collections.emptyMap(), metadata); + + assertNotNull(decision); + assertTrue(decision.getReasons().contains( + String.format("Variation (%s) is mapped to flag (%s) and user (%s) in the forced decision map.", variationKey, flagKey, userId) + )); + } + @Test + public void setForcedDecisionsAndCallDecideExperimentRuleToDecision() { + String flagKey = "feature_1"; + String ruleKey = "exp_with_audience"; + String variationKey = "a"; + + optimizely = new Optimizely.Builder() + .withDatafile(datafile) + .withEventProcessor(new ForwardingEventProcessor(eventHandler, null)) + .build(); + + OptimizelyUserContext optimizelyUserContext = new OptimizelyUserContext( + optimizely, + userId, + Collections.emptyMap()); + + OptimizelyDecisionContext optimizelyDecisionContext = new OptimizelyDecisionContext(flagKey, ruleKey); + OptimizelyForcedDecision optimizelyForcedDecision = new OptimizelyForcedDecision(variationKey); + optimizelyUserContext.setForcedDecision(optimizelyDecisionContext, optimizelyForcedDecision); + assertEquals(variationKey, optimizelyUserContext.getForcedDecision(optimizelyDecisionContext).getVariationKey()); + optimizely.addDecisionNotificationHandler( + decisionNotification -> { + Assert.assertEquals(decisionNotification.getDecisionInfo().get(DECISION_EVENT_DISPATCHED), true); + isListenerCalled = true; + }); + + isListenerCalled = false; + + // Test to confirm decide uses proper FD + OptimizelyDecision decision = optimizelyUserContext.decide(flagKey, Arrays.asList(OptimizelyDecideOption.INCLUDE_REASONS)); + + assertTrue(isListenerCalled); + + String variationId = "10389729780"; + String experimentId = "10390977673"; + + + eventHandler.expectImpression(experimentId, variationId, userId, Collections.emptyMap()); + + assertNotNull(decision); + assertTrue(decision.getReasons().contains( + String.format("Variation (%s) is mapped to flag (%s), rule (%s) and user (%s) in the forced decision map.", variationKey, flagKey, ruleKey, userId) + )); + } + + @Test + public void setForcedDecisionsAndCallDecideDeliveryRuleToDecision() { + String flagKey = "feature_1"; + String ruleKey = "3332020515"; + String variationKey = "3324490633"; + + optimizely = new Optimizely.Builder() + .withDatafile(datafile) + .withEventProcessor(new ForwardingEventProcessor(eventHandler, null)) + .build(); + + OptimizelyUserContext optimizelyUserContext = new OptimizelyUserContext( + optimizely, + userId, + Collections.emptyMap()); + + OptimizelyDecisionContext optimizelyDecisionContext = new OptimizelyDecisionContext(flagKey, ruleKey); + OptimizelyForcedDecision optimizelyForcedDecision = new OptimizelyForcedDecision(variationKey); + optimizelyUserContext.setForcedDecision(optimizelyDecisionContext, optimizelyForcedDecision); + assertEquals(variationKey, optimizelyUserContext.getForcedDecision(optimizelyDecisionContext).getVariationKey()); + + optimizely.addDecisionNotificationHandler( + decisionNotification -> { + Assert.assertEquals(decisionNotification.getDecisionInfo().get(DECISION_EVENT_DISPATCHED), true); + isListenerCalled = true; + }); + + isListenerCalled = false; + + // Test to confirm decide uses proper FD + OptimizelyDecision decision = optimizelyUserContext.decide(flagKey, Arrays.asList(OptimizelyDecideOption.INCLUDE_REASONS)); + + assertTrue(isListenerCalled); + + String variationId = "3324490633"; + String experimentId = "3332020515"; + + + eventHandler.expectImpression(experimentId, variationId, userId, Collections.emptyMap()); + + assertNotNull(decision); + assertTrue(decision.getReasons().contains( + String.format("Variation (%s) is mapped to flag (%s), rule (%s) and user (%s) in the forced decision map.", variationKey, flagKey, ruleKey, userId) + )); + } + /********************************************[END DECIDE TESTS WITH FDs]******************************************/ // utils Map createUserProfileMap(String experimentId, String variationId) { From f236148ef59ff78f208624031220a473bfb70b0f Mon Sep 17 00:00:00 2001 From: Jake Brown Date: Tue, 2 Nov 2021 13:53:17 -0400 Subject: [PATCH 38/39] Fix OptimizelyDecisionContext tests to use -569XZilms as the divider when making the object key. --- .../java/com/optimizely/ab/OptimizelyDecisionContextTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core-api/src/test/java/com/optimizely/ab/OptimizelyDecisionContextTest.java b/core-api/src/test/java/com/optimizely/ab/OptimizelyDecisionContextTest.java index 4b0e95fbc..daaf59d61 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyDecisionContextTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyDecisionContextTest.java @@ -25,7 +25,7 @@ public class OptimizelyDecisionContextTest { public void initializeOptimizelyDecisionContextWithFlagKeyAndRuleKey() { String flagKey = "test-flag-key"; String ruleKey = "1029384756"; - String expectedKey = flagKey + ruleKey; + String expectedKey = flagKey + OptimizelyDecisionContext.OPTI_KEY_DIVIDER + ruleKey; OptimizelyDecisionContext optimizelyDecisionContext = new OptimizelyDecisionContext(flagKey, ruleKey); assertEquals(flagKey, optimizelyDecisionContext.getFlagKey()); assertEquals(ruleKey, optimizelyDecisionContext.getRuleKey()); @@ -35,7 +35,7 @@ public void initializeOptimizelyDecisionContextWithFlagKeyAndRuleKey() { @Test public void initializeOptimizelyDecisionContextWithFlagKey() { String flagKey = "test-flag-key"; - String expectedKey = flagKey + OptimizelyDecisionContext.OPTI_NULL_RULE_KEY; + String expectedKey = flagKey + OptimizelyDecisionContext.OPTI_KEY_DIVIDER + OptimizelyDecisionContext.OPTI_NULL_RULE_KEY; OptimizelyDecisionContext optimizelyDecisionContext = new OptimizelyDecisionContext(flagKey, null); assertEquals(flagKey, optimizelyDecisionContext.getFlagKey()); assertEquals(OptimizelyDecisionContext.OPTI_NULL_RULE_KEY, optimizelyDecisionContext.getRuleKey()); From af58da8ef42ed236ef20942caccb433c4397fe00 Mon Sep 17 00:00:00 2001 From: "FOLIO3PK\\muhammadnoman" Date: Wed, 3 Nov 2021 18:17:16 +0500 Subject: [PATCH 39/39] Changed concurrent hashmap to synchronized map as it was causing null pointer exception --- .../main/java/com/optimizely/ab/OptimizelyUserContext.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 47c81948f..0851d0ce9 100644 --- a/core-api/src/main/java/com/optimizely/ab/OptimizelyUserContext.java +++ b/core-api/src/main/java/com/optimizely/ab/OptimizelyUserContext.java @@ -60,9 +60,9 @@ public OptimizelyUserContext(@Nonnull Optimizely optimizely, this.optimizely = optimizely; this.userId = userId; if (attributes != null) { - this.attributes = new ConcurrentHashMap<>(new HashMap<>(attributes)); + this.attributes = Collections.synchronizedMap(new HashMap<>(attributes)); } else { - this.attributes = new ConcurrentHashMap<>(new HashMap<>()); + this.attributes = Collections.synchronizedMap(new HashMap<>()); } this.forcedDecisionsMap = forcedDecisionsMap != null ? forcedDecisionsMap : new ConcurrentHashMap<>(); }