From 0e27cccecbb032f415b22d89f074f932b391ec06 Mon Sep 17 00:00:00 2001 From: Eric Ethington Date: Mon, 6 Dec 2021 14:45:47 -0700 Subject: [PATCH 1/3] Added 'multiple' operator to allow complex criteria matching on payload items. --- st2common/st2common/operators.py | 99 ++++ st2common/tests/unit/test_operators.py | 612 +++++++++++++++++++++++++ st2reactor/st2reactor/rules/filter.py | 5 +- st2reactor/tests/unit/test_filter.py | 16 + 4 files changed, 730 insertions(+), 2 deletions(-) diff --git a/st2common/st2common/operators.py b/st2common/st2common/operators.py index 6896e87658..50ae133038 100644 --- a/st2common/st2common/operators.py +++ b/st2common/st2common/operators.py @@ -161,6 +161,103 @@ def search(value, criteria_pattern, criteria_condition, check_function): return rtn +def multiple(value, criteria_pattern, criteria_condition, check_function): + """ + Allow comparison of payload items to multiple criteria using different logicial conditions. + Performs same function as the "search" operator and contains additional features. + + value: the payload items + condition: one of: + * all2all - true if all payload items match all criteria items + * all2any - true if all payload items match any criteria items + * any2any - true if any payload items match any criteria items + * any2all - true if any payload items match all criteria items + * all - same as all2all (useful to maintain backward compatibility with search operator) + * any - same as any2all (useful to maintain backward compatibility with search operator) + pattern: a dictionary of criteria to apply to each item of the list + + This operator has O(n) algorithmic complexity in terms of number of child patterns. + This operator has O(n) algorithmic complexity in terms of number of payload fields. + + However, it has O(n_patterns * n_payloads) algorithmic complexity, where: + n_patterns = number of child patterns + n_payloads = number of fields in payload + It is therefore very easy to write a slow rule when using this operator. + + This operator should ONLY be used when trying to match a small number of child patterns and/or + a small number of payload list elements. + + Data from the trigger: + + { + "fields": [ + { + "field_name": "waterLevel", + "to_value": 45, + } + ] + } + + And an example usage in criteria: + + --- + criteria: + trigger.fields: + type: multiple + # Controls whether this criteria has to match any or all items of the list + condition: all2all # all2any, any2all or any2any + pattern: + # "#" and text after are ignored. This allows dictionary keys to be unique but refer to the same field + # Any text can go after the hash. + item.field_name#1: + type: "greaterthan" + pattern: 40 + + item.field_name#2: + type: "lessthan" + pattern: 50 + """ + if isinstance(value, dict): + value = [value] + criteria_condition_list = criteria_condition.split('2', 1) + if (not((len(criteria_condition_list) == 1 and (criteria_condition_list[0] == 'any' or criteria_condition_list[0] == 'all')) or + (len(criteria_condition_list) == 2 and (criteria_condition_list[0] == 'any' or criteria_condition_list[0] == 'all') and + (criteria_condition_list[1] == 'any' or criteria_condition_list[1] == 'all')))): + raise UnrecognizedConditionError( + "The '%s' condition is not recognized for type multiple, 'any', 'all', 'any2any', 'any2all', 'all2any'" + " and 'all2all are allowed" % criteria_condition + ) + payloadItemMatch = any + if criteria_condition_list[0] == 'all': + payloadItemMatch = all + patternMatch = all + if len(criteria_condition_list) == 2 and criteria_condition_list[1] == 'any': + patternMatch = any + + rtn = payloadItemMatch( + [ + # any/all payload item can match + patternMatch( + [ + # Match any/all patterns + check_function( + child_criterion_k, + child_criterion_v, + PayloadLookup( + child_payload, prefix=TRIGGER_ITEM_PAYLOAD_PREFIX + ) + ) + for child_criterion_k, child_criterion_v in six.iteritems( + criteria_pattern + ) + ] + ) + for child_payload in value + ] + ) + return rtn + + def equals(value, criteria_pattern): if criteria_pattern is None: return False @@ -412,6 +509,7 @@ def ensure_operators_are_strings(value, criteria_pattern): NINSIDE_LONG = "ninside" NINSIDE_SHORT = "nin" SEARCH = "search" +MULTIPLE = "multiple" # operator lookups operators = { @@ -448,4 +546,5 @@ def ensure_operators_are_strings(value, criteria_pattern): NINSIDE_LONG: ninside, NINSIDE_SHORT: ninside, SEARCH: search, + MULTIPLE: multiple } diff --git a/st2common/tests/unit/test_operators.py b/st2common/tests/unit/test_operators.py index 5917e4277c..9561150876 100644 --- a/st2common/tests/unit/test_operators.py +++ b/st2common/tests/unit/test_operators.py @@ -565,6 +565,618 @@ def record_function_args(criterion_k, criterion_v, payload_lookup): ) +class MultipleOperatorTest(unittest2.TestCase): + # First few tests are the same as search to show it can do everything search can do. + # The later tests show the extra stuff the multiple operator can do. + def test_multiple_with_weird_condition(self): + op = operators.get_operator("multiple") + + with self.assertRaises(operators.UnrecognizedConditionError): + op([], [], "weird", None) + + def test_multiple_any_true(self): + op = operators.get_operator("multiple") + + called_function_args = [] + + def record_function_args(criterion_k, criterion_v, payload_lookup): + called_function_args.append( + { + "criterion_k": criterion_k, + "criterion_v": criterion_v, + "payload_lookup": { + "field_name": payload_lookup.get_value("item.field_name")[0], + "to_value": payload_lookup.get_value("item.to_value")[0], + }, + } + ) + return len(called_function_args) < 3 + + payload = [ + { + "field_name": "Status", + "to_value": "Approved", + }, + { + "field_name": "Assigned to", + "to_value": "Stanley", + }, + ] + + criteria_pattern = { + "item.field_name": { + "type": "equals", + "pattern": "Status", + }, + "item.to_value": { + "type": "equals", + "pattern": "Approved", + }, + } + + result = op(payload, criteria_pattern, "any", record_function_args) + + self.assertTrue(result) + self.assertTrue( + list_of_dicts_strict_equal( + called_function_args, + [ + # Outer loop: payload -> {'field_name': "Status", 'to_value': "Approved"} + { + # Inner loop: criterion -> item.field_name: {'type': "equals", 'pattern': "Status"} + "criterion_k": "item.field_name", + "criterion_v": { + "type": "equals", + "pattern": "Status", + }, + "payload_lookup": { + "field_name": "Status", + "to_value": "Approved", + }, + }, + { + # Inner loop: criterion -> item.to_value: {'type': "equals", 'pattern': "Approved"} + "criterion_k": "item.to_value", + "criterion_v": { + "type": "equals", + "pattern": "Approved", + }, + "payload_lookup": { + "field_name": "Status", + "to_value": "Approved", + }, + }, + # Outer loop: payload -> {'field_name': "Assigned to", 'to_value': "Stanley"} + { + # Inner loop: criterion -> item.field_name: {'type': "equals", 'pattern': "Status"} + "criterion_k": "item.field_name", + "criterion_v": { + "type": "equals", + "pattern": "Status", + }, + "payload_lookup": { + "field_name": "Assigned to", + "to_value": "Stanley", + }, + }, + { + # Inner loop: criterion -> item.to_value: {'type': "equals", 'pattern': "Approved"} + "criterion_k": "item.to_value", + "criterion_v": { + "type": "equals", + "pattern": "Approved", + }, + "payload_lookup": { + "field_name": "Assigned to", + "to_value": "Stanley", + }, + }, + ], + ) + ) + + def test_multiple_any_false(self): + op = operators.get_operator("multiple") + + called_function_args = [] + + def record_function_args(criterion_k, criterion_v, payload_lookup): + called_function_args.append( + { + "criterion_k": criterion_k, + "criterion_v": criterion_v, + "payload_lookup": { + "field_name": payload_lookup.get_value("item.field_name")[0], + "to_value": payload_lookup.get_value("item.to_value")[0], + }, + } + ) + return (len(called_function_args) % 2) == 0 + + payload = [ + { + "field_name": "Status", + "to_value": "Denied", + }, + { + "field_name": "Assigned to", + "to_value": "Stanley", + }, + ] + + criteria_pattern = { + "item.field_name": { + "type": "equals", + "pattern": "Status", + }, + "item.to_value": { + "type": "equals", + "pattern": "Approved", + }, + } + + result = op(payload, criteria_pattern, "any", record_function_args) + + self.assertFalse(result) + self.assertEqual( + called_function_args, + [ + # Outer loop: payload -> {'field_name': "Status", 'to_value': "Denied"} + { + # Inner loop: criterion -> item.field_name: {'type': "equals", 'pattern': "Status"} + "criterion_k": "item.field_name", + "criterion_v": { + "type": "equals", + "pattern": "Status", + }, + "payload_lookup": { + "field_name": "Status", + "to_value": "Denied", + }, + }, + { + # Inner loop: criterion -> item.to_value: {'type': "equals", 'pattern': "Approved"} + "criterion_k": "item.to_value", + "criterion_v": { + "type": "equals", + "pattern": "Approved", + }, + "payload_lookup": { + "field_name": "Status", + "to_value": "Denied", + }, + }, + # Outer loop: payload -> {'field_name': "Assigned to", 'to_value': "Stanley"} + { + # Inner loop: criterion -> item.to_value: {'type': "equals", 'pattern': "Approved"} + "criterion_k": "item.field_name", + "criterion_v": { + "type": "equals", + "pattern": "Status", + }, + "payload_lookup": { + "field_name": "Assigned to", + "to_value": "Stanley", + }, + }, + { + # Inner loop: criterion -> item.to_value: {'type': "equals", 'pattern': "Approved"} + "criterion_k": "item.to_value", + "criterion_v": { + "type": "equals", + "pattern": "Approved", + }, + "payload_lookup": { + "field_name": "Assigned to", + "to_value": "Stanley", + }, + }, + ], + ) + + def test_multiple_all_false(self): + op = operators.get_operator("multiple") + + called_function_args = [] + + def record_function_args(criterion_k, criterion_v, payload_lookup): + called_function_args.append( + { + "criterion_k": criterion_k, + "criterion_v": criterion_v, + "payload_lookup": { + "field_name": payload_lookup.get_value("item.field_name")[0], + "to_value": payload_lookup.get_value("item.to_value")[0], + }, + } + ) + return (len(called_function_args) % 2) == 0 + + payload = [ + { + "field_name": "Status", + "to_value": "Approved", + }, + { + "field_name": "Assigned to", + "to_value": "Stanley", + }, + ] + + criteria_pattern = { + "item.field_name": { + "type": "equals", + "pattern": "Status", + }, + "item.to_value": { + "type": "equals", + "pattern": "Approved", + }, + } + + result = op(payload, criteria_pattern, "all", record_function_args) + + self.assertFalse(result) + self.assertEqual( + called_function_args, + [ + # Outer loop: payload -> {'field_name': "Status", 'to_value': "Approved"} + { + # Inner loop: item.field_name -> {'type': "equals", 'pattern': "Status"} + "criterion_k": "item.field_name", + "criterion_v": { + "type": "equals", + "pattern": "Status", + }, + "payload_lookup": { + "field_name": "Status", + "to_value": "Approved", + }, + }, + { + # Inner loop: item.to_value -> {'type': "equals", 'pattern': "Approved"} + "criterion_k": "item.to_value", + "criterion_v": { + "type": "equals", + "pattern": "Approved", + }, + "payload_lookup": { + "field_name": "Status", + "to_value": "Approved", + }, + }, + # Outer loop: payload -> {'field_name': "Assigned to", 'to_value': "Stanley"} + { + # Inner loop: item.field_name -> {'type': "equals", 'pattern': "Status"} + "criterion_k": "item.field_name", + "criterion_v": { + "type": "equals", + "pattern": "Status", + }, + "payload_lookup": { + "field_name": "Assigned to", + "to_value": "Stanley", + }, + }, + { + # Inner loop: item.to_value -> {'type': "equals", 'pattern': "Approved"} + "criterion_k": "item.to_value", + "criterion_v": { + "type": "equals", + "pattern": "Approved", + }, + "payload_lookup": { + "field_name": "Assigned to", + "to_value": "Stanley", + }, + }, + ], + ) + + + def test_multiple_all_true(self): + op = operators.get_operator("multiple") + + called_function_args = [] + + def record_function_args(criterion_k, criterion_v, payload_lookup): + called_function_args.append( + { + "criterion_k": criterion_k, + "criterion_v": criterion_v, + "payload_lookup": { + "field_name": payload_lookup.get_value("item.field_name")[0], + "to_value": payload_lookup.get_value("item.to_value")[0], + }, + } + ) + return True + + payload = [ + { + "field_name": "Status", + "to_value": "Approved", + }, + { + "field_name": "Signed off by", + "to_value": "Approved", + }, + ] + + criteria_pattern = { + "item.field_name": { + "type": "startswith", + "pattern": "S", + }, + "item.to_value": { + "type": "equals", + "pattern": "Approved", + }, + } + + result = op(payload, criteria_pattern, "all", record_function_args) + + self.assertTrue(result) + self.assertEqual( + called_function_args, + [ + # Outer loop: payload -> {'field_name': "Status", 'to_value': "Approved"} + { + # Inner loop: item.field_name -> {'type': "startswith", 'pattern': "S"} + "criterion_k": "item.field_name", + "criterion_v": { + "type": "startswith", + "pattern": "S", + }, + "payload_lookup": { + "field_name": "Status", + "to_value": "Approved", + }, + }, + { + # Inner loop: item.to_value -> {'type': "equals", 'pattern': "Approved"} + "criterion_k": "item.to_value", + "criterion_v": { + "type": "equals", + "pattern": "Approved", + }, + "payload_lookup": { + "field_name": "Status", + "to_value": "Approved", + }, + }, + # Outer loop: payload -> {'field_name': "Signed off by", 'to_value': "Approved"} + { + # Inner loop: item.field_name -> {'type': "startswith", 'pattern': "S"} + "criterion_k": "item.field_name", + "criterion_v": { + "type": "startswith", + "pattern": "S", + }, + "payload_lookup": { + "field_name": "Signed off by", + "to_value": "Approved", + }, + }, + { + # Inner loop: item.to_value -> {'type': "equals", 'pattern': "Approved"} + "criterion_k": "item.to_value", + "criterion_v": { + "type": "equals", + "pattern": "Approved", + }, + "payload_lookup": { + "field_name": "Signed off by", + "to_value": "Approved", + }, + }, + ], + ) + + + def _test_function(self, criterion_k, criterion_v, payload_lookup): + op = operators.get_operator(criterion_v['type']) + return op(payload_lookup.get_value("item.to_value")[0], criterion_v["pattern"]) + + + def test_multiple_any2any(self): + # true if any payload items match any criteria + op = operators.get_operator('multiple') + + payload = [ + { + "field_name": "waterLevel", + "to_value": 30, + }, + { + "field_name": "waterLevel", + "to_value": 45, + } + ] + + criteria_pattern = { + "item.waterLevel#1": { + "type": "lessthan", + "pattern": 40, + }, + "item.waterLevel#2": { + "type": "greaterthan", + "pattern": 50, + }, + } + + result = op(payload, criteria_pattern, "any2any", self._test_function) + self.assertTrue(result) + + payload[0]['to_value'] = 44 + + result = op(payload, criteria_pattern, "any2any", self._test_function) + self.assertFalse(result) + + + def test_multiple_any2all(self): + # true if any payload items match all criteria + op = operators.get_operator("multiple") + payload = [ + { + "field_name": "waterLevel", + "to_value": 45, + }, + { + "field_name": "waterLevel", + "to_value": 20, + }, + ] + + criteria_pattern = { + "item.waterLevel#1": { + "type": "greaterthan", + "pattern": 40, + }, + "item.waterLevel#2": { + "type": "lessthan", + "pattern": 50, + }, + "item.waterLevel#3": { + "type": "equals", + "pattern": 46, + } + } + + result = op(payload, criteria_pattern, "any2all", self._test_function) + self.assertFalse(result) + + payload[0]['to_value'] = 46 + + result = op(payload, criteria_pattern, "any2all", self._test_function) + self.assertTrue(result) + + payload[0]['to_value'] = 45 + del criteria_pattern['item.waterLevel#3'] + + result = op(payload, criteria_pattern, "any2all", self._test_function) + self.assertTrue(result) + + + def test_multiple_all2any(self): + # true if all payload items match any criteria + op = operators.get_operator("multiple") + payload = [ + { + "field_name": "waterLevel", + "to_value": 45, + }, + { + "field_name": "waterLevel", + "to_value": 20, + }, + ] + + criteria_pattern = { + "item.waterLevel#1": { + "type": "greaterthan", + "pattern": 40, + }, + "item.waterLevel#2": { + "type": "lessthan", + "pattern": 50, + }, + "item.waterLevel#3": { + "type": "equals", + "pattern": 46, + } + } + + result = op(payload, criteria_pattern, "all2any", self._test_function) + self.assertTrue(result) + + criteria_pattern['item.waterLevel#2']['type'] = 'greaterthan' + + result = op(payload, criteria_pattern, "all2any", self._test_function) + self.assertFalse(result) + + + def test_multiple_all2all(self): + # true if all payload items match all criteria items + op = operators.get_operator("multiple") + payload = [ + { + "field_name": "waterLevel", + "to_value": 45, + }, + { + "field_name": "waterLevel", + "to_value": 46, + } + ] + + criteria_pattern = { + "item.waterLevel#1": { + "type": "greaterthan", + "pattern": 40, + }, + "item.waterLevel#2": { + "type": "lessthan", + "pattern": 50, + } + } + + result = op(payload, criteria_pattern, "all2all", self._test_function) + self.assertTrue(result) + + payload[0]['to_value'] = 30 + + result = op(payload, criteria_pattern, "all2all", self._test_function) + self.assertFalse(result) + + payload[0]['to_value'] = 45 + + criteria_pattern['item.waterLevel#3'] = { + "type": "equals", + "pattern": 46, + } + + result = op(payload, criteria_pattern, "all2all", self._test_function) + self.assertFalse(result) + + + def test_multiple_payload_dict(self): + op = operators.get_operator("multiple") + payload = { + "field_name": "waterLevel", + "to_value": 45, + } + + criteria_pattern = { + "item.waterLevel#1": { + "type": "greaterthan", + "pattern": 40, + }, + "item.waterLevel#2": { + "type": "lessthan", + "pattern": 50, + } + } + + result = op(payload, criteria_pattern, "all2all", self._test_function) + self.assertTrue(result) + + payload['to_value'] = 30 + + result = op(payload, criteria_pattern, "all2all", self._test_function) + self.assertFalse(result) + + payload['to_value'] = 45 + + criteria_pattern['item.waterLevel#3'] = { + "type": "equals", + "pattern": 46, + } + + result = op(payload, criteria_pattern, "all2all", self._test_function) + self.assertFalse(result) + + class OperatorTest(unittest2.TestCase): def test_matchwildcard(self): op = operators.get_operator("matchwildcard") diff --git a/st2reactor/st2reactor/rules/filter.py b/st2reactor/st2reactor/rules/filter.py index 838adc5480..c92f764247 100644 --- a/st2reactor/st2reactor/rules/filter.py +++ b/st2reactor/st2reactor/rules/filter.py @@ -154,8 +154,9 @@ def _check_criterion(self, criterion_k, criterion_v, payload_lookup): return (False, None, None) + criterion_k_hash_strip = criterion_k.split('#', 1)[0] try: - matches = payload_lookup.get_value(criterion_k) + matches = payload_lookup.get_value(criterion_k_hash_strip) # pick value if only 1 matches else will end up being an array match. if matches: payload_value = matches[0] if len(matches) > 0 else matches @@ -171,7 +172,7 @@ def _check_criterion(self, criterion_k, criterion_v, payload_lookup): op_func = criteria_operators.get_operator(criteria_operator) try: - if criteria_operator == criteria_operators.SEARCH: + if (criteria_operator == criteria_operators.SEARCH) or (criteria_operator == criteria_operators.MULTIPLE): result = op_func( value=payload_value, criteria_pattern=criteria_pattern, diff --git a/st2reactor/tests/unit/test_filter.py b/st2reactor/tests/unit/test_filter.py index d1e42eaece..a4a9514852 100644 --- a/st2reactor/tests/unit/test_filter.py +++ b/st2reactor/tests/unit/test_filter.py @@ -414,3 +414,19 @@ class MockSystemLookup(object): } f = RuleFilter(MOCK_TRIGGER_INSTANCE, MOCK_TRIGGER, rule) self.assertTrue(f.filter()) + + def test_hash_strip_int_value(self): + rule = MOCK_RULE_1 + rule.criteria = {"trigger.int": {"type": "gt", "pattern": 0}, "trigger.int#2": {"type": "lt", "pattern": 2}} + f = RuleFilter(MOCK_TRIGGER_INSTANCE, MOCK_TRIGGER, rule) + self.assertTrue(f.filter(), "equals check should have passed.") + + rule = MOCK_RULE_1 + rule.criteria = {"trigger.int": {"type": "gt", "pattern": 2}, "trigger.int#2": {"type": "lt", "pattern": 3}} + f = RuleFilter(MOCK_TRIGGER_INSTANCE, MOCK_TRIGGER, rule) + self.assertFalse(f.filter(), "trigger value is gt than 0 but didn't match.") + + rule = MOCK_RULE_1 + rule.criteria = {"trigger.int#1": {"type": "lt", "pattern": 2}} + f = RuleFilter(MOCK_TRIGGER_INSTANCE, MOCK_TRIGGER, rule) + self.assertTrue(f.filter(), "trigger value is gt than 0 but didn't match.") From c742b2f06c1da570e903747c19f24083307d31d4 Mon Sep 17 00:00:00 2001 From: Eric Ethington Date: Wed, 19 Jan 2022 11:39:19 -0700 Subject: [PATCH 2/3] Enhanced 'search' operator to allow complex criteria matching on payload items --- st2common/st2common/operators.py | 143 ++------ st2common/tests/unit/test_operators.py | 489 ++----------------------- st2reactor/st2reactor/rules/filter.py | 5 +- st2reactor/tests/unit/test_filter.py | 10 +- 4 files changed, 69 insertions(+), 578 deletions(-) diff --git a/st2common/st2common/operators.py b/st2common/st2common/operators.py index 50ae133038..d71f770f03 100644 --- a/st2common/st2common/operators.py +++ b/st2common/st2common/operators.py @@ -58,8 +58,10 @@ def search(value, criteria_pattern, criteria_condition, check_function): value: the payload list to search condition: one of: - * any - return true if any items of the list match and false if none of them match - * all - return true if all items of the list match and false if any of them do not match + * any - return true if any payload items of the list match all criteria items + * all - return true if all payload items of the list match all criteria items + * all2any - return true if all payload items of the list match any criteria items + * any2any - return true if any payload items match any criteria items pattern: a dictionary of criteria to apply to each item of the list This operator has O(n) algorithmic complexity in terms of number of child patterns. @@ -86,18 +88,20 @@ def search(value, criteria_pattern, criteria_condition, check_function): ] } - And an example usage in criteria: + Example #1 --- criteria: trigger.fields: type: search # Controls whether this criteria has to match any or all items of the list - condition: any # or all + condition: any # or all or all2any or any2any pattern: # Here our context is each item of the list # All of these patterns have to match the item for the item to match # These are simply other operators applied to each item in the list + # "#" and text after are ignored. + # This allows dictionary keys to be unique but refer to the same field item.field_name: type: "equals" pattern: "Status" @@ -105,110 +109,7 @@ def search(value, criteria_pattern, criteria_condition, check_function): item.to_value: type: "equals" pattern: "Approved" - """ - if criteria_condition == "any": - # Any item of the list can match all patterns - rtn = any( - [ - # Any payload item can match - all( - [ - # Match all patterns - check_function( - child_criterion_k, - child_criterion_v, - PayloadLookup( - child_payload, prefix=TRIGGER_ITEM_PAYLOAD_PREFIX - ), - ) - for child_criterion_k, child_criterion_v in six.iteritems( - criteria_pattern - ) - ] - ) - for child_payload in value - ] - ) - elif criteria_condition == "all": - # Every item of the list must match all patterns - rtn = all( - [ - # All payload items must match - all( - [ - # Match all patterns - check_function( - child_criterion_k, - child_criterion_v, - PayloadLookup( - child_payload, prefix=TRIGGER_ITEM_PAYLOAD_PREFIX - ), - ) - for child_criterion_k, child_criterion_v in six.iteritems( - criteria_pattern - ) - ] - ) - for child_payload in value - ] - ) - else: - raise UnrecognizedConditionError( - "The '%s' search condition is not recognized, only 'any' " - "and 'all' are allowed" % criteria_condition - ) - - return rtn - - -def multiple(value, criteria_pattern, criteria_condition, check_function): - """ - Allow comparison of payload items to multiple criteria using different logicial conditions. - Performs same function as the "search" operator and contains additional features. - value: the payload items - condition: one of: - * all2all - true if all payload items match all criteria items - * all2any - true if all payload items match any criteria items - * any2any - true if any payload items match any criteria items - * any2all - true if any payload items match all criteria items - * all - same as all2all (useful to maintain backward compatibility with search operator) - * any - same as any2all (useful to maintain backward compatibility with search operator) - pattern: a dictionary of criteria to apply to each item of the list - - This operator has O(n) algorithmic complexity in terms of number of child patterns. - This operator has O(n) algorithmic complexity in terms of number of payload fields. - - However, it has O(n_patterns * n_payloads) algorithmic complexity, where: - n_patterns = number of child patterns - n_payloads = number of fields in payload - It is therefore very easy to write a slow rule when using this operator. - - This operator should ONLY be used when trying to match a small number of child patterns and/or - a small number of payload list elements. - - Data from the trigger: - - { - "fields": [ - { - "field_name": "waterLevel", - "to_value": 45, - } - ] - } - - And an example usage in criteria: - - --- - criteria: - trigger.fields: - type: multiple - # Controls whether this criteria has to match any or all items of the list - condition: all2all # all2any, any2all or any2any - pattern: - # "#" and text after are ignored. This allows dictionary keys to be unique but refer to the same field - # Any text can go after the hash. item.field_name#1: type: "greaterthan" pattern: 40 @@ -219,20 +120,20 @@ def multiple(value, criteria_pattern, criteria_condition, check_function): """ if isinstance(value, dict): value = [value] - criteria_condition_list = criteria_condition.split('2', 1) - if (not((len(criteria_condition_list) == 1 and (criteria_condition_list[0] == 'any' or criteria_condition_list[0] == 'all')) or - (len(criteria_condition_list) == 2 and (criteria_condition_list[0] == 'any' or criteria_condition_list[0] == 'all') and - (criteria_condition_list[1] == 'any' or criteria_condition_list[1] == 'all')))): - raise UnrecognizedConditionError( - "The '%s' condition is not recognized for type multiple, 'any', 'all', 'any2any', 'any2all', 'all2any'" - " and 'all2all are allowed" % criteria_condition - ) - payloadItemMatch = any - if criteria_condition_list[0] == 'all': - payloadItemMatch = all + payloadItemMatch = all patternMatch = all - if len(criteria_condition_list) == 2 and criteria_condition_list[1] == 'any': + if criteria_condition == "any": + payloadItemMatch = any + elif criteria_condition == "all2any": + patternMatch = any + elif criteria_condition == "any2any": + payloadItemMatch = any patternMatch = any + elif criteria_condition != "all": + raise UnrecognizedConditionError( + "The '%s' condition is not recognized for type search, 'any', 'all', 'any2any'" + " and 'all2any' are allowed" % criteria_condition + ) rtn = payloadItemMatch( [ @@ -245,7 +146,7 @@ def multiple(value, criteria_pattern, criteria_condition, check_function): child_criterion_v, PayloadLookup( child_payload, prefix=TRIGGER_ITEM_PAYLOAD_PREFIX - ) + ), ) for child_criterion_k, child_criterion_v in six.iteritems( criteria_pattern @@ -509,7 +410,6 @@ def ensure_operators_are_strings(value, criteria_pattern): NINSIDE_LONG = "ninside" NINSIDE_SHORT = "nin" SEARCH = "search" -MULTIPLE = "multiple" # operator lookups operators = { @@ -546,5 +446,4 @@ def ensure_operators_are_strings(value, criteria_pattern): NINSIDE_LONG: ninside, NINSIDE_SHORT: ninside, SEARCH: search, - MULTIPLE: multiple } diff --git a/st2common/tests/unit/test_operators.py b/st2common/tests/unit/test_operators.py index 9561150876..b7049ba13c 100644 --- a/st2common/tests/unit/test_operators.py +++ b/st2common/tests/unit/test_operators.py @@ -564,424 +564,13 @@ def record_function_args(criterion_k, criterion_v, payload_lookup): ], ) - -class MultipleOperatorTest(unittest2.TestCase): - # First few tests are the same as search to show it can do everything search can do. - # The later tests show the extra stuff the multiple operator can do. - def test_multiple_with_weird_condition(self): - op = operators.get_operator("multiple") - - with self.assertRaises(operators.UnrecognizedConditionError): - op([], [], "weird", None) - - def test_multiple_any_true(self): - op = operators.get_operator("multiple") - - called_function_args = [] - - def record_function_args(criterion_k, criterion_v, payload_lookup): - called_function_args.append( - { - "criterion_k": criterion_k, - "criterion_v": criterion_v, - "payload_lookup": { - "field_name": payload_lookup.get_value("item.field_name")[0], - "to_value": payload_lookup.get_value("item.to_value")[0], - }, - } - ) - return len(called_function_args) < 3 - - payload = [ - { - "field_name": "Status", - "to_value": "Approved", - }, - { - "field_name": "Assigned to", - "to_value": "Stanley", - }, - ] - - criteria_pattern = { - "item.field_name": { - "type": "equals", - "pattern": "Status", - }, - "item.to_value": { - "type": "equals", - "pattern": "Approved", - }, - } - - result = op(payload, criteria_pattern, "any", record_function_args) - - self.assertTrue(result) - self.assertTrue( - list_of_dicts_strict_equal( - called_function_args, - [ - # Outer loop: payload -> {'field_name': "Status", 'to_value': "Approved"} - { - # Inner loop: criterion -> item.field_name: {'type': "equals", 'pattern': "Status"} - "criterion_k": "item.field_name", - "criterion_v": { - "type": "equals", - "pattern": "Status", - }, - "payload_lookup": { - "field_name": "Status", - "to_value": "Approved", - }, - }, - { - # Inner loop: criterion -> item.to_value: {'type': "equals", 'pattern': "Approved"} - "criterion_k": "item.to_value", - "criterion_v": { - "type": "equals", - "pattern": "Approved", - }, - "payload_lookup": { - "field_name": "Status", - "to_value": "Approved", - }, - }, - # Outer loop: payload -> {'field_name': "Assigned to", 'to_value': "Stanley"} - { - # Inner loop: criterion -> item.field_name: {'type': "equals", 'pattern': "Status"} - "criterion_k": "item.field_name", - "criterion_v": { - "type": "equals", - "pattern": "Status", - }, - "payload_lookup": { - "field_name": "Assigned to", - "to_value": "Stanley", - }, - }, - { - # Inner loop: criterion -> item.to_value: {'type': "equals", 'pattern': "Approved"} - "criterion_k": "item.to_value", - "criterion_v": { - "type": "equals", - "pattern": "Approved", - }, - "payload_lookup": { - "field_name": "Assigned to", - "to_value": "Stanley", - }, - }, - ], - ) - ) - - def test_multiple_any_false(self): - op = operators.get_operator("multiple") - - called_function_args = [] - - def record_function_args(criterion_k, criterion_v, payload_lookup): - called_function_args.append( - { - "criterion_k": criterion_k, - "criterion_v": criterion_v, - "payload_lookup": { - "field_name": payload_lookup.get_value("item.field_name")[0], - "to_value": payload_lookup.get_value("item.to_value")[0], - }, - } - ) - return (len(called_function_args) % 2) == 0 - - payload = [ - { - "field_name": "Status", - "to_value": "Denied", - }, - { - "field_name": "Assigned to", - "to_value": "Stanley", - }, - ] - - criteria_pattern = { - "item.field_name": { - "type": "equals", - "pattern": "Status", - }, - "item.to_value": { - "type": "equals", - "pattern": "Approved", - }, - } - - result = op(payload, criteria_pattern, "any", record_function_args) - - self.assertFalse(result) - self.assertEqual( - called_function_args, - [ - # Outer loop: payload -> {'field_name': "Status", 'to_value': "Denied"} - { - # Inner loop: criterion -> item.field_name: {'type': "equals", 'pattern': "Status"} - "criterion_k": "item.field_name", - "criterion_v": { - "type": "equals", - "pattern": "Status", - }, - "payload_lookup": { - "field_name": "Status", - "to_value": "Denied", - }, - }, - { - # Inner loop: criterion -> item.to_value: {'type': "equals", 'pattern': "Approved"} - "criterion_k": "item.to_value", - "criterion_v": { - "type": "equals", - "pattern": "Approved", - }, - "payload_lookup": { - "field_name": "Status", - "to_value": "Denied", - }, - }, - # Outer loop: payload -> {'field_name': "Assigned to", 'to_value': "Stanley"} - { - # Inner loop: criterion -> item.to_value: {'type': "equals", 'pattern': "Approved"} - "criterion_k": "item.field_name", - "criterion_v": { - "type": "equals", - "pattern": "Status", - }, - "payload_lookup": { - "field_name": "Assigned to", - "to_value": "Stanley", - }, - }, - { - # Inner loop: criterion -> item.to_value: {'type': "equals", 'pattern': "Approved"} - "criterion_k": "item.to_value", - "criterion_v": { - "type": "equals", - "pattern": "Approved", - }, - "payload_lookup": { - "field_name": "Assigned to", - "to_value": "Stanley", - }, - }, - ], - ) - - def test_multiple_all_false(self): - op = operators.get_operator("multiple") - - called_function_args = [] - - def record_function_args(criterion_k, criterion_v, payload_lookup): - called_function_args.append( - { - "criterion_k": criterion_k, - "criterion_v": criterion_v, - "payload_lookup": { - "field_name": payload_lookup.get_value("item.field_name")[0], - "to_value": payload_lookup.get_value("item.to_value")[0], - }, - } - ) - return (len(called_function_args) % 2) == 0 - - payload = [ - { - "field_name": "Status", - "to_value": "Approved", - }, - { - "field_name": "Assigned to", - "to_value": "Stanley", - }, - ] - - criteria_pattern = { - "item.field_name": { - "type": "equals", - "pattern": "Status", - }, - "item.to_value": { - "type": "equals", - "pattern": "Approved", - }, - } - - result = op(payload, criteria_pattern, "all", record_function_args) - - self.assertFalse(result) - self.assertEqual( - called_function_args, - [ - # Outer loop: payload -> {'field_name': "Status", 'to_value': "Approved"} - { - # Inner loop: item.field_name -> {'type': "equals", 'pattern': "Status"} - "criterion_k": "item.field_name", - "criterion_v": { - "type": "equals", - "pattern": "Status", - }, - "payload_lookup": { - "field_name": "Status", - "to_value": "Approved", - }, - }, - { - # Inner loop: item.to_value -> {'type': "equals", 'pattern': "Approved"} - "criterion_k": "item.to_value", - "criterion_v": { - "type": "equals", - "pattern": "Approved", - }, - "payload_lookup": { - "field_name": "Status", - "to_value": "Approved", - }, - }, - # Outer loop: payload -> {'field_name': "Assigned to", 'to_value': "Stanley"} - { - # Inner loop: item.field_name -> {'type': "equals", 'pattern': "Status"} - "criterion_k": "item.field_name", - "criterion_v": { - "type": "equals", - "pattern": "Status", - }, - "payload_lookup": { - "field_name": "Assigned to", - "to_value": "Stanley", - }, - }, - { - # Inner loop: item.to_value -> {'type': "equals", 'pattern': "Approved"} - "criterion_k": "item.to_value", - "criterion_v": { - "type": "equals", - "pattern": "Approved", - }, - "payload_lookup": { - "field_name": "Assigned to", - "to_value": "Stanley", - }, - }, - ], - ) - - - def test_multiple_all_true(self): - op = operators.get_operator("multiple") - - called_function_args = [] - - def record_function_args(criterion_k, criterion_v, payload_lookup): - called_function_args.append( - { - "criterion_k": criterion_k, - "criterion_v": criterion_v, - "payload_lookup": { - "field_name": payload_lookup.get_value("item.field_name")[0], - "to_value": payload_lookup.get_value("item.to_value")[0], - }, - } - ) - return True - - payload = [ - { - "field_name": "Status", - "to_value": "Approved", - }, - { - "field_name": "Signed off by", - "to_value": "Approved", - }, - ] - - criteria_pattern = { - "item.field_name": { - "type": "startswith", - "pattern": "S", - }, - "item.to_value": { - "type": "equals", - "pattern": "Approved", - }, - } - - result = op(payload, criteria_pattern, "all", record_function_args) - - self.assertTrue(result) - self.assertEqual( - called_function_args, - [ - # Outer loop: payload -> {'field_name': "Status", 'to_value': "Approved"} - { - # Inner loop: item.field_name -> {'type': "startswith", 'pattern': "S"} - "criterion_k": "item.field_name", - "criterion_v": { - "type": "startswith", - "pattern": "S", - }, - "payload_lookup": { - "field_name": "Status", - "to_value": "Approved", - }, - }, - { - # Inner loop: item.to_value -> {'type': "equals", 'pattern': "Approved"} - "criterion_k": "item.to_value", - "criterion_v": { - "type": "equals", - "pattern": "Approved", - }, - "payload_lookup": { - "field_name": "Status", - "to_value": "Approved", - }, - }, - # Outer loop: payload -> {'field_name': "Signed off by", 'to_value': "Approved"} - { - # Inner loop: item.field_name -> {'type': "startswith", 'pattern': "S"} - "criterion_k": "item.field_name", - "criterion_v": { - "type": "startswith", - "pattern": "S", - }, - "payload_lookup": { - "field_name": "Signed off by", - "to_value": "Approved", - }, - }, - { - # Inner loop: item.to_value -> {'type': "equals", 'pattern': "Approved"} - "criterion_k": "item.to_value", - "criterion_v": { - "type": "equals", - "pattern": "Approved", - }, - "payload_lookup": { - "field_name": "Signed off by", - "to_value": "Approved", - }, - }, - ], - ) - - def _test_function(self, criterion_k, criterion_v, payload_lookup): - op = operators.get_operator(criterion_v['type']) + op = operators.get_operator(criterion_v["type"]) return op(payload_lookup.get_value("item.to_value")[0], criterion_v["pattern"]) - - def test_multiple_any2any(self): + def test_search_any2any(self): # true if any payload items match any criteria - op = operators.get_operator('multiple') + op = operators.get_operator("search") payload = [ { @@ -991,7 +580,7 @@ def test_multiple_any2any(self): { "field_name": "waterLevel", "to_value": 45, - } + }, ] criteria_pattern = { @@ -1008,15 +597,14 @@ def test_multiple_any2any(self): result = op(payload, criteria_pattern, "any2any", self._test_function) self.assertTrue(result) - payload[0]['to_value'] = 44 + payload[0]["to_value"] = 44 result = op(payload, criteria_pattern, "any2any", self._test_function) self.assertFalse(result) - - def test_multiple_any2all(self): + def test_search_any(self): # true if any payload items match all criteria - op = operators.get_operator("multiple") + op = operators.get_operator("search") payload = [ { "field_name": "waterLevel", @@ -1040,27 +628,26 @@ def test_multiple_any2all(self): "item.waterLevel#3": { "type": "equals", "pattern": 46, - } + }, } - result = op(payload, criteria_pattern, "any2all", self._test_function) + result = op(payload, criteria_pattern, "any", self._test_function) self.assertFalse(result) - payload[0]['to_value'] = 46 + payload[0]["to_value"] = 46 - result = op(payload, criteria_pattern, "any2all", self._test_function) + result = op(payload, criteria_pattern, "any", self._test_function) self.assertTrue(result) - payload[0]['to_value'] = 45 - del criteria_pattern['item.waterLevel#3'] + payload[0]["to_value"] = 45 + del criteria_pattern["item.waterLevel#3"] - result = op(payload, criteria_pattern, "any2all", self._test_function) + result = op(payload, criteria_pattern, "any", self._test_function) self.assertTrue(result) - - def test_multiple_all2any(self): + def test_search_all2any(self): # true if all payload items match any criteria - op = operators.get_operator("multiple") + op = operators.get_operator("search") payload = [ { "field_name": "waterLevel", @@ -1084,21 +671,20 @@ def test_multiple_all2any(self): "item.waterLevel#3": { "type": "equals", "pattern": 46, - } + }, } result = op(payload, criteria_pattern, "all2any", self._test_function) self.assertTrue(result) - criteria_pattern['item.waterLevel#2']['type'] = 'greaterthan' + criteria_pattern["item.waterLevel#2"]["type"] = "greaterthan" result = op(payload, criteria_pattern, "all2any", self._test_function) self.assertFalse(result) - - def test_multiple_all2all(self): + def test_search_all(self): # true if all payload items match all criteria items - op = operators.get_operator("multiple") + op = operators.get_operator("search") payload = [ { "field_name": "waterLevel", @@ -1107,7 +693,7 @@ def test_multiple_all2all(self): { "field_name": "waterLevel", "to_value": 46, - } + }, ] criteria_pattern = { @@ -1118,30 +704,29 @@ def test_multiple_all2all(self): "item.waterLevel#2": { "type": "lessthan", "pattern": 50, - } + }, } - result = op(payload, criteria_pattern, "all2all", self._test_function) + result = op(payload, criteria_pattern, "all", self._test_function) self.assertTrue(result) - payload[0]['to_value'] = 30 + payload[0]["to_value"] = 30 - result = op(payload, criteria_pattern, "all2all", self._test_function) + result = op(payload, criteria_pattern, "all", self._test_function) self.assertFalse(result) - payload[0]['to_value'] = 45 + payload[0]["to_value"] = 45 - criteria_pattern['item.waterLevel#3'] = { + criteria_pattern["item.waterLevel#3"] = { "type": "equals", "pattern": 46, } - result = op(payload, criteria_pattern, "all2all", self._test_function) + result = op(payload, criteria_pattern, "all", self._test_function) self.assertFalse(result) - - def test_multiple_payload_dict(self): - op = operators.get_operator("multiple") + def test_search_payload_dict(self): + op = operators.get_operator("search") payload = { "field_name": "waterLevel", "to_value": 45, @@ -1155,25 +740,25 @@ def test_multiple_payload_dict(self): "item.waterLevel#2": { "type": "lessthan", "pattern": 50, - } + }, } - result = op(payload, criteria_pattern, "all2all", self._test_function) + result = op(payload, criteria_pattern, "all", self._test_function) self.assertTrue(result) - payload['to_value'] = 30 + payload["to_value"] = 30 - result = op(payload, criteria_pattern, "all2all", self._test_function) + result = op(payload, criteria_pattern, "all", self._test_function) self.assertFalse(result) - payload['to_value'] = 45 + payload["to_value"] = 45 - criteria_pattern['item.waterLevel#3'] = { + criteria_pattern["item.waterLevel#3"] = { "type": "equals", "pattern": 46, } - result = op(payload, criteria_pattern, "all2all", self._test_function) + result = op(payload, criteria_pattern, "all", self._test_function) self.assertFalse(result) diff --git a/st2reactor/st2reactor/rules/filter.py b/st2reactor/st2reactor/rules/filter.py index c92f764247..f2b4c7b753 100644 --- a/st2reactor/st2reactor/rules/filter.py +++ b/st2reactor/st2reactor/rules/filter.py @@ -154,7 +154,8 @@ def _check_criterion(self, criterion_k, criterion_v, payload_lookup): return (False, None, None) - criterion_k_hash_strip = criterion_k.split('#', 1)[0] + # Avoids the dict unique keys limitation. Allows multiple evaluations of the same payload item by a rule. + criterion_k_hash_strip = criterion_k.split("#", 1)[0] try: matches = payload_lookup.get_value(criterion_k_hash_strip) # pick value if only 1 matches else will end up being an array match. @@ -172,7 +173,7 @@ def _check_criterion(self, criterion_k, criterion_v, payload_lookup): op_func = criteria_operators.get_operator(criteria_operator) try: - if (criteria_operator == criteria_operators.SEARCH) or (criteria_operator == criteria_operators.MULTIPLE): + if criteria_operator == criteria_operators.SEARCH: result = op_func( value=payload_value, criteria_pattern=criteria_pattern, diff --git a/st2reactor/tests/unit/test_filter.py b/st2reactor/tests/unit/test_filter.py index a4a9514852..fd7c9d5741 100644 --- a/st2reactor/tests/unit/test_filter.py +++ b/st2reactor/tests/unit/test_filter.py @@ -417,12 +417,18 @@ class MockSystemLookup(object): def test_hash_strip_int_value(self): rule = MOCK_RULE_1 - rule.criteria = {"trigger.int": {"type": "gt", "pattern": 0}, "trigger.int#2": {"type": "lt", "pattern": 2}} + rule.criteria = { + "trigger.int": {"type": "gt", "pattern": 0}, + "trigger.int#2": {"type": "lt", "pattern": 2}, + } f = RuleFilter(MOCK_TRIGGER_INSTANCE, MOCK_TRIGGER, rule) self.assertTrue(f.filter(), "equals check should have passed.") rule = MOCK_RULE_1 - rule.criteria = {"trigger.int": {"type": "gt", "pattern": 2}, "trigger.int#2": {"type": "lt", "pattern": 3}} + rule.criteria = { + "trigger.int": {"type": "gt", "pattern": 2}, + "trigger.int#2": {"type": "lt", "pattern": 3}, + } f = RuleFilter(MOCK_TRIGGER_INSTANCE, MOCK_TRIGGER, rule) self.assertFalse(f.filter(), "trigger value is gt than 0 but didn't match.") From 9925093b639e3cc374e189ce5350bf4d77eeff40 Mon Sep 17 00:00:00 2001 From: Eric Ethington Date: Wed, 19 Jan 2022 12:03:48 -0700 Subject: [PATCH 3/3] updated changelog with enhanced search entry --- CHANGELOG.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 83e57db146..6cd8eb2f6a 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -60,6 +60,10 @@ Added Contributed by @khushboobhatia01 +* Enhanced 'search' operator to allow complex criteria matching on payload items. #5482 + + Contributed by @erceth + Fixed ~~~~~