From 56657fa859369b30b3fe6930ecd518cc079c5cc5 Mon Sep 17 00:00:00 2001 From: Robert Svensson Date: Thu, 30 Jan 2020 22:20:30 +0100 Subject: [PATCH] Axis - config flow use new helper functions (#31286) * Make use of new config flow helpers Simplify Axis entry config to work with config flow helpers * Keep old device data for rollback purposes --- homeassistant/components/axis/__init__.py | 33 ++++++- homeassistant/components/axis/camera.py | 17 ++-- homeassistant/components/axis/config_flow.py | 92 +++++++++----------- homeassistant/components/axis/device.py | 30 ++++--- homeassistant/components/axis/strings.json | 5 +- tests/components/axis/test_config_flow.py | 63 ++++++++------ tests/components/axis/test_device.py | 27 +++--- tests/components/axis/test_init.py | 46 +++++++++- 8 files changed, 183 insertions(+), 130 deletions(-) diff --git a/homeassistant/components/axis/__init__.py b/homeassistant/components/axis/__init__.py index 5c928aa9f31..5294e30ed6f 100644 --- a/homeassistant/components/axis/__init__.py +++ b/homeassistant/components/axis/__init__.py @@ -1,15 +1,23 @@ """Support for Axis devices.""" +import logging + from homeassistant.const import ( CONF_DEVICE, + CONF_HOST, CONF_MAC, + CONF_PASSWORD, + CONF_PORT, CONF_TRIGGER_TIME, + CONF_USERNAME, EVENT_HOMEASSISTANT_STOP, ) from .const import CONF_CAMERA, CONF_EVENTS, DEFAULT_TRIGGER_TIME, DOMAIN from .device import AxisNetworkDevice, get_device +LOGGER = logging.getLogger(__name__) + async def async_setup(hass, config): """Old way to set up Axis devices.""" @@ -35,7 +43,7 @@ async def async_setup_entry(hass, config_entry): config_entry, unique_id=device.api.vapix.params.system_serialnumber ) - hass.data[DOMAIN][device.serial] = device + hass.data[DOMAIN][config_entry.unique_id] = device await device.async_update_device_registry() @@ -52,7 +60,13 @@ async def async_unload_entry(hass, config_entry): async def async_populate_options(hass, config_entry): """Populate default options for device.""" - device = await get_device(hass, config_entry.data[CONF_DEVICE]) + device = await get_device( + hass, + host=config_entry.data[CONF_HOST], + port=config_entry.data[CONF_PORT], + username=config_entry.data[CONF_USERNAME], + password=config_entry.data[CONF_PASSWORD], + ) supported_formats = device.vapix.params.image_format camera = bool(supported_formats) @@ -64,3 +78,18 @@ async def async_populate_options(hass, config_entry): } hass.config_entries.async_update_entry(config_entry, options=options) + + +async def async_migrate_entry(hass, config_entry): + """Migrate old entry.""" + LOGGER.debug("Migrating from version %s", config_entry.version) + + # Flatten configuration but keep old data if user rollbacks HASS + if config_entry.version == 1: + config_entry.data = {**config_entry.data, **config_entry.data[CONF_DEVICE]} + + config_entry.version = 2 + + LOGGER.info("Migration to version %s successful", config_entry.version) + + return True diff --git a/homeassistant/components/axis/camera.py b/homeassistant/components/axis/camera.py index 6b82c938a99..51d6b6805cc 100644 --- a/homeassistant/components/axis/camera.py +++ b/homeassistant/components/axis/camera.py @@ -9,7 +9,6 @@ from homeassistant.components.mjpeg.camera import ( ) from homeassistant.const import ( CONF_AUTHENTICATION, - CONF_DEVICE, CONF_HOST, CONF_NAME, CONF_PASSWORD, @@ -35,15 +34,13 @@ async def async_setup_entry(hass, config_entry, async_add_entities): config = { CONF_NAME: config_entry.data[CONF_NAME], - CONF_USERNAME: config_entry.data[CONF_DEVICE][CONF_USERNAME], - CONF_PASSWORD: config_entry.data[CONF_DEVICE][CONF_PASSWORD], + CONF_USERNAME: config_entry.data[CONF_USERNAME], + CONF_PASSWORD: config_entry.data[CONF_PASSWORD], CONF_MJPEG_URL: AXIS_VIDEO.format( - config_entry.data[CONF_DEVICE][CONF_HOST], - config_entry.data[CONF_DEVICE][CONF_PORT], + config_entry.data[CONF_HOST], config_entry.data[CONF_PORT], ), CONF_STILL_IMAGE_URL: AXIS_IMAGE.format( - config_entry.data[CONF_DEVICE][CONF_HOST], - config_entry.data[CONF_DEVICE][CONF_PORT], + config_entry.data[CONF_HOST], config_entry.data[CONF_PORT], ), CONF_AUTHENTICATION: HTTP_DIGEST_AUTHENTICATION, } @@ -76,14 +73,14 @@ class AxisCamera(AxisEntityBase, MjpegCamera): async def stream_source(self): """Return the stream source.""" return AXIS_STREAM.format( - self.device.config_entry.data[CONF_DEVICE][CONF_USERNAME], - self.device.config_entry.data[CONF_DEVICE][CONF_PASSWORD], + self.device.config_entry.data[CONF_USERNAME], + self.device.config_entry.data[CONF_PASSWORD], self.device.host, ) def _new_address(self): """Set new device address for video stream.""" - port = self.device.config_entry.data[CONF_DEVICE][CONF_PORT] + port = self.device.config_entry.data[CONF_PORT] self._mjpeg_url = AXIS_VIDEO.format(self.device.host, port) self._still_image_url = AXIS_IMAGE.format(self.device.host, port) diff --git a/homeassistant/components/axis/config_flow.py b/homeassistant/components/axis/config_flow.py index 88c1cab98c1..29658c19c5b 100644 --- a/homeassistant/components/axis/config_flow.py +++ b/homeassistant/components/axis/config_flow.py @@ -4,7 +4,6 @@ import voluptuous as vol from homeassistant import config_entries from homeassistant.const import ( - CONF_DEVICE, CONF_HOST, CONF_MAC, CONF_NAME, @@ -33,16 +32,12 @@ DEFAULT_PORT = 80 class AxisFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): """Handle a Axis config flow.""" - VERSION = 1 + VERSION = 2 CONNECTION_CLASS = config_entries.CONN_CLASS_LOCAL_PUSH def __init__(self): """Initialize the Axis config flow.""" self.device_config = {} - self.model = None - self.name = None - self.serial_number = None - self.discovery_schema = {} self.import_schema = {} @@ -55,24 +50,32 @@ class AxisFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): if user_input is not None: try: + device = await get_device( + self.hass, + host=user_input[CONF_HOST], + port=user_input[CONF_PORT], + username=user_input[CONF_USERNAME], + password=user_input[CONF_PASSWORD], + ) + + serial_number = device.vapix.params.system_serialnumber + await self.async_set_unique_id(serial_number) + + self._abort_if_unique_id_configured( + updates={ + CONF_HOST: user_input[CONF_HOST], + CONF_PORT: user_input[CONF_PORT], + } + ) + self.device_config = { CONF_HOST: user_input[CONF_HOST], CONF_PORT: user_input[CONF_PORT], CONF_USERNAME: user_input[CONF_USERNAME], CONF_PASSWORD: user_input[CONF_PASSWORD], + CONF_MAC: serial_number, + CONF_MODEL: device.vapix.params.prodnbr, } - device = await get_device(self.hass, self.device_config) - - self.serial_number = device.vapix.params.system_serialnumber - config_entry = await self.async_set_unique_id(self.serial_number) - if config_entry: - return self._update_entry( - config_entry, - host=user_input[CONF_HOST], - port=user_input[CONF_PORT], - ) - - self.model = device.vapix.params.prodnbr return await self._create_entry() @@ -101,41 +104,23 @@ class AxisFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): Generate a name to be used as a prefix for device entities. """ + model = self.device_config[CONF_MODEL] same_model = [ entry.data[CONF_NAME] for entry in self.hass.config_entries.async_entries(DOMAIN) - if entry.data[CONF_MODEL] == self.model + if entry.data[CONF_MODEL] == model ] - self.name = f"{self.model}" + name = model for idx in range(len(same_model) + 1): - self.name = f"{self.model} {idx}" - if self.name not in same_model: + name = f"{model} {idx}" + if name not in same_model: break - data = { - CONF_DEVICE: self.device_config, - CONF_NAME: self.name, - CONF_MAC: self.serial_number, - CONF_MODEL: self.model, - } + self.device_config[CONF_NAME] = name - title = f"{self.model} - {self.serial_number}" - return self.async_create_entry(title=title, data=data) - - def _update_entry(self, entry, host, port): - """Update existing entry.""" - if ( - entry.data[CONF_DEVICE][CONF_HOST] == host - and entry.data[CONF_DEVICE][CONF_PORT] == port - ): - return self.async_abort(reason="already_configured") - - entry.data[CONF_DEVICE][CONF_HOST] = host - entry.data[CONF_DEVICE][CONF_PORT] = port - - self.hass.config_entries.async_update_entry(entry) - return self.async_abort(reason="updated_configuration") + title = f"{model} - {self.device_config[CONF_MAC]}" + return self.async_create_entry(title=title, data=self.device_config) async def async_step_zeroconf(self, discovery_info): """Prepare configuration for a discovered Axis device.""" @@ -147,18 +132,19 @@ class AxisFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): if discovery_info[CONF_HOST].startswith("169.254"): return self.async_abort(reason="link_local_address") - config_entry = await self.async_set_unique_id(serial_number) - if config_entry: - return self._update_entry( - config_entry, - host=discovery_info[CONF_HOST], - port=discovery_info[CONF_PORT], - ) + await self.async_set_unique_id(serial_number) + + self._abort_if_unique_id_configured( + updates={ + CONF_HOST: discovery_info[CONF_HOST], + CONF_PORT: discovery_info[CONF_PORT], + } + ) # pylint: disable=no-member # https://github.com/PyCQA/pylint/issues/3167 self.context["title_placeholders"] = { - "name": discovery_info["hostname"][:-7], - "host": discovery_info[CONF_HOST], + CONF_NAME: discovery_info["hostname"][:-7], + CONF_HOST: discovery_info[CONF_HOST], } self.discovery_schema = { diff --git a/homeassistant/components/axis/device.py b/homeassistant/components/axis/device.py index 85ad59268df..a204136e018 100644 --- a/homeassistant/components/axis/device.py +++ b/homeassistant/components/axis/device.py @@ -7,9 +7,7 @@ import axis from axis.streammanager import SIGNAL_PLAYING from homeassistant.const import ( - CONF_DEVICE, CONF_HOST, - CONF_MAC, CONF_NAME, CONF_PASSWORD, CONF_PORT, @@ -42,7 +40,7 @@ class AxisNetworkDevice: @property def host(self): """Return the host of this device.""" - return self.config_entry.data[CONF_DEVICE][CONF_HOST] + return self.config_entry.data[CONF_HOST] @property def model(self): @@ -75,7 +73,13 @@ class AxisNetworkDevice: async def async_setup(self): """Set up the device.""" try: - self.api = await get_device(self.hass, self.config_entry.data[CONF_DEVICE]) + self.api = await get_device( + self.hass, + host=self.config_entry.data[CONF_HOST], + port=self.config_entry.data[CONF_PORT], + username=self.config_entry.data[CONF_USERNAME], + password=self.config_entry.data[CONF_PASSWORD], + ) except CannotConnect: raise ConfigEntryNotReady @@ -126,7 +130,7 @@ class AxisNetworkDevice: This is a static method because a class method (bound method), can not be used with weak references. """ - device = hass.data[DOMAIN][entry.data[CONF_MAC]] + device = hass.data[DOMAIN][entry.unique_id] device.api.config.host = device.host async_dispatcher_send(hass, device.event_new_address) @@ -197,15 +201,15 @@ class AxisNetworkDevice: return True -async def get_device(hass, config): +async def get_device(hass, host, port, username, password): """Create a Axis device.""" device = axis.AxisDevice( loop=hass.loop, - host=config[CONF_HOST], - username=config[CONF_USERNAME], - password=config[CONF_PASSWORD], - port=config[CONF_PORT], + host=host, + port=port, + username=username, + password=password, web_proto="http", ) @@ -224,13 +228,11 @@ async def get_device(hass, config): return device except axis.Unauthorized: - LOGGER.warning( - "Connected to device at %s but not registered.", config[CONF_HOST] - ) + LOGGER.warning("Connected to device at %s but not registered.", host) raise AuthenticationRequired except (asyncio.TimeoutError, axis.RequestError): - LOGGER.error("Error connecting to the Axis device at %s", config[CONF_HOST]) + LOGGER.error("Error connecting to the Axis device at %s", host) raise CannotConnect except axis.AxisException: diff --git a/homeassistant/components/axis/strings.json b/homeassistant/components/axis/strings.json index 7facd7060ad..b50a5c546b8 100644 --- a/homeassistant/components/axis/strings.json +++ b/homeassistant/components/axis/strings.json @@ -23,8 +23,7 @@ "already_configured": "Device is already configured", "bad_config_file": "Bad data from config file", "link_local_address": "Link local addresses are not supported", - "not_axis_device": "Discovered device not an Axis device", - "updated_configuration": "Updated device configuration with new host address" + "not_axis_device": "Discovered device not an Axis device" } } -} +} \ No newline at end of file diff --git a/tests/components/axis/test_config_flow.py b/tests/components/axis/test_config_flow.py index 809c71c5cb1..2e4c3e9f8be 100644 --- a/tests/components/axis/test_config_flow.py +++ b/tests/components/axis/test_config_flow.py @@ -4,7 +4,7 @@ from unittest.mock import Mock, patch from homeassistant.components import axis from homeassistant.components.axis import config_flow -from .test_device import MAC, setup_axis_integration +from .test_device import MAC, MODEL, NAME, setup_axis_integration from tests.common import MockConfigEntry, mock_coro @@ -54,12 +54,10 @@ async def test_flow_manual_configuration(hass): assert result["type"] == "create_entry" assert result["title"] == f"prodnbr - {MAC}" assert result["data"] == { - axis.CONF_DEVICE: { - config_flow.CONF_HOST: "1.2.3.4", - config_flow.CONF_USERNAME: "user", - config_flow.CONF_PASSWORD: "pass", - config_flow.CONF_PORT: 80, - }, + config_flow.CONF_HOST: "1.2.3.4", + config_flow.CONF_USERNAME: "user", + config_flow.CONF_PASSWORD: "pass", + config_flow.CONF_PORT: 80, config_flow.CONF_MAC: MAC, config_flow.CONF_MODEL: "prodnbr", config_flow.CONF_NAME: "prodnbr 0", @@ -95,11 +93,8 @@ async def test_manual_configuration_update_configuration(hass): ) assert result["type"] == "abort" - assert result["reason"] == "updated_configuration" - assert ( - device.config_entry.data[config_flow.CONF_DEVICE][config_flow.CONF_HOST] - == "2.3.4.5" - ) + assert result["reason"] == "already_configured" + assert device.config_entry.data[config_flow.CONF_HOST] == "2.3.4.5" async def test_flow_fails_already_configured(hass): @@ -223,12 +218,10 @@ async def test_flow_create_entry_multiple_existing_entries_of_same_model(hass): assert result["type"] == "create_entry" assert result["title"] == f"prodnbr - {MAC}" assert result["data"] == { - axis.CONF_DEVICE: { - config_flow.CONF_HOST: "1.2.3.4", - config_flow.CONF_USERNAME: "user", - config_flow.CONF_PASSWORD: "pass", - config_flow.CONF_PORT: 80, - }, + config_flow.CONF_HOST: "1.2.3.4", + config_flow.CONF_USERNAME: "user", + config_flow.CONF_PASSWORD: "pass", + config_flow.CONF_PORT: 80, config_flow.CONF_MAC: MAC, config_flow.CONF_MODEL: "prodnbr", config_flow.CONF_NAME: "prodnbr 2", @@ -271,12 +264,10 @@ async def test_zeroconf_flow(hass): assert result["type"] == "create_entry" assert result["title"] == f"prodnbr - {MAC}" assert result["data"] == { - axis.CONF_DEVICE: { - config_flow.CONF_HOST: "1.2.3.4", - config_flow.CONF_USERNAME: "user", - config_flow.CONF_PASSWORD: "pass", - config_flow.CONF_PORT: 80, - }, + config_flow.CONF_HOST: "1.2.3.4", + config_flow.CONF_USERNAME: "user", + config_flow.CONF_PASSWORD: "pass", + config_flow.CONF_PORT: 80, config_flow.CONF_MAC: MAC, config_flow.CONF_MODEL: "prodnbr", config_flow.CONF_NAME: "prodnbr 0", @@ -310,6 +301,15 @@ async def test_zeroconf_flow_updated_configuration(hass): """Test that zeroconf update configuration with new parameters.""" device = await setup_axis_integration(hass) assert device.host == "1.2.3.4" + assert device.config_entry.data == { + config_flow.CONF_HOST: "1.2.3.4", + config_flow.CONF_PORT: 80, + config_flow.CONF_USERNAME: "username", + config_flow.CONF_PASSWORD: "password", + config_flow.CONF_MAC: MAC, + config_flow.CONF_MODEL: MODEL, + config_flow.CONF_NAME: NAME, + } result = await hass.config_entries.flow.async_init( config_flow.DOMAIN, @@ -323,11 +323,16 @@ async def test_zeroconf_flow_updated_configuration(hass): ) assert result["type"] == "abort" - assert result["reason"] == "updated_configuration" - assert device.host == "2.3.4.5" - assert ( - device.config_entry.data[config_flow.CONF_DEVICE][config_flow.CONF_PORT] == 8080 - ) + assert result["reason"] == "already_configured" + assert device.config_entry.data == { + config_flow.CONF_HOST: "2.3.4.5", + config_flow.CONF_PORT: 8080, + config_flow.CONF_USERNAME: "username", + config_flow.CONF_PASSWORD: "password", + config_flow.CONF_MAC: MAC, + config_flow.CONF_MODEL: MODEL, + config_flow.CONF_NAME: NAME, + } 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 b175d22cfb4..3d2ed432c1c 100644 --- a/tests/components/axis/test_device.py +++ b/tests/components/axis/test_device.py @@ -14,18 +14,14 @@ MAC = "00408C12345" MODEL = "model" NAME = "name" -DEVICE_DATA = { - axis.device.CONF_HOST: "1.2.3.4", - axis.device.CONF_USERNAME: "username", - axis.device.CONF_PASSWORD: "password", - axis.device.CONF_PORT: 80, -} - -ENTRY_OPTIONS = {axis.device.CONF_CAMERA: True, axis.device.CONF_EVENTS: True} +ENTRY_OPTIONS = {axis.CONF_CAMERA: True, axis.CONF_EVENTS: True} ENTRY_CONFIG = { - axis.device.CONF_DEVICE: DEVICE_DATA, - axis.device.CONF_MAC: MAC, + axis.CONF_HOST: "1.2.3.4", + axis.CONF_USERNAME: "username", + axis.CONF_PASSWORD: "password", + axis.CONF_PORT: 80, + axis.CONF_MAC: MAC, axis.device.CONF_MODEL: MODEL, axis.device.CONF_NAME: NAME, } @@ -76,6 +72,7 @@ async def setup_axis_integration( connection_class=config_entries.CONN_CLASS_LOCAL_PUSH, options=deepcopy(options), entry_id="1", + version=2, ) config_entry.add_to_hass(hass) @@ -116,10 +113,10 @@ async def test_device_setup(hass): assert forward_entry_setup.mock_calls[1][1] == (entry, "binary_sensor") assert forward_entry_setup.mock_calls[2][1] == (entry, "switch") - assert device.host == DEVICE_DATA[axis.device.CONF_HOST] + assert device.host == ENTRY_CONFIG[axis.CONF_HOST] assert device.model == ENTRY_CONFIG[axis.device.CONF_MODEL] assert device.name == ENTRY_CONFIG[axis.device.CONF_NAME] - assert device.serial == ENTRY_CONFIG[axis.device.CONF_MAC] + assert device.serial == ENTRY_CONFIG[axis.CONF_MAC] async def test_update_address(hass): @@ -204,7 +201,7 @@ async def test_get_device_fails(hass): with patch( "axis.param_cgi.Params.update_brand", side_effect=axislib.Unauthorized ), pytest.raises(axis.errors.AuthenticationRequired): - await axis.device.get_device(hass, DEVICE_DATA) + await axis.device.get_device(hass, host="", port="", username="", password="") async def test_get_device_device_unavailable(hass): @@ -212,7 +209,7 @@ async def test_get_device_device_unavailable(hass): with patch( "axis.param_cgi.Params.update_brand", side_effect=axislib.RequestError ), pytest.raises(axis.errors.CannotConnect): - await axis.device.get_device(hass, DEVICE_DATA) + await axis.device.get_device(hass, host="", port="", username="", password="") async def test_get_device_unknown_error(hass): @@ -220,4 +217,4 @@ async def test_get_device_unknown_error(hass): with patch( "axis.param_cgi.Params.update_brand", side_effect=axislib.AxisException ), pytest.raises(axis.errors.AuthenticationRequired): - await axis.device.get_device(hass, DEVICE_DATA) + await axis.device.get_device(hass, host="", port="", username="", password="") diff --git a/tests/components/axis/test_init.py b/tests/components/axis/test_init.py index 748bb539369..643ddf58ac0 100644 --- a/tests/components/axis/test_init.py +++ b/tests/components/axis/test_init.py @@ -4,7 +4,7 @@ from unittest.mock import Mock, patch from homeassistant.components import axis from homeassistant.setup import async_setup_component -from .test_device import MAC, setup_axis_integration +from .test_device import ENTRY_CONFIG, MAC, setup_axis_integration from tests.common import MockConfigEntry, mock_coro @@ -16,7 +16,7 @@ async def test_setup_device_already_configured(hass): assert await async_setup_component( hass, axis.DOMAIN, - {axis.DOMAIN: {"device_name": {axis.config_flow.CONF_HOST: "1.2.3.4"}}}, + {axis.DOMAIN: {"device_name": {axis.CONF_HOST: "1.2.3.4"}}}, ) assert not mock_config_entries.flow.mock_calls @@ -38,7 +38,7 @@ async def test_setup_entry(hass): async def test_setup_entry_fails(hass): """Test successful setup of entry.""" entry = MockConfigEntry( - domain=axis.DOMAIN, data={axis.device.CONF_MAC: "0123"}, options=True + domain=axis.DOMAIN, data={axis.CONF_MAC: "0123"}, options=True ) mock_device = Mock() @@ -63,7 +63,7 @@ async def test_unload_entry(hass): async def test_populate_options(hass): """Test successful populate options.""" - entry = MockConfigEntry(domain=axis.DOMAIN, data={"device": {}}) + entry = MockConfigEntry(domain=axis.DOMAIN, data=ENTRY_CONFIG) entry.add_to_hass(hass) with patch.object(axis, "get_device", return_value=mock_coro(Mock())): @@ -75,3 +75,41 @@ async def test_populate_options(hass): axis.CONF_EVENTS: True, axis.CONF_TRIGGER_TIME: axis.DEFAULT_TRIGGER_TIME, } + + +async def test_migrate_entry(hass): + """Test successful migration of entry data.""" + legacy_config = { + axis.CONF_DEVICE: { + axis.CONF_HOST: "1.2.3.4", + axis.CONF_USERNAME: "username", + axis.CONF_PASSWORD: "password", + axis.CONF_PORT: 80, + }, + axis.CONF_MAC: "mac", + axis.device.CONF_MODEL: "model", + axis.device.CONF_NAME: "name", + } + entry = MockConfigEntry(domain=axis.DOMAIN, data=legacy_config) + + assert entry.data == legacy_config + assert entry.version == 1 + + await axis.async_migrate_entry(hass, entry) + + assert entry.data == { + axis.CONF_DEVICE: { + axis.CONF_HOST: "1.2.3.4", + axis.CONF_USERNAME: "username", + axis.CONF_PASSWORD: "password", + axis.CONF_PORT: 80, + }, + axis.CONF_HOST: "1.2.3.4", + axis.CONF_USERNAME: "username", + axis.CONF_PASSWORD: "password", + axis.CONF_PORT: 80, + axis.CONF_MAC: "mac", + axis.device.CONF_MODEL: "model", + axis.device.CONF_NAME: "name", + } + assert entry.version == 2