From: Marcel van der Veldt Date: Fri, 12 Apr 2024 22:54:50 +0000 (+0200) Subject: Fix cleanup on removal of a provider (#1222) X-Git-Url: https://git.kitaultman.com/?a=commitdiff_plain;h=c952a8a5ff823527aeaafd51b4868a167d5f41d5;p=music-assistant-server.git Fix cleanup on removal of a provider (#1222) --- diff --git a/music_assistant/common/helpers/uri.py b/music_assistant/common/helpers/uri.py index f789d86b..7ac7e325 100644 --- a/music_assistant/common/helpers/uri.py +++ b/music_assistant/common/helpers/uri.py @@ -47,7 +47,7 @@ async def parse_uri(uri: str, validate_id: bool = False) -> tuple[MediaType, str provider_instance_id_or_domain, rest = uri.split("://", 1) media_type_str, item_id = rest.split("/", 1) media_type = MediaType(media_type_str) - elif ":" in uri: + elif ":" in uri and len(uri.split(":")) == 3: # spotify new-style uri provider_instance_id_or_domain, media_type_str, item_id = uri.split(":") media_type = MediaType(media_type_str) diff --git a/music_assistant/server/controllers/music.py b/music_assistant/server/controllers/music.py index 20dd5f33..cbf03c37 100644 --- a/music_assistant/server/controllers/music.py +++ b/music_assistant/server/controllers/music.py @@ -59,6 +59,7 @@ if TYPE_CHECKING: DEFAULT_SYNC_INTERVAL = 3 * 60 # default sync interval in minutes CONF_SYNC_INTERVAL = "sync_interval" +CONF_DELETED_PROVIDERS = "deleted_providers" class MusicController(CoreController): @@ -111,6 +112,11 @@ class MusicController(CoreController): await self._setup_database() sync_interval = config.get_value(CONF_SYNC_INTERVAL) self.logger.info("Using a sync interval of %s minutes.", sync_interval) + # make sure to finish any removal jobs + for removed_provider in self.mass.config.get_raw_core_config_value( + self.domain, CONF_DELETED_PROVIDERS, [] + ): + await self.cleanup_provider(removed_provider) self._schedule_sync() async def close(self) -> None: @@ -654,10 +660,26 @@ class MusicController(CoreController): async def cleanup_provider(self, provider_instance: str) -> None: """Cleanup provider records from the database.""" + deleted_providers = self.mass.config.get_raw_core_config_value( + self.domain, CONF_DELETED_PROVIDERS, [] + ) + # we add the provider to this hidden config setting just to make sure that + # we can survive this over a restart to make sure that entries are cleaned up + if provider_instance not in deleted_providers: + deleted_providers.append(provider_instance) + self.mass.config.set_raw_core_config_value( + self.domain, CONF_DELETED_PROVIDERS, deleted_providers + ) + self.mass.config.save(True) + # clean cache items from deleted provider(s) await self.mass.cache.clear(provider_instance) # cleanup media items from db matched to deleted provider + self.logger.info( + "Removing provider %s from library, this can take a a while...", provider_instance + ) + errors = 0 for ctrl in ( # order is important here to recursively cleanup bottom up self.mass.music.radio, @@ -668,7 +690,25 @@ class MusicController(CoreController): ): prov_items = await ctrl.get_library_items_by_prov_id(provider_instance) for item in prov_items: - await ctrl.remove_provider_mappings(item.item_id, provider_instance) + try: + await ctrl.remove_provider_mappings(item.item_id, provider_instance) + except Exception as err: + # we dont want the whole removal process to stall on one item + # so in case of an unexpected error, we log and move on. + self.logger.warning("Error while removing %s: %s", item.uri, str(err)) + errors += 1 + + if errors == 0: + # cleanup successful, remove from the deleted_providers setting + self.logger.info("Provider %s removed from library", provider_instance) + deleted_providers.remove(provider_instance) + self.mass.config.set_raw_core_config_value( + self.domain, CONF_DELETED_PROVIDERS, deleted_providers + ) + else: + self.logger.warning( + "Provider %s was not not fully removed from library", provider_instance + ) def _schedule_sync(self) -> None: """Schedule the periodic sync.""" diff --git a/music_assistant/server/providers/filesystem_local/base.py b/music_assistant/server/providers/filesystem_local/base.py index baf575b6..e3122015 100644 --- a/music_assistant/server/providers/filesystem_local/base.py +++ b/music_assistant/server/providers/filesystem_local/base.py @@ -393,12 +393,12 @@ class FileSystemProviderBase(MusicProvider): await controller.remove_item_from_library(library_item.item_id) # check if any albums need to be cleaned up for album_id in album_ids: - if not self.mass.music.albums.tracks(album_id, "library"): + if not await self.mass.music.albums.tracks(album_id, "library"): await self.mass.music.albums.remove_item_from_library(album_id) # check if any artists need to be cleaned up for artist_id in artist_ids: artist_albums = await self.mass.music.artists.albums(artist_id, "library") - artist_tracks = self.mass.music.artists.tracks(artist_id, "library") + artist_tracks = await self.mass.music.artists.tracks(artist_id, "library") if not (artist_albums or artist_tracks): await self.mass.music.artists.remove_item_from_library(album_id) @@ -672,6 +672,7 @@ class FileSystemProviderBase(MusicProvider): content_type=ContentType.try_parse(tags.format), sample_rate=tags.sample_rate, bit_depth=tags.bits_per_sample, + channels=tags.channels, bit_rate=tags.bit_rate, ), ) diff --git a/music_assistant/server/server.py b/music_assistant/server/server.py index 2e233bdb..e95b42a9 100644 --- a/music_assistant/server/server.py +++ b/music_assistant/server/server.py @@ -493,7 +493,6 @@ class MusicAssistant: for sync_task in self.music.in_progress_syncs: if sync_task.provider_instance == instance_id: sync_task.task.cancel() - await sync_task.task # check if there are no other providers dependent of this provider for dep_prov in self.providers: if dep_prov.manifest.depends_on == provider.domain: