Fix impression sent from feature experiment variation toggled off. (#193)

Author: mikeproeng37

core-api/src/main/java/com/optimizely/ab/Optimizely.java

@@ -347,10 +347,7 @@ else if (userId == null) {
         Map<String, String> filteredAttributes = filterAttributes(projectConfig, attributes);
 
         FeatureDecision featureDecision = decisionService.getVariationForFeature(featureFlag, userId, filteredAttributes);
-        if (featureDecision.variation == null || !featureDecision.variation.getFeatureEnabled()) {
-            logger.info("Feature \"{}\" is not enabled for user \"{}\".", featureKey, userId);
-            return false;
-        } else {
+        if (featureDecision.variation != null) {
             if (featureDecision.decisionSource.equals(FeatureDecision.DecisionSource.EXPERIMENT)) {
                 sendImpression(
                         projectConfig,
@@ -362,9 +359,14 @@ else if (userId == null) {
                 logger.info("The user \"{}\" is not included in an experiment for feature \"{}\".",
                         userId, featureKey);
             }
-            logger.info("Feature \"{}\" is enabled for user \"{}\".", featureKey, userId);
-            return true;
+            if (featureDecision.variation.getFeatureEnabled()) {
+                logger.info("Feature \"{}\" is enabled for user \"{}\".", featureKey, userId);
+                return true;
+            }
         }
+
+        logger.info("Feature \"{}\" is not enabled for user \"{}\".", featureKey, userId);
+        return false;
     }
 
     /**

core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java

@@ -3479,15 +3479,13 @@ public void isFeatureEnabledReturnsTrueButDoesNotSendWhenUserIsBucketedIntoVaria
      * false so feature enabled will return false
      */
     @Test
-    public void isFeatureEnabledWithExperimentKeyForcedOfFeatureEnabledFalse() throws Exception {
+    public void isFeatureEnabledWithExperimentKeyForcedOffFeatureEnabledFalse() throws Exception {
         assumeTrue(datafileVersion >= Integer.parseInt(ProjectConfig.Version.V4.toString()));
         Experiment activatedExperiment = validProjectConfig.getExperimentKeyMapping().get(EXPERIMENT_MULTIVARIATE_EXPERIMENT_KEY);
         Variation forcedVariation = activatedExperiment.getVariations().get(2);
-        EventBuilder mockEventBuilder = mock(EventBuilder.class);
 
         Optimizely optimizely = Optimizely.builder(validDatafile, mockEventHandler)
                 .withBucketing(mockBucketer)
-                .withEventBuilder(mockEventBuilder)
                 .withConfig(validProjectConfig)
                 .withErrorHandler(mockErrorHandler)
                 .build();
@@ -3506,11 +3504,9 @@ public void isFeatureEnabledWithExperimentKeyForcedWithNoFeatureEnabledSet() thr
         assumeTrue(datafileVersion >= Integer.parseInt(ProjectConfig.Version.V4.toString()));
         Experiment activatedExperiment = validProjectConfig.getExperimentKeyMapping().get(EXPERIMENT_DOUBLE_FEATURE_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();
@@ -3584,6 +3580,44 @@ public void isFeatureEnabledFalseWhenFeatureEnabledOfVariationIsFalse() throws E
 
     }
 
+    /**
+     * Verify {@link Optimizely#isFeatureEnabled(String, String)} calls into
+     * {@link Optimizely#isFeatureEnabled(String, String, Map)} and they both
+     * return False
+     * when the user is bucketed an feature test variation that is turned off.
+     * @throws Exception
+     */
+    @Test
+    public void isFeatureEnabledReturnsFalseAndDispatchesWhenUserIsBucketedIntoAnExperimentVariationToggleOff() 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());
+
+        Experiment activatedExperiment = validProjectConfig.getExperimentKeyMapping().get(EXPERIMENT_MULTIVARIATE_EXPERIMENT_KEY);
+        Variation variation = new Variation("2", "variation_toggled_off", false, null);
+
+        FeatureDecision featureDecision = new FeatureDecision(activatedExperiment, variation, FeatureDecision.DecisionSource.EXPERIMENT);
+        doReturn(featureDecision).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(mockEventHandler, times(1)).dispatchEvent(any(LogEvent.class));
+    }
+
     /** Integration Test
      * Verify {@link Optimizely#isFeatureEnabled(String, String, Map)}
      * returns True