diff --git a/crates/pdfs-core/src/db/migrations.rs b/crates/pdfs-core/src/db/migrations.rs index 1689224..08b946b 100644 --- a/crates/pdfs-core/src/db/migrations.rs +++ b/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; +"; diff --git a/crates/pdfs-core/src/db/sync.rs b/crates/pdfs-core/src/db/sync.rs index af93b07..960c00a 100644 --- a/crates/pdfs-core/src/db/sync.rs +++ b/crates/pdfs-core/src/db/sync.rs @@ -45,6 +45,12 @@ pub struct StoredSyncEntry { pub local_mtime_ns: Option, pub remote_rev: Option, pub remote_hash: Option, + /// 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, } 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::>>()?; @@ -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, ])?; } } diff --git a/crates/pdfs-core/src/db/tests.rs b/crates/pdfs-core/src/db/tests.rs index 63d4c1c..549eb8c 100644 --- a/crates/pdfs-core/src/db/tests.rs +++ b/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(); diff --git a/crates/pdfs-fuse/src/lib.rs b/crates/pdfs-fuse/src/lib.rs index 229bc66..8d02340 100644 --- a/crates/pdfs-fuse/src/lib.rs +++ b/crates/pdfs-fuse/src/lib.rs @@ -6165,11 +6165,61 @@ fn is_gone(e: &(dyn std::error::Error + 'static)) -> bool { /// (`sync.rs`, `conflict_path`), so the two halves of the product name the same /// situation the same way and a user only has to learn it once. fn conflict_name(name: &str, stamp: i64) -> String { + conflict_name_as(name, conflict_label(), stamp) +} + +/// [`conflict_name`] with the machine's label given. +fn conflict_name_as(name: &str, label: &str, stamp: i64) -> String { let path = Path::new(name); let stem = path.file_stem().and_then(|s| s.to_str()).unwrap_or(name); + let tag = conflict_tag(label, stamp, 0); match path.extension().and_then(|s| s.to_str()) { - Some(ext) => format!("{stem} (sync-conflict {stamp}).{ext}"), - None => format!("{stem} (sync-conflict {stamp})"), + Some(ext) => format!("{stem} (sync-conflict {tag}).{ext}"), + None => format!("{stem} (sync-conflict {tag})"), + } +} + +/// This machine's name as its conflict copies carry it (`docs/BUGS.md` B251). +/// +/// Two computers syncing one folder that each keep a copy of the same file in +/// the same second would otherwise make the same name on Drive, and nobody +/// could tell which computer a copy came from. Read once, from the hostname. +fn conflict_label() -> &'static str { + static LABEL: OnceLock = OnceLock::new(); + LABEL.get_or_init(|| conflict_label_of(&this_hostname())) +} + +/// `host` as one token: anything but ASCII letters, digits, `-`, `_` and `.` +/// becomes `-`, so the label holds no space for `conflict_base_name` to trip +/// on and no `/` or `)` to break the name. +fn conflict_label_of(host: &str) -> String { + let label: String = host + .chars() + .take(32) + .map(|c| { + if c.is_ascii_alphanumeric() || matches!(c, '-' | '_' | '.') { + c + } else { + '-' + } + }) + .collect(); + label.trim_matches('-').to_string() +} + +/// The inside of a conflict copy's marker: `