[FSSDK-11547] Impression event + notification adjustment (#387)

Author: junaed-optimizely

OptimizelySDK.Tests/DecisionServiceHoldoutTest.cs

@@ -25,6 +25,8 @@
 using OptimizelySDK.Config;
 using OptimizelySDK.Entity;
 using OptimizelySDK.ErrorHandler;
+using OptimizelySDK.Event;
+using OptimizelySDK.Event.Entity;
 using OptimizelySDK.Logger;
 using OptimizelySDK.OptimizelyDecisions;
 
@@ -34,6 +36,7 @@ namespace OptimizelySDK.Tests
     public class DecisionServiceHoldoutTest
     {
         private Mock<ILogger> LoggerMock;
+        private Mock<EventProcessor> EventProcessorMock;
         private DecisionService DecisionService;
         private DatafileProjectConfig Config;
         private JObject TestData;
@@ -46,6 +49,7 @@ public class DecisionServiceHoldoutTest
         public void Initialize()
         {
             LoggerMock = new Mock<ILogger>();
+            EventProcessorMock = new Mock<EventProcessor>();
 
             // Load test data
             var testDataPath = Path.Combine(TestContext.CurrentContext.TestDirectory,
@@ -242,5 +246,90 @@ public void TestGetVariationsForFeatureList_Holdout_DecisionReasons()
                 Assert.IsTrue(decisionWithReasons.DecisionReasons.ToReport().Count > 0, "Should have decision reasons");
             }
         }
+
+        [Test]
+        public void TestImpressionEventForHoldout()
+        {
+            var featureFlag = Config.FeatureKeyMap["test_flag_1"];
+            var userAttributes = new UserAttributes();
+
+            var eventDispatcher = new Event.Dispatcher.DefaultEventDispatcher(LoggerMock.Object);
+            var optimizelyWithMockedEvents = new Optimizely(
+                TestData["datafileWithHoldouts"].ToString(),
+                eventDispatcher,
+                LoggerMock.Object,
+                new ErrorHandler.NoOpErrorHandler(),
+                null, // userProfileService
+                false, // skipJsonValidation
+                EventProcessorMock.Object
+            );
+
+            EventProcessorMock.Setup(ep => ep.Process(It.IsAny<ImpressionEvent>()));
+
+            var userContext = optimizelyWithMockedEvents.CreateUserContext(TestUserId, userAttributes);
+            var decision = userContext.Decide(featureFlag.Key);
+
+            Assert.IsNotNull(decision, "Decision should not be null");
+            Assert.IsNotNull(decision.RuleKey, "RuleKey should not be null");
+
+            var actualHoldout = Config.Holdouts?.FirstOrDefault(h => h.Key == decision.RuleKey);
+
+            Assert.IsNotNull(actualHoldout,
+                $"RuleKey '{decision.RuleKey}' should correspond to a holdout experiment");
+            Assert.AreEqual(featureFlag.Key, decision.FlagKey, "Flag key should match");
+
+            var holdoutVariation = actualHoldout.Variations.FirstOrDefault(v => v.Key == decision.VariationKey);
+
+            Assert.IsNotNull(holdoutVariation,
+                $"Variation '{decision.VariationKey}' should be from the chosen holdout '{actualHoldout.Key}'");
+
+            Assert.AreEqual(holdoutVariation.FeatureEnabled, decision.Enabled,
+                "Enabled flag should match holdout variation's featureEnabled value");
+
+            EventProcessorMock.Verify(ep => ep.Process(It.IsAny<ImpressionEvent>()), Times.Once,
+                "Impression event should be processed exactly once for holdout decision");
+
+            EventProcessorMock.Verify(ep => ep.Process(It.Is<ImpressionEvent>(ie =>
+                ie.Experiment.Key == actualHoldout.Key &&
+                ie.Experiment.Id == actualHoldout.Id &&
+                ie.Timestamp > 0 &&
+                ie.UserId == TestUserId
+            )), Times.Once, "Impression event should contain correct holdout experiment details");
+        }
+
+        [Test]
+        public void TestImpressionEventForHoldout_DisableDecisionEvent()
+        {
+            var featureFlag = Config.FeatureKeyMap["test_flag_1"];
+            var userAttributes = new UserAttributes();
+
+            var eventDispatcher = new Event.Dispatcher.DefaultEventDispatcher(LoggerMock.Object);
+            var optimizelyWithMockedEvents = new Optimizely(
+                TestData["datafileWithHoldouts"].ToString(),
+                eventDispatcher,
+                LoggerMock.Object,
+                new ErrorHandler.NoOpErrorHandler(),
+                null, // userProfileService
+                false, // skipJsonValidation
+                EventProcessorMock.Object
+            );
+
+            EventProcessorMock.Setup(ep => ep.Process(It.IsAny<ImpressionEvent>()));
+
+            var userContext = optimizelyWithMockedEvents.CreateUserContext(TestUserId, userAttributes);
+            var decision = userContext.Decide(featureFlag.Key, new[] { OptimizelyDecideOption.DISABLE_DECISION_EVENT });
+
+            Assert.IsNotNull(decision, "Decision should not be null");
+            Assert.IsNotNull(decision.RuleKey, "User should be bucketed into a holdout");
+
+            var chosenHoldout = Config.Holdouts?.FirstOrDefault(h => h.Key == decision.RuleKey);
+
+            Assert.IsNotNull(chosenHoldout, $"Holdout '{decision.RuleKey}' should exist in config");
+
+            Assert.AreEqual(featureFlag.Key, decision.FlagKey, "Flag key should match");
+
+            EventProcessorMock.Verify(ep => ep.Process(It.IsAny<ImpressionEvent>()), Times.Never,
+                "No impression event should be processed when DISABLE_DECISION_EVENT option is used");
+        }
     }
 }

