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/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 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..63cab2f4 100644 --- a/src/services/artist_enrichment.rs +++ b/src/services/artist_enrichment.rs @@ -29,7 +29,7 @@ impl DomainServices { ) -> 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) + self.enrichment_of(user_id, artist_id, &detail.artist.name, ARTIST_PAGE_SIMILAR) .await?, ); Ok(detail) @@ -43,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 } @@ -51,6 +51,7 @@ impl DomainServices { async fn enrichment_of( &self, user_id: Uuid, + artist_id: Uuid, name: &str, similar_limit: usize, ) -> Result { @@ -71,6 +72,20 @@ impl DomainServices { .await?; let bio = merged_bio(&rows); let image = merged_image(&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 @@ -78,7 +93,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 +101,7 @@ impl DomainServices { state, bio, image, + banner, similar, }) } @@ -332,6 +348,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 +391,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..f2e20992 100644 --- a/tests/enrichment.rs +++ b/tests/enrichment.rs @@ -2077,3 +2077,340 @@ 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()); + // 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 + .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..67313680 100644 --- a/webapp/e2e/studio-nocturne.spec.ts +++ b/webapp/e2e/studio-nocturne.spec.ts @@ -5463,6 +5463,66 @@ 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; + // 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", + ...(chosen || linked + ? { 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(); + + 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; + 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..b8afeaaa 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,13 @@ 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. - const banner = value.banner_hash ?? null; + // 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. + // 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( @@ -168,6 +173,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",