chore: compose operation

frontend/rust-lib/flowy-text-block/src/event_handler.rs

@@ -1,7 +1,7 @@
 use crate::entities::{EditParams, EditPayloadPB, ExportDataPB, ExportParams, ExportPayloadPB, TextBlockPB};
 use crate::TextEditorManager;
 use flowy_error::FlowyError;
-use flowy_sync::entities::text_block::{TextBlockDeltaPB, TextBlockIdPB};
+use flowy_sync::entities::text_block::TextBlockIdPB;
 use lib_dispatch::prelude::{data_result, AppData, Data, DataResult};
 use std::convert::TryInto;
 use std::sync::Arc;

frontend/rust-lib/flowy-text-block/src/manager.rs

@@ -1,4 +1,4 @@
-use crate::entities::{EditParams, EditPayloadPB};
+use crate::entities::EditParams;
 use crate::queue::TextBlockRevisionCompactor;
 use crate::{editor::TextBlockEditor, errors::FlowyError, TextEditorCloudService};
 use bytes::Bytes;

shared-lib/lib-ot/Cargo.toml

@@ -7,7 +7,7 @@ edition = "2018"
 
 [dependencies]
 bytecount = "0.6.0"
-serde = { version = "1.0", features = ["derive"] }
+serde = { version = "1.0", features = ["derive", "rc"] }
 #protobuf = {version = "2.18.0"}
 #flowy-derive = { path = "../flowy-derive" }
 tokio = { version = "1", features = ["sync"] }

shared-lib/lib-ot/src/core/document/node_tree.rs

@@ -3,6 +3,7 @@ use crate::core::document::path::Path;
 use crate::core::{Node, NodeBodyChangeset, NodeData, NodeOperation, OperationTransform, Transaction};
 use crate::errors::{ErrorBuilder, OTError, OTErrorCode};
 use indextree::{Arena, Children, FollowingSiblings, NodeId};
+use std::rc::Rc;
 
 use super::NodeOperationList;
 
@@ -26,14 +27,13 @@ impl NodeTree {
     }
 
     pub fn from_bytes(root_name: &str, bytes: Vec<u8>) -> Result<Self, OTError> {
-        let operations = NodeOperationList::from_bytes(bytes)?.into_inner();
+        let operations = NodeOperationList::from_bytes(bytes)?;
         Self::from_operations(root_name, operations)
     }
 
