From 3a5056643a56bcc8cabd6fa79bcc633234894fc2 Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Sat, 24 Oct 2026 04:01:45 +0200 Subject: [PATCH 1/3] feat: an automatic artist banner from TheAudioDB's fanart RFC-014 steps 2 and 3. theaudiodb joins WAVEFLOW_METADATA_PROVIDERS, off by default and keyed by WAVEFLOW_METADATA_THEAUDIODB_KEY; it is asked for fanart only, never faster than once every two seconds, and not built with images off. The fanarts are tried in order, a refused one giving way to the next, from TheAudioDB's image hosts only. The first real one is cached on metadata_artist.fanart_hash and linked onto artist.fanart_hash for every artist with the name, one artist event each; a later miss unlinks it, an unreached refresh changes nothing. The artist detail's banner_hash is the chosen banner, else that fanart, with enrichment.banner crediting TheAudioDB while it is shown; the artwork route serves it to members and the sweep keeps it. The web client credits it under the header. Signed-off-by: InstaZDLL --- docs/operations.md | 21 +- docs/rfcs/RFC-011-metadata-enrichment.md | 4 + .../20261024010000_artist_fanart.sql | 17 + src/config.rs | 46 ++- src/database.rs | 148 +++++++- src/enrichment.rs | 137 +++++++- src/services/artist_enrichment.rs | 26 +- src/services/artist_images.rs | 10 +- src/services/artwork.rs | 1 + src/services/catalog.rs | 7 +- src/services/mod.rs | 13 +- tests/enrichment.rs | 330 ++++++++++++++++++ webapp/e2e/studio-nocturne.spec.ts | 50 +++ webapp/src/api/catalog.ts | 5 + webapp/src/pages/artist-page.tsx | 23 +- webapp/src/state/i18n.tsx | 2 + 16 files changed, 806 insertions(+), 34 deletions(-) create mode 100644 migrations-v2/20261024010000_artist_fanart.sql diff --git a/docs/operations.md b/docs/operations.md index 9d668213..512f770d 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -182,12 +182,13 @@ WAVEFLOW_METADATA_LASTFM_KEY=… # your own Last.fm API application's key | Variable | Default | What it does | | --- | --- | --- | -| `WAVEFLOW_METADATA_PROVIDERS` | empty | The switch: an ordered, comma-separated list. Empty is off. The providers are `lastfm` (biographies and similar artists) and `deezer` (artist portraits, no key) | +| `WAVEFLOW_METADATA_PROVIDERS` | empty | The switch: an ordered, comma-separated list. Empty is off. The providers are `lastfm` (biographies and similar artists), `deezer` (artist portraits, no key) and `theaudiodb` (artist banners only — not its biographies) | | `WAVEFLOW_METADATA_LASTFM_KEY` | — | Required when `lastfm` is listed; the server refuses to start without it. No key ships in the binary | -| `WAVEFLOW_METADATA_IMAGES` | on | `off` keeps the lookups and refuses artist image downloads; `deezer` is then not asked at all, since portraits are all it is asked for | +| `WAVEFLOW_METADATA_THEAUDIODB_KEY` | — | Required when `theaudiodb` is listed; the server refuses to start without it. TheAudioDB's public test key is not used | +| `WAVEFLOW_METADATA_IMAGES` | on | `off` keeps the lookups and refuses artist image downloads; `deezer` and `theaudiodb` are then not asked at all, since images are all they are asked for | | `WAVEFLOW_METADATA_TTL_DAYS` | 30 | How long an answer with content is kept before it is asked again | | `WAVEFLOW_METADATA_MISS_TTL_DAYS` | 7 | How long "the provider knows nothing about this artist" is kept | -| `WAVEFLOW_METADATA_RATE_PER_SEC` | 4 | Requests per second, per provider | +| `WAVEFLOW_METADATA_RATE_PER_SEC` | 4 | Requests per second, per provider. TheAudioDB is never asked faster than once every two seconds, its free tier's ceiling, whatever this says | | `WAVEFLOW_METADATA_REQUEST_TIMEOUT_SECS` | 10 | How long one request may take before it counts as unanswered | **Scrobbling's Last.fm key does not switch this on.** An application you @@ -209,9 +210,21 @@ PNG or WebP by its own bytes, and at most 10 MiB. A picture that fails any of these is not fetched and the artist is remembered as having none until the short TTL runs out. +With `theaudiodb` listed, also `www.theaudiodb.com`, with your key in the +request path, and then the artist's **fanart** — the wide photograph the +artist page draws behind its header when no banner was chosen by hand — +under the same rules, from TheAudioDB's image hosts (`r2.theaudiodb.com`, +`www.theaudiodb.com`). Its fanarts are tried in order and the first real one +is kept; the 1000×185 strips TheAudioDB calls banners are never asked for. +A banner chosen by hand always wins over it, and the page credits TheAudioDB +whenever its fanart is the one shown. If a later answer no longer offers the +fanart, the artist goes back to having no automatic banner; a request that +fails changes nothing. + **What it stores.** In `waveflow.db`, for each provider and each artist name asked about: the biography, the ranked list of similar artists by name, the -portrait's hash, whether the provider knew the artist, and when to ask again. +portrait's and the banner's hashes, whether the provider knew the artist, and +when to ask again. The portrait itself goes in the artwork store beside the covers, and every artist with that name shows it through the `coverArt` it already carries — **back it up with the rest of `artwork_dir`**: unlike a cover, it cannot be diff --git a/docs/rfcs/RFC-011-metadata-enrichment.md b/docs/rfcs/RFC-011-metadata-enrichment.md index 9d7dce64..df57bfd9 100644 --- a/docs/rfcs/RFC-011-metadata-enrichment.md +++ b/docs/rfcs/RFC-011-metadata-enrichment.md @@ -149,6 +149,10 @@ Settings, under `WAVEFLOW_METADATA_*`, in `src/config.rs` with the rest: widening: an empty provider list means no outbound request of any kind, and this setting cannot contradict it. - `WAVEFLOW_METADATA_LASTFM_KEY` — the operator's own application key. +- `WAVEFLOW_METADATA_THEAUDIODB_KEY` — added by + [RFC-014](RFC-014-automatic-artist-banner.md) for the `theaudiodb` + provider, which is asked for artist banners only, under the same rule: no + key ships in the binary. - `WAVEFLOW_METADATA_TTL_DAYS`, `WAVEFLOW_METADATA_MISS_TTL_DAYS`, `WAVEFLOW_METADATA_RATE_PER_SEC`, `WAVEFLOW_METADATA_REQUEST_TIMEOUT_SECS`. diff --git a/migrations-v2/20261024010000_artist_fanart.sql b/migrations-v2/20261024010000_artist_fanart.sql new file mode 100644 index 00000000..2dee738e --- /dev/null +++ b/migrations-v2/20261024010000_artist_fanart.sql @@ -0,0 +1,17 @@ +-- An automatic artist banner, from TheAudioDB's fanart (RFC-014 decision 3). +-- +-- The cache records the fanart a provider answered, as `image_hash` records a +-- portrait, and loses it the same way when the sweeper takes the artwork row. +ALTER TABLE metadata_artist + ADD COLUMN fanart_hash TEXT REFERENCES artwork(hash) ON DELETE SET NULL; + +-- Every artist row with the name carries it, in every library, written the way +-- `artwork_hash` is: by the fill, only where it changes, with the artist event. +-- It is the provider's and never a choice — a banner chosen by hand stays in +-- `artist_image_choice`. `ON DELETE SET NULL` as on `artwork_hash`: the sweep +-- keeps what an artist names, so this only ever clears a link to artwork that +-- is already gone. +ALTER TABLE artist + ADD COLUMN fanart_hash TEXT REFERENCES artwork(hash) ON DELETE SET NULL; + +CREATE INDEX artist_fanart ON artist(fanart_hash); diff --git a/src/config.rs b/src/config.rs index a1f2062c..84554712 100644 --- a/src/config.rs +++ b/src/config.rs @@ -200,8 +200,8 @@ pub struct ScrobbleDestination { #[derive(Clone)] pub struct MetadataSettings { /// `WAVEFLOW_METADATA_PROVIDERS`: an ordered, comma-separated list, empty - /// by default, of `lastfm` and `deezer`. The order is the order fields are - /// merged in when read. + /// by default, of `lastfm`, `deezer` and `theaudiodb`. The order is the + /// order fields are merged in when read. pub providers: Vec, /// `WAVEFLOW_METADATA_IMAGES`, on by default, `off` to keep the lookups and /// refuse decision 8's image fetch. A narrowing only: it switches nothing @@ -210,6 +210,11 @@ pub struct MetadataSettings { /// `WAVEFLOW_METADATA_LASTFM_KEY`, the operator's own application key. /// Required when `providers` names `lastfm`: no key ships in this binary. pub lastfm_key: Option, + /// `WAVEFLOW_METADATA_THEAUDIODB_KEY`, the operator's own TheAudioDB key. + /// Required when `providers` names `theaudiodb` (RFC-014 decision 1): the + /// public test key `waveflow-core` uses for one desktop user is not one a + /// server may share with everyone else's. + pub theaudiodb_key: Option, /// `WAVEFLOW_METADATA_TTL_DAYS`: how long an answer with content stands. pub ttl: Duration, /// `WAVEFLOW_METADATA_MISS_TTL_DAYS`: how long "the provider knows nothing" @@ -230,6 +235,7 @@ impl MetadataSettings { providers: Vec::new(), images: true, lastfm_key: None, + theaudiodb_key: None, ttl: Duration::from_secs(30 * 24 * 60 * 60), miss_ttl: Duration::from_secs(7 * 24 * 60 * 60), rate_per_sec: 4, @@ -251,6 +257,10 @@ impl std::fmt::Debug for MetadataSettings { "lastfm_key", &self.lastfm_key.as_ref().map(|_| "[redacted]"), ) + .field( + "theaudiodb_key", + &self.theaudiodb_key.as_ref().map(|_| "[redacted]"), + ) .field("ttl", &self.ttl) .field("miss_ttl", &self.miss_ttl) .field("rate_per_sec", &self.rate_per_sec) @@ -1056,9 +1066,11 @@ fn normalize_public_url(value: &str) -> anyhow::Result { fn parse_metadata_settings() -> anyhow::Result { let defaults = MetadataSettings::off(); let lastfm_key = read_trimmed_env("WAVEFLOW_METADATA_LASTFM_KEY"); + let theaudiodb_key = read_trimmed_env("WAVEFLOW_METADATA_THEAUDIODB_KEY"); let providers = parse_metadata_providers( &std::env::var("WAVEFLOW_METADATA_PROVIDERS").unwrap_or_default(), lastfm_key.is_some(), + theaudiodb_key.is_some(), )?; let days = |name: &str, default: Duration| -> anyhow::Result { let days: u64 = parse_positive_env(name, default.as_secs() / (24 * 60 * 60))?; @@ -1070,6 +1082,7 @@ fn parse_metadata_settings() -> anyhow::Result { providers, images: parse_bool_env("WAVEFLOW_METADATA_IMAGES", defaults.images)?, lastfm_key, + theaudiodb_key, ttl: days("WAVEFLOW_METADATA_TTL_DAYS", defaults.ttl)?, miss_ttl: days("WAVEFLOW_METADATA_MISS_TTL_DAYS", defaults.miss_ttl)?, rate_per_sec: parse_positive_env("WAVEFLOW_METADATA_RATE_PER_SEC", defaults.rate_per_sec)?, @@ -1090,6 +1103,7 @@ fn parse_metadata_settings() -> anyhow::Result { fn parse_metadata_providers( list: &str, has_lastfm_key: bool, + has_theaudiodb_key: bool, ) -> anyhow::Result> { let mut providers = Vec::new(); for name in list @@ -1107,6 +1121,11 @@ fn parse_metadata_providers( "WAVEFLOW_METADATA_PROVIDERS names lastfm without WAVEFLOW_METADATA_LASTFM_KEY" ); } + if provider == crate::enrichment::Provider::TheAudioDb && !has_theaudiodb_key { + anyhow::bail!( + "WAVEFLOW_METADATA_PROVIDERS names theaudiodb without WAVEFLOW_METADATA_THEAUDIODB_KEY" + ); + } providers.push(provider); } Ok(providers) @@ -1236,26 +1255,33 @@ mod tests { fn metadata_providers_are_read_strictly() { use crate::enrichment::Provider; - assert_eq!(parse_metadata_providers("", false).unwrap(), []); - assert_eq!(parse_metadata_providers(" , ", false).unwrap(), []); + assert_eq!(parse_metadata_providers("", false, false).unwrap(), []); + assert_eq!(parse_metadata_providers(" , ", false, false).unwrap(), []); assert_eq!( - parse_metadata_providers(" lastfm ", true).unwrap(), + parse_metadata_providers(" lastfm ", true, false).unwrap(), [Provider::LastFm] ); // No key ships in the binary: naming the provider without one is a // misconfiguration, not a quiet off. - assert!(parse_metadata_providers("lastfm", false).is_err()); - assert!(parse_metadata_providers("lastfm,lastfm", true).is_err()); - assert!(parse_metadata_providers("last.fm", true).is_err()); + assert!(parse_metadata_providers("lastfm", false, false).is_err()); + assert!(parse_metadata_providers("lastfm,lastfm", true, false).is_err()); + assert!(parse_metadata_providers("last.fm", true, false).is_err()); // Deezer asks for no key, so none is required of it. assert_eq!( - parse_metadata_providers("lastfm, deezer", true).unwrap(), + parse_metadata_providers("lastfm, deezer", true, false).unwrap(), [Provider::LastFm, Provider::Deezer] ); assert_eq!( - parse_metadata_providers("deezer", false).unwrap(), + parse_metadata_providers("deezer", false, false).unwrap(), [Provider::Deezer] ); + // TheAudioDB asks for a key of its own (RFC-014 decision 1), and the + // Last.fm one does not stand in for it. + assert!(parse_metadata_providers("theaudiodb", true, false).is_err()); + assert_eq!( + parse_metadata_providers("deezer, theaudiodb", false, true).unwrap(), + [Provider::Deezer, Provider::TheAudioDb] + ); } #[test] diff --git a/src/database.rs b/src/database.rs index a831d3a2..fa9361f3 100644 --- a/src/database.rs +++ b/src/database.rs @@ -1923,7 +1923,8 @@ impl Database { name_canonical: &str, ) -> Result, sqlx::Error> { let rows = sqlx::query( - "SELECT provider, outcome, bio_summary, bio_full, image_hash, fetched_at, expires_at \ + "SELECT provider, outcome, bio_summary, bio_full, image_hash, fanart_hash, \ + fetched_at, expires_at \ FROM metadata_artist WHERE name_canonical = ?", ) .bind(name_canonical) @@ -1945,6 +1946,7 @@ impl Database { bio_summary: row.try_get("bio_summary")?, bio_full: row.try_get("bio_full")?, image_hash: row.try_get("image_hash")?, + fanart_hash: row.try_get("fanart_hash")?, fetched_at: row.try_get("fetched_at")?, expires_at: row.try_get("expires_at")?, }); @@ -2012,6 +2014,37 @@ impl Database { tx.commit().await } + /// Links the banner `hash` to every artist called `name_canonical` that + /// does not show it yet — [`Self::link_artist_portrait`], for the fanart + /// of RFC-014. + pub(crate) async fn link_artist_fanart( + &self, + name_canonical: &str, + hash: &str, + ) -> Result<(), sqlx::Error> { + let unlinked: bool = sqlx::query_scalar( + "SELECT EXISTS (SELECT 1 FROM artist \ + WHERE canonical_name = ? AND fanart_hash IS NOT ?)", + ) + .bind(name_canonical) + .bind(hash) + .fetch_one(&self.pool) + .await?; + if !unlinked { + return Ok(()); + } + let _writer = self.writer_guard().await; + let mut tx = self.pool.begin().await?; + link_fanart_in( + &mut tx, + name_canonical, + Some(hash), + crate::authentication::now_ms(), + ) + .await?; + tx.commit().await + } + /// Records what a provider answered: its row, its whole neighbour list, /// and its portrait — the artwork row and every artist it belongs to — /// in one transaction. @@ -2032,7 +2065,17 @@ impl Database { ) -> Result<(), sqlx::Error> { let _writer = self.writer_guard().await; let mut tx = self.pool.begin().await?; - if let Some(image) = &answer.image { + // What this provider's row named before, read before it is replaced: + // a refresh without the banner unlinks it (RFC-014 decision 3). + let previous_fanart: Option = sqlx::query_scalar( + "SELECT fanart_hash FROM metadata_artist WHERE provider = ? AND name_canonical = ?", + ) + .bind(provider.as_str()) + .bind(name_canonical) + .fetch_optional(&mut *tx) + .await? + .flatten(); + for image in [&answer.image, &answer.fanart].into_iter().flatten() { sqlx::query( "INSERT INTO artwork (hash, format, source, byte_size, created_at) \ VALUES (?, ?, ?, ?, ?) ON CONFLICT (hash) DO NOTHING", @@ -2048,11 +2091,12 @@ impl Database { sqlx::query( "INSERT INTO metadata_artist \ (provider, name_canonical, outcome, bio_summary, bio_full, image_hash, \ - fetched_at, expires_at) \ - VALUES (?, ?, ?, ?, ?, ?, ?, ?) \ + fanart_hash, fetched_at, expires_at) \ + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?) \ ON CONFLICT (provider, name_canonical) DO UPDATE SET \ outcome = excluded.outcome, bio_summary = excluded.bio_summary, \ bio_full = excluded.bio_full, image_hash = excluded.image_hash, \ + fanart_hash = excluded.fanart_hash, \ fetched_at = excluded.fetched_at, expires_at = excluded.expires_at", ) .bind(provider.as_str()) @@ -2061,6 +2105,7 @@ impl Database { .bind(answer.bio_summary.as_deref()) .bind(answer.bio_full.as_deref()) .bind(answer.image.as_ref().map(|image| image.hash.as_str())) + .bind(answer.fanart.as_ref().map(|fanart| fanart.hash.as_str())) .bind(fetched_at) .bind(expires_at) .execute(&mut *tx) @@ -2068,6 +2113,17 @@ impl Database { if let Some(image) = &answer.image { link_portrait_in(&mut tx, name_canonical, &image.hash, fetched_at).await?; } + match (&answer.fanart, previous_fanart) { + (Some(fanart), _) => { + link_fanart_in(&mut tx, name_canonical, Some(&fanart.hash), fetched_at).await?; + } + // A miss, or a hit that no longer carries the fanart: the rows + // still showing the one this provider named go back to none. + (None, Some(previous)) => { + unlink_fanart_in(&mut tx, name_canonical, &previous, fetched_at).await?; + } + (None, None) => {} + } sqlx::query( "DELETE FROM metadata_artist_similar WHERE provider = ? AND name_canonical = ?", ) @@ -2149,6 +2205,90 @@ async fn link_portrait_in( Ok(()) } +/// Points every artist called `name_canonical` at the banner `hash` +/// (RFC-014 decision 3), as [`link_portrait_in`] does the portrait: only rows +/// that move, one artist event each, and only while the artwork row exists and +/// a cache row still names it. +async fn link_fanart_in( + tx: &mut sqlx::Transaction<'_, sqlx::Sqlite>, + name_canonical: &str, + hash: Option<&str>, + now: i64, +) -> Result<(), sqlx::Error> { + let Some(hash) = hash else { + return Ok(()); + }; + let moved = sqlx::query( + "UPDATE artist SET fanart_hash = ?, updated_at = ? \ + WHERE canonical_name = ? AND fanart_hash IS NOT ? \ + AND EXISTS (SELECT 1 FROM artwork WHERE artwork.hash = ?) \ + AND EXISTS (SELECT 1 FROM metadata_artist \ + WHERE name_canonical = ? AND fanart_hash = ?) \ + RETURNING id, library_id", + ) + .bind(hash) + .bind(now) + .bind(name_canonical) + .bind(hash) + .bind(hash) + .bind(name_canonical) + .bind(hash) + .fetch_all(&mut **tx) + .await?; + record_artist_moves(tx, moved, now).await +} + +/// Takes the banner `previous` off every artist called `name_canonical` that +/// still shows it: the provider no longer offers it (RFC-014 decision 3). Only +/// that hash — a row already showing a newer one is not this refresh's to +/// clear. +async fn unlink_fanart_in( + tx: &mut sqlx::Transaction<'_, sqlx::Sqlite>, + name_canonical: &str, + previous: &str, + now: i64, +) -> Result<(), sqlx::Error> { + let moved = sqlx::query( + "UPDATE artist SET fanart_hash = NULL, updated_at = ? \ + WHERE canonical_name = ? AND fanart_hash = ? \ + RETURNING id, library_id", + ) + .bind(now) + .bind(name_canonical) + .bind(previous) + .fetch_all(&mut **tx) + .await?; + record_artist_moves(tx, moved, now).await +} + +/// One artist event per moved row, in the row's own library. +async fn record_artist_moves( + tx: &mut sqlx::Transaction<'_, sqlx::Sqlite>, + moved: Vec, + now: i64, +) -> Result<(), sqlx::Error> { + for row in moved { + let id: String = row.try_get("id")?; + let library_id: String = row.try_get("library_id")?; + let parse = + |value: &str| Uuid::parse_str(value).map_err(|error| sqlx::Error::Decode(error.into())); + crate::catalog::record_library_event( + tx, + crate::catalog::LibraryChange { + library_id: parse(&library_id)?, + entity_type: "artist", + entity_id: parse(&id)?, + action: "upsert", + payload: serde_json::json!({}), + changed_at: now, + origin_device_id: None, + }, + ) + .await?; + } + Ok(()) +} + #[cfg(test)] mod tests { use super::*; diff --git a/src/enrichment.rs b/src/enrichment.rs index 204a1bbe..b384bc95 100644 --- a/src/enrichment.rs +++ b/src/enrichment.rs @@ -31,6 +31,7 @@ use waveflow_core::metadata::{ deezer::{is_placeholder_artist_picture, DeezerArtistHit, DeezerClient, DeezerError}, lastfm::LastfmClient, name_match::{normalize_name, select_by_name}, + theaudiodb::TheAudioDbClient, LookupError, }; @@ -51,6 +52,19 @@ const DEEZER_BASE_URL: &str = "https://api.deezer.com"; /// naming any other host is dropped without being contacted. const DEEZER_IMAGE_HOSTS: [&str; 2] = ["e-cdns-images.dzcdn.net", "cdn-images.dzcdn.net"]; +/// TheAudioDB's API root, a constant like the others (RFC-014 decision 1). The +/// operator's key travels in the path, so this URL is never logged either. +const THEAUDIODB_BASE_URL: &str = "https://www.theaudiodb.com/api/v1/json"; + +/// The hosts TheAudioDB serves its fanart from (RFC-014 decision 2). A fanart +/// URL naming any other host is dropped without being contacted. +const THEAUDIODB_IMAGE_HOSTS: [&str; 2] = ["r2.theaudiodb.com", "www.theaudiodb.com"]; + +/// TheAudioDB's own ceiling, whatever the shared rate says: thirty requests a +/// minute on its free tier (RFC-014 decision 5). The pace is the slower of this +/// and the operator's. +const THEAUDIODB_INTERVAL: Duration = Duration::from_secs(2); + /// How many neighbours are asked for and kept. Subsonic's `getArtistInfo` /// defaults to twenty; asking for more leaves room for the ones a caller cannot /// see, which decision 7 drops at read time. @@ -73,6 +87,9 @@ pub enum Provider { /// Portraits only: Deezer has no biography, and its related artists are /// not asked for. Deezer, + /// Banners only (RFC-014): TheAudioDB's fanart. Its biographies wait on a + /// language this server does not know. + TheAudioDb, } impl Provider { @@ -80,6 +97,7 @@ impl Provider { match value { "lastfm" => Some(Self::LastFm), "deezer" => Some(Self::Deezer), + "theaudiodb" => Some(Self::TheAudioDb), _ => None, } } @@ -89,6 +107,7 @@ impl Provider { match self { Self::LastFm => "lastfm", Self::Deezer => "deezer", + Self::TheAudioDb => "theaudiodb", } } } @@ -128,6 +147,8 @@ pub struct CachedArtist { /// The portrait this provider supplied, in the artwork store. Cleared by /// the database when the sweeper takes the artwork row. pub image_hash: Option, + /// The banner this provider supplied (RFC-014), cleared the same way. + pub fanart_hash: Option, pub fetched_at: i64, pub expires_at: i64, } @@ -176,6 +197,8 @@ pub(crate) struct ArtistAnswer { pub(crate) bio_full: Option, pub(crate) similar: Vec, pub(crate) image: Option, + /// The banner, from TheAudioDB's fanart (RFC-014). + pub(crate) fanart: Option, } impl ArtistAnswer { @@ -185,6 +208,7 @@ impl ArtistAnswer { || self.bio_full.is_some() || !self.similar.is_empty() || self.image.is_some() + || self.fanart.is_some() { Outcome::Hit } else { @@ -202,6 +226,9 @@ pub struct Endpoints { pub deezer: String, /// The hosts a Deezer picture may be fetched from. pub deezer_image_hosts: Vec, + pub theaudiodb: String, + /// The hosts a TheAudioDB fanart may be fetched from. + pub theaudiodb_image_hosts: Vec, /// The addresses a picture may be fetched from. Only the test suite, /// whose image host is on loopback, sets anything but the default. pub image_addresses: AddressPolicy, @@ -213,6 +240,8 @@ impl Default for Endpoints { lastfm: LASTFM_BASE_URL.to_owned(), deezer: DEEZER_BASE_URL.to_owned(), deezer_image_hosts: DEEZER_IMAGE_HOSTS.map(str::to_owned).to_vec(), + theaudiodb: THEAUDIODB_BASE_URL.to_owned(), + theaudiodb_image_hosts: THEAUDIODB_IMAGE_HOSTS.map(str::to_owned).to_vec(), image_addresses: AddressPolicy::default(), } } @@ -261,6 +290,10 @@ enum Client { client: DeezerClient, image_hosts: Vec, }, + TheAudioDb { + client: TheAudioDbClient, + image_hosts: Vec, + }, } impl Enrichment { @@ -309,11 +342,33 @@ impl Enrichment { client: DeezerClient::with_http(http, endpoints.deezer.clone()), image_hosts: endpoints.deezer_image_hosts.clone(), }, + // Banners are all it is asked for (RFC-014 decision 1). + Provider::TheAudioDb if !settings.images => continue, + Provider::TheAudioDb => { + let key = settings.theaudiodb_key.clone().ok_or_else(|| { + anyhow::anyhow!("metadata provider theaudiodb is configured without a key") + })?; + Client::TheAudioDb { + client: TheAudioDbClient::with_http( + http, + endpoints.theaudiodb.clone(), + key, + ) + .map_err(|_| anyhow::anyhow!("the TheAudioDB address is not https"))?, + image_hosts: endpoints.theaudiodb_image_hosts.clone(), + } + } + }; + let pace = match provider { + Provider::TheAudioDb => { + Pace::with_floor(settings.rate_per_sec, THEAUDIODB_INTERVAL) + } + Provider::LastFm | Provider::Deezer => Pace::new(settings.rate_per_sec), }; sources.push(Source { provider, client, - pace: Pace::new(settings.rate_per_sec), + pace, }); } if sources.is_empty() { @@ -471,6 +526,12 @@ impl Enrichment { tracing::warn!(%error, "cached artist portrait could not be linked"); } } + // And the banner, by the same reasoning (RFC-014 decision 3). + if let Some(hash) = ordered.iter().find_map(|row| row.fanart_hash.as_deref()) { + if let Err(error) = engine.db.link_artist_fanart(&canonical, hash).await { + tracing::warn!(%error, "cached artist banner could not be linked"); + } + } Ok(ordered) } @@ -531,6 +592,10 @@ impl Engine { client, image_hosts, } => self.ask_deezer(source, client, image_hosts, name).await, + Client::TheAudioDb { + client, + image_hosts, + } => self.ask_theaudiodb(source, client, image_hosts, name).await, }; let answer = match answer { Ok(answer) => answer, @@ -610,6 +675,7 @@ impl Engine { bio_full, similar, image: None, + fanart: None, }) } @@ -673,10 +739,67 @@ impl Engine { client, image_hosts, } => Some((source, client, image_hosts.as_slice())), - Client::LastFm { .. } => None, + Client::LastFm { .. } | Client::TheAudioDb { .. } => None, }) } + /// The artist search, then the first fanart that is really one (RFC-014 + /// decision 2). + /// + /// The fanarts are tried in TheAudioDB's order: one the fetcher refuses + /// for what it is gives way to the next, and the first stored ends the + /// search. None accepted, or no artist by that name, is a miss. A picture + /// host that could not be reached is unreached, like the search, and + /// writes nothing: it says nothing about the artist. + async fn ask_theaudiodb( + &self, + source: &Source, + client: &TheAudioDbClient, + image_hosts: &[String], + name: &str, + ) -> Result { + source.pace.wait().await; + // English is asked only because the call takes a language: the + // biography it selects is not kept. + let Some(artist) = self.bounded(client.artist_info(name, "en")).await? else { + return Ok(ArtistAnswer::default()); + }; + for fanart in &artist.fanart_urls { + source.pace.wait().await; + let fetched = portrait::fetch( + fanart, + image_hosts, + self.image_addresses, + self.request_timeout, + ) + .await; + let picture = match fetched { + Ok(picture) => picture, + Err(refusal) if refusal.is_answer() => { + tracing::debug!( + provider = source.provider.as_str(), + verdict = %refusal, + "artist fanart refused; trying the next" + ); + continue; + } + Err(refusal) => return Err(Verdict::Image(refusal)), + }; + portrait::store(&self.artwork_dir, &picture) + .await + .map_err(|_| Verdict::Store)?; + return Ok(ArtistAnswer { + fanart: Some(StoredPortrait { + hash: picture.hash, + format: picture.format, + byte_size: picture.bytes.len() as i64, + }), + ..ArtistAnswer::default() + }); + } + Ok(ArtistAnswer::default()) + } + /// One request, under the request timeout; anything but an answer is a /// verdict. async fn bounded( @@ -796,6 +919,16 @@ impl Pace { } } + /// The operator's rate, but never faster than one request per `floor`: + /// a provider's own ceiling, which the shared setting cannot raise. + fn with_floor(rate_per_sec: u32, floor: Duration) -> Self { + let pace = Self::new(rate_per_sec); + Self { + interval: pace.interval.max(floor), + ..pace + } + } + async fn wait(&self) { let slot = { let mut next = self diff --git a/src/services/artist_enrichment.rs b/src/services/artist_enrichment.rs index 12430168..4865d7f3 100644 --- a/src/services/artist_enrichment.rs +++ b/src/services/artist_enrichment.rs @@ -28,10 +28,13 @@ impl DomainServices { artist_id: Uuid, ) -> Result { let mut detail = self.artist(user_id, artist_id).await?; - detail.enrichment = Some( - self.enrichment_of(user_id, &detail.artist.name, ARTIST_PAGE_SIMILAR) - .await?, - ); + let mut enrichment = self + .enrichment_of(user_id, &detail.artist.name, ARTIST_PAGE_SIMILAR) + .await?; + if detail.image_choice.banner.is_some() { + enrichment.banner = None; + } + detail.enrichment = Some(enrichment); Ok(detail) } @@ -71,6 +74,7 @@ impl DomainServices { .await?; let bio = merged_bio(&rows); let image = merged_image(&rows); + let banner = merged_banner(&rows); // Only a fresh row is an answer. An expired one has just had its // refresh scheduled, and an expired miss is not "nobody knows" but // "being asked again" — `pending`, as decision 11 defines it. Stale @@ -78,7 +82,7 @@ impl DomainServices { let now = crate::authentication::now_ms(); let answered = rows.iter().filter(|row| row.expires_at > now).count(); let state = state_of( - bio.is_some() || image.is_some() || similar.is_some(), + bio.is_some() || image.is_some() || banner.is_some() || similar.is_some(), answered, engine.provider_count(), ); @@ -86,6 +90,7 @@ impl DomainServices { state, bio, image, + banner, similar, }) } @@ -332,6 +337,16 @@ fn merged_image(rows: &[CachedArtist]) -> Option { }) } +/// The first configured provider with a banner supplies it (RFC-014). +fn merged_banner(rows: &[CachedArtist]) -> Option { + rows.iter().find_map(|row| { + row.fanart_hash.as_ref().map(|hash| EnrichedImage { + artwork_hash: hash.clone(), + source: source_of(row), + }) + }) +} + /// Content wins over everything; without it, a provider that has not /// answered makes the lookup `pending`, and only when every one of them has /// answered is it `none`. @@ -365,6 +380,7 @@ mod tests { bio_summary: bio.map(str::to_owned), bio_full: None, image_hash: None, + fanart_hash: None, fetched_at, expires_at: fetched_at + 1, } diff --git a/src/services/artist_images.rs b/src/services/artist_images.rs index 3ac4350d..b488f0e7 100644 --- a/src/services/artist_images.rs +++ b/src/services/artist_images.rs @@ -120,7 +120,8 @@ pub(super) async fn write_choice( } impl DomainServices { - /// The chosen banner and what was chosen, for the artist page. Read after + /// The banner shown and what was chosen, for the artist page: the chosen + /// banner, or else the provider's fanart (RFC-014 decision 4). Read after /// access to the artist was established by the caller. pub(super) async fn artist_image_choices( &self, @@ -149,6 +150,13 @@ impl DomainServices { _ => {} } } + if banner.is_none() { + banner = sqlx::query_scalar("SELECT fanart_hash FROM artist WHERE id=?") + .bind(artist_id.to_string()) + .fetch_optional(self.db.pool()) + .await? + .flatten(); + } Ok((banner, choices)) } diff --git a/src/services/artwork.rs b/src/services/artwork.rs index afab3f07..ec9fbe57 100644 --- a/src/services/artwork.rs +++ b/src/services/artwork.rs @@ -230,6 +230,7 @@ impl DomainServices { let removed = sqlx::query( "DELETE FROM artwork WHERE \ NOT EXISTS (SELECT 1 FROM artist WHERE artist.artwork_hash = artwork.hash) \ + AND NOT EXISTS (SELECT 1 FROM artist WHERE artist.fanart_hash = artwork.hash) \ AND NOT EXISTS (SELECT 1 FROM album WHERE album.artwork_hash = artwork.hash) \ AND NOT EXISTS (SELECT 1 FROM track WHERE track.artwork_hash = artwork.hash) \ AND NOT EXISTS (SELECT 1 FROM artist_image_choice c WHERE c.artwork_hash = artwork.hash) \ diff --git a/src/services/catalog.rs b/src/services/catalog.rs index 00562817..7238d02b 100644 --- a/src/services/catalog.rs +++ b/src/services/catalog.rs @@ -232,6 +232,7 @@ impl DomainServices { SELECT 1 FROM track t JOIN library_member m ON m.library_id=t.library_id WHERE t.artwork_hash=a.hash AND m.user_id=? \ UNION SELECT 1 FROM album al JOIN library_member m ON m.library_id=al.library_id WHERE al.artwork_hash=a.hash AND m.user_id=? \ UNION SELECT 1 FROM artist ar JOIN library_member m ON m.library_id=ar.library_id WHERE ar.artwork_hash=a.hash AND m.user_id=? \ + UNION SELECT 1 FROM artist ar JOIN library_member m ON m.library_id=ar.library_id WHERE ar.fanart_hash=a.hash AND m.user_id=? \ UNION SELECT 1 FROM artist_image_choice c JOIN artist ar ON ar.id=c.artist_id JOIN library_member m ON m.library_id=ar.library_id WHERE c.artwork_hash=a.hash AND m.user_id=? \ UNION SELECT 1 FROM artist_image_candidate c JOIN artist_image_search s ON s.id=c.search_id JOIN artist ar ON ar.id=s.artist_id JOIN library_member m ON m.library_id=ar.library_id WHERE c.artwork_hash=a.hash AND m.user_id=? AND m.role IN ('owner', 'manager') \ ) UNION ALL SELECT a.hash, a.format FROM track t JOIN library_member m ON m.library_id=t.library_id JOIN artwork a ON a.hash=t.artwork_hash WHERE t.id=? AND m.user_id=? \ @@ -241,8 +242,10 @@ impl DomainServices { // A chosen image (RFC-013) is served to the members of the library // whose artist names it, and an artist's id resolves to its effective // portrait, the chosen one first. A Deezer preview is served to those - // who may pick it: the owners and managers of the artist's library. - .bind(id).bind(user_id.to_string()).bind(user_id.to_string()).bind(user_id.to_string()).bind(user_id.to_string()).bind(user_id.to_string()) + // who may pick it: the owners and managers of the artist's library. An + // automatic banner (RFC-014), to the members of a library whose artist + // names it. + .bind(id).bind(user_id.to_string()).bind(user_id.to_string()).bind(user_id.to_string()).bind(user_id.to_string()).bind(user_id.to_string()).bind(user_id.to_string()) .bind(id).bind(user_id.to_string()).bind(id).bind(user_id.to_string()).bind(id).bind(user_id.to_string()) .fetch_optional(self.db.pool()).await?; row.map(|row| Ok::<_, sqlx::Error>((row.try_get("hash")?, row.try_get("format")?))) diff --git a/src/services/mod.rs b/src/services/mod.rs index fa584814..a2d04184 100644 --- a/src/services/mod.rs +++ b/src/services/mod.rs @@ -972,9 +972,9 @@ pub struct ArtistDetail { /// the prefill decision 6 refuses. #[serde(skip_serializing_if = "Option::is_none")] pub enrichment: Option, - /// The banner chosen for this artist by hand (RFC-013 decision 3), absent - /// when none was. There is no automatic banner: a client without one - /// draws what it drew before. + /// The banner this artist shows: the one chosen by hand (RFC-013 + /// decision 3), or else TheAudioDB's fanart (RFC-014 decision 4). Absent + /// when there is neither, and a client then draws what it drew before. #[serde(skip_serializing_if = "Option::is_none")] pub banner_hash: Option, /// What was chosen by hand, kind by kind, so a client can say "chosen" @@ -1015,6 +1015,10 @@ pub struct ArtistEnrichment { pub bio: Option, /// The first configured provider with a portrait supplies it. pub image: Option, + /// The automatic banner and where it came from (RFC-014 decision 4). + /// Present only while it is the banner shown: a banner chosen by hand + /// came from no provider, and this must not say otherwise. + pub banner: Option, /// The first configured provider whose neighbours include artists the /// caller can see supplies the list — those artists only, best first. pub similar: Option, @@ -1026,6 +1030,7 @@ impl ArtistEnrichment { state: EnrichmentState::Disabled, bio: None, image: None, + banner: None, similar: None, } } @@ -1063,7 +1068,7 @@ pub struct EnrichedBio { pub source: EnrichmentSource, } -/// A portrait, as a key into the artwork store. +/// A portrait or a banner, as a key into the artwork store. #[derive(Debug, Clone, Serialize, ToSchema)] pub struct EnrichedImage { /// Served by the artwork route, as every `artwork_hash` is. diff --git a/tests/enrichment.rs b/tests/enrichment.rs index 038dfc7d..f7346bd7 100644 --- a/tests/enrichment.rs +++ b/tests/enrichment.rs @@ -2077,3 +2077,333 @@ async fn a_search_outlives_the_ordinary_request_timeout() { 20 ); } + +// ── Banners: TheAudioDB's fanart (RFC-014) ── + +/// What the fake TheAudioDB answers to a search. +#[derive(Clone, Copy, PartialEq, Eq)] +enum Fanarts { + /// Björk, with four fanarts: one on a host off the allowlist, one that is + /// not an image, then two real ones. + Four, + /// Björk, with one real fanart. + One, + /// `{"artists": null}`: TheAudioDB knows nobody by that name. + Nobody, + /// A `429`, as TheAudioDB answers a key over its ceiling. + Limited, +} + +#[derive(Clone)] +struct AudioDb { + mode: Arc>, + searches: Arc, + images: Arc, + /// The key each search carried in its path. + keys: Arc>>, + port: u16, + /// Made up for the run, so two runs never share a hash. + token: Arc, +} + +impl AudioDb { + /// The bytes the fake serves for the picture called `name`: a JPEG by its + /// first bytes, which is all the fetcher checks. + fn picture(&self, name: &str) -> Vec { + let mut bytes = vec![0xff, 0xd8, 0xff, 0xe0]; + bytes.extend_from_slice(self.token.as_bytes()); + bytes.extend_from_slice(name.as_bytes()); + bytes + } +} + +async fn audiodb_search( + State(fake): State, + axum::extract::Path(key): axum::extract::Path, +) -> Response { + fake.searches.fetch_add(1, Ordering::SeqCst); + fake.keys.lock().unwrap().push(key); + let port = fake.port; + let picture = |name: &str| format!("http://127.0.0.1:{port}/img/{name}.jpg"); + let artist = match *fake.mode.lock().unwrap() { + Fanarts::Limited => return StatusCode::TOO_MANY_REQUESTS.into_response(), + Fanarts::Nobody => return json(r#"{"artists":null}"#.to_owned()), + Fanarts::Four => serde_json::json!({ + "strArtist": "Björk", + "strArtistFanart": format!("http://localhost:{port}/img/elsewhere.jpg"), + "strArtistFanart2": picture("broken"), + "strArtistFanart3": picture("second"), + "strArtistFanart4": picture("third"), + "strArtistBanner": picture("strip"), + }), + Fanarts::One => serde_json::json!({ + "strArtist": "Björk", + "strArtistFanart": picture("only"), + }), + }; + json(serde_json::json!({ "artists": [artist] }).to_string()) +} + +async fn audiodb_picture( + State(fake): State, + axum::extract::Path(name): axum::extract::Path, +) -> Response { + fake.images.fetch_add(1, Ordering::SeqCst); + let bytes = if name == "broken.jpg" { + b"no picture here".to_vec() + } else { + fake.picture(&name) + }; + ([(header::CONTENT_TYPE, "image/jpeg")], bytes).into_response() +} + +async fn fake_audiodb(mode: Fanarts) -> (AudioDb, String) { + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let port = listener.local_addr().unwrap().port(); + let fake = AudioDb { + mode: Arc::new(Mutex::new(mode)), + searches: Arc::default(), + images: Arc::default(), + keys: Arc::default(), + port, + token: Arc::new(run_key()), + }; + let router = Router::new() + .route("/{key}/search.php", get(audiodb_search)) + .route("/img/{name}", get(audiodb_picture)) + .with_state(fake.clone()); + tokio::spawn(async move { axum::serve(listener, router).await.unwrap() }); + (fake, format!("http://127.0.0.1:{port}")) +} + +fn audiodb_settings(key: Option<&str>) -> MetadataSettings { + MetadataSettings { + providers: vec![Provider::TheAudioDb], + theaudiodb_key: key.map(str::to_owned), + ..settings(Vec::new(), &run_key()) + } +} + +fn audiodb_engine( + state: &waveflow_server::AppState, + base_url: &str, + settings: &MetadataSettings, +) -> anyhow::Result { + Enrichment::with_endpoints( + state.db.clone(), + settings, + state.artwork_dir.clone(), + Endpoints { + theaudiodb: base_url.to_owned(), + theaudiodb_image_hosts: vec!["127.0.0.1".to_owned()], + image_addresses: AddressPolicy::AllowLoopback, + ..Endpoints::default() + }, + ) +} + +async fn fanarts_of(state: &waveflow_server::AppState) -> Vec> { + sqlx::query_scalar("SELECT fanart_hash FROM artist WHERE name = 'Björk' ORDER BY id") + .fetch_all(state.db.pool()) + .await + .unwrap() +} + +async fn artwork_rows(state: &waveflow_server::AppState, hash: &str) -> i64 { + sqlx::query_scalar("SELECT COUNT(*) FROM artwork WHERE hash = ?") + .bind(hash) + .fetch_one(state.db.pool()) + .await + .unwrap() +} + +/// Asks again: the cache row is made stale, the artist visited, the fill +/// awaited. +async fn refill(state: &waveflow_server::AppState, enrichment: &Enrichment) { + sqlx::query("UPDATE metadata_artist SET expires_at = 0") + .execute(state.db.pool()) + .await + .unwrap(); + enrichment.artist("Björk").await.unwrap(); + enrichment.idle().await; +} + +/// RFC-014 decisions 1 to 5 end to end. The operator's key is the one sent; +/// the fanarts are tried in order, the one off the allowlist never contacted +/// and the one that is not an image passed over, and the first real one is +/// linked on every artist with the name, one event each; the strip TheAudioDB +/// calls a banner is never asked. The artist page shows it as the banner and +/// credits TheAudioDB, the artwork route serves it to a member, and the +/// requests keep TheAudioDB's own pace however fast the shared rate is. +#[tokio::test] +async fn a_banner_is_the_first_real_fanart_and_is_shown_on_every_artist() { + let (_temp, state, owner, other) = bjork_twice().await; + let (fake, base_url) = fake_audiodb(Fanarts::Four).await; + let key = run_key(); + let enrichment = audiodb_engine(&state, &base_url, &audiodb_settings(Some(&key))).unwrap(); + let started = std::time::Instant::now(); + enrichment.artist("Björk").await.unwrap(); + enrichment.idle().await; + let took = started.elapsed(); + + assert_eq!(*fake.keys.lock().unwrap(), [key]); + // The one that is not an image and the first real one; neither the host + // off the allowlist, nor the third, nor the strip. + assert_eq!(fake.images.load(Ordering::SeqCst), 2); + // Three requests at one every two seconds, the shared rate notwithstanding. + assert!(took >= Duration::from_secs(4), "three requests in {took:?}"); + let linked = fanarts_of(&state).await; + assert_eq!(linked.len(), 2); + let hash = linked[0].clone().expect("the banner is linked"); + assert_eq!(linked[1].as_deref(), Some(hash.as_str())); + assert_eq!( + hash, + waveflow_server::portrait::Portrait::from_bytes(fake.picture("second.jpg")) + .unwrap() + .hash + ); + assert!(state.artwork_dir.join(format!("{hash}.jpg")).is_file()); + assert_eq!(artist_events(&state).await, 2); + + state.services.use_enrichment(enrichment); + let bjork = artist_id(&state, owner, "Björk").await; + let page = state.services.artist_page(owner, bjork).await.unwrap(); + assert_eq!(page.banner_hash.as_deref(), Some(hash.as_str())); + let banner = page + .enrichment + .expect("the native page always carries it") + .banner + .expect("the automatic banner is credited"); + assert_eq!(banner.artwork_hash, hash); + assert_eq!(banner.source.provider, "theaudiodb"); + assert!(state + .services + .artwork_for_user(other, &hash) + .await + .unwrap() + .is_some()); +} + +/// A banner chosen by hand wins, and is not credited to a provider; removing +/// it brings TheAudioDB's back (RFC-014 decision 4, amending RFC-013's +/// decision 5). +#[tokio::test] +async fn a_chosen_banner_wins_and_the_fanart_comes_back_when_it_goes() { + let (_temp, state, owner, _other) = bjork_twice().await; + let (_fake, base_url) = fake_audiodb(Fanarts::One).await; + let enrichment = + audiodb_engine(&state, &base_url, &audiodb_settings(Some(&run_key()))).unwrap(); + enrichment.artist("Björk").await.unwrap(); + enrichment.idle().await; + state.services.use_enrichment(enrichment); + let bjork = artist_id(&state, owner, "Björk").await; + let fanart = fanarts_of(&state).await[0].clone().unwrap(); + + let mut chosen = portrait_bytes(); + chosen.extend_from_slice(b"chosen"); + state + .services + .choose_artist_image( + owner, + bjork, + waveflow_server::services::ArtistImageKind::Banner, + chosen, + ) + .await + .unwrap(); + let page = state.services.artist_page(owner, bjork).await.unwrap(); + assert_ne!(page.banner_hash.as_deref(), Some(fanart.as_str())); + assert!(page.banner_hash.is_some()); + assert!(page.enrichment.unwrap().banner.is_none()); + + state + .services + .remove_artist_image( + owner, + bjork, + waveflow_server::services::ArtistImageKind::Banner, + ) + .await + .unwrap(); + let page = state.services.artist_page(owner, bjork).await.unwrap(); + assert_eq!(page.banner_hash.as_deref(), Some(fanart.as_str())); + assert!(page.enrichment.unwrap().banner.is_some()); +} + +/// RFC-014 decisions 3 and 6: the sweep keeps a linked banner. A refresh that +/// cannot reach TheAudioDB — a 429 — unlinks nothing; one that comes back a +/// miss unlinks it from every artist, one event each, and the sweep then +/// takes its row. +#[tokio::test] +async fn a_miss_unlinks_the_banner_and_an_unreached_refresh_does_not() { + let (_temp, state, _owner, _other) = bjork_twice().await; + let (fake, base_url) = fake_audiodb(Fanarts::One).await; + let enrichment = + audiodb_engine(&state, &base_url, &audiodb_settings(Some(&run_key()))).unwrap(); + enrichment.artist("Björk").await.unwrap(); + enrichment.idle().await; + let fanart = fanarts_of(&state).await[0].clone().unwrap(); + state.services.sweep_artwork_store().await.unwrap(); + assert_eq!(artwork_rows(&state, &fanart).await, 1); + assert_eq!(artist_events(&state).await, 2); + + *fake.mode.lock().unwrap() = Fanarts::Limited; + refill(&state, &enrichment).await; + assert_eq!( + fanarts_of(&state).await, + [Some(fanart.clone()), Some(fanart.clone())] + ); + assert_eq!(artist_events(&state).await, 2); + + *fake.mode.lock().unwrap() = Fanarts::Nobody; + refill(&state, &enrichment).await; + assert_eq!(fanarts_of(&state).await, [None, None]); + assert_eq!(artist_events(&state).await, 4); + state.services.sweep_artwork_store().await.unwrap(); + assert_eq!(artwork_rows(&state, &fanart).await, 0); + assert_eq!(fake.searches.load(Ordering::SeqCst), 3); +} + +/// RFC-014 decision 1: TheAudioDB is not built without the operator's key, +/// nor with images off, since banners are all it is asked for. +#[tokio::test] +async fn theaudiodb_needs_its_key_and_images_on() { + let (_temp, _config, state) = support::test_app().await; + let (fake, base_url) = fake_audiodb(Fanarts::One).await; + assert!(audiodb_engine(&state, &base_url, &audiodb_settings(None)).is_err()); + + let off = MetadataSettings { + images: false, + ..audiodb_settings(Some(&run_key())) + }; + let enrichment = audiodb_engine(&state, &base_url, &off).unwrap(); + assert!(!enrichment.is_enabled()); + enrichment.artist("Björk").await.unwrap(); + enrichment.idle().await; + assert_eq!(fake.searches.load(Ordering::SeqCst), 0); +} + +/// An artist row that lost its banner — re-identified, or scanned since — is +/// linked again from the cache on the next visit, without a second download, +/// as a portrait is. +#[tokio::test] +async fn a_cached_banner_is_linked_again_without_a_second_download() { + let (_temp, state, _owner, _other) = bjork_twice().await; + let (fake, base_url) = fake_audiodb(Fanarts::One).await; + let enrichment = + audiodb_engine(&state, &base_url, &audiodb_settings(Some(&run_key()))).unwrap(); + enrichment.artist("Björk").await.unwrap(); + enrichment.idle().await; + let linked = fanarts_of(&state).await; + sqlx::query("UPDATE artist SET fanart_hash = NULL") + .execute(state.db.pool()) + .await + .unwrap(); + + enrichment.artist("Björk").await.unwrap(); + enrichment.idle().await; + + assert!(linked[0].is_some()); + assert_eq!(fanarts_of(&state).await, linked); + assert_eq!(fake.images.load(Ordering::SeqCst), 1); +} diff --git a/webapp/e2e/studio-nocturne.spec.ts b/webapp/e2e/studio-nocturne.spec.ts index 453b6518..698a46c5 100644 --- a/webapp/e2e/studio-nocturne.spec.ts +++ b/webapp/e2e/studio-nocturne.spec.ts @@ -5463,6 +5463,56 @@ test("shows a listener the artist's banner with nothing to change", async ({ await expect(hero.getByRole("button", { name: "Search Deezer" })).toHaveCount(0); }); +/** + * The automatic banner (RFC-014): TheAudioDB's fanart is drawn behind the + * header and credited to TheAudioDB, whose terms ask for it; a banner chosen + * by hand is credited to nobody. + */ +test("credits TheAudioDB for an automatic banner and nobody for a chosen one", async ({ + page, +}) => { + await serveArtistImages(page); + let chosen = false; + await page.route("**/api/v2/artists/artist-1", (route) => + route.fulfill({ + json: { + ...artistDetail, + library_id: "library-1", + banner_hash: (chosen ? "b" : "c").repeat(64), + ...(chosen + ? { image_choice: { banner: { source: "upload", set_at: 1 } } } + : {}), + enrichment: { + state: "ready", + bio: null, + image: null, + similar: null, + banner: chosen + ? null + : { + artwork_hash: "c".repeat(64), + source: { + provider: "theaudiodb", + fetched_at: Date.UTC(2026, 9, 24), + }, + }, + }, + }, + }), + ); + + await page.goto("/artists/artist-1"); + const hero = page.locator(".artist-hero"); + await expect(hero.locator("img.artist-banner")).toBeVisible(); + await expect(hero.getByText(/^Banner from TheAudioDB, /)).toBeVisible(); + await expectNoWcagViolations(page); + + chosen = true; + await page.reload(); + await expect(hero.locator("img.artist-banner")).toBeVisible(); + await expect(hero.getByText(/^Banner from /)).toHaveCount(0); +}); + /** * Choosing the portrait from Deezer (RFC-013 step 4): the picker searches the * artist's name as it opens, shows previews the server holds and nothing from diff --git a/webapp/src/api/catalog.ts b/webapp/src/api/catalog.ts index e692a9fc..f2ed2889 100644 --- a/webapp/src/api/catalog.ts +++ b/webapp/src/api/catalog.ts @@ -81,6 +81,11 @@ export type ArtistEnrichment = { source: EnrichmentSource; } | null; image: { artwork_hash: string; source: EnrichmentSource } | null; + /** + * The automatic banner and its provider (RFC-014), present only while it is + * the banner shown. Optional so an older server parses. + */ + banner?: { artwork_hash: string; source: EnrichmentSource } | null; similar: { artists: Artist[]; source: EnrichmentSource } | null; }; diff --git a/webapp/src/pages/artist-page.tsx b/webapp/src/pages/artist-page.tsx index c7483f06..081b84cd 100644 --- a/webapp/src/pages/artist-page.tsx +++ b/webapp/src/pages/artist-page.tsx @@ -19,6 +19,7 @@ import { mayCorrectTracks, useLibraryScope } from "../state/library-scope"; const PROVIDER_NAMES: Record = { lastfm: "Last.fm", deezer: "Deezer", + theaudiodb: "TheAudioDB", }; /** @@ -115,9 +116,11 @@ export function ArtistPage({ artistId }: { artistId: string }) { // The artist row carries the portrait once the server has linked it; the // enrichment names it as soon as it is cached, which may be a visit sooner. const portrait = value.artwork_hash ?? image?.artwork_hash ?? null; - // The banner chosen by hand (RFC-013), or else the portrait, blurred, as the - // desktop draws it: a header that carries the artist even with no banner. + // The banner chosen by hand (RFC-013) or TheAudioDB's fanart (RFC-014), or + // else the portrait, blurred, as the desktop draws it: a header that carries + // the artist even with no banner. const banner = value.banner_hash ?? null; + const bannerSource = value.enrichment?.banner ?? null; const backdrop = banner ?? portrait; // Offered where the server would take it: owners and managers. const mayChoose = mayCorrectTracks( @@ -168,6 +171,22 @@ export function ArtistPage({ artistId }: { artistId: string }) { })}

) : null} + {/* Credited only while it is the banner shown: a chosen one came + from no provider. TheAudioDB's terms ask for the credit. */} + {bannerSource && + !value.image_choice?.banner && + bannerSource.artwork_hash === banner ? ( +

+ {t("artists.bannerSource", { + provider: + PROVIDER_NAMES[bannerSource.source.provider] ?? + bannerSource.source.provider, + date: new Intl.DateTimeFormat(locale, { + dateStyle: "medium", + }).format(new Date(bannerSource.source.fetched_at)), + })} +

+ ) : null} {mayChoose ? : null} diff --git a/webapp/src/state/i18n.tsx b/webapp/src/state/i18n.tsx index e834244d..4426e224 100644 --- a/webapp/src/state/i18n.tsx +++ b/webapp/src/state/i18n.tsx @@ -270,6 +270,7 @@ const en = { "artists.bioSource": "Biography from {provider}, {date}.", "artists.similarSource": "Suggested by {provider}, {date}.", "artists.portraitSource": "Portrait from {provider}, {date}.", + "artists.bannerSource": "Banner from {provider}, {date}.", "favourites.detail": { one: "{count} saved track", other: "{count} saved tracks", @@ -984,6 +985,7 @@ const fr: Record = { "artists.bioSource": "Biographie fournie par {provider}, le {date}.", "artists.similarSource": "Suggérés par {provider}, le {date}.", "artists.portraitSource": "Portrait fourni par {provider}, le {date}.", + "artists.bannerSource": "Bannière fournie par {provider}, le {date}.", "favourites.detail": { one: "{count} piste enregistrée", other: "{count} pistes enregistrées", From 55f65294cf048e677fc3c04403a63eccab438920 Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Sat, 24 Oct 2026 04:02:17 +0200 Subject: [PATCH 2/3] docs: RFC-014 names #314 in its Implemented by line Signed-off-by: InstaZDLL --- docs/rfcs/RFC-014-automatic-artist-banner.md | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/docs/rfcs/RFC-014-automatic-artist-banner.md b/docs/rfcs/RFC-014-automatic-artist-banner.md index 71597bae..3b3f6b93 100644 --- a/docs/rfcs/RFC-014-automatic-artist-banner.md +++ b/docs/rfcs/RFC-014-automatic-artist-banner.md @@ -3,8 +3,12 @@ - **Status**: Proposed - **Implemented by**: in progress, in the order of "The order of the work" below: [#312](https://github.com/InstaZDLL/waveflow-server/pull/312) (this - RFC). The *Status* field above never flips in this project; this line is the - one that says what is running. + RFC) and [#314](https://github.com/InstaZDLL/waveflow-server/pull/314) + (steps 2 and 3 together: the `theaudiodb` provider and its key, the fill and + its pace, the fanart columns, linking and unlinking, the effective banner + and `enrichment.banner`, the sweep, and the credit in the web client). The + *Status* field above never flips in this project; this line is the one that + says what is running. - **Date**: 2026-10-24 - **Authors**: the WaveFlow project - **Depends on**: [RFC-011](RFC-011-metadata-enrichment.md) (the providers, the From dd949a14e943febd03e4a8a484a4edd6dd83bbf0 Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Sat, 24 Oct 2026 04:18:58 +0200 Subject: [PATCH 3/3] fix: credit an automatic banner only while it is shown, from both entry points artist_enrichment, which getArtistInfo reads, now drops enrichment.banner when a banner was chosen by hand, as the artist page did: the rule moves into enrichment_of so both entry points answer the same. The web client falls back on enrichment.banner when the artist row is not linked yet, as it already does for the portrait. Signed-off-by: InstaZDLL --- src/services/artist_enrichment.rs | 29 ++++++++++++++++++++--------- tests/enrichment.rs | 7 +++++++ webapp/e2e/studio-nocturne.spec.ts | 12 +++++++++++- webapp/src/pages/artist-page.tsx | 4 +++- 4 files changed, 41 insertions(+), 11 deletions(-) diff --git a/src/services/artist_enrichment.rs b/src/services/artist_enrichment.rs index 4865d7f3..63cab2f4 100644 --- a/src/services/artist_enrichment.rs +++ b/src/services/artist_enrichment.rs @@ -28,13 +28,10 @@ impl DomainServices { artist_id: Uuid, ) -> Result { let mut detail = self.artist(user_id, artist_id).await?; - let mut enrichment = self - .enrichment_of(user_id, &detail.artist.name, ARTIST_PAGE_SIMILAR) - .await?; - if detail.image_choice.banner.is_some() { - enrichment.banner = None; - } - detail.enrichment = Some(enrichment); + detail.enrichment = Some( + self.enrichment_of(user_id, artist_id, &detail.artist.name, ARTIST_PAGE_SIMILAR) + .await?, + ); Ok(detail) } @@ -46,7 +43,7 @@ impl DomainServices { similar_limit: usize, ) -> Result { let summary = self.artist_summary(user_id, artist_id).await?; - self.enrichment_of(user_id, &summary.artist.name, similar_limit) + self.enrichment_of(user_id, artist_id, &summary.artist.name, similar_limit) .await } @@ -54,6 +51,7 @@ impl DomainServices { async fn enrichment_of( &self, user_id: Uuid, + artist_id: Uuid, name: &str, similar_limit: usize, ) -> Result { @@ -74,7 +72,20 @@ impl DomainServices { .await?; let bio = merged_bio(&rows); let image = merged_image(&rows); - let banner = merged_banner(&rows); + // Credited only while it is the banner shown (RFC-014 decision 4): a + // banner chosen by hand came from no provider, whichever entry point + // asks. + let chosen_banner: bool = sqlx::query_scalar( + "SELECT EXISTS (SELECT 1 FROM artist_image_choice WHERE artist_id=? AND kind='banner')", + ) + .bind(artist_id.to_string()) + .fetch_one(self.db.pool()) + .await?; + let banner = if chosen_banner { + None + } else { + merged_banner(&rows) + }; // Only a fresh row is an answer. An expired one has just had its // refresh scheduled, and an expired miss is not "nobody knows" but // "being asked again" — `pending`, as decision 11 defines it. Stale diff --git a/tests/enrichment.rs b/tests/enrichment.rs index f7346bd7..f2e20992 100644 --- a/tests/enrichment.rs +++ b/tests/enrichment.rs @@ -2315,6 +2315,13 @@ async fn a_chosen_banner_wins_and_the_fanart_comes_back_when_it_goes() { assert_ne!(page.banner_hash.as_deref(), Some(fanart.as_str())); assert!(page.banner_hash.is_some()); assert!(page.enrichment.unwrap().banner.is_none()); + // The same rule from the other entry point, the one getArtistInfo reads. + let enrichment = state + .services + .artist_enrichment(owner, bjork, 20) + .await + .unwrap(); + assert!(enrichment.banner.is_none()); state .services diff --git a/webapp/e2e/studio-nocturne.spec.ts b/webapp/e2e/studio-nocturne.spec.ts index 698a46c5..67313680 100644 --- a/webapp/e2e/studio-nocturne.spec.ts +++ b/webapp/e2e/studio-nocturne.spec.ts @@ -5473,12 +5473,17 @@ test("credits TheAudioDB for an automatic banner and nobody for a chosen one", a }) => { await serveArtistImages(page); let chosen = false; + // Cached but not yet linked on the artist row: the enrichment alone names + // it, as it can on the visit after an artist is re-identified. + let linked = false; await page.route("**/api/v2/artists/artist-1", (route) => route.fulfill({ json: { ...artistDetail, library_id: "library-1", - banner_hash: (chosen ? "b" : "c").repeat(64), + ...(chosen || linked + ? { banner_hash: (chosen ? "b" : "c").repeat(64) } + : {}), ...(chosen ? { image_choice: { banner: { source: "upload", set_at: 1 } } } : {}), @@ -5505,6 +5510,11 @@ test("credits TheAudioDB for an automatic banner and nobody for a chosen one", a const hero = page.locator(".artist-hero"); await expect(hero.locator("img.artist-banner")).toBeVisible(); await expect(hero.getByText(/^Banner from TheAudioDB, /)).toBeVisible(); + + linked = true; + await page.reload(); + await expect(hero.locator("img.artist-banner")).toBeVisible(); + await expect(hero.getByText(/^Banner from TheAudioDB, /)).toBeVisible(); await expectNoWcagViolations(page); chosen = true; diff --git a/webapp/src/pages/artist-page.tsx b/webapp/src/pages/artist-page.tsx index 081b84cd..b8afeaaa 100644 --- a/webapp/src/pages/artist-page.tsx +++ b/webapp/src/pages/artist-page.tsx @@ -119,8 +119,10 @@ export function ArtistPage({ artistId }: { artistId: string }) { // The banner chosen by hand (RFC-013) or TheAudioDB's fanart (RFC-014), or // else the portrait, blurred, as the desktop draws it: a header that carries // the artist even with no banner. - const banner = value.banner_hash ?? null; + // The enrichment names an automatic banner as soon as it is cached, which + // may be a visit before the artist row is linked again — as the portrait. const bannerSource = value.enrichment?.banner ?? null; + const banner = value.banner_hash ?? bannerSource?.artwork_hash ?? null; const backdrop = banner ?? portrait; // Offered where the server would take it: owners and managers. const mayChoose = mayCorrectTracks(