Skip to content

Commit 9e087cd

Browse files
mnoman09mnoman09
committed
refactored Code
1 parent acbe4d3 commit 9e087cd

File tree

6 files changed

+107
-188
lines changed

6 files changed

+107
-188
lines changed

‎OptimizelySDK.Tests/OptimizelyUserContextTest.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,7 @@ public void TestGetForcedDecisionReturnsNullIfInvalidConfig()
227227
[Test]
228228
public void TestSetForcedDecisionReturnsFalseForNullConfig()
229229
{
230-
var optly = new Optimizely(new FallbackProjectConfigManager(null));
230+
var optly = new Optimizely(new FallbackProjectConfigManager(null), logger: LoggerMock.Object);
231231

232232
var user = optly.CreateUserContext(UserID);
233233

@@ -236,7 +236,7 @@ public void TestSetForcedDecisionReturnsFalseForNullConfig()
236236
var result = user.SetForcedDecision(context, decision);
237237

238238
Assert.IsFalse(result);
239-
//TODO: should assert logger is called
239+
LoggerMock.Verify(log => log.Log(LogLevel.ERROR, "Optimizely SDK not configured properly yet."), Times.Once);
240240
}
241241

242242
[Test]

‎OptimizelySDK.Tests/ProjectConfigTest.cs

