Fix orphaned devices not being removed during integration startup (home-assistant#155900)

Author: Sab44

homeassistant/components/libre_hardware_monitor/coordinator.py

@@ -4,7 +4,6 @@
 
 from datetime import timedelta
 import logging
-from types import MappingProxyType
 
 from librehardwaremonitor_api import (
     LibreHardwareMonitorClient,
@@ -55,15 +54,11 @@ def __init__(
         device_entries: list[DeviceEntry] = dr.async_entries_for_config_entry(
             registry=dr.async_get(self.hass), config_entry_id=config_entry.entry_id
         )
-        self._previous_devices: MappingProxyType[DeviceId, DeviceName] = (
-            MappingProxyType(
-                {
-                    DeviceId(next(iter(device.identifiers))[1]): DeviceName(device.name)
-                    for device in device_entries
-                    if device.identifiers and device.name
-                }
-            )
-        )
+        self._previous_devices: dict[DeviceId, DeviceName] = {
+            DeviceId(next(iter(device.identifiers))[1]): DeviceName(device.name)
+            for device in device_entries
+            if device.identifiers and device.name
+        }
 
     async def _async_update_data(self) -> LibreHardwareMonitorData:
         try:
@@ -75,7 +70,9 @@ async def _async_update_data(self) -> LibreHardwareMonitorData:
         except LibreHardwareMonitorNoDevicesError as err:
             raise UpdateFailed("No sensor data available, will retry") from err
 
-        await self._async_handle_changes_in_devices(lhm_data.main_device_ids_and_names)
+        await self._async_handle_changes_in_devices(
+            dict(lhm_data.main_device_ids_and_names)
+        )
 
         return lhm_data
 
@@ -92,18 +89,21 @@ async def _async_refresh(
         )
 
     async def _async_handle_changes_in_devices(
-        self, detected_devices: MappingProxyType[DeviceId, DeviceName]
+        self, detected_devices: dict[DeviceId, DeviceName]
     ) -> None:
         """Handle device changes by deleting devices from / adding devices to Home Assistant."""
+        detected_devices = {
+            DeviceId(f"{self.config_entry.entry_id}_{detected_id}"): device_name
+            for detected_id, device_name in detected_devices.items()
+        }
+
         previous_device_ids = set(self._previous_devices.keys())
         detected_device_ids = set(detected_devices.keys())
 
-        if previous_device_ids == detected_device_ids:
-            return
+        _LOGGER.debug("Previous device_ids: %s", previous_device_ids)
+        _LOGGER.debug("Detected device_ids: %s", detected_device_ids)
 
-        if self.data is None:
-            # initial update during integration startup
-            self._previous_devices = detected_devices  # type: ignore[unreachable]
+        if previous_device_ids == detected_device_ids:
             return
 
         if orphaned_devices := previous_device_ids - detected_device_ids:
@@ -114,13 +114,21 @@ async def _async_handle_changes_in_devices(
             device_registry = dr.async_get(self.hass)
             for device_id in orphaned_devices:
                 if device := device_registry.async_get_device(
-                    identifiers={(DOMAIN, f"{self.config_entry.entry_id}_{device_id}")}
+                    identifiers={(DOMAIN, device_id)}
                 ):
+                    _LOGGER.debug(
+                        "Removing device: %s", self._previous_devices[device_id]
+                    )
                     device_registry.async_update_device(
                         device_id=device.id,
                         remove_config_entry_id=self.config_entry.entry_id,
                     )
 
+        if self.data is None:
+            # initial update during integration startup
+            self._previous_devices = detected_devices  # type: ignore[unreachable]
+            return
+
         if new_devices := detected_device_ids - previous_device_ids:
             _LOGGER.warning(
                 "New Device(s) detected, reload integration to add them to Home Assistant: %s",

tests/components/libre_hardware_monitor/test_sensor.py

@@ -4,7 +4,7 @@
 from datetime import timedelta
 import logging
 from types import MappingProxyType
-from unittest.mock import AsyncMock, patch
+from unittest.mock import AsyncMock
 
 from freezegun.api import FrozenDateTimeFactory
 from librehardwaremonitor_api import (
@@ -26,6 +26,7 @@
 from homeassistant.const import STATE_UNAVAILABLE, STATE_UNKNOWN
 from homeassistant.core import HomeAssistant
 from homeassistant.helpers import device_registry as dr, entity_registry as er
+from homeassistant.helpers.device_registry import DeviceEntry
 
 from . import init_integration
 
@@ -152,42 +153,74 @@ async def test_sensor_state_is_unknown_when_no_sensor_data_is_provided(
     assert state.state == STATE_UNKNOWN
 
 
-async def test_orphaned_devices_are_removed(
+async def test_orphaned_devices_are_removed_if_not_present_after_update(
     hass: HomeAssistant,
     mock_lhm_client: AsyncMock,
     mock_config_entry: MockConfigEntry,
     freezer: FrozenDateTimeFactory,
+    device_registry: dr.DeviceRegistry,
 ) -> None:
-    """Test that devices in HA that do not receive updates are removed."""
+    """Test that devices in HA that are not found in LHM's data after sensor update are removed."""
+    orphaned_device = await _mock_orphaned_device(
+        device_registry, hass, mock_config_entry, mock_lhm_client
+    )
+
+    freezer.tick(timedelta(DEFAULT_SCAN_INTERVAL))
+    async_fire_time_changed(hass)
+    await hass.async_block_till_done()
+
+    assert device_registry.async_get(orphaned_device.id) is None
+
+
+async def test_orphaned_devices_are_removed_if_not_present_during_startup(
+    hass: HomeAssistant,
+    mock_lhm_client: AsyncMock,
+    mock_config_entry: MockConfigEntry,
+    device_registry: dr.DeviceRegistry,
+) -> None:
+    """Test that devices in HA that are not found in LHM's data during integration startup are removed."""
+    orphaned_device = await _mock_orphaned_device(
+        device_registry, hass, mock_config_entry, mock_lhm_client
+    )
+
+    hass.config_entries.async_schedule_reload(mock_config_entry.entry_id)
+
+    assert device_registry.async_get(orphaned_device.id) is None
+
+
+async def _mock_orphaned_device(
+    device_registry: dr.DeviceRegistry,
+    hass: HomeAssistant,
+    mock_config_entry: MockConfigEntry,
+    mock_lhm_client: AsyncMock,
+) -> DeviceEntry:
     await init_integration(hass, mock_config_entry)
 
+    removed_device = "lpc-nct6687d-0"
+    previous_data = mock_lhm_client.get_data.return_value
+
     mock_lhm_client.get_data.return_value = LibreHardwareMonitorData(
         main_device_ids_and_names=MappingProxyType(
             {
-                DeviceId("amdcpu-0"): DeviceName("AMD Ryzen 7 7800X3D"),
-                DeviceId("gpu-nvidia-0"): DeviceName("NVIDIA GeForce RTX 4080 SUPER"),
+                device_id: name
+                for (device_id, name) in previous_data.main_device_ids_and_names.items()
+                if device_id != removed_device
+            }
+        ),
+        sensor_data=MappingProxyType(
+            {
+                sensor_id: data
+                for (sensor_id, data) in previous_data.sensor_data.items()
+                if not sensor_id.startswith(removed_device)
             }
         ),
-        sensor_data=mock_lhm_client.get_data.return_value.sensor_data,
     )
 
-    device_registry = dr.async_get(hass)
-    orphaned_device = device_registry.async_get_or_create(
+    return device_registry.async_get_or_create(
         config_entry_id=mock_config_entry.entry_id,
-        identifiers={(DOMAIN, f"{mock_config_entry.entry_id}_lpc-nct6687d-0")},
+        identifiers={(DOMAIN, f"{mock_config_entry.entry_id}_{removed_device}")},
     )
 
-    with patch.object(
-        device_registry,
-        "async_remove_device",
-        wraps=device_registry.async_update_device,
-    ) as mock_remove:
-        freezer.tick(timedelta(DEFAULT_SCAN_INTERVAL))
-        async_fire_time_changed(hass)
-        await hass.async_block_till_done()
-
-        mock_remove.assert_called_once_with(orphaned_device.id)
-
 
 async def test_integration_does_not_log_new_devices_on_first_refresh(
     hass: HomeAssistant,