From 54f4bfc2706c6f42f039557632ff1fe7ee621a1f Mon Sep 17 00:00:00 2001 From: Jae Kim Date: Thu, 17 Dec 2020 15:01:15 -0800 Subject: [PATCH 1/9] change reasons to a response tuple --- .../java/com/optimizely/ab/Optimizely.java | 43 ++- .../com/optimizely/ab/bucketing/Bucketer.java | 44 ++- .../ab/bucketing/DecisionService.java | 267 +++++++++--------- .../ab/config/audience/AndCondition.java | 8 +- .../config/audience/AudienceIdCondition.java | 13 +- .../ab/config/audience/Condition.java | 6 +- .../ab/config/audience/EmptyCondition.java | 5 +- .../ab/config/audience/NotCondition.java | 10 +- .../ab/config/audience/NullCondition.java | 6 +- .../ab/config/audience/OrCondition.java | 7 +- .../ab/config/audience/UserAttribute.java | 27 +- .../ab/internal/ExperimentUtils.java | 107 +++---- .../optimizelydecision/DecisionReasons.java | 28 +- .../optimizelydecision/DecisionResponse.java | 24 ++ .../DefaultDecisionReasons.java | 43 +-- .../ErrorsDecisionReasons.java | 56 ---- .../AudienceConditionEvaluationTest.java | 3 +- 17 files changed, 307 insertions(+), 390 deletions(-) create mode 100644 core-api/src/main/java/com/optimizely/ab/optimizelydecision/DecisionResponse.java delete mode 100644 core-api/src/main/java/com/optimizely/ab/optimizelydecision/ErrorsDecisionReasons.java 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 75979e335..f47d35450 100644 --- a/core-api/src/main/java/com/optimizely/ab/Optimizely.java +++ b/core-api/src/main/java/com/optimizely/ab/Optimizely.java @@ -34,11 +34,7 @@ import com.optimizely.ab.optimizelyconfig.OptimizelyConfig; import com.optimizely.ab.optimizelyconfig.OptimizelyConfigManager; import com.optimizely.ab.optimizelyconfig.OptimizelyConfigService; -import com.optimizely.ab.optimizelydecision.DecisionMessage; -import com.optimizely.ab.optimizelydecision.DecisionReasons; -import com.optimizely.ab.optimizelydecision.DefaultDecisionReasons; -import com.optimizely.ab.optimizelydecision.OptimizelyDecideOption; -import com.optimizely.ab.optimizelydecision.OptimizelyDecision; +import com.optimizely.ab.optimizelydecision.*; import com.optimizely.ab.optimizelyjson.OptimizelyJSON; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -427,7 +423,7 @@ private Boolean isFeatureEnabled(@Nonnull ProjectConfig projectConfig, Map copiedAttributes = copyAttributes(attributes); FeatureDecision.DecisionSource decisionSource = FeatureDecision.DecisionSource.ROLLOUT; - FeatureDecision featureDecision = decisionService.getVariationForFeature(featureFlag, userId, copiedAttributes, projectConfig); + FeatureDecision featureDecision = decisionService.getVariationForFeature(featureFlag, userId, copiedAttributes, projectConfig).getResult(); Boolean featureEnabled = false; SourceInfo sourceInfo = new RolloutSourceInfo(); if (featureDecision.decisionSource != null) { @@ -736,7 +732,7 @@ T getFeatureVariableValueForType(@Nonnull String featureKey, String variableValue = variable.getDefaultValue(); Map copiedAttributes = copyAttributes(attributes); - FeatureDecision featureDecision = decisionService.getVariationForFeature(featureFlag, userId, copiedAttributes, projectConfig); + FeatureDecision featureDecision = decisionService.getVariationForFeature(featureFlag, userId, copiedAttributes, projectConfig).getResult(); Boolean featureEnabled = false; if (featureDecision.variation != null) { if (featureDecision.variation.getFeatureEnabled()) { @@ -869,7 +865,7 @@ public OptimizelyJSON getAllFeatureVariables(@Nonnull String featureKey, } Map copiedAttributes = copyAttributes(attributes); - FeatureDecision featureDecision = decisionService.getVariationForFeature(featureFlag, userId, copiedAttributes, projectConfig); + FeatureDecision featureDecision = decisionService.getVariationForFeature(featureFlag, userId, copiedAttributes, projectConfig).getResult(); Boolean featureEnabled = false; Variation variation = featureDecision.variation; @@ -970,7 +966,7 @@ private Variation getVariation(@Nonnull ProjectConfig projectConfig, @Nonnull String userId, @Nonnull Map attributes) throws UnknownExperimentException { Map copiedAttributes = copyAttributes(attributes); - Variation variation = decisionService.getVariation(experiment, userId, copiedAttributes, projectConfig); + Variation variation = decisionService.getVariation(experiment, userId, copiedAttributes, projectConfig).getResult(); String notificationType = NotificationCenter.DecisionNotificationType.AB_TEST.toString(); @@ -1084,7 +1080,7 @@ public Variation getForcedVariation(@Nonnull String experimentKey, return null; } - return decisionService.getForcedVariation(experiment, userId); + return decisionService.getForcedVariation(experiment, userId).getResult(); } /** @@ -1182,13 +1178,14 @@ OptimizelyDecision decide(@Nonnull OptimizelyUserContext user, DecisionReasons decisionReasons = DefaultDecisionReasons.newInstance(allOptions); Map copiedAttributes = new HashMap<>(attributes); - FeatureDecision flagDecision = decisionService.getVariationForFeature( + DecisionResponse decisionVariation = decisionService.getVariationForFeature( flag, userId, copiedAttributes, projectConfig, - allOptions, - decisionReasons); + allOptions); + FeatureDecision flagDecision = decisionVariation.getResult(); + decisionReasons.merge(decisionVariation.getReasons()); Boolean flagEnabled = false; if (flagDecision.variation != null) { @@ -1200,11 +1197,12 @@ OptimizelyDecision decide(@Nonnull OptimizelyUserContext user, Map variableMap = new HashMap<>(); if (!allOptions.contains(OptimizelyDecideOption.EXCLUDE_VARIABLES)) { - variableMap = getDecisionVariableMap( + DecisionResponse> decisionVariables = getDecisionVariableMap( flag, flagDecision.variation, - flagEnabled, - decisionReasons); + flagEnabled); + variableMap = decisionVariables.getResult(); + decisionReasons.merge(decisionVariables.getReasons()); } OptimizelyJSON optimizelyJSON = new OptimizelyJSON(variableMap); @@ -1305,10 +1303,11 @@ private List getAllOptions(List return copiedOptions; } - private Map getDecisionVariableMap(@Nonnull FeatureFlag flag, - @Nonnull Variation variation, - @Nonnull Boolean featureEnabled, - @Nonnull DecisionReasons decisionReasons) { + private DecisionResponse> getDecisionVariableMap(@Nonnull FeatureFlag flag, + @Nonnull Variation variation, + @Nonnull Boolean featureEnabled) { + DecisionReasons reasons = new DecisionReasons(); + Map valuesMap = new HashMap(); for (FeatureVariable variable : flag.getVariables()) { String value = variable.getDefaultValue(); @@ -1321,7 +1320,7 @@ private Map getDecisionVariableMap(@Nonnull FeatureFlag flag, Object convertedValue = convertStringToType(value, variable.getType()); if (convertedValue == null) { - decisionReasons.addError(DecisionMessage.VARIABLE_VALUE_INVALID.reason(variable.getKey())); + reasons.addError(DecisionMessage.VARIABLE_VALUE_INVALID.reason(variable.getKey())); } else if (convertedValue instanceof OptimizelyJSON) { convertedValue = ((OptimizelyJSON) convertedValue).toMap(); } @@ -1329,7 +1328,7 @@ private Map getDecisionVariableMap(@Nonnull FeatureFlag flag, valuesMap.put(variable.getKey(), convertedValue); } - return valuesMap; + return new DecisionResponse(valuesMap, reasons); } /** diff --git a/core-api/src/main/java/com/optimizely/ab/bucketing/Bucketer.java b/core-api/src/main/java/com/optimizely/ab/bucketing/Bucketer.java index 0429dd585..87d71d7da 100644 --- a/core-api/src/main/java/com/optimizely/ab/bucketing/Bucketer.java +++ b/core-api/src/main/java/com/optimizely/ab/bucketing/Bucketer.java @@ -20,6 +20,7 @@ import com.optimizely.ab.bucketing.internal.MurmurHash3; import com.optimizely.ab.config.*; import com.optimizely.ab.optimizelydecision.DecisionReasons; +import com.optimizely.ab.optimizelydecision.DecisionResponse; import com.optimizely.ab.optimizelydecision.DefaultDecisionReasons; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -69,8 +70,7 @@ private String bucketToEntity(int bucketValue, List trafficAl private Experiment bucketToExperiment(@Nonnull Group group, @Nonnull String bucketingId, - @Nonnull ProjectConfig projectConfig, - @Nonnull DecisionReasons reasons) { + @Nonnull ProjectConfig projectConfig) { // "salt" the bucket id using the group id String bucketKey = bucketingId + group.getId(); @@ -89,9 +89,10 @@ private Experiment bucketToExperiment(@Nonnull Group group, return null; } - private Variation bucketToVariation(@Nonnull Experiment experiment, - @Nonnull String bucketingId, - @Nonnull DecisionReasons reasons) { + private DecisionResponse bucketToVariation(@Nonnull Experiment experiment, + @Nonnull String bucketingId) { + DecisionReasons reasons = DefaultDecisionReasons.newInstance(); + // "salt" the bucket id using the experiment id String experimentId = experiment.getId(); String experimentKey = experiment.getKey(); @@ -111,13 +112,13 @@ private Variation bucketToVariation(@Nonnull Experiment experiment, experimentKey); logger.info(message); - return bucketedVariation; + return new DecisionResponse(bucketedVariation, reasons); } // user was not bucketed to a variation String message = reasons.addInfo("User with bucketingId \"%s\" is not in any variation of experiment \"%s\".", bucketingId, experimentKey); logger.info(message); - return null; + return new DecisionResponse(null, reasons); } /** @@ -126,14 +127,14 @@ private Variation bucketToVariation(@Nonnull Experiment experiment, * @param experiment The Experiment in which the user is to be bucketed. * @param bucketingId string A customer-assigned value used to create the key for the murmur hash. * @param projectConfig The current projectConfig - * @param reasons Decision log messages - * @return Variation the user is bucketed into or null. + * @return A {@link DecisionResponse} including the {@link Variation} that user is bucketed into (or null) and the decision reasons */ @Nullable - public Variation bucket(@Nonnull Experiment experiment, - @Nonnull String bucketingId, - @Nonnull ProjectConfig projectConfig, - @Nonnull DecisionReasons reasons) { + public DecisionResponse bucket(@Nonnull Experiment experiment, + @Nonnull String bucketingId, + @Nonnull ProjectConfig projectConfig) { + DecisionReasons reasons = DefaultDecisionReasons.newInstance(); + // ---------- Bucket User ---------- String groupId = experiment.getGroupId(); // check whether the experiment belongs to a group @@ -141,11 +142,11 @@ public Variation bucket(@Nonnull Experiment experiment, Group experimentGroup = projectConfig.getGroupIdMapping().get(groupId); // bucket to an experiment only if group entities are to be mutually exclusive if (experimentGroup.getPolicy().equals(Group.RANDOM_POLICY)) { - Experiment bucketedExperiment = bucketToExperiment(experimentGroup, bucketingId, projectConfig, reasons); + Experiment bucketedExperiment = bucketToExperiment(experimentGroup, bucketingId, projectConfig); if (bucketedExperiment == null) { String message = reasons.addInfo("User with bucketingId \"%s\" is not in any experiment of group %s.", bucketingId, experimentGroup.getId()); logger.info(message); - return null; + return new DecisionResponse(null, reasons); } else { } @@ -155,7 +156,7 @@ public Variation bucket(@Nonnull Experiment experiment, String message = reasons.addInfo("User with bucketingId \"%s\" is not in experiment \"%s\" of group %s.", bucketingId, experiment.getKey(), experimentGroup.getId()); logger.info(message); - return null; + return new DecisionResponse(null, reasons); } String message = reasons.addInfo("User with bucketingId \"%s\" is in experiment \"%s\" of group %s.", bucketingId, experiment.getKey(), @@ -164,14 +165,9 @@ public Variation bucket(@Nonnull Experiment experiment, } } - return bucketToVariation(experiment, bucketingId, reasons); - } - - @Nullable - public Variation bucket(@Nonnull Experiment experiment, - @Nonnull String bucketingId, - @Nonnull ProjectConfig projectConfig) { - return bucket(experiment, bucketingId, projectConfig, DefaultDecisionReasons.newInstance()); + DecisionResponse decisionResponse = bucketToVariation(experiment, bucketingId); + reasons.merge(decisionResponse.getReasons()); + return new DecisionResponse<>(decisionResponse.getResult(), reasons); } //======== Helper methods ========// 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 588f6eefb..55c363c13 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 @@ -21,6 +21,7 @@ import com.optimizely.ab.internal.ControlAttribute; import com.optimizely.ab.internal.ExperimentUtils; import com.optimizely.ab.optimizelydecision.DecisionReasons; +import com.optimizely.ab.optimizelydecision.DecisionResponse; import com.optimizely.ab.optimizelydecision.DefaultDecisionReasons; import com.optimizely.ab.optimizelydecision.OptimizelyDecideOption; import org.slf4j.Logger; @@ -86,30 +87,36 @@ public DecisionService(@Nonnull Bucketer bucketer, * @param filteredAttributes The user's attributes. This should be filtered to just attributes in the Datafile. * @param projectConfig The current projectConfig * @param options An array of decision options - * @param reasons Decision log messages - * @return The {@link Variation} the user is allocated into. + * @return A {@link DecisionResponse} including the {@link Variation} that user is bucketed into (or null) and the decision reasons */ @Nullable - public Variation getVariation(@Nonnull Experiment experiment, - @Nonnull String userId, - @Nonnull Map filteredAttributes, - @Nonnull ProjectConfig projectConfig, - @Nonnull List options, - @Nonnull DecisionReasons reasons) { - if (!ExperimentUtils.isExperimentActive(experiment, reasons)) { - return null; + public DecisionResponse getVariation(@Nonnull Experiment experiment, + @Nonnull String userId, + @Nonnull Map filteredAttributes, + @Nonnull ProjectConfig projectConfig, + @Nonnull List options) { + DecisionReasons reasons = DefaultDecisionReasons.newInstance(); + + if (!experiment.isActive()) { + String message = reasons.addInfo("Experiment \"%s\" is not running.", experiment.getKey()); + logger.info(message); + return new DecisionResponse(null, reasons); } // look for forced bucketing first. - Variation variation = getForcedVariation(experiment, userId, reasons); + DecisionResponse decisionVariation = getForcedVariation(experiment, userId); + reasons.merge(decisionVariation.getReasons()); + Variation variation = decisionVariation.getResult(); // check for whitelisting if (variation == null) { - variation = getWhitelistedVariation(experiment, userId, reasons); + decisionVariation = getWhitelistedVariation(experiment, userId); + reasons.merge(decisionVariation.getReasons()); + variation = decisionVariation.getResult(); } if (variation != null) { - return variation; + return new DecisionResponse(variation, reasons); } // fetch the user profile map from the user profile service @@ -136,42 +143,49 @@ public Variation getVariation(@Nonnull Experiment experiment, // check if user exists in user profile if (userProfile != null) { - variation = getStoredVariation(experiment, userProfile, projectConfig, reasons); + decisionVariation = getStoredVariation(experiment, userProfile, projectConfig); + reasons.merge(decisionVariation.getReasons()); + variation = decisionVariation.getResult(); // return the stored variation if it exists if (variation != null) { - return variation; + return new DecisionResponse(variation, reasons); } } else { // if we could not find a user profile, make a new one userProfile = new UserProfile(userId, new HashMap()); } } - if (ExperimentUtils.doesUserMeetAudienceConditions(projectConfig, experiment, filteredAttributes, EXPERIMENT, experiment.getKey(), reasons)) { - String bucketingId = getBucketingId(userId, filteredAttributes, reasons); - variation = bucketer.bucket(experiment, bucketingId, projectConfig, reasons); + DecisionResponse decisionBoolean = ExperimentUtils.doesUserMeetAudienceConditions(projectConfig, experiment, filteredAttributes, EXPERIMENT, experiment.getKey()); + reasons.merge(decisionBoolean.getReasons()); + if (decisionBoolean.getResult()) { + String bucketingId = getBucketingId(userId, filteredAttributes); + + decisionVariation = bucketer.bucket(experiment, bucketingId, projectConfig); + reasons.merge(decisionVariation.getReasons()); + variation = decisionVariation.getResult(); if (variation != null) { if (userProfileService != null && !ignoreUPS) { - saveVariation(experiment, variation, userProfile, reasons); + saveVariation(experiment, variation, userProfile); } else { logger.debug("This decision will not be saved since the UserProfileService is null."); } } - return variation; + return new DecisionResponse(variation, reasons); } String message = reasons.addInfo("User \"%s\" does not meet conditions to be in experiment \"%s\".", userId, experiment.getKey()); logger.info(message); - return null; + return new DecisionResponse(null, reasons); } @Nullable - public Variation getVariation(@Nonnull Experiment experiment, - @Nonnull String userId, - @Nonnull Map filteredAttributes, - @Nonnull ProjectConfig projectConfig) { - return getVariation(experiment, userId, filteredAttributes, projectConfig, Collections.emptyList(), DefaultDecisionReasons.newInstance()); + public DecisionResponse getVariation(@Nonnull Experiment experiment, + @Nonnull String userId, + @Nonnull Map filteredAttributes, + @Nonnull ProjectConfig projectConfig) { + return getVariation(experiment, userId, filteredAttributes, projectConfig, Collections.emptyList()); } /** @@ -182,22 +196,28 @@ public Variation getVariation(@Nonnull Experiment experiment, * @param filteredAttributes A map of filtered attributes. * @param projectConfig The current projectConfig * @param options An array of decision options - * @param reasons Decision log messages - * @return {@link FeatureDecision} + * @return A {@link DecisionResponse} including the {@link FeatureDecision} and the decision reasons */ @Nonnull - public FeatureDecision getVariationForFeature(@Nonnull FeatureFlag featureFlag, - @Nonnull String userId, - @Nonnull Map filteredAttributes, - @Nonnull ProjectConfig projectConfig, - @Nonnull List options, - @Nonnull DecisionReasons reasons) { + public DecisionResponse getVariationForFeature(@Nonnull FeatureFlag featureFlag, + @Nonnull String userId, + @Nonnull Map filteredAttributes, + @Nonnull ProjectConfig projectConfig, + @Nonnull List options) { + DecisionReasons reasons = DefaultDecisionReasons.newInstance(); + if (!featureFlag.getExperimentIds().isEmpty()) { for (String experimentId : featureFlag.getExperimentIds()) { Experiment experiment = projectConfig.getExperimentIdMapping().get(experimentId); - Variation variation = getVariation(experiment, userId, filteredAttributes, projectConfig, options, reasons); + + DecisionResponse decisionVariation = getVariation(experiment, userId, filteredAttributes, projectConfig, options); + reasons.merge(decisionVariation.getReasons()); + Variation variation = decisionVariation.getResult(); + if (variation != null) { - return new FeatureDecision(experiment, variation, FeatureDecision.DecisionSource.FEATURE_TEST); + return new DecisionResponse( + new FeatureDecision(experiment, variation, FeatureDecision.DecisionSource.FEATURE_TEST), + reasons); } } } else { @@ -205,7 +225,10 @@ public FeatureDecision getVariationForFeature(@Nonnull FeatureFlag featureFlag, logger.info(message); } - FeatureDecision featureDecision = getVariationForFeatureInRollout(featureFlag, userId, filteredAttributes, projectConfig, reasons); + DecisionResponse decisionFeature = getVariationForFeatureInRollout(featureFlag, userId, filteredAttributes, projectConfig); + reasons.merge(decisionFeature.getReasons()); + FeatureDecision featureDecision = decisionFeature.getResult(); + if (featureDecision.variation == null) { String message = reasons.addInfo("The user \"%s\" was not bucketed into a rollout for feature flag \"%s\".", userId, featureFlag.getKey()); @@ -215,16 +238,15 @@ public FeatureDecision getVariationForFeature(@Nonnull FeatureFlag featureFlag, userId, featureFlag.getKey()); logger.info(message); } - return featureDecision; + return new DecisionResponse(featureDecision, reasons); } @Nonnull - public FeatureDecision getVariationForFeature(@Nonnull FeatureFlag featureFlag, - @Nonnull String userId, - @Nonnull Map filteredAttributes, - @Nonnull ProjectConfig projectConfig) { - - return getVariationForFeature(featureFlag, userId, filteredAttributes, projectConfig,Collections.emptyList(), DefaultDecisionReasons.newInstance()); + public DecisionResponse getVariationForFeature(@Nonnull FeatureFlag featureFlag, + @Nonnull String userId, + @Nonnull Map filteredAttributes, + @Nonnull ProjectConfig projectConfig) { + return getVariationForFeature(featureFlag, userId, filteredAttributes, projectConfig,Collections.emptyList()); } /** @@ -236,42 +258,52 @@ public FeatureDecision getVariationForFeature(@Nonnull FeatureFlag featureFlag, * @param userId User Identifier * @param filteredAttributes A map of filtered attributes. * @param projectConfig The current projectConfig - * @param reasons Decision log messages - * @return {@link FeatureDecision} + * @return A {@link DecisionResponse} including the {@link FeatureDecision} and the decision reasons */ @Nonnull - FeatureDecision getVariationForFeatureInRollout(@Nonnull FeatureFlag featureFlag, - @Nonnull String userId, - @Nonnull Map filteredAttributes, - @Nonnull ProjectConfig projectConfig, - @Nonnull DecisionReasons reasons) { + DecisionResponse getVariationForFeatureInRollout(@Nonnull FeatureFlag featureFlag, + @Nonnull String userId, + @Nonnull Map filteredAttributes, + @Nonnull ProjectConfig projectConfig) { + DecisionReasons reasons = DefaultDecisionReasons.newInstance(); + // use rollout to get variation for feature if (featureFlag.getRolloutId().isEmpty()) { String message = reasons.addInfo("The feature flag \"%s\" is not used in a rollout.", featureFlag.getKey()); logger.info(message); - return new FeatureDecision(null, null, null); + return new DecisionResponse(new FeatureDecision(null, null, null), reasons); } Rollout rollout = projectConfig.getRolloutIdMapping().get(featureFlag.getRolloutId()); if (rollout == null) { String message = reasons.addInfo("The rollout with id \"%s\" was not found in the datafile for feature flag \"%s\".", featureFlag.getRolloutId(), featureFlag.getKey()); logger.error(message); - return new FeatureDecision(null, null, null); + return new DecisionResponse(new FeatureDecision(null, null, null), reasons); } // for all rules before the everyone else rule int rolloutRulesLength = rollout.getExperiments().size(); - String bucketingId = getBucketingId(userId, filteredAttributes, reasons); + String bucketingId = getBucketingId(userId, filteredAttributes); + Variation variation; + DecisionResponse decisionMeetAudience; + DecisionResponse decisionVariation; for (int i = 0; i < rolloutRulesLength - 1; i++) { Experiment rolloutRule = rollout.getExperiments().get(i); - if (ExperimentUtils.doesUserMeetAudienceConditions(projectConfig, rolloutRule, filteredAttributes, RULE, Integer.toString(i + 1), reasons)) { - variation = bucketer.bucket(rolloutRule, bucketingId, projectConfig, reasons); + + decisionMeetAudience = ExperimentUtils.doesUserMeetAudienceConditions(projectConfig, rolloutRule, filteredAttributes, 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 FeatureDecision(rolloutRule, variation, - FeatureDecision.DecisionSource.ROLLOUT); + 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\".", userId, i + 1); logger.debug(message); @@ -280,24 +312,23 @@ FeatureDecision getVariationForFeatureInRollout(@Nonnull FeatureFlag featureFlag // get last rule which is the fall back rule Experiment finalRule = rollout.getExperiments().get(rolloutRulesLength - 1); - if (ExperimentUtils.doesUserMeetAudienceConditions(projectConfig, finalRule, filteredAttributes, RULE, "Everyone Else", reasons)) { - variation = bucketer.bucket(finalRule, bucketingId, projectConfig, reasons); + + decisionMeetAudience = ExperimentUtils.doesUserMeetAudienceConditions(projectConfig, finalRule, filteredAttributes, RULE, "Everyone Else"); + reasons.merge(decisionMeetAudience.getReasons()); + if (decisionMeetAudience.getResult()) { + decisionVariation = bucketer.bucket(finalRule, bucketingId, projectConfig); + variation = decisionVariation.getResult(); + reasons.merge(decisionVariation.getReasons()); + if (variation != null) { String message = reasons.addInfo("User \"%s\" meets conditions for targeting rule \"Everyone Else\".", userId); logger.debug(message); - return new FeatureDecision(finalRule, variation, - FeatureDecision.DecisionSource.ROLLOUT); + return new DecisionResponse( + new FeatureDecision(finalRule, variation, FeatureDecision.DecisionSource.ROLLOUT), + reasons); } } - return new FeatureDecision(null, null, null); - } - - @Nonnull - FeatureDecision getVariationForFeatureInRollout(@Nonnull FeatureFlag featureFlag, - @Nonnull String userId, - @Nonnull Map filteredAttributes, - @Nonnull ProjectConfig projectConfig) { - return getVariationForFeatureInRollout(featureFlag, userId, filteredAttributes, projectConfig, DefaultDecisionReasons.newInstance()); + return new DecisionResponse(new FeatureDecision(null, null, null), reasons); } /** @@ -305,14 +336,14 @@ FeatureDecision getVariationForFeatureInRollout(@Nonnull FeatureFlag featureFlag * * @param experiment {@link Experiment} in which user is to be bucketed. * @param userId User Identifier - * @param reasons Decision log messages * @return null if the user is not whitelisted into any variation * {@link Variation} the user is bucketed into if the user has a specified whitelisted variation. */ @Nullable - Variation getWhitelistedVariation(@Nonnull Experiment experiment, - @Nonnull String userId, - @Nonnull DecisionReasons reasons) { + DecisionResponse getWhitelistedVariation(@Nonnull Experiment experiment, + @Nonnull String userId) { + DecisionReasons reasons = DefaultDecisionReasons.newInstance(); + // if a user has a forced variation mapping, return the respective variation Map userIdToVariationKeyMap = experiment.getUserIdToVariationKeyMap(); if (userIdToVariationKeyMap.containsKey(userId)) { @@ -326,16 +357,9 @@ Variation getWhitelistedVariation(@Nonnull Experiment experiment, forcedVariationKey, userId); logger.error(message); } - return forcedVariation; + return new DecisionResponse(forcedVariation, reasons); } - return null; - } - - @Nullable - Variation getWhitelistedVariation(@Nonnull Experiment experiment, - @Nonnull String userId) { - - return getWhitelistedVariation(experiment, userId, DefaultDecisionReasons.newInstance()); + return new DecisionResponse(null, reasons); } /** @@ -344,15 +368,14 @@ Variation getWhitelistedVariation(@Nonnull Experiment experiment, * @param experiment {@link Experiment} in which the user was bucketed. * @param userProfile {@link UserProfile} of the user. * @param projectConfig The current projectConfig - * @param reasons Decision log messages * @return null if the {@link UserProfileService} implementation is null or the user was not previously bucketed. * else return the {@link Variation} the user was previously bucketed into. */ @Nullable - Variation getStoredVariation(@Nonnull Experiment experiment, - @Nonnull UserProfile userProfile, - @Nonnull ProjectConfig projectConfig, - @Nonnull DecisionReasons reasons) { + DecisionResponse getStoredVariation(@Nonnull Experiment experiment, + @Nonnull UserProfile userProfile, + @Nonnull ProjectConfig projectConfig) { + DecisionReasons reasons = DefaultDecisionReasons.newInstance(); // ---------- Check User Profile for Sticky Bucketing ---------- // If a user profile instance is present then check it for a saved variation @@ -371,40 +394,31 @@ Variation getStoredVariation(@Nonnull Experiment experiment, savedVariation.getKey(), experimentKey, userProfile.userId); logger.info(message); // A variation is stored for this combined bucket id - return savedVariation; + return new DecisionResponse(savedVariation, reasons); } else { String message = reasons.addInfo("User \"%s\" was previously bucketed into variation with ID \"%s\" for experiment \"%s\", but no matching variation was found for that user. We will re-bucket the user.", userProfile.userId, variationId, experimentKey); logger.info(message); - return null; + return new DecisionResponse(null, reasons); } } else { String message = reasons.addInfo("No previously activated variation of experiment \"%s\" for user \"%s\" found in user profile.", experimentKey, userProfile.userId); logger.info(message); - return null; + return new DecisionResponse(null, reasons); } } - @Nullable - Variation getStoredVariation(@Nonnull Experiment experiment, - @Nonnull UserProfile userProfile, - @Nonnull ProjectConfig projectConfig) { - return getStoredVariation(experiment, userProfile, projectConfig, DefaultDecisionReasons.newInstance()); - } - /** * Save a {@link Variation} of an {@link Experiment} for a user in the {@link UserProfileService}. * * @param experiment The experiment the user was buck * @param variation The Variation to save. * @param userProfile A {@link UserProfile} instance of the user information. - * @param reasons Decision log messages */ void saveVariation(@Nonnull Experiment experiment, @Nonnull Variation variation, - @Nonnull UserProfile userProfile, - @Nonnull DecisionReasons reasons) { + @Nonnull UserProfile userProfile) { // only save if the user has implemented a user profile service if (userProfileService != null) { @@ -431,42 +445,28 @@ void saveVariation(@Nonnull Experiment experiment, } } - void saveVariation(@Nonnull Experiment experiment, - @Nonnull Variation variation, - @Nonnull UserProfile userProfile) { - saveVariation(experiment, variation, userProfile, DefaultDecisionReasons.newInstance()); - } - /** * Get the bucketingId of a user if a bucketingId exists in attributes, or else default to userId. * * @param userId The userId of the user. * @param filteredAttributes The user's attributes. This should be filtered to just attributes in the Datafile. - * @param reasons Decision log messages * @return bucketingId if it is a String type in attributes. * else return userId */ String getBucketingId(@Nonnull String userId, - @Nonnull Map filteredAttributes, - @Nonnull DecisionReasons reasons) { + @Nonnull Map filteredAttributes) { String bucketingId = userId; if (filteredAttributes != null && filteredAttributes.containsKey(ControlAttribute.BUCKETING_ATTRIBUTE.toString())) { if (String.class.isInstance(filteredAttributes.get(ControlAttribute.BUCKETING_ATTRIBUTE.toString()))) { bucketingId = (String) filteredAttributes.get(ControlAttribute.BUCKETING_ATTRIBUTE.toString()); logger.debug("BucketingId is valid: \"{}\"", bucketingId); } else { - String message = reasons.addInfo("BucketingID attribute is not a string. Defaulted to userId"); - logger.warn(message); + logger.warn("BucketingID attribute is not a string. Defaulted to userId"); } } return bucketingId; } - String getBucketingId(@Nonnull String userId, - @Nonnull Map filteredAttributes) { - return getBucketingId(userId, filteredAttributes, DefaultDecisionReasons.newInstance()); - } - public ConcurrentHashMap> getForcedVariationMapping() { return forcedVariationMapping; } @@ -500,6 +500,7 @@ public boolean setForcedVariation(@Nonnull Experiment experiment, // if the user id is invalid, return false. if (!validateUserId(userId)) { + logger.error("User ID is invalid"); return false; } @@ -545,17 +546,18 @@ public boolean setForcedVariation(@Nonnull Experiment experiment, * * @param experiment The experiment forced. * @param userId The user ID to be used for bucketing. - * @param reasons Decision log messages * @return The variation the user was bucketed into. This value can be null if the * forced variation fails. */ @Nullable - public Variation getForcedVariation(@Nonnull Experiment experiment, - @Nonnull String userId, - @Nonnull DecisionReasons reasons) { - // if the user id is invalid, return false. + public DecisionResponse getForcedVariation(@Nonnull Experiment experiment, + @Nonnull String userId) { + DecisionReasons reasons = DefaultDecisionReasons.newInstance(); + if (!validateUserId(userId)) { - return null; + String message = reasons.addInfo("User ID is invalid"); + logger.error(message); + return new DecisionResponse(null, reasons); } Map experimentToVariation = getForcedVariationMapping().get(userId); @@ -567,19 +569,13 @@ public Variation getForcedVariation(@Nonnull Experiment experiment, String message = reasons.addInfo("Variation \"%s\" is mapped to experiment \"%s\" and user \"%s\" in the forced variation map", variation.getKey(), experiment.getKey(), userId); logger.debug(message); - return variation; + return new DecisionResponse(variation, reasons); } } else { logger.debug("No variation for experiment \"{}\" mapped to user \"{}\" in the forced variation map ", experiment.getKey(), userId); } } - return null; - } - - @Nullable - public Variation getForcedVariation(@Nonnull Experiment experiment, - @Nonnull String userId) { - return getForcedVariation(experiment, userId, DefaultDecisionReasons.newInstance()); + return new DecisionResponse(null, reasons); } /** @@ -589,12 +585,7 @@ public Variation getForcedVariation(@Nonnull Experiment experiment, * @return whether the user ID is valid */ private boolean validateUserId(String userId) { - if (userId == null) { - logger.error("User ID is invalid"); - return false; - } - - return true; + return (userId != null); } } diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/AndCondition.java b/core-api/src/main/java/com/optimizely/ab/config/audience/AndCondition.java index 20c15e95d..8b458d059 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/AndCondition.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/AndCondition.java @@ -17,10 +17,10 @@ package com.optimizely.ab.config.audience; import com.optimizely.ab.config.ProjectConfig; -import com.optimizely.ab.optimizelydecision.DecisionReasons; import javax.annotation.Nonnull; import javax.annotation.Nullable; +import javax.annotation.concurrent.Immutable; import java.util.List; import java.util.Map; @@ -40,9 +40,7 @@ public List getConditions() { } @Nullable - public Boolean evaluate(ProjectConfig config, - Map attributes, - DecisionReasons reasons) { + public Boolean evaluate(ProjectConfig config, Map attributes) { if (conditions == null) return null; boolean foundNull = false; // According to the matrix where: @@ -53,7 +51,7 @@ public Boolean evaluate(ProjectConfig config, // true and true is true // null and null is null for (Condition condition : conditions) { - Boolean conditionEval = condition.evaluate(config, attributes, reasons); + Boolean conditionEval = condition.evaluate(config, attributes); if (conditionEval == null) { foundNull = true; } else if (!conditionEval) { // false with nulls or trues is false. diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/AudienceIdCondition.java b/core-api/src/main/java/com/optimizely/ab/config/audience/AudienceIdCondition.java index fb076a90d..57a4e5bec 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/AudienceIdCondition.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/AudienceIdCondition.java @@ -18,12 +18,14 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonIgnoreProperties; +import com.fasterxml.jackson.annotation.JsonProperty; import com.optimizely.ab.config.ProjectConfig; -import com.optimizely.ab.optimizelydecision.DecisionReasons; +import com.optimizely.ab.internal.InvalidAudienceCondition; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import javax.annotation.Nullable; +import javax.annotation.concurrent.Immutable; import java.util.Map; import java.util.Objects; @@ -64,19 +66,16 @@ public String getAudienceId() { @Nullable @Override - public Boolean evaluate(ProjectConfig config, - Map attributes, - DecisionReasons reasons) { + public Boolean evaluate(ProjectConfig config, Map attributes) { if (config != null) { audience = config.getAudienceIdMapping().get(audienceId); } if (audience == null) { - String message = reasons.addInfo("Audience %s could not be found.", audienceId); - logger.error(message); + logger.error("Audience {} could not be found.", audienceId); return null; } logger.debug("Starting to evaluate audience \"{}\" with conditions: {}.", audience.getId(), audience.getConditions()); - Boolean result = audience.getConditions().evaluate(config, attributes, reasons); + Boolean result = audience.getConditions().evaluate(config, attributes); logger.debug("Audience \"{}\" evaluated to {}.", audience.getId(), result); return result; } diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/Condition.java b/core-api/src/main/java/com/optimizely/ab/config/audience/Condition.java index 4d108214c..772d2b03e 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/Condition.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/Condition.java @@ -17,7 +17,6 @@ package com.optimizely.ab.config.audience; import com.optimizely.ab.config.ProjectConfig; -import com.optimizely.ab.optimizelydecision.DecisionReasons; import javax.annotation.Nullable; import java.util.Map; @@ -28,8 +27,5 @@ public interface Condition { @Nullable - Boolean evaluate(ProjectConfig config, - Map attributes, - DecisionReasons reasons); - + Boolean evaluate(ProjectConfig config, Map attributes); } diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/EmptyCondition.java b/core-api/src/main/java/com/optimizely/ab/config/audience/EmptyCondition.java index b5978d200..8f8aedeae 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/EmptyCondition.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/EmptyCondition.java @@ -16,7 +16,6 @@ package com.optimizely.ab.config.audience; import com.optimizely.ab.config.ProjectConfig; -import com.optimizely.ab.optimizelydecision.DecisionReasons; import javax.annotation.Nullable; import java.util.Map; @@ -24,9 +23,7 @@ public class EmptyCondition implements Condition { @Nullable @Override - public Boolean evaluate(ProjectConfig config, - Map attributes, - DecisionReasons reasons) { + public Boolean evaluate(ProjectConfig config, Map attributes) { return true; } diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/NotCondition.java b/core-api/src/main/java/com/optimizely/ab/config/audience/NotCondition.java index 8a523bb8d..b7f45f2ac 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/NotCondition.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/NotCondition.java @@ -17,11 +17,11 @@ package com.optimizely.ab.config.audience; import com.optimizely.ab.config.ProjectConfig; -import com.optimizely.ab.optimizelydecision.DecisionReasons; -import javax.annotation.Nonnull; import javax.annotation.Nullable; import javax.annotation.concurrent.Immutable; +import javax.annotation.Nonnull; + import java.util.Map; /** @@ -41,11 +41,9 @@ public Condition getCondition() { } @Nullable - public Boolean evaluate(ProjectConfig config, - Map attributes, - DecisionReasons reasons) { + public Boolean evaluate(ProjectConfig config, Map attributes) { - Boolean conditionEval = condition == null ? null : condition.evaluate(config, attributes, reasons); + Boolean conditionEval = condition == null ? null : condition.evaluate(config, attributes); return (conditionEval == null ? null : !conditionEval); } diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/NullCondition.java b/core-api/src/main/java/com/optimizely/ab/config/audience/NullCondition.java index ef76d92ad..fcf5100db 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/NullCondition.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/NullCondition.java @@ -16,7 +16,6 @@ package com.optimizely.ab.config.audience; import com.optimizely.ab.config.ProjectConfig; -import com.optimizely.ab.optimizelydecision.DecisionReasons; import javax.annotation.Nullable; import java.util.Map; @@ -24,10 +23,7 @@ public class NullCondition implements Condition { @Nullable @Override - public Boolean evaluate(ProjectConfig config, - Map attributes, - DecisionReasons reasons) { + public Boolean evaluate(ProjectConfig config, Map attributes) { return null; } - } diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/OrCondition.java b/core-api/src/main/java/com/optimizely/ab/config/audience/OrCondition.java index b2c2f0afe..70572a9a9 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/OrCondition.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/OrCondition.java @@ -17,7 +17,6 @@ package com.optimizely.ab.config.audience; import com.optimizely.ab.config.ProjectConfig; -import com.optimizely.ab.optimizelydecision.DecisionReasons; import javax.annotation.Nonnull; import javax.annotation.Nullable; @@ -46,13 +45,11 @@ public List getConditions() { // false or false is false // null or null is null @Nullable - public Boolean evaluate(ProjectConfig config, - Map attributes, - DecisionReasons reasons) { + public Boolean evaluate(ProjectConfig config, Map attributes) { if (conditions == null) return null; boolean foundNull = false; for (Condition condition : conditions) { - Boolean conditionEval = condition.evaluate(config, attributes, reasons); + Boolean conditionEval = condition.evaluate(config, attributes); if (conditionEval == null) { // true with falses and nulls is still true foundNull = true; } else if (conditionEval) { diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/UserAttribute.java b/core-api/src/main/java/com/optimizely/ab/config/audience/UserAttribute.java index 152bb7048..277f2f184 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/UserAttribute.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/UserAttribute.java @@ -21,7 +21,6 @@ import com.fasterxml.jackson.annotation.JsonProperty; import com.optimizely.ab.config.ProjectConfig; import com.optimizely.ab.config.audience.match.*; -import com.optimizely.ab.optimizelydecision.DecisionReasons; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -72,9 +71,7 @@ public Object getValue() { } @Nullable - public Boolean evaluate(ProjectConfig config, - Map attributes, - DecisionReasons reasons) { + public Boolean evaluate(ProjectConfig config, Map attributes) { if (attributes == null) { attributes = Collections.emptyMap(); } @@ -82,8 +79,7 @@ public Boolean evaluate(ProjectConfig config, Object userAttributeValue = attributes.get(name); if (!"custom_attribute".equals(type)) { - String message = reasons.addInfo("Audience condition \"%s\" uses an unknown condition type. You may need to upgrade to a newer release of the Optimizely SDK.", this); - logger.warn(message); + logger.warn("Audience condition \"{}\" uses an unknown condition type. You may need to upgrade to a newer release of the Optimizely SDK.", this); return null; // unknown type } // check user attribute value is equal @@ -98,27 +94,26 @@ public Boolean evaluate(ProjectConfig config, } catch(UnknownValueTypeException e) { if (!attributes.containsKey(name)) { //Missing attribute value - String message = reasons.addInfo("Audience condition \"%s\" evaluated to UNKNOWN because no value was passed for user attribute \"%s\"", this, name); - logger.debug(message); + logger.debug("Audience condition \"{}\" evaluated to UNKNOWN because no value was passed for user attribute \"{}\"", this, name); } else { //if attribute value is not valid if (userAttributeValue != null) { - String message = reasons.addInfo("Audience condition \"%s\" evaluated to UNKNOWN because a value of type \"%s\" was passed for user attribute \"%s\"", + logger.warn( + "Audience condition \"{}\" evaluated to UNKNOWN because a value of type \"{}\" was passed for user attribute \"{}\"", this, userAttributeValue.getClass().getCanonicalName(), name); - logger.warn(message); } else { - String message = reasons.addInfo("Audience condition \"%s\" evaluated to UNKNOWN because a null value was passed for user attribute \"%s\"", this, name); - logger.debug(message); + logger.debug( + "Audience condition \"{}\" evaluated to UNKNOWN because a null value was passed for user attribute \"{}\"", + this, + name); } } } catch (UnknownMatchTypeException | UnexpectedValueTypeException e) { - String message = reasons.addInfo("Audience condition \"%s\" " + e.getMessage(), this); - logger.warn(message); + logger.warn("Audience condition \"{}\" " + e.getMessage(), this); } catch (NullPointerException e) { - String message = reasons.addInfo("attribute or value null for match %s", match != null ? match : "legacy condition"); - logger.error(message, e); + logger.error("attribute or value null for match {}", match != null ? match : "legacy condition", e); } return null; } diff --git a/core-api/src/main/java/com/optimizely/ab/internal/ExperimentUtils.java b/core-api/src/main/java/com/optimizely/ab/internal/ExperimentUtils.java index e24cc1b6a..133ed5992 100644 --- a/core-api/src/main/java/com/optimizely/ab/internal/ExperimentUtils.java +++ b/core-api/src/main/java/com/optimizely/ab/internal/ExperimentUtils.java @@ -22,6 +22,7 @@ import com.optimizely.ab.config.audience.Condition; import com.optimizely.ab.config.audience.OrCondition; import com.optimizely.ab.optimizelydecision.DecisionReasons; +import com.optimizely.ab.optimizelydecision.DecisionResponse; import com.optimizely.ab.optimizelydecision.DefaultDecisionReasons; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -39,29 +40,6 @@ public final class ExperimentUtils { private ExperimentUtils() { } - /** - * Helper method to validate all pre-conditions before bucketing a user. - * - * @param experiment the experiment we are validating pre-conditions for - * @param reasons Decision log messages - * @return whether the pre-conditions are satisfied - */ - public static boolean isExperimentActive(@Nonnull Experiment experiment, - @Nonnull DecisionReasons reasons) { - - if (!experiment.isActive()) { - String message = reasons.addInfo("Experiment \"%s\" is not running.", experiment.getKey()); - logger.info(message); - return false; - } - - return true; - } - - public static boolean isExperimentActive(@Nonnull Experiment experiment) { - return isExperimentActive(experiment, DefaultDecisionReasons.newInstance()); - } - /** * Determines whether a user satisfies audience conditions for the experiment. * @@ -70,55 +48,44 @@ public static boolean isExperimentActive(@Nonnull Experiment experiment) { * @param attributes the attributes of the user * @param loggingEntityType It can be either experiment or rule. * @param loggingKey In case of loggingEntityType is experiment it will be experiment key or else it will be rule number. - * @param reasons Decision log messages * @return whether the user meets the criteria for the experiment */ - public static boolean doesUserMeetAudienceConditions(@Nonnull ProjectConfig projectConfig, - @Nonnull Experiment experiment, - @Nonnull Map attributes, - @Nonnull String loggingEntityType, - @Nonnull String loggingKey, - @Nonnull DecisionReasons reasons) { + public static DecisionResponse doesUserMeetAudienceConditions(@Nonnull ProjectConfig projectConfig, + @Nonnull Experiment experiment, + @Nonnull Map attributes, + @Nonnull String loggingEntityType, + @Nonnull String loggingKey) { + DecisionReasons reasons = DefaultDecisionReasons.newInstance(); + + DecisionResponse decisionResponse; if (experiment.getAudienceConditions() != null) { logger.debug("Evaluating audiences for {} \"{}\": {}.", loggingEntityType, loggingKey, experiment.getAudienceConditions()); - Boolean resolveReturn = evaluateAudienceConditions(projectConfig, experiment, attributes, loggingEntityType, loggingKey, reasons); - return resolveReturn == null ? false : resolveReturn; + decisionResponse = evaluateAudienceConditions(projectConfig, experiment, attributes, loggingEntityType, loggingKey); } else { - Boolean resolveReturn = evaluateAudience(projectConfig, experiment, attributes, loggingEntityType, loggingKey, reasons); - return Boolean.TRUE.equals(resolveReturn); + decisionResponse = evaluateAudience(projectConfig, experiment, attributes, loggingEntityType, loggingKey); } - } - /** - * Determines whether a user satisfies audience conditions for the experiment. - * - * @param projectConfig the current projectConfig - * @param experiment the experiment we are evaluating audiences for - * @param attributes the attributes of the user - * @param loggingEntityType It can be either experiment or rule. - * @param loggingKey In case of loggingEntityType is experiment it will be experiment key or else it will be rule number. - * @return whether the user meets the criteria for the experiment - */ - public static boolean doesUserMeetAudienceConditions(@Nonnull ProjectConfig projectConfig, - @Nonnull Experiment experiment, - @Nonnull Map attributes, - @Nonnull String loggingEntityType, - @Nonnull String loggingKey) { - return doesUserMeetAudienceConditions(projectConfig, experiment, attributes, loggingEntityType, loggingKey, DefaultDecisionReasons.newInstance()); + Boolean resolveReturn = decisionResponse.getResult(); + reasons.merge(decisionResponse.getReasons()); + + return new DecisionResponse( + resolveReturn == null ? false : resolveReturn, + reasons); } @Nullable - public static Boolean evaluateAudience(@Nonnull ProjectConfig projectConfig, - @Nonnull Experiment experiment, - @Nonnull Map attributes, - @Nonnull String loggingEntityType, - @Nonnull String loggingKey, - @Nonnull DecisionReasons reasons) { + public static DecisionResponse evaluateAudience(@Nonnull ProjectConfig projectConfig, + @Nonnull Experiment experiment, + @Nonnull Map attributes, + @Nonnull String loggingEntityType, + @Nonnull String loggingKey) { + DecisionReasons reasons = DefaultDecisionReasons.newInstance(); + List experimentAudienceIds = experiment.getAudienceIds(); // if there are no audiences, ALL users should be part of the experiment if (experimentAudienceIds.isEmpty()) { - return true; + return new DecisionResponse(true, reasons); } List conditions = new ArrayList<>(); @@ -131,35 +98,35 @@ public static Boolean evaluateAudience(@Nonnull ProjectConfig projectConfig, logger.debug("Evaluating audiences for {} \"{}\": {}.", loggingEntityType, loggingKey, conditions); - Boolean result = implicitOr.evaluate(projectConfig, attributes, reasons); - + Boolean result = implicitOr.evaluate(projectConfig, attributes); String message = reasons.addInfo("Audiences for %s \"%s\" collectively evaluated to %s.", loggingEntityType, loggingKey, result); logger.info(message); - return result; + return new DecisionResponse(result, reasons); } @Nullable - public static Boolean evaluateAudienceConditions(@Nonnull ProjectConfig projectConfig, - @Nonnull Experiment experiment, - @Nonnull Map attributes, - @Nonnull String loggingEntityType, - @Nonnull String loggingKey, - @Nonnull DecisionReasons reasons) { + public static DecisionResponse evaluateAudienceConditions(@Nonnull ProjectConfig projectConfig, + @Nonnull Experiment experiment, + @Nonnull Map attributes, + @Nonnull String loggingEntityType, + @Nonnull String loggingKey) { + DecisionReasons reasons = DefaultDecisionReasons.newInstance(); Condition conditions = experiment.getAudienceConditions(); if (conditions == null) return null; + Boolean result = null; try { - Boolean result = conditions.evaluate(projectConfig, attributes, reasons); + result = conditions.evaluate(projectConfig, attributes); String message = reasons.addInfo("Audiences for %s \"%s\" collectively evaluated to %s.", loggingEntityType, loggingKey, result); logger.info(message); - return result; } catch (Exception e) { String message = reasons.addInfo("Condition invalid: %s", e.getMessage()); logger.error(message); - return null; } + + return new DecisionResponse(result, reasons); } } diff --git a/core-api/src/main/java/com/optimizely/ab/optimizelydecision/DecisionReasons.java b/core-api/src/main/java/com/optimizely/ab/optimizelydecision/DecisionReasons.java index 0983ee4d2..c8311d1bc 100644 --- a/core-api/src/main/java/com/optimizely/ab/optimizelydecision/DecisionReasons.java +++ b/core-api/src/main/java/com/optimizely/ab/optimizelydecision/DecisionReasons.java @@ -16,14 +16,34 @@ */ package com.optimizely.ab.optimizelydecision; +import java.util.ArrayList; import java.util.List; -public interface DecisionReasons { +public class DecisionReasons { - public void addError(String format, Object... args); + private final List errors = new ArrayList<>(); + private final List infos = new ArrayList<>(); - public String addInfo(String format, Object... args); + public void addError(String format, Object... args) { + String message = String.format(format, args); + errors.add(message); + } - public List toReport(); + public String addInfo(String format, Object... args) { + String message = String.format(format, args); + infos.add(message); + return message; + } + + public void merge(DecisionReasons target) { + errors.addAll(target.errors); + infos.addAll(target.infos); + } + + public List toReport() { + List reasons = new ArrayList<>(errors); + reasons.addAll(infos); + return reasons; + } } diff --git a/core-api/src/main/java/com/optimizely/ab/optimizelydecision/DecisionResponse.java b/core-api/src/main/java/com/optimizely/ab/optimizelydecision/DecisionResponse.java new file mode 100644 index 000000000..3f85e827e --- /dev/null +++ b/core-api/src/main/java/com/optimizely/ab/optimizelydecision/DecisionResponse.java @@ -0,0 +1,24 @@ +package com.optimizely.ab.optimizelydecision; + +import javax.annotation.Nonnull; +import javax.annotation.Nullable; + +public class DecisionResponse { + private T result; + private DecisionReasons reasons; + + public DecisionResponse(@Nullable T result, @Nonnull DecisionReasons reasons) { + this.result = result; + this.reasons = reasons; + } + + @Nullable + public T getResult() { + return result; + } + + @Nonnull + public DecisionReasons getReasons() { + return reasons; + } +} diff --git a/core-api/src/main/java/com/optimizely/ab/optimizelydecision/DefaultDecisionReasons.java b/core-api/src/main/java/com/optimizely/ab/optimizelydecision/DefaultDecisionReasons.java index dd84d04fe..f00f5ae66 100644 --- a/core-api/src/main/java/com/optimizely/ab/optimizelydecision/DefaultDecisionReasons.java +++ b/core-api/src/main/java/com/optimizely/ab/optimizelydecision/DefaultDecisionReasons.java @@ -1,3 +1,19 @@ +/** + * + * Copyright 2020, 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. + */ /** * * Copyright 2020, Optimizely and contributors @@ -17,38 +33,23 @@ package com.optimizely.ab.optimizelydecision; import javax.annotation.Nullable; -import java.util.ArrayList; import java.util.List; -public class DefaultDecisionReasons implements DecisionReasons { - - private final List errors = new ArrayList<>(); - private final List infos = new ArrayList<>(); +public class DefaultDecisionReasons extends DecisionReasons { public static DecisionReasons newInstance(@Nullable List options) { - if (options != null && options.contains(OptimizelyDecideOption.INCLUDE_REASONS)) return new DefaultDecisionReasons(); - else return new ErrorsDecisionReasons(); + if (options != null && options.contains(OptimizelyDecideOption.INCLUDE_REASONS)) return new DecisionReasons(); + else return new DefaultDecisionReasons(); } public static DecisionReasons newInstance() { return newInstance(null); } - public void addError(String format, Object... args) { - String message = String.format(format, args); - errors.add(message); - } - + @Override public String addInfo(String format, Object... args) { - String message = String.format(format, args); - infos.add(message); - return message; - } - - public List toReport() { - List reasons = new ArrayList<>(errors); - reasons.addAll(infos); - return reasons; + // skip tracking and pass-through reasons other than critical errors. + return String.format(format, args); } } diff --git a/core-api/src/main/java/com/optimizely/ab/optimizelydecision/ErrorsDecisionReasons.java b/core-api/src/main/java/com/optimizely/ab/optimizelydecision/ErrorsDecisionReasons.java deleted file mode 100644 index 91875eece..000000000 --- a/core-api/src/main/java/com/optimizely/ab/optimizelydecision/ErrorsDecisionReasons.java +++ /dev/null @@ -1,56 +0,0 @@ -/** - * - * Copyright 2020, 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. - */ -/** - * - * Copyright 2020, 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.optimizelydecision; - -import java.util.ArrayList; -import java.util.List; - -public class ErrorsDecisionReasons implements DecisionReasons { - - private final List errors = new ArrayList<>(); - - public void addError(String format, Object... args) { - String message = String.format(format, args); - errors.add(message); - } - - public String addInfo(String format, Object... args) { - // skip tracking and pass-through reasons other than critical errors. - return String.format(format, args); - } - - public List toReport() { - return new ArrayList<>(errors); - } - -} diff --git a/core-api/src/test/java/com/optimizely/ab/config/audience/AudienceConditionEvaluationTest.java b/core-api/src/test/java/com/optimizely/ab/config/audience/AudienceConditionEvaluationTest.java index 216099298..35acf1783 100644 --- a/core-api/src/test/java/com/optimizely/ab/config/audience/AudienceConditionEvaluationTest.java +++ b/core-api/src/test/java/com/optimizely/ab/config/audience/AudienceConditionEvaluationTest.java @@ -19,7 +19,6 @@ import ch.qos.logback.classic.Level; import com.optimizely.ab.internal.LogbackVerifier; import com.optimizely.ab.optimizelydecision.DecisionReasons; -import com.optimizely.ab.optimizelydecision.DefaultDecisionReasons; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import org.junit.Before; import org.junit.Rule; @@ -44,7 +43,7 @@ public class AudienceConditionEvaluationTest { Map testUserAttributes; Map testTypedUserAttributes; - DecisionReasons reasons = DefaultDecisionReasons.newInstance(); + DecisionReasons reasons = DecisionReasons.newInstance(); @Before public void initialize() { From da53ecaf90f785d08fa0f3e678fd432743ed9313 Mon Sep 17 00:00:00 2001 From: Jae Kim Date: Thu, 17 Dec 2020 15:31:06 -0800 Subject: [PATCH 2/9] recover audience testing --- .../ab/bucketing/DecisionService.java | 2 +- .../ab/internal/ExperimentUtils.java | 10 + .../AudienceConditionEvaluationTest.java | 443 +++++++++--------- 3 files changed, 231 insertions(+), 224 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 55c363c13..d00eaf6bb 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 @@ -97,7 +97,7 @@ public DecisionResponse getVariation(@Nonnull Experiment experiment, @Nonnull List options) { DecisionReasons reasons = DefaultDecisionReasons.newInstance(); - if (!experiment.isActive()) { + if (!ExperimentUtils.isExperimentActive(experiment)) { String message = reasons.addInfo("Experiment \"%s\" is not running.", experiment.getKey()); logger.info(message); return new DecisionResponse(null, reasons); diff --git a/core-api/src/main/java/com/optimizely/ab/internal/ExperimentUtils.java b/core-api/src/main/java/com/optimizely/ab/internal/ExperimentUtils.java index 133ed5992..b595a7677 100644 --- a/core-api/src/main/java/com/optimizely/ab/internal/ExperimentUtils.java +++ b/core-api/src/main/java/com/optimizely/ab/internal/ExperimentUtils.java @@ -40,6 +40,16 @@ public final class ExperimentUtils { private ExperimentUtils() { } + /** + * Helper method to validate all pre-conditions before bucketing a user. + * + * @param experiment the experiment we are validating pre-conditions for + * @return whether the pre-conditions are satisfied + */ + public static boolean isExperimentActive(@Nonnull Experiment experiment) { + return experiment.isActive(); + } + /** * Determines whether a user satisfies audience conditions for the experiment. * diff --git a/core-api/src/test/java/com/optimizely/ab/config/audience/AudienceConditionEvaluationTest.java b/core-api/src/test/java/com/optimizely/ab/config/audience/AudienceConditionEvaluationTest.java index 35acf1783..0a6e41ddc 100644 --- a/core-api/src/test/java/com/optimizely/ab/config/audience/AudienceConditionEvaluationTest.java +++ b/core-api/src/test/java/com/optimizely/ab/config/audience/AudienceConditionEvaluationTest.java @@ -18,7 +18,6 @@ import ch.qos.logback.classic.Level; import com.optimizely.ab.internal.LogbackVerifier; -import com.optimizely.ab.optimizelydecision.DecisionReasons; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import org.junit.Before; import org.junit.Rule; @@ -43,8 +42,6 @@ public class AudienceConditionEvaluationTest { Map testUserAttributes; Map testTypedUserAttributes; - DecisionReasons reasons = DecisionReasons.newInstance(); - @Before public void initialize() { testUserAttributes = new HashMap<>(); @@ -69,7 +66,7 @@ public void userAttributeEvaluateTrue() throws Exception { assertNull(testInstance.getMatch()); assertEquals(testInstance.getName(), "browser_type"); assertEquals(testInstance.getType(), "custom_attribute"); - assertTrue(testInstance.evaluate(null, testUserAttributes, reasons)); + assertTrue(testInstance.evaluate(null, testUserAttributes)); } /** @@ -78,7 +75,7 @@ public void userAttributeEvaluateTrue() throws Exception { @Test public void userAttributeEvaluateFalse() throws Exception { UserAttribute testInstance = new UserAttribute("browser_type", "custom_attribute", null, "firefox"); - assertFalse(testInstance.evaluate(null, testUserAttributes, reasons)); + assertFalse(testInstance.evaluate(null, testUserAttributes)); } /** @@ -87,7 +84,7 @@ public void userAttributeEvaluateFalse() throws Exception { @Test public void userAttributeUnknownAttribute() throws Exception { UserAttribute testInstance = new UserAttribute("unknown_dim", "custom_attribute", null, "unknown"); - assertFalse(testInstance.evaluate(null, testUserAttributes, reasons)); + assertFalse(testInstance.evaluate(null, testUserAttributes)); } /** @@ -96,7 +93,7 @@ public void userAttributeUnknownAttribute() throws Exception { @Test public void invalidMatchCondition() throws Exception { UserAttribute testInstance = new UserAttribute("browser_type", "unknown_dimension", null, "chrome"); - assertNull(testInstance.evaluate(null, testUserAttributes, reasons)); + assertNull(testInstance.evaluate(null, testUserAttributes)); } /** @@ -105,7 +102,7 @@ public void invalidMatchCondition() throws Exception { @Test public void invalidMatch() throws Exception { UserAttribute testInstance = new UserAttribute("browser_type", "custom_attribute", "blah", "chrome"); - assertNull(testInstance.evaluate(null, testUserAttributes, reasons)); + assertNull(testInstance.evaluate(null, testUserAttributes)); logbackVerifier.expectMessage(Level.WARN, "Audience condition \"{name='browser_type', type='custom_attribute', match='blah', value='chrome'}\" uses an unknown match type. You may need to upgrade to a newer release of the Optimizely SDK"); } @@ -116,7 +113,7 @@ public void invalidMatch() throws Exception { @Test public void unexpectedAttributeType() throws Exception { UserAttribute testInstance = new UserAttribute("browser_type", "custom_attribute", "gt", 20); - assertNull(testInstance.evaluate(null, testUserAttributes, reasons)); + assertNull(testInstance.evaluate(null, testUserAttributes)); logbackVerifier.expectMessage(Level.WARN, "Audience condition \"{name='browser_type', type='custom_attribute', match='gt', value=20}\" evaluated to UNKNOWN because a value of type \"java.lang.String\" was passed for user attribute \"browser_type\""); } @@ -127,7 +124,7 @@ public void unexpectedAttributeType() throws Exception { @Test public void unexpectedAttributeTypeNull() throws Exception { UserAttribute testInstance = new UserAttribute("browser_type", "custom_attribute", "gt", 20); - assertNull(testInstance.evaluate(null, Collections.singletonMap("browser_type", null), reasons)); + assertNull(testInstance.evaluate(null, Collections.singletonMap("browser_type", null))); logbackVerifier.expectMessage(Level.DEBUG, "Audience condition \"{name='browser_type', type='custom_attribute', match='gt', value=20}\" evaluated to UNKNOWN because a null value was passed for user attribute \"browser_type\""); } @@ -139,7 +136,7 @@ public void unexpectedAttributeTypeNull() throws Exception { @Test public void missingAttribute() throws Exception { UserAttribute testInstance = new UserAttribute("browser_type", "custom_attribute", "gt", 20); - assertNull(testInstance.evaluate(null, Collections.EMPTY_MAP, reasons)); + assertNull(testInstance.evaluate(null, Collections.EMPTY_MAP)); logbackVerifier.expectMessage(Level.DEBUG, "Audience condition \"{name='browser_type', type='custom_attribute', match='gt', value=20}\" evaluated to UNKNOWN because no value was passed for user attribute \"browser_type\""); } @@ -150,7 +147,7 @@ public void missingAttribute() throws Exception { @Test public void nullAttribute() throws Exception { UserAttribute testInstance = new UserAttribute("browser_type", "custom_attribute", "gt", 20); - assertNull(testInstance.evaluate(null, null, reasons)); + assertNull(testInstance.evaluate(null, null)); logbackVerifier.expectMessage(Level.DEBUG, "Audience condition \"{name='browser_type', type='custom_attribute', match='gt', value=20}\" evaluated to UNKNOWN because no value was passed for user attribute \"browser_type\""); } @@ -161,7 +158,7 @@ public void nullAttribute() throws Exception { @Test public void unknownConditionType() throws Exception { UserAttribute testInstance = new UserAttribute("browser_type", "blah", "exists", "firefox"); - assertNull(testInstance.evaluate(null, testUserAttributes, reasons)); + assertNull(testInstance.evaluate(null, testUserAttributes)); logbackVerifier.expectMessage(Level.WARN, "Audience condition \"{name='browser_type', type='blah', match='exists', value='firefox'}\" uses an unknown condition type. You may need to upgrade to a newer release of the Optimizely SDK."); } @@ -175,9 +172,9 @@ public void existsMatchConditionEmptyStringEvaluatesTrue() throws Exception { UserAttribute testInstance = new UserAttribute("browser_type", "custom_attribute", "exists", "firefox"); Map attributes = new HashMap<>(); attributes.put("browser_type", ""); - assertTrue(testInstance.evaluate(null, attributes, reasons)); + assertTrue(testInstance.evaluate(null, attributes)); attributes.put("browser_type", null); - assertFalse(testInstance.evaluate(null, attributes, reasons)); + assertFalse(testInstance.evaluate(null, attributes)); } /** @@ -187,16 +184,16 @@ public void existsMatchConditionEmptyStringEvaluatesTrue() throws Exception { @Test public void existsMatchConditionEvaluatesTrue() throws Exception { UserAttribute testInstance = new UserAttribute("browser_type", "custom_attribute", "exists", "firefox"); - assertTrue(testInstance.evaluate(null, testUserAttributes, reasons)); + assertTrue(testInstance.evaluate(null, testUserAttributes)); UserAttribute testInstanceBoolean = new UserAttribute("is_firefox", "custom_attribute", "exists", false); UserAttribute testInstanceInteger = new UserAttribute("num_size", "custom_attribute", "exists", 5); UserAttribute testInstanceDouble = new UserAttribute("num_counts", "custom_attribute", "exists", 4.55); UserAttribute testInstanceObject = new UserAttribute("meta_data", "custom_attribute", "exists", testUserAttributes); - assertTrue(testInstanceBoolean.evaluate(null, testTypedUserAttributes, reasons)); - assertTrue(testInstanceInteger.evaluate(null, testTypedUserAttributes, reasons)); - assertTrue(testInstanceDouble.evaluate(null, testTypedUserAttributes, reasons)); - assertTrue(testInstanceObject.evaluate(null, testTypedUserAttributes, reasons)); + assertTrue(testInstanceBoolean.evaluate(null, testTypedUserAttributes)); + assertTrue(testInstanceInteger.evaluate(null, testTypedUserAttributes)); + assertTrue(testInstanceDouble.evaluate(null, testTypedUserAttributes)); + assertTrue(testInstanceObject.evaluate(null, testTypedUserAttributes)); } /** @@ -207,8 +204,8 @@ public void existsMatchConditionEvaluatesTrue() throws Exception { public void existsMatchConditionEvaluatesFalse() throws Exception { UserAttribute testInstance = new UserAttribute("bad_var", "custom_attribute", "exists", "chrome"); UserAttribute testInstanceNull = new UserAttribute("null_val", "custom_attribute", "exists", "chrome"); - assertFalse(testInstance.evaluate(null, testTypedUserAttributes, reasons)); - assertFalse(testInstanceNull.evaluate(null, testTypedUserAttributes, reasons)); + assertFalse(testInstance.evaluate(null, testTypedUserAttributes)); + assertFalse(testInstanceNull.evaluate(null, testTypedUserAttributes)); } /** @@ -223,11 +220,11 @@ public void exactMatchConditionEvaluatesTrue() { UserAttribute testInstanceFloat = new UserAttribute("num_size", "custom_attribute", "exact", (float) 3); UserAttribute testInstanceDouble = new UserAttribute("num_counts", "custom_attribute", "exact", 3.55); - assertTrue(testInstanceString.evaluate(null, testUserAttributes, reasons)); - assertTrue(testInstanceBoolean.evaluate(null, testTypedUserAttributes, reasons)); - assertTrue(testInstanceInteger.evaluate(null, testTypedUserAttributes, reasons)); - assertTrue(testInstanceFloat.evaluate(null, Collections.singletonMap("num_size", (float) 3), reasons)); - assertTrue(testInstanceDouble.evaluate(null, testTypedUserAttributes, reasons)); + assertTrue(testInstanceString.evaluate(null, testUserAttributes)); + assertTrue(testInstanceBoolean.evaluate(null, testTypedUserAttributes)); + assertTrue(testInstanceInteger.evaluate(null, testTypedUserAttributes)); + assertTrue(testInstanceFloat.evaluate(null, Collections.singletonMap("num_size", (float) 3))); + assertTrue(testInstanceDouble.evaluate(null, testTypedUserAttributes)); } /** @@ -260,22 +257,22 @@ public void exactMatchConditionEvaluatesNullWithInvalidUserAttr() { assertNull(testInstanceInteger.evaluate( null, - Collections.singletonMap("num_size", bigInteger), reasons)); + Collections.singletonMap("num_size", bigInteger))); assertNull(testInstanceFloat.evaluate( null, - Collections.singletonMap("num_size", invalidFloatValue), reasons)); + Collections.singletonMap("num_size", invalidFloatValue))); assertNull(testInstanceDouble.evaluate( null, - Collections.singletonMap("num_counts", infinitePositiveInfiniteDouble), reasons)); + Collections.singletonMap("num_counts", infinitePositiveInfiniteDouble))); assertNull(testInstanceDouble.evaluate( null, - Collections.singletonMap("num_counts", infiniteNegativeInfiniteDouble), reasons)); + Collections.singletonMap("num_counts", infiniteNegativeInfiniteDouble))); assertNull(testInstanceDouble.evaluate( null, Collections.singletonMap("num_counts", - Collections.singletonMap("num_counts", infiniteNANDouble)), reasons)); + Collections.singletonMap("num_counts", infiniteNANDouble)))); assertNull(testInstanceDouble.evaluate( null, Collections.singletonMap("num_counts", - Collections.singletonMap("num_counts", largeDouble)), reasons)); + Collections.singletonMap("num_counts", largeDouble)))); } /** @@ -293,10 +290,10 @@ public void invalidExactMatchConditionEvaluatesNull() { UserAttribute testInstanceNegativeInfiniteDouble = new UserAttribute("num_counts", "custom_attribute", "exact", infiniteNegativeInfiniteDouble); UserAttribute testInstanceNANDouble = new UserAttribute("num_counts", "custom_attribute", "exact", infiniteNANDouble); - assertNull(testInstanceInteger.evaluate(null, testTypedUserAttributes, reasons)); - assertNull(testInstancePositiveInfinite.evaluate(null, testTypedUserAttributes, reasons)); - assertNull(testInstanceNegativeInfiniteDouble.evaluate(null, testTypedUserAttributes, reasons)); - assertNull(testInstanceNANDouble.evaluate(null, testTypedUserAttributes, reasons)); + assertNull(testInstanceInteger.evaluate(null, testTypedUserAttributes)); + assertNull(testInstancePositiveInfinite.evaluate(null, testTypedUserAttributes)); + assertNull(testInstanceNegativeInfiniteDouble.evaluate(null, testTypedUserAttributes)); + assertNull(testInstanceNANDouble.evaluate(null, testTypedUserAttributes)); } /** @@ -310,10 +307,10 @@ public void exactMatchConditionEvaluatesFalse() { UserAttribute testInstanceInteger = new UserAttribute("num_size", "custom_attribute", "exact", 5); UserAttribute testInstanceDouble = new UserAttribute("num_counts", "custom_attribute", "exact", 5.55); - assertFalse(testInstanceString.evaluate(null, testUserAttributes, reasons)); - assertFalse(testInstanceBoolean.evaluate(null, testTypedUserAttributes, reasons)); - assertFalse(testInstanceInteger.evaluate(null, testTypedUserAttributes, reasons)); - assertFalse(testInstanceDouble.evaluate(null, testTypedUserAttributes, reasons)); + assertFalse(testInstanceString.evaluate(null, testUserAttributes)); + assertFalse(testInstanceBoolean.evaluate(null, testTypedUserAttributes)); + assertFalse(testInstanceInteger.evaluate(null, testTypedUserAttributes)); + assertFalse(testInstanceDouble.evaluate(null, testTypedUserAttributes)); } /** @@ -329,15 +326,15 @@ public void exactMatchConditionEvaluatesNull() { UserAttribute testInstanceDouble = new UserAttribute("num_counts", "custom_attribute", "exact", "3.55"); UserAttribute testInstanceNull = new UserAttribute("null_val", "custom_attribute", "exact", "null_val"); - assertNull(testInstanceObject.evaluate(null, testTypedUserAttributes, reasons)); - assertNull(testInstanceString.evaluate(null, testUserAttributes, reasons)); - assertNull(testInstanceBoolean.evaluate(null, testTypedUserAttributes, reasons)); - assertNull(testInstanceInteger.evaluate(null, testTypedUserAttributes, reasons)); - assertNull(testInstanceDouble.evaluate(null, testTypedUserAttributes, reasons)); - assertNull(testInstanceNull.evaluate(null, testTypedUserAttributes, reasons)); + assertNull(testInstanceObject.evaluate(null, testTypedUserAttributes)); + assertNull(testInstanceString.evaluate(null, testUserAttributes)); + assertNull(testInstanceBoolean.evaluate(null, testTypedUserAttributes)); + assertNull(testInstanceInteger.evaluate(null, testTypedUserAttributes)); + assertNull(testInstanceDouble.evaluate(null, testTypedUserAttributes)); + assertNull(testInstanceNull.evaluate(null, testTypedUserAttributes)); Map attr = new HashMap<>(); attr.put("browser_type", "true"); - assertNull(testInstanceString.evaluate(null, attr, reasons)); + assertNull(testInstanceString.evaluate(null, attr)); } /** @@ -351,13 +348,13 @@ public void gtMatchConditionEvaluatesTrue() { UserAttribute testInstanceFloat = new UserAttribute("num_size", "custom_attribute", "gt", (float) 2); UserAttribute testInstanceDouble = new UserAttribute("num_counts", "custom_attribute", "gt", 2.55); - assertTrue(testInstanceInteger.evaluate(null, testTypedUserAttributes, reasons)); - assertTrue(testInstanceFloat.evaluate(null, Collections.singletonMap("num_size", (float) 3), reasons)); - assertTrue(testInstanceDouble.evaluate(null, testTypedUserAttributes, reasons)); + assertTrue(testInstanceInteger.evaluate(null, testTypedUserAttributes)); + assertTrue(testInstanceFloat.evaluate(null, Collections.singletonMap("num_size", (float) 3))); + assertTrue(testInstanceDouble.evaluate(null, testTypedUserAttributes)); Map badAttributes = new HashMap<>(); badAttributes.put("num_size", "bobs burgers"); - assertNull(testInstanceInteger.evaluate(null, badAttributes, reasons)); + assertNull(testInstanceInteger.evaluate(null, badAttributes)); } /** @@ -391,22 +388,22 @@ public void gtMatchConditionEvaluatesNullWithInvalidUserAttr() { assertNull(testInstanceInteger.evaluate( null, - Collections.singletonMap("num_size", bigInteger), reasons)); + Collections.singletonMap("num_size", bigInteger))); assertNull(testInstanceFloat.evaluate( null, - Collections.singletonMap("num_size", invalidFloatValue), reasons)); + Collections.singletonMap("num_size", invalidFloatValue))); assertNull(testInstanceDouble.evaluate( null, - Collections.singletonMap("num_counts", infinitePositiveInfiniteDouble), reasons)); + Collections.singletonMap("num_counts", infinitePositiveInfiniteDouble))); assertNull(testInstanceDouble.evaluate( null, - Collections.singletonMap("num_counts", infiniteNegativeInfiniteDouble), reasons)); + Collections.singletonMap("num_counts", infiniteNegativeInfiniteDouble))); assertNull(testInstanceDouble.evaluate( null, Collections.singletonMap("num_counts", - Collections.singletonMap("num_counts", infiniteNANDouble)), reasons)); + Collections.singletonMap("num_counts", infiniteNANDouble)))); assertNull(testInstanceDouble.evaluate( null, Collections.singletonMap("num_counts", - Collections.singletonMap("num_counts", largeDouble)), reasons)); + Collections.singletonMap("num_counts", largeDouble)))); } /** @@ -424,10 +421,10 @@ public void gtMatchConditionEvaluatesNullWithInvalidAttr() { UserAttribute testInstanceNegativeInfiniteDouble = new UserAttribute("num_counts", "custom_attribute", "gt", infiniteNegativeInfiniteDouble); UserAttribute testInstanceNANDouble = new UserAttribute("num_counts", "custom_attribute", "gt", infiniteNANDouble); - assertNull(testInstanceInteger.evaluate(null, testTypedUserAttributes, reasons)); - assertNull(testInstancePositiveInfinite.evaluate(null, testTypedUserAttributes, reasons)); - assertNull(testInstanceNegativeInfiniteDouble.evaluate(null, testTypedUserAttributes, reasons)); - assertNull(testInstanceNANDouble.evaluate(null, testTypedUserAttributes, reasons)); + assertNull(testInstanceInteger.evaluate(null, testTypedUserAttributes)); + assertNull(testInstancePositiveInfinite.evaluate(null, testTypedUserAttributes)); + assertNull(testInstanceNegativeInfiniteDouble.evaluate(null, testTypedUserAttributes)); + assertNull(testInstanceNANDouble.evaluate(null, testTypedUserAttributes)); } /** @@ -440,8 +437,8 @@ public void gtMatchConditionEvaluatesFalse() { UserAttribute testInstanceInteger = new UserAttribute("num_size", "custom_attribute", "gt", 5); UserAttribute testInstanceDouble = new UserAttribute("num_counts", "custom_attribute", "gt", 5.55); - assertFalse(testInstanceInteger.evaluate(null, testTypedUserAttributes, reasons)); - assertFalse(testInstanceDouble.evaluate(null, testTypedUserAttributes, reasons)); + assertFalse(testInstanceInteger.evaluate(null, testTypedUserAttributes)); + assertFalse(testInstanceDouble.evaluate(null, testTypedUserAttributes)); } /** @@ -455,10 +452,10 @@ public void gtMatchConditionEvaluatesNull() { UserAttribute testInstanceObject = new UserAttribute("meta_data", "custom_attribute", "gt", 3.5); UserAttribute testInstanceNull = new UserAttribute("null_val", "custom_attribute", "gt", 3.5); - assertNull(testInstanceString.evaluate(null, testUserAttributes, reasons)); - assertNull(testInstanceBoolean.evaluate(null, testTypedUserAttributes, reasons)); - assertNull(testInstanceObject.evaluate(null, testTypedUserAttributes, reasons)); - assertNull(testInstanceNull.evaluate(null, testTypedUserAttributes, reasons)); + assertNull(testInstanceString.evaluate(null, testUserAttributes)); + assertNull(testInstanceBoolean.evaluate(null, testTypedUserAttributes)); + assertNull(testInstanceObject.evaluate(null, testTypedUserAttributes)); + assertNull(testInstanceNull.evaluate(null, testTypedUserAttributes)); } @@ -473,13 +470,13 @@ public void geMatchConditionEvaluatesTrue() { UserAttribute testInstanceFloat = new UserAttribute("num_size", "custom_attribute", "ge", (float) 2); UserAttribute testInstanceDouble = new UserAttribute("num_counts", "custom_attribute", "ge", 2.55); - assertTrue(testInstanceInteger.evaluate(null, testTypedUserAttributes, reasons)); - assertTrue(testInstanceFloat.evaluate(null, Collections.singletonMap("num_size", (float) 2), reasons)); - assertTrue(testInstanceDouble.evaluate(null, testTypedUserAttributes, reasons)); + assertTrue(testInstanceInteger.evaluate(null, testTypedUserAttributes)); + assertTrue(testInstanceFloat.evaluate(null, Collections.singletonMap("num_size", (float) 2))); + assertTrue(testInstanceDouble.evaluate(null, testTypedUserAttributes)); Map badAttributes = new HashMap<>(); badAttributes.put("num_size", "bobs burgers"); - assertNull(testInstanceInteger.evaluate(null, badAttributes, reasons)); + assertNull(testInstanceInteger.evaluate(null, badAttributes)); } /** @@ -513,22 +510,22 @@ public void geMatchConditionEvaluatesNullWithInvalidUserAttr() { assertNull(testInstanceInteger.evaluate( null, - Collections.singletonMap("num_size", bigInteger), reasons)); + Collections.singletonMap("num_size", bigInteger))); assertNull(testInstanceFloat.evaluate( null, - Collections.singletonMap("num_size", invalidFloatValue), reasons)); + Collections.singletonMap("num_size", invalidFloatValue))); assertNull(testInstanceDouble.evaluate( null, - Collections.singletonMap("num_counts", infinitePositiveInfiniteDouble), reasons)); + Collections.singletonMap("num_counts", infinitePositiveInfiniteDouble))); assertNull(testInstanceDouble.evaluate( null, - Collections.singletonMap("num_counts", infiniteNegativeInfiniteDouble), reasons)); + Collections.singletonMap("num_counts", infiniteNegativeInfiniteDouble))); assertNull(testInstanceDouble.evaluate( null, Collections.singletonMap("num_counts", - Collections.singletonMap("num_counts", infiniteNANDouble)), reasons)); + Collections.singletonMap("num_counts", infiniteNANDouble)))); assertNull(testInstanceDouble.evaluate( null, Collections.singletonMap("num_counts", - Collections.singletonMap("num_counts", largeDouble)), reasons)); + Collections.singletonMap("num_counts", largeDouble)))); } /** @@ -546,10 +543,10 @@ public void geMatchConditionEvaluatesNullWithInvalidAttr() { UserAttribute testInstanceNegativeInfiniteDouble = new UserAttribute("num_counts", "custom_attribute", "ge", infiniteNegativeInfiniteDouble); UserAttribute testInstanceNANDouble = new UserAttribute("num_counts", "custom_attribute", "ge", infiniteNANDouble); - assertNull(testInstanceInteger.evaluate(null, testTypedUserAttributes, reasons)); - assertNull(testInstancePositiveInfinite.evaluate(null, testTypedUserAttributes, reasons)); - assertNull(testInstanceNegativeInfiniteDouble.evaluate(null, testTypedUserAttributes, reasons)); - assertNull(testInstanceNANDouble.evaluate(null, testTypedUserAttributes, reasons)); + assertNull(testInstanceInteger.evaluate(null, testTypedUserAttributes)); + assertNull(testInstancePositiveInfinite.evaluate(null, testTypedUserAttributes)); + assertNull(testInstanceNegativeInfiniteDouble.evaluate(null, testTypedUserAttributes)); + assertNull(testInstanceNANDouble.evaluate(null, testTypedUserAttributes)); } /** @@ -562,8 +559,8 @@ public void geMatchConditionEvaluatesFalse() { UserAttribute testInstanceInteger = new UserAttribute("num_size", "custom_attribute", "ge", 5); UserAttribute testInstanceDouble = new UserAttribute("num_counts", "custom_attribute", "ge", 5.55); - assertFalse(testInstanceInteger.evaluate(null, testTypedUserAttributes, reasons)); - assertFalse(testInstanceDouble.evaluate(null, testTypedUserAttributes, reasons)); + assertFalse(testInstanceInteger.evaluate(null, testTypedUserAttributes)); + assertFalse(testInstanceDouble.evaluate(null, testTypedUserAttributes)); } /** @@ -577,10 +574,10 @@ public void geMatchConditionEvaluatesNull() { UserAttribute testInstanceObject = new UserAttribute("meta_data", "custom_attribute", "ge", 3.5); UserAttribute testInstanceNull = new UserAttribute("null_val", "custom_attribute", "ge", 3.5); - assertNull(testInstanceString.evaluate(null, testUserAttributes, reasons)); - assertNull(testInstanceBoolean.evaluate(null, testTypedUserAttributes, reasons)); - assertNull(testInstanceObject.evaluate(null, testTypedUserAttributes, reasons)); - assertNull(testInstanceNull.evaluate(null, testTypedUserAttributes, reasons)); + assertNull(testInstanceString.evaluate(null, testUserAttributes)); + assertNull(testInstanceBoolean.evaluate(null, testTypedUserAttributes)); + assertNull(testInstanceObject.evaluate(null, testTypedUserAttributes)); + assertNull(testInstanceNull.evaluate(null, testTypedUserAttributes)); } @@ -594,8 +591,8 @@ public void ltMatchConditionEvaluatesTrue() { UserAttribute testInstanceInteger = new UserAttribute("num_size", "custom_attribute", "lt", 5); UserAttribute testInstanceDouble = new UserAttribute("num_counts", "custom_attribute", "lt", 5.55); - assertTrue(testInstanceInteger.evaluate(null, testTypedUserAttributes, reasons)); - assertTrue(testInstanceDouble.evaluate(null, testTypedUserAttributes, reasons)); + assertTrue(testInstanceInteger.evaluate(null, testTypedUserAttributes)); + assertTrue(testInstanceDouble.evaluate(null, testTypedUserAttributes)); } /** @@ -608,8 +605,8 @@ public void ltMatchConditionEvaluatesFalse() { UserAttribute testInstanceInteger = new UserAttribute("num_size", "custom_attribute", "lt", 2); UserAttribute testInstanceDouble = new UserAttribute("num_counts", "custom_attribute", "lt", 2.55); - assertFalse(testInstanceInteger.evaluate(null, testTypedUserAttributes, reasons)); - assertFalse(testInstanceDouble.evaluate(null, testTypedUserAttributes, reasons)); + assertFalse(testInstanceInteger.evaluate(null, testTypedUserAttributes)); + assertFalse(testInstanceDouble.evaluate(null, testTypedUserAttributes)); } /** @@ -623,10 +620,10 @@ public void ltMatchConditionEvaluatesNull() { UserAttribute testInstanceObject = new UserAttribute("meta_data", "custom_attribute", "lt", 3.5); UserAttribute testInstanceNull = new UserAttribute("null_val", "custom_attribute", "lt", 3.5); - assertNull(testInstanceString.evaluate(null, testUserAttributes, reasons)); - assertNull(testInstanceBoolean.evaluate(null, testTypedUserAttributes, reasons)); - assertNull(testInstanceObject.evaluate(null, testTypedUserAttributes, reasons)); - assertNull(testInstanceNull.evaluate(null, testTypedUserAttributes, reasons)); + assertNull(testInstanceString.evaluate(null, testUserAttributes)); + assertNull(testInstanceBoolean.evaluate(null, testTypedUserAttributes)); + assertNull(testInstanceObject.evaluate(null, testTypedUserAttributes)); + assertNull(testInstanceNull.evaluate(null, testTypedUserAttributes)); } /** @@ -660,22 +657,22 @@ public void ltMatchConditionEvaluatesNullWithInvalidUserAttr() { assertNull(testInstanceInteger.evaluate( null, - Collections.singletonMap("num_size", bigInteger), reasons)); + Collections.singletonMap("num_size", bigInteger))); assertNull(testInstanceFloat.evaluate( null, - Collections.singletonMap("num_size", invalidFloatValue), reasons)); + Collections.singletonMap("num_size", invalidFloatValue))); assertNull(testInstanceDouble.evaluate( null, - Collections.singletonMap("num_counts", infinitePositiveInfiniteDouble), reasons)); + Collections.singletonMap("num_counts", infinitePositiveInfiniteDouble))); assertNull(testInstanceDouble.evaluate( null, - Collections.singletonMap("num_counts", infiniteNegativeInfiniteDouble), reasons)); + Collections.singletonMap("num_counts", infiniteNegativeInfiniteDouble))); assertNull(testInstanceDouble.evaluate( null, Collections.singletonMap("num_counts", - Collections.singletonMap("num_counts", infiniteNANDouble)), reasons)); + Collections.singletonMap("num_counts", infiniteNANDouble)))); assertNull(testInstanceDouble.evaluate( null, Collections.singletonMap("num_counts", - Collections.singletonMap("num_counts", largeDouble)), reasons)); + Collections.singletonMap("num_counts", largeDouble)))); } /** @@ -693,10 +690,10 @@ public void ltMatchConditionEvaluatesNullWithInvalidAttributes() { UserAttribute testInstanceNegativeInfiniteDouble = new UserAttribute("num_counts", "custom_attribute", "lt", infiniteNegativeInfiniteDouble); UserAttribute testInstanceNANDouble = new UserAttribute("num_counts", "custom_attribute", "lt", infiniteNANDouble); - assertNull(testInstanceInteger.evaluate(null, testTypedUserAttributes, reasons)); - assertNull(testInstancePositiveInfinite.evaluate(null, testTypedUserAttributes, reasons)); - assertNull(testInstanceNegativeInfiniteDouble.evaluate(null, testTypedUserAttributes, reasons)); - assertNull(testInstanceNANDouble.evaluate(null, testTypedUserAttributes, reasons)); + assertNull(testInstanceInteger.evaluate(null, testTypedUserAttributes)); + assertNull(testInstancePositiveInfinite.evaluate(null, testTypedUserAttributes)); + assertNull(testInstanceNegativeInfiniteDouble.evaluate(null, testTypedUserAttributes)); + assertNull(testInstanceNANDouble.evaluate(null, testTypedUserAttributes)); } @@ -710,8 +707,8 @@ public void leMatchConditionEvaluatesTrue() { UserAttribute testInstanceInteger = new UserAttribute("num_size", "custom_attribute", "le", 5); UserAttribute testInstanceDouble = new UserAttribute("num_counts", "custom_attribute", "le", 5.55); - assertTrue(testInstanceInteger.evaluate(null, testTypedUserAttributes, reasons)); - assertTrue(testInstanceDouble.evaluate(null, Collections.singletonMap("num_counts", 5.55), reasons)); + assertTrue(testInstanceInteger.evaluate(null, testTypedUserAttributes)); + assertTrue(testInstanceDouble.evaluate(null, Collections.singletonMap("num_counts", 5.55))); } /** @@ -724,8 +721,8 @@ public void leMatchConditionEvaluatesFalse() { UserAttribute testInstanceInteger = new UserAttribute("num_size", "custom_attribute", "le", 2); UserAttribute testInstanceDouble = new UserAttribute("num_counts", "custom_attribute", "le", 2.55); - assertFalse(testInstanceInteger.evaluate(null, testTypedUserAttributes, reasons)); - assertFalse(testInstanceDouble.evaluate(null, testTypedUserAttributes, reasons)); + assertFalse(testInstanceInteger.evaluate(null, testTypedUserAttributes)); + assertFalse(testInstanceDouble.evaluate(null, testTypedUserAttributes)); } /** @@ -739,10 +736,10 @@ public void leMatchConditionEvaluatesNull() { UserAttribute testInstanceObject = new UserAttribute("meta_data", "custom_attribute", "le", 3.5); UserAttribute testInstanceNull = new UserAttribute("null_val", "custom_attribute", "le", 3.5); - assertNull(testInstanceString.evaluate(null, testUserAttributes, reasons)); - assertNull(testInstanceBoolean.evaluate(null, testTypedUserAttributes, reasons)); - assertNull(testInstanceObject.evaluate(null, testTypedUserAttributes, reasons)); - assertNull(testInstanceNull.evaluate(null, testTypedUserAttributes, reasons)); + assertNull(testInstanceString.evaluate(null, testUserAttributes)); + assertNull(testInstanceBoolean.evaluate(null, testTypedUserAttributes)); + assertNull(testInstanceObject.evaluate(null, testTypedUserAttributes)); + assertNull(testInstanceNull.evaluate(null, testTypedUserAttributes)); } /** @@ -776,22 +773,22 @@ public void leMatchConditionEvaluatesNullWithInvalidUserAttr() { assertNull(testInstanceInteger.evaluate( null, - Collections.singletonMap("num_size", bigInteger), reasons)); + Collections.singletonMap("num_size", bigInteger))); assertNull(testInstanceFloat.evaluate( null, - Collections.singletonMap("num_size", invalidFloatValue), reasons)); + Collections.singletonMap("num_size", invalidFloatValue))); assertNull(testInstanceDouble.evaluate( null, - Collections.singletonMap("num_counts", infinitePositiveInfiniteDouble), reasons)); + Collections.singletonMap("num_counts", infinitePositiveInfiniteDouble))); assertNull(testInstanceDouble.evaluate( null, - Collections.singletonMap("num_counts", infiniteNegativeInfiniteDouble), reasons)); + Collections.singletonMap("num_counts", infiniteNegativeInfiniteDouble))); assertNull(testInstanceDouble.evaluate( null, Collections.singletonMap("num_counts", - Collections.singletonMap("num_counts", infiniteNANDouble)), reasons)); + Collections.singletonMap("num_counts", infiniteNANDouble)))); assertNull(testInstanceDouble.evaluate( null, Collections.singletonMap("num_counts", - Collections.singletonMap("num_counts", largeDouble)), reasons)); + Collections.singletonMap("num_counts", largeDouble)))); } /** @@ -809,10 +806,10 @@ public void leMatchConditionEvaluatesNullWithInvalidAttributes() { UserAttribute testInstanceNegativeInfiniteDouble = new UserAttribute("num_counts", "custom_attribute", "le", infiniteNegativeInfiniteDouble); UserAttribute testInstanceNANDouble = new UserAttribute("num_counts", "custom_attribute", "le", infiniteNANDouble); - assertNull(testInstanceInteger.evaluate(null, testTypedUserAttributes, reasons)); - assertNull(testInstancePositiveInfinite.evaluate(null, testTypedUserAttributes, reasons)); - assertNull(testInstanceNegativeInfiniteDouble.evaluate(null, testTypedUserAttributes, reasons)); - assertNull(testInstanceNANDouble.evaluate(null, testTypedUserAttributes, reasons)); + assertNull(testInstanceInteger.evaluate(null, testTypedUserAttributes)); + assertNull(testInstancePositiveInfinite.evaluate(null, testTypedUserAttributes)); + assertNull(testInstanceNegativeInfiniteDouble.evaluate(null, testTypedUserAttributes)); + assertNull(testInstanceNANDouble.evaluate(null, testTypedUserAttributes)); } /** @@ -822,7 +819,7 @@ public void leMatchConditionEvaluatesNullWithInvalidAttributes() { @Test public void substringMatchConditionEvaluatesTrue() { UserAttribute testInstanceString = new UserAttribute("browser_type", "custom_attribute", "substring", "chrome"); - assertTrue(testInstanceString.evaluate(null, testUserAttributes, reasons)); + assertTrue(testInstanceString.evaluate(null, testUserAttributes)); } /** @@ -832,7 +829,7 @@ public void substringMatchConditionEvaluatesTrue() { @Test public void substringMatchConditionPartialMatchEvaluatesTrue() { UserAttribute testInstanceString = new UserAttribute("browser_type", "custom_attribute", "substring", "chro"); - assertTrue(testInstanceString.evaluate(null, testUserAttributes, reasons)); + assertTrue(testInstanceString.evaluate(null, testUserAttributes)); } /** @@ -842,7 +839,7 @@ public void substringMatchConditionPartialMatchEvaluatesTrue() { @Test public void substringMatchConditionEvaluatesFalse() { UserAttribute testInstanceString = new UserAttribute("browser_type", "custom_attribute", "substring", "chr0me"); - assertFalse(testInstanceString.evaluate(null, testUserAttributes, reasons)); + assertFalse(testInstanceString.evaluate(null, testUserAttributes)); } /** @@ -857,11 +854,11 @@ public void substringMatchConditionEvaluatesNull() { UserAttribute testInstanceObject = new UserAttribute("meta_data", "custom_attribute", "substring", "chrome1"); UserAttribute testInstanceNull = new UserAttribute("null_val", "custom_attribute", "substring", "chrome1"); - assertNull(testInstanceBoolean.evaluate(null, testTypedUserAttributes, reasons)); - assertNull(testInstanceInteger.evaluate(null, testTypedUserAttributes, reasons)); - assertNull(testInstanceDouble.evaluate(null, testTypedUserAttributes, reasons)); - assertNull(testInstanceObject.evaluate(null, testTypedUserAttributes, reasons)); - assertNull(testInstanceNull.evaluate(null, testTypedUserAttributes, reasons)); + assertNull(testInstanceBoolean.evaluate(null, testTypedUserAttributes)); + assertNull(testInstanceInteger.evaluate(null, testTypedUserAttributes)); + assertNull(testInstanceDouble.evaluate(null, testTypedUserAttributes)); + assertNull(testInstanceObject.evaluate(null, testTypedUserAttributes)); + assertNull(testInstanceNull.evaluate(null, testTypedUserAttributes)); } //======== Semantic version evaluation tests ========// @@ -872,7 +869,7 @@ public void testSemanticVersionEqualsMatchInvalidInput() { Map testAttributes = new HashMap(); testAttributes.put("version", 2.0); UserAttribute testInstanceString = new UserAttribute("version", "custom_attribute", "semver_eq", "2.0.0"); - assertNull(testInstanceString.evaluate(null, testAttributes, reasons)); + assertNull(testInstanceString.evaluate(null, testAttributes)); } @Test @@ -880,7 +877,7 @@ public void semanticVersionInvalidMajorShouldBeNumberOnly() { Map testAttributes = new HashMap(); testAttributes.put("version", "a.1.2"); UserAttribute testInstanceString = new UserAttribute("version", "custom_attribute", "semver_eq", "2.0.0"); - assertNull(testInstanceString.evaluate(null, testAttributes, reasons)); + assertNull(testInstanceString.evaluate(null, testAttributes)); } @Test @@ -888,7 +885,7 @@ public void semanticVersionInvalidMinorShouldBeNumberOnly() { Map testAttributes = new HashMap(); testAttributes.put("version", "1.b.2"); UserAttribute testInstanceString = new UserAttribute("version", "custom_attribute", "semver_eq", "2.0.0"); - assertNull(testInstanceString.evaluate(null, testAttributes, reasons)); + assertNull(testInstanceString.evaluate(null, testAttributes)); } @Test @@ -896,7 +893,7 @@ public void semanticVersionInvalidPatchShouldBeNumberOnly() { Map testAttributes = new HashMap(); testAttributes.put("version", "1.2.c"); UserAttribute testInstanceString = new UserAttribute("version", "custom_attribute", "semver_eq", "2.0.0"); - assertNull(testInstanceString.evaluate(null, testAttributes, reasons)); + assertNull(testInstanceString.evaluate(null, testAttributes)); } // Test SemanticVersionEqualsMatch returns null if given invalid UserCondition Variable type @@ -905,7 +902,7 @@ public void testSemanticVersionEqualsMatchInvalidUserConditionVariable() { Map testAttributes = new HashMap(); testAttributes.put("version", "2.0"); UserAttribute testInstanceString = new UserAttribute("version", "custom_attribute", "semver_eq", 2.0); - assertNull(testInstanceString.evaluate(null, testAttributes, reasons)); + assertNull(testInstanceString.evaluate(null, testAttributes)); } // Test SemanticVersionGTMatch returns null if given invalid value type @@ -914,7 +911,7 @@ public void testSemanticVersionGTMatchInvalidInput() { Map testAttributes = new HashMap(); testAttributes.put("version", false); UserAttribute testInstanceString = new UserAttribute("version", "custom_attribute", "semver_gt", "2.0.0"); - assertNull(testInstanceString.evaluate(null, testAttributes, reasons)); + assertNull(testInstanceString.evaluate(null, testAttributes)); } // Test SemanticVersionGEMatch returns null if given invalid value type @@ -923,7 +920,7 @@ public void testSemanticVersionGEMatchInvalidInput() { Map testAttributes = new HashMap(); testAttributes.put("version", 2); UserAttribute testInstanceString = new UserAttribute("version", "custom_attribute", "semver_ge", "2.0.0"); - assertNull(testInstanceString.evaluate(null, testAttributes, reasons)); + assertNull(testInstanceString.evaluate(null, testAttributes)); } // Test SemanticVersionLTMatch returns null if given invalid value type @@ -932,7 +929,7 @@ public void testSemanticVersionLTMatchInvalidInput() { Map testAttributes = new HashMap(); testAttributes.put("version", 2); UserAttribute testInstanceString = new UserAttribute("version", "custom_attribute", "semver_lt", "2.0.0"); - assertNull(testInstanceString.evaluate(null, testAttributes, reasons)); + assertNull(testInstanceString.evaluate(null, testAttributes)); } // Test SemanticVersionLEMatch returns null if given invalid value type @@ -941,7 +938,7 @@ public void testSemanticVersionLEMatchInvalidInput() { Map testAttributes = new HashMap(); testAttributes.put("version", 2); UserAttribute testInstanceString = new UserAttribute("version", "custom_attribute", "semver_le", "2.0.0"); - assertNull(testInstanceString.evaluate(null, testAttributes, reasons)); + assertNull(testInstanceString.evaluate(null, testAttributes)); } // Test if not same when targetVersion is only major.minor.patch and version is major.minor @@ -950,7 +947,7 @@ public void testIsSemanticNotSameConditionValueMajorMinorPatch() { Map testAttributes = new HashMap(); testAttributes.put("version", "1.2"); UserAttribute testInstanceString = new UserAttribute("version", "custom_attribute", "semver_eq", "1.2.0"); - assertFalse(testInstanceString.evaluate(null, testAttributes, reasons)); + assertFalse(testInstanceString.evaluate(null, testAttributes)); } // Test if same when target is only major but user condition checks only major.minor,patch @@ -959,7 +956,7 @@ public void testIsSemanticSameSingleDigit() { Map testAttributes = new HashMap(); testAttributes.put("version", "3.0.0"); UserAttribute testInstanceString = new UserAttribute("version", "custom_attribute", "semver_eq", "3"); - assertTrue(testInstanceString.evaluate(null, testAttributes, reasons)); + assertTrue(testInstanceString.evaluate(null, testAttributes)); } // Test if greater when User value patch is greater even when its beta @@ -968,7 +965,7 @@ public void testIsSemanticGreaterWhenUserConditionComparesMajorMinorAndPatchVers Map testAttributes = new HashMap(); testAttributes.put("version", "3.1.1-beta"); UserAttribute testInstanceString = new UserAttribute("version", "custom_attribute", "semver_gt", "3.1.0"); - assertTrue(testInstanceString.evaluate(null, testAttributes, reasons)); + assertTrue(testInstanceString.evaluate(null, testAttributes)); } // Test if greater when preRelease is greater alphabetically @@ -977,7 +974,7 @@ public void testIsSemanticGreaterWhenMajorMinorPatchReleaseVersionCharacter() { Map testAttributes = new HashMap(); testAttributes.put("version", "3.1.1-beta.y.1+1.1"); UserAttribute testInstanceString = new UserAttribute("version", "custom_attribute", "semver_gt", "3.1.1-beta.x.1+1.1"); - assertTrue(testInstanceString.evaluate(null, testAttributes, reasons)); + assertTrue(testInstanceString.evaluate(null, testAttributes)); } // Test if greater when preRelease version number is greater @@ -986,7 +983,7 @@ public void testIsSemanticGreaterWhenMajorMinorPatchPreReleaseVersionNum() { Map testAttributes = new HashMap(); testAttributes.put("version", "3.1.1-beta.x.2+1.1"); UserAttribute testInstanceString = new UserAttribute("version", "custom_attribute", "semver_gt", "3.1.1-beta.x.1+1.1"); - assertTrue(testInstanceString.evaluate(null, testAttributes, reasons)); + assertTrue(testInstanceString.evaluate(null, testAttributes)); } // Test if equals semantic version even when only same preRelease is passed in user attribute and no build meta @@ -995,7 +992,7 @@ public void testIsSemanticEqualWhenMajorMinorPatchPreReleaseVersionNum() { Map testAttributes = new HashMap(); testAttributes.put("version", "3.1.1-beta.x.1"); UserAttribute testInstanceString = new UserAttribute("version", "custom_attribute", "semver_eq", "3.1.1-beta.x.1"); - assertTrue(testInstanceString.evaluate(null, testAttributes, reasons)); + assertTrue(testInstanceString.evaluate(null, testAttributes)); } // Test if not same @@ -1004,7 +1001,7 @@ public void testIsSemanticNotSameReturnsFalse() { Map testAttributes = new HashMap(); testAttributes.put("version", "2.1.2"); UserAttribute testInstanceString = new UserAttribute("version", "custom_attribute", "semver_eq", "2.1.1"); - assertFalse(testInstanceString.evaluate(null, testAttributes, reasons)); + assertFalse(testInstanceString.evaluate(null, testAttributes)); } // Test when target is full semantic version major.minor.patch @@ -1013,7 +1010,7 @@ public void testIsSemanticSameFull() { Map testAttributes = new HashMap(); testAttributes.put("version", "3.0.1"); UserAttribute testInstanceString = new UserAttribute("version", "custom_attribute", "semver_eq", "3.0.1"); - assertTrue(testInstanceString.evaluate(null, testAttributes, reasons)); + assertTrue(testInstanceString.evaluate(null, testAttributes)); } // Test compare less when user condition checks only major.minor @@ -1022,7 +1019,7 @@ public void testIsSemanticLess() { Map testAttributes = new HashMap(); testAttributes.put("version", "2.1.6"); UserAttribute testInstanceString = new UserAttribute("version", "custom_attribute", "semver_lt", "2.2"); - assertTrue(testInstanceString.evaluate(null, testAttributes, reasons)); + assertTrue(testInstanceString.evaluate(null, testAttributes)); } // When user condition checks major.minor but target is major.minor.patch then its equals @@ -1031,7 +1028,7 @@ public void testIsSemanticLessFalse() { Map testAttributes = new HashMap(); testAttributes.put("version", "2.1.0"); UserAttribute testInstanceString = new UserAttribute("version", "custom_attribute", "semver_lt", "2.1"); - assertFalse(testInstanceString.evaluate(null, testAttributes, reasons)); + assertFalse(testInstanceString.evaluate(null, testAttributes)); } // Test compare less when target is full major.minor.patch @@ -1040,7 +1037,7 @@ public void testIsSemanticFullLess() { Map testAttributes = new HashMap(); testAttributes.put("version", "2.1.6"); UserAttribute testInstanceString = new UserAttribute("version", "custom_attribute", "semver_lt", "2.1.9"); - assertTrue(testInstanceString.evaluate(null, testAttributes, reasons)); + assertTrue(testInstanceString.evaluate(null, testAttributes)); } // Test compare greater when user condition checks only major.minor @@ -1049,7 +1046,7 @@ public void testIsSemanticMore() { Map testAttributes = new HashMap(); testAttributes.put("version", "2.3.6"); UserAttribute testInstanceString = new UserAttribute("version", "custom_attribute", "semver_gt", "2.2"); - assertTrue(testInstanceString.evaluate(null, testAttributes, reasons)); + assertTrue(testInstanceString.evaluate(null, testAttributes)); } // Test compare greater when both are major.minor.patch-beta but target is greater than user condition @@ -1058,7 +1055,7 @@ public void testIsSemanticMoreWhenBeta() { Map testAttributes = new HashMap(); testAttributes.put("version", "2.3.6-beta"); UserAttribute testInstanceString = new UserAttribute("version", "custom_attribute", "semver_gt", "2.3.5-beta"); - assertTrue(testInstanceString.evaluate(null, testAttributes, reasons)); + assertTrue(testInstanceString.evaluate(null, testAttributes)); } // Test compare greater when target is major.minor.patch @@ -1067,7 +1064,7 @@ public void testIsSemanticFullMore() { Map testAttributes = new HashMap(); testAttributes.put("version", "2.1.7"); UserAttribute testInstanceString = new UserAttribute("version", "custom_attribute", "semver_gt", "2.1.6"); - assertTrue(testInstanceString.evaluate(null, testAttributes, reasons)); + assertTrue(testInstanceString.evaluate(null, testAttributes)); } // Test compare greater when target is major.minor.patch is smaller then it returns false @@ -1076,7 +1073,7 @@ public void testSemanticVersionGTFullMoreReturnsFalse() { Map testAttributes = new HashMap(); testAttributes.put("version", "2.1.9"); UserAttribute testInstanceString = new UserAttribute("version", "custom_attribute", "semver_gt", "2.1.10"); - assertFalse(testInstanceString.evaluate(null, testAttributes, reasons)); + assertFalse(testInstanceString.evaluate(null, testAttributes)); } // Test compare equal when both are exactly same - major.minor.patch-beta @@ -1085,7 +1082,7 @@ public void testIsSemanticFullEqual() { Map testAttributes = new HashMap(); testAttributes.put("version", "2.1.9-beta"); UserAttribute testInstanceString = new UserAttribute("version", "custom_attribute", "semver_eq", "2.1.9-beta"); - assertTrue(testInstanceString.evaluate(null, testAttributes, reasons)); + assertTrue(testInstanceString.evaluate(null, testAttributes)); } // Test compare equal when both major.minor.patch is same, but due to beta user condition is smaller @@ -1094,7 +1091,7 @@ public void testIsSemanticLessWhenBeta() { Map testAttributes = new HashMap(); testAttributes.put("version", "2.1.9"); UserAttribute testInstanceString = new UserAttribute("version", "custom_attribute", "semver_gt", "2.1.9-beta"); - assertTrue(testInstanceString.evaluate(null, testAttributes, reasons)); + assertTrue(testInstanceString.evaluate(null, testAttributes)); } // Test compare greater when target is major.minor.patch-beta and user condition only compares major.minor.patch @@ -1103,7 +1100,7 @@ public void testIsSemanticGreaterBeta() { Map testAttributes = new HashMap(); testAttributes.put("version", "2.1.9"); UserAttribute testInstanceString = new UserAttribute("version", "custom_attribute", "semver_gt", "2.1.9-beta"); - assertTrue(testInstanceString.evaluate(null, testAttributes, reasons)); + assertTrue(testInstanceString.evaluate(null, testAttributes)); } // Test compare equal when target is major.minor.patch @@ -1112,7 +1109,7 @@ public void testIsSemanticLessEqualsWhenEqualsReturnsTrue() { Map testAttributes = new HashMap(); testAttributes.put("version", "2.1.9"); UserAttribute testInstanceString = new UserAttribute("version", "custom_attribute", "semver_le", "2.1.9"); - assertTrue(testInstanceString.evaluate(null, testAttributes, reasons)); + assertTrue(testInstanceString.evaluate(null, testAttributes)); } // Test compare less when target is major.minor.patch @@ -1121,7 +1118,7 @@ public void testIsSemanticLessEqualsWhenLessReturnsTrue() { Map testAttributes = new HashMap(); testAttributes.put("version", "2.132.9"); UserAttribute testInstanceString = new UserAttribute("version", "custom_attribute", "semver_le", "2.233.91"); - assertTrue(testInstanceString.evaluate(null, testAttributes, reasons)); + assertTrue(testInstanceString.evaluate(null, testAttributes)); } // Test compare less when target is major.minor.patch @@ -1130,7 +1127,7 @@ public void testIsSemanticLessEqualsWhenGreaterReturnsFalse() { Map testAttributes = new HashMap(); testAttributes.put("version", "2.233.91"); UserAttribute testInstanceString = new UserAttribute("version", "custom_attribute", "semver_le", "2.132.009"); - assertFalse(testInstanceString.evaluate(null, testAttributes, reasons)); + assertFalse(testInstanceString.evaluate(null, testAttributes)); } // Test compare equal when target is major.minor.patch @@ -1139,7 +1136,7 @@ public void testIsSemanticGreaterEqualsWhenEqualsReturnsTrue() { Map testAttributes = new HashMap(); testAttributes.put("version", "2.1.9"); UserAttribute testInstanceString = new UserAttribute("version", "custom_attribute", "semver_ge", "2.1.9"); - assertTrue(testInstanceString.evaluate(null, testAttributes, reasons)); + assertTrue(testInstanceString.evaluate(null, testAttributes)); } // Test compare less when target is major.minor.patch @@ -1148,7 +1145,7 @@ public void testIsSemanticGreaterEqualsWhenLessReturnsTrue() { Map testAttributes = new HashMap(); testAttributes.put("version", "2.233.91"); UserAttribute testInstanceString = new UserAttribute("version", "custom_attribute", "semver_ge", "2.132.9"); - assertTrue(testInstanceString.evaluate(null, testAttributes, reasons)); + assertTrue(testInstanceString.evaluate(null, testAttributes)); } // Test compare less when target is major.minor.patch @@ -1157,7 +1154,7 @@ public void testIsSemanticGreaterEqualsWhenLessReturnsFalse() { Map testAttributes = new HashMap(); testAttributes.put("version", "2.132.009"); UserAttribute testInstanceString = new UserAttribute("version", "custom_attribute", "semver_ge", "2.233.91"); - assertFalse(testInstanceString.evaluate(null, testAttributes, reasons)); + assertFalse(testInstanceString.evaluate(null, testAttributes)); } /** @@ -1166,7 +1163,7 @@ public void testIsSemanticGreaterEqualsWhenLessReturnsFalse() { @Test public void notConditionEvaluateNull() { NotCondition notCondition = new NotCondition(new NullCondition()); - assertNull(notCondition.evaluate(null, testUserAttributes, reasons)); + assertNull(notCondition.evaluate(null, testUserAttributes)); } /** @@ -1175,11 +1172,11 @@ public void notConditionEvaluateNull() { @Test public void notConditionEvaluateTrue() { UserAttribute userAttribute = mock(UserAttribute.class); - when(userAttribute.evaluate(eq(null), eq(testUserAttributes), anyObject())).thenReturn(false); + when(userAttribute.evaluate(null, testUserAttributes)).thenReturn(false); NotCondition notCondition = new NotCondition(userAttribute); - assertTrue(notCondition.evaluate(null, testUserAttributes, reasons)); - verify(userAttribute, times(1)).evaluate(eq(null), eq(testUserAttributes), anyObject()); + assertTrue(notCondition.evaluate(null, testUserAttributes)); + verify(userAttribute, times(1)).evaluate(null, testUserAttributes); } /** @@ -1188,11 +1185,11 @@ public void notConditionEvaluateTrue() { @Test public void notConditionEvaluateFalse() { UserAttribute userAttribute = mock(UserAttribute.class); - when(userAttribute.evaluate(eq(null), eq(testUserAttributes), anyObject())).thenReturn(true); + when(userAttribute.evaluate(null, testUserAttributes)).thenReturn(true); NotCondition notCondition = new NotCondition(userAttribute); - assertFalse(notCondition.evaluate(null, testUserAttributes, reasons)); - verify(userAttribute, times(1)).evaluate(eq(null), eq(testUserAttributes), anyObject()); + assertFalse(notCondition.evaluate(null, testUserAttributes)); + verify(userAttribute, times(1)).evaluate(null, testUserAttributes); } /** @@ -1201,20 +1198,20 @@ public void notConditionEvaluateFalse() { @Test public void orConditionEvaluateTrue() { UserAttribute userAttribute1 = mock(UserAttribute.class); - when(userAttribute1.evaluate(eq(null), eq(testUserAttributes), anyObject())).thenReturn(true); + when(userAttribute1.evaluate(null, testUserAttributes)).thenReturn(true); UserAttribute userAttribute2 = mock(UserAttribute.class); - when(userAttribute2.evaluate(eq(null), eq(testUserAttributes), anyObject())).thenReturn(false); + when(userAttribute2.evaluate(null, testUserAttributes)).thenReturn(false); List conditions = new ArrayList(); conditions.add(userAttribute1); conditions.add(userAttribute2); OrCondition orCondition = new OrCondition(conditions); - assertTrue(orCondition.evaluate(null, testUserAttributes, reasons)); - verify(userAttribute1, times(1)).evaluate(eq(null), eq(testUserAttributes), anyObject()); + assertTrue(orCondition.evaluate(null, testUserAttributes)); + verify(userAttribute1, times(1)).evaluate(null, testUserAttributes); // shouldn't be called due to short-circuiting in 'Or' evaluation - verify(userAttribute2, times(0)).evaluate(eq(null), eq(testUserAttributes), anyObject()); + verify(userAttribute2, times(0)).evaluate(null, testUserAttributes); } /** @@ -1223,20 +1220,20 @@ public void orConditionEvaluateTrue() { @Test public void orConditionEvaluateTrueWithNullAndTrue() { UserAttribute userAttribute1 = mock(UserAttribute.class); - when(userAttribute1.evaluate(eq(null), eq(testUserAttributes), anyObject())).thenReturn(null); + when(userAttribute1.evaluate(null, testUserAttributes)).thenReturn(null); UserAttribute userAttribute2 = mock(UserAttribute.class); - when(userAttribute2.evaluate(eq(null), eq(testUserAttributes), anyObject())).thenReturn(true); + when(userAttribute2.evaluate(null, testUserAttributes)).thenReturn(true); List conditions = new ArrayList(); conditions.add(userAttribute1); conditions.add(userAttribute2); OrCondition orCondition = new OrCondition(conditions); - assertTrue(orCondition.evaluate(null, testUserAttributes, reasons)); - verify(userAttribute1, times(1)).evaluate(eq(null), eq(testUserAttributes), anyObject()); + assertTrue(orCondition.evaluate(null, testUserAttributes)); + verify(userAttribute1, times(1)).evaluate(null, testUserAttributes); // shouldn't be called due to short-circuiting in 'Or' evaluation - verify(userAttribute2, times(1)).evaluate(eq(null), eq(testUserAttributes), anyObject()); + verify(userAttribute2, times(1)).evaluate(null, testUserAttributes); } /** @@ -1245,20 +1242,20 @@ public void orConditionEvaluateTrueWithNullAndTrue() { @Test public void orConditionEvaluateNullWithNullAndFalse() { UserAttribute userAttribute1 = mock(UserAttribute.class); - when(userAttribute1.evaluate(eq(null), eq(testUserAttributes), anyObject())).thenReturn(null); + when(userAttribute1.evaluate(null, testUserAttributes)).thenReturn(null); UserAttribute userAttribute2 = mock(UserAttribute.class); - when(userAttribute2.evaluate(eq(null), eq(testUserAttributes), anyObject())).thenReturn(false); + when(userAttribute2.evaluate(null, testUserAttributes)).thenReturn(false); List conditions = new ArrayList(); conditions.add(userAttribute1); conditions.add(userAttribute2); OrCondition orCondition = new OrCondition(conditions); - assertNull(orCondition.evaluate(null, testUserAttributes, reasons)); - verify(userAttribute1, times(1)).evaluate(eq(null), eq(testUserAttributes), anyObject()); + assertNull(orCondition.evaluate(null, testUserAttributes)); + verify(userAttribute1, times(1)).evaluate(null, testUserAttributes); // shouldn't be called due to short-circuiting in 'Or' evaluation - verify(userAttribute2, times(1)).evaluate(eq(null), eq(testUserAttributes), anyObject()); + verify(userAttribute2, times(1)).evaluate(null, testUserAttributes); } /** @@ -1267,20 +1264,20 @@ public void orConditionEvaluateNullWithNullAndFalse() { @Test public void orConditionEvaluateFalseWithFalseAndFalse() { UserAttribute userAttribute1 = mock(UserAttribute.class); - when(userAttribute1.evaluate(eq(null), eq(testUserAttributes), anyObject())).thenReturn(false); + when(userAttribute1.evaluate(null, testUserAttributes)).thenReturn(false); UserAttribute userAttribute2 = mock(UserAttribute.class); - when(userAttribute2.evaluate(eq(null), eq(testUserAttributes), anyObject())).thenReturn(false); + when(userAttribute2.evaluate(null, testUserAttributes)).thenReturn(false); List conditions = new ArrayList(); conditions.add(userAttribute1); conditions.add(userAttribute2); OrCondition orCondition = new OrCondition(conditions); - assertFalse(orCondition.evaluate(null, testUserAttributes, reasons)); - verify(userAttribute1, times(1)).evaluate(eq(null), eq(testUserAttributes), anyObject()); + assertFalse(orCondition.evaluate(null, testUserAttributes)); + verify(userAttribute1, times(1)).evaluate(null, testUserAttributes); // shouldn't be called due to short-circuiting in 'Or' evaluation - verify(userAttribute2, times(1)).evaluate(eq(null), eq(testUserAttributes), anyObject()); + verify(userAttribute2, times(1)).evaluate(null, testUserAttributes); } /** @@ -1289,19 +1286,19 @@ public void orConditionEvaluateFalseWithFalseAndFalse() { @Test public void orConditionEvaluateFalse() { UserAttribute userAttribute1 = mock(UserAttribute.class); - when(userAttribute1.evaluate(eq(null), eq(testUserAttributes), anyObject())).thenReturn(false); + when(userAttribute1.evaluate(null, testUserAttributes)).thenReturn(false); UserAttribute userAttribute2 = mock(UserAttribute.class); - when(userAttribute2.evaluate(eq(null), eq(testUserAttributes), anyObject())).thenReturn(false); + when(userAttribute2.evaluate(null, testUserAttributes)).thenReturn(false); List conditions = new ArrayList(); conditions.add(userAttribute1); conditions.add(userAttribute2); OrCondition orCondition = new OrCondition(conditions); - assertFalse(orCondition.evaluate(null, testUserAttributes, reasons)); - verify(userAttribute1, times(1)).evaluate(eq(null), eq(testUserAttributes), anyObject()); - verify(userAttribute2, times(1)).evaluate(eq(null), eq(testUserAttributes), anyObject()); + assertFalse(orCondition.evaluate(null, testUserAttributes)); + verify(userAttribute1, times(1)).evaluate(null, testUserAttributes); + verify(userAttribute2, times(1)).evaluate(null, testUserAttributes); } /** @@ -1310,19 +1307,19 @@ public void orConditionEvaluateFalse() { @Test public void andConditionEvaluateTrue() { OrCondition orCondition1 = mock(OrCondition.class); - when(orCondition1.evaluate(eq(null), eq(testUserAttributes), anyObject())).thenReturn(true); + when(orCondition1.evaluate(null, testUserAttributes)).thenReturn(true); OrCondition orCondition2 = mock(OrCondition.class); - when(orCondition2.evaluate(eq(null), eq(testUserAttributes), anyObject())).thenReturn(true); + when(orCondition2.evaluate(null, testUserAttributes)).thenReturn(true); List conditions = new ArrayList(); conditions.add(orCondition1); conditions.add(orCondition2); AndCondition andCondition = new AndCondition(conditions); - assertTrue(andCondition.evaluate(null, testUserAttributes, reasons)); - verify(orCondition1, times(1)).evaluate(eq(null), eq(testUserAttributes), anyObject()); - verify(orCondition2, times(1)).evaluate(eq(null), eq(testUserAttributes), anyObject()); + assertTrue(andCondition.evaluate(null, testUserAttributes)); + verify(orCondition1, times(1)).evaluate(null, testUserAttributes); + verify(orCondition2, times(1)).evaluate(null, testUserAttributes); } /** @@ -1331,19 +1328,19 @@ public void andConditionEvaluateTrue() { @Test public void andConditionEvaluateFalseWithNullAndFalse() { OrCondition orCondition1 = mock(OrCondition.class); - when(orCondition1.evaluate(eq(null), eq(testUserAttributes), anyObject())).thenReturn(null); + when(orCondition1.evaluate(null, testUserAttributes)).thenReturn(null); OrCondition orCondition2 = mock(OrCondition.class); - when(orCondition2.evaluate(eq(null), eq(testUserAttributes), anyObject())).thenReturn(false); + when(orCondition2.evaluate(null, testUserAttributes)).thenReturn(false); List conditions = new ArrayList(); conditions.add(orCondition1); conditions.add(orCondition2); AndCondition andCondition = new AndCondition(conditions); - assertFalse(andCondition.evaluate(null, testUserAttributes, reasons)); - verify(orCondition1, times(1)).evaluate(eq(null), eq(testUserAttributes), anyObject()); - verify(orCondition2, times(1)).evaluate(eq(null), eq(testUserAttributes), anyObject()); + assertFalse(andCondition.evaluate(null, testUserAttributes)); + verify(orCondition1, times(1)).evaluate(null, testUserAttributes); + verify(orCondition2, times(1)).evaluate(null, testUserAttributes); } /** @@ -1352,19 +1349,19 @@ public void andConditionEvaluateFalseWithNullAndFalse() { @Test public void andConditionEvaluateNullWithNullAndTrue() { OrCondition orCondition1 = mock(OrCondition.class); - when(orCondition1.evaluate(eq(null), eq(testUserAttributes), anyObject())).thenReturn(null); + when(orCondition1.evaluate(null, testUserAttributes)).thenReturn(null); OrCondition orCondition2 = mock(OrCondition.class); - when(orCondition2.evaluate(eq(null), eq(testUserAttributes), anyObject())).thenReturn(true); + when(orCondition2.evaluate(null, testUserAttributes)).thenReturn(true); List conditions = new ArrayList(); conditions.add(orCondition1); conditions.add(orCondition2); AndCondition andCondition = new AndCondition(conditions); - assertNull(andCondition.evaluate(null, testUserAttributes, reasons)); - verify(orCondition1, times(1)).evaluate(eq(null), eq(testUserAttributes), anyObject()); - verify(orCondition2, times(1)).evaluate(eq(null), eq(testUserAttributes), anyObject()); + assertNull(andCondition.evaluate(null, testUserAttributes)); + verify(orCondition1, times(1)).evaluate(null, testUserAttributes); + verify(orCondition2, times(1)).evaluate(null, testUserAttributes); } /** @@ -1373,10 +1370,10 @@ public void andConditionEvaluateNullWithNullAndTrue() { @Test public void andConditionEvaluateFalse() { OrCondition orCondition1 = mock(OrCondition.class); - when(orCondition1.evaluate(eq(null), eq(testUserAttributes), anyObject())).thenReturn(false); + when(orCondition1.evaluate(null, testUserAttributes)).thenReturn(false); OrCondition orCondition2 = mock(OrCondition.class); - when(orCondition2.evaluate(eq(null), eq(testUserAttributes), anyObject())).thenReturn(true); + when(orCondition2.evaluate(null, testUserAttributes)).thenReturn(true); // and[false, true] List conditions = new ArrayList(); @@ -1384,13 +1381,13 @@ public void andConditionEvaluateFalse() { conditions.add(orCondition2); AndCondition andCondition = new AndCondition(conditions); - assertFalse(andCondition.evaluate(null, testUserAttributes, reasons)); - verify(orCondition1, times(1)).evaluate(eq(null), eq(testUserAttributes), anyObject()); + assertFalse(andCondition.evaluate(null, testUserAttributes)); + verify(orCondition1, times(1)).evaluate(null, testUserAttributes); // shouldn't be called due to short-circuiting in 'And' evaluation - verify(orCondition2, times(0)).evaluate(eq(null), eq(testUserAttributes), anyObject()); + verify(orCondition2, times(0)).evaluate(null, testUserAttributes); OrCondition orCondition3 = mock(OrCondition.class); - when(orCondition3.evaluate(null, testUserAttributes, reasons)).thenReturn(null); + when(orCondition3.evaluate(null, testUserAttributes)).thenReturn(null); // and[null, false] List conditions2 = new ArrayList(); @@ -1398,7 +1395,7 @@ public void andConditionEvaluateFalse() { conditions2.add(orCondition1); AndCondition andCondition2 = new AndCondition(conditions2); - assertFalse(andCondition2.evaluate(null, testUserAttributes, reasons)); + assertFalse(andCondition2.evaluate(null, testUserAttributes)); // and[true, false, null] List conditions3 = new ArrayList(); @@ -1407,7 +1404,7 @@ public void andConditionEvaluateFalse() { conditions3.add(orCondition1); AndCondition andCondition3 = new AndCondition(conditions3); - assertFalse(andCondition3.evaluate(null, testUserAttributes, reasons)); + assertFalse(andCondition3.evaluate(null, testUserAttributes)); } /** @@ -1439,8 +1436,8 @@ public void nullValueEvaluate() { attributeValue ); - assertNull(nullValueAttribute.evaluate(null, Collections.emptyMap(), reasons)); - assertNull(nullValueAttribute.evaluate(null, Collections.singletonMap(attributeName, attributeValue), reasons)); - assertNull(nullValueAttribute.evaluate(null, (Collections.singletonMap(attributeName, "")), reasons)); + assertNull(nullValueAttribute.evaluate(null, Collections.emptyMap())); + assertNull(nullValueAttribute.evaluate(null, Collections.singletonMap(attributeName, attributeValue))); + assertNull(nullValueAttribute.evaluate(null, (Collections.singletonMap(attributeName, "")))); } } From 15a8a7f620556d06be9e4c4ac82ba5b8f2c3098c Mon Sep 17 00:00:00 2001 From: Jae Kim Date: Thu, 17 Dec 2020 17:28:34 -0800 Subject: [PATCH 3/9] fix DecisionService tests --- .../optimizelydecision/DecisionResponse.java | 8 + .../com/optimizely/ab/OptimizelyTest.java | 27 +-- .../ab/bucketing/DecisionServiceTest.java | 213 +++++++++--------- .../ab/event/internal/EventFactoryTest.java | 7 +- .../ab/internal/ExperimentUtilsTest.java | 32 +-- 5 files changed, 143 insertions(+), 144 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/optimizelydecision/DecisionResponse.java b/core-api/src/main/java/com/optimizely/ab/optimizelydecision/DecisionResponse.java index 3f85e827e..e549c1195 100644 --- a/core-api/src/main/java/com/optimizely/ab/optimizelydecision/DecisionResponse.java +++ b/core-api/src/main/java/com/optimizely/ab/optimizelydecision/DecisionResponse.java @@ -12,6 +12,14 @@ public DecisionResponse(@Nullable T result, @Nonnull DecisionReasons reasons) { this.reasons = reasons; } + public static DecisionResponse responseNoReasons(@Nullable E result) { + return new DecisionResponse(result, DefaultDecisionReasons.newInstance()); + } + + public static DecisionResponse nullNoReasons() { + return new DecisionResponse(null, DefaultDecisionReasons.newInstance()); + } + @Nullable public T getResult() { return result; 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 6cb7eb360..01fd8c87a 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java @@ -34,6 +34,7 @@ import com.optimizely.ab.internal.ControlAttribute; import com.optimizely.ab.internal.LogbackVerifier; import com.optimizely.ab.notification.*; +import com.optimizely.ab.optimizelydecision.DecisionResponse; import com.optimizely.ab.optimizelyjson.OptimizelyJSON; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import org.junit.Before; @@ -377,7 +378,7 @@ public void activateForNullVariation() throws Exception { Experiment activatedExperiment = validProjectConfig.getExperiments().get(0); Map testUserAttributes = Collections.singletonMap("browser_type", "chromey"); - when(mockBucketer.bucket(eq(activatedExperiment), eq(testBucketingId), eq(validProjectConfig), anyObject())).thenReturn(null); + when(mockBucketer.bucket(eq(activatedExperiment), eq(testBucketingId), eq(validProjectConfig))).thenReturn(DecisionResponse.nullNoReasons()); logbackVerifier.expectMessage(Level.INFO, "Not activating user \"userId\" for experiment \"" + activatedExperiment.getKey() + "\"."); @@ -936,7 +937,7 @@ public void activateWithInvalidDatafile() throws Exception { assertNull(expectedVariation); // make sure we didn't even attempt to bucket the user - verify(mockBucketer, never()).bucket(any(Experiment.class), anyString(), any(ProjectConfig.class), anyObject()); + verify(mockBucketer, never()).bucket(any(Experiment.class), anyString(), any(ProjectConfig.class)); } //======== track tests ========// @@ -1237,7 +1238,7 @@ public void trackWithInvalidDatafile() throws Exception { optimizely.track("event_with_launched_and_running_experiments", genericUserId); // make sure we didn't even attempt to bucket the user or fire any conversion events - verify(mockBucketer, never()).bucket(any(Experiment.class), anyString(), any(ProjectConfig.class), anyObject()); + verify(mockBucketer, never()).bucket(any(Experiment.class), anyString(), any(ProjectConfig.class)); verify(mockEventHandler, never()).dispatchEvent(any(LogEvent.class)); } @@ -1254,7 +1255,7 @@ public void getVariation() throws Exception { Optimizely optimizely = optimizelyBuilder.withBucketing(mockBucketer).build(); - when(mockBucketer.bucket(eq(activatedExperiment), eq(testUserId), eq(validProjectConfig), anyObject())).thenReturn(bucketedVariation); + when(mockBucketer.bucket(eq(activatedExperiment), eq(testUserId), eq(validProjectConfig))).thenReturn(DecisionResponse.responseNoReasons(bucketedVariation)); Map testUserAttributes = new HashMap<>(); testUserAttributes.put("browser_type", "chrome"); @@ -1264,7 +1265,7 @@ public void getVariation() throws Exception { testUserAttributes); // verify that the bucketing algorithm was called correctly - verify(mockBucketer).bucket(eq(activatedExperiment), eq(testUserId), eq(validProjectConfig), anyObject()); + verify(mockBucketer).bucket(eq(activatedExperiment), eq(testUserId), eq(validProjectConfig)); assertThat(actualVariation, is(bucketedVariation)); // verify that we didn't attempt to dispatch an event @@ -1285,13 +1286,13 @@ public void getVariationWithExperimentKey() throws Exception { .withConfig(noAudienceProjectConfig) .build(); - when(mockBucketer.bucket(eq(activatedExperiment), eq(testUserId), eq(noAudienceProjectConfig), anyObject())).thenReturn(bucketedVariation); + when(mockBucketer.bucket(eq(activatedExperiment), eq(testUserId), eq(noAudienceProjectConfig))).thenReturn(DecisionResponse.responseNoReasons(bucketedVariation)); // activate the experiment Variation actualVariation = optimizely.getVariation(activatedExperiment.getKey(), testUserId); // verify that the bucketing algorithm was called correctly - verify(mockBucketer).bucket(eq(activatedExperiment), eq(testUserId), eq(noAudienceProjectConfig), anyObject()); + verify(mockBucketer).bucket(eq(activatedExperiment), eq(testUserId), eq(noAudienceProjectConfig)); assertThat(actualVariation, is(bucketedVariation)); // verify that we didn't attempt to dispatch an event @@ -1346,7 +1347,7 @@ public void getVariationWithAudiences() throws Exception { Experiment experiment = validProjectConfig.getExperiments().get(0); Variation bucketedVariation = experiment.getVariations().get(0); - when(mockBucketer.bucket(eq(experiment), eq(testUserId), eq(validProjectConfig), anyObject())).thenReturn(bucketedVariation); + when(mockBucketer.bucket(eq(experiment), eq(testUserId), eq(validProjectConfig))).thenReturn(DecisionResponse.responseNoReasons(bucketedVariation)); Optimizely optimizely = optimizelyBuilder.withBucketing(mockBucketer).build(); @@ -1355,7 +1356,7 @@ public void getVariationWithAudiences() throws Exception { Variation actualVariation = optimizely.getVariation(experiment.getKey(), testUserId, testUserAttributes); - verify(mockBucketer).bucket(eq(experiment), eq(testUserId), eq(validProjectConfig), anyObject()); + verify(mockBucketer).bucket(eq(experiment), eq(testUserId), eq(validProjectConfig)); assertThat(actualVariation, is(bucketedVariation)); } @@ -1396,7 +1397,7 @@ public void getVariationNoAudiences() throws Exception { Experiment experiment = noAudienceProjectConfig.getExperiments().get(0); Variation bucketedVariation = experiment.getVariations().get(0); - when(mockBucketer.bucket(eq(experiment), eq(testUserId), eq(noAudienceProjectConfig), anyObject())).thenReturn(bucketedVariation); + when(mockBucketer.bucket(eq(experiment), eq(testUserId), eq(noAudienceProjectConfig))).thenReturn(DecisionResponse.responseNoReasons(bucketedVariation)); Optimizely optimizely = optimizelyBuilder .withConfig(noAudienceProjectConfig) @@ -1405,7 +1406,7 @@ public void getVariationNoAudiences() throws Exception { Variation actualVariation = optimizely.getVariation(experiment.getKey(), testUserId); - verify(mockBucketer).bucket(eq(experiment), eq(testUserId), eq(noAudienceProjectConfig), anyObject()); + verify(mockBucketer).bucket(eq(experiment), eq(testUserId), eq(noAudienceProjectConfig)); assertThat(actualVariation, is(bucketedVariation)); } @@ -1463,7 +1464,7 @@ public void getVariationForGroupExperimentWithMatchingAttributes() throws Except attributes.put("browser_type", "chrome"); } - when(mockBucketer.bucket(eq(experiment), eq("user"), eq(validProjectConfig), anyObject())).thenReturn(variation); + when(mockBucketer.bucket(eq(experiment), eq("user"), eq(validProjectConfig))).thenReturn(DecisionResponse.responseNoReasons(variation)); Optimizely optimizely = optimizelyBuilder.withBucketing(mockBucketer).build(); @@ -1521,7 +1522,7 @@ public void getVariationWithInvalidDatafile() throws Exception { assertNull(variation); // make sure we didn't even attempt to bucket the user - verify(mockBucketer, never()).bucket(any(Experiment.class), anyString(), any(ProjectConfig.class), anyObject()); + verify(mockBucketer, never()).bucket(any(Experiment.class), anyString(), any(ProjectConfig.class)); } //======== Notification listeners ========// 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 d12cd9a56..59dc47b22 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 @@ -20,6 +20,7 @@ import com.optimizely.ab.error.ErrorHandler; import com.optimizely.ab.internal.ControlAttribute; import com.optimizely.ab.internal.LogbackVerifier; +import com.optimizely.ab.optimizelydecision.DecisionResponse; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import org.junit.Before; import org.junit.Rule; @@ -88,15 +89,15 @@ 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)); + assertNull(decisionService.getVariation(experiment, genericUserId, 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, whitelistedUserId, Collections.emptyMap(), validProjectConfig), is(expectedVariation)); + assertThat(decisionService.getVariation(experiment, whitelistedUserId, Collections.emptyMap(), validProjectConfig).getResult(), is(expectedVariation)); - verify(decisionService).getWhitelistedVariation(eq(experiment), eq(whitelistedUserId), anyObject()); - verify(decisionService, never()).getStoredVariation(eq(experiment), any(UserProfile.class), any(ProjectConfig.class), anyObject()); + verify(decisionService).getWhitelistedVariation(eq(experiment), eq(whitelistedUserId)); + verify(decisionService, never()).getStoredVariation(eq(experiment), any(UserProfile.class), any(ProjectConfig.class)); } /** @@ -110,19 +111,19 @@ 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)); + assertNull(decisionService.getVariation(experiment, 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), is(expectedVariation)); + assertThat(decisionService.getVariation(experiment, 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), anyObject()); - assertEquals(decisionService.getWhitelistedVariation(experiment, whitelistedUserId), whitelistVariation); + 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)); - assertThat(decisionService.getVariation(experiment, whitelistedUserId, Collections.emptyMap(), validProjectConfig), is(whitelistVariation)); + assertNull(decisionService.getForcedVariation(experiment, whitelistedUserId).getResult()); + assertThat(decisionService.getVariation(experiment, whitelistedUserId, Collections.emptyMap(), validProjectConfig).getResult(), is(whitelistVariation)); } /** @@ -135,16 +136,16 @@ 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)); + assertNull(decisionService.getVariation(experiment, 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), is(expectedVariation)); + assertThat(decisionService.getVariation(experiment, genericUserId, Collections.emptyMap(), validProjectConfig).getResult(), is(expectedVariation)); - verify(decisionService, never()).getStoredVariation(eq(experiment), any(UserProfile.class), eq(validProjectConfig), anyObject()); + verify(decisionService, never()).getStoredVariation(eq(experiment), any(UserProfile.class), eq(validProjectConfig)); assertEquals(decisionService.setForcedVariation(experiment, genericUserId, null), true); - assertNull(decisionService.getForcedVariation(experiment, genericUserId)); + assertNull(decisionService.getForcedVariation(experiment, genericUserId).getResult()); } /** @@ -164,18 +165,18 @@ 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)); + assertNull(decisionService.getVariation(experiment, 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)); + decisionService.getVariation(experiment, 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)); + decisionService.getVariation(experiment, userProfileId, Collections.emptyMap(), validProjectConfig).getResult()); assertTrue(decisionService.setForcedVariation(experiment, userProfileId, null)); - assertNull(decisionService.getForcedVariation(experiment, userProfileId)); + assertNull(decisionService.getForcedVariation(experiment, userProfileId).getResult()); } /** @@ -195,11 +196,11 @@ 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)); + assertNull(decisionService.getVariation(experiment, 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)); + decisionService.getVariation(experiment, userProfileId, Collections.emptyMap(), validProjectConfig).getResult()); } @@ -216,7 +217,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)); + assertNull(decisionService.getVariation(experiment, "userId", Collections.emptyMap(), validProjectConfig).getResult()); // we call getVariation 3 times on an experiment that is not running. logbackVerifier.expectMessage(Level.INFO, @@ -227,12 +228,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)); + assertNull(decisionService.getVariation(experiment, "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)); + assertNull(decisionService.getVariation(experiment, "userId", Collections.emptyMap(), validProjectConfig).getResult()); } @@ -264,7 +265,7 @@ public void getVariationForFeatureReturnsNullWhenFeatureFlagExperimentIdsIsEmpty emptyFeatureFlag, genericUserId, Collections.emptyMap(), - validProjectConfig); + validProjectConfig).getResult(); assertNull(featureDecision.variation); assertNull(featureDecision.decisionSource); @@ -283,21 +284,19 @@ public void getVariationForFeatureReturnsNullWhenItGetsNoVariationsForExperiment FeatureFlag spyFeatureFlag = spy(FEATURE_FLAG_MULTI_VARIATE_FEATURE); // do not bucket to any experiments - doReturn(null).when(decisionService).getVariation( + doReturn(DecisionResponse.nullNoReasons()).when(decisionService).getVariation( any(Experiment.class), anyString(), anyMapOf(String.class, String.class), any(ProjectConfig.class), - anyObject(), anyObject() ); // do not bucket to any rollouts - doReturn(new FeatureDecision(null, null, null)).when(decisionService).getVariationForFeatureInRollout( + doReturn(DecisionResponse.responseNoReasons(new FeatureDecision(null, null, null))).when(decisionService).getVariationForFeatureInRollout( any(FeatureFlag.class), anyString(), anyMapOf(String.class, String.class), - any(ProjectConfig.class), - anyObject() + any(ProjectConfig.class) ); // try to get a variation back from the decision service for the feature flag @@ -306,7 +305,7 @@ public void getVariationForFeatureReturnsNullWhenItGetsNoVariationsForExperiment genericUserId, Collections.emptyMap(), validProjectConfig - ); + ).getResult(); assertNull(featureDecision.variation); assertNull(featureDecision.decisionSource); @@ -327,21 +326,19 @@ public void getVariationForFeatureReturnsNullWhenItGetsNoVariationsForExperiment public void getVariationForFeatureReturnsVariationReturnedFromGetVariation() { FeatureFlag spyFeatureFlag = spy(ValidProjectConfigV4.FEATURE_FLAG_MUTEX_GROUP_FEATURE); - doReturn(null).when(decisionService).getVariation( + doReturn(DecisionResponse.nullNoReasons()).when(decisionService).getVariation( eq(ValidProjectConfigV4.EXPERIMENT_MUTEX_GROUP_EXPERIMENT_1), anyString(), anyMapOf(String.class, String.class), any(ProjectConfig.class), - anyObject(), anyObject() ); - doReturn(ValidProjectConfigV4.VARIATION_MUTEX_GROUP_EXP_2_VAR_1).when(decisionService).getVariation( + 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(ProjectConfig.class), - anyObject(), anyObject() ); @@ -350,7 +347,7 @@ public void getVariationForFeatureReturnsVariationReturnedFromGetVariation() { genericUserId, Collections.emptyMap(), v4ProjectConfig - ); + ).getResult(); assertEquals(ValidProjectConfigV4.VARIATION_MUTEX_GROUP_EXP_2_VAR_1, featureDecision.variation); assertEquals(FeatureDecision.DecisionSource.FEATURE_TEST, featureDecision.decisionSource); @@ -376,24 +373,22 @@ public void getVariationForFeatureReturnsVariationFromExperimentBeforeRollout() Variation rolloutVariation = rolloutExperiment.getVariations().get(0); // return variation for experiment - doReturn(experimentVariation) + doReturn(DecisionResponse.responseNoReasons(experimentVariation)) .when(decisionService).getVariation( eq(featureExperiment), anyString(), anyMapOf(String.class, String.class), any(ProjectConfig.class), - anyObject(), anyObject() ); // return variation for rollout - doReturn(new FeatureDecision(rolloutExperiment, rolloutVariation, FeatureDecision.DecisionSource.ROLLOUT)) + doReturn(DecisionResponse.responseNoReasons(new FeatureDecision(rolloutExperiment, rolloutVariation, FeatureDecision.DecisionSource.ROLLOUT))) .when(decisionService).getVariationForFeatureInRollout( eq(featureFlag), anyString(), anyMapOf(String.class, String.class), - any(ProjectConfig.class), - anyObject() + any(ProjectConfig.class) ); // make sure we get the right variation back @@ -402,7 +397,7 @@ public void getVariationForFeatureReturnsVariationFromExperimentBeforeRollout() genericUserId, Collections.emptyMap(), v4ProjectConfig - ); + ).getResult(); assertEquals(experimentVariation, featureDecision.variation); assertEquals(FeatureDecision.DecisionSource.FEATURE_TEST, featureDecision.decisionSource); @@ -411,8 +406,7 @@ public void getVariationForFeatureReturnsVariationFromExperimentBeforeRollout() any(FeatureFlag.class), anyString(), anyMapOf(String.class, String.class), - any(ProjectConfig.class), - anyObject() + any(ProjectConfig.class) ); // make sure we ask for experiment bucketing once @@ -421,7 +415,6 @@ public void getVariationForFeatureReturnsVariationFromExperimentBeforeRollout() anyString(), anyMapOf(String.class, String.class), any(ProjectConfig.class), - anyObject(), anyObject() ); } @@ -442,24 +435,22 @@ public void getVariationForFeatureReturnsVariationFromRolloutWhenExperimentFails Variation rolloutVariation = rolloutExperiment.getVariations().get(0); // return variation for experiment - doReturn(null) + doReturn(DecisionResponse.nullNoReasons()) .when(decisionService).getVariation( eq(featureExperiment), anyString(), anyMapOf(String.class, String.class), any(ProjectConfig.class), - anyObject(), anyObject() ); // return variation for rollout - doReturn(new FeatureDecision(rolloutExperiment, rolloutVariation, FeatureDecision.DecisionSource.ROLLOUT)) + doReturn(DecisionResponse.responseNoReasons(new FeatureDecision(rolloutExperiment, rolloutVariation, FeatureDecision.DecisionSource.ROLLOUT))) .when(decisionService).getVariationForFeatureInRollout( eq(featureFlag), anyString(), anyMapOf(String.class, String.class), - any(ProjectConfig.class), - anyObject() + any(ProjectConfig.class) ); // make sure we get the right variation back @@ -468,7 +459,7 @@ public void getVariationForFeatureReturnsVariationFromRolloutWhenExperimentFails genericUserId, Collections.emptyMap(), v4ProjectConfig - ); + ).getResult(); assertEquals(rolloutVariation, featureDecision.variation); assertEquals(FeatureDecision.DecisionSource.ROLLOUT, featureDecision.decisionSource); @@ -477,8 +468,7 @@ public void getVariationForFeatureReturnsVariationFromRolloutWhenExperimentFails any(FeatureFlag.class), anyString(), anyMapOf(String.class, String.class), - any(ProjectConfig.class), - anyObject() + any(ProjectConfig.class) ); // make sure we ask for experiment bucketing once @@ -487,7 +477,6 @@ public void getVariationForFeatureReturnsVariationFromRolloutWhenExperimentFails anyString(), anyMapOf(String.class, String.class), any(ProjectConfig.class), - anyObject(), anyObject() ); @@ -517,7 +506,7 @@ public void getVariationForFeatureInRolloutReturnsNullWhenFeatureIsNotAttachedTo genericUserId, Collections.emptyMap(), validProjectConfig - ); + ).getResult(); assertNull(featureDecision.variation); assertNull(featureDecision.decisionSource); @@ -534,7 +523,7 @@ public void getVariationForFeatureInRolloutReturnsNullWhenFeatureIsNotAttachedTo @Test public void getVariationForFeatureInRolloutReturnsNullWhenUserIsExcludedFromAllTraffic() { Bucketer mockBucketer = mock(Bucketer.class); - when(mockBucketer.bucket(any(Experiment.class), anyString(), any(ProjectConfig.class), anyObject())).thenReturn(null); + when(mockBucketer.bucket(any(Experiment.class), anyString(), any(ProjectConfig.class))).thenReturn(DecisionResponse.nullNoReasons()); DecisionService decisionService = new DecisionService( mockBucketer, @@ -549,14 +538,14 @@ public void getVariationForFeatureInRolloutReturnsNullWhenUserIsExcludedFromAllT ATTRIBUTE_HOUSE_KEY, AUDIENCE_GRYFFINDOR_VALUE ), v4ProjectConfig - ); + ).getResult(); assertNull(featureDecision.variation); assertNull(featureDecision.decisionSource); // with fall back bucketing, the user has at most 2 chances to get bucketed with traffic allocation // one chance with the audience rollout rule // one chance with the everyone else rule - verify(mockBucketer, atMost(2)).bucket(any(Experiment.class), anyString(), any(ProjectConfig.class), anyObject()); + verify(mockBucketer, atMost(2)).bucket(any(Experiment.class), anyString(), any(ProjectConfig.class)); } /** @@ -567,7 +556,7 @@ public void getVariationForFeatureInRolloutReturnsNullWhenUserIsExcludedFromAllT @Test public void getVariationForFeatureInRolloutReturnsNullWhenUserFailsAllAudiencesAndTraffic() { Bucketer mockBucketer = mock(Bucketer.class); - when(mockBucketer.bucket(any(Experiment.class), anyString(), any(ProjectConfig.class), anyObject())).thenReturn(null); + when(mockBucketer.bucket(any(Experiment.class), anyString(), any(ProjectConfig.class))).thenReturn(DecisionResponse.nullNoReasons()); DecisionService decisionService = new DecisionService(mockBucketer, mockErrorHandler, null); @@ -576,12 +565,12 @@ public void getVariationForFeatureInRolloutReturnsNullWhenUserFailsAllAudiencesA genericUserId, Collections.emptyMap(), v4ProjectConfig - ); + ).getResult(); assertNull(featureDecision.variation); assertNull(featureDecision.decisionSource); // user is only bucketed once for the everyone else rule - verify(mockBucketer, times(1)).bucket(any(Experiment.class), anyString(), any(ProjectConfig.class), anyObject()); + verify(mockBucketer, times(1)).bucket(any(Experiment.class), anyString(), any(ProjectConfig.class)); } /** @@ -595,7 +584,7 @@ public void getVariationForFeatureInRolloutReturnsVariationWhenUserFailsAllAudie Rollout rollout = ROLLOUT_2; Experiment everyoneElseRule = rollout.getExperiments().get(rollout.getExperiments().size() - 1); Variation expectedVariation = everyoneElseRule.getVariations().get(0); - when(mockBucketer.bucket(eq(everyoneElseRule), anyString(), any(ProjectConfig.class), anyObject())).thenReturn(expectedVariation); + when(mockBucketer.bucket(eq(everyoneElseRule), anyString(), any(ProjectConfig.class))).thenReturn(DecisionResponse.responseNoReasons(expectedVariation)); DecisionService decisionService = new DecisionService( mockBucketer, @@ -608,7 +597,7 @@ public void getVariationForFeatureInRolloutReturnsVariationWhenUserFailsAllAudie genericUserId, Collections.emptyMap(), v4ProjectConfig - ); + ).getResult(); logbackVerifier.expectMessage(Level.DEBUG, "Evaluating audiences for rule \"1\": [3468206642]."); logbackVerifier.expectMessage(Level.INFO, "Audiences for rule \"1\" collectively evaluated to null."); logbackVerifier.expectMessage(Level.DEBUG, "Evaluating audiences for rule \"2\": [3988293898]."); @@ -621,7 +610,7 @@ public void getVariationForFeatureInRolloutReturnsVariationWhenUserFailsAllAudie assertEquals(FeatureDecision.DecisionSource.ROLLOUT, featureDecision.decisionSource); // verify user is only bucketed once for everyone else rule - verify(mockBucketer, times(1)).bucket(any(Experiment.class), anyString(), any(ProjectConfig.class), anyObject()); + verify(mockBucketer, times(1)).bucket(any(Experiment.class), anyString(), any(ProjectConfig.class)); } /** @@ -636,8 +625,8 @@ public void getVariationForFeatureInRolloutReturnsVariationWhenUserFailsTrafficI Rollout rollout = ROLLOUT_2; Experiment everyoneElseRule = rollout.getExperiments().get(rollout.getExperiments().size() - 1); Variation expectedVariation = everyoneElseRule.getVariations().get(0); - when(mockBucketer.bucket(any(Experiment.class), anyString(), any(ProjectConfig.class), anyObject())).thenReturn(null); - when(mockBucketer.bucket(eq(everyoneElseRule), anyString(), any(ProjectConfig.class), anyObject())).thenReturn(expectedVariation); + when(mockBucketer.bucket(any(Experiment.class), anyString(), any(ProjectConfig.class))).thenReturn(DecisionResponse.nullNoReasons()); + when(mockBucketer.bucket(eq(everyoneElseRule), anyString(), any(ProjectConfig.class))).thenReturn(DecisionResponse.responseNoReasons(expectedVariation)); DecisionService decisionService = new DecisionService( mockBucketer, @@ -652,14 +641,14 @@ public void getVariationForFeatureInRolloutReturnsVariationWhenUserFailsTrafficI ATTRIBUTE_HOUSE_KEY, AUDIENCE_GRYFFINDOR_VALUE ), v4ProjectConfig - ); + ).getResult(); assertEquals(expectedVariation, featureDecision.variation); assertEquals(FeatureDecision.DecisionSource.ROLLOUT, featureDecision.decisionSource); logbackVerifier.expectMessage(Level.DEBUG, "User \"genericUserId\" meets conditions for targeting rule \"Everyone Else\"."); // verify user is only bucketed once for everyone else rule - verify(mockBucketer, times(2)).bucket(any(Experiment.class), anyString(), any(ProjectConfig.class), anyObject()); + verify(mockBucketer, times(2)).bucket(any(Experiment.class), anyString(), any(ProjectConfig.class)); } /** @@ -678,9 +667,9 @@ public void getVariationForFeatureInRolloutReturnsVariationWhenUserFailsTrafficI Variation englishCitizenVariation = englishCitizensRule.getVariations().get(0); Experiment everyoneElseRule = rollout.getExperiments().get(rollout.getExperiments().size() - 1); Variation expectedVariation = everyoneElseRule.getVariations().get(0); - when(mockBucketer.bucket(any(Experiment.class), anyString(), any(ProjectConfig.class), anyObject())).thenReturn(null); - when(mockBucketer.bucket(eq(everyoneElseRule), anyString(), any(ProjectConfig.class), anyObject())).thenReturn(expectedVariation); - when(mockBucketer.bucket(eq(englishCitizensRule), anyString(), any(ProjectConfig.class), anyObject())).thenReturn(englishCitizenVariation); + when(mockBucketer.bucket(any(Experiment.class), anyString(), any(ProjectConfig.class))).thenReturn(DecisionResponse.nullNoReasons()); + when(mockBucketer.bucket(eq(everyoneElseRule), anyString(), any(ProjectConfig.class))).thenReturn(DecisionResponse.responseNoReasons(expectedVariation)); + when(mockBucketer.bucket(eq(englishCitizensRule), anyString(), any(ProjectConfig.class))).thenReturn(DecisionResponse.responseNoReasons(englishCitizenVariation)); DecisionService decisionService = new DecisionService( mockBucketer, @@ -700,12 +689,12 @@ public void getVariationForFeatureInRolloutReturnsVariationWhenUserFailsTrafficI ) ), v4ProjectConfig - ); + ).getResult(); assertEquals(expectedVariation, featureDecision.variation); assertEquals(FeatureDecision.DecisionSource.ROLLOUT, featureDecision.decisionSource); // verify user is only bucketed once for everyone else rule - verify(mockBucketer, times(2)).bucket(any(Experiment.class), anyString(), any(ProjectConfig.class), anyObject()); + verify(mockBucketer, times(2)).bucket(any(Experiment.class), anyString(), any(ProjectConfig.class)); } /** @@ -721,9 +710,9 @@ public void getVariationForFeatureInRolloutReturnsVariationWhenUserFailsTargetin Variation englishCitizenVariation = englishCitizensRule.getVariations().get(0); Experiment everyoneElseRule = rollout.getExperiments().get(rollout.getExperiments().size() - 1); Variation everyoneElseVariation = everyoneElseRule.getVariations().get(0); - when(mockBucketer.bucket(any(Experiment.class), anyString(), any(ProjectConfig.class), anyObject())).thenReturn(null); - when(mockBucketer.bucket(eq(everyoneElseRule), anyString(), any(ProjectConfig.class), anyObject())).thenReturn(everyoneElseVariation); - when(mockBucketer.bucket(eq(englishCitizensRule), anyString(), any(ProjectConfig.class), anyObject())).thenReturn(englishCitizenVariation); + when(mockBucketer.bucket(any(Experiment.class), anyString(), any(ProjectConfig.class))).thenReturn(DecisionResponse.nullNoReasons()); + when(mockBucketer.bucket(eq(everyoneElseRule), anyString(), any(ProjectConfig.class))).thenReturn(DecisionResponse.responseNoReasons(everyoneElseVariation)); + when(mockBucketer.bucket(eq(englishCitizensRule), anyString(), any(ProjectConfig.class))).thenReturn(DecisionResponse.responseNoReasons(englishCitizenVariation)); DecisionService decisionService = new DecisionService(mockBucketer, mockErrorHandler, null); @@ -734,7 +723,7 @@ public void getVariationForFeatureInRolloutReturnsVariationWhenUserFailsTargetin ATTRIBUTE_NATIONALITY_KEY, AUDIENCE_ENGLISH_CITIZENS_VALUE ), v4ProjectConfig - ); + ).getResult(); assertEquals(englishCitizenVariation, featureDecision.variation); assertEquals(FeatureDecision.DecisionSource.ROLLOUT, featureDecision.decisionSource); logbackVerifier.expectMessage(Level.INFO, "Audiences for rule \"2\" collectively evaluated to null"); @@ -743,7 +732,7 @@ public void getVariationForFeatureInRolloutReturnsVariationWhenUserFailsTargetin logbackVerifier.expectMessage(Level.DEBUG, "Audience \"4194404272\" evaluated to true."); logbackVerifier.expectMessage(Level.INFO, "Audiences for rule \"3\" collectively evaluated to true"); // verify user is only bucketed once for everyone else rule - verify(mockBucketer, times(1)).bucket(any(Experiment.class), anyString(), any(ProjectConfig.class), anyObject()); + verify(mockBucketer, times(1)).bucket(any(Experiment.class), anyString(), any(ProjectConfig.class)); } //========= white list tests ==========/ @@ -755,7 +744,7 @@ public void getVariationForFeatureInRolloutReturnsVariationWhenUserFailsTargetin public void getWhitelistedReturnsForcedVariation() { logbackVerifier.expectMessage(Level.INFO, "User \"" + whitelistedUserId + "\" is forced in variation \"" + whitelistedVariation.getKey() + "\"."); - assertEquals(whitelistedVariation, decisionService.getWhitelistedVariation(whitelistedExperiment, whitelistedUserId)); + assertEquals(whitelistedVariation, decisionService.getWhitelistedVariation(whitelistedExperiment, whitelistedUserId).getResult()); } /** @@ -784,7 +773,7 @@ public void getWhitelistedWithInvalidVariation() throws Exception { Level.ERROR, "Variation \"" + invalidVariationKey + "\" is not in the datafile. Not activating user \"" + userId + "\"."); - assertNull(decisionService.getWhitelistedVariation(experiment, userId)); + assertNull(decisionService.getWhitelistedVariation(experiment, userId).getResult()); } /** @@ -792,7 +781,7 @@ public void getWhitelistedWithInvalidVariation() throws Exception { */ @Test public void getWhitelistedReturnsNullWhenUserIsNotWhitelisted() throws Exception { - assertNull(decisionService.getWhitelistedVariation(whitelistedExperiment, genericUserId)); + assertNull(decisionService.getWhitelistedVariation(whitelistedExperiment, genericUserId).getResult()); } //======== User Profile tests =========// @@ -822,7 +811,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)); + decisionService.getVariation(experiment, userProfileId, Collections.emptyMap(), noAudienceProjectConfig).getResult()); verify(userProfileService).lookup(userProfileId); } @@ -845,7 +834,7 @@ public void getStoredVariationLogsWhenLookupReturnsNull() throws Exception { logbackVerifier.expectMessage(Level.INFO, "No previously activated variation of experiment " + "\"" + experiment.getKey() + "\" for user \"" + userProfileId + "\" found in user profile."); - assertNull(decisionService.getStoredVariation(experiment, userProfile, noAudienceProjectConfig)); + assertNull(decisionService.getStoredVariation(experiment, userProfile, noAudienceProjectConfig).getResult()); } /** @@ -874,7 +863,7 @@ public void getStoredVariationReturnsNullWhenVariationIsNoLongerInConfig() throw "experiment \"" + experiment.getKey() + "\", but no matching variation " + "was found for that user. We will re-bucket the user."); - assertNull(decisionService.getStoredVariation(experiment, storedUserProfile, noAudienceProjectConfig)); + assertNull(decisionService.getStoredVariation(experiment, storedUserProfile, noAudienceProjectConfig).getResult()); } /** @@ -896,12 +885,12 @@ public void getVariationSavesBucketedVariationIntoUserProfile() throws Exception Collections.singletonMap(experiment.getId(), decision)); Bucketer mockBucketer = mock(Bucketer.class); - when(mockBucketer.bucket(eq(experiment), eq(userProfileId), eq(noAudienceProjectConfig), anyObject())).thenReturn(variation); + when(mockBucketer.bucket(eq(experiment), eq(userProfileId), eq(noAudienceProjectConfig))).thenReturn(DecisionResponse.responseNoReasons(variation)); DecisionService decisionService = new DecisionService(mockBucketer, mockErrorHandler, userProfileService); assertEquals(variation, decisionService.getVariation( - experiment, userProfileId, Collections.emptyMap(), noAudienceProjectConfig) + experiment, userProfileId, Collections.emptyMap(), noAudienceProjectConfig).getResult() ); logbackVerifier.expectMessage(Level.INFO, String.format("Saved variation \"%s\" of experiment \"%s\" for user \"" + userProfileId + "\".", variation.getId(), @@ -958,10 +947,10 @@ public void getVariationSavesANewUserProfile() throws Exception { UserProfileService userProfileService = mock(UserProfileService.class); DecisionService decisionService = new DecisionService(bucketer, mockErrorHandler, userProfileService); - when(bucketer.bucket(eq(experiment), eq(userProfileId), eq(noAudienceProjectConfig), anyObject())).thenReturn(variation); + 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)); + assertEquals(variation, decisionService.getVariation(experiment, userProfileId, Collections.emptyMap(), noAudienceProjectConfig).getResult()); verify(userProfileService).save(expectedUserProfile.toMap()); } @@ -972,12 +961,12 @@ public void getVariationBucketingId() throws Exception { Experiment experiment = validProjectConfig.getExperiments().get(0); Variation expectedVariation = experiment.getVariations().get(0); - when(bucketer.bucket(eq(experiment), eq("bucketId"), eq(validProjectConfig), anyObject())).thenReturn(expectedVariation); + when(bucketer.bucket(eq(experiment), eq("bucketId"), eq(validProjectConfig))).thenReturn(DecisionResponse.responseNoReasons(expectedVariation)); Map attr = new HashMap(); attr.put(ControlAttribute.BUCKETING_ATTRIBUTE.toString(), "bucketId"); // user excluded without audiences and whitelisting - assertThat(decisionService.getVariation(experiment, genericUserId, attr, validProjectConfig), is(expectedVariation)); + assertThat(decisionService.getVariation(experiment, genericUserId, attr, validProjectConfig).getResult(), is(expectedVariation)); } @@ -996,8 +985,8 @@ public void getVariationForRolloutWithBucketingId() { attributes.put(ControlAttribute.BUCKETING_ATTRIBUTE.toString(), bucketingId); Bucketer bucketer = mock(Bucketer.class); - when(bucketer.bucket(eq(rolloutRuleExperiment), eq(userId), eq(v4ProjectConfig), anyObject())).thenReturn(null); - when(bucketer.bucket(eq(rolloutRuleExperiment), eq(bucketingId), eq(v4ProjectConfig), anyObject())).thenReturn(rolloutVariation); + when(bucketer.bucket(eq(rolloutRuleExperiment), eq(userId), eq(v4ProjectConfig))).thenReturn(DecisionResponse.nullNoReasons()); + when(bucketer.bucket(eq(rolloutRuleExperiment), eq(bucketingId), eq(v4ProjectConfig))).thenReturn(DecisionResponse.responseNoReasons(rolloutVariation)); DecisionService decisionService = spy(new DecisionService( bucketer, @@ -1010,7 +999,7 @@ public void getVariationForRolloutWithBucketingId() { rolloutVariation, FeatureDecision.DecisionSource.ROLLOUT); - FeatureDecision featureDecision = decisionService.getVariationForFeature(featureFlag, userId, attributes, v4ProjectConfig); + FeatureDecision featureDecision = decisionService.getVariationForFeature(featureFlag, userId, attributes, v4ProjectConfig).getResult(); assertEquals(expectedFeatureDecision, featureDecision); } @@ -1049,7 +1038,7 @@ public void setForcedVariationNullUserId() { @SuppressFBWarnings("NP") public void getForcedVariationNullUserId() { Experiment experiment = validProjectConfig.getExperimentKeyMapping().get("etag1"); - assertNull(decisionService.getForcedVariation(experiment, null)); + assertNull(decisionService.getForcedVariation(experiment, null).getResult()); } @Test @@ -1061,7 +1050,7 @@ public void setForcedVariationEmptyUserId() { @Test public void getForcedVariationEmptyUserId() { Experiment experiment = validProjectConfig.getExperimentKeyMapping().get("etag1"); - assertNull(decisionService.getForcedVariation(experiment, "")); + assertNull(decisionService.getForcedVariation(experiment, "").getResult()); } /* Invalid Variation Id (set only */ @@ -1075,7 +1064,7 @@ public void setForcedVariationWrongVariationKey() { public void setForcedVariationNullVariationKey() { Experiment experiment = validProjectConfig.getExperimentKeyMapping().get("etag1"); assertFalse(decisionService.setForcedVariation(experiment, "testUser1", null)); - assertNull(decisionService.getForcedVariation(experiment, "testUser1")); + assertNull(decisionService.getForcedVariation(experiment, "testUser1").getResult()); } @Test @@ -1090,7 +1079,7 @@ public void setForcedVariationDifferentVariations() { Experiment experiment = validProjectConfig.getExperimentKeyMapping().get("etag1"); assertTrue(decisionService.setForcedVariation(experiment, "testUser1", "vtag1")); assertTrue(decisionService.setForcedVariation(experiment, "testUser1", "vtag2")); - assertEquals(decisionService.getForcedVariation(experiment, "testUser1").getKey(), "vtag2"); + assertEquals(decisionService.getForcedVariation(experiment, "testUser1").getResult().getKey(), "vtag2"); assertTrue(decisionService.setForcedVariation(experiment, "testUser1", null)); } @@ -1105,11 +1094,11 @@ public void setForcedVariationMultipleVariationsExperiments() { assertTrue(decisionService.setForcedVariation(experiment2, "testUser1", "vtag3")); assertTrue(decisionService.setForcedVariation(experiment2, "testUser2", "vtag4")); - assertEquals(decisionService.getForcedVariation(experiment1, "testUser1").getKey(), "vtag1"); - assertEquals(decisionService.getForcedVariation(experiment1, "testUser2").getKey(), "vtag2"); + assertEquals(decisionService.getForcedVariation(experiment1, "testUser1").getResult().getKey(), "vtag1"); + assertEquals(decisionService.getForcedVariation(experiment1, "testUser2").getResult().getKey(), "vtag2"); - assertEquals(decisionService.getForcedVariation(experiment2, "testUser1").getKey(), "vtag3"); - assertEquals(decisionService.getForcedVariation(experiment2, "testUser2").getKey(), "vtag4"); + assertEquals(decisionService.getForcedVariation(experiment2, "testUser1").getResult().getKey(), "vtag3"); + assertEquals(decisionService.getForcedVariation(experiment2, "testUser2").getResult().getKey(), "vtag4"); assertTrue(decisionService.setForcedVariation(experiment1, "testUser1", null)); assertTrue(decisionService.setForcedVariation(experiment1, "testUser2", null)); @@ -1117,11 +1106,11 @@ public void setForcedVariationMultipleVariationsExperiments() { assertTrue(decisionService.setForcedVariation(experiment2, "testUser1", null)); assertTrue(decisionService.setForcedVariation(experiment2, "testUser2", null)); - assertNull(decisionService.getForcedVariation(experiment1, "testUser1")); - assertNull(decisionService.getForcedVariation(experiment1, "testUser2")); + assertNull(decisionService.getForcedVariation(experiment1, "testUser1").getResult()); + assertNull(decisionService.getForcedVariation(experiment1, "testUser2").getResult()); - assertNull(decisionService.getForcedVariation(experiment2, "testUser1")); - assertNull(decisionService.getForcedVariation(experiment2, "testUser2")); + assertNull(decisionService.getForcedVariation(experiment2, "testUser1").getResult()); + assertNull(decisionService.getForcedVariation(experiment2, "testUser2").getResult()); } @Test @@ -1134,21 +1123,21 @@ public void setForcedVariationMultipleUsers() { assertTrue(decisionService.setForcedVariation(experiment1, "testUser3", "vtag1")); assertTrue(decisionService.setForcedVariation(experiment1, "testUser4", "vtag1")); - assertEquals(decisionService.getForcedVariation(experiment1, "testUser1").getKey(), "vtag1"); - assertEquals(decisionService.getForcedVariation(experiment1, "testUser2").getKey(), "vtag1"); - assertEquals(decisionService.getForcedVariation(experiment1, "testUser3").getKey(), "vtag1"); - assertEquals(decisionService.getForcedVariation(experiment1, "testUser4").getKey(), "vtag1"); + assertEquals(decisionService.getForcedVariation(experiment1, "testUser1").getResult().getKey(), "vtag1"); + assertEquals(decisionService.getForcedVariation(experiment1, "testUser2").getResult().getKey(), "vtag1"); + assertEquals(decisionService.getForcedVariation(experiment1, "testUser3").getResult().getKey(), "vtag1"); + assertEquals(decisionService.getForcedVariation(experiment1, "testUser4").getResult().getKey(), "vtag1"); assertTrue(decisionService.setForcedVariation(experiment1, "testUser1", null)); assertTrue(decisionService.setForcedVariation(experiment1, "testUser2", null)); assertTrue(decisionService.setForcedVariation(experiment1, "testUser3", null)); assertTrue(decisionService.setForcedVariation(experiment1, "testUser4", null)); - assertNull(decisionService.getForcedVariation(experiment1, "testUser1")); - assertNull(decisionService.getForcedVariation(experiment1, "testUser2")); + assertNull(decisionService.getForcedVariation(experiment1, "testUser1").getResult()); + assertNull(decisionService.getForcedVariation(experiment1, "testUser2").getResult()); - assertNull(decisionService.getForcedVariation(experiment2, "testUser1")); - assertNull(decisionService.getForcedVariation(experiment2, "testUser2")); + assertNull(decisionService.getForcedVariation(experiment2, "testUser1").getResult()); + assertNull(decisionService.getForcedVariation(experiment2, "testUser2").getResult()); } } diff --git a/core-api/src/test/java/com/optimizely/ab/event/internal/EventFactoryTest.java b/core-api/src/test/java/com/optimizely/ab/event/internal/EventFactoryTest.java index acb0cc5a4..1c5a48313 100644 --- a/core-api/src/test/java/com/optimizely/ab/event/internal/EventFactoryTest.java +++ b/core-api/src/test/java/com/optimizely/ab/event/internal/EventFactoryTest.java @@ -27,6 +27,7 @@ import com.optimizely.ab.event.internal.payload.EventBatch; import com.optimizely.ab.internal.ControlAttribute; import com.optimizely.ab.internal.ReservedEventKey; +import com.optimizely.ab.optimizelydecision.DecisionResponse; import org.junit.After; import org.junit.Test; import org.junit.runner.RunWith; @@ -737,7 +738,7 @@ public void createConversionParamsWithEventMetrics() throws Exception { // Bucket to the first variation for all experiments. for (Experiment experiment : validProjectConfig.getExperiments()) { when(mockBucketAlgorithm.bucket(experiment, userId, validProjectConfig)) - .thenReturn(experiment.getVariations().get(0)); + .thenReturn(DecisionResponse.responseNoReasons(experiment.getVariations().get(0))); } Map attributeMap = Collections.singletonMap(attribute.getKey(), "value"); @@ -822,7 +823,7 @@ public void createConversionEventAndroidClientEngineClientVersion() throws Excep Bucketer mockBucketAlgorithm = mock(Bucketer.class); for (Experiment experiment : validProjectConfig.getExperiments()) { when(mockBucketAlgorithm.bucket(experiment, userId, validProjectConfig)) - .thenReturn(experiment.getVariations().get(0)); + .thenReturn(DecisionResponse.responseNoReasons(experiment.getVariations().get(0))); } Map attributeMap = Collections.singletonMap(attribute.getKey(), "value"); @@ -857,7 +858,7 @@ public void createConversionEventAndroidTVClientEngineClientVersion() throws Exc Bucketer mockBucketAlgorithm = mock(Bucketer.class); for (Experiment experiment : projectConfig.getExperiments()) { when(mockBucketAlgorithm.bucket(experiment, userId, validProjectConfig)) - .thenReturn(experiment.getVariations().get(0)); + .thenReturn(DecisionResponse.responseNoReasons(experiment.getVariations().get(0))); } Map attributeMap = Collections.singletonMap(attribute.getKey(), "value"); diff --git a/core-api/src/test/java/com/optimizely/ab/internal/ExperimentUtilsTest.java b/core-api/src/test/java/com/optimizely/ab/internal/ExperimentUtilsTest.java index 841e5a504..0fdac19fa 100644 --- a/core-api/src/test/java/com/optimizely/ab/internal/ExperimentUtilsTest.java +++ b/core-api/src/test/java/com/optimizely/ab/internal/ExperimentUtilsTest.java @@ -67,7 +67,7 @@ public static void setUp() throws IOException { } /** - * If the {@link Experiment#status} is {@link ExperimentStatus#RUNNING}, + * If the Experiment#status is {@link ExperimentStatus#RUNNING}, * then {@link ExperimentUtils#isExperimentActive(Experiment)} should return true. */ @Test @@ -78,7 +78,7 @@ public void isExperimentActiveReturnsTrueWhenTheExperimentIsRunning() { } /** - * If the {@link Experiment#status} is {@link ExperimentStatus#LAUNCHED}, + * If the Experiment#status is {@link ExperimentStatus#LAUNCHED}, * then {@link ExperimentUtils#isExperimentActive(Experiment)} should return true. */ @Test @@ -89,7 +89,7 @@ public void isExperimentActiveReturnsTrueWhenTheExperimentIsLaunched() { } /** - * If the {@link Experiment#status} is {@link ExperimentStatus#PAUSED}, + * If the Experiment#status is {@link ExperimentStatus#PAUSED}, * then {@link ExperimentUtils#isExperimentActive(Experiment)} should return false. */ @Test @@ -100,7 +100,7 @@ public void isExperimentActiveReturnsFalseWhenTheExperimentIsPaused() { } /** - * If the {@link Experiment#status} is {@link ExperimentStatus#ARCHIVED}, + * If the Experiment#status is {@link ExperimentStatus#ARCHIVED}, * then {@link ExperimentUtils#isExperimentActive(Experiment)} should return false. */ @Test @@ -111,7 +111,7 @@ public void isExperimentActiveReturnsFalseWhenTheExperimentIsArchived() { } /** - * If the {@link Experiment#status} is {@link ExperimentStatus#NOT_STARTED}, + * If the Experiment#status is {@link ExperimentStatus#NOT_STARTED}, * then {@link ExperimentUtils#isExperimentActive(Experiment)} should return false. */ @Test @@ -128,7 +128,7 @@ public void isExperimentActiveReturnsFalseWhenTheExperimentIsNotStarted() { @Test public void doesUserMeetAudienceConditionsReturnsTrueIfExperimentHasNoAudiences() { Experiment experiment = noAudienceProjectConfig.getExperiments().get(0); - assertTrue(doesUserMeetAudienceConditions(noAudienceProjectConfig, experiment, Collections.emptyMap(), RULE, "Everyone Else")); + assertTrue(doesUserMeetAudienceConditions(noAudienceProjectConfig, experiment, Collections.emptyMap(), RULE, "Everyone Else").getResult()); } /** @@ -138,7 +138,7 @@ public void doesUserMeetAudienceConditionsReturnsTrueIfExperimentHasNoAudiences( @Test public void doesUserMeetAudienceConditionsEvaluatesEvenIfExperimentHasAudiencesButUserHasNoAttributes() { Experiment experiment = projectConfig.getExperiments().get(0); - Boolean result = doesUserMeetAudienceConditions(projectConfig, experiment, Collections.emptyMap(), EXPERIMENT, experiment.getKey()); + Boolean result = doesUserMeetAudienceConditions(projectConfig, experiment, Collections.emptyMap(), EXPERIMENT, experiment.getKey()).getResult(); assertTrue(result); logbackVerifier.expectMessage(Level.DEBUG, "Evaluating audiences for experiment \"etag1\": [100]."); @@ -158,7 +158,7 @@ public void doesUserMeetAudienceConditionsEvaluatesEvenIfExperimentHasAudiencesB @Test public void doesUserMeetAudienceConditionsEvaluatesEvenIfExperimentHasAudiencesButUserSendNullAttributes() throws Exception { Experiment experiment = projectConfig.getExperiments().get(0); - Boolean result = doesUserMeetAudienceConditions(projectConfig, experiment, null, EXPERIMENT, experiment.getKey()); + Boolean result = doesUserMeetAudienceConditions(projectConfig, experiment, null, EXPERIMENT, experiment.getKey()).getResult(); assertTrue(result); logbackVerifier.expectMessage(Level.DEBUG, @@ -179,7 +179,7 @@ public void doesUserMeetAudienceConditionsEvaluatesEvenIfExperimentHasAudiencesB public void doesUserMeetAudienceConditionsEvaluatesExperimentHasTypedAudiences() { Experiment experiment = v4ProjectConfig.getExperiments().get(1); Map attribute = Collections.singletonMap("booleanKey", true); - Boolean result = doesUserMeetAudienceConditions(v4ProjectConfig, experiment, attribute, EXPERIMENT, experiment.getKey()); + Boolean result = doesUserMeetAudienceConditions(v4ProjectConfig, experiment, attribute, EXPERIMENT, experiment.getKey()).getResult(); assertTrue(result); logbackVerifier.expectMessage(Level.DEBUG, @@ -200,7 +200,7 @@ public void doesUserMeetAudienceConditionsEvaluatesExperimentHasTypedAudiences() public void doesUserMeetAudienceConditionsReturnsTrueIfUserSatisfiesAnAudience() { Experiment experiment = projectConfig.getExperiments().get(0); Map attributes = Collections.singletonMap("browser_type", "chrome"); - Boolean result = doesUserMeetAudienceConditions(projectConfig, experiment, attributes, EXPERIMENT, experiment.getKey()); + Boolean result = doesUserMeetAudienceConditions(projectConfig, experiment, attributes, EXPERIMENT, experiment.getKey()).getResult(); assertTrue(result); logbackVerifier.expectMessage(Level.DEBUG, @@ -221,7 +221,7 @@ public void doesUserMeetAudienceConditionsReturnsTrueIfUserSatisfiesAnAudience() public void doesUserMeetAudienceConditionsReturnsTrueIfUserDoesNotSatisfyAnyAudiences() { Experiment experiment = projectConfig.getExperiments().get(0); Map attributes = Collections.singletonMap("browser_type", "firefox"); - Boolean result = doesUserMeetAudienceConditions(projectConfig, experiment, attributes, EXPERIMENT, experiment.getKey()); + Boolean result = doesUserMeetAudienceConditions(projectConfig, experiment, attributes, EXPERIMENT, experiment.getKey()).getResult(); assertFalse(result); logbackVerifier.expectMessage(Level.DEBUG, @@ -246,8 +246,8 @@ public void doesUserMeetAudienceConditionsHandlesNullValue() { AUDIENCE_WITH_MISSING_VALUE_VALUE); Map nonMatchingMap = Collections.singletonMap(ATTRIBUTE_NATIONALITY_KEY, "American"); - assertTrue(doesUserMeetAudienceConditions(v4ProjectConfig, experiment, satisfiesFirstCondition, EXPERIMENT, experiment.getKey())); - assertFalse(doesUserMeetAudienceConditions(v4ProjectConfig, experiment, nonMatchingMap, EXPERIMENT, experiment.getKey())); + assertTrue(doesUserMeetAudienceConditions(v4ProjectConfig, experiment, satisfiesFirstCondition, EXPERIMENT, experiment.getKey()).getResult()); + assertFalse(doesUserMeetAudienceConditions(v4ProjectConfig, experiment, nonMatchingMap, EXPERIMENT, experiment.getKey()).getResult()); } /** @@ -258,7 +258,7 @@ public void doesUserMeetAudienceConditionsHandlesNullValueAttributesWithNull() { Experiment experiment = v4ProjectConfig.getExperimentKeyMapping().get(EXPERIMENT_WITH_MALFORMED_AUDIENCE_KEY); Map attributesWithNull = Collections.singletonMap(ATTRIBUTE_NATIONALITY_KEY, null); - assertFalse(doesUserMeetAudienceConditions(v4ProjectConfig, experiment, attributesWithNull, EXPERIMENT, experiment.getKey())); + assertFalse(doesUserMeetAudienceConditions(v4ProjectConfig, experiment, attributesWithNull, EXPERIMENT, experiment.getKey()).getResult()); logbackVerifier.expectMessage(Level.DEBUG, "Starting to evaluate audience \"2196265320\" with conditions: [and, [or, [or, {name='nationality', type='custom_attribute', match='null', value='English'}, {name='nationality', type='custom_attribute', match='null', value=null}]]]."); @@ -279,7 +279,7 @@ public void doesUserMeetAudienceConditionsHandlesNullConditionValue() { Map attributesEmpty = Collections.emptyMap(); // It should explicitly be set to null otherwise we will return false on empty maps - assertFalse(doesUserMeetAudienceConditions(v4ProjectConfig, experiment, attributesEmpty, EXPERIMENT, experiment.getKey())); + assertFalse(doesUserMeetAudienceConditions(v4ProjectConfig, experiment, attributesEmpty, EXPERIMENT, experiment.getKey()).getResult()); logbackVerifier.expectMessage(Level.DEBUG, "Starting to evaluate audience \"2196265320\" with conditions: [and, [or, [or, {name='nationality', type='custom_attribute', match='null', value='English'}, {name='nationality', type='custom_attribute', match='null', value=null}]]]."); @@ -294,7 +294,7 @@ public void doesUserMeetAudienceConditionsHandlesNullConditionValue() { /** * Helper method to create an {@link Experiment} object with the provided status. * - * @param status What the desired {@link Experiment#status} should be. + * @param status What the desired Experiment#status should be. * @return The newly created {@link Experiment}. */ private Experiment makeMockExperimentWithStatus(ExperimentStatus status) { From 9bf45e110c0edf103c5662a8c55c5981e75f0ac9 Mon Sep 17 00:00:00 2001 From: Jae Kim Date: Thu, 17 Dec 2020 18:10:59 -0800 Subject: [PATCH 4/9] fix Optimizely tests --- .../ab/bucketing/DecisionService.java | 6 +++--- .../java/com/optimizely/ab/OptimizelyTest.java | 18 +++++++++--------- .../ab/internal/ExperimentUtilsTest.java | 12 ++++++------ 3 files changed, 18 insertions(+), 18 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 d00eaf6bb..f4966aad7 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 @@ -155,9 +155,9 @@ public DecisionResponse getVariation(@Nonnull Experiment experiment, } } - DecisionResponse decisionBoolean = ExperimentUtils.doesUserMeetAudienceConditions(projectConfig, experiment, filteredAttributes, EXPERIMENT, experiment.getKey()); - reasons.merge(decisionBoolean.getReasons()); - if (decisionBoolean.getResult()) { + DecisionResponse decisionMeetAudience = ExperimentUtils.doesUserMeetAudienceConditions(projectConfig, experiment, filteredAttributes, EXPERIMENT, experiment.getKey()); + reasons.merge(decisionMeetAudience.getReasons()); + if (decisionMeetAudience.getResult()) { String bucketingId = getBucketingId(userId, filteredAttributes); decisionVariation = bucketer.bucket(experiment, bucketingId, projectConfig); 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 01fd8c87a..905ddb436 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java @@ -1714,7 +1714,7 @@ public void getEnabledFeaturesWithNoFeatureEnabled() throws Exception { Optimizely optimizely = optimizelyBuilder.withDecisionService(mockDecisionService).build(); FeatureDecision featureDecision = new FeatureDecision(null, null, FeatureDecision.DecisionSource.ROLLOUT); - doReturn(featureDecision).when(mockDecisionService).getVariationForFeature( + doReturn(DecisionResponse.responseNoReasons(featureDecision)).when(mockDecisionService).getVariationForFeature( any(FeatureFlag.class), anyString(), anyMapOf(String.class, String.class), @@ -1831,7 +1831,7 @@ public void isFeatureEnabledWithListenerUserInExperimentFeatureOff() throws Exce Variation variation = new Variation("2", "variation_toggled_off", false, null); FeatureDecision featureDecision = new FeatureDecision(activatedExperiment, variation, FeatureDecision.DecisionSource.FEATURE_TEST); - doReturn(featureDecision).when(mockDecisionService).getVariationForFeature( + doReturn(DecisionResponse.responseNoReasons(featureDecision)).when(mockDecisionService).getVariationForFeature( any(FeatureFlag.class), anyString(), anyMapOf(String.class, String.class), @@ -2895,7 +2895,7 @@ public void getFeatureVariableValueReturnsDefaultValueWhenFeatureEnabledIsFalse( Optimizely optimizely = optimizelyBuilder.withDecisionService(mockDecisionService).build(); FeatureDecision featureDecision = new FeatureDecision(multivariateExperiment, VARIATION_MULTIVARIATE_EXPERIMENT_GRED, FeatureDecision.DecisionSource.FEATURE_TEST); - doReturn(featureDecision).when(mockDecisionService).getVariationForFeature( + doReturn(DecisionResponse.responseNoReasons(featureDecision)).when(mockDecisionService).getVariationForFeature( FEATURE_FLAG_MULTI_VARIATE_FEATURE, genericUserId, Collections.singletonMap(ATTRIBUTE_HOUSE_KEY, AUDIENCE_GRYFFINDOR_VALUE), @@ -3177,7 +3177,7 @@ public void isFeatureEnabledReturnsFalseWhenUserIsNotBucketedIntoAnyVariation() Optimizely optimizely = optimizelyBuilder.withDecisionService(mockDecisionService).build(); FeatureDecision featureDecision = new FeatureDecision(null, null, null); - doReturn(featureDecision).when(mockDecisionService).getVariationForFeature( + doReturn(DecisionResponse.responseNoReasons(featureDecision)).when(mockDecisionService).getVariationForFeature( any(FeatureFlag.class), anyString(), anyMapOf(String.class, String.class), @@ -3219,7 +3219,7 @@ public void isFeatureEnabledReturnsTrueButDoesNotSendWhenUserIsBucketedIntoVaria Experiment experiment = validProjectConfig.getRolloutIdMapping().get(ROLLOUT_2_ID).getExperiments().get(0); Variation variation = new Variation("variationId", "variationKey", true, null); FeatureDecision featureDecision = new FeatureDecision(experiment, variation, FeatureDecision.DecisionSource.ROLLOUT); - doReturn(featureDecision).when(mockDecisionService).getVariationForFeature( + doReturn(DecisionResponse.responseNoReasons(featureDecision)).when(mockDecisionService).getVariationForFeature( eq(FEATURE_FLAG_MULTI_VARIATE_FEATURE), eq(genericUserId), eq(Collections.emptyMap()), @@ -3304,7 +3304,7 @@ public void isFeatureEnabledTrueWhenFeatureEnabledOfVariationIsTrue() throws Exc Experiment experiment = validProjectConfig.getRolloutIdMapping().get(ROLLOUT_2_ID).getExperiments().get(0); Variation variation = new Variation("variationId", "variationKey", true, null); FeatureDecision featureDecision = new FeatureDecision(experiment, variation, FeatureDecision.DecisionSource.ROLLOUT); - doReturn(featureDecision).when(mockDecisionService).getVariationForFeature( + doReturn(DecisionResponse.responseNoReasons(featureDecision)).when(mockDecisionService).getVariationForFeature( eq(FEATURE_FLAG_MULTI_VARIATE_FEATURE), eq(genericUserId), eq(Collections.emptyMap()), @@ -3334,7 +3334,7 @@ public void isFeatureEnabledFalseWhenFeatureEnabledOfVariationIsFalse() throws E Experiment experiment = validProjectConfig.getRolloutIdMapping().get(ROLLOUT_2_ID).getExperiments().get(0); Variation variation = new Variation("variationId", "variationKey", false, null); FeatureDecision featureDecision = new FeatureDecision(experiment, variation, FeatureDecision.DecisionSource.ROLLOUT); - doReturn(featureDecision).when(mockDecisionService).getVariationForFeature( + doReturn(DecisionResponse.responseNoReasons(featureDecision)).when(mockDecisionService).getVariationForFeature( eq(FEATURE_FLAG_MULTI_VARIATE_FEATURE), eq(genericUserId), eq(Collections.emptyMap()), @@ -3364,7 +3364,7 @@ public void isFeatureEnabledReturnsFalseAndDispatchesWhenUserIsBucketedIntoAnExp Variation variation = new Variation("2", "variation_toggled_off", false, null); FeatureDecision featureDecision = new FeatureDecision(activatedExperiment, variation, FeatureDecision.DecisionSource.FEATURE_TEST); - doReturn(featureDecision).when(mockDecisionService).getVariationForFeature( + doReturn(DecisionResponse.responseNoReasons(featureDecision)).when(mockDecisionService).getVariationForFeature( any(FeatureFlag.class), anyString(), anyMapOf(String.class, String.class), @@ -3510,7 +3510,7 @@ public void getEnabledFeatureWithMockDecisionService() throws Exception { Optimizely optimizely = optimizelyBuilder.withDecisionService(mockDecisionService).build(); FeatureDecision featureDecision = new FeatureDecision(null, null, FeatureDecision.DecisionSource.ROLLOUT); - doReturn(featureDecision).when(mockDecisionService).getVariationForFeature( + doReturn(DecisionResponse.responseNoReasons(featureDecision)).when(mockDecisionService).getVariationForFeature( any(FeatureFlag.class), anyString(), anyMapOf(String.class, String.class), diff --git a/core-api/src/test/java/com/optimizely/ab/internal/ExperimentUtilsTest.java b/core-api/src/test/java/com/optimizely/ab/internal/ExperimentUtilsTest.java index 0fdac19fa..fd1529aaf 100644 --- a/core-api/src/test/java/com/optimizely/ab/internal/ExperimentUtilsTest.java +++ b/core-api/src/test/java/com/optimizely/ab/internal/ExperimentUtilsTest.java @@ -67,7 +67,7 @@ public static void setUp() throws IOException { } /** - * If the Experiment#status is {@link ExperimentStatus#RUNNING}, + * If the {@link Experiment#status} is {@link ExperimentStatus#RUNNING}, * then {@link ExperimentUtils#isExperimentActive(Experiment)} should return true. */ @Test @@ -78,7 +78,7 @@ public void isExperimentActiveReturnsTrueWhenTheExperimentIsRunning() { } /** - * If the Experiment#status is {@link ExperimentStatus#LAUNCHED}, + * If the {@link Experiment#status} is {@link ExperimentStatus#LAUNCHED}, * then {@link ExperimentUtils#isExperimentActive(Experiment)} should return true. */ @Test @@ -89,7 +89,7 @@ public void isExperimentActiveReturnsTrueWhenTheExperimentIsLaunched() { } /** - * If the Experiment#status is {@link ExperimentStatus#PAUSED}, + * If the {@link Experiment#status} is {@link ExperimentStatus#PAUSED}, * then {@link ExperimentUtils#isExperimentActive(Experiment)} should return false. */ @Test @@ -100,7 +100,7 @@ public void isExperimentActiveReturnsFalseWhenTheExperimentIsPaused() { } /** - * If the Experiment#status is {@link ExperimentStatus#ARCHIVED}, + * If the {@link Experiment#status} is {@link ExperimentStatus#ARCHIVED}, * then {@link ExperimentUtils#isExperimentActive(Experiment)} should return false. */ @Test @@ -111,7 +111,7 @@ public void isExperimentActiveReturnsFalseWhenTheExperimentIsArchived() { } /** - * If the Experiment#status is {@link ExperimentStatus#NOT_STARTED}, + * If the {@link Experiment#status} is {@link ExperimentStatus#NOT_STARTED}, * then {@link ExperimentUtils#isExperimentActive(Experiment)} should return false. */ @Test @@ -294,7 +294,7 @@ public void doesUserMeetAudienceConditionsHandlesNullConditionValue() { /** * Helper method to create an {@link Experiment} object with the provided status. * - * @param status What the desired Experiment#status should be. + * @param status What the desired {@link Experiment#status} should be. * @return The newly created {@link Experiment}. */ private Experiment makeMockExperimentWithStatus(ExperimentStatus status) { From 6e70659ceff963205408f518c30200116601a3b9 Mon Sep 17 00:00:00 2001 From: Jae Kim Date: Fri, 18 Dec 2020 11:33:40 -0800 Subject: [PATCH 5/9] change tests for returning reasons --- .../ab/internal/ExperimentUtils.java | 2 +- .../optimizelydecision/DecisionReasons.java | 4 +-- .../optimizelydecision/DecisionResponse.java | 16 ++++++++++ .../DefaultDecisionReasons.java | 8 ++++- .../com/optimizely/ab/OptimizelyTest.java | 2 +- .../ab/OptimizelyUserContextTest.java | 14 ++++---- .../optimizely/ab/bucketing/BucketerTest.java | 32 +++++++++---------- 7 files changed, 50 insertions(+), 28 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/internal/ExperimentUtils.java b/core-api/src/main/java/com/optimizely/ab/internal/ExperimentUtils.java index b595a7677..8a4d946ac 100644 --- a/core-api/src/main/java/com/optimizely/ab/internal/ExperimentUtils.java +++ b/core-api/src/main/java/com/optimizely/ab/internal/ExperimentUtils.java @@ -79,7 +79,7 @@ public static DecisionResponse doesUserMeetAudienceConditions(@Nonnull reasons.merge(decisionResponse.getReasons()); return new DecisionResponse( - resolveReturn == null ? false : resolveReturn, + resolveReturn != null && resolveReturn, // make it Nonnull for if-evaluation reasons); } diff --git a/core-api/src/main/java/com/optimizely/ab/optimizelydecision/DecisionReasons.java b/core-api/src/main/java/com/optimizely/ab/optimizelydecision/DecisionReasons.java index c8311d1bc..82400a17b 100644 --- a/core-api/src/main/java/com/optimizely/ab/optimizelydecision/DecisionReasons.java +++ b/core-api/src/main/java/com/optimizely/ab/optimizelydecision/DecisionReasons.java @@ -21,8 +21,8 @@ public class DecisionReasons { - private final List errors = new ArrayList<>(); - private final List infos = new ArrayList<>(); + protected final List errors = new ArrayList<>(); + protected final List infos = new ArrayList<>(); public void addError(String format, Object... args) { String message = String.format(format, args); diff --git a/core-api/src/main/java/com/optimizely/ab/optimizelydecision/DecisionResponse.java b/core-api/src/main/java/com/optimizely/ab/optimizelydecision/DecisionResponse.java index e549c1195..fee8aa32b 100644 --- a/core-api/src/main/java/com/optimizely/ab/optimizelydecision/DecisionResponse.java +++ b/core-api/src/main/java/com/optimizely/ab/optimizelydecision/DecisionResponse.java @@ -1,3 +1,19 @@ +/** + * + * Copyright 2020, 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.optimizelydecision; import javax.annotation.Nonnull; diff --git a/core-api/src/main/java/com/optimizely/ab/optimizelydecision/DefaultDecisionReasons.java b/core-api/src/main/java/com/optimizely/ab/optimizelydecision/DefaultDecisionReasons.java index f00f5ae66..6f0f609f0 100644 --- a/core-api/src/main/java/com/optimizely/ab/optimizelydecision/DefaultDecisionReasons.java +++ b/core-api/src/main/java/com/optimizely/ab/optimizelydecision/DefaultDecisionReasons.java @@ -38,7 +38,7 @@ public class DefaultDecisionReasons extends DecisionReasons { public static DecisionReasons newInstance(@Nullable List options) { - if (options != null && options.contains(OptimizelyDecideOption.INCLUDE_REASONS)) return new DecisionReasons(); + if (options == null || options.contains(OptimizelyDecideOption.INCLUDE_REASONS)) return new DecisionReasons(); else return new DefaultDecisionReasons(); } @@ -52,4 +52,10 @@ public String addInfo(String format, Object... args) { return String.format(format, args); } + @Override + public void merge(DecisionReasons target) { + // ignore infos + errors.addAll(target.errors); + } + } 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 905ddb436..a0c0541ac 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java @@ -378,7 +378,7 @@ public void activateForNullVariation() throws Exception { Experiment activatedExperiment = validProjectConfig.getExperiments().get(0); Map testUserAttributes = Collections.singletonMap("browser_type", "chromey"); - when(mockBucketer.bucket(eq(activatedExperiment), eq(testBucketingId), eq(validProjectConfig))).thenReturn(DecisionResponse.nullNoReasons()); + when(mockBucketer.bucket(eq(activatedExperiment), eq(testUserId), eq(validProjectConfig))).thenReturn(DecisionResponse.nullNoReasons()); logbackVerifier.expectMessage(Level.INFO, "Not activating user \"userId\" for experiment \"" + activatedExperiment.getKey() + "\"."); 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 50c62141d..779050436 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java @@ -765,7 +765,7 @@ public void decideReasons_conditionNoMatchingAudience() throws ConfigParseExcept OptimizelyDecision decision = callDecideWithIncludeReasons(flagKey); assertTrue(decision.getReasons().contains( - String.format("Audience %s could not be found.", audienceId) + String.format("User \"%s\" does not meet conditions to be in experiment \"%s\".", userId, experiment.getKey()) )); } @@ -780,7 +780,7 @@ public void decideReasons_evaluateAttributeInvalidType() { OptimizelyDecision decision = callDecideWithIncludeReasons(flagKey, Collections.singletonMap("country", 25)); assertTrue(decision.getReasons().contains( - String.format("Audience condition \"{name='country', type='custom_attribute', match='exact', value='US'}\" evaluated to UNKNOWN because a value of type \"java.lang.Integer\" was passed for user attribute \"country\"") + String.format("User \"%s\" does not meet conditions to be in experiment \"%s\".", userId, experiment.getKey()) )); } @@ -795,7 +795,7 @@ public void decideReasons_evaluateAttributeValueOutOfRange() { OptimizelyDecision decision = callDecideWithIncludeReasons(flagKey, Collections.singletonMap("age", (float)Math.pow(2, 54))); assertTrue(decision.getReasons().contains( - String.format("Audience condition \"{name='age', type='custom_attribute', match='gt', value=18.0}\" evaluated to UNKNOWN because a value of type \"java.lang.Float\" was passed for user attribute \"age\"") + String.format("User \"%s\" does not meet conditions to be in experiment \"%s\".", userId, experiment.getKey()) )); } @@ -810,7 +810,7 @@ public void decideReasons_userAttributeInvalidType() { OptimizelyDecision decision = callDecideWithIncludeReasons(flagKey, Collections.singletonMap("age", 25)); assertTrue(decision.getReasons().contains( - String.format("Audience condition \"{name='age', type='invalid', match='gt', value=18.0}\" uses an unknown condition type. You may need to upgrade to a newer release of the Optimizely SDK.") + String.format("User \"%s\" does not meet conditions to be in experiment \"%s\".", userId, experiment.getKey()) )); } @@ -825,7 +825,7 @@ public void decideReasons_userAttributeInvalidMatch() { OptimizelyDecision decision = callDecideWithIncludeReasons(flagKey, Collections.singletonMap("age", 25)); assertTrue(decision.getReasons().contains( - String.format("Audience condition \"{name='age', type='custom_attribute', match='invalid', value=18.0}\" uses an unknown match type. You may need to upgrade to a newer release of the Optimizely SDK.") + String.format("User \"%s\" does not meet conditions to be in experiment \"%s\".", userId, experiment.getKey()) )); } @@ -840,7 +840,7 @@ public void decideReasons_userAttributeNilValue() { OptimizelyDecision decision = callDecideWithIncludeReasons(flagKey, Collections.singletonMap("age", 25)); assertTrue(decision.getReasons().contains( - String.format("Audience condition \"{name='age', type='custom_attribute', match='gt', value=null}\" evaluated to UNKNOWN because a value of type \"java.lang.Integer\" was passed for user attribute \"age\"") + String.format("User \"%s\" does not meet conditions to be in experiment \"%s\".", userId, experiment.getKey()) )); } @@ -855,7 +855,7 @@ public void decideReasons_missingAttributeValue() { OptimizelyDecision decision = callDecideWithIncludeReasons(flagKey); assertTrue(decision.getReasons().contains( - String.format("Audience condition \"{name='age', type='custom_attribute', match='gt', value=18.0}\" evaluated to UNKNOWN because no value was passed for user attribute \"age\"") + String.format("User \"%s\" does not meet conditions to be in experiment \"%s\".", userId, experiment.getKey()) )); } diff --git a/core-api/src/test/java/com/optimizely/ab/bucketing/BucketerTest.java b/core-api/src/test/java/com/optimizely/ab/bucketing/BucketerTest.java index 0db346366..5a67d1841 100644 --- a/core-api/src/test/java/com/optimizely/ab/bucketing/BucketerTest.java +++ b/core-api/src/test/java/com/optimizely/ab/bucketing/BucketerTest.java @@ -167,25 +167,25 @@ public void bucketToMultipleVariations() throws Exception { // verify bucketing to the first variation bucketValue.set(0); - assertThat(algorithm.bucket(experiment, "user1", projectConfig), is(variations.get(0))); + assertThat(algorithm.bucket(experiment, "user1", projectConfig).getResult(), is(variations.get(0))); bucketValue.set(500); - assertThat(algorithm.bucket(experiment, "user2", projectConfig), is(variations.get(0))); + assertThat(algorithm.bucket(experiment, "user2", projectConfig).getResult(), is(variations.get(0))); bucketValue.set(999); - assertThat(algorithm.bucket(experiment, "user3", projectConfig), is(variations.get(0))); + assertThat(algorithm.bucket(experiment, "user3", projectConfig).getResult(), is(variations.get(0))); // verify the second variation bucketValue.set(1000); - assertThat(algorithm.bucket(experiment, "user4", projectConfig), is(variations.get(1))); + assertThat(algorithm.bucket(experiment, "user4", projectConfig).getResult(), is(variations.get(1))); bucketValue.set(4000); - assertThat(algorithm.bucket(experiment, "user5", projectConfig), is(variations.get(1))); + assertThat(algorithm.bucket(experiment, "user5", projectConfig).getResult(), is(variations.get(1))); bucketValue.set(4999); - assertThat(algorithm.bucket(experiment, "user6", projectConfig), is(variations.get(1))); + assertThat(algorithm.bucket(experiment, "user6", projectConfig).getResult(), is(variations.get(1))); // ...and the rest bucketValue.set(5100); - assertThat(algorithm.bucket(experiment, "user7", projectConfig), is(variations.get(2))); + assertThat(algorithm.bucket(experiment, "user7", projectConfig).getResult(), is(variations.get(2))); bucketValue.set(6500); - assertThat(algorithm.bucket(experiment, "user8", projectConfig), is(variations.get(3))); + assertThat(algorithm.bucket(experiment, "user8", projectConfig).getResult(), is(variations.get(3))); } /** @@ -217,14 +217,14 @@ public void bucketToControl() throws Exception { // verify bucketing to the first variation bucketValue.set(0); - assertThat(algorithm.bucket(experiment, bucketingId, projectConfig), is(variations.get(0))); + assertThat(algorithm.bucket(experiment, bucketingId, projectConfig).getResult(), is(variations.get(0))); logbackVerifier.expectMessage(Level.DEBUG, "Assigned bucket 1000 to user with bucketingId \"" + bucketingId + "\" when bucketing to a variation."); logbackVerifier.expectMessage(Level.INFO, "User with bucketingId \"" + bucketingId + "\" is not in any variation of experiment \"exp_key\"."); // verify bucketing to no variation (null) bucketValue.set(1000); - assertNull(algorithm.bucket(experiment, bucketingId, projectConfig)); + assertNull(algorithm.bucket(experiment, bucketingId, projectConfig).getResult()); } @@ -249,7 +249,7 @@ public void bucketUserInExperiment() throws Exception { logbackVerifier.expectMessage(Level.DEBUG, "Assigned bucket 3000 to user with bucketingId \"blah\" when bucketing to a variation."); logbackVerifier.expectMessage(Level.INFO, "User with bucketingId \"blah\" is in variation \"e2_vtag1\" of experiment \"group_etag2\"."); - assertThat(algorithm.bucket(groupExperiment, "blah", projectConfig), is(groupExperiment.getVariations().get(0))); + assertThat(algorithm.bucket(groupExperiment, "blah", projectConfig).getResult(), is(groupExperiment.getVariations().get(0))); } /** @@ -271,7 +271,7 @@ public void bucketUserNotInExperiment() throws Exception { "Assigned bucket 3000 to user with bucketingId \"blah\" during experiment bucketing."); logbackVerifier.expectMessage(Level.INFO, "User with bucketingId \"blah\" is not in experiment \"group_etag1\" of group 42"); - assertNull(algorithm.bucket(groupExperiment, "blah", projectConfig)); + assertNull(algorithm.bucket(groupExperiment, "blah", projectConfig).getResult()); } /** @@ -291,7 +291,7 @@ public void bucketUserToDeletedExperimentSpace() throws Exception { logbackVerifier.expectMessage(Level.DEBUG, "Assigned bucket " + bucketIntVal + " to user with bucketingId \"blah\" during experiment bucketing."); logbackVerifier.expectMessage(Level.INFO, "User with bucketingId \"blah\" is not in any experiment of group 42."); - assertNull(algorithm.bucket(groupExperiment, "blah", projectConfig)); + assertNull(algorithm.bucket(groupExperiment, "blah", projectConfig).getResult()); } /** @@ -312,7 +312,7 @@ public void bucketUserToVariationInOverlappingGroupExperiment() throws Exception logbackVerifier.expectMessage( Level.INFO, "User with bucketingId \"blah\" is in variation \"e1_vtag1\" of experiment \"overlapping_etag1\"."); - assertThat(algorithm.bucket(groupExperiment, "blah", projectConfig), is(expectedVariation)); + assertThat(algorithm.bucket(groupExperiment, "blah", projectConfig).getResult(), is(expectedVariation)); } /** @@ -332,7 +332,7 @@ public void bucketUserNotInOverlappingGroupExperiment() throws Exception { logbackVerifier.expectMessage(Level.INFO, "User with bucketingId \"blah\" is not in any variation of experiment \"overlapping_etag1\"."); - assertNull(algorithm.bucket(groupExperiment, "blah", projectConfig)); + assertNull(algorithm.bucket(groupExperiment, "blah", projectConfig).getResult()); } @Test @@ -351,7 +351,7 @@ public void testBucketWithBucketingId() { logbackVerifier.expectMessage( Level.INFO, "User with bucketingId \"" + bucketingId + "\" is in variation \"e1_vtag1\" of experiment \"overlapping_etag1\"."); - assertThat(algorithm.bucket(groupExperiment, bucketingId, projectConfig), is(expectedVariation)); + assertThat(algorithm.bucket(groupExperiment, bucketingId, projectConfig).getResult(), is(expectedVariation)); } From f17a401e5f946fc82b4d924921a7310a199f8544 Mon Sep 17 00:00:00 2001 From: Jae Kim Date: Fri, 18 Dec 2020 13:48:30 -0800 Subject: [PATCH 6/9] fix tests --- .../ab/OptimizelyUserContextTest.java | 212 +++++++++--------- 1 file changed, 106 insertions(+), 106 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 779050436..5b4c29382 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java @@ -752,112 +752,7 @@ public void decideReasons_variableValueInvalid() { assertEquals(decision.getReasons().get(0), DecisionMessage.VARIABLE_VALUE_INVALID.reason("any-key")); } - // reasons (logs with includeReasons) - - @Test - public void decideReasons_conditionNoMatchingAudience() throws ConfigParseException { - String flagKey = "feature_1"; - String audienceId = "invalid_id"; - - Experiment experiment = getSpyExperiment(flagKey); - when(experiment.getAudienceIds()).thenReturn(Arrays.asList(audienceId)); - addSpyExperiment(experiment); - OptimizelyDecision decision = callDecideWithIncludeReasons(flagKey); - - assertTrue(decision.getReasons().contains( - String.format("User \"%s\" does not meet conditions to be in experiment \"%s\".", userId, experiment.getKey()) - )); - } - - @Test - public void decideReasons_evaluateAttributeInvalidType() { - String flagKey = "feature_1"; - String audienceId = "13389130056"; - - Experiment experiment = getSpyExperiment(flagKey); - when(experiment.getAudienceIds()).thenReturn(Arrays.asList(audienceId)); - addSpyExperiment(experiment); - OptimizelyDecision decision = callDecideWithIncludeReasons(flagKey, Collections.singletonMap("country", 25)); - - assertTrue(decision.getReasons().contains( - String.format("User \"%s\" does not meet conditions to be in experiment \"%s\".", userId, experiment.getKey()) - )); - } - - @Test - public void decideReasons_evaluateAttributeValueOutOfRange() { - String flagKey = "feature_1"; - String audienceId = "age_18"; - - Experiment experiment = getSpyExperiment(flagKey); - when(experiment.getAudienceIds()).thenReturn(Arrays.asList(audienceId)); - addSpyExperiment(experiment); - OptimizelyDecision decision = callDecideWithIncludeReasons(flagKey, Collections.singletonMap("age", (float)Math.pow(2, 54))); - - assertTrue(decision.getReasons().contains( - String.format("User \"%s\" does not meet conditions to be in experiment \"%s\".", userId, experiment.getKey()) - )); - } - - @Test - public void decideReasons_userAttributeInvalidType() { - String flagKey = "feature_1"; - String audienceId = "invalid_type"; - - Experiment experiment = getSpyExperiment(flagKey); - when(experiment.getAudienceIds()).thenReturn(Arrays.asList(audienceId)); - addSpyExperiment(experiment); - OptimizelyDecision decision = callDecideWithIncludeReasons(flagKey, Collections.singletonMap("age", 25)); - - assertTrue(decision.getReasons().contains( - String.format("User \"%s\" does not meet conditions to be in experiment \"%s\".", userId, experiment.getKey()) - )); - } - - @Test - public void decideReasons_userAttributeInvalidMatch() { - String flagKey = "feature_1"; - String audienceId = "invalid_match"; - - Experiment experiment = getSpyExperiment(flagKey); - when(experiment.getAudienceIds()).thenReturn(Arrays.asList(audienceId)); - addSpyExperiment(experiment); - OptimizelyDecision decision = callDecideWithIncludeReasons(flagKey, Collections.singletonMap("age", 25)); - - assertTrue(decision.getReasons().contains( - String.format("User \"%s\" does not meet conditions to be in experiment \"%s\".", userId, experiment.getKey()) - )); - } - - @Test - public void decideReasons_userAttributeNilValue() { - String flagKey = "feature_1"; - String audienceId = "nil_value"; - - Experiment experiment = getSpyExperiment(flagKey); - when(experiment.getAudienceIds()).thenReturn(Arrays.asList(audienceId)); - addSpyExperiment(experiment); - OptimizelyDecision decision = callDecideWithIncludeReasons(flagKey, Collections.singletonMap("age", 25)); - - assertTrue(decision.getReasons().contains( - String.format("User \"%s\" does not meet conditions to be in experiment \"%s\".", userId, experiment.getKey()) - )); - } - - @Test - public void decideReasons_missingAttributeValue() { - String flagKey = "feature_1"; - String audienceId = "age_18"; - - Experiment experiment = getSpyExperiment(flagKey); - when(experiment.getAudienceIds()).thenReturn(Arrays.asList(audienceId)); - addSpyExperiment(experiment); - OptimizelyDecision decision = callDecideWithIncludeReasons(flagKey); - - assertTrue(decision.getReasons().contains( - String.format("User \"%s\" does not meet conditions to be in experiment \"%s\".", userId, experiment.getKey()) - )); - } + // reasons (infos with includeReasons) @Test public void decideReasons_experimentNotRunning() { @@ -1084,6 +979,111 @@ public void decideReasons_userNotInExperiment() { )); } + @Test + public void decideReasons_conditionNoMatchingAudience() throws ConfigParseException { + String flagKey = "feature_1"; + String audienceId = "invalid_id"; + + Experiment experiment = getSpyExperiment(flagKey); + when(experiment.getAudienceIds()).thenReturn(Arrays.asList(audienceId)); + addSpyExperiment(experiment); + OptimizelyDecision decision = callDecideWithIncludeReasons(flagKey); + + assertTrue(decision.getReasons().contains( + String.format("Audiences for experiment \"%s\" collectively evaluated to null.", experiment.getKey()) + )); + } + + @Test + public void decideReasons_evaluateAttributeInvalidType() { + String flagKey = "feature_1"; + String audienceId = "13389130056"; + + Experiment experiment = getSpyExperiment(flagKey); + when(experiment.getAudienceIds()).thenReturn(Arrays.asList(audienceId)); + addSpyExperiment(experiment); + OptimizelyDecision decision = callDecideWithIncludeReasons(flagKey, Collections.singletonMap("country", 25)); + + assertTrue(decision.getReasons().contains( + String.format("Audiences for experiment \"%s\" collectively evaluated to null.", experiment.getKey()) + )); + } + + @Test + public void decideReasons_evaluateAttributeValueOutOfRange() { + String flagKey = "feature_1"; + String audienceId = "age_18"; + + Experiment experiment = getSpyExperiment(flagKey); + when(experiment.getAudienceIds()).thenReturn(Arrays.asList(audienceId)); + addSpyExperiment(experiment); + OptimizelyDecision decision = callDecideWithIncludeReasons(flagKey, Collections.singletonMap("age", (float)Math.pow(2, 54))); + + assertTrue(decision.getReasons().contains( + String.format("Audiences for experiment \"%s\" collectively evaluated to null.", experiment.getKey()) + )); + } + + @Test + public void decideReasons_userAttributeInvalidType() { + String flagKey = "feature_1"; + String audienceId = "invalid_type"; + + Experiment experiment = getSpyExperiment(flagKey); + when(experiment.getAudienceIds()).thenReturn(Arrays.asList(audienceId)); + addSpyExperiment(experiment); + OptimizelyDecision decision = callDecideWithIncludeReasons(flagKey, Collections.singletonMap("age", 25)); + + assertTrue(decision.getReasons().contains( + String.format("Audiences for experiment \"%s\" collectively evaluated to null.", experiment.getKey()) + )); + } + + @Test + public void decideReasons_userAttributeInvalidMatch() { + String flagKey = "feature_1"; + String audienceId = "invalid_match"; + + Experiment experiment = getSpyExperiment(flagKey); + when(experiment.getAudienceIds()).thenReturn(Arrays.asList(audienceId)); + addSpyExperiment(experiment); + OptimizelyDecision decision = callDecideWithIncludeReasons(flagKey, Collections.singletonMap("age", 25)); + + assertTrue(decision.getReasons().contains( + String.format("Audiences for experiment \"%s\" collectively evaluated to null.", experiment.getKey()) + )); + } + + @Test + public void decideReasons_userAttributeNilValue() { + String flagKey = "feature_1"; + String audienceId = "nil_value"; + + Experiment experiment = getSpyExperiment(flagKey); + when(experiment.getAudienceIds()).thenReturn(Arrays.asList(audienceId)); + addSpyExperiment(experiment); + OptimizelyDecision decision = callDecideWithIncludeReasons(flagKey, Collections.singletonMap("age", 25)); + + assertTrue(decision.getReasons().contains( + String.format("Audiences for experiment \"%s\" collectively evaluated to null.", experiment.getKey()) + )); + } + + @Test + public void decideReasons_missingAttributeValue() { + String flagKey = "feature_1"; + String audienceId = "age_18"; + + Experiment experiment = getSpyExperiment(flagKey); + when(experiment.getAudienceIds()).thenReturn(Arrays.asList(audienceId)); + addSpyExperiment(experiment); + OptimizelyDecision decision = callDecideWithIncludeReasons(flagKey); + + assertTrue(decision.getReasons().contains( + String.format("Audiences for experiment \"%s\" collectively evaluated to null.", experiment.getKey()) + )); + } + // utils Map createUserProfileMap(String experimentId, String variationId) { From de659dc3136c424e28f5b99c14be767a409d2a6b Mon Sep 17 00:00:00 2001 From: Jae Kim Date: Tue, 5 Jan 2021 10:55:42 -0800 Subject: [PATCH 7/9] fix doc comments for reasons --- .../optimizely/ab/bucketing/DecisionService.java | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 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 f4966aad7..8cb52486a 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 @@ -196,7 +196,7 @@ public DecisionResponse getVariation(@Nonnull Experiment experiment, * @param filteredAttributes A map of filtered attributes. * @param projectConfig The current projectConfig * @param options An array of decision options - * @return A {@link DecisionResponse} including the {@link FeatureDecision} and the decision reasons + * @return A {@link DecisionResponse} including a {@link FeatureDecision} and the decision reasons */ @Nonnull public DecisionResponse getVariationForFeature(@Nonnull FeatureFlag featureFlag, @@ -258,7 +258,7 @@ public DecisionResponse getVariationForFeature(@Nonnull Feature * @param userId User Identifier * @param filteredAttributes A map of filtered attributes. * @param projectConfig The current projectConfig - * @return A {@link DecisionResponse} including the {@link FeatureDecision} and the decision reasons + * @return A {@link DecisionResponse} including a {@link FeatureDecision} and the decision reasons */ @Nonnull DecisionResponse getVariationForFeatureInRollout(@Nonnull FeatureFlag featureFlag, @@ -336,8 +336,8 @@ DecisionResponse getVariationForFeatureInRollout(@Nonnull Featu * * @param experiment {@link Experiment} in which user is to be bucketed. * @param userId User Identifier - * @return null if the user is not whitelisted into any variation - * {@link Variation} the user is bucketed into if the user has a specified whitelisted variation. + * @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 user is not whitelisted into any variation. */ @Nullable DecisionResponse getWhitelistedVariation(@Nonnull Experiment experiment, @@ -368,8 +368,8 @@ DecisionResponse getWhitelistedVariation(@Nonnull Experiment experime * @param experiment {@link Experiment} in which the user was bucketed. * @param userProfile {@link UserProfile} of the user. * @param projectConfig The current projectConfig - * @return null if the {@link UserProfileService} implementation is null or the user was not previously bucketed. - * else return the {@link Variation} the user was previously bucketed into. + * @return A {@link DecisionResponse} including the {@link Variation} that user was previously bucketed into (or null) + * and the decision reasons. The variation can be null if the {@link UserProfileService} implementation is null or the user was not previously bucketed. */ @Nullable DecisionResponse getStoredVariation(@Nonnull Experiment experiment, @@ -546,8 +546,8 @@ public boolean setForcedVariation(@Nonnull Experiment experiment, * * @param experiment The experiment forced. * @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. + * @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. */ @Nullable public DecisionResponse getForcedVariation(@Nonnull Experiment experiment, From 705db4fdce9ed2bdf1ef7d591a4db433f69eb6e9 Mon Sep 17 00:00:00 2001 From: Jae Kim Date: Wed, 13 Jan 2021 10:06:03 -0800 Subject: [PATCH 8/9] change to nonnull for all DecisionResponse --- .../src/main/java/com/optimizely/ab/Optimizely.java | 3 ++- .../java/com/optimizely/ab/bucketing/Bucketer.java | 6 +++--- .../com/optimizely/ab/bucketing/DecisionService.java | 12 ++++++------ .../com/optimizely/ab/internal/ExperimentUtils.java | 8 ++++---- 4 files changed, 15 insertions(+), 14 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 f47d35450..4440608b3 100644 --- a/core-api/src/main/java/com/optimizely/ab/Optimizely.java +++ b/core-api/src/main/java/com/optimizely/ab/Optimizely.java @@ -1,5 +1,5 @@ /**************************************************************************** - * Copyright 2016-2020, Optimizely, Inc. and contributors * + * Copyright 2016-2021, Optimizely, Inc. and contributors * * * * Licensed under the Apache License, Version 2.0 (the "License"); * * you may not use this file except in compliance with the License. * @@ -1303,6 +1303,7 @@ private List getAllOptions(List return copiedOptions; } + @Nonnull private DecisionResponse> getDecisionVariableMap(@Nonnull FeatureFlag flag, @Nonnull Variation variation, @Nonnull Boolean featureEnabled) { diff --git a/core-api/src/main/java/com/optimizely/ab/bucketing/Bucketer.java b/core-api/src/main/java/com/optimizely/ab/bucketing/Bucketer.java index 87d71d7da..b92d2cf15 100644 --- a/core-api/src/main/java/com/optimizely/ab/bucketing/Bucketer.java +++ b/core-api/src/main/java/com/optimizely/ab/bucketing/Bucketer.java @@ -1,6 +1,6 @@ /** * - * Copyright 2016-2017, 2019, Optimizely and contributors + * Copyright 2016-2017, 2019-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. @@ -26,7 +26,6 @@ import org.slf4j.LoggerFactory; import javax.annotation.Nonnull; -import javax.annotation.Nullable; import javax.annotation.concurrent.Immutable; import java.util.List; @@ -89,6 +88,7 @@ private Experiment bucketToExperiment(@Nonnull Group group, return null; } + @Nonnull private DecisionResponse bucketToVariation(@Nonnull Experiment experiment, @Nonnull String bucketingId) { DecisionReasons reasons = DefaultDecisionReasons.newInstance(); @@ -129,7 +129,7 @@ private DecisionResponse bucketToVariation(@Nonnull Experiment experi * @param projectConfig The current projectConfig * @return A {@link DecisionResponse} including the {@link Variation} that user is bucketed into (or null) and the decision reasons */ - @Nullable + @Nonnull public DecisionResponse bucket(@Nonnull Experiment experiment, @Nonnull String bucketingId, @Nonnull ProjectConfig projectConfig) { 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 8cb52486a..8f7eeaca5 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 @@ -1,5 +1,5 @@ /**************************************************************************** - * Copyright 2017-2020, Optimizely, Inc. and contributors * + * Copyright 2017-2021, Optimizely, Inc. and contributors * * * * Licensed under the Apache License, Version 2.0 (the "License"); * * you may not use this file except in compliance with the License. * @@ -89,7 +89,7 @@ public DecisionService(@Nonnull Bucketer bucketer, * @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 */ - @Nullable + @Nonnull public DecisionResponse getVariation(@Nonnull Experiment experiment, @Nonnull String userId, @Nonnull Map filteredAttributes, @@ -180,7 +180,7 @@ public DecisionResponse getVariation(@Nonnull Experiment experiment, return new DecisionResponse(null, reasons); } - @Nullable + @Nonnull public DecisionResponse getVariation(@Nonnull Experiment experiment, @Nonnull String userId, @Nonnull Map filteredAttributes, @@ -339,7 +339,7 @@ DecisionResponse getVariationForFeatureInRollout(@Nonnull Featu * @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 user is not whitelisted into any variation. */ - @Nullable + @Nonnull DecisionResponse getWhitelistedVariation(@Nonnull Experiment experiment, @Nonnull String userId) { DecisionReasons reasons = DefaultDecisionReasons.newInstance(); @@ -371,7 +371,7 @@ DecisionResponse getWhitelistedVariation(@Nonnull Experiment experime * @return A {@link DecisionResponse} including the {@link Variation} that user was previously bucketed into (or null) * and the decision reasons. The variation can be null if the {@link UserProfileService} implementation is null or the user was not previously bucketed. */ - @Nullable + @Nonnull DecisionResponse getStoredVariation(@Nonnull Experiment experiment, @Nonnull UserProfile userProfile, @Nonnull ProjectConfig projectConfig) { @@ -549,7 +549,7 @@ public boolean setForcedVariation(@Nonnull Experiment experiment, * @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. */ - @Nullable + @Nonnull public DecisionResponse getForcedVariation(@Nonnull Experiment experiment, @Nonnull String userId) { DecisionReasons reasons = DefaultDecisionReasons.newInstance(); diff --git a/core-api/src/main/java/com/optimizely/ab/internal/ExperimentUtils.java b/core-api/src/main/java/com/optimizely/ab/internal/ExperimentUtils.java index 8a4d946ac..374043b7f 100644 --- a/core-api/src/main/java/com/optimizely/ab/internal/ExperimentUtils.java +++ b/core-api/src/main/java/com/optimizely/ab/internal/ExperimentUtils.java @@ -1,6 +1,6 @@ /** * - * Copyright 2017-2020, Optimizely and contributors + * Copyright 2017-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. @@ -28,7 +28,6 @@ import org.slf4j.LoggerFactory; import javax.annotation.Nonnull; -import javax.annotation.Nullable; import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -60,6 +59,7 @@ public static boolean isExperimentActive(@Nonnull Experiment experiment) { * @param loggingKey In case of loggingEntityType is experiment it will be experiment key or else it will be rule number. * @return whether the user meets the criteria for the experiment */ + @Nonnull public static DecisionResponse doesUserMeetAudienceConditions(@Nonnull ProjectConfig projectConfig, @Nonnull Experiment experiment, @Nonnull Map attributes, @@ -83,7 +83,7 @@ public static DecisionResponse doesUserMeetAudienceConditions(@Nonnull reasons); } - @Nullable + @Nonnull public static DecisionResponse evaluateAudience(@Nonnull ProjectConfig projectConfig, @Nonnull Experiment experiment, @Nonnull Map attributes, @@ -115,7 +115,7 @@ public static DecisionResponse evaluateAudience(@Nonnull ProjectConfig return new DecisionResponse(result, reasons); } - @Nullable + @Nonnull public static DecisionResponse evaluateAudienceConditions(@Nonnull ProjectConfig projectConfig, @Nonnull Experiment experiment, @Nonnull Map attributes, From 5db7a242218c28a80054a1b08533200d8e1a3039 Mon Sep 17 00:00:00 2001 From: Jae Kim Date: Wed, 13 Jan 2021 10:21:34 -0800 Subject: [PATCH 9/9] fix null decision response --- .../main/java/com/optimizely/ab/internal/ExperimentUtils.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core-api/src/main/java/com/optimizely/ab/internal/ExperimentUtils.java b/core-api/src/main/java/com/optimizely/ab/internal/ExperimentUtils.java index 374043b7f..c1494bbda 100644 --- a/core-api/src/main/java/com/optimizely/ab/internal/ExperimentUtils.java +++ b/core-api/src/main/java/com/optimizely/ab/internal/ExperimentUtils.java @@ -124,7 +124,7 @@ public static DecisionResponse evaluateAudienceConditions(@Nonnull Proj DecisionReasons reasons = DefaultDecisionReasons.newInstance(); Condition conditions = experiment.getAudienceConditions(); - if (conditions == null) return null; + if (conditions == null) return new DecisionResponse(null, reasons); Boolean result = null; try {