From 4b654de2e6a20b44f4ac16b2edf12c92340a0da6 Mon Sep 17 00:00:00 2001 From: appflowy Date: Wed, 14 Sep 2022 12:03:52 +0800 Subject: [PATCH] chore: updata path documentation --- .../lib-ot/src/core/document/node_tree.rs | 4 +- .../lib-ot/src/core/document/operation.rs | 57 +++------ shared-lib/lib-ot/src/core/document/path.rs | 65 +++++++++-- .../lib-ot/src/core/document/transaction.rs | 6 +- .../lib-ot/tests/node/operation_test.rs | 74 +++++++++--- shared-lib/lib-ot/tests/node/script.rs | 2 +- shared-lib/lib-ot/tests/node/tree_test.rs | 109 ++++++++++++++++-- 7 files changed, 243 insertions(+), 74 deletions(-) diff --git a/shared-lib/lib-ot/src/core/document/node_tree.rs b/shared-lib/lib-ot/src/core/document/node_tree.rs index 87e8e427f3..53f538cf77 100644 --- a/shared-lib/lib-ot/src/core/document/node_tree.rs +++ b/shared-lib/lib-ot/src/core/document/node_tree.rs @@ -216,8 +216,8 @@ impl NodeTree { return Ok(()); } - /// Append the node to the end of the children list if index greater or equal to the - /// length of the children. + // Append the node to the end of the children list if index greater or equal to the + // length of the children. if index >= parent.children(&self.arena).count() { self.append_nodes(&parent, nodes); return Ok(()); diff --git a/shared-lib/lib-ot/src/core/document/operation.rs b/shared-lib/lib-ot/src/core/document/operation.rs index 9117a0aeb7..eb51eeac3d 100644 --- a/shared-lib/lib-ot/src/core/document/operation.rs +++ b/shared-lib/lib-ot/src/core/document/operation.rs @@ -66,15 +66,12 @@ impl NodeOperation { } } - pub fn mut_path(&mut self, f: F) - where - F: FnOnce(&mut Path), - { + pub fn get_mut_path(&mut self) -> &mut Path { match self { - NodeOperation::Insert { path, .. } => f(path), - NodeOperation::UpdateAttributes { path, .. } => f(path), - NodeOperation::Delete { path, .. } => f(path), - NodeOperation::UpdateBody { path, .. } => f(path), + NodeOperation::Insert { path, .. } => path, + NodeOperation::UpdateAttributes { path, .. } => path, + NodeOperation::Delete { path, .. } => path, + NodeOperation::UpdateBody { path, .. } => path, } } @@ -104,15 +101,15 @@ impl NodeOperation { } } - /// Transform the `other` operation into a new operation that carries the changes made by - /// the current operation. + /// Make the `other` operation to be applied to the version that has been modified. + /// 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. /// /// # Arguments /// /// * `other`: The operation that is going to be transformed /// - /// returns: NodeOperation - /// /// # Examples /// /// ``` @@ -130,39 +127,21 @@ impl NodeOperation { /// nodes: vec![node_2], /// }; /// - /// assert_eq!(serde_json::to_string(&op_2).unwrap(), r#"{"op":"insert","path":[0,1], - /// "nodes":[{"type":"text_2"}]}"#); + /// assert_eq!(serde_json::to_string(&op_2).unwrap(), r#"{"op":"insert","path":[0,1],"nodes":[{"type":"text_2"}]}"#); /// - /// let new_op = op_1.transform(&op_2); - /// assert_eq!(serde_json::to_string(&new_op).unwrap(), r#"{"op":"insert","path":[0,2], - /// "nodes":[{"type":"text_2"}]}"#); + /// op_1.transform(&mut op_2); + /// assert_eq!(serde_json::to_string(&op_2).unwrap(), r#"{"op":"insert","path":[0,2],"nodes":[{"type":"text_2"}]}"#); /// /// ``` - pub fn transform(&self, other: &NodeOperation) -> NodeOperation { - let mut other = other.clone(); + pub fn transform(&self, other: &mut NodeOperation) { match self { NodeOperation::Insert { path, nodes } => { - let new_path = Path::transform(path, other.get_path(), nodes.len() as i64); - other.mut_path(|path| *path = new_path); + let new_path = path.transform(other.get_path(), nodes.len()); + *other.get_mut_path() = new_path; } - NodeOperation::Delete { path: a_path, nodes } => { - let new_path = Path::transform(a_path, other.get_path(), nodes.len() as i64); - other.mut_path(|path| *path = new_path); - } - _ => {} - } - other - } - - pub fn mut_transform(&self, other: &mut NodeOperation) { - match self { - NodeOperation::Insert { path, nodes } => { - let new_path = Path::transform(path, other.get_path(), nodes.len() as i64); - other.mut_path(|path| *path = new_path); - } - NodeOperation::Delete { path: a_path, nodes } => { - let new_path = Path::transform(a_path, other.get_path(), nodes.len() as i64); - other.mut_path(|path| *path = new_path); + NodeOperation::Delete { path, nodes } => { + let new_path = path.transform(other.get_path(), nodes.len()); + *other.get_mut_path() = new_path; } _ => {} } diff --git a/shared-lib/lib-ot/src/core/document/path.rs b/shared-lib/lib-ot/src/core/document/path.rs index 245abde36c..16fecfcefe 100644 --- a/shared-lib/lib-ot/src/core/document/path.rs +++ b/shared-lib/lib-ot/src/core/document/path.rs @@ -1,5 +1,6 @@ use serde::{Deserialize, Serialize}; +/// The `Path` represents as a path to reference to the node in the `NodeTree`. #[derive(Clone, Serialize, Deserialize, Eq, PartialEq, Debug)] pub struct Path(pub Vec); @@ -48,8 +49,56 @@ impl From<&[usize]> for Path { } impl Path { + /// + /// + /// The path will be changed is + /// # Arguments + /// + /// * `other`: + /// * `offset`: represents the len of nodes referenced by this path + /// + /// returns: Path + /// + /// # Examples + /// + /// ``` + /// + /// ``` + pub fn transform(&self, other: &Path, offset: usize) -> Path { + if self.len() > other.len() { + return other.clone(); + } + if self.is_empty() || other.is_empty() { + return other.clone(); + } + for i in 0..(self.len() - 1) { + if self.0[i] != other.0[i] { + return other.clone(); + } + } + + // Splits the `Path` into two part. The suffix will contain the last element of the `Path`. + let second_last_index = self.0.len() - 1; + let mut prefix: Vec = self.0[0..second_last_index].into(); + let mut suffix: Vec = other.0[self.0.len()..].into(); + let last_value = self.0.last().unwrap().clone(); + + let other_second_last_value = other.0[second_last_index]; + + // + if last_value <= other_second_last_value { + prefix.push(other_second_last_value + offset); + } else { + prefix.push(other_second_last_value); + } + + // concat the prefix and suffix into a new path + prefix.append(&mut suffix); + Path(prefix) + } + // delta is default to be 1 - pub fn transform(pre_insert_path: &Path, b: &Path, offset: i64) -> Path { + pub fn transform3(pre_insert_path: &Path, b: &Path, offset: i64) -> Path { if pre_insert_path.len() > b.len() { return b.clone(); } @@ -83,12 +132,12 @@ mod tests { #[test] fn path_transform_test_1() { assert_eq!( - { Path::transform(&Path(vec![0, 1]), &Path(vec![0, 1]), 1) }.0, + { Path::transform3(&Path(vec![0, 1]), &Path(vec![0, 1]), 1) }.0, vec![0, 2] ); assert_eq!( - { Path::transform(&Path(vec![0, 1]), &Path(vec![0, 1]), 5) }.0, + { Path::transform3(&Path(vec![0, 1]), &Path(vec![0, 1]), 5) }.0, vec![0, 6] ); } @@ -96,7 +145,7 @@ mod tests { #[test] fn path_transform_test_2() { assert_eq!( - { Path::transform(&Path(vec![0, 1]), &Path(vec![0, 2]), 1) }.0, + { Path::transform3(&Path(vec![0, 1]), &Path(vec![0, 2]), 1) }.0, vec![0, 3] ); } @@ -104,7 +153,7 @@ mod tests { #[test] fn path_transform_test_3() { assert_eq!( - { Path::transform(&Path(vec![0, 1]), &Path(vec![0, 2, 7, 8, 9]), 1) }.0, + { Path::transform3(&Path(vec![0, 1]), &Path(vec![0, 2, 7, 8, 9]), 1) }.0, vec![0, 3, 7, 8, 9] ); } @@ -112,15 +161,15 @@ mod tests { #[test] fn path_transform_no_changed_test() { assert_eq!( - { Path::transform(&Path(vec![0, 1, 2]), &Path(vec![0, 0, 7, 8, 9]), 1) }.0, + { 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::transform(&Path(vec![0, 1, 2]), &Path(vec![0, 1]), 1) }.0, + { Path::transform3(&Path(vec![0, 1, 2]), &Path(vec![0, 1]), 1) }.0, vec![0, 1] ); assert_eq!( - { Path::transform(&Path(vec![1, 1]), &Path(vec![1, 0]), 1) }.0, + { Path::transform3(&Path(vec![1, 1]), &Path(vec![1, 0]), 1) }.0, vec![1, 0] ); } diff --git a/shared-lib/lib-ot/src/core/document/transaction.rs b/shared-lib/lib-ot/src/core/document/transaction.rs index 20e58fe1ec..28959bf7f2 100644 --- a/shared-lib/lib-ot/src/core/document/transaction.rs +++ b/shared-lib/lib-ot/src/core/document/transaction.rs @@ -27,10 +27,14 @@ impl Transaction { self.operations.into_inner() } + /// Make the `other` to be applied to the version that has been modified. + /// + /// 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() { for operation in self.operations.iter() { - operation.mut_transform(other_operation); + operation.transform(other_operation); } } } diff --git a/shared-lib/lib-ot/tests/node/operation_test.rs b/shared-lib/lib-ot/tests/node/operation_test.rs index ba8e76b4d9..bbd67ab1dd 100644 --- a/shared-lib/lib-ot/tests/node/operation_test.rs +++ b/shared-lib/lib-ot/tests/node/operation_test.rs @@ -81,7 +81,7 @@ fn operation_insert_op_transform_test() { nodes: vec![node_1], }; - let insert_2 = NodeOperation::Insert { + let mut insert_2 = NodeOperation::Insert { path: Path(vec![0, 1]), nodes: vec![node_2], }; @@ -89,29 +89,33 @@ fn operation_insert_op_transform_test() { // let mut node_tree = NodeTree::new("root"); // node_tree.apply_op(insert_1.clone()).unwrap(); - let new_op = op_1.transform(&insert_2); - let json = serde_json::to_string(&new_op).unwrap(); + op_1.transform(&mut insert_2); + let json = serde_json::to_string(&insert_2).unwrap(); assert_eq!(json, r#"{"op":"insert","path":[0,2],"nodes":[{"type":"text_2"}]}"#); } #[test] -fn operation_insert_transform_test() { +fn operation_insert_transform_one_level_path_test() { let mut test = NodeTest::new(); let node_data_1 = NodeDataBuilder::new("text_1").build(); let node_data_2 = NodeDataBuilder::new("text_2").build(); let node_data_3 = NodeDataBuilder::new("text_3").build(); let node_3: Node = node_data_3.clone().into(); + // 0: text_1 + // 1: text_2 // - // rev_id:1 0: text_1 - // rev_id:2 1: text_2 + // Insert a new operation with rev_id 1,but the rev_id:1 is already exist, so + // it needs to be transformed. + // 1:text_3 => 2:text_3 // - // Insert a new operation with rev_id 1.But the rev_id:1 is already exist, so - // it needs to do the transform. - // - // --> 1:text_3 - // transform into: - // --> 2:text_3 + // 0: text_1 + // 1: text_2 + // 2: text_3 // + // If the rev_id of the insert operation is 3. then the tree will be: + // 0: text_1 + // 1: text_3 + // 2: text_2 let scripts = vec![ InsertNode { path: 0.into(), @@ -137,7 +141,45 @@ fn operation_insert_transform_test() { } #[test] -fn operation_delete_transform_test() { +fn operation_insert_transform_multiple_level_path_test() { + let mut test = NodeTest::new(); + let node_data_1 = NodeDataBuilder::new("text_1") + .add_node(NodeDataBuilder::new("text_1_1").build()) + .add_node(NodeDataBuilder::new("text_1_2").build()) + .build(); + + let node_data_2 = NodeDataBuilder::new("text_2") + .add_node(NodeDataBuilder::new("text_2_1").build()) + .add_node(NodeDataBuilder::new("text_2_2").build()) + .build(); + + let node_data_3 = NodeDataBuilder::new("text_3").build(); + let scripts = vec![ + InsertNode { + path: 0.into(), + node_data: node_data_1.clone(), + rev_id: 1, + }, + InsertNode { + path: 1.into(), + node_data: node_data_2.clone(), + rev_id: 2, + }, + InsertNode { + path: 1.into(), + node_data: node_data_3.clone(), + rev_id: 1, + }, + AssertNode { + path: 2.into(), + expected: Some(node_data_3.into()), + }, + ]; + test.run_scripts(scripts); +} + +#[test] +fn operation_delete_transform_path_test() { let mut test = NodeTest::new(); let node_data_1 = NodeDataBuilder::new("text_1").build(); let node_data_2 = NodeDataBuilder::new("text_2").build(); @@ -167,7 +209,6 @@ fn operation_delete_transform_test() { node_data: node_data_3.clone(), rev_id: 3, }, - // // 0: text_1 // 1: text_3 // 2: text_2 @@ -178,6 +219,10 @@ fn operation_delete_transform_test() { path: 1.into(), rev_id: 3, }, + // After perform the delete action, the tree will be: + // 0: text_1 + // 1: text_3 + AssertNumberOfNodesAtPath { path: None, len: 2 }, AssertNode { path: 1.into(), expected: Some(node_3), @@ -186,7 +231,6 @@ fn operation_delete_transform_test() { path: 2.into(), expected: None, }, - AssertNumberOfNodesAtPath { path: None, len: 2 }, ]; test.run_scripts(scripts); } diff --git a/shared-lib/lib-ot/tests/node/script.rs b/shared-lib/lib-ot/tests/node/script.rs index 2ebc7c9fba..7c5f8f0515 100644 --- a/shared-lib/lib-ot/tests/node/script.rs +++ b/shared-lib/lib-ot/tests/node/script.rs @@ -149,7 +149,7 @@ impl NodeTest { 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 { + for rev_id in rev_id..=self.rev_id { let old_transaction = self.rev_operations.get(&rev_id).unwrap(); old_transaction.transform(transaction); } diff --git a/shared-lib/lib-ot/tests/node/tree_test.rs b/shared-lib/lib-ot/tests/node/tree_test.rs index 90427457d0..56314585b1 100644 --- a/shared-lib/lib-ot/tests/node/tree_test.rs +++ b/shared-lib/lib-ot/tests/node/tree_test.rs @@ -101,8 +101,6 @@ fn node_insert_node_in_ordered_nodes_test() { let path_3: Path = 2.into(); let node_3 = NodeData::new("text_3"); - let path_4: Path = 3.into(); - let scripts = vec![ InsertNode { path: path_1.clone(), @@ -119,13 +117,18 @@ fn node_insert_node_in_ordered_nodes_test() { node_data: node_3.clone(), rev_id: 3, }, - // 0:note_1 , 1: note_2_1, 2: note_3 + // 0:text_1 + // 1:text_2_1 + // 2:text_3 InsertNode { path: path_2.clone(), node_data: node_2_2.clone(), rev_id: 4, }, - // 0:note_1 , 1:note_2_2, 2: note_2_1, 3: note_3 + // 0:text_1 + // 1:text_2_2 + // 2:text_2_1 + // 3:text_3 AssertNodeData { path: path_1, expected: Some(node_1), @@ -138,15 +141,105 @@ fn node_insert_node_in_ordered_nodes_test() { path: path_3, expected: Some(node_2_1), }, - AssertNodeData { - path: path_4, - expected: Some(node_3), - }, AssertNumberOfNodesAtPath { path: None, len: 4 }, ]; test.run_scripts(scripts); } +#[test] +fn node_insert_nested_nodes_test() { + let mut test = NodeTest::new(); + let node_data_1_1 = NodeDataBuilder::new("text_1_1").build(); + let node_data_1_2 = NodeDataBuilder::new("text_1_2").build(); + let node_data_1 = NodeDataBuilder::new("text_1") + .add_node(node_data_1_1.clone()) + .add_node(node_data_1_2.clone()) + .build(); + + let node_data_2_1 = NodeDataBuilder::new("text_2_1").build(); + let node_data_2_2 = NodeDataBuilder::new("text_2_2").build(); + let node_data_2 = NodeDataBuilder::new("text_2") + .add_node(node_data_2_1.clone()) + .add_node(node_data_2_2.clone()) + .build(); + + let scripts = vec![ + InsertNode { + path: 0.into(), + node_data: node_data_1.clone(), + rev_id: 1, + }, + InsertNode { + path: 1.into(), + node_data: node_data_2.clone(), + rev_id: 2, + }, + // the tree will be: + // 0:text_1 + // 0:text_1_1 + // 1:text_1_2 + // 1:text_2 + // 0:text_2_1 + // 1:text_2_2 + AssertNode { + path: vec![0, 0].into(), + expected: Some(node_data_1_1.into()), + }, + AssertNode { + path: vec![0, 1].into(), + expected: Some(node_data_1_2.into()), + }, + AssertNode { + path: vec![1, 0].into(), + expected: Some(node_data_2_1.into()), + }, + AssertNode { + path: vec![1, 1].into(), + expected: Some(node_data_2_2.into()), + }, + ]; + test.run_scripts(scripts); +} + +#[test] +fn node_insert_node_before_existing_nested_nodes_test() { + let mut test = NodeTest::new(); + let node_data_1_1 = NodeDataBuilder::new("text_1_1").build(); + let node_data_1_2 = NodeDataBuilder::new("text_1_2").build(); + let node_data_1 = NodeDataBuilder::new("text_1") + .add_node(node_data_1_1.clone()) + .add_node(node_data_1_2.clone()) + .build(); + + let scripts = vec![ + InsertNode { + path: 0.into(), + node_data: node_data_1.clone(), + rev_id: 1, + }, + // 0:text_1 + // 0:text_1_1 + // 1:text_1_2 + InsertNode { + path: 0.into(), + node_data: NodeDataBuilder::new("text_0").build(), + rev_id: 2, + }, + // 0:text_0 + // 1:text_1 + // 0:text_1_1 + // 1:text_1_2 + AssertNode { + path: vec![1, 0].into(), + expected: Some(node_data_1_1.into()), + }, + AssertNode { + path: vec![1, 1].into(), + expected: Some(node_data_1_2.into()), + }, + ]; + test.run_scripts(scripts); +} #[test] fn node_insert_with_attributes_test() { let mut test = NodeTest::new();