Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,11 @@ public PersistentDataStoreWrapper.PerEnvironmentData getPerEnvironmentData() {
return throwExceptionIfNull(perEnvironmentData);
}

@Nullable
public PersistentDataStoreWrapper.PerEnvironmentData getPerEnvironmentDataIfAvailable() {
return perEnvironmentData;
}

@Nullable
public TransactionalDataStore getTransactionalDataStore() {
return transactionalDataStore;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,17 +64,39 @@ final class ContextDataManager implements TransactionalDataStore {
/** Selector from the last applied changeset that carried one; in-memory only, not persisted. */
@NonNull private Selector currentSelector = Selector.EMPTY;

/**
* @param skipCacheLoad true when an FDv2 cache initializer will handle loading cached
* flags as the first step in the initializer chain, making the
* cache load in {@link #switchToContext} redundant
*/
ContextDataManager(
@NonNull ClientContext clientContext,
@NonNull PersistentDataStoreWrapper.PerEnvironmentData environmentStore,
int maxCachedContexts
int maxCachedContexts,
boolean skipCacheLoad
) {
this.environmentStore = environmentStore;
this.index = environmentStore.getIndex();
this.maxCachedContexts = maxCachedContexts;
this.taskExecutor = ClientContextImpl.get(clientContext).getTaskExecutor();
this.logger = clientContext.getBaseLogger();
switchToContext(clientContext.getEvaluationContext());
if (skipCacheLoad) {
setCurrentContext(clientContext.getEvaluationContext());
} else {
switchToContext(clientContext.getEvaluationContext());
}
}

/**
* Sets the current context without loading cached data. Used in the FDv2 path where
* the {@code FDv2CacheInitializer} handles cache loading as part of the initializer chain.
*
* @param context the context to switch to
*/
public void setCurrentContext(@NonNull LDContext context) {
synchronized (lock) {
currentContext = context;
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import com.launchdarkly.sdk.android.integrations.PollingSynchronizerBuilder;
import com.launchdarkly.sdk.android.integrations.StreamingSynchronizerBuilder;
import com.launchdarkly.sdk.android.interfaces.ServiceEndpoints;
import com.launchdarkly.sdk.android.subsystems.CachedFlagStore;
import com.launchdarkly.sdk.android.subsystems.DataSourceBuildInputs;
import com.launchdarkly.sdk.android.subsystems.DataSourceBuilder;
import com.launchdarkly.sdk.android.subsystems.Initializer;
Expand Down Expand Up @@ -138,6 +139,18 @@ public Synchronizer build(DataSourceBuildInputs inputs) {
}
}

static final class CacheInitializerBuilderImpl implements DataSourceBuilder<Initializer> {
@Override
public Initializer build(DataSourceBuildInputs inputs) {
return new FDv2CacheInitializer(
inputs.getCachedFlagStore(),
inputs.getEvaluationContext(),
inputs.getSharedExecutor(),
inputs.getBaseLogger()
);
}
}

/**
* Returns a builder for a polling initializer.
* <p>
Expand Down Expand Up @@ -188,6 +201,7 @@ public static StreamingSynchronizerBuilder streamingSynchronizer() {
*/
@NonNull
public static Map<ConnectionMode, ModeDefinition> makeDefaultModeTable() {
DataSourceBuilder<Initializer> cacheInitializer = new CacheInitializerBuilderImpl();
DataSourceBuilder<Initializer> pollingInitializer = pollingInitializer();
DataSourceBuilder<Synchronizer> pollingSynchronizer = pollingSynchronizer();
DataSourceBuilder<Synchronizer> streamingSynchronizer = streamingSynchronizer();
Expand All @@ -202,32 +216,28 @@ public static Map<ConnectionMode, ModeDefinition> makeDefaultModeTable() {

Map<ConnectionMode, ModeDefinition> table = new LinkedHashMap<>();
table.put(ConnectionMode.STREAMING, new ModeDefinition(
// TODO: cacheInitializer — add once implemented
Arrays.asList(/* cacheInitializer, */ pollingInitializer),
Arrays.asList(cacheInitializer, pollingInitializer),
Arrays.asList(streamingSynchronizer, pollingSynchronizer),
fdv1FallbackPollingSynchronizerForeground
));
table.put(ConnectionMode.POLLING, new ModeDefinition(
// TODO: Arrays.asList(cacheInitializer) — add once implemented
Collections.<DataSourceBuilder<Initializer>>emptyList(),
Collections.singletonList(cacheInitializer),
Collections.singletonList(pollingSynchronizer),
fdv1FallbackPollingSynchronizerForeground
));
table.put(ConnectionMode.OFFLINE, new ModeDefinition(
// TODO: Arrays.asList(cacheInitializer) — add once implemented
Collections.<DataSourceBuilder<Initializer>>emptyList(),
Collections.singletonList(cacheInitializer),
Collections.<DataSourceBuilder<Synchronizer>>emptyList(),
null
));
table.put(ConnectionMode.ONE_SHOT, new ModeDefinition(
// TODO: cacheInitializer and streamingInitializer — add once implemented
Arrays.asList(/* cacheInitializer, */ pollingInitializer /*, streamingInitializer, */),
// TODO: streamingInitializer — add once implemented
Arrays.asList(cacheInitializer, pollingInitializer /*, streamingInitializer */),
Collections.<DataSourceBuilder<Synchronizer>>emptyList(),
null
));
table.put(ConnectionMode.BACKGROUND, new ModeDefinition(
// TODO: Arrays.asList(cacheInitializer) — add once implemented
Collections.<DataSourceBuilder<Initializer>>emptyList(),
Copy link
Copy Markdown

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.OFFLINE causes initialization to fail when there's no cached data. Previously, OFFLINE had zero initializers and zero synchronizers, so FDv2DataSource.start() hit the hasAvailableSources()=false fast path and reported immediate success (VALID). Now, on cache miss the initializer returns interrupted, anyDataReceived stays false, no synchronizers exist, and maybeReportUnexpectedExhaustion fires — reporting DataSourceState.OFF with a failure and calling tryCompleteStart(false, ...). This leaves isInitialized() returning false for a mode where the user explicitly chose not to connect.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 70bd195. Configure here.

Collections.singletonList(cacheInitializer),
Collections.singletonList(backgroundPollingSynchronizer),
fdv1FallbackPollingSynchronizerBackground
));
Expand Down
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 {
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.

Here's a summary of how the spec requirements and the js-core implementation were used when developing this class:

Decision Source Reasoning
Selector.EMPTY on cache result CONNMODE 4.1.2 Cache is unverified; empty selector tells the orchestrator to continue to the polling initializer for a real selector
persist=false on ChangeSet CONNMODE 4.1.2 Don't re-write data we just read from cache
Cache miss = interrupted js-core pattern Fast failure so the orchestrator immediately moves on; interrupted is the correct signal (not terminalError, which would stop the chain)
fdv1Fallback=false always Logic Cache is local storage, no server headers are involved
Nullable cachedFlagStore Testing pragmatism Test contexts don't have a persistent store; graceful degradation avoids test setup burden


@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
Expand Up @@ -2,6 +2,7 @@

import androidx.annotation.NonNull;

import com.launchdarkly.sdk.android.subsystems.CachedFlagStore;
import com.launchdarkly.sdk.android.subsystems.ClientContext;
import com.launchdarkly.sdk.android.subsystems.ComponentConfigurer;
import com.launchdarkly.sdk.android.subsystems.DataSource;
Expand Down Expand Up @@ -149,18 +150,30 @@ public void close() {
}

private DataSourceBuildInputs makeInputs(ClientContext clientContext) {
TransactionalDataStore store = ClientContextImpl.get(clientContext).getTransactionalDataStore();
ClientContextImpl impl = ClientContextImpl.get(clientContext);
TransactionalDataStore store = impl.getTransactionalDataStore();
SelectorSource selectorSource = store != null
? new SelectorSourceFacade(store)
: () -> com.launchdarkly.sdk.fdv2.Selector.EMPTY;

PersistentDataStoreWrapper.PerEnvironmentData envData = impl.getPerEnvironmentDataIfAvailable();
CachedFlagStore cachedFlagStore = envData != null
? context -> {
String hashedId = LDUtil.urlSafeBase64HashedContextId(context);
EnvironmentData stored = envData.getContextData(hashedId);
return stored != null ? stored.getAll() : null;
}
: null;

return new DataSourceBuildInputs(
clientContext.getEvaluationContext(),
clientContext.getServiceEndpoints(),
clientContext.getHttp(),
clientContext.isEvaluationReasons(),
selectorSource,
sharedExecutor,
ClientContextImpl.get(clientContext).getPlatformState().getCacheDir(),
impl.getPlatformState().getCacheDir(),
cachedFlagStore,
clientContext.getBaseLogger()
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}
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.

We might be able to replace the if (config.dataSource instanceof FDv2DataSourceBuilder) { with something from ConnectivityManager here. I think ConnectivityManager has an internal variable that describes "FDv1 or FDv2?", but that internal variable isn't exposed via a getter function. Let me know if you think that's cleaner.

connectivityManager.switchToContext(context, onCompleteListener);
eventProcessor.recordIdentifyEvent(context);
}
Expand Down
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 {
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.

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.

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.

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);
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.launchdarkly.sdk.android.subsystems;

import androidx.annotation.NonNull;
import androidx.annotation.Nullable;

import com.launchdarkly.logging.LDLogger;
import com.launchdarkly.sdk.LDContext;
Expand Down Expand Up @@ -31,6 +32,8 @@ public final class DataSourceBuildInputs {
private final SelectorSource selectorSource;
private final ScheduledExecutorService sharedExecutor;
private final File cacheDir;
@Nullable
private final CachedFlagStore cachedFlagStore;
private final LDLogger baseLogger;

/**
Expand All @@ -44,6 +47,8 @@ public final class DataSourceBuildInputs {
* @param sharedExecutor shared executor for scheduling tasks; owned and shut down by
* the calling data source, so components must not shut it down
* @param cacheDir the platform's cache directory for HTTP-level caching
* @param cachedFlagStore read access to cached flag data, or null if no persistent
* store is configured
* @param baseLogger the base logger instance
*/
public DataSourceBuildInputs(
Expand All @@ -54,6 +59,7 @@ public DataSourceBuildInputs(
SelectorSource selectorSource,
ScheduledExecutorService sharedExecutor,
@NonNull File cacheDir,
@Nullable CachedFlagStore cachedFlagStore,
LDLogger baseLogger
) {
this.evaluationContext = evaluationContext;
Expand All @@ -63,6 +69,7 @@ public DataSourceBuildInputs(
this.selectorSource = selectorSource;
this.sharedExecutor = sharedExecutor;
this.cacheDir = cacheDir;
this.cachedFlagStore = cachedFlagStore;
this.baseLogger = baseLogger;
}

Expand Down Expand Up @@ -133,6 +140,17 @@ public File getCacheDir() {
return cacheDir;
}

/**
* Returns read access to cached flag data, or null if no persistent store
* is configured. Used by the cache initializer to load stored flags.
*
* @return the cached flag store, or null
*/
@Nullable
public CachedFlagStore getCachedFlagStore() {
return cachedFlagStore;
}

/**
* Returns the base logger instance.
*
Expand Down
Loading
Loading