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.
This commit is contained in:
Antti Kettunen
parent
03711c10df
commit
dab58766b7
2 changed files with 157 additions and 43 deletions
|
|
@ -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
|
||||
|
|
|
|||
129
tests/test_library_reorganize.py
Normal file
129
tests/test_library_reorganize.py
Normal file
|
|
@ -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)}
|
||||
Loading…
Reference in a new issue