-
Notifications
You must be signed in to change notification settings - Fork 23
chore: FDv2 Cache Initializer #342
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 5 commits
Commits
Show all changes
24 commits
Select commit
Hold shift + click to select a range
e044023
[SDK-2070] feat: add CachedFlagStore to DataSourceBuildInputs for cac…
aaron-zeisler 596bc97
[SDK-2070] feat: implement FDv2CacheInitializer and CacheInitializerB…
aaron-zeisler 563ec22
[SDK-2070] feat: wire cache initializer into the default mode table
aaron-zeisler dbe5c65
[SDK-2070] Added comments where cache initialization is redundant
aaron-zeisler 70bd195
[SDK-2070] refactor: skip redundant cache load in FDv2 path
aaron-zeisler f856058
[SDK-2070] refactor: replace CachedFlagStore with ReadOnlyPerEnvironm…
aaron-zeisler 24c962f
[SDK-2070] fix: return ChangeSetType.None on cache miss instead of in…
aaron-zeisler e1617fe
Merge branch 'main' into aaronz/SDK-2070/cache-initializer
tanderson-ld 7b5ed27
[SDK-2070] fix: defer init completion to synchronizers and treat cach…
aaron-zeisler 6aeea41
[SDK-2070] Addressing code review comments
aaron-zeisler b8a50de
[SDK-2070] Merge branch 'aaronz/SDK-2070/cache-initializer' of github…
aaron-zeisler 3f0066b
[SDK-2070] Run FDv2CacheInitializer synchronously to eliminate execut…
aaron-zeisler 8f5353a
[SDK-2070] Add isRequiredBeforeStartup() marker to Initializer interface
aaron-zeisler 4e9746d
[SDK-2070] Build initializers eagerly in FDv2DataSourceBuilder
aaron-zeisler b9b1851
[SDK-2070] Update SourceManager to accept pre-built initializers with…
aaron-zeisler 4a82402
[SDK-2070] Add two-pass initializer execution in FDv2DataSource
aaron-zeisler e8caa9f
[SDK-2070] Add tests for pre-startup initializer execution
aaron-zeisler 41ac382
[SDK-2070] Addressing code review: remove synchronizer awareness from…
aaron-zeisler 9e51057
[SDK-2070] Addressing code review: use DataSourceBuildInputsInternal …
aaron-zeisler 3050faf
Merge branch 'main' into aaronz/SDK-2070/cache-initializer
aaron-zeisler 2bdaf5a
[SDK-2070] Merge branch 'aaronz/SDK-2070/cache-initializer-guarantee-…
aaron-zeisler 5e9fa17
[SDK-2070] Split cache initializers from general initializers in FDv2…
aaron-zeisler 9fbd135
[SDK-2070] chore: remove unused imports from branch-edited files
aaron-zeisler fc931a8
[SDK-2070] Addressing code review comments
aaron-zeisler File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
99 changes: 99 additions & 0 deletions
99
...y-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/FDv2CacheInitializer.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,99 @@ | ||||||||||||||||||||
| package com.launchdarkly.sdk.android; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| import androidx.annotation.NonNull; | ||||||||||||||||||||
| import androidx.annotation.Nullable; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| import com.launchdarkly.logging.LDLogger; | ||||||||||||||||||||
| import com.launchdarkly.sdk.LDContext; | ||||||||||||||||||||
| import com.launchdarkly.sdk.android.DataModel.Flag; | ||||||||||||||||||||
| import com.launchdarkly.sdk.android.subsystems.CachedFlagStore; | ||||||||||||||||||||
| import com.launchdarkly.sdk.android.subsystems.FDv2SourceResult; | ||||||||||||||||||||
| import com.launchdarkly.sdk.android.subsystems.Initializer; | ||||||||||||||||||||
| import com.launchdarkly.sdk.fdv2.ChangeSet; | ||||||||||||||||||||
| import com.launchdarkly.sdk.fdv2.ChangeSetType; | ||||||||||||||||||||
| import com.launchdarkly.sdk.fdv2.Selector; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| import java.util.Map; | ||||||||||||||||||||
| import java.util.concurrent.Executor; | ||||||||||||||||||||
| import java.util.concurrent.Future; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| /** | ||||||||||||||||||||
| * FDv2 cache initializer: loads persisted flag data from the local cache as the first | ||||||||||||||||||||
| * step in the initializer chain. | ||||||||||||||||||||
| * <p> | ||||||||||||||||||||
| * Per CONNMODE 4.1.2, the cache initializer returns data with {@code persist=false} | ||||||||||||||||||||
| * and {@link Selector#EMPTY} (no selector), so the orchestrator continues to the next | ||||||||||||||||||||
| * initializer (polling) to obtain a verified selector from the server. This provides | ||||||||||||||||||||
| * immediate flag values from cache while the network initializer fetches fresh data. | ||||||||||||||||||||
|
aaron-zeisler marked this conversation as resolved.
Outdated
|
||||||||||||||||||||
| * <p> | ||||||||||||||||||||
| * A cache miss is reported as an {@link FDv2SourceResult.Status#interrupted} status, | ||||||||||||||||||||
| * causing the orchestrator to move to the next initializer without delay. | ||||||||||||||||||||
| */ | ||||||||||||||||||||
| final class FDv2CacheInitializer implements Initializer { | ||||||||||||||||||||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here's a summary of how the spec requirements and the js-core implementation were used when developing this class:
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| @Nullable | ||||||||||||||||||||
| private final CachedFlagStore cachedFlagStore; | ||||||||||||||||||||
| private final LDContext context; | ||||||||||||||||||||
| private final Executor executor; | ||||||||||||||||||||
| private final LDLogger logger; | ||||||||||||||||||||
| private final LDAwaitFuture<FDv2SourceResult> shutdownFuture = new LDAwaitFuture<>(); | ||||||||||||||||||||
|
|
||||||||||||||||||||
| FDv2CacheInitializer( | ||||||||||||||||||||
| @Nullable CachedFlagStore cachedFlagStore, | ||||||||||||||||||||
| @NonNull LDContext context, | ||||||||||||||||||||
| @NonNull Executor executor, | ||||||||||||||||||||
| @NonNull LDLogger logger | ||||||||||||||||||||
| ) { | ||||||||||||||||||||
| this.cachedFlagStore = cachedFlagStore; | ||||||||||||||||||||
| this.context = context; | ||||||||||||||||||||
| this.executor = executor; | ||||||||||||||||||||
| this.logger = logger; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| @Override | ||||||||||||||||||||
| @NonNull | ||||||||||||||||||||
| public Future<FDv2SourceResult> run() { | ||||||||||||||||||||
| LDAwaitFuture<FDv2SourceResult> resultFuture = new LDAwaitFuture<>(); | ||||||||||||||||||||
|
|
||||||||||||||||||||
| executor.execute(() -> { | ||||||||||||||||||||
| try { | ||||||||||||||||||||
| if (cachedFlagStore == null) { | ||||||||||||||||||||
| logger.debug("No persistent store configured; skipping cache"); | ||||||||||||||||||||
| resultFuture.set(FDv2SourceResult.status( | ||||||||||||||||||||
| FDv2SourceResult.Status.interrupted( | ||||||||||||||||||||
| new LDFailure("No persistent store", LDFailure.FailureType.UNKNOWN_ERROR)), | ||||||||||||||||||||
| false)); | ||||||||||||||||||||
| return; | ||||||||||||||||||||
| } | ||||||||||||||||||||
| Map<String, Flag> flags = cachedFlagStore.getCachedFlags(context); | ||||||||||||||||||||
| if (flags == null) { | ||||||||||||||||||||
| logger.debug("Cache miss for context"); | ||||||||||||||||||||
| resultFuture.set(FDv2SourceResult.status( | ||||||||||||||||||||
| FDv2SourceResult.Status.interrupted( | ||||||||||||||||||||
| new LDFailure("No cached data", LDFailure.FailureType.UNKNOWN_ERROR)), | ||||||||||||||||||||
| false)); | ||||||||||||||||||||
| return; | ||||||||||||||||||||
| } | ||||||||||||||||||||
| ChangeSet<Map<String, Flag>> changeSet = new ChangeSet<>( | ||||||||||||||||||||
| ChangeSetType.Full, | ||||||||||||||||||||
| Selector.EMPTY, | ||||||||||||||||||||
| flags, | ||||||||||||||||||||
| null, | ||||||||||||||||||||
| false); | ||||||||||||||||||||
| logger.debug("Cache hit: loaded {} flags for context", flags.size()); | ||||||||||||||||||||
| resultFuture.set(FDv2SourceResult.changeSet(changeSet, false)); | ||||||||||||||||||||
| } catch (Exception e) { | ||||||||||||||||||||
| logger.warn("Cache initializer failed: {}", e.toString()); | ||||||||||||||||||||
| resultFuture.set(FDv2SourceResult.status( | ||||||||||||||||||||
| FDv2SourceResult.Status.interrupted(e), false)); | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
cursor[bot] marked this conversation as resolved.
|
||||||||||||||||||||
| }); | ||||||||||||||||||||
|
aaron-zeisler marked this conversation as resolved.
Outdated
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| return LDFutures.anyOf(shutdownFuture, resultFuture); | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| @Override | ||||||||||||||||||||
| public void close() { | ||||||||||||||||||||
| shutdownFuture.set(FDv2SourceResult.status(FDv2SourceResult.Status.shutdown(), false)); | ||||||||||||||||||||
| } | ||||||||||||||||||||
| } | ||||||||||||||||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
27 changes: 27 additions & 0 deletions
27
...oid-client-sdk/src/main/java/com/launchdarkly/sdk/android/subsystems/CachedFlagStore.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| package com.launchdarkly.sdk.android.subsystems; | ||
|
|
||
| import androidx.annotation.Nullable; | ||
|
|
||
| import com.launchdarkly.sdk.LDContext; | ||
| import com.launchdarkly.sdk.android.DataModel; | ||
|
|
||
| import java.util.Map; | ||
|
|
||
| /** | ||
| * Provides read access to cached flag data for a specific evaluation context. | ||
| * <p> | ||
| * This interface bridges the persistence layer with FDv2 data source builders, | ||
| * allowing the cache initializer to load stored flags without depending on | ||
| * package-private types. | ||
| */ | ||
| public interface CachedFlagStore { | ||
|
aaron-zeisler marked this conversation as resolved.
Outdated
|
||
| /** | ||
| * Returns the cached flag data for the given context, or null if no | ||
| * cached data exists. | ||
| * | ||
| * @param context the evaluation context to look up | ||
| * @return the cached flags, or null on cache miss | ||
| */ | ||
| @Nullable | ||
| Map<String, DataModel.Flag> getCachedFlags(LDContext context); | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.