Skip to content

Commit d359a0f

Browse files
mnoman09msohailhussain
mnoman09
and
msohailhussain
authored
Refact: Refactored Forced decision (#287)
* Refactored and resolved comments given on PR * some suggested code. * unit test fixes. * typo fix * Removed getVariationFromExperiment as it was extra loop fixed some bugs and other minor issue due to which tests were failing * Added forcedDecisionStore class * more refactoring. * refactored Code * refactored and removed commented code * Added forcedDecisionStoreTests * conditional copy. * Fixed soure rollout issue * comment fixed. Co-authored-by: Sohail Hussain <[email protected]>
1 parent 6478ed6 commit d359a0f

21 files changed

+471
-336
lines changed

‎OptimizelySDK.Net35/OptimizelySDK.Net35.csproj‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -322,6 +322,9 @@
322322
</Compile>
323323
<Compile Include="..\OptimizelySDK\OptimizelyDecisionContext.cs">
324324
<Link>OptimizelyDecisionContext.cs</Link>
325+
</Compile>
326+
<Compile Include="..\OptimizelySDK\ForcedDecisionsStore.cs">
327+
<Link>ForcedDecisionsStore.cs</Link>
325328
</Compile>
326329
<Compile Include="..\OptimizelySDK\OptimizelyForcedDecision.cs">
327330
<Link>OptimizelyForcedDecision.cs</Link>

‎OptimizelySDK.Net40/OptimizelySDK.Net40.csproj‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -345,6 +345,9 @@
345345
</Compile>
346346
<Compile Include="..\OptimizelySDK\OptimizelyDecisionContext.cs">
347347
<Link>OptimizelyDecisionContext.cs</Link>
348+
</Compile>
349+
<Compile Include="..\OptimizelySDK\ForcedDecisionsStore.cs">
350+
<Link>ForcedDecisionsStore.cs</Link>
348351
</Compile>
349352
<Compile Include="..\OptimizelySDK\OptimizelyForcedDecision.cs">
350353
<Link>OptimizelyForcedDecision.cs</Link>

‎OptimizelySDK.NetStandard16/OptimizelySDK.NetStandard16.csproj‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@
9696
<Compile Include="..\OptimizelySDK\OptimizelyUserContext.cs" />
9797
<Compile Include="..\OptimizelySDK\Config\FallbackProjectConfigManager.cs" />
9898
<Compile Include="..\OptimizelySDK\OptimizelyDecisionContext.cs" />
99+
<Compile Include="..\OptimizelySDK\ForcedDecisionsStore.cs" />
99100
<Compile Include="..\OptimizelySDK\OptimizelyForcedDecision.cs">
100101
<Link>OptimizelyForcedDecision.cs</Link>
101102
</Compile>

‎OptimizelySDK.NetStandard20/OptimizelySDK.NetStandard20.csproj‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -307,6 +307,9 @@
307307
<Compile Include="..\OptimizelySDK\OptimizelyDecisionContext.cs">
308308
<Link>OptimizelyDecisionContext.cs</Link>
309309
</Compile>
310+
<Compile Include="..\OptimizelySDK\ForcedDecisionsStore.cs">
311+
<Link>ForcedDecisionsStore.cs</Link>
312+
</Compile>
310313
<Compile Include="..\OptimizelySDK\OptimizelyForcedDecision.cs">
311314
<Link>OptimizelyForcedDecision.cs</Link>
312315
</Compile>

‎OptimizelySDK.Tests/ConfigTest/AtomicProjectConfigManagerTest.cs‎ renamed to ‎OptimizelySDK.Tests/ConfigTest/FallbackProjectConfigManagerTest.cs‎

File renamed without changes.

‎OptimizelySDK.Tests/DecisionServiceTest.cs‎

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -82,10 +82,10 @@ public void TestGetVariationForcedVariationPrecedesAudienceEval()
8282
OptimizelyUserContextMock = new Mock<OptimizelyUserContext>(optlyObject, WhitelistedUserId, new UserAttributes(), ErrorHandlerMock.Object, LoggerMock.Object);
8383
OptimizelyUserContextMock.Setup(ouc => ouc.GetUserId()).Returns(GenericUserId);
8484
// user excluded without audiences and whitelisting
85-
Assert.IsNull(decisionService.GetVariation(experiment, OptimizelyUserContextMock.Object, ProjectConfig, new UserAttributes()).ResultObject);
85+
Assert.IsNull(decisionService.GetVariation(experiment, OptimizelyUserContextMock.Object, ProjectConfig).ResultObject);
8686

8787
OptimizelyUserContextMock.Setup(ouc => ouc.GetUserId()).Returns(WhitelistedUserId);
88-
var actualVariation = decisionService.GetVariation(experiment, OptimizelyUserContextMock.Object, ProjectConfig, new UserAttributes());
88+
var actualVariation = decisionService.GetVariation(experiment, OptimizelyUserContextMock.Object, ProjectConfig);
8989

9090
LoggerMock.Verify(l => l.Log(LogLevel.INFO, string.Format("User \"{0}\" is forced in variation \"vtag5\".", WhitelistedUserId)), Times.Once);
9191

@@ -110,7 +110,7 @@ public void TestGetVariationLogsErrorWhenUserProfileMapItsNull()
110110
var options = new OptimizelyDecideOption[] { OptimizelyDecideOption.INCLUDE_REASONS };
111111
OptimizelyUserContextMock.Setup(ouc => ouc.GetUserId()).Returns(GenericUserId);
112112

113-
var variationResult = decisionService.GetVariation(experiment, OptimizelyUserContextMock.Object, ProjectConfig, new UserAttributes(), options);
113+
var variationResult = decisionService.GetVariation(experiment, OptimizelyUserContextMock.Object, ProjectConfig, options);
114114
Assert.AreEqual(variationResult.DecisionReasons.ToReport(true)[0], "We were unable to get a user profile map from the UserProfileService.");
115115
Assert.AreEqual(variationResult.DecisionReasons.ToReport(true)[1], "Audiences for experiment \"etag3\" collectively evaluated to FALSE");
116116
Assert.AreEqual(variationResult.DecisionReasons.ToReport(true)[2], "User \"genericUserId\" does not meet conditions to be in experiment \"etag3\".");
@@ -136,15 +136,15 @@ public void TestGetVariationEvaluatesUserProfileBeforeAudienceTargeting()
136136

137137
DecisionService decisionService = new DecisionService(BucketerMock.Object, ErrorHandlerMock.Object, UserProfileServiceMock.Object, LoggerMock.Object);
138138

139-
decisionService.GetVariation(experiment, OptimizelyUserContextMock.Object, ProjectConfig, new UserAttributes());
139+
decisionService.GetVariation(experiment, OptimizelyUserContextMock.Object, ProjectConfig);
140140

141141
LoggerMock.Verify(l => l.Log(LogLevel.INFO, string.Format("User \"{0}\" does not meet conditions to be in experiment \"{1}\".",
142142
GenericUserId, experiment.Key)), Times.Once);
143143

144144
OptimizelyUserContextMock.Setup(ouc => ouc.GetUserId()).Returns(UserProfileId);
145145

146146
// ensure that a user with a saved user profile, sees the same variation regardless of audience evaluation
147-
decisionService.GetVariation(experiment, OptimizelyUserContextMock.Object, ProjectConfig, new UserAttributes());
147+
decisionService.GetVariation(experiment, OptimizelyUserContextMock.Object, ProjectConfig);
148148

149149
BucketerMock.Verify(_ => _.Bucket(It.IsAny<ProjectConfig>(), It.IsAny<Experiment>(), It.IsAny<string>(), It.IsAny<string>()), Times.Never);
150150
}
@@ -234,7 +234,7 @@ public void TestBucketReturnsVariationStoredInUserProfile()
234234
OptimizelyUserContextMock = new Mock<OptimizelyUserContext>(optlyObject, WhitelistedUserId, new UserAttributes(), ErrorHandlerMock.Object, LoggerMock.Object);
235235
OptimizelyUserContextMock.Setup(ouc => ouc.GetUserId()).Returns(UserProfileId);
236236

