From 8637b4c59ea05573df14085e239168fd5d39401a Mon Sep 17 00:00:00 2001 From: Marcel van der Veldt Date: Thu, 30 Mar 2023 21:06:51 +0200 Subject: [PATCH] Prefer instance id over domain (#597) * prefer instance id over domain * Update docstring Co-authored-by: micha91 * adjust to is_unique property * fix preloading album tracks * add (shortlived) cache to get_provider_item * force_refresh * typos * typo --------- Co-authored-by: micha91 --- .../server/controllers/media/albums.py | 10 ++--- .../server/controllers/media/base.py | 36 +++++++++------- .../server/controllers/media/playlists.py | 2 +- .../server/controllers/media/radio.py | 2 +- .../server/controllers/media/tracks.py | 16 ++++--- music_assistant/server/controllers/music.py | 24 +++++++---- .../server/models/music_provider.py | 42 +++++++++++++++---- .../server/providers/filesystem_local/base.py | 29 +++++++++---- .../server/providers/plex/__init__.py | 27 ++++++++---- 9 files changed, 128 insertions(+), 60 deletions(-) diff --git a/music_assistant/server/controllers/media/albums.py b/music_assistant/server/controllers/media/albums.py index 529f57f1..977638c5 100644 --- a/music_assistant/server/controllers/media/albums.py +++ b/music_assistant/server/controllers/media/albums.py @@ -69,9 +69,9 @@ class AlbumsController(MediaControllerBase[Album]): if album.artist: album.artist = await self.mass.music.artists.get( album.artist.item_id, - album.artist.provider, + provider_instance=album.artist.provider, lazy=True, - details=album.artist, + details=None if isinstance(album.artist, ItemMapping) else album.artist, add_to_db=add_to_db, ) return album @@ -133,7 +133,7 @@ class AlbumsController(MediaControllerBase[Album]): """Add album to local db and return the database item.""" # grab additional metadata await self.mass.metadata.get_album_metadata(item) - existing = await self.get_db_item_by_prov_id(item.item_id, item.provider) + existing = await self.get_db_item_by_prov_id(item.item_id, provider_instance=item.provider) if existing: db_item = await self.update_db_item(existing.item_id, item) else: @@ -148,7 +148,7 @@ class AlbumsController(MediaControllerBase[Album]): prov_mapping.item_id, prov_mapping.provider_instance ): await self.mass.music.tracks.get( - track.item_id, track.provider, details=track, add_to_db=True + track.item_id, provider_instance=track.provider, details=track, add_to_db=True ) self.mass.signal_event( EventType.MEDIA_ITEM_UPDATED if existing else EventType.MEDIA_ITEM_ADDED, @@ -446,7 +446,7 @@ class AlbumsController(MediaControllerBase[Album]): return ItemMapping.from_item(artist) if db_artist := await self.mass.music.artists.get_db_item_by_prov_id( - artist.item_id, provider_domain=artist.provider + artist.item_id, provider_instance=artist.provider ): return ItemMapping.from_item(db_artist) diff --git a/music_assistant/server/controllers/media/base.py b/music_assistant/server/controllers/media/base.py index 03929884..68bf0236 100644 --- a/music_assistant/server/controllers/media/base.py +++ b/music_assistant/server/controllers/media/base.py @@ -133,7 +133,7 @@ class MediaControllerBase(Generic[ItemCls], metaclass=ABCMeta): provider_domain or provider_instance ), "provider_domain or provider_instance must be supplied" if not add_to_db and "database" in (provider_domain, provider_instance): - return await self.get_provider_item(item_id, provider_instance or provider_domain) + return await self.get_db_item(item_id) if details and details.provider == "database": details = None db_item = await self.get_db_item_by_prov_id( @@ -152,7 +152,9 @@ class MediaControllerBase(Generic[ItemCls], metaclass=ABCMeta): return db_item if not details and provider_instance: # no details provider nor in db, fetch them from the provider - details = await self.get_provider_item(item_id, provider_instance) + details = await self.get_provider_item( + item_id, provider_instance, force_refresh=force_refresh + ) if not details and provider_domain: # check providers for given provider domain one by one for prov in self.mass.music.providers: @@ -160,7 +162,9 @@ class MediaControllerBase(Generic[ItemCls], metaclass=ABCMeta): continue if prov.domain == provider_domain: try: - details = await self.get_provider_item(item_id, prov.domain) + details = await self.get_provider_item( + item_id, prov.domain, force_refresh=force_refresh + ) except MediaNotFoundError: pass else: @@ -409,21 +413,23 @@ class MediaControllerBase(Generic[ItemCls], metaclass=ABCMeta): self.mass.signal_event(EventType.MEDIA_ITEM_UPDATED, db_item.uri, db_item) async def get_provider_item( - self, - item_id: str, - provider_domain_or_instance_id: str, + self, item_id: str, provider_domain_or_instance_id: str, force_refresh: bool = False ) -> ItemCls: """Return item details for the given provider item id.""" + cache_key = ( + f"provider_item.{self.media_type.value}.{provider_domain_or_instance_id}.{item_id}" + ) if provider_domain_or_instance_id == "database": - item = await self.get_db_item(item_id) - else: - provider = self.mass.get_provider(provider_domain_or_instance_id) - item = (await provider.get_item(self.media_type, item_id)) if provider else None - if not item: - raise MediaNotFoundError( - f"{self.media_type.value}://{item_id} not found on provider {provider_domain_or_instance_id}" # noqa: E501 - ) - return item + return await self.get_db_item(item_id) + if not force_refresh and (cache := await self.mass.cache.get(cache_key)): + return self.item_cls.from_dict(cache) + if provider := self.mass.get_provider(provider_domain_or_instance_id): + item = await provider.get_item(self.media_type, item_id) + await self.mass.cache.set(cache_key, item.to_dict(), 3600) + return item + raise MediaNotFoundError( + f"{self.media_type.value}://{item_id} not found on provider {provider_domain_or_instance_id}" # noqa: E501 + ) async def remove_prov_mapping(self, item_id: int, provider_instance: str) -> None: """Remove provider id(s) from item.""" diff --git a/music_assistant/server/controllers/media/playlists.py b/music_assistant/server/controllers/media/playlists.py index 379fd4de..192346a5 100644 --- a/music_assistant/server/controllers/media/playlists.py +++ b/music_assistant/server/controllers/media/playlists.py @@ -63,7 +63,7 @@ class PlaylistController(MediaControllerBase[Playlist]): """Add playlist to local db and return the new database item.""" item.metadata.last_refresh = int(time()) await self.mass.metadata.get_playlist_metadata(item) - existing = await self.get_db_item_by_prov_id(item.item_id, item.provider) + existing = await self.get_db_item_by_prov_id(item.item_id, provider_instance=item.provider) if existing: db_item = await self.update_db_item(existing.item_id, item) else: diff --git a/music_assistant/server/controllers/media/radio.py b/music_assistant/server/controllers/media/radio.py index 1f304a69..8c7760da 100644 --- a/music_assistant/server/controllers/media/radio.py +++ b/music_assistant/server/controllers/media/radio.py @@ -61,7 +61,7 @@ class RadioController(MediaControllerBase[Radio]): """Add radio to local db and return the new database item.""" item.metadata.last_refresh = int(time()) await self.mass.metadata.get_radio_metadata(item) - existing = await self.get_db_item_by_prov_id(item.item_id, item.provider) + existing = await self.get_db_item_by_prov_id(item.item_id, provider_instance=item.provider) if existing: db_item = await self.update_db_item(existing.item_id, item) else: diff --git a/music_assistant/server/controllers/media/tracks.py b/music_assistant/server/controllers/media/tracks.py index 85dc6201..da07835b 100644 --- a/music_assistant/server/controllers/media/tracks.py +++ b/music_assistant/server/controllers/media/tracks.py @@ -73,9 +73,9 @@ class TracksController(MediaControllerBase[Track]): elif track.album: track.album = await self.mass.music.albums.get( track.album.item_id, - track.album.provider, + provider_instance=track.album.provider, lazy=True, - details=track.album, + details=None if isinstance(track.album, ItemMapping) else track.album, add_to_db=add_to_db, ) except MediaNotFoundError: @@ -86,7 +86,11 @@ class TracksController(MediaControllerBase[Track]): for artist in track.artists: full_artists.append( await self.mass.music.artists.get( - artist.item_id, artist.provider, lazy=True, details=artist, add_to_db=add_to_db + artist.item_id, + provider_instance=artist.provider, + lazy=True, + details=None if isinstance(artist, ItemMapping) else artist, + add_to_db=add_to_db, ) ) track.artists = full_artists @@ -98,7 +102,7 @@ class TracksController(MediaControllerBase[Track]): assert item.artists # grab additional metadata await self.mass.metadata.get_track_metadata(item) - existing = await self.get_db_item_by_prov_id(item.item_id, item.provider) + existing = await self.get_db_item_by_prov_id(item.item_id, provider_instance=item.provider) if existing: db_item = await self.update_db_item(existing.item_id, item) else: @@ -156,7 +160,9 @@ class TracksController(MediaControllerBase[Track]): track = await self.get(item_id, provider_domain or provider_instance, add_to_db=False) return await asyncio.gather( *[ - self.mass.music.albums.get(album.item_id, album.provider, add_to_db=False) + self.mass.music.albums.get( + album.item_id, provider_instance=album.provider, add_to_db=False + ) for album in track.albums ] ) diff --git a/music_assistant/server/controllers/music.py b/music_assistant/server/controllers/music.py index 1518c938..82e2edd0 100755 --- a/music_assistant/server/controllers/music.py +++ b/music_assistant/server/controllers/music.py @@ -124,16 +124,15 @@ class MusicController: :param limit: number of items to return in the search (per type). """ # include results from all (unique) music providers - provider_domains = {item.domain for item in self.providers} results_per_provider: list[SearchResults] = await asyncio.gather( *[ self.search_provider( search_query, media_types, - provider_domain=provider_domain, + provider_instance=provider_instance, limit=limit, ) - for provider_domain in provider_domains + for provider_instance in self.get_unique_providers() ] ) # return result from all providers while keeping index @@ -253,7 +252,7 @@ class MusicController: for prov in self.providers: if prov.instance_id == provider_domain_or_instance_id: provider_instance = prov.instance_id - provider_domain = None + provider_domain = prov.domain break else: provider_instance = None @@ -332,7 +331,7 @@ class MusicController: self.add_to_library( media_type=item.media_type, item_id=item.item_id, - provider_domain=item.provider, + provider_instance=item.provider, ) ) ) @@ -367,6 +366,7 @@ class MusicController: media_type=item.media_type, item_id=item.item_id, provider_domain=item.provider, + provider_instance=item.provider, ) ) ) @@ -398,7 +398,7 @@ class MusicController: return await self.get_item( media_item.media_type, media_item.item_id, - provider_domain=media_item.provider, + provider_instance=media_item.provider, force_refresh=True, lazy=False, add_to_db=True, @@ -479,7 +479,11 @@ class MusicController: """ for media_item in items: self.mass.create_task( - self.add_to_library(media_item.media_type, media_item.item_id, media_item.provider) + self.add_to_library( + media_item.media_type, + media_item.item_id, + provider_instance=media_item.provider, + ) ) async def library_remove_items(self, items: list[MediaItemType]) -> None: @@ -490,7 +494,9 @@ class MusicController: for media_item in items: self.mass.create_task( self.remove_from_library( - media_item.media_type, media_item.item_id, media_item.provider + media_item.media_type, + media_item.item_id, + provider_instance=media_item.provider, ) ) @@ -526,7 +532,7 @@ class MusicController: instances = set() domains = set() for provider in self.providers: - if provider.domain not in domains or provider.is_file(): + if provider.domain not in domains or provider.is_unique: instances.add(provider.instance_id) domains.add(provider.domain) return instances diff --git a/music_assistant/server/models/music_provider.py b/music_assistant/server/models/music_provider.py index e042dff0..72c8f6e1 100644 --- a/music_assistant/server/models/music_provider.py +++ b/music_assistant/server/models/music_provider.py @@ -27,6 +27,19 @@ class MusicProvider(Provider): Music Provider implementations should inherit from this base model. """ + @property + def is_unique(self) -> bool: + """ + Return True if the (non user related) data in this provider instance is unique. + + For example on a global streaming provider (like Spotify), + the data on all instances is the same. + For a file provider each instance has other items. + Setting this to False will only query one instance of the provider for search and lookups. + Setting this to True will query all instances of this provider for search and lookups. + """ + return False + async def search( self, search_query: str, @@ -396,17 +409,33 @@ class MusicProvider(Provider): ) if not db_item: # dump the item in the db, rich metadata is lazy loaded later - db_item = await controller.add_db_item(prov_item) + db_item = await controller.get(prov_item) elif ( db_item.metadata.checksum and prov_item.metadata.checksum ) and db_item.metadata.checksum != prov_item.metadata.checksum: # item checksum changed db_item = await controller.update_db_item(db_item.item_id, prov_item) - # preload album/playlist tracks - if prov_item.media_type == (MediaType.ALBUM, MediaType.PLAYLIST): + # add album tracks to the db too + if prov_item.media_type == MediaType.ALBUM: + prov_item: Album # noqa: PLW2901 for track in controller.tracks(prov_item.item_id, prov_item.provider): - await self.mass.music.tracks.add_db_item(track) + track: Track # noqa: PLW2901 + track.album = db_item + await self.mass.music.tracks.get( + track.item_id, + provider_instance=self.instance_id, + lazy=False, + details=track, + add_to_db=True, + ) + # preload playlist tracks listing, do not load them in the db + # because that would make the sync very slow and has not much benefit + if prov_item.media_type == MediaType.PLAYLIST: + async for track in controller.tracks( + prov_item.item_id, provider_instance=self.instance_id + ): + pass cur_db_ids.add(db_item.item_id) if not db_item.in_library: await controller.set_db_library(db_item.item_id, True) @@ -424,11 +453,6 @@ class MusicProvider(Provider): # only mark the item as not in library and leave the metadata in db await controller.set_db_library(db_item.item_id, False) - def is_file(self) -> bool: - """Return if this is a FileSystem based provider.""" - # override this is needed - return self.domain.startswith("filesystem") - # DO NOT OVERRIDE BELOW def library_supported(self, media_type: MediaType) -> bool: diff --git a/music_assistant/server/providers/filesystem_local/base.py b/music_assistant/server/providers/filesystem_local/base.py index bd027de5..8ef3ec61 100644 --- a/music_assistant/server/providers/filesystem_local/base.py +++ b/music_assistant/server/providers/filesystem_local/base.py @@ -173,6 +173,19 @@ class FileSystemProviderBase(MusicProvider): # DEFAULT/GENERIC IMPLEMENTATION BELOW # should normally not be needed to override + @property + def is_unique(self) -> bool: + """ + Return True if the (non user related) data in this provider instance is unique. + + For example on a global streaming provider (like Spotify), + the data on all instances is the same. + For a file provider each instance has other items. + Setting this to False will only query one instance of the provider for search and lookups. + Setting this to True will query all instances of this provider for search and lookups. + """ + return True + async def search( self, search_query: str, media_types=list[MediaType] | None, limit: int = 5 # noqa: ARG002 ) -> SearchResults: @@ -213,7 +226,7 @@ class FileSystemProviderBase(MusicProvider): subitems.append( BrowseFolder( item_id=item.path, - provider=self.domain, + provider=self.instance_id, path=f"{self.instance_id}://{item.path}", name=item.name, ) @@ -249,7 +262,7 @@ class FileSystemProviderBase(MusicProvider): return BrowseFolder( item_id=item_path, - provider=self.domain, + provider=self.instance_id, path=path, name=item_path or self.name, # make sure to sort the resulting listing @@ -386,7 +399,7 @@ class FileSystemProviderBase(MusicProvider): file_item = await self.resolve(prov_playlist_id) playlist = Playlist( file_item.path, - provider=self.domain, + provider=self.instance_id, name=file_item.name.replace(f".{file_item.ext}", ""), ) playlist.is_editable = file_item.ext != "pls" # can only edit m3u playlists @@ -581,7 +594,7 @@ class FileSystemProviderBase(MusicProvider): name, version = parse_title_and_version(tags.title, tags.version) track = Track( item_id=file_item.path, - provider=self.domain, + provider=self.instance_id, name=name, version=version, ) @@ -724,10 +737,10 @@ class FileSystemProviderBase(MusicProvider): artist = Artist( artist_path, - self.domain, + self.instance_id, name, provider_mappings={ - ProviderMapping(artist_path, self.domain, self.instance_id, url=artist_path) + ProviderMapping(artist_path, self.instance_id, self.instance_id, url=artist_path) }, musicbrainz_id=VARIOUS_ARTISTS_ID if compare_strings(name, VARIOUS_ARTISTS) else None, ) @@ -774,11 +787,11 @@ class FileSystemProviderBase(MusicProvider): album = Album( album_path, - self.domain, + self.instance_id, name, artists=artists, provider_mappings={ - ProviderMapping(album_path, self.domain, self.instance_id, url=album_path) + ProviderMapping(album_path, self.instance_id, self.instance_id, url=album_path) }, ) diff --git a/music_assistant/server/providers/plex/__init__.py b/music_assistant/server/providers/plex/__init__.py index b50f4ff2..f53c80f6 100644 --- a/music_assistant/server/providers/plex/__init__.py +++ b/music_assistant/server/providers/plex/__init__.py @@ -108,10 +108,6 @@ class PlexProvider(MusicProvider): self._plex_server.library.section, self.config.get_value(CONF_LIBRARY_NAME) ) - async def resolve_image(self, path: str) -> str | bytes | AsyncGenerator[bytes, None]: - """Return the full image URL including the auth token.""" - return self._plex_server.url(path, True) - @property def supported_features(self) -> tuple[ProviderFeature, ...]: """Return a list of supported features.""" @@ -125,6 +121,23 @@ class PlexProvider(MusicProvider): ProviderFeature.ARTIST_ALBUMS, ) + @property + def is_unique(self) -> bool: + """ + Return True if the (non user related) data in this provider instance is unique. + + For example on a global streaming provider (like Spotify), + the data on all instances is the same. + For a file provider each instance has other items. + Setting this to False will only query one instance of the provider for search and lookups. + Setting this to True will query all instances of this provider for search and lookups. + """ + return True + + async def resolve_image(self, path: str) -> str | bytes | AsyncGenerator[bytes, None]: + """Return the full image URL including the auth token.""" + return self._plex_server.url(path, True) + async def _run_async(self, call: Callable, *args, **kwargs): return await self.mass.create_task(call, *args, **kwargs) @@ -201,7 +214,7 @@ class PlexProvider(MusicProvider): album_id = plex_album.key album = Album( item_id=album_id, - provider=self.instance_id, + provider=self.domain, name=plex_album.title, ) if plex_album.year: @@ -230,7 +243,7 @@ class PlexProvider(MusicProvider): artist_id = plex_artist.key if not artist_id: raise InvalidDataError("Artist does not have a valid ID") - artist = Artist(item_id=artist_id, name=plex_artist.title, provider=self.instance_id) + artist = Artist(item_id=artist_id, name=plex_artist.title, provider=self.domain) if plex_artist.summary: artist.metadata.description = plex_artist.summary if thumb := plex_artist.firstAttr("thumb", "parentThumb", "grandparentThumb"): @@ -248,7 +261,7 @@ class PlexProvider(MusicProvider): async def _parse_playlist(self, plex_playlist: PlexPlaylist) -> Playlist: """Parse a Plex Playlist response to a Playlist object.""" playlist = Playlist( - item_id=plex_playlist.key, provider=self.instance_id, name=plex_playlist.title + item_id=plex_playlist.key, provider=self.domain, name=plex_playlist.title ) if plex_playlist.summary: playlist.metadata.description = plex_playlist.summary -- 2.34.1