From d149bffb070934fc2d5a6430004f22085e4f6881 Mon Sep 17 00:00:00 2001 From: Jan Bouwhuis Date: Thu, 19 Oct 2023 17:34:43 +0200 Subject: [PATCH] Do not fail MQTT setup if lights configured via yaml can't be validated (#101649) * Add light * Deduplicate code * Follow up comment --- .../components/mqtt/config_integration.py | 6 +- .../components/mqtt/light/__init__.py | 41 ++---- .../components/mqtt/light/schema_basic.py | 17 +-- .../components/mqtt/light/schema_json.py | 17 +-- .../components/mqtt/light/schema_template.py | 17 +-- homeassistant/components/mqtt/mixins.py | 135 ++++++++++-------- tests/components/mqtt/test_init.py | 17 +-- tests/components/mqtt/test_light.py | 5 +- tests/components/mqtt/test_light_json.py | 18 +-- 9 files changed, 107 insertions(+), 166 deletions(-) diff --git a/homeassistant/components/mqtt/config_integration.py b/homeassistant/components/mqtt/config_integration.py index 975ddfe6386..da42e6f42d5 100644 --- a/homeassistant/components/mqtt/config_integration.py +++ b/homeassistant/components/mqtt/config_integration.py @@ -26,7 +26,6 @@ from . import ( humidifier as humidifier_platform, image as image_platform, lawn_mower as lawn_mower_platform, - light as light_platform, lock as lock_platform, number as number_platform, scene as scene_platform, @@ -100,14 +99,11 @@ CONFIG_SCHEMA_BASE = vol.Schema( cv.ensure_list, [lawn_mower_platform.PLATFORM_SCHEMA_MODERN], # type: ignore[has-type] ), + Platform.LIGHT.value: vol.All(cv.ensure_list, [dict]), Platform.LOCK.value: vol.All( cv.ensure_list, [lock_platform.PLATFORM_SCHEMA_MODERN], # type: ignore[has-type] ), - Platform.LIGHT.value: vol.All( - cv.ensure_list, - [light_platform.PLATFORM_SCHEMA_MODERN], # type: ignore[has-type] - ), Platform.NUMBER.value: vol.All( cv.ensure_list, [number_platform.PLATFORM_SCHEMA_MODERN], # type: ignore[has-type] diff --git a/homeassistant/components/mqtt/light/__init__.py b/homeassistant/components/mqtt/light/__init__.py index 2c70490ac5e..15431616658 100644 --- a/homeassistant/components/mqtt/light/__init__.py +++ b/homeassistant/components/mqtt/light/__init__.py @@ -1,7 +1,6 @@ """Support for MQTT lights.""" from __future__ import annotations -import functools from typing import Any import voluptuous as vol @@ -10,24 +9,24 @@ from homeassistant.components import light from homeassistant.config_entries import ConfigEntry from homeassistant.core import HomeAssistant from homeassistant.helpers.entity_platform import AddEntitiesCallback -from homeassistant.helpers.typing import ConfigType, DiscoveryInfoType +from homeassistant.helpers.typing import ConfigType -from ..mixins import async_setup_entry_helper +from ..mixins import async_mqtt_entry_helper from .schema import CONF_SCHEMA, MQTT_LIGHT_SCHEMA_SCHEMA from .schema_basic import ( DISCOVERY_SCHEMA_BASIC, PLATFORM_SCHEMA_MODERN_BASIC, - async_setup_entity_basic, + MqttLight, ) from .schema_json import ( DISCOVERY_SCHEMA_JSON, PLATFORM_SCHEMA_MODERN_JSON, - async_setup_entity_json, + MqttLightJson, ) from .schema_template import ( DISCOVERY_SCHEMA_TEMPLATE, PLATFORM_SCHEMA_MODERN_TEMPLATE, - async_setup_entity_template, + MqttLightTemplate, ) @@ -70,25 +69,13 @@ async def async_setup_entry( async_add_entities: AddEntitiesCallback, ) -> None: """Set up MQTT lights through YAML and through MQTT discovery.""" - setup = functools.partial( - _async_setup_entity, hass, async_add_entities, config_entry=config_entry - ) - await async_setup_entry_helper(hass, light.DOMAIN, setup, DISCOVERY_SCHEMA) - - -async def _async_setup_entity( - hass: HomeAssistant, - async_add_entities: AddEntitiesCallback, - config: ConfigType, - config_entry: ConfigEntry, - discovery_data: DiscoveryInfoType | None = None, -) -> None: - """Set up a MQTT Light.""" - setup_entity = { - "basic": async_setup_entity_basic, - "json": async_setup_entity_json, - "template": async_setup_entity_template, - } - await setup_entity[config[CONF_SCHEMA]]( - hass, config, async_add_entities, config_entry, discovery_data + await async_mqtt_entry_helper( + hass, + config_entry, + None, + light.DOMAIN, + async_add_entities, + DISCOVERY_SCHEMA, + PLATFORM_SCHEMA_MODERN, + {"basic": MqttLight, "json": MqttLightJson, "template": MqttLightTemplate}, ) diff --git a/homeassistant/components/mqtt/light/schema_basic.py b/homeassistant/components/mqtt/light/schema_basic.py index 65c05501658..2ca0a7e7e47 100644 --- a/homeassistant/components/mqtt/light/schema_basic.py +++ b/homeassistant/components/mqtt/light/schema_basic.py @@ -28,7 +28,6 @@ from homeassistant.components.light import ( LightEntityFeature, valid_supported_color_modes, ) -from homeassistant.config_entries import ConfigEntry from homeassistant.const import ( CONF_NAME, CONF_OPTIMISTIC, @@ -36,11 +35,10 @@ from homeassistant.const import ( CONF_PAYLOAD_ON, STATE_ON, ) -from homeassistant.core import HomeAssistant, callback +from homeassistant.core import callback import homeassistant.helpers.config_validation as cv -from homeassistant.helpers.entity_platform import AddEntitiesCallback from homeassistant.helpers.restore_state import RestoreEntity -from homeassistant.helpers.typing import ConfigType, DiscoveryInfoType +from homeassistant.helpers.typing import ConfigType import homeassistant.util.color as color_util from .. import subscription @@ -228,17 +226,6 @@ DISCOVERY_SCHEMA_BASIC = vol.All( ) -async def async_setup_entity_basic( - hass: HomeAssistant, - config: ConfigType, - async_add_entities: AddEntitiesCallback, - config_entry: ConfigEntry, - discovery_data: DiscoveryInfoType | None, -) -> None: - """Set up a MQTT Light.""" - async_add_entities([MqttLight(hass, config, config_entry, discovery_data)]) - - class MqttLight(MqttEntity, LightEntity, RestoreEntity): """Representation of a MQTT light.""" diff --git a/homeassistant/components/mqtt/light/schema_json.py b/homeassistant/components/mqtt/light/schema_json.py index 462280b1516..6f70ff34051 100644 --- a/homeassistant/components/mqtt/light/schema_json.py +++ b/homeassistant/components/mqtt/light/schema_json.py @@ -32,7 +32,6 @@ from homeassistant.components.light import ( filter_supported_color_modes, valid_supported_color_modes, ) -from homeassistant.config_entries import ConfigEntry from homeassistant.const import ( CONF_BRIGHTNESS, CONF_COLOR_TEMP, @@ -44,12 +43,11 @@ from homeassistant.const import ( CONF_XY, STATE_ON, ) -from homeassistant.core import HomeAssistant, callback +from homeassistant.core import callback import homeassistant.helpers.config_validation as cv -from homeassistant.helpers.entity_platform import AddEntitiesCallback from homeassistant.helpers.json import json_dumps from homeassistant.helpers.restore_state import RestoreEntity -from homeassistant.helpers.typing import ConfigType, DiscoveryInfoType +from homeassistant.helpers.typing import ConfigType import homeassistant.util.color as color_util from homeassistant.util.json import json_loads_object @@ -166,17 +164,6 @@ PLATFORM_SCHEMA_MODERN_JSON = vol.All( ) -async def async_setup_entity_json( - hass: HomeAssistant, - config: ConfigType, - async_add_entities: AddEntitiesCallback, - config_entry: ConfigEntry, - discovery_data: DiscoveryInfoType | None, -) -> None: - """Set up a MQTT JSON Light.""" - async_add_entities([MqttLightJson(hass, config, config_entry, discovery_data)]) - - class MqttLightJson(MqttEntity, LightEntity, RestoreEntity): """Representation of a MQTT JSON light.""" diff --git a/homeassistant/components/mqtt/light/schema_template.py b/homeassistant/components/mqtt/light/schema_template.py index a225ce43efa..e4900053fb3 100644 --- a/homeassistant/components/mqtt/light/schema_template.py +++ b/homeassistant/components/mqtt/light/schema_template.py @@ -20,7 +20,6 @@ from homeassistant.components.light import ( LightEntityFeature, filter_supported_color_modes, ) -from homeassistant.config_entries import ConfigEntry from homeassistant.const import ( CONF_NAME, CONF_OPTIMISTIC, @@ -28,11 +27,10 @@ from homeassistant.const import ( STATE_OFF, STATE_ON, ) -from homeassistant.core import HomeAssistant, callback +from homeassistant.core import callback import homeassistant.helpers.config_validation as cv -from homeassistant.helpers.entity_platform import AddEntitiesCallback from homeassistant.helpers.restore_state import RestoreEntity -from homeassistant.helpers.typing import ConfigType, DiscoveryInfoType, TemplateVarsType +from homeassistant.helpers.typing import ConfigType, TemplateVarsType import homeassistant.util.color as color_util from .. import subscription @@ -113,17 +111,6 @@ DISCOVERY_SCHEMA_TEMPLATE = vol.All( ) -async def async_setup_entity_template( - hass: HomeAssistant, - config: ConfigType, - async_add_entities: AddEntitiesCallback, - config_entry: ConfigEntry, - discovery_data: DiscoveryInfoType | None, -) -> None: - """Set up a MQTT Template light.""" - async_add_entities([MqttLightTemplate(hass, config, config_entry, discovery_data)]) - - class MqttLightTemplate(MqttEntity, LightEntity, RestoreEntity): """Representation of a MQTT Template light.""" diff --git a/homeassistant/components/mqtt/mixins.py b/homeassistant/components/mqtt/mixins.py index 1138663c851..ddc2703d820 100644 --- a/homeassistant/components/mqtt/mixins.py +++ b/homeassistant/components/mqtt/mixins.py @@ -4,6 +4,7 @@ from __future__ import annotations from abc import ABC, abstractmethod import asyncio from collections.abc import Callable, Coroutine +import functools from functools import partial, wraps import logging from typing import TYPE_CHECKING, Any, Protocol, cast, final @@ -81,6 +82,7 @@ from .const import ( CONF_OBJECT_ID, CONF_ORIGIN, CONF_QOS, + CONF_SCHEMA, CONF_SUGGESTED_AREA, CONF_SW_VERSION, CONF_TOPIC, @@ -272,6 +274,38 @@ def async_handle_schema_error( ) +async def _async_discover( + hass: HomeAssistant, + domain: str, + async_setup: partial[Coroutine[Any, Any, None]], + discovery_payload: MQTTDiscoveryPayload, +) -> None: + """Discover and add an MQTT entity, automation or tag.""" + if not mqtt_config_entry_enabled(hass): + _LOGGER.warning( + ( + "MQTT integration is disabled, skipping setup of discovered item " + "MQTT %s, payload %s" + ), + domain, + discovery_payload, + ) + return + discovery_data = discovery_payload.discovery_data + try: + await async_setup(discovery_payload) + except vol.Invalid as err: + discovery_hash = discovery_data[ATTR_DISCOVERY_HASH] + clear_discovery_hash(hass, discovery_hash) + async_dispatcher_send(hass, MQTT_DISCOVERY_DONE.format(discovery_hash), None) + async_handle_schema_error(discovery_payload, err) + except Exception: + discovery_hash = discovery_data[ATTR_DISCOVERY_HASH] + clear_discovery_hash(hass, discovery_hash) + async_dispatcher_send(hass, MQTT_DISCOVERY_DONE.format(discovery_hash), None) + raise + + async def async_setup_entry_helper( hass: HomeAssistant, domain: str, @@ -281,43 +315,25 @@ async def async_setup_entry_helper( """Set up entity, automation or tag creation dynamically through MQTT discovery.""" mqtt_data = get_mqtt_data(hass) - async def async_discover(discovery_payload: MQTTDiscoveryPayload) -> None: - """Discover and add an MQTT entity, automation or tag.""" - if not mqtt_config_entry_enabled(hass): - _LOGGER.warning( - ( - "MQTT integration is disabled, skipping setup of discovered item " - "MQTT %s, payload %s" - ), - domain, - discovery_payload, - ) - return - discovery_data = discovery_payload.discovery_data - try: - config: DiscoveryInfoType = discovery_schema(discovery_payload) - await async_setup(config, discovery_data=discovery_data) - except vol.Invalid as err: - discovery_hash = discovery_data[ATTR_DISCOVERY_HASH] - clear_discovery_hash(hass, discovery_hash) - async_dispatcher_send( - hass, MQTT_DISCOVERY_DONE.format(discovery_hash), None - ) - async_handle_schema_error(discovery_payload, err) - except Exception: - discovery_hash = discovery_data[ATTR_DISCOVERY_HASH] - clear_discovery_hash(hass, discovery_hash) - async_dispatcher_send( - hass, MQTT_DISCOVERY_DONE.format(discovery_hash), None - ) - raise + async def async_setup_from_discovery( + discovery_payload: MQTTDiscoveryPayload, + ) -> None: + """Set up an MQTT entity, automation or tag from discovery.""" + config: DiscoveryInfoType = discovery_schema(discovery_payload) + await async_setup(config, discovery_data=discovery_payload.discovery_data) mqtt_data.reload_dispatchers.append( async_dispatcher_connect( - hass, MQTT_DISCOVERY_NEW.format(domain, "mqtt"), async_discover + hass, + MQTT_DISCOVERY_NEW.format(domain, "mqtt"), + functools.partial( + _async_discover, hass, domain, async_setup_from_discovery + ), ) ) + # The setup of manual configured MQTT entities will be migrated to async_mqtt_entry_helper. + # The following setup code will be cleaned up after the last entity platform has been migrated. async def _async_setup_entities() -> None: """Set up MQTT items from configuration.yaml.""" mqtt_data = get_mqtt_data(hass) @@ -342,54 +358,43 @@ async def async_setup_entry_helper( async def async_mqtt_entry_helper( hass: HomeAssistant, entry: ConfigEntry, - entity_class: type[MqttEntity], + entity_class: type[MqttEntity] | None, domain: str, async_add_entities: AddEntitiesCallback, discovery_schema: vol.Schema, platform_schema_modern: vol.Schema, + schema_class_mapping: dict[str, type[MqttEntity]] | None = None, ) -> None: """Set up entity, automation or tag creation dynamically through MQTT discovery.""" mqtt_data = get_mqtt_data(hass) - async def async_discover(discovery_payload: MQTTDiscoveryPayload) -> None: - """Discover and add an MQTT entity, automation or tag.""" - if not mqtt_config_entry_enabled(hass): - _LOGGER.warning( - ( - "MQTT integration is disabled, skipping setup of discovered item " - "MQTT %s, payload %s" - ), - domain, - discovery_payload, - ) - return - discovery_data = discovery_payload.discovery_data - try: - config: DiscoveryInfoType = discovery_schema(discovery_payload) - async_add_entities([entity_class(hass, config, entry, discovery_data)]) - except vol.Invalid as err: - discovery_hash = discovery_data[ATTR_DISCOVERY_HASH] - clear_discovery_hash(hass, discovery_hash) - async_dispatcher_send( - hass, MQTT_DISCOVERY_DONE.format(discovery_hash), None - ) - async_handle_schema_error(discovery_payload, err) - except Exception: - discovery_hash = discovery_data[ATTR_DISCOVERY_HASH] - clear_discovery_hash(hass, discovery_hash) - async_dispatcher_send( - hass, MQTT_DISCOVERY_DONE.format(discovery_hash), None - ) - raise + async def async_setup_from_discovery( + discovery_payload: MQTTDiscoveryPayload, + ) -> None: + """Set up an MQTT entity from discovery.""" + nonlocal entity_class + config: DiscoveryInfoType = discovery_schema(discovery_payload) + if schema_class_mapping is not None: + entity_class = schema_class_mapping[config[CONF_SCHEMA]] + if TYPE_CHECKING: + assert entity_class is not None + async_add_entities( + [entity_class(hass, config, entry, discovery_payload.discovery_data)] + ) mqtt_data.reload_dispatchers.append( async_dispatcher_connect( - hass, MQTT_DISCOVERY_NEW.format(domain, "mqtt"), async_discover + hass, + MQTT_DISCOVERY_NEW.format(domain, "mqtt"), + functools.partial( + _async_discover, hass, domain, async_setup_from_discovery + ), ) ) async def _async_setup_entities() -> None: """Set up MQTT items from configuration.yaml.""" + nonlocal entity_class mqtt_data = get_mqtt_data(hass) if not (config_yaml := mqtt_data.config): return @@ -404,6 +409,10 @@ async def async_mqtt_entry_helper( for yaml_config in yaml_configs: try: config = platform_schema_modern(yaml_config) + if schema_class_mapping is not None: + entity_class = schema_class_mapping[config[CONF_SCHEMA]] + if TYPE_CHECKING: + assert entity_class is not None entities.append(entity_class(hass, config, entry, None)) except vol.Invalid as ex: error = str(ex) diff --git a/tests/components/mqtt/test_init.py b/tests/components/mqtt/test_init.py index 2c6ee5d1b20..dc81e3d82b9 100644 --- a/tests/components/mqtt/test_init.py +++ b/tests/components/mqtt/test_init.py @@ -2114,35 +2114,30 @@ async def test_handle_message_callback( } ], ) -@patch("homeassistant.components.mqtt.PLATFORMS", []) +@patch("homeassistant.components.mqtt.PLATFORMS", [Platform.LIGHT]) async def test_setup_manual_mqtt_with_platform_key( hass: HomeAssistant, mqtt_mock_entry: MqttMockHAClientGenerator, caplog: pytest.LogCaptureFixture, ) -> None: """Test set up a manual MQTT item with a platform key.""" - with pytest.raises(AssertionError): - await mqtt_mock_entry() + assert await mqtt_mock_entry() assert ( - "Invalid config for [mqtt]: [platform] is an invalid option for [mqtt]" + "extra keys not allowed @ data['platform'] for manual configured MQTT light item" in caplog.text ) @pytest.mark.parametrize("hass_config", [{mqtt.DOMAIN: {"light": {"name": "test"}}}]) -@patch("homeassistant.components.mqtt.PLATFORMS", []) +@patch("homeassistant.components.mqtt.PLATFORMS", [Platform.LIGHT]) async def test_setup_manual_mqtt_with_invalid_config( hass: HomeAssistant, mqtt_mock_entry: MqttMockHAClientGenerator, caplog: pytest.LogCaptureFixture, ) -> None: """Test set up a manual MQTT item with an invalid config.""" - with pytest.raises(AssertionError): - await mqtt_mock_entry() - assert ( - "Invalid config for [mqtt]: required key not provided @ data['mqtt'][0]['light'][0]['command_topic']. " - "Got None. (See ?, line ?)" in caplog.text - ) + assert await mqtt_mock_entry() + assert "required key not provided" in caplog.text @patch("homeassistant.components.mqtt.PLATFORMS", []) diff --git a/tests/components/mqtt/test_light.py b/tests/components/mqtt/test_light.py index 58d37943403..7de6c08f269 100644 --- a/tests/components/mqtt/test_light.py +++ b/tests/components/mqtt/test_light.py @@ -253,9 +253,8 @@ async def test_fail_setup_if_no_command_topic( caplog: pytest.LogCaptureFixture, ) -> None: """Test if command fails with command topic.""" - with pytest.raises(AssertionError): - await mqtt_mock_entry() - assert "Invalid config for [mqtt]: required key not provided" in caplog.text + assert await mqtt_mock_entry() + assert "required key not provided" in caplog.text @pytest.mark.parametrize( diff --git a/tests/components/mqtt/test_light_json.py b/tests/components/mqtt/test_light_json.py index 7df4dbc6e82..e7471829856 100644 --- a/tests/components/mqtt/test_light_json.py +++ b/tests/components/mqtt/test_light_json.py @@ -197,9 +197,8 @@ async def test_fail_setup_if_no_command_topic( caplog: pytest.LogCaptureFixture, ) -> None: """Test if setup fails with no command topic.""" - with pytest.raises(AssertionError): - await mqtt_mock_entry() - assert "Invalid config for [mqtt]: required key not provided" in caplog.text + assert await mqtt_mock_entry() + assert "required key not provided" in caplog.text @pytest.mark.parametrize( @@ -217,12 +216,8 @@ async def test_fail_setup_if_color_mode_deprecated( caplog: pytest.LogCaptureFixture, ) -> None: """Test if setup fails if color mode is combined with deprecated config keys.""" - with pytest.raises(AssertionError): - await mqtt_mock_entry() - assert ( - "Invalid config for [mqtt]: color_mode must not be combined with any of" - in caplog.text - ) + assert await mqtt_mock_entry() + assert "color_mode must not be combined with any of" in caplog.text @pytest.mark.parametrize( @@ -250,7 +245,7 @@ async def test_fail_setup_if_color_mode_deprecated( COLOR_MODES_CONFIG, ({"supported_color_modes": ["unknown"]},), ), - "Invalid config for [mqtt]: value must be one of [ None: """Test if setup fails if supported color modes is invalid.""" - with pytest.raises(AssertionError): - await mqtt_mock_entry() + assert await mqtt_mock_entry() assert error in caplog.text