Skip to content

Commit 92b20c3

Browse files
[FSSDK-12165] fix: cmab bug bash issue (#614)
* Add logs for cmab service * add cmab success message * Fix cmab cache timeout issue * Update runner to mac os 15 * Update xcode version * Update log level debug to info * Sperate missing key error
1 parent 58dade6 commit 92b20c3

File tree

7 files changed

+116
-34
lines changed

7 files changed

+116
-34
lines changed

Sources/CMAB/CmabClient.swift

Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,14 @@ import Foundation
1818

1919
enum CmabClientError: Error, Equatable {
2020
case fetchFailed(String)
21-
case invalidResponse
21+
case invalidResponse(String)
2222

2323
var message: String {
2424
switch self {
2525
case .fetchFailed(let message):
2626
return message
27-
case .invalidResponse:
28-
return "Invalid response from CMA-B server"
27+
case .invalidResponse(let reason):
28+
return "Invalid response from CMA-B server: \(reason)"
2929

3030
}
3131
}
@@ -159,34 +159,44 @@ class DefaultCmabClient: CmabClient {
159159
request.setValue("application/json", forHTTPHeaderField: "Content-Type")
160160

161161
guard let httpBody = try? JSONSerialization.data(withJSONObject: requestBody, options: []) else {
162+
self.logger.e("Failed to encode request body: \(requestBody)")
162163
completion(.failure(CmabClientError.fetchFailed("Failed to encode request body")))
163164
return
164165
}
166+
165167
request.httpBody = httpBody
166168

169+
self.logger.d("Fetching CMAB decision: \(url) with body: \(requestBody)")
170+
167171
let task = session.dataTask(with: request) { data, response, error in
168172
if let error = error {
173+
self.logger.e(error.localizedDescription)
169174
completion(.failure(CmabClientError.fetchFailed(error.localizedDescription)))
170175
return
171176
}
172177
guard let httpResponse = response as? HTTPURLResponse, let data = data, (200...299).contains(httpResponse.statusCode) else {
173178
let code = (response as? HTTPURLResponse)?.statusCode ?? -1
174-
completion(.failure(CmabClientError.fetchFailed("HTTP error code: \(code)")))
179+
let cmabError = CmabClientError.fetchFailed("HTTP error code: \(code)")
180+
self.logger.e(cmabError.message)
181+
completion(.failure(cmabError))
175182
return
176183
}
177184
do {
178-
if
179-
let json = try JSONSerialization.jsonObject(with: data, options: []) as? [String: Any],
180-
self.validateResponse(body: json),
181-
let predictions = json["predictions"] as? [[String: Any]],
182-
let variationId = predictions.first?["variation_id"] as? String
185+
if let json = try JSONSerialization.jsonObject(with: data, options: []) as? [String: Any],
186+
self.validateResponse(body: json),
187+
let predictions = json["predictions"] as? [[String: Any]],
188+
let variationId = predictions.first?["variation_id"] as? String
183189
{
184190
completion(.success(variationId))
185191
} else {
186-
completion(.failure(CmabClientError.invalidResponse))
192+
let error = CmabClientError.invalidResponse("Response missing 'predictions' array or 'variation_id' field")
193+
self.logger.e(error.message)
194+
completion(.failure(error))
187195
}
188196
} catch {
189-
completion(.failure(CmabClientError.invalidResponse))
197+
let error = CmabClientError.invalidResponse("JSON parsing failed: \(error.localizedDescription)")
198+
self.logger.e(error.message)
199+
completion(.failure(error))
190200
}
191201
}
192202
task.resume()

Sources/CMAB/CmabService.swift

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ protocol CmabService {
4444
typealias CmabCache = LruCache<String, CmabCacheValue>
4545

4646
class DefaultCmabService: CmabService {
47-
typealias UserAttributes = [String : Any?]
47+
typealias UserAttributes = [String: Any?]
4848

4949
let cmabClient: CmabClient
5050
let cmabCache: CmabCache
@@ -97,33 +97,38 @@ class DefaultCmabService: CmabService {
9797
let userId = userContext.userId
9898

9999
if options.contains(.ignoreCmabCache) {
100-
self.logger.i("Ignoring CMAB cache.")
100+
self.logger.i("Ignoring CMAB cache for user \(userId) and rule \(ruleId)")
101101
fetchDecision(ruleId: ruleId, userId: userId, attributes: filteredAttributes, completion: completion)
102102
return
103103
}
104104

105105
if options.contains(.resetCmabCache) {
106-
self.logger.i("Resetting CMAB cache.")
106+
self.logger.i("Resetting CMAB cache for user \(userId) and rule \(ruleId)")
107107
cmabCache.reset()
108108
}
109109

110110
let cacheKey = getCacheKey(userId: userId, ruleId: ruleId)
111111

112112
if options.contains(.invalidateUserCmabCache) {
113-
self.logger.i("Invalidating user CMAB cache.")
113+
self.logger.i("Invalidating CMAB cache for user \(userId) and rule \(ruleId)")
114114
self.cmabCache.remove(key: cacheKey)
115115
}
116116

117117
let attributesHash = hashAttributes(filteredAttributes)
118118

119-
if let cachedValue = cmabCache.lookup(key: cacheKey), cachedValue.attributesHash == attributesHash {
120-
let decision = CmabDecision(variationId: cachedValue.variationId, cmabUUID: cachedValue.cmabUUID)
121-
self.logger.i("Returning cached CMAB decision.")
122-
completion(.success(decision))
123-
return
119+
if let cachedValue = cmabCache.lookup(key: cacheKey) {
120+
if cachedValue.attributesHash == attributesHash {
121+
let decision = CmabDecision(variationId: cachedValue.variationId, cmabUUID: cachedValue.cmabUUID)
122+
self.logger.i("CMAB cache hit for user \(userId) and rule \(ruleId)")
123+
completion(.success(decision))
124+
return
125+
} else {
126+
self.logger.i("CMAB cache attributes mismatch for user \(userId) and rule \(ruleId), fetching new decision")
127+
cmabCache.remove(key: cacheKey)
128+
}
129+
124130
} else {
125-
self.logger.i("CMAB decision not found in cache.")
126-
cmabCache.remove(key: cacheKey)
131+
self.logger.i("CMAB cache miss for user \(userId) and rule \(ruleId)")
127132
}
128133

129134
fetchDecision(ruleId: ruleId, userId: userId, attributes: filteredAttributes) { result in
@@ -133,7 +138,6 @@ class DefaultCmabService: CmabService {
133138
variationId: decision.variationId,
134139
cmabUUID: decision.cmabUUID
135140
)
136-
self.logger.i("Featched CMAB decision and cached it.")
137141
self.cmabCache.save(key: cacheKey, value: cacheValue)
138142
}
139143
completion(result)
@@ -148,11 +152,11 @@ class DefaultCmabService: CmabService {
148152
cmabClient.fetchDecision(ruleId: ruleId, userId: userId, attributes: attributes, cmabUUID: cmabUUID) { result in
149153
switch result {
150154
case .success(let variaitonId):
151-
self.logger.i("Fetched CMAB decision: \(variaitonId)")
152155
let decision = CmabDecision(variationId: variaitonId, cmabUUID: cmabUUID)
156+
self.logger.i("Featched CMAB decision, (variationId: \(decision.variationId), cmabUUID: \(decision.cmabUUID))")
153157
completion(.success(decision))
154158
case .failure(let error):
155-
self.logger.e("Failed to fetch CMAB decision: \(error)")
159+
self.logger.e("Failed to fetch CMAB decision, error: \(error)")
156160
completion(.failure(error))
157161
}
158162
}

Sources/Implementation/DefaultDecisionService.swift

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ typealias UserProfile = OPTUserProfileService.UPProfile
3535
class DefaultDecisionService: OPTDecisionService {
3636
let bucketer: OPTBucketer
3737
let userProfileService: OPTUserProfileService
38-
let cmabService: CmabService
38+
var cmabService: CmabService
3939
let group: DispatchGroup = DispatchGroup()
4040
// thread-safe lazy logger load (after HandlerRegisterService ready)
4141
private let threadSafeLogger = ThreadSafeLogger()
@@ -123,6 +123,9 @@ class DefaultDecisionService: OPTDecisionService {
123123
switch response {
124124
case .success(let decision):
125125
cmabDecision = decision
126+
let info = LogMessage.cmabFetchSuccess(decision.variationId, decision.cmabUUID, _expKey: experiment.key)
127+
self.logger.d(info)
128+
reasons.addInfo(info)
126129
case .failure:
127130
let info = LogMessage.cmabFetchFailed(experiment.key)
128131
self.logger.e(info)
@@ -239,7 +242,6 @@ class DefaultDecisionService: OPTDecisionService {
239242
return DecisionResponse(result: variationDecision, reasons: reasons)
240243
}
241244

242-
243245
var variationDecision: VariationDecision?
244246
// ---- check if the user passes audience targeting before bucketing ----
245247
let audienceResponse = doesMeetAudienceConditions(config: config,

Sources/Optimizely/OptimizelyClient.swift

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -110,20 +110,24 @@ open class OptimizelyClient: NSObject {
110110
let logger = logger ?? DefaultLogger()
111111
type(of: logger).logLevel = defaultLogLevel ?? .info
112112

113-
let cmabService = DefaultCmabService.createDefault(config: cmabConfig ?? CmabConfig())
114-
115113
self.registerServices(sdkKey: sdkKey,
116114
logger: logger,
117115
eventDispatcher: eventDispatcher ?? DefaultEventDispatcher.sharedInstance,
118116
datafileHandler: datafileHandler ?? DefaultDatafileHandler(),
119-
decisionService: DefaultDecisionService(userProfileService: userProfileService, cmabService: cmabService),
117+
decisionService: DefaultDecisionService(userProfileService: userProfileService),
120118
notificationCenter: DefaultNotificationCenter())
121119

122120
self.logger = HandlerRegistryService.shared.injectLogger()
123121
self.eventDispatcher = HandlerRegistryService.shared.injectEventDispatcher(sdkKey: self.sdkKey)
124122
self.datafileHandler = HandlerRegistryService.shared.injectDatafileHandler(sdkKey: self.sdkKey)
125123
self.decisionService = HandlerRegistryService.shared.injectDecisionService(sdkKey: self.sdkKey)
126124
self.notificationCenter = HandlerRegistryService.shared.injectNotificationCenter(sdkKey: self.sdkKey)
125+
126+
// Set cmabService after injection to handle re-initialization with the same sdkKey.
127+
// HandlerRegistryService won't replace existing bindings, so we mutate the instance instead.
128+
let cmabService = DefaultCmabService.createDefault(config: cmabConfig ?? CmabConfig())
129+
(self.decisionService as? DefaultDecisionService)?.cmabService = cmabService
130+
127131
if let _vuid = vuid {
128132
self.odpManager.vuid = _vuid
129133
sendInitializedEvent(vuid: _vuid)

Sources/Utils/LogMessage.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ enum LogMessage {
7676
case failedToAssignValue
7777
case valueForKeyNotFound(_ key: String)
7878
case lowPeriodicDownloadInterval
79+
case cmabFetchSuccess(_ variationId: String, _ cmabUUID: String, _expKey: String)
7980
case cmabFetchFailed(_ expKey: String)
8081
case cmabNotSupportedInSyncMode
8182
}
@@ -148,6 +149,7 @@ extension LogMessage: CustomStringConvertible {
148149
case .failedToAssignValue: message = "Value for path could not be assigned to provided type."
149150
case .valueForKeyNotFound(let key): message = "Value for JSON key (\(key)) not found."
150151
case .lowPeriodicDownloadInterval: message = "Polling intervals below 30 seconds are not recommended."
152+
case .cmabFetchSuccess(let variationId, let cmabUUID, let expKey): message = "Successfully fetched CMAB decision, variationId: \(variationId), cmabUUID: \(cmabUUID) for experiment \(expKey)."
151153
case .cmabFetchFailed(let key): message = "Failed to fetch CMAB data for experiment \(key)."
152154
case .cmabNotSupportedInSyncMode: message = "CMAB is not supported in sync mode."
153155
}

Tests/OptimizelyTests-APIs/OptimizelyClientTests_Cmab_Config.swift

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,4 +74,56 @@ class OptimizelyClientTests_Cmab_Config: XCTestCase {
7474
XCTAssertEqual(1800, cmabCache.timeoutInSecs)
7575
XCTAssertEqual("http://fowardslash.com/predict/rule-12345/v1/", cmabClient.getUrl(ruleId: "rule-12345")?.absoluteString)
7676
}
77+
78+
// MARK: - Re-initialization with Same SDK Key
79+
80+
func test_cmab_reinitialization_same_sdkKey_updates_config() {
81+
let sdkKey = "test-sdk-key-reinit"
82+
83+
// First initialization with 3 second timeout
84+
let config1 = CmabConfig(cacheSize: 10, cacheTimeoutInSecs: 3,
85+
predictionEndpoint: "https://endpoint1.com/%@")
86+
var optimizely = OptimizelyClient(sdkKey: sdkKey, cmabConfig: config1)
87+
var cmabService = ((optimizely.decisionService as! DefaultDecisionService).cmabService as! DefaultCmabService)
88+
var cmabCache = cmabService.cmabCache
89+
var cmabClient = cmabService.cmabClient as! DefaultCmabClient
90+
91+
XCTAssertEqual(10, cmabCache.maxSize, "First init: cache size should be 10")
92+
XCTAssertEqual(3, cmabCache.timeoutInSecs, "First init: cache timeout should be 3")
93+
XCTAssertEqual("https://endpoint1.com/%@", cmabClient.predictionEndpoint, "First init: should use endpoint1")
94+
95+
// Re-initialize with SAME sdkKey but different config (5 second timeout)
96+
let config2 = CmabConfig(cacheSize: 50, cacheTimeoutInSecs: 5,
97+
predictionEndpoint: "https://endpoint2.com/%@")
98+
optimizely = OptimizelyClient(sdkKey: sdkKey, cmabConfig: config2)
99+
cmabService = ((optimizely.decisionService as! DefaultDecisionService).cmabService as! DefaultCmabService)
100+
cmabCache = cmabService.cmabCache
101+
cmabClient = cmabService.cmabClient as! DefaultCmabClient
102+
103+
XCTAssertEqual(50, cmabCache.maxSize, "Second init: cache size should be updated to 50")
104+
XCTAssertEqual(5, cmabCache.timeoutInSecs, "Second init: cache timeout should be updated to 5")
105+
XCTAssertEqual("https://endpoint2.com/%@", cmabClient.predictionEndpoint, "Second init: endpoint should be updated")
106+
}
107+
108+
func test_cmab_different_sdkKeys_maintain_separate_configs() {
109+
// Initialize first client with sdkKey1
110+
let sdkKey1 = "test-sdk-key-1"
111+
let config1 = CmabConfig(cacheSize: 10, cacheTimeoutInSecs: 100)
112+
let client1 = OptimizelyClient(sdkKey: sdkKey1, cmabConfig: config1)
113+
let cmabService1 = ((client1.decisionService as! DefaultDecisionService).cmabService as! DefaultCmabService)
114+
let cmabCache1 = cmabService1.cmabCache
115+
116+
// Initialize second client with sdkKey2
117+
let sdkKey2 = "test-sdk-key-2"
118+
let config2 = CmabConfig(cacheSize: 50, cacheTimeoutInSecs: 500)
119+
let client2 = OptimizelyClient(sdkKey: sdkKey2, cmabConfig: config2)
120+
let cmabService2 = ((client2.decisionService as! DefaultDecisionService).cmabService as! DefaultCmabService)
121+
let cmabCache2 = cmabService2.cmabCache
122+
123+
// Verify both clients maintain their own configs independently
124+
XCTAssertEqual(10, cmabCache1.maxSize, "Client 1 should have cache size 10")
125+
XCTAssertEqual(100, cmabCache1.timeoutInSecs, "Client 1 should have timeout 100")
126+
XCTAssertEqual(50, cmabCache2.maxSize, "Client 2 should have cache size 50")
127+
XCTAssertEqual(500, cmabCache2.timeoutInSecs, "Client 2 should have timeout 500")
128+
}
77129
}

Tests/OptimizelyTests-Common/CMABClientTests.swift

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -175,15 +175,19 @@ class DefaultCmabClientTests: XCTestCase {
175175
(Data("not a json".utf8), HTTPURLResponse(url: URL(string: "https://prediction.cmab.optimizely.com/predict/abc")!,
176176
statusCode: 200, httpVersion: nil, headerFields: nil), nil)
177177
]
178-
178+
179179
let expectation = self.expectation(description: "Completion called")
180180
client.fetchDecision(
181181
ruleId: "abc", userId: "user1",
182-
attributes: ["foo": "bar"],
182+
attributes: ["foo": "bar"],
183183
cmabUUID: "uuid"
184184
) { result in
185185
if case let .failure(error) = result {
186-
XCTAssertTrue(error is CmabClientError)
186+
if case .invalidResponse(let reason) = error as? CmabClientError {
187+
XCTAssertTrue(reason.contains("JSON parsing failed"))
188+
} else {
189+
XCTFail("Expected CmabClientError.invalidResponse for JSON parsing failure")
190+
}
187191
XCTAssertEqual(self.mockSession.callCount, 1)
188192
} else {
189193
XCTFail("Expected failure on invalid JSON")
@@ -209,7 +213,11 @@ class DefaultCmabClientTests: XCTestCase {
209213
cmabUUID: "uuid-1234"
210214
) { result in
211215
if case let .failure(error) = result {
212-
XCTAssertEqual(error as? CmabClientError, .invalidResponse)
216+
if case .invalidResponse(let reason) = error as? CmabClientError {
217+
XCTAssertTrue(reason.contains("Response missing 'predictions' array or 'variation_id' field"))
218+
} else {
219+
XCTFail("Expected CmabClientError.invalidResponse")
220+
}
213221
XCTAssertEqual(self.mockSession.callCount, 1)
214222
} else {
215223
XCTFail("Expected failure on invalid response structure")

0 commit comments

Comments
 (0)