diff --git a/core/metadata/__init__.py b/core/metadata/__init__.py index e98e7d92..ea434e48 100644 --- a/core/metadata/__init__.py +++ b/core/metadata/__init__.py @@ -24,6 +24,7 @@ from core.metadata.registry import ( METADATA_SOURCE_PRIORITY, clear_cached_metadata_client, clear_cached_metadata_clients, + clear_cached_profile_spotify_client, get_client_for_source, get_deezer_client, get_discogs_client, @@ -31,10 +32,12 @@ from core.metadata.registry import ( get_itunes_client, get_primary_client, get_primary_source, + get_spotify_client_for_profile, get_registered_runtime_client, get_source_priority, get_spotify_client, is_hydrabase_enabled, + register_profile_spotify_credentials_provider, register_runtime_clients, ) from core.metadata.service import MetadataProvider, MetadataService, get_metadata_service @@ -54,6 +57,7 @@ __all__ = [ "check_single_completion", "clear_cached_metadata_client", "clear_cached_metadata_clients", + "clear_cached_profile_spotify_client", "get_album_for_source", "get_album_tracks_for_source", "get_artist_album_tracks", @@ -71,12 +75,14 @@ __all__ = [ "get_musicmap_similar_artists", "get_primary_client", "get_primary_source", + "get_spotify_client_for_profile", "get_registered_runtime_client", "get_spotify_client", "get_source_priority", "iter_artist_discography_completion_events", "iter_musicmap_similar_artist_events", "is_hydrabase_enabled", + "register_profile_spotify_credentials_provider", "register_runtime_clients", "resolve_album_reference", ] diff --git a/core/metadata/registry.py b/core/metadata/registry.py index 210cea68..595c8549 100644 --- a/core/metadata/registry.py +++ b/core/metadata/registry.py @@ -8,6 +8,7 @@ instead of importing `web_server`. from __future__ import annotations import threading +import hashlib from typing import Any, Callable, Dict, Optional from utils.logging_config import get_logger @@ -28,6 +29,7 @@ _runtime_clients: Dict[str, Any] = { "hydrabase": None, } _dev_mode_enabled_provider: Callable[[], bool] = lambda: False +_profile_spotify_credentials_provider: Callable[[int], Any] = lambda profile_id: None def register_runtime_clients( @@ -51,6 +53,16 @@ def register_runtime_clients( _dev_mode_enabled_provider = dev_mode_enabled_provider or (lambda: False) +def register_profile_spotify_credentials_provider( + provider: Optional[Callable[[int], Any]] = _UNSET, +) -> None: + """Register a callable that returns per-profile Spotify credentials.""" + global _profile_spotify_credentials_provider + with _runtime_clients_lock: + if provider is not _UNSET: + _profile_spotify_credentials_provider = provider or (lambda profile_id: None) + + def get_registered_runtime_client(name: str) -> Any: with _runtime_clients_lock: return _runtime_clients.get(name) @@ -71,6 +83,14 @@ def clear_cached_metadata_client(cache_key: str) -> None: _client_cache.pop(cache_key, None) +def clear_cached_profile_spotify_client(profile_id: int) -> None: + """Clear any cached Spotify client for a specific profile.""" + prefix = f"spotify_profile::{profile_id}::" + with _client_cache_lock: + for key in [key for key in _client_cache if key.startswith(prefix)]: + _client_cache.pop(key, None) + + def _get_config_value(key: str, default: Any = None) -> Any: try: from config.settings import config_manager @@ -132,6 +152,61 @@ def get_spotify_client(client_factory: Optional[MetadataClientFactory] = None): return client +def _build_profile_spotify_cache_key(profile_id: int, creds: Dict[str, Any]) -> str: + fingerprint = hashlib.sha256( + f"{profile_id}:{creds.get('client_id', '')}:{creds.get('client_secret', '')}:{creds.get('redirect_uri', '')}".encode( + "utf-8" + ) + ).hexdigest() + return f"spotify_profile::{profile_id}::{fingerprint}" + + +def get_spotify_client_for_profile(profile_id: Optional[int] = None): + """Get a profile-specific Spotify client or fall back to the global one.""" + if profile_id is None or profile_id == 1: + return get_spotify_client() + + try: + creds = _profile_spotify_credentials_provider(profile_id) + if not creds or not creds.get("client_id"): + return get_spotify_client() + except Exception: + return get_spotify_client() + + cache_key = _build_profile_spotify_cache_key(profile_id, creds) + with _client_cache_lock: + client = _client_cache.get(cache_key) + if client is not None and getattr(client, "sp", None) is not None: + return client + + try: + from core.spotify_client import SpotifyClient + from spotipy.oauth2 import SpotifyOAuth + import spotipy + + auth_manager = SpotifyOAuth( + client_id=creds["client_id"], + client_secret=creds["client_secret"], + redirect_uri=creds.get("redirect_uri", "http://127.0.0.1:8888/callback"), + scope="user-library-read user-read-private playlist-read-private playlist-read-collaborative user-read-email user-follow-read", + cache_path=f"config/.spotify_cache_profile_{profile_id}", + state=f"profile_{profile_id}", + ) + + profile_client = SpotifyClient() + profile_client.sp = spotipy.Spotify(auth_manager=auth_manager, retries=0, requests_timeout=15) + profile_client.user_id = None + + with _client_cache_lock: + _client_cache[cache_key] = profile_client + + logger.info("Created per-profile Spotify client for profile %s", profile_id) + return profile_client + except Exception as e: + logger.error("Failed to create per-profile Spotify client for profile %s: %s", profile_id, e) + return get_spotify_client() + + def get_deezer_client(client_factory: Optional[MetadataClientFactory] = None): """Get cached Deezer client keyed by current access token.""" current_token = _get_config_value("deezer.access_token", None) diff --git a/core/metadata_service.py b/core/metadata_service.py index 71a5a057..5391a80b 100644 --- a/core/metadata_service.py +++ b/core/metadata_service.py @@ -41,6 +41,7 @@ from core.metadata.registry import ( METADATA_SOURCE_PRIORITY, clear_cached_metadata_client, clear_cached_metadata_clients, + clear_cached_profile_spotify_client, get_client_for_source, get_deezer_client, get_discogs_client, @@ -48,10 +49,12 @@ from core.metadata.registry import ( get_itunes_client, get_primary_client, get_primary_source, + get_spotify_client_for_profile, get_registered_runtime_client, get_source_priority, get_spotify_client, is_hydrabase_enabled, + register_profile_spotify_credentials_provider, register_runtime_clients, ) from core.metadata.service import MetadataProvider, MetadataService, get_metadata_service @@ -92,6 +95,7 @@ __all__ = [ "check_single_completion", "clear_cached_metadata_client", "clear_cached_metadata_clients", + "clear_cached_profile_spotify_client", "get_album_for_source", "get_album_tracks_for_source", "get_artist_album_tracks", @@ -109,12 +113,14 @@ __all__ = [ "get_musicmap_similar_artists", "get_primary_client", "get_primary_source", + "get_spotify_client_for_profile", "get_registered_runtime_client", "get_spotify_client", "get_source_priority", "iter_artist_discography_completion_events", "iter_musicmap_similar_artist_events", "is_hydrabase_enabled", + "register_profile_spotify_credentials_provider", "register_runtime_clients", "requests", "resolve_album_reference", diff --git a/tests/metadata/test_metadata_cache.py b/tests/metadata/test_metadata_cache.py index 5c08acb0..79bb457f 100644 --- a/tests/metadata/test_metadata_cache.py +++ b/tests/metadata/test_metadata_cache.py @@ -47,8 +47,10 @@ from config.settings import config_manager @pytest.fixture(autouse=True) def _clear_metadata_client_cache(): metadata_registry.clear_cached_metadata_clients() + metadata_registry.register_profile_spotify_credentials_provider(lambda profile_id: None) yield metadata_registry.clear_cached_metadata_clients() + metadata_registry.register_profile_spotify_credentials_provider(lambda profile_id: None) def test_primary_client_is_cached_for_same_source(monkeypatch): @@ -110,6 +112,90 @@ def test_deezer_client_cache_tracks_token(monkeypatch): assert calls["deezer"] == 2 +def test_profile_spotify_client_is_cached_per_profile(monkeypatch): + calls = {"spotify": 0, "oauth": 0} + creds_by_profile = { + 2: {"client_id": "cid-a", "client_secret": "sec-a", "redirect_uri": "uri-a"}, + 3: {"client_id": "cid-b", "client_secret": "sec-b", "redirect_uri": "uri-b"}, + } + + class FakeSpotifyClient: + def __init__(self): + calls["spotify"] += 1 + self.sp = None + self.user_id = None + + class FakeOAuth: + def __init__(self, *args, **kwargs): + calls["oauth"] += 1 + + class FakeSpotify: + def __init__(self, *args, **kwargs): + self.auth_manager = kwargs.get("auth_manager") + + monkeypatch.setattr("core.spotify_client.SpotifyClient", FakeSpotifyClient) + monkeypatch.setattr(sys.modules["spotipy"].oauth2, "SpotifyOAuth", FakeOAuth) + monkeypatch.setattr(sys.modules["spotipy"], "Spotify", FakeSpotify) + metadata_registry.register_profile_spotify_credentials_provider(lambda profile_id: creds_by_profile.get(profile_id)) + + first = metadata_registry.get_spotify_client_for_profile(2) + second = metadata_registry.get_spotify_client_for_profile(2) + third = metadata_registry.get_spotify_client_for_profile(3) + + assert first is second + assert first is not third + assert calls["spotify"] == 2 + assert calls["oauth"] == 2 + + +def test_clear_cached_profile_spotify_client_only_affects_one_profile(monkeypatch): + calls = {"spotify": 0} + creds_by_profile = { + 2: {"client_id": "cid-a", "client_secret": "sec-a", "redirect_uri": "uri-a"}, + 3: {"client_id": "cid-b", "client_secret": "sec-b", "redirect_uri": "uri-b"}, + } + + class FakeSpotifyClient: + def __init__(self): + calls["spotify"] += 1 + self.sp = None + self.user_id = None + + class FakeOAuth: + def __init__(self, *args, **kwargs): + pass + + class FakeSpotify: + def __init__(self, *args, **kwargs): + pass + + monkeypatch.setattr("core.spotify_client.SpotifyClient", FakeSpotifyClient) + monkeypatch.setattr(sys.modules["spotipy"].oauth2, "SpotifyOAuth", FakeOAuth) + monkeypatch.setattr(sys.modules["spotipy"], "Spotify", FakeSpotify) + metadata_registry.register_profile_spotify_credentials_provider(lambda profile_id: creds_by_profile.get(profile_id)) + + first_profile = metadata_registry.get_spotify_client_for_profile(2) + other_profile = metadata_registry.get_spotify_client_for_profile(3) + + metadata_registry.clear_cached_profile_spotify_client(2) + + refreshed_profile = metadata_registry.get_spotify_client_for_profile(2) + same_other_profile = metadata_registry.get_spotify_client_for_profile(3) + + assert refreshed_profile is not first_profile + assert same_other_profile is other_profile + assert calls["spotify"] == 3 + + +def test_profile_spotify_client_falls_back_to_global_when_no_credentials(monkeypatch): + global_client = object() + + monkeypatch.setattr(metadata_registry, "get_spotify_client", lambda client_factory=None: global_client) + metadata_registry.register_profile_spotify_credentials_provider(lambda profile_id: None) + + assert metadata_registry.get_spotify_client_for_profile(2) is global_client + + class _FakeHydrabaseClient: def __init__(self, connected=True): self._connected = connected diff --git a/web_server.py b/web_server.py index c0691ba1..0909768e 100644 --- a/web_server.py +++ b/web_server.py @@ -97,6 +97,7 @@ from core.matching_engine import MusicMatchingEngine from core.database_update_worker import DatabaseUpdateWorker from core.web_scan_manager import WebScanManager from core.metadata.cache import get_metadata_cache +from core.metadata import registry as metadata_registry from core.metadata.registry import ( clear_cached_metadata_client, get_spotify_client, @@ -484,10 +485,6 @@ def admin_only(view_fn): return view_fn(*args, **kwargs) return wrapper -# ── Per-profile Spotify client cache ── -_profile_spotify_clients = {} # profile_id -> SpotifyClient -_profile_spotify_lock = threading.Lock() - def get_spotify_client_for_profile(profile_id=None): """Get the Spotify client for the current profile. @@ -497,53 +494,7 @@ def get_spotify_client_for_profile(profile_id=None): """ if profile_id is None: profile_id = get_current_profile_id() - - # Admin (profile 1) always uses global client - if profile_id == 1: - return spotify_client - - # Check if this profile has custom Spotify credentials - try: - db = get_database() - creds = db.get_profile_spotify(profile_id) - if not creds or not creds.get('client_id'): - return spotify_client # No custom creds — use global - except Exception: - return spotify_client - - # Check cache (don't hold lock during auth check — could block on network) - with _profile_spotify_lock: - cached = _profile_spotify_clients.get(profile_id) - if cached and cached.sp is not None: - return cached - - # Create a new SpotifyClient for this profile - try: - from spotipy.oauth2 import SpotifyOAuth - import spotipy - - auth_manager = SpotifyOAuth( - client_id=creds['client_id'], - client_secret=creds['client_secret'], - redirect_uri=creds.get('redirect_uri', 'http://127.0.0.1:8888/callback'), - scope="user-library-read user-read-private playlist-read-private playlist-read-collaborative user-read-email user-follow-read", - cache_path=f'config/.spotify_cache_profile_{profile_id}' - ) - - # Create a bare SpotifyClient and immediately set the profile-specific - # spotipy instance (overwrites the global-config one from __init__) - profile_client = SpotifyClient() - profile_client.sp = spotipy.Spotify(auth_manager=auth_manager, retries=0, requests_timeout=15) - profile_client.user_id = None # Will be fetched lazily - - with _profile_spotify_lock: - _profile_spotify_clients[profile_id] = profile_client - - logger.info(f"Created per-profile Spotify client for profile {profile_id}") - return profile_client - except Exception as e: - logger.error(f"Failed to create per-profile Spotify client for profile {profile_id}: {e}") - return spotify_client # Fall back to global + return metadata_registry.get_spotify_client_for_profile(profile_id) # Valid page IDs for profile permission validation VALID_PAGE_IDS = {'dashboard', 'sync', 'search', 'downloads', 'discover', 'artists', 'automations', 'library', 'import', 'settings', 'help'} @@ -7619,8 +7570,7 @@ def spotify_callback(): token_info = auth_manager.get_access_token(auth_code) if token_info: # Invalidate cached profile client so it gets recreated with new tokens - with _profile_spotify_lock: - _profile_spotify_clients.pop(profile_id_from_state, None) + metadata_registry.clear_cached_profile_spotify_client(profile_id_from_state) add_activity_item("", "Spotify Auth Complete", f"Profile {profile_id_from_state} authenticated with Spotify", "Now") return "
Your personal Spotify account is now connected. You can close this window.
" else: @@ -28576,6 +28526,7 @@ def save_profile_spotify_creds(): success = db.set_profile_spotify(profile_id, client_id, client_secret, redirect_uri) if success: + metadata_registry.clear_cached_profile_spotify_client(profile_id) return jsonify({'success': True}) return jsonify({'success': False, 'error': 'Failed to save credentials'}), 500 except Exception as e: @@ -28597,6 +28548,7 @@ def delete_profile_spotify_creds(): WHERE id = ? """, (profile_id,)) conn.commit() + metadata_registry.clear_cached_profile_spotify_client(profile_id) return jsonify({'success': True}) except Exception as e: return jsonify({'success': False, 'error': str(e)}), 500 @@ -39705,6 +39657,9 @@ register_runtime_clients( hydrabase_client=hydrabase_client, dev_mode_enabled_provider=lambda: dev_mode_enabled, ) +metadata_registry.register_profile_spotify_credentials_provider( + lambda profile_id: get_database().get_profile_spotify(profile_id) +) # --- Hydrabase Auto-Reconnect --- try: