From 8a1ae0094644936c38cba40214e1b2f4169efdf7 Mon Sep 17 00:00:00 2001 From: Antti Kettunen Date: Wed, 15 Apr 2026 07:18:29 +0300 Subject: [PATCH] Utilize global Spotify client in repair jobs Album completeness and any other repair job now uses the centralized source/client helpers instead of a worker-local Spotify client or override plumbing - This keeps source selection aligned with the configured primary provider and removes the last Spotify-only special case from the job path. This change ultimately is a step towards further centralizing the Spotify client access and the associated `is_spotify_authenticated` check. - Currently these look-ups are done all over the place in different feature implementations directly, but moving forward, any feature that uses `get_primary_client` or `get_client_for_source` to access the Spotify client, won't have to duplicate any rate-limiting or auth checks as long as these getters are used --- core/metadata_service.py | 5 ++--- core/repair_jobs/album_completeness.py | 9 ++++----- core/repair_worker.py | 14 ++++++-------- tests/test_album_completeness_job.py | 18 +++++++++--------- 4 files changed, 21 insertions(+), 25 deletions(-) diff --git a/core/metadata_service.py b/core/metadata_service.py index 48bbecdd..5becc9ee 100644 --- a/core/metadata_service.py +++ b/core/metadata_service.py @@ -97,14 +97,13 @@ def get_client_for_source(source: str): return None -def get_album_tracks_for_source(source: str, album_id: str, client: Any = None): +def get_album_tracks_for_source(source: str, album_id: str): """Get album tracks for an exact source. Returns Spotify-compatible dict/list data or None. No fallback swaps. """ - if client is None: - client = get_client_for_source(source) + client = get_client_for_source(source) if not client: return None diff --git a/core/repair_jobs/album_completeness.py b/core/repair_jobs/album_completeness.py index cfb90b23..6c6dc8e2 100644 --- a/core/repair_jobs/album_completeness.py +++ b/core/repair_jobs/album_completeness.py @@ -256,7 +256,7 @@ class AlbumCompletenessJob(RepairJob): album_id = self._get_album_id_for_source(source, album_ids) if not album_id: continue - api_tracks = self._get_album_tracks(context, source, album_id) + api_tracks = self._get_album_tracks(source, album_id) items = self._extract_track_items(api_tracks) if items: return len(items) @@ -288,7 +288,7 @@ class AlbumCompletenessJob(RepairJob): source_album_id = self._get_album_id_for_source(source, album_ids) if not source_album_id: continue - api_tracks = self._get_album_tracks(context, source, source_album_id) + api_tracks = self._get_album_tracks(source, source_album_id) if self._extract_track_items(api_tracks): break @@ -353,14 +353,13 @@ class AlbumCompletenessJob(RepairJob): def _get_album_id_for_source(self, source: str, album_ids: dict) -> str: return album_ids.get(source, '') - def _get_album_tracks(self, context: JobContext, source: str, album_id: str): + def _get_album_tracks(self, source: str, album_id: str): """Fetch album tracks from a specific source.""" try: if source not in ('spotify', 'itunes', 'deezer', 'discogs', 'hydrabase'): return None - client = context.spotify_client if source == 'spotify' else None - return get_album_tracks_for_source(source, album_id, client=client) + return get_album_tracks_for_source(source, album_id) except Exception as e: logger.debug("Error getting %s album tracks for %s: %s", source.capitalize(), album_id, e) return None diff --git a/core/repair_worker.py b/core/repair_worker.py index 5985ea5a..e556eadf 100644 --- a/core/repair_worker.py +++ b/core/repair_worker.py @@ -105,7 +105,6 @@ class RepairWorker: self._on_job_finish = None # (job_id, status, result) -> None # Lazy client accessors - self._spotify_client = None self._itunes_client = None self._mb_client = None self._acoustid_client = None @@ -151,13 +150,12 @@ class RepairWorker: # ------------------------------------------------------------------ @property def spotify_client(self): - if self._spotify_client is None: - try: - from core.spotify_client import SpotifyClient - self._spotify_client = SpotifyClient() - except Exception as e: - logger.error("Failed to initialize SpotifyClient: %s", e) - return self._spotify_client + try: + from core.metadata_service import get_client_for_source + return get_client_for_source('spotify') + except Exception as e: + logger.error("Failed to resolve shared Spotify client: %s", e) + return None @property def itunes_client(self): diff --git a/tests/test_album_completeness_job.py b/tests/test_album_completeness_job.py index 65ee11b7..b15d4fd9 100644 --- a/tests/test_album_completeness_job.py +++ b/tests/test_album_completeness_job.py @@ -177,8 +177,8 @@ def test_album_completeness_uses_primary_provider_first(monkeypatch): monkeypatch.setattr( album_completeness_module, "get_album_tracks_for_source", - lambda source, album_id, client=None: ( - calls.append((source, album_id, client)) or + lambda source, album_id: ( + calls.append((source, album_id)) or ( deezer_client.get_album_tracks(album_id) if source == "deezer" else itunes_client.get_album_tracks(album_id) if source == "itunes" else @@ -192,7 +192,7 @@ def test_album_completeness_uses_primary_provider_first(monkeypatch): missing_tracks = job._find_missing_tracks(context, "deezer", 42, album_ids) assert expected_total == 2 - assert calls == [("deezer", "deezer-album", None), ("deezer", "deezer-album", None)] + assert calls == [("deezer", "deezer-album"), ("deezer", "deezer-album")] assert deezer_client.calls == ["deezer-album", "deezer-album"] assert context.spotify_client.calls == [] assert itunes_client.calls == [] @@ -227,8 +227,8 @@ def test_album_completeness_supports_discogs_primary(monkeypatch): monkeypatch.setattr( album_completeness_module, "get_album_tracks_for_source", - lambda source, album_id, client=None: ( - calls.append((source, album_id, client)) or + lambda source, album_id: ( + calls.append((source, album_id)) or ( discogs_client.get_album_tracks(album_id) if source == "discogs" else itunes_client.get_album_tracks(album_id) if source == "itunes" else @@ -243,7 +243,7 @@ def test_album_completeness_supports_discogs_primary(monkeypatch): missing_tracks = job._find_missing_tracks(context, "discogs", 42, album_ids) assert expected_total == 3 - assert calls == [("discogs", "discogs-release", None), ("discogs", "discogs-release", None)] + assert calls == [("discogs", "discogs-release"), ("discogs", "discogs-release")] assert discogs_client.calls == ["discogs-release", "discogs-release"] assert context.spotify_client.calls == [] assert itunes_client.calls == [] @@ -278,8 +278,8 @@ def test_album_completeness_supports_hydrabase_primary(monkeypatch): monkeypatch.setattr( album_completeness_module, "get_album_tracks_for_source", - lambda source, album_id, client=None: ( - calls.append((source, album_id, client)) or + lambda source, album_id: ( + calls.append((source, album_id)) or ( hydrabase_client.get_album_tracks_dict(album_id) if source == "hydrabase" else itunes_client.get_album_tracks(album_id) if source == "itunes" else @@ -294,7 +294,7 @@ def test_album_completeness_supports_hydrabase_primary(monkeypatch): missing_tracks = job._find_missing_tracks(context, "hydrabase", 42, album_ids) assert expected_total == 4 - assert calls == [("hydrabase", "soul-album", None), ("hydrabase", "soul-album", None)] + assert calls == [("hydrabase", "soul-album"), ("hydrabase", "soul-album")] assert hydrabase_client.calls == ["soul-album", "soul-album"] assert context.spotify_client.calls == [] assert itunes_client.calls == []