fix(fuse,core): keep a mirrored folder safe when another computer syncs it

A mirrored folder assumed it was the only client writing to its folder on
Drive. With a second computer syncing the same folder, edits could be lost:

- The first sync took a same-size file at the same path for the same file.
  It now records the baseline only if the local file's SHA-1 matches Drive's
  digest, and otherwise keeps a conflict copy (B248).
- An upload replaced a revision another client had committed since the pass
  looked. The upload now checks Drive's active revision first, and an edit
  that read back with someone else's digest is left for the next pass to keep
  as a conflict copy (B249).
- A download recorded a revision committed while it ran as synced, so that
  edit never arrived. The baseline now records the revision the pass planned
  to download (B250).
- Conflict copies now carry the computer's hostname, so two computers do not
  make the same name on Drive (B251).

The baseline gains the revision id it was synced at (schema 44), and a new
revision counts as a remote change even at the same mtime and size.

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

@@ -11,7 +11,7 @@ use super::Db;
 use crate::Result;
 
 /// Current schema version. Bump on every forward migration added below.
-pub(super) const SCHEMA_VERSION: i64 = 43;
+pub(super) const SCHEMA_VERSION: i64 = 44;
 
 impl Db {
     pub(super) fn migrate(&self) -> Result<()> {
@@ -450,6 +450,23 @@ impl Db {
                 tx.execute_batch(MIGRATION_V43)?;
             }
         }
+        if current < 44 {
+            // Same guards as V26-V42.
+            let has_column: bool = tx.query_row(
+                "SELECT COUNT(*) FROM pragma_table_info('sync_entry') \
+                 WHERE name = 'remote_revision_id'",
+                [],
+                |row| row.get::<_, i64>(0),
+            )? > 0;
+            let has_entries: bool = tx.query_row(
+                "SELECT COUNT(*) FROM sqlite_master WHERE type = 'table' AND name = 'sync_entry'",
+                [],
+                |row| row.get::<_, i64>(0),
+            )? > 0;
+            if has_entries && !has_column {
+                tx.execute_batch(MIGRATION_V44)?;
+            }
+        }
         tx.execute(
             "INSERT INTO sync_state (key, value) VALUES ('schema_version', ?1)
              ON CONFLICT(key) DO UPDATE SET value = excluded.value",
@@ -1351,3 +1368,17 @@ ALTER TABLE pending_op ADD COLUMN sent_to TEXT;
 const MIGRATION_V43: &str = "
 CREATE INDEX IF NOT EXISTS pending_op_kind_name ON pending_op(kind, name);
 ";
+
+/// Schema v44: a synced file's baseline remembers the revision it was synced
+/// at.
+///
+/// A mirrored file edited here is uploaded as a new revision of whatever is
+/// current on Drive, and another computer syncing the same folder may have
+/// committed one since this pass looked. The revision id is what the upload
+/// checks first, so that edit becomes a conflict copy instead of being
+/// replaced (B249). `NULL` means "this row predates the column", and the check
+/// falls back to the modification time and size; each row gains a real value
+/// the next time its path is synced.
+const MIGRATION_V44: &str = "
+ALTER TABLE sync_entry ADD COLUMN remote_revision_id TEXT;
+";

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

@@ -45,6 +45,12 @@ pub struct StoredSyncEntry {
     pub local_mtime_ns: Option<i64>,
     pub remote_rev: Option<String>,
     pub remote_hash: Option<String>,
+    /// The id of the remote revision this row was synced at. A mirror upload
+    /// checks it before it supersedes what Drive holds, because another client
+    /// may have committed a revision since (`docs/BUGS.md` B249). `None` on a
+    /// row written before schema 44, or by a pass that did not learn it; the
+    /// check then falls back to `remote_rev` and `remote_hash`.
+    pub remote_revision_id: Option<String>,
 }
 
 impl Db {
@@ -188,7 +194,7 @@ impl Db {
         let conn = self.read();
         let mut stmt = conn.prepare(
             "SELECT rel_path, remote_uid, local_mtime, local_size, remote_rev, remote_hash,
-                    local_mtime_ns
+                    local_mtime_ns, remote_revision_id
              FROM sync_entry WHERE folder_id = ?1",
         )?;
         let rows = stmt.query_map(params![folder_id], |r| {
@@ -200,6 +206,7 @@ impl Db {
                 remote_rev: r.get(4)?,
                 remote_hash: r.get(5)?,
                 local_mtime_ns: r.get(6)?,
+                remote_revision_id: r.get(7)?,
             })
         })?;
         let mut map = HashMap::new();
@@ -238,15 +245,16 @@ impl Db {
         conn.execute(
             "INSERT INTO sync_entry
                (folder_id, rel_path, remote_uid, local_mtime, local_size, remote_rev,
-                remote_hash, local_mtime_ns)
-             VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8)
+                remote_hash, local_mtime_ns, remote_revision_id)
+             VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9)
              ON CONFLICT(folder_id, rel_path) DO UPDATE SET
-               remote_uid     = excluded.remote_uid,
-               local_mtime    = excluded.local_mtime,
-               local_size     = excluded.local_size,
-               remote_rev     = excluded.remote_rev,
-               remote_hash    = excluded.remote_hash,
-               local_mtime_ns = excluded.local_mtime_ns",
+               remote_uid         = excluded.remote_uid,
+               local_mtime        = excluded.local_mtime,
+               local_size         = excluded.local_size,
+               remote_rev         = excluded.remote_rev,
+               remote_hash        = excluded.remote_hash,
+               local_mtime_ns     = excluded.local_mtime_ns,
+               remote_revision_id = excluded.remote_revision_id",
             params![
                 folder_id,
                 e.rel_path,
@@ -256,6 +264,7 @@ impl Db {
                 e.remote_rev,
                 e.remote_hash,
                 e.local_mtime_ns,
+                e.remote_revision_id,
             ],
         )?;
         Ok(())
@@ -306,7 +315,7 @@ impl Db {
         {
             let mut select = tx.prepare(
                 "SELECT rel_path, remote_uid, local_mtime, local_size, remote_rev, remote_hash,
-                        local_mtime_ns
+                        local_mtime_ns, remote_revision_id
                    FROM sync_entry
                   WHERE folder_id = ?1
                     AND (rel_path = ?2 OR substr(rel_path, 1, length(?2) + 1) = ?2 || '/')",
@@ -321,6 +330,7 @@ impl Db {
                         remote_rev: r.get(4)?,
                         remote_hash: r.get(5)?,
                         local_mtime_ns: r.get(6)?,
+                        remote_revision_id: r.get(7)?,
                     })
                 })?
                 .collect::<rusqlite::Result<Vec<_>>>()?;
@@ -332,8 +342,8 @@ impl Db {
             let mut insert = tx.prepare(
                 "INSERT OR REPLACE INTO sync_entry
                    (folder_id, rel_path, remote_uid, local_mtime, local_size, remote_rev,
-                    remote_hash, local_mtime_ns)
-                 VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8)",
+                    remote_hash, local_mtime_ns, remote_revision_id)
+                 VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9)",
             )?;
             for row in rows {
                 let rel = format!("{to_rel}{}", &row.rel_path[from_rel.len()..]);
@@ -346,6 +356,7 @@ impl Db {
                     row.remote_rev,
                     row.remote_hash,
                     row.local_mtime_ns,
+                    row.remote_revision_id,
                 ])?;
             }
         }

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

