fix(library): Navidrome artist letter buckets + safer index open/swap (#1144) (#1145)

This commit is contained in:
cucadmuh
2026-06-21 20:55:35 +03:00
committed by GitHub
parent 986550c854
commit d9969ed76f
26 changed files with 1088 additions and 115 deletions
@@ -0,0 +1,6 @@
-- Artist browse sort key (Navidrome OrderArtistName parity) + server ignoredArticles watermark.
-- Applied idempotently from store.rs `apply_migration_14` (per-column guard) so a
-- partial apply recovers; keep this file in sync as the canonical DDL.
ALTER TABLE artist ADD COLUMN name_sort TEXT;
ALTER TABLE sync_state ADD COLUMN ignored_articles TEXT;
CREATE INDEX IF NOT EXISTS idx_artist_name_sort ON artist(server_id, name_sort);
@@ -39,7 +39,7 @@ const ALBUM_COLUMNS: &str = "a.server_id, a.id, a.name, a.artist, a.artist_id, \
WHERE t.server_id = a.server_id AND t.album_id = a.id AND t.deleted = 0)), \
a.genre, a.cover_art_id, a.starred_at, a.synced_at, a.raw_json";
const ARTIST_COLUMNS: &str = "ar.server_id, ar.id, ar.name, ar.album_count, \
const ARTIST_COLUMNS: &str = "ar.server_id, ar.id, ar.name, ar.name_sort, ar.album_count, \
ar.synced_at, ar.raw_json";
/// Flat track projection used when browsing albums in advanced search.
@@ -527,7 +527,9 @@ fn build_artist_from_table(
}
}
let order = order_clause(&req.sort, EntityKind::Artist)
.unwrap_or_else(|| "ORDER BY ar.name COLLATE NOCASE ASC, ar.id ASC".to_string());
.unwrap_or_else(|| {
"ORDER BY COALESCE(ar.name_sort, ar.name) COLLATE NOCASE ASC, ar.id ASC".to_string()
});
query_rows(
store,
ARTIST_COLUMNS,
@@ -794,10 +796,16 @@ fn build_artist_from_fts(
if !seen.insert(artist_id.clone()) {
continue;
}
let name = artist.unwrap_or_default();
let name_sort = crate::artist_sort::sort_key_for_display_name(
&name,
crate::artist_sort::DEFAULT_IGNORED_ARTICLES,
);
deduped.push(LibraryArtistDto {
server_id,
id: artist_id,
name: artist.unwrap_or_default(),
name,
name_sort: Some(name_sort),
album_count: None,
synced_at,
raw_json: Value::Null,
@@ -1179,13 +1187,14 @@ fn map_album(r: &rusqlite::Row<'_>) -> rusqlite::Result<LibraryAlbumDto> {
}
fn map_artist(r: &rusqlite::Row<'_>) -> rusqlite::Result<LibraryArtistDto> {
let raw: Option<String> = r.get(5)?;
let raw: Option<String> = r.get(6)?;
Ok(LibraryArtistDto {
server_id: r.get(0)?,
id: r.get(1)?,
name: r.get(2)?,
album_count: r.get(3)?,
synced_at: r.get(4)?,
name_sort: r.get(3)?,
album_count: r.get(4)?,
synced_at: r.get(5)?,
raw_json: parse_raw_json(raw),
})
}
@@ -1211,10 +1220,16 @@ fn map_album_from_tracks(r: &rusqlite::Row<'_>) -> rusqlite::Result<LibraryAlbum
}
fn map_artist_from_tracks(r: &rusqlite::Row<'_>) -> rusqlite::Result<LibraryArtistDto> {
let name: String = r.get(2)?;
let name_sort = crate::artist_sort::sort_key_for_display_name(
&name,
crate::artist_sort::DEFAULT_IGNORED_ARTICLES,
);
Ok(LibraryArtistDto {
server_id: r.get(0)?,
id: r.get(1)?,
name: r.get(2)?,
name,
name_sort: Some(name_sort),
album_count: Some(r.get(3)?),
synced_at: r.get(4)?,
raw_json: Value::Null,
@@ -1289,7 +1304,7 @@ fn sort_column(field: &str, entity: EntityKind) -> Option<&'static str> {
("name", EntityKind::Album) => Some("a.name COLLATE NOCASE"),
("year", EntityKind::Album) => Some("a.year"),
("artist", EntityKind::Album) => Some("a.artist COLLATE NOCASE"),
("name", EntityKind::Artist) => Some("ar.name COLLATE NOCASE"),
("name", EntityKind::Artist) => Some("COALESCE(ar.name_sort, ar.name) COLLATE NOCASE"),
// SQLite built-in: ORDER BY RANDOM() LIMIT N — fast pseudo-random sample,
// no index scan needed beyond the row-id range. Direction is ignored.
("random", _) => Some("RANDOM()"),
@@ -0,0 +1,63 @@
//! Display-name → sort/bucket key (Navidrome `SanitizeFieldForSortingNoArticle` subset).
/// Navidrome default (`utils/str/sanitize_strings_test.go`).
pub const DEFAULT_IGNORED_ARTICLES: &str = "The El La Los Las Le Les Os As O A";
/// Strip leading articles from a display name (case-insensitive article match).
pub fn strip_leading_articles(name: &str, ignored_articles: &str) -> String {
let trimmed = name.trim();
for article in ignored_articles.split(' ').filter(|s| !s.is_empty()) {
let prefix = format!("{} ", article);
if trimmed.len() >= prefix.len()
&& trimmed[..prefix.len()].eq_ignore_ascii_case(&prefix)
{
return trimmed[prefix.len()..].trim_start().to_string();
}
}
trimmed.to_string()
}
/// Lowercase sort key used for SQL `ORDER BY` and UI letter buckets.
pub fn sort_key_for_display_name(name: &str, ignored_articles: &str) -> String {
strip_leading_articles(name, ignored_articles).to_lowercase()
}
pub fn ignored_articles_or_default(ignored_articles: Option<&str>) -> &str {
match ignored_articles.map(str::trim).filter(|s| !s.is_empty()) {
Some(s) => s,
None => DEFAULT_IGNORED_ARTICLES,
}
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn strips_the_from_beatles() {
let key = sort_key_for_display_name("The Beatles", DEFAULT_IGNORED_ARTICLES);
assert_eq!(key, "beatles");
}
#[test]
fn strips_the_from_kinks() {
let key = sort_key_for_display_name("The Kinks", DEFAULT_IGNORED_ARTICLES);
assert_eq!(key, "kinks");
}
#[test]
fn leaves_non_article_names() {
assert_eq!(
sort_key_for_display_name("Adele", DEFAULT_IGNORED_ARTICLES),
"adele"
);
}
#[test]
fn custom_ignored_articles() {
assert_eq!(
sort_key_for_display_name("The Beatles", "The"),
"beatles"
);
}
}
@@ -180,8 +180,8 @@ pub async fn library_get_status(
conn.query_row(
"SELECT sync_phase, capability_flags, library_tier, last_full_sync_at, \
last_delta_sync_at, next_poll_at, server_last_scan_iso, \
indexes_last_modified_ms, artists_last_modified_ms, local_track_count, \
server_track_count, last_error \
indexes_last_modified_ms, artists_last_modified_ms, ignored_articles, \
local_track_count, server_track_count, last_error \
FROM sync_state WHERE server_id = ?1 AND library_scope = ?2",
params![server_id, scope],
|r| {
@@ -195,9 +195,10 @@ pub async fn library_get_status(
server_last_scan_iso: r.get(6)?,
indexes_last_modified_ms: r.get(7)?,
artists_last_modified_ms: r.get(8)?,
local_track_count: r.get(9)?,
server_track_count: r.get(10)?,
last_error: r.get(11)?,
ignored_articles: r.get(9)?,
local_track_count: r.get(10)?,
server_track_count: r.get(11)?,
last_error: r.get(12)?,
})
},
)
@@ -246,6 +247,7 @@ pub async fn library_get_status(
server_last_scan_iso: row.server_last_scan_iso,
indexes_last_modified_ms: row.indexes_last_modified_ms,
artists_last_modified_ms: row.artists_last_modified_ms,
ignored_articles: row.ignored_articles,
local_track_count,
server_track_count: row.server_track_count,
last_error: row.last_error,
@@ -622,6 +624,7 @@ struct SyncStateRow {
server_last_scan_iso: Option<String>,
indexes_last_modified_ms: Option<i64>,
artists_last_modified_ms: Option<i64>,
ignored_articles: Option<String>,
local_track_count: Option<i64>,
server_track_count: Option<i64>,
last_error: Option<String>,
@@ -28,6 +28,8 @@ pub struct SyncStateDto {
pub server_last_scan_iso: Option<String>,
pub indexes_last_modified_ms: Option<i64>,
pub artists_last_modified_ms: Option<i64>,
/// Space-separated leading articles from the server's `getArtists` response.
pub ignored_articles: Option<String>,
pub local_track_count: Option<i64>,
pub server_track_count: Option<i64>,
pub last_error: Option<String>,
@@ -476,6 +478,8 @@ pub struct LibraryArtistDto {
pub server_id: String,
pub id: String,
pub name: String,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub name_sort: Option<String>,
pub album_count: Option<i64>,
pub synced_at: i64,
pub raw_json: Value,
@@ -861,6 +865,7 @@ mod tests {
server_last_scan_iso: None,
indexes_last_modified_ms: None,
artists_last_modified_ms: None,
ignored_articles: None,
local_track_count: None,
server_track_count: None,
last_error: None,
@@ -17,6 +17,7 @@ pub mod analysis_backfill_policy;
pub mod library_readiness;
pub mod artist_artwork;
pub mod artist_lossless_browse;
pub mod artist_sort;
pub mod cover_backfill;
pub mod cover_resolve;
pub mod canonical;
@@ -193,6 +193,7 @@ fn query_artists(
server_id: r.get(0)?,
id: r.get(1)?,
name: r.get::<_, Option<String>>(2)?.unwrap_or_default(),
name_sort: None,
album_count: None,
synced_at: r.get(3)?,
raw_json: serde_json::Value::Null,
@@ -0,0 +1,198 @@
//! `artist` table — browse index rows from `getArtists` and track-derived backfill.
use rusqlite::{params, Transaction};
use crate::artist_sort::{ignored_articles_or_default, sort_key_for_display_name};
use crate::store::LibraryStore;
use psysonic_integration::subsonic::ArtistIndex;
pub struct ArtistRepository<'a> {
store: &'a LibraryStore,
}
impl<'a> ArtistRepository<'a> {
pub fn new(store: &'a LibraryStore) -> Self {
Self { store }
}
/// Upsert artists from a Subsonic `getArtists` / `getIndexes` body.
pub fn upsert_index(
&self,
server_id: &str,
index: &ArtistIndex,
synced_at: i64,
) -> Result<u32, String> {
let ignored = ignored_articles_or_default(index.ignored_articles.as_deref());
let mut count = 0u32;
self.store.with_conn_mut("artist.upsert_index", |conn| {
let tx = conn.transaction()?;
for bucket in &index.index {
for artist in &bucket.artist {
let name_sort = sort_key_for_display_name(&artist.name, ignored);
upsert_artist_row(
&tx,
server_id,
&artist.id,
&artist.name,
&name_sort,
artist.album_count,
synced_at,
)?;
count += 1;
}
}
tx.commit()?;
Ok(())
})?;
Ok(count)
}
/// Materialize missing `artist` rows from synced tracks (pre-pass backfill).
pub fn backfill_from_tracks(
&self,
server_id: &str,
ignored_articles: &str,
synced_at: i64,
) -> Result<u32, String> {
let rows: Vec<(String, String)> = self
.store
.with_read_conn(|conn| {
let mut stmt = conn.prepare(
"SELECT artist_id, MAX(artist) \
FROM track \
WHERE server_id = ?1 AND deleted = 0 \
AND artist_id IS NOT NULL AND artist_id != '' \
AND artist IS NOT NULL AND artist != '' \
AND NOT EXISTS ( \
SELECT 1 FROM artist ar \
WHERE ar.server_id = track.server_id AND ar.id = track.artist_id \
) \
GROUP BY artist_id",
)?;
let collected = stmt
.query_map(params![server_id], |row| {
Ok((row.get::<_, String>(0)?, row.get::<_, String>(1)?))
})?
.collect::<rusqlite::Result<Vec<_>>>()?;
Ok(collected)
})
.map_err(|e| e.to_string())?;
if rows.is_empty() {
return Ok(0);
}
let mut count = 0u32;
self.store.with_conn_mut("artist.backfill_from_tracks", |conn| {
let tx = conn.transaction()?;
for (id, name) in &rows {
let name_sort = sort_key_for_display_name(name, ignored_articles);
upsert_artist_row(&tx, server_id, id, name, &name_sort, None, synced_at)?;
count += 1;
}
tx.commit()?;
Ok(())
})?;
Ok(count)
}
/// One-time repair: fill `name_sort` where null (upgrade path).
pub fn backfill_null_name_sort(&self, ignored_articles: &str) -> Result<u32, String> {
let rows: Vec<(String, String, String)> = self
.store
.with_read_conn(|conn| {
let mut stmt =
conn.prepare("SELECT server_id, id, name FROM artist WHERE name_sort IS NULL")?;
let collected = stmt
.query_map([], |row| {
Ok((
row.get::<_, String>(0)?,
row.get::<_, String>(1)?,
row.get::<_, String>(2)?,
))
})?
.collect::<rusqlite::Result<Vec<_>>>()?;
Ok(collected)
})
.map_err(|e| e.to_string())?;
if rows.is_empty() {
return Ok(0);
}
let mut count = 0u32;
self.store.with_conn_mut("artist.backfill_null_name_sort", |conn| {
let tx = conn.transaction()?;
for (server_id, id, name) in &rows {
let name_sort = sort_key_for_display_name(name, ignored_articles);
tx.execute(
"UPDATE artist SET name_sort = ?1 WHERE server_id = ?2 AND id = ?3",
params![name_sort, server_id, id],
)?;
count += 1;
}
tx.commit()?;
Ok(())
})?;
Ok(count)
}
}
fn upsert_artist_row(
tx: &Transaction<'_>,
server_id: &str,
id: &str,
name: &str,
name_sort: &str,
album_count: Option<i64>,
synced_at: i64,
) -> rusqlite::Result<()> {
tx.execute(
"INSERT INTO artist (server_id, id, name, name_sort, album_count, synced_at) \
VALUES (?1, ?2, ?3, ?4, ?5, ?6) \
ON CONFLICT(server_id, id) DO UPDATE SET \
name = excluded.name, \
name_sort = excluded.name_sort, \
album_count = COALESCE(excluded.album_count, artist.album_count), \
synced_at = excluded.synced_at",
params![server_id, id, name, name_sort, album_count, synced_at],
)?;
Ok(())
}
#[cfg(test)]
mod tests {
use super::*;
use crate::store::LibraryStore;
use psysonic_integration::subsonic::{ArtistIndex, ArtistRef, IndexBucket};
#[test]
fn upsert_index_stores_name_sort_for_the_beatles() {
let store = LibraryStore::open_in_memory();
let repo = ArtistRepository::new(&store);
let index = ArtistIndex {
last_modified_ms: Some(1),
ignored_articles: Some("The".into()),
index: vec![IndexBucket {
name: "B".into(),
artist: vec![ArtistRef {
id: "ar_1".into(),
name: "The Beatles".into(),
album_count: Some(3),
cover_art: None,
}],
}],
};
repo.upsert_index("s1", &index, 1000).unwrap();
let name_sort: String = store
.with_conn("misc", |c| {
c.query_row(
"SELECT name_sort FROM artist WHERE server_id = 's1' AND id = 'ar_1'",
[],
|r| r.get(0),
)
})
.unwrap();
assert_eq!(name_sort, "beatles");
}
}
@@ -1,3 +1,4 @@
pub mod artist;
pub mod artifact;
pub mod fact;
pub mod play_session;
@@ -5,6 +6,7 @@ pub mod sync_state;
pub mod track;
pub mod track_id_history;
pub use artist::ArtistRepository;
pub use artifact::ArtifactRepository;
pub use fact::FactRepository;
pub use play_session::PlaySessionRepository;
@@ -561,6 +561,44 @@ impl<'a> SyncStateRepository<'a> {
})
}
/// Read `ignored_articles` from the last `getArtists` pass (Navidrome
/// `IgnoredArticles` string — space-separated article tokens).
pub fn get_ignored_articles(
&self,
server_id: &str,
library_scope: &str,
) -> Result<Option<String>, String> {
self.read(|conn| {
conn.query_row(
"SELECT ignored_articles FROM sync_state \
WHERE server_id = ?1 AND library_scope = ?2",
params![server_id, library_scope],
|row| row.get::<_, Option<String>>(0),
)
.optional()
})
.map(|opt| opt.flatten())
}
/// Persist server `ignoredArticles` for local artist sort-key computation.
pub fn set_ignored_articles(
&self,
server_id: &str,
library_scope: &str,
ignored_articles: &str,
) -> Result<(), String> {
self.store.with_conn("sync_state.set_ignored_articles", |conn| {
conn.execute(
"INSERT INTO sync_state (server_id, library_scope, ignored_articles) \
VALUES (?1, ?2, ?3) \
ON CONFLICT(server_id, library_scope) DO UPDATE SET \
ignored_articles = excluded.ignored_articles",
params![server_id, library_scope, ignored_articles],
)?;
Ok(())
})
}
/// Write `library_tier` (spec §6.2.2 — `small` / `medium` / `huge`
/// / `unknown`). Drives the adaptive poll interval; PR-3d wires
/// the EWMA loop that picks this.
+477 -58
View File
@@ -4,12 +4,18 @@ use std::sync::atomic::{AtomicBool, AtomicU64, Ordering};
use std::sync::Mutex;
use std::time::Duration;
use rusqlite::{params, Connection, OpenFlags};
use rusqlite::{params, Connection, OpenFlags, OptionalExtension};
use tauri::Manager;
/// Current head of the embedded migrations. Bump each time a new
/// `migrations/NNN_*.sql` is added.
pub const LIBRARY_DB_SCHEMA_VERSION: i64 = 13;
///
/// Migration checklist (wiring, data backfill, open/swap path):
/// psysonic-workdocs `ai/agent-rules/08-library-db-migrations.md`.
pub const LIBRARY_DB_SCHEMA_VERSION: i64 = 14;
/// One-time data repair after migration 014 (`artist.name_sort`).
pub(crate) const ARTIST_NAME_SORT_RECONCILE_ID: &str = "artist_name_sort_reconcile_v1";
/// Lowest applied schema version the current code can advance from purely
/// additively. If a DB carries a version below this, the breaking-bump hook
@@ -30,6 +36,8 @@ pub(crate) const MIGRATION_012_TRACK_GENRE_LEGACY: &str =
/// artwork (fanart.tv) — image-scraper §12. Pure CREATE TABLE IF NOT EXISTS.
pub(crate) const MIGRATION_013_ARTIST_ARTWORK_LOOKUP: &str =
include_str!("../migrations/013_artist_artwork_lookup.sql");
pub(crate) const MIGRATION_014_ARTIST_NAME_SORT: &str =
include_str!("../migrations/014_artist_name_sort.sql");
/// Embedded migrations. Ordered ascending by `version`; the runner sorts
/// defensively before applying so the source order can stay readable.
@@ -37,6 +45,7 @@ const MIGRATIONS: &[(i64, &str)] = &[
(1, INITIAL_SQL),
(12, MIGRATION_012_TRACK_GENRE_LEGACY),
(13, MIGRATION_013_ARTIST_ARTWORK_LOOKUP),
(14, MIGRATION_014_ARTIST_NAME_SORT),
];
/// Idempotent repair — also runs after the migration runner on every open so
@@ -70,6 +79,9 @@ pub struct LibraryStore {
read_conn: Mutex<Connection>,
/// IS-3 bulk ingest in progress — read paths skip write-lock work.
bulk_ingest_active: AtomicBool,
/// `swap_database_file` / `restore_database_backup` — fail fast instead of
/// touching in-memory placeholder connections while the file is offline.
swap_in_progress: AtomicBool,
}
impl LibraryStore {
@@ -83,10 +95,7 @@ impl LibraryStore {
fn open_file(db_path: &Path) -> Result<Self, String> {
let write_conn = Connection::open(db_path).map_err(|e| e.to_string())?;
configure_write_connection(&write_conn).map_err(|e| e.to_string())?;
run_migrations(&write_conn).map_err(|e| e.to_string())?;
ensure_genre_tags_schema(&write_conn).map_err(|e| e.to_string())?;
checkpoint_wal_conn(&write_conn, "open").map_err(|e| e.to_string())?;
prepare_write_connection_for_open(&write_conn).map_err(|e| e.to_string())?;
let read_conn = Connection::open_with_flags(db_path, OpenFlags::SQLITE_OPEN_READ_ONLY)
.map_err(|e| e.to_string())?;
configure_read_connection(&read_conn).map_err(|e| e.to_string())?;
@@ -94,6 +103,7 @@ impl LibraryStore {
write_conn: Mutex::new(write_conn),
read_conn: Mutex::new(read_conn),
bulk_ingest_active: AtomicBool::new(false),
swap_in_progress: AtomicBool::new(false),
})
}
@@ -102,14 +112,14 @@ impl LibraryStore {
let uri = in_memory_uri();
let write_conn = Connection::open(&uri).expect("in-memory write connection");
configure_write_connection(&write_conn).expect("write pragmas");
run_migrations(&write_conn).expect("schema migration");
ensure_genre_tags_schema(&write_conn).expect("genre tags schema");
prepare_write_connection_for_open(&write_conn).expect("schema migration");
let read_conn = Connection::open(&uri).expect("in-memory read connection");
configure_read_connection(&read_conn).expect("read pragmas");
Self {
write_conn: Mutex::new(write_conn),
read_conn: Mutex::new(read_conn),
bulk_ingest_active: AtomicBool::new(false),
swap_in_progress: AtomicBool::new(false),
}
}
@@ -122,6 +132,36 @@ impl LibraryStore {
self.bulk_ingest_active.load(Ordering::Acquire)
}
fn swap_in_progress(&self) -> bool {
self.swap_in_progress.load(Ordering::Acquire)
}
fn lock_write_conn(&self) -> Result<std::sync::MutexGuard<'_, Connection>, String> {
if self.swap_in_progress() {
return Err("library database swap in progress".to_string());
}
match self.write_conn.lock() {
Ok(guard) => Ok(guard),
Err(poisoned) => {
crate::app_eprintln!("[library-db] write lock was poisoned — recovering");
Ok(poisoned.into_inner())
}
}
}
fn lock_read_conn(&self) -> Result<std::sync::MutexGuard<'_, Connection>, String> {
if self.swap_in_progress() {
return Err("library database swap in progress".to_string());
}
match self.read_conn.lock() {
Ok(guard) => Ok(guard),
Err(poisoned) => {
crate::app_eprintln!("[library-db] read lock was poisoned — recovering");
Ok(poisoned.into_inner())
}
}
}
/// Writer connection — sync ingest, migrations, mutations.
///
/// `op` is logged on slow writes (`[library-db] SLOW write op=…`) — use a
@@ -134,13 +174,10 @@ impl LibraryStore {
f: impl FnOnce(&Connection) -> rusqlite::Result<R>,
) -> Result<R, String> {
let lock_start = std::time::Instant::now();
let conn = self
.write_conn
.lock()
.map_err(|_| "library store write lock poisoned".to_string())?;
let conn = self.lock_write_conn()?;
let lock_wait_ms = lock_start.elapsed().as_millis();
let exec_start = std::time::Instant::now();
let out = f(&conn).map_err(|e| e.to_string());
let out = run_conn_closure(&conn, f);
let exec_ms = exec_start.elapsed().as_millis();
log_write_op(op, lock_wait_ms, exec_ms);
out
@@ -151,11 +188,8 @@ impl LibraryStore {
&self,
f: impl FnOnce(&Connection) -> rusqlite::Result<R>,
) -> Result<R, String> {
let conn = self
.read_conn
.lock()
.map_err(|_| "library store read lock poisoned".to_string())?;
f(&conn).map_err(|e| e.to_string())
let conn = self.lock_read_conn()?;
run_conn_closure(&conn, f)
}
pub(crate) fn with_conn_mut<R>(
@@ -172,13 +206,10 @@ impl LibraryStore {
f: impl FnOnce(&mut Connection) -> rusqlite::Result<R>,
) -> Result<(R, WriteOpTiming), String> {
let lock_start = std::time::Instant::now();
let mut conn = self
.write_conn
.lock()
.map_err(|_| "library store write lock poisoned".to_string())?;
let mut conn = self.lock_write_conn()?;
let lock_wait_ms = lock_start.elapsed().as_millis() as u64;
let exec_start = std::time::Instant::now();
let out = f(&mut conn).map_err(|e| e.to_string())?;
let out = run_conn_mut_closure(&mut conn, f)?;
let exec_ms = exec_start.elapsed().as_millis() as u64;
log_write_op(op, lock_wait_ms as u128, exec_ms as u128);
Ok((out, WriteOpTiming { lock_wait_ms, exec_ms }))
@@ -192,8 +223,8 @@ impl LibraryStore {
}
/// Atomically switch the active sqlite file while replacing long-lived
/// write/read connections under the same locks so no command can keep
/// writing to the old inode after the swap.
/// write/read connections. Other threads see `library database swap in
/// progress` while the file is offline instead of touching placeholder DBs.
pub fn swap_database_file(
&self,
active_path: &Path,
@@ -202,14 +233,14 @@ impl LibraryStore {
if !destination_path.exists() {
return Ok(None);
}
let mut write_conn = self
.write_conn
.lock()
.map_err(|_| "library store write lock poisoned".to_string())?;
let mut read_conn = self
.read_conn
.lock()
.map_err(|_| "library store read lock poisoned".to_string())?;
let mut swap_guard = SwapInProgressGuard::new(self);
let mut write_conn = self.write_conn.lock().map_err(|_| {
"library store write lock poisoned during database swap".to_string()
})?;
let mut read_conn = self.read_conn.lock().map_err(|_| {
"library store read lock poisoned during database swap".to_string()
})?;
let write_tmp = Connection::open_in_memory().map_err(|e| e.to_string())?;
let read_tmp = Connection::open_in_memory().map_err(|e| e.to_string())?;
@@ -237,28 +268,68 @@ impl LibraryStore {
let _ = move_sidecar(&backup, active_path, "-wal");
let _ = move_sidecar(&backup, active_path, "-shm");
}
drop(read_conn);
drop(write_conn);
let (reopened_write, reopened_read) = open_database_connections(active_path)
.map_err(|e| format!("library swap reopen failed after rename error: {e}"))?;
let mut write_conn = self.write_conn.lock().map_err(|_| {
"library store write lock poisoned during database swap".to_string()
})?;
let mut read_conn = self.read_conn.lock().map_err(|_| {
"library store read lock poisoned during database swap".to_string()
})?;
*write_conn = reopened_write;
*read_conn = reopened_read;
swap_guard.release();
return Err(err.to_string());
}
let reopened_write = Connection::open(active_path).map_err(|e| e.to_string())?;
configure_write_connection(&reopened_write).map_err(|e| e.to_string())?;
let reopened_read = Connection::open_with_flags(active_path, OpenFlags::SQLITE_OPEN_READ_ONLY)
.map_err(|e| e.to_string())?;
configure_read_connection(&reopened_read).map_err(|e| e.to_string())?;
*write_conn = reopened_write;
*read_conn = reopened_read;
Ok(Some(backup))
drop(read_conn);
drop(write_conn);
let reopen = open_database_connections(active_path);
let mut write_conn = self.write_conn.lock().map_err(|_| {
"library store write lock poisoned during database swap".to_string()
})?;
let mut read_conn = self.read_conn.lock().map_err(|_| {
"library store read lock poisoned during database swap".to_string()
})?;
match reopen {
Ok((reopened_write, reopened_read)) => {
*write_conn = reopened_write;
*read_conn = reopened_read;
swap_guard.release();
Ok(Some(backup))
}
Err(open_err) => {
if backup.exists() {
if active_path.exists() {
remove_db_with_sidecars(active_path).ok();
}
let _ = fs::rename(&backup, active_path);
let _ = move_sidecar(&backup, active_path, "-wal");
let _ = move_sidecar(&backup, active_path, "-shm");
}
let (reopened_write, reopened_read) = open_database_connections(active_path)
.map_err(|e| format!("library swap reopen failed after revert: {e}"))?;
*write_conn = reopened_write;
*read_conn = reopened_read;
swap_guard.release();
Err(format!("library swap failed: {open_err}"))
}
}
}
pub fn restore_database_backup(&self, backup_path: &Path, active_path: &Path) -> Result<(), String> {
let mut write_conn = self
.write_conn
.lock()
.map_err(|_| "library store write lock poisoned".to_string())?;
let mut read_conn = self
.read_conn
.lock()
.map_err(|_| "library store read lock poisoned".to_string())?;
let mut swap_guard = SwapInProgressGuard::new(self);
let mut write_conn = self.write_conn.lock().map_err(|_| {
"library store write lock poisoned during database restore".to_string()
})?;
let mut read_conn = self.read_conn.lock().map_err(|_| {
"library store read lock poisoned during database restore".to_string()
})?;
let write_tmp = Connection::open_in_memory().map_err(|e| e.to_string())?;
let read_tmp = Connection::open_in_memory().map_err(|e| e.to_string())?;
@@ -276,13 +347,21 @@ impl LibraryStore {
move_sidecar(backup_path, active_path, "-shm")?;
}
let reopened_write = Connection::open(active_path).map_err(|e| e.to_string())?;
configure_write_connection(&reopened_write).map_err(|e| e.to_string())?;
let reopened_read = Connection::open_with_flags(active_path, OpenFlags::SQLITE_OPEN_READ_ONLY)
.map_err(|e| e.to_string())?;
configure_read_connection(&reopened_read).map_err(|e| e.to_string())?;
drop(read_conn);
drop(write_conn);
let (reopened_write, reopened_read) =
open_database_connections(active_path).map_err(|e| e.to_string())?;
let mut write_conn = self.write_conn.lock().map_err(|_| {
"library store write lock poisoned during database restore".to_string()
})?;
let mut read_conn = self.read_conn.lock().map_err(|_| {
"library store read lock poisoned during database restore".to_string()
})?;
*write_conn = reopened_write;
*read_conn = reopened_read;
swap_guard.release();
Ok(())
}
}
@@ -310,6 +389,74 @@ fn log_write_op(op: &str, lock_wait_ms: u128, exec_ms: u128) {
}
}
struct SwapInProgressGuard<'a> {
store: &'a LibraryStore,
released: bool,
}
impl<'a> SwapInProgressGuard<'a> {
fn new(store: &'a LibraryStore) -> Self {
store.swap_in_progress.store(true, Ordering::Release);
Self {
store,
released: false,
}
}
fn release(&mut self) {
if !self.released {
self.store.swap_in_progress.store(false, Ordering::Release);
self.released = true;
}
}
}
impl Drop for SwapInProgressGuard<'_> {
fn drop(&mut self) {
self.release();
}
}
fn run_conn_closure<R>(
conn: &Connection,
f: impl FnOnce(&Connection) -> rusqlite::Result<R>,
) -> Result<R, String> {
let out = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| f(conn)));
match out {
Ok(result) => result.map_err(|e| e.to_string()),
Err(payload) => {
let detail = panic_payload_to_string(payload);
crate::app_eprintln!("[library-db] connection query panicked: {detail}");
Err(format!("library connection query panicked: {detail}"))
}
}
}
fn run_conn_mut_closure<R>(
conn: &mut Connection,
f: impl FnOnce(&mut Connection) -> rusqlite::Result<R>,
) -> Result<R, String> {
let out = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| f(conn)));
match out {
Ok(result) => result.map_err(|e| e.to_string()),
Err(payload) => {
let detail = panic_payload_to_string(payload);
crate::app_eprintln!("[library-db] connection mutation panicked: {detail}");
Err(format!("library connection mutation panicked: {detail}"))
}
}
}
fn panic_payload_to_string(payload: Box<dyn std::any::Any + Send>) -> String {
if let Some(msg) = payload.downcast_ref::<&str>() {
msg.to_string()
} else if let Some(msg) = payload.downcast_ref::<String>() {
msg.clone()
} else {
"unknown panic payload".to_string()
}
}
fn library_db_path(app: &tauri::AppHandle) -> Result<PathBuf, String> {
let base = app.path().app_data_dir().map_err(|e| e.to_string())?;
let db_dir = base.join("databases").join("library");
@@ -429,6 +576,151 @@ fn checkpoint_wal_conn(conn: &Connection, op: &str) -> rusqlite::Result<()> {
Ok(())
}
/// Open write + read handles after migrations, one-time repairs, and WAL checkpoint.
fn open_database_connections(db_path: &Path) -> rusqlite::Result<(Connection, Connection)> {
let write_conn = Connection::open(db_path)?;
configure_write_connection(&write_conn)?;
prepare_write_connection_for_open(&write_conn)?;
let read_conn = Connection::open_with_flags(db_path, OpenFlags::SQLITE_OPEN_READ_ONLY)?;
configure_read_connection(&read_conn)?;
Ok((write_conn, read_conn))
}
fn prepare_write_connection_for_open(conn: &Connection) -> rusqlite::Result<()> {
run_migrations(conn)?;
maybe_reconcile_artist_name_sort(conn)?;
ensure_genre_tags_schema(conn)?;
checkpoint_wal_conn(conn, "open")?;
Ok(())
}
fn artist_name_sort_column_exists(conn: &Connection) -> rusqlite::Result<bool> {
let column_exists: i64 = conn
.query_row(
"SELECT COUNT(*) FROM pragma_table_info('artist') WHERE name = 'name_sort'",
[],
|row| row.get(0),
)
.unwrap_or(0);
Ok(column_exists > 0)
}
fn sync_state_ignored_articles_column_exists(conn: &Connection) -> rusqlite::Result<bool> {
let column_exists: i64 = conn
.query_row(
"SELECT COUNT(*) FROM pragma_table_info('sync_state') WHERE name = 'ignored_articles'",
[],
|row| row.get(0),
)
.unwrap_or(0);
Ok(column_exists > 0)
}
/// Apply schema 014 idempotently — mirrors `migrations/014_artist_name_sort.sql`
/// but tolerates a partial prior apply (missing one column / re-run).
fn apply_migration_14(conn: &Connection) -> rusqlite::Result<()> {
if !artist_name_sort_column_exists(conn)? {
conn.execute_batch("ALTER TABLE artist ADD COLUMN name_sort TEXT;")?;
}
if !sync_state_ignored_articles_column_exists(conn)? {
conn.execute_batch("ALTER TABLE sync_state ADD COLUMN ignored_articles TEXT;")?;
}
conn.execute_batch(
"CREATE INDEX IF NOT EXISTS idx_artist_name_sort ON artist(server_id, name_sort);",
)?;
finish_migration_14_reconcile(conn)?;
Ok(())
}
fn record_schema_migration(conn: &Connection, version: i64) -> rusqlite::Result<()> {
conn.execute(
"INSERT INTO schema_migrations (version, applied_at) VALUES (?1, strftime('%s','now'))",
params![version],
)?;
Ok(())
}
fn finish_migration_14_reconcile(conn: &Connection) -> rusqlite::Result<()> {
if !artist_name_sort_reconcile_completed(conn)? {
repair_artist_name_sort_keys(conn)?;
mark_artist_name_sort_reconcile_completed(conn)?;
}
Ok(())
}
fn artist_name_sort_reconcile_completed(conn: &Connection) -> rusqlite::Result<bool> {
let completed: Option<Option<i64>> = conn
.query_row(
"SELECT completed_at FROM library_data_migration WHERE id = ?1",
params![ARTIST_NAME_SORT_RECONCILE_ID],
|row| row.get(0),
)
.optional()?;
Ok(completed.flatten().is_some())
}
fn mark_artist_name_sort_reconcile_completed(conn: &Connection) -> rusqlite::Result<()> {
conn.execute(
"INSERT INTO library_data_migration (id, cursor_rowid, started_at, completed_at) \
VALUES (?1, 0, strftime('%s','now'), strftime('%s','now')) \
ON CONFLICT(id) DO UPDATE SET completed_at = excluded.completed_at",
params![ARTIST_NAME_SORT_RECONCILE_ID],
)?;
Ok(())
}
/// One-time reconcile after schema 014 — not on every open (avoids long write locks at startup).
fn maybe_reconcile_artist_name_sort(conn: &Connection) -> rusqlite::Result<()> {
if !artist_name_sort_column_exists(conn)? {
return Ok(());
}
if artist_name_sort_reconcile_completed(conn)? {
return Ok(());
}
repair_artist_name_sort_keys(conn)?;
mark_artist_name_sort_reconcile_completed(conn)?;
Ok(())
}
/// Reconcile `artist.name_sort` with display `name` (upgrade / stale rows).
fn repair_artist_name_sort_keys(conn: &Connection) -> rusqlite::Result<()> {
let table_exists: i64 = conn
.query_row(
"SELECT COUNT(*) FROM sqlite_master WHERE type = 'table' AND name = 'artist'",
[],
|row| row.get(0),
)
.unwrap_or(0);
if table_exists == 0 {
return Ok(());
}
if !artist_name_sort_column_exists(conn)? {
return Ok(());
}
let ignored = crate::artist_sort::DEFAULT_IGNORED_ARTICLES;
let tx = conn.unchecked_transaction()?;
{
let mut stmt = tx.prepare("SELECT server_id, id, name, name_sort FROM artist")?;
let mut rows = stmt.query([])?;
while let Some(row) = rows.next()? {
let server_id: String = row.get(0)?;
let id: String = row.get(1)?;
let name: String = row.get(2)?;
let current: Option<String> = row.get(3)?;
let expected = crate::artist_sort::sort_key_for_display_name(&name, ignored);
if current.as_deref() == Some(&expected) {
continue;
}
tx.execute(
"UPDATE artist SET name_sort = ?1 WHERE server_id = ?2 AND id = ?3",
rusqlite::params![expected, server_id, id],
)?;
}
}
tx.commit()?;
Ok(())
}
fn run_migrations(conn: &Connection) -> rusqlite::Result<MigrationOutcome> {
run_migrations_with(
conn,
@@ -477,11 +769,17 @@ pub(crate) fn run_migrations_with(
if already > 0 {
continue;
}
if version == 14 {
// Applied idempotently (per-column ADD + IF NOT EXISTS index) so a
// partial DDL apply — one ALTER landed before a crash, no
// schema_migrations row — recovers instead of failing on a
// duplicate-column re-run of the batch.
apply_migration_14(conn)?;
record_schema_migration(conn, version)?;
continue;
}
conn.execute_batch(sql)?;
conn.execute(
"INSERT INTO schema_migrations (version, applied_at) VALUES (?1, strftime('%s','now'))",
params![version],
)?;
record_schema_migration(conn, version)?;
}
Ok(MigrationOutcome::Applied)
}
@@ -782,4 +1080,125 @@ mod tests {
.unwrap();
assert_eq!(outcome, MigrationOutcome::Applied);
}
#[test]
fn artist_name_sort_reconcile_runs_once_and_sets_name_sort() {
let store = LibraryStore::open_in_memory();
store
.with_conn_mut("test.seed_artist", |conn| {
conn.execute(
"INSERT INTO artist (server_id, id, name, name_sort, synced_at) \
VALUES ('s1', 'ar1', 'The Beatles', 'the beatles', 1)",
[],
)?;
conn.execute(
"DELETE FROM library_data_migration WHERE id = ?1",
params![ARTIST_NAME_SORT_RECONCILE_ID],
)?;
Ok(())
})
.expect("seed artist");
store
.with_conn("test.reconcile", maybe_reconcile_artist_name_sort)
.expect("reconcile");
let name_sort: String = store
.with_read_conn(|conn| {
conn.query_row(
"SELECT name_sort FROM artist WHERE server_id = 's1' AND id = 'ar1'",
[],
|r| r.get(0),
)
})
.expect("read name_sort");
assert_eq!(name_sort, "beatles");
let completed_before: i64 = store
.with_read_conn(|conn| {
conn.query_row(
"SELECT completed_at FROM library_data_migration WHERE id = ?1",
params![ARTIST_NAME_SORT_RECONCILE_ID],
|r| r.get(0),
)
})
.expect("reconcile marker");
assert!(completed_before > 0);
store
.with_conn("test.reconcile_again", maybe_reconcile_artist_name_sort)
.expect("reconcile again");
let name_sort_after: String = store
.with_read_conn(|conn| {
conn.query_row(
"SELECT name_sort FROM artist WHERE server_id = 's1' AND id = 'ar1'",
[],
|r| r.get(0),
)
})
.expect("read name_sort again");
assert_eq!(name_sort_after, "beatles");
}
#[test]
fn migration_14_recovers_partial_schema_without_schema_migrations_row() {
let uri = in_memory_uri();
let conn = Connection::open(&uri).expect("connection");
configure_write_connection(&conn).expect("pragmas");
let migrations_through_13: &[(i64, &str)] = &[
(1, INITIAL_SQL),
(12, MIGRATION_012_TRACK_GENRE_LEGACY),
(13, MIGRATION_013_ARTIST_ARTWORK_LOOKUP),
];
run_migrations_with(
&conn,
migrations_through_13,
LIBRARY_DB_MIN_COMPATIBLE_VERSION,
no_op_hook,
)
.expect("migrate through v13");
conn.execute_batch(MIGRATION_014_ARTIST_NAME_SORT)
.expect("apply ddl only");
let recorded: i64 = conn
.query_row(
"SELECT COUNT(*) FROM schema_migrations WHERE version = 14",
[],
|r| r.get(0),
)
.expect("count migration");
assert_eq!(recorded, 0);
run_migrations_with(
&conn,
MIGRATIONS,
LIBRARY_DB_MIN_COMPATIBLE_VERSION,
no_op_hook,
)
.expect("recover partial migration");
let recorded_after: i64 = conn
.query_row(
"SELECT COUNT(*) FROM schema_migrations WHERE version = 14",
[],
|r| r.get(0),
)
.expect("count migration after");
assert_eq!(recorded_after, 1);
}
#[test]
fn read_conn_recovers_after_closure_panic() {
let store = LibraryStore::open_in_memory();
let first: Result<i64, String> = store.with_read_conn(|_conn| {
panic!("simulated read panic");
});
assert!(first.is_err());
let ok: i64 = store
.with_read_conn(|conn| conn.query_row("SELECT 1", [], |r| r.get(0)))
.expect("read after panic recovery");
assert_eq!(ok, 1);
}
}
@@ -0,0 +1,32 @@
//! Persist Subsonic `getArtists` / `getIndexes` bodies into the local `artist` table.
use crate::repos::{ArtistRepository, SyncStateRepository};
use crate::store::LibraryStore;
use psysonic_integration::subsonic::ArtistIndex;
use super::error::SyncError;
pub fn apply_artist_index(
store: &LibraryStore,
server_id: &str,
library_scope: &str,
index: &ArtistIndex,
) -> Result<(), SyncError> {
let synced_at = super::now_unix_ms();
let ignored = crate::artist_sort::ignored_articles_or_default(
index.ignored_articles.as_deref(),
);
let sync_state = SyncStateRepository::new(store);
sync_state
.set_ignored_articles(server_id, library_scope, ignored)
.map_err(SyncError::Storage)?;
let repo = ArtistRepository::new(store);
repo.upsert_index(server_id, index, synced_at).map_err(SyncError::Storage)?;
repo.backfill_from_tracks(server_id, ignored, synced_at).map_err(SyncError::Storage)?;
if let Some(ms) = index.last_modified_ms {
sync_state
.set_artists_last_modified_ms(server_id, library_scope, ms)
.map_err(SyncError::Storage)?;
}
Ok(())
}
@@ -204,8 +204,20 @@ impl<'a> DeltaSyncRunner<'a> {
}
}
// DS-9 — stamp watermarks.
// DS-9 — stamp watermarks + refresh artist browse index when applicable.
if let Some(ms) = probe.next_artists_watermark {
let scope = self.library_scope_opt();
if let Ok(index) = self.subsonic.get_artists(scope).await {
super::artist_index::apply_artist_index(
self.store,
&self.server_id,
&self.library_scope,
&index,
)?;
}
// Advance the watermark to the probed value regardless of the index
// refresh result — a failed/empty `getArtists` must not force a full
// refetch on every delta. Wins over the index's own last-modified.
sync_state
.set_artists_last_modified_ms(&self.server_id, &self.library_scope, ms)
.map_err(SyncError::Storage)?;
@@ -529,13 +541,7 @@ struct DeltaPollOutcome {
next_artists_watermark: Option<i64>,
}
fn now_unix_ms() -> i64 {
use std::time::{SystemTime, UNIX_EPOCH};
SystemTime::now()
.duration_since(UNIX_EPOCH)
.map(|d| d.as_millis().min(i64::MAX as u128) as i64)
.unwrap_or(0)
}
use super::now_unix_ms;
async fn retry_with_backoff<'a, F, FFut, T, E>(
runner: &DeltaSyncRunner<'a>,
@@ -1179,7 +1179,7 @@ impl<'a> InitialSyncRunner<'a> {
async fn run_artist_pass(
&self,
sync_state: &SyncStateRepository<'_>,
_sync_state: &SyncStateRepository<'_>,
) -> Result<(), SyncError> {
let scope = self.library_scope_opt();
let artists = retry_with_backoff(
@@ -1190,11 +1190,12 @@ impl<'a> InitialSyncRunner<'a> {
.await
.ok();
if let Some(index) = artists {
if let Some(ms) = index.last_modified_ms {
sync_state
.set_artists_last_modified_ms(&self.server_id, &self.library_scope, ms)
.map_err(SyncError::Storage)?;
}
super::artist_index::apply_artist_index(
self.store,
&self.server_id,
&self.library_scope,
&index,
)?;
}
Ok(())
}
@@ -1227,13 +1228,7 @@ fn is_empty_cursor(v: &Value) -> bool {
matches!(v, Value::Object(o) if o.is_empty())
}
fn now_unix_ms() -> i64 {
use std::time::{SystemTime, UNIX_EPOCH};
SystemTime::now()
.duration_since(UNIX_EPOCH)
.map(|d| d.as_millis().min(i64::MAX as u128) as i64)
.unwrap_or(0)
}
use super::now_unix_ms;
/// Wrap an async closure in §6.8 backoff. Retries on `SyncError::Transport`
/// up to `MAX_ATTEMPTS_PER_BATCH`, sleeping per the backoff schedule
@@ -6,6 +6,7 @@
//! `DeltaSyncRunner` / background scheduler / Tauri surface follow in
//! PR-3c / PR-3d / PR-5.
pub mod artist_index;
pub mod backoff;
pub mod bandwidth;
pub mod budget;
@@ -38,3 +39,12 @@ pub use scheduler::{BackgroundScheduler, SchedulerTickReport, DEFAULT_TOMBSTONE_
pub use strategy::IngestStrategy;
pub use supervisor::SyncSupervisor;
pub use tombstone::{should_auto_reconcile, TombstoneReconciler, TombstoneReport};
/// Wall-clock milliseconds since the Unix epoch, saturating to `i64::MAX`.
pub(crate) fn now_unix_ms() -> i64 {
use std::time::{SystemTime, UNIX_EPOCH};
SystemTime::now()
.duration_since(UNIX_EPOCH)
.map(|d| d.as_millis().min(i64::MAX as u128) as i64)
.unwrap_or(0)
}