Skip to content

Commit

Permalink
Fix incorrect calculation in indexTree.treePosToPath operation (#158)
Browse files Browse the repository at this point in the history
  • Loading branch information
@7hong13
7hong13 authored Feb 23, 2024
1 parent 0f64efb commit 79f4573
Show file tree
Hide file tree
Showing 3 changed files with 130 additions and 13 deletions.
2 changes: 1 addition & 1 deletion gradle.properties
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ android.suppressUnsupportedOptionWarnings=android.suppressUnsupportedOptionWarni
kotlin.code.style=official
kotlin.mpp.stability.nowarn=true
GROUP=dev.yorkie
VERSION_NAME=0.4.15-rc
VERSION_NAME=0.4.15-rc2
POM_DESCRIPTION=Document store for building collaborative editing applications.
POM_INCEPTION_YEAR=2022
POM_URL=https://github.com/yorkie-team/yorkie-android-sdk
Expand Down
105 changes: 105 additions & 0 deletions yorkie/src/androidTest/kotlin/dev/yorkie/document/json/JsonTreeTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,11 @@ import kotlinx.coroutines.awaitAll
import kotlinx.coroutines.delay
import kotlinx.coroutines.flow.asFlow
import kotlinx.coroutines.flow.emptyFlow
import kotlinx.coroutines.flow.filter
import kotlinx.coroutines.flow.filterIsInstance
import kotlinx.coroutines.flow.flatMapConcat
import kotlinx.coroutines.flow.map
import kotlinx.coroutines.flow.onEach
import kotlinx.coroutines.launch
import kotlinx.coroutines.withTimeout
import org.junit.Test
Expand Down Expand Up @@ -1805,6 +1807,109 @@ class JsonTreeTest {
}
}

@Test
fun test_returning_correct_range_path() {
withTwoClientsAndDocuments(realTimeSync = false) { c1, c2, d1, d2, _ ->
updateAndSync(
Updater(c1, d1) { root, _ ->
root.setNewTree(
"t",
element("r") {
element("c") {
element("u") {
element("p") {
element("n")
}
}
}
element("c") {
element("p") {
element("n")
}
}
},
)
},
Updater(c2, d1),
)
var expectedXml = "<r><c><u><p><n></n></p></u></c><c><p><n></n></p></c></r>"
assertTreesXmlEquals(expectedXml, d1)
assertTreesXmlEquals(expectedXml, d2)

updateAndSync(
Updater(c1, d1),
Updater(c2, d2) { root, _ ->
root.rootTree().editByPath(listOf(1, 0, 0, 0), listOf(1, 0, 0, 0), text { "1" })
root.rootTree().editByPath(listOf(1, 0, 0, 1), listOf(1, 0, 0, 1), text { "2" })
root.rootTree().editByPath(listOf(1, 0, 0, 2), listOf(1, 0, 0, 2), text { "3" })
},
)
expectedXml = "<r><c><u><p><n></n></p></u></c><c><p><n>123</n></p></c></r>"
assertTreesXmlEquals(expectedXml, d1)
assertTreesXmlEquals(expectedXml, d2)

updateAndSync(
Updater(c1, d1) { root, _ ->
root.rootTree()
.editByPath(listOf(1, 0, 0, 1), listOf(1, 0, 0, 1), text { "abcdefgh" })
},
Updater(c2, d1),
)
expectedXml = "<r><c><u><p><n></n></p></u></c><c><p><n>1abcdefgh23</n></p></c></r>"
assertTreesXmlEquals(expectedXml, d1)
assertTreesXmlEquals(expectedXml, d2)

updateAndSync(
Updater(c1, d1),
Updater(c2, d2) { root, _ ->
root.rootTree().editByPath(listOf(1, 0, 0, 5), listOf(1, 0, 0, 5), text { "4" })
root.rootTree().editByPath(listOf(1, 0, 0, 6), listOf(1, 0, 0, 7))
root.rootTree().editByPath(listOf(1, 0, 0, 6), listOf(1, 0, 0, 6), text { "5" })
},
)

val d2Events = mutableListOf<Document.Event>()

fun handleOpInfo(operation: TreeEditOpInfo) {
val (_, _, fromPath, toPath) = operation
assertEquals(listOf(1, 0, 0, 7), fromPath)
assertEquals(listOf(1, 0, 0, 8), toPath)
}

val collectJob = launch(start = CoroutineStart.UNDISPATCHED) {
d2.events
.filter { it is LocalChange || it is RemoteChange }
.onEach { event ->
when (event) {
is LocalChange -> {
(event.changeInfo.operations.firstOrNull() as? TreeEditOpInfo)
?.let(::handleOpInfo)
}

else -> {
val operations = (event as RemoteChange).changeInfo.operations
(operations.firstOrNull() as? TreeEditOpInfo)?.let(::handleOpInfo)
}
}
}.collect(d2Events::add)
}

updateAndSync(
Updater(c1, d1),
Updater(c2, d2) { root, _ ->
root.rootTree().editByPath(listOf(1, 0, 0, 7), listOf(1, 0, 0, 8))
},
)
expectedXml = "<r><c><u><p><n></n></p></u></c><c><p><n>1abcd45gh23</n></p></c></r>"
assertTreesXmlEquals(expectedXml, d1)
assertTreesXmlEquals(expectedXml, d2)

assertIs<LocalChange>(d2Events.firstOrNull())

collectJob.cancel()
}
}

companion object {

fun JsonObject.rootTree() = getAs<JsonTree>("t")
Expand Down
36 changes: 24 additions & 12 deletions yorkie/src/main/kotlin/dev/yorkie/util/IndexTree.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -203,18 +203,27 @@ internal class IndexTree<T : IndexTreeNode<T>>(val root: T) {
fun treePosToPath(treePos: TreePos<T>): List<Int> {
var node = treePos.node

val path = if (node.isText) {
val parent = node.parent
val offset = parent?.findOffset(node) ?: return emptyList()
if (offset == -1) {
throw IllegalArgumentException("invalid treePos: $treePos")
}
val path = when {
node.isText -> {
val parent = node.parent
val offset = parent?.findOffset(node) ?: return emptyList()
if (offset == -1) {
throw IllegalArgumentException("invalid treePos: $treePos")
}

val sizeOfLeftSiblings = addSizeOfLeftSiblings(parent, offset)
node = parent
mutableListOf(sizeOfLeftSiblings + treePos.offset)
} else {
mutableListOf(treePos.offset)
val sizeOfLeftSiblings = addSizeOfLeftSiblings(parent, offset)
node = parent
mutableListOf(sizeOfLeftSiblings + treePos.offset)
}
node.hasTextChild -> {
// TODO(hackerwins): The function does not consider the situation
// where Element and Text nodes are mixed in the Element's Children.
val sizeOfLeftSiblings = addSizeOfLeftSiblings(node, treePos.offset)
mutableListOf(sizeOfLeftSiblings)
}
else -> {
mutableListOf(treePos.offset)
}
}

while (node.parent != null) {
Expand Down Expand Up @@ -404,8 +413,11 @@ internal abstract class IndexTreeNode<T : IndexTreeNode<T>>(children: MutableLis
val allChildren: List<T>
get() = childrenInternal.toList()

/**
* Returns true if the node's children consist of only text children.
*/
val hasTextChild: Boolean
get() = children.any { it.isText }
get() = children.isNotEmpty() && children.all { it.isText }

init {
if (isText && childrenInternal.isNotEmpty()) {
Expand Down

0 comments on commit 79f4573

Please sign in to comment.