Fix entity removal leaks in abode (#183860)

homeassistant/components/abode/camera.py

@@ -1,6 +1,7 @@
 """Support for Abode Security System cameras."""
 
 from datetime import timedelta
+from functools import partial
 from typing import Any, cast, override
 
 from jaraco.abode.devices.base import Device
@@ -10,8 +11,8 @@ import requests
 from requests.models import Response
 
 from homeassistant.components.camera import Camera
-from homeassistant.core import Event, HomeAssistant
-from homeassistant.helpers.dispatcher import async_dispatcher_connect
+from homeassistant.core import HomeAssistant
+from homeassistant.helpers.dispatcher import async_dispatcher_connect, dispatcher_send
 from homeassistant.helpers.entity_platform import AddConfigEntryEntitiesCallback
 from homeassistant.util import Throttle
 
@@ -30,8 +31,17 @@ async def async_setup_entry(
     """Set up Abode camera devices."""
     data = entry.runtime_data
 
+    # jaraco.abode can't remove timeline callbacks, so register one per entry
+    # and let cameras subscribe and unsubscribe through the dispatcher.
+    signal = f"abode_camera_timeline_capture_{entry.entry_id}"
+    await hass.async_add_executor_job(
+        data.abode.events.add_timeline_callback,
+        timeline.CAPTURE_IMAGE,
+        partial(dispatcher_send, hass, signal),
+    )
+
     async_add_entities(
-        AbodeCamera(data, device, timeline.CAPTURE_IMAGE)
+        AbodeCamera(data, device, signal)
         for device in data.abode.get_devices(generic_type="camera")
     )
 
@@ -42,11 +52,11 @@ class AbodeCamera(AbodeDevice, Camera):
     _device: AbodeCam
     _attr_name = None
 
-    def __init__(self, data: AbodeSystem, device: Device, event: Event) -> None:
+    def __init__(self, data: AbodeSystem, device: Device, timeline_signal: str) -> None:
         """Initialize the Abode device."""
         AbodeDevice.__init__(self, data, device)
         Camera.__init__(self)
-        self._event = event
+        self._timeline_signal = timeline_signal
         self._response: Response | None = None
 
     @override
@@ -54,10 +64,10 @@ class AbodeCamera(AbodeDevice, Camera):
         """Subscribe Abode events."""
         await super().async_added_to_hass()
 
-        self.hass.async_add_executor_job(
-            self._data.abode.events.add_timeline_callback,
-            self._event,
-            self._capture_callback,
+        self.async_on_remove(
+            async_dispatcher_connect(
+                self.hass, self._timeline_signal, self._capture_callback
+            )
         )
 
         signal = f"abode_camera_capture_{self.entity_id}"

homeassistant/components/abode/entity.py

@@ -37,6 +37,7 @@ class AbodeEntity(Entity):
     @override
     async def async_will_remove_from_hass(self) -> None:
         """Unsubscribe from Abode connection status updates."""
+        self._data.entity_ids.discard(self.entity_id)
         await self.hass.async_add_executor_job(
             self._data.abode.events.remove_connection_status_callback, self.unique_id
         )

tests/components/abode/test_camera.py

@@ -1,7 +1,10 @@
 """Tests for the Abode camera device."""
 
-from unittest.mock import patch
+from collections.abc import Callable, Generator
+from typing import Any
+from unittest.mock import MagicMock, patch
 
+from jaraco.abode.helpers import timeline
 import pytest
 from syrupy.assertion import SnapshotAssertion
 
@@ -13,7 +16,9 @@ from homeassistant.helpers import entity_registry as er
 
 from .common import setup_platform
 
-from tests.common import snapshot_platform
+from tests.common import MockConfigEntry, snapshot_platform
+
+CAMERA_ENTITY_ID = "camera.test_cam"
 
 
 @pytest.fixture(autouse=True)
@@ -80,3 +85,75 @@ async def test_camera_off(hass: HomeAssistant) -> None:
         )
         await hass.async_block_till_done()
         mock_capture.assert_called_once_with(True)
+
+
+@pytest.fixture
+def mock_update_image_location() -> Generator[MagicMock]:
+    """Mock the camera image location update and skip the image download."""
+    with (
+        patch(
+            "jaraco.abode.devices.camera.Camera.update_image_location"
+        ) as mock_update,
+        patch("homeassistant.components.abode.camera.AbodeCamera.get_image"),
+    ):
+        yield mock_update
+
+
+def _timeline_capture_callbacks(
+    config_entry: MockConfigEntry,
+) -> list[Callable[[Any], None]]:
+    """Return the capture image timeline callbacks registered with jaraco.abode."""
+    events = config_entry.runtime_data.abode.events
+    return events._timeline_callbacks[timeline.CAPTURE_IMAGE["event_code"]]
+
+
+async def _fire_capture(
+    hass: HomeAssistant, callbacks: list[Callable[[Any], None]]
+) -> None:
+    """Fire a capture image timeline event from the jaraco.abode thread."""
+    for capture_callback in callbacks:
+        await hass.async_add_executor_job(capture_callback, timeline.CAPTURE_IMAGE)
+    await hass.async_block_till_done()
+
+
+async def test_timeline_capture_after_entity_removed(
+    hass: HomeAssistant,
+    entity_registry: er.EntityRegistry,
+    mock_update_image_location: MagicMock,
+) -> None:
+    """Test a removed camera no longer handles timeline capture events."""
+    config_entry = await setup_platform(hass, CAMERA_DOMAIN)
+    callbacks = _timeline_capture_callbacks(config_entry)
+
+    await _fire_capture(hass, callbacks)
+    mock_update_image_location.assert_called_once()
+
+    entity_registry.async_remove(CAMERA_ENTITY_ID)
+    await hass.async_block_till_done()
+    assert hass.states.get(CAMERA_ENTITY_ID) is None
+
+    mock_update_image_location.reset_mock()
+    await _fire_capture(hass, callbacks)
+    mock_update_image_location.assert_not_called()
+
+
+async def test_timeline_capture_after_entity_readded(
+    hass: HomeAssistant,
+    entity_registry: er.EntityRegistry,
+    mock_update_image_location: MagicMock,
+) -> None:
+    """Test a re-added camera handles each timeline capture event once."""
+    config_entry = await setup_platform(hass, CAMERA_DOMAIN)
+
+    # Changing the entity_id removes and re-adds the same entity object.
+    entity_registry.async_update_entity(
+        CAMERA_ENTITY_ID, new_entity_id="camera.renamed_cam"
+    )
+    await hass.async_block_till_done()
+    assert hass.states.get("camera.renamed_cam")
+
+    callbacks = _timeline_capture_callbacks(config_entry)
+    assert len(callbacks) == 1
+
+    await _fire_capture(hass, callbacks)
+    mock_update_image_location.assert_called_once()

tests/components/abode/test_switch.py

@@ -101,3 +101,30 @@ async def test_trigger_automation(hass: HomeAssistant) -> None:
         await hass.async_block_till_done()
 
         mock.assert_called_once()
+
+
+async def test_entity_id_tracking_after_removal(
+    hass: HomeAssistant, entity_registry: er.EntityRegistry
+) -> None:
+    """Test a removed switch is no longer tracked in the Abode entity ids."""
+    config_entry = await setup_platform(hass, SWITCH_DOMAIN)
+    assert DEVICE_ID in config_entry.runtime_data.entity_ids
+
+    entity_registry.async_remove(DEVICE_ID)
+    await hass.async_block_till_done()
+
+    assert DEVICE_ID not in config_entry.runtime_data.entity_ids
+
+
+async def test_entity_id_tracking_after_entity_id_change(
+    hass: HomeAssistant, entity_registry: er.EntityRegistry
+) -> None:
+    """Test a renamed switch is tracked by its new entity id only."""
+    config_entry = await setup_platform(hass, SWITCH_DOMAIN)
+
+    # Changing the entity_id removes and re-adds the same entity object.
+    entity_registry.async_update_entity(DEVICE_ID, new_entity_id="switch.renamed")
+    await hass.async_block_till_done()
+
+    assert DEVICE_ID not in config_entry.runtime_data.entity_ids
+    assert "switch.renamed" in config_entry.runtime_data.entity_ids