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 0362c4be3..36d91f77d 100644 --- a/core-api/src/main/java/com/optimizely/ab/Optimizely.java +++ b/core-api/src/main/java/com/optimizely/ab/Optimizely.java @@ -175,6 +175,16 @@ Variation activate(@Nonnull ProjectConfig projectConfig, return null; } + sendImpression(projectConfig, experiment, userId, filteredAttributes, variation); + + return variation; + } + + private void sendImpression(@Nonnull ProjectConfig projectConfig, + @Nonnull Experiment experiment, + @Nonnull String userId, + @Nonnull Map filteredAttributes, + @Nonnull Variation variation) { if (experiment.isRunning()) { LogEvent impressionEvent = eventBuilder.createImpressionEvent( projectConfig, @@ -196,8 +206,6 @@ Variation activate(@Nonnull ProjectConfig projectConfig, } else { logger.info("Experiment has \"Launched\" status so not dispatching event during activation."); } - - return variation; } //======== track calls ========// @@ -293,10 +301,9 @@ public void track(@Nonnull String eventName, * @param userId The ID of the user. * @return True if the feature is enabled. * False if the feature is disabled. - * Will always return True if toggling the feature is disabled. - * Will return Null if the feature is not found. + * False if the feature is not found. */ - public @Nullable Boolean isFeatureEnabled(@Nonnull String featureKey, + public @Nonnull Boolean isFeatureEnabled(@Nonnull String featureKey, @Nonnull String userId) { return isFeatureEnabled(featureKey, userId, Collections.emptyMap()); } @@ -310,13 +317,43 @@ public void track(@Nonnull String eventName, * @param attributes The user's attributes. * @return True if the feature is enabled. * False if the feature is disabled. - * Will always return True if toggling the feature is disabled. - * Will return Null if the feature is not found. + * False if the feature is not found. */ - public @Nullable Boolean isFeatureEnabled(@Nonnull String featureKey, + public @Nonnull Boolean isFeatureEnabled(@Nonnull String featureKey, @Nonnull String userId, @Nonnull Map attributes) { - return getFeatureVariableBoolean(featureKey, "", userId, attributes); + FeatureFlag featureFlag = projectConfig.getFeatureKeyMapping().get(featureKey); + if (featureFlag == null) { + logger.info("No feature flag was found for key \"" + featureKey + "\"."); + return false; + } + + Map filteredAttributes = filterAttributes(projectConfig, attributes); + + Variation variation = decisionService.getVariationForFeature(featureFlag, userId, filteredAttributes); + + if (variation != null) { + Experiment experiment = projectConfig.getExperimentForVariationId(variation.getId()); + if (experiment != null) { + // the user is in an experiment for the feature + sendImpression( + projectConfig, + experiment, + userId, + filteredAttributes, + variation); + } + else { + logger.info("The user \"" + userId + + "\" is not being experimented on in feature \"" + featureKey + "\"."); + } + logger.info("Feature \"" + featureKey + "\" is enabled for user \"" + userId + "\"."); + return true; + } + else { + logger.info("Feature \"" + featureKey + "\" is not enabled for user \"" + userId + "\"."); + return false; + } } /** 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 256472461..ffb891e29 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 @@ -20,9 +20,11 @@ import com.optimizely.ab.config.audience.Audience; import com.optimizely.ab.config.audience.Condition; +import javax.annotation.Nullable; import javax.annotation.concurrent.Immutable; import java.util.ArrayList; import java.util.Collections; +import java.util.HashMap; import java.util.List; import java.util.Map; @@ -80,6 +82,7 @@ public String toString() { // other mappings private final Map> liveVariableIdToExperimentsMapping; private final Map> variationToLiveVariableUsageInstanceMapping; + private final Map variationIdToExperimentMapping; // v2 constructor public ProjectConfig(String accountId, String projectId, String version, String revision, List groups, @@ -146,6 +149,14 @@ public ProjectConfig(String accountId, allExperiments.addAll(aggregateGroupExperiments(groups)); this.experiments = Collections.unmodifiableList(allExperiments); + Map variationIdToExperimentMap = new HashMap(); + for (Experiment experiment : this.experiments) { + for (Variation variation: experiment.getVariations()) { + variationIdToExperimentMap.put(variation.getId(), experiment); + } + } + this.variationIdToExperimentMapping = Collections.unmodifiableMap(variationIdToExperimentMap); + // generate the name mappers this.attributeKeyMapping = ProjectConfigUtils.generateNameMapping(attributes); this.eventNameMapping = ProjectConfigUtils.generateNameMapping(this.events); @@ -172,6 +183,10 @@ public ProjectConfig(String accountId, } } + public @Nullable Experiment getExperimentForVariationId(String variationId) { + return this.variationIdToExperimentMapping.get(variationId); + } + private List aggregateGroupExperiments(List groups) { List groupExperiments = new ArrayList(); for (Group group : groups) { 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 e981704e4..92ffcddcc 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java @@ -23,6 +23,7 @@ import com.optimizely.ab.config.Attribute; import com.optimizely.ab.config.EventType; import com.optimizely.ab.config.Experiment; +import com.optimizely.ab.config.FeatureFlag; import com.optimizely.ab.config.LiveVariable; import com.optimizely.ab.config.LiveVariableUsageInstance; import com.optimizely.ab.config.ProjectConfig; @@ -36,6 +37,7 @@ import com.optimizely.ab.event.LogEvent; import com.optimizely.ab.event.internal.EventBuilder; import com.optimizely.ab.event.internal.EventBuilderV2; +import com.optimizely.ab.event.internal.payload.Feature; import com.optimizely.ab.internal.LogbackVerifier; import com.optimizely.ab.notification.NotificationListener; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; @@ -97,6 +99,7 @@ import static org.hamcrest.Matchers.hasEntry; import static org.hamcrest.Matchers.hasKey; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assume.assumeTrue; @@ -2371,6 +2374,166 @@ public void getFeatureVariableValueReturnsVariationValueWhenUserGetsBucketedToVa assertEquals(expectedValue, value); } + /** + * Verify {@link Optimizely#isFeatureEnabled(String, String)} calls into + * {@link Optimizely#isFeatureEnabled(String, String, Map)} and they both + * return False + * when the APIs are called with an feature key that is not in the datafile. + * @throws Exception + */ + @Test + public void isFeatureEnabledReturnsFalseWhenFeatureFlagKeyIsInvalid() throws Exception { + + String invalidFeatureKey = "nonexistent feature key"; + + Optimizely spyOptimizely = spy(Optimizely.builder(validDatafile, mockEventHandler) + .withConfig(validProjectConfig) + .withDecisionService(mockDecisionService) + .build()); + + assertFalse(spyOptimizely.isFeatureEnabled(invalidFeatureKey, genericUserId)); + + logbackVerifier.expectMessage( + Level.INFO, + "No feature flag was found for key \"" + invalidFeatureKey + "\"." + ); + verify(spyOptimizely, times(1)).isFeatureEnabled( + eq(invalidFeatureKey), + eq(genericUserId), + eq(Collections.emptyMap()) + ); + verify(mockDecisionService, never()).getVariation( + any(Experiment.class), + anyString(), + anyMapOf(String.class, String.class)); + verify(mockEventHandler, never()).dispatchEvent(any(LogEvent.class)); + } + + /** + * Verify {@link Optimizely#isFeatureEnabled(String, String)} calls into + * {@link Optimizely#isFeatureEnabled(String, String, Map)} and they both + * return False + * when the user is not bucketed into any variation for the feature. + * @throws Exception + */ + @Test + public void isFeatureEnabledReturnsFalseWhenUserIsNotBucketedIntoAnyVariation() throws Exception { + assumeTrue(datafileVersion >= Integer.parseInt(ProjectConfig.Version.V4.toString())); + + String validFeatureKey = FEATURE_MULTI_VARIATE_FEATURE_KEY; + + Optimizely spyOptimizely = spy(Optimizely.builder(validDatafile, mockEventHandler) + .withConfig(validProjectConfig) + .withDecisionService(mockDecisionService) + .build()); + + doReturn(null).when(mockDecisionService).getVariationForFeature( + any(FeatureFlag.class), + anyString(), + anyMapOf(String.class, String.class) + ); + + assertFalse(spyOptimizely.isFeatureEnabled(validFeatureKey, genericUserId)); + + logbackVerifier.expectMessage( + Level.INFO, + "Feature \"" + validFeatureKey + + "\" is not enabled for user \"" + genericUserId + "\"." + ); + verify(spyOptimizely).isFeatureEnabled( + eq(validFeatureKey), + eq(genericUserId), + eq(Collections.emptyMap()) + ); + verify(mockDecisionService).getVariationForFeature( + eq(FEATURE_FLAG_MULTI_VARIATE_FEATURE), + eq(genericUserId), + eq(Collections.emptyMap()) + ); + verify(mockEventHandler, never()).dispatchEvent(any(LogEvent.class)); + } + + /** + * Verify {@link Optimizely#isFeatureEnabled(String, String)} calls into + * {@link Optimizely#isFeatureEnabled(String, String, Map)} and they both + * return True + * when the user is bucketed into a variation for the feature. + * An impression event should not be dispatched since the user was not bucketed into an Experiment. + * @throws Exception + */ + @Test + public void isFeatureEnabledReturnsTrueButDoesNotSendWhenUserIsBucketedIntoVariationWithoutExperiment() throws Exception { + assumeTrue(datafileVersion >= Integer.parseInt(ProjectConfig.Version.V4.toString())); + + String validFeatureKey = FEATURE_MULTI_VARIATE_FEATURE_KEY; + + Optimizely spyOptimizely = spy(Optimizely.builder(validDatafile, mockEventHandler) + .withConfig(validProjectConfig) + .withDecisionService(mockDecisionService) + .build()); + + doReturn(new Variation("variationId", "variationKey")).when(mockDecisionService).getVariationForFeature( + eq(FEATURE_FLAG_MULTI_VARIATE_FEATURE), + eq(genericUserId), + eq(Collections.emptyMap()) + ); + + assertTrue(spyOptimizely.isFeatureEnabled(validFeatureKey, genericUserId)); + + logbackVerifier.expectMessage( + Level.INFO, + "The user \"" + genericUserId + + "\" is not being experimented on in feature \"" + validFeatureKey + "\"." + ); + logbackVerifier.expectMessage( + Level.INFO, + "Feature \"" + validFeatureKey + + "\" is enabled for user \"" + genericUserId + "\"." + ); + verify(spyOptimizely).isFeatureEnabled( + eq(validFeatureKey), + eq(genericUserId), + eq(Collections.emptyMap()) + ); + verify(mockDecisionService).getVariationForFeature( + eq(FEATURE_FLAG_MULTI_VARIATE_FEATURE), + eq(genericUserId), + eq(Collections.emptyMap()) + ); + verify(mockEventHandler, never()).dispatchEvent(any(LogEvent.class)); + } + + /** Integration Test + * Verify {@link Optimizely#isFeatureEnabled(String, String, Map)} + * returns True + * when the user is bucketed into a variation for the feature. + * The user is also bucketed into an experiment, so we verify that an event is dispatched. + * @throws Exception + */ + @Test + public void isFeatureEnabledReturnsTrueAndDispatchesEventWhenUserIsBucketedIntoAnExperiment() throws Exception { + assumeTrue(datafileVersion >= Integer.parseInt(ProjectConfig.Version.V4.toString())); + + String validFeatureKey = FEATURE_MULTI_VARIATE_FEATURE_KEY; + + Optimizely optimizely = Optimizely.builder(validDatafile, mockEventHandler) + .withConfig(validProjectConfig) + .build(); + + assertTrue(optimizely.isFeatureEnabled( + validFeatureKey, + genericUserId, + Collections.singletonMap(ATTRIBUTE_HOUSE_KEY, AUDIENCE_GRYFFINDOR_VALUE) + )); + + logbackVerifier.expectMessage( + Level.INFO, + "Feature \"" + validFeatureKey + + "\" is enabled for user \"" + genericUserId + "\"." + ); + verify(mockEventHandler, times(1)).dispatchEvent(any(LogEvent.class)); + } + //======== Helper methods ========// private Experiment createUnknownExperiment() {