From 4bebe31a776d215a4a986b99387f6fadf1479089 Mon Sep 17 00:00:00 2001
From: JOOHOJANG <46807540+JOOHOJANG@users.noreply.github.com>
Date: Thu, 29 Jun 2023 15:27:01 +0900
Subject: [PATCH] Fix garbage collection bug (#555)

Make it traverse the whole tree including tombstones
---
 src/document/crdt/tree.ts   |  2 +-
 src/util/index_tree.ts      | 21 +++++++++++++++++++++
 test/integration/gc_test.ts | 34 ++++++++++++++++++++++++++++++++++
 3 files changed, 56 insertions(+), 1 deletion(-)

diff --git a/src/document/crdt/tree.ts b/src/document/crdt/tree.ts
index be7560ecf..a164c9f2e 100644
--- a/src/document/crdt/tree.ts
+++ b/src/document/crdt/tree.ts
@@ -663,7 +663,7 @@ export class CRDTTree extends CRDTGCElement {
       }
     }
 
-    this.indexTree.traverse((treeNode) => {
+    this.indexTree.traverseAll((treeNode) => {
       if (nodesToRemoved.has(treeNode)) {
         const parent = treeNode.parent;
 
diff --git a/src/util/index_tree.ts b/src/util/index_tree.ts
index 06c8a0f6f..c183732ec 100644
--- a/src/util/index_tree.ts
+++ b/src/util/index_tree.ts
@@ -523,6 +523,20 @@ export function traverse<T extends IndexTreeNode<T>>(
   callback(node, depth);
 }
 
+/**
+ * `traverseAll` traverses the whole tree (include tombstones) with postorder traversal.
+ */
+function traverseAll<T extends IndexTreeNode<T>>(
+  node: T,
+  callback: (node: T, depth: number) => void,
+  depth = 0,
+) {
+  for (const child of node._children) {
+    traverseAll(child, callback, depth + 1);
+  }
+  callback(node, depth);
+}
+
 /**
  * `findTreePos` finds the position of the given index in the given node.
  */
@@ -675,6 +689,13 @@ export class IndexTree<T extends IndexTreeNode<T>> {
     traverse(this.root, callback, 0);
   }
 
+  /**
+   * `traverseAll` traverses the whole tree (include tombstones) with postorder traversal.
+   */
+  traverseAll(callback: (node: T) => void): void {
+    traverseAll(this.root, callback, 0);
+  }
+
   /**
    * `split` splits the node at the given index.
    */
diff --git a/test/integration/gc_test.ts b/test/integration/gc_test.ts
index e267bc602..c2ea2770b 100644
--- a/test/integration/gc_test.ts
+++ b/test/integration/gc_test.ts
@@ -7,6 +7,23 @@ import {
   toDocKey,
 } from '@yorkie-js-sdk/test/integration/integration_helper';
 import { Text } from '@yorkie-js-sdk/src/yorkie';
+import { CRDTTreeNode } from '@yorkie-js-sdk/src/document/crdt/tree';
+import { IndexTreeNode } from '@yorkie-js-sdk/src/util/index_tree';
+
+// `getNodeLength` returns the number of nodes in the given tree.
+function getNodeLength(root: IndexTreeNode<CRDTTreeNode>) {
+  let size = 0;
+
+  size += root._children.length;
+
+  if (root._children.length) {
+    root._children.forEach((child) => {
+      size += getNodeLength(child);
+    });
+  }
+
+  return size;
+}
 
 describe('Garbage Collection', function () {
   it('garbage collection test', function () {
@@ -204,9 +221,16 @@ describe('Garbage Collection', function () {
     });
 
     // [text(a), text(b)]
+    let nodeLengthBeforeGC = getNodeLength(
+      doc.getRoot().t.getIndexTree().getRoot(),
+    );
     assert.equal(doc.getGarbageLen(), 2);
     assert.equal(doc.garbageCollect(MaxTimeTicket), 2);
     assert.equal(doc.getGarbageLen(), 0);
+    let nodeLengthAfterGC = getNodeLength(
+      doc.getRoot().t.getIndexTree().getRoot(),
+    );
+    assert.equal(nodeLengthBeforeGC - nodeLengthAfterGC, 1);
 
     doc.update((root) => {
       root.t.editByPath([0, 0, 0], [0, 0, 2], { type: 'text', value: 'cv' });
@@ -214,9 +238,14 @@ describe('Garbage Collection', function () {
     });
 
     // [text(cd)]
+    nodeLengthBeforeGC = getNodeLength(
+      doc.getRoot().t.getIndexTree().getRoot(),
+    );
     assert.equal(doc.getGarbageLen(), 1);
     assert.equal(doc.garbageCollect(MaxTimeTicket), 1);
     assert.equal(doc.getGarbageLen(), 0);
+    nodeLengthAfterGC = getNodeLength(doc.getRoot().t.getIndexTree().getRoot());
+    assert.equal(nodeLengthBeforeGC - nodeLengthAfterGC, 1);
 
     doc.update((root) => {
       root.t.editByPath([0], [1], {
@@ -227,9 +256,14 @@ describe('Garbage Collection', function () {
     });
 
     // [p, tn, tn, text(cv), text(cd)]
+    nodeLengthBeforeGC = getNodeLength(
+      doc.getRoot().t.getIndexTree().getRoot(),
+    );
     assert.equal(doc.getGarbageLen(), 5);
     assert.equal(doc.garbageCollect(MaxTimeTicket), 5);
     assert.equal(doc.getGarbageLen(), 0);
+    nodeLengthAfterGC = getNodeLength(doc.getRoot().t.getIndexTree().getRoot());
+    assert.equal(nodeLengthBeforeGC - nodeLengthAfterGC, 6);
   });
 
   it('Can handle tree garbage collection for multi client', async function () {