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

@@ -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:
+        if not self.api:
-            retry_delay = 2 ** (tries + 1)
+            raise ConfigEntryNotReady
-            _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
 
         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:

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)
+            raise ConfigEntryNotReady
-            _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
 
         _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()

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)
+            raise ConfigEntryNotReady
-            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
 
         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:

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)
+            raise ConfigEntryNotReady
-            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
 
         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

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):

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)):
+    with patch.object(
-        assert await deconz_gateway.async_setup() is False
+            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()

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',
+    with patch(
-               return_value=mock_coro(False)):
+        'pydeconz.DeconzSession.async_load_parameters',
-        assert await deconz.async_setup_entry(hass, entry) is False
+        return_value=mock_coro(False)
+    ), pytest.raises(ConfigEntryNotReady):
+        await deconz.async_setup_entry(hass, entry)
 
 
 async def test_setup_entry_successful(hass):

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',
+    with patch.object(
-                      side_effect=errors.HmipcConnectionError):
+            hap, 'get_hap', side_effect=errors.HmipcConnectionError
-        assert await hap.async_setup() is False
+    ), 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

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):
+    with patch.object(
-        assert await hue_bridge.async_setup() is False
+            bridge, 'get_bridge', side_effect=errors.CannotConnect
-
+    ), pytest.raises(ConfigEntryNotReady):
-    assert len(hass.helpers.event.async_call_later.mock_calls) == 1
+        await hue_bridge.async_setup()
-    # 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
 
 
 async def test_reset_if_entry_had_wrong_auth():

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',
+    with patch.object(
-                      side_effect=errors.CannotConnect):
+            controller, 'get_controller', side_effect=errors.CannotConnect
-        assert await unifi_controller.async_setup() is False
+    ), pytest.raises(ConfigEntryNotReady):
-
+        await unifi_controller.async_setup()
-    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_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()

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
     }