diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 58824bd2..d2214004 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -37,6 +37,8 @@ Fixed * When inspecting custom YAQL/Jinja function to see if there is a context arg, use getargspec for py2 and getfullargspec for py3. (bug fix) * Check syntax on with items task to ensure action is indented correctly. Fixes #184 (bug fix) +* Fix variable inspection where ctx().get() method calls are identified as errors. + Fixes StackStorm/st2#4866 (bug fix) 1.0.0 ----- diff --git a/orquesta/expressions/base.py b/orquesta/expressions/base.py index 1aacfc24..af27263a 100644 --- a/orquesta/expressions/base.py +++ b/orquesta/expressions/base.py @@ -144,7 +144,6 @@ def evaluate(statement, data=None): def extract_vars(statement): - variables = [] if isinstance(statement, dict): diff --git a/orquesta/expressions/jinja.py b/orquesta/expressions/jinja.py index 296f0fb3..8c4dd1de 100644 --- a/orquesta/expressions/jinja.py +++ b/orquesta/expressions/jinja.py @@ -14,6 +14,7 @@ import functools import inspect +import itertools import logging import re import six @@ -53,16 +54,23 @@ class JinjaEvaluator(expr_base.Evaluator): _regex_pattern = '{{.*?}}' _regex_parser = re.compile(_regex_pattern) - _regex_dot_pattern = '[a-zA-Z0-9_\'"\.\[\]\(\)]*' - _regex_ctx_pattern_1 = 'ctx\(\)\.%s' % _regex_dot_pattern - _regex_ctx_pattern_2 = 'ctx\([\'|"]?{0}[\'|"]?\)[\.{0}]?'.format(_regex_dot_pattern) - _regex_var_pattern = '.*?(%s|%s).*?' % (_regex_ctx_pattern_1, _regex_ctx_pattern_2) - _regex_var_parser = re.compile(_regex_var_pattern) - - _regex_dot_extract = '([a-zA-Z0-9_\-]*)' - _regex_ctx_extract_1 = 'ctx\(\)\.%s' % _regex_dot_extract - _regex_ctx_extract_2 = 'ctx\([\'|"]?%s(%s)' % (_regex_dot_extract, _regex_dot_pattern) - _regex_var_extracts = ['%s\.?' % _regex_ctx_extract_1, '%s\.?' % _regex_ctx_extract_2] + _regex_ctx_ref_pattern = r'[][a-zA-Z0-9_\'"\.()]*' + # match any of: + # word boundary ctx(*) + # word boundary ctx()* + # word boundary ctx().* + # word boundary ctx(*)* + # word boundary ctx(*).* + _regex_ctx_pattern = r'\bctx\([\'"]?{0}[\'"]?\)\.?{0}'.format(_regex_ctx_ref_pattern) + _regex_ctx_var_parser = re.compile(_regex_ctx_pattern) + + _regex_var = r'[a-zA-Z0-9_-]+' + _regex_var_extracts = [ + r'(?<=\bctx\(\)\.)({})\b(?!\()\.?'.format(_regex_var), # extract x in ctx().x + r'(?:\bctx\(({})\))'.format(_regex_var), # extract x in ctx(x) + r'(?:\bctx\(\'({})\'\))'.format(_regex_var), # extract x in ctx('x') + r'(?:\bctx\("({})"\))'.format(_regex_var) # extract x in ctx("x") + ] _block_delimiter = '{%}' _regex_block_pattern = '{%.*?%}' @@ -234,9 +242,11 @@ def extract_vars(cls, text): if not isinstance(text, six.string_types): raise ValueError('Text to be evaluated is not typeof string.') - variables = [] + results = [ + cls._regex_ctx_var_parser.findall(expr.strip(cls._delimiter).strip()) + for expr in cls._regex_parser.findall(text) + ] - for expr in cls._regex_parser.findall(text): - variables.extend(cls._regex_var_parser.findall(expr)) + variables = [v.strip() for v in itertools.chain.from_iterable(results)] return sorted(list(set(variables))) diff --git a/orquesta/expressions/yql.py b/orquesta/expressions/yql.py index 0676e076..a172d74c 100644 --- a/orquesta/expressions/yql.py +++ b/orquesta/expressions/yql.py @@ -13,6 +13,7 @@ # limitations under the License. import inspect +import itertools import logging import re import six @@ -53,16 +54,23 @@ class YAQLEvaluator(expr_base.Evaluator): _regex_pattern = '<%.*?%>' _regex_parser = re.compile(_regex_pattern) - _regex_dot_pattern = '[a-zA-Z0-9_\'"\.\[\]\(\)]*' - _regex_ctx_pattern_1 = 'ctx\(\)\.%s' % _regex_dot_pattern - _regex_ctx_pattern_2 = 'ctx\([\'|"]?{0}[\'|"]?\)[\.{0}]?'.format(_regex_dot_pattern) - _regex_var_pattern = '.*?(%s|%s).*?' % (_regex_ctx_pattern_1, _regex_ctx_pattern_2) - _regex_var_parser = re.compile(_regex_var_pattern) - - _regex_dot_extract = '([a-zA-Z0-9_\-]*)' - _regex_ctx_extract_1 = 'ctx\(\)\.%s' % _regex_dot_extract - _regex_ctx_extract_2 = 'ctx\([\'|"]?%s(%s)' % (_regex_dot_extract, _regex_dot_pattern) - _regex_var_extracts = ['%s\.?' % _regex_ctx_extract_1, '%s\.?' % _regex_ctx_extract_2] + _regex_ctx_ref_pattern = r'[][a-zA-Z0-9_\'"\.()]*' + # match any of: + # word boundary ctx(*) + # word boundary ctx()* + # word boundary ctx().* + # word boundary ctx(*)* + # word boundary ctx(*).* + _regex_ctx_pattern = r'\bctx\([\'"]?{0}[\'"]?\)\.?{0}'.format(_regex_ctx_ref_pattern) + _regex_ctx_var_parser = re.compile(_regex_ctx_pattern) + + _regex_var = r'[a-zA-Z0-9_-]+' + _regex_var_extracts = [ + r'(?<=\bctx\(\)\.)({})\b(?!\()\.?'.format(_regex_var), # extract x in ctx().x + r'(?:\bctx\(({})\))'.format(_regex_var), # extract x in ctx(x) + r'(?:\bctx\(\'({})\'\))'.format(_regex_var), # extract x in ctx('x') + r'(?:\bctx\("({})"\))'.format(_regex_var) # extract x in ctx("x") + ] _engine = yaql.language.factory.YaqlFactory().create() _root_ctx = yaql.create_context() @@ -153,9 +161,11 @@ def extract_vars(cls, text): if not isinstance(text, six.string_types): raise ValueError('Text to be evaluated is not typeof string.') - variables = [] + results = [ + cls._regex_ctx_var_parser.findall(expr.strip(cls._delimiter).strip()) + for expr in cls._regex_parser.findall(text) + ] - for expr in cls._regex_parser.findall(text): - variables.extend(cls._regex_var_parser.findall(expr)) + variables = [v.strip() for v in itertools.chain.from_iterable(results)] return sorted(list(set(variables))) diff --git a/orquesta/tests/unit/expressions/test_facade_jinja_ctx_by_dot_notation.py b/orquesta/tests/unit/expressions/test_facade_jinja_ctx_by_dot_notation.py index 374fe7f9..d77ddf72 100644 --- a/orquesta/tests/unit/expressions/test_facade_jinja_ctx_by_dot_notation.py +++ b/orquesta/tests/unit/expressions/test_facade_jinja_ctx_by_dot_notation.py @@ -19,7 +19,10 @@ class JinjaFacadeVariableExtractionTest(test_base.ExpressionFacadeEvaluatorTest): def test_empty_extraction(self): - expr = '{{ just_text and _not_a_var }}' + expr = ( + '{{ just_text and $not_a_var and notctx().bar and ' + 'ctx(). and ctx().() and ctx().-foobar and ctx().foobar() }}' + ) self.assertListEqual([], expr_base.extract_vars(expr)) @@ -52,12 +55,14 @@ def test_single_functional_var_extraction(self): self.assertListEqual(expected_vars, expr_base.extract_vars(expr)) def test_multiple_vars_extraction(self): - expr = '{{ ctx().foobar ctx().foo.get(bar) ctx().fu.bar ctx().fu.bar[0] }}' + expr = '{{ctx().fubar ctx().foobar ctx().foo.get(bar) ctx().fu.bar ctx().foobaz.bar[0] }}' expected_vars = [ ('jinja', expr, 'foo'), ('jinja', expr, 'foobar'), - ('jinja', expr, 'fu') + ('jinja', expr, 'foobaz'), + ('jinja', expr, 'fu'), + ('jinja', expr, 'fubar') ] self.assertListEqual(expected_vars, expr_base.extract_vars(expr)) diff --git a/orquesta/tests/unit/expressions/test_facade_jinja_ctx_by_function_arg.py b/orquesta/tests/unit/expressions/test_facade_jinja_ctx_by_function_arg.py index 69d429ab..b341da41 100644 --- a/orquesta/tests/unit/expressions/test_facade_jinja_ctx_by_function_arg.py +++ b/orquesta/tests/unit/expressions/test_facade_jinja_ctx_by_function_arg.py @@ -19,7 +19,15 @@ class JinjaFacadeVariableExtractionTest(test_base.ExpressionFacadeEvaluatorTest): def test_empty_extraction(self): - expr = '{{ just_text and _not_a_var }}' + expr = ( + '{{ just_text and $not_a_var and ' + 'notctx(foo) and notctx("bar") and notctx(\'fu\') ' + 'ctx("foo\') and ctx(\'foo") and ctx(foo") and ' + 'ctx("foo) and ctx(foo\') and ctx(\'foo) and ' + 'ctx(-foo) and ctx("-bar") and ctx(\'-fu\') and ' + 'ctx(foo.bar) and ctx("foo.bar") and ctx(\'foo.bar\') and ' + 'ctx(foo()) and ctx("foo()") and ctx(\'foo()\') }}' + ) self.assertListEqual([], expr_base.extract_vars(expr)) @@ -52,12 +60,17 @@ def test_single_functional_var_extraction(self): self.assertListEqual(expected_vars, expr_base.extract_vars(expr)) def test_multiple_vars_extraction(self): - expr = '{{ ctx("foobar") ctx("foo").get(bar) ctx("fu").bar ctx("fu").bar[0] }}' + expr = ( + '{{ctx("fubar") ctx("foobar") ctx("foo").get(bar) ' + 'ctx("fu").bar ctx("foobaz").bar[0] }}' + ) expected_vars = [ ('jinja', expr, 'foo'), ('jinja', expr, 'foobar'), - ('jinja', expr, 'fu') + ('jinja', expr, 'foobaz'), + ('jinja', expr, 'fu'), + ('jinja', expr, 'fubar') ] self.assertListEqual(expected_vars, expr_base.extract_vars(expr)) @@ -108,3 +121,21 @@ def test_vars_extraction_from_dict(self): ] self.assertListEqual(expected_vars, expr_base.extract_vars(expr)) + + def test_ignore_ctx_dict_funcs(self): + expr = '{{ctx().keys() and ctx().values() and ctx().set("b", 3) }}' + + expected_vars = [] + + self.assertListEqual(expected_vars, expr_base.extract_vars(expr)) + + def test_ignore_ctx_get_func_calls(self): + expr = ( + '{{ctx().get(foo) and ctx().get(bar) and ctx().get("fu") and ctx().get(\'baz\') and ' + 'ctx().get(foo, "bar") and ctx().get("fu", "bar") and ctx().get(\'bar\', \'foo\') and ' + 'ctx().get("foo\') and ctx().get(\'foo") and ctx().get("foo) and ctx().get(foo") }}' + ) + + expected_vars = [] + + self.assertListEqual(expected_vars, expr_base.extract_vars(expr)) diff --git a/orquesta/tests/unit/expressions/test_facade_yaql_ctx_by_dot_notation.py b/orquesta/tests/unit/expressions/test_facade_yaql_ctx_by_dot_notation.py index 1d51a3be..9c6081a3 100644 --- a/orquesta/tests/unit/expressions/test_facade_yaql_ctx_by_dot_notation.py +++ b/orquesta/tests/unit/expressions/test_facade_yaql_ctx_by_dot_notation.py @@ -19,7 +19,10 @@ class YAQLFacadeVariableExtractionTest(test_base.ExpressionFacadeEvaluatorTest): def test_empty_extraction(self): - expr = '<% just_text and $not_a_var %>' + expr = ( + '<% just_text and $not_a_var and notctx().bar and ' + 'ctx(). and ctx().() and ctx().-foobar and ctx().foobar() %>' + ) self.assertListEqual([], expr_base.extract_vars(expr)) @@ -52,12 +55,14 @@ def test_single_functional_var_extraction(self): self.assertListEqual(expected_vars, expr_base.extract_vars(expr)) def test_multiple_vars_extraction(self): - expr = '<% ctx().foobar ctx().foo.get(bar) ctx().fu.bar ctx().fu.bar[0] %>' + expr = '<%ctx().fubar ctx().foobar ctx().foo.get(bar) ctx().fu.bar ctx().foobaz.bar[0] %>' expected_vars = [ ('yaql', expr, 'foo'), ('yaql', expr, 'foobar'), - ('yaql', expr, 'fu') + ('yaql', expr, 'foobaz'), + ('yaql', expr, 'fu'), + ('yaql', expr, 'fubar') ] self.assertListEqual(expected_vars, expr_base.extract_vars(expr)) diff --git a/orquesta/tests/unit/expressions/test_facade_yaql_ctx_by_function_arg.py b/orquesta/tests/unit/expressions/test_facade_yaql_ctx_by_function_arg.py index 30af3f0a..4880f9a7 100644 --- a/orquesta/tests/unit/expressions/test_facade_yaql_ctx_by_function_arg.py +++ b/orquesta/tests/unit/expressions/test_facade_yaql_ctx_by_function_arg.py @@ -19,7 +19,15 @@ class YAQLFacadeVariableExtractionTest(test_base.ExpressionFacadeEvaluatorTest): def test_empty_extraction(self): - expr = '<% just_text and $not_a_var %>' + expr = ( + '<% just_text and $not_a_var and ' + 'notctx(foo) and notctx("bar") and notctx(\'fu\') ' + 'ctx("foo\') and ctx(\'foo") and ctx(foo") and ' + 'ctx("foo) and ctx(foo\') and ctx(\'foo) and ' + 'ctx(-foo) and ctx("-bar") and ctx(\'-fu\') and ' + 'ctx(foo.bar) and ctx("foo.bar") and ctx(\'foo.bar\') and ' + 'ctx(foo()) and ctx("foo()") and ctx(\'foo()\') %>' + ) self.assertListEqual([], expr_base.extract_vars(expr)) @@ -52,12 +60,14 @@ def test_single_functional_var_extraction(self): self.assertListEqual(expected_vars, expr_base.extract_vars(expr)) def test_multiple_vars_extraction(self): - expr = '<% ctx(foobar) ctx(foo).get(bar) ctx(fu).bar ctx(fu).bar[0] %>' + expr = '<%ctx(fubar) ctx(foobar) ctx(foo).get(bar) ctx(fu).bar ctx(foobaz).bar[0] %>' expected_vars = [ ('yaql', expr, 'foo'), ('yaql', expr, 'foobar'), - ('yaql', expr, 'fu') + ('yaql', expr, 'foobaz'), + ('yaql', expr, 'fu'), + ('yaql', expr, 'fubar') ] self.assertListEqual(expected_vars, expr_base.extract_vars(expr)) @@ -108,3 +118,21 @@ def test_vars_extraction_from_dict(self): ] self.assertListEqual(expected_vars, expr_base.extract_vars(expr)) + + def test_ignore_ctx_dict_funcs(self): + expr = '<%ctx().keys() and ctx().values() and ctx().set("b", 3) %>' + + expected_vars = [] + + self.assertListEqual(expected_vars, expr_base.extract_vars(expr)) + + def test_ignore_ctx_get_func_calls(self): + expr = ( + '<%ctx().get(foo) and ctx().get(bar) and ctx().get("fu") and ctx().get(\'baz\') and ' + 'ctx().get(foo, "bar") and ctx().get("fu", "bar") and ctx().get(\'bar\', \'foo\') and ' + 'ctx().get("foo\') and ctx().get(\'foo") and ctx().get("foo) and ctx().get(foo") %>' + ) + + expected_vars = [] + + self.assertListEqual(expected_vars, expr_base.extract_vars(expr)) diff --git a/orquesta/tests/unit/expressions/test_jinja_ctx_by_dot_notation.py b/orquesta/tests/unit/expressions/test_jinja_ctx_by_dot_notation.py index 1fc86e1b..d3755a7e 100644 --- a/orquesta/tests/unit/expressions/test_jinja_ctx_by_dot_notation.py +++ b/orquesta/tests/unit/expressions/test_jinja_ctx_by_dot_notation.py @@ -23,7 +23,7 @@ def setUpClass(cls): super(JinjaVariableExtractionTest, cls).setUpClass() def test_empty_extraction(self): - expr = '{{ just_text and _not_a_var }}' + expr = '{{ just_text and $not_a_var and notctx().bar }}' self.assertListEqual([], self.evaluator.extract_vars(expr)) @@ -64,13 +64,14 @@ def test_single_functional_var_extraction(self): self.assertListEqual(expected_vars, self.evaluator.extract_vars(expr)) def test_multiple_vars_extraction(self): - expr = '{{ ctx().foobar ctx().foo.get(bar) ctx().fu.bar ctx().fu.bar[0] }}' + expr = '{{ctx().fubar ctx().foobar ctx().foo.get(bar) ctx().fu.bar ctx().foobaz.bar[0] }}' expected_vars = [ 'ctx().foobar', + 'ctx().foobaz.bar[0]', 'ctx().foo.get(bar)', - 'ctx().fu.bar', - 'ctx().fu.bar[0]' + 'ctx().fubar', + 'ctx().fu.bar' ] self.assertListEqual( diff --git a/orquesta/tests/unit/expressions/test_jinja_ctx_by_function_arg.py b/orquesta/tests/unit/expressions/test_jinja_ctx_by_function_arg.py index 47ccedc4..b1ed7b62 100644 --- a/orquesta/tests/unit/expressions/test_jinja_ctx_by_function_arg.py +++ b/orquesta/tests/unit/expressions/test_jinja_ctx_by_function_arg.py @@ -23,7 +23,7 @@ def setUpClass(cls): super(JinjaVariableExtractionTest, cls).setUpClass() def test_empty_extraction(self): - expr = '{{ just_text and _not_a_var }}' + expr = '{{ just_text and $not_a_var and notctx(foo) and notctx("bar") and notctx(\'fu\') }}' self.assertListEqual([], self.evaluator.extract_vars(expr)) @@ -82,13 +82,14 @@ def test_single_functional_var_extraction(self): self.assertListEqual(expected_vars, self.evaluator.extract_vars(expr)) def test_multiple_vars_extraction(self): - expr = '{{ ctx(foobar) ctx(foo).get(bar) ctx(fu).bar ctx(fu).bar[0] }}' + expr = '{{ctx(fubar) ctx(foobar) ctx(foo).get(bar) ctx(fu).bar ctx(foobaz).bar[0] }}' expected_vars = [ 'ctx(foobar)', + 'ctx(foobaz).bar[0]', 'ctx(foo).get(bar)', - 'ctx(fu).bar', - 'ctx(fu).bar[0]' + 'ctx(fubar)', + 'ctx(fu).bar' ] self.assertListEqual( diff --git a/orquesta/tests/unit/expressions/test_yaql_ctx_by_dot_notation.py b/orquesta/tests/unit/expressions/test_yaql_ctx_by_dot_notation.py index 7fac7a1c..d6456544 100644 --- a/orquesta/tests/unit/expressions/test_yaql_ctx_by_dot_notation.py +++ b/orquesta/tests/unit/expressions/test_yaql_ctx_by_dot_notation.py @@ -23,7 +23,7 @@ def setUpClass(cls): super(YAQLVariableExtractionTest, cls).setUpClass() def test_empty_extraction(self): - expr = '<% just_text and $not_a_var %>' + expr = '<% just_text and $not_a_var and notctx().bar %>' self.assertListEqual([], self.evaluator.extract_vars(expr)) @@ -64,13 +64,14 @@ def test_single_functional_var_extraction(self): self.assertListEqual(expected_vars, self.evaluator.extract_vars(expr)) def test_multiple_vars_extraction(self): - expr = '<% ctx().foobar ctx().foo.get(bar) ctx().fu.bar ctx().fu.bar[0] %>' + expr = '<%ctx().fubar ctx().foobar ctx().foo.get(bar) ctx().fu.bar ctx().foobaz.bar[0] %>' expected_vars = [ 'ctx().foobar', + 'ctx().foobaz.bar[0]', 'ctx().foo.get(bar)', - 'ctx().fu.bar', - 'ctx().fu.bar[0]' + 'ctx().fubar', + 'ctx().fu.bar' ] self.assertListEqual( diff --git a/orquesta/tests/unit/expressions/test_yaql_ctx_by_function_arg.py b/orquesta/tests/unit/expressions/test_yaql_ctx_by_function_arg.py index 9a675ad2..34c9daab 100644 --- a/orquesta/tests/unit/expressions/test_yaql_ctx_by_function_arg.py +++ b/orquesta/tests/unit/expressions/test_yaql_ctx_by_function_arg.py @@ -23,7 +23,7 @@ def setUpClass(cls): super(YAQLVariableExtractionTest, cls).setUpClass() def test_empty_extraction(self): - expr = '<% just_text and $not_a_var %>' + expr = '<% just_text and $not_a_var and notctx(foo) and notctx("bar") and notctx(\'fu\') %>' self.assertListEqual([], self.evaluator.extract_vars(expr)) @@ -82,13 +82,14 @@ def test_single_functional_var_extraction(self): self.assertListEqual(expected_vars, self.evaluator.extract_vars(expr)) def test_multiple_vars_extraction(self): - expr = '<% ctx(foobar) ctx(foo).get(bar) ctx(fu).bar ctx(fu).bar[0] %>' + expr = '<%ctx(fubar) ctx(foobar) ctx(foo).get(bar) ctx(fu).bar ctx(foobaz).bar[0] %>' expected_vars = [ 'ctx(foobar)', + 'ctx(foobaz).bar[0]', 'ctx(foo).get(bar)', - 'ctx(fu).bar', - 'ctx(fu).bar[0]' + 'ctx(fubar)', + 'ctx(fu).bar' ] self.assertListEqual(