Fix matter remove config entry device (#87571)

homeassistant/components/matter/__init__.py

@@ -32,7 +32,7 @@ from .addon import get_addon_manager
 from .api import async_register_api
 from .const import CONF_INTEGRATION_CREATED_ADDON, CONF_USE_ADDON, DOMAIN, LOGGER
 from .device_platform import DEVICE_PLATFORM
-from .helpers import MatterEntryData, get_matter
+from .helpers import MatterEntryData, get_matter, get_node_from_device_entry
 
 CONNECT_TIMEOUT = 10
 LISTEN_READY_TIMEOUT = 30
@@ -192,23 +192,13 @@ async def async_remove_config_entry_device(
     hass: HomeAssistant, config_entry: ConfigEntry, device_entry: dr.DeviceEntry
 ) -> bool:
     """Remove a config entry from a device."""
-    unique_id = None
+    node = await get_node_from_device_entry(hass, device_entry)
 
-    for ident in device_entry.identifiers:
+    if node is None:
-        if ident[0] == DOMAIN:
-            unique_id = ident[1]
-            break
-
-    if not unique_id:
         return True
 
-    matter_entry_data: MatterEntryData = hass.data[DOMAIN][config_entry.entry_id]
+    matter = get_matter(hass)
-    matter_client = matter_entry_data.adapter.matter_client
+    await matter.matter_client.remove_node(node.node_id)
-
-    for node in await matter_client.get_nodes():
-        if node.unique_id == unique_id:
-            await matter_client.remove_node(node.node_id)
-            break
 
     return True
 

homeassistant/components/matter/diagnostics.py

@@ -11,8 +11,7 @@ from homeassistant.config_entries import ConfigEntry
 from homeassistant.core import HomeAssistant
 from homeassistant.helpers import device_registry as dr
 
-from .const import DOMAIN, ID_TYPE_DEVICE_ID
+from .helpers import get_matter, get_node_from_device_entry
-from .helpers import get_device_id, get_matter
 
 ATTRIBUTES_TO_REDACT = {"chip.clusters.Objects.BasicInformation.Attributes.Location"}
 
@@ -53,28 +52,14 @@ async def async_get_device_diagnostics(
 ) -> dict[str, Any]:
     """Return diagnostics for a device."""
     matter = get_matter(hass)
-    device_id_type_prefix = f"{ID_TYPE_DEVICE_ID}_"
-    device_id_full = next(
-        identifier[1]
-        for identifier in device.identifiers
-        if identifier[0] == DOMAIN and identifier[1].startswith(device_id_type_prefix)
-    )
-    device_id = device_id_full.lstrip(device_id_type_prefix)
-
     server_diagnostics = await matter.matter_client.get_diagnostics()
-
+    node = await get_node_from_device_entry(hass, device)
-    node = next(
-        node
-        for node in await matter.matter_client.get_nodes()
-        for node_device in node.node_devices
-        if get_device_id(server_diagnostics.info, node_device) == device_id
-    )
 
     return {
         "server_info": remove_serialization_type(
             dataclass_to_dict(server_diagnostics.info)
         ),
         "node": redact_matter_attributes(
-            remove_serialization_type(dataclass_to_dict(node))
+            remove_serialization_type(dataclass_to_dict(node) if node else {})
         ),
     }

homeassistant/components/matter/helpers.py

@@ -6,8 +6,9 @@ from dataclasses import dataclass
 from typing import TYPE_CHECKING
 
 from homeassistant.core import HomeAssistant, callback
+from homeassistant.helpers import device_registry as dr
 
-from .const import DOMAIN
+from .const import DOMAIN, ID_TYPE_DEVICE_ID
 
 if TYPE_CHECKING:
     from matter_server.common.models.node import MatterNode
@@ -58,3 +59,42 @@ def get_device_id(
     # Append nodedevice(type) to differentiate between a root node
     # and bridge within Home Assistant devices.
     return f"{operational_instance_id}-{node_device.__class__.__name__}"
+
+
+async def get_node_from_device_entry(
+    hass: HomeAssistant, device: dr.DeviceEntry
+) -> MatterNode | None:
+    """Return MatterNode from device entry."""
+    matter = get_matter(hass)
+    device_id_type_prefix = f"{ID_TYPE_DEVICE_ID}_"
+    device_id_full = next(
+        (
+            identifier[1]
+            for identifier in device.identifiers
+            if identifier[0] == DOMAIN
+            and identifier[1].startswith(device_id_type_prefix)
+        ),
+        None,
+    )
+
+    if device_id_full is None:
+        raise ValueError(f"Device {device.id} is not a Matter device")
+
+    device_id = device_id_full.lstrip(device_id_type_prefix)
+    matter_client = matter.matter_client
+    server_info = matter_client.server_info
+
+    if server_info is None:
+        raise RuntimeError("Matter server information is not available")
+
+    node = next(
+        (
+            node
+            for node in await matter_client.get_nodes()
+            for node_device in node.node_devices
+            if get_device_id(server_info, node_device) == device_id
+        ),
+        None,
+    )
+
+    return node

tests/components/matter/test_helpers.py

@@ -3,11 +3,20 @@ from __future__ import annotations
 
 from unittest.mock import MagicMock
 
-from homeassistant.components.matter.helpers import get_device_id
+import pytest
+
+from homeassistant.components.matter.const import DOMAIN
+from homeassistant.components.matter.helpers import (
+    get_device_id,
+    get_node_from_device_entry,
+)
 from homeassistant.core import HomeAssistant
+from homeassistant.helpers import device_registry as dr
 
 from .common import setup_integration_with_node_fixture
 
+from tests.common import MockConfigEntry
+
 
 async def test_get_device_id(
     hass: HomeAssistant,
@@ -20,3 +29,42 @@ async def test_get_device_id(
     device_id = get_device_id(matter_client.server_info, node.node_devices[0])
 
     assert device_id == "00000000000004D2-0000000000000005-MatterNodeDevice"
+
+
+async def test_get_node_from_device_entry(
+    hass: HomeAssistant,
+    matter_client: MagicMock,
+) -> None:
+    """Test get_node_from_device_entry."""
+    device_registry = dr.async_get(hass)
+    other_domain = "other_domain"
+    other_config_entry = MockConfigEntry(domain=other_domain)
+    other_device_entry = device_registry.async_get_or_create(
+        config_entry_id=other_config_entry.entry_id,
+        identifiers={(other_domain, "1234")},
+    )
+    node = await setup_integration_with_node_fixture(
+        hass, "device_diagnostics", matter_client
+    )
+    config_entry = hass.config_entries.async_entries(DOMAIN)[0]
+    device_entry = dr.async_entries_for_config_entry(
+        device_registry, config_entry.entry_id
+    )[0]
+    assert device_entry
+    node_from_device_entry = await get_node_from_device_entry(hass, device_entry)
+
+    assert node_from_device_entry is node
+
+    with pytest.raises(ValueError) as value_error:
+        await get_node_from_device_entry(hass, other_device_entry)
+
+    assert f"Device {other_device_entry.id} is not a Matter device" in str(
+        value_error.value
+    )
+
+    matter_client.server_info = None
+
+    with pytest.raises(RuntimeError) as runtime_error:
+        node_from_device_entry = await get_node_from_device_entry(hass, device_entry)
+
+    assert "Matter server information is not available" in str(runtime_error.value)

tests/components/matter/test_init.py

@@ -2,9 +2,10 @@
 from __future__ import annotations
 
 import asyncio
-from collections.abc import Generator
+from collections.abc import Awaitable, Callable, Generator
 from unittest.mock import AsyncMock, MagicMock, call, patch
 
+from aiohttp import ClientWebSocketResponse
 from matter_server.client.exceptions import CannotConnect, InvalidServerVersion
 from matter_server.common.helpers.util import dataclass_from_dict
 from matter_server.common.models.error import MatterError
@@ -16,9 +17,14 @@ from homeassistant.components.matter.const import DOMAIN
 from homeassistant.config_entries import ConfigEntryDisabler, ConfigEntryState
 from homeassistant.const import STATE_UNAVAILABLE
 from homeassistant.core import HomeAssistant
-from homeassistant.helpers import issue_registry as ir
+from homeassistant.helpers import (
+    device_registry as dr,
+    entity_registry as er,
+    issue_registry as ir,
+)
+from homeassistant.setup import async_setup_component
 
-from .common import load_and_parse_node_fixture
+from .common import load_and_parse_node_fixture, setup_integration_with_node_fixture
 
 from tests.common import MockConfigEntry
 
@@ -587,3 +593,76 @@ async def test_remove_entry(
     assert entry.state is ConfigEntryState.NOT_LOADED
     assert len(hass.config_entries.async_entries(DOMAIN)) == 0
     assert "Failed to uninstall the Matter Server add-on" in caplog.text
+
+
+async def test_remove_config_entry_device(
+    hass: HomeAssistant,
+    matter_client: MagicMock,
+    hass_ws_client: Callable[[HomeAssistant], Awaitable[ClientWebSocketResponse]],
+) -> None:
+    """Test that a device can be removed ok."""
+    assert await async_setup_component(hass, "config", {})
+    await setup_integration_with_node_fixture(hass, "device_diagnostics", matter_client)
+    await hass.async_block_till_done()
+
+    config_entry = hass.config_entries.async_entries(DOMAIN)[0]
+    device_registry = dr.async_get(hass)
+    device_entry = dr.async_entries_for_config_entry(
+        device_registry, config_entry.entry_id
+    )[0]
+    entity_registry = er.async_get(hass)
+    entity_id = "light.m5stamp_lighting_app"
+
+    assert device_entry
+    assert entity_registry.async_get(entity_id)
+    assert hass.states.get(entity_id)
+
+    client = await hass_ws_client(hass)
+    await client.send_json(
+        {
+            "id": 5,
+            "type": "config/device_registry/remove_config_entry",
+            "config_entry_id": config_entry.entry_id,
+            "device_id": device_entry.id,
+        }
+    )
+    response = await client.receive_json()
+    assert response["success"]
+    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)
+
+
+async def test_remove_config_entry_device_no_node(
+    hass: HomeAssistant,
+    matter_client: MagicMock,
+    integration: MockConfigEntry,
+    hass_ws_client: Callable[[HomeAssistant], Awaitable[ClientWebSocketResponse]],
+) -> None:
+    """Test that a device can be removed ok without an existing node."""
+    assert await async_setup_component(hass, "config", {})
+    config_entry = integration
+    device_registry = dr.async_get(hass)
+    device_entry = device_registry.async_get_or_create(
+        config_entry_id=config_entry.entry_id,
+        identifiers={
+            (DOMAIN, "deviceid_00000000000004D2-0000000000000005-MatterNodeDevice")
+        },
+    )
+
+    client = await hass_ws_client(hass)
+    await client.send_json(
+        {
+            "id": 5,
+            "type": "config/device_registry/remove_config_entry",
+            "config_entry_id": config_entry.entry_id,
+            "device_id": device_entry.id,
+        }
+    )
+    response = await client.receive_json()
+    assert response["success"]
+    await hass.async_block_till_done()
+
+    assert not device_registry.async_get(device_entry.id)