spotify: playlist-write client + single-source OAuth scope (#945, increment 2)
For exporting a mirrored playlist back to Spotify:
- The OAuth scope string was duplicated verbatim in 5 places (spotify_client, the per-profile
registry, and 3 web_server callbacks) — a drift hazard where the authorize URL and token
exchange could request different scopes and silently re-prompt/deny. Extracted ONE
SPOTIFY_OAUTH_SCOPE constant and pointed all 5 at it, and added playlist-modify-public/private
there. Existing users re-auth once to grant write; reads are unaffected.
- SpotifyClient.create_or_update_playlist(name, track_ids, existing_id=None): creates a playlist
owned by the authed user, or replaces an existing one's tracks in place (idempotent re-export).
Chunks at Spotify's 100-track cap. A pre-scope token gets a clear "reconnect Spotify" message
instead of a raw 403. Returns {success, playlist_id, url, added, error}.
6 tests: create-new adds tracks, update replaces (no create), >100 chunking, empty → error (no API
calls), not-authed → error, insufficient-scope → reconnect message. 268 spotify/oauth tests green,
ruff clean. Additive — read paths and existing tokens unchanged.
Next: Deezer write via the ARL gw-light gateway, then the export-job branch + endpoint + modal.
This commit is contained in:
parent
ede62824ad
commit
37c2b9b569
4 changed files with 146 additions and 8 deletions
|
|
@ -230,7 +230,7 @@ def get_spotify_client_for_profile(profile_id: Optional[int] = None):
|
|||
return client
|
||||
|
||||
try:
|
||||
from core.spotify_client import SpotifyClient, normalize_spotify_oauth_config
|
||||
from core.spotify_client import SpotifyClient, normalize_spotify_oauth_config, SPOTIFY_OAUTH_SCOPE
|
||||
from spotipy.oauth2 import SpotifyOAuth
|
||||
import spotipy
|
||||
|
||||
|
|
@ -243,7 +243,7 @@ def get_spotify_client_for_profile(profile_id: Optional[int] = None):
|
|||
client_id=normalized_creds.get("client_id", client_id),
|
||||
client_secret=normalized_creds.get("client_secret", client_secret),
|
||||
redirect_uri=normalized_creds.get("redirect_uri", redirect_uri),
|
||||
scope="user-library-read user-read-private playlist-read-private playlist-read-collaborative user-read-email user-follow-read",
|
||||
scope=SPOTIFY_OAUTH_SCOPE,
|
||||
cache_path=cache_path,
|
||||
state=f"profile_{profile_id}",
|
||||
)
|
||||
|
|
|
|||
|
|
@ -14,6 +14,18 @@ from core.metadata.cache import get_metadata_cache
|
|||
logger = get_logger("spotify_client")
|
||||
|
||||
|
||||
# Single source of truth for the Spotify OAuth scope. Used by EVERY SpotifyOAuth
|
||||
# construction (the client, the per-profile registry, and all web_server callbacks) so
|
||||
# the authorize URL and token exchange can never request different scopes — a mismatch
|
||||
# silently re-prompts or denies. The `playlist-modify-*` pair powers exporting a mirrored
|
||||
# playlist back to Spotify (#945); existing users re-auth once to grant it.
|
||||
SPOTIFY_OAUTH_SCOPE = (
|
||||
"user-library-read user-read-private playlist-read-private "
|
||||
"playlist-read-collaborative user-read-email user-follow-read "
|
||||
"playlist-modify-public playlist-modify-private"
|
||||
)
|
||||
|
||||
|
||||
def normalize_spotify_oauth_config(config: Optional[Dict[str, Any]]) -> Dict[str, Any]:
|
||||
"""Normalize Spotify OAuth config before building an auth manager.
|
||||
|
||||
|
|
@ -744,7 +756,7 @@ class SpotifyClient:
|
|||
client_id=config['client_id'],
|
||||
client_secret=config['client_secret'],
|
||||
redirect_uri=config.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",
|
||||
scope=SPOTIFY_OAUTH_SCOPE,
|
||||
cache_handler=DatabaseTokenCache(config_manager)
|
||||
)
|
||||
|
||||
|
|
@ -1136,6 +1148,56 @@ class SpotifyClient:
|
|||
return playlists
|
||||
return []
|
||||
|
||||
def create_or_update_playlist(self, name, track_ids, *, existing_id=None,
|
||||
public=False, description=""):
|
||||
"""Create a Spotify playlist owned by the authed user (or replace an existing
|
||||
one's tracks in place), for exporting a mirrored playlist back to Spotify (#945).
|
||||
|
||||
``track_ids`` are Spotify track IDs (the stored ``spotify_track_id`` per library
|
||||
track). ``existing_id`` set → replace that playlist's contents (idempotent
|
||||
re-export); unset → create a new playlist. Requires the ``playlist-modify-*``
|
||||
scope — a token issued before that scope was added gets a clear "reconnect"
|
||||
error rather than a raw 403. Returns
|
||||
``{success, playlist_id, url, added, error}``.
|
||||
"""
|
||||
if not self.is_spotify_authenticated():
|
||||
return {"success": False, "error": "Spotify is not connected"}
|
||||
uris = [f"spotify:track:{t}" for t in (track_ids or []) if t]
|
||||
if not uris:
|
||||
return {"success": False, "error": "No matching Spotify tracks to export"}
|
||||
try:
|
||||
playlist_id = existing_id
|
||||
if playlist_id:
|
||||
# Replace contents (re-export). replace_items caps at 100 — seed with the
|
||||
# first 100, then append the rest in 100-track chunks.
|
||||
self.sp.playlist_replace_items(playlist_id, uris[:100])
|
||||
for i in range(100, len(uris), 100):
|
||||
self.sp.playlist_add_items(playlist_id, uris[i:i + 100])
|
||||
else:
|
||||
user_id = (self.sp.current_user() or {}).get("id")
|
||||
created = self.sp.user_playlist_create(
|
||||
user_id, name, public=public, description=description,
|
||||
)
|
||||
playlist_id = (created or {}).get("id")
|
||||
if not playlist_id:
|
||||
return {"success": False, "error": "Spotify did not return a playlist id"}
|
||||
for i in range(0, len(uris), 100):
|
||||
self.sp.playlist_add_items(playlist_id, uris[i:i + 100])
|
||||
return {
|
||||
"success": True,
|
||||
"playlist_id": playlist_id,
|
||||
"url": f"https://open.spotify.com/playlist/{playlist_id}",
|
||||
"added": len(uris),
|
||||
}
|
||||
except Exception as e:
|
||||
msg = str(e)
|
||||
if "403" in msg or "scope" in msg.lower() or "insufficient" in msg.lower():
|
||||
return {"success": False,
|
||||
"error": "Reconnect Spotify to grant playlist write access "
|
||||
"(Settings → reconnect Spotify)."}
|
||||
_detect_and_set_rate_limit(e, "create_or_update_playlist")
|
||||
return {"success": False, "error": msg}
|
||||
|
||||
@rate_limited
|
||||
def get_saved_tracks_count(self) -> int:
|
||||
"""Get the total count of user's saved/liked songs without fetching all tracks"""
|
||||
|
|
|
|||
|
|
@ -87,4 +87,80 @@ def test_no_input():
|
|||
"redirect_uri": ""
|
||||
}
|
||||
assert normalize_spotify_oauth_config(None) == {}
|
||||
assert normalize_spotify_oauth_config(config) == expected
|
||||
assert normalize_spotify_oauth_config(config) == expected
|
||||
|
||||
# ── create_or_update_playlist: export a mirrored playlist back to Spotify (#945) ──
|
||||
|
||||
from core.spotify_client import SpotifyClient as _SpotifyClient
|
||||
|
||||
|
||||
class _FakeSp:
|
||||
def __init__(self):
|
||||
self.calls = []
|
||||
|
||||
def current_user(self):
|
||||
self.calls.append(('current_user',))
|
||||
return {'id': 'user-1'}
|
||||
|
||||
def user_playlist_create(self, user_id, name, public=False, description=''):
|
||||
self.calls.append(('create', user_id, name, public))
|
||||
return {'id': 'pl-new'}
|
||||
|
||||
def playlist_add_items(self, pid, uris):
|
||||
self.calls.append(('add', pid, list(uris)))
|
||||
|
||||
def playlist_replace_items(self, pid, uris):
|
||||
self.calls.append(('replace', pid, list(uris)))
|
||||
|
||||
|
||||
def _spotify_with(sp, authed=True):
|
||||
c = _SpotifyClient.__new__(_SpotifyClient)
|
||||
c.sp = sp
|
||||
c.is_spotify_authenticated = lambda: authed
|
||||
return c
|
||||
|
||||
|
||||
def test_create_new_playlist_adds_tracks():
|
||||
sp = _FakeSp()
|
||||
res = _spotify_with(sp).create_or_update_playlist('My Mix', ['a', 'b', 'c'])
|
||||
assert res['success'] and res['playlist_id'] == 'pl-new'
|
||||
assert res['url'] == 'https://open.spotify.com/playlist/pl-new'
|
||||
assert res['added'] == 3
|
||||
assert ('create', 'user-1', 'My Mix', False) in sp.calls
|
||||
assert ('add', 'pl-new', ['spotify:track:a', 'spotify:track:b', 'spotify:track:c']) in sp.calls
|
||||
|
||||
|
||||
def test_update_existing_replaces_no_create():
|
||||
sp = _FakeSp()
|
||||
res = _spotify_with(sp).create_or_update_playlist('My Mix', ['a', 'b'], existing_id='pl-x')
|
||||
assert res['success'] and res['playlist_id'] == 'pl-x'
|
||||
assert ('replace', 'pl-x', ['spotify:track:a', 'spotify:track:b']) in sp.calls
|
||||
assert not any(c[0] == 'create' for c in sp.calls)
|
||||
|
||||
|
||||
def test_chunks_over_100_tracks():
|
||||
sp = _FakeSp()
|
||||
res = _spotify_with(sp).create_or_update_playlist('Big', [str(i) for i in range(250)])
|
||||
assert res['added'] == 250
|
||||
adds = [c for c in sp.calls if c[0] == 'add']
|
||||
assert len(adds) == 3 and len(adds[0][2]) == 100 and len(adds[2][2]) == 50
|
||||
|
||||
|
||||
def test_empty_tracks_errors_no_api_calls():
|
||||
sp = _FakeSp()
|
||||
res = _spotify_with(sp).create_or_update_playlist('X', [])
|
||||
assert not res['success'] and 'No matching' in res['error']
|
||||
assert sp.calls == []
|
||||
|
||||
|
||||
def test_not_authed_errors():
|
||||
res = _spotify_with(_FakeSp(), authed=False).create_or_update_playlist('X', ['a'])
|
||||
assert not res['success'] and 'not connected' in res['error']
|
||||
|
||||
|
||||
def test_insufficient_scope_says_reconnect():
|
||||
class _ScopeErr(_FakeSp):
|
||||
def user_playlist_create(self, *a, **k):
|
||||
raise Exception('403 Forbidden: insufficient client scope')
|
||||
res = _spotify_with(_ScopeErr()).create_or_update_playlist('X', ['a'])
|
||||
assert not res['success'] and 'Reconnect Spotify' in res['error']
|
||||
|
|
|
|||
|
|
@ -82,7 +82,7 @@ if not pp_logger.handlers:
|
|||
_pp_handler.setFormatter(_logging.Formatter("%(asctime)s - %(message)s", datefmt="%Y-%m-%d %H:%M:%S"))
|
||||
pp_logger.addHandler(_pp_handler)
|
||||
pp_logger.propagate = False
|
||||
from core.spotify_client import SpotifyClient, Playlist as SpotifyPlaylist, Track as SpotifyTrack, _is_globally_rate_limited as _spotify_rate_limited
|
||||
from core.spotify_client import SpotifyClient, Playlist as SpotifyPlaylist, Track as SpotifyTrack, _is_globally_rate_limited as _spotify_rate_limited, SPOTIFY_OAUTH_SCOPE
|
||||
from core.plex_client import PlexClient
|
||||
from core.ui_appearance import is_firefox_user_agent, resolve_worker_orbs_default
|
||||
from plexapi.myplex import MyPlexAccount, MyPlexPinLogin
|
||||
|
|
@ -4803,7 +4803,7 @@ def _profile_spotify_oauth(profile_id_int):
|
|||
client_id=client_id,
|
||||
client_secret=client_secret,
|
||||
redirect_uri=redirect_uri,
|
||||
scope="user-library-read user-read-private playlist-read-private playlist-read-collaborative user-read-email user-follow-read",
|
||||
scope=SPOTIFY_OAUTH_SCOPE,
|
||||
cache_path=f'config/.spotify_cache_profile_{profile_id_int}',
|
||||
state=f'profile_{profile_id_int}',
|
||||
show_dialog=True,
|
||||
|
|
@ -5233,7 +5233,7 @@ def spotify_callback():
|
|||
client_id=config['client_id'],
|
||||
client_secret=config['client_secret'],
|
||||
redirect_uri=configured_uri,
|
||||
scope="user-library-read user-read-private playlist-read-private playlist-read-collaborative user-read-email user-follow-read",
|
||||
scope=SPOTIFY_OAUTH_SCOPE,
|
||||
cache_path='config/.spotify_cache'
|
||||
)
|
||||
|
||||
|
|
@ -35993,7 +35993,7 @@ def start_oauth_callback_servers():
|
|||
client_id=config['client_id'],
|
||||
client_secret=config['client_secret'],
|
||||
redirect_uri=configured_uri,
|
||||
scope="user-library-read user-read-private playlist-read-private playlist-read-collaborative user-read-email user-follow-read",
|
||||
scope=SPOTIFY_OAUTH_SCOPE,
|
||||
cache_path='config/.spotify_cache'
|
||||
)
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue