openstack/deb-mistral
Code Issues Proposed changes

Allow to use both name and id to access action definitions

Browse Source 
- Support name and id to access via GET /v2/actions/
- Support name and id to access via DELETE /v2/actions/

TODO:
- Make other apis of database layer consistent with
  workflow and action api.
- Support name and id to access via PUT /v2/actions/

Change-Id: I8ce55c0eb1cab7581ba719fbbb6be7b7dced27e0
Implements: blueprint mistral-actions-identifier
Closes-Bug: #1584643
This commit is contained in:
Marcos Fermin Lobo 2016-06-06 14:55:58 +02:00 committed by hardik
parent 242cb41008
commit 0d8a3a8c64
3 changed files with 83 additions and 51 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
mistral/api/controllers/v2/action.py
View File

@ -102,12 +102,16 @@ class ActionsController(rest.RestController, hooks.HookController):
@rest_utils.wrap_wsme_controller_exception @rest_utils.wrap_wsme_controller_exception
@wsme_pecan.wsexpose(Action, wtypes.text) @wsme_pecan.wsexpose(Action, wtypes.text)
def get(self, name): def get(self, identifier):
"""Return the named action.""" """Return the named action.
acl.enforce('actions:get', context.ctx())
LOG.info("Fetch action [name=%s]" % name)
db_model = db_api.get_action_definition(name) :param identifier: ID or name of the Action to get.
"""
acl.enforce('actions:get', context.ctx())
LOG.info("Fetch action [identifier=%s]" % identifier)
db_model = db_api.get_action_definition(identifier)
return Action.from_dict(db_model.to_dict()) return Action.from_dict(db_model.to_dict())
@ -169,19 +173,19 @@ class ActionsController(rest.RestController, hooks.HookController):
@rest_utils.wrap_wsme_controller_exception @rest_utils.wrap_wsme_controller_exception
@wsme_pecan.wsexpose(None, wtypes.text, status_code=204) @wsme_pecan.wsexpose(None, wtypes.text, status_code=204)
def delete(self, name): def delete(self, identifier):
"""Delete the named action.""" """Delete the named action."""
acl.enforce('actions:delete', context.ctx()) acl.enforce('actions:delete', context.ctx())
LOG.info("Delete action [name=%s]" % name) LOG.info("Delete action [identifier=%s]" % identifier)
with db_api.transaction(): with db_api.transaction():
db_model = db_api.get_action_definition(name) db_model = db_api.get_action_definition(identifier)
if db_model.is_system: if db_model.is_system:
msg = "Attempt to delete a system action: %s" % name msg = "Attempt to delete a system action: %s" % identifier
raise exc.DataAccessException(msg) raise exc.DataAccessException(msg)
db_api.delete_action_definition(name) db_api.delete_action_definition(identifier)
@wsme_pecan.wsexpose(Actions, types.uuid, int, types.uniquelist, @wsme_pecan.wsexpose(Actions, types.uuid, int, types.uniquelist,
types.list, types.uniquelist, wtypes.text, types.list, types.uniquelist, wtypes.text,

63
mistral/db/v2/sqlalchemy/api.py
View File

@ -21,7 +21,7 @@ from oslo_db import exception as db_exc
from oslo_db import sqlalchemy as oslo_sqlalchemy from oslo_db import sqlalchemy as oslo_sqlalchemy
from oslo_db.sqlalchemy import utils as db_utils from oslo_db.sqlalchemy import utils as db_utils
from oslo_log import log as logging from oslo_log import log as logging
from oslo_utils import uuidutils from oslo_utils import uuidutils # noqa
import sqlalchemy as sa import sqlalchemy as sa
from mistral.db.sqlalchemy import base as b from mistral.db.sqlalchemy import base as b
@ -224,6 +224,18 @@ def _get_db_object_by_id(model, id):
return _secure_query(model).filter_by(id=id).first() return _secure_query(model).filter_by(id=id).first()
def _get_db_object_by_name_or_id(model, identifier):
query = _secure_query(model)
query = query.filter(
sa.or_(
model.id == identifier,
model.name == identifier
)
)
return query.first()
# Workbook definitions. # Workbook definitions.
def get_workbook(name): def get_workbook(name):
@ -313,9 +325,10 @@ def get_workflow_definition(identifier):
uuid. uuid.
:return: Workflow definition. :return: Workflow definition.
""" """
wf_def = (_get_workflow_definition_by_id(identifier) wf_def = _get_db_object_by_name_or_id(
if uuidutils.is_uuid_like(identifier) models.WorkflowDefinition,
else _get_workflow_definition(identifier)) identifier
)
if not wf_def: if not wf_def:
raise exc.DBEntityNotFoundError( raise exc.DBEntityNotFoundError(
@ -326,7 +339,7 @@ def get_workflow_definition(identifier):
def get_workflow_definition_by_id(id): def get_workflow_definition_by_id(id):
wf_def = _get_workflow_definition_by_id(id) wf_def = _get_db_object_by_id(models.WorkflowDefinition, id)
if not wf_def: if not wf_def:
raise exc.DBEntityNotFoundError( raise exc.DBEntityNotFoundError(
@ -337,7 +350,7 @@ def get_workflow_definition_by_id(id):
def load_workflow_definition(name): def load_workflow_definition(name):
return _get_workflow_definition(name) return _get_db_object_by_name(models.WorkflowDefinition, name)
def get_workflow_definitions(sort_keys=['created_at'], fields=None, **kwargs): def get_workflow_definitions(sort_keys=['created_at'], fields=None, **kwargs):
@ -403,7 +416,7 @@ def update_workflow_definition(identifier, values, session=None):
@b.session_aware() @b.session_aware()
def create_or_update_workflow_definition(name, values, session=None): def create_or_update_workflow_definition(name, values, session=None):
if not _get_workflow_definition(name): if not _get_db_object_by_name(models.WorkflowDefinition, name):
return create_workflow_definition(values) return create_workflow_definition(values)
else: else:
return update_workflow_definition(name, values) return update_workflow_definition(name, values)
@ -458,14 +471,6 @@ def delete_workflow_definitions(**kwargs):
return _delete_all(models.WorkflowDefinition, **kwargs) return _delete_all(models.WorkflowDefinition, **kwargs)
def _get_workflow_definition(name):
return _get_db_object_by_name(models.WorkflowDefinition, name)
def _get_workflow_definition_by_id(id):
return _get_db_object_by_id(models.WorkflowDefinition, id)
# Action definitions. # Action definitions.
def get_action_definition_by_id(id): def get_action_definition_by_id(id):
@ -479,19 +484,22 @@ def get_action_definition_by_id(id):
return action_def return action_def
def get_action_definition(name): def get_action_definition(identifier):
a_def = _get_action_definition(name) a_def = _get_db_object_by_name_or_id(
models.ActionDefinition,
identifier
)
if not a_def: if not a_def:
raise exc.DBEntityNotFoundError( raise exc.DBEntityNotFoundError(
"Action definition not found [action_name=%s]" % name "Action definition not found [action_name=%s]" % identifier
) )
return a_def return a_def
def load_action_definition(name): def load_action_definition(name):
return _get_action_definition(name) return _get_db_object_by_name(models.ActionDefinition, name)
def get_action_definitions(sort_keys=['name'], **kwargs): def get_action_definitions(sort_keys=['name'], **kwargs):
@ -520,7 +528,7 @@ def create_action_definition(values, session=None):
@b.session_aware() @b.session_aware()
def update_action_definition(name, values, session=None): def update_action_definition(name, values, session=None):
a_def = _get_action_definition(name) a_def = _get_db_object_by_name(models.ActionDefinition, name)
if not a_def: if not a_def:
raise exc.DBEntityNotFoundError( raise exc.DBEntityNotFoundError(
@ -534,20 +542,15 @@ def update_action_definition(name, values, session=None):
@b.session_aware() @b.session_aware()
def create_or_update_action_definition(name, values, session=None): def create_or_update_action_definition(name, values, session=None):
if not _get_action_definition(name): if not _get_db_object_by_name(models.ActionDefinition, name):
return create_action_definition(values) return create_action_definition(values)
else: else:
return update_action_definition(name, values) return update_action_definition(name, values)
@b.session_aware() @b.session_aware()
def delete_action_definition(name, session=None): def delete_action_definition(identifier, session=None):
a_def = _get_action_definition(name) a_def = get_action_definition(identifier)
if not a_def:
raise exc.DBEntityNotFoundError(
"Action definition not found [action_name=%s]" % name
)
session.delete(a_def) session.delete(a_def)
@ -557,10 +560,6 @@ def delete_action_definitions(**kwargs):
return _delete_all(models.ActionDefinition, **kwargs) return _delete_all(models.ActionDefinition, **kwargs)
def _get_action_definition(name):
return _get_db_object_by_name(models.ActionDefinition, name)
# Common executions. # Common executions.
def get_execution(id): def get_execution(id):

47
mistral/tests/unit/api/v2/test_actions.py
View File

@ -124,21 +124,44 @@ MOCK_DUPLICATE = mock.MagicMock(side_effect=exc.DBDuplicateEntryError())
class TestActionsController(base.APITest): class TestActionsController(base.APITest):
@mock.patch.object(db_api, "get_action_definition", MOCK_ACTION) @mock.patch.object(
db_api, "get_action_definition", MOCK_ACTION)
def test_get(self): def test_get(self):
resp = self.app.get('/v2/actions/my_action') resp = self.app.get('/v2/actions/my_action')
self.assertEqual(200, resp.status_int) self.assertEqual(200, resp.status_int)
self.assertDictEqual(ACTION, resp.json) self.assertDictEqual(ACTION, resp.json)
@mock.patch.object(db_api, "get_action_definition", MOCK_NOT_FOUND) @mock.patch.object(
db_api, "get_action_definition", MOCK_NOT_FOUND)
def test_get_not_found(self): def test_get_not_found(self):
resp = self.app.get('/v2/actions/my_action', expect_errors=True) resp = self.app.get('/v2/actions/my_action', expect_errors=True)
self.assertEqual(404, resp.status_int) self.assertEqual(404, resp.status_int)
@mock.patch.object(db_api, "get_action_definition", MOCK_ACTION)
@mock.patch.object(db_api, "update_action_definition", MOCK_UPDATED_ACTION) @mock.patch.object(db_api, "update_action_definition", MOCK_UPDATED_ACTION)
@mock.patch.object(
db_api, "get_action_definition", MOCK_ACTION)
def test_get_by_id(self):
url = '/v2/actions/{0}'.format(ACTION['id'])
resp = self.app.get(url)
self.assertEqual(200, resp.status_int)
self.assertEqual(ACTION['id'], resp.json['id'])
@mock.patch.object(
db_api, "get_action_definition", MOCK_NOT_FOUND)
def test_get_by_id_not_found(self):
url = '/v2/actions/1234'
resp = self.app.get(url, expect_errors=True)
self.assertEqual(404, resp.status_int)
@mock.patch.object(
db_api, "get_action_definition", MOCK_ACTION)
@mock.patch.object(
db_api, "update_action_definition", MOCK_UPDATED_ACTION
)
def test_put(self): def test_put(self):
resp = self.app.put( resp = self.app.put(
'/v2/actions', '/v2/actions',
@ -178,7 +201,8 @@ class TestActionsController(base.APITest):
self.assertEqual(404, resp.status_int) self.assertEqual(404, resp.status_int)
@mock.patch.object(db_api, "get_action_definition", MOCK_SYSTEM_ACTION) @mock.patch.object(
db_api, "get_action_definition", MOCK_SYSTEM_ACTION)
def test_put_system(self): def test_put_system(self):
resp = self.app.put( resp = self.app.put(
'/v2/actions', '/v2/actions',
@ -257,7 +281,8 @@ class TestActionsController(base.APITest):
self.assertEqual(409, resp.status_int) self.assertEqual(409, resp.status_int)
@mock.patch.object(db_api, "get_action_definition", MOCK_ACTION) @mock.patch.object(
db_api, "get_action_definition", MOCK_ACTION)
@mock.patch.object(db_api, "delete_action_definition", MOCK_DELETE) @mock.patch.object(db_api, "delete_action_definition", MOCK_DELETE)
def test_delete(self): def test_delete(self):
resp = self.app.delete('/v2/actions/my_action') resp = self.app.delete('/v2/actions/my_action')
@ -270,7 +295,8 @@ class TestActionsController(base.APITest):
self.assertEqual(404, resp.status_int) self.assertEqual(404, resp.status_int)
@mock.patch.object(db_api, "get_action_definition", MOCK_SYSTEM_ACTION) @mock.patch.object(
db_api, "get_action_definition", MOCK_SYSTEM_ACTION)
def test_delete_system(self): def test_delete_system(self):
resp = self.app.delete('/v2/actions/std.echo', expect_errors=True) resp = self.app.delete('/v2/actions/std.echo', expect_errors=True)
@ -278,7 +304,8 @@ class TestActionsController(base.APITest):
self.assertIn('Attempt to delete a system action: std.echo', self.assertIn('Attempt to delete a system action: std.echo',
resp.json['faultstring']) resp.json['faultstring'])
@mock.patch.object(db_api, "get_action_definitions", MOCK_ACTIONS) @mock.patch.object(
db_api, "get_action_definitions", MOCK_ACTIONS)
def test_get_all(self): def test_get_all(self):
resp = self.app.get('/v2/actions') resp = self.app.get('/v2/actions')
@ -287,7 +314,8 @@ class TestActionsController(base.APITest):
self.assertEqual(1, len(resp.json['actions'])) self.assertEqual(1, len(resp.json['actions']))
self.assertDictEqual(ACTION, resp.json['actions'][0]) self.assertDictEqual(ACTION, resp.json['actions'][0])
@mock.patch.object(db_api, "get_action_definitions", MOCK_EMPTY) @mock.patch.object(
db_api, "get_action_definitions", MOCK_EMPTY)
def test_get_all_empty(self): def test_get_all_empty(self):
resp = self.app.get('/v2/actions') resp = self.app.get('/v2/actions')
@ -295,7 +323,8 @@ class TestActionsController(base.APITest):
self.assertEqual(0, len(resp.json['actions'])) self.assertEqual(0, len(resp.json['actions']))
@mock.patch.object(db_api, "get_action_definitions", MOCK_ACTIONS) @mock.patch.object(
db_api, "get_action_definitions", MOCK_ACTIONS)
def test_get_all_pagination(self): def test_get_all_pagination(self):
resp = self.app.get( resp = self.app.get(
'/v2/actions?limit=1&sort_keys=id,name') '/v2/actions?limit=1&sort_keys=id,name')