From 882f5ed0794c2dbfc1ff8ffa291f44a3809af381 Mon Sep 17 00:00:00 2001 From: Paulus Schoutsen Date: Wed, 13 Feb 2019 20:36:06 -0800 Subject: [PATCH] Don't directly update config entries (#20877) * Don't directly update config entries * Use ConfigEntryNotReady * Fix tests * Remove old test * Lint --- homeassistant/components/deconz/gateway.py | 29 +++------------ .../components/homematicip_cloud/hap.py | 20 ++--------- homeassistant/components/hue/bridge.py | 24 ++----------- homeassistant/components/unifi/controller.py | 24 ++----------- homeassistant/config_entries.py | 6 ++-- tests/components/deconz/test_gateway.py | 25 ++++--------- tests/components/deconz/test_init.py | 9 +++-- .../components/homematicip_cloud/test_hap.py | 10 ++++-- tests/components/hue/test_bridge.py | 33 ++++------------- tests/components/unifi/test_controller.py | 36 ++++--------------- tests/test_config_entries.py | 7 +++- 11 files changed, 54 insertions(+), 169 deletions(-) diff --git a/homeassistant/components/deconz/gateway.py b/homeassistant/components/deconz/gateway.py index 8d33e011b943..fe9fc4b77523 100644 --- a/homeassistant/components/deconz/gateway.py +++ b/homeassistant/components/deconz/gateway.py @@ -1,5 +1,5 @@ """Representation of a deCONZ gateway.""" -from homeassistant import config_entries +from homeassistant.exceptions import ConfigEntryNotReady from homeassistant.const import CONF_EVENT, CONF_ID from homeassistant.core import EventOrigin, callback from homeassistant.helpers import aiohttp_client @@ -8,7 +8,7 @@ from homeassistant.helpers.dispatcher import ( from homeassistant.util import slugify from .const import ( - _LOGGER, DECONZ_REACHABLE, CONF_ALLOW_CLIP_SENSOR, SUPPORTED_PLATFORMS) + DECONZ_REACHABLE, CONF_ALLOW_CLIP_SENSOR, SUPPORTED_PLATFORMS) class DeconzGateway: @@ -20,7 +20,6 @@ class DeconzGateway: self.config_entry = config_entry self.available = True self.api = None - self._cancel_retry_setup = None self.deconz_ids = {} self.events = [] @@ -35,22 +34,8 @@ class DeconzGateway: self.async_connection_status_callback ) - if self.api is False: - retry_delay = 2 ** (tries + 1) - _LOGGER.error( - "Error connecting to deCONZ gateway. Retrying in %d seconds", - retry_delay) - - async def retry_setup(_now): - """Retry setup.""" - if await self.async_setup(tries + 1): - # This feels hacky, we should find a better way to do this - self.config_entry.state = config_entries.ENTRY_STATE_LOADED - - self._cancel_retry_setup = hass.helpers.event.async_call_later( - retry_delay, retry_setup) - - return False + if not self.api: + raise ConfigEntryNotReady for component in SUPPORTED_PLATFORMS: hass.async_create_task( @@ -107,12 +92,6 @@ class DeconzGateway: Will cancel any scheduled setup retry and will unload the config entry. """ - # If we have a retry scheduled, we were never setup. - if self._cancel_retry_setup is not None: - self._cancel_retry_setup() - self._cancel_retry_setup = None - return True - self.api.close() for component in SUPPORTED_PLATFORMS: diff --git a/homeassistant/components/homematicip_cloud/hap.py b/homeassistant/components/homematicip_cloud/hap.py index 37507a1fca83..9af6669652d9 100644 --- a/homeassistant/components/homematicip_cloud/hap.py +++ b/homeassistant/components/homematicip_cloud/hap.py @@ -2,7 +2,7 @@ import asyncio import logging -from homeassistant import config_entries +from homeassistant.exceptions import ConfigEntryNotReady from homeassistant.core import callback from homeassistant.helpers.aiohttp_client import async_get_clientsession @@ -84,7 +84,6 @@ class HomematicipHAP: self._retry_task = None self._tries = 0 self._accesspoint_connected = True - self._retry_setup = None async def async_setup(self, tries=0): """Initialize connection.""" @@ -96,20 +95,7 @@ class HomematicipHAP: self.config_entry.data.get(HMIPC_NAME) ) except HmipcConnectionError: - retry_delay = 2 ** min(tries, 8) - _LOGGER.error("Error connecting to HomematicIP with HAP %s. " - "Retrying in %d seconds", - self.config_entry.data.get(HMIPC_HAPID), retry_delay) - - async def retry_setup(_now): - """Retry setup.""" - if await self.async_setup(tries + 1): - self.config_entry.state = config_entries.ENTRY_STATE_LOADED - - self._retry_setup = self.hass.helpers.event.async_call_later( - retry_delay, retry_setup) - - return False + raise ConfigEntryNotReady _LOGGER.info("Connected to HomematicIP with HAP %s", self.config_entry.data.get(HMIPC_HAPID)) @@ -209,8 +195,6 @@ class HomematicipHAP: async def async_reset(self): """Close the websocket connection.""" self._ws_close_requested = True - if self._retry_setup is not None: - self._retry_setup.cancel() if self._retry_task is not None: self._retry_task.cancel() await self.home.disable_events() diff --git a/homeassistant/components/hue/bridge.py b/homeassistant/components/hue/bridge.py index 93241622f0ba..6e3d818db682 100644 --- a/homeassistant/components/hue/bridge.py +++ b/homeassistant/components/hue/bridge.py @@ -5,6 +5,7 @@ import async_timeout import voluptuous as vol from homeassistant import config_entries +from homeassistant.exceptions import ConfigEntryNotReady from homeassistant.helpers import aiohttp_client, config_validation as cv from .const import DOMAIN, LOGGER @@ -30,7 +31,6 @@ class HueBridge: self.allow_groups = allow_groups self.available = True self.api = None - self._cancel_retry_setup = None @property def host(self): @@ -59,20 +59,7 @@ class HueBridge: return False except CannotConnect: - retry_delay = 2 ** (tries + 1) - LOGGER.error("Error connecting to the Hue bridge at %s. Retrying " - "in %d seconds", host, retry_delay) - - async def retry_setup(_now): - """Retry setup.""" - if await self.async_setup(tries + 1): - # This feels hacky, we should find a better way to do this - self.config_entry.state = config_entries.ENTRY_STATE_LOADED - - self._cancel_retry_setup = hass.helpers.event.async_call_later( - retry_delay, retry_setup) - - return False + raise ConfigEntryNotReady except Exception: # pylint: disable=broad-except LOGGER.exception('Unknown error connecting with Hue bridge at %s', @@ -97,13 +84,6 @@ class HueBridge: # The bridge can be in 3 states: # - Setup was successful, self.api is not None # - Authentication was wrong, self.api is None, not retrying setup. - # - Host was down. self.api is None, we're retrying setup - - # If we have a retry scheduled, we were never setup. - if self._cancel_retry_setup is not None: - self._cancel_retry_setup() - self._cancel_retry_setup = None - return True # If the authentication was wrong. if self.api is None: diff --git a/homeassistant/components/unifi/controller.py b/homeassistant/components/unifi/controller.py index abd102f6187b..2b9aa89fef24 100644 --- a/homeassistant/components/unifi/controller.py +++ b/homeassistant/components/unifi/controller.py @@ -4,7 +4,7 @@ import async_timeout from aiohttp import CookieJar -from homeassistant import config_entries +from homeassistant.exceptions import ConfigEntryNotReady from homeassistant.const import CONF_HOST from homeassistant.helpers import aiohttp_client @@ -22,7 +22,6 @@ class UniFiController: self.available = True self.api = None self.progress = None - self._cancel_retry_setup = None @property def host(self): @@ -47,20 +46,7 @@ class UniFiController: await self.api.initialize() except CannotConnect: - retry_delay = 2 ** (tries + 1) - LOGGER.error("Error connecting to the UniFi controller. Retrying " - "in %d seconds", retry_delay) - - async def retry_setup(_now): - """Retry setup.""" - if await self.async_setup(tries + 1): - # This feels hacky, we should find a better way to do this - self.config_entry.state = config_entries.ENTRY_STATE_LOADED - - self._cancel_retry_setup = hass.helpers.event.async_call_later( - retry_delay, retry_setup) - - return False + raise ConfigEntryNotReady except Exception: # pylint: disable=broad-except LOGGER.error( @@ -80,12 +66,6 @@ class UniFiController: Will cancel any scheduled setup retry and will unload the config entry. """ - # If we have a retry scheduled, we were never setup. - if self._cancel_retry_setup is not None: - self._cancel_retry_setup() - self._cancel_retry_setup = None - return True - # If the authentication was wrong. if self.api is None: return True diff --git a/homeassistant/config_entries.py b/homeassistant/config_entries.py index 7c6da2644f66..c7dfc0c889b9 100644 --- a/homeassistant/config_entries.py +++ b/homeassistant/config_entries.py @@ -127,6 +127,7 @@ from homeassistant.util.decorator import Registry _LOGGER = logging.getLogger(__name__) +_UNDEF = object() SOURCE_USER = 'user' SOURCE_DISCOVERY = 'discovery' @@ -441,9 +442,10 @@ class ConfigEntries: for entry in config['entries']] @callback - def async_update_entry(self, entry, *, data): + def async_update_entry(self, entry, *, data=_UNDEF): """Update a config entry.""" - entry.data = data + if data is not _UNDEF: + entry.data = data self._async_schedule_save() async def async_forward_entry_setup(self, entry, component): diff --git a/tests/components/deconz/test_gateway.py b/tests/components/deconz/test_gateway.py index 3411f96b981e..dbc45c955b56 100644 --- a/tests/components/deconz/test_gateway.py +++ b/tests/components/deconz/test_gateway.py @@ -1,6 +1,9 @@ """Test deCONZ gateway.""" from unittest.mock import Mock, patch +import pytest + +from homeassistant.exceptions import ConfigEntryNotReady from homeassistant.components.deconz import gateway from tests.common import mock_coro @@ -56,8 +59,10 @@ async def test_gateway_retry(): deconz_gateway = gateway.DeconzGateway(hass, entry) - with patch.object(gateway, 'get_gateway', return_value=mock_coro(False)): - assert await deconz_gateway.async_setup() is False + with patch.object( + gateway, 'get_gateway', return_value=mock_coro(False) + ), pytest.raises(ConfigEntryNotReady): + await deconz_gateway.async_setup() async def test_connection_status(hass): @@ -118,22 +123,6 @@ async def test_shutdown(): assert len(deconz_gateway.api.close.mock_calls) == 1 -async def test_reset_cancel_retry(): - """Verify async reset can handle a scheduled retry.""" - hass = Mock() - entry = Mock() - entry.data = ENTRY_CONFIG - - deconz_gateway = gateway.DeconzGateway(hass, entry) - - with patch.object(gateway, 'get_gateway', return_value=mock_coro(False)): - assert await deconz_gateway.async_setup() is False - - assert deconz_gateway._cancel_retry_setup is not None - - assert await deconz_gateway.async_reset() is True - - async def test_reset_after_successful_setup(): """Verify that reset works on a setup component.""" hass = Mock() diff --git a/tests/components/deconz/test_init.py b/tests/components/deconz/test_init.py index 5fa8ddcfe38e..cbba47eb4315 100644 --- a/tests/components/deconz/test_init.py +++ b/tests/components/deconz/test_init.py @@ -4,6 +4,7 @@ from unittest.mock import Mock, patch import pytest import voluptuous as vol +from homeassistant.exceptions import ConfigEntryNotReady from homeassistant.setup import async_setup_component from homeassistant.components import deconz @@ -79,9 +80,11 @@ async def test_setup_entry_no_available_bridge(hass): """Test setup entry fails if deCONZ is not available.""" entry = Mock() entry.data = {'host': '1.2.3.4', 'port': 80, 'api_key': '1234567890ABCDEF'} - with patch('pydeconz.DeconzSession.async_load_parameters', - return_value=mock_coro(False)): - assert await deconz.async_setup_entry(hass, entry) is False + with patch( + 'pydeconz.DeconzSession.async_load_parameters', + return_value=mock_coro(False) + ), pytest.raises(ConfigEntryNotReady): + await deconz.async_setup_entry(hass, entry) async def test_setup_entry_successful(hass): diff --git a/tests/components/homematicip_cloud/test_hap.py b/tests/components/homematicip_cloud/test_hap.py index 521920b92815..c39e7d4e26bd 100644 --- a/tests/components/homematicip_cloud/test_hap.py +++ b/tests/components/homematicip_cloud/test_hap.py @@ -1,6 +1,9 @@ """Test HomematicIP Cloud accesspoint.""" from unittest.mock import Mock, patch +import pytest + +from homeassistant.exceptions import ConfigEntryNotReady from homeassistant.components.homematicip_cloud import hap as hmipc from homeassistant.components.homematicip_cloud import const, errors from tests.common import mock_coro, mock_coro_func @@ -82,9 +85,10 @@ async def test_hap_setup_connection_error(): hmipc.HMIPC_NAME: 'hmip', } hap = hmipc.HomematicipHAP(hass, entry) - with patch.object(hap, 'get_hap', - side_effect=errors.HmipcConnectionError): - assert await hap.async_setup() is False + with patch.object( + hap, 'get_hap', side_effect=errors.HmipcConnectionError + ), pytest.raises(ConfigEntryNotReady): + await hap.async_setup() assert len(hass.async_add_job.mock_calls) == 0 assert len(hass.config_entries.flow.async_init.mock_calls) == 0 diff --git a/tests/components/hue/test_bridge.py b/tests/components/hue/test_bridge.py index ceb30091970f..855a12e26208 100644 --- a/tests/components/hue/test_bridge.py +++ b/tests/components/hue/test_bridge.py @@ -1,6 +1,9 @@ """Test Hue bridge.""" from unittest.mock import Mock, patch +import pytest + +from homeassistant.exceptions import ConfigEntryNotReady from homeassistant.components.hue import bridge, errors from tests.common import mock_coro @@ -48,32 +51,10 @@ async def test_bridge_setup_timeout(hass): entry.data = {'host': '1.2.3.4', 'username': 'mock-username'} hue_bridge = bridge.HueBridge(hass, entry, False, False) - with patch.object(bridge, 'get_bridge', side_effect=errors.CannotConnect): - assert await hue_bridge.async_setup() is False - - assert len(hass.helpers.event.async_call_later.mock_calls) == 1 - # Assert we are going to wait 2 seconds - assert hass.helpers.event.async_call_later.mock_calls[0][1][0] == 2 - - -async def test_reset_cancels_retry_setup(): - """Test resetting a bridge while we're waiting to retry setup.""" - hass = Mock() - entry = Mock() - entry.data = {'host': '1.2.3.4', 'username': 'mock-username'} - hue_bridge = bridge.HueBridge(hass, entry, False, False) - - with patch.object(bridge, 'get_bridge', side_effect=errors.CannotConnect): - assert await hue_bridge.async_setup() is False - - mock_call_later = hass.helpers.event.async_call_later - - assert len(mock_call_later.mock_calls) == 1 - - assert await hue_bridge.async_reset() - - assert len(mock_call_later.mock_calls) == 2 - assert len(mock_call_later.return_value.mock_calls) == 1 + with patch.object( + bridge, 'get_bridge', side_effect=errors.CannotConnect + ), pytest.raises(ConfigEntryNotReady): + await hue_bridge.async_setup() async def test_reset_if_entry_had_wrong_auth(): diff --git a/tests/components/unifi/test_controller.py b/tests/components/unifi/test_controller.py index b3b222d902ae..e5e1d84bfcdf 100644 --- a/tests/components/unifi/test_controller.py +++ b/tests/components/unifi/test_controller.py @@ -1,6 +1,9 @@ """Test UniFi Controller.""" from unittest.mock import Mock, patch +import pytest + +from homeassistant.exceptions import ConfigEntryNotReady from homeassistant.components import unifi from homeassistant.components.unifi import controller, errors @@ -103,13 +106,10 @@ async def test_controller_not_accessible(): unifi_controller = controller.UniFiController(hass, entry) - with patch.object(controller, 'get_controller', - side_effect=errors.CannotConnect): - assert await unifi_controller.async_setup() is False - - assert len(hass.helpers.event.async_call_later.mock_calls) == 1 - # Assert we are going to wait 2 seconds - assert hass.helpers.event.async_call_later.mock_calls[0][1][0] == 2 + with patch.object( + controller, 'get_controller', side_effect=errors.CannotConnect + ), pytest.raises(ConfigEntryNotReady): + await unifi_controller.async_setup() async def test_controller_unknown_error(): @@ -128,28 +128,6 @@ async def test_controller_unknown_error(): assert not hass.helpers.event.async_call_later.mock_calls -async def test_reset_cancels_retry_setup(): - """Resetting a controller while we're waiting to retry setup.""" - hass = Mock() - entry = Mock() - entry.data = ENTRY_CONFIG - - unifi_controller = controller.UniFiController(hass, entry) - - with patch.object(controller, 'get_controller', - side_effect=errors.CannotConnect): - assert await unifi_controller.async_setup() is False - - mock_call_later = hass.helpers.event.async_call_later - - assert len(mock_call_later.mock_calls) == 1 - - assert await unifi_controller.async_reset() - - assert len(mock_call_later.mock_calls) == 2 - assert len(mock_call_later.return_value.mock_calls) == 1 - - async def test_reset_if_entry_had_wrong_auth(): """Calling reset when the entry contains wrong auth.""" hass = Mock() diff --git a/tests/test_config_entries.py b/tests/test_config_entries.py index 59777e2e6bbc..496ad7852754 100644 --- a/tests/test_config_entries.py +++ b/tests/test_config_entries.py @@ -411,13 +411,18 @@ async def test_updating_entry_data(manager): entry = MockConfigEntry( domain='test', data={'first': True}, + state=config_entries.ENTRY_STATE_SETUP_ERROR, ) entry.add_to_manager(manager) + manager.async_update_entry(entry) + assert entry.data == { + 'first': True + } + manager.async_update_entry(entry, data={ 'second': True }) - assert entry.data == { 'second': True }