From 8d1cf553de9d170b36e95ef81b99fdd81b867d34 Mon Sep 17 00:00:00 2001 From: Robert Svensson Date: Sun, 24 Mar 2019 19:27:32 +0100 Subject: [PATCH] Support deCONZ library with exception handling (#21952) --- homeassistant/components/deconz/__init__.py | 5 +- .../components/deconz/config_flow.py | 45 ++++++++--- homeassistant/components/deconz/errors.py | 18 +++++ homeassistant/components/deconz/gateway.py | 42 +++++++--- requirements_all.txt | 2 +- requirements_test_all.txt | 2 +- tests/components/deconz/test_climate.py | 13 ++-- tests/components/deconz/test_config_flow.py | 77 ++++++++++++------- tests/components/deconz/test_gateway.py | 37 +++++++-- tests/components/deconz/test_init.py | 14 +++- 10 files changed, 190 insertions(+), 65 deletions(-) create mode 100644 homeassistant/components/deconz/errors.py diff --git a/homeassistant/components/deconz/__init__.py b/homeassistant/components/deconz/__init__.py index d107cba8f7b..957bb569110 100644 --- a/homeassistant/components/deconz/__init__.py +++ b/homeassistant/components/deconz/__init__.py @@ -12,7 +12,7 @@ from .config_flow import configured_hosts from .const import DEFAULT_PORT, DOMAIN, _LOGGER from .gateway import DeconzGateway -REQUIREMENTS = ['pydeconz==52'] +REQUIREMENTS = ['pydeconz==53'] CONFIG_SCHEMA = vol.Schema({ DOMAIN: vol.Schema({ @@ -124,8 +124,7 @@ async def async_setup_entry(hass, config_entry): scenes = set(gateway.api.scenes.keys()) sensors = set(gateway.api.sensors.keys()) - if not await gateway.api.async_load_parameters(): - return + await gateway.api.async_load_parameters() gateway.async_add_device_callback( 'group', [group diff --git a/homeassistant/components/deconz/config_flow.py b/homeassistant/components/deconz/config_flow.py index 8f90f303fca..cabb5b46ece 100644 --- a/homeassistant/components/deconz/config_flow.py +++ b/homeassistant/components/deconz/config_flow.py @@ -1,4 +1,6 @@ """Config flow to configure deCONZ component.""" +import asyncio +import async_timeout import voluptuous as vol from homeassistant import config_entries @@ -32,15 +34,12 @@ class DeconzFlowHandler(config_entries.ConfigFlow): self.deconz_config = {} async def async_step_user(self, user_input=None): - """Handle a flow initialized by the user.""" - return await self.async_step_init(user_input) - - async def async_step_init(self, user_input=None): """Handle a deCONZ config flow start. Only allows one instance to be set up. If only one bridge is found go to link step. If more than one bridge is found let user choose bridge to link. + If no bridge is found allow user to manually input configuration. """ from pydeconz.utils import async_discovery @@ -52,11 +51,18 @@ class DeconzFlowHandler(config_entries.ConfigFlow): if bridge[CONF_HOST] == user_input[CONF_HOST]: self.deconz_config = bridge return await self.async_step_link() + self.deconz_config = user_input return await self.async_step_link() session = aiohttp_client.async_get_clientsession(self.hass) - self.bridges = await async_discovery(session) + + try: + with async_timeout.timeout(10): + self.bridges = await async_discovery(session) + + except asyncio.TimeoutError: + self.bridges = [] if len(self.bridges) == 1: self.deconz_config = self.bridges[0] @@ -64,8 +70,10 @@ class DeconzFlowHandler(config_entries.ConfigFlow): if len(self.bridges) > 1: hosts = [] + for bridge in self.bridges: hosts.append(bridge[CONF_HOST]) + return self.async_show_form( step_id='init', data_schema=vol.Schema({ @@ -74,7 +82,7 @@ class DeconzFlowHandler(config_entries.ConfigFlow): ) return self.async_show_form( - step_id='user', + step_id='init', data_schema=vol.Schema({ vol.Required(CONF_HOST): str, vol.Required(CONF_PORT, default=DEFAULT_PORT): int, @@ -83,18 +91,27 @@ class DeconzFlowHandler(config_entries.ConfigFlow): async def async_step_link(self, user_input=None): """Attempt to link with the deCONZ bridge.""" + from pydeconz.errors import ResponseError, RequestError from pydeconz.utils import async_get_api_key errors = {} if user_input is not None: if configured_hosts(self.hass): return self.async_abort(reason='one_instance_only') + session = aiohttp_client.async_get_clientsession(self.hass) - api_key = await async_get_api_key(session, **self.deconz_config) - if api_key: + + try: + with async_timeout.timeout(10): + api_key = await async_get_api_key( + session, **self.deconz_config) + + except (ResponseError, RequestError, asyncio.TimeoutError): + errors['base'] = 'no_key' + + else: self.deconz_config[CONF_API_KEY] = api_key return await self.async_step_options() - errors['base'] = 'no_key' return self.async_show_form( step_id='link', @@ -117,8 +134,14 @@ class DeconzFlowHandler(config_entries.ConfigFlow): if CONF_BRIDGEID not in self.deconz_config: session = aiohttp_client.async_get_clientsession(self.hass) - self.deconz_config[CONF_BRIDGEID] = await async_get_bridgeid( - session, **self.deconz_config) + try: + with async_timeout.timeout(10): + self.deconz_config[CONF_BRIDGEID] = \ + await async_get_bridgeid( + session, **self.deconz_config) + + except asyncio.TimeoutError: + return self.async_abort(reason='no_bridges') return self.async_create_entry( title='deCONZ-' + self.deconz_config[CONF_BRIDGEID], diff --git a/homeassistant/components/deconz/errors.py b/homeassistant/components/deconz/errors.py new file mode 100644 index 00000000000..be13e579ce0 --- /dev/null +++ b/homeassistant/components/deconz/errors.py @@ -0,0 +1,18 @@ +"""Errors for the deCONZ component.""" +from homeassistant.exceptions import HomeAssistantError + + +class DeconzException(HomeAssistantError): + """Base class for deCONZ exceptions.""" + + +class AlreadyConfigured(DeconzException): + """Gateway is already configured.""" + + +class AuthenticationRequired(DeconzException): + """Unknown error occurred.""" + + +class CannotConnect(DeconzException): + """Unable to connect to the gateway.""" diff --git a/homeassistant/components/deconz/gateway.py b/homeassistant/components/deconz/gateway.py index 829485e1e92..6629d4eec14 100644 --- a/homeassistant/components/deconz/gateway.py +++ b/homeassistant/components/deconz/gateway.py @@ -1,6 +1,9 @@ """Representation of a deCONZ gateway.""" +import asyncio +import async_timeout + from homeassistant.exceptions import ConfigEntryNotReady -from homeassistant.const import CONF_EVENT, CONF_ID +from homeassistant.const import CONF_EVENT, CONF_HOST, CONF_ID from homeassistant.core import EventOrigin, callback from homeassistant.helpers import aiohttp_client from homeassistant.helpers.dispatcher import ( @@ -10,6 +13,7 @@ from homeassistant.util import slugify from .const import ( _LOGGER, DECONZ_REACHABLE, CONF_ALLOW_CLIP_SENSOR, NEW_DEVICE, NEW_SENSOR, SUPPORTED_PLATFORMS) +from .errors import AuthenticationRequired, CannotConnect class DeconzGateway: @@ -26,18 +30,23 @@ class DeconzGateway: self.events = [] self.listeners = [] - async def async_setup(self, tries=0): + async def async_setup(self): """Set up a deCONZ gateway.""" hass = self.hass - self.api = await get_gateway( - hass, self.config_entry.data, self.async_add_device_callback, - self.async_connection_status_callback - ) + try: + self.api = await get_gateway( + hass, self.config_entry.data, self.async_add_device_callback, + self.async_connection_status_callback + ) - if not self.api: + except CannotConnect: raise ConfigEntryNotReady + except Exception: # pylint: disable=broad-except + _LOGGER.error('Error connecting with deCONZ gateway.') + return False + for component in SUPPORTED_PLATFORMS: hass.async_create_task( hass.config_entries.async_forward_entry_setup( @@ -113,17 +122,26 @@ class DeconzGateway: async def get_gateway(hass, config, async_add_device_callback, async_connection_status_callback): """Create a gateway object and verify configuration.""" - from pydeconz import DeconzSession + from pydeconz import DeconzSession, errors session = aiohttp_client.async_get_clientsession(hass) + deconz = DeconzSession(hass.loop, session, **config, async_add_device=async_add_device_callback, connection_status=async_connection_status_callback) - result = await deconz.async_load_parameters() - - if result: + try: + with async_timeout.timeout(10): + await deconz.async_load_parameters() return deconz - return result + + except errors.Unauthorized: + _LOGGER.warning("Invalid key for deCONZ at %s.", config[CONF_HOST]) + raise AuthenticationRequired + + except (asyncio.TimeoutError, errors.RequestError): + _LOGGER.error( + "Error connecting to deCONZ gateway at %s", config[CONF_HOST]) + raise CannotConnect class DeconzEvent: diff --git a/requirements_all.txt b/requirements_all.txt index 14e845074e6..15ae90ebe0b 100644 --- a/requirements_all.txt +++ b/requirements_all.txt @@ -1001,7 +1001,7 @@ pydaikin==1.1.0 pydanfossair==0.0.7 # homeassistant.components.deconz -pydeconz==52 +pydeconz==53 # homeassistant.components.zwave pydispatcher==2.0.5 diff --git a/requirements_test_all.txt b/requirements_test_all.txt index 731f7fa9d22..05a14e18fc0 100644 --- a/requirements_test_all.txt +++ b/requirements_test_all.txt @@ -200,7 +200,7 @@ pyHS100==0.3.4 pyblackbird==0.5 # homeassistant.components.deconz -pydeconz==52 +pydeconz==53 # homeassistant.components.zwave pydispatcher==2.0.5 diff --git a/tests/components/deconz/test_climate.py b/tests/components/deconz/test_climate.py index fa274f1d676..953bb776419 100644 --- a/tests/components/deconz/test_climate.py +++ b/tests/components/deconz/test_climate.py @@ -46,11 +46,14 @@ async def setup_gateway(hass, data, allow_clip_sensor=True): """Load the deCONZ sensor platform.""" from pydeconz import DeconzSession - session = Mock(put=asynctest.CoroutineMock( - return_value=Mock(status=200, - json=asynctest.CoroutineMock(), - text=asynctest.CoroutineMock(), - ) + response = Mock( + status=200, json=asynctest.CoroutineMock(), + text=asynctest.CoroutineMock()) + response.content_type = 'application/json' + + session = Mock( + put=asynctest.CoroutineMock( + return_value=response ) ) diff --git a/tests/components/deconz/test_config_flow.py b/tests/components/deconz/test_config_flow.py index 9e1d6a2fca1..20c74a82883 100644 --- a/tests/components/deconz/test_config_flow.py +++ b/tests/components/deconz/test_config_flow.py @@ -1,7 +1,8 @@ """Tests for deCONZ config flow.""" -import pytest +from unittest.mock import patch + +import asyncio -import voluptuous as vol from homeassistant.components.deconz import config_flow from tests.common import MockConfigEntry @@ -12,15 +13,17 @@ async def test_flow_works(hass, aioclient_mock): """Test that config flow works.""" aioclient_mock.get(pydeconz.utils.URL_DISCOVER, json=[ {'id': 'id', 'internalipaddress': '1.2.3.4', 'internalport': 80} - ]) + ], headers={'content-type': 'application/json'}) aioclient_mock.post('http://1.2.3.4:80/api', json=[ {"success": {"username": "1234567890ABCDEF"}} - ]) + ], headers={'content-type': 'application/json'}) flow = config_flow.DeconzFlowHandler() flow.hass = hass + await flow.async_step_user() await flow.async_step_link(user_input={}) + result = await flow.async_step_options( user_input={'allow_clip_sensor': True, 'allow_deconz_groups': True}) @@ -41,35 +44,53 @@ async def test_flow_already_registered_bridge(hass): MockConfigEntry(domain='deconz', data={ 'host': '1.2.3.4' }).add_to_hass(hass) + flow = config_flow.DeconzFlowHandler() flow.hass = hass - result = await flow.async_step_init() + result = await flow.async_step_user() assert result['type'] == 'abort' +async def test_flow_bridge_discovery_fails(hass, aioclient_mock): + """Test config flow works when discovery fails.""" + flow = config_flow.DeconzFlowHandler() + flow.hass = hass + + with patch('pydeconz.utils.async_discovery', + side_effect=asyncio.TimeoutError): + result = await flow.async_step_user() + + assert result['type'] == 'form' + assert result['step_id'] == 'init' + + async def test_flow_no_discovered_bridges(hass, aioclient_mock): """Test config flow discovers no bridges.""" - aioclient_mock.get(pydeconz.utils.URL_DISCOVER, json=[]) + aioclient_mock.get(pydeconz.utils.URL_DISCOVER, json=[], + headers={'content-type': 'application/json'}) + flow = config_flow.DeconzFlowHandler() flow.hass = hass - result = await flow.async_step_init() + result = await flow.async_step_user() assert result['type'] == 'form' - assert result['step_id'] == 'user' + assert result['step_id'] == 'init' async def test_flow_one_bridge_discovered(hass, aioclient_mock): """Test config flow discovers one bridge.""" aioclient_mock.get(pydeconz.utils.URL_DISCOVER, json=[ {'id': 'id', 'internalipaddress': '1.2.3.4', 'internalport': 80} - ]) + ], headers={'content-type': 'application/json'}) + flow = config_flow.DeconzFlowHandler() flow.hass = hass - result = await flow.async_step_init() + result = await flow.async_step_user() assert result['type'] == 'form' assert result['step_id'] == 'link' + assert flow.deconz_config['host'] == '1.2.3.4' async def test_flow_two_bridges_discovered(hass, aioclient_mock): @@ -77,19 +98,14 @@ async def test_flow_two_bridges_discovered(hass, aioclient_mock): aioclient_mock.get(pydeconz.utils.URL_DISCOVER, json=[ {'id': 'id1', 'internalipaddress': '1.2.3.4', 'internalport': 80}, {'id': 'id2', 'internalipaddress': '5.6.7.8', 'internalport': 80} - ]) + ], headers={'content-type': 'application/json'}) + flow = config_flow.DeconzFlowHandler() flow.hass = hass - result = await flow.async_step_init() - assert result['type'] == 'form' - assert result['step_id'] == 'init' - - with pytest.raises(vol.Invalid): - assert result['data_schema']({'host': '0.0.0.0'}) - - result['data_schema']({'host': '1.2.3.4'}) - result['data_schema']({'host': '5.6.7.8'}) + result = await flow.async_step_user() + assert result['data_schema']({'host': '1.2.3.4'}) + assert result['data_schema']({'host': '5.6.7.8'}) async def test_flow_two_bridges_selection(hass, aioclient_mock): @@ -101,7 +117,7 @@ async def test_flow_two_bridges_selection(hass, aioclient_mock): {'bridgeid': 'id2', 'host': '5.6.7.8', 'port': 80} ] - result = await flow.async_step_init(user_input={'host': '1.2.3.4'}) + result = await flow.async_step_user(user_input={'host': '1.2.3.4'}) assert result['type'] == 'form' assert result['step_id'] == 'link' assert flow.deconz_config['host'] == '1.2.3.4' @@ -110,25 +126,28 @@ async def test_flow_two_bridges_selection(hass, aioclient_mock): async def test_flow_manual_configuration(hass, aioclient_mock): """Test config flow with manual input.""" aioclient_mock.get(pydeconz.utils.URL_DISCOVER, json=[]) + flow = config_flow.DeconzFlowHandler() flow.hass = hass user_input = {'host': '1.2.3.4', 'port': 80} - result = await flow.async_step_init(user_input) + result = await flow.async_step_user(user_input) assert result['type'] == 'form' assert result['step_id'] == 'link' assert flow.deconz_config == user_input -async def test_link_no_api_key(hass, aioclient_mock): +async def test_link_no_api_key(hass): """Test config flow should abort if no API key was possible to retrieve.""" - aioclient_mock.post('http://1.2.3.4:80/api', json=[]) flow = config_flow.DeconzFlowHandler() flow.hass = hass flow.deconz_config = {'host': '1.2.3.4', 'port': 80} - result = await flow.async_step_link(user_input={}) + with patch('pydeconz.utils.async_get_api_key', + side_effect=pydeconz.errors.ResponseError): + result = await flow.async_step_link(user_input={}) + assert result['type'] == 'form' assert result['step_id'] == 'link' assert result['errors'] == {'base': 'no_key'} @@ -143,6 +162,7 @@ async def test_link_already_registered_bridge(hass): MockConfigEntry(domain='deconz', data={ 'host': '1.2.3.4' }).add_to_hass(hass) + flow = config_flow.DeconzFlowHandler() flow.hass = hass flow.deconz_config = {'host': '1.2.3.4', 'port': 80} @@ -155,6 +175,7 @@ async def test_bridge_discovery(hass): """Test a bridge being discovered.""" flow = config_flow.DeconzFlowHandler() flow.hass = hass + result = await flow.async_step_discovery({ 'host': '1.2.3.4', 'port': 80, @@ -222,14 +243,18 @@ async def test_import_with_api_key(hass): async def test_options(hass, aioclient_mock): """Test that options work and that bridgeid can be requested.""" aioclient_mock.get('http://1.2.3.4:80/api/1234567890ABCDEF/config', - json={"bridgeid": "id"}) + json={"bridgeid": "id"}, + headers={'content-type': 'application/json'}) + flow = config_flow.DeconzFlowHandler() flow.hass = hass flow.deconz_config = {'host': '1.2.3.4', 'port': 80, 'api_key': '1234567890ABCDEF'} + result = await flow.async_step_options( user_input={'allow_clip_sensor': False, 'allow_deconz_groups': False}) + assert result['type'] == 'create_entry' assert result['title'] == 'deCONZ-id' assert result['data'] == { diff --git a/tests/components/deconz/test_gateway.py b/tests/components/deconz/test_gateway.py index d73f225b2ac..6006ff66898 100644 --- a/tests/components/deconz/test_gateway.py +++ b/tests/components/deconz/test_gateway.py @@ -4,10 +4,13 @@ from unittest.mock import Mock, patch import pytest from homeassistant.exceptions import ConfigEntryNotReady -from homeassistant.components.deconz import gateway +from homeassistant.components.deconz import errors, gateway from tests.common import mock_coro +import pydeconz + + ENTRY_CONFIG = { "host": "1.2.3.4", "port": 80, @@ -62,11 +65,25 @@ async def test_gateway_retry(): deconz_gateway = gateway.DeconzGateway(hass, entry) with patch.object( - gateway, 'get_gateway', return_value=mock_coro(False) - ), pytest.raises(ConfigEntryNotReady): + gateway, 'get_gateway', side_effect=errors.CannotConnect), \ + pytest.raises(ConfigEntryNotReady): await deconz_gateway.async_setup() +async def test_gateway_setup_fails(): + """Retry setup.""" + hass = Mock() + entry = Mock() + entry.data = ENTRY_CONFIG + + deconz_gateway = gateway.DeconzGateway(hass, entry) + + with patch.object(gateway, 'get_gateway', side_effect=Exception): + result = await deconz_gateway.async_setup() + + assert not result + + async def test_connection_status(hass): """Make sure that connection status triggers a dispatcher send.""" entry = Mock() @@ -170,10 +187,20 @@ async def test_get_gateway(hass): assert await gateway.get_gateway(hass, ENTRY_CONFIG, Mock(), Mock()) -async def test_get_gateway_fails(hass): +async def test_get_gateway_fails_unauthorized(hass): """Failed call.""" with patch('pydeconz.DeconzSession.async_load_parameters', - return_value=mock_coro(False)): + side_effect=pydeconz.errors.Unauthorized), \ + pytest.raises(errors.AuthenticationRequired): + assert await gateway.get_gateway( + hass, ENTRY_CONFIG, Mock(), Mock()) is False + + +async def test_get_gateway_fails_cannot_connect(hass): + """Failed call.""" + with patch('pydeconz.DeconzSession.async_load_parameters', + side_effect=pydeconz.errors.RequestError), \ + pytest.raises(errors.CannotConnect): assert await gateway.get_gateway( hass, ENTRY_CONFIG, Mock(), Mock()) is False diff --git a/tests/components/deconz/test_init.py b/tests/components/deconz/test_init.py index cbba47eb431..e0afadccc81 100644 --- a/tests/components/deconz/test_init.py +++ b/tests/components/deconz/test_init.py @@ -1,6 +1,7 @@ """Test deCONZ component setup process.""" from unittest.mock import Mock, patch +import asyncio import pytest import voluptuous as vol @@ -76,13 +77,22 @@ async def test_setup_entry_already_registered_bridge(hass): assert await deconz.async_setup_entry(hass, {}) is False +async def test_setup_entry_fails(hass): + """Test setup entry fails if deCONZ is not available.""" + entry = Mock() + entry.data = {'host': '1.2.3.4', 'port': 80, 'api_key': '1234567890ABCDEF'} + with patch('pydeconz.DeconzSession.async_load_parameters', + side_effect=Exception): + await deconz.async_setup_entry(hass, entry) + + async def test_setup_entry_no_available_bridge(hass): """Test setup entry fails if deCONZ is not available.""" entry = Mock() entry.data = {'host': '1.2.3.4', 'port': 80, 'api_key': '1234567890ABCDEF'} with patch( 'pydeconz.DeconzSession.async_load_parameters', - return_value=mock_coro(False) + side_effect=asyncio.TimeoutError ), pytest.raises(ConfigEntryNotReady): await deconz.async_setup_entry(hass, entry) @@ -185,6 +195,7 @@ async def test_service_refresh_devices(hass): }) entry.add_to_hass(hass) mock_registry = Mock() + with patch.object(deconz, 'DeconzGateway') as mock_gateway, \ patch('homeassistant.helpers.device_registry.async_get_registry', return_value=mock_coro(mock_registry)): @@ -196,6 +207,7 @@ async def test_service_refresh_devices(hass): await hass.services.async_call( 'deconz', 'device_refresh', service_data={}) await hass.async_block_till_done() + with patch.object(hass.data[deconz.DOMAIN].api, 'async_load_parameters', return_value=mock_coro(False)): await hass.services.async_call(