237-
var actualVariation = decisionService.GetVariation(experiment, OptimizelyUserContextMock.Object, ProjectConfig, new UserAttributes());
237+
var actualVariation = decisionService.GetVariation(experiment, OptimizelyUserContextMock.Object, ProjectConfig);
238238

239239
Assertions.AreEqual(variation, actualVariation.ResultObject);
240240

@@ -316,7 +316,7 @@ public void TestGetVariationSavesBucketedVariationIntoUserProfile()
316316

317317
OptimizelyUserContextMock.Setup(ouc => ouc.GetUserId()).Returns(UserProfileId);
318318

319-
Assert.IsTrue(TestData.CompareObjects(variation.ResultObject, decisionService.GetVariation(experiment, OptimizelyUserContextMock.Object, ProjectConfig, new UserAttributes()).ResultObject));
319+
Assert.IsTrue(TestData.CompareObjects(variation.ResultObject, decisionService.GetVariation(experiment, OptimizelyUserContextMock.Object, ProjectConfig).ResultObject));
320320

321321
LoggerMock.Verify(l => l.Log(LogLevel.INFO, string.Format("Saved variation \"{0}\" of experiment \"{1}\" for user \"{2}\".", variation.ResultObject.Id,
322322
experiment.Id, UserProfileId)), Times.Once);
@@ -374,7 +374,7 @@ public void TestGetVariationSavesANewUserProfile()
374374
UserProfileServiceMock.Object, LoggerMock.Object);
375375
OptimizelyUserContextMock.Setup(ouc => ouc.GetUserId()).Returns(UserProfileId);
376376

