From aab7a43d3d6c11c62b0815cb0eee511c888dd309 Mon Sep 17 00:00:00 2001 From: Simone Chemelli Date: Sun, 20 Sep 2026 17:51:14 +0200 Subject: [PATCH] Add stale device removal to Mikrotik (#181760) --- homeassistant/components/mikrotik/__init__.py | 31 +++++- .../components/mikrotik/quality_scale.yaml | 2 +- tests/components/mikrotik/__init__.py | 35 ++++-- tests/components/mikrotik/test_init.py | 105 +++++++++++++++++- 4 files changed, 158 insertions(+), 15 deletions(-) diff --git a/homeassistant/components/mikrotik/__init__.py b/homeassistant/components/mikrotik/__init__.py index d8157f757d62..dda1db9b93bd 100644 --- a/homeassistant/components/mikrotik/__init__.py +++ b/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 diff --git a/homeassistant/components/mikrotik/quality_scale.yaml b/homeassistant/components/mikrotik/quality_scale.yaml index 898c0f804d04..fab069b2c67b 100644 --- a/homeassistant/components/mikrotik/quality_scale.yaml +++ b/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 diff --git a/tests/components/mikrotik/__init__.py b/tests/components/mikrotik/__init__.py index 2391545725c8..5d6fc0443b95 100644 --- a/tests/components/mikrotik/__init__.py +++ b/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() diff --git a/tests/components/mikrotik/test_init.py b/tests/components/mikrotik/test_init.py index 67a5a6b665b8..1e833aebc9a4 100644 --- a/tests/components/mikrotik/test_init.py +++ b/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 + )