From 8c03f805e55054d05e39f12b96a09116bbd1d72d Mon Sep 17 00:00:00 2001 From: emontnemery Date: Sun, 22 Sep 2019 17:44:43 +0200 Subject: [PATCH 1/5] Improve validation of device automation config --- .../components/automation/__init__.py | 68 ++++++++++++++- homeassistant/components/automation/device.py | 14 +++- homeassistant/components/config/__init__.py | 15 +++- homeassistant/components/config/automation.py | 7 +- .../components/device_automation/__init__.py | 82 +++++++++++++++++++ homeassistant/config.py | 32 +++++--- homeassistant/helpers/condition.py | 23 ++++++ homeassistant/helpers/config_validation.py | 2 +- homeassistant/helpers/script.py | 29 ++++--- homeassistant/loader.py | 2 +- 10 files changed, 240 insertions(+), 34 deletions(-) diff --git a/homeassistant/components/automation/__init__.py b/homeassistant/components/automation/__init__.py index f0529f126f1e73..891601e082e826 100644 --- a/homeassistant/components/automation/__init__.py +++ b/homeassistant/components/automation/__init__.py @@ -23,15 +23,21 @@ SERVICE_TURN_ON, STATE_ON, ) +from homeassistant.config import async_log_exception, config_without_domain from homeassistant.core import Context, CoreState from homeassistant.exceptions import HomeAssistantError -from homeassistant.helpers import condition, extract_domain_configs, script +from homeassistant.helpers import ( + condition, + config_per_platform, + extract_domain_configs, + script, +) import homeassistant.helpers.config_validation as cv from homeassistant.helpers.config_validation import ENTITY_SERVICE_SCHEMA from homeassistant.helpers.entity import ToggleEntity from homeassistant.helpers.entity_component import EntityComponent from homeassistant.helpers.restore_state import RestoreEntity -from homeassistant.loader import bind_hass +from homeassistant.loader import bind_hass, IntegrationNotFound from homeassistant.util.dt import parse_datetime, utcnow @@ -372,6 +378,64 @@ def device_state_attributes(self): return {CONF_ID: self._id} +async def async_validate_config_item(hass, config): + """Validate config item. + + This method is a coroutine. + """ + p_validated = PLATFORM_SCHEMA(config) + + triggers = [] + for trigger in p_validated[CONF_TRIGGER]: + trigger_platform = importlib.import_module( + ".{}".format(trigger[CONF_PLATFORM]), __name__ + ) + if hasattr(trigger_platform, "async_validate_trigger_config"): + trigger = await trigger_platform.async_validate_trigger_config( + hass, trigger + ) + triggers.append(trigger) + p_validated[CONF_TRIGGER] = triggers + + if CONF_CONDITION in p_validated: + conditions = [] + for cond in p_validated[CONF_CONDITION]: + cond = await condition.async_validate_condition_config(hass, cond) + conditions.append(cond) + p_validated[CONF_CONDITION] = conditions + + actions = [] + for action in p_validated[CONF_ACTION]: + action = await script.async_validate_action_config(hass, action) + actions.append(action) + p_validated[CONF_ACTION] = actions + + return p_validated + + +async def async_validate_config(hass, config): + """Validate config. + + This method is a coroutine. + """ + automations = [] + for _, p_config in config_per_platform(config, DOMAIN): + try: + p_validated = await async_validate_config_item(hass, p_config) + except (vol.Invalid, HomeAssistantError, IntegrationNotFound) as ex: + async_log_exception(ex, DOMAIN, p_config, hass) + continue + + automations.append(p_validated) + + # Create a copy of the configuration with all config for current + # component removed and add validated config back in. + config = config_without_domain(config, DOMAIN) + config[DOMAIN] = automations + + return config + + async def _async_process_config(hass, config, component): """Process config and add automations. diff --git a/homeassistant/components/automation/device.py b/homeassistant/components/automation/device.py index b090484ab6785d..e25b2a677f90c6 100644 --- a/homeassistant/components/automation/device.py +++ b/homeassistant/components/automation/device.py @@ -1,8 +1,8 @@ """Offer device oriented automation.""" import voluptuous as vol +import homeassistant.components.device_automation as device_automation from homeassistant.const import CONF_DOMAIN, CONF_PLATFORM -from homeassistant.loader import async_get_integration # mypy: allow-untyped-defs, no-check-untyped-defs @@ -13,8 +13,14 @@ ) +async def async_validate_trigger_config(hass, config): + """Validate config. + + This method is a coroutine. + """ + return await device_automation.async_validate_trigger_config(hass, config) + + async def async_trigger(hass, config, action, automation_info): """Listen for trigger.""" - integration = await async_get_integration(hass, config[CONF_DOMAIN]) - platform = integration.get_platform("device_automation") - return await platform.async_trigger(hass, config, action, automation_info) + return await device_automation.async_trigger(hass, config, action, automation_info) diff --git a/homeassistant/components/config/__init__.py b/homeassistant/components/config/__init__.py index 6d4b465fceb3dc..6aca65964dd940 100644 --- a/homeassistant/components/config/__init__.py +++ b/homeassistant/components/config/__init__.py @@ -6,9 +6,10 @@ import voluptuous as vol from homeassistant.core import callback +from homeassistant.components.http import HomeAssistantView from homeassistant.const import EVENT_COMPONENT_LOADED, CONF_ID +from homeassistant.exceptions import HomeAssistantError from homeassistant.setup import ATTR_COMPONENT -from homeassistant.components.http import HomeAssistantView from homeassistant.util.yaml import load_yaml, dump DOMAIN = "config" @@ -80,6 +81,7 @@ def __init__( data_schema, *, post_write_hook=None, + data_validator=None, ): """Initialize a config view.""" self.url = f"/api/config/{component}/{config_type}/{{config_key}}" @@ -88,6 +90,7 @@ def __init__( self.key_schema = key_schema self.data_schema = data_schema self.post_write_hook = post_write_hook + self.data_validator = data_validator def _empty_config(self): """Empty config if file not found.""" @@ -128,14 +131,18 @@ async def post(self, request, config_key): except vol.Invalid as err: return self.json_message(f"Key malformed: {err}", 400) + hass = request.app["hass"] + try: # We just validate, we don't store that data because # we don't want to store the defaults. - self.data_schema(data) - except vol.Invalid as err: + if self.data_validator: + await self.data_validator(hass, data) + else: + self.data_schema(data) + except (vol.Invalid, HomeAssistantError) as err: return self.json_message(f"Message malformed: {err}", 400) - hass = request.app["hass"] path = hass.config.path(self.path) current = await self.read_config(hass) diff --git a/homeassistant/components/config/automation.py b/homeassistant/components/config/automation.py index 17efdba3fb573f..2ed30b5212f93a 100644 --- a/homeassistant/components/config/automation.py +++ b/homeassistant/components/config/automation.py @@ -2,7 +2,11 @@ from collections import OrderedDict import uuid -from homeassistant.components.automation import DOMAIN, PLATFORM_SCHEMA +from homeassistant.components.automation import ( + DOMAIN, + PLATFORM_SCHEMA, + async_validate_config_item, +) from homeassistant.const import CONF_ID, SERVICE_RELOAD import homeassistant.helpers.config_validation as cv @@ -26,6 +30,7 @@ async def hook(hass): cv.string, PLATFORM_SCHEMA, post_write_hook=hook, + data_validator=async_validate_config_item, ) ) return True diff --git a/homeassistant/components/device_automation/__init__.py b/homeassistant/components/device_automation/__init__.py index 9508dd9c849a93..18cc46ae79b3cf 100644 --- a/homeassistant/components/device_automation/__init__.py +++ b/homeassistant/components/device_automation/__init__.py @@ -13,6 +13,8 @@ from homeassistant.helpers.typing import ConfigType from homeassistant.loader import async_get_integration, IntegrationNotFound +from .exceptions import InvalidDeviceAutomationConfig + DOMAIN = "device_automation" _LOGGER = logging.getLogger(__name__) @@ -32,6 +34,79 @@ async def async_setup(hass, config): return True +async def async_get_device_automation_platform(hass, config): + """Load device automation platform for integration. + + Throws InvalidDeviceAutomationConfig if the integration is not found or does not support device automation. + This method is a coroutine. + """ + try: + integration = await async_get_integration(hass, config[CONF_DOMAIN]) + platform = integration.get_platform("device_automation") + except IntegrationNotFound: + raise InvalidDeviceAutomationConfig( + f"Integration '{config[CONF_DOMAIN]}' not found" + ) + except ImportError: + raise InvalidDeviceAutomationConfig( + f"Integration '{config[CONF_DOMAIN]}' does not support device automations" + ) + + return platform + + +async def async_validate_action_config(hass, config): + """Validate config. + + This method is a coroutine. + """ + platform = await async_get_device_automation_platform(hass, config) + if not hasattr(platform, "async_get_actions"): + raise InvalidDeviceAutomationConfig( + f"Integration '{config[CONF_DOMAIN]}' does not support device automation actions" + ) + + return platform.ACTION_SCHEMA(config) + + +async def async_validate_condition_config(hass, config): + """Validate config. + + This method is a coroutine. + """ + platform = await async_get_device_automation_platform(hass, config) + if not hasattr(platform, "async_get_conditions"): + raise InvalidDeviceAutomationConfig( + f"Integration '{config[CONF_DOMAIN]}' does not support device automation conditions" + ) + + return platform.CONDITION_SCHEMA(config) + + +async def async_validate_trigger_config(hass, config): + """Validate config. + + This method is a coroutine. + """ + platform = await async_get_device_automation_platform(hass, config) + if not hasattr(platform, "async_get_triggers"): + raise InvalidDeviceAutomationConfig( + f"Integration '{config[CONF_DOMAIN]}' does not support device automation triggers" + ) + + return platform.TRIGGER_SCHEMA(config) + + +async def async_handle_action(hass, action, variables, context): + """Perform the device automation specified in the action. + + This method is a coroutine. + """ + integration = await async_get_integration(hass, action[CONF_DOMAIN]) + platform = integration.get_platform("device_automation") + await platform.async_call_action_from_config(hass, action, variables, context) + + async def async_device_condition_from_config( hass: HomeAssistant, config: ConfigType, config_validation: bool = True ) -> Callable[..., bool]: @@ -46,6 +121,13 @@ async def async_device_condition_from_config( ) +async def async_trigger(hass, config, action, automation_info): + """Listen for trigger.""" + integration = await async_get_integration(hass, config[CONF_DOMAIN]) + platform = integration.get_platform("device_automation") + return await platform.async_trigger(hass, config, action, automation_info) + + async def _async_get_device_automations_from_domain(hass, domain, fname, device_id): """List device automations.""" integration = None diff --git a/homeassistant/config.py b/homeassistant/config.py index d3bd97dad8f777..4b6f00a0605bde 100644 --- a/homeassistant/config.py +++ b/homeassistant/config.py @@ -416,7 +416,7 @@ def process_ha_config_upgrade(hass: HomeAssistant) -> None: @callback def async_log_exception( - ex: vol.Invalid, domain: str, config: Dict, hass: HomeAssistant + ex: Exception, domain: str, config: Dict, hass: HomeAssistant ) -> None: """Log an error for configuration validation. @@ -428,23 +428,26 @@ def async_log_exception( @callback -def _format_config_error(ex: vol.Invalid, domain: str, config: Dict) -> str: +def _format_config_error(ex: Exception, domain: str, config: Dict) -> str: """Generate log exception for configuration validation. This method must be run in the event loop. """ message = f"Invalid config for [{domain}]: " - if "extra keys not allowed" in ex.error_message: - message += ( - "[{option}] is an invalid option for [{domain}]. " - "Check: {domain}->{path}.".format( - option=ex.path[-1], - domain=domain, - path="->".join(str(m) for m in ex.path), + if isinstance(ex, vol.Invalid): + if "extra keys not allowed" in ex.error_message: + message += ( + "[{option}] is an invalid option for [{domain}]. " + "Check: {domain}->{path}.".format( + option=ex.path[-1], + domain=domain, + path="->".join(str(m) for m in ex.path), + ) ) - ) + else: + message += "{}.".format(humanize_error(config, ex)) else: - message += "{}.".format(humanize_error(config, ex)) + message += str(ex) try: domain_config = config.get(domain, config) @@ -717,6 +720,13 @@ async def async_process_component_config( _LOGGER.error("Unable to import %s: %s", domain, ex) return None + if hasattr(component, "async_validate_config"): + try: + return await component.async_validate_config(hass, config) # type: ignore + except (vol.Invalid, HomeAssistantError) as ex: + async_log_exception(ex, domain, config, hass) + return None + if hasattr(component, "CONFIG_SCHEMA"): try: return component.CONFIG_SCHEMA(config) # type: ignore diff --git a/homeassistant/helpers/condition.py b/homeassistant/helpers/condition.py index 133251e779d1db..7e449b35d5f382 100644 --- a/homeassistant/helpers/condition.py +++ b/homeassistant/helpers/condition.py @@ -13,6 +13,7 @@ from homeassistant.components import zone as zone_cmp from homeassistant.components.device_automation import ( # noqa: F401 pylint: disable=unused-import async_device_condition_from_config as async_device_from_config, + async_validate_condition_config as async_validate_device_condition_config, ) from homeassistant.const import ( ATTR_GPS_ACCURACY, @@ -488,3 +489,25 @@ def if_in_zone(hass: HomeAssistant, variables: TemplateVarsType = None) -> bool: return zone(hass, zone_entity_id, entity_id) return if_in_zone + + +async def async_validate_condition_config( + hass: HomeAssistant, config: ConfigType +) -> ConfigType: + """Validate config. + + This method is a coroutine. + """ + condition = config[CONF_CONDITION] + if condition in ("and", "or"): + conditions = [] + for sub_cond in config["conditions"]: + sub_cond = async_validate_condition_config(hass, sub_cond) + conditions.append(sub_cond) + config["conditions"] = conditions + + if condition == "device": + config = cv.DEVICE_CONDITION_SCHEMA(config) + return await async_validate_device_condition_config(hass, config) + + return config diff --git a/homeassistant/helpers/config_validation.py b/homeassistant/helpers/config_validation.py index 952fa41c42c1ce..8117eb5e7268de 100644 --- a/homeassistant/helpers/config_validation.py +++ b/homeassistant/helpers/config_validation.py @@ -90,7 +90,7 @@ def validate(obj: Dict) -> Dict: for k in obj.keys(): if k in keys: return obj - raise vol.Invalid("must contain one of {}.".format(", ".join(keys))) + raise vol.Invalid("must contain at least one of {}.".format(", ".join(keys))) return validate diff --git a/homeassistant/helpers/script.py b/homeassistant/helpers/script.py index 23728b651098aa..ab361e007325e1 100644 --- a/homeassistant/helpers/script.py +++ b/homeassistant/helpers/script.py @@ -8,13 +8,9 @@ import voluptuous as vol +import homeassistant.components.device_automation as device_automation from homeassistant.core import HomeAssistant, Context, callback, CALLBACK_TYPE -from homeassistant.const import ( - CONF_CONDITION, - CONF_DEVICE_ID, - CONF_DOMAIN, - CONF_TIMEOUT, -) +from homeassistant.const import CONF_CONDITION, CONF_DEVICE_ID, CONF_TIMEOUT from homeassistant import exceptions from homeassistant.helpers import ( service, @@ -27,7 +23,6 @@ async_track_template, ) from homeassistant.helpers.typing import ConfigType -from homeassistant.loader import async_get_integration import homeassistant.util.dt as date_util from homeassistant.util.async_ import run_coroutine_threadsafe, run_callback_threadsafe @@ -86,6 +81,22 @@ def call_from_config( Script(hass, cv.SCRIPT_SCHEMA(config)).run(variables, context) +async def async_validate_action_config( + hass: HomeAssistant, config: ConfigType +) -> ConfigType: + """Validate config. + + This method is a coroutine. + """ + action_type = _determine_action(config) + + if action_type == ACTION_DEVICE_AUTOMATION: + config = await device_automation.async_validate_action_config(hass, config) + # TODO: Special validation also if ACTION_CHECK_CONDITION: CONDITION_SCHEMA is not enough + + return config + + class _StopScript(Exception): """Throw if script needs to stop.""" @@ -335,9 +346,7 @@ async def _async_device_automation(self, action, variables, context): """ self.last_action = action.get(CONF_ALIAS, "device automation") self._log("Executing step %s" % self.last_action) - integration = await async_get_integration(self.hass, action[CONF_DOMAIN]) - platform = integration.get_platform("device_automation") - await platform.async_call_action_from_config( + await device_automation.async_handle_action( self.hass, action, variables, context ) diff --git a/homeassistant/loader.py b/homeassistant/loader.py index 1a9a3d256acb2d..272271eefb31af 100644 --- a/homeassistant/loader.py +++ b/homeassistant/loader.py @@ -307,7 +307,7 @@ class IntegrationNotFound(LoaderError): def __init__(self, domain: str) -> None: """Initialize a component not found error.""" - super().__init__(f"Integration {domain} not found.") + super().__init__(f"Integration '{domain}' not found.") self.domain = domain From 1778c33fc2eb6a472e2fdb5f533cf2dbee1edfeb Mon Sep 17 00:00:00 2001 From: emontnemery Date: Sun, 22 Sep 2019 19:30:35 +0200 Subject: [PATCH 2/5] Fix lint, typing, tests, review comments --- .../components/automation/__init__.py | 10 ++------ homeassistant/components/automation/device.py | 5 +--- .../components/device_automation/__init__.py | 25 ++++++------------- homeassistant/helpers/condition.py | 5 +--- homeassistant/helpers/script.py | 6 +---- tests/helpers/test_check_config.py | 4 +-- tests/scripts/test_check_config.py | 4 +-- 7 files changed, 16 insertions(+), 43 deletions(-) diff --git a/homeassistant/components/automation/__init__.py b/homeassistant/components/automation/__init__.py index 891601e082e826..c06853c53d7eec 100644 --- a/homeassistant/components/automation/__init__.py +++ b/homeassistant/components/automation/__init__.py @@ -379,10 +379,7 @@ def device_state_attributes(self): async def async_validate_config_item(hass, config): - """Validate config item. - - This method is a coroutine. - """ + """Validate config item.""" p_validated = PLATFORM_SCHEMA(config) triggers = [] @@ -414,10 +411,7 @@ async def async_validate_config_item(hass, config): async def async_validate_config(hass, config): - """Validate config. - - This method is a coroutine. - """ + """Validate config.""" automations = [] for _, p_config in config_per_platform(config, DOMAIN): try: diff --git a/homeassistant/components/automation/device.py b/homeassistant/components/automation/device.py index e25b2a677f90c6..800c27ebbca6a1 100644 --- a/homeassistant/components/automation/device.py +++ b/homeassistant/components/automation/device.py @@ -14,10 +14,7 @@ async def async_validate_trigger_config(hass, config): - """Validate config. - - This method is a coroutine. - """ + """Validate config.""" return await device_automation.async_validate_trigger_config(hass, config) diff --git a/homeassistant/components/device_automation/__init__.py b/homeassistant/components/device_automation/__init__.py index 18cc46ae79b3cf..3a14f259a177d9 100644 --- a/homeassistant/components/device_automation/__init__.py +++ b/homeassistant/components/device_automation/__init__.py @@ -38,7 +38,6 @@ async def async_get_device_automation_platform(hass, config): """Load device automation platform for integration. Throws InvalidDeviceAutomationConfig if the integration is not found or does not support device automation. - This method is a coroutine. """ try: integration = await async_get_integration(hass, config[CONF_DOMAIN]) @@ -56,10 +55,7 @@ async def async_get_device_automation_platform(hass, config): async def async_validate_action_config(hass, config): - """Validate config. - - This method is a coroutine. - """ + """Validate config.""" platform = await async_get_device_automation_platform(hass, config) if not hasattr(platform, "async_get_actions"): raise InvalidDeviceAutomationConfig( @@ -69,11 +65,10 @@ async def async_validate_action_config(hass, config): return platform.ACTION_SCHEMA(config) -async def async_validate_condition_config(hass, config): - """Validate config. - - This method is a coroutine. - """ +async def async_validate_condition_config( + hass: HomeAssistant, config: ConfigType +) -> ConfigType: + """Validate config.""" platform = await async_get_device_automation_platform(hass, config) if not hasattr(platform, "async_get_conditions"): raise InvalidDeviceAutomationConfig( @@ -84,10 +79,7 @@ async def async_validate_condition_config(hass, config): async def async_validate_trigger_config(hass, config): - """Validate config. - - This method is a coroutine. - """ + """Validate config.""" platform = await async_get_device_automation_platform(hass, config) if not hasattr(platform, "async_get_triggers"): raise InvalidDeviceAutomationConfig( @@ -98,10 +90,7 @@ async def async_validate_trigger_config(hass, config): async def async_handle_action(hass, action, variables, context): - """Perform the device automation specified in the action. - - This method is a coroutine. - """ + """Perform the device automation specified in the action.""" integration = await async_get_integration(hass, action[CONF_DOMAIN]) platform = integration.get_platform("device_automation") await platform.async_call_action_from_config(hass, action, variables, context) diff --git a/homeassistant/helpers/condition.py b/homeassistant/helpers/condition.py index 7e449b35d5f382..9a80a8e41baa86 100644 --- a/homeassistant/helpers/condition.py +++ b/homeassistant/helpers/condition.py @@ -494,10 +494,7 @@ def if_in_zone(hass: HomeAssistant, variables: TemplateVarsType = None) -> bool: async def async_validate_condition_config( hass: HomeAssistant, config: ConfigType ) -> ConfigType: - """Validate config. - - This method is a coroutine. - """ + """Validate config.""" condition = config[CONF_CONDITION] if condition in ("and", "or"): conditions = [] diff --git a/homeassistant/helpers/script.py b/homeassistant/helpers/script.py index ab361e007325e1..3edb789ed3c1a5 100644 --- a/homeassistant/helpers/script.py +++ b/homeassistant/helpers/script.py @@ -84,15 +84,11 @@ def call_from_config( async def async_validate_action_config( hass: HomeAssistant, config: ConfigType ) -> ConfigType: - """Validate config. - - This method is a coroutine. - """ + """Validate config.""" action_type = _determine_action(config) if action_type == ACTION_DEVICE_AUTOMATION: config = await device_automation.async_validate_action_config(hass, config) - # TODO: Special validation also if ACTION_CHECK_CONDITION: CONDITION_SCHEMA is not enough return config diff --git a/tests/helpers/test_check_config.py b/tests/helpers/test_check_config.py index 9e5ea15293a484..5228f0d4882228 100644 --- a/tests/helpers/test_check_config.py +++ b/tests/helpers/test_check_config.py @@ -75,7 +75,7 @@ async def test_component_platform_not_found(hass, loop): assert res.keys() == {"homeassistant"} assert res.errors[0] == CheckConfigError( - "Component error: beer - Integration beer not found.", None, None + "Component error: beer - Integration 'beer' not found.", None, None ) # Only 1 error expected @@ -95,7 +95,7 @@ async def test_component_platform_not_found_2(hass, loop): assert res["light"] == [] assert res.errors[0] == CheckConfigError( - "Platform error light.beer - Integration beer not found.", None, None + "Platform error light.beer - Integration 'beer' not found.", None, None ) # Only 1 error expected diff --git a/tests/scripts/test_check_config.py b/tests/scripts/test_check_config.py index bd4f37bd1357c3..18143c088be58a 100644 --- a/tests/scripts/test_check_config.py +++ b/tests/scripts/test_check_config.py @@ -63,7 +63,7 @@ def test_component_platform_not_found(isfile_patch, loop): assert res["components"].keys() == {"homeassistant"} assert res["except"] == { check_config.ERROR_STR: [ - "Component error: beer - Integration beer not found." + "Component error: beer - Integration 'beer' not found." ] } assert res["secret_cache"] == {} @@ -77,7 +77,7 @@ def test_component_platform_not_found(isfile_patch, loop): assert res["components"]["light"] == [] assert res["except"] == { check_config.ERROR_STR: [ - "Platform error light.beer - Integration beer not found." + "Platform error light.beer - Integration 'beer' not found." ] } assert res["secret_cache"] == {} From 7498c655383c07b310d1911482c97463d856ed5c Mon Sep 17 00:00:00 2001 From: emontnemery Date: Sun, 22 Sep 2019 22:13:51 +0200 Subject: [PATCH 3/5] Add tests --- homeassistant/helpers/condition.py | 2 +- .../components/device_automation/test_init.py | 191 ++++++++++++++++++ 2 files changed, 192 insertions(+), 1 deletion(-) diff --git a/homeassistant/helpers/condition.py b/homeassistant/helpers/condition.py index 9a80a8e41baa86..6afdbcb4e4ab59 100644 --- a/homeassistant/helpers/condition.py +++ b/homeassistant/helpers/condition.py @@ -499,7 +499,7 @@ async def async_validate_condition_config( if condition in ("and", "or"): conditions = [] for sub_cond in config["conditions"]: - sub_cond = async_validate_condition_config(hass, sub_cond) + sub_cond = await async_validate_condition_config(hass, sub_cond) conditions.append(sub_cond) config["conditions"] = conditions diff --git a/tests/components/device_automation/test_init.py b/tests/components/device_automation/test_init.py index b05c04a16f1dc5..c1ed232666d421 100644 --- a/tests/components/device_automation/test_init.py +++ b/tests/components/device_automation/test_init.py @@ -2,6 +2,7 @@ import pytest from homeassistant.setup import async_setup_component +import homeassistant.components.automation as automation from homeassistant.components.websocket_api.const import TYPE_RESULT from homeassistant.helpers import device_registry @@ -161,3 +162,193 @@ async def test_websocket_get_triggers(hass, hass_ws_client, device_reg, entity_r assert msg["success"] triggers = msg["result"] assert _same_lists(triggers, expected_triggers) + + +async def test_automation_with_non_existing_integration(hass, caplog): + """Test automation with an error in script.""" + assert await async_setup_component( + hass, + automation.DOMAIN, + { + automation.DOMAIN: { + "alias": "hello", + "trigger": {"platform": "device", "domain": "beer"}, + "action": {"service": "test.automation", "entity_id": "hello.world"}, + } + }, + ) + + hass.bus.async_fire("test_event") + await hass.async_block_till_done() + assert "Integration 'beer' not found" in caplog.text + + +async def test_automation_with_integration_without_device_automation(hass, caplog): + """Test automation with an error in script.""" + assert await async_setup_component( + hass, + automation.DOMAIN, + { + automation.DOMAIN: { + "alias": "hello", + "trigger": {"platform": "device", "domain": "mqtt"}, + "action": {"service": "test.automation", "entity_id": "hello.world"}, + } + }, + ) + + hass.bus.async_fire("test_event") + await hass.async_block_till_done() + assert "Integration 'mqtt' does not support device automations" in caplog.text + + +async def test_automation_with_integration_without_device_automation_action( + hass, caplog +): + """Test automation with an error in script.""" + assert await async_setup_component( + hass, + automation.DOMAIN, + { + automation.DOMAIN: { + "alias": "hello", + "trigger": {"platform": "event", "event_type": "test_event1"}, + "action": {"device_id": "", "domain": "test"}, + } + }, + ) + + hass.bus.async_fire("test_event") + await hass.async_block_till_done() + assert ( + "Integration 'test' does not support device automation actions" in caplog.text + ) + + +async def test_automation_with_integration_without_device_automation_condition( + hass, caplog +): + """Test automation with an error in script.""" + assert await async_setup_component( + hass, + automation.DOMAIN, + { + automation.DOMAIN: { + "alias": "hello", + "trigger": {"platform": "event", "event_type": "test_event1"}, + "condition": {"condition": "device", "domain": "test"}, + "action": {"service": "test.automation", "entity_id": "hello.world"}, + } + }, + ) + + hass.bus.async_fire("test_event") + await hass.async_block_till_done() + assert ( + "Integration 'test' does not support device automation conditions" + in caplog.text + ) + + +async def test_automation_with_integration_without_device_automation_trigger( + hass, caplog +): + """Test automation with an error in script.""" + assert await async_setup_component( + hass, + automation.DOMAIN, + { + automation.DOMAIN: { + "alias": "hello", + "trigger": {"platform": "device", "domain": "test"}, + "action": {"service": "test.automation", "entity_id": "hello.world"}, + } + }, + ) + + hass.bus.async_fire("test_event") + await hass.async_block_till_done() + assert ( + "Integration 'test' does not support device automation triggers" in caplog.text + ) + + +async def test_automation_with_bad_action(hass, caplog): + """Test automation with an error in script.""" + assert await async_setup_component( + hass, + automation.DOMAIN, + { + automation.DOMAIN: { + "alias": "hello", + "trigger": {"platform": "event", "event_type": "test_event1"}, + "action": {"device_id": "", "domain": "light"}, + } + }, + ) + + hass.bus.async_fire("test_event") + await hass.async_block_till_done() + assert "required key not provided" in caplog.text + + +async def test_automation_with_bad_condition(hass, caplog): + """Test automation with an error in script.""" + assert await async_setup_component( + hass, + automation.DOMAIN, + { + automation.DOMAIN: { + "alias": "hello", + "trigger": {"platform": "event", "event_type": "test_event1"}, + "condition": {"condition": "device", "domain": "light"}, + "action": {"service": "test.automation", "entity_id": "hello.world"}, + } + }, + ) + + hass.bus.async_fire("test_event") + await hass.async_block_till_done() + assert "required key not provided" in caplog.text + + +async def test_automation_with_bad_sub_condition(hass, caplog): + """Test automation with an error in script.""" + assert await async_setup_component( + hass, + automation.DOMAIN, + { + automation.DOMAIN: { + "alias": "hello", + "trigger": {"platform": "event", "event_type": "test_event1"}, + "condition": { + "condition": "and", + "conditions": [{"condition": "device", "domain": "light"}], + }, + "action": {"service": "test.automation", "entity_id": "hello.world"}, + } + }, + ) + + hass.bus.async_fire("test_event") + await hass.async_block_till_done() + assert "required key not provided" in caplog.text + + +async def test_automation_with_bad_trigger(hass, caplog): + """Test automation with an error in script.""" + assert await async_setup_component( + hass, + automation.DOMAIN, + { + automation.DOMAIN: { + "alias": "hello", + "trigger": {"platform": "device", "domain": "light"}, + "action": {"service": "test.automation", "entity_id": "hello.world"}, + } + }, + ) + + hass.bus.async_fire("test_event") + await hass.async_block_till_done() + assert "required key not provided" in caplog.text From 4ecc6d7be92a966aaffcd8a405cf07655e7a495f Mon Sep 17 00:00:00 2001 From: emontnemery Date: Mon, 23 Sep 2019 07:16:39 +0200 Subject: [PATCH 4/5] Add missing file --- tests/testing_config/custom_components/test/device_automation.py | 1 + 1 file changed, 1 insertion(+) create mode 100644 tests/testing_config/custom_components/test/device_automation.py diff --git a/tests/testing_config/custom_components/test/device_automation.py b/tests/testing_config/custom_components/test/device_automation.py new file mode 100644 index 00000000000000..f5ff261efed7dc --- /dev/null +++ b/tests/testing_config/custom_components/test/device_automation.py @@ -0,0 +1 @@ +"""Provide a non functioning mock platform for device_automation test.""" From bb2f2874aeeb8f419d3050fca9b63cc70bc446dd Mon Sep 17 00:00:00 2001 From: emontnemery Date: Mon, 23 Sep 2019 16:40:58 +0200 Subject: [PATCH 5/5] Improve tests, handle device condition in action --- .../components/automation/__init__.py | 18 +- homeassistant/helpers/script.py | 2 + .../components/device_automation/test_init.py | 178 +++++++++++++++--- 3 files changed, 161 insertions(+), 37 deletions(-) diff --git a/homeassistant/components/automation/__init__.py b/homeassistant/components/automation/__init__.py index c06853c53d7eec..0b4c22a72772c0 100644 --- a/homeassistant/components/automation/__init__.py +++ b/homeassistant/components/automation/__init__.py @@ -380,10 +380,10 @@ def device_state_attributes(self): async def async_validate_config_item(hass, config): """Validate config item.""" - p_validated = PLATFORM_SCHEMA(config) + config = PLATFORM_SCHEMA(config) triggers = [] - for trigger in p_validated[CONF_TRIGGER]: + for trigger in config[CONF_TRIGGER]: trigger_platform = importlib.import_module( ".{}".format(trigger[CONF_PLATFORM]), __name__ ) @@ -392,22 +392,22 @@ async def async_validate_config_item(hass, config): hass, trigger ) triggers.append(trigger) - p_validated[CONF_TRIGGER] = triggers + config[CONF_TRIGGER] = triggers - if CONF_CONDITION in p_validated: + if CONF_CONDITION in config: conditions = [] - for cond in p_validated[CONF_CONDITION]: + for cond in config[CONF_CONDITION]: cond = await condition.async_validate_condition_config(hass, cond) conditions.append(cond) - p_validated[CONF_CONDITION] = conditions + config[CONF_CONDITION] = conditions actions = [] - for action in p_validated[CONF_ACTION]: + for action in config[CONF_ACTION]: action = await script.async_validate_action_config(hass, action) actions.append(action) - p_validated[CONF_ACTION] = actions + config[CONF_ACTION] = actions - return p_validated + return config async def async_validate_config(hass, config): diff --git a/homeassistant/helpers/script.py b/homeassistant/helpers/script.py index 3edb789ed3c1a5..54a9843ec7cc36 100644 --- a/homeassistant/helpers/script.py +++ b/homeassistant/helpers/script.py @@ -89,6 +89,8 @@ async def async_validate_action_config( if action_type == ACTION_DEVICE_AUTOMATION: config = await device_automation.async_validate_action_config(hass, config) + if action_type == ACTION_CHECK_CONDITION and config[CONF_CONDITION] == "device": + config = await device_automation.async_validate_condition_config(hass, config) return config diff --git a/tests/components/device_automation/test_init.py b/tests/components/device_automation/test_init.py index c1ed232666d421..1bf6609ba246f2 100644 --- a/tests/components/device_automation/test_init.py +++ b/tests/components/device_automation/test_init.py @@ -4,10 +4,16 @@ from homeassistant.setup import async_setup_component import homeassistant.components.automation as automation from homeassistant.components.websocket_api.const import TYPE_RESULT +from homeassistant.const import STATE_ON, STATE_OFF, CONF_PLATFORM from homeassistant.helpers import device_registry -from tests.common import MockConfigEntry, mock_device_registry, mock_registry +from tests.common import ( + MockConfigEntry, + async_mock_service, + mock_device_registry, + mock_registry, +) @pytest.fixture @@ -165,7 +171,7 @@ async def test_websocket_get_triggers(hass, hass_ws_client, device_reg, entity_r async def test_automation_with_non_existing_integration(hass, caplog): - """Test automation with an error in script.""" + """Test device automation with non existing integration.""" assert await async_setup_component( hass, automation.DOMAIN, @@ -178,13 +184,11 @@ async def test_automation_with_non_existing_integration(hass, caplog): }, ) - hass.bus.async_fire("test_event") - await hass.async_block_till_done() assert "Integration 'beer' not found" in caplog.text async def test_automation_with_integration_without_device_automation(hass, caplog): - """Test automation with an error in script.""" + """Test automation with integration wihtout device automation.""" assert await async_setup_component( hass, automation.DOMAIN, @@ -197,15 +201,13 @@ async def test_automation_with_integration_without_device_automation(hass, caplo }, ) - hass.bus.async_fire("test_event") - await hass.async_block_till_done() assert "Integration 'mqtt' does not support device automations" in caplog.text async def test_automation_with_integration_without_device_automation_action( hass, caplog ): - """Test automation with an error in script.""" + """Test automation with integration without device action support.""" assert await async_setup_component( hass, automation.DOMAIN, @@ -218,8 +220,6 @@ async def test_automation_with_integration_without_device_automation_action( }, ) - hass.bus.async_fire("test_event") - await hass.async_block_till_done() assert ( "Integration 'test' does not support device automation actions" in caplog.text ) @@ -228,7 +228,7 @@ async def test_automation_with_integration_without_device_automation_action( async def test_automation_with_integration_without_device_automation_condition( hass, caplog ): - """Test automation with an error in script.""" + """Test automation with integration without device condition support.""" assert await async_setup_component( hass, automation.DOMAIN, @@ -242,8 +242,6 @@ async def test_automation_with_integration_without_device_automation_condition( }, ) - hass.bus.async_fire("test_event") - await hass.async_block_till_done() assert ( "Integration 'test' does not support device automation conditions" in caplog.text @@ -253,7 +251,7 @@ async def test_automation_with_integration_without_device_automation_condition( async def test_automation_with_integration_without_device_automation_trigger( hass, caplog ): - """Test automation with an error in script.""" + """Test automation with integration without device trigger support.""" assert await async_setup_component( hass, automation.DOMAIN, @@ -266,15 +264,13 @@ async def test_automation_with_integration_without_device_automation_trigger( }, ) - hass.bus.async_fire("test_event") - await hass.async_block_till_done() assert ( "Integration 'test' does not support device automation triggers" in caplog.text ) async def test_automation_with_bad_action(hass, caplog): - """Test automation with an error in script.""" + """Test automation with bad device action.""" assert await async_setup_component( hass, automation.DOMAIN, @@ -287,13 +283,28 @@ async def test_automation_with_bad_action(hass, caplog): }, ) - hass.bus.async_fire("test_event") - await hass.async_block_till_done() + assert "required key not provided" in caplog.text + + +async def test_automation_with_bad_condition_action(hass, caplog): + """Test automation with bad device action.""" + assert await async_setup_component( + hass, + automation.DOMAIN, + { + automation.DOMAIN: { + "alias": "hello", + "trigger": {"platform": "event", "event_type": "test_event1"}, + "action": {"condition": "device", "device_id": "", "domain": "light"}, + } + }, + ) + assert "required key not provided" in caplog.text async def test_automation_with_bad_condition(hass, caplog): - """Test automation with an error in script.""" + """Test automation with bad device condition.""" assert await async_setup_component( hass, automation.DOMAIN, @@ -307,13 +318,128 @@ async def test_automation_with_bad_condition(hass, caplog): }, ) - hass.bus.async_fire("test_event") - await hass.async_block_till_done() assert "required key not provided" in caplog.text +@pytest.fixture +def calls(hass): + """Track calls to a mock serivce.""" + return async_mock_service(hass, "test", "automation") + + +async def test_automation_with_sub_condition(hass, calls): + """Test automation with device condition under and/or conditions.""" + DOMAIN = "light" + platform = getattr(hass.components, f"test.{DOMAIN}") + + platform.init() + assert await async_setup_component(hass, DOMAIN, {DOMAIN: {CONF_PLATFORM: "test"}}) + + ent1, ent2, ent3 = platform.ENTITIES + + assert await async_setup_component( + hass, + automation.DOMAIN, + { + automation.DOMAIN: [ + { + "trigger": {"platform": "event", "event_type": "test_event1"}, + "condition": [ + { + "condition": "and", + "conditions": [ + { + "condition": "device", + "domain": DOMAIN, + "device_id": "", + "entity_id": ent1.entity_id, + "type": "is_on", + }, + { + "condition": "device", + "domain": DOMAIN, + "device_id": "", + "entity_id": ent2.entity_id, + "type": "is_on", + }, + ], + } + ], + "action": { + "service": "test.automation", + "data_template": { + "some": "and {{ trigger.%s }}" + % "}} - {{ trigger.".join(("platform", "event.event_type")) + }, + }, + }, + { + "trigger": {"platform": "event", "event_type": "test_event1"}, + "condition": [ + { + "condition": "or", + "conditions": [ + { + "condition": "device", + "domain": DOMAIN, + "device_id": "", + "entity_id": ent1.entity_id, + "type": "is_on", + }, + { + "condition": "device", + "domain": DOMAIN, + "device_id": "", + "entity_id": ent2.entity_id, + "type": "is_on", + }, + ], + } + ], + "action": { + "service": "test.automation", + "data_template": { + "some": "or {{ trigger.%s }}" + % "}} - {{ trigger.".join(("platform", "event.event_type")) + }, + }, + }, + ] + }, + ) + await hass.async_block_till_done() + assert hass.states.get(ent1.entity_id).state == STATE_ON + assert hass.states.get(ent2.entity_id).state == STATE_OFF + assert len(calls) == 0 + + hass.bus.async_fire("test_event1") + await hass.async_block_till_done() + assert len(calls) == 1 + assert calls[0].data["some"] == "or event - test_event1" + + hass.states.async_set(ent1.entity_id, STATE_OFF) + hass.bus.async_fire("test_event1") + await hass.async_block_till_done() + assert len(calls) == 1 + + hass.states.async_set(ent2.entity_id, STATE_ON) + hass.bus.async_fire("test_event1") + await hass.async_block_till_done() + assert len(calls) == 2 + assert calls[1].data["some"] == "or event - test_event1" + + hass.states.async_set(ent1.entity_id, STATE_ON) + hass.bus.async_fire("test_event1") + await hass.async_block_till_done() + assert len(calls) == 4 + assert _same_lists( + [calls[2].data["some"], calls[3].data["some"]], + ["or event - test_event1", "and event - test_event1"], + ) + + async def test_automation_with_bad_sub_condition(hass, caplog): - """Test automation with an error in script.""" + """Test automation with bad device condition under and/or conditions.""" assert await async_setup_component( hass, automation.DOMAIN, @@ -330,13 +456,11 @@ async def test_automation_with_bad_sub_condition(hass, caplog): }, ) - hass.bus.async_fire("test_event") - await hass.async_block_till_done() assert "required key not provided" in caplog.text async def test_automation_with_bad_trigger(hass, caplog): - """Test automation with an error in script.""" + """Test automation with bad device trigger.""" assert await async_setup_component( hass, automation.DOMAIN, @@ -349,6 +473,4 @@ async def test_automation_with_bad_trigger(hass, caplog): }, ) - hass.bus.async_fire("test_event") - await hass.async_block_till_done() assert "required key not provided" in caplog.text