fix(fuse,core): keep a rename made while its create is on the wire

A rename of a node whose create is still queued rewrites the create's
target, but the drain had already copied the op. When the create
landed, the row was deleted unread and the rename was lost (B136).

finish_create now learns where the create was sent and where it landed,
and queues a rename of the real uid when the row has moved on since.
adopt_real_uid keeps the tree's name and folder while that rename is
queued. A landed folder's children also stop naming its placeholder as
their parent in memory, which put the placeholder back on their rows.

crates/pdfs-core/src/db/mod.rs

@@ -56,8 +56,9 @@ pub use local::LocalFileHit;
 pub use maintenance::{DbStats, VacuumOutcome};
 pub use nodes::{PublishedSharedRoot, SearchHit, StoredNode};
 pub use ops::{
-    AttachedBlob, FAILING_ATTEMPTS, LOCAL_VOLUME, OP_CREATE, OP_MKDIR, OP_RENAME, OP_REVISION,
-    OP_TRASH, PARK_EXPIRY_MS, PARK_UNTIL, PendingCounts, PendingOp, RenameMeta, op_supersedes,
+    AttachedBlob, CreateLanding, FAILING_ATTEMPTS, LOCAL_VOLUME, OP_CREATE, OP_MKDIR, OP_RENAME,
+    OP_REVISION, OP_TRASH, PARK_EXPIRY_MS, PARK_UNTIL, PendingCounts, PendingOp, RenameMeta,
+    op_supersedes,
 };
 pub use photos::{StoredPhoto, StoredPlace, THUMB_HAVE, THUMB_NONE, THUMB_UNKNOWN, TimelineRow};
 pub use pins::PinRow;

crates/pdfs-core/src/db/ops.rs