OptimizelySDK.Tests/OptimizelyUserContextHoldoutTest.cs

@@ -28,7 +28,9 @@
 using OptimizelySDK.Event;
 using OptimizelySDK.Event.Dispatcher;
 using OptimizelySDK.Logger;
+using OptimizelySDK.Notifications;
 using OptimizelySDK.OptimizelyDecisions;
+using OptimizelySDK.Utils;
 
 namespace OptimizelySDK.Tests
 {
@@ -572,6 +574,92 @@ public void TestDecideReasons_HoldoutDecisionContainsRelevantReasons()
                 "Should contain holdout-related decision reasoning");
         }
 
+
+        #endregion
+
+        #region Notification test
+
+        [Test]
+        public void TestDecide_HoldoutNotificationContent()
+        {
+            var capturedNotifications = new List<Dictionary<string, object>>();
+
+            NotificationCenter.DecisionCallback notificationCallback =
+                (decisionType, userId, userAttributes, decisionInfo) =>
+                {
+                    capturedNotifications.Add(new Dictionary<string, object>(decisionInfo));
+                };
+
+            OptimizelyInstance.NotificationCenter.AddNotification(
+                NotificationCenter.NotificationType.Decision,
+                notificationCallback);
+
+            var userContext = OptimizelyInstance.CreateUserContext(TestUserId,
+                new UserAttributes { { "country", "us" } });
+            var decision = userContext.Decide("test_flag_1");
+
+            Assert.AreEqual(1, capturedNotifications.Count,
+                "Should have captured exactly one decision notification");
+
+            var notification = capturedNotifications.First();
+
+            Assert.IsTrue(notification.ContainsKey("ruleKey"),
+                "Notification should contain ruleKey");
+
+            var ruleKey = notification["ruleKey"]?.ToString();
+
+            Assert.IsNotNull(ruleKey, "RuleKey should not be null");
+
+            var holdoutExperiment = Config.Holdouts?.FirstOrDefault(h => h.Key == ruleKey);
+
+            Assert.IsNotNull(holdoutExperiment,
+                $"RuleKey '{ruleKey}' should correspond to a holdout experiment");
+            Assert.IsTrue(notification.ContainsKey("flagKey"),
+                "Holdout notification should contain flagKey");
+            Assert.IsTrue(notification.ContainsKey("enabled"),
+                "Holdout notification should contain enabled flag");
+            Assert.IsTrue(notification.ContainsKey("variationKey"),
+                "Holdout notification should contain variationKey");
+            Assert.IsTrue(notification.ContainsKey("experimentId"),
+                "Holdout notification should contain experimentId");
+            Assert.IsTrue(notification.ContainsKey("variationId"),
+                "Holdout notification should contain variationId");
+
+            var flagKey = notification["flagKey"]?.ToString();
+
+            Assert.AreEqual("test_flag_1", flagKey, "FlagKey should match the requested flag");
+
+            var experimentId = notification["experimentId"]?.ToString();
+            Assert.AreEqual(holdoutExperiment.Id, experimentId,
+                "ExperimentId in notification should match holdout experiment ID");
+
+            var variationId = notification["variationId"]?.ToString();
+            var holdoutVariation = holdoutExperiment.Variations?.FirstOrDefault(v => v.Id == variationId);
+
+            Assert.IsNotNull(holdoutVariation,
+                $"VariationId '{variationId}' should correspond to a holdout variation");
+
+            var variationKey = notification["variationKey"]?.ToString();
+
+            Assert.AreEqual(holdoutVariation.Key, variationKey,
+                "VariationKey in notification should match holdout variation key");
+
+            var enabled = notification["enabled"];
+
+            Assert.IsNotNull(enabled, "Enabled flag should be present in notification");
+            Assert.AreEqual(holdoutVariation.FeatureEnabled, (bool)enabled,
+                "Enabled flag should match holdout variation's featureEnabled value");
+
+            Assert.IsTrue(Config.FeatureKeyMap.ContainsKey(flagKey),
+                $"FlagKey '{flagKey}' should exist in config");
+
+            Assert.IsTrue(notification.ContainsKey("variables"),
+                "Notification should contain variables");
+            Assert.IsTrue(notification.ContainsKey("reasons"),
+                "Notification should contain reasons");
+            Assert.IsTrue(notification.ContainsKey("decisionEventDispatched"),
+                "Notification should contain decisionEventDispatched");
+        }
         #endregion
     }
 }

