From 6b94ba2af3fc05c24af9361d45c87d8c59db9425 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=E1=84=80=E1=85=B5=E1=86=B7=E1=84=92=E1=85=AD=E1=84=8B?=
 =?UTF-8?q?=E1=85=AE=5BSE=5D=5BSmartEditor=5D?= <hyowoo.kim@navercorp.com>
Date: Thu, 25 Jan 2024 15:21:11 +0900
Subject: [PATCH] Fix invalid TreeChanges in concurrent Tree.Style

---
 .../dev/yorkie/document/json/JsonTreeTest.kt  | 15 +------
 .../dev/yorkie/document/crdt/CrdtTree.kt      | 45 +++++++++----------
 2 files changed, 24 insertions(+), 36 deletions(-)

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 4f3bbdde2..f43e42662 100644
--- a/yorkie/src/androidTest/kotlin/dev/yorkie/document/json/JsonTreeTest.kt
+++ b/yorkie/src/androidTest/kotlin/dev/yorkie/document/json/JsonTreeTest.kt
@@ -17,7 +17,6 @@ import dev.yorkie.document.json.TreeBuilder.text
 import dev.yorkie.document.operation.OperationInfo
 import dev.yorkie.document.operation.OperationInfo.SetOpInfo
 import dev.yorkie.document.operation.OperationInfo.TreeEditOpInfo
-import dev.yorkie.document.operation.OperationInfo.TreeStyleOpInfo
 import kotlin.test.assertEquals
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.CoroutineStart
@@ -1721,10 +1720,7 @@ class JsonTreeTest {
                         "$.t",
                     ),
                     // client1 changed attributes on path [0]
-                    /* FIXME: TreeStyleOpInfo here should have not been emitted, as path [0] is already deleted.
-                     This causes users who depend on OperationInfo event streams to not be consistent with the actual document.
-                     And the resulting document would be as below:
-
+                    /* assert style changes on already deleted path is not applied
                      {
                         "t": {
                             "type": "root",
@@ -1734,20 +1730,13 @@ class JsonTreeTest {
                                     "children": [],
                                     "attributes": {
                                         "id": "2",
-                                        "value": "changed"
+                                        "value": "init"
                                     }
                                 }
                             ]
                         }
                      }
                      */
-                    TreeStyleOpInfo(
-                        0,
-                        0,
-                        listOf(0),
-                        mapOf("value" to "changed"),
-                        "$.t",
-                    ),
                 ),
                 document2Ops,
             )
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 366cc266e..fbc400ed2 100644
--- a/yorkie/src/main/kotlin/dev/yorkie/document/crdt/CrdtTree.kt
+++ b/yorkie/src/main/kotlin/dev/yorkie/document/crdt/CrdtTree.kt
@@ -58,32 +58,31 @@ internal class CrdtTree(
     ): List<TreeChange> {
         val (fromParent, fromLeft) = findNodesAndSplitText(range.first, executedAt)
         val (toParent, toLeft) = findNodesAndSplitText(range.second, executedAt)
-        val changes = listOf(
-            TreeChange(
-                type = TreeChangeType.Style,
-                from = toIndex(fromParent, fromLeft),
-                to = toIndex(toParent, toLeft),
-                fromPath = toPath(fromParent, fromLeft),
-                toPath = toPath(toParent, toLeft),
-                actorID = executedAt.actorID,
-                attributes = attributes,
-            ),
-        )
-
-        traverseInPosRange(
-            fromParent = fromParent,
-            fromLeft = fromLeft,
-            toParent = toParent,
-            toLeft = toLeft,
-        ) { (node, _), _ ->
-            if (!node.isRemoved && attributes != null && !node.isText) {
-                attributes.forEach { (key, value) ->
-                    node.setAttribute(key, value, executedAt)
+        return buildList {
+            traverseInPosRange(
+                fromParent = fromParent,
+                fromLeft = fromLeft,
+                toParent = toParent,
+                toLeft = toLeft,
+            ) { (node, _), _ ->
+                if (!node.isRemoved && attributes != null && !node.isText) {
+                    attributes.forEach { (key, value) ->
+                        node.setAttribute(key, value, executedAt)
+                    }
+                    add(
+                        TreeChange(
+                            type = TreeChangeType.Style,
+                            from = toIndex(fromParent, fromLeft),
+                            to = toIndex(toParent, toLeft),
+                            fromPath = toPath(fromParent, fromLeft),
+                            toPath = toPath(toParent, toLeft),
+                            actorID = executedAt.actorID,
+                            attributes = attributes,
+                        ),
+                    )
                 }
             }
         }
-
-        return changes
     }
 
     private fun toPath(parentNode: CrdtTreeNode, leftSiblingNode: CrdtTreeNode): List<Int> {