fix(fuse): a file renamed while its upload is read back keeps its new name (B172)

crates/pdfs-fuse/src/drain.rs

@@ -2294,6 +2294,14 @@ impl Core {
         blob: &Path,
         adopt: impl FnOnce(i64, u64),
     ) -> Option<Node> {
+        // Asked before the read-back: the op went before it, so a rename queued
+        // behind the upload can land while Drive answers, and the answer may
+        // still have the old name (`docs/BUGS.md` B172).
+        let placed = self.placement(uid);
+        let renaming = self
+            .db
+            .has_pending_op(&uid.to_string(), OP_RENAME)
+            .unwrap_or(false);
         let fetched = self.fetch_node_remote(uid);
         if fetched.is_ok() {
             self.sealed_unread.lock().remove(uid);
@@ -2376,37 +2384,42 @@ impl Core {
         self.release_pending(uid, blob);
         #[cfg(test)]
         self.drive.landing(uid);
-        self.place_landed(uid, &node);
+        self.place_landed(uid, &node, placed, renaming);
         self.landed.lock().remove(uid);
         Some(node)
     }
 
     /// Put a node whose upload just landed into the tree, unless a newer write
-    /// is queued for it.
-    fn place_landed(&self, uid: &NodeUid, node: &Node) {
+    /// is queued for it. `before` is where the tree had the node and
+    /// `renaming` whether a rename of it was queued, both from before the
+    /// read-back.
+    fn place_landed(
+        &self,
+        uid: &NodeUid,
+        node: &Node,
+        before: Option<(String, Option<NodeUid>)>,
+        renaming: bool,
+    ) {
         // Ordered so that a write queued *during* the fetch above is still
         // caught: it took its baseline from the node's optimistic stamp, and
         // this overwrites it with the revision the server actually holds.
         if self.rebaseline_pending(uid, node) {
             return;
         }
-        // A rename queued behind the upload has not reached Drive yet, and
-        // until it lands the tree keeps the name and folder the user gave the
-        // file, as `adopt_real_uid` does. Taking Drive's put the file back
-        // under its old name. A rename queued after the check below has
-        // already moved the entry when the tree is updated, so a name that
-        // changed in between is the user's too (docs/BUGS.md B146).
-        let placed = |st: &crate::state::State| {
-            let entry = st.by_uid.get(uid).and_then(|ino| st.entries.get(ino))?;
-            Some((entry.node.name.clone(), entry.node.parent_uid.clone()))
-        };
-        let before = placed(&self.state());
-        let renaming = self
-            .db
-            .has_pending_op(&uid.to_string(), OP_RENAME)
-            .unwrap_or(false);
+        // A rename queued behind the upload has not reached Drive yet, or
+        // landed after Drive read the node back, and the tree keeps the name
+        // and folder the user gave the file, as `adopt_real_uid` does. Taking
+        // Drive's put the file back under its old name. A rename queued since
+        // the read-back began has already moved the entry when the tree is
+        // updated, so a name that changed in between is the user's too
+        // (docs/BUGS.md B146, B172).
+        let renaming = renaming
+            || self
+                .db
+                .has_pending_op(&uid.to_string(), OP_RENAME)
+                .unwrap_or(false);
         self.for_each_state(|st| {
-            let moved = placed(st) != before;
+            let moved = placed_in(st, uid) != before;
             let Some(entry) = st.by_uid.get(uid).and_then(|ino| st.entries.get(ino)) else {
                 return;
             };
@@ -2420,6 +2433,11 @@ impl Core {
         });
     }
 
+    /// Where the tree has `uid`: its name and parent.
+    fn placement(&self, uid: &NodeUid) -> Option<(String, Option<NodeUid>)> {
+        placed_in(&self.state(), uid)
+    }
+
     /// Rebase every write handle open on `uid` onto `node`, a revision this
     /// daemon has just put there itself (`WriteHandle::rebase_onto`).
     fn rebase_open_writes(&self, uid: &NodeUid, node: &Node) {
@@ -2675,6 +2693,12 @@ fn keeps_landed_upload(pinned: bool, len: u64, budget: u64) -> bool {
     pinned || budget == 0 || len <= budget / LANDED_KEEP_SHARE
 }
 
+/// Where `st` has `uid`: its name and parent.
+fn placed_in(st: &crate::state::State, uid: &NodeUid) -> Option<(String, Option<NodeUid>)> {
+    let entry = st.by_uid.get(uid).and_then(|ino| st.entries.get(ino))?;
+    Some((entry.node.name.clone(), entry.node.parent_uid.clone()))
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;

crates/pdfs-fuse/src/sim/daemon.rs

@@ -463,6 +463,68 @@ mod tests {
         let _ = std::fs::remove_dir_all(&dir);
     }
 
+    #[test]
+    #[ignore = "mounts FUSE: run with `cargo test -p pdfs-fuse sim:: -- --ignored`"]
+    fn a_file_renamed_while_its_upload_is_read_back_keeps_its_new_name() {
+        // The rename landed while the drain read the uploaded file back. The
+        // answer still had the old name, and the drain found no rename queued,
+        // so the mount showed the file under its old name and the new one
+        // answered ENOENT (B172).
+        let drive = FakeDrive::new();
+        let dir = scratch("renamed-while-read-back");
+        let daemon = Daemon::start(&dir, drive.client(1, Faults::lan())).unwrap();
+        let mnt = daemon.mountpoint.clone();
+        let file = |bytes: &[u8]| Some(Entry::File(Arc::new(bytes.to_vec())));
+
+        std::fs::write(mnt.join("a.txt"), b"first").unwrap();
+        assert!(wait_until(Duration::from_secs(30), || {
+            daemon.pending().is_ok_and(|items| items.is_empty())
+                && drive.tree().get("a.txt").cloned() == file(b"first")
+        }));
+        let uid = drive.lookup("a.txt").unwrap();
+        let mut held = daemon.client.hold_answer_to_next_read(&uid);
+        std::fs::write(mnt.join("a.txt"), b"second").unwrap();
+        assert!(wait_until(Duration::from_secs(30), || held.reached()));
+        std::fs::rename(mnt.join("a.txt"), mnt.join("f.txt")).unwrap();
+        // Each read is answered in turn until the upload's op is retired; the
+        // answer to the read-back after it is held until the rename has landed.
+        let reading_back = loop {
+            if daemon
+                .pending()
+                .is_ok_and(|items| items.iter().all(|op| op.kind != "revision"))
+            {
+                break held;
+            }
+            let next = daemon.client.hold_answer_to_next_read(&uid);
+            drop(held);
+            assert!(wait_until(Duration::from_secs(30), || next.reached()));
+            held = next;
+        };
+        assert!(
+            wait_until(Duration::from_secs(30), || drive.lookup("f.txt").is_some()),
+            "{:?} {:?}",
+            daemon.pending(),
+            drive.tree()
+        );
+        drop(reading_back);
+        assert!(
+            wait_until(Duration::from_secs(30), || {
+                daemon.pending().is_ok_and(|items| items.is_empty())
+                    && drive.tree().get("f.txt").cloned() == file(b"second")
+            }),
+            "{:?} {:?}",
+            daemon.pending(),
+            drive.tree()
+        );
+        // Long enough for the drain to have put the read-back into the tree.
+        std::thread::sleep(Duration::from_millis(200));
+        assert_eq!(std::fs::read(mnt.join("f.txt")).unwrap(), b"second");
+        assert!(!mnt.join("a.txt").exists());
+
+        assert!(matches!(daemon.stop(), Ok(MountOutcome::Shutdown)));
+        let _ = std::fs::remove_dir_all(&dir);
+    }
+
     #[test]
     #[ignore = "mounts FUSE: run with `cargo test -p pdfs-fuse sim:: -- --ignored`"]
     fn a_file_moved_while_its_create_was_on_the_wire_keeps_its_new_name() {