@@ -1327,6 +1327,7 @@ fn a_rebuild_keeps_the_queue_baselines_and_pins_of_the_damaged_file() {
             local_size: 6,
             remote_rev: Some("7".into()),
             remote_hash: Some("8".into()),
+            remote_revision_id: None,
         },
     )
     .unwrap();
@@ -5343,6 +5344,7 @@ fn baseline_row(rel: &str) -> StoredSyncEntry {
         local_size: 7,
         remote_rev: Some("1700000000".into()),
         remote_hash: Some("7".into()),
+        remote_revision_id: None,
     }
 }
 
@@ -5437,6 +5439,7 @@ fn mirror_uid_lookup_covers_the_root_and_synced_descendants_only() {
             local_size: 2,
             remote_rev: Some("3".into()),
             remote_hash: Some("2".into()),
+            remote_revision_id: None,
             local_mtime_ns: None,
         },
     )
@@ -5454,6 +5457,7 @@ fn mirror_uid_lookup_covers_the_root_and_synced_descendants_only() {
             local_size: 2,
             remote_rev: Some("3".into()),
             remote_hash: Some("2".into()),
+            remote_revision_id: None,
             local_mtime_ns: None,
         },
     )
@@ -6631,6 +6635,71 @@ fn migration_v27_adds_the_access_deferral_column_to_a_v26_queue() {
     let _ = std::fs::remove_file(format!("{}.lock", path.display()));
 }
 
