Improve device automation validation (#86143)

homeassistant/components/device_automation/action.py

@@ -1,16 +1,17 @@
 """Device action validator."""
 from __future__ import annotations
 
-from typing import Any, Protocol, cast
+from typing import Any, Protocol
 
 import voluptuous as vol
 
 from homeassistant.const import CONF_DOMAIN
 from homeassistant.core import Context, HomeAssistant
+from homeassistant.helpers import config_validation as cv
 from homeassistant.helpers.typing import ConfigType
 
 from . import DeviceAutomationType, async_get_device_automation_platform
-from .exceptions import InvalidDeviceAutomationConfig
+from .helpers import async_validate_device_automation_config
 
 
 class DeviceAutomationActionProtocol(Protocol):
@@ -50,15 +51,9 @@ async def async_validate_action_config(
     hass: HomeAssistant, config: ConfigType
 ) -> ConfigType:
     """Validate config."""
-    try:
+    return await async_validate_device_automation_config(
-        platform = await async_get_device_automation_platform(
+        hass, config, cv.DEVICE_ACTION_SCHEMA, DeviceAutomationType.ACTION
-            hass, config[CONF_DOMAIN], DeviceAutomationType.ACTION
+    )
-        )
-        if hasattr(platform, "async_validate_action_config"):
-            return await platform.async_validate_action_config(hass, config)
-        return cast(ConfigType, platform.ACTION_SCHEMA(config))
-    except InvalidDeviceAutomationConfig as err:
-        raise vol.Invalid(str(err) or "Invalid action configuration") from err
 
 
 async def async_call_action_from_config(

homeassistant/components/device_automation/condition.py

@@ -1,7 +1,7 @@
 """Validate device conditions."""
 from __future__ import annotations
 
-from typing import TYPE_CHECKING, Any, Protocol, cast
+from typing import TYPE_CHECKING, Any, Protocol
 
 import voluptuous as vol
 
@@ -11,7 +11,7 @@ from homeassistant.helpers import config_validation as cv
 from homeassistant.helpers.typing import ConfigType
 
 from . import DeviceAutomationType, async_get_device_automation_platform
-from .exceptions import InvalidDeviceAutomationConfig
+from .helpers import async_validate_device_automation_config
 
 if TYPE_CHECKING:
     from homeassistant.helpers import condition
@@ -50,16 +50,9 @@ async def async_validate_condition_config(
     hass: HomeAssistant, config: ConfigType
 ) -> ConfigType:
     """Validate device condition config."""
-    try:
+    return await async_validate_device_automation_config(
-        config = cv.DEVICE_CONDITION_SCHEMA(config)
+        hass, config, cv.DEVICE_CONDITION_SCHEMA, DeviceAutomationType.CONDITION
-        platform = await async_get_device_automation_platform(
+    )
-            hass, config[CONF_DOMAIN], DeviceAutomationType.CONDITION
-        )
-        if hasattr(platform, "async_validate_condition_config"):
-            return await platform.async_validate_condition_config(hass, config)
-        return cast(ConfigType, platform.CONDITION_SCHEMA(config))
-    except InvalidDeviceAutomationConfig as err:
-        raise vol.Invalid(str(err) or "Invalid condition configuration") from err
 
 
 async def async_condition_from_config(

homeassistant/components/device_automation/helpers.py

@@ -0,0 +1,80 @@
+"""Helpers for device oriented automations."""
+from __future__ import annotations
+
+from typing import cast
+
+import voluptuous as vol
+
+from homeassistant.const import CONF_DEVICE_ID, CONF_DOMAIN
+from homeassistant.core import HomeAssistant
+from homeassistant.helpers import device_registry as dr
+from homeassistant.helpers.typing import ConfigType
+
+from . import DeviceAutomationType, async_get_device_automation_platform
+from .exceptions import InvalidDeviceAutomationConfig
+
+DYNAMIC_VALIDATOR = {
+    DeviceAutomationType.ACTION: "async_validate_action_config",
+    DeviceAutomationType.CONDITION: "async_validate_condition_config",
+    DeviceAutomationType.TRIGGER: "async_validate_trigger_config",
+}
+
+STATIC_VALIDATOR = {
+    DeviceAutomationType.ACTION: "ACTION_SCHEMA",
+    DeviceAutomationType.CONDITION: "CONDITION_SCHEMA",
+    DeviceAutomationType.TRIGGER: "TRIGGER_SCHEMA",
+}
+
+
+async def async_validate_device_automation_config(
+    hass: HomeAssistant,
+    config: ConfigType,
+    automation_schema: vol.Schema,
+    automation_type: DeviceAutomationType,
+) -> ConfigType:
+    """Validate config."""
+    validated_config: ConfigType = automation_schema(config)
+    platform = await async_get_device_automation_platform(
+        hass, validated_config[CONF_DOMAIN], automation_type
+    )
+    if not hasattr(platform, DYNAMIC_VALIDATOR[automation_type]):
+        # Pass the unvalidated config to avoid mutating the raw config twice
+        return cast(
+            ConfigType, getattr(platform, STATIC_VALIDATOR[automation_type])(config)
+        )
+
+    # Only call the dynamic validator if the referenced device exists and the relevant
+    # config entry is loaded
+    registry = dr.async_get(hass)
+    if not (device := registry.async_get(validated_config[CONF_DEVICE_ID])):
+        # The device referenced by the device trigger does not exist
+        raise InvalidDeviceAutomationConfig(
+            f"Unknown device '{validated_config[CONF_DEVICE_ID]}'"
+        )
+
+    device_config_entry = None
+    for entry_id in device.config_entries:
+        if (
+            not (entry := hass.config_entries.async_get_entry(entry_id))
+            or entry.domain != validated_config[CONF_DOMAIN]
+        ):
+            continue
+        device_config_entry = entry
+        break
+
+    if not device_config_entry:
+        # The config entry referenced by the device trigger does not exist
+        raise InvalidDeviceAutomationConfig(
+            f"Device '{validated_config[CONF_DEVICE_ID]}' has no config entry from "
+            f"domain '{validated_config[CONF_DOMAIN]}'"
+        )
+
+    if not await hass.config_entries.async_wait_component(device_config_entry):
+        # The component could not be loaded, skip the dynamic validation
+        return validated_config
+
+    # Pass the unvalidated config to avoid mutating the raw config twice
+    return cast(
+        ConfigType,
+        await getattr(platform, DYNAMIC_VALIDATOR[automation_type])(hass, config),
+    )

homeassistant/components/device_automation/trigger.py

@@ -1,13 +1,12 @@
 """Offer device oriented automation."""
 from __future__ import annotations
 
-from typing import Any, Protocol, cast
+from typing import Any, Protocol
 
 import voluptuous as vol
 
-from homeassistant.const import CONF_DEVICE_ID, CONF_DOMAIN
+from homeassistant.const import CONF_DOMAIN
 from homeassistant.core import CALLBACK_TYPE, HomeAssistant
-from homeassistant.helpers import device_registry as dr
 from homeassistant.helpers.trigger import TriggerActionType, TriggerInfo
 from homeassistant.helpers.typing import ConfigType
 
@@ -16,7 +15,7 @@ from . import (
     DeviceAutomationType,
     async_get_device_automation_platform,
 )
-from .exceptions import InvalidDeviceAutomationConfig
+from .helpers import async_validate_device_automation_config
 
 TRIGGER_SCHEMA = DEVICE_TRIGGER_BASE_SCHEMA.extend({}, extra=vol.ALLOW_EXTRA)
 
@@ -58,36 +57,9 @@ async def async_validate_trigger_config(
     hass: HomeAssistant, config: ConfigType
 ) -> ConfigType:
     """Validate config."""
-    try:
+    return await async_validate_device_automation_config(
-        platform = await async_get_device_automation_platform(
+        hass, config, TRIGGER_SCHEMA, DeviceAutomationType.TRIGGER
-            hass, config[CONF_DOMAIN], DeviceAutomationType.TRIGGER
+    )
-        )
-        if not hasattr(platform, "async_validate_trigger_config"):
-            return cast(ConfigType, platform.TRIGGER_SCHEMA(config))
-
-        # Only call the dynamic validator if the relevant config entry is loaded
-        registry = dr.async_get(hass)
-        if not (device := registry.async_get(config[CONF_DEVICE_ID])):
-            return config
-
-        device_config_entry = None
-        for entry_id in device.config_entries:
-            if not (entry := hass.config_entries.async_get_entry(entry_id)):
-                continue
-            if entry.domain != config[CONF_DOMAIN]:
-                continue
-            device_config_entry = entry
-            break
-
-        if not device_config_entry:
-            return config
-
-        if not await hass.config_entries.async_wait_component(device_config_entry):
-            return config
-
-        return await platform.async_validate_trigger_config(hass, config)
-    except InvalidDeviceAutomationConfig as err:
-        raise vol.Invalid(str(err) or "Invalid trigger configuration") from err
 
 
 async def async_attach_trigger(

homeassistant/components/zha/device_trigger.py

@@ -41,7 +41,7 @@ async def async_validate_trigger_config(
         zha_device.device_automation_triggers is None
         or trigger not in zha_device.device_automation_triggers
     ):
-        raise InvalidDeviceAutomationConfig
+        raise InvalidDeviceAutomationConfig(f"device does not have trigger {trigger}")
 
     return config
 

tests/components/device_automation/test_init.py

@@ -867,7 +867,7 @@ async def test_automation_with_device_action(hass, caplog, fake_integration):
 
 
 async def test_automation_with_dynamically_validated_action(
-    hass, caplog, fake_integration
+    hass, caplog, device_reg, fake_integration
 ):
     """Test device automation with an action which is dynamically validated."""
 
@@ -875,6 +875,14 @@ async def test_automation_with_dynamically_validated_action(
     module = module_cache["fake_integration.device_action"]
     module.async_validate_action_config = AsyncMock()
 
+    config_entry = MockConfigEntry(domain="fake_integration", data={})
+    config_entry.state = config_entries.ConfigEntryState.LOADED
+    config_entry.add_to_hass(hass)
+    device_entry = device_reg.async_get_or_create(
+        config_entry_id=config_entry.entry_id,
+        connections={(device_registry.CONNECTION_NETWORK_MAC, "12:34:56:AB:CD:EF")},
+    )
+
     assert await async_setup_component(
         hass,
         automation.DOMAIN,
@@ -882,7 +890,7 @@ async def test_automation_with_dynamically_validated_action(
             automation.DOMAIN: {
                 "alias": "hello",
                 "trigger": {"platform": "event", "event_type": "test_event1"},
-                "action": {"device_id": "", "domain": "fake_integration"},
+                "action": {"device_id": device_entry.id, "domain": "fake_integration"},
             }
         },
     )
@@ -940,7 +948,7 @@ async def test_automation_with_device_condition(hass, caplog, fake_integration):
 
 
 async def test_automation_with_dynamically_validated_condition(
-    hass, caplog, fake_integration
+    hass, caplog, device_reg, fake_integration
 ):
     """Test device automation with a condition which is dynamically validated."""
 
@@ -948,6 +956,14 @@ async def test_automation_with_dynamically_validated_condition(
     module = module_cache["fake_integration.device_condition"]
     module.async_validate_condition_config = AsyncMock()
 
+    config_entry = MockConfigEntry(domain="fake_integration", data={})
+    config_entry.state = config_entries.ConfigEntryState.LOADED
+    config_entry.add_to_hass(hass)
+    device_entry = device_reg.async_get_or_create(
+        config_entry_id=config_entry.entry_id,
+        connections={(device_registry.CONNECTION_NETWORK_MAC, "12:34:56:AB:CD:EF")},
+    )
+
     assert await async_setup_component(
         hass,
         automation.DOMAIN,
@@ -957,7 +973,7 @@ async def test_automation_with_dynamically_validated_condition(
                 "trigger": {"platform": "event", "event_type": "test_event1"},
                 "condition": {
                     "condition": "device",
-                    "device_id": "none",
+                    "device_id": device_entry.id,
                     "domain": "fake_integration",
                 },
                 "action": {"service": "test.automation", "entity_id": "hello.world"},
@@ -1121,6 +1137,24 @@ async def test_automation_with_bad_condition_action(hass, caplog):
     assert "required key not provided" in caplog.text
 
 
+async def test_automation_with_bad_condition_missing_domain(hass, caplog):
+    """Test automation with bad device condition."""
+    assert await async_setup_component(
+        hass,
+        automation.DOMAIN,
+        {
+            automation.DOMAIN: {
+                "alias": "hello",
+                "trigger": {"platform": "event", "event_type": "test_event1"},
+                "condition": {"condition": "device", "device_id": "hello.device"},
+                "action": {"service": "test.automation", "entity_id": "hello.world"},
+            }
+        },
+    )
+
+    assert "required key not provided @ data['condition'][0]['domain']" in caplog.text
+
+
 async def test_automation_with_bad_condition(hass, caplog):
     """Test automation with bad device condition."""
     assert await async_setup_component(
@@ -1305,3 +1339,105 @@ async def test_websocket_device_not_found(hass, hass_ws_client):
     assert msg["id"] == 1
     assert not msg["success"]
     assert msg["error"] == {"code": "not_found", "message": "Device not found"}
+
+
+async def test_automation_with_unknown_device(hass, caplog, fake_integration):
+    """Test device automation with a trigger with an unknown device."""
+
+    module_cache = hass.data.setdefault(loader.DATA_COMPONENTS, {})
+    module = module_cache["fake_integration.device_trigger"]
+    module.async_validate_trigger_config = AsyncMock()
+
+    assert await async_setup_component(
+        hass,
+        automation.DOMAIN,
+        {
+            automation.DOMAIN: {
+                "alias": "hello",
+                "trigger": {
+                    "platform": "device",
+                    "device_id": "no_such_device",
+                    "domain": "fake_integration",
+                },
+                "action": {"service": "test.automation", "entity_id": "hello.world"},
+            }
+        },
+    )
+
+    module.async_validate_trigger_config.assert_not_awaited()
+    assert (
+        "Automation with alias 'hello' failed to setup triggers and has been disabled: "
+        "Unknown device 'no_such_device'" in caplog.text
+    )
+
+
+async def test_automation_with_device_wrong_domain(
+    hass, caplog, device_reg, fake_integration
+):
+    """Test device automation where the device doesn't have the right config entry."""
+
+    module_cache = hass.data.setdefault(loader.DATA_COMPONENTS, {})
+    module = module_cache["fake_integration.device_trigger"]
+    module.async_validate_trigger_config = AsyncMock()
+
+    device_entry = device_reg.async_get_or_create(
+        config_entry_id="not_fake_integration_config_entry",
+        connections={(device_registry.CONNECTION_NETWORK_MAC, "12:34:56:AB:CD:EF")},
+    )
+    assert await async_setup_component(
+        hass,
+        automation.DOMAIN,
+        {
+            automation.DOMAIN: {
+                "alias": "hello",
+                "trigger": {
+                    "platform": "device",
+                    "device_id": device_entry.id,
+                    "domain": "fake_integration",
+                },
+                "action": {"service": "test.automation", "entity_id": "hello.world"},
+            }
+        },
+    )
+
+    module.async_validate_trigger_config.assert_not_awaited()
+    assert (
+        "Automation with alias 'hello' failed to setup triggers and has been disabled: "
+        f"Device '{device_entry.id}' has no config entry from domain 'fake_integration'"
+        in caplog.text
+    )
+
+
+async def test_automation_with_device_component_not_loaded(
+    hass, caplog, device_reg, fake_integration
+):
+    """Test device automation where the device's config entry is not loaded."""
+
+    module_cache = hass.data.setdefault(loader.DATA_COMPONENTS, {})
+    module = module_cache["fake_integration.device_trigger"]
+    module.async_validate_trigger_config = AsyncMock()
+    module.async_attach_trigger = AsyncMock()
+
+    config_entry = MockConfigEntry(domain="fake_integration", data={})
+    config_entry.add_to_hass(hass)
+    device_entry = device_reg.async_get_or_create(
+        config_entry_id=config_entry.entry_id,
+        connections={(device_registry.CONNECTION_NETWORK_MAC, "12:34:56:AB:CD:EF")},
+    )
+    assert await async_setup_component(
+        hass,
+        automation.DOMAIN,
+        {
+            automation.DOMAIN: {
+                "alias": "hello",
+                "trigger": {
+                    "platform": "device",
+                    "device_id": device_entry.id,
+                    "domain": "fake_integration",
+                },
+                "action": {"service": "test.automation", "entity_id": "hello.world"},
+            }
+        },
+    )
+
+    module.async_validate_trigger_config.assert_not_awaited()

tests/components/zha/test_device_trigger.py

@@ -297,7 +297,7 @@ async def test_device_offline_fires(
 
 
 async def test_exception_no_triggers(hass, mock_devices, calls, caplog):
-    """Test for exception on event triggers firing."""
+    """Test for exception when validating device triggers."""
 
     _, zha_device = mock_devices
 
@@ -327,11 +327,14 @@ async def test_exception_no_triggers(hass, mock_devices, calls, caplog):
         },
     )
     await hass.async_block_till_done()
-    assert "Invalid trigger configuration" in caplog.text
+    assert (
+        "Unnamed automation failed to setup triggers and has been disabled: "
+        "device does not have trigger ('junk', 'junk')" in caplog.text
+    )
 
 
 async def test_exception_bad_trigger(hass, mock_devices, calls, caplog):
-    """Test for exception on event triggers firing."""
+    """Test for exception when validating device triggers."""
 
     zigpy_device, zha_device = mock_devices
 
@@ -369,43 +372,7 @@ async def test_exception_bad_trigger(hass, mock_devices, calls, caplog):
         },
     )
     await hass.async_block_till_done()
-    assert "Invalid trigger configuration" in caplog.text
+    assert (
-
+        "Unnamed automation failed to setup triggers and has been disabled: "
-
+        "device does not have trigger ('junk', 'junk')" in caplog.text
-@pytest.mark.skip(reason="Temporarily disabled until automation validation is improved")
-async def test_exception_no_device(hass, mock_devices, calls, caplog):
-    """Test for exception on event triggers firing."""
-
-    zigpy_device, zha_device = mock_devices
-
-    zigpy_device.device_automation_triggers = {
-        (SHAKEN, SHAKEN): {COMMAND: COMMAND_SHAKE},
-        (DOUBLE_PRESS, DOUBLE_PRESS): {COMMAND: COMMAND_DOUBLE},
-        (SHORT_PRESS, SHORT_PRESS): {COMMAND: COMMAND_SINGLE},
-        (LONG_PRESS, LONG_PRESS): {COMMAND: COMMAND_HOLD},
-        (LONG_RELEASE, LONG_RELEASE): {COMMAND: COMMAND_HOLD},
-    }
-
-    await async_setup_component(
-        hass,
-        automation.DOMAIN,
-        {
-            automation.DOMAIN: [
-                {
-                    "trigger": {
-                        "device_id": "no_such_device_id",
-                        "domain": "zha",
-                        "platform": "device",
-                        "type": "junk",
-                        "subtype": "junk",
-                    },
-                    "action": {
-                        "service": "test.automation",
-                        "data": {"message": "service called"},
-                    },
-                }
-            ]
-        },
     )
-    await hass.async_block_till_done()
-    assert "Invalid trigger configuration" in caplog.text

tests/helpers/test_condition.py

@@ -3280,14 +3280,12 @@ async def test_trigger(hass):
 async def test_platform_async_validate_condition_config(hass):
     """Test platform.async_validate_condition_config will be called if it exists."""
     config = {CONF_DEVICE_ID: "test", CONF_DOMAIN: "test", CONF_CONDITION: "device"}
-    platform = AsyncMock()
     with patch(
-        "homeassistant.components.device_automation.condition.async_get_device_automation_platform",
+        "homeassistant.components.device_automation.condition.async_validate_condition_config",
-        return_value=platform,
+        AsyncMock(),
-    ):
+    ) as device_automation_validate_condition_mock:
-        platform.async_validate_condition_config.return_value = config
         await condition.async_validate_condition_config(hass, config)
-        platform.async_validate_condition_config.assert_awaited()
+        device_automation_validate_condition_mock.assert_awaited()
 
 
 async def test_disabled_condition(hass: HomeAssistant) -> None:

tests/helpers/test_script.py

@@ -4633,14 +4633,12 @@ async def test_breakpoints_2(hass):
 async def test_platform_async_validate_action_config(hass):
     """Test platform.async_validate_action_config will be called if it exists."""
     config = {CONF_DEVICE_ID: "test", CONF_DOMAIN: "test"}
-    platform = AsyncMock()
     with patch(
-        "homeassistant.components.device_automation.action.async_get_device_automation_platform",
+        "homeassistant.components.device_automation.action.async_validate_action_config",
-        return_value=platform,
+        return_value=AsyncMock(),
-    ):
+    ) as device_automation_validate_action_mock:
-        platform.async_validate_action_config.return_value = config
         await script.async_validate_action_config(hass, config)
-        platform.async_validate_action_config.assert_awaited()
+        device_automation_validate_action_mock.assert_awaited()
 
 
 async def test_stop_action(hass, caplog):