Fix issues when disabling a player provider (#2388)

After the player refactor some parts of the disabling and deleting of player providers weren't updated yet to use the new APIs.
Also `lookup_key` is now used for `provider_filters` since it was already expected by callers.

diff --git a/music_assistant/controllers/config.py b/music_assistant/controllers/config.py
index 250f3c8dae1f10f8e55c6a62aaeec0cff80e2a02..2d49e91ae33a88eb1d1db7337f516772bf90c3e1 100644 (file)
--- a/music_assistant/controllers/config.py
+++ b/music_assistant/controllers/config.py
@@ -300,11 +300,11 @@ class ConfigController:
             # cleanup entries in library
             await self.mass.music.cleanup_provider(instance_id)
         if existing["type"] == "player":
-            # cleanup entries in player manager
+            # all players should already be removed by now through unload_provider
             for player in list(self.mass.players):
                 if player.provider.instance_id != instance_id:
                     continue
-                self.mass.players.remove(player.player_id, cleanup_config=True)
+                self.mass.players.delete_player_config(player.player_id)
             # cleanup remaining player configs
             for player_conf in list(self.get(CONF_PLAYERS, {}).values()):
                 if player_conf["provider"] == instance_id:
@@ -442,7 +442,7 @@ class ConfigController:
                 raise ActionUnavailable("Can not remove config for an active player!")
             # tell the player manager to remove the player if its lingering around
             # set permanent to false otherwise we end up in an infinite loop
-            self.mass.players.unregister(player_id, permanent=False)
+            await self.mass.players.unregister(player_id, permanent=False)
         # remove the actual config if all of the above passed
         self.remove(conf_key)
         # Also remove the DSP config if it exists
@@ -979,12 +979,7 @@ class ConfigController:
                 if dep_prov.manifest.depends_on == config.domain:
                     await self.mass.unload_provider(dep_prov.instance_id)
             await self.mass.unload_provider(config.instance_id)
-            if config.type == ProviderType.PLAYER:
-                # cleanup entries in player manager
-                for player in self.mass.players.all(return_unavailable=True, return_disabled=True):
-                    if player.provider.instance_id != instance_id:
-                        continue
-                    self.mass.players.remove(player.player_id, cleanup_config=False)
+            # For player providers, unload_provider should have removed all its players by now
         return config
 
     async def _add_provider_config(

diff --git a/music_assistant/controllers/players.py b/music_assistant/controllers/players.py
index f9aea235b9a493449e4205f9b9df01f6abef2ad5..7ac0451fc39293c71fe70fe7a94b9487b621ca20 100644 (file)
--- a/music_assistant/controllers/players.py
+++ b/music_assistant/controllers/players.py
@@ -161,7 +161,7 @@ class PlayerController(CoreController):
 
         :param return_unavailable [bool]: Include unavailable players.
         :param return_disabled [bool]: Include disabled players.
-        :param provider_filter [str]: Optional filter by provider ID.
+        :param provider_filter [str]: Optional filter by provider lookup key.
 
         :return: List of Player objects.
         """
@@ -170,7 +170,7 @@ class PlayerController(CoreController):
             for player in self._players.values()
             if (player.available or return_unavailable)
             and (player.enabled or return_disabled)
-            and (provider_filter is None or player.provider.instance_id == provider_filter)
+            and (provider_filter is None or player.provider.lookup_key == provider_filter)
         ]
 
     @api_command("players/all")
@@ -185,7 +185,7 @@ class PlayerController(CoreController):
 
         :param return_unavailable [bool]: Include unavailable players.
         :param return_disabled [bool]: Include disabled players.
-        :param provider_filter [str]: Optional filter by provider ID.
+        :param provider_filter [str]: Optional filter by provider lookup key.
 
         :return: List of PlayerState objects.
         """
@@ -1242,7 +1242,7 @@ class PlayerController(CoreController):
         self.mass.player_queues.on_player_remove(player_id, permanent=permanent)
         await player.on_unload()
         if permanent:
-            self.mass.config.remove(f"players/{player_id}")
+            self.delete_player_config(player_id)
         self.mass.signal_event(EventType.PLAYER_REMOVED, player_id)
 
     @api_command("players/remove")
@@ -1254,11 +1254,8 @@ class PlayerController(CoreController):
         """
         player = self.get(player_id)
         if player is None:
-            # we simply permanently delete the player by wiping its config
-            conf_key = f"{CONF_PLAYERS}/{player_id}"
-            dsp_conf_key = f"{CONF_PLAYER_DSP}/{player_id}"
-            for key in (conf_key, dsp_conf_key):
-                self.mass.config.remove(key)
+            # we simply permanently delete the player config since it is not registered
+            self.delete_player_config(player_id)
             return
         if player.type == PlayerType.GROUP:
             # Handle group player removal
@@ -1273,6 +1270,20 @@ class PlayerController(CoreController):
                 await group_player.set_members(
                     player_ids_to_remove=[player_id],
                 )
+        # We removed the player and can now clean up its config
+        self.delete_player_config(player_id)
+
+    def delete_player_config(self, player_id: str) -> None:
+        """
+        Permanently delete a player's configuration.
+
+        Should only be called for players that are not registered by the player controller.
+        """
+        # we simply permanently delete the player by wiping its config
+        conf_key = f"{CONF_PLAYERS}/{player_id}"
+        dsp_conf_key = f"{CONF_PLAYER_DSP}/{player_id}"
+        for key in (conf_key, dsp_conf_key):
+            self.mass.config.remove(key)
 
     def signal_player_state_update(
         self,