From b2b940fc3281267c1f944b9e33fa789ab56962a2 Mon Sep 17 00:00:00 2001 From: Erik Montnemery Date: Thu, 3 Oct 2024 22:27:15 +0200 Subject: [PATCH] Remove assumption in ConfigEntryItems about unique unique_id (#127399) --- homeassistant/config_entries.py | 17 ++++++++++------ tests/test_config_entries.py | 35 +++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 6 deletions(-) diff --git a/homeassistant/config_entries.py b/homeassistant/config_entries.py index 404ae1c91dd..f9dc9191c8e 100644 --- a/homeassistant/config_entries.py +++ b/homeassistant/config_entries.py @@ -1558,7 +1558,7 @@ class ConfigEntryItems(UserDict[str, ConfigEntry]): super().__init__() self._hass = hass self._domain_index: dict[str, list[ConfigEntry]] = {} - self._domain_unique_id_index: dict[str, dict[str, ConfigEntry]] = {} + self._domain_unique_id_index: dict[str, dict[str, list[ConfigEntry]]] = {} def values(self) -> ValuesView[ConfigEntry]: """Return the underlying values to avoid __iter__ overhead.""" @@ -1601,9 +1601,9 @@ class ConfigEntryItems(UserDict[str, ConfigEntry]): report_issue, ) - self._domain_unique_id_index.setdefault(entry.domain, {})[ - unique_id_hash - ] = entry + self._domain_unique_id_index.setdefault(entry.domain, {}).setdefault( + unique_id_hash, [] + ).append(entry) def _unindex_entry(self, entry_id: str) -> None: """Unindex an entry.""" @@ -1616,7 +1616,9 @@ class ConfigEntryItems(UserDict[str, ConfigEntry]): # Check type first to avoid expensive isinstance call if type(unique_id) is not str and not isinstance(unique_id, Hashable): # noqa: E721 unique_id = str(entry.unique_id) # type: ignore[unreachable] - del self._domain_unique_id_index[domain][unique_id] + self._domain_unique_id_index[domain][unique_id].remove(entry) + if not self._domain_unique_id_index[domain][unique_id]: + del self._domain_unique_id_index[domain][unique_id] if not self._domain_unique_id_index[domain]: del self._domain_unique_id_index[domain] @@ -1647,7 +1649,10 @@ class ConfigEntryItems(UserDict[str, ConfigEntry]): # Check type first to avoid expensive isinstance call if type(unique_id) is not str and not isinstance(unique_id, Hashable): # noqa: E721 unique_id = str(unique_id) # type: ignore[unreachable] - return self._domain_unique_id_index.get(domain, {}).get(unique_id) + entries = self._domain_unique_id_index.get(domain, {}).get(unique_id) + if not entries: + return None + return entries[0] class ConfigEntryStore(storage.Store[dict[str, list[dict[str, Any]]]]): diff --git a/tests/test_config_entries.py b/tests/test_config_entries.py index 9cba19ef3b1..92cec00ccdf 100644 --- a/tests/test_config_entries.py +++ b/tests/test_config_entries.py @@ -512,6 +512,41 @@ async def test_remove_entry( assert not entity_entry_list +async def test_remove_entry_non_unique_unique_id( + hass: HomeAssistant, + manager: config_entries.ConfigEntries, + entity_registry: er.EntityRegistry, +) -> None: + """Test that we can remove entry with colliding unique_id.""" + entry_1 = MockConfigEntry( + domain="test_other", entry_id="test1", unique_id="not_unique" + ) + entry_1.add_to_manager(manager) + entry_2 = MockConfigEntry( + domain="test_other", entry_id="test2", unique_id="not_unique" + ) + entry_2.add_to_manager(manager) + entry_3 = MockConfigEntry( + domain="test_other", entry_id="test3", unique_id="not_unique" + ) + entry_3.add_to_manager(manager) + + # Check all config entries exist + assert manager.async_entry_ids() == [ + "test1", + "test2", + "test3", + ] + + # Remove entries + assert await manager.async_remove("test1") == {"require_restart": False} + await hass.async_block_till_done() + assert await manager.async_remove("test2") == {"require_restart": False} + await hass.async_block_till_done() + assert await manager.async_remove("test3") == {"require_restart": False} + await hass.async_block_till_done() + + async def test_remove_entry_cancels_reauth( hass: HomeAssistant, manager: config_entries.ConfigEntries,