From 0da94c20b0ec219615443ff9dac4b9d5542fbe93 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 31 Aug 2023 13:47:01 -0400 Subject: [PATCH] Significantly reduce overhead to filter event triggers (#99376) * fast * cleanups * cleanups * cleanups * comment * comment * add more cover * comment * pull more examples from forums to validate cover --- .../homeassistant/triggers/event.py | 66 +++++++++---- .../homeassistant/triggers/test_event.py | 93 ++++++++++++++++++- 2 files changed, 138 insertions(+), 21 deletions(-) diff --git a/homeassistant/components/homeassistant/triggers/event.py b/homeassistant/components/homeassistant/triggers/event.py index d0e74d5b04e..a4266a70add 100644 --- a/homeassistant/components/homeassistant/triggers/event.py +++ b/homeassistant/components/homeassistant/triggers/event.py @@ -1,6 +1,7 @@ """Offer event listening automation rules.""" from __future__ import annotations +from collections.abc import ItemsView from typing import Any import voluptuous as vol @@ -47,9 +48,8 @@ async def async_attach_trigger( event_types = template.render_complex( config[CONF_EVENT_TYPE], variables, limited=True ) - removes = [] - - event_data_schema = None + event_data_schema: vol.Schema | None = None + event_data_items: ItemsView | None = None if CONF_EVENT_DATA in config: # Render the schema input template.attach(hass, config[CONF_EVENT_DATA]) @@ -57,13 +57,21 @@ async def async_attach_trigger( event_data.update( template.render_complex(config[CONF_EVENT_DATA], variables, limited=True) ) - # Build the schema - event_data_schema = vol.Schema( - {vol.Required(key): value for key, value in event_data.items()}, - extra=vol.ALLOW_EXTRA, - ) + # Build the schema or a an items view if the schema is simple + # and does not contain sub-dicts. We explicitly do not check for + # list like the context data below since lists are a special case + # only for context data. (see test test_event_data_with_list) + if any(isinstance(value, dict) for value in event_data.values()): + event_data_schema = vol.Schema( + {vol.Required(key): value for key, value in event_data.items()}, + extra=vol.ALLOW_EXTRA, + ) + else: + # Use a simple items comparison if possible + event_data_items = event_data.items() - event_context_schema = None + event_context_schema: vol.Schema | None = None + event_context_items: ItemsView | None = None if CONF_EVENT_CONTEXT in config: # Render the schema input template.attach(hass, config[CONF_EVENT_CONTEXT]) @@ -71,14 +79,23 @@ async def async_attach_trigger( event_context.update( template.render_complex(config[CONF_EVENT_CONTEXT], variables, limited=True) ) - # Build the schema - event_context_schema = vol.Schema( - { - vol.Required(key): _schema_value(value) - for key, value in event_context.items() - }, - extra=vol.ALLOW_EXTRA, - ) + # Build the schema or a an items view if the schema is simple + # and does not contain lists. Lists are a special case to support + # matching events by user_id. (see test test_if_fires_on_multiple_user_ids) + # This can likely be optimized further in the future to handle the + # multiple user_id case without requiring expensive schema + # validation. + if any(isinstance(value, list) for value in event_context.values()): + event_context_schema = vol.Schema( + { + vol.Required(key): _schema_value(value) + for key, value in event_context.items() + }, + extra=vol.ALLOW_EXTRA, + ) + else: + # Use a simple items comparison if possible + event_context_items = event_context.items() job = HassJob(action, f"event trigger {trigger_info}") @@ -88,9 +105,20 @@ async def async_attach_trigger( try: # Check that the event data and context match the configured # schema if one was provided - if event_data_schema: + if event_data_items: + # Fast path for simple items comparison + if not (event.data.items() >= event_data_items): + return False + elif event_data_schema: + # Slow path for schema validation event_data_schema(event.data) - if event_context_schema: + + if event_context_items: + # Fast path for simple items comparison + if not (event.context.as_dict().items() >= event_context_items): + return False + elif event_context_schema: + # Slow path for schema validation event_context_schema(dict(event.context.as_dict())) except vol.Invalid: # If event doesn't match, skip event diff --git a/tests/components/homeassistant/triggers/test_event.py b/tests/components/homeassistant/triggers/test_event.py index 6fc7e5055ed..d996cd74da7 100644 --- a/tests/components/homeassistant/triggers/test_event.py +++ b/tests/components/homeassistant/triggers/test_event.py @@ -288,7 +288,11 @@ async def test_if_fires_on_event_with_empty_data_and_context_config( async def test_if_fires_on_event_with_nested_data(hass: HomeAssistant, calls) -> None: - """Test the firing of events with nested data.""" + """Test the firing of events with nested data. + + This test exercises the slow path of using vol.Schema to validate + matching event data. + """ assert await async_setup_component( hass, automation.DOMAIN, @@ -311,6 +315,87 @@ async def test_if_fires_on_event_with_nested_data(hass: HomeAssistant, calls) -> assert len(calls) == 1 +async def test_if_fires_on_event_with_empty_data(hass: HomeAssistant, calls) -> None: + """Test the firing of events with empty data. + + This test exercises the fast path to validate matching event data. + """ + assert await async_setup_component( + hass, + automation.DOMAIN, + { + automation.DOMAIN: { + "trigger": { + "platform": "event", + "event_type": "test_event", + "event_data": {}, + }, + "action": {"service": "test.automation"}, + } + }, + ) + hass.bus.async_fire("test_event", {"any_attr": {}}) + await hass.async_block_till_done() + assert len(calls) == 1 + + +async def test_if_fires_on_sample_zha_event(hass: HomeAssistant, calls) -> None: + """Test the firing of events with a sample zha event. + + This test exercises the fast path to validate matching event data. + """ + assert await async_setup_component( + hass, + automation.DOMAIN, + { + automation.DOMAIN: { + "trigger": { + "platform": "event", + "event_type": "zha_event", + "event_data": { + "device_ieee": "00:15:8d:00:02:93:04:11", + "command": "attribute_updated", + "args": { + "attribute_id": 0, + "attribute_name": "on_off", + "value": True, + }, + }, + }, + "action": {"service": "test.automation"}, + } + }, + ) + + hass.bus.async_fire( + "zha_event", + { + "device_ieee": "00:15:8d:00:02:93:04:11", + "unique_id": "00:15:8d:00:02:93:04:11:1:0x0006", + "endpoint_id": 1, + "cluster_id": 6, + "command": "attribute_updated", + "args": {"attribute_id": 0, "attribute_name": "on_off", "value": True}, + }, + ) + await hass.async_block_till_done() + assert len(calls) == 1 + + hass.bus.async_fire( + "zha_event", + { + "device_ieee": "00:15:8d:00:02:93:04:11", + "unique_id": "00:15:8d:00:02:93:04:11:1:0x0006", + "endpoint_id": 1, + "cluster_id": 6, + "command": "attribute_updated", + "args": {"attribute_id": 0, "attribute_name": "on_off", "value": False}, + }, + ) + await hass.async_block_till_done() + assert len(calls) == 1 + + async def test_if_not_fires_if_event_data_not_matches( hass: HomeAssistant, calls ) -> None: @@ -362,7 +447,11 @@ async def test_if_not_fires_if_event_context_not_matches( async def test_if_fires_on_multiple_user_ids( hass: HomeAssistant, calls, context_with_user ) -> None: - """Test the firing of event when the trigger has multiple user ids.""" + """Test the firing of event when the trigger has multiple user ids. + + This test exercises the slow path of using vol.Schema to validate + matching event context. + """ assert await async_setup_component( hass, automation.DOMAIN,