Fix ONVIF camera entities ids getting shuffled on reload (#139676)

homeassistant/components/onvif/__init__.py

@@ -19,8 +19,9 @@ from homeassistant.const import (
     HTTP_DIGEST_AUTHENTICATION,
     Platform,
 )
-from homeassistant.core import HomeAssistant
+from homeassistant.core import HomeAssistant, callback
 from homeassistant.exceptions import ConfigEntryAuthFailed, ConfigEntryNotReady
+from homeassistant.helpers import entity_registry as er
 
 from .const import (
     CONF_ENABLE_WEBHOOKS,
@@ -99,6 +100,8 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
     if device.capabilities.imaging:
         device.platforms += [Platform.SWITCH]
 
+    _async_migrate_camera_entities_unique_ids(hass, entry, device)
+
     await hass.config_entries.async_forward_entry_setups(entry, device.platforms)
 
     entry.async_on_unload(
@@ -155,3 +158,58 @@ async def async_populate_options(hass: HomeAssistant, entry: ConfigEntry) -> Non
     }
 
     hass.config_entries.async_update_entry(entry, options=options)
+
+
+@callback
+def _async_migrate_camera_entities_unique_ids(
+    hass: HomeAssistant, config_entry: ConfigEntry, device: ONVIFDevice
+) -> None:
+    """Migrate unique ids of camera entities from profile index to profile token."""
+    entity_reg = er.async_get(hass)
+    entities: list[er.RegistryEntry] = er.async_entries_for_config_entry(
+        entity_reg, config_entry.entry_id
+    )
+
+    mac_or_serial = device.info.mac or device.info.serial_number
+    old_uid_start = f"{mac_or_serial}_"
+    new_uid_start = f"{mac_or_serial}#"
+
+    for entity in entities:
+        if entity.domain != Platform.CAMERA:
+            continue
+
+        if (
+            not entity.unique_id.startswith(old_uid_start)
+            and entity.unique_id != mac_or_serial
+        ):
+            continue
+
+        index = 0
+        if entity.unique_id.startswith(old_uid_start):
+            try:
+                index = int(entity.unique_id[len(old_uid_start) :])
+            except ValueError:
+                LOGGER.error(
+                    "Failed to migrate unique id for '%s' as the ONVIF profile index could not be parsed from unique id '%s'",
+                    entity.entity_id,
+                    entity.unique_id,
+                )
+                continue
+        try:
+            token = device.profiles[index].token
+        except IndexError:
+            LOGGER.error(
+                "Failed to migrate unique id for '%s' as the ONVIF profile index '%d' parsed from unique id '%s' could not be found",
+                entity.entity_id,
+                index,
+                entity.unique_id,
+            )
+            continue
+        new_uid = f"{new_uid_start}{token}"
+        LOGGER.debug(
+            "Migrating unique id for '%s' from '%s' to '%s'",
+            entity.entity_id,
+            entity.unique_id,
+            new_uid,
+        )
+        entity_reg.async_update_entity(entity.entity_id, new_unique_id=new_uid)

homeassistant/components/onvif/camera.py

@@ -117,10 +117,7 @@ class ONVIFCameraEntity(ONVIFBaseEntity, Camera):
         self._attr_entity_registry_enabled_default = (
             device.max_resolution == profile.video.resolution.width
         )
-        if profile.index:
+        self._attr_unique_id = f"{self.mac_or_serial}#{profile.token}"
-            self._attr_unique_id = f"{self.mac_or_serial}_{profile.index}"
-        else:
-            self._attr_unique_id = self.mac_or_serial
         self._attr_name = f"{device.name} {profile.name}"
 
     @property

tests/components/onvif/__init__.py

@@ -123,7 +123,7 @@ def setup_mock_onvif_camera(
     mock_onvif_camera.side_effect = mock_constructor
 
 
-def setup_mock_device(mock_device, capabilities=None):
+def setup_mock_device(mock_device, capabilities=None, profiles=None):
     """Prepare mock ONVIFDevice."""
     mock_device.async_setup = AsyncMock(return_value=True)
     mock_device.port = 80
@@ -145,7 +145,7 @@ def setup_mock_device(mock_device, capabilities=None):
         ptz=None,
         video_source_token=None,
     )
-    mock_device.profiles = [profile1]
+    mock_device.profiles = profiles or [profile1]
     mock_device.events = MagicMock(
         webhook_manager=MagicMock(state=WebHookManagerState.STARTED),
         pullpoint_manager=MagicMock(state=PullPointManagerState.PAUSED),

tests/components/onvif/test_init.py

@@ -0,0 +1,102 @@
+"""Tests for the ONVIF integration __init__ module."""
+
+from unittest.mock import MagicMock, patch
+
+import pytest
+
+from homeassistant.core import HomeAssistant
+from homeassistant.helpers import entity_registry as er
+
+from . import MAC, setup_mock_device
+
+from tests.common import MockConfigEntry
+
+
+@pytest.mark.asyncio
+async def test_migrate_camera_entities_unique_ids(hass: HomeAssistant) -> None:
+    """Test that camera entities unique ids get migrated properly."""
+    config_entry = MockConfigEntry(domain="onvif", unique_id=MAC)
+    config_entry.add_to_hass(hass)
+
+    entity_registry = er.async_get(hass)
+
+    entity_with_only_mac = entity_registry.async_get_or_create(
+        domain="camera",
+        platform="onvif",
+        unique_id=MAC,
+        config_entry=config_entry,
+    )
+    entity_with_index = entity_registry.async_get_or_create(
+        domain="camera",
+        platform="onvif",
+        unique_id=f"{MAC}_1",
+        config_entry=config_entry,
+    )
+    # This one should not be migrated (different domain)
+    entity_sensor = entity_registry.async_get_or_create(
+        domain="sensor",
+        platform="onvif",
+        unique_id=MAC,
+        config_entry=config_entry,
+    )
+    # This one should not be migrated (already migrated)
+    entity_migrated = entity_registry.async_get_or_create(
+        domain="camera",
+        platform="onvif",
+        unique_id=f"{MAC}#profile_token_2",
+        config_entry=config_entry,
+    )
+    # Unparsable index
+    entity_unparsable_index = entity_registry.async_get_or_create(
+        domain="camera",
+        platform="onvif",
+        unique_id=f"{MAC}_a",
+        config_entry=config_entry,
+    )
+    # Unexisting index
+    entity_unexisting_index = entity_registry.async_get_or_create(
+        domain="camera",
+        platform="onvif",
+        unique_id=f"{MAC}_9",
+        config_entry=config_entry,
+    )
+
+    with patch("homeassistant.components.onvif.ONVIFDevice") as mock_device:
+        setup_mock_device(
+            mock_device,
+            capabilities=None,
+            profiles=[
+                MagicMock(token="profile_token_0"),
+                MagicMock(token="profile_token_1"),
+                MagicMock(token="profile_token_2"),
+            ],
+        )
+        await hass.config_entries.async_setup(config_entry.entry_id)
+        await hass.async_block_till_done()
+
+    entity_with_only_mac = entity_registry.async_get(entity_with_only_mac.entity_id)
+    entity_with_index = entity_registry.async_get(entity_with_index.entity_id)
+    entity_sensor = entity_registry.async_get(entity_sensor.entity_id)
+    entity_migrated = entity_registry.async_get(entity_migrated.entity_id)
+
+    assert entity_with_only_mac is not None
+    assert entity_with_only_mac.unique_id == f"{MAC}#profile_token_0"
+
+    assert entity_with_index is not None
+    assert entity_with_index.unique_id == f"{MAC}#profile_token_1"
+
+    # Make sure the sensor entity is unchanged
+    assert entity_sensor is not None
+    assert entity_sensor.unique_id == MAC
+
+    # Make sure the already migrated entity is unchanged
+    assert entity_migrated is not None
+    assert entity_migrated.unique_id == f"{MAC}#profile_token_2"
+
+    # Make sure the unparsable index entity is unchanged
+    assert entity_unparsable_index is not None
+    assert entity_unparsable_index.unique_id == f"{MAC}_a"
+
+    # Make sure the unexisting index entity is unchanged
+    assert entity_unexisting_index is not None
+    assert entity_unexisting_index.unique_id == f"{MAC}_9"