From 568962552b03036c5d0209d8e8e21b11ce08ba43 Mon Sep 17 00:00:00 2001 From: Dermot Duffy Date: Tue, 26 Jan 2021 12:39:02 -0800 Subject: [PATCH] Remove hyperion entry from registry only when instances are fully removed (#44488) --- homeassistant/components/hyperion/light.py | 44 ++++++++----- tests/components/hyperion/test_light.py | 72 ++++++++++++++++------ 2 files changed, 80 insertions(+), 36 deletions(-) diff --git a/homeassistant/components/hyperion/light.py b/homeassistant/components/hyperion/light.py index 92c7c1f3c02..634cf2f0afd 100644 --- a/homeassistant/components/hyperion/light.py +++ b/homeassistant/components/hyperion/light.py @@ -29,7 +29,10 @@ from homeassistant.helpers.dispatcher import ( async_dispatcher_connect, async_dispatcher_send, ) -from homeassistant.helpers.entity_registry import async_get_registry +from homeassistant.helpers.entity_registry import ( + async_entries_for_config_entry, + async_get_registry, +) from homeassistant.helpers.typing import ( ConfigType, DiscoveryInfoType, @@ -235,33 +238,38 @@ async def async_setup_entry( async def async_instances_to_entities_raw(instances: List[Dict[str, Any]]) -> None: registry = await async_get_registry(hass) entities_to_add: List[HyperionLight] = [] - desired_unique_ids: Set[str] = set() + running_unique_ids: Set[str] = set() + stopped_unique_ids: Set[str] = set() server_id = cast(str, config_entry.unique_id) # In practice, an instance can be in 3 states as seen by this function: # - # * Exists, and is running: Add it to hass. - # * Exists, but is not running: Cannot add yet, but should not delete it either. - # It will show up as "unavailable". - # * No longer exists: Delete it from hass. + # * Exists, and is running: Should be present in HASS/registry. + # * Exists, but is not running: Cannot add it yet, but entity may have be + # registered from a previous time it was running. + # * No longer exists at all: Should not be present in HASS/registry. # Add instances that are missing. for instance in instances: instance_id = instance.get(const.KEY_INSTANCE) - if instance_id is None or not instance.get(const.KEY_RUNNING, False): + if instance_id is None: continue unique_id = get_hyperion_unique_id( server_id, instance_id, TYPE_HYPERION_LIGHT ) - desired_unique_ids.add(unique_id) - if unique_id in current_entities: + if not instance.get(const.KEY_RUNNING, False): + stopped_unique_ids.add(unique_id) + continue + running_unique_ids.add(unique_id) + + if unique_id in live_entities: continue hyperion_client = await async_create_connect_hyperion_client( host, port, instance=instance_id, token=token ) if not hyperion_client: continue - current_entities.add(unique_id) + live_entities.add(unique_id) entities_to_add.append( HyperionLight( unique_id, @@ -271,19 +279,21 @@ async def async_setup_entry( ) ) - # Delete instances that are no longer present on this server. - for unique_id in current_entities - desired_unique_ids: - current_entities.remove(unique_id) + # Remove entities that are are not running instances on Hyperion: + for unique_id in live_entities - running_unique_ids: + live_entities.remove(unique_id) async_dispatcher_send(hass, SIGNAL_INSTANCE_REMOVED.format(unique_id)) - entity_id = registry.async_get_entity_id(LIGHT_DOMAIN, DOMAIN, unique_id) - if entity_id: - registry.async_remove(entity_id) + + # Deregister instances that are no longer present on this server. + for entry in async_entries_for_config_entry(registry, config_entry.entry_id): + if entry.unique_id not in running_unique_ids.union(stopped_unique_ids): + registry.async_remove(entry.entity_id) async_add_entities(entities_to_add) # Readability note: This variable is kept alive in the context of the callback to # async_instances_to_entities below. - current_entities: Set[str] = set() + live_entities: Set[str] = set() await async_instances_to_entities_raw( hass.data[DOMAIN][config_entry.entry_id][CONF_ROOT_CLIENT].instances, diff --git a/tests/components/hyperion/test_light.py b/tests/components/hyperion/test_light.py index 9590d7be760..8d14555c42b 100644 --- a/tests/components/hyperion/test_light.py +++ b/tests/components/hyperion/test_light.py @@ -265,6 +265,8 @@ async def test_setup_config_entry_not_ready_load_state_fail( async def test_setup_config_entry_dynamic_instances(hass: HomeAssistantType) -> None: """Test dynamic changes in the omstamce configuration.""" + registry = await async_get_registry(hass) + config_entry = add_test_config_entry(hass) master_client = create_mock_client() @@ -283,28 +285,12 @@ async def test_setup_config_entry_dynamic_instances(hass: HomeAssistantType) -> assert hass.states.get(TEST_ENTITY_ID_1) is not None assert hass.states.get(TEST_ENTITY_ID_2) is not None - # Inject a new instances update (remove instance 1, add instance 3) assert master_client.set_callbacks.called + + # == Inject a new instances update (stop instance 1, add instance 3) instance_callback = master_client.set_callbacks.call_args[0][0][ f"{const.KEY_INSTANCE}-{const.KEY_UPDATE}" ] - with patch( - "homeassistant.components.hyperion.client.HyperionClient", - return_value=entity_client, - ): - instance_callback( - { - const.KEY_SUCCESS: True, - const.KEY_DATA: [TEST_INSTANCE_2, TEST_INSTANCE_3], - } - ) - await hass.async_block_till_done() - - assert hass.states.get(TEST_ENTITY_ID_1) is None - assert hass.states.get(TEST_ENTITY_ID_2) is not None - assert hass.states.get(TEST_ENTITY_ID_3) is not None - - # Inject a new instances update (re-add instance 1, but not running) with patch( "homeassistant.components.hyperion.client.HyperionClient", return_value=entity_client, @@ -325,7 +311,55 @@ async def test_setup_config_entry_dynamic_instances(hass: HomeAssistantType) -> assert hass.states.get(TEST_ENTITY_ID_2) is not None assert hass.states.get(TEST_ENTITY_ID_3) is not None - # Inject a new instances update (re-add instance 1, running) + # Instance 1 is stopped, it should still be registered. + assert registry.async_is_registered(TEST_ENTITY_ID_1) + + # == Inject a new instances update (remove instance 1) + assert master_client.set_callbacks.called + instance_callback = master_client.set_callbacks.call_args[0][0][ + f"{const.KEY_INSTANCE}-{const.KEY_UPDATE}" + ] + with patch( + "homeassistant.components.hyperion.client.HyperionClient", + return_value=entity_client, + ): + instance_callback( + { + const.KEY_SUCCESS: True, + const.KEY_DATA: [TEST_INSTANCE_2, TEST_INSTANCE_3], + } + ) + await hass.async_block_till_done() + + assert hass.states.get(TEST_ENTITY_ID_1) is None + assert hass.states.get(TEST_ENTITY_ID_2) is not None + assert hass.states.get(TEST_ENTITY_ID_3) is not None + + # Instance 1 is removed, it should not still be registered. + assert not registry.async_is_registered(TEST_ENTITY_ID_1) + + # == Inject a new instances update (re-add instance 1, but not running) + with patch( + "homeassistant.components.hyperion.client.HyperionClient", + return_value=entity_client, + ): + instance_callback( + { + const.KEY_SUCCESS: True, + const.KEY_DATA: [ + {**TEST_INSTANCE_1, "running": False}, + TEST_INSTANCE_2, + TEST_INSTANCE_3, + ], + } + ) + await hass.async_block_till_done() + + assert hass.states.get(TEST_ENTITY_ID_1) is None + assert hass.states.get(TEST_ENTITY_ID_2) is not None + assert hass.states.get(TEST_ENTITY_ID_3) is not None + + # == Inject a new instances update (re-add instance 1, running) with patch( "homeassistant.components.hyperion.client.HyperionClient", return_value=entity_client,