From 7eaed1d5332fd4e12ea3b31d0a45888a40481efd Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Wed, 10 Jun 2026 12:28:03 -0700 Subject: [PATCH] Profiles: per-profile Spotify builds for own-app creds OR a token cache MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review/regression fix: the shared-app gate I added wrongly required a token cache even for profiles that set their OWN app creds (legacy), breaking the per-profile caching tests. Now a per-profile client is built when the profile has its own app creds OR has connected (its own token cache) — otherwise it falls back to the global/admin client. Made the fallback-safety tests order-independent (monkeypatch get_spotify_client to a sentinel) since the real global client isn't a stable singleton across the suite. 10 metadata-cache + resolution tests pass together. --- core/metadata/registry.py | 18 ++++++++---- tests/test_profile_spotify_resolution.py | 35 ++++++++++++++---------- 2 files changed, 33 insertions(+), 20 deletions(-) diff --git a/core/metadata/registry.py b/core/metadata/registry.py index a5cc48d2..4e6f343d 100644 --- a/core/metadata/registry.py +++ b/core/metadata/registry.py @@ -203,17 +203,23 @@ def get_spotify_client_for_profile(profile_id: Optional[int] = None): except Exception: return get_spotify_client() + import os + cache_path = f"config/.spotify_cache_profile_{profile_id}" + own_app = bool(creds.get("client_id") and creds.get("client_secret")) + connected = os.path.exists(cache_path) + # Build a per-profile client when the profile has its OWN app creds (legacy) + # OR has connected via the shared app (its own token cache exists). A profile + # with neither uses the global/admin client — so background workers and + # unconnected users are unaffected. + if not own_app and not connected: + return get_spotify_client() + # Effective OAuth app creds: the profile's own (legacy) else the global app. client_id = creds.get("client_id") or _get_config_value("spotify.client_id", "") client_secret = creds.get("client_secret") or _get_config_value("spotify.client_secret", "") redirect_uri = (creds.get("redirect_uri") or _get_config_value("spotify.redirect_uri", "http://127.0.0.1:8888/callback")) - - import os - cache_path = f"config/.spotify_cache_profile_{profile_id}" - # Only use a per-profile client when this profile has actually connected - # (its own token cache exists). Otherwise use the global/admin client. - if not client_id or not os.path.exists(cache_path): + if not client_id: return get_spotify_client() cache_key = _build_profile_spotify_cache_key( diff --git a/tests/test_profile_spotify_resolution.py b/tests/test_profile_spotify_resolution.py index ad753b5c..825e3a72 100644 --- a/tests/test_profile_spotify_resolution.py +++ b/tests/test_profile_spotify_resolution.py @@ -1,9 +1,14 @@ """Per-profile Spotify client resolution falls back safely (shared-app model). The builder must return the GLOBAL client for admin (profile 1) and for any -non-admin profile that hasn't connected its own Spotify (no token cache) — so -background workers and existing users are unaffected. A per-profile client only -appears once that profile has its own .spotify_cache_profile_. +non-admin profile that hasn't connected its own Spotify (no token cache, no own +app creds) — so background workers and existing users are unaffected. A +per-profile client only appears once that profile has its own app creds OR its +own .spotify_cache_profile_. + +We monkeypatch get_spotify_client to a sentinel so "fell back to global" is an +exact, order-independent identity check (the real global client isn't a stable +singleton across the suite). """ from __future__ import annotations @@ -13,17 +18,19 @@ import os from core.metadata import registry -def test_admin_and_none_use_global_client(): - g = registry.get_spotify_client() - assert registry.get_spotify_client_for_profile(1) is g - assert registry.get_spotify_client_for_profile(None) is g +def test_admin_and_none_use_global_client(monkeypatch): + sentinel = object() + monkeypatch.setattr(registry, "get_spotify_client", lambda *a, **k: sentinel) + registry.register_profile_spotify_credentials_provider(lambda pid: None) + assert registry.get_spotify_client_for_profile(1) is sentinel + assert registry.get_spotify_client_for_profile(None) is sentinel -def test_unconnected_profile_falls_back_to_global(tmp_path, monkeypatch): - # A non-admin profile with no token cache must resolve to the global client. - g = registry.get_spotify_client() - # ensure no stray cache file for this id +def test_unconnected_profile_falls_back_to_global(monkeypatch): + sentinel = object() + monkeypatch.setattr(registry, "get_spotify_client", lambda *a, **k: sentinel) + # No own app creds for this profile, and no token cache → must use global. + registry.register_profile_spotify_credentials_provider(lambda pid: None) pid = 987654 - cache = f"config/.spotify_cache_profile_{pid}" - assert not os.path.exists(cache) - assert registry.get_spotify_client_for_profile(pid) is g + assert not os.path.exists(f"config/.spotify_cache_profile_{pid}") + assert registry.get_spotify_client_for_profile(pid) is sentinel