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 <tom.noble@bjss.com>

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 {

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,

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])

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