From f0163c700c0db37c3c69ba759c49ca4082bcd157 Mon Sep 17 00:00:00 2001 From: Jeehyun Kim Date: Fri, 7 Jun 2024 15:37:07 +0900 Subject: [PATCH] Fix miscalculation of tree size in concurrent editing (#202) * Fix miscalculation of tree size in concurrent editing * call updateDescendantSize() always --- gradle.properties | 2 +- .../dev/yorkie/document/json/JsonTreeTest.kt | 94 +++++++++++++++++++ .../dev/yorkie/document/crdt/CrdtTree.kt | 6 +- .../main/kotlin/dev/yorkie/util/IndexTree.kt | 10 +- 4 files changed, 101 insertions(+), 11 deletions(-) diff --git a/gradle.properties b/gradle.properties index afc97624..3b2fef5c 100644 --- a/gradle.properties +++ b/gradle.properties @@ -11,7 +11,7 @@ android.suppressUnsupportedOptionWarnings=android.suppressUnsupportedOptionWarni kotlin.code.style=official kotlin.mpp.stability.nowarn=true GROUP=dev.yorkie -VERSION_NAME=0.4.21-rc +VERSION_NAME=0.4.23 POM_DESCRIPTION=Document store for building collaborative editing applications. POM_INCEPTION_YEAR=2022 POM_URL=https://github.com/yorkie-team/yorkie-android-sdk diff --git a/yorkie/src/androidTest/kotlin/dev/yorkie/document/json/JsonTreeTest.kt b/yorkie/src/androidTest/kotlin/dev/yorkie/document/json/JsonTreeTest.kt index 32cd62e1..5f6f3487 100644 --- a/yorkie/src/androidTest/kotlin/dev/yorkie/document/json/JsonTreeTest.kt +++ b/yorkie/src/androidTest/kotlin/dev/yorkie/document/json/JsonTreeTest.kt @@ -3,6 +3,8 @@ package dev.yorkie.document.json import androidx.test.ext.junit.runners.AndroidJUnit4 import com.google.gson.reflect.TypeToken import dev.yorkie.TreeTest +import dev.yorkie.api.toByteString +import dev.yorkie.api.toCrdtTree import dev.yorkie.core.Client import dev.yorkie.core.Client.SyncMode.Manual import dev.yorkie.core.GENERAL_TIMEOUT @@ -12,6 +14,7 @@ import dev.yorkie.core.withTwoClientsAndDocuments import dev.yorkie.document.Document import dev.yorkie.document.Document.Event.LocalChange import dev.yorkie.document.Document.Event.RemoteChange +import dev.yorkie.document.crdt.toXml import dev.yorkie.document.json.JsonTree.ElementNode import dev.yorkie.document.json.JsonTree.TreeNode import dev.yorkie.document.json.TreeBuilder.element @@ -2300,6 +2303,97 @@ class JsonTreeTest { } } + @Test + fun test_calculating_index_tree_size_during_concurrent_editing() { + withTwoClientsAndDocuments(syncMode = Manual) { c1, c2, d1, d2, _ -> + updateAndSync( + Updater(c1, d1) { root, _ -> + root.setNewTree( + "t", + element("doc") { + element("p") { + text { "hello" } + } + }, + ) + }, + Updater(c2, d2), + ) + assertTreesXmlEquals("

hello

", d1, d2) + + updateAndSync( + Updater(c1, d1) { root, _ -> + root.rootTree().edit(0, 7) + }, + Updater(c2, d2) { root, _ -> + root.rootTree().edit(1, 2, text { "p" }) + }, + ) { + assertTreesXmlEquals("", d1) + assertEquals(0, d1.getRoot().rootTree().size) + + assertTreesXmlEquals("

pello

", d2) + assertEquals(7, d2.getRoot().rootTree().size) + } + + assertTreesXmlEquals("", d1, d2) + assertEquals(d1.getRoot().rootTree().size, d2.getRoot().rootTree().size) + } + } + + @Test + fun test_index_tree_consistency_from_snapshot() { + withTwoClientsAndDocuments(syncMode = Manual) { c1, c2, d1, d2, _ -> + updateAndSync( + Updater(c1, d1) { root, _ -> + root.setNewTree( + "t", + element("r") { + element("p") + }, + ) + }, + Updater(c2, d2), + ) + assertTreesXmlEquals("

", d1, d2) + + updateAndSync( + Updater(c1, d1) { root, _ -> + root.rootTree().edit(0, 2) + }, + Updater(c2, d2) { root, _ -> + root.rootTree().edit(1, 1, element("i") { text { "a" } }) + root.rootTree().edit(2, 3, text { "b" }) + }, + ) { + assertTreesXmlEquals("", d1) + assertEquals(0, d1.getRoot().rootTree().size) + + assertTreesXmlEquals("

b

", d2) + assertEquals(5, d2.getRoot().rootTree().size) + } + assertTreesXmlEquals("", d1, d2) + + val d1Nodes = mutableListOf>() + val d2Nodes = mutableListOf>() + val sNodes = mutableListOf>() + + d1.getRoot().rootTree().indexTree.traverseAll { node, _ -> + d1Nodes.add(Triple(node.toXml(), node.size, node.isRemoved)) + } + d2.getRoot().rootTree().indexTree.traverseAll { node, _ -> + d2Nodes.add(Triple(node.toXml(), node.size, node.isRemoved)) + } + val sRoot = d1.getRoot().target["t"].toByteString().toCrdtTree() + sRoot.indexTree.traverseAll { node, _ -> + sNodes.add(Triple(node.toXml(), node.size, node.isRemoved)) + } + + assertEquals(d1Nodes, d2Nodes) + assertEquals(d1Nodes, sNodes) + } + } + companion object { fun JsonObject.rootTree() = getAs("t") diff --git a/yorkie/src/main/kotlin/dev/yorkie/document/crdt/CrdtTree.kt b/yorkie/src/main/kotlin/dev/yorkie/document/crdt/CrdtTree.kt index cab39a70..2b9d5401 100644 --- a/yorkie/src/main/kotlin/dev/yorkie/document/crdt/CrdtTree.kt +++ b/yorkie/src/main/kotlin/dev/yorkie/document/crdt/CrdtTree.kt @@ -862,11 +862,7 @@ internal data class CrdtTreeNode private constructor( removedAt = executedAt } if (alived) { - if (parent?.removedAt == null) { - updateAncestorSize() - } else { - requireNotNull(parent).size -= paddedSize - } + updateAncestorSize() } } diff --git a/yorkie/src/main/kotlin/dev/yorkie/util/IndexTree.kt b/yorkie/src/main/kotlin/dev/yorkie/util/IndexTree.kt index 95b509d8..9d846aff 100644 --- a/yorkie/src/main/kotlin/dev/yorkie/util/IndexTree.kt +++ b/yorkie/src/main/kotlin/dev/yorkie/util/IndexTree.kt @@ -444,6 +444,9 @@ internal abstract class IndexTreeNode> { while (parent != null) { parent.size += paddedSize * sign + if (parent.isRemoved) { + break + } parent = parent.parent } } @@ -453,12 +456,9 @@ internal abstract class IndexTreeNode> { * the tree is newly created and the size of the descendants is not calculated. */ fun updateDescendantSize(): Int { - if (isRemoved) { - size = 0 - return 0 + size += children.sumOf { node -> + node.updateDescendantSize().takeUnless { node.isRemoved } ?: 0 } - - size += children.sumOf { it.updateDescendantSize() } return paddedSize }