Skip to content

Commit e7e11bf

Browse files
fix(map): Emit valueChanged event when there are pending local ops after remote clear (#26010)
When a remote clear operation is received while there are pending local set operations, the map emits a "clear" event but the map still contains keys. This made the clear event unreliable as an indicator of map state, requiring the oracle to re-read all keys after every clear event. This change adds functionality to emit `valueChanged` event for keys which are deleted during remote clears. This way the customers would have to just focus on the `valueChanged` event and not `clear` event for remote ops. [ADO#48665](https://dev.azure.com/fluidframework/internal/_workitems/edit/48665) Follow-up: [ADO#55457](https://dev.azure.com/fluidframework/internal/_workitems/edit/55457)
1 parent 818db6b commit e7e11bf

File tree

3 files changed

+39
-10
lines changed

3 files changed

+39
-10
lines changed

packages/dds/map/src/mapKernel.ts

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -685,8 +685,8 @@ export class MapKernel {
685685
local: boolean,
686686
localOpMetadata: PendingLocalOpMetadata | undefined,
687687
) => {
688-
this.sequencedData.clear();
689688
if (local) {
689+
this.sequencedData.clear();
690690
const pendingClear = this.pendingData.shift();
691691
assert(
692692
pendingClear !== undefined &&
@@ -695,10 +695,35 @@ export class MapKernel {
695695
0xbf6 /* Got a local clear message we weren't expecting */,
696696
);
697697
} else {
698+
const keysToDelete: { key: string; previousValue: unknown }[] = [];
699+
for (const [key, value] of this.sequencedData) {
700+
// Check if this key has pending local operations that supersede the remote op
701+
const hasPendingOps = this.pendingData.some(
702+
(entry) =>
703+
(entry.type === "delete" && entry.key === key) ||
704+
(entry.type === "lifetime" && entry.key === key),
705+
);
706+
// Only collect keys without pending operations (i.e., actually deleted)
707+
if (!hasPendingOps) {
708+
keysToDelete.push({ key, previousValue: value.value });
709+
}
710+
}
711+
this.sequencedData.clear();
712+
698713
// Only emit for remote ops, we would have already emitted for local ops. Only emit if there
699714
// is no optimistically-applied local pending clear that would supersede this remote clear.
700715
if (!this.pendingData.some((entry) => entry.type === "clear")) {
701716
this.eventEmitter.emit("clear", local, this.eventEmitter);
717+
718+
// Emit delete-like valueChanged events for keys that were removed
719+
for (const { key, previousValue } of keysToDelete) {
720+
this.eventEmitter.emit(
721+
"valueChanged",
722+
{ key, previousValue },
723+
local,
724+
this.eventEmitter,
725+
);
726+
}
702727
}
703728
}
704729
},

packages/dds/map/src/test/mapOracle.ts

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -42,13 +42,8 @@ export class SharedMapOracle {
4242
};
4343

4444
private readonly onClear = (local: boolean): void => {
45-
this.oracle.clear();
46-
47-
// AB#48665: https://dev.azure.com/fluidframework/internal/_workitems/edit/48665
48-
if (!local) {
49-
for (const [k, v] of this.fuzzMap.entries()) {
50-
this.oracle.set(k, v);
51-
}
45+
if (local) {
46+
this.oracle.clear();
5247
}
5348
};
5449

packages/dds/map/src/test/mocha/map.spec.ts

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -517,18 +517,27 @@ describe("Map", () => {
517517

518518
containerRuntimeFactory.processSomeMessages(2);
519519

520-
assert.equal(valuesChanged.length, 2);
520+
// After remote clear: emit clear + valueChanged delete events for removed keys
521+
assert.equal(valuesChanged.length, 3, "2 initial sets + 1 delete for map2key");
521522
assert.equal(valuesChanged[0].key, "map1Key");
522523
assert.equal(valuesChanged[0].previousValue, undefined);
523524
assert.equal(valuesChanged[1].key, "map2key");
524525
assert.equal(valuesChanged[1].previousValue, undefined);
526+
assert.equal(valuesChanged[2].key, "map2key", "Delete event for map2key");
527+
assert.equal(
528+
valuesChanged[2].previousValue,
529+
"value2",
530+
"Previous value before clear",
531+
);
525532
assert.equal(clearCount, 1);
526533
assert.equal(map1.size, 1);
527534
assert.equal(map1.get("map1Key"), "value1");
528535

529536
containerRuntimeFactory.processSomeMessages(2);
530537

531-
assert.equal(valuesChanged.length, 2);
538+
assert.equal(valuesChanged.length, 4, "One more delete for map1Key");
539+
assert.equal(valuesChanged[3].key, "map1Key");
540+
assert.equal(valuesChanged[3].previousValue, "value1");
532541
assert.equal(clearCount, 2);
533542
assert.equal(map1.size, 0);
534543
});

0 commit comments

Comments
 (0)