Add RHTNode removal to converter for consistency (#201)

yorkie-team · Jun 7, 2024 · b3ebce3 · b3ebce3
1 parent 59ca4ab
commit b3ebce3
Show file tree

Hide file tree

Showing 5 changed files with 58 additions and 19 deletions.
diff --git a/yorkie/proto/yorkie/v1/resources.proto b/yorkie/proto/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 {

diff --git a/yorkie/src/main/kotlin/dev/yorkie/api/ElementConverter.kt b/yorkie/src/main/kotlin/dev/yorkie/api/ElementConverter.kt
@@ -8,6 +8,7 @@ import dev.yorkie.api.v1.JSONElementKt.jSONObject
 import dev.yorkie.api.v1.JSONElementKt.primitive
 import dev.yorkie.api.v1.JSONElementKt.text
 import dev.yorkie.api.v1.JSONElementKt.tree
+import dev.yorkie.api.v1.NodeAttr
 import dev.yorkie.api.v1.jSONElement
 import dev.yorkie.api.v1.jSONElementSimple
 import dev.yorkie.api.v1.movedAtOrNull
@@ -44,6 +45,7 @@ import dev.yorkie.document.crdt.RgaTreeSplitNode
 import dev.yorkie.document.crdt.RgaTreeSplitNodeID
 import dev.yorkie.document.crdt.RgaTreeSplitPos
 import dev.yorkie.document.crdt.Rht
+import dev.yorkie.document.crdt.RhtNode
 import dev.yorkie.document.crdt.TextValue
 import dev.yorkie.document.time.ActorID
 import dev.yorkie.document.time.TimeTicket.Companion.InitialTimeTicket
@@ -237,11 +239,7 @@ internal fun PBTreeNode.toCrdtTreeNode(): CrdtTreeNode {
         CrdtTreeElement(
             id,
             type,
-            attributes = Rht().also {
-                attributesMap.forEach { (key, value) ->
-                    it.set(key, value.value, value.updatedAt.toTimeTicket())
-                }
-            },
+            attributes = attributesMap.toRht(),
         )
     }.apply {
         convertedRemovedAt?.let(::remove)
@@ -262,6 +260,24 @@ internal fun PBTreeNodeID.toCrdtTreeNodeID(): CrdtTreeNodeID {
     return CrdtTreeNodeID(createdAt.toTimeTicket(), offset)
 }
 
+private fun Iterable<RhtNode>.toPBRht(): Map<String, NodeAttr> {
+    return associate { node ->
+        node.key to nodeAttr {
+            value = node.value
+            updatedAt = node.executedAt.toPBTimeTicket()
+            isRemoved = node.isRemoved
+        }
+    }
+}
+
+private fun Map<String, NodeAttr>.toRht(): Rht {
+    return Rht().apply {
+        entries.forEach { (key, node) ->
+            setInternal(key, node.value, node.updatedAt.toTimeTicket(), node.isRemoved)
+        }
+    }
+}
+
 internal fun CrdtElement.toPBJsonElement(): PBJsonElement {
     return when (this) {
         is CrdtObject -> toPBJsonObject()
@@ -333,14 +349,7 @@ internal fun CrdtTreeNode.toPBTreeNodes(): List<PBTreeNode> {
                     removedAt = it
                 }
                 depth = nodeDepth
-                attributes.putAll(
-                    node.rhtNodes.associate {
-                        it.key to nodeAttr {
-                            value = it.value
-                            updatedAt = it.executedAt.toPBTimeTicket()
-                        }
-                    },
-                )
+                attributes.putAll(node.rhtNodes.toPBRht())
             }
             add(pbTreeNode)
         }

diff --git a/yorkie/src/main/kotlin/dev/yorkie/document/crdt/Rht.kt b/yorkie/src/main/kotlin/dev/yorkie/document/crdt/Rht.kt
@@ -46,6 +46,19 @@ internal class Rht : Collection<RhtNode> {
         return RhtSetResult(null, null)
     }
 
+    fun setInternal(
+        key: String,
+        value: String,
+        executedAt: TimeTicket,
+        removed: Boolean,
+    ) {
+        val node = RhtNode(key, value, executedAt, removed)
+        nodeMapByKey[key] = node
+        if (removed) {
+            numberOfRemovedElements++
+        }
+    }
+
     /**
      * Removes the Element of the given [key].
      */
@@ -84,9 +97,8 @@ internal class Rht : Collection<RhtNode> {
 
     fun deepCopy(): Rht {
         val rht = Rht()
-        nodeMapByKey.forEach {
-            val node = it.value
-            rht.set(node.key, node.value, node.executedAt, node.isRemoved)
+        nodeMapByKey.values.forEach { node ->
+            rht.setInternal(node.key, node.value, node.executedAt, node.isRemoved)
         }
         return rht
     }

diff --git a/yorkie/src/test/kotlin/dev/yorkie/api/ConverterTest.kt b/yorkie/src/test/kotlin/dev/yorkie/api/ConverterTest.kt
@@ -421,6 +421,9 @@ class ConverterTest {
     @Test
     fun `should encode and decode tree properly`() = runTest {
         val document = Document(Document.Key(""))
+
+        fun JsonObject.tree() = getAs<JsonTree>("t")
+
         document.updateAsync { root, _ ->
             root.setNewTree(
                 "t",
@@ -429,18 +432,22 @@ class ConverterTest {
                     element("p") { text { "34" } }
                 },
             ).editByPath(listOf(0, 1), listOf(1, 1))
-        }.await()
 
-        fun JsonObject.tree() = getAs<JsonTree>("t")
+            root.tree().style(0, 1, mapOf("b" to "t", "i" to "t"))
+            assertEquals("""<r><p b="t" i="t">14</p></r>""", root.tree().toXml())
+
+            root.tree().removeStyle(0, 1, listOf("i"))
+        }.await()
 
-        assertEquals("<r><p>14</p></r>", document.getRoot().tree().toXml())
+        assertEquals("""<r><p b="t">14</p></r>""", document.getRoot().tree().toXml())
         assertEquals(4, document.getRoot().tree().size)
 
         val tree = document.getRoot().target["t"]
         val bytes = tree.toByteString()
         val obj = bytes.toCrdtTree()
         assertEquals(document.getRoot().tree().target.nodeSize, obj.nodeSize)
         assertEquals(document.getRoot().tree().size, obj.size)
+        assertEquals(document.getRoot().tree().toXml(), obj.toXml())
     }
 
     private class TestOperation(

diff --git a/yorkie/src/test/kotlin/dev/yorkie/document/crdt/RhtTest.kt b/yorkie/src/test/kotlin/dev/yorkie/document/crdt/RhtTest.kt
@@ -231,6 +231,16 @@ class RhtTest {
         }
     }
 
+    @Test
+    fun `should deepcopy correctly`() {
+        target.set("key1", "value1", issueTime())
+        target.remove("key2", issueTime())
+
+        val copiedRht = target.deepCopy()
+        assertEquals(target.toJson(), copiedRht.toJson())
+        assertEquals(target.size, copiedRht.size)
+    }
+
     private fun Rht.toTestString(): String {
         return nodeKeyValueMap.entries.joinToString("") { "${it.key}:${it.value}" }
     }