Handle reverse operation for nest object that other peers deleted

yorkie-team · Nov 22, 2023 · b4b6504 · b4b6504
1 parent 663005f
commit b4b6504
Show file tree

Hide file tree

Showing 4 changed files with 162 additions and 10 deletions.
diff --git a/src/document/crdt/root.ts b/src/document/crdt/root.ts
@@ -115,6 +115,16 @@ export class CRDTRoot {
     return pair.element;
   }
 
+  /**
+   * `findElementPairByCreatedAt` returns the element and parent pair
+   * of given creation time.
+   */
+  public findElementPairByCreatedAt(
+    createdAt: TimeTicket,
+  ): CRDTElementPair | undefined {
+    return this.elementPairMapByCreatedAt.get(createdAt.toIDString());
+  }
+
   /**
    * `createSubPaths` creates an array of the sub paths for the given element.
    */

diff --git a/src/document/operation/remove_operation.ts b/src/document/operation/remove_operation.ts
@@ -72,12 +72,18 @@ export class RemoveOperation extends Operation {
     }
 
     // NOTE(chacha912): Handle cases where operation cannot be executed during undo and redo.
-    const targetElem = parentObject.getByID(this.createdAt);
-    if (
-      source === OpSource.UndoRedo &&
-      (parentObject.getRemovedAt() || !targetElem || targetElem.isRemoved())
-    ) {
-      return;
+    if (source === OpSource.UndoRedo) {
+      const targetElem = parentObject.getByID(this.createdAt);
+      if (targetElem?.isRemoved()) {
+        return;
+      }
+      let parent: CRDTContainer | undefined = parentObject;
+      while (parent) {
+        if (parent.getRemovedAt()) {
+          return;
+        }
+        parent = root.findElementPairByCreatedAt(parent.getCreatedAt())?.parent;
+      }
     }
     const key = parentObject.subPathOf(this.createdAt);
     const reverseOp = this.toReverseOperation(parentObject);

diff --git a/src/document/operation/set_operation.ts b/src/document/operation/set_operation.ts
@@ -76,8 +76,14 @@ export class SetOperation extends Operation {
     }
     const obj = parentObject as CRDTObject;
     // NOTE(chacha912): Handle cases where operation cannot be executed during undo and redo.
-    if (source === OpSource.UndoRedo && obj.getRemovedAt()) {
-      return;
+    if (source === OpSource.UndoRedo) {
+      let parent: CRDTContainer | undefined = obj;
+      while (parent) {
+        if (parent.getRemovedAt()) {
+          return;
+        }
+        parent = root.findElementPairByCreatedAt(parent.getCreatedAt())?.parent;
+      }
     }
     const previousValue = obj.get(this.key);
     const reverseOp = this.toReverseOperation(previousValue);

diff --git a/test/integration/object_test.ts b/test/integration/object_test.ts
@@ -407,7 +407,7 @@ describe('Object', function () {
       assert.equal(doc2.toSortedJSON(), '{"shape":{"point":{"x":0,"y":0}}}');
     });
 
-    it(`Should handle reverse (set) operation targeting elements deleted by other peers`, async function ({
+    it(`Should handle reverse set operation for elements that other peers deleted`, async function ({
       task,
     }) {
       // Test scenario:
@@ -454,12 +454,80 @@ describe('Object', function () {
       assert.equal(doc1.toSortedJSON(), '{}');
       assert.equal(doc1.getRedoStackForTest().length, 0);
       assert.equal(doc1.history.canRedo(), false);
+    });
+
+    it(`Should handle reverse set operation for elements (nested objects) that other peers deleted`, async function ({
+      task,
+    }) {
+      // Test scenario:
+      // c1: create shape
+      // c1: set shape.circle.point to { x: 1, y: 1 }
+      // c2: delete shape
+      // c1: undo(no changes as the shape was deleted)
+      interface TestDoc {
+        shape?: { circle: { point: { x: number; y: number } } };
+      }
+      const docKey = toDocKey(`${task.name}-${new Date().getTime()}`);
+      const doc1 = new Document<TestDoc>(docKey);
+      const doc2 = new Document<TestDoc>(docKey);
+
+      const client1 = new Client(testRPCAddr);
+      const client2 = new Client(testRPCAddr);
+      await client1.activate();
+      await client2.activate();
+
+      await client1.attach(doc1, { isRealtimeSync: false });
+      doc1.update((root) => {
+        root.shape = { circle: { point: { x: 0, y: 0 } } };
+      });
       await client1.sync();
+      assert.equal(
+        doc1.toSortedJSON(),
+        '{"shape":{"circle":{"point":{"x":0,"y":0}}}}',
+      );
+
+      await client2.attach(doc2, { isRealtimeSync: false });
+      assert.equal(
+        doc2.toSortedJSON(),
+        '{"shape":{"circle":{"point":{"x":0,"y":0}}}}',
+      );
+
+      doc1.update((root) => {
+        root.shape!.circle.point = { x: 1, y: 1 };
+      });
+      await client1.sync();
+      await client2.sync();
+      assert.equal(
+        doc1.toSortedJSON(),
+        '{"shape":{"circle":{"point":{"x":1,"y":1}}}}',
+      );
+      assert.equal(
+        doc2.toSortedJSON(),
+        '{"shape":{"circle":{"point":{"x":1,"y":1}}}}',
+      );
+      doc2.update((root) => {
+        delete root.shape;
+      }, 'delete shape');
       await client2.sync();
       await client1.sync();
+      assert.equal(doc1.toSortedJSON(), '{}');
+      assert.equal(doc2.toSortedJSON(), '{}');
+
+      const c1ID = client1.getID()!.slice(-2);
+      assert.deepEqual(
+        doc1.getUndoStackForTest().at(-1)?.map(toStringHistoryOp),
+        [`2:${c1ID}:2.SET.point={"x":0,"y":0}`],
+      );
+      doc1.history.undo();
+      assert.equal(doc1.toSortedJSON(), '{}');
+      await client1.sync();
+      await client2.sync();
+      assert.equal(doc2.toSortedJSON(), '{}');
+      assert.equal(doc1.getRedoStackForTest().length, 0);
+      assert.equal(doc1.history.canRedo(), false);
     });
 
-    it(`Should handle reverse (remove) operation targeting elements deleted by other peers`, async function ({
+    it(`Should handle reverse remove operation for elements that other peers deleted`, async function ({
       task,
     }) {
       // Test scenario:
@@ -501,9 +569,71 @@ describe('Object', function () {
       assert.equal(doc1.toSortedJSON(), '{}');
       assert.equal(doc1.getRedoStackForTest().length, 0);
       assert.equal(doc1.history.canRedo(), false);
+    });
+
+    it(`Should handle reverse remove operation for elements (nested objects) that other peers deleted`, async function ({
+      task,
+    }) {
+      // Test scenario:
+      // c1: set shape.circle.point to { x: 0, y: 0 }
+      // c2: delete shape
+      // c1: undo(no changes as the shape was deleted)
+      interface TestDoc {
+        shape?: {
+          circle?: { point?: { x?: number; y?: number }; color?: string };
+        };
+      }
+      const docKey = toDocKey(`${task.name}-${new Date().getTime()}`);
+      const doc1 = new Document<TestDoc>(docKey);
+      const doc2 = new Document<TestDoc>(docKey);
+
+      const client1 = new Client(testRPCAddr);
+      const client2 = new Client(testRPCAddr);
+      await client1.activate();
+      await client2.activate();
+
+      await client1.attach(doc1, { isRealtimeSync: false });
+      doc1.update((root) => {
+        root.shape = { circle: { color: 'red' } };
+      });
       await client1.sync();
+      assert.equal(doc1.toSortedJSON(), '{"shape":{"circle":{"color":"red"}}}');
+
+      await client2.attach(doc2, { isRealtimeSync: false });
+      assert.equal(doc2.toSortedJSON(), '{"shape":{"circle":{"color":"red"}}}');
+
+      doc1.update((root) => {
+        root.shape!.circle!.point = { x: 0, y: 0 };
+      });
+      await client1.sync();
+      await client2.sync();
+      assert.equal(
+        doc1.toSortedJSON(),
+        '{"shape":{"circle":{"color":"red","point":{"x":0,"y":0}}}}',
+      );
+      assert.equal(
+        doc2.toSortedJSON(),
+        '{"shape":{"circle":{"color":"red","point":{"x":0,"y":0}}}}',
+      );
+      doc2.update((root) => {
+        delete root.shape;
+      }, 'delete shape');
       await client2.sync();
       await client1.sync();
+      assert.equal(doc1.toSortedJSON(), '{}');
+      assert.equal(doc2.toSortedJSON(), '{}');
+
+      const c1ID = client1.getID()!.slice(-2);
+      assert.deepEqual(
+        doc1.getUndoStackForTest().at(-1)?.map(toStringHistoryOp),
+        [`2:${c1ID}:2.REMOVE.3:${c1ID}:1`],
+      );
+      doc1.history.undo();
+      assert.equal(doc1.toSortedJSON(), '{}');
+      await client1.sync();
+      await client2.sync();
+      assert.equal(doc2.toSortedJSON(), '{}');
+      assert.deepEqual(doc1.getRedoStackForTest().length, 0);
     });
 
     it(`Should not propagate changes when there is no applied undo operation`, async function ({