From 6aa9c295cc751d299fe3c2120019fdb28250abf6 Mon Sep 17 00:00:00 2001 From: Md Junaed Hossain <169046794+junaed-optimizely@users.noreply.github.com> Date: Tue, 26 Aug 2025 22:26:31 +0600 Subject: [PATCH 1/6] [FSSDK-11547] adjustment --- .../DecisionServiceHoldoutTest.cs | 89 +++++++++++++++++++ .../OptimizelyUserContextHoldoutTest.cs | 88 ++++++++++++++++++ OptimizelySDK/Optimizely.cs | 3 +- 3 files changed, 179 insertions(+), 1 deletion(-) diff --git a/OptimizelySDK.Tests/DecisionServiceHoldoutTest.cs b/OptimizelySDK.Tests/DecisionServiceHoldoutTest.cs index a9457cdb..29d229ba 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..9c321659 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/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; From fcbfed61a99cb1ac44bc633e1ed6f0134e547040 Mon Sep 17 00:00:00 2001 From: Md Junaed Hossain <169046794+junaed-optimizely@users.noreply.github.com> Date: Tue, 26 Aug 2025 22:30:11 +0600 Subject: [PATCH 2/6] [FSSDK-11547] region adjustment --- OptimizelySDK.Tests/OptimizelyUserContextHoldoutTest.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/OptimizelySDK.Tests/OptimizelyUserContextHoldoutTest.cs b/OptimizelySDK.Tests/OptimizelyUserContextHoldoutTest.cs index 9c321659..978d207a 100644 --- a/OptimizelySDK.Tests/OptimizelyUserContextHoldoutTest.cs +++ b/OptimizelySDK.Tests/OptimizelyUserContextHoldoutTest.cs @@ -577,7 +577,7 @@ public void TestDecideReasons_HoldoutDecisionContainsRelevantReasons() #endregion - # region Notification test + #region Notification test [Test] public void TestDecide_HoldoutNotificationContent() From 0ae4a65d6a08e90dcad8e803a48c0cc2275677a6 Mon Sep 17 00:00:00 2001 From: Md Junaed Hossain <169046794+junaed-optimizely@users.noreply.github.com> Date: Tue, 26 Aug 2025 22:33:38 +0600 Subject: [PATCH 3/6] [FSSDK-11547] lint fix --- .../DecisionServiceHoldoutTest.cs | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/OptimizelySDK.Tests/DecisionServiceHoldoutTest.cs b/OptimizelySDK.Tests/DecisionServiceHoldoutTest.cs index 29d229ba..3d34e151 100644 --- a/OptimizelySDK.Tests/DecisionServiceHoldoutTest.cs +++ b/OptimizelySDK.Tests/DecisionServiceHoldoutTest.cs @@ -250,7 +250,7 @@ public void TestGetVariationsForFeatureList_Holdout_DecisionReasons() [Test] public void TestImpressionEventForHoldout() { - var featureFlag = Config.FeatureKeyMap["test_flag_1"]; + var featureFlag = Config.FeatureKeyMap["test_flag_1"]; var userAttributes = new UserAttributes(); var eventDispatcher = new Event.Dispatcher.DefaultEventDispatcher(LoggerMock.Object); @@ -271,24 +271,24 @@ public void TestImpressionEventForHoldout() 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, + 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, + 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 && @@ -321,13 +321,13 @@ public void TestImpressionEventForHoldout_DisableDecisionEvent() 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"); } From 728267ec87310be9cc529f2bff4665d75283e480 Mon Sep 17 00:00:00 2001 From: Md Junaed Hossain <169046794+junaed-optimizely@users.noreply.github.com> Date: Thu, 28 Aug 2025 00:34:41 +0600 Subject: [PATCH 4/6] [FSSDK-11547] fsc fix attempt 1 --- OptimizelySDK/Bucketing/DecisionService.cs | 4 ++-- OptimizelySDK/Config/DatafileProjectConfig.cs | 8 ++++---- OptimizelySDK/ProjectConfig.cs | 6 +++--- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/OptimizelySDK/Bucketing/DecisionService.cs b/OptimizelySDK/Bucketing/DecisionService.cs index ad5487a2..5fc0e9d2 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); @@ -945,7 +945,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( 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/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 From e8188940ad8cf17c2c00592c112067daa3dfa5d1 Mon Sep 17 00:00:00 2001 From: Md Junaed Hossain <169046794+junaed-optimizely@users.noreply.github.com> Date: Thu, 28 Aug 2025 19:22:48 +0600 Subject: [PATCH 5/6] [FSSDK-11547] fsc fix attempt 2 --- OptimizelySDK/Bucketing/DecisionService.cs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/OptimizelySDK/Bucketing/DecisionService.cs b/OptimizelySDK/Bucketing/DecisionService.cs index 5fc0e9d2..e179ab83 100644 --- a/OptimizelySDK/Bucketing/DecisionService.cs +++ b/OptimizelySDK/Bucketing/DecisionService.cs @@ -956,10 +956,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.NewResult(null, reasons); } /// /// Finds a validated forced decision. From 5063b233d84837a76f4b6bacf8a5dc0d85fd9354 Mon Sep 17 00:00:00 2001 From: Md Junaed Hossain <169046794+junaed-optimizely@users.noreply.github.com> Date: Thu, 28 Aug 2025 21:41:39 +0600 Subject: [PATCH 6/6] [FSSDK-11547] fsc fix attempt 3 --- OptimizelySDK/Bucketing/DecisionService.cs | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/OptimizelySDK/Bucketing/DecisionService.cs b/OptimizelySDK/Bucketing/DecisionService.cs index e179ab83..7bc8054b 100644 --- a/OptimizelySDK/Bucketing/DecisionService.cs +++ b/OptimizelySDK/Bucketing/DecisionService.cs @@ -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(); @@ -955,8 +949,7 @@ ProjectConfig config } reasons.AddInfo($"User \"{userId}\" is not bucketed into holdout variation \"{holdout.Key}\"."); - - return Result.NewResult(null, reasons); + return Result.NullResult(reasons); } /// /// Finds a validated forced decision.