OptimizelySDK/Bucketing/DecisionService.cs

@@ -759,7 +759,7 @@ public virtual Result<FeatureDecision> GetDecisionForFlag(
             var userId = user.GetUserId();
 
             // Check holdouts first (highest priority)
-            var holdouts = projectConfig.GetHoldoutsForFlag(featureFlag.Key);
+            var holdouts = projectConfig.GetHoldoutsForFlag(featureFlag.Id);
             foreach (var holdout in holdouts)
             {
                 var holdoutDecision = GetVariationForHoldout(holdout, user, projectConfig);
@@ -915,10 +915,7 @@ ProjectConfig config
                 var infoMessage = $"Holdout \"{holdout.Key}\" is not running.";
                 Logger.Log(LogLevel.INFO, infoMessage);
                 reasons.AddInfo(infoMessage);
-                return Result<FeatureDecision>.NewResult(
-                    new FeatureDecision(null, null, FeatureDecision.DECISION_SOURCE_HOLDOUT),
-                    reasons
-                );
+                return Result<FeatureDecision>.NullResult(reasons);
             }
 
             var audienceResult = ExperimentUtils.DoesUserMeetAudienceConditions(
@@ -934,18 +931,15 @@ ProjectConfig config
             if (!audienceResult.ResultObject)
             {
                 reasons.AddInfo($"User \"{userId}\" does not meet conditions for holdout ({holdout.Key}).");
-                return Result<FeatureDecision>.NewResult(
-                    new FeatureDecision(null, null, FeatureDecision.DECISION_SOURCE_HOLDOUT),
-                    reasons
-                );
+                return Result<FeatureDecision>.NullResult(reasons);
             }
 
             var attributes = user.GetAttributes();
             var bucketingIdResult = GetBucketingId(userId, attributes);
             var bucketedVariation = Bucketer.Bucket(config, holdout, bucketingIdResult.ResultObject, userId);
             reasons += bucketedVariation.DecisionReasons;
 
-            if (bucketedVariation.ResultObject != null)
+            if (bucketedVariation.ResultObject != null && !string.IsNullOrEmpty(bucketedVariation.ResultObject.Key))
             {
                 reasons.AddInfo($"User \"{userId}\" is bucketed into holdout variation \"{bucketedVariation.ResultObject.Key}\".");
                 return Result<FeatureDecision>.NewResult(
@@ -955,11 +949,7 @@ ProjectConfig config
             }
 
             reasons.AddInfo($"User \"{userId}\" is not bucketed into holdout variation \"{holdout.Key}\".");
-
-            return Result<FeatureDecision>.NewResult(
-                new FeatureDecision(null, null, FeatureDecision.DECISION_SOURCE_HOLDOUT),
-                reasons
-            );
+            return Result<FeatureDecision>.NullResult(reasons);
         }
         /// <summary>
         /// Finds a validated forced decision.

OptimizelySDK/Config/DatafileProjectConfig.cs

@@ -875,13 +875,13 @@ public string ToDatafile()
         }
 
         /// <summary>
-        /// Get holdout instances associated with the given feature flag key.
+        /// Get holdout instances associated with the given feature flag Id.
         /// </summary>
-        /// <param name="flagKey">Feature flag key</param>
+        /// <param name="flagId">Feature flag Id</param>
         /// <returns>Array of holdouts associated with the flag, empty array if none</returns>
-        public Holdout[] GetHoldoutsForFlag(string flagKey)
+        public Holdout[] GetHoldoutsForFlag(string flagId)
         {
-            var holdouts = _holdoutConfig?.GetHoldoutsForFlag(flagKey);
+            var holdouts = _holdoutConfig?.GetHoldoutsForFlag(flagId);
             return holdouts?.ToArray() ?? new Holdout[0];
         }
         /// Returns the datafile corresponding to ProjectConfig

OptimizelySDK/Optimizely.cs

@@ -564,7 +564,8 @@ public virtual bool IsFeatureEnabled(string featureKey, string userId,
 
                 // This information is only necessary for feature tests.
                 // For rollouts experiments and variations are an implementation detail only.
-                if (decision?.Source == FeatureDecision.DECISION_SOURCE_FEATURE_TEST)
+                if (decision?.Source == FeatureDecision.DECISION_SOURCE_FEATURE_TEST ||
+                    decision?.Source == FeatureDecision.DECISION_SOURCE_HOLDOUT)
                 {
                     decisionSource = decision.Source;
                     sourceInfo["experimentKey"] = decision.Experiment.Key;

OptimizelySDK/ProjectConfig.cs

@@ -321,11 +321,11 @@ public interface ProjectConfig
         Holdout GetHoldout(string holdoutId);
 
         /// <summary>
-        /// Get holdout instances associated with the given feature flag key.
+        /// Get holdout instances associated with the given feature flag Id.
         /// </summary>
-        /// <param name="flagKey">Feature flag key</param>
+        /// <param name="flagKey">Feature flag Id</param>
         /// <returns>Array of holdouts associated with the flag, empty array if none</returns>
-        Holdout[] GetHoldoutsForFlag(string flagKey);
+        Holdout[] GetHoldoutsForFlag(string flagId);
 
         /// <summary>
         /// Returns the datafile corresponding to ProjectConfig