diff --git a/mistral/db/sqlalchemy/types.py b/mistral/db/sqlalchemy/types.py index 7949d8f0c..9617875bd 100644 --- a/mistral/db/sqlalchemy/types.py +++ b/mistral/db/sqlalchemy/types.py @@ -29,12 +29,20 @@ class JsonEncoded(sa.TypeDecorator): def process_bind_param(self, value, dialect): if value is not None: - value = jsonutils.dumps(value) + # We need to convert the root of the given object graph into + # a primitive by hand so that we also enable conversion of + # object of custom classes into primitives. Otherwise, they are + # ignored by the "json" lib. + value = jsonutils.dumps( + jsonutils.to_primitive(value, convert_instances=True) + ) + return value def process_result_value(self, value, dialect): if value is not None: value = jsonutils.loads(value) + return value diff --git a/mistral/expressions/yaql_expression.py b/mistral/expressions/yaql_expression.py index bb7ff4985..e48f223a7 100644 --- a/mistral/expressions/yaql_expression.py +++ b/mistral/expressions/yaql_expression.py @@ -167,10 +167,8 @@ class InlineYAQLEvaluator(YAQLEvaluator): @classmethod def evaluate(cls, expression, data_context): LOG.debug( - "Start to evaluate YAQL expression. " - "[expression='%s', context=%s]", - expression, - data_context + "Starting to evaluate YAQL expression. " + "[expression='%s']", expression ) result = expression @@ -179,19 +177,17 @@ class InlineYAQLEvaluator(YAQLEvaluator): if found_expressions: for expr in found_expressions: trim_expr = expr.strip("<%>") - evaluated = super(InlineYAQLEvaluator, - cls).evaluate(trim_expr, data_context) + + evaluated = super(InlineYAQLEvaluator, cls).evaluate( + trim_expr, + data_context + ) + if len(expression) == len(expr): result = evaluated else: result = result.replace(expr, str(evaluated)) - LOG.debug( - "Finished evaluation. [expression='%s', result: %s]", - expression, - result - ) - return result @classmethod diff --git a/mistral/tests/unit/engine/test_dataflow.py b/mistral/tests/unit/engine/test_dataflow.py index ece9fedeb..3d0d541c3 100644 --- a/mistral/tests/unit/engine/test_dataflow.py +++ b/mistral/tests/unit/engine/test_dataflow.py @@ -14,6 +14,7 @@ # limitations under the License. from oslo_config import cfg +from oslo_serialization import jsonutils from mistral.db.v2 import api as db_api from mistral.db.v2.sqlalchemy import models @@ -1157,3 +1158,59 @@ class DataFlowTest(test_base.BaseTest): self.assertEqual(2, len(res)) self.assertIn('v1', res) self.assertIn('v2', res) + + def test_context_view_repr(self): + ctx = data_flow.ContextView( + {'k1': 'v1'}, + {'k2': 'v2'}, + {3: 3} + ) + + str_repr = str(ctx) + + self.assertIsNotNone(str_repr) + self.assertFalse(str_repr == "{}") + self.assertEqual("{'k1': 'v1', 'k2': 'v2', 3: 3}", str_repr) + + ctx = data_flow.ContextView() + + self.assertEqual('{}', str(ctx)) + + def test_context_view_as_root_json(self): + ctx = data_flow.ContextView( + {'k1': 'v1'}, + {'k2': 'v2'}, + ) + + json_str = jsonutils.dumps( + jsonutils.to_primitive(ctx, convert_instances=True) + ) + + self.assertIsNotNone(json_str) + self.assertNotEqual('{}', json_str) + + # We can't use regular dict comparison because key order + # is not defined. + self.assertIn('"k1": "v1"', json_str) + self.assertIn('"k2": "v2"', json_str) + + def test_context_view_as_nested_json(self): + ctx = data_flow.ContextView( + {'k1': 'v1'}, + {'k2': 'v2'}, + ) + + d = {'root': ctx} + + json_str = jsonutils.dumps( + jsonutils.to_primitive(d, convert_instances=True) + ) + + self.assertIsNotNone(json_str) + self.assertNotEqual('{"root": {}}', json_str) + + # We can't use regular dict comparison because key order + # is not defined. + self.assertIn('"k1": "v1"', json_str) + self.assertIn('"k1": "v1"', json_str) + self.assertIn('"root"', json_str) diff --git a/mistral/tests/unit/engine/test_disabled_yaql_conversion.py b/mistral/tests/unit/engine/test_disabled_yaql_conversion.py index 56bd4e9c6..fe640b3a7 100644 --- a/mistral/tests/unit/engine/test_disabled_yaql_conversion.py +++ b/mistral/tests/unit/engine/test_disabled_yaql_conversion.py @@ -16,6 +16,7 @@ from mistral.db.v2 import api as db_api from mistral.engine import engine_server from mistral import exceptions as exc +from mistral.expressions import yaql_expression from mistral.services import workflows as wf_service from mistral.tests.unit.engine import base as engine_test_base @@ -51,6 +52,12 @@ class DisabledYAQLConversionTest(engine_test_base.EngineTestCase): self.override_config('convert_input_data', False, 'yaql') self.override_config('convert_output_data', False, 'yaql') + # At this point YAQL engine has already been initialized with the + # default value of config options. So we need to set the corresponding + # constant to None so it gets initialized again with the new values + # upon the first use. + yaql_expression.YAQL_ENGINE = None + wf_text = """--- version: '2.0' @@ -101,6 +108,10 @@ class DisabledYAQLConversionTest(engine_test_base.EngineTestCase): self.override_config('convert_input_data', True, 'yaql') self.override_config('convert_output_data', False, 'yaql') + # Setting YAQL engine to None so it reinitialized again with the + # right values upon the next use. + yaql_expression.YAQL_ENGINE = None + eng_svc = engine_server.get_oslo_service(setup_profiler=False) self.assertRaisesWithMessage( @@ -109,3 +120,50 @@ class DisabledYAQLConversionTest(engine_test_base.EngineTestCase): "so 'yaql.convert_input_data' must also be set to False.", eng_svc.start ) + + def test_root_context(self): + # Both input and output data conversion in YAQL need to be disabled + # so that we're sure that there won't be any surprises from YAQL + # like some YAQL internal types included in expression results. + self.override_config('convert_input_data', False, 'yaql') + self.override_config('convert_output_data', False, 'yaql') + + # Setting YAQL engine to None so it reinitialized again with the + # right values upon the next use. + yaql_expression.YAQL_ENGINE = None + + wf_text = """--- + version: '2.0' + + wf: + input: + - param: default_val + + tasks: + task1: + action: std.echo output=<% $ %> + publish: + result: <% task().result %> + """ + + wf_service.create_workflows(wf_text) + + # Start workflow. + wf_ex = self.engine.start_workflow('wf') + + self.await_workflow_success(wf_ex.id) + + with db_api.transaction(): + # Note: We need to reread execution to access related tasks. + wf_ex = db_api.get_workflow_execution(wf_ex.id) + + t_ex = self._assert_single_item( + wf_ex.task_executions, + name='task1' + ) + + action_ex = t_ex.action_executions[0] + + self.assertTrue(len(action_ex.input) > 0) + self.assertIn('output', action_ex.input) + self.assertIn('param', action_ex.input['output']) diff --git a/mistral/tests/unit/engine/test_javascript_action.py b/mistral/tests/unit/engine/test_javascript_action.py index 36fa267e2..d5501c5a4 100644 --- a/mistral/tests/unit/engine/test_javascript_action.py +++ b/mistral/tests/unit/engine/test_javascript_action.py @@ -18,12 +18,12 @@ from oslo_utils import importutils import testtools from mistral.db.v2 import api as db_api +from mistral.expressions import yaql_expression from mistral.services import workflows as wf_service from mistral.tests.unit.engine import base from mistral.utils import javascript 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') @@ -60,10 +60,8 @@ class JavaScriptEngineTest(base.EngineTestCase): 'This test requires that py_mini_racer library was ' 'installed') def test_py_mini_racer_javascript_action(self): - cfg.CONF.set_default( - 'js_implementation', - 'py_mini_racer' - ) + cfg.CONF.set_default('js_implementation', 'py_mini_racer') + length = 1000 wf_service.create_workflows(JAVASCRIPT_WORKFLOW) @@ -85,6 +83,58 @@ class JavaScriptEngineTest(base.EngineTestCase): self.assertEqual(length / 2, task_ex.published['res']) + @testtools.skipIf(not importutils.try_import('py_mini_racer'), + 'This test requires that py_mini_racer library was ' + 'installed') + def test_py_mini_racer_javascript_action_disabled_yaql_conversion(self): + cfg.CONF.set_default('js_implementation', 'py_mini_racer') + + # Both input and output data conversion in YAQL need to be disabled + # so that we're sure that there won't be any surprises from YAQL + # like some YAQL internal types included in expression results. + self.override_config('convert_input_data', False, 'yaql') + self.override_config('convert_output_data', False, 'yaql') + + # Setting YAQL engine to None so it reinitialized again with the + # right values upon the next use. + yaql_expression.YAQL_ENGINE = None + + wf_text = """--- + version: '2.0' + + wf: + input: + - param: default_val + + tasks: + task1: + action: std.js + input: + context: <% $ %> + script: > + return $.param + publish: + result: <% task().result %> + """ + + wf_service.create_workflows(wf_text) + + # Start workflow. + wf_ex = self.engine.start_workflow('wf') + + self.await_workflow_success(wf_ex.id) + + with db_api.transaction(): + # Note: We need to reread execution to access related tasks. + wf_ex = db_api.get_workflow_execution(wf_ex.id) + + t_ex = self._assert_single_item( + wf_ex.task_executions, + name='task1' + ) + + self.assertDictEqual({'result': 'default_val'}, t_ex.published) + @mock.patch.object(javascript, 'evaluate', fake_evaluate) def test_fake_javascript_action_data_context(self): length = 1000 diff --git a/mistral/utils/javascript.py b/mistral/utils/javascript.py index fb65ceb92..b08cc1d19 100644 --- a/mistral/utils/javascript.py +++ b/mistral/utils/javascript.py @@ -1,4 +1,5 @@ # Copyright 2015 - Mirantis, Inc. +# Copyright 2020 - Nokia Software. # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -13,11 +14,11 @@ # limitations under the License. import abc -import json from mistral import config as cfg from mistral import exceptions as exc +from oslo_serialization import jsonutils from oslo_utils import importutils from stevedore import driver from stevedore import extension @@ -46,44 +47,66 @@ class JSEvaluator(object): class PyV8Evaluator(JSEvaluator): @classmethod - def evaluate(cls, script, context): + def evaluate(cls, script, ctx): if not _PYV8: raise exc.MistralException( "PyV8 module is not available. Please install PyV8." ) - with _PYV8.JSContext() as ctx: + with _PYV8.JSContext() as js_ctx: # Prepare data context and way for interaction with it. - ctx.eval('$ = %s' % json.dumps(context)) + # NOTE: it's important to enable conversion of custom types + # into JSON to account for classes like ContextView. + ctx_str = jsonutils.dumps( + jsonutils.to_primitive(ctx, convert_instances=True) + ) + + js_ctx.eval('$ = %s' % ctx_str) + + result = js_ctx.eval(script) - result = ctx.eval(script) return _PYV8.convert(result) class V8EvalEvaluator(JSEvaluator): @classmethod - def evaluate(cls, script, context): + def evaluate(cls, script, ctx): if not _V8EVAL: raise exc.MistralException( "v8eval module is not available. Please install v8eval." ) v8 = _V8EVAL.V8() - return v8.eval(('$ = %s; %s' % (json.dumps(context), script)).encode( - encoding='UTF-8')) + + # NOTE: it's important to enable conversion of custom types + # into JSON to account for classes like ContextView. + ctx_str = jsonutils.dumps( + jsonutils.to_primitive(ctx, convert_instances=True) + ) + + return v8.eval( + ('$ = %s; %s' % (ctx_str, script)).encode(encoding='UTF-8') + ) class PyMiniRacerEvaluator(JSEvaluator): @classmethod - def evaluate(cls, script, context): + def evaluate(cls, script, ctx): if not _PY_MINI_RACER: raise exc.MistralException( "PyMiniRacer module is not available. Please install " "PyMiniRacer." ) - ctx = _PY_MINI_RACER.MiniRacer() - return ctx.eval(('$ = {}; {}'.format(json.dumps(context), script))) + js_ctx = _PY_MINI_RACER.MiniRacer() + + # NOTE: it's important to enable conversion of custom types + # into JSON to account for classes like ContextView. + ctx_str = jsonutils.dumps( + jsonutils.to_primitive(ctx, convert_instances=True) + ) + + return js_ctx.eval(('$ = {}; {}'.format(ctx_str, script))) _mgr = extension.ExtensionManager( @@ -107,5 +130,5 @@ def get_js_evaluator(): return _EVALUATOR -def evaluate(script, context): - return get_js_evaluator().evaluate(script, context) +def evaluate(script, ctx): + return get_js_evaluator().evaluate(script, ctx) diff --git a/mistral/workflow/data_flow.py b/mistral/workflow/data_flow.py index d0778932d..793cb6ed2 100644 --- a/mistral/workflow/data_flow.py +++ b/mistral/workflow/data_flow.py @@ -123,6 +123,11 @@ class ContextView(dict): def __delitem__(self, key): self._raise_immutable_error() + def __repr__(self): + return ''.join( + ['{', ', '.join([str(d)[1:-1] for d in self.dicts]), '}'] + ) + def evaluate_upstream_context(upstream_task_execs): published_vars = {}