Improving services/workbooks

* Take care of workbook updates
* Unit tests

Change-Id: Iae9d13f335b8cca2409ad0731091edbbaa24a271

mistral/services/workbooks.py

@@ -19,6 +19,7 @@ from oslo.config import cfg
 from mistral import context
 from mistral.db.v1 import api as db_api_v1
 from mistral.db.v2 import api as db_api_v2
+from mistral import exceptions as exc
 from mistral.services import scheduler
 from mistral.services import trusts
 from mistral.workbook import parser as spec_parser
@@ -41,6 +42,7 @@ def update_workbook_v1(workbook_name, values):
 
 def create_workbook_v2(values):
     _add_security_info(values)
+    _update_specification(values)
 
     db_api_v2.start_tx()
 
@@ -57,10 +59,12 @@ def create_workbook_v2(values):
 
 
 def update_workbook_v2(workbook_name, values):
+    _update_specification(values)
+
     db_api_v2.start_tx()
 
     try:
-        wb_db = db_api_v1.workbook_update(workbook_name, values)
+        wb_db = db_api_v2.update_workbook(workbook_name, values)
 
         _check_workbook_definition_update(wb_db, values)
 
@@ -72,33 +76,41 @@ def update_workbook_v2(workbook_name, values):
 
 
 def _check_workbook_definition_update(wb_db, values):
-    if 'definition' not in values:
+    if 'spec' not in values:
         return
 
-    wb_spec = spec_parser.get_workbook_spec_from_yaml(values['definition'])
+    wb_spec = spec_parser.get_workbook_spec(values['spec'])
 
-    _create_actions(wb_db, wb_spec.get_actions())
-    _create_workflows(wb_db, wb_spec.get_workflows())
+    _create_or_update_actions(wb_db, wb_spec.get_actions())
+    _create_or_update_workflows(wb_db, wb_spec.get_workflows())
 
 
-def _create_actions(wb_db, actions_spec):
+def _create_or_update_actions(wb_db, actions_spec):
     if actions_spec:
         # TODO(rakhmerov): Complete when action DB model is added.
         pass
 
 
-def _create_workflows(wb_db, workflows_spec):
+def _create_or_update_workflows(wb_db, workflows_spec):
     if workflows_spec:
         for wf_spec in workflows_spec:
-            db_api_v2.create_workflow(
-                {
-                    'name': '%s.%s' % (wb_db.name, wf_spec.get_name()),
-                    'spec': wf_spec.to_dict(),
-                    'scope': wb_db.scope,
-                    'trust_id': wb_db.trust_id,
-                    'project_id': wb_db.project_id
-                }
-            )
+            wf_name = '%s.%s' % (wb_db.name, wf_spec.get_name())
+
+            values = {
+                'name': wf_name,
+                'spec': wf_spec.to_dict(),
+                'scope': wb_db.scope,
+                'trust_id': wb_db.trust_id,
+                'project_id': wb_db.project_id
+            }
+
+            # TODO(rakhmerov): This looks really ugly. Need more decent way.
+            try:
+                db_api_v2.get_workflow(wf_name)
+
+                db_api_v2.update_workflow(wf_name, values)
+            except exc.NotFoundException:
+                db_api_v2.create_workflow(values)
 
 
 def _add_security_info(values):
@@ -107,3 +119,13 @@ def _add_security_info(values):
             'trust_id': trusts.create_trust().id,
             'project_id': context.ctx().project_id
         })
+
+
+def _update_specification(values):
+    # No need to do anything if specification gets pushed explicitly.
+    if 'spec' in values:
+        return
+
+    if 'definition' in values:
+        spec = spec_parser.get_workbook_spec_from_yaml(values['definition'])
+        values['spec'] = spec.to_dict()

mistral/tests/unit/services/test_workbook_service.py

