From 7dba6db5b125eb5bc08c5b66444359db98f2aaf1 Mon Sep 17 00:00:00 2001
From: Hackerwins <susukang98@gmail.com>
Date: Thu, 9 May 2024 14:27:52 +0900
Subject: [PATCH] Fix invalid tree style changes

---
 src/document/crdt/rht.ts      |  5 +++-
 src/document/crdt/tree.ts     | 37 +++++++++++++++++++--------
 test/integration/tree_test.ts | 47 +++++++++++++++++++++++++++++++++++
 3 files changed, 77 insertions(+), 12 deletions(-)

diff --git a/src/document/crdt/rht.ts b/src/document/crdt/rht.ts
index b06a88ce7..195b38049 100644
--- a/src/document/crdt/rht.ts
+++ b/src/document/crdt/rht.ts
@@ -102,7 +102,7 @@ export class RHT {
   /**
    * `set` sets the value of the given key.
    */
-  public set(key: string, value: string, executedAt: TimeTicket): void {
+  public set(key: string, value: string, executedAt: TimeTicket): boolean {
     const prev = this.nodeMapByKey.get(key);
 
     if (prev === undefined || executedAt.after(prev.getUpdatedAt())) {
@@ -111,7 +111,10 @@ export class RHT {
       }
       const node = RHTNode.of(key, value, executedAt, false);
       this.nodeMapByKey.set(key, node);
+      return true;
     }
+
+    return false;
   }
 
   /**
diff --git a/src/document/crdt/tree.ts b/src/document/crdt/tree.ts
index df2547aa8..de1e444c8 100644
--- a/src/document/crdt/tree.ts
+++ b/src/document/crdt/tree.ts
@@ -766,7 +766,9 @@ export class CRDTTree extends CRDTGCElement {
     const [toParent, toLeft] = this.findNodesAndSplitText(range[1], editedAt);
 
     const changes: Array<TreeChange> = [];
-    const value = attributes ? parseObjectValues(attributes) : undefined;
+    const attrs: { [key: string]: any } = attributes
+      ? parseObjectValues(attributes)
+      : {};
     const createdAtMapByActor = new Map<string, TimeTicket>();
     this.traverseInPosRange(
       fromParent,
@@ -795,19 +797,32 @@ export class CRDTTree extends CRDTGCElement {
             node.attrs = new RHT();
           }
 
+          const affectedKeys = new Set<string>();
           for (const [key, value] of Object.entries(attributes)) {
-            node.attrs.set(key, value, editedAt);
+            if (node.attrs.set(key, value, editedAt)) {
+              affectedKeys.add(key);
+            }
           }
 
-          changes.push({
-            type: TreeChangeType.Style,
-            from: this.toIndex(fromParent, fromLeft),
-            to: this.toIndex(toParent, toLeft),
-            fromPath: this.toPath(fromParent, fromLeft),
-            toPath: this.toPath(toParent, toLeft),
-            actor: editedAt.getActorID(),
-            value,
-          });
+          if (affectedKeys.size > 0) {
+            const affectedAttrs = Array.from(affectedKeys).reduce(
+              (acc: { [key: string]: any }, key) => {
+                acc[key] = attrs[key];
+                return acc;
+              },
+              {},
+            );
+
+            changes.push({
+              type: TreeChangeType.Style,
+              from: this.toIndex(fromParent, fromLeft),
+              to: this.toIndex(toParent, toLeft),
+              fromPath: this.toPath(fromParent, fromLeft),
+              toPath: this.toPath(toParent, toLeft),
+              actor: editedAt.getActorID(),
+              value: affectedAttrs,
+            });
+          }
         }
       },
     );
diff --git a/test/integration/tree_test.ts b/test/integration/tree_test.ts
index f9fe1956f..7ab31df3b 100644
--- a/test/integration/tree_test.ts
+++ b/test/integration/tree_test.ts
@@ -4632,6 +4632,53 @@ describe('TreeChange', () => {
       );
     }, task.name);
   });
+
+  it('Concurrent style and style', async function ({ task }) {
+    await withTwoClientsAndDocuments<{ t: Tree }>(async (c1, d1, c2, d2) => {
+      d1.update((root) => {
+        root.t = new Tree({
+          type: 'doc',
+          children: [
+            { type: 'p', children: [{ type: 'text', value: 'hello' }] },
+          ],
+        });
+      });
+      await c1.sync();
+      await c2.sync();
+      assert.equal(d1.getRoot().t.toXML(), d2.getRoot().t.toXML());
+      assert.equal(d1.getRoot().t.toXML(), /*html*/ `<doc><p>hello</p></doc>`);
+
+      const [ops1, ops2] = subscribeDocs(d1, d2);
+
+      d1.update((r) => r.t.style(0, 1, { bold: 'true' }));
+      d2.update((r) => r.t.style(0, 1, { bold: 'false' }));
+      await c1.sync();
+      await c2.sync();
+      await c1.sync();
+      assert.equal(d1.getRoot().t.toXML(), d2.getRoot().t.toXML());
+      assert.equal(
+        d1.getRoot().t.toXML(),
+        /*html*/ `<doc><p bold="false">hello</p></doc>`,
+      );
+
+      assert.deepEqual(
+        ops1.map((it) => {
+          return { type: it.type, from: it.from, to: it.to, value: it.value };
+        }),
+        [
+          { type: 'tree-style', from: 0, to: 1, value: { bold: 'true' } },
+          { type: 'tree-style', from: 0, to: 1, value: { bold: 'false' } },
+        ],
+      );
+
+      assert.deepEqual(
+        ops2.map((it) => {
+          return { type: it.type, from: it.from, to: it.to, value: it.value };
+        }),
+        [{ type: 'tree-style', from: 0, to: 1, value: { bold: 'false' } }],
+      );
+    }, task.name);
+  });
 });
 
 function subscribeDocs(