Skip to content

Commit 7b212f4

Browse files
mfahadahmedmikeproeng37
mfahadahmed
authored and
mikeproeng37
committed
GetFeatureVariableValueForType method now returns correct variable value for rollout rule (#47)
1 parent ca1e7c8 commit 7b212f4

File tree

6 files changed

+56
-31
lines changed

6 files changed

+56
-31
lines changed

‎OptimizelySDK.Tests/DecisionServiceTest.cs‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -501,7 +501,7 @@ public void TestGetVariationForFeatureExperimentGivenNonMutexGroupAndUserIsBucke
501501
{
502502
var experiment = ProjectConfig.GetExperimentFromKey("test_experiment_multivariate");
503503
var variation = ProjectConfig.GetVariationFromId("test_experiment_multivariate", "122231");
504-
var expectedDecision = new FeatureDecision(experiment.Id, variation.Id, FeatureDecision.DECISION_SOURCE_EXPERIMENT);
504+
var expectedDecision = new FeatureDecision(experiment, variation, FeatureDecision.DECISION_SOURCE_EXPERIMENT);
505505
var userAttributes = new UserAttributes();
506506

507507
DecisionServiceMock.Setup(ds => ds.GetVariation(ProjectConfig.GetExperimentFromKey("test_experiment_multivariate"),
@@ -522,7 +522,7 @@ public void TestGetVariationForFeatureExperimentGivenMutexGroupAndUserIsBucketed
522522
var mutexExperiment = ProjectConfig.GetExperimentFromKey("group_experiment_1");
523523
var variation = mutexExperiment.Variations[0];
524524
var userAttributes = new UserAttributes();
525-
var expectedDecision = new FeatureDecision(mutexExperiment.Id, variation.Id, FeatureDecision.DECISION_SOURCE_EXPERIMENT);
525+
var expectedDecision = new FeatureDecision(mutexExperiment, variation, FeatureDecision.DECISION_SOURCE_EXPERIMENT);
526526

527527
DecisionServiceMock.Setup(ds => ds.GetVariation(ProjectConfig.GetExperimentFromKey("group_experiment_1"), "user1",
528528
userAttributes)).Returns(variation);
@@ -585,7 +585,7 @@ public void TestGetVariationForFeatureRolloutWhenUserIsBucketedInTheTargetingRul
585585
var rollout = ProjectConfig.GetRolloutFromId(rolloutId);
586586
var experiment = rollout.Experiments[0];
587587
var variation = experiment.Variations[0];
588-
var expectedDecision = new FeatureDecision(experiment.Id, variation.Id, FeatureDecision.DECISION_SOURCE_ROLLOUT);
588+
var expectedDecision = new FeatureDecision(experiment, variation, FeatureDecision.DECISION_SOURCE_ROLLOUT);
589589

590590
var userAttributes = new UserAttributes {
591591
{ "browser_type", "chrome" }
@@ -613,7 +613,7 @@ public void TestGetVariationForFeatureRolloutWhenUserIsNotBucketedInTheTargeting
613613
var experiment = rollout.Experiments[0];
614614
var everyoneElseRule = rollout.Experiments[rollout.Experiments.Count - 1];
615615
var variation = everyoneElseRule.Variations[0];
616-
var expectedDecision = new FeatureDecision(everyoneElseRule.Id, variation.Id, FeatureDecision.DECISION_SOURCE_ROLLOUT);
616+
var expectedDecision = new FeatureDecision(everyoneElseRule, variation, FeatureDecision.DECISION_SOURCE_ROLLOUT);
617617

618618
var userAttributes = new UserAttributes {
619619
{ "browser_type", "chrome" }
@@ -669,7 +669,7 @@ public void TestGetVariationForFeatureRolloutWhenUserDoesNotQualifyForAnyTargeti
669669
var experiment1 = rollout.Experiments[1];
670670
var everyoneElseRule = rollout.Experiments[rollout.Experiments.Count - 1];
671671
var variation = everyoneElseRule.Variations[0];
672-
var expectedDecision = new FeatureDecision(everyoneElseRule.Id, variation.Id, FeatureDecision.DECISION_SOURCE_ROLLOUT);
672+
var expectedDecision = new FeatureDecision(everyoneElseRule, variation, FeatureDecision.DECISION_SOURCE_ROLLOUT);
673673

674674
BucketerMock.Setup(bm => bm.Bucket(It.IsAny<ProjectConfig>(), everyoneElseRule, It.IsAny<string>(), It.IsAny<string>())).Returns(variation);
675675
var decisionService = new DecisionService(BucketerMock.Object, ErrorHandlerMock.Object, ProjectConfig, null, LoggerMock.Object);
@@ -708,7 +708,7 @@ public void TestGetVariationForFeatureRolloutAudienceAndTrafficeAllocationCheck(
708708
});
709709

710710
// Returned variation id should be '177773' because of audience 'iPhone users in San Francisco'.
711-
var expectedDecision = new FeatureDecision(expWithAudienceiPhoneUsers.Id, varWithAudienceiPhoneUsers.Id, FeatureDecision.DECISION_SOURCE_ROLLOUT);
711+
var expectedDecision = new FeatureDecision(expWithAudienceiPhoneUsers, varWithAudienceiPhoneUsers, FeatureDecision.DECISION_SOURCE_ROLLOUT);
712712
Assert.IsTrue(TestData.CompareObjects(expectedDecision, actualDecision));
713713

714714
// Calling with audience Chrome users.
@@ -718,15 +718,15 @@ public void TestGetVariationForFeatureRolloutAudienceAndTrafficeAllocationCheck(
718718
});
719719

720720
// Returned variation id should be '177771' because of audience 'Chrome users'.
721-
expectedDecision = new FeatureDecision(expWithAudienceChromeUsers.Id, varWithAudienceChromeUsers.Id, FeatureDecision.DECISION_SOURCE_ROLLOUT);
721+
expectedDecision = new FeatureDecision(expWithAudienceChromeUsers, varWithAudienceChromeUsers, FeatureDecision.DECISION_SOURCE_ROLLOUT);
722722
Assert.IsTrue(TestData.CompareObjects(expectedDecision, actualDecision));
723723

724724
// Calling with no audience.
725725
mockBucketer.Setup(bm => bm.GenerateBucketValue(It.IsAny<string>())).Returns(8000);
726726
actualDecision = decisionService.GetVariationForFeatureRollout(featureFlag, GenericUserId, new UserAttributes());
727727

728728
// Returned variation id should be of everyone else rule because of no audience.
729-
expectedDecision = new FeatureDecision(expWithNoAudience.Id, varWithNoAudience.Id, FeatureDecision.DECISION_SOURCE_ROLLOUT);
729+
expectedDecision = new FeatureDecision(expWithNoAudience, varWithNoAudience, FeatureDecision.DECISION_SOURCE_ROLLOUT);
730730
Assert.IsTrue(TestData.CompareObjects(expectedDecision, actualDecision));
731731

732732
// Calling with audience 'Chrome users' and traffice allocation '9500'.
@@ -752,7 +752,7 @@ public void TestGetVariationForFeatureWhenTheUserIsBucketedIntoFeatureExperiment
752752
var expectedExperimentId = featureFlag.ExperimentIds[0];
753753
var expectedExperiment = ProjectConfig.GetExperimentFromId(expectedExperimentId);
754754
var variation = expectedExperiment.Variations[0];
755-
var expectedDecision = new FeatureDecision(expectedExperimentId, variation.Id, FeatureDecision.DECISION_SOURCE_EXPERIMENT);
755+
var expectedDecision = new FeatureDecision(expectedExperiment, variation, FeatureDecision.DECISION_SOURCE_EXPERIMENT);
756756

757757
DecisionServiceMock.Setup(ds => ds.GetVariationForFeatureExperiment(It.IsAny<FeatureFlag>(), It.IsAny<string>(),
758758
It.IsAny<UserAttributes>())).Returns(expectedDecision);
@@ -771,7 +771,7 @@ public void TestGetVariationForFeatureWhenTheUserIsNotBucketedIntoFeatureExperim
771771
var rollout = ProjectConfig.GetRolloutFromId(rolloutId);
772772
var expectedExperiment = rollout.Experiments[0];
773773
var variation = expectedExperiment.Variations[0];
774-
var expectedDecision = new FeatureDecision(expectedExperiment.Id, variation.Id, FeatureDecision.DECISION_SOURCE_ROLLOUT);
774+
var expectedDecision = new FeatureDecision(expectedExperiment, variation, FeatureDecision.DECISION_SOURCE_ROLLOUT);
775775

776776
DecisionServiceMock.Setup(ds => ds.GetVariationForFeatureExperiment(It.IsAny<FeatureFlag>(), It.IsAny<string>(),
777777
It.IsAny<UserAttributes>())).Returns<Variation>(null);
@@ -808,7 +808,7 @@ public void TestGetVariationForFeatureWhenTheUserIsBuckedtedInBothExperimentAndR
808808
var featureFlag = ProjectConfig.GetFeatureFlagFromKey("string_single_variable_feature");
809809
var experiment = ProjectConfig.GetExperimentFromKey("test_experiment_with_feature_rollout");
810810
var variation = ProjectConfig.GetVariationFromId("test_experiment_with_feature_rollout", "122236");
811-
var expectedDecision = new FeatureDecision(experiment.Id, variation.Id, FeatureDecision.DECISION_SOURCE_EXPERIMENT);
811+
var expectedDecision = new FeatureDecision(experiment, variation, FeatureDecision.DECISION_SOURCE_EXPERIMENT);
812812
var userAttributes = new UserAttributes {
813813
{ "browser_type", "chrome" }
814814
};
@@ -822,7 +822,7 @@ public void TestGetVariationForFeatureWhenTheUserIsBuckedtedInBothExperimentAndR
822822
var rollout = ProjectConfig.GetRolloutFromId(featureFlag.RolloutId);
823823
var rolloutExperiment = rollout.Experiments[0];
824824
var rolloutVariation = rolloutExperiment.Variations[0];
825-
var expectedRolloutDecision = new FeatureDecision(rolloutExperiment.Id, rolloutVariation.Id, FeatureDecision.DECISION_SOURCE_ROLLOUT);
825+
var expectedRolloutDecision = new FeatureDecision(rolloutExperiment, rolloutVariation, FeatureDecision.DECISION_SOURCE_ROLLOUT);
826826

827827
BucketerMock.Setup(bm => bm.Bucket(ProjectConfig, rolloutExperiment, It.IsAny<string>(), It.IsAny<string>())).Returns(rolloutVariation);
828828
var actualRolloutDecision = DecisionServiceMock.Object.GetVariationForFeatureRollout(featureFlag, "user1", userAttributes);

‎OptimizelySDK.Tests/OptimizelyTest.cs‎

Lines changed: 31 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1346,7 +1346,7 @@ public void TestGetFeatureVariableValueForTypeGivenFeatureFlagIsEnabledForUserAn
13461346
var featureFlag = Config.GetFeatureFlagFromKey("double_single_variable_feature");
13471347
var experiment = Config.GetExperimentFromKey("test_experiment_integer_feature");
13481348
var differentVariation = Config.GetVariationFromKey("test_experiment_integer_feature", "control");
1349-
var expectedDecision = new FeatureDecision(experiment.Id, differentVariation.Id, FeatureDecision.DECISION_SOURCE_EXPERIMENT);
1349+
var expectedDecision = new FeatureDecision(experiment, differentVariation, FeatureDecision.DECISION_SOURCE_EXPERIMENT);
13501350
var variableKey = "double_variable";
13511351
var variableType = FeatureVariable.VariableType.DOUBLE;
13521352
var expectedValue = "14.99";
@@ -1376,7 +1376,7 @@ public void TestGetFeatureVariableValueForTypeGivenFeatureFlagIsEnabledForUserAn
13761376
var expectedValue = "42.42";
13771377
var experiment = Config.GetExperimentFromKey("test_experiment_double_feature");
13781378
var variation = Config.GetVariationFromKey("test_experiment_double_feature", "control");
1379-
var decision = new FeatureDecision(experiment.Id, variation.Id, FeatureDecision.DECISION_SOURCE_EXPERIMENT);
1379+
var decision = new FeatureDecision(experiment, variation, FeatureDecision.DECISION_SOURCE_EXPERIMENT);
13801380

13811381
DecisionServiceMock.Setup(ds => ds.GetVariationForFeature(featureFlag, TestUserId, null)).Returns(decision);
13821382

@@ -1390,6 +1390,32 @@ public void TestGetFeatureVariableValueForTypeGivenFeatureFlagIsEnabledForUserAn
13901390
$@"Returning variable value ""{variableValue}"" for variation ""{variation.Key}"" of feature flag ""{featureKey}""."));
13911391
}
13921392

1393+
// Verify that GetFeatureVariableValueForType returns correct variable value for rollout rule.
1394+
[Test]
1395+
public void TestGetFeatureVariableValueForTypeWithRolloutRule()
1396+
{
1397+
var featureKey = "boolean_single_variable_feature";
1398+
var featureFlag = Config.GetFeatureFlagFromKey(featureKey);
1399+
var variableKey = "boolean_variable";
1400+
//experimentid - 177772
1401+
var experiment = Config.Rollouts[0].Experiments[1];
1402+
var variation = Config.GetVariationFromId(experiment.Key, "177773");
1403+
var decision = new FeatureDecision(experiment, variation, FeatureDecision.DECISION_SOURCE_ROLLOUT);
1404+
var expectedVariableValue = false;
1405+
1406+
DecisionServiceMock.Setup(ds => ds.GetVariationForFeature(featureFlag, TestUserId, null)).Returns(decision);
1407+
1408+
var optly = Helper.CreatePrivateOptimizely();
1409+
optly.SetFieldOrProperty("DecisionService", DecisionServiceMock.Object);
1410+
1411+
// Calling GetFeatureVariableBoolean to get GetFeatureVariableValueForType returned value casted in bool.
1412+
var actualVariableValue = (bool?)optly.Invoke("GetFeatureVariableBoolean", featureKey, variableKey, TestUserId, null);
1413+
1414+
// Verify that variable value 'false' has been returned from GetFeatureVariableValueForType as it is the value
1415+
// stored in rollout rule '177772'.
1416+
Assert.AreEqual(expectedVariableValue, actualVariableValue);
1417+
}
1418+
13931419
#endregion // Test GetFeatureVariableValueForType method
13941420

13951421
#region Test IsFeatureEnabled method
@@ -1468,7 +1494,7 @@ public void TestIsFeatureEnabledGivenFeatureFlagIsEnabledAndUserIsNotBeingExperi
14681494
var experiment = rollout.Experiments[0];
14691495
var variation = experiment.Variations[0];
14701496
var featureFlag = Config.GetFeatureFlagFromKey(featureKey);
1471-
var decision = new FeatureDecision(experiment.Id, variation.Id, FeatureDecision.DECISION_SOURCE_ROLLOUT);
1497+
var decision = new FeatureDecision(experiment, variation, FeatureDecision.DECISION_SOURCE_ROLLOUT);
14721498

14731499
DecisionServiceMock.Setup(ds => ds.GetVariationForFeature(featureFlag, TestUserId, null)).Returns(decision);
14741500

@@ -1495,7 +1521,7 @@ public void TestIsFeatureEnabledGivenFeatureFlagIsEnabledAndUserIsBeingExperimen
14951521
var experiment = Config.GetExperimentFromKey("test_experiment_double_feature");
14961522
var variation = Config.GetVariationFromKey("test_experiment_double_feature", "control");
14971523
var featureFlag = Config.GetFeatureFlagFromKey(featureKey);
1498-
var decision = new FeatureDecision(experiment.Id, variation.Id, FeatureDecision.DECISION_SOURCE_EXPERIMENT);
1524+
var decision = new FeatureDecision(experiment, variation, FeatureDecision.DECISION_SOURCE_EXPERIMENT);
14991525

15001526
DecisionServiceMock.Setup(ds => ds.GetVariationForFeature(featureFlag, TestUserId, null)).Returns(decision);
15011527

@@ -1567,7 +1593,7 @@ public void TestActivateListener(UserAttributes userAttributes)
15671593
var experiment = Config.GetExperimentFromKey(experimentKey);
15681594
var variation = Config.GetVariationFromKey(experimentKey, variationKey);
15691595
var featureFlag = Config.GetFeatureFlagFromKey(featureKey);
1570-
var decision = new FeatureDecision(experiment.Id, variation.Id, FeatureDecision.DECISION_SOURCE_EXPERIMENT);
1596+
var decision = new FeatureDecision(experiment, variation, FeatureDecision.DECISION_SOURCE_EXPERIMENT);
15711597
var logEvent = new LogEvent("https://logx.optimizely.com/v1/events", OptimizelyHelper.SingleParameter,
15721598
"POST", new Dictionary<string, string> { });
15731599

‎OptimizelySDK/Bucketing/DecisionService.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -302,7 +302,7 @@ public virtual FeatureDecision GetVariationForFeatureRollout(FeatureFlag feature
302302
break;
303303
}
304304

305-
return new FeatureDecision(rolloutRule.Id, variation.Id, FeatureDecision.DECISION_SOURCE_ROLLOUT);
305+
return new FeatureDecision(rolloutRule, variation, FeatureDecision.DECISION_SOURCE_ROLLOUT);
306306
}
307307
else
308308
{
@@ -315,7 +315,7 @@ public virtual FeatureDecision GetVariationForFeatureRollout(FeatureFlag feature
315315
variation = Bucketer.Bucket(ProjectConfig, everyoneElseRolloutRule, bucketingId, userId);
316316

317317
if (variation != null && !string.IsNullOrEmpty(variation.Id))
318-
return new FeatureDecision(everyoneElseRolloutRule.Id, variation.Id, FeatureDecision.DECISION_SOURCE_ROLLOUT);
318+
return new FeatureDecision(everyoneElseRolloutRule, variation, FeatureDecision.DECISION_SOURCE_ROLLOUT);
319319

320320
Logger.Log(LogLevel.DEBUG, $"User \"{userId}\" is excluded from \"Everyone Else\" rule for feature flag \"{featureFlag.Key}\".");
321321
return null;
@@ -355,7 +355,7 @@ public virtual FeatureDecision GetVariationForFeatureExperiment(FeatureFlag feat
355355
if (variation != null && !string.IsNullOrEmpty(variation.Id))
356356
{
357357
Logger.Log(LogLevel.INFO, $"The user \"{userId}\" is bucketed into experiment \"{experiment.Key}\" of feature \"{featureFlag.Key}\".");
358-
return new FeatureDecision(experimentId, variation.Id, FeatureDecision.DECISION_SOURCE_EXPERIMENT);
358+
return new FeatureDecision(experiment, variation, FeatureDecision.DECISION_SOURCE_EXPERIMENT);
359359
}
360360
}
361361

‎OptimizelySDK/Entity/FeatureDecision.cs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,14 +24,14 @@ public class FeatureDecision
2424
public const string DECISION_SOURCE_EXPERIMENT = "experiment";
2525
public const string DECISION_SOURCE_ROLLOUT = "rollout";
2626

27-
public string ExperimentId { get; }
28-
public string VariationId { get; }
27+
public Experiment Experiment { get; }
28+
public Variation Variation { get; }
2929
public string Source { get; }
3030

31-
public FeatureDecision(string experimentId, string variationId, string source)
31+
public FeatureDecision(Experiment experiment, Variation variation, string source)
3232
{
33-
ExperimentId = experimentId;
34-
VariationId = variationId;
33+
Experiment = experiment;
34+
Variation = variation;
3535
Source = source;
3636
}
3737
}

‎OptimizelySDK/Optimizely.cs‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -363,9 +363,8 @@ public virtual bool IsFeatureEnabled(string featureKey, string userId, UserAttri
363363

364364
if (decision.Source == FeatureDecision.DECISION_SOURCE_EXPERIMENT)
365365
{
366-
var experiment = Config.GetExperimentFromId(decision.ExperimentId);
367-
var variation = Config.GetVariationFromId(experiment.Key, decision.VariationId);
368-
SendImpressionEvent(experiment, variation, userId, userAttributes);
366+
367+
SendImpressionEvent(decision.Experiment, decision.Variation, userId, userAttributes);
369368
}
370369
else
371370
{
@@ -429,8 +428,7 @@ public virtual string GetFeatureVariableValueForType(string featureKey, string v
429428

430429
if (decision != null)
431430
{
432-
var experiment = Config.GetExperimentFromId(decision.ExperimentId);
433-
var variation = Config.GetVariationFromId(experiment.Key, decision.VariationId);
431+
var variation = decision.Variation;
434432
var featureVariableUsageInstance = variation.GetFeatureVariableUsageFromId(featureVariable.Id);
435433

436434
if (featureVariableUsageInstance != null)

‎OptimizelySDK/ProjectConfig.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,7 @@ private Dictionary<string, Dictionary<string, Variation>> _VariationIdMap
125125
/// </summary>
126126
private Dictionary<string, Rollout> _RolloutIdMap;
127127
public Dictionary<string, Rollout> RolloutIdMap { get { return _RolloutIdMap; } }
128+
128129

129130
//========================= Callbacks ===========================
130131

@@ -247,7 +248,7 @@ private void Initialize()
247248
{
248249
_VariationKeyMap[rolloutRule.Key] = new Dictionary<string, Variation>();
249250
_VariationIdMap[rolloutRule.Key] = new Dictionary<string, Variation>();
250-
251+
251252
if (rolloutRule.Variations != null)
252253
{
253254
foreach (var variation in rolloutRule.Variations)

0 commit comments

Comments
 (0)