377-
var actualVariation = decisionService.GetVariation(experiment, OptimizelyUserContextMock.Object, ProjectConfig, new UserAttributes());
377+
var actualVariation = decisionService.GetVariation(experiment, OptimizelyUserContextMock.Object, ProjectConfig);
378378

379379
Assertions.AreEqual(variation.ResultObject, actualVariation.ResultObject);
380380

@@ -578,12 +578,12 @@ public void TestGetVariationForFeatureExperimentGivenNonMutexGroupAndUserIsBucke
578578

579579
var optlyObject = new Optimizely(TestData.Datafile, new ValidEventDispatcher(), LoggerMock.Object);
580580

581-
OptimizelyUserContextMock = new Mock<OptimizelyUserContext>(optlyObject, WhitelistedUserId, new UserAttributes(), ErrorHandlerMock.Object, LoggerMock.Object);
581+
OptimizelyUserContextMock = new Mock<OptimizelyUserContext>(optlyObject, WhitelistedUserId, userAttributes, ErrorHandlerMock.Object, LoggerMock.Object);
582582

583583
OptimizelyUserContextMock.Setup(ouc => ouc.GetUserId()).Returns("user1");
584584

585585
DecisionServiceMock.Setup(ds => ds.GetVariation(ProjectConfig.GetExperimentFromKey("test_experiment_multivariate"),
586-
OptimizelyUserContextMock.Object, ProjectConfig, userAttributes, It.IsAny<OptimizelyDecideOption[]>())).Returns(variation);
586+
OptimizelyUserContextMock.Object, ProjectConfig, It.IsAny<OptimizelyDecideOption[]>())).Returns(variation);
587587

588588
var featureFlag = ProjectConfig.GetFeatureFlagFromKey("multi_variate_feature");
589589
var decision = DecisionServiceMock.Object.GetVariationForFeatureExperiment(featureFlag, OptimizelyUserContextMock.Object, userAttributes, ProjectConfig, new OptimizelyDecideOption[] { });
@@ -604,11 +604,10 @@ public void TestGetVariationForFeatureExperimentGivenMutexGroupAndUserIsBucketed
604604

605605
var optlyObject = new Optimizely(TestData.Datafile, new ValidEventDispatcher(), LoggerMock.Object);
606606

607-
OptimizelyUserContextMock = new Mock<OptimizelyUserContext>(optlyObject, WhitelistedUserId, new UserAttributes(), ErrorHandlerMock.Object, LoggerMock.Object);
607+
OptimizelyUserContextMock = new Mock<OptimizelyUserContext>(optlyObject, WhitelistedUserId, userAttributes, ErrorHandlerMock.Object, LoggerMock.Object);
608608
OptimizelyUserContextMock.Setup(ouc => ouc.GetUserId()).Returns("user1");
609609

610-
DecisionServiceMock.Setup(ds => ds.GetVariation(ProjectConfig.GetExperimentFromKey("group_experiment_1"), OptimizelyUserContextMock.Object, ProjectConfig,
611-
userAttributes)).Returns(variation);
610+
DecisionServiceMock.Setup(ds => ds.GetVariation(ProjectConfig.GetExperimentFromKey("group_experiment_1"), OptimizelyUserContextMock.Object, ProjectConfig)).Returns(variation);
612611

613612
var featureFlag = ProjectConfig.GetFeatureFlagFromKey("boolean_feature");
614613
var actualDecision = DecisionServiceMock.Object.GetVariationForFeatureExperiment(featureFlag, OptimizelyUserContextMock.Object, userAttributes, ProjectConfig, new OptimizelyDecideOption[] { });
@@ -625,7 +624,7 @@ public void TestGetVariationForFeatureExperimentGivenMutexGroupAndUserNotBuckete
625624
var mutexExperiment = ProjectConfig.GetExperimentFromKey("group_experiment_1");
626625
OptimizelyUserContextMock.Setup(ouc => ouc.GetUserId()).Returns("user1");
627626

