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)}