From 1fb8aa52cb8a71e2f399ce1d8fe0c450b596780e Mon Sep 17 00:00:00 2001 From: jee-hyun-kim Date: Wed, 5 Jun 2024 20:48:16 +0900 Subject: [PATCH] Add RHTNode removal to converter for consistency --- yorkie/proto/yorkie/v1/resources.proto | 1 + .../kotlin/dev/yorkie/api/ElementConverter.kt | 35 ++++++++++++------- .../kotlin/dev/yorkie/document/crdt/Rht.kt | 18 ++++++++-- .../kotlin/dev/yorkie/api/ConverterTest.kt | 13 +++++-- .../dev/yorkie/document/crdt/RhtTest.kt | 10 ++++++ 5 files changed, 58 insertions(+), 19 deletions(-) diff --git a/yorkie/proto/yorkie/v1/resources.proto b/yorkie/proto/yorkie/v1/resources.proto index 50a63b72d..d1dc92d86 100644 --- 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 index 5ae315e44..b7ba77032 100644 --- 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.toPBRht(): Map { + return associate { node -> + node.key to nodeAttr { + value = node.value + updatedAt = node.executedAt.toPBTimeTicket() + isRemoved = node.isRemoved + } + } +} + +private fun Map.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 { 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 index 5ac430552..52dc54199 100644 --- 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 { 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 { 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 index b5df75b5d..2881dfdbb 100644 --- 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("t") + document.updateAsync { root, _ -> root.setNewTree( "t", @@ -429,11 +432,14 @@ class ConverterTest { element("p") { text { "34" } } }, ).editByPath(listOf(0, 1), listOf(1, 1)) - }.await() - fun JsonObject.tree() = getAs("t") + root.tree().style(0, 1, mapOf("b" to "t", "i" to "t")) + assertEquals("""

14

""", root.tree().toXml()) + + root.tree().removeStyle(0, 1, listOf("i")) + }.await() - assertEquals("

14

", document.getRoot().tree().toXml()) + assertEquals("""

14

""", document.getRoot().tree().toXml()) assertEquals(4, document.getRoot().tree().size) val tree = document.getRoot().target["t"] @@ -441,6 +447,7 @@ class ConverterTest { 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 index a678250da..4cbffe0e6 100644 --- 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}" } }