diff --git a/.changeset/dead-container-cache.md b/.changeset/dead-container-cache.md new file mode 100644 index 000000000..766957625 --- /dev/null +++ b/.changeset/dead-container-cache.md @@ -0,0 +1,5 @@ +--- +"loro-crdt": patch +--- + +Fix containers that stayed deleted after being revived. After `isDeleted()` was called on the metadata of a tree node under a deleted ancestor, moving the subtree back (locally or through an import) left that metadata reported as deleted, and edits to it threw until the document was reloaded. The same happened to a movable-list child revived by importing a concurrent move. Cached deletions of tree metadata and movable-list children are now dropped whenever a tree move or a movable-list change is applied. diff --git a/AGENTS.md b/AGENTS.md index d1c2488f8..df857e4d9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -41,6 +41,8 @@ Loro is a Rust CRDT workspace with JS/WASM packaging and a MoonBit codec. - Bounded decoded-value cache in `InnerStore` (second-chance FIFO, eviction safety contract, loro-dev/loro#1092): [context/container-value-cache.md](context/container-value-cache.md). +- Which deletions `DocState::is_deleted` caches, and how tree/movable-list + children are revived: [context/dead-container-cache.md](context/dead-container-cache.md). - Movable-list `apply_diff`/undo: delete + re-insert of a child becomes a move, cursor invariant, pitfalls: [context/movable-list-apply-diff.md](context/movable-list-apply-diff.md). - Context backlog: [context/CONTEXT-GAPS.md](context/CONTEXT-GAPS.md). diff --git a/context/dead-container-cache.md b/context/dead-container-cache.md new file mode 100644 index 000000000..5e42fd44a --- /dev/null +++ b/context/dead-container-cache.md @@ -0,0 +1,78 @@ +# Dead Container Cache + +Verified against code 2026-09-28. + +`DocState::is_deleted` (`crates/loro-internal/src/state/dead_containers_cache.rs`) +answers whether a container is unreachable from a root: it walks the arena +parent chain and asks each parent whether it still holds the child +(`DocState::contains_logical_child`). It runs for every local op +(`Transaction::apply_local_op` rejects ops on deleted containers) and for each +handler's `is_deleted()`. + +## What is cached + +A deleted answer is cached for the container whose edge is broken and every +container below it on the walked chain, in one of two sets: + +- `final_deletions`: the parent that lost the chain is a Map (a key only + changes to a newer value, so a replaced or deleted child never becomes the + value again) or a List (deletes are final). While the state is at the + oplog's latest version, no later version gives these back. While it is + behind (after `checkout`, or detached while imports arrive), a container + created after the state's version is also cut at its map or list parent, + and the next checkout forward brings it in. +- `revivable`: the parent is a Tree or a MovableList. Moving a node, or one of + its ancestors, out of a deleted subtree revives the node's metadata and + everything below it (a local `mov`, an imported move, or a concurrent move + whose lamport is greater than the delete's). A movable-list delete + concurrent with a move of the same element keeps the element when the move + has the greater lamport, so importing the move revives it. + +Not cached: children behind a mergeable edge (the parent map's marker can come +back) and containers without a parent in the arena (a later op can still +attach them). + +Release builds return a cached `true` without walking; debug builds always +walk and assert that no cached entry on the chain contradicts the result. + +## Invalidation + +- `DocState::apply_diff` clears both sets when the direction mode is + `Checkout` (moving backwards can revive anything) or the diff comes from a + checkout (`EventTriggerKind::Checkout`). The latter covers forward + checkouts (`attach`, `checkout_to_latest`, `checkout` to a newer version, + and `import_batch`'s reattach), whose direction mode is `Import`, `Linear`, + or `ImportGreaterUpdates`. The only checkout triggered as `Import` is the + snapshot import into an empty document, whose state is new. +- `DocState::apply_diff` in any other mode clears `revivable` when the batch + touches a Tree or MovableList container. +- `DocState::apply_local_op` clears `revivable` before a tree `Move`. A local + op cannot address a deleted movable-list element. +- Resets replace the cache. + +`clear_revivable` is O(1) when the set is empty, so documents that never query +deleted containers pay nothing. + +Undo, redo, `revert_to`, and `apply_diff` do not revive container IDs by +themselves: `Handler::apply_diff` creates a new tree node for a deleted target +and remaps its metadata, and map/list children are recreated with new IDs. +Their tree moves still go through `apply_local_op`. Only mergeable children +keep their IDs, and those are not cached. + +Before 2026-09-28 every deletion went into one set that forward transitions +never cleared (`clear_alive` removed nothing, since only deletions were +stored), so after `is_deleted()` a revived container still counted as deleted +in release builds and rejected edits until the document was reloaded. The +same held for a container queried at an older version and brought in by a +forward checkout. + +The debug assertion runs while the caller holds the state lock, so a stale +entry aborts the process (the unwinding code locks the poisoned state again), +like any other internal assertion under that lock. + +## Tests + +- `crates/loro/tests/dead_container_cache.rs`: every revival route. Run it with + `--release` as well; debug builds recompute the answer. +- Unit tests in `dead_containers_cache.rs` check the cache entries directly, + so they also fail in debug builds when an invalidation is missing. diff --git a/crates/loro-internal/src/state.rs b/crates/loro-internal/src/state.rs index f44d487f7..7bb0db897 100644 --- a/crates/loro-internal/src/state.rs +++ b/crates/loro-internal/src/state.rs @@ -18,7 +18,7 @@ use tracing::{info_span, instrument, warn}; use crate::{ configure::{Configure, DefaultRandom, SecureRandomGenerator}, - container::{idx::ContainerIdx, richtext::config::StyleConfigMap}, + container::{idx::ContainerIdx, richtext::config::StyleConfigMap, tree::tree_op::TreeOp}, cursor::{Cursor, PosType}, delta::TreeExternalDiff, diff_calc::{DiffCalculator, DiffMode}, @@ -27,7 +27,7 @@ use crate::{ handler::ValueOrHandler, id::PeerID, lock::{LoroLockGroup, LoroMutex}, - op::{Op, RawOp}, + op::{Op, RawOp, RawOpContent}, version::{Frontiers, VersionVector}, ContainerDiff, ContainerType, DocDiff, InternalString, LoroDocInner, LoroValue, OpLog, }; @@ -681,17 +681,22 @@ impl DocState { } // `diff_mode` here is the DIRECTION mode (`origin_diff_mode` from // `calc_diff_internal`), not the mode the calculators computed with: - // Checkout means the transition may go backwards, so any cached - // dead/alive knowledge can be invalidated; every other mode implies a - // forward transition, where alive-markers may change but dead - // containers stay dead unless a diff revives them. - match diff_mode { - DiffMode::Checkout => { - self.dead_containers_cache.clear(); - } - _ => { - self.dead_containers_cache.clear_alive(); - } + // Checkout means the transition may go backwards, which can revive any + // container. Any checkout also clears it, even a forward one + // (`attach`, `checkout_to_latest`): while the state was behind the + // oplog, a container created later looked cut at its map or list + // parent. Every other forward transition keeps the state at the + // oplog's latest version, where only a tree or movable-list move can + // revive a container (see `dead_containers_cache.rs`). + if diff_mode == DiffMode::Checkout || diff.by == EventTriggerKind::Checkout { + self.dead_containers_cache.clear(); + } else if diffs.iter().any(|d| { + matches!( + d.idx.get_type(), + ContainerType::Tree | ContainerType::MovableList + ) + }) { + self.dead_containers_cache.clear_revivable(); } self.pre_txn(diff.origin.clone(), diff.by); @@ -997,11 +1002,14 @@ impl DocState { if self.in_txn { self.changed_idx_in_txn.insert(op.container); } - let ret = state.apply_local_op(raw_op, op)?; - if !ret.deleted_containers.is_empty() { - self.dead_containers_cache.clear_alive(); + // A local move can take a node out of a deleted subtree. Movable-list + // elements cannot be revived locally: a deleted one has no index. + if let RawOpContent::Tree(tree_op) = &raw_op.content { + if matches!(**tree_op, TreeOp::Move { .. }) { + self.dead_containers_cache.clear_revivable(); + } } - + state.apply_local_op(raw_op, op)?; Ok(()) } diff --git a/crates/loro-internal/src/state/AGENTS.md b/crates/loro-internal/src/state/AGENTS.md index 60e6c9a28..d35af4227 100644 --- a/crates/loro-internal/src/state/AGENTS.md +++ b/crates/loro-internal/src/state/AGENTS.md @@ -22,8 +22,10 @@ before changing mergeable child behavior. [../../../../context/shallow-snapshot-style-redaction.md](../../../../context/shallow-snapshot-style-redaction.md) before changing the richtext snapshot codec or that pass. - `mergeable.rs`: logical child edge resolution for mergeable containers. -- `dead_containers_cache.rs`: dead/alive tracking and marker-driven mergeable - reactivation. +- `dead_containers_cache.rs`: dead/alive tracking. Map/list deletions are + final; tree and movable-list ones are dropped whenever a move could revive + them; mergeable ones are never cached: + [../../../../context/dead-container-cache.md](../../../../context/dead-container-cache.md). - `unknown_state.rs` and `../diff_calc/unknown.rs`: forward compatibility for unknown container types. diff --git a/crates/loro-internal/src/state/dead_containers_cache.rs b/crates/loro-internal/src/state/dead_containers_cache.rs index 3a15e3fa2..d3382fa7a 100644 --- a/crates/loro-internal/src/state/dead_containers_cache.rs +++ b/crates/loro-internal/src/state/dead_containers_cache.rs @@ -1,20 +1,37 @@ use super::DocState; use crate::container::idx::ContainerIdx; -use rustc_hash::FxHashMap; +use loro_common::ContainerType; +use rustc_hash::FxHashSet; use smallvec::SmallVec; +/// Containers known to be deleted. See `context/dead-container-cache.md`. #[derive(Default, Debug, Clone)] pub(super) struct DeadContainersCache { - cache: FxHashMap, + /// Removed from a Map or List: no later version gives them back. + final_deletions: FxHashSet, + /// Cut off at a Tree or MovableList edge: a later move can revive them. + revivable: FxHashSet, } impl DeadContainersCache { + /// Must be called whenever the state may move to a version that is not a + /// descendant of the current one (checkout, reset), because that can + /// revive any container. pub fn clear(&mut self) { - self.cache.clear(); + self.final_deletions.clear(); + self.revivable.clear(); } - pub(crate) fn clear_alive(&mut self) { - self.cache.retain(|_, is_deleted| *is_deleted); + /// Must be called whenever a tree or movable-list change is applied going + /// forward, because it may move a deleted node or element back. + pub fn clear_revivable(&mut self) { + if !self.revivable.is_empty() { + self.revivable.clear(); + } + } + + fn contains(&self, idx: &ContainerIdx) -> bool { + self.final_deletions.contains(idx) || self.revivable.contains(idx) } } @@ -22,8 +39,7 @@ impl DocState { pub(crate) fn is_deleted(&mut self, idx: ContainerIdx) -> bool { #[cfg(not(debug_assertions))] { - // Cache stores only deleted containers. - if self.dead_containers_cache.cache.contains_key(&idx) { + if self.dead_containers_cache.contains(&idx) { return true; } } @@ -34,14 +50,15 @@ impl DocState { visited.push(idx); let mut idx = idx; let mut depends_on_mergeable_edge = false; - let is_deleted = loop { + // The parent that no longer holds the chain, if any. + let (is_deleted, cut_at) = loop { let id = self.arena.idx_to_id(idx).unwrap(); if id.is_mergeable() { depends_on_mergeable_edge = true; } if let Some(parent_idx) = self.arena.get_parent(idx) { if !self.contains_logical_child(parent_idx, &id) { - break true; + break (true, Some(parent_idx)); } idx = parent_idx; @@ -49,39 +66,34 @@ impl DocState { } else { // No parent in the arena: top-level Roots are always alive; anything else // (including a mergeable Root whose parent edge was never wired) is treated - // as deleted. - break !id.is_root() || id.is_mergeable(); + // as deleted. A later op can still attach it, so this is never cached. + break (!id.is_root() || id.is_mergeable(), None); } }; + // Every container on the walked chain shares the answer: below the + // missing edge all of them are deleted, otherwise all are alive. #[cfg(debug_assertions)] - { - if !depends_on_mergeable_edge { - if let Some(cached_is_deleted) = self.dead_containers_cache.cache.get(&idx) { - assert_eq!(is_deleted, *cached_is_deleted); - } - } + for idx in visited.iter() { + assert!( + is_deleted || !self.dead_containers_cache.contains(idx), + "stale dead-container cache entry for {:?}", + self.arena.idx_to_id(*idx) + ); } // A mergeable ancestor can be deleted and later reactivated by changing the parent map's // marker. Do not cache deletion for any descendant whose liveness depends on that // logical edge, including ordinary children nested inside a mergeable map. - if depends_on_mergeable_edge { - if !is_deleted { - for idx in visited { - self.dead_containers_cache.cache.remove(&idx); + if is_deleted && !depends_on_mergeable_edge { + match cut_at.map(|parent| parent.get_type()) { + Some(ContainerType::Map | ContainerType::List) => { + self.dead_containers_cache.final_deletions.extend(visited) } - } - return is_deleted; - } - - if is_deleted { - for idx in visited { - self.dead_containers_cache.cache.insert(idx, true); - } - } else { - for idx in visited { - self.dead_containers_cache.cache.remove(&idx); + Some(ContainerType::Tree | ContainerType::MovableList) => { + self.dead_containers_cache.revivable.extend(visited) + } + _ => {} } } @@ -90,7 +102,7 @@ impl DocState { #[cfg(test)] pub(crate) fn dead_cache_entry(&self, idx: ContainerIdx) -> Option { - self.dead_containers_cache.cache.get(&idx).copied() + self.dead_containers_cache.contains(&idx).then_some(true) } } @@ -243,4 +255,129 @@ mod tests { "imported reactivation must drop the stale deleted-cache entry" ); } + + /// A tree node under a deleted ancestor is revived by moving it out, so a + /// local move must drop the cached deletion of its metadata. + #[test] + fn local_tree_move_drops_revivable_deletions() { + use crate::state::TreeParentId; + + let doc = LoroDoc::new_auto_commit(); + doc.set_peer_id(1).unwrap(); + let tree = doc.get_tree("tree"); + let parent = tree.create(TreeParentId::Root).unwrap(); + let child = tree.create(TreeParentId::Node(parent)).unwrap(); + let meta = tree.get_meta(child).unwrap(); + doc.commit_then_renew(); + tree.delete(parent).unwrap(); + doc.commit_then_renew(); + + let idx = doc.state.lock().arena.id_to_idx(&meta.id()).unwrap(); + assert!(meta.is_deleted()); + assert_eq!(doc.state.lock().dead_cache_entry(idx), Some(true)); + + tree.mov(child, TreeParentId::Root).unwrap(); + assert_eq!(doc.state.lock().dead_cache_entry(idx), None); + doc.commit_then_renew(); + assert!(!meta.is_deleted()); + } + + /// A movable-list element deleted locally is revived by a concurrent move + /// with a greater lamport, so importing movable-list changes must drop the + /// cached deletion. + #[test] + fn movable_list_import_drops_revivable_deletions() { + use crate::{loro::ExportMode, MapHandler}; + + let doc = LoroDoc::new_auto_commit(); + doc.set_peer_id(1).unwrap(); + let list = doc.get_movable_list("list"); + list.insert(0, 0).unwrap(); + let child = list + .insert_container(1, MapHandler::new_detached()) + .unwrap(); + doc.commit_then_renew(); + let other = LoroDoc::new_auto_commit(); + other.set_peer_id(2).unwrap(); + other + .import(&doc.export(ExportMode::all_updates()).unwrap()) + .unwrap(); + other.get_map("pad").insert("k", 1).unwrap(); + other.commit_then_renew(); + other.get_movable_list("list").mov(1, 0).unwrap(); + other.commit_then_renew(); + + list.delete(1, 1).unwrap(); + doc.commit_then_renew(); + let idx = doc.state.lock().arena.id_to_idx(&child.id()).unwrap(); + assert!(child.is_deleted()); + assert_eq!(doc.state.lock().dead_cache_entry(idx), Some(true)); + + doc.import(&other.export(ExportMode::all_updates()).unwrap()) + .unwrap(); + assert_eq!(doc.state.lock().dead_cache_entry(idx), None); + assert!(!child.is_deleted()); + } + + /// Map and list removals are final for every later version, so they are + /// cached for the child and everything below it. + #[test] + fn map_and_list_child_deletions_are_cached() { + use crate::{ListHandler, MapHandler}; + + let doc = LoroDoc::new_auto_commit(); + doc.set_peer_id(1).unwrap(); + let map = doc.get_map("map"); + let in_map = map + .insert_container("k", MapHandler::new_detached()) + .unwrap(); + let nested = in_map + .insert_container("t", TextHandler::new_detached()) + .unwrap(); + let list = doc.get_list("list"); + let in_list = list + .insert_container(0, ListHandler::new_detached()) + .unwrap(); + doc.commit_then_renew(); + map.delete("k").unwrap(); + list.delete(0, 1).unwrap(); + doc.commit_then_renew(); + + assert!(nested.is_deleted()); + assert!(in_list.is_deleted()); + let state = doc.state.lock(); + for id in [in_map.id(), nested.id(), in_list.id()] { + let idx = state.arena.id_to_idx(&id).unwrap(); + assert_eq!(state.dead_cache_entry(idx), Some(true), "{id:?}"); + } + } + + /// While checked out to an older version, a map child created later is + /// cut at its map parent and cached as a final deletion. A forward + /// checkout is not a `DiffMode::Checkout` transition, but it must still + /// drop that entry. + #[test] + fn forward_checkout_drops_deletions_cached_behind_the_oplog() { + use crate::MapHandler; + + let doc = LoroDoc::new_auto_commit(); + doc.set_peer_id(1).unwrap(); + let map = doc.get_map("m"); + map.insert("a", 1).unwrap(); + doc.commit_then_renew(); + let v1 = doc.oplog_frontiers(); + let child = map + .insert_container("c", MapHandler::new_detached()) + .unwrap(); + doc.commit_then_renew(); + + doc.checkout(&v1).unwrap(); + let idx = doc.state.lock().arena.id_to_idx(&child.id()).unwrap(); + assert!(child.is_deleted()); + assert_eq!(doc.state.lock().dead_cache_entry(idx), Some(true)); + + doc.checkout_to_latest(); + assert_eq!(doc.state.lock().dead_cache_entry(idx), None); + assert!(!child.is_deleted()); + } } diff --git a/crates/loro-wasm/tests/dead_container_cache.test.ts b/crates/loro-wasm/tests/dead_container_cache.test.ts new file mode 100644 index 000000000..f06bc890b --- /dev/null +++ b/crates/loro-wasm/tests/dead_container_cache.test.ts @@ -0,0 +1,124 @@ +import { describe, expect, it } from "vitest"; +import { LoroDoc, LoroMap, LoroText } from "../bundler/index"; + +// A container that was reported deleted must count as alive again once it is +// revived. See context/dead-container-cache.md. + +function deletedSubtree() { + const doc = new LoroDoc(); + doc.setPeerId(1n); + const tree = doc.getTree("tree"); + const parent = tree.createNode(); + const child = tree.createNode(parent.id); + const grandchild = tree.createNode(child.id); + doc.commit(); + tree.delete(parent.id); + doc.commit(); + + const meta = tree.getNodeByID(grandchild.id)!.data; + expect(meta.isDeleted()).toBe(true); + expect(() => { + meta.set("k", 1); + doc.commit(); + }).toThrow(); + return { doc, tree, child, grandchild, meta }; +} + +describe("dead container cache", () => { + it("revives tree metadata after a local move", () => { + const { doc, tree, child, grandchild, meta } = deletedSubtree(); + tree.move(child.id, undefined, 0); + doc.commit(); + + expect(tree.isNodeDeleted(grandchild.id)).toBe(false); + expect(meta.isDeleted()).toBe(false); + meta.set("key", 1); + doc.commit(); + expect(tree.getNodeByID(grandchild.id)!.data.get("key")).toBe(1); + }); + + it("revives tree metadata after an imported move", () => { + const { doc, child, grandchild, meta } = deletedSubtree(); + const other = new LoroDoc(); + other.setPeerId(2n); + other.import(doc.export({ mode: "update" })); + other.getTree("tree").move(child.id, undefined, 0); + other.commit(); + + doc.import(other.export({ mode: "update", from: doc.oplogVersion() })); + expect(doc.getTree("tree").isNodeDeleted(grandchild.id)).toBe(false); + expect(meta.isDeleted()).toBe(false); + meta.set("key", 1); + doc.commit(); + }); + + it("revives a movable list child after importing a concurrent move", () => { + const a = new LoroDoc(); + a.setPeerId(1n); + const list = a.getMovableList("list"); + list.insert(0, 0); + const child = list.insertContainer(1, new LoroMap()); + child.set("x", 1); + a.commit(); + + const b = new LoroDoc(); + b.setPeerId(2n); + b.import(a.export({ mode: "update" })); + b.getMap("pad").set("k", 1); + b.commit(); + b.getMovableList("list").move(1, 0); + b.commit(); + + list.delete(1, 1); + a.commit(); + expect(child.isDeleted()).toBe(true); + + a.import(b.export({ mode: "update" })); + expect(list.length).toBe(2); + expect(child.isDeleted()).toBe(false); + child.set("y", 2); + a.commit(); + }); +}); + +describe("dead container cache after a forward checkout", () => { + it("drops the answer given at an older version", () => { + const doc = new LoroDoc(); + doc.setPeerId(1n); + doc.getMap("m").set("a", 1); + doc.commit(); + const v1 = doc.frontiers(); + const child = doc.getMap("m").setContainer("c", new LoroMap()); + doc.commit(); + + doc.checkout(v1); + expect(child.isDeleted()).toBe(true); + doc.checkoutToLatest(); + expect(child.isDeleted()).toBe(false); + child.set("x", 1); + doc.commit(); + expect(child.get("x")).toBe(1); + }); + + it("drops the answer given while detached after attach", () => { + const a = new LoroDoc(); + a.setPeerId(1n); + a.getMap("m").set("a", 1); + a.commit(); + const b = new LoroDoc(); + b.setPeerId(2n); + b.import(a.export({ mode: "update" })); + const text = a.getMap("m").setContainer("c", new LoroText()); + a.commit(); + + b.detach(); + b.import(a.export({ mode: "update" })); + const inB = b.getContainerById(text.id) as LoroText; + expect(inB.isDeleted()).toBe(true); + b.attach(); + expect(inB.isDeleted()).toBe(false); + inB.insert(0, "x"); + b.commit(); + expect(inB.toString()).toBe("x"); + }); +}); diff --git a/crates/loro/tests/dead_container_cache.rs b/crates/loro/tests/dead_container_cache.rs new file mode 100644 index 000000000..69682a57b --- /dev/null +++ b/crates/loro/tests/dead_container_cache.rs @@ -0,0 +1,344 @@ +//! A container's "deleted" answer must follow every way the container can be +//! revived. `DocState` caches deletions (`dead_containers_cache.rs`); these +//! tests query `is_deleted()` first so that an entry exists, then revive the +//! container and check both the answer and that edits are accepted. +//! +//! On `main` the cache kept the stale answer in release builds (debug builds +//! recompute and only cross-check one entry), so run these with `--release` +//! as well. + +use loro::{ + ContainerTrait, ExportMode, Frontiers, LoroDoc, LoroMap, LoroMovableList, LoroText, LoroTree, + TreeID, TreeParentId, UndoManager, +}; + +struct DeletedSubtree { + doc: LoroDoc, + tree: LoroTree, + child: TreeID, + grandchild: TreeID, + /// The metadata of `grandchild`, queried while its grandparent is deleted. + meta: LoroMap, + alive_version: Frontiers, +} + +/// `parent -> child -> grandchild`, with `parent` deleted and the deletion of +/// `grandchild`'s metadata already observed. +fn deleted_subtree(peer: u64) -> DeletedSubtree { + let doc = LoroDoc::new(); + doc.set_peer_id(peer).unwrap(); + let tree = doc.get_tree("tree"); + let parent = tree.create(TreeParentId::Root).unwrap(); + let child = tree.create(parent).unwrap(); + let grandchild = tree.create(child).unwrap(); + tree.get_meta(grandchild).unwrap().insert("v", 0).unwrap(); + doc.commit(); + let alive_version = doc.state_frontiers(); + tree.delete(parent).unwrap(); + doc.commit(); + + let meta = tree.get_meta(grandchild).unwrap(); + assert!(meta.is_deleted()); + assert!(meta.insert("k", 1).is_err()); + DeletedSubtree { + doc, + tree, + child, + grandchild, + meta, + alive_version, + } +} + +fn assert_meta_alive_and_editable(s: &DeletedSubtree) { + assert!(!s.tree.is_node_deleted(&s.grandchild).unwrap()); + assert!(!s.meta.is_deleted()); + s.meta.insert("after", 1).unwrap(); + s.doc.commit(); + assert_eq!( + s.tree + .get_meta(s.grandchild) + .unwrap() + .get("after") + .unwrap() + .into_value() + .unwrap(), + 1.into() + ); +} + +#[test] +fn local_move_revives_metadata_after_is_deleted_query() { + let s = deleted_subtree(1); + s.tree.mov(s.child, TreeParentId::Root).unwrap(); + s.doc.commit(); + assert_meta_alive_and_editable(&s); +} + +#[test] +fn move_and_edit_in_one_transaction_after_is_deleted_query() { + let s = deleted_subtree(1); + s.tree.mov(s.child, TreeParentId::Root).unwrap(); + s.meta.insert("k", 1).unwrap(); + s.doc.commit(); + assert_meta_alive_and_editable(&s); +} + +#[test] +fn imported_move_revives_metadata_after_is_deleted_query() { + let s = deleted_subtree(1); + let other = LoroDoc::new(); + other.set_peer_id(2).unwrap(); + other + .import(&s.doc.export(ExportMode::all_updates()).unwrap()) + .unwrap(); + other + .get_tree("tree") + .mov(s.child, TreeParentId::Root) + .unwrap(); + other.commit(); + + s.doc + .import( + &other + .export(ExportMode::updates(&s.doc.oplog_vv())) + .unwrap(), + ) + .unwrap(); + assert_meta_alive_and_editable(&s); +} + +#[test] +fn concurrent_move_revives_metadata_after_is_deleted_query() { + // The delete and the move are concurrent; the move has the greater lamport, + // so importing it revives the subtree. + let doc = LoroDoc::new(); + doc.set_peer_id(1).unwrap(); + let tree = doc.get_tree("tree"); + let parent = tree.create(TreeParentId::Root).unwrap(); + let child = tree.create(parent).unwrap(); + doc.commit(); + let other = LoroDoc::new(); + other.set_peer_id(2).unwrap(); + other + .import(&doc.export(ExportMode::all_updates()).unwrap()) + .unwrap(); + + tree.delete(parent).unwrap(); + doc.commit(); + let meta = tree.get_meta(child).unwrap(); + assert!(meta.is_deleted()); + + let other_tree = other.get_tree("tree"); + other_tree.create(TreeParentId::Root).unwrap(); + other.commit(); + other_tree.mov(parent, TreeParentId::Root).unwrap(); + other.commit(); + doc.import(&other.export(ExportMode::all_updates()).unwrap()) + .unwrap(); + + assert!(!tree.is_node_deleted(&child).unwrap()); + assert!(!meta.is_deleted()); + meta.insert("k", 1).unwrap(); + doc.commit(); +} + +/// `node` moved under a deleted node, with the deletion of its metadata +/// already observed. +fn node_moved_into_deleted_subtree(doc: &LoroDoc) -> (LoroTree, TreeID, LoroMap, Frontiers) { + let tree = doc.get_tree("tree"); + let deleted = tree.create(TreeParentId::Root).unwrap(); + let node = tree.create(TreeParentId::Root).unwrap(); + tree.get_meta(node).unwrap().insert("v", 1).unwrap(); + doc.commit(); + tree.delete(deleted).unwrap(); + doc.commit(); + let alive_version = doc.state_frontiers(); + tree.mov(node, deleted).unwrap(); + doc.commit(); + + let meta = tree.get_meta(node).unwrap(); + assert!(meta.is_deleted()); + assert!(meta.insert("k", 1).is_err()); + (tree, node, meta, alive_version) +} + +/// Undo and `revert_to` go through `Handler::apply_diff`, which never moves a +/// deleted node back: it creates a new node and copies the metadata. The old +/// metadata stays deleted, and the new one is alive. +fn assert_recreated_as_new_node(tree: &LoroTree, node: TreeID, meta: &LoroMap) { + assert!(tree.is_node_deleted(&node).unwrap()); + assert!(meta.is_deleted()); + assert!(meta.insert("k", 1).is_err()); + + let roots = tree.roots(); + assert_eq!(roots.len(), 1); + assert_ne!(roots[0], node); + let new_meta = tree.get_meta(roots[0]).unwrap(); + assert!(!new_meta.is_deleted()); + assert_eq!(new_meta.get("v").unwrap().into_value().unwrap(), 1.into()); + new_meta.insert("k", 1).unwrap(); +} + +#[test] +fn undo_after_is_deleted_query_recreates_node() { + let doc = LoroDoc::new(); + doc.set_peer_id(1).unwrap(); + let mut undo = UndoManager::new(&doc); + let (tree, node, meta, _) = node_moved_into_deleted_subtree(&doc); + + assert!(undo.undo().unwrap()); + assert_recreated_as_new_node(&tree, node, &meta); + doc.commit(); +} + +#[test] +fn revert_to_after_is_deleted_query_recreates_node() { + let doc = LoroDoc::new(); + doc.set_peer_id(1).unwrap(); + let (tree, node, meta, alive_version) = node_moved_into_deleted_subtree(&doc); + + doc.revert_to(&alive_version).unwrap(); + doc.commit(); + assert_recreated_as_new_node(&tree, node, &meta); + doc.commit(); +} + +#[test] +fn checkout_revives_metadata_after_is_deleted_query() { + let s = deleted_subtree(1); + s.doc.checkout(&s.alive_version).unwrap(); + assert!(!s.tree.is_node_deleted(&s.grandchild).unwrap()); + assert!(!s.meta.is_deleted()); + + s.doc.checkout_to_latest(); + assert!(s.meta.is_deleted()); +} + +#[test] +fn imported_movable_list_move_revives_child_after_is_deleted_query() { + // Loro keeps a movable-list element deleted by one peer if another peer + // concurrently moves it with a greater lamport. + let a = LoroDoc::new(); + a.set_peer_id(1).unwrap(); + let list = a.get_movable_list("list"); + list.insert(0, 0).unwrap(); + let child = list.insert_container(1, LoroMap::new()).unwrap(); + child.insert("x", 1).unwrap(); + a.commit(); + + let b = LoroDoc::new(); + b.set_peer_id(2).unwrap(); + b.import(&a.export(ExportMode::all_updates()).unwrap()) + .unwrap(); + b.get_map("pad").insert("k", 1).unwrap(); + b.commit(); + b.get_movable_list("list").mov(1, 0).unwrap(); + b.commit(); + + list.delete(1, 1).unwrap(); + a.commit(); + assert!(child.is_deleted()); + assert!(child.insert("y", 1).is_err()); + + a.import(&b.export(ExportMode::all_updates()).unwrap()) + .unwrap(); + assert_eq!(list.len(), 2); + assert!(!child.is_deleted()); + child.insert("y", 2).unwrap(); + a.commit(); +} + +#[test] +fn map_and_list_deletions_stay_deleted() { + let doc = LoroDoc::new(); + doc.set_peer_id(1).unwrap(); + let map = doc.get_map("map"); + let in_map = map.insert_container("k", LoroMap::new()).unwrap(); + let list = doc.get_list("list"); + let in_list = list.insert_container(0, LoroMovableList::new()).unwrap(); + doc.commit(); + map.delete("k").unwrap(); + list.delete(0, 1).unwrap(); + doc.commit(); + + for _ in 0..2 { + assert!(in_map.is_deleted()); + assert!(in_list.is_deleted()); + assert!(in_map.insert("x", 1).is_err()); + assert!(in_list.insert(0, 1).is_err()); + } +} + +/// While checked out to an older version, a container created later is +/// reported deleted (it is cut at its map parent in that state). Going +/// forward again, by `checkout_to_latest`, `attach`, or a checkout to a newer +/// version, must drop that answer. +#[test] +fn forward_checkout_revives_containers_created_after_the_old_version() { + let doc = LoroDoc::new(); + doc.set_peer_id(1).unwrap(); + doc.get_map("m").insert("a", 1).unwrap(); + let tree = doc.get_tree("tree"); + let node = tree.create(TreeParentId::Root).unwrap(); + doc.commit(); + let v1 = doc.oplog_frontiers(); + let child = doc + .get_map("m") + .insert_container("c", LoroMap::new()) + .unwrap(); + let title = tree + .get_meta(node) + .unwrap() + .insert_container("title", LoroText::new()) + .unwrap(); + doc.commit(); + let v2 = doc.oplog_frontiers(); + doc.get_map("m").insert("b", 2).unwrap(); + doc.commit(); + + for forward in [ + |doc: &LoroDoc, _: &Frontiers| doc.checkout_to_latest(), + |doc: &LoroDoc, _: &Frontiers| doc.attach(), + |doc: &LoroDoc, v2: &Frontiers| doc.checkout(v2).unwrap(), + ] { + doc.checkout(&v1).unwrap(); + assert!(child.is_deleted()); + assert!(title.is_deleted()); + forward(&doc, &v2); + assert!(!child.is_deleted()); + assert!(!title.is_deleted()); + } + + doc.checkout_to_latest(); + child.insert("x", 1).unwrap(); + title.insert(0, "x").unwrap(); + doc.commit(); +} + +#[test] +fn attach_after_detached_import_revives_containers_queried_while_detached() { + let a = LoroDoc::new(); + a.set_peer_id(1).unwrap(); + a.get_map("m").insert("a", 1).unwrap(); + a.commit(); + let b = LoroDoc::new(); + b.set_peer_id(2).unwrap(); + b.import(&a.export(ExportMode::all_updates()).unwrap()) + .unwrap(); + let text = a + .get_map("m") + .insert_container("c", LoroText::new()) + .unwrap(); + a.commit(); + + b.detach(); + b.import(&a.export(ExportMode::all_updates()).unwrap()) + .unwrap(); + let in_b = b.get_text(text.id()); + assert!(in_b.is_deleted(), "not in b's detached state yet"); + b.attach(); + assert!(!in_b.is_deleted()); + in_b.insert(0, "x").unwrap(); + b.commit(); +}