From f78f6637406487beb6da926048c1dd009c9f5512 Mon Sep 17 00:00:00 2001 From: Nils Pukropp Date: Sat, 19 Sep 2026 14:31:40 +0200 Subject: [PATCH] feat(core,fuse): restore the whole trashed tree, and bump the SDK to 0.6.5 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The trash is a flat list of uids, but a delete has a shape: a folder, and often items inside it that were trashed separately beforehand. Restoring one of those in isolation is not what it means — a folder came back without the contents deleted with it, and a file came back inside a folder that is still trashed, which is to say nowhere the user can see. - Schema 29: `trash.parent_uid`, filled by the trash refresh from the node's parent, plus an index for the walk. - `expand_restore` takes the connected piece of the trashed tree around what was asked for (every trashed descendant, every trashed ancestor) and orders it into waves, shallowest first: the server cannot put a node back under a parent that is still trashed. A uid the listing does not know about is restored on its own, because the listing is materialised in chunks and can be stale. - `Core::restore` restores wave by wave, still streaming per-node outcomes, so one node the server refuses leaves the rest restored. The SDK bump brings the 6cbf2f44 port: permanent API errors are no longer retried on block transfers, expired block URLs are re-minted on the whole-file and public-link download paths, and a server-supplied Retry-After is capped at 120 s. --- Cargo.lock | 8 +- Cargo.toml | 4 +- crates/pdfs-core/src/db/migrations.rs | 37 +++- crates/pdfs-core/src/db/tests.rs | 14 ++ crates/pdfs-core/src/db/trash.rs | 27 ++- crates/pdfs-fuse/src/lib.rs | 276 +++++++++++++++++++++++--- docs/CHANGELOG.md | 20 +- docs/DEVELOPMENT.md | 2 + 8 files changed, 350 insertions(+), 38 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 18a6dde..3e405a7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3668,9 +3668,9 @@ dependencies = [ [[package]] name = "proton-drive-rs" -version = "0.6.4" +version = "0.6.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ae1ebbfe1696ae497807d87699a55d860612ceac250b9d60825bae0baccd622a" +checksum = "46cbe05623d9b73bd48b1473f42d43180142b2ee676b1cb9e9a06b562c60ba26" dependencies = [ "async-trait", "base64 0.23.1", @@ -3695,9 +3695,9 @@ dependencies = [ [[package]] name = "proton-sdk" -version = "0.6.4" +version = "0.6.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "de5426ca4b5682b6dc0399e7359f343c0146da6d8465196ee49bb8825f1a90ee" +checksum = "7310d93bed440dfe4fd60306923f6a491901ee0d618b65ace4e5e22ff0387c1f" dependencies = [ "aes-gcm 0.11.1", "async-trait", diff --git a/Cargo.toml b/Cargo.toml index 34537e9..a37dc30 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -15,8 +15,8 @@ keywords = ["proton", "drive", "fuse", "sync", "linux"] categories = ["filesystem", "command-line-utilities"] [workspace.dependencies] -proton-sdk = { version = "0.6.4" } -proton-drive-rs = { version = "0.6.4" } +proton-sdk = { version = "0.6.5" } +proton-drive-rs = { version = "0.6.5" } # Internal crates. pdfs-core = { path = "crates/pdfs-core" } diff --git a/crates/pdfs-core/src/db/migrations.rs b/crates/pdfs-core/src/db/migrations.rs index f410aad..966daf9 100644 --- a/crates/pdfs-core/src/db/migrations.rs +++ b/crates/pdfs-core/src/db/migrations.rs @@ -9,7 +9,7 @@ use super::Db; use crate::Result; /// Current schema version. Bump on every forward migration added below. -pub(super) const SCHEMA_VERSION: i64 = 28; +pub(super) const SCHEMA_VERSION: i64 = 29; impl Db { pub(super) fn migrate(&self) -> Result<()> { @@ -218,6 +218,24 @@ impl Db { tx.execute_batch(MIGRATION_V28)?; } } + if current < 29 { + // Same guards as V26/V27/V28: an old fixture may not have `trash`, + // and a fixture rewound from the current schema already has the + // column. + let has_column: bool = tx.query_row( + "SELECT COUNT(*) FROM pragma_table_info('trash') WHERE name = 'parent_uid'", + [], + |row| row.get::<_, i64>(0), + )? > 0; + let has_trash: bool = tx.query_row( + "SELECT COUNT(*) FROM sqlite_master WHERE type = 'table' AND name = 'trash'", + [], + |row| row.get::<_, i64>(0), + )? > 0; + if has_trash && !has_column { + tx.execute_batch(MIGRATION_V29)?; + } + } tx.execute( "INSERT INTO sync_state (key, value) VALUES ('schema_version', ?1) ON CONFLICT(key) DO UPDATE SET value = excluded.value", @@ -846,3 +864,20 @@ ALTER TABLE pending_op ADD COLUMN access_deferred_since INTEGER NOT NULL DEFAULT const MIGRATION_V28: &str = " ALTER TABLE sync_entry ADD COLUMN local_mtime_ns INTEGER; "; + +/// Schema v29: the trash listing remembers each trashed node's parent, so a +/// restore can reason about the shape of what it is putting back. +/// +/// Trash is a flat list of uids on the wire, but the user deletes a *tree*: a +/// folder and, often, items inside it that were trashed separately. Restoring +/// one of those in isolation is what the parent link fixes — a child restored +/// under a still-trashed parent lands somewhere invisible, and a folder restored +/// without its separately-trashed contents comes back empty. +/// +/// `NULL` means "this row predates the column" (or the node is a volume root); +/// the next trash refresh fills it in, and until then the restore behaves as it +/// did before. +const MIGRATION_V29: &str = " +ALTER TABLE trash ADD COLUMN parent_uid TEXT; +CREATE INDEX IF NOT EXISTS idx_trash_parent ON trash(parent_uid); +"; diff --git a/crates/pdfs-core/src/db/tests.rs b/crates/pdfs-core/src/db/tests.rs index 9f03a6c..5a4c314 100644 --- a/crates/pdfs-core/src/db/tests.rs +++ b/crates/pdfs-core/src/db/tests.rs @@ -254,6 +254,7 @@ fn trash_replace_lists_folders_first() { is_dir: false, size: 10, mtime: 1, + parent_uid: Some("t2".into()), }, StoredTrash { uid: "t2".into(), @@ -261,6 +262,7 @@ fn trash_replace_lists_folders_first() { is_dir: true, size: 0, mtime: 2, + parent_uid: None, }, ]) .unwrap(); @@ -271,6 +273,18 @@ fn trash_replace_lists_folders_first() { ["Alpha", "zeta.txt"] ); + // The parent links come back with the rows: a restore reads the tree shape + // from them. + let mut parents = db.trash_parents().unwrap(); + parents.sort(); + assert_eq!( + parents, + [ + ("t1".to_string(), Some("t2".to_string())), + ("t2".to_string(), None), + ] + ); + // A replace is a replace: emptying the trash on the server empties it here. db.trash_replace(&[]).unwrap(); assert!(db.trash_list().unwrap().is_empty()); diff --git a/crates/pdfs-core/src/db/trash.rs b/crates/pdfs-core/src/db/trash.rs index 583aeb9..f7c90dc 100644 --- a/crates/pdfs-core/src/db/trash.rs +++ b/crates/pdfs-core/src/db/trash.rs @@ -11,6 +11,10 @@ pub struct StoredTrash { pub is_dir: bool, pub size: i64, pub mtime: i64, + /// The folder this node was trashed from, when it is known. `None` for a + /// row written before schema v29, and for a node whose parent the server + /// did not report. + pub parent_uid: Option, } impl Db { @@ -20,7 +24,8 @@ impl Db { tx.execute("DELETE FROM trash", [])?; { let mut stmt = tx.prepare( - "INSERT INTO trash (uid, name, is_dir, size, mtime) VALUES (?1, ?2, ?3, ?4, ?5)", + "INSERT INTO trash (uid, name, is_dir, size, mtime, parent_uid) \ + VALUES (?1, ?2, ?3, ?4, ?5, ?6)", )?; for item in items { stmt.execute(params![ @@ -28,7 +33,8 @@ impl Db { item.name, item.is_dir as i64, item.size, - item.mtime + item.mtime, + item.parent_uid ])?; } } @@ -41,7 +47,7 @@ impl Db { pub fn trash_list(&self) -> Result> { let conn = self.read(); let mut stmt = conn.prepare( - "SELECT uid, name, is_dir, size, mtime FROM trash + "SELECT uid, name, is_dir, size, mtime, parent_uid FROM trash ORDER BY is_dir DESC, name COLLATE NOCASE", )?; let rows = stmt.query_map([], |r| { @@ -51,6 +57,7 @@ impl Db { is_dir: r.get::<_, i64>(2)? != 0, size: r.get(3)?, mtime: r.get(4)?, + parent_uid: r.get(5)?, }) })?; let mut items = Vec::new(); @@ -60,5 +67,19 @@ impl Db { Ok(items) } + /// Every trashed node paired with the folder it was trashed from, for the + /// restore's tree walk. Kept separate from [`Db::trash_list`] because the + /// restore wants only the links, not the names, sizes or ordering. + pub fn trash_parents(&self) -> Result)>> { + let conn = self.read(); + let mut stmt = conn.prepare("SELECT uid, parent_uid FROM trash")?; + let rows = stmt.query_map([], |r| Ok((r.get(0)?, r.get(1)?)))?; + let mut pairs = Vec::new(); + for row in rows { + pairs.push(row?); + } + Ok(pairs) + } + // ---- device sync (devices.md) ----------------------------------------- } diff --git a/crates/pdfs-fuse/src/lib.rs b/crates/pdfs-fuse/src/lib.rs index 86433a8..1c04cc6 100644 --- a/crates/pdfs-fuse/src/lib.rs +++ b/crates/pdfs-fuse/src/lib.rs @@ -4366,6 +4366,9 @@ impl Core { is_dir: node.is_folder(), size: node_size(&node) as i64, mtime: node.modification_time, + // The folder the node will go back to, which is also what tells + // a restore whether that folder is itself in the trash. + parent_uid: node.parent_uid.as_ref().map(|uid| uid.to_string()), })); // Cumulative, so the table is always a prefix of the real trash // rather than a mix of this refresh and the last one. @@ -4471,15 +4474,48 @@ impl Core { .collect() } - /// Restore trashed nodes to the folders they were trashed from. The parents - /// are read *before* the restore — a restored node reappears in a listing the - /// daemon may already have cached, so each destination folder is invalidated - /// and re-enumerated on next access. + /// Restore trashed nodes to the folders they were trashed from, together + /// with the rest of the trashed tree they belong to. + /// + /// The trash is a flat list, but a user deletes a shape: a folder, and often + /// items inside it that were trashed separately beforehand. Restoring one of + /// those in isolation is not what anyone means by it — a folder would come + /// back without the contents that were deleted with it, and a file would come + /// back inside a folder that is still in the trash, which is to say nowhere + /// the user can see. So a restore takes the whole connected piece: + /// every trashed descendant of what was asked for, and every trashed ancestor + /// above it. See [`expand_restore`] for the exact rule. + /// + /// The waves are restored shallowest first, because a node cannot be put back + /// under a parent that is still trashed. + /// + /// The destination parents are read *before* the restore — a restored node + /// reappears in a listing the daemon may already have cached, so each one is + /// invalidated and re-enumerated on next access. fn restore(&self, uids: &[String]) -> CoreResult { - let parsed = Self::parse_uids(uids)?; + let requested = Self::parse_uids(uids)?; + // A listing we could not read is not a reason to refuse the restore: it + // only costs the expansion, and the requested nodes still go back. + let relations = self.db.trash_parents().unwrap_or_default(); + let waves = expand_restore(uids, &relations); + let waves: Vec> = if waves.is_empty() { + vec![requested.clone()] + } else { + waves + .iter() + .map(|wave| { + wave.iter() + .filter_map(|uid| parse_uid(uid)) + .collect::>() + }) + .filter(|wave: &Vec| !wave.is_empty()) + .collect() + }; + + let all: Vec = waves.iter().flatten().cloned().collect(); let parents: Vec = self .rt - .block_on(self.client.enumerate_nodes(&parsed)) + .block_on(self.client.enumerate_nodes(&all)) .map_err(|e| CoreError::from_api(&e, "enumerate nodes"))? .into_iter() .filter_map(|n| n.parent_uid) @@ -4490,27 +4526,29 @@ impl Core { // half way through a large restore leaves local state describing what the // server actually did. Only a batch that restored nothing is an error the // caller can act on. - let mut restored: Vec = Vec::with_capacity(parsed.len()); + let mut restored: Vec = Vec::with_capacity(all.len()); let mut first_error: Option = None; - self.rt - .block_on(async { - let mut outcomes = std::pin::pin!(self.client.restore_nodes_streaming(&parsed)); - while let Some(item) = outcomes.next().await { - let (uid, outcome) = item?; - match outcome { - Ok(()) => { - self.hidden.lock().remove(&uid); - restored.push(uid); - } - Err(e) => { - warn!(%uid, error = %e, "restore failed for a node"); - first_error.get_or_insert(e); + for wave in &waves { + self.rt + .block_on(async { + let mut outcomes = std::pin::pin!(self.client.restore_nodes_streaming(wave)); + while let Some(item) = outcomes.next().await { + let (uid, outcome) = item?; + match outcome { + Ok(()) => { + self.hidden.lock().remove(&uid); + restored.push(uid); + } + Err(e) => { + warn!(%uid, error = %e, "restore failed for a node"); + first_error.get_or_insert(e); + } } } - } - Ok::<(), ProtonError>(()) - }) - .map_err(|e| CoreError::from_api(&e, "restore"))?; + Ok::<(), ProtonError>(()) + }) + .map_err(|e| CoreError::from_api(&e, "restore"))?; + } if restored.is_empty() && let Some(error) = first_error { @@ -5285,6 +5323,96 @@ fn node_size(node: &Node) -> u64 { } } +/// Expand a restore request across the trashed tree, and order it so a parent is +/// always restored before its children. +/// +/// `relations` is the persisted trash listing as `(uid, parent_uid)` pairs; a +/// parent that is not itself in the listing is a live folder, which ends the +/// walk upward. The rule is: +/// +/// - every trashed descendant of a requested node joins the restore, so a folder +/// comes back with the items that were trashed inside it; +/// - every trashed ancestor of a requested node joins it too, so a file never +/// lands in a folder that is still in the trash; +/// - a requested uid the listing does not know about is restored on its own, +/// because a stale or half-materialised listing must not silently drop it. +/// +/// The result is one wave per depth, shallowest first. Waves are disjoint and +/// together contain each uid exactly once. +fn expand_restore( + requested: &[String], + relations: &[(String, Option)], +) -> Vec> { + let parents: HashMap<&str, Option<&str>> = relations + .iter() + .map(|(uid, parent)| (uid.as_str(), parent.as_deref())) + .collect(); + let mut children: HashMap<&str, Vec<&str>> = HashMap::new(); + for (uid, parent) in relations { + // Only a trashed parent: a live one keeps its child as a root here. + if let Some(parent) = parent.as_deref() + && parents.contains_key(parent) + { + children.entry(parent).or_default().push(uid.as_str()); + } + } + + let mut selected: HashSet<&str> = HashSet::new(); + for uid in requested { + let uid = uid.as_str(); + if !parents.contains_key(uid) { + selected.insert(uid); + continue; + } + // Upward: every trashed ancestor, so the destination folder exists again. + let mut cursor = Some(uid); + while let Some(current) = cursor { + // A parent outside the listing is live: the walk stops under it. + if !parents.contains_key(current) || !selected.insert(current) { + break; + } + cursor = parents.get(current).copied().flatten(); + } + // Downward: the whole trashed subtree, so the folder comes back full. + let mut stack = vec![uid]; + while let Some(current) = stack.pop() { + for child in children.get(current).into_iter().flatten() { + if selected.insert(child) { + stack.push(child); + } + } + } + } + + // Depth within the *trash*, not within the drive: a node whose parent is live + // (or unknown) is at depth 0 and can be restored straight away. + let mut waves: Vec> = Vec::new(); + for uid in &selected { + let mut depth = 0usize; + let mut cursor = parents.get(uid).copied().flatten(); + // Only trashed ancestors count, and the walk is bounded by the listing's + // size: server data that somehow describes a cycle must not spin here. + while let Some(current) = cursor + && parents.contains_key(current) + && depth < parents.len() + { + depth += 1; + cursor = parents.get(current).copied().flatten(); + } + if waves.len() <= depth { + waves.resize(depth + 1, Vec::new()); + } + waves[depth].push((*uid).to_string()); + } + // Stable output, so a restore of the same trash is the same sequence of calls + // every time — the batches are what shows up in the log and in the tests. + for wave in &mut waves { + wave.sort(); + } + waves.retain(|wave| !wave.is_empty()); + waves +} + /// The server revision id of a node's active revision, if it is a file that has /// one. The stable identity the drain conflict-checks against (see [`Baseline`]). fn node_revision_id(node: &Node) -> Option { @@ -5985,9 +6113,9 @@ mod tests { use super::{ Access, AccessFlags, Errno, HashMap, Intervals, PendingRevision, RootListingSnapshot, SELF_CHANGE_TTL_MS, ShareId, SharedWithMeItem, StateRegistry, VirtualRootPlan, - accepted_share_provenance, conflict_name, copy_pending_for_truncate, fuse_name, - is_stale_mount, node_visible, note_self_change, parse_node_uid, prepare_shared_roots, - preserve_on_access_denied, publish_virtual_root_in_listing, + accepted_share_provenance, conflict_name, copy_pending_for_truncate, expand_restore, + fuse_name, is_stale_mount, node_visible, note_self_change, parse_node_uid, + prepare_shared_roots, preserve_on_access_denied, publish_virtual_root_in_listing, reconcile_virtual_root_in_listing, release_can_discard_unlinked, release_must_retain_queued_trash, release_unlinked_entry, rename_needs_queue, require_node_parent_access, require_rename_access, resolve_anywhere_with, @@ -5995,6 +6123,100 @@ mod tests { }; use super::{Db, WriteAuthority}; + /// `(uid, parent)` pairs in the shape [`Db::trash_parents`] returns. + fn relations(pairs: &[(&str, Option<&str>)]) -> Vec<(String, Option)> { + pairs + .iter() + .map(|(uid, parent)| (uid.to_string(), parent.map(str::to_string))) + .collect() + } + + fn uids(values: &[&str]) -> Vec { + values.iter().map(|v| v.to_string()).collect() + } + + /// Restoring a folder brings back what was trashed inside it, and the folder + /// goes first: the server has nowhere to put a child whose parent is trashed. + #[test] + fn restoring_a_folder_restores_its_trashed_contents_parents_first() { + let trash = relations(&[ + ("dir", Some("live")), + ("file", Some("dir")), + ("deep", Some("sub")), + ("sub", Some("dir")), + ]); + assert_eq!( + expand_restore(&uids(&["dir"]), &trash), + vec![ + vec!["dir".to_string()], + vec!["file".to_string(), "sub".to_string()], + vec!["deep".to_string()], + ] + ); + } + + /// Restoring a file inside a trashed folder restores the folder too — + /// otherwise the file comes back somewhere the user cannot reach. + #[test] + fn restoring_content_restores_the_trashed_parents_above_it() { + let trash = relations(&[ + ("dir", Some("live")), + ("sub", Some("dir")), + ("file", Some("sub")), + ("other", Some("dir")), + ]); + // `other` is a sibling of `sub`, not an ancestor of `file`: it stays put. + assert_eq!( + expand_restore(&uids(&["file"]), &trash), + vec![ + vec!["dir".to_string()], + vec!["sub".to_string()], + vec!["file".to_string()], + ] + ); + } + + /// A node whose parent is not in the trash is already at the top: one wave, + /// no ancestors invented. + #[test] + fn a_node_under_a_live_parent_restores_on_its_own() { + let trash = relations(&[("file", Some("live")), ("elsewhere", None)]); + assert_eq!( + expand_restore(&uids(&["file"]), &trash), + vec![vec!["file".to_string()]] + ); + } + + /// The listing is materialised in chunks and can be stale, so a uid it does + /// not know about must still be restored rather than dropped. + #[test] + fn an_unlisted_uid_is_still_restored() { + let trash = relations(&[("dir", None)]); + assert_eq!( + expand_restore(&uids(&["missing"]), &trash), + vec![vec!["missing".to_string()]] + ); + } + + /// Asking for both a folder and something inside it restores each node once. + #[test] + fn overlapping_requests_do_not_duplicate_nodes() { + let trash = relations(&[("dir", None), ("file", Some("dir"))]); + assert_eq!( + expand_restore(&uids(&["dir", "file"]), &trash), + vec![vec!["dir".to_string()], vec!["file".to_string()]] + ); + } + + /// A parent chain that loops back on itself must terminate. + #[test] + fn a_cyclic_listing_terminates() { + let trash = relations(&[("a", Some("b")), ("b", Some("a"))]); + let waves = expand_restore(&uids(&["a"]), &trash); + let total: usize = waves.iter().map(Vec::len).sum(); + assert_eq!(total, 2); + } + /// The collapse [`Core::require_uid_writable`] performs, without a `Core`: /// every non-writable authority is one `EACCES` on the syscall path. fn require_uid_access(db: &Db, uid: &NodeUid, live: &[Access]) -> Result<(), Errno> { diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 7d7aa8e..963ce13 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -11,7 +11,25 @@ scratch (user data in `staging/` and `recovery/` is never touched by this). ## [Unreleased] -Nothing yet. +Schema: **29** (`trash.parent_uid`); SDK bumped to `proton-sdk` / `proton-drive-rs` **0.6.5**. + +### Changed +- **Restore puts back the whole tree.** The trash is a flat list, but you delete a shape: a + folder, and often things you had already deleted inside it. Restoring a folder now also + restores the items that were trashed inside it, and restoring one of those items first + restores the trashed folders above it — otherwise the file came back into a folder that is + still in the trash, which is to say nowhere you can see it. Parents are restored before their + children, and each node is still restored individually, so one node the server refuses does + not cancel the rest. + +### Fixed +- **A permanently rejected block upload is no longer retried (SDK 0.6.5).** The SDK now knows + which API errors are worth replaying: 4xx is permanent, except 404, 408 and 429. +- **Long downloads no longer fail on expired block URLs (SDK 0.6.5).** A download whose block + URLs lapse mid-transfer re-mints them once instead of failing, on the whole-file path and on + public links, as reads already did. +- **A long rate-limit answer can no longer look like a hang (SDK 0.6.5).** A server-supplied + `Retry-After` is now capped at 120 s. ## [1.10.2] — 2026-09-16 diff --git a/docs/DEVELOPMENT.md b/docs/DEVELOPMENT.md index 97af1dc..4390a99 100644 --- a/docs/DEVELOPMENT.md +++ b/docs/DEVELOPMENT.md @@ -158,6 +158,8 @@ Needed a small additive SDK change (`ProtonDriveClient::with_entity_repository`, SDK 0.5.0 made `trash_nodes` / `restore_nodes` / `delete_nodes` report one outcome per node instead of failing the whole call (mirroring upstream's streamed `NodeActionResult`). `pdfs_core::batch::into_unit` collapses the single-node calls back to a `Result`; `batch::split` handles a collected batch. +A restore is expanded before it is sent (`expand_restore`, `pdfs-fuse/src/lib.rs`): the persisted trash listing carries each row's `parent_uid` (schema 29), so a restore takes the connected piece of the trashed tree — every trashed descendant of what was asked for, and every trashed ancestor above it — and sends it shallowest wave first, because the server cannot put a node back under a parent that is still trashed. A uid the listing does not know about (it is materialised in chunks, and can be stale) is restored on its own rather than dropped. + The trash view's restore and permanent-delete use the SDK's **streaming** variants (`restore_nodes_streaming` / `delete_nodes_streaming`) and apply local state per node as each batch lands rather than after the last one. For a permanent delete — which is irreversible — that means a daemon interrupted mid-batch has forgotten exactly the nodes the server destroyed, no more and no fewer. Both report per-node failures and count only what actually succeeded; only a batch where nothing succeeded is an error.