Lines changed: 34 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -427,44 +427,43 @@ public void TestFlagVariations()
427427
{
428428
var allVariations = Config?.FlagVariationMap;
429429

430-
var expectedVariations1 = new List<Dictionary<string, List<Variation>>>();
431-
var expectedVariationList = new List<Variation>
430+
var expectedVariationDict = new Dictionary<string, Variation>
432431
{
433-
new Variation
434-
{
435-
FeatureEnabled = true,
436-
Id = "7722260071",
437-
Key = "group_exp_1_var_1",
438-
FeatureVariableUsageInstances = new List<FeatureVariableUsage> { new FeatureVariableUsage { Id = "155563", Value= "groupie_1_v1" } }
439-
},
440-
new Variation
441-
{
442-
FeatureEnabled = true,
443-
Id = "7722360022",
444-
Key = "group_exp_1_var_2",
445-
FeatureVariableUsageInstances = new List<FeatureVariableUsage> { new FeatureVariableUsage { Id = "155563", Value= "groupie_1_v2" } }
446-
},
447-
new Variation
448-
{
449-
FeatureEnabled = false,
450-
Id = "7713030086",
451-
Key = "group_exp_2_var_1",
452-
FeatureVariableUsageInstances = new List<FeatureVariableUsage> { new FeatureVariableUsage { Id = "155563", Value= "groupie_2_v1" } }
453-
},
454-
new Variation
455-
{
456-
FeatureEnabled = false,
457-
Id = "7725250007",
458-
Key = "group_exp_2_var_2",
459-
FeatureVariableUsageInstances = new List<FeatureVariableUsage> { new FeatureVariableUsage { Id = "155563", Value= "groupie_2_v2" } }
432+
{ "group_exp_1_var_1", new Variation
433+
{
434+
FeatureEnabled = true,
435+
Id = "7722260071",
436+
Key = "group_exp_1_var_1",
437+
FeatureVariableUsageInstances = new List<FeatureVariableUsage> { new FeatureVariableUsage { Id = "155563", Value= "groupie_1_v1" } }
438+
}
439+
},
440+
{ "group_exp_1_var_2", new Variation
441+
{
442+
FeatureEnabled = true,
443+
Id = "7722360022",
444+
Key = "group_exp_1_var_2",
445+
FeatureVariableUsageInstances = new List<FeatureVariableUsage> { new FeatureVariableUsage { Id = "155563", Value= "groupie_1_v2" } }
446+
}
447+
},
448+
{ "group_exp_2_var_1", new Variation
449+
{
450+
FeatureEnabled = false,
451+
Id = "7713030086",
452+
Key = "group_exp_2_var_1",
453+
FeatureVariableUsageInstances = new List<FeatureVariableUsage> { new FeatureVariableUsage { Id = "155563", Value= "groupie_2_v1" } }
454+
}
455+
},
456+
{ "group_exp_2_var_2", new Variation
457+
{
458+
FeatureEnabled = false,
459+
Id = "7725250007",
460+
Key = "group_exp_2_var_2",
461+
FeatureVariableUsageInstances = new List<FeatureVariableUsage> { new FeatureVariableUsage { Id = "155563", Value= "groupie_2_v2" } }
462+
}
460463
}
461464
};
462-
expectedVariations1.Add(new Dictionary<string, List<Variation>> { { "boolean_feature", expectedVariationList } });
463-
var filteredActualVariation = allVariations["boolean_feature"].Select(v => v.Value).ToList().OrderBy(v => v.Id);
464-
//var variations1 = allVariations.Where(v => v.Key == "boolean_feature").Select(v => new Dictionary<string, List<Variation>> { { v.Key, v.Value } }).ToList();
465-
/// TODO: Need to correct it.
466-
//TestData.CompareObjects(expectedVariations1, variations1);
467-
//Assertions.AreEquivalent(expectedVariations1["boolean_featur, filteredActualVariation);
465+
var filteredActualFlagVariations = allVariations["boolean_feature"];
466+
TestData.CompareObjects(expectedVariationDict, filteredActualFlagVariations);
468467
}
469468

470469
[Test]

‎OptimizelySDK/Bucketing/DecisionService.cs

Lines changed: 61 additions & 118 deletions
Original file line numberDiff line numberDiff line change
@@ -449,21 +449,68 @@ public virtual Result<FeatureDecision> GetVariationForFeatureRollout(FeatureFlag
449449
return Result<FeatureDecision>.NullResult(reasons);
450450
}
451451

452+
var userId = user.GetUserId();
453+
var attributes = user.GetAttributes();
454+
452455
var index = 0;
453456
while (index < rolloutRulesLength)
454457
{
455-
/// TODO: add findvalidated forced decision here, no need to add separate function.
456-
var decisionResult = GetVariationFromDeliveryRule(config, featureFlag.Key, rolloutRules, index, user);
457-
reasons += decisionResult.DecisionReasons;
458+
// To skip rules
459+
var skipToEveryoneElse = false;
458460

459-
if (decisionResult.ResultObject?.Variation?.Key != null)
461+
//Check forced decision first
462+
var rule = rolloutRules[index];
463+
var decisionContext = new OptimizelyDecisionContext(featureFlag.Key, rule.Key);
464+
var forcedDecisionResponse = user.FindValidatedForcedDecision(decisionContext, config);
465+
466+
reasons += forcedDecisionResponse.DecisionReasons;
467+
if (forcedDecisionResponse.ResultObject != null)
468+
{
469+
return Result<FeatureDecision>.NewResult(new FeatureDecision(rule, forcedDecisionResponse.ResultObject, null), reasons);
470+
}
471+
472+
// Regular decision
473+
474+
// Get Bucketing ID from user attributes.
475+
var bucketingIdResult = GetBucketingId(userId, attributes);
476+
reasons += bucketingIdResult.DecisionReasons;
477+
478+
var everyoneElse = index == rolloutRulesLength - 1;
479+
480+
var loggingKey = everyoneElse ? "Everyone Else" : string.Format("{0}", index + 1);
481+
482+
// Evaluate if user meets the audience condition of this rollout rule
483+
var doesUserMeetAudienceConditionsResult = ExperimentUtils.DoesUserMeetAudienceConditions(config, rule, attributes, LOGGING_KEY_TYPE_RULE, rule.Key, Logger);
484+
reasons += doesUserMeetAudienceConditionsResult.DecisionReasons;
485+
if (doesUserMeetAudienceConditionsResult.ResultObject)
486+
{
487+
Logger.Log(LogLevel.INFO, reasons.AddInfo($"User \"{userId}\" meets condition for targeting rule \"{loggingKey}\"."));
488+
489+
var bucketedVariation = Bucketer.Bucket(config, rule, bucketingIdResult.ResultObject, userId);
490+
reasons += bucketedVariation?.DecisionReasons;
491+
492+
if (bucketedVariation?.ResultObject?.Key != null)
493+
{
494+
Logger.Log(LogLevel.INFO, reasons.AddInfo($"User \"{userId}\" is in the traffic group of targeting rule \"{loggingKey}\"."));
495+
496+
return Result<FeatureDecision>.NewResult(new FeatureDecision(rule, bucketedVariation.ResultObject, FeatureDecision.DECISION_SOURCE_ROLLOUT), reasons);
497+
}
498+
else if (!everyoneElse)
499+
{
500+
//skip this logging for everyoneElse rule since this has a message not for everyoneElse
501+
Logger.Log(LogLevel.INFO, reasons.AddInfo($"User \"{userId}\" is not in the traffic group for targeting rule \"{loggingKey}\". Checking EveryoneElse rule now."));
502+
skipToEveryoneElse = true;
503+
}
504+
}
505+
else
460506
{
461-
return Result<FeatureDecision>.NewResult(new FeatureDecision(rolloutRules[index], decisionResult.ResultObject.Variation, FeatureDecision.DECISION_SOURCE_ROLLOUT), reasons);
507+
Logger.Log(LogLevel.DEBUG, reasons.AddInfo($"User \"{userId}\" does not meet the conditions for targeting rule \"{loggingKey}\"."));
462508
}
463509

464510
// the last rule is special for "Everyone Else"
465-
index = decisionResult.SkipToEveryoneElse ? (rolloutRulesLength - 1) : (index + 1);
511+
index = skipToEveryoneElse ? (rolloutRulesLength - 1) : (index + 1);
466512
}
513+
467514
return Result<FeatureDecision>.NullResult(reasons);
468515
}
469516

