From c44a7fe358e7a4a85b3c4b102768a47784b2201b Mon Sep 17 00:00:00 2001 From: Jan Bouwhuis Date: Mon, 29 May 2023 15:44:09 +0200 Subject: [PATCH] Do not trigger reload when unloading or reloading MQTT entry (#93588) * Do not trigger reload when unloading MQTT entry * More cleanup * cleanup async_reload_manual_mqtt_items * Add test * Improve test * Remove unuse mocks from test * Add discovery item in test --- homeassistant/components/mqtt/__init__.py | 40 +----- homeassistant/components/mqtt/models.py | 2 - tests/components/mqtt/test_init.py | 154 ++++++++++++++++++++++ 3 files changed, 155 insertions(+), 41 deletions(-) diff --git a/homeassistant/components/mqtt/__init__.py b/homeassistant/components/mqtt/__init__.py index d3806044fcc..3fb6c8d2c48 100644 --- a/homeassistant/components/mqtt/__init__.py +++ b/homeassistant/components/mqtt/__init__.py @@ -29,10 +29,7 @@ from homeassistant.helpers import config_validation as cv, event, template from homeassistant.helpers.device_registry import DeviceEntry from homeassistant.helpers.dispatcher import async_dispatcher_connect from homeassistant.helpers.entity_platform import async_get_platforms -from homeassistant.helpers.reload import ( - async_integration_yaml_config, - async_reload_integration_platforms, -) +from homeassistant.helpers.reload import async_integration_yaml_config from homeassistant.helpers.service import async_register_admin_service from homeassistant.helpers.typing import ConfigType @@ -374,7 +371,6 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: conf: ConfigType, ) -> None: """Forward the config entry setup to the platforms and set up discovery.""" - reload_manual_setup: bool = False # Local import to avoid circular dependencies # pylint: disable-next=import-outside-toplevel from . import device_automation, tag @@ -399,35 +395,12 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: ) # Setup reload service after all platforms have loaded await async_setup_reload_service() - # When the entry is reloaded, also reload manual set up items to enable MQTT - if mqtt_data.reload_entry: - mqtt_data.reload_entry = False - reload_manual_setup = True - - # When the entry was disabled before, reload manual set up items to enable - # MQTT again - if mqtt_data.reload_needed: - mqtt_data.reload_needed = False - reload_manual_setup = True - - if reload_manual_setup: - await async_reload_manual_mqtt_items(hass) await async_forward_entry_setup_and_setup_discovery(entry, conf) return True -async def async_reload_manual_mqtt_items(hass: HomeAssistant) -> None: - """Reload manual configured MQTT items.""" - await hass.services.async_call( - DOMAIN, - SERVICE_RELOAD, - {}, - blocking=True, - ) - - @websocket_api.websocket_command( {vol.Required("type"): "mqtt/device/debug_info", vol.Required("device_id"): str} ) @@ -570,17 +543,6 @@ async def async_unload_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: # Cleanup listeners mqtt_client.cleanup() - # Trigger reload manual MQTT items at entry setup - if (mqtt_entry_status := mqtt_config_entry_enabled(hass)) is False: - # The entry is disabled reload legacy manual items when - # the entry is enabled again - mqtt_data.reload_needed = True - elif mqtt_entry_status is True: - # The entry is reloaded: - # Trigger re-fetching the yaml config at entry setup - mqtt_data.reload_entry = True - # Reload the legacy yaml platform to make entities unavailable - await async_reload_integration_platforms(hass, DOMAIN, RELOADABLE_PLATFORMS) # Cleanup entity registry hooks registry_hooks = mqtt_data.discovery_registry_hooks while registry_hooks: diff --git a/homeassistant/components/mqtt/models.py b/homeassistant/components/mqtt/models.py index eac333e2a7a..aeae184dc89 100644 --- a/homeassistant/components/mqtt/models.py +++ b/homeassistant/components/mqtt/models.py @@ -307,11 +307,9 @@ class MqttData: integration_unsubscribe: dict[str, CALLBACK_TYPE] = field(default_factory=dict) last_discovery: float = 0.0 reload_dispatchers: list[CALLBACK_TYPE] = field(default_factory=list) - reload_entry: bool = False reload_handlers: dict[str, Callable[[], Coroutine[Any, Any, None]]] = field( default_factory=dict ) - reload_needed: bool = False state_write_requests: EntityTopicState = field(default_factory=EntityTopicState) subscriptions_to_restore: list[Subscription] = field(default_factory=list) tags: dict[str, dict[str, MQTTTagScanner]] = field(default_factory=dict) diff --git a/tests/components/mqtt/test_init.py b/tests/components/mqtt/test_init.py index fa8d2e42dda..adbef1464a2 100644 --- a/tests/components/mqtt/test_init.py +++ b/tests/components/mqtt/test_init.py @@ -23,6 +23,8 @@ from homeassistant.const import ( EVENT_HOMEASSISTANT_STARTED, EVENT_HOMEASSISTANT_STOP, SERVICE_RELOAD, + STATE_UNAVAILABLE, + STATE_UNKNOWN, Platform, UnitOfTemperature, ) @@ -3737,3 +3739,155 @@ async def test_link_config_entry( ) await hass.async_block_till_done() assert _check_entities() == 2 + + +@pytest.mark.parametrize( + "hass_config", + [ + { + "mqtt": { + "sensor": [ + { + "name": "test_manual1", + "unique_id": "test_manual_unique_id123", + "state_topic": "test-topic_manual1", + }, + { + "name": "test_manual3", + "unique_id": "test_manual_unique_id789", + "state_topic": "test-topic_manual3", + }, + ] + } + } + ], +) +async def test_reload_config_entry( + hass: HomeAssistant, + mqtt_mock_entry: MqttMockHAClientGenerator, +) -> None: + """Test manual entities reloaded and set up correctly.""" + await mqtt_mock_entry() + + # set up item through discovery + config_discovery = { + "name": "test_discovery", + "unique_id": "test_discovery_unique456", + "state_topic": "test-topic_discovery", + } + async_fire_mqtt_message( + hass, "homeassistant/sensor/bla/config", json.dumps(config_discovery) + ) + await hass.async_block_till_done() + await hass.async_block_till_done() + assert hass.states.get("sensor.test_discovery") is not None + + entry = hass.config_entries.async_entries(mqtt.DOMAIN)[0] + + def _check_entities() -> int: + entities: list[Entity] = [] + mqtt_platforms = async_get_platforms(hass, mqtt.DOMAIN) + for mqtt_platform in mqtt_platforms: + assert mqtt_platform.config_entry is entry + entities += (entity for entity in mqtt_platform.entities.values()) + + return len(entities) + + # assert on initial set up manual items + + async_fire_mqtt_message(hass, "test-topic_manual1", "manual1_intial") + async_fire_mqtt_message(hass, "test-topic_manual3", "manual3_intial") + + assert (state := hass.states.get("sensor.test_manual1")) is not None + assert state.attributes["friendly_name"] == "test_manual1" + assert state.state == "manual1_intial" + assert (state := hass.states.get("sensor.test_manual3")) is not None + assert state.attributes["friendly_name"] == "test_manual3" + assert state.state == "manual3_intial" + assert _check_entities() == 3 + + # Reload the entry with a new configuration.yaml + # Mock configuration.yaml was updated + # The first item was updated, a new item was added, an item was removed + hass_config_new = { + "mqtt": { + "sensor": [ + { + "name": "test_manual1_updated", + "unique_id": "test_manual_unique_id123", + "state_topic": "test-topic_manual1_updated", + }, + { + "name": "test_manual2_new", + "unique_id": "test_manual_unique_id456", + "state_topic": "test-topic_manual2", + }, + ] + } + } + with patch( + "homeassistant.config.load_yaml_config_file", return_value=hass_config_new + ): + assert await hass.config_entries.async_reload(entry.entry_id) + assert entry.state is ConfigEntryState.LOADED + await hass.async_block_till_done() + + assert (state := hass.states.get("sensor.test_manual1")) is not None + assert state.attributes["friendly_name"] == "test_manual1_updated" + assert state.state == STATE_UNKNOWN + assert (state := hass.states.get("sensor.test_manual2_new")) is not None + assert state.attributes["friendly_name"] == "test_manual2_new" + assert state.state is STATE_UNKNOWN + # State of test_manual3 is still loaded but is unavailable + assert (state := hass.states.get("sensor.test_manual3")) is not None + assert state.state is STATE_UNAVAILABLE + assert (state := hass.states.get("sensor.test_discovery")) is not None + assert state.state is STATE_UNAVAILABLE + # The entity is not loaded anymore + assert _check_entities() == 2 + + async_fire_mqtt_message(hass, "test-topic_manual1_updated", "manual1_update") + async_fire_mqtt_message(hass, "test-topic_manual2", "manual2_update") + async_fire_mqtt_message(hass, "test-topic_manual3", "manual3_update") + + assert (state := hass.states.get("sensor.test_manual1")) is not None + assert state.state == "manual1_update" + assert (state := hass.states.get("sensor.test_manual2_new")) is not None + assert state.state == "manual2_update" + assert (state := hass.states.get("sensor.test_manual3")) is not None + assert state.state is STATE_UNAVAILABLE + + # Reload manual configured items and assert again + with patch( + "homeassistant.config.load_yaml_config_file", return_value=hass_config_new + ): + await hass.services.async_call( + "mqtt", + SERVICE_RELOAD, + {}, + blocking=True, + ) + await hass.async_block_till_done() + + assert (state := hass.states.get("sensor.test_manual1")) is not None + assert state.attributes["friendly_name"] == "test_manual1_updated" + assert state.state == STATE_UNKNOWN + assert (state := hass.states.get("sensor.test_manual2_new")) is not None + assert state.attributes["friendly_name"] == "test_manual2_new" + assert state.state == STATE_UNKNOWN + assert (state := hass.states.get("sensor.test_manual3")) is not None + assert state.state == STATE_UNAVAILABLE + assert _check_entities() == 2 + + async_fire_mqtt_message( + hass, "test-topic_manual1_updated", "manual1_update_after_reload" + ) + async_fire_mqtt_message(hass, "test-topic_manual2", "manual2_update_after_reload") + async_fire_mqtt_message(hass, "test-topic_manual3", "manual3_update_after_reload") + + assert (state := hass.states.get("sensor.test_manual1")) is not None + assert state.state == "manual1_update_after_reload" + assert (state := hass.states.get("sensor.test_manual2_new")) is not None + assert state.state == "manual2_update_after_reload" + assert (state := hass.states.get("sensor.test_manual3")) is not None + assert state.state is STATE_UNAVAILABLE