Skip to content

chore: transactional ContextDataManager in prep for FDv2 #322

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.

Sign up for GitHub

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
tanderson-ld wants to merge 2 commits into
base: main
Choose a base branch
from
Open

chore: transactional ContextDataManager in prep for FDv2 #322

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7576985
chore: transactional ContextDataManager in prep for FDv2
tanderson-ld Feb 18, 2026
1e3da90
fixing DataSourceUpdateSinkV2 impl and locking issue
tanderson-ld Feb 19, 2026
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
11 changes: 10 additions & 1 deletion ...ly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ConnectivityManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,14 @@
import com.launchdarkly.logging.LogValues;
import com.launchdarkly.sdk.LDContext;
import com.launchdarkly.sdk.android.subsystems.Callback;
import com.launchdarkly.sdk.android.subsystems.ChangeSet;
import com.launchdarkly.sdk.android.subsystems.ClientContext;
import com.launchdarkly.sdk.android.subsystems.ComponentConfigurer;
import com.launchdarkly.sdk.android.subsystems.DataSource;
import com.launchdarkly.sdk.android.subsystems.DataSourceUpdateSink;
import com.launchdarkly.sdk.android.subsystems.DataSourceUpdateSinkV2;
import com.launchdarkly.sdk.android.subsystems.EventProcessor;
import com.launchdarkly.sdk.internal.fdv2.sources.Selector;

