Skip to content

Commit 3c66bdc

Browse files
muzahidul-optimuzahidul-opti
authored
[FSSDK-12033] fix: ignore UPS for cmab decision (#620)
* fix: ignore UPS for cmab decision done * fix: exclude cmab decision from ups * clean up * Add profile tracker test case for cmab decision
1 parent a829b2b commit 3c66bdc

File tree

4 files changed

+57
-16
lines changed

4 files changed

+57
-16
lines changed

‎DemoSwiftApp/Samples/SamplesForAPI.swift‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -444,7 +444,7 @@ class SamplesForAPI {
444444
userId: "USER_123",
445445
attributes: ["country": "us"]
446446
)
447-
let options: [OptimizelyDecideOption] = [.ignoreCmabCache, .ignoreUserProfileService]
447+
let options: [OptimizelyDecideOption] = [.ignoreCmabCache]
448448
let decision = await user.decideAsync(key: FLAG_KEY, options: options)
449449
print("CMAB decision: \(decision)")
450450
}
@@ -462,7 +462,7 @@ class SamplesForAPI {
462462
userId: "USER_123",
463463
attributes: ["country": "us"]
464464
)
465-
let options: [OptimizelyDecideOption] = [.ignoreCmabCache, .ignoreUserProfileService]
465+
let options: [OptimizelyDecideOption] = [.ignoreCmabCache]
466466
user.decideAsync(key: FLAG_KEY, options: options, completion: { decision in
467467
print("CMAB decision: \(decision)")
468468
})
@@ -494,7 +494,7 @@ class SamplesForAPI {
494494
userId: "USER_123",
495495
attributes: ["country": "us"]
496496
)
497-
let options: [OptimizelyDecideOption] = [.ignoreCmabCache, .ignoreUserProfileService]
497+
let options: [OptimizelyDecideOption] = [.ignoreCmabCache]
498498
let decision = await user.decideAsync(key: FLAG_KEY, options: options)
499499
print("CMAB decision: \(decision)")
500500
}
@@ -512,7 +512,7 @@ class SamplesForAPI {
512512
userId: "USER_123",
513513
attributes: ["country": "us"]
514514
)
515-
let options: [OptimizelyDecideOption] = [.ignoreCmabCache, .ignoreUserProfileService]
515+
let options: [OptimizelyDecideOption] = [.ignoreCmabCache]
516516
user.decideAsync(key: FLAG_KEY, options: options, completion: { decision in
517517
print("CMAB decision: \(decision)")
518518
})

