-
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
Open
aaron-zeisler
wants to merge
14
commits into
main
Choose a base branch
from
aaronz/SDK-2070/cache-initializer
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
14 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 41ac382
[SDK-2070] Addressing code review: remove synchronizer awareness from…
aaron-zeisler 9e51057
[SDK-2070] Addressing code review: use DataSourceBuildInputsInternal …
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
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
75 changes: 75 additions & 0 deletions
75
...-client-sdk/src/main/java/com/launchdarkly/sdk/android/DataSourceBuildInputsInternal.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,75 @@ | ||
| 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.interfaces.ServiceEndpoints; | ||
| import com.launchdarkly.sdk.android.subsystems.DataSourceBuildInputs; | ||
| import com.launchdarkly.sdk.android.subsystems.HttpConfiguration; | ||
|
|
||
| import java.io.File; | ||
| import java.util.concurrent.ScheduledExecutorService; | ||
|
|
||
| /** | ||
| * Package-private subclass of {@link DataSourceBuildInputs} that carries additional | ||
| * internal-only dependencies not exposed in the public API. | ||
| * <p> | ||
| * This follows the same pattern as {@link ClientContextImpl} extending | ||
| * {@link com.launchdarkly.sdk.android.subsystems.ClientContext}: the public base class | ||
| * defines the stable contract for customer-implemented components, while this subclass | ||
| * adds SDK-internal properties that our built-in components can access via | ||
| * {@link #get(DataSourceBuildInputs)}. | ||
| * <p> | ||
| * This class is for internal SDK use only. It is not subject to any backwards | ||
| * compatibility guarantees. | ||
| */ | ||
| final class DataSourceBuildInputsInternal extends DataSourceBuildInputs { | ||
|
|
||
| @Nullable | ||
| private final PersistentDataStoreWrapper.ReadOnlyPerEnvironmentData perEnvironmentData; | ||
|
|
||
| DataSourceBuildInputsInternal( | ||
| LDContext evaluationContext, | ||
| ServiceEndpoints serviceEndpoints, | ||
| HttpConfiguration http, | ||
| boolean evaluationReasons, | ||
| SelectorSource selectorSource, | ||
| ScheduledExecutorService sharedExecutor, | ||
| @NonNull File cacheDir, | ||
| LDLogger baseLogger, | ||
| @Nullable PersistentDataStoreWrapper.ReadOnlyPerEnvironmentData perEnvironmentData | ||
| ) { | ||
| super(evaluationContext, serviceEndpoints, http, evaluationReasons, | ||
| selectorSource, sharedExecutor, cacheDir, baseLogger); | ||
| this.perEnvironmentData = perEnvironmentData; | ||
| } | ||
|
|
||
| /** | ||
| * Unwraps a {@link DataSourceBuildInputs} to obtain the internal subclass. | ||
| * If the instance is already a {@code DataSourceBuildInputsInternal}, it is | ||
| * returned directly. Otherwise a wrapper is created with null internal fields. | ||
| */ | ||
| static DataSourceBuildInputsInternal get(DataSourceBuildInputs inputs) { | ||
| if (inputs instanceof DataSourceBuildInputsInternal) { | ||
| return (DataSourceBuildInputsInternal) inputs; | ||
| } | ||
| return new DataSourceBuildInputsInternal( | ||
| inputs.getEvaluationContext(), | ||
| inputs.getServiceEndpoints(), | ||
| inputs.getHttp(), | ||
| inputs.isEvaluationReasons(), | ||
| inputs.getSelectorSource(), | ||
| inputs.getSharedExecutor(), | ||
| inputs.getCacheDir(), | ||
| inputs.getBaseLogger(), | ||
| null | ||
| ); | ||
| } | ||
|
|
||
| @Nullable | ||
| PersistentDataStoreWrapper.ReadOnlyPerEnvironmentData getPerEnvironmentDataIfAvailable() { | ||
| return perEnvironmentData; | ||
| } | ||
| } |
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
106 changes: 106 additions & 0 deletions
106
...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,106 @@ | ||||||||||||||||||||
| 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.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.Collections; | ||||||||||||||||||||
| import java.util.Map; | ||||||||||||||||||||
| import java.util.concurrent.Future; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| /** | ||||||||||||||||||||
| * FDv2 cache initializer: loads persisted flag data from the local cache. | ||||||||||||||||||||
| * <p> | ||||||||||||||||||||
| * Per CONNMODE 4.1.2, a cache hit returns data with {@code persist=false} and | ||||||||||||||||||||
| * {@link Selector#EMPTY} (no selector). | ||||||||||||||||||||
| * <p> | ||||||||||||||||||||
| * All non-hit outcomes — cache miss, missing persistent store, and exceptions during | ||||||||||||||||||||
| * cache read — are returned as a {@link ChangeSetType#None} changeset, signaling | ||||||||||||||||||||
| * "no data available" rather than an error. A corrupt or unreadable cache is | ||||||||||||||||||||
| * semantically equivalent to an empty cache: neither provides usable data. | ||||||||||||||||||||
| * <p> | ||||||||||||||||||||
| * The cache read runs synchronously on the caller's thread because the underlying | ||||||||||||||||||||
| * {@code SharedPreferences} access is fast enough that executor dispatch overhead | ||||||||||||||||||||
| * would dominate the total time. | ||||||||||||||||||||
| */ | ||||||||||||||||||||
| 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 PersistentDataStoreWrapper.ReadOnlyPerEnvironmentData envData; | ||||||||||||||||||||
| private final LDContext context; | ||||||||||||||||||||
| private final LDLogger logger; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| FDv2CacheInitializer( | ||||||||||||||||||||
| @Nullable PersistentDataStoreWrapper.ReadOnlyPerEnvironmentData envData, | ||||||||||||||||||||
| @NonNull LDContext context, | ||||||||||||||||||||
| @NonNull LDLogger logger | ||||||||||||||||||||
| ) { | ||||||||||||||||||||
| this.envData = envData; | ||||||||||||||||||||
| this.context = context; | ||||||||||||||||||||
| this.logger = logger; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| @Override | ||||||||||||||||||||
| @NonNull | ||||||||||||||||||||
| public Future<FDv2SourceResult> run() { | ||||||||||||||||||||
| FDv2SourceResult result; | ||||||||||||||||||||
| try { | ||||||||||||||||||||
| if (envData == null) { | ||||||||||||||||||||
| logger.debug("No persistent store configured; skipping cache"); | ||||||||||||||||||||
| result = FDv2SourceResult.changeSet(new ChangeSet<>( | ||||||||||||||||||||
| ChangeSetType.None, | ||||||||||||||||||||
| Selector.EMPTY, | ||||||||||||||||||||
| Collections.emptyMap(), | ||||||||||||||||||||
| null, | ||||||||||||||||||||
| false), false); | ||||||||||||||||||||
| } else { | ||||||||||||||||||||
| String hashedContextId = LDUtil.urlSafeBase64HashedContextId(context); | ||||||||||||||||||||
| EnvironmentData stored = envData.getContextData(hashedContextId); | ||||||||||||||||||||
| if (stored == null) { | ||||||||||||||||||||
| logger.debug("Cache miss for context"); | ||||||||||||||||||||
| result = FDv2SourceResult.changeSet(new ChangeSet<>( | ||||||||||||||||||||
| ChangeSetType.None, | ||||||||||||||||||||
| Selector.EMPTY, | ||||||||||||||||||||
| Collections.emptyMap(), | ||||||||||||||||||||
| null, | ||||||||||||||||||||
| false), false); | ||||||||||||||||||||
| } else { | ||||||||||||||||||||
| Map<String, Flag> flags = stored.getAll(); | ||||||||||||||||||||
| ChangeSet<Map<String, Flag>> changeSet = new ChangeSet<>( | ||||||||||||||||||||
| ChangeSetType.Full, | ||||||||||||||||||||
| Selector.EMPTY, | ||||||||||||||||||||
| flags, | ||||||||||||||||||||
| null, | ||||||||||||||||||||
| false); | ||||||||||||||||||||
| logger.debug("Cache hit: loaded {} flags for context", flags.size()); | ||||||||||||||||||||
| result = FDv2SourceResult.changeSet(changeSet, false); | ||||||||||||||||||||
| } | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
cursor[bot] marked this conversation as resolved.
|
||||||||||||||||||||
| } catch (Exception e) { | ||||||||||||||||||||
| logger.warn("Cache initializer failed: {}", e.toString()); | ||||||||||||||||||||
| result = FDv2SourceResult.changeSet(new ChangeSet<>( | ||||||||||||||||||||
| ChangeSetType.None, | ||||||||||||||||||||
| Selector.EMPTY, | ||||||||||||||||||||
| Collections.emptyMap(), | ||||||||||||||||||||
| null, | ||||||||||||||||||||
| false), false); | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| LDAwaitFuture<FDv2SourceResult> future = new LDAwaitFuture<>(); | ||||||||||||||||||||
| future.set(result); | ||||||||||||||||||||
| return future; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| @Override | ||||||||||||||||||||
| public void close() { | ||||||||||||||||||||
| // No-op: the cache read runs synchronously in run(), so there is nothing to cancel. | ||||||||||||||||||||
| } | ||||||||||||||||||||
| } | ||||||||||||||||||||
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.