Merge pull request #196 from StackStorm/fix-variables-evaluation

Fix the yaql/jinja vars extraction to ignore methods of base ctx() dict

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

orquesta/expressions/base.py

@@ -144,7 +144,6 @@ def evaluate(statement, data=None):
 
 
 def extract_vars(statement):
-
     variables = []
 
     if isinstance(statement, dict):

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

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

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

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

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

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

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(

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(

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(