Fix cleanup on removal of a provider (#1222)
authorMarcel van der Veldt <m.vanderveldt@outlook.com>
Fri, 12 Apr 2024 22:54:50 +0000 (00:54 +0200)
committerGitHub <noreply@github.com>
Fri, 12 Apr 2024 22:54:50 +0000 (00:54 +0200)
music_assistant/common/helpers/uri.py
music_assistant/server/controllers/music.py
music_assistant/server/providers/filesystem_local/base.py
music_assistant/server/server.py

index f789d86bb52ac602fc8d33cccb7eef44b6634482..7ac7e325be6fd5b7e494f7df86d171669d1bbe2d 100644 (file)
@@ -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)
index 20dd5f33aeee920f78057b8d7076df736de158ac..cbf03c3727976042a69a489d8f4fc35b7c59d966 100644 (file)
@@ -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."""
index baf575b67ae7db42aaf336091fc12887dfa44997..e3122015c82ec6539f3c16c11279998e5dd1ab85 100644 (file)
@@ -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,
                     ),
                 )
index 2e233bdbf9866aba918d89b0638eb64110ce3f7a..e95b42a9f374650c274236c54e3c6137a3b11de1 100644 (file)
@@ -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: