From 13349e76ed16183cb7b8542ca2416abd0e4a80ce Mon Sep 17 00:00:00 2001
From: "J. Nick Koston" <nick@koston.org>
Date: Fri, 28 Jul 2023 12:19:20 -0500
Subject: [PATCH] Avoid firing update coordinator callbacks when nothing has
 changed (#97268)

---
 homeassistant/helpers/update_coordinator.py |  22 +++-
 tests/helpers/test_update_coordinator.py    | 115 ++++++++++++++++++++
 2 files changed, 135 insertions(+), 2 deletions(-)

diff --git a/homeassistant/helpers/update_coordinator.py b/homeassistant/helpers/update_coordinator.py
index 36dd7d27d4a..8057e77de4f 100644
--- a/homeassistant/helpers/update_coordinator.py
+++ b/homeassistant/helpers/update_coordinator.py
@@ -54,7 +54,12 @@ class BaseDataUpdateCoordinatorProtocol(Protocol):
 
 
 class DataUpdateCoordinator(BaseDataUpdateCoordinatorProtocol, Generic[_DataT]):
-    """Class to manage fetching data from single endpoint."""
+    """Class to manage fetching data from single endpoint.
+
+    Setting :attr:`always_update` to ``False`` will cause coordinator to only
+    callback listeners when data has changed. This requires that the data
+    implements ``__eq__`` or uses a python object that already does.
+    """
 
     def __init__(
         self,
@@ -65,6 +70,7 @@ class DataUpdateCoordinator(BaseDataUpdateCoordinatorProtocol, Generic[_DataT]):
         update_interval: timedelta | None = None,
         update_method: Callable[[], Awaitable[_DataT]] | None = None,
         request_refresh_debouncer: Debouncer[Coroutine[Any, Any, None]] | None = None,
+        always_update: bool = True,
     ) -> None:
         """Initialize global data updater."""
         self.hass = hass
@@ -74,6 +80,7 @@ class DataUpdateCoordinator(BaseDataUpdateCoordinatorProtocol, Generic[_DataT]):
         self.update_interval = update_interval
         self._shutdown_requested = False
         self.config_entry = config_entries.current_entry.get()
+        self.always_update = always_update
 
         # It's None before the first successful update.
         # Components should call async_config_entry_first_refresh
@@ -277,7 +284,10 @@ class DataUpdateCoordinator(BaseDataUpdateCoordinatorProtocol, Generic[_DataT]):
 
         if log_timing := self.logger.isEnabledFor(logging.DEBUG):
             start = monotonic()
+
         auth_failed = False
+        previous_update_success = self.last_update_success
+        previous_data = self.data
 
         try:
             self.data = await self._async_update_data()
@@ -371,7 +381,15 @@ class DataUpdateCoordinator(BaseDataUpdateCoordinatorProtocol, Generic[_DataT]):
             if not auth_failed and self._listeners and not self.hass.is_stopping:
                 self._schedule_refresh()
 
-        self.async_update_listeners()
+        if not self.last_update_success and not previous_update_success:
+            return
+
+        if (
+            self.always_update
+            or self.last_update_success != previous_update_success
+            or previous_data != self.data
+        ):
+            self.async_update_listeners()
 
     @callback
     def async_set_update_error(self, err: Exception) -> None:
diff --git a/tests/helpers/test_update_coordinator.py b/tests/helpers/test_update_coordinator.py
index 91f761b5bb6..4258a508c34 100644
--- a/tests/helpers/test_update_coordinator.py
+++ b/tests/helpers/test_update_coordinator.py
@@ -594,3 +594,118 @@ async def test_async_set_update_error(
 
     # Remove callbacks to avoid lingering timers
     remove_callbacks()
+
+
+async def test_only_callback_on_change_when_always_update_is_false(
+    crd: update_coordinator.DataUpdateCoordinator[int], caplog: pytest.LogCaptureFixture
+) -> None:
+    """Test we do not callback listeners unless something has actually changed when always_update is false."""
+    update_callback = Mock()
+    crd.always_update = False
+    remove_callbacks = crd.async_add_listener(update_callback)
+    mocked_data = None
+    mocked_exception = None
+
+    async def _update_method() -> int:
+        nonlocal mocked_data
+        nonlocal mocked_exception
+        if mocked_exception is not None:
+            raise mocked_exception
+        return mocked_data
+
+    crd.update_method = _update_method
+
+    mocked_data = {"a": 1}
+    await crd.async_refresh()
+    update_callback.assert_called_once()
+    update_callback.reset_mock()
+
+    mocked_data = {"a": 1}
+    await crd.async_refresh()
+    update_callback.assert_not_called()
+    update_callback.reset_mock()
+
+    mocked_data = None
+    mocked_exception = aiohttp.ClientError("Client Failure #1")
+    await crd.async_refresh()
+    update_callback.assert_called_once()
+    update_callback.reset_mock()
+
+    mocked_data = None
+    mocked_exception = aiohttp.ClientError("Client Failure #1")
+    await crd.async_refresh()
+    update_callback.assert_not_called()
+    update_callback.reset_mock()
+
+    mocked_exception = None
+    mocked_data = {"a": 1}
+    await crd.async_refresh()
+    update_callback.assert_called_once()
+    update_callback.reset_mock()
+
+    mocked_data = {"a": 1}
+    await crd.async_refresh()
+    update_callback.assert_not_called()
+    update_callback.reset_mock()
+
+    mocked_data = {"a": 2}
+    await crd.async_refresh()
+    update_callback.assert_called_once()
+    update_callback.reset_mock()
+
+    mocked_data = {"a": 2}
+    await crd.async_refresh()
+    update_callback.assert_not_called()
+    update_callback.reset_mock()
+
+    mocked_data = {"a": 2, "b": 3}
+    await crd.async_refresh()
+    update_callback.assert_called_once()
+    update_callback.reset_mock()
+
+    remove_callbacks()
+
+
+async def test_always_callback_when_always_update_is_true(
+    crd: update_coordinator.DataUpdateCoordinator[int], caplog: pytest.LogCaptureFixture
+) -> None:
+    """Test we callback listeners even though the data is the same when always_update is True."""
+    update_callback = Mock()
+    remove_callbacks = crd.async_add_listener(update_callback)
+    mocked_data = None
+    mocked_exception = None
+
+    async def _update_method() -> int:
+        nonlocal mocked_data
+        nonlocal mocked_exception
+        if mocked_exception is not None:
+            raise mocked_exception
+        return mocked_data
+
+    crd.update_method = _update_method
+
+    mocked_data = {"a": 1}
+    await crd.async_refresh()
+    update_callback.assert_called_once()
+    update_callback.reset_mock()
+
+    mocked_data = {"a": 1}
+    await crd.async_refresh()
+    update_callback.assert_called_once()
+    update_callback.reset_mock()
+
+    # But still don't fire it if we are only getting
+    # failure over and over
+    mocked_data = None
+    mocked_exception = aiohttp.ClientError("Client Failure #1")
+    await crd.async_refresh()
+    update_callback.assert_called_once()
+    update_callback.reset_mock()
+
+    mocked_data = None
+    mocked_exception = aiohttp.ClientError("Client Failure #1")
+    await crd.async_refresh()
+    update_callback.assert_not_called()
+    update_callback.reset_mock()
+
+    remove_callbacks()