+/// V44 adds the remote revision a baseline row was synced at. A row carried
+/// over from V43 has none, and the mirror then compares the modification time
+/// and size as before, so upgrading changes nothing about what is in step
+/// (`docs/BUGS.md` B249).
+#[test]
+fn migration_v44_adds_the_synced_revision_without_disturbing_a_v43_baseline() {
+    let path = std::env::temp_dir().join(format!(
+        "pdfs-db-v43-fixture-{}-{}.db",
+        std::process::id(),
+        std::time::SystemTime::now()
+            .duration_since(std::time::UNIX_EPOCH)
+            .unwrap()
+            .as_nanos()
+    ));
+    let row = StoredSyncEntry {
+        rel_path: "notes.txt".into(),
+        remote_uid: Some("vol~notes".into()),
+        local_mtime: 1_700_000_000,
+        local_mtime_ns: Some(1_700_000_000_123_456_789),
+        local_size: 42,
+        remote_rev: Some("1700000000".into()),
+        remote_hash: Some("42".into()),
+        remote_revision_id: Some("rev-1".into()),
+    };
+    {
+        let db = Db::open(&path).unwrap();
+        let folder = db
+            .sync_folder_add("/home/me/Mirror", "vol~root", "share")
+            .unwrap();
+        db.sync_entry_upsert(folder, &row).unwrap();
+    }
+    {
+        // Put the file back in the state a released V43 database was in.
+        let conn = rusqlite::Connection::open(&path).unwrap();
+        conn.execute_batch(
+            "ALTER TABLE sync_entry DROP COLUMN remote_revision_id;
+             UPDATE sync_state SET value = '43' WHERE key = 'schema_version';",
+        )
+        .unwrap();
+    }
+
+    let db = Db::open(&path).unwrap();
+    let entries = db.sync_entries(1).unwrap();
+    let entry = entries.get("notes.txt").expect("the baseline row survives");
+    assert_eq!(
+        entry,
+        &StoredSyncEntry {
+            remote_revision_id: None,
+            ..row.clone()
+        }
+    );
+
+    // And the column is live, including through a move of the row.
+    db.sync_entry_upsert(1, &row).unwrap();
+    db.sync_entries_move(1, "notes.txt", 1, "moved.txt")
+        .unwrap();
+    let entries = db.sync_entries(1).unwrap();
+    assert_eq!(
+        entries["moved.txt"].remote_revision_id.as_deref(),
+        Some("rev-1")
+    );
+    drop(db);
+    let _ = std::fs::remove_file(&path);
+}
+
 /// A mirror baseline carried over from V27 keeps its rows, and each one reads
 /// back with no nanosecond time. That `None` is the whole point: it makes the
 /// comparison fall back to whole seconds for rows written before the column
@@ -6661,6 +6730,7 @@ fn migration_v28_adds_sub_second_local_times_without_disturbing_a_v27_baseline()
                 local_size: 42,
                 remote_rev: Some("1700000000".into()),
                 remote_hash: Some("42".into()),
+                remote_revision_id: None,
             },
         )
         .unwrap();