mirrors/loro
1
0
Fork 0
mirror of https://github.com/loro-dev/loro.git synced 2025-02-10 22:39:29 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

fix: tree undo when converting movement to creation, new target id should be used (#468)

Browse source 
* test: a checkout event err

* fix: tree undo convert moving to creating should use new target tree id

---------

Co-authored-by: Zixuan Chen <remch183@outlook.com>
This commit is contained in:
Leon Zhao 2024-09-20 16:51:37 +08:00 committed by GitHub
parent d460346585
commit aab6730b4d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 137 additions and 11 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
crates/fuzz/src/container/tree.rs
View file

@ -10,6 +10,7 @@ use loro::{
event::Diff, Container, ContainerID, ContainerType, LoroDoc, LoroError, LoroTree, LoroValue,
TreeExternalDiff, TreeID,
};
use tracing::trace;
use crate::{
actions::{Actionable, FromGenericAction, GenericAction},
@ -420,8 +421,8 @@ impl ApplyDiff for TreeTracker {
}
fn apply_diff(&mut self, diff: Diff) {
// trace!("current tree: {:#?}", &self.tree);
// trace!("applying diff: {:#?}", &diff);
trace!("current tree: {:#?}", &self.tree);
trace!("applying diff: {:#?}", &diff);
let diff = diff.as_tree().unwrap();
for diff in &diff.diff {
let target = diff.target;
@ -438,6 +439,7 @@ impl ApplyDiff for TreeTracker {
self.create_node(target, &parent.tree_id(), position.to_string(), index);
}
TreeExternalDiff::Delete { .. } => {
trace!("To delete {:?}", &target);
let node = self.find_node_by_id(target).unwrap();
if let Some(parent) = node.parent {
let parent = self.find_node_by_id_mut(parent).unwrap();

93
crates/fuzz/tests/test.rs
View file

@ -10948,6 +10948,99 @@ fn gc_fuzz_21() {
)
}
#[test]
fn gc_fuzz_22() {
test_multi_sites_with_gc(
5,
vec![FuzzTarget::All],
&mut [
Handle {
site: 59,
target: 27,
container: 147,
action: Generic(GenericAction {
value: I32(-1819044973),
bool: true,
key: 16814995,
pos: 6590743253515379200,
length: 50581795804069894,
prop: 506381212763488288,
}),
},
Handle {
site: 7,
target: 7,
container: 7,
action: Generic(GenericAction {
value: I32(1792),
bool: false,
key: 0,
pos: 18446580246477012992,
length: 0,
prop: 8097874551267853056,
}),
},
SyncAll,
Handle {
site: 27,
target: 27,
container: 27,
action: Generic(GenericAction {
value: Container(Text),
bool: true,
key: 2475922323,
pos: 761987946344649619,
length: 4216591183764737623,
prop: 7308324466053836044,
}),
},
Handle {
site: 59,
target: 27,
container: 147,
action: Generic(GenericAction {
value: I32(-1819044973),
bool: true,
key: 16814995,
pos: 6590743253515379200,
length: 50581795804069894,
prop: 506381212763488288,
}),
},
SyncAll,
Handle {
site: 27,
target: 27,
container: 27,
action: Generic(GenericAction {
value: Container(Text),
bool: true,
key: 2475922323,
pos: 761987946344649619,
length: 9583474564706815575,
prop: 7809911865117314106,
}),
},
Handle {
site: 27,
target: 27,
container: 27,
action: Generic(GenericAction {
value: I32(-1819082494),
bool: true,
key: 2475922323,
pos: 5644992284945547520,
length: 881162909321221975,
prop: 6874019576048676717,
}),
},
SyncAllUndo {
site: 159,
op_len: 2678038431,
},
],
)
}
#[test]
fn minify() {
minify_error(

13
crates/loro-internal/src/diff_calc/tree.rs
View file

@ -467,12 +467,23 @@ impl Ord for MoveLamportAndID {
}
}
#[derive(Debug, Clone, Default)]
#[derive(Clone, Default)]
pub(crate) struct TreeCacheForDiff {
tree: FxHashMap<TreeID, BTreeSet<MoveLamportAndID>>,
current_vv: VersionVector,
}
impl std::fmt::Debug for TreeCacheForDiff {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
writeln!(f, "TreeCacheForDiff {{ tree: ")?;
for (id, ops) in self.tree.iter() {
writeln!(f, " {} -> {:?}", id, ops)?;
}
writeln!(f, " current_vv: {:?}", self.current_vv)?;
Ok(())
}
}
impl TreeCacheForDiff {
fn is_ancestor_of(&self, maybe_ancestor: &TreeID, node_id: &TreeParentId) -> bool {
if !self.tree.contains_key(maybe_ancestor) {

16
crates/loro-internal/src/handler.rs
View file

@ -1226,7 +1226,21 @@ impl Handler {
remap_tree_id(p, container_remap)
}
remap_tree_id(&mut target, container_remap);
x.move_at_with_target_for_apply_diff(parent, position, target)?;
// determine if the target is deleted
if x.is_node_unexist(&target) || x.is_node_deleted(&target).unwrap() {
// create the target node, we should use the new target id
let new_target = x.__internal__next_tree_id();
if x.create_at_with_target_for_apply_diff(
parent, position, new_target,
)? {
container_remap.insert(
target.associated_meta_container(),
new_target.associated_meta_container(),
);
}
} else {
x.move_at_with_target_for_apply_diff(parent, position, target)?;
}
}
TreeExternalDiff::Delete { .. } => {
remap_tree_id(&mut target, container_remap);

10
crates/loro-internal/src/handler/tree.rs
View file

@ -414,10 +414,10 @@ impl TreeHandler {
unreachable!();
};
// the move node does not exist, create it
if self.is_node_unexist(&target) || self.is_node_deleted(&target).unwrap() {
return self.create_at_with_target_for_apply_diff(parent, position, target);
}
// // the move node does not exist, create it
// if self.is_node_unexist(&target) || self.is_node_deleted(&target).unwrap() {
// return self.create_at_with_target_for_apply_diff(parent, position, target);
// }
if let Some(p) = self.get_node_parent(&target) {
if p == parent {
@ -721,7 +721,7 @@ impl TreeHandler {
}
}
/// Get the parent of the node, if the node is deleted or does not exist, return None
/// Get the parent of the node, if the node does not exist, return None
pub fn get_node_parent(&self, target: &TreeID) -> Option<TreeParentId> {
match &self.inner {
MaybeDetached::Detached(t) => {

10
crates/loro-internal/src/state/tree_state.rs
View file

@ -14,6 +14,7 @@ use std::collections::VecDeque;
use std::fmt::Debug;
use std::ops::{Deref, DerefMut};
use std::sync::{Arc, Mutex, Weak};
use tracing::trace;
use super::{ContainerState, DiffApplyContext};
use crate::container::idx::ContainerIdx;
@ -699,7 +700,7 @@ impl TreeState {
}
}
/// Get the parent of the node, if the node is deleted or does not exist, return None
/// Get the parent of the node, if the node does not exist, return None
pub fn parent(&self, target: &TreeID) -> Option<TreeParentId> {
self.trees.get(target).map(|x| x.parent)
}
@ -969,6 +970,7 @@ impl ContainerState for TreeState {
if self.is_node_deleted(&target).unwrap() {
if was_alive {
// delete event
trace!("DEL from c");
ans.push(TreeDiffItem {
target,
action: TreeExternalDiff::Delete {
@ -1035,10 +1037,11 @@ impl ContainerState for TreeState {
}
TreeInternalDiff::Delete { parent, position } => {
let mut send_event = true;
if need_check && self.is_node_deleted(&target).unwrap() {
if self.is_node_deleted(&target).unwrap() {
send_event = false;
}
if send_event {
trace!("DEL from A");
ans.push(TreeDiffItem {
target,
action: TreeExternalDiff::Delete {
@ -1057,6 +1060,8 @@ impl ContainerState for TreeState {
TreeInternalDiff::UnCreate => {
// maybe the node created and moved to the parent deleted
if !self.is_node_deleted(&target).unwrap() {
trace!("tree {:#?}", &self.trees);
trace!("DEL from b {:?}", target);
ans.push(TreeDiffItem {
target,
action: TreeExternalDiff::Delete {
@ -1296,6 +1301,7 @@ pub(crate) fn get_meta_value(nodes: &mut Vec<LoroValue>, state: &mut DocState) {
}
}
#[derive(Debug)]
pub(crate) struct TreeNode {
pub(crate) id: TreeID,
pub(crate) parent: TreeParentId,