Skip to content

Add context to event trigger #40932

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
balloob merged 1 commit into from
Oct 1, 2020
Merged

Add context to event trigger #40932

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 22 additions & 15 deletions homeassistant/components/homeassistant/triggers/event.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

CONF_EVENT_TYPE = "event_type"
CONF_EVENT_DATA = "event_data"
CONF_EVENT_CONTEXT = "context"

_LOGGER = logging.getLogger(__name__)

Expand All @@ -19,36 +20,42 @@
vol.Required(CONF_PLATFORM): "event",
vol.Required(CONF_EVENT_TYPE): cv.string,
vol.Optional(CONF_EVENT_DATA): dict,
vol.Optional(CONF_EVENT_CONTEXT): dict,
}
)


def _populate_schema(config, config_parameter):
if config_parameter not in config:
Copy link
Copy Markdown
Member

@MartinHjelmare MartinHjelmare Oct 1, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The existing behavior was changed slightly with this check. The old check only set an event data schema if the config had a populated event data dict. The current check sets a schema as long as the event data key is in the config. It doesn't check for a truthy event data dict.

Copy link
Copy Markdown
Contributor Author

@OnFreund OnFreund Oct 1, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. I'm not sure this makes any practical difference (since the schema will be empty), but I'll fix this together with the multiple user selection below.

Copy link
Copy Markdown
Contributor Author

@OnFreund OnFreund Oct 2, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/home-assistant/core/pull/41036/files#diff-aa600ac67a083c2d308272918338761fR36-R37

return None

return vol.Schema(
{vol.Required(key): value for key, value in config[config_parameter].items()},
extra=vol.ALLOW_EXTRA,
)


async def async_attach_trigger(
hass, config, action, automation_info, *, platform_type="event"
):
"""Listen for events based on configuration."""
event_type = config.get(CONF_EVENT_TYPE)
event_data_schema = None
if config.get(CONF_EVENT_DATA):
event_data_schema = vol.Schema(
{
vol.Required(key): value
for key, value in config.get(CONF_EVENT_DATA).items()
},
extra=vol.ALLOW_EXTRA,
)
event_data_schema = _populate_schema(config, CONF_EVENT_DATA)
event_context_schema = _populate_schema(config, CONF_EVENT_CONTEXT)
Copy link
Copy Markdown
Member

@balloob balloob Oct 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just realized that matching straight up on context like this is going to be annoying if one wants to match multiple users.

Copy link
Copy Markdown
Contributor Author

@OnFreund OnFreund Oct 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. How about something like:

if isinstance(value, list):
    vol.Required(key): vol.In(value)
else:
    vol.Required(key): value

when building the schema? This can also be used for event_data, not just context.

Copy link
Copy Markdown
Contributor Author

@OnFreund OnFreund Oct 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Downside is that this could be a breaking change if the user intended to match a list.

Copy link
Copy Markdown
Contributor Author

@OnFreund OnFreund Oct 2, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed it here:
#41036

To ensure backwards compatibility I only added this for context, not data, even though I think it could be a nice touch for data as well.


@callback
def handle_event(event):
"""Listen for events and calls the action when data matches."""
if event_data_schema:
# Check that the event data matches the configured
try:
# Check that the event data and context match the configured
# schema if one was provided
try:
if event_data_schema:
event_data_schema(event.data)
except vol.Invalid:
# If event data doesn't match requested schema, skip event
return
if event_context_schema:
event_context_schema(event.context.as_dict())
except vol.Invalid:
# If event doesn't match, skip event
return

hass.async_run_job(
action,
Expand Down
73 changes: 62 additions & 11 deletions tests/components/homeassistant/triggers/test_event.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,12 @@ def calls(hass):
return async_mock_service(hass, "test", "automation")


@pytest.fixture
def context_with_user():
"""Track calls to a mock service."""
return Context(user_id="test_user_id")


@pytest.fixture(autouse=True)
def setup_comp(hass):
"""Initialize components."""
Expand Down Expand Up @@ -53,8 +59,8 @@ async def test_if_fires_on_event(hass, calls):
assert len(calls) == 1


async def test_if_fires_on_event_extra_data(hass, calls):
"""Test the firing of events still matches with event data."""
async def test_if_fires_on_event_extra_data(hass, calls, context_with_user):
"""Test the firing of events still matches with event data and context."""
assert await async_setup_component(
hass,
automation.DOMAIN,
Expand All @@ -65,8 +71,9 @@ async def test_if_fires_on_event_extra_data(hass, calls):
}
},
)

