From 7ceaebb0866e45441df184091fa455908e5d82d3 Mon Sep 17 00:00:00 2001 From: Christian Lackas Date: Fri, 15 May 2026 21:19:04 +0200 Subject: [PATCH] Fix homematicip_cloud config entry setup crash after migration to 2026.5.0 (#170156) --- .../components/homematicip_cloud/__init__.py | 69 ++++++- .../components/homematicip_cloud/migration.py | 17 +- .../components/homematicip_cloud/test_init.py | 170 ++++++++++++++++++ 3 files changed, 247 insertions(+), 9 deletions(-) diff --git a/homeassistant/components/homematicip_cloud/__init__.py b/homeassistant/components/homematicip_cloud/__init__.py index e3c242275429..c5ee14543cc6 100644 --- a/homeassistant/components/homematicip_cloud/__init__.py +++ b/homeassistant/components/homematicip_cloud/__init__.py @@ -25,7 +25,7 @@ from .const import ( HMIPC_NAME, ) from .hap import HomematicIPConfigEntry, HomematicipHAP -from .migration import _migrate_unique_id +from .migration import _match_legacy_class_name, _migrate_unique_id from .services import async_setup_services _LOGGER = logging.getLogger(__name__) @@ -157,6 +157,73 @@ async def async_migrate_entry( ) entity_registry.async_remove(entry.entity_id) + # Pre-pass: deduplicate legacy entries that would migrate to the same + # new unique_id, and drop legacy entries whose target is already + # occupied by a stable-format entry from a previously-aborted + # migration. Two collision shapes are handled here: + # + # a) Two or more legacy entries share the same new target id (e.g. + # HomematicipNotificationLight + HomematicipNotificationLightV2 + # for the same HmIP-BSL after firmware 2.0.0, or Switch + + # SwitchMeasuring on a device whose capability class changed). + # + # b) One legacy entry shares its target with a stable-format entry + # that was successfully migrated on a previous run before the + # run aborted on a sibling collision. async_migrate_entries + # commits each update individually with no rollback, so partial + # migration is the steady state for any user who already hit + # this bug at least once. + # + # When deduplicating pure-legacy groups, prefer the entry whose + # legacy class name is longer — that is the more specific variant + # (V2 over V1, Measuring over plain) and the one HA has been + # actively binding to since the class transition. + legacy_by_target: dict[tuple[str, str], list[er.RegistryEntry]] = {} + stable_targets: set[tuple[str, str]] = set() + for entry in er.async_entries_for_config_entry( + entity_registry, config_entry.entry_id + ): + new_id = _migrate_unique_id(entry.unique_id) + if new_id is None: + # Stable-format entry — record so we can detect (b). + stable_targets.add((entry.domain, entry.unique_id)) + continue + legacy_by_target.setdefault((entry.domain, new_id), []).append(entry) + + for key, group in legacy_by_target.items(): + if key in stable_targets: + # (b): stable entry already occupies the target. Drop every + # legacy duplicate; the surviving stable entry stays put. + for dup in group: + _LOGGER.warning( + "Removing legacy registry entry %s (%s) — its" + " migration target %s is already in use by a stable" + " entry from a previously-aborted migration", + dup.entity_id, + dup.unique_id, + key[1], + ) + entity_registry.async_remove(dup.entity_id) + continue + if len(group) <= 1: + continue + # (a): multiple legacy entries collide on the same target. + group.sort( + key=lambda e: len(_match_legacy_class_name(e.unique_id) or ""), + reverse=True, + ) + keeper, *duplicates = group + for dup in duplicates: + _LOGGER.warning( + "Removing duplicate registry entry %s (%s) — collides" + " with %s on migration to %s", + dup.entity_id, + dup.unique_id, + keeper.entity_id, + key[1], + ) + entity_registry.async_remove(dup.entity_id) + @callback def _update_unique_id( entity_entry: er.RegistryEntry, diff --git a/homeassistant/components/homematicip_cloud/migration.py b/homeassistant/components/homematicip_cloud/migration.py index 632a830e9597..6091b8255d95 100644 --- a/homeassistant/components/homematicip_cloud/migration.py +++ b/homeassistant/components/homematicip_cloud/migration.py @@ -168,6 +168,14 @@ _NOTIFICATION_LIGHT_RE = re.compile(r"^(Top|Bottom)_(.+)$") _NOTIFICATION_LIGHT_CHANNEL_MAP = {"Top": 2, "Bottom": 3} +def _match_legacy_class_name(old_unique_id: str) -> str | None: + """Return the legacy class name that prefixes ``old_unique_id``, if any.""" + for class_name in _SORTED_CLASS_NAMES: + if old_unique_id.startswith(class_name + "_"): + return class_name + return None + + def _migrate_unique_id(old_unique_id: str) -> str | None: """Convert an old-format unique_id to the new format. @@ -180,14 +188,7 @@ def _migrate_unique_id(old_unique_id: str) -> str | None: {device_id}_{channel}_{feature_id} (device entities) {device_id}_{feature_id} (group/home entities) """ - # Find the matching class name (longest first) - matched_class: str | None = None - for class_name in _SORTED_CLASS_NAMES: - prefix = class_name + "_" - if old_unique_id.startswith(prefix): - matched_class = class_name - break - + matched_class = _match_legacy_class_name(old_unique_id) if matched_class is None: return None diff --git a/tests/components/homematicip_cloud/test_init.py b/tests/components/homematicip_cloud/test_init.py index e532eaaada4d..fb76f01be12c 100644 --- a/tests/components/homematicip_cloud/test_init.py +++ b/tests/components/homematicip_cloud/test_init.py @@ -362,3 +362,173 @@ async def test_migrate_battery_and_obsolete_access_point( assert entity_registry.async_get_entity_id( "binary_sensor", DOMAIN, "3014F711ABCD_0_battery" ) + + +@pytest.mark.parametrize( + ("platform", "old_unique_id_a", "old_unique_id_b", "new_unique_id"), + [ + ( + "light", + "HomematicipNotificationLight_Top_3014F711ABCD", + "HomematicipNotificationLightV2_Top_3014F711ABCD", + "3014F711ABCD_2_notification_light", + ), + ( + "switch", + "HomematicipSwitch_3014F711ABCD", + "HomematicipSwitchMeasuring_3014F711ABCD", + "3014F711ABCD_1_switch", + ), + ], + ids=["notification_light_v1_v2", "switch_vs_measuring"], +) +async def test_migrate_unique_id_collision( + hass: HomeAssistant, + mock_config_entry_v1: MockConfigEntry, + entity_registry: er.EntityRegistry, + caplog: pytest.LogCaptureFixture, + platform: str, + old_unique_id_a: str, + old_unique_id_b: str, + new_unique_id: str, +) -> None: + """Test that legacy duplicates targeting the same new id are deduped.""" + entity_a = entity_registry.async_get_or_create( + platform, + DOMAIN, + old_unique_id_a, + config_entry=mock_config_entry_v1, + ) + entity_b = entity_registry.async_get_or_create( + platform, + DOMAIN, + old_unique_id_b, + config_entry=mock_config_entry_v1, + ) + + with patch("homeassistant.components.homematicip_cloud.HomematicipHAP") as mock_hap: + instance = mock_hap.return_value + instance.async_setup = AsyncMock(return_value=True) + instance.home.id = "1" + instance.home.modelType = "mock-type" + instance.home.name = "mock-name" + instance.home.label = "mock-label" + instance.home.currentAPVersion = "mock-ap-version" + instance.async_reset = AsyncMock(return_value=True) + + await hass.config_entries.async_setup(mock_config_entry_v1.entry_id) + await hass.async_block_till_done() + + assert mock_config_entry_v1.version == 2 + # The longer class-name match (entity_b: V2 / Measuring) wins the collision + surviving_id = entity_registry.async_get_entity_id(platform, DOMAIN, new_unique_id) + assert surviving_id == entity_b.entity_id + # The shorter-class entry was removed + assert entity_registry.async_get(entity_a.entity_id) is None + assert "Removing duplicate registry entry" in caplog.text + + +async def test_migrate_unique_id_partial_prior_run( + hass: HomeAssistant, + mock_config_entry_v1: MockConfigEntry, + entity_registry: er.EntityRegistry, + caplog: pytest.LogCaptureFixture, +) -> None: + """Test recovery from a previously-aborted migration. + + async_migrate_entries commits each entry update individually with no + rollback. If a user already hit the original collision once, one legacy + entry was migrated to the stable format and the other remained. On the + next startup the migration must drop the leftover legacy entry instead + of crashing again on the same collision. + """ + # Simulate previous-run state: one entry at the new (stable) target, + # plus the still-legacy duplicate that was never migrated. + stable_entry = entity_registry.async_get_or_create( + "light", + DOMAIN, + "3014F711ABCD_2_notification_light", + config_entry=mock_config_entry_v1, + ) + stale_legacy = entity_registry.async_get_or_create( + "light", + DOMAIN, + "HomematicipNotificationLight_Top_3014F711ABCD", + config_entry=mock_config_entry_v1, + ) + + with patch("homeassistant.components.homematicip_cloud.HomematicipHAP") as mock_hap: + instance = mock_hap.return_value + instance.async_setup = AsyncMock(return_value=True) + instance.home.id = "1" + instance.home.modelType = "mock-type" + instance.home.name = "mock-name" + instance.home.label = "mock-label" + instance.home.currentAPVersion = "mock-ap-version" + instance.async_reset = AsyncMock(return_value=True) + + await hass.config_entries.async_setup(mock_config_entry_v1.entry_id) + await hass.async_block_till_done() + + assert mock_config_entry_v1.version == 2 + # The stable entry from the previous run keeps its place. + surviving_id = entity_registry.async_get_entity_id( + "light", DOMAIN, "3014F711ABCD_2_notification_light" + ) + assert surviving_id == stable_entry.entity_id + # The leftover legacy entry was removed, not migrated. + assert entity_registry.async_get(stale_legacy.entity_id) is None + assert "already in use by a stable entry" in caplog.text + + +async def test_migrate_unique_id_three_way_collision( + hass: HomeAssistant, + mock_config_entry_v1: MockConfigEntry, + entity_registry: er.EntityRegistry, +) -> None: + """Test that more than two legacy entries can share a target safely. + + The pre-pass groups by computed new id, so any number of legacy + entries pointing at the same target should collapse to one survivor. + """ + short = entity_registry.async_get_or_create( + "switch", + DOMAIN, + "HomematicipSwitch_3014F711ABCD", + config_entry=mock_config_entry_v1, + ) + multi_channel = entity_registry.async_get_or_create( + "switch", + DOMAIN, + "HomematicipMultiSwitch_Channel1_3014F711ABCD", + config_entry=mock_config_entry_v1, + ) + longest = entity_registry.async_get_or_create( + "switch", + DOMAIN, + "HomematicipSwitchMeasuring_3014F711ABCD", + config_entry=mock_config_entry_v1, + ) + + with patch("homeassistant.components.homematicip_cloud.HomematicipHAP") as mock_hap: + instance = mock_hap.return_value + instance.async_setup = AsyncMock(return_value=True) + instance.home.id = "1" + instance.home.modelType = "mock-type" + instance.home.name = "mock-name" + instance.home.label = "mock-label" + instance.home.currentAPVersion = "mock-ap-version" + instance.async_reset = AsyncMock(return_value=True) + + await hass.config_entries.async_setup(mock_config_entry_v1.entry_id) + await hass.async_block_till_done() + + assert mock_config_entry_v1.version == 2 + # The longest legacy class name wins. + surviving_id = entity_registry.async_get_entity_id( + "switch", DOMAIN, "3014F711ABCD_1_switch" + ) + assert surviving_id == longest.entity_id + # The shorter and multi-channel legacy entries were removed. + assert entity_registry.async_get(short.entity_id) is None + assert entity_registry.async_get(multi_channel.entity_id) is None