diff --git a/config/settings.py b/config/settings.py index 78b019fc..a2c0630d 100644 --- a/config/settings.py +++ b/config/settings.py @@ -605,7 +605,11 @@ class ConfigManager: "metadata_enhancement": { "enabled": True, "embed_album_art": True, - "post_process_order": ["musicbrainz", "deezer", "audiodb", "tidal", "qobuz", "lastfm", "genius"] + "post_process_order": ["musicbrainz", "deezer", "audiodb", "tidal", "qobuz", "lastfm", "genius"], + # Ordered preferred cover-art sources (empty = use the + # download's own art, i.e. today's behavior). Resolved + walked + # with fallback by core/metadata/art_sources.py. + "album_art_order": [] }, "musicbrainz": { "embed_tags": True diff --git a/core/metadata/art_lookup.py b/core/metadata/art_lookup.py new file mode 100644 index 00000000..22bb46a3 --- /dev/null +++ b/core/metadata/art_lookup.py @@ -0,0 +1,282 @@ +"""Per-source album-art lookups + availability for the cover-art picker. + +Bridges the pure resolver in ``art_sources.py`` to the real metadata clients. +Each supported source contributes two things: + +- **availability** — is this source usable for the current user right now? + Free sources (CAA, Deezer, iTunes, AudioDB) are always available; account + sources (Spotify) only when connected. This is what powers "not everybody + has access to every source": the UI offers only available sources and the + resolver skips the rest. +- **lookup** — ``(artist, album, metadata) -> cover_url | None``, calling an + EXISTING client method. Every lookup is individually guarded so any error or + miss degrades to ``None`` (the resolver then falls through to the next + source, finally to the download's own art) — a flaky source can never raise + into, or break, a download. + +Lookups are cached per album via ``build_art_lookup`` so resolving art for a +16-track album hits each source at most once. + +Client accessors are imported lazily inside each function to keep this module +import-light (the pure resolver + its tests never pull a network client). +""" + +from __future__ import annotations + +import re +from typing import Callable, Dict, List, Optional + +from utils.logging_config import get_logger + +from core.metadata.art_sources import ART_CAPABLE_SOURCES + +logger = get_logger("metadata.art_lookup") + +# Sources that need no account/config — always offered. +_FREE_SOURCES = ("caa", "deezer", "itunes", "audiodb") + + +# --------------------------------------------------------------------------- +# Availability +# --------------------------------------------------------------------------- + + +def _spotify_available() -> bool: + """Spotify art is only usable when the user is connected. Reuse the + canonical accessor, which returns a client only when authenticated.""" + try: + from core.metadata.registry import get_client_for_source + return get_client_for_source("spotify") is not None + except Exception: + return False + + +_AVAILABILITY: Dict[str, Callable[[], bool]] = { + "spotify": _spotify_available, +} + + +def is_art_source_available(source: str) -> bool: + """Is ``source`` usable right now? Free sources are always available; + account sources defer to their connection check. Unknown/unsupported + sources are never available.""" + name = (source or "").strip().lower() + if name not in ART_CAPABLE_SOURCES: + return False + if name in _FREE_SOURCES: + return True + check = _AVAILABILITY.get(name) + return bool(check()) if check else False + + +def available_art_sources() -> List[str]: + """The supported art sources currently usable for this user, in the + default priority order — for populating the settings UI.""" + return [s for s in ART_CAPABLE_SOURCES if is_art_source_available(s)] + + +# --------------------------------------------------------------------------- +# Album-match validation. Every client's search returns its top hit +# unvalidated (results[0]), so a source that lacks the album could hand back a +# DIFFERENT one — embedding wrong-album art, which is worse than the download's +# own cover. We therefore confirm the returned album matches the request before +# trusting its art; a mismatch returns None so the resolver falls through, +# preserving the "worst case = the cover you'd get today" guarantee. +# --------------------------------------------------------------------------- + +_STOPWORDS = {"the", "a", "an"} + + +def _norm(s) -> str: + return re.sub(r"[^a-z0-9]+", " ", str(s or "").lower()).strip() + + +def _significant_tokens(s) -> set: + return {t for t in _norm(s).split() if t not in _STOPWORDS} + + +def _result_album_artist(obj): + """Best-effort (album_name, artist_name) from a search result — handles + both raw dicts (Deezer/AudioDB) and dataclasses (iTunes/Spotify).""" + if isinstance(obj, dict): + name = obj.get("title") or obj.get("name") or obj.get("strAlbum") or "" + artist = obj.get("strArtist") or "" + a = obj.get("artist") + if not artist and isinstance(a, dict): + artist = a.get("name", "") + elif not artist and isinstance(a, str): + artist = a + return name, artist + name = getattr(obj, "name", None) or getattr(obj, "title", None) or "" + arts = getattr(obj, "artists", None) + if arts is None: + arts = getattr(obj, "artist", None) + if isinstance(arts, (list, tuple)): + artist = ", ".join(str(x) for x in arts if x) + else: + artist = str(arts) if arts else "" + return name, artist + + +def _album_matches(req_artist, req_album, got_artist, got_album) -> bool: + """True when the returned album plausibly IS the requested one. + + Both album and artist are compared by significant-token subset (stopwords + dropped), which tolerates leading articles, word order, "(Deluxe)"/ + "- Remastered" suffixes, punctuation, and "feat."/"&"/multi-artist ordering. + Lenient enough to never reject a genuine hit — a false reject just falls + back to today's art — yet strict enough to drop a different album (the + generic-title case, e.g. two "Greatest Hits", is caught by the artist gate).""" + ra, ga = _significant_tokens(req_album), _significant_tokens(got_album) + if not ra or not ga: + return False + if not (ra <= ga or ga <= ra): + return False + ta, tg = _significant_tokens(req_artist), _significant_tokens(got_artist) + if not ta: + return True # requested artist unknown -> album match suffices + if not tg: + return False # asked for an artist, none returned -> can't confirm + return ta <= tg or tg <= ta or bool(ta & tg) + + +# --------------------------------------------------------------------------- +# Per-source lookups — each returns a cover URL or None, never raises. +# Non-CAA sources validate the returned album before trusting its art. +# --------------------------------------------------------------------------- + + +def _caa_art(artist: str, album: str, metadata: dict) -> Optional[str]: + # Cover Art Archive is keyed by the MusicBrainz release id, so it's THE + # release's art by definition — no fuzzy match to validate. Resolves only + # once MusicBrainz enrichment has found the release. + mbid = (metadata or {}).get("musicbrainz_release_id") + if not mbid: + return None + return f"https://coverartarchive.org/release/{mbid}/front-1200" + + +def _deezer_art(artist: str, album: str, metadata: dict) -> Optional[str]: + from core.metadata.registry import get_deezer_client + client = get_deezer_client() + if not client: + return None + data = client.search_album(artist, album) + if not data: + return None + got_album, got_artist = _result_album_artist(data) + if not _album_matches(artist, album, got_artist, got_album): + return None + url = data.get("cover_xl") or data.get("cover_big") or data.get("cover_medium") + if not url: + return None + try: + from core.deezer_client import _upgrade_deezer_cover_url + return _upgrade_deezer_cover_url(url) + except Exception: + return url + + +def _itunes_art(artist: str, album: str, metadata: dict) -> Optional[str]: + from core.metadata.registry import get_itunes_client + client = get_itunes_client() + if not client: + return None + for alb in (client.search_albums(f"{artist} {album}") or []): + url = getattr(alb, "image_url", None) + if not url: + continue + got_album, got_artist = _result_album_artist(alb) + if _album_matches(artist, album, got_artist, got_album): + return url + return None + + +def _audiodb_art(artist: str, album: str, metadata: dict) -> Optional[str]: + from core.audiodb_client import AudioDBClient + data = AudioDBClient().search_album(artist, album) + if not data: + return None + got_album, got_artist = _result_album_artist(data) + if not _album_matches(artist, album, got_artist, got_album): + return None + return data.get("strAlbumThumb") or None + + +def _spotify_art(artist: str, album: str, metadata: dict) -> Optional[str]: + from core.metadata.registry import get_client_for_source + client = get_client_for_source("spotify") + if not client: + return None + for alb in (client.search_albums(f"{artist} {album}") or []): + url = getattr(alb, "image_url", None) + if not url: + continue + got_album, got_artist = _result_album_artist(alb) + if _album_matches(artist, album, got_artist, got_album): + return url + return None + + +_LOOKUPS: Dict[str, Callable[[str, str, dict], Optional[str]]] = { + "caa": _caa_art, + "deezer": _deezer_art, + "itunes": _itunes_art, + "audiodb": _audiodb_art, + "spotify": _spotify_art, +} + + +def select_preferred_art_url( + artist: Optional[str], + album: Optional[str], + metadata: Optional[dict], + configured_order, +) -> Optional[str]: + """Pick a cover-art URL from the user's configured source order, or None. + + ``None`` means "feature off, or nothing in the list resolved" — the caller + then keeps its existing art (today's behavior), so the worst case is simply + the cover you'd get today. This is the single entry point the art pipeline + calls; it's a no-op (returns ``None`` immediately) unless ``album_art_order`` + is an explicit non-empty list, which keeps every existing install untouched. + """ + if not isinstance(configured_order, (list, tuple)) or not configured_order: + return None + from core.metadata.art_sources import effective_art_order, resolve_cover_art + order = [s for s in effective_art_order(configured_order) if is_art_source_available(s)] + if not order: + return None + lookup = build_art_lookup(artist or "", album or "", metadata or {}) + url, _src = resolve_cover_art(order, lookup) + return url + + +def build_art_lookup( + artist: str, + album: str, + metadata: Optional[dict] = None, +) -> Callable[[str], Optional[str]]: + """Return a ``source_name -> cover_url | None`` callable for one album, + suitable to pass straight to ``art_sources.resolve_cover_art``. Results are + cached per source so re-resolving across an album's tracks costs at most one + lookup per source, and every lookup is guarded (errors → None).""" + meta = metadata or {} + cache: Dict[str, Optional[str]] = {} + + def lookup(source: str) -> Optional[str]: + name = (source or "").strip().lower() + if name in cache: + return cache[name] + fn = _LOOKUPS.get(name) + url: Optional[str] = None + if fn is not None: + try: + url = fn(artist, album, meta) + except Exception as exc: + logger.debug("[art] %s lookup failed: %s", name, exc) + url = None + cache[name] = url + return url + + return lookup diff --git a/core/metadata/art_sources.py b/core/metadata/art_sources.py new file mode 100644 index 00000000..98dfdd3e --- /dev/null +++ b/core/metadata/art_sources.py @@ -0,0 +1,106 @@ +"""Cover-art source selection. + +Picks album cover art from a user-ordered list of sources, falling back down +the list and finally to the download's own art when nothing in the list +resolves. This generalizes the legacy single ``prefer_caa_art`` toggle into an +ordered, mix-and-match preference (Sokhi's request) while preserving today's +behavior byte-for-byte when no order is configured. + +The module is deliberately pure and import-light: the ordering + fallback +contract is unit-testable without network, config, or a DB. The actual +per-source art lookups are injected as callables (a registry the caller +builds), so this module never imports a metadata client — that keeps the +selection logic fast to test and impossible to break with a client-side +network change. +""" + +from __future__ import annotations + +from typing import Callable, Optional, Sequence, Tuple + +# Sources we can reliably pull album cover art from, in a sensible default +# priority. This is the set the UI offers and that ``effective_art_order`` +# accepts — anything else in a saved order is filtered out. +# +# Genius (lyrics) and Last.fm (deprecated/unreliable images) are excluded — no +# dependable album covers. Tidal/Qobuz/HiFi are deferred: their album lookups +# return IDs that need cover-URL construction and they lack a clean core-side +# client accessor, so rather than ship extraction that silently yields nothing +# we add them once that's verified. The current set covers the universally +# available free sources plus Spotify (the common connected account source). +ART_CAPABLE_SOURCES: Tuple[str, ...] = ( + "caa", "deezer", "itunes", "spotify", "audiodb", +) + +# Minimum byte size for a fetched image to count as a real cover. Mirrors the +# existing Cover Art Archive guard in ``artwork.py`` (a 1x1 pixel, a +# placeholder, or a truncated download is a miss, not a hit). +MIN_VALID_ART_BYTES = 1000 + + +def effective_art_order( + order, + *, + prefer_caa_art: bool = False, +) -> list: + """Resolve the configured art-source order into a concrete priority list. + + Rules (in priority): + - A configured non-empty list wins. It's lower-cased, trimmed, filtered to + known art-capable sources, and de-duplicated (first occurrence kept). + - An empty / missing / all-invalid list preserves **legacy behavior**: + ``['caa']`` when ``prefer_caa_art`` is on (Cover Art Archive first, then + the download's own art), else ``[]`` (use the download's own art only). + + The empty-list case is what makes the feature non-breaking: an install that + has never touched the new setting resolves to exactly today's logic. + """ + if isinstance(order, (list, tuple)): + seen = set() + deduped = [] + for raw in order: + name = str(raw).strip().lower() + if not name or name not in ART_CAPABLE_SOURCES or name in seen: + continue + seen.add(name) + deduped.append(name) + if deduped: + return deduped + return ["caa"] if prefer_caa_art else [] + + +def resolve_cover_art( + order: Sequence[str], + lookup: Callable[[str], Optional[str]], + *, + validate: Optional[Callable[[str, str], bool]] = None, +) -> Tuple[Optional[str], Optional[str]]: + """Walk ``order`` and return ``(art_url, source_name)`` for the first source + whose ``lookup(source)`` yields a URL that passes ``validate`` (if given). + + Returns ``(None, None)`` when nothing in the list resolves — the caller then + falls back to the download's own art (today's default), so art is never + *worse* than before. + + Robustness contract: + - ``lookup`` is ``source_name -> url | None``. A source that returns a + falsy value is skipped. + - An exception raised by ``lookup`` or ``validate`` for one source is + swallowed and treated as a miss, so a single flaky source can never + abort the whole chain (or the download). + """ + for source in order: + try: + url = lookup(source) + except Exception: + url = None + if not url: + continue + if validate is not None: + try: + if not validate(source, url): + continue + except Exception: + continue + return url, source + return None, None diff --git a/core/metadata/artwork.py b/core/metadata/artwork.py index 0da28c58..f9b159fa 100644 --- a/core/metadata/artwork.py +++ b/core/metadata/artwork.py @@ -8,7 +8,7 @@ import urllib.request from ipaddress import ip_address from urllib.parse import quote, urlparse -from core.imports.context import get_import_context_album +from core.imports.context import get_import_context_album, get_import_context_artist from core.metadata.common import ( get_config_manager, get_image_dimensions, @@ -340,8 +340,27 @@ def embed_album_art_metadata(audio_file, metadata: dict): image_data = None mime_type = None + # User-preferred cover-art source. When album_art_order is a non-empty + # list it is the SOLE authority for preferred art (put 'caa' in it to use + # Cover Art Archive), and the legacy prefer_caa_art toggle below is + # skipped. With no list this is a no-op and behavior is exactly as before. + album_art_order = cfg.get("metadata_enhancement.album_art_order") + art_list_active = isinstance(album_art_order, (list, tuple)) and len(album_art_order) > 0 + try: + from core.metadata.art_lookup import select_preferred_art_url + preferred_url = select_preferred_art_url( + metadata.get("album_artist") or metadata.get("artist"), + metadata.get("album"), + metadata, + album_art_order, + ) + if preferred_url: + image_data, mime_type = _fetch_art_bytes(preferred_url) + except Exception as exc: + logger.debug("Preferred art-source selection failed: %s", exc) + release_mbid = metadata.get("musicbrainz_release_id") - if release_mbid and cfg.get("metadata_enhancement.prefer_caa_art", False): + if not image_data and not art_list_active and release_mbid and cfg.get("metadata_enhancement.prefer_caa_art", False): try: # 1200px CDN thumbnail, not the flaky bare /front original. caa_url = f"https://coverartarchive.org/release/{release_mbid}/front-1200" @@ -395,7 +414,12 @@ def download_cover_art(album_info: dict, target_dir: str, context: dict = None): cover_path = os.path.join(target_dir, "cover.jpg") album_info = album_info or {} release_mbid = album_info.get("musicbrainz_release_id") - prefer_caa = cfg.get("metadata_enhancement.prefer_caa_art", False) + # When a preferred-art priority list is configured it is the sole + # authority, so the legacy CAA toggle is neutralized for this whole + # function (it gates the existing-file upgrade logic too). + _art_order = cfg.get("metadata_enhancement.album_art_order") + _art_list_active = isinstance(_art_order, (list, tuple)) and len(_art_order) > 0 + prefer_caa = cfg.get("metadata_enhancement.prefer_caa_art", False) and not _art_list_active if os.path.exists(cover_path): if release_mbid and prefer_caa: @@ -412,7 +436,27 @@ def download_cover_art(album_info: dict, target_dir: str, context: dict = None): is_upgrade = False image_data = None - if release_mbid and prefer_caa: + + # User-preferred cover-art source (no-op unless album_art_order is set). + # cover.jpg only supports the artist+album sources here (no MBID in + # album_info), which matches today's CAA-only special-casing. + try: + from core.metadata.art_lookup import select_preferred_art_url + artist_ctx = get_import_context_artist(context) if context else {} + preferred_url = select_preferred_art_url( + (artist_ctx or {}).get("name"), + album_info.get("album_name"), + album_info, + cfg.get("metadata_enhancement.album_art_order"), + ) + if preferred_url: + pref_data, _ = _fetch_art_bytes(preferred_url) + if pref_data and len(pref_data) > 1000: + image_data = pref_data + except Exception as exc: + logger.debug("Preferred art-source selection failed: %s", exc) + + if not image_data and release_mbid and prefer_caa: try: # 1200px CDN thumbnail, not the flaky bare /front original. caa_url = f"https://coverartarchive.org/release/{release_mbid}/front-1200" diff --git a/tests/metadata/test_art_lookup.py b/tests/metadata/test_art_lookup.py new file mode 100644 index 00000000..9ca9c1bf --- /dev/null +++ b/tests/metadata/test_art_lookup.py @@ -0,0 +1,248 @@ +"""Seam tests for the per-source album-art lookups + availability. + +Real clients are stubbed (monkeypatched at the lazy-import sites), so these +exercise the field-extraction, caching, guarding, and availability gating +without any network. +""" + +from __future__ import annotations + +from types import SimpleNamespace +from unittest.mock import MagicMock + +import pytest + +from core.metadata import art_lookup + + +# --------------------------------------------------------------------------- +# Availability — "not everybody has every source" +# --------------------------------------------------------------------------- + + +def test_free_sources_always_available(): + for s in ("caa", "deezer", "itunes", "audiodb"): + assert art_lookup.is_art_source_available(s) is True + + +def test_unknown_or_unsupported_source_unavailable(): + assert art_lookup.is_art_source_available("tidal") is False # deferred + assert art_lookup.is_art_source_available("genius") is False + assert art_lookup.is_art_source_available("") is False + + +def test_spotify_availability_follows_connection(monkeypatch): + import core.metadata.registry as registry + monkeypatch.setattr(registry, "get_client_for_source", lambda s: object()) + assert art_lookup.is_art_source_available("spotify") is True + monkeypatch.setattr(registry, "get_client_for_source", lambda s: None) + assert art_lookup.is_art_source_available("spotify") is False + + +def test_spotify_availability_swallows_errors(monkeypatch): + import core.metadata.registry as registry + def _boom(_s): + raise RuntimeError("registry down") + monkeypatch.setattr(registry, "get_client_for_source", _boom) + assert art_lookup.is_art_source_available("spotify") is False + + +def test_available_sources_lists_free_plus_connected_spotify(monkeypatch): + import core.metadata.registry as registry + monkeypatch.setattr(registry, "get_client_for_source", lambda s: object()) + avail = art_lookup.available_art_sources() + assert avail == ["caa", "deezer", "itunes", "spotify", "audiodb"] + monkeypatch.setattr(registry, "get_client_for_source", lambda s: None) + assert "spotify" not in art_lookup.available_art_sources() + + +# --------------------------------------------------------------------------- +# Per-source extraction +# --------------------------------------------------------------------------- + + +def test_caa_art_builds_url_from_release_mbid(): + url = art_lookup._caa_art("A", "B", {"musicbrainz_release_id": "abc-123"}) + assert url == "https://coverartarchive.org/release/abc-123/front-1200" + + +def test_caa_art_none_without_mbid(): + assert art_lookup._caa_art("A", "B", {}) is None + + +def test_deezer_art_extracts_and_prefers_largest(monkeypatch): + import core.metadata.registry as registry + client = MagicMock() + client.search_album.return_value = {"title": "B", "artist": {"name": "A"}, + "cover_big": "http://x/big.jpg", + "cover_xl": "http://x/xl.jpg"} + monkeypatch.setattr(registry, "get_deezer_client", lambda *a, **k: client) + # _upgrade_deezer_cover_url returns non-Deezer URLs unchanged. + assert art_lookup._deezer_art("A", "B", {}) == "http://x/xl.jpg" + + +def test_deezer_art_none_when_no_cover_fields(monkeypatch): + import core.metadata.registry as registry + client = MagicMock() + # Matches the album but carries no cover_* keys. + client.search_album.return_value = {"title": "B", "artist": {"name": "A"}, "id": 1} + monkeypatch.setattr(registry, "get_deezer_client", lambda *a, **k: client) + assert art_lookup._deezer_art("A", "B", {}) is None + + +def test_deezer_art_rejects_wrong_album(monkeypatch): + import core.metadata.registry as registry + client = MagicMock() + client.search_album.return_value = {"title": "A Totally Different Record", + "artist": {"name": "A"}, + "cover_xl": "http://x/wrong.jpg"} + monkeypatch.setattr(registry, "get_deezer_client", lambda *a, **k: client) + # Wrong album -> no art (falls back to today's cover), never wrong art. + assert art_lookup._deezer_art("A", "B", {}) is None + + +def test_deezer_art_rejects_wrong_artist(monkeypatch): + import core.metadata.registry as registry + client = MagicMock() + client.search_album.return_value = {"title": "21", "artist": {"name": "Someone Else"}, + "cover_xl": "http://x/wrong.jpg"} + monkeypatch.setattr(registry, "get_deezer_client", lambda *a, **k: client) + # Album matches but the artist doesn't -> reject (don't embed wrong art). + assert art_lookup._deezer_art("Adele", "21", {}) is None + + +def test_itunes_art_returns_first_matching_album_image(monkeypatch): + import core.metadata.registry as registry + client = MagicMock() + client.search_albums.return_value = [ + SimpleNamespace(name="B", artists=["A"], image_url=None), # match but no art -> skip + SimpleNamespace(name="Wrong", artists=["A"], image_url="http://it/wrong.jpg"), # art but wrong album -> skip + SimpleNamespace(name="B (Deluxe)", artists=["A"], image_url="http://it/600.jpg"), # match + art + ] + monkeypatch.setattr(registry, "get_itunes_client", lambda *a, **k: client) + assert art_lookup._itunes_art("A", "B", {}) == "http://it/600.jpg" + + +def test_audiodb_art_extracts_thumb(monkeypatch): + import core.audiodb_client as adb + fake = MagicMock() + fake.search_album.return_value = {"strAlbum": "B", "strArtist": "A", + "strAlbumThumb": "http://adb/cover.jpg"} + monkeypatch.setattr(adb, "AudioDBClient", lambda *a, **k: fake) + assert art_lookup._audiodb_art("A", "B", {}) == "http://adb/cover.jpg" + + +def test_spotify_art_uses_connected_client(monkeypatch): + import core.metadata.registry as registry + client = MagicMock() + client.search_albums.return_value = [ + SimpleNamespace(name="B", artists=["A"], image_url="http://sp/640.jpg")] + monkeypatch.setattr(registry, "get_client_for_source", lambda s: client) + assert art_lookup._spotify_art("A", "B", {}) == "http://sp/640.jpg" + + +# --- album-match validation (the wrong-art guard) --- + + +def test_album_matches_exact_and_suffix_variants(): + assert art_lookup._album_matches("Taylor Swift", "1989", "Taylor Swift", "1989") + assert art_lookup._album_matches("Taylor Swift", "1989", "Taylor Swift", "1989 (Deluxe)") + assert art_lookup._album_matches("Pink Floyd", "The Dark Side of the Moon", + "Pink Floyd", "Dark Side of the Moon - Remastered") + + +def test_album_matches_multi_artist_and_feat(): + assert art_lookup._album_matches("Drake", "Scorpion", "Drake & Future", "Scorpion") + assert art_lookup._album_matches("Tyler, The Creator", "IGOR", "Tyler The Creator", "IGOR") + + +def test_album_matches_rejects_wrong_album_or_artist(): + assert not art_lookup._album_matches("Adele", "21", "Adele", "Completely Different") + # Generic album title, different artist -> the artist gate rejects it. + assert not art_lookup._album_matches("Coldplay", "Greatest Hits", "Other Band", "Greatest Hits") + assert not art_lookup._album_matches("Adele", "21", "Adele", "") + assert not art_lookup._album_matches("Adele", "", "Adele", "21") + + +def test_album_matches_unknown_requested_artist_allows_album_match(): + # cover.jpg path may lack artist context -> album match alone suffices. + assert art_lookup._album_matches("", "1989", "Taylor Swift", "1989") + + +# --------------------------------------------------------------------------- +# build_art_lookup — caching + guarding +# --------------------------------------------------------------------------- + + +def test_lookup_is_cached_per_source(monkeypatch): + import core.metadata.registry as registry + client = MagicMock() + client.search_album.return_value = {"title": "B", "artist": {"name": "A"}, + "cover_xl": "http://x/xl.jpg"} + monkeypatch.setattr(registry, "get_deezer_client", lambda *a, **k: client) + lookup = art_lookup.build_art_lookup("A", "B", {}) + first = lookup("deezer") + second = lookup("deezer") + assert first == second == "http://x/xl.jpg" + # Cached: the underlying client was only hit once across both calls. + assert client.search_album.call_count == 1 + + +def test_lookup_guards_source_exceptions(monkeypatch): + import core.metadata.registry as registry + def _boom(*a, **k): + raise RuntimeError("network down") + monkeypatch.setattr(registry, "get_deezer_client", _boom) + lookup = art_lookup.build_art_lookup("A", "B", {}) + assert lookup("deezer") is None # swallowed, not raised + + +def test_lookup_unknown_source_returns_none(): + lookup = art_lookup.build_art_lookup("A", "B", {}) + assert lookup("tidal") is None + assert lookup("bogus") is None + + +# --------------------------------------------------------------------------- +# select_preferred_art_url — the gate the artwork pipeline calls +# --------------------------------------------------------------------------- + + +def test_selector_feature_off_returns_none(): + # The critical non-breaking case: no configured order -> no-op, caller + # keeps today's art. + assert art_lookup.select_preferred_art_url("A", "B", {}, None) is None + assert art_lookup.select_preferred_art_url("A", "B", {}, []) is None + assert art_lookup.select_preferred_art_url("A", "B", {}, "deezer") is None + + +def test_selector_resolves_first_available_source(monkeypatch): + import core.metadata.registry as registry + client = MagicMock() + client.search_album.return_value = {"title": "B", "artist": {"name": "A"}, + "cover_xl": "http://x/xl.jpg"} + monkeypatch.setattr(registry, "get_deezer_client", lambda *a, **k: client) + # Order lists an unsupported source first (filtered out), then deezer. + url = art_lookup.select_preferred_art_url("A", "B", {}, ["tidal", "deezer"]) + assert url == "http://x/xl.jpg" + + +def test_selector_none_when_order_has_no_available_sources(monkeypatch): + import core.metadata.registry as registry + monkeypatch.setattr(registry, "get_client_for_source", lambda s: None) # spotify off + # 'tidal' unsupported, 'spotify' unavailable -> empty effective order. + assert art_lookup.select_preferred_art_url("A", "B", {}, ["tidal", "spotify"]) is None + + +def test_selector_none_when_nothing_resolves(monkeypatch): + import core.metadata.registry as registry + client = MagicMock() + client.search_album.return_value = None # deezer has no match + monkeypatch.setattr(registry, "get_deezer_client", lambda *a, **k: client) + assert art_lookup.select_preferred_art_url("A", "B", {}, ["deezer"]) is None + + +def test_selector_caa_uses_release_mbid(monkeypatch): + url = art_lookup.select_preferred_art_url( + "A", "B", {"musicbrainz_release_id": "mbid-9"}, ["caa"]) + assert url == "https://coverartarchive.org/release/mbid-9/front-1200" diff --git a/tests/metadata/test_art_sources.py b/tests/metadata/test_art_sources.py new file mode 100644 index 00000000..0e70c383 --- /dev/null +++ b/tests/metadata/test_art_sources.py @@ -0,0 +1,114 @@ +"""Unit tests for the pure cover-art source selection logic. + +Pins the ordering + fallback + back-compat contract that the artwork +integration relies on. No network, config, or DB — the per-source lookups are +injected, so these tests are fast and deterministic. +""" + +from __future__ import annotations + +from core.metadata.art_sources import ( + ART_CAPABLE_SOURCES, + effective_art_order, + resolve_cover_art, +) + + +# --------------------------------------------------------------------------- +# effective_art_order — config resolution + legacy back-compat +# --------------------------------------------------------------------------- + + +def test_configured_order_wins_and_is_normalized(): + assert effective_art_order(["Deezer", " CAA ", "iTunes"]) == ["deezer", "caa", "itunes"] + + +def test_unknown_sources_filtered_out(): + # 'genius'/'lastfm' aren't art-capable; 'bogus' is unknown. + assert effective_art_order(["genius", "deezer", "bogus", "lastfm"]) == ["deezer"] + + +def test_duplicates_collapsed_keeping_first_position(): + assert effective_art_order(["deezer", "caa", "deezer", "CAA"]) == ["deezer", "caa"] + + +def test_empty_order_with_prefer_caa_is_legacy_caa_first(): + # Back-compat: an un-migrated install with prefer_caa_art on behaves as + # 'CAA first, then the download's own art' — exactly today's logic. + assert effective_art_order([], prefer_caa_art=True) == ["caa"] + assert effective_art_order(None, prefer_caa_art=True) == ["caa"] + + +def test_empty_order_without_prefer_caa_is_default_only(): + # The critical non-breaking case: no list + no prefer_caa => empty order, + # so the caller uses the download's own art (today's default). + assert effective_art_order([], prefer_caa_art=False) == [] + assert effective_art_order(None) == [] + assert effective_art_order("not-a-list") == [] + + +def test_all_invalid_entries_fall_back_to_legacy(): + assert effective_art_order(["genius", "lastfm"], prefer_caa_art=True) == ["caa"] + assert effective_art_order(["genius", "lastfm"], prefer_caa_art=False) == [] + + +def test_art_capable_sources_excludes_lyrics_only_sources(): + assert "caa" in ART_CAPABLE_SOURCES + assert "deezer" in ART_CAPABLE_SOURCES + assert "genius" not in ART_CAPABLE_SOURCES + assert "lastfm" not in ART_CAPABLE_SOURCES + + +# --------------------------------------------------------------------------- +# resolve_cover_art — ordered walk + fallback + robustness +# --------------------------------------------------------------------------- + + +def test_first_source_with_art_wins(): + art = {"caa": "http://caa/x.jpg", "deezer": "http://dz/y.jpg"} + url, src = resolve_cover_art(["deezer", "caa"], art.get) + assert (url, src) == ("http://dz/y.jpg", "deezer") + + +def test_falls_through_to_next_source_when_missing(): + art = {"deezer": None, "caa": "http://caa/x.jpg"} + url, src = resolve_cover_art(["deezer", "caa"], art.get) + assert (url, src) == ("http://caa/x.jpg", "caa") + + +def test_returns_none_when_nothing_resolves(): + url, src = resolve_cover_art(["deezer", "caa"], lambda _s: None) + assert (url, src) == (None, None) + + +def test_empty_order_returns_none_so_caller_uses_default(): + url, src = resolve_cover_art([], lambda _s: "http://should/not/be/called.jpg") + assert (url, src) == (None, None) + + +def test_validate_rejection_skips_to_next_source(): + art = {"deezer": "http://dz/tiny.jpg", "caa": "http://caa/big.jpg"} + # Pretend deezer's image fails validation (e.g. too small / placeholder). + def _validate(source, url): + return source != "deezer" + url, src = resolve_cover_art(["deezer", "caa"], art.get, validate=_validate) + assert (url, src) == ("http://caa/big.jpg", "caa") + + +def test_lookup_exception_is_treated_as_miss_not_fatal(): + def _lookup(source): + if source == "deezer": + raise RuntimeError("network down") + return "http://caa/x.jpg" + url, src = resolve_cover_art(["deezer", "caa"], _lookup) + assert (url, src) == ("http://caa/x.jpg", "caa") + + +def test_validate_exception_is_treated_as_miss(): + def _validate(source, url): + if source == "deezer": + raise ValueError("bad image header") + return True + art = {"deezer": "http://dz/x.jpg", "caa": "http://caa/x.jpg"} + url, src = resolve_cover_art(["deezer", "caa"], art.get, validate=_validate) + assert (url, src) == ("http://caa/x.jpg", "caa") diff --git a/web_server.py b/web_server.py index b3d6499f..d7719c50 100644 --- a/web_server.py +++ b/web_server.py @@ -2996,6 +2996,28 @@ def handle_settings(): return jsonify({"error": str(e)}), 500 +@app.route('/api/metadata/art-sources', methods=['GET']) +def get_art_sources(): + """Cover-art sources the current user can actually use, in default order. + + Free sources (CAA/Deezer/iTunes/AudioDB) are always available; account + sources (Spotify) only when connected. The settings UI uses this to offer + only sources the user is set up with ("not everybody has every source"). + """ + try: + from core.metadata.art_lookup import available_art_sources + labels = { + 'caa': 'Cover Art Archive', 'deezer': 'Deezer', 'itunes': 'iTunes', + 'spotify': 'Spotify', 'audiodb': 'TheAudioDB', + } + available = [{'id': s, 'name': labels.get(s, s.title())} + for s in available_art_sources()] + return jsonify({'available': available}) + except Exception as e: + logger.error(f"Error listing art sources: {e}") + return jsonify({'available': [], 'error': str(e)}), 500 + + @app.route('/api/dev-mode', methods=['GET', 'POST']) def handle_dev_mode(): global dev_mode_enabled diff --git a/webui/index.html b/webui/index.html index a17a9b9e..659ba7f4 100644 --- a/webui/index.html +++ b/webui/index.html @@ -5470,7 +5470,12 @@ Use MusicBrainz Cover Art Archive for album art - Higher resolution but quality may vary. When off, uses Spotify/iTunes/Deezer art (consistent 640x640). + Higher resolution but quality may vary. When off, uses Spotify/iTunes/Deezer art (consistent 640x640). Ignored when a preferred album art source list is set below — add Cover Art Archive to that list instead. + +