Add RHTNode removal to converter for consistency (#842)

This commit addresses the missing `isRemoved` encoding in the RHT.
Similar to other CRDTs like ElementRHT, including tombstone nodes like
`isRemoved` during encoding is crucial. However, the RHT did not
include tombstone nodes in its encoding, leading to inconsistencies in
snapshots.
---------

Co-authored-by: Youngteac Hong <[email protected]>

src/api/converter.ts

@@ -592,6 +592,22 @@ function toTreeNodesWhenEdit(nodes: Array<CRDTTreeNode>): Array<PbTreeNodes> {
   return pbTreeNodesList;
 }
 
+/**
+ * `toRHT` converts the given model to Protobuf format.
+ */
+function toRHT(rht: RHT): { [key: string]: PbNodeAttr } {
+  const pbRHT: { [key: string]: PbNodeAttr } = {};
+  for (const node of rht) {
+    pbRHT[node.getKey()] = new PbNodeAttr({
+      value: node.getValue(),
+      updatedAt: toTimeTicket(node.getUpdatedAt()),
+      isRemoved: node.isRemoved(),
+    });
+  }
+
+  return pbRHT;
+}
+
 /**
  * `toTreeNodes` converts the given model to Protobuf format.
  */
@@ -620,12 +636,7 @@ function toTreeNodes(node: CRDTTreeNode): Array<PbTreeNode> {
     }
 
     if (n.attrs) {
-      for (const attr of n.attrs) {
-        pbTreeNode.attributes[attr.getKey()] = new PbNodeAttr({
-          value: attr.getValue(),
-          updatedAt: toTimeTicket(attr.getUpdatedAt()),
-        });
-      }
+      pbTreeNode.attributes = toRHT(n.attrs);
     }
 
     pbTreeNodes.push(pbTreeNode);
@@ -1053,6 +1064,23 @@ function fromTreeNodes(
   return CRDTTree.create(root, InitialTimeTicket).getRoot();
 }
 
+/**
+ * `fromRHT` converts the given Protobuf format to model format.
+ */
+function fromRHT(pbRHT: { [key: string]: PbNodeAttr }): RHT {
+  const rht = RHT.create();
+  for (const [key, pbRHTNode] of Object.entries(pbRHT)) {
+    rht.setInternal(
+      key,
+      pbRHTNode.value,
+      fromTimeTicket(pbRHTNode.updatedAt)!,
+      pbRHTNode.isRemoved,
+    );
+  }
+
+  return rht;
+}
+
 /**
  * `fromTreeNode` converts the given Protobuf format to model format.
  */
@@ -1063,11 +1091,7 @@ function fromTreeNode(pbTreeNode: PbTreeNode): CRDTTreeNode {
   if (node.isText) {
     node.value = pbTreeNode.value;
   } else if (pbAttrs.length) {
-    const attrs = RHT.create();
-    for (const [key, value] of pbAttrs) {
-      attrs.set(key, value.value, fromTimeTicket(value.updatedAt)!);
-    }
-    node.attrs = attrs;
+    node.attrs = fromRHT(pbTreeNode.attributes);
   }
 
   if (pbTreeNode.insPrevId) {

src/api/yorkie/v1/resources.proto

@@ -226,6 +226,7 @@ message RGANode {
 message NodeAttr {
   string value = 1;
   TimeTicket updated_at = 2;
+  bool is_removed = 3;
 }
 
 message TextNode {

src/api/yorkie/v1/resources_pb.d.ts

@@ -1228,6 +1228,11 @@ export declare class NodeAttr extends Message<NodeAttr> {
    */
   updatedAt?: TimeTicket;
 
+  /**
+   * @generated from field: bool is_removed = 3;
+   */
+  isRemoved: boolean;
+
   constructor(data?: PartialMessage<NodeAttr>);
 
   static readonly runtime: typeof proto3;

src/api/yorkie/v1/resources_pb.js

@@ -436,6 +436,7 @@ const NodeAttr = proto3.makeMessageType(
   () => [
     { no: 1, name: "value", kind: "scalar", T: 9 /* ScalarType.STRING */ },
     { no: 2, name: "updated_at", kind: "message", T: TimeTicket },
+    { no: 3, name: "is_removed", kind: "scalar", T: 8 /* ScalarType.BOOL */ },
   ],
 );
 

src/document/crdt/rht.ts

@@ -118,6 +118,13 @@ export class RHT {
     return new RHT();
   }
 
+  /**
+   * `getNodeMapByKey` returns the hashtable of RHT.
+   */
+  public getNodeMapByKey(): Map<string, RHTNode> {
+    return this.nodeMapByKey;
+  }
+
   /**
    * `set` sets the value of the given key.
    */
@@ -149,6 +156,23 @@ export class RHT {
     return [undefined, undefined];
   }
 
+  /**
+   * SetInternal sets the value of the given key internally.
+   */
+  public setInternal(
+    key: string,
+    value: string,
+    executedAt: TimeTicket,
+    removed: boolean,
+  ) {
+    const node = RHTNode.of(key, value, executedAt, removed);
+    this.nodeMapByKey.set(key, node);
+
+    if (removed) {
+      this.numberOfRemovedElement++;
+    }
+  }
+
   /**
    * `remove` removes the Element of the given key.
    */
@@ -213,7 +237,12 @@ export class RHT {
   public deepcopy(): RHT {
     const rht = new RHT();
     for (const [, node] of this.nodeMapByKey) {
-      rht.set(node.getKey(), node.getValue(), node.getUpdatedAt());
+      rht.setInternal(
+        node.getKey(),
+        node.getValue(),
+        node.getUpdatedAt(),
+        node.isRemoved(),
+      );
     }
     return rht;
   }

test/unit/api/converter_test.ts

@@ -103,8 +103,13 @@ describe('Converter', function () {
       });
 
       root.tree.editByPath([0, 1], [1, 1]);
+
+      root.tree.style(0, 1, { b: 't', i: 't' });
+      assert.equal(root.tree.toXML(), '<r><p b="t" i="t">14</p></r>');
+
+      root.tree.removeStyle(0, 1, ['i']);
     });
-    assert.equal(doc.getRoot().tree.toXML(), /*html*/ `<r><p>14</p></r>`);
+    assert.equal(doc.getRoot().tree.toXML(), /*html*/ `<r><p b="t">14</p></r>`);
     assert.equal(doc.getRoot().tree.getSize(), 4);
 
     const bytes = converter.objectToBytes(doc.getRootObject());
@@ -119,5 +124,9 @@ describe('Converter', function () {
       doc.getRoot().tree.getSize(),
       (obj.get('tree') as unknown as Tree).getSize(),
     );
+    assert.equal(
+      doc.getRoot().tree.toXML(),
+      (obj.get('tree') as unknown as Tree).toXML(),
+    );
   });
 });

test/unit/document/crdt/rht_test.ts

@@ -121,6 +121,16 @@ describe('RHT interface', function () {
     assert.equal(jsonObj.testKey2, testData.testKey2);
     assert.equal(jsonObj.testKey3, testData.testKey3);
   });
+
+  it('should deepcopy correctly', function () {
+    const rht = RHT.create();
+    rht.set('key1', 'value1', timeT());
+    rht.remove('key2', timeT());
+
+    const rht2 = rht.deepcopy();
+    assert.equal(rht.toJSON(), rht2.toJSON());
+    assert.equal(rht.size(), rht2.size());
+  });
 });
 
 describe('RHT', () => {