import java.lang.ref.WeakReference;
import java.util.ArrayList;
Expand Down Expand Up @@ -74,7 +77,7 @@ class ConnectivityManager {
// This has two purposes: 1. to decouple the data source implementation from the details of how
// data is stored; 2. to implement additional logic that does not depend on what kind of data
// source we're using, like "if there was an error, update the ConnectionInformation."
private class DataSourceUpdateSinkImpl implements DataSourceUpdateSink {
private class DataSourceUpdateSinkImpl implements DataSourceUpdateSink, DataSourceUpdateSinkV2 {
private final ContextDataManager contextDataManager;

DataSourceUpdateSinkImpl(ContextDataManager contextDataManager) {
Expand All @@ -93,6 +96,12 @@ public void upsert(LDContext context, DataModel.Flag item) {
// Currently, contextDataManager is responsible for firing any necessary flag change events.
}

@Override
public void apply(@NonNull LDContext context, @NonNull ChangeSet changeSet) {
contextDataManager.apply(context, changeSet);
// Currently, contextDataManager is responsible for firing any necessary flag change events.
}

@Override
public void setStatus(ConnectionMode newConnectionMode, Throwable error) {
if (error == null) {
Expand Down
211 changes: 141 additions & 70 deletions ...kly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ContextDataManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,12 @@
import com.launchdarkly.logging.LDLogLevel;
import com.launchdarkly.logging.LDLogger;
import com.launchdarkly.sdk.LDContext;
import com.launchdarkly.sdk.android.subsystems.ChangeSet;
import com.launchdarkly.sdk.android.subsystems.ChangeSetType;
import com.launchdarkly.sdk.android.subsystems.ClientContext;
import com.launchdarkly.sdk.android.subsystems.TransactionalDataStore;
import com.launchdarkly.sdk.android.DataModel.Flag;
import com.launchdarkly.sdk.internal.fdv2.sources.Selector;

import java.util.ArrayList;
import java.util.Collection;
Expand Down Expand Up @@ -38,7 +42,7 @@
* implementation of PersistentDataStore was used to create the PersistentDataStoreWrapper, and
* deferred listener calls are done via the {@link TaskExecutor} abstraction.
*/
final class ContextDataManager {
final class ContextDataManager implements TransactionalDataStore {
private final PersistentDataStoreWrapper.PerEnvironmentData environmentStore;
private final int maxCachedContexts;
private final TaskExecutor taskExecutor;
Expand All @@ -57,6 +61,9 @@ final class ContextDataManager {
@NonNull private volatile EnvironmentData flags = new EnvironmentData();
@NonNull private volatile ContextIndex index;

/** Selector from the last applied changeset that carried one; in-memory only, not persisted. */
@NonNull private volatile Selector currentSelector = Selector.EMPTY;

ContextDataManager(
@NonNull ClientContext clientContext,
@NonNull PersistentDataStoreWrapper.PerEnvironmentData environmentStore,
Expand Down Expand Up @@ -86,7 +93,7 @@ public void switchToContext(@NonNull LDContext context) {
currentContext = context;
}

EnvironmentData storedData = getStoredData(currentContext);
EnvironmentData storedData = getStoredData(context);
if (storedData == null) {
logger.debug("No stored flag data is available for this context");
// here we return to not alter current in memory flag state as
Expand All @@ -96,7 +103,8 @@ public void switchToContext(@NonNull LDContext context) {
}

logger.debug("Using stored flag data for this context");
initDataInternal(context, storedData, false);
// when we switch context, we don't have a selector because we don't currently support persisting the selector.
applyFullData(context, Selector.EMPTY, storedData.getAll(), false);
Copy link

@cursor cursor bot Feb 19, 2026

Choose a reason for hiding this comment

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

Stale selector persists after context switch

Medium Severity

When switchToContext is called, currentSelector is never reset. The comment at line 106 states the intent is to have no selector after switching, but applyFullData is called with Selector.EMPTY, and the if (!selector.isEmpty()) guard prevents the assignment. Additionally, when storedData is null, the method returns early without any selector reset. This leaves a stale selector from the previous context, which could cause incorrect basis requests in FDv2.

Additional Locations (1)

Fix in Cursor Fix in Web

}

/**
Expand All @@ -111,73 +119,8 @@ public void initData(
@NonNull EnvironmentData newData
) {
logger.debug("Initializing with new flag data for this context");
initDataInternal(context, newData, true);
}

private void initDataInternal(
@NonNull LDContext context,
@NonNull EnvironmentData newData,
boolean writeFlagsToPersistentStore
) {

String contextId = LDUtil.urlSafeBase64HashedContextId(context);
String fingerprint = LDUtil.urlSafeBase64Hash(context);
EnvironmentData oldData;
ContextIndex newIndex;

synchronized (lock) {
if (!context.equals(currentContext)) {
// if incoming new data is not for the current context, reject it.
return;
}

oldData = flags;
flags = newData;

if (writeFlagsToPersistentStore) {
List<String> removedContextIds = new ArrayList<>();
newIndex = index.updateTimestamp(contextId, System.currentTimeMillis())
.prune(maxCachedContexts, removedContextIds);
index = newIndex;

for (String removedContextId: removedContextIds) {
environmentStore.removeContextData(removedContextId);
logger.debug("Removed flag data for context {} from persistent store", removedContextId);
}

environmentStore.setContextData(contextId, fingerprint, newData);
environmentStore.setIndex(newIndex);

if (logger.isEnabled(LDLogLevel.DEBUG)) {
logger.debug("Stored context index is now: {}", newIndex.toJson());
}

logger.debug("Updated flag data for context {} in persistent store", contextId);
}
}

// Determine which flags were updated and notify listeners, if any
Set<String> updatedFlagKeys = new HashSet<>();
for (Flag newFlag: newData.values()) {
Flag oldFlag = oldData.getFlag(newFlag.getKey());
if (oldFlag == null || !oldFlag.getValue().equals(newFlag.getValue())) {
// if the flag is new or the value has changed, notify. This logic can be run if
// the context changes, which can result in an evaluation change even if the version
// of the flag stays the same. You will notice this logic slightly differs from
// upsert. Upsert should only be calling to listeners if the value has changed,
// but we left upsert alone out of fear of that we'd uncover bugs in customer code
// if we added conditionals in upsert
updatedFlagKeys.add(newFlag.getKey());
}
}
for (Flag oldFlag: oldData.values()) {
// if old flag is no longer appearing, notify
if (newData.getFlag(oldFlag.getKey()) == null) {
updatedFlagKeys.add(oldFlag.getKey());
}
}
notifyAllFlagsListeners(updatedFlagKeys);
notifyFlagListeners(updatedFlagKeys);
// init data is called in the FDv1 path, which does not have a selector
applyFullData(context, Selector.EMPTY, newData.getAll(), true);
}

/**
Expand Down Expand Up @@ -260,6 +203,134 @@ public boolean upsert(@NonNull LDContext context, @NonNull Flag flag) {
return true;
}

@Override
public void apply(@NonNull LDContext context, @NonNull ChangeSet changeSet) {
switch (changeSet.getType()) {
case Full:
applyFullData(context, changeSet.getSelector(), changeSet.getItems(), changeSet.shouldPersist());
break;
case Partial:
applyPartialData(context, changeSet.getSelector(), changeSet.getItems(), changeSet.shouldPersist());
break;
case None:
default:
break;
Copy link

@cursor cursor bot Feb 19, 2026

Choose a reason for hiding this comment

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

None changeset silently drops its selector

Medium Severity

The ChangeSetType.None documentation explicitly states the changeset "may still carry a selector to store in memory," but the apply method's None case just breaks without updating currentSelector. A None changeset with a non-empty selector will have that selector silently discarded. Both Full and Partial branches update the selector, but None does not.

Additional Locations (1)

Fix in Cursor Fix in Web

}
}

private void applyFullData(
@NonNull LDContext context,
@NonNull Selector selector,
Map<String, Flag> items,
boolean shouldPersist
) {
EnvironmentData newData = EnvironmentData.usingExistingFlagsMap(items);
EnvironmentData oldData;

synchronized (lock) {
if (!context.equals(currentContext)) {
return;
}
if (!selector.isEmpty()) {
currentSelector = selector;
}
oldData = flags;
flags = newData;

if (shouldPersist) {
String contextId = LDUtil.urlSafeBase64HashedContextId(context);
String fingerprint = LDUtil.urlSafeBase64Hash(context);
List<String> removedContextIds = new ArrayList<>();
ContextIndex newIndex = index.updateTimestamp(contextId, System.currentTimeMillis())
.prune(maxCachedContexts, removedContextIds);
index = newIndex;

for (String removedContextId : removedContextIds) {
environmentStore.removeContextData(removedContextId);
logger.debug("Removed flag data for context {} from persistent store", removedContextId);
}

environmentStore.setContextData(contextId, fingerprint, newData);
environmentStore.setIndex(newIndex);

if (logger.isEnabled(LDLogLevel.DEBUG)) {
logger.debug("Stored context index is now: {}", newIndex.toJson());
}
logger.debug("Updated flag data for context {} in persistent store", contextId);
}
}

// Determine which flags were updated and notify listeners, if any.
// If the flag is new or the value has changed, notify. This logic can be run if
// the context changes, which can result in an evaluation change even if the version
// of the flag stays the same. You will notice this logic slightly differs from
// upsert. Upsert should only be calling to listeners if the value has changed,
// but we left upsert alone out of fear that we'd uncover bugs in customer code
// if we added conditionals in upsert.
Set<String> updatedFlagKeys = new HashSet<>();
for (Flag newFlag : newData.values()) {
Flag oldFlag = oldData.getFlag(newFlag.getKey());
if (oldFlag == null || !oldFlag.getValue().equals(newFlag.getValue())) {
updatedFlagKeys.add(newFlag.getKey());
}
}
for (Flag oldFlag : oldData.values()) {
if (newData.getFlag(oldFlag.getKey()) == null) {
updatedFlagKeys.add(oldFlag.getKey());
}
}
notifyAllFlagsListeners(updatedFlagKeys);
notifyFlagListeners(updatedFlagKeys);
}

private void applyPartialData(
@NonNull LDContext context,
@NonNull Selector selector,
Map<String, Flag> items,
boolean shouldPersist
) {
EnvironmentData updatedFlags;
Set<String> updatedFlagKeys = new HashSet<>();

synchronized (lock) {
if (!context.equals(currentContext)) {
return;
}
if (!selector.isEmpty()) {
currentSelector = selector;
}
Map<String, Flag> merged = new HashMap<>(flags.getAll());
for (Map.Entry<String, Flag> entry : items.entrySet()) {
String key = entry.getKey();
Flag incoming = entry.getValue();
Flag existing = merged.get(key);
if (existing == null || existing.getVersion() < incoming.getVersion()) {
merged.put(key, incoming);
updatedFlagKeys.add(key);
}
}
updatedFlags = EnvironmentData.usingExistingFlagsMap(merged);
flags = updatedFlags;

if (shouldPersist) {
String hashedContextId = LDUtil.urlSafeBase64HashedContextId(context);
String fingerprint = LDUtil.urlSafeBase64Hash(context);
environmentStore.setContextData(hashedContextId, fingerprint, updatedFlags);
index = index.updateTimestamp(hashedContextId, System.currentTimeMillis());
environmentStore.setIndex(index);
}
}

notifyAllFlagsListeners(updatedFlagKeys);
notifyFlagListeners(updatedFlagKeys);
}

@Override
@NonNull
public Selector getSelector() {
return currentSelector;
}

public void registerListener(String key, FeatureFlagChangeListener listener) {
Map<FeatureFlagChangeListener, Boolean> backingMap = new ConcurrentHashMap<>();
Set<FeatureFlagChangeListener> newSet = Collections.newSetFromMap(backingMap);
Expand Down
Loading