diff --git a/CHANGELOG.rst b/CHANGELOG.rst index d2214004..b4f57342 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -39,6 +39,9 @@ Fixed * Check syntax on with items task to ensure action is indented correctly. Fixes #184 (bug fix) * Fix variable inspection where ctx().get() method calls are identified as errors. Fixes StackStorm/st2#4866 (bug fix) +* Fix a problem of TypeError orccuring when a list (or dict) value that contains unhashable typed + value (list or dict) is passed in some YAQL operators (e.g. distinct()). Fixes #176 (bug fix) + Contributed by Hiroyasu Ohyama (@userlocalhost) 1.0.0 ----- diff --git a/orquesta/expressions/functions/workflow.py b/orquesta/expressions/functions/workflow.py index 2ed8c8f0..502ebd5c 100644 --- a/orquesta/expressions/functions/workflow.py +++ b/orquesta/expressions/functions/workflow.py @@ -12,6 +12,8 @@ # See the License for the specific language governing permissions and # limitations under the License. +import collections + from orquesta import constants from orquesta import exceptions as exc from orquesta import statuses @@ -115,8 +117,8 @@ def item_(context, key=None): if not key: return current_item - if not isinstance(current_item, dict): - raise exc.ExpressionEvaluationException('Item is not type of dict.') + if not isinstance(current_item, collections.Mapping): + raise exc.ExpressionEvaluationException('Item is not type of collections.Mapping.') if key not in current_item: raise exc.ExpressionEvaluationException('Item does not have key "%s".' % key) diff --git a/orquesta/expressions/yql.py b/orquesta/expressions/yql.py index a172d74c..b42ed6ec 100644 --- a/orquesta/expressions/yql.py +++ b/orquesta/expressions/yql.py @@ -20,6 +20,7 @@ import yaql import yaql.language.exceptions as yaql_exc +import yaql.language.utils as yaql_utils from orquesta import exceptions as exc from orquesta.expressions import base as expr_base @@ -79,7 +80,15 @@ class YAQLEvaluator(expr_base.Evaluator): @classmethod def contextualize(cls, data): ctx = cls._root_ctx.create_child_context() - ctx['__vars'] = data or {} + + # Some yaql expressions (e.g. distinct()) refer to hash value of variable. + # But some built-in Python type values (e.g. list and dict) don't have __hash__() method. + # The convert_input_data method parses specified variable and convert it to hashable one. + if isinstance(data, yaql_utils.SequenceType) or isinstance(data, yaql_utils.MappingType): + ctx['__vars'] = yaql_utils.convert_input_data(data) + else: + ctx['__vars'] = data or {} + ctx['__state'] = ctx['__vars'].get('__state') ctx['__current_task'] = ctx['__vars'].get('__current_task') ctx['__current_item'] = ctx['__vars'].get('__current_item') diff --git a/orquesta/tests/unit/conducting/native/test_task_rendering_for_with_items.py b/orquesta/tests/unit/conducting/native/test_task_rendering_for_with_items.py index 44ae14ad..2a6e6c2f 100644 --- a/orquesta/tests/unit/conducting/native/test_task_rendering_for_with_items.py +++ b/orquesta/tests/unit/conducting/native/test_task_rendering_for_with_items.py @@ -82,7 +82,7 @@ def test_bad_item_type(self): 'type': 'error', 'message': ( 'YaqlEvaluationException: Unable to evaluate expression \'<% item(x) %>\'. ' - 'ExpressionEvaluationException: Item is not type of dict.' + 'ExpressionEvaluationException: Item is not type of collections.Mapping.' ), 'task_id': 'task1', 'route': 0 diff --git a/orquesta/tests/unit/conducting/test_workflow_conductor_data_flow.py b/orquesta/tests/unit/conducting/test_workflow_conductor_data_flow.py index b52d9faa..318038ba 100644 --- a/orquesta/tests/unit/conducting/test_workflow_conductor_data_flow.py +++ b/orquesta/tests/unit/conducting/test_workflow_conductor_data_flow.py @@ -20,47 +20,84 @@ from orquesta.specs import native as native_specs from orquesta import statuses from orquesta.tests.unit import base as test_base +import yaql.language.utils as yaql_utils class WorkflowConductorDataFlowTest(test_base.WorkflowConductorTest): - def _prep_conductor(self, context=None, inputs=None, status=None): - wf_def = """ - version: 1.0 - - description: A basic sequential workflow. - - input: - - a1 - - b1: <% ctx().a1 %> - - vars: - - a2: <% ctx().b1 %> - - b2: <% ctx().a2 %> - - output: - - a5: <% ctx().b4 %> - - b5: <% ctx().a5 %> - - tasks: - task1: - action: core.noop - next: - - when: <% succeeded() %> - publish: - - a3: <% ctx().b2 %> - - b3: <% ctx().a3 %> - do: task2 - task2: - action: core.noop - next: - - when: <% succeeded() %> - publish: a4=<% ctx().b3 %> b4=<% ctx().a4 %> - do: task3 - task3: - action: core.noop - """ - + wf_def_yaql = """ + version: 1.0 + + description: A basic sequential workflow. + + input: + - a1 + - b1: <% ctx().a1 %> + + vars: + - a2: <% ctx().b1 %> + - b2: <% ctx().a2 %> + + output: + - a5: <% ctx().b4 %> + - b5: <% ctx().a5 %> + + tasks: + task1: + action: core.noop + next: + - when: <% succeeded() %> + publish: + - a3: <% ctx().b2 %> + - b3: <% ctx().a3 %> + do: task2 + task2: + action: core.noop + next: + - when: <% succeeded() %> + publish: a4=<% ctx().b3 %> b4=<% ctx().a4 %> + do: task3 + task3: + action: core.noop + """ + + wf_def_jinja = """ + version: 1.0 + + description: A basic sequential workflow. + + input: + - a1 + - b1: '{{ ctx("a1") }}' + + vars: + - a2: '{{ ctx("b1") }}' + - b2: '{{ ctx("a2") }}' + + output: + - a5: '{{ ctx("b4") }}' + - b5: '{{ ctx("a5") }}' + + tasks: + task1: + action: core.noop + next: + - when: '{{ succeeded() }}' + publish: + - a3: '{{ ctx("b2") }}' + - b3: '{{ ctx("a3") }}' + do: task2 + task2: + action: core.noop + next: + - when: '{{ succeeded() }}' + publish: a4='{{ ctx("b3") }}' b4='{{ ctx("a4") }}' + do: task3 + task3: + action: core.noop + """ + + def _prep_conductor(self, wf_def, context=None, inputs=None, status=None): spec = native_specs.WorkflowSpec(wf_def) self.assertDictEqual(spec.inspect(), {}) @@ -76,33 +113,52 @@ def _prep_conductor(self, context=None, inputs=None, status=None): return conductor + def _get_combined_value(self, callstack_depth=0): + # This returns dict typed value all Python built-in type values + # which orquesta spec could accept. + if callstack_depth < 2: + return { + 'null': None, + 'integer_positive': 123, + 'integer_negative': -123, + 'number_positive': 99.99, + 'number_negative': -99.99, + 'string': 'xyz', + 'boolean_true': True, + 'boolean_false': False, + 'array': list(self._get_combined_value(callstack_depth + 1).values()), + 'object': self._get_combined_value(callstack_depth + 1), + } + else: + return {} + + def _assert_data_flow(self, inputs, expected_output): + # This assert method checks input value would be handled and published + # as an expected type and value with both YAQL and Jinja expressions. + for wf_def in [self.wf_def_jinja, self.wf_def_yaql]: + conductor = self._prep_conductor(wf_def, inputs=inputs, status=statuses.RUNNING) + + for i in range(1, len(conductor.spec.tasks) + 1): + task_name = 'task' + str(i) + forward_statuses = [statuses.RUNNING, statuses.SUCCEEDED] + self.forward_task_statuses(conductor, task_name, forward_statuses) + + # Render workflow output and checkout workflow status and output. + conductor.render_workflow_output() + self.assertEqual(conductor.get_workflow_status(), statuses.SUCCEEDED) + self.assertDictEqual(conductor.get_workflow_output(), expected_output) + def assert_data_flow(self, input_value): inputs = {'a1': input_value} expected_output = {'a5': inputs['a1'], 'b5': inputs['a1']} - conductor = self._prep_conductor(inputs=inputs, status=statuses.RUNNING) - for i in range(1, len(conductor.spec.tasks) + 1): - task_name = 'task' + str(i) - self.forward_task_statuses(conductor, task_name, [statuses.RUNNING, statuses.SUCCEEDED]) - - # Render workflow output and checkout workflow status and output. - conductor.render_workflow_output() - self.assertEqual(conductor.get_workflow_status(), statuses.SUCCEEDED) - self.assertDictEqual(conductor.get_workflow_output(), expected_output) + self._assert_data_flow(inputs, expected_output) def assert_unicode_data_flow(self, input_value): inputs = {u'a1': unicode(input_value, 'utf8') if six.PY2 else input_value} expected_output = {u'a5': inputs['a1'], u'b5': inputs['a1']} - conductor = self._prep_conductor(inputs=inputs, status=statuses.RUNNING) - - for i in range(1, len(conductor.spec.tasks) + 1): - task_name = 'task' + str(i) - self.forward_task_statuses(conductor, task_name, [statuses.RUNNING, statuses.SUCCEEDED]) - # Render workflow output and checkout workflow status and output. - conductor.render_workflow_output() - self.assertEqual(conductor.get_workflow_status(), statuses.SUCCEEDED) - self.assertDictEqual(conductor.get_workflow_output(), expected_output) + self._assert_data_flow(inputs, expected_output) def test_data_flow_string(self): self.assert_data_flow('xyz') @@ -119,11 +175,20 @@ def test_data_flow_boolean(self): self.assert_data_flow(True) self.assert_data_flow(False) + def test_data_flow_none(self): + self.assert_data_flow(None) + def test_data_flow_dict(self): - self.assert_data_flow({'x': 123, 'y': 'abc'}) + mapping_typed_data = self._get_combined_value() + + self.assertIsInstance(mapping_typed_data, yaql_utils.MappingType) + self.assert_data_flow(mapping_typed_data) def test_data_flow_list(self): - self.assert_data_flow([123, 'abc', True]) + sequence_typed_data = list(self._get_combined_value().values()) + + self.assertIsInstance(sequence_typed_data, yaql_utils.SequenceType) + self.assert_data_flow(sequence_typed_data) def test_data_flow_unicode(self): self.assert_unicode_data_flow('光合作用') diff --git a/orquesta/tests/unit/expressions/test_facade_yaql_evaluate.py b/orquesta/tests/unit/expressions/test_facade_yaql_evaluate.py index 9d519e72..661d2d57 100644 --- a/orquesta/tests/unit/expressions/test_facade_yaql_evaluate.py +++ b/orquesta/tests/unit/expressions/test_facade_yaql_evaluate.py @@ -269,3 +269,24 @@ def test_custom_function_failure(self): expr_base.evaluate, expr ) + + def test_distinct_operator(self): + test_cases = [ + { + 'expr': '<% ctx(val).distinct() %>', + 'input': {'val': [1, 2, 3, 1]}, + 'expect': [1, 2, 3] + }, + { + 'expr': '<% ctx(val).distinct() %>', + 'input': {'val': [{'a': 1}, {'b': 2}, {'a': 1}]}, + 'expect': [{'a': 1}, {'b': 2}] + }, + { + 'expr': '<% ctx(val).distinct($[1]) %>', + 'input': {'val': [['a', 1], ['b', 2], ['c', 1], ['a', 3]]}, + 'expect': [['a', 1], ['b', 2], ['a', 3]] + } + ] + for case in test_cases: + self.assertEqual(case['expect'], expr_base.evaluate(case['expr'], case['input']))