@@ -504,140 +551,36 @@ public virtual Result<FeatureDecision> GetVariationForFeatureExperiment(FeatureF
504551
if (string.IsNullOrEmpty(experiment.Key))
505552
continue;
506553

507-
508554
var forcedDecisionResponse = user.FindValidatedForcedDecision(
509-
new OptimizelyDecisionContext(featureFlag.Key, experiment?.Key), config);
555+
new OptimizelyDecisionContext(featureFlag.Key, experiment?.Key),
556+
config);
510557
reasons += forcedDecisionResponse.DecisionReasons;
511558

512-
if (forcedDecisionResponse?.ResultObject != null) {
559+
if (forcedDecisionResponse?.ResultObject != null)
560+
{
513561
decisionVariation = forcedDecisionResponse.ResultObject;
514-
} else {
562+
}
563+
else
564+
{
515565
var decisionResponse = GetVariation(experiment, user, config, options);
516-
566+
517567
reasons += decisionResponse?.DecisionReasons;
518-
519568
decisionVariation = decisionResponse.ResultObject;
520569
}
521570

522-
523-
524571
if (decisionVariation?.Id != null)
525572
{
526573
Logger.Log(LogLevel.INFO, reasons.AddInfo($"The user \"{userId}\" is bucketed into experiment \"{experiment.Key}\" of feature \"{featureFlag.Key}\"."));
527574

528575
var featureDecision = new FeatureDecision(experiment, decisionVariation, FeatureDecision.DECISION_SOURCE_FEATURE_TEST);
529576
return Result<FeatureDecision>.NewResult(featureDecision, reasons);
530577
}
531-
532578
}
533579

534580
Logger.Log(LogLevel.INFO, reasons.AddInfo($"The user \"{userId}\" is not bucketed into any of the experiments on the feature \"{featureFlag.Key}\"."));
535581
return Result<FeatureDecision>.NullResult(reasons);
536582
}
537583

