From ce2ccf7d0a75a434ac697219d7d21483a625f9fd Mon Sep 17 00:00:00 2001 From: appflowy Date: Wed, 3 Nov 2021 13:52:33 +0800 Subject: [PATCH] [rust]: fix delta serial bugs & add some tests --- .../presentation/stack_page/doc/doc_page.dart | 3 +- .../presentation/widgets/menu/menu.dart | 3 +- .../src/services/doc/doc_controller.rs | 4 + .../src/services/doc/document/document.rs | 3 +- .../src/services/doc/revision/model.rs | 4 +- .../src/services/doc/revision/persistence.rs | 13 ++- .../tests/editor/attribute_test.rs | 42 ++++++++- rust-lib/flowy-document/tests/editor/mod.rs | 10 +++ .../flowy-document/tests/editor/serde_test.rs | 17 +++- .../flowy-ot/src/core/attributes/attribute.rs | 14 ++- .../src/core/attributes/attributes_serde.rs | 88 ++++++++++++------- 11 files changed, 152 insertions(+), 49 deletions(-) diff --git a/app_flowy/lib/workspace/presentation/stack_page/doc/doc_page.dart b/app_flowy/lib/workspace/presentation/stack_page/doc/doc_page.dart index 057c698eec..8bdaf0b821 100644 --- a/app_flowy/lib/workspace/presentation/stack_page/doc/doc_page.dart +++ b/app_flowy/lib/workspace/presentation/stack_page/doc/doc_page.dart @@ -41,7 +41,8 @@ class _DocPageState extends State { ], child: BlocBuilder(builder: (context, state) { return state.loadState.map( - loading: (_) => const FlowyProgressIndicator(), + // loading: (_) => const FlowyProgressIndicator(), + loading: (_) => SizedBox.expand(child: Container(color: Colors.white)), finish: (result) => result.successOrFail.fold( (_) { if (state.forceClose) { diff --git a/app_flowy/lib/workspace/presentation/widgets/menu/menu.dart b/app_flowy/lib/workspace/presentation/widgets/menu/menu.dart index e7c7147ee6..9d1b830669 100644 --- a/app_flowy/lib/workspace/presentation/widgets/menu/menu.dart +++ b/app_flowy/lib/workspace/presentation/widgets/menu/menu.dart @@ -4,7 +4,6 @@ import 'package:flowy_infra/size.dart'; import 'package:flowy_infra_ui/style_widget/scrolling/styled_list.dart'; import 'package:flowy_infra_ui/widget/spacing.dart'; import 'package:flowy_sdk/protobuf/flowy-user/user_profile.pb.dart'; -import 'package:flowy_sdk/protobuf/flowy-workspace/app_create.pb.dart'; import 'package:flowy_sdk/protobuf/flowy-workspace/view_create.pb.dart'; import 'package:flutter/material.dart'; import 'package:flutter_bloc/flutter_bloc.dart'; @@ -164,7 +163,6 @@ class MenuSharedState extends ChangeNotifier { super.addListener(() { if (_forcedOpenView != null) { callback(_forcedOpenView!); - _forcedOpenView = null; } }); } @@ -181,6 +179,7 @@ class MenuSharedState extends ChangeNotifier { selectedView = view; notifyListeners(); } + _forcedOpenView = null; } set selectedView(View? view) { diff --git a/rust-lib/flowy-document/src/services/doc/doc_controller.rs b/rust-lib/flowy-document/src/services/doc/doc_controller.rs index ff0536dc6f..0a7bf5063f 100644 --- a/rust-lib/flowy-document/src/services/doc/doc_controller.rs +++ b/rust-lib/flowy-document/src/services/doc/doc_controller.rs @@ -74,6 +74,10 @@ impl DocController { Ok(()) } + // the delta's data that contains attributes with null value will be considered + // as None e.g. + // json : {"retain":7,"attributes":{"bold":null}} + // deserialize delta: [ {retain: 7, attributes: {Bold: AttributeValue(None)}} ] #[tracing::instrument(level = "debug", skip(self, delta), err)] pub(crate) async fn edit_doc(&self, delta: DocDelta) -> Result { let edit_doc_ctx = self.cache.get(&delta.doc_id)?; diff --git a/rust-lib/flowy-document/src/services/doc/document/document.rs b/rust-lib/flowy-document/src/services/doc/document/document.rs index 576302c805..2e4f8a2963 100644 --- a/rust-lib/flowy-document/src/services/doc/document/document.rs +++ b/rust-lib/flowy-document/src/services/doc/document/document.rs @@ -68,6 +68,7 @@ impl Document { } pub fn compose_delta(&mut self, delta: &Delta) -> Result<(), DocError> { + log::trace!("😁 {} compose {}", &self.delta.to_json(), delta.to_json()); let composed_delta = self.delta.compose(delta)?; let mut undo_delta = delta.invert(&self.delta); @@ -88,7 +89,7 @@ impl Document { self.history.record(undo_delta); } - log::trace!("document delta: {}", &composed_delta); + log::trace!("😁😁 compose result: {}", composed_delta.to_json()); self.set_delta(composed_delta); Ok(()) } diff --git a/rust-lib/flowy-document/src/services/doc/revision/model.rs b/rust-lib/flowy-document/src/services/doc/revision/model.rs index 6efa7f1d48..136f833248 100644 --- a/rust-lib/flowy-document/src/services/doc/revision/model.rs +++ b/rust-lib/flowy-document/src/services/doc/revision/model.rs @@ -13,12 +13,12 @@ use tokio::sync::broadcast; pub type RevIdReceiver = broadcast::Receiver; pub type RevIdSender = broadcast::Sender; -pub struct RevisionContext { +pub struct RevisionRecord { pub revision: Revision, pub state: RevState, } -impl RevisionContext { +impl RevisionRecord { pub fn new(revision: Revision) -> Self { Self { revision, diff --git a/rust-lib/flowy-document/src/services/doc/revision/persistence.rs b/rust-lib/flowy-document/src/services/doc/revision/persistence.rs index 07ed273c12..51cc130691 100644 --- a/rust-lib/flowy-document/src/services/doc/revision/persistence.rs +++ b/rust-lib/flowy-document/src/services/doc/revision/persistence.rs @@ -19,10 +19,10 @@ use tokio::{ pub struct RevisionStore { doc_id: String, persistence: Arc, - revs_map: Arc>, + revs_map: Arc>, pending_tx: PendingSender, pending_revs: Arc>>, - delay_save: RwLock>>, + defer_save_oper: RwLock>>, server: Arc, } @@ -45,7 +45,7 @@ impl RevisionStore { revs_map, pending_revs, pending_tx, - delay_save: RwLock::new(None), + defer_save_oper: RwLock::new(None), server, }); @@ -75,7 +75,7 @@ impl RevisionStore { let pending_rev = PendingRevId::new(revision.rev_id, sender); self.pending_revs.write().await.push_back(pending_rev); - self.revs_map.insert(revision.rev_id, RevisionContext::new(revision)); + self.revs_map.insert(revision.rev_id, RevisionRecord::new(revision)); let _ = self.pending_tx.send(PendingMsg::Revision { ret: receiver }); self.save_revisions().await; @@ -94,7 +94,7 @@ impl RevisionStore { } async fn save_revisions(&self) { - if let Some(handler) = self.delay_save.write().await.take() { + if let Some(handler) = self.defer_save_oper.write().await.take() { handler.abort(); } @@ -105,7 +105,7 @@ impl RevisionStore { let revs_map = self.revs_map.clone(); let persistence = self.persistence.clone(); - *self.delay_save.write().await = Some(tokio::spawn(async move { + *self.defer_save_oper.write().await = Some(tokio::spawn(async move { tokio::time::sleep(Duration::from_millis(300)).await; let ids = revs_map.iter().map(|kv| kv.key().clone()).collect::>(); let revisions_state = revs_map @@ -194,7 +194,6 @@ async fn fetch_from_local(doc_id: &str, persistence: Arc) -> DocRes }, } } - Result::::Ok(Doc { id: doc_id, data: delta.to_json(), diff --git a/rust-lib/flowy-document/tests/editor/attribute_test.rs b/rust-lib/flowy-document/tests/editor/attribute_test.rs index b4bbcbcf42..8de8550dda 100644 --- a/rust-lib/flowy-document/tests/editor/attribute_test.rs +++ b/rust-lib/flowy-document/tests/editor/attribute_test.rs @@ -1,6 +1,7 @@ use crate::editor::{TestBuilder, TestOp::*}; use flowy_document::services::doc::{FlowyDoc, PlainDoc}; -use flowy_ot::core::{Interval, NEW_LINE, WHITESPACE}; +use flowy_ot::core::{Delta, DeltaBuilder, Interval, OperationTransformable, NEW_LINE, WHITESPACE}; +use std::str::FromStr; #[test] fn attributes_bold_added() { @@ -736,3 +737,42 @@ fn attributes_preserve_list_format_on_merge() { TestBuilder::new().run_script::(ops); } + +#[test] +fn delta_compose() { + let mut delta = Delta::from_json(r#"[{"insert":"\n"}]"#).unwrap(); + let deltas = vec![ + Delta::from_json(r#"[{"retain":1,"attributes":{"list":"unchecked"}}]"#).unwrap(), + Delta::from_json(r#"[{"insert":"a"}]"#).unwrap(), + Delta::from_json(r#"[{"retain":1},{"insert":"\n","attributes":{"list":"unchecked"}}]"#).unwrap(), + Delta::from_json(r#"[{"retain":2},{"retain":1,"attributes":{"list":""}}]"#).unwrap(), + ]; + + for d in deltas { + delta = delta.compose(&d).unwrap(); + } + assert_eq!( + delta.to_json(), + r#"[{"insert":"a"},{"insert":"\n","attributes":{"list":"unchecked"}},{"insert":"\n"}]"# + ); + + let ops = vec![ + AssertDocJson(0, r#"[{"insert":"\n"}]"#), + Insert(0, "a", 0), + AssertDocJson(0, r#"[{"insert":"a\n"}]"#), + Bullet(0, Interval::new(0, 1), true), + AssertDocJson(0, r#"[{"insert":"a"},{"insert":"\n","attributes":{"list":"bullet"}}]"#), + Insert(0, NEW_LINE, 1), + AssertDocJson( + 0, + r#"[{"insert":"a"},{"insert":"\n\n","attributes":{"list":"bullet"}}]"#, + ), + Insert(0, NEW_LINE, 2), + AssertDocJson( + 0, + r#"[{"insert":"a"},{"insert":"\n","attributes":{"list":"bullet"}},{"insert":"\n"}]"#, + ), + ]; + + TestBuilder::new().run_script::(ops); +} diff --git a/rust-lib/flowy-document/tests/editor/mod.rs b/rust-lib/flowy-document/tests/editor/mod.rs index 981963153b..5f834c1010 100644 --- a/rust-lib/flowy-document/tests/editor/mod.rs +++ b/rust-lib/flowy-document/tests/editor/mod.rs @@ -105,16 +105,20 @@ impl TestBuilder { TestOp::Insert(delta_i, s, index) => { let document = &mut self.documents[*delta_i]; let delta = document.insert(*index, s).unwrap(); + log::debug!("Insert delta: {}", delta.to_json()); + self.deltas.insert(*delta_i, Some(delta)); }, TestOp::Delete(delta_i, iv) => { let document = &mut self.documents[*delta_i]; let delta = document.replace(*iv, "").unwrap(); + log::trace!("Delete delta: {}", delta.to_json()); self.deltas.insert(*delta_i, Some(delta)); }, TestOp::Replace(delta_i, iv, s) => { let document = &mut self.documents[*delta_i]; let delta = document.replace(*iv, s).unwrap(); + log::trace!("Replace delta: {}", delta.to_json()); self.deltas.insert(*delta_i, Some(delta)); }, TestOp::InsertBold(delta_i, s, iv) => { @@ -126,6 +130,7 @@ impl TestBuilder { let document = &mut self.documents[*delta_i]; let attribute = Attribute::Bold(*enable); let delta = document.format(*iv, attribute).unwrap(); + log::trace!("Bold delta: {}", delta.to_json()); self.deltas.insert(*delta_i, Some(delta)); }, TestOp::Italic(delta_i, iv, enable) => { @@ -135,24 +140,29 @@ impl TestBuilder { false => Attribute::Italic(false), }; let delta = document.format(*iv, attribute).unwrap(); + log::trace!("Italic delta: {}", delta.to_json()); self.deltas.insert(*delta_i, Some(delta)); }, TestOp::Header(delta_i, iv, level) => { let document = &mut self.documents[*delta_i]; let attribute = Attribute::Header(*level); let delta = document.format(*iv, attribute).unwrap(); + log::trace!("Header delta: {}", delta.to_json()); self.deltas.insert(*delta_i, Some(delta)); }, TestOp::Link(delta_i, iv, link) => { let document = &mut self.documents[*delta_i]; let attribute = Attribute::Link(link.to_owned()); let delta = document.format(*iv, attribute).unwrap(); + log::trace!("Link delta: {}", delta.to_json()); self.deltas.insert(*delta_i, Some(delta)); }, TestOp::Bullet(delta_i, iv, enable) => { let document = &mut self.documents[*delta_i]; let attribute = Attribute::Bullet(*enable); let delta = document.format(*iv, attribute).unwrap(); + log::debug!("Bullet delta: {}", delta.to_json()); + self.deltas.insert(*delta_i, Some(delta)); }, TestOp::Transform(delta_a_i, delta_b_i) => { diff --git a/rust-lib/flowy-document/tests/editor/serde_test.rs b/rust-lib/flowy-document/tests/editor/serde_test.rs index a7959b9b7e..2158a21eb4 100644 --- a/rust-lib/flowy-document/tests/editor/serde_test.rs +++ b/rust-lib/flowy-document/tests/editor/serde_test.rs @@ -82,8 +82,21 @@ fn delta_deserialize_null_test() { let json = r#"[ {"retain":7,"attributes":{"bold":null}} ]"#; - let delta = Delta::from_json(json).unwrap(); - println!("{}", delta); + let delta1 = Delta::from_json(json).unwrap(); + + let mut attribute = Attribute::Bold(true); + attribute.value = AttributeValue(None); + let delta2 = DeltaBuilder::new().retain_with_attributes(7, attribute.into()).build(); + + assert_eq!(delta2.to_json(), r#"[{"retain":7,"attributes":{"bold":""}}]"#); + assert_eq!(delta1, delta2); +} + +#[test] +fn delta_serde_null_test() { + let mut attribute = Attribute::Bold(true); + attribute.value = AttributeValue(None); + assert_eq!(attribute.to_json(), r#"{"bold":""}"#); } #[test] diff --git a/rust-lib/flowy-ot/src/core/attributes/attribute.rs b/rust-lib/flowy-ot/src/core/attributes/attribute.rs index e798c20db7..4674cb0b8f 100644 --- a/rust-lib/flowy-ot/src/core/attributes/attribute.rs +++ b/rust-lib/flowy-ot/src/core/attributes/attribute.rs @@ -3,8 +3,10 @@ use crate::{block_attribute, core::Attributes, ignore_attribute, inline_attribute, list_attribute}; use lazy_static::lazy_static; +use serde_json::Error; use std::{collections::HashSet, fmt, fmt::Formatter, iter::FromIterator}; use strum_macros::Display; + #[derive(Debug, Clone)] pub struct Attribute { pub key: AttributeKey, @@ -41,6 +43,16 @@ impl Attribute { list_attribute!(Ordered, "ordered"); list_attribute!(Checked, "checked"); list_attribute!(UnChecked, "unchecked"); + + pub fn to_json(&self) -> String { + match serde_json::to_string(self) { + Ok(json) => json, + Err(e) => { + log::error!("Attribute serialize to str failed: {}", e); + "".to_owned() + }, + } + } } impl fmt::Display for Attribute { @@ -100,7 +112,7 @@ pub enum AttributeKey { // pub trait AttributeValueData<'a>: Serialize + Deserialize<'a> {} #[derive(Debug, Clone, PartialEq, Eq, Hash)] -pub struct AttributeValue(pub(crate) Option); +pub struct AttributeValue(pub Option); impl std::convert::From<&usize> for AttributeValue { fn from(val: &usize) -> Self { AttributeValue::from(*val) } diff --git a/rust-lib/flowy-ot/src/core/attributes/attributes_serde.rs b/rust-lib/flowy-ot/src/core/attributes/attributes_serde.rs index e23e3fa9ad..34f19da47b 100644 --- a/rust-lib/flowy-ot/src/core/attributes/attributes_serde.rs +++ b/rust-lib/flowy-ot/src/core/attributes/attributes_serde.rs @@ -1,9 +1,10 @@ #[rustfmt::skip] use crate::core::AttributeValue; -use crate::core::{AttributeKey, Attributes}; +use crate::core::{Attribute, AttributeKey, Attributes}; use serde::{ de, de::{MapAccess, Visitor}, + ser, ser::SerializeMap, Deserialize, Deserializer, @@ -12,6 +13,17 @@ use serde::{ }; use std::fmt; +impl Serialize for Attribute { + fn serialize(&self, serializer: S) -> Result<::Ok, ::Error> + where + S: Serializer, + { + let mut map = serializer.serialize_map(Some(1))?; + let _ = serial_attribute(&mut map, &self.key, &self.value)?; + map.end() + } +} + impl Serialize for Attributes { fn serialize(&self, serializer: S) -> Result where @@ -23,42 +35,53 @@ impl Serialize for Attributes { let mut map = serializer.serialize_map(Some(self.inner.len()))?; for (k, v) in &self.inner { - if let Some(v) = &v.0 { - match k { - AttributeKey::Bold - | AttributeKey::Italic - | AttributeKey::Underline - | AttributeKey::StrikeThrough - | AttributeKey::CodeBlock - | AttributeKey::QuoteBlock => match &v.parse::() { - Ok(value) => map.serialize_entry(k, value)?, - Err(e) => log::error!("Serial {:?} failed. {:?}", k, e), - }, - - AttributeKey::Font - | AttributeKey::Size - | AttributeKey::Header - | AttributeKey::Indent - | AttributeKey::Width - | AttributeKey::Height => match &v.parse::() { - Ok(value) => map.serialize_entry(k, value)?, - Err(e) => log::error!("Serial {:?} failed. {:?}", k, e), - }, - - AttributeKey::Link - | AttributeKey::Color - | AttributeKey::Background - | AttributeKey::Align - | AttributeKey::List => { - map.serialize_entry(k, v)?; - }, - } - } + let _ = serial_attribute(&mut map, k, v)?; } map.end() } } +fn serial_attribute(map_serializer: &mut S, key: &AttributeKey, value: &AttributeValue) -> Result<(), E> +where + S: SerializeMap, + E: From<::Error>, +{ + if let Some(v) = &value.0 { + match key { + AttributeKey::Bold + | AttributeKey::Italic + | AttributeKey::Underline + | AttributeKey::StrikeThrough + | AttributeKey::CodeBlock + | AttributeKey::QuoteBlock => match &v.parse::() { + Ok(value) => map_serializer.serialize_entry(&key, value)?, + Err(e) => log::error!("Serial {:?} failed. {:?}", &key, e), + }, + + AttributeKey::Font + | AttributeKey::Size + | AttributeKey::Header + | AttributeKey::Indent + | AttributeKey::Width + | AttributeKey::Height => match &v.parse::() { + Ok(value) => map_serializer.serialize_entry(&key, value)?, + Err(e) => log::error!("Serial {:?} failed. {:?}", &key, e), + }, + + AttributeKey::Link + | AttributeKey::Color + | AttributeKey::Background + | AttributeKey::Align + | AttributeKey::List => { + map_serializer.serialize_entry(&key, v)?; + }, + } + } else { + map_serializer.serialize_entry(&key, "")?; + } + Ok(()) +} + impl<'de> Deserialize<'de> for Attributes { fn deserialize(deserializer: D) -> Result where @@ -190,6 +213,7 @@ impl<'de> Deserialize<'de> for AttributeValue { where E: de::Error, { + // the value that contains null will be processed here. Ok(AttributeValue(None)) }