Skip to content

Commit 9463dd1

Browse files
fix: offline intitalization using cached config (#238)
* fix: offline intitalization using cached config * chore: cleanup setting userConfig, new updateUserConfig method * chore: close existing SSE connection * fix: try increasing test timeout
1 parent f334749 commit 9463dd1

File tree

4 files changed

+158
-53
lines changed

4 files changed

+158
-53
lines changed

DevCycle/DevCycleClient.swift

Lines changed: 83 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ enum ClientError: Error {
2121
case InvalidUser
2222
case MissingUserOrFeatureVariationsMap
2323
case MissingUser
24+
case ConfigFetchFailed
2425
}
2526

2627
public typealias ClientInitializedHandler = (Error?) -> Void
@@ -43,14 +44,13 @@ public class DevCycleClient {
4344
var inactivityDelayMS: Double = 120000
4445

4546
private var service: DevCycleServiceProtocol?
46-
private var cacheService: CacheServiceProtocol = CacheService()
47-
private var cache: Cache?
47+
internal var cacheService: CacheServiceProtocol = CacheService()
4848
var sseConnection: SSEConnectionProtocol?
4949
private var flushTimer: Timer?
5050
private var closed: Bool = false
5151
private var inactivityWorkItem: DispatchWorkItem?
5252
private var variableInstanceDictonary = [String: NSMapTable<AnyObject, AnyObject>]()
53-
private var isConfigCached: Bool = false
53+
internal var isConfigCached: Bool = false
5454
private var disableAutomaticEventLogging: Bool = false
5555
private var disableCustomEventLogging: Bool = false
5656

@@ -152,15 +152,23 @@ public class DevCycleClient {
152152
user: user, enableEdgeDB: self.enableEdgeDB, extraParams: nil,
153153
completion: { [weak self] config, error in
154154
guard let self = self else { return }
155+
156+
var finalError: Error? = error
157+
155158
if let error = error {
156159
Log.error("Error getting config: \(error)", tags: ["setup"])
157-
self.cache = self.cacheService.load(user: user)
158-
} else {
159-
if let config = config {
160-
Log.debug("Config: \(config)", tags: ["setup"])
160+
161+
// If network failed but we have a cached config, don't return error
162+
if self.config?.userConfig != nil {
163+
Log.info("Using cached config due to network error")
164+
finalError = nil
161165
}
162-
self.config?.userConfig = config
163-
self.isConfigCached = false
166+
} else if let config = config {
167+
Log.debug("Config: \(config)", tags: ["setup"])
168+
self.updateUserConfig(config)
169+
} else {
170+
Log.error("No config returned for setup", tags: ["setup"])
171+
finalError = ClientError.ConfigFetchFailed
164172
}
165173

166174
if let config = config,
@@ -181,12 +189,10 @@ public class DevCycleClient {
181189
}
182190
}
183191

184-
self.setupSSEConnection()
185-
186192
for handler in self.configCompletionHandlers {
187-
handler(error)
193+
handler(finalError)
188194
}
189-
callback?(error)
195+
callback?(finalError)
190196
self.initialized = true
191197
self.configCompletionHandlers = []
192198
})
@@ -229,14 +235,26 @@ public class DevCycleClient {
229235
guard let self = self else { return }
230236
if let error = error {
231237
Log.error("Error getting config: \(error)", tags: ["refetchConfig"])
238+
} else if let config = config {
239+
self.updateUserConfig(config)
232240
} else {
233-
self.config?.userConfig = config
234-
self.isConfigCached = false
241+
Log.error("No config returned for refetchConfig", tags: ["refetchConfig"])
235242
}
236243
})
237244
}
238245
}
239246

