diff --git a/mistral/api/controllers/v2/action.py b/mistral/api/controllers/v2/action.py index 09b219b4b..0f94ef815 100644 --- a/mistral/api/controllers/v2/action.py +++ b/mistral/api/controllers/v2/action.py @@ -82,12 +82,7 @@ class ActionsController(rest.RestController, hooks.HookController): LOG.debug("Update action(s) [definition=%s]", definition) scope = pecan.request.GET.get('scope', 'private') - - if scope not in resources.SCOPE_TYPES.values: - raise exc.InvalidModelException( - "Scope must be one of the following: %s; actual: " - "%s" % (resources.SCOPE_TYPES.values, scope) - ) + resources.Action.validate_scope(scope) @rest_utils.rest_retry_on_db_error def _update_actions(): @@ -120,11 +115,7 @@ class ActionsController(rest.RestController, hooks.HookController): scope = pecan.request.GET.get('scope', 'private') pecan.response.status = 201 - if scope not in resources.SCOPE_TYPES.values: - raise exc.InvalidModelException( - "Scope must be one of the following: %s; actual: " - "%s" % (resources.SCOPE_TYPES.values, scope) - ) + resources.Action.validate_scope(scope) LOG.debug("Create action(s) [definition=%s]", definition) diff --git a/mistral/api/controllers/v2/resources.py b/mistral/api/controllers/v2/resources.py index aed11b5d8..c1166b41a 100644 --- a/mistral/api/controllers/v2/resources.py +++ b/mistral/api/controllers/v2/resources.py @@ -18,13 +18,25 @@ from wsme import types as wtypes from mistral.api.controllers import resource from mistral.api.controllers.v2 import types +from mistral import exceptions as exc from mistral import utils from mistral.workflow import states SCOPE_TYPES = wtypes.Enum(str, 'private', 'public') -class Workbook(resource.Resource): +class ScopedResource(object): + """Utilities for scoped resources""" + @classmethod + def validate_scope(cls, scope): + if scope not in SCOPE_TYPES.values: + raise exc.InvalidModelException( + "Scope must be one of the following: %s; actual: " + "%s" % (SCOPE_TYPES.values, scope) + ) + + +class Workbook(resource.Resource, ScopedResource): """Workbook resource.""" id = wtypes.text @@ -68,7 +80,7 @@ class Workbooks(resource.ResourceList): return cls(workbooks=[Workbook.sample()]) -class Workflow(resource.Resource): +class Workflow(resource.Resource, ScopedResource): """Workflow resource.""" id = wtypes.text @@ -169,7 +181,7 @@ class Workflows(resource.ResourceList): return workflows_sample -class Action(resource.Resource): +class Action(resource.Resource, ScopedResource): """Action resource. NOTE: *name* is immutable. Note that name and description get inferred diff --git a/mistral/api/controllers/v2/workbook.py b/mistral/api/controllers/v2/workbook.py index 8e795c259..a3e30c64c 100644 --- a/mistral/api/controllers/v2/workbook.py +++ b/mistral/api/controllers/v2/workbook.py @@ -66,12 +66,15 @@ class WorkbooksController(rest.RestController, hooks.HookController): acl.enforce('workbooks:update', context.ctx()) definition = pecan.request.text + scope = pecan.request.GET.get('scope', 'private') + + resources.Workbook.validate_scope(scope) LOG.debug("Update workbook [definition=%s]", definition) wb_db = rest_utils.rest_retry_on_db_error( workbooks.update_workbook_v2 - )(definition) + )(definition, scope=scope) return resources.Workbook.from_db_model(wb_db).to_json() @@ -82,12 +85,15 @@ class WorkbooksController(rest.RestController, hooks.HookController): acl.enforce('workbooks:create', context.ctx()) definition = pecan.request.text + scope = pecan.request.GET.get('scope', 'private') + + resources.Workbook.validate_scope(scope) LOG.debug("Create workbook [definition=%s]", definition) wb_db = rest_utils.rest_retry_on_db_error( workbooks.create_workbook_v2 - )(definition) + )(definition, scope=scope) pecan.response.status = 201 diff --git a/mistral/api/controllers/v2/workflow.py b/mistral/api/controllers/v2/workflow.py index 726a90af6..0186367f9 100644 --- a/mistral/api/controllers/v2/workflow.py +++ b/mistral/api/controllers/v2/workflow.py @@ -116,11 +116,7 @@ class WorkflowsController(rest.RestController, hooks.HookController): definition = pecan.request.text scope = pecan.request.GET.get('scope', 'private') - if scope not in resources.SCOPE_TYPES.values: - raise exc.InvalidModelException( - "Scope must be one of the following: %s; actual: " - "%s" % (resources.SCOPE_TYPES.values, scope) - ) + resources.Workflow.validate_scope(scope) LOG.debug("Update workflow(s) [definition=%s]", definition) @@ -156,11 +152,7 @@ class WorkflowsController(rest.RestController, hooks.HookController): scope = pecan.request.GET.get('scope', 'private') pecan.response.status = 201 - if scope not in resources.SCOPE_TYPES.values: - raise exc.InvalidModelException( - "Scope must be one of the following: %s; actual: " - "%s" % (resources.SCOPE_TYPES.values, scope) - ) + resources.Workflow.validate_scope(scope) LOG.debug("Create workflow(s) [definition=%s]", definition) diff --git a/mistral/tests/unit/api/v2/test_workbooks.py b/mistral/tests/unit/api/v2/test_workbooks.py index b1e9e301d..c961fd456 100644 --- a/mistral/tests/unit/api/v2/test_workbooks.py +++ b/mistral/tests/unit/api/v2/test_workbooks.py @@ -14,7 +14,6 @@ # limitations under the License. import copy -import datetime import mock import sqlalchemy as sa @@ -27,29 +26,45 @@ from mistral.tests.unit.api import base WORKBOOK_DEF = """ --- -version: 2.0 +version: '2.0' name: 'test' + +workflows: + flow: + type: direct + tasks: + task1: + action: step param="Hello !" + +actions: + step: + input: + - param + base: std.echo output="<% $.param %>"" """ UPDATED_WORKBOOK_DEF = """ --- -version: 2.0 +version: '2.0' name: 'book' -""" -WORKBOOK_DB = models.Workbook( - id='123', - name='book', - definition=WORKBOOK_DEF, - tags=['deployment', 'demo'], - scope="public", - created_at=datetime.datetime(1970, 1, 1), - updated_at=datetime.datetime(1970, 1, 1) -) +workflows: + flow: + type: direct + tasks: + task1: + action: step arg="Hello !" + +actions: + step: + input: + - param + base: std.echo output="<% $.param %>"" +""" WORKBOOK = { 'id': '123', - 'name': 'book', + 'name': 'test', 'definition': WORKBOOK_DEF, 'tags': ['deployment', 'demo'], 'scope': 'public', @@ -57,6 +72,32 @@ WORKBOOK = { 'updated_at': '1970-01-01 00:00:00' } +ACTION = { + 'id': '123e4567-e89b-12d3-a456-426655440000', + 'name': 'step', + 'is_system': False, + 'description': 'My super cool action.', + 'tags': ['test', 'v2'], + 'definition': '' +} + +WF = { + 'id': '123e4567-e89b-12d3-a456-426655440000', + 'name': 'flow', + 'definition': '', + 'created_at': '1970-01-01 00:00:00', + 'updated_at': '1970-01-01 00:00:00', +} + +ACTION_DB = models.ActionDefinition() +ACTION_DB.update(ACTION) + +WORKBOOK_DB = models.Workbook() +WORKBOOK_DB.update(WORKBOOK) + +WF_DB = models.WorkflowDefinition() +WF_DB.update(WF) + WORKBOOK_DB_PROJECT_ID = WORKBOOK_DB.get_clone() WORKBOOK_DB_PROJECT_ID.project_id = '' @@ -174,6 +215,38 @@ class TestWorkbooksController(base.APITest): self.assertEqual(400, resp.status_int) self.assertIn("Invalid DSL", resp.body.decode()) + @mock.patch.object(db_api, "update_workbook") + @mock.patch.object(db_api, "create_or_update_workflow_definition") + @mock.patch.object(db_api, "create_or_update_action_definition") + def test_put_public(self, mock_action, mock_wf, mock_wb): + mock_wb.return_value = UPDATED_WORKBOOK_DB + mock_wf.return_value = WF_DB + mock_action.return_value = ACTION_DB + + resp = self.app.put( + '/v2/workbooks?scope=public', + UPDATED_WORKBOOK_DEF, + headers={'Content-Type': 'text/plain'} + ) + + self.assertEqual(200, resp.status_int) + self.assertDictEqual(UPDATED_WORKBOOK, resp.json) + + self.assertEqual("public", mock_wb.call_args[0][1]['scope']) + self.assertEqual("public", mock_wf.call_args[0][1]['scope']) + self.assertEqual("public", mock_action.call_args[0][1]['scope']) + + def test_put_wrong_scope(self): + resp = self.app.put( + '/v2/workbooks?scope=unique', + UPDATED_WORKBOOK_DEF, + headers={'Content-Type': 'text/plain'}, + expect_errors=True + ) + + self.assertEqual(400, resp.status_int) + self.assertIn("Scope must be one of the following", resp.body.decode()) + @mock.patch.object(workbooks, "create_workbook_v2", MOCK_WORKBOOK) def test_post(self): resp = self.app.post( @@ -207,6 +280,38 @@ class TestWorkbooksController(base.APITest): self.assertEqual(400, resp.status_int) self.assertIn("Invalid DSL", resp.body.decode()) + @mock.patch.object(db_api, "create_workbook") + @mock.patch.object(db_api, "create_or_update_workflow_definition") + @mock.patch.object(db_api, "create_or_update_action_definition") + def test_post_public(self, mock_action, mock_wf, mock_wb): + mock_wb.return_value = WORKBOOK_DB + mock_wf.return_value = WF_DB + mock_action.return_value = ACTION_DB + + resp = self.app.post( + '/v2/workbooks?scope=public', + WORKBOOK_DEF, + headers={'Content-Type': 'text/plain'} + ) + + self.assertEqual(201, resp.status_int) + self.assertEqual(WORKBOOK, resp.json) + + self.assertEqual("public", mock_wb.call_args[0][0]['scope']) + self.assertEqual("public", mock_wf.call_args[0][1]['scope']) + self.assertEqual("public", mock_action.call_args[0][1]['scope']) + + def test_post_wrong_scope(self): + resp = self.app.post( + '/v2/workbooks?scope=unique', + WORKBOOK_DEF, + headers={'Content-Type': 'text/plain'}, + expect_errors=True + ) + + self.assertEqual(400, resp.status_int) + self.assertIn("Scope must be one of the following", resp.body.decode()) + @mock.patch.object(db_api, "delete_workbook", MOCK_DELETE) def test_delete(self): resp = self.app.delete('/v2/workbooks/123') diff --git a/mistral/tests/unit/api/v2/test_workflows.py b/mistral/tests/unit/api/v2/test_workflows.py index 66c980dbd..dc744c833 100644 --- a/mistral/tests/unit/api/v2/test_workflows.py +++ b/mistral/tests/unit/api/v2/test_workflows.py @@ -293,6 +293,17 @@ class TestWorkflowsController(base.APITest): self.assertEqual("public", mock_update.call_args[0][1]['scope']) + def test_put_wrong_scope(self): + resp = self.app.put( + '/v2/workflows?scope=unique', + UPDATED_WF_DEFINITION, + headers={'Content-Type': 'text/plain'}, + expect_errors=True + ) + + self.assertEqual(400, resp.status_int) + self.assertIn("Scope must be one of the following", resp.body.decode()) + @mock.patch.object( db_api, "update_workflow_definition", MOCK_WF_WITH_INPUT ) @@ -381,10 +392,7 @@ class TestWorkflowsController(base.APITest): self.assertEqual("public", mock_mtd.call_args[0][0]['scope']) - @mock.patch.object(db_api, "create_action_definition") - def test_post_wrong_scope(self, mock_mtd): - mock_mtd.return_value = WF_DB - + def test_post_wrong_scope(self): resp = self.app.post( '/v2/workflows?scope=unique', WF_DEFINITION,