From: Marcel van der Veldt Date: Wed, 8 May 2024 22:31:40 +0000 (+0200) Subject: Fix several issues with playlists edits (#1283) X-Git-Url: https://git.kitaultman.com/?a=commitdiff_plain;h=b69180b33e5922ae195b6916e9b4a968c4216be1;p=music-assistant-server.git Fix several issues with playlists edits (#1283) --- diff --git a/music_assistant/server/controllers/media/base.py b/music_assistant/server/controllers/media/base.py index 6a020913..b744398a 100644 --- a/music_assistant/server/controllers/media/base.py +++ b/music_assistant/server/controllers/media/base.py @@ -514,8 +514,7 @@ class MediaControllerBase(Generic[ItemCls], metaclass=ABCMeta): if provider := self.mass.get_provider(provider_instance_id_or_domain): with suppress(MediaNotFoundError): if item := await provider.get_item(self.media_type, item_id): - if item.metadata.cache_checksum != "no_cache": - await self.mass.cache.set(cache_key, item.to_dict()) + await self.mass.cache.set(cache_key, item.to_dict()) return item # if we reach this point all possibilities failed and the item could not be found. # There is a possibility that the (streaming) provider changed the id of the item diff --git a/music_assistant/server/controllers/media/playlists.py b/music_assistant/server/controllers/media/playlists.py index bb7569e6..13977be4 100644 --- a/music_assistant/server/controllers/media/playlists.py +++ b/music_assistant/server/controllers/media/playlists.py @@ -55,6 +55,7 @@ class PlaylistController(MediaControllerBase[Playlist]): item_id, provider_instance_id_or_domain, force_refresh=force_refresh, + lazy=not force_refresh, ) prov = next(x for x in playlist.provider_mappings) tracks = await self._get_provider_playlist_tracks( @@ -102,7 +103,7 @@ class PlaylistController(MediaControllerBase[Playlist]): raise ProviderUnavailableError(msg) cur_playlist_track_ids = set() cur_playlist_track_uris = set() - async for item in self.tracks(playlist_prov_map.item_id, playlist_prov.instance_id): + for item in await self.get_all_playlist_tracks(playlist): cur_playlist_track_uris.add(item.item_id) cur_playlist_track_uris.add(item.uri) @@ -112,6 +113,9 @@ class PlaylistController(MediaControllerBase[Playlist]): for uri in uris: # skip if item already in the playlist if uri in cur_playlist_track_uris: + self.logger.info( + "Not adding %s to playlist %s - it already exists", uri, playlist.name + ) continue # parse uri for further processing @@ -119,11 +123,17 @@ class PlaylistController(MediaControllerBase[Playlist]): # skip if item already in the playlist if item_id in cur_playlist_track_ids: + self.logger.warning( + "Not adding %s to playlist %s - it already exists", uri, playlist.name + ) continue # skip non-track items # TODO: revisit this once we support audiobooks and podcasts ? if media_type != MediaType.TRACK: + self.logger.warning( + "Not adding %s to playlist %s - not a track", uri, playlist.name + ) continue # special: the builtin provider can handle uri's from all providers (with uri as id) @@ -131,57 +141,91 @@ class PlaylistController(MediaControllerBase[Playlist]): # note: we try not to add library uri's to the builtin playlists # so we can survive db rebuilds ids_to_add.add(uri) + self.logger.info( + "Adding %s to playlist %s", + uri, + playlist.name, + ) continue - # handle library uri (we need to fully unwrap it) - if provider_instance_id_or_domain == "library": - # library item, fetch full object - db_track = await self.mass.music.tracks.get_library_item(item_id) - # a track can contain multiple versions on the same provider - # simply sort by quality and just add the first available version - for track_version in sorted( - db_track.provider_mappings, key=lambda x: x.quality, reverse=True - ): - if not track_version.available: - continue - if track_version.item_id in cur_playlist_track_ids: - break # already existing in the playlist - item_prov = self.mass.get_provider(track_version.provider_instance) - if not item_prov: - continue - track_version_uri = create_uri( - MediaType.TRACK, - item_prov.lookup_key, - track_version.item_id, + # if target playlist is an exact provider match, we can add it + if provider_instance_id_or_domain != "library": + item_prov = self.mass.get_provider(provider_instance_id_or_domain) + if not item_prov or not item_prov.available: + self.logger.warning( + "Skip adding %s to playlist: Provider %s is not available", + uri, + provider_instance_id_or_domain, ) - if track_version_uri in cur_playlist_track_uris: - break # already existing in the playlist - if playlist_prov.domain == "builtin": - # the builtin provider can handle uri's from all providers (with uri as id) - ids_to_add.add(track_version_uri) - break - if item_prov.lookup_key == playlist_prov.lookup_key: - ids_to_add.add(track_version.item_id) - break - continue + continue + if item_prov.lookup_key == playlist_prov.lookup_key: + ids_to_add.add(item_id) + continue - # all other: if target playlist is an exact provider match, we can add it - item_prov = self.mass.get_provider(provider_instance_id_or_domain) - if not item_prov or not item_prov.available: + # ensure we have a full library track + db_track = await self.mass.music.tracks.get( + item_id, provider_instance_id_or_domain, lazy=False, add_to_library=True + ) + # a track can contain multiple versions on the same provider + # simply sort by quality and just add the first available version + for track_version in sorted( + db_track.provider_mappings, key=lambda x: x.quality, reverse=True + ): + if not track_version.available: + continue + if track_version.item_id in cur_playlist_track_ids: + break # already existing in the playlist + item_prov = self.mass.get_provider(track_version.provider_instance) + if not item_prov: + continue + track_version_uri = create_uri( + MediaType.TRACK, + item_prov.lookup_key, + track_version.item_id, + ) + if track_version_uri in cur_playlist_track_uris: + self.logger.warning( + "Not adding %s to playlist %s - it already exists", + db_track.name, + playlist.name, + ) + break # already existing in the playlist + if playlist_prov.domain == "builtin": + # the builtin provider can handle uri's from all providers (with uri as id) + ids_to_add.add(track_version_uri) + self.logger.info( + "Adding %s to playlist %s", + db_track.name, + playlist.name, + ) + break + if item_prov.lookup_key == playlist_prov.lookup_key: + ids_to_add.add(track_version.item_id) + self.logger.info( + "Adding %s to playlist %s", + db_track.name, + playlist.name, + ) + break + else: self.logger.warning( - "Skip adding uri %s to playlist: Provider %s is not available", - uri, - provider_instance_id_or_domain, + "Can't add %s to playlist %s - it is not available provider %s", + db_track.name, + playlist.name, + playlist_prov.name, ) - continue - if item_prov.lookup_key == playlist_prov.lookup_key: - ids_to_add.add(item_id) + + if not ids_to_add: + return # actually add the tracks to the playlist on the provider await playlist_prov.add_playlist_tracks(playlist_prov_map.item_id, list(ids_to_add)) # invalidate cache so tracks get refreshed - cache_key = f"{playlist_prov.lookup_key}.playlist.{playlist_prov_map.item_id}.tracks" - await self.mass.cache.delete(cache_key) + await self.get( + playlist.item_id, + playlist.provider, + force_refresh=True, + ) async def add_playlist_track(self, db_playlist_id: str | int, track_uri: str) -> None: """Add (single) track to playlist.""" @@ -209,8 +253,33 @@ class PlaylistController(MediaControllerBase[Playlist]): continue await provider.remove_playlist_tracks(prov_mapping.item_id, positions_to_remove) # invalidate cache so tracks get refreshed - cache_key = f"{provider.lookup_key}.playlist.{prov_mapping.item_id}.tracks" - await self.mass.cache.delete(cache_key) + await self.get( + playlist.item_id, + playlist.provider, + force_refresh=True, + ) + + async def get_all_playlist_tracks(self, playlist: Playlist) -> list[PlaylistTrack]: + """Return all tracks for given playlist (by unwrapping the paged listing).""" + result: list[PlaylistTrack] = [] + offset = 0 + limit = 50 + self.logger.debug( + "Fetching all tracks for playlist %s", + playlist.name, + ) + while True: + paged_items = await self.mass.music.playlists.tracks( + item_id=playlist.item_id, + provider_instance_id_or_domain=playlist.provider, + offset=offset, + limit=limit, + ) + result += paged_items.items + if paged_items.count != limit: + break + offset += paged_items.count + return result async def _add_library_item(self, item: Playlist) -> int: """Add a new record to the database.""" @@ -296,12 +365,9 @@ class PlaylistController(MediaControllerBase[Playlist]): f"provider_item.track.{provider.lookup_key}.{item_id}", item.to_dict() ) # store (serializable items) in cache - if cache_checksum != "no_cache": - self.mass.create_task( - self.mass.cache.set( - cache_key, [x.to_dict() for x in result], checksum=cache_checksum - ) - ) + self.mass.create_task( + self.mass.cache.set(cache_key, [x.to_dict() for x in result], checksum=cache_checksum) + ) return result async def _get_provider_dynamic_tracks( diff --git a/music_assistant/server/controllers/player_queues.py b/music_assistant/server/controllers/player_queues.py index 4e112210..7cc84373 100644 --- a/music_assistant/server/controllers/player_queues.py +++ b/music_assistant/server/controllers/player_queues.py @@ -33,8 +33,6 @@ from music_assistant.common.models.media_items import ( AlbumTrack, MediaItemType, PagedItems, - Playlist, - PlaylistTrack, media_from_dict, ) from music_assistant.common.models.player import PlayerMedia @@ -343,7 +341,7 @@ class PlayerQueuesController(CoreController): if radio_mode: radio_source.append(media_item) elif media_item.media_type == MediaType.PLAYLIST: - tracks += await self.get_playlist_tracks(media_item) + tracks += await self.mass.music.playlists.get_all_playlist_tracks(media_item) await self.mass.music.mark_item_played( media_item.media_type, media_item.item_id, media_item.provider ) @@ -1224,34 +1222,6 @@ class PlayerQueuesController(CoreController): in_library_only=album_items_conf == "library_tracks", ) - async def get_playlist_tracks(self, playlist: Playlist) -> list[PlaylistTrack]: - """Return all tracks for given playlist.""" - result: list[PlaylistTrack] = [] - offset = 0 - limit = 50 - self.logger.debug( - "Fetching tracks to play for playlist %s", - playlist.name, - ) - while True: - paged_items = await self.mass.music.playlists.tracks( - item_id=playlist.item_id, - provider_instance_id_or_domain=playlist.provider, - offset=offset, - limit=limit, - ) - result += paged_items.items - if paged_items.count < limit: - break - offset += paged_items.count - if offset == 500: - self.logger.info( - "Adding tracks for playlist %s to the queue which " - "has more than 500 items, this can take a while.", - playlist.name, - ) - return result - def __get_queue_stream_index(self, queue: PlayerQueue, player: Player) -> tuple[int, int]: """Calculate current queue index and current track elapsed time.""" # player is playing a constant stream so we need to do this the hard way diff --git a/music_assistant/server/providers/builtin/__init__.py b/music_assistant/server/providers/builtin/__init__.py index 9e5d6954..48a4bcf0 100644 --- a/music_assistant/server/providers/builtin/__init__.py +++ b/music_assistant/server/providers/builtin/__init__.py @@ -63,7 +63,6 @@ CONF_KEY_TRACKS = "stored_tracks" CONF_KEY_PLAYLISTS = "stored_playlists" -ALL_LIBRARY_TRACKS = "all_library_tracks" ALL_FAVORITE_TRACKS = "all_favorite_tracks" RANDOM_ARTIST = "random_artist" RANDOM_ALBUM = "random_album" @@ -71,15 +70,14 @@ RANDOM_TRACKS = "random_tracks" RECENTLY_PLAYED = "recently_played" BUILTIN_PLAYLISTS = { - ALL_LIBRARY_TRACKS: "All library tracks", ALL_FAVORITE_TRACKS: "All favorited tracks", RANDOM_ARTIST: "Random Artist (from library)", RANDOM_ALBUM: "Random Album (from library)", - RANDOM_TRACKS: "100 Random tracks (from library)", + RANDOM_TRACKS: "500 Random tracks (from library)", RECENTLY_PLAYED: "Recently played tracks", } -COLLAGE_IMAGE_PLAYLISTS = (ALL_FAVORITE_TRACKS, ALL_LIBRARY_TRACKS, RANDOM_TRACKS) +COLLAGE_IMAGE_PLAYLISTS = (ALL_FAVORITE_TRACKS, RANDOM_TRACKS) DEFAULT_THUMB = MediaItemImage( type=ImageType.THUMB, @@ -236,7 +234,7 @@ class BuiltinProvider(MusicProvider): images=[DEFAULT_THUMB] if prov_playlist_id in COLLAGE_IMAGE_PLAYLISTS else [DEFAULT_THUMB, DEFAULT_FANART], - cache_checksum="no_cache", + cache_checksum=str(time.time()), ), ) # user created universal playlist @@ -504,22 +502,18 @@ class BuiltinProvider(MusicProvider): ) -> AsyncGenerator[Track, None]: """Get all playlist tracks for given builtin playlist id.""" result: list[Track] = [] - if builtin_playlist_id == ALL_LIBRARY_TRACKS: - db_result = await self.mass.music.tracks.library_items(limit=2500, order_by="RANDOM()") - for idx, item in enumerate(db_result.items): - item.position = idx - result.append(item) - return result if builtin_playlist_id == ALL_FAVORITE_TRACKS: res = await self.mass.music.tracks.library_items( - favorite=True, limit=2500, order_by="RANDOM()" + favorite=True, limit=2500, order_by="RANDOM(), play_count" ) for idx, item in enumerate(res.items, 1): item.position = idx result.append(item) return result if builtin_playlist_id == RANDOM_TRACKS: - res = await self.mass.music.tracks.library_items(limit=100, order_by="RANDOM()") + res = await self.mass.music.tracks.library_items( + limit=500, order_by="RANDOM(), play_count" + ) for idx, item in enumerate(res.items, 1): item.position = idx result.append(item) diff --git a/music_assistant/server/providers/filesystem_local/base.py b/music_assistant/server/providers/filesystem_local/base.py index 79d835b5..ab75b96c 100644 --- a/music_assistant/server/providers/filesystem_local/base.py +++ b/music_assistant/server/providers/filesystem_local/base.py @@ -622,7 +622,7 @@ class FileSystemProviderBase(MusicProvider): # build new playlist data new_playlist_data = "#EXTM3U\n" for item in playlist_items: - playlist_data += f"\n#EXTINF:{item.length or 0},{item.title}\n{item.path}\n" + new_playlist_data += f"\n#EXTINF:{item.length or 0},{item.title}\n{item.path}\n" await self.write_file_content(prov_playlist_id, new_playlist_data.encode("utf-8")) async def create_playlist(self, name: str) -> Playlist: diff --git a/music_assistant/server/providers/spotify/__init__.py b/music_assistant/server/providers/spotify/__init__.py index 8e1ea762..0fc00d6a 100644 --- a/music_assistant/server/providers/spotify/__init__.py +++ b/music_assistant/server/providers/spotify/__init__.py @@ -125,7 +125,7 @@ class SpotifyProvider(MusicProvider): """Implementation of a Spotify MusicProvider.""" _auth_token: str | None = None - _sp_user: str | None = None + _sp_user: dict[str, Any] | None = None _librespot_bin: str | None = None # rate limiter needs to be specified on provider-level, # so make it an instance attribute @@ -153,6 +153,7 @@ class SpotifyProvider(MusicProvider): ProviderFeature.LIBRARY_PLAYLISTS_EDIT, ProviderFeature.LIBRARY_TRACKS_EDIT, ProviderFeature.PLAYLIST_TRACKS_EDIT, + ProviderFeature.PLAYLIST_CREATE, ProviderFeature.BROWSE, ProviderFeature.SEARCH, ProviderFeature.ARTIST_ALBUMS, @@ -355,37 +356,33 @@ class SpotifyProvider(MusicProvider): async def library_add(self, item: MediaItemType): """Add item to library.""" - result = False if item.media_type == MediaType.ARTIST: - result = await self._put_data("me/following", {"ids": [item.item_id]}, type="artist") + await self._put_data("me/following", {"ids": [item.item_id]}, type="artist") elif item.media_type == MediaType.ALBUM: - result = await self._put_data("me/albums", {"ids": [item.item_id]}) + await self._put_data("me/albums", {"ids": [item.item_id]}) elif item.media_type == MediaType.TRACK: - result = await self._put_data("me/tracks", {"ids": [item.item_id]}) + await self._put_data("me/tracks", {"ids": [item.item_id]}) elif item.media_type == MediaType.PLAYLIST: - result = await self._put_data( - f"playlists/{item.item_id}/followers", data={"public": False} - ) - return result + await self._put_data(f"playlists/{item.item_id}/followers", data={"public": False}) + return True async def library_remove(self, prov_item_id, media_type: MediaType): """Remove item from library.""" - result = False if media_type == MediaType.ARTIST: - result = await self._delete_data("me/following", {"ids": [prov_item_id]}, type="artist") + await self._delete_data("me/following", {"ids": [prov_item_id]}, type="artist") elif media_type == MediaType.ALBUM: - result = await self._delete_data("me/albums", {"ids": [prov_item_id]}) + await self._delete_data("me/albums", {"ids": [prov_item_id]}) elif media_type == MediaType.TRACK: - result = await self._delete_data("me/tracks", {"ids": [prov_item_id]}) + await self._delete_data("me/tracks", {"ids": [prov_item_id]}) elif media_type == MediaType.PLAYLIST: - result = await self._delete_data(f"playlists/{prov_item_id}/followers") - return result + await self._delete_data(f"playlists/{prov_item_id}/followers") + return True async def add_playlist_tracks(self, prov_playlist_id: str, prov_track_ids: list[str]): """Add track(s) to playlist.""" track_uris = [f"spotify:track:{track_id}" for track_id in prov_track_ids] data = {"uris": track_uris} - return await self._post_data(f"playlists/{prov_playlist_id}/tracks", data=data) + await self._post_data(f"playlists/{prov_playlist_id}/tracks", data=data) async def remove_playlist_tracks( self, prov_playlist_id: str, positions_to_remove: tuple[int, ...] @@ -396,7 +393,13 @@ class SpotifyProvider(MusicProvider): for track in await self.get_playlist_tracks(prov_playlist_id, pos, pos): track_uris.append({"uri": f"spotify:track:{track.item_id}"}) data = {"tracks": track_uris} - return await self._delete_data(f"playlists/{prov_playlist_id}/tracks", data=data) + await self._delete_data(f"playlists/{prov_playlist_id}/tracks", data=data) + + async def create_playlist(self, name: str) -> Playlist: + """Create a new playlist on provider with given name.""" + data = {"name": name, "public": False} + new_playlist = await self._post_data(f"users/{self._sp_user['id']}/playlists", data=data) + return self._parse_playlist(new_playlist) async def get_similar_tracks(self, prov_track_id, limit=25) -> list[Track]: """Retrieve a dynamic list of tracks based on the provided item.""" @@ -616,6 +619,8 @@ class SpotifyProvider(MusicProvider): remotely_accessible=True, ) ] + if playlist.owner is None: + playlist.owner = self._sp_user["display_name"] playlist.metadata.cache_checksum = str(playlist_obj["snapshot_id"]) return playlist @@ -799,7 +804,7 @@ class SpotifyProvider(MusicProvider): return await response.json(loads=json_loads) @throttle_with_retries - async def _delete_data(self, endpoint, data=None, **kwargs) -> str: + async def _delete_data(self, endpoint, data=None, **kwargs) -> None: """Delete data from api.""" url = f"https://api.spotify.com/v1/{endpoint}" token = await self.login() @@ -817,10 +822,9 @@ class SpotifyProvider(MusicProvider): if response.status in (502, 503): raise ResourceTemporarilyUnavailable(backoff_time=30) response.raise_for_status() - return await response.text() @throttle_with_retries - async def _put_data(self, endpoint, data=None, **kwargs) -> str: + async def _put_data(self, endpoint, data=None, **kwargs) -> dict[str, Any]: """Put data on api.""" url = f"https://api.spotify.com/v1/{endpoint}" token = await self.login() @@ -838,10 +842,10 @@ class SpotifyProvider(MusicProvider): if response.status in (502, 503): raise ResourceTemporarilyUnavailable(backoff_time=30) response.raise_for_status() - return await response.text() + return await response.json(loads=json_loads) @throttle_with_retries - async def _post_data(self, endpoint, data=None, **kwargs) -> str: + async def _post_data(self, endpoint, data=None, **kwargs) -> dict[str, Any]: """Post data on api.""" url = f"https://api.spotify.com/v1/{endpoint}" token = await self.login() @@ -859,7 +863,7 @@ class SpotifyProvider(MusicProvider): if response.status in (502, 503): raise ResourceTemporarilyUnavailable(backoff_time=30) response.raise_for_status() - return await response.text() + return await response.json(loads=json_loads) async def get_librespot_binary(self): """Find the correct librespot binary belonging to the platform.""" diff --git a/music_assistant/server/providers/tidal/__init__.py b/music_assistant/server/providers/tidal/__init__.py index 82f80d64..5e42a464 100644 --- a/music_assistant/server/providers/tidal/__init__.py +++ b/music_assistant/server/providers/tidal/__init__.py @@ -52,7 +52,7 @@ from music_assistant.server.models.music_provider import MusicProvider from .helpers import ( DEFAULT_LIMIT, - add_remove_playlist_tracks, + add_playlist_tracks, create_playlist, get_album, get_album_tracks, @@ -69,6 +69,7 @@ from .helpers import ( get_track, get_track_url, library_items_add_remove, + remove_playlist_tracks, search, ) @@ -395,9 +396,7 @@ class TidalProvider(MusicProvider): async def add_playlist_tracks(self, prov_playlist_id: str, prov_track_ids: list[str]) -> None: """Add track(s) to playlist.""" tidal_session = await self._get_tidal_session() - return await add_remove_playlist_tracks( - tidal_session, prov_playlist_id, prov_track_ids, add=True - ) + return await add_playlist_tracks(tidal_session, prov_playlist_id, prov_track_ids) async def remove_playlist_tracks( self, prov_playlist_id: str, positions_to_remove: tuple[int, ...] @@ -410,9 +409,7 @@ class TidalProvider(MusicProvider): prov_track_ids.append(track.item_id) if len(prov_track_ids) == len(positions_to_remove): break - return await add_remove_playlist_tracks( - tidal_session, prov_playlist_id, prov_track_ids, add=False - ) + return await remove_playlist_tracks(tidal_session, prov_playlist_id, prov_track_ids) async def create_playlist(self, name: str) -> Playlist: """Create a new playlist on provider with given name.""" diff --git a/music_assistant/server/providers/tidal/helpers.py b/music_assistant/server/providers/tidal/helpers.py index 2286a900..b933ede4 100644 --- a/music_assistant/server/providers/tidal/helpers.py +++ b/music_assistant/server/providers/tidal/helpers.py @@ -287,14 +287,23 @@ async def get_playlist_tracks( return await asyncio.to_thread(inner) -async def add_remove_playlist_tracks( - session: TidalSession, prov_playlist_id: str, track_ids: list[str], add: bool = True +async def add_playlist_tracks( + session: TidalSession, prov_playlist_id: str, track_ids: list[str] ) -> None: - """Async wrapper around the tidal Playlist.add and Playlist.remove function.""" + """Async wrapper around the tidal Playlist.add function.""" + + def inner() -> None: + TidalUserPlaylist(session, prov_playlist_id).add(track_ids) + + return await asyncio.to_thread(inner) + + +async def remove_playlist_tracks( + session: TidalSession, prov_playlist_id: str, track_ids: list[str] +) -> None: + """Async wrapper around the tidal Playlist.remove function.""" def inner() -> None: - if add: - TidalUserPlaylist(session, prov_playlist_id).add(track_ids) for item in track_ids: TidalUserPlaylist(session, prov_playlist_id).remove_by_id(int(item))