From 160948e3c6f4dbc3ec9a53ff62155ffcee5ff0a6 Mon Sep 17 00:00:00 2001 From: Vitalii Solodilov Date: Sun, 20 May 2018 14:34:44 +0400 Subject: [PATCH] Use on-clause and retry_policy get_spec for validation This patch places restrictions on the content of the on-success, on-complete, on-error, retry, concurrency, timeout, wait-before, wait-after and pause-before statements. test_direct_transition was refactored to exclude repeated test cases for on clause keys. Co-Authored-By: Vitalii Solodilov Closes-Bug: #1714341 Change-Id: I8b314c8759a46111a81cf4a9400aa1cab2ea5201 Signed-off-by: Vitalii Solodilov --- mistral/lang/v2/on_clause.py | 83 +++++++++++++++++---- mistral/lang/v2/policies.py | 2 +- mistral/lang/v2/task_defaults.py | 19 ++--- mistral/lang/v2/tasks.py | 19 ++--- mistral/tests/unit/lang/v2/test_tasks.py | 95 ++++++++++-------------- 5 files changed, 131 insertions(+), 87 deletions(-) diff --git a/mistral/lang/v2/on_clause.py b/mistral/lang/v2/on_clause.py index 7a6ebd664..cb65312af 100644 --- a/mistral/lang/v2/on_clause.py +++ b/mistral/lang/v2/on_clause.py @@ -19,26 +19,81 @@ from mistral.lang import types from mistral.lang.v2 import base from mistral.lang.v2 import publish +NEXT_TASK = { + "oneOf": [ + { + "type": "string", + "pattern": "^\w+$", + "description": "Task name (e.g.: `task1`)" + }, + { + "type": "string", + "pattern": "^\w+ \w+=(.*)$", + "description": "Task name with dict parameter " + "(e.g.: `fail msg=\"test\"`, " + "`fail msg=<% task() %>`)" + }, + { + "type": "string", + "pattern": "^\w+\\(\w+=(.*)\\)$", + "description": "Task name with func parameter " + "(e.g.: `fail(msg=\"test\")`, " + "`fail(msg=<% task() %>)`)" + } + ] +} + +TASK_WITH_EXPRESSION = { + "type": "object", + "minProperties": 1, + "maxProperties": 1, + "patternProperties": { + x['pattern']: types.EXPRESSION for x in NEXT_TASK['oneOf'] + }, + "description": "All next task variants plus expression (e.g.: " + "`task1: <% $.vm_id != null %>`, " + "`fail(msg=\"test\"): <% $.vm_id != null %>`)." +} + +LIST_OF_TASKS = { + "type": "array", + "items": { + "oneOf": [ + NEXT_TASK, + TASK_WITH_EXPRESSION + ] + }, + "uniqueItems": True, + "minItems": 1 +} + +ADVANCED_PUBLISHING_DICT = { + "type": "object", + "minProperties": 1, + "properties": { + "publish": publish.PublishSpec.get_schema(), + "next": { + "oneOf": [ + NEXT_TASK, + TASK_WITH_EXPRESSION, + LIST_OF_TASKS + ] + } + }, + "additionalProperties": False +} + class OnClauseSpec(base.BaseSpec): - _simple_schema = { + _schema = { "oneOf": [ - types.NONEMPTY_STRING, - types.UNIQUE_STRING_OR_EXPRESSION_CONDITION_LIST + NEXT_TASK, + TASK_WITH_EXPRESSION, + LIST_OF_TASKS, + ADVANCED_PUBLISHING_DICT ] } - _advanced_schema = { - "type": "object", - "properties": { - "publish": types.NONEMPTY_DICT, - "next": _simple_schema, - }, - "additionalProperties": False - } - - _schema = {"oneOf": [_simple_schema, _advanced_schema]} - def __init__(self, data): super(OnClauseSpec, self).__init__(data) diff --git a/mistral/lang/v2/policies.py b/mistral/lang/v2/policies.py index 77586bbaf..775b07e98 100644 --- a/mistral/lang/v2/policies.py +++ b/mistral/lang/v2/policies.py @@ -23,7 +23,7 @@ class PoliciesSpec(base.BaseSpec): _schema = { "type": "object", "properties": { - "retry": types.ANY, + "retry": retry_policy.RetrySpec.get_schema(), "wait-before": types.EXPRESSION_OR_POSITIVE_INTEGER, "wait-after": types.EXPRESSION_OR_POSITIVE_INTEGER, "timeout": types.EXPRESSION_OR_POSITIVE_INTEGER, diff --git a/mistral/lang/v2/task_defaults.py b/mistral/lang/v2/task_defaults.py index acb591c39..acb1b4803 100644 --- a/mistral/lang/v2/task_defaults.py +++ b/mistral/lang/v2/task_defaults.py @@ -19,6 +19,7 @@ from mistral.lang import types from mistral.lang.v2 import base from mistral.lang.v2 import on_clause from mistral.lang.v2 import policies +from mistral.lang.v2 import retry_policy # TODO(rakhmerov): This specification should be broken into two separate @@ -31,15 +32,15 @@ class TaskDefaultsSpec(base.BaseSpec): _schema = { "type": "object", "properties": { - "retry": types.ANY, - "wait-before": types.ANY, - "wait-after": types.ANY, - "timeout": types.ANY, - "pause-before": types.ANY, - "concurrency": types.ANY, - "on-complete": types.ANY, - "on-success": types.ANY, - "on-error": types.ANY, + "retry": retry_policy.RetrySpec.get_schema(), + "wait-before": types.EXPRESSION_OR_POSITIVE_INTEGER, + "wait-after": types.EXPRESSION_OR_POSITIVE_INTEGER, + "timeout": types.EXPRESSION_OR_POSITIVE_INTEGER, + "pause-before": types.EXPRESSION_OR_BOOLEAN, + "concurrency": types.EXPRESSION_OR_POSITIVE_INTEGER, + "on-complete": on_clause.OnClauseSpec.get_schema(), + "on-success": on_clause.OnClauseSpec.get_schema(), + "on-error": on_clause.OnClauseSpec.get_schema(), "safe-rerun": types.EXPRESSION_OR_BOOLEAN, "requires": { "oneOf": [types.NONEMPTY_STRING, types.UNIQUE_STRING_LIST] diff --git a/mistral/lang/v2/tasks.py b/mistral/lang/v2/tasks.py index 13c7f6ba8..96ee80403 100644 --- a/mistral/lang/v2/tasks.py +++ b/mistral/lang/v2/tasks.py @@ -25,6 +25,7 @@ from mistral.lang.v2 import base from mistral.lang.v2 import on_clause from mistral.lang.v2 import policies from mistral.lang.v2 import publish +from mistral.lang.v2 import retry_policy from mistral import utils from mistral.workflow import states @@ -73,12 +74,12 @@ class TaskSpec(base.BaseSpec): }, "publish": types.NONEMPTY_DICT, "publish-on-error": types.NONEMPTY_DICT, - "retry": types.ANY, - "wait-before": types.ANY, - "wait-after": types.ANY, - "timeout": types.ANY, - "pause-before": types.ANY, - "concurrency": types.ANY, + "retry": retry_policy.RetrySpec.get_schema(), + "wait-before": types.EXPRESSION_OR_POSITIVE_INTEGER, + "wait-after": types.EXPRESSION_OR_POSITIVE_INTEGER, + "timeout": types.EXPRESSION_OR_POSITIVE_INTEGER, + "pause-before": types.EXPRESSION_OR_BOOLEAN, + "concurrency": types.EXPRESSION_OR_POSITIVE_INTEGER, "target": types.NONEMPTY_STRING, "keep-result": types.EXPRESSION_OR_BOOLEAN, "safe-rerun": types.EXPRESSION_OR_BOOLEAN @@ -279,9 +280,9 @@ class DirectWorkflowTaskSpec(TaskSpec): types.POSITIVE_INTEGER ] }, - "on-complete": types.ANY, - "on-success": types.ANY, - "on-error": types.ANY + "on-complete": on_clause.OnClauseSpec.get_schema(), + "on-success": on_clause.OnClauseSpec.get_schema(), + "on-error": on_clause.OnClauseSpec.get_schema() } } diff --git a/mistral/tests/unit/lang/v2/test_tasks.py b/mistral/tests/unit/lang/v2/test_tasks.py index 962b53c83..4054c702f 100644 --- a/mistral/tests/unit/lang/v2/test_tasks.py +++ b/mistral/tests/unit/lang/v2/test_tasks.py @@ -276,63 +276,41 @@ class TaskSpecValidation(v2_base.WorkflowSpecValidationTestCase): def test_direct_transition(self): tests = [ - ({'on-success': ['email']}, False), - ({'on-success': [{'email': '<% 1 %>'}]}, False), - ({'on-success': [{'email': '<% 1 %>'}, 'echo']}, False), - ({'on-success': [{'email': '<% $.v1 in $.v2 %>'}]}, False), - ({'on-success': [{'email': '<% * %>'}]}, True), - ({'on-success': [{'email': '{{ 1 }}'}]}, False), - ({'on-success': [{'email': '{{ 1 }}'}, 'echo']}, False), - ({'on-success': [{'email': '{{ _.v1 in _.v2 }}'}]}, False), - ({'on-success': [{'email': '{{ * }}'}]}, True), - ({'on-success': 'email'}, False), - ({'on-success': None}, True), - ({'on-success': ['']}, True), - ({'on-success': []}, True), - ({'on-success': ['email', 'email']}, True), - ({'on-success': ['email', 12345]}, True), - ({'on-error': ['email']}, False), - ({'on-error': [{'email': '<% 1 %>'}]}, False), - ({'on-error': [{'email': '<% 1 %>'}, 'echo']}, False), - ({'on-error': [{'email': '<% $.v1 in $.v2 %>'}]}, False), - ({'on-error': [{'email': '<% * %>'}]}, True), - ({'on-error': [{'email': '{{ 1 }}'}]}, False), - ({'on-error': [{'email': '{{ 1 }}'}, 'echo']}, False), - ({'on-error': [{'email': '{{ _.v1 in _.v2 }}'}]}, False), - ({'on-error': [{'email': '{{ * }}'}]}, True), - ({'on-error': 'email'}, False), - ({'on-error': None}, True), - ({'on-error': ['']}, True), - ({'on-error': []}, True), - ({'on-error': ['email', 'email']}, True), - ({'on-error': ['email', 12345]}, True), - ({'on-complete': ['email']}, False), - ({'on-complete': [{'email': '<% 1 %>'}]}, False), - ({'on-complete': [{'email': '<% 1 %>'}, 'echo']}, False), - ({'on-complete': [{'email': '<% $.v1 in $.v2 %>'}]}, False), - ({'on-complete': [{'email': '<% * %>'}]}, True), - ({'on-complete': [{'email': '{{ 1 }}'}]}, False), - ({'on-complete': [{'email': '{{ 1 }}'}, 'echo']}, False), - ({'on-complete': [{'email': '{{ _.v1 in _.v2 }}'}]}, False), - ({'on-complete': [{'email': '{{ * }}'}]}, True), - ({'on-complete': 'email'}, False), - ({'on-complete': None}, True), - ({'on-complete': ['']}, True), - ({'on-complete': []}, True), - ({'on-complete': ['email', 'email']}, True), - ({'on-complete': ['email', 12345]}, True) + (['email'], False), + (['email%'], True), + ([{'email': '<% 1 %>'}], False), + ([{'email': '<% 1 %>'}, {'email': '<% 1 %>'}], True), + ([{'email': '<% 1 %>', 'more_email': '<% 2 %>'}], True), + (['email'], False), + ([{'email': '<% 1 %>'}, 'echo'], False), + ([{'email': '<% $.v1 in $.v2 %>'}], False), + ([{'email': '<% * %>'}], True), + ([{'email': '{{ 1 }}'}], False), + ([{'email': '{{ 1 }}'}, 'echo'], False), + ([{'email': '{{ _.v1 in _.v2 }}'}], False), + ([{'email': '{{ * }}'}], True), + ('email', False), + ('fail msg="<% task().result %>"', False), + ('fail(msg=<% task() %>)', False), + (None, True), + ([''], True), + ([], True), + (['email', 'email'], True), + (['email', 12345], True) ] - for transition, expect_error in tests: - overlay = {'test': {'tasks': {}}} + for on_clause_key in ['on-error', 'on-success', 'on-complete']: + for on_clause_value, expect_error in tests: + overlay = {'test': {'tasks': {}}} - utils.merge_dicts(overlay['test']['tasks'], {'get': transition}) + utils.merge_dicts(overlay['test']['tasks'], + {'get': {on_clause_key: on_clause_value}}) - self._parse_dsl_spec( - add_tasks=True, - changes=overlay, - expect_error=expect_error - ) + self._parse_dsl_spec( + add_tasks=True, + changes=overlay, + expect_error=expect_error + ) def test_direct_transition_advanced_schema(self): tests = [ @@ -431,6 +409,9 @@ class TaskSpecValidation(v2_base.WorkflowSpecValidationTestCase): ({'on-success': {'next': 'email'}}, False), ({'on-success': {'next': ['email']}}, False), ({'on-success': {'next': [{'email': 'email'}]}}, True), + ({'on-success': {'next': [{'email': 'email', + 'more_email': 'more_email'}]}}, True), + ({'on-success': {'next': {'email': 'email'}}}, True), ({'on-error': {'publish': {'var1': 1234}}}, True), ({'on-error': {'publish': {'branch': {'var1': 1234}}}}, False), ( @@ -526,6 +507,9 @@ class TaskSpecValidation(v2_base.WorkflowSpecValidationTestCase): ({'on-error': {'next': 'email'}}, False), ({'on-error': {'next': ['email']}}, False), ({'on-error': {'next': [{'email': 'email'}]}}, True), + ({'on-error': {'next': [{'email': 'email', + 'more_email': 'more_email'}]}}, True), + ({'on-error': {'next': {'email': 'email'}}}, True), ({'on-complete': {'publish': {'var1': 1234}}}, True), ({'on-complete': {'publish': {'branch': {'var1': 1234}}}}, False), ( @@ -620,7 +604,10 @@ class TaskSpecValidation(v2_base.WorkflowSpecValidationTestCase): ({'on-complete': {'next': [{'email': '<% $.v1 %>'}]}}, False), ({'on-complete': {'next': 'email'}}, False), ({'on-complete': {'next': ['email']}}, False), - ({'on-complete': {'next': [{'email': 'email'}]}}, True) + ({'on-complete': {'next': [{'email': 'email'}]}}, True), + ({'on-complete': {'next': [{'email': 'email', + 'more_email': 'more_email'}]}}, True), + ({'on-complete': {'next': {'email': 'email'}}}, True) ] for transition, expect_error in tests: