mirror of
https://github.com/home-assistant/core
synced 2024-10-04 07:58:43 +02:00
Fix race in TimestampDataUpdateCoordinator (#115542)
* Fix race in TimestampDataUpdateCoordinator The last_update_success_time value was being set after the listeners were fired which could lead to a loop because the listener may re-trigger an update because it thinks the data is stale * coverage * docstring
This commit is contained in:
parent
08e2b655be
commit
edd75a9d5f
@ -401,6 +401,8 @@ class DataUpdateCoordinator(BaseDataUpdateCoordinatorProtocol, Generic[_DataT]):
|
||||
if not auth_failed and self._listeners and not self.hass.is_stopping:
|
||||
self._schedule_refresh()
|
||||
|
||||
self._async_refresh_finished()
|
||||
|
||||
if not self.last_update_success and not previous_update_success:
|
||||
return
|
||||
|
||||
@ -411,6 +413,15 @@ class DataUpdateCoordinator(BaseDataUpdateCoordinatorProtocol, Generic[_DataT]):
|
||||
):
|
||||
self.async_update_listeners()
|
||||
|
||||
@callback
|
||||
def _async_refresh_finished(self) -> None:
|
||||
"""Handle when a refresh has finished.
|
||||
|
||||
Called when refresh is finished before listeners are updated.
|
||||
|
||||
To be overridden by subclasses.
|
||||
"""
|
||||
|
||||
@callback
|
||||
def async_set_update_error(self, err: Exception) -> None:
|
||||
"""Manually set an error, log the message and notify listeners."""
|
||||
@ -444,20 +455,9 @@ class TimestampDataUpdateCoordinator(DataUpdateCoordinator[_DataT]):
|
||||
|
||||
last_update_success_time: datetime | None = None
|
||||
|
||||
async def _async_refresh(
|
||||
self,
|
||||
log_failures: bool = True,
|
||||
raise_on_auth_failed: bool = False,
|
||||
scheduled: bool = False,
|
||||
raise_on_entry_error: bool = False,
|
||||
) -> None:
|
||||
"""Refresh data."""
|
||||
await super()._async_refresh(
|
||||
log_failures,
|
||||
raise_on_auth_failed,
|
||||
scheduled,
|
||||
raise_on_entry_error,
|
||||
)
|
||||
@callback
|
||||
def _async_refresh_finished(self) -> None:
|
||||
"""Handle when a refresh has finished."""
|
||||
if self.last_update_success:
|
||||
self.last_update_success_time = utcnow()
|
||||
|
||||
|
@ -1,6 +1,6 @@
|
||||
"""Tests for the update coordinator."""
|
||||
|
||||
from datetime import timedelta
|
||||
from datetime import datetime, timedelta
|
||||
import logging
|
||||
from unittest.mock import AsyncMock, Mock, patch
|
||||
import urllib.error
|
||||
@ -12,7 +12,7 @@ import requests
|
||||
|
||||
from homeassistant import config_entries
|
||||
from homeassistant.const import EVENT_HOMEASSISTANT_STOP
|
||||
from homeassistant.core import CoreState, HomeAssistant
|
||||
from homeassistant.core import CoreState, HomeAssistant, callback
|
||||
from homeassistant.exceptions import ConfigEntryNotReady
|
||||
from homeassistant.helpers import update_coordinator
|
||||
from homeassistant.util.dt import utcnow
|
||||
@ -715,3 +715,35 @@ async def test_always_callback_when_always_update_is_true(
|
||||
update_callback.reset_mock()
|
||||
|
||||
remove_callbacks()
|
||||
|
||||
|
||||
async def test_timestamp_date_update_coordinator(hass: HomeAssistant) -> None:
|
||||
"""Test last_update_success_time is set before calling listeners."""
|
||||
last_update_success_times: list[datetime | None] = []
|
||||
|
||||
async def refresh() -> int:
|
||||
return 1
|
||||
|
||||
crd = update_coordinator.TimestampDataUpdateCoordinator[int](
|
||||
hass,
|
||||
_LOGGER,
|
||||
name="test",
|
||||
update_method=refresh,
|
||||
update_interval=timedelta(seconds=10),
|
||||
)
|
||||
|
||||
@callback
|
||||
def listener():
|
||||
last_update_success_times.append(crd.last_update_success_time)
|
||||
|
||||
unsub = crd.async_add_listener(listener)
|
||||
|
||||
await crd.async_refresh()
|
||||
|
||||
assert len(last_update_success_times) == 1
|
||||
# Ensure the time is set before the listener is called
|
||||
assert last_update_success_times != [None]
|
||||
|
||||
unsub()
|
||||
await crd.async_refresh()
|
||||
assert len(last_update_success_times) == 1
|
||||
|
Loading…
Reference in New Issue
Block a user