From 13cc6718445b7fb35c3e8f95cc7e4f73c2de40bc Mon Sep 17 00:00:00 2001 From: Michael <35783820+mib1185@users.noreply.github.com> Date: Sat, 28 Aug 2021 21:11:51 +0200 Subject: [PATCH] Re-configuration possibilities for Synology DSM (#53285) * add automated host/ip reconfig via SSDP * add reconfig of existing entry * adjust tests * adjust tests again * use self._async_current_entries() * _async_get_existing_entry() * apply suggestions --- .../components/synology_dsm/config_flow.py | 46 ++++--- .../components/synology_dsm/strings.json | 3 +- .../synology_dsm/translations/de.json | 3 +- .../synology_dsm/translations/en.json | 3 +- .../synology_dsm/test_config_flow.py | 112 ++++++++++++------ 5 files changed, 109 insertions(+), 58 deletions(-) diff --git a/homeassistant/components/synology_dsm/config_flow.py b/homeassistant/components/synology_dsm/config_flow.py index 97f9e4343fa..003af39ecdf 100644 --- a/homeassistant/components/synology_dsm/config_flow.py +++ b/homeassistant/components/synology_dsm/config_flow.py @@ -228,14 +228,14 @@ class SynologyDSMFlowHandler(ConfigFlow, domain=DOMAIN): if user_input.get(CONF_VOLUMES): config_data[CONF_VOLUMES] = user_input[CONF_VOLUMES] - if existing_entry and self.reauth_conf: + if existing_entry: self.hass.config_entries.async_update_entry( existing_entry, data=config_data ) await self.hass.config_entries.async_reload(existing_entry.entry_id) - return self.async_abort(reason="reauth_successful") - if existing_entry: - return self.async_abort(reason="already_configured") + if self.reauth_conf: + return self.async_abort(reason="reauth_successful") + return self.async_abort(reason="reconfigure_successful") return self.async_create_entry(title=host, data=config_data) @@ -246,14 +246,26 @@ class SynologyDSMFlowHandler(ConfigFlow, domain=DOMAIN): discovery_info[ssdp.ATTR_UPNP_FRIENDLY_NAME].split("(", 1)[0].strip() ) - mac = discovery_info[ssdp.ATTR_UPNP_SERIAL].upper() + discovered_mac = discovery_info[ssdp.ATTR_UPNP_SERIAL].upper() # Synology NAS can broadcast on multiple IP addresses, since they can be connected to multiple ethernets. # The serial of the NAS is actually its MAC address. - if self._mac_already_configured(mac): - return self.async_abort(reason="already_configured") - await self.async_set_unique_id(mac) - self._abort_if_unique_id_configured() + existing_entry = self._async_get_existing_entry(discovered_mac) + + if existing_entry and existing_entry.data[CONF_HOST] != parsed_url.hostname: + _LOGGER.debug( + "Update host from '%s' to '%s' for NAS '%s' via SSDP discovery", + existing_entry.data[CONF_HOST], + parsed_url.hostname, + existing_entry.unique_id, + ) + self.hass.config_entries.async_update_entry( + existing_entry, + data={**existing_entry.data, CONF_HOST: parsed_url.hostname}, + ) + return self.async_abort(reason="reconfigure_successful") + if existing_entry: + return self.async_abort(reason="already_configured") self.discovered_conf = { CONF_NAME: friendly_name, @@ -295,14 +307,14 @@ class SynologyDSMFlowHandler(ConfigFlow, domain=DOMAIN): return await self.async_step_user(user_input) - def _mac_already_configured(self, mac: str) -> bool: - """See if we already have configured a NAS with this MAC address.""" - existing_macs = [ - mac.replace("-", "") - for entry in self._async_current_entries() - for mac in entry.data.get(CONF_MAC, []) - ] - return mac in existing_macs + def _async_get_existing_entry(self, discovered_mac: str) -> ConfigEntry | None: + """See if we already have a configured NAS with this MAC address.""" + for entry in self._async_current_entries(): + if discovered_mac in [ + mac.replace("-", "") for mac in entry.data.get(CONF_MAC, []) + ]: + return entry + return None class SynologyDSMOptionsFlowHandler(OptionsFlow): diff --git a/homeassistant/components/synology_dsm/strings.json b/homeassistant/components/synology_dsm/strings.json index 6baaaaef9f6..9a7500e08bc 100644 --- a/homeassistant/components/synology_dsm/strings.json +++ b/homeassistant/components/synology_dsm/strings.json @@ -48,7 +48,8 @@ }, "abort": { "already_configured": "[%key:common::config_flow::abort::already_configured_device%]", - "reauth_successful": "[%key:common::config_flow::abort::reauth_successful%]" + "reauth_successful": "[%key:common::config_flow::abort::reauth_successful%]", + "reconfigure_successful": "Re-configuration was successful" } }, "options": { diff --git a/homeassistant/components/synology_dsm/translations/de.json b/homeassistant/components/synology_dsm/translations/de.json index 86c154e8567..aca87d46d70 100644 --- a/homeassistant/components/synology_dsm/translations/de.json +++ b/homeassistant/components/synology_dsm/translations/de.json @@ -2,7 +2,8 @@ "config": { "abort": { "already_configured": "Ger\u00e4t ist bereits konfiguriert", - "reauth_successful": "Die erneute Authentifizierung war erfolgreich" + "reauth_successful": "Die erneute Authentifizierung war erfolgreich", + "reconfigure_successful": "Die Anpassung der Konfiguration war erfolgreich" }, "error": { "cannot_connect": "Verbindung fehlgeschlagen", diff --git a/homeassistant/components/synology_dsm/translations/en.json b/homeassistant/components/synology_dsm/translations/en.json index 0231f8ddb3c..15384a1690e 100644 --- a/homeassistant/components/synology_dsm/translations/en.json +++ b/homeassistant/components/synology_dsm/translations/en.json @@ -2,7 +2,8 @@ "config": { "abort": { "already_configured": "Device is already configured", - "reauth_successful": "Re-authentication was successful" + "reauth_successful": "Re-authentication was successful", + "reconfigure_successful": "Re-configuration was successful" }, "error": { "cannot_connect": "Failed to connect", diff --git a/tests/components/synology_dsm/test_config_flow.py b/tests/components/synology_dsm/test_config_flow.py index cf043c2ce5f..34434eb4a43 100644 --- a/tests/components/synology_dsm/test_config_flow.py +++ b/tests/components/synology_dsm/test_config_flow.py @@ -305,22 +305,29 @@ async def test_reauth(hass: HomeAssistant, service: MagicMock): assert result["reason"] == "reauth_successful" -async def test_abort_if_already_setup(hass: HomeAssistant, service: MagicMock): - """Test we abort if the account is already setup.""" +async def test_reconfig_user(hass: HomeAssistant, service: MagicMock): + """Test re-configuration of already existing entry by user.""" MockConfigEntry( domain=DOMAIN, - data={CONF_HOST: HOST, CONF_USERNAME: USERNAME, CONF_PASSWORD: PASSWORD}, + data={ + CONF_HOST: "wrong_host", + CONF_USERNAME: USERNAME, + CONF_PASSWORD: PASSWORD, + }, unique_id=SERIAL, ).add_to_hass(hass) - # Should fail, same HOST:PORT (flow) - result = await hass.config_entries.flow.async_init( - DOMAIN, - context={"source": SOURCE_USER}, - data={CONF_HOST: HOST, CONF_USERNAME: USERNAME, CONF_PASSWORD: PASSWORD}, - ) - assert result["type"] == data_entry_flow.RESULT_TYPE_ABORT - assert result["reason"] == "already_configured" + with patch( + "homeassistant.config_entries.ConfigEntries.async_reload", + return_value=True, + ): + result = await hass.config_entries.flow.async_init( + DOMAIN, + context={"source": SOURCE_USER}, + data={CONF_HOST: HOST, CONF_USERNAME: USERNAME, CONF_PASSWORD: PASSWORD}, + ) + assert result["type"] == data_entry_flow.RESULT_TYPE_ABORT + assert result["reason"] == "reconfigure_successful" async def test_login_failed(hass: HomeAssistant, service: MagicMock): @@ -379,33 +386,6 @@ async def test_missing_data_after_login(hass: HomeAssistant, service_failed: Mag assert result["errors"] == {"base": "missing_data"} -async def test_form_ssdp_already_configured(hass: HomeAssistant, service: MagicMock): - """Test ssdp abort when the serial number is already configured.""" - await setup.async_setup_component(hass, "persistent_notification", {}) - - MockConfigEntry( - domain=DOMAIN, - data={ - CONF_HOST: HOST, - CONF_USERNAME: USERNAME, - CONF_PASSWORD: PASSWORD, - CONF_MAC: MACS, - }, - unique_id=SERIAL, - ).add_to_hass(hass) - - result = await hass.config_entries.flow.async_init( - DOMAIN, - context={"source": SOURCE_SSDP}, - data={ - ssdp.ATTR_SSDP_LOCATION: "http://192.168.1.5:5000", - ssdp.ATTR_UPNP_FRIENDLY_NAME: "mydsm", - ssdp.ATTR_UPNP_SERIAL: "001132XXXX59", # Existing in MACS[0], but SSDP does not have `-` - }, - ) - assert result["type"] == data_entry_flow.RESULT_TYPE_ABORT - - async def test_form_ssdp(hass: HomeAssistant, service: MagicMock): """Test we can setup from ssdp.""" await setup.async_setup_component(hass, "persistent_notification", {}) @@ -442,6 +422,62 @@ async def test_form_ssdp(hass: HomeAssistant, service: MagicMock): assert result["data"].get(CONF_VOLUMES) is None +async def test_reconfig_ssdp(hass: HomeAssistant, service: MagicMock): + """Test re-configuration of already existing entry by ssdp.""" + await setup.async_setup_component(hass, "persistent_notification", {}) + + MockConfigEntry( + domain=DOMAIN, + data={ + CONF_HOST: "wrong_host", + CONF_USERNAME: USERNAME, + CONF_PASSWORD: PASSWORD, + CONF_MAC: MACS, + }, + unique_id=SERIAL, + ).add_to_hass(hass) + + result = await hass.config_entries.flow.async_init( + DOMAIN, + context={"source": SOURCE_SSDP}, + data={ + ssdp.ATTR_SSDP_LOCATION: "http://192.168.1.5:5000", + ssdp.ATTR_UPNP_FRIENDLY_NAME: "mydsm", + ssdp.ATTR_UPNP_SERIAL: "001132XXXX59", # Existing in MACS[0], but SSDP does not have `-` + }, + ) + assert result["type"] == data_entry_flow.RESULT_TYPE_ABORT + assert result["reason"] == "reconfigure_successful" + + +async def test_existing_ssdp(hass: HomeAssistant, service: MagicMock): + """Test abort of already existing entry by ssdp.""" + await setup.async_setup_component(hass, "persistent_notification", {}) + + MockConfigEntry( + domain=DOMAIN, + data={ + CONF_HOST: "192.168.1.5", + CONF_USERNAME: USERNAME, + CONF_PASSWORD: PASSWORD, + CONF_MAC: MACS, + }, + unique_id=SERIAL, + ).add_to_hass(hass) + + result = await hass.config_entries.flow.async_init( + DOMAIN, + context={"source": SOURCE_SSDP}, + data={ + ssdp.ATTR_SSDP_LOCATION: "http://192.168.1.5:5000", + ssdp.ATTR_UPNP_FRIENDLY_NAME: "mydsm", + ssdp.ATTR_UPNP_SERIAL: "001132XXXX59", # Existing in MACS[0], but SSDP does not have `-` + }, + ) + assert result["type"] == data_entry_flow.RESULT_TYPE_ABORT + assert result["reason"] == "already_configured" + + async def test_options_flow(hass: HomeAssistant, service: MagicMock): """Test config flow options.""" config_entry = MockConfigEntry(