Profiles: per-profile Spotify builds for own-app creds OR a token cache
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.
This commit is contained in:
parent
e8bd9c8018
commit
7eaed1d533
2 changed files with 33 additions and 20 deletions
|
|
@ -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(
|
||||
|
|
|
|||
|
|
@ -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_<id>.
|
||||
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_<id>.
|
||||
|
||||
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
|
||||
|
|
|
|||
Loading…
Reference in a new issue