From 99f227229e49e5497b26205d3f444367c333da2a Mon Sep 17 00:00:00 2001 From: Aaron Collins Date: Tue, 3 Oct 2023 21:11:21 +1300 Subject: [PATCH] Remove duplicated device before daikin migration (#99900) Co-authored-by: Erik Montnemery --- homeassistant/components/daikin/__init__.py | 31 +++++- tests/components/daikin/test_init.py | 105 +++++++++++++++++++- 2 files changed, 128 insertions(+), 8 deletions(-) diff --git a/homeassistant/components/daikin/__init__.py b/homeassistant/components/daikin/__init__.py index f6fd399f855..eda7976e572 100644 --- a/homeassistant/components/daikin/__init__.py +++ b/homeassistant/components/daikin/__init__.py @@ -135,9 +135,11 @@ async def async_migrate_unique_id( ) -> None: """Migrate old entry.""" dev_reg = dr.async_get(hass) + ent_reg = er.async_get(hass) old_unique_id = config_entry.unique_id new_unique_id = api.device.mac - new_name = api.device.values.get("name") + new_mac = dr.format_mac(new_unique_id) + new_name = api.name @callback def _update_unique_id(entity_entry: er.RegistryEntry) -> dict[str, str] | None: @@ -147,15 +149,36 @@ async def async_migrate_unique_id( if new_unique_id == old_unique_id: return + duplicate = dev_reg.async_get_device( + connections={(CONNECTION_NETWORK_MAC, new_mac)}, identifiers=None + ) + + # Remove duplicated device + if duplicate is not None: + if config_entry.entry_id in duplicate.config_entries: + _LOGGER.debug( + "Removing duplicated device %s", + duplicate.name, + ) + + # The automatic cleanup in entity registry is scheduled as a task, remove + # the entities manually to avoid unique_id collision when the entities + # are migrated. + duplicate_entities = er.async_entries_for_device( + ent_reg, duplicate.id, True + ) + for entity in duplicate_entities: + ent_reg.async_remove(entity.entity_id) + + dev_reg.async_remove_device(duplicate.id) + # Migrate devices for device_entry in dr.async_entries_for_config_entry( dev_reg, config_entry.entry_id ): for connection in device_entry.connections: if connection[1] == old_unique_id: - new_connections = { - (CONNECTION_NETWORK_MAC, dr.format_mac(new_unique_id)) - } + new_connections = {(CONNECTION_NETWORK_MAC, new_mac)} _LOGGER.debug( "Migrating device %s connections to %s", diff --git a/tests/components/daikin/test_init.py b/tests/components/daikin/test_init.py index a6a58b4fb39..3b5f81ae2e5 100644 --- a/tests/components/daikin/test_init.py +++ b/tests/components/daikin/test_init.py @@ -1,11 +1,13 @@ """Define tests for the Daikin init.""" import asyncio +from datetime import timedelta from unittest.mock import AsyncMock, PropertyMock, patch from aiohttp import ClientConnectionError +from freezegun.api import FrozenDateTimeFactory import pytest -from homeassistant.components.daikin import update_unique_id +from homeassistant.components.daikin import DaikinApi, update_unique_id from homeassistant.components.daikin.const import DOMAIN, KEY_MAC from homeassistant.config_entries import ConfigEntryState from homeassistant.const import CONF_HOST @@ -14,7 +16,7 @@ from homeassistant.helpers import device_registry as dr, entity_registry as er from .test_config_flow import HOST, MAC -from tests.common import MockConfigEntry +from tests.common import MockConfigEntry, async_fire_time_changed @pytest.fixture @@ -28,6 +30,7 @@ def mock_daikin(): with patch("homeassistant.components.daikin.Appliance") as Appliance: Appliance.factory.side_effect = mock_daikin_factory type(Appliance).update_status = AsyncMock() + type(Appliance).device_ip = PropertyMock(return_value=HOST) type(Appliance).inside_temperature = PropertyMock(return_value=22) type(Appliance).target_temperature = PropertyMock(return_value=22) type(Appliance).zones = PropertyMock(return_value=[("Zone 1", "0", 0)]) @@ -47,6 +50,67 @@ DATA = { INVALID_DATA = {**DATA, "name": None, "mac": HOST} +async def test_duplicate_removal(hass: HomeAssistant, mock_daikin) -> None: + """Test duplicate device removal.""" + config_entry = MockConfigEntry( + domain=DOMAIN, + unique_id=HOST, + title=None, + data={CONF_HOST: HOST, KEY_MAC: HOST}, + ) + config_entry.add_to_hass(hass) + entity_registry = er.async_get(hass) + device_registry = dr.async_get(hass) + + type(mock_daikin).mac = PropertyMock(return_value=HOST) + type(mock_daikin).values = PropertyMock(return_value=INVALID_DATA) + + with patch( + "homeassistant.components.daikin.async_migrate_unique_id", return_value=None + ): + assert await hass.config_entries.async_setup(config_entry.entry_id) + + await hass.async_block_till_done() + + assert config_entry.unique_id != MAC + + type(mock_daikin).mac = PropertyMock(return_value=MAC) + type(mock_daikin).values = PropertyMock(return_value=DATA) + + assert await hass.config_entries.async_reload(config_entry.entry_id) + await hass.async_block_till_done() + + assert ( + device_registry.async_get_device({}, {(KEY_MAC, MAC)}).name + == "DaikinAP00000" + ) + + assert device_registry.async_get_device({}, {(KEY_MAC, HOST)}).name is None + + assert entity_registry.async_get("climate.daikin_127_0_0_1").unique_id == HOST + assert entity_registry.async_get("switch.none_zone_1").unique_id.startswith( + HOST + ) + + assert entity_registry.async_get("climate.daikinap00000").unique_id == MAC + assert entity_registry.async_get( + "switch.daikinap00000_zone_1" + ).unique_id.startswith(MAC) + + assert await hass.config_entries.async_reload(config_entry.entry_id) + await hass.async_block_till_done() + + assert ( + device_registry.async_get_device({}, {(KEY_MAC, MAC)}).name == "DaikinAP00000" + ) + + assert entity_registry.async_get("climate.daikinap00000") is None + assert entity_registry.async_get("switch.daikinap00000_zone_1") is None + + assert entity_registry.async_get("climate.daikin_127_0_0_1").unique_id == MAC + assert entity_registry.async_get("switch.none_zone_1").unique_id.startswith(MAC) + + async def test_unique_id_migrate(hass: HomeAssistant, mock_daikin) -> None: """Test unique id migration.""" config_entry = MockConfigEntry( @@ -97,8 +161,41 @@ async def test_unique_id_migrate(hass: HomeAssistant, mock_daikin) -> None: assert entity_registry.async_get("switch.none_zone_1").unique_id.startswith(MAC) +async def test_client_update_connection_error( + hass: HomeAssistant, mock_daikin, freezer: FrozenDateTimeFactory +) -> None: + """Test client connection error on update.""" + config_entry = MockConfigEntry( + domain=DOMAIN, + unique_id=MAC, + data={CONF_HOST: HOST, KEY_MAC: MAC}, + ) + config_entry.add_to_hass(hass) + er.async_get(hass) + + type(mock_daikin).mac = PropertyMock(return_value=MAC) + type(mock_daikin).values = PropertyMock(return_value=DATA) + + await hass.config_entries.async_setup(config_entry.entry_id) + + api: DaikinApi = hass.data[DOMAIN][config_entry.entry_id] + + assert api.available is True + + type(mock_daikin).update_status.side_effect = ClientConnectionError + + freezer.tick(timedelta(seconds=90)) + async_fire_time_changed(hass) + + await hass.async_block_till_done() + + assert api.available is False + + assert mock_daikin.update_status.call_count == 2 + + async def test_client_connection_error(hass: HomeAssistant, mock_daikin) -> None: - """Test unique id migration.""" + """Test client connection error on setup.""" config_entry = MockConfigEntry( domain=DOMAIN, unique_id=MAC, @@ -114,7 +211,7 @@ async def test_client_connection_error(hass: HomeAssistant, mock_daikin) -> None async def test_timeout_error(hass: HomeAssistant, mock_daikin) -> None: - """Test unique id migration.""" + """Test timeout error on setup.""" config_entry = MockConfigEntry( domain=DOMAIN, unique_id=MAC,