247+
private func updateUserConfig(_ config: UserConfig) {
248+
let oldSSEURL = self.config?.userConfig?.sse?.url
249+
self.config?.userConfig = config
250+
self.isConfigCached = false
251+
252+
let newSSEURL = config.sse?.url
253+
if newSSEURL != nil && oldSSEURL != newSSEURL {
254+
self.setupSSEConnection()
255+
}
256+
}
257+
240258
private func setupSSEConnection() {
241259
if let disableRealtimeUpdates = self.options?.disableRealtimeUpdates, disableRealtimeUpdates
242260
{
@@ -252,6 +270,13 @@ public class DevCycleClient {
252270
Log.error("Failed to parse SSE URL in config")
253271
return
254272
}
273+
274+
if self.sseConnection != nil {
275+
Log.debug("Closing existing SSE connection")
276+
self.sseConnection?.close()
277+
self.sseConnection = nil
278+
}
279+
255280
if let inactivityDelay = self.config?.userConfig?.sse?.inactivityDelay {
256281
self.inactivityDelayMS = Double(inactivityDelay)
257282
}
@@ -418,21 +443,41 @@ public class DevCycleClient {
418443
user: updateUser, enableEdgeDB: self.enableEdgeDB, extraParams: nil,
419444
completion: { [weak self] config, error in
420445
guard let self = self else { return }
446+
421447
if let error = error {
422448
Log.error(
423449
"Error getting config: \(error) for user_id \(String(describing: updateUser.userId))",
424450
tags: ["identify"])
425-
self.cache = self.cacheService.load(user: updateUser)
451+
452+
// Try to use cached config for the new user
426453
self.useCachedConfigForUser(user: updateUser)
427-
} else {
428-
if let config = config {
429-
Log.debug("Config: \(config)", tags: ["identify"])
454+
455+
// If we have a cached config, proceed without error
456+
if self.config?.userConfig != nil {
457+
Log.info(
458+
"Using cached config for identifyUser due to network error: \(error)",
459+
tags: ["identify"])
460+
self.user = user
461+
callback?(nil, self.config?.userConfig?.variables)
462+
return
463+
} else {
464+
// No cached config available, return error and don't change client state
465+
Log.error(
466+
"Error getting config for identifyUser: \(error)", tags: ["identify"])
467+
callback?(error, nil)
468+
return
430469
}
431-
self.config?.userConfig = config
432-
self.isConfigCached = false
433470
}
434-
self.user = user
435-
callback?(error, self.config?.userConfig?.variables)
471+
472+
if let config = config {
473+
Log.debug("IdentifyUser config: \(config)", tags: ["identify"])
474+
self.updateUserConfig(config)
475+
self.user = user
476+
callback?(nil, self.config?.userConfig?.variables)
477+
} else {
478+
Log.error("No config returned for identifyUser", tags: ["identify"])
479+
callback?(ClientError.ConfigFetchFailed, nil)
480+
}
436481
})
437482
}
438483

@@ -457,7 +502,6 @@ public class DevCycleClient {
457502
}
458503

459504
public func resetUser(callback: IdentifyCompletedHandler? = nil) throws {
460-
self.cache = cacheService.load(user: self.user!)
461505
self.flushEvents()
462506

463507
let cachedAnonUserId = self.cacheService.getAnonUserId()
@@ -470,7 +514,10 @@ public class DevCycleClient {
470514
user: anonUser, enableEdgeDB: self.enableEdgeDB, extraParams: nil,
471515
completion: { [weak self] config, error in
472516
guard let self = self else { return }
473-
guard error == nil else {
517+
518+
if let error = error {
519+
Log.error("Error getting config for resetUser: \(error)", tags: ["reset"])
520+
// Restore previous anonymous user ID on error and don't change client state
474521
if let previousAnonUserId = cachedAnonUserId {
475522
self.cacheService.setAnonUserId(anonUserId: previousAnonUserId)
476523
}
@@ -479,12 +526,14 @@ public class DevCycleClient {
479526
}
480527

481528
if let config = config {
482-
Log.debug("Config: \(config)", tags: ["reset"])
529+
Log.debug("ResetUser config: \(config)", tags: ["reset"])
530+
self.updateUserConfig(config)
531+
self.user = anonUser
532+
callback?(nil, config.variables)
533+
} else {
534+
Log.error("No config returned for resetUser", tags: ["reset"])
535+
callback?(ClientError.ConfigFetchFailed, nil)
483536
}
484-
self.config?.userConfig = config
485-
self.isConfigCached = false
486-
self.user = anonUser
487-
callback?(error, config?.variables)
488537
})
489538
}
490539

@@ -676,12 +725,10 @@ public class DevCycleClient {
676725
}
677726

678727
private func useCachedConfigForUser(user: DevCycleUser) {
679-
var cachedConfig: UserConfig?
680-
if let disableConfigCache = options?.disableConfigCache, !disableConfigCache {
681-
cachedConfig = cacheService.getConfig(user: user)
682-
}
683-
684-
if cachedConfig != nil {
728+
// Load cached config by default, unless explicitly disabled
729+
if options?.disableConfigCache != true,
730+
let cachedConfig = cacheService.getConfig(user: user)
731+
{
685732
self.config?.userConfig = cachedConfig
686733
self.isConfigCached = true
687734
Log.debug("Loaded config from cache for user_id \(String(describing: user.userId))")

DevCycle/Models/Cache.swift

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
import Foundation
88

99
protocol CacheServiceProtocol {
10-
func load(user: DevCycleUser) -> Cache
1110
func setAnonUserId(anonUserId: String)
1211
func getAnonUserId() -> String?
1312
func clearAnonUserId()
@@ -17,11 +16,6 @@ protocol CacheServiceProtocol {
1716
func migrateLegacyCache()
1817
}
1918

20-
struct Cache {
21-
var config: UserConfig?
22-
var anonUserId: String?
23-
}
24-
2519
class CacheService: CacheServiceProtocol {
2620
struct CacheKeys {
2721
static let anonUserId = "ANONYMOUS_USER_ID"
@@ -41,12 +35,7 @@ class CacheService: CacheServiceProtocol {
4135

4236
init(configCacheTTL: Int = DEFAULT_CONFIG_CACHE_TTL) {
4337
self.configCacheTTL = configCacheTTL
44-
}
45-
46-
func load(user: DevCycleUser) -> Cache {
4738
migrateLegacyCache()
48-
49-
return Cache(config: getConfig(user: user), anonUserId: getAnonUserId())
5039
}
5140

5241
func setAnonUserId(anonUserId: String) {

DevCycleTests/Models/DevCycleClientTests.swift

Lines changed: 75 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ class DevCycleClientTest: XCTestCase {
161161
expectation.fulfill()
162162
}
163163

164-
wait(for: [expectation], timeout: 1.0)
164+
wait(for: [expectation], timeout: 2.0)
165165
client.close(callback: nil)
166166
}
167167

@@ -714,6 +714,60 @@ class DevCycleClientTest: XCTestCase {
714714
wait(for: [expectation], timeout: 1.0)
715715
client.close(callback: nil)
716716
}
717+
718+
func testIdentifyUserWithCacheAvailableDoesNotReturnError() {
719+
let expectation = XCTestExpectation(
720+
description: "identifyUser with cache available should not return error")
721+
let failedService = MockFailedConnectionService()
722+
723+
// Create a mock cache service that returns a cached config
724+
let mockCacheService = MockCacheServiceWithConfig(userConfig: self.userConfig)
725+
726+
let client = try! self.builder.user(self.user).sdkKey("dvc_mobile_my_sdk_key").service(
727+
failedService
728+
).build(onInitialized: nil)
729+
730+
// Replace the cache service with our mock that has a cached config
731+
client.cacheService = mockCacheService
732+
733+
// Initialize the client's config object
734+
client.config = DVCConfig(sdkKey: "dvc_mobile_my_sdk_key", user: self.user)
735+
736+
client.setup(
737+
service: failedService,
738+
callback: { error in
739+
// Build process should work with cache
740+
XCTAssertNil(error, "Build should not return error when cache is available")
741+
742+
do {
743+
let newUser = try DevCycleUser.builder().userId("new_user").build()
744+
745+
// identifyUser should work with cached config even when network fails
746+
try client.identifyUser(
747+
user: newUser,
748+
callback: { error, variables in
749+
XCTAssertNil(
750+
error,
751+
"identifyUser should not return error when cache is available")
752+
XCTAssertNotNil(
753+
variables,
754+
"identifyUser should return variables when cache is available")
755+
XCTAssertTrue(
756+
client.isConfigCached, "Config should be marked as cached")
757+
XCTAssertEqual(
758+
client.user?.userId, newUser.userId,
759+
"User should be updated to new user")
760+
expectation.fulfill()
761+
})
762+
} catch {
763+
XCTFail("identifyUser should not throw when cache is available")
764+
expectation.fulfill()
765+
}
766+
})
767+
768+
wait(for: [expectation], timeout: 100.0)
769+
client.close(callback: nil)
770+
}
717771
}
718772

719773
extension DevCycleClientTest {
@@ -833,4 +887,24 @@ extension DevCycleClientTest {
833887
self.reopenCalled = true
834888
}
835889
}
890+
891+
private class MockCacheServiceWithConfig: CacheServiceProtocol {
892+
private let userConfig: UserConfig
893+
894+
init(userConfig: UserConfig) {
895+
self.userConfig = userConfig
896+
}
897+
898+
func setAnonUserId(anonUserId: String) {}
899+
func getAnonUserId() -> String? { return nil }
900+
func clearAnonUserId() {}
901+
func saveConfig(user: DevCycleUser, configToSave: Data?) {}
902+
func getConfig(user: DevCycleUser) -> UserConfig? {
903+
return self.userConfig
904+
}
905+
func getOrCreateAnonUserId() -> String {
906+
return "mock-anon-id"
907+
}
908+
func migrateLegacyCache() {}
909+
}
836910
}

DevCycleTests/Networking/DevCycleServiceTests.swift

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -135,12 +135,7 @@ class DevCycleServiceTests: XCTestCase {
135135

136136
extension DevCycleServiceTests {
137137
class MockCacheService: CacheServiceProtocol {
138-
var loadCalled = false
139138
var saveConfigCalled = false
140-
func load(user: DevCycleUser) -> Cache {
141-
self.loadCalled = true
142-
return Cache(config: nil, anonUserId: nil)
143-
}
144139

145140
func setAnonUserId(anonUserId: String) {
146141
// TODO: update implementation for tests

0 commit comments

Comments
 (0)