From d250a66d5553f659d5d4e13a2edd71040f03323d Mon Sep 17 00:00:00 2001 From: Jc2k Date: Sun, 7 Jul 2024 22:59:53 +0100 Subject: [PATCH] Add typing for filesystem_local (#1461) --- music_assistant/common/models/media_items.py | 12 +- .../server/controllers/media/base.py | 4 +- .../server/models/music_provider.py | 7 +- .../providers/filesystem_local/__init__.py | 20 +-- .../server/providers/filesystem_local/base.py | 121 ++++++++++-------- .../server/providers/jellyfin/__init__.py | 8 +- .../server/providers/radiobrowser/__init__.py | 4 +- mypy.ini | 2 +- 8 files changed, 98 insertions(+), 80 deletions(-) diff --git a/music_assistant/common/models/media_items.py b/music_assistant/common/models/media_items.py index 1b8b5755..3ec3adaa 100644 --- a/music_assistant/common/models/media_items.py +++ b/music_assistant/common/models/media_items.py @@ -2,7 +2,7 @@ from __future__ import annotations -from collections.abc import Iterable +from collections.abc import Iterable, Sequence from dataclasses import dataclass, field, fields from typing import TYPE_CHECKING, Any, TypeGuard, TypeVar, cast @@ -526,11 +526,11 @@ MediaItemType = Artist | Album | PlaylistTrack | Track | Radio | Playlist | Brow class SearchResults(DataClassDictMixin): """Model for results from a search query.""" - artists: list[Artist | ItemMapping] = field(default_factory=list) - albums: list[Album | ItemMapping] = field(default_factory=list) - tracks: list[Track | ItemMapping] = field(default_factory=list) - playlists: list[Playlist | ItemMapping] = field(default_factory=list) - radio: list[Radio | ItemMapping] = field(default_factory=list) + artists: Sequence[Artist | ItemMapping] = field(default_factory=list) + albums: Sequence[Album | ItemMapping] = field(default_factory=list) + tracks: Sequence[Track | ItemMapping] = field(default_factory=list) + playlists: Sequence[Playlist | ItemMapping] = field(default_factory=list) + radio: Sequence[Radio | ItemMapping] = field(default_factory=list) def media_from_dict(media_item: dict[str, Any]) -> MediaItemType | ItemMapping: diff --git a/music_assistant/server/controllers/media/base.py b/music_assistant/server/controllers/media/base.py index 68eeca6a..eeb3b9e0 100644 --- a/music_assistant/server/controllers/media/base.py +++ b/music_assistant/server/controllers/media/base.py @@ -91,7 +91,7 @@ class MediaControllerBase(Generic[ItemCls], metaclass=ABCMeta): self._db_add_lock = asyncio.Lock() async def add_item_to_library( - self, item: Track, metadata_lookup: bool = True, overwrite_existing: bool = False + self, item: ItemCls, metadata_lookup: bool = True, overwrite_existing: bool = False ) -> ItemCls: """Add item to library and return the new (or updated) database item.""" new_item = False @@ -758,7 +758,7 @@ class MediaControllerBase(Generic[ItemCls], metaclass=ABCMeta): provider: str | None = None, extra_query: str | None = None, extra_query_params: dict[str, Any] | None = None, - ) -> list[ItemCls] | int: + ) -> list[ItemCls]: """Fetch MediaItem records from database given a custom (WHERE) clause.""" sql_query = self.base_query query_params = extra_query_params or {} diff --git a/music_assistant/server/models/music_provider.py b/music_assistant/server/models/music_provider.py index 683342bf..8c6403b8 100644 --- a/music_assistant/server/models/music_provider.py +++ b/music_assistant/server/models/music_provider.py @@ -12,6 +12,7 @@ from music_assistant.common.models.media_items import ( Album, Artist, BrowseFolder, + ItemMapping, MediaItemType, Playlist, Radio, @@ -257,6 +258,8 @@ class MusicProvider(Provider): Will only be called when the stream_type is set to CUSTOM. """ + if False: + yield raise NotImplementedError async def on_streamed(self, streamdetails: StreamDetails, seconds_streamed: int) -> None: @@ -283,7 +286,9 @@ class MusicProvider(Provider): return await self.get_radio(prov_item_id) return await self.get_track(prov_item_id) - async def browse(self, path: str, offset: int, limit: int) -> Sequence[MediaItemType]: + async def browse( + self, path: str, offset: int, limit: int + ) -> Sequence[MediaItemType | ItemMapping]: """Browse this provider's items. :param path: The path to browse, (e.g. provider_id://artists). diff --git a/music_assistant/server/providers/filesystem_local/__init__.py b/music_assistant/server/providers/filesystem_local/__init__.py index a98d5186..33f02d23 100644 --- a/music_assistant/server/providers/filesystem_local/__init__.py +++ b/music_assistant/server/providers/filesystem_local/__init__.py @@ -51,7 +51,7 @@ async def setup( msg = f"Music Directory {conf_path} does not exist" raise SetupFailedError(msg) prov = LocalFileSystemProvider(mass, manifest, config) - prov.base_path = config.get_value(CONF_PATH) + prov.base_path = str(config.get_value(CONF_PATH)) await prov.check_write_access() mass.call_later(30, prov.migrate_playlists) return prov @@ -77,14 +77,14 @@ async def get_config_entries( ) -def sorted_scandir(base_path: str, sub_path: str) -> list[os.DirEntry]: +def sorted_scandir(base_path: str, sub_path: str) -> list[FileSystemItem]: """Implement os.scandir that returns (naturally) sorted entries.""" - def nat_key(name: str) -> tuple[int, str]: + def nat_key(name: str) -> tuple[int | str, ...]: """Sort key for natural sorting.""" return tuple(int(s) if s.isdigit() else s for s in re.split(r"(\d+)", name)) - def create_item(entry: os.DirEntry): + def create_item(entry: os.DirEntry) -> FileSystemItem: """Create FileSystemItem from os.DirEntry.""" absolute_path = get_absolute_path(base_path, entry.path) stat = entry.stat(follow_symlinks=False) @@ -168,7 +168,7 @@ class LocalFileSystemProvider(FileSystemProviderBase): """ absolute_path = get_absolute_path(self.base_path, file_path) - def _create_item(): + def _create_item() -> FileSystemItem: stat = os.stat(absolute_path, follow_symlinks=False) return FileSystemItem( filename=os.path.basename(file_path), @@ -190,7 +190,7 @@ class LocalFileSystemProvider(FileSystemProviderBase): if not file_path: return False # guard abs_path = get_absolute_path(self.base_path, file_path) - return await exists(abs_path) + return bool(await exists(abs_path)) async def read_file_content(self, file_path: str, seek: int = 0) -> AsyncGenerator[bytes, None]: """Yield (binary) contents of file in chunks of bytes.""" @@ -229,11 +229,11 @@ class LocalFileSystemProvider(FileSystemProviderBase): continue if item.ext != "m3u": continue - playlist_data = b"" + playlist_bytes = b"" async for chunk in self.read_file_content(item.absolute_path): - playlist_data += chunk - encoding_details = await asyncio.to_thread(cchardet.detect, playlist_data) - playlist_data = playlist_data.decode(encoding_details["encoding"] or "utf-8") + playlist_bytes += chunk + encoding_details = await asyncio.to_thread(cchardet.detect, playlist_bytes) + playlist_data = playlist_bytes.decode(encoding_details["encoding"] or "utf-8") # a (legacy) playlist file created by MA does not have EXTINFO tags and has uri's if "EXTINF" in playlist_data or "://" not in playlist_data: continue diff --git a/music_assistant/server/providers/filesystem_local/base.py b/music_assistant/server/providers/filesystem_local/base.py index c40e1973..a81b0441 100644 --- a/music_assistant/server/providers/filesystem_local/base.py +++ b/music_assistant/server/providers/filesystem_local/base.py @@ -36,6 +36,8 @@ from music_assistant.common.models.media_items import ( ProviderMapping, SearchResults, Track, + UniqueList, + is_track, ) from music_assistant.common.models.streamdetails import StreamDetails from music_assistant.constants import ( @@ -181,7 +183,9 @@ class FileSystemProviderBase(MusicProvider): AsyncGenerator yielding FileSystemItem objects. """ - yield + # mypy will infer wrong type without an explicit yield + # https://github.com/python/mypy/issues/5070 + yield # type: ignore[misc] @abstractmethod async def resolve(self, file_path: str) -> FileSystemItem: @@ -194,6 +198,9 @@ class FileSystemProviderBase(MusicProvider): @abstractmethod async def read_file_content(self, file_path: str, seek: int = 0) -> AsyncGenerator[bytes, None]: """Yield (binary) contents of file in chunks of bytes.""" + # mypy will infer wrong type without an explicit yield + # https://github.com/python/mypy/issues/5070 + yield # type: ignore[misc] @abstractmethod async def write_file_content(self, file_path: str, data: bytes) -> None: @@ -221,7 +228,7 @@ class FileSystemProviderBase(MusicProvider): async def search( self, search_query: str, - media_types=list[MediaType] | None, + media_types: list[MediaType] | None, limit: int = 5, ) -> SearchResults: """Perform search on this file based musicprovider.""" @@ -252,7 +259,7 @@ class FileSystemProviderBase(MusicProvider): ) return result - async def browse(self, path: str, offset: int, limit: int) -> list[MediaItemType]: + async def browse(self, path: str, offset: int, limit: int) -> list[MediaItemType | ItemMapping]: """Browse this provider's items. :param path: The path to browse, (e.g. provid://artists). @@ -260,7 +267,7 @@ class FileSystemProviderBase(MusicProvider): if offset: # we do not support pagination return [] - items: list[MediaItemType] = [] + items: list[MediaItemType | ItemMapping] = [] item_path = path.split("://", 1)[1] if not item_path: item_path = "" @@ -300,6 +307,7 @@ class FileSystemProviderBase(MusicProvider): async def sync_library(self, media_types: tuple[MediaType, ...]) -> None: """Run library sync for this provider.""" + assert self.mass.music.database file_checksums: dict[str, str] = {} query = ( f"SELECT provider_item_id, details FROM {DB_TABLE_PROVIDER_MAPPINGS} " @@ -362,6 +370,7 @@ class FileSystemProviderBase(MusicProvider): async def _process_orphaned_albums_and_artists(self) -> None: """Process deletion of orphaned albums and artists.""" + assert self.mass.music.database # Remove albums without any tracks query = ( f"SELECT item_id FROM {DB_TABLE_ALBUMS} " @@ -409,7 +418,7 @@ class FileSystemProviderBase(MusicProvider): if library_item := await controller.get_library_item_by_prov_id( file_path, self.instance_id ): - if library_item.media_type == MediaType.TRACK: + if is_track(library_item): if library_item.album: album_ids.add(library_item.album.item_id) # need to fetch the library album to resolve the itemmapping @@ -451,6 +460,7 @@ class FileSystemProviderBase(MusicProvider): for prov_mapping in track.provider_mappings: if prov_mapping.provider_instance == self.instance_id: full_track = await self.get_track(prov_mapping.item_id) + assert isinstance(full_track.album, Album) return full_track.album msg = f"Album not found: {prov_album_id}" raise MediaNotFoundError(msg) @@ -525,11 +535,11 @@ class FileSystemProviderBase(MusicProvider): _, ext = prov_playlist_id.rsplit(".", 1) try: # get playlist file contents - playlist_data = b"" + playlist_bytes = b"" async for chunk in self.read_file_content(prov_playlist_id): - playlist_data += chunk - encoding_details = await asyncio.to_thread(cchardet.detect, playlist_data) - playlist_data = playlist_data.decode(encoding_details["encoding"] or "utf-8") + playlist_bytes += chunk + encoding_details = await asyncio.to_thread(cchardet.detect, playlist_bytes) + playlist_data = playlist_bytes.decode(encoding_details["encoding"] or "utf-8") if ext in ("m3u", "m3u8"): playlist_lines = parse_m3u(playlist_data) @@ -574,18 +584,19 @@ class FileSystemProviderBase(MusicProvider): except MusicAssistantError as err: self.logger.warning("Could not parse uri/file %s to track: %s", line, str(err)) - return None + + return None async def add_playlist_tracks(self, prov_playlist_id: str, prov_track_ids: list[str]) -> None: """Add track(s) to playlist.""" if not await self.exists(prov_playlist_id): msg = f"Playlist path does not exist: {prov_playlist_id}" raise MediaNotFoundError(msg) - playlist_data = b"" + playlist_bytes = b"" async for chunk in self.read_file_content(prov_playlist_id): - playlist_data += chunk - encoding_details = await asyncio.to_thread(cchardet.detect, playlist_data) - playlist_data = playlist_data.decode(encoding_details["encoding"] or "utf-8") + playlist_bytes += chunk + encoding_details = await asyncio.to_thread(cchardet.detect, playlist_bytes) + playlist_data = playlist_bytes.decode(encoding_details["encoding"] or "utf-8") for file_path in prov_track_ids: track = await self.get_track(file_path) playlist_data += f"\n#EXTINF:{track.duration or 0},{track.name}\n{file_path}\n" @@ -602,11 +613,11 @@ class FileSystemProviderBase(MusicProvider): raise MediaNotFoundError(msg) _, ext = prov_playlist_id.rsplit(".", 1) # get playlist file contents - playlist_data = b"" + playlist_bytes = b"" async for chunk in self.read_file_content(prov_playlist_id): - playlist_data += chunk - encoding_details = await asyncio.to_thread(cchardet.detect, playlist_data) - playlist_data = playlist_data.decode(encoding_details["encoding"] or "utf-8") + playlist_bytes += chunk + encoding_details = await asyncio.to_thread(cchardet.detect, playlist_bytes) + playlist_data = playlist_bytes.decode(encoding_details["encoding"] or "utf-8") # get current contents first if ext in ("m3u", "m3u8"): playlist_items = parse_m3u(playlist_data) @@ -713,8 +724,8 @@ class FileSystemProviderBase(MusicProvider): details=file_item.checksum, ) }, - disc_number=tags.disc, - track_number=tags.track, + disc_number=tags.disc or 0, + track_number=tags.track or 0, ) if isrc_tags := tags.isrc: @@ -724,6 +735,9 @@ class FileSystemProviderBase(MusicProvider): if acoustid := tags.get("acoustidid"): track.external_ids.add((ExternalID.ACOUSTID, acoustid)) + album: Album | None + album_artists: list[Artist] = [] + # album if tags.album: # work out if we have an album and/or disc folder @@ -734,7 +748,6 @@ class FileSystemProviderBase(MusicProvider): album_dir = get_album_dir(file_item.path, tags.album, disc_dir) # album artist(s) - album_artists = [] if tags.album_artists: for index, album_artist_str in enumerate(tags.album_artists): artist = await self._parse_artist(album_artist_str, album_path=album_dir) @@ -776,7 +789,7 @@ class FileSystemProviderBase(MusicProvider): ) album_artists = [await self._parse_artist(name=VARIOUS_ARTISTS_NAME)] - track.album = await self._parse_album( + album = track.album = await self._parse_album( tags.album, track_path=file_item.path, album_path=album_dir, @@ -788,10 +801,8 @@ class FileSystemProviderBase(MusicProvider): # track artist(s) for index, track_artist_str in enumerate(tags.artists): # reuse album artist details if possible - if track.album and ( - album_artist := next( - (x for x in track.album.artists if x.name == track_artist_str), None - ) + if album and ( + album_artist := next((x for x in album_artists if x.name == track_artist_str), None) ): artist = album_artist else: @@ -809,25 +820,27 @@ class FileSystemProviderBase(MusicProvider): # we do not actually embed the image in the metadata because that would consume too # much space and bandwidth. Instead we set the filename as value so the image can # be retrieved later in realtime. - track.metadata.images = [ - MediaItemImage( - type=ImageType.THUMB, - path=file_item.path, - provider=self.instance_id, - remotely_accessible=False, - ) - ] + track.metadata.images = UniqueList( + [ + MediaItemImage( + type=ImageType.THUMB, + path=file_item.path, + provider=self.instance_id, + remotely_accessible=False, + ) + ] + ) - if track.album and not track.album.metadata.images: + if album and not album.metadata.images: # set embedded cover on album if it does not have one yet - track.album.metadata.images = track.metadata.images + album.metadata.images = track.metadata.images # copy album image from track (only if the album itself doesn't have an image) # this deals with embedded images from filesystem providers - if track.album and not track.album.image and track.image: - track.album.metadata.images = [track.image] + if album and not album.image and track.image: + album.metadata.images = UniqueList([track.image]) # parse other info - track.duration = tags.duration or 0 + track.duration = int(tags.duration or 0) track.metadata.genres = set(tags.genres) if tags.disc: track.disc_number = tags.disc @@ -839,20 +852,20 @@ class FileSystemProviderBase(MusicProvider): if explicit_tag is not None: track.metadata.explicit = explicit_tag == "1" track.mbid = tags.musicbrainz_recordingid - track.metadata.chapters = tags.chapters - if track.album: - if not track.album.mbid: - track.album.mbid = tags.musicbrainz_releasegroupid - if not track.album.year: - track.album.year = tags.year - track.album.album_type = tags.album_type - track.album.metadata.explicit = track.metadata.explicit + track.metadata.chapters = UniqueList(tags.chapters) + if album: + if not album.mbid: + album.mbid = tags.musicbrainz_releasegroupid + if not album.year: + album.year = tags.year + album.album_type = tags.album_type + album.metadata.explicit = track.metadata.explicit # set checksum to invalidate any cached listings track.metadata.cache_checksum = file_item.checksum - if track.album: + if album: # use track checksum for album(artists) too - track.album.metadata.cache_checksum = track.metadata.cache_checksum - for artist in track.album.artists: + album.metadata.cache_checksum = track.metadata.cache_checksum + for artist in album_artists: artist.metadata.cache_checksum = track.metadata.cache_checksum return track @@ -867,6 +880,7 @@ class FileSystemProviderBase(MusicProvider): """Parse Artist metadata into an Artist object.""" cache_key = f"{self.instance_id}-artistdata-{name}-{artist_path}" if cache := await self.mass.cache.get(cache_key): + assert isinstance(cache, Artist) return cache if not artist_path and album_path: # try to find (album)artist folder based on album path @@ -936,7 +950,7 @@ class FileSystemProviderBase(MusicProvider): artist.metadata.genres = set(split_items(genre)) # find local images if images := await self._get_local_images(artist_path): - artist.metadata.images = images + artist.metadata.images = UniqueList(images) await self.mass.cache.set(cache_key, artist, expiration=120) return artist @@ -954,6 +968,7 @@ class FileSystemProviderBase(MusicProvider): """Parse Album metadata into an Album object.""" cache_key = f"{self.instance_id}-albumdata-{name}-{album_path}" if cache := await self.mass.cache.get(cache_key): + assert isinstance(cache, Album) return cache if album_path: @@ -971,7 +986,7 @@ class FileSystemProviderBase(MusicProvider): provider=self.instance_id, name=name, sort_name=sort_name, - artists=artists, + artists=UniqueList(artists), provider_mappings={ ProviderMapping( item_id=item_id, @@ -1017,7 +1032,7 @@ class FileSystemProviderBase(MusicProvider): # find local images if images := await self._get_local_images(folder_path): if album.metadata.images is None: - album.metadata.images = images + album.metadata.images = UniqueList(images) else: album.metadata.images += images diff --git a/music_assistant/server/providers/jellyfin/__init__.py b/music_assistant/server/providers/jellyfin/__init__.py index a41f8724..0f6c253a 100644 --- a/music_assistant/server/providers/jellyfin/__init__.py +++ b/music_assistant/server/providers/jellyfin/__init__.py @@ -256,13 +256,13 @@ class JellyfinProvider(MusicProvider): search_results = SearchResults() if artists: - search_results.artists += artists.result() + search_results.artists = artists.result() if albums: - search_results.albums += albums.result() + search_results.albums = albums.result() if tracks: - search_results.tracks += tracks.result() + search_results.tracks = tracks.result() if playlists: - search_results.playlists += playlists.result() + search_results.playlists = playlists.result() return search_results diff --git a/music_assistant/server/providers/radiobrowser/__init__.py b/music_assistant/server/providers/radiobrowser/__init__.py index 50a3ac63..673d6c32 100644 --- a/music_assistant/server/providers/radiobrowser/__init__.py +++ b/music_assistant/server/providers/radiobrowser/__init__.py @@ -99,9 +99,7 @@ class RadioBrowserProvider(MusicProvider): return result searchresult = await self.radios.search(name=search_query, limit=limit) - - for item in searchresult: - result.radio.append(await self._parse_radio(item)) + result.radio = [await self._parse_radio(item) for item in searchresult] return result diff --git a/mypy.ini b/mypy.ini index 2b11bae0..128a6ce9 100644 --- a/mypy.ini +++ b/mypy.ini @@ -21,4 +21,4 @@ disallow_untyped_decorators = true disallow_untyped_defs = true warn_return_any = true warn_unreachable = true -packages=tests,music_assistant.client,music_assistant.common,music_assistant.server.providers.builtin,music_assistant.server.providers.jellyfin,music_assistant.server.providers.radiobrowser +packages=tests,music_assistant.client,music_assistant.common,music_assistant.server.providers.builtin,music_assistant.server.providers.filesystem_local,music_assistant.server.providers.jellyfin,music_assistant.server.providers.radiobrowser -- 2.34.1