-
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
base: main
Are you sure you want to change the base?
Changes from all commits
e044023
596bc97
563ec22
dbe5c65
70bd195
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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. | ||||||||||||||||||||
| * <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)); | ||||||||||||||||||||
| } | ||||||||||||||||||||
| }); | ||||||||||||||||||||
|
|
||||||||||||||||||||
| return LDFutures.anyOf(shutdownFuture, resultFuture); | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| @Override | ||||||||||||||||||||
| public void close() { | ||||||||||||||||||||
| shutdownFuture.set(FDv2SourceResult.status(FDv2SourceResult.Status.shutdown(), false)); | ||||||||||||||||||||
| } | ||||||||||||||||||||
| } | ||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -421,10 +421,12 @@ protected LDClient( | |
| taskExecutor | ||
| ); | ||
|
|
||
| boolean usingFDv2 = config.dataSource instanceof FDv2DataSourceBuilder; | ||
| this.contextDataManager = new ContextDataManager( | ||
| clientContextImpl, | ||
| environmentStore, | ||
| config.getMaxCachedContexts() | ||
| config.getMaxCachedContexts(), | ||
| usingFDv2 | ||
| ); | ||
|
|
||
| eventProcessor = config.events.build(clientContextImpl); | ||
|
|
@@ -497,11 +499,17 @@ private void identifyInternal(@NonNull LDContext context, | |
|
|
||
| clientContextImpl = clientContextImpl.setEvaluationContext(context); | ||
|
|
||
| // Calling initFromStoredData updates the current flag state *if* stored flags exist for | ||
| // this context. If they don't, it has no effect. Currently we do *not* return early from | ||
| // initialization just because stored flags exist; we're just making them available in case | ||
| // initialization times out or otherwise fails. | ||
| contextDataManager.switchToContext(context); | ||
| // Load cached flags for the new context so they're available in case initialization | ||
| // times out or otherwise fails. This does not short-circuit initialization — the data | ||
| // source still performs its network request regardless. | ||
| if (config.dataSource instanceof FDv2DataSourceBuilder) { | ||
| // FDv2: just set the context; the FDv2CacheInitializer handles cache loading | ||
| // as the first step in the initializer chain. | ||
| contextDataManager.setCurrentContext(context); | ||
| } else { | ||
| // FDv1: load cached flags immediately while the data source fetches from the network. | ||
| contextDataManager.switchToContext(context); | ||
| } | ||
|
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. We might be able to replace the |
||
| connectivityManager.switchToContext(context, onCompleteListener); | ||
| eventProcessor.recordIdentifyEvent(context); | ||
| } | ||
|
|
||
| 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 { | ||
|
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. Is it acceptable for this interface to be public? It's public because it's been added to the DataSourceBuildInputs class, which is public. As part of the "Inputs" class, it gets passed down into DataSourceBuilder.build(), where it's used to build the cache 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. PerEnvironmentData is the concrete class that implements this interface. |
||
| /** | ||
| * 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); | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OFFLINE mode cache miss causes initialization failure regression
High Severity
Adding the cache initializer to
ConnectionMode.OFFLINEcauses initialization to fail when there's no cached data. Previously, OFFLINE had zero initializers and zero synchronizers, soFDv2DataSource.start()hit thehasAvailableSources()=falsefast path and reported immediate success (VALID). Now, on cache miss the initializer returnsinterrupted,anyDataReceivedstays false, no synchronizers exist, andmaybeReportUnexpectedExhaustionfires — reportingDataSourceState.OFFwith a failure and callingtryCompleteStart(false, ...). This leavesisInitialized()returning false for a mode where the user explicitly chose not to connect.Additional Locations (1)
launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/FDv2CacheInitializer.java#L68-L75Reviewed by Cursor Bugbot for commit 70bd195. Configure here.