core, desktop: Db::conn everywhere, Change::default, notes_where, and shared row mappings

From the audit (#5179, core and desktop half).

- Every `db.0.lock().map_err(|e| e.to_string())?` (about 50 sites in core and
  the desktop) is now `db.conn()?`. The few sites that deliberately handle
  a poisoned lock differently, and the tests, keep their own spelling.
- push::Change derives Default, so its four constructors name only the
  fields they set.
- store: list_notes, reminders, titles and search share notes_where (ids
  from a query, each loaded through load_note). Labels share
  LABEL_SELECT/label_row, and saved filters share
  SAVED_FILTER_SELECT/saved_filter_row.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

core/src/local/mod.rs

@@ -26,12 +26,9 @@ pub struct Db(pub Mutex<Connection>);
 impl Db {
     /// Lock the store, reporting a poisoned lock as a message rather than a panic.
     ///
-    /// Every consumer was writing `db.0.lock().map_err(|e| e.to_string())?` at each
-    /// call site. Beyond the repetition, that spelling forces the caller to NAME
-    /// `rusqlite::Connection` in any helper that returns the guard — which would make
-    /// rusqlite a dependency of a layer whose whole point is not to know what the
-    /// store is made of. Returning it from here means callers can bind the guard by
-    /// inference and never name the type.
+    /// The one way every caller takes the lock. Returning the guard from here lets a
+    /// caller bind it by inference without naming `rusqlite::Connection`, so layers
+    /// above the store need not depend on what it is made of.
     ///
     /// A poisoned lock means some earlier call panicked while holding it. The store
     /// is not necessarily corrupt, but this connection can't be trusted blind, so it

core/src/local/store.rs

@@ -8,7 +8,7 @@
 //! up with the values the frontend sends.
 
 use chrono::{DateTime, Duration, SecondsFormat, Utc};
-use rusqlite::{params, params_from_iter, Connection, OptionalExtension};
+use rusqlite::{params, params_from_iter, Connection, OptionalExtension, Params};
 use serde_json::{json, Value};
 use uuid::Uuid;
 
@@ -439,9 +439,16 @@ pub fn list_notes(conn: &Connection, q: &ListQuery) -> rusqlite::Result<Vec<Note
         " ORDER BY pinned DESC, position DESC, updated_at DESC"
     });
 
+    notes_where(conn, &sql, params_from_iter(binds.iter()))
+}
+
+/// The notes whose ids `sql` selects, in its order, each loaded in full. Every list
+/// the UI reads goes through `load_note`, so a list and the note it opens can never
+/// disagree about what the note is.
+fn notes_where<P: Params>(conn: &Connection, sql: &str, params: P) -> rusqlite::Result<Vec<Note>> {
     let ids: Vec<String> = {
-        let mut stmt = conn.prepare(&sql)?;
-        let rows = stmt.query_map(params_from_iter(binds.iter()), |r| r.get::<_, String>(0))?;
+        let mut stmt = conn.prepare(sql)?;
+        let rows = stmt.query_map(params, |r| r.get::<_, String>(0))?;
         rows.collect::<rusqlite::Result<Vec<String>>>()?
     };
     ids.iter().map(|id| load_note(conn, id)).collect()
@@ -452,15 +459,14 @@ pub fn get_note(conn: &Connection, id: &str) -> rusqlite::Result<Note> {
 }
 
 pub fn reminders(conn: &Connection) -> rusqlite::Result<Vec<Note>> {
-    let ids: Vec<String> = {
-        let mut stmt =
-            // The owner's reminders: a note shared with us neither alerts us nor is ours
-            // to clear, the same as on the server.
-            conn.prepare("SELECT id FROM notes WHERE trashed = 0 AND remind_at IS NOT NULL AND permission = 'owner' ORDER BY remind_at ASC")?;
-        let rows = stmt.query_map([], |r| r.get::<_, String>(0))?;
-        rows.collect::<rusqlite::Result<Vec<String>>>()?
-    };
-    ids.iter().map(|id| load_note(conn, id)).collect()
+    // The owner's reminders: a note shared with us neither alerts us nor is ours to
+    // clear, the same as on the server.
+    notes_where(
+        conn,
+        "SELECT id FROM notes WHERE trashed = 0 AND remind_at IS NOT NULL AND permission = 'owner'
+          ORDER BY remind_at ASC",
+        [],
+    )
 }
 
 /// Reminders due at or before `now_ms`, soonest first, trash excluded.
@@ -501,35 +507,24 @@ pub fn due_reminders(conn: &Connection, now_ms: i64) -> rusqlite::Result<Vec<Due
 }
 
 pub fn titles(conn: &Connection) -> rusqlite::Result<Vec<TitleEntry>> {
-    // Names come from `load_note` so the palette and the card can never disagree
-    // about what a note is called. The command palette reads this; correctness
-    // beats one query per note at personal scale.
-    let ids: Vec<String> = {
-        let mut stmt = conn.prepare("SELECT id FROM notes WHERE trashed = 0")?;
-        let rows = stmt.query_map([], |r| r.get(0))?;
-        rows.collect::<rusqlite::Result<Vec<String>>>()?
-    };
-    ids.iter()
-        .map(|id| {
-            let note = load_note(conn, id)?;
-            Ok(TitleEntry {
-                id: note.id,
-                title: note.display_title,
-            })
+    // The command palette reads this; one query per note is fine at personal scale.
+    let notes = notes_where(conn, "SELECT id FROM notes WHERE trashed = 0", [])?;
+    Ok(notes
+        .into_iter()
+        .map(|note| TitleEntry {
+            id: note.id,
+            title: note.display_title,
         })
-        .collect()
+        .collect())
 }
 
 pub fn search(conn: &Connection, q: &str) -> rusqlite::Result<Vec<Note>> {
     let pat = format!("%{}%", escape_like(q));
-    let ids: Vec<String> = {
-        let mut stmt = conn.prepare(
-            "SELECT id FROM notes WHERE trashed = 0 AND body LIKE ?1 ESCAPE '\\' ORDER BY updated_at DESC",
-        )?;
-        let rows = stmt.query_map([&pat], |r| r.get::<_, String>(0))?;
-        rows.collect::<rusqlite::Result<Vec<String>>>()?
-    };
-    ids.iter().map(|id| load_note(conn, id)).collect()
+    notes_where(
+        conn,
+        "SELECT id FROM notes WHERE trashed = 0 AND body LIKE ?1 ESCAPE '\\' ORDER BY updated_at DESC",
+        [&pat],
+    )
 }
 
 // ---- notes: write -----------------------------------------------------------
@@ -1031,39 +1026,28 @@ pub fn restore_revision(conn: &Connection, id: &str, rev_id: &str) -> rusqlite::
 
 // ---- labels -----------------------------------------------------------------
 
+/// A label with how many live notes carry it; `label_row` reads it.
+const LABEL_SELECT: &str = "SELECT l.id, l.name, l.color,
+        (SELECT COUNT(*) FROM note_labels nl JOIN notes n ON n.id = nl.note_id
+         WHERE nl.label_id = l.id AND n.trashed = 0)
+     FROM labels l";
+
+fn label_row(r: &rusqlite::Row<'_>) -> rusqlite::Result<Label> {
+    Ok(Label {
+        id: r.get(0)?,
+        name: r.get(1)?,
+        color: r.get(2)?,
+        count: Some(r.get(3)?),
+    })
+}
+
 fn load_label(conn: &Connection, id: &str) -> rusqlite::Result<Label> {
-    conn.query_row(
-        "SELECT l.id, l.name, l.color,
-                (SELECT COUNT(*) FROM note_labels nl JOIN notes n ON n.id = nl.note_id
-                 WHERE nl.label_id = l.id AND n.trashed = 0)
-         FROM labels l WHERE l.id = ?1",
-        [id],
-        |r| {
-            Ok(Label {
-                id: r.get(0)?,
-                name: r.get(1)?,
-                color: r.get(2)?,
-                count: Some(r.get(3)?),
-            })
-        },
-    )
+    conn.query_row(&format!("{LABEL_SELECT} WHERE l.id = ?1"), [id], label_row)
 }
 
 pub fn list_labels(conn: &Connection) -> rusqlite::Result<Vec<Label>> {
-    let mut stmt = conn.prepare(
-        "SELECT l.id, l.name, l.color,
-                (SELECT COUNT(*) FROM note_labels nl JOIN notes n ON n.id = nl.note_id
-                 WHERE nl.label_id = l.id AND n.trashed = 0)
-         FROM labels l ORDER BY l.name COLLATE NOCASE",
-    )?;
-    let rows = stmt.query_map([], |r| {
-        Ok(Label {
-            id: r.get(0)?,
-            name: r.get(1)?,
-            color: r.get(2)?,
-            count: Some(r.get(3)?),
-        })
-    })?;
+    let mut stmt = conn.prepare(&format!("{LABEL_SELECT} ORDER BY l.name COLLATE NOCASE"))?;
+    let rows = stmt.query_map([], label_row)?;
     rows.collect()
 }
 
@@ -1170,19 +1154,25 @@ pub fn merge_labels(
 
 // ---- saved filters ----------------------------------------------------------
 
+const SAVED_FILTER_SELECT: &str = "SELECT id, name, params, position FROM saved_filters";
+
+/// A stored filter. Params that no longer parse read as no filter at all rather
+/// than failing the whole list.
+fn saved_filter_row(r: &rusqlite::Row<'_>) -> rusqlite::Result<SavedFilter> {
+    let params_str: String = r.get(2)?;
+    Ok(SavedFilter {
+        id: r.get(0)?,
+        name: r.get(1)?,
+        params: serde_json::from_str(&params_str).unwrap_or_else(|_| json!({})),
+        position: r.get(3)?,
+    })
+}
+
 pub fn list_saved_filters(conn: &Connection) -> rusqlite::Result<Vec<SavedFilter>> {
-    let mut stmt =
-        conn.prepare("SELECT id, name, params, position FROM saved_filters ORDER BY position ASC, name COLLATE NOCASE")?;
-    let rows = stmt.query_map([], |r| {
-        let params_str: String = r.get(2)?;
-        let params = serde_json::from_str(&params_str).unwrap_or_else(|_| serde_json::json!({}));
-        Ok(SavedFilter {
-            id: r.get(0)?,
-            name: r.get(1)?,
-            params,
-            position: r.get(3)?,
-        })
-    })?;
+    let mut stmt = conn.prepare(&format!(
+        "{SAVED_FILTER_SELECT} ORDER BY position ASC, name COLLATE NOCASE"
+    ))?;
+    let rows = stmt.query_map([], saved_filter_row)?;
     rows.collect()
 }
 
@@ -1225,19 +1215,9 @@ pub fn rename_saved_filter(
         params![name, id],
     )?;
     conn.query_row(
-        "SELECT id, name, params, position FROM saved_filters WHERE id = ?1",
+        &format!("{SAVED_FILTER_SELECT} WHERE id = ?1"),
         [id],
-        |r| {
-            let params_str: String = r.get(2)?;
-            let params =
-                serde_json::from_str(&params_str).unwrap_or_else(|_| serde_json::json!({}));
-            Ok(SavedFilter {
-                id: r.get(0)?,
-                name: r.get(1)?,
-                params,
-                position: r.get(3)?,
-            })
-        },
+        saved_filter_row,
     )
 }
 

core/src/sync/engine.rs

@@ -67,7 +67,7 @@ pub async fn run_cycle(
         } else {
             state::SHARES
         };
-        let conn = db.0.lock().map_err(|e| e.to_string())?;
+        let conn = db.conn()?;
         if state::begin_shares(&conn, level).map_err(|e| e.to_string())? {
             log::info!("the server shares more of notes now; pulling everything once");
         }
@@ -91,7 +91,7 @@ pub async fn run_cycle(
     let retention = server.and_then(|s| s.trash_retention_days);
 
     let status = {
-        let conn = db.0.lock().map_err(|e| e.to_string())?;
+        let conn = db.conn()?;
         if let Some(days) = retention {
             state::set_server_retention(&conn, days as i64).map_err(|e| e.to_string())?;
         }

core/src/sync/pull.rs

@@ -79,7 +79,7 @@ async fn download_missing_blobs(
     token: &str,
 ) -> Result<(usize, usize), String> {
     let wanted = {
-        let conn = db.0.lock().map_err(|e| e.to_string())?;
+        let conn = db.conn()?;
         hashed_attachments(&conn).map_err(|e| e.to_string())?
     };
 
@@ -446,7 +446,7 @@ pub async fn run(
 
     loop {
         let since = {
-            let conn = db.0.lock().map_err(|e| e.to_string())?;
+            let conn = db.conn()?;
             state::read(&conn).map_err(|e| e.to_string())?.last_cursor
         };
 
@@ -463,7 +463,7 @@ pub async fn run(
 
         let has_more = page.has_more;
         let applied = {
-            let conn = db.0.lock().map_err(|e| e.to_string())?;
+            let conn = db.conn()?;
             apply_page(&conn, &page).map_err(|e| e.to_string())?
         };
         total.absorb(applied);

core/src/sync/push.rs

@@ -64,7 +64,7 @@ impl PushSummary {
 /// One entry in the `changes` array. Notes and labels share the envelope; serde skips
 /// the fields that don't apply, so the server sees exactly the shape docs/sync.md
 /// describes for each entity.
-#[derive(Debug, Serialize)]
+#[derive(Debug, Default, Serialize)]
 pub struct Change {
     pub entity: &'static str,
     pub id: String,
@@ -110,18 +110,7 @@ impl Change {
             id,
             op: "delete",
             edited_at,
-            body: None,
-            color: None,
-            pinned: None,
-            archived: None,
-            trashed: None,
-            remind_at: None,
-            recurrence: None,
-            position: None,
-            label_ids: None,
-            created_at: None,
-            name: None,
-            state_at: None,
+            ..Default::default()
         }
     }
 }
@@ -237,16 +226,7 @@ fn collect_labels(conn: &Connection, out: &mut Vec<Change>, limit: usize) -> rus
             name: Some(r.get(1)?),
             color: Some(r.get(2)?),
             edited_at: r.get(3)?,
-            body: None,
-            pinned: None,
-            archived: None,
-            trashed: None,
-            remind_at: None,
-            recurrence: None,
-            position: None,
-            label_ids: None,
-            created_at: None,
-            state_at: None,
+            ..Default::default()
         })
     })?;
     for row in rows {
@@ -342,17 +322,11 @@ fn note_change(conn: &Connection, id: &str, shared_state: bool) -> rusqlite::Res
             op: "upsert",
             edited_at: row.updated_at,
             body: (row.permission == "edit").then_some(row.body),
-            color: None,
             pinned: own.then_some(row.pinned),
             archived: own.then_some(row.archived),
-            trashed: None,
-            remind_at: None,
-            recurrence: None,
             position: own.then_some(row.position),
-            label_ids: None,
-            created_at: None,
-            name: None,
             state_at,
+            ..Default::default()
         });
     }
 
@@ -374,8 +348,6 @@ fn note_change(conn: &Connection, id: &str, shared_state: bool) -> rusqlite::Res
         // server's last-write-wins comparison runs against.
         edited_at: row.updated_at,
         body: Some(row.body),
-        // A note has no colour to send. See the field on `Change`.
-        color: None,
         pinned: Some(row.pinned),
         archived: Some(row.archived),
         trashed: Some(row.trashed),
@@ -384,8 +356,7 @@ fn note_change(conn: &Connection, id: &str, shared_state: bool) -> rusqlite::Res
         position: Some(row.position),
         label_ids: Some(label_ids),
         created_at: Some(row.created_at),
-        name: None,
-        state_at: None,
+        ..Default::default()
     })
 }
 
