From 3942613bf433ccd93ba475f30b561ba4953f0a9f Mon Sep 17 00:00:00 2001 From: Tom Noble <53005340+TSNoble@users.noreply.github.com> Date: Sat, 10 Apr 2021 14:27:38 +0100 Subject: [PATCH] Enhancement/3837 (#3847) * Move event pattern validation into EventPattern class and apply enhanced pattern logic to all Rules * Fix exists filtering logic to only match leaf nodes in event * Apply black formatting * Replace JSONDecodeError with ValueError for Python2 compatibility * Update unit test names * Move event pattern tests into test_event_pattern.py * Apply black formatting Co-authored-by: TSNoble --- moto/events/models.py | 84 +++++++++-------------- moto/events/responses.py | 2 +- tests/test_events/test_event_pattern.py | 90 +++++++++++++++++++++++++ tests/test_events/test_events.py | 83 ----------------------- 4 files changed, 124 insertions(+), 135 deletions(-) create mode 100644 tests/test_events/test_event_pattern.py diff --git a/moto/events/models.py b/moto/events/models.py index 9b9b4d50d..69970ae2e 100644 --- a/moto/events/models.py +++ b/moto/events/models.py @@ -33,7 +33,7 @@ class Rule(CloudFormationModel): def __init__(self, name, region_name, **kwargs): self.name = name self.region_name = region_name - self.event_pattern = kwargs.get("EventPattern") + self.event_pattern = EventPattern(kwargs.get("EventPattern")) self.schedule_exp = kwargs.get("ScheduleExpression") self.state = kwargs.get("State") or "ENABLED" self.description = kwargs.get("Description") @@ -100,7 +100,7 @@ class Rule(CloudFormationModel): if event_bus_name != self.event_bus_name: return - if not self._validate_event(event): + if not self.event_pattern.matches_event(event): return # supported targets @@ -123,23 +123,6 @@ class Rule(CloudFormationModel): else: raise NotImplementedError("Expr not defined for {0}".format(type(self))) - def _validate_event(self, event): - for field, pattern in json.loads(self.event_pattern).items(): - if not isinstance(pattern, list): - # to keep it simple at the beginning only pattern with 1 level of depth are validated - continue - - if isinstance(pattern[0], dict): - if "exists" in pattern[0]: - if pattern[0]["exists"] and field not in event: - return False - elif not pattern[0]["exists"] and field in event: - return False - elif event.get(field) not in pattern: - return False - - return True - def _parse_arn(self, arn): # http://docs.aws.amazon.com/general/latest/gr/aws-arns-and-namespaces.html # this method needs probably some more fine tuning, @@ -191,8 +174,7 @@ class Rule(CloudFormationModel): archive_name, archive_uuid = resource_id.split(":") archive = events_backends[self.region_name].archives.get(archive_name) if archive.uuid == archive_uuid: - if archive.event_pattern.matches_event(event): - archive.events.append(event) + archive.events.append(event) def _send_to_sqs_queue(self, resource_id, event, group_id=None): from moto.sqs import sqs_backends @@ -413,7 +395,7 @@ class Archive(CloudFormationModel): if description: self.description = description if event_pattern: - self.event_pattern = event_pattern + self.event_pattern = EventPattern(event_pattern) if retention: self.retention = retention @@ -562,11 +544,36 @@ class Replay(BaseModel): class EventPattern: def __init__(self, filter): - self._filter = json.loads(filter) if filter else None + self._filter = self._load_event_pattern(filter) + if not self._validate_event_pattern(self._filter): + raise InvalidEventPatternException def __str__(self): return json.dumps(self._filter) + def _load_event_pattern(self, pattern): + try: + return json.loads(pattern) if pattern else None + except ValueError: + raise InvalidEventPatternException + + def _validate_event_pattern(self, pattern): + # values in the event pattern have to be either a dict or an array + if pattern is None: + return True + + dicts_valid = [ + self._validate_event_pattern(value) + for value in pattern.values() + if isinstance(value, dict) + ] + non_dicts_valid = [ + isinstance(value, list) + for value in pattern.values() + if not isinstance(value, dict) + ] + return all(dicts_valid) and all(non_dicts_valid) + def matches_event(self, event): if not self._filter: return True @@ -600,9 +607,10 @@ class EventPattern: def _does_item_match_named_filter(self, item, filter): filter_name, filter_value = list(filter.items())[0] if filter_name == "exists": - item_exists = item is not None + is_leaf_node = not isinstance(item, dict) + leaf_exists = is_leaf_node and item is not None should_exist = filter_value - return item_exists if should_exist else not item_exists + return leaf_exists if should_exist else not leaf_exists if filter_name == "prefix": prefix = filter_value return item.startswith(prefix) @@ -1052,9 +1060,6 @@ class EventsBackend(BaseBackend): "Member must have length less than or equal to 48".format(name) ) - if event_pattern: - self._validate_event_pattern(event_pattern) - event_bus = self._get_event_bus(source_arn) if name in self.archives: @@ -1104,26 +1109,6 @@ class EventsBackend(BaseBackend): return archive - def _validate_event_pattern(self, pattern): - try: - json_pattern = json.loads(pattern) - except ValueError: # json.JSONDecodeError exists since Python 3.5 - raise InvalidEventPatternException - - if not self._is_event_value_an_array(json_pattern): - raise InvalidEventPatternException - - def _is_event_value_an_array(self, pattern): - # the values of a key in the event pattern have to be either a dict or an array - for value in pattern.values(): - if isinstance(value, dict): - if not self._is_event_value_an_array(value): - return False - elif not isinstance(value, list): - return False - - return True - def describe_archive(self, name): archive = self.archives.get(name) @@ -1168,9 +1153,6 @@ class EventsBackend(BaseBackend): if not archive: raise ResourceNotFoundException("Archive {} does not exist.".format(name)) - if event_pattern: - self._validate_event_pattern(event_pattern) - archive.update(description, event_pattern, retention) return { diff --git a/moto/events/responses.py b/moto/events/responses.py index 78109c7f5..d7e4cc5ac 100644 --- a/moto/events/responses.py +++ b/moto/events/responses.py @@ -20,7 +20,7 @@ class EventsHandler(BaseResponse): return { "Name": rule.name, "Arn": rule.arn, - "EventPattern": rule.event_pattern, + "EventPattern": str(rule.event_pattern), "State": rule.state, "Description": rule.description, "ScheduleExpression": rule.schedule_exp, diff --git a/tests/test_events/test_event_pattern.py b/tests/test_events/test_event_pattern.py new file mode 100644 index 000000000..934c69d8b --- /dev/null +++ b/tests/test_events/test_event_pattern.py @@ -0,0 +1,90 @@ +import json + +import pytest + +from moto.events.models import EventPattern + + +def test_event_pattern_with_allowed_values_event_filter(): + pattern = EventPattern(json.dumps({"source": ["foo", "bar"]})) + assert pattern.matches_event({"source": "foo"}) + assert pattern.matches_event({"source": "bar"}) + assert not pattern.matches_event({"source": "baz"}) + + +def test_event_pattern_with_nested_event_filter(): + pattern = EventPattern(json.dumps({"detail": {"foo": ["bar"]}})) + assert pattern.matches_event({"detail": {"foo": "bar"}}) + assert not pattern.matches_event({"detail": {"foo": "baz"}}) + + +def test_event_pattern_with_exists_event_filter(): + foo_exists = EventPattern(json.dumps({"detail": {"foo": [{"exists": True}]}})) + assert foo_exists.matches_event({"detail": {"foo": "bar"}}) + assert not foo_exists.matches_event({"detail": {}}) + # exists filters only match leaf nodes of an event + assert not foo_exists.matches_event({"detail": {"foo": {"bar": "baz"}}}) + + foo_not_exists = EventPattern(json.dumps({"detail": {"foo": [{"exists": False}]}})) + assert not foo_not_exists.matches_event({"detail": {"foo": "bar"}}) + assert foo_not_exists.matches_event({"detail": {}}) + assert foo_not_exists.matches_event({"detail": {"foo": {"bar": "baz"}}}) + + bar_exists = EventPattern(json.dumps({"detail": {"bar": [{"exists": True}]}})) + assert not bar_exists.matches_event({"detail": {"foo": "bar"}}) + assert not bar_exists.matches_event({"detail": {}}) + + bar_not_exists = EventPattern(json.dumps({"detail": {"bar": [{"exists": False}]}})) + assert bar_not_exists.matches_event({"detail": {"foo": "bar"}}) + assert bar_not_exists.matches_event({"detail": {}}) + + +def test_event_pattern_with_prefix_event_filter(): + pattern = EventPattern(json.dumps({"detail": {"foo": [{"prefix": "bar"}]}})) + assert pattern.matches_event({"detail": {"foo": "bar"}}) + assert pattern.matches_event({"detail": {"foo": "bar!"}}) + assert not pattern.matches_event({"detail": {"foo": "ba"}}) + + +@pytest.mark.parametrize( + "operator, compare_to, should_match, should_not_match", + [ + ("<", 1, [0], [1, 2]), + ("<=", 1, [0, 1], [2]), + ("=", 1, [1], [0, 2]), + (">", 1, [2], [0, 1]), + (">=", 1, [1, 2], [0]), + ], +) +def test_event_pattern_with_single_numeric_event_filter( + operator, compare_to, should_match, should_not_match +): + pattern = EventPattern( + json.dumps({"detail": {"foo": [{"numeric": [operator, compare_to]}]}}) + ) + for number in should_match: + assert pattern.matches_event({"detail": {"foo": number}}) + for number in should_not_match: + assert not pattern.matches_event({"detail": {"foo": number}}) + + +def test_event_pattern_with_multi_numeric_event_filter(): + events = [{"detail": {"foo": number}} for number in range(5)] + + one_or_two = EventPattern( + json.dumps({"detail": {"foo": [{"numeric": [">=", 1, "<", 3]}]}}) + ) + assert not one_or_two.matches_event(events[0]) + assert one_or_two.matches_event(events[1]) + assert one_or_two.matches_event(events[2]) + assert not one_or_two.matches_event(events[3]) + assert not one_or_two.matches_event(events[4]) + + two_or_three = EventPattern( + json.dumps({"detail": {"foo": [{"numeric": [">", 1, "<=", 3]}]}}) + ) + assert not two_or_three.matches_event(events[0]) + assert not two_or_three.matches_event(events[1]) + assert two_or_three.matches_event(events[2]) + assert two_or_three.matches_event(events[3]) + assert not two_or_three.matches_event(events[4]) diff --git a/tests/test_events/test_events.py b/tests/test_events/test_events.py index 8d2047a69..5d68f6d56 100644 --- a/tests/test_events/test_events.py +++ b/tests/test_events/test_events.py @@ -14,7 +14,6 @@ from moto import mock_logs from moto.core import ACCOUNT_ID from moto.core.utils import iso_8601_datetime_without_milliseconds from moto.events import mock_events -from moto.events.models import EventPattern RULES = [ {"Name": "test1", "ScheduleExpression": "rate(5 minutes)"}, @@ -1518,88 +1517,6 @@ def test_archive_event_with_bus_arn(): response["SizeBytes"].should.be.greater_than(0) -def test_archive_with_allowed_values_event_filter(): - pattern = EventPattern(json.dumps({"source": ["foo", "bar"]})) - assert pattern.matches_event({"source": "foo"}) - assert pattern.matches_event({"source": "bar"}) - assert not pattern.matches_event({"source": "baz"}) - - -def test_archive_with_nested_event_filter(): - pattern = EventPattern(json.dumps({"detail": {"foo": ["bar"]}})) - assert pattern.matches_event({"detail": {"foo": "bar"}}) - assert not pattern.matches_event({"detail": {"foo": "baz"}}) - - -def test_archive_with_exists_event_filter(): - foo_exists = EventPattern(json.dumps({"detail": {"foo": [{"exists": True}]}})) - assert foo_exists.matches_event({"detail": {"foo": "bar"}}) - assert not foo_exists.matches_event({"detail": {}}) - - foo_not_exists = EventPattern(json.dumps({"detail": {"foo": [{"exists": False}]}})) - assert not foo_not_exists.matches_event({"detail": {"foo": "bar"}}) - assert foo_not_exists.matches_event({"detail": {}}) - - bar_exists = EventPattern(json.dumps({"detail": {"bar": [{"exists": True}]}})) - assert not bar_exists.matches_event({"detail": {"foo": "bar"}}) - assert not bar_exists.matches_event({"detail": {}}) - - bar_not_exists = EventPattern(json.dumps({"detail": {"bar": [{"exists": False}]}})) - assert bar_not_exists.matches_event({"detail": {"foo": "bar"}}) - assert bar_not_exists.matches_event({"detail": {}}) - - -def test_archive_with_prefix_event_filter(): - pattern = EventPattern(json.dumps({"detail": {"foo": [{"prefix": "bar"}]}})) - assert pattern.matches_event({"detail": {"foo": "bar"}}) - assert pattern.matches_event({"detail": {"foo": "bar!"}}) - assert not pattern.matches_event({"detail": {"foo": "ba"}}) - - -@pytest.mark.parametrize( - "operator, compare_to, should_match, should_not_match", - [ - ("<", 1, [0], [1, 2]), - ("<=", 1, [0, 1], [2]), - ("=", 1, [1], [0, 2]), - (">", 1, [2], [0, 1]), - (">=", 1, [1, 2], [0]), - ], -) -def test_archive_with_single_numeric_event_filter( - operator, compare_to, should_match, should_not_match -): - pattern = EventPattern( - json.dumps({"detail": {"foo": [{"numeric": [operator, compare_to]}]}}) - ) - for number in should_match: - assert pattern.matches_event({"detail": {"foo": number}}) - for number in should_not_match: - assert not pattern.matches_event({"detail": {"foo": number}}) - - -def test_archive_with_multi_numeric_event_filter(): - events = [{"detail": {"foo": number}} for number in range(5)] - - one_or_two = EventPattern( - json.dumps({"detail": {"foo": [{"numeric": [">=", 1, "<", 3]}]}}) - ) - assert not one_or_two.matches_event(events[0]) - assert one_or_two.matches_event(events[1]) - assert one_or_two.matches_event(events[2]) - assert not one_or_two.matches_event(events[3]) - assert not one_or_two.matches_event(events[4]) - - two_or_three = EventPattern( - json.dumps({"detail": {"foo": [{"numeric": [">", 1, "<=", 3]}]}}) - ) - assert not two_or_three.matches_event(events[0]) - assert not two_or_three.matches_event(events[1]) - assert two_or_three.matches_event(events[2]) - assert two_or_three.matches_event(events[3]) - assert not two_or_three.matches_event(events[4]) - - @mock_events def test_start_replay(): # given