628-
DecisionServiceMock.Setup(ds => ds.GetVariation(It.IsAny<Experiment>(), It.IsAny<OptimizelyUserContext>(), ProjectConfig, It.IsAny<UserAttributes>(), It.IsAny<OptimizelyDecideOption[]>()))
627+
DecisionServiceMock.Setup(ds => ds.GetVariation(It.IsAny<Experiment>(), It.IsAny<OptimizelyUserContext>(), ProjectConfig, It.IsAny<OptimizelyDecideOption[]>()))
629628
.Returns(Result<Variation>.NullResult(null));
630629

631630
var featureFlag = ProjectConfig.GetFeatureFlagFromKey("boolean_feature");
@@ -975,7 +974,7 @@ public void TestGetVariationForFeatureWhenTheUserIsBuckedtedInBothExperimentAndR
975974
OptimizelyUserContextMock = new Mock<OptimizelyUserContext>(optlyObject, WhitelistedUserId, userAttributes, ErrorHandlerMock.Object, LoggerMock.Object);
976975
OptimizelyUserContextMock.Setup(ouc => ouc.GetUserId()).Returns(UserProfileId);
977976

978-
DecisionServiceMock.Setup(ds => ds.GetVariation(experiment, OptimizelyUserContextMock.Object, ProjectConfig, userAttributes, It.IsAny<OptimizelyDecideOption[]>())).Returns(variation);
977+
DecisionServiceMock.Setup(ds => ds.GetVariation(experiment, OptimizelyUserContextMock.Object, ProjectConfig, It.IsAny<OptimizelyDecideOption[]>())).Returns(variation);
979978
var actualDecision = DecisionServiceMock.Object.GetVariationForFeatureExperiment(featureFlag, OptimizelyUserContextMock.Object, userAttributes, ProjectConfig, new OptimizelyDecideOption[] { });
980979

981980
// The user is bucketed into feature experiment's variation.