hass.bus.async_fire("test_event", {"extra_key": "extra_data"})
hass.bus.async_fire(
"test_event", {"extra_key": "extra_data"}, context=context_with_user
)
await hass.async_block_till_done()
assert len(calls) == 1

Expand All @@ -82,8 +89,8 @@ async def test_if_fires_on_event_extra_data(hass, calls):
assert len(calls) == 1


async def test_if_fires_on_event_with_data(hass, calls):
"""Test the firing of events with data."""
async def test_if_fires_on_event_with_data_and_context(hass, calls, context_with_user):
"""Test the firing of events with data and context."""
assert await async_setup_component(
hass,
automation.DOMAIN,
Expand All @@ -96,6 +103,7 @@ async def test_if_fires_on_event_with_data(hass, calls):
"some_attr": "some_value",
"second_attr": "second_value",
},
"context": {"user_id": context_with_user.user_id},
},
"action": {"service": "test.automation"},
}
Expand All @@ -105,17 +113,31 @@ async def test_if_fires_on_event_with_data(hass, calls):
hass.bus.async_fire(
"test_event",
{"some_attr": "some_value", "another": "value", "second_attr": "second_value"},
context=context_with_user,
)
await hass.async_block_till_done()
assert len(calls) == 1

hass.bus.async_fire("test_event", {"some_attr": "some_value", "another": "value"})
hass.bus.async_fire(
"test_event",
{"some_attr": "some_value", "another": "value"},
context=context_with_user,
)
await hass.async_block_till_done()
assert len(calls) == 1 # No new call

hass.bus.async_fire(
"test_event",
{"some_attr": "some_value", "another": "value", "second_attr": "second_value"},
)
await hass.async_block_till_done()
assert len(calls) == 1


async def test_if_fires_on_event_with_empty_data_config(hass, calls):
"""Test the firing of events with empty data config.
async def test_if_fires_on_event_with_empty_data_and_context_config(
hass, calls, context_with_user
):
"""Test the firing of events with empty data and context config.

The frontend automation editor can produce configurations with an
empty dict for event_data instead of no key.
Expand All @@ -129,13 +151,18 @@ async def test_if_fires_on_event_with_empty_data_config(hass, calls):
"platform": "event",
"event_type": "test_event",
"event_data": {},
"context": {},
},
"action": {"service": "test.automation"},
}
},
)

hass.bus.async_fire("test_event", {"some_attr": "some_value", "another": "value"})
hass.bus.async_fire(
"test_event",
{"some_attr": "some_value", "another": "value"},
context=context_with_user,
)
await hass.async_block_till_done()
assert len(calls) == 1

Expand Down Expand Up @@ -165,7 +192,7 @@ async def test_if_fires_on_event_with_nested_data(hass, calls):


async def test_if_not_fires_if_event_data_not_matches(hass, calls):
"""Test firing of event if no match."""
"""Test firing of event if no data match."""
assert await async_setup_component(
hass,
automation.DOMAIN,
Expand All @@ -184,3 +211,27 @@ async def test_if_not_fires_if_event_data_not_matches(hass, calls):
hass.bus.async_fire("test_event", {"some_attr": "some_other_value"})
await hass.async_block_till_done()
assert len(calls) == 0


async def test_if_not_fires_if_event_context_not_matches(
hass, calls, context_with_user
):
"""Test firing of event if no context match."""
assert await async_setup_component(
hass,
automation.DOMAIN,
{
automation.DOMAIN: {
"trigger": {
"platform": "event",
"event_type": "test_event",
"context": {"user_id": "some_user"},
},
"action": {"service": "test.automation"},
}
},
)

hass.bus.async_fire("test_event", {}, context=context_with_user)
await hass.async_block_till_done()
assert len(calls) == 0