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 fa1b824c7..090fa209a 100644 --- a/core-api/src/main/java/com/optimizely/ab/Optimizely.java +++ b/core-api/src/main/java/com/optimizely/ab/Optimizely.java @@ -53,6 +53,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; /** * Top-level container class for Optimizely functionality. @@ -615,6 +616,42 @@ Variation getVariation(@Nonnull String experimentKey, return decisionService.getVariation(experiment,userId,filteredAttributes); } + /** + * Force a user into a variation for a given experiment. + * The forced variation value does not persist across application launches. + * If the experiment key is not in the project file, this call fails and returns false. + * If the variationKey is not in the experiment, this call fails. + * @param experimentKey The key for the experiment. + * @param userId The user ID to be used for bucketing. + * @param variationKey The variation key to force the user into. If the variation key is null + * then the forcedVariation for that experiment is removed. + * + * @return boolean A boolean value that indicates if the set completed successfully. + */ + public boolean setForcedVariation(@Nonnull String experimentKey, + @Nonnull String userId, + @Nullable String variationKey) { + + + return projectConfig.setForcedVariation(experimentKey, userId, variationKey); + } + + /** + * Gets the forced variation for a given user and experiment. + * This method just calls into the {@link com.optimizely.ab.config.ProjectConfig#getForcedVariation(String, String)} + * method of the same signature. + * + * @param experimentKey The key for the experiment. + * @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. + */ + public @Nullable Variation getForcedVariation(@Nonnull String experimentKey, + @Nonnull String userId) { + return projectConfig.getForcedVariation(experimentKey, userId); + } + /** * @return the current {@link ProjectConfig} instance. */ 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 8d6137321..50b9241b8 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 @@ -82,8 +82,14 @@ public DecisionService(@Nonnull Bucketer bucketer, return null; } + // look for forced bucketing first. + Variation variation = projectConfig.getForcedVariation(experiment.getKey(), userId); + // check for whitelisting - Variation variation = getWhitelistedVariation(experiment, userId); + if (variation == null) { + variation = getWhitelistedVariation(experiment, userId); + } + if (variation != null) { return variation; } diff --git a/core-api/src/main/java/com/optimizely/ab/config/ProjectConfig.java b/core-api/src/main/java/com/optimizely/ab/config/ProjectConfig.java index 77e69ad2e..cbbc35fd9 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/ProjectConfig.java +++ b/core-api/src/main/java/com/optimizely/ab/config/ProjectConfig.java @@ -19,7 +19,10 @@ import com.fasterxml.jackson.annotation.JsonIgnoreProperties; import com.optimizely.ab.config.audience.Audience; import com.optimizely.ab.config.audience.Condition; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import javax.annotation.Nonnull; import javax.annotation.Nullable; import javax.annotation.concurrent.Immutable; import java.util.ArrayList; @@ -27,6 +30,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; /** * Represents the Optimizely Project configuration. @@ -54,6 +58,10 @@ public String toString() { } } + // logger + private static final Logger logger = LoggerFactory.getLogger(ProjectConfig.class); + + // ProjectConfig properties private final String accountId; private final String projectId; private final String revision; @@ -85,6 +93,14 @@ public String toString() { private final Map> variationToLiveVariableUsageInstanceMapping; private final Map variationIdToExperimentMapping; + /** + * Forced variations supersede any other mappings. They are transient and are not persistent or part of + * the actual datafile. This contains all the forced variations + * set by the user by calling {@link ProjectConfig#setForcedVariation(String, String, String)} (it is not the same as the + * whitelisting forcedVariations data structure in the Experiments class). + */ + private transient ConcurrentHashMap> forcedVariationMapping = new ConcurrentHashMap>(); + // v2 constructor public ProjectConfig(String accountId, String projectId, String version, String revision, List groups, List experiments, List attributes, List eventType, @@ -318,6 +334,136 @@ public Map getFeatureKeyMapping() { return featureKeyMapping; } + public ConcurrentHashMap> getForcedVariationMapping() { return forcedVariationMapping; } + + /** + * Force a user into a variation for a given experiment. + * The forced variation value does not persist across application launches. + * If the experiment key is not in the project file, this call fails and returns false. + * + * @param experimentKey The key for the experiment. + * @param userId The user ID to be used for bucketing. + * @param variationKey The variation key to force the user into. If the variation key is null + * then the forcedVariation for that experiment is removed. + * + * @return boolean A boolean value that indicates if the set completed successfully. + */ + public boolean setForcedVariation(@Nonnull String experimentKey, + @Nonnull String userId, + @Nullable String variationKey) { + + // if the experiment is not a valid experiment key, don't set it. + Experiment experiment = getExperimentKeyMapping().get(experimentKey); + if (experiment == null){ + logger.error("Experiment {} does not exist in ProjectConfig for project {}", experimentKey, projectId); + return false; + } + + Variation variation = null; + + // keep in mind that you can pass in a variationKey that is null if you want to + // remove the variation. + if (variationKey != null) { + variation = experiment.getVariationKeyToVariationMap().get(variationKey); + // if the variation is not part of the experiment, return false. + if (variation == null) { + logger.error("Variation {} does not exist for experiment {}", variationKey, experimentKey); + return false; + } + } + + // if the user id is invalid, return false. + if (userId == null || userId.trim().isEmpty()) { + logger.error("User ID is invalid"); + return false; + } + + ConcurrentHashMap experimentToVariation; + if (!forcedVariationMapping.containsKey(userId)) { + forcedVariationMapping.putIfAbsent(userId, new ConcurrentHashMap()); + } + experimentToVariation = forcedVariationMapping.get(userId); + + boolean retVal = true; + // if it is null remove the variation if it exists. + if (variationKey == null) { + String removedVariationId = experimentToVariation.remove(experiment.getId()); + if (removedVariationId != null) { + Variation removedVariation = experiment.getVariationIdToVariationMap().get(removedVariationId); + if (removedVariation != null) { + logger.debug("Variation mapped to experiment \"%s\" has been removed for user \"%s\"", experiment.getKey(), userId); + } + else { + logger.debug("Removed forced variation that did not exist in experiment"); + } + } + else { + logger.debug("No variation for experiment {}", experimentKey); + retVal = false; + } + } + else { + String previous = experimentToVariation.put(experiment.getId(), variation.getId()); + logger.debug("Set variation \"%s\" for experiment \"%s\" and user \"%s\" in the forced variation map.", + variation.getKey(), experiment.getKey(), userId); + if (previous != null) { + Variation previousVariation = experiment.getVariationIdToVariationMap().get(previous); + if (previousVariation != null) { + logger.debug("forced variation {} replaced forced variation {} in forced variation map.", + variation.getKey(), previousVariation.getKey()); + } + } + } + + return retVal; + } + + /** + * Gets the forced variation for a given user and experiment. + * + * @param experimentKey The key for the experiment. + * @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. + */ + public @Nullable Variation getForcedVariation(@Nonnull String experimentKey, + @Nonnull String userId) { + + // if the user id is invalid, return false. + if (userId == null || userId.trim().isEmpty()) { + logger.error("User ID is invalid"); + return null; + } + + if (experimentKey == null || experimentKey.isEmpty()) { + logger.error("experiment key is invalid"); + return null; + } + + Map experimentToVariation = getForcedVariationMapping().get(userId); + if (experimentToVariation != null) { + Experiment experiment = getExperimentKeyMapping().get(experimentKey); + if (experiment == null) { + logger.debug("No experiment \"%s\" mapped to user \"%s\" in the forced variation map ", experimentKey, userId); + return null; + } + String variationId = experimentToVariation.get(experiment.getId()); + if (variationId != null) { + Variation variation = experiment.getVariationIdToVariationMap().get(variationId); + if (variation != null) { + logger.debug("Variation \"%s\" is mapped to experiment \"%s\" and user \"%s\" in the forced variation map", + variation.getKey(), experimentKey, userId); + return variation; + } + } + else { + logger.debug("No variation for experiment \"%s\" mapped to user \"%s\" in the forced variation map ", experimentKey, userId); + } + } + return null; + } + @Override public String toString() { return "ProjectConfig{" + @@ -344,6 +490,7 @@ public String toString() { ", groupIdMapping=" + groupIdMapping + ", liveVariableIdToExperimentsMapping=" + liveVariableIdToExperimentsMapping + ", variationToLiveVariableUsageInstanceMapping=" + variationToLiveVariableUsageInstanceMapping + + ", forcedVariationMapping=" + forcedVariationMapping + ", variationIdToExperimentMapping=" + variationIdToExperimentMapping + '}'; } 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 5765fb036..82fbbf8de 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java @@ -55,10 +55,11 @@ import java.io.IOException; import java.util.Arrays; import java.util.Collection; -import java.util.Collections; +import java.util.Map; import java.util.HashMap; import java.util.List; -import java.util.Map; +import java.util.Collections; +import java.util.ArrayList; import static com.optimizely.ab.config.ProjectConfigTestUtils.noAudienceProjectConfigJsonV2; import static com.optimizely.ab.config.ProjectConfigTestUtils.noAudienceProjectConfigJsonV3; @@ -193,7 +194,7 @@ public OptimizelyTest(int datafileVersion, public void activateEndToEnd() throws Exception { Experiment activatedExperiment; Map testUserAttributes = new HashMap(); - if(datafileVersion == 4) { + if(datafileVersion >= 4) { activatedExperiment = validProjectConfig.getExperimentKeyMapping().get(EXPERIMENT_MULTIVARIATE_EXPERIMENT_KEY); testUserAttributes.put(ATTRIBUTE_HOUSE_KEY, AUDIENCE_GRYFFINDOR_VALUE); } @@ -212,6 +213,7 @@ public void activateEndToEnd() throws Exception { .build(); Map testParams = new HashMap(); + testParams.put("test", "params"); LogEvent logEventToDispatch = new LogEvent(RequestMethod.GET, "test_url", testParams, ""); when(mockEventBuilder.createImpressionEvent(validProjectConfig, activatedExperiment, bucketedVariation, "userId", @@ -296,6 +298,161 @@ public void activateWhenExperimentIsNotInProject() throws Exception { optimizely.activate(unknownExperiment, "userId"); } + /** + * Verify that the {@link Optimizely#activate(String, String, Map)} call + * uses forced variation to force the user into the second variation. The mock bucket returns + * the first variation. Then remove the forced variation and confirm that the forced variation is null. + */ + @Test + public void activateWithExperimentKeyForced() throws Exception { + Experiment activatedExperiment = validProjectConfig.getExperiments().get(0); + Variation forcedVariation = activatedExperiment.getVariations().get(1); + Variation bucketedVariation = activatedExperiment.getVariations().get(0); + EventBuilder mockEventBuilder = mock(EventBuilder.class); + + Optimizely optimizely = Optimizely.builder(validDatafile, mockEventHandler) + .withBucketing(mockBucketer) + .withEventBuilder(mockEventBuilder) + .withConfig(validProjectConfig) + .withErrorHandler(mockErrorHandler) + .build(); + + optimizely.setForcedVariation(activatedExperiment.getKey(), "userId", forcedVariation.getKey() ); + + Map testUserAttributes = new HashMap(); + if (datafileVersion >= 4) { + testUserAttributes.put(ATTRIBUTE_HOUSE_KEY, AUDIENCE_GRYFFINDOR_VALUE); + } + else { + testUserAttributes.put("browser_type", "chrome"); + } + + Map testParams = new HashMap(); + testParams.put("test", "params"); + + LogEvent logEventToDispatch = new LogEvent(RequestMethod.GET, "test_url", testParams, ""); + when(mockEventBuilder.createImpressionEvent(eq(validProjectConfig), eq(activatedExperiment), eq(forcedVariation), + eq("userId"), eq(testUserAttributes))) + .thenReturn(logEventToDispatch); + + when(mockBucketer.bucket(activatedExperiment, "userId")) + .thenReturn(bucketedVariation); + + // activate the experiment + Variation actualVariation = optimizely.activate(activatedExperiment.getKey(), "userId", testUserAttributes); + + assertThat(actualVariation, is(forcedVariation)); + + verify(mockEventHandler).dispatchEvent(logEventToDispatch); + + optimizely.setForcedVariation(activatedExperiment.getKey(), "userId", null ); + + assertEquals(optimizely.getForcedVariation(activatedExperiment.getKey(), "userId"), null); + + } + + /** + * Verify that the {@link Optimizely#getVariation(String, String, Map)} call + * uses forced variation to force the user into the second variation. The mock bucket returns + * the first variation. Then remove the forced variation and confirm that the forced variation is null. + */ + @Test + public void getVariationWithExperimentKeyForced() throws Exception { + Experiment activatedExperiment = validProjectConfig.getExperiments().get(0); + Variation forcedVariation = activatedExperiment.getVariations().get(1); + Variation bucketedVariation = activatedExperiment.getVariations().get(0); + EventBuilder mockEventBuilder = mock(EventBuilder.class); + + Optimizely optimizely = Optimizely.builder(validDatafile, mockEventHandler) + .withBucketing(mockBucketer) + .withEventBuilder(mockEventBuilder) + .withConfig(validProjectConfig) + .withErrorHandler(mockErrorHandler) + .build(); + + optimizely.setForcedVariation(activatedExperiment.getKey(), "userId", forcedVariation.getKey() ); + + Map testUserAttributes = new HashMap(); + if (datafileVersion >= 4) { + testUserAttributes.put(ATTRIBUTE_HOUSE_KEY, AUDIENCE_GRYFFINDOR_VALUE); + } + else { + testUserAttributes.put("browser_type", "chrome"); + } + + Map testParams = new HashMap(); + testParams.put("test", "params"); + + LogEvent logEventToDispatch = new LogEvent(RequestMethod.GET, "test_url", testParams, ""); + when(mockEventBuilder.createImpressionEvent(eq(validProjectConfig), eq(activatedExperiment), eq(forcedVariation), + eq("userId"), eq(testUserAttributes))) + .thenReturn(logEventToDispatch); + + when(mockBucketer.bucket(activatedExperiment, "userId")) + .thenReturn(bucketedVariation); + + // activate the experiment + Variation actualVariation = optimizely.getVariation(activatedExperiment.getKey(), "userId", testUserAttributes); + + assertThat(actualVariation, is(forcedVariation)); + + optimizely.setForcedVariation(activatedExperiment.getKey(), "userId", null ); + + assertEquals(optimizely.getForcedVariation(activatedExperiment.getKey(), "userId"), null); + + actualVariation = optimizely.getVariation(activatedExperiment.getKey(), "userId", testUserAttributes); + + assertThat(actualVariation, is(bucketedVariation)); + } + + /** + * Verify that the {@link Optimizely#activate(String, String, Map)} call + * uses forced variation to force the user into the second variation. The mock bucket returns + * the first variation. Then remove the forced variation and confirm that the forced variation is null. + */ + @Test + public void isFeatureEnabledWithExperimentKeyForced() throws Exception { + assumeTrue(datafileVersion >= Integer.parseInt(ProjectConfig.Version.V4.toString())); + + Experiment activatedExperiment = validProjectConfig.getExperimentKeyMapping().get(EXPERIMENT_MULTIVARIATE_EXPERIMENT_KEY); + Variation forcedVariation = activatedExperiment.getVariations().get(1); + EventBuilder mockEventBuilder = mock(EventBuilder.class); + + Optimizely optimizely = Optimizely.builder(validDatafile, mockEventHandler) + .withBucketing(mockBucketer) + .withEventBuilder(mockEventBuilder) + .withConfig(validProjectConfig) + .withErrorHandler(mockErrorHandler) + .build(); + + optimizely.setForcedVariation(activatedExperiment.getKey(), "userId", forcedVariation.getKey() ); + + Map testUserAttributes = new HashMap(); + if (datafileVersion < 4) { + testUserAttributes.put("browser_type", "chrome"); + } + + Map testParams = new HashMap(); + testParams.put("test", "params"); + + LogEvent logEventToDispatch = new LogEvent(RequestMethod.GET, "test_url", testParams, ""); + when(mockEventBuilder.createImpressionEvent(eq(validProjectConfig), eq(activatedExperiment), eq(forcedVariation), + eq("userId"), eq(testUserAttributes))) + .thenReturn(logEventToDispatch); + + // activate the experiment + assertTrue(optimizely.isFeatureEnabled(FEATURE_FLAG_MULTI_VARIATE_FEATURE.getKey(), "userId")); + + verify(mockEventHandler).dispatchEvent(logEventToDispatch); + + assertTrue(optimizely.setForcedVariation(activatedExperiment.getKey(), "userId", null )); + + assertNull(optimizely.getForcedVariation(activatedExperiment.getKey(), "userId")); + + assertFalse(optimizely.isFeatureEnabled(FEATURE_FLAG_MULTI_VARIATE_FEATURE.getKey(), "userId")); + + } + /** * Verify that the {@link Optimizely#activate(String, String, Map)} call * correctly builds an endpoint url and request params @@ -315,7 +472,7 @@ public void activateWithExperimentKey() throws Exception { .build(); Map testUserAttributes = new HashMap(); - if (datafileVersion == 4) { + if (datafileVersion >= 4) { testUserAttributes.put(ATTRIBUTE_HOUSE_KEY, AUDIENCE_GRYFFINDOR_VALUE); } else { @@ -458,7 +615,7 @@ public void activateWithUnknownAttribute() throws Exception { .build(); Map testUserAttributes = new HashMap(); - if (datafileVersion == 4) { + if (datafileVersion >= 4) { testUserAttributes.put(ATTRIBUTE_HOUSE_KEY, AUDIENCE_GRYFFINDOR_VALUE); } else { @@ -913,6 +1070,80 @@ public void activateLaunchedExperimentDoesNotDispatchEvent() throws Exception { //======== track tests ========// + /** + * Verify that the {@link Optimizely#track(String, String)} call correctly builds a V2 event and passes it + * through {@link EventHandler#dispatchEvent(LogEvent)}. + */ + @Test + public void trackEventEndToEndForced() throws Exception { + EventType eventType; + String datafile; + ProjectConfig config; + if (datafileVersion >= 4) { + config = spy(validProjectConfig); + eventType = validProjectConfig.getEventNameMapping().get(EVENT_BASIC_EVENT_KEY); + datafile = validDatafile; + } + else { + config = spy(noAudienceProjectConfig); + eventType = noAudienceProjectConfig.getEventTypes().get(0); + datafile = noAudienceDatafile; + } + List allExperiments = new ArrayList(); + allExperiments.add(config.getExperiments().get(0)); + EventBuilder eventBuilderV2 = new EventBuilderV2(); + DecisionService spyDecisionService = spy(new DecisionService(mockBucketer, + mockErrorHandler, + config, + null)); + + Optimizely optimizely = Optimizely.builder(datafile, mockEventHandler) + .withDecisionService(spyDecisionService) + .withEventBuilder(eventBuilderV2) + .withConfig(noAudienceProjectConfig) + .withErrorHandler(mockErrorHandler) + .build(); + + // Bucket to null for all experiments. However, only a subset of the experiments will actually + // call the bucket function. + for (Experiment experiment : allExperiments) { + when(mockBucketer.bucket(experiment, "userId")) + .thenReturn(null); + } + // Force to the first variation for all experiments. However, only a subset of the experiments will actually + // call get forced. + for (Experiment experiment : allExperiments) { + optimizely.projectConfig.setForcedVariation(experiment.getKey(), + "userId", experiment.getVariations().get(0).getKey()); + } + + // call track + optimizely.track(eventType.getKey(), "userId"); + + // verify that the bucketing algorithm was called only on experiments corresponding to the specified goal. + List experimentsForEvent = config.getExperimentsForEventKey(eventType.getKey()); + for (Experiment experiment : allExperiments) { + if (experiment.isRunning() && experimentsForEvent.contains(experiment)) { + verify(spyDecisionService).getVariation(experiment, "userId", + Collections.emptyMap()); + verify(config).getForcedVariation(experiment.getKey(), "userId"); + } else { + verify(spyDecisionService, never()).getVariation(experiment, "userId", + Collections.emptyMap()); + } + } + + // verify that dispatchEvent was called + verify(mockEventHandler).dispatchEvent(any(LogEvent.class)); + + for (Experiment experiment : allExperiments) { + assertEquals(optimizely.projectConfig.getForcedVariation(experiment.getKey(), "userId"), experiment.getVariations().get(0)); + optimizely.projectConfig.setForcedVariation(experiment.getKey(), "userId", null); + assertNull(optimizely.projectConfig.getForcedVariation(experiment.getKey(), "userId")); + } + + } + /** * Verify that the {@link Optimizely#track(String, String)} call correctly builds a V2 event and passes it * through {@link EventHandler#dispatchEvent(LogEvent)}. @@ -922,13 +1153,13 @@ public void trackEventEndToEnd() throws Exception { EventType eventType; String datafile; ProjectConfig config; - if (datafileVersion == 4) { - config = validProjectConfig; + if (datafileVersion >= 4) { + config = spy(validProjectConfig); eventType = validProjectConfig.getEventNameMapping().get(EVENT_BASIC_EVENT_KEY); datafile = validDatafile; } else { - config = noAudienceProjectConfig; + config = spy(noAudienceProjectConfig); eventType = noAudienceProjectConfig.getEventTypes().get(0); datafile = noAudienceDatafile; } @@ -937,7 +1168,7 @@ public void trackEventEndToEnd() throws Exception { EventBuilder eventBuilderV2 = new EventBuilderV2(); DecisionService spyDecisionService = spy(new DecisionService(mockBucketer, mockErrorHandler, - validProjectConfig, + config, null)); Optimizely optimizely = Optimizely.builder(datafile, mockEventHandler) @@ -963,6 +1194,7 @@ public void trackEventEndToEnd() throws Exception { if (experiment.isRunning() && experimentsForEvent.contains(experiment)) { verify(spyDecisionService).getVariation(experiment, "userId", Collections.emptyMap()); + verify(config).getForcedVariation(experiment.getKey(), "userId"); } else { verify(spyDecisionService, never()).getVariation(experiment, "userId", Collections.emptyMap()); @@ -1021,7 +1253,7 @@ public void trackEventWithUnknownEventKeyAndRaiseExceptionErrorHandler() throws public void trackEventWithAttributes() throws Exception { Attribute attribute = validProjectConfig.getAttributes().get(0); EventType eventType; - if (datafileVersion == 4) { + if (datafileVersion >= 4) { eventType = validProjectConfig.getEventNameMapping().get(EVENT_BASIC_EVENT_KEY); } else { @@ -1094,7 +1326,7 @@ public void trackEventWithAttributes() throws Exception { justification="testing nullness contract violation") public void trackEventWithNullAttributes() throws Exception { EventType eventType; - if (datafileVersion == 4) { + if (datafileVersion >= 4) { eventType = validProjectConfig.getEventNameMapping().get(EVENT_BASIC_EVENT_KEY); } else { @@ -1166,7 +1398,7 @@ public void trackEventWithNullAttributes() throws Exception { @Test public void trackEventWithNullAttributeValues() throws Exception { EventType eventType; - if (datafileVersion == 4) { + if (datafileVersion >= 4) { eventType = validProjectConfig.getEventNameMapping().get(EVENT_BASIC_EVENT_KEY); } else { @@ -1238,7 +1470,7 @@ public void trackEventWithNullAttributeValues() throws Exception { @SuppressWarnings("unchecked") public void trackEventWithUnknownAttribute() throws Exception { EventType eventType; - if (datafileVersion == 4) { + if (datafileVersion >= 4) { eventType = validProjectConfig.getEventNameMapping().get(EVENT_BASIC_EVENT_KEY); } else { @@ -1309,7 +1541,7 @@ public void trackEventWithUnknownAttribute() throws Exception { @Test public void trackEventWithEventTags() throws Exception { EventType eventType; - if (datafileVersion == 4) { + if (datafileVersion >= 4) { eventType = validProjectConfig.getEventNameMapping().get(EVENT_BASIC_EVENT_KEY); } else { @@ -1392,7 +1624,7 @@ public void trackEventWithEventTags() throws Exception { justification="testing nullness contract violation") public void trackEventWithNullEventTags() throws Exception { EventType eventType; - if (datafileVersion == 4) { + if (datafileVersion >= 4) { eventType = validProjectConfig.getEventNameMapping().get(EVENT_BASIC_EVENT_KEY); } else { @@ -1456,7 +1688,7 @@ public void trackEventWithNullEventTags() throws Exception { @Test public void trackEventWithNoValidExperiments() throws Exception { EventType eventType; - if (datafileVersion == 4) { + if (datafileVersion >= 4) { eventType = validProjectConfig.getEventNameMapping().get(EVENT_BASIC_EVENT_KEY); } else { @@ -1506,7 +1738,7 @@ public void trackDispatchEventThrowsException() throws Exception { @Test public void trackDoesNotSendEventWhenExperimentsAreLaunchedOnly() throws Exception { EventType eventType; - if (datafileVersion == 4) { + if (datafileVersion >= 4) { eventType = validProjectConfig.getEventNameMapping().get(EVENT_LAUNCHED_EXPERIMENT_ONLY_KEY); } else { @@ -1563,7 +1795,7 @@ public void trackDoesNotSendEventWhenExperimentsAreLaunchedOnly() throws Excepti public void trackDispatchesWhenEventHasLaunchedAndRunningExperiments() throws Exception { EventBuilder mockEventBuilder = mock(EventBuilder.class); EventType eventType; - if (datafileVersion == 4) { + if (datafileVersion >= 4) { eventType = validProjectConfig.getEventNameMapping().get(EVENT_BASIC_EVENT_KEY); } else { @@ -1763,7 +1995,7 @@ public void getVariationWithAudiences() throws Exception { @Test public void getVariationWithAudiencesNoAttributes() throws Exception { Experiment experiment; - if (datafileVersion == 4) { + if (datafileVersion >= 4) { experiment = validProjectConfig.getExperimentKeyMapping().get(EXPERIMENT_MULTIVARIATE_EXPERIMENT_KEY); } else { @@ -1853,7 +2085,7 @@ public void getVariationForGroupExperimentWithMatchingAttributes() throws Except Variation variation = experiment.getVariations().get(0); Map attributes = new HashMap(); - if (datafileVersion == 4) { + if (datafileVersion >= 4) { attributes.put(ATTRIBUTE_HOUSE_KEY, AUDIENCE_GRYFFINDOR_VALUE); } else { @@ -1897,7 +2129,7 @@ public void getVariationForGroupExperimentWithNonMatchingAttributes() throws Exc @Test public void getVariationExperimentStatusPrecedesForcedVariation() throws Exception { Experiment experiment; - if (datafileVersion == 4) { + if (datafileVersion >= 4) { experiment = validProjectConfig.getExperimentKeyMapping().get(EXPERIMENT_PAUSED_EXPERIMENT_KEY); } else { @@ -1924,7 +2156,7 @@ public void getVariationExperimentStatusPrecedesForcedVariation() throws Excepti public void addNotificationListener() throws Exception { Experiment activatedExperiment; EventType eventType; - if (datafileVersion == 4) { + if (datafileVersion >= 4) { activatedExperiment = validProjectConfig.getExperimentKeyMapping().get(EXPERIMENT_BASIC_EXPERIMENT_KEY); eventType = validProjectConfig.getEventNameMapping().get(EVENT_BASIC_EVENT_KEY); } @@ -2074,7 +2306,7 @@ public void removeNotificationListener() throws Exception { public void clearNotificationListeners() throws Exception { Experiment activatedExperiment; Map attributes = new HashMap(); - if (datafileVersion == 4) { + if (datafileVersion >= 4) { activatedExperiment = validProjectConfig.getExperimentKeyMapping().get(EXPERIMENT_MULTIVARIATE_EXPERIMENT_KEY); attributes.put(ATTRIBUTE_HOUSE_KEY, AUDIENCE_GRYFFINDOR_VALUE); } 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 d5f731877..eef46cc5a 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 @@ -39,14 +39,16 @@ import java.util.Map; import static com.optimizely.ab.config.ProjectConfigTestUtils.noAudienceProjectConfigV3; +import static com.optimizely.ab.config.ProjectConfigTestUtils.validConfigJsonV2; import static com.optimizely.ab.config.ProjectConfigTestUtils.validProjectConfigV3; import static com.optimizely.ab.config.ProjectConfigTestUtils.validProjectConfigV4; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.assertFalse; import static org.mockito.Matchers.any; -import static org.mockito.Matchers.anyMap; import static org.mockito.Matchers.anyMapOf; import static org.mockito.Matchers.anyString; import static org.mockito.Matchers.eq; @@ -58,6 +60,7 @@ import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import static org.mockito.Mockito.times; public class DecisionServiceTest { @@ -90,11 +93,11 @@ public static void setUp() throws Exception { //========= getVariation tests =========/ /** - * Verify that {@link DecisionService#getVariation(Experiment, String, Map)} gives precedence to forced variation bucketing - * over audience evaluation. + * Verify that {@link DecisionService#getVariation(Experiment, String, Map)} + * gives precedence to forced variation bucketing over audience evaluation. */ @Test - public void getVariationForcedVariationPrecedesAudienceEval() throws Exception { + public void getVariationWhitelistedPrecedesAudienceEval() throws Exception { Bucketer bucketer = spy(new Bucketer(validProjectConfig)); DecisionService decisionService = spy(new DecisionService(bucketer, mockErrorHandler, validProjectConfig, null)); Experiment experiment = validProjectConfig.getExperiments().get(0); @@ -112,6 +115,100 @@ public void getVariationForcedVariationPrecedesAudienceEval() throws Exception { verify(decisionService, never()).getStoredVariation(eq(experiment), any(UserProfile.class)); } + /** + * Verify that {@link DecisionService#getVariation(Experiment, String, Map)} + * gives precedence to forced variation bucketing over whitelisting. + */ + @Test + public void getForcedVariationBeforeWhitelisting() throws Exception { + Bucketer bucketer = new Bucketer(validProjectConfig); + DecisionService decisionService = spy(new DecisionService(bucketer, mockErrorHandler, validProjectConfig, null)); + Experiment experiment = validProjectConfig.getExperiments().get(0); + Variation whitelistVariation = experiment.getVariations().get(0); + Variation expectedVariation = experiment.getVariations().get(1); + + // user excluded without audiences and whitelisting + assertNull(decisionService.getVariation(experiment, genericUserId, Collections.emptyMap())); + + logbackVerifier.expectMessage(Level.INFO, "User \"" + genericUserId + "\" does not meet conditions to be in experiment \"etag1\"."); + + // set the runtimeForcedVariation + validProjectConfig.setForcedVariation(experiment.getKey(), whitelistedUserId, expectedVariation.getKey()); + // no attributes provided for a experiment that has an audience + assertThat(decisionService.getVariation(experiment, whitelistedUserId, Collections.emptyMap()), is(expectedVariation)); + + //verify(decisionService).getForcedVariation(experiment.getKey(), whitelistedUserId); + verify(decisionService, never()).getStoredVariation(eq(experiment), any(UserProfile.class)); + assertEquals(decisionService.getWhitelistedVariation(experiment, whitelistedUserId), whitelistVariation); + assertTrue(validProjectConfig.setForcedVariation(experiment.getKey(), whitelistedUserId,null)); + assertNull(validProjectConfig.getForcedVariation(experiment.getKey(), whitelistedUserId)); + assertThat(decisionService.getVariation(experiment, whitelistedUserId, Collections.emptyMap()), is(whitelistVariation)); + } + + /** + * Verify that {@link DecisionService#getVariation(Experiment, String, Map)} + * gives precedence to forced variation bucketing over audience evaluation. + */ + @Test + public void getVariationForcedPrecedesAudienceEval() throws Exception { + Bucketer bucketer = spy(new Bucketer(validProjectConfig)); + DecisionService decisionService = spy(new DecisionService(bucketer, mockErrorHandler, validProjectConfig, null)); + Experiment experiment = validProjectConfig.getExperiments().get(0); + Variation expectedVariation = experiment.getVariations().get(1); + + // user excluded without audiences and whitelisting + assertNull(decisionService.getVariation(experiment, genericUserId, Collections.emptyMap())); + + logbackVerifier.expectMessage(Level.INFO, "User \"" + genericUserId + "\" does not meet conditions to be in experiment \"etag1\"."); + + // set the runtimeForcedVariation + validProjectConfig.setForcedVariation(experiment.getKey(), genericUserId, expectedVariation.getKey()); + // no attributes provided for a experiment that has an audience + assertThat(decisionService.getVariation(experiment, genericUserId, Collections.emptyMap()), is(expectedVariation)); + + verify(decisionService, never()).getStoredVariation(eq(experiment), any(UserProfile.class)); + assertEquals(validProjectConfig.setForcedVariation(experiment.getKey(), genericUserId,null), true); + assertNull(validProjectConfig.getForcedVariation(experiment.getKey(), genericUserId)); + } + + /** + * Verify that {@link DecisionService#getVariation(Experiment, String, Map)} + * gives precedence to forced variation bucketing over user profile. + */ + @Test + public void getVariationForcedBeforeUserProfile() throws Exception { + Experiment experiment = validProjectConfig.getExperiments().get(0); + Variation variation = experiment.getVariations().get(0); + Bucketer bucketer = spy(new Bucketer(validProjectConfig)); + Decision decision = new Decision(variation.getId()); + UserProfile userProfile = new UserProfile(userProfileId, + Collections.singletonMap(experiment.getId(), decision)); + UserProfileService userProfileService = mock(UserProfileService.class); + when(userProfileService.lookup(userProfileId)).thenReturn(userProfile.toMap()); + + DecisionService decisionService = spy(new DecisionService(bucketer, + mockErrorHandler, validProjectConfig, userProfileService)); + + // ensure that normal users still get excluded from the experiment when they fail audience evaluation + assertNull(decisionService.getVariation(experiment, genericUserId, Collections.emptyMap())); + + logbackVerifier.expectMessage(Level.INFO, + "User \"" + genericUserId + "\" does not meet conditions to be in experiment \"" + + experiment.getKey() + "\"."); + + // 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())); + + Variation forcedVariation = experiment.getVariations().get(1); + validProjectConfig.setForcedVariation(experiment.getKey(), userProfileId, forcedVariation.getKey()); + assertEquals(forcedVariation, + decisionService.getVariation(experiment, userProfileId, Collections.emptyMap())); + assertTrue(validProjectConfig.setForcedVariation(experiment.getKey(), userProfileId, null)); + assertNull(validProjectConfig.getForcedVariation(experiment.getKey(), userProfileId)); + + + } /** * Verify that {@link DecisionService#getVariation(Experiment, String, Map)} @@ -141,6 +238,45 @@ public void getVariationEvaluatesUserProfileBeforeAudienceTargeting() throws Exc // 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())); + + } + + /** + * Verify that {@link DecisionService#getVariation(Experiment, String, Map)} + * gives a null variation on a Experiment that is not running. Set the forced variation. + * And, test to make sure that after setting forced variation, the getVariation still returns + * null. + */ + @Test + public void getVariationOnNonRunningExperimentWithForcedVariation() { + Experiment experiment = validProjectConfig.getExperiments().get(1); + assertFalse(experiment.isRunning()); + Variation variation = experiment.getVariations().get(0); + Bucketer bucketer = new Bucketer(validProjectConfig); + + DecisionService decisionService = spy(new DecisionService(bucketer, + mockErrorHandler, validProjectConfig, null)); + + // ensure that the not running variation returns null with no forced variation set. + assertNull(decisionService.getVariation(experiment, "userId", Collections.emptyMap())); + + // we call getVariation 3 times on an experiment that is not running. + logbackVerifier.expectMessage(Level.INFO, + "Experiment \"etag2\" is not running.", times(3)); + + // set a forced variation on the user that got back null + assertTrue(validProjectConfig.setForcedVariation(experiment.getKey(), "userId", variation.getKey())); + + // 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())); + + // set the forced variation back to null + assertTrue(validProjectConfig.setForcedVariation(experiment.getKey(), "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())); + + } //========== get Variation for Feature tests ==========// @@ -252,7 +388,7 @@ public void getVariationForFeatureReturnsVariationReturnedFromGetVarition() { * Test {@link DecisionService#getWhitelistedVariation(Experiment, String)} correctly returns a whitelisted variation. */ @Test - public void getForcedVariationReturnsForcedVariation() { + public void getWhitelistedReturnsForcedVariation() { Bucketer bucketer = new Bucketer(validProjectConfig); DecisionService decisionService = new DecisionService(bucketer, mockErrorHandler, validProjectConfig, null); @@ -266,7 +402,7 @@ public void getForcedVariationReturnsForcedVariation() { * when an invalid variation key is found in the forced variations mapping. */ @Test - public void getForcedVariationWithInvalidVariation() throws Exception { + public void getWhitelistedWithInvalidVariation() throws Exception { String userId = "testUser1"; String invalidVariationKey = "invalidVarKey"; @@ -297,7 +433,7 @@ public void getForcedVariationWithInvalidVariation() throws Exception { * Verify that {@link DecisionService#getWhitelistedVariation(Experiment, String)} returns null when user is not whitelisted. */ @Test - public void getForcedVariationReturnsNullWhenUserIsNotWhitelisted() throws Exception { + public void getWhitelistedReturnsNullWhenUserIsNotWhitelisted() throws Exception { Bucketer bucketer = new Bucketer(validProjectConfig); DecisionService decisionService = new DecisionService(bucketer, mockErrorHandler, validProjectConfig, null); diff --git a/core-api/src/test/java/com/optimizely/ab/config/ProjectConfigTest.java b/core-api/src/test/java/com/optimizely/ab/config/ProjectConfigTest.java index c0cedc238..609dfddba 100644 --- a/core-api/src/test/java/com/optimizely/ab/config/ProjectConfigTest.java +++ b/core-api/src/test/java/com/optimizely/ab/config/ProjectConfigTest.java @@ -33,6 +33,9 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.assertEquals; + import org.junit.Before; import org.junit.Test; @@ -195,4 +198,151 @@ public void verifyAnonymizeIPIsFalseByDefault() throws Exception { ProjectConfig v2ProjectConfig = ProjectConfigTestUtils.validProjectConfigV2(); assertFalse(v2ProjectConfig.getAnonymizeIP()); } + + /** + * Invalid User IDs + + User ID is null + User ID is an empty string + Invalid Experiment IDs + + Experiment key does not exist in the datafile + Experiment key is null + Experiment key is an empty string + Invalid Variation IDs [set only] + + Variation key does not exist in the datafile + Variation key is null + Variation key is an empty string + Multiple set calls [set only] + + Call set variation with different variations on one user/experiment to confirm that each set is expected. + Set variation on multiple variations for one user. + Set variations for multiple users. + */ + /* UserID test */ + @Test + @SuppressFBWarnings("NP") + public void setForcedVariationNullUserId() { + boolean b = projectConfig.setForcedVariation("etag1", null, "vtag1"); + assertFalse(b); + } + @Test + @SuppressFBWarnings("NP") + public void getForcedVariationNullUserId() { + assertNull(projectConfig.getForcedVariation("etag1", null)); + } + + @Test + public void setForcedVariationEmptyUserId() { + assertFalse(projectConfig.setForcedVariation("etag1", "", "vtag1")); + } + @Test + public void getForcedVariationEmptyUserId() { + assertNull(projectConfig.getForcedVariation("etag1", "")); + } + + /* Invalid Experiement */ + @Test + @SuppressFBWarnings("NP") + public void setForcedVariationNullExperimentKey() { + assertFalse(projectConfig.setForcedVariation(null, "testUser1", "vtag1")); + } + @Test + @SuppressFBWarnings("NP") + public void getForcedVariationNullExperimentKey() { + assertNull(projectConfig.getForcedVariation(null, "testUser1")); + } + + @Test + public void setForcedVariationWrongExperimentKey() { + assertFalse(projectConfig.setForcedVariation("wrongKey", "testUser1", "vtag1")); + + } + @Test + public void getForcedVariationWrongExperimentKey() { + assertNull(projectConfig.getForcedVariation("wrongKey", "testUser1")); + } + + @Test + public void setForcedVariationEmptyExperimentKey() { + assertFalse(projectConfig.setForcedVariation("", "testUser1", "vtag1")); + + } + @Test + public void getForcedVariationEmptyExperimentKey() { + assertNull(projectConfig.getForcedVariation("", "testUser1")); + } + + /* Invalid Variation Id (set only */ + @Test + public void setForcedVariationWrongVariationKey() { + assertFalse(projectConfig.setForcedVariation("etag1", "testUser1", "vtag3")); + } + + @Test + public void setForcedVariationNullVariationKey() { + assertFalse(projectConfig.setForcedVariation("etag1", "testUser1", null)); + assertNull(projectConfig.getForcedVariation("etag1", "testUser1")); + } + + @Test + public void setForcedVariationEmptyVariationKey() { + assertFalse(projectConfig.setForcedVariation("etag1", "testUser1", "")); + } + + /* Multiple set calls (set only */ + @Test + public void setForcedVariationDifferentVariations() { + assertTrue(projectConfig.setForcedVariation("etag1", "testUser1", "vtag1")); + assertTrue(projectConfig.setForcedVariation("etag1", "testUser1", "vtag2")); + assertEquals(projectConfig.getForcedVariation("etag1", "testUser1").getKey(), "vtag2"); + assertTrue(projectConfig.setForcedVariation("etag1", "testUser1", null)); + } + + @Test + public void setForcedVariationMultipleVariationsExperiments() { + assertTrue(projectConfig.setForcedVariation("etag1", "testUser1", "vtag1")); + assertTrue(projectConfig.setForcedVariation("etag1", "testUser2", "vtag2")); + assertTrue(projectConfig.setForcedVariation("etag2", "testUser1", "vtag3")); + assertTrue(projectConfig.setForcedVariation("etag2", "testUser2", "vtag4")); + assertEquals(projectConfig.getForcedVariation("etag1", "testUser1").getKey(), "vtag1"); + assertEquals(projectConfig.getForcedVariation("etag1", "testUser2").getKey(), "vtag2"); + assertEquals(projectConfig.getForcedVariation("etag2", "testUser1").getKey(), "vtag3"); + assertEquals(projectConfig.getForcedVariation("etag2", "testUser2").getKey(), "vtag4"); + assertTrue(projectConfig.setForcedVariation("etag1", "testUser1", null)); + assertTrue(projectConfig.setForcedVariation("etag1", "testUser2", null)); + assertTrue(projectConfig.setForcedVariation("etag2", "testUser1", null)); + assertTrue(projectConfig.setForcedVariation("etag2", "testUser2", null)); + assertNull(projectConfig.getForcedVariation("etag1", "testUser1")); + assertNull(projectConfig.getForcedVariation("etag1", "testUser2")); + assertNull(projectConfig.getForcedVariation("etag2", "testUser1")); + assertNull(projectConfig.getForcedVariation("etag2", "testUser2")); + + + } + + @Test + public void setForcedVariationMultipleUsers() { + assertTrue(projectConfig.setForcedVariation("etag1", "testUser1", "vtag1")); + assertTrue(projectConfig.setForcedVariation("etag1", "testUser2", "vtag1")); + assertTrue(projectConfig.setForcedVariation("etag1", "testUser3", "vtag1")); + assertTrue(projectConfig.setForcedVariation("etag1", "testUser4", "vtag1")); + + assertEquals(projectConfig.getForcedVariation("etag1", "testUser1").getKey(), "vtag1"); + assertEquals(projectConfig.getForcedVariation("etag1", "testUser2").getKey(), "vtag1"); + assertEquals(projectConfig.getForcedVariation("etag1", "testUser3").getKey(), "vtag1"); + assertEquals(projectConfig.getForcedVariation("etag1", "testUser4").getKey(), "vtag1"); + + assertTrue(projectConfig.setForcedVariation("etag1", "testUser1", null)); + assertTrue(projectConfig.setForcedVariation("etag1", "testUser2", null)); + assertTrue(projectConfig.setForcedVariation("etag1", "testUser3", null)); + assertTrue(projectConfig.setForcedVariation("etag1", "testUser4", null)); + + assertNull(projectConfig.getForcedVariation("etag1", "testUser1")); + assertNull(projectConfig.getForcedVariation("etag1", "testUser2")); + assertNull(projectConfig.getForcedVariation("etag2", "testUser1")); + assertNull(projectConfig.getForcedVariation("etag2", "testUser2")); + + } } \ No newline at end of file diff --git a/core-api/src/test/java/com/optimizely/ab/event/internal/EventBuilderV2Test.java b/core-api/src/test/java/com/optimizely/ab/event/internal/EventBuilderV2Test.java index 841315924..53719f720 100644 --- a/core-api/src/test/java/com/optimizely/ab/event/internal/EventBuilderV2Test.java +++ b/core-api/src/test/java/com/optimizely/ab/event/internal/EventBuilderV2Test.java @@ -70,7 +70,6 @@ import static org.hamcrest.Matchers.containsInAnyOrder; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; @@ -172,8 +171,8 @@ public void createImpressionEventIgnoresUnknownAttributes() throws Exception { // verify that no Feature is created for "unknownAtrribute" -> "blahValue" for (Feature feature : impression.getUserFeatures()) { - assertNotEquals(feature.getName(), "unknownAttribute"); - assertNotEquals(feature.getValue(), "blahValue"); + assertFalse(feature.getName() == "unknownAttribute"); + assertFalse(feature.getValue() == "blahValue"); } }