From 94c5dbfd161aa4c4ae8f18934e7b1cc5770963cf Mon Sep 17 00:00:00 2001 From: Michael <35783820+mib1185@users.noreply.github.com> Date: Wed, 30 Mar 2022 01:53:53 +0200 Subject: [PATCH] Rework config flow in AVM Fritz!Tools (#67767) --- homeassistant/components/fritz/__init__.py | 6 +- homeassistant/components/fritz/config_flow.py | 62 +++++++++---------- tests/components/fritz/test_config_flow.py | 58 +++++++++-------- 3 files changed, 60 insertions(+), 66 deletions(-) diff --git a/homeassistant/components/fritz/__init__.py b/homeassistant/components/fritz/__init__.py index 0b334ff616a..55253fe3d1e 100644 --- a/homeassistant/components/fritz/__init__.py +++ b/homeassistant/components/fritz/__init__.py @@ -1,7 +1,7 @@ """Support for AVM Fritz!Box functions.""" import logging -from fritzconnection.core.exceptions import FritzSecurityError +from fritzconnection.core.exceptions import FritzConnectionException from homeassistant.config_entries import ConfigEntry from homeassistant.const import CONF_HOST, CONF_PASSWORD, CONF_PORT, CONF_USERNAME @@ -28,10 +28,10 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: try: await avm_wrapper.async_setup(entry.options) - except FritzSecurityError as ex: - raise ConfigEntryAuthFailed from ex except FRITZ_EXCEPTIONS as ex: raise ConfigEntryNotReady from ex + except FritzConnectionException as ex: + raise ConfigEntryAuthFailed from ex if ( "X_AVM-DE_UPnP1" in avm_wrapper.connection.services diff --git a/homeassistant/components/fritz/config_flow.py b/homeassistant/components/fritz/config_flow.py index 046f00ba3a9..17457522bb0 100644 --- a/homeassistant/components/fritz/config_flow.py +++ b/homeassistant/components/fritz/config_flow.py @@ -6,6 +6,7 @@ import socket from typing import Any from urllib.parse import ParseResult, urlparse +from fritzconnection import FritzConnection from fritzconnection.core.exceptions import FritzConnectionException, FritzSecurityError import voluptuous as vol @@ -19,7 +20,6 @@ from homeassistant.const import CONF_HOST, CONF_PASSWORD, CONF_PORT, CONF_USERNA from homeassistant.core import callback from homeassistant.data_entry_flow import FlowResult -from .common import AvmWrapper from .const import ( CONF_OLD_DISCOVERY, DEFAULT_CONF_OLD_DISCOVERY, @@ -49,29 +49,25 @@ class FritzBoxToolsFlowHandler(ConfigFlow, domain=DOMAIN): def __init__(self) -> None: """Initialize FRITZ!Box Tools flow.""" self._host: str | None = None - self._entry: ConfigEntry - self._name: str - self._password: str + self._entry: ConfigEntry | None = None + self._name: str = "" + self._password: str = "" self._port: int | None = None - self._username: str - self.avm_wrapper: AvmWrapper + self._username: str = "" + self._model: str = "" - async def fritz_tools_init(self) -> str | None: + def fritz_tools_init(self) -> str | None: """Initialize FRITZ!Box Tools class.""" - if not self._host or not self._port: - return None - - self.avm_wrapper = AvmWrapper( - hass=self.hass, - host=self._host, - port=self._port, - username=self._username, - password=self._password, - ) - try: - await self.avm_wrapper.async_setup() + connection = FritzConnection( + address=self._host, + port=self._port, + user=self._username, + password=self._password, + timeout=60.0, + pool_maxsize=30, + ) except FritzSecurityError: return ERROR_AUTH_INVALID except FritzConnectionException: @@ -80,9 +76,11 @@ class FritzBoxToolsFlowHandler(ConfigFlow, domain=DOMAIN): _LOGGER.exception("Unexpected exception") return ERROR_UNKNOWN + self._model = connection.call_action("DeviceInfo:1", "GetInfo")["NewModelName"] + if ( - "X_AVM-DE_UPnP1" in self.avm_wrapper.connection.services - and not (await self.avm_wrapper.async_get_upnp_configuration())["NewEnable"] + "X_AVM-DE_UPnP1" in connection.services + and not connection.call_action("X_AVM-DE_UPnP1", "GetInfo")["NewEnable"] ): return ERROR_UPNP_NOT_CONFIGURED @@ -109,10 +107,10 @@ class FritzBoxToolsFlowHandler(ConfigFlow, domain=DOMAIN): return self.async_create_entry( title=self._name, data={ - CONF_HOST: self.avm_wrapper.host, - CONF_PASSWORD: self.avm_wrapper.password, - CONF_PORT: self.avm_wrapper.port, - CONF_USERNAME: self.avm_wrapper.username, + CONF_HOST: self._host, + CONF_PASSWORD: self._password, + CONF_PORT: self._port, + CONF_USERNAME: self._username, }, options={ CONF_CONSIDER_HOME: DEFAULT_CONSIDER_HOME.total_seconds(), @@ -163,7 +161,7 @@ class FritzBoxToolsFlowHandler(ConfigFlow, domain=DOMAIN): self._username = user_input[CONF_USERNAME] self._password = user_input[CONF_PASSWORD] - error = await self.fritz_tools_init() + error = await self.hass.async_add_executor_job(self.fritz_tools_init) if error: errors["base"] = error @@ -213,8 +211,8 @@ class FritzBoxToolsFlowHandler(ConfigFlow, domain=DOMAIN): self._username = user_input[CONF_USERNAME] self._password = user_input[CONF_PASSWORD] - if not (error := await self.fritz_tools_init()): - self._name = self.avm_wrapper.model + if not (error := await self.hass.async_add_executor_job(self.fritz_tools_init)): + self._name = self._model if await self.async_check_configured_entry(): error = "already_configured" @@ -226,10 +224,7 @@ class FritzBoxToolsFlowHandler(ConfigFlow, domain=DOMAIN): async def async_step_reauth(self, data: dict[str, Any]) -> FlowResult: """Handle flow upon an API authentication error.""" - if cfg_entry := self.hass.config_entries.async_get_entry( - self.context["entry_id"] - ): - self._entry = cfg_entry + self._entry = self.hass.config_entries.async_get_entry(self.context["entry_id"]) self._host = data[CONF_HOST] self._port = data[CONF_PORT] self._username = data[CONF_USERNAME] @@ -265,11 +260,12 @@ class FritzBoxToolsFlowHandler(ConfigFlow, domain=DOMAIN): self._username = user_input[CONF_USERNAME] self._password = user_input[CONF_PASSWORD] - if error := await self.fritz_tools_init(): + if error := await self.hass.async_add_executor_job(self.fritz_tools_init): return self._show_setup_form_reauth_confirm( user_input=user_input, errors={"base": error} ) + assert isinstance(self._entry, ConfigEntry) self.hass.config_entries.async_update_entry( self._entry, data={ diff --git a/tests/components/fritz/test_config_flow.py b/tests/components/fritz/test_config_flow.py index 502757c2c67..6d014782842 100644 --- a/tests/components/fritz/test_config_flow.py +++ b/tests/components/fritz/test_config_flow.py @@ -38,9 +38,9 @@ from tests.common import MockConfigEntry async def test_user(hass: HomeAssistant, fc_class_mock, mock_get_source_ip): """Test starting a flow by user.""" with patch( - "homeassistant.components.fritz.common.FritzConnection", + "homeassistant.components.fritz.config_flow.FritzConnection", side_effect=fc_class_mock, - ), patch("homeassistant.components.fritz.common.FritzStatus"), patch( + ), patch( "homeassistant.components.fritz.common.FritzBoxTools._update_device_info", return_value=MOCK_FIRMWARE_INFO, ), patch( @@ -91,9 +91,9 @@ async def test_user_already_configured( mock_config.add_to_hass(hass) with patch( - "homeassistant.components.fritz.common.FritzConnection", + "homeassistant.components.fritz.config_flow.FritzConnection", side_effect=fc_class_mock, - ), patch("homeassistant.components.fritz.common.FritzStatus"), patch( + ), patch( "homeassistant.components.fritz.common.FritzBoxTools._update_device_info", return_value=MOCK_FIRMWARE_INFO, ), patch( @@ -134,9 +134,9 @@ async def test_exception_security(hass: HomeAssistant, mock_get_source_ip): assert result["step_id"] == "user" with patch( - "homeassistant.components.fritz.common.FritzConnection", + "homeassistant.components.fritz.config_flow.FritzConnection", side_effect=FritzSecurityError, - ), patch("homeassistant.components.fritz.common.FritzStatus"): + ): result = await hass.config_entries.flow.async_configure( result["flow_id"], user_input=MOCK_USER_DATA @@ -157,9 +157,9 @@ async def test_exception_connection(hass: HomeAssistant, mock_get_source_ip): assert result["step_id"] == "user" with patch( - "homeassistant.components.fritz.common.FritzConnection", + "homeassistant.components.fritz.config_flow.FritzConnection", side_effect=FritzConnectionException, - ), patch("homeassistant.components.fritz.common.FritzStatus"): + ): result = await hass.config_entries.flow.async_configure( result["flow_id"], user_input=MOCK_USER_DATA @@ -180,9 +180,9 @@ async def test_exception_unknown(hass: HomeAssistant, mock_get_source_ip): assert result["step_id"] == "user" with patch( - "homeassistant.components.fritz.common.FritzConnection", + "homeassistant.components.fritz.config_flow.FritzConnection", side_effect=OSError, - ), patch("homeassistant.components.fritz.common.FritzStatus"): + ): result = await hass.config_entries.flow.async_configure( result["flow_id"], user_input=MOCK_USER_DATA @@ -202,9 +202,9 @@ async def test_reauth_successful( mock_config.add_to_hass(hass) with patch( - "homeassistant.components.fritz.common.FritzConnection", + "homeassistant.components.fritz.config_flow.FritzConnection", side_effect=fc_class_mock, - ), patch("homeassistant.components.fritz.common.FritzStatus"), patch( + ), patch( "homeassistant.components.fritz.common.FritzBoxTools._update_device_info", return_value=MOCK_FIRMWARE_INFO, ), patch( @@ -252,9 +252,9 @@ async def test_reauth_not_successful( mock_config.add_to_hass(hass) with patch( - "homeassistant.components.fritz.common.FritzConnection", + "homeassistant.components.fritz.config_flow.FritzConnection", side_effect=FritzConnectionException, - ), patch("homeassistant.components.fritz.common.FritzStatus"): + ): result = await hass.config_entries.flow.async_init( DOMAIN, @@ -291,9 +291,9 @@ async def test_ssdp_already_configured( mock_config.add_to_hass(hass) with patch( - "homeassistant.components.fritz.common.FritzConnection", + "homeassistant.components.fritz.config_flow.FritzConnection", side_effect=fc_class_mock, - ), patch("homeassistant.components.fritz.common.FritzStatus"), patch( + ), patch( "homeassistant.components.fritz.config_flow.socket.gethostbyname", return_value=MOCK_IPS["fritz.box"], ): @@ -318,9 +318,9 @@ async def test_ssdp_already_configured_host( mock_config.add_to_hass(hass) with patch( - "homeassistant.components.fritz.common.FritzConnection", + "homeassistant.components.fritz.config_flow.FritzConnection", side_effect=fc_class_mock, - ), patch("homeassistant.components.fritz.common.FritzStatus"), patch( + ), patch( "homeassistant.components.fritz.config_flow.socket.gethostbyname", return_value=MOCK_IPS["fritz.box"], ): @@ -345,9 +345,9 @@ async def test_ssdp_already_configured_host_uuid( mock_config.add_to_hass(hass) with patch( - "homeassistant.components.fritz.common.FritzConnection", + "homeassistant.components.fritz.config_flow.FritzConnection", side_effect=fc_class_mock, - ), patch("homeassistant.components.fritz.common.FritzStatus"), patch( + ), patch( "homeassistant.components.fritz.config_flow.socket.gethostbyname", return_value=MOCK_IPS["fritz.box"], ): @@ -364,9 +364,9 @@ async def test_ssdp_already_in_progress_host( ): """Test starting a flow from discovery twice.""" with patch( - "homeassistant.components.fritz.common.FritzConnection", + "homeassistant.components.fritz.config_flow.FritzConnection", side_effect=fc_class_mock, - ), patch("homeassistant.components.fritz.common.FritzStatus"): + ): result = await hass.config_entries.flow.async_init( DOMAIN, context={"source": SOURCE_SSDP}, data=MOCK_SSDP_DATA @@ -387,9 +387,9 @@ async def test_ssdp_already_in_progress_host( async def test_ssdp(hass: HomeAssistant, fc_class_mock, mock_get_source_ip): """Test starting a flow from discovery.""" with patch( - "homeassistant.components.fritz.common.FritzConnection", + "homeassistant.components.fritz.config_flow.FritzConnection", side_effect=fc_class_mock, - ), patch("homeassistant.components.fritz.common.FritzStatus"), patch( + ), patch( "homeassistant.components.fritz.common.FritzBoxTools._update_device_info", return_value=MOCK_FIRMWARE_INFO, ), patch( @@ -430,9 +430,9 @@ async def test_ssdp(hass: HomeAssistant, fc_class_mock, mock_get_source_ip): async def test_ssdp_exception(hass: HomeAssistant, mock_get_source_ip): """Test starting a flow from discovery but no device found.""" with patch( - "homeassistant.components.fritz.common.FritzConnection", + "homeassistant.components.fritz.config_flow.FritzConnection", side_effect=FritzConnectionException, - ), patch("homeassistant.components.fritz.common.FritzStatus"): + ): result = await hass.config_entries.flow.async_init( DOMAIN, context={"source": SOURCE_SSDP}, data=MOCK_SSDP_DATA @@ -459,11 +459,9 @@ async def test_options_flow(hass: HomeAssistant, fc_class_mock, mock_get_source_ mock_config.add_to_hass(hass) with patch( - "homeassistant.components.fritz.common.FritzConnection", + "homeassistant.components.fritz.config_flow.FritzConnection", side_effect=fc_class_mock, - ), patch("homeassistant.components.fritz.common.FritzStatus"), patch( - "homeassistant.components.fritz.common.FritzBoxTools" - ): + ), patch("homeassistant.components.fritz.common.FritzBoxTools"): result = await hass.config_entries.options.async_init(mock_config.entry_id) assert result["type"] == RESULT_TYPE_FORM assert result["step_id"] == "init"