From 48fca3bb2727b393d29f2246bc4df98ba8c35ed6 Mon Sep 17 00:00:00 2001 From: epenet <6771947+epenet@users.noreply.github.com> Date: Thu, 9 Mar 2023 21:16:52 +0100 Subject: [PATCH] Fix missing debouncer cancel in update coordinator (#89383) * Fix missing debouncer cancel in update coordinator * Improve * Adjust with comment * Adjust again * Simplify PR * Adjust tests to avoid lingering timer * Improve --- homeassistant/helpers/update_coordinator.py | 4 ++++ tests/helpers/test_update_coordinator.py | 14 +++++++++++++- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/homeassistant/helpers/update_coordinator.py b/homeassistant/helpers/update_coordinator.py index e8ca1a1f91d..e9de580bad1 100644 --- a/homeassistant/helpers/update_coordinator.py +++ b/homeassistant/helpers/update_coordinator.py @@ -148,6 +148,8 @@ class DataUpdateCoordinator(BaseDataUpdateCoordinatorProtocol, Generic[_T]): self._unsub_refresh() self._unsub_refresh = None + self._debounced_refresh.async_cancel() + def async_contexts(self) -> Generator[Any, None, None]: """Return all registered contexts.""" yield from ( @@ -163,6 +165,8 @@ class DataUpdateCoordinator(BaseDataUpdateCoordinatorProtocol, Generic[_T]): if self.config_entry and self.config_entry.pref_disable_polling: return + # We do not cancel the debouncer here. If the refresh interval is shorter + # than the debouncer cooldown, this would cause the debounce to never be called if self._unsub_refresh: self._unsub_refresh() self._unsub_refresh = None diff --git a/tests/helpers/test_update_coordinator.py b/tests/helpers/test_update_coordinator.py index 6bba27af654..9f904f08020 100644 --- a/tests/helpers/test_update_coordinator.py +++ b/tests/helpers/test_update_coordinator.py @@ -146,6 +146,9 @@ async def test_request_refresh(crd) -> None: assert crd.data == 1 assert crd.last_update_success is True + # Cleanup to avoid lingering timer + crd._unschedule_refresh() + async def test_request_refresh_no_auto_update(crd_without_update_interval) -> None: """Test request refresh for update coordinator without automatic update.""" @@ -160,6 +163,9 @@ async def test_request_refresh_no_auto_update(crd_without_update_interval) -> No assert crd.data == 1 assert crd.last_update_success is True + # Cleanup to avoid lingering timer + crd._unschedule_refresh() + @pytest.mark.parametrize( "err_msg", @@ -293,7 +299,8 @@ async def test_coordinator_entity(crd: update_coordinator.DataUpdateCoordinator[ ) as mock_async_on_remove: await entity.async_added_to_hass() - assert mock_async_on_remove.called + mock_async_on_remove.assert_called_once() + _on_remove_callback = mock_async_on_remove.call_args[0][0] # Verify we do not update if the entity is disabled crd.last_update_success = False @@ -303,6 +310,11 @@ async def test_coordinator_entity(crd: update_coordinator.DataUpdateCoordinator[ assert list(crd.async_contexts()) == [context] + # Call remove callback to cleanup debouncer and avoid lingering timer + assert len(crd._listeners) == 1 + _on_remove_callback() + assert len(crd._listeners) == 0 + async def test_async_set_updated_data(crd) -> None: """Test async_set_updated_data for update coordinator."""