From f9250211b8f585056a88792f79279773c57be5f9 Mon Sep 17 00:00:00 2001 From: Nils Pukropp Date: Sun, 19 Jul 2026 17:45:47 +0200 Subject: [PATCH] fixed missing trash logging --- crates/pdfs-core/src/db/tests.rs | 30 ++++++++++++++++++++++++++++++ crates/pdfs-fuse/src/lib.rs | 27 ++++++++++++++++----------- crates/pdfs-fuse/src/state.rs | 9 ++++++--- 3 files changed, 52 insertions(+), 14 deletions(-) diff --git a/crates/pdfs-core/src/db/tests.rs b/crates/pdfs-core/src/db/tests.rs index 94f1c15..fd76c5a 100644 --- a/crates/pdfs-core/src/db/tests.rs +++ b/crates/pdfs-core/src/db/tests.rs @@ -364,6 +364,36 @@ fn children_if_listed_gated_on_flag() { assert_eq!(kids.len(), 2); } +/// The shape of the `mv`-loses-the-file bug (bugs.md B1): a rename deletes the +/// moved node's row, so a destination left marked `listed` is rebuilt from the DB +/// without it — the file is gone from the source and absent from the destination, +/// with `rename(2)` having reported success. Clearing the flag is what forces the +/// re-enumeration that finds it again. +#[test] +fn a_deleted_child_leaves_a_listed_parent_serving_a_stale_listing() { + let db = Db::open_in_memory().unwrap(); + db.upsert_node(&folder("root", None, "My Files")).unwrap(); + db.upsert_node(&folder("dst", Some("root"), "dest")).unwrap(); + db.upsert_node(&file("f1", "root", "a.txt", 1)).unwrap(); + db.set_listed(&uid("dst"), true).unwrap(); + + // The rename moved a.txt into `dst` server-side, then forgot the node. + db.delete_node(&uid("f1")).unwrap(); + + // `dst` is still flagged listed, so the DB fast path answers — and the file + // it was just moved into is nowhere in the result. + assert!( + db.children_if_listed(&uid("dst")) + .unwrap() + .unwrap() + .is_empty() + ); + + // Clearing the flag is the only thing that sends the next read to the server. + db.set_listed(&uid("dst"), false).unwrap(); + assert!(db.children_if_listed(&uid("dst")).unwrap().is_none()); +} + #[test] fn children_if_listed_excludes_trashed() { let db = Db::open_in_memory().unwrap(); diff --git a/crates/pdfs-fuse/src/lib.rs b/crates/pdfs-fuse/src/lib.rs index c717ae3..cd53887 100644 --- a/crates/pdfs-fuse/src/lib.rs +++ b/crates/pdfs-fuse/src/lib.rs @@ -2308,16 +2308,8 @@ impl Core { /// the next `ListDir`/`readdir` re-enumerates it from the server. Backs /// [`CtlRequest::Refresh`] with a [`RefreshScope::Dir`] scope. fn refresh_dir(&self, rel: &Path) -> CoreResult<()> { - let (ino, uid) = self.resolve(rel)?; - // Clear the DB flag directly rather than through - // `State::invalidate_listing`, which no-ops when the folder has no - // in-memory listing to drop — exactly the case for a folder hydrated - // from the DB but not read yet this run, which is precisely the stale - // listing worth refreshing. - self.db - .set_listed(&uid, false) - .map_err(|e| CoreError::internal(format!("db: {e:?}")))?; - self.state.lock().children.remove(&ino); + let (ino, _uid) = self.resolve(rel)?; + self.state.lock().invalidate_listing(ino); Ok(()) } @@ -2990,6 +2982,8 @@ impl ProtonFs { .block_on(self.core.client.trash_nodes(std::slice::from_ref(&uid))) { error!(%uid, error = %e, "trash failed"); + self.core + .log_activity(ActivityKind::Trash, name, e.to_string(), false); reply.error(Errno::EIO); return; } @@ -2998,6 +2992,11 @@ impl ProtonFs { self.core.cache.evict(&uid); self.core.evict_reader(&uid); self.core.invalidate_trash(); + // Every other trash site records itself; this one did not, which made a + // file found in the trash impossible to attribute after the fact — the + // activity log was the only record and it showed nothing (bugs.md B2). + self.core + .log_activity(ActivityKind::Trash, name, "trashed from the mount", true); reply.ok(); } } @@ -4040,9 +4039,15 @@ impl Filesystem for ProtonFs { } // Forget the node so it re-interns under its new parent, and drop the // destination listing so it re-enumerates on next access. + // + // The drop has to go through `invalidate_listing`, not `children.remove`: + // `forget` deletes the node's DB row, so a destination still marked + // `listed` would be rebuilt from the DB by `ensure_children` — without the + // node we just moved into it. The file would be gone from the source and + // absent from the destination, with the rename having reported success. let mut st = self.core.state.lock(); st.forget(&uid); - st.children.remove(&newparent); + st.invalidate_listing(newparent); reply.ok(); } diff --git a/crates/pdfs-fuse/src/state.rs b/crates/pdfs-fuse/src/state.rs index 36d0c2c..9b26761 100644 --- a/crates/pdfs-fuse/src/state.rs +++ b/crates/pdfs-fuse/src/state.rs @@ -254,10 +254,13 @@ impl State { /// Drop a directory's cached child listing and mark it unlisted in the DB, /// so the next access re-enumerates instead of trusting a stale listing. + /// + /// The DB flag is cleared whether or not the listing was resident. A folder + /// trimmed from the hot cache but still `listed` in the DB is exactly the + /// case that needs invalidating — returning early there would leave + /// `ensure_children` free to rebuild the stale listing from disk. pub(crate) fn invalidate_listing(&mut self, ino: u64) { - if self.children.remove(&ino).is_none() { - return; - } + self.children.remove(&ino); if let Some(e) = self.entries.get(&ino) { let uid = e.uid.clone(); if let Err(err) = self.db.set_listed(&uid, false) {