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

Author: mikeproeng37

OptimizelySDK.Tests/OptimizelyTest.cs

@@ -38,6 +38,7 @@ public class OptimizelyTest
         private ProjectConfig Config;
         private Mock<EventBuilder> EventBuilderMock;
         private Mock<IErrorHandler> ErrorHandlerMock;
+        private Mock<IEventDispatcher> EventDispatcherMock;
         private Optimizely Optimizely;
         private IEventDispatcher EventDispatcher;
         private const string TestUserId = "testUserId";
@@ -73,19 +74,19 @@ public void Initialize()
                 logger: LoggerMock.Object,
                 errorHandler: new NoOpErrorHandler());
 
-            EventDispatcher = new ValidEventDispatcher();
-            Optimizely = new Optimizely(TestData.Datafile, EventDispatcher, LoggerMock.Object, ErrorHandlerMock.Object);
+            EventDispatcherMock = new Mock<IEventDispatcher>();
+            Optimizely = new Optimizely(TestData.Datafile, EventDispatcherMock.Object, LoggerMock.Object, ErrorHandlerMock.Object);
 
             Helper = new OptimizelyHelper
             {
                 Datafile = TestData.Datafile,
-                EventDispatcher = EventDispatcher,
+                EventDispatcher = EventDispatcherMock.Object,
                 Logger = LoggerMock.Object,
                 ErrorHandler = ErrorHandlerMock.Object,
                 SkipJsonValidation = false,
             };
 
-            OptimizelyMock = new Mock<Optimizely>(TestData.Datafile, EventDispatcher, LoggerMock.Object, ErrorHandlerMock.Object, null, false)
+            OptimizelyMock = new Mock<Optimizely>(TestData.Datafile, EventDispatcherMock.Object, LoggerMock.Object, ErrorHandlerMock.Object, null, false)
             {
                 CallBase = true
             };
@@ -1539,6 +1540,34 @@ public void TestIsFeatureEnabledGivenFeatureFlagIsEnabledAndUserIsBeingExperimen
                 $@"Feature flag ""{featureKey}"" is enabled for user ""{TestUserId}""."));
         }
 
+        // Should return false and send an impression event when feature is enabled for the user 
+        // and user is being experimented.
+        [Test]
+        public void TestIsFeatureEnabledGivenFeatureFlagIsNotEnabledAndUserIsBeingExperimented()
+        {
+            var featureKey = "double_single_variable_feature";
+            var experiment = Config.GetExperimentFromKey("test_experiment_double_feature");
+            var variation = Config.GetVariationFromKey("test_experiment_double_feature", "variation");
+            var featureFlag = Config.GetFeatureFlagFromKey(featureKey);
+            var decision = new FeatureDecision(experiment, variation, FeatureDecision.DECISION_SOURCE_EXPERIMENT);
+
+            DecisionServiceMock.Setup(ds => ds.GetVariationForFeature(featureFlag, TestUserId, null)).Returns(decision);
+
+            var optly = Helper.CreatePrivateOptimizely();
+            optly.SetFieldOrProperty("DecisionService", DecisionServiceMock.Object);
+
+            bool result = (bool)optly.Invoke("IsFeatureEnabled", featureKey, TestUserId, null);
+            Assert.False(result);
+
+            // SendImpressionEvent() gets called.
+            LoggerMock.Verify(l => l.Log(LogLevel.INFO,
+                $@"The user ""{TestUserId}"" is not being experimented on feature ""{featureKey}""."), Times.Never);
+
+            LoggerMock.Verify(l => l.Log(LogLevel.INFO,
+                $@"Feature flag ""{featureKey}"" is not enabled for user ""{TestUserId}""."));
+            EventDispatcherMock.Verify(dispatcher => dispatcher.DispatchEvent(It.IsAny<LogEvent>()));
+        }
+
         // Verify that IsFeatureEnabled returns true if a variation does not get found in the feature
         // flag experiment but found in the rollout rule.
         [Test]

OptimizelySDK/Optimizely.cs

@@ -366,14 +366,12 @@ public Variation GetForcedVariation(string experimentKey, string userId)
         /// <returns>True if feature is enabled, false or null otherwise</returns>
         public virtual bool IsFeatureEnabled(string featureKey, string userId, UserAttributes userAttributes = null)
         {
-            if (string.IsNullOrEmpty(userId))
-            {
+            if (string.IsNullOrEmpty(userId)) {
                 Logger.Log(LogLevel.ERROR, "User ID must not be empty.");
                 return false;
             }
 
-            if (string.IsNullOrEmpty(featureKey))
-            {
+            if (string.IsNullOrEmpty(featureKey)) {
                 Logger.Log(LogLevel.ERROR, "Feature flag key must not be empty.");
                 return false;
             }
@@ -386,19 +384,21 @@ public virtual bool IsFeatureEnabled(string featureKey, string userId, UserAttri
                 return false;
 
             var decision = DecisionService.GetVariationForFeature(featureFlag, userId, userAttributes);
-            if (decision == null || !decision.Variation.IsFeatureEnabled)
-            {
-                Logger.Log(LogLevel.INFO, $@"Feature flag ""{featureKey}"" is not enabled for user ""{userId}"".");
-                return false;
+            if (decision != null) {
+                if (decision.Source == FeatureDecision.DECISION_SOURCE_EXPERIMENT) {
+                    SendImpressionEvent(decision.Experiment, decision.Variation, userId, userAttributes);
+                } else {
+                    Logger.Log(LogLevel.INFO, $@"The user ""{userId}"" is not being experimented on feature ""{featureKey}"".");
+                }
+                if (decision.Variation.IsFeatureEnabled) {
+                    Logger.Log(LogLevel.INFO, $@"Feature flag ""{featureKey}"" is enabled for user ""{userId}"".");
+                    return true;
+                }
             }
 
-            if (decision.Source == FeatureDecision.DECISION_SOURCE_EXPERIMENT)
-                SendImpressionEvent(decision.Experiment, decision.Variation, userId, userAttributes);
-            else
-                Logger.Log(LogLevel.INFO, $@"The user ""{userId}"" is not being experimented on feature ""{featureKey}"".");
 
-            Logger.Log(LogLevel.INFO, $@"Feature flag ""{featureKey}"" is enabled for user ""{userId}"".");
-            return true;
+            Logger.Log(LogLevel.INFO, $@"Feature flag ""{featureKey}"" is not enabled for user ""{userId}"".");
+            return false;
         }
 
         /// <summary>