From c8c7afed2da38bb42be95ff1b10515a7b1af12e6 Mon Sep 17 00:00:00 2001 From: Marcel van der Veldt Date: Thu, 18 Apr 2024 10:49:59 +0200 Subject: [PATCH 1/6] Fix overwriting of information on force refresh --- music_assistant/common/models/media_items.py | 4 +- .../server/controllers/media/albums.py | 22 ++++++++--- .../server/controllers/media/artists.py | 20 +++++++--- .../server/controllers/media/base.py | 19 +++++---- .../server/controllers/media/playlists.py | 22 +++++++---- .../server/controllers/media/radio.py | 22 +++++++---- .../server/controllers/media/tracks.py | 39 +++++++++++++------ 7 files changed, 102 insertions(+), 46 deletions(-) diff --git a/music_assistant/common/models/media_items.py b/music_assistant/common/models/media_items.py index 3dfbdce3a..2eea03522 100644 --- a/music_assistant/common/models/media_items.py +++ b/music_assistant/common/models/media_items.py @@ -213,7 +213,9 @@ def update( if new_val is None: continue cur_val = getattr(self, fld.name) - if isinstance(cur_val, list) and isinstance(new_val, list): + if allow_overwrite and new_val: + setattr(self, fld.name, new_val) + elif isinstance(cur_val, list) and isinstance(new_val, list): new_val = merge_lists(cur_val, new_val) setattr(self, fld.name, new_val) elif isinstance(cur_val, set) and isinstance(new_val, list): diff --git a/music_assistant/server/controllers/media/albums.py b/music_assistant/server/controllers/media/albums.py index deb83705d..1e8dd3f18 100644 --- a/music_assistant/server/controllers/media/albums.py +++ b/music_assistant/server/controllers/media/albums.py @@ -102,6 +102,7 @@ async def add_item_to_library( self, item: Album, metadata_lookup: bool = True, + overwrite_existing: bool = False, add_album_tracks: bool = False, ) -> Album: """Add album to library and return the database item.""" @@ -118,16 +119,22 @@ async def add_item_to_library( library_item = None if cur_item := await self.get_library_item_by_prov_id(item.item_id, item.provider): # existing item match by provider id - library_item = await self.update_item_in_library(cur_item.item_id, item) + library_item = await self.update_item_in_library( + cur_item.item_id, item, overwrite=overwrite_existing + ) elif cur_item := await self.get_library_item_by_external_ids(item.external_ids): # existing item match by external id - library_item = await self.update_item_in_library(cur_item.item_id, item) + library_item = await self.update_item_in_library( + cur_item.item_id, item, overwrite=overwrite_existing + ) else: # search by name async for db_item in self.iter_library_items(search=item.name): if compare_album(db_item, item): # existing item found: update it - library_item = await self.update_item_in_library(db_item.item_id, item) + library_item = await self.update_item_in_library( + db_item.item_id, item, overwrite=overwrite_existing + ) break if not library_item: # actually add a new item in the library db @@ -139,6 +146,7 @@ async def add_item_to_library( await self._match(library_item) library_item = await self.get_library_item(library_item.item_id) # also add album tracks + # TODO: make this configurable if add_album_tracks and item.provider != "library": async with asyncio.TaskGroup() as tg: for track in await self._get_provider_album_tracks(item.item_id, item.provider): @@ -159,8 +167,8 @@ async def update_item_in_library( """Update existing record in the database.""" db_id = int(item_id) # ensure integer cur_item = await self.get_library_item(db_id) - metadata = cur_item.metadata.update(getattr(update, "metadata", None), overwrite) - provider_mappings = self._get_provider_mappings(cur_item, update, overwrite) + metadata = cur_item.metadata.update(update.metadata, overwrite) + provider_mappings = self._get_provider_mappings(cur_item, update) album_artists = await self._get_artist_mappings(cur_item, update, overwrite) if getattr(update, "album_type", AlbumType.UNKNOWN) != AlbumType.UNKNOWN: album_type = update.album_type @@ -173,7 +181,9 @@ async def update_item_in_library( {"item_id": db_id}, { "name": update.name if overwrite else cur_item.name, - "sort_name": update.sort_name if overwrite else cur_item.sort_name, + "sort_name": update.sort_name + if overwrite + else cur_item.sort_name or update.sort_name, "sort_artist": sort_artist, "version": update.version if overwrite else cur_item.version, "year": update.year if overwrite else cur_item.year or update.year, diff --git a/music_assistant/server/controllers/media/artists.py b/music_assistant/server/controllers/media/artists.py index 3b0d95426..dfb509d64 100644 --- a/music_assistant/server/controllers/media/artists.py +++ b/music_assistant/server/controllers/media/artists.py @@ -61,6 +61,7 @@ async def add_item_to_library( self, item: Artist | ItemMapping, metadata_lookup: bool = True, + overwrite_existing: bool = False, ) -> Artist: """Add artist to library and return the database item.""" if isinstance(item, ItemMapping): @@ -73,10 +74,14 @@ async def add_item_to_library( library_item = None if cur_item := await self.get_library_item_by_prov_id(item.item_id, item.provider): # existing item match by provider id - library_item = await self.update_item_in_library(cur_item.item_id, item) + library_item = await self.update_item_in_library( + cur_item.item_id, item, overwrite=overwrite_existing + ) elif cur_item := await self.get_library_item_by_external_ids(item.external_ids): # existing item match by external id - library_item = await self.update_item_in_library(cur_item.item_id, item) + library_item = await self.update_item_in_library( + cur_item.item_id, item, overwrite=overwrite_existing + ) else: # search by name async for db_item in self.iter_library_items(search=item.name): @@ -85,9 +90,10 @@ async def add_item_to_library( # NOTE: if we matched an artist by name this could theoretically lead to # collisions but the chance is so small it is not worth the additional # overhead of grabbing the musicbrainz id upfront - library_item = await self.update_item_in_library(db_item.item_id, item) + library_item = await self.update_item_in_library( + db_item.item_id, item, overwrite=overwrite_existing + ) break - await asyncio.sleep(0) # yield to eventloop if not library_item: # actually add (or update) the item in the library db # use the lock to prevent a race condition of the same item being added twice @@ -112,7 +118,7 @@ async def update_item_in_library( db_id = int(item_id) # ensure integer cur_item = await self.get_library_item(db_id) metadata = cur_item.metadata.update(getattr(update, "metadata", None), overwrite) - provider_mappings = self._get_provider_mappings(cur_item, update, overwrite) + provider_mappings = self._get_provider_mappings(cur_item, update) cur_item.external_ids.update(update.external_ids) # enforce various artists name + id mbid = cur_item.mbid @@ -126,7 +132,9 @@ async def update_item_in_library( {"item_id": db_id}, { "name": update.name if overwrite else cur_item.name, - "sort_name": update.sort_name if overwrite else cur_item.sort_name, + "sort_name": update.sort_name + if overwrite + else cur_item.sort_name or update.sort_name, "external_ids": serialize_to_json( update.external_ids if overwrite else cur_item.external_ids ), diff --git a/music_assistant/server/controllers/media/base.py b/music_assistant/server/controllers/media/base.py index 977eeb130..30bd1e8ff 100644 --- a/music_assistant/server/controllers/media/base.py +++ b/music_assistant/server/controllers/media/base.py @@ -52,7 +52,9 @@ def __init__(self, mass: MusicAssistant) -> None: self.logger = logging.getLogger(f"{MASS_LOGGER_NAME}.music.{self.media_type.value}") @abstractmethod - async def add_item_to_library(self, item: ItemCls, metadata_lookup: bool = True) -> ItemCls: + async def add_item_to_library( + self, item: ItemCls, metadata_lookup: bool = True, overwrite_existing: bool = False + ) -> ItemCls: """Add item to library and return the database item.""" raise NotImplementedError @@ -161,9 +163,6 @@ async def get( add_to_library: bool = False, ) -> ItemCls: """Return (full) details for a single media item.""" - if provider_instance_id_or_domain == "database": - # backwards compatibility - to remove when 2.0 stable is released - provider_instance_id_or_domain = "library" # always prefer the full library item if we have it library_item = await self.get_library_item_by_prov_id( item_id, @@ -208,8 +207,15 @@ async def get( # in 99% of the cases we just return lazy because we want the details as fast as possible # only if we really need to wait for the result (e.g. to prevent race conditions), # we can set lazy to false and we await the job to complete. + overwrite_existing = force_refresh and library_item is not None task_id = f"add_{self.media_type.value}.{details.provider}.{details.item_id}" - add_task = self.mass.create_task(self.add_item_to_library, item=details, task_id=task_id) + add_task = self.mass.create_task( + self.add_item_to_library, + item=details, + metadata_lookup=True, + overwrite_existing=overwrite_existing, + task_id=task_id, + ) if not lazy: await add_task return add_task.result() @@ -677,13 +683,10 @@ def _get_provider_mappings( self, org_item: ItemCls, update_item: ItemCls | ItemMapping | None = None, - overwrite: bool = False, ) -> set[ProviderMapping]: """Get/merge provider mappings for an item.""" if not update_item or isinstance(update_item, ItemMapping): return org_item.provider_mappings - if overwrite and update_item.provider_mappings: - return update_item.provider_mappings return {*update_item.provider_mappings, *org_item.provider_mappings} async def _get_artist_mappings( diff --git a/music_assistant/server/controllers/media/playlists.py b/music_assistant/server/controllers/media/playlists.py index e45107ced..9d1505373 100644 --- a/music_assistant/server/controllers/media/playlists.py +++ b/music_assistant/server/controllers/media/playlists.py @@ -53,7 +53,9 @@ def __init__(self, *args, **kwargs) -> None: "music/playlists/remove_playlist_tracks", self.remove_playlist_tracks ) - async def add_item_to_library(self, item: Playlist, metadata_lookup: bool = True) -> Playlist: + async def add_item_to_library( + self, item: Playlist, metadata_lookup: bool = True, overwrite_existing: bool = False + ) -> Playlist: """Add playlist to library and return the new database item.""" if isinstance(item, ItemMapping): metadata_lookup = False @@ -68,10 +70,14 @@ async def add_item_to_library(self, item: Playlist, metadata_lookup: bool = True library_item = None if cur_item := await self.get_library_item_by_prov_id(item.item_id, item.provider): # existing item match by provider id - library_item = await self.update_item_in_library(cur_item.item_id, item) + library_item = await self.update_item_in_library( + cur_item.item_id, item, overwrite=overwrite_existing + ) elif cur_item := await self.get_library_item_by_external_ids(item.external_ids): # existing item match by external id - library_item = await self.update_item_in_library(cur_item.item_id, item) + library_item = await self.update_item_in_library( + cur_item.item_id, item, overwrite=overwrite_existing + ) if not library_item: # actually add a new item in the library db # use the lock to prevent a race condition of the same item being added twice @@ -98,16 +104,18 @@ async def update_item_in_library( db_id = int(item_id) # ensure integer cur_item = await self.get_library_item(db_id) metadata = cur_item.metadata.update(getattr(update, "metadata", None), overwrite) - provider_mappings = self._get_provider_mappings(cur_item, update, overwrite) + provider_mappings = self._get_provider_mappings(cur_item, update) cur_item.external_ids.update(update.external_ids) await self.mass.music.database.update( self.db_table, {"item_id": db_id}, { # always prefer name/owner from updated item here - "name": update.name or cur_item.name, - "sort_name": update.sort_name or cur_item.sort_name, - "owner": update.owner or cur_item.sort_name, + "name": update.name if overwrite else cur_item.name, + "sort_name": update.sort_name + if overwrite + else cur_item.sort_name or update.sort_name, + "owner": update.owner or cur_item.owner, "is_editable": update.is_editable, "metadata": serialize_to_json(metadata), "provider_mappings": serialize_to_json(provider_mappings), diff --git a/music_assistant/server/controllers/media/radio.py b/music_assistant/server/controllers/media/radio.py index c4b1c5392..c1dc17a67 100644 --- a/music_assistant/server/controllers/media/radio.py +++ b/music_assistant/server/controllers/media/radio.py @@ -63,7 +63,9 @@ async def versions( # return the aggregated result return all_versions.values() - async def add_item_to_library(self, item: Radio, metadata_lookup: bool = True) -> Radio: + async def add_item_to_library( + self, item: Radio, metadata_lookup: bool = True, overwrite_existing: bool = False + ) -> Radio: """Add radio to library and return the new database item.""" if isinstance(item, ItemMapping): metadata_lookup = False @@ -80,14 +82,18 @@ async def add_item_to_library(self, item: Radio, metadata_lookup: bool = True) - library_item = None if cur_item := await self.get_library_item_by_prov_id(item.item_id, item.provider): # existing item match by provider id - library_item = await self.update_item_in_library(cur_item.item_id, item) + library_item = await self.update_item_in_library( + cur_item.item_id, item, overwrite=overwrite_existing + ) elif cur_item := await self.get_library_item_by_external_ids(item.external_ids): # existing item match by external id - library_item = await self.update_item_in_library(cur_item.item_id, item) + library_item = await self.update_item_in_library( + cur_item.item_id, item, overwrite=overwrite_existing + ) else: # search by name async for db_item in self.iter_library_items(search=item.name): - if compare_strings(db_item.name, item.name): + if compare_strings(db_item.name, item.name, strict=True): # existing item found: update it library_item = await self.update_item_in_library(db_item.item_id, item) break @@ -110,7 +116,7 @@ async def update_item_in_library( db_id = int(item_id) # ensure integer cur_item = await self.get_library_item(db_id) metadata = cur_item.metadata.update(getattr(update, "metadata", None), overwrite) - provider_mappings = self._get_provider_mappings(cur_item, update, overwrite) + provider_mappings = self._get_provider_mappings(cur_item, update) cur_item.external_ids.update(update.external_ids) match = {"item_id": db_id} await self.mass.music.database.update( @@ -118,8 +124,10 @@ async def update_item_in_library( match, { # always prefer name from updated item here - "name": update.name or cur_item.name, - "sort_name": update.sort_name or cur_item.sort_name, + "name": update.name if overwrite else cur_item.name, + "sort_name": update.sort_name + if overwrite + else cur_item.sort_name or update.sort_name, "metadata": serialize_to_json(metadata), "provider_mappings": serialize_to_json(provider_mappings), "external_ids": serialize_to_json( diff --git a/music_assistant/server/controllers/media/tracks.py b/music_assistant/server/controllers/media/tracks.py index a15e64652..bd8047c1b 100644 --- a/music_assistant/server/controllers/media/tracks.py +++ b/music_assistant/server/controllers/media/tracks.py @@ -127,7 +127,9 @@ async def get( track.artists = track_artists return track - async def add_item_to_library(self, item: Track, metadata_lookup: bool = True) -> Track: + async def add_item_to_library( + self, item: Track, metadata_lookup: bool = True, overwrite_existing: bool = False + ) -> Track: """Add track to library and return the new database item.""" if not isinstance(item, Track): msg = "Not a valid Track object (ItemMapping can not be added to db)" @@ -157,18 +159,23 @@ async def add_item_to_library(self, item: Track, metadata_lookup: bool = True) - library_item = None if cur_item := await self.get_library_item_by_prov_id(item.item_id, item.provider): # existing item match by provider id - library_item = await self.update_item_in_library(cur_item.item_id, item) + library_item = await self.update_item_in_library( + cur_item.item_id, item, overwrite=overwrite_existing + ) elif cur_item := await self.get_library_item_by_external_ids(item.external_ids): # existing item match by external id - library_item = await self.update_item_in_library(cur_item.item_id, item) + library_item = await self.update_item_in_library( + cur_item.item_id, item, overwrite=overwrite_existing + ) else: # search by name async for db_item in self.iter_library_items(search=item.name): if compare_track(db_item, item): # existing item found: update it - library_item = await self.update_item_in_library(db_item.item_id, item) + library_item = await self.update_item_in_library( + db_item.item_id, item, overwrite=overwrite_existing + ) break - await asyncio.sleep(0) # yield to eventloop if not library_item: # actually add a new item in the library db # use the lock to prevent a race condition of the same item being added twice @@ -193,17 +200,19 @@ async def update_item_in_library( db_id = int(item_id) # ensure integer cur_item = await self.get_library_item(db_id) metadata = cur_item.metadata.update(getattr(update, "metadata", None), overwrite) - provider_mappings = self._get_provider_mappings(cur_item, update, overwrite) + provider_mappings = self._get_provider_mappings(cur_item, update) track_artists = await self._get_artist_mappings(cur_item, update, overwrite=overwrite) cur_item.external_ids.update(update.external_ids) await self.mass.music.database.update( self.db_table, {"item_id": db_id}, { - "name": update.name or cur_item.name, - "sort_name": update.sort_name or cur_item.sort_name, - "version": update.version or cur_item.version, - "duration": getattr(update, "duration", None) or cur_item.duration, + "name": update.name if overwrite else cur_item.name, + "sort_name": update.sort_name + if overwrite + else cur_item.sort_name or update.sort_name, + "version": update.version if overwrite else cur_item.version or update.version, + "duration": update.duration if overwrite else cur_item.duration or update.duration, "artists": serialize_to_json(track_artists), "metadata": serialize_to_json(metadata), "provider_mappings": serialize_to_json(provider_mappings), @@ -425,7 +434,15 @@ async def _add_library_item(self, item: Track) -> Track: async def _set_track_album( self, db_id: int, album: Album, disc_number: int, track_number: int ) -> None: - """Store AlbumTrack info.""" + """ + Store Track Album info. + + A track can exist on multiple albums so we have a mapping table between + albums and tracks which stores the relation between the two and it also + stores the track and disc number of the track within an album. + For digital releases, the discnumber will be just 0 or 1. + Track number should start counting at 1. + """ db_album = None if album.provider == "library": db_album = album From 7f7879d6fc3816efa79db551ecb8c4610642350e Mon Sep 17 00:00:00 2001 From: Marcel van der Veldt Date: Thu, 18 Apr 2024 18:04:28 +0200 Subject: [PATCH 2/6] convert joined tables to json --- music_assistant/common/models/media_items.py | 52 ++++++- music_assistant/constants.py | 4 +- .../server/controllers/media/albums.py | 59 ++++++-- .../server/controllers/media/artists.py | 10 +- .../server/controllers/media/base.py | 62 +++++---- .../server/controllers/media/tracks.py | 60 ++++++-- music_assistant/server/controllers/music.py | 131 +++++++++++++++--- .../server/providers/filesystem_local/base.py | 12 +- 8 files changed, 303 insertions(+), 87 deletions(-) diff --git a/music_assistant/common/models/media_items.py b/music_assistant/common/models/media_items.py index 2eea03522..4449a3838 100644 --- a/music_assistant/common/models/media_items.py +++ b/music_assistant/common/models/media_items.py @@ -332,11 +332,19 @@ class ItemMapping(_MediaItemBase): """Representation of a minimized item object.""" available: bool = True + image: MediaItemImage | None = None @classmethod def from_item(cls, item: MediaItem) -> ItemMapping: """Create ItemMapping object from regular item.""" - return cls.from_dict(item.to_dict()) + thumb_image = None + if item.metadata and item.metadata.images: + for img in item.metadata.images: + if img.type != ImageType.THUMB: + continue + thumb_image = img + break + return cls.from_dict({**item.to_dict(), "image": thumb_image.to_dict()}) @dataclass(kw_only=True) @@ -366,6 +374,9 @@ class Track(MediaItem): version: str = "" artists: list[Artist | ItemMapping] = field(default_factory=list) album: Album | ItemMapping | None = None # optional + disc_number: int | None = None # required for album tracks + track_number: int | None = None # required for album tracks + position: int | None = None # required for playlist tracks def __hash__(self): """Return custom hash.""" @@ -396,18 +407,45 @@ def artist_str(self) -> str: @dataclass(kw_only=True) class AlbumTrack(Track): - """Model for a track on an album.""" + """ + Model for a track on an album. - album: Album | ItemMapping # required - disc_number: int = 0 - track_number: int = 0 + Same as regular Track but with explicit and required definitions of + album, disc_number and track_number + """ + + album: Album + disc_number: int + track_number: int + + @classmethod + def from_track(cls: Self, track: Track, album: Album | None = None) -> Self: + """Cast Track to AlbumTrack.""" + if album: + track.album = album + assert isinstance(track.album, Album) + assert track.disc_number is not None + assert track.track_number is not None + assert track.track_number > 0 + return cast(AlbumTrack, track) @dataclass(kw_only=True) class PlaylistTrack(Track): - """Model for a track on a playlist.""" + """ + Model for a track on a playlist. - position: int # required + Same as regular Track but with explicit and required definition of position. + """ + + position: int + + @classmethod + def from_track(cls: Self, track: Track) -> Self: + """Cast Track to PlaylistTrack.""" + assert track.position is not None + assert track.position > 0 + return cast(AlbumTrack, track) @dataclass(kw_only=True) diff --git a/music_assistant/constants.py b/music_assistant/constants.py index 52a6272e2..1c60d71bc 100644 --- a/music_assistant/constants.py +++ b/music_assistant/constants.py @@ -5,7 +5,7 @@ API_SCHEMA_VERSION: Final[int] = 24 MIN_SCHEMA_VERSION: Final[int] = 24 -DB_SCHEMA_VERSION: Final[int] = 29 +DB_SCHEMA_VERSION: Final[int] = 30 MASS_LOGGER_NAME: Final[str] = "music_assistant" @@ -76,6 +76,8 @@ DB_TABLE_ALBUMS: Final[str] = "albums" DB_TABLE_TRACKS: Final[str] = "tracks" DB_TABLE_ALBUM_TRACKS: Final[str] = "albumtracks" +DB_TABLE_TRACK_ARTISTS: Final[str] = "trackartists" +DB_TABLE_ALBUM_ARTISTS: Final[str] = "albumartists" DB_TABLE_PLAYLISTS: Final[str] = "playlists" DB_TABLE_RADIOS: Final[str] = "radios" DB_TABLE_CACHE: Final[str] = "cache" diff --git a/music_assistant/server/controllers/media/albums.py b/music_assistant/server/controllers/media/albums.py index 1e8dd3f18..f7cee6f99 100644 --- a/music_assistant/server/controllers/media/albums.py +++ b/music_assistant/server/controllers/media/albums.py @@ -24,7 +24,14 @@ MediaType, Track, ) -from music_assistant.constants import DB_TABLE_ALBUM_TRACKS, DB_TABLE_ALBUMS, DB_TABLE_TRACKS +from music_assistant.constants import ( + DB_TABLE_ALBUM_ARTISTS, + DB_TABLE_ALBUM_TRACKS, + DB_TABLE_ALBUMS, + DB_TABLE_ARTISTS, + DB_TABLE_PROVIDER_MAPPINGS, + DB_TABLE_TRACKS, +) from music_assistant.server.controllers.media.base import MediaControllerBase from music_assistant.server.helpers.compare import ( compare_album, @@ -47,6 +54,35 @@ def __init__(self, *args, **kwargs) -> None: """Initialize class.""" super().__init__(*args, **kwargs) self._db_add_lock = asyncio.Lock() + self.base_query = f""" + SELECT + {self.db_table}.*, + {DB_TABLE_ARTISTS}.sort_name AS sort_artist, + json_group_array( + json_object( + 'item_id', {DB_TABLE_PROVIDER_MAPPINGS}.provider_item_id, + 'provider_domain', {DB_TABLE_PROVIDER_MAPPINGS}.provider_domain, + 'provider_instance', {DB_TABLE_PROVIDER_MAPPINGS}.provider_instance, + 'available', {DB_TABLE_PROVIDER_MAPPINGS}.available, + 'url', {DB_TABLE_PROVIDER_MAPPINGS}.url, + 'audio_format', json({DB_TABLE_PROVIDER_MAPPINGS}.audio_format), + 'details', {DB_TABLE_PROVIDER_MAPPINGS}.details + )) as {DB_TABLE_PROVIDER_MAPPINGS}, + json_group_array( + json_object( + 'item_id', {DB_TABLE_ARTISTS}.item_id, + 'provider', 'library', + 'name', {DB_TABLE_ARTISTS}.name, + 'sort_name', {DB_TABLE_ARTISTS}.sort_name, + 'media_type', 'artist' + )) as {DB_TABLE_ARTISTS} + FROM {self.db_table} + LEFT JOIN {DB_TABLE_ALBUM_ARTISTS} on {DB_TABLE_ALBUM_ARTISTS}.album_id = {self.db_table}.item_id + LEFT JOIN {DB_TABLE_ARTISTS} on {DB_TABLE_ARTISTS}.item_id = {DB_TABLE_ALBUM_ARTISTS}.artist_id + LEFT JOIN {DB_TABLE_PROVIDER_MAPPINGS} + ON {self.db_table}.item_id = {DB_TABLE_PROVIDER_MAPPINGS}.item_id + AND {DB_TABLE_PROVIDER_MAPPINGS}.media_type == '{self.media_type.value}' + """ # noqa: E501 # register api handlers self.mass.register_api_command("music/albums/library_items", self.library_items) self.mass.register_api_command( @@ -366,20 +402,15 @@ async def _get_db_album_tracks( """Return in-database album tracks for the given database album.""" db_id = int(item_id) # ensure integer db_album = await self.get_library_item(db_id) - result: list[AlbumTrack] = [] - query = ( - f"SELECT * FROM {DB_TABLE_TRACKS} INNER JOIN albumtracks " - "ON albumtracks.track_id = tracks.item_id WHERE albumtracks.album_id = :album_id" + subquery = f"SELECT track_id FROM {DB_TABLE_ALBUM_TRACKS} WHERE album_id = {item_id}" + query = f"WHERE {DB_TABLE_TRACKS}.item_id in ({subquery})" + return sorted( + [ + AlbumTrack.from_track(track, db_album) + async for track in self.mass.music.tracks.iter_library_items(extra_query=query) + ], + key=lambda x: (x.disc_number, x.track_number), ) - track_rows = await self.mass.music.database.get_rows_from_query(query, {"album_id": db_id}) - for album_track_row in track_rows: - album_track = AlbumTrack.from_dict( - self._parse_db_row({**album_track_row, "album": db_album.to_dict()}) - ) - if db_album.metadata.images: - album_track.metadata.images = db_album.metadata.images - result.append(album_track) - return sorted(result, key=lambda x: (x.disc_number, x.track_number)) async def _match(self, db_album: Album) -> None: """Try to find match on all (streaming) providers for the provided (database) album. diff --git a/music_assistant/server/controllers/media/artists.py b/music_assistant/server/controllers/media/artists.py index dfb509d64..c4fee1679 100644 --- a/music_assistant/server/controllers/media/artists.py +++ b/music_assistant/server/controllers/media/artists.py @@ -21,8 +21,10 @@ Track, ) from music_assistant.constants import ( + DB_TABLE_ALBUM_ARTISTS, DB_TABLE_ALBUMS, DB_TABLE_ARTISTS, + DB_TABLE_TRACK_ARTISTS, DB_TABLE_TRACKS, VARIOUS_ARTISTS_ID_MBID, VARIOUS_ARTISTS_NAME, @@ -300,8 +302,8 @@ async def get_library_artist_tracks( item_id: str | int, ) -> list[Track]: """Return all tracks for an artist in the library.""" - # TODO: adjust to json query instead of text search? - query = f"WHERE tracks.artists LIKE '%\"{item_id}\"%'" + subquery = f"SELECT track_id FROM {DB_TABLE_TRACK_ARTISTS} WHERE artist_id = {item_id}" + query = f"WHERE {DB_TABLE_TRACKS}.item_id in ({subquery})" paged_list = await self.mass.music.tracks.library_items(extra_query=query) return paged_list.items @@ -347,8 +349,8 @@ async def get_library_artist_albums( item_id: str | int, ) -> list[Album]: """Return all in-library albums for an artist.""" - # TODO: adjust to json query instead of text search? - query = f"WHERE albums.artists LIKE '%\"{item_id}\"%'" + subquery = f"SELECT album_id FROM {DB_TABLE_ALBUM_ARTISTS} WHERE artist_id = {item_id}" + query = f"WHERE {DB_TABLE_ALBUMS}.item_id in ({subquery})" paged_list = await self.mass.music.albums.library_items(extra_query=query) return paged_list.items diff --git a/music_assistant/server/controllers/media/base.py b/music_assistant/server/controllers/media/base.py index 30bd1e8ff..36e35ad77 100644 --- a/music_assistant/server/controllers/media/base.py +++ b/music_assistant/server/controllers/media/base.py @@ -35,7 +35,7 @@ ItemCls = TypeVar("ItemCls", bound="MediaItemType") REFRESH_INTERVAL = 60 * 60 * 24 * 30 -JSON_KEYS = ("artists", "metadata", "provider_mappings", "external_ids") +JSON_KEYS = ("artists", "album", "albums", "metadata", "provider_mappings", "external_ids") class MediaControllerBase(Generic[ItemCls], metaclass=ABCMeta): @@ -48,7 +48,25 @@ class MediaControllerBase(Generic[ItemCls], metaclass=ABCMeta): def __init__(self, mass: MusicAssistant) -> None: """Initialize class.""" self.mass = mass - self.base_query = f"SELECT * FROM {self.db_table}" + self.base_query = f""" + SELECT + {self.db_table}.*, + json_group_array( + json_object( + 'item_id', {DB_TABLE_PROVIDER_MAPPINGS}.provider_item_id, + 'provider_domain', {DB_TABLE_PROVIDER_MAPPINGS}.provider_domain, + 'provider_instance', {DB_TABLE_PROVIDER_MAPPINGS}.provider_instance, + 'available', {DB_TABLE_PROVIDER_MAPPINGS}.available, + 'url', {DB_TABLE_PROVIDER_MAPPINGS}.url, + 'audio_format', json({DB_TABLE_PROVIDER_MAPPINGS}.audio_format), + 'details', {DB_TABLE_PROVIDER_MAPPINGS}.details + )) as {DB_TABLE_PROVIDER_MAPPINGS} + FROM {self.db_table} + LEFT JOIN {DB_TABLE_PROVIDER_MAPPINGS} + ON {self.db_table}.item_id = {DB_TABLE_PROVIDER_MAPPINGS}.item_id + AND {DB_TABLE_PROVIDER_MAPPINGS}.media_type == '{self.media_type.value}' + """ + self.sql_group_by = f"{self.db_table}.item_id" self.logger = logging.getLogger(f"{MASS_LOGGER_NAME}.music.{self.media_type.value}") @abstractmethod @@ -109,7 +127,6 @@ async def library_items( query_parts.append( f"({self.db_table}.name LIKE :search " f" OR {self.db_table}.sort_name LIKE :search" - f" OR {self.db_table}.artists LIKE :search)" ) else: query_parts.append(f"{self.db_table}.name LIKE :search") @@ -119,7 +136,7 @@ async def library_items( if query_parts: # concetenate all where queries sql_query += " WHERE " + " AND ".join(query_parts) - sql_query += f" ORDER BY {order_by}" + sql_query += f" GROUP BY {self.sql_group_by} ORDER BY {order_by}" items = await self._get_library_items_by_query( sql_query, params, limit=limit, offset=offset ) @@ -135,6 +152,8 @@ async def iter_library_items( favorite: bool | None = None, search: str | None = None, order_by: str = "sort_name", + extra_query: str | None = None, + extra_query_params: dict[str, Any] | None = None, ) -> AsyncGenerator[ItemCls, None]: """Iterate all in-database items.""" limit: int = 500 @@ -146,6 +165,8 @@ async def iter_library_items( limit=limit, offset=offset, order_by=order_by, + extra_query=extra_query, + extra_query_params=extra_query_params, ) for item in next_items.items: yield item @@ -232,10 +253,7 @@ async def search( # create safe search string search_query = search_query.replace("/", " ").replace("'", "") if provider_instance_id_or_domain == "library": - return [ - self.item_cls.from_dict(self._parse_db_row(db_row)) - for db_row in await self.mass.music.database.search(self.db_table, search_query) - ] + return [item async for item in await self.iter_library_items(search=search_query)] prov = self.mass.get_provider(provider_instance_id_or_domain) if prov is None: return [] @@ -293,11 +311,11 @@ async def get_provider_mapping(self, item: ItemCls) -> tuple[str, str]: return (None, None) async def get_library_item(self, item_id: int | str) -> ItemCls: - """Get record by id.""" + """Get single library item by id.""" db_id = int(item_id) # ensure integer - match = {"item_id": db_id} - if db_row := await self.mass.music.database.get_row(self.db_table, match): - return self.item_cls.from_dict(self._parse_db_row(db_row)) + extra_query = f"WHERE {self.db_table}.item_id is {item_id}" + async for db_item in self.iter_library_items(extra_query=extra_query): + return db_item msg = f"{self.media_type.value} not found in library: {db_id}" raise MediaNotFoundError(msg) @@ -741,26 +759,16 @@ def _parse_db_row(db_row: Mapping) -> dict[str, Any]: """Parse raw db Mapping into a dict.""" db_row_dict = dict(db_row) db_row_dict["provider"] = "library" + for key in JSON_KEYS: if key in db_row_dict and db_row_dict[key] not in (None, ""): db_row_dict[key] = json_loads(db_row_dict[key]) + if "favorite" in db_row_dict: db_row_dict["favorite"] = bool(db_row_dict["favorite"]) if "item_id" in db_row_dict: db_row_dict["item_id"] = str(db_row_dict["item_id"]) - if "album" not in db_row_dict and (album_id := db_row_dict.get("album_id")): - # handle joined result with (limited) album data as ItemMapping - db_row_dict["album"] = { - "media_type": "album", - "item_id": str(album_id), - "provider": "library", - "name": db_row_dict["album_name"], - "version": db_row_dict["album_version"], - } - db_row_dict["album"] = ItemMapping.from_dict(db_row_dict["album"]) - if db_row_dict["album_metadata"]: - # copy album image - album_metadata = json_loads(db_row_dict["album_metadata"]) - if album_metadata and album_metadata["images"]: - db_row_dict["metadata"]["images"] = album_metadata["images"] + # copy album image to itemmapping single image + if "album" in db_row and (images := db_row.get("images")): + db_row["album"]["image"] = next((x for x in images if x["type"] == "thumb"), None) return db_row_dict diff --git a/music_assistant/server/controllers/media/tracks.py b/music_assistant/server/controllers/media/tracks.py index bd8047c1b..95cc70ec3 100644 --- a/music_assistant/server/controllers/media/tracks.py +++ b/music_assistant/server/controllers/media/tracks.py @@ -16,7 +16,14 @@ UnsupportedFeaturedException, ) from music_assistant.common.models.media_items import Album, ItemMapping, Track -from music_assistant.constants import DB_TABLE_ALBUM_TRACKS, DB_TABLE_TRACKS +from music_assistant.constants import ( + DB_TABLE_ALBUM_TRACKS, + DB_TABLE_ALBUMS, + DB_TABLE_ARTISTS, + DB_TABLE_PROVIDER_MAPPINGS, + DB_TABLE_TRACK_ARTISTS, + DB_TABLE_TRACKS, +) from music_assistant.server.helpers.compare import ( compare_artists, compare_track, @@ -36,13 +43,50 @@ class TracksController(MediaControllerBase[Track]): def __init__(self, *args, **kwargs) -> None: """Initialize class.""" super().__init__(*args, **kwargs) - self.base_query = ( - "SELECT tracks.*, albums.item_id as album_id, " - "albums.name AS album_name, albums.version as album_version, " - "albums.metadata as album_metadata FROM tracks " - "LEFT JOIN albumtracks on albumtracks.track_id = tracks.item_id " - "LEFT JOIN albums on albums.item_id = albumtracks.album_id" - ) + self.base_query = f""" + SELECT + {self.db_table}.*, + {DB_TABLE_ARTISTS}.sort_name AS sort_artist, + {DB_TABLE_ARTISTS}.sort_name AS sort_album, + json_group_array( + json_object( + 'item_id', {DB_TABLE_PROVIDER_MAPPINGS}.provider_item_id, + 'provider_domain', {DB_TABLE_PROVIDER_MAPPINGS}.provider_domain, + 'provider_instance', {DB_TABLE_PROVIDER_MAPPINGS}.provider_instance, + 'available', {DB_TABLE_PROVIDER_MAPPINGS}.available, + 'url', {DB_TABLE_PROVIDER_MAPPINGS}.url, + 'audio_format', json({DB_TABLE_PROVIDER_MAPPINGS}.audio_format), + 'details', {DB_TABLE_PROVIDER_MAPPINGS}.details + )) as {DB_TABLE_PROVIDER_MAPPINGS}, + json_group_array( + json_object( + 'item_id', {DB_TABLE_ARTISTS}.item_id, + 'provider', 'library', + 'name', {DB_TABLE_ARTISTS}.name, + 'sort_name', {DB_TABLE_ARTISTS}.sort_name, + 'media_type', 'artist' + )) as {DB_TABLE_ARTISTS}, + json_object( + 'item_id', {DB_TABLE_ALBUMS}.item_id, + 'provider', 'library', + 'name', {DB_TABLE_ALBUMS}.name, + 'sort_name', {DB_TABLE_ALBUMS}.sort_name, + 'version', {DB_TABLE_ALBUMS}.version, + 'images', json_extract({DB_TABLE_ALBUMS}.metadata, '$.images'), + 'media_type', 'artist' + ) as album, + {DB_TABLE_ALBUM_TRACKS}.disc_number, + {DB_TABLE_ALBUM_TRACKS}.track_number + FROM {self.db_table} + LEFT JOIN {DB_TABLE_TRACK_ARTISTS} on {DB_TABLE_TRACK_ARTISTS}.track_id = {self.db_table}.item_id + LEFT JOIN {DB_TABLE_ARTISTS} on {DB_TABLE_ARTISTS}.item_id = {DB_TABLE_TRACK_ARTISTS}.artist_id + LEFT JOIN {DB_TABLE_ALBUM_TRACKS} on {DB_TABLE_ALBUM_TRACKS}.track_id = {self.db_table}.item_id + LEFT JOIN {DB_TABLE_ALBUMS} on {DB_TABLE_ALBUMS}.item_id = {DB_TABLE_ALBUM_TRACKS}.album_id + LEFT JOIN {DB_TABLE_PROVIDER_MAPPINGS} + ON {self.db_table}.item_id = {DB_TABLE_PROVIDER_MAPPINGS}.item_id + AND {DB_TABLE_PROVIDER_MAPPINGS}.media_type == '{self.media_type.value}' + """ # noqa: E501 + self.sql_group_by = f"{self.db_table}.item_id, {DB_TABLE_ALBUMS}.item_id" self._db_add_lock = asyncio.Lock() # register api handlers self.mass.register_api_command("music/tracks/library_items", self.library_items) diff --git a/music_assistant/server/controllers/music.py b/music_assistant/server/controllers/music.py index c31ad3c11..4a9add29f 100644 --- a/music_assistant/server/controllers/music.py +++ b/music_assistant/server/controllers/music.py @@ -11,6 +11,7 @@ from typing import TYPE_CHECKING from music_assistant.common.helpers.datetime import utc_timestamp +from music_assistant.common.helpers.json import json_dumps, json_loads from music_assistant.common.helpers.uri import parse_uri from music_assistant.common.models.config_entries import ConfigEntry, ConfigValueType from music_assistant.common.models.enums import ( @@ -32,6 +33,7 @@ from music_assistant.common.models.streamdetails import LoudnessMeasurement from music_assistant.constants import ( DB_SCHEMA_VERSION, + DB_TABLE_ALBUM_ARTISTS, DB_TABLE_ALBUM_TRACKS, DB_TABLE_ALBUMS, DB_TABLE_ARTISTS, @@ -40,6 +42,7 @@ DB_TABLE_PROVIDER_MAPPINGS, DB_TABLE_RADIOS, DB_TABLE_SETTINGS, + DB_TABLE_TRACK_ARTISTS, DB_TABLE_TRACK_LOUDNESS, DB_TABLE_TRACKS, PROVIDERS_WITH_SHAREABLE_URLS, @@ -746,8 +749,8 @@ async def _setup_database(self) -> None: db_path_backup = db_path + ".backup" await asyncio.to_thread(shutil.copyfile, db_path, db_path_backup) - # handle db migration from previous schema to this one - if prev_version in (27, 28): + # handle db migration from previous schema(s) to this one + if prev_version in (27, 28, 29): self.logger.info( "Performing database migration from %s to %s", prev_version, @@ -755,9 +758,78 @@ async def _setup_database(self) -> None: ) self.logger.warning("DATABASE MIGRATION IN PROGRESS - THIS CAN TAKE A WHILE") - # migrate loudness measurements table - await self.database.execute(f"DROP TABLE IF EXISTS {DB_TABLE_TRACK_LOUDNESS}") - await self.__create_database_tables() + # recreate loudness measurements table + if prev_version in (27, 28): + await self.database.execute(f"DROP TABLE IF EXISTS {DB_TABLE_TRACK_LOUDNESS}") + await self.__create_database_tables() + + # # migrate track artists + async for db_track in self.database.iter_items(DB_TABLE_TRACKS): + for track_artist in json_loads(db_track["artists"]): + await self.database.insert_or_replace( + DB_TABLE_TRACK_ARTISTS, + { + "track_id": db_track["item_id"], + "artist_id": int(track_artist["item_id"]), + }, + ) + await self.database.execute(f"ALTER TABLE {DB_TABLE_TRACKS} DROP COLUMN artists;") + await self.database.execute( + f"ALTER TABLE {DB_TABLE_TRACKS} DROP COLUMN sort_artist;" + ) + + # # migrate album artists + async for db_album in self.database.iter_items(DB_TABLE_ALBUMS): + for album_artist in json_loads(db_album["artists"]): + await self.database.insert_or_replace( + DB_TABLE_ALBUM_ARTISTS, + { + "album_id": db_album["item_id"], + "artist_id": int(album_artist["item_id"]), + }, + ) + await self.database.execute(f"ALTER TABLE {DB_TABLE_ALBUMS} DROP COLUMN artists;") + await self.database.execute( + f"ALTER TABLE {DB_TABLE_ALBUMS} DROP COLUMN sort_artist;" + ) + + # migrate provider_mappings + await self.database.execute( + f"ALTER TABLE {DB_TABLE_PROVIDER_MAPPINGS} ADD [available] BOOLEAN DEFAULT 1;" + ) + await self.database.execute( + f"ALTER TABLE {DB_TABLE_PROVIDER_MAPPINGS} ADD [url] TEXT;" + ) + await self.database.execute( + f"ALTER TABLE {DB_TABLE_PROVIDER_MAPPINGS} ADD [audio_format] json;" + ) + await self.database.execute( + f"ALTER TABLE {DB_TABLE_PROVIDER_MAPPINGS} ADD [details] json;" + ) + + for media_type_str in ("track", "album", "artist", "playlist", "radio"): + table = f"{media_type_str}s" + async for db_item in self.database.iter_items(table): + for db_prov_map in json_loads(db_item["provider_mappings"]): + await self.database.insert_or_replace( + DB_TABLE_PROVIDER_MAPPINGS, + { + "media_type": media_type_str, + "item_id": int(db_item["item_id"]), + "provider_domain": db_prov_map["provider_domain"], + "provider_instance": db_prov_map["provider_instance"], + "provider_item_id": db_prov_map["item_id"], + "available": db_prov_map["available"], + "url": db_prov_map["url"], + "audio_format": json_dumps(db_prov_map["audio_format"]) + if db_prov_map["audio_format"] + else None, + "details": db_prov_map["details"], + }, + ) + await self.database.execute( + f"ALTER TABLE {table} DROP COLUMN provider_mappings;" + ) # db migration succeeded self.logger.info( @@ -777,7 +849,6 @@ async def _setup_database(self) -> None: DB_TABLE_TRACKS, DB_TABLE_ALBUMS, DB_TABLE_ARTISTS, - DB_TABLE_TRACKS, DB_TABLE_PLAYLISTS, DB_TABLE_RADIOS, DB_TABLE_PROVIDER_MAPPINGS, @@ -831,12 +902,10 @@ async def __create_database_tables(self) -> None: item_id INTEGER PRIMARY KEY AUTOINCREMENT, name TEXT NOT NULL, sort_name TEXT NOT NULL, - sort_artist TEXT, album_type TEXT NOT NULL, year INTEGER, version TEXT, favorite BOOLEAN DEFAULT 0, - artists json NOT NULL, metadata json NOT NULL, provider_mappings json NOT NULL, external_ids json NOT NULL, @@ -862,11 +931,9 @@ async def __create_database_tables(self) -> None: item_id INTEGER PRIMARY KEY AUTOINCREMENT, name TEXT NOT NULL, sort_name TEXT NOT NULL, - sort_artist TEXT, version TEXT, duration INTEGER, favorite BOOLEAN DEFAULT 0, - artists json NOT NULL, metadata json NOT NULL, provider_mappings json NOT NULL, external_ids json NOT NULL, @@ -876,13 +943,34 @@ async def __create_database_tables(self) -> None: ) await self.database.execute( f"""CREATE TABLE IF NOT EXISTS {DB_TABLE_ALBUM_TRACKS}( - track_id INTEGER NOT NULL, - album_id INTEGER NOT NULL, - disc_number INTEGER NOT NULL, - track_number INTEGER NOT NULL, + [id] INTEGER PRIMARY KEY AUTOINCREMENT, + [track_id] INTEGER NOT NULL, + [album_id] INTEGER NOT NULL, + [disc_number] INTEGER NOT NULL, + [track_number] INTEGER NOT NULL, UNIQUE(track_id, album_id) );""" ) + await self.database.execute( + f"""CREATE TABLE IF NOT EXISTS {DB_TABLE_TRACK_ARTISTS}( + [id] INTEGER PRIMARY KEY AUTOINCREMENT, + [track_id] INTEGER NOT NULL, + [artist_id] INTEGER NOT NULL, + FOREIGN KEY([track_id]) REFERENCES [tracks]([item_id]), + FOREIGN KEY([artist_id]) REFERENCES [artists]([item_id]), + UNIQUE(track_id, artist_id) + );""" + ) + await self.database.execute( + f"""CREATE TABLE IF NOT EXISTS {DB_TABLE_ALBUM_ARTISTS}( + [id] INTEGER PRIMARY KEY AUTOINCREMENT, + [album_id] INTEGER NOT NULL, + [artist_id] INTEGER NOT NULL, + FOREIGN KEY([album_id]) REFERENCES [albums]([item_id]), + FOREIGN KEY([artist_id]) REFERENCES [artists]([item_id]), + UNIQUE(album_id, artist_id) + );""" + ) await self.database.execute( f"""CREATE TABLE IF NOT EXISTS {DB_TABLE_PLAYLISTS}( item_id INTEGER PRIMARY KEY AUTOINCREMENT, @@ -913,11 +1001,16 @@ async def __create_database_tables(self) -> None: ) await self.database.execute( f"""CREATE TABLE IF NOT EXISTS {DB_TABLE_PROVIDER_MAPPINGS}( - media_type TEXT NOT NULL, - item_id INTEGER NOT NULL, - provider_domain TEXT NOT NULL, - provider_instance TEXT NOT NULL, - provider_item_id TEXT NOT NULL, + [id] INTEGER PRIMARY KEY AUTOINCREMENT, + [media_type] TEXT NOT NULL, + [item_id] INTEGER NOT NULL, + [provider_domain] TEXT NOT NULL, + [provider_instance] TEXT NOT NULL, + [provider_item_id] TEXT NOT NULL, + [available] BOOLEAN DEFAULT 1, + [url] text, + [audio_format] json, + [details] json, UNIQUE(media_type, provider_instance, provider_item_id) );""" ) diff --git a/music_assistant/server/providers/filesystem_local/base.py b/music_assistant/server/providers/filesystem_local/base.py index e289c5da2..683c49868 100644 --- a/music_assistant/server/providers/filesystem_local/base.py +++ b/music_assistant/server/providers/filesystem_local/base.py @@ -230,34 +230,32 @@ async def search( # instead we make some (slow) freaking queries to the db ;-) params = { "name": f"%{search_query}%", - "provider_instance": f"%{self.instance_id}%", + "provider_instance": self.instance_id, } # ruff: noqa: E501 if media_types is None or MediaType.TRACK in media_types: - query = ( - "WHERE tracks.name LIKE :name AND tracks.provider_mappings LIKE :provider_instance" - ) + query = "WHERE tracks.name LIKE :name AND provider_mappings.provider_instance = :provider_instance" result.tracks = ( await self.mass.music.tracks.library_items( extra_query=query, extra_query_params=params ) ).items if media_types is None or MediaType.ALBUM in media_types: - query = "WHERE name LIKE :name AND provider_mappings LIKE :provider_instance" + query = "WHERE albums.name LIKE :name AND provider_mappings.provider_instance = :provider_instance" result.albums = ( await self.mass.music.albums.library_items( extra_query=query, extra_query_params=params ) ).items if media_types is None or MediaType.ARTIST in media_types: - query = "WHERE name LIKE :name AND provider_mappings LIKE :provider_instance" + query = "WHERE artists.name LIKE :name AND provider_mappings.provider_instance = :provider_instance" result.artists = ( await self.mass.music.artists.library_items( extra_query=query, extra_query_params=params ) ).items if media_types is None or MediaType.PLAYLIST in media_types: - query = "WHERE name LIKE :name AND provider_mappings LIKE :provider_instance" + query = "WHERE playlists.name LIKE :name AND provider_mappings.provider_instance = :provider_instance" result.playlists = ( await self.mass.music.playlists.library_items( extra_query=query, extra_query_params=params From c9d4c40d2202d3c08121e9409d2b7c7b7226e69c Mon Sep 17 00:00:00 2001 From: Marcel van der Veldt Date: Thu, 18 Apr 2024 20:32:45 +0200 Subject: [PATCH 3/6] fix remaining queries --- .../server/controllers/media/albums.py | 75 +++++++-- .../server/controllers/media/artists.py | 7 +- .../server/controllers/media/base.py | 141 +++++------------ .../server/controllers/media/playlists.py | 5 +- .../server/controllers/media/radio.py | 5 +- .../server/controllers/media/tracks.py | 148 ++++++++++++------ .../server/providers/filesystem_local/base.py | 8 +- 7 files changed, 210 insertions(+), 179 deletions(-) diff --git a/music_assistant/server/controllers/media/albums.py b/music_assistant/server/controllers/media/albums.py index f7cee6f99..2ba79d246 100644 --- a/music_assistant/server/controllers/media/albums.py +++ b/music_assistant/server/controllers/media/albums.py @@ -20,6 +20,7 @@ Album, AlbumTrack, AlbumType, + Artist, ItemMapping, MediaType, Track, @@ -67,7 +68,7 @@ def __init__(self, *args, **kwargs) -> None: 'url', {DB_TABLE_PROVIDER_MAPPINGS}.url, 'audio_format', json({DB_TABLE_PROVIDER_MAPPINGS}.audio_format), 'details', {DB_TABLE_PROVIDER_MAPPINGS}.details - )) as {DB_TABLE_PROVIDER_MAPPINGS}, + )) filter ( where {DB_TABLE_PROVIDER_MAPPINGS}.item_id is not null) as {DB_TABLE_PROVIDER_MAPPINGS}, json_group_array( json_object( 'item_id', {DB_TABLE_ARTISTS}.item_id, @@ -75,7 +76,7 @@ def __init__(self, *args, **kwargs) -> None: 'name', {DB_TABLE_ARTISTS}.name, 'sort_name', {DB_TABLE_ARTISTS}.sort_name, 'media_type', 'artist' - )) as {DB_TABLE_ARTISTS} + )) filter ( where {DB_TABLE_ARTISTS}.name is not null) as {DB_TABLE_ARTISTS} FROM {self.db_table} LEFT JOIN {DB_TABLE_ALBUM_ARTISTS} on {DB_TABLE_ALBUM_ARTISTS}.album_id = {self.db_table}.item_id LEFT JOIN {DB_TABLE_ARTISTS} on {DB_TABLE_ARTISTS}.item_id = {DB_TABLE_ALBUM_ARTISTS}.artist_id @@ -204,13 +205,10 @@ async def update_item_in_library( db_id = int(item_id) # ensure integer cur_item = await self.get_library_item(db_id) metadata = cur_item.metadata.update(update.metadata, overwrite) - provider_mappings = self._get_provider_mappings(cur_item, update) - album_artists = await self._get_artist_mappings(cur_item, update, overwrite) if getattr(update, "album_type", AlbumType.UNKNOWN) != AlbumType.UNKNOWN: album_type = update.album_type else: album_type = cur_item.album_type - sort_artist = album_artists[0].sort_name cur_item.external_ids.update(update.external_ids) await self.mass.music.database.update( self.db_table, @@ -220,13 +218,10 @@ async def update_item_in_library( "sort_name": update.sort_name if overwrite else cur_item.sort_name or update.sort_name, - "sort_artist": sort_artist, "version": update.version if overwrite else cur_item.version, "year": update.year if overwrite else cur_item.year or update.year, "album_type": album_type.value, - "artists": serialize_to_json(album_artists), "metadata": serialize_to_json(metadata), - "provider_mappings": serialize_to_json(provider_mappings), "external_ids": serialize_to_json( update.external_ids if overwrite else cur_item.external_ids ), @@ -234,7 +229,10 @@ async def update_item_in_library( }, ) # update/set provider_mappings table - await self._set_provider_mappings(db_id, provider_mappings) + await self._set_provider_mappings(db_id, update.provider_mappings, overwrite=overwrite) + # set album artist(s) + await self._set_album_artists(db_id, update.artists, overwrite=overwrite) + self.logger.debug("updated %s in database: %s", update.name, db_id) # get full created object library_item = await self.get_library_item(db_id) @@ -296,8 +294,6 @@ async def versions( async def _add_library_item(self, item: Album) -> Album: """Add a new record to the database.""" - album_artists = await self._get_artist_mappings(item) - sort_artist = album_artists[0].sort_name if album_artists else "" new_item = await self.mass.music.database.insert( self.db_table, { @@ -308,9 +304,6 @@ async def _add_library_item(self, item: Album) -> Album: "album_type": item.album_type, "year": item.year, "metadata": serialize_to_json(item.metadata), - "provider_mappings": serialize_to_json(item.provider_mappings), - "artists": serialize_to_json(album_artists), - "sort_artist": sort_artist, "external_ids": serialize_to_json(item.external_ids), "timestamp_added": int(utc_timestamp()), "timestamp_modified": int(utc_timestamp()), @@ -319,6 +312,8 @@ async def _add_library_item(self, item: Album) -> Album: db_id = new_item["item_id"] # update/set provider_mappings table await self._set_provider_mappings(db_id, item.provider_mappings) + # set album artist(s) + await self._set_album_artists(db_id, item.artists) self.logger.debug("added %s to database", item.name) # return the full item we just added return await self.get_library_item(db_id) @@ -472,3 +467,55 @@ async def find_prov_match(provider: MusicProvider): db_album.name, provider.name, ) + + async def _set_album_artists( + self, db_id: int, artists: list[Artist | ItemMapping], overwrite: bool + ) -> None: + """Store Album Artists.""" + if overwrite: + # on overwrite, clear the album_artists table first + await self.mass.music.database.delete( + DB_TABLE_ALBUM_ARTISTS, + { + "album_id": db_id, + }, + ) + for artist in artists: + await self._set_album_artist(db_id, artist=artist) + + async def _set_album_artist(self, db_id: int, artist: Artist | ItemMapping) -> None: + """Store Album Artist info.""" + db_artist: Album | ItemMapping = None + if artist.provider == "library": + db_artist = artist + elif existing := await self.mass.music.artists.get_library_item_by_prov_id( + artist.item_id, artist.provider + ): + db_artist = existing + else: + # not an existing artist, we need to fetch before we can add it to the library + if isinstance(artist, ItemMapping): + artist = await self.mass.music.artists.get_provider_item( + artist.item_id, artist.provider, fallback=artist + ) + with contextlib.suppress(MediaNotFoundError, AssertionError, InvalidDataError): + db_artist = await self.mass.music.artists.add_item_to_library( + artist, metadata_lookup=False + ) + if not db_artist: + # this should not happen but streaming providers can be awful sometimes + self.logger.warning( + "Unable to resolve Artist %s for album %s, " + "album will be added to the library without this artist!", + artist.uri, + db_id, + ) + return + # write (or update) record in album_artists table + await self.mass.music.database.insert_or_replace( + DB_TABLE_ALBUM_ARTISTS, + { + "album_id": db_id, + "artist_id": int(db_artist.item_id), + }, + ) diff --git a/music_assistant/server/controllers/media/artists.py b/music_assistant/server/controllers/media/artists.py index c4fee1679..24328cd8d 100644 --- a/music_assistant/server/controllers/media/artists.py +++ b/music_assistant/server/controllers/media/artists.py @@ -120,7 +120,6 @@ async def update_item_in_library( db_id = int(item_id) # ensure integer cur_item = await self.get_library_item(db_id) metadata = cur_item.metadata.update(getattr(update, "metadata", None), overwrite) - provider_mappings = self._get_provider_mappings(cur_item, update) cur_item.external_ids.update(update.external_ids) # enforce various artists name + id mbid = cur_item.mbid @@ -141,12 +140,11 @@ async def update_item_in_library( update.external_ids if overwrite else cur_item.external_ids ), "metadata": serialize_to_json(metadata), - "provider_mappings": serialize_to_json(provider_mappings), "timestamp_modified": int(utc_timestamp()), }, ) - # update/set provider_mappings table - await self._set_provider_mappings(db_id, provider_mappings) + ## update/set provider_mappings table + await self._set_provider_mappings(db_id, update.provider_mappings, overwrite=overwrite) self.logger.debug("updated %s in database: %s", update.name, db_id) # get full created object library_item = await self.get_library_item(db_id) @@ -372,7 +370,6 @@ async def _add_library_item(self, item: Artist) -> Artist: "favorite": item.favorite, "external_ids": serialize_to_json(item.external_ids), "metadata": serialize_to_json(item.metadata), - "provider_mappings": serialize_to_json(item.provider_mappings), "timestamp_added": int(utc_timestamp()), "timestamp_modified": int(utc_timestamp()), }, diff --git a/music_assistant/server/controllers/media/base.py b/music_assistant/server/controllers/media/base.py index 36e35ad77..b166c052e 100644 --- a/music_assistant/server/controllers/media/base.py +++ b/music_assistant/server/controllers/media/base.py @@ -8,16 +8,11 @@ from time import time from typing import TYPE_CHECKING, Any, Generic, TypeVar -from music_assistant.common.helpers.json import json_loads, serialize_to_json +from music_assistant.common.helpers.json import json_dumps, json_loads, serialize_to_json from music_assistant.common.models.enums import EventType, ExternalID, MediaType, ProviderFeature -from music_assistant.common.models.errors import ( - InvalidDataError, - MediaNotFoundError, - ProviderUnavailableError, -) +from music_assistant.common.models.errors import MediaNotFoundError, ProviderUnavailableError from music_assistant.common.models.media_items import ( Album, - Artist, ItemMapping, MediaItemType, PagedItems, @@ -49,23 +44,23 @@ def __init__(self, mass: MusicAssistant) -> None: """Initialize class.""" self.mass = mass self.base_query = f""" - SELECT - {self.db_table}.*, - json_group_array( - json_object( - 'item_id', {DB_TABLE_PROVIDER_MAPPINGS}.provider_item_id, - 'provider_domain', {DB_TABLE_PROVIDER_MAPPINGS}.provider_domain, - 'provider_instance', {DB_TABLE_PROVIDER_MAPPINGS}.provider_instance, - 'available', {DB_TABLE_PROVIDER_MAPPINGS}.available, - 'url', {DB_TABLE_PROVIDER_MAPPINGS}.url, - 'audio_format', json({DB_TABLE_PROVIDER_MAPPINGS}.audio_format), - 'details', {DB_TABLE_PROVIDER_MAPPINGS}.details - )) as {DB_TABLE_PROVIDER_MAPPINGS} - FROM {self.db_table} - LEFT JOIN {DB_TABLE_PROVIDER_MAPPINGS} - ON {self.db_table}.item_id = {DB_TABLE_PROVIDER_MAPPINGS}.item_id - AND {DB_TABLE_PROVIDER_MAPPINGS}.media_type == '{self.media_type.value}' - """ + SELECT + {self.db_table}.*, + json_group_array( + json_object( + 'item_id', {DB_TABLE_PROVIDER_MAPPINGS}.provider_item_id, + 'provider_domain', {DB_TABLE_PROVIDER_MAPPINGS}.provider_domain, + 'provider_instance', {DB_TABLE_PROVIDER_MAPPINGS}.provider_instance, + 'available', {DB_TABLE_PROVIDER_MAPPINGS}.available, + 'url', {DB_TABLE_PROVIDER_MAPPINGS}.url, + 'audio_format', json({DB_TABLE_PROVIDER_MAPPINGS}.audio_format), + 'details', {DB_TABLE_PROVIDER_MAPPINGS}.details + )) filter ( where {DB_TABLE_PROVIDER_MAPPINGS}.item_id is not null) as {DB_TABLE_PROVIDER_MAPPINGS} + FROM {self.db_table} + LEFT JOIN {DB_TABLE_PROVIDER_MAPPINGS} + ON {self.db_table}.item_id = {DB_TABLE_PROVIDER_MAPPINGS}.item_id + AND {DB_TABLE_PROVIDER_MAPPINGS}.media_type == '{self.media_type.value}' + """ # noqa: E501 self.sql_group_by = f"{self.db_table}.item_id" self.logger = logging.getLogger(f"{MASS_LOGGER_NAME}.music.{self.media_type.value}") @@ -125,8 +120,7 @@ async def library_items( params["search"] = f"%{search}%" if self.media_type in (MediaType.ALBUM, MediaType.TRACK): query_parts.append( - f"({self.db_table}.name LIKE :search " - f" OR {self.db_table}.sort_name LIKE :search" + f"({self.db_table}.name LIKE :search OR {self.db_table}.sort_name LIKE :search)" ) else: query_parts.append(f"{self.db_table}.name LIKE :search") @@ -658,102 +652,41 @@ async def _get_library_items_by_query( ] async def _set_provider_mappings( - self, item_id: str | int, provider_mappings: Iterable[ProviderMapping] + self, + item_id: str | int, + provider_mappings: Iterable[ProviderMapping], + overwrite: bool = False, ) -> None: """Update the provider_items table for the media item.""" db_id = int(item_id) # ensure integer - # get current mappings (if any) - cur_mappings: set[ProviderMapping] = set() - match = {"media_type": self.media_type.value, "item_id": db_id} - for db_row in await self.mass.music.database.get_rows(DB_TABLE_PROVIDER_MAPPINGS, match): - cur_mappings.add( - ProviderMapping( - item_id=db_row["provider_item_id"], - provider_domain=db_row["provider_domain"], - provider_instance=db_row["provider_instance"], - ) - ) - # delete removed mappings - for prov_mapping in cur_mappings: - if prov_mapping not in set(provider_mappings): + if overwrite: + # on overwrite, clear the provider_mappings table first + # this is done for filesystem provider changing the path (and thus item_id) + for provider_mapping in provider_mappings: await self.mass.music.database.delete( DB_TABLE_PROVIDER_MAPPINGS, { - **match, - "provider_domain": prov_mapping.provider_domain, - "provider_instance": prov_mapping.provider_instance, - "provider_item_id": prov_mapping.item_id, + "media_type": self.media_type.value, + "item_id": db_id, + "provider_instance": provider_mapping.provider_instance, }, ) - # add entries for provider_mapping in provider_mappings: await self.mass.music.database.insert_or_replace( DB_TABLE_PROVIDER_MAPPINGS, { - **match, + "media_type": self.media_type.value, + "item_id": db_id, "provider_domain": provider_mapping.provider_domain, "provider_instance": provider_mapping.provider_instance, "provider_item_id": provider_mapping.item_id, + "available": provider_mapping.available, + "url": provider_mapping.url, + "audio_format": json_dumps(provider_mapping.audio_format), + "details": provider_mapping.details, }, ) - def _get_provider_mappings( - self, - org_item: ItemCls, - update_item: ItemCls | ItemMapping | None = None, - ) -> set[ProviderMapping]: - """Get/merge provider mappings for an item.""" - if not update_item or isinstance(update_item, ItemMapping): - return org_item.provider_mappings - return {*update_item.provider_mappings, *org_item.provider_mappings} - - async def _get_artist_mappings( - self, - org_item: Album | Track, - update_item: Album | Track | ItemMapping | None = None, - overwrite: bool = False, - ) -> list[ItemMapping]: - """Extract (database) album/track artist(s) as ItemMapping.""" - artist_mappings: list[ItemMapping] = [] - if update_item is None or isinstance(update_item, ItemMapping): - source_artists = org_item.artists - elif overwrite and update_item.artists: - source_artists = update_item.artists - else: - source_artists = org_item.artists + update_item.artists - for artist in source_artists: - artist_mapping = await self._get_artist_mapping(artist) - if artist_mapping not in artist_mappings: - artist_mappings.append(artist_mapping) - return artist_mappings - - async def _get_artist_mapping(self, artist: Artist | ItemMapping) -> ItemMapping: - """Extract (database) track artist as ItemMapping.""" - if artist.provider == "library": - if isinstance(artist, ItemMapping): - return artist - return ItemMapping.from_item(artist) - - if db_artist := await self.mass.music.artists.get_library_item_by_prov_id( - artist.item_id, artist.provider - ): - return ItemMapping.from_item(db_artist) - - # try to request the full item - artist = await self.mass.music.artists.get_provider_item( - artist.item_id, artist.provider, fallback=artist - ) - with suppress(MediaNotFoundError, AssertionError, InvalidDataError): - db_artist = await self.mass.music.artists.add_item_to_library( - artist, metadata_lookup=False - ) - return ItemMapping.from_item(db_artist) - # fallback to just the provider item - # this can happen for unavailable items - if isinstance(artist, ItemMapping): - return artist - return ItemMapping.from_item(artist) - @staticmethod def _parse_db_row(db_row: Mapping) -> dict[str, Any]: """Parse raw db Mapping into a dict.""" diff --git a/music_assistant/server/controllers/media/playlists.py b/music_assistant/server/controllers/media/playlists.py index 9d1505373..7df3c8436 100644 --- a/music_assistant/server/controllers/media/playlists.py +++ b/music_assistant/server/controllers/media/playlists.py @@ -104,7 +104,6 @@ async def update_item_in_library( db_id = int(item_id) # ensure integer cur_item = await self.get_library_item(db_id) metadata = cur_item.metadata.update(getattr(update, "metadata", None), overwrite) - provider_mappings = self._get_provider_mappings(cur_item, update) cur_item.external_ids.update(update.external_ids) await self.mass.music.database.update( self.db_table, @@ -118,7 +117,6 @@ async def update_item_in_library( "owner": update.owner or cur_item.owner, "is_editable": update.is_editable, "metadata": serialize_to_json(metadata), - "provider_mappings": serialize_to_json(provider_mappings), "external_ids": serialize_to_json( update.external_ids if overwrite else cur_item.external_ids ), @@ -126,7 +124,7 @@ async def update_item_in_library( }, ) # update/set provider_mappings table - await self._set_provider_mappings(db_id, provider_mappings) + await self._set_provider_mappings(db_id, update.provider_mappings, overwrite=overwrite) self.logger.debug("updated %s in database: %s", update.name, db_id) # get full created object library_item = await self.get_library_item(db_id) @@ -324,7 +322,6 @@ async def _add_library_item(self, item: Playlist) -> Playlist: "is_editable": item.is_editable, "favorite": item.favorite, "metadata": serialize_to_json(item.metadata), - "provider_mappings": serialize_to_json(item.provider_mappings), "external_ids": serialize_to_json(item.external_ids), "timestamp_added": int(utc_timestamp()), "timestamp_modified": int(utc_timestamp()), diff --git a/music_assistant/server/controllers/media/radio.py b/music_assistant/server/controllers/media/radio.py index c1dc17a67..35f2f3890 100644 --- a/music_assistant/server/controllers/media/radio.py +++ b/music_assistant/server/controllers/media/radio.py @@ -116,7 +116,6 @@ async def update_item_in_library( db_id = int(item_id) # ensure integer cur_item = await self.get_library_item(db_id) metadata = cur_item.metadata.update(getattr(update, "metadata", None), overwrite) - provider_mappings = self._get_provider_mappings(cur_item, update) cur_item.external_ids.update(update.external_ids) match = {"item_id": db_id} await self.mass.music.database.update( @@ -129,7 +128,6 @@ async def update_item_in_library( if overwrite else cur_item.sort_name or update.sort_name, "metadata": serialize_to_json(metadata), - "provider_mappings": serialize_to_json(provider_mappings), "external_ids": serialize_to_json( update.external_ids if overwrite else cur_item.external_ids ), @@ -137,7 +135,7 @@ async def update_item_in_library( }, ) # update/set provider_mappings table - await self._set_provider_mappings(db_id, provider_mappings) + await self._set_provider_mappings(db_id, update.provider_mappings, overwrite=overwrite) self.logger.debug("updated %s in database: %s", update.name, db_id) # get full created object library_item = await self.get_library_item(db_id) @@ -160,7 +158,6 @@ async def _add_library_item(self, item: Radio) -> Radio: "sort_name": item.sort_name, "favorite": item.favorite, "metadata": serialize_to_json(item.metadata), - "provider_mappings": serialize_to_json(item.provider_mappings), "external_ids": serialize_to_json(item.external_ids), "timestamp_added": int(utc_timestamp()), "timestamp_modified": int(utc_timestamp()), diff --git a/music_assistant/server/controllers/media/tracks.py b/music_assistant/server/controllers/media/tracks.py index 95cc70ec3..c00791a75 100644 --- a/music_assistant/server/controllers/media/tracks.py +++ b/music_assistant/server/controllers/media/tracks.py @@ -15,7 +15,7 @@ MusicAssistantError, UnsupportedFeaturedException, ) -from music_assistant.common.models.media_items import Album, ItemMapping, Track +from music_assistant.common.models.media_items import Album, Artist, ItemMapping, Track from music_assistant.constants import ( DB_TABLE_ALBUM_TRACKS, DB_TABLE_ALBUMS, @@ -57,7 +57,7 @@ def __init__(self, *args, **kwargs) -> None: 'url', {DB_TABLE_PROVIDER_MAPPINGS}.url, 'audio_format', json({DB_TABLE_PROVIDER_MAPPINGS}.audio_format), 'details', {DB_TABLE_PROVIDER_MAPPINGS}.details - )) as {DB_TABLE_PROVIDER_MAPPINGS}, + )) filter ( where {DB_TABLE_PROVIDER_MAPPINGS}.item_id is not null) as {DB_TABLE_PROVIDER_MAPPINGS}, json_group_array( json_object( 'item_id', {DB_TABLE_ARTISTS}.item_id, @@ -65,7 +65,7 @@ def __init__(self, *args, **kwargs) -> None: 'name', {DB_TABLE_ARTISTS}.name, 'sort_name', {DB_TABLE_ARTISTS}.sort_name, 'media_type', 'artist' - )) as {DB_TABLE_ARTISTS}, + )) filter ( where {DB_TABLE_ARTISTS}.name is not null) as {DB_TABLE_ARTISTS}, json_object( 'item_id', {DB_TABLE_ALBUMS}.item_id, 'provider', 'library', @@ -187,7 +187,7 @@ async def add_item_to_library( # grab additional metadata if metadata_lookup: await self.mass.metadata.get_track_metadata(item) - # allow track image from album (only if albumtype = single) + # copy track image from album (only if albumtype = single) if ( not item.image and isinstance(item.album, Album) @@ -244,8 +244,6 @@ async def update_item_in_library( db_id = int(item_id) # ensure integer cur_item = await self.get_library_item(db_id) metadata = cur_item.metadata.update(getattr(update, "metadata", None), overwrite) - provider_mappings = self._get_provider_mappings(cur_item, update) - track_artists = await self._get_artist_mappings(cur_item, update, overwrite=overwrite) cur_item.external_ids.update(update.external_ids) await self.mass.music.database.update( self.db_table, @@ -257,9 +255,7 @@ async def update_item_in_library( else cur_item.sort_name or update.sort_name, "version": update.version if overwrite else cur_item.version or update.version, "duration": update.duration if overwrite else cur_item.duration or update.duration, - "artists": serialize_to_json(track_artists), "metadata": serialize_to_json(metadata), - "provider_mappings": serialize_to_json(provider_mappings), "timestamp_modified": int(utc_timestamp()), "external_ids": serialize_to_json( update.external_ids if overwrite else cur_item.external_ids @@ -267,24 +263,28 @@ async def update_item_in_library( }, ) # update/set provider_mappings table - await self._set_provider_mappings(db_id, provider_mappings) - # handle track album + await self._set_provider_mappings(db_id, update.provider_mappings, overwrite=overwrite) + + # update/set track album if update.album: await self._set_track_album( db_id=db_id, album=update.album, disc_number=getattr(update, "disc_number", None) or 0, - track_number=getattr(update, "track_number", None) or 0, + track_number=getattr(update, "track_number", None) or 1, + overwrite=overwrite, ) - # get full created object + + # set track artist(s) + await self._set_track_artists(db_id, update.artists, overwrite=overwrite) + + # get full/final created object library_item = await self.get_library_item(db_id) - # only signal event if we're not running a sync (to prevent a floodstorm of events) - if not self.mass.music.get_running_sync_tasks(): - self.mass.signal_event( - EventType.MEDIA_ITEM_UPDATED, - library_item.uri, - library_item, - ) + self.mass.signal_event( + EventType.MEDIA_ITEM_UPDATED, + library_item.uri, + library_item, + ) self.logger.debug("updated %s in database: %s", update.name, db_id) # return the full item we just updated return library_item @@ -441,8 +441,6 @@ async def _get_dynamic_tracks( async def _add_library_item(self, item: Track) -> Track: """Add a new item record to the database.""" - track_artists = await self._get_artist_mappings(item) - sort_artist = track_artists[0].sort_name new_item = await self.mass.music.database.insert( self.db_table, { @@ -453,9 +451,6 @@ async def _add_library_item(self, item: Track) -> Track: "favorite": item.favorite, "external_ids": serialize_to_json(item.external_ids), "metadata": serialize_to_json(item.metadata), - "provider_mappings": serialize_to_json(item.provider_mappings), - "artists": serialize_to_json(track_artists), - "sort_artist": sort_artist, "timestamp_added": int(utc_timestamp()), "timestamp_modified": int(utc_timestamp()), }, @@ -463,6 +458,8 @@ async def _add_library_item(self, item: Track) -> Track: db_id = new_item["item_id"] # update/set provider_mappings table await self._set_provider_mappings(db_id, item.provider_mappings) + # set track artist(s) + await self._set_track_artists(db_id, item.artists) # handle track album if item.album: await self._set_track_album( @@ -476,7 +473,12 @@ async def _add_library_item(self, item: Track) -> Track: return await self.get_library_item(db_id) async def _set_track_album( - self, db_id: int, album: Album, disc_number: int, track_number: int + self, + db_id: int, + album: Album | ItemMapping, + disc_number: int, + track_number: int, + overwrite: bool = False, ) -> None: """ Store Track Album info. @@ -487,15 +489,25 @@ async def _set_track_album( For digital releases, the discnumber will be just 0 or 1. Track number should start counting at 1. """ - db_album = None + if overwrite and album.provider.startswith("filesystem"): + # on overwrite, clear the album_tracks table first + # this is done for filesystem providers only (to account for changing ID3 tags) + # TODO: find a better way to deal with this as this doesn't cover all (edge) cases + await self.mass.music.database.delete( + DB_TABLE_ALBUM_TRACKS, + { + "track_id": db_id, + }, + ) + db_album: Album | ItemMapping = None if album.provider == "library": db_album = album - elif existing := await self.mass.music.artists.get_library_item_by_prov_id( + elif existing := await self.mass.music.albums.get_library_item_by_prov_id( album.item_id, album.provider ): db_album = existing else: - # not an existing album, we need to fetch and add it + # not an existing album, we need to fetch before we can add it to the library if isinstance(album, ItemMapping): album = await self.mass.music.albums.get_provider_item( album.item_id, album.provider, fallback=album @@ -504,32 +516,74 @@ async def _set_track_album( db_album = await self.mass.music.albums.add_item_to_library( album, metadata_lookup=False, add_album_tracks=False ) - if not db_album: + # this should not happen but streaming providers can be awful sometimes self.logger.warning( - "Unable to resolve Album for track %s, " - "track will be added to the library without album", + "Unable to resolve Album %s for track %s, " + "track will be added to the library without this album!", album.uri, + db_id, ) return - album_mapping = {"track_id": db_id, "album_id": int(db_album.item_id)} - if db_row := await self.mass.music.database.get_row(DB_TABLE_ALBUM_TRACKS, album_mapping): - # update existing - await self.mass.music.database.update( - DB_TABLE_ALBUM_TRACKS, - album_mapping, + # write (or update) record in album_tracks table + await self.mass.music.database.insert_or_replace( + DB_TABLE_ALBUM_TRACKS, + { + "track_id": db_id, + "album_id": int(db_album.item_id), + "disc_number": disc_number, + "track_number": track_number, + }, + ) + + async def _set_track_artists( + self, db_id: int, artists: list[Artist | ItemMapping], overwrite: bool = False + ) -> None: + """Store Track Artists.""" + if overwrite: + # on overwrite, clear the track_artists table first + await self.mass.music.database.delete( + DB_TABLE_TRACK_ARTISTS, { - "disc_number": disc_number or db_row["disc_number"], - "track_number": track_number or db_row["track_number"], + "track_id": db_id, }, ) + for artist in artists: + await self._set_track_artist(db_id, artist=artist) + + async def _set_track_artist(self, db_id: int, artist: Artist | ItemMapping) -> None: + """Store Track Artist info.""" + db_artist: Album | ItemMapping = None + if artist.provider == "library": + db_artist = artist + elif existing := await self.mass.music.artists.get_library_item_by_prov_id( + artist.item_id, artist.provider + ): + db_artist = existing else: - # create new albumtrack record - await self.mass.music.database.insert_or_replace( - DB_TABLE_ALBUM_TRACKS, - { - **album_mapping, - "disc_number": disc_number, - "track_number": track_number, - }, + # not an existing artist, we need to fetch before we can add it to the library + if isinstance(artist, ItemMapping): + artist = await self.mass.music.artists.get_provider_item( + artist.item_id, artist.provider, fallback=artist + ) + with suppress(MediaNotFoundError, AssertionError, InvalidDataError): + db_artist = await self.mass.music.artists.add_item_to_library( + artist, metadata_lookup=False, add_album_tracks=False + ) + if not db_artist: + # this should not happen but streaming providers can be awful sometimes + self.logger.warning( + "Unable to resolve Artist %s for track %s, " + "track will be added to the library without this artist!", + artist.uri, + db_id, ) + return + # write (or update) record in track_artists table + await self.mass.music.database.insert_or_replace( + DB_TABLE_TRACK_ARTISTS, + { + "track_id": db_id, + "artist_id": int(db_artist.item_id), + }, + ) diff --git a/music_assistant/server/providers/filesystem_local/base.py b/music_assistant/server/providers/filesystem_local/base.py index 683c49868..3d570931e 100644 --- a/music_assistant/server/providers/filesystem_local/base.py +++ b/music_assistant/server/providers/filesystem_local/base.py @@ -4,6 +4,7 @@ import asyncio import contextlib +import logging import os from abc import abstractmethod from dataclasses import dataclass @@ -367,7 +368,12 @@ async def sync_library(self, media_types: tuple[MediaType, ...]) -> None: ) except Exception as err: # pylint: disable=broad-except # we don't want the whole sync to crash on one file so we catch all exceptions here - self.logger.error("Error processing %s - %s", item.path, str(err)) + self.logger.error( + "Error processing %s - %s", + item.path, + str(err), + exc_info=err if self.logger.isEnabledFor(logging.DEBUG) else None, + ) async def _process_deletions(self, deleted_files: set[str]) -> None: """Process all deletions.""" From 1c3aebc3a33b839e59e3c33f02d14ee36b8c16eb Mon Sep 17 00:00:00 2001 From: Marcel van der Veldt Date: Thu, 18 Apr 2024 21:28:19 +0200 Subject: [PATCH 4/6] optimize lookup queries --- .../server/controllers/media/base.py | 49 ++++++++----------- 1 file changed, 20 insertions(+), 29 deletions(-) diff --git a/music_assistant/server/controllers/media/base.py b/music_assistant/server/controllers/media/base.py index b166c052e..31bb0aa71 100644 --- a/music_assistant/server/controllers/media/base.py +++ b/music_assistant/server/controllers/media/base.py @@ -357,9 +357,10 @@ async def get_library_item_by_external_id( """Get the library item for the given external id.""" query = self.base_query + f" WHERE {self.db_table}.external_ids LIKE :external_id_str" if external_id_type: - external_id_str = f'%("{external_id_type}", "{external_id}")%' + external_id_str = f'%"{external_id_type}","{external_id}"%' else: external_id_str = f'%"{external_id}"%' + query += f" GROUP BY {self.sql_group_by}" for item in await self._get_library_items_by_query( query=query, query_params={"external_id_str": external_id_str} ): @@ -383,36 +384,26 @@ async def get_library_items_by_prov_id( offset: int = 0, ) -> list[ItemCls]: """Fetch all records from library for given provider.""" + query_parts = [] + prov_ids_str = str(tuple(provider_item_ids or ())) + if prov_ids_str.endswith(",)"): + prov_ids_str = prov_ids_str.replace(",)", ")") + if provider_instance_id_or_domain == "library": - if provider_item_ids is not None: - prov_ids_string = str(tuple(int(x) for x in provider_item_ids)) - if prov_ids_string.endswith(",)"): - prov_ids_string = prov_ids_string.replace(",)", ")") - extra_query = f"item_id in {prov_ids_string}" - else: - extra_query = None - paged_list = await self.library_items( - limit=limit, offset=offset, extra_query=extra_query + # request for specific library id's + if provider_item_ids: + query_parts.append(f"{self.db_table}.item_id in {prov_ids_str}") + else: + # provider filtered response + query_parts.append( + f"(provider_mappings.provider_instance = '{provider_instance_id_or_domain}' " + f"OR provider_mappings.provider_domain = '{provider_instance_id_or_domain}')" ) - return paged_list.items - - # we use the separate provider_mappings table to perform quick lookups - # from provider id's to database id's because this is faster - # (and more compatible) than querying the provider_mappings json column - subquery = ( - f"SELECT item_id FROM {DB_TABLE_PROVIDER_MAPPINGS} WHERE " - f" media_type = '{self.media_type.value}' AND " - f"(provider_instance = '{provider_instance_id_or_domain}' " - f"OR provider_domain = '{provider_instance_id_or_domain}')" - ) - if provider_item_ids is not None: - prov_ids = str(tuple(provider_item_ids)) - if prov_ids.endswith(",)"): - prov_ids = prov_ids.replace(",)", ")") - subquery += f" AND provider_item_id in {prov_ids}" - # final query is a where query from the subquery - # that queries the provider_mappings table - query = f"WHERE {self.db_table}.item_id in ({subquery})" + if provider_item_ids: + query_parts.append(f"provider_mappings.provider_item_id in {prov_ids_str}") + + # build final query + query = "WHERE " + " AND ".join(query_parts) paged_list = await self.library_items(limit=limit, offset=offset, extra_query=query) return paged_list.items From b71bf6561375c76817b49fd430b13f77a4584b86 Mon Sep 17 00:00:00 2001 From: Marcel van der Veldt Date: Thu, 18 Apr 2024 21:51:45 +0200 Subject: [PATCH 5/6] fix for images --- music_assistant/server/controllers/media/artists.py | 2 +- music_assistant/server/controllers/media/base.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/music_assistant/server/controllers/media/artists.py b/music_assistant/server/controllers/media/artists.py index 24328cd8d..7e417b851 100644 --- a/music_assistant/server/controllers/media/artists.py +++ b/music_assistant/server/controllers/media/artists.py @@ -169,7 +169,7 @@ async def library_items( ) -> PagedItems: """Get in-database (album) artists.""" if album_artists_only: - artist_query = "artists.sort_name in (select albums.sort_artist from albums)" + artist_query = "artists.item_id in (select albumartists.artist_id from albumartists)" extra_query = f"{extra_query} AND {artist_query}" if extra_query else artist_query return await super().library_items( favorite=favorite, diff --git a/music_assistant/server/controllers/media/base.py b/music_assistant/server/controllers/media/base.py index 31bb0aa71..b86f82574 100644 --- a/music_assistant/server/controllers/media/base.py +++ b/music_assistant/server/controllers/media/base.py @@ -693,6 +693,6 @@ def _parse_db_row(db_row: Mapping) -> dict[str, Any]: if "item_id" in db_row_dict: db_row_dict["item_id"] = str(db_row_dict["item_id"]) # copy album image to itemmapping single image - if "album" in db_row and (images := db_row.get("images")): - db_row["album"]["image"] = next((x for x in images if x["type"] == "thumb"), None) + if "album" in db_row_dict and (images := db_row_dict["album"].get("images")): + db_row_dict["album"]["image"] = next((x for x in images if x["type"] == "thumb"), None) return db_row_dict From 23e6f4de611f4943fae512b26d95dd5058200214 Mon Sep 17 00:00:00 2001 From: Marcel van der Veldt Date: Thu, 18 Apr 2024 23:35:33 +0200 Subject: [PATCH 6/6] More fixes --- music_assistant/common/models/media_items.py | 4 +-- .../server/controllers/media/albums.py | 10 +++++--- .../server/controllers/media/base.py | 25 +++---------------- .../server/controllers/media/tracks.py | 15 +++++++---- 4 files changed, 21 insertions(+), 33 deletions(-) diff --git a/music_assistant/common/models/media_items.py b/music_assistant/common/models/media_items.py index 4449a3838..407d58f4e 100644 --- a/music_assistant/common/models/media_items.py +++ b/music_assistant/common/models/media_items.py @@ -337,6 +337,8 @@ class ItemMapping(_MediaItemBase): @classmethod def from_item(cls, item: MediaItem) -> ItemMapping: """Create ItemMapping object from regular item.""" + if isinstance(item, ItemMapping): + return item thumb_image = None if item.metadata and item.metadata.images: for img in item.metadata.images: @@ -426,7 +428,6 @@ def from_track(cls: Self, track: Track, album: Album | None = None) -> Self: assert isinstance(track.album, Album) assert track.disc_number is not None assert track.track_number is not None - assert track.track_number > 0 return cast(AlbumTrack, track) @@ -444,7 +445,6 @@ class PlaylistTrack(Track): def from_track(cls: Self, track: Track) -> Self: """Cast Track to PlaylistTrack.""" assert track.position is not None - assert track.position > 0 return cast(AlbumTrack, track) diff --git a/music_assistant/server/controllers/media/albums.py b/music_assistant/server/controllers/media/albums.py index 2ba79d246..6bcb7f4f6 100644 --- a/music_assistant/server/controllers/media/albums.py +++ b/music_assistant/server/controllers/media/albums.py @@ -469,7 +469,7 @@ async def find_prov_match(provider: MusicProvider): ) async def _set_album_artists( - self, db_id: int, artists: list[Artist | ItemMapping], overwrite: bool + self, db_id: int, artists: list[Artist | ItemMapping], overwrite: bool = False ) -> None: """Store Album Artists.""" if overwrite: @@ -481,9 +481,11 @@ async def _set_album_artists( }, ) for artist in artists: - await self._set_album_artist(db_id, artist=artist) + await self._set_album_artist(db_id, artist=artist, overwrite=overwrite) - async def _set_album_artist(self, db_id: int, artist: Artist | ItemMapping) -> None: + async def _set_album_artist( + self, db_id: int, artist: Artist | ItemMapping, overwrite: bool = False + ) -> None: """Store Album Artist info.""" db_artist: Album | ItemMapping = None if artist.provider == "library": @@ -500,7 +502,7 @@ async def _set_album_artist(self, db_id: int, artist: Artist | ItemMapping) -> N ) with contextlib.suppress(MediaNotFoundError, AssertionError, InvalidDataError): db_artist = await self.mass.music.artists.add_item_to_library( - artist, metadata_lookup=False + artist, metadata_lookup=False, overwrite_existing=overwrite ) if not db_artist: # this should not happen but streaming providers can be awful sometimes diff --git a/music_assistant/server/controllers/media/base.py b/music_assistant/server/controllers/media/base.py index b86f82574..6d37e695b 100644 --- a/music_assistant/server/controllers/media/base.py +++ b/music_assistant/server/controllers/media/base.py @@ -8,7 +8,7 @@ from time import time from typing import TYPE_CHECKING, Any, Generic, TypeVar -from music_assistant.common.helpers.json import json_dumps, json_loads, serialize_to_json +from music_assistant.common.helpers.json import json_dumps, json_loads from music_assistant.common.models.enums import EventType, ExternalID, MediaType, ProviderFeature from music_assistant.common.models.errors import MediaNotFoundError, ProviderUnavailableError from music_assistant.common.models.media_items import ( @@ -488,15 +488,6 @@ async def add_provider_mapping( # ignore if the mapping is already present if provider_mapping in library_item.provider_mappings: return - # update item's db record - library_item.provider_mappings.add(provider_mapping) - await self.mass.music.database.update( - self.db_table, - {"item_id": db_id}, - { - "provider_mappings": serialize_to_json(library_item.provider_mappings), - }, - ) # update provider_mappings table await self._set_provider_mappings( item_id=item_id, provider_mappings=library_item.provider_mappings @@ -530,13 +521,7 @@ async def remove_provider_mapping( for x in library_item.provider_mappings if x.provider_instance != provider_instance_id and x.item_id != provider_item_id } - match = {"item_id": db_id} if library_item.provider_mappings: - await self.mass.music.database.update( - self.db_table, - match, - {"provider_mappings": serialize_to_json(library_item.provider_mappings)}, - ) self.logger.debug( "removed provider_mapping %s/%s from item id %s", provider_instance_id, @@ -572,13 +557,7 @@ async def remove_provider_mappings(self, item_id: str | int, provider_instance_i library_item.provider_mappings = { x for x in library_item.provider_mappings if x.provider_instance != provider_instance_id } - match = {"item_id": db_id} if library_item.provider_mappings: - await self.mass.music.database.update( - self.db_table, - match, - {"provider_mappings": serialize_to_json(library_item.provider_mappings)}, - ) self.logger.debug( "removed all provider mappings for provider %s from item id %s", provider_instance_id, @@ -692,6 +671,8 @@ def _parse_db_row(db_row: Mapping) -> dict[str, Any]: db_row_dict["favorite"] = bool(db_row_dict["favorite"]) if "item_id" in db_row_dict: db_row_dict["item_id"] = str(db_row_dict["item_id"]) + if "album" in db_row_dict and db_row_dict["album"]["item_id"] is None: + db_row_dict.pop("album") # copy album image to itemmapping single image if "album" in db_row_dict and (images := db_row_dict["album"].get("images")): db_row_dict["album"]["image"] = next((x for x in images if x["type"] == "thumb"), None) diff --git a/music_assistant/server/controllers/media/tracks.py b/music_assistant/server/controllers/media/tracks.py index c00791a75..a6a9e9283 100644 --- a/music_assistant/server/controllers/media/tracks.py +++ b/music_assistant/server/controllers/media/tracks.py @@ -73,7 +73,7 @@ def __init__(self, *args, **kwargs) -> None: 'sort_name', {DB_TABLE_ALBUMS}.sort_name, 'version', {DB_TABLE_ALBUMS}.version, 'images', json_extract({DB_TABLE_ALBUMS}.metadata, '$.images'), - 'media_type', 'artist' + 'media_type', 'album' ) as album, {DB_TABLE_ALBUM_TRACKS}.disc_number, {DB_TABLE_ALBUM_TRACKS}.track_number @@ -514,7 +514,10 @@ async def _set_track_album( ) with suppress(MediaNotFoundError, AssertionError, InvalidDataError): db_album = await self.mass.music.albums.add_item_to_library( - album, metadata_lookup=False, add_album_tracks=False + album, + metadata_lookup=False, + overwrite_existing=overwrite, + add_album_tracks=False, ) if not db_album: # this should not happen but streaming providers can be awful sometimes @@ -549,9 +552,11 @@ async def _set_track_artists( }, ) for artist in artists: - await self._set_track_artist(db_id, artist=artist) + await self._set_track_artist(db_id, artist=artist, overwrite=overwrite) - async def _set_track_artist(self, db_id: int, artist: Artist | ItemMapping) -> None: + async def _set_track_artist( + self, db_id: int, artist: Artist | ItemMapping, overwrite: bool = False + ) -> None: """Store Track Artist info.""" db_artist: Album | ItemMapping = None if artist.provider == "library": @@ -568,7 +573,7 @@ async def _set_track_artist(self, db_id: int, artist: Artist | ItemMapping) -> N ) with suppress(MediaNotFoundError, AssertionError, InvalidDataError): db_artist = await self.mass.music.artists.add_item_to_library( - artist, metadata_lookup=False, add_album_tracks=False + artist, metadata_lookup=False, overwrite_existing=overwrite ) if not db_artist: # this should not happen but streaming providers can be awful sometimes