Fix incorrect indexes in TreeChange (#181)

* Fix incorrect indexes in TreeChange

* Update swift-integration.yml

* Update swift-integration.yml

* Update swift-integration.yml

* Change TreeStyleOpValue to optional

.github/workflows/swift-integration.yml

@@ -14,8 +14,10 @@ jobs:
       with:
         xcode-version: latest-stable
     - uses: actions/checkout@v4
+    - name: Update Brew
+      run: brew update
     - name: Setup yorkie server
-      run: brew install yorkie
+      run: brew reinstall yorkie
     - name: Run yorkie server
       run: yorkie server & 
     - name: Run tests

Sources/API/Converter.swift

@@ -1005,10 +1005,10 @@ extension Converter {
     static func fromTreeNode(_ pbTreeNode: PbTreeNode) -> CRDTTreeNode {
         let id = fromTreeNodeID(pbTreeNode.id)
         let node = CRDTTreeNode(id: id, type: pbTreeNode.type)
-
+        
         if node.isText {
             node.value = pbTreeNode.value as NSString
-        } else {
+        } else if !pbTreeNode.attributes.isEmpty {
             node.attrs = RHT()
 
             pbTreeNode.attributes.forEach { key, value in

Sources/Document/CRDT/CRDTTree.swift

@@ -98,12 +98,12 @@ extension CRDTTreePos {
     }
 
     /**
-     * `toTreeNodes` converts the pos to parent and left sibling nodes.
+     * `toTreeNodePair` converts the pos to parent and left sibling nodes.
      * If the position points to the middle of a node, then the left sibling node
      * is the node that contains the position. Otherwise, the left sibling node is
      * the node that is located at the left of the position.
      */
-    func toTreeNodes(tree: CRDTTree) throws -> (CRDTTreeNode, CRDTTreeNode) {
+    func toTreeNodePair(tree: CRDTTree) throws -> TreeNodePair {
         let parentID = self.parentID
         let leftSiblingID = self.leftSiblingID
         let parentNode = tree.findFloorNode(parentID)
@@ -232,6 +232,12 @@ public struct CRDTTreeNodeIDStruct: Codable {
  */
 typealias TreePosRange = (CRDTTreePos, CRDTTreePos)
 
+/**
+ * `TreeNodePair` represents a pair of CRDTTreeNode. It represents the position
+ * of the node in the tree with the left and parent nodes.
+ */
+typealias TreeNodePair = (CRDTTreeNode, CRDTTreeNode)
+
 /**
  * `TreePosStructRange` represents a pair of CRDTTreePosStruct.
  */
@@ -597,9 +603,9 @@ class CRDTTree: CRDTElement {
      * This is different from `TreePos` which is a position of the tree in the
      * physical perspective.
      */
-    func findNodesAndSplitText(_ pos: CRDTTreePos, _ editedAt: TimeTicket? = nil) throws -> (CRDTTreeNode, CRDTTreeNode) {
+    func findNodesAndSplitText(_ pos: CRDTTreePos, _ editedAt: TimeTicket? = nil) throws -> TreeNodePair {
         // 01. Find the parent and left sibling node of the given position.
-        let (parent, leftSibling) = try pos.toTreeNodes(tree: self)
+        let (parent, leftSibling) = try pos.toTreeNodePair(tree: self)
         var leftNode = leftSibling
 
         // 02. Determine whether the position is left-most and the exact parent
@@ -636,7 +642,7 @@ class CRDTTree: CRDTElement {
      * `style` applies the given attributes of the given range.
      */
     @discardableResult
-    func style(_ range: TreePosRange, _ attributes: [String: String]?, _ editedAt: TimeTicket, _ maxCreatedAtMapByActor: [String: TimeTicket]?) throws -> ([String: TimeTicket], [GCPair], [TreeChange]) {
+    func style(_ range: TreePosRange, _ attributes: [String: String]?, _ editedAt: TimeTicket, _ maxCreatedAtMapByActor: [String: TimeTicket]? = nil) throws -> ([String: TimeTicket], [GCPair], [TreeChange]) {
         let (fromParent, fromLeft) = try self.findNodesAndSplitText(range.0, editedAt)
         let (toParent, toLeft) = try self.findNodesAndSplitText(range.1, editedAt)
 
@@ -646,10 +652,10 @@ class CRDTTree: CRDTElement {
         try self.traverseInPosRange(fromParent, fromLeft, toParent, toLeft) { token, _ in
             let (node, _) = token
             let actorID = node.createdAt.actorID
-            var maxCreatedAt: TimeTicket? = maxCreatedAtMapByActor != nil ? maxCreatedAtMapByActor?[actorID] ?? TimeTicket.initial : TimeTicket.max
+            let maxCreatedAt = maxCreatedAtMapByActor != nil ? maxCreatedAtMapByActor![actorID] ?? TimeTicket.initial : TimeTicket.max
 
-            if node.canStyle(editedAt, maxCreatedAt!), !node.isText, let attributes {
-                maxCreatedAt = createdAtMapByActor[actorID]
+            if node.canStyle(editedAt, maxCreatedAt), !node.isText, let attributes {
+                let maxCreatedAt = createdAtMapByActor[actorID]
                 let createdAt = node.createdAt
                 if maxCreatedAt == nil || createdAt.after(maxCreatedAt!) {
                     createdAtMapByActor[actorID] = createdAt
@@ -666,13 +672,16 @@ class CRDTTree: CRDTElement {
                     }
                 }
 
+                let parentOfNode = node.parent!
+                let previousNode = node.prevSibling ?? node.parent!
+
                 if !affectedAttrs.isEmpty {
                     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),
+                                                  from: self.toIndex(parentOfNode, previousNode),
+                                                  to: self.toIndex(node, node),
+                                                  fromPath: self.toPath(parentOfNode, previousNode),
+                                                  toPath: self.toPath(node, node),
                                                   value: TreeChangeValue.attributes(affectedAttrs),
                                                   splitLevel: 0) // dummy value.
                     )
@@ -690,16 +699,26 @@ class CRDTTree: CRDTElement {
     /**
      * `removeStyle` removes the given attributes of the given range.
      */
-    func removeStyle(_ range: TreePosRange, _ attributesToRemove: [String], _ editedAt: TimeTicket) throws -> ([GCPair], [TreeChange]) {
+    func removeStyle(_ range: TreePosRange, _ attributesToRemove: [String], _ editedAt: TimeTicket, _ maxCreatedAtMapByActor: [String: TimeTicket]? = nil) throws -> ([String: TimeTicket], [GCPair], [TreeChange]) {
         let (fromParent, fromLeft) = try self.findNodesAndSplitText(range.0, editedAt)
         let (toParent, toLeft) = try self.findNodesAndSplitText(range.1, editedAt)
         var changes: [TreeChange] = []
+        var createdAtMapByActor = [String: TimeTicket]()
         var pairs = [GCPair]()
         let value = TreeChangeValue.attributesToRemove(attributesToRemove)
 
         try self.traverseInPosRange(fromParent, fromLeft, toParent, toLeft) { token, _ in
             let (node, _) = token
-            if node.isRemoved == false, node.isText == false {
+            let actorID = node.createdAt.actorID
+            let maxCreatedAt = maxCreatedAtMapByActor != nil ? maxCreatedAtMapByActor![actorID] ?? TimeTicket.initial : TimeTicket.max
+
+            if node.canStyle(editedAt, maxCreatedAt), !attributesToRemove.isEmpty {
+                let maxCreatedAt = createdAtMapByActor[actorID]
+                let createdAt = node.createdAt
+                if maxCreatedAt == nil || createdAt.after(maxCreatedAt!) {
+                    createdAtMapByActor[actorID] = createdAt
+                }
+
                 if node.attrs == nil {
                     node.attrs = RHT()
                 }
@@ -710,27 +729,30 @@ class CRDTTree: CRDTElement {
                     }
                 }
 
+                let parentOfNode = node.parent!
+                let previousNode = node.prevSibling ?? node.parent!
+
                 try changes.append(TreeChange(actor: editedAt.actorID,
                                               type: .removeStyle,
-                                              from: self.toIndex(fromParent, fromLeft),
-                                              to: self.toIndex(toParent, toLeft),
-                                              fromPath: self.toPath(fromParent, fromLeft),
-                                              toPath: self.toPath(toParent, toLeft),
+                                              from: self.toIndex(parentOfNode, previousNode),
+                                              to: self.toIndex(node, node),
+                                              fromPath: self.toPath(parentOfNode, previousNode),
+                                              toPath: self.toPath(node, node),
                                               value: value,
                                               splitLevel: 0) // dummy value.
                 )
             }
         }
 
-        return (pairs, changes)
+        return (createdAtMapByActor, pairs, changes)
     }
 
     /**
      * `edit` edits the tree with the given range and content.
      * If the content is undefined, the range will be removed.
      */
     @discardableResult
-    func edit(_ range: TreePosRange, _ contents: [CRDTTreeNode]?, _ splitLevel: Int32, _ editedAt: TimeTicket, _ maxCreatedAtMapByActor: [String: TimeTicket] = [:], _ issueTimeTicket: () -> TimeTicket) throws -> ([TreeChange], [GCPair], [String: TimeTicket]) {
+    func edit(_ range: TreePosRange, _ contents: [CRDTTreeNode]?, _ splitLevel: Int32, _ editedAt: TimeTicket, _ issueTimeTicket: () -> TimeTicket, _ maxCreatedAtMapByActor: [String: TimeTicket]? = nil) throws -> ([TreeChange], [GCPair], [String: TimeTicket]) {
         // 01. find nodes from the given range and split nodes.
         let (fromParent, fromLeft) = try self.findNodesAndSplitText(range.0, editedAt)
         let (toParent, toLeft) = try self.findNodesAndSplitText(range.1, editedAt)
@@ -760,7 +782,7 @@ class CRDTTree: CRDTElement {
 
             let actorID = node.createdAt.actorID
 
-            let maxCreatedAt = maxCreatedAtMapByActor.isEmpty == false ? maxCreatedAtMapByActor[actorID] ?? TimeTicket.initial : TimeTicket.max
+            let maxCreatedAt = maxCreatedAtMapByActor != nil ? maxCreatedAtMapByActor![actorID] ?? TimeTicket.initial : TimeTicket.max
 
             // NOTE(sejongk): If the node is removable or its parent is going to
             // be removed, then this node should be removed.
@@ -886,7 +908,7 @@ class CRDTTree: CRDTElement {
     func editT(_ range: (Int, Int), _ contents: [CRDTTreeNode]?, _ splitLevel: Int32, _ editedAt: TimeTicket, _ issueTimeTicket: () -> TimeTicket) throws {
         let fromPos = try self.findPos(range.0)
         let toPos = try self.findPos(range.1)
-        try self.edit((fromPos, toPos), contents, splitLevel, editedAt, [:], issueTimeTicket)
+        try self.edit((fromPos, toPos), contents, splitLevel, editedAt, issueTimeTicket)
     }
 
     /**

Sources/Document/CRDT/RHT.swift

@@ -132,7 +132,7 @@ class RHT {
      * `get` returns the value of the given key.
      */
     func get(key: String) throws -> String {
-        guard let node = self.nodeMapByKey[key] else {
+        guard self.has(key: key), let node = self.nodeMapByKey[key] else {
             let log = "can't find the given node with: \(key)"
             Logger.critical(log)
             throw YorkieError.unexpected(message: log)

Sources/Document/Json/JSONTree.swift

@@ -384,7 +384,7 @@ public class JSONTree {
 
         let ticket = context.issueTimeTicket
 
-        let (maxCreationMapByActor, pairs, _) = try tree.style((fromPos, toPos), stringAttrs, ticket, nil)
+        let (maxCreationMapByActor, pairs, _) = try tree.style((fromPos, toPos), stringAttrs, ticket)
 
         context.push(operation: TreeStyleOperation(parentCreatedAt: tree.createdAt,
                                                    fromPos: fromPos,
@@ -442,7 +442,7 @@ public class JSONTree {
 
         let ticket = context.issueTimeTicket
 
-        let (pairs, _) = try tree.removeStyle((fromPos, toPos), attributesToRemove, ticket)
+        let (maxCreationMapByActor, pairs, _) = try tree.removeStyle((fromPos, toPos), attributesToRemove, ticket)
 
         for pair in pairs {
             self.context?.registerGCPair(pair)
@@ -451,7 +451,7 @@ public class JSONTree {
         context.push(operation: TreeStyleOperation(parentCreatedAt: tree.createdAt,
                                                    fromPos: fromPos,
                                                    toPos: toPos,
-                                                   maxCreatedAtMapByActor: [:],
+                                                   maxCreatedAtMapByActor: maxCreationMapByActor,
                                                    attributes: [:],
                                                    attributesToRemove: attributesToRemove,
                                                    executedAt: ticket)

Sources/Document/Operation/Operation.swift

@@ -194,13 +194,31 @@ public struct TreeEditOpInfo: OperationInfo {
     }
 }
 
+public enum TreeStyleOpValue: Equatable {
+    case attributes([String: Any])
+    case attributesToRemove([String])
+
+    public static func == (lhs: TreeStyleOpValue, rhs: TreeStyleOpValue) -> Bool {
+        switch (lhs, rhs) {
+        case (.attributes(let lhsAttributes), .attributes(let rhsAttributes)):
+            // Assuming [String: Any] comparison based on keys and string descriptions of values
+            return NSDictionary(dictionary: lhsAttributes).isEqual(to: rhsAttributes)
+        case (.attributesToRemove(let lhsAttributesToRemove), .attributesToRemove(let rhsAttributesToRemove)):
+            return lhsAttributesToRemove == rhsAttributesToRemove
+        default:
+            return false
+        }
+    }
+}
+
 public struct TreeStyleOpInfo: OperationInfo {
     public let type: OperationInfoType = .treeStyle
     public let path: String
     public let from: Int
     public let to: Int
     public let fromPath: [Int]
-    public let value: [String: Any]
+    public let toPath: [Int]
+    public let value: TreeStyleOpValue?
 
     public static func == (lhs: TreeStyleOpInfo, rhs: TreeStyleOpInfo) -> Bool {
         if lhs.type != rhs.type {
@@ -218,6 +236,9 @@ public struct TreeStyleOpInfo: OperationInfo {
         if lhs.fromPath != rhs.fromPath {
             return false
         }
+        if lhs.toPath != rhs.toPath {
+            return false
+        }
 
         if !(lhs.value == rhs.value) {
             return false

Sources/Document/Operation/TreeEditOperation.swift

@@ -71,7 +71,7 @@ struct TreeEditOperation: Operation {
          * Therefore, it is possible to simulate later timeTickets using `editedAt` and the length of `contents`.
          * This logic might be unclear; consider refactoring for multi-level concurrent editing in the Tree implementation.
          */
-        let (changes, pairs, _) = try tree.edit((self.fromPos, self.toPos), self.contents?.compactMap { $0.deepcopy() }, self.splitLevel, editedAt, self.maxCreatedAtMapByActor) {
+        let (changes, pairs, _) = try tree.edit((self.fromPos, self.toPos), self.contents?.compactMap { $0.deepcopy() }, self.splitLevel, editedAt, {
             var delimiter = editedAt.delimiter
             if let contents {
                 delimiter += UInt32(contents.count)
@@ -81,7 +81,7 @@ struct TreeEditOperation: Operation {
             editedAt = TimeTicket(lamport: editedAt.lamport, delimiter: delimiter, actorID: editedAt.actorID)
 
             return editedAt
-        }
+        }, self.maxCreatedAtMapByActor)
 
         for pair in pairs {
             root.registerGCPair(pair)

Sources/Document/Operation/TreeSytleOperation.swift

@@ -70,7 +70,7 @@ class TreeStyleOperation: Operation {
         if self.attributes.isEmpty == false {
             (_, pairs, changes) = try tree.style((self.fromPos, self.toPos), self.attributes, self.executedAt, self.maxCreatedAtMapByActor)
         } else {
-            (pairs, changes) = try tree.removeStyle((self.fromPos, self.toPos), self.attributesToRemove, self.executedAt)
+            (_, pairs, changes) = try tree.removeStyle((self.fromPos, self.toPos), self.attributesToRemove, self.executedAt, self.maxCreatedAtMapByActor)
         }
 
         guard let path = try? root.createPath(createdAt: parentCreatedAt) else {
@@ -82,11 +82,14 @@ class TreeStyleOperation: Operation {
         }
 
         return changes.compactMap { change in
-            let attributes: [String: Any] = {
-                if case .attributes(let attributes) = change.value {
-                    return attributes.toJSONObejct
-                } else {
-                    return [:]
+            let values: TreeStyleOpValue? = {
+                switch change.value {
+                case .attributes(let attributes):
+                    return .attributes(attributes.toJSONObejct)
+                case .attributesToRemove(let keys):
+                    return .attributesToRemove(keys)
+                default:
+                    return nil
                 }
             }()
 
@@ -95,7 +98,8 @@ class TreeStyleOperation: Operation {
                 from: change.from,
                 to: change.to,
                 fromPath: change.fromPath,
-                value: attributes
+                toPath: change.toPath,
+                value: values
             )
         }
     }

Sources/Util/IndexTree.swift

@@ -208,6 +208,21 @@ extension IndexTreeNode {
         return parent.children[safe: offset + 1]
     }
 
+    /**
+     * `prevSibling` returns the previous sibling of the node.
+     */
+    var prevSibling: Self? {
+        guard let parent else {
+            return nil
+        }
+
+        guard let offset = try? parent.findOffset(node: self) else {
+            return nil
+        }
+
+        return parent.children[safe: offset - 1]
+    }
+
     /**
      * `splitText` splits the given node at the given offset.
      */

Tests/Integration/IntegrationHelper.swift

@@ -85,8 +85,9 @@ struct TreeEditOpInfoForDebug: OperationInfoForDebug {
 struct TreeStyleOpInfoForDebug: OperationInfoForDebug {
     let from: Int?
     let to: Int?
-    let value: [String: Codable]?
+    let value: TreeStyleOpValue?
     let fromPath: [Int]?
+    let toPath: [Int]?
 
     func compare(_ operation: TreeStyleOpInfo) {
         if let value = from {
@@ -96,16 +97,14 @@ struct TreeStyleOpInfoForDebug: OperationInfoForDebug {
             XCTAssertEqual(value, operation.to)
         }
         if let value = value {
-            XCTAssertEqual(value.count, operation.value.count)
-            if operation.value.count - 1 >= 0 {
-                for key in operation.value.keys {
-                    XCTAssertEqual(value[key]?.toJSONString, convertToJSONString(operation.value[key] ?? NSNull()))
-                }
-            }
+            XCTAssertEqual(value, operation.value)
         }
         if let value = fromPath {
             XCTAssertEqual(value, operation.fromPath)
         }
+        if let value = toPath {
+            XCTAssertEqual(value, operation.toPath)
+        }
     }
 }