diff --git a/homeassistant/components/hue/__init__.py b/homeassistant/components/hue/__init__.py index 57057004479..7239efafd10 100644 --- a/homeassistant/components/hue/__init__.py +++ b/homeassistant/components/hue/__init__.py @@ -2,16 +2,14 @@ import ipaddress import logging +from aiohue.util import normalize_bridge_id import voluptuous as vol from homeassistant import config_entries, core -from homeassistant.const import CONF_FILENAME, CONF_HOST +from homeassistant.const import CONF_HOST from homeassistant.helpers import config_validation as cv, device_registry as dr -from .bridge import HueBridge, normalize_bridge_id -from .config_flow import ( # Loading the config flow file will register the flow - configured_hosts, -) +from .bridge import HueBridge from .const import DOMAIN _LOGGER = logging.getLogger(__name__) @@ -32,8 +30,6 @@ BRIDGE_CONFIG_SCHEMA = vol.Schema( { # Validate as IP address and then convert back to a string. vol.Required(CONF_HOST): vol.All(ipaddress.ip_address, cv.string), - # This is for legacy reasons and is only used for importing auth. - vol.Optional(CONF_FILENAME, default=PHUE_CONFIG_FILE): cv.string, vol.Optional( CONF_ALLOW_UNREACHABLE, default=DEFAULT_ALLOW_UNREACHABLE ): cv.boolean, @@ -65,7 +61,6 @@ async def async_setup(hass, config): hass.data[DOMAIN] = {} hass.data[DATA_CONFIGS] = {} - configured = configured_hosts(hass) # User has configured bridges if CONF_BRIDGES not in conf: @@ -73,29 +68,28 @@ async def async_setup(hass, config): bridges = conf[CONF_BRIDGES] + configured_hosts = set( + entry.data["host"] for entry in hass.config_entries.async_entries(DOMAIN) + ) + for bridge_conf in bridges: host = bridge_conf[CONF_HOST] # Store config in hass.data so the config entry can find it hass.data[DATA_CONFIGS][host] = bridge_conf - # If configured, the bridge will be set up during config entry phase - if host in configured: + if host in configured_hosts: continue - # No existing config entry found, try importing it or trigger link - # config flow if no existing auth. Because we're inside the setup of - # this component we'll have to use hass.async_add_job to avoid a - # deadlock: creating a config entry will set up the component but the - # setup would block till the entry is created! + # No existing config entry found, trigger link config flow. Because we're + # inside the setup of this component we'll have to use hass.async_add_job + # to avoid a deadlock: creating a config entry will set up the component + # but the setup would block till the entry is created! hass.async_create_task( hass.config_entries.flow.async_init( DOMAIN, context={"source": config_entries.SOURCE_IMPORT}, - data={ - "host": bridge_conf[CONF_HOST], - "path": bridge_conf[CONF_FILENAME], - }, + data={"host": bridge_conf[CONF_HOST]}, ) ) diff --git a/homeassistant/components/hue/bridge.py b/homeassistant/components/hue/bridge.py index 0ed6e3a9911..58a744dd5b0 100644 --- a/homeassistant/components/hue/bridge.py +++ b/homeassistant/components/hue/bridge.py @@ -6,6 +6,7 @@ import async_timeout import slugify as unicode_slug import voluptuous as vol +from homeassistant import core from homeassistant.exceptions import ConfigEntryNotReady from homeassistant.helpers import aiohttp_client, config_validation as cv @@ -45,8 +46,15 @@ class HueBridge: host = self.host hass = self.hass + bridge = aiohue.Bridge( + host, + username=self.config_entry.data["username"], + websession=aiohttp_client.async_get_clientsession(hass), + ) + try: - self.api = await get_bridge(hass, host, self.config_entry.data["username"]) + await authenticate_bridge(hass, bridge) + except AuthenticationRequired: # Usernames can become invalid if hub is reset or user removed. # We are going to fail the config entry setup and initiate a new @@ -63,6 +71,8 @@ class HueBridge: LOGGER.exception("Unknown error connecting with Hue bridge at %s", host) return False + self.api = bridge + hass.async_create_task( hass.config_entries.async_forward_entry_setup(self.config_entry, "light") ) @@ -175,16 +185,12 @@ class HueBridge: create_config_flow(self.hass, self.host) -async def get_bridge(hass, host, username=None): +async def authenticate_bridge(hass: core.HomeAssistant, bridge: aiohue.Bridge): """Create a bridge object and verify authentication.""" - bridge = aiohue.Bridge( - host, username=username, websession=aiohttp_client.async_get_clientsession(hass) - ) - try: with async_timeout.timeout(10): # Create username if we don't have one - if not username: + if not bridge.username: device_name = unicode_slug.slugify( hass.config.location_name, max_length=19 ) @@ -193,7 +199,6 @@ async def get_bridge(hass, host, username=None): # Initialize bridge (and validate our username) await bridge.initialize() - return bridge except (aiohue.LinkButtonNotPressed, aiohue.Unauthorized): raise AuthenticationRequired except (asyncio.TimeoutError, aiohue.RequestError): @@ -201,25 +206,3 @@ async def get_bridge(hass, host, username=None): except aiohue.AiohueException: LOGGER.exception("Unknown Hue linking error occurred") raise AuthenticationRequired - - -def normalize_bridge_id(bridge_id: str): - """Normalize a bridge identifier. - - There are three sources where we receive bridge ID from: - - ssdp/upnp: /description.xml, field root/device/serialNumber - - nupnp: "id" field - - Hue Bridge API: config.bridgeid - - The SSDP/UPNP source does not contain the middle 4 characters compared - to the other sources. In all our tests the middle 4 characters are "fffe". - """ - if len(bridge_id) == 16: - return bridge_id[0:6] + bridge_id[-6:] - - if len(bridge_id) == 12: - return bridge_id - - LOGGER.warning("Unexpected bridge id number found: %s", bridge_id) - - return bridge_id diff --git a/homeassistant/components/hue/config_flow.py b/homeassistant/components/hue/config_flow.py index 882bf5b70db..f2d7c6d1e8a 100644 --- a/homeassistant/components/hue/config_flow.py +++ b/homeassistant/components/hue/config_flow.py @@ -1,51 +1,24 @@ """Config flow to configure Philips Hue.""" import asyncio -import json -import os +from typing import Dict, Optional -from aiohue.discovery import discover_nupnp +import aiohue +from aiohue.discovery import discover_nupnp, normalize_bridge_id import async_timeout import voluptuous as vol -from homeassistant import config_entries +from homeassistant import config_entries, core from homeassistant.components.ssdp import ATTR_MANUFACTURERURL, ATTR_NAME -from homeassistant.core import callback from homeassistant.helpers import aiohttp_client -from .bridge import get_bridge, normalize_bridge_id -from .const import DOMAIN, LOGGER +from .bridge import authenticate_bridge +from .const import DOMAIN, LOGGER # pylint: disable=unused-import from .errors import AuthenticationRequired, CannotConnect HUE_MANUFACTURERURL = "http://www.philips.com" HUE_IGNORED_BRIDGE_NAMES = ["HASS Bridge", "Espalexa"] -@callback -def configured_hosts(hass): - """Return a set of the configured hosts.""" - return set( - entry.data["host"] for entry in hass.config_entries.async_entries(DOMAIN) - ) - - -def _find_username_from_config(hass, filename): - """Load username from config. - - This was a legacy way of configuring Hue until Home Assistant 0.67. - """ - path = hass.config.path(filename) - - if not os.path.isfile(path): - return None - - with open(path) as inp: - try: - return list(json.load(inp).values())[0]["username"] - except ValueError: - # If we get invalid JSON - return None - - class HueFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): """Handle a Hue config flow.""" @@ -56,23 +29,45 @@ class HueFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): def __init__(self): """Initialize the Hue flow.""" - self.host = None + self.bridge: Optional[aiohue.Bridge] = None + self.discovered_bridges: Optional[Dict[str, aiohue.Bridge]] = None async def async_step_user(self, user_input=None): """Handle a flow initialized by the user.""" + # This is for backwards compatibility. return await self.async_step_init(user_input) + @core.callback + def _async_get_bridge(self, host: str, bridge_id: Optional[str] = None): + """Return a bridge object.""" + if bridge_id is not None: + bridge_id = normalize_bridge_id(bridge_id) + + return aiohue.Bridge( + host, + websession=aiohttp_client.async_get_clientsession(self.hass), + bridge_id=bridge_id, + ) + async def async_step_init(self, user_input=None): """Handle a flow start.""" - if user_input is not None: - self.host = self.context["host"] = user_input["host"] - return await self.async_step_link() - - websession = aiohttp_client.async_get_clientsession(self.hass) + if ( + user_input is not None + and self.discovered_bridges is not None + # pylint: disable=unsupported-membership-test + and user_input["id"] in self.discovered_bridges + ): + # pylint: disable=unsubscriptable-object + self.bridge = self.discovered_bridges[user_input["id"]] + await self.async_set_unique_id(self.bridge.id, raise_on_progress=False) + # We pass user input to link so it will attempt to link right away + return await self.async_step_link({}) try: with async_timeout.timeout(5): - bridges = await discover_nupnp(websession=websession) + bridges = await discover_nupnp( + websession=aiohttp_client.async_get_clientsession(self.hass) + ) except asyncio.TimeoutError: return self.async_abort(reason="discover_timeout") @@ -80,20 +75,28 @@ class HueFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): return self.async_abort(reason="no_bridges") # Find already configured hosts - configured = configured_hosts(self.hass) + already_configured = self._async_current_ids() + bridges = [bridge for bridge in bridges if bridge.id not in already_configured] - hosts = [bridge.host for bridge in bridges if bridge.host not in configured] - - if not hosts: + if not bridges: return self.async_abort(reason="all_configured") - if len(hosts) == 1: - self.host = hosts[0] + if len(bridges) == 1: + self.bridge = bridges[0] + await self.async_set_unique_id(self.bridge.id, raise_on_progress=False) return await self.async_step_link() + self.discovered_bridges = {bridge.id: bridge for bridge in bridges} + return self.async_show_form( step_id="init", - data_schema=vol.Schema({vol.Required("host"): vol.In(hosts)}), + data_schema=vol.Schema( + { + vol.Required("id"): vol.In( + {bridge.id: bridge.host for bridge in bridges} + ) + } + ), ) async def async_step_link(self, user_input=None): @@ -102,31 +105,39 @@ class HueFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): Given a configured host, will ask the user to press the link button to connect to the bridge. """ + if user_input is None: + return self.async_show_form(step_id="link") + + bridge = self.bridge + assert bridge is not None errors = {} - # We will always try linking in case the user has already pressed - # the link button. try: - bridge = await get_bridge(self.hass, self.host, username=None) + await authenticate_bridge(self.hass, bridge) - return await self._entry_from_bridge(bridge) + # Can happen if we come from import. + if self.unique_id is None: + await self.async_set_unique_id( + normalize_bridge_id(bridge.id), raise_on_progress=False + ) + + return self.async_create_entry( + title=bridge.config.name, + data={"host": bridge.host, "username": bridge.username}, + ) except AuthenticationRequired: errors["base"] = "register_failed" except CannotConnect: - LOGGER.error("Error connecting to the Hue bridge at %s", self.host) + LOGGER.error("Error connecting to the Hue bridge at %s", bridge.host) errors["base"] = "linking" except Exception: # pylint: disable=broad-except LOGGER.exception( - "Unknown error connecting with Hue bridge at %s", self.host + "Unknown error connecting with Hue bridge at %s", bridge.host ) errors["base"] = "linking" - # If there was no user input, do not show the errors. - if user_input is None: - errors = {} - return self.async_show_form(step_id="link", errors=errors) async def async_step_ssdp(self, discovery_info): @@ -135,113 +146,55 @@ class HueFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): This flow is triggered by the SSDP component. It will check if the host is already configured and delegate to the import step if not. """ + # Filter out non-Hue bridges #1 if discovery_info[ATTR_MANUFACTURERURL] != HUE_MANUFACTURERURL: return self.async_abort(reason="not_hue_bridge") + # Filter out non-Hue bridges #2 if any( name in discovery_info.get(ATTR_NAME, "") for name in HUE_IGNORED_BRIDGE_NAMES ): return self.async_abort(reason="not_hue_bridge") - host = self.context["host"] = discovery_info.get("host") + if "host" not in discovery_info or "serial" not in discovery_info: + return self.async_abort(reason="not_hue_bridge") - if any( - host == flow["context"].get("host") for flow in self._async_in_progress() - ): - return self.async_abort(reason="already_in_progress") - - if host in configured_hosts(self.hass): - return self.async_abort(reason="already_configured") - - bridge_id = discovery_info.get("serial") - - await self.async_set_unique_id(normalize_bridge_id(bridge_id)) - - return await self.async_step_import( - { - "host": host, - # This format is the legacy format that Hue used for discovery - "path": f"phue-{bridge_id}.conf", - } + bridge = self._async_get_bridge( + discovery_info["host"], discovery_info["serial"] ) + await self.async_set_unique_id(bridge.id) + self._abort_if_unique_id_configured() + self.bridge = bridge + return await self.async_step_link() + async def async_step_homekit(self, homekit_info): """Handle HomeKit discovery.""" - host = self.context["host"] = homekit_info.get("host") - - if any( - host == flow["context"].get("host") for flow in self._async_in_progress() - ): - return self.async_abort(reason="already_in_progress") - - if host in configured_hosts(self.hass): - return self.async_abort(reason="already_configured") - - await self.async_set_unique_id( - normalize_bridge_id(homekit_info["properties"]["id"].replace(":", "")) + bridge = self._async_get_bridge( + homekit_info["host"], homekit_info["properties"]["id"] ) - return await self.async_step_import({"host": host}) + await self.async_set_unique_id(bridge.id) + self._abort_if_unique_id_configured() + self.bridge = bridge + return await self.async_step_link() async def async_step_import(self, import_info): """Import a new bridge as a config entry. - Will read authentication from Phue config file if available. - This flow is triggered by `async_setup` for both configured and discovered bridges. Triggered for any bridge that does not have a config entry yet (based on host). This flow is also triggered by `async_step_discovery`. - - If an existing config file is found, we will validate the credentials - and create an entry. Otherwise we will delegate to `link` step which - will ask user to link the bridge. """ - host = self.context["host"] = import_info["host"] - path = import_info.get("path") + # Check if host exists, abort if so. + if any( + import_info["host"] == entry.data["host"] + for entry in self._async_current_entries() + ): + return self.async_abort(reason="already_configured") - if path is not None: - username = await self.hass.async_add_job( - _find_username_from_config, self.hass, self.hass.config.path(path) - ) - else: - username = None - - try: - bridge = await get_bridge(self.hass, host, username) - - LOGGER.info("Imported authentication for %s from %s", host, path) - - return await self._entry_from_bridge(bridge) - except AuthenticationRequired: - self.host = host - - LOGGER.info("Invalid authentication for %s, requesting link.", host) - - return await self.async_step_link() - - except CannotConnect: - LOGGER.error("Error connecting to the Hue bridge at %s", host) - return self.async_abort(reason="cannot_connect") - - except Exception: # pylint: disable=broad-except - LOGGER.exception("Unknown error connecting with Hue bridge at %s", host) - return self.async_abort(reason="unknown") - - async def _entry_from_bridge(self, bridge): - """Return a config entry from an initialized bridge.""" - # Remove all other entries of hubs with same ID or host - host = bridge.host - bridge_id = bridge.config.bridgeid - - if self.unique_id is None: - await self.async_set_unique_id( - normalize_bridge_id(bridge_id), raise_on_progress=False - ) - - return self.async_create_entry( - title=bridge.config.name, - data={"host": host, "bridge_id": bridge_id, "username": bridge.username}, - ) + self.bridge = self._async_get_bridge(import_info["host"]) + return await self.async_step_link() diff --git a/homeassistant/components/hue/manifest.json b/homeassistant/components/hue/manifest.json index c90b6181559..75384e012e0 100644 --- a/homeassistant/components/hue/manifest.json +++ b/homeassistant/components/hue/manifest.json @@ -3,21 +3,15 @@ "name": "Philips Hue", "config_flow": true, "documentation": "https://www.home-assistant.io/integrations/hue", - "requirements": [ - "aiohue==1.9.2" - ], + "requirements": ["aiohue==1.10.1"], "ssdp": [ { "manufacturer": "Royal Philips Electronics" } ], "homekit": { - "models": [ - "BSB002" - ] + "models": ["BSB002"] }, "dependencies": [], - "codeowners": [ - "@balloob" - ] + "codeowners": ["@balloob"] } diff --git a/homeassistant/config_entries.py b/homeassistant/config_entries.py index 09ee186da0f..d39fc4803ea 100644 --- a/homeassistant/config_entries.py +++ b/homeassistant/config_entries.py @@ -75,10 +75,6 @@ class OperationNotAllowed(ConfigError): """Raised when a config entry operation is not allowed.""" -class UniqueIdInProgress(data_entry_flow.AbortFlow): - """Error to indicate that the unique Id is in progress.""" - - class ConfigEntry: """Hold a configuration entry.""" @@ -379,6 +375,7 @@ class ConfigEntry: "system_options": self.system_options.as_dict(), "source": self.source, "connection_class": self.connection_class, + "unique_id": self.unique_id, } @@ -482,6 +479,8 @@ class ConfigEntries: options=entry.get("options"), # New in 0.98 system_options=entry.get("system_options", {}), + # New in 0.104 + unique_id=entry.get("unique_id"), ) for entry in config["entries"] ] @@ -617,11 +616,20 @@ class ConfigEntries: # Check if config entry exists with unique ID. Unload it. existing_entry = None - unique_id = flow.context.get("unique_id") - if unique_id is not None: + if flow.unique_id is not None: + # Abort all flows in progress with same unique ID. + for progress_flow in self.flow.async_progress(): + if ( + progress_flow["handler"] == flow.handler + and progress_flow["flow_id"] != flow.flow_id + and progress_flow["context"].get("unique_id") == flow.unique_id + ): + self.flow.async_abort(progress_flow["flow_id"]) + + # Find existing entry. for check_entry in self.async_entries(result["handler"]): - if check_entry.unique_id == unique_id: + if check_entry.unique_id == flow.unique_id: existing_entry = check_entry break @@ -643,16 +651,17 @@ class ConfigEntries: system_options={}, source=flow.context["source"], connection_class=flow.CONNECTION_CLASS, - unique_id=unique_id, + unique_id=flow.unique_id, ) self._entries.append(entry) - self._async_schedule_save() await self.async_setup(entry.entry_id) if existing_entry is not None: await self.async_remove(existing_entry.entry_id) + self._async_schedule_save() + result["result"] = entry return result @@ -723,8 +732,6 @@ async def _old_conf_migrator(old_config: Dict[str, Any]) -> Dict[str, Any]: class ConfigFlow(data_entry_flow.FlowHandler): """Base class for config flows with some helpers.""" - unique_id = None - def __init_subclass__(cls, domain: Optional[str] = None, **kwargs: Any) -> None: """Initialize a subclass, register if possible.""" super().__init_subclass__(**kwargs) # type: ignore @@ -733,12 +740,30 @@ class ConfigFlow(data_entry_flow.FlowHandler): CONNECTION_CLASS = CONN_CLASS_UNKNOWN + @property + def unique_id(self) -> Optional[str]: + """Return unique ID if available.""" + # pylint: disable=no-member + if not self.context: + return None + + return cast(Optional[str], self.context.get("unique_id")) + @staticmethod @callback def async_get_options_flow(config_entry: ConfigEntry) -> "OptionsFlow": """Get the options flow for this handler.""" raise data_entry_flow.UnknownHandler + @callback + def _abort_if_unique_id_configured(self) -> None: + """Abort if the unique ID is already configured.""" + if self.unique_id is None: + return + + if self.unique_id in self._async_current_ids(): + raise data_entry_flow.AbortFlow("already_configured") + async def async_set_unique_id( self, unique_id: str, *, raise_on_progress: bool = True ) -> Optional[ConfigEntry]: @@ -749,7 +774,7 @@ class ConfigFlow(data_entry_flow.FlowHandler): if raise_on_progress: for progress in self._async_in_progress(): if progress["context"].get("unique_id") == unique_id: - raise UniqueIdInProgress("already_in_progress") + raise data_entry_flow.AbortFlow("already_in_progress") # pylint: disable=no-member self.context["unique_id"] = unique_id @@ -766,6 +791,15 @@ class ConfigFlow(data_entry_flow.FlowHandler): assert self.hass is not None return self.hass.config_entries.async_entries(self.handler) + @callback + def _async_current_ids(self) -> Set[Optional[str]]: + """Return current unique IDs.""" + assert self.hass is not None + return set( + entry.unique_id + for entry in self.hass.config_entries.async_entries(self.handler) + ) + @callback def _async_in_progress(self) -> List[Dict]: """Return other in progress flows for current domain.""" diff --git a/homeassistant/core.py b/homeassistant/core.py index 0002019fdfa..e76673f5727 100644 --- a/homeassistant/core.py +++ b/homeassistant/core.py @@ -1134,6 +1134,9 @@ class ServiceRegistry: self._services[domain].pop(service) + if not self._services[domain]: + self._services.pop(domain) + self._hass.bus.async_fire( EVENT_SERVICE_REMOVED, {ATTR_DOMAIN: domain, ATTR_SERVICE: service} ) diff --git a/requirements_all.txt b/requirements_all.txt index a8d454aac99..7984cd4a32a 100644 --- a/requirements_all.txt +++ b/requirements_all.txt @@ -163,7 +163,7 @@ aioharmony==0.1.13 aiohttp_cors==0.7.0 # homeassistant.components.hue -aiohue==1.9.2 +aiohue==1.10.1 # homeassistant.components.imap aioimaplib==0.7.15 diff --git a/requirements_test_all.txt b/requirements_test_all.txt index 074d5bd6c8e..20f8e8efc76 100644 --- a/requirements_test_all.txt +++ b/requirements_test_all.txt @@ -63,7 +63,7 @@ aioesphomeapi==2.6.1 aiohttp_cors==0.7.0 # homeassistant.components.hue -aiohue==1.9.2 +aiohue==1.10.1 # homeassistant.components.notion aionotion==1.1.0 diff --git a/tests/components/hue/test_bridge.py b/tests/components/hue/test_bridge.py index b66733e7c76..03966560d8d 100644 --- a/tests/components/hue/test_bridge.py +++ b/tests/components/hue/test_bridge.py @@ -9,107 +9,110 @@ from homeassistant.exceptions import ConfigEntryNotReady from tests.common import mock_coro -async def test_bridge_setup(): +async def test_bridge_setup(hass): """Test a successful setup.""" - hass = Mock() entry = Mock() - api = Mock() + api = Mock(initialize=mock_coro) entry.data = {"host": "1.2.3.4", "username": "mock-username"} hue_bridge = bridge.HueBridge(hass, entry, False, False) - with patch.object(bridge, "get_bridge", return_value=mock_coro(api)): + with patch("aiohue.Bridge", return_value=api), patch.object( + hass.config_entries, "async_forward_entry_setup" + ) as mock_forward: assert await hue_bridge.async_setup() is True assert hue_bridge.api is api - forward_entries = set( - c[1][1] for c in hass.config_entries.async_forward_entry_setup.mock_calls - ) - assert len(hass.config_entries.async_forward_entry_setup.mock_calls) == 3 + assert len(mock_forward.mock_calls) == 3 + forward_entries = set(c[1][1] for c in mock_forward.mock_calls) assert forward_entries == set(["light", "binary_sensor", "sensor"]) -async def test_bridge_setup_invalid_username(): +async def test_bridge_setup_invalid_username(hass): """Test we start config flow if username is no longer whitelisted.""" - 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.AuthenticationRequired): - assert await hue_bridge.async_setup() is False - - assert len(hass.async_create_task.mock_calls) == 1 - assert len(hass.config_entries.flow.async_init.mock_calls) == 1 - assert hass.config_entries.flow.async_init.mock_calls[0][2]["data"] == { - "host": "1.2.3.4" - } - - -async def test_bridge_setup_timeout(hass): - """Test we retry to connect if we cannot connect.""" - 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 + bridge, "authenticate_bridge", side_effect=errors.AuthenticationRequired + ), patch.object( + hass.config_entries.flow, "async_init", return_value=mock_coro() + ) as mock_init: + assert await hue_bridge.async_setup() is False + + assert len(mock_init.mock_calls) == 1 + assert mock_init.mock_calls[0][2]["data"] == {"host": "1.2.3.4"} + + +async def test_bridge_setup_timeout(hass): + """Test we retry to connect if we cannot connect.""" + entry = Mock() + entry.data = {"host": "1.2.3.4", "username": "mock-username"} + hue_bridge = bridge.HueBridge(hass, entry, False, False) + + with patch.object( + bridge, "authenticate_bridge", side_effect=errors.CannotConnect ), pytest.raises(ConfigEntryNotReady): await hue_bridge.async_setup() -async def test_reset_if_entry_had_wrong_auth(): +async def test_reset_if_entry_had_wrong_auth(hass): """Test calling reset when the entry contained wrong auth.""" - 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.AuthenticationRequired): + with patch.object( + bridge, "authenticate_bridge", side_effect=errors.AuthenticationRequired + ), patch.object(bridge, "create_config_flow") as mock_create: assert await hue_bridge.async_setup() is False - assert len(hass.async_create_task.mock_calls) == 1 + assert len(mock_create.mock_calls) == 1 assert await hue_bridge.async_reset() -async def test_reset_unloads_entry_if_setup(): +async def test_reset_unloads_entry_if_setup(hass): """Test calling reset while the entry has been 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", return_value=mock_coro(Mock())): + with patch.object( + bridge, "authenticate_bridge", return_value=mock_coro(Mock()) + ), patch("aiohue.Bridge", return_value=Mock()), patch.object( + hass.config_entries, "async_forward_entry_setup" + ) as mock_forward: assert await hue_bridge.async_setup() is True - assert len(hass.services.async_register.mock_calls) == 1 - assert len(hass.config_entries.async_forward_entry_setup.mock_calls) == 3 + assert len(hass.services.async_services()) == 1 + assert len(mock_forward.mock_calls) == 3 - hass.config_entries.async_forward_entry_unload.return_value = mock_coro(True) - assert await hue_bridge.async_reset() + with patch.object( + hass.config_entries, "async_forward_entry_unload", return_value=mock_coro(True) + ) as mock_forward: + assert await hue_bridge.async_reset() - assert len(hass.config_entries.async_forward_entry_unload.mock_calls) == 3 - assert len(hass.services.async_remove.mock_calls) == 1 + assert len(mock_forward.mock_calls) == 3 + assert len(hass.services.async_services()) == 0 -async def test_handle_unauthorized(): +async def test_handle_unauthorized(hass): """Test handling an unauthorized error on update.""" - 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", return_value=mock_coro(Mock())): + with patch.object( + bridge, "authenticate_bridge", return_value=mock_coro(Mock()) + ), patch("aiohue.Bridge", return_value=Mock()): assert await hue_bridge.async_setup() is True assert hue_bridge.authorized is True - await hue_bridge.handle_unauthorized_error() + with patch.object(bridge, "create_config_flow") as mock_create: + await hue_bridge.handle_unauthorized_error() assert hue_bridge.authorized is False - assert len(hass.async_create_task.mock_calls) == 4 - assert len(hass.config_entries.flow.async_init.mock_calls) == 1 - assert hass.config_entries.flow.async_init.mock_calls[0][2]["data"] == { - "host": "1.2.3.4" - } + assert len(mock_create.mock_calls) == 1 + assert mock_create.mock_calls[0][1][1] == "1.2.3.4" diff --git a/tests/components/hue/test_config_flow.py b/tests/components/hue/test_config_flow.py index 030f6ade1fa..fe9a1f0e32c 100644 --- a/tests/components/hue/test_config_flow.py +++ b/tests/components/hue/test_config_flow.py @@ -6,50 +6,52 @@ import aiohue import pytest import voluptuous as vol -from homeassistant.components.hue import config_flow, const, errors +from homeassistant import data_entry_flow +from homeassistant.components.hue import config_flow, const from tests.common import MockConfigEntry, mock_coro -async def test_flow_works(hass, aioclient_mock): +async def test_flow_works(hass): """Test config flow .""" - aioclient_mock.get( - const.API_NUPNP, json=[{"internalipaddress": "1.2.3.4", "id": "bla"}] - ) + mock_bridge = Mock() + mock_bridge.host = "1.2.3.4" + mock_bridge.username = None + mock_bridge.config.name = "Mock Bridge" + mock_bridge.id = "aabbccddeeff" + + async def mock_create_user(username): + mock_bridge.username = username + + mock_bridge.create_user = mock_create_user + mock_bridge.initialize.return_value = mock_coro() flow = config_flow.HueFlowHandler() flow.hass = hass flow.context = {} - await flow.async_step_init() - with patch("aiohue.Bridge") as mock_bridge: + with patch( + "homeassistant.components.hue.config_flow.discover_nupnp", + return_value=mock_coro([mock_bridge]), + ): + result = await flow.async_step_init() - def mock_constructor(host, websession, username=None): - """Fake the bridge constructor.""" - mock_bridge.host = host - return mock_bridge + assert result["type"] == "form" + assert result["step_id"] == "link" - mock_bridge.side_effect = mock_constructor - mock_bridge.username = "username-abc" - mock_bridge.config.name = "Mock Bridge" - mock_bridge.config.bridgeid = "bridge-id-1234" - mock_bridge.create_user.return_value = mock_coro() - mock_bridge.initialize.return_value = mock_coro() + assert flow.context["unique_id"] == "aabbccddeeff" - result = await flow.async_step_link(user_input={}) - - assert mock_bridge.host == "1.2.3.4" - assert len(mock_bridge.create_user.mock_calls) == 1 - assert len(mock_bridge.initialize.mock_calls) == 1 + result = await flow.async_step_link(user_input={}) assert result["type"] == "create_entry" assert result["title"] == "Mock Bridge" assert result["data"] == { "host": "1.2.3.4", - "bridge_id": "bridge-id-1234", - "username": "username-abc", + "username": "home-assistant#test-home", } + assert len(mock_bridge.initialize.mock_calls) == 1 + async def test_flow_no_discovered_bridges(hass, aioclient_mock): """Test config flow discovers no bridges.""" @@ -66,9 +68,12 @@ async def test_flow_all_discovered_bridges_exist(hass, aioclient_mock): aioclient_mock.get( const.API_NUPNP, json=[{"internalipaddress": "1.2.3.4", "id": "bla"}] ) - MockConfigEntry(domain="hue", data={"host": "1.2.3.4"}).add_to_hass(hass) + MockConfigEntry( + domain="hue", unique_id="bla", data={"host": "1.2.3.4"} + ).add_to_hass(hass) flow = config_flow.HueFlowHandler() flow.hass = hass + flow.context = {} result = await flow.async_step_init() assert result["type"] == "abort" @@ -81,6 +86,7 @@ async def test_flow_one_bridge_discovered(hass, aioclient_mock): ) flow = config_flow.HueFlowHandler() flow.hass = hass + flow.context = {} result = await flow.async_step_init() assert result["type"] == "form" @@ -104,10 +110,10 @@ async def test_flow_two_bridges_discovered(hass, aioclient_mock): assert result["step_id"] == "init" with pytest.raises(vol.Invalid): - assert result["data_schema"]({"host": "0.0.0.0"}) + assert result["data_schema"]({"id": "not-discovered"}) - result["data_schema"]({"host": "1.2.3.4"}) - result["data_schema"]({"host": "5.6.7.8"}) + result["data_schema"]({"id": "bla"}) + result["data_schema"]({"id": "beer"}) async def test_flow_two_bridges_discovered_one_new(hass, aioclient_mock): @@ -119,14 +125,17 @@ async def test_flow_two_bridges_discovered_one_new(hass, aioclient_mock): {"internalipaddress": "5.6.7.8", "id": "beer"}, ], ) - MockConfigEntry(domain="hue", data={"host": "1.2.3.4"}).add_to_hass(hass) + MockConfigEntry( + domain="hue", unique_id="bla", data={"host": "1.2.3.4"} + ).add_to_hass(hass) flow = config_flow.HueFlowHandler() flow.hass = hass + flow.context = {} result = await flow.async_step_init() assert result["type"] == "form" assert result["step_id"] == "link" - assert flow.host == "5.6.7.8" + assert flow.bridge.host == "5.6.7.8" async def test_flow_timeout_discovery(hass): @@ -147,6 +156,7 @@ async def test_flow_link_timeout(hass): """Test config flow .""" flow = config_flow.HueFlowHandler() flow.hass = hass + flow.bridge = Mock() with patch("aiohue.Bridge.create_user", side_effect=asyncio.TimeoutError): result = await flow.async_step_link({}) @@ -160,9 +170,11 @@ async def test_flow_link_button_not_pressed(hass): """Test config flow .""" flow = config_flow.HueFlowHandler() flow.hass = hass + flow.bridge = Mock( + username=None, create_user=Mock(side_effect=aiohue.LinkButtonNotPressed) + ) - with patch("aiohue.Bridge.create_user", side_effect=aiohue.LinkButtonNotPressed): - result = await flow.async_step_link({}) + result = await flow.async_step_link({}) assert result["type"] == "form" assert result["step_id"] == "link" @@ -173,6 +185,7 @@ async def test_flow_link_unknown_host(hass): """Test config flow .""" flow = config_flow.HueFlowHandler() flow.hass = hass + flow.bridge = Mock() with patch("aiohue.Bridge.create_user", side_effect=aiohue.RequestError): result = await flow.async_step_link({}) @@ -188,16 +201,13 @@ async def test_bridge_ssdp(hass): flow.hass = hass flow.context = {} - with patch.object( - config_flow, "get_bridge", side_effect=errors.AuthenticationRequired - ): - result = await flow.async_step_ssdp( - { - "host": "0.0.0.0", - "serial": "1234", - "manufacturerURL": config_flow.HUE_MANUFACTURERURL, - } - ) + result = await flow.async_step_ssdp( + { + "host": "0.0.0.0", + "serial": "1234", + "manufacturerURL": config_flow.HUE_MANUFACTURERURL, + } + ) assert result["type"] == "form" assert result["step_id"] == "link" @@ -255,47 +265,22 @@ async def test_bridge_ssdp_espalexa(hass): async def test_bridge_ssdp_already_configured(hass): """Test if a discovered bridge has already been configured.""" - MockConfigEntry(domain="hue", data={"host": "0.0.0.0"}).add_to_hass(hass) + MockConfigEntry( + domain="hue", unique_id="1234", data={"host": "0.0.0.0"} + ).add_to_hass(hass) flow = config_flow.HueFlowHandler() flow.hass = hass flow.context = {} - result = await flow.async_step_ssdp( - { - "host": "0.0.0.0", - "serial": "1234", - "manufacturerURL": config_flow.HUE_MANUFACTURERURL, - } - ) - - assert result["type"] == "abort" - - -async def test_import_with_existing_config(hass): - """Test importing a host with an existing config file.""" - flow = config_flow.HueFlowHandler() - flow.hass = hass - flow.context = {} - - bridge = Mock() - bridge.username = "username-abc" - bridge.config.bridgeid = "bridge-id-1234" - bridge.config.name = "Mock Bridge" - bridge.host = "0.0.0.0" - - with patch.object( - config_flow, "_find_username_from_config", return_value="mock-user" - ), patch.object(config_flow, "get_bridge", return_value=mock_coro(bridge)): - result = await flow.async_step_import({"host": "0.0.0.0", "path": "bla.conf"}) - - assert result["type"] == "create_entry" - assert result["title"] == "Mock Bridge" - assert result["data"] == { - "host": "0.0.0.0", - "bridge_id": "bridge-id-1234", - "username": "username-abc", - } + with pytest.raises(data_entry_flow.AbortFlow): + await flow.async_step_ssdp( + { + "host": "0.0.0.0", + "serial": "1234", + "manufacturerURL": config_flow.HUE_MANUFACTURERURL, + } + ) async def test_import_with_no_config(hass): @@ -304,45 +289,12 @@ async def test_import_with_no_config(hass): flow.hass = hass flow.context = {} - with patch.object( - config_flow, "get_bridge", side_effect=errors.AuthenticationRequired - ): - result = await flow.async_step_import({"host": "0.0.0.0"}) + result = await flow.async_step_import({"host": "0.0.0.0"}) assert result["type"] == "form" assert result["step_id"] == "link" -async def test_import_with_existing_but_invalid_config(hass): - """Test importing a host with a config file with invalid username.""" - flow = config_flow.HueFlowHandler() - flow.hass = hass - flow.context = {} - - with patch.object( - config_flow, "_find_username_from_config", return_value="mock-user" - ), patch.object( - config_flow, "get_bridge", side_effect=errors.AuthenticationRequired - ): - result = await flow.async_step_import({"host": "0.0.0.0", "path": "bla.conf"}) - - assert result["type"] == "form" - assert result["step_id"] == "link" - - -async def test_import_cannot_connect(hass): - """Test importing a host that we cannot conncet to.""" - flow = config_flow.HueFlowHandler() - flow.hass = hass - flow.context = {} - - with patch.object(config_flow, "get_bridge", side_effect=errors.CannotConnect): - result = await flow.async_step_import({"host": "0.0.0.0"}) - - assert result["type"] == "abort" - assert result["reason"] == "cannot_connect" - - async def test_creating_entry_removes_entries_for_same_host_or_bridge(hass): """Test that we clean up entries for same host and bridge. @@ -351,38 +303,45 @@ async def test_creating_entry_removes_entries_for_same_host_or_bridge(hass): all existing entries that either have same IP or same bridge_id. """ orig_entry = MockConfigEntry( - domain="hue", - data={"host": "0.0.0.0", "bridge_id": "id-1234"}, - unique_id="id-1234", + domain="hue", data={"host": "0.0.0.0", "username": "aaaa"}, unique_id="id-1234", ) orig_entry.add_to_hass(hass) MockConfigEntry( - domain="hue", - data={"host": "1.2.3.4", "bridge_id": "id-5678"}, - unique_id="id-5678", + domain="hue", data={"host": "1.2.3.4", "username": "bbbb"}, unique_id="id-5678", ).add_to_hass(hass) assert len(hass.config_entries.async_entries("hue")) == 2 bridge = Mock() bridge.username = "username-abc" - bridge.config.bridgeid = "id-1234" bridge.config.name = "Mock Bridge" bridge.host = "0.0.0.0" + bridge.id = "id-1234" - with patch.object( - config_flow, "_find_username_from_config", return_value="mock-user" - ), patch.object(config_flow, "get_bridge", return_value=mock_coro(bridge)): + with patch( + "aiohue.Bridge", return_value=bridge, + ): result = await hass.config_entries.flow.async_init( "hue", data={"host": "2.2.2.2"}, context={"source": "import"} ) + assert result["type"] == "form" + assert result["step_id"] == "link" + + with patch( + "homeassistant.components.hue.config_flow.authenticate_bridge", + return_value=mock_coro(), + ), patch( + "homeassistant.components.hue.async_setup_entry", + side_effect=lambda _, _2: mock_coro(True), + ): + result = await hass.config_entries.flow.async_configure(result["flow_id"], {}) + assert result["type"] == "create_entry" assert result["title"] == "Mock Bridge" assert result["data"] == { "host": "0.0.0.0", - "bridge_id": "id-1234", "username": "username-abc", } entries = hass.config_entries.async_entries("hue") @@ -398,17 +357,14 @@ async def test_bridge_homekit(hass): flow.hass = hass flow.context = {} - with patch.object( - config_flow, "get_bridge", side_effect=errors.AuthenticationRequired - ): - result = await flow.async_step_homekit( - { - "host": "0.0.0.0", - "serial": "1234", - "manufacturerURL": config_flow.HUE_MANUFACTURERURL, - "properties": {"id": "aa:bb:cc:dd:ee:ff"}, - } - ) + result = await flow.async_step_homekit( + { + "host": "0.0.0.0", + "serial": "1234", + "manufacturerURL": config_flow.HUE_MANUFACTURERURL, + "properties": {"id": "aa:bb:cc:dd:ee:ff"}, + } + ) assert result["type"] == "form" assert result["step_id"] == "link" @@ -416,12 +372,15 @@ async def test_bridge_homekit(hass): async def test_bridge_homekit_already_configured(hass): """Test if a HomeKit discovered bridge has already been configured.""" - MockConfigEntry(domain="hue", data={"host": "0.0.0.0"}).add_to_hass(hass) + MockConfigEntry( + domain="hue", unique_id="aabbccddeeff", data={"host": "0.0.0.0"} + ).add_to_hass(hass) flow = config_flow.HueFlowHandler() flow.hass = hass flow.context = {} - result = await flow.async_step_homekit({"host": "0.0.0.0"}) - - assert result["type"] == "abort" + with pytest.raises(data_entry_flow.AbortFlow): + await flow.async_step_homekit( + {"host": "0.0.0.0", "properties": {"id": "aa:bb:cc:dd:ee:ff"}} + ) diff --git a/tests/components/hue/test_init.py b/tests/components/hue/test_init.py index d064ff9f340..b48d66990e8 100644 --- a/tests/components/hue/test_init.py +++ b/tests/components/hue/test_init.py @@ -9,13 +9,10 @@ from tests.common import MockConfigEntry, mock_coro async def test_setup_with_no_config(hass): """Test that we do not discover anything or try to set up a bridge.""" - with patch.object(hass, "config_entries") as mock_config_entries, patch.object( - hue, "configured_hosts", return_value=[] - ): - assert await async_setup_component(hass, hue.DOMAIN, {}) is True + assert await async_setup_component(hass, hue.DOMAIN, {}) is True # No flows started - assert len(mock_config_entries.flow.mock_calls) == 0 + assert len(hass.config_entries.flow.async_progress()) == 0 # No configs stored assert hass.data[hue.DOMAIN] == {} @@ -23,9 +20,9 @@ async def test_setup_with_no_config(hass): async def test_setup_defined_hosts_known_auth(hass): """Test we don't initiate a config entry if config bridge is known.""" - with patch.object(hass, "config_entries") as mock_config_entries, patch.object( - hue, "configured_hosts", return_value=["0.0.0.0"] - ): + MockConfigEntry(domain="hue", data={"host": "0.0.0.0"}).add_to_hass(hass) + + with patch.object(hue, "async_setup_entry", return_value=mock_coro(True)): assert ( await async_setup_component( hass, @@ -34,7 +31,6 @@ async def test_setup_defined_hosts_known_auth(hass): hue.DOMAIN: { hue.CONF_BRIDGES: { hue.CONF_HOST: "0.0.0.0", - hue.CONF_FILENAME: "bla.conf", hue.CONF_ALLOW_HUE_GROUPS: False, hue.CONF_ALLOW_UNREACHABLE: True, } @@ -45,13 +41,12 @@ async def test_setup_defined_hosts_known_auth(hass): ) # Flow started for discovered bridge - assert len(mock_config_entries.flow.mock_calls) == 0 + assert len(hass.config_entries.flow.async_progress()) == 0 # Config stored for domain. assert hass.data[hue.DATA_CONFIGS] == { "0.0.0.0": { hue.CONF_HOST: "0.0.0.0", - hue.CONF_FILENAME: "bla.conf", hue.CONF_ALLOW_HUE_GROUPS: False, hue.CONF_ALLOW_UNREACHABLE: True, } @@ -60,40 +55,30 @@ async def test_setup_defined_hosts_known_auth(hass): async def test_setup_defined_hosts_no_known_auth(hass): """Test we initiate config entry if config bridge is not known.""" - with patch.object(hass, "config_entries") as mock_config_entries, patch.object( - hue, "configured_hosts", return_value=[] - ): - mock_config_entries.flow.async_init.return_value = mock_coro() - assert ( - await async_setup_component( - hass, - hue.DOMAIN, - { - hue.DOMAIN: { - hue.CONF_BRIDGES: { - hue.CONF_HOST: "0.0.0.0", - hue.CONF_FILENAME: "bla.conf", - hue.CONF_ALLOW_HUE_GROUPS: False, - hue.CONF_ALLOW_UNREACHABLE: True, - } + assert ( + await async_setup_component( + hass, + hue.DOMAIN, + { + hue.DOMAIN: { + hue.CONF_BRIDGES: { + hue.CONF_HOST: "0.0.0.0", + hue.CONF_ALLOW_HUE_GROUPS: False, + hue.CONF_ALLOW_UNREACHABLE: True, } - }, - ) - is True + } + }, ) + is True + ) # Flow started for discovered bridge - assert len(mock_config_entries.flow.mock_calls) == 1 - assert mock_config_entries.flow.mock_calls[0][2]["data"] == { - "host": "0.0.0.0", - "path": "bla.conf", - } + assert len(hass.config_entries.flow.async_progress()) == 1 # Config stored for domain. assert hass.data[hue.DATA_CONFIGS] == { "0.0.0.0": { hue.CONF_HOST: "0.0.0.0", - hue.CONF_FILENAME: "bla.conf", hue.CONF_ALLOW_HUE_GROUPS: False, hue.CONF_ALLOW_UNREACHABLE: True, } @@ -126,7 +111,6 @@ async def test_config_passed_to_config_entry(hass): hue.DOMAIN: { hue.CONF_BRIDGES: { hue.CONF_HOST: "0.0.0.0", - hue.CONF_FILENAME: "bla.conf", hue.CONF_ALLOW_HUE_GROUPS: False, hue.CONF_ALLOW_UNREACHABLE: True, } @@ -166,7 +150,7 @@ async def test_unload_entry(hass): return_value=mock_coro(Mock()), ): mock_bridge.return_value.async_setup.return_value = mock_coro(True) - mock_bridge.return_value.api.config = Mock() + mock_bridge.return_value.api.config = Mock(bridgeid="aabbccddeeff") assert await async_setup_component(hass, hue.DOMAIN, {}) is True assert len(mock_bridge.return_value.mock_calls) == 1 diff --git a/tests/test_config_entries.py b/tests/test_config_entries.py index a9ae4eb59ac..d83de450227 100644 --- a/tests/test_config_entries.py +++ b/tests/test_config_entries.py @@ -434,8 +434,8 @@ async def test_saving_and_loading(hass): VERSION = 5 CONNECTION_CLASS = config_entries.CONN_CLASS_LOCAL_POLL - @asyncio.coroutine - def async_step_user(self, user_input=None): + async def async_step_user(self, user_input=None): + await self.async_set_unique_id("unique") return self.async_create_entry(title="Test Title", data={"token": "abcd"}) with patch.dict(config_entries.HANDLERS, {"test": TestFlow}): @@ -477,6 +477,7 @@ async def test_saving_and_loading(hass): assert orig.data == loaded.data assert orig.source == loaded.source assert orig.connection_class == loaded.connection_class + assert orig.unique_id == loaded.unique_id async def test_forward_entry_sets_up_component(hass): @@ -1108,3 +1109,40 @@ async def test_unique_id_in_progress(hass, manager): assert result2["type"] == data_entry_flow.RESULT_TYPE_ABORT assert result2["reason"] == "already_in_progress" + + +async def test_finish_flow_aborts_progress(hass, manager): + """Test that when finishing a flow, we abort other flows in progress with unique ID.""" + mock_integration( + hass, + MockModule("comp", async_setup_entry=MagicMock(return_value=mock_coro(True))), + ) + mock_entity_platform(hass, "config_flow.comp", None) + + class TestFlow(config_entries.ConfigFlow): + + VERSION = 1 + + async def async_step_user(self, user_input=None): + await self.async_set_unique_id("mock-unique-id", raise_on_progress=False) + + if user_input is None: + return self.async_show_form(step_id="discovery") + + return self.async_create_entry(title="yo", data={}) + + with patch.dict(config_entries.HANDLERS, {"comp": TestFlow}): + # Create one to be in progress + result = await manager.flow.async_init( + "comp", context={"source": config_entries.SOURCE_USER} + ) + assert result["type"] == data_entry_flow.RESULT_TYPE_FORM + + # Will finish and cancel other one. + result2 = await manager.flow.async_init( + "comp", context={"source": config_entries.SOURCE_USER}, data={} + ) + + assert result2["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY + + assert len(hass.config_entries.flow.async_progress()) == 0