From 16d7c5663739e3671b4564045a2ce1404b1befc9 Mon Sep 17 00:00:00 2001 From: Erik Montnemery Date: Mon, 20 Jul 2026 19:36:36 +0200 Subject: [PATCH] Simplify async_remove_helper_devices (#176919) --- homeassistant/helpers/helper_integration.py | 56 ++----------- tests/helpers/test_helper_integration.py | 89 ++------------------- 2 files changed, 14 insertions(+), 131 deletions(-) diff --git a/homeassistant/helpers/helper_integration.py b/homeassistant/helpers/helper_integration.py index 62f589fadff2..222c5d749a1d 100644 --- a/homeassistant/helpers/helper_integration.py +++ b/homeassistant/helpers/helper_integration.py @@ -146,14 +146,14 @@ def async_remove_helper_devices( now owns a split of it (linked by the pre-migration composite_device_id); a helper that declared the source device's identifiers or connections in its device_info afterwards now owns a fork (a separate device that copied that identity). This removes the helper's - duplicate device(s) and relinks its entities to the source device. + duplicate device(s) and relinks its entities to the source device, or detaches them when + the source has no concrete device to hold the link. :param helper_config_entry_id: The config entry id of the helper being migrated. - :param source_device_id: The device the helper should link to. May be the pre-migration - composite id (as a helper that stored the device id before it was split passes) or a - concrete source device. May also be None when no source device is selected: in sweep - mode the helper's devices are then removed and its entities left without a device; - in targeted mode this is a no-op. + :param source_device_id: The device the helper should link its entities to. A concrete + device relinks the entities to it. A pre-migration composite id and None have no + concrete device to hold the link, so the entities are detached; in targeted mode + a composite or None source with no matching duplicate is a no-op. :param sweep_helper_devices: By default only the helper's single duplicate of source_device_id (a split or fork) is removed. When True, every device the helper owns except source_device_id and keep_device_ids is removed instead - @@ -193,19 +193,7 @@ def async_remove_helper_devices( composite_device_id = ( source_device.composite_device_id if source_is_concrete else source_device_id ) - split_devices = ( - device_registry.async_get_devices_for_composite_device_id(composite_device_id) - if composite_device_id is not None - else [] - ) - - # The helper's entities are relinked to the source device: itself when concrete, else - # the composite's recorded primary owner's split. - target_device_id = ( - source_device_id - if source_is_concrete - else _composite_source_split_id(split_devices, helper_config_entry_id) - ) + target_device_id = source_device_id if source_is_concrete else None if sweep_helper_devices: _sweep_helper_devices( @@ -226,36 +214,6 @@ def async_remove_helper_devices( ) -def _composite_source_split_id( - split_devices: list[dr.DeviceEntry], helper_config_entry_id: str -) -> str | None: - """Return the source split of a composite device. - - This is the split owned by the composite's recorded primary config entry - the rule - _restore_composite_device uses. Devices migrated from before 2024.7 have no recorded - primary (primary_config_entry, and thus composite_primary_config_entry, is None), so - fall back to a surviving non-helper split, mirroring _restore_composite_device's - fallback to the first split. None when source_device_id is not a composite id or the - helper owns every split. - """ - non_helper_splits = [ - device - for device in split_devices - if device.config_entry_id != helper_config_entry_id - ] - if not non_helper_splits: - return None - primary_config_entry_id = split_devices[0].composite_primary_config_entry - return next( - ( - device.id - for device in non_helper_splits - if device.config_entry_id == primary_config_entry_id - ), - non_helper_splits[0].id, - ) - - def _remove_duplicate_helper_device( device_registry: dr.DeviceRegistry, entity_registry: er.EntityRegistry, diff --git a/tests/helpers/test_helper_integration.py b/tests/helpers/test_helper_integration.py index 0e899de2c522..8bf01c5915b8 100644 --- a/tests/helpers/test_helper_integration.py +++ b/tests/helpers/test_helper_integration.py @@ -526,16 +526,6 @@ async def test_async_handle_source_entity_new_entity_id( "source_via_composite_id", [pytest.param(True, id="composite_id"), pytest.param(False, id="source_split")], ) -@pytest.mark.parametrize( - "record_composite_primary", - [ - pytest.param(True, id="recorded_primary"), - # Devices migrated from before 2024.7 have no primary_config_entry, so their splits - # carry composite_primary_config_entry=None; the source split is then found by - # falling back to the surviving non-helper split. - pytest.param(False, id="no_recorded_primary"), - ], -) async def test_async_remove_helper_devices( hass: HomeAssistant, device_registry: dr.DeviceRegistry, @@ -543,7 +533,6 @@ async def test_async_remove_helper_devices( helper_identifiers: set[tuple[str, str]], helper_has_composite_identifiers: bool, source_via_composite_id: bool, - record_composite_primary: bool, ) -> None: """Test migrating a helper off a device it co-owned before the migration split. @@ -551,17 +540,14 @@ async def test_async_remove_helper_devices( the pre-migration id as their composite id. The helper's split is found via that id - both while it still carries the identifiers copied at the split and once the helper has re-registered and pruned them to its own - and whether the caller passes the composite - id or the concrete source split. Its entities move onto the source split; its split is - removed. + id or the concrete source split. Its split is removed; its entities move onto the source + split when a concrete device is passed, or are detached when only the composite id is. """ source_config_entry = MockConfigEntry(domain=SOURCE_DOMAIN) source_config_entry.add_to_hass(hass) helper_config_entry = MockConfigEntry(domain=HELPER_DOMAIN) helper_config_entry.add_to_hass(hass) composite_id = "pre_split_composite_id" - composite_primary = ( - source_config_entry.entry_id if record_composite_primary else None - ) source_split = device_registry.async_get_or_create( config_entry_id=source_config_entry.entry_id, @@ -571,16 +557,14 @@ async def test_async_remove_helper_devices( config_entry_id=helper_config_entry.entry_id, identifiers=helper_identifiers, ) - # Both are splits of the same pre-migration device, sharing its id and primary owner + # Both are splits of the same pre-migration device, sharing its id device_registry.devices[source_split.id] = attr.evolve( source_split, composite_device_id=composite_id, - composite_primary_config_entry=composite_primary, ) device_registry.devices[helper_split.id] = attr.evolve( helper_split, composite_device_id=composite_id, - composite_primary_config_entry=composite_primary, has_composite_identifiers=helper_has_composite_identifiers, ) # A helper entity on the helper's split, plus one not linked to any device @@ -601,10 +585,12 @@ async def test_async_remove_helper_devices( source_device_id=composite_id if source_via_composite_id else source_split.id, ) - # The helper's entity moved onto the source split; its own split was removed + # The helper's split was removed. Its entity moved onto the source split when a concrete + # source was passed; with only the composite id there is no concrete device, so it detaches. + expected_device_id = None if source_via_composite_id else source_split.id assert ( entity_registry.async_get(helper_entity_entry.entity_id).device_id - == source_split.id + == expected_device_id ) assert ( entity_registry.async_get(extra_helper_entity_entry.entity_id).device_id is None @@ -613,67 +599,6 @@ async def test_async_remove_helper_devices( assert device_registry.async_get(source_split.id) is not None -async def test_async_remove_helper_devices_multiple_co_owners( - hass: HomeAssistant, - device_registry: dr.DeviceRegistry, - entity_registry: er.EntityRegistry, -) -> None: - """With more than two co-owners, the helper's entities go to the primary's split. - - A pre-migration device can be co-owned by more than two config entries. The source - split is identified by the composite's recorded primary owner, not by split order, so - the helper's entities are not relinked onto an unrelated integration's split. - """ - source_config_entry = MockConfigEntry(domain=SOURCE_DOMAIN) - source_config_entry.add_to_hass(hass) - other_config_entry = MockConfigEntry(domain="other") - other_config_entry.add_to_hass(hass) - helper_config_entry = MockConfigEntry(domain=HELPER_DOMAIN) - helper_config_entry.add_to_hass(hass) - composite_id = "pre_split_composite_id" - - # other_split is indexed before source_split, so a split-order heuristic would wrongly - # pick it as the source; the recorded primary (source) must win instead. - other_split = device_registry.async_get_or_create( - config_entry_id=other_config_entry.entry_id, identifiers={("other", "1")} - ) - source_split = device_registry.async_get_or_create( - config_entry_id=source_config_entry.entry_id, identifiers={(SOURCE_DOMAIN, "1")} - ) - helper_split = device_registry.async_get_or_create( - config_entry_id=helper_config_entry.entry_id, identifiers={(HELPER_DOMAIN, "1")} - ) - for split in (other_split, source_split, helper_split): - device_registry.devices[split.id] = attr.evolve( - split, - composite_device_id=composite_id, - composite_primary_config_entry=source_config_entry.entry_id, - ) - helper_entity_entry = entity_registry.async_get_or_create( - "sensor", - HELPER_DOMAIN, - "1", - config_entry=helper_config_entry, - device_id=helper_split.id, - ) - - async_remove_helper_devices( - hass, - helper_config_entry_id=helper_config_entry.entry_id, - source_device_id=composite_id, - ) - - # Relinked to the primary owner's (source) split, not the unrelated other split, which - # is left untouched - assert ( - entity_registry.async_get(helper_entity_entry.entity_id).device_id - == source_split.id - ) - assert device_registry.async_get(helper_split.id) is None - assert device_registry.async_get(other_split.id) is not None - assert device_registry.async_get(source_split.id) is not None - - async def test_async_remove_helper_devices_fork( hass: HomeAssistant, device_registry: dr.DeviceRegistry,