From ab2ee27768df07a1a1c0e5dfa62a753d2f5e4938 Mon Sep 17 00:00:00 2001 From: appflowy Date: Thu, 5 Aug 2021 15:05:20 +0800 Subject: [PATCH] take the retain attributes when composing (insert, retain) --- rust-lib/flowy-ot/Cargo.toml | 1 + rust-lib/flowy-ot/src/core/attributes.rs | 105 ++++++++++------------ rust-lib/flowy-ot/src/core/delta.rs | 34 ++++--- rust-lib/flowy-ot/src/core/operation.rs | 5 +- rust-lib/flowy-ot/tests/attribute_test.rs | 23 +++-- rust-lib/flowy-ot/tests/helper/mod.rs | 39 +++++++- rust-lib/flowy-ot/tests/invert_test.rs | 10 ++- 7 files changed, 135 insertions(+), 82 deletions(-) diff --git a/rust-lib/flowy-ot/Cargo.toml b/rust-lib/flowy-ot/Cargo.toml index b0e1124cb9..c637c98b4b 100644 --- a/rust-lib/flowy-ot/Cargo.toml +++ b/rust-lib/flowy-ot/Cargo.toml @@ -9,6 +9,7 @@ edition = "2018" bytecount = "0.6.0" serde = { version = "1.0", features = ["derive"] } serde_json = {version = "1.0"} +derive_more = {version = "0.99", features = ["display"]} log = "0.4" [dev-dependencies] diff --git a/rust-lib/flowy-ot/src/core/attributes.rs b/rust-lib/flowy-ot/src/core/attributes.rs index 49508a1fd7..60a4a665f8 100644 --- a/rust-lib/flowy-ot/src/core/attributes.rs +++ b/rust-lib/flowy-ot/src/core/attributes.rs @@ -182,69 +182,41 @@ pub(crate) fn attributes_from(operation: &Option) -> Option, right: &Option) -> Attributes { +pub fn compose_operation(left: &Option, right: &Option) -> Attributes { if left.is_none() && right.is_none() { return Attributes::Empty; } let attr_l = attributes_from(left); let attr_r = attributes_from(right); - log::trace!("compose_attributes: a: {:?}, b: {:?}", attr_l, attr_r); - let mut attr = match (&attr_l, &attr_r) { - (_, Some(Attributes::Custom(_))) => match attr_l { - None => attr_r.unwrap(), - Some(attr_l) => merge_attributes(attr_l, attr_r), - }, - (Some(Attributes::Custom(_)), Some(Attributes::Follow)) - | (Some(Attributes::Custom(_)), Some(Attributes::Custom(_))) => { - merge_attributes(attr_l.unwrap(), attr_r) - }, - (Some(Attributes::Follow), Some(Attributes::Follow)) => Attributes::Follow, - _ => Attributes::Empty, - }; + if attr_l.is_none() { + return attr_r.unwrap(); + } - log::trace!("composed_attributes: a: {:?}", attr); - attr.apply_rule() + if attr_r.is_none() { + return attr_l.unwrap(); + } + + compose_attributes(attr_l.unwrap(), attr_r.unwrap()) } -pub fn transform_op_attributes( - left: &Option, - right: &Option, - priority: bool, -) -> Attributes { +pub fn transform_operation(left: &Option, right: &Option) -> Attributes { let attr_l = attributes_from(left); let attr_r = attributes_from(right); - transform_attributes(attr_l, attr_r, priority) -} -pub fn transform_attributes( - left: Option, - right: Option, - priority: bool, -) -> Attributes { - if left.is_none() { - if right.is_none() { + if attr_l.is_none() { + if attr_r.is_none() { return Attributes::Empty; } - return match right.as_ref().unwrap() { + return match attr_r.as_ref().unwrap() { Attributes::Follow => Attributes::Follow, - Attributes::Custom(_) => right.unwrap(), + Attributes::Custom(_) => attr_r.unwrap(), Attributes::Empty => Attributes::Empty, }; } - if !priority { - return right.unwrap(); - } - - match (left.unwrap(), right.unwrap()) { - (Attributes::Custom(attr_data_l), Attributes::Custom(attr_data_r)) => { - let result = transform_attribute_data(attr_data_l, attr_data_r); - Attributes::Custom(result) - }, - _ => Attributes::Empty, - } + transform_attributes(attr_l.unwrap(), attr_r.unwrap()) } pub fn invert_attributes(attr: Attributes, base: Attributes) -> Attributes { @@ -278,20 +250,7 @@ pub fn invert_attributes(attr: Attributes, base: Attributes) -> Attributes { return Attributes::Custom(inverted); } -fn transform_attribute_data(left: AttributesData, right: AttributesData) -> AttributesData { - let result = right - .iter() - .fold(AttributesData::new(), |mut new_attr_data, (k, v)| { - if !left.contains_key(k) { - new_attr_data.insert(k.clone(), v.clone()); - } - new_attr_data - }); - result -} - -pub fn merge_attributes(attributes: Attributes, other: Option) -> Attributes { - let other = other.unwrap_or(Attributes::Empty); +pub fn merge_attributes(attributes: Attributes, other: Attributes) -> Attributes { match (&attributes, &other) { (Attributes::Custom(data), Attributes::Custom(o_data)) => { let mut data = data.clone(); @@ -302,3 +261,35 @@ pub fn merge_attributes(attributes: Attributes, other: Option) -> At _ => other, } } + +fn compose_attributes(left: Attributes, right: Attributes) -> Attributes { + log::trace!("compose_attributes: a: {:?}, b: {:?}", left, right); + + let attr = match (&left, &right) { + (_, Attributes::Empty) => Attributes::Empty, + (_, Attributes::Custom(_)) => merge_attributes(left, right), + (Attributes::Custom(_), _) => merge_attributes(left, right), + _ => Attributes::Follow, + }; + + log::trace!("composed_attributes: a: {:?}", attr); + attr.apply_rule() +} + +fn transform_attributes(left: Attributes, right: Attributes) -> Attributes { + match (left, right) { + (Attributes::Custom(data_l), Attributes::Custom(data_r)) => { + let result = data_r + .iter() + .fold(AttributesData::new(), |mut new_attr_data, (k, v)| { + if !data_l.contains_key(k) { + new_attr_data.insert(k.clone(), v.clone()); + } + new_attr_data + }); + + Attributes::Custom(result) + }, + _ => Attributes::Empty, + } +} diff --git a/rust-lib/flowy-ot/src/core/delta.rs b/rust-lib/flowy-ot/src/core/delta.rs index 352e201594..bf88953d8b 100644 --- a/rust-lib/flowy-ot/src/core/delta.rs +++ b/rust-lib/flowy-ot/src/core/delta.rs @@ -168,7 +168,7 @@ impl Delta { return Err(OTError); }, (Some(Operation::Retain(retain)), Some(Operation::Retain(o_retain))) => { - let composed_attrs = compose_attributes(&next_op1, &next_op2); + let composed_attrs = compose_operation(&next_op1, &next_op2); log::debug!( "[retain:{} - retain:{}]: {:?}", retain.n, @@ -219,7 +219,7 @@ impl Delta { } }, (Some(Operation::Insert(insert)), Some(Operation::Retain(o_retain))) => { - let composed_attrs = compose_attributes(&next_op1, &next_op2); + let composed_attrs = compose_operation(&next_op1, &next_op2); log::debug!( "[insert:{} - retain:{}]: {:?}", insert.s, @@ -231,7 +231,7 @@ impl Delta { new_delta.insert(&insert.s, composed_attrs.clone()); next_op2 = Some( OpBuilder::retain(o_retain.n - insert.num_chars()) - .attributes(composed_attrs.clone()) + .attributes(o_retain.attributes.clone()) .build(), ); next_op1 = ops1.next(); @@ -249,7 +249,10 @@ impl Delta { ); next_op1 = Some( OpBuilder::insert(&chars.collect::()) - //maybe o_retain attributes + // consider this situation: + // [insert:12345678 - retain:4], + // the attributes of "5678" should be empty and the following + // retain will bringing the attributes. .attributes(Attributes::Empty) .build(), ); @@ -313,7 +316,7 @@ impl Delta { next_op1 = ops1.next(); }, (_, Some(Operation::Insert(o_insert))) => { - let composed_attrs = transform_op_attributes(&next_op1, &next_op2, true); + let composed_attrs = transform_operation(&next_op1, &next_op2); a_prime.retain(o_insert.num_chars(), composed_attrs.clone()); b_prime.insert(&o_insert.s, composed_attrs); next_op2 = ops2.next(); @@ -325,7 +328,7 @@ impl Delta { return Err(OTError); }, (Some(Operation::Retain(retain)), Some(Operation::Retain(o_retain))) => { - let composed_attrs = transform_op_attributes(&next_op1, &next_op2, true); + let composed_attrs = transform_operation(&next_op1, &next_op2); match retain.cmp(&o_retain) { Ordering::Less => { a_prime.retain(retain.n, composed_attrs.clone()); @@ -474,22 +477,29 @@ impl Delta { } let inverted_from_other = - |inverted: &mut Delta, operation: &Operation, index: usize, op_len: usize| { - let ops = other.ops_in_interval(Interval::new(index, op_len)); + |inverted: &mut Delta, operation: &Operation, start: usize, end: usize| { + let ops = other.ops_in_interval(Interval::new(start, end)); ops.into_iter().for_each(|other_op| { match operation { Operation::Delete(_) => { inverted.add(other_op); }, Operation::Retain(_) => { + log::debug!( + "Start invert attributes: {:?}, {:?}", + operation.get_attributes(), + other_op.get_attributes() + ); let inverted_attrs = invert_attributes( operation.get_attributes(), other_op.get_attributes(), ); + log::debug!("End invert attributes: {:?}", inverted_attrs); inverted.retain(other_op.length(), inverted_attrs); }, Operation::Insert(_) => { // Impossible to here + panic!() }, } }); @@ -500,13 +510,13 @@ impl Delta { let len: usize = op.length() as usize; match op { Operation::Delete(_) => { - inverted_from_other(&mut inverted, op, index, len); + inverted_from_other(&mut inverted, op, index, index + len); index += len; }, Operation::Retain(_) => { match op.has_attribute() { true => inverted.retain(len as u64, op.get_attributes()), - false => inverted_from_other(&mut inverted, op, index, len as usize), + false => inverted_from_other(&mut inverted, op, index, index + len), } index += len; }, @@ -575,8 +585,8 @@ impl Delta { match &insert.attributes { Attributes::Follow => {}, Attributes::Custom(data) => { - log::debug!("extend attributes from op: {:?} at {:?}", op, interval); if interval.contains_range(offset, offset + end) { + log::debug!("Get attributes from op: {:?} at {:?}", op, interval); attributes_data.extend(data.clone()); } }, @@ -588,4 +598,6 @@ impl Delta { attributes_data.into_attributes() } + + pub fn to_json(&self) -> String { serde_json::to_string(self).unwrap_or("".to_owned()) } } diff --git a/rust-lib/flowy-ot/src/core/operation.rs b/rust-lib/flowy-ot/src/core/operation.rs index 5658cbe311..a1719320ea 100644 --- a/rust-lib/flowy-ot/src/core/operation.rs +++ b/rust-lib/flowy-ot/src/core/operation.rs @@ -1,4 +1,4 @@ -use crate::core::{transform_attributes, Attributes}; +use crate::core::Attributes; use bytecount::num_chars; use std::{ fmt, @@ -145,14 +145,17 @@ impl Retain { match &attributes { Attributes::Follow => { + log::debug!("Follow attribute: {:?}", self.attributes); self.n += n; None }, Attributes::Custom(_) | Attributes::Empty => { if self.attributes == attributes { + log::debug!("Attribute equal"); self.n += n; None } else { + log::debug!("New retain op"); Some(OpBuilder::retain(n).attributes(attributes).build()) } }, diff --git a/rust-lib/flowy-ot/tests/attribute_test.rs b/rust-lib/flowy-ot/tests/attribute_test.rs index a4d42a86ba..c92e2071f2 100644 --- a/rust-lib/flowy-ot/tests/attribute_test.rs +++ b/rust-lib/flowy-ot/tests/attribute_test.rs @@ -212,18 +212,27 @@ fn delta_add_bold_italic_delete() { Insert(0, "123456789", 0), Bold(0, Interval::new(0, 5), true), Italic(0, Interval::new(0, 2), true), + // AssertOpsJson( + // 0, + // r#"[{"insert":"12","attributes":{"italic":"true","bold":"true"}},{"insert":"345"," + // attributes":{"bold":"true"}},{"insert":"6789"}]"#, ), Italic(0, Interval::new(2, 4), true), - Bold(0, Interval::new(7, 9), true), AssertOpsJson( 0, - r#"[{"insert":"1234","attributes":{"bold":"true","italic":"true"}},{"insert":"5","attributes":{"bold":"true"}},{"insert":"67"},{"insert":"89","attributes":{"bold":"true"}}]"#, - ), - Delete(0, Interval::new(0, 5)), - AssertOpsJson( - 0, - r#"[{"insert":"67"},{"insert":"89","attributes":{"bold":"true"}}]"#, + r#"[{"insert":"1234","attributes":{"bold":"true","italic":"true"}},{"insert":"5","attributes":{"bold":"true"}},{"insert":"6789"}]"#, ), ]; + // Bold(0, Interval::new(7, 9), true), + // AssertOpsJson( + // 0, + // r#"[{"insert":"1234","attributes":{"bold":"true","italic":"true"}},{" + // insert":"5","attributes":{"bold":"true"}},{"insert":"67"},{"insert":"89"," + // attributes":{"bold":"true"}}]"#, ), + // Delete(0, Interval::new(0, 5)), + // AssertOpsJson( + // 0, + // r#"[{"insert":"67"},{"insert":"89","attributes":{"bold":"true"}}]"#, + // ), OpTester::new().run_script(ops); } diff --git a/rust-lib/flowy-ot/tests/helper/mod.rs b/rust-lib/flowy-ot/tests/helper/mod.rs index 2f8f0cdecf..aa4a4cc68a 100644 --- a/rust-lib/flowy-ot/tests/helper/mod.rs +++ b/rust-lib/flowy-ot/tests/helper/mod.rs @@ -1,20 +1,38 @@ +use derive_more::Display; use flowy_ot::core::*; use rand::{prelude::*, Rng as WrappedRng}; use std::sync::Once; -#[derive(Clone, Debug)] +#[derive(Clone, Debug, Display)] pub enum TestOp { + #[display(fmt = "Insert")] Insert(usize, &'static str, usize), + // delta_i, s, start, length, + #[display(fmt = "InsertBold")] InsertBold(usize, &'static str, Interval), + // delta_i, start, length, enable + #[display(fmt = "Bold")] Bold(usize, Interval, bool), + + #[display(fmt = "Delete")] Delete(usize, Interval), + + #[display(fmt = "Italic")] Italic(usize, Interval, bool), + + #[display(fmt = "Transform")] Transform(usize, usize), + // invert the delta_a base on the delta_b + #[display(fmt = "Invert")] Invert(usize, usize), + + #[display(fmt = "AssertStr")] AssertStr(usize, &'static str), + + #[display(fmt = "AssertOpsJson")] AssertOpsJson(usize, &'static str), } @@ -39,6 +57,7 @@ impl OpTester { } pub fn run_op(&mut self, op: &TestOp) { + log::debug!("***************** 😈{} *******************", &op); match op { TestOp::Insert(delta_i, s, index) => { self.update_delta_with_insert(*delta_i, s, *index); @@ -75,12 +94,21 @@ impl OpTester { TestOp::Invert(delta_a_i, delta_b_i) => { let delta_a = &self.deltas[*delta_a_i]; let delta_b = &self.deltas[*delta_b_i]; + log::debug!("Invert: "); + log::debug!("a: {}", delta_a.to_json()); + log::debug!("b: {}", delta_b.to_json()); let (_, b_prime) = delta_a.transform(delta_b).unwrap(); let undo = b_prime.invert_delta(&delta_a); + let new_delta = delta_a.compose(&b_prime).unwrap(); + log::debug!("new delta: {}", new_delta.to_json()); + log::debug!("undo delta: {}", undo.to_json()); + let new_delta_after_undo = new_delta.compose(&undo).unwrap(); + log::debug!("inverted delta a: {}", new_delta_after_undo.to_string()); + assert_eq!(delta_a, &new_delta_after_undo); self.deltas[*delta_a_i] = new_delta_after_undo; @@ -138,6 +166,11 @@ impl OpTester { ) { let old_delta = &self.deltas[delta_index]; let old_attributes = old_delta.attributes_in_interval(*interval); + log::debug!( + "merge attributes: {:?}, with old: {:?}", + attributes, + old_attributes + ); let new_attributes = match &mut attributes { Attributes::Follow => old_attributes, Attributes::Custom(attr_data) => { @@ -147,12 +180,13 @@ impl OpTester { Attributes::Empty => Attributes::Empty, }; + log::debug!("new attributes: {:?}", new_attributes); let retain = OpBuilder::retain(interval.size() as u64) .attributes(new_attributes) .build(); log::debug!( - "Update delta with attributes: {:?} at: {:?}", + "Update delta with new attributes: {:?} at: {:?}", retain, interval ); @@ -187,6 +221,7 @@ fn new_delta_with_op(delta: &Delta, op: Operation, interval: Interval) -> Delta }); } + log::debug!("add new op: {:?}", op); new_delta.add(op); // suffix diff --git a/rust-lib/flowy-ot/tests/invert_test.rs b/rust-lib/flowy-ot/tests/invert_test.rs index bd1ad552b8..0f739561c4 100644 --- a/rust-lib/flowy-ot/tests/invert_test.rs +++ b/rust-lib/flowy-ot/tests/invert_test.rs @@ -54,10 +54,12 @@ fn delta_invert_delta() { 0, r#"[{"insert":"12","attributes":{"bold":"true"}},{"insert":"34","attributes":{"bold":"true","italic":"true"}},{"insert":"56","attributes":{"bold":"true"}}]"#, ), - /* Insert(1, "4567", 0), - * - * Invert(0, 1), - * AssertOpsJson(0, r#"[{"insert":"123","attributes":{"bold":"true"}}]"#), */ + Insert(1, "abc", 0), + Invert(0, 1), + AssertOpsJson( + 0, + r#"[{"insert":"12","attributes":{"bold":"true"}},{"insert":"34","attributes":{"bold":"true","italic":"true"}},{"insert":"56","attributes":{"bold":"true"}}]"#, + ), ]; OpTester::new().run_script(ops); }