Merge "Add missing query paramater `scope` to the workbook api"

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')
-
+        resources.Action.validate_scope(scope)
-        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)
-            )
 
         @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:
+        resources.Action.validate_scope(scope)
-            raise exc.InvalidModelException(
-                "Scope must be one of the following: %s; actual: "
-                "%s" % (resources.SCOPE_TYPES.values, scope)
-            )
 
         LOG.debug("Create action(s) [definition=%s]", definition)
 

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

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
 

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:
+        resources.Workflow.validate_scope(scope)
-            raise exc.InvalidModelException(
-                "Scope must be one of the following: %s; actual: "
-                "%s" % (resources.SCOPE_TYPES.values, 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:
+        resources.Workflow.validate_scope(scope)
-            raise exc.InvalidModelException(
-                "Scope must be one of the following: %s; actual: "
-                "%s" % (resources.SCOPE_TYPES.values, scope)
-            )
 
         LOG.debug("Create workflow(s) [definition=%s]", definition)
 

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(
+workflows:
-    id='123',
+  flow:
-    name='book',
+    type: direct
-    definition=WORKBOOK_DEF,
+    tasks:
-    tags=['deployment', 'demo'],
+      task1:
-    scope="public",
+        action: step arg="Hello !"
-    created_at=datetime.datetime(1970, 1, 1),
+
-    updated_at=datetime.datetime(1970, 1, 1)
+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 = '<default-project>'
 
@@ -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')

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):
-    def test_post_wrong_scope(self, mock_mtd):
-        mock_mtd.return_value = WF_DB
-
         resp = self.app.post(
             '/v2/workflows?scope=unique',
             WF_DEFINITION,