From 6f76389990305c48a46d987dfd6861e66e81f7d9 Mon Sep 17 00:00:00 2001 From: Jung gyun Ahn Date: Fri, 17 May 2024 16:58:36 +0900 Subject: [PATCH] Fix invalid tree style changes (#167) * Fix invalid tree style changes * fix lint error --- Sources/Document/CRDT/CRDTTree.swift | 34 ++++++++++----- Sources/Document/CRDT/RHT.swift | 21 +++++---- Tests/Integration/TreeIntegrationTests.swift | 46 ++++++++++++++++++++ 3 files changed, 80 insertions(+), 21 deletions(-) diff --git a/Sources/Document/CRDT/CRDTTree.swift b/Sources/Document/CRDT/CRDTTree.swift index dc8e019f..f302ddd4 100644 --- a/Sources/Document/CRDT/CRDTTree.swift +++ b/Sources/Document/CRDT/CRDTTree.swift @@ -588,7 +588,6 @@ class CRDTTree: CRDTGCElement { let (toParent, toLeft) = try self.findNodesAndSplitText(range.1, editedAt) var changes: [TreeChange] = [] - let value = attributes != nil ? TreeChangeValue.attributes(attributes!) : nil var createdAtMapByActor = [String: TimeTicket]() try self.traverseInPosRange(fromParent, fromLeft, toParent, toLeft) { token, _ in @@ -606,19 +605,30 @@ class CRDTTree: CRDTGCElement { if node.attrs == nil { node.attrs = RHT() } - for (key, value) in attributes ?? [:] { - node.attrs?.set(key: key, value: value, executedAt: editedAt) + var affectedKeys = Set() + for (key, value) in attributes ?? [:] where node.attrs?.set(key: key, value: value, executedAt: editedAt) ?? false { + affectedKeys.insert(key) } - try changes.append(TreeChange(actor: editedAt.actorID, - type: .style, - from: self.toIndex(fromParent, fromLeft), - to: self.toIndex(toParent, toLeft), - fromPath: self.toPath(fromParent, fromLeft), - toPath: self.toPath(toParent, toLeft), - value: value, - splitLevel: 0) // dummy value. - ) + if !affectedKeys.isEmpty { + var affectedAttrs = [String: String]() + + for affectedKey in affectedKeys { + if let attr = attributes?[affectedKey] { + affectedAttrs[affectedKey] = attr + } + } + + try changes.append(TreeChange(actor: editedAt.actorID, + type: .style, + from: self.toIndex(fromParent, fromLeft), + to: self.toIndex(toParent, toLeft), + fromPath: self.toPath(fromParent, fromLeft), + toPath: self.toPath(toParent, toLeft), + value: TreeChangeValue.attributes(affectedAttrs), + splitLevel: 0) // dummy value. + ) + } } } diff --git a/Sources/Document/CRDT/RHT.swift b/Sources/Document/CRDT/RHT.swift index d9140a68..7fcff03f 100644 --- a/Sources/Document/CRDT/RHT.swift +++ b/Sources/Document/CRDT/RHT.swift @@ -37,19 +37,22 @@ class RHT { /** * `set` sets the value of the given key. */ - func set(key: String, value: String, executedAt: TimeTicket) { - if let prev = nodeMapByKey[key] { - if executedAt.after(prev.updatedAt) { - if !prev.isRemoved { - self.numberOfRemovedElement -= 1 - } - let node = RHTNode(key: key, value: value, updatedAt: executedAt, isRemoved: false) - self.nodeMapByKey[key] = node + @discardableResult + func set(key: String, value: String, executedAt: TimeTicket) -> Bool { + let prev = self.nodeMapByKey[key] + + if prev == nil || executedAt.after(prev!.updatedAt) { + if prev != nil && !prev!.isRemoved { + self.numberOfRemovedElement -= 1 } - } else { + let node = RHTNode(key: key, value: value, updatedAt: executedAt, isRemoved: false) self.nodeMapByKey[key] = node + + return true } + + return false } /** diff --git a/Tests/Integration/TreeIntegrationTests.swift b/Tests/Integration/TreeIntegrationTests.swift index 13a43159..5e8538db 100644 --- a/Tests/Integration/TreeIntegrationTests.swift +++ b/Tests/Integration/TreeIntegrationTests.swift @@ -3765,6 +3765,52 @@ final class TreeIntegrationTreeChangeGeneration: XCTestCase { } } + func test_concurrent_style_and_style() async throws { + try await withTwoClientsAndDocuments(self.description) { c1, d1, c2, d2 in + try await d1.update { root, _ in + root.t = JSONTree(initialRoot: + JSONTreeElementNode(type: "doc", + children: [ + JSONTreeElementNode(type: "p", + children: [ + JSONTreeTextNode(value: "hello") + ]) + ]) + ) + } + + try await c1.sync() + try await c2.sync() + + var d1XML = await(d1.getRoot().t as? JSONTree)?.toXML() + var d2XML = await(d2.getRoot().t as? JSONTree)?.toXML() + XCTAssertEqual(d1XML, d2XML) + XCTAssertEqual(d1XML, /* html */ "

hello

") + + await subscribeDocs(d1, + d2, + [TreeStyleOpInfoForDebug(from: 0, to: 1, value: ["bold": "true"], fromPath: nil), + TreeStyleOpInfoForDebug(from: 0, to: 1, value: ["bold": "false"], fromPath: nil)], + [TreeStyleOpInfoForDebug(from: 0, to: 1, value: ["bold": "false"], fromPath: nil)]) + + try await d1.update { root, _ in + try (root.t as? JSONTree)?.style(0, 1, ["bold": "true"]) + } + try await d2.update { root, _ in + try (root.t as? JSONTree)?.style(0, 1, ["bold": "false"]) + } + + try await c1.sync() + try await c2.sync() + try await c1.sync() + + d1XML = await(d1.getRoot().t as? JSONTree)?.toXML() + d2XML = await(d2.getRoot().t as? JSONTree)?.toXML() + XCTAssertEqual(d1XML, d2XML) + XCTAssertEqual(d1XML, /* html */ "

hello

") + } + } + func test_emoji() async throws { try await withTwoClientsAndDocuments(self.description) { c1, d1, c2, d2 in try await d1.update { root, _ in