‎OptimizelySDK.Tests/ForcedDecisionsStoreTest.cs‎

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
/**
2+
*
3+
* Copyright 2021, Optimizely and contributors
4+
*
5+
* Licensed under the Apache License, Version 2.0 (the "License");
6+
* you may not use this file except in compliance with the License.
7+
* You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
18+
using NUnit.Framework;
19+
20+
namespace OptimizelySDK.Tests
21+
{
22+
[TestFixture]
23+
public class ForcedDecisionsStoreTest
24+
{
25+
[Test]
26+
public void ForcedDecisionStoreGetSetForcedDecisionWithBothRuleAndFlagKey()
27+
{
28+
var expectedForcedDecision1 = new OptimizelyForcedDecision("sample_variation_key");
29+
var expectedForcedDecision2 = new OptimizelyForcedDecision("sample_variation_key_2");
30+
var context1 = new OptimizelyDecisionContext("flag_key", "rule_key");
31+
var context2 = new OptimizelyDecisionContext("flag_key", "rule_key1");
32+
var forcedDecisionStore = new ForcedDecisionsStore();
33+
forcedDecisionStore[context1] = expectedForcedDecision1;
34+
forcedDecisionStore[context2] = expectedForcedDecision2;
35+
36+
Assert.AreEqual(forcedDecisionStore.Count, 2);
37+
Assert.AreEqual(forcedDecisionStore[context1].VariationKey, expectedForcedDecision1.VariationKey);
38+
Assert.AreEqual(forcedDecisionStore[context2].VariationKey, expectedForcedDecision2.VariationKey);
39+
}
40+
41+
[Test]
42+
public void ForcedDecisionStoreNullFlagKeyForcedDecisionContext()
43+
{
44+
var expectedForcedDecision = new OptimizelyForcedDecision("sample_variation_key");
45+
var context = new OptimizelyDecisionContext(null, "rule_key");
46+
var forcedDecisionStore = new ForcedDecisionsStore();
47+
forcedDecisionStore[context] = expectedForcedDecision;
48+
49+
Assert.AreEqual(forcedDecisionStore.Count, 0);
50+
}
51+
52+
[Test]
53+
public void ForcedDecisionStoreNullContextForcedDecisionContext()
54+
{
55+
var expectedForcedDecision = new OptimizelyForcedDecision("sample_variation_key");
56+
OptimizelyDecisionContext context = null;
57+
var forcedDecisionStore = new ForcedDecisionsStore();
58+
forcedDecisionStore[context] = expectedForcedDecision;
59+
60+
Assert.AreEqual(forcedDecisionStore.Count, 0);
61+
}
62+
63+
[Test]
64+
public void ForcedDecisionStoreGetForcedDecisionWithBothRuleAndFlagKey()
65+
{
66+
var expectedForcedDecision1 = new OptimizelyForcedDecision("sample_variation_key");
67+
var context1 = new OptimizelyDecisionContext("flag_key", "rule_key");
68+
var NullFlagKeyContext = new OptimizelyDecisionContext(null, "rule_key");
69+
var forcedDecisionStore = new ForcedDecisionsStore();
70+
forcedDecisionStore[context1] = expectedForcedDecision1;
71+
72+
Assert.AreEqual(forcedDecisionStore.Count, 1);
73+
Assert.AreEqual(forcedDecisionStore[context1].VariationKey, expectedForcedDecision1.VariationKey);
74+
Assert.IsNull(forcedDecisionStore[NullFlagKeyContext]);
75+
}
76+
77+
[Test]
78+
public void ForcedDecisionStoreRemoveForcedDecisionTrue()
79+
{
80+
var expectedForcedDecision1 = new OptimizelyForcedDecision("sample_variation_key");
81+
var expectedForcedDecision2 = new OptimizelyForcedDecision("sample_variation_key_2");
82+
var context1 = new OptimizelyDecisionContext("flag_key", "rule_key");
83+
var context2 = new OptimizelyDecisionContext("flag_key", "rule_key1");
84+
var forcedDecisionStore = new ForcedDecisionsStore();
85+
forcedDecisionStore[context1] = expectedForcedDecision1;
86+
forcedDecisionStore[context2] = expectedForcedDecision2;
87+
88+
Assert.AreEqual(forcedDecisionStore.Count, 2);
89+
Assert.IsTrue(forcedDecisionStore.Remove(context2));
90+
Assert.AreEqual(forcedDecisionStore.Count, 1);
91+
Assert.AreEqual(forcedDecisionStore[context1].VariationKey, expectedForcedDecision1.VariationKey);
92+
Assert.IsNull(forcedDecisionStore[context2]);
93+
}
94+
95+
[Test]
96+
public void ForcedDecisionStoreRemoveForcedDecisionContextRuleKeyNotMatched()
97+
{
98+
var expectedForcedDecision = new OptimizelyForcedDecision("sample_variation_key");
99+
var contextNotMatched = new OptimizelyDecisionContext("flag_key", "");
100+
var context = new OptimizelyDecisionContext("flag_key", "rule_key");
101+
var forcedDecisionStore = new ForcedDecisionsStore();
102+
forcedDecisionStore[context] = expectedForcedDecision;
103+
104+
Assert.AreEqual(forcedDecisionStore.Count, 1);
105+
Assert.IsFalse(forcedDecisionStore.Remove(contextNotMatched));
106+
Assert.AreEqual(forcedDecisionStore.Count, 1);
107+
}
108+
109+
[Test]
110+
public void ForcedDecisionStoreRemoveAllForcedDecisionContext()
111+
{
112+
var expectedForcedDecision = new OptimizelyForcedDecision("sample_variation_key");
113+
var context = new OptimizelyDecisionContext("flag_key", "rule_key");
114+
var forcedDecisionStore = new ForcedDecisionsStore();
115+
forcedDecisionStore[context] = expectedForcedDecision;
116+
117+
Assert.AreEqual(forcedDecisionStore.Count, 1);
118+
forcedDecisionStore.RemoveAll();
119+
Assert.AreEqual(forcedDecisionStore.Count, 0);
120+
}
121+
122+
}
123+
}

‎OptimizelySDK.Tests/OptimizelySDK.Tests.csproj‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@
7575
<Compile Include="AudienceConditionsTests\ConditionsTest.cs" />
7676
<Compile Include="ConfigTest\HttpProjectConfigManagerTest.cs" />
7777
<Compile Include="ConfigTest\PollingProjectConfigManagerTest.cs" />
78-
<Compile Include="ConfigTest\AtomicProjectConfigManagerTest.cs" />
78+
<Compile Include="ConfigTest\FallbackProjectConfigManagerTest.cs" />
7979
<Compile Include="DecisionServiceTest.cs" />
8080
<Compile Include="DefaultErrorHandlerTest.cs" />
8181
<Compile Include="EventTests\EventProcessorProps.cs" />
@@ -93,6 +93,7 @@
9393
<Compile Include="NotificationTests\NotificationCenterTests.cs" />
9494
<Compile Include="ClientConfigHandlerTest.cs" />
9595
<Compile Include="OptimizelyTest.cs" />
96+
<Compile Include="ForcedDecisionsStoreTest.cs" />
9697
<Compile Include="OptimizelyUserContextTest.cs" />
9798
<Compile Include="Properties\AssemblyInfo.cs" />
9899
<Compile Include="TestBucketer.cs" />

0 commit comments

Comments
 (0)