From 7c8313a93f99a5fd924058c272cecf99121448f3 Mon Sep 17 00:00:00 2001 From: Youngteac Hong Date: Fri, 27 Oct 2023 12:10:08 +0900 Subject: [PATCH] Fix missing collection of removed elements from the root (#676) A document has two structures representing the user data: clone and root. When elements are removed by document.update, they are collected to removedElementMapByCreatedAt for garbage collection. However, we found a bug in that the collection is missing in the root. So this commit fixes it. Co-authored-by: Yourim Cha --- src/document/document.ts | 7 +++++++ src/document/operation/set_operation.ts | 6 +++++- test/integration/gc_test.ts | 23 +++++++++++++++++++++++ test/unit/document/document_test.ts | 2 +- 4 files changed, 36 insertions(+), 2 deletions(-) diff --git a/src/document/document.ts b/src/document/document.ts index 601b24655..d5b5f8f29 100644 --- a/src/document/document.ts +++ b/src/document/document.ts @@ -977,6 +977,13 @@ export class Document { return this.root.getGarbageLen(); } + /** + * `getGarbageLenFromClone` returns the length of elements should be purged from clone. + */ + public getGarbageLenFromClone(): number { + return this.clone!.root.getGarbageLen(); + } + /** * `toJSON` returns the JSON encoding of this document. */ diff --git a/src/document/operation/set_operation.ts b/src/document/operation/set_operation.ts index dae98d860..88884ff44 100644 --- a/src/document/operation/set_operation.ts +++ b/src/document/operation/set_operation.ts @@ -68,8 +68,12 @@ export class SetOperation extends Operation { } const obj = parentObject as CRDTObject; const value = this.value.deepcopy(); - obj.set(this.key, value); + const removed = obj.set(this.key, value); root.registerElement(value, obj); + if (removed) { + root.registerRemovedElement(removed); + } + return { opInfos: [ { diff --git a/test/integration/gc_test.ts b/test/integration/gc_test.ts index 9a8142646..3ea460201 100644 --- a/test/integration/gc_test.ts +++ b/test/integration/gc_test.ts @@ -586,4 +586,27 @@ describe('Garbage Collection', function () { await client1.deactivate(); await client2.deactivate(); }); + + it('Can collect removed elements from both root and clone', async function ({ + task, + }) { + type TestDoc = { point: { x: number; y: number } }; + const docKey = toDocKey(`${task.name}-${new Date().getTime()}`); + const doc = new yorkie.Document(docKey); + const cli = new yorkie.Client(testRPCAddr); + await cli.activate(); + + await cli.attach(doc, { isRealtimeSync: false }); + doc.update((root) => { + root.point = { x: 0, y: 0 }; + }); + doc.update((root) => { + root.point = { x: 1, y: 1 }; + }); + doc.update((root) => { + root.point = { x: 2, y: 2 }; + }); + assert.equal(doc.getGarbageLen(), 6); + assert.equal(doc.getGarbageLenFromClone(), 6); + }); }); diff --git a/test/unit/document/document_test.ts b/test/unit/document/document_test.ts index ea326bd6b..ccdac7e8f 100644 --- a/test/unit/document/document_test.ts +++ b/test/unit/document/document_test.ts @@ -1189,7 +1189,7 @@ describe.sequential('Document', function () { delete root.a; }); assert.equal('{}', doc.toSortedJSON()); - assert.equal(1, doc.getGarbageLen()); + assert.equal(2, doc.getGarbageLen()); doc.garbageCollect(MaxTimeTicket); assert.equal('{}', doc.toSortedJSON());