538-
/// <summary>
539-
/// TODO: Remove this one as well. Keep it simple.
540-
/// </summary>
541-
/// <param name="config"></param>
542-
/// <param name="key"></param>
543-
/// <param name="rules"></param>
544-
/// <param name="ruleIndex"></param>
545-
/// <param name="user"></param>
546-
/// <returns></returns>
547-
private Result<FeatureDecision> GetVariationFromDeliveryRule(ProjectConfig config, string key, List<Experiment> rules, int ruleIndex, OptimizelyUserContext user)
548-
{
549-
var reasons = new DecisionReasons();
550-
551-
bool skipToEveryoneElse = false;
552-
553-
//Check forced decision first
554-
var rule = rules[ruleIndex];
555-
var decisionContext = new OptimizelyDecisionContext(key, rule.Key);
556-
var forcedDecisionResponse = user.FindValidatedForcedDecision(decisionContext, config);
557-
558-
reasons += forcedDecisionResponse.DecisionReasons;
559-
if (forcedDecisionResponse.ResultObject != null)
560-
{
561-
return Result<FeatureDecision>.NewResult(new FeatureDecision(rule, forcedDecisionResponse.ResultObject, null), skipToEveryoneElse, reasons);
562-
}
563-
564-
// Regular decision
565-
var userId = user.GetUserId();
566-
var attributes = user.GetAttributes();
567-
568-
// Get Bucketing ID from user attributes.
569-
var bucketingIdResult = GetBucketingId(userId, attributes);
570-
reasons += bucketingIdResult.DecisionReasons;
571-
572-
var everyoneElse = ruleIndex == rules.Count - 1;
573-
574-
var loggingKey = everyoneElse ? "Everyone Else" : ruleIndex + 1 + "";
575-
576-
Result<Variation> bucketedVariation = null;
577-
578-
// Evaluate if user meets the audience condition of this rollout rule
579-
var doesUserMeetAudienceConditionsResult = ExperimentUtils.DoesUserMeetAudienceConditions(config, rule, attributes, LOGGING_KEY_TYPE_RULE, rule.Key, Logger);
580-
reasons += doesUserMeetAudienceConditionsResult.DecisionReasons;
581-
if (doesUserMeetAudienceConditionsResult.ResultObject)
582-
{
583-
Logger.Log(LogLevel.INFO, reasons.AddInfo($"User \"{userId}\" meets condition for targeting rule \"{loggingKey}\"."));
584-
585-
bucketedVariation = Bucketer.Bucket(config, rule, bucketingIdResult.ResultObject, userId);
586-
reasons += bucketedVariation?.DecisionReasons;
587-
588-
if (bucketedVariation?.ResultObject?.Key != null)
589-
{
590-
Logger.Log(LogLevel.INFO, reasons.AddInfo($"User \"{userId}\" is in the traffic group of targeting rule \"{loggingKey}\"."));
591-
}
592-
else if (!everyoneElse)
593-
{
594-
//skip this loggng for everyoneElse rule since this has a message not for everyoneElse
595-
Logger.Log(LogLevel.INFO, reasons.AddInfo($"User \"{userId}\" is not in the traffic group for targeting rule \"{loggingKey}\". Checking EveryoneElse rule now."));
596-
skipToEveryoneElse = true;
597-
}
598-
}
599-
else
600-
{
601-
Logger.Log(LogLevel.DEBUG, reasons.AddInfo($"User \"{userId}\" does not meet the conditions for targeting rule \"{loggingKey}\"."));
602-
}
603-
604-
return Result<FeatureDecision>.NewResult(new FeatureDecision(rule, bucketedVariation?.ResultObject, null), skipToEveryoneElse, reasons);
605-
}
606-
607-
/// <summary>
608-
/// TODO: Need to remove.
609-
/// </summary>
610-
/// <param name="config"></param>
611-
/// <param name="key"></param>
612-
/// <param name="experiment"></param>
613-
/// <param name="user"></param>
614-
/// <param name="options"></param>
615-
/// <returns></returns>
616-
private Result<Variation> GetVariationFromExperimentRule(ProjectConfig config, string key, Experiment experiment, OptimizelyUserContext user, OptimizelyDecideOption[] options)
617-
{
618-
var reasons = new DecisionReasons();
619-
620-
var decisionContext = new OptimizelyDecisionContext(key, experiment?.Key);
621-
622-
var forcedDecisionResponse = user.FindValidatedForcedDecision(decisionContext, config);
623-
624-
reasons += forcedDecisionResponse.DecisionReasons;
625-
626-
var variation = forcedDecisionResponse?.ResultObject;
627-
628-
if (variation != null)
629-
{
630-
return Result<Variation>.NewResult(variation, reasons);
631-
}
632-
633-
var decisionResponse = GetVariation(experiment, user, config, options);
634-
635-
reasons += decisionResponse?.DecisionReasons;
636-
637-
return Result<Variation>.NewResult(decisionResponse?.ResultObject, reasons);
638-
}
639-
640-
641584
/// <summary>
642585
/// Get the variation the user is bucketed into for the FeatureFlag
643586
/// </summary>

