Skip to content

Commit 66c2554

Browse files
authored
feat: Add FDv2 payload application via FlagManager.applyChanges (#299)
## What this adds Two pieces the FDv2 data pipeline (a later PR) needs to apply payloads to the flag store correctly. Nothing produces FDv2 payloads for the flag manager yet, so behavior is unchanged. ### Envelope version is authoritative for flag-eval objects The flag-eval mapper now parses objects with the `put-object` envelope version substituted in, replacing any in-object value. The FDv2 wire format moved versioning onto the envelope; live service objects carry only `flagVersion`: ```json {"key":"greekTest","kind":"flag-eval","version":420, "object":{"flagVersion":7,"value":true,"variation":0,"trackEvents":false}} ``` The previous parse required an in-object `version`, which made every real payload fail with "FDv2 payload contained invalid data." The contract test harness includes an in-object version in its mock data, which is why the suites did not catch it; verified against the live service. This matches the JS implementation, which spreads the object over `version: update.version`. ### `FlagManager.applyChanges` Applies a changeset from an FDv2 payload, handling all three transfer types in one place — matching the JS implementation's `applyChanges(context, updates, type)` — plumbed through `FlagPersistence`/`FlagUpdater`: - **full** — replaces all stored flags. This delegates to the existing init path, so it keeps init's semantics: the context becomes the active context (as with an FDv1 put), changes are reported by diffing against the previous state (including deletions), and the cache is always written — replacing the stored flags with an empty set is a change. `environmentId` is applied on this path. - **partial** — applies the individual updates without per-item version comparison. FDv2 orders data at the payload level, so a lower envelope version must still apply — the existing `upsert` discards it as out-of-order, which is correct for FDv1 patches but wrong here (the harness's "ignores model version" test sends exactly this). Updates are context-checked like `upsert`, a single change event covers the affected keys, and an empty update set changes nothing and skips the cache write. - **none** — takes no action: no store mutation, no events, no cache write. The payload confirms the SDK is already up to date; freshness and status handling stay with the caller. Putting the type dispatch inside the flag manager (rather than having the data pipeline choose between `init` and a partial-only method) keeps the full/partial/none semantics in one self-documenting place and prevents a caller from applying a full payload through the merge path by mistake. ## Known pre-existing divergence, intentionally not addressed here Change detection (`FlagUpdater._hasChanged`) compares evaluation details by **value only** — `LDEvaluationDetail.==` ignores `variationIndex` and `reason` — so a variation- or reason-only change applies to the store without a `FlagsChangedEvent`, despite the event's documentation promising reason changes. This is shared behavior across `init`/`upsert`/`applyChanges` (the JS SDK compares the whole descriptor) and predates this PR; fixing it here would mix a behavior change into otherwise-inert plumbing, so it is left for a separate change. ## Testing Mapper tests pin the live wire shape (no in-object version parses; envelope version wins over a differing in-object value), alongside the existing put/delete/unknown-kind coverage. Flag updater tests cover partial transfers (no version comparison, a single change event for the changed keys, tombstones, rejection for an inactive context), full transfers (replacement with change events for the differences including deletions, making the context active, setting the environment ID), and a transfer of none leaving the store untouched and emitting nothing. Persistence-level tests assert a partial apply writes through to the cache, a rejected apply does not, an empty partial and a transfer of none skip the write, and a full transfer always writes — even an empty one. SDK-2186 <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes core flag-store update semantics for a new FDv2 path (no per-item version on partial applies, full replace behavior); production behavior is unchanged until callers wire it, but mistakes when integrating could cause stale or overwritten flags. > > **Overview** > Prepares the Dart common client for FDv2 by fixing **flag-eval** parsing and adding a single entry point to apply FDv2 changesets to the flag store (not wired from the data pipeline yet). > > **Flag-eval mapper:** When deserializing `flag-eval` objects, the put-object **envelope `version`** is merged into the JSON passed to `LDEvaluationResultSerialization.fromJson`, overriding any in-object `version`. That matches live FDv2 wire data where objects only carry `flagVersion`, which previously caused parse failures. > > **`FlagManager.applyChanges`:** New API plumbed through `FlagPersistence` and `FlagUpdater` for `PayloadType.full`, `partial`, and `none`. **Full** replaces all flags via the existing `init` path (active context, diff-based change events, optional `environmentId`, cache always written). **Partial** merges updates **without** per-item version checks (unlike `upsert`), rejects inactive contexts, batches change notifications, and skips cache when the update map is empty. **None** is a no-op (store, events, cache unchanged). > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 268c45b. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 43d899e commit 66c2554

8 files changed

Lines changed: 452 additions & 4 deletions

File tree

packages/common_client/lib/src/data_sources/fdv2/flag_eval_mapper.dart

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,15 +22,22 @@ Map<String, ItemDescriptor> mapUpdatesToItemDescriptors(List<Update> updates) {
2222
result[update.key] = ItemDescriptor(version: update.version);
2323
} else if (update.object case final object?) {
2424
try {
25-
final evalResult = LDEvaluationResultSerialization.fromJson(object);
25+
final evalResult = LDEvaluationResultSerialization.fromJson({
26+
...object,
27+
// The envelope version is authoritative for FDv2 objects. The
28+
// flag-eval object itself carries only flagVersion -- there is
29+
// no in-object version on the wire -- so the envelope version
30+
// becomes the result's version, replacing any present.
31+
'version': update.version,
32+
});
2633
result[update.key] = ItemDescriptor(
2734
version: update.version,
2835
flag: evalResult,
2936
);
3037
} catch (_) {
31-
// Per spec 4.1.2.1: treat unparseable flag_eval as a data source
32-
// error. Rethrow so the caller can discard the in-progress payload
33-
// and reconnect.
38+
// Treat an unparseable flag_eval as a data source error. Rethrow
39+
// so the caller can discard the in-progress payload and
40+
// reconnect.
3441
rethrow;
3542
}
3643
}

packages/common_client/lib/src/flag_manager/flag_manager.dart

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import 'package:launchdarkly_dart_common/launchdarkly_dart_common.dart';
22

3+
import '../data_sources/fdv2/payload.dart';
34
import '../item_descriptor.dart';
45
import 'flag_store.dart';
56
import 'flag_persistence.dart';
@@ -55,6 +56,17 @@ final class FlagManager {
5556
LDContext context, String key, ItemDescriptor item) async =>
5657
_flagPersistence.upsert(context, key, item);
5758

59+
/// Applies a changeset from an FDv2 payload. A full transfer replaces all
60+
/// stored flags, a partial transfer applies the individual updates without
61+
/// per-item version comparison (FDv2 orders data at the payload level),
62+
/// and a transfer of none takes no action. [environmentId] applies only
63+
/// to full transfers.
64+
Future<bool> applyChanges(LDContext context,
65+
Map<String, ItemDescriptor> updates, PayloadType type,
66+
{String? environmentId}) async =>
67+
_flagPersistence.applyChanges(context, updates, type,
68+
environmentId: environmentId);
69+
5870
/// Asynchronously load cached values from persistence.
5971
Future<bool> loadCached(LDContext context) async {
6072
return _flagPersistence.loadCached(context);

packages/common_client/lib/src/flag_manager/flag_persistence.dart

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import 'dart:convert';
22

33
import 'package:launchdarkly_dart_common/launchdarkly_dart_common.dart';
4+
import '../data_sources/fdv2/payload.dart';
45
import '../item_descriptor.dart';
56
import '../persistence/persistence.dart';
67
import 'context_index.dart';
@@ -63,6 +64,23 @@ final class FlagPersistence {
6364
return false;
6465
}
6566

67+
Future<bool> applyChanges(
68+
LDContext context, Map<String, ItemDescriptor> updates, PayloadType type,
69+
{String? environmentId}) async {
70+
if (_updater.applyChanges(context, updates, type,
71+
environmentId: environmentId)) {
72+
// A transfer of none, or a partial transfer with no updates, changes
73+
// nothing, so the cache write is skipped. A full transfer always
74+
// writes; replacing the stored flags with an empty set is a change.
75+
if (type == PayloadType.full ||
76+
(type == PayloadType.partial && updates.isNotEmpty)) {
77+
await _storeCache(context);
78+
}
79+
return true;
80+
}
81+
return false;
82+
}
83+
6684
Future<bool> loadCached(LDContext context) async {
6785
final json = await _persistence?.read(
6886
_environmentKey, encodePersistenceKey(context.canonicalKey));

packages/common_client/lib/src/flag_manager/flag_updater.dart

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import 'dart:async';
22

33
import 'package:launchdarkly_dart_common/launchdarkly_dart_common.dart';
4+
import '../data_sources/fdv2/payload.dart';
45
import '../item_descriptor.dart';
56
import 'flag_store.dart';
67

@@ -101,6 +102,44 @@ final class FlagUpdater {
101102
return true;
102103
}
103104

105+
/// Applies a changeset from an FDv2 payload. A full transfer replaces all
106+
/// stored flags, a partial transfer applies the individual updates, and a
107+
/// transfer of none takes no action. FDv2 orders data at the payload
108+
/// level, so unlike [upsert] there is no per-item version comparison.
109+
///
110+
/// A full transfer makes [context] the active context; a partial transfer
111+
/// is rejected when [context] is not the active context. [environmentId]
112+
/// applies only to full transfers.
113+
bool applyChanges(
114+
LDContext context, Map<String, ItemDescriptor> updates, PayloadType type,
115+
{String? environmentId}) {
116+
switch (type) {
117+
case PayloadType.full:
118+
init(context, updates, environmentId: environmentId);
119+
return true;
120+
case PayloadType.partial:
121+
if (_activeContextKey != context.canonicalKey) {
122+
_logger.warn('Received an update for an inactive context.');
123+
return false;
124+
}
125+
126+
final changedKeys = <String>[];
127+
for (final MapEntry(key: key, value: item) in updates.entries) {
128+
if (_controller.hasListener &&
129+
_hasChanged(key, _flagStore.get(key), item)) {
130+
changedKeys.add(key);
131+
}
132+
_flagStore.insertOrUpdate(key, item);
133+
}
134+
if (changedKeys.isNotEmpty) {
135+
_sendNotifications(changedKeys);
136+
}
137+
return true;
138+
case PayloadType.none:
139+
return true;
140+
}
141+
}
142+
104143
void close() {
105144
_controller.close();
106145
}

packages/common_client/test/data_sources/fdv2/flag_eval_mapper_test.dart

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,53 @@ void main() {
3030
result['my-flag']!.flag!.detail.value, equals(LDValue.ofBool(true)));
3131
});
3232

33+
test(
34+
'parses an object without an in-object version, using the envelope '
35+
'version', () {
36+
// The live service's flag-eval objects carry only flagVersion; the
37+
// version lives on the put-object envelope.
38+
final updates = [
39+
Update(
40+
kind: flagEvalKind,
41+
key: 'my-flag',
42+
version: 420,
43+
object: {
44+
'flagVersion': 7,
45+
'value': true,
46+
'variation': 0,
47+
'trackEvents': false,
48+
},
49+
),
50+
];
51+
52+
final result = mapUpdatesToItemDescriptors(updates);
53+
54+
expect(result['my-flag']!.version, equals(420));
55+
expect(result['my-flag']!.flag!.version, equals(420));
56+
expect(result['my-flag']!.flag!.flagVersion, equals(7));
57+
expect(
58+
result['my-flag']!.flag!.detail.value, equals(LDValue.ofBool(true)));
59+
});
60+
61+
test('the envelope version replaces a differing in-object version', () {
62+
final updates = [
63+
Update(
64+
kind: flagEvalKind,
65+
key: 'my-flag',
66+
version: 10,
67+
object: {
68+
'value': true,
69+
'version': 3,
70+
'variation': 0,
71+
},
72+
),
73+
];
74+
75+
final result = mapUpdatesToItemDescriptors(updates);
76+
77+
expect(result['my-flag']!.flag!.version, equals(10));
78+
});
79+
3380
test('converts delete update to tombstone', () {
3481
final updates = [
3582
Update(

packages/common_client/test/flag_persistence_test.dart

Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import 'dart:convert';
22

33
import 'package:crypto/crypto.dart';
4+
import 'package:launchdarkly_common_client/src/data_sources/fdv2/payload.dart';
45
import 'package:launchdarkly_common_client/src/flag_manager/flag_persistence.dart';
56
import 'package:launchdarkly_common_client/src/flag_manager/flag_store.dart';
67
import 'package:launchdarkly_common_client/src/flag_manager/flag_updater.dart';
@@ -31,6 +32,150 @@ void main() {
3132
};
3233

3334
group('with persistence', () {
35+
test('it stores cache on a partial applyChanges', () async {
36+
final flagStore = FlagStore();
37+
final mockPersistence = MockPersistence();
38+
final flagPersistence = FlagPersistence(
39+
persistence: mockPersistence,
40+
updater: FlagUpdater(flagStore: flagStore, logger: logger),
41+
store: flagStore,
42+
sdkKey: sdkKey,
43+
maxCachedContexts: 5,
44+
logger: logger,
45+
stamper: () => DateTime.fromMillisecondsSinceEpoch(0));
46+
47+
final context = LDContextBuilder().kind('user', 'user-key').build();
48+
await flagPersistence.init(context, basicData);
49+
50+
final updated = LDEvaluationResult(
51+
version: 3,
52+
detail: LDEvaluationDetail(
53+
LDValue.ofString('updated'), 0, LDEvaluationReason.off()));
54+
expect(
55+
await flagPersistence.applyChanges(
56+
context,
57+
{'flagA': ItemDescriptor(version: 3, flag: updated)},
58+
PayloadType.partial),
59+
true);
60+
61+
final contextPersistenceKey =
62+
sha256.convert(utf8.encode(context.canonicalKey)).toString();
63+
final cached =
64+
mockPersistence.storage[sdkKeyPersistence]![contextPersistenceKey]!;
65+
expect(cached, contains('updated'),
66+
reason: 'a successful apply writes through to the cache');
67+
});
68+
69+
test(
70+
'it does not store cache for an inactive context on a partial '
71+
'applyChanges', () async {
72+
final flagStore = FlagStore();
73+
final mockPersistence = MockPersistence();
74+
final flagPersistence = FlagPersistence(
75+
persistence: mockPersistence,
76+
updater: FlagUpdater(flagStore: flagStore, logger: logger),
77+
store: flagStore,
78+
sdkKey: sdkKey,
79+
maxCachedContexts: 5,
80+
logger: logger,
81+
stamper: () => DateTime.fromMillisecondsSinceEpoch(0));
82+
83+
final context = LDContextBuilder().kind('user', 'user-key').build();
84+
final otherContext =
85+
LDContextBuilder().kind('user', 'other-user-key').build();
86+
await flagPersistence.init(context, basicData);
87+
88+
final updated = LDEvaluationResult(
89+
version: 3,
90+
detail: LDEvaluationDetail(
91+
LDValue.ofString('updated'), 0, LDEvaluationReason.off()));
92+
expect(
93+
await flagPersistence.applyChanges(
94+
otherContext,
95+
{'flagA': ItemDescriptor(version: 3, flag: updated)},
96+
PayloadType.partial),
97+
false);
98+
99+
final otherPersistenceKey =
100+
sha256.convert(utf8.encode(otherContext.canonicalKey)).toString();
101+
expect(mockPersistence.storage[sdkKeyPersistence],
102+
isNot(contains(otherPersistenceKey)),
103+
reason: 'a rejected apply must not write the cache');
104+
});
105+
106+
test('it skips the cache write for an empty partial applyChanges',
107+
() async {
108+
final flagStore = FlagStore();
109+
final mockPersistence = MockPersistence();
110+
final flagPersistence = FlagPersistence(
111+
persistence: mockPersistence,
112+
updater: FlagUpdater(flagStore: flagStore, logger: logger),
113+
store: flagStore,
114+
sdkKey: sdkKey,
115+
maxCachedContexts: 5,
116+
logger: logger,
117+
stamper: () => DateTime.fromMillisecondsSinceEpoch(0));
118+
119+
final context = LDContextBuilder().kind('user', 'user-key').build();
120+
await flagPersistence.init(context, basicData);
121+
final writesAfterInit = mockPersistence.setCallCount;
122+
123+
expect(
124+
await flagPersistence.applyChanges(context, {}, PayloadType.partial),
125+
true);
126+
expect(mockPersistence.setCallCount, writesAfterInit,
127+
reason: 'an empty payload changes nothing');
128+
});
129+
130+
test('it skips the cache write for an applyChanges of none', () async {
131+
final flagStore = FlagStore();
132+
final mockPersistence = MockPersistence();
133+
final flagPersistence = FlagPersistence(
134+
persistence: mockPersistence,
135+
updater: FlagUpdater(flagStore: flagStore, logger: logger),
136+
store: flagStore,
137+
sdkKey: sdkKey,
138+
maxCachedContexts: 5,
139+
logger: logger,
140+
stamper: () => DateTime.fromMillisecondsSinceEpoch(0));
141+
142+
final context = LDContextBuilder().kind('user', 'user-key').build();
143+
await flagPersistence.init(context, basicData);
144+
final writesAfterInit = mockPersistence.setCallCount;
145+
146+
expect(await flagPersistence.applyChanges(context, {}, PayloadType.none),
147+
true);
148+
expect(mockPersistence.setCallCount, writesAfterInit,
149+
reason: 'a transfer of none changes nothing');
150+
});
151+
152+
test('it stores cache for a full applyChanges, even an empty one',
153+
() async {
154+
final flagStore = FlagStore();
155+
final mockPersistence = MockPersistence();
156+
final flagPersistence = FlagPersistence(
157+
persistence: mockPersistence,
158+
updater: FlagUpdater(flagStore: flagStore, logger: logger),
159+
store: flagStore,
160+
sdkKey: sdkKey,
161+
maxCachedContexts: 5,
162+
logger: logger,
163+
stamper: () => DateTime.fromMillisecondsSinceEpoch(0));
164+
165+
final context = LDContextBuilder().kind('user', 'user-key').build();
166+
await flagPersistence.init(context, basicData);
167+
168+
expect(await flagPersistence.applyChanges(context, {}, PayloadType.full),
169+
true);
170+
171+
final contextPersistenceKey =
172+
sha256.convert(utf8.encode(context.canonicalKey)).toString();
173+
final cached =
174+
mockPersistence.storage[sdkKeyPersistence]![contextPersistenceKey]!;
175+
expect(cached, '{}',
176+
reason: 'replacing the stored flags with an empty set is a change '
177+
'and must be persisted');
178+
});
34179
test('it stores cache on init', () async {
35180
final flagStore = FlagStore();
36181
final mockPersistence = MockPersistence();

0 commit comments

Comments
 (0)