fix: changeset composing (#1398)

frontend/app_flowy/pubspec.lock

@@ -35,7 +35,7 @@ packages:
       path: "packages/appflowy_editor"
       relative: true
     source: path
-    version: "0.0.6"
+    version: "0.0.7"
   appflowy_popover:
     dependency: "direct main"
     description:

frontend/rust-lib/flowy-document/tests/editor/op_test.rs

@@ -36,8 +36,7 @@ fn attributes_insert_text_at_middle() {
 
 #[test]
 fn delta_get_ops_in_interval_1() {
-    let operations = OperationsBuilder::new().insert("123").insert("4").build();
-    let delta = DeltaTextOperationBuilder::from_operations(operations);
+    let delta = DeltaTextOperationBuilder::new().insert("123").insert("4").build();
 
     let mut iterator = OperationIterator::from_interval(&delta, Interval::new(0, 4));
     assert_eq!(iterator.ops(), delta.ops);

shared-lib/lib-ot/src/core/delta/builder.rs

@@ -1,6 +1,5 @@
 use crate::core::delta::operation::OperationAttributes;
 use crate::core::delta::{trim, DeltaOperations};
-use crate::core::DeltaOperation;
 
 /// A builder for creating new [Operations] objects.
 ///
@@ -39,12 +38,13 @@ where
         DeltaOperationBuilder::default()
     }
 
-    pub fn from_operations(operations: Vec<DeltaOperation<T>>) -> DeltaOperations<T> {
-        let mut delta = DeltaOperationBuilder::default().build();
-        operations.into_iter().for_each(|operation| {
-            delta.add(operation);
+    pub fn from_delta_operation(delta_operation: DeltaOperations<T>) -> Self {
+        debug_assert!(delta_operation.utf16_base_len == 0);
+        let mut builder = DeltaOperationBuilder::new();
+        delta_operation.ops.into_iter().for_each(|operation| {
+            builder.delta.add(operation);
         });
-        delta
+        builder
     }
 
     /// Retain the 'n' characters with the attributes. Use 'retain' instead if you don't

shared-lib/lib-ot/src/core/delta/operation/builder.rs

@@ -1,51 +0,0 @@
-use crate::core::delta::operation::{DeltaOperation, EmptyAttributes, OperationAttributes};
-
-// pub type RichTextOpBuilder = OperationsBuilder<TextAttributes>;
-pub type PlainTextOpBuilder = OperationsBuilder<EmptyAttributes>;
-
-#[derive(Default)]
-pub struct OperationsBuilder<T: OperationAttributes> {
-    operations: Vec<DeltaOperation<T>>,
-}
-
-impl<T> OperationsBuilder<T>
-where
-    T: OperationAttributes,
-{
-    pub fn new() -> OperationsBuilder<T> {
-        OperationsBuilder::default()
-    }
-
-    pub fn retain_with_attributes(mut self, n: usize, attributes: T) -> OperationsBuilder<T> {
-        let retain = DeltaOperation::retain_with_attributes(n, attributes);
-        self.operations.push(retain);
-        self
-    }
-
-    pub fn retain(mut self, n: usize) -> OperationsBuilder<T> {
-        let retain = DeltaOperation::retain(n);
-        self.operations.push(retain);
-        self
-    }
-
-    pub fn delete(mut self, n: usize) -> OperationsBuilder<T> {
-        self.operations.push(DeltaOperation::Delete(n));
-        self
-    }
-
-    pub fn insert_with_attributes(mut self, s: &str, attributes: T) -> OperationsBuilder<T> {
-        let insert = DeltaOperation::insert_with_attributes(s, attributes);
-        self.operations.push(insert);
-        self
-    }
-
-    pub fn insert(mut self, s: &str) -> OperationsBuilder<T> {
-        let insert = DeltaOperation::insert(s);
-        self.operations.push(insert);
-        self
-    }
-
-    pub fn build(self) -> Vec<DeltaOperation<T>> {
-        self.operations
-    }
-}

shared-lib/lib-ot/src/core/delta/operation/mod.rs

@@ -1,8 +1,6 @@
 #![allow(clippy::module_inception)]
-mod builder;
 mod operation;
 mod operation_serde;
 
-pub use builder::*;
 pub use operation::*;
 pub use operation_serde::*;

shared-lib/lib-ot/src/core/delta/ops.rs

@@ -224,6 +224,10 @@ where
         Ok(new_s)
     }
 
+    pub fn inverted(&self) -> Self {
+        self.invert_str("")
+    }
+
     /// Computes the inverse [Delta]. The inverse of an operation is the
     /// operation that reverts the effects of the operation     
     /// # Arguments

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

@@ -212,7 +212,7 @@ impl Changeset {
                     inverted: _,
                 },
             ) => {
-                let original = delta.invert(inverted);
+                let original = delta.compose(inverted)?;
                 let new_delta = delta.compose(other_delta)?;
                 let new_inverted = new_delta.invert(&original);
 

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

@@ -234,16 +234,6 @@ impl NodeOperations {
             }
         }
 
-        // if let Some(operations) = self.inner.get_mut(other.get_path()) {
-        //     if let Some(last_operation) = operations.last_mut() {
-        //         if last_operation.can_compose(&other) {
-        //             let mut_operation = Arc::make_mut(last_operation);
-        //             if mut_operation.compose(&other).is_ok() {
-        //                 return;
-        //             }
-        //         }
-        //     }
-        // }
         // If the passed-in operation can't be composed, then append it to the end.
         self.inner.push(other);
     }

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

@@ -0,0 +1,194 @@
+use crate::node::script::NodeScript::*;
+use crate::node::script::NodeTest;
+use lib_ot::core::{AttributeEntry, Changeset, NodeData, OperationTransform};
+use lib_ot::text_delta::DeltaTextOperationBuilder;
+
+#[test]
+fn changeset_delta_compose_delta_test() {
+    // delta 1
+    let delta_1 = DeltaTextOperationBuilder::new().insert("Hello world").build();
+    let inverted_1 = delta_1.inverted();
+    let mut changeset_1 = Changeset::Delta {
+        delta: delta_1.clone(),
+        inverted: inverted_1,
+    };
+
+    // delta 2
+    let delta_2 = DeltaTextOperationBuilder::new()
+        .retain(delta_1.utf16_target_len)
+        .insert("!")
+        .build();
+    let inverted_2 = delta_2.inverted();
+    let changeset_2 = Changeset::Delta {
+        delta: delta_2,
+        inverted: inverted_2,
+    };
+
+    // compose
+    changeset_1.compose(&changeset_2).unwrap();
+
+    if let Changeset::Delta { delta, inverted } = changeset_1 {
+        assert_eq!(delta.content().unwrap(), "Hello world!");
+        let new_delta = delta.compose(&inverted).unwrap();
+        assert_eq!(new_delta.content().unwrap(), "");
+    }
+}
+
+#[test]
+fn operation_compose_delta_changeset_then_invert_test() {
+    let delta = DeltaTextOperationBuilder::new().insert("Hello world").build();
+    let inverted = delta.inverted();
+    let changeset = Changeset::Delta {
+        delta: delta.clone(),
+        inverted: inverted.clone(),
+    };
+
+    let mut test = NodeTest::new();
+    let text_node = NodeData::new("text");
+    let scripts = vec![
+        InsertNode {
+            path: 0.into(),
+            node_data: text_node,
+            rev_id: 1,
+        },
+        UpdateBody {
+            path: 0.into(),
+            changeset: changeset.clone(),
+        },
+        AssertNodeDelta {
+            path: 0.into(),
+            expected: delta.clone(),
+        },
+        UpdateBody {
+            path: 0.into(),
+            changeset: changeset.inverted(),
+        },
+        AssertNodeDelta {
+            path: 0.into(),
+            expected: delta.compose(&inverted).unwrap(),
+        },
+    ];
+    test.run_scripts(scripts);
+}
+
+#[test]
+fn operation_compose_multiple_delta_changeset_then_invert_test() {
+    // delta 1
+    let delta_1 = DeltaTextOperationBuilder::new().insert("Hello world").build();
+    let inverted_1 = delta_1.inverted();
+    let changeset_1 = Changeset::Delta {
+        delta: delta_1.clone(),
+        inverted: inverted_1,
+    };
+
+    // delta 2
+    let delta_2 = DeltaTextOperationBuilder::new()
+        .retain(delta_1.utf16_target_len)
+        .insert("!")
+        .build();
+    let inverted_2 = delta_2.inverted();
+    let changeset_2 = Changeset::Delta {
+        delta: delta_2.clone(),
+        inverted: inverted_2,
+    };
+
+    // delta 3
+    let delta_3 = DeltaTextOperationBuilder::new()
+        .retain(delta_2.utf16_target_len)
+        .insert("AppFlowy")
+        .build();
+    let inverted_3 = delta_3.inverted();
+    let changeset_3 = Changeset::Delta {
+        delta: delta_3.clone(),
+        inverted: inverted_3,
+    };
+
+    let mut test = NodeTest::new();
+    let text_node = NodeData::new("text");
+    let scripts = vec![
+        InsertNode {
+            path: 0.into(),
+            node_data: text_node,
+            rev_id: 1,
+        },
+        UpdateBody {
+            path: 0.into(),
+            changeset: changeset_1.clone(),
+        },
+        UpdateBody {
+            path: 0.into(),
+            changeset: changeset_2.clone(),
+        },
+        UpdateBody {
+            path: 0.into(),
+            changeset: changeset_3.clone(),
+        },
+        AssertNodeDelta {
+            path: 0.into(),
+            expected: delta_1.compose(&delta_2).unwrap().compose(&delta_3).unwrap(),
+        },
+        UpdateBody {
+            path: 0.into(),
+            changeset: changeset_3.inverted(),
+        },
+        AssertNodeDeltaContent {
+            path: 0.into(),
+            expected: r#"Hello world!"#,
+        },
+        UpdateBody {
+            path: 0.into(),
+            changeset: changeset_2.inverted(),
+        },
+        AssertNodeDeltaContent {
+            path: 0.into(),
+            expected: r#"Hello world"#,
+        },
+        UpdateBody {
+            path: 0.into(),
+            changeset: changeset_1.inverted(),
+        },
+        AssertNodeDeltaContent {
+            path: 0.into(),
+            expected: r#""#,
+        },
+    ];
+    test.run_scripts(scripts);
+}
+
+#[test]
+#[should_panic]
+fn changeset_delta_compose_attributes_test() {
+    // delta 1
+    let delta = DeltaTextOperationBuilder::new().insert("Hello world").build();
+    let inverted = delta.inverted();
+    let mut delta_changeset = Changeset::Delta { delta, inverted };
+
+    // attributes
+    let attribute_changeset = Changeset::Attributes {
+        new: Default::default(),
+        old: Default::default(),
+    };
+
+    // compose
+    delta_changeset.compose(&attribute_changeset).unwrap();
+}
+
+#[test]
+fn changeset_attributes_compose_attributes_test() {
+    // attributes
+    let mut changeset_1 = Changeset::Attributes {
+        new: AttributeEntry::new("bold", true).into(),
+        old: Default::default(),
+    };
+
+    let changeset_2 = Changeset::Attributes {
+        new: AttributeEntry::new("Italic", true).into(),
+        old: Default::default(),
+    };
+    // compose
+    changeset_1.compose(&changeset_2).unwrap();
+
+    if let Changeset::Attributes { new, old: _ } = changeset_1 {
+        assert_eq!(new, AttributeEntry::new("Italic", true).into());
+    }
+}

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

@@ -1,4 +1,6 @@
+mod changeset_compose_test;
 mod operation_attribute_test;
+mod operation_compose_test;
 mod operation_delete_test;
 mod operation_delta_test;
 mod operation_insert_test;

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

@@ -9,7 +9,7 @@ fn operation_update_attribute_with_float_value_test() {
     let scripts = vec![
         InsertNode {
             path: 0.into(),
-            node_data: text_node.clone(),
+            node_data: text_node,
             rev_id: 1,
         },
         UpdateBody {
@@ -34,7 +34,7 @@ fn operation_update_attribute_with_negative_value_test() {
     let scripts = vec![
         InsertNode {
             path: 0.into(),
-            node_data: text_node.clone(),
+            node_data: text_node,
             rev_id: 1,
         },
         UpdateBody {

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

@@ -0,0 +1,135 @@
+use lib_ot::core::{Changeset, NodeOperation};
+
+#[test]
+fn operation_insert_compose_delta_update_test() {
+    let insert_operation = NodeOperation::Insert {
+        path: 0.into(),
+        nodes: vec![],
+    };
+
+    let update_operation = NodeOperation::Update {
+        path: 0.into(),
+        changeset: Changeset::Delta {
+            delta: Default::default(),
+            inverted: Default::default(),
+        },
+    };
+
+    assert!(insert_operation.can_compose(&update_operation))
+}
+
+#[test]
+fn operation_insert_compose_attribute_update_test() {
+    let insert_operation = NodeOperation::Insert {
+        path: 0.into(),
+        nodes: vec![],
+    };
+
+    let update_operation = NodeOperation::Update {
+        path: 0.into(),
+        changeset: Changeset::Attributes {
+            new: Default::default(),
+            old: Default::default(),
+        },
+    };
+
+    assert!(!insert_operation.can_compose(&update_operation))
+}
+#[test]
+fn operation_insert_compose_update_with_diff_path_test() {
+    let insert_operation = NodeOperation::Insert {
+        path: 0.into(),
+        nodes: vec![],
+    };
+
+    let update_operation = NodeOperation::Update {
+        path: 1.into(),
+        changeset: Changeset::Attributes {
+            new: Default::default(),
+            old: Default::default(),
+        },
+    };
+
+    assert!(!insert_operation.can_compose(&update_operation))
+}
+
+#[test]
+fn operation_insert_compose_insert_operation_test() {
+    let insert_operation = NodeOperation::Insert {
+        path: 0.into(),
+        nodes: vec![],
+    };
+
+    assert!(!insert_operation.can_compose(&NodeOperation::Insert {
+        path: 0.into(),
+        nodes: vec![],
+    }),)
+}
+
+#[test]
+fn operation_update_compose_insert_operation_test() {
+    let update_operation = NodeOperation::Update {
+        path: 0.into(),
+        changeset: Changeset::Attributes {
+            new: Default::default(),
+            old: Default::default(),
+        },
+    };
+
+    assert!(!update_operation.can_compose(&NodeOperation::Insert {
+        path: 0.into(),
+        nodes: vec![],
+    }))
+}
+#[test]
+fn operation_update_compose_update_test() {
+    let update_operation_1 = NodeOperation::Update {
+        path: 0.into(),
+        changeset: Changeset::Attributes {
+            new: Default::default(),
+            old: Default::default(),
+        },
+    };
+    let update_operation_2 = NodeOperation::Update {
+        path: 0.into(),
+        changeset: Changeset::Attributes {
+            new: Default::default(),
+            old: Default::default(),
+        },
+    };
+
+    assert!(update_operation_1.can_compose(&update_operation_2));
+}
+#[test]
+fn operation_update_compose_update_with_diff_path_test() {
+    let update_operation_1 = NodeOperation::Update {
+        path: 0.into(),
+        changeset: Changeset::Attributes {
+            new: Default::default(),
+            old: Default::default(),
+        },
+    };
+    let update_operation_2 = NodeOperation::Update {
+        path: 1.into(),
+        changeset: Changeset::Attributes {
+            new: Default::default(),
+            old: Default::default(),
+        },
+    };
+
+    assert!(!update_operation_1.can_compose(&update_operation_2));
+}
+
+#[test]
+fn operation_insert_compose_insert_test() {
+    let insert_operation_1 = NodeOperation::Insert {
+        path: 0.into(),
+        nodes: vec![],
+    };
+    let insert_operation_2 = NodeOperation::Insert {
+        path: 0.into(),
+        nodes: vec![],
+    };
+
+    assert!(!insert_operation_1.can_compose(&insert_operation_2));
+}

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

@@ -55,6 +55,10 @@ pub enum NodeScript {
         path: Path,
         expected: DeltaTextOperations,
     },
+    AssertNodeDeltaContent {
+        path: Path,
+        expected: &'static str,
+    },
     #[allow(dead_code)]
     AssertTreeJSON {
         expected: String,
@@ -165,6 +169,14 @@ impl NodeTest {
                     panic!("Node body type not match, expect Delta");
                 }
             }
+            NodeScript::AssertNodeDeltaContent { path, expected } => {
+                let node = self.node_tree.get_node_at_path(&path).unwrap();
+                if let Body::Delta(delta) = node.body.clone() {
+                    debug_assert_eq!(delta.content().unwrap(), expected);
+                } else {
+                    panic!("Node body type not match, expect Delta");
+                }
+            }
             NodeScript::AssertTreeJSON { expected } => {
                 let json = serde_json::to_string(&self.node_tree).unwrap();
                 assert_eq!(json, expected)