From 69fccec14729244c98cf6b41504efad687334256 Mon Sep 17 00:00:00 2001 From: mkmer Date: Tue, 19 Dec 2023 01:30:02 -0500 Subject: [PATCH] Clean up device registry for doors that no longer exist in Aladdin Connect (#99743) * Remove devices that no longer exist * Run Black after merge * config 2 devices then 1 devices * clean up device assertions * More generic device check * Add request from Honeywell PR * remove unnecesary test optimize dont_remove * remove unnecessary test * Actually test same id different domain * Test correct id * refactor remove test * Remove .get for non optional keys * Comprehension for all_device_ids * Fix DR test, remove `remove` * fix entities for full test coverage * remove unused variable assignment * Additional assertions confirming other domain * Assertion error * new method for identifier loop * device_entries for lists --- .../components/aladdin_connect/cover.py | 29 ++++ tests/components/aladdin_connect/test_init.py | 128 +++++++++++++++++- 2 files changed, 153 insertions(+), 4 deletions(-) diff --git a/homeassistant/components/aladdin_connect/cover.py b/homeassistant/components/aladdin_connect/cover.py index 604ac61300d..f4104a39365 100644 --- a/homeassistant/components/aladdin_connect/cover.py +++ b/homeassistant/components/aladdin_connect/cover.py @@ -11,6 +11,7 @@ from homeassistant.config_entries import ConfigEntry from homeassistant.const import STATE_CLOSED, STATE_CLOSING, STATE_OPENING from homeassistant.core import HomeAssistant from homeassistant.exceptions import HomeAssistantError, PlatformNotReady +import homeassistant.helpers.device_registry as dr from homeassistant.helpers.device_registry import DeviceInfo from homeassistant.helpers.entity_platform import AddEntitiesCallback @@ -33,6 +34,34 @@ async def async_setup_entry( async_add_entities( (AladdinDevice(acc, door, config_entry) for door in doors), ) + remove_stale_devices(hass, config_entry, doors) + + +def remove_stale_devices( + hass: HomeAssistant, config_entry: ConfigEntry, devices: list[dict] +) -> None: + """Remove stale devices from device registry.""" + device_registry = dr.async_get(hass) + device_entries = dr.async_entries_for_config_entry( + device_registry, config_entry.entry_id + ) + all_device_ids = {f"{door['device_id']}-{door['door_number']}" for door in devices} + + for device_entry in device_entries: + device_id: str | None = None + + for identifier in device_entry.identifiers: + if identifier[0] == DOMAIN: + device_id = identifier[1] + break + + if device_id is None or device_id not in all_device_ids: + # If device_id is None an invalid device entry was found for this config entry. + # If the device_id is not in existing device ids it's a stale device entry. + # Remove config entry from this device entry in either case. + device_registry.async_update_device( + device_entry.id, remove_config_entry_id=config_entry.entry_id + ) class AladdinDevice(CoverEntity): diff --git a/tests/components/aladdin_connect/test_init.py b/tests/components/aladdin_connect/test_init.py index 294ec81b970..2fc09d1641d 100644 --- a/tests/components/aladdin_connect/test_init.py +++ b/tests/components/aladdin_connect/test_init.py @@ -7,10 +7,14 @@ from aiohttp import ClientConnectionError from homeassistant.components.aladdin_connect.const import DOMAIN from homeassistant.config_entries import ConfigEntryState from homeassistant.core import HomeAssistant +from homeassistant.helpers import device_registry as dr + +from .conftest import DEVICE_CONFIG_OPEN from tests.common import AsyncMock, MockConfigEntry CONFIG = {"username": "test-user", "password": "test-password"} +ID = "533255-1" async def test_setup_get_doors_errors(hass: HomeAssistant) -> None: @@ -40,7 +44,7 @@ async def test_setup_login_error( config_entry = MockConfigEntry( domain=DOMAIN, data=CONFIG, - unique_id="test-id", + unique_id=ID, ) config_entry.add_to_hass(hass) mock_aladdinconnect_api.login.return_value = False @@ -59,7 +63,7 @@ async def test_setup_connection_error( config_entry = MockConfigEntry( domain=DOMAIN, data=CONFIG, - unique_id="test-id", + unique_id=ID, ) config_entry.add_to_hass(hass) mock_aladdinconnect_api.login.side_effect = ClientConnectionError @@ -75,7 +79,7 @@ async def test_setup_component_no_error(hass: HomeAssistant) -> None: config_entry = MockConfigEntry( domain=DOMAIN, data=CONFIG, - unique_id="test-id", + unique_id=ID, ) config_entry.add_to_hass(hass) with patch( @@ -116,7 +120,7 @@ async def test_load_and_unload( config_entry = MockConfigEntry( domain=DOMAIN, data=CONFIG, - unique_id="test-id", + unique_id=ID, ) config_entry.add_to_hass(hass) @@ -133,3 +137,119 @@ async def test_load_and_unload( assert await config_entry.async_unload(hass) await hass.async_block_till_done() assert config_entry.state == ConfigEntryState.NOT_LOADED + + +async def test_stale_device_removal( + hass: HomeAssistant, mock_aladdinconnect_api: MagicMock +) -> None: + """Test component setup missing door device is removed.""" + DEVICE_CONFIG_DOOR_2 = { + "device_id": 533255, + "door_number": 2, + "name": "home 2", + "status": "open", + "link_status": "Connected", + "serial": "12346", + "model": "02", + } + config_entry = MockConfigEntry( + domain=DOMAIN, + data=CONFIG, + unique_id=ID, + ) + config_entry.add_to_hass(hass) + mock_aladdinconnect_api.get_doors = AsyncMock( + return_value=[DEVICE_CONFIG_OPEN, DEVICE_CONFIG_DOOR_2] + ) + config_entry_other = MockConfigEntry( + domain="OtherDomain", + data=CONFIG, + unique_id="unique_id", + ) + config_entry_other.add_to_hass(hass) + + device_registry = dr.async_get(hass) + device_entry_other = device_registry.async_get_or_create( + config_entry_id=config_entry_other.entry_id, + identifiers={("OtherDomain", "533255-2")}, + ) + device_registry.async_update_device( + device_entry_other.id, + add_config_entry_id=config_entry.entry_id, + merge_identifiers={(DOMAIN, "533255-2")}, + ) + + with patch( + "homeassistant.components.aladdin_connect.AladdinConnectClient", + return_value=mock_aladdinconnect_api, + ): + assert await hass.config_entries.async_setup(config_entry.entry_id) + await hass.async_block_till_done() + + assert config_entry.state == ConfigEntryState.LOADED + assert len(hass.config_entries.async_entries(DOMAIN)) == 1 + + device_registry = dr.async_get(hass) + + device_entries = dr.async_entries_for_config_entry( + device_registry, config_entry.entry_id + ) + + assert len(device_entries) == 2 + assert any((DOMAIN, "533255-1") in device.identifiers for device in device_entries) + assert any((DOMAIN, "533255-2") in device.identifiers for device in device_entries) + assert any( + ("OtherDomain", "533255-2") in device.identifiers for device in device_entries + ) + + device_entries_other = dr.async_entries_for_config_entry( + device_registry, config_entry_other.entry_id + ) + assert len(device_entries_other) == 1 + assert any( + (DOMAIN, "533255-2") in device.identifiers for device in device_entries_other + ) + assert any( + ("OtherDomain", "533255-2") in device.identifiers + for device in device_entries_other + ) + + assert await config_entry.async_unload(hass) + await hass.async_block_till_done() + assert config_entry.state == ConfigEntryState.NOT_LOADED + + mock_aladdinconnect_api.get_doors = AsyncMock(return_value=[DEVICE_CONFIG_OPEN]) + with patch( + "homeassistant.components.aladdin_connect.AladdinConnectClient", + return_value=mock_aladdinconnect_api, + ): + assert await hass.config_entries.async_setup(config_entry.entry_id) + await hass.async_block_till_done() + + assert config_entry.state == ConfigEntryState.LOADED + assert len(hass.config_entries.async_entries(DOMAIN)) == 1 + + device_entries = dr.async_entries_for_config_entry( + device_registry, config_entry.entry_id + ) + assert len(device_entries) == 1 + assert any((DOMAIN, "533255-1") in device.identifiers for device in device_entries) + assert not any( + (DOMAIN, "533255-2") in device.identifiers for device in device_entries + ) + assert not any( + ("OtherDomain", "533255-2") in device.identifiers for device in device_entries + ) + + device_entries_other = dr.async_entries_for_config_entry( + device_registry, config_entry_other.entry_id + ) + + assert len(device_entries_other) == 1 + assert any( + ("OtherDomain", "533255-2") in device.identifiers + for device in device_entries_other + ) + assert any( + (DOMAIN, "533255-2") in device.identifiers for device in device_entries_other + )