Refactor expressions

* This patch moves code related to YAQL and Jinja into their
  specific modules so that there isn't any module that works with
  both. It makes it easier to understand how code related to one
  of these technologies works.
* Custome built-in functions for YAQL and Jinja are now in a
  separate module. It's easier now to see what's related with
  the expression framework now and what's with integration part,
  i.e. functions themselves.
* Renamed the base module of expressions similar to other packages.
* Other style changes.

Change-Id: I94f57a6534b9c10e202205dfae4d039296c26407

mistral/engine/actions.py

@@ -517,11 +517,13 @@ class AdHocAction(PythonAction):
 
     @profiler.trace('ad-hoc-action-gather-base-actions', hide_args=True)
     def _gather_base_actions(self, action_def, base_action_def):
-        """Find all base ad-hoc actions and store them
+        """Find all base ad-hoc actions and store them.
 
-        An ad-hoc action may be based on another ad-hoc action (and this
+        An ad-hoc action may be based on another ad-hoc action and this
-        recursively). Using twice the same base action is not allowed to
+        works recursively, so that the base action can also be based on an
-        avoid infinite loops. It stores the list of ad-hoc actions.
+        ad-hoc action. Using the same base action more than once in this
+        action hierarchy is not allowed to avoid infinite loops.
+        The method stores the list of ad-hoc actions.
 
         :param action_def: Action definition
         :type action_def: ActionDefinition
@@ -532,6 +534,7 @@ class AdHocAction(PythonAction):
         """
 
         self.adhoc_action_defs = [action_def]
+
         original_base_name = self.action_spec.get_name()
         action_names = set([original_base_name])
 

mistral/expressions/base.py

@@ -15,6 +15,8 @@
 
 import abc
 
+from stevedore import extension
+
 
 class Evaluator(object):
     """Expression evaluator interface.
@@ -53,3 +55,22 @@ class Evaluator(object):
         :return: True if string is expression
         """
         pass
+
+
+def get_custom_functions():
+    """Get custom functions.
+
+    Retrieves the list of custom functions used in YAQL/Jinja expressions.
+    """
+    # {name => function object).
+    result = dict()
+
+    mgr = extension.ExtensionManager(
+        namespace='mistral.expression.functions',
+        invoke_on_load=False
+    )
+
+    for name in mgr.names():
+        result[name] = mgr[name].plugin
+
+    return result

mistral/expressions/jinja_expression.py

@@ -12,6 +12,7 @@
 #    See the License for the specific language governing permissions and
 #    limitations under the License.
 
+from functools import partial
 import re
 
 import jinja2
@@ -22,8 +23,7 @@ from oslo_log import log as logging
 import six
 
 from mistral import exceptions as exc
-from mistral.expressions.base_expression import Evaluator
+from mistral.expressions import base
-from mistral.utils import expression_utils
 
 
 LOG = logging.getLogger(__name__)
@@ -41,13 +41,33 @@ _environment = SandboxedEnvironment(
     lstrip_blocks=True
 )
 
-_filters = expression_utils.get_custom_functions()
+_filters = base.get_custom_functions()
 
 for name in _filters:
     _environment.filters[name] = _filters[name]
 
 
-class JinjaEvaluator(Evaluator):
+def get_jinja_context(data_context):
+    new_ctx = {'_': data_context}
+
+    _register_jinja_functions(new_ctx)
+
+    if isinstance(data_context, dict):
+        new_ctx['__env'] = data_context.get('__env')
+        new_ctx['__execution'] = data_context.get('__execution')
+        new_ctx['__task_execution'] = data_context.get('__task_execution')
+
+    return new_ctx
+
+
+def _register_jinja_functions(jinja_ctx):
+    functions = base.get_custom_functions()
+
+    for name in functions:
+        jinja_ctx[name] = partial(functions[name], jinja_ctx['_'])
+
+
+class JinjaEvaluator(base.Evaluator):
     _env = _environment.overlay()
 
     @classmethod
@@ -62,18 +82,13 @@ class JinjaEvaluator(Evaluator):
 
             parser.parse_expression()
         except jinja2.exceptions.TemplateError as e:
-            raise exc.JinjaGrammarException(
+            raise exc.JinjaGrammarException("Syntax error '%s'." % str(e))
-                "Syntax error '%s'." % str(e)
-            )
 
     @classmethod
     def evaluate(cls, expression, data_context):
-        ctx = expression_utils.get_jinja_context(data_context)
+        ctx = get_jinja_context(data_context)
 
-        result = cls._env.compile_expression(
+        result = cls._env.compile_expression(expression, **JINJA_OPTS)(**ctx)
-            expression,
-            **JINJA_OPTS
-        )(**ctx)
 
         # For StrictUndefined values, UndefinedError only gets raised when
         # the value is accessed, not when it gets created. The simplest way
@@ -90,7 +105,7 @@ class JinjaEvaluator(Evaluator):
         return False
 
 
-class InlineJinjaEvaluator(Evaluator):
+class InlineJinjaEvaluator(base.Evaluator):
     # The regular expression for Jinja variables and blocks
     find_expression_pattern = re.compile(JINJA_REGEXP)
     find_block_pattern = re.compile(JINJA_BLOCK_REGEXP)
@@ -126,7 +141,8 @@ class InlineJinjaEvaluator(Evaluator):
             if patterns[0][0] == expression:
                 result = JinjaEvaluator.evaluate(patterns[0][1], data_context)
             else:
-                ctx = expression_utils.get_jinja_context(data_context)
+                ctx = get_jinja_context(data_context)
+
                 result = cls._env.from_string(expression).render(**ctx)
         except Exception as e:
             # NOTE(rakhmerov): if we hit a database error then we need to

mistral/expressions/std_functions.py

@@ -1,4 +1,5 @@
-# Copyright 2015 - Mirantis, Inc.
+# Copyright 2013 - Mirantis, Inc.
+# Copyright 2015 - StackStorm, Inc.
 # Copyright 2016 - Brocade Communications Systems, Inc.
 #
 #    Licensed under the Apache License, Version 2.0 (the "License");
@@ -13,163 +14,67 @@
 #    See the License for the specific language governing permissions and
 #    limitations under the License.
 
-from functools import partial
 import warnings
 
 from oslo_log import log as logging
 from oslo_serialization import jsonutils
-from stevedore import extension
 import yaml
-from yaml import representer
-import yaql
 
-from yaql.language import utils as yaql_utils
-
-from mistral.config import cfg
 from mistral.db.v2 import api as db_api
 from mistral.utils import filter_utils
 from mistral_lib import utils
 
-# TODO(rakhmerov): it's work around the bug in YAQL.
+# Additional YAQL/Jinja functions provided by Mistral out of the box.
-# YAQL shouldn't expose internal types to custom functions.
-representer.SafeRepresenter.add_representer(
-    yaql_utils.FrozenDict,
-    representer.SafeRepresenter.represent_dict
-)
-
-LOG = logging.getLogger(__name__)
-ROOT_YAQL_CONTEXT = None
-
-
-def get_yaql_context(data_context):
-    global ROOT_YAQL_CONTEXT
-
-    if not ROOT_YAQL_CONTEXT:
-        ROOT_YAQL_CONTEXT = yaql.create_context()
-
-        _register_yaql_functions(ROOT_YAQL_CONTEXT)
-
-    new_ctx = ROOT_YAQL_CONTEXT.create_child_context()
-
-    new_ctx['$'] = (
-        data_context if not cfg.CONF.yaql.convert_input_data
-        else yaql_utils.convert_input_data(data_context)
-    )
-
-    if isinstance(data_context, dict):
-        new_ctx['__env'] = data_context.get('__env')
-        new_ctx['__execution'] = data_context.get('__execution')
-        new_ctx['__task_execution'] = data_context.get('__task_execution')
-
-    return new_ctx
-
-
-def get_jinja_context(data_context):
-    new_ctx = {
-        '_': data_context
-    }
-
-    _register_jinja_functions(new_ctx)
-
-    if isinstance(data_context, dict):
-        new_ctx['__env'] = data_context.get('__env')
-        new_ctx['__execution'] = data_context.get('__execution')
-        new_ctx['__task_execution'] = data_context.get('__task_execution')
-
-    return new_ctx
-
-
-def get_custom_functions():
-    """Get custom functions
-
-    Retrieves the list of custom evaluation functions
-    """
-    functions = dict()
-
-    mgr = extension.ExtensionManager(
-        namespace='mistral.expression.functions',
-        invoke_on_load=False
-    )
-
-    for name in mgr.names():
-        functions[name] = mgr[name].plugin
-
-    return functions
-
-
-def _register_yaql_functions(yaql_ctx):
-    functions = get_custom_functions()
-
-    for name in functions:
-        yaql_ctx.register_function(functions[name], name=name)
-
-
-def _register_jinja_functions(jinja_ctx):
-    functions = get_custom_functions()
-
-    for name in functions:
-        jinja_ctx[name] = partial(functions[name], jinja_ctx['_'])
-
-
-# Additional YAQL functions needed by Mistral.
 # If a function name ends with underscore then it doesn't need to pass
 # the name of the function when context registers it.
 
+LOG = logging.getLogger(__name__)
+
+
 def env_(context):
     return context['__env']
 
 
-def executions_(context,
+def executions_(context, id=None, root_execution_id=None, state=None,
-                id=None,
+                from_time=None, to_time=None):
-                root_execution_id=None,
+    fltr = {}
-                state=None,
-                from_time=None,
-                to_time=None
-                ):
-
-    filter = {}
 
     if id is not None:
-        filter = filter_utils.create_or_update_filter(
+        fltr = filter_utils.create_or_update_filter('id', id, "eq", fltr)
-            'id',
-            id,
-            "eq",
-            filter
-        )
 
     if root_execution_id is not None:
-        filter = filter_utils.create_or_update_filter(
+        fltr = filter_utils.create_or_update_filter(
             'root_execution_id',
             root_execution_id,
-            "eq",
+            'eq',
-            filter
+            fltr
         )
 
     if state is not None:
-        filter = filter_utils.create_or_update_filter(
+        fltr = filter_utils.create_or_update_filter(
             'state',
             state,
-            "eq",
+            'eq',
-            filter
+            fltr
         )
 
     if from_time is not None:
-        filter = filter_utils.create_or_update_filter(
+        fltr = filter_utils.create_or_update_filter(
             'created_at',
             from_time,
-            "gte",
+            'gte',
-            filter
+            fltr
         )
 
     if to_time is not None:
-        filter = filter_utils.create_or_update_filter(
+        fltr = filter_utils.create_or_update_filter(
             'created_at',
             to_time,
-            "lt",
+            'lt',
-            filter
+            fltr
         )
 
-    return db_api.get_workflow_executions(**filter)
+    return db_api.get_workflow_executions(**fltr)
 
 
 def execution_(context):

mistral/expressions/yaql_expression.py

@@ -20,16 +20,18 @@ import re
 from oslo_db import exception as db_exc
 from oslo_log import log as logging
 import six
+from yaml import representer
+import yaql
 from yaql.language import exceptions as yaql_exc
 from yaql.language import factory
 from yaql.language import utils as yaql_utils
 
 from mistral.config import cfg
 from mistral import exceptions as exc
-from mistral.expressions.base_expression import Evaluator
+from mistral.expressions import base
-from mistral.utils import expression_utils
 from mistral_lib import utils
 
+
 LOG = logging.getLogger(__name__)
 
 _YAQL_CONF = cfg.CONF.yaql
@@ -38,6 +40,45 @@ INLINE_YAQL_REGEXP = '<%.*?%>'
 
 YAQL_ENGINE = None
 
+ROOT_YAQL_CONTEXT = None
+
+# TODO(rakhmerov): it's work around the bug in YAQL.
+# YAQL shouldn't expose internal types to custom functions.
+representer.SafeRepresenter.add_representer(
+    yaql_utils.FrozenDict,
+    representer.SafeRepresenter.represent_dict
+)
+
+
+def get_yaql_context(data_context):
+    global ROOT_YAQL_CONTEXT
+
+    if not ROOT_YAQL_CONTEXT:
+        ROOT_YAQL_CONTEXT = yaql.create_context()
+
+        _register_yaql_functions(ROOT_YAQL_CONTEXT)
+
+    new_ctx = ROOT_YAQL_CONTEXT.create_child_context()
+
+    new_ctx['$'] = (
+        data_context if not cfg.CONF.yaql.convert_input_data
+        else yaql_utils.convert_input_data(data_context)
+    )
+
+    if isinstance(data_context, dict):
+        new_ctx['__env'] = data_context.get('__env')
+        new_ctx['__execution'] = data_context.get('__execution')
+        new_ctx['__task_execution'] = data_context.get('__task_execution')
+
+    return new_ctx
+
+
+def _register_yaql_functions(yaql_ctx):
+    functions = base.get_custom_functions()
+
+    for name in functions:
+        yaql_ctx.register_function(functions[name], name=name)
+
 
 def get_yaql_engine_options():
     return {
@@ -97,7 +138,7 @@ def _sanitize_yaql_result(result):
     return result if not inspect.isgenerator(result) else list(result)
 
 
-class YAQLEvaluator(Evaluator):
+class YAQLEvaluator(base.Evaluator):
     @classmethod
     def validate(cls, expression):
         try:
@@ -111,7 +152,7 @@ class YAQLEvaluator(Evaluator):
 
         try:
             result = get_yaql_engine_class()(expression).evaluate(
-                context=expression_utils.get_yaql_context(data_context)
+                context=get_yaql_context(data_context)
             )
         except Exception as e:
             # NOTE(rakhmerov): if we hit a database error then we need to

mistral/tests/unit/engine/test_error_handling.py

@@ -18,10 +18,11 @@ from oslo_db import exception as db_exc
 
 from mistral.db.v2 import api as db_api
 from mistral import exceptions as exc
+from mistral.expressions import jinja_expression
+from mistral.expressions import yaql_expression
 from mistral.services import workbooks as wb_service
 from mistral.services import workflows as wf_service
 from mistral.tests.unit.engine import base
-from mistral.utils import expression_utils
 from mistral.workflow import states
 from mistral_lib import actions as actions_base
 
@@ -743,11 +744,11 @@ class ErrorHandlingEngineTest(base.EngineTestCase):
         self.assertIn("UnicodeDecodeError: utf", task_ex.state_info)
 
     @mock.patch(
-        'mistral.utils.expression_utils.get_yaql_context',
+        'mistral.expressions.yaql_expression.get_yaql_context',
         mock.MagicMock(
             side_effect=[
                 db_exc.DBDeadlock(),  # Emulating DB deadlock
-                expression_utils.get_yaql_context({})  # Successful run
+                yaql_expression.get_yaql_context({})  # Successful run
             ]
         )
     )
@@ -783,11 +784,11 @@ class ErrorHandlingEngineTest(base.EngineTestCase):
             self.assertDictEqual({'my_var': 2}, task_ex.published)
 
     @mock.patch(
-        'mistral.utils.expression_utils.get_jinja_context',
+        'mistral.expressions.jinja_expression.get_jinja_context',
         mock.MagicMock(
             side_effect=[
                 db_exc.DBDeadlock(),  # Emulating DB deadlock
-                expression_utils.get_jinja_context({})  # Successful run
+                jinja_expression.get_jinja_context({})  # Successful run
             ]
         )
     )

mistral/tests/unit/engine/test_yaql_functions.py

@@ -15,11 +15,11 @@
 from oslo_config import cfg
 
 from mistral.db.v2 import api as db_api
+from mistral.expressions import std_functions
 from mistral.services import workflows as wf_service
 from mistral.tests.unit.engine import base as engine_test_base
 from mistral.workflow import states
 
-
 # Use the set_default method to set value otherwise in certain test cases
 # the change in value is not permanent.
 cfg.CONF.set_default('auth_enable', False, group='pecan')
@@ -459,3 +459,28 @@ class YAQLFunctionsEngineTest(engine_test_base.EngineTestCase):
         self.assertIsNotNone(json_str)
         self.assertIn('"key1": "foo"', json_str)
         self.assertIn('"key2": "bar"', json_str)
+
+    def test_yaml_dump(self):
+        data = [
+            {
+                "this": "is valid",
+            },
+            {
+                "so": "is this",
+                "and": "this too",
+                "might": "as well",
+            },
+            "validaswell"
+        ]
+
+        expected = (
+            "- this: is valid\n"
+            "- and: this too\n"
+            "  might: as well\n"
+            "  so: is this\n"
+            "- validaswell\n"
+        )
+
+        yaml_str = std_functions.yaml_dump_(None, data)
+
+        self.assertEqual(expected, yaml_str)

mistral/tests/unit/expressions/test_jinja_expression.py

@@ -71,7 +71,7 @@ WF_EXECS = [
 ]
 
 
-class JinjaEvaluatorTest(base.BaseTest):
+class JinjaEvaluatorTest(base.DbTestCase):
     def setUp(self):
         super(JinjaEvaluatorTest, self).setUp()
 
@@ -79,6 +79,7 @@ class JinjaEvaluatorTest(base.BaseTest):
 
     def test_expression_result(self):
         res = self._evaluator.evaluate('_.server', DATA)
+
         self.assertEqual({
             'id': '03ea824a-aa24-4105-9131-66c48ae54acf',
             'name': 'cloud-fedora',
@@ -86,9 +87,11 @@ class JinjaEvaluatorTest(base.BaseTest):
         }, res)
 
         res = self._evaluator.evaluate('_.server.id', DATA)
+
         self.assertEqual('03ea824a-aa24-4105-9131-66c48ae54acf', res)
 
         res = self._evaluator.evaluate("_.server.status == 'ACTIVE'", DATA)
+
         self.assertTrue(res)
 
     def test_select_result(self):
@@ -96,7 +99,9 @@ class JinjaEvaluatorTest(base.BaseTest):
             '_.servers|selectattr("name", "equalto", "ubuntu")',
             SERVERS
         )
+
         item = list(res)[0]
+
         self.assertEqual({'name': 'ubuntu'}, item)
 
     def test_function_string(self):
@@ -104,8 +109,11 @@ class JinjaEvaluatorTest(base.BaseTest):
         self.assertEqual('3', self._evaluator.evaluate('_|string', 3))
 
     def test_function_len(self):
-        self.assertEqual(3,
+        self.assertEqual(
-                         self._evaluator.evaluate('_|length', 'hey'))
+            3,
+            self._evaluator.evaluate('_|length', 'hey')
+        )
+
         data = [{'some': 'thing'}]
 
         self.assertEqual(
@@ -184,10 +192,12 @@ class JinjaEvaluatorTest(base.BaseTest):
 
     def test_function_env(self):
         ctx = {'__env': 'some'}
+
         self.assertEqual(ctx['__env'], self._evaluator.evaluate('env()', ctx))
 
     def test_filter_env(self):
         ctx = {'__env': 'some'}
+
         self.assertEqual(ctx['__env'], self._evaluator.evaluate('_|env', ctx))
 
     @mock.patch('mistral.db.v2.api.get_task_executions')
@@ -196,6 +206,7 @@ class JinjaEvaluatorTest(base.BaseTest):
                                                 task_executions):
         task = mock.MagicMock(return_value={})
         task_executions.return_value = [task]
+
         ctx = {
             '__task_execution': None,
             '__execution': {

mistral/tests/unit/utils/test_expression_utils.py

@@ -1,44 +0,0 @@
-# Copyright 2014 - Mirantis, Inc.
-#
-#    Licensed under the Apache License, Version 2.0 (the "License");
-#    you may not use this file except in compliance with the License.
-#    You may obtain a copy of the License at
-#
-#        http://www.apache.org/licenses/LICENSE-2.0
-#
-#    Unless required by applicable law or agreed to in writing, software
-#    distributed under the License is distributed on an "AS IS" BASIS,
-#    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-#    See the License for the specific language governing permissions and
-#    limitations under the License.
-
-from mistral.tests.unit import base
-from mistral.utils import expression_utils as e_u
-
-
-JSON_INPUT = [
-    {
-        "this": "is valid",
-    },
-    {
-        "so": "is this",
-        "and": "this too",
-        "might": "as well",
-    },
-    "validaswell"
-]
-
-
-JSON_TO_YAML_STR = """- this: is valid
-- and: this too
-  might: as well
-  so: is this
-- validaswell
-"""
-
-
-class ExpressionUtilsTest(base.BaseTest):
-    def test_yaml_dump(self):
-        yaml_str = e_u.yaml_dump_(None, JSON_INPUT)
-
-        self.assertEqual(JSON_TO_YAML_STR, yaml_str)

setup.cfg

@@ -77,19 +77,19 @@ mistral.notification.publishers =
 
 mistral.expression.functions =
     # json_pp was deprecated in Queens and will be removed in the S cycle
-    json_pp = mistral.utils.expression_utils:json_pp_
+    json_pp = mistral.expressions.std_functions:json_pp_
 
-    env = mistral.utils.expression_utils:env_
+    env = mistral.expressions.std_functions:env_
-    execution = mistral.utils.expression_utils:execution_
+    execution = mistral.expressions.std_functions:execution_
-    executions = mistral.utils.expression_utils:executions_
+    executions = mistral.expressions.std_functions:executions_
-    global = mistral.utils.expression_utils:global_
+    global = mistral.expressions.std_functions:global_
-    json_parse = mistral.utils.expression_utils:json_parse_
+    json_parse = mistral.expressions.std_functions:json_parse_
-    json_dump = mistral.utils.expression_utils:json_dump_
+    json_dump = mistral.expressions.std_functions:json_dump_
-    task = mistral.utils.expression_utils:task_
+    task = mistral.expressions.std_functions:task_
-    tasks = mistral.utils.expression_utils:tasks_
+    tasks = mistral.expressions.std_functions:tasks_
-    uuid = mistral.utils.expression_utils:uuid_
+    uuid = mistral.expressions.std_functions:uuid_
-    yaml_parse = mistral.utils.expression_utils:yaml_parse_
+    yaml_parse = mistral.expressions.std_functions:yaml_parse_
-    yaml_dump = mistral.utils.expression_utils:yaml_dump_
+    yaml_dump = mistral.expressions.std_functions:yaml_dump_
 
 mistral.expression.evaluators =
     yaql = mistral.expressions.yaql_expression:InlineYAQLEvaluator