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 <git@frenck.dev>

* 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 <paulus@home-assistant.io>

* 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 <git@frenck.dev>
Co-authored-by: Paulus Schoutsen <paulus@home-assistant.io>
This commit is contained in:
J. Nick Koston 2020-08-08 13:23:56 -05:00 committed by GitHub
parent 1cb161017e
commit 74c23c3e96
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 272 additions and 82 deletions

View file

@ -198,7 +198,9 @@ class HueFlowHandler(config_entries.ConfigFlow, domain=DOMAIN):
bridge = self._async_get_bridge(host, discovery_info[ssdp.ATTR_UPNP_SERIAL]) bridge = self._async_get_bridge(host, discovery_info[ssdp.ATTR_UPNP_SERIAL])
await self.async_set_unique_id(bridge.id) 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 self.bridge = bridge
return await self.async_step_link() return await self.async_step_link()

View file

@ -329,7 +329,9 @@ class KonnectedFlowHandler(config_entries.ConfigFlow, domain=DOMAIN):
if user_input is None: if user_input is None:
# abort and update an existing config entry if host info changes # abort and update an existing config entry if host info changes
await self.async_set_unique_id(self.data[CONF_ID]) 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( return self.async_show_form(
step_id="confirm", step_id="confirm",
description_placeholders={ description_placeholders={

View file

@ -750,23 +750,43 @@ class ConfigEntries:
data: dict = _UNDEF, data: dict = _UNDEF,
options: dict = _UNDEF, options: dict = _UNDEF,
system_options: dict = _UNDEF, system_options: dict = _UNDEF,
) -> None: ) -> bool:
"""Update a config entry.""" """Update a config entry.
if unique_id is not _UNDEF:
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) 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) 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) 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) 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) entry.system_options.update(**system_options)
if not changed:
return False
for listener_ref in entry.update_listeners: for listener_ref in entry.update_listeners:
listener = listener_ref() listener = listener_ref()
if listener is not None: if listener is not None:
@ -774,6 +794,8 @@ class ConfigEntries:
self._async_schedule_save() self._async_schedule_save()
return True
async def async_forward_entry_setup(self, entry: ConfigEntry, domain: str) -> bool: async def async_forward_entry_setup(self, entry: ConfigEntry, domain: str) -> bool:
"""Forward the setup of an entry to a different component. """Forward the setup of an entry to a different component.
@ -850,7 +872,7 @@ class ConfigFlow(data_entry_flow.FlowHandler):
@callback @callback
def _abort_if_unique_id_configured( 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: ) -> None:
"""Abort if the unique ID is already configured.""" """Abort if the unique ID is already configured."""
assert self.hass assert self.hass
@ -859,10 +881,14 @@ class ConfigFlow(data_entry_flow.FlowHandler):
for entry in self._async_current_entries(): for entry in self._async_current_entries():
if entry.unique_id == self.unique_id: if entry.unique_id == self.unique_id:
if updates is not None and not updates.items() <= entry.data.items(): if updates is not None:
self.hass.config_entries.async_update_entry( changed = self.hass.config_entries.async_update_entry(
entry, data={**entry.data, **updates} 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") raise data_entry_flow.AbortFlow("already_configured")
async def async_set_unique_id( async def async_set_unique_id(

View file

@ -67,7 +67,11 @@ async def test_manual_configuration_update_configuration(hass):
assert result["type"] == "form" assert result["type"] == "form"
assert result["step_id"] == "user" 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 = await hass.config_entries.flow.async_configure(
result["flow_id"], result["flow_id"],
user_input={ user_input={
@ -77,10 +81,12 @@ async def test_manual_configuration_update_configuration(hass):
CONF_PORT: 80, CONF_PORT: 80,
}, },
) )
await hass.async_block_till_done()
assert result["type"] == "abort" assert result["type"] == "abort"
assert result["reason"] == "already_configured" assert result["reason"] == "already_configured"
assert device.host == "2.3.4.5" assert device.host == "2.3.4.5"
assert len(mock_setup_entry.mock_calls) == 1
async def test_flow_fails_already_configured(hass): async def test_flow_fails_already_configured(hass):
@ -282,6 +288,11 @@ async def test_zeroconf_flow_updated_configuration(hass):
CONF_NAME: NAME, CONF_NAME: NAME,
} }
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( result = await hass.config_entries.flow.async_init(
AXIS_DOMAIN, AXIS_DOMAIN,
data={ data={
@ -292,6 +303,7 @@ async def test_zeroconf_flow_updated_configuration(hass):
}, },
context={"source": "zeroconf"}, context={"source": "zeroconf"},
) )
await hass.async_block_till_done()
assert result["type"] == "abort" assert result["type"] == "abort"
assert result["reason"] == "already_configured" assert result["reason"] == "already_configured"
@ -304,6 +316,7 @@ async def test_zeroconf_flow_updated_configuration(hass):
CONF_MODEL: MODEL, CONF_MODEL: MODEL,
CONF_NAME: NAME, CONF_NAME: NAME,
} }
assert len(mock_setup_entry.mock_calls) == 1
async def test_zeroconf_flow_ignore_non_axis_device(hass): async def test_zeroconf_flow_ignore_non_axis_device(hass):

View file

@ -290,6 +290,9 @@ async def test_update_address(hass):
device = await setup_axis_integration(hass) device = await setup_axis_integration(hass)
assert device.api.config.host == "1.2.3.4" assert device.api.config.host == "1.2.3.4"
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( await hass.config_entries.flow.async_init(
AXIS_DOMAIN, AXIS_DOMAIN,
data={ data={
@ -303,6 +306,7 @@ async def test_update_address(hass):
await hass.async_block_till_done() await hass.async_block_till_done()
assert device.api.config.host == "2.3.4.5" assert device.api.config.host == "2.3.4.5"
assert len(mock_setup_entry.mock_calls) == 1
async def test_device_unavailable(hass): async def test_device_unavailable(hass):

View file

@ -1,6 +1,7 @@
"""Tests for deCONZ config flow.""" """Tests for deCONZ config flow."""
import asyncio import asyncio
from asynctest.mock import patch
import pydeconz import pydeconz
from homeassistant import data_entry_flow from homeassistant import data_entry_flow
@ -399,6 +400,9 @@ async def test_ssdp_discovery_update_configuration(hass):
"""Test if a discovered bridge is configured but updates with new attributes.""" """Test if a discovered bridge is configured but updates with new attributes."""
gateway = await setup_deconz_integration(hass) gateway = await setup_deconz_integration(hass)
with patch(
"homeassistant.components.deconz.async_setup_entry", return_value=True,
) as mock_setup_entry:
result = await hass.config_entries.flow.async_init( result = await hass.config_entries.flow.async_init(
DOMAIN, DOMAIN,
data={ data={
@ -408,10 +412,12 @@ async def test_ssdp_discovery_update_configuration(hass):
}, },
context={"source": "ssdp"}, context={"source": "ssdp"},
) )
await hass.async_block_till_done()
assert result["type"] == data_entry_flow.RESULT_TYPE_ABORT assert result["type"] == data_entry_flow.RESULT_TYPE_ABORT
assert result["reason"] == "already_configured" assert result["reason"] == "already_configured"
assert gateway.config_entry.data[CONF_HOST] == "2.3.4.5" 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): 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["step_id"] == "hassio_confirm"
assert result["description_placeholders"] == {"addon": "Mock Addon"} assert result["description_placeholders"] == {"addon": "Mock Addon"}
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 = await hass.config_entries.flow.async_configure(
result["flow_id"], user_input={} result["flow_id"], user_input={}
) )
await hass.async_block_till_done()
assert result["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY assert result["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY
assert result["result"].data == { assert result["result"].data == {
@ -479,12 +491,17 @@ async def test_flow_hassio_discovery(hass):
CONF_PORT: 80, CONF_PORT: 80,
CONF_API_KEY: API_KEY, 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): async def test_hassio_discovery_update_configuration(hass):
"""Test we can update an existing config entry.""" """Test we can update an existing config entry."""
gateway = await setup_deconz_integration(hass) gateway = await setup_deconz_integration(hass)
with patch(
"homeassistant.components.deconz.async_setup_entry", return_value=True,
) as mock_setup_entry:
result = await hass.config_entries.flow.async_init( result = await hass.config_entries.flow.async_init(
DOMAIN, DOMAIN,
data={ data={
@ -495,12 +512,14 @@ async def test_hassio_discovery_update_configuration(hass):
}, },
context={"source": "hassio"}, context={"source": "hassio"},
) )
await hass.async_block_till_done()
assert result["type"] == data_entry_flow.RESULT_TYPE_ABORT assert result["type"] == data_entry_flow.RESULT_TYPE_ABORT
assert result["reason"] == "already_configured" assert result["reason"] == "already_configured"
assert gateway.config_entry.data[CONF_HOST] == "2.3.4.5" 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_PORT] == 8080
assert gateway.config_entry.data[CONF_API_KEY] == "updated" 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): async def test_hassio_discovery_dont_update_configuration(hass):

View file

@ -135,6 +135,9 @@ async def test_update_address(hass):
gateway = await setup_deconz_integration(hass) gateway = await setup_deconz_integration(hass)
assert gateway.api.host == "1.2.3.4" assert gateway.api.host == "1.2.3.4"
with patch(
"homeassistant.components.deconz.async_setup_entry", return_value=True,
) as mock_setup_entry:
await hass.config_entries.flow.async_init( await hass.config_entries.flow.async_init(
deconz.config_flow.DOMAIN, deconz.config_flow.DOMAIN,
data={ data={
@ -148,6 +151,7 @@ async def test_update_address(hass):
await hass.async_block_till_done() await hass.async_block_till_done()
assert gateway.api.host == "2.3.4.5" assert gateway.api.host == "2.3.4.5"
assert len(mock_setup_entry.mock_calls) == 1
async def test_reset_after_successful_setup(hass): async def test_reset_after_successful_setup(hass):

View file

@ -85,6 +85,7 @@ async def test_form_updates_unique_id(hass):
result2 = await hass.config_entries.flow.async_configure( result2 = await hass.config_entries.flow.async_configure(
result["flow_id"], TEST_CONNECTION, result["flow_id"], TEST_CONNECTION,
) )
await hass.async_block_till_done()
assert result2["type"] == "abort" assert result2["type"] == "abort"
assert result2["reason"] == "already_configured" assert result2["reason"] == "already_configured"
@ -242,11 +243,19 @@ async def test_discovery_updates_unique_id(hass):
entry.add_to_hass(hass) entry.add_to_hass(hass)
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( result = await hass.config_entries.flow.async_init(
DOMAIN, context={"source": "zeroconf"}, data=TEST_DISCOVERY DOMAIN, context={"source": "zeroconf"}, data=TEST_DISCOVERY
) )
await hass.async_block_till_done()
assert result["type"] == "abort" assert result["type"] == "abort"
assert result["reason"] == "already_configured" assert result["reason"] == "already_configured"
assert entry.data == TEST_DISCOVERY_RESULT assert entry.data == TEST_DISCOVERY_RESULT
assert len(mock_setup.mock_calls) == 1
assert len(mock_setup_entry.mock_calls) == 1

View file

@ -624,10 +624,10 @@ async def test_updating_entry_data(manager):
) )
entry.add_to_manager(manager) entry.add_to_manager(manager)
manager.async_update_entry(entry) assert manager.async_update_entry(entry) is False
assert entry.data == {"first": True} 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} 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) 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} 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 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.""" """Test that we update an entry if there already is an entry with unique ID."""
hass.config.components.add("comp") hass.config.components.add("comp")
entry = MockConfigEntry( 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): async def async_step_user(self, user_input=None):
"""Test user step.""" """Test user step."""
await self.async_set_unique_id("mock-unique-id") 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( result = await manager.flow.async_init(
"comp", context={"source": config_entries.SOURCE_USER} "comp", context={"source": config_entries.SOURCE_USER}
) )
await hass.async_block_till_done()
assert result["type"] == "abort" assert result["type"] == "abort"
assert result["reason"] == "already_configured" assert result["reason"] == "already_configured"
assert entry.data["host"] == "1.1.1.1" assert entry.data["host"] == "1.1.1.1"
assert entry.data["additional"] == "data" 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): 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): async def async_step_user(self, user_input=None):
"""Test user step.""" """Test user step."""
await self.async_set_unique_id("mock-unique-id") 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( with patch.dict(config_entries.HANDLERS, {"comp": TestFlow}), patch(
"homeassistant.config_entries.ConfigEntries.async_update_entry" "homeassistant.config_entries.ConfigEntries.async_reload"
) as async_update_entry: ) as async_reload:
result = await manager.flow.async_init( result = await manager.flow.async_init(
"comp", context={"source": config_entries.SOURCE_USER} "comp", context={"source": config_entries.SOURCE_USER}
) )
await hass.async_block_till_done()
assert result["type"] == "abort" assert result["type"] == "abort"
assert result["reason"] == "already_configured" assert result["reason"] == "already_configured"
assert entry.data["host"] == "0.0.0.0" assert entry.data["host"] == "0.0.0.0"
assert entry.data["additional"] == "data" 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): async def test_unique_id_in_progress(hass, manager):
@ -1567,9 +1618,12 @@ async def test_async_setup_update_entry(hass):
async def async_step_import(self, user_input): async def async_step_import(self, user_input):
"""Test import step updating existing entry.""" """Test import step updating existing entry."""
assert (
self.hass.config_entries.async_update_entry( self.hass.config_entries.async_update_entry(
entry, data={"value": "updated"} entry, data={"value": "updated"}
) )
is True
)
return self.async_abort(reason="yo") return self.async_abort(reason="yo")
with patch.dict(config_entries.HANDLERS, {"comp": TestFlow}): with patch.dict(config_entries.HANDLERS, {"comp": TestFlow}):
@ -1758,3 +1812,60 @@ async def test_default_discovery_abort_on_new_unique_flow(hass, manager):
flows = hass.config_entries.flow.async_progress() flows = hass.config_entries.flow.async_progress()
assert len(flows) == 1 assert len(flows) == 1
assert flows[0]["context"]["unique_id"] == "mock-unique-id" 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