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
This commit is contained in:
parent
1a459412a3
commit
8a1ae00946
4 changed files with 21 additions and 25 deletions
|
|
@ -97,14 +97,13 @@ def get_client_for_source(source: str):
|
||||||
return None
|
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.
|
"""Get album tracks for an exact source.
|
||||||
|
|
||||||
Returns Spotify-compatible dict/list data or None.
|
Returns Spotify-compatible dict/list data or None.
|
||||||
No fallback swaps.
|
No fallback swaps.
|
||||||
"""
|
"""
|
||||||
if client is None:
|
client = get_client_for_source(source)
|
||||||
client = get_client_for_source(source)
|
|
||||||
if not client:
|
if not client:
|
||||||
return None
|
return None
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -256,7 +256,7 @@ class AlbumCompletenessJob(RepairJob):
|
||||||
album_id = self._get_album_id_for_source(source, album_ids)
|
album_id = self._get_album_id_for_source(source, album_ids)
|
||||||
if not album_id:
|
if not album_id:
|
||||||
continue
|
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)
|
items = self._extract_track_items(api_tracks)
|
||||||
if items:
|
if items:
|
||||||
return len(items)
|
return len(items)
|
||||||
|
|
@ -288,7 +288,7 @@ class AlbumCompletenessJob(RepairJob):
|
||||||
source_album_id = self._get_album_id_for_source(source, album_ids)
|
source_album_id = self._get_album_id_for_source(source, album_ids)
|
||||||
if not source_album_id:
|
if not source_album_id:
|
||||||
continue
|
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):
|
if self._extract_track_items(api_tracks):
|
||||||
break
|
break
|
||||||
|
|
||||||
|
|
@ -353,14 +353,13 @@ class AlbumCompletenessJob(RepairJob):
|
||||||
def _get_album_id_for_source(self, source: str, album_ids: dict) -> str:
|
def _get_album_id_for_source(self, source: str, album_ids: dict) -> str:
|
||||||
return album_ids.get(source, '')
|
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."""
|
"""Fetch album tracks from a specific source."""
|
||||||
try:
|
try:
|
||||||
if source not in ('spotify', 'itunes', 'deezer', 'discogs', 'hydrabase'):
|
if source not in ('spotify', 'itunes', 'deezer', 'discogs', 'hydrabase'):
|
||||||
return None
|
return None
|
||||||
|
|
||||||
client = context.spotify_client if source == 'spotify' else None
|
return get_album_tracks_for_source(source, album_id)
|
||||||
return get_album_tracks_for_source(source, album_id, client=client)
|
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
logger.debug("Error getting %s album tracks for %s: %s", source.capitalize(), album_id, e)
|
logger.debug("Error getting %s album tracks for %s: %s", source.capitalize(), album_id, e)
|
||||||
return None
|
return None
|
||||||
|
|
|
||||||
|
|
@ -105,7 +105,6 @@ class RepairWorker:
|
||||||
self._on_job_finish = None # (job_id, status, result) -> None
|
self._on_job_finish = None # (job_id, status, result) -> None
|
||||||
|
|
||||||
# Lazy client accessors
|
# Lazy client accessors
|
||||||
self._spotify_client = None
|
|
||||||
self._itunes_client = None
|
self._itunes_client = None
|
||||||
self._mb_client = None
|
self._mb_client = None
|
||||||
self._acoustid_client = None
|
self._acoustid_client = None
|
||||||
|
|
@ -151,13 +150,12 @@ class RepairWorker:
|
||||||
# ------------------------------------------------------------------
|
# ------------------------------------------------------------------
|
||||||
@property
|
@property
|
||||||
def spotify_client(self):
|
def spotify_client(self):
|
||||||
if self._spotify_client is None:
|
try:
|
||||||
try:
|
from core.metadata_service import get_client_for_source
|
||||||
from core.spotify_client import SpotifyClient
|
return get_client_for_source('spotify')
|
||||||
self._spotify_client = SpotifyClient()
|
except Exception as e:
|
||||||
except Exception as e:
|
logger.error("Failed to resolve shared Spotify client: %s", e)
|
||||||
logger.error("Failed to initialize SpotifyClient: %s", e)
|
return None
|
||||||
return self._spotify_client
|
|
||||||
|
|
||||||
@property
|
@property
|
||||||
def itunes_client(self):
|
def itunes_client(self):
|
||||||
|
|
|
||||||
|
|
@ -177,8 +177,8 @@ def test_album_completeness_uses_primary_provider_first(monkeypatch):
|
||||||
monkeypatch.setattr(
|
monkeypatch.setattr(
|
||||||
album_completeness_module,
|
album_completeness_module,
|
||||||
"get_album_tracks_for_source",
|
"get_album_tracks_for_source",
|
||||||
lambda source, album_id, client=None: (
|
lambda source, album_id: (
|
||||||
calls.append((source, album_id, client)) or
|
calls.append((source, album_id)) or
|
||||||
(
|
(
|
||||||
deezer_client.get_album_tracks(album_id) if source == "deezer" else
|
deezer_client.get_album_tracks(album_id) if source == "deezer" else
|
||||||
itunes_client.get_album_tracks(album_id) if source == "itunes" 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)
|
missing_tracks = job._find_missing_tracks(context, "deezer", 42, album_ids)
|
||||||
|
|
||||||
assert expected_total == 2
|
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 deezer_client.calls == ["deezer-album", "deezer-album"]
|
||||||
assert context.spotify_client.calls == []
|
assert context.spotify_client.calls == []
|
||||||
assert itunes_client.calls == []
|
assert itunes_client.calls == []
|
||||||
|
|
@ -227,8 +227,8 @@ def test_album_completeness_supports_discogs_primary(monkeypatch):
|
||||||
monkeypatch.setattr(
|
monkeypatch.setattr(
|
||||||
album_completeness_module,
|
album_completeness_module,
|
||||||
"get_album_tracks_for_source",
|
"get_album_tracks_for_source",
|
||||||
lambda source, album_id, client=None: (
|
lambda source, album_id: (
|
||||||
calls.append((source, album_id, client)) or
|
calls.append((source, album_id)) or
|
||||||
(
|
(
|
||||||
discogs_client.get_album_tracks(album_id) if source == "discogs" else
|
discogs_client.get_album_tracks(album_id) if source == "discogs" else
|
||||||
itunes_client.get_album_tracks(album_id) if source == "itunes" 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)
|
missing_tracks = job._find_missing_tracks(context, "discogs", 42, album_ids)
|
||||||
|
|
||||||
assert expected_total == 3
|
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 discogs_client.calls == ["discogs-release", "discogs-release"]
|
||||||
assert context.spotify_client.calls == []
|
assert context.spotify_client.calls == []
|
||||||
assert itunes_client.calls == []
|
assert itunes_client.calls == []
|
||||||
|
|
@ -278,8 +278,8 @@ def test_album_completeness_supports_hydrabase_primary(monkeypatch):
|
||||||
monkeypatch.setattr(
|
monkeypatch.setattr(
|
||||||
album_completeness_module,
|
album_completeness_module,
|
||||||
"get_album_tracks_for_source",
|
"get_album_tracks_for_source",
|
||||||
lambda source, album_id, client=None: (
|
lambda source, album_id: (
|
||||||
calls.append((source, album_id, client)) or
|
calls.append((source, album_id)) or
|
||||||
(
|
(
|
||||||
hydrabase_client.get_album_tracks_dict(album_id) if source == "hydrabase" else
|
hydrabase_client.get_album_tracks_dict(album_id) if source == "hydrabase" else
|
||||||
itunes_client.get_album_tracks(album_id) if source == "itunes" 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)
|
missing_tracks = job._find_missing_tracks(context, "hydrabase", 42, album_ids)
|
||||||
|
|
||||||
assert expected_total == 4
|
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 hydrabase_client.calls == ["soul-album", "soul-album"]
|
||||||
assert context.spotify_client.calls == []
|
assert context.spotify_client.calls == []
|
||||||
assert itunes_client.calls == []
|
assert itunes_client.calls == []
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue