From 0dbab33c4c1aada5bcf96712d31ab7fdf3e4d1bb Mon Sep 17 00:00:00 2001 From: Renat Akhmerov Date: Mon, 23 Mar 2020 17:42:28 +0700 Subject: [PATCH] Fix serialization of structures that might contain YAQL types * When YAQL output data conversion is disabled there's still an issue caused by presence of not JSON-compatible types within a YAQL result. The internal Mistral code is already able to deal with that (due to the previous changes) by checking that and converting them to what's needed. However, JSON serialization may still not work if it's done via the standard "json" library. The library simply doesn't handle those non-standard types and raises an exception. We have a sanitizing function that all YAQL results go through, however, it doesn't make sense to do recursive sanitizing for performance reasons. It does make sense to convert data as late as possible to avoid redundant data manipulations. So the sanitizing function handles only the root object in the object graph. The solution for this problem is to use our own utility function based on the "oslo_serialization.jsonutils" that is able to deal with at least part of the mentioned types, specifically FrozenDict and iterators. Generators are still a problem and this new function takes care of that separately, assuming that any generator is just a special iterator and hence represents a collection, i.e. a list in JSON terms. It works for all the cases we've encountered so far working with YAQL. * Used the new function "utils.to_json_str()" everywhere for JSON serialization, including the action "std.http". * Added necessary unit tests. Closes-Bug: #1869168 Depends-On: I1081a44a6f305eb1dfe68a5bad30110385130725 Change-Id: I9e73ea7cbba215c3e1d174b5189be27c640c4d42 --- mistral/actions/std_actions.py | 6 +- mistral/db/sqlalchemy/types.py | 19 +--- mistral/expressions/yaql_expression.py | 5 +- mistral/tests/unit/engine/test_dataflow.py | 10 +-- .../unit/expressions/test_yaql_expression.py | 2 + .../test_yaql_json_serialization.py | 89 +++++++++++++++++++ mistral/utils/__init__.py | 51 +++++++++++ mistral/utils/javascript.py | 24 ++--- 8 files changed, 161 insertions(+), 45 deletions(-) create mode 100644 mistral/tests/unit/expressions/test_yaql_json_serialization.py diff --git a/mistral/actions/std_actions.py b/mistral/actions/std_actions.py index b07f4e7b8..dba5b8d94 100644 --- a/mistral/actions/std_actions.py +++ b/mistral/actions/std_actions.py @@ -16,7 +16,6 @@ from email import header from email.mime import multipart from email.mime import text -import json as json_lib import smtplib import time @@ -25,6 +24,7 @@ import requests import six from mistral import exceptions as exc +from mistral import utils from mistral.utils import javascript from mistral.utils import ssh_utils from mistral_lib import actions @@ -182,7 +182,7 @@ class HTTPAction(actions.Action): self.url = url self.method = method self.params = params - self.body = json_lib.dumps(body) if isinstance(body, dict) else body + self.body = utils.to_json_str(body) if isinstance(body, dict) else body self.json = json self.headers = headers self.cookies = cookies @@ -456,7 +456,7 @@ class SSHAction(actions.Action): return raise_exc(parent_exc=e) def test(self, context): - return json_lib.dumps(self.params) + return utils.to_json_str(self.params) class SSHProxiedAction(SSHAction): diff --git a/mistral/db/sqlalchemy/types.py b/mistral/db/sqlalchemy/types.py index 9617875bd..038fc0b4c 100644 --- a/mistral/db/sqlalchemy/types.py +++ b/mistral/db/sqlalchemy/types.py @@ -16,11 +16,12 @@ # expressed by json-strings # -from oslo_serialization import jsonutils import sqlalchemy as sa from sqlalchemy.dialects import mysql from sqlalchemy.ext import mutable +from mistral import utils + class JsonEncoded(sa.TypeDecorator): """Represents an immutable structure as a json-encoded string.""" @@ -28,22 +29,10 @@ class JsonEncoded(sa.TypeDecorator): impl = sa.Text def process_bind_param(self, value, dialect): - if value is not None: - # 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 + return utils.to_json_str(value) def process_result_value(self, value, dialect): - if value is not None: - value = jsonutils.loads(value) - - return value + return utils.from_json_str(value) class MutableList(mutable.Mutable, list): diff --git a/mistral/expressions/yaql_expression.py b/mistral/expressions/yaql_expression.py index 99801c6c1..1a467bfcc 100644 --- a/mistral/expressions/yaql_expression.py +++ b/mistral/expressions/yaql_expression.py @@ -135,7 +135,10 @@ def _sanitize_yaql_result(result): if isinstance(result, yaql_utils.FrozenDict): return result._d - return result if not inspect.isgenerator(result) else list(result) + if inspect.isgenerator(result): + return list(result) + + return result class YAQLEvaluator(base.Evaluator): diff --git a/mistral/tests/unit/engine/test_dataflow.py b/mistral/tests/unit/engine/test_dataflow.py index dfc68fb0d..e9feba762 100644 --- a/mistral/tests/unit/engine/test_dataflow.py +++ b/mistral/tests/unit/engine/test_dataflow.py @@ -14,7 +14,6 @@ # 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 @@ -24,6 +23,7 @@ from mistral.services import workbooks as wb_service from mistral.services import workflows as wf_service from mistral.tests.unit import base as test_base from mistral.tests.unit.engine import base as engine_test_base +from mistral import utils from mistral.workflow import data_flow from mistral.workflow import states @@ -1444,9 +1444,7 @@ class DataFlowTest(test_base.BaseTest): {'k2': 'v2'}, ) - json_str = jsonutils.dumps( - jsonutils.to_primitive(ctx, convert_instances=True) - ) + json_str = utils.to_json_str(ctx) self.assertIsNotNone(json_str) self.assertNotEqual('{}', json_str) @@ -1464,9 +1462,7 @@ class DataFlowTest(test_base.BaseTest): d = {'root': ctx} - json_str = jsonutils.dumps( - jsonutils.to_primitive(d, convert_instances=True) - ) + json_str = utils.to_json_str(d) self.assertIsNotNone(json_str) self.assertNotEqual('{"root": {}}', json_str) diff --git a/mistral/tests/unit/expressions/test_yaql_expression.py b/mistral/tests/unit/expressions/test_yaql_expression.py index 3506dcfb9..4b8739912 100644 --- a/mistral/tests/unit/expressions/test_yaql_expression.py +++ b/mistral/tests/unit/expressions/test_yaql_expression.py @@ -27,6 +27,7 @@ from mistral.expressions import yaql_expression as expr from mistral.tests.unit import base from mistral_lib import utils + CONF = cfg.CONF DATA = { @@ -96,6 +97,7 @@ class YaqlEvaluatorTest(base.BaseTest): '$.servers.where($.name = ubuntu)', SERVERS ) + item = list(res)[0] self.assertEqual({'name': 'ubuntu'}, item) diff --git a/mistral/tests/unit/expressions/test_yaql_json_serialization.py b/mistral/tests/unit/expressions/test_yaql_json_serialization.py new file mode 100644 index 000000000..41317b048 --- /dev/null +++ b/mistral/tests/unit/expressions/test_yaql_json_serialization.py @@ -0,0 +1,89 @@ +# Copyright 2013 - Mirantis, Inc. +# Copyright 2015 - StackStorm, Inc. +# Copyright 2016 - Brocade Communications Systems, 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 yaql.language import utils as yaql_utils + +from mistral.tests.unit import base +from mistral import utils + + +class YaqlJsonSerializationTest(base.BaseTest): + def test_serialize_frozen_dict(self): + data = yaql_utils.FrozenDict(a=1, b=2, c=iter([1, 2, 3])) + + json_str = utils.to_json_str(data) + + self.assertIsNotNone(json_str) + + self.assertIn('"a": 1', json_str) + self.assertIn('"b": 2', json_str) + self.assertIn('"c": [1, 2, 3]', json_str) + + def test_serialize_generator(self): + def _list_stream(_list): + for i in _list: + yield i + + gen = _list_stream( + [1, yaql_utils.FrozenDict(a=1), _list_stream([12, 15])] + ) + + self.assertEqual('[1, {"a": 1}, [12, 15]]', utils.to_json_str(gen)) + + def test_serialize_dict_of_generators(self): + def _f(cnt): + for i in range(1, cnt + 1): + yield i + + data = {'numbers': _f(3)} + + self.assertEqual('{"numbers": [1, 2, 3]}', utils.to_json_str(data)) + + def test_serialize_range(self): + self.assertEqual("[1, 2, 3, 4]", utils.to_json_str(range(1, 5))) + + def test_serialize_iterator_of_frozen_dicts(self): + data = iter( + [ + yaql_utils.FrozenDict(a=1, b=2, c=iter([1, 2, 3])), + yaql_utils.FrozenDict( + a=11, + b=yaql_utils.FrozenDict(b='222'), + c=iter( + [ + 1, + yaql_utils.FrozenDict( + a=iter([4, yaql_utils.FrozenDict(a=99)]) + ) + ] + ) + ) + ] + ) + + json_str = utils.to_json_str(data) + + self.assertIsNotNone(json_str) + + # Checking the first item. + self.assertIn('"a": 1', json_str) + self.assertIn('"b": 2', json_str) + self.assertIn('"c": [1, 2, 3]', json_str) + + # Checking the first item. + self.assertIn('"a": 11', json_str) + self.assertIn('"b": {"b": "222"}', json_str) + self.assertIn('"c": [1, {"a": [4, {"a": 99}]}]', json_str) diff --git a/mistral/utils/__init__.py b/mistral/utils/__init__.py index 38c2f17d2..9f0494951 100644 --- a/mistral/utils/__init__.py +++ b/mistral/utils/__init__.py @@ -15,12 +15,14 @@ # limitations under the License. import contextlib +import inspect import os import shutil import tempfile import threading from oslo_concurrency import processutils +from oslo_serialization import jsonutils from mistral import exceptions as exc @@ -101,3 +103,52 @@ def generate_key_pair(key_length=2048): public_key = open(public_key_path).read() return private_key, public_key + + +def to_json_str(obj): + """Serializes an object into a JSON string. + + :param obj: Object to serialize. + :return: JSON string. + """ + + if obj is None: + return None + + def _fallback(value): + if inspect.isgenerator(value): + result = list(value) + + # The result of the generator call may be again not primitive + # so we need to call "to_primitive" again with the same fallback + # function. Note that the endless recursion here is not a problem + # because "to_primitive" limits the depth for custom classes, + # if they are present in the object graph being traversed. + return jsonutils.to_primitive( + result, + convert_instances=True, + fallback=_fallback + ) + + return 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. + return jsonutils.dumps( + jsonutils.to_primitive(obj, convert_instances=True, fallback=_fallback) + ) + + +def from_json_str(json_str): + """Reconstructs an object from a JSON string. + + :param json_str: A JSON string. + :return: Deserialized object. + """ + + if json_str is None: + return None + + return jsonutils.loads(json_str) diff --git a/mistral/utils/javascript.py b/mistral/utils/javascript.py index f16ad348e..2b6895b66 100644 --- a/mistral/utils/javascript.py +++ b/mistral/utils/javascript.py @@ -17,8 +17,8 @@ import abc from mistral import config as cfg from mistral import exceptions as exc +from mistral import utils -from oslo_serialization import jsonutils from oslo_utils import importutils from stevedore import driver from stevedore import extension @@ -55,13 +55,7 @@ class PyV8Evaluator(JSEvaluator): with _PYV8.JSContext() as js_ctx: # Prepare data context and way for interaction with it. - # 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) + js_ctx.eval('$ = %s' % utils.to_json_str(ctx)) result = js_ctx.eval(script) @@ -78,11 +72,7 @@ class V8EvalEvaluator(JSEvaluator): v8 = _V8EVAL.V8() - # 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) - ) + ctx_str = utils.to_json_str(ctx) return v8.eval( ('$ = %s; %s' % (ctx_str, script)).encode(encoding='UTF-8') @@ -100,14 +90,10 @@ class PyMiniRacerEvaluator(JSEvaluator): 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(utils.to_json_str(ctx), script) ) - return js_ctx.eval(('$ = {}; {}'.format(ctx_str, script))) - _mgr = extension.ExtensionManager( namespace='mistral.expression.evaluators',