From 1093c915942b63c81c385e65e1dd506e7caa23c5 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Tue, 12 May 2015 00:22:05 +0200 Subject: [PATCH 1/6] Allow dots in TriggerInstance payload by using EscapedDictField which automatically escapes special values. --- st2common/st2common/models/db/reactor.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/st2common/st2common/models/db/reactor.py b/st2common/st2common/models/db/reactor.py index 61397e1d68..0466bce68c 100644 --- a/st2common/st2common/models/db/reactor.py +++ b/st2common/st2common/models/db/reactor.py @@ -91,7 +91,7 @@ class TriggerInstanceDB(stormbase.StormFoundationDB): occurrence_time (datetime): time of occurrence of the trigger. """ trigger = me.StringField() - payload = me.DictField() + payload = stormbase.EscapedDictField() occurrence_time = me.DateTimeField() From 7a78d63716972e3e816438bfbc0b37329e14457f Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Tue, 12 May 2015 00:33:37 +0200 Subject: [PATCH 2/6] Fix Makefile - make sure we exit with non-zero if pylint task fails. --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 1d85a120f4..7892126c8f 100644 --- a/Makefile +++ b/Makefile @@ -90,7 +90,7 @@ pylint: requirements .pylint echo "==========================================================="; \ echo "Running pylint on" $$component; \ echo "==========================================================="; \ - . $(VIRTUALENV_DIR)/bin/activate; pylint -E --rcfile=./.pylintrc --load-plugins=pylint_plugins.api_models $$component/$$component; \ + . $(VIRTUALENV_DIR)/bin/activate; pylint -E --rcfile=./.pylintrc --load-plugins=pylint_plugins.api_models $$component/$$component || exit 1; \ done .PHONY: flake8 From 63e9f3ad1d1f9743284f29a8e2a5ae3610397e81 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Tue, 12 May 2015 00:36:33 +0200 Subject: [PATCH 3/6] Fix bug spot by pylint. --- st2actions/st2actions/resultstracker.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/st2actions/st2actions/resultstracker.py b/st2actions/st2actions/resultstracker.py index 1bb5d9060f..6159a50389 100644 --- a/st2actions/st2actions/resultstracker.py +++ b/st2actions/st2actions/resultstracker.py @@ -49,7 +49,7 @@ def start(self, wait=False): def wait(self): super(ResultsTracker, self).wait() - for thread in self._query_threads(): + for thread in self._query_threads: thread.wait() def shutdown(self): From ab6c75630863f7013efda34fc2cbd6c24972dbc5 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Tue, 12 May 2015 01:05:47 +0200 Subject: [PATCH 4/6] Add a test with a trigger instance which payload contains dots in the keys and values for the rules engine. --- st2reactor/tests/unit/test_rule_matcher.py | 28 ++++++++++++++++++---- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/st2reactor/tests/unit/test_rule_matcher.py b/st2reactor/tests/unit/test_rule_matcher.py index 8b527d46ee..0d563864bd 100644 --- a/st2reactor/tests/unit/test_rule_matcher.py +++ b/st2reactor/tests/unit/test_rule_matcher.py @@ -25,6 +25,7 @@ class RuleMatcherTest(DbTestCase): + rules = [] def test_get_matching_rules(self): self._setup_sample_trigger('st2.test.trigger1') @@ -40,6 +41,21 @@ def test_get_matching_rules(self): self.assertTrue(matching_rules is not None) self.assertEqual(len(matching_rules), 1) + def test_trigger_instance_payload_with_special_values(self): + # Test a rule where TriggerInstance payload contains a dot (".") + self._setup_sample_trigger('st2.test.trigger2') + trigger_instance = container_utils.create_trigger_instance( + 'dummy_pack_1.st2.test.trigger2', + {'k1': 't1_p_v', 'k2.k2': 'v2', 'k3.more.nested.deep': 'some.value'}, + datetime.datetime.utcnow() + ) + trigger = get_trigger_db_by_ref(trigger_instance.trigger) + rules = self._get_sample_rules() + rules_matcher = RulesMatcher(trigger_instance, trigger, rules) + matching_rules = rules_matcher.get_matching_rules() + self.assertTrue(matching_rules is not None) + self.assertEqual(len(matching_rules), 1) + def _setup_sample_trigger(self, name): trigtype = TriggerTypeDB() trigtype.name = name @@ -58,7 +74,9 @@ def _setup_sample_trigger(self, name): Trigger.add_or_update(created) def _get_sample_rules(self): - rules = [] + if self.rules: + # Make sure rules are created only once + return self.rules RULE_1 = { 'enabled': True, @@ -85,7 +103,7 @@ def _get_sample_rules(self): rule_api = RuleAPI(**RULE_1) rule_db = RuleAPI.to_model(rule_api) rule_db = Rule.add_or_update(rule_db) - rules.append(rule_db) + self.rules.append(rule_db) RULE_2 = { # Rule should match. 'enabled': True, @@ -112,7 +130,7 @@ def _get_sample_rules(self): rule_api = RuleAPI(**RULE_2) rule_db = RuleAPI.to_model(rule_api) rule_db = Rule.add_or_update(rule_db) - rules.append(rule_db) + self.rules.append(rule_db) RULE_3 = { 'enabled': False, # Disabled rule shouldn't match. @@ -139,6 +157,6 @@ def _get_sample_rules(self): rule_api = RuleAPI(**RULE_3) rule_db = RuleAPI.to_model(rule_api) rule_db = Rule.add_or_update(rule_db) - rules.append(rule_db) + self.rules.append(rule_db) - return rules + return self.rules From 53f628a562505e65116ec84dc2f2b804770a7502 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Tue, 12 May 2015 01:09:00 +0200 Subject: [PATCH 5/6] Update changelog. --- CHANGELOG.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 9861e0a09d..5cbba13f17 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -9,6 +9,8 @@ in development * Add support for caching of the retrieved auth tokens to the CLI. (new-feature) * Throw a more-user friendly exception when enforcing a rule if an action referenced inside the rule definition doesn't exist. (improvement) +* Fix a bug with the rule evaluation failing if the trigger payload contained a key with a + dot in the name. (bug-fix) v0.9.0 - April 29, 2015 ----------------------- From e09b8b7a0176e88302af32fb66286b54b743dea3 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Tue, 12 May 2015 01:09:45 +0200 Subject: [PATCH 6/6] Also add a test case for payload containing dollar sign. --- st2reactor/tests/unit/test_rule_matcher.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/st2reactor/tests/unit/test_rule_matcher.py b/st2reactor/tests/unit/test_rule_matcher.py index 0d563864bd..a7347b29cd 100644 --- a/st2reactor/tests/unit/test_rule_matcher.py +++ b/st2reactor/tests/unit/test_rule_matcher.py @@ -42,11 +42,12 @@ def test_get_matching_rules(self): self.assertEqual(len(matching_rules), 1) def test_trigger_instance_payload_with_special_values(self): - # Test a rule where TriggerInstance payload contains a dot (".") + # Test a rule where TriggerInstance payload contains a dot (".") and $ self._setup_sample_trigger('st2.test.trigger2') trigger_instance = container_utils.create_trigger_instance( 'dummy_pack_1.st2.test.trigger2', - {'k1': 't1_p_v', 'k2.k2': 'v2', 'k3.more.nested.deep': 'some.value'}, + {'k1': 't1_p_v', 'k2.k2': 'v2', 'k3.more.nested.deep': 'some.value', + 'k4.even.more.nested$': 'foo', 'yep$aaa': 'b'}, datetime.datetime.utcnow() ) trigger = get_trigger_db_by_ref(trigger_instance.trigger)