Skip to content

feat: use cached config if avail on init with openfeature provider#287

Merged
luxscious merged 5 commits intomainfrom
ICP-2248-use-cached-config-if-avail-on-init-openfeature
Apr 16, 2026
Merged

feat: use cached config if avail on init with openfeature provider#287
luxscious merged 5 commits intomainfrom
ICP-2248-use-cached-config-if-avail-on-init-openfeature

Conversation

@luxscious
Copy link
Copy Markdown
Contributor

@luxscious luxscious commented Apr 7, 2026

Implements open-feature/protocol#64 — cache-first initialization for static-context
providers.

feat: cache-first initialization

  • SDK initializes immediately from cached config when available, fetches fresh values in the background
  • Cache miss path unchanged — blocks on network as before
  • Evaluations served from cache surface reason = "CACHED"

feat: OpenFeature provider events

  • PROVIDER_READY emitted immediately on cache hit
  • PROVIDER_CONFIGURATION_CHANGED emitted when background refresh delivers fresh values
  • PROVIDER_ERROR emitted on auth/config errors (400/401/403) during background refresh

feat: cache management

  • Persisted cache cleared on auth/config errors so next cold start fetches fresh
  • User switch clears in-memory config immediately to prevent serving stale values for the wrong identity

fix: HTTP status code classification

  • 403 and other unmapped codes were incorrectly collapsed to 500 and treated as retryable — now classified
    correctly

@luxscious luxscious force-pushed the ICP-2248-use-cached-config-if-avail-on-init-openfeature branch 6 times, most recently from 66525c1 to 93c4cb3 Compare April 8, 2026 16:04
@luxscious luxscious marked this pull request as ready for review April 9, 2026 15:49
@luxscious luxscious requested a review from a team as a code owner April 9, 2026 15:49
Copilot AI review requested due to automatic review settings April 9, 2026 15:49
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Implements cache-first initialization for static-context providers, adds OpenFeature provider event support, and refines HTTP status classification to better distinguish auth/config errors from retryable server errors.

Changes:

  • Add persisted-cache management utilities (per-user cache clearing) and expand HTTP response code handling (403/429 + better default mapping).
  • Enable cache-first DevCycleClient initialization with background refresh and config-updated callbacks.
  • Add/extend tests around cache-first init behavior, cached evaluation reason mapping, and shared-prefs cache clearing.

Reviewed changes

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

Show a summary per file
File Description
android-client-sdk/src/main/java/com/devcycle/sdk/android/api/DevCycleClient.kt Cache-first init, background refresh, config-updated callbacks, identity-switch behavior changes
android-client-sdk/src/main/java/com/devcycle/sdk/android/openfeature/DevCycleProvider.kt Adds OpenFeature provider event observation + cached-eval reason mapping
android-client-sdk/src/main/java/com/devcycle/sdk/android/util/DVCSharedPrefs.kt Adds clearConfigForUser for persisted cache management
android-client-sdk/src/main/java/com/devcycle/sdk/android/model/HttpResponseCode.kt Adds 403/429 and adjusts unmapped-code classification
android-client-sdk/src/main/java/com/devcycle/sdk/android/model/EvalReason.kt Adds source and helper for marking cached evaluations
android-client-sdk/src/main/java/com/devcycle/sdk/android/exception/DVCRequestException.kt Carries raw status code + adds auth error classification
android-client-sdk/src/main/java/com/devcycle/sdk/android/api/Request.kt Propagates raw HTTP status into DVCRequestException
android-client-sdk/src/test/java/com/devcycle/sdk/android/api/DevCycleClientTests.kt Adds tests for cache-first init + user-change/cache-clear scenarios (needs fixes)
android-client-sdk/src/test/java/com/devcycle/sdk/android/openfeature/DevCycleProviderTest.kt Adds tests for cache-hit init behavior + cached reason mapping
android-client-sdk/src/test/java/com/devcycle/sdk/android/util/DVCSharedPrefsTests.kt Adds tests for clearing config/expiry per user
android-client-sdk/src/test/java/com/devcycle/sdk/android/model/EvalReasonTests.kt Adds unit tests for EvalReason.withCachedSource

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

Comment thread android-client-sdk/src/main/java/com/devcycle/sdk/android/api/DevCycleClient.kt Outdated
Comment thread android-client-sdk/src/main/java/com/devcycle/sdk/android/api/DevCycleClient.kt Outdated
Comment thread android-client-sdk/src/main/java/com/devcycle/sdk/android/api/DevCycleClient.kt Outdated
@luxscious luxscious force-pushed the ICP-2248-use-cached-config-if-avail-on-init-openfeature branch 2 times, most recently from ca38781 to 705caac Compare April 9, 2026 16:31
Copilot AI review requested due to automatic review settings April 9, 2026 16:31
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 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.