-    pub fn from_operations(root_name: &str, operations: Vec<NodeOperation>) -> Result<Self, OTError> {
+    pub fn from_operations(root_name: &str, operations: NodeOperationList) -> Result<Self, OTError> {
         let mut node_tree = NodeTree::new(root_name);
-
-        for operation in operations {
+        for operation in operations.into_inner().into_iter() {
             let _ = node_tree.apply_op(operation)?;
         }
         Ok(node_tree)
@@ -54,13 +54,14 @@ impl NodeTree {
     /// # Examples
     ///
     /// ```
+    /// use std::rc::Rc;
     /// use lib_ot::core::{NodeOperation, NodeTree, NodeData, Path};
     /// let nodes = vec![NodeData::new("text".to_string())];
     /// let root_path: Path = vec![0].into();
     /// let op = NodeOperation::Insert {path: root_path.clone(),nodes };
     ///
     /// let mut node_tree = NodeTree::new("root");
-    /// node_tree.apply_op(op).unwrap();
+    /// node_tree.apply_op(Rc::new(op)).unwrap();
     /// let node_id = node_tree.node_id_at_path(&root_path).unwrap();
     /// let node_path = node_tree.path_from_node_id(node_id);
     /// debug_assert_eq!(node_path, root_path);
@@ -105,23 +106,25 @@ impl NodeTree {
         counter
     }
 
-    ///
+    /// Returns the note_id at the position of the tree with id note_id
     /// # Arguments
     ///
-    /// * `node_id`:
-    /// * `index`:
+    /// * `node_id`: the node id of the child's parent
+    /// * `index`: index of the node in parent children list
     ///
     /// returns: Option<NodeId>
     ///
     /// # Examples
     ///
     /// ```
+    /// use std::rc::Rc;
     /// use lib_ot::core::{NodeOperation, NodeTree, NodeData, Path};
     /// let node_1 = NodeData::new("text".to_string());
     /// let inserted_path: Path = vec![0].into();
     ///
     /// let mut node_tree = NodeTree::new("root");
-    /// node_tree.apply_op(NodeOperation::Insert {path: inserted_path.clone(),nodes: vec![node_1.clone()] }).unwrap();
+    /// let op = NodeOperation::Insert {path: inserted_path.clone(),nodes: vec![node_1.clone()] };
+    /// node_tree.apply_op(Rc::new(op)).unwrap();
     ///
     /// let node_2 = node_tree.get_node_at_path(&inserted_path).unwrap();
     /// assert_eq!(node_2.node_type, node_1.node_type);
@@ -137,6 +140,10 @@ impl NodeTree {
         None
     }
 
+    /// Returns all children whose parent node id is node_id
+    ///
+    /// * `node_id`: the children's parent node id
+    ///
     pub fn children_from_node(&self, node_id: NodeId) -> Children<'_, Node> {
         node_id.children(&self.arena)
     }
@@ -159,7 +166,7 @@ impl NodeTree {
         node_id.following_siblings(&self.arena)
     }
 
-    pub fn apply(&mut self, transaction: Transaction) -> Result<(), OTError> {
+    pub fn apply_transaction(&mut self, transaction: Transaction) -> Result<(), OTError> {
         let operations = transaction.into_operations();
         for operation in operations {
             self.apply_op(operation)?;
@@ -167,10 +174,15 @@ impl NodeTree {
         Ok(())
     }
 
-    pub fn apply_op(&mut self, op: NodeOperation) -> Result<(), OTError> {
+    pub fn apply_op(&mut self, op: Rc<NodeOperation>) -> Result<(), OTError> {
+        let op = match Rc::try_unwrap(op) {
+            Ok(op) => op,
+            Err(op) => op.as_ref().clone(),
+        };
+
         match op {
             NodeOperation::Insert { path, nodes } => self.insert_nodes(&path, nodes),
-            NodeOperation::UpdateAttributes { path, attributes, .. } => self.update_attributes(&path, attributes),
+            NodeOperation::UpdateAttributes { path, new, .. } => self.update_attributes(&path, new),
             NodeOperation::UpdateBody { path, changeset } => self.update_body(&path, changeset),
             NodeOperation::Delete { path, nodes } => self.delete_node(&path, nodes),
         }

shared-lib/lib-ot/src/core/document/operation.rs

@@ -1,8 +1,9 @@
 use crate::core::attributes::Attributes;
 use crate::core::document::path::Path;
-use crate::core::{NodeBodyChangeset, NodeData, OperationTransform};
+use crate::core::{NodeBodyChangeset, NodeData};
 use crate::errors::OTError;
 use serde::{Deserialize, Serialize};
+use std::rc::Rc;
 
 #[derive(Debug, Clone, Serialize, Deserialize)]
 #[serde(tag = "op")]
@@ -10,12 +11,11 @@ pub enum NodeOperation {
     #[serde(rename = "insert")]
     Insert { path: Path, nodes: Vec<NodeData> },
 
-    #[serde(rename = "update")]
+    #[serde(rename = "update-attribute")]
     UpdateAttributes {
         path: Path,
-        attributes: Attributes,
-        #[serde(rename = "oldAttributes")]
-        old_attributes: Attributes,
+        new: Attributes,
+        old: Attributes,
     },
 
     #[serde(rename = "update-body")]
@@ -27,35 +27,6 @@ pub enum NodeOperation {
     Delete { path: Path, nodes: Vec<NodeData> },
 }
 
-// impl OperationTransform for NodeOperation {
-//     fn compose(&self, other: &Self) -> Result<Self, OTError>
-//     where
-//         Self: Sized,
-//     {
-//         match self {
-//             NodeOperation::Insert { path, nodes } => {
-//                 let new_path = Path::transform(path, other.path(), nodes.len() as i64);
-//                 Ok((self.clone(), other.clone_with_new_path(new_path)))
-//             }
-//             NodeOperation::Delete { path, nodes } => {
-//                 let new_path = Path::transform(path, other.path(), nodes.len() as i64);
-//                 other.clone_with_new_path(new_path)
-//             }
-//             _ => other.clone(),
-//         }
-//     }
-//
-//     fn transform(&self, other: &Self) -> Result<(Self, Self), OTError>
-//     where
-//         Self: Sized,
-//     {
-//         todo!()
-//     }
-//
-//     fn invert(&self, other: &Self) -> Self {
-//         todo!()
-//     }
-// }
 impl NodeOperation {
     pub fn get_path(&self) -> &Path {
         match self {
@@ -83,12 +54,12 @@ impl NodeOperation {
             },
             NodeOperation::UpdateAttributes {
                 path,
-                attributes,
-                old_attributes,
+                new: attributes,
+                old: old_attributes,
             } => NodeOperation::UpdateAttributes {
                 path: path.clone(),
-                attributes: old_attributes.clone(),
-                old_attributes: attributes.clone(),
+                new: old_attributes.clone(),
+                old: attributes.clone(),
             },
             NodeOperation::Delete { path, nodes } => NodeOperation::Insert {
                 path: path.clone(),
@@ -101,10 +72,10 @@ impl NodeOperation {
         }
     }
 
-    /// Make the `other` operation to be applied to the version that has been modified.
+    /// Make the `other` operation can be applied to the version after applying the `self` operation.
     /// The semantics of transform is used when editing conflicts occur, which is often determined by the version id。
-    /// For example, if the inserted position has been acquired by others, then you need to do transform to make sure
-    /// your position is value.
+    /// For example, if the inserted position has been acquired by others, then it's needed to do the transform to
+    /// make sure the inserted position is right.
     ///
     /// # Arguments
     ///
@@ -143,24 +114,30 @@ impl NodeOperation {
                 let new_path = path.transform(other.get_path(), nodes.len());
                 *other.get_mut_path() = new_path;
             }
-            _ => {}
+            _ => {
+                // Only insert/delete will change the path.
+            }
         }
     }
 }
 
 #[derive(Debug, Clone, Serialize, Deserialize, Default)]
 pub struct NodeOperationList {
-    operations: Vec<NodeOperation>,
+    operations: Vec<Rc<NodeOperation>>,
 }
 
 impl NodeOperationList {
-    pub fn into_inner(self) -> Vec<NodeOperation> {
+    pub fn into_inner(self) -> Vec<Rc<NodeOperation>> {
         self.operations
     }
+
+    pub fn add_op(&mut self, operation: NodeOperation) {
+        self.operations.push(Rc::new(operation));
+    }
 }
 
 impl std::ops::Deref for NodeOperationList {
-    type Target = Vec<NodeOperation>;
+    type Target = Vec<Rc<NodeOperation>>;
 
     fn deref(&self) -> &Self::Target {
         &self.operations
@@ -175,13 +152,15 @@ impl std::ops::DerefMut for NodeOperationList {
 
 impl std::convert::From<Vec<NodeOperation>> for NodeOperationList {
     fn from(operations: Vec<NodeOperation>) -> Self {
-        Self { operations }
+        Self::new(operations)
     }
 }
 
 impl NodeOperationList {
     pub fn new(operations: Vec<NodeOperation>) -> Self {
-        Self { operations }
+        Self {
+            operations: operations.into_iter().map(Rc::new).collect(),
+        }
     }
 
     pub fn from_bytes(bytes: Vec<u8>) -> Result<Self, OTError> {

shared-lib/lib-ot/src/core/document/path.rs

@@ -1,6 +1,28 @@
 use serde::{Deserialize, Serialize};
 
 /// The `Path` represents as a path to reference to the node in the `NodeTree`.
+/// ┌─────────┐
+/// │  Root   │
+/// └─────────┼──────────┐
+///           │0: Node A │
+///           └──────────┼────────────┐
+///                      │0: Node A-1 │  
+///                      ├────────────┤
+///                      │1: Node A-2 │
+///           ┌──────────┼────────────┘
+///           │1: Node B │
+///           └──────────┼────────────┐
+///                      │0: Node B-1 │
+///                      ├────────────┤
+///                      │1: Node B-2 │
+///           ┌──────────┼────────────┘
+///           │2: Node C │
+///           └──────────┘
+///
+/// The path of  Node A will be [0]
+/// The path of  Node A-1 will be [0,0]
+/// The path of  Node A-2 will be [0,1]
+/// The path of  Node B-2 will be [1,1]
 #[derive(Clone, Serialize, Deserialize, Eq, PartialEq, Debug)]
 pub struct Path(pub Vec<usize>);
 
@@ -49,21 +71,90 @@ impl From<&[usize]> for Path {
 }
 
 impl Path {
+    /// Calling this function if there are two changes want to modify the same path.
     ///
-    ///
-    /// The path will be changed is
     /// # Arguments
     ///
-    /// * `other`:
-    /// * `offset`: represents the len of nodes referenced by this path
+    /// * `other`: the path that need to be transformed  
+    /// * `offset`: represents the len of nodes referenced by the current path
+    ///
+    /// If two changes modify the same path or the path was shared by them. Then it needs to do the
+    /// transformation to make sure the changes are applied to the right path.
     ///
-    /// returns: Path
+    /// returns: the path represents the position that the other path reference to.
     ///
     /// # Examples
     ///
     /// ```
+    /// use lib_ot::core::Path;
+    /// let path = Path(vec![0, 1]);
+    /// for (old_path, len_of_nodes, expected_path) in vec![
+    ///     // Try to modify the path [0, 1], but someone has inserted  one element before the
+    ///     // current path [0,1] in advance. That causes the modified path [0,1] to no longer
+    ///     // valid. It needs to do the transformation to get the right path.
+    ///     //
+    ///     // [0,2] is the path you want to modify.
+    ///     (Path(vec![0, 1]), 1, Path(vec![0, 2])),
+    ///     (Path(vec![0, 1]), 5, Path(vec![0, 6])),
+    ///     (Path(vec![0, 2]), 1, Path(vec![0, 3])),
+    ///     // Try to modify the path [0, 2,3,4], but someone has inserted one element before the
+    ///     // current path [0,1] in advance. That cause the prefix path [0,2] of [0,2,3,4]
+    ///     // no longer valid.
+    ///     // It needs to do the transformation to get the right path. So [0,2] is transformed to [0,3]
+    ///     // and the suffix [3,4] of the [0,2,3,4] remains the same. So the transformed result is
+    ///     //
+    ///     // [0,3,3,4]
+    ///     (Path(vec![0, 2, 3, 4]), 1, Path(vec![0, 3, 3, 4])),
+    /// ] {
+    ///     assert_eq!(path.transform(&old_path, len_of_nodes), expected_path);
+    /// }
+    /// // The path remains the same in the following test. Because the shared path is not changed.
+    /// let path = Path(vec![0, 1, 2]);
+    /// for (old_path, len_of_nodes, expected_path) in vec![
+    ///     // Try to modify the path [0,0,0,1,2], but someone has inserted one element
+    ///     // before [0,1,2]. [0,0,0,1,2] and [0,1,2] share the same path [0,x], because
+    ///     // the element was inserted at [0,1,2] that didn't affect the shared path [0, x].
+    ///     // So, after the transformation, the path is not changed.
+    ///     (Path(vec![0, 0, 0, 1, 2]), 1, Path(vec![0, 0, 0, 1, 2])),
+    ///     (Path(vec![0, 1]), 1, Path(vec![0, 1])),
+    /// ] {
+    ///     assert_eq!(path.transform(&old_path, len_of_nodes), expected_path);
+    /// }
     ///
+    /// let path = Path(vec![1, 1]);
+    /// for (old_path, len_of_nodes, expected_path) in vec![(Path(vec![1, 0]), 1, Path(vec![1, 0]))] {
+    ///     assert_eq!(path.transform(&old_path, len_of_nodes), expected_path);
+    /// }
     /// ```
+    /// For example, client A and client B want to insert a node at the same index, the server applies
+    /// the changes made by client B. But, before applying the client A's changes, server transforms
+    /// the changes first in order to make sure that client A modify the right position. After that,
+    /// the changes can be applied to the server.
+    ///
+    /// ┌──────────┐            ┌──────────┐               ┌──────────┐
+    /// │ Client A │            │  Server  │               │ Client B │
+    /// └─────┬────┘            └─────┬────┘               └────┬─────┘
+    ///       │                       │   ┌ ─ ─ ─ ─ ─ ─ ─ ┐     │
+    ///       │                       │    Root                 │
+    ///       │                       │   │    0:A        │     │
+    ///       │                       │    ─ ─ ─ ─ ─ ─ ─ ─      │
+    ///       │                       │ ◀───────────────────────│
+    ///       │                       │    Insert B at index 1  │
+    ///       │                       │                         │
+    ///       │                       │   ┌ ─ ─ ─ ─ ─ ─ ─ ┐     │
+    ///       │                       │    Root                 │
+    ///       │                       │   │    0:A        │     │
+    ///       ├──────────────────────▶│        1:B              │
+    ///       │ Insert C at index 1   │   └ ─ ─ ─ ─ ─ ─ ─ ┘     │
+    ///       │                       │                         │
+    ///       │                       │ transform index 1 to 2  │
+    ///       │                       │                         │
+    ///       │                       │  ┌ ─ ─ ─ ─ ─ ─ ─ ─      │
+    ///       │                       │   Root            │     │
+    ///       │                       │  │    0:A               │
+    ///       ▼                       ▼       1:B         │     ▼
+    ///                                  │    2:C
+    ///                                   ─ ─ ─ ─ ─ ─ ─ ─ ┘
     pub fn transform(&self, other: &Path, offset: usize) -> Path {
         if self.len() > other.len() {
             return other.clone();
@@ -81,7 +172,7 @@ impl Path {
         let second_last_index = self.0.len() - 1;
         let mut prefix: Vec<usize> = self.0[0..second_last_index].into();
         let mut suffix: Vec<usize> = other.0[self.0.len()..].into();
-        let last_value = self.0.last().unwrap().clone();
+        let last_value = *self.0.last().unwrap();
 
         let other_second_last_value = other.0[second_last_index];
 
@@ -96,81 +187,4 @@ impl Path {
         prefix.append(&mut suffix);
         Path(prefix)
     }
-
-    // delta is default to be 1
-    pub fn transform3(pre_insert_path: &Path, b: &Path, offset: i64) -> Path {
-        if pre_insert_path.len() > b.len() {
-            return b.clone();
-        }
-        if pre_insert_path.is_empty() || b.is_empty() {
-            return b.clone();
-        }
-        // check the prefix
-        for i in 0..(pre_insert_path.len() - 1) {
-            if pre_insert_path.0[i] != b.0[i] {
-                return b.clone();
-            }
-        }
-        let mut prefix: Vec<usize> = pre_insert_path.0[0..(pre_insert_path.len() - 1)].into();
-        let mut suffix: Vec<usize> = b.0[pre_insert_path.0.len()..].into();
-        let prev_insert_last: usize = *pre_insert_path.0.last().unwrap();
-        let b_at_index = b.0[pre_insert_path.0.len() - 1];
-        if prev_insert_last <= b_at_index {
-            prefix.push(((b_at_index as i64) + offset) as usize);
-        } else {
-            prefix.push(b_at_index);
-        }
-        prefix.append(&mut suffix);
-
-        Path(prefix)
-    }
-}
-
-#[cfg(test)]
-mod tests {
-    use crate::core::Path;
-    #[test]
-    fn path_transform_test_1() {
-        assert_eq!(
-            { Path::transform3(&Path(vec![0, 1]), &Path(vec![0, 1]), 1) }.0,
-            vec![0, 2]
-        );
-
-        assert_eq!(
-            { Path::transform3(&Path(vec![0, 1]), &Path(vec![0, 1]), 5) }.0,
-            vec![0, 6]
-        );
-    }
-
-    #[test]
-    fn path_transform_test_2() {
-        assert_eq!(
-            { Path::transform3(&Path(vec![0, 1]), &Path(vec![0, 2]), 1) }.0,
-            vec![0, 3]
-        );
-    }
-
-    #[test]
-    fn path_transform_test_3() {
-        assert_eq!(
-            { Path::transform3(&Path(vec![0, 1]), &Path(vec![0, 2, 7, 8, 9]), 1) }.0,
-            vec![0, 3, 7, 8, 9]
-        );
-    }
-
-    #[test]
-    fn path_transform_no_changed_test() {
-        assert_eq!(
-            { Path::transform3(&Path(vec![0, 1, 2]), &Path(vec![0, 0, 7, 8, 9]), 1) }.0,
-            vec![0, 0, 7, 8, 9]
-        );
-        assert_eq!(
-            { Path::transform3(&Path(vec![0, 1, 2]), &Path(vec![0, 1]), 1) }.0,
-            vec![0, 1]
-        );
-        assert_eq!(
-            { Path::transform3(&Path(vec![1, 1]), &Path(vec![1, 0]), 1) }.0,
-            vec![1, 0]
-        );
-    }
 }

shared-lib/lib-ot/src/core/document/transaction.rs

@@ -1,20 +1,20 @@
 use crate::core::attributes::Attributes;
 use crate::core::document::path::Path;
 use crate::core::{NodeData, NodeOperation, NodeTree};
+use crate::errors::OTError;
 use indextree::NodeId;
+use std::rc::Rc;
 
 use super::{NodeBodyChangeset, NodeOperationList};
 
-#[derive(Debug, Clone)]
+#[derive(Debug, Clone, Default)]
 pub struct Transaction {
     operations: NodeOperationList,
 }
 
 impl Transaction {
     pub fn new() -> Self {
-        Transaction {
-            operations: vec![].into(),
-        }
+        Self::default()
     }
 
     pub fn from_operations<T: Into<NodeOperationList>>(operations: T) -> Self {
@@ -23,25 +23,36 @@ impl Transaction {
         }
     }
 
-    pub fn into_operations(self) -> Vec<NodeOperation> {
+    pub fn into_operations(self) -> Vec<Rc<NodeOperation>> {
         self.operations.into_inner()
     }
 
-    /// Make the `other` to be applied to the version that has been modified.
+    /// Make the `other` can be applied to the version after applying the `self` transaction.
     ///
     /// The semantics of transform is used when editing conflicts occur, which is often determined by the version id。
     /// the operations of the transaction will be transformed into the conflict operations.
-    pub fn transform(&self, other: &mut Transaction) {
-        for other_operation in other.iter_mut() {
+    pub fn transform(&self, other: &Transaction) -> Result<Transaction, OTError> {
+        let mut new_transaction = other.clone();
+        for other_operation in new_transaction.iter_mut() {
+            let other_operation = Rc::make_mut(other_operation);
             for operation in self.operations.iter() {
                 operation.transform(other_operation);
             }
         }
+        Ok(new_transaction)
+    }
+
+    pub fn compose(&mut self, other: &Transaction) -> Result<(), OTError> {
+        // For the moment, just append `other` operations to the end of `self`.
+        for operation in other.operations.iter() {
+            self.operations.push(operation.clone());
+        }
+        Ok(())
     }
 }
 
 impl std::ops::Deref for Transaction {
-    type Target = NodeOperationList;
+    type Target = Vec<Rc<NodeOperation>>;
 
     fn deref(&self) -> &Self::Target {
         &self.operations
@@ -85,7 +96,7 @@ impl<'a> TransactionBuilder<'a> {
     /// let transaction = TransactionBuilder::new(&node_tree)
     ///     .insert_nodes_at_path(0,vec![ NodeData::new("text_1"),  NodeData::new("text_2")])
     ///     .finalize();
-    ///  node_tree.apply(transaction).unwrap();
+    ///  node_tree.apply_transaction(transaction).unwrap();
     ///
     ///  node_tree.node_id_at_path(vec![0, 0]);
     /// ```
@@ -115,7 +126,7 @@ impl<'a> TransactionBuilder<'a> {
     /// let transaction = TransactionBuilder::new(&node_tree)
     ///     .insert_node_at_path(0, NodeData::new("text"))
     ///     .finalize();
-    ///  node_tree.apply(transaction).unwrap();
+    ///  node_tree.apply_transaction(transaction).unwrap();
     /// ```
     ///
     pub fn insert_node_at_path<T: Into<Path>>(self, path: T, node: NodeData) -> Self {
@@ -133,10 +144,10 @@ impl<'a> TransactionBuilder<'a> {
                     }
                 }
 
-                self.operations.push(NodeOperation::UpdateAttributes {
+                self.operations.add_op(NodeOperation::UpdateAttributes {
                     path: path.clone(),
-                    attributes,
-                    old_attributes,
+                    new: attributes,
+                    old: old_attributes,
                 });
             }
             None => tracing::warn!("Update attributes at path: {:?} failed. Node is not exist", path),
@@ -147,7 +158,7 @@ impl<'a> TransactionBuilder<'a> {
     pub fn update_body_at_path(mut self, path: &Path, changeset: NodeBodyChangeset) -> Self {
         match self.node_tree.node_id_at_path(path) {
             Some(_) => {
-                self.operations.push(NodeOperation::UpdateBody {
+                self.operations.add_op(NodeOperation::UpdateBody {
                     path: path.clone(),
                     changeset,
                 });
@@ -169,7 +180,7 @@ impl<'a> TransactionBuilder<'a> {
             node = self.node_tree.following_siblings(node).next().unwrap();
         }
 
-        self.operations.push(NodeOperation::Delete {
+        self.operations.add_op(NodeOperation::Delete {
             path: path.clone(),
             nodes: deleted_nodes,
         });
@@ -193,7 +204,7 @@ impl<'a> TransactionBuilder<'a> {
     }
 
     pub fn push(mut self, op: NodeOperation) -> Self {
-        self.operations.push(op);
+        self.operations.add_op(op);
         self
     }
 

shared-lib/lib-ot/tests/node/operation_test.rs

@@ -1,6 +1,6 @@
 use crate::node::script::NodeScript::*;
 use crate::node::script::NodeTest;
-use lib_ot::core::{AttributeBuilder, Node, NodeTree, Transaction, TransactionBuilder};
+use lib_ot::core::{AttributeBuilder, Node};
 use lib_ot::{
     core::{NodeBodyChangeset, NodeData, NodeDataBuilder, NodeOperation, Path},
     text_delta::TextDeltaBuilder,
@@ -35,15 +35,14 @@ fn operation_insert_node_with_children_serde_test() {
 fn operation_update_node_attributes_serde_test() {
     let operation = NodeOperation::UpdateAttributes {
         path: Path(vec![0, 1]),
-        attributes: AttributeBuilder::new().insert("bold", true).build(),
-        old_attributes: AttributeBuilder::new().insert("bold", false).build(),
+        new: AttributeBuilder::new().insert("bold", true).build(),
+        old: AttributeBuilder::new().insert("bold", false).build(),
     };
 
     let result = serde_json::to_string(&operation).unwrap();
-
     assert_eq!(
         result,
-        r#"{"op":"update","path":[0,1],"attributes":{"bold":true},"oldAttributes":{"bold":null}}"#
+        r#"{"op":"update-attribute","path":[0,1],"new":{"bold":true},"old":{"bold":null}}"#
     );
 }
 
@@ -119,17 +118,17 @@ fn operation_insert_transform_one_level_path_test() {
     let scripts = vec![
         InsertNode {
             path: 0.into(),
-            node_data: node_data_1.clone(),
+            node_data: node_data_1,
             rev_id: 1,
         },
         InsertNode {
             path: 1.into(),
-            node_data: node_data_2.clone(),
+            node_data: node_data_2,
             rev_id: 2,
         },
         InsertNode {
             path: 1.into(),
-            node_data: node_data_3.clone(),
+            node_data: node_data_3,
             rev_id: 1,
         },
         AssertNode {
@@ -157,12 +156,12 @@ fn operation_insert_transform_multiple_level_path_test() {
     let scripts = vec![
         InsertNode {
             path: 0.into(),
-            node_data: node_data_1.clone(),
+            node_data: node_data_1,
             rev_id: 1,
         },
         InsertNode {
             path: 1.into(),
-            node_data: node_data_2.clone(),
+            node_data: node_data_2,
             rev_id: 2,
         },
         InsertNode {
@@ -189,12 +188,12 @@ fn operation_delete_transform_path_test() {
     let scripts = vec![
         InsertNode {
             path: 0.into(),
-            node_data: node_data_1.clone(),
+            node_data: node_data_1,
             rev_id: 1,
         },
         InsertNode {
             path: 1.into(),
-            node_data: node_data_2.clone(),
+            node_data: node_data_2,
             rev_id: 2,
         },
         // The node's in the tree will be:
@@ -206,7 +205,7 @@ fn operation_delete_transform_path_test() {
         // but it was moved to index 2.
         InsertNode {
             path: 1.into(),
-            node_data: node_data_3.clone(),
+            node_data: node_data_3,
             rev_id: 3,
         },
         // 0: text_1

shared-lib/lib-ot/tests/node/script.rs

@@ -1,4 +1,4 @@
-use lib_ot::core::{Node, NodeOperation, Transaction};
+use lib_ot::core::{Node, Transaction};
 use lib_ot::{
     core::attributes::Attributes,
     core::{NodeBody, NodeBodyChangeset, NodeData, NodeTree, Path, TransactionBuilder},
@@ -144,14 +144,14 @@ impl NodeTest {
     fn apply_transaction(&mut self, transaction: Transaction) {
         self.rev_id += 1;
         self.rev_operations.insert(self.rev_id, transaction.clone());
-        self.node_tree.apply(transaction).unwrap();
+        self.node_tree.apply_transaction(transaction).unwrap();
     }
 
     fn transform_transaction_if_need(&mut self, transaction: &mut Transaction, rev_id: usize) {
         if self.rev_id >= rev_id {
             for rev_id in rev_id..=self.rev_id {
                 let old_transaction = self.rev_operations.get(&rev_id).unwrap();
-                old_transaction.transform(transaction);
+                *transaction = old_transaction.transform(transaction).unwrap();
             }
         }
     }

shared-lib/lib-ot/tests/node/tree_test.rs

@@ -114,7 +114,7 @@ fn node_insert_node_in_ordered_nodes_test() {
         },
         InsertNode {
             path: path_3.clone(),
-            node_data: node_3.clone(),
+            node_data: node_3,
             rev_id: 3,
         },
         // 0:text_1
@@ -166,12 +166,12 @@ fn node_insert_nested_nodes_test() {
     let scripts = vec![
         InsertNode {
             path: 0.into(),
-            node_data: node_data_1.clone(),
+            node_data: node_data_1,
             rev_id: 1,
         },
         InsertNode {
             path: 1.into(),
-            node_data: node_data_2.clone(),
+            node_data: node_data_2,
             rev_id: 2,
         },
         // the tree will be:
@@ -214,7 +214,7 @@ fn node_insert_node_before_existing_nested_nodes_test() {
     let scripts = vec![
         InsertNode {
             path: 0.into(),
-            node_data: node_data_1.clone(),
+            node_data: node_data_1,
             rev_id: 1,
         },
         // 0:text_1