From 699c2c73a534e9a2e648fdecdbcbfa11fc043088 Mon Sep 17 00:00:00 2001 From: Zixuan Chen Date: Thu, 7 Dec 2023 15:25:33 +0800 Subject: [PATCH] Fix partial inclusion bug in Tracker (#214) * fix: partial inclusion bug in Tracker * fix: rm redundant logs --- .../src/container/richtext/tracker.rs | 26 ++++++++++------- crates/loro-internal/src/diff_calc.rs | 3 -- crates/loro-internal/tests/test.rs | 29 ++++++++++++++++++- 3 files changed, 43 insertions(+), 15 deletions(-) diff --git a/crates/loro-internal/src/container/richtext/tracker.rs b/crates/loro-internal/src/container/richtext/tracker.rs index 1177c1f8..14347d0e 100644 --- a/crates/loro-internal/src/container/richtext/tracker.rs +++ b/crates/loro-internal/src/container/richtext/tracker.rs @@ -72,8 +72,7 @@ impl Tracker { pub(crate) fn insert(&mut self, mut op_id: ID, mut pos: usize, mut content: RichtextChunk) { let last_id = op_id.inc(content.len() as Counter - 1); let applied_counter_end = self.applied_vv.get(&last_id.peer).copied().unwrap_or(0); - if applied_counter_end > last_id.counter { - // the op is included in the applied vv + if applied_counter_end > op_id.counter { if !self.current_vv.includes_id(last_id) { // PERF: may be slow let mut updates = Default::default(); @@ -87,9 +86,13 @@ impl Tracker { ); self.batch_update(updates, false); } - return; - } else if applied_counter_end > op_id.counter { - // need to slice the content + + if applied_counter_end > last_id.counter { + // the op is included in the applied vv + return; + } + + // the op is partially included, need to slice the content let start = (applied_counter_end - op_id.counter) as usize; op_id.counter = applied_counter_end; pos += start; @@ -137,8 +140,7 @@ impl Tracker { pub(crate) fn delete(&mut self, mut op_id: ID, mut pos: usize, mut len: usize, reverse: bool) { let last_id = op_id.inc(len as Counter - 1); let applied_counter_end = self.applied_vv.get(&last_id.peer).copied().unwrap_or(0); - if applied_counter_end > last_id.counter { - // the op is included in the applied_vv + if applied_counter_end > op_id.counter { if !self.current_vv.includes_id(last_id) { // PERF: may be slow let mut updates = Default::default(); @@ -148,9 +150,12 @@ impl Tracker { ); self.batch_update(updates, false); } - return; - } else if applied_counter_end > op_id.counter { - // need to slice the op + + if applied_counter_end > last_id.counter { + return; + } + + // the op is partially included, need to slice the op let start = (applied_counter_end - op_id.counter) as usize; op_id.counter = applied_counter_end; len -= start; @@ -176,7 +181,6 @@ impl Tracker { let mut cur_id = op_id; for id_span in ans { - debug_log::debug_dbg!(&cur_id, id_span, reverse); let len = id_span.atom_len(); self.id_to_cursor .push(cur_id, id_to_cursor::Cursor::Delete(id_span)); diff --git a/crates/loro-internal/src/diff_calc.rs b/crates/loro-internal/src/diff_calc.rs index 0e4b393b..3ea7899c 100644 --- a/crates/loro-internal/src/diff_calc.rs +++ b/crates/loro-internal/src/diff_calc.rs @@ -1,7 +1,6 @@ use std::sync::Arc; pub(super) mod tree; -use debug_log::debug_dbg; use itertools::Itertools; pub(crate) use tree::TreeDeletedSetTrait; pub(super) use tree::TreeDiffCache; @@ -76,7 +75,6 @@ impl DiffCalculator { after: &crate::VersionVector, after_frontiers: Option<&Frontiers>, ) -> Vec { - debug_dbg!(&before, &after, &oplog); if self.has_all { let include_before = self.last_vv.includes_vv(before); let include_after = self.last_vv.includes_vv(after); @@ -724,7 +722,6 @@ impl DiffCalculatorTrait for RichtextDiffCalculator { // FIXME: handle new containers when inserting richtext style like comments - // debug_log::debug_dbg!(&delta, from, to); // debug_log::debug_dbg!(&self.tracker); InternalDiff::RichtextRaw(delta) } diff --git a/crates/loro-internal/tests/test.rs b/crates/loro-internal/tests/test.rs index e94f1f99..71dde6d6 100644 --- a/crates/loro-internal/tests/test.rs +++ b/crates/loro-internal/tests/test.rs @@ -1,6 +1,6 @@ use std::sync::{atomic::AtomicBool, Arc, Mutex}; -use loro_common::{ContainerID, ContainerType, LoroValue, ID}; +use loro_common::{ContainerID, ContainerType, LoroResult, LoroValue, ID}; use loro_internal::{ container::richtext::TextStyleInfoFlag, handler::{Handler, ValueOrContainer}, @@ -9,6 +9,33 @@ use loro_internal::{ }; use serde_json::json; +#[test] +fn issue_211() -> LoroResult<()> { + let doc1 = LoroDoc::new_auto_commit(); + let doc2 = LoroDoc::new_auto_commit(); + doc1.get_text("text").insert(0, "T")?; + doc2.merge(&doc1)?; + let v0 = doc1.oplog_frontiers(); + doc1.get_text("text").insert(1, "A")?; + doc2.get_text("text").insert(1, "B")?; + doc1.checkout(&v0)?; + doc2.checkout(&v0)?; + doc1.checkout_to_latest(); + doc2.checkout_to_latest(); + // let v1_of_doc1 = doc1.oplog_frontiers(); + let v1_of_doc2 = doc2.oplog_frontiers(); + doc2.get_text("text").insert(2, "B")?; + doc2.checkout(&v1_of_doc2)?; + doc2.checkout(&v0)?; + assert_eq!( + doc2.get_deep_value().to_json_value(), + json!({ + "text": "T" + }) + ); + Ok(()) +} + #[test] fn mark_with_the_same_key_value_should_be_skipped() { let a = LoroDoc::new_auto_commit();