@luxscious luxscious force-pushed the ICP-2248-use-cached-config-if-avail-on-init-openfeature branch from 705caac to 4fe81d7 Compare April 9, 2026 17:06
Copilot AI review requested due to automatic review settings April 9, 2026 17:11
@luxscious luxscious force-pushed the ICP-2248-use-cached-config-if-avail-on-init-openfeature branch from 4fe81d7 to 2c9f270 Compare April 9, 2026 17:11
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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


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

Comment thread android-client-sdk/src/main/java/com/devcycle/sdk/android/api/DevCycleClient.kt Outdated
@luxscious luxscious force-pushed the ICP-2248-use-cached-config-if-avail-on-init-openfeature branch from 2c9f270 to 2cd29c6 Compare April 9, 2026 18:00
Copilot AI review requested due to automatic review settings April 9, 2026 18:45
@luxscious luxscious force-pushed the ICP-2248-use-cached-config-if-avail-on-init-openfeature branch from 2cd29c6 to 938aedb Compare April 9, 2026 18:45
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 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.

Comment thread android-client-sdk/src/main/java/com/devcycle/sdk/android/api/DevCycleClient.kt Outdated
Comment on lines 256 to +262
override fun onError(error: Throwable) {
DevCycleLogger.d("Error fetching config for user_id %s: %s", updatedUser.userId, error.message)

if (error is DVCRequestException && (error.isAuthError || error.statusCode == 400)) {
dvcSharedPrefs.clearConfigForUser(updatedUser)
DevCycleLogger.w("Config error during identifyUser (${error.statusCode}). Persisted cache cleared.")
latestIdentifiedUser = previousUser
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR description says a user switch "clears in-memory config immediately to prevent serving stale values for the wrong identity", but in identifyUser the existing in-memory config (and user used for evaluations/events) is kept until the async fetch completes. That means evaluations right after identifyUser can still be served from the previous user's config. If the intent is to stop serving stale values immediately on user switch, consider clearing/invalidating the in-memory config state (or gating evaluations) at the start of the identify flow.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be fine, as we await for the config for the new identity to succeed before clearing cache

@luxscious luxscious force-pushed the ICP-2248-use-cached-config-if-avail-on-init-openfeature branch from 938aedb to 06742b9 Compare April 9, 2026 19:37
Copilot AI review requested due to automatic review settings April 9, 2026 19:45
@luxscious luxscious force-pushed the ICP-2248-use-cached-config-if-avail-on-init-openfeature branch from 06742b9 to cdabaf0 Compare April 9, 2026 19:45
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 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.

Comment on lines +681 to +685
override fun onSuccess(result: Map<String, BaseConfigVariable>) {
DevCycleLogger.d("Background refresh succeeded")
}

override fun onError(error: Throwable) {
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In performBackgroundRefresh(), you capture userAtRefreshStart, but refetchConfig() always fetches using latestIdentifiedUser (and will queue using whatever latestIdentifiedUser is at queue-time). If the user changes concurrently (e.g., identifyUser runs while the refresh is being scheduled/queued), the refresh request may be executed for a different user than userAtRefreshStart, yet on error you clear the persisted cache for userAtRefreshStart. This can clear the wrong user's cache and misattribute PROVIDER_ERROR/config errors. Consider refactoring so background refresh fetches for a specific user (e.g., pass userAtRefreshStart into refetchConfig/fetchConfig) and uses that same user for any cache-clearing decisions.

Suggested change
override fun onSuccess(result: Map<String, BaseConfigVariable>) {
DevCycleLogger.d("Background refresh succeeded")
}
override fun onError(error: Throwable) {
override fun onSuccess(result: Map<String, BaseConfigVariable>) {
if (latestIdentifiedUser != userAtRefreshStart) {
DevCycleLogger.d("Background refresh completed after user changed; ignoring stale result")
return
}
DevCycleLogger.d("Background refresh succeeded")
}
override fun onError(error: Throwable) {
if (latestIdentifiedUser != userAtRefreshStart) {
DevCycleLogger.w("Background refresh failed after user changed; skipping stale cache/error handling.")
return
}

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this is out of the scope of this PR. It's a pre-existing concurrency edge case in refetchConfig that exists independently of our changes

@luxscious luxscious requested a review from jsalaber April 9, 2026 19:59
@luxscious luxscious merged commit 4fe5c62 into main Apr 16, 2026
6 checks passed
@luxscious luxscious deleted the ICP-2248-use-cached-config-if-avail-on-init-openfeature branch April 16, 2026 11:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants