From 40c572d48e64d3f38e4e38a413eb628edd1cb3b1 Mon Sep 17 00:00:00 2001 From: Sonali Deshpande <48232592+sonalideshpandemsft@users.noreply.github.com> Date: Mon, 18 Aug 2025 15:28:30 +0000 Subject: [PATCH 01/11] wip --- .../dds/cell/src/test/cell.rollback.spec.ts | 5 + .../src/test/sharedString.rollback.spec.ts | 127 ++++++++++++++++++ 2 files changed, 132 insertions(+) create mode 100644 packages/dds/cell/src/test/cell.rollback.spec.ts create mode 100644 packages/dds/sequence/src/test/sharedString.rollback.spec.ts diff --git a/packages/dds/cell/src/test/cell.rollback.spec.ts b/packages/dds/cell/src/test/cell.rollback.spec.ts new file mode 100644 index 000000000000..f5fd565afe47 --- /dev/null +++ b/packages/dds/cell/src/test/cell.rollback.spec.ts @@ -0,0 +1,5 @@ +/*! + * Copyright (c) Microsoft Corporation and contributors. All rights reserved. + * Licensed under the MIT License. + */ + diff --git a/packages/dds/sequence/src/test/sharedString.rollback.spec.ts b/packages/dds/sequence/src/test/sharedString.rollback.spec.ts new file mode 100644 index 000000000000..bf0526cd548c --- /dev/null +++ b/packages/dds/sequence/src/test/sharedString.rollback.spec.ts @@ -0,0 +1,127 @@ +/*! + * Copyright (c) Microsoft Corporation and contributors. All rights reserved. + * Licensed under the MIT License. + */ + +import { strict as assert } from "assert"; + +import { AttachState } from "@fluidframework/container-definitions/internal"; +import { + MockContainerRuntimeFactory, + MockFluidDataStoreRuntime, + MockStorage, +} from "@fluidframework/test-runtime-utils/internal"; + +import { SharedStringFactory } from "../sequenceFactory.js"; +import { SharedStringClass } from "../sharedString.js"; + +function setupSharedStringRollbackTest() { + const containerRuntimeFactory = new MockContainerRuntimeFactory({ flushMode: 1 }); // TurnBased + const dataStoreRuntime = new MockFluidDataStoreRuntime({ clientId: "1" }); + const containerRuntime = containerRuntimeFactory.createContainerRuntime(dataStoreRuntime); + const sharedString = new SharedStringClass( + dataStoreRuntime, + "shared-string-1", + SharedStringFactory.Attributes, + ); + dataStoreRuntime.setAttachState(AttachState.Attached); + sharedString.initializeLocal(); + sharedString.connect({ + deltaConnection: dataStoreRuntime.createDeltaConnection(), + objectStorage: new MockStorage(), + }); + return { + sharedString, + dataStoreRuntime, + containerRuntimeFactory, + containerRuntime, + }; +} + +describe("SharedString with rollback", () => { + it("should rollback text insert operation", () => { + const { sharedString, containerRuntimeFactory, containerRuntime } = + setupSharedStringRollbackTest(); + + // Initial text + sharedString.insertText(0, "abc"); + assert.equal(sharedString.getText(), "abc", "text after insert"); + // containerRuntime.flush(); + containerRuntimeFactory.processAllMessages(); + + // Insert new text we plan to roll back + sharedString.insertText(3, "XYZ"); + assert.equal(sharedString.getText(), "abcXYZ", "text after insert"); + + // Rollback should revert the insert + containerRuntime.rollback?.(); + assert.equal(sharedString.getText(), "", "text reverted after rollback"); + }); + + it("can replace text with rollback", async () => { + const { sharedString, containerRuntimeFactory, containerRuntime } = + setupSharedStringRollbackTest(); + sharedString.insertText(0, "hello world"); + + sharedString.replaceText(6, 11, "there!"); + assert.equal(sharedString.getText(), "hello there!", "Could not replace text"); + + containerRuntimeFactory.processAllMessages(); + + sharedString.replaceText(0, 5, "hi"); + assert.equal(sharedString.getText(), "hi there!", "Could not replace text at beginning"); + + containerRuntime.rollback?.(); + assert.equal(sharedString.getText(), "", "text reverted after rollback"); + }); + + it("can remove text with rollback", async () => { + const { sharedString, containerRuntimeFactory, containerRuntime } = + setupSharedStringRollbackTest(); + sharedString.insertText(0, "hello world"); + + sharedString.removeText(5, 11); + assert.equal(sharedString.getText(), "hello", "Could not remove text"); + + sharedString.removeText(0, 3); + assert.equal(sharedString.getText(), "lo", "Could not remove text from beginning"); + containerRuntimeFactory.processAllMessages(); + + containerRuntime.rollback?.(); + + assert.equal(sharedString.getText(), "", "text reverted after rollback"); + }); + + it("can annotate the text", async () => { + const { sharedString, containerRuntimeFactory, containerRuntime } = + setupSharedStringRollbackTest(); + const text = "hello world"; + const styleProps = { style: "bold" }; + sharedString.insertText(0, text, styleProps); + + for (let i = 0; i < text.length; i++) { + assert.deepEqual( + { ...sharedString.getPropertiesAtPosition(i) }, + { ...styleProps }, + "Could not add props", + ); + } + + const colorProps = { color: "green" }; + sharedString.annotateRange(6, text.length, colorProps); + + for (let i = 6; i < text.length; i++) { + assert.deepEqual( + { ...sharedString.getPropertiesAtPosition(i) }, + { ...styleProps, ...colorProps }, + "Could not annotate props", + ); + } + + containerRuntimeFactory.processAllMessages(); + + containerRuntime.rollback?.(); + + assert.equal(sharedString.getText(), "", "text reverted after rollback"); + }); +}); From 4078565419a6db9bf8b06ac5805d81b16c4fc9ef Mon Sep 17 00:00:00 2001 From: Sonali Deshpande <48232592+sonalideshpandemsft@users.noreply.github.com> Date: Tue, 19 Aug 2025 17:20:37 +0000 Subject: [PATCH 02/11] Add basic tests --- .../dds/cell/src/test/cell.rollback.spec.ts | 5 - .../src/test/sharedString.rollback.spec.ts | 511 ++++++++++++++++-- 2 files changed, 469 insertions(+), 47 deletions(-) delete mode 100644 packages/dds/cell/src/test/cell.rollback.spec.ts diff --git a/packages/dds/cell/src/test/cell.rollback.spec.ts b/packages/dds/cell/src/test/cell.rollback.spec.ts deleted file mode 100644 index f5fd565afe47..000000000000 --- a/packages/dds/cell/src/test/cell.rollback.spec.ts +++ /dev/null @@ -1,5 +0,0 @@ -/*! - * Copyright (c) Microsoft Corporation and contributors. All rights reserved. - * Licensed under the MIT License. - */ - diff --git a/packages/dds/sequence/src/test/sharedString.rollback.spec.ts b/packages/dds/sequence/src/test/sharedString.rollback.spec.ts index bf0526cd548c..a5dda21bd11b 100644 --- a/packages/dds/sequence/src/test/sharedString.rollback.spec.ts +++ b/packages/dds/sequence/src/test/sharedString.rollback.spec.ts @@ -10,13 +10,14 @@ import { MockContainerRuntimeFactory, MockFluidDataStoreRuntime, MockStorage, + type MockContainerRuntime, } from "@fluidframework/test-runtime-utils/internal"; import { SharedStringFactory } from "../sequenceFactory.js"; import { SharedStringClass } from "../sharedString.js"; function setupSharedStringRollbackTest() { - const containerRuntimeFactory = new MockContainerRuntimeFactory({ flushMode: 1 }); // TurnBased + const containerRuntimeFactory = new MockContainerRuntimeFactory({ flushMode: 1 }); const dataStoreRuntime = new MockFluidDataStoreRuntime({ clientId: "1" }); const containerRuntime = containerRuntimeFactory.createContainerRuntime(dataStoreRuntime); const sharedString = new SharedStringClass( @@ -38,90 +39,516 @@ function setupSharedStringRollbackTest() { }; } -describe("SharedString with rollback", () => { - it("should rollback text insert operation", () => { +// Helper to create another client attached to the same containerRuntimeFactory +function createAdditionalClient( + containerRuntimeFactory: MockContainerRuntimeFactory, + id: string = "client-2", +): { + sharedString: SharedStringClass; + dataStoreRuntime: MockFluidDataStoreRuntime; + containerRuntime: MockContainerRuntime; +} { + const dataStoreRuntime = new MockFluidDataStoreRuntime({ clientId: id }); + const containerRuntime = containerRuntimeFactory.createContainerRuntime(dataStoreRuntime); + const sharedString = new SharedStringClass( + dataStoreRuntime, + `shared-string-${id}`, + SharedStringFactory.Attributes, + ); + dataStoreRuntime.setAttachState(AttachState.Attached); + sharedString.initializeLocal(); + sharedString.connect({ + deltaConnection: dataStoreRuntime.createDeltaConnection(), + objectStorage: new MockStorage(), + }); + return { sharedString, dataStoreRuntime, containerRuntime }; +} + +describe("SharedString rollback change events", () => { + it("rollback insert emits remove", () => { const { sharedString, containerRuntimeFactory, containerRuntime } = setupSharedStringRollbackTest(); - // Initial text + // Apply insert we will rollback sharedString.insertText(0, "abc"); - assert.equal(sharedString.getText(), "abc", "text after insert"); - // containerRuntime.flush(); containerRuntimeFactory.processAllMessages(); + assert.equal(sharedString.getText(), "abc", "text after insert"); - // Insert new text we plan to roll back - sharedString.insertText(3, "XYZ"); - assert.equal(sharedString.getText(), "abcXYZ", "text after insert"); - - // Rollback should revert the insert containerRuntime.rollback?.(); + + // Expect rollback event to be a remove assert.equal(sharedString.getText(), "", "text reverted after rollback"); }); - it("can replace text with rollback", async () => { + it("rollback remove emits insert", () => { const { sharedString, containerRuntimeFactory, containerRuntime } = setupSharedStringRollbackTest(); - sharedString.insertText(0, "hello world"); + sharedString.insertText(0, "hello"); + containerRuntimeFactory.processAllMessages(); + containerRuntime.flush(); + assert.equal(sharedString.getText(), "hello"); + + // Apply remove we will rollback + sharedString.removeText(0, 5); + assert.equal(sharedString.getText(), "", "text after remove"); - sharedString.replaceText(6, 11, "there!"); - assert.equal(sharedString.getText(), "hello there!", "Could not replace text"); + containerRuntime.rollback?.(); + + // Expect rollback event to be an insert + assert.equal(sharedString.getText(), "hello", "rollback discards remove"); + }); +}); + +describe("SharedString rollback with multiple clients (insert/remove)", () => { + it("Client1 insert + Client2 insert + rollback on Client1", () => { + const { + sharedString: client1, + containerRuntimeFactory, + containerRuntime: cr1, + } = setupSharedStringRollbackTest(); + const { sharedString: client2, containerRuntime: cr2 } = createAdditionalClient( + containerRuntimeFactory, + "2", + ); + // Baseline text + client1.insertText(0, "hello"); + cr1.flush(); containerRuntimeFactory.processAllMessages(); - sharedString.replaceText(0, 5, "hi"); - assert.equal(sharedString.getText(), "hi there!", "Could not replace text at beginning"); + assert.equal(client1.getText(), "hello"); + assert.equal(client2.getText(), "hello"); - containerRuntime.rollback?.(); - assert.equal(sharedString.getText(), "", "text reverted after rollback"); + // Both clients make local edits (pending) + client1.insertText(5, " world"); // pending on Client1 + client2.insertText(5, " there"); // pending on Client2 + + // Before processing, remote should not see each other's pending edits + assert.equal(client1.getText(), "hello world"); + assert.equal(client2.getText(), "hello there"); + + // Process messages to synchronize + cr1.flush(); + cr2.flush(); + containerRuntimeFactory.processAllMessages(); + + // Both clients see each other's committed edits + assert.equal(client1.getText(), "hello there world"); + assert.equal(client2.getText(), "hello there world"); + + // Rollback pending edits on Client1 (which were already flushed locally) + // To illustrate rollback, add another local insert + client1.insertText(17, "!"); + assert.equal(client1.getText(), "hello there world!"); + + cr1.rollback?.(); + assert.equal( + client1.getText(), + "hello there world", + "rollback discards Client1 pending insert", + ); + assert.equal(client2.getText(), "hello there world", "remote unchanged"); }); - it("can remove text with rollback", async () => { - const { sharedString, containerRuntimeFactory, containerRuntime } = - setupSharedStringRollbackTest(); - sharedString.insertText(0, "hello world"); + it("Client1 remove + Client2 insert + rollback on Client1", () => { + const { + sharedString: client1, + containerRuntimeFactory, + containerRuntime: cr1, + } = setupSharedStringRollbackTest(); + const { sharedString: client2, containerRuntime: cr2 } = createAdditionalClient( + containerRuntimeFactory, + "2", + ); + + // Baseline text + client1.insertText(0, "abcdef"); + cr1.flush(); + containerRuntimeFactory.processAllMessages(); + + assert.equal(client2.getText(), "abcdef"); + + // Client1 removes locally (pending) + client1.removeText(0, 3); // "abc" removed locally + assert.equal(client1.getText(), "def"); + assert.equal(client2.getText(), "abcdef"); - sharedString.removeText(5, 11); - assert.equal(sharedString.getText(), "hello", "Could not remove text"); + // Client2 inserts locally (pending) + client2.insertText(3, "XYZ"); // adds at position 3 + assert.equal(client2.getText(), "abcXYZdef"); + assert.equal(client1.getText(), "def"); - sharedString.removeText(0, 3); - assert.equal(sharedString.getText(), "lo", "Could not remove text from beginning"); + // Flush both and process + cr1.flush(); + cr2.flush(); containerRuntimeFactory.processAllMessages(); - containerRuntime.rollback?.(); + // Texts converge + assert.equal(client1.getText(), "XYZdef"); + assert.equal(client2.getText(), "XYZdef"); - assert.equal(sharedString.getText(), "", "text reverted after rollback"); + // Rollback Client1 pending removes (none left) just to confirm no crash + cr1.rollback?.(); + assert.equal(client1.getText(), "XYZdef"); + assert.equal(client2.getText(), "XYZdef"); }); - it("can annotate the text", async () => { - const { sharedString, containerRuntimeFactory, containerRuntime } = - setupSharedStringRollbackTest(); + it("Client1 insert + Client2 remove + rollback on Client1", () => { + const { + sharedString: client1, + containerRuntimeFactory, + containerRuntime: cr1, + } = setupSharedStringRollbackTest(); + const { sharedString: client2, containerRuntime: cr2 } = createAdditionalClient( + containerRuntimeFactory, + "2", + ); + + // Baseline text + client1.insertText(0, "123456"); + cr1.flush(); + containerRuntimeFactory.processAllMessages(); + + // Client1 inserts locally (pending) + client1.insertText(6, "ABC"); + assert.equal(client1.getText(), "123456ABC"); + assert.equal(client2.getText(), "123456"); + + // Client2 removes some text locally (pending) + client2.removeText(2, 4); // removes "34" + assert.equal(client2.getText(), "1256"); + assert.equal(client1.getText(), "123456ABC"); + + // Rollback Client1 pending insert (if any) + cr1.rollback?.(); + assert.equal(client1.getText(), "123456", "rollback discards pending insert"); + + // Flush both and process + cr1.flush(); + cr2.flush(); + containerRuntimeFactory.processAllMessages(); + + assert.equal(client1.getText(), "1256", "rollback removes Client1 insert"); + assert.equal(client2.getText(), "1256", "remote unchanged after rollback"); + }); + + it("Client1 insert + Client2 insert + rollback on Client2", () => { + const { + sharedString: client1, + containerRuntimeFactory, + containerRuntime: cr1, + } = setupSharedStringRollbackTest(); + const { sharedString: client2, containerRuntime: cr2 } = createAdditionalClient( + containerRuntimeFactory, + "2", + ); + + // Baseline text + client1.insertText(0, "hello"); + cr1.flush(); + containerRuntimeFactory.processAllMessages(); + + assert.equal(client1.getText(), "hello"); + assert.equal(client2.getText(), "hello"); + + // Both clients make local edits (pending) + client1.insertText(5, " world"); // pending on Client1 + client2.insertText(5, " there"); // pending on Client2 + + // Before processing, remote should not see each other's pending edits + assert.equal(client1.getText(), "hello world"); + assert.equal(client2.getText(), "hello there"); + + // Process messages to synchronize + cr1.flush(); + cr2.flush(); + containerRuntimeFactory.processAllMessages(); + + // Both clients see each other's committed edits + assert.equal(client1.getText(), "hello there world"); + assert.equal(client2.getText(), "hello there world"); + + // Rollback pending edits on Client1 (which were already flushed locally) + // To illustrate rollback, add another local insert + client2.insertText(17, "!"); + assert.equal(client2.getText(), "hello there world!"); + + cr2.rollback?.(); + assert.equal( + client2.getText(), + "hello there world", + "rollback discards Client2 pending insert", + ); + assert.equal(client1.getText(), "hello there world", "remote unchanged"); + }); +}); + +describe("SharedString replaceText with rollback and two clients", () => { + it("Client1 replaceText + rollback without remote changes", () => { + const { + sharedString: client1, + containerRuntimeFactory, + containerRuntime: cr1, + } = setupSharedStringRollbackTest(); + const { sharedString: client2 } = createAdditionalClient(containerRuntimeFactory, "2"); + + // Baseline text + client1.insertText(0, "hello world"); + cr1.flush(); + containerRuntimeFactory.processAllMessages(); + assert.equal(client2.getText(), "hello world"); + + // Client1 replaces text locally (pending) + client1.replaceText(6, 11, "there!"); + assert.equal(client1.getText(), "hello there!"); + assert.equal(client2.getText(), "hello world"); + + // Rollback pending replace + cr1.rollback?.(); + assert.equal(client1.getText(), "hello world", "rollback restores original text"); + assert.equal(client2.getText(), "hello world", "remote unchanged"); + }); + + it("Client1 multiple replaceText + rollback", () => { + const { + sharedString: client1, + containerRuntimeFactory, + containerRuntime: cr1, + } = setupSharedStringRollbackTest(); + const { sharedString: client2 } = createAdditionalClient(containerRuntimeFactory, "2"); + + client1.insertText(0, "hello world"); + cr1.flush(); + containerRuntimeFactory.processAllMessages(); + + // Multiple local replacements + client1.replaceText(6, 11, "there!"); + client1.replaceText(0, 5, "hi"); + assert.equal(client1.getText(), "hi there!"); + assert.equal(client2.getText(), "hello world"); + + // Rollback all pending replaces + cr1.rollback?.(); + assert.equal(client1.getText(), "hello world", "rollback restores all replaced text"); + assert.equal(client2.getText(), "hello world", "remote unchanged"); + }); + + it("Client1 replaceText with concurrent remote remove", () => { + const { + sharedString: client1, + containerRuntimeFactory, + containerRuntime: cr1, + } = setupSharedStringRollbackTest(); + const { sharedString: client2, containerRuntime: cr2 } = createAdditionalClient( + containerRuntimeFactory, + "2", + ); + + // Baseline text + client1.insertText(0, "abcdef"); + cr1.flush(); + containerRuntimeFactory.processAllMessages(); + + // Client1 replaces locally (pending) + client1.replaceText(2, 4, "XY"); // "abXYef" + assert.equal(client1.getText(), "abXYef"); + + // Client2 removes text concurrently (pending) + client2.removeText(1, 3); // removes "bX" in their local view + assert.equal(client2.getText(), "adef"); + + // Rollback Client1 before flushing + cr1.rollback?.(); + assert.equal(client1.getText(), "abcdef", "rollback restores original text on Client1"); + + // Flush both and process messages + cr1.flush(); + cr2.flush(); + containerRuntimeFactory.processAllMessages(); + + // After processing, text converges + assert.equal(client1.getText(), "adef"); + assert.equal(client2.getText(), "adef"); + }); + + it("replaceText: Rollback on both clients", () => { + const { + sharedString: client1, + containerRuntimeFactory, + containerRuntime: cr1, + } = setupSharedStringRollbackTest(); + const { sharedString: client2, containerRuntime: cr2 } = createAdditionalClient( + containerRuntimeFactory, + "2", + ); + + // Baseline text + client1.insertText(0, "abcdef"); + cr1.flush(); + containerRuntimeFactory.processAllMessages(); + + // Client1 replaces locally (pending) + client1.replaceText(2, 4, "XY"); // "abXYef" + assert.equal(client1.getText(), "abXYef"); + + // Client2 removes text concurrently (pending) + client2.removeText(1, 3); // removes "bX" in their local view + assert.equal(client2.getText(), "adef"); + + // Rollback Client1 before flushing + cr1.rollback?.(); + assert.equal(client1.getText(), "abcdef", "rollback restores original text on Client1"); + + // Rollback Client1 before flushing + cr2.rollback?.(); + assert.equal(client2.getText(), "abcdef", "rollback restores original text on Client2"); + + // Flush both and process messages + cr1.flush(); + cr2.flush(); + containerRuntimeFactory.processAllMessages(); + + // After processing, text converges + assert.equal(client1.getText(), "abcdef"); + assert.equal(client2.getText(), "abcdef"); + }); +}); + +describe("SharedString annotate with rollback", () => { + it("can annotate text and rollback without remote changes", () => { + const { + sharedString: client1, + containerRuntimeFactory, + containerRuntime: cr1, + } = setupSharedStringRollbackTest(); + const { sharedString: client2 } = createAdditionalClient(containerRuntimeFactory, "2"); + const text = "hello world"; const styleProps = { style: "bold" }; - sharedString.insertText(0, text, styleProps); + client1.insertText(0, text, styleProps); + cr1.flush(); + containerRuntimeFactory.processAllMessages(); + assert.equal(client2.getText(), text); + + // Annotate a range locally (pending) + const colorProps = { color: "green" }; + client1.annotateRange(6, text.length, colorProps); + + for (let i = 6; i < text.length; i++) { + assert.deepEqual( + { ...client1.getPropertiesAtPosition(i) }, + { ...styleProps, ...colorProps }, + "Could not annotate props locally", + ); + } + + // Rollback pending annotation + cr1.rollback?.(); for (let i = 0; i < text.length; i++) { assert.deepEqual( - { ...sharedString.getPropertiesAtPosition(i) }, + { ...client1.getPropertiesAtPosition(i) }, { ...styleProps }, - "Could not add props", + "Rollback reverted annotations", ); } - const colorProps = { color: "green" }; - sharedString.annotateRange(6, text.length, colorProps); + // Remote client remains unchanged + for (let i = 0; i < text.length; i++) { + assert.deepEqual( + { ...client2.getPropertiesAtPosition(i) }, + { ...styleProps }, + "Remote client unchanged", + ); + } + }); + + it("can handle null annotations with rollback", () => { + const { + sharedString: client1, + containerRuntimeFactory, + containerRuntime: cr1, + } = setupSharedStringRollbackTest(); + const { sharedString: client2 } = createAdditionalClient(containerRuntimeFactory, "2"); + + const text = "hello world"; + const startingProps = { style: "bold", color: null }; + client1.insertText(0, text, startingProps); + cr1.flush(); + containerRuntimeFactory.processAllMessages(); + + for (let i = 0; i < text.length; i++) { + assert.strictEqual(client1.getPropertiesAtPosition(i)?.color, undefined); + assert.strictEqual(client2.getPropertiesAtPosition(i)?.color, undefined); + } + + // Annotate locally with null values (pending) + const updatedProps = { style: null }; + client1.annotateRange(6, text.length, updatedProps); + + for (let i = 6; i < text.length; i++) { + assert.strictEqual(client1.getPropertiesAtPosition(i)?.style, undefined); + } + + // Rollback pending annotation + cr1.rollback?.(); for (let i = 6; i < text.length; i++) { assert.deepEqual( - { ...sharedString.getPropertiesAtPosition(i) }, - { ...styleProps, ...colorProps }, - "Could not annotate props", + { ...client1.getPropertiesAtPosition(i) }, + { style: "bold" }, + "Rollback restores original props", ); } + }); + + it("handles multiple annotations with rollback", () => { + const { + sharedString: client1, + containerRuntimeFactory, + containerRuntime: cr1, + } = setupSharedStringRollbackTest(); + const { sharedString: client2 } = createAdditionalClient(containerRuntimeFactory, "2"); + const text = "hello world"; + client1.insertText(0, text); + cr1.flush(); containerRuntimeFactory.processAllMessages(); - containerRuntime.rollback?.(); + const styleProps = { style: "italic" }; + const colorProps = { color: "red" }; - assert.equal(sharedString.getText(), "", "text reverted after rollback"); + // Annotate ranges with different props + client1.annotateRange(0, 5, styleProps); + client1.annotateRange(6, 11, colorProps); + + // Verify pending annotations locally + for (let i = 0; i < 5; i++) { + assert.deepEqual( + { ...client1.getPropertiesAtPosition(i) }, + { ...styleProps }, + "Could not add styleProps", + ); + } + for (let i = 6; i < 11; i++) { + assert.deepEqual( + { ...client1.getPropertiesAtPosition(i) }, + { ...colorProps }, + "Could not add colorProps", + ); + } + + // Rollback pending annotations + cr1.rollback?.(); + + // Verify annotations reverted + for (let i = 0; i < text.length; i++) { + assert.deepEqual({ ...client1.getPropertiesAtPosition(i) }, {}, "Could not add props"); + } + + // Remote client should remain unchanged (no annotations) + for (let i = 0; i < text.length; i++) { + assert.deepEqual({ ...client2.getPropertiesAtPosition(i) }, {}, "Could not add props"); + } }); }); From 8617906743a57199b06b074878c42b12ef3dc5ae Mon Sep 17 00:00:00 2001 From: Sonali Deshpande <48232592+sonalideshpandemsft@users.noreply.github.com> Date: Tue, 19 Aug 2025 22:10:09 +0000 Subject: [PATCH 03/11] sequenceDelta --- .../src/test/sharedString.rollback.spec.ts | 72 +++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/packages/dds/sequence/src/test/sharedString.rollback.spec.ts b/packages/dds/sequence/src/test/sharedString.rollback.spec.ts index a5dda21bd11b..9558a972c3c1 100644 --- a/packages/dds/sequence/src/test/sharedString.rollback.spec.ts +++ b/packages/dds/sequence/src/test/sharedString.rollback.spec.ts @@ -6,6 +6,7 @@ import { strict as assert } from "assert"; import { AttachState } from "@fluidframework/container-definitions/internal"; +import { MergeTreeDeltaType } from "@fluidframework/merge-tree/internal"; import { MockContainerRuntimeFactory, MockFluidDataStoreRuntime, @@ -552,3 +553,74 @@ describe("SharedString annotate with rollback", () => { } }); }); + +describe("SharedString rollback triggers correct sequenceDelta events with text", () => { + it("insert, remove, annotate, and replaceText rollback trigger correct sequenceDelta events", () => { + const { sharedString, containerRuntimeFactory, containerRuntime } = + setupSharedStringRollbackTest(); + + const events: { op: string; text: string }[] = []; + sharedString.on("sequenceDelta", ({ deltaOperation, ranges, isLocal }) => { + if (!isLocal) return; + switch (deltaOperation) { + case MergeTreeDeltaType.INSERT: + events.push({ op: "insert", text: sharedString.getText() }); + break; + case MergeTreeDeltaType.REMOVE: + events.push({ op: "remove", text: sharedString.getText() }); + break; + case MergeTreeDeltaType.ANNOTATE: + events.push({ op: "annotate", text: sharedString.getText() }); + break; + default: + throw new Error(`Unexpected deltaOperation: ${deltaOperation}`); + } + }); + + // --- Insert and rollback --- + sharedString.insertText(0, "hello"); + containerRuntimeFactory.processAllMessages(); + assert.equal(sharedString.getText(), "hello"); + containerRuntime.rollback?.(); + assert( + events.some((e) => e.op === "remove" && e.text === ""), + "Rollback of insert should trigger remove of correct text", + ); + + events.length = 0; + + // --- Remove and rollback --- + sharedString.insertText(0, "world"); + containerRuntimeFactory.processAllMessages(); + sharedString.removeText(0, 5); + assert.equal(sharedString.getText(), ""); + containerRuntime.rollback?.(); + assert( + events.some((e) => e.op === "insert" && e.text === "world"), + "Rollback of remove should trigger insert of correct text", + ); + + events.length = 0; + + // --- Annotate and rollback --- + sharedString.insertText(0, "abc"); + containerRuntimeFactory.processAllMessages(); + const styleProps = { style: "bold" }; + sharedString.annotateRange(0, 3, styleProps); + Array.from({ length: 3 }).forEach((_, i) => + assert.deepEqual({ ...sharedString.getPropertiesAtPosition(i) }, { ...styleProps }), + ); + containerRuntime.rollback?.(); + Array.from({ length: 3 }).forEach((_, i) => + assert.deepEqual( + { ...sharedString.getPropertiesAtPosition(i) }, + {}, + "Rollback of annotate should clear properties", + ), + ); + assert( + events.some((e) => e.op === "annotate" && e.text === "abc"), + "Rollback of annotate should trigger annotate event with correct text", + ); + }); +}); From b54d0167a688cd98c5c570c4dcfbe2f52044a0df Mon Sep 17 00:00:00 2001 From: Sonali Deshpande <48232592+sonalideshpandemsft@users.noreply.github.com> Date: Wed, 20 Aug 2025 15:36:54 +0000 Subject: [PATCH 04/11] Add tests for multiple clients --- .../src/test/sharedString.rollback.spec.ts | 187 +++++++++++++----- 1 file changed, 135 insertions(+), 52 deletions(-) diff --git a/packages/dds/sequence/src/test/sharedString.rollback.spec.ts b/packages/dds/sequence/src/test/sharedString.rollback.spec.ts index 9558a972c3c1..7d9bbd9e9eda 100644 --- a/packages/dds/sequence/src/test/sharedString.rollback.spec.ts +++ b/packages/dds/sequence/src/test/sharedString.rollback.spec.ts @@ -14,7 +14,7 @@ import { type MockContainerRuntime, } from "@fluidframework/test-runtime-utils/internal"; -import { SharedStringFactory } from "../sequenceFactory.js"; +import { SharedStringFactory, type SharedString } from "../sequenceFactory.js"; import { SharedStringClass } from "../sharedString.js"; function setupSharedStringRollbackTest() { @@ -65,41 +65,6 @@ function createAdditionalClient( return { sharedString, dataStoreRuntime, containerRuntime }; } -describe("SharedString rollback change events", () => { - it("rollback insert emits remove", () => { - const { sharedString, containerRuntimeFactory, containerRuntime } = - setupSharedStringRollbackTest(); - - // Apply insert we will rollback - sharedString.insertText(0, "abc"); - containerRuntimeFactory.processAllMessages(); - assert.equal(sharedString.getText(), "abc", "text after insert"); - - containerRuntime.rollback?.(); - - // Expect rollback event to be a remove - assert.equal(sharedString.getText(), "", "text reverted after rollback"); - }); - - it("rollback remove emits insert", () => { - const { sharedString, containerRuntimeFactory, containerRuntime } = - setupSharedStringRollbackTest(); - sharedString.insertText(0, "hello"); - containerRuntimeFactory.processAllMessages(); - containerRuntime.flush(); - assert.equal(sharedString.getText(), "hello"); - - // Apply remove we will rollback - sharedString.removeText(0, 5); - assert.equal(sharedString.getText(), "", "text after remove"); - - containerRuntime.rollback?.(); - - // Expect rollback event to be an insert - assert.equal(sharedString.getText(), "hello", "rollback discards remove"); - }); -}); - describe("SharedString rollback with multiple clients (insert/remove)", () => { it("Client1 insert + Client2 insert + rollback on Client1", () => { const { @@ -555,12 +520,13 @@ describe("SharedString annotate with rollback", () => { }); describe("SharedString rollback triggers correct sequenceDelta events with text", () => { - it("insert, remove, annotate, and replaceText rollback trigger correct sequenceDelta events", () => { - const { sharedString, containerRuntimeFactory, containerRuntime } = - setupSharedStringRollbackTest(); + interface Event { + op: string; + text: string; + } - const events: { op: string; text: string }[] = []; - sharedString.on("sequenceDelta", ({ deltaOperation, ranges, isLocal }) => { + function setupDeltaListener(sharedString: SharedString, events: Event[]) { + sharedString.on("sequenceDelta", ({ deltaOperation, isLocal }) => { if (!isLocal) return; switch (deltaOperation) { case MergeTreeDeltaType.INSERT: @@ -576,51 +542,168 @@ describe("SharedString rollback triggers correct sequenceDelta events with text" throw new Error(`Unexpected deltaOperation: ${deltaOperation}`); } }); + } + + it("rollback of insert triggers remove", () => { + const { sharedString, containerRuntimeFactory, containerRuntime } = + setupSharedStringRollbackTest(); + const events: Event[] = []; + setupDeltaListener(sharedString, events); - // --- Insert and rollback --- sharedString.insertText(0, "hello"); containerRuntimeFactory.processAllMessages(); assert.equal(sharedString.getText(), "hello"); + containerRuntime.rollback?.(); + assert( events.some((e) => e.op === "remove" && e.text === ""), "Rollback of insert should trigger remove of correct text", ); + }); - events.length = 0; + it("rollback of remove triggers insert", () => { + const { sharedString, containerRuntimeFactory, containerRuntime } = + setupSharedStringRollbackTest(); + const events: Event[] = []; + setupDeltaListener(sharedString, events); - // --- Remove and rollback --- sharedString.insertText(0, "world"); containerRuntimeFactory.processAllMessages(); sharedString.removeText(0, 5); assert.equal(sharedString.getText(), ""); + containerRuntime.rollback?.(); + assert( events.some((e) => e.op === "insert" && e.text === "world"), "Rollback of remove should trigger insert of correct text", ); + }); - events.length = 0; + it("rollback of annotate clears properties", () => { + const { sharedString, containerRuntimeFactory, containerRuntime } = + setupSharedStringRollbackTest(); + const events: Event[] = []; + setupDeltaListener(sharedString, events); - // --- Annotate and rollback --- sharedString.insertText(0, "abc"); containerRuntimeFactory.processAllMessages(); + const styleProps = { style: "bold" }; sharedString.annotateRange(0, 3, styleProps); - Array.from({ length: 3 }).forEach((_, i) => - assert.deepEqual({ ...sharedString.getPropertiesAtPosition(i) }, { ...styleProps }), - ); + for (let i = 0; i < 3; i++) { + assert.deepEqual({ ...sharedString.getPropertiesAtPosition(i) }, styleProps); + } + containerRuntime.rollback?.(); - Array.from({ length: 3 }).forEach((_, i) => + + for (let i = 0; i < 3; i++) { assert.deepEqual( { ...sharedString.getPropertiesAtPosition(i) }, {}, "Rollback of annotate should clear properties", - ), - ); + ); + } + assert( events.some((e) => e.op === "annotate" && e.text === "abc"), "Rollback of annotate should trigger annotate event with correct text", ); }); + + it("multi-client: rollback of insert triggers remove", () => { + const { + sharedString: client1, + containerRuntimeFactory, + containerRuntime: cr1, + } = setupSharedStringRollbackTest(); + const { sharedString: client2 } = createAdditionalClient(containerRuntimeFactory, "2"); + + const eventsClient1: Event[] = []; + setupDeltaListener(client1, eventsClient1); + + client1.insertText(0, "hello"); + cr1.flush(); + containerRuntimeFactory.processAllMessages(); + assert.equal(client1.getText(), "hello"); + assert.equal(client2.getText(), "hello"); + + client1.insertText(5, "world"); + cr1.rollback?.(); + + assert( + eventsClient1.some((e) => e.op === "remove" && e.text === "hello"), + "Rollback of insert should trigger remove of correct text on client1", + ); + assert.equal(client1.getText(), "hello"); + assert.equal(client2.getText(), "hello"); + }); + + it("multi-client: rollback of remove triggers insert", () => { + const { + sharedString: client1, + containerRuntimeFactory, + containerRuntime: cr1, + } = setupSharedStringRollbackTest(); + const { sharedString: client2 } = createAdditionalClient(containerRuntimeFactory, "2"); + + const eventsClient1: Event[] = []; + setupDeltaListener(client1, eventsClient1); + + client1.insertText(0, "world"); + cr1.flush(); + containerRuntimeFactory.processAllMessages(); + + client1.removeText(0, 5); + assert.equal(client1.getText(), ""); + assert.equal(client2.getText(), "world"); + + cr1.rollback?.(); + + assert( + eventsClient1.some((e) => e.op === "insert" && e.text === "world"), + "Rollback of remove should trigger insert of correct text on client1", + ); + assert.equal(client1.getText(), "world"); + assert.equal(client2.getText(), "world"); + }); + + it("multi-client: rollback of annotate clears properties", () => { + const { + sharedString: client1, + containerRuntimeFactory, + containerRuntime: cr1, + } = setupSharedStringRollbackTest(); + createAdditionalClient(containerRuntimeFactory, "2"); + + const eventsClient1: Event[] = []; + setupDeltaListener(client1, eventsClient1); + + client1.insertText(0, "abc"); + cr1.flush(); + containerRuntimeFactory.processAllMessages(); + + const styleProps = { style: "bold" }; + client1.annotateRange(0, 3, styleProps); + + for (let i = 0; i < 3; i++) { + assert.deepEqual({ ...client1.getPropertiesAtPosition(i) }, styleProps); + } + + cr1.rollback?.(); + + for (let i = 0; i < 3; i++) { + assert.deepEqual( + { ...client1.getPropertiesAtPosition(i) }, + {}, + "Rollback of annotate should clear properties", + ); + } + + assert( + eventsClient1.some((e) => e.op === "annotate" && e.text === "abc"), + "Rollback of annotate should trigger annotate event with correct text", + ); + }); }); From 9451072fd21c9ace38e0fec84d84a086167fb04f Mon Sep 17 00:00:00 2001 From: Sonali Deshpande <48232592+sonalideshpandemsft@users.noreply.github.com> Date: Thu, 21 Aug 2025 16:16:58 +0000 Subject: [PATCH 05/11] nits --- .../dds/cell/src/test/cell.rollback.spec.ts | 6 + .../mocha/directory.rollback.events.spec.ts | 489 ++++++++++++++++++ .../src/test/sharedString.rollback.spec.ts | 2 +- 3 files changed, 496 insertions(+), 1 deletion(-) create mode 100644 packages/dds/cell/src/test/cell.rollback.spec.ts create mode 100644 packages/dds/map/src/test/mocha/directory.rollback.events.spec.ts diff --git a/packages/dds/cell/src/test/cell.rollback.spec.ts b/packages/dds/cell/src/test/cell.rollback.spec.ts new file mode 100644 index 000000000000..162888276ff4 --- /dev/null +++ b/packages/dds/cell/src/test/cell.rollback.spec.ts @@ -0,0 +1,6 @@ +/*! + * Copyright (c) Microsoft Corporation and contributors. All rights reserved. + * Licensed under the MIT License. + */ + + diff --git a/packages/dds/map/src/test/mocha/directory.rollback.events.spec.ts b/packages/dds/map/src/test/mocha/directory.rollback.events.spec.ts new file mode 100644 index 000000000000..5270c7c0102d --- /dev/null +++ b/packages/dds/map/src/test/mocha/directory.rollback.events.spec.ts @@ -0,0 +1,489 @@ +/*! + * Copyright (c) Microsoft Corporation and contributors. All rights reserved. + * Licensed under the MIT License. + */ + +import { strict as assert } from "node:assert"; + +import { AttachState } from "@fluidframework/container-definitions"; +import { + MockContainerRuntimeFactory, + MockFluidDataStoreRuntime, + MockStorage, + type MockContainerRuntime, +} from "@fluidframework/test-runtime-utils/internal"; + +import { DirectoryFactory } from "../../directoryFactory.js"; +import type { ISharedDirectory, IValueChanged } from "../../interfaces.js"; + +interface RollbackTestSetup { + sharedDirectory: ISharedDirectory; + dataStoreRuntime: MockFluidDataStoreRuntime; + containerRuntimeFactory: MockContainerRuntimeFactory; + containerRuntime: MockContainerRuntime; +} + +const directoryFactory = new DirectoryFactory(); + +function setupRollbackTest(): RollbackTestSetup { + const containerRuntimeFactory = new MockContainerRuntimeFactory({ flushMode: 1 }); // TurnBased + const dataStoreRuntime = new MockFluidDataStoreRuntime({ clientId: "1" }); + const containerRuntime = containerRuntimeFactory.createContainerRuntime(dataStoreRuntime); + const sharedDirectory = directoryFactory.create(dataStoreRuntime, "shared-directory-1"); + dataStoreRuntime.setAttachState(AttachState.Attached); + sharedDirectory.connect({ + deltaConnection: dataStoreRuntime.createDeltaConnection(), + objectStorage: new MockStorage(), + }); + return { + sharedDirectory, + dataStoreRuntime, + containerRuntimeFactory, + containerRuntime, + }; +} + +// Helper to create another client attached to the same containerRuntimeFactory +function createAdditionalClient( + containerRuntimeFactory: MockContainerRuntimeFactory, + id: string = "client-2", +): { + sharedDirectory: ISharedDirectory; + containerRuntime: MockContainerRuntime; +} { + const dataStoreRuntime = new MockFluidDataStoreRuntime({ clientId: id }); + const containerRuntime = containerRuntimeFactory.createContainerRuntime(dataStoreRuntime); + const sharedDirectory = directoryFactory.create(dataStoreRuntime, `shared-directory-${id}`); + dataStoreRuntime.setAttachState(AttachState.Attached); + sharedDirectory.connect({ + deltaConnection: dataStoreRuntime.createDeltaConnection(), + objectStorage: new MockStorage(), + }); + return { sharedDirectory, containerRuntime }; +} + +describe("SharedDirectory rollback and valueChanged event", () => { + let sharedDirectory: ISharedDirectory; + let containerRuntime: MockContainerRuntime; + let containerRuntimeFactory: MockContainerRuntimeFactory; + + beforeEach(() => { + ({ sharedDirectory, containerRuntime, containerRuntimeFactory } = setupRollbackTest()); + }); + + it("should trigger valueChanged event on rollback of set", () => { + const events: IValueChanged[] = []; + sharedDirectory.on("valueChanged", (e) => events.push(e)); + + sharedDirectory.set("key1", "value1"); + assert.strictEqual(sharedDirectory.get("key1"), "value1"); + assert.strictEqual(events.length, 1); + + containerRuntime.rollback?.(); + + assert.strictEqual(sharedDirectory.get("key1"), undefined); + assert.strictEqual(events.length, 2); + assert.strictEqual(events[1].key, "key1"); + assert.strictEqual(events[1].previousValue, "value1"); + }); + + it("should trigger valueChanged event on rollback of delete", () => { + sharedDirectory.set("key1", "value1"); + containerRuntime.flush(); + containerRuntimeFactory.processAllMessages(); + + const events: IValueChanged[] = []; + sharedDirectory.on("valueChanged", (e) => events.push(e)); + + sharedDirectory.delete("key1"); + assert.strictEqual(sharedDirectory.get("key1"), undefined); + assert.strictEqual(events.length, 1); + + containerRuntime.rollback?.(); + + assert.strictEqual(sharedDirectory.get("key1"), "value1"); + assert.strictEqual(events.length, 2); + assert.strictEqual(events[1].key, "key1"); + assert.strictEqual(events[1].previousValue, undefined); + }); + + it("should trigger valueChanged events on rollback of clear", () => { + sharedDirectory.set("key1", "value1"); + sharedDirectory.set("key2", "value2"); + containerRuntime.flush(); + containerRuntimeFactory.processAllMessages(); + + const events: IValueChanged[] = []; + let clears = 0; + sharedDirectory.on("valueChanged", (e) => events.push(e)); + sharedDirectory.on("clear", () => clears++); + + sharedDirectory.clear(); + assert.strictEqual(sharedDirectory.get("key1"), undefined); + assert.strictEqual(sharedDirectory.get("key2"), undefined); + assert.strictEqual(clears, 1); + assert.strictEqual(events.length, 0); + + containerRuntime.rollback?.(); + + assert.strictEqual(sharedDirectory.get("key1"), "value1"); + assert.strictEqual(sharedDirectory.get("key2"), "value2"); + assert.strictEqual(events.length, 2); + assert.strictEqual(events[0].key, "key1"); + assert.strictEqual(events[1].key, "key2"); + }); +}); + +describe("SharedDirectory rollback and valueChanged event with multiple clients", () => { + let client1: ISharedDirectory; + let client2: ISharedDirectory; + let containerRuntime1: MockContainerRuntime; + let containerRuntime2: MockContainerRuntime; + let containerRuntimeFactory: MockContainerRuntimeFactory; + + beforeEach(() => { + ({ + sharedDirectory: client1, + containerRuntimeFactory, + containerRuntime: containerRuntime1, + } = setupRollbackTest()); + + ({ sharedDirectory: client2, containerRuntime: containerRuntime2 } = + createAdditionalClient(containerRuntimeFactory, "2")); + }); + + it("should trigger valueChanged event on rollback of set for one client", () => { + const eventsClient1: IValueChanged[] = []; + const eventsClient2: IValueChanged[] = []; + + client1.on("valueChanged", (e) => eventsClient1.push(e)); + client2.on("valueChanged", (e) => eventsClient2.push(e)); + + client1.set("key1", "value1"); + assert.strictEqual(client1.get("key1"), "value1"); + + // Rollback only affects client1's local changes + containerRuntime1.rollback?.(); + + assert.strictEqual(client1.get("key1"), undefined); + assert.strictEqual(eventsClient1.length, 2); + assert.strictEqual(eventsClient1[0].key, "key1"); + assert.strictEqual(eventsClient1[0].previousValue, "value1"); + + // client2 never had the change locally, so no events + assert.strictEqual(eventsClient2.length, 0); + }); + + it("should trigger valueChanged event on rollback of delete for one client", () => { + client1.set("key1", "value1"); + containerRuntime1.flush(); + containerRuntimeFactory.processAllMessages(); + + const eventsClient1: IValueChanged[] = []; + const eventsClient2: IValueChanged[] = []; + client1.on("valueChanged", (e) => eventsClient1.push(e)); + client2.on("valueChanged", (e) => eventsClient2.push(e)); + + client1.delete("key1"); + assert.strictEqual(client1.get("key1"), undefined); + + containerRuntime1.rollback?.(); + + assert.strictEqual(client1.get("key1"), "value1"); + assert.strictEqual(eventsClient1.length, 2); + assert.strictEqual(eventsClient1[0].key, "key1"); + assert.strictEqual(eventsClient1[0].previousValue, undefined); + + assert.strictEqual(eventsClient2.length, 0); + }); + + it("should trigger valueChanged events on rollback of clear for one client", () => { + client1.set("key1", "value1"); + client1.set("key2", "value2"); + containerRuntime1.flush(); + containerRuntimeFactory.processAllMessages(); + + const eventsClient1: IValueChanged[] = []; + const eventsClient2: IValueChanged[] = []; + let clearsClient1 = 0; + let clearsClient2 = 0; + + client1.on("valueChanged", (e) => eventsClient1.push(e)); + client2.on("valueChanged", (e) => eventsClient2.push(e)); + + client1.on("clear", () => clearsClient1++); + client2.on("clear", () => clearsClient2++); + + client1.clear(); + assert.strictEqual(client1.get("key1"), undefined); + assert.strictEqual(client1.get("key2"), undefined); + assert.strictEqual(clearsClient1, 1); + assert.strictEqual(eventsClient1.length, 0); + + containerRuntime1.rollback?.(); + + assert.strictEqual(client1.get("key1"), "value1"); + assert.strictEqual(client1.get("key2"), "value2"); + assert.strictEqual(eventsClient1.length, 2); + assert.strictEqual(eventsClient1[0].key, "key1"); + assert.strictEqual(eventsClient1[1].key, "key2"); + + // client2 should remain unaffected + assert.strictEqual(eventsClient2.length, 0); + assert.strictEqual(clearsClient2, 0); + }); +}); + +describe("SharedDirectory rollback event correctness with multiple clients", () => { + let client1: ISharedDirectory; + let client2: ISharedDirectory; + let containerRuntime1: MockContainerRuntime; + let containerRuntime2: MockContainerRuntime; + let containerRuntimeFactory: MockContainerRuntimeFactory; + + beforeEach(() => { + ({ + sharedDirectory: client1, + containerRuntimeFactory, + containerRuntime: containerRuntime1, + } = setupRollbackTest()); + + ({ sharedDirectory: client2, containerRuntime: containerRuntime2 } = + createAdditionalClient(containerRuntimeFactory, "2")); + }); + + it("should fire valueChanged events for every state change, not just match final state", () => { + const eventsClient1: IValueChanged[] = []; + const eventsClient2: IValueChanged[] = []; + + client1.on("valueChanged", (e) => eventsClient1.push(e)); + client2.on("valueChanged", (e) => eventsClient2.push(e)); + + // Step 1: Client1 sets key1 + client1.set("key1", "value1"); + assert.strictEqual(client1.get("key1"), "value1"); + assert.strictEqual(client2.get("key1"), "value1"); + + // Step 2: Client2 sets key1 to a different value + client2.set("key1", "value2"); + assert.strictEqual(client1.get("key1"), "value2"); + assert.strictEqual(client2.get("key1"), "value2"); + + // Step 3: Client1 deletes key1 + client1.delete("key1"); + assert.strictEqual(client1.get("key1"), undefined); + assert.strictEqual(client2.get("key1"), undefined); + + // Rollback Client1 + containerRuntime1.rollback?.(); + + // Final state should match expected rollback state + assert.strictEqual(client1.get("key1"), "value2"); + assert.strictEqual(client2.get("key1"), "value2"); + + // Check that events for **each intermediate state change** fired correctly + assert.deepStrictEqual( + eventsClient1.map(({ key, previousValue }: IValueChanged) => ({ + key, + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment + previousValue, + })), + [ + { key: "key1", previousValue: undefined }, // set to "value1" + { key: "key1", previousValue: "value1" }, // overwritten by client2 + { key: "key1", previousValue: "value2" }, // deleted + { key: "key1", previousValue: undefined }, // rollback restores to "value2" + ], + ); + + // Client2 did not rollback, so should see events for the set from client1 and its own set + assert.deepStrictEqual( + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment + eventsClient2.map(({ key, previousValue }): IValueChanged => ({ key, previousValue })), + [ + { key: "key1", previousValue: undefined }, // initial set by client1 + { key: "key1", previousValue: "value1" }, // set to value2 by itself + { key: "key1", previousValue: "value2" }, // deletion from client1 + ], + ); + }); +}); + +describe("SharedDirectory rollback events", () => { + let sharedDirectory: ISharedDirectory; + let containerRuntime: MockContainerRuntime; + let containerRuntimeFactory: MockContainerRuntimeFactory; + + beforeEach(() => { + ({ sharedDirectory, containerRuntime, containerRuntimeFactory } = setupRollbackTest()); + }); + + it("should trigger subDirectoryDeleted event on rollback of subDirectoryCreated", () => { + const events: { path: string; local: boolean; target: ISharedDirectory }[] = []; + + sharedDirectory.on("subDirectoryDeleted", (path, local, target) => { + events.push({ path, local, target }); + }); + + // Create a subdirectory + sharedDirectory.createSubDirectory("subDir1"); + assert( + sharedDirectory.getWorkingDirectory("subDir1") !== undefined, + "subdirectory should exist after creation", + ); + + // Rollback + containerRuntime.rollback?.(); + + // The subdirectory should be removed + assert.strictEqual( + sharedDirectory.getWorkingDirectory("subDir1"), + undefined, + "subdirectory should be removed after rollback", + ); + + // The subDirectoryDeleted event should be triggered + assert.strictEqual( + events.length, + 1, + "one subDirectoryDeleted event should have been emitted", + ); + assert.strictEqual(events[0].path, "subDir1"); + assert.strictEqual(events[0].local, true); + assert.strictEqual(events[0].target, sharedDirectory); + }); + + it("should trigger subDirectoryCreated event on rollback of subDirectoryDeleted", () => { + const events: { path: string; local: boolean; target: ISharedDirectory }[] = []; + + sharedDirectory.createSubDirectory("subDir2"); + + sharedDirectory.on("subDirectoryCreated", (path, local, target) => { + events.push({ path, local, target }); + }); + + // Delete the subdirectory + sharedDirectory.deleteSubDirectory("subDir2"); + assert.strictEqual( + sharedDirectory.getWorkingDirectory("subDir2"), + undefined, + "subdirectory should be deleted", + ); + + // Rollback + containerRuntime.rollback?.(); + + // The subdirectory should be restored + assert( + sharedDirectory.get("subDir2") !== undefined, + "subdirectory should exist after rollback", + ); + + // The subDirectoryCreated event should be triggered + assert.strictEqual( + events.length, + 1, + "one subDirectoryCreated event should have been emitted", + ); + assert.strictEqual(events[0].path, "subDir2"); + assert.strictEqual(events[0].local, true); + assert.strictEqual(events[0].target, sharedDirectory); + }); +}); + +describe("SharedDirectory rollback with multiple clients", () => { + let client1: ISharedDirectory; + let client2: ISharedDirectory; + let containerRuntime1: MockContainerRuntime; + let containerRuntime2: MockContainerRuntime; + let containerRuntimeFactory: MockContainerRuntimeFactory; + + beforeEach(() => { + ({ + sharedDirectory: client1, + containerRuntimeFactory, + containerRuntime: containerRuntime1, + } = setupRollbackTest()); + + ({ sharedDirectory: client2, containerRuntime: containerRuntime2 } = + createAdditionalClient(containerRuntimeFactory, "2")); + }); + + it("should only rollback local subDirectoryCreated changes for one client", () => { + const eventsClient1: { path: string; local: boolean; target: ISharedDirectory }[] = []; + const eventsClient2: { path: string; local: boolean; target: ISharedDirectory }[] = []; + + client1.on("subDirectoryDeleted", (path, local, target) => + eventsClient1.push({ path, local, target }), + ); + client2.on("subDirectoryDeleted", (path, local, target) => + eventsClient2.push({ path, local, target }), + ); + + // Client1 creates a subdirectory + client1.createSubDirectory("sharedDir"); + + // Client2 creates a subdirectory + client2.createSubDirectory("sharedDir2"); + + // Check that directories exist + assert(client1.get("sharedDir") !== undefined); + assert(client2.get("sharedDir2") !== undefined); + + // Rollback only affects client1's local changes + containerRuntime1.rollback?.(); + + // Client1's local subdirectory should be gone + assert.strictEqual(client1.get("sharedDir"), undefined); + + // Client2's subdirectory remains + assert(client1.get("sharedDir2") !== undefined); + + // Events triggered + assert.strictEqual(eventsClient1.length, 1); + assert.strictEqual(eventsClient1[0].path, "sharedDir"); + assert(eventsClient1[0].local); + + // Client2 did not get any subDirectoryDeleted events for client1's rollback + assert.strictEqual(eventsClient2.length, 0); + }); + + it("should rollback local subDirectoryDeleted changes for one client", () => { + const eventsClient1: { path: string; local: boolean; target: ISharedDirectory }[] = []; + const eventsClient2: { path: string; local: boolean; target: ISharedDirectory }[] = []; + + // Create initial directories + client1.createSubDirectory("dir1"); + client2.createSubDirectory("dir2"); + + client1.on("subDirectoryCreated", (path, local, target) => + eventsClient1.push({ path, local, target }), + ); + client2.on("subDirectoryCreated", (path, local, target) => + eventsClient2.push({ path, local, target }), + ); + + // Client1 deletes its own directory + client1.deleteSubDirectory("dir1"); + + // Client2 deletes its own directory + client2.deleteSubDirectory("dir2"); + + // Rollback client1 + containerRuntime1.rollback?.(); + + // Client1's directory should be restored + assert(client1.get("dir1") !== undefined); + + // Client2's directory remains deleted + assert.strictEqual(client1.get("dir2"), undefined); + + // Event triggered for client1 + assert.strictEqual(eventsClient1.length, 1); + assert.strictEqual(eventsClient1[0].path, "dir1"); + assert(eventsClient1[0].local); + + // Client2 did not get any subDirectoryCreated events from client1 rollback + assert.strictEqual(eventsClient2.length, 0); + }); +}); diff --git a/packages/dds/sequence/src/test/sharedString.rollback.spec.ts b/packages/dds/sequence/src/test/sharedString.rollback.spec.ts index 7d9bbd9e9eda..2193a986a0b7 100644 --- a/packages/dds/sequence/src/test/sharedString.rollback.spec.ts +++ b/packages/dds/sequence/src/test/sharedString.rollback.spec.ts @@ -365,7 +365,7 @@ describe("SharedString replaceText with rollback and two clients", () => { cr1.rollback?.(); assert.equal(client1.getText(), "abcdef", "rollback restores original text on Client1"); - // Rollback Client1 before flushing + // Rollback Client2 before flushing cr2.rollback?.(); assert.equal(client2.getText(), "abcdef", "rollback restores original text on Client2"); From 4b501165d2fa7a7687394533033993fb045e45cb Mon Sep 17 00:00:00 2001 From: Sonali Deshpande <48232592+sonalideshpandemsft@users.noreply.github.com> Date: Fri, 22 Aug 2025 19:28:58 +0000 Subject: [PATCH 06/11] Rollback tests for cell --- .../dds/cell/src/test/cell.rollback.spec.ts | 187 ++++++++++++++++++ 1 file changed, 187 insertions(+) diff --git a/packages/dds/cell/src/test/cell.rollback.spec.ts b/packages/dds/cell/src/test/cell.rollback.spec.ts index 162888276ff4..ea30242f4901 100644 --- a/packages/dds/cell/src/test/cell.rollback.spec.ts +++ b/packages/dds/cell/src/test/cell.rollback.spec.ts @@ -3,4 +3,191 @@ * Licensed under the MIT License. */ +import { strict as assert } from "node:assert"; +import { AttachState } from "@fluidframework/container-definitions"; +import { + MockContainerRuntimeFactory, + MockFluidDataStoreRuntime, + MockStorage, + type MockContainerRuntime, +} from "@fluidframework/test-runtime-utils/internal"; + +import { CellFactory } from "../cellFactory.js"; +import type { ISharedCell } from "../interfaces.js"; + +interface RollbackTestSetup { + cell: ISharedCell; + dataStoreRuntime: MockFluidDataStoreRuntime; + containerRuntimeFactory: MockContainerRuntimeFactory; + containerRuntime: MockContainerRuntime; +} + +const mapFactory = new CellFactory(); + +function setupRollbackTest(id: string): RollbackTestSetup { + const containerRuntimeFactory = new MockContainerRuntimeFactory({ flushMode: 1 }); + const dataStoreRuntime = new MockFluidDataStoreRuntime({ clientId: "1" }); + const containerRuntime = containerRuntimeFactory.createContainerRuntime(dataStoreRuntime); + const cell = mapFactory.create(dataStoreRuntime, id); + dataStoreRuntime.setAttachState(AttachState.Attached); + cell.connect({ + deltaConnection: dataStoreRuntime.createDeltaConnection(), + objectStorage: new MockStorage(), + }); + return { + cell, + dataStoreRuntime, + containerRuntimeFactory, + containerRuntime, + }; +} + +// Helper to create another client attached to the same containerRuntimeFactory +function createAdditionalClient( + containerRuntimeFactory: MockContainerRuntimeFactory, + id: string = "client-2", +): { + cell: ISharedCell; + dataStoreRuntime: MockFluidDataStoreRuntime; + containerRuntime: MockContainerRuntime; +} { + const dataStoreRuntime = new MockFluidDataStoreRuntime({ clientId: id }); + const containerRuntime = containerRuntimeFactory.createContainerRuntime(dataStoreRuntime); + const cell = mapFactory.create(dataStoreRuntime, `cell-${id}`); + dataStoreRuntime.setAttachState(AttachState.Attached); + cell.connect({ + deltaConnection: dataStoreRuntime.createDeltaConnection(), + objectStorage: new MockStorage(), + }); + return { cell, dataStoreRuntime, containerRuntime }; +} + +describe("Cell with rollback", () => { + it("should emit valueChanged on set and rollback should re-emit previous value", async () => { + const { cell, containerRuntime } = setupRollbackTest("client-1"); + + const events: (string | undefined)[] = []; + + cell.on("valueChanged", (value) => events.push("valueChanged")); + + cell.set(10); + assert.equal(cell.get(), 10); + + containerRuntime.rollback?.(); + + assert.equal(cell.get(), undefined); + + assert.deepEqual(events, ["valueChanged"]); + }); + + it("should emit delete on delete, and rollback should re-emit last valueChanged", async () => { + const { cell, containerRuntimeFactory, containerRuntime } = setupRollbackTest("client-1"); + + const events: (string | undefined)[] = []; + + cell.on("valueChanged", (value) => events.push("valueChanged")); + cell.on("delete", () => events.push("delete")); + + cell.set(42); + containerRuntime.flush(); + containerRuntimeFactory.processAllMessages(); + + cell.delete(); + assert(cell.empty()); + + // rollback delete + containerRuntime.rollback?.(); + assert.equal(cell.get(), 42); + + // delete triggers delete event, rollback restores valueChanged + assert.deepEqual(events, ["valueChanged", "delete", "valueChanged"]); + }); +}); + +describe("SharedCell rollback events with multiple clients", () => { + it("should emit valueChanged on set and rollback should re-emit previous value across clients", async () => { + // Setup two clients + const { + cell: cell1, + containerRuntimeFactory, + containerRuntime: runtime1, + } = setupRollbackTest("client-1"); + const { cell: cell2 } = createAdditionalClient(containerRuntimeFactory); + + const events1: string[] = []; + const events2: string[] = []; + + // Listen to valueChanged events on both clients + cell1.on("valueChanged", () => events1.push("valueChanged")); + cell2.on("valueChanged", () => events2.push("valueChanged")); + + // Client 1 sets a value + cell1.set(10); + assert.equal(cell1.get(), 10); + + // Propagate ops to client 2 + runtime1.flush(); + containerRuntimeFactory.processAllMessages(); + + assert.equal(cell2.get(), 10); + + cell1.set(100); + assert.equal(cell1.get(), 100); + assert.equal(cell2.get(), 10); + + // Rollback on client 1 + runtime1.rollback?.(); + + assert.equal(cell1.get(), 10); + assert.equal(cell2.get(), 10); + + // Both clients should have seen the events + assert.deepEqual(events1, ["valueChanged", "valueChanged", "valueChanged"]); + assert.deepEqual(events2, ["valueChanged"]); + }); + + it("should emit delete on delete, and rollback should re-emit last valueChanged across clients", async () => { + // Setup two clients + const { + cell: cell1, + containerRuntimeFactory, + containerRuntime: runtime1, + } = setupRollbackTest("client-1"); + const { cell: cell2 } = createAdditionalClient(containerRuntimeFactory); + + const events1: string[] = []; + const events2: string[] = []; + + // Attach listeners + cell1.on("valueChanged", () => events1.push("valueChanged")); + cell1.on("delete", () => events1.push("delete")); + + cell2.on("valueChanged", () => events2.push("valueChanged")); + cell2.on("delete", () => events2.push("delete")); + + // Set initial value and propagate + cell1.set(42); + runtime1.flush(); + containerRuntimeFactory.processAllMessages(); + + assert.equal(cell2.get(), 42); + + // Delete the value + cell1.delete(); + + assert(cell1.empty()); + assert.equal(cell2.get(), 42); + + // Rollback delete + runtime1.rollback?.(); + + // After rollback, value is restored + assert.equal(cell1.get(), 42); + assert.equal(cell2.get(), 42); + + // Event order + assert.deepEqual(events1, ["valueChanged", "delete", "valueChanged"]); + assert.deepEqual(events2, ["valueChanged"]); + }); +}); From e0fd9f04d94e62c9a3cf03ad2b7b90483b733f99 Mon Sep 17 00:00:00 2001 From: Sonali Deshpande <48232592+sonalideshpandemsft@users.noreply.github.com> Date: Mon, 25 Aug 2025 15:40:22 +0000 Subject: [PATCH 07/11] move helper functions --- .../dds/test-dds-utils/src/ddsTestUtils.ts | 92 +++++++++++++++++++ packages/dds/test-dds-utils/src/index.ts | 6 ++ 2 files changed, 98 insertions(+) create mode 100644 packages/dds/test-dds-utils/src/ddsTestUtils.ts diff --git a/packages/dds/test-dds-utils/src/ddsTestUtils.ts b/packages/dds/test-dds-utils/src/ddsTestUtils.ts new file mode 100644 index 000000000000..f883b2fe24d9 --- /dev/null +++ b/packages/dds/test-dds-utils/src/ddsTestUtils.ts @@ -0,0 +1,92 @@ +/*! + * Copyright (c) Microsoft Corporation and contributors. All rights reserved. + * Licensed under the MIT License. + */ + +import { AttachState } from "@fluidframework/container-definitions"; +import type { + IChannel, + IChannelServices, +} from "@fluidframework/datastore-definitions/internal"; +import { + MockFluidDataStoreRuntime, + MockStorage, + type MockContainerRuntime, + MockContainerRuntimeFactory, +} from "@fluidframework/test-runtime-utils/internal"; + +/** + * @internal + */ +export type DDSCreator = ( + runtime: MockFluidDataStoreRuntime, + id: string, +) => T; + +/** + * @internal + */ +export interface RollbackTestSetup { + dds: T; + dataStoreRuntime: MockFluidDataStoreRuntime; + containerRuntimeFactory: MockContainerRuntimeFactory; + containerRuntime: MockContainerRuntime; +} + +/** + * Setup rollback tests + * @internal + */ +export function setupRollbackTest( + id: string, + createDDS: DDSCreator, + opts?: { initialize?: (dds: T) => void }, +): RollbackTestSetup { + const containerRuntimeFactory = new MockContainerRuntimeFactory({ flushMode: 1 }); + const dataStoreRuntime = new MockFluidDataStoreRuntime({ clientId: "1" }); + const containerRuntime = containerRuntimeFactory.createContainerRuntime(dataStoreRuntime); + + const dds = createDDS(dataStoreRuntime, id); + + dataStoreRuntime.setAttachState(AttachState.Attached); + opts?.initialize?.(dds); + + const services: IChannelServices = { + deltaConnection: dataStoreRuntime.createDeltaConnection(), + objectStorage: new MockStorage(), + }; + dds.connect(services); + + return { dds, dataStoreRuntime, containerRuntimeFactory, containerRuntime }; +} + +/** + * Create a new client + * @internal + */ +export function createAdditionalClient( + containerRuntimeFactory: MockContainerRuntimeFactory, + id: string, + createDDS: DDSCreator, + opts?: { initialize?: (dds: T) => void }, +): { + dds: T; + dataStoreRuntime: MockFluidDataStoreRuntime; + containerRuntime: MockContainerRuntime; +} { + const dataStoreRuntime = new MockFluidDataStoreRuntime({ clientId: id }); + const containerRuntime = containerRuntimeFactory.createContainerRuntime(dataStoreRuntime); + + const dds = createDDS(dataStoreRuntime, id); + + dataStoreRuntime.setAttachState(AttachState.Attached); + opts?.initialize?.(dds); + + const services: IChannelServices = { + deltaConnection: dataStoreRuntime.createDeltaConnection(), + objectStorage: new MockStorage(), + }; + dds.connect(services); + + return { dds, dataStoreRuntime, containerRuntime }; +} diff --git a/packages/dds/test-dds-utils/src/index.ts b/packages/dds/test-dds-utils/src/index.ts index 693f2a229e3a..dc1cab347492 100644 --- a/packages/dds/test-dds-utils/src/index.ts +++ b/packages/dds/test-dds-utils/src/index.ts @@ -34,3 +34,9 @@ export { export type { ISnapshotSuite } from "./ddsSnapshotHarness.js"; export { createSnapshotSuite } from "./ddsSnapshotHarness.js"; export type { Client, FuzzSerializedIdCompressor } from "./clientLoading.js"; +export { + setupRollbackTest, + createAdditionalClient, + type RollbackTestSetup, + type DDSCreator, +} from "./ddsTestUtils.js"; From 80f07d835507fbec8f5a310505b5106558b339f4 Mon Sep 17 00:00:00 2001 From: Sonali Deshpande <48232592+sonalideshpandemsft@users.noreply.github.com> Date: Mon, 25 Aug 2025 16:32:37 +0000 Subject: [PATCH 08/11] use helper --- .../dds/cell/src/test/cell.rollback.spec.ts | 95 ++-- .../mocha/directory.rollback.events.spec.ts | 489 ------------------ .../src/test/mocha/directory.rollback.spec.ts | 474 ++++++++++------- .../src/test/sharedString.rollback.spec.ts | 357 ++++++------- 4 files changed, 460 insertions(+), 955 deletions(-) delete mode 100644 packages/dds/map/src/test/mocha/directory.rollback.events.spec.ts diff --git a/packages/dds/cell/src/test/cell.rollback.spec.ts b/packages/dds/cell/src/test/cell.rollback.spec.ts index ea30242f4901..c99144a9613d 100644 --- a/packages/dds/cell/src/test/cell.rollback.spec.ts +++ b/packages/dds/cell/src/test/cell.rollback.spec.ts @@ -5,67 +5,19 @@ import { strict as assert } from "node:assert"; -import { AttachState } from "@fluidframework/container-definitions"; -import { - MockContainerRuntimeFactory, - MockFluidDataStoreRuntime, - MockStorage, - type MockContainerRuntime, -} from "@fluidframework/test-runtime-utils/internal"; +import { setupRollbackTest, createAdditionalClient } from "@fluid-private/test-dds-utils"; import { CellFactory } from "../cellFactory.js"; import type { ISharedCell } from "../interfaces.js"; -interface RollbackTestSetup { - cell: ISharedCell; - dataStoreRuntime: MockFluidDataStoreRuntime; - containerRuntimeFactory: MockContainerRuntimeFactory; - containerRuntime: MockContainerRuntime; -} - -const mapFactory = new CellFactory(); - -function setupRollbackTest(id: string): RollbackTestSetup { - const containerRuntimeFactory = new MockContainerRuntimeFactory({ flushMode: 1 }); - const dataStoreRuntime = new MockFluidDataStoreRuntime({ clientId: "1" }); - const containerRuntime = containerRuntimeFactory.createContainerRuntime(dataStoreRuntime); - const cell = mapFactory.create(dataStoreRuntime, id); - dataStoreRuntime.setAttachState(AttachState.Attached); - cell.connect({ - deltaConnection: dataStoreRuntime.createDeltaConnection(), - objectStorage: new MockStorage(), - }); - return { - cell, - dataStoreRuntime, - containerRuntimeFactory, - containerRuntime, - }; -} - -// Helper to create another client attached to the same containerRuntimeFactory -function createAdditionalClient( - containerRuntimeFactory: MockContainerRuntimeFactory, - id: string = "client-2", -): { - cell: ISharedCell; - dataStoreRuntime: MockFluidDataStoreRuntime; - containerRuntime: MockContainerRuntime; -} { - const dataStoreRuntime = new MockFluidDataStoreRuntime({ clientId: id }); - const containerRuntime = containerRuntimeFactory.createContainerRuntime(dataStoreRuntime); - const cell = mapFactory.create(dataStoreRuntime, `cell-${id}`); - dataStoreRuntime.setAttachState(AttachState.Attached); - cell.connect({ - deltaConnection: dataStoreRuntime.createDeltaConnection(), - objectStorage: new MockStorage(), - }); - return { cell, dataStoreRuntime, containerRuntime }; -} +const cellFactory = new CellFactory(); describe("Cell with rollback", () => { it("should emit valueChanged on set and rollback should re-emit previous value", async () => { - const { cell, containerRuntime } = setupRollbackTest("client-1"); + const { dds: cell, containerRuntime } = setupRollbackTest( + "cell-1", + (rt, id): ISharedCell => cellFactory.create(rt, id), + ); const events: (string | undefined)[] = []; @@ -82,7 +34,14 @@ describe("Cell with rollback", () => { }); it("should emit delete on delete, and rollback should re-emit last valueChanged", async () => { - const { cell, containerRuntimeFactory, containerRuntime } = setupRollbackTest("client-1"); + const { + dds: cell, + containerRuntimeFactory, + containerRuntime, + } = setupRollbackTest( + "cell-1", + (rt, id): ISharedCell => cellFactory.create(rt, id), + ); const events: (string | undefined)[] = []; @@ -109,11 +68,18 @@ describe("SharedCell rollback events with multiple clients", () => { it("should emit valueChanged on set and rollback should re-emit previous value across clients", async () => { // Setup two clients const { - cell: cell1, + dds: cell1, containerRuntimeFactory, containerRuntime: runtime1, - } = setupRollbackTest("client-1"); - const { cell: cell2 } = createAdditionalClient(containerRuntimeFactory); + } = setupRollbackTest( + "client-1", + (rt, id): ISharedCell => cellFactory.create(rt, id), + ); + const { dds: cell2 } = createAdditionalClient( + containerRuntimeFactory, + "client-2", + (rt, id): ISharedCell => cellFactory.create(rt, `cell-${id}`), + ); const events1: string[] = []; const events2: string[] = []; @@ -150,11 +116,18 @@ describe("SharedCell rollback events with multiple clients", () => { it("should emit delete on delete, and rollback should re-emit last valueChanged across clients", async () => { // Setup two clients const { - cell: cell1, + dds: cell1, containerRuntimeFactory, containerRuntime: runtime1, - } = setupRollbackTest("client-1"); - const { cell: cell2 } = createAdditionalClient(containerRuntimeFactory); + } = setupRollbackTest( + "client-1", + (rt, id): ISharedCell => cellFactory.create(rt, id), + ); + const { dds: cell2 } = createAdditionalClient( + containerRuntimeFactory, + "client-2", + (rt, id): ISharedCell => cellFactory.create(rt, `cell-${id}`), + ); const events1: string[] = []; const events2: string[] = []; diff --git a/packages/dds/map/src/test/mocha/directory.rollback.events.spec.ts b/packages/dds/map/src/test/mocha/directory.rollback.events.spec.ts deleted file mode 100644 index 5270c7c0102d..000000000000 --- a/packages/dds/map/src/test/mocha/directory.rollback.events.spec.ts +++ /dev/null @@ -1,489 +0,0 @@ -/*! - * Copyright (c) Microsoft Corporation and contributors. All rights reserved. - * Licensed under the MIT License. - */ - -import { strict as assert } from "node:assert"; - -import { AttachState } from "@fluidframework/container-definitions"; -import { - MockContainerRuntimeFactory, - MockFluidDataStoreRuntime, - MockStorage, - type MockContainerRuntime, -} from "@fluidframework/test-runtime-utils/internal"; - -import { DirectoryFactory } from "../../directoryFactory.js"; -import type { ISharedDirectory, IValueChanged } from "../../interfaces.js"; - -interface RollbackTestSetup { - sharedDirectory: ISharedDirectory; - dataStoreRuntime: MockFluidDataStoreRuntime; - containerRuntimeFactory: MockContainerRuntimeFactory; - containerRuntime: MockContainerRuntime; -} - -const directoryFactory = new DirectoryFactory(); - -function setupRollbackTest(): RollbackTestSetup { - const containerRuntimeFactory = new MockContainerRuntimeFactory({ flushMode: 1 }); // TurnBased - const dataStoreRuntime = new MockFluidDataStoreRuntime({ clientId: "1" }); - const containerRuntime = containerRuntimeFactory.createContainerRuntime(dataStoreRuntime); - const sharedDirectory = directoryFactory.create(dataStoreRuntime, "shared-directory-1"); - dataStoreRuntime.setAttachState(AttachState.Attached); - sharedDirectory.connect({ - deltaConnection: dataStoreRuntime.createDeltaConnection(), - objectStorage: new MockStorage(), - }); - return { - sharedDirectory, - dataStoreRuntime, - containerRuntimeFactory, - containerRuntime, - }; -} - -// Helper to create another client attached to the same containerRuntimeFactory -function createAdditionalClient( - containerRuntimeFactory: MockContainerRuntimeFactory, - id: string = "client-2", -): { - sharedDirectory: ISharedDirectory; - containerRuntime: MockContainerRuntime; -} { - const dataStoreRuntime = new MockFluidDataStoreRuntime({ clientId: id }); - const containerRuntime = containerRuntimeFactory.createContainerRuntime(dataStoreRuntime); - const sharedDirectory = directoryFactory.create(dataStoreRuntime, `shared-directory-${id}`); - dataStoreRuntime.setAttachState(AttachState.Attached); - sharedDirectory.connect({ - deltaConnection: dataStoreRuntime.createDeltaConnection(), - objectStorage: new MockStorage(), - }); - return { sharedDirectory, containerRuntime }; -} - -describe("SharedDirectory rollback and valueChanged event", () => { - let sharedDirectory: ISharedDirectory; - let containerRuntime: MockContainerRuntime; - let containerRuntimeFactory: MockContainerRuntimeFactory; - - beforeEach(() => { - ({ sharedDirectory, containerRuntime, containerRuntimeFactory } = setupRollbackTest()); - }); - - it("should trigger valueChanged event on rollback of set", () => { - const events: IValueChanged[] = []; - sharedDirectory.on("valueChanged", (e) => events.push(e)); - - sharedDirectory.set("key1", "value1"); - assert.strictEqual(sharedDirectory.get("key1"), "value1"); - assert.strictEqual(events.length, 1); - - containerRuntime.rollback?.(); - - assert.strictEqual(sharedDirectory.get("key1"), undefined); - assert.strictEqual(events.length, 2); - assert.strictEqual(events[1].key, "key1"); - assert.strictEqual(events[1].previousValue, "value1"); - }); - - it("should trigger valueChanged event on rollback of delete", () => { - sharedDirectory.set("key1", "value1"); - containerRuntime.flush(); - containerRuntimeFactory.processAllMessages(); - - const events: IValueChanged[] = []; - sharedDirectory.on("valueChanged", (e) => events.push(e)); - - sharedDirectory.delete("key1"); - assert.strictEqual(sharedDirectory.get("key1"), undefined); - assert.strictEqual(events.length, 1); - - containerRuntime.rollback?.(); - - assert.strictEqual(sharedDirectory.get("key1"), "value1"); - assert.strictEqual(events.length, 2); - assert.strictEqual(events[1].key, "key1"); - assert.strictEqual(events[1].previousValue, undefined); - }); - - it("should trigger valueChanged events on rollback of clear", () => { - sharedDirectory.set("key1", "value1"); - sharedDirectory.set("key2", "value2"); - containerRuntime.flush(); - containerRuntimeFactory.processAllMessages(); - - const events: IValueChanged[] = []; - let clears = 0; - sharedDirectory.on("valueChanged", (e) => events.push(e)); - sharedDirectory.on("clear", () => clears++); - - sharedDirectory.clear(); - assert.strictEqual(sharedDirectory.get("key1"), undefined); - assert.strictEqual(sharedDirectory.get("key2"), undefined); - assert.strictEqual(clears, 1); - assert.strictEqual(events.length, 0); - - containerRuntime.rollback?.(); - - assert.strictEqual(sharedDirectory.get("key1"), "value1"); - assert.strictEqual(sharedDirectory.get("key2"), "value2"); - assert.strictEqual(events.length, 2); - assert.strictEqual(events[0].key, "key1"); - assert.strictEqual(events[1].key, "key2"); - }); -}); - -describe("SharedDirectory rollback and valueChanged event with multiple clients", () => { - let client1: ISharedDirectory; - let client2: ISharedDirectory; - let containerRuntime1: MockContainerRuntime; - let containerRuntime2: MockContainerRuntime; - let containerRuntimeFactory: MockContainerRuntimeFactory; - - beforeEach(() => { - ({ - sharedDirectory: client1, - containerRuntimeFactory, - containerRuntime: containerRuntime1, - } = setupRollbackTest()); - - ({ sharedDirectory: client2, containerRuntime: containerRuntime2 } = - createAdditionalClient(containerRuntimeFactory, "2")); - }); - - it("should trigger valueChanged event on rollback of set for one client", () => { - const eventsClient1: IValueChanged[] = []; - const eventsClient2: IValueChanged[] = []; - - client1.on("valueChanged", (e) => eventsClient1.push(e)); - client2.on("valueChanged", (e) => eventsClient2.push(e)); - - client1.set("key1", "value1"); - assert.strictEqual(client1.get("key1"), "value1"); - - // Rollback only affects client1's local changes - containerRuntime1.rollback?.(); - - assert.strictEqual(client1.get("key1"), undefined); - assert.strictEqual(eventsClient1.length, 2); - assert.strictEqual(eventsClient1[0].key, "key1"); - assert.strictEqual(eventsClient1[0].previousValue, "value1"); - - // client2 never had the change locally, so no events - assert.strictEqual(eventsClient2.length, 0); - }); - - it("should trigger valueChanged event on rollback of delete for one client", () => { - client1.set("key1", "value1"); - containerRuntime1.flush(); - containerRuntimeFactory.processAllMessages(); - - const eventsClient1: IValueChanged[] = []; - const eventsClient2: IValueChanged[] = []; - client1.on("valueChanged", (e) => eventsClient1.push(e)); - client2.on("valueChanged", (e) => eventsClient2.push(e)); - - client1.delete("key1"); - assert.strictEqual(client1.get("key1"), undefined); - - containerRuntime1.rollback?.(); - - assert.strictEqual(client1.get("key1"), "value1"); - assert.strictEqual(eventsClient1.length, 2); - assert.strictEqual(eventsClient1[0].key, "key1"); - assert.strictEqual(eventsClient1[0].previousValue, undefined); - - assert.strictEqual(eventsClient2.length, 0); - }); - - it("should trigger valueChanged events on rollback of clear for one client", () => { - client1.set("key1", "value1"); - client1.set("key2", "value2"); - containerRuntime1.flush(); - containerRuntimeFactory.processAllMessages(); - - const eventsClient1: IValueChanged[] = []; - const eventsClient2: IValueChanged[] = []; - let clearsClient1 = 0; - let clearsClient2 = 0; - - client1.on("valueChanged", (e) => eventsClient1.push(e)); - client2.on("valueChanged", (e) => eventsClient2.push(e)); - - client1.on("clear", () => clearsClient1++); - client2.on("clear", () => clearsClient2++); - - client1.clear(); - assert.strictEqual(client1.get("key1"), undefined); - assert.strictEqual(client1.get("key2"), undefined); - assert.strictEqual(clearsClient1, 1); - assert.strictEqual(eventsClient1.length, 0); - - containerRuntime1.rollback?.(); - - assert.strictEqual(client1.get("key1"), "value1"); - assert.strictEqual(client1.get("key2"), "value2"); - assert.strictEqual(eventsClient1.length, 2); - assert.strictEqual(eventsClient1[0].key, "key1"); - assert.strictEqual(eventsClient1[1].key, "key2"); - - // client2 should remain unaffected - assert.strictEqual(eventsClient2.length, 0); - assert.strictEqual(clearsClient2, 0); - }); -}); - -describe("SharedDirectory rollback event correctness with multiple clients", () => { - let client1: ISharedDirectory; - let client2: ISharedDirectory; - let containerRuntime1: MockContainerRuntime; - let containerRuntime2: MockContainerRuntime; - let containerRuntimeFactory: MockContainerRuntimeFactory; - - beforeEach(() => { - ({ - sharedDirectory: client1, - containerRuntimeFactory, - containerRuntime: containerRuntime1, - } = setupRollbackTest()); - - ({ sharedDirectory: client2, containerRuntime: containerRuntime2 } = - createAdditionalClient(containerRuntimeFactory, "2")); - }); - - it("should fire valueChanged events for every state change, not just match final state", () => { - const eventsClient1: IValueChanged[] = []; - const eventsClient2: IValueChanged[] = []; - - client1.on("valueChanged", (e) => eventsClient1.push(e)); - client2.on("valueChanged", (e) => eventsClient2.push(e)); - - // Step 1: Client1 sets key1 - client1.set("key1", "value1"); - assert.strictEqual(client1.get("key1"), "value1"); - assert.strictEqual(client2.get("key1"), "value1"); - - // Step 2: Client2 sets key1 to a different value - client2.set("key1", "value2"); - assert.strictEqual(client1.get("key1"), "value2"); - assert.strictEqual(client2.get("key1"), "value2"); - - // Step 3: Client1 deletes key1 - client1.delete("key1"); - assert.strictEqual(client1.get("key1"), undefined); - assert.strictEqual(client2.get("key1"), undefined); - - // Rollback Client1 - containerRuntime1.rollback?.(); - - // Final state should match expected rollback state - assert.strictEqual(client1.get("key1"), "value2"); - assert.strictEqual(client2.get("key1"), "value2"); - - // Check that events for **each intermediate state change** fired correctly - assert.deepStrictEqual( - eventsClient1.map(({ key, previousValue }: IValueChanged) => ({ - key, - // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment - previousValue, - })), - [ - { key: "key1", previousValue: undefined }, // set to "value1" - { key: "key1", previousValue: "value1" }, // overwritten by client2 - { key: "key1", previousValue: "value2" }, // deleted - { key: "key1", previousValue: undefined }, // rollback restores to "value2" - ], - ); - - // Client2 did not rollback, so should see events for the set from client1 and its own set - assert.deepStrictEqual( - // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment - eventsClient2.map(({ key, previousValue }): IValueChanged => ({ key, previousValue })), - [ - { key: "key1", previousValue: undefined }, // initial set by client1 - { key: "key1", previousValue: "value1" }, // set to value2 by itself - { key: "key1", previousValue: "value2" }, // deletion from client1 - ], - ); - }); -}); - -describe("SharedDirectory rollback events", () => { - let sharedDirectory: ISharedDirectory; - let containerRuntime: MockContainerRuntime; - let containerRuntimeFactory: MockContainerRuntimeFactory; - - beforeEach(() => { - ({ sharedDirectory, containerRuntime, containerRuntimeFactory } = setupRollbackTest()); - }); - - it("should trigger subDirectoryDeleted event on rollback of subDirectoryCreated", () => { - const events: { path: string; local: boolean; target: ISharedDirectory }[] = []; - - sharedDirectory.on("subDirectoryDeleted", (path, local, target) => { - events.push({ path, local, target }); - }); - - // Create a subdirectory - sharedDirectory.createSubDirectory("subDir1"); - assert( - sharedDirectory.getWorkingDirectory("subDir1") !== undefined, - "subdirectory should exist after creation", - ); - - // Rollback - containerRuntime.rollback?.(); - - // The subdirectory should be removed - assert.strictEqual( - sharedDirectory.getWorkingDirectory("subDir1"), - undefined, - "subdirectory should be removed after rollback", - ); - - // The subDirectoryDeleted event should be triggered - assert.strictEqual( - events.length, - 1, - "one subDirectoryDeleted event should have been emitted", - ); - assert.strictEqual(events[0].path, "subDir1"); - assert.strictEqual(events[0].local, true); - assert.strictEqual(events[0].target, sharedDirectory); - }); - - it("should trigger subDirectoryCreated event on rollback of subDirectoryDeleted", () => { - const events: { path: string; local: boolean; target: ISharedDirectory }[] = []; - - sharedDirectory.createSubDirectory("subDir2"); - - sharedDirectory.on("subDirectoryCreated", (path, local, target) => { - events.push({ path, local, target }); - }); - - // Delete the subdirectory - sharedDirectory.deleteSubDirectory("subDir2"); - assert.strictEqual( - sharedDirectory.getWorkingDirectory("subDir2"), - undefined, - "subdirectory should be deleted", - ); - - // Rollback - containerRuntime.rollback?.(); - - // The subdirectory should be restored - assert( - sharedDirectory.get("subDir2") !== undefined, - "subdirectory should exist after rollback", - ); - - // The subDirectoryCreated event should be triggered - assert.strictEqual( - events.length, - 1, - "one subDirectoryCreated event should have been emitted", - ); - assert.strictEqual(events[0].path, "subDir2"); - assert.strictEqual(events[0].local, true); - assert.strictEqual(events[0].target, sharedDirectory); - }); -}); - -describe("SharedDirectory rollback with multiple clients", () => { - let client1: ISharedDirectory; - let client2: ISharedDirectory; - let containerRuntime1: MockContainerRuntime; - let containerRuntime2: MockContainerRuntime; - let containerRuntimeFactory: MockContainerRuntimeFactory; - - beforeEach(() => { - ({ - sharedDirectory: client1, - containerRuntimeFactory, - containerRuntime: containerRuntime1, - } = setupRollbackTest()); - - ({ sharedDirectory: client2, containerRuntime: containerRuntime2 } = - createAdditionalClient(containerRuntimeFactory, "2")); - }); - - it("should only rollback local subDirectoryCreated changes for one client", () => { - const eventsClient1: { path: string; local: boolean; target: ISharedDirectory }[] = []; - const eventsClient2: { path: string; local: boolean; target: ISharedDirectory }[] = []; - - client1.on("subDirectoryDeleted", (path, local, target) => - eventsClient1.push({ path, local, target }), - ); - client2.on("subDirectoryDeleted", (path, local, target) => - eventsClient2.push({ path, local, target }), - ); - - // Client1 creates a subdirectory - client1.createSubDirectory("sharedDir"); - - // Client2 creates a subdirectory - client2.createSubDirectory("sharedDir2"); - - // Check that directories exist - assert(client1.get("sharedDir") !== undefined); - assert(client2.get("sharedDir2") !== undefined); - - // Rollback only affects client1's local changes - containerRuntime1.rollback?.(); - - // Client1's local subdirectory should be gone - assert.strictEqual(client1.get("sharedDir"), undefined); - - // Client2's subdirectory remains - assert(client1.get("sharedDir2") !== undefined); - - // Events triggered - assert.strictEqual(eventsClient1.length, 1); - assert.strictEqual(eventsClient1[0].path, "sharedDir"); - assert(eventsClient1[0].local); - - // Client2 did not get any subDirectoryDeleted events for client1's rollback - assert.strictEqual(eventsClient2.length, 0); - }); - - it("should rollback local subDirectoryDeleted changes for one client", () => { - const eventsClient1: { path: string; local: boolean; target: ISharedDirectory }[] = []; - const eventsClient2: { path: string; local: boolean; target: ISharedDirectory }[] = []; - - // Create initial directories - client1.createSubDirectory("dir1"); - client2.createSubDirectory("dir2"); - - client1.on("subDirectoryCreated", (path, local, target) => - eventsClient1.push({ path, local, target }), - ); - client2.on("subDirectoryCreated", (path, local, target) => - eventsClient2.push({ path, local, target }), - ); - - // Client1 deletes its own directory - client1.deleteSubDirectory("dir1"); - - // Client2 deletes its own directory - client2.deleteSubDirectory("dir2"); - - // Rollback client1 - containerRuntime1.rollback?.(); - - // Client1's directory should be restored - assert(client1.get("dir1") !== undefined); - - // Client2's directory remains deleted - assert.strictEqual(client1.get("dir2"), undefined); - - // Event triggered for client1 - assert.strictEqual(eventsClient1.length, 1); - assert.strictEqual(eventsClient1[0].path, "dir1"); - assert(eventsClient1[0].local); - - // Client2 did not get any subDirectoryCreated events from client1 rollback - assert.strictEqual(eventsClient2.length, 0); - }); -}); diff --git a/packages/dds/map/src/test/mocha/directory.rollback.spec.ts b/packages/dds/map/src/test/mocha/directory.rollback.spec.ts index 4359b972c7c0..f1edac0a8ce0 100644 --- a/packages/dds/map/src/test/mocha/directory.rollback.spec.ts +++ b/packages/dds/map/src/test/mocha/directory.rollback.spec.ts @@ -5,72 +5,40 @@ import { strict as assert } from "node:assert"; -import { AttachState } from "@fluidframework/container-definitions"; -import { +import { setupRollbackTest, createAdditionalClient } from "@fluid-private/test-dds-utils"; +import type { MockContainerRuntimeFactory, - MockFluidDataStoreRuntime, - MockStorage, - type MockContainerRuntime, + MockContainerRuntime, } from "@fluidframework/test-runtime-utils/internal"; import { DirectoryFactory } from "../../directoryFactory.js"; import type { ISharedDirectory, IValueChanged } from "../../interfaces.js"; -interface RollbackTestSetup { - sharedDirectory: ISharedDirectory; - dataStoreRuntime: MockFluidDataStoreRuntime; - containerRuntimeFactory: MockContainerRuntimeFactory; - containerRuntime: MockContainerRuntime; -} - const directoryFactory = new DirectoryFactory(); -function setupRollbackTest(): RollbackTestSetup { - const containerRuntimeFactory = new MockContainerRuntimeFactory({ flushMode: 1 }); // TurnBased - const dataStoreRuntime = new MockFluidDataStoreRuntime({ clientId: "1" }); - const containerRuntime = containerRuntimeFactory.createContainerRuntime(dataStoreRuntime); - const sharedDirectory = directoryFactory.create(dataStoreRuntime, "shared-directory-1"); - dataStoreRuntime.setAttachState(AttachState.Attached); - sharedDirectory.connect({ - deltaConnection: dataStoreRuntime.createDeltaConnection(), - objectStorage: new MockStorage(), - }); - return { - sharedDirectory, - dataStoreRuntime, - containerRuntimeFactory, - containerRuntime, - }; -} - -// Helper to create another client attached to the same containerRuntimeFactory -function createAdditionalClient( - containerRuntimeFactory: MockContainerRuntimeFactory, - id: string = "client-2", -): { - sharedDirectory: ISharedDirectory; - dataStoreRuntime: MockFluidDataStoreRuntime; - containerRuntime: MockContainerRuntime; -} { - const dataStoreRuntime = new MockFluidDataStoreRuntime({ clientId: id }); - const containerRuntime = containerRuntimeFactory.createContainerRuntime(dataStoreRuntime); - const sharedDirectory = directoryFactory.create(dataStoreRuntime, `shared-directory-${id}`); - dataStoreRuntime.setAttachState(AttachState.Attached); - sharedDirectory.connect({ - deltaConnection: dataStoreRuntime.createDeltaConnection(), - objectStorage: new MockStorage(), - }); - return { sharedDirectory, dataStoreRuntime, containerRuntime }; -} - describe("SharedDirectory rollback", () => { describe("Storage operations (root subdirectory)", () => { + let sharedDirectory: ISharedDirectory; + let containerRuntime: MockContainerRuntime; + let containerRuntimeFactory: MockContainerRuntimeFactory; + let sharedDirectory2: ISharedDirectory; + let containerRuntime2: MockContainerRuntime; + + beforeEach(() => { + ({ + dds: sharedDirectory, + containerRuntimeFactory, + containerRuntime, + } = setupRollbackTest( + "client-1", + (rt, id): ISharedDirectory => directoryFactory.create(rt, id), + )); + }); + it("should rollback set operation", () => { - const { sharedDirectory, containerRuntime } = setupRollbackTest(); const valueChanges: IValueChanged[] = []; - sharedDirectory.on("valueChanged", (event: IValueChanged) => { - valueChanges.push(event); - }); + sharedDirectory.on("valueChanged", (event: IValueChanged) => valueChanges.push(event)); + sharedDirectory.set("key1", "value1"); assert.strictEqual( sharedDirectory.get("key1"), @@ -78,124 +46,76 @@ describe("SharedDirectory rollback", () => { "Failed getting pending value", ); assert.strictEqual(valueChanges.length, 1, "Should have one value change event"); + containerRuntime.rollback?.(); + assert.strictEqual( sharedDirectory.get("key1"), undefined, "Value should be rolled back", ); assert.strictEqual(valueChanges.length, 2, "Should have two value change events"); - assert.strictEqual(valueChanges[1].key, "key1", "Second event should be for key1"); - assert.strictEqual( - valueChanges[1].previousValue, - "value1", - "Second event previousValue should be pre-rollback value", - ); + assert.strictEqual(valueChanges[1].key, "key1"); + assert.strictEqual(valueChanges[1].previousValue, "value1"); }); it("should rollback delete operation", () => { - const { sharedDirectory, containerRuntimeFactory, containerRuntime } = - setupRollbackTest(); sharedDirectory.set("key1", "value1"); containerRuntime.flush(); containerRuntimeFactory.processAllMessages(); + const valueChanges: IValueChanged[] = []; - sharedDirectory.on("valueChanged", (event: IValueChanged) => { - valueChanges.push(event); - }); + sharedDirectory.on("valueChanged", (event: IValueChanged) => valueChanges.push(event)); + sharedDirectory.delete("key1"); - assert.strictEqual( - sharedDirectory.get("key1"), - undefined, - "Pending value should reflect the delete", - ); - assert.strictEqual(valueChanges.length, 1, "Should have one value change event"); + assert.strictEqual(sharedDirectory.get("key1"), undefined); + assert.strictEqual(valueChanges.length, 1); + containerRuntime.rollback?.(); - assert.strictEqual( - sharedDirectory.get("key1"), - "value1", - "Value should be restored by rollback", - ); - assert.strictEqual(valueChanges.length, 2, "Should have two value change events"); - assert.strictEqual(valueChanges[1].key, "key1", "Second event should be for key1"); - assert.strictEqual( - valueChanges[1].previousValue, - undefined, - "Second event previousValue should be pre-rollback value", - ); + + assert.strictEqual(sharedDirectory.get("key1"), "value1"); + assert.strictEqual(valueChanges.length, 2); + assert.strictEqual(valueChanges[1].key, "key1"); + assert.strictEqual(valueChanges[1].previousValue, undefined); }); it("should rollback clear operation", () => { - const { sharedDirectory, containerRuntimeFactory, containerRuntime } = - setupRollbackTest(); sharedDirectory.set("key1", "value1"); sharedDirectory.set("key2", "value2"); containerRuntime.flush(); containerRuntimeFactory.processAllMessages(); + const valueChanges: IValueChanged[] = []; - let clears: number = 0; - sharedDirectory.on("valueChanged", (event: IValueChanged) => { - valueChanges.push(event); - }); - sharedDirectory.on("clear", () => { - clears++; - }); + let clears = 0; + sharedDirectory.on("valueChanged", (e) => valueChanges.push(e)); + sharedDirectory.on("clear", () => clears++); sharedDirectory.clear(); - assert.strictEqual( - sharedDirectory.get("key1"), - undefined, - "Pending value for key1 should reflect the clear", - ); - assert.strictEqual( - sharedDirectory.get("key2"), - undefined, - "Pending value for key2 should reflect the clear", - ); - assert.strictEqual(valueChanges.length, 0, "Should have no value change events"); - assert.strictEqual(clears, 1, "Should have one clear event"); + assert.strictEqual(sharedDirectory.get("key1"), undefined); + assert.strictEqual(sharedDirectory.get("key2"), undefined); + assert.strictEqual(valueChanges.length, 0); + assert.strictEqual(clears, 1); + containerRuntime.rollback?.(); - assert.strictEqual( - sharedDirectory.get("key1"), - "value1", - "Value should be restored by rollback", - ); - assert.strictEqual( - sharedDirectory.get("key2"), - "value2", - "Value should be restored by rollback", - ); - assert.strictEqual(valueChanges.length, 2, "Should have two value change events"); - assert.strictEqual(valueChanges[0].key, "key1", "First event should be for key1"); - assert.strictEqual( - valueChanges[0].previousValue, - undefined, - "First event previousValue should be pre-rollback value", - ); - assert.strictEqual(valueChanges[1].key, "key2", "Second event should be for key2"); - assert.strictEqual( - valueChanges[1].previousValue, - undefined, - "Second event previousValue should be pre-rollback value", - ); + + assert.strictEqual(sharedDirectory.get("key1"), "value1"); + assert.strictEqual(sharedDirectory.get("key2"), "value2"); + assert.strictEqual(valueChanges.length, 2); + assert.strictEqual(valueChanges[0].key, "key1"); + assert.strictEqual(valueChanges[1].key, "key2"); }); it("should rollback multiple operations in sequence", () => { - const { sharedDirectory, containerRuntimeFactory, containerRuntime } = - setupRollbackTest(); sharedDirectory.set("key1", "value1"); sharedDirectory.set("key2", "value2"); containerRuntime.flush(); containerRuntimeFactory.processAllMessages(); + let valueChanges: IValueChanged[] = []; - let clears: number = 0; - sharedDirectory.on("valueChanged", (event: IValueChanged) => { - valueChanges.push(event); - }); - sharedDirectory.on("clear", () => { - clears++; - }); + let clears = 0; + sharedDirectory.on("valueChanged", (e) => valueChanges.push(e)); + sharedDirectory.on("clear", () => clears++); sharedDirectory.set("key3", "value3"); sharedDirectory.delete("key1"); @@ -203,29 +123,10 @@ describe("SharedDirectory rollback", () => { sharedDirectory.clear(); sharedDirectory.set("key4", "value4"); - assert.deepStrictEqual( - [...sharedDirectory.entries()], - [["key4", "value4"]], - "Directory should have expected entries pre-rollback", - ); - assert.deepStrictEqual( - valueChanges, - [ - // Set key3 - { key: "key3", path: "/", previousValue: undefined }, - // Delete key1 - { key: "key1", path: "/", previousValue: "value1" }, - // Set key2 to a new value - { key: "key2", path: "/", previousValue: "value2" }, - // Clear happens here, no valueChange event for clear - // Set key4 - { key: "key4", path: "/", previousValue: undefined }, - ], - "Value changes should match expected pre-rollback", - ); - assert.strictEqual(clears, 1, "Should have one clear event"); + assert.deepStrictEqual([...sharedDirectory.entries()], [["key4", "value4"]]); + assert.strictEqual(clears, 1); - // Reset event monitoring and roll back. + // Reset events and rollback valueChanges = []; clears = 0; containerRuntime.rollback?.(); @@ -236,34 +137,16 @@ describe("SharedDirectory rollback", () => { ["key1", "value1"], ["key2", "value2"], ], - "Directory should have expected entries post-rollback", ); - assert.deepStrictEqual( - valueChanges, - [ - // Roll back the final key4 set - { key: "key4", path: "/", previousValue: "value4" }, - // Roll back the clear - { key: "key2", path: "/", previousValue: undefined }, - { key: "key3", path: "/", previousValue: undefined }, - // Roll back the key2 set - { key: "key2", path: "/", previousValue: "newValue2" }, - // Roll back the key1 delete - { key: "key1", path: "/", previousValue: undefined }, - // Roll back the key3 set - { key: "key3", path: "/", previousValue: "value3" }, - ], - "Value changes should match expected post-rollback", - ); - assert.strictEqual(clears, 0, "Should have no clear events"); + assert.strictEqual(clears, 0); }); it("should rollback local changes in presence of remote changes from another client", () => { - const { sharedDirectory, containerRuntimeFactory, containerRuntime } = - setupRollbackTest(); - // Create a second client - const { sharedDirectory: sharedDirectory2, containerRuntime: containerRuntime2 } = - createAdditionalClient(containerRuntimeFactory); + ({ dds: sharedDirectory2, containerRuntime: containerRuntime2 } = createAdditionalClient( + containerRuntimeFactory, + "client-2", + (rt, id): ISharedDirectory => directoryFactory.create(rt, `cell-${id}`), + )); sharedDirectory.set("key1", "value1"); sharedDirectory.set("key2", "value2"); @@ -283,12 +166,9 @@ describe("SharedDirectory rollback", () => { assert.deepStrictEqual( [...sharedDirectory.entries()], [ - // Note key4 comes before key3 even though we have an optimistic value for it, - // because sharedDirectory2 set them in that order. Iteration order matches the sequenced perspective. ["key4", "value4"], ["key3", "value3"], ], - "Directory should have expected entries pre-rollback", ); containerRuntime.rollback?.(); @@ -300,16 +180,28 @@ describe("SharedDirectory rollback", () => { ["key4", "value4"], ["key3", "otherValue3"], ], - "Directory should have expected entries post-rollback", ); }); }); describe("Storage operations (nested subdirectories)", () => { + let sharedDirectory: ISharedDirectory; + let containerRuntime: MockContainerRuntime; + let containerRuntimeFactory: MockContainerRuntimeFactory; + let sharedDirectory2: ISharedDirectory; + let containerRuntime2: MockContainerRuntime; + + beforeEach(() => { + ({ + dds: sharedDirectory, + containerRuntimeFactory, + containerRuntime, + } = setupRollbackTest( + "client-1", + (rt, id): ISharedDirectory => directoryFactory.create(rt, id), + )); + }); it("should rollback all basic operations (set, delete, clear) in subdirectories and nested subdirectories", () => { - const { sharedDirectory, containerRuntimeFactory, containerRuntime } = - setupRollbackTest(); - const subDir = sharedDirectory.createSubDirectory("subdir"); const level1 = sharedDirectory.createSubDirectory("level1"); const level3 = level1.createSubDirectory("level2").createSubDirectory("level3"); @@ -383,10 +275,11 @@ describe("SharedDirectory rollback", () => { }); it("should rollback subdirectory operations with concurrent remote changes", () => { - const { sharedDirectory, containerRuntimeFactory, containerRuntime } = - setupRollbackTest(); - const { sharedDirectory: sharedDirectory2, containerRuntime: containerRuntime2 } = - createAdditionalClient(containerRuntimeFactory); + ({ dds: sharedDirectory2, containerRuntime: containerRuntime2 } = createAdditionalClient( + containerRuntimeFactory, + "client-2", + (rt, id): ISharedDirectory => directoryFactory.create(rt, `cell-${id}`), + )); const subDir1 = sharedDirectory.createSubDirectory("shared"); const nestedDir1 = subDir1.createSubDirectory("nested"); @@ -443,9 +336,6 @@ describe("SharedDirectory rollback", () => { }); it("should rollback complex mixed operations across multiple subdirectory levels", () => { - const { sharedDirectory, containerRuntimeFactory, containerRuntime } = - setupRollbackTest(); - const dirA = sharedDirectory.createSubDirectory("dirA"); const dirB = sharedDirectory.createSubDirectory("dirB"); const nestedDir = dirA.createSubDirectory("nested"); @@ -523,3 +413,195 @@ describe("SharedDirectory rollback", () => { }); }); }); + +describe("SharedDirectory rollback events", () => { + let sharedDirectory: ISharedDirectory; + let containerRuntime: MockContainerRuntime; + let containerRuntimeFactory: MockContainerRuntimeFactory; + + beforeEach(() => { + ({ + dds: sharedDirectory, + containerRuntime, + containerRuntimeFactory, + } = setupRollbackTest( + "shared-map", + (rt, id): ISharedDirectory => directoryFactory.create(rt, id), + )); + }); + + it("should trigger subDirectoryDeleted event on rollback of subDirectoryCreated", () => { + const events: { path: string; local: boolean; target: ISharedDirectory }[] = []; + + sharedDirectory.on("subDirectoryDeleted", (path, local, target) => { + events.push({ path, local, target }); + }); + + // Create a subdirectory + sharedDirectory.createSubDirectory("subDir1"); + assert( + sharedDirectory.getWorkingDirectory("subDir1") !== undefined, + "subdirectory should exist after creation", + ); + + // Rollback + containerRuntime.rollback?.(); + + // The subdirectory should be removed + assert.strictEqual( + sharedDirectory.getWorkingDirectory("subDir1"), + undefined, + "subdirectory should be removed after rollback", + ); + + // The subDirectoryDeleted event should be triggered + assert.strictEqual( + events.length, + 1, + "one subDirectoryDeleted event should have been emitted", + ); + assert.strictEqual(events[0].path, "subDir1"); + assert.strictEqual(events[0].local, true); + assert.strictEqual(events[0].target, sharedDirectory); + }); + + it("should trigger subDirectoryCreated event on rollback of subDirectoryDeleted", () => { + const events: { path: string; local: boolean; target: ISharedDirectory }[] = []; + + sharedDirectory.createSubDirectory("subDir2"); + + sharedDirectory.on("subDirectoryCreated", (path, local, target) => { + events.push({ path, local, target }); + }); + + // Delete the subdirectory + sharedDirectory.deleteSubDirectory("subDir2"); + assert.strictEqual( + sharedDirectory.getWorkingDirectory("subDir2"), + undefined, + "subdirectory should be deleted", + ); + + // Rollback + containerRuntime.rollback?.(); + + // The subdirectory should be restored + assert( + sharedDirectory.get("subDir2") !== undefined, + "subdirectory should exist after rollback", + ); + + // The subDirectoryCreated event should be triggered + assert.strictEqual( + events.length, + 1, + "one subDirectoryCreated event should have been emitted", + ); + assert.strictEqual(events[0].path, "subDir2"); + assert.strictEqual(events[0].local, true); + assert.strictEqual(events[0].target, sharedDirectory); + }); +}); + +describe("SharedDirectory rollback with multiple clients", () => { + let client1: ISharedDirectory; + let client2: ISharedDirectory; + let containerRuntime1: MockContainerRuntime; + let containerRuntime2: MockContainerRuntime; + let containerRuntimeFactory: MockContainerRuntimeFactory; + + beforeEach(() => { + ({ + dds: client1, + containerRuntime: containerRuntime1, + containerRuntimeFactory, + } = setupRollbackTest( + "client-1", + (rt, id): ISharedDirectory => directoryFactory.create(rt, id), + )); + + ({ dds: client2, containerRuntime: containerRuntime2 } = createAdditionalClient( + containerRuntimeFactory, + "client-2", + (rt, id): ISharedDirectory => directoryFactory.create(rt, `directory-${id}`), + )); + }); + + it("should only rollback local subDirectoryCreated changes for one client", () => { + const eventsClient1: { path: string; local: boolean; target: ISharedDirectory }[] = []; + const eventsClient2: { path: string; local: boolean; target: ISharedDirectory }[] = []; + + client1.on("subDirectoryDeleted", (path, local, target) => + eventsClient1.push({ path, local, target }), + ); + client2.on("subDirectoryDeleted", (path, local, target) => + eventsClient2.push({ path, local, target }), + ); + + // Client1 creates a subdirectory + client1.createSubDirectory("sharedDir"); + + // Client2 creates a subdirectory + client2.createSubDirectory("sharedDir2"); + + // Check that directories exist + assert(client1.get("sharedDir") !== undefined); + assert(client2.get("sharedDir2") !== undefined); + + // Rollback only affects client1's local changes + containerRuntime1.rollback?.(); + + // Client1's local subdirectory should be gone + assert.strictEqual(client1.get("sharedDir"), undefined); + + // Client2's subdirectory remains + assert(client1.get("sharedDir2") !== undefined); + + // Events triggered + assert.strictEqual(eventsClient1.length, 1); + assert.strictEqual(eventsClient1[0].path, "sharedDir"); + assert(eventsClient1[0].local); + + // Client2 did not get any subDirectoryDeleted events for client1's rollback + assert.strictEqual(eventsClient2.length, 0); + }); + + it("should rollback local subDirectoryDeleted changes for one client", () => { + const eventsClient1: { path: string; local: boolean; target: ISharedDirectory }[] = []; + const eventsClient2: { path: string; local: boolean; target: ISharedDirectory }[] = []; + + // Create initial directories + client1.createSubDirectory("dir1"); + client2.createSubDirectory("dir2"); + + client1.on("subDirectoryCreated", (path, local, target) => + eventsClient1.push({ path, local, target }), + ); + client2.on("subDirectoryCreated", (path, local, target) => + eventsClient2.push({ path, local, target }), + ); + + // Client1 deletes its own directory + client1.deleteSubDirectory("dir1"); + + // Client2 deletes its own directory + client2.deleteSubDirectory("dir2"); + + // Rollback client1 + containerRuntime1.rollback?.(); + + // Client1's directory should be restored + assert(client1.get("dir1") !== undefined); + + // Client2's directory remains deleted + assert.strictEqual(client1.get("dir2"), undefined); + + // Event triggered for client1 + assert.strictEqual(eventsClient1.length, 1); + assert.strictEqual(eventsClient1[0].path, "dir1"); + assert(eventsClient1[0].local); + + // Client2 did not get any subDirectoryCreated events from client1 rollback + assert.strictEqual(eventsClient2.length, 0); + }); +}); diff --git a/packages/dds/sequence/src/test/sharedString.rollback.spec.ts b/packages/dds/sequence/src/test/sharedString.rollback.spec.ts index 2193a986a0b7..965656e18410 100644 --- a/packages/dds/sequence/src/test/sharedString.rollback.spec.ts +++ b/packages/dds/sequence/src/test/sharedString.rollback.spec.ts @@ -5,78 +5,47 @@ import { strict as assert } from "assert"; -import { AttachState } from "@fluidframework/container-definitions/internal"; +import { setupRollbackTest, createAdditionalClient } from "@fluid-private/test-dds-utils"; import { MergeTreeDeltaType } from "@fluidframework/merge-tree/internal"; -import { +import type { + MockContainerRuntime, MockContainerRuntimeFactory, - MockFluidDataStoreRuntime, - MockStorage, - type MockContainerRuntime, } from "@fluidframework/test-runtime-utils/internal"; import { SharedStringFactory, type SharedString } from "../sequenceFactory.js"; import { SharedStringClass } from "../sharedString.js"; -function setupSharedStringRollbackTest() { - const containerRuntimeFactory = new MockContainerRuntimeFactory({ flushMode: 1 }); - const dataStoreRuntime = new MockFluidDataStoreRuntime({ clientId: "1" }); - const containerRuntime = containerRuntimeFactory.createContainerRuntime(dataStoreRuntime); - const sharedString = new SharedStringClass( - dataStoreRuntime, - "shared-string-1", - SharedStringFactory.Attributes, - ); - dataStoreRuntime.setAttachState(AttachState.Attached); - sharedString.initializeLocal(); - sharedString.connect({ - deltaConnection: dataStoreRuntime.createDeltaConnection(), - objectStorage: new MockStorage(), - }); - return { - sharedString, - dataStoreRuntime, - containerRuntimeFactory, - containerRuntime, - }; -} - -// Helper to create another client attached to the same containerRuntimeFactory -function createAdditionalClient( - containerRuntimeFactory: MockContainerRuntimeFactory, - id: string = "client-2", -): { - sharedString: SharedStringClass; - dataStoreRuntime: MockFluidDataStoreRuntime; - containerRuntime: MockContainerRuntime; -} { - const dataStoreRuntime = new MockFluidDataStoreRuntime({ clientId: id }); - const containerRuntime = containerRuntimeFactory.createContainerRuntime(dataStoreRuntime); - const sharedString = new SharedStringClass( - dataStoreRuntime, - `shared-string-${id}`, - SharedStringFactory.Attributes, - ); - dataStoreRuntime.setAttachState(AttachState.Attached); - sharedString.initializeLocal(); - sharedString.connect({ - deltaConnection: dataStoreRuntime.createDeltaConnection(), - objectStorage: new MockStorage(), - }); - return { sharedString, dataStoreRuntime, containerRuntime }; -} - describe("SharedString rollback with multiple clients (insert/remove)", () => { - it("Client1 insert + Client2 insert + rollback on Client1", () => { - const { - sharedString: client1, - containerRuntimeFactory, - containerRuntime: cr1, - } = setupSharedStringRollbackTest(); - const { sharedString: client2, containerRuntime: cr2 } = createAdditionalClient( + let client1: SharedStringClass; + let client2: SharedStringClass; + let cr1: MockContainerRuntime; + let cr2: MockContainerRuntime; + let containerRuntimeFactory: MockContainerRuntimeFactory; + + beforeEach(() => { + // First client + const setup = setupRollbackTest( + "shared-string-1", + (rt, id) => new SharedStringClass(rt, id, SharedStringFactory.Attributes), + { initialize: (dds) => dds.initializeLocal() }, + ); + client1 = setup.dds; + containerRuntimeFactory = setup.containerRuntimeFactory; + cr1 = setup.containerRuntime; + + // Second client + const additional = createAdditionalClient( containerRuntimeFactory, "2", + (rt, id) => + new SharedStringClass(rt, `shared-string-${id}`, SharedStringFactory.Attributes), + { initialize: (dds) => dds.initializeLocal() }, ); + client2 = additional.dds; + cr2 = additional.containerRuntime; + }); + it("Client1 insert + Client2 insert + rollback on Client1", () => { // Baseline text client1.insertText(0, "hello"); cr1.flush(); @@ -117,16 +86,6 @@ describe("SharedString rollback with multiple clients (insert/remove)", () => { }); it("Client1 remove + Client2 insert + rollback on Client1", () => { - const { - sharedString: client1, - containerRuntimeFactory, - containerRuntime: cr1, - } = setupSharedStringRollbackTest(); - const { sharedString: client2, containerRuntime: cr2 } = createAdditionalClient( - containerRuntimeFactory, - "2", - ); - // Baseline text client1.insertText(0, "abcdef"); cr1.flush(); @@ -160,16 +119,6 @@ describe("SharedString rollback with multiple clients (insert/remove)", () => { }); it("Client1 insert + Client2 remove + rollback on Client1", () => { - const { - sharedString: client1, - containerRuntimeFactory, - containerRuntime: cr1, - } = setupSharedStringRollbackTest(); - const { sharedString: client2, containerRuntime: cr2 } = createAdditionalClient( - containerRuntimeFactory, - "2", - ); - // Baseline text client1.insertText(0, "123456"); cr1.flush(); @@ -199,16 +148,6 @@ describe("SharedString rollback with multiple clients (insert/remove)", () => { }); it("Client1 insert + Client2 insert + rollback on Client2", () => { - const { - sharedString: client1, - containerRuntimeFactory, - containerRuntime: cr1, - } = setupSharedStringRollbackTest(); - const { sharedString: client2, containerRuntime: cr2 } = createAdditionalClient( - containerRuntimeFactory, - "2", - ); - // Baseline text client1.insertText(0, "hello"); cr1.flush(); @@ -250,14 +189,35 @@ describe("SharedString rollback with multiple clients (insert/remove)", () => { }); describe("SharedString replaceText with rollback and two clients", () => { - it("Client1 replaceText + rollback without remote changes", () => { - const { - sharedString: client1, - containerRuntimeFactory, - containerRuntime: cr1, - } = setupSharedStringRollbackTest(); - const { sharedString: client2 } = createAdditionalClient(containerRuntimeFactory, "2"); + let client1: SharedStringClass; + let client2: SharedStringClass; + let cr1: MockContainerRuntime; + let cr2: MockContainerRuntime; + let containerRuntimeFactory: MockContainerRuntimeFactory; + + beforeEach(() => { + // First client + const setup = setupRollbackTest( + "shared-string-1", + (rt, id) => new SharedStringClass(rt, id, SharedStringFactory.Attributes), + { initialize: (dds) => dds.initializeLocal() }, + ); + client1 = setup.dds; + containerRuntimeFactory = setup.containerRuntimeFactory; + cr1 = setup.containerRuntime; + // Second client + const additional = createAdditionalClient( + containerRuntimeFactory, + "2", + (rt, id) => + new SharedStringClass(rt, `shared-string-${id}`, SharedStringFactory.Attributes), + { initialize: (dds) => dds.initializeLocal() }, + ); + client2 = additional.dds; + cr2 = additional.containerRuntime; + }); + it("Client1 replaceText + rollback without remote changes", () => { // Baseline text client1.insertText(0, "hello world"); cr1.flush(); @@ -276,13 +236,6 @@ describe("SharedString replaceText with rollback and two clients", () => { }); it("Client1 multiple replaceText + rollback", () => { - const { - sharedString: client1, - containerRuntimeFactory, - containerRuntime: cr1, - } = setupSharedStringRollbackTest(); - const { sharedString: client2 } = createAdditionalClient(containerRuntimeFactory, "2"); - client1.insertText(0, "hello world"); cr1.flush(); containerRuntimeFactory.processAllMessages(); @@ -300,16 +253,6 @@ describe("SharedString replaceText with rollback and two clients", () => { }); it("Client1 replaceText with concurrent remote remove", () => { - const { - sharedString: client1, - containerRuntimeFactory, - containerRuntime: cr1, - } = setupSharedStringRollbackTest(); - const { sharedString: client2, containerRuntime: cr2 } = createAdditionalClient( - containerRuntimeFactory, - "2", - ); - // Baseline text client1.insertText(0, "abcdef"); cr1.flush(); @@ -338,16 +281,6 @@ describe("SharedString replaceText with rollback and two clients", () => { }); it("replaceText: Rollback on both clients", () => { - const { - sharedString: client1, - containerRuntimeFactory, - containerRuntime: cr1, - } = setupSharedStringRollbackTest(); - const { sharedString: client2, containerRuntime: cr2 } = createAdditionalClient( - containerRuntimeFactory, - "2", - ); - // Baseline text client1.insertText(0, "abcdef"); cr1.flush(); @@ -381,14 +314,34 @@ describe("SharedString replaceText with rollback and two clients", () => { }); describe("SharedString annotate with rollback", () => { - it("can annotate text and rollback without remote changes", () => { - const { - sharedString: client1, + let client1: SharedStringClass; + let client2: SharedStringClass; + let cr1: MockContainerRuntime; + let containerRuntimeFactory: MockContainerRuntimeFactory; + + beforeEach(() => { + // First client + const setup = setupRollbackTest( + "shared-string-1", + (rt, id) => new SharedStringClass(rt, id, SharedStringFactory.Attributes), + { initialize: (dds) => dds.initializeLocal() }, + ); + client1 = setup.dds; + containerRuntimeFactory = setup.containerRuntimeFactory; + cr1 = setup.containerRuntime; + + // Second client + const additional = createAdditionalClient( containerRuntimeFactory, - containerRuntime: cr1, - } = setupSharedStringRollbackTest(); - const { sharedString: client2 } = createAdditionalClient(containerRuntimeFactory, "2"); + "2", + (rt, id) => + new SharedStringClass(rt, `shared-string-${id}`, SharedStringFactory.Attributes), + { initialize: (dds) => dds.initializeLocal() }, + ); + client2 = additional.dds; + }); + it("can annotate text and rollback without remote changes", () => { const text = "hello world"; const styleProps = { style: "bold" }; client1.insertText(0, text, styleProps); @@ -430,13 +383,6 @@ describe("SharedString annotate with rollback", () => { }); it("can handle null annotations with rollback", () => { - const { - sharedString: client1, - containerRuntimeFactory, - containerRuntime: cr1, - } = setupSharedStringRollbackTest(); - const { sharedString: client2 } = createAdditionalClient(containerRuntimeFactory, "2"); - const text = "hello world"; const startingProps = { style: "bold", color: null }; client1.insertText(0, text, startingProps); @@ -469,13 +415,6 @@ describe("SharedString annotate with rollback", () => { }); it("handles multiple annotations with rollback", () => { - const { - sharedString: client1, - containerRuntimeFactory, - containerRuntime: cr1, - } = setupSharedStringRollbackTest(); - const { sharedString: client2 } = createAdditionalClient(containerRuntimeFactory, "2"); - const text = "hello world"; client1.insertText(0, text); cr1.flush(); @@ -519,42 +458,67 @@ describe("SharedString annotate with rollback", () => { }); }); +interface Event { + op: string; + text: string; +} + +function setupDeltaListener(sharedString: SharedString, events: Event[]) { + sharedString.on("sequenceDelta", ({ deltaOperation, isLocal }) => { + if (!isLocal) return; + switch (deltaOperation) { + case MergeTreeDeltaType.INSERT: + events.push({ op: "insert", text: sharedString.getText() }); + break; + case MergeTreeDeltaType.REMOVE: + events.push({ op: "remove", text: sharedString.getText() }); + break; + case MergeTreeDeltaType.ANNOTATE: + events.push({ op: "annotate", text: sharedString.getText() }); + break; + default: + throw new Error(`Unexpected deltaOperation: ${deltaOperation}`); + } + }); +} + describe("SharedString rollback triggers correct sequenceDelta events with text", () => { - interface Event { - op: string; - text: string; - } - - function setupDeltaListener(sharedString: SharedString, events: Event[]) { - sharedString.on("sequenceDelta", ({ deltaOperation, isLocal }) => { - if (!isLocal) return; - switch (deltaOperation) { - case MergeTreeDeltaType.INSERT: - events.push({ op: "insert", text: sharedString.getText() }); - break; - case MergeTreeDeltaType.REMOVE: - events.push({ op: "remove", text: sharedString.getText() }); - break; - case MergeTreeDeltaType.ANNOTATE: - events.push({ op: "annotate", text: sharedString.getText() }); - break; - default: - throw new Error(`Unexpected deltaOperation: ${deltaOperation}`); - } - }); - } + let client1: SharedStringClass; + let client2: SharedStringClass; + let containerRuntimeFactory: MockContainerRuntimeFactory; + let cr1: MockContainerRuntime; + + beforeEach(() => { + // First client + const setup = setupRollbackTest( + "shared-string-1", + (rt, id) => new SharedStringClass(rt, id, SharedStringFactory.Attributes), + { initialize: (dds) => dds.initializeLocal() }, + ); + client1 = setup.dds; + containerRuntimeFactory = setup.containerRuntimeFactory; + cr1 = setup.containerRuntime; + + // Second client + const additional = createAdditionalClient( + containerRuntimeFactory, + "2", + (rt, id) => + new SharedStringClass(rt, `shared-string-${id}`, SharedStringFactory.Attributes), + { initialize: (dds) => dds.initializeLocal() }, + ); + client2 = additional.dds; + }); it("rollback of insert triggers remove", () => { - const { sharedString, containerRuntimeFactory, containerRuntime } = - setupSharedStringRollbackTest(); const events: Event[] = []; - setupDeltaListener(sharedString, events); + setupDeltaListener(client1, events); - sharedString.insertText(0, "hello"); + client1.insertText(0, "hello"); containerRuntimeFactory.processAllMessages(); - assert.equal(sharedString.getText(), "hello"); + assert.equal(client1.getText(), "hello"); - containerRuntime.rollback?.(); + cr1.rollback?.(); assert( events.some((e) => e.op === "remove" && e.text === ""), @@ -563,17 +527,15 @@ describe("SharedString rollback triggers correct sequenceDelta events with text" }); it("rollback of remove triggers insert", () => { - const { sharedString, containerRuntimeFactory, containerRuntime } = - setupSharedStringRollbackTest(); const events: Event[] = []; - setupDeltaListener(sharedString, events); + setupDeltaListener(client1, events); - sharedString.insertText(0, "world"); + client1.insertText(0, "world"); containerRuntimeFactory.processAllMessages(); - sharedString.removeText(0, 5); - assert.equal(sharedString.getText(), ""); + client1.removeText(0, 5); + assert.equal(client1.getText(), ""); - containerRuntime.rollback?.(); + cr1.rollback?.(); assert( events.some((e) => e.op === "insert" && e.text === "world"), @@ -582,25 +544,23 @@ describe("SharedString rollback triggers correct sequenceDelta events with text" }); it("rollback of annotate clears properties", () => { - const { sharedString, containerRuntimeFactory, containerRuntime } = - setupSharedStringRollbackTest(); const events: Event[] = []; - setupDeltaListener(sharedString, events); + setupDeltaListener(client1, events); - sharedString.insertText(0, "abc"); + client1.insertText(0, "abc"); containerRuntimeFactory.processAllMessages(); const styleProps = { style: "bold" }; - sharedString.annotateRange(0, 3, styleProps); + client1.annotateRange(0, 3, styleProps); for (let i = 0; i < 3; i++) { - assert.deepEqual({ ...sharedString.getPropertiesAtPosition(i) }, styleProps); + assert.deepEqual({ ...client1.getPropertiesAtPosition(i) }, styleProps); } - containerRuntime.rollback?.(); + cr1.rollback?.(); for (let i = 0; i < 3; i++) { assert.deepEqual( - { ...sharedString.getPropertiesAtPosition(i) }, + { ...client1.getPropertiesAtPosition(i) }, {}, "Rollback of annotate should clear properties", ); @@ -613,13 +573,6 @@ describe("SharedString rollback triggers correct sequenceDelta events with text" }); it("multi-client: rollback of insert triggers remove", () => { - const { - sharedString: client1, - containerRuntimeFactory, - containerRuntime: cr1, - } = setupSharedStringRollbackTest(); - const { sharedString: client2 } = createAdditionalClient(containerRuntimeFactory, "2"); - const eventsClient1: Event[] = []; setupDeltaListener(client1, eventsClient1); @@ -641,13 +594,6 @@ describe("SharedString rollback triggers correct sequenceDelta events with text" }); it("multi-client: rollback of remove triggers insert", () => { - const { - sharedString: client1, - containerRuntimeFactory, - containerRuntime: cr1, - } = setupSharedStringRollbackTest(); - const { sharedString: client2 } = createAdditionalClient(containerRuntimeFactory, "2"); - const eventsClient1: Event[] = []; setupDeltaListener(client1, eventsClient1); @@ -670,13 +616,6 @@ describe("SharedString rollback triggers correct sequenceDelta events with text" }); it("multi-client: rollback of annotate clears properties", () => { - const { - sharedString: client1, - containerRuntimeFactory, - containerRuntime: cr1, - } = setupSharedStringRollbackTest(); - createAdditionalClient(containerRuntimeFactory, "2"); - const eventsClient1: Event[] = []; setupDeltaListener(client1, eventsClient1); From 6998f8ccb27180e5bdd8956e24ca09effdc507fb Mon Sep 17 00:00:00 2001 From: Sonali Deshpande <48232592+sonalideshpandemsft@users.noreply.github.com> Date: Tue, 26 Aug 2025 15:23:43 +0000 Subject: [PATCH 09/11] feedback --- .../src/test/mocha/directory.rollback.spec.ts | 535 +++++++----------- .../src/test/sharedString.rollback.spec.ts | 193 +++++-- 2 files changed, 337 insertions(+), 391 deletions(-) diff --git a/packages/dds/map/src/test/mocha/directory.rollback.spec.ts b/packages/dds/map/src/test/mocha/directory.rollback.spec.ts index 7929ae7cc59f..7887fe4a37a5 100644 --- a/packages/dds/map/src/test/mocha/directory.rollback.spec.ts +++ b/packages/dds/map/src/test/mocha/directory.rollback.spec.ts @@ -6,46 +6,23 @@ import { strict as assert } from "node:assert"; import { setupRollbackTest, createAdditionalClient } from "@fluid-private/test-dds-utils"; -import type { - MockContainerRuntimeFactory, - MockContainerRuntime, -} from "@fluidframework/test-runtime-utils/internal"; import { DirectoryFactory } from "../../directoryFactory.js"; import type { ISharedDirectory, IValueChanged } from "../../interfaces.js"; const directoryFactory = new DirectoryFactory(); -interface Event { - event: string; - path: string; - local: boolean; - target: ISharedDirectory; -} - describe("SharedDirectory rollback", () => { describe("Storage operations (root subdirectory)", () => { - let sharedDirectory: ISharedDirectory; - let containerRuntime: MockContainerRuntime; - let containerRuntimeFactory: MockContainerRuntimeFactory; - let sharedDirectory2: ISharedDirectory; - let containerRuntime2: MockContainerRuntime; - - beforeEach(() => { - ({ - dds: sharedDirectory, - containerRuntimeFactory, - containerRuntime, - } = setupRollbackTest( - "client-1", - (rt, id): ISharedDirectory => directoryFactory.create(rt, id), - )); - }); - it("should rollback set operation", () => { + const { dds: sharedDirectory, containerRuntime } = setupRollbackTest( + "shared-map", + (rt, id): ISharedDirectory => directoryFactory.create(rt, id), + ); const valueChanges: IValueChanged[] = []; - sharedDirectory.on("valueChanged", (event: IValueChanged) => valueChanges.push(event)); - + sharedDirectory.on("valueChanged", (event: IValueChanged) => { + valueChanges.push(event); + }); sharedDirectory.set("key1", "value1"); assert.strictEqual( sharedDirectory.get("key1"), @@ -53,76 +30,142 @@ describe("SharedDirectory rollback", () => { "Failed getting pending value", ); assert.strictEqual(valueChanges.length, 1, "Should have one value change event"); - containerRuntime.rollback?.(); - assert.strictEqual( sharedDirectory.get("key1"), undefined, "Value should be rolled back", ); assert.strictEqual(valueChanges.length, 2, "Should have two value change events"); - assert.strictEqual(valueChanges[1].key, "key1"); - assert.strictEqual(valueChanges[1].previousValue, "value1"); + assert.strictEqual(valueChanges[1].key, "key1", "Second event should be for key1"); + assert.strictEqual( + valueChanges[1].previousValue, + "value1", + "Second event previousValue should be pre-rollback value", + ); }); it("should rollback delete operation", () => { + const { + dds: sharedDirectory, + containerRuntime, + containerRuntimeFactory, + } = setupRollbackTest( + "shared-map", + (rt, id): ISharedDirectory => directoryFactory.create(rt, id), + ); sharedDirectory.set("key1", "value1"); containerRuntime.flush(); containerRuntimeFactory.processAllMessages(); - const valueChanges: IValueChanged[] = []; - sharedDirectory.on("valueChanged", (event: IValueChanged) => valueChanges.push(event)); - + sharedDirectory.on("valueChanged", (event: IValueChanged) => { + valueChanges.push(event); + }); sharedDirectory.delete("key1"); - assert.strictEqual(sharedDirectory.get("key1"), undefined); - assert.strictEqual(valueChanges.length, 1); - + assert.strictEqual( + sharedDirectory.get("key1"), + undefined, + "Pending value should reflect the delete", + ); + assert.strictEqual(valueChanges.length, 1, "Should have one value change event"); containerRuntime.rollback?.(); - - assert.strictEqual(sharedDirectory.get("key1"), "value1"); - assert.strictEqual(valueChanges.length, 2); - assert.strictEqual(valueChanges[1].key, "key1"); - assert.strictEqual(valueChanges[1].previousValue, undefined); + assert.strictEqual( + sharedDirectory.get("key1"), + "value1", + "Value should be restored by rollback", + ); + assert.strictEqual(valueChanges.length, 2, "Should have two value change events"); + assert.strictEqual(valueChanges[1].key, "key1", "Second event should be for key1"); + assert.strictEqual( + valueChanges[1].previousValue, + undefined, + "Second event previousValue should be pre-rollback value", + ); }); it("should rollback clear operation", () => { + const { + dds: sharedDirectory, + containerRuntime, + containerRuntimeFactory, + } = setupRollbackTest( + "shared-map", + (rt, id): ISharedDirectory => directoryFactory.create(rt, id), + ); sharedDirectory.set("key1", "value1"); sharedDirectory.set("key2", "value2"); containerRuntime.flush(); containerRuntimeFactory.processAllMessages(); - const valueChanges: IValueChanged[] = []; - let clears = 0; - sharedDirectory.on("valueChanged", (e) => valueChanges.push(e)); - sharedDirectory.on("clear", () => clears++); + let clears: number = 0; + sharedDirectory.on("valueChanged", (event: IValueChanged) => { + valueChanges.push(event); + }); + sharedDirectory.on("clear", () => { + clears++; + }); sharedDirectory.clear(); - assert.strictEqual(sharedDirectory.get("key1"), undefined); - assert.strictEqual(sharedDirectory.get("key2"), undefined); - assert.strictEqual(valueChanges.length, 0); - assert.strictEqual(clears, 1); - + assert.strictEqual( + sharedDirectory.get("key1"), + undefined, + "Pending value for key1 should reflect the clear", + ); + assert.strictEqual( + sharedDirectory.get("key2"), + undefined, + "Pending value for key2 should reflect the clear", + ); + assert.strictEqual(valueChanges.length, 0, "Should have no value change events"); + assert.strictEqual(clears, 1, "Should have one clear event"); containerRuntime.rollback?.(); - - assert.strictEqual(sharedDirectory.get("key1"), "value1"); - assert.strictEqual(sharedDirectory.get("key2"), "value2"); - assert.strictEqual(valueChanges.length, 2); - assert.strictEqual(valueChanges[0].key, "key1"); - assert.strictEqual(valueChanges[1].key, "key2"); + assert.strictEqual( + sharedDirectory.get("key1"), + "value1", + "Value should be restored by rollback", + ); + assert.strictEqual( + sharedDirectory.get("key2"), + "value2", + "Value should be restored by rollback", + ); + assert.strictEqual(valueChanges.length, 2, "Should have two value change events"); + assert.strictEqual(valueChanges[0].key, "key1", "First event should be for key1"); + assert.strictEqual( + valueChanges[0].previousValue, + undefined, + "First event previousValue should be pre-rollback value", + ); + assert.strictEqual(valueChanges[1].key, "key2", "Second event should be for key2"); + assert.strictEqual( + valueChanges[1].previousValue, + undefined, + "Second event previousValue should be pre-rollback value", + ); }); it("should rollback multiple operations in sequence", () => { + const { + dds: sharedDirectory, + containerRuntime, + containerRuntimeFactory, + } = setupRollbackTest( + "shared-map", + (rt, id): ISharedDirectory => directoryFactory.create(rt, id), + ); sharedDirectory.set("key1", "value1"); sharedDirectory.set("key2", "value2"); containerRuntime.flush(); containerRuntimeFactory.processAllMessages(); - let valueChanges: IValueChanged[] = []; - let clears = 0; - sharedDirectory.on("valueChanged", (e) => valueChanges.push(e)); - sharedDirectory.on("clear", () => clears++); + let clears: number = 0; + sharedDirectory.on("valueChanged", (event: IValueChanged) => { + valueChanges.push(event); + }); + sharedDirectory.on("clear", () => { + clears++; + }); sharedDirectory.set("key3", "value3"); sharedDirectory.delete("key1"); @@ -130,10 +173,29 @@ describe("SharedDirectory rollback", () => { sharedDirectory.clear(); sharedDirectory.set("key4", "value4"); - assert.deepStrictEqual([...sharedDirectory.entries()], [["key4", "value4"]]); - assert.strictEqual(clears, 1); + assert.deepStrictEqual( + [...sharedDirectory.entries()], + [["key4", "value4"]], + "Directory should have expected entries pre-rollback", + ); + assert.deepStrictEqual( + valueChanges, + [ + // Set key3 + { key: "key3", path: "/", previousValue: undefined }, + // Delete key1 + { key: "key1", path: "/", previousValue: "value1" }, + // Set key2 to a new value + { key: "key2", path: "/", previousValue: "value2" }, + // Clear happens here, no valueChange event for clear + // Set key4 + { key: "key4", path: "/", previousValue: undefined }, + ], + "Value changes should match expected pre-rollback", + ); + assert.strictEqual(clears, 1, "Should have one clear event"); - // Reset events and rollback + // Reset event monitoring and roll back. valueChanges = []; clears = 0; containerRuntime.rollback?.(); @@ -144,16 +206,45 @@ describe("SharedDirectory rollback", () => { ["key1", "value1"], ["key2", "value2"], ], + "Directory should have expected entries post-rollback", ); - assert.strictEqual(clears, 0); + assert.deepStrictEqual( + valueChanges, + [ + // Roll back the final key4 set + { key: "key4", path: "/", previousValue: "value4" }, + // Roll back the clear + { key: "key2", path: "/", previousValue: undefined }, + { key: "key3", path: "/", previousValue: undefined }, + // Roll back the key2 set + { key: "key2", path: "/", previousValue: "newValue2" }, + // Roll back the key1 delete + { key: "key1", path: "/", previousValue: undefined }, + // Roll back the key3 set + { key: "key3", path: "/", previousValue: "value3" }, + ], + "Value changes should match expected post-rollback", + ); + assert.strictEqual(clears, 0, "Should have no clear events"); }); it("should rollback local changes in presence of remote changes from another client", () => { - ({ dds: sharedDirectory2, containerRuntime: containerRuntime2 } = createAdditionalClient( + const { + dds: sharedDirectory, + containerRuntime, containerRuntimeFactory, - "client-2", - (rt, id): ISharedDirectory => directoryFactory.create(rt, `cell-${id}`), - )); + } = setupRollbackTest( + "shared-map", + (rt, id): ISharedDirectory => directoryFactory.create(rt, id), + ); + // Create a second client + + const { dds: sharedDirectory2, containerRuntime: containerRuntime2 } = + createAdditionalClient( + containerRuntimeFactory, + "client-2", + (rt, id): ISharedDirectory => directoryFactory.create(rt, `directory-${id}`), + ); sharedDirectory.set("key1", "value1"); sharedDirectory.set("key2", "value2"); @@ -173,9 +264,12 @@ describe("SharedDirectory rollback", () => { assert.deepStrictEqual( [...sharedDirectory.entries()], [ + // Note key4 comes before key3 even though we have an optimistic value for it, + // because sharedDirectory2 set them in that order. Iteration order matches the sequenced perspective. ["key4", "value4"], ["key3", "value3"], ], + "Directory should have expected entries pre-rollback", ); containerRuntime.rollback?.(); @@ -187,28 +281,22 @@ describe("SharedDirectory rollback", () => { ["key4", "value4"], ["key3", "otherValue3"], ], + "Directory should have expected entries post-rollback", ); }); }); describe("Storage operations (nested subdirectories)", () => { - let sharedDirectory: ISharedDirectory; - let containerRuntime: MockContainerRuntime; - let containerRuntimeFactory: MockContainerRuntimeFactory; - let sharedDirectory2: ISharedDirectory; - let containerRuntime2: MockContainerRuntime; - - beforeEach(() => { - ({ + it("should rollback all basic operations (set, delete, clear) in subdirectories and nested subdirectories", () => { + const { dds: sharedDirectory, - containerRuntimeFactory, containerRuntime, + containerRuntimeFactory, } = setupRollbackTest( - "client-1", + "shared-map", (rt, id): ISharedDirectory => directoryFactory.create(rt, id), - )); - }); - it("should rollback all basic operations (set, delete, clear) in subdirectories and nested subdirectories", () => { + ); + const subDir = sharedDirectory.createSubDirectory("subdir"); const level1 = sharedDirectory.createSubDirectory("level1"); const level3 = level1.createSubDirectory("level2").createSubDirectory("level3"); @@ -282,11 +370,21 @@ describe("SharedDirectory rollback", () => { }); it("should rollback subdirectory operations with concurrent remote changes", () => { - ({ dds: sharedDirectory2, containerRuntime: containerRuntime2 } = createAdditionalClient( + const { + dds: sharedDirectory, + containerRuntime, containerRuntimeFactory, - "client-2", - (rt, id): ISharedDirectory => directoryFactory.create(rt, `cell-${id}`), - )); + } = setupRollbackTest( + "shared-map", + (rt, id): ISharedDirectory => directoryFactory.create(rt, id), + ); + + const { dds: sharedDirectory2, containerRuntime: containerRuntime2 } = + createAdditionalClient( + containerRuntimeFactory, + "client-2", + (rt, id): ISharedDirectory => directoryFactory.create(rt, `directory-${id}`), + ); const subDir1 = sharedDirectory.createSubDirectory("shared"); const nestedDir1 = subDir1.createSubDirectory("nested"); @@ -343,6 +441,15 @@ describe("SharedDirectory rollback", () => { }); it("should rollback complex mixed operations across multiple subdirectory levels", () => { + const { + dds: sharedDirectory, + containerRuntime, + containerRuntimeFactory, + } = setupRollbackTest( + "shared-map", + (rt, id): ISharedDirectory => directoryFactory.create(rt, id), + ); + const dirA = sharedDirectory.createSubDirectory("dirA"); const dirB = sharedDirectory.createSubDirectory("dirB"); const nestedDir = dirA.createSubDirectory("nested"); @@ -420,253 +527,3 @@ describe("SharedDirectory rollback", () => { }); }); }); - -describe("SharedDirectory rollback events", () => { - let sharedDirectory: ISharedDirectory; - let containerRuntime: MockContainerRuntime; - let containerRuntimeFactory: MockContainerRuntimeFactory; - - beforeEach(() => { - ({ - dds: sharedDirectory, - containerRuntime, - containerRuntimeFactory, - } = setupRollbackTest( - "shared-map", - (rt, id): ISharedDirectory => directoryFactory.create(rt, id), - )); - }); - - it("should trigger subDirectoryDeleted event on rollback of subDirectoryCreated", () => { - const events: Event[] = []; - - sharedDirectory.on("subDirectoryDeleted", (path, local, target) => { - events.push({ event: "subDirectoryDeleted", path, local, target }); - }); - - // Create a subdirectory - sharedDirectory.createSubDirectory("subDir1"); - assert( - sharedDirectory.getWorkingDirectory("subDir1") !== undefined, - "subdirectory should exist after creation", - ); - - // Rollback - containerRuntime.rollback?.(); - - // The subdirectory should be removed - assert.strictEqual( - sharedDirectory.getWorkingDirectory("subDir1"), - undefined, - "subdirectory should be removed after rollback", - ); - - // The subDirectoryDeleted event should be triggered - assert.strictEqual( - events.length, - 1, - "one subDirectoryDeleted event should have been emitted", - ); - assert.strictEqual(events[0].path, "subDir1"); - assert.strictEqual(events[0].local, true); - assert.strictEqual(events[0].target, sharedDirectory); - }); - - it("should trigger subDirectoryCreated event on rollback of subDirectoryDeleted", () => { - const events: Event[] = []; - - sharedDirectory.createSubDirectory("subDir2"); - - containerRuntime.flush(); - containerRuntimeFactory.processAllMessages(); - - sharedDirectory.on("subDirectoryCreated", (path, local, target) => { - events.push({ event: "subDirectoryCreated", path, local, target }); - }); - - // Delete the subdirectory - sharedDirectory.deleteSubDirectory("subDir2"); - assert.strictEqual( - sharedDirectory.getWorkingDirectory("subDir2"), - undefined, - "subdirectory should be deleted", - ); - - // Rollback - containerRuntime.rollback?.(); - - // The subdirectory should be restored - assert( - sharedDirectory.getWorkingDirectory("subDir2") !== undefined, - "subdirectory should exist after rollback", - ); - - // The subDirectoryCreated event should be triggered - assert.strictEqual( - events.length, - 1, - "one subDirectoryCreated event should have been emitted", - ); - assert.strictEqual(events[0].path, "subDir2"); - assert.strictEqual(events[0].local, true); - assert.strictEqual(events[0].target, sharedDirectory); - }); -}); - -describe("SharedDirectory rollback with multiple clients", () => { - let client1: ISharedDirectory; - let client2: ISharedDirectory; - let containerRuntime1: MockContainerRuntime; - let containerRuntime2: MockContainerRuntime; - let containerRuntimeFactory: MockContainerRuntimeFactory; - - beforeEach(() => { - ({ - dds: client1, - containerRuntime: containerRuntime1, - containerRuntimeFactory, - } = setupRollbackTest( - "client-1", - (rt, id): ISharedDirectory => directoryFactory.create(rt, id), - )); - - ({ dds: client2, containerRuntime: containerRuntime2 } = createAdditionalClient( - containerRuntimeFactory, - "client-2", - (rt, id): ISharedDirectory => directoryFactory.create(rt, `directory-${id}`), - )); - }); - - it("should only rollback local subDirectoryCreated changes for one client", () => { - const eventsClient1: Event[] = []; - const eventsClient2: Event[] = []; - - client1.on("subDirectoryDeleted", (path, local, target) => - eventsClient1.push({ event: "subDirectoryDeleted", path, local, target }), - ); - client2.on("subDirectoryDeleted", (path, local, target) => - eventsClient2.push({ event: "subDirectoryDeleted", path, local, target }), - ); - - // Client1 creates a subdirectory - client1.createSubDirectory("sharedDir"); - - // Client2 creates a subdirectory - client2.createSubDirectory("sharedDir2"); - - containerRuntime1.flush(); - containerRuntime2.flush(); - containerRuntimeFactory.processAllMessages(); - - // Check that directories exist - assert(client2.getWorkingDirectory("/sharedDir") !== undefined); - assert(client1.getWorkingDirectory("/sharedDir2") !== undefined); - - client1.createSubDirectory("foo"); - - // Rollback only affects client1's local changes - containerRuntime1.rollback?.(); - - // Client1's local subdirectory should be gone - assert.strictEqual(client1.getSubDirectory("foo"), undefined); - - // Client2's subdirectory remains - assert(client1.getWorkingDirectory("/sharedDir2") !== undefined); - - // Events triggered - assert.strictEqual(eventsClient1.length, 1); - assert.strictEqual(eventsClient1[0].path, "foo"); - assert(eventsClient1[0].local); - - // Client2 did not get any subDirectoryDeleted events for client1's rollback - assert.strictEqual(eventsClient2.length, 0); - }); - - it("should rollback local subDirectoryDeleted changes for one client", () => { - const eventsClient1: Event[] = []; - const eventsClient2: Event[] = []; - - client1.on("subDirectoryCreated", (path, local, target) => - eventsClient1.push({ event: "subDirectoryCreated", path, local, target }), - ); - client2.on("subDirectoryCreated", (path, local, target) => - eventsClient2.push({ event: "subDirectoryCreated", path, local, target }), - ); - - client1.on("subDirectoryDeleted", (path, local, target) => - eventsClient1.push({ event: "subDirectoryDeleted", path, local, target }), - ); - client2.on("subDirectoryDeleted", (path, local, target) => - eventsClient2.push({ event: "subDirectoryDeleted", path, local, target }), - ); - - // Create initial directories - client1.createSubDirectory("dir1"); - client2.createSubDirectory("dir2"); - - containerRuntime1.flush(); - containerRuntimeFactory.processAllMessages(); - - // Client1 deletes its own directory - client1.deleteSubDirectory("dir1"); - - // Client2 deletes its own directory - client2.deleteSubDirectory("dir2"); - - // Rollback client1 - containerRuntime1.rollback?.(); - - // Client1's directory should be restored - assert(client1.getWorkingDirectory("/dir1") !== undefined); - - // Client2's directory remains deleted - assert.strictEqual(client1.getWorkingDirectory("/dir2"), undefined); - - assert.strictEqual(eventsClient1.length, 3); - assert.strictEqual(eventsClient1[0].path, "dir1"); - assert(eventsClient1[0].local); - - assert.strictEqual(eventsClient2.length, 3); - }); - - it("should handle simultaneous directory deletions and rollback for one client", () => { - const eventsClient1: Event[] = []; - const eventsClient2: Event[] = []; - - client1.on("subDirectoryCreated", (path, local, target) => - eventsClient1.push({ event: "subDirectoryCreated", path, local, target }), - ); - client2.on("subDirectoryCreated", (path, local, target) => - eventsClient2.push({ event: "subDirectoryCreated", path, local, target }), - ); - - client2.on("subDirectoryDeleted", (path, local, target) => { - eventsClient2.push({ event: "subDirectoryDeleted", path, local, target }); - }); - - // Initial directories created independently - client1.createSubDirectory("dir1").createSubDirectory("foo"); - client2.createSubDirectory("dir2"); - - containerRuntime1.flush(); - containerRuntime2.flush(); - containerRuntimeFactory.processAllMessages(); - - client2.deleteSubDirectory("dir1"); - - // Rollback client1 deletion - containerRuntime2.rollback?.(); - - assert.strictEqual(eventsClient1.length, 3); - assert.strictEqual(eventsClient1[0].path, "dir1"); - assert(eventsClient1[0].local); - assert.strictEqual(eventsClient1[1].path, "dir1/foo"); - assert(eventsClient1[1].local); - - assert.strictEqual(eventsClient2.length, 5); - assert.strictEqual(eventsClient2[0].path, "dir2"); - assert.strictEqual(eventsClient2[0].event, "subDirectoryCreated"); - assert.strictEqual(eventsClient2[3].path, "dir1"); - assert.strictEqual(eventsClient2[3].event, "subDirectoryDeleted"); - }); -}); diff --git a/packages/dds/sequence/src/test/sharedString.rollback.spec.ts b/packages/dds/sequence/src/test/sharedString.rollback.spec.ts index 965656e18410..99367e2c9ac8 100644 --- a/packages/dds/sequence/src/test/sharedString.rollback.spec.ts +++ b/packages/dds/sequence/src/test/sharedString.rollback.spec.ts @@ -16,36 +16,21 @@ import { SharedStringFactory, type SharedString } from "../sequenceFactory.js"; import { SharedStringClass } from "../sharedString.js"; describe("SharedString rollback with multiple clients (insert/remove)", () => { - let client1: SharedStringClass; - let client2: SharedStringClass; - let cr1: MockContainerRuntime; - let cr2: MockContainerRuntime; - let containerRuntimeFactory: MockContainerRuntimeFactory; - beforeEach(() => { - // First client - const setup = setupRollbackTest( + it("Client1 insert + Client2 insert + rollback on Client1", () => { + const { dds: client1, containerRuntimeFactory, containerRuntime: cr1 } = setupRollbackTest( "shared-string-1", (rt, id) => new SharedStringClass(rt, id, SharedStringFactory.Attributes), { initialize: (dds) => dds.initializeLocal() }, ); - client1 = setup.dds; - containerRuntimeFactory = setup.containerRuntimeFactory; - cr1 = setup.containerRuntime; - // Second client - const additional = createAdditionalClient( + const { dds: client2, containerRuntime: cr2 } = createAdditionalClient( containerRuntimeFactory, "2", (rt, id) => new SharedStringClass(rt, `shared-string-${id}`, SharedStringFactory.Attributes), { initialize: (dds) => dds.initializeLocal() }, ); - client2 = additional.dds; - cr2 = additional.containerRuntime; - }); - - it("Client1 insert + Client2 insert + rollback on Client1", () => { // Baseline text client1.insertText(0, "hello"); cr1.flush(); @@ -86,6 +71,19 @@ describe("SharedString rollback with multiple clients (insert/remove)", () => { }); it("Client1 remove + Client2 insert + rollback on Client1", () => { + const { dds: client1, containerRuntimeFactory, containerRuntime: cr1 } = setupRollbackTest( + "shared-string-1", + (rt, id) => new SharedStringClass(rt, id, SharedStringFactory.Attributes), + { initialize: (dds) => dds.initializeLocal() }, + ); + + const { dds: client2, containerRuntime: cr2 } = createAdditionalClient( + containerRuntimeFactory, + "2", + (rt, id) => + new SharedStringClass(rt, `shared-string-${id}`, SharedStringFactory.Attributes), + { initialize: (dds) => dds.initializeLocal() }, + ); // Baseline text client1.insertText(0, "abcdef"); cr1.flush(); @@ -119,6 +117,19 @@ describe("SharedString rollback with multiple clients (insert/remove)", () => { }); it("Client1 insert + Client2 remove + rollback on Client1", () => { + const { dds: client1, containerRuntimeFactory, containerRuntime: cr1 } = setupRollbackTest( + "shared-string-1", + (rt, id) => new SharedStringClass(rt, id, SharedStringFactory.Attributes), + { initialize: (dds) => dds.initializeLocal() }, + ); + + const { dds: client2, containerRuntime: cr2 } = createAdditionalClient( + containerRuntimeFactory, + "2", + (rt, id) => + new SharedStringClass(rt, `shared-string-${id}`, SharedStringFactory.Attributes), + { initialize: (dds) => dds.initializeLocal() }, + ); // Baseline text client1.insertText(0, "123456"); cr1.flush(); @@ -148,6 +159,19 @@ describe("SharedString rollback with multiple clients (insert/remove)", () => { }); it("Client1 insert + Client2 insert + rollback on Client2", () => { + const { dds: client1, containerRuntimeFactory, containerRuntime: cr1 } = setupRollbackTest( + "shared-string-1", + (rt, id) => new SharedStringClass(rt, id, SharedStringFactory.Attributes), + { initialize: (dds) => dds.initializeLocal() }, + ); + + const { dds: client2, containerRuntime: cr2 } = createAdditionalClient( + containerRuntimeFactory, + "2", + (rt, id) => + new SharedStringClass(rt, `shared-string-${id}`, SharedStringFactory.Attributes), + { initialize: (dds) => dds.initializeLocal() }, + ); // Baseline text client1.insertText(0, "hello"); cr1.flush(); @@ -189,35 +213,48 @@ describe("SharedString rollback with multiple clients (insert/remove)", () => { }); describe("SharedString replaceText with rollback and two clients", () => { - let client1: SharedStringClass; - let client2: SharedStringClass; - let cr1: MockContainerRuntime; - let cr2: MockContainerRuntime; - let containerRuntimeFactory: MockContainerRuntimeFactory; - - beforeEach(() => { - // First client - const setup = setupRollbackTest( + // let client1: SharedStringClass; + // let client2: SharedStringClass; + // let cr1: MockContainerRuntime; + // let cr2: MockContainerRuntime; + // let containerRuntimeFactory: MockContainerRuntimeFactory; + + // beforeEach(() => { + // // First client + // const setup = setupRollbackTest( + // "shared-string-1", + // (rt, id) => new SharedStringClass(rt, id, SharedStringFactory.Attributes), + // { initialize: (dds) => dds.initializeLocal() }, + // ); + // client1 = setup.dds; + // containerRuntimeFactory = setup.containerRuntimeFactory; + // cr1 = setup.containerRuntime; + + // // Second client + // const additional = createAdditionalClient( + // containerRuntimeFactory, + // "2", + // (rt, id) => + // new SharedStringClass(rt, `shared-string-${id}`, SharedStringFactory.Attributes), + // { initialize: (dds) => dds.initializeLocal() }, + // ); + // client2 = additional.dds; + // cr2 = additional.containerRuntime; + // }); + it("Client1 replaceText + rollback without remote changes", () => { + const { dds: client1, containerRuntimeFactory, containerRuntime: cr1 } = setupRollbackTest( "shared-string-1", (rt, id) => new SharedStringClass(rt, id, SharedStringFactory.Attributes), { initialize: (dds) => dds.initializeLocal() }, ); - client1 = setup.dds; - containerRuntimeFactory = setup.containerRuntimeFactory; - cr1 = setup.containerRuntime; - // Second client - const additional = createAdditionalClient( + const { dds: client2, containerRuntime: cr2 } = createAdditionalClient( containerRuntimeFactory, "2", (rt, id) => new SharedStringClass(rt, `shared-string-${id}`, SharedStringFactory.Attributes), { initialize: (dds) => dds.initializeLocal() }, ); - client2 = additional.dds; - cr2 = additional.containerRuntime; - }); - it("Client1 replaceText + rollback without remote changes", () => { // Baseline text client1.insertText(0, "hello world"); cr1.flush(); @@ -236,6 +273,19 @@ describe("SharedString replaceText with rollback and two clients", () => { }); it("Client1 multiple replaceText + rollback", () => { + const { dds: client1, containerRuntimeFactory, containerRuntime: cr1 } = setupRollbackTest( + "shared-string-1", + (rt, id) => new SharedStringClass(rt, id, SharedStringFactory.Attributes), + { initialize: (dds) => dds.initializeLocal() }, + ); + + const { dds: client2, containerRuntime: cr2 } = createAdditionalClient( + containerRuntimeFactory, + "2", + (rt, id) => + new SharedStringClass(rt, `shared-string-${id}`, SharedStringFactory.Attributes), + { initialize: (dds) => dds.initializeLocal() }, + ); client1.insertText(0, "hello world"); cr1.flush(); containerRuntimeFactory.processAllMessages(); @@ -253,6 +303,19 @@ describe("SharedString replaceText with rollback and two clients", () => { }); it("Client1 replaceText with concurrent remote remove", () => { + const { dds: client1, containerRuntimeFactory, containerRuntime: cr1 } = setupRollbackTest( + "shared-string-1", + (rt, id) => new SharedStringClass(rt, id, SharedStringFactory.Attributes), + { initialize: (dds) => dds.initializeLocal() }, + ); + + const { dds: client2, containerRuntime: cr2 } = createAdditionalClient( + containerRuntimeFactory, + "2", + (rt, id) => + new SharedStringClass(rt, `shared-string-${id}`, SharedStringFactory.Attributes), + { initialize: (dds) => dds.initializeLocal() }, + ); // Baseline text client1.insertText(0, "abcdef"); cr1.flush(); @@ -281,6 +344,19 @@ describe("SharedString replaceText with rollback and two clients", () => { }); it("replaceText: Rollback on both clients", () => { + const { dds: client1, containerRuntimeFactory, containerRuntime: cr1 } = setupRollbackTest( + "shared-string-1", + (rt, id) => new SharedStringClass(rt, id, SharedStringFactory.Attributes), + { initialize: (dds) => dds.initializeLocal() }, + ); + + const { dds: client2, containerRuntime: cr2 } = createAdditionalClient( + containerRuntimeFactory, + "2", + (rt, id) => + new SharedStringClass(rt, `shared-string-${id}`, SharedStringFactory.Attributes), + { initialize: (dds) => dds.initializeLocal() }, + ); // Baseline text client1.insertText(0, "abcdef"); cr1.flush(); @@ -314,34 +390,21 @@ describe("SharedString replaceText with rollback and two clients", () => { }); describe("SharedString annotate with rollback", () => { - let client1: SharedStringClass; - let client2: SharedStringClass; - let cr1: MockContainerRuntime; - let containerRuntimeFactory: MockContainerRuntimeFactory; - beforeEach(() => { - // First client - const setup = setupRollbackTest( + it("can annotate text and rollback without remote changes", () => { + const { dds: client1, containerRuntimeFactory, containerRuntime: cr1 } = setupRollbackTest( "shared-string-1", (rt, id) => new SharedStringClass(rt, id, SharedStringFactory.Attributes), { initialize: (dds) => dds.initializeLocal() }, ); - client1 = setup.dds; - containerRuntimeFactory = setup.containerRuntimeFactory; - cr1 = setup.containerRuntime; - // Second client - const additional = createAdditionalClient( + const { dds: client2 } = createAdditionalClient( containerRuntimeFactory, "2", (rt, id) => new SharedStringClass(rt, `shared-string-${id}`, SharedStringFactory.Attributes), { initialize: (dds) => dds.initializeLocal() }, ); - client2 = additional.dds; - }); - - it("can annotate text and rollback without remote changes", () => { const text = "hello world"; const styleProps = { style: "bold" }; client1.insertText(0, text, styleProps); @@ -383,6 +446,19 @@ describe("SharedString annotate with rollback", () => { }); it("can handle null annotations with rollback", () => { + const { dds: client1, containerRuntimeFactory, containerRuntime: cr1 } = setupRollbackTest( + "shared-string-1", + (rt, id) => new SharedStringClass(rt, id, SharedStringFactory.Attributes), + { initialize: (dds) => dds.initializeLocal() }, + ); + + const { dds: client2, containerRuntime: cr2 } = createAdditionalClient( + containerRuntimeFactory, + "2", + (rt, id) => + new SharedStringClass(rt, `shared-string-${id}`, SharedStringFactory.Attributes), + { initialize: (dds) => dds.initializeLocal() }, + ); const text = "hello world"; const startingProps = { style: "bold", color: null }; client1.insertText(0, text, startingProps); @@ -415,6 +491,19 @@ describe("SharedString annotate with rollback", () => { }); it("handles multiple annotations with rollback", () => { + const { dds: client1, containerRuntimeFactory, containerRuntime: cr1 } = setupRollbackTest( + "shared-string-1", + (rt, id) => new SharedStringClass(rt, id, SharedStringFactory.Attributes), + { initialize: (dds) => dds.initializeLocal() }, + ); + + const { dds: client2, containerRuntime: cr2 } = createAdditionalClient( + containerRuntimeFactory, + "2", + (rt, id) => + new SharedStringClass(rt, `shared-string-${id}`, SharedStringFactory.Attributes), + { initialize: (dds) => dds.initializeLocal() }, + ); const text = "hello world"; client1.insertText(0, text); cr1.flush(); From 3d58f53606a4895375ed1e41592ec7ca00868d55 Mon Sep 17 00:00:00 2001 From: Sonali Deshpande <48232592+sonalideshpandemsft@users.noreply.github.com> Date: Tue, 26 Aug 2025 23:46:31 +0000 Subject: [PATCH 10/11] intervalcollection: wip --- .../intervalCollection.event.rollback.spec.ts | 400 ++++++++++++++++++ .../src/test/sharedString.rollback.spec.ts | 104 +++-- 2 files changed, 459 insertions(+), 45 deletions(-) create mode 100644 packages/dds/sequence/src/test/intervalCollection.event.rollback.spec.ts diff --git a/packages/dds/sequence/src/test/intervalCollection.event.rollback.spec.ts b/packages/dds/sequence/src/test/intervalCollection.event.rollback.spec.ts new file mode 100644 index 000000000000..ae18a1b9cadd --- /dev/null +++ b/packages/dds/sequence/src/test/intervalCollection.event.rollback.spec.ts @@ -0,0 +1,400 @@ +/*! + * Copyright (c) Microsoft Corporation and contributors. All rights reserved. + * Licensed under the MIT License. + */ + +import { strict as assert } from "assert"; + +import { setupRollbackTest, createAdditionalClient } from "@fluid-private/test-dds-utils"; + +import { IntervalCollection } from "../intervalCollection.js"; +// import type { SequenceInterval } from "../intervals/index.js"; +import { SharedStringFactory } from "../sequenceFactory.js"; +import { SharedStringClass } from "../sharedString.js"; + +describe("changeInterval: SharedString IntervalCollection rollback events", () => { + describe("changeInterval: single client", () => { + it("should trigger changeInterval on rollback of local endpoint modification", () => { + const { + dds: sharedString, + containerRuntimeFactory, + containerRuntime, + } = setupRollbackTest( + "shared-string-1", + (rt, id) => new SharedStringClass(rt, id, SharedStringFactory.Attributes), + { initialize: (dds) => dds.initializeLocal() }, + ); + + const collection = sharedString.getIntervalCollection("test"); + assert(collection instanceof IntervalCollection); + + sharedString.insertText(0, "abcde"); + const interval = collection.add({ start: 1, end: 3 }); + containerRuntime.flush(); + containerRuntimeFactory.processAllMessages(); + + let eventArgs: any = null; + collection.on("changeInterval", (i, previousInterval, local, op, slide) => { + eventArgs = { event: "changeInterval", i, previousInterval, local, op, slide }; + }); + + // Local change (not flushed) + collection.change(interval.getIntervalId(), { start: 2, end: 4 }); + assert(eventArgs.event === "changeInterval", "changeInterval event fired"); + + containerRuntime.rollback?.(); + + assert(eventArgs.local, "change is local"); + + // even a rollback triggers the event with slide: true. why? + // assert(!eventArgs.slide, "slide should be false"); + assert.equal( + sharedString.localReferencePositionToPosition(eventArgs.i.start), + 1, + "start reverted after rollback", + ); + + assert.equal( + sharedString.localReferencePositionToPosition(eventArgs.i.end), + 3, + "start reverted after rollback", + ); + }); + }); + + describe("multi-client(changeInterval)", () => { + it("should restore interval state after rollback, ignoring op and local flags", () => { + const { + dds: sharedString, + containerRuntimeFactory, + containerRuntime, + } = setupRollbackTest( + "shared-string-1", + (rt, id) => new SharedStringClass(rt, id, SharedStringFactory.Attributes), + { initialize: (dds) => dds.initializeLocal() }, + ); + + const collection = sharedString.getIntervalCollection("test"); + assert(collection instanceof IntervalCollection); + + const { dds: sharedString2, containerRuntime: containerRuntime2 } = + createAdditionalClient( + containerRuntimeFactory, + "2", + (rt, id) => + new SharedStringClass(rt, `shared-string-${id}`, SharedStringFactory.Attributes), + { initialize: (dds) => dds.initializeLocal() }, + ); + + const collection2 = sharedString2.getIntervalCollection("test"); + assert( + collection2 instanceof IntervalCollection, + "IntervalCollection instance expected", + ); + + let eventArgs: any = null; + let eventArgs2: any = null; + collection.on("changeInterval", (i, previousInterval) => { + eventArgs = { event: "changeInterval", i, previousInterval }; + }); + + collection2.on("changeInterval", (i, previousInterval) => { + eventArgs2 = { event: "changeInterval", i, previousInterval }; + }); + + sharedString.insertText(0, "abcde"); + containerRuntimeFactory.processAllMessages(); + + const interval = collection.add({ start: 0, end: 3 }); + const intervalId = interval.getIntervalId(); + containerRuntime.flush(); + containerRuntimeFactory.processAllMessages(); + + collection2.change(intervalId, { start: 1, end: 3 }); + assert( + eventArgs2.event === "changeInterval", + "changeInterval event fired by collection2", + ); + containerRuntime2.flush(); + containerRuntimeFactory.processAllMessages(); + + collection.change(intervalId, { start: 2, end: 3 }); + collection2.change(intervalId, { start: 3, end: 4 }); + + // Rollback local change + containerRuntime.rollback?.(); + + assert(eventArgs.event === "changeInterval", "changeInterval event fired"); + // The interval should reflect the remote change after rollback + assert.equal( + sharedString.localReferencePositionToPosition(eventArgs.i.start), + 1, + "start reflects remote change after rollback", + ); + + assert.equal( + sharedString.localReferencePositionToPosition(eventArgs.i.end), + 3, + "end reflects remote change after rollback", + ); + + assert.equal( + sharedString.localReferencePositionToPosition(eventArgs.previousInterval.start), + -1, + "previousInterval reflects local change before rollback", + ); + + assert.equal( + sharedString.localReferencePositionToPosition(eventArgs.previousInterval.end), + -1, + "previousInterval end reflects local change before rollback", + ); + + assert.equal( + sharedString2.localReferencePositionToPosition(eventArgs2.previousInterval.start), + 1, + "previousInterval reflects change by remote", + ); + + assert.equal( + sharedString2.localReferencePositionToPosition(eventArgs2.previousInterval.end), + 3, + "previousInterval end reflects change by remote", + ); + }); + + it("should fire correct events for local rollback and remote delete", () => { + const { + dds: ss1, + containerRuntimeFactory, + containerRuntime, + } = setupRollbackTest( + "shared-string-1", + (rt, id) => new SharedStringClass(rt, id, SharedStringFactory.Attributes), + { initialize: (dds) => dds.initializeLocal() }, + ); + + const collection1 = ss1.getIntervalCollection("test"); + ss1.insertText(0, "abcde"); + containerRuntimeFactory.processAllMessages(); + + const interval = collection1.add({ start: 0, end: 3 }); + const intervalId = interval.getIntervalId(); + containerRuntime.flush(); + containerRuntimeFactory.processAllMessages(); + + // Remote client + const { dds: ss2, containerRuntime: cr2 } = createAdditionalClient( + containerRuntimeFactory, + "2", + (rt, id) => + new SharedStringClass(rt, `shared-string-${id}`, SharedStringFactory.Attributes), + { initialize: (dds) => dds.initializeLocal() }, + ); + const collection2 = ss2.getIntervalCollection("test"); + + // Capture events + const events1: any[] = []; + collection1.on("changeInterval", (i, prev) => events1.push({ type: "change", i, prev })); + + const events2: any[] = []; + collection2.on("changeInterval", (i, prev) => events2.push({ type: "change", i, prev })); + + // Local unflushed change + collection1.change(intervalId, { start: 1, end: 2 }); + + // Rollback local change (interval still exists) + containerRuntime.rollback?.(); + containerRuntimeFactory.processAllMessages(); + + // Local events should capture the rollback + const rollbackEvent = events1.find( + (e) => ss1.localReferencePositionToPosition(e.i.start) === 0, + ); + assert(rollbackEvent, "rollback changeInterval captured"); + assert.equal( + ss1.localReferencePositionToPosition(rollbackEvent.i.end), + 3, + "rollback restores correct end position", + ); + + // Remote client deletes interval + collection2.removeIntervalById(intervalId); + cr2.flush(); + containerRuntimeFactory.processAllMessages(); + + // Attempting local change now does not fire events + collection1.change(intervalId, { start: 2, end: 3 }); + const newChangeEvents = events1.filter((e) => e.i.start === 2); + assert(newChangeEvents.length === 0, "no changeInterval fired on deleted interval"); + }); + }); +}); + +describe("addInterval/deleteInterval: SharedString IntervalCollection rollback events", () => { + it("should trigger addInterval on rollback of a locally added interval", () => { + const { + dds: sharedString, + containerRuntimeFactory, + containerRuntime, + } = setupRollbackTest( + "shared-string-1", + (rt, id) => new SharedStringClass(rt, id, SharedStringFactory.Attributes), + { initialize: (dds) => dds.initializeLocal() }, + ); + + const collection = sharedString.getIntervalCollection("test"); + assert(collection instanceof IntervalCollection); + + sharedString.insertText(0, "abcde"); + containerRuntimeFactory.processAllMessages(); + + let addEventArgs: any = null; + collection.on("addInterval", (interval1, local) => { + addEventArgs = { interval1, local }; + }); + + // Add interval locally + const interval = collection.add({ start: 1, end: 3 }); + + // Rollback the local addition + containerRuntime.rollback?.(); + + assert(addEventArgs, "addInterval event fired on rollback"); + // assert.strictEqual(addEventArgs.interval, interval, "correct interval in event"); + assert.strictEqual(addEventArgs.local, true, "event marked as local"); + // Interval should be removed after rollback + assert.strictEqual(collection.getIntervalById(interval.getIntervalId()), undefined); + assert(interval.disposed, "interval disposed after rollback"); + }); + + it("should trigger deleteInterval on rollback of a locally removed interval", () => { + const { + dds: sharedString, + containerRuntimeFactory, + containerRuntime, + } = setupRollbackTest( + "shared-string-1", + (rt, id) => new SharedStringClass(rt, id, SharedStringFactory.Attributes), + { initialize: (dds) => dds.initializeLocal() }, + ); + + const collection = sharedString.getIntervalCollection("test"); + assert(collection instanceof IntervalCollection); + + sharedString.insertText(0, "abcde"); + containerRuntimeFactory.processAllMessages(); + + // Add interval and flush so it's in the collection + const interval = collection.add({ start: 1, end: 3 }); + containerRuntime.flush(); + containerRuntimeFactory.processAllMessages(); + + let deleteEventArgs: any = null; + collection.on("deleteInterval", (deletedInterval, local) => { + deleteEventArgs = { deletedInterval, local }; + }); + + // Remove interval locally (not flushed) + collection.removeIntervalById(interval.getIntervalId()); + + // Rollback the local removal + containerRuntime.rollback?.(); + + assert(deleteEventArgs, "deleteInterval event fired on rollback"); + assert.strictEqual(deleteEventArgs.deletedInterval, interval, "correct interval in event"); + assert.strictEqual(deleteEventArgs.local, true, "event marked as local"); + // Interval should be restored after rollback + const restored = collection.getIntervalById(interval.getIntervalId()); + assert.strictEqual(restored, interval, "interval restored after rollback"); + assert(!interval.disposed, "interval not disposed after rollback"); + }); +}); + +describe("multi-client(addInterval/deleteInterval): SharedString IntervalCollection rollback events", () => { + it("should fire addInterval on rollback of local add with multiple clients", () => { + const { + dds: ss1, + containerRuntimeFactory, + containerRuntime, + } = setupRollbackTest( + "shared-string-1", + (rt, id) => new SharedStringClass(rt, id, SharedStringFactory.Attributes), + { initialize: (dds) => dds.initializeLocal() }, + ); + + const collection1 = ss1.getIntervalCollection("test"); + ss1.insertText(0, "abcde"); + containerRuntimeFactory.processAllMessages(); + + let addEventArgs: any = null; + collection1.on("addInterval", (int, local) => { + addEventArgs = { int, local }; + }); + + // Local add + const interval = collection1.add({ start: 1, end: 3 }); + + // Rollback local add + containerRuntime.rollback?.(); + + assert(addEventArgs, "addInterval event fired on rollback"); + assert.strictEqual(addEventArgs.local, true, "event marked as local"); + assert.strictEqual( + collection1.getIntervalById(interval.getIntervalId()), + undefined, + "interval removed", + ); + // assert(interval.disposed, "interval disposed after rollback"); + }); + + it("should fire deleteInterval on rollback of local remove with multiple clients", () => { + const { + dds: ss1, + containerRuntimeFactory, + containerRuntime, + } = setupRollbackTest( + "shared-string-1", + (rt, id) => new SharedStringClass(rt, id, SharedStringFactory.Attributes), + { initialize: (dds) => dds.initializeLocal() }, + ); + + const collection1 = ss1.getIntervalCollection("test"); + ss1.insertText(0, "abcde"); + containerRuntimeFactory.processAllMessages(); + + const interval = collection1.add({ start: 1, end: 3 }); + containerRuntime.flush(); + containerRuntimeFactory.processAllMessages(); + + // Second client modifies collection + const { dds: ss2 } = createAdditionalClient( + containerRuntimeFactory, + "2", + (rt, id) => + new SharedStringClass(rt, `shared-string-${id}`, SharedStringFactory.Attributes), + { initialize: (dds) => dds.initializeLocal() }, + ); + ss2.getIntervalCollection("test"); + + let deleteEventArgs: any = null; + collection1.on("deleteInterval", (deletedInterval, local) => { + deleteEventArgs = { deletedInterval, local }; + }); + + // Local removal + collection1.removeIntervalById(interval.getIntervalId()); + + // Rollback local remove + containerRuntime.rollback?.(); + + assert(deleteEventArgs, "deleteInterval event fired on rollback"); + assert.strictEqual(deleteEventArgs.deletedInterval, interval, "correct interval in event"); + assert.strictEqual(deleteEventArgs.local, true, "event marked as local"); + + // Interval restored + const restored = collection1.getIntervalById(interval.getIntervalId()); + assert.strictEqual(restored, interval, "interval restored after rollback"); + // assert(!interval.disposed, "interval not disposed after rollback"); + }); +}); diff --git a/packages/dds/sequence/src/test/sharedString.rollback.spec.ts b/packages/dds/sequence/src/test/sharedString.rollback.spec.ts index 99367e2c9ac8..e6f343d4a1d6 100644 --- a/packages/dds/sequence/src/test/sharedString.rollback.spec.ts +++ b/packages/dds/sequence/src/test/sharedString.rollback.spec.ts @@ -16,9 +16,12 @@ import { SharedStringFactory, type SharedString } from "../sequenceFactory.js"; import { SharedStringClass } from "../sharedString.js"; describe("SharedString rollback with multiple clients (insert/remove)", () => { - it("Client1 insert + Client2 insert + rollback on Client1", () => { - const { dds: client1, containerRuntimeFactory, containerRuntime: cr1 } = setupRollbackTest( + const { + dds: client1, + containerRuntimeFactory, + containerRuntime: cr1, + } = setupRollbackTest( "shared-string-1", (rt, id) => new SharedStringClass(rt, id, SharedStringFactory.Attributes), { initialize: (dds) => dds.initializeLocal() }, @@ -71,7 +74,11 @@ describe("SharedString rollback with multiple clients (insert/remove)", () => { }); it("Client1 remove + Client2 insert + rollback on Client1", () => { - const { dds: client1, containerRuntimeFactory, containerRuntime: cr1 } = setupRollbackTest( + const { + dds: client1, + containerRuntimeFactory, + containerRuntime: cr1, + } = setupRollbackTest( "shared-string-1", (rt, id) => new SharedStringClass(rt, id, SharedStringFactory.Attributes), { initialize: (dds) => dds.initializeLocal() }, @@ -117,7 +124,11 @@ describe("SharedString rollback with multiple clients (insert/remove)", () => { }); it("Client1 insert + Client2 remove + rollback on Client1", () => { - const { dds: client1, containerRuntimeFactory, containerRuntime: cr1 } = setupRollbackTest( + const { + dds: client1, + containerRuntimeFactory, + containerRuntime: cr1, + } = setupRollbackTest( "shared-string-1", (rt, id) => new SharedStringClass(rt, id, SharedStringFactory.Attributes), { initialize: (dds) => dds.initializeLocal() }, @@ -159,7 +170,11 @@ describe("SharedString rollback with multiple clients (insert/remove)", () => { }); it("Client1 insert + Client2 insert + rollback on Client2", () => { - const { dds: client1, containerRuntimeFactory, containerRuntime: cr1 } = setupRollbackTest( + const { + dds: client1, + containerRuntimeFactory, + containerRuntime: cr1, + } = setupRollbackTest( "shared-string-1", (rt, id) => new SharedStringClass(rt, id, SharedStringFactory.Attributes), { initialize: (dds) => dds.initializeLocal() }, @@ -213,42 +228,18 @@ describe("SharedString rollback with multiple clients (insert/remove)", () => { }); describe("SharedString replaceText with rollback and two clients", () => { - // let client1: SharedStringClass; - // let client2: SharedStringClass; - // let cr1: MockContainerRuntime; - // let cr2: MockContainerRuntime; - // let containerRuntimeFactory: MockContainerRuntimeFactory; - - // beforeEach(() => { - // // First client - // const setup = setupRollbackTest( - // "shared-string-1", - // (rt, id) => new SharedStringClass(rt, id, SharedStringFactory.Attributes), - // { initialize: (dds) => dds.initializeLocal() }, - // ); - // client1 = setup.dds; - // containerRuntimeFactory = setup.containerRuntimeFactory; - // cr1 = setup.containerRuntime; - - // // Second client - // const additional = createAdditionalClient( - // containerRuntimeFactory, - // "2", - // (rt, id) => - // new SharedStringClass(rt, `shared-string-${id}`, SharedStringFactory.Attributes), - // { initialize: (dds) => dds.initializeLocal() }, - // ); - // client2 = additional.dds; - // cr2 = additional.containerRuntime; - // }); it("Client1 replaceText + rollback without remote changes", () => { - const { dds: client1, containerRuntimeFactory, containerRuntime: cr1 } = setupRollbackTest( + const { + dds: client1, + containerRuntimeFactory, + containerRuntime: cr1, + } = setupRollbackTest( "shared-string-1", (rt, id) => new SharedStringClass(rt, id, SharedStringFactory.Attributes), { initialize: (dds) => dds.initializeLocal() }, ); - const { dds: client2, containerRuntime: cr2 } = createAdditionalClient( + const { dds: client2 } = createAdditionalClient( containerRuntimeFactory, "2", (rt, id) => @@ -273,13 +264,17 @@ describe("SharedString replaceText with rollback and two clients", () => { }); it("Client1 multiple replaceText + rollback", () => { - const { dds: client1, containerRuntimeFactory, containerRuntime: cr1 } = setupRollbackTest( + const { + dds: client1, + containerRuntimeFactory, + containerRuntime: cr1, + } = setupRollbackTest( "shared-string-1", (rt, id) => new SharedStringClass(rt, id, SharedStringFactory.Attributes), { initialize: (dds) => dds.initializeLocal() }, ); - const { dds: client2, containerRuntime: cr2 } = createAdditionalClient( + const { dds: client2 } = createAdditionalClient( containerRuntimeFactory, "2", (rt, id) => @@ -303,7 +298,11 @@ describe("SharedString replaceText with rollback and two clients", () => { }); it("Client1 replaceText with concurrent remote remove", () => { - const { dds: client1, containerRuntimeFactory, containerRuntime: cr1 } = setupRollbackTest( + const { + dds: client1, + containerRuntimeFactory, + containerRuntime: cr1, + } = setupRollbackTest( "shared-string-1", (rt, id) => new SharedStringClass(rt, id, SharedStringFactory.Attributes), { initialize: (dds) => dds.initializeLocal() }, @@ -344,7 +343,11 @@ describe("SharedString replaceText with rollback and two clients", () => { }); it("replaceText: Rollback on both clients", () => { - const { dds: client1, containerRuntimeFactory, containerRuntime: cr1 } = setupRollbackTest( + const { + dds: client1, + containerRuntimeFactory, + containerRuntime: cr1, + } = setupRollbackTest( "shared-string-1", (rt, id) => new SharedStringClass(rt, id, SharedStringFactory.Attributes), { initialize: (dds) => dds.initializeLocal() }, @@ -390,9 +393,12 @@ describe("SharedString replaceText with rollback and two clients", () => { }); describe("SharedString annotate with rollback", () => { - it("can annotate text and rollback without remote changes", () => { - const { dds: client1, containerRuntimeFactory, containerRuntime: cr1 } = setupRollbackTest( + const { + dds: client1, + containerRuntimeFactory, + containerRuntime: cr1, + } = setupRollbackTest( "shared-string-1", (rt, id) => new SharedStringClass(rt, id, SharedStringFactory.Attributes), { initialize: (dds) => dds.initializeLocal() }, @@ -446,13 +452,17 @@ describe("SharedString annotate with rollback", () => { }); it("can handle null annotations with rollback", () => { - const { dds: client1, containerRuntimeFactory, containerRuntime: cr1 } = setupRollbackTest( + const { + dds: client1, + containerRuntimeFactory, + containerRuntime: cr1, + } = setupRollbackTest( "shared-string-1", (rt, id) => new SharedStringClass(rt, id, SharedStringFactory.Attributes), { initialize: (dds) => dds.initializeLocal() }, ); - const { dds: client2, containerRuntime: cr2 } = createAdditionalClient( + const { dds: client2 } = createAdditionalClient( containerRuntimeFactory, "2", (rt, id) => @@ -491,13 +501,17 @@ describe("SharedString annotate with rollback", () => { }); it("handles multiple annotations with rollback", () => { - const { dds: client1, containerRuntimeFactory, containerRuntime: cr1 } = setupRollbackTest( + const { + dds: client1, + containerRuntimeFactory, + containerRuntime: cr1, + } = setupRollbackTest( "shared-string-1", (rt, id) => new SharedStringClass(rt, id, SharedStringFactory.Attributes), { initialize: (dds) => dds.initializeLocal() }, ); - const { dds: client2, containerRuntime: cr2 } = createAdditionalClient( + const { dds: client2 } = createAdditionalClient( containerRuntimeFactory, "2", (rt, id) => From e4ec91af4de1f7d581257b4712c0ff1cf9ed1fa2 Mon Sep 17 00:00:00 2001 From: Sonali Deshpande <48232592+sonalideshpandemsft@users.noreply.github.com> Date: Mon, 1 Sep 2025 22:23:09 +0000 Subject: [PATCH 11/11] feedback --- packages/dds/cell/src/test/cell.rollback.spec.ts | 9 ++++++++- .../src/test/mocha/directory.rollback.spec.ts | 16 ++++++++-------- .../intervalCollection.event.rollback.spec.ts | 1 - 3 files changed, 16 insertions(+), 10 deletions(-) diff --git a/packages/dds/cell/src/test/cell.rollback.spec.ts b/packages/dds/cell/src/test/cell.rollback.spec.ts index c99144a9613d..47e3304eea63 100644 --- a/packages/dds/cell/src/test/cell.rollback.spec.ts +++ b/packages/dds/cell/src/test/cell.rollback.spec.ts @@ -23,6 +23,8 @@ describe("Cell with rollback", () => { cell.on("valueChanged", (value) => events.push("valueChanged")); + cell.on("delete", () => events.push("delete")); + cell.set(10); assert.equal(cell.get(), 10); @@ -30,7 +32,7 @@ describe("Cell with rollback", () => { assert.equal(cell.get(), undefined); - assert.deepEqual(events, ["valueChanged"]); + assert.deepEqual(events, ["valueChanged", "delete"]); }); it("should emit delete on delete, and rollback should re-emit last valueChanged", async () => { @@ -49,6 +51,7 @@ describe("Cell with rollback", () => { cell.on("delete", () => events.push("delete")); cell.set(42); + assert.equal(events.shift(), "valueChanged"); containerRuntime.flush(); containerRuntimeFactory.processAllMessages(); @@ -155,6 +158,10 @@ describe("SharedCell rollback events with multiple clients", () => { // Rollback delete runtime1.rollback?.(); + // After rollback, this flush/process should not affect cell2. + runtime1.flush(); + containerRuntimeFactory.processAllMessages(); + // After rollback, value is restored assert.equal(cell1.get(), 42); assert.equal(cell2.get(), 42); diff --git a/packages/dds/map/src/test/mocha/directory.rollback.spec.ts b/packages/dds/map/src/test/mocha/directory.rollback.spec.ts index 7887fe4a37a5..a9364f544bac 100644 --- a/packages/dds/map/src/test/mocha/directory.rollback.spec.ts +++ b/packages/dds/map/src/test/mocha/directory.rollback.spec.ts @@ -16,7 +16,7 @@ describe("SharedDirectory rollback", () => { describe("Storage operations (root subdirectory)", () => { it("should rollback set operation", () => { const { dds: sharedDirectory, containerRuntime } = setupRollbackTest( - "shared-map", + "client-1", (rt, id): ISharedDirectory => directoryFactory.create(rt, id), ); const valueChanges: IValueChanged[] = []; @@ -51,7 +51,7 @@ describe("SharedDirectory rollback", () => { containerRuntime, containerRuntimeFactory, } = setupRollbackTest( - "shared-map", + "client-1", (rt, id): ISharedDirectory => directoryFactory.create(rt, id), ); sharedDirectory.set("key1", "value1"); @@ -89,7 +89,7 @@ describe("SharedDirectory rollback", () => { containerRuntime, containerRuntimeFactory, } = setupRollbackTest( - "shared-map", + "client-1", (rt, id): ISharedDirectory => directoryFactory.create(rt, id), ); sharedDirectory.set("key1", "value1"); @@ -151,7 +151,7 @@ describe("SharedDirectory rollback", () => { containerRuntime, containerRuntimeFactory, } = setupRollbackTest( - "shared-map", + "client-1", (rt, id): ISharedDirectory => directoryFactory.create(rt, id), ); sharedDirectory.set("key1", "value1"); @@ -234,7 +234,7 @@ describe("SharedDirectory rollback", () => { containerRuntime, containerRuntimeFactory, } = setupRollbackTest( - "shared-map", + "client-1", (rt, id): ISharedDirectory => directoryFactory.create(rt, id), ); // Create a second client @@ -293,7 +293,7 @@ describe("SharedDirectory rollback", () => { containerRuntime, containerRuntimeFactory, } = setupRollbackTest( - "shared-map", + "client-1", (rt, id): ISharedDirectory => directoryFactory.create(rt, id), ); @@ -375,7 +375,7 @@ describe("SharedDirectory rollback", () => { containerRuntime, containerRuntimeFactory, } = setupRollbackTest( - "shared-map", + "client-1", (rt, id): ISharedDirectory => directoryFactory.create(rt, id), ); @@ -446,7 +446,7 @@ describe("SharedDirectory rollback", () => { containerRuntime, containerRuntimeFactory, } = setupRollbackTest( - "shared-map", + "client-1", (rt, id): ISharedDirectory => directoryFactory.create(rt, id), ); diff --git a/packages/dds/sequence/src/test/intervalCollection.event.rollback.spec.ts b/packages/dds/sequence/src/test/intervalCollection.event.rollback.spec.ts index ae18a1b9cadd..27bbe733c368 100644 --- a/packages/dds/sequence/src/test/intervalCollection.event.rollback.spec.ts +++ b/packages/dds/sequence/src/test/intervalCollection.event.rollback.spec.ts @@ -8,7 +8,6 @@ import { strict as assert } from "assert"; import { setupRollbackTest, createAdditionalClient } from "@fluid-private/test-dds-utils"; import { IntervalCollection } from "../intervalCollection.js"; -// import type { SequenceInterval } from "../intervals/index.js"; import { SharedStringFactory } from "../sequenceFactory.js"; import { SharedStringClass } from "../sharedString.js";