From 097b853220f00580aacc249644d3e496f5b319bb Mon Sep 17 00:00:00 2001 From: Fabio Manganiello Date: Thu, 12 Dec 2019 20:08:02 +0100 Subject: [PATCH 1/3] Support for playlist management (creation and tracks add/rm/swap). See #22 (https://github.com/mopidy/mopidy-spotify/issues/22) --- mopidy_spotify/playlists.py | 113 +++++++++++++++++++++++++++++++++++- mopidy_spotify/web.py | 19 +++++- 2 files changed, 126 insertions(+), 6 deletions(-) diff --git a/mopidy_spotify/playlists.py b/mopidy_spotify/playlists.py index 305920ea..bdbba1e8 100644 --- a/mopidy_spotify/playlists.py +++ b/mopidy_spotify/playlists.py @@ -52,6 +52,33 @@ def _get_playlist(self, uri, as_items=False): as_items, ) + def _playlist_edit(self, playlist, method, **kwargs): + user_id = playlist.uri.split(':')[-3] + playlist_id = playlist.uri.split(':')[-1] + url = f'users/{user_id}/playlists/{playlist_id}/tracks' + method = getattr(self._backend._web_client, method.lower()) + if not method: + self.logger.error(f'Invalid HTTP method "{method}"') + return playlist + + logger.debug(f'API request: {method} {url}') + response = method( + url, headers={'Content-Type': 'application/json'}, json=kwargs) + + logger.debug(f'API response: {response}') + + if response and 'error' not in response: + # TODO invalidating the whole cache is probably a bit much if we have + # updated only one playlist - maybe we should expose an API to clear + # cache items by key? + self._backend._web_client.clear_cache() + return self.lookup(playlist.uri) + else: + logging.error('Error on playlist item(s) removal: {}'.format( + response['error'] if response else '(Unknown error)')) + + return playlist + def refresh(self): if not self._backend._web_client.logged_in: return @@ -68,13 +95,93 @@ def refresh(self): self._loaded = True def create(self, name): - pass # TODO + logger.info(f'Creating playlist {name}') + url = f'users/{user_id}/playlists' + response = self._backend._web_client.post( + url, headers={'Content-Type': 'application/json'}) + + return self.lookup(response['uri']) def delete(self, uri): - pass # TODO + # Playlist deletion is not implemented in the web API, see + # https://github.com/spotify/web-api/issues/555 + pass def save(self, playlist): - pass # TODO + # Note that for sake of simplicity the diff calculation between the + # old and new playlist won't take duplicate items into account + # (i.e. tracks that occur multiple times in the same playlist) + saved_playlist = self.lookup(playlist.uri) + if not saved_playlist: + return + + new_tracks = {track.uri: track for track in playlist.tracks} + cur_tracks = {track.uri: track for track in saved_playlist.tracks} + removed_uris = set([track.uri + for track in saved_playlist.tracks + if track.uri not in new_tracks]) + + # Remove tracks logic + if removed_uris: + logger.info('Removing {} tracks from playlist {}: {}'.format( + len(removed_uris), playlist.name, removed_uris)) + + cur_tracks = { + track.uri: track + for track in self._playlist_edit( + playlist, method='delete', + tracks=[{'uri': uri for uri in removed_uris}]).tracks + } + + # Add tracks logic + position = None + added_uris = {} + + for i, track in enumerate(playlist.tracks): + if track.uri not in cur_tracks: + if position is None: + position = i + added_uris[position] = [] + added_uris[position].append(track.uri) + else: + position = None + + if added_uris: + for pos, uris in added_uris.items(): + logger.info(f'Adding {uris} to playlist {playlist.name}') + + cur_tracks = { + track.uri: track + for track in self._playlist_edit( + playlist, method='post', + uris=uris, position=pos).tracks + } + + # Swap tracks logic + cur_tracks_by_uri = {} + + for i, track in enumerate(playlist.tracks): + if i >= len(saved_playlist.tracks): + break + + if track.uri != saved_playlist.tracks[i].uri: + cur_tracks_by_uri[saved_playlist.tracks[i].uri] = i + + if track.uri in cur_tracks_by_uri: + cur_pos = cur_tracks_by_uri[track.uri] + new_pos = i+1 + logger.info('Moving item position [{}] to [{}] in playlist {}'. + format(cur_pos, new_pos, playlist.name)) + + cur_tracks = { + track.uri: track + for track in self._playlist_edit( + playlist, method='put', + range_start=cur_pos, insert_before=new_pos).tracks + } + + self._backend._web_client.clear_cache() + return self.lookup(saved_playlist.uri) def playlist_lookup(session, web_client, uri, bitrate, as_items=False): diff --git a/mopidy_spotify/web.py b/mopidy_spotify/web.py index 0bdb64e1..96546509 100644 --- a/mopidy_spotify/web.py +++ b/mopidy_spotify/web.py @@ -65,7 +65,7 @@ def __init__( self._headers = {"Content-Type": "application/json"} self._session = utils.get_requests_session(proxy_config or {}) - def get(self, path, cache=None, *args, **kwargs): + def request(self, method, path, *args, cache=None, **kwargs): if self._authorization_failed: logger.debug("Blocking request as previous authorization failed.") return {} @@ -82,7 +82,6 @@ def get(self, path, cache=None, *args, **kwargs): return cached_result kwargs.setdefault("headers", {}).update(cached_result.etag_headers) - # TODO: Factor this out once we add more methods. # TODO: Don't silently error out. try: if self._should_refresh_token(): @@ -93,9 +92,11 @@ def get(self, path, cache=None, *args, **kwargs): # Make sure our headers always override user supplied ones. kwargs.setdefault("headers", {}).update(self._headers) - result = self._request_with_retries("GET", path, *args, **kwargs) + result = self._request_with_retries(method.upper(), path, *args, **kwargs) if result is None or "error" in result: + logger.error('Spotify web API call failed: {} {}: {}'.format( + method.upper(), path, result)) return {} if self._should_cache_response(cache, result): @@ -106,6 +107,18 @@ def get(self, path, cache=None, *args, **kwargs): return result + def get(self, path, cache=None, *args, **kwargs): + return self.request('GET', path, cache, *args, **kwargs) + + def post(self, path, *args, **kwargs): + return self.request('POST', path, cache=None, *args, **kwargs) + + def put(self, path, *args, **kwargs): + return self.request('PUT', path, cache=None, *args, **kwargs) + + def delete(self, path, *args, **kwargs): + return self.request('DELETE', path, cache=None, *args, **kwargs) + def _should_cache_response(self, cache, response): return cache is not None and response.status_ok From 9f0cab3b9dc1806f768f255bdea70c8841fc1349 Mon Sep 17 00:00:00 2001 From: Fabio Manganiello Date: Sat, 14 Dec 2019 00:40:50 +0100 Subject: [PATCH 2/3] Playlist changes support: addressed comments in #236 --- mopidy_spotify/playlists.py | 56 ++++++++++++++++++------------------- mopidy_spotify/web.py | 3 +- 2 files changed, 29 insertions(+), 30 deletions(-) diff --git a/mopidy_spotify/playlists.py b/mopidy_spotify/playlists.py index bdbba1e8..961fa0a6 100644 --- a/mopidy_spotify/playlists.py +++ b/mopidy_spotify/playlists.py @@ -52,32 +52,29 @@ def _get_playlist(self, uri, as_items=False): as_items, ) + @staticmethod + def _get_user_and_playlist_id_from_uri(uri): + user_id = uri.split(':')[-3] + playlist_id = uri.split(':')[-1] + return user_id, playlist_id + def _playlist_edit(self, playlist, method, **kwargs): - user_id = playlist.uri.split(':')[-3] - playlist_id = playlist.uri.split(':')[-1] + user_id, playlist_id = self._get_user_and_playlist_id_from_uri(playlist.uri) url = f'users/{user_id}/playlists/{playlist_id}/tracks' method = getattr(self._backend._web_client, method.lower()) if not method: - self.logger.error(f'Invalid HTTP method "{method}"') - return playlist + raise AttributeError(f'Invalid HTTP method "{method}"') logger.debug(f'API request: {method} {url}') - response = method( - url, headers={'Content-Type': 'application/json'}, json=kwargs) + response = method(url, json=kwargs) logger.debug(f'API response: {response}') - if response and 'error' not in response: - # TODO invalidating the whole cache is probably a bit much if we have - # updated only one playlist - maybe we should expose an API to clear - # cache items by key? - self._backend._web_client.clear_cache() - return self.lookup(playlist.uri) - else: - logging.error('Error on playlist item(s) removal: {}'.format( - response['error'] if response else '(Unknown error)')) - - return playlist + # TODO invalidating the whole cache is probably a bit much if we have + # updated only one playlist - maybe we should expose an API to clear + # cache items by key? + self._backend._web_client.clear_cache() + return self.lookup(playlist.uri) def refresh(self): if not self._backend._web_client.logged_in: @@ -96,10 +93,8 @@ def refresh(self): def create(self, name): logger.info(f'Creating playlist {name}') - url = f'users/{user_id}/playlists' - response = self._backend._web_client.post( - url, headers={'Content-Type': 'application/json'}) - + url = f'users/{web_client.user_id}/playlists' + response = self._backend._web_client.post(url) return self.lookup(response['uri']) def delete(self, uri): @@ -117,14 +112,12 @@ def save(self, playlist): new_tracks = {track.uri: track for track in playlist.tracks} cur_tracks = {track.uri: track for track in saved_playlist.tracks} - removed_uris = set([track.uri - for track in saved_playlist.tracks - if track.uri not in new_tracks]) + removed_uris = set(cur_tracks.keys()).difference(set(new_tracks.keys())) # Remove tracks logic if removed_uris: - logger.info('Removing {} tracks from playlist {}: {}'.format( - len(removed_uris), playlist.name, removed_uris)) + logger.info(f'Removing {len(removed_uris)} tracks from playlist ' + + f'{saved_playlist.name}: {removed_uris}') cur_tracks = { track.uri: track @@ -170,8 +163,8 @@ def save(self, playlist): if track.uri in cur_tracks_by_uri: cur_pos = cur_tracks_by_uri[track.uri] new_pos = i+1 - logger.info('Moving item position [{}] to [{}] in playlist {}'. - format(cur_pos, new_pos, playlist.name)) + logger.info(f'Moving item position [{cur_pos}] to [{new_pos}] in ' + + f'playlist {playlist.name}') cur_tracks = { track.uri: track @@ -180,6 +173,13 @@ def save(self, playlist): range_start=cur_pos, insert_before=new_pos).tracks } + # Playlist rename logic + if playlist.name != saved_playlist.name: + logger.info(f'Renaming playlist [{saved_playlist.name}] to [{playlist.name}]') + user_id, playlist_id = self._get_user_and_playlist_id_from_uri(saved_playlist.uri) + self._backend._web_client.put(f'users/{user_id}/playlists/{playlist_id}', + json={'name': playlist.name}) + self._backend._web_client.clear_cache() return self.lookup(saved_playlist.uri) diff --git a/mopidy_spotify/web.py b/mopidy_spotify/web.py index 96546509..ef96782e 100644 --- a/mopidy_spotify/web.py +++ b/mopidy_spotify/web.py @@ -95,8 +95,7 @@ def request(self, method, path, *args, cache=None, **kwargs): result = self._request_with_retries(method.upper(), path, *args, **kwargs) if result is None or "error" in result: - logger.error('Spotify web API call failed: {} {}: {}'.format( - method.upper(), path, result)) + logger.error(f'Spotify web API call failed: {method.upper()} {path}: {result}') return {} if self._should_cache_response(cache, result): From 9185dc6745e25b67a8f241f25e31679b7266f98a Mon Sep 17 00:00:00 2001 From: Fabio Manganiello Date: Sat, 14 Dec 2019 14:42:59 +0100 Subject: [PATCH 3/3] Added support for paging playlist edit calls It also addresses the comments in #236 --- mopidy_spotify/playlists.py | 40 ++++++++++++++++++++++--------------- 1 file changed, 24 insertions(+), 16 deletions(-) diff --git a/mopidy_spotify/playlists.py b/mopidy_spotify/playlists.py index 961fa0a6..cf501b72 100644 --- a/mopidy_spotify/playlists.py +++ b/mopidy_spotify/playlists.py @@ -11,6 +11,9 @@ class SpotifyPlaylistsProvider(backend.PlaylistsProvider): + # Maximum number of items accepted by the Spotify Web API + _chunk_size = 100 + def __init__(self, backend): self._backend = backend self._timeout = self._backend._config["spotify"]["timeout"] @@ -58,6 +61,11 @@ def _get_user_and_playlist_id_from_uri(uri): playlist_id = uri.split(':')[-1] return user_id, playlist_id + @staticmethod + def partitions(lst, n): + for i in range(0, len(lst), n): + yield lst[i:i+n] + def _playlist_edit(self, playlist, method, **kwargs): user_id, playlist_id = self._get_user_and_playlist_id_from_uri(playlist.uri) url = f'users/{user_id}/playlists/{playlist_id}/tracks' @@ -94,7 +102,8 @@ def refresh(self): def create(self, name): logger.info(f'Creating playlist {name}') url = f'users/{web_client.user_id}/playlists' - response = self._backend._web_client.post(url) + response = self._backend._web_client.post(url, json={'name': name}) + self.refresh() return self.lookup(response['uri']) def delete(self, uri): @@ -119,12 +128,10 @@ def save(self, playlist): logger.info(f'Removing {len(removed_uris)} tracks from playlist ' + f'{saved_playlist.name}: {removed_uris}') - cur_tracks = { - track.uri: track - for track in self._playlist_edit( - playlist, method='delete', - tracks=[{'uri': uri for uri in removed_uris}]).tracks - } + for chunk in self.partitions(removed_uris, self._chunk_size): + saved_playlist = self._playlist_edit(saved_playlist, method='delete', + tracks=[{'uri': uri for uri in removed_uris}]) + cur_tracks = {track.uri: track for track in saved_playlist.tracks} # Add tracks logic position = None @@ -141,14 +148,15 @@ def save(self, playlist): if added_uris: for pos, uris in added_uris.items(): - logger.info(f'Adding {uris} to playlist {playlist.name}') + logger.info(f'Adding {uris} to playlist {saved_playlist.name}') + processed_tracks = 0 + + for chunk in self.partitions(uris): + saved_playlist = self._playlist_edit(saved_playlist, method='post', + uris=chunk, position=pos+processed_tracks) - cur_tracks = { - track.uri: track - for track in self._playlist_edit( - playlist, method='post', - uris=uris, position=pos).tracks - } + cur_tracks = {track.uri: track for track in saved_playlist.tracks} + processed_tracks += len(chunk) # Swap tracks logic cur_tracks_by_uri = {} @@ -164,12 +172,12 @@ def save(self, playlist): cur_pos = cur_tracks_by_uri[track.uri] new_pos = i+1 logger.info(f'Moving item position [{cur_pos}] to [{new_pos}] in ' + - f'playlist {playlist.name}') + f'playlist {saved_playlist.name}') cur_tracks = { track.uri: track for track in self._playlist_edit( - playlist, method='put', + saved_playlist, method='put', range_start=cur_pos, insert_before=new_pos).tracks }