From 74c23c3e962bf12021f2ce465ef8ebab6c83ed89 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 8 Aug 2020 13:23:56 -0500 Subject: [PATCH] Add support for reload_on_update to _abort_if_unique_id_configured (#38638) * Add support for reload_on_update to _abort_if_unique_id_configured async_update_entry now avoids firing update listeners and writing the storage if there are no actual changes. * Actually add the tests * collapse branch * Update homeassistant/config_entries.py Co-authored-by: Franck Nijhof * handle entries that lack the ability to reload * reduce * adjust konnected tests * update axis tests * fix blocking * more mocking * config flow tests outside of test_config_flow * reduce * volumio * Update homeassistant/config_entries.py Co-authored-by: Paulus Schoutsen * set reload_on_update=False for integrations that implement self._abort_if_unique_id_configured(updates= and a reload listen * get rid of copy * revert test change Co-authored-by: Franck Nijhof Co-authored-by: Paulus Schoutsen --- homeassistant/components/hue/config_flow.py | 4 +- .../components/konnected/config_flow.py | 4 +- homeassistant/config_entries.py | 46 ++++-- tests/components/axis/test_config_flow.py | 35 +++-- tests/components/axis/test_device.py | 26 ++-- tests/components/deconz/test_config_flow.py | 63 +++++--- tests/components/deconz/test_gateway.py | 26 ++-- tests/components/volumio/test_config_flow.py | 15 +- tests/test_config_entries.py | 135 ++++++++++++++++-- 9 files changed, 272 insertions(+), 82 deletions(-) diff --git a/homeassistant/components/hue/config_flow.py b/homeassistant/components/hue/config_flow.py index 66c93ead846..f16d1f74cf6 100644 --- a/homeassistant/components/hue/config_flow.py +++ b/homeassistant/components/hue/config_flow.py @@ -198,7 +198,9 @@ class HueFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): bridge = self._async_get_bridge(host, discovery_info[ssdp.ATTR_UPNP_SERIAL]) await self.async_set_unique_id(bridge.id) - self._abort_if_unique_id_configured(updates={CONF_HOST: bridge.host}) + self._abort_if_unique_id_configured( + updates={CONF_HOST: bridge.host}, reload_on_update=False + ) self.bridge = bridge return await self.async_step_link() diff --git a/homeassistant/components/konnected/config_flow.py b/homeassistant/components/konnected/config_flow.py index f545c5f2f2a..c9cbfe03ae7 100644 --- a/homeassistant/components/konnected/config_flow.py +++ b/homeassistant/components/konnected/config_flow.py @@ -329,7 +329,9 @@ class KonnectedFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): if user_input is None: # abort and update an existing config entry if host info changes await self.async_set_unique_id(self.data[CONF_ID]) - self._abort_if_unique_id_configured(updates=self.data) + self._abort_if_unique_id_configured( + updates=self.data, reload_on_update=False + ) return self.async_show_form( step_id="confirm", description_placeholders={ diff --git a/homeassistant/config_entries.py b/homeassistant/config_entries.py index f36e2c6accb..90d9c623fac 100644 --- a/homeassistant/config_entries.py +++ b/homeassistant/config_entries.py @@ -750,23 +750,43 @@ class ConfigEntries: data: dict = _UNDEF, options: dict = _UNDEF, system_options: dict = _UNDEF, - ) -> None: - """Update a config entry.""" - if unique_id is not _UNDEF: + ) -> bool: + """Update a config entry. + + If the entry was changed, the update_listeners are + fired and this function returns True + + If the entry was not changed, the update_listeners are + not fired and this function returns False + """ + changed = False + + if unique_id is not _UNDEF and entry.unique_id != unique_id: + changed = True entry.unique_id = cast(Optional[str], unique_id) - if title is not _UNDEF: + if title is not _UNDEF and entry.title != title: + changed = True entry.title = cast(str, title) - if data is not _UNDEF: + if data is not _UNDEF and entry.data != data: # type: ignore + changed = True entry.data = MappingProxyType(data) - if options is not _UNDEF: + if options is not _UNDEF and entry.options != options: # type: ignore + changed = True entry.options = MappingProxyType(options) - if system_options is not _UNDEF: + if ( + system_options is not _UNDEF + and entry.system_options.as_dict() != system_options + ): + changed = True entry.system_options.update(**system_options) + if not changed: + return False + for listener_ref in entry.update_listeners: listener = listener_ref() if listener is not None: @@ -774,6 +794,8 @@ class ConfigEntries: self._async_schedule_save() + return True + async def async_forward_entry_setup(self, entry: ConfigEntry, domain: str) -> bool: """Forward the setup of an entry to a different component. @@ -850,7 +872,7 @@ class ConfigFlow(data_entry_flow.FlowHandler): @callback def _abort_if_unique_id_configured( - self, updates: Optional[Dict[Any, Any]] = None + self, updates: Optional[Dict[Any, Any]] = None, reload_on_update: bool = True, ) -> None: """Abort if the unique ID is already configured.""" assert self.hass @@ -859,10 +881,14 @@ class ConfigFlow(data_entry_flow.FlowHandler): for entry in self._async_current_entries(): if entry.unique_id == self.unique_id: - if updates is not None and not updates.items() <= entry.data.items(): - self.hass.config_entries.async_update_entry( + if updates is not None: + changed = self.hass.config_entries.async_update_entry( entry, data={**entry.data, **updates} ) + if changed and reload_on_update: + self.hass.async_create_task( + self.hass.config_entries.async_reload(entry.entry_id) + ) raise data_entry_flow.AbortFlow("already_configured") async def async_set_unique_id( diff --git a/tests/components/axis/test_config_flow.py b/tests/components/axis/test_config_flow.py index aa7d9db9027..df0ee32389a 100644 --- a/tests/components/axis/test_config_flow.py +++ b/tests/components/axis/test_config_flow.py @@ -67,7 +67,11 @@ async def test_manual_configuration_update_configuration(hass): assert result["type"] == "form" assert result["step_id"] == "user" - with patch("axis.vapix.session_request", new=vapix_session_request): + with patch( + "homeassistant.components.axis.async_setup_entry", return_value=True, + ) as mock_setup_entry, patch( + "axis.vapix.session_request", new=vapix_session_request + ): result = await hass.config_entries.flow.async_configure( result["flow_id"], user_input={ @@ -77,10 +81,12 @@ async def test_manual_configuration_update_configuration(hass): CONF_PORT: 80, }, ) + await hass.async_block_till_done() assert result["type"] == "abort" assert result["reason"] == "already_configured" assert device.host == "2.3.4.5" + assert len(mock_setup_entry.mock_calls) == 1 async def test_flow_fails_already_configured(hass): @@ -282,16 +288,22 @@ async def test_zeroconf_flow_updated_configuration(hass): CONF_NAME: NAME, } - result = await hass.config_entries.flow.async_init( - AXIS_DOMAIN, - data={ - CONF_HOST: "2.3.4.5", - CONF_PORT: 8080, - "hostname": "name", - "properties": {"macaddress": MAC}, - }, - context={"source": "zeroconf"}, - ) + with patch( + "homeassistant.components.axis.async_setup_entry", return_value=True, + ) as mock_setup_entry, patch( + "axis.vapix.session_request", new=vapix_session_request + ): + result = await hass.config_entries.flow.async_init( + AXIS_DOMAIN, + data={ + CONF_HOST: "2.3.4.5", + CONF_PORT: 8080, + "hostname": "name", + "properties": {"macaddress": MAC}, + }, + context={"source": "zeroconf"}, + ) + await hass.async_block_till_done() assert result["type"] == "abort" assert result["reason"] == "already_configured" @@ -304,6 +316,7 @@ async def test_zeroconf_flow_updated_configuration(hass): CONF_MODEL: MODEL, CONF_NAME: NAME, } + assert len(mock_setup_entry.mock_calls) == 1 async def test_zeroconf_flow_ignore_non_axis_device(hass): diff --git a/tests/components/axis/test_device.py b/tests/components/axis/test_device.py index 4350764c486..c8583a8ce03 100644 --- a/tests/components/axis/test_device.py +++ b/tests/components/axis/test_device.py @@ -290,19 +290,23 @@ async def test_update_address(hass): device = await setup_axis_integration(hass) assert device.api.config.host == "1.2.3.4" - await hass.config_entries.flow.async_init( - AXIS_DOMAIN, - data={ - "host": "2.3.4.5", - "port": 80, - "hostname": "name", - "properties": {"macaddress": MAC}, - }, - context={"source": "zeroconf"}, - ) - await hass.async_block_till_done() + with patch("axis.vapix.session_request", new=vapix_session_request), patch( + "homeassistant.components.axis.async_setup_entry", return_value=True, + ) as mock_setup_entry: + await hass.config_entries.flow.async_init( + AXIS_DOMAIN, + data={ + "host": "2.3.4.5", + "port": 80, + "hostname": "name", + "properties": {"macaddress": MAC}, + }, + context={"source": "zeroconf"}, + ) + await hass.async_block_till_done() assert device.api.config.host == "2.3.4.5" + assert len(mock_setup_entry.mock_calls) == 1 async def test_device_unavailable(hass): diff --git a/tests/components/deconz/test_config_flow.py b/tests/components/deconz/test_config_flow.py index b29e4303f7b..0a536bcda5b 100644 --- a/tests/components/deconz/test_config_flow.py +++ b/tests/components/deconz/test_config_flow.py @@ -1,6 +1,7 @@ """Tests for deCONZ config flow.""" import asyncio +from asynctest.mock import patch import pydeconz from homeassistant import data_entry_flow @@ -399,19 +400,24 @@ async def test_ssdp_discovery_update_configuration(hass): """Test if a discovered bridge is configured but updates with new attributes.""" gateway = await setup_deconz_integration(hass) - result = await hass.config_entries.flow.async_init( - DOMAIN, - data={ - ssdp.ATTR_SSDP_LOCATION: "http://2.3.4.5:80/", - ssdp.ATTR_UPNP_MANUFACTURER_URL: DECONZ_MANUFACTURERURL, - ssdp.ATTR_UPNP_SERIAL: BRIDGEID, - }, - context={"source": "ssdp"}, - ) + with patch( + "homeassistant.components.deconz.async_setup_entry", return_value=True, + ) as mock_setup_entry: + result = await hass.config_entries.flow.async_init( + DOMAIN, + data={ + ssdp.ATTR_SSDP_LOCATION: "http://2.3.4.5:80/", + ssdp.ATTR_UPNP_MANUFACTURER_URL: DECONZ_MANUFACTURERURL, + ssdp.ATTR_UPNP_SERIAL: BRIDGEID, + }, + context={"source": "ssdp"}, + ) + await hass.async_block_till_done() assert result["type"] == data_entry_flow.RESULT_TYPE_ABORT assert result["reason"] == "already_configured" assert gateway.config_entry.data[CONF_HOST] == "2.3.4.5" + assert len(mock_setup_entry.mock_calls) == 1 async def test_ssdp_discovery_dont_update_configuration(hass): @@ -469,9 +475,15 @@ async def test_flow_hassio_discovery(hass): assert result["step_id"] == "hassio_confirm" assert result["description_placeholders"] == {"addon": "Mock Addon"} - result = await hass.config_entries.flow.async_configure( - result["flow_id"], user_input={} - ) + with patch( + "homeassistant.components.deconz.async_setup", return_value=True + ) as mock_setup, patch( + "homeassistant.components.deconz.async_setup_entry", return_value=True, + ) as mock_setup_entry: + result = await hass.config_entries.flow.async_configure( + result["flow_id"], user_input={} + ) + await hass.async_block_till_done() assert result["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY assert result["result"].data == { @@ -479,28 +491,35 @@ async def test_flow_hassio_discovery(hass): CONF_PORT: 80, CONF_API_KEY: API_KEY, } + assert len(mock_setup.mock_calls) == 1 + assert len(mock_setup_entry.mock_calls) == 1 async def test_hassio_discovery_update_configuration(hass): """Test we can update an existing config entry.""" gateway = await setup_deconz_integration(hass) - result = await hass.config_entries.flow.async_init( - DOMAIN, - data={ - CONF_HOST: "2.3.4.5", - CONF_PORT: 8080, - CONF_API_KEY: "updated", - CONF_SERIAL: BRIDGEID, - }, - context={"source": "hassio"}, - ) + with patch( + "homeassistant.components.deconz.async_setup_entry", return_value=True, + ) as mock_setup_entry: + result = await hass.config_entries.flow.async_init( + DOMAIN, + data={ + CONF_HOST: "2.3.4.5", + CONF_PORT: 8080, + CONF_API_KEY: "updated", + CONF_SERIAL: BRIDGEID, + }, + context={"source": "hassio"}, + ) + await hass.async_block_till_done() assert result["type"] == data_entry_flow.RESULT_TYPE_ABORT assert result["reason"] == "already_configured" assert gateway.config_entry.data[CONF_HOST] == "2.3.4.5" assert gateway.config_entry.data[CONF_PORT] == 8080 assert gateway.config_entry.data[CONF_API_KEY] == "updated" + assert len(mock_setup_entry.mock_calls) == 1 async def test_hassio_discovery_dont_update_configuration(hass): diff --git a/tests/components/deconz/test_gateway.py b/tests/components/deconz/test_gateway.py index 9af1a3151e8..4236888a5a6 100644 --- a/tests/components/deconz/test_gateway.py +++ b/tests/components/deconz/test_gateway.py @@ -135,19 +135,23 @@ async def test_update_address(hass): gateway = await setup_deconz_integration(hass) assert gateway.api.host == "1.2.3.4" - await hass.config_entries.flow.async_init( - deconz.config_flow.DOMAIN, - data={ - ssdp.ATTR_SSDP_LOCATION: "http://2.3.4.5:80/", - ssdp.ATTR_UPNP_MANUFACTURER_URL: deconz.config_flow.DECONZ_MANUFACTURERURL, - ssdp.ATTR_UPNP_SERIAL: BRIDGEID, - ssdp.ATTR_UPNP_UDN: "uuid:456DEF", - }, - context={"source": "ssdp"}, - ) - await hass.async_block_till_done() + with patch( + "homeassistant.components.deconz.async_setup_entry", return_value=True, + ) as mock_setup_entry: + await hass.config_entries.flow.async_init( + deconz.config_flow.DOMAIN, + data={ + ssdp.ATTR_SSDP_LOCATION: "http://2.3.4.5:80/", + ssdp.ATTR_UPNP_MANUFACTURER_URL: deconz.config_flow.DECONZ_MANUFACTURERURL, + ssdp.ATTR_UPNP_SERIAL: BRIDGEID, + ssdp.ATTR_UPNP_UDN: "uuid:456DEF", + }, + context={"source": "ssdp"}, + ) + await hass.async_block_till_done() assert gateway.api.host == "2.3.4.5" + assert len(mock_setup_entry.mock_calls) == 1 async def test_reset_after_successful_setup(hass): diff --git a/tests/components/volumio/test_config_flow.py b/tests/components/volumio/test_config_flow.py index a7ed4773142..6fc7390da79 100644 --- a/tests/components/volumio/test_config_flow.py +++ b/tests/components/volumio/test_config_flow.py @@ -85,6 +85,7 @@ async def test_form_updates_unique_id(hass): result2 = await hass.config_entries.flow.async_configure( result["flow_id"], TEST_CONNECTION, ) + await hass.async_block_till_done() assert result2["type"] == "abort" assert result2["reason"] == "already_configured" @@ -242,11 +243,19 @@ async def test_discovery_updates_unique_id(hass): entry.add_to_hass(hass) - result = await hass.config_entries.flow.async_init( - DOMAIN, context={"source": "zeroconf"}, data=TEST_DISCOVERY - ) + with patch( + "homeassistant.components.volumio.async_setup", return_value=True + ) as mock_setup, patch( + "homeassistant.components.volumio.async_setup_entry", return_value=True, + ) as mock_setup_entry: + result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": "zeroconf"}, data=TEST_DISCOVERY + ) + await hass.async_block_till_done() assert result["type"] == "abort" assert result["reason"] == "already_configured" assert entry.data == TEST_DISCOVERY_RESULT + assert len(mock_setup.mock_calls) == 1 + assert len(mock_setup_entry.mock_calls) == 1 diff --git a/tests/test_config_entries.py b/tests/test_config_entries.py index 6d513697daf..ab28ecc7af3 100644 --- a/tests/test_config_entries.py +++ b/tests/test_config_entries.py @@ -624,10 +624,10 @@ async def test_updating_entry_data(manager): ) entry.add_to_manager(manager) - manager.async_update_entry(entry) + assert manager.async_update_entry(entry) is False assert entry.data == {"first": True} - manager.async_update_entry(entry, data={"second": True}) + assert manager.async_update_entry(entry, data={"second": True}) is True assert entry.data == {"second": True} @@ -658,7 +658,7 @@ async def test_update_entry_options_and_trigger_listener(hass, manager): entry.add_update_listener(update_listener) - manager.async_update_entry(entry, options={"second": True}) + assert manager.async_update_entry(entry, options={"second": True}) is True assert entry.options == {"second": True} @@ -1120,7 +1120,7 @@ async def test_unique_id_existing_entry(hass, manager): assert len(async_remove_entry.mock_calls) == 1 -async def test_unique_id_update_existing_entry(hass, manager): +async def test_unique_id_update_existing_entry_without_reload(hass, manager): """Test that we update an entry if there already is an entry with unique ID.""" hass.config.components.add("comp") entry = MockConfigEntry( @@ -1143,17 +1143,65 @@ async def test_unique_id_update_existing_entry(hass, manager): async def async_step_user(self, user_input=None): """Test user step.""" await self.async_set_unique_id("mock-unique-id") - await self._abort_if_unique_id_configured(updates={"host": "1.1.1.1"}) + await self._abort_if_unique_id_configured( + updates={"host": "1.1.1.1"}, reload_on_update=False + ) - with patch.dict(config_entries.HANDLERS, {"comp": TestFlow}): + with patch.dict(config_entries.HANDLERS, {"comp": TestFlow}), patch( + "homeassistant.config_entries.ConfigEntries.async_reload" + ) as async_reload: result = await manager.flow.async_init( "comp", context={"source": config_entries.SOURCE_USER} ) + await hass.async_block_till_done() assert result["type"] == "abort" assert result["reason"] == "already_configured" assert entry.data["host"] == "1.1.1.1" assert entry.data["additional"] == "data" + assert len(async_reload.mock_calls) == 0 + + +async def test_unique_id_update_existing_entry_with_reload(hass, manager): + """Test that we update an entry if there already is an entry with unique ID and we reload on changes.""" + hass.config.components.add("comp") + entry = MockConfigEntry( + domain="comp", + data={"additional": "data", "host": "0.0.0.0"}, + unique_id="mock-unique-id", + ) + entry.add_to_hass(hass) + + mock_integration( + hass, MockModule("comp"), + ) + mock_entity_platform(hass, "config_flow.comp", None) + + class TestFlow(config_entries.ConfigFlow): + """Test flow.""" + + VERSION = 1 + + async def async_step_user(self, user_input=None): + """Test user step.""" + await self.async_set_unique_id("mock-unique-id") + await self._abort_if_unique_id_configured( + updates={"host": "1.1.1.1"}, reload_on_update=True + ) + + with patch.dict(config_entries.HANDLERS, {"comp": TestFlow}), patch( + "homeassistant.config_entries.ConfigEntries.async_reload" + ) as async_reload: + result = await manager.flow.async_init( + "comp", context={"source": config_entries.SOURCE_USER} + ) + await hass.async_block_till_done() + + assert result["type"] == "abort" + assert result["reason"] == "already_configured" + assert entry.data["host"] == "1.1.1.1" + assert entry.data["additional"] == "data" + assert len(async_reload.mock_calls) == 1 async def test_unique_id_not_update_existing_entry(hass, manager): @@ -1179,20 +1227,23 @@ async def test_unique_id_not_update_existing_entry(hass, manager): async def async_step_user(self, user_input=None): """Test user step.""" await self.async_set_unique_id("mock-unique-id") - await self._abort_if_unique_id_configured(updates={"host": "0.0.0.0"}) + await self._abort_if_unique_id_configured( + updates={"host": "0.0.0.0"}, reload_on_update=True + ) with patch.dict(config_entries.HANDLERS, {"comp": TestFlow}), patch( - "homeassistant.config_entries.ConfigEntries.async_update_entry" - ) as async_update_entry: + "homeassistant.config_entries.ConfigEntries.async_reload" + ) as async_reload: result = await manager.flow.async_init( "comp", context={"source": config_entries.SOURCE_USER} ) + await hass.async_block_till_done() assert result["type"] == "abort" assert result["reason"] == "already_configured" assert entry.data["host"] == "0.0.0.0" assert entry.data["additional"] == "data" - assert len(async_update_entry.mock_calls) == 0 + assert len(async_reload.mock_calls) == 0 async def test_unique_id_in_progress(hass, manager): @@ -1567,8 +1618,11 @@ async def test_async_setup_update_entry(hass): async def async_step_import(self, user_input): """Test import step updating existing entry.""" - self.hass.config_entries.async_update_entry( - entry, data={"value": "updated"} + assert ( + self.hass.config_entries.async_update_entry( + entry, data={"value": "updated"} + ) + is True ) return self.async_abort(reason="yo") @@ -1758,3 +1812,60 @@ async def test_default_discovery_abort_on_new_unique_flow(hass, manager): flows = hass.config_entries.flow.async_progress() assert len(flows) == 1 assert flows[0]["context"]["unique_id"] == "mock-unique-id" + + +async def test_updating_entry_with_and_without_changes(manager): + """Test that we can update an entry data.""" + entry = MockConfigEntry( + domain="test", + data={"first": True}, + title="thetitle", + options={"option": True}, + unique_id="abc123", + state=config_entries.ENTRY_STATE_SETUP_ERROR, + ) + entry.add_to_manager(manager) + + assert manager.async_update_entry(entry) is False + assert manager.async_update_entry(entry, data={"second": True}) is True + assert manager.async_update_entry(entry, data={"second": True}) is False + assert ( + manager.async_update_entry(entry, data={"second": True, "third": 456}) is True + ) + assert ( + manager.async_update_entry(entry, data={"second": True, "third": 456}) is False + ) + assert manager.async_update_entry(entry, options={"second": True}) is True + assert manager.async_update_entry(entry, options={"second": True}) is False + assert ( + manager.async_update_entry(entry, options={"second": True, "third": "123"}) + is True + ) + assert ( + manager.async_update_entry(entry, options={"second": True, "third": "123"}) + is False + ) + assert ( + manager.async_update_entry(entry, system_options={"disable_new_entities": True}) + is True + ) + assert ( + manager.async_update_entry(entry, system_options={"disable_new_entities": True}) + is False + ) + assert ( + manager.async_update_entry( + entry, system_options={"disable_new_entities": False} + ) + is True + ) + assert ( + manager.async_update_entry( + entry, system_options={"disable_new_entities": False} + ) + is False + ) + assert manager.async_update_entry(entry, title="thetitle") is False + assert manager.async_update_entry(entry, title="newtitle") is True + assert manager.async_update_entry(entry, unique_id="abc123") is False + assert manager.async_update_entry(entry, unique_id="abc1234") is True