@@ -0,0 +1,167 @@
+# Copyright 2014 - Mirantis, 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 oslo.config import cfg
+
+from mistral.db.v2 import api as db_api
+from mistral.openstack.common import log as logging
+from mistral.services import workbooks as wb_service
+from mistral.tests.unit.engine1 import base
+from mistral.workbook import parser as spec_parser
+
+LOG = logging.getLogger(__name__)
+
+# Use the set_default method to set value otherwise in certain test cases
+# the change in value is not permanent.
+cfg.CONF.set_default('auth_enable', False, group='pecan')
+
+
+WORKBOOK = """
+---
+Version: '2.0'
+
+Workflows:
+  wf1:
+    type: reverse
+    parameters:
+      - param1
+    output:
+      result: $.result
+
+    tasks:
+      task1:
+        action: std.echo output="{$.param1}"
+        publish:
+          result: $
+
+  wf2:
+    type: direct
+    start_task: task1
+    output:
+      result: $.result
+
+    tasks:
+      task1:
+        workflow: my_wb.wf1 param1='Hi'
+        workflow_parameters:
+            task_name: task1
+        publish:
+          result: "The result of subworkflow is '{$.final_result}'"
+"""
+
+UPDATED_WORKBOOK = """
+---
+Version: '2.0'
+
+Workflows:
+  wf1:
+    type: direct
+    start_task: task1
+    output:
+      result: $.result
+
+    tasks:
+      task1:
+        workflow: my_wb.wf2 param1='Hi'
+        workflow_parameters:
+            task_name: task1
+        publish:
+          result: "The result of subworkflow is '{$.final_result}'"
+
+  wf2:
+    type: reverse
+    parameters:
+      - param1
+    output:
+      result: $.result
+
+    tasks:
+      task1:
+        action: std.echo output="{$.param1}"
+        publish:
+          result: $
+"""
+
+
+class SubworkflowsTest(base.EngineTestCase):
+    def test_create_workbook(self):
+        wb_db = wb_service.create_workbook_v2({
+            'name': 'my_wb',
+            'definition': WORKBOOK,
+            'tags': ['test']
+        })
+
+        self.assertIsNotNone(wb_db)
+        self.assertEqual('my_wb', wb_db.name)
+        self.assertEqual(WORKBOOK, wb_db.definition)
+        self.assertIsNotNone(wb_db.spec)
+        self.assertListEqual(['test'], wb_db.tags)
+
+        db_wfs = db_api.get_workflows()
+
+        self.assertEqual(2, len(db_wfs))
+
+        # Workflow 1.
+        wf1_db = self._assert_single_item(db_wfs, name='my_wb.wf1')
+        wf1_spec = spec_parser.get_workflow_spec(wf1_db.spec)
+
+        self.assertEqual('wf1', wf1_spec.get_name())
+        self.assertEqual('reverse', wf1_spec.get_type())
+
+        # Workflow 2.
+        wf2_db = self._assert_single_item(db_wfs, name='my_wb.wf2')
+        wf2_spec = spec_parser.get_workflow_spec(wf2_db.spec)
+
+        self.assertEqual('wf2', wf2_spec.get_name())
+        self.assertEqual('direct', wf2_spec.get_type())
+
+    def test_update_workbook(self):
+        # Create workbook.
+        wb_db = wb_service.create_workbook_v2({
+            'name': 'my_wb',
+            'definition': WORKBOOK,
+            'tags': ['test']
+        })
+
+        self.assertIsNotNone(wb_db)
+        self.assertEqual(2, len(db_api.get_workflows()))
+
+        # Update workbook.
+        wb_db = wb_service.update_workbook_v2(
+            'my_wb',
+            {'definition': UPDATED_WORKBOOK}
+        )
+
+        self.assertIsNotNone(wb_db)
+        self.assertEqual('my_wb', wb_db.name)
+        self.assertEqual(UPDATED_WORKBOOK, wb_db.definition)
+        self.assertListEqual(['test'], wb_db.tags)
+
+        db_wfs = db_api.get_workflows()
+
+        self.assertEqual(2, len(db_wfs))
+
+        # Workflow 1.
+        wf1_db = self._assert_single_item(db_wfs, name='my_wb.wf1')
+        wf1_spec = spec_parser.get_workflow_spec(wf1_db.spec)
+
+        self.assertEqual('wf1', wf1_spec.get_name())
+        self.assertEqual('direct', wf1_spec.get_type())
+
+        # Workflow 2.
+        wf2_db = self._assert_single_item(db_wfs, name='my_wb.wf2')
+        wf2_spec = spec_parser.get_workflow_spec(wf2_db.spec)
+
+        self.assertEqual('wf2', wf2_spec.get_name())
+        self.assertEqual('reverse', wf2_spec.get_type())