diff --git a/docs/operations.md b/docs/operations.md index e3e1523..9d66821 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -182,9 +182,9 @@ 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. Today the only provider is `lastfm` | +| `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_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 | +| `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_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 | @@ -201,9 +201,22 @@ request carries the artist's name and your key, and no redirect is followed. Requests are spaced to the rate above, and simultaneous visits to one artist share one request. +With `deezer` listed, also `api.deezer.com`, which needs no key, and then the +picture it names — but only from Deezer's image hosts +(`e-cdns-images.dzcdn.net`, `cdn-images.dzcdn.net`), only over https, only at +a public address (the address checked is the one connected to), only a JPEG, +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. + **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, whether -the provider knew the artist, and when to ask again. Nothing in it belongs to an +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. +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 +extracted again from your files, only downloaded again once the cache entry +expires. Nothing in it belongs to an account, and nothing records who asked. A failed request stores nothing, so the next visit asks again. diff --git a/docs/rfcs/RFC-007-library-event-stream.md b/docs/rfcs/RFC-007-library-event-stream.md index c7a5a34..74a8017 100644 --- a/docs/rfcs/RFC-007-library-event-stream.md +++ b/docs/rfcs/RFC-007-library-event-stream.md @@ -231,6 +231,18 @@ over this server and **written nowhere**. An artist photo cannot change because there never is one. The question reopens the day something writes it, and on that day this is the line to re-read. +> **Amended 2026-10-16 (#268): an artist now emits one, for its portrait only.** +> [RFC-011](RFC-011-metadata-enrichment.md) decision 8 writes +> `artist.artwork_hash` — a portrait fetched from a metadata provider — so the +> case the desktop reserved exists. The rule is the album's: an `upsert` with an +> empty payload, emitted **only when the hash actually moved**, by the same +> `UPDATE … WHERE artwork_hash IS NOT ? RETURNING` pattern, so a refresh that +> fetched the same picture says nothing. Artist rows are per library, so a +> portrait shared by a name emits one event per row, each in its own library. +> `library_event.entity_type` admitted `'artist'` from the start, so no +> migration was needed. The counters stay derived and still emit nothing: the +> reasoning above holds for everything but the photo. + ## Decision 7 — two bounds, because one alone is always wrong > **Built.** See the *Implemented by* line in the header. diff --git a/docs/rfcs/RFC-011-metadata-enrichment.md b/docs/rfcs/RFC-011-metadata-enrichment.md index 4f471ef..32cb361 100644 --- a/docs/rfcs/RFC-011-metadata-enrichment.md +++ b/docs/rfcs/RFC-011-metadata-enrichment.md @@ -6,9 +6,11 @@ InstaZDLL/WaveFlow#855 (the core's clients accept a client, an address and a key), #270 (the pin moved onto them), #271 (the cache and the engine that fills it), #272 (`getArtistInfo`, `getArtistInfo2` and the native artist - page read it), and the pull request closing #267 (`getSimilarSongs` and - `getSimilarSongs2` draw from it). Portraits are still to come. The *Status* field above never flips in this - project; this line is the one that says what is running. + page read it), #273 and #274 (`getSimilarSongs` and `getSimilarSongs2` draw + from it), and the pull request closing #268 (portraits from Deezer). The web + client and the compatibility campaign are still to come. The *Status* field + above never flips in this project; this line is the one that says what is + running. - **Date**: 2026-10-14 - **Revised**: 2026-10-15, while implementing the cache (#265). Decisions 4, 6 and 10 rested on attachment points the code does not hold; each correction diff --git a/migrations-v2/20261016000000_metadata_artist_image.sql b/migrations-v2/20261016000000_metadata_artist_image.sql new file mode 100644 index 0000000..1dfac03 --- /dev/null +++ b/migrations-v2/20261016000000_metadata_artist_image.sql @@ -0,0 +1,10 @@ +-- RFC-011 decision 8: the portrait a provider pointed at, once fetched. +-- +-- The hash is kept on the provider's cache row as well as on `artist`, so an +-- artist re-identified by a PID spec change is linked again without a second +-- download. `ON DELETE SET NULL` is what makes that safe: the sweeper counts +-- `artist`, `album` and `track` as references and not this column, so an +-- artwork row nobody shows may go, and the cache row then forgets it rather +-- than naming a file that no longer exists. The next refresh fetches it again. +ALTER TABLE metadata_artist + ADD COLUMN image_hash TEXT REFERENCES artwork(hash) ON DELETE SET NULL; diff --git a/src/config.rs b/src/config.rs index 0f3b3d0..a1f2062 100644 --- a/src/config.rs +++ b/src/config.rs @@ -200,7 +200,8 @@ pub struct ScrobbleDestination { #[derive(Clone)] pub struct MetadataSettings { /// `WAVEFLOW_METADATA_PROVIDERS`: an ordered, comma-separated list, empty - /// by default. The order is the order fields are merged in when read. + /// by default, of `lastfm` and `deezer`. 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 @@ -1246,6 +1247,15 @@ mod tests { 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()); + // Deezer asks for no key, so none is required of it. + assert_eq!( + parse_metadata_providers("lastfm, deezer", true).unwrap(), + [Provider::LastFm, Provider::Deezer] + ); + assert_eq!( + parse_metadata_providers("deezer", false).unwrap(), + [Provider::Deezer] + ); } #[test] diff --git a/src/database.rs b/src/database.rs index d0557d4..ca12221 100644 --- a/src/database.rs +++ b/src/database.rs @@ -1917,7 +1917,7 @@ impl Database { name_canonical: &str, ) -> Result, sqlx::Error> { let rows = sqlx::query( - "SELECT provider, outcome, bio_summary, bio_full, fetched_at, expires_at \ + "SELECT provider, outcome, bio_summary, bio_full, image_hash, fetched_at, expires_at \ FROM metadata_artist WHERE name_canonical = ?", ) .bind(name_canonical) @@ -1938,6 +1938,7 @@ impl Database { outcome, bio_summary: row.try_get("bio_summary")?, bio_full: row.try_get("bio_full")?, + image_hash: row.try_get("image_hash")?, fetched_at: row.try_get("fetched_at")?, expires_at: row.try_get("expires_at")?, }); @@ -1970,12 +1971,51 @@ impl Database { .collect() } - /// Records what a provider answered: its row and its whole neighbour list, + /// Links the portrait `hash` to every artist called `name_canonical` that + /// does not show it yet, emitting one artist event per row it moved. + /// + /// Read first without the writer gate, since this runs on the read path + /// and almost always finds nothing to do. The artwork row is checked in + /// the same statement as the link: the sweeper may have taken it since + /// the cache row was read. + pub(crate) async fn link_artist_portrait( + &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 artwork_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_portrait_in( + &mut tx, + name_canonical, + 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. /// - /// The writer gate is taken here and nowhere earlier — the request that - /// produced `answer` is over before this is called, so no network I/O ever - /// runs under it. + /// One transaction is what keeps the sweeper from ever seeing the artwork + /// row unreferenced: it deletes under this same gate, so it runs wholly + /// before or wholly after. The writer gate is taken here and nowhere + /// earlier — the requests that produced `answer` are over, and the + /// portrait's file is already in the store, so no network or file I/O + /// ever runs under it. pub(crate) async fn write_metadata_artist( &self, provider: crate::enrichment::Provider, @@ -1986,24 +2026,42 @@ impl Database { ) -> Result<(), sqlx::Error> { let _writer = self.writer_guard().await; let mut tx = self.pool.begin().await?; + if let Some(image) = &answer.image { + sqlx::query( + "INSERT INTO artwork (hash, format, source, byte_size, created_at) \ + VALUES (?, ?, ?, ?, ?) ON CONFLICT (hash) DO NOTHING", + ) + .bind(&image.hash) + .bind(image.format) + .bind(provider.as_str()) + .bind(image.byte_size) + .bind(fetched_at) + .execute(&mut *tx) + .await?; + } sqlx::query( "INSERT INTO metadata_artist \ - (provider, name_canonical, outcome, bio_summary, bio_full, fetched_at, expires_at) \ - VALUES (?, ?, ?, ?, ?, ?, ?) \ + (provider, name_canonical, outcome, bio_summary, bio_full, image_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, fetched_at = excluded.fetched_at, \ - expires_at = excluded.expires_at", + bio_full = excluded.bio_full, image_hash = excluded.image_hash, \ + fetched_at = excluded.fetched_at, expires_at = excluded.expires_at", ) .bind(provider.as_str()) .bind(name_canonical) .bind(answer.outcome().as_str()) .bind(answer.bio_summary.as_deref()) .bind(answer.bio_full.as_deref()) + .bind(answer.image.as_ref().map(|image| image.hash.as_str())) .bind(fetched_at) .bind(expires_at) .execute(&mut *tx) .await?; + if let Some(image) = &answer.image { + link_portrait_in(&mut tx, name_canonical, &image.hash, fetched_at).await?; + } sqlx::query( "DELETE FROM metadata_artist_similar WHERE provider = ? AND name_canonical = ?", ) @@ -2030,6 +2088,61 @@ impl Database { } } +/// Points every artist called `name_canonical` at the portrait `hash`, and +/// records one artist event for each row that actually moved — RFC-007 +/// decision 6's rule for albums, which an artist now follows: a refresh that +/// fetched the same picture changes nothing and says nothing. +/// +/// Artist rows are per library, and so are events, so each moved row carries +/// its own library. A row whose artwork row is gone is not linked: the cache +/// may name a hash the sweeper has since taken. Nor is a hash the cache no +/// longer names: a read that saw the old row, then repairs after a fill +/// replaced it, would put the old portrait back and announce it. +async fn link_portrait_in( + tx: &mut sqlx::Transaction<'_, sqlx::Sqlite>, + name_canonical: &str, + hash: &str, + now: i64, +) -> Result<(), sqlx::Error> { + let moved = sqlx::query( + "UPDATE artist SET artwork_hash = ?, updated_at = ? \ + WHERE canonical_name = ? AND artwork_hash IS NOT ? \ + AND EXISTS (SELECT 1 FROM artwork WHERE artwork.hash = ?) \ + AND EXISTS (SELECT 1 FROM metadata_artist \ + WHERE name_canonical = ? AND image_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?; + 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::*; @@ -2541,4 +2654,70 @@ mod tests { ); } } + + /// A read takes the cache row without the gate, so a fill can replace and + /// link a newer portrait before the read's repair runs. That repair must + /// not put the hash it read back over the newer one, nor announce it. + #[tokio::test] + async fn a_repair_does_not_restore_a_portrait_the_cache_replaced() { + let (_temp, database) = migrated_database().await; + const ACCOUNT: &str = "6f1d1b6e-0f1a-4a3d-9c21-000000000021"; + const LIBRARY: &str = "6f1d1b6e-0f1a-4a3d-9c21-000000000022"; + const ARTIST: &str = "6f1d1b6e-0f1a-4a3d-9c21-000000000023"; + let read = "a".repeat(64); + let replaced = "b".repeat(64); + for (statement, binds) in [ + ( + "INSERT INTO account (id, username, password_hash, role, created_at, updated_at) \ + VALUES (?, 'owner', 'this-account-never-authenticates', 'admin', 0, 0)", + vec![ACCOUNT], + ), + ( + "INSERT INTO library (id, owner_user_id, name, root_path, visibility, \ + created_at, updated_at) \ + VALUES (?, ?, 'Library', '/music', 'private', 0, 0)", + vec![LIBRARY, ACCOUNT], + ), + ( + "INSERT INTO artwork (hash, format, source, byte_size, created_at) \ + VALUES (?, 'jpg', 'deezer', 1, 0), (?, 'jpg', 'deezer', 1, 0)", + vec![read.as_str(), replaced.as_str()], + ), + ( + "INSERT INTO artist (id, library_id, name, canonical_name, artwork_hash, \ + created_at, updated_at) \ + VALUES (?, ?, 'Nova Kern', 'nova kern', ?, 0, 0)", + vec![ARTIST, LIBRARY, replaced.as_str()], + ), + ( + "INSERT INTO metadata_artist (provider, name_canonical, outcome, image_hash, \ + fetched_at, expires_at) \ + VALUES ('deezer', 'nova kern', 'hit', ?, 0, 0)", + vec![replaced.as_str()], + ), + ] { + let mut query = sqlx::query(statement); + for bind in binds { + query = query.bind(bind.to_owned()); + } + query.execute(&database.pool).await.expect("seed row"); + } + + database + .link_artist_portrait("nova kern", &read) + .await + .expect("repair"); + + let linked: String = sqlx::query_scalar("SELECT artwork_hash FROM artist WHERE id = ?") + .bind(ARTIST) + .fetch_one(&database.pool) + .await + .expect("artist row"); + assert!(linked == replaced, "the newer portrait was replaced"); + let events: i64 = sqlx::query_scalar("SELECT COUNT(*) FROM library_event") + .fetch_one(&database.pool) + .await + .expect("event count"); + assert_eq!(events, 0); + } } diff --git a/src/enrichment.rs b/src/enrichment.rs index 8bf656c..e62f6eb 100644 --- a/src/enrichment.rs +++ b/src/enrichment.rs @@ -21,19 +21,36 @@ //! URL nor a response body is ever written to a log (decision 9). use std::{ + path::PathBuf, sync::Arc, time::{Duration, Instant}, }; use dashmap::DashMap; -use waveflow_core::metadata::{lastfm::LastfmClient, LookupError}; +use waveflow_core::metadata::{ + deezer::{DeezerClient, DeezerError}, + lastfm::LastfmClient, + name_match::{normalize_name, select_by_name}, + LookupError, +}; -use crate::{config::MetadataSettings, database::Database}; +use crate::{ + config::MetadataSettings, + database::Database, + portrait::{self, AddressPolicy, Refusal}, +}; /// Last.fm's API root. A constant, not a setting (decision 9): there is no /// operator field, so there is no URL to validate. const LASTFM_BASE_URL: &str = "https://ws.audioscrobbler.com/2.0/"; +/// Deezer's API root, a constant for the same reason. It asks for no key. +const DEEZER_BASE_URL: &str = "https://api.deezer.com"; + +/// The hosts Deezer serves its pictures from (decision 8). A picture URL +/// naming any other host is dropped without being contacted. +const DEEZER_IMAGE_HOSTS: [&str; 2] = ["e-cdns-images.dzcdn.net", "cdn-images.dzcdn.net"]; + /// 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. @@ -53,12 +70,16 @@ pub const MAX_IN_FLIGHT: usize = 64; #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub enum Provider { LastFm, + /// Portraits only: Deezer has no biography, and its related artists are + /// not asked for. + Deezer, } impl Provider { pub fn parse(value: &str) -> Option { match value { "lastfm" => Some(Self::LastFm), + "deezer" => Some(Self::Deezer), _ => None, } } @@ -67,6 +88,7 @@ impl Provider { pub fn as_str(self) -> &'static str { match self { Self::LastFm => "lastfm", + Self::Deezer => "deezer", } } } @@ -103,10 +125,21 @@ pub struct CachedArtist { pub outcome: Outcome, pub bio_summary: Option, pub bio_full: Option, + /// The portrait this provider supplied, in the artwork store. Cleared by + /// the database when the sweeper takes the artwork row. + pub image_hash: Option, pub fetched_at: i64, pub expires_at: i64, } +/// A portrait already in the artwork store, waiting for its row. +#[derive(Debug)] +pub(crate) struct StoredPortrait { + pub(crate) hash: String, + pub(crate) format: &'static str, + pub(crate) byte_size: i64, +} + /// One neighbour in a provider's cached list. #[derive(Debug, Clone)] pub struct CachedSimilar { @@ -121,12 +154,17 @@ pub(crate) struct ArtistAnswer { pub(crate) bio_summary: Option, pub(crate) bio_full: Option, pub(crate) similar: Vec, + pub(crate) image: Option, } impl ArtistAnswer { /// A hit carries something; an answer with nothing in it is a miss. pub(crate) fn outcome(&self) -> Outcome { - if self.bio_summary.is_some() || self.bio_full.is_some() || !self.similar.is_empty() { + if self.bio_summary.is_some() + || self.bio_full.is_some() + || !self.similar.is_empty() + || self.image.is_some() + { Outcome::Hit } else { Outcome::Miss @@ -140,12 +178,21 @@ impl ArtistAnswer { #[derive(Debug, Clone)] pub struct Endpoints { pub lastfm: String, + pub deezer: String, + /// The hosts a Deezer picture may be fetched from. + pub deezer_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, } impl Default for Endpoints { fn default() -> Self { Self { lastfm: LASTFM_BASE_URL.to_owned(), + deezer: DEEZER_BASE_URL.to_owned(), + deezer_image_hosts: DEEZER_IMAGE_HOSTS.map(str::to_owned).to_vec(), + image_addresses: AddressPolicy::default(), } } } @@ -167,6 +214,9 @@ struct Engine { ttl_ms: i64, miss_ttl_ms: i64, request_timeout: Duration, + /// Where fetched portraits are stored: the artwork store. + artwork_dir: PathBuf, + image_addresses: AddressPolicy, /// The fills in flight, by provider and canonical name. A visit that finds /// its key here schedules nothing: the fill already running answers it. in_flight: DashMap<(Provider, String), ()>, @@ -182,7 +232,14 @@ struct Source { } enum Client { - LastFm { client: LastfmClient, key: String }, + LastFm { + client: LastfmClient, + key: String, + }, + Deezer { + client: DeezerClient, + image_hosts: Vec, + }, } impl Enrichment { @@ -192,18 +249,24 @@ impl Enrichment { } /// The engine `settings` describe, against the providers' real addresses. - pub fn new(db: Database, settings: &MetadataSettings) -> anyhow::Result { - Self::with_endpoints(db, settings, Endpoints::default()) + pub fn new( + db: Database, + settings: &MetadataSettings, + artwork_dir: PathBuf, + ) -> anyhow::Result { + Self::with_endpoints(db, settings, artwork_dir, Endpoints::default()) } /// [`Self::new`], with the providers reached at `endpoints`. /// /// Only the configured providers are built. A key placed with no provider /// naming it builds nothing — the provider list is the switch, and a key - /// is not consent (decision 3). + /// is not consent (decision 3). With `WAVEFLOW_METADATA_IMAGES=off`, + /// Deezer is not built either: portraits are all it is asked for. pub fn with_endpoints( db: Database, settings: &MetadataSettings, + artwork_dir: PathBuf, endpoints: Endpoints, ) -> anyhow::Result { let mut sources = Vec::new(); @@ -220,6 +283,11 @@ impl Enrichment { key, } } + Provider::Deezer if !settings.images => continue, + Provider::Deezer => Client::Deezer { + client: DeezerClient::with_http(http, endpoints.deezer.clone()), + image_hosts: endpoints.deezer_image_hosts.clone(), + }, }; sources.push(Source { provider, @@ -237,6 +305,8 @@ impl Enrichment { ttl_ms: duration_ms(settings.ttl), miss_ttl_ms: duration_ms(settings.miss_ttl), request_timeout: settings.request_timeout, + artwork_dir, + image_addresses: endpoints.image_addresses, in_flight: DashMap::new(), running: tokio::sync::watch::channel(0).0, })), @@ -280,6 +350,16 @@ impl Enrichment { } ordered.extend(row.cloned()); } + // A portrait already fetched, and an artist row that does not show + // it: an artist re-identified by a PID spec change, or one scanned + // since. Linked again from the cache, without a second download. A + // repair, not the answer: if it fails, the page is still served and + // the next visit tries again. + if let Some(hash) = ordered.iter().find_map(|row| row.image_hash.as_deref()) { + if let Err(error) = engine.db.link_artist_portrait(&canonical, hash).await { + tracing::warn!(%error, "cached artist portrait could not be linked"); + } + } Ok(ordered) } @@ -336,6 +416,10 @@ impl Engine { let source = &self.sources[index]; let answer = match &source.client { Client::LastFm { client, key } => self.ask_lastfm(source, client, key, name).await, + Client::Deezer { + client, + image_hosts, + } => self.ask_deezer(source, client, image_hosts, name).await, }; let answer = match answer { Ok(answer) => answer, @@ -414,18 +498,75 @@ impl Engine { bio_summary, bio_full, similar, + image: None, + }) + } + + /// The artist search, then the picture it names — decision 8. + /// + /// No artist by that name, or one without a picture, is a miss. So is a + /// picture the fetcher refuses for what it is — off the allowlist, at a + /// refused address, not an image, too large — since asking again would + /// get the same answer. A picture host that could not be reached is + /// unreached, like the search, and writes nothing. + async fn ask_deezer( + &self, + source: &Source, + client: &DeezerClient, + image_hosts: &[String], + name: &str, + ) -> Result { + source.pace.wait().await; + let hits = self.bounded(client.search_artist(name)).await?; + let picture = select_by_name(hits, &normalize_name(name), |hit| Some(hit.name.as_str())) + .and_then(|hit| hit.best_picture()); + let Some(picture) = picture else { + return Ok(ArtistAnswer::default()); + }; + let fetched = portrait::fetch( + &picture, + image_hosts, + self.image_addresses, + self.request_timeout, + ) + .await; + let portrait = match fetched { + Ok(portrait) => portrait, + Err(refusal) if refusal.is_answer() => { + tracing::warn!( + provider = source.provider.as_str(), + verdict = %refusal, + "artist portrait refused; cached as a miss" + ); + return Ok(ArtistAnswer::default()); + } + Err(refusal) => return Err(Verdict::Image(refusal)), + }; + portrait::store(&self.artwork_dir, &portrait) + .await + .map_err(|_| Verdict::Store)?; + Ok(ArtistAnswer { + image: Some(StoredPortrait { + hash: portrait.hash, + format: portrait.format, + byte_size: portrait.bytes.len() as i64, + }), + ..ArtistAnswer::default() }) } /// One request, under the request timeout; anything but an answer is a /// verdict. - async fn bounded( + async fn bounded( &self, - request: impl std::future::Future>, - ) -> Result { + request: impl std::future::Future>, + ) -> Result + where + Verdict: From, + { match tokio::time::timeout(self.request_timeout, request).await { Ok(Ok(answer)) => Ok(answer), - Ok(Err(error)) => Err(Verdict::of(&error)), + Ok(Err(error)) => Err(Verdict::from(error)), Err(_) => Err(Verdict::Timeout), } } @@ -450,25 +591,55 @@ impl Drop for Flight { enum Verdict { Timeout, Transport, - Refused { code: i64 }, - Status { status: u16 }, + Refused { + code: i64, + }, + Status { + status: u16, + }, TooLarge, Undecodable, + RateLimited, + /// The picture host, not the provider, was not reached. + Image(Refusal), + /// The picture came back and could not be written to the artwork store. + Store, } -impl Verdict { - fn of(error: &LookupError) -> Self { +impl From for Verdict { + fn from(error: LookupError) -> Self { match error { LookupError::Http(error) if error.is_timeout() => Self::Timeout, LookupError::Http(_) => Self::Transport, - LookupError::Refused { code } => Self::Refused { code: *code }, - LookupError::Status { status } => Self::Status { status: *status }, + LookupError::Refused { code } => Self::Refused { code }, + LookupError::Status { status } => Self::Status { status }, LookupError::TooLarge { .. } => Self::TooLarge, LookupError::Decode(_) => Self::Undecodable, } } } +impl From for Verdict { + fn from(error: DeezerError) -> Self { + match error { + DeezerError::Http(error) if error.is_timeout() => Self::Timeout, + DeezerError::Http(error) => { + error + .status() + .map_or(Self::Transport, |status| Self::Status { + status: status.as_u16(), + }) + } + DeezerError::Api(error) => Self::Refused { + code: error.code.unwrap_or_default(), + }, + DeezerError::RateLimited => Self::RateLimited, + DeezerError::TooLarge { .. } => Self::TooLarge, + DeezerError::Decode(_) => Self::Undecodable, + } + } +} + impl std::fmt::Display for Verdict { fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { @@ -478,6 +649,9 @@ impl std::fmt::Display for Verdict { Self::Status { status } => write!(formatter, "answered HTTP {status}"), Self::TooLarge => formatter.write_str("answer over the size ceiling"), Self::Undecodable => formatter.write_str("answer not understood"), + Self::RateLimited => formatter.write_str("rate-limited"), + Self::Image(refusal) => refusal.fmt(formatter), + Self::Store => formatter.write_str("portrait could not be stored"), } } } diff --git a/src/lib.rs b/src/lib.rs index 1e408ca..f79df84 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -13,6 +13,7 @@ pub mod oauth; pub mod outbound; pub mod pid; pub mod playlist_file; +pub mod portrait; pub mod scanner; pub mod scrobblers; pub mod security; @@ -683,7 +684,11 @@ pub async fn initialize(config: &Config) -> anyhow::Result { services.reconcile_scrobble_links().await?; // Off unless the operator named a provider, and then it still makes no // request until somebody asks about an artist (RFC-011 decisions 3 and 6). - services.use_enrichment(enrichment::Enrichment::new(db.clone(), &config.metadata)?); + services.use_enrichment(enrichment::Enrichment::new( + db.clone(), + &config.metadata, + config.artwork_dir.clone(), + )?); Ok(AppState { db, auth, diff --git a/src/outbound.rs b/src/outbound.rs index 2f9ac95..936510c 100644 --- a/src/outbound.rs +++ b/src/outbound.rs @@ -38,10 +38,16 @@ const CONNECT_SHARE: u32 = 3; /// Which of the two fired was a coin toss, so which protection applied was a /// coin toss. pub fn client(timeout: Duration) -> Result { + builder(timeout).build() +} + +/// The same rules, left open for one caller to add to: the portrait fetcher +/// pins the address it checked (`ClientBuilder::resolve`), which is a property +/// of one connection rather than of every client. +pub(crate) fn builder(timeout: Duration) -> reqwest::ClientBuilder { reqwest::Client::builder() .redirect(reqwest::redirect::Policy::none()) .connect_timeout(timeout / CONNECT_SHARE) .no_proxy() .user_agent(concat!("WaveFlowServer/", env!("CARGO_PKG_VERSION"))) - .build() } diff --git a/src/portrait.rs b/src/portrait.rs new file mode 100644 index 0000000..10e9901 --- /dev/null +++ b/src/portrait.rs @@ -0,0 +1,355 @@ +//! The narrow image fetcher of RFC-011 decision 8. +//! +//! A provider's answer names an image URL, and that URL is untrusted input: it +//! came out of a response, not out of the operator's configuration. So the +//! fetcher dereferences it only when every one of these holds: +//! +//! - **https**, and the host is on the provider's **allowlist**, compared +//! exactly. A URL pointing anywhere else is dropped, not fetched. +//! - **Every address the name resolves to is public.** Loopback, private, +//! link-local, shared, unspecified and multicast ranges are refused, IPv4 +//! inside IPv6 included. +//! - **The address checked is the address connected to.** The client is built +//! for this one fetch with the checked address pinned +//! (`ClientBuilder::resolve`), so the name is not looked up a second time by +//! a resolver whose answer could differ. +//! - **No redirect** (the shared client's policy), a **size ceiling** enforced +//! while reading, and a **content type** among the three the artwork route +//! serves — confirmed by the bytes themselves, not taken from the header. +//! +//! The address policy is a parameter for one reason: the test suite's image +//! host lives on loopback, which the default refuses. [`AddressPolicy::default`] +//! is production, and a test asserts it refuses loopback, so the suite cannot +//! prove the fetcher only with its main protection switched off. + +use std::{ + net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddr}, + path::Path, + time::Duration, +}; + +/// The largest portrait accepted. Deezer's largest size is a 1000×1000 JPEG, +/// well under a mebibyte; ten leaves room without letting a provider fill the +/// disk. +pub const MAX_IMAGE_BYTES: usize = 10 * 1024 * 1024; + +/// Which addresses a fetch may connect to. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +pub enum AddressPolicy { + /// Public addresses only, over https. Production. + #[default] + Public, + /// Public addresses, and loopback over plain http as well. For the test + /// suite's listeners, and for nothing else. + AllowLoopback, +} + +impl AddressPolicy { + /// Whether a connection to `ip` is allowed. + pub fn admits(self, ip: IpAddr) -> bool { + let ip = match ip { + IpAddr::V6(v6) => v6.to_ipv4_mapped().map_or(ip, IpAddr::V4), + v4 => v4, + }; + if ip.is_loopback() { + return self == Self::AllowLoopback; + } + match ip { + IpAddr::V4(v4) => is_public_v4(v4), + IpAddr::V6(v6) => is_public_v6(v6), + } + } +} + +fn is_public_v4(ip: Ipv4Addr) -> bool { + let [a, b, ..] = ip.octets(); + !(ip.is_private() + || ip.is_link_local() + || ip.is_unspecified() + || ip.is_broadcast() + || ip.is_multicast() + || ip.is_documentation() + // 100.64.0.0/10, carrier-grade NAT: another operator's private range. + || (a == 100 && (64..128).contains(&b)) + // 0.0.0.0/8, "this network". + || a == 0 + // 198.18.0.0/15, benchmarking: routed inside some private networks. + || (a == 198 && (b == 18 || b == 19)) + // 240.0.0.0/4, reserved. + || a >= 240 + // 192.0.0.0/24, protocol assignments, and 192.88.99.0/24, the 6to4 + // relay anycast. + || ip.octets()[..3] == [192, 0, 0] + || ip.octets()[..3] == [192, 88, 99]) +} + +/// IPv6 is admitted by what it is, not by what it is not: global unicast, +/// `2000::/3`, is the only space a public host answers from, and everything +/// outside it — unique and link local, site local, NAT64, IPv4-mapped, +/// IPv4-compatible, discard — is refused without being listed. Inside it, +/// the prefixes that carry an IPv4 address a gateway connects to (which may +/// be a private one) are refused whole rather than decoded. +fn is_public_v6(ip: Ipv6Addr) -> bool { + let [first, second, ..] = ip.segments(); + (first & 0xe000) == 0x2000 + // 2001::/32, Teredo. + && !(first == 0x2001 && second == 0) + // 2001:db8::/32, documentation. + && !(first == 0x2001 && second == 0x0db8) + // 2002::/16, 6to4. + && first != 0x2002 +} + +/// A fetched portrait, checked and hashed, not yet stored. +#[derive(Debug)] +pub struct Portrait { + /// BLAKE3 of the bytes, hexadecimal: the artwork store's key. + pub hash: String, + /// `jpg`, `png` or `webp`, from the bytes. + pub format: &'static str, + pub bytes: Vec, +} + +/// Why a portrait was not fetched. Carries nothing from the URL or the answer. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Refusal { + /// Not https, or a host off the allowlist: the provider pointed somewhere + /// it should not have. Nothing was contacted. + Destination, + /// The name resolved to an address the policy refuses. Nothing was + /// contacted. + Address, + /// The answer is not one of the three image types, by header or by bytes. + NotAnImage, + /// Over [`MAX_IMAGE_BYTES`]. + TooLarge, + /// The name did not resolve, the connection failed, or the deadline passed. + Unreached, + /// The host answered something other than success. + Status(u16), +} + +impl Refusal { + /// Whether this says something about the image itself, which a cache may + /// remember, rather than about the way to it, which the next visit should + /// try again. + pub fn is_answer(self) -> bool { + matches!( + self, + Self::Destination | Self::Address | Self::NotAnImage | Self::TooLarge + ) + } +} + +impl std::fmt::Display for Refusal { + fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Destination => formatter.write_str("image destination not allowed"), + Self::Address => formatter.write_str("image address not allowed"), + Self::NotAnImage => formatter.write_str("answer is not an image"), + Self::TooLarge => formatter.write_str("image over the size ceiling"), + Self::Unreached => formatter.write_str("image host unreached"), + Self::Status(status) => write!(formatter, "image host answered HTTP {status}"), + } + } +} + +/// Fetches `url` under every rule above, within `timeout` overall. +pub async fn fetch( + url: &str, + hosts: &[String], + policy: AddressPolicy, + timeout: Duration, +) -> Result { + tokio::time::timeout(timeout, fetch_within(url, hosts, policy, timeout)) + .await + .unwrap_or(Err(Refusal::Unreached)) +} + +async fn fetch_within( + url: &str, + hosts: &[String], + policy: AddressPolicy, + timeout: Duration, +) -> Result { + let url = url::Url::parse(url).map_err(|_| Refusal::Destination)?; + let plain = match url.scheme() { + "https" => false, + "http" if policy == AddressPolicy::AllowLoopback => true, + _ => return Err(Refusal::Destination), + }; + let host = url + .host_str() + .map(str::to_ascii_lowercase) + .ok_or(Refusal::Destination)?; + if !hosts + .iter() + .any(|allowed| allowed.eq_ignore_ascii_case(&host)) + { + return Err(Refusal::Destination); + } + let port = url.port_or_known_default().ok_or(Refusal::Destination)?; + let addresses: Vec = match url.host() { + Some(url::Host::Ipv4(ip)) => vec![SocketAddr::new(IpAddr::V4(ip), port)], + Some(url::Host::Ipv6(ip)) => vec![SocketAddr::new(IpAddr::V6(ip), port)], + _ => tokio::net::lookup_host((host.as_str(), port)) + .await + .map_err(|_| Refusal::Unreached)? + .collect(), + }; + // Every answer, not the first: a name that resolves to one public and one + // private address is refused, whichever the client would have picked. + if addresses.is_empty() { + return Err(Refusal::Unreached); + } + if !addresses.iter().all(|address| policy.admits(address.ip())) { + return Err(Refusal::Address); + } + let pinned = addresses[0]; + if plain && !pinned.ip().is_loopback() { + return Err(Refusal::Destination); + } + let client = crate::outbound::builder(timeout) + .resolve(&host, pinned) + .build() + .map_err(|_| Refusal::Unreached)?; + let mut response = client + .get(url) + .send() + .await + .map_err(|_| Refusal::Unreached)?; + if !response.status().is_success() { + return Err(Refusal::Status(response.status().as_u16())); + } + let declared = response + .headers() + .get(reqwest::header::CONTENT_TYPE) + .and_then(|value| value.to_str().ok()) + .and_then(|value| value.split(';').next()) + .map(|value| value.trim().to_ascii_lowercase()) + .and_then(|value| format_of_mime(&value)) + .ok_or(Refusal::NotAnImage)?; + if response + .content_length() + .is_some_and(|length| length > MAX_IMAGE_BYTES as u64) + { + return Err(Refusal::TooLarge); + } + let mut bytes = Vec::new(); + while let Some(chunk) = response.chunk().await.map_err(|_| Refusal::Unreached)? { + if bytes.len() + chunk.len() > MAX_IMAGE_BYTES { + return Err(Refusal::TooLarge); + } + bytes.extend_from_slice(&chunk); + } + let format = sniff(&bytes).ok_or(Refusal::NotAnImage)?; + if format != declared { + return Err(Refusal::NotAnImage); + } + Ok(Portrait { + hash: blake3::hash(&bytes).to_hex().to_string(), + format, + bytes, + }) +} + +fn format_of_mime(mime: &str) -> Option<&'static str> { + match mime { + "image/jpeg" | "image/jpg" => Some("jpg"), + "image/png" => Some("png"), + "image/webp" => Some("webp"), + _ => None, + } +} + +/// The format the bytes are, among the three the artwork route serves. +fn sniff(bytes: &[u8]) -> Option<&'static str> { + if bytes.starts_with(&[0xff, 0xd8, 0xff]) { + Some("jpg") + } else if bytes.starts_with(b"\x89PNG\r\n\x1a\n") { + Some("png") + } else if bytes.len() >= 12 && &bytes[..4] == b"RIFF" && &bytes[8..12] == b"WEBP" { + Some("webp") + } else { + None + } +} + +/// Puts `portrait` in the artwork store, `.` under +/// `artwork_dir`, unless it is already there. +/// +/// Written beside the target and renamed into place, so a reader never sees +/// half a file. Called before the writer gate is taken: this is file I/O. +pub async fn store(artwork_dir: &Path, portrait: &Portrait) -> std::io::Result<()> { + let target = artwork_dir.join(format!("{}.{}", portrait.hash, portrait.format)); + if tokio::fs::try_exists(&target).await? { + return Ok(()); + } + tokio::fs::create_dir_all(artwork_dir).await?; + let staging = artwork_dir.join(format!("{}.part", uuid::Uuid::new_v4().simple())); + tokio::fs::write(&staging, &portrait.bytes).await?; + if let Err(error) = tokio::fs::rename(&staging, &target).await { + let _ = tokio::fs::remove_file(&staging).await; + // Another fill stored the same bytes first; the content is identical. + if tokio::fs::try_exists(&target).await.unwrap_or(false) { + return Ok(()); + } + return Err(error); + } + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + + /// The protection the suite cannot exercise end to end, since its image + /// host is on loopback: the default refuses loopback and every private + /// range, and admits a public address. + #[test] + fn the_default_policy_refuses_a_loopback_address() { + let policy = AddressPolicy::default(); + for refused in [ + "127.0.0.1", + "::1", + "::ffff:127.0.0.1", + "10.1.2.3", + "172.16.0.1", + "192.168.1.1", + "169.254.169.254", + "100.64.0.1", + "0.0.0.0", + "::", + "fd00::1", + "fe80::1", + "::ffff:192.168.1.1", + "198.18.0.1", + "240.0.0.1", + "fec0::1", + "64:ff9b::a00:1", + "64:ff9b:1::1", + "2002:a00:1::1", + "::10.0.0.1", + "192.0.0.1", + "192.88.99.1", + "2001:0:a00:1::1", + "2001:db8::1", + "100::1", + "::ffff:0:a00:1", + ] { + assert!(!policy.admits(refused.parse().unwrap()), "{refused}"); + } + assert!(policy.admits("93.184.216.34".parse().unwrap())); + assert!(policy.admits("2606:4700::1111".parse().unwrap())); + assert!(AddressPolicy::AllowLoopback.admits("127.0.0.1".parse().unwrap())); + assert!(!AddressPolicy::AllowLoopback.admits("10.0.0.1".parse().unwrap())); + } + + #[test] + fn the_bytes_decide_the_format() { + assert_eq!(sniff(&[0xff, 0xd8, 0xff, 0xe0]), Some("jpg")); + assert_eq!(sniff(b"\x89PNG\r\n\x1a\n...."), Some("png")); + assert_eq!(sniff(b"RIFF\0\0\0\0WEBPVP8 "), Some("webp")); + assert_eq!(sniff(b""), None); + } +} diff --git a/src/services/artist_enrichment.rs b/src/services/artist_enrichment.rs index bd4ada4..3ebb5d5 100644 --- a/src/services/artist_enrichment.rs +++ b/src/services/artist_enrichment.rs @@ -353,6 +353,7 @@ mod tests { outcome, bio_summary: bio.map(str::to_owned), bio_full: None, + image_hash: None, fetched_at, expires_at: fetched_at + 1, } diff --git a/src/services/artwork.rs b/src/services/artwork.rs index de69699..cdd1510 100644 --- a/src/services/artwork.rs +++ b/src/services/artwork.rs @@ -40,6 +40,12 @@ //! reported as a dead link meanwhile, rather than bytes that no longer exist //! anywhere. //! +//! An artist portrait (RFC-011 decision 8) is the exception: its bytes came +//! from a provider, not from a file on disk, and no scan writes them back. The +//! window costs it more — a dead link until the cache entry expires and the +//! next visit downloads it again, up to the hit TTL — and is the same width, +//! since its row is committed in one transaction under this same gate. +//! //! That is an argument for tolerating the window, not for pretending it is //! shut. It is narrow, it needs a scan and a sweep to interleave at one //! instant, and no test here forces that ordering. diff --git a/tests/enrichment.rs b/tests/enrichment.rs index 322ae0e..530c66c 100644 --- a/tests/enrichment.rs +++ b/tests/enrichment.rs @@ -30,6 +30,7 @@ use axum::{ use waveflow_server::{ config::MetadataSettings, enrichment::{Endpoints, Enrichment, Outcome, Provider}, + portrait::AddressPolicy, }; #[derive(Clone, Copy)] @@ -152,8 +153,10 @@ fn engine( Enrichment::with_endpoints( state.db.clone(), settings, + state.artwork_dir.clone(), Endpoints { lastfm: listener.base_url.clone(), + ..Endpoints::default() }, ) .unwrap() @@ -446,8 +449,10 @@ async fn the_key_never_reaches_the_log() { let closed = Enrichment::with_endpoints( state.db.clone(), &settings, + state.artwork_dir.clone(), Endpoints { lastfm: "http://127.0.0.1:1/2.0/".to_owned(), + ..Endpoints::default() }, ) .unwrap(); @@ -1061,3 +1066,290 @@ async fn a_neighbour_s_album_of_guests_takes_one_share() { assert_eq!(songs.len(), 4); assert_eq!(own, 2); } + +// ── Portraits: Deezer and the narrow fetcher (decision 8) ── + +/// A fake Deezer: the artist search, and the picture it points at, served by +/// the same loopback listener under whatever host name the test chooses. +#[derive(Clone)] +struct Deezer { + searches: Arc, + images: Arc, + picture: Arc, + bytes: Arc>, +} + +struct DeezerListener { + fake: Deezer, + base_url: String, +} + +/// A JPEG by its first bytes, which is all the fetcher checks; the rest is +/// made up for the run so two runs never share a hash. +fn portrait_bytes() -> Vec { + let mut bytes = vec![0xff, 0xd8, 0xff, 0xe0]; + bytes.extend_from_slice(run_key().as_bytes()); + bytes +} + +async fn deezer_search(State(fake): State) -> Response { + fake.searches.fetch_add(1, Ordering::SeqCst); + json( + serde_json::json!({"data": [ + {"id": 1, "name": "Björk", "picture_xl": *fake.picture} + ]}) + .to_string(), + ) +} + +async fn deezer_picture(State(fake): State) -> Response { + fake.images.fetch_add(1, Ordering::SeqCst); + ( + [(header::CONTENT_TYPE, "image/jpeg")], + (*fake.bytes).clone(), + ) + .into_response() +} + +async fn fake_deezer(picture_host: &str) -> DeezerListener { + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let port = listener.local_addr().unwrap().port(); + let fake = Deezer { + searches: Arc::default(), + images: Arc::default(), + picture: Arc::new(format!("http://{picture_host}:{port}/img/portrait.jpg")), + bytes: Arc::new(portrait_bytes()), + }; + let router = Router::new() + .route("/search/artist", get(deezer_search)) + .route("/img/portrait.jpg", get(deezer_picture)) + .with_state(fake.clone()); + tokio::spawn(async move { axum::serve(listener, router).await.unwrap() }); + DeezerListener { + fake, + base_url: format!("http://127.0.0.1:{port}"), + } +} + +/// An engine asking the fake Deezer, fetching pictures from `hosts` under +/// `policy`. +fn deezer_engine( + state: &waveflow_server::AppState, + settings: &MetadataSettings, + listener: &DeezerListener, + hosts: &[&str], + policy: AddressPolicy, +) -> Enrichment { + Enrichment::with_endpoints( + state.db.clone(), + settings, + state.artwork_dir.clone(), + Endpoints { + deezer: listener.base_url.clone(), + deezer_image_hosts: hosts.iter().map(|host| (*host).to_owned()).collect(), + image_addresses: policy, + ..Endpoints::default() + }, + ) + .unwrap() +} + +/// Björk in a library of each of two accounts: two artist rows, one name. +async fn bjork_twice() -> ( + tempfile::TempDir, + waveflow_server::AppState, + uuid::Uuid, + uuid::Uuid, +) { + let (temp, config, state) = support::test_app().await; + let owner = account(&state, "portrait-owner").await; + let other = account(&state, "portrait-other").await; + library_of(&state, &config, owner, "mine", &["Björk"]).await; + library_of(&state, &config, other, "theirs", &["Björk"]).await; + (temp, state, owner, other) +} + +async fn artist_events(state: &waveflow_server::AppState) -> i64 { + sqlx::query_scalar("SELECT COUNT(*) FROM library_event WHERE entity_type = 'artist'") + .fetch_one(state.db.pool()) + .await + .unwrap() +} + +async fn portrait_of(state: &waveflow_server::AppState, user: uuid::Uuid) -> Option { + let bjork = artist_id(state, user, "Björk").await; + state + .services + .artist(user, bjork) + .await + .unwrap() + .artist + .artwork_hash +} + +/// Decision 8 end to end: the picture is fetched once, stored in the artwork +/// store under its hash, and every artist row with the name shows it — one +/// event for each, in its own library. +#[tokio::test] +async fn a_portrait_is_fetched_once_and_shown_on_every_artist_with_the_name() { + let (_temp, state, owner, other) = bjork_twice().await; + let listener = fake_deezer("127.0.0.1").await; + let enrichment = deezer_engine( + &state, + &settings(vec![Provider::Deezer], &run_key()), + &listener, + &["127.0.0.1"], + AddressPolicy::AllowLoopback, + ); + enrichment.artist("Björk").await.unwrap(); + enrichment.idle().await; + + let mine = portrait_of(&state, owner).await; + let hash = mine.clone().expect("the portrait is linked"); + assert_eq!(portrait_of(&state, other).await, mine); + assert!(state.artwork_dir.join(format!("{hash}.jpg")).is_file()); + let source: String = sqlx::query_scalar("SELECT source FROM artwork WHERE hash = ?") + .bind(&hash) + .fetch_one(state.db.pool()) + .await + .unwrap(); + assert_eq!(source, "deezer"); + assert_eq!(artist_events(&state).await, 2); + assert_eq!(listener.fake.images.load(Ordering::SeqCst), 1); +} + +/// A picture URL naming a host off the allowlist is dropped without being +/// contacted, and the answer is cached as a miss rather than retried. +#[tokio::test] +async fn a_host_outside_the_allowlist_is_never_contacted() { + let (_temp, state, owner, _other) = bjork_twice().await; + let listener = fake_deezer("127.0.0.1").await; + let enrichment = deezer_engine( + &state, + &settings(vec![Provider::Deezer], &run_key()), + &listener, + &["localhost"], + AddressPolicy::AllowLoopback, + ); + enrichment.artist("Björk").await.unwrap(); + enrichment.idle().await; + + assert_eq!(listener.fake.searches.load(Ordering::SeqCst), 1); + assert_eq!(listener.fake.images.load(Ordering::SeqCst), 0); + let rows = enrichment.artist("Björk").await.unwrap(); + assert_eq!(rows[0].outcome, Outcome::Miss); + assert_eq!(portrait_of(&state, owner).await, None); +} + +/// Production's policy, with the fake's own host allowlisted: still never +/// contacted, because it is on loopback. +#[tokio::test] +async fn the_default_policy_does_not_reach_a_loopback_host() { + let (_temp, state, _owner, _other) = bjork_twice().await; + let listener = fake_deezer("127.0.0.1").await; + let enrichment = deezer_engine( + &state, + &settings(vec![Provider::Deezer], &run_key()), + &listener, + &["127.0.0.1"], + AddressPolicy::default(), + ); + enrichment.artist("Björk").await.unwrap(); + enrichment.idle().await; + + assert_eq!(listener.fake.searches.load(Ordering::SeqCst), 1); + assert_eq!(listener.fake.images.load(Ordering::SeqCst), 0); +} + +/// The artwork row, the cache row and the artist link are one transaction: +/// when the link cannot be written, neither is the artwork row — nothing is +/// left for the sweeper to find unreferenced, and nothing claims an image. +#[tokio::test] +async fn the_artwork_row_and_the_hash_land_together_or_not_at_all() { + let (_temp, state, _owner, _other) = bjork_twice().await; + sqlx::query( + "CREATE TRIGGER refuse_portrait BEFORE UPDATE OF artwork_hash ON artist \ + BEGIN SELECT RAISE(ABORT, 'refused by the test'); END", + ) + .execute(state.db.pool()) + .await + .unwrap(); + let listener = fake_deezer("127.0.0.1").await; + let enrichment = deezer_engine( + &state, + &settings(vec![Provider::Deezer], &run_key()), + &listener, + &["127.0.0.1"], + AddressPolicy::AllowLoopback, + ); + enrichment.artist("Björk").await.unwrap(); + enrichment.idle().await; + + assert_eq!(listener.fake.images.load(Ordering::SeqCst), 1); + let artwork: i64 = sqlx::query_scalar("SELECT COUNT(*) FROM artwork WHERE source = 'deezer'") + .fetch_one(state.db.pool()) + .await + .unwrap(); + let cached: i64 = sqlx::query_scalar("SELECT COUNT(*) FROM metadata_artist") + .fetch_one(state.db.pool()) + .await + .unwrap(); + assert_eq!((artwork, cached), (0, 0)); +} + +/// RFC-007 decision 6's rule: an event only when `artwork_hash` moved. The +/// row expires at once, so the second visit fetches the same picture again, +/// and says nothing about it. +#[tokio::test] +async fn an_unchanged_portrait_emits_no_event() { + let (_temp, state, _owner, _other) = bjork_twice().await; + let listener = fake_deezer("127.0.0.1").await; + let enrichment = deezer_engine( + &state, + &MetadataSettings { + ttl: Duration::ZERO, + ..settings(vec![Provider::Deezer], &run_key()) + }, + &listener, + &["127.0.0.1"], + AddressPolicy::AllowLoopback, + ); + enrichment.artist("Björk").await.unwrap(); + enrichment.idle().await; + assert_eq!(artist_events(&state).await, 2); + enrichment.artist("Björk").await.unwrap(); + enrichment.idle().await; + + assert_eq!(listener.fake.images.load(Ordering::SeqCst), 2); + assert_eq!(artist_events(&state).await, 2); +} + +/// An artist row that lost its portrait — re-identified by a PID spec change, +/// or scanned since — is linked again from the cache on the next visit, +/// without a second download. +#[tokio::test] +async fn a_cached_portrait_is_linked_again_without_a_second_download() { + let (_temp, state, owner, _other) = bjork_twice().await; + let listener = fake_deezer("127.0.0.1").await; + let enrichment = deezer_engine( + &state, + &settings(vec![Provider::Deezer], &run_key()), + &listener, + &["127.0.0.1"], + AddressPolicy::AllowLoopback, + ); + enrichment.artist("Björk").await.unwrap(); + enrichment.idle().await; + let linked = portrait_of(&state, owner).await; + sqlx::query("UPDATE artist SET artwork_hash = NULL") + .execute(state.db.pool()) + .await + .unwrap(); + + enrichment.artist("Björk").await.unwrap(); + enrichment.idle().await; + + assert!(linked.is_some()); + assert_eq!(portrait_of(&state, owner).await, linked); + assert_eq!(listener.fake.images.load(Ordering::SeqCst), 1); +}