feat: thread-safe user attribute update tracking and safe reading/updating of evaluations (#116)

Author: duyhungtnn

Bucketeer/Sources/Internal/Evaluation/EvaluationInteractor.swift

@@ -47,9 +47,11 @@ final class EvaluationInteractorImpl: EvaluationInteractor {
 
         let logger = self.logger
         let evaluatedAt = evaluationStorage.evaluatedAt
-        let userAttributesUpdated = evaluationStorage.userAttributesUpdated
+        let userAttributesState = evaluationStorage.userAttributesState
+        let userAttributesUpdated = userAttributesState.isUpdated
         let currentEvaluationsId = evaluationStorage.currentEvaluationsId
         let featureTag = evaluationStorage.featureTag
+
         apiClient.getEvaluations(
             user: user,
             userEvaluationsId: currentEvaluationsId,
@@ -62,8 +64,8 @@ final class EvaluationInteractorImpl: EvaluationInteractor {
                 let newEvaluationsId = response.userEvaluationsId
                 if currentEvaluationsId == newEvaluationsId {
                     logger?.debug(message: "Nothing to sync")
-                    // Reset UserAttributesUpdated
-                    self?.evaluationStorage.clearUserAttributesUpdated()
+                    // Clear logic is now encapsulated in `evaluationStorage` via the state snapshot
+                    self?.evaluationStorage.clearUserAttributesUpdated(state: userAttributesState)
                     completion?(result)
                     return
                 }
@@ -97,7 +99,9 @@ final class EvaluationInteractorImpl: EvaluationInteractor {
                     return
                 }
 
-                self?.evaluationStorage.clearUserAttributesUpdated()
+                // Clear logic is now encapsulated in `evaluationStorage` via the state snapshot
+                self?.evaluationStorage.clearUserAttributesUpdated(state: userAttributesState)
+
                 if shouldNotifyListener {
                     // Update listeners should be called on the main thread
                     // to avoid unintentional lock on Interactor's execution thread.

Bucketeer/Sources/Internal/Evaluation/EvaluationMemCacheDao.swift

@@ -4,17 +4,27 @@ protocol KeyValueCache {
     func get(key: String) -> DataCacheType?
 }
 
-class InMemoryCache<T:Any> : KeyValueCache {
+class InMemoryCache<T: Any>: KeyValueCache {
     typealias DataCacheType = T
 
-    private var dict: [String : T] = [:]
+    private var dict: [String: T] = [:]
+    // A concurrent queue allows multiple reads to execute in parallel
+    private let queue = DispatchQueue(label: "io.bucketeer.InMemoryCache", attributes: .concurrent)
 
     func set(key: String, value: T) {
-        dict[key] = value
+        // .barrier ensures exclusive access: it waits for pending reads to finish and blocks new ones during the write.
+        // .sync ensures the update completes before returning, guaranteeing that subsequent reads immediately receive the new value.
+        queue.sync(flags: .barrier) {
+            self.dict[key] = value
+        }
     }
 
     func get(key: String) -> T? {
-        return dict[key]
+        // .sync waits for this read block to run, then returns its value.
+        // Reads can run in parallel on the concurrent queue, but this call will wait if a barrier write is in progress or queued ahead of it.
+        queue.sync {
+            return dict[key]
+        }
     }
 }
 

Bucketeer/Sources/Internal/Evaluation/EvaluationStorage.swift

@@ -20,10 +20,42 @@ protocol EvaluationStorage {
     var featureTag: String { get }
     // expected set evaluatedAt from `deleteAllAndInsert` or `update` only
     var evaluatedAt: String { get }
-    var userAttributesUpdated: Bool { get }
+
+    // Current version and flag set when `setUserAttributesUpdated()` is called
+    var userAttributesState: UserAttributesState { get }
 
     func clearCurrentEvaluationsId()
     func setFeatureTag(value: String)
     func setUserAttributesUpdated()
-    func clearUserAttributesUpdated()
+
+    /// Atomically clear the user-attributes-updated flag if the stored version equals `state.version`.
+    /// - Parameter state: Snapshot obtained from `userAttributesState` before a network request.
+    /// - Returns: `true` if the flag was cleared (stored flag was `true` and versions matched); `false` otherwise.
+    /// - Thread-safety: Implementations MUST perform the compare-and-swap under the storage's internal lock.
+    /// - Note: `version` is an in-memory, session-only counter; implementations may persist only the
+    /// boolean flag (e.g., in `UserDefaults`), but the `version` must be treated as transient.
+    @discardableResult func clearUserAttributesUpdated(state: UserAttributesState) -> Bool
+}
+
+/// Snapshot representing the current user-attributes update state for the current app session.
+///
+/// - `version`: Monotonically increasing counter. Incremented each time `setUserAttributesUpdated()`
+///   is called to indicate a new update event. This value is kept in memory only and is not
+///   persisted across app restarts.
+/// - `isUpdated`: `true` if user attributes have been modified since the last evaluation (i.e., a
+///   new update event exists); `false` otherwise. Implementations may persist this flag (for
+///   example, in `UserDefaults`) to survive restarts.
+///
+/// Use this struct as an in-memory snapshot to coordinate whether evaluations must be refreshed
+/// due to user attribute changes during a single session. It is not intended to be persisted as a
+/// whole across app restarts.
+///
+/// The ephemeral nature of the version counter is intentional. We employ an Optimistic Locking pattern where the integer value does not need to persist across app restarts:
+/// - On restart: The persisted `isUpdated` flag is authoritative. If `true`, an immediate sync is triggered, regardless of the version.
+/// - During runtime: The `version` acts as a transaction ID to safely handle race conditions where user attributes change while a background fetch is in progress.
+/// - Correctness: The update flag is only cleared via a compare-and-swap check (`currentVersion == capturedVersion`).
+/// If the user modifies attributes during a fetch, the `version` increments, the check fails, and the flag remains `true` to schedule a subsequent sync.
+struct UserAttributesState {
+    let version: Int
+    let isUpdated: Bool
 }

Bucketeer/Sources/Internal/Evaluation/EvaluationStorageImpl.swift

@@ -12,16 +12,24 @@ final class EvaluationStorageImpl: EvaluationStorage {
         return evaluationUserDefaultsDao.evaluatedAt
     }
 
-    var userAttributesUpdated: Bool {
-        return evaluationUserDefaultsDao.userAttributesUpdated
+    var userAttributesState: UserAttributesState {
+        // read atomically 2 values
+        return setUserAttributesUpdatedLock.withLock {
+            return UserAttributesState(
+                version: userAttributesUpdatedVersion,
+                isUpdated: evaluationUserDefaultsDao.userAttributesUpdated
+            )
+        }
     }
 
     private let userId: String
-    // Expected SQL Dao
     private let evaluationSQLDao: EvaluationSQLDao
-    // Expected in-memory cache Dao
     private let evaluationMemCacheDao: EvaluationMemCacheDao
     private let evaluationUserDefaultsDao: EvaluationUserDefaultsDao
+    private let setUserAttributesUpdatedLock = NSLock()
+    /// Version counter used as an in-memory transaction id for attribute updates.
+    /// Protected by `setUserAttributesUpdatedLock`.
+    private var userAttributesUpdatedVersion: Int = 0
 
     init(
         userId: String,
@@ -40,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],
@@ -55,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],
@@ -87,6 +99,14 @@ final class EvaluationStorageImpl: EvaluationStorage {
 
     // getBy will return the data from the cache to speed up the response time
     func getBy(featureId: String) -> Evaluation? {
+        // evaluationMemCacheDao is thread-safe (uses internal concurrent queue).
+        // We rely on it without adding extra locks even though this storage layer can be accessed from multiple threads:
+        // writes and most operations are serialized on the SDK queue, but reads (like getBy) may be invoked from the
+        // main/UI thread concurrently with background SDK operations.
+        //
+        // We access the memory cache directly without waiting for pending database writes.
+        // If we enforced strict consistency (locking during Disk I/O with SQL), this method would block the calling thread (often the Main Thread), causing UI freezes.
+        // This behavior prioritizes application responsiveness, accepting momentary data staleness during background updates.
         return evaluationMemCacheDao.get(key: userId)?.first { evaluation in
             evaluation.featureId == featureId
         } ?? nil
@@ -106,10 +126,28 @@ final class EvaluationStorageImpl: EvaluationStorage {
     }
 
     func setUserAttributesUpdated() {
-        evaluationUserDefaultsDao.setUserAttributesUpdated(value: true)
+        setUserAttributesUpdatedLock.withLock {
+            // Increment version on every update
+            userAttributesUpdatedVersion += 1
+            evaluationUserDefaultsDao.setUserAttributesUpdated(value: true)
+        }
     }
 
-    func clearUserAttributesUpdated() {
-        evaluationUserDefaultsDao.setUserAttributesUpdated(value: false)
+    // Called from SDK queue (fetch callback)
+    @discardableResult func clearUserAttributesUpdated(state: UserAttributesState) -> Bool {
+        guard state.isUpdated else {
+            // No-op if flag is already false
+            return false
+        }
+        return setUserAttributesUpdatedLock.withLock {
+            // Only clear if the version matches what we captured at the start of the request.
+            // If userAttributesUpdatedVersion > version, it means a new update happened
+            // while the request was in-flight, so we MUST NOT clear the flag.
+            if userAttributesUpdatedVersion == state.version {
+                evaluationUserDefaultsDao.setUserAttributesUpdated(value: false)
+                return true
+            }
+            return false
+        }
     }
 }

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)
         }
     }
 

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"
+}

Bucketeer/Sources/Internal/Utils/Defaults.swift

@@ -4,6 +4,7 @@ protocol Defaults {
     func string(forKey defaultName: String) -> String?
     func bool(forKey defaultName: String) -> Bool
     func set(_ value: Any?, forKey defaultName: String)
+    func removeObject(forKey defaultName: String)
 }
 
 extension UserDefaults: Defaults {}

BucketeerTests/BKTClientTests.swift

@@ -148,7 +148,7 @@ final class BKTClientTests: XCTestCase {
             // mark the next request ready
             requestCount = 2
             XCTAssertEqual(
-                dataModule.evaluationStorage.userAttributesUpdated,
+                dataModule.evaluationStorage.userAttributesState.isUpdated,
                 true, "userAttributesUpdated should be true")
             client.fetchEvaluations(timeoutMillis: nil) { error in
                 XCTAssertEqual(error, nil)