Fix ability to remove orphan device in Music Assistant integration (#139431)

* Fix ability to remove orphan device in Music Assistant integration

* Add test

* Remove orphaned device entries at startup as well

* adjust mocked client

homeassistant/components/music_assistant/__init__.py

@@ -9,7 +9,7 @@ from typing import TYPE_CHECKING
 from music_assistant_client import MusicAssistantClient
 from music_assistant_client.exceptions import CannotConnect, InvalidServerVersion
 from music_assistant_models.enums import EventType
-from music_assistant_models.errors import MusicAssistantError
+from music_assistant_models.errors import ActionUnavailable, MusicAssistantError
 
 from homeassistant.config_entries import ConfigEntry, ConfigEntryState
 from homeassistant.const import CONF_URL, EVENT_HOMEASSISTANT_STOP, Platform
@@ -23,7 +23,7 @@ from homeassistant.helpers.issue_registry import (
     async_delete_issue,
 )
 
-from .actions import register_actions
+from .actions import get_music_assistant_client, register_actions
 from .const import DOMAIN, LOGGER
 
 if TYPE_CHECKING:
@@ -137,6 +137,18 @@ async def async_setup_entry(
         mass.subscribe(handle_player_removed, EventType.PLAYER_REMOVED)
     )
 
+    # check if any playerconfigs have been removed while we were disconnected
+    all_player_configs = await mass.config.get_player_configs()
+    player_ids = {player.player_id for player in all_player_configs}
+    dev_reg = dr.async_get(hass)
+    dev_entries = dr.async_entries_for_config_entry(dev_reg, entry.entry_id)
+    for device in dev_entries:
+        for identifier in device.identifiers:
+            if identifier[0] == DOMAIN and identifier[1] not in player_ids:
+                dev_reg.async_update_device(
+                    device.id, remove_config_entry_id=entry.entry_id
+                )
+
     return True
 
 
@@ -174,3 +186,31 @@ async def async_unload_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
         await mass_entry_data.mass.disconnect()
 
     return unload_ok
+
+
+async def async_remove_config_entry_device(
+    hass: HomeAssistant, config_entry: ConfigEntry, device_entry: dr.DeviceEntry
+) -> bool:
+    """Remove a config entry from a device."""
+    player_id = next(
+        (
+            identifier[1]
+            for identifier in device_entry.identifiers
+            if identifier[0] == DOMAIN
+        ),
+        None,
+    )
+    if player_id is None:
+        # this should not be possible at all, but guard it anyways
+        return False
+    mass = get_music_assistant_client(hass, config_entry.entry_id)
+    if mass.players.get(player_id) is None:
+        # player is already removed on the server, this is an orphaned device
+        return True
+    # try to remove the player from the server
+    try:
+        await mass.config.remove_player_config(player_id)
+    except ActionUnavailable:
+        return False
+    else:
+        return True

tests/components/music_assistant/conftest.py

@@ -8,6 +8,7 @@ from music_assistant_client.music import Music
 from music_assistant_client.player_queues import PlayerQueues
 from music_assistant_client.players import Players
 from music_assistant_models.api import ServerInfoMessage
+from music_assistant_models.config_entries import PlayerConfig
 import pytest
 
 from homeassistant.components.music_assistant.config_flow import CONF_URL
@@ -68,6 +69,21 @@ async def music_assistant_client_fixture() -> AsyncGenerator[MagicMock]:
         client.music = Music(client)
         client.server_url = client.server_info.base_url
         client.get_media_item_image_url = MagicMock(return_value=None)
+        client.config = MagicMock()
+
+        async def get_player_configs() -> list[PlayerConfig]:
+            """Mock get player configs."""
+            # simply return a mock config for each player
+            return [
+                PlayerConfig(
+                    values={},
+                    provider=player.provider,
+                    player_id=player.player_id,
+                )
+                for player in client.players
+            ]
+
+        client.config.get_player_configs = get_player_configs
 
         yield client
 

tests/components/music_assistant/test_init.py

@@ -0,0 +1,70 @@
+"""Test the Music Assistant integration init."""
+
+from __future__ import annotations
+
+from unittest.mock import AsyncMock, MagicMock
+
+from music_assistant_models.errors import ActionUnavailable
+
+from homeassistant.components.music_assistant.const import DOMAIN
+from homeassistant.core import HomeAssistant
+from homeassistant.helpers import device_registry as dr, entity_registry as er
+from homeassistant.setup import async_setup_component
+
+from .common import setup_integration_from_fixtures
+
+from tests.typing import WebSocketGenerator
+
+
+async def test_remove_config_entry_device(
+    hass: HomeAssistant,
+    device_registry: dr.DeviceRegistry,
+    entity_registry: er.EntityRegistry,
+    music_assistant_client: MagicMock,
+    hass_ws_client: WebSocketGenerator,
+) -> None:
+    """Test device removal."""
+    assert await async_setup_component(hass, "config", {})
+    await setup_integration_from_fixtures(hass, music_assistant_client)
+    await hass.async_block_till_done()
+    config_entry = hass.config_entries.async_entries(DOMAIN)[0]
+    client = await hass_ws_client(hass)
+
+    # test if the removal should be denied if the device is still in use
+    device_entry = dr.async_entries_for_config_entry(
+        device_registry, config_entry.entry_id
+    )[0]
+    entity_id = "media_player.test_player_1"
+    assert device_entry
+    assert entity_registry.async_get(entity_id)
+    assert hass.states.get(entity_id)
+    music_assistant_client.config.remove_player_config = AsyncMock(
+        side_effect=ActionUnavailable
+    )
+    response = await client.remove_device(device_entry.id, config_entry.entry_id)
+    assert music_assistant_client.config.remove_player_config.call_count == 1
+    assert response["success"] is False
+
+    # test if the removal should be allowed if the device is not in use
+    music_assistant_client.config.remove_player_config = AsyncMock()
+    response = await client.remove_device(device_entry.id, config_entry.entry_id)
+    assert response["success"] is True
+    await hass.async_block_till_done()
+    assert not device_registry.async_get(device_entry.id)
+    assert not entity_registry.async_get(entity_id)
+    assert not hass.states.get(entity_id)
+
+    # test if the removal succeeds if its no longer provided by the server
+    mass_player_id = "00:00:00:00:00:02"
+    music_assistant_client.players._players.pop(mass_player_id)
+    device_entry = dr.async_entries_for_config_entry(
+        device_registry, config_entry.entry_id
+    )[0]
+    entity_id = "media_player.my_super_test_player_2"
+    assert device_entry
+    assert entity_registry.async_get(entity_id)
+    assert hass.states.get(entity_id)
+    music_assistant_client.config.remove_player_config = AsyncMock()
+    response = await client.remove_device(device_entry.id, config_entry.entry_id)
+    assert music_assistant_client.config.remove_player_config.call_count == 0
+    assert response["success"] is True