Avoid supplemental discovery of ignored upnp entry (#53250)

homeassistant/components/upnp/config_flow.py

@@ -166,8 +166,7 @@ class UpnpFlowHandler(config_entries.ConfigFlow, domain=DOMAIN):
         discovery = discovery_info_to_discovery(discovery_info)
 
         # Ensure not already configuring/configured.
-        discovery = await Device.async_supplement_discovery(self.hass, discovery)
+        unique_id = discovery[DISCOVERY_USN]
-        unique_id = discovery[DISCOVERY_UNIQUE_ID]
         await self.async_set_unique_id(unique_id)
         self._abort_if_unique_id_configured(
             updates={CONFIG_ENTRY_HOSTNAME: discovery[DISCOVERY_HOSTNAME]}
@@ -183,6 +182,9 @@ class UpnpFlowHandler(config_entries.ConfigFlow, domain=DOMAIN):
                 )
                 return self.async_abort(reason="discovery_ignored")
 
+        # Get more data about the device.
+        discovery = await Device.async_supplement_discovery(self.hass, discovery)
+
         # Store discovery.
         self._discoveries = [discovery]
 

homeassistant/components/upnp/device.py

@@ -40,11 +40,15 @@ from .const import (
 
 def discovery_info_to_discovery(discovery_info: Mapping) -> Mapping:
     """Convert a SSDP-discovery to 'our' discovery."""
+    location = discovery_info[ssdp.ATTR_SSDP_LOCATION]
+    parsed = urlparse(location)
+    hostname = parsed.hostname
     return {
         DISCOVERY_UDN: discovery_info[ssdp.ATTR_UPNP_UDN],
         DISCOVERY_ST: discovery_info[ssdp.ATTR_SSDP_ST],
         DISCOVERY_LOCATION: discovery_info[ssdp.ATTR_SSDP_LOCATION],
         DISCOVERY_USN: discovery_info[ssdp.ATTR_SSDP_USN],
+        DISCOVERY_HOSTNAME: hostname,
     }
 
 

tests/components/upnp/test_config_flow.py

@@ -2,6 +2,7 @@
 
 from datetime import timedelta
 from unittest.mock import AsyncMock, Mock, patch
+from urllib.parse import urlparse
 
 from homeassistant import config_entries, data_entry_flow
 from homeassistant.components import ssdp
@@ -33,7 +34,7 @@ from tests.common import MockConfigEntry, async_fire_time_changed
 async def test_flow_ssdp_discovery(hass: HomeAssistant):
     """Test config flow: discovered + configured through ssdp."""
     udn = "uuid:device_1"
-    location = "dummy"
+    location = "http://dummy"
     mock_device = MockDevice(udn)
     ssdp_discoveries = [
         {
@@ -93,7 +94,7 @@ async def test_flow_ssdp_discovery(hass: HomeAssistant):
 async def test_flow_ssdp_incomplete_discovery(hass: HomeAssistant):
     """Test config flow: incomplete discovery through ssdp."""
     udn = "uuid:device_1"
-    location = "dummy"
+    location = "http://dummy"
     mock_device = MockDevice(udn)
 
     # Discovered via step ssdp.
@@ -112,9 +113,9 @@ async def test_flow_ssdp_incomplete_discovery(hass: HomeAssistant):
 
 
 async def test_flow_ssdp_discovery_ignored(hass: HomeAssistant):
-    """Test config flow: discovery through ssdp, but ignored."""
+    """Test config flow: discovery through ssdp, but ignored, as hostname is used by existing config entry."""
     udn = "uuid:device_random_1"
-    location = "dummy"
+    location = "http://dummy"
     mock_device = MockDevice(udn)
 
     # Existing entry.
@@ -123,46 +124,31 @@ async def test_flow_ssdp_discovery_ignored(hass: HomeAssistant):
         data={
             CONFIG_ENTRY_UDN: "uuid:device_random_2",
             CONFIG_ENTRY_ST: mock_device.device_type,
-            CONFIG_ENTRY_HOSTNAME: mock_device.hostname,
+            CONFIG_ENTRY_HOSTNAME: urlparse(location).hostname,
         },
         options={CONFIG_ENTRY_SCAN_INTERVAL: DEFAULT_SCAN_INTERVAL},
     )
     config_entry.add_to_hass(hass)
 
-    discoveries = [
+    # Discovered via step ssdp, but ignored.
-        {
+    result = await hass.config_entries.flow.async_init(
-            DISCOVERY_LOCATION: location,
+        DOMAIN,
-            DISCOVERY_NAME: mock_device.name,
+        context={"source": config_entries.SOURCE_SSDP},
-            DISCOVERY_ST: mock_device.device_type,
+        data={
-            DISCOVERY_UDN: mock_device.udn,
+            ssdp.ATTR_SSDP_LOCATION: location,
-            DISCOVERY_UNIQUE_ID: mock_device.unique_id,
+            ssdp.ATTR_SSDP_ST: mock_device.device_type,
-            DISCOVERY_USN: mock_device.usn,
+            ssdp.ATTR_SSDP_USN: mock_device.usn,
-            DISCOVERY_HOSTNAME: mock_device.hostname,
+            ssdp.ATTR_UPNP_UDN: mock_device.udn,
-        }
+        },
-    ]
+    )
-
+    assert result["type"] == data_entry_flow.RESULT_TYPE_ABORT
-    with patch.object(
+    assert result["reason"] == "discovery_ignored"
-        Device, "async_supplement_discovery", AsyncMock(return_value=discoveries[0])
-    ):
-        # Discovered via step ssdp, but ignored.
-        result = await hass.config_entries.flow.async_init(
-            DOMAIN,
-            context={"source": config_entries.SOURCE_SSDP},
-            data={
-                ssdp.ATTR_SSDP_LOCATION: location,
-                ssdp.ATTR_SSDP_ST: mock_device.device_type,
-                ssdp.ATTR_SSDP_USN: mock_device.usn,
-                ssdp.ATTR_UPNP_UDN: mock_device.udn,
-            },
-        )
-        assert result["type"] == data_entry_flow.RESULT_TYPE_ABORT
-        assert result["reason"] == "discovery_ignored"
 
 
 async def test_flow_user(hass: HomeAssistant):
     """Test config flow: discovered + configured through user."""
     udn = "uuid:device_1"
-    location = "dummy"
+    location = "http://dummy"
     mock_device = MockDevice(udn)
     ssdp_discoveries = [
         {
@@ -217,7 +203,7 @@ async def test_flow_import(hass: HomeAssistant):
     """Test config flow: discovered + configured through configuration.yaml."""
     udn = "uuid:device_1"
     mock_device = MockDevice(udn)
-    location = "dummy"
+    location = "http://dummy"
     ssdp_discoveries = [
         {
             ssdp.ATTR_SSDP_LOCATION: location,