openstack
/
mistral
Code Issues Proposed changes

Fix ContextView JSON serialization 

* With disabled YAQL data output conversion, YAQL may return
  instances of ContextView which can't be properly saved into
  DB. This happens because Mistral serialization code doesn't
  turn on JSON conversion of custom objects, and they are just
  ignored by the "json" lib when it encounters them.
* Fixed how Mistral serializes context for Javascript evaluation
  to address the same problem.
* Implemented __repr__ method of ContextView.
* Removed logging of "data_context" from YAQL evaluation because
  previously it was always empty (because the string represetation
  of ContextView was always "{}") and now it may be very big, like
  megabytes, and the log gets populated too fast. It makes sense to
  log YAQL data context only when an error happened. In this case
  it helps to investigate an issue.
* Added all required unit tests.
* Fixed the tests for disabled YAQL conversion. In fact, they
  didn't test it properly because data conversion wasn't disabled.

Closes-Bug: #1867899
Change-Id: I12b4d0c5f1f49990d8ae09b72f73c0da96254a86
This commit is contained in:
Renat Akhmerov 2020-03-13 16:55:25 +07:00
parent dbfc0bea22
commit 019cffb3ab
7 changed files with 228 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
mistral/db/sqlalchemy/types.py
View File

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

20
mistral/expressions/yaql_expression.py
View File

@ -208,10 +208,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
@ -220,19 +218,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,
utils.cut(result, length=200)
)
return result
@classmethod

57
mistral/tests/unit/engine/test_dataflow.py
View File

@ -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
@ -1419,3 +1420,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)

58
mistral/tests/unit/engine/test_disabled_yaql_conversion.py
View File

@ -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'])

60
mistral/tests/unit/engine/test_javascript_action.py
View File

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

49
mistral/utils/javascript.py
View File

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

5
mistral/workflow/data_flow.py
View File

@ -124,6 +124,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 = {}