diff --git a/OptimizelySDK.Tests/DecisionServiceHoldoutTest.cs b/OptimizelySDK.Tests/DecisionServiceHoldoutTest.cs index a9457cdb..3d34e151 100644 --- a/OptimizelySDK.Tests/DecisionServiceHoldoutTest.cs +++ b/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 LoggerMock; + private Mock EventProcessorMock; private DecisionService DecisionService; private DatafileProjectConfig Config; private JObject TestData; @@ -46,6 +49,7 @@ public class DecisionServiceHoldoutTest public void Initialize() { LoggerMock = new Mock(); + EventProcessorMock = new Mock(); // 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())); + + 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()), Times.Once, + "Impression event should be processed exactly once for holdout decision"); + + EventProcessorMock.Verify(ep => ep.Process(It.Is(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())); + + 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()), Times.Never, + "No impression event should be processed when DISABLE_DECISION_EVENT option is used"); + } } } diff --git a/OptimizelySDK.Tests/OptimizelyUserContextHoldoutTest.cs b/OptimizelySDK.Tests/OptimizelyUserContextHoldoutTest.cs index 369dabb9..978d207a 100644 --- a/OptimizelySDK.Tests/OptimizelyUserContextHoldoutTest.cs +++ b/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>(); + + NotificationCenter.DecisionCallback notificationCallback = + (decisionType, userId, userAttributes, decisionInfo) => + { + capturedNotifications.Add(new Dictionary(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 } } diff --git a/OptimizelySDK/Bucketing/DecisionService.cs b/OptimizelySDK/Bucketing/DecisionService.cs index ad5487a2..7bc8054b 100644 --- a/OptimizelySDK/Bucketing/DecisionService.cs +++ b/OptimizelySDK/Bucketing/DecisionService.cs @@ -759,7 +759,7 @@ public virtual Result 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.NewResult( - new FeatureDecision(null, null, FeatureDecision.DECISION_SOURCE_HOLDOUT), - reasons - ); + return Result.NullResult(reasons); } var audienceResult = ExperimentUtils.DoesUserMeetAudienceConditions( @@ -934,10 +931,7 @@ ProjectConfig config if (!audienceResult.ResultObject) { reasons.AddInfo($"User \"{userId}\" does not meet conditions for holdout ({holdout.Key})."); - return Result.NewResult( - new FeatureDecision(null, null, FeatureDecision.DECISION_SOURCE_HOLDOUT), - reasons - ); + return Result.NullResult(reasons); } var attributes = user.GetAttributes(); @@ -945,7 +939,7 @@ ProjectConfig config 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.NewResult( @@ -955,11 +949,7 @@ ProjectConfig config } reasons.AddInfo($"User \"{userId}\" is not bucketed into holdout variation \"{holdout.Key}\"."); - - return Result.NewResult( - new FeatureDecision(null, null, FeatureDecision.DECISION_SOURCE_HOLDOUT), - reasons - ); + return Result.NullResult(reasons); } /// /// Finds a validated forced decision. diff --git a/OptimizelySDK/Config/DatafileProjectConfig.cs b/OptimizelySDK/Config/DatafileProjectConfig.cs index 6721832e..e701bc4e 100644 --- a/OptimizelySDK/Config/DatafileProjectConfig.cs +++ b/OptimizelySDK/Config/DatafileProjectConfig.cs @@ -875,13 +875,13 @@ public string ToDatafile() } /// - /// Get holdout instances associated with the given feature flag key. + /// Get holdout instances associated with the given feature flag Id. /// - /// Feature flag key + /// Feature flag Id /// Array of holdouts associated with the flag, empty array if none - 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 diff --git a/OptimizelySDK/Optimizely.cs b/OptimizelySDK/Optimizely.cs index 23483360..9040ea17 100644 --- a/OptimizelySDK/Optimizely.cs +++ b/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; diff --git a/OptimizelySDK/ProjectConfig.cs b/OptimizelySDK/ProjectConfig.cs index 6a2b5259..992c900b 100644 --- a/OptimizelySDK/ProjectConfig.cs +++ b/OptimizelySDK/ProjectConfig.cs @@ -321,11 +321,11 @@ public interface ProjectConfig Holdout GetHoldout(string holdoutId); /// - /// Get holdout instances associated with the given feature flag key. + /// Get holdout instances associated with the given feature flag Id. /// - /// Feature flag key + /// Feature flag Id /// Array of holdouts associated with the flag, empty array if none - Holdout[] GetHoldoutsForFlag(string flagKey); + Holdout[] GetHoldoutsForFlag(string flagId); /// /// Returns the datafile corresponding to ProjectConfig