‎OptimizelySDK/Config/DatafileProjectConfig.cs

Lines changed: 9 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -362,41 +362,30 @@ private void Initialize()
362362
foreach (var feature in FeatureFlags)
363363
{
364364
/// TODO: give proper name.
365-
var map = new Dictionary<string, Variation>();
365+
var flagVariationToVariationDict = new Dictionary<string, Variation>();
366366
foreach (var experimentId in feature.ExperimentIds ?? new List<string>())
367367
{
368-
var variationsKeyToVariationsMap = ExperimentIdMap[experimentId].VariationKeyToVariationMap;
369-
370-
foreach (var kv in variationsKeyToVariationsMap) {
371-
map[kv.Key] = kv.Value;
368+
foreach (var variationDictKV in ExperimentIdMap[experimentId].VariationKeyToVariationMap) {
369+
flagVariationToVariationDict[variationDictKV.Key] = variationDictKV.Value;
372370
}
373371

374372
if (ExperimentFeatureMap.ContainsKey(experimentId))
373+
{
375374
ExperimentFeatureMap[experimentId].Add(feature.Id);
375+
}
376376
else
377+
{
377378
ExperimentFeatureMap[experimentId] = new List<string> { feature.Id };
379+
}
378380
}
379381
if (RolloutIdMap.TryGetValue(feature.RolloutId, out var rolloutRules)) {
380382
var rolloutRulesVariations = rolloutRules.Experiments.SelectMany(ex => ex.Variations);
381383
foreach (var rolloutRuleVariation in rolloutRulesVariations) {
382-
map[rolloutRuleVariation.Key] = rolloutRuleVariation;
384+
flagVariationToVariationDict[rolloutRuleVariation.Key] = rolloutRuleVariation;
383385
}
384386
}
385387

386-
387-
flagToVariationsMap[feature.Key] = map;
388-
389-
//// Get the Flag variation map to use
390-
//var variationIdToVariationsDict = new Dictionary<string, Variation>();
391-
//foreach (var variation in from rule in GetAllRulesForFlag(feature)
392-
// from variation in rule.Variations
393-
// where !variationIdToVariationsDict.ContainsKey(variation.Id)
394-
// select variation)
395-
//{
396-
// variationIdToVariationsDict.Add(variation.Id, variation);
397-
//}
398-
//// Grab all the variations from the flag experiments and rollouts and add to flagVariationsMap
399-
//variationsDict[feature.Key] = variationIdToVariationsDict.Values.ToList<Variation>();
388+
flagToVariationsMap[feature.Key] = flagVariationToVariationDict;
400389
}
401390
_FlagVariationMap = flagToVariationsMap;
402391
}

‎OptimizelySDK/Entity/Result.cs

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -21,19 +21,7 @@ public class Result<T>
2121
{
2222
public T ResultObject;
2323
public DecisionReasons DecisionReasons;
24-
public bool SkipToEveryoneElse;
2524

26-
/// <summary>
27-
/// Result object with boolean variable as well
28-
/// </summary>
29-
/// <param name="resultObject"></param>
30-
/// <param name="skipToEveryoneElse"></param>
31-
/// <param name="decisionReasons"></param>
32-
/// <returns></returns>
33-
public static Result<T> NewResult(T resultObject, bool skipToEveryoneElse, DecisionReasons decisionReasons)
34-
{
35-
return new Result<T> { DecisionReasons = decisionReasons, ResultObject = resultObject, SkipToEveryoneElse = skipToEveryoneElse };
36-
}
3725
public static Result<T> NewResult(T resultObject, DecisionReasons decisionReasons)
3826
{
3927
return new Result<T> { DecisionReasons = decisionReasons, ResultObject = resultObject };

0 commit comments

Comments
 (0)