fix(qbz-qt): preserve media server album artwork

cx
This commit is contained in:
vicrodh committed 2026-08-26 01:37:09 -06:00
1 parent d45e7d9430
commit 40339e564d
7 files changed
+317 -94

No files matched your search

+85
View File
@@ -343,6 +343,36 @@ pub fn item_exists(
Ok(exists)
}
/// Fill an artwork snapshot that was absent when an item was inserted.
/// Existing non-empty artwork is authoritative (including a user-selected
/// snapshot) and is never overwritten. Every duplicate of the same
/// source-owned item in the collection is repaired together.
pub fn backfill_item_artwork(
conn: &Connection,
collection_id: &str,
source: AlbumSource,
source_item_id: &str,
artwork_url: &str,
) -> Result<usize> {
if artwork_url.trim().is_empty() {
return Ok(0);
}
conn.execute(
"UPDATE mixtape_collection_items
SET artwork_url = ?1
WHERE collection_id = ?2
AND source = ?3
AND source_item_id = ?4
AND (artwork_url IS NULL OR TRIM(artwork_url) = '')",
params![
artwork_url,
collection_id,
serialize_source(source),
source_item_id,
],
)
}
pub fn remove_item(conn: &Connection, collection_id: &str, position: i32) -> Result<()> {
conn.execute(
"DELETE FROM mixtape_collection_items
@@ -653,6 +683,61 @@ mod tests {
assert!(matches!(items[2].item_type, ItemType::Playlist));
}
#[test]
fn artwork_backfill_only_repairs_empty_snapshots() {
let conn = fresh_db();
let c = create_collection(
&conn,
CollectionKind::Collection,
"remote",
None,
CollectionSourceType::Manual,
None,
)
.unwrap();
add_item(
&conn,
&c.id,
ItemType::Album,
AlbumSource::Local,
"jellyfin:album-1",
"Album",
None,
None,
None,
None,
)
.unwrap();
assert_eq!(
backfill_item_artwork(
&conn,
&c.id,
AlbumSource::Local,
"jellyfin:album-1",
"jellyfin:album-1/tag",
)
.unwrap(),
1
);
assert_eq!(
backfill_item_artwork(
&conn,
&c.id,
AlbumSource::Local,
"jellyfin:album-1",
"jellyfin:new-tag",
)
.unwrap(),
0,
"a resolved refresh must not replace an existing snapshot"
);
assert_eq!(
list_items(&conn, &c.id).unwrap()[0].artwork_url.as_deref(),
Some("jellyfin:album-1/tag")
);
}
#[test]
fn remove_item_compacts_positions() {
let conn = fresh_db();
+45 -21
View File
@@ -427,6 +427,21 @@ fn build_recent_sections() -> Vec<HomeSection> {
/// most one bounded album lookup per distinct source/version. The exact played
/// row wins for a track card (therefore its disc cover); an album card falls
/// back to the first available disc/collection cover in that physical copy.
fn recent_artwork_from_rows(
rows: &[qbz_library::LocalTrack],
exact_id: Option<i64>,
scope: crate::local_rows::ArtworkScope,
) -> String {
exact_id
.and_then(|id| rows.iter().find(|row| row.id == id))
.and_then(|row| crate::local_rows::portable_artwork_ref(row, scope))
.or_else(|| {
rows.iter()
.find_map(|row| crate::local_rows::portable_artwork_ref(row, scope))
})
.unwrap_or_default()
}
fn backfill_recent_artwork(
albums: &mut [crate::recently_qt::RecentAlbum],
tracks: &mut [crate::recently_qt::RecentTrack],
@@ -454,27 +469,18 @@ fn backfill_recent_artwork(
rows
}).clone()
};
let queue_art = |row: &qbz_library::LocalTrack| {
crate::local_playback::local_queue_track(row)
.artwork_url
.unwrap_or_default()
};
for track in tracks.iter_mut() {
if !track.artwork_url.is_empty() && !track.album_artwork_url.is_empty() {
continue;
}
let rows = rows_for(&track.source, &track.album_id);
let row_id = track.id.parse::<u64>().ok().map(|id| id as i64);
let exact = row_id.and_then(|id| rows.iter().find(|row| row.id == id));
let candidate = exact
.filter(|row| row.artwork_path.as_deref().is_some_and(|art| !art.is_empty()))
.or_else(|| {
rows.iter()
.find(|row| row.artwork_path.as_deref().is_some_and(|art| !art.is_empty()))
});
if let Some(row) = candidate {
let art = queue_art(row);
let art = recent_artwork_from_rows(
&rows,
row_id,
crate::local_rows::ArtworkScope::Track,
);
if !art.is_empty() {
if track.artwork_url.is_empty() {
track.artwork_url = art.clone();
}
@@ -485,12 +491,11 @@ fn backfill_recent_artwork(
}
for album in albums.iter_mut().filter(|album| album.artwork_url.is_empty()) {
let rows = rows_for(&album.source, &album.id);
if let Some(row) = rows
.iter()
.find(|row| row.artwork_path.as_deref().is_some_and(|art| !art.is_empty()))
{
album.artwork_url = queue_art(row);
}
album.artwork_url = recent_artwork_from_rows(
&rows,
None,
crate::local_rows::ArtworkScope::Album,
);
}
}
@@ -2196,4 +2201,23 @@ mod tests {
assert_eq!(doc["recentlyPlayedPlaylists"]["kind"], "playlist");
assert_eq!(doc["continueListening"]["kind"], "slimTracks");
}
#[test]
fn recent_artwork_backfill_reads_remote_collection_tokens() {
let rows = vec![qbz_library::LocalTrack {
id: 42,
source: Some("jellyfin".into()),
artwork_path: None,
collection_artwork_path: Some("album-7/tag".into()),
..Default::default()
}];
assert_eq!(
recent_artwork_from_rows(&rows, Some(42), crate::local_rows::ArtworkScope::Track),
"jellyfin:album-7/tag"
);
assert_eq!(
recent_artwork_from_rows(&rows, None, crate::local_rows::ArtworkScope::Album),
"jellyfin:album-7/tag"
);
}
}
+11 -6
View File
@@ -923,14 +923,19 @@ pub fn add_to_mixtape(id: String) {
// carried a url. This call site was the only place that could produce
// them.
//
// A local absolute path is CORRECT here and is not the file:// cache
// path the track sites are warned off: the store keeps local paths raw
// and the mosaic passes them through unrewritten, which is what lets a
// collection of local albums render with no network at all.
// The portable reference is safe to persist: local covers are encoded
// file URLs, Plex remains a server-relative thumb, and Jellyfin /
// Subsonic tokens carry their source prefix without credentials. The
// collection layer wins because this payload represents an ALBUM, not
// one disc/track.
artwork_url: tracks
.iter()
.find_map(|t| t.artwork_path.as_ref().filter(|p| !p.is_empty()))
.cloned(),
.find_map(|track| {
crate::local_rows::portable_artwork_ref(
track,
crate::local_rows::ArtworkScope::Album,
)
}),
year: None,
track_count: Some(tracks.len() as i32),
};
+4 -30
View File
@@ -251,36 +251,10 @@ pub fn local_queue_track(t: &LocalTrack) -> QueueTrack {
let is_offline = src == "qobuz_download";
let is_plex = src == "plex";
let is_media = src == "jellyfin" || src == "subsonic";
// A Plex row carries a RAW server-relative thumb path; it must stay raw
// so the now-playing bar / queue resolve it from current creds.
// `file://`-prefixing it poisons it into a local-read miss.
let artwork_url = t
.artwork_path
.as_ref()
.filter(|path| !path.is_empty())
.or_else(|| {
t.collection_artwork_path
.as_ref()
.filter(|path| !path.is_empty())
})
.map(|p| {
if is_plex || p.starts_with("file://") {
p.clone()
} else if is_media {
// SOURCE-TAGGED, because a media server's token is meaningless on
// its own: a Jellyfin image tag and a Subsonic coverArt id are
// both opaque, and the now-playing bar / queue / MPRIS resolve an
// `artwork_url` through the shared taxonomy with no row in hand.
// The tag is what lets that taxonomy hand it back to the source
// that issued it instead of guessing (`artwork_qt::classify`).
format!("{src}:{p}")
} else {
// Encode `#`, `?` and `%` exactly like every QML artwork path.
// Box/disc folder names commonly contain these; a raw file URL
// would make MPRIS treat the tail as a fragment.
crate::artwork_qt::file_url(p)
}
});
let artwork_url = crate::local_rows::portable_artwork_ref(
t,
crate::local_rows::ArtworkScope::Track,
);
let sample_rate_khz = if t.sample_rate >= 1000.0 {
t.sample_rate / 1000.0
} else {
+77 -1
View File
@@ -340,6 +340,53 @@ pub fn disc_cover_url(t: &LocalTrack) -> String {
.unwrap_or_default()
}
/// Which layer of a source-owned artwork reference a consumer needs.
/// Album surfaces prefer the collection/box cover; track surfaces prefer the
/// item/disc cover. Both fall back to the other layer when their preferred one
/// is absent.
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub(crate) enum ArtworkScope {
Album,
Track,
}
/// A portable artwork reference for a `LocalTrack`.
///
/// This is the single boundary between source-owned opaque tokens and Qt
/// surfaces that may no longer have the source row at render time (Recents,
/// MyQBZ, queue, search). Jellyfin/Subsonic tokens are source-qualified so the
/// shared artwork resolver never has to guess their owner; Plex keeps its
/// server-relative path; local files become encoded `file://` URLs. The value
/// is safe to persist because it contains no media-server credentials.
pub(crate) fn portable_artwork_ref(t: &LocalTrack, scope: ArtworkScope) -> Option<String> {
let item = t.artwork_path.as_deref().filter(|value| !value.is_empty());
let collection = t
.collection_artwork_path
.as_deref()
.filter(|value| !value.is_empty());
let token = match scope {
ArtworkScope::Album => collection.or(item),
ArtworkScope::Track => item.or(collection),
}?;
let source = t
.source
.as_deref()
.and_then(SourceId::from_word)
.unwrap_or(SourceId::LOCAL);
if source == SourceId::JELLYFIN || source == SourceId::SUBSONIC {
return Some(format!("{}:{token}", source.as_str()));
}
if source == SourceId::PLEX
|| token.starts_with("http://")
|| token.starts_with("https://")
|| token.starts_with("file://")
{
return Some(token.to_owned());
}
Some(crate::artwork_qt::file_url(token))
}
pub fn track_key(id: i64) -> String {
format!("track:{id}")
}
@@ -568,7 +615,8 @@ pub fn to_json<T: Serialize>(value: &T) -> String {
#[cfg(test)]
mod tests {
use super::album_favorite_source;
use super::{album_favorite_source, portable_artwork_ref, ArtworkScope};
use qbz_library::LocalTrack;
fn sources(words: &[&str]) -> Vec<String> {
words.iter().map(|word| (*word).to_string()).collect()
@@ -599,4 +647,32 @@ mod tests {
None
);
}
#[test]
fn portable_artwork_ref_preserves_source_and_scope() {
let jellyfin = LocalTrack {
source: Some("jellyfin".into()),
artwork_path: Some("track-id/disc-tag".into()),
collection_artwork_path: Some("album-id/album-tag".into()),
..Default::default()
};
assert_eq!(
portable_artwork_ref(&jellyfin, ArtworkScope::Track).as_deref(),
Some("jellyfin:track-id/disc-tag")
);
assert_eq!(
portable_artwork_ref(&jellyfin, ArtworkScope::Album).as_deref(),
Some("jellyfin:album-id/album-tag")
);
let navidrome = LocalTrack {
source: Some("navidrome".into()),
collection_artwork_path: Some("al-123_hash".into()),
..Default::default()
};
assert_eq!(
portable_artwork_ref(&navidrome, ArtworkScope::Album).as_deref(),
Some("subsonic:al-123_hash")
);
}
}
+58 -2
View File
@@ -703,6 +703,37 @@ pub(crate) fn get_collection(id: &str) -> Option<MixtapeCollection> {
.flatten()
}
/// Persist source-resolved cover references for legacy MyQBZ items that were
/// inserted without an artwork snapshot. One DB connection handles the whole
/// resolve wave; the repository refuses to overwrite any non-empty value.
fn backfill_resolved_artwork(
collection_id: &str,
repairs: &[(AlbumSource, String, String)],
) -> usize {
crate::library_db_qt::with_db(false, |db| {
Ok(db.with_connection(|conn| {
repairs.iter().fold(0usize, |count, (source, id, art)| {
match qbz_mixtape::repo::backfill_item_artwork(
conn,
collection_id,
source.clone(),
id,
art,
) {
Ok(changed) => count + changed,
Err(error) => {
log::warn!(
"[qbz-qt] MyQBZ artwork backfill failed for {collection_id}/{id}: {error}"
);
count
}
}
})
}))
})
.unwrap_or_default()
}
// ---------------------------------------------------------------------------
// Row building
// ---------------------------------------------------------------------------
@@ -1584,15 +1615,40 @@ fn resolve_items(runtime: Arc<AppRuntime<LoggingAdapter>>) {
// The user may have navigated to another collection while this
// task queued behind the cap — its result is then dead weight.
if current_id() != collection_id {
return;
return None;
}
let tracks = crate::myqbz_play_qt::fetch_item_tracks(&runtime, &item).await;
let resolved = resolve_from_tracks(&item, &tracks);
let repair = item
.artwork_url
.as_deref()
.is_none_or(str::is_empty)
.then(|| resolved.artwork_url.clone())
.filter(|art| !art.is_empty())
.map(|art| (item.source.clone(), item.source_item_id.clone(), art));
apply_resolved(&item, resolved, &collection_id);
repair
}));
}
let mut repairs = Vec::new();
for handle in handles {
let _ = handle.await;
if let Ok(Some(repair)) = handle.await {
repairs.push(repair);
}
}
if !repairs.is_empty() {
let id = collection_id.clone();
let repaired = tokio::task::spawn_blocking(move || {
backfill_resolved_artwork(&id, &repairs)
})
.await
.unwrap_or_default();
if repaired > 0 {
// The index cards keep item snapshots in their mosaic cache.
// One reload after the whole resolve wave makes the repair
// visible when Back returns there; never one reload per item.
crate::myqbz_qt::reload_grids();
}
}
});
}
+37 -34
View File
@@ -98,7 +98,8 @@ pub(crate) fn local_album_artist(t: &qbz_library::LocalTrack) -> String {
.unwrap_or_else(|| t.artist.clone())
}
/// Split a local artwork path into this port's `(art_url, art_path)` pair.
/// Split one source-owned artwork reference into this port's
/// `(art_url, art_path)` pair.
///
/// **This is a DELIBERATE divergence from the reference and must not be
/// "fixed" back.** The Slint stores one RAW path with any `file://` STRIPPED,
@@ -107,39 +108,21 @@ pub(crate) fn local_album_artist(t: &qbz_library::LocalTrack) -> String {
///
/// - a local filesystem cover becomes a `file://` url in `art_path`, handed
/// straight to QML;
/// - a Plex thumb or an http(s) cover goes to `art_url`, so it rides the
/// SHARED `attach_urls` + `download_missing` pipeline exactly like the Qobuz
/// rows and shares its cache entry with the Local Library grid;
/// - Plex, HTTP and source-qualified Jellyfin/Subsonic references go to
/// `art_url`, so they ride the SHARED `attach_urls` + `download_missing`
/// pipeline exactly like Qobuz rows;
/// - nothing usable yields two empty strings.
pub(crate) fn local_art_split(source: Option<&str>, path: Option<&str>) -> (String, String) {
let Some(p) = path.filter(|p| !p.is_empty()) else {
pub(crate) fn local_art_split(
track: &qbz_library::LocalTrack,
scope: crate::local_rows::ArtworkScope,
) -> (String, String) {
let Some(reference) = crate::local_rows::portable_artwork_ref(track, scope) else {
return (String::new(), String::new());
};
// The ROW's source decides, not the shape of the string (design 02 §9
// stage 4). `classify` stood here and would read a Jellyfin token as a
// filesystem path purely because it starts with `/`.
// Resolved here rather than deferred: this list is the cortinilla's
// bounded result set (76 rows), not the whole library, and the split it
// needs — `art_path` for a file, `art_url` for a fetch — is decided per
// row by definition.
let Some((src, token)) = crate::local_rows::art_token(source, p) else {
return (String::new(), String::new());
};
match qbz_source::registry().artwork_token(src, &token, qbz_source::ArtSize::Card) {
qbz_source::ArtRef::File(f) => (
String::new(),
crate::artwork_qt::file_url(&f.to_string_lossy()),
),
// The RAW token, not the resolved url: `attach_urls` /
// `download_missing` re-resolve it downstream and memoize under it, so
// this row shares its cache entry with the Local Library grid. That
// sharing is the whole reason this split exists.
qbz_source::ArtRef::Fetch { .. } => (p.to_string(), String::new()),
// Nothing usable, or a server that is not connected — two empty
// strings either way, which is what leaves the placeholder up.
qbz_source::ArtRef::None | qbz_source::ArtRef::Unavailable(_) => {
(String::new(), String::new())
}
if reference.starts_with("file://") {
(String::new(), reference)
} else {
(reference, String::new())
}
}
@@ -212,7 +195,8 @@ pub(crate) fn derive_local_album_rows(
if out.len() >= cap {
continue; // keep counting for an honest has_more
}
let (art_url, art_path) = local_art_split(t.source.as_deref(), t.artwork_path.as_deref());
let (art_url, art_path) =
local_art_split(t, crate::local_rows::ArtworkScope::Album);
out.push(CortRow {
kind: "album".into(),
id: key,
@@ -264,7 +248,8 @@ pub(crate) fn derive_local_artist_rows(
if out.len() >= cap {
continue;
}
let (art_url, art_path) = local_art_split(t.source.as_deref(), t.artwork_path.as_deref());
let (art_url, art_path) =
local_art_split(t, crate::local_rows::ArtworkScope::Album);
out.push(CortRow {
kind: "artist".into(),
id: String::new(),
@@ -288,7 +273,8 @@ pub(crate) fn derive_local_artist_rows(
/// media action. `id` is the library row id; the router resolves the concrete
/// `LocalTrack` back from the per-query snapshot, NOT from this id.
pub(crate) fn map_local_track_to_cort_row(t: &qbz_library::LocalTrack) -> CortRow {
let (art_url, art_path) = local_art_split(t.source.as_deref(), t.artwork_path.as_deref());
let (art_url, art_path) =
local_art_split(t, crate::local_rows::ArtworkScope::Track);
// "artist · album" when both exist, else whichever does (U+00B7).
let subtitle = match (t.artist.is_empty(), t.album.is_empty()) {
(false, false) => format!("{} · {}", t.artist, t.album),
@@ -563,6 +549,23 @@ mod tests {
assert_eq!(row.quality_detail, "24-bit / 96 kHz");
}
#[test]
fn remote_collection_art_keeps_its_source_in_search_rows() {
let mut jellyfin = track("The Writing on the Wall", "Iron Maiden", "Senjutsu");
jellyfin.source = Some("jellyfin".into());
jellyfin.artwork_path = None;
jellyfin.collection_artwork_path = Some("jf-album/tag".into());
let (albums, _) = derive_local_album_rows(&[jellyfin], 3, "Senjutsu");
assert_eq!(albums[0].art_url, "jellyfin:jf-album/tag");
let mut navidrome = track("Song", "Artist", "Navidrome Album");
navidrome.source = Some("navidrome".into());
navidrome.artwork_path = None;
navidrome.collection_artwork_path = Some("al-42_hash".into());
let row = map_local_track_to_cort_row(&navidrome);
assert_eq!(row.art_url, "subsonic:al-42_hash");
}
/// has_more must describe the MATCHING set. Counting rejected groups
/// would light up "View more" over a destination with nothing extra.
#[test]