@@ -228,6 +228,28 @@ pub struct PendingCounts {
 /// is retrying at the slowest rate it ever will and has been for minutes.
 pub const FAILING_ATTEMPTS: i64 = 6;
 
+/// A queued create that has landed, for [`Db::finish_create`].
+#[derive(Debug, Clone, Copy)]
+pub struct CreateLanding<'a> {
+    /// The placeholder the node was known by.
+    pub local: &'a str,
+    /// The uid Drive gave it.
+    pub real: &'a str,
+    /// The parent and name the op held when the drain took it.
+    pub sent: (&'a str, &'a str),
+    /// The parent and name the node has on Drive. They differ from `sent`
+    /// when the name was taken or the folder was gone.
+    pub landed: (&'a str, &'a str),
+}
+
+struct CreateRow {
+    blob: Option<String>,
+    meta: Option<String>,
+    parent_uid: Option<String>,
+    name: Option<String>,
+    created_at: i64,
+}
+
 /// The outcome of folding freshly written bytes into a queued create.
 #[derive(Debug, Clone)]
 pub struct AttachedBlob {
@@ -401,27 +423,44 @@ impl Db {
     /// becomes a revision of `real` instead, with its sidecar passed through
     /// `rewrite` so it names the real node. Returns that blob and sidecar, or
     /// `None` when the create is simply done.
+    ///
+    /// A rename while the create is on the wire rewrites the row's target
+    /// ([`Db::rewrite_op_target`]), which the create no longer reads. When the
+    /// row names another parent or name than `landing.sent`, a rename of `real`
+    /// to that target is queued in the same transaction, or the move would be
+    /// lost with the row.
     pub fn finish_create(
         &self,
         id: i64,
         uploaded: Option<&str>,
-        local: &str,
-        real: &str,
+        landing: &CreateLanding<'_>,
         rewrite: impl FnOnce(&str) -> Option<String>,
     ) -> Result<Option<(String, String)>> {
+        let (local, real) = (landing.local, landing.real);
         let mut conn = self.conn.lock();
         let tx = conn.transaction()?;
-        let row: Option<(Option<String>, Option<String>)> = tx
+        let row: Option<CreateRow> = tx
             .query_row(
-                "SELECT blob_path, meta_json FROM pending_op WHERE id = ?1",
+                "SELECT blob_path, meta_json, parent_uid, name, created_at
+                 FROM pending_op WHERE id = ?1",
                 params![id],
-                |r| Ok((r.get(0)?, r.get(1)?)),
+                |r| {
+                    Ok(CreateRow {
+                        blob: r.get(0)?,
+                        meta: r.get(1)?,
+                        parent_uid: r.get(2)?,
+                        name: r.get(3)?,
+                        created_at: r.get(4)?,
+                    })
+                },
             )
             .optional()?;
-        let newer = match row {
-            Some((Some(blob), Some(meta))) if uploaded != Some(blob.as_str()) => {
-                rewrite(&meta).map(|meta| (blob, meta))
-            }
+        let newer = match &row {
+            Some(CreateRow {
+                blob: Some(blob),
+                meta: Some(meta),
+                ..
+            }) if uploaded != Some(blob.as_str()) => rewrite(meta).map(|meta| (blob.clone(), meta)),
             _ => None,
         };
         match &newer {
@@ -438,6 +477,28 @@ impl Db {
                 tx.execute("DELETE FROM pending_op WHERE id = ?1", params![id])?;
             }
         }
+        if let Some(CreateRow {
+            parent_uid: Some(parent),
+            name: Some(name),
+            created_at,
+            ..
+        }) = &row
+            && (parent.as_str(), name.as_str()) != landing.sent
+        {
+            let meta = serde_json::to_string(&RenameMeta {
+                original_parent_uid: landing.landed.0.to_string(),
+                original_name: Some(landing.landed.1.to_string()),
+            })?;
+            tx.execute(
+                "DELETE FROM pending_op WHERE uid = ?1 AND kind = ?2",
+                params![real, OP_RENAME],
+            )?;
+            tx.execute(
+                "INSERT INTO pending_op (kind, uid, parent_uid, name, meta_json, created_at)
+                 VALUES (?1, ?2, ?3, ?4, ?5, ?6)",
+                params![OP_RENAME, real, parent, name, meta, created_at],
+            )?;
+        }
         tx.execute(
             "UPDATE pending_op SET parent_uid = ?2 WHERE parent_uid = ?1",
             params![local, real],

crates/pdfs-core/src/db/tests.rs

@@ -2853,6 +2853,7 @@ fn a_write_attached_while_its_create_uploads_becomes_a_revision() {
     let db = Db::open_in_memory().unwrap();
     let local = "local~abc";
     let real = uid("real").to_string();
+    let parent = uid("parent").to_string();
     let create = |blob: Option<&str>| PendingOp {
         id: 0,
         kind: OP_CREATE.to_string(),
@@ -2874,9 +2875,12 @@ fn a_write_attached_while_its_create_uploads_becomes_a_revision() {
         .expect("create is still queued");
 
     let newer = db
-        .finish_create(id, None, local, &real, |json| {
-            Some(json.replace("local~abc", "vol~real"))
-        })
+        .finish_create(
+            id,
+            None,
+            &landing(local, &real, &parent, "new.txt"),
+            |json| Some(json.replace("local~abc", "vol~real")),
+        )
         .unwrap();
 
     assert_eq!(
@@ -2899,6 +2903,7 @@ fn a_create_whose_blob_landed_is_retired() {
     let db = Db::open_in_memory().unwrap();
     let local = "local~abc";
     let real = uid("real").to_string();
+    let parent = uid("parent").to_string();
     let (id, _) = db
         .enqueue_op(&PendingOp {
             id: 0,
@@ -2930,9 +2935,12 @@ fn a_create_whose_blob_landed_is_retired() {
     .unwrap();
 
     let newer = db
-        .finish_create(id, Some("/staging/sent"), local, &real, |_| {
-            panic!("an unchanged create has no sidecar to rewrite")
-        })
+        .finish_create(
+            id,
+            Some("/staging/sent"),
+            &landing(local, &real, &parent, "new.txt"),
+            |_| panic!("an unchanged create has no sidecar to rewrite"),
+        )
         .unwrap();
 
     assert_eq!(newer, None);
@@ -2941,6 +2949,64 @@ fn a_create_whose_blob_landed_is_retired() {
     assert_eq!(ops[0].parent_uid.as_deref(), Some(real.as_str()));
 }
 
+/// The parent and name a create was sent with, landing where it was sent.
+fn landing<'a>(local: &'a str, real: &'a str, parent: &'a str, name: &'a str) -> CreateLanding<'a> {
+    CreateLanding {
+        local,
+        real,
+        sent: (parent, name),
+        landed: (parent, name),
+    }
+}
+
+#[test]
+fn a_rename_made_while_its_create_uploads_is_queued_for_the_landed_node() {
+    let db = Db::open_in_memory().unwrap();
+    let local = "local~abc";
+    let real = uid("real").to_string();
+    let parent = uid("parent").to_string();
+    let (id, _) = db
+        .enqueue_op(&PendingOp {
+            id: 0,
+            kind: OP_CREATE.to_string(),
+            uid: local.to_string(),
+            parent_uid: Some(uid("parent").to_string()),
+            name: Some("new.txt".to_string()),
+            blob_path: Some("/staging/sent".to_string()),
+            meta_json: Some("{}".to_string()),
+            created_at: 1,
+            attempts: 0,
+            last_error: None,
+            next_attempt_at: 0,
+        })
+        .unwrap();
+    // The drain took the create; the user moves the file into a folder that
+    // is itself still queued.
+    db.rewrite_op_target(local, "local~dir", "moved.txt")
+        .unwrap();
+
+    let mut landed = landing(local, &real, &parent, "new.txt");
+    landed.landed.1 = "new (conflict).txt";
+    db.finish_create(id, Some("/staging/sent"), &landed, |_| None)
+        .unwrap();
+
+    let ops = db.pending_ops().unwrap();
+    assert_eq!(ops.len(), 1, "the move outlives the create");
+    assert_eq!(ops[0].kind, OP_RENAME);
+    assert_eq!(ops[0].uid, real);
+    assert_eq!(ops[0].parent_uid.as_deref(), Some("local~dir"));
+    assert_eq!(ops[0].name.as_deref(), Some("moved.txt"));
+    let meta: RenameMeta = serde_json::from_str(ops[0].meta_json.as_deref().unwrap()).unwrap();
+    assert_eq!(meta.original_parent_uid, uid("parent").to_string());
+    assert_eq!(meta.original_name.as_deref(), Some("new (conflict).txt"));
+
+    // The folder lands next and the move follows it.
+    db.remap_local_uid("local~dir", &uid("dir").to_string())
+        .unwrap();
+    let ops = db.pending_ops().unwrap();
+    assert_eq!(ops[0].parent_uid, Some(uid("dir").to_string()));
+}
+
 #[test]
 fn draining_a_folder_repoints_its_queued_children() {
     let db = Db::open_in_memory().unwrap();
@@ -3002,8 +3068,12 @@ fn a_landed_folder_keeps_a_row_until_the_server_copy_replaces_it() {
     db.finish_create(
         id,
         None,
-        &local.to_string(),
-        &uid("real").to_string(),
+        &landing(
+            &local.to_string(),
+            &uid("real").to_string(),
+            &uid("root").to_string(),
+            "New folder",
+        ),
         |_| None,
     )
     .unwrap();

crates/pdfs-fuse/src/drain.rs

@@ -33,7 +33,8 @@ use pdfs_core::batch;
 use pdfs_core::cache::{Baseline, StagedWrite};
 use pdfs_core::control::{ActivityKind, TransferDirection};
 use pdfs_core::db::{
-    OP_CREATE, OP_MKDIR, OP_RENAME, OP_REVISION, OP_TRASH, PARK_EXPIRY_MS, PendingOp, RenameMeta,
+    CreateLanding, OP_CREATE, OP_MKDIR, OP_RENAME, OP_REVISION, OP_TRASH, PARK_EXPIRY_MS,
+    PendingOp, RenameMeta,
 };
 use proton_drive_rs::proton_sdk::ids::NodeUid;
 use proton_drive_rs::{Node, NodeKind};
@@ -851,6 +852,7 @@ impl Core {
         // create was queued because nobody heard back, so adopt it. And a node
         // whose trash is still queued is about to let go of it.
         let mut name = wanted.clone();
+        let mut home = parent.clone();
         let mut adopted = false;
         let mut real = self.create_drained_node(op, &parent, &name);
         if real.as_ref().is_err_and(|e| is_already_exists(e.as_ref()))
@@ -893,6 +895,7 @@ impl Core {
                 name = conflict_name(&wanted, now_secs());
                 real = self.create_drained_node(op, &root, &name);
             }
+            home = root;
             if real.is_ok() {
                 self.log_activity(
                     ActivityKind::Upload,
@@ -910,7 +913,13 @@ impl Core {
         // An adopted node was made empty, so a blob this op carries has not been
         // uploaded: it goes on as a revision of the node instead.
         let uploaded = (!adopted).then_some(op.blob_path.as_deref()).flatten();
-        let newer = self.retire_create(op, uploaded, &local, &real)?;
+        let landing = CreateLanding {
+            local: &local.to_string(),
+            real: &real.to_string(),
+            sent: (parent_str, &wanted),
+            landed: (&home.to_string(), &name),
+        };
+        let newer = self.retire_create(op, uploaded, &landing, &local, &real)?;
         // The uploaded blob is the new file's content, so it becomes the cached
         // content, as for a revision. A partial write fills its gaps from it;
         // without it the next write open was refused (`docs/BUGS.md` B123).
@@ -1056,19 +1065,17 @@ impl Core {
         &self,
         op: &PendingOp,
         uploaded: Option<&str>,
+        landing: &CreateLanding<'_>,
         local: &NodeUid,
         real: &NodeUid,
     ) -> Result<bool, Box<dyn std::error::Error>> {
-        let real_str = real.to_string();
         let newer = {
             let mut pending = self.pending.lock();
-            let newer =
-                self.db
-                    .finish_create(op.id, uploaded, &local.to_string(), &real_str, |json| {
-                        let mut meta: StagedWrite = serde_json::from_str(json).ok()?;
-                        meta.uid = real_str.clone();
-                        serde_json::to_string(&meta).ok()
-                    })?;
+            let newer = self.db.finish_create(op.id, uploaded, landing, |json| {
+                let mut meta: StagedWrite = serde_json::from_str(json).ok()?;
+                meta.uid = landing.real.to_string();
+                serde_json::to_string(&meta).ok()
+            })?;
             pending.remove(local);
             if let Some((blob, json)) = &newer {
                 let meta: StagedWrite = serde_json::from_str(json)?;
@@ -1091,6 +1098,15 @@ impl Core {
                 if let Some(e) = st.entries.get_mut(&ino) {
                     e.uid = real.clone();
                 }
+                // Its children still name the placeholder as their parent, and
+                // the next write of one would put it back on the row.
+                for kid in st.children.get(&ino).cloned().unwrap_or_default() {
+                    if let Some(e) = st.entries.get_mut(&kid)
+                        && e.node.parent_uid.as_ref() == Some(local)
+                    {
+                        e.node.parent_uid = Some(real.clone());
+                    }
+                }
             }
             for aw in st.active_writes.values_mut() {
                 if aw.uid == *local {
@@ -1156,9 +1172,13 @@ impl Core {
         adopt: impl FnOnce(&Node),
     ) -> Result<(), Box<dyn std::error::Error>> {
         // Drive answers a create before it lists the new node.
-        let node = self
+        let mut node = self
             .read_back(real)
             .map_err(|e| self.errno_error(e, "fetch node"))?;
+        // A rename made while the create was on the wire is queued against the
+        // real uid (`Db::finish_create`). Until it lands, the tree keeps the
+        // name and folder the user gave it.
+        let renaming = self.db.has_pending_op(&real.to_string(), OP_RENAME)?;
         // Before open handles are rebased onto the node, so one released in
         // between finds what `adopt` keeps (`docs/BUGS.md` B123).
         adopt(&node);
@@ -1174,6 +1194,10 @@ impl Core {
             let Some(&ino) = st.by_uid.get(real) else {
                 return;
             };
+            if renaming && let Some(e) = st.entries.get(&ino) {
+                node.name = e.node.name.clone();
+                node.parent_uid = e.node.parent_uid.clone();
+            }
             let mut landed = node.clone();
             st.keep_open_write_size(ino, &mut landed);
             if let Some(e) = st.entries.get_mut(&ino) {