From 6dca19ca1eefefe840001ecc560a60e55e16232d Mon Sep 17 00:00:00 2001 From: Antti Kettunen Date: Wed, 15 Apr 2026 19:35:18 +0300 Subject: [PATCH 1/5] Use metadata source priority in unknown artist fixer job Unknown artist resolution now uses the shared metadata source priority and only filters to the sources that can actually participate in this job. Deezer and iTunes remain direct lookup sources, while Hydrabase can now join the title-search path when it is the configured priority source. --- core/repair_jobs/unknown_artist_fixer.py | 234 ++++++++++++++-------- tests/test_unknown_artist_fixer.py | 238 +++++++++++++++++++++++ 2 files changed, 388 insertions(+), 84 deletions(-) create mode 100644 tests/test_unknown_artist_fixer.py diff --git a/core/repair_jobs/unknown_artist_fixer.py b/core/repair_jobs/unknown_artist_fixer.py index c81663c1..01086989 100644 --- a/core/repair_jobs/unknown_artist_fixer.py +++ b/core/repair_jobs/unknown_artist_fixer.py @@ -8,9 +8,8 @@ import os import re import shutil import sys -import time -from core.metadata_service import get_client_for_source, get_primary_client, get_primary_source +from core.metadata_service import get_client_for_source, get_primary_source, get_source_priority from core.repair_jobs import register_job from core.repair_jobs.base import JobContext, JobResult, RepairJob from utils.logging_config import get_logger @@ -18,6 +17,8 @@ from utils.logging_config import get_logger logger = get_logger("repair_job.unknown_artist_fixer") _UNKNOWN_NAMES = {'unknown artist', 'unknown', ''} +_TRACK_ID_SOURCES = {'spotify', 'deezer', 'itunes'} +_TITLE_SEARCH_SOURCES = {'spotify', 'deezer', 'itunes', 'hydrabase'} # Sidecar extensions to move alongside audio files _SIDECAR_EXTS = {'.lrc', '.jpg', '.jpeg', '.png', '.nfo', '.txt', '.cue'} @@ -249,6 +250,7 @@ class UnknownArtistFixerJob(RepairJob): Returns dict with artist, album, track_number, year, etc. or None.""" title = track['title'] or '' + primary_source = get_primary_source() # Priority 1: Read embedded file tags try: @@ -269,96 +271,160 @@ class UnknownArtistFixerJob(RepairJob): except Exception as e: logger.debug(f"Failed to read tags from {resolved_path}: {e}") - # Priority 2: Look up by source track ID using the appropriate client. - # Try the primary source's ID first, then fall back to any available ID - # with its matching client so we never pass a Deezer/iTunes ID to Spotify - # (or vice-versa). - _primary = get_primary_source() - _id_candidates = [] - for _src in [_primary] + [s for s in ('spotify', 'deezer', 'itunes') if s != _primary]: - _tid = track.get(f'{_src}_track_id') - if _tid: - _id_candidates.append((_src, _tid)) - source_id = None - _lookup_client = None - for _src, _tid in _id_candidates: - _c = get_client_for_source(_src) - if _c: - source_id = _tid - _lookup_client = _c - break - if source_id and _lookup_client: + # Priority 2: Look up by source track ID + for source, source_id in self._iter_source_track_ids(track, primary_source): + client = get_client_for_source(source) + if not client or not hasattr(client, 'get_track_details'): + continue try: - details = _lookup_client.get_track_details(str(source_id)) - if details and details.get('primary_artist'): - artist = details['primary_artist'] - if artist.lower() not in _UNKNOWN_NAMES: - album = details.get('album', {}) - album_name = album.get('name', '') if isinstance(album, dict) else str(album) - return { - 'artist': artist, - 'album': album_name, - 'title': details.get('name', title), - 'track_number': details.get('track_number'), - 'disc_number': details.get('disc_number', 1), - 'year': (album.get('release_date', '') or '')[:4] if isinstance(album, dict) else '', - 'image_url': album.get('images', [{}])[0].get('url', '') if isinstance(album, dict) and album.get('images') else '', - 'source': 'track_id_lookup', - 'confidence': 0.95, - } + details = client.get_track_details(str(source_id)) + corrected = self._build_corrected_metadata( + details, + fallback_title=title, + source=f"{source}_track_id_lookup", + confidence=0.95, + ) + if corrected: + return corrected except Exception as e: - logger.debug(f"Track ID lookup failed for {source_id}: {e}") + logger.debug(f"Track ID lookup failed for {source} {source_id}: {e}") - # Priority 3: Search by title using the configured primary metadata source - _search_client = get_primary_client() - if title and _search_client: - try: - results = _search_client.search_tracks(title, limit=5) - if results: - # Score candidates - from difflib import SequenceMatcher - best = None - best_score = 0 - for r in results: - name_sim = SequenceMatcher(None, title.lower(), r.name.lower()).ratio() - # Boost if album matches - album_name = r.album if hasattr(r, 'album') else '' - if album_name and track.get('album_title'): - album_sim = SequenceMatcher(None, track['album_title'].lower(), album_name.lower()).ratio() - name_sim = (name_sim * 0.7) + (album_sim * 0.3) - if name_sim > best_score: - best_score = name_sim - best = r + # Priority 3: Search by title + if title: + for source in self._iter_source_priority(primary_source, _TITLE_SEARCH_SOURCES): + client = get_client_for_source(source) + if not client or not hasattr(client, 'search_tracks'): + continue + try: + results = client.search_tracks(title, limit=5) + if not results: + continue - if best and best_score >= 0.7: - artist = best.artists[0] if best.artists else '' - if artist and artist.lower() not in _UNKNOWN_NAMES: - # Get full details for track_number + best, best_score = self._pick_best_track_candidate(title, track.get('album_title'), results) + if not best or best_score < 0.7: + continue + + full_details = None + if hasattr(client, 'get_track_details') and getattr(best, 'id', None): + try: + full_details = client.get_track_details(str(best.id)) + except Exception: full_details = None - try: - full_details = _search_client.get_track_details(best.id) - except Exception: - pass - album_data = full_details.get('album', {}) if full_details else {} - return { - 'artist': artist, - 'album': best.album if hasattr(best, 'album') else '', - 'title': best.name, - 'track_number': full_details.get('track_number') if full_details else None, - 'disc_number': full_details.get('disc_number', 1) if full_details else 1, - 'year': (album_data.get('release_date', '') or '')[:4] if isinstance(album_data, dict) else '', - 'image_url': getattr(best, 'image_url', '') or '', - 'source': 'title_search', - 'confidence': round(best_score, 3), - } - except Exception as e: - logger.debug(f"Title search failed for '{title}': {e}") - # Rate limit courtesy - if context.sleep_or_stop(0.2): - return None + + corrected = self._build_corrected_metadata( + full_details or best, + fallback_title=title, + source=f"{source}_title_search", + confidence=round(best_score, 3), + ) + if corrected: + return corrected + except Exception as e: + logger.debug(f"Title search failed for '{title}' via {source}: {e}") + # Rate limit courtesy + if context.sleep_or_stop(0.2): + return None return None + @staticmethod + def _get_track_value(payload, key, default=None): + if isinstance(payload, dict): + return payload.get(key, default) + return getattr(payload, key, default) + + def _iter_source_track_ids(self, track: dict, primary_source: str): + source_fields = { + 'spotify': 'spotify_track_id', + 'deezer': 'deezer_track_id', + 'itunes': 'itunes_track_id', + } + ordered_sources = [source for source in self._iter_source_priority(primary_source, _TRACK_ID_SOURCES) if source in source_fields] + for source in ordered_sources: + source_id = track.get(source_fields[source]) + if source_id: + yield source, source_id + + @staticmethod + def _iter_source_priority(primary_source: str, allowed_sources: set[str]): + return [source for source in get_source_priority(primary_source) if source in allowed_sources] + + def _pick_best_track_candidate(self, title: str, album_title: str, results): + from difflib import SequenceMatcher + + best = None + best_score = 0.0 + title_lower = title.lower() + album_lower = album_title.lower() if album_title else '' + + for candidate in results: + candidate_name = self._get_track_value(candidate, 'name', '') or '' + if not candidate_name: + continue + name_sim = SequenceMatcher(None, title_lower, candidate_name.lower()).ratio() + + candidate_album = self._get_track_value(candidate, 'album', '') or '' + if album_lower and candidate_album: + if isinstance(candidate_album, dict): + candidate_album = candidate_album.get('name') or candidate_album.get('title') or '' + album_sim = SequenceMatcher(None, album_lower, str(candidate_album).lower()).ratio() + name_sim = (name_sim * 0.7) + (album_sim * 0.3) + + if name_sim > best_score: + best_score = name_sim + best = candidate + + return best, best_score + + def _build_corrected_metadata(self, payload, fallback_title: str, source: str, confidence: float): + if not payload: + return None + + artist = self._get_track_value(payload, 'primary_artist', '') or '' + artists = self._get_track_value(payload, 'artists', []) or [] + if not artist and artists: + if isinstance(artists, list): + first_artist = artists[0] + if isinstance(first_artist, dict): + artist = first_artist.get('name', '') + else: + artist = str(first_artist) + + artist = (artist or '').strip() + if not artist or artist.lower() in _UNKNOWN_NAMES: + return None + + album = self._get_track_value(payload, 'album', {}) or {} + if isinstance(album, dict): + album_name = album.get('name', '') or album.get('title', '') or '' + year = (album.get('release_date', '') or '')[:4] + image_url = '' + images = album.get('images') or [] + if images: + first_image = images[0] + if isinstance(first_image, dict): + image_url = first_image.get('url', '') or '' + else: + album_name = str(album) + year = '' + image_url = '' + + image_url = self._get_track_value(payload, 'image_url', image_url) or image_url + + title = self._get_track_value(payload, 'name', fallback_title) or fallback_title + + return { + 'artist': artist, + 'album': album_name, + 'title': title, + 'track_number': self._get_track_value(payload, 'track_number'), + 'disc_number': self._get_track_value(payload, 'disc_number', 1) or 1, + 'year': year, + 'image_url': image_url, + 'source': source, + 'confidence': confidence, + } + def _apply_fix(self, context, track, corrected, resolved_path, expected_rel, transfer, fix_tags, reorganize_files): """Apply the fix: re-tag file, move to correct path, update DB.""" diff --git a/tests/test_unknown_artist_fixer.py b/tests/test_unknown_artist_fixer.py new file mode 100644 index 00000000..2c6d7c90 --- /dev/null +++ b/tests/test_unknown_artist_fixer.py @@ -0,0 +1,238 @@ +import sys +import types +from types import SimpleNamespace + +if "spotipy" not in sys.modules: + spotipy = types.ModuleType("spotipy") + + class _DummySpotify: + def __init__(self, *args, **kwargs): + pass + + oauth2 = types.ModuleType("spotipy.oauth2") + + class _DummyOAuth: + def __init__(self, *args, **kwargs): + pass + + spotipy.Spotify = _DummySpotify + oauth2.SpotifyOAuth = _DummyOAuth + oauth2.SpotifyClientCredentials = _DummyOAuth + spotipy.oauth2 = oauth2 + sys.modules["spotipy"] = spotipy + sys.modules["spotipy.oauth2"] = oauth2 + +if "config.settings" not in sys.modules: + config_pkg = types.ModuleType("config") + settings_mod = types.ModuleType("config.settings") + + class _DummyConfigManager: + def get(self, key, default=None): + return default + + settings_mod.config_manager = _DummyConfigManager() + config_pkg.settings = settings_mod + sys.modules["config"] = config_pkg + sys.modules["config.settings"] = settings_mod + +from core.repair_jobs.unknown_artist_fixer import UnknownArtistFixerJob +import core.repair_jobs.unknown_artist_fixer as unknown_artist_fixer_module + + +class _FakeClient: + def __init__(self, track_details=None, search_results=None): + self.track_details = track_details or {} + self.search_results = search_results or {} + self.get_calls = [] + self.search_calls = [] + + def get_track_details(self, track_id): + self.get_calls.append(track_id) + return self.track_details.get(track_id) + + def search_tracks(self, query, limit=5): + self.search_calls.append((query, limit)) + return self.search_results.get(query, []) + + +def _install_tag_reader(monkeypatch, tags=None): + fake_module = types.ModuleType("core.tag_writer") + fake_module.read_file_tags = lambda path: tags or {} + monkeypatch.setitem(sys.modules, "core.tag_writer", fake_module) + + +def test_unknown_artist_fixer_uses_primary_source_track_id_first(monkeypatch): + job = UnknownArtistFixerJob() + _install_tag_reader(monkeypatch) + + deezer_client = _FakeClient( + track_details={ + "dz-1": { + "primary_artist": "Deezer Artist", + "album": { + "name": "Deezer Album", + "release_date": "2024-02-01", + "images": [{"url": "https://img/deezer"}], + }, + "name": "Deezer Song", + "track_number": 7, + "disc_number": 1, + } + } + ) + spotify_client = _FakeClient( + track_details={ + "sp-1": { + "primary_artist": "Spotify Artist", + "album": { + "name": "Spotify Album", + "release_date": "2023-01-01", + "images": [{"url": "https://img/spotify"}], + }, + "name": "Spotify Song", + "track_number": 1, + "disc_number": 1, + } + } + ) + + monkeypatch.setattr(unknown_artist_fixer_module, "get_primary_source", lambda: "deezer") + monkeypatch.setattr( + unknown_artist_fixer_module, + "get_client_for_source", + lambda source: {"deezer": deezer_client, "spotify": spotify_client}.get(source), + ) + + track = { + "title": "Unknown Title", + "album_title": "Unknown Album", + "spotify_track_id": "sp-1", + "deezer_track_id": "dz-1", + "itunes_track_id": "", + } + + result = job._resolve_metadata(SimpleNamespace(), track, "/tmp/track.flac") + + assert result["artist"] == "Deezer Artist" + assert result["album"] == "Deezer Album" + assert result["source"] == "deezer_track_id_lookup" + assert deezer_client.get_calls == ["dz-1"] + assert spotify_client.get_calls == [] + + +def test_unknown_artist_fixer_searches_primary_source_first(monkeypatch): + job = UnknownArtistFixerJob() + _install_tag_reader(monkeypatch) + + candidate = SimpleNamespace( + id="dz-song-1", + name="Matching Title", + album="Matching Album", + artists=["Deezer Artist"], + image_url="https://img/deezer-search", + ) + + deezer_client = _FakeClient( + track_details={ + "dz-song-1": { + "primary_artist": "Deezer Artist", + "album": { + "name": "Matching Album", + "release_date": "2024-03-02", + "images": [{"url": "https://img/deezer-full"}], + }, + "name": "Matching Title", + "track_number": 4, + "disc_number": 1, + } + }, + search_results={"Matching Title": [candidate]}, + ) + spotify_client = _FakeClient( + search_results={ + "Matching Title": [ + SimpleNamespace( + id="sp-song-1", + name="Matching Title", + album="Matching Album", + artists=["Spotify Artist"], + image_url="https://img/spotify-search", + ) + ] + } + ) + + monkeypatch.setattr(unknown_artist_fixer_module, "get_primary_source", lambda: "deezer") + monkeypatch.setattr( + unknown_artist_fixer_module, + "get_client_for_source", + lambda source: {"deezer": deezer_client, "spotify": spotify_client}.get(source), + ) + + track = { + "title": "Matching Title", + "album_title": "Matching Album", + "spotify_track_id": "", + "deezer_track_id": "", + "itunes_track_id": "", + } + + result = job._resolve_metadata(SimpleNamespace(sleep_or_stop=lambda seconds: False), track, "/tmp/track.flac") + + assert result["artist"] == "Deezer Artist" + assert result["album"] == "Matching Album" + assert result["source"] == "deezer_title_search" + assert deezer_client.search_calls == [("Matching Title", 5)] + assert spotify_client.search_calls == [] + + +def test_unknown_artist_fixer_supports_hydrabase_title_search(monkeypatch): + job = UnknownArtistFixerJob() + _install_tag_reader(monkeypatch) + + hydrabase_candidate = SimpleNamespace( + id="hy-song-1", + name="Hydra Match", + album="Hydra Album", + artists=["Hydra Artist"], + image_url="https://img/hydra-search", + ) + hydrabase_client = _FakeClient( + track_details={ + "hy-song-1": { + "primary_artist": "Hydra Artist", + "album": { + "name": "Hydra Album", + "release_date": "2024-04-03", + "images": [{"url": "https://img/hydra-full"}], + }, + "name": "Hydra Match", + "track_number": 2, + "disc_number": 1, + } + }, + search_results={"Hydra Match": [hydrabase_candidate]}, + ) + spotify_client = _FakeClient() + + monkeypatch.setattr(unknown_artist_fixer_module, "get_primary_source", lambda: "hydrabase") + monkeypatch.setattr( + unknown_artist_fixer_module, + "get_client_for_source", + lambda source: {"hydrabase": hydrabase_client, "spotify": spotify_client}.get(source), + ) + + track = { + "title": "Hydra Match", + "album_title": "Hydra Album", + "spotify_track_id": "", + "deezer_track_id": "", + "itunes_track_id": "", + } + + result = job._resolve_metadata(SimpleNamespace(sleep_or_stop=lambda seconds: False), track, "/tmp/track.flac") + + assert result["artist"] == "Hydra Artist" + assert result["source"] == "hydrabase_title_search" + assert hydrabase_client.search_calls == [("Hydra Match", 5)] + assert spotify_client.search_calls == [] From e7faa9f02f42a95c1169036e7b284080d4576486 Mon Sep 17 00:00:00 2001 From: Antti Kettunen Date: Wed, 15 Apr 2026 19:48:59 +0300 Subject: [PATCH 2/5] Use metadata source priority in track number repair job Use the shared metadata source priority when resolving album IDs, album searches, and tracklists in track number repair. Keeps Deezer and iTunes ahead of Spotify where configured, while still allowing the job to fall back through other supported sources. --- core/repair_jobs/track_number_repair.py | 232 +++++++++++++----------- tests/test_track_number_repair.py | 116 ++++++++++++ 2 files changed, 242 insertions(+), 106 deletions(-) create mode 100644 tests/test_track_number_repair.py diff --git a/core/repair_jobs/track_number_repair.py b/core/repair_jobs/track_number_repair.py index ac39b9d4..65080101 100644 --- a/core/repair_jobs/track_number_repair.py +++ b/core/repair_jobs/track_number_repair.py @@ -1,8 +1,9 @@ """Track Number Repair Job — fixes embedded track number tags and filename prefixes. Detects albums where 3+ files share the same track number (the "all tracks = 01" -bug pattern), then uses cascading API lookups (Spotify → iTunes → MusicBrainz → -AudioDB) to resolve the correct tracklist and repair each file. +bug pattern), then uses cascading API lookups in metadata-source priority order +before falling back to MusicBrainz and AudioDB to resolve the correct tracklist +and repair each file. """ import os @@ -10,6 +11,12 @@ import re from difflib import SequenceMatcher from typing import Any, Dict, List, Optional, Tuple +from core.metadata_service import ( + get_album_tracks_for_source, + get_client_for_source, + get_primary_source, + get_source_priority, +) from core.repair_jobs import register_job from core.repair_jobs.base import JobContext, JobResult, RepairJob from utils.logging_config import get_logger @@ -24,6 +31,14 @@ _PLACEHOLDER_IDS = { 'unknown', 'none', 'null', '', } +_SOURCE_ALBUM_ID_COLUMNS = ( + ('spotify', 'spotify_album_id'), + ('itunes', 'itunes_album_id'), + ('deezer', 'deezer_id'), + ('discogs', 'discogs_id'), + ('hydrabase', 'soul_id'), +) + @register_job class TrackNumberRepairJob(RepairJob): @@ -32,8 +47,9 @@ class TrackNumberRepairJob(RepairJob): description = 'Detects mismatched track numbers using API lookups (dry run by default)' help_text = ( 'Scans album folders and compares each file\'s track number against the correct ' - 'tracklist from Spotify or iTunes. If a file\'s embedded track number doesn\'t match ' - 'the API data, the job creates a finding showing what needs to change.\n\n' + 'tracklist from the configured metadata sources. If a file\'s embedded track ' + 'number doesn\'t match the API data, the job creates a finding showing what ' + 'needs to change.\n\n' 'In dry run mode (default), no files are modified — you review each proposed change ' 'in the Findings tab and decide what to approve. Disable dry run in settings to let ' 'the job automatically rename and re-number files.\n\n' @@ -187,7 +203,7 @@ class TrackNumberRepairJob(RepairJob): logger.info("Anomaly detected in %s — %d files share track number(s): %s", os.path.basename(folder_path), sum(duped.values()), duped) - # Resolve album tracklist via cascading fallbacks + # Resolve album tracklist via source-aware cascading fallbacks api_tracks = self._resolve_album_tracklist(file_track_data, folder_path, context, scan_state) if not api_tracks: result.skipped += len(filenames) @@ -253,38 +269,24 @@ class TrackNumberRepairJob(RepairJob): cache = scan_state['album_tracks_cache'] folder_name = os.path.basename(folder_path) + primary_source = get_primary_source() + source_priority = get_source_priority(primary_source) - # Fallback 0: Check DB first — if these files are tracked and their album - # has a spotify_album_id, use that directly without reading file tags - db_album_id, db_spotify_album_id, db_itunes_album_id = _lookup_album_ids_from_db( - file_track_data, context - ) - if db_spotify_album_id and _is_valid_album_id(db_spotify_album_id): - tracks = _get_album_tracklist(db_spotify_album_id, context, cache) - if tracks: - logger.info("[Repair] %s — resolved via DB spotify_album_id: %s", folder_name, db_spotify_album_id) - return tracks - if db_itunes_album_id and _is_valid_album_id(db_itunes_album_id): - tracks = _get_album_tracklist(db_itunes_album_id, context, cache) - if tracks: - logger.info("[Repair] %s — resolved via DB itunes_album_id: %s", folder_name, db_itunes_album_id) - return tracks + # Fallback 0: Check DB first. If any tracked file already has source IDs, + # prefer the configured source order and use the first available album ID. + source_album_ids = _lookup_album_ids_from_db(file_track_data, context) # Collect available IDs from file tags (fallback when DB has no IDs) - spotify_album_id = None - itunes_album_id = None spotify_track_id = None mb_album_id = None album_name = None artist_name = None for fpath, fname, _ in file_track_data: - if not spotify_album_id or not itunes_album_id: + if 'spotify' not in source_album_ids or 'itunes' not in source_album_ids: aid, source = _read_album_id_from_file(fpath) - if aid and source == 'spotify' and not spotify_album_id: - spotify_album_id = aid - elif aid and source == 'itunes' and not itunes_album_id: - itunes_album_id = aid + if aid and source in ('spotify', 'itunes') and source not in source_album_ids: + source_album_ids[source] = aid if not spotify_track_id: spotify_track_id = _read_spotify_track_id_from_file(fpath) @@ -295,31 +297,27 @@ class TrackNumberRepairJob(RepairJob): if not album_name: album_name, artist_name = _read_album_artist_from_file(fpath) - if spotify_album_id and itunes_album_id and spotify_track_id and mb_album_id and album_name: + if source_album_ids and spotify_track_id and mb_album_id and album_name: break - # Fallback 1: Spotify album ID from file tags - if spotify_album_id and _is_valid_album_id(spotify_album_id): - tracks = _get_album_tracklist(spotify_album_id, context, cache) - if tracks: - logger.info("[Repair] %s — resolved via Spotify album ID: %s", folder_name, spotify_album_id) - return tracks + # Fallback 1: Album IDs from DB / file tags, using source priority + for source in source_priority: + album_id = source_album_ids.get(source) + if album_id and _is_valid_album_id(album_id): + tracks = _get_album_tracklist(source, album_id, cache) + if tracks: + logger.info("[Repair] %s — resolved via %s album ID: %s", + folder_name, source, album_id) + return tracks - # Fallback 2: iTunes album ID - if itunes_album_id and _is_valid_album_id(itunes_album_id): - tracks = _get_album_tracklist(itunes_album_id, context, cache) - if tracks: - logger.info("[Repair] %s — resolved via iTunes album ID: %s", folder_name, itunes_album_id) - return tracks - - # Fallback 3: Spotify track ID → discover album ID - client = context.spotify_client - if spotify_track_id and client and client.is_spotify_authenticated() and not context.is_spotify_rate_limited(): + # Fallback 2: Spotify track ID → discover album ID + client = get_client_for_source('spotify') + if spotify_track_id and client: try: track_details = client.get_track_details(spotify_track_id) if track_details and track_details.get('album', {}).get('id'): real_album_id = track_details['album']['id'] - tracks = _get_album_tracklist(real_album_id, context, cache) + tracks = _get_album_tracklist('spotify', real_album_id, cache) if tracks: logger.info("[Repair] %s — resolved via Spotify track ID %s → album %s", folder_name, spotify_track_id, real_album_id) @@ -327,29 +325,35 @@ class TrackNumberRepairJob(RepairJob): except Exception as e: logger.debug("Spotify track lookup failed for %s: %s", spotify_track_id, e) - # Fallback 4: Search Spotify/iTunes by album name + artist - if album_name and client and not context.is_spotify_rate_limited(): - try: - query = f"{artist_name} {album_name}" if artist_name else album_name - results = client.search_albums(query, limit=5) - if results: - best = results[0] - tracks = _get_album_tracklist(best.id, context, cache) - if tracks: - logger.info("[Repair] %s — resolved via album search: '%s' → %s", - folder_name, query, best.id) - return tracks - except Exception as e: - logger.debug("Album search failed for '%s': %s", album_name, e) + # Fallback 3: Search metadata sources by album name + artist + if album_name: + query = f"{artist_name} {album_name}" if artist_name else album_name + for source in source_priority: + client = get_client_for_source(source) + if not client or not hasattr(client, 'search_albums'): + continue + try: + results = client.search_albums(query, limit=5) + if results: + best = results[0] + best_album_id = getattr(best, 'id', None) if not isinstance(best, dict) else best.get('id') + if best_album_id: + tracks = _get_album_tracklist(source, str(best_album_id), cache) + if tracks: + logger.info("[Repair] %s — resolved via %s album search: '%s' → %s", + folder_name, source, query, best_album_id) + return tracks + except Exception as e: + logger.debug("%s album search failed for '%s': %s", source.capitalize(), album_name, e) - # Fallback 5: MusicBrainz album ID from tags + # Fallback 4: MusicBrainz album ID from tags if mb_album_id: tracks = _get_tracklist_from_musicbrainz(mb_album_id, context, cache) if tracks: logger.info("[Repair] %s — resolved via MusicBrainz album ID: %s", folder_name, mb_album_id) return tracks - # Fallback 6: AudioDB → MusicBrainz + # Fallback 5: AudioDB → MusicBrainz if album_name and artist_name: adb_mb_id = _get_musicbrainz_id_via_audiodb(artist_name, album_name, context) if adb_mb_id and adb_mb_id != mb_album_id: @@ -800,25 +804,36 @@ def _update_db_file_path(db, old_path: str, new_path: str): def _lookup_album_ids_from_db(file_track_data: List[Tuple[str, str, Any]], - context: JobContext) -> Tuple[Optional[str], Optional[str], Optional[str]]: + context: JobContext) -> Dict[str, Optional[str]]: """Look up album IDs from the database using file paths. Checks if any of the files in this folder are tracked in the DB, and if so, - returns the album's (album_id, spotify_album_id, itunes_album_id). + returns a mapping of metadata source -> album ID. This avoids expensive file tag reads and API calls when the DB already knows. """ if not context.db: - return None, None, None + return {} conn = None try: conn = context.db._get_connection() cursor = conn.cursor() + cursor.execute("PRAGMA table_info(albums)") + album_columns = {row[1] for row in cursor.fetchall()} + + selected_sources = [ + (source, column) + for source, column in _SOURCE_ALBUM_ID_COLUMNS + if column in album_columns + ] + if not selected_sources: + return {} # Try each file path until we find one tracked in the DB for fpath, _, _ in file_track_data: - cursor.execute(""" - SELECT t.album_id, al.spotify_album_id, al.itunes_album_id + select_cols = ", ".join(f"al.{column}" for _source, column in selected_sources) + cursor.execute(f""" + SELECT {select_cols} FROM tracks t JOIN albums al ON al.id = t.album_id WHERE t.file_path = ? @@ -826,7 +841,11 @@ def _lookup_album_ids_from_db(file_track_data: List[Tuple[str, str, Any]], """, (fpath,)) row = cursor.fetchone() if row: - return row[0], row[1], row[2] + return { + source: str(row[idx]) + for idx, (source, _column) in enumerate(selected_sources) + if row[idx] + } except Exception as e: logger.debug("Error looking up album IDs from DB: %s", e) @@ -834,7 +853,7 @@ def _lookup_album_ids_from_db(file_track_data: List[Tuple[str, str, Any]], if conn: conn.close() - return None, None, None + return {} def _lookup_album_artist_art(file_track_data: List[Tuple[str, str, Any]], @@ -1018,52 +1037,53 @@ def _repair_single_track(file_path: str, filename: str, api_tracks: List[Dict], return True -def _get_album_tracklist(album_id: str, context: JobContext, - cache: dict) -> Optional[List[Dict]]: - """Fetch an album tracklist from Spotify or iTunes, with per-scan caching. +def _normalize_album_track_items(data) -> List[Dict[str, Any]]: + """Normalize album track payloads to a list of dicts.""" + if isinstance(data, list): + return data + if not isinstance(data, dict): + return [] + items = data.get('items') + if isinstance(items, list): + return items + tracks = data.get('tracks') + if isinstance(tracks, list): + return tracks + if isinstance(tracks, dict): + nested_items = tracks.get('items') + if isinstance(nested_items, list): + return nested_items + return [] + + +def _get_album_tracklist(source: str, album_id: str, cache: dict) -> Optional[List[Dict]]: + """Fetch an album tracklist from a specific source, with per-scan caching. Returns a list of dicts with at least 'name' and 'track_number' keys, or None if lookup fails. """ - if album_id in cache: - return cache[album_id] + cache_key = f"{source}:{album_id}" + if cache_key in cache: + return cache[cache_key] result = None - # Try Spotify first - client = context.spotify_client - if client and client.is_spotify_authenticated() and not context.is_spotify_rate_limited(): - try: - data = client.get_album_tracks(album_id) - if data and 'items' in data and data['items']: - result = [ - { - 'name': item.get('name', ''), - 'track_number': item.get('track_number'), - 'disc_number': item.get('disc_number', 1), - } - for item in data['items'] - ] - except Exception as e: - logger.debug("Spotify get_album_tracks failed for %s: %s", album_id, e) + try: + data = get_album_tracks_for_source(source, album_id) + items = _normalize_album_track_items(data) + if items: + result = [ + { + 'name': item.get('name', '') if isinstance(item, dict) else getattr(item, 'name', ''), + 'track_number': item.get('track_number') if isinstance(item, dict) else getattr(item, 'track_number', None), + 'disc_number': item.get('disc_number', 1) if isinstance(item, dict) else getattr(item, 'disc_number', 1), + } + for item in items + ] + except Exception as e: + logger.debug("%s get_album_tracks failed for %s: %s", source.capitalize(), album_id, e) - # Fallback to iTunes - if not result and context.itunes_client: - try: - data = context.itunes_client.get_album_tracks(album_id) - if data and 'items' in data and data['items']: - result = [ - { - 'name': item.get('name', ''), - 'track_number': item.get('track_number'), - 'disc_number': item.get('disc_number', 1), - } - for item in data['items'] - ] - except Exception as e: - logger.debug("iTunes get_album_tracks failed for %s: %s", album_id, e) - - cache[album_id] = result + cache[cache_key] = result return result diff --git a/tests/test_track_number_repair.py b/tests/test_track_number_repair.py new file mode 100644 index 00000000..7b5cfb77 --- /dev/null +++ b/tests/test_track_number_repair.py @@ -0,0 +1,116 @@ +import sys +import types +from types import SimpleNamespace + +# Stub optional Spotify dependency so metadata_service can import in tests. +if 'spotipy' not in sys.modules: + spotipy = types.ModuleType('spotipy') + oauth2 = types.ModuleType('spotipy.oauth2') + + class _DummySpotify: + pass + + class _DummyOAuth: + pass + + spotipy.Spotify = _DummySpotify + oauth2.SpotifyOAuth = _DummyOAuth + oauth2.SpotifyClientCredentials = _DummyOAuth + spotipy.oauth2 = oauth2 + sys.modules['spotipy'] = spotipy + sys.modules['spotipy.oauth2'] = oauth2 + +if 'config.settings' not in sys.modules: + config_mod = types.ModuleType('config') + settings_mod = types.ModuleType('config.settings') + + class _DummyConfigManager: + def get(self, key, default=None): + return default + + settings_mod.config_manager = _DummyConfigManager() + config_mod.settings = settings_mod + sys.modules['config'] = config_mod + sys.modules['config.settings'] = settings_mod + +from core.repair_jobs import track_number_repair as tnr + + +class _FakeSearchClient: + def __init__(self, search_results=None): + self.search_results = search_results or [] + self.search_calls = [] + + def search_albums(self, query, limit=10): + self.search_calls.append((query, limit)) + return self.search_results + + +def _make_context(): + return SimpleNamespace(db=None, mb_client=None) + + +def test_resolve_album_tracklist_prefers_primary_source_album_id(monkeypatch): + context = _make_context() + file_track_data = [('/music/test.flac', '01 - Track.flac', None)] + cache = {'album_tracks_cache': {}, 'title_similarity': 0.8} + calls = [] + + monkeypatch.setattr(tnr, 'get_primary_source', lambda: 'deezer') + monkeypatch.setattr( + tnr, + '_lookup_album_ids_from_db', + lambda *_args, **_kwargs: {'spotify': 'sp-album', 'itunes': 'it-album', 'deezer': 'dz-album'}, + ) + monkeypatch.setattr(tnr, '_read_album_id_from_file', lambda *_args, **_kwargs: (None, None)) + monkeypatch.setattr(tnr, '_read_spotify_track_id_from_file', lambda *_args, **_kwargs: None) + monkeypatch.setattr(tnr, '_read_musicbrainz_album_id_from_file', lambda *_args, **_kwargs: None) + monkeypatch.setattr(tnr, '_read_album_artist_from_file', lambda *_args, **_kwargs: (None, None)) + + def fake_get_album_tracks_for_source(source, album_id): + calls.append((source, album_id)) + return {'items': [{'name': f'{source} track', 'track_number': 1, 'disc_number': 1}]} + + monkeypatch.setattr(tnr, 'get_album_tracks_for_source', fake_get_album_tracks_for_source) + + result = tnr.TrackNumberRepairJob()._resolve_album_tracklist(file_track_data, '/music', context, cache) + + assert result is not None + assert result[0]['name'] == 'deezer track' + assert calls == [('deezer', 'dz-album')] + + +def test_resolve_album_tracklist_uses_source_priority_for_search(monkeypatch): + context = _make_context() + file_track_data = [('/music/test.flac', 'Track.flac', None)] + cache = {'album_tracks_cache': {}, 'title_similarity': 0.8} + track_calls = [] + + discogs_client = _FakeSearchClient([SimpleNamespace(id='dg-album')]) + spotify_client = _FakeSearchClient([SimpleNamespace(id='sp-album')]) + + monkeypatch.setattr(tnr, 'get_primary_source', lambda: 'discogs') + monkeypatch.setattr(tnr, '_lookup_album_ids_from_db', lambda *_args, **_kwargs: {}) + monkeypatch.setattr(tnr, '_read_album_id_from_file', lambda *_args, **_kwargs: (None, None)) + monkeypatch.setattr(tnr, '_read_spotify_track_id_from_file', lambda *_args, **_kwargs: None) + monkeypatch.setattr(tnr, '_read_musicbrainz_album_id_from_file', lambda *_args, **_kwargs: None) + monkeypatch.setattr(tnr, '_read_album_artist_from_file', lambda *_args, **_kwargs: ('Album Title', 'Artist Name')) + monkeypatch.setattr( + tnr, + 'get_client_for_source', + lambda source: {'discogs': discogs_client, 'spotify': spotify_client}.get(source), + ) + + def fake_get_album_tracks_for_source(source, album_id): + track_calls.append((source, album_id)) + return {'items': [{'name': f'{source} track', 'track_number': 1, 'disc_number': 1}]} + + monkeypatch.setattr(tnr, 'get_album_tracks_for_source', fake_get_album_tracks_for_source) + + result = tnr.TrackNumberRepairJob()._resolve_album_tracklist(file_track_data, '/music', context, cache) + + assert result is not None + assert result[0]['name'] == 'discogs track' + assert discogs_client.search_calls == [('Artist Name Album Title', 5)] + assert spotify_client.search_calls == [] + assert track_calls == [('discogs', 'dg-album')] From 6df6ecb560d2ffd6db39c2fa7c3e4157ee772ef7 Mon Sep 17 00:00:00 2001 From: Antti Kettunen Date: Wed, 15 Apr 2026 20:09:01 +0300 Subject: [PATCH 3/5] Respect source preference in cover art repair job Cover art lookup now honors an explicit prefer_source first, falls back to the runtime primary metadata source when unset, and uses the shared source priority for the remaining fallbacks. --- core/repair_jobs/missing_cover_art.py | 164 +++++++++++++++---------- tests/test_missing_cover_art.py | 167 ++++++++++++++++++++++++++ 2 files changed, 268 insertions(+), 63 deletions(-) create mode 100644 tests/test_missing_cover_art.py diff --git a/core/repair_jobs/missing_cover_art.py b/core/repair_jobs/missing_cover_art.py index 2ed9bae6..1e406ee3 100644 --- a/core/repair_jobs/missing_cover_art.py +++ b/core/repair_jobs/missing_cover_art.py @@ -1,5 +1,6 @@ """Missing Cover Art Filler Job — finds albums without artwork and locates art from APIs.""" +from core.metadata_service import get_client_for_source, get_primary_source, get_source_priority from core.repair_jobs import register_job from core.repair_jobs.base import JobContext, JobResult, RepairJob from utils.logging_config import get_logger @@ -11,29 +12,36 @@ logger = get_logger("repair_job.cover_art") class MissingCoverArtJob(RepairJob): job_id = 'missing_cover_art' display_name = 'Cover Art Filler' - description = 'Finds albums missing artwork and locates art from Spotify/iTunes' + description = 'Finds albums missing artwork and locates art from metadata sources' help_text = ( 'Scans your library for albums that have no cover art stored in the database. ' - 'For each missing cover, it searches Spotify and iTunes APIs using the album name ' - 'and artist to find matching artwork.\n\n' + 'For each missing cover, it searches the configured metadata sources using the ' + 'album name and artist to find matching artwork. If Prefer Source is set, that ' + 'source is tried first; otherwise the primary metadata source is used.\n\n' 'When artwork is found, a finding is created with the image URL so you can review ' 'and apply it. The job does not download or embed artwork automatically.\n\n' 'Settings:\n' - '- Prefer Source: Which API to try first for artwork (spotify or itunes)' + '- Prefer Source: Optional source to try first; otherwise the primary metadata source is used' ) icon = 'repair-icon-coverart' default_enabled = True default_interval_hours = 48 - default_settings = { - 'prefer_source': 'spotify', - } + default_settings = {} auto_fix = False def scan(self, context: JobContext) -> JobResult: result = JobResult() settings = self._get_settings(context) - prefer_source = settings.get('prefer_source', 'spotify') + primary_source = get_primary_source() + source_priority = get_source_priority(primary_source) + prefer_source = settings.get('prefer_source') + if prefer_source and prefer_source != primary_source and prefer_source in source_priority: + source_priority.remove(prefer_source) + source_priority.insert(0, prefer_source) + if primary_source in source_priority: + source_priority.remove(primary_source) + source_priority.insert(1, primary_source) # Fetch albums with missing artwork albums = [] @@ -41,9 +49,31 @@ class MissingCoverArtJob(RepairJob): try: conn = context.db._get_connection() cursor = conn.cursor() - cursor.execute(""" - SELECT al.id, al.title, ar.name, al.spotify_album_id, al.thumb_url, - ar.thumb_url + cursor.execute("PRAGMA table_info(albums)") + album_columns = {column[1] for column in cursor.fetchall()} + + select_cols = [ + "al.id", + "al.title", + "ar.name", + "al.spotify_album_id", + "al.thumb_url", + "ar.thumb_url", + ] + column_map = [ + ("itunes_album_id", "al.itunes_album_id"), + ("deezer_album_id", "al.deezer_id"), + ("discogs_album_id", "al.discogs_id"), + ("hydrabase_album_id", "al.soul_id"), + ] + column_index = {} + for alias, column in column_map: + if column.split('.', 1)[1] in album_columns: + column_index[alias] = len(select_cols) + select_cols.append(f"{column} AS {alias}") + + cursor.execute(f""" + SELECT {', '.join(select_cols)} FROM albums al LEFT JOIN artists ar ON ar.id = al.artist_id WHERE (al.thumb_url IS NULL OR al.thumb_url = '') @@ -67,15 +97,20 @@ class MissingCoverArtJob(RepairJob): if context.report_progress: context.report_progress(phase=f'Searching artwork for {total} albums...', total=total) - spotify_skipped = False # Track if we've logged the rate limit skip - for i, row in enumerate(albums): if context.check_stop(): return result if i % 10 == 0 and context.wait_if_paused(): return result - album_id, title, artist_name, spotify_album_id, _, artist_thumb = row + album_id, title, artist_name, spotify_album_id, _, artist_thumb = row[:6] + source_album_ids = { + 'spotify': spotify_album_id, + 'itunes': row[column_index['itunes_album_id']] if 'itunes_album_id' in column_index else None, + 'deezer': row[column_index['deezer_album_id']] if 'deezer_album_id' in column_index else None, + 'discogs': row[column_index['discogs_album_id']] if 'discogs_album_id' in column_index else None, + 'hydrabase': row[column_index['hydrabase_album_id']] if 'hydrabase_album_id' in column_index else None, + } result.scanned += 1 if context.report_progress: @@ -88,24 +123,11 @@ class MissingCoverArtJob(RepairJob): artwork_url = None - # Check Spotify rate limit at the top level — skip Spotify entirely if banned - spotify_available = not context.is_spotify_rate_limited() - if not spotify_available and not spotify_skipped: - logger.info("Spotify rate limited — skipping Spotify artwork lookups, using fallback only") - spotify_skipped = True - - # Try to find artwork URL from APIs - if prefer_source == 'spotify' and spotify_available: - artwork_url = self._try_spotify(spotify_album_id, title, artist_name, context) - if not artwork_url: - artwork_url = self._try_itunes(title, artist_name, context) - elif prefer_source == 'spotify' and not spotify_available: - # Spotify preferred but rate limited — use fallback only - artwork_url = self._try_itunes(title, artist_name, context) - else: - artwork_url = self._try_itunes(title, artist_name, context) - if not artwork_url and spotify_available: - artwork_url = self._try_spotify(spotify_album_id, title, artist_name, context) + # Try source-specific IDs first, then title/artist search, in priority order. + for source in source_priority: + artwork_url = self._try_source(source, source_album_ids.get(source), title, artist_name) + if artwork_url: + break if artwork_url: if context.report_progress: @@ -151,47 +173,63 @@ class MissingCoverArtJob(RepairJob): result.scanned, result.findings_created, result.skipped) return result - def _try_spotify(self, spotify_album_id, title, artist_name, context): - """Try to get album art from Spotify.""" - client = context.spotify_client + def _try_source(self, source, source_album_id, title, artist_name): + """Try to get album art from a specific metadata source.""" + client = get_client_for_source(source) if not client: return None + query = f"{artist_name} {title}" if artist_name else title + try: - if context.is_spotify_rate_limited(): - return None + if source_album_id: + album_data = self._get_album_for_source(source, client, source_album_id) + artwork_url = self._extract_artwork_url(album_data) + if artwork_url: + return artwork_url - # If we have a Spotify album ID, fetch directly - if spotify_album_id and client.is_spotify_authenticated(): - album_data = client.get_album(spotify_album_id) - if album_data: - images = album_data.get('images', []) - if images: - return images[0].get('url') - - # Search by name - if title and client.is_spotify_authenticated(): - query = f"{artist_name} {title}" if artist_name else title + if query and hasattr(client, 'search_albums'): results = client.search_albums(query, limit=1) - if results and hasattr(results[0], 'image_url') and results[0].image_url: - return results[0].image_url + if results: + artwork_url = self._extract_artwork_url(results[0]) + if artwork_url: + return artwork_url + candidate_id = self._extract_album_id(results[0]) + if candidate_id: + album_data = self._get_album_for_source(source, client, candidate_id) + return self._extract_artwork_url(album_data) except Exception as e: - logger.debug("Spotify art lookup failed for '%s': %s", title, e) + logger.debug("%s art lookup failed for '%s': %s", source.capitalize(), title, e) return None - def _try_itunes(self, title, artist_name, context): - """Try to get album art from iTunes.""" - client = context.itunes_client - if not client: - return None + @staticmethod + def _get_album_for_source(source, client, album_id): + if source == 'spotify': + return client.get_album(album_id) + return client.get_album(album_id, include_tracks=False) - try: - query = f"{artist_name} {title}" if artist_name else title - results = client.search_albums(query, limit=1) - if results and hasattr(results[0], 'image_url') and results[0].image_url: - return results[0].image_url - except Exception as e: - logger.debug("iTunes art lookup failed for '%s': %s", title, e) + @staticmethod + def _extract_album_id(item): + if hasattr(item, 'id'): + return getattr(item, 'id', None) + if isinstance(item, dict): + return item.get('id') + return None + + @staticmethod + def _extract_artwork_url(item): + if not item: + return None + if hasattr(item, 'image_url') and getattr(item, 'image_url', None): + return item.image_url + if isinstance(item, dict): + if item.get('image_url'): + return item['image_url'] + images = item.get('images') or [] + if images and isinstance(images, list): + first = images[0] + if isinstance(first, dict): + return first.get('url') return None def _get_settings(self, context: JobContext) -> dict: diff --git a/tests/test_missing_cover_art.py b/tests/test_missing_cover_art.py new file mode 100644 index 00000000..da6f5463 --- /dev/null +++ b/tests/test_missing_cover_art.py @@ -0,0 +1,167 @@ +import sqlite3 +import sys +import types +from types import SimpleNamespace + +# Stub optional Spotify dependency so metadata_service can import in tests. +if 'spotipy' not in sys.modules: + spotipy = types.ModuleType('spotipy') + oauth2 = types.ModuleType('spotipy.oauth2') + + class _DummySpotify: + pass + + class _DummyOAuth: + pass + + spotipy.Spotify = _DummySpotify + oauth2.SpotifyOAuth = _DummyOAuth + oauth2.SpotifyClientCredentials = _DummyOAuth + spotipy.oauth2 = oauth2 + sys.modules['spotipy'] = spotipy + sys.modules['spotipy.oauth2'] = oauth2 + +if 'config.settings' not in sys.modules: + config_mod = types.ModuleType('config') + settings_mod = types.ModuleType('config.settings') + + class _DummyConfigManager: + def get(self, key, default=None): + return default + + settings_mod.config_manager = _DummyConfigManager() + config_mod.settings = settings_mod + sys.modules['config'] = config_mod + sys.modules['config.settings'] = settings_mod + +from core.repair_jobs import missing_cover_art as mca + + +class _FakeClient: + def __init__(self, album_image=None, search_image=None): + self.album_image = album_image + self.search_image = search_image + self.get_album_calls = [] + self.search_calls = [] + + def get_album(self, album_id, include_tracks=False): + self.get_album_calls.append((album_id, include_tracks)) + if self.album_image is None: + return None + if isinstance(self.album_image, str): + return {'images': [{'url': self.album_image}]} + return self.album_image + + def search_albums(self, query, limit=1): + self.search_calls.append((query, limit)) + if self.search_image is None: + return [] + return [SimpleNamespace(id='search-album', image_url=self.search_image)] + + +def _make_db(album_row): + conn = sqlite3.connect(':memory:') + conn.row_factory = sqlite3.Row + cursor = conn.cursor() + cursor.execute( + """ + CREATE TABLE artists ( + id INTEGER PRIMARY KEY, + name TEXT, + thumb_url TEXT + ) + """ + ) + cursor.execute( + """ + CREATE TABLE albums ( + id INTEGER PRIMARY KEY, + title TEXT, + artist_id INTEGER, + thumb_url TEXT, + spotify_album_id TEXT, + itunes_album_id TEXT, + deezer_id TEXT, + discogs_id TEXT, + soul_id TEXT + ) + """ + ) + cursor.execute( + "INSERT INTO artists (id, name, thumb_url) VALUES (?, ?, ?)", + (1, 'Artist', 'https://artist/thumb'), + ) + cursor.execute( + """ + INSERT INTO albums + (id, title, artist_id, thumb_url, spotify_album_id, itunes_album_id, deezer_id, discogs_id, soul_id) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?) + """, + album_row, + ) + conn.commit() + return conn + + +def _make_context(conn, prefer_source=None): + job_settings = {} + if prefer_source is not None: + job_settings['prefer_source'] = prefer_source + settings = {'repair.jobs.missing_cover_art.settings': job_settings} + findings = [] + return SimpleNamespace( + db=SimpleNamespace(_get_connection=lambda: conn), + config_manager=SimpleNamespace(get=lambda key, default=None: settings.get(key, default)), + check_stop=lambda: False, + wait_if_paused=lambda: False, + update_progress=lambda *args, **kwargs: None, + report_progress=lambda *args, **kwargs: None, + create_finding=lambda **kwargs: findings.append(kwargs), + findings=findings, + ) + + +def test_missing_cover_art_prefers_explicit_source_over_primary(monkeypatch): + conn = _make_db((1, 'Album', 1, '', 'sp-album', 'it-album', 'dz-album', 'dg-album', 'hy-album')) + context = _make_context(conn, prefer_source='spotify') + + deezer_client = _FakeClient(album_image='https://img/deezer-direct') + spotify_client = _FakeClient(album_image='https://img/spotify-direct') + + monkeypatch.setattr(mca, 'get_primary_source', lambda: 'deezer') + monkeypatch.setattr( + mca, + 'get_client_for_source', + lambda source: {'deezer': deezer_client, 'spotify': spotify_client}.get(source), + ) + + result = mca.MissingCoverArtJob().scan(context) + + assert result.findings_created == 1 + assert spotify_client.get_album_calls == [('sp-album', False)] + assert deezer_client.get_album_calls == [] + assert context.findings[0]['details']['found_artwork_url'] == 'https://img/spotify-direct' + + +def test_missing_cover_art_uses_primary_when_prefer_unset(monkeypatch): + conn = _make_db((1, 'Album', 1, '', None, None, None, None, None)) + context = _make_context(conn) + + discogs_client = _FakeClient(search_image='https://img/discogs-search') + spotify_client = _FakeClient(search_image='https://img/spotify-search') + itunes_client = _FakeClient(search_image='https://img/itunes-search') + + monkeypatch.setattr(mca, 'get_primary_source', lambda: 'discogs') + monkeypatch.setattr( + mca, + 'get_client_for_source', + lambda source: {'discogs': discogs_client, 'spotify': spotify_client, 'itunes': itunes_client}.get(source), + ) + + result = mca.MissingCoverArtJob().scan(context) + + assert result.findings_created == 1 + assert discogs_client.search_calls == [('Artist Album', 1)] + assert spotify_client.search_calls == [] + assert itunes_client.search_calls == [] + assert context.findings[0]['details']['found_artwork_url'] == 'https://img/discogs-search' From 03711c10df79d22916d842843c3ed9525dcd9293 Mon Sep 17 00:00:00 2001 From: Antti Kettunen Date: Wed, 15 Apr 2026 20:28:09 +0300 Subject: [PATCH 4/5] Make metadata gap filler source-aware Only fetch source track details when ISRC enrichment is enabled, and show resolved source/track provenance in the UI. --- core/repair_jobs/metadata_gap_filler.py | 132 +++++++++++++---- tests/test_metadata_gap_filler.py | 179 ++++++++++++++++++++++++ webui/static/script.js | 2 + 3 files changed, 287 insertions(+), 26 deletions(-) create mode 100644 tests/test_metadata_gap_filler.py diff --git a/core/repair_jobs/metadata_gap_filler.py b/core/repair_jobs/metadata_gap_filler.py index f709b857..ead800b1 100644 --- a/core/repair_jobs/metadata_gap_filler.py +++ b/core/repair_jobs/metadata_gap_filler.py @@ -1,8 +1,6 @@ """Metadata Gap Filler Job — finds tracks missing key metadata and locates it from APIs.""" -import time - -from core.metadata_service import get_primary_source +from core.metadata_service import get_client_for_source, get_primary_source, get_source_priority from core.repair_jobs import register_job from core.repair_jobs.base import JobContext, JobResult, RepairJob from utils.logging_config import get_logger @@ -40,6 +38,7 @@ class MetadataGapFillerJob(RepairJob): settings = self._get_settings(context) fill_isrc = settings.get('fill_isrc', True) fill_mb_id = settings.get('fill_musicbrainz_id', True) + source_priority = get_source_priority(get_primary_source()) # Build WHERE clauses for missing fields (only columns that exist on tracks) conditions = [] @@ -53,27 +52,47 @@ class MetadataGapFillerJob(RepairJob): where = " OR ".join(conditions) - # Fetch tracks with gaps, prioritizing those with spotify_track_id + # Fetch tracks with gaps, prioritizing those with source track IDs. tracks = [] conn = None try: conn = context.db._get_connection() cursor = conn.cursor() + cursor.execute("PRAGMA table_info(tracks)") + track_columns = {column[1] for column in cursor.fetchall()} + + select_cols = [ + "t.id", + "t.title", + "ar.name", + "al.title", + "t.isrc", + "t.musicbrainz_recording_id", + "al.thumb_url", + "ar.thumb_url", + ] + column_map = [ + ("spotify_track_id", "t.spotify_track_id"), + ("itunes_track_id", "t.itunes_track_id"), + ("deezer_track_id", "t.deezer_track_id"), + ] + column_index = {} + for alias, column in column_map: + if column.split('.', 1)[1] in track_columns: + column_index[alias] = len(select_cols) + select_cols.append(f"{column} AS {alias}") + cursor.execute(f""" - SELECT t.id, t.title, ar.name, al.title, t.spotify_track_id, - t.isrc, t.musicbrainz_recording_id, - al.thumb_url, ar.thumb_url + SELECT {', '.join(select_cols)} FROM tracks t LEFT JOIN artists ar ON ar.id = t.artist_id LEFT JOIN albums al ON al.id = t.album_id WHERE t.title IS NOT NULL AND t.title != '' AND ({where}) - ORDER BY - CASE WHEN t.spotify_track_id IS NOT NULL AND t.spotify_track_id != '' THEN 0 ELSE 1 END, - t.id LIMIT 500 """) tracks = cursor.fetchall() + tracks = sorted(tracks, key=lambda row: _track_row_priority(row, column_index, source_priority)) except Exception as e: logger.error("Error fetching tracks with metadata gaps: %s", e, exc_info=True) result.errors += 1 @@ -97,7 +116,12 @@ class MetadataGapFillerJob(RepairJob): if i % 20 == 0 and context.wait_if_paused(): return result - track_id, title, artist_name, album_title, spotify_track_id, isrc, mb_id, album_thumb, artist_thumb = row + track_id, title, artist_name, album_title, isrc, mb_id, album_thumb, artist_thumb = row[:8] + source_track_ids = { + 'spotify': row[column_index['spotify_track_id']] if 'spotify_track_id' in column_index else None, + 'itunes': row[column_index['itunes_track_id']] if 'itunes_track_id' in column_index else None, + 'deezer': row[column_index['deezer_track_id']] if 'deezer_track_id' in column_index else None, + } result.scanned += 1 if context.report_progress: @@ -108,20 +132,29 @@ class MetadataGapFillerJob(RepairJob): log_type='info' ) found_fields = {} + resolved_source = None + resolved_track_id = None - # Try Spotify enrichment for ISRC — only when Spotify is the configured primary source. - # If Deezer/iTunes is primary, Spotify may still be authenticated for playlist sync - # but should not be called here, as it burns API quota unnecessarily. - if spotify_track_id and context.spotify_client and not context.is_spotify_rate_limited() and get_primary_source() == 'spotify': - try: - track_data = context.spotify_client.get_track_details(spotify_track_id) - if track_data: - if fill_isrc and not isrc: - ext_ids = track_data.get('external_ids', {}) - if ext_ids.get('isrc'): - found_fields['isrc'] = ext_ids['isrc'] - except Exception as e: - logger.debug("Spotify enrichment failed for track %s: %s", track_id, e) + # Try source-aware track detail lookups only when ISRC enrichment is enabled. + if fill_isrc and not isrc: + for source in source_priority: + track_source_id = source_track_ids.get(source) + if not track_source_id: + continue + try: + client = get_client_for_source(source) + if not client or not hasattr(client, 'get_track_details'): + continue + track_data = client.get_track_details(track_source_id) + if track_data: + isrc_value = _extract_isrc(track_data) + if isrc_value: + found_fields['isrc'] = isrc_value + resolved_source = source + resolved_track_id = track_source_id + break + except Exception as e: + logger.debug("%s enrichment failed for track %s: %s", source.capitalize(), track_id, e) # Try MusicBrainz for MB recording ID if fill_mb_id and not mb_id and context.mb_client: @@ -161,7 +194,9 @@ class MetadataGapFillerJob(RepairJob): 'title': title, 'artist': artist_name, 'album': album_title, - 'spotify_track_id': spotify_track_id, + 'track_ids': source_track_ids, + 'resolved_source': resolved_source, + 'resolved_track_id': resolved_track_id, 'found_fields': found_fields, 'album_thumb_url': album_thumb or None, 'artist_thumb_url': artist_thumb or None, @@ -175,7 +210,7 @@ class MetadataGapFillerJob(RepairJob): result.skipped += 1 # Rate limit API calls - if spotify_track_id: + if fill_isrc and any(source_track_ids.values()): if context.sleep_or_stop(0.5): return result @@ -215,3 +250,48 @@ class MetadataGapFillerJob(RepairJob): finally: if conn: conn.close() + + +def _extract_isrc(track_data): + """Extract ISRC from a track detail payload.""" + if not track_data or not isinstance(track_data, dict): + return None + + external_ids = track_data.get('external_ids') + if isinstance(external_ids, dict): + isrc = external_ids.get('isrc') + if isrc: + return isrc + + isrc = track_data.get('isrc') + if isrc: + return isrc + + raw_data = track_data.get('raw_data') + if isinstance(raw_data, dict): + external_ids = raw_data.get('external_ids') + if isinstance(external_ids, dict) and external_ids.get('isrc'): + return external_ids['isrc'] + if raw_data.get('isrc'): + return raw_data['isrc'] + + return None + + +def _track_row_priority(row, column_index, source_priority): + """Sort rows by the first source track ID available in priority order.""" + source_columns = { + 'spotify': 'spotify_track_id', + 'itunes': 'itunes_track_id', + 'deezer': 'deezer_track_id', + } + + for idx, source in enumerate(source_priority): + column_name = source_columns.get(source) + if not column_name: + continue + column_pos = column_index.get(column_name) + if column_pos is not None and row[column_pos]: + return idx + + return len(source_priority) diff --git a/tests/test_metadata_gap_filler.py b/tests/test_metadata_gap_filler.py new file mode 100644 index 00000000..ba2ab44f --- /dev/null +++ b/tests/test_metadata_gap_filler.py @@ -0,0 +1,179 @@ +import sqlite3 +import sys +import types +from types import SimpleNamespace + +# Stub optional Spotify dependency so metadata_service can import in tests. +if 'spotipy' not in sys.modules: + spotipy = types.ModuleType('spotipy') + oauth2 = types.ModuleType('spotipy.oauth2') + + class _DummySpotify: + pass + + class _DummyOAuth: + pass + + spotipy.Spotify = _DummySpotify + oauth2.SpotifyOAuth = _DummyOAuth + oauth2.SpotifyClientCredentials = _DummyOAuth + spotipy.oauth2 = oauth2 + sys.modules['spotipy'] = spotipy + sys.modules['spotipy.oauth2'] = oauth2 + +if 'config.settings' not in sys.modules: + config_mod = types.ModuleType('config') + settings_mod = types.ModuleType('config.settings') + + class _DummyConfigManager: + def get(self, key, default=None): + return default + + settings_mod.config_manager = _DummyConfigManager() + config_mod.settings = settings_mod + sys.modules['config'] = config_mod + sys.modules['config.settings'] = settings_mod + +from core.repair_jobs import metadata_gap_filler as mgf + + +class _FakeTrackClient: + def __init__(self, source_name, isrc=None): + self.source_name = source_name + self.isrc = isrc + self.calls = [] + + def get_track_details(self, track_id): + self.calls.append(track_id) + if self.isrc is None: + return None + return { + 'id': track_id, + 'external_ids': {'isrc': self.isrc}, + } + + +class _FakeMBClient: + def __init__(self): + self.calls = [] + + def search_recording(self, title, artist_name=None, limit=1): + self.calls.append((title, artist_name, limit)) + return [{'id': 'mb-recording'}] + + +def _make_db(): + conn = sqlite3.connect(':memory:') + conn.row_factory = sqlite3.Row + cursor = conn.cursor() + cursor.execute( + """ + CREATE TABLE artists ( + id INTEGER PRIMARY KEY, + name TEXT, + thumb_url TEXT + ) + """ + ) + cursor.execute( + """ + CREATE TABLE albums ( + id INTEGER PRIMARY KEY, + title TEXT, + thumb_url TEXT + ) + """ + ) + cursor.execute( + """ + CREATE TABLE tracks ( + id INTEGER PRIMARY KEY, + title TEXT, + artist_id INTEGER, + album_id INTEGER, + spotify_track_id TEXT, + itunes_track_id TEXT, + deezer_track_id TEXT, + isrc TEXT, + musicbrainz_recording_id TEXT + ) + """ + ) + cursor.execute("INSERT INTO artists (id, name, thumb_url) VALUES (1, 'Artist', '')") + cursor.execute("INSERT INTO albums (id, title, thumb_url) VALUES (1, 'Album', '')") + cursor.execute( + """ + INSERT INTO tracks + (id, title, artist_id, album_id, spotify_track_id, itunes_track_id, deezer_track_id, isrc, musicbrainz_recording_id) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?) + """, + (1, 'Track Title', 1, 1, 'sp-1', None, 'dz-1', '', ''), + ) + conn.commit() + return conn + + +def _make_context(conn): + findings = [] + return SimpleNamespace( + db=SimpleNamespace(_get_connection=lambda: conn), + config_manager=SimpleNamespace(get=lambda key, default=None: default), + check_stop=lambda: False, + wait_if_paused=lambda: False, + update_progress=lambda *args, **kwargs: None, + report_progress=lambda *args, **kwargs: None, + sleep_or_stop=lambda seconds: False, + mb_client=_FakeMBClient(), + create_finding=lambda **kwargs: findings.append(kwargs), + findings=findings, + ) + + +def test_metadata_gap_filler_prefers_primary_track_source(monkeypatch): + conn = _make_db() + context = _make_context(conn) + + spotify_client = _FakeTrackClient('spotify', isrc='SP-ISRC') + deezer_client = _FakeTrackClient('deezer', isrc='DZ-ISRC') + itunes_client = _FakeTrackClient('itunes', isrc=None) + + monkeypatch.setattr(mgf, 'get_primary_source', lambda: 'deezer') + monkeypatch.setattr( + mgf, + 'get_client_for_source', + lambda source: {'spotify': spotify_client, 'deezer': deezer_client, 'itunes': itunes_client}.get(source), + ) + + result = mgf.MetadataGapFillerJob().scan(context) + + assert result.findings_created == 1 + assert deezer_client.calls == ['dz-1'] + assert spotify_client.calls == [] + assert context.findings[0]['details']['found_fields']['isrc'] == 'DZ-ISRC' + assert context.findings[0]['details']['resolved_source'] == 'deezer' + assert context.findings[0]['details']['resolved_track_id'] == 'dz-1' + + +def test_metadata_gap_filler_skips_track_detail_lookup_when_isrc_disabled(monkeypatch): + conn = _make_db() + context = _make_context(conn) + + spotify_client = _FakeTrackClient('spotify', isrc='SP-ISRC') + deezer_client = _FakeTrackClient('deezer', isrc='DZ-ISRC') + + monkeypatch.setattr(mgf, 'get_primary_source', lambda: 'deezer') + monkeypatch.setattr( + mgf, + 'get_client_for_source', + lambda source: {'spotify': spotify_client, 'deezer': deezer_client}.get(source), + ) + + job = mgf.MetadataGapFillerJob() + monkeypatch.setattr(job, '_get_settings', lambda context: {'fill_isrc': False, 'fill_musicbrainz_id': True}) + + result = job.scan(context) + + assert result.findings_created == 1 + assert spotify_client.calls == [] + assert deezer_client.calls == [] + assert context.findings[0]['details']['found_fields']['musicbrainz_recording_id'] == 'mb-recording' diff --git a/webui/static/script.js b/webui/static/script.js index 34a033d9..2af1524b 100644 --- a/webui/static/script.js +++ b/webui/static/script.js @@ -64089,6 +64089,8 @@ function _renderFindingDetail(f) { if (d.album) rows.push(['Album', d.album]); if (d.title) rows.push(['Title', d.title]); if (d.spotify_track_id) rows.push(['Spotify ID', d.spotify_track_id]); + if (d.resolved_source) rows.push(['Resolved Source', d.resolved_source]); + if (d.resolved_track_id) rows.push(['Resolved Track ID', d.resolved_track_id]); if (d.found_fields && typeof d.found_fields === 'object') { Object.entries(d.found_fields).forEach(([k, v]) => { rows.push([`Found: ${k}`, String(v), 'success']); From dab58766b767955cafdd73d213217154a21c6e87 Mon Sep 17 00:00:00 2001 From: Antti Kettunen Date: Wed, 15 Apr 2026 21:18:54 +0300 Subject: [PATCH 5/5] Make library reorganize source-aware Respect the configured metadata source order when looking up album years, and re-check provider availability during the scan so Spotify can drop out cleanly if it becomes rate-limited. --- core/repair_jobs/library_reorganize.py | 71 ++++++-------- tests/test_library_reorganize.py | 129 +++++++++++++++++++++++++ 2 files changed, 157 insertions(+), 43 deletions(-) create mode 100644 tests/test_library_reorganize.py diff --git a/core/repair_jobs/library_reorganize.py b/core/repair_jobs/library_reorganize.py index 68afd5bd..4afa9552 100644 --- a/core/repair_jobs/library_reorganize.py +++ b/core/repair_jobs/library_reorganize.py @@ -16,8 +16,8 @@ import os import re import shutil import sys -import time +from core.metadata_service import get_client_for_source, get_primary_source, get_source_priority from core.repair_jobs import register_job from core.repair_jobs.base import JobContext, JobResult, RepairJob from utils.logging_config import get_logger @@ -758,10 +758,10 @@ class LibraryReorganizeJob(RepairJob): return years def _lookup_years_from_api(self, context, missing_pairs) -> dict: - """Batch-lookup release years from Spotify/iTunes/Deezer for albums not found in DB. + """Batch-lookup release years from the configured metadata providers for albums not found in DB. Args: - context: JobContext with spotify_client, config_manager + context: JobContext with config_manager missing_pairs: set of (artist, album) tuples needing year lookup Returns: @@ -771,37 +771,17 @@ class LibraryReorganizeJob(RepairJob): if not missing_pairs: return years - # Determine which client to use - search_client = None - source_name = 'unknown' - if context.spotify_client and hasattr(context.spotify_client, 'search_albums'): - try: - if context.spotify_client.is_spotify_authenticated(): - search_client = context.spotify_client - source_name = 'Spotify' - except Exception: - pass - - if not search_client: - # Try fallback (iTunes/Deezer) - try: - from core.metadata_service import get_primary_client - search_client = get_primary_client() - source_name = 'fallback' - except Exception: - pass - - if not search_client or not hasattr(search_client, 'search_albums'): - return years + primary_source = get_primary_source() + source_priority = get_source_priority(primary_source) # Cap lookups to avoid excessive API calls max_lookups = 200 pairs_list = list(missing_pairs)[:max_lookups] - logger.info("Looking up %d album years from %s API", len(pairs_list), source_name) + logger.info("Looking up %d album years from configured metadata providers", len(pairs_list)) if context.report_progress: context.report_progress( - phase=f'Looking up {len(pairs_list)} album years from {source_name}...', + phase=f'Looking up {len(pairs_list)} album years from metadata providers...', log_line=f'Fetching release years for {len(pairs_list)} albums', log_type='info' ) @@ -809,22 +789,27 @@ class LibraryReorganizeJob(RepairJob): for artist, album in pairs_list: if context.check_stop(): break - try: - results = search_client.search_albums(f"{artist} {album}", limit=3) - if results: - for r in results: - release_date = getattr(r, 'release_date', '') or '' - if release_date and len(release_date) >= 4: - year_str = release_date[:4] - if year_str.isdigit(): - key = (artist.lower(), album.lower()) - years[key] = year_str - break - import time - if context.sleep_or_stop(0.1): # Rate limit courtesy - break - except Exception as e: - logger.debug("API year lookup failed for %s - %s: %s", artist, album, e) + key = (artist.lower(), album.lower()) + for source_name in source_priority: + try: + search_client = get_client_for_source(source_name) + if not search_client or not hasattr(search_client, 'search_albums'): + continue + results = search_client.search_albums(f"{artist} {album}", limit=3) + if results: + for r in results: + release_date = getattr(r, 'release_date', '') or '' + if release_date and len(release_date) >= 4: + year_str = release_date[:4] + if year_str.isdigit(): + years[key] = year_str + break + if key in years: + break + if context.sleep_or_stop(0.1): # Rate limit courtesy + return years + except Exception as e: + logger.debug("API year lookup failed for %s - %s via %s: %s", artist, album, source_name, e) logger.info("API year lookup: found %d/%d years", len(years), len(pairs_list)) return years diff --git a/tests/test_library_reorganize.py b/tests/test_library_reorganize.py new file mode 100644 index 00000000..d0fd6afb --- /dev/null +++ b/tests/test_library_reorganize.py @@ -0,0 +1,129 @@ +import sys +import types +from types import SimpleNamespace + +# Stub optional Spotify dependency so metadata_service can import in tests. +if 'spotipy' not in sys.modules: + spotipy = types.ModuleType('spotipy') + oauth2 = types.ModuleType('spotipy.oauth2') + + class _DummySpotify: + pass + + class _DummyOAuth: + pass + + spotipy.Spotify = _DummySpotify + oauth2.SpotifyOAuth = _DummyOAuth + oauth2.SpotifyClientCredentials = _DummyOAuth + spotipy.oauth2 = oauth2 + sys.modules['spotipy'] = spotipy + sys.modules['spotipy.oauth2'] = oauth2 + +if 'config.settings' not in sys.modules: + config_mod = types.ModuleType('config') + settings_mod = types.ModuleType('config.settings') + + class _DummyConfigManager: + def get(self, key, default=None): + return default + + settings_mod.config_manager = _DummyConfigManager() + config_mod.settings = settings_mod + sys.modules['config'] = config_mod + sys.modules['config.settings'] = settings_mod + +from core.repair_jobs import library_reorganize as lr + + +class _FakeSearchClient: + def __init__(self, source_name, year=None): + self.source_name = source_name + self.year = year + self.calls = [] + + def search_albums(self, query, limit=3): + self.calls.append((query, limit)) + if self.year is None: + return [] + return [SimpleNamespace(release_date=f"{self.year}-01-01")] + + +def test_lookup_years_prefers_primary_source(monkeypatch): + deezer_client = _FakeSearchClient('deezer', '2022') + spotify_client = _FakeSearchClient('spotify', '1999') + + monkeypatch.setattr(lr, 'get_primary_source', lambda: 'deezer') + monkeypatch.setattr(lr, 'get_source_priority', lambda primary: [primary, 'itunes', 'spotify']) + monkeypatch.setattr( + lr, + 'get_client_for_source', + lambda source: {'deezer': deezer_client, 'spotify': spotify_client}.get(source), + ) + + job = lr.LibraryReorganizeJob() + result = job._lookup_years_from_api(SimpleNamespace(report_progress=None, check_stop=lambda: False, sleep_or_stop=lambda *_: False), {('Artist', 'Album')}) + + assert result == {('artist', 'album'): '2022'} + assert deezer_client.calls == [('Artist Album', 3)] + assert spotify_client.calls == [] + + +def test_lookup_years_falls_through_to_later_source(monkeypatch): + deezer_client = _FakeSearchClient('deezer', None) + spotify_client = _FakeSearchClient('spotify', '1999') + + monkeypatch.setattr(lr, 'get_primary_source', lambda: 'deezer') + monkeypatch.setattr(lr, 'get_source_priority', lambda primary: [primary, 'itunes', 'spotify']) + monkeypatch.setattr( + lr, + 'get_client_for_source', + lambda source: {'deezer': deezer_client, 'spotify': spotify_client}.get(source), + ) + + job = lr.LibraryReorganizeJob() + result = job._lookup_years_from_api( + SimpleNamespace(report_progress=None, check_stop=lambda: False, sleep_or_stop=lambda *_: False), + {('Artist', 'Album')}, + ) + + assert result == {('artist', 'album'): '1999'} + assert deezer_client.calls == [('Artist Album', 3)] + assert spotify_client.calls == [('Artist Album', 3)] + + +def test_lookup_years_rechecks_client_availability_per_album(monkeypatch): + availability = {'spotify': True} + + class _SpotifyClient(_FakeSearchClient): + def search_albums(self, query, limit=3): + self.calls.append((query, limit)) + availability['spotify'] = False + return [] + + spotify_client = _SpotifyClient('spotify', None) + itunes_client = _FakeSearchClient('itunes', '2002') + helper_calls = [] + + def fake_get_client_for_source(source): + helper_calls.append(source) + if source == 'spotify' and not availability['spotify']: + return None + return {'spotify': spotify_client, 'itunes': itunes_client}.get(source) + + monkeypatch.setattr(lr, 'get_primary_source', lambda: 'spotify') + monkeypatch.setattr(lr, 'get_source_priority', lambda primary: [primary, 'itunes']) + monkeypatch.setattr(lr, 'get_client_for_source', fake_get_client_for_source) + + job = lr.LibraryReorganizeJob() + result = job._lookup_years_from_api( + SimpleNamespace(report_progress=None, check_stop=lambda: False, sleep_or_stop=lambda *_: False), + {('Artist A', 'Album A'), ('Artist B', 'Album B')}, + ) + + assert result == {('artist a', 'album a'): '2002', ('artist b', 'album b'): '2002'} + assert helper_calls.count('spotify') == 2 + assert helper_calls.count('itunes') == 2 + assert len(spotify_client.calls) == 1 + assert spotify_client.calls[0] in [('Artist A Album A', 3), ('Artist B Album B', 3)] + assert set(itunes_client.calls) == {('Artist A Album A', 3), ('Artist B Album B', 3)}