From 4cd2351bcc0bb9af7383685650d84bbfadaa2106 Mon Sep 17 00:00:00 2001 From: Steven B <51370195+sdb9696@users.noreply.github.com> Date: Tue, 9 Apr 2024 10:08:46 +0100 Subject: [PATCH] Update and migrate ring non string unique ids (#115265) Co-authored-by: J. Nick Koston --- homeassistant/components/ring/__init__.py | 31 ++++- homeassistant/components/ring/camera.py | 2 +- homeassistant/components/ring/light.py | 2 +- tests/components/ring/test_camera.py | 4 +- tests/components/ring/test_init.py | 138 +++++++++++++++++++++- tests/components/ring/test_light.py | 4 +- 6 files changed, 172 insertions(+), 9 deletions(-) diff --git a/homeassistant/components/ring/__init__.py b/homeassistant/components/ring/__init__.py index e3697d4fccc..ffa99704526 100644 --- a/homeassistant/components/ring/__init__.py +++ b/homeassistant/components/ring/__init__.py @@ -9,8 +9,8 @@ from ring_doorbell import Auth, Ring from homeassistant.config_entries import ConfigEntry from homeassistant.const import APPLICATION_NAME, CONF_TOKEN, __version__ -from homeassistant.core import HomeAssistant, ServiceCall -from homeassistant.helpers import device_registry as dr +from homeassistant.core import HomeAssistant, ServiceCall, callback +from homeassistant.helpers import device_registry as dr, entity_registry as er from homeassistant.helpers.issue_registry import IssueSeverity, async_create_issue from .const import ( @@ -44,6 +44,8 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: ) ring = Ring(auth) + await _migrate_old_unique_ids(hass, entry.entry_id) + devices_coordinator = RingDataCoordinator(hass, ring) notifications_coordinator = RingNotificationsCoordinator(hass, ring) await devices_coordinator.async_config_entry_first_refresh() @@ -111,3 +113,28 @@ async def async_remove_config_entry_device( ) -> bool: """Remove a config entry from a device.""" return True + + +async def _migrate_old_unique_ids(hass: HomeAssistant, entry_id: str) -> None: + entity_registry = er.async_get(hass) + + @callback + def _async_migrator(entity_entry: er.RegistryEntry) -> dict[str, str] | None: + # Old format for camera and light was int + if isinstance(entity_entry.unique_id, int): + new_unique_id = str(entity_entry.unique_id) + if existing_entity_id := entity_registry.async_get_entity_id( + entity_entry.domain, entity_entry.platform, new_unique_id + ): + _LOGGER.error( + "Cannot migrate to unique_id '%s', already exists for '%s', " + "You may have to delete unavailable ring entities", + new_unique_id, + existing_entity_id, + ) + return None + _LOGGER.info("Fixing non string unique id %s", entity_entry.unique_id) + return {"new_unique_id": new_unique_id} + return None + + await er.async_migrate_entries(hass, entry_id, _async_migrator) diff --git a/homeassistant/components/ring/camera.py b/homeassistant/components/ring/camera.py index ec0f4ca3fab..b9d73afe6de 100644 --- a/homeassistant/components/ring/camera.py +++ b/homeassistant/components/ring/camera.py @@ -67,7 +67,7 @@ class RingCam(RingEntity, Camera): self._video_url = None self._image = None self._expires_at = dt_util.utcnow() - FORCE_REFRESH_INTERVAL - self._attr_unique_id = device.id + self._attr_unique_id = str(device.id) if device.has_capability(MOTION_DETECTION_CAPABILITY): self._attr_motion_detection_enabled = device.motion_detection diff --git a/homeassistant/components/ring/light.py b/homeassistant/components/ring/light.py index 10d13e59810..b9e1c8c38b4 100644 --- a/homeassistant/components/ring/light.py +++ b/homeassistant/components/ring/light.py @@ -58,7 +58,7 @@ class RingLight(RingEntity, LightEntity): def __init__(self, device, coordinator): """Initialize the light.""" super().__init__(device, coordinator) - self._attr_unique_id = device.id + self._attr_unique_id = str(device.id) self._attr_is_on = device.lights == ON_STATE self._no_updates_until = dt_util.utcnow() diff --git a/tests/components/ring/test_camera.py b/tests/components/ring/test_camera.py index de61d7a1452..dde1252d5b8 100644 --- a/tests/components/ring/test_camera.py +++ b/tests/components/ring/test_camera.py @@ -25,10 +25,10 @@ async def test_entity_registry( entity_registry = er.async_get(hass) entry = entity_registry.async_get("camera.front") - assert entry.unique_id == 765432 + assert entry.unique_id == "765432" entry = entity_registry.async_get("camera.internal") - assert entry.unique_id == 345678 + assert entry.unique_id == "345678" @pytest.mark.parametrize( diff --git a/tests/components/ring/test_init.py b/tests/components/ring/test_init.py index 3ca686c37a4..664f8ff1973 100644 --- a/tests/components/ring/test_init.py +++ b/tests/components/ring/test_init.py @@ -8,9 +8,13 @@ import requests_mock from ring_doorbell import AuthenticationError, RingError, RingTimeout from homeassistant.components import ring +from homeassistant.components.camera import DOMAIN as CAMERA_DOMAIN +from homeassistant.components.light import DOMAIN as LIGHT_DOMAIN from homeassistant.components.ring import DOMAIN from homeassistant.config_entries import SOURCE_REAUTH, ConfigEntryState +from homeassistant.const import CONF_USERNAME from homeassistant.core import HomeAssistant +from homeassistant.helpers import entity_registry as er from homeassistant.helpers.issue_registry import IssueRegistry from homeassistant.setup import async_setup_component from homeassistant.util import dt as dt_util @@ -253,7 +257,7 @@ async def test_issue_deprecated_service_ring_update( await hass.config_entries.async_setup(mock_config_entry.entry_id) await hass.async_block_till_done() - _ = await hass.services.async_call(DOMAIN, "update", {}, blocking=True) + await hass.services.async_call(DOMAIN, "update", {}, blocking=True) issue = issue_registry.async_get_issue("ring", "deprecated_service_ring_update") assert issue @@ -266,3 +270,135 @@ async def test_issue_deprecated_service_ring_update( "This is deprecated and will stop working in Home Assistant 2024.10. " "Use 'homeassistant.update_entity' instead which updates all ring entities" ) in caplog.text + + +@pytest.mark.parametrize( + ("domain", "old_unique_id"), + [ + ( + LIGHT_DOMAIN, + 123456, + ), + ( + CAMERA_DOMAIN, + 654321, + ), + ], +) +async def test_update_unique_id( + hass: HomeAssistant, + entity_registry: er.EntityRegistry, + caplog: pytest.LogCaptureFixture, + requests_mock: requests_mock.Mocker, + domain: str, + old_unique_id: int | str, +) -> None: + """Test unique_id update of integration.""" + entry = MockConfigEntry( + title="Ring", + domain=DOMAIN, + data={ + CONF_USERNAME: "foo@bar.com", + "token": {"access_token": "mock-token"}, + }, + unique_id="foo@bar.com", + ) + entry.add_to_hass(hass) + + entity = entity_registry.async_get_or_create( + domain=domain, + platform=DOMAIN, + unique_id=old_unique_id, + config_entry=entry, + ) + assert entity.unique_id == old_unique_id + assert await hass.config_entries.async_setup(entry.entry_id) + await hass.async_block_till_done() + + entity_migrated = entity_registry.async_get(entity.entity_id) + assert entity_migrated + assert entity_migrated.unique_id == str(old_unique_id) + assert (f"Fixing non string unique id {old_unique_id}") in caplog.text + + +async def test_update_unique_id_existing( + hass: HomeAssistant, + entity_registry: er.EntityRegistry, + caplog: pytest.LogCaptureFixture, + requests_mock: requests_mock.Mocker, +) -> None: + """Test unique_id update of integration.""" + old_unique_id = 123456 + entry = MockConfigEntry( + title="Ring", + domain=DOMAIN, + data={ + CONF_USERNAME: "foo@bar.com", + "token": {"access_token": "mock-token"}, + }, + unique_id="foo@bar.com", + ) + entry.add_to_hass(hass) + + entity = entity_registry.async_get_or_create( + domain=CAMERA_DOMAIN, + platform=DOMAIN, + unique_id=old_unique_id, + config_entry=entry, + ) + entity_existing = entity_registry.async_get_or_create( + domain=CAMERA_DOMAIN, + platform=DOMAIN, + unique_id=str(old_unique_id), + config_entry=entry, + ) + assert entity.unique_id == old_unique_id + assert entity_existing.unique_id == str(old_unique_id) + assert await hass.config_entries.async_setup(entry.entry_id) + await hass.async_block_till_done() + + entity_not_migrated = entity_registry.async_get(entity.entity_id) + entity_existing = entity_registry.async_get(entity_existing.entity_id) + assert entity_not_migrated + assert entity_existing + assert entity_not_migrated.unique_id == old_unique_id + assert ( + f"Cannot migrate to unique_id '{old_unique_id}', " + f"already exists for '{entity_existing.entity_id}', " + "You may have to delete unavailable ring entities" + ) in caplog.text + + +async def test_update_unique_id_no_update( + hass: HomeAssistant, + entity_registry: er.EntityRegistry, + caplog: pytest.LogCaptureFixture, + requests_mock: requests_mock.Mocker, +) -> None: + """Test unique_id update of integration.""" + correct_unique_id = "123456" + entry = MockConfigEntry( + title="Ring", + domain=DOMAIN, + data={ + CONF_USERNAME: "foo@bar.com", + "token": {"access_token": "mock-token"}, + }, + unique_id="foo@bar.com", + ) + entry.add_to_hass(hass) + + entity = entity_registry.async_get_or_create( + domain=CAMERA_DOMAIN, + platform=DOMAIN, + unique_id="123456", + config_entry=entry, + ) + assert entity.unique_id == correct_unique_id + assert await hass.config_entries.async_setup(entry.entry_id) + await hass.async_block_till_done() + + entity_migrated = entity_registry.async_get(entity.entity_id) + assert entity_migrated + assert entity_migrated.unique_id == correct_unique_id + assert "Fixing non string unique id" not in caplog.text diff --git a/tests/components/ring/test_light.py b/tests/components/ring/test_light.py index 621c0b8f1d0..ac0f3b70d27 100644 --- a/tests/components/ring/test_light.py +++ b/tests/components/ring/test_light.py @@ -25,10 +25,10 @@ async def test_entity_registry( entity_registry = er.async_get(hass) entry = entity_registry.async_get("light.front_light") - assert entry.unique_id == 765432 + assert entry.unique_id == "765432" entry = entity_registry.async_get("light.internal_light") - assert entry.unique_id == 345678 + assert entry.unique_id == "345678" async def test_light_off_reports_correctly(