refactor: evaluation user defaults key management

duyhungtnn · duyhungtnn · commit dcd7cd1d016b · 2026-01-19T21:21:59.000+07:00
Introduced EvaluationUserDefaultsKey enum to centralize user defaults keys and updated references throughout the codebase. Added UserDefaults.removeAllEvaluationData() helper for test cleanup. Updated tests and mocks to use new key management and helper method, improving maintainability and reducing duplication.
diff --git a/Bucketeer/Sources/Internal/Evaluation/EvaluationStorageImpl.swift b/Bucketeer/Sources/Internal/Evaluation/EvaluationStorageImpl.swift
@@ -48,6 +48,8 @@ final class EvaluationStorageImpl: EvaluationStorage {
         evaluationMemCacheDao.get(key: userId) ?? []
     }
 
+    /// Deletes all evaluations and inserts new evaluations in storage.
+    /// - Note: Caller must ensure this is called from the SDK queue. Not thread-safe
     func deleteAllAndInsert(
         evaluationId: String,
         evaluations: [Evaluation],
@@ -63,6 +65,8 @@ final class EvaluationStorageImpl: EvaluationStorage {
         evaluationMemCacheDao.set(key: userId, value: evaluations)
     }
 
+    /// Updates evaluations in storage.
+    /// - Note: Caller must ensure this is called from the SDK queue. Not thread-safe for concurrent writes.
     func update(
         evaluationId: String ,
         evaluations: [Evaluation],
diff --git a/Bucketeer/Sources/Internal/Evaluation/EvaluationUserDefaultDaoImpl.swift b/Bucketeer/Sources/Internal/Evaluation/EvaluationUserDefaultDaoImpl.swift
@@ -1,10 +1,7 @@
 import Foundation
 
 class EvaluationUserDefaultDaoImpl: EvaluationUserDefaultsDao {
-    private static let userEvaluationsIdKey = "bucketeer_user_evaluations_id"
-    private static let featureTagKey = "bucketeer_feature_tag"
-    private static let evaluatedAtKey = "bucketeer_evaluated_at"
-    private static let userAttributesUpdatedKey = "bucketeer_user_attributes_updated"
+
     private let defs: Defaults
 
     init(defaults: Defaults) {
@@ -13,34 +10,34 @@ class EvaluationUserDefaultDaoImpl: EvaluationUserDefaultsDao {
 
     var userAttributesUpdated: Bool {
         get {
-            return defs.bool(forKey: Self.userAttributesUpdatedKey)
+            return defs.bool(forKey: EvaluationUserDefaultsKey.userAttributesUpdated.rawValue)
         }
         set {
-            defs.set(newValue, forKey: Self.userAttributesUpdatedKey)
+            defs.set(newValue, forKey: EvaluationUserDefaultsKey.userAttributesUpdated.rawValue)
         }
     }
     var currentEvaluationsId: String {
         get {
-            return defs.string(forKey: Self.userEvaluationsIdKey) ?? ""
+            return defs.string(forKey: EvaluationUserDefaultsKey.userEvaluationsId.rawValue) ?? ""
         }
         set {
-            defs.set(newValue, forKey: Self.userEvaluationsIdKey)
+            defs.set(newValue, forKey: EvaluationUserDefaultsKey.userEvaluationsId.rawValue)
         }
     }
     var featureTag: String {
         get {
-            return defs.string(forKey: Self.featureTagKey) ?? ""
+            return defs.string(forKey: EvaluationUserDefaultsKey.featureTag.rawValue) ?? ""
         }
         set {
-            defs.set(newValue, forKey: Self.featureTagKey)
+            defs.set(newValue, forKey: EvaluationUserDefaultsKey.featureTag.rawValue)
         }
     }
     var evaluatedAt: String {
         get {
-            return defs.string(forKey: Self.evaluatedAtKey) ?? "0"
+            return defs.string(forKey: EvaluationUserDefaultsKey.evaluatedAt.rawValue) ?? "0"
         }
         set {
-            defs.set(newValue, forKey: Self.evaluatedAtKey)
+            defs.set(newValue, forKey: EvaluationUserDefaultsKey.evaluatedAt.rawValue)
         }
     }
 
@@ -59,12 +56,4 @@ class EvaluationUserDefaultDaoImpl: EvaluationUserDefaultsDao {
     func setUserAttributesUpdated(value: Bool) {
         userAttributesUpdated = value
     }
-
-    // Delete all related data for testing purposes
-    func deleteAll() {
-        defs.removeObject(forKey: Self.userEvaluationsIdKey)
-        defs.removeObject(forKey: Self.featureTagKey)
-        defs.removeObject(forKey: Self.evaluatedAtKey)
-        defs.removeObject(forKey: Self.userAttributesUpdatedKey)
-    }
 }
diff --git a/Bucketeer/Sources/Internal/Evaluation/EvaluationUserDefaultsDao.swift b/Bucketeer/Sources/Internal/Evaluation/EvaluationUserDefaultsDao.swift
@@ -11,3 +11,10 @@ protocol EvaluationUserDefaultsDao {
     func setEvaluatedAt(value: String)
     func setUserAttributesUpdated(value: Bool)
 }
+
+enum EvaluationUserDefaultsKey: String {
+    case userEvaluationsId = "bucketeer_user_evaluations_id"
+    case featureTag = "bucketeer_feature_tag"
+    case evaluatedAt = "bucketeer_evaluated_at"
+    case userAttributesUpdated = "bucketeer_user_attributes_updated"
+}
diff --git a/BucketeerTests/E2E/E2EBKTClientForceUpdateTests.swift b/BucketeerTests/E2E/E2EBKTClientForceUpdateTests.swift
@@ -9,7 +9,7 @@ final class E2EBKTClientForceUpdateTests: XCTestCase {
 
     override func setUp() async throws {
         try await super.setUp()
-        EvaluationUserDefaultDaoImpl(defaults: UserDefaults.standard).deleteAll()
+        UserDefaults.removeAllEvaluationData()
     }
 
     @MainActor
@@ -18,7 +18,7 @@ final class E2EBKTClientForceUpdateTests: XCTestCase {
 
         try await BKTClient.shared.flush()
         try BKTClient.destroy()
-        EvaluationUserDefaultDaoImpl(defaults: UserDefaults.standard).deleteAll()
+        UserDefaults.removeAllEvaluationData()
         try FileManager.default.removeItem(at: .database)
     }
 
diff --git a/BucketeerTests/E2E/E2EEvaluationTests.swift b/BucketeerTests/E2E/E2EEvaluationTests.swift
@@ -10,7 +10,7 @@ final class E2EEvaluationTests: XCTestCase {
     override func setUp() async throws {
         try await super.setUp()
 
-        EvaluationUserDefaultDaoImpl(defaults: UserDefaults.standard).deleteAll()
+        UserDefaults.removeAllEvaluationData()
 
         let config = try BKTConfig.e2e()
         let user = try BKTUser.Builder().with(id: USER_ID).build()
@@ -26,7 +26,7 @@ final class E2EEvaluationTests: XCTestCase {
 
         try await BKTClient.shared.flush()
         try BKTClient.destroy()
-        EvaluationUserDefaultDaoImpl(defaults: UserDefaults.standard).deleteAll()
+        UserDefaults.removeAllEvaluationData()
         try FileManager.default.removeItem(at: .database)
     }
 
diff --git a/BucketeerTests/E2E/E2EEventTests.swift b/BucketeerTests/E2E/E2EEventTests.swift
@@ -9,7 +9,7 @@ final class E2EEventTests: XCTestCase {
 
     override func setUp() async throws {
         try await super.setUp()
-        EvaluationUserDefaultDaoImpl(defaults: UserDefaults.standard).deleteAll()
+        UserDefaults.removeAllEvaluationData()
 
         let config = try BKTConfig.e2e()
         let user = try BKTUser.Builder().with(id: USER_ID).build()
@@ -24,7 +24,7 @@ final class E2EEventTests: XCTestCase {
 
         try await BKTClient.shared.flush()
         try BKTClient.destroy()
-        EvaluationUserDefaultDaoImpl(defaults: UserDefaults.standard).deleteAll()
+        UserDefaults.removeAllEvaluationData()
         try FileManager.default.removeItem(at: .database)
     }
 
diff --git a/BucketeerTests/E2E/E2EMetricsEventTests.swift b/BucketeerTests/E2E/E2EMetricsEventTests.swift
@@ -10,7 +10,7 @@ final class E2EMetricsEventTests: XCTestCase {
     override func tearDown() async throws {
         try await super.tearDown()
         try BKTClient.destroy()
-        EvaluationUserDefaultDaoImpl(defaults: UserDefaults.standard).deleteAll()
+        UserDefaults.removeAllEvaluationData()
         try? FileManager.default.removeItem(at: .database)
     }
 
diff --git a/BucketeerTests/E2E/E2ETestHelpers.swift b/BucketeerTests/E2E/E2ETestHelpers.swift
@@ -196,3 +196,13 @@ final class MockAsyncEventUpdateListener: EvaluationUpdateListener {
         mockContinuation?.resume(returning: true)
     }
 }
+
+extension UserDefaults {
+    static func removeAllEvaluationData() {
+        let userDefs = UserDefaults.standard
+        userDefs.removeObject(forKey: EvaluationUserDefaultsKey.userEvaluationsId.rawValue)
+        userDefs.removeObject(forKey: EvaluationUserDefaultsKey.featureTag.rawValue)
+        userDefs.removeObject(forKey: EvaluationUserDefaultsKey.evaluatedAt.rawValue)
+        userDefs.removeObject(forKey: EvaluationUserDefaultsKey.userAttributesUpdated.rawValue)
+    }
+}
diff --git a/BucketeerTests/EvaluationInteractorTests.swift b/BucketeerTests/EvaluationInteractorTests.swift
@@ -83,7 +83,7 @@ final class EvaluationInteractorTests: XCTestCase {
             "currentEvaluationsId should be empty string"
         )
         XCTAssertEqual(
-            storage.userAttributesUpdated,
+            storage.userAttributesState.isUpdated,
             false,
             "userAttributesUpdated should be false"
         )
@@ -119,7 +119,7 @@ final class EvaluationInteractorTests: XCTestCase {
             "currentEvaluationsId should not be a empty string after fetch evaluation success"
         )
         XCTAssertEqual(
-            storage.userAttributesUpdated,
+            storage.userAttributesState.isUpdated,
             false,
             "userAttributesUpdated should be false"
         )
@@ -455,11 +455,10 @@ final class EvaluationInteractorTests: XCTestCase {
         storage.currentEvaluationsId =  baseUserEvaluationsId
         storage.featureTag =  config.featureTag
         storage.evaluatedAt = evaluationCreatedAt
-        storage.userAttributesUpdated = false
         XCTAssertEqual(storage.currentEvaluationsId, baseUserEvaluationsId)
         XCTAssertEqual(storage.featureTag, config.featureTag)
         XCTAssertEqual(storage.evaluatedAt, evaluationCreatedAt)
-        XCTAssertEqual(storage.userAttributesUpdated, false)
+        XCTAssertEqual(storage.userAttributesState.isUpdated, false)
         let api = MockApiClient(
             getEvaluationsHandler: { user, userEvaluationsId, _, condition, completion in
                 XCTAssertEqual(user, .mock1)
@@ -485,13 +484,13 @@ final class EvaluationInteractorTests: XCTestCase {
         XCTAssertEqual(storage.currentEvaluationsId, baseUserEvaluationsId)
         XCTAssertEqual(storage.featureTag, config.featureTag)
         XCTAssertEqual(storage.evaluatedAt, evaluationCreatedAt)
-        XCTAssertEqual(storage.userAttributesUpdated, false)
+        XCTAssertEqual(storage.userAttributesState.isUpdated, false)
 
         interactor.setUserAttributesUpdated()
         XCTAssertEqual(storage.currentEvaluationsId, baseUserEvaluationsId)
         XCTAssertEqual(storage.featureTag, config.featureTag)
         XCTAssertEqual(storage.evaluatedAt, evaluationCreatedAt)
-        XCTAssertEqual(storage.userAttributesUpdated, true)
+        XCTAssertEqual(storage.userAttributesState.isUpdated, true)
 
         interactor.fetch(user: .mock1) { result in
             switch result {
@@ -508,7 +507,7 @@ final class EvaluationInteractorTests: XCTestCase {
         XCTAssertEqual(storage.featureTag, config.featureTag)
         XCTAssertEqual(storage.evaluatedAt, UserEvaluations.mock1.createdAt)
         // because `userAttributesUpdated` == true before fetch new evaluations, now it should be `false`
-        XCTAssertEqual(storage.userAttributesUpdated, false, "userAttributesUpdated should be `false`")
+        XCTAssertEqual(storage.userAttributesState.isUpdated, false, "userAttributesUpdated should be `false`")
 
         wait(for: [expectation], timeout: 1)
     }
@@ -605,7 +604,7 @@ final class EvaluationInteractorTests: XCTestCase {
         XCTAssertEqual(storage.featureTag, config.featureTag)
         XCTAssertEqual(storage.evaluatedAt, UserEvaluations.mock1ForceUpdate.createdAt)
         // because `userAttributesUpdated` == true before fetch new evaluations, now it should be `false`
-        XCTAssertEqual(storage.userAttributesUpdated, false, "userAttributesUpdated should be `false`")
+        XCTAssertEqual(storage.userAttributesState.isUpdated, false, "userAttributesUpdated should be `false`")
 
         wait(for: [expectation], timeout: 0.1)
     }
@@ -676,7 +675,7 @@ final class EvaluationInteractorTests: XCTestCase {
         XCTAssertEqual(storage.featureTag, config.featureTag)
         XCTAssertEqual(storage.evaluatedAt, UserEvaluations.mock1UpsertAndArchivedFeature.createdAt)
         // because `userAttributesUpdated` == true before fetch new evaluations, now it should be `false`
-        XCTAssertEqual(storage.userAttributesUpdated, false, "userAttributesUpdated should be `false`")
+        XCTAssertEqual(storage.userAttributesState.isUpdated, false, "userAttributesUpdated should be `false`")
 
         wait(for: [expectation], timeout: 0.1)
     }
diff --git a/BucketeerTests/EvaluationStorageTests.swift b/BucketeerTests/EvaluationStorageTests.swift
@@ -227,7 +227,6 @@ final class EvaluationStorageTests: XCTestCase {
         storage.setUserAttributesUpdated()
 
         let userAttributesState = storage.userAttributesState
-        let updatedVersion = userAttributesState.version
         storage.setFeatureTag(value: "featureTagForTest")
 
         let result = try storage.update(
diff --git a/BucketeerTests/Mock/MockEvaluationStorage.swift b/BucketeerTests/Mock/MockEvaluationStorage.swift
@@ -30,15 +30,6 @@ final class MockEvaluationStorage: EvaluationStorage {
         }
     }
 
-    var userAttributesUpdated: Bool {
-        get {
-            return evaluationUserDefaultsDao.userAttributesUpdated
-        }
-        set {
-            evaluationUserDefaultsDao.userAttributesUpdated = newValue
-        }
-    }
-
     typealias PutHandler = ((String, [Evaluation]) throws -> Void)
     typealias GetHandler = () throws -> [Evaluation]
     typealias DeleteAllAndInsertHandler = ([Evaluation]) throws -> Void
@@ -56,6 +47,14 @@ final class MockEvaluationStorage: EvaluationStorage {
     var setUserAttributesUpdatedLock = NSLock()
     // protected by setUserAttributesUpdatedLock
     private var userAttributesUpdatedVersion: Int = 0
+    private var userAttributesUpdated: Bool {
+        get {
+            return evaluationUserDefaultsDao.userAttributesUpdated
+        }
+        set {
+            evaluationUserDefaultsDao.userAttributesUpdated = newValue
+        }
+    }
 
     init(userId: String,
          getHandler: GetHandler? = nil,

Original file line number	Diff line number	Diff line change
`@@ -1,10 +1,7 @@`
`1`	`1`	`import Foundation`
`2`	`2`
`3`	`3`	`class EvaluationUserDefaultDaoImpl: EvaluationUserDefaultsDao {`
`4`		`- private static let userEvaluationsIdKey = "bucketeer_user_evaluations_id"`
`5`		`- private static let featureTagKey = "bucketeer_feature_tag"`
`6`		`- private static let evaluatedAtKey = "bucketeer_evaluated_at"`
`7`		`- private static let userAttributesUpdatedKey = "bucketeer_user_attributes_updated"`
	`4`	`+`
`8`	`5`	`private let defs: Defaults`
`9`	`6`
`10`	`7`	`init(defaults: Defaults) {`
`@@ -13,34 +10,34 @@ class EvaluationUserDefaultDaoImpl: EvaluationUserDefaultsDao {`
`13`	`10`
`14`	`11`	`var userAttributesUpdated: Bool {`
`15`	`12`	`get {`
`16`		`- return defs.bool(forKey: Self.userAttributesUpdatedKey)`
	`13`	`+ return defs.bool(forKey: EvaluationUserDefaultsKey.userAttributesUpdated.rawValue)`
`17`	`14`	`}`
`18`	`15`	`set {`
`19`		`- defs.set(newValue, forKey: Self.userAttributesUpdatedKey)`
	`16`	`+ defs.set(newValue, forKey: EvaluationUserDefaultsKey.userAttributesUpdated.rawValue)`
`20`	`17`	`}`
`21`	`18`	`}`
`22`	`19`	`var currentEvaluationsId: String {`
`23`	`20`	`get {`
`24`		`- return defs.string(forKey: Self.userEvaluationsIdKey) ?? ""`
	`21`	`+ return defs.string(forKey: EvaluationUserDefaultsKey.userEvaluationsId.rawValue) ?? ""`
`25`	`22`	`}`
`26`	`23`	`set {`
`27`		`- defs.set(newValue, forKey: Self.userEvaluationsIdKey)`
	`24`	`+ defs.set(newValue, forKey: EvaluationUserDefaultsKey.userEvaluationsId.rawValue)`
`28`	`25`	`}`
`29`	`26`	`}`
`30`	`27`	`var featureTag: String {`
`31`	`28`	`get {`
`32`		`- return defs.string(forKey: Self.featureTagKey) ?? ""`
	`29`	`+ return defs.string(forKey: EvaluationUserDefaultsKey.featureTag.rawValue) ?? ""`
`33`	`30`	`}`
`34`	`31`	`set {`
`35`		`- defs.set(newValue, forKey: Self.featureTagKey)`
	`32`	`+ defs.set(newValue, forKey: EvaluationUserDefaultsKey.featureTag.rawValue)`
`36`	`33`	`}`
`37`	`34`	`}`
`38`	`35`	`var evaluatedAt: String {`
`39`	`36`	`get {`
`40`		`- return defs.string(forKey: Self.evaluatedAtKey) ?? "0"`
	`37`	`+ return defs.string(forKey: EvaluationUserDefaultsKey.evaluatedAt.rawValue) ?? "0"`
`41`	`38`	`}`
`42`	`39`	`set {`
`43`		`- defs.set(newValue, forKey: Self.evaluatedAtKey)`
	`40`	`+ defs.set(newValue, forKey: EvaluationUserDefaultsKey.evaluatedAt.rawValue)`
`44`	`41`	`}`
`45`	`42`	`}`
`46`	`43`
`@@ -59,12 +56,4 @@ class EvaluationUserDefaultDaoImpl: EvaluationUserDefaultsDao {`
`59`	`56`	`func setUserAttributesUpdated(value: Bool) {`
`60`	`57`	`userAttributesUpdated = value`
`61`	`58`	`}`
`62`		`-`
`63`		`- // Delete all related data for testing purposes`
`64`		`- func deleteAll() {`
`65`		`- defs.removeObject(forKey: Self.userEvaluationsIdKey)`
`66`		`- defs.removeObject(forKey: Self.featureTagKey)`
`67`		`- defs.removeObject(forKey: Self.evaluatedAtKey)`
`68`		`- defs.removeObject(forKey: Self.userAttributesUpdatedKey)`
`69`		`- }`
`70`	`59`	`}`
Original file line number	Diff line number	Diff line change
`@@ -9,7 +9,7 @@ final class E2EBKTClientForceUpdateTests: XCTestCase {`
`9`	`9`
`10`	`10`	`override func setUp() async throws {`
`11`	`11`	`try await super.setUp()`
`12`		`- EvaluationUserDefaultDaoImpl(defaults: UserDefaults.standard).deleteAll()`
	`12`	`+ UserDefaults.removeAllEvaluationData()`
`13`	`13`	`}`
`14`	`14`
`15`	`15`	`@MainActor`
`@@ -18,7 +18,7 @@ final class E2EBKTClientForceUpdateTests: XCTestCase {`
`18`	`18`
`19`	`19`	`try await BKTClient.shared.flush()`
`20`	`20`	`try BKTClient.destroy()`
`21`		`- EvaluationUserDefaultDaoImpl(defaults: UserDefaults.standard).deleteAll()`
	`21`	`+ UserDefaults.removeAllEvaluationData()`
`22`	`22`	`try FileManager.default.removeItem(at: .database)`
`23`	`23`	`}`
`24`	`24`
Original file line number	Diff line number	Diff line change
`@@ -10,7 +10,7 @@ final class E2EMetricsEventTests: XCTestCase {`
`10`	`10`	`override func tearDown() async throws {`
`11`	`11`	`try await super.tearDown()`
`12`	`12`	`try BKTClient.destroy()`
`13`		`- EvaluationUserDefaultDaoImpl(defaults: UserDefaults.standard).deleteAll()`
	`13`	`+ UserDefaults.removeAllEvaluationData()`
`14`	`14`	`try? FileManager.default.removeItem(at: .database)`
`15`	`15`	`}`
`16`	`16`
Original file line number	Diff line number	Diff line change
`@@ -196,3 +196,13 @@ final class MockAsyncEventUpdateListener: EvaluationUpdateListener {`
`196`	`196`	`mockContinuation?.resume(returning: true)`
`197`	`197`	`}`
`198`	`198`	`}`
	`199`	`+`
	`200`	`+extension UserDefaults {`
	`201`	`+ static func removeAllEvaluationData() {`
	`202`	`+ let userDefs = UserDefaults.standard`
	`203`	`+ userDefs.removeObject(forKey: EvaluationUserDefaultsKey.userEvaluationsId.rawValue)`
	`204`	`+ userDefs.removeObject(forKey: EvaluationUserDefaultsKey.featureTag.rawValue)`
	`205`	`+ userDefs.removeObject(forKey: EvaluationUserDefaultsKey.evaluatedAt.rawValue)`
	`206`	`+ userDefs.removeObject(forKey: EvaluationUserDefaultsKey.userAttributesUpdated.rawValue)`
	`207`	`+ }`
	`208`	`+}`