Move profile Spotify cache into registry
- let core.metadata.registry own per-profile Spotify client caching - register the DB-backed profile credentials provider from web_server.py - invalidate only the affected profile cache entry on save, delete, and auth
This commit is contained in:
parent
11be8834eb
commit
e6c2bee427
5 changed files with 181 additions and 53 deletions
|
|
@ -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",
|
||||
]
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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",
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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 "<h1>Spotify Authentication Successful!</h1><p>Your personal Spotify account is now connected. You can close this window.</p>"
|
||||
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:
|
||||
|
|
|
|||
Loading…
Reference in a new issue