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 6eead11c6..d041bfad3 100644 --- a/core-api/src/main/java/com/optimizely/ab/Optimizely.java +++ b/core-api/src/main/java/com/optimizely/ab/Optimizely.java @@ -24,6 +24,7 @@ import com.optimizely.ab.config.DatafileProjectConfig; import com.optimizely.ab.config.EventType; import com.optimizely.ab.config.Experiment; +import com.optimizely.ab.config.ExperimentCore; import com.optimizely.ab.config.FeatureFlag; import com.optimizely.ab.config.FeatureVariable; import com.optimizely.ab.config.FeatureVariableUsageInstance; @@ -319,7 +320,7 @@ private void sendImpression(@Nonnull ProjectConfig projectConfig, * @param ruleType It can either be experiment in case impression event is sent from activate or it's feature-test or rollout */ private boolean sendImpression(@Nonnull ProjectConfig projectConfig, - @Nullable Experiment experiment, + @Nullable ExperimentCore experiment, @Nonnull String userId, @Nonnull Map filteredAttributes, @Nullable Variation variation, @@ -344,13 +345,17 @@ private boolean sendImpression(@Nonnull ProjectConfig projectConfig, if (experiment != null) { logger.info("Activating user \"{}\" in experiment \"{}\".", userId, experiment.getKey()); } + + // Legacy API methods only apply to the Experiment type and not to Holdout. + boolean isExperimentType = experiment instanceof Experiment; + // Kept For backwards compatibility. // This notification is deprecated and the new DecisionNotifications // are sent via their respective method calls. - if (notificationCenter.getNotificationManager(ActivateNotification.class).size() > 0) { + if (notificationCenter.getNotificationManager(ActivateNotification.class).size() > 0 && isExperimentType) { LogEvent impressionEvent = EventFactory.createLogEvent(userEvent); ActivateNotification activateNotification = new ActivateNotification( - experiment, userId, filteredAttributes, variation, impressionEvent); + (Experiment)experiment, userId, filteredAttributes, variation, impressionEvent); notificationCenter.send(activateNotification); } return true; 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 b92d2cf15..35fa21c71 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 @@ -16,25 +16,32 @@ */ package com.optimizely.ab.bucketing; +import java.util.List; + +import javax.annotation.Nonnull; +import javax.annotation.concurrent.Immutable; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import com.optimizely.ab.annotations.VisibleForTesting; import com.optimizely.ab.bucketing.internal.MurmurHash3; -import com.optimizely.ab.config.*; +import com.optimizely.ab.config.Experiment; +import com.optimizely.ab.config.ExperimentCore; +import com.optimizely.ab.config.Group; +import com.optimizely.ab.config.ProjectConfig; +import com.optimizely.ab.config.TrafficAllocation; +import com.optimizely.ab.config.Variation; 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; - -import javax.annotation.Nonnull; -import javax.annotation.concurrent.Immutable; -import java.util.List; /** * Default Optimizely bucketing algorithm that evenly distributes users using the Murmur3 hash of some provided * identifier. *

* The user identifier must be provided in the first data argument passed to - * {@link #bucket(Experiment, String, ProjectConfig)} and must be non-null and non-empty. + * {@link #bucket(ExperimentCore, String, ProjectConfig)} and must be non-null and non-empty. * * @see MurmurHash */ @@ -89,7 +96,7 @@ private Experiment bucketToExperiment(@Nonnull Group group, } @Nonnull - private DecisionResponse bucketToVariation(@Nonnull Experiment experiment, + private DecisionResponse bucketToVariation(@Nonnull ExperimentCore experiment, @Nonnull String bucketingId) { DecisionReasons reasons = DefaultDecisionReasons.newInstance(); @@ -130,7 +137,7 @@ private DecisionResponse bucketToVariation(@Nonnull Experiment experi * @return A {@link DecisionResponse} including the {@link Variation} that user is bucketed into (or null) and the decision reasons */ @Nonnull - public DecisionResponse bucket(@Nonnull Experiment experiment, + public DecisionResponse bucket(@Nonnull ExperimentCore experiment, @Nonnull String bucketingId, @Nonnull ProjectConfig projectConfig) { DecisionReasons reasons = DefaultDecisionReasons.newInstance(); 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 ff48ffb99..b7536aab5 100644 --- a/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java +++ b/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java @@ -15,27 +15,39 @@ ***************************************************************************/ package com.optimizely.ab.bucketing; +import java.util.AbstractMap; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; + +import javax.annotation.Nonnull; +import javax.annotation.Nullable; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import com.optimizely.ab.OptimizelyDecisionContext; import com.optimizely.ab.OptimizelyForcedDecision; import com.optimizely.ab.OptimizelyRuntimeException; import com.optimizely.ab.OptimizelyUserContext; -import com.optimizely.ab.config.*; +import com.optimizely.ab.config.Experiment; +import com.optimizely.ab.config.FeatureFlag; +import com.optimizely.ab.config.Holdout; +import com.optimizely.ab.config.ProjectConfig; +import com.optimizely.ab.config.Rollout; +import com.optimizely.ab.config.Variation; import com.optimizely.ab.error.ErrorHandler; import com.optimizely.ab.internal.ControlAttribute; import com.optimizely.ab.internal.ExperimentUtils; +import static com.optimizely.ab.internal.LoggingConstants.LoggingEntityType.EXPERIMENT; +import static com.optimizely.ab.internal.LoggingConstants.LoggingEntityType.RULE; 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; -import org.slf4j.LoggerFactory; -import javax.annotation.Nonnull; -import javax.annotation.Nullable; -import java.util.*; -import java.util.concurrent.ConcurrentHashMap; - -import static com.optimizely.ab.internal.LoggingConstants.LoggingEntityType.EXPERIMENT; -import static com.optimizely.ab.internal.LoggingConstants.LoggingEntityType.RULE; /** * Optimizely's decision service that determines which variation of an experiment the user will be allocated to. @@ -240,10 +252,22 @@ public List> getVariationsForFeatureList(@Non List> decisions = new ArrayList<>(); - for (FeatureFlag featureFlag: featureFlags) { + flagLoop: for (FeatureFlag featureFlag: featureFlags) { DecisionReasons reasons = DefaultDecisionReasons.newInstance(); reasons.merge(upsReasons); + List holdouts = projectConfig.getHoldoutForFlag(featureFlag.getId()); + if (!holdouts.isEmpty()) { + for (Holdout holdout : holdouts) { + DecisionResponse holdoutDecision = getVariationForHoldout(holdout, user, projectConfig); + reasons.merge(holdoutDecision.getReasons()); + if (holdoutDecision.getResult() != null) { + decisions.add(new DecisionResponse<>(new FeatureDecision(holdout, holdoutDecision.getResult(), FeatureDecision.DecisionSource.HOLDOUT), reasons)); + continue flagLoop; + } + } + } + DecisionResponse decisionVariationResponse = getVariationFromExperiment(projectConfig, featureFlag, user, options, userProfileTracker); reasons.merge(decisionVariationResponse.getReasons()); @@ -419,6 +443,54 @@ DecisionResponse getWhitelistedVariation(@Nonnull Experiment experime return new DecisionResponse(null, reasons); } + /** + * Determines the variation for a holdout rule. + * + * @param holdout The holdout rule to evaluate. + * @param user The user context. + * @param projectConfig The current project configuration. + * @return A {@link DecisionResponse} with the variation (if any) and reasons. + */ + @Nonnull + DecisionResponse getVariationForHoldout(@Nonnull Holdout holdout, + @Nonnull OptimizelyUserContext user, + @Nonnull ProjectConfig projectConfig) { + DecisionReasons reasons = DefaultDecisionReasons.newInstance(); + + if (!holdout.isActive()) { + String message = reasons.addInfo("Holdout (%s) is not running.", holdout.getKey()); + logger.info(message); + return new DecisionResponse<>(null, reasons); + } + + DecisionResponse decisionMeetAudience = ExperimentUtils.doesUserMeetAudienceConditions(projectConfig, holdout, user, EXPERIMENT, holdout.getKey()); + reasons.merge(decisionMeetAudience.getReasons()); + + if (decisionMeetAudience.getResult()) { + // User meets audience conditions for holdout + String audienceMatchMessage = reasons.addInfo("User (%s) meets audience conditions for holdout (%s).", user.getUserId(), holdout.getKey()); + logger.info(audienceMatchMessage); + + String bucketingId = getBucketingId(user.getUserId(), user.getAttributes()); + DecisionResponse decisionVariation = bucketer.bucket(holdout, bucketingId, projectConfig); + reasons.merge(decisionVariation.getReasons()); + Variation variation = decisionVariation.getResult(); + + if (variation != null) { + String message = reasons.addInfo("User (%s) is in variation (%s) of holdout (%s).", user.getUserId(), variation.getKey(), holdout.getKey()); + logger.info(message); + } else { + String message = reasons.addInfo("User (%s) is in no holdout variation.", user.getUserId()); + logger.info(message); + } + return new DecisionResponse<>(variation, reasons); + } + + String message = reasons.addInfo("User (%s) does not meet conditions for holdout (%s).", user.getUserId(), holdout.getKey()); + logger.info(message); + return new DecisionResponse<>(null, reasons); + } + // TODO: Logically, it makes sense to move this method to UserProfileTracker. But some tests are also calling this // method, requiring us to refactor those tests as well. We'll look to refactor this later. diff --git a/core-api/src/main/java/com/optimizely/ab/bucketing/FeatureDecision.java b/core-api/src/main/java/com/optimizely/ab/bucketing/FeatureDecision.java index b0f0a11ed..e53172e0a 100644 --- a/core-api/src/main/java/com/optimizely/ab/bucketing/FeatureDecision.java +++ b/core-api/src/main/java/com/optimizely/ab/bucketing/FeatureDecision.java @@ -15,17 +15,17 @@ ***************************************************************************/ package com.optimizely.ab.bucketing; -import com.optimizely.ab.config.Experiment; -import com.optimizely.ab.config.Variation; - import javax.annotation.Nullable; +import com.optimizely.ab.config.ExperimentCore; +import com.optimizely.ab.config.Variation; + public class FeatureDecision { /** - * The {@link Experiment} the Feature is associated with. + * The {@link ExperimentCore} the Feature is associated with. */ @Nullable - public Experiment experiment; + public ExperimentCore experiment; /** * The {@link Variation} the user was bucketed into. @@ -41,7 +41,8 @@ public class FeatureDecision { public enum DecisionSource { FEATURE_TEST("feature-test"), - ROLLOUT("rollout"); + ROLLOUT("rollout"), + HOLDOUT("holdout"); private final String key; @@ -58,11 +59,11 @@ public String toString() { /** * Initialize a FeatureDecision object. * - * @param experiment The {@link Experiment} the Feature is associated with. + * @param experiment The {@link ExperimentCore} the Feature is associated with. * @param variation The {@link Variation} the user was bucketed into. * @param decisionSource The source of the variation. */ - public FeatureDecision(@Nullable Experiment experiment, @Nullable Variation variation, + public FeatureDecision(@Nullable ExperimentCore experiment, @Nullable Variation variation, @Nullable DecisionSource decisionSource) { this.experiment = experiment; this.variation = variation; diff --git a/core-api/src/main/java/com/optimizely/ab/event/internal/UserEventFactory.java b/core-api/src/main/java/com/optimizely/ab/event/internal/UserEventFactory.java index 9c44f455b..c8687f7a6 100644 --- a/core-api/src/main/java/com/optimizely/ab/event/internal/UserEventFactory.java +++ b/core-api/src/main/java/com/optimizely/ab/event/internal/UserEventFactory.java @@ -16,23 +16,26 @@ */ package com.optimizely.ab.event.internal; +import java.util.Map; + +import javax.annotation.Nonnull; +import javax.annotation.Nullable; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import com.optimizely.ab.bucketing.FeatureDecision; -import com.optimizely.ab.config.Experiment; +import com.optimizely.ab.config.ExperimentCore; import com.optimizely.ab.config.ProjectConfig; import com.optimizely.ab.config.Variation; import com.optimizely.ab.event.internal.payload.DecisionMetadata; import com.optimizely.ab.internal.EventTagUtils; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -import javax.annotation.Nonnull; -import javax.annotation.Nullable; -import java.util.Map; public class UserEventFactory { private static final Logger logger = LoggerFactory.getLogger(UserEventFactory.class); public static ImpressionEvent createImpressionEvent(@Nonnull ProjectConfig projectConfig, - @Nullable Experiment activatedExperiment, + @Nullable ExperimentCore activatedExperiment, @Nullable Variation variation, @Nonnull String userId, @Nonnull Map attributes, 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 8da421885..2abb131c6 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 @@ -16,8 +16,17 @@ */ package com.optimizely.ab.internal; +import java.util.ArrayList; +import java.util.List; + +import javax.annotation.Nonnull; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import com.optimizely.ab.OptimizelyUserContext; import com.optimizely.ab.config.Experiment; +import com.optimizely.ab.config.ExperimentCore; import com.optimizely.ab.config.ProjectConfig; import com.optimizely.ab.config.audience.AudienceIdCondition; import com.optimizely.ab.config.audience.Condition; @@ -25,13 +34,6 @@ 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; - -import javax.annotation.Nonnull; -import java.util.ArrayList; -import java.util.List; -import java.util.Map; public final class ExperimentUtils { @@ -62,7 +64,7 @@ public static boolean isExperimentActive(@Nonnull Experiment experiment) { */ @Nonnull public static DecisionResponse doesUserMeetAudienceConditions(@Nonnull ProjectConfig projectConfig, - @Nonnull Experiment experiment, + @Nonnull ExperimentCore experiment, @Nonnull OptimizelyUserContext user, @Nonnull String loggingEntityType, @Nonnull String loggingKey) { @@ -86,7 +88,7 @@ public static DecisionResponse doesUserMeetAudienceConditions(@Nonnull @Nonnull public static DecisionResponse evaluateAudience(@Nonnull ProjectConfig projectConfig, - @Nonnull Experiment experiment, + @Nonnull ExperimentCore experiment, @Nonnull OptimizelyUserContext user, @Nonnull String loggingEntityType, @Nonnull String loggingKey) { @@ -118,7 +120,7 @@ public static DecisionResponse evaluateAudience(@Nonnull ProjectConfig @Nonnull public static DecisionResponse evaluateAudienceConditions(@Nonnull ProjectConfig projectConfig, - @Nonnull Experiment experiment, + @Nonnull ExperimentCore experiment, @Nonnull OptimizelyUserContext user, @Nonnull String loggingEntityType, @Nonnull String loggingKey) { diff --git a/core-api/src/main/java/com/optimizely/ab/notification/ActivateNotification.java b/core-api/src/main/java/com/optimizely/ab/notification/ActivateNotification.java index dc70079de..b94db2857 100644 --- a/core-api/src/main/java/com/optimizely/ab/notification/ActivateNotification.java +++ b/core-api/src/main/java/com/optimizely/ab/notification/ActivateNotification.java @@ -16,13 +16,13 @@ */ package com.optimizely.ab.notification; +import java.util.Map; + import com.optimizely.ab.annotations.VisibleForTesting; import com.optimizely.ab.config.Experiment; import com.optimizely.ab.config.Variation; import com.optimizely.ab.event.LogEvent; -import java.util.Map; - /** * ActivateNotification supplies notification for AB activatation. * diff --git a/core-api/src/main/java/com/optimizely/ab/notification/ActivateNotificationListener.java b/core-api/src/main/java/com/optimizely/ab/notification/ActivateNotificationListener.java index 4ca602c77..982431268 100644 --- a/core-api/src/main/java/com/optimizely/ab/notification/ActivateNotificationListener.java +++ b/core-api/src/main/java/com/optimizely/ab/notification/ActivateNotificationListener.java @@ -17,13 +17,14 @@ package com.optimizely.ab.notification; +import java.util.Map; + +import javax.annotation.Nonnull; + import com.optimizely.ab.config.Experiment; import com.optimizely.ab.config.Variation; import com.optimizely.ab.event.LogEvent; -import javax.annotation.Nonnull; -import java.util.Map; - /** * ActivateNotificationListener handles the activate event notification. * diff --git a/core-api/src/main/java/com/optimizely/ab/notification/ActivateNotificationListenerInterface.java b/core-api/src/main/java/com/optimizely/ab/notification/ActivateNotificationListenerInterface.java index c0a1e3a73..c5ae2901f 100644 --- a/core-api/src/main/java/com/optimizely/ab/notification/ActivateNotificationListenerInterface.java +++ b/core-api/src/main/java/com/optimizely/ab/notification/ActivateNotificationListenerInterface.java @@ -16,13 +16,14 @@ */ package com.optimizely.ab.notification; +import java.util.Map; + +import javax.annotation.Nonnull; + import com.optimizely.ab.config.Experiment; import com.optimizely.ab.config.Variation; import com.optimizely.ab.event.LogEvent; -import javax.annotation.Nonnull; -import java.util.Map; - /** * ActivateNotificationListenerInterface provides and interface for activate event notification. * 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 bb2d36192..a0b555d66 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java @@ -2084,4 +2084,187 @@ OptimizelyDecision callDecideWithIncludeReasons(String flagKey) { return callDecideWithIncludeReasons(flagKey, Collections.emptyMap()); } + private Optimizely createOptimizelyWithHoldouts() throws Exception { + String holdoutDatafile = com.google.common.io.Resources.toString( + com.google.common.io.Resources.getResource("config/holdouts-project-config.json"), + com.google.common.base.Charsets.UTF_8 + ); + return new Optimizely.Builder().withDatafile(holdoutDatafile).withEventProcessor(new ForwardingEventProcessor(eventHandler, null)).build(); + } + + @Test + public void decisionNotification_with_holdout() throws Exception { + // Use holdouts datafile + Optimizely optWithHoldout = createOptimizelyWithHoldouts(); + String flagKey = "boolean_feature"; + String userId = "user123"; + String ruleKey = "basic_holdout"; // holdout rule key + String variationKey = "ho_off_key"; // holdout (off) variation key + String experimentId = "10075323428"; // holdout experiment id in holdouts-project-config.json + String variationId = "$opt_dummy_variation_id";// dummy variation id used for holdout impressions + String expectedReason = "User (" + userId + ") is in variation (" + variationKey + ") of holdout (" + ruleKey + ")."; + + Map attrs = new HashMap<>(); + attrs.put("$opt_bucketing_id", "ppid160000"); // deterministic bucketing into basic_holdout + attrs.put("nationality", "English"); // non-reserved attribute should appear in impression & notification + + OptimizelyUserContext user = optWithHoldout.createUserContext(userId, attrs); + + // Register notification handler similar to decisionNotification test + isListenerCalled = false; + optWithHoldout.addDecisionNotificationHandler(decisionNotification -> { + Assert.assertEquals(NotificationCenter.DecisionNotificationType.FLAG.toString(), decisionNotification.getType()); + Assert.assertEquals(userId, decisionNotification.getUserId()); + + Assert.assertEquals(attrs, decisionNotification.getAttributes()); + + Map info = decisionNotification.getDecisionInfo(); + Assert.assertEquals(flagKey, info.get(FLAG_KEY)); + Assert.assertEquals(variationKey, info.get(VARIATION_KEY)); + Assert.assertEquals(false, info.get(ENABLED)); + Assert.assertEquals(ruleKey, info.get(RULE_KEY)); + Assert.assertEquals(experimentId, info.get(EXPERIMENT_ID)); + Assert.assertEquals(variationId, info.get(VARIATION_ID)); + // Variables should be empty because feature is disabled by holdout + Assert.assertTrue(((Map) info.get(VARIABLES)).isEmpty()); + // Event should be dispatched (no DISABLE_DECISION_EVENT option) + Assert.assertEquals(true, info.get(DECISION_EVENT_DISPATCHED)); + + @SuppressWarnings("unchecked") + List reasons = (List) info.get(REASONS); + Assert.assertTrue("Expected holdout reason present", reasons.contains(expectedReason)); + isListenerCalled = true; + }); + + // Execute decision with INCLUDE_REASONS so holdout reason is present + OptimizelyDecision decision = user.decide(flagKey, Collections.singletonList(OptimizelyDecideOption.INCLUDE_REASONS)); + assertTrue(isListenerCalled); + + // Sanity checks on returned decision + assertEquals(variationKey, decision.getVariationKey()); + assertFalse(decision.getEnabled()); + assertTrue(decision.getReasons().contains(expectedReason)); + + // Impression expectation (nationality only) + DecisionMetadata metadata = new DecisionMetadata.Builder() + .setFlagKey(flagKey) + .setRuleKey(ruleKey) + .setRuleType("holdout") + .setVariationKey(variationKey) + .setEnabled(false) + .build(); + eventHandler.expectImpression(experimentId, variationId, userId, Collections.singletonMap("nationality", "English"), metadata); + + // Log expectation (reuse existing pattern) + logbackVerifier.expectMessage(Level.INFO, expectedReason); + } + @Test + public void decide_for_keys_with_holdout() throws Exception { + Optimizely optWithHoldout = createOptimizelyWithHoldouts(); + String userId = "user123"; + Map attrs = new HashMap<>(); + attrs.put("$opt_bucketing_id", "ppid160000"); + OptimizelyUserContext user = optWithHoldout.createUserContext(userId, attrs); + + List flagKeys = Arrays.asList( + "boolean_feature", // previously validated basic_holdout membership + "double_single_variable_feature", // also subject to global/basic holdout + "integer_single_variable_feature" // also subject to global/basic holdout + ); + + Map decisions = user.decideForKeys(flagKeys, Collections.singletonList(OptimizelyDecideOption.INCLUDE_REASONS)); + assertEquals(3, decisions.size()); + + String holdoutExperimentId = "10075323428"; // basic_holdout id + String variationId = "$opt_dummy_variation_id"; + String variationKey = "ho_off_key"; + String expectedReason = "User (" + userId + ") is in variation (" + variationKey + ") of holdout (basic_holdout)."; + + for (String flagKey : flagKeys) { + OptimizelyDecision d = decisions.get(flagKey); + assertNotNull(d); + assertEquals(flagKey, d.getFlagKey()); + assertEquals(variationKey, d.getVariationKey()); + assertFalse(d.getEnabled()); + assertTrue("Expected holdout reason for flag " + flagKey, d.getReasons().contains(expectedReason)); + DecisionMetadata metadata = new DecisionMetadata.Builder() + .setFlagKey(flagKey) + .setRuleKey("basic_holdout") + .setRuleType("holdout") + .setVariationKey(variationKey) + .setEnabled(false) + .build(); + // attributes map expected empty (reserved $opt_ attribute filtered out) + eventHandler.expectImpression(holdoutExperimentId, variationId, userId, Collections.emptyMap(), metadata); + } + + // At least one log message confirming holdout membership + logbackVerifier.expectMessage(Level.INFO, expectedReason); + } + + @Test + public void decide_all_with_holdout() throws Exception { + + Optimizely optWithHoldout = createOptimizelyWithHoldouts(); + String userId = "user123"; + Map attrs = new HashMap<>(); + // ppid120000 buckets user into holdout_included_flags + attrs.put("$opt_bucketing_id", "ppid120000"); + OptimizelyUserContext user = optWithHoldout.createUserContext(userId, attrs); + + // All flag keys present in holdouts-project-config.json + List allFlagKeys = Arrays.asList( + "boolean_feature", + "double_single_variable_feature", + "integer_single_variable_feature", + "boolean_single_variable_feature", + "string_single_variable_feature", + "multi_variate_feature", + "multi_variate_future_feature", + "mutex_group_feature" + ); + + // Flags INCLUDED in holdout_included_flags (only these should be holdout decisions) + List includedInHoldout = Arrays.asList( + "boolean_feature", + "double_single_variable_feature", + "integer_single_variable_feature" + ); + + Map decisions = user.decideAll(Arrays.asList( + OptimizelyDecideOption.INCLUDE_REASONS, + OptimizelyDecideOption.DISABLE_DECISION_EVENT + )); + assertEquals(allFlagKeys.size(), decisions.size()); + + String holdoutExperimentId = "1007543323427"; // holdout_included_flags id + String variationId = "$opt_dummy_variation_id"; + String variationKey = "ho_off_key"; + String expectedReason = "User (" + userId + ") is in variation (" + variationKey + ") of holdout (holdout_included_flags)."; + + int holdoutCount = 0; + for (String flagKey : allFlagKeys) { + OptimizelyDecision d = decisions.get(flagKey); + assertNotNull("Missing decision for flag " + flagKey, d); + if (includedInHoldout.contains(flagKey)) { + // Should be holdout decision + assertEquals(variationKey, d.getVariationKey()); + assertFalse(d.getEnabled()); + assertTrue("Expected holdout reason for flag " + flagKey, d.getReasons().contains(expectedReason)); + DecisionMetadata metadata = new DecisionMetadata.Builder() + .setFlagKey(flagKey) + .setRuleKey("holdout_included_flags") + .setRuleType("holdout") + .setVariationKey(variationKey) + .setEnabled(false) + .build(); + holdoutCount++; + } else { + // Should NOT be a holdout decision + assertFalse("Non-included flag should not have holdout reason: " + flagKey, d.getReasons().contains(expectedReason)); + } + } + assertEquals("Expected exactly the included flags to be in holdout", includedInHoldout.size(), holdoutCount); + logbackVerifier.expectMessage(Level.INFO, expectedReason); + } } 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 d818826d4..220a62efa 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 @@ -15,35 +15,86 @@ ***************************************************************************/ package com.optimizely.ab.bucketing; -import ch.qos.logback.classic.Level; +import java.util.AbstractMap; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.anyMapOf; +import static org.mockito.Matchers.anyObject; +import static org.mockito.Matchers.anyString; +import static org.mockito.Matchers.eq; +import org.mockito.Mock; +import static org.mockito.Mockito.atMost; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; +import org.mockito.junit.MockitoJUnit; +import org.mockito.junit.MockitoRule; + import com.optimizely.ab.Optimizely; import com.optimizely.ab.OptimizelyDecisionContext; import com.optimizely.ab.OptimizelyForcedDecision; import com.optimizely.ab.OptimizelyUserContext; -import com.optimizely.ab.config.*; +import com.optimizely.ab.config.DatafileProjectConfigTestUtils; +import static com.optimizely.ab.config.DatafileProjectConfigTestUtils.noAudienceProjectConfigV3; +import static com.optimizely.ab.config.DatafileProjectConfigTestUtils.validProjectConfigV3; +import static com.optimizely.ab.config.DatafileProjectConfigTestUtils.validProjectConfigV4; +import com.optimizely.ab.config.Experiment; +import com.optimizely.ab.config.FeatureFlag; +import com.optimizely.ab.config.ProjectConfig; +import com.optimizely.ab.config.Rollout; +import com.optimizely.ab.config.TrafficAllocation; +import com.optimizely.ab.config.ValidProjectConfigV4; +import static com.optimizely.ab.config.ValidProjectConfigV4.ATTRIBUTE_HOUSE_KEY; +import static com.optimizely.ab.config.ValidProjectConfigV4.ATTRIBUTE_NATIONALITY_KEY; +import static com.optimizely.ab.config.ValidProjectConfigV4.AUDIENCE_ENGLISH_CITIZENS_VALUE; +import static com.optimizely.ab.config.ValidProjectConfigV4.AUDIENCE_GRYFFINDOR_VALUE; +import static com.optimizely.ab.config.ValidProjectConfigV4.FEATURE_FLAG_BOOLEAN_FEATURE; +import static com.optimizely.ab.config.ValidProjectConfigV4.FEATURE_FLAG_MULTI_VARIATE_FEATURE; +import static com.optimizely.ab.config.ValidProjectConfigV4.FEATURE_FLAG_MULTI_VARIATE_FUTURE_FEATURE; +import static com.optimizely.ab.config.ValidProjectConfigV4.FEATURE_FLAG_MUTEX_GROUP_FEATURE; +import static com.optimizely.ab.config.ValidProjectConfigV4.FEATURE_FLAG_SINGLE_VARIABLE_BOOLEAN; +import static com.optimizely.ab.config.ValidProjectConfigV4.FEATURE_FLAG_SINGLE_VARIABLE_INTEGER; +import static com.optimizely.ab.config.ValidProjectConfigV4.FEATURE_MULTI_VARIATE_FEATURE_KEY; +import static com.optimizely.ab.config.ValidProjectConfigV4.HOLDOUT_BASIC_HOLDOUT; +import static com.optimizely.ab.config.ValidProjectConfigV4.HOLDOUT_EXCLUDED_FLAGS_HOLDOUT; +import static com.optimizely.ab.config.ValidProjectConfigV4.HOLDOUT_INCLUDED_FLAGS_HOLDOUT; +import static com.optimizely.ab.config.ValidProjectConfigV4.HOLDOUT_TYPEDAUDIENCE_HOLDOUT; +import static com.optimizely.ab.config.ValidProjectConfigV4.ROLLOUT_2; +import static com.optimizely.ab.config.ValidProjectConfigV4.ROLLOUT_3_EVERYONE_ELSE_RULE; +import static com.optimizely.ab.config.ValidProjectConfigV4.ROLLOUT_3_EVERYONE_ELSE_RULE_ENABLED_VARIATION; +import static com.optimizely.ab.config.ValidProjectConfigV4.VARIATION_HOLDOUT_VARIATION_OFF; +import static com.optimizely.ab.config.ValidProjectConfigV4.generateValidProjectConfigV4_holdout; +import com.optimizely.ab.config.Variation; import com.optimizely.ab.error.ErrorHandler; import com.optimizely.ab.internal.ControlAttribute; import com.optimizely.ab.internal.LogbackVerifier; import com.optimizely.ab.optimizelydecision.DecisionReasons; import com.optimizely.ab.optimizelydecision.DecisionResponse; -import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; -import org.junit.Before; -import org.junit.Rule; -import org.junit.Test; -import org.mockito.Mock; -import org.mockito.junit.MockitoJUnit; -import org.mockito.junit.MockitoRule; -import java.util.*; - -import static com.optimizely.ab.config.DatafileProjectConfigTestUtils.*; -import static com.optimizely.ab.config.ValidProjectConfigV4.*; +import ch.qos.logback.classic.Level; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import static junit.framework.TestCase.assertEquals; -import static org.hamcrest.CoreMatchers.is; -import static org.hamcrest.MatcherAssert.assertThat; -import static org.junit.Assert.*; -import static org.mockito.Matchers.*; -import static org.mockito.Mockito.*; public class DecisionServiceTest { @@ -1228,4 +1279,106 @@ public void setForcedVariationMultipleUsers() { assertNull(decisionService.getForcedVariation(experiment2, "testUser2").getResult()); } + @Test + public void getVariationForFeatureReturnHoldoutDecisionForGlobalHoldout() { + ProjectConfig holdoutProjectConfig = generateValidProjectConfigV4_holdout(); + + Bucketer mockBucketer = new Bucketer(); + + DecisionService decisionService = new DecisionService(mockBucketer, mockErrorHandler, null); + + Map attributes = new HashMap<>(); + attributes.put("$opt_bucketing_id", "ppid160000"); + FeatureDecision featureDecision = decisionService.getVariationForFeature( + FEATURE_FLAG_BOOLEAN_FEATURE, + optimizely.createUserContext("user123", attributes), + holdoutProjectConfig + ).getResult(); + + assertEquals(HOLDOUT_BASIC_HOLDOUT, featureDecision.experiment); + assertEquals(VARIATION_HOLDOUT_VARIATION_OFF, featureDecision.variation); + assertEquals(FeatureDecision.DecisionSource.HOLDOUT, featureDecision.decisionSource); + + logbackVerifier.expectMessage(Level.INFO, "User (user123) is in variation (ho_off_key) of holdout (basic_holdout)."); + } + + @Test + public void includedFlagsHoldoutOnlyAppliestoSpecificFlags() { + ProjectConfig holdoutProjectConfig = generateValidProjectConfigV4_holdout(); + + Bucketer mockBucketer = new Bucketer(); + + DecisionService decisionService = new DecisionService(mockBucketer, mockErrorHandler, null); + + Map attributes = new HashMap<>(); + attributes.put("$opt_bucketing_id", "ppid120000"); + FeatureDecision featureDecision = decisionService.getVariationForFeature( + FEATURE_FLAG_BOOLEAN_FEATURE, + optimizely.createUserContext("user123", attributes), + holdoutProjectConfig + ).getResult(); + + assertEquals(HOLDOUT_INCLUDED_FLAGS_HOLDOUT, featureDecision.experiment); + assertEquals(VARIATION_HOLDOUT_VARIATION_OFF, featureDecision.variation); + assertEquals(FeatureDecision.DecisionSource.HOLDOUT, featureDecision.decisionSource); + + logbackVerifier.expectMessage(Level.INFO, "User (user123) is in variation (ho_off_key) of holdout (holdout_included_flags)."); + } + + @Test + public void excludedFlagsHoldoutAppliesToAllExceptSpecified() { + ProjectConfig holdoutProjectConfig = generateValidProjectConfigV4_holdout(); + + Bucketer mockBucketer = new Bucketer(); + + DecisionService decisionService = new DecisionService(mockBucketer, mockErrorHandler, null); + + Map attributes = new HashMap<>(); + attributes.put("$opt_bucketing_id", "ppid300002"); + FeatureDecision excludedDecision = decisionService.getVariationForFeature( + FEATURE_FLAG_SINGLE_VARIABLE_BOOLEAN, // excluded from ho (holdout_excluded_flags) + optimizely.createUserContext("user123", attributes), + holdoutProjectConfig + ).getResult(); + + assertNotEquals(FeatureDecision.DecisionSource.HOLDOUT, excludedDecision.decisionSource); + + FeatureDecision featureDecision = decisionService.getVariationForFeature( + FEATURE_FLAG_SINGLE_VARIABLE_INTEGER, // excluded from ho (holdout_excluded_flags) + optimizely.createUserContext("user123", attributes), + holdoutProjectConfig + ).getResult(); + + assertEquals(HOLDOUT_EXCLUDED_FLAGS_HOLDOUT, featureDecision.experiment); + assertEquals(VARIATION_HOLDOUT_VARIATION_OFF, featureDecision.variation); + assertEquals(FeatureDecision.DecisionSource.HOLDOUT, featureDecision.decisionSource); + + logbackVerifier.expectMessage(Level.INFO, "User (user123) is in variation (ho_off_key) of holdout (holdout_excluded_flags)."); + } + + @Test + public void userMeetsHoldoutAudienceConditions() { + ProjectConfig holdoutProjectConfig = generateValidProjectConfigV4_holdout(); + + Bucketer mockBucketer = new Bucketer(); + + DecisionService decisionService = new DecisionService(mockBucketer, mockErrorHandler, null); + + Map attributes = new HashMap<>(); + attributes.put("$opt_bucketing_id", "ppid543400"); + attributes.put("booleanKey", true); + attributes.put("integerKey", 1); + + FeatureDecision featureDecision = decisionService.getVariationForFeature( + FEATURE_FLAG_BOOLEAN_FEATURE, + optimizely.createUserContext("user123", attributes), + holdoutProjectConfig + ).getResult(); + + assertEquals(HOLDOUT_TYPEDAUDIENCE_HOLDOUT, featureDecision.experiment); + assertEquals(VARIATION_HOLDOUT_VARIATION_OFF, featureDecision.variation); + assertEquals(FeatureDecision.DecisionSource.HOLDOUT, featureDecision.decisionSource); + + logbackVerifier.expectMessage(Level.INFO, "User (user123) is in variation (ho_off_key) of holdout (typed_audience_holdout)."); + } } diff --git a/core-api/src/test/java/com/optimizely/ab/config/ValidProjectConfigV4.java b/core-api/src/test/java/com/optimizely/ab/config/ValidProjectConfigV4.java index 0d8f5d3c0..0291c0ce1 100644 --- a/core-api/src/test/java/com/optimizely/ab/config/ValidProjectConfigV4.java +++ b/core-api/src/test/java/com/optimizely/ab/config/ValidProjectConfigV4.java @@ -235,7 +235,7 @@ public class ValidProjectConfigV4 { // features private static final String FEATURE_BOOLEAN_FEATURE_ID = "4195505407"; private static final String FEATURE_BOOLEAN_FEATURE_KEY = "boolean_feature"; - private static final FeatureFlag FEATURE_FLAG_BOOLEAN_FEATURE = new FeatureFlag( + public static final FeatureFlag FEATURE_FLAG_BOOLEAN_FEATURE = new FeatureFlag( FEATURE_BOOLEAN_FEATURE_ID, FEATURE_BOOLEAN_FEATURE_KEY, "", @@ -294,7 +294,7 @@ public class ValidProjectConfigV4 { FeatureVariable.BOOLEAN_TYPE, null ); - private static final FeatureFlag FEATURE_FLAG_SINGLE_VARIABLE_BOOLEAN = new FeatureFlag( + public static final FeatureFlag FEATURE_FLAG_SINGLE_VARIABLE_BOOLEAN = new FeatureFlag( FEATURE_SINGLE_VARIABLE_BOOLEAN_ID, FEATURE_SINGLE_VARIABLE_BOOLEAN_KEY, "", @@ -490,7 +490,7 @@ public class ValidProjectConfigV4 { VARIATION_BASIC_EXPERIMENT_VARIATION_A_KEY, Collections.emptyList() ); - private static final Variation VARIATION_HOLDOUT_VARIATION_OFF = new Variation( + public static final Variation VARIATION_HOLDOUT_VARIATION_OFF = new Variation( "$opt_dummy_variation_id", "ho_off_key", false @@ -536,7 +536,7 @@ public class ValidProjectConfigV4 { ) ) ); - private static final Holdout HOLDOUT_BASIC_HOLDOUT = new Holdout( + public static final Holdout HOLDOUT_BASIC_HOLDOUT = new Holdout( "10075323428", "basic_holdout", Holdout.HoldoutStatus.RUNNING.toString(), @@ -547,7 +547,7 @@ public class ValidProjectConfigV4 { ), DatafileProjectConfigTestUtils.createListOfObjects( new TrafficAllocation( - "327323", + "$opt_dummy_variation_id", 500 ) ), @@ -566,7 +566,7 @@ public class ValidProjectConfigV4 { ), DatafileProjectConfigTestUtils.createListOfObjects( new TrafficAllocation( - "327323", + "$opt_dummy_variation_id", 0 ) ), @@ -574,7 +574,7 @@ public class ValidProjectConfigV4 { null ); - private static final Holdout HOLDOUT_INCLUDED_FLAGS_HOLDOUT = new Holdout( + public static final Holdout HOLDOUT_INCLUDED_FLAGS_HOLDOUT = new Holdout( "1007543323427", "holdout_included_flags", Holdout.HoldoutStatus.RUNNING.toString(), @@ -585,7 +585,7 @@ public class ValidProjectConfigV4 { ), DatafileProjectConfigTestUtils.createListOfObjects( new TrafficAllocation( - "327323", + "$opt_dummy_variation_id", 2000 ) ), @@ -597,7 +597,7 @@ public class ValidProjectConfigV4 { null ); - private static final Holdout HOLDOUT_EXCLUDED_FLAGS_HOLDOUT = new Holdout( + public static final Holdout HOLDOUT_EXCLUDED_FLAGS_HOLDOUT = new Holdout( "100753234214", "holdout_excluded_flags", Holdout.HoldoutStatus.RUNNING.toString(), @@ -608,7 +608,7 @@ public class ValidProjectConfigV4 { ), DatafileProjectConfigTestUtils.createListOfObjects( new TrafficAllocation( - "327323", + "$opt_dummy_variation_id", 1500 ) ), @@ -620,7 +620,7 @@ public class ValidProjectConfigV4 { ) ); - private static final Holdout HOLDOUT_TYPEDAUDIENCE_HOLDOUT = new Holdout( + public static final Holdout HOLDOUT_TYPEDAUDIENCE_HOLDOUT = new Holdout( "10075323429", "typed_audience_holdout", Holdout.HoldoutStatus.RUNNING.toString(), @@ -636,7 +636,7 @@ public class ValidProjectConfigV4 { ), DatafileProjectConfigTestUtils.createListOfObjects( new TrafficAllocation( - "327323", + "$opt_dummy_variation_id", 1000 ) ), diff --git a/core-api/src/test/java/com/optimizely/ab/notification/ActivateNotificationListenerTest.java b/core-api/src/test/java/com/optimizely/ab/notification/ActivateNotificationListenerTest.java index f7fcda09b..844e51700 100644 --- a/core-api/src/test/java/com/optimizely/ab/notification/ActivateNotificationListenerTest.java +++ b/core-api/src/test/java/com/optimizely/ab/notification/ActivateNotificationListenerTest.java @@ -16,19 +16,20 @@ */ package com.optimizely.ab.notification; -import com.optimizely.ab.config.Experiment; -import com.optimizely.ab.config.Variation; -import com.optimizely.ab.event.LogEvent; -import org.junit.Before; -import org.junit.Test; - -import javax.annotation.Nonnull; import java.util.Collections; import java.util.Map; -import static org.junit.Assert.*; +import javax.annotation.Nonnull; + +import static org.junit.Assert.assertEquals; +import org.junit.Before; +import org.junit.Test; import static org.mockito.Mockito.mock; +import com.optimizely.ab.config.Experiment; +import com.optimizely.ab.config.Variation; +import com.optimizely.ab.event.LogEvent; + public class ActivateNotificationListenerTest { private static final Experiment EXPERIMENT = mock(Experiment.class); diff --git a/core-api/src/test/java/com/optimizely/ab/notification/NotificationCenterTest.java b/core-api/src/test/java/com/optimizely/ab/notification/NotificationCenterTest.java index c9e911029..d3c55cccb 100644 --- a/core-api/src/test/java/com/optimizely/ab/notification/NotificationCenterTest.java +++ b/core-api/src/test/java/com/optimizely/ab/notification/NotificationCenterTest.java @@ -16,29 +16,31 @@ */ package com.optimizely.ab.notification; -import ch.qos.logback.classic.Level; -import com.optimizely.ab.OptimizelyRuntimeException; -import com.optimizely.ab.config.Experiment; -import com.optimizely.ab.config.Variation; -import com.optimizely.ab.event.LogEvent; -import com.optimizely.ab.internal.LogbackVerifier; -import org.junit.After; -import org.junit.Before; -import org.junit.Rule; -import org.junit.Test; - -import javax.annotation.Nonnull; import java.util.Collections; import java.util.List; import java.util.Map; import java.util.concurrent.atomic.AtomicBoolean; -import static junit.framework.TestCase.assertNotSame; -import static junit.framework.TestCase.assertTrue; +import javax.annotation.Nonnull; + +import org.junit.After; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; import static org.mockito.Mockito.mock; +import com.optimizely.ab.OptimizelyRuntimeException; +import com.optimizely.ab.config.Experiment; +import com.optimizely.ab.config.Variation; +import com.optimizely.ab.event.LogEvent; +import com.optimizely.ab.internal.LogbackVerifier; + +import ch.qos.logback.classic.Level; +import static junit.framework.TestCase.assertNotSame; +import static junit.framework.TestCase.assertTrue; + public class NotificationCenterTest { private NotificationCenter notificationCenter; private ActivateNotificationListener activateNotification; diff --git a/core-api/src/test/resources/config/holdouts-project-config.json b/core-api/src/test/resources/config/holdouts-project-config.json index 5a83fad17..585ae8572 100644 --- a/core-api/src/test/resources/config/holdouts-project-config.json +++ b/core-api/src/test/resources/config/holdouts-project-config.json @@ -483,7 +483,7 @@ "trafficAllocation": [ { "endOfRange": 0, - "entityId": "327323" + "entityId": "$opt_dummy_variation_id" } ], "variations": [ @@ -502,7 +502,7 @@ "trafficAllocation": [ { "endOfRange": 2000, - "entityId": "327323" + "entityId": "$opt_dummy_variation_id" } ], "variations": [ @@ -526,7 +526,7 @@ "trafficAllocation": [ { "endOfRange": 500, - "entityId": "327323" + "entityId": "$opt_dummy_variation_id" } ], "variations": [ @@ -544,7 +544,7 @@ "trafficAllocation": [ { "endOfRange": 1000, - "entityId": "327323" + "entityId": "$opt_dummy_variation_id" } ], "variations": [ @@ -565,7 +565,7 @@ "trafficAllocation": [ { "endOfRange": 1500, - "entityId": "327323" + "entityId": "$opt_dummy_variation_id" } ], "variations": [