From 7e3df7602d4cf90e3f33114368d0b64eef81ec82 Mon Sep 17 00:00:00 2001 From: Nils Pukropp Date: Sat, 3 Oct 2026 15:10:51 +0200 Subject: [PATCH] feat(core): give every node row a local id that never changes First step of phase 2 of docs/MILESTONE-3.0.0.md. Schema 36 rebuilds `nodes` with `lid INTEGER PRIMARY KEY AUTOINCREMENT`. Each row takes its old rowid as its lid, so the search index, keyed by rowid, stays valid. AUTOINCREMENT keeps a deleted row's lid from coming back. `uid` stays unique and becomes nullable, for nodes that are not on Drive yet. A landed create's row now takes the real uid in place, keeping its lid. When a listing had already stored the real node, its content is kept under the placeholder's lid, which the node was known by first. Nothing reads the lid yet apart from `Db::lid_of`. --- crates/pdfs-core/src/db/migrations.rs | 55 ++++++++- crates/pdfs-core/src/db/nodes.rs | 42 +++++-- crates/pdfs-core/src/db/tests.rs | 153 ++++++++++++++++++++++++++ 3 files changed, 239 insertions(+), 11 deletions(-) diff --git a/crates/pdfs-core/src/db/migrations.rs b/crates/pdfs-core/src/db/migrations.rs index 6d073ce..92136b2 100644 --- a/crates/pdfs-core/src/db/migrations.rs +++ b/crates/pdfs-core/src/db/migrations.rs @@ -9,7 +9,7 @@ use super::Db; use crate::Result; /// Current schema version. Bump on every forward migration added below. -pub(super) const SCHEMA_VERSION: i64 = 35; +pub(super) const SCHEMA_VERSION: i64 = 36; impl Db { pub(super) fn migrate(&self) -> Result<()> { @@ -332,6 +332,22 @@ impl Db { tx.execute_batch(MIGRATION_V35)?; } } + if current < 36 { + // Same guards as V26-V35. + let has_column: bool = tx.query_row( + "SELECT COUNT(*) FROM pragma_table_info('nodes') WHERE name = 'lid'", + [], + |row| row.get::<_, i64>(0), + )? > 0; + let has_nodes: bool = tx.query_row( + "SELECT COUNT(*) FROM sqlite_master WHERE type = 'table' AND name = 'nodes'", + [], + |row| row.get::<_, i64>(0), + )? > 0; + if has_nodes && !has_column { + tx.execute_batch(MIGRATION_V36)?; + } + } tx.execute( "INSERT INTO sync_state (key, value) VALUES ('schema_version', ?1) ON CONFLICT(key) DO UPDATE SET value = excluded.value", @@ -1059,3 +1075,40 @@ ALTER TABLE photos ADD COLUMN similar_hash INTEGER; const MIGRATION_V35: &str = " ALTER TABLE trash ADD COLUMN trashed_at INTEGER; "; + +/// Schema v36: every node row has a local id (`lid`), phase 2 of +/// docs/MILESTONE-3.0.0.md. +/// +/// The `uid` was the row's key, so a node had no identity until Drive minted +/// one, and a create that landed moved its row to a new key. The `lid` is given +/// when the row is first written and never changes or comes back, which is what +/// `AUTOINCREMENT` adds over a plain rowid. It takes the old rowid, so the +/// search index, keyed by rowid since v15, stays valid. The `uid` becomes +/// nullable for a node that is not on Drive yet. +const MIGRATION_V36: &str = " +CREATE TABLE nodes_v36 ( + lid INTEGER PRIMARY KEY AUTOINCREMENT, + uid TEXT UNIQUE, + parent_uid TEXT, + name TEXT NOT NULL, + is_dir INTEGER NOT NULL, + size INTEGER, + mtime INTEGER NOT NULL, + revision_hash TEXT, + trashed INTEGER NOT NULL DEFAULT 0, + node_json TEXT, + listed INTEGER NOT NULL DEFAULT 0, + path TEXT +); +INSERT INTO nodes_v36 + (lid, uid, parent_uid, name, is_dir, size, mtime, revision_hash, trashed, node_json, listed, + path) + SELECT rowid, uid, parent_uid, name, is_dir, size, mtime, revision_hash, trashed, node_json, + listed, path + FROM nodes; +DROP TABLE nodes; +ALTER TABLE nodes_v36 RENAME TO nodes; +CREATE INDEX idx_nodes_parent ON nodes(parent_uid); +CREATE INDEX idx_nodes_name_nocase ON nodes(name COLLATE NOCASE); +CREATE INDEX idx_nodes_parent_live ON nodes(parent_uid) WHERE trashed = 0; +"; diff --git a/crates/pdfs-core/src/db/nodes.rs b/crates/pdfs-core/src/db/nodes.rs index c5e73df..d5bebda 100644 --- a/crates/pdfs-core/src/db/nodes.rs +++ b/crates/pdfs-core/src/db/nodes.rs @@ -544,6 +544,18 @@ impl Db { } } + /// The local id of the node stored under `uid`, or `None` when it has no + /// row. It stays the same for as long as the row exists, including when a + /// create lands and the row takes the uid Drive gave it. + pub fn lid_of(&self, uid: &str) -> Result> { + let conn = self.read(); + Ok(conn + .query_row("SELECT lid FROM nodes WHERE uid = ?1", params![uid], |r| { + r.get(0) + }) + .optional()?) + } + /// Read the persisted incremental-sync cursor (a `DriveEventId`), if any. /// The daemon resumes from this on restart instead of reseeding to the /// server head, so changes made while unmounted are still applied (P2). @@ -608,7 +620,8 @@ struct PriorRow { /// and was deferred five seconds behind a folder that had just landed /// (`docs/BUGS.md` B120). The placeholder's own node, readdressed, stands in /// until the server's copy is upserted over it. A row the real uid already -/// has is newer than the placeholder and is kept. +/// has is newer than the placeholder, so its node is kept, under the +/// placeholder's `lid`: the node was known by that one first. pub(super) fn adopt_placeholder_row_tx( tx: &Transaction<'_>, local: &str, @@ -620,22 +633,31 @@ pub(super) fn adopt_placeholder_row_tx( )?; let row: Option<(i64, Option)> = tx .query_row( - "SELECT rowid, node_json FROM nodes WHERE uid = ?1", + "SELECT lid, node_json FROM nodes WHERE uid = ?1", params![local], |row| Ok((row.get(0)?, row.get(1)?)), ) .optional()?; - let Some((rowid, json)) = row else { + let Some((lid, json)) = row else { return Ok(()); }; - tx.execute("DELETE FROM nodes WHERE uid = ?1", params![local])?; - tx.execute("DELETE FROM nodes_fts WHERE rowid = ?1", params![rowid])?; - let known: bool = tx.query_row( - "SELECT EXISTS(SELECT 1 FROM nodes WHERE uid = ?1)", - params![real], - |row| row.get(0), + let known: Option<(i64, Option)> = tx + .query_row( + "SELECT lid, node_json FROM nodes WHERE uid = ?1", + params![real], + |row| Ok((row.get(0)?, row.get(1)?)), + ) + .optional()?; + if let Some((known_lid, _)) = known { + tx.execute("DELETE FROM nodes WHERE lid = ?1", params![known_lid])?; + tx.execute("DELETE FROM nodes_fts WHERE rowid = ?1", params![known_lid])?; + } + tx.execute( + "UPDATE nodes SET uid = ?2 WHERE lid = ?1", + params![lid, real], )?; - let (false, Some(json), Some(real)) = (known, json, parse_node_uid(real)) else { + let json = known.and_then(|(_, known_json)| known_json).or(json); + let (Some(json), Some(real)) = (json, parse_node_uid(real)) else { return Ok(()); }; let mut node: Node = serde_json::from_str(&json)?; diff --git a/crates/pdfs-core/src/db/tests.rs b/crates/pdfs-core/src/db/tests.rs index b985fc5..d2cfe72 100644 --- a/crates/pdfs-core/src/db/tests.rs +++ b/crates/pdfs-core/src/db/tests.rs @@ -3056,6 +3056,159 @@ fn a_landed_placeholder_never_overwrites_the_server_copy() { ); } +#[test] +fn a_node_keeps_its_lid_across_updates_and_a_new_one_never_reuses_it() { + let db = Db::open_in_memory().unwrap(); + db.upsert_node(&folder("root", None, "My Files")).unwrap(); + db.upsert_node(&file("a", "root", "a.txt", 1)).unwrap(); + db.upsert_node(&file("b", "root", "b.txt", 1)).unwrap(); + let a = db.lid_of(&uid("a").to_string()).unwrap().unwrap(); + let b = db.lid_of(&uid("b").to_string()).unwrap().unwrap(); + + db.upsert_node(&file("a", "root", "renamed.txt", 2)) + .unwrap(); + assert_eq!(db.lid_of(&uid("a").to_string()).unwrap(), Some(a)); + + db.delete_node(&uid("b")).unwrap(); + db.upsert_node(&file("c", "root", "c.txt", 1)).unwrap(); + let c = db.lid_of(&uid("c").to_string()).unwrap().unwrap(); + assert!(c > b, "lid {c} reuses one a deleted row had"); +} + +#[test] +fn a_landed_placeholder_keeps_its_lid() { + let db = Db::open_in_memory().unwrap(); + db.upsert_node(&folder("root", None, "My Files")).unwrap(); + for known in [false, true] { + let local = NodeUid::new(VolumeId::from("local"), LinkId::from(format!("{known}"))); + let real = uid(&format!("real-{known}")); + let mut placeholder = folder("dir", Some("root"), "New folder"); + placeholder.uid = local.clone(); + db.upsert_node(&placeholder).unwrap(); + let lid = db.lid_of(&local.to_string()).unwrap().unwrap(); + if known { + // The event stream reported the folder before the drain got round + // to it. + let mut listed = folder("x", Some("root"), "New folder (1)"); + listed.uid = real.clone(); + db.upsert_node(&listed).unwrap(); + } + + db.remap_local_uid(&local.to_string(), &real.to_string()) + .unwrap(); + + assert_eq!(db.lid_of(&local.to_string()).unwrap(), None); + assert_eq!(db.lid_of(&real.to_string()).unwrap(), Some(lid)); + let name = if known { + "New folder (1)" + } else { + "New folder" + }; + assert_eq!( + db.node_by_uid(&real.to_string()).unwrap().map(|n| n.name), + Some(name.to_string()) + ); + assert_eq!(db.search(name, 10).unwrap().len(), 1, "{name}"); + } +} + +#[test] +fn migration_v36_gives_every_node_its_rowid_as_lid() { + let path = std::env::temp_dir().join(format!( + "pdfs-db-v35-fixture-{}-{}.db", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_nanos() + )); + let local = NodeUid::new(VolumeId::from("local"), LinkId::from("dir")); + { + let db = Db::open(&path).unwrap(); + db.upsert_node(&folder("root", None, "My Files")).unwrap(); + let mut placeholder = folder("dir", Some("root"), "Offline folder"); + placeholder.uid = local.clone(); + db.upsert_node(&placeholder).unwrap(); + db.upsert_node(&file("gone", "root", "gone.txt", 1)) + .unwrap(); + db.upsert_node(&file("kept", "root", "kept-report.txt", 1)) + .unwrap(); + db.delete_node(&uid("gone")).unwrap(); + let (id, _) = db + .enqueue_op(&PendingOp { + id: 0, + kind: OP_MKDIR.to_string(), + uid: local.to_string(), + parent_uid: Some(uid("root").to_string()), + name: Some("Offline folder".to_string()), + blob_path: None, + meta_json: None, + created_at: 1, + attempts: 0, + last_error: None, + next_attempt_at: 0, + }) + .unwrap(); + db.record_op_failure(id, "network unreachable", 5_000) + .unwrap(); + } + { + // Put the file back in the state a released V35 database was in: + // keyed by uid, with the rowids of the rows it held. + let conn = rusqlite::Connection::open(&path).unwrap(); + conn.execute_batch( + "CREATE TABLE nodes_v35 ( + uid TEXT PRIMARY KEY, parent_uid TEXT, name TEXT NOT NULL, + is_dir INTEGER NOT NULL, size INTEGER, mtime INTEGER NOT NULL, + revision_hash TEXT, trashed INTEGER NOT NULL DEFAULT 0, node_json TEXT, + listed INTEGER NOT NULL DEFAULT 0, path TEXT + ); + INSERT INTO nodes_v35 (rowid, uid, parent_uid, name, is_dir, size, mtime, + revision_hash, trashed, node_json, listed, path) + SELECT lid, uid, parent_uid, name, is_dir, size, mtime, revision_hash, + trashed, node_json, listed, path FROM nodes; + DROP TABLE nodes; + ALTER TABLE nodes_v35 RENAME TO nodes; + CREATE INDEX idx_nodes_parent ON nodes(parent_uid); + CREATE INDEX idx_nodes_name_nocase ON nodes(name COLLATE NOCASE); + CREATE INDEX idx_nodes_parent_live ON nodes(parent_uid) WHERE trashed = 0; + DELETE FROM sqlite_sequence WHERE name = 'nodes'; + UPDATE sync_state SET value = '35' WHERE key = 'schema_version';", + ) + .unwrap(); + } + let rowids: Vec<(String, i64)> = { + let conn = rusqlite::Connection::open(&path).unwrap(); + let mut stmt = conn + .prepare("SELECT uid, rowid FROM nodes ORDER BY rowid") + .unwrap(); + stmt.query_map([], |r| Ok((r.get(0)?, r.get(1)?))) + .unwrap() + .map(|row| row.unwrap()) + .collect() + }; + assert_eq!(rowids.len(), 3); + + let db = Db::open(&path).unwrap(); + for (uid, rowid) in &rowids { + assert_eq!(db.lid_of(uid).unwrap(), Some(*rowid), "{uid}"); + } + let found = db.search("report", 10).unwrap(); + assert_eq!(found.len(), 1, "the search index still finds the node"); + assert_eq!(found[0].node.uid, uid("kept")); + let ops = db.pending_ops().unwrap(); + assert_eq!(ops.len(), 1); + assert_eq!(ops[0].uid, local.to_string()); + assert_eq!(ops[0].attempts, 1); + assert_eq!(ops[0].last_error.as_deref(), Some("network unreachable")); + + db.upsert_node(&file("new", "root", "new.txt", 1)).unwrap(); + let newest = db.lid_of(&uid("new").to_string()).unwrap().unwrap(); + assert!(newest > rowids.last().unwrap().1); + drop(db); + let _ = std::fs::remove_file(&path); +} + #[test] fn a_failed_op_stays_queued_with_backoff() { let db = Db::open_in_memory().unwrap();