Add stale device removal to Mikrotik (#181760)

homeassistant/components/mikrotik/__init__.py

@@ -5,8 +5,9 @@ from typing import Any
 from librouteros import Api
 
 from homeassistant.const import Platform
-from homeassistant.core import HomeAssistant
+from homeassistant.core import HomeAssistant, callback
 from homeassistant.helpers import device_registry as dr
+from homeassistant.util import slugify
 
 from .const import ATTR_MANUFACTURER, DOMAIN
 from .coordinator import (
@@ -56,6 +57,34 @@ async def async_setup_entry(
         sw_version=coordinator.firmware,
     )
 
+    @callback
+    def _async_remove_stale_devices() -> None:
+        """Remove interface devices the hub no longer reports."""
+        known_identifiers = {(DOMAIN, coordinator.serial_num)}
+        for interface in coordinator.api.interfaces:
+            if (mac := interface.get("mac-address")) and (
+                name := interface.get("name")
+            ):
+                known_identifiers.add((DOMAIN, f"{slugify(mac)}_{name}"))
+
+        for device_entry in dr.async_entries_for_config_entry(
+            device_registry, config_entry.entry_id
+        ):
+            own_identifiers = {
+                identifier
+                for identifier in device_entry.identifiers
+                if identifier[0] == DOMAIN
+            }
+            # device-tracker clients are linked by MAC connection only, so an
+            # entry without an own identifier is never an interface device
+            if own_identifiers and own_identifiers.isdisjoint(known_identifiers):
+                device_registry.async_remove_device(device_entry.id)
+
+    _async_remove_stale_devices()
+    config_entry.async_on_unload(
+        coordinator.async_add_listener(_async_remove_stale_devices)
+    )
+
     await hass.config_entries.async_forward_entry_setups(config_entry, PLATFORMS)
 
     return True

homeassistant/components/mikrotik/quality_scale.yaml

@@ -70,7 +70,7 @@ rules:
   repair-issues:
     status: exempt
     comment: no known use cases for repair issues or flows, yet
-  stale-devices: todo
+  stale-devices: done
 
   # Platinum
   async-dependency: todo

tests/components/mikrotik/__init__.py

@@ -1,5 +1,6 @@
 """Tests for the Mikrotik integration."""
 
+from collections.abc import Callable
 from typing import Any
 from unittest.mock import patch
 
@@ -67,6 +68,25 @@ def _build_command_responses(
     }
 
 
+def build_mock_command(responses: dict[str, Any]) -> Callable[..., Any]:
+    """Build a ``MikrotikData.command`` replacement from a command/response map.
+
+    Any command missing from ``responses`` returns an empty dict, matching the
+    hub returning no rows for that service.
+    """
+
+    def mock_command(
+        self,
+        cmd: str,
+        params: dict[str, Any] | None = None,
+        suppress_errors: bool = False,
+        during_setup: bool = False,
+    ) -> Any:
+        return responses.get(cmd, {})
+
+    return mock_command
+
+
 async def setup_integration(
     hass: HomeAssistant,
     config_entry: MockConfigEntry,
@@ -76,16 +96,11 @@ async def setup_integration(
     """Set up the component with mocked Mikrotik command responses."""
     config_entry.add_to_hass(hass)
 
-    def mock_command(
-        self,
-        cmd: str,
-        params: dict[str, Any] | None = None,
-        suppress_errors: bool = False,
-        during_setup: bool = False,
-    ) -> Any:
-        return command_responses.get(cmd, {})
-
-    with patch.object(mikrotik.coordinator.MikrotikData, "command", new=mock_command):
+    with patch.object(
+        mikrotik.coordinator.MikrotikData,
+        "command",
+        new=build_mock_command(command_responses),
+    ):
         assert await hass.config_entries.async_setup(config_entry.entry_id)
         await hass.async_block_till_done()
 

tests/components/mikrotik/test_init.py

@@ -9,12 +9,15 @@ from freezegun.api import FrozenDateTimeFactory
 from librouteros.exceptions import ConnectionClosed, LibRouterosError
 import pytest
 
+from homeassistant.components import mikrotik
 from homeassistant.components.mikrotik.const import (
     ARP,
     CONF_ARP_PING,
     CONF_FORCE_DHCP,
     DHCP,
+    DOMAIN,
     IDENTITY,
+    INTERFACE,
     MIKROTIK_SERVICES,
     PING,
     ROUTERBOARD,
@@ -22,12 +25,23 @@ from homeassistant.components.mikrotik.const import (
 from homeassistant.config_entries import SOURCE_REAUTH, ConfigEntryState
 from homeassistant.const import CONF_VERIFY_SSL
 from homeassistant.core import HomeAssistant
+from homeassistant.helpers import device_registry as dr
 from homeassistant.helpers.update_coordinator import UpdateFailed
-from homeassistant.util import dt as dt_util
+from homeassistant.util import dt as dt_util, slugify
 
-from . import setup_integration
+from . import build_mock_command, setup_integration, setup_mikrotik_entry
 from .conftest import MockConfigEntryFactory
-from .const import ARP_DATA, DHCP_DATA, MOCK_DATA
+from .const import (
+    ARP_DATA,
+    BRIDGE1_INTERFACE,
+    DHCP_DATA,
+    ETHER1_INTERFACE,
+    INTERFACE_DATA,
+    MOCK_DATA,
+    ROUTERBOARD_DATA,
+    TEST_SERIAL_NUMBER,
+    WLAN1_INTERFACE,
+)
 
 from tests.common import async_fire_time_changed
 
@@ -36,6 +50,11 @@ _BASE_COMMAND_RESPONSES: dict[str, list[dict[str, Any]]] = {
 }
 
 
+def _interface_identifier(interface: dict[str, Any]) -> tuple[str, str]:
+    """Return the device registry identifier used for an interface."""
+    return (DOMAIN, f"{slugify(interface['mac-address'])}_{interface['name']}")
+
+
 def _command_side_effect(
     error_cmd: str, error: Exception
 ) -> Callable[..., list[dict[str, Any]]]:
@@ -355,3 +374,83 @@ async def test_unload_entry(
 
     assert entry.state is ConfigEntryState.NOT_LOADED
     mock_api.close.assert_called_once()
+
+
+async def test_stale_interface_devices_are_removed(
+    hass: HomeAssistant,
+    device_registry: dr.DeviceRegistry,
+    mock_config_entry: MockConfigEntryFactory,
+) -> None:
+    """Test interface devices missing from the hub data are removed on setup."""
+    entry = mock_config_entry()
+    entry.add_to_hass(hass)
+
+    stale_device = device_registry.async_get_or_create(
+        config_entry_id=entry.entry_id,
+        identifiers={(DOMAIN, "0a_0b_0c_0d_0e_0f_wlan9")},
+    )
+    # a device-tracker client is linked by MAC connection only and must survive
+    client_device = device_registry.async_get_or_create(
+        config_entry_id=entry.entry_id,
+        connections={(dr.CONNECTION_NETWORK_MAC, "00:00:00:00:00:09")},
+    )
+
+    command = build_mock_command(
+        {
+            MIKROTIK_SERVICES[IDENTITY]: [{"name": "Mikrotik"}],
+            MIKROTIK_SERVICES[ROUTERBOARD]: ROUTERBOARD_DATA,
+            MIKROTIK_SERVICES[INTERFACE]: INTERFACE_DATA,
+        }
+    )
+
+    with patch.object(mikrotik.coordinator.MikrotikData, "command", new=command):
+        assert await hass.config_entries.async_setup(entry.entry_id)
+        await hass.async_block_till_done()
+
+    assert device_registry.async_get(stale_device.id) is None
+    assert device_registry.async_get(client_device.id) is not None
+    assert (
+        device_registry.async_get_device_by_identifier(
+            (DOMAIN, TEST_SERIAL_NUMBER), config_entry_id=entry.entry_id
+        )
+        is not None
+    )
+    assert (
+        device_registry.async_get_device_by_identifier(
+            _interface_identifier(ETHER1_INTERFACE), config_entry_id=entry.entry_id
+        )
+        is not None
+    )
+
+
+async def test_stale_interface_device_removed_on_coordinator_update(
+    hass: HomeAssistant, device_registry: dr.DeviceRegistry
+) -> None:
+    """Test an interface device is removed once the hub stops reporting it."""
+    config_entry = await setup_mikrotik_entry(hass, interface_data=INTERFACE_DATA)
+
+    wlan1_identifier = _interface_identifier(WLAN1_INTERFACE)
+    assert device_registry.async_get_device_by_identifier(
+        wlan1_identifier, config_entry_id=config_entry.entry_id
+    )
+
+    command = build_mock_command(
+        {MIKROTIK_SERVICES[INTERFACE]: [ETHER1_INTERFACE, BRIDGE1_INTERFACE]}
+    )
+
+    with patch.object(mikrotik.coordinator.MikrotikData, "command", new=command):
+        async_fire_time_changed(hass, dt_util.utcnow() + timedelta(seconds=10))
+        await hass.async_block_till_done(wait_background_tasks=True)
+
+    assert (
+        device_registry.async_get_device_by_identifier(
+            wlan1_identifier, config_entry_id=config_entry.entry_id
+        )
+        is None
+    )
+    assert device_registry.async_get_device_by_identifier(
+        _interface_identifier(ETHER1_INTERFACE), config_entry_id=config_entry.entry_id
+    )
+    assert device_registry.async_get_device_by_identifier(
+        (DOMAIN, TEST_SERIAL_NUMBER), config_entry_id=config_entry.entry_id
+    )