From 9f2043e41e7c1c8a35331bc42ae93cbcd1e8b0e5 Mon Sep 17 00:00:00 2001
From: Nikolay Mahotkin <nmakhotkin@mirantis.com>
Date: Mon, 9 Jun 2014 14:37:13 +0400
Subject: [PATCH] Fix create execution when workbook does not exist

 * Controller return 404 Not Found if Workbook doesn't exist
   or workbook definition is empty (in case of empty
   Definition we can't start Execution)

Closes-bug: #1325914

Change-Id: I5ae18d378916c6eba81088603414300891f1b816
---
 functionaltests/api/v1/test_mistral_basic.py  |  1 -
 mistral/api/controllers/v1/execution.py       | 18 ++++++-----
 mistral/db/api.py                             |  8 ++++-
 .../api/v1/controllers/test_executions.py     | 30 +++++++++++++++++++
 4 files changed, 47 insertions(+), 10 deletions(-)

diff --git a/functionaltests/api/v1/test_mistral_basic.py b/functionaltests/api/v1/test_mistral_basic.py
index 7b04052f..f1f15cc8 100644
--- a/functionaltests/api/v1/test_mistral_basic.py
+++ b/functionaltests/api/v1/test_mistral_basic.py
@@ -181,7 +181,6 @@ class AdvancedTests(base.TestCaseAdvanced):
 
     #TODO(smurashov): Need to add test which would check task update
 
-    @testtools.skip('https://bugs.launchpad.net/mistral/+bug/1325914')
     @test.attr(type='negative')
     def test_create_execution_in_nonexistent_workbook(self):
         self.assertRaises(exceptions.NotFound, self._create_execution,
diff --git a/mistral/api/controllers/v1/execution.py b/mistral/api/controllers/v1/execution.py
index 7333bc8b..09f8cad9 100644
--- a/mistral/api/controllers/v1/execution.py
+++ b/mistral/api/controllers/v1/execution.py
@@ -116,16 +116,18 @@ class ExecutionsController(rest.RestController):
         LOG.debug("Create execution [workbook_name=%s, execution=%s]" %
                   (workbook_name, execution))
 
-        context = None
-        if execution.context:
-            context = json.loads(execution.context)
+        if (db_api.workbook_get(workbook_name)
+                and db_api.workbook_definition_get(workbook_name)):
+            context = None
+            if execution.context:
+                context = json.loads(execution.context)
 
-        engine = pecan.request.context['engine']
-        values = engine.start_workflow_execution(execution.workbook_name,
-                                                 execution.task,
-                                                 context)
+            engine = pecan.request.context['engine']
+            values = engine.start_workflow_execution(execution.workbook_name,
+                                                     execution.task,
+                                                     context)
 
-        return Execution.from_dict(values)
+            return Execution.from_dict(values)
 
     @rest_utils.wrap_wsme_controller_exception
     @wsme_pecan.wsexpose(None, wtypes.text, wtypes.text, status_code=204)
diff --git a/mistral/db/api.py b/mistral/db/api.py
index ac89c682..7efa9101 100644
--- a/mistral/db/api.py
+++ b/mistral/db/api.py
@@ -14,6 +14,8 @@
 #    See the License for the specific language governing permissions and
 #    limitations under the License.
 
+from mistral import exceptions
+
 from mistral.openstack.common.db import api as db_api
 from mistral.openstack.common import log as logging
 
@@ -78,7 +80,11 @@ def workbooks_get():
 
 
 def workbook_definition_get(workbook_name):
-    return IMPL.workbook_get(workbook_name)['definition']
+    definition = IMPL.workbook_get(workbook_name)['definition']
+    if not definition:
+        raise exceptions.NotFoundException("Definition of workbook "
+                                           "%s is empty." % workbook_name)
+    return definition
 
 
 def workbook_definition_put(workbook_name, text):
diff --git a/mistral/tests/api/v1/controllers/test_executions.py b/mistral/tests/api/v1/controllers/test_executions.py
index a14f91a8..11c75390 100644
--- a/mistral/tests/api/v1/controllers/test_executions.py
+++ b/mistral/tests/api/v1/controllers/test_executions.py
@@ -41,6 +41,16 @@ EXECS = [
     }
 ]
 
+
+WORKBOOKS = [
+    {
+        'name': "my_workbook",
+        'description': "My cool Mistral workbook",
+        'tags': ['deployment', 'demo']
+    }
+]
+
+
 UPDATED_EXEC = EXECS[0].copy()
 UPDATED_EXEC['state'] = 'STOPPED'
 
@@ -88,7 +98,13 @@ class TestExecutionsController(base.FunctionalTest):
 
     @mock.patch.object(engine.EngineClient, 'start_workflow_execution',
                        mock.MagicMock(return_value=EXECS[0]))
+    @mock.patch.object(db_api, 'workbook_definition_get',
+                       mock.Mock(return_value="Workflow:"))
     def test_post(self):
+        my_workbook = WORKBOOKS[0]
+        self.app.post_json('/v1/workbooks',
+                           my_workbook)
+
         new_exec = EXECS[0].copy()
         new_exec['context'] = json.dumps(new_exec['context'])
 
@@ -97,6 +113,20 @@ class TestExecutionsController(base.FunctionalTest):
         self.assertEqual(resp.status_int, 201)
         self.assertDictEqual(EXECS[0], canonize(resp.json))
 
+    @mock.patch.object(engine.EngineClient, 'start_workflow_execution',
+                       mock.MagicMock(return_value=EXECS[0]))
+    def test_post_definition_empty(self):
+        my_workbook = WORKBOOKS[0]
+        self.app.post_json('/v1/workbooks',
+                           my_workbook)
+
+        new_exec = EXECS[0].copy()
+        new_exec['context'] = json.dumps(new_exec['context'])
+
+        resp = self.app.post_json('/v1/workbooks/my_workbook/executions',
+                                  new_exec, expect_errors=True)
+        self.assertEqual(resp.status_int, 404)
+
     @mock.patch.object(engine.EngineClient, 'start_workflow_execution',
                        mock.MagicMock(side_effect=ex.MistralException))
     def test_post_throws_exception(self):