fix: movable list undo/redo error (#507)

* fix: movable list undo/redo error

* chore: rm log

* chore: rm unused

* chore: use deno 2.x

* ci: remove build step
This commit is contained in:
Zixuan Chen 2024-10-11 23:36:04 +08:00 committed by GitHub
parent 9680be4103
commit b5e153a33d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 892 additions and 31 deletions

View file

@ -27,7 +27,7 @@ jobs:
- name: Setup Deno
uses: denoland/setup-deno@v1
with:
deno-version: v1.x
deno-version: v2.x
- name: Install wasm-opt
run: npm i wasm-opt -g

View file

@ -22,7 +22,7 @@ jobs:
version: "latest"
- uses: denoland/setup-deno@v1
with:
deno-version: v1.x
deno-version: v2.x
- uses: actions/setup-node@v3
with:
node-version: 20
@ -42,7 +42,5 @@ jobs:
- uses: Swatinem/rust-cache@v2
- name: Check
run: cargo clippy --all-features -- -Dwarnings
- name: Build
run: cargo build --verbose
- name: Run rust tests
run: deno task test-all

View file

@ -11726,6 +11726,863 @@ fn tree_event_parent_not_found() {
)
}
#[test]
fn unknown() {
test_multi_sites(
5,
vec![FuzzTarget::All],
&mut [
SyncAll,
Handle {
site: 0,
target: 49,
container: 22,
action: Generic(GenericAction {
value: Container(MovableList),
bool: false,
key: 4294938768,
pos: 2097991952006905855,
length: 10416835838496021789,
prop: 2097865012311789712,
}),
},
Handle {
site: 29,
target: 29,
container: 29,
action: Generic(GenericAction {
value: I32(488447261),
bool: true,
key: 488447261,
pos: 394849716870429,
length: 10378828040806145280,
prop: 18446743004382043421,
}),
},
SyncAll,
SyncAll,
SyncAll,
SyncAll,
SyncAll,
SyncAll,
SyncAll,
SyncAll,
SyncAll,
SyncAll,
SyncAll,
SyncAll,
SyncAll,
SyncAll,
SyncAll,
SyncAll,
SyncAll,
Undo {
site: 127,
op_len: 2139062143,
},
Handle {
site: 7,
target: 7,
container: 7,
action: Generic(GenericAction {
value: I32(117901063),
bool: true,
key: 117901063,
pos: 506381209866536711,
length: 506381209866536711,
prop: 506381209866536711,
}),
},
Handle {
site: 7,
target: 7,
container: 7,
action: Generic(GenericAction {
value: I32(7),
bool: false,
key: 922746880,
pos: 506381209866536711,
length: 290208427752752903,
prop: 506381209866536711,
}),
},
Handle {
site: 7,
target: 249,
container: 240,
action: Generic(GenericAction {
value: I32(-185273100),
bool: true,
key: 488447261,
pos: 2097865012304223517,
length: 3474305256856826420,
prop: 2097865012304230198,
}),
},
Handle {
site: 29,
target: 25,
container: 29,
action: Generic(GenericAction {
value: I32(-1869574000),
bool: false,
key: 488447261,
pos: 2097865012157422877,
length: 2097865012304223517,
prop: 2097865012304223517,
}),
},
Handle {
site: 0,
target: 0,
container: 29,
action: Generic(GenericAction {
value: I32(-1138913271),
bool: true,
key: 488447261,
pos: 2097865012304223517,
length: 2097865012304223517,
prop: 2097865012304223517,
}),
},
Handle {
site: 29,
target: 29,
container: 29,
action: Generic(GenericAction {
value: Container(Text),
bool: true,
key: 488447261,
pos: 2097865012304223517,
length: 2097865012304223517,
prop: 2097865012304223517,
}),
},
Handle {
site: 29,
target: 29,
container: 29,
action: Generic(GenericAction {
value: Container(Unknown(255)),
bool: true,
key: 0,
pos: 13474770085092589312,
length: 10416835838496021789,
prop: 2097865012311789712,
}),
},
Handle {
site: 29,
target: 29,
container: 29,
action: Generic(GenericAction {
value: I32(488447261),
bool: true,
key: 488447261,
pos: 394849716870429,
length: 10378828040806145280,
prop: 18446743004382043421,
}),
},
SyncAll,
SyncAll,
SyncAll,
SyncAll,
SyncAll,
SyncAll,
SyncAll,
SyncAll,
SyncAll,
SyncAll,
SyncAll,
SyncAll,
SyncAll,
SyncAll,
SyncAll,
SyncAll,
SyncAll,
Undo {
site: 127,
op_len: 2139062143,
},
Handle {
site: 7,
target: 7,
container: 7,
action: Generic(GenericAction {
value: I32(117901063),
bool: true,
key: 117901063,
pos: 506381209866536711,
length: 506381209866536711,
prop: 506381209866536711,
}),
},
Handle {
site: 7,
target: 7,
container: 7,
action: Generic(GenericAction {
value: I32(7),
bool: false,
key: 922746880,
pos: 506381209866536711,
length: 290208427752752903,
prop: 506381209866536711,
}),
},
Handle {
site: 7,
target: 249,
container: 240,
action: Generic(GenericAction {
value: I32(-185273100),
bool: true,
key: 488447261,
pos: 2097865012304223517,
length: 3474305256856826420,
prop: 2097865012304230198,
}),
},
Handle {
site: 29,
target: 25,
container: 29,
action: Generic(GenericAction {
value: I32(-1869574000),
bool: false,
key: 488447261,
pos: 2097865012153228573,
length: 2097865012304223517,
prop: 2097865012304223517,
}),
},
Handle {
site: 0,
target: 0,
container: 29,
action: Generic(GenericAction {
value: I32(-1138913271),
bool: true,
key: 488447261,
pos: 2097865012304223517,
length: 2097865012304223517,
prop: 2097865012304223517,
}),
},
Handle {
site: 29,
target: 29,
container: 29,
action: Generic(GenericAction {
value: Container(Text),
bool: true,
key: 488447261,
pos: 2097865012304223517,
length: 2097865012304223517,
prop: 2097865012304223517,
}),
},
Handle {
site: 29,
target: 29,
container: 29,
action: Generic(GenericAction {
value: Container(Unknown(255)),
bool: true,
key: 0,
pos: 13474770085092589312,
length: 18446744073709486369,
prop: 2089670231998729471,
}),
},
Handle {
site: 144,
target: 144,
container: 144,
action: Generic(GenericAction {
value: I32(488447261),
bool: true,
key: 488447261,
pos: 18374403900856671517,
length: 15987178339521527808,
prop: 11791512336840195838,
}),
},
SyncAllUndo {
site: 147,
op_len: 2475922323,
},
SyncAllUndo {
site: 147,
op_len: 2475922323,
},
SyncAllUndo {
site: 147,
op_len: 2475922323,
},
SyncAllUndo {
site: 147,
op_len: 2475922323,
},
SyncAllUndo {
site: 147,
op_len: 2475922323,
},
SyncAllUndo {
site: 147,
op_len: 2475922323,
},
SyncAllUndo {
site: 147,
op_len: 2475922323,
},
SyncAllUndo {
site: 147,
op_len: 2475922323,
},
SyncAllUndo {
site: 147,
op_len: 2475922323,
},
SyncAllUndo {
site: 155,
op_len: 4292972707,
},
SyncAllUndo {
site: 147,
op_len: 2475922323,
},
SyncAllUndo {
site: 147,
op_len: 2475922323,
},
SyncAllUndo {
site: 147,
op_len: 2475922323,
},
SyncAllUndo {
site: 149,
op_len: 3784352667,
},
Handle {
site: 0,
target: 0,
container: 0,
action: Generic(GenericAction {
value: I32(-1157627904),
bool: true,
key: 4280548865,
pos: 18446744073709551615,
length: 18446744073709551615,
prop: 18446744073709551615,
}),
},
SyncAll,
SyncAll,
SyncAll,
Undo {
site: 127,
op_len: 2139062143,
},
Handle {
site: 7,
target: 7,
container: 7,
action: Generic(GenericAction {
value: I32(117901063),
bool: true,
key: 117901063,
pos: 506381209866536711,
length: 506381209866536711,
prop: 506381209866536711,
}),
},
Handle {
site: 7,
target: 7,
container: 7,
action: Generic(GenericAction {
value: I32(7),
bool: false,
key: 922746880,
pos: 506381209866536711,
length: 290208427752752903,
prop: 506381209866536711,
}),
},
Handle {
site: 7,
target: 249,
container: 240,
action: Generic(GenericAction {
value: I32(-185273100),
bool: true,
key: 488447261,
pos: 2097865012304223517,
length: 3474305256856826420,
prop: 2097865012304230198,
}),
},
Handle {
site: 29,
target: 25,
container: 29,
action: Generic(GenericAction {
value: I32(-1869574000),
bool: false,
key: 488447261,
pos: 2097865012157422877,
length: 2097865012304223517,
prop: 2097865012304223517,
}),
},
Handle {
site: 0,
target: 0,
container: 29,
action: Generic(GenericAction {
value: I32(-1138913271),
bool: true,
key: 488447261,
pos: 2097865012304223517,
length: 2097865012304223517,
prop: 2097865012304223517,
}),
},
Handle {
site: 29,
target: 29,
container: 29,
action: Generic(GenericAction {
value: Container(Text),
bool: true,
key: 488447261,
pos: 2097865012304223517,
length: 2097865012304223517,
prop: 2097865012304223517,
}),
},
Handle {
site: 29,
target: 29,
container: 29,
action: Generic(GenericAction {
value: Container(Unknown(255)),
bool: true,
key: 0,
pos: 13474770085092589312,
length: 10416835838496021789,
prop: 2097865012311789712,
}),
},
Handle {
site: 29,
target: 29,
container: 29,
action: Generic(GenericAction {
value: I32(488447261),
bool: true,
key: 488447261,
pos: 394849716870429,
length: 10378828040806145280,
prop: 18446743004382043421,
}),
},
SyncAll,
SyncAll,
SyncAll,
SyncAll,
SyncAll,
SyncAll,
SyncAll,
SyncAll,
SyncAll,
SyncAll,
SyncAll,
SyncAll,
SyncAll,
SyncAll,
SyncAll,
SyncAll,
SyncAll,
Undo {
site: 127,
op_len: 2139062143,
},
Handle {
site: 7,
target: 7,
container: 7,
action: Generic(GenericAction {
value: I32(117901063),
bool: true,
key: 117901063,
pos: 506381209866536711,
length: 506381209866536711,
prop: 506381209866536711,
}),
},
Handle {
site: 7,
target: 7,
container: 7,
action: Generic(GenericAction {
value: I32(7),
bool: false,
key: 922746880,
pos: 506381209866536711,
length: 290208427752752903,
prop: 506381209866536711,
}),
},
Handle {
site: 7,
target: 249,
container: 240,
action: Generic(GenericAction {
value: I32(-185273100),
bool: true,
key: 488447261,
pos: 2097865012304223517,
length: 3474305256856826420,
prop: 2097865012304230198,
}),
},
Handle {
site: 29,
target: 25,
container: 29,
action: Generic(GenericAction {
value: I32(-1869574000),
bool: false,
key: 488447261,
pos: 2097865012153228573,
length: 2097865012304223517,
prop: 2097865012304223517,
}),
},
Handle {
site: 0,
target: 0,
container: 29,
action: Generic(GenericAction {
value: I32(-1138913271),
bool: true,
key: 488447261,
pos: 2097865012304223517,
length: 2097865012304223517,
prop: 2097865012304223517,
}),
},
Handle {
site: 29,
target: 29,
container: 29,
action: Generic(GenericAction {
value: Container(Text),
bool: true,
key: 488447261,
pos: 2097865012304223517,
length: 2097865012304223517,
prop: 2097865012304223517,
}),
},
Handle {
site: 0,
target: 0,
container: 0,
action: Generic(GenericAction {
value: Container(Unknown(255)),
bool: true,
key: 0,
pos: 13474770085092589312,
length: 18446744073709486369,
prop: 2089670231998729471,
}),
},
Handle {
site: 144,
target: 144,
container: 144,
action: Generic(GenericAction {
value: I32(488447261),
bool: true,
key: 488447261,
pos: 18374403900856671517,
length: 15987178339521527808,
prop: 2097865011832487678,
}),
},
Handle {
site: 29,
target: 7,
container: 7,
action: Generic(GenericAction {
value: Container(Unknown(255)),
bool: true,
key: 4294967295,
pos: 18446744073709551615,
length: 18446744073709551615,
prop: 18446744073709551615,
}),
},
SyncAll,
SyncAll,
SyncAll,
SyncAll,
SyncAll,
SyncAll,
SyncAll,
SyncAll,
SyncAll,
SyncAll,
Undo {
site: 127,
op_len: 2139062143,
},
Handle {
site: 7,
target: 7,
container: 7,
action: Generic(GenericAction {
value: I32(-184088825),
bool: false,
key: 117901063,
pos: 506381209866536711,
length: 506381209866536711,
prop: 506381209866536711,
}),
},
Handle {
site: 7,
target: 7,
container: 7,
action: Generic(GenericAction {
value: I32(7),
bool: false,
key: 922746880,
pos: 506381209866536711,
length: 290208427752752903,
prop: 506381209866536711,
}),
},
Handle {
site: 7,
target: 249,
container: 240,
action: Generic(GenericAction {
value: I32(-185273100),
bool: true,
key: 488447261,
pos: 18446743099240422685,
length: 18446744073709551615,
prop: 18446744073709551615,
}),
},
SyncAll,
SyncAll,
Undo {
site: 127,
op_len: 2139062143,
},
Handle {
site: 7,
target: 7,
container: 7,
action: Generic(GenericAction {
value: I32(117901063),
bool: true,
key: 117901063,
pos: 506381209866536711,
length: 506381209866536711,
prop: 506381209866536711,
}),
},
Handle {
site: 7,
target: 7,
container: 7,
action: Generic(GenericAction {
value: I32(7),
bool: false,
key: 922746880,
pos: 506381209866536711,
length: 290208427752752903,
prop: 506381209866536711,
}),
},
Handle {
site: 7,
target: 249,
container: 240,
action: Generic(GenericAction {
value: I32(-185273100),
bool: true,
key: 488447261,
pos: 2097865012304223517,
length: 3474305256856826420,
prop: 2097865012304230198,
}),
},
Handle {
site: 29,
target: 25,
container: 29,
action: Generic(GenericAction {
value: I32(-1869574000),
bool: false,
key: 488447261,
pos: 2097865012153228573,
length: 2097865012304223517,
prop: 2097865012304223517,
}),
},
Handle {
site: 0,
target: 0,
container: 29,
action: Generic(GenericAction {
value: I32(-1138913271),
bool: true,
key: 488447261,
pos: 2097865012304223517,
length: 2097865012304223517,
prop: 2097865012304223517,
}),
},
Handle {
site: 29,
target: 29,
container: 29,
action: Generic(GenericAction {
value: Container(Text),
bool: true,
key: 488447261,
pos: 2097865012304223517,
length: 2097865012304223517,
prop: 2097865012304223517,
}),
},
Handle {
site: 29,
target: 29,
container: 29,
action: Generic(GenericAction {
value: Container(Unknown(255)),
bool: true,
key: 0,
pos: 13474770085092589312,
length: 18446744073709486369,
prop: 2089670231998729471,
}),
},
Handle {
site: 144,
target: 144,
container: 144,
action: Generic(GenericAction {
value: I32(488447261),
bool: true,
key: 488447261,
pos: 18374403900856671517,
length: 15987178339521527808,
prop: 11791512336840195838,
}),
},
SyncAllUndo {
site: 147,
op_len: 2475922323,
},
SyncAllUndo {
site: 147,
op_len: 2475922323,
},
SyncAllUndo {
site: 147,
op_len: 2475922323,
},
SyncAllUndo {
site: 147,
op_len: 2475922323,
},
SyncAllUndo {
site: 147,
op_len: 2475922323,
},
SyncAllUndo {
site: 147,
op_len: 2475922323,
},
SyncAllUndo {
site: 147,
op_len: 2475922323,
},
SyncAllUndo {
site: 147,
op_len: 2475922323,
},
SyncAllUndo {
site: 147,
op_len: 2475922323,
},
SyncAllUndo {
site: 155,
op_len: 4292972707,
},
SyncAllUndo {
site: 147,
op_len: 2475922323,
},
SyncAllUndo {
site: 147,
op_len: 2475922323,
},
SyncAllUndo {
site: 147,
op_len: 2475922323,
},
SyncAllUndo {
site: 149,
op_len: 3784352667,
},
Handle {
site: 0,
target: 0,
container: 0,
action: Generic(GenericAction {
value: I32(-1157627904),
bool: true,
key: 4280548865,
pos: 18446744073709551615,
length: 18446744073709551615,
prop: 18446744073709551615,
}),
},
SyncAll,
SyncAll,
SyncAll,
Undo {
site: 127,
op_len: 2139062143,
},
Handle {
site: 7,
target: 7,
container: 7,
action: Generic(GenericAction {
value: I32(-510654713),
bool: true,
key: 16777215,
pos: 1095216660480,
length: 18446742979058794496,
prop: 82188550010830847,
}),
},
],
)
}
#[test]
fn minify() {
minify_error(

View file

@ -32,7 +32,7 @@ use std::{
ops::Deref,
sync::{Arc, Mutex, Weak},
};
use tracing::{debug, error, info, instrument, trace};
use tracing::{error, info, instrument, trace};
pub use tree::TreeHandler;
mod movable_list_apply_delta;

View file

@ -38,15 +38,14 @@ impl MovableListHandler {
unimplemented!();
}
MaybeDetached::Attached(_) => {
debug!(
"Movable list value before apply_delta: {:#?}",
self.get_deep_value_with_id()
);
debug!("Applying delta: {:#?}", &delta);
// debug!(
// "Movable list value before apply_delta: {:#?}",
// self.get_deep_value_with_id()
// );
// debug!("Applying delta: {:#?}", &delta);
// Preprocess deletions to build a map of containers to delete.
let mut to_delete = self.preprocess_deletions(&delta)?;
let mut to_delete = self.preprocess_deletions(&delta);
// Process insertions and moves.
let mut index = 0;
let mut index_shift = 0;
@ -81,13 +80,15 @@ impl MovableListHandler {
next_deleted: &mut next_deleted,
};
self.process_replacements(value, attr, &mut context)?;
self.process_replacements(value, attr, &mut context)
.unwrap();
}
}
}
// Apply any remaining deletions.
self.apply_remaining_deletions(&delta, &mut deleted_indices)?;
self.apply_remaining_deletions(&delta, &mut deleted_indices)
.unwrap();
Ok(())
}
@ -109,7 +110,7 @@ impl MovableListHandler {
loro_delta::array_vec::ArrayVec<ValueOrHandler, 8>,
crate::event::ListDeltaMeta,
>,
) -> LoroResult<FxHashMap<ContainerID, usize>> {
) -> FxHashMap<ContainerID, usize> {
let mut index = 0;
let mut to_delete = FxHashMap::default();
@ -131,7 +132,7 @@ impl MovableListHandler {
}
}
Ok(to_delete)
to_delete
}
/// Handles deletions within a replace operation.
@ -190,8 +191,13 @@ impl MovableListHandler {
}
ValueOrHandler::Handler(handler) => {
let mut old_id = handler.id();
if !context.to_delete.contains_key(&old_id) {
while let Some(new_id) = context.container_remap.get(&old_id) {
old_id = new_id.clone();
if context.to_delete.contains_key(&old_id) {
break;
}
}
}
if let Some(old_index) = context.to_delete.remove(&old_id) {

View file

@ -1008,6 +1008,7 @@ impl ContainerState for MovableListState {
unreachable!()
};
// let start_value = self.get_value();
if cfg!(debug_assertions) {
self.inner.check_consistency();
}
@ -1248,9 +1249,8 @@ impl ContainerState for MovableListState {
}
}
if cfg!(debug_assertions) {
// if cfg!(debug_assertions) {
// self.inner.check_consistency();
// let start_value = start_value.unwrap();
// let mut end_value = start_value.clone();
// end_value.apply_diff_shallow(&[Diff::List(event.clone())]);
// let cur_value = self.get_value();
@ -1260,7 +1260,7 @@ impl ContainerState for MovableListState {
// start_value, event, cur_value, end_value
// );
// self.check_get_child_index_correctly();
}
// }
Diff::List(event)
}