From fd6335a66e002ba9979039a34263ac54275426ca Mon Sep 17 00:00:00 2001 From: Antti Kettunen Date: Sat, 11 Apr 2026 13:55:10 +0300 Subject: [PATCH] Add / improve metadata client caching Clients are for the most part being initialized per-request, which leads to a lot of redundant client initialization, as well as noise on the logs, since each client initialization emits a row on the logs, eg. 'Deezer client initialized' --- core/metadata_service.py | 127 +++++++++++++++++++------- core/watchlist_scanner.py | 12 +-- tests/test_metadata_service_cache.py | 107 ++++++++++++++++++++++ web_server.py | 131 ++++++++++----------------- 4 files changed, 255 insertions(+), 122 deletions(-) create mode 100644 tests/test_metadata_service_cache.py diff --git a/core/metadata_service.py b/core/metadata_service.py index 9173f809..829fd210 100644 --- a/core/metadata_service.py +++ b/core/metadata_service.py @@ -7,6 +7,7 @@ the logic. This prevents bugs where different files have different defaults or auth checks. """ +import threading from typing import List, Optional, Dict, Any, Literal from core.spotify_client import SpotifyClient from core.itunes_client import iTunesClient @@ -16,6 +17,9 @@ logger = get_logger("metadata_service") MetadataProvider = Literal["spotify", "itunes", "auto"] +_client_cache_lock = threading.RLock() +_client_cache: Dict[str, Any] = {} + # ============================================================================= # CANONICAL SOURCE SELECTION — all code should use these two functions @@ -58,8 +62,89 @@ def get_primary_client(): This is THE single source of truth for "which client should I call?" """ - source = get_primary_source() + return _get_client_for_source(get_primary_source()) + + +def get_deezer_client(): + """Get cached Deezer client. + + Deezer client is safe to reuse across requests because it owns no + request-specific state beyond the current access token. + """ + from core.deezer_client import DeezerClient + try: + from config.settings import config_manager + current_token = config_manager.get('deezer.access_token', None) + except Exception: + current_token = None + + cache_key = f"deezer::{current_token or ''}" + with _client_cache_lock: + client = _client_cache.get(cache_key) + if client is None: + client = DeezerClient() + _client_cache[cache_key] = client + return client + + +def get_itunes_client(): + """Get cached iTunes client.""" + with _client_cache_lock: + client = _client_cache.get("itunes") + if client is None: + client = iTunesClient() + _client_cache["itunes"] = client + return client + + +def get_discogs_client(token: Optional[str] = None): + """Get cached Discogs client. + + Discogs auth changes are token-driven, so the cache key tracks the + current configured token. + """ + if token is None: + try: + from config.settings import config_manager + current_token = config_manager.get('discogs.token', '') or '' + except Exception: + current_token = '' + else: + current_token = token or '' + + cache_key = f"discogs::{current_token}" + with _client_cache_lock: + client = _client_cache.get(cache_key) + if client is None: + from core.discogs_client import DiscogsClient + client = DiscogsClient(token=current_token or None) + _client_cache[cache_key] = client + return client + + +def get_hydrabase_client(): + """Return current Hydrabase client if connected, else iTunes fallback.""" + try: + import importlib + ws = importlib.import_module('web_server') + client = getattr(ws, 'hydrabase_client', None) + if client and client.is_connected(): + return client + except Exception: + pass + return get_itunes_client() + + +def clear_cached_metadata_clients(): + """Clear cached metadata clients. + + Useful for tests and config reload flows. + """ + with _client_cache_lock: + _client_cache.clear() + +def _get_client_for_source(source: str): if source == 'spotify': try: import importlib @@ -69,38 +154,18 @@ def get_primary_client(): return sc except Exception: pass - # Spotify selected but unavailable — fall back to Deezer - from core.deezer_client import DeezerClient - return DeezerClient() + return get_deezer_client() if source == 'deezer': - from core.deezer_client import DeezerClient - return DeezerClient() + return get_deezer_client() if source == 'discogs': - try: - from config.settings import config_manager - token = config_manager.get('discogs.token', '') - if token: - from core.discogs_client import DiscogsClient - return DiscogsClient(token=token) - except Exception: - pass - return iTunesClient() + return get_discogs_client() if source == 'hydrabase': - try: - import importlib - ws = importlib.import_module('web_server') - client = getattr(ws, 'hydrabase_client', None) - if client and client.is_connected(): - return client - except Exception: - pass - return iTunesClient() + return get_hydrabase_client() - # Default: iTunes - return iTunesClient() + return get_itunes_client() # ============================================================================= @@ -141,7 +206,7 @@ class MetadataService: self.preferred_provider = preferred_provider self.spotify = SpotifyClient() self._fallback_source = get_primary_source() - self.itunes = get_primary_client() # May be iTunesClient or DeezerClient + self.itunes = _get_client_for_source(self._fallback_source) self._log_initialization() @@ -311,13 +376,9 @@ class MetadataService: """Reload configuration for both clients""" logger.info("Reloading metadata service configuration") self.spotify.reload_config() - # Re-create fallback client in case the setting changed new_source = get_primary_source() - if new_source != self._fallback_source: - self._fallback_source = new_source - self.itunes = get_primary_client() - elif hasattr(self.itunes, 'reload_config'): - self.itunes.reload_config() + self._fallback_source = new_source + self.itunes = _get_client_for_source(new_source) self._log_initialization() diff --git a/core/watchlist_scanner.py b/core/watchlist_scanner.py index bb57e6e5..23f253c2 100644 --- a/core/watchlist_scanner.py +++ b/core/watchlist_scanner.py @@ -1169,9 +1169,9 @@ class WatchlistScanner: results = client.search_artists(artist_name, limit=5) return self._best_artist_match(results, artist_name) - # Fallback: create a fresh Deezer client - from core.deezer_client import DeezerClient - client = DeezerClient() + # Fallback: use cached Deezer client + from core.metadata_service import get_deezer_client + client = get_deezer_client() results = client.search_artists(artist_name, limit=5) return self._best_artist_match(results, artist_name) except Exception as e: @@ -1181,8 +1181,8 @@ class WatchlistScanner: def _match_to_discogs(self, artist_name: str) -> Optional[str]: """Match artist name to Discogs ID using fuzzy name comparison.""" try: - from core.discogs_client import DiscogsClient - client = DiscogsClient() + from core.metadata_service import get_discogs_client + client = get_discogs_client() results = client.search_artists(artist_name, limit=5) return self._best_artist_match(results, artist_name) except Exception as e: @@ -3266,4 +3266,4 @@ def get_watchlist_scanner(spotify_client: SpotifyClient) -> WatchlistScanner: global _watchlist_scanner_instance if _watchlist_scanner_instance is None: _watchlist_scanner_instance = WatchlistScanner(spotify_client) - return _watchlist_scanner_instance \ No newline at end of file + return _watchlist_scanner_instance diff --git a/tests/test_metadata_service_cache.py b/tests/test_metadata_service_cache.py new file mode 100644 index 00000000..e6abecd4 --- /dev/null +++ b/tests/test_metadata_service_cache.py @@ -0,0 +1,107 @@ +import sys +import types + +import pytest + + +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 import metadata_service +from config.settings import config_manager + + +@pytest.fixture(autouse=True) +def _clear_metadata_client_cache(): + metadata_service.clear_cached_metadata_clients() + yield + metadata_service.clear_cached_metadata_clients() + + +def test_primary_client_is_cached_for_same_source(monkeypatch): + calls = {"deezer": 0} + + class FakeDeezerClient: + def __init__(self): + calls["deezer"] += 1 + + monkeypatch.setattr(metadata_service, "get_primary_source", lambda: "deezer") + monkeypatch.setattr("core.deezer_client.DeezerClient", FakeDeezerClient) + + first = metadata_service.get_primary_client() + second = metadata_service.get_primary_client() + + assert first is second + assert calls["deezer"] == 1 + + +def test_primary_client_switches_cache_by_source(monkeypatch): + calls = {"deezer": 0, "itunes": 0} + sources = iter(["deezer", "itunes"]) + + class FakeDeezerClient: + def __init__(self): + calls["deezer"] += 1 + + class FakeITunesClient: + def __init__(self): + calls["itunes"] += 1 + + monkeypatch.setattr(metadata_service, "get_primary_source", lambda: next(sources)) + monkeypatch.setattr("core.deezer_client.DeezerClient", FakeDeezerClient) + monkeypatch.setattr(metadata_service, "iTunesClient", FakeITunesClient) + + deezer_client = metadata_service.get_primary_client() + itunes_client = metadata_service.get_primary_client() + + assert deezer_client is not itunes_client + assert calls["deezer"] == 1 + assert calls["itunes"] == 1 + + +def test_deezer_client_cache_tracks_token(monkeypatch): + tokens = iter(["token-a", "token-b"]) + calls = {"deezer": 0} + + class FakeDeezerClient: + def __init__(self): + calls["deezer"] += 1 + + monkeypatch.setattr("core.deezer_client.DeezerClient", FakeDeezerClient) + monkeypatch.setattr(config_manager, "get", lambda key, default=None: next(tokens) if key == "deezer.access_token" else default) + + first = metadata_service.get_deezer_client() + second = metadata_service.get_deezer_client() + + assert first is not second + assert calls["deezer"] == 2 diff --git a/web_server.py b/web_server.py index 7165ee3d..4485274c 100644 --- a/web_server.py +++ b/web_server.py @@ -701,8 +701,7 @@ def _register_automation_handlers(): elif source == 'deezer': try: - from core.deezer_client import DeezerClient - deezer = DeezerClient() + deezer = _get_deezer_client() playlist_data = deezer.get_playlist(source_id) if playlist_data and playlist_data.get('tracks'): tracks = [] @@ -7109,12 +7108,8 @@ def deezer_callback(): config_manager.set('deezer.access_token', access_token) # Reload the global deezer client to pick up the token - global deezer_client - if deezer_client: - deezer_client.reload_config() - else: - from core.deezer_client import DeezerClient - deezer_client = DeezerClient() + deezer_client = _get_deezer_client() + deezer_client.reload_config() add_activity_item("✅", "Deezer Auth Complete", "Deezer account connected via OAuth", "Now") logger.info("Deezer OAuth authentication successful") @@ -8008,15 +8003,13 @@ def enhanced_search_source(source_name): else: return jsonify({"artists": [], "albums": [], "tracks": [], "available": False}) elif source_name == 'itunes': - from core.itunes_client import iTunesClient - client = iTunesClient() + client = _get_itunes_client() elif source_name == 'deezer': client = _get_deezer_client() elif source_name == 'discogs': token = config_manager.get('discogs.token', '') if token: - from core.discogs_client import DiscogsClient - client = DiscogsClient(token=token) + client = _get_discogs_client(token) else: return jsonify({"artists": [], "albums": [], "tracks": [], "available": False}) elif source_name == 'hydrabase': @@ -10181,8 +10174,7 @@ def get_artist_image(artist_id): source_override = request.args.get('source', '') if source_override == 'itunes': - from core.itunes_client import iTunesClient - client = iTunesClient() + client = _get_itunes_client() image_url = client._get_artist_image_from_albums(artist_id) return jsonify({"success": True, "image_url": image_url}) elif source_override == 'deezer': @@ -10190,8 +10182,7 @@ def get_artist_image(artist_id): image_url = client._get_artist_image_from_albums(artist_id) return jsonify({"success": True, "image_url": image_url}) elif source_override == 'discogs': - from core.discogs_client import DiscogsClient - client = DiscogsClient() + client = _get_discogs_client() image_url = client._get_artist_image_from_albums(artist_id) return jsonify({"success": True, "image_url": image_url}) elif source_override == 'hydrabase': @@ -10201,8 +10192,7 @@ def get_artist_image(artist_id): client = _get_deezer_client() image_url = client._get_artist_image_from_albums(artist_id) elif plugin == 'itunes' or artist_id.isdigit(): - from core.itunes_client import iTunesClient - client = iTunesClient() + client = _get_itunes_client() image_url = client._get_artist_image_from_albums(artist_id) else: image_url = None @@ -10256,8 +10246,7 @@ def get_artist_discography(artist_id): if albums: active_source = 'spotify' elif source_override == 'itunes': - from core.itunes_client import iTunesClient - itunes_cl = iTunesClient() + itunes_cl = _get_itunes_client() albums = itunes_cl.get_artist_albums(artist_id, album_type='album,single', limit=50) if albums: active_source = 'itunes' @@ -10267,8 +10256,7 @@ def get_artist_discography(artist_id): if albums: active_source = 'deezer' elif source_override == 'discogs': - from core.discogs_client import DiscogsClient - discogs_cl = DiscogsClient() + discogs_cl = _get_discogs_client() albums = discogs_cl.get_artist_albums(artist_id, album_type='album,single', limit=50) if albums: active_source = 'discogs' @@ -10277,8 +10265,7 @@ def get_artist_discography(artist_id): if plugin == 'deezer': hb_cl = _get_deezer_client() elif plugin == 'itunes' or artist_id.isdigit(): - from core.itunes_client import iTunesClient - hb_cl = iTunesClient() + hb_cl = _get_itunes_client() else: hb_cl = spotify_client albums = hb_cl.get_artist_albums(artist_id, album_type='album,single', limit=50) @@ -10288,20 +10275,17 @@ def get_artist_discography(artist_id): # If direct ID lookup failed but we have artist name, search by name if not albums and artist_name: if source_override == 'itunes': - from core.itunes_client import iTunesClient - cl = iTunesClient() + cl = _get_itunes_client() elif source_override == 'hydrabase': plugin = request.args.get('plugin', '').lower() if plugin == 'deezer': cl = _get_deezer_client() else: - from core.itunes_client import iTunesClient - cl = iTunesClient() + cl = _get_itunes_client() elif source_override == 'deezer': cl = _get_deezer_client() elif source_override == 'discogs': - from core.discogs_client import DiscogsClient - cl = DiscogsClient() + cl = _get_discogs_client() elif source_override == 'spotify' and spotify_available: cl = spotify_client else: @@ -10663,20 +10647,17 @@ def get_artist_album_tracks(artist_id, album_id): source_override = request.args.get('source', '') client = spotify_client if source_override == 'itunes': - from core.itunes_client import iTunesClient - client = iTunesClient() + client = _get_itunes_client() elif source_override == 'hydrabase': plugin = request.args.get('plugin', '').lower() if plugin == 'deezer': client = _get_deezer_client() elif plugin == 'itunes' or album_id.isdigit(): - from core.itunes_client import iTunesClient - client = iTunesClient() + client = _get_itunes_client() elif source_override == 'deezer': client = _get_deezer_client() elif source_override == 'discogs': - from core.discogs_client import DiscogsClient - client = DiscogsClient() + client = _get_discogs_client() print(f"🎵 Fetching tracks for album: {album_id} by artist: {artist_id} (source: {source_override or 'auto'})") @@ -10779,8 +10760,7 @@ def download_discography(artist_id): else: fallback_src = _get_metadata_fallback_source() if fallback_src == 'itunes': - from core.itunes_client import iTunesClient - client = iTunesClient() + client = _get_itunes_client() elif fallback_src == 'deezer': client = _get_deezer_client() @@ -14112,8 +14092,7 @@ def redownload_search_metadata(track_id): if spotify_client and spotify_client.is_authenticated(): sources_to_search.append(('spotify', spotify_client)) try: - from core.itunes_client import iTunesClient - sources_to_search.append(('itunes', iTunesClient())) + sources_to_search.append(('itunes', _get_itunes_client())) except Exception as e: logger.debug(f"iTunes client not available for redownload search: {e}") try: @@ -14360,8 +14339,7 @@ def redownload_start(track_id): if full_track_details and full_track_details.get('album', {}).get('id'): full_album_data = spotify_client.get_album(full_track_details['album']['id']) elif meta_source == 'itunes': - from core.itunes_client import iTunesClient - _it = iTunesClient() + _it = _get_itunes_client() results = _it._lookup(id=meta_id, entity='song') if results: for r in results: @@ -17517,8 +17495,7 @@ def _apply_path_template(template: str, context: dict) -> str: itunes_artist_id = context.get('_itunes_artist_id') if itunes_artist_id and (',' in album_artist_value or ' & ' in album_artist_value): try: - from core.itunes_client import iTunesClient - resolved = iTunesClient().resolve_primary_artist(itunes_artist_id) + resolved = _get_itunes_client().resolve_primary_artist(itunes_artist_id) if resolved and resolved != album_artist_value: album_artist_value = resolved except Exception: @@ -18023,8 +18000,7 @@ def _extract_spotify_metadata(context: dict, artist: dict, album_info: dict) -> _src = original_search.get('_source') or _track_info_ctx.get('_source', '') if _aid.isdigit() and _src != 'deezer': try: - from core.itunes_client import iTunesClient - resolved = iTunesClient().resolve_primary_artist(_aid) + resolved = _get_itunes_client().resolve_primary_artist(_aid) if resolved and resolved != _raw_album_artist: _raw_album_artist = resolved except Exception: @@ -31177,23 +31153,20 @@ def get_album_tracks(album_id): # Use explicit source client when overridden (prevents numeric ID misrouting) client = spotify_client if source_override == 'itunes': - from core.itunes_client import iTunesClient - client = iTunesClient() + client = _get_itunes_client() elif source_override == 'hydrabase': # Hydrabase IDs originate from whichever plugin the peer runs. # 'plugin' param is authoritative; fall back to ID format detection. plugin = request.args.get('plugin', '').lower() if plugin == 'itunes' or (not plugin and album_id.isdigit()): - from core.itunes_client import iTunesClient - client = iTunesClient() + client = _get_itunes_client() elif plugin == 'deezer': client = _get_deezer_client() # else: spotify (default) elif source_override == 'deezer': client = _get_deezer_client() elif source_override == 'discogs': - from core.discogs_client import DiscogsClient - client = DiscogsClient() + client = _get_discogs_client() album_data = client.get_album(album_id) if not album_data: @@ -31608,8 +31581,7 @@ def get_discover_album(source, album_id): fallback_client = _get_deezer_client() fallback_source = 'deezer' else: - from core.itunes_client import iTunesClient - fallback_client = iTunesClient() + fallback_client = _get_itunes_client() fallback_source = 'itunes' album_data = fallback_client.get_album(album_id) @@ -33556,19 +33528,20 @@ def cancel_tidal_sync(playlist_id): deezer_discovery_states = {} # Key: playlist_id, Value: discovery state deezer_discovery_executor = ThreadPoolExecutor(max_workers=3, thread_name_prefix="deezer_discovery") -# Lazy-initialized global DeezerClient instance -_deezer_client_instance = None -_deezer_client_lock = threading.Lock() - def _get_deezer_client(): - """Get or create the global DeezerClient instance (thread-safe).""" - global _deezer_client_instance - if _deezer_client_instance is None: - with _deezer_client_lock: - if _deezer_client_instance is None: - from core.deezer_client import DeezerClient - _deezer_client_instance = DeezerClient() - return _deezer_client_instance + """Get cached Deezer client.""" + from core.metadata_service import get_deezer_client + return get_deezer_client() + +def _get_itunes_client(): + """Get cached iTunes client.""" + from core.metadata_service import get_itunes_client + return get_itunes_client() + +def _get_discogs_client(token=None): + """Get cached Discogs client.""" + from core.metadata_service import get_discogs_client + return get_discogs_client(token) def _get_metadata_fallback_source(): """Get the configured primary metadata source. @@ -33593,17 +33566,13 @@ def _get_metadata_fallback_client(): if source == 'discogs': token = config_manager.get('discogs.token', '') if token: - from core.discogs_client import DiscogsClient - return DiscogsClient(token=token) - from core.itunes_client import iTunesClient - return iTunesClient() + return _get_discogs_client(token) + return _get_itunes_client() if source == 'hydrabase': if hydrabase_client and hydrabase_client.is_connected(): return hydrabase_client - from core.itunes_client import iTunesClient - return iTunesClient() - from core.itunes_client import iTunesClient - return iTunesClient() + return _get_itunes_client() + return _get_itunes_client() @app.route('/api/deezer/arl-status', methods=['GET']) def get_deezer_arl_status(): @@ -38401,8 +38370,7 @@ def add_to_watchlist(): try: if source == 'discogs': # Discogs: fetch artist image from API - from core.discogs_client import DiscogsClient - dc = DiscogsClient() + dc = _get_discogs_client() dc_data = dc.get_artist(artist_id) if dc_data: image_url = dc_data.get('image_url') @@ -42150,17 +42118,15 @@ def _match_liked_artists_to_all_sources(database, profile_id: int): if spotify_client and spotify_client.is_spotify_authenticated(): search_clients['spotify'] = spotify_client try: - from core.itunes_client import iTunesClient - search_clients['itunes'] = iTunesClient() + search_clients['itunes'] = _get_itunes_client() except Exception: pass try: - search_clients['deezer'] = DeezerClient() + search_clients['deezer'] = _get_deezer_client() except Exception: pass try: - from core.discogs_client import DiscogsClient - dc = DiscogsClient() + dc = _get_discogs_client() # Only use Discogs if token is configured from config.settings import config_manager as _cm if _cm.get('discogs.token', ''): @@ -43173,8 +43139,7 @@ def get_artist_map_explore(): center_genres = sa.genres if hasattr(sa, 'genres') else [] artist_found = True if not artist_found: - from core.itunes_client import iTunesClient - ic = iTunesClient() + ic = _get_itunes_client() results = ic.search_artists(artist_name, limit=1) if results and len(results) > 0: ia = results[0]