From c67f53dc43d46c69ecb7e78a28c812bb17e9c156 Mon Sep 17 00:00:00 2001 From: Paulus Schoutsen Date: Sat, 1 Feb 2020 08:14:28 -0800 Subject: [PATCH] Remove hour delay before checking for updates (#31368) * Check for updates at startup * Add 100% test coverage for update_coordinator * Address comments --- homeassistant/components/updater/__init__.py | 56 +++---- .../components/updater/binary_sensor.py | 58 +++---- homeassistant/helpers/update_coordinator.py | 28 ++-- tests/components/updater/test_init.py | 156 ++++-------------- tests/helpers/test_update_coordinator.py | 123 ++++++++++++++ 5 files changed, 223 insertions(+), 198 deletions(-) create mode 100644 tests/helpers/test_update_coordinator.py diff --git a/homeassistant/components/updater/__init__.py b/homeassistant/components/updater/__init__.py index 42eb988ed56..826da31c5d5 100644 --- a/homeassistant/components/updater/__init__.py +++ b/homeassistant/components/updater/__init__.py @@ -12,11 +12,9 @@ from distro import linux_distribution # pylint: disable=import-error import voluptuous as vol from homeassistant.const import __version__ as current_version -from homeassistant.helpers import discovery, event +from homeassistant.helpers import discovery, update_coordinator from homeassistant.helpers.aiohttp_client import async_get_clientsession import homeassistant.helpers.config_validation as cv -from homeassistant.helpers.dispatcher import async_dispatcher_send -import homeassistant.util.dt as dt_util _LOGGER = logging.getLogger(__name__) @@ -28,8 +26,6 @@ CONF_COMPONENT_REPORTING = "include_used_components" DOMAIN = "updater" -DISPATCHER_REMOTE_UPDATE = "updater_remote_update" - UPDATER_URL = "https://updater.home-assistant.io/" UPDATER_UUID_FILE = ".uuid" @@ -84,10 +80,6 @@ async def async_setup(hass, config): # This component only makes sense in release versions _LOGGER.info("Running on 'dev', only analytics will be submitted") - hass.async_create_task( - discovery.async_load_platform(hass, "binary_sensor", DOMAIN, {}, config) - ) - config = config.get(DOMAIN, {}) if config.get(CONF_REPORTING): huuid = await hass.async_add_job(_load_uuid, hass) @@ -96,18 +88,17 @@ async def async_setup(hass, config): include_components = config.get(CONF_COMPONENT_REPORTING) - async def check_new_version(now): + async def check_new_version(): """Check if a new version is available and report if one is.""" - result = await get_newest_version(hass, huuid, include_components) + newest, release_notes = await get_newest_version( + hass, huuid, include_components + ) - if result is None: - return - - newest, release_notes = result + _LOGGER.debug("Fetched version %s: %s", newest, release_notes) # Skip on dev - if newest is None or "dev" in current_version: - return + if "dev" in current_version: + return Updater(False, "", "") # Load data from supervisor on Hass.io if hass.components.hassio.is_hassio(): @@ -116,20 +107,29 @@ async def async_setup(hass, config): # Validate version update_available = False if StrictVersion(newest) > StrictVersion(current_version): - _LOGGER.info("The latest available version of Home Assistant is %s", newest) + _LOGGER.debug( + "The latest available version of Home Assistant is %s", newest + ) update_available = True elif StrictVersion(newest) == StrictVersion(current_version): - _LOGGER.info("You are on the latest version (%s) of Home Assistant", newest) + _LOGGER.debug( + "You are on the latest version (%s) of Home Assistant", newest + ) elif StrictVersion(newest) < StrictVersion(current_version): _LOGGER.debug("Local version is newer than the latest version (%s)", newest) - updater = Updater(update_available, newest, release_notes) - async_dispatcher_send(hass, DISPATCHER_REMOTE_UPDATE, updater) + _LOGGER.debug("Update available: %s", update_available) - # Update daily, start 1 hour after startup - _dt = dt_util.utcnow() + timedelta(hours=1) - event.async_track_utc_time_change( - hass, check_new_version, hour=_dt.hour, minute=_dt.minute, second=_dt.second + return Updater(update_available, newest, release_notes) + + coordinator = hass.data[DOMAIN] = update_coordinator.DataUpdateCoordinator( + hass, _LOGGER, "Home Assistant update", check_new_version, timedelta(days=1) + ) + + await coordinator.async_refresh() + + hass.async_create_task( + discovery.async_load_platform(hass, "binary_sensor", DOMAIN, {}, config) ) return True @@ -164,17 +164,17 @@ async def get_newest_version(hass, huuid, include_components): ) except (asyncio.TimeoutError, aiohttp.ClientError): _LOGGER.error("Could not contact Home Assistant Update to check for updates") - return None + raise update_coordinator.UpdateFailed try: res = await req.json() except ValueError: _LOGGER.error("Received invalid JSON from Home Assistant Update") - return None + raise update_coordinator.UpdateFailed try: res = RESPONSE_SCHEMA(res) return res["version"], res["release-notes"] except vol.Invalid: _LOGGER.error("Got unexpected response: %s", res) - return None + raise update_coordinator.UpdateFailed diff --git a/homeassistant/components/updater/binary_sensor.py b/homeassistant/components/updater/binary_sensor.py index 3e026a87d4d..60f5cfedf6e 100644 --- a/homeassistant/components/updater/binary_sensor.py +++ b/homeassistant/components/updater/binary_sensor.py @@ -1,26 +1,24 @@ """Support for Home Assistant Updater binary sensors.""" from homeassistant.components.binary_sensor import BinarySensorDevice -from homeassistant.core import callback -from homeassistant.helpers.dispatcher import async_dispatcher_connect -from . import ATTR_NEWEST_VERSION, ATTR_RELEASE_NOTES, DISPATCHER_REMOTE_UPDATE, Updater +from . import ATTR_NEWEST_VERSION, ATTR_RELEASE_NOTES, DOMAIN as UPDATER_DOMAIN async def async_setup_platform(hass, config, async_add_entities, discovery_info=None): """Set up the updater binary sensors.""" - async_add_entities([UpdaterBinary()]) + if discovery_info is None: + return + + async_add_entities([UpdaterBinary(hass.data[UPDATER_DOMAIN])]) class UpdaterBinary(BinarySensorDevice): """Representation of an updater binary sensor.""" - def __init__(self): + def __init__(self, coordinator): """Initialize the binary sensor.""" - self._update_available = None - self._release_notes = None - self._newest_version = None - self._unsub_dispatcher = None + self.coordinator = coordinator @property def name(self) -> str: @@ -35,12 +33,12 @@ class UpdaterBinary(BinarySensorDevice): @property def is_on(self) -> bool: """Return true if the binary sensor is on.""" - return self._update_available + return self.coordinator.data.update_available @property def available(self) -> bool: """Return True if entity is available.""" - return self._update_available is not None + return not self.coordinator.failed_last_update @property def should_poll(self) -> bool: @@ -50,32 +48,24 @@ class UpdaterBinary(BinarySensorDevice): @property def device_state_attributes(self) -> dict: """Return the optional state attributes.""" - data = super().device_state_attributes - if data is None: - data = {} - if self._release_notes: - data[ATTR_RELEASE_NOTES] = self._release_notes - if self._newest_version: - data[ATTR_NEWEST_VERSION] = self._newest_version + data = {} + if self.coordinator.data.release_notes: + data[ATTR_RELEASE_NOTES] = self.coordinator.data.release_notes + if self.coordinator.data.newest_version: + data[ATTR_NEWEST_VERSION] = self.coordinator.data.newest_version return data async def async_added_to_hass(self): """Register update dispatcher.""" - - @callback - def async_state_update(updater: Updater): - """Update callback.""" - self._newest_version = updater.newest_version - self._release_notes = updater.release_notes - self._update_available = updater.update_available - self.async_schedule_update_ha_state() - - self._unsub_dispatcher = async_dispatcher_connect( - self.hass, DISPATCHER_REMOTE_UPDATE, async_state_update - ) + self.coordinator.async_add_listener(self.async_write_ha_state) async def async_will_remove_from_hass(self): - """Register update dispatcher.""" - if self._unsub_dispatcher is not None: - self._unsub_dispatcher() - self._unsub_dispatcher = None + """When removed from hass.""" + self.coordinator.async_remove_listener(self.async_write_ha_state) + + async def async_update(self): + """Update the entity. + + Only used by the generic entity update service. + """ + await self.coordinator.async_request_refresh() diff --git a/homeassistant/helpers/update_coordinator.py b/homeassistant/helpers/update_coordinator.py index dc990637e31..a882e2880b1 100644 --- a/homeassistant/helpers/update_coordinator.py +++ b/homeassistant/helpers/update_coordinator.py @@ -11,6 +11,9 @@ from homeassistant.util.dt import utcnow from .debounce import Debouncer +REQUEST_REFRESH_DEFAULT_COOLDOWN = 10 +REQUEST_REFRESH_DEFAULT_IMMEDIATE = True + class UpdateFailed(Exception): """Raised when an update has failed.""" @@ -26,7 +29,7 @@ class DataUpdateCoordinator: name: str, update_method: Callable[[], Awaitable], update_interval: timedelta, - request_refresh_debouncer: Debouncer, + request_refresh_debouncer: Optional[Debouncer] = None, ): """Initialize global data updater.""" self.hass = hass @@ -41,8 +44,15 @@ class DataUpdateCoordinator: self._unsub_refresh: Optional[CALLBACK_TYPE] = None self._request_refresh_task: Optional[asyncio.TimerHandle] = None self.failed_last_update = False + if request_refresh_debouncer is None: + request_refresh_debouncer = Debouncer( + hass, + logger, + REQUEST_REFRESH_DEFAULT_COOLDOWN, + REQUEST_REFRESH_DEFAULT_IMMEDIATE, + ) self._debounced_refresh = request_refresh_debouncer - request_refresh_debouncer.function = self._async_do_refresh + request_refresh_debouncer.function = self.async_refresh @callback def async_add_listener(self, update_callback: CALLBACK_TYPE) -> None: @@ -64,14 +74,6 @@ class DataUpdateCoordinator: self._unsub_refresh() self._unsub_refresh = None - async def async_refresh(self) -> None: - """Refresh the data.""" - if self._unsub_refresh: - self._unsub_refresh() - self._unsub_refresh = None - - await self._async_do_refresh() - @callback def _schedule_refresh(self) -> None: """Schedule a refresh.""" @@ -86,7 +88,7 @@ class DataUpdateCoordinator: async def _handle_refresh_interval(self, _now: datetime) -> None: """Handle a refresh interval occurrence.""" self._unsub_refresh = None - await self._async_do_refresh() + await self.async_refresh() async def async_request_refresh(self) -> None: """Request a refresh. @@ -95,8 +97,8 @@ class DataUpdateCoordinator: """ await self._debounced_refresh.async_call() - async def _async_do_refresh(self) -> None: - """Time to update.""" + async def async_refresh(self) -> None: + """Update data.""" if self._unsub_refresh: self._unsub_refresh() self._unsub_refresh = None diff --git a/tests/components/updater/test_init.py b/tests/components/updater/test_init.py index 07b5cb059bf..10fa026db29 100644 --- a/tests/components/updater/test_init.py +++ b/tests/components/updater/test_init.py @@ -1,20 +1,15 @@ """The tests for the Updater component.""" import asyncio -from datetime import timedelta -from unittest.mock import Mock, patch +from unittest.mock import Mock +from asynctest import patch import pytest from homeassistant.components import updater +from homeassistant.helpers.update_coordinator import UpdateFailed from homeassistant.setup import async_setup_component -import homeassistant.util.dt as dt_util -from tests.common import ( - MockDependency, - async_fire_time_changed, - mock_component, - mock_coro, -) +from tests.common import MockDependency, mock_component, mock_coro NEW_VERSION = "10000.0" MOCK_VERSION = "10.0" @@ -32,95 +27,39 @@ def mock_distro(): yield +@pytest.fixture(autouse=True) +def mock_version(): + """Mock current version.""" + with patch("homeassistant.components.updater.current_version", MOCK_VERSION): + yield + + @pytest.fixture(name="mock_get_newest_version") def mock_get_newest_version_fixture(): """Fixture to mock get_newest_version.""" - with patch("homeassistant.components.updater.get_newest_version") as mock: + with patch( + "homeassistant.components.updater.get_newest_version", + return_value=(NEW_VERSION, RELEASE_NOTES), + ) as mock: yield mock -@pytest.fixture(name="mock_get_uuid") +@pytest.fixture(name="mock_get_uuid", autouse=True) def mock_get_uuid_fixture(): """Fixture to mock get_uuid.""" with patch("homeassistant.components.updater._load_uuid") as mock: yield mock -@pytest.fixture(name="mock_utcnow") -def mock_utcnow_fixture(): - """Fixture to mock utcnow.""" - with patch("homeassistant.components.updater.dt_util") as mock: - yield mock.utcnow - - -async def test_new_version_shows_entity_startup( - hass, mock_get_uuid, mock_get_newest_version -): - """Test if binary sensor is unavailable at first.""" - mock_get_uuid.return_value = MOCK_HUUID - mock_get_newest_version.return_value = mock_coro((NEW_VERSION, RELEASE_NOTES)) - - res = await async_setup_component(hass, updater.DOMAIN, {updater.DOMAIN: {}}) - assert res, "Updater failed to set up" - - await hass.async_block_till_done() - assert hass.states.is_state("binary_sensor.updater", "unavailable") - assert "newest_version" not in hass.states.get("binary_sensor.updater").attributes - assert "release_notes" not in hass.states.get("binary_sensor.updater").attributes - - -async def test_rename_entity(hass, mock_get_uuid, mock_get_newest_version, mock_utcnow): - """Test if renaming the binary sensor works correctly.""" - mock_get_uuid.return_value = MOCK_HUUID - mock_get_newest_version.return_value = mock_coro((NEW_VERSION, RELEASE_NOTES)) - - now = dt_util.utcnow() - later = now + timedelta(hours=1) - mock_utcnow.return_value = now - - res = await async_setup_component(hass, updater.DOMAIN, {updater.DOMAIN: {}}) - assert res, "Updater failed to set up" - - await hass.async_block_till_done() - assert hass.states.is_state("binary_sensor.updater", "unavailable") - assert hass.states.get("binary_sensor.new_entity_id") is None - - entity_registry = await hass.helpers.entity_registry.async_get_registry() - entity_registry.async_update_entity( - "binary_sensor.updater", new_entity_id="binary_sensor.new_entity_id" - ) - - await hass.async_block_till_done() - assert hass.states.is_state("binary_sensor.new_entity_id", "unavailable") - assert hass.states.get("binary_sensor.updater") is None - - with patch("homeassistant.components.updater.current_version", MOCK_VERSION): - async_fire_time_changed(hass, later) - await hass.async_block_till_done() - - assert hass.states.is_state("binary_sensor.new_entity_id", "on") - assert hass.states.get("binary_sensor.updater") is None - - async def test_new_version_shows_entity_true( - hass, mock_get_uuid, mock_get_newest_version, mock_utcnow + hass, mock_get_uuid, mock_get_newest_version ): """Test if sensor is true if new version is available.""" mock_get_uuid.return_value = MOCK_HUUID - mock_get_newest_version.return_value = mock_coro((NEW_VERSION, RELEASE_NOTES)) - now = dt_util.utcnow() - later = now + timedelta(hours=1) - mock_utcnow.return_value = now - - res = await async_setup_component(hass, updater.DOMAIN, {updater.DOMAIN: {}}) - assert res, "Updater failed to set up" + assert await async_setup_component(hass, updater.DOMAIN, {updater.DOMAIN: {}}) await hass.async_block_till_done() - with patch("homeassistant.components.updater.current_version", MOCK_VERSION): - async_fire_time_changed(hass, later) - await hass.async_block_till_done() - assert hass.states.is_state("binary_sensor.updater", "on") assert ( hass.states.get("binary_sensor.updater").attributes["newest_version"] @@ -133,23 +72,15 @@ async def test_new_version_shows_entity_true( async def test_same_version_shows_entity_false( - hass, mock_get_uuid, mock_get_newest_version, mock_utcnow + hass, mock_get_uuid, mock_get_newest_version ): """Test if sensor is false if no new version is available.""" mock_get_uuid.return_value = MOCK_HUUID mock_get_newest_version.return_value = mock_coro((MOCK_VERSION, "")) - now = dt_util.utcnow() - later = now + timedelta(hours=1) - mock_utcnow.return_value = now - - res = await async_setup_component(hass, updater.DOMAIN, {updater.DOMAIN: {}}) - assert res, "Updater failed to set up" + assert await async_setup_component(hass, updater.DOMAIN, {updater.DOMAIN: {}}) await hass.async_block_till_done() - with patch("homeassistant.components.updater.current_version", MOCK_VERSION): - async_fire_time_changed(hass, later) - await hass.async_block_till_done() assert hass.states.is_state("binary_sensor.updater", "off") assert ( @@ -159,29 +90,18 @@ async def test_same_version_shows_entity_false( assert "release_notes" not in hass.states.get("binary_sensor.updater").attributes -async def test_disable_reporting( - hass, mock_get_uuid, mock_get_newest_version, mock_utcnow -): +async def test_disable_reporting(hass, mock_get_uuid, mock_get_newest_version): """Test we do not gather analytics when disable reporting is active.""" mock_get_uuid.return_value = MOCK_HUUID mock_get_newest_version.return_value = mock_coro((MOCK_VERSION, "")) - now = dt_util.utcnow() - later = now + timedelta(hours=1) - mock_utcnow.return_value = now - - res = await async_setup_component( + assert await async_setup_component( hass, updater.DOMAIN, {updater.DOMAIN: {"reporting": False}} ) - assert res, "Updater failed to set up" - await hass.async_block_till_done() - with patch("homeassistant.components.updater.current_version", MOCK_VERSION): - async_fire_time_changed(hass, later) - await hass.async_block_till_done() assert hass.states.is_state("binary_sensor.updater", "off") - res = await updater.get_newest_version(hass, MOCK_HUUID, MOCK_CONFIG) + await updater.get_newest_version(hass, MOCK_HUUID, MOCK_CONFIG) call = mock_get_newest_version.mock_calls[0][1] assert call[0] is hass assert call[1] is None @@ -215,9 +135,10 @@ async def test_error_fetching_new_version_timeout(hass): with patch( "homeassistant.helpers.system_info.async_get_system_info", Mock(return_value=mock_coro({"fake": "bla"})), - ), patch("async_timeout.timeout", side_effect=asyncio.TimeoutError): - res = await updater.get_newest_version(hass, MOCK_HUUID, False) - assert res is None + ), patch("async_timeout.timeout", side_effect=asyncio.TimeoutError), pytest.raises( + UpdateFailed + ): + await updater.get_newest_version(hass, MOCK_HUUID, False) async def test_error_fetching_new_version_bad_json(hass, aioclient_mock): @@ -227,9 +148,8 @@ async def test_error_fetching_new_version_bad_json(hass, aioclient_mock): with patch( "homeassistant.helpers.system_info.async_get_system_info", Mock(return_value=mock_coro({"fake": "bla"})), - ): - res = await updater.get_newest_version(hass, MOCK_HUUID, False) - assert res is None + ), pytest.raises(UpdateFailed): + await updater.get_newest_version(hass, MOCK_HUUID, False) async def test_error_fetching_new_version_invalid_response(hass, aioclient_mock): @@ -245,31 +165,21 @@ async def test_error_fetching_new_version_invalid_response(hass, aioclient_mock) with patch( "homeassistant.helpers.system_info.async_get_system_info", Mock(return_value=mock_coro({"fake": "bla"})), - ): - res = await updater.get_newest_version(hass, MOCK_HUUID, False) - assert res is None + ), pytest.raises(UpdateFailed): + await updater.get_newest_version(hass, MOCK_HUUID, False) async def test_new_version_shows_entity_after_hour_hassio( - hass, mock_get_uuid, mock_get_newest_version, mock_utcnow + hass, mock_get_uuid, mock_get_newest_version ): """Test if binary sensor gets updated if new version is available / Hass.io.""" mock_get_uuid.return_value = MOCK_HUUID - mock_get_newest_version.return_value = mock_coro((NEW_VERSION, RELEASE_NOTES)) mock_component(hass, "hassio") hass.data["hassio_hass_version"] = "999.0" - now = dt_util.utcnow() - later = now + timedelta(hours=1) - mock_utcnow.return_value = now - - res = await async_setup_component(hass, updater.DOMAIN, {updater.DOMAIN: {}}) - assert res, "Updater failed to set up" + assert await async_setup_component(hass, updater.DOMAIN, {updater.DOMAIN: {}}) await hass.async_block_till_done() - with patch("homeassistant.components.updater.current_version", MOCK_VERSION): - async_fire_time_changed(hass, later) - await hass.async_block_till_done() assert hass.states.is_state("binary_sensor.updater", "on") assert ( diff --git a/tests/helpers/test_update_coordinator.py b/tests/helpers/test_update_coordinator.py new file mode 100644 index 00000000000..8c792506833 --- /dev/null +++ b/tests/helpers/test_update_coordinator.py @@ -0,0 +1,123 @@ +"""Tests for the update coordinator.""" +from datetime import timedelta +import logging + +from asynctest import CoroutineMock, Mock +import pytest + +from homeassistant.helpers import update_coordinator +from homeassistant.util.dt import utcnow + +from tests.common import async_fire_time_changed + +LOGGER = logging.getLogger(__name__) + + +@pytest.fixture +def crd(hass): + """Coordinator mock.""" + calls = [] + + async def refresh(): + calls.append(None) + return len(calls) + + crd = update_coordinator.DataUpdateCoordinator( + hass, LOGGER, "test", refresh, timedelta(seconds=10), + ) + return crd + + +async def test_async_refresh(crd): + """Test async_refresh for update coordinator.""" + assert crd.data is None + await crd.async_refresh() + assert crd.data == 1 + assert crd.failed_last_update is False + + updates = [] + + def update_callback(): + updates.append(crd.data) + + crd.async_add_listener(update_callback) + + await crd.async_refresh() + + assert updates == [2] + + crd.async_remove_listener(update_callback) + + await crd.async_refresh() + + assert updates == [2] + + +async def test_request_refresh(crd): + """Test request refresh for update coordinator.""" + assert crd.data is None + await crd.async_request_refresh() + assert crd.data == 1 + assert crd.failed_last_update is False + + # Second time we hit the debonuce + await crd.async_request_refresh() + assert crd.data == 1 + assert crd.failed_last_update is False + + +async def test_refresh_fail(crd, caplog): + """Test a failing update function.""" + crd.update_method = CoroutineMock(side_effect=update_coordinator.UpdateFailed) + + await crd.async_refresh() + + assert crd.data is None + assert crd.failed_last_update is True + assert "Error fetching test data" in caplog.text + + crd.update_method = CoroutineMock(return_value=1) + + await crd.async_refresh() + + assert crd.data == 1 + assert crd.failed_last_update is False + + crd.update_method = CoroutineMock(side_effect=ValueError) + caplog.clear() + + await crd.async_refresh() + + assert crd.data == 1 # value from previous fetch + assert crd.failed_last_update is True + assert "Unexpected error fetching test data" in caplog.text + + +async def test_update_interval(hass, crd): + """Test update interval works.""" + # Test we don't update without subscriber + async_fire_time_changed(hass, utcnow() + crd.update_interval) + await hass.async_block_till_done() + assert crd.data is None + + # Add subscriber + update_callback = Mock() + crd.async_add_listener(update_callback) + + # Test twice we update with subscriber + async_fire_time_changed(hass, utcnow() + crd.update_interval) + await hass.async_block_till_done() + assert crd.data == 1 + + async_fire_time_changed(hass, utcnow() + crd.update_interval) + await hass.async_block_till_done() + assert crd.data == 2 + + # Test removing listener + crd.async_remove_listener(update_callback) + + async_fire_time_changed(hass, utcnow() + crd.update_interval) + await hass.async_block_till_done() + + # Test we stop updating after we lose last subscriber + assert crd.data == 2