From c4a62bee37bb9f3fde5080f2f3629fcd10975991 Mon Sep 17 00:00:00 2001 From: leeeon233 Date: Wed, 15 Mar 2023 14:29:05 +0800 Subject: [PATCH] fix: delta compose delete insert --- crates/loro-internal/src/delta/seq.rs | 32 +- .../loro-internal/src/fuzz/recursive_txn.rs | 393 ++++++++---------- crates/loro-internal/src/transaction.rs | 3 + 3 files changed, 196 insertions(+), 232 deletions(-) diff --git a/crates/loro-internal/src/delta/seq.rs b/crates/loro-internal/src/delta/seq.rs index e64b697b..d8740451 100644 --- a/crates/loro-internal/src/delta/seq.rs +++ b/crates/loro-internal/src/delta/seq.rs @@ -294,16 +294,19 @@ impl Delta { if new_op.is_insert() && last_op.is_insert() { let value = last_op.as_insert_mut().unwrap().0; value.value_extend(new_op.into_insert().unwrap().0); - self.vec.push(last_op); + // self.vec.push(last_op); + self.vec.insert(index - 1, last_op); return true; } else if new_op.is_retain() && last_op.is_retain() { *last_op.as_retain_mut().unwrap().0 += new_op.content_len(); - self.vec.push(last_op); + // self.vec.push(last_op); + self.vec.insert(index - 1, last_op); return true; } } - self.vec.push(last_op); + // self.vec.push(last_op); + self.vec.insert(index - 1, last_op); } if index == self.vec.len() { self.vec.push(new_op); @@ -511,6 +514,29 @@ mod test { assert_eq!(a.compose(b), Delta::new().insert("112323".to_string())); } + #[test] + fn delete_failed() { + let a: Delta = Delta::new() + .retain(2) + .insert("[31354]") + .retain(1) + .insert("[31354]") + .retain(12) + .insert("[31354]"); + let b: Delta = Delta::new().retain(27).delete(3); + assert_eq!( + a.compose(b), + Delta::new() + .retain(2) + .insert("[31354]") + .retain(1) + .insert("[31354]") + .retain(10) + .insert("31354]") + .delete(2) + ); + } + #[test] fn insert_insert() { let a = TestDelta::new().insert("a"); diff --git a/crates/loro-internal/src/fuzz/recursive_txn.rs b/crates/loro-internal/src/fuzz/recursive_txn.rs index 0b45d699..e989880d 100644 --- a/crates/loro-internal/src/fuzz/recursive_txn.rs +++ b/crates/loro-internal/src/fuzz/recursive_txn.rs @@ -1323,17 +1323,55 @@ mod failed_tests { test_multi_sites( 2, &mut [ - Map { - site: 0, + Text { + site: 2, container_idx: 0, - key: 1, - value: Container(C::List), + pos: 0, + value: 31354, + is_del: false, }, - List { - site: 0, - container_idx: 1, - key: 0, - value: I32(1), + Text { + site: 2, + container_idx: 0, + pos: 3, + value: 31354, + is_del: false, + }, + Text { + site: 2, + container_idx: 0, + pos: 10, + value: 31354, + is_del: false, + }, + SyncAll, + Text { + site: 2, + container_idx: 0, + pos: 3, + value: 31354, + is_del: false, + }, + Text { + site: 2, + container_idx: 0, + pos: 2, + value: 31354, + is_del: false, + }, + Text { + site: 2, + container_idx: 0, + pos: 29, + value: 31354, + is_del: false, + }, + Text { + site: 2, + container_idx: 0, + pos: 27, + value: 3, + is_del: true, }, ], ) @@ -1345,17 +1383,10 @@ mod failed_tests { minify_error( 5, vec![ - Map { - site: 0, - container_idx: 0, - key: 1, - value: Container(C::List), - }, - SyncAll, SyncAll, SyncAll, Map { - site: 10, + site: 0, container_idx: 255, key: 255, value: Container(C::List), @@ -1364,36 +1395,81 @@ mod failed_tests { SyncAll, SyncAll, SyncAll, + SyncAll, + SyncAll, + SyncAll, + SyncAll, + SyncAll, + SyncAll, + SyncAll, + SyncAll, + SyncAll, + SyncAll, + SyncAll, + SyncAll, + SyncAll, + SyncAll, + SyncAll, + SyncAll, + SyncAll, + SyncAll, + SyncAll, + SyncAll, + SyncAll, + SyncAll, + SyncAll, + SyncAll, + SyncAll, + SyncAll, + SyncAll, + SyncAll, + SyncAll, + SyncAll, + SyncAll, + SyncAll, + SyncAll, + SyncAll, + SyncAll, + SyncAll, + SyncAll, + SyncAll, + SyncAll, + SyncAll, + SyncAll, + SyncAll, + SyncAll, Map { - site: 255, - container_idx: 37, - key: 37, - value: Null, + site: 0, + container_idx: 0, + key: 0, + value: I32(-1650614981), }, - Map { - site: 37, - container_idx: 37, - key: 37, - value: Null, - }, - Map { - site: 37, - container_idx: 37, - key: 37, - value: Null, - }, - SyncAll, - SyncAll, - SyncAll, - SyncAll, - SyncAll, - SyncAll, - SyncAll, Text { - site: 255, - container_idx: 255, - pos: 10, - value: 65290, + site: 122, + container_idx: 122, + pos: 122, + value: 31354, + is_del: false, + }, + Text { + site: 122, + container_idx: 122, + pos: 122, + value: 31354, + is_del: false, + }, + Text { + site: 122, + container_idx: 122, + pos: 122, + value: 31354, + is_del: false, + }, + Text { + site: 122, + container_idx: 122, + pos: 122, + value: 65535, is_del: true, }, SyncAll, @@ -1402,197 +1478,56 @@ mod failed_tests { SyncAll, SyncAll, SyncAll, + SyncAll, + SyncAll, + SyncAll, + SyncAll, + SyncAll, + SyncAll, + SyncAll, + SyncAll, + SyncAll, + SyncAll, + Map { + site: 0, + container_idx: 0, + key: 0, + value: I32(-1650614981), + }, + Text { + site: 122, + container_idx: 122, + pos: 122, + value: 31354, + is_del: false, + }, + Text { + site: 122, + container_idx: 122, + pos: 122, + value: 31354, + is_del: false, + }, + Text { + site: 122, + container_idx: 122, + pos: 122, + value: 31354, + is_del: false, + }, + Text { + site: 122, + container_idx: 122, + pos: 255, + value: 65283, + is_del: true, + }, Map { site: 255, container_idx: 255, key: 255, value: Container(C::List), }, - SyncAll, - SyncAll, - SyncAll, - SyncAll, - Map { - site: 37, - container_idx: 37, - key: 37, - value: Null, - }, - Map { - site: 37, - container_idx: 37, - key: 37, - value: Null, - }, - Map { - site: 37, - container_idx: 37, - key: 37, - value: Null, - }, - SyncAll, - SyncAll, - SyncAll, - SyncAll, - SyncAll, - SyncAll, - SyncAll, - SyncAll, - SyncAll, - SyncAll, - SyncAll, - SyncAll, - SyncAll, - SyncAll, - SyncAll, - SyncAll, - SyncAll, - SyncAll, - SyncAll, - List { - site: 92, - container_idx: 92, - key: 92, - value: Container(C::List), - }, - Map { - site: 177, - container_idx: 255, - key: 255, - value: I32(1549556779), - }, - List { - site: 92, - container_idx: 92, - key: 92, - value: I32(1549556828), - }, - List { - site: 92, - container_idx: 92, - key: 92, - value: I32(1549556828), - }, - List { - site: 92, - container_idx: 92, - key: 92, - value: I32(1549556828), - }, - List { - site: 92, - container_idx: 92, - key: 92, - value: I32(1549556828), - }, - List { - site: 92, - container_idx: 73, - key: 73, - value: Null, - }, - List { - site: 255, - container_idx: 73, - key: 0, - value: Null, - }, - Map { - site: 0, - container_idx: 0, - key: 0, - value: Null, - }, - Map { - site: 0, - container_idx: 0, - key: 0, - value: Null, - }, - Map { - site: 0, - container_idx: 0, - key: 0, - value: Null, - }, - Map { - site: 0, - container_idx: 0, - key: 0, - value: Null, - }, - Map { - site: 0, - container_idx: 0, - key: 0, - value: Null, - }, - Map { - site: 0, - container_idx: 0, - key: 0, - value: Null, - }, - Map { - site: 0, - container_idx: 0, - key: 0, - value: Null, - }, - Map { - site: 0, - container_idx: 0, - key: 0, - value: Null, - }, - Map { - site: 0, - container_idx: 0, - key: 0, - value: Null, - }, - Map { - site: 0, - container_idx: 0, - key: 0, - value: Null, - }, - Map { - site: 0, - container_idx: 0, - key: 0, - value: Null, - }, - Map { - site: 0, - container_idx: 0, - key: 0, - value: Null, - }, - Map { - site: 0, - container_idx: 0, - key: 0, - value: Null, - }, - Map { - site: 0, - container_idx: 0, - key: 0, - value: Null, - }, - Map { - site: 0, - container_idx: 0, - key: 0, - value: Null, - }, - Map { - site: 0, - container_idx: 0, - key: 0, - value: Null, - }, Map { site: 0, container_idx: 0, diff --git a/crates/loro-internal/src/transaction.rs b/crates/loro-internal/src/transaction.rs index c3350274..0a7abda6 100644 --- a/crates/loro-internal/src/transaction.rs +++ b/crates/loro-internal/src/transaction.rs @@ -140,7 +140,10 @@ impl Transaction { pub(crate) fn append_event(&mut self, idx: ContainerIdx, event: RawEvent) { // cache events if let Some(old) = self.pending_events.get_mut(&idx) { + // println!("old event {:?}", old.diff); + // println!("new event {:?}", event.diff); compose_two_events(old, event); + // println!("res {:?}\n", old.diff) } else { self.pending_events.insert(idx, event); }