Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
f41d71d
feat: add thread safety to EvaluationStorageImpl with NSLock
duyhungtnn Dec 31, 2025
4270d07
fix: indentation
duyhungtnn Dec 31, 2025
e10a481
test(e2e): wrap evaluationStorage assertions in dispatchQueue.sync
duyhungtnn Dec 31, 2025
8e506bc
chore: add detailed locking rationale and improve doc comments
duyhungtnn Jan 3, 2026
6697cca
fix: whitespace in EvaluationStorageImpl.swift
duyhungtnn Jan 3, 2026
7f77261
test: integration test - Thread-safety for concurrent read/write access
duyhungtnn Jan 4, 2026
212ae8e
refactor: EvaluationStorageImpl concurrency model
duyhungtnn Jan 4, 2026
6a61189
chore: Use sync barrier for cache set operation
duyhungtnn Jan 4, 2026
62d80dc
chore: remove redundant comments and clarify test expectations
duyhungtnn Jan 4, 2026
1308def
chore: improve concurrency comments and test error handling
duyhungtnn Jan 4, 2026
25bd059
test: add concurrency tests for InMemoryCache
duyhungtnn Jan 4, 2026
46999d9
chore: add comments explaining cache access in getBy method
duyhungtnn Jan 4, 2026
65a5e60
chore: wrap setUserAttributesUpdated in execute block for thread safety
duyhungtnn Jan 6, 2026
42fad03
revert: call setUserAttributesUpdated without execute wrapper
duyhungtnn Jan 6, 2026
230aabf
chore: add versioning to userAttributesUpdated flag
duyhungtnn Jan 7, 2026
2623adb
test: add concurrency test for setUserAttributesUpdated
duyhungtnn Jan 7, 2026
64d09fb
test: improve concurrency test for user attributes update
duyhungtnn Jan 7, 2026
3fb3996
fix: race condition in MockEvaluationStorage lock usage
duyhungtnn Jan 7, 2026
ed3916b
chore: clarify threading and versioning in evaluation storage
duyhungtnn Jan 7, 2026
f60f41b
refactor: async test for user attribute updates
duyhungtnn Jan 7, 2026
14df540
refactor: user attributes update state handling
duyhungtnn Jan 9, 2026
26b65b4
chore: clarify UserAttributesState documentation
duyhungtnn Jan 9, 2026
0d29aa1
refactor: userAttributesUpdated to userAttributesState
duyhungtnn Jan 9, 2026
5fa480a
chore: add documentation for userAttributesUpdatedVersion
duyhungtnn Jan 9, 2026
2162f33
refactor: user attributes clear logic to use state snapshot
duyhungtnn Jan 9, 2026
de12249
fix: e2e random fail because cache data
duyhungtnn Jan 9, 2026
8fb0bcc
Remove outdated comment on userAttributesUpdated race condition
duyhungtnn Jan 13, 2026
2402c99
Make MockDefaults thread-safe with concurrent queue
duyhungtnn Jan 13, 2026
7d2fbaf
Clarify UserAttributesState versioning and cleanup code
duyhungtnn Jan 13, 2026
6aecf11
Clarify comments on versioning and race conditions
duyhungtnn Jan 13, 2026
dcd7cd1
refactor: evaluation user defaults key management
duyhungtnn Jan 19, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions Bucketeer/Sources/Internal/Evaluation/EvaluationInteractor.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
}
Expand Down Expand Up @@ -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.
Expand Down
18 changes: 14 additions & 4 deletions Bucketeer/Sources/Internal/Evaluation/EvaluationMemCacheDao.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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]
}
}
}

Expand Down
36 changes: 34 additions & 2 deletions Bucketeer/Sources/Internal/Evaluation/EvaluationStorage.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
52 changes: 45 additions & 7 deletions Bucketeer/Sources/Internal/Evaluation/EvaluationStorageImpl.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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],
Expand All @@ -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],
Expand Down Expand Up @@ -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
Expand All @@ -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
}
}
}
Original file line number Diff line number Diff line change
@@ -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) {
Expand All @@ -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)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
1 change: 1 addition & 0 deletions Bucketeer/Sources/Internal/Utils/Defaults.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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 {}
2 changes: 1 addition & 1 deletion BucketeerTests/BKTClientTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading