Merge "Refactor get_task_spec using mechanism of polymorphic DSL entities"

mistral/tests/functional/api/v2/test_mistral_basic_v2.py

@@ -356,8 +356,8 @@ class ExecutionTestsV2(base.TestCase):
             self.client.delete_obj('workflows', wf)
         self.client.workflows = []
 
-        for wf in self.client.executions:
+        for ex in self.client.executions:
-            self.client.delete_obj('executions', wf)
+            self.client.delete_obj('executions', ex)
         self.client.executions = []
 
         super(ExecutionTestsV2, self).tearDown()

mistral/tests/unit/api/v2/test_workbooks.py

@@ -146,8 +146,7 @@ class TestWorkbooksController(base.FunctionalTest):
         )
 
         self.assertEqual(resp.status_int, 400)
-        self.assertIn("Task properties 'action' and 'workflow' "
+        self.assertIn("Invalid DSL", resp.body)
-                      "can't be specified both", resp.body)
 
     @mock.patch.object(workbooks, "create_workbook_v2", MOCK_WORKBOOK)
     def test_post(self):
@@ -180,8 +179,7 @@ class TestWorkbooksController(base.FunctionalTest):
         )
 
         self.assertEqual(resp.status_int, 400)
-        self.assertIn("Task properties 'action' and 'workflow' "
+        self.assertIn("Invalid DSL", resp.body)
-                      "can't be specified both", resp.body)
 
     @mock.patch.object(db_api, "delete_workbook", MOCK_DELETE)
     def test_delete(self):
@@ -232,8 +230,7 @@ class TestWorkbooksController(base.FunctionalTest):
 
         self.assertEqual(resp.status_int, 200)
         self.assertFalse(resp.json['valid'])
-        self.assertIn("Task properties 'action' and 'workflow' "
+        self.assertIn("Invalid DSL", resp.json['error'])
-                      "can't be specified both", resp.json['error'])
 
     def test_validate_dsl_parse_exception(self):
         resp = self.app.post(

mistral/tests/unit/api/v2/test_workflows.py

@@ -221,8 +221,7 @@ class TestWorkflowsController(base.FunctionalTest):
         )
 
         self.assertEqual(resp.status_int, 400)
-        self.assertIn("Task properties 'action' and 'workflow' "
+        self.assertIn("Invalid DSL", resp.body)
-                      "can't be specified both", resp.body)
 
     @mock.patch.object(db_api, "create_workflow_definition")
     def test_post(self, mock_mtd):
@@ -264,8 +263,7 @@ class TestWorkflowsController(base.FunctionalTest):
         )
 
         self.assertEqual(resp.status_int, 400)
-        self.assertIn("Task properties 'action' and 'workflow' "
+        self.assertIn("Invalid DSL", resp.body)
-                      "can't be specified both", resp.body)
 
     @mock.patch.object(db_api, "delete_workflow_definition", MOCK_DELETE)
     def test_delete(self):
@@ -416,8 +414,7 @@ class TestWorkflowsController(base.FunctionalTest):
 
         self.assertEqual(resp.status_int, 200)
         self.assertFalse(resp.json['valid'])
