From 77eb6853b82954fd740dae8831e49934f4488de6 Mon Sep 17 00:00:00 2001 From: Zixuan Chen Date: Fri, 19 Jan 2024 22:20:27 +0800 Subject: [PATCH] fix: time travel back should be able to nullify rich text span (#254) --- .../loro-internal/src/container/richtext.rs | 6 -- .../src/container/richtext/richtext_state.rs | 70 ++++++++++++---- .../src/container/richtext/style_range_map.rs | 31 +++++++- crates/loro-internal/src/delta/text.rs | 61 +++++++++----- crates/loro-internal/src/event.rs | 2 + crates/loro-internal/src/fuzz/richtext.rs | 11 +-- crates/loro-internal/src/handler.rs | 28 +++++-- .../loro-internal/src/state/richtext_state.rs | 79 +++++++++++++++++-- crates/loro-internal/src/txn.rs | 4 +- crates/loro-internal/src/value.rs | 2 +- crates/loro/tests/loro_rust_test.rs | 34 +++++++- 11 files changed, 256 insertions(+), 72 deletions(-) diff --git a/crates/loro-internal/src/container/richtext.rs b/crates/loro-internal/src/container/richtext.rs index 714181ca..5f27d1df 100644 --- a/crates/loro-internal/src/container/richtext.rs +++ b/crates/loro-internal/src/container/richtext.rs @@ -60,12 +60,6 @@ pub(crate) enum StyleKey { } impl StyleKey { - pub fn to_attr_key(&self) -> String { - match self { - Self::Key(key) => key.to_string(), - } - } - pub fn key(&self) -> &InternalString { match self { Self::Key(key) => key, diff --git a/crates/loro-internal/src/container/richtext/richtext_state.rs b/crates/loro-internal/src/container/richtext/richtext_state.rs index 57044857..d4df8562 100644 --- a/crates/loro-internal/src/container/richtext/richtext_state.rs +++ b/crates/loro-internal/src/container/richtext/richtext_state.rs @@ -6,7 +6,10 @@ use generic_btree::{ }; use loro_common::{IdSpan, LoroValue, ID}; use serde::{ser::SerializeStruct, Serialize}; -use std::fmt::{Display, Formatter}; +use std::{ + fmt::{Display, Formatter}, + ops::RangeBounds, +}; use std::{ ops::{Add, AddAssign, Range, Sub}, str::Utf8Error, @@ -32,7 +35,7 @@ use self::{ use super::{ query_by_len::{IndexQuery, QueryByLen}, - style_range_map::{IterAnchorItem, StyleRangeMap, Styles}, + style_range_map::{self, IterAnchorItem, StyleRangeMap, Styles}, AnchorType, RichtextSpan, StyleOp, }; @@ -276,11 +279,13 @@ mod text_chunk { let mut start = 0; let mut end = 0; let mut started = false; + let mut last_unicode_index = 0; for (unicode_index, (i, c)) in self.as_str().char_indices().enumerate() { if unicode_index == range.start { start = i; started = true; } + if unicode_index == range.end { end = i; break; @@ -288,6 +293,14 @@ mod text_chunk { if started { utf16_len += c.len_utf16(); } + + last_unicode_index = unicode_index; + } + + assert!(started); + if end == 0 { + assert_eq!(last_unicode_index + 1, range.end); + end = self.bytes.len(); } let ans = Self { @@ -1662,7 +1675,7 @@ impl RichtextState { &mut self, pos: usize, len: usize, - mut f: impl FnMut(RichtextStateChunk), + mut f: Option<&mut dyn FnMut(RichtextStateChunk)>, ) -> (usize, usize) { assert!( pos + len <= self.len_entity(), @@ -1721,13 +1734,25 @@ impl RichtextState { updater.update(&*elem); match elem { RichtextStateChunk::Text(text) => { + if let Some(f) = f { + let span = text.slice(start_cursor.offset..start_cursor.offset + len); + f(RichtextStateChunk::Text(span)); + } let (next, event_len_) = text.delete_by_entity_index(start_cursor.offset, len); event_len = event_len_; (true, next.map(RichtextStateChunk::Text), None) } RichtextStateChunk::Style { .. } => { - *elem = RichtextStateChunk::Text(TextChunk::new_empty()); + if let Some(f) = f { + let v = std::mem::replace( + elem, + RichtextStateChunk::Text(TextChunk::new_empty()), + ); + f(v); + } else { + *elem = RichtextStateChunk::Text(TextChunk::new_empty()); + } (true, None, None) } } @@ -1744,7 +1769,9 @@ impl RichtextState { let mut updater = StyleRangeUpdater::new(self.style_ranges.as_mut(), pos); for iter in generic_btree::iter::Drain::new(&mut self.tree, start, end) { updater.update(&iter); - f(iter) + if let Some(f) = f.as_mut() { + f(iter) + } } if let Some(s) = self.style_ranges.as_mut() { @@ -1828,7 +1855,6 @@ impl RichtextState { } pub fn get_richtext_value(&self) -> LoroValue { - self.check_consistency_between_content_and_style_ranges(); let mut ans: Vec = Vec::new(); let mut last_attributes: Option = None; for span in self.iter() { @@ -2003,6 +2029,14 @@ impl RichtextState { .map(|x| x.estimate_size()) .unwrap_or(0) } + + /// Iter style ranges in the given range in entity index + pub(crate) fn iter_style_range( + &self, + range: impl RangeBounds, + ) -> Option> { + self.style_ranges.as_ref().map(|x| x.iter_range(range)) + } } use converter::ContinuousIndexConverter; @@ -2103,7 +2137,7 @@ mod test { self.state.drain_by_entity_index( range.entity_start, range.entity_end - range.entity_start, - |_| {}, + None, ); } } @@ -2487,11 +2521,15 @@ mod test { wrapper.insert(0, "Hello World!"); wrapper.mark(0..5, bold(0)); let mut count = 0; - wrapper.state.drain_by_entity_index(0, 7, |span| { - if matches!(span, RichtextStateChunk::Style { .. }) { - count += 1; - } - }); + wrapper.state.drain_by_entity_index( + 0, + 7, + Some(&mut |span| { + if matches!(span, RichtextStateChunk::Style { .. }) { + count += 1; + } + }), + ); assert_eq!(count, 2); assert_eq!( @@ -2507,8 +2545,8 @@ mod test { let mut wrapper = SimpleWrapper::default(); wrapper.insert(0, "Hello World!"); wrapper.mark(0..5, bold(0)); - wrapper.state.drain_by_entity_index(6, 1, |_| {}); - wrapper.state.drain_by_entity_index(0, 1, |_| {}); + wrapper.state.drain_by_entity_index(6, 1, None); + wrapper.state.drain_by_entity_index(0, 1, None); assert_eq!( wrapper.state.get_richtext_value().to_json_value(), json!([{ @@ -2522,8 +2560,8 @@ mod test { let mut wrapper = SimpleWrapper::default(); wrapper.insert(0, "Hello World!"); wrapper.mark(2..5, bold(0)); - wrapper.state.drain_by_entity_index(6, 1, |_| {}); - wrapper.state.drain_by_entity_index(1, 2, |_| {}); + wrapper.state.drain_by_entity_index(6, 1, None); + wrapper.state.drain_by_entity_index(1, 2, None); assert_eq!( wrapper.state.get_richtext_value().to_json_value(), json!([{ diff --git a/crates/loro-internal/src/container/richtext/style_range_map.rs b/crates/loro-internal/src/container/richtext/style_range_map.rs index 4a0ea62e..f6046d1d 100644 --- a/crates/loro-internal/src/container/richtext/style_range_map.rs +++ b/crates/loro-internal/src/container/richtext/style_range_map.rs @@ -3,7 +3,7 @@ use std::{ collections::BTreeSet, - ops::{ControlFlow, Deref, DerefMut, Range}, + ops::{ControlFlow, Deref, DerefMut, Range, RangeBounds}, sync::Arc, usize, }; @@ -91,9 +91,9 @@ impl DerefMut for Styles { pub(super) static EMPTY_STYLES: Lazy = Lazy::new(Default::default); #[derive(Debug, Clone)] -pub(super) struct Elem { - styles: Styles, - len: usize, +pub(crate) struct Elem { + pub(crate) styles: Styles, + pub(crate) len: usize, } #[derive(Clone, Default, Debug, PartialEq, Eq)] @@ -313,6 +313,29 @@ impl StyleRangeMap { } } + pub(crate) fn iter_range( + &self, + range: impl RangeBounds, + ) -> impl Iterator + '_ { + let start = match range.start_bound() { + std::ops::Bound::Included(x) => *x, + std::ops::Bound::Excluded(x) => *x + 1, + std::ops::Bound::Unbounded => 0, + }; + + let end = match range.end_bound() { + std::ops::Bound::Included(x) => *x + 1, + std::ops::Bound::Excluded(x) => *x, + std::ops::Bound::Unbounded => usize::MAX, + }; + + let start = self.tree.query::(&start).unwrap(); + let end = self.tree.query::(&end).unwrap(); + self.tree + .iter_range(start.cursor..end.cursor) + .map(|x| x.elem) + } + /// Return the expected style anchors with their indexes. pub(super) fn iter_anchors(&self) -> impl Iterator + '_ { let mut index = 0; diff --git a/crates/loro-internal/src/delta/text.rs b/crates/loro-internal/src/delta/text.rs index d915acbb..ae4433f9 100644 --- a/crates/loro-internal/src/delta/text.rs +++ b/crates/loro-internal/src/delta/text.rs @@ -1,18 +1,18 @@ use std::sync::Arc; use fxhash::FxHashMap; -use loro_common::{LoroValue, PeerID}; +use loro_common::{InternalString, LoroValue, PeerID}; use serde::{Deserialize, Serialize}; use crate::change::Lamport; -use crate::container::richtext::{Style, StyleKey, Styles}; +use crate::container::richtext::{Style, Styles}; use crate::ToJson; use super::Meta; #[derive(Debug, Default, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct StyleMeta { - map: FxHashMap, + map: FxHashMap, } #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] @@ -39,7 +39,7 @@ impl From<&Styles> for StyleMeta { for (key, value) in styles.iter() { if let Some(value) = value.get() { map.insert( - key.clone(), + key.key().clone(), StyleMetaItem { value: value.to_value(), lamport: value.lamport, @@ -85,35 +85,56 @@ impl Meta for StyleMeta { } impl StyleMeta { - pub(crate) fn iter(&self) -> impl Iterator + '_ { + pub(crate) fn iter(&self) -> impl Iterator + '_ { self.map.iter().map(|(key, style)| { ( key.clone(), Style { - key: key.key().clone(), + key: key.clone(), data: style.value.clone(), }, ) }) } - pub(crate) fn insert(&mut self, key: StyleKey, value: StyleMetaItem) { + pub(crate) fn insert(&mut self, key: InternalString, value: StyleMetaItem) { self.map.insert(key, value); } - pub(crate) fn to_value(&self) -> LoroValue { - LoroValue::Map(Arc::new( - self.map - .iter() - .filter_map(|(key, value)| { - if value.value.is_null() { - return None; - } + pub(crate) fn contains_key(&self, key: &InternalString) -> bool { + self.map.contains_key(key) + } - Some((key.to_attr_key(), value.value.clone())) - }) - .collect(), - )) + pub(crate) fn to_value(&self) -> LoroValue { + LoroValue::Map(Arc::new(self.to_map_without_null_value())) + } + + fn to_map_without_null_value(&self) -> FxHashMap { + self.map + .iter() + .filter_map(|(key, value)| { + if value.value.is_null() { + None + } else { + Some((key.to_string(), value.value.clone())) + } + }) + .collect() + } + + pub(crate) fn to_map(&self) -> FxHashMap { + self.map + .iter() + .map(|(key, value)| (key.to_string(), value.value.clone())) + .collect() + } + + pub(crate) fn to_option_map(&self) -> Option> { + if self.is_empty() { + return None; + } + + Some(self.to_map()) } } @@ -122,7 +143,7 @@ impl ToJson for StyleMeta { let mut map = serde_json::Map::new(); for (key, style) in self.iter() { let value = serde_json::to_value(&style.data).unwrap(); - map.insert(key.to_attr_key(), value); + map.insert(key.to_string(), value); } serde_json::Value::Object(map) diff --git a/crates/loro-internal/src/event.rs b/crates/loro-internal/src/event.rs index 49aa1f17..8b7bfe84 100644 --- a/crates/loro-internal/src/event.rs +++ b/crates/loro-internal/src/event.rs @@ -161,6 +161,8 @@ impl From for DiffVariant { #[derive(Clone, Debug, EnumAsInner)] pub enum Diff { List(Delta>), + // TODO: refactor, doesn't make much sense to use `StyleMeta` here, because sometime style + // don't have peer and lamport info /// - When feature `wasm` is enabled, it should use utf16 indexes. /// - When feature `wasm` is disabled, it should use unicode indexes. Text(Delta), diff --git a/crates/loro-internal/src/fuzz/richtext.rs b/crates/loro-internal/src/fuzz/richtext.rs index b6783244..e337d32b 100644 --- a/crates/loro-internal/src/fuzz/richtext.rs +++ b/crates/loro-internal/src/fuzz/richtext.rs @@ -12,8 +12,7 @@ use crate::{ array_mut_ref, container::ContainerID, delta::DeltaItem, id::PeerID, ContainerType, LoroValue, }; use crate::{ - container::richtext::StyleKey, event::Diff, handler::TextDelta, loro::LoroDoc, value::ToJson, - version::Frontiers, TextHandler, + event::Diff, handler::TextDelta, loro::LoroDoc, value::ToJson, version::Frontiers, TextHandler, }; const STYLES_NAME: [&str; 4] = ["bold", "comment", "link", "highlight"]; @@ -96,9 +95,7 @@ impl Actor { let attributes: FxHashMap<_, _> = attributes .iter() .filter(|(_, v)| !v.data.is_null()) - .map(|(k, v)| match k { - StyleKey::Key(k) => (k.to_string(), v.data), - }) + .map(|(k, v)| (k.to_string(), v.data)) .collect(); let attributes = if attributes.is_empty() { None @@ -118,9 +115,7 @@ impl Actor { let attributes: FxHashMap<_, _> = attributes .iter() .filter(|(_, v)| !v.data.is_null()) - .map(|(k, v)| match k { - StyleKey::Key(k) => (k.to_string(), v.data), - }) + .map(|(k, v)| (k.to_string(), v.data)) .collect(); let attributes = if attributes.is_empty() { None diff --git a/crates/loro-internal/src/handler.rs b/crates/loro-internal/src/handler.rs index f89bed22..301b39cc 100644 --- a/crates/loro-internal/src/handler.rs +++ b/crates/loro-internal/src/handler.rs @@ -7,11 +7,11 @@ use crate::{ richtext::richtext_state::PosType, tree::tree_op::TreeOp, }, - delta::{TreeDiffItem, TreeExternalDiff}, + delta::{DeltaItem, StyleMeta, TreeDiffItem, TreeExternalDiff}, op::ListSlice, state::RichtextState, txn::EventHint, - utils::utf16::count_utf16_len, + utils::{string_slice::StringSlice, utf16::count_utf16_len}, }; use enum_as_inner::EnumAsInner; use fxhash::FxHashMap; @@ -43,6 +43,25 @@ pub enum TextDelta { }, } +impl From<&DeltaItem> for TextDelta { + fn from(value: &DeltaItem) -> Self { + match value { + crate::delta::DeltaItem::Retain { retain, attributes } => TextDelta::Retain { + retain: *retain, + attributes: attributes.to_option_map(), + }, + crate::delta::DeltaItem::Insert { insert, attributes } => TextDelta::Insert { + insert: insert.to_string(), + attributes: attributes.to_option_map(), + }, + crate::delta::DeltaItem::Delete { + delete, + attributes: _, + } => TextDelta::Delete { delete: *delete }, + } + } +} + /// Flatten attributes that allow overlap #[derive(Clone)] pub struct TextHandler { @@ -346,10 +365,7 @@ impl TextHandler { let mut override_styles = Vec::new(); if let Some(attr) = attr { // current styles - let map: FxHashMap<_, _> = styles - .iter() - .map(|x| (x.0.key().clone(), x.1.data)) - .collect(); + let map: FxHashMap<_, _> = styles.iter().map(|x| (x.0.clone(), x.1.data)).collect(); debug_log::debug_dbg!(&map); for (key, style) in map.iter() { match attr.get(key.deref()) { diff --git a/crates/loro-internal/src/state/richtext_state.rs b/crates/loro-internal/src/state/richtext_state.rs index 7f552e92..1ffd6219 100644 --- a/crates/loro-internal/src/state/richtext_state.rs +++ b/crates/loro-internal/src/state/richtext_state.rs @@ -18,7 +18,7 @@ use crate::{ }, }, container::{list::list_op, richtext::richtext_state::RichtextStateChunk}, - delta::{Delta, DeltaItem, StyleMeta}, + delta::{Delta, DeltaItem, StyleMeta, StyleMetaItem}, encoding::{EncodeMode, StateSnapshotDecodeContext, StateSnapshotEncoder}, event::{Diff, Index, InternalDiff}, op::{Op, RawOp}, @@ -180,7 +180,7 @@ impl ContainerState for RichtextState { event_index: start_event_index, } = style_starts.remove(style).unwrap(); - let mut delta: Delta = + let mut delta: Delta = Delta::new().retain(start_event_index); // we need to + 1 because we also need to annotate the end anchor let event = self.state.get_mut().annotate_style_range_with_event( @@ -202,15 +202,78 @@ impl ContainerState for RichtextState { delete: len, attributes: _, } => { - let (start, end) = - self.state - .get_mut() - .drain_by_entity_index(entity_index, *len, |_| {}); + let mut deleted_style_chunks = Vec::new(); + let (start, end) = self.state.get_mut().drain_by_entity_index( + entity_index, + *len, + Some(&mut |c| { + if matches!(c, RichtextStateChunk::Style { .. }) { + deleted_style_chunks.push(c); + } + }), + ); + if start > event_index { ans = ans.retain(start - event_index); event_index = start; } + for chunk in deleted_style_chunks { + if let RichtextStateChunk::Style { style, anchor_type } = chunk { + match anchor_type { + AnchorType::Start => { + style_starts.insert( + style, + Pos { + entity_index, + event_index, + }, + ); + } + AnchorType::End => { + let Pos { + entity_index: start_entity_index, + event_index: start_event_index, + } = style_starts.remove(&style).unwrap(); + if event_index == start_event_index { + debug_assert_eq!(start_entity_index, entity_index); + // deleted by this batch, can be ignored + continue; + } + + // Otherwise, we need to calculate the new styles with the key between the ranges + let mut delta: Delta = + Delta::new().retain(start_event_index); + if let Some(iter) = self + .state + .get_mut() + .iter_style_range(start_entity_index..entity_index) + { + for style_range in iter { + let mut style_meta: StyleMeta = + (&style_range.styles).into(); + if !style_meta.contains_key(&style.key) { + style_meta.insert( + style.key.clone(), + StyleMetaItem { + lamport: 0, + peer: 0, + value: LoroValue::Null, + }, + ) + } + delta = + delta.retain_with_meta(style_range.len, style_meta); + } + } + + delta = delta.chop(); + style_delta = style_delta.compose(delta); + } + } + } + } + ans = ans.delete(end - start); } } @@ -284,7 +347,7 @@ impl ContainerState for RichtextState { } => { self.state .get_mut() - .drain_by_entity_index(entity_index, *len, |_| {}); + .drain_by_entity_index(entity_index, *len, None); } } } @@ -314,7 +377,7 @@ impl ContainerState for RichtextState { self.state.get_mut().drain_by_entity_index( del.start() as usize, rle::HasLength::atom_len(&del), - |_| {}, + None, ); } list_op::InnerListOp::StyleStart { diff --git a/crates/loro-internal/src/txn.rs b/crates/loro-internal/src/txn.rs index 402fede1..7669e3e5 100644 --- a/crates/loro-internal/src/txn.rs +++ b/crates/loro-internal/src/txn.rs @@ -16,7 +16,7 @@ use crate::{ container::{ idx::ContainerIdx, list::list_op::{DeleteSpan, InnerListOp}, - richtext::{Style, StyleKey}, + richtext::Style, IntoContainerId, }, delta::{ @@ -503,7 +503,7 @@ fn change_to_diff( EventHint::Mark { start, end, style } => { let mut meta = StyleMeta::default(); meta.insert( - StyleKey::Key(style.key.clone()), + style.key.clone(), StyleMetaItem { lamport, peer: change.id.peer, diff --git a/crates/loro-internal/src/value.rs b/crates/loro-internal/src/value.rs index d646c1c3..758fdf24 100644 --- a/crates/loro-internal/src/value.rs +++ b/crates/loro-internal/src/value.rs @@ -457,7 +457,7 @@ pub mod wasm { let obj = Object::new(); for (key, style) in value.iter() { let value = JsValue::from(style.data); - js_sys::Reflect::set(&obj, &JsValue::from_str(&key.to_attr_key()), &value).unwrap(); + js_sys::Reflect::set(&obj, &JsValue::from_str(&key), &value).unwrap(); } obj.into_js_result().unwrap() diff --git a/crates/loro/tests/loro_rust_test.rs b/crates/loro/tests/loro_rust_test.rs index 280e67bc..5c0c58b8 100644 --- a/crates/loro/tests/loro_rust_test.rs +++ b/crates/loro/tests/loro_rust_test.rs @@ -1,4 +1,36 @@ -use loro_internal::{delta::DeltaItem, DiffEvent, LoroResult}; +use std::sync::Arc; + +use loro::LoroDoc; +use loro_internal::{delta::DeltaItem, handler::TextDelta, DiffEvent, LoroResult}; + +#[test] +fn travel_back_should_remove_styles() { + let mut doc = LoroDoc::new(); + let doc2 = LoroDoc::new(); + let text = doc.get_text("text"); + let text2 = doc2.get_text("text"); + doc.subscribe( + &text.id(), + Arc::new(move |x| { + let Some(text) = x.container.diff.as_text() else { + return; + }; + + let delta: Vec = text.iter().map(|x| x.into()).collect(); + text2.apply_delta(&delta).unwrap(); + }), + ); + + let text2 = doc2.get_text("text"); + text.insert(0, "Hello world!").unwrap(); + doc.commit(); + let f = doc.state_frontiers(); + text.mark(0..5, "bold", true).unwrap(); + doc.commit(); + assert_eq!(text.to_delta(), text2.to_delta()); + doc.checkout(&f).unwrap(); + assert_eq!(text.to_delta(), text2.to_delta()); +} #[test] fn list() -> LoroResult<()> {