mirror of
https://github.com/narrrl/proton-drive-linux.git
synced 2026-11-04 00:47:00 +01:00
fixed missing trash logging
This commit is contained in:
1 parent
e8f11f7c42
commit
f9250211b8
3 files changed
+52
-14
No files matched your search
@@ -364,6 +364,36 @@ fn children_if_listed_gated_on_flag() {
|
|||||||
assert_eq!(kids.len(), 2);
|
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]
|
#[test]
|
||||||
fn children_if_listed_excludes_trashed() {
|
fn children_if_listed_excludes_trashed() {
|
||||||
let db = Db::open_in_memory().unwrap();
|
let db = Db::open_in_memory().unwrap();
|
||||||
|
|||||||
+16
-11
@@ -2308,16 +2308,8 @@ impl Core {
|
|||||||
/// the next `ListDir`/`readdir` re-enumerates it from the server. Backs
|
/// the next `ListDir`/`readdir` re-enumerates it from the server. Backs
|
||||||
/// [`CtlRequest::Refresh`] with a [`RefreshScope::Dir`] scope.
|
/// [`CtlRequest::Refresh`] with a [`RefreshScope::Dir`] scope.
|
||||||
fn refresh_dir(&self, rel: &Path) -> CoreResult<()> {
|
fn refresh_dir(&self, rel: &Path) -> CoreResult<()> {
|
||||||
let (ino, uid) = self.resolve(rel)?;
|
let (ino, _uid) = self.resolve(rel)?;
|
||||||
// Clear the DB flag directly rather than through
|
self.state.lock().invalidate_listing(ino);
|
||||||
// `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);
|
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -2990,6 +2982,8 @@ impl ProtonFs {
|
|||||||
.block_on(self.core.client.trash_nodes(std::slice::from_ref(&uid)))
|
.block_on(self.core.client.trash_nodes(std::slice::from_ref(&uid)))
|
||||||
{
|
{
|
||||||
error!(%uid, error = %e, "trash failed");
|
error!(%uid, error = %e, "trash failed");
|
||||||
|
self.core
|
||||||
|
.log_activity(ActivityKind::Trash, name, e.to_string(), false);
|
||||||
reply.error(Errno::EIO);
|
reply.error(Errno::EIO);
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
@@ -2998,6 +2992,11 @@ impl ProtonFs {
|
|||||||
self.core.cache.evict(&uid);
|
self.core.cache.evict(&uid);
|
||||||
self.core.evict_reader(&uid);
|
self.core.evict_reader(&uid);
|
||||||
self.core.invalidate_trash();
|
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();
|
reply.ok();
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -4040,9 +4039,15 @@ impl Filesystem for ProtonFs {
|
|||||||
}
|
}
|
||||||
// Forget the node so it re-interns under its new parent, and drop the
|
// Forget the node so it re-interns under its new parent, and drop the
|
||||||
// destination listing so it re-enumerates on next access.
|
// 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();
|
let mut st = self.core.state.lock();
|
||||||
st.forget(&uid);
|
st.forget(&uid);
|
||||||
st.children.remove(&newparent);
|
st.invalidate_listing(newparent);
|
||||||
reply.ok();
|
reply.ok();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -254,10 +254,13 @@ impl State {
|
|||||||
|
|
||||||
/// Drop a directory's cached child listing and mark it unlisted in the DB,
|
/// 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.
|
/// 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) {
|
pub(crate) fn invalidate_listing(&mut self, ino: u64) {
|
||||||
if self.children.remove(&ino).is_none() {
|
self.children.remove(&ino);
|
||||||
return;
|
|
||||||
}
|
|
||||||
if let Some(e) = self.entries.get(&ino) {
|
if let Some(e) = self.entries.get(&ino) {
|
||||||
let uid = e.uid.clone();
|
let uid = e.uid.clone();
|
||||||
if let Err(err) = self.db.set_listed(&uid, false) {
|
if let Err(err) = self.db.set_listed(&uid, false) {
|
||||||
|
|||||||
Reference in new issue
Block a user