Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 10 commits into from
Apr 10, 2020
2 changes: 2 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
-----
Expand Down
1 change: 0 additions & 1 deletion orquesta/expressions/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,6 @@ def evaluate(statement, data=None):


def extract_vars(statement):

variables = []

if isinstance(statement, dict):
Expand Down
36 changes: 23 additions & 13 deletions orquesta/expressions/jinja.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

import functools
import inspect
import itertools
import logging
import re
import six
Expand Down Expand Up @@ -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_pattern = '[a-zA-Z0-9_\'"\.\[\]\(\)]*'
_regex_ctx_patterns = [
'^ctx\(\)\.%s' % _regex_ctx_pattern, # line start ctx().*
m4dcoder marked this conversation as resolved.
Show resolved Hide resolved
'^ctx\([\'|"]?{0}[\'|"]?\)[\.{0}]?'.format(_regex_ctx_pattern), # line start ctx(*).*
blag marked this conversation as resolved.
Show resolved Hide resolved
'[\s]ctx\(\)\.%s' % _regex_ctx_pattern, # whitespace ctx().*
'[\s]ctx\([\'|"]?{0}[\'|"]?\)[\.{0}]?'.format(_regex_ctx_pattern) # whitespace ctx(*).*
]
_regex_ctx_var = '.*?(%s).*?' % '|'.join(_regex_ctx_patterns)
_regex_ctx_var_parser = re.compile(_regex_ctx_var)

_regex_var = '[a-zA-Z0-9_\-]+'
_regex_var_extracts = [
r'(?<=^ctx\(\)\.)(\b%s\b)(?!\()\.?' % _regex_var, # extract x in ctx().x
r'(?<=^ctx\()(\b%s\b)(?=\))\.?' % _regex_var, # extract x in ctx(x)
r'(?<=^ctx\(\')(\b%s\b)(?=\'\))\.?' % _regex_var, # extract x in ctx('x')
r'(?<=^ctx\(")(\b%s\b)(?="\))\.?' % _regex_var # extract x in ctx("x")
]

_block_delimiter = '{%}'
_regex_block_pattern = '{%.*?%}'
Expand Down Expand Up @@ -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)))
36 changes: 23 additions & 13 deletions orquesta/expressions/yql.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
# limitations under the License.

import inspect
import itertools
import logging
import re
import six
Expand Down Expand Up @@ -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_pattern = '[a-zA-Z0-9_\'"\.\[\]\(\)]*'
_regex_ctx_patterns = [
'^ctx\(\)\.%s' % _regex_ctx_pattern, # line start ctx().*
'^ctx\([\'|"]?{0}[\'|"]?\)[\.{0}]?'.format(_regex_ctx_pattern), # line start ctx(*).*
'[\s]ctx\(\)\.%s' % _regex_ctx_pattern, # whitespace ctx().*
'[\s]ctx\([\'|"]?{0}[\'|"]?\)[\.{0}]?'.format(_regex_ctx_pattern) # whitespace ctx(*).*
]
_regex_ctx_var = '.*?(%s).*?' % '|'.join(_regex_ctx_patterns)
_regex_ctx_var_parser = re.compile(_regex_ctx_var)

_regex_var = '[a-zA-Z0-9_\-]+'
_regex_var_extracts = [
r'(?<=^ctx\(\)\.)(\b%s\b)(?!\()\.?' % _regex_var, # extract x in ctx().x
r'(?<=^ctx\()(\b%s\b)(?=\))\.?' % _regex_var, # extract x in ctx(x)
r'(?<=^ctx\(\')(\b%s\b)(?=\'\))\.?' % _regex_var, # extract x in ctx('x')
r'(?<=^ctx\(")(\b%s\b)(?="\))\.?' % _regex_var # extract x in ctx("x")
]

_engine = yaql.language.factory.YaqlFactory().create()
_root_ctx = yaql.create_context()
Expand Down Expand Up @@ -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)))
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
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 fooctx(foo) and fooctx("bar") and fooctx(\'fu\') }}'

self.assertListEqual([], expr_base.extract_vars(expr))

Expand Down Expand Up @@ -52,12 +52,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))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
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\') }}'

self.assertListEqual([], expr_base.extract_vars(expr))

Expand Down Expand Up @@ -52,12 +52,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))
Expand Down Expand Up @@ -108,3 +113,20 @@ 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\') }}'
)

expected_vars = []

self.assertListEqual(expected_vars, expr_base.extract_vars(expr))
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
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 fooctx(foo) and fooctx("bar") and fooctx(\'fu\') %>'

self.assertListEqual([], expr_base.extract_vars(expr))

Expand Down Expand Up @@ -52,12 +52,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))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
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\') %>'

self.assertListEqual([], expr_base.extract_vars(expr))

Expand Down Expand Up @@ -52,12 +52,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))
Expand Down Expand Up @@ -108,3 +110,20 @@ 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\') %>'
)

expected_vars = []

self.assertListEqual(expected_vars, expr_base.extract_vars(expr))
Original file line number Diff line number Diff line change
Expand Up @@ -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 fooctx().bar }}'

self.assertListEqual([], self.evaluator.extract_vars(expr))

Expand Down Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 fooctx(foo) and fooctx("bar") and fooctx(\'fu\') }}'

self.assertListEqual([], self.evaluator.extract_vars(expr))

Expand Down Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 fooctx().bar %>'

self.assertListEqual([], self.evaluator.extract_vars(expr))

Expand Down Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 fooctx(foo) and fooctx("bar") and fooctx(\'fu\') %>'

self.assertListEqual([], self.evaluator.extract_vars(expr))

Expand Down Expand Up @@ -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(
Expand Down