-        self.assertIn("Task properties 'action' and 'workflow' "
+        self.assertIn("Invalid DSL", resp.json['error'])
-                      "can't be specified both", resp.json['error'])
 
     def test_validate_dsl_parse_exception(self):
         resp = self.app.post(

mistral/tests/unit/workbook/v2/test_tasks.py

@@ -15,33 +15,14 @@
 
 from oslo_log import log as logging
 
-from mistral import exceptions
-from mistral.tests import base
 from mistral.tests.unit.workbook.v2 import base as v2_base
 from mistral import utils
-from mistral.workbook.v2 import tasks
 from mistral.workbook.v2 import workflows
 
 
 LOG = logging.getLogger(__name__)
 
 
-class TaskSpecListTest(base.BaseTest):
-    def test_get_class(self):
-        spec_list_cls = tasks.TaskSpecList.get_class('direct')
-
-        self.assertIs(spec_list_cls, tasks.DirectWfTaskSpecList)
-
-    def test_get_class_notfound(self):
-        exc = self.assertRaises(
-            exceptions.NotFoundException,
-            tasks.TaskSpecList.get_class,
-            "invalid"
-        )
-
-        self.assertIn("Can not find task list specification", str(exc))
-
-
 class TaskSpecValidation(v2_base.WorkflowSpecValidationTestCase):
     def test_type_injection(self):
         tests = [

mistral/tests/unit/workbook/v2/test_workflows.py

@@ -20,7 +20,6 @@ import yaml
 from mistral import exceptions as exc
 from mistral.tests.unit.workbook.v2 import base
 from mistral import utils
-from mistral.workbook.v2 import tasks
 
 LOG = logging.getLogger(__name__)
 
@@ -56,8 +55,6 @@ class WorkflowSpecValidation(base.WorkflowSpecValidationTestCase):
         self.assertEqual(1, len(wfs_spec.get_workflows()))
         self.assertEqual('test', wfs_spec.get_workflows()[0].get_name())
         self.assertEqual('direct', wfs_spec.get_workflows()[0].get_type())
-        self.assertIsInstance(wfs_spec.get_workflows()[0].get_tasks(),
-                              tasks.DirectWfTaskSpecList)
 
     def test_direct_workflow_invalid_task(self):
         overlay = {
@@ -102,8 +99,6 @@ class WorkflowSpecValidation(base.WorkflowSpecValidationTestCase):
         self.assertEqual(1, len(wfs_spec.get_workflows()))
         self.assertEqual('test', wfs_spec.get_workflows()[0].get_name())
         self.assertEqual('reverse', wfs_spec.get_workflows()[0].get_type())
-        self.assertIsInstance(wfs_spec.get_workflows()[0].get_tasks(),
-                              tasks.ReverseWfTaskSpecList)
 
     def test_reverse_workflow_invalid_task(self):
         overlay = {'test': {'type': 'reverse', 'tasks': {}}}

mistral/workbook/parser.py

@@ -115,13 +115,6 @@ def get_workflow_list_spec_from_yaml(text):
 
 def get_task_spec(spec_dict):
     if _get_spec_version(spec_dict) == V2_0:
-        workflow_type = spec_dict.get('type')
+        return base.instantiate_spec(tasks_v2.TaskSpec, spec_dict)
-
-        if workflow_type == 'direct':
-            return tasks_v2.DirectWorkflowTaskSpec(spec_dict)
-        elif workflow_type == 'reverse':
-            return tasks_v2.ReverseWorkflowTaskSpec(spec_dict)
-        else:
-            raise Exception('Unsupported workflow type "%s".' % workflow_type)
 
     return None

mistral/workbook/v2/tasks.py

@@ -33,7 +33,7 @@ WITH_ITEMS_PTRN = re.compile(
 
 class TaskSpec(base.BaseSpec):
     # See http://json-schema.org
-    _type = None
+    _polymorphic_key = ('type', 'direct')
 
     _schema = {
         "type": "object",
@@ -58,7 +58,27 @@ class TaskSpec(base.BaseSpec):
             "target": types.NONEMPTY_STRING,
             "keep-result": types.YAQL_OR_BOOLEAN
         },
-        "additionalProperties": False
+        "additionalProperties": False,
+        "anyOf": [
+            {
+                "not": {
+                    "type": "object",
+                    "required": ["action", "workflow"]
+                },
+            },
+            {
+                "oneOf": [
+                    {
+                        "type": "object",
+                        "required": ["action"]
+                    },
+                    {
+                        "type": "object",
+                        "required": ["workflow"]
+                    }
+                ]
+            }
+        ]
     }
 
     def __init__(self, data):
@@ -83,22 +103,16 @@ class TaskSpec(base.BaseSpec):
         self._target = data.get('target')
         self._keep_result = data.get('keep-result', True)
 
-        self._inject_type()
         self._process_action_and_workflow()
 
     def validate_schema(self):
         super(TaskSpec, self).validate_schema()
 
+        self._transform_with_items()
+
         action = self._data.get('action')
         workflow = self._data.get('workflow')
 
-        if action and workflow:
-            msg = ("Task properties 'action' and 'workflow' can't be"
-                   " specified both: %s" % self._data)
-            raise exc.InvalidModelException(msg)
-
-        self._transform_with_items()
-
         # Validate YAQL expressions.
         if action or workflow:
             inline_params = self._parse_cmd_and_input(action or workflow)[1]
@@ -146,10 +160,6 @@ class TaskSpec(base.BaseSpec):
 
         return with_items
 
-    def _inject_type(self):
-        if self._type:
-            self._data['type'] = self._type
-
     def _process_action_and_workflow(self):
         params = {}
 
@@ -198,7 +208,7 @@ class TaskSpec(base.BaseSpec):
 
 
 class DirectWorkflowTaskSpec(TaskSpec):
-    _type = 'direct'
+    _polymorphic_value = 'direct'
 
     _on_clause_type = {
         "oneOf": [
@@ -210,7 +220,7 @@ class DirectWorkflowTaskSpec(TaskSpec):
     _direct_workflow_schema = {
         "type": "object",
         "properties": {
-            "type": {"enum": [_type]},
+            "type": {"enum": [_polymorphic_value]},
             "join": {
                 "oneOf": [
                     {"enum": ["all", "one"]},
@@ -270,12 +280,12 @@ class DirectWorkflowTaskSpec(TaskSpec):
 
 
 class ReverseWorkflowTaskSpec(TaskSpec):
-    _type = 'reverse'
+    _polymorphic_value = 'reverse'
 
     _reverse_workflow_schema = {
         "type": "object",
         "properties": {
-            "type": {"enum": [_type]},
+            "type": {"enum": [_polymorphic_value]},
             "requires": {
                 "oneOf": [types.NONEMPTY_STRING, types.UNIQUE_STRING_LIST]
             }
@@ -299,28 +309,3 @@ class ReverseWorkflowTaskSpec(TaskSpec):
 
 class TaskSpecList(base.BaseSpecList):
     item_class = TaskSpec
-
-    @staticmethod
-    def get_class(wf_type):
-        """Gets a task specification list class by given workflow type.
-
-        :param wf_type: Workflow type
-        :returns: Task specification list class
-        """
-        for spec_list_cls in utils.iter_subclasses(TaskSpecList):
-            if wf_type == spec_list_cls.__type__:
-                return spec_list_cls
-
-        msg = ("Can not find task list specification with workflow type:"
-               " %s" % wf_type)
-        raise exc.NotFoundException(msg)
-
-
-class DirectWfTaskSpecList(TaskSpecList):
-    __type__ = 'direct'
-    item_class = DirectWorkflowTaskSpec
-
-
-class ReverseWfTaskSpecList(TaskSpecList):
-    __type__ = 'reverse'
-    item_class = ReverseWorkflowTaskSpec

mistral/workbook/v2/workflows.py

@@ -57,10 +57,13 @@ class WorkflowSpec(base.BaseSpec):
             'task-defaults',
             task_defaults.TaskDefaultsSpec
         )
-        self._tasks = self._spec_property(
+
-            'tasks',
+        # Inject 'type' here, so instantiate_spec function can recognize the
-            tasks.TaskSpecList.get_class(self._type)
+        # specific subclass of TaskSpec.
-        )
+        for task in self._data.get('tasks').itervalues():
+            task['type'] = self._type
+
+        self._tasks = self._spec_property('tasks', tasks.TaskSpecList)
 
     def validate_schema(self):
         super(WorkflowSpec, self).validate_schema()