@@ -610,7 +581,7 @@ async fn upload_pending(
     token: &str,
 ) -> Result<PushSummary, String> {
     let wanted = {
-        let conn = db.0.lock().map_err(|e| e.to_string())?;
+        let conn = db.conn()?;
         pending_uploads(&conn).map_err(|e| e.to_string())?
     };
     let mut summary = PushSummary::default();
@@ -636,7 +607,7 @@ async fn upload_pending(
             )),
         };
         {
-            let conn = db.0.lock().map_err(|e| e.to_string())?;
+            let conn = db.conn()?;
             settle_upload(&conn, &upload.id, &result).map_err(|e| e.to_string())?;
         }
         match result {
@@ -669,7 +640,7 @@ pub async fn run(
 
     loop {
         let batch = {
-            let conn = db.0.lock().map_err(|e| e.to_string())?;
+            let conn = db.conn()?;
             collect(&conn, BATCH, accepts).map_err(|e| e.to_string())?
         };
         if batch.is_empty() {
@@ -680,7 +651,7 @@ pub async fn run(
         let results = parse_results(&raw)?;
 
         let applied = {
-            let conn = db.0.lock().map_err(|e| e.to_string())?;
+            let conn = db.conn()?;
             apply_results(&conn, &batch, &results).map_err(|e| e.to_string())?
         };
         // Everything rejected clears nothing, so the same batch would be collected

core/src/sync/sharing.rs

@@ -18,7 +18,7 @@ pub const NEEDS_SERVER: &str =
     "Sharing is between people on a server. Link this device to one in Sync to share notes.";
 
 fn link(db: &Db) -> Result<(String, String), String> {
-    let conn = db.0.lock().map_err(|e| e.to_string())?;
+    let conn = db.conn()?;
     let link = state::read(&conn).map_err(|e| e.to_string())?;
     match (link.server_url, link.device_token) {
         (Some(url), Some(token)) => Ok((url, token)),
@@ -29,7 +29,7 @@ fn link(db: &Db) -> Result<(String, String), String> {
 /// Set the local note's `shared` flag from the server's list of its shares. Not a
 /// local edit, so it leaves `dirty` and `updated_at` alone.
 fn mark_shared(db: &Db, note_id: &str, shares: &[NoteShare]) -> Result<(), String> {
-    let conn = db.0.lock().map_err(|e| e.to_string())?;
+    let conn = db.conn()?;
     conn.execute(
         "UPDATE notes SET shared = ?2 WHERE id = ?1 AND permission = 'owner'",
         params![note_id, !shares.is_empty()],