From b0bafff22c92c5016e99f90cef32826dd7b04c89 Mon Sep 17 00:00:00 2001 From: Vincent Chan Date: Mon, 22 Aug 2022 16:46:24 +0800 Subject: [PATCH] feat: introduce error to apply method --- frontend/rust-lib/Cargo.lock | 7 + .../lib-ot/src/core/document/document.rs | 120 +++++++++++----- .../src/core/document/document_operation.rs | 136 +++++++++--------- shared-lib/lib-ot/src/core/document/node.rs | 9 +- .../lib-ot/src/core/document/transaction.rs | 18 ++- shared-lib/lib-ot/src/errors.rs | 3 +- shared-lib/lib-ot/tests/main.rs | 34 +++-- 7 files changed, 193 insertions(+), 134 deletions(-) diff --git a/frontend/rust-lib/Cargo.lock b/frontend/rust-lib/Cargo.lock index 7534492ab0..4119cc1edf 100644 --- a/frontend/rust-lib/Cargo.lock +++ b/frontend/rust-lib/Cargo.lock @@ -1618,6 +1618,12 @@ dependencies = [ "serde", ] +[[package]] +name = "indextree" +version = "4.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "42b4b46b3311ebd8e5cd44f6b03b36e0f48a70552cf6b036afcebc5626794066" + [[package]] name = "instant" version = "0.1.12" @@ -1766,6 +1772,7 @@ dependencies = [ "bytes", "dashmap", "derive_more", + "indextree", "lazy_static", "log", "md5", diff --git a/shared-lib/lib-ot/src/core/document/document.rs b/shared-lib/lib-ot/src/core/document/document.rs index 07626e1d75..063fe408b0 100644 --- a/shared-lib/lib-ot/src/core/document/document.rs +++ b/shared-lib/lib-ot/src/core/document/document.rs @@ -1,8 +1,6 @@ use crate::core::document::position::Position; -use crate::core::{ - DeleteOperation, DocumentOperation, InsertOperation, NodeAttributes, NodeData, TextEditOperation, Transaction, - UpdateOperation, -}; +use crate::core::{DocumentOperation, NodeAttributes, NodeData, OperationTransform, TextDelta, Transaction}; +use crate::errors::{ErrorBuilder, OTError, OTErrorCode}; use indextree::{Arena, NodeId}; pub struct DocumentTree { @@ -86,42 +84,48 @@ impl DocumentTree { None } - pub fn apply(&mut self, transaction: Transaction) { + pub fn apply(&mut self, transaction: Transaction) -> Result<(), OTError> { for op in &transaction.operations { - self.apply_op(op); + self.apply_op(op)?; } + Ok(()) } - fn apply_op(&mut self, op: &DocumentOperation) { + fn apply_op(&mut self, op: &DocumentOperation) -> Result<(), OTError> { match op { - DocumentOperation::Insert(op) => self.apply_insert(op), - DocumentOperation::Update(op) => self.apply_update(op), - DocumentOperation::Delete(op) => self.apply_delete(op), - DocumentOperation::TextEdit(op) => self.apply_text_edit(op), + DocumentOperation::Insert { path, nodes } => self.apply_insert(path, nodes), + DocumentOperation::Update { path, attributes, .. } => self.apply_update(path, attributes), + DocumentOperation::Delete { path, nodes } => self.apply_delete(path, nodes.len()), + DocumentOperation::TextEdit { path, delta, .. } => self.apply_text_edit(path, delta), } } - fn apply_insert(&mut self, op: &InsertOperation) { - let parent_path = &op.path.0[0..(op.path.0.len() - 1)]; - let last_index = op.path.0[op.path.0.len() - 1]; - let parent_node = self.node_at_path(&Position(parent_path.to_vec())); - if let Some(parent_node) = parent_node { - let mut inserted_nodes = Vec::new(); + fn apply_insert(&mut self, path: &Position, nodes: &Vec) -> Result<(), OTError> { + let parent_path = &path.0[0..(path.0.len() - 1)]; + let last_index = path.0[path.0.len() - 1]; + let parent_node = self + .node_at_path(&Position(parent_path.to_vec())) + .ok_or(ErrorBuilder::new(OTErrorCode::PathNotFound).build())?; + let mut inserted_nodes = Vec::new(); - for node in &op.nodes { - inserted_nodes.push(self.arena.new_node(node.clone())); - } - - self.insert_child_at_index(parent_node, last_index, &inserted_nodes); + for node in nodes { + inserted_nodes.push(self.arena.new_node(node.clone())); } + + self.insert_child_at_index(parent_node, last_index, &inserted_nodes) } - fn insert_child_at_index(&mut self, parent: NodeId, index: usize, insert_children: &[NodeId]) { + fn insert_child_at_index( + &mut self, + parent: NodeId, + index: usize, + insert_children: &[NodeId], + ) -> Result<(), OTError> { if index == 0 && parent.children(&self.arena).next().is_none() { for id in insert_children { parent.append(*id, &mut self.arena); } - return; + return Ok(()); } let children_length = parent.children(&self.arena).fold(0, |counter, _| counter + 1); @@ -130,32 +134,72 @@ impl DocumentTree { for id in insert_children { parent.append(*id, &mut self.arena); } - return; + return Ok(()); } - let node_to_insert = self.child_at_index_of_path(parent, index).unwrap(); + let node_to_insert = self + .child_at_index_of_path(parent, index) + .ok_or(ErrorBuilder::new(OTErrorCode::PathNotFound).build())?; for id in insert_children { node_to_insert.insert_before(*id, &mut self.arena); } + Ok(()) } - fn apply_update(&self, op: &UpdateOperation) { - let update_node = self.node_at_path(&op.path).unwrap(); - let node_data = self.arena.get(update_node).unwrap(); - let new_attributes = { - let old_attributes = node_data.get().attributes.borrow(); - NodeAttributes::compose(&old_attributes, &op.attributes) + fn apply_update(&mut self, path: &Position, attributes: &NodeAttributes) -> Result<(), OTError> { + let update_node = self + .node_at_path(path) + .ok_or(ErrorBuilder::new(OTErrorCode::PathNotFound).build())?; + let node_data = self.arena.get_mut(update_node).unwrap(); + // let new_node = NodeData { + // ..node_data.get().clone() + // attributes: + // }; + let new_node = { + let old_attributes = &node_data.get().attributes; + let new_attributes = NodeAttributes::compose(&old_attributes, attributes); + NodeData { + attributes: new_attributes, + ..node_data.get().clone() + } }; - node_data.get().attributes.replace(new_attributes); + *node_data.get_mut() = new_node; + Ok(()) } - fn apply_delete(&mut self, op: &DeleteOperation) { - let update_node = self.node_at_path(&op.path).unwrap(); - update_node.remove_subtree(&mut self.arena); + fn apply_delete(&mut self, path: &Position, len: usize) -> Result<(), OTError> { + let mut update_node = self + .node_at_path(path) + .ok_or(ErrorBuilder::new(OTErrorCode::PathNotFound).build())?; + for _ in 0..len { + let next = update_node.following_siblings(&self.arena).next(); + update_node.remove_subtree(&mut self.arena); + if let Some(next_id) = next { + update_node = next_id; + } else { + break; + } + } + Ok(()) } - fn apply_text_edit(&self, _op: &TextEditOperation) { - unimplemented!() + fn apply_text_edit(&mut self, path: &Position, delta: &TextDelta) -> Result<(), OTError> { + let edit_node = self + .node_at_path(path) + .ok_or(ErrorBuilder::new(OTErrorCode::PathNotFound).build())?; + let node_data = self.arena.get_mut(edit_node).unwrap(); + let new_delta = if let Some(old_delta) = &node_data.get().delta { + Some(old_delta.compose(delta)?) + } else { + None + }; + if let Some(new_delta) = new_delta { + *node_data.get_mut() = NodeData { + delta: Some(new_delta), + ..node_data.get().clone() + }; + }; + Ok(()) } } diff --git a/shared-lib/lib-ot/src/core/document/document_operation.rs b/shared-lib/lib-ot/src/core/document/document_operation.rs index 5a19f45a75..0ce16ae89c 100644 --- a/shared-lib/lib-ot/src/core/document/document_operation.rs +++ b/shared-lib/lib-ot/src/core/document/document_operation.rs @@ -3,102 +3,98 @@ use crate::core::{NodeAttributes, NodeData, TextDelta}; #[derive(Clone)] pub enum DocumentOperation { - Insert(InsertOperation), - Update(UpdateOperation), - Delete(DeleteOperation), - TextEdit(TextEditOperation), + Insert { + path: Position, + nodes: Vec, + }, + Update { + path: Position, + attributes: NodeAttributes, + old_attributes: NodeAttributes, + }, + Delete { + path: Position, + nodes: Vec, + }, + TextEdit { + path: Position, + delta: TextDelta, + inverted: TextDelta, + }, } impl DocumentOperation { pub fn path(&self) -> &Position { match self { - DocumentOperation::Insert(insert) => &insert.path, - DocumentOperation::Update(update) => &update.path, - DocumentOperation::Delete(delete) => &delete.path, - DocumentOperation::TextEdit(text_edit) => &text_edit.path, + DocumentOperation::Insert { path, .. } => path, + DocumentOperation::Update { path, .. } => path, + DocumentOperation::Delete { path, .. } => path, + DocumentOperation::TextEdit { path, .. } => path, } } pub fn invert(&self) -> DocumentOperation { match self { - DocumentOperation::Insert(insert_operation) => DocumentOperation::Delete(DeleteOperation { - path: insert_operation.path.clone(), - nodes: insert_operation.nodes.clone(), - }), - DocumentOperation::Update(update_operation) => DocumentOperation::Update(UpdateOperation { - path: update_operation.path.clone(), - attributes: update_operation.old_attributes.clone(), - old_attributes: update_operation.attributes.clone(), - }), - DocumentOperation::Delete(delete_operation) => DocumentOperation::Insert(InsertOperation { - path: delete_operation.path.clone(), - nodes: delete_operation.nodes.clone(), - }), - DocumentOperation::TextEdit(text_edit_operation) => DocumentOperation::TextEdit(TextEditOperation { - path: text_edit_operation.path.clone(), - delta: text_edit_operation.inverted.clone(), - inverted: text_edit_operation.delta.clone(), - }), + DocumentOperation::Insert { path, nodes } => DocumentOperation::Delete { + path: path.clone(), + nodes: nodes.clone(), + }, + DocumentOperation::Update { + path, + attributes, + old_attributes, + } => DocumentOperation::Update { + path: path.clone(), + attributes: old_attributes.clone(), + old_attributes: attributes.clone(), + }, + DocumentOperation::Delete { path, nodes } => DocumentOperation::Insert { + path: path.clone(), + nodes: nodes.clone(), + }, + DocumentOperation::TextEdit { path, delta, inverted } => DocumentOperation::TextEdit { + path: path.clone(), + delta: inverted.clone(), + inverted: delta.clone(), + }, } } pub fn clone_with_new_path(&self, path: Position) -> DocumentOperation { match self { - DocumentOperation::Insert(insert) => DocumentOperation::Insert(InsertOperation { + DocumentOperation::Insert { nodes, .. } => DocumentOperation::Insert { path, - nodes: insert.nodes.clone(), - }), - DocumentOperation::Update(update) => DocumentOperation::Update(UpdateOperation { + nodes: nodes.clone(), + }, + DocumentOperation::Update { + attributes, + old_attributes, + .. + } => DocumentOperation::Update { path, - attributes: update.attributes.clone(), - old_attributes: update.old_attributes.clone(), - }), - DocumentOperation::Delete(delete) => DocumentOperation::Delete(DeleteOperation { + attributes: attributes.clone(), + old_attributes: old_attributes.clone(), + }, + DocumentOperation::Delete { nodes, .. } => DocumentOperation::Delete { path, - nodes: delete.nodes.clone(), - }), - DocumentOperation::TextEdit(text_edit) => DocumentOperation::TextEdit(TextEditOperation { + nodes: nodes.clone(), + }, + DocumentOperation::TextEdit { delta, inverted, .. } => DocumentOperation::TextEdit { path, - delta: text_edit.delta.clone(), - inverted: text_edit.inverted.clone(), - }), + delta: delta.clone(), + inverted: inverted.clone(), + }, } } pub fn transform(a: &DocumentOperation, b: &DocumentOperation) -> DocumentOperation { match a { - DocumentOperation::Insert(insert_op) => { - let new_path = Position::transform(a.path(), b.path(), insert_op.nodes.len() as i64); + DocumentOperation::Insert { path: a_path, nodes } => { + let new_path = Position::transform(a_path, b.path(), nodes.len() as i64); b.clone_with_new_path(new_path) } - DocumentOperation::Delete(delete_op) => { - let new_path = Position::transform(a.path(), b.path(), delete_op.nodes.len() as i64); + DocumentOperation::Delete { path: a_path, nodes } => { + let new_path = Position::transform(a_path, b.path(), nodes.len() as i64); b.clone_with_new_path(new_path) } _ => b.clone(), } } } - -#[derive(Clone)] -pub struct InsertOperation { - pub path: Position, - pub nodes: Vec, -} - -#[derive(Clone)] -pub struct UpdateOperation { - pub path: Position, - pub attributes: NodeAttributes, - pub old_attributes: NodeAttributes, -} - -#[derive(Clone)] -pub struct DeleteOperation { - pub path: Position, - pub nodes: Vec, -} - -#[derive(Clone)] -pub struct TextEditOperation { - pub path: Position, - pub delta: TextDelta, - pub inverted: TextDelta, -} diff --git a/shared-lib/lib-ot/src/core/document/node.rs b/shared-lib/lib-ot/src/core/document/node.rs index a3ec5e729a..e7c5e0e68f 100644 --- a/shared-lib/lib-ot/src/core/document/node.rs +++ b/shared-lib/lib-ot/src/core/document/node.rs @@ -1,19 +1,18 @@ use crate::core::{NodeAttributes, TextDelta}; -use std::cell::RefCell; #[derive(Clone)] pub struct NodeData { pub node_type: String, - pub attributes: RefCell, - pub delta: RefCell>, + pub attributes: NodeAttributes, + pub delta: Option, } impl NodeData { pub fn new(node_type: &str) -> NodeData { NodeData { node_type: node_type.into(), - attributes: RefCell::new(NodeAttributes::new()), - delta: RefCell::new(None), + attributes: NodeAttributes::new(), + delta: None, } } } diff --git a/shared-lib/lib-ot/src/core/document/transaction.rs b/shared-lib/lib-ot/src/core/document/transaction.rs index 99d5ee8169..00697b7da1 100644 --- a/shared-lib/lib-ot/src/core/document/transaction.rs +++ b/shared-lib/lib-ot/src/core/document/transaction.rs @@ -1,7 +1,5 @@ use crate::core::document::position::Position; -use crate::core::{ - DeleteOperation, DocumentOperation, DocumentTree, InsertOperation, NodeAttributes, NodeData, UpdateOperation, -}; +use crate::core::{DocumentOperation, DocumentTree, NodeAttributes, NodeData}; use std::collections::HashMap; pub struct Transaction { @@ -28,10 +26,10 @@ impl<'a> TransactionBuilder<'a> { } pub fn insert_nodes_at_path(&mut self, path: &Position, nodes: &[NodeData]) { - self.push(DocumentOperation::Insert(InsertOperation { + self.push(DocumentOperation::Insert { path: path.clone(), nodes: nodes.to_vec(), - })); + }); } pub fn update_attributes_at_path(&mut self, path: &Position, attributes: HashMap>) { @@ -40,7 +38,7 @@ impl<'a> TransactionBuilder<'a> { let node_data = self.document.arena.get(node).unwrap().get(); for key in attributes.keys() { - let old_attrs = node_data.attributes.borrow(); + let old_attrs = &node_data.attributes; let old_value = match old_attrs.0.get(key.as_str()) { Some(value) => value.clone(), None => None, @@ -48,11 +46,11 @@ impl<'a> TransactionBuilder<'a> { old_attributes.insert(key.clone(), old_value); } - self.push(DocumentOperation::Update(UpdateOperation { + self.push(DocumentOperation::Update { path: path.clone(), attributes: NodeAttributes(attributes), old_attributes: NodeAttributes(old_attributes), - })) + }) } pub fn delete_node_at_path(&mut self, path: &Position) { @@ -69,10 +67,10 @@ impl<'a> TransactionBuilder<'a> { node = node.following_siblings(&self.document.arena).next().unwrap(); } - self.operations.push(DocumentOperation::Delete(DeleteOperation { + self.operations.push(DocumentOperation::Delete { path: path.clone(), nodes: deleted_nodes, - })) + }) } pub fn push(&mut self, op: DocumentOperation) { diff --git a/shared-lib/lib-ot/src/errors.rs b/shared-lib/lib-ot/src/errors.rs index e7aea28bcf..eb313c784f 100644 --- a/shared-lib/lib-ot/src/errors.rs +++ b/shared-lib/lib-ot/src/errors.rs @@ -60,7 +60,7 @@ impl std::convert::From for OTError { } } -#[derive(Debug, Clone)] +#[derive(Debug, Clone, Eq, PartialEq)] pub enum OTErrorCode { IncompatibleLength, ApplyInsertFail, @@ -74,6 +74,7 @@ pub enum OTErrorCode { DuplicatedRevision, RevisionIDConflict, Internal, + PathNotFound, } pub struct ErrorBuilder { diff --git a/shared-lib/lib-ot/tests/main.rs b/shared-lib/lib-ot/tests/main.rs index 1595c20f29..7daf74e957 100644 --- a/shared-lib/lib-ot/tests/main.rs +++ b/shared-lib/lib-ot/tests/main.rs @@ -1,4 +1,5 @@ use lib_ot::core::{DocumentTree, NodeData, Position, TransactionBuilder}; +use lib_ot::errors::OTErrorCode; use std::collections::HashMap; #[test] @@ -15,7 +16,7 @@ fn test_documents() { tb.insert_nodes_at_path(&vec![0].into(), &vec![NodeData::new("text")]); tb.finalize() }; - document.apply(transaction); + document.apply(transaction).unwrap(); assert!(document.node_at_path(&vec![0].into()).is_some()); let node = document.node_at_path(&vec![0].into()).unwrap(); @@ -30,14 +31,14 @@ fn test_documents() { ); tb.finalize() }; - document.apply(transaction); + document.apply(transaction).unwrap(); let transaction = { let mut tb = TransactionBuilder::new(&document); tb.delete_node_at_path(&vec![0].into()); tb.finalize() }; - document.apply(transaction); + document.apply(transaction).unwrap(); assert!(document.node_at_path(&vec![0].into()).is_none()); } @@ -51,14 +52,14 @@ fn test_inserts_nodes() { tb.insert_nodes_at_path(&vec![2].into(), &vec![NodeData::new("text")]); tb.finalize() }; - document.apply(transaction); + document.apply(transaction).unwrap(); let transaction = { let mut tb = TransactionBuilder::new(&document); tb.insert_nodes_at_path(&vec![1].into(), &vec![NodeData::new("text")]); tb.finalize() }; - document.apply(transaction); + document.apply(transaction).unwrap(); } #[test] @@ -71,18 +72,18 @@ fn test_update_nodes() { tb.insert_nodes_at_path(&vec![2].into(), &vec![NodeData::new("text")]); tb.finalize() }; - document.apply(transaction); + document.apply(transaction).unwrap(); let transaction = { let mut tb = TransactionBuilder::new(&document); tb.update_attributes_at_path(&vec![1].into(), HashMap::from([("bolded".into(), Some("true".into()))])); tb.finalize() }; - document.apply(transaction); + document.apply(transaction).unwrap(); let node = document.node_at_path(&Position(vec![1])).unwrap(); let node_data = document.arena.get(node).unwrap().get(); - let is_bold = node_data.attributes.borrow().0.get("bolded").unwrap().clone(); + let is_bold = node_data.attributes.0.get("bolded").unwrap().clone(); assert_eq!(is_bold.unwrap(), "true"); } @@ -96,15 +97,28 @@ fn test_delete_nodes() { tb.insert_nodes_at_path(&vec![2].into(), &vec![NodeData::new("text")]); tb.finalize() }; - document.apply(transaction); + document.apply(transaction).unwrap(); let transaction = { let mut tb = TransactionBuilder::new(&document); tb.delete_node_at_path(&Position(vec![1])); tb.finalize() }; - document.apply(transaction); + document.apply(transaction).unwrap(); let len = document.root.children(&document.arena).fold(0, |count, _| count + 1); assert_eq!(len, 2); } + +#[test] +fn test_errors() { + let mut document = DocumentTree::new(); + let transaction = { + let mut tb = TransactionBuilder::new(&document); + tb.insert_nodes_at_path(&vec![0].into(), &vec![NodeData::new("text")]); + tb.insert_nodes_at_path(&vec![100].into(), &vec![NodeData::new("text")]); + tb.finalize() + }; + let result = document.apply(transaction); + assert_eq!(result.err().unwrap().code, OTErrorCode::PathNotFound); +}