From b51f82dc0d7641c9fd43c314470166305cb6c2f1 Mon Sep 17 00:00:00 2001 From: mj Date: Sat, 12 Oct 2024 13:32:32 +0900 Subject: [PATCH 1/2] feat: update `removedAt` for deleted nodes during range deletions - Add logic to update `removedAt` for nodes with outdated deletion times - Ensure that garbage collection accounts for the latest `removedAt` updates - Maintain existing behavior for local operations while enhancing remote operation consistency --- packages/sdk/src/document/crdt/tree.ts | 43 ++++++++++++++------------ 1 file changed, 23 insertions(+), 20 deletions(-) diff --git a/packages/sdk/src/document/crdt/tree.ts b/packages/sdk/src/document/crdt/tree.ts index 03127041a..0527e5196 100644 --- a/packages/sdk/src/document/crdt/tree.ts +++ b/packages/sdk/src/document/crdt/tree.ts @@ -14,36 +14,36 @@ * limitations under the License. */ +import { CRDTElement } from '@yorkie-js-sdk/src/document/crdt/element'; import { - TimeTicket, InitialTimeTicket, - TimeTicketStruct, MaxTimeTicket, + TimeTicket, + TimeTicketStruct, } from '@yorkie-js-sdk/src/document/time/ticket'; -import { CRDTElement } from '@yorkie-js-sdk/src/document/crdt/element'; +import type * as Devtools from '@yorkie-js-sdk/src/devtools/types'; +import { GCChild, GCPair, GCParent } from '@yorkie-js-sdk/src/document/crdt/gc'; +import { Indexable } from '@yorkie-js-sdk/src/document/document'; +import { escapeString } from '@yorkie-js-sdk/src/document/json/strings'; +import { Comparator } from '@yorkie-js-sdk/src/util/comparator'; +import { Code, YorkieError } from '@yorkie-js-sdk/src/util/error'; +import type { + DefaultTextType, + TreeNodeType, + TreeToken, +} from '@yorkie-js-sdk/src/util/index_tree'; import { IndexTree, - TreePos, IndexTreeNode, - traverseAll, TokenType, + TreePos, + traverseAll, } from '@yorkie-js-sdk/src/util/index_tree'; -import { RHT, RHTNode } from './rht'; -import { ActorID } from './../time/actor_id'; import { LLRBTree } from '@yorkie-js-sdk/src/util/llrb_tree'; -import { Comparator } from '@yorkie-js-sdk/src/util/comparator'; import { parseObjectValues } from '@yorkie-js-sdk/src/util/object'; -import type { - DefaultTextType, - TreeNodeType, - TreeToken, -} from '@yorkie-js-sdk/src/util/index_tree'; -import { Indexable } from '@yorkie-js-sdk/src/document/document'; -import type * as Devtools from '@yorkie-js-sdk/src/devtools/types'; -import { escapeString } from '@yorkie-js-sdk/src/document/json/strings'; -import { GCChild, GCPair, GCParent } from '@yorkie-js-sdk/src/document/crdt/gc'; -import { Code, YorkieError } from '@yorkie-js-sdk/src/util/error'; +import { ActorID } from './../time/actor_id'; +import { RHT, RHTNode } from './rht'; /** * `TreeNode` represents a node in the tree. @@ -1094,6 +1094,9 @@ export class CRDTTree extends CRDTElement implements GCParent { nodesToBeRemoved.push(node); } tokensToBeRemoved.push([node, tokenType]); + } else if (node.isRemoved && editedAt.after(node.removedAt!)) { + node.removedAt = editedAt; + pairs.push({ parent: this, child: node }); } }, ); @@ -1371,8 +1374,8 @@ export class CRDTTree extends CRDTElement implements GCParent { const treePos = node.isText ? { node, offset: 0 } : parentNode && leftChildNode - ? this.toTreePos(parentNode, leftChildNode) - : null; + ? this.toTreePos(parentNode, leftChildNode) + : null; if (treePos) { index = this.indexTree.indexOf(treePos); From ff644b06c35a7af0674c09a7afc8b892b1cfb163 Mon Sep 17 00:00:00 2001 From: Sejong Kim Date: Sun, 13 Oct 2024 19:15:50 +0900 Subject: [PATCH 2/2] Add test case --- packages/sdk/test/helper/helper.ts | 9 ++++- .../sdk/test/unit/document/crdt/tree_test.ts | 33 ++++++++++++++++++- 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/packages/sdk/test/helper/helper.ts b/packages/sdk/test/helper/helper.ts index 041ed557f..eb5e77423 100644 --- a/packages/sdk/test/helper/helper.ts +++ b/packages/sdk/test/helper/helper.ts @@ -187,7 +187,7 @@ export async function assertThrowsAsync( message?: string, ) { // eslint-disable-next-line @typescript-eslint/no-empty-function - let errFn = () => {}; + let errFn = () => { }; try { await fn(); } catch (e) { @@ -285,3 +285,10 @@ export function posT(offset = 0): CRDTTreeNodeID { export function timeT(): TimeTicket { return dummyContext.issueTimeTicket(); } + +/** + * `getLTT` is a helpher function that returns the last time ticket. + */ +export function getLTT(): TimeTicket { + return dummyContext.getLastTimeTicket(); +} diff --git a/packages/sdk/test/unit/document/crdt/tree_test.ts b/packages/sdk/test/unit/document/crdt/tree_test.ts index 1b3997a7d..8dce25929 100644 --- a/packages/sdk/test/unit/document/crdt/tree_test.ts +++ b/packages/sdk/test/unit/document/crdt/tree_test.ts @@ -28,7 +28,7 @@ import { TreeNodeForTest, } from '@yorkie-js-sdk/src/document/crdt/tree'; import { stringifyObjectValues } from '@yorkie-js-sdk/src/util/object'; -import { idT, posT, timeT } from '@yorkie-js-sdk/test/helper/helper'; +import { idT, posT, timeT, getLTT } from '@yorkie-js-sdk/test/helper/helper'; describe('CRDTTreeNode', function () { it('Can be created', function () { @@ -231,6 +231,37 @@ describe('CRDTTree.Edit', function () { ); assert.equal(t.toIndex(parent, left), 0); }); + + it('Update removedAt for already deleted nodes during range deletions', function () { + // 0 + // + const t = new CRDTTree(new CRDTTreeNode(posT(), 'r'), timeT()); + assert.equal(t.getRoot().size, 0); + assert.equal(t.toXML(), /*html*/ ``); + + // 1 + //

h e l l o

+ const p = new CRDTTreeNode(posT(), 'p', []); + const txt = new CRDTTreeNode(posT(), 'text', 'hello'); + p.insertAt(txt, 0); + t.editT([0, 0], [p], 0, timeT(), timeT); + assert.equal(t.toXML(), /*html*/ `

hello

`); + + // 2 + //

+ t.editT([1, 6], undefined, 0, timeT(), timeT); + assert.equal(t.toXML(), /*html*/ `

`); + assert.isFalse(p.isRemoved); + assert.isTrue(txt.getRemovedAt()?.equals(getLTT())); + + // 3 + // + t.editT([0, 2], undefined, 0, timeT(), timeT); + assert.equal(t.toXML(), /*html*/ ``); + assert.isTrue(p.getRemovedAt()?.equals(getLTT())); + assert.isTrue(txt.getRemovedAt()?.equals(getLTT())); + }); + }); describe('CRDTTree.Split', function () {