Skip to content

Commit

Permalink
Fix how Mistral prepares data for evaluating a YAQL expression
Browse files Browse the repository at this point in the history
* If we use the built-in YAQL function 'str' in a workflow then it
  doesn't represent lists as '[item1, item3, ...]' but instead
  creates '(item1, item2,...). This is because the standard YAQL
  function 'yaql_utils.convert_input_data', which is needed to
  convert a initial user data into an internal YAQL format,
  converts all sequences (except strings) into tuples.
  This patch overrides this behavior for sequences that are not
  strings and tuples so that they now get converted into lists.
  YAQL uses tuples because it needs to obtain a safe immutable
  structure to make calculations upon. But in Mistral list is
  more suitable because lots of users care about string
  representations. Immutability is not so important because
  Mistral code base guarantees that the initial data context
  for an expression won't be changed while an expression is
  being evaluated by YAQL.
* "str" YAQL function used to work well but it was broken in
  https://review.openstack.org/#/c/477816/ that added additional
  context preparation in order to fix the issue
  https://bugs.launchpad.net/mistral/+bug/1772864

Change-Id: I69d32f8772418d586d6c414842bb54aada217481
Closes-Bug: #1815710
  • Loading branch information
rakhmerov committed Feb 14, 2019
1 parent 58d6634 commit a39db2d
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 1 deletion.
40 changes: 40 additions & 0 deletions mistral/tests/unit/engine/test_yaql_functions.py
Expand Up @@ -13,6 +13,7 @@
# limitations under the License.

from oslo_config import cfg
import six

from mistral.db.v2 import api as db_api
from mistral.services import workflows as wf_service
Expand Down Expand Up @@ -431,3 +432,42 @@ def test_yaml_dump_function(self):
self.assertIsNotNone(json_str)
self.assertIn('"key1": "foo"', json_str)
self.assertIn('"key2": "bar"', json_str)

def test_built_in_str_function(self):
wf_text = """---
version: '2.0'
wf:
input:
- my_list
tasks:
task1:
publish:
val: <% str($.my_list) %>
"""

wf_service.create_workflows(wf_text)

wf_ex = self.engine.start_workflow(
'wf',
wf_input={
'my_list': [
{
'k1': 'v1',
'k2': 'v2'
}
]
}
)

self.await_workflow_success(wf_ex.id)

with db_api.transaction(read_only=True):
wf_ex = db_api.get_workflow_execution(wf_ex.id)

val = wf_ex.task_executions[0].published['val']

self.assertIsInstance(val, six.string_types)
self.assertIn('[', val)
self.assertIn(']', val)
24 changes: 23 additions & 1 deletion mistral/utils/expression_utils.py
Expand Up @@ -14,6 +14,7 @@
# limitations under the License.

from functools import partial
import six
import warnings

from oslo_log import log as logging
Expand All @@ -39,6 +40,27 @@
ROOT_YAQL_CONTEXT = None


def _convert_yaql_input_data(obj, rec=None):
# NOTE(rakhmerov): We have to define our own wrapper function
# around the function 'convert_input_data' from 'yaql_utils'
# because the latter always converts all sequences (except strings)
# into tuples, and it in turn breaks a number of things. For example,
# if we use the built-in 'str' YAQL function with an argument of the
# type 'list' then the result will be '(item1, item2 ..., itemN,)'
# instead of '[item1, item2 ..., itemN]'.
# So we override this behavior for sequences that are not strings and
# tuples.
if rec is None:
rec = _convert_yaql_input_data

if (isinstance(obj, yaql_utils.SequenceType) and
not isinstance(obj, six.string_types) and
not isinstance(obj, tuple)):
return list(rec(t, rec) for t in obj)
else:
return yaql_utils.convert_input_data(obj, rec)


def get_yaql_context(data_context):
global ROOT_YAQL_CONTEXT

Expand All @@ -48,7 +70,7 @@ def get_yaql_context(data_context):
_register_yaql_functions(ROOT_YAQL_CONTEXT)

new_ctx = ROOT_YAQL_CONTEXT.create_child_context()
new_ctx['$'] = yaql_utils.convert_input_data(data_context)
new_ctx['$'] = _convert_yaql_input_data(data_context)

if isinstance(data_context, dict):
new_ctx['__env'] = data_context.get('__env')
Expand Down

0 comments on commit a39db2d

Please sign in to comment.