test concurrent deletions (#166)

* test concurrent deletions * handle TreeEditOperation only if it has valid TreePosRange (#168) --------- Co-authored-by: Jeehyun Kim <[email protected]>
yorkie-team · Apr 15, 2024 · d626db5 · d626db5
1 parent 95a2fee
commit d626db5
Show file tree

Hide file tree

Showing 3 changed files with 159 additions and 1 deletion.
diff --git a/yorkie/src/androidTest/kotlin/dev/yorkie/core/ClientTest.kt b/yorkie/src/androidTest/kotlin/dev/yorkie/core/ClientTest.kt
@@ -15,6 +15,7 @@ import dev.yorkie.document.Document.Event.LocalChange
 import dev.yorkie.document.Document.Event.RemoteChange
 import dev.yorkie.document.json.JsonCounter
 import dev.yorkie.document.json.JsonPrimitive
+import dev.yorkie.document.json.JsonTree
 import dev.yorkie.document.json.JsonTreeTest.Companion.assertTreesXmlEquals
 import dev.yorkie.document.json.JsonTreeTest.Companion.rootTree
 import dev.yorkie.document.json.TreeBuilder.element
@@ -23,6 +24,7 @@ import dev.yorkie.document.operation.OperationInfo
 import java.util.UUID
 import kotlin.test.assertContentEquals
 import kotlin.test.assertEquals
+import kotlin.test.assertFalse
 import kotlin.test.assertIs
 import kotlin.test.assertNotEquals
 import kotlin.test.assertTrue
@@ -32,9 +34,11 @@ import kotlinx.coroutines.delay
 import kotlinx.coroutines.flow.filterIsInstance
 import kotlinx.coroutines.flow.filterNot
 import kotlinx.coroutines.flow.first
+import kotlinx.coroutines.joinAll
 import kotlinx.coroutines.launch
 import kotlinx.coroutines.runBlocking
 import kotlinx.coroutines.withTimeout
+import kotlinx.coroutines.withTimeoutOrNull
 import org.junit.Test
 import org.junit.runner.RunWith
 
@@ -469,7 +473,10 @@ class ClientTest {
 
             // Simulate the situation in the runSyncLoop where a pushpull request has been sent
             // but a response has not yet been received.
-            c2.syncAsync().await()
+            d2Events.clear()
+            val deferred = c2.syncAsync()
+            c2.changeSyncMode(d2, RealtimePushOnly)
+            deferred.await()
 
             // In push-only mode, remote-change events should not occur.
             d2Events.clear()
@@ -496,6 +503,147 @@ class ClientTest {
         }
     }
 
+    @Test
+    fun test_concurrent_deletions() {
+        withTwoClientsAndDocuments { c1, c2, d1, d2, _ ->
+            repeat(10) { repeat ->
+                d1.updateAsync { root, _ ->
+                    root.setNewTree(
+                        "t",
+                        element("doc") {
+                            repeat(100) {
+                                text { "1" }
+                            }
+                        },
+                    )
+                }.await()
+
+                while (d1.toJson() != d2.toJson()) {
+                    delay(100)
+                }
+
+                listOf(
+                    launch {
+                        c1.changeSyncMode(d1, RealtimePushOnly)
+                        d1.updateAsync { root, _ ->
+                            val tree = root.getAs<JsonTree>("t")
+                            val size = (tree.rootTreeNode as JsonTree.ElementNode).children.size
+                            if (size > 99) {
+                                tree.editByPath(
+                                    listOf(99),
+                                    listOf(100),
+                                )
+                            }
+                        }.await()
+                        c1.changeSyncMode(d1, Realtime)
+                        delay(10)
+
+                        c1.changeSyncMode(d1, RealtimePushOnly)
+                        d1.updateAsync { root, _ ->
+                            val tree = root.getAs<JsonTree>("t")
+                            val size = (tree.rootTreeNode as JsonTree.ElementNode).children.size
+                            if (size > 31) {
+                                tree.editByPath(
+                                    listOf(30),
+                                    listOf(99.coerceAtMost(size)),
+                                )
+                            }
+                        }.await()
+                        c1.changeSyncMode(d1, Realtime)
+                        delay(10)
+
+                        c1.changeSyncMode(d1, RealtimePushOnly)
+                        d1.updateAsync { root, _ ->
+                            val tree = root.getAs<JsonTree>("t")
+                            val size = (tree.rootTreeNode as JsonTree.ElementNode).children.size
+                            if (size > 0) {
+                                tree.editByPath(
+                                    listOf(0),
+                                    listOf(30.coerceAtMost(size)),
+                                )
+                            }
+                        }.await()
+                        c1.changeSyncMode(d1, Realtime)
+                    },
+                    launch {
+                        repeat(100) {
+                            c2.changeSyncMode(d2, RealtimePushOnly)
+                            d2.updateAsync { root, _ ->
+                                val tree = root.getAs<JsonTree>("t")
+                                val size = (tree.rootTreeNode as JsonTree.ElementNode).children.size
+                                if (size > 0) {
+                                    tree.editByPath(
+                                        listOf((100 - it - 1).coerceIn(0 until size)),
+                                        listOf((100 - it).coerceIn(1..size)),
+                                    )
+                                }
+                            }.await()
+                            c2.changeSyncMode(d2, Realtime)
+                            delay(10)
+                        }
+                    },
+                ).joinAll()
+
+                suspend fun checkEmpty(document: Document): Boolean {
+                    return (
+                        document.getRoot()
+                            .getAs<JsonTree>("t").rootTreeNode as JsonTree.ElementNode
+                        ).children.isEmpty()
+                }
+
+                withTimeoutOrNull(15_000) {
+                    while (!checkEmpty(d1) || !checkEmpty(d2)) {
+                        delay(100)
+                    }
+                } ?: run {
+                    error(
+                        "empty check failed on ${repeat + 1}th test\n" +
+                            "d1: ${d1.toJson()}\nd2: ${d2.toJson()}",
+                    )
+                }
+
+                assertTrue(checkEmpty(d1))
+                assertTrue(checkEmpty(d2))
+
+                listOf(
+                    launch {
+                        d1.updateAsync { root, _ ->
+                            val tree = root.getAs<JsonTree>("t")
+                            tree.editByPath(
+                                listOf(0),
+                                listOf(0),
+                                text { "0" },
+                            )
+                        }.await()
+                    },
+                    launch {
+                        d2.updateAsync { root, _ ->
+                            val tree = root.getAs<JsonTree>("t")
+                            tree.editByPath(
+                                listOf(0),
+                                listOf(0),
+                                text { "2" },
+                            )
+                        }.await()
+                    },
+                ).joinAll()
+
+                withTimeoutOrNull(15_000) {
+                    while (d1.toJson() != d2.toJson()) {
+                        delay(100)
+                    }
+                } ?: run {
+                    error("failed on ${repeat + 1}th test\nd1: ${d1.toJson()}\nd2: ${d2.toJson()}")
+                }
+
+                assertFalse(checkEmpty(d1))
+                assertFalse(checkEmpty(d2))
+
+                assertEquals(d1.toJson(), d2.toJson())
+            }
+        }
+    }
+
     @Test
     fun test_not_include_changes_from_push_only_after_switching_to_realtime() {
         runBlocking {

diff --git a/yorkie/src/main/kotlin/dev/yorkie/document/crdt/CrdtTree.kt b/yorkie/src/main/kotlin/dev/yorkie/document/crdt/CrdtTree.kt
@@ -460,6 +460,12 @@ internal class CrdtTree(
         return value.takeIf { key.createdAt == id.createdAt }
     }
 
+    fun checkPosRangeValid(posRange: TreePosRange): Boolean {
+        return listOf(posRange.first, posRange.second).all {
+            findFloorNode(it.parentID) != null && findFloorNode(it.leftSiblingID) != null
+        }
+    }
+
     /**
      * Move the given [source] range to the given [target] range.
      */

diff --git a/yorkie/src/main/kotlin/dev/yorkie/document/operation/TreeEditOperation.kt b/yorkie/src/main/kotlin/dev/yorkie/document/operation/TreeEditOperation.kt
@@ -33,6 +33,10 @@ internal data class TreeEditOperation(
             YorkieLogger.e(TAG, "fail to execute, only Tree can execute edit")
             return emptyList()
         }
+        if (!tree.checkPosRangeValid(fromPos to toPos)) {
+            YorkieLogger.e(TAG, "has invalid pos range, skip executing the operation")
+            return emptyList()
+        }
 
         val editedAt = executedAt
         val changes =