Prevent remote-change events from occurring in push-only mode (#161)

yorkie-team · Mar 12, 2024 · 9d270ac · 9d270ac
1 parent 2f1b4da
commit 9d270ac
Show file tree

Hide file tree

Showing 4 changed files with 79 additions and 3 deletions.
diff --git 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.15-rc2
+VERSION_NAME=0.4.15
 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/core/ClientTest.kt b/yorkie/src/androidTest/kotlin/dev/yorkie/core/ClientTest.kt
@@ -12,6 +12,10 @@ import dev.yorkie.document.Document.Event.RemoteChange
 import dev.yorkie.document.change.CheckPoint
 import dev.yorkie.document.json.JsonCounter
 import dev.yorkie.document.json.JsonPrimitive
+import dev.yorkie.document.json.JsonTreeTest.Companion.assertTreesXmlEquals
+import dev.yorkie.document.json.JsonTreeTest.Companion.rootTree
+import dev.yorkie.document.json.TreeBuilder.element
+import dev.yorkie.document.json.TreeBuilder.text
 import dev.yorkie.document.operation.OperationInfo
 import java.util.UUID
 import kotlin.test.assertContentEquals
@@ -526,4 +530,72 @@ class ClientTest {
             client.close()
         }
     }
+
+    @Test
+    fun test_prevent_remote_change_in_push_only_mode() {
+        withTwoClientsAndDocuments { c1, c2, d1, d2, _ ->
+            val d1Events = mutableListOf<Document.Event>()
+            val d2Events = mutableListOf<Document.Event>()
+            val collectJobs = listOf(
+                launch(start = CoroutineStart.UNDISPATCHED) {
+                    d1.events.filterNot { it is Document.Event.PresenceChange }
+                        .collect(d1Events::add)
+                },
+                launch(start = CoroutineStart.UNDISPATCHED) {
+                    d2.events.filterNot { it is Document.Event.PresenceChange }
+                        .collect(d2Events::add)
+                },
+            )
+
+            d1.updateAsync { root, _ ->
+                root.setNewTree(
+                    "t",
+                    element("doc") {
+                        element("p") { text { "12" } }
+                        element("p") { text { "34" } }
+                    },
+                )
+            }.await()
+
+            withTimeout(GENERAL_TIMEOUT) {
+                while (d2Events.isEmpty()) {
+                    delay(50)
+                }
+            }
+            assertIs<RemoteChange>(d2Events.first())
+            assertTreesXmlEquals("<doc><p>12</p><p>34</p></doc>", d1, d2)
+
+            d1.updateAsync { root, _ ->
+                root.rootTree().edit(2, 2, text { "a" })
+            }.await()
+            c1.syncAsync().await()
+
+            // Simulate the situation in the runSyncLoop where a pushpull request has been sent
+            // but a response has not yet been received.
+            c2.syncAsync().await()
+
+            // In push-only mode, remote-change events should not occur.
+            d2Events.clear()
+            c2.pauseRemoteChanges(d2)
+
+            delay(100) // Keep the push-only state.
+            assertTrue(d2Events.none { it is RemoteChange })
+
+            c2.resumeRemoteChanges(d2)
+
+            d2.updateAsync { root, _ ->
+                root.rootTree().edit(2, 2, text { "b" })
+            }.await()
+
+            withTimeout(GENERAL_TIMEOUT) {
+                while (d1Events.size < 3) {
+                    delay(50)
+                }
+            }
+            assertIs<RemoteChange>(d1Events.last())
+            assertTreesXmlEquals("<doc><p>1ba2</p><p>34</p></doc>", d1)
+
+            collectJobs.forEach(Job::cancel)
+        }
+    }
 }
diff --git a/yorkie/src/main/kotlin/dev/yorkie/core/Client.kt b/yorkie/src/main/kotlin/dev/yorkie/core/Client.kt
@@ -222,11 +222,15 @@ public class Client @VisibleForTesting internal constructor(
                         val responsePack = response.changePack.toChangePack()
                         // NOTE(7hong13, chacha912, hackerwins): If syncLoop already executed with
                         // PushPull, ignore the response when the syncMode is PushOnly.
-                        if (responsePack.hasChanges && syncMode == SyncMode.PushOnly) {
+                        if (responsePack.hasChanges &&
+                            attachments.value[document.key]?.syncMode == SyncMode.PushOnly
+                        ) {
                             return@runCatching
                         }
 
                         document.applyChangePack(responsePack)
+                        // NOTE(chacha912): If a document has been removed, watchStream should
+                        // be disconnected to not receive an event for that document.
                         if (document.status == DocumentStatus.Removed) {
                             attachments.value -= document.key
                         }

diff --git a/yorkie/src/main/kotlin/dev/yorkie/document/Document.kt b/yorkie/src/main/kotlin/dev/yorkie/document/Document.kt
@@ -309,7 +309,7 @@ public class Document(
             }
 
             val (opInfos, newPresences) = change.execute(root, _presences.value)
-            if (change.hasOperations) {
+            if (opInfos.isNotEmpty()) {
                 eventStream.emit(Event.RemoteChange(change.toChangeInfo(opInfos)))
             }