diff --git a/mistral/api/controllers/v1/execution.py b/mistral/api/controllers/v1/execution.py index db75749a9..aab6cd07d 100644 --- a/mistral/api/controllers/v1/execution.py +++ b/mistral/api/controllers/v1/execution.py @@ -18,15 +18,14 @@ import json import pecan from pecan import rest -from pecan import abort from wsme import types as wtypes import wsmeext.pecan as wsme_pecan -from mistral import exceptions as ex from mistral.api.controllers.v1 import task from mistral.openstack.common import log as logging from mistral.api.controllers import resource from mistral.db import api as db_api +from mistral.utils import rest_utils LOG = logging.getLogger(__name__) @@ -75,17 +74,15 @@ class ExecutionsController(rest.RestController): tasks = task.TasksController() + @rest_utils.wrap_wsme_controller_exception @wsme_pecan.wsexpose(Execution, wtypes.text, wtypes.text) def get(self, workbook_name, id): LOG.debug("Fetch execution [workbook_name=%s, id=%s]" % (workbook_name, id)) - #TODO (nmakhotkin) This should be refactored later - try: - values = db_api.execution_get(workbook_name, id) - return Execution.from_dict(values) - except ex.NotFoundException as e: - abort(404, e.message) + values = db_api.execution_get(workbook_name, id) + return Execution.from_dict(values) + @rest_utils.wrap_wsme_controller_exception @wsme_pecan.wsexpose(Execution, wtypes.text, wtypes.text, body=Execution) def put(self, workbook_name, id, execution): LOG.debug("Update execution [workbook_name=%s, id=%s, execution=%s]" % @@ -97,35 +94,30 @@ class ExecutionsController(rest.RestController): return Execution.from_dict(values) + @rest_utils.wrap_wsme_controller_exception @wsme_pecan.wsexpose(Execution, wtypes.text, body=Execution, status_code=201) def post(self, workbook_name, execution): LOG.debug("Create execution [workbook_name=%s, execution=%s]" % (workbook_name, execution)) - try: - context = None - if execution.context: - context = json.loads(execution.context) + 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) - except ex.MistralException as e: - #TODO(nmakhotkin) we should use thing such a decorator here - abort(400, e.message) + engine = pecan.request.context['engine'] + values = engine.start_workflow_execution(execution.workbook_name, + execution.task, + context) return Execution.from_dict(values) + @rest_utils.wrap_wsme_controller_exception @wsme_pecan.wsexpose(None, wtypes.text, wtypes.text, status_code=204) def delete(self, workbook_name, id): LOG.debug("Delete execution [workbook_name=%s, id=%s]" % (workbook_name, id)) - try: - db_api.execution_delete(workbook_name, id) - except ex.NotFoundException as e: - abort(404, e.message) + db_api.execution_delete(workbook_name, id) @wsme_pecan.wsexpose(Executions, wtypes.text) def get_all(self, workbook_name): diff --git a/mistral/api/controllers/v1/listener.py b/mistral/api/controllers/v1/listener.py index 64581e8fb..2bd12a80a 100644 --- a/mistral/api/controllers/v1/listener.py +++ b/mistral/api/controllers/v1/listener.py @@ -15,13 +15,13 @@ # limitations under the License. from pecan import rest -from pecan import abort from wsme import types as wtypes import wsmeext.pecan as wsme_pecan from mistral.openstack.common import log as logging from mistral.api.controllers import resource from mistral.db import api as db_api +from mistral.utils import rest_utils LOG = logging.getLogger(__name__) @@ -58,17 +58,14 @@ class Listeners(resource.Resource): class ListenersController(rest.RestController): + @rest_utils.wrap_wsme_controller_exception @wsme_pecan.wsexpose(Listener, wtypes.text, wtypes.text) def get(self, workbook_name, id): LOG.debug("Fetch listener [workbook_name=%s, id=%s]" % (workbook_name, id)) values = db_api.listener_get(workbook_name, id) - - if not values: - abort(404) - else: - return Listener.from_dict(values) + return Listener.from_dict(values) @wsme_pecan.wsexpose(Listener, wtypes.text, wtypes.text, body=Listener) def put(self, workbook_name, id, listener): @@ -79,6 +76,7 @@ class ListenersController(rest.RestController): return Listener.from_dict(values) + @rest_utils.wrap_wsme_controller_exception @wsme_pecan.wsexpose(Listener, wtypes.text, body=Listener, status_code=201) def post(self, workbook_name, listener): LOG.debug("Create listener [workbook_name=%s, listener=%s]" % @@ -88,6 +86,7 @@ class ListenersController(rest.RestController): return Listener.from_dict(values) + @rest_utils.wrap_wsme_controller_exception @wsme_pecan.wsexpose(None, wtypes.text, wtypes.text, status_code=204) def delete(self, workbook_name, id): LOG.debug("Delete listener [workbook_name=%s, id=%s]" % diff --git a/mistral/api/controllers/v1/task.py b/mistral/api/controllers/v1/task.py index c026dc9fa..49b7d93cd 100644 --- a/mistral/api/controllers/v1/task.py +++ b/mistral/api/controllers/v1/task.py @@ -16,13 +16,13 @@ import pecan from pecan import rest -from pecan import abort from wsme import types as wtypes import wsmeext.pecan as wsme_pecan from mistral.openstack.common import log as logging from mistral.api.controllers import resource from mistral.db import api as db_api +from mistral.utils import rest_utils LOG = logging.getLogger(__name__) @@ -48,18 +48,16 @@ class Tasks(resource.Resource): class TasksController(rest.RestController): + @rest_utils.wrap_wsme_controller_exception @wsme_pecan.wsexpose(Task, wtypes.text, wtypes.text, wtypes.text) def get(self, workbook_name, execution_id, id): LOG.debug("Fetch task [workbook_name=%s, execution_id=%s, id=%s]" % (workbook_name, execution_id, id)) values = db_api.task_get(workbook_name, execution_id, id) - - if not values: - abort(404) - return Task.from_dict(values) + @rest_utils.wrap_wsme_controller_exception @wsme_pecan.wsexpose(Task, wtypes.text, wtypes.text, wtypes.text, body=Task) def put(self, workbook_name, execution_id, id, task): diff --git a/mistral/api/controllers/v1/workbook.py b/mistral/api/controllers/v1/workbook.py index 93bf9ab58..57426198f 100644 --- a/mistral/api/controllers/v1/workbook.py +++ b/mistral/api/controllers/v1/workbook.py @@ -15,16 +15,15 @@ # limitations under the License. from pecan import rest -from pecan import abort from wsme import types as wtypes import wsmeext.pecan as wsme_pecan -from mistral import exceptions as ex from mistral.api.controllers.v1 import workbook_definition from mistral.api.controllers.v1 import listener from mistral.api.controllers.v1 import execution from mistral.api.controllers import resource from mistral.services import workbooks +from mistral.utils import rest_utils from mistral.openstack.common import log as logging from mistral.db import api as db_api @@ -51,17 +50,15 @@ class WorkbooksController(rest.RestController): listeners = listener.ListenersController() executions = execution.ExecutionsController() + @rest_utils.wrap_wsme_controller_exception @wsme_pecan.wsexpose(Workbook, wtypes.text) def get(self, name): LOG.debug("Fetch workbook [name=%s]" % name) - #TODO (nmakhotkin) This should be refactored later - try: - values = db_api.workbook_get(name) - return Workbook.from_dict(values) - except ex.NotFoundException as e: - abort(404, e.message) + values = db_api.workbook_get(name) + return Workbook.from_dict(values) + @rest_utils.wrap_wsme_controller_exception @wsme_pecan.wsexpose(Workbook, wtypes.text, body=Workbook) def put(self, name, workbook): LOG.debug("Update workbook [name=%s, workbook=%s]" % (name, workbook)) @@ -69,23 +66,18 @@ class WorkbooksController(rest.RestController): return Workbook.from_dict(db_api.workbook_update(name, workbook.to_dict())) + @rest_utils.wrap_wsme_controller_exception @wsme_pecan.wsexpose(Workbook, body=Workbook, status_code=201) def post(self, workbook): LOG.debug("Create workbook [workbook=%s]" % workbook) - try: - wb = workbooks.create_workbook(workbook.to_dict()) - return Workbook.from_dict(wb) - except ex.MistralException as e: - #TODO(nmakhotkin) we should use thing such a decorator here - abort(400, e.message) + wb = workbooks.create_workbook(workbook.to_dict()) + return Workbook.from_dict(wb) + @rest_utils.wrap_wsme_controller_exception @wsme_pecan.wsexpose(None, wtypes.text, status_code=204) def delete(self, name): LOG.debug("Delete workbook [name=%s]" % name) - try: - db_api.workbook_delete(name) - except ex.NotFoundException as e: - abort(404, e.message) + db_api.workbook_delete(name) @wsme_pecan.wsexpose(Workbooks) def get_all(self): diff --git a/mistral/api/controllers/v1/workbook_definition.py b/mistral/api/controllers/v1/workbook_definition.py index a97e6578f..ea6112210 100644 --- a/mistral/api/controllers/v1/workbook_definition.py +++ b/mistral/api/controllers/v1/workbook_definition.py @@ -14,38 +14,34 @@ # See the License for the specific language governing permissions and # limitations under the License. -from pecan import abort from pecan import rest from pecan import expose from pecan import request -import six -from mistral import exceptions as exc from mistral.openstack.common import log as logging from mistral.db import api as db_api from mistral.services import scheduler +from mistral.utils import rest_utils LOG = logging.getLogger(__name__) class WorkbookDefinitionController(rest.RestController): + @rest_utils.wrap_pecan_controller_exception @expose() def get(self, workbook_name): LOG.debug("Fetch workbook definition [workbook_name=%s]" % workbook_name) - return db_api.workbook_definition_get(workbook_name) + @rest_utils.wrap_pecan_controller_exception @expose(content_type="text/plain") def put(self, workbook_name): text = request.text LOG.debug("Update workbook definition [workbook_name=%s, text=%s]" % (workbook_name, text)) - try: - wb = db_api.workbook_definition_put(workbook_name, text) - scheduler.create_associated_triggers(wb) - return wb['definition'] - except exc.MistralException as e: - abort(400, six.text_type(e)) + wb = db_api.workbook_definition_put(workbook_name, text) + scheduler.create_associated_triggers(wb) + return wb['definition'] diff --git a/mistral/exceptions.py b/mistral/exceptions.py index e854321d3..386eec6ab 100644 --- a/mistral/exceptions.py +++ b/mistral/exceptions.py @@ -26,6 +26,14 @@ class MistralException(ex.Error): message = "An unknown exception occurred" http_code = 500 + @property + def code(self): + """This is here for webob to read. + + https://github.com/Pylons/webob/blob/master/webob/exc.py + """ + return self.http_code + def __str__(self): return self.message diff --git a/mistral/tests/api/v1/controllers/test_executions.py b/mistral/tests/api/v1/controllers/test_executions.py index c7ea4d38d..a14f91a83 100644 --- a/mistral/tests/api/v1/controllers/test_executions.py +++ b/mistral/tests/api/v1/controllers/test_executions.py @@ -62,10 +62,11 @@ class TestExecutionsController(base.FunctionalTest): self.assertDictEqual(EXECS[0], canonize(resp.json)) @mock.patch.object(db_api, 'execution_get', - mock.MagicMock(side_effect=ex.NotFoundException)) + mock.MagicMock(side_effect=ex.NotFoundException())) def test_get_empty(self): - - self.assertNotFound('/v1/workbooks/my_workbook/executions/123') + resp = self.app.get('/v1/workbooks/my_workbook/executions/123', + expect_errors=True) + self.assertEqual(resp.status_int, 404) @mock.patch.object(db_api, 'execution_update', mock.MagicMock(return_value=UPDATED_EXEC)) @@ -76,6 +77,15 @@ class TestExecutionsController(base.FunctionalTest): self.assertEqual(resp.status_int, 200) self.assertDictEqual(UPDATED_EXEC, canonize(resp.json)) + @mock.patch.object(db_api, 'execution_update', + mock.MagicMock( + side_effect=ex.NotFoundException())) + def test_put_not_found(self): + resp = self.app.put_json('/v1/workbooks/my_workbook/executions/123', + dict(state='STOPPED'), expect_errors=True) + + self.assertEqual(resp.status_int, 404) + @mock.patch.object(engine.EngineClient, 'start_workflow_execution', mock.MagicMock(return_value=EXECS[0])) def test_post(self): @@ -102,6 +112,14 @@ class TestExecutionsController(base.FunctionalTest): self.assertEqual(resp.status_int, 204) + @mock.patch.object(db_api, 'execution_delete', + mock.MagicMock(side_effect=ex.NotFoundException)) + def test_delete_not_found(self): + resp = self.app.delete('/v1/workbooks/my_workbook/executions/123', + expect_errors=True) + + self.assertEqual(resp.status_int, 404) + @mock.patch.object(db_api, 'executions_get', mock.MagicMock(return_value=EXECS)) def test_get_all(self): diff --git a/mistral/tests/api/v1/controllers/test_listeners.py b/mistral/tests/api/v1/controllers/test_listeners.py index cad34bc37..e55f7a278 100644 --- a/mistral/tests/api/v1/controllers/test_listeners.py +++ b/mistral/tests/api/v1/controllers/test_listeners.py @@ -16,11 +16,10 @@ import mock +from mistral import exceptions from mistral.tests.api import base from mistral.db import api as db_api -# TODO: later we need additional tests verifying all the errors etc. - LISTENERS = [ { 'id': "1", @@ -43,6 +42,15 @@ class TestListenersController(base.FunctionalTest): self.assertEqual(resp.status_int, 200) self.assertDictEqual(LISTENERS[0], resp.json) + @mock.patch.object(db_api, "listener_get", + mock.MagicMock( + side_effect=exceptions.NotFoundException())) + def test_get_not_found(self): + resp = self.app.get('/v1/workbooks/my_workbook/listeners/1', + expect_errors=True) + + self.assertEqual(resp.status_int, 404) + @mock.patch.object(db_api, "listener_update", mock.MagicMock(return_value=UPDATED_LSNR)) def test_put(self): @@ -52,6 +60,16 @@ class TestListenersController(base.FunctionalTest): self.assertEqual(resp.status_int, 200) self.assertDictEqual(UPDATED_LSNR, resp.json) + @mock.patch.object(db_api, "listener_update", + mock.MagicMock( + side_effect=exceptions.NotFoundException())) + def test_put_not_found(self): + resp = self.app.put_json('/v1/workbooks/my_workbook/listeners/1', + dict(description='new description'), + expect_errors=True) + + self.assertEqual(resp.status_int, 404) + @mock.patch.object(db_api, "listener_create", mock.MagicMock(return_value=LISTENERS[0])) def test_post(self): @@ -68,6 +86,15 @@ class TestListenersController(base.FunctionalTest): self.assertEqual(resp.status_int, 204) + @mock.patch.object(db_api, "listener_delete", + mock.MagicMock( + side_effect=exceptions.NotFoundException())) + def test_delete_not_found(self): + resp = self.app.delete('/v1/workbooks/my_workbook/listeners/1', + expect_errors=True) + + self.assertEqual(resp.status_int, 404) + @mock.patch.object(db_api, "listeners_get", mock.MagicMock(return_value=LISTENERS)) def test_get_all(self): diff --git a/mistral/tests/api/v1/controllers/test_workbook_definition.py b/mistral/tests/api/v1/controllers/test_workbook_definition.py index 85fa9e5b1..3773750d7 100644 --- a/mistral/tests/api/v1/controllers/test_workbook_definition.py +++ b/mistral/tests/api/v1/controllers/test_workbook_definition.py @@ -16,11 +16,10 @@ import mock +from mistral import exceptions from mistral.tests.api import base from mistral.db import api as db_api -# TODO: later we need additional tests verifying all the errors etc. - DEFINITION = "my definition" NEW_DEFINITION = """ @@ -57,6 +56,16 @@ class TestWorkbookDefinitionController(base.FunctionalTest): self.assertEqual(resp.status_int, 200) self.assertEqual(DEFINITION, resp.text) + @mock.patch.object(db_api, "workbook_definition_get", + mock.MagicMock( + side_effect=exceptions.NotFoundException())) + def test_get_not_found(self): + resp = self.app.get('/v1/workbooks/my_workbook/definition', + headers={"Content-Type": "text/plain"}, + expect_errors=True) + + self.assertEqual(resp.status_int, 404) + @mock.patch.object(db_api, "workbook_definition_put", mock.MagicMock(return_value={ 'name': 'my_workbook', @@ -76,3 +85,14 @@ class TestWorkbookDefinitionController(base.FunctionalTest): self.assertEqual(triggers[0]['name'], 'create-vms') self.assertEqual(triggers[0]['pattern'], '* * * * *') self.assertEqual(triggers[0]['workbook_name'], 'my_workbook') + + @mock.patch.object(db_api, "workbook_definition_put", + mock.MagicMock( + side_effect=exceptions.NotFoundException())) + def test_put_not_found(self): + resp = self.app.put('/v1/workbooks/my_workbook/definition', + NEW_DEFINITION, + headers={"Content-Type": "text/plain"}, + expect_errors=True) + + self.assertEqual(resp.status_int, 404) diff --git a/mistral/tests/api/v1/controllers/test_workbooks.py b/mistral/tests/api/v1/controllers/test_workbooks.py index 56f4d4450..b7557677a 100644 --- a/mistral/tests/api/v1/controllers/test_workbooks.py +++ b/mistral/tests/api/v1/controllers/test_workbooks.py @@ -16,11 +16,10 @@ import mock +from mistral import exceptions from mistral.tests.api import base from mistral.db import api as db_api -# TODO: later we need additional tests verifying all the errors etc. - WORKBOOKS = [ { 'name': "my_workbook", @@ -42,6 +41,13 @@ class TestWorkbooksController(base.FunctionalTest): self.assertEqual(resp.status_int, 200) self.assertDictEqual(WORKBOOKS[0], resp.json) + @mock.patch.object(db_api, "workbook_get", + mock.MagicMock( + side_effect=exceptions.NotFoundException())) + def test_get_not_found(self): + resp = self.app.get('/v1/workbooks/dev_null', expect_errors=True) + self.assertEqual(resp.status_int, 404) + @mock.patch.object(db_api, "workbook_update", mock.MagicMock(return_value=UPDATED_WORKBOOK)) def test_put(self): @@ -51,6 +57,15 @@ class TestWorkbooksController(base.FunctionalTest): self.assertEqual(resp.status_int, 200) self.assertDictEqual(UPDATED_WORKBOOK, resp.json) + @mock.patch.object(db_api, "workbook_update", + mock.MagicMock( + side_effect=exceptions.NotFoundException())) + def test_put_not_found(self): + resp = self.app.put_json('/v1/workbooks/my_workbook', + dict(description='new description'), + expect_errors=True) + self.assertEqual(resp.status_int, 404) + @mock.patch.object(db_api, "workbook_create", mock.MagicMock(return_value=WORKBOOKS[0])) @mock.patch("mistral.services.trusts.create_trust", @@ -61,6 +76,16 @@ class TestWorkbooksController(base.FunctionalTest): self.assertEqual(resp.status_int, 201) self.assertDictEqual(WORKBOOKS[0], resp.json) + @mock.patch.object(db_api, "workbook_create", + mock.MagicMock(side_effect=exceptions.DBDuplicateEntry)) + @mock.patch("mistral.services.trusts.create_trust", + mock.MagicMock(return_value=WORKBOOKS[0])) + def test_post_dup(self): + resp = self.app.post_json('/v1/workbooks', WORKBOOKS[0], + expect_errors=True) + + self.assertEqual(resp.status_int, 409) + @mock.patch.object(db_api, "workbook_delete", mock.MagicMock(return_value=None)) def test_delete(self): @@ -68,6 +93,14 @@ class TestWorkbooksController(base.FunctionalTest): self.assertEqual(resp.status_int, 204) + @mock.patch.object(db_api, "workbook_delete", + mock.MagicMock( + side_effect=exceptions.NotFoundException())) + def test_delete_not_found(self): + resp = self.app.delete('/v1/workbooks/my_workbook', expect_errors=True) + + self.assertEqual(resp.status_int, 404) + @mock.patch.object(db_api, "workbooks_get", mock.MagicMock(return_value=WORKBOOKS)) def test_get_all(self): diff --git a/mistral/utils/rest_utils.py b/mistral/utils/rest_utils.py new file mode 100644 index 000000000..36a20dbb9 --- /dev/null +++ b/mistral/utils/rest_utils.py @@ -0,0 +1,52 @@ +# -*- coding: utf-8 -*- +# +# 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. + +import functools + +import pecan +import six +from wsme import exc + +from mistral import exceptions as ex + + +def wrap_wsme_controller_exception(func): + """This decorator wraps controllers method to manage wsme exceptions: + In case of expected error it aborts the request with specific status code. + """ + @functools.wraps(func) + def wrapped(*args, **kwargs): + try: + return func(*args, **kwargs) + except ex.MistralException as excp: + pecan.response.translatable_error = excp + raise exc.ClientSideError(msg=six.text_type(excp), + status_code=excp.http_code) + return wrapped + + +def wrap_pecan_controller_exception(func): + """This decorator wraps controllers method to manage pecan exceptions: + In case of expected error it aborts the request with specific status code. + """ + @functools.wraps(func) + def wrapped(*args, **kwargs): + try: + return func(*args, **kwargs) + except ex.MistralException as excp: + pecan.response.translatable_error = excp + pecan.abort(excp.http_code, six.text_type(excp)) + return wrapped