‎Sources/Implementation/DefaultDecisionService.swift‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -277,7 +277,12 @@ class DefaultDecisionService: OPTDecisionService {
277277
let info = LogMessage.userBucketedIntoVariationInExperiment(userId, experiment.key, variation.key)
278278
logger.i(info)
279279
reasons.addInfo(info)
280-
userProfileTracker?.updateProfile(experiment: experiment, variation: variation)
280+
281+
// CMAB decision shouldn't be in the UPS
282+
if !experiment.isCmab {
283+
userProfileTracker?.updateProfile(experiment: experiment, variation: variation)
284+
}
285+
281286
} else {
282287
let info = LogMessage.userNotBucketedIntoVariation(userId)
283288
logger.i(info)

‎Tests/OptimizelyTests-Common/DecisionServiceTests_Experiments.swift‎

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -732,6 +732,38 @@ extension DecisionServiceTests_Experiments {
732732
XCTAssertNotNil(variation)
733733
XCTAssertEqual(variation?.key, kVariationKeyA)
734734
}
735+
736+
func testCMABVariationDoesnotTrackByProfileTracker() {
737+
self.config.project.typedAudiences = try! OTUtils.model(from: sampleTypedAudiencesData)
738+
var cmabExperiment: Experiment = try! OTUtils.model(from: sampleExperimentData)
739+
cmabExperiment.cmab = try! OTUtils.model(from: ["trafficAllocation": 10000, "attributeIds": ["10389729780"]])
740+
self.config.project.experiments = [cmabExperiment]
741+
let mocCmabService = MockCmabService()
742+
mocCmabService.variationId = "10389729780" // kVariationKeyA
743+
let ups = DefaultUserProfileService()
744+
self.decisionService = DefaultDecisionService(userProfileService: ups, cmabService: mocCmabService)
745+
746+
let user = optimizely.createUserContext(
747+
userId: kUserId,
748+
attributes: kAttributesCountryMatch
749+
)
750+
751+
let tracker = UserProfileTracker(userId: "user_1234", userProfileService: ups, logger: self.decisionService.logger)
752+
tracker.loadUserProfile()
753+
754+
let decision = self.decisionService.getVariation(config: config,
755+
experiment: cmabExperiment,
756+
user: user,
757+
options: nil,
758+
isAsync: true,
759+
userProfileTracker: tracker)
760+
761+
let variation = decision.result?.variation
762+
XCTAssertNotNil(variation)
763+
XCTAssertEqual(variation?.key, kVariationKeyA)
764+
XCTAssertFalse(tracker.profileUpdated)
765+
XCTAssertTrue(tracker.userProfile!.isEmpty)
766+
}
735767

736768
func testGetVariationWithCMABZeroTrafficAllocation() {
737769
// Test when traffic allocation is 0%
@@ -834,6 +866,7 @@ extension DecisionServiceTests_Experiments {
834866
self.config.project.experiments = [cmabExperiment]
835867
let mocCmabService = MockCmabService()
836868
mocCmabService.variationId = "unknown_var_id"
869+
mocCmabService.cmabUUID = "test_UUID_1234"
837870

838871
self.decisionService = DefaultDecisionService(userProfileService: DefaultUserProfileService(), cmabService: mocCmabService)
839872

@@ -843,8 +876,10 @@ extension DecisionServiceTests_Experiments {
843876
)
844877

845878
let expectedReasons = DecisionReasons()
879+
expectedReasons.addInfo(LogMessage.cmabFetchSuccess("unknown_var_id", "test_UUID_1234", _expKey: cmabExperiment.key))
846880
expectedReasons.addInfo(LogMessage.userNotBucketedIntoVariation(user.userId))
847881

882+
848883
let decision = self.decisionService.getVariation(config: config,
849884
experiment: cmabExperiment,
850885
user: user,
@@ -885,14 +920,15 @@ fileprivate struct MockError: Error {
885920
fileprivate class MockCmabService: DefaultCmabService {
886921
var error: Error?
887922
var variationId: String?
923+
var cmabUUID: String?
888924

889925
init() {
890926
super.init(cmabClient: DefaultCmabClient(), cmabCache: CmabCache(size: 10, timeoutInSecs: 10))
891927
}
892928

893929
override func getDecision(config: ProjectConfig, userContext: OptimizelyUserContext, ruleId: String, options: [OptimizelyDecideOption]) -> Result<CmabDecision, any Error> {
894930
if let variationId = self.variationId {
895-
let cmabUUID = UUID().uuidString
931+
let cmabUUID = self.cmabUUID ?? UUID().uuidString
896932
return .success(CmabDecision(variationId: variationId, cmabUUID: cmabUUID))
897933
} else {
898934
return .failure(self.error ?? MockError())

‎Tests/OptimizelyTests-Common/OptimizelyUserContextTests_Decide_CMAB.swift‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ class OptimizelyUserContextTests_Decide_CMAB: XCTestCase {
132132
let user = optimizely.createUserContext(userId: kUserId, attributes: ["gender": "f"])
133133

134134
// Test multiple decisions with decideAsync
135-
user.decideAsync(keys: featureKeys, options: [.ignoreUserProfileService]) { decisions in
135+
user.decideAsync(keys: featureKeys) { decisions in
136136

137137
// Verify correct number of decisions were returned
138138
XCTAssertEqual(decisions.count, 2)
@@ -175,7 +175,7 @@ class OptimizelyUserContextTests_Decide_CMAB: XCTestCase {
175175
wait(for: [expectation], timeout: 5) // Increased timeout for reliability
176176
}
177177

178-
func testDecideAsync_cmabWithUserProfileCahing() {
178+
func testDecideAsync_cmabIgnoreUPSCacheing() {
179179
let expectation1 = XCTestExpectation(description: "First CMAB decision")
180180
let expectation2 = XCTestExpectation(description: "Second CMAB decision")
181181

@@ -192,17 +192,17 @@ class OptimizelyUserContextTests_Decide_CMAB: XCTestCase {
192192
attributes: ["gender": "f", "age": 25]
193193
)
194194

195-
// First decision cache into user profile
195+
196196
user.decideAsync(key: "feature_1") { decision in
197197
XCTAssertEqual(decision.variationKey, "a")
198198
XCTAssertEqual(self.mockCmabService.decisionCallCount, 1)
199199
expectation1.fulfill()
200200

201-
// Second decision (should use cache)
201+
// Second decision, ignore UPS, fetch decision again
202202
user.decideAsync(key: "feature_1") { decision in
203203
XCTAssertEqual(decision.variationKey, "a")
204-
// Call count should still be 1 (cached)
205-
XCTAssertEqual(self.mockCmabService.decisionCallCount, 1)
204+
// Call count should be increased by 1
205+
XCTAssertEqual(self.mockCmabService.decisionCallCount, 2)
206206
expectation2.fulfill()
207207
}
208208
}
@@ -228,17 +228,17 @@ class OptimizelyUserContextTests_Decide_CMAB: XCTestCase {
228228
userId: kUserId,
229229
attributes: ["gender": "f", "age": 25]
230230
)
231-
user.decideAsync(key: "feature_1", options: [.ignoreUserProfileService, .ignoreCmabCache]) { decision in
231+
user.decideAsync(key: "feature_1", options: [.ignoreCmabCache]) { decision in
232232
XCTAssertEqual(decision.variationKey, "a")
233233
XCTAssertTrue(self.mockCmabService.ignoreCacheUsed)
234234
exp1.fulfill()
235235
}
236-
user.decideAsync(key: "feature_1", options: [.ignoreUserProfileService, .resetCmabCache]) { decision in
236+
user.decideAsync(key: "feature_1", options: [.resetCmabCache]) { decision in
237237
XCTAssertEqual(decision.variationKey, "a")
238238
XCTAssertTrue(self.mockCmabService.resetCacheCache)
239239
exp2.fulfill()
240240
}
241-
user.decideAsync(key: "feature_1", options: [.ignoreUserProfileService, .invalidateUserCmabCache]) { decision in
241+
user.decideAsync(key: "feature_1", options: [.invalidateUserCmabCache]) { decision in
242242
XCTAssertEqual(decision.variationKey, "a")
243243
XCTAssertTrue(self.mockCmabService.invalidateUserCmabCache)
244244
exp3.fulfill()
@@ -263,7 +263,7 @@ class OptimizelyUserContextTests_Decide_CMAB: XCTestCase {
263263
attributes: ["gender": "f", "age": 25]
264264
)
265265

266-
user.decideAsync(key: "feature_1", options: [.ignoreUserProfileService, .includeReasons]) { decision in
266+
user.decideAsync(key: "feature_1", options: [.includeReasons]) { decision in
267267
XCTAssertTrue(decision.reasons.contains(LogMessage.cmabFetchFailed("exp_with_audience").reason))
268268
expectation.fulfill()
269269
}

0 commit comments

Comments
 (0)