feat: thread-safe user attribute update tracking and safe reading/updating of evaluations

[duyhungtnn]

This pull request introduces a version-based concurrency mechanism to track user attribute updates in the evaluation storage system. The main goal is to prevent race conditions that could cause user attribute updates to be lost during concurrent evaluation fetches and updates. The implementation replaces the previous boolean flag with a versioned state object, ensures thread-safety with explicit locking, and updates all related interfaces, implementations, and tests accordingly.

Concurrency and Race Condition Handling:

• Introduced a UserAttributesState data class containing both a boolean flag and a version number to track user attribute updates, ensuring that updates are not lost during concurrent operations. [1] [2]
• Updated EvaluationStorageImpl to use a private lock and version counter for user attribute updates, only clearing the update flag if the version matches the state captured at the start of a fetch, thus preventing race conditions.
• Added a new test class EvaluationStorageConcurrencyTest to verify correct versioning and to ensure that updates are not lost during concurrent fetches and updates.

API and Interface Changes:

• Changed the EvaluationStorage interface: replaced getUserAttributesUpdated() with getUserAttributesState(), and made clearUserAttributesUpdated() accept a UserAttributesState parameter for version-aware clearing.
• Updated all usages in EvaluationInteractor and related classes to use the new versioned state API, passing the captured state to clearUserAttributesUpdated() and using getUserAttributesState() instead of the old boolean method. [1] [2] [3]

Thread Safety Improvements:

• Updated MemCacheImpl and EvaluationStorageImpl to use private lock objects for synchronization, avoiding potential deadlocks or lock contention by not synchronizing on this. [1] [2]

Test Updates and Coverage:

• Modified existing tests to use the new UserAttributesState API and added assertions for the versioned state. [1] [2] [3] [4] [5] [6] [7]
• Updated test mocks and error handling to support the new state-based methods. [1] [2] [3] [4]

These changes collectively make user attribute update tracking robust against concurrency issues and ensure correctness in multi-threaded scenarios.

[Copilot]

Pull request overview

Adds a versioned, thread-safe mechanism for tracking user attribute updates to prevent race conditions during evaluation fetch/clear flows in the Bucketeer Android SDK.

Changes:

• Introduces UserAttributesState (flag + version) and updates EvaluationStorage API/implementation to clear updates only when versions match.
• Updates EvaluationInteractor to capture state for safe clearing after fetch operations.
• Adds/updates unit tests (including a concurrency test) and synchronizes MemCache access.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/evaluation/storage/UserAttributesState.kt	Adds a new state object (updated flag + version) for concurrency-safe tracking.
bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/evaluation/storage/EvaluationStorage.kt	Updates storage interface to support versioned clear + state retrieval.
bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/evaluation/storage/EvaluationStorageImpl.kt	Implements versioned user-attributes tracking with synchronized access.
bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/evaluation/EvaluationInteractor.kt	Captures user attribute state and uses it when clearing update flags after fetches.
bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/cache/MemCache.kt	Adds synchronization to in-memory cache set/get for thread safety.
bucketeer/src/test/kotlin/io/bucketeer/sdk/android/internal/evaluation/storage/EvaluationStorageImplTest.kt	Updates tests to validate version-aware clearing behavior.
bucketeer/src/test/kotlin/io/bucketeer/sdk/android/internal/evaluation/storage/EvaluationStorageConcurrencyTest.kt	Adds concurrency-focused tests for version increments and race-condition behavior.
bucketeer/src/test/kotlin/io/bucketeer/sdk/android/internal/evaluation/EvaluationInteractorCaptureErrorTests.kt	Updates mock storage to satisfy new interface contract.
.github/workflows/copilot-instructions.md	Adds AI assistant instructions documentation.

Comments suppressed due to low confidence (2)

.github/workflows/copilot-instructions.md:162

• The last lines include leftover markup (</content>, <parameter name="filePath">...) and a developer-specific absolute path. This looks like an accidental paste and makes the instructions file misleading/noisy; please remove these artifacts. Also, if the intent is to provide GitHub Copilot custom instructions, the expected location is typically .github/copilot-instructions.md (not under workflows/).

- `.github/workflows/build.yml` - Main build pipeline
- `.github/workflows/e2e.yml` - End-to-end testing</content>

bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/evaluation/EvaluationInteractor.kt:70

• getUserAttributesUpdated() and getUserAttributesState() are read in two separate calls. If setUserAttributesUpdated() is invoked between them, the request can be sent with userAttributesUpdated=false while the captured state reflects the newer update, reintroducing the race this PR is trying to fix. Prefer a single atomic read: fetch userAttributesState first (synchronized) and derive userAttributesUpdated from userAttributesState.userAttributesUpdated for the request condition, using the same captured state for clearing.

    val currentEvaluationsId = evaluationStorage.getCurrentEvaluationId()
    val evaluatedAt = evaluationStorage.getEvaluatedAt()
    val userAttributesUpdated = evaluationStorage.getUserAttributesUpdated()
    val userAttributesState = evaluationStorage.getUserAttributesState()

    val condition =
      UserEvaluationCondition(
        evaluatedAt = evaluatedAt,
        userAttributesUpdated = userAttributesUpdated,
      )

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 10 out of 11 changed files in this pull request and generated 3 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 10 out of 11 changed files in this pull request and generated 1 comment.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 9 out of 10 changed files in this pull request and generated 2 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 11 out of 12 changed files in this pull request and generated 5 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 11 out of 12 changed files in this pull request and generated no new comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[duyhungtnn]

@cre8ivejp please help me to take a look

[cre8ivejp]

The word version seems confusing, and since we don't have version control and it is always reset when the SDK initializes, I was thinking it would be better if you used something like updatedAt instead.
WDYT?

[duyhungtnn]

I agree version is confusing here since it's not persisted and resets on SDK init, and we don't have version control on it.

However, I think updatedAt might also be misleading because this value is a counter (0, 1, 2...) rather than a timestamp.

What about userAttributesUpdateSequence (or just updateSequence in the state object)? This makes it clear that:

• It's a sequence number for ordering updates
• It's session-scoped (resets on init)
• It's used for optimistic concurrency control
  WDYT?