From 9c93860cd8fd8567f86255584ec455d0e4c68d7f Mon Sep 17 00:00:00 2001 From: Nils Pukropp Date: Sun, 4 Oct 2026 13:26:19 +0200 Subject: [PATCH] refactor(core): rows and ops name a node's parent by local id; a landing rewrites nothing below it --- crates/pdfs-core/src/db/nodes.rs | 247 +++++++++++++----------- crates/pdfs-core/src/db/ops.rs | 54 +++--- crates/pdfs-core/src/db/pins.rs | 28 +-- crates/pdfs-core/src/db/share_access.rs | 13 +- crates/pdfs-core/src/db/tests.rs | 98 ++++++++++ crates/pdfs-core/src/db/utils.rs | 12 +- docs/MILESTONE-3.0.0-STATUS.md | 15 +- 7 files changed, 301 insertions(+), 166 deletions(-) diff --git a/crates/pdfs-core/src/db/nodes.rs b/crates/pdfs-core/src/db/nodes.rs index 4de542c..e1c1d46 100644 --- a/crates/pdfs-core/src/db/nodes.rs +++ b/crates/pdfs-core/src/db/nodes.rs @@ -3,6 +3,7 @@ use std::collections::HashSet; +use rusqlite::types::Value; use rusqlite::{OptionalExtension, Transaction, params}; use super::Db; @@ -65,12 +66,13 @@ impl Db { /// snapshot while offline, including after a live event expired its TTL. pub fn visible_children(&self, parent: &NodeUid) -> Result> { let conn = self.read(); - let mut stmt = conn.prepare( - "SELECT node_json FROM nodes - WHERE parent_uid = ?1 AND trashed = 0 AND node_json IS NOT NULL - ORDER BY name, uid", - )?; - let rows = stmt.query_map([parent.to_string()], |row| row.get::<_, String>(0))?; + let (key, parent) = parent_key(&conn, &parent.to_string())?; + let mut stmt = conn.prepare(&format!( + "SELECT {NODE_NOW} FROM nodes n LEFT JOIN nodes p ON p.lid = n.parent_lid + WHERE n.{key} = ?1 AND n.trashed = 0 AND n.node_json IS NOT NULL + ORDER BY n.name, n.uid" + ))?; + let rows = stmt.query_map([parent], |row| row.get::<_, String>(0))?; let mut nodes = Vec::new(); for row in rows { nodes.push(serde_json::from_str(&row?)?); @@ -307,12 +309,14 @@ impl Db { /// Check if a folder node has any non-trashed children in the database. pub fn has_children(&self, parent_uid: &NodeUid) -> Result { - let uid_str = parent_uid.to_string(); let conn = self.read(); + let (key, parent) = parent_key(&conn, &parent_uid.to_string())?; let count: i64 = conn.query_row( - "SELECT COUNT(*) FROM nodes - WHERE parent_uid = ?1 AND trashed = 0 AND node_json IS NOT NULL", - params![uid_str], + &format!( + "SELECT COUNT(*) FROM nodes + WHERE {key} = ?1 AND trashed = 0 AND node_json IS NOT NULL" + ), + params![parent], |row| row.get(0), )?; Ok(count > 0) @@ -349,11 +353,11 @@ impl Db { pub fn path_relative_to(&self, ancestor_uid: &str, uid: &str) -> Result> { let conn = self.read(); let mut stmt = conn.prepare( - "WITH RECURSIVE anc(uid, parent_uid, name, depth) AS ( - SELECT uid, parent_uid, name, 0 FROM nodes WHERE uid = ?1 + "WITH RECURSIVE anc(uid, parent_lid, name, depth) AS ( + SELECT uid, parent_lid, name, 0 FROM nodes WHERE uid = ?1 UNION ALL - SELECT n.uid, n.parent_uid, n.name, anc.depth + 1 - FROM nodes n JOIN anc ON n.uid = anc.parent_uid + SELECT n.uid, n.parent_lid, n.name, anc.depth + 1 + FROM nodes n JOIN anc ON n.lid = anc.parent_lid WHERE anc.depth < 1024 ) SELECT uid, name FROM anc ORDER BY depth", @@ -402,12 +406,13 @@ impl Db { let conn = self.read(); let rows: Vec = if query.chars().count() < TRIGRAM_MIN { let pat = format!("%{}%", like_escape(query)); - let mut stmt = conn.prepare( - "SELECT node_json, uid, path FROM nodes - WHERE name LIKE ?1 ESCAPE '\\' AND trashed = 0 AND node_json IS NOT NULL - AND (?3 IS NULL OR path IS NULL OR path LIKE ?3 ESCAPE '\\') - ORDER BY name LIMIT ?2", - )?; + let mut stmt = conn.prepare(&format!( + "SELECT {NODE_NOW}, n.uid, n.path + FROM nodes n LEFT JOIN nodes p ON p.lid = n.parent_lid + WHERE n.name LIKE ?1 ESCAPE '\\' AND n.trashed = 0 AND n.node_json IS NOT NULL + AND (?3 IS NULL OR n.path IS NULL OR n.path LIKE ?3 ESCAPE '\\') + ORDER BY n.name LIMIT ?2" + ))?; collect_hits(stmt.query_map(params![pat, limit as i64, scope_pat], hit_row)?)? } else { // Escape double quotes and quote each term, then combine with AND so @@ -417,13 +422,14 @@ impl Db { .map(|word| format!("\"{}\"", word.replace('"', "\"\""))) .collect::>() .join(" AND "); - let mut stmt = conn.prepare( - "SELECT n.node_json, n.uid, n.path + let mut stmt = conn.prepare(&format!( + "SELECT {NODE_NOW}, n.uid, n.path FROM nodes_fts f JOIN nodes n ON n.rowid = f.rowid + LEFT JOIN nodes p ON p.lid = n.parent_lid WHERE f.name MATCH ?1 AND n.trashed = 0 AND n.node_json IS NOT NULL AND (?3 IS NULL OR n.path IS NULL OR n.path LIKE ?3 ESCAPE '\\') - ORDER BY f.rank LIMIT ?2", - )?; + ORDER BY f.rank LIMIT ?2" + ))?; collect_hits(stmt.query_map(params![phrase, limit as i64, scope_pat], hit_row)?)? }; @@ -460,20 +466,22 @@ impl Db { let lane_limit = limit.div_ceil(2); let mut rows: Vec = if terms.is_empty() { let pat = format!("%{}%", like_escape(query)); - let mut stmt = conn.prepare( - "SELECT node_json, uid, path FROM nodes - WHERE name LIKE ?1 ESCAPE '\\' AND trashed = 0 AND node_json IS NOT NULL - ORDER BY name LIMIT ?2", - )?; + let mut stmt = conn.prepare(&format!( + "SELECT {NODE_NOW}, n.uid, n.path + FROM nodes n LEFT JOIN nodes p ON p.lid = n.parent_lid + WHERE n.name LIKE ?1 ESCAPE '\\' AND n.trashed = 0 AND n.node_json IS NOT NULL + ORDER BY n.name LIMIT ?2" + ))?; collect_hits(stmt.query_map(params![pat, limit as i64], hit_row)?)? } else { let expression = terms.join(" OR "); - let mut stmt = conn.prepare( - "SELECT n.node_json, n.uid, n.path + let mut stmt = conn.prepare(&format!( + "SELECT {NODE_NOW}, n.uid, n.path FROM nodes_fts f JOIN nodes n ON n.rowid = f.rowid + LEFT JOIN nodes p ON p.lid = n.parent_lid WHERE nodes_fts MATCH ?1 AND n.trashed = 0 AND n.node_json IS NOT NULL - ORDER BY f.rank LIMIT ?2", - )?; + ORDER BY f.rank LIMIT ?2" + ))?; collect_hits(stmt.query_map(params![expression, lane_limit as i64], hit_row)?)? }; // Trigram rank does not know that a name *starts* with the query, and a @@ -483,12 +491,13 @@ impl Db { if !terms.is_empty() && let Some(first) = query.chars().next() { - let mut stmt = conn.prepare( - "SELECT node_json, uid, path FROM nodes - WHERE name COLLATE NOCASE LIKE ?1 ESCAPE '\\' - AND trashed = 0 AND node_json IS NOT NULL - ORDER BY name COLLATE NOCASE LIMIT ?2", - )?; + let mut stmt = conn.prepare(&format!( + "SELECT {NODE_NOW}, n.uid, n.path + FROM nodes n LEFT JOIN nodes p ON p.lid = n.parent_lid + WHERE n.name COLLATE NOCASE LIKE ?1 ESCAPE '\\' + AND n.trashed = 0 AND n.node_json IS NOT NULL + ORDER BY n.name COLLATE NOCASE LIMIT ?2" + ))?; let mut extra = Vec::new(); for pattern in [ format!("{}%", like_escape(query)), @@ -538,24 +547,20 @@ impl Db { /// stored below it may still hold the placeholder. pub fn load_all(&self) -> Result> { let conn = self.read(); - let mut stmt = conn.prepare( - "SELECT n.node_json, n.listed, n.lid, p.uid + let mut stmt = conn.prepare(&format!( + "SELECT {NODE_NOW}, n.listed, n.lid FROM nodes n LEFT JOIN nodes p ON p.lid = n.parent_lid - WHERE n.node_json IS NOT NULL", - )?; + WHERE n.node_json IS NOT NULL" + ))?; let rows = stmt.query_map([], |row| { let json: String = row.get(0)?; let listed: i64 = row.get(1)?; - let parent: Option = row.get(3)?; - Ok((json, listed != 0, row.get(2)?, parent)) + Ok((json, listed != 0, row.get(2)?)) })?; let mut out = Vec::new(); for row in rows { - let (json, listed, lid, parent) = row?; - let mut node: Node = serde_json::from_str(&json)?; - if let Some(parent) = parent.as_deref().and_then(parse_node_uid) { - node.parent_uid = Some(parent); - } + let (json, listed, lid) = row?; + let node: Node = serde_json::from_str(&json)?; out.push(StoredNode { node, listed, lid }); } Ok(out) @@ -568,7 +573,10 @@ impl Db { let conn = self.read(); let json: Option = conn .query_row( - "SELECT node_json FROM nodes WHERE uid = ?1 AND node_json IS NOT NULL", + &format!( + "SELECT {NODE_NOW} FROM nodes n LEFT JOIN nodes p ON p.lid = n.parent_lid + WHERE n.uid = ?1 AND n.node_json IS NOT NULL" + ), params![uid], |r| r.get(0), ) @@ -613,20 +621,26 @@ impl Db { let mut known = tx.prepare_cached("SELECT lid FROM nodes WHERE uid = ?1")?; let mut stub = tx.prepare_cached( "INSERT INTO nodes (uid, parent_uid, name, is_dir, mtime, trashed, parent_lid) - VALUES (?1, ?2, ?3, ?4, ?5, ?6, (SELECT lid FROM nodes WHERE uid = ?2))", + VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7)", )?; for node in nodes { let uid = node.uid.to_string(); let lid = match known.query_row(params![uid], |r| r.get(0)).optional()? { Some(lid) => lid, None => { + let parent = node.parent_uid.as_ref().map(ToString::to_string); + let parent_lid = match &parent { + Some(parent) => node_lid(&tx, parent)?, + None => None, + }; stub.execute(params![ uid, - node.parent_uid.as_ref().map(ToString::to_string), + parent, node.name, node.is_folder() as i64, node.modification_time, node.trashed as i64, + parent_lid, ])?; let lid = tx.last_insert_rowid(); adopt_children_tx(&tx, &uid, lid)?; @@ -699,11 +713,12 @@ impl Db { /// had: a listing dropped as stale may miss or keep a child. pub fn known_children(&self, parent: &NodeUid) -> Result> { let conn = self.read(); - let mut stmt = conn.prepare( - "SELECT node_json FROM nodes - WHERE parent_uid = ?1 AND node_json IS NOT NULL AND trashed = 0", - )?; - let rows = stmt.query_map(params![parent.to_string()], |r| r.get::<_, String>(0))?; + let (key, parent) = parent_key(&conn, &parent.to_string())?; + let mut stmt = conn.prepare(&format!( + "SELECT {NODE_NOW} FROM nodes n LEFT JOIN nodes p ON p.lid = n.parent_lid + WHERE n.{key} = ?1 AND n.node_json IS NOT NULL AND n.trashed = 0" + ))?; + let rows = stmt.query_map(params![parent], |r| r.get::<_, String>(0))?; let mut out = Vec::new(); for json in rows { out.push(serde_json::from_str(&json?)?); @@ -721,13 +736,13 @@ impl Db { /// until the create lands (`docs/BUGS.md` B132). pub fn queued_children(&self, parent: &NodeUid) -> Result<(Vec, HashSet)> { let conn = self.read(); - let parent = parent.to_string(); - let mut stmt = conn.prepare( - "SELECT n.node_json FROM nodes n - WHERE n.parent_uid = ?1 AND n.node_json IS NOT NULL AND n.trashed = 0 - AND EXISTS (SELECT 1 FROM pending_op p - WHERE p.uid = n.uid AND p.kind IN (?2, ?3, ?4))", - )?; + let (key, parent) = parent_key(&conn, &parent.to_string())?; + let mut stmt = conn.prepare(&format!( + "SELECT {NODE_NOW} FROM nodes n LEFT JOIN nodes p ON p.lid = n.parent_lid + WHERE n.{key} = ?1 AND n.node_json IS NOT NULL AND n.trashed = 0 + AND EXISTS (SELECT 1 FROM pending_op o + WHERE (o.lid = n.lid OR o.uid = n.uid) AND o.kind IN (?2, ?3, ?4))" + ))?; let rows = stmt.query_map(params![parent, OP_CREATE, OP_MKDIR, OP_RENAME], |r| { r.get::<_, String>(0) })?; @@ -735,10 +750,10 @@ impl Db { for json in rows { here.push(serde_json::from_str(&json?)?); } - let mut stmt = conn.prepare( - "SELECT DISTINCT p.uid FROM pending_op p JOIN nodes n ON n.uid = p.uid - WHERE p.kind = ?2 AND n.parent_uid IS NOT ?1", - )?; + let mut stmt = conn.prepare(&format!( + "SELECT DISTINCT n.uid FROM pending_op p JOIN nodes n ON n.lid = p.lid OR n.uid = p.uid + WHERE p.kind = ?2 AND n.{key} IS NOT ?1" + ))?; let gone = stmt .query_map(params![parent, OP_RENAME], |r| r.get::<_, String>(0))? .collect::, _>>()?; @@ -777,8 +792,29 @@ pub(super) fn node_lid(conn: &rusqlite::Connection, uid: &str) -> Result Result<(&'static str, Value)> { + Ok(match node_lid(conn, parent)? { + Some(lid) => ("parent_lid", Value::Integer(lid)), + None => ("parent_uid", Value::Text(parent.to_owned())), + }) +} + +/// `n.node_json` naming its parent as the parent's row `p`, joined by +/// `n.parent_lid`, has it now. A folder that lands takes its real uid in its +/// own row only, so a node stored below it still names the stand-in. +const NODE_NOW: &str = "CASE WHEN p.uid IS NULL OR p.uid = n.parent_uid THEN n.node_json + ELSE json_set(n.node_json, '$.parent_uid', json_object( + 'volume_id', substr(p.uid, 1, instr(p.uid, '~') - 1), + 'link_id', substr(p.uid, instr(p.uid, '~') + 1))) END"; + +/// Hand a drained placeholder's row to the real uid it landed as. +/// +/// Nothing else is rewritten. The rows below it and the ops made inside it +/// name it by its local id, which stays; what reads them for a uid takes the +/// one the row has now ([`NODE_NOW`], `PARENT_NOW` in `ops.rs`). /// /// Dropping the placeholder row and waiting for the server's copy of the real /// node left a window with no row for either uid. A queued child drained in @@ -800,19 +836,6 @@ pub(super) fn adopt_placeholder_row_tx( params![local, real], )?; tx.execute("DELETE FROM pins WHERE uid = ?1", params![local])?; - // The node a row holds names its parent too, and is what a restart builds - // the tree from: a folder that landed with no write to its children after - // left them under a parent no row has (`docs/BUGS.md` B140). - let real_json = parse_node_uid(real) - .map(|uid| serde_json::to_string(&uid)) - .transpose()?; - tx.execute( - "UPDATE nodes SET parent_uid = ?2, - node_json = CASE WHEN ?3 IS NULL OR node_json IS NULL THEN node_json - ELSE json_set(node_json, '$.parent_uid', json(?3)) END - WHERE parent_uid = ?1", - params![local, real, real_json], - )?; let row: Option<(i64, Option)> = tx .query_row( "SELECT lid, node_json FROM nodes WHERE uid = ?1", @@ -829,11 +852,6 @@ pub(super) fn adopt_placeholder_row_tx( let Some((lid, json)) = row else { return Ok(()); }; - // The ops made inside the folder name it by its local id, which stays. - tx.execute( - "UPDATE pending_op SET parent_uid = ?2 WHERE parent_lid = ?1", - params![lid, real], - )?; let known: Option<(i64, Option)> = tx .query_row( "SELECT lid, node_json FROM nodes WHERE uid = ?1", @@ -908,13 +926,16 @@ fn upsert_node_tx(tx: &Transaction<'_>, node: &Node) -> Result<()> { let (parent_lid, path) = match &parent_uid { None => (None, String::new()), Some(parent) => { - let parent_row: Option<(i64, Option)> = tx - .query_row( - "SELECT lid, path FROM nodes WHERE uid = ?1", - params![parent], - |row| Ok((row.get(0)?, row.get(1)?)), - ) - .optional()?; + let parent_row: Option<(i64, Option)> = match node_lid(tx, parent)? { + Some(lid) => tx + .query_row( + "SELECT lid, path FROM nodes WHERE lid = ?1", + params![lid], + |row| Ok((row.get(0)?, row.get(1)?)), + ) + .optional()?, + None => None, + }; let (lid, parent_path) = parent_row.unzip(); ( lid, @@ -980,7 +1001,7 @@ fn upsert_node_tx(tx: &Transaction<'_>, node: &Node) -> Result<()> { || prior.trashed != node.trashed }); if node.is_folder() && subtree_moved { - reindex_subtree_tx(tx, &uid, &path, indexable)?; + reindex_subtree_tx(tx, rowid, &path, indexable)?; } Ok(()) } @@ -1001,22 +1022,22 @@ fn upsert_node_tx(tx: &Transaction<'_>, node: &Node) -> Result<()> { /// this deep. fn reindex_subtree_tx( tx: &Transaction<'_>, - folder: &str, + folder: i64, folder_path: &str, folder_indexable: bool, ) -> Result<()> { let descendants: Vec<(i64, String, bool)> = { let mut stmt = tx.prepare( - "WITH RECURSIVE sub(rowid, uid, name, path, blocked, depth) AS ( - SELECT n.rowid, n.uid, n.name, + "WITH RECURSIVE sub(rowid, name, path, blocked, depth) AS ( + SELECT n.rowid, n.name, CASE WHEN ?2 = '' THEN n.name ELSE ?2 || '/' || n.name END, n.trashed, 0 - FROM nodes n WHERE n.parent_uid = ?1 + FROM nodes n WHERE n.parent_lid = ?1 UNION ALL - SELECT n.rowid, n.uid, n.name, + SELECT n.rowid, n.name, sub.path || '/' || n.name, MAX(sub.blocked, n.trashed), sub.depth + 1 - FROM nodes n JOIN sub ON n.parent_uid = sub.uid + FROM nodes n JOIN sub ON n.parent_lid = sub.rowid WHERE sub.depth < 256 ) SELECT rowid, path, blocked FROM sub", @@ -1057,7 +1078,8 @@ fn reindex_subtree_tx( } fn direct_child_uids_tx(tx: &Transaction<'_>, parent: &str) -> Result> { - let mut stmt = tx.prepare("SELECT uid FROM nodes WHERE parent_uid = ?1")?; + let (key, parent) = parent_key(tx, parent)?; + let mut stmt = tx.prepare(&format!("SELECT uid FROM nodes WHERE {key} = ?1"))?; let rows = stmt.query_map([parent], |row| row.get::<_, String>(0))?; let mut uids = Vec::new(); for row in rows { @@ -1082,14 +1104,14 @@ fn direct_child_uids_tx(tx: &Transaction<'_>, parent: &str) -> Result, uid: &str) -> Result { let indexable: i64 = tx.query_row( - "WITH RECURSIVE ancestors(uid, parent_uid, trashed, depth, path) AS ( - SELECT uid, parent_uid, trashed, 0, char(31) || uid || char(31) + "WITH RECURSIVE ancestors(lid, parent_lid, trashed, depth, path) AS ( + SELECT lid, parent_lid, trashed, 0, ',' || lid || ',' FROM nodes WHERE uid = ?1 UNION ALL - SELECT n.uid, n.parent_uid, n.trashed, a.depth + 1, - a.path || n.uid || char(31) - FROM ancestors a JOIN nodes n ON n.uid = a.parent_uid - WHERE instr(a.path, char(31) || n.uid || char(31)) = 0 + SELECT n.lid, n.parent_lid, n.trashed, a.depth + 1, + a.path || n.lid || ',' + FROM ancestors a JOIN nodes n ON n.lid = a.parent_lid + WHERE instr(a.path, ',' || n.lid || ',') = 0 ) SELECT CASE WHEN COUNT(*) = 0 THEN 0 @@ -1097,8 +1119,7 @@ fn node_is_indexable_tx(tx: &Transaction<'_>, uid: &str) -> Result { WHEN EXISTS ( SELECT 1 FROM ancestors a WHERE a.depth = (SELECT MAX(depth) FROM ancestors) - AND a.parent_uid IS NOT NULL - AND EXISTS (SELECT 1 FROM nodes p WHERE p.uid = a.parent_uid) + AND EXISTS (SELECT 1 FROM nodes p WHERE p.lid = a.parent_lid) ) THEN 0 ELSE 1 END @@ -1116,7 +1137,7 @@ fn tombstone_subtree_tx(tx: &Transaction<'_>, root_uid: &str) -> Result<()> { SELECT rowid, uid, node_json FROM nodes WHERE uid = ?1 UNION SELECT n.rowid, n.uid, n.node_json FROM nodes n - JOIN subtree s ON n.parent_uid = s.uid + JOIN subtree s ON n.parent_lid = s.rowid ) SELECT rowid, uid, node_json FROM subtree", )?; diff --git a/crates/pdfs-core/src/db/ops.rs b/crates/pdfs-core/src/db/ops.rs index 22d3a59..6a8c631 100644 --- a/crates/pdfs-core/src/db/ops.rs +++ b/crates/pdfs-core/src/db/ops.rs @@ -110,25 +110,29 @@ pub fn op_supersedes(kind: &str) -> bool { fn drop_doomed_ops(tx: &rusqlite::Transaction<'_>, uid: &str) -> Result> { const DOOMED: &str = " WITH RECURSIVE - below(uid) AS ( - SELECT ?1 + below(uid, lid) AS ( + SELECT ?1, ?2 UNION - SELECT n.uid FROM nodes n JOIN below b ON n.parent_uid = b.uid + SELECT n.uid, n.lid FROM nodes n JOIN below b + ON n.parent_lid = b.lid OR (n.parent_lid IS NULL AND n.parent_uid = b.uid) ), - queued(uid) AS ( - SELECT ?1 + queued(uid, lid) AS ( + SELECT ?1, ?2 UNION - SELECT p.uid FROM pending_op p + SELECT p.uid, p.lid FROM pending_op p WHERE p.kind IN ('create', 'mkdir') - AND p.parent_uid IN (SELECT uid FROM below) + AND (p.parent_lid IN (SELECT lid FROM below) + OR p.parent_uid IN (SELECT uid FROM below)) UNION - SELECT p.uid FROM pending_op p JOIN queued q ON p.parent_uid = q.uid + SELECT p.uid, p.lid FROM pending_op p JOIN queued q + ON p.parent_lid = q.lid OR p.parent_uid = q.uid ), doomed(id) AS ( SELECT id FROM pending_op WHERE uid IN (SELECT uid FROM queued) - OR lid = ?2 - OR (kind = 'revision' AND uid IN (SELECT uid FROM below)) + OR lid IN (SELECT lid FROM queued) + OR (kind = 'revision' + AND (uid IN (SELECT uid FROM below) OR lid IN (SELECT lid FROM below))) )"; const SENT: &str = "kind IN ('create', 'mkdir') AND (claimed_at <> 0 OR attempts > 0)"; let lid = node_lid(tx, uid)?; @@ -147,8 +151,10 @@ fn drop_doomed_ops(tx: &rusqlite::Transaction<'_>, uid: &str) -> Result = tx .query_row( - "SELECT blob_path, meta_json, parent_uid, name, created_at - FROM pending_op WHERE id = ?1 AND kind <> ?2", + &format!( + "SELECT blob_path, meta_json, {PARENT_NOW}, name, created_at + FROM pending_op WHERE id = ?1 AND kind <> ?2" + ), params![id, OP_TRASH], |r| { Ok(CreateRow { @@ -1112,13 +1120,13 @@ impl Db { /// the one query that does. pub fn parked_create_ops(&self) -> Result> { let conn = self.read(); - let mut stmt = conn.prepare( - "SELECT id, kind, uid, parent_uid, name, blob_path, meta_json, created_at, + let mut stmt = conn.prepare(&format!( + "SELECT id, kind, {UID_NOW}, {PARENT_NOW}, name, blob_path, meta_json, created_at, attempts, last_error, next_attempt_at FROM pending_op WHERE next_attempt_at >= ?1 AND kind IN (?2, ?3) - ORDER BY created_at", - )?; + ORDER BY created_at" + ))?; let rows = stmt .query_map(params![PARK_UNTIL, OP_CREATE, OP_MKDIR], |r| { Ok(PendingOp { @@ -1300,9 +1308,11 @@ impl Db { ) -> Result { let conn = self.conn.lock(); let n = conn.execute( - "UPDATE pending_op SET next_attempt_at = 0 - WHERE id = ?1 AND kind IN (?4, ?5) AND next_attempt_at < ?6 - AND (parent_uid IS NOT ?2 OR name IS NOT ?3)", + &format!( + "UPDATE pending_op SET next_attempt_at = 0 + WHERE id = ?1 AND kind IN (?4, ?5) AND next_attempt_at < ?6 + AND ({PARENT_NOW} IS NOT ?2 OR name IS NOT ?3)" + ), params![id, parent_uid, name, OP_CREATE, OP_MKDIR, PARK_UNTIL], )?; Ok(n > 0) diff --git a/crates/pdfs-core/src/db/pins.rs b/crates/pdfs-core/src/db/pins.rs index 0b201db..27bfc9e 100644 --- a/crates/pdfs-core/src/db/pins.rs +++ b/crates/pdfs-core/src/db/pins.rs @@ -64,11 +64,11 @@ impl Db { } /// Whether `uid` is pinned — directly, or because a strict ancestor folder - /// is pinned recursively. The ancestor check walks `parent_uid` to the root + /// is pinned recursively. The ancestor check walks `parent_lid` to the root /// via a CTE; a direct pin is honoured even when the node has no `nodes` row /// yet (e.g. a CLI that never hydrates the node cache). /// - /// The walk is depth-capped like [`super::utils::path_of`]: a `parent_uid` + /// The walk is depth-capped like [`super::utils::path_of`]: a parent /// cycle is corrupt data the API can hand us, and `UNION ALL` over one never /// terminates — while holding the daemon's only SQLite connection, on a path /// that runs per cached read. @@ -85,11 +85,11 @@ impl Db { let anc: Option = conn .query_row( &format!( - "WITH RECURSIVE anc(uid, parent_uid, depth) AS ( - SELECT uid, parent_uid, 0 FROM nodes WHERE uid = ?1 + "WITH RECURSIVE anc(uid, parent_lid, depth) AS ( + SELECT uid, parent_lid, 0 FROM nodes WHERE uid = ?1 UNION ALL - SELECT n.uid, n.parent_uid, anc.depth + 1 - FROM nodes n JOIN anc ON n.uid = anc.parent_uid + SELECT n.uid, n.parent_lid, anc.depth + 1 + FROM nodes n JOIN anc ON n.lid = anc.parent_lid WHERE anc.depth < {MAX_PATH_DEPTH} ) SELECT 1 FROM anc a JOIN pins p ON p.uid = a.uid @@ -109,10 +109,11 @@ impl Db { pub fn pinned_uids(&self) -> Result> { let conn = self.read(); let mut stmt = conn.prepare( - "WITH RECURSIVE sub(uid) AS ( - SELECT uid FROM pins WHERE recursive = 1 + "WITH RECURSIVE sub(uid, lid) AS ( + SELECT p.uid, n.lid FROM pins p LEFT JOIN nodes n ON n.uid = p.uid + WHERE p.recursive = 1 UNION - SELECT n.uid FROM nodes n JOIN sub ON n.parent_uid = sub.uid + SELECT n.uid, n.lid FROM nodes n JOIN sub ON n.parent_lid = sub.lid ) SELECT uid FROM sub UNION @@ -126,15 +127,16 @@ impl Db { Ok(out) } - /// Every descendant uid of `folder` (all depths), via a `parent_uid` CTE. + /// Every descendant uid of `folder` (all depths), via a `parent_lid` CTE. /// Used to evict a recursively-pinned subtree's cached blobs on unpin. pub fn descendants(&self, folder: &str) -> Result> { let conn = self.read(); let mut stmt = conn.prepare( - "WITH RECURSIVE sub(uid) AS ( - SELECT uid FROM nodes WHERE parent_uid = ?1 + "WITH RECURSIVE sub(uid, lid) AS ( + SELECT c.uid, c.lid FROM nodes c JOIN nodes f ON c.parent_lid = f.lid + WHERE f.uid = ?1 UNION - SELECT n.uid FROM nodes n JOIN sub ON n.parent_uid = sub.uid + SELECT n.uid, n.lid FROM nodes n JOIN sub ON n.parent_lid = sub.lid ) SELECT uid FROM sub", )?; diff --git a/crates/pdfs-core/src/db/share_access.rs b/crates/pdfs-core/src/db/share_access.rs index 6779856..9ec0ede 100644 --- a/crates/pdfs-core/src/db/share_access.rs +++ b/crates/pdfs-core/src/db/share_access.rs @@ -107,18 +107,19 @@ impl Db { } let value = conn .query_row( - "WITH RECURSIVE ancestors(uid, parent_uid, depth, path) AS ( - SELECT uid, parent_uid, 0, char(31) || uid || char(31) + "WITH RECURSIVE ancestors(uid, parent_uid, parent_lid, depth, path) AS ( + SELECT uid, parent_uid, parent_lid, 0, char(31) || uid || char(31) FROM nodes WHERE uid = ?1 UNION ALL - SELECT a.parent_uid, n.parent_uid, a.depth + 1, - a.path || a.parent_uid || char(31) + SELECT COALESCE(n.uid, a.parent_uid), n.parent_uid, n.parent_lid, + a.depth + 1, + a.path || COALESCE(n.uid, a.parent_uid) || char(31) FROM ancestors a - LEFT JOIN nodes n ON n.uid = a.parent_uid + LEFT JOIN nodes n ON n.lid = a.parent_lid WHERE a.parent_uid IS NOT NULL AND instr( a.path, - char(31) || a.parent_uid || char(31) + char(31) || COALESCE(n.uid, a.parent_uid) || char(31) ) = 0 ) SELECT sa.access diff --git a/crates/pdfs-core/src/db/tests.rs b/crates/pdfs-core/src/db/tests.rs index 580694c..5567709 100644 --- a/crates/pdfs-core/src/db/tests.rs +++ b/crates/pdfs-core/src/db/tests.rs @@ -3900,6 +3900,104 @@ fn a_landed_folder_hands_its_children_over_in_the_stored_node_too() { assert_eq!(names, ["a.txt"]); } +/// A folder made here that lands takes its real uid in its own row only. What +/// was made inside it names it by local id, and is found under the real uid. +#[test] +fn a_landed_folder_rewrites_no_row_or_op_below_it() { + let db = Db::open_in_memory().unwrap(); + db.upsert_node(&folder("root", None, "My Files")).unwrap(); + let root = uid("root").to_string(); + let dir_lid = db.add_local_row(&root, "dir", true, 1).unwrap(); + let local_dir = local_uid(dir_lid); + let mut dir = folder("x", Some("root"), "dir"); + dir.uid = NodeUid::new(VolumeId::from("local"), LinkId::from(dir_lid.to_string())); + db.upsert_node(&dir).unwrap(); + let child_lid = db.add_local_row(&local_dir, "a.txt", false, 1).unwrap(); + let local_child = local_uid(child_lid); + let mut child = file("y", "x", "a.txt", 3); + child.uid = NodeUid::new(VolumeId::from("local"), LinkId::from(child_lid.to_string())); + child.parent_uid = Some(dir.uid.clone()); + db.upsert_node(&child).unwrap(); + db.enqueue_op(&op_on(OP_CREATE, &local_child, Some(&local_dir))) + .unwrap(); + + let real_dir = uid("dir").to_string(); + db.land_placeholder_row(&local_dir, &real_dir).unwrap(); + + let (stored_parent, op_parent): (String, String) = { + let conn = db.conn.lock(); + ( + conn.query_row( + "SELECT parent_uid FROM nodes WHERE lid = ?1", + [child_lid], + |r| r.get(0), + ) + .unwrap(), + conn.query_row("SELECT parent_uid FROM pending_op", [], |r| r.get(0)) + .unwrap(), + ) + }; + assert_eq!( + (stored_parent.as_str(), op_parent.as_str()), + (local_dir.as_str(), local_dir.as_str()), + "the child's row and op are not rewritten" + ); + + let children = db.known_children(&uid("dir")).unwrap(); + assert_eq!(children.len(), 1); + assert_eq!(children[0].parent_uid, Some(uid("dir"))); + assert!(db.has_children(&uid("dir")).unwrap()); + assert_eq!( + db.node_path(&local_child).unwrap().as_deref(), + Some("dir/a.txt") + ); + assert_eq!( + db.pending_ops().unwrap()[0].parent_uid.as_deref(), + Some(real_dir.as_str()), + "the drain sends the folder's real uid" + ); + + db.delete_ops_for_uid(&real_dir).unwrap(); + assert!( + db.pending_ops().unwrap().is_empty(), + "removing the folder takes the create made inside it" + ); +} + +/// A create queued inside a folder made here is sent under the folder's real +/// uid once it lands. Landing where it was sent is no move. +#[test] +fn a_create_sent_into_a_landed_folder_lands_where_it_was_queued() { + let db = Db::open_in_memory().unwrap(); + db.upsert_node(&folder("root", None, "My Files")).unwrap(); + let dir_lid = db + .add_local_row(&uid("root").to_string(), "dir", true, 1) + .unwrap(); + let local_dir = local_uid(dir_lid); + let child_lid = db.add_local_row(&local_dir, "new.txt", false, 1).unwrap(); + let local_child = local_uid(child_lid); + let (id, _) = db + .enqueue_op(&op_on(OP_CREATE, &local_child, Some(&local_dir))) + .unwrap(); + let real_dir = uid("dir").to_string(); + db.land_placeholder_row(&local_dir, &real_dir).unwrap(); + + let claimed = db.claim_next_due_op(1).unwrap().unwrap(); + assert_eq!(claimed.parent_uid.as_deref(), Some(real_dir.as_str())); + let real = uid("new").to_string(); + db.finish_create( + id, + Some("/staging/create"), + &landing(&local_child, &real, &real_dir, "new.txt"), + |_| None, + ) + .unwrap(); + assert!( + db.pending_ops().unwrap().is_empty(), + "no rename is queued for a file that landed where it was sent" + ); +} + #[test] fn migration_v36_gives_every_node_its_rowid_as_lid() { let path = std::env::temp_dir().join(format!( diff --git a/crates/pdfs-core/src/db/utils.rs b/crates/pdfs-core/src/db/utils.rs index c8500b5..eacc390 100644 --- a/crates/pdfs-core/src/db/utils.rs +++ b/crates/pdfs-core/src/db/utils.rs @@ -72,18 +72,18 @@ pub(super) fn path_of(conn: &Connection, uid: &str) -> Result { } } -/// Resolve a path by walking `parent_uid` to the root via a recursive CTE. +/// Resolve a path by walking `parent_lid` to the root via a recursive CTE. /// -/// A cycle in `parent_uid` is corrupt data the API can still hand us, and +/// A cycle in the parents is corrupt data the API can still hand us, and /// `UNION ALL` over one never terminates — the walk is therefore depth-capped /// and returns the truncated path rather than hanging the caller. pub(super) fn walk_path_of(conn: &Connection, uid: &str) -> Result { let mut stmt = conn.prepare(&format!( - "WITH RECURSIVE anc(uid, parent_uid, name, depth) AS ( - SELECT uid, parent_uid, name, 0 FROM nodes WHERE uid = ?1 + "WITH RECURSIVE anc(parent_lid, parent_uid, name, depth) AS ( + SELECT parent_lid, parent_uid, name, 0 FROM nodes WHERE uid = ?1 UNION ALL - SELECT n.uid, n.parent_uid, n.name, anc.depth + 1 - FROM nodes n JOIN anc ON n.uid = anc.parent_uid + SELECT n.parent_lid, n.parent_uid, n.name, anc.depth + 1 + FROM nodes n JOIN anc ON n.lid = anc.parent_lid WHERE anc.depth < {MAX_PATH_DEPTH} ) SELECT name, parent_uid FROM anc ORDER BY depth DESC" diff --git a/docs/MILESTONE-3.0.0-STATUS.md b/docs/MILESTONE-3.0.0-STATUS.md index 310e2de..e2b44c3 100644 --- a/docs/MILESTONE-3.0.0-STATUS.md +++ b/docs/MILESTONE-3.0.0-STATUS.md @@ -34,17 +34,20 @@ Tracks `docs/MILESTONE-3.0.0.md`. Update it with every commit that moves a phase through `parent_lid`, and a landing folder rewrites the ops inside it by local id. - [x] Node rows link to their parent by lid (`nodes.parent_lid`, schema 40), and a restart places each node under its parent's row as it is now. - - [ ] Queries over children and subtrees by `parent_lid`, and no rewrite of the children's - `parent_uid` when a folder lands (R3). Both go with the placeholders: until then the - strings are kept in step. - - [ ] No more placeholders: `nodes.uid` is `NULL` until a node lands. In four parts: + - [x] Queries over children, subtrees and ancestors by `parent_lid`, and no rewrite of the + children's `parent_uid` when a folder lands (R3), nor of the ops made inside it (R2). + - [ ] No more placeholders: a node made here goes by its local id until it lands. In four + parts: - [x] 6a: a node made here goes by `local~`, a function of its row, instead of a minted `local~-`; schema 41 rewrites the old placeholders (nodes, ops, pins). - [x] 6b: an op is found by its node's local id under either uid, the claim keeps one op per local id on the wire, and the drain sends the uid the row has now; `remap_local_uid` is gone. - - [ ] 6c: a local node's row has `uid NULL`; children, subtrees and ops' parents by lid, - so a landing create rewrites no other row or op; pins by lid. + - [x] 6c: children, subtrees, ancestors and ops' parents by lid, so a landing create + rewrites no other row or op; what is read for a uid names the parent as its row has it + now. The row keeps its stand-in in `nodes.uid` rather than `NULL`: it is as stable, + and `NULL` would have changed some 40 node queries for no change in behaviour. Pins + stay keyed by uid and move with the row in the landing transaction (B148). - [ ] 6d: in memory, `pending`, `hidden` and `creating` by lid; `is_local_uid` checks become `remote_uid().is_none()`; a write or rename after landing queues under the stand-in, so `landed_uid`, `follow_landed_create`, `landed_row_uid` and the