From 3c1d221444a5f2e15a0e59b4ba38e7f71e4e0c23 Mon Sep 17 00:00:00 2001 From: mfahadahmed Date: Tue, 26 Mar 2019 19:32:55 +0500 Subject: [PATCH 1/2] feat(api): Feature variable APIs now return default variable value when featureEnabled property is false. --- OptimizelySDK.Tests/OptimizelyTest.cs | 205 +++++++++++++++++++++++ OptimizelySDK.Tests/ProjectConfigTest.cs | 20 +++ OptimizelySDK.Tests/TestData.json | 59 ++++++- OptimizelySDK/Optimizely.cs | 4 +- 4 files changed, 285 insertions(+), 3 deletions(-) diff --git a/OptimizelySDK.Tests/OptimizelyTest.cs b/OptimizelySDK.Tests/OptimizelyTest.cs index d8abb758..9462ed24 100644 --- a/OptimizelySDK.Tests/OptimizelyTest.cs +++ b/OptimizelySDK.Tests/OptimizelyTest.cs @@ -1306,6 +1306,211 @@ public void TestGetFeatureVariableStringReturnTypecastedValue() Assert.Null(OptimizelyMock.Object.GetFeatureVariableString(featureKey, variableKeyNull, TestUserId, null)); } + #region Feature Toggle Tests + + [Test] + public void TestGetFeatureVariableDoubleReturnsRightValueWhenUserBuckedIntoFeatureExperimentAndVariationIsToggleOn() + { + var featureKey = "double_single_variable_feature"; + var featureFlag = Config.GetFeatureFlagFromKey(featureKey); + var variableKey = "double_variable"; + var expectedValue = 42.42; + var experiment = Config.GetExperimentFromKey("test_experiment_double_feature"); + var variation = Config.GetVariationFromKey("test_experiment_double_feature", "control"); + 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); + + var variableValue = (double)optly.Invoke("GetFeatureVariableDouble", featureKey, variableKey, TestUserId, null); + Assert.AreEqual(expectedValue, variableValue); + } + + [Test] + public void TestGetFeatureVariableIntegerReturnsRightValueWhenUserBuckedIntoFeatureExperimentAndVariationIsToggleOn() + { + var featureKey = "integer_single_variable_feature"; + var featureFlag = Config.GetFeatureFlagFromKey(featureKey); + var variableKey = "integer_variable"; + var expectedValue = 13; + var experiment = Config.GetExperimentFromKey("test_experiment_integer_feature"); + var variation = Config.GetVariationFromKey("test_experiment_integer_feature", "variation"); + var decision = new FeatureDecision(experiment, variation, FeatureDecision.DECISION_SOURCE_EXPERIMENT); + var userAttributes = new UserAttributes + { + { "device_type", "iPhone" }, + { "company", "Optimizely" }, + { "location", "San Francisco" } + }; + + DecisionServiceMock.Setup(ds => ds.GetVariationForFeature(featureFlag, TestUserId, userAttributes)).Returns(decision); + + var optly = Helper.CreatePrivateOptimizely(); + optly.SetFieldOrProperty("DecisionService", DecisionServiceMock.Object); + + var variableValue = (int)optly.Invoke("GetFeatureVariableInteger", featureKey, variableKey, TestUserId, userAttributes); + Assert.AreEqual(expectedValue, variableValue); + } + + [Test] + public void TestGetFeatureVariableDoubleReturnsDefaultValueWhenUserBuckedIntoFeatureExperimentAndVariationIsToggleOff() + { + var featureKey = "double_single_variable_feature"; + var featureFlag = Config.GetFeatureFlagFromKey(featureKey); + var variableKey = "double_variable"; + var expectedValue = 14.99; + var experiment = Config.GetExperimentFromKey("test_experiment_double_feature"); + var variation = Config.GetVariationFromKey("test_experiment_double_feature", "variation"); + 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); + + var variableValue = (double)optly.Invoke("GetFeatureVariableDouble", featureKey, variableKey, TestUserId, null); + Assert.AreEqual(expectedValue, variableValue); + } + + [Test] + public void TestGetFeatureVariableIntegerReturnsDefaultValueWhenUserBuckedIntoFeatureExperimentAndVariationIsToggleOff() + { + var featureKey = "integer_single_variable_feature"; + var featureFlag = Config.GetFeatureFlagFromKey(featureKey); + var variableKey = "integer_variable"; + var expectedValue = 7; + var experiment = Config.GetExperimentFromKey("test_experiment_integer_feature"); + var variation = Config.GetVariationFromKey("test_experiment_integer_feature", "control"); + var decision = new FeatureDecision(experiment, variation, FeatureDecision.DECISION_SOURCE_EXPERIMENT); + var userAttributes = new UserAttributes + { + { "device_type", "iPhone" }, + { "company", "Optimizely" }, + { "location", "San Francisco" } + }; + + DecisionServiceMock.Setup(ds => ds.GetVariationForFeature(featureFlag, TestUserId, userAttributes)).Returns(decision); + + var optly = Helper.CreatePrivateOptimizely(); + optly.SetFieldOrProperty("DecisionService", DecisionServiceMock.Object); + + var variableValue = (int)optly.Invoke("GetFeatureVariableInteger", featureKey, variableKey, TestUserId, userAttributes); + Assert.AreEqual(expectedValue, variableValue); + } + + [Test] + public void TestGetFeatureVariableBooleanReturnsRightValueWhenUserBuckedIntoRolloutAndVariationIsToggleOn() + { + var featureKey = "boolean_single_variable_feature"; + var featureFlag = Config.GetFeatureFlagFromKey(featureKey); + var variableKey = "boolean_variable"; + var expectedValue = true; + var experiment = Config.GetRolloutFromId("166660").Experiments[0]; + var variation = Config.GetVariationFromKey(experiment.Key, "177771"); + var decision = new FeatureDecision(experiment, variation, FeatureDecision.DECISION_SOURCE_ROLLOUT); + + DecisionServiceMock.Setup(ds => ds.GetVariationForFeature(featureFlag, TestUserId, null)).Returns(decision); + + var optly = Helper.CreatePrivateOptimizely(); + optly.SetFieldOrProperty("DecisionService", DecisionServiceMock.Object); + + var variableValue = (bool)optly.Invoke("GetFeatureVariableBoolean", featureKey, variableKey, TestUserId, null); + Assert.AreEqual(expectedValue, variableValue); + } + + [Test] + public void TestGetFeatureVariableStringReturnsRightValueWhenUserBuckedIntoRolloutAndVariationIsToggleOn() + { + var featureKey = "string_single_variable_feature"; + var featureFlag = Config.GetFeatureFlagFromKey(featureKey); + var variableKey = "string_variable"; + var expectedValue = "cta_4"; + var experiment = Config.GetRolloutFromId("166661").Experiments[0]; + var variation = Config.GetVariationFromKey(experiment.Key, "177775"); + var decision = new FeatureDecision(experiment, variation, FeatureDecision.DECISION_SOURCE_ROLLOUT); + var userAttributes = new UserAttributes + { + { "device_type", "iPhone" }, + { "company", "Optimizely" }, + { "location", "San Francisco" } + }; + + DecisionServiceMock.Setup(ds => ds.GetVariationForFeature(featureFlag, TestUserId, userAttributes)).Returns(decision); + + var optly = Helper.CreatePrivateOptimizely(); + optly.SetFieldOrProperty("DecisionService", DecisionServiceMock.Object); + + var variableValue = (string)optly.Invoke("GetFeatureVariableString", featureKey, variableKey, TestUserId, userAttributes); + Assert.AreEqual(expectedValue, variableValue); + } + + [Test] + public void TestGetFeatureVariableBooleanReturnsDefaultValueWhenUserBuckedIntoRolloutAndVariationIsToggleOff() + { + var featureKey = "boolean_single_variable_feature"; + var featureFlag = Config.GetFeatureFlagFromKey(featureKey); + var variableKey = "boolean_variable"; + var expectedValue = true; + var experiment = Config.GetRolloutFromId("166660").Experiments[3]; + var variation = Config.GetVariationFromKey(experiment.Key, "177782"); + var decision = new FeatureDecision(experiment, variation, FeatureDecision.DECISION_SOURCE_ROLLOUT); + + DecisionServiceMock.Setup(ds => ds.GetVariationForFeature(featureFlag, TestUserId, null)).Returns(decision); + + var optly = Helper.CreatePrivateOptimizely(); + optly.SetFieldOrProperty("DecisionService", DecisionServiceMock.Object); + + var variableValue = (bool)optly.Invoke("GetFeatureVariableBoolean", featureKey, variableKey, TestUserId, null); + Assert.AreEqual(expectedValue, variableValue); + } + + [Test] + public void TestGetFeatureVariableStringReturnsDefaultValueWhenUserBuckedIntoRolloutAndVariationIsToggleOff() + { + var featureKey = "string_single_variable_feature"; + var featureFlag = Config.GetFeatureFlagFromKey(featureKey); + var variableKey = "string_variable"; + var expectedValue = "wingardium leviosa"; + var experiment = Config.GetRolloutFromId("166661").Experiments[2]; + var variation = Config.GetVariationFromKey(experiment.Key, "177784"); + var decision = new FeatureDecision(experiment, variation, FeatureDecision.DECISION_SOURCE_ROLLOUT); + var userAttributes = new UserAttributes + { + { "device_type", "iPhone" }, + { "company", "Optimizely" }, + { "location", "San Francisco" } + }; + + DecisionServiceMock.Setup(ds => ds.GetVariationForFeature(featureFlag, TestUserId, userAttributes)).Returns(decision); + + var optly = Helper.CreatePrivateOptimizely(); + optly.SetFieldOrProperty("DecisionService", DecisionServiceMock.Object); + + var variableValue = (string)optly.Invoke("GetFeatureVariableString", featureKey, variableKey, TestUserId, userAttributes); + Assert.AreEqual(expectedValue, variableValue); + } + + [Test] + public void TestGetFeatureVariableDoubleReturnsDefaultValueWhenUserNotBuckedIntoBothFeatureExperimentAndRollout() + { + var featureKey = "double_single_variable_feature"; + var featureFlag = Config.GetFeatureFlagFromKey("double_single_variable_feature"); + var variableKey = "double_variable"; + var expectedValue = 14.99; + + DecisionServiceMock.Setup(ds => ds.GetVariationForFeature(featureFlag, TestUserId, null)).Returns(null); + + var optly = Helper.CreatePrivateOptimizely(); + optly.SetFieldOrProperty("DecisionService", DecisionServiceMock.Object); + + var variableValue = (double)optly.Invoke("GetFeatureVariableDouble", featureKey, variableKey, TestUserId, null); + Assert.AreEqual(expectedValue, variableValue); + } + + #endregion Feature Toggle Tests + #endregion // Test GetFeatureVariable TypeCasting #region Test GetFeatureVariableValueForType method diff --git a/OptimizelySDK.Tests/ProjectConfigTest.cs b/OptimizelySDK.Tests/ProjectConfigTest.cs index 8f0135e2..7d8dfa07 100644 --- a/OptimizelySDK.Tests/ProjectConfigTest.cs +++ b/OptimizelySDK.Tests/ProjectConfigTest.cs @@ -211,6 +211,16 @@ public void TestInit() {"177780", Config.GetVariationFromKey("177779", "177780") } } }, + { "177781", new Dictionary + { + {"177782", Config.GetVariationFromKey("177781", "177782") } + } + }, + { "177783", new Dictionary + { + {"177784", Config.GetVariationFromKey("177783", "177784") } + } + }, { "etag1", new Dictionary { {"vtag1", Config.GetVariationFromKey("etag1", "vtag1") }, @@ -317,6 +327,16 @@ public void TestInit() {"177780", Config.GetVariationFromId("177779", "177780") } } }, + { "177781", new Dictionary + { + {"177782", Config.GetVariationFromId("177781", "177782") } + } + }, + { "177783", new Dictionary + { + {"177784", Config.GetVariationFromId("177783", "177784") } + } + }, { "etag1", new Dictionary { {"276", Config.GetVariationFromId("etag1", "276") }, diff --git a/OptimizelySDK.Tests/TestData.json b/OptimizelySDK.Tests/TestData.json index f7615fbf..932da873 100644 --- a/OptimizelySDK.Tests/TestData.json +++ b/OptimizelySDK.Tests/TestData.json @@ -722,6 +722,32 @@ "endOfRange": 9000 } ] + }, + { + "id": "177781", + "key": "177781", + "status": "Running", + "layerId": "166660", + "audienceIds": [], + "variations": [ + { + "id": "177782", + "key": "177782", + "variables": [ + { + "id": "155556", + "value": "false" + } + ], + "featureEnabled": false + } + ], + "trafficAllocation": [ + { + "entityId": "177778", + "endOfRange": 9000 + } + ] } ] }, @@ -738,7 +764,12 @@ { "id": "177775", "key": "177775", - "variables": [], + "variables": [ + { + "id": "155558", + "value": "cta_4" + } + ], "featureEnabled": true } ], @@ -769,6 +800,32 @@ "endOfRange": 1500 } ] + }, + { + "id": "177783", + "key": "177783", + "status": "Running", + "layerId": "166661", + "audienceIds": [], + "variations": [ + { + "id": "177784", + "key": "177784", + "variables": [ + { + "id": "155558", + "value": "cta_5" + } + ], + "featureEnabled": false + } + ], + "trafficAllocation": [ + { + "entityId": "177780", + "endOfRange": 1500 + } + ] } ] } diff --git a/OptimizelySDK/Optimizely.cs b/OptimizelySDK/Optimizely.cs index d2521da8..28ca5837 100644 --- a/OptimizelySDK/Optimizely.cs +++ b/OptimizelySDK/Optimizely.cs @@ -426,7 +426,7 @@ public virtual string GetFeatureVariableValueForType(string featureKey, string v $@"Variable is of type ""{featureVariable.Type}"", but you requested it as type ""{variableType}""."); return null; } - + var variableValue = featureVariable.DefaultValue; var decision = DecisionService.GetVariationForFeature(featureFlag, userId, userAttributes); @@ -435,7 +435,7 @@ public virtual string GetFeatureVariableValueForType(string featureKey, string v var variation = decision.Variation; var featureVariableUsageInstance = variation.GetFeatureVariableUsageFromId(featureVariable.Id); - if (featureVariableUsageInstance != null) + if (featureVariableUsageInstance != null && variation.FeatureEnabled == true) { variableValue = featureVariableUsageInstance.Value; Logger.Log(LogLevel.INFO, From ce7504e18ade711dc7b6c1802652019819796777 Mon Sep 17 00:00:00 2001 From: mfahadahmed Date: Wed, 27 Mar 2019 12:27:25 +0500 Subject: [PATCH 2/2] Fixed log message. --- OptimizelySDK.Tests/OptimizelyTest.cs | 18 ++++++++++++++++++ OptimizelySDK/Optimizely.cs | 19 ++++++++++++------- 2 files changed, 30 insertions(+), 7 deletions(-) diff --git a/OptimizelySDK.Tests/OptimizelyTest.cs b/OptimizelySDK.Tests/OptimizelyTest.cs index 9462ed24..9d1152ab 100644 --- a/OptimizelySDK.Tests/OptimizelyTest.cs +++ b/OptimizelySDK.Tests/OptimizelyTest.cs @@ -1326,6 +1326,8 @@ public void TestGetFeatureVariableDoubleReturnsRightValueWhenUserBuckedIntoFeatu var variableValue = (double)optly.Invoke("GetFeatureVariableDouble", featureKey, variableKey, TestUserId, null); Assert.AreEqual(expectedValue, variableValue); + + LoggerMock.Verify(l => l.Log(LogLevel.INFO, $@"Returning variable value ""{variableValue}"" for variation ""{variation.Key}"" of feature flag ""{featureKey}"".")); } [Test] @@ -1352,6 +1354,8 @@ public void TestGetFeatureVariableIntegerReturnsRightValueWhenUserBuckedIntoFeat var variableValue = (int)optly.Invoke("GetFeatureVariableInteger", featureKey, variableKey, TestUserId, userAttributes); Assert.AreEqual(expectedValue, variableValue); + + LoggerMock.Verify(l => l.Log(LogLevel.INFO, $@"Returning variable value ""{variableValue}"" for variation ""{variation.Key}"" of feature flag ""{featureKey}"".")); } [Test] @@ -1372,6 +1376,8 @@ public void TestGetFeatureVariableDoubleReturnsDefaultValueWhenUserBuckedIntoFea var variableValue = (double)optly.Invoke("GetFeatureVariableDouble", featureKey, variableKey, TestUserId, null); Assert.AreEqual(expectedValue, variableValue); + + LoggerMock.Verify(l => l.Log(LogLevel.INFO, $@"Feature ""{featureKey}"" is not enabled for user {TestUserId}. Returning default value for variable ""{variableKey}"".")); } [Test] @@ -1398,6 +1404,8 @@ public void TestGetFeatureVariableIntegerReturnsDefaultValueWhenUserBuckedIntoFe var variableValue = (int)optly.Invoke("GetFeatureVariableInteger", featureKey, variableKey, TestUserId, userAttributes); Assert.AreEqual(expectedValue, variableValue); + + LoggerMock.Verify(l => l.Log(LogLevel.INFO, $@"Feature ""{featureKey}"" is not enabled for user {TestUserId}. Returning default value for variable ""{variableKey}"".")); } [Test] @@ -1418,6 +1426,8 @@ public void TestGetFeatureVariableBooleanReturnsRightValueWhenUserBuckedIntoRoll var variableValue = (bool)optly.Invoke("GetFeatureVariableBoolean", featureKey, variableKey, TestUserId, null); Assert.AreEqual(expectedValue, variableValue); + + LoggerMock.Verify(l => l.Log(LogLevel.INFO, $@"Returning variable value ""true"" for variation ""{variation.Key}"" of feature flag ""{featureKey}"".")); } [Test] @@ -1444,6 +1454,8 @@ public void TestGetFeatureVariableStringReturnsRightValueWhenUserBuckedIntoRollo var variableValue = (string)optly.Invoke("GetFeatureVariableString", featureKey, variableKey, TestUserId, userAttributes); Assert.AreEqual(expectedValue, variableValue); + + LoggerMock.Verify(l => l.Log(LogLevel.INFO, $@"Returning variable value ""{variableValue}"" for variation ""{variation.Key}"" of feature flag ""{featureKey}"".")); } [Test] @@ -1464,6 +1476,8 @@ public void TestGetFeatureVariableBooleanReturnsDefaultValueWhenUserBuckedIntoRo var variableValue = (bool)optly.Invoke("GetFeatureVariableBoolean", featureKey, variableKey, TestUserId, null); Assert.AreEqual(expectedValue, variableValue); + + LoggerMock.Verify(l => l.Log(LogLevel.INFO, $@"Feature ""{featureKey}"" is not enabled for user {TestUserId}. Returning default value for variable ""{variableKey}"".")); } [Test] @@ -1490,6 +1504,8 @@ public void TestGetFeatureVariableStringReturnsDefaultValueWhenUserBuckedIntoRol var variableValue = (string)optly.Invoke("GetFeatureVariableString", featureKey, variableKey, TestUserId, userAttributes); Assert.AreEqual(expectedValue, variableValue); + + LoggerMock.Verify(l => l.Log(LogLevel.INFO, $@"Feature ""{featureKey}"" is not enabled for user {TestUserId}. Returning default value for variable ""{variableKey}"".")); } [Test] @@ -1507,6 +1523,8 @@ public void TestGetFeatureVariableDoubleReturnsDefaultValueWhenUserNotBuckedInto var variableValue = (double)optly.Invoke("GetFeatureVariableDouble", featureKey, variableKey, TestUserId, null); Assert.AreEqual(expectedValue, variableValue); + + LoggerMock.Verify(l => l.Log(LogLevel.INFO, $@"User ""{TestUserId}"" is not in any variation for feature flag ""{featureKey}"", returning default value ""{variableValue}"".")); } #endregion Feature Toggle Tests diff --git a/OptimizelySDK/Optimizely.cs b/OptimizelySDK/Optimizely.cs index 28ca5837..9e4c931c 100644 --- a/OptimizelySDK/Optimizely.cs +++ b/OptimizelySDK/Optimizely.cs @@ -435,22 +435,27 @@ public virtual string GetFeatureVariableValueForType(string featureKey, string v var variation = decision.Variation; var featureVariableUsageInstance = variation.GetFeatureVariableUsageFromId(featureVariable.Id); - if (featureVariableUsageInstance != null && variation.FeatureEnabled == true) + if (featureVariableUsageInstance != null) { - variableValue = featureVariableUsageInstance.Value; - Logger.Log(LogLevel.INFO, - $@"Returning variable value ""{variableValue}"" for variation ""{variation.Key}"" of feature flag ""{featureFlag.Key}""."); + if (variation.FeatureEnabled == true) + { + variableValue = featureVariableUsageInstance.Value; + Logger.Log(LogLevel.INFO, $@"Returning variable value ""{variableValue}"" for variation ""{variation.Key}"" of feature flag ""{featureKey}""."); + } + else + { + Logger.Log(LogLevel.INFO, $@"Feature ""{featureKey}"" is not enabled for user {userId}. Returning default value for variable ""{variableKey}""."); + } } else { - Logger.Log(LogLevel.INFO, - $@"Variable ""{variableKey}"" is not used in variation ""{variation.Key}"", returning default value ""{variableValue}""."); + Logger.Log(LogLevel.INFO, $@"Variable ""{variableKey}"" is not used in variation ""{variation.Key}"", returning default value ""{variableValue}""."); } } else { Logger.Log(LogLevel.INFO, - $@"User ""{userId}"" is not in any variation for feature flag ""{featureFlag.Key}"", returning default value ""{variableValue}""."); + $@"User ""{userId}"" is not in any variation for feature flag ""{featureKey}"", returning default value ""{variableValue}""."); } return variableValue;