diff --git a/mistral/api/controllers/resource.py b/mistral/api/controllers/resource.py index c9897c95b..0330fba37 100644 --- a/mistral/api/controllers/resource.py +++ b/mistral/api/controllers/resource.py @@ -16,6 +16,8 @@ import json from wsme import types as wtypes +from mistral import utils + class Resource(wtypes.Base): """REST API Resource.""" @@ -39,15 +41,21 @@ class Resource(wtypes.Base): for key, val in d.items(): if hasattr(obj, key): - setattr(obj, key, val) + # Convert all datetime values to strings. + setattr(obj, key, utils.datetime_to_str(val)) return obj @classmethod - def from_db_model(cls): - # TODO(rakhmerov): Once we implement this method, - # this will significantly reduce memory footprint. - raise NotImplementedError + def from_db_model(cls, db_model): + obj = cls() + + for col_name, col_val in db_model.iter_columns(): + if hasattr(obj, col_name): + # Convert all datetime values to strings. + setattr(obj, col_name, utils.datetime_to_str(col_val)) + + return obj def __str__(self): """WSME based implementation of __str__.""" diff --git a/mistral/api/controllers/v2/action.py b/mistral/api/controllers/v2/action.py index b1e6b837a..ebbd442c1 100644 --- a/mistral/api/controllers/v2/action.py +++ b/mistral/api/controllers/v2/action.py @@ -54,11 +54,12 @@ class ActionsController(rest.RestController, hooks.HookController): """ acl.enforce('actions:get', context.ctx()) + LOG.info("Fetch action [identifier=%s]", identifier) db_model = db_api.get_action_definition(identifier) - return resources.Action.from_dict(db_model.to_dict()) + return resources.Action.from_db_model(db_model) @rest_utils.wrap_pecan_controller_exception @pecan.expose(content_type="text/plain") @@ -69,8 +70,11 @@ class ActionsController(rest.RestController, hooks.HookController): of multiple actions. In this case they all will be updated. """ acl.enforce('actions:update', context.ctx()) + definition = pecan.request.text + LOG.info("Update action(s) [definition=%s]", definition) + scope = pecan.request.GET.get('scope', 'private') if scope not in resources.SCOPE_TYPES.values: @@ -86,8 +90,9 @@ class ActionsController(rest.RestController, hooks.HookController): identifier=identifier ) - models_dicts = [db_act.to_dict() for db_act in db_acts] - action_list = [resources.Action.from_dict(act) for act in models_dicts] + action_list = [ + resources.Action.from_db_model(db_act) for db_act in db_acts + ] return resources.Actions(actions=action_list).to_json() @@ -100,6 +105,7 @@ class ActionsController(rest.RestController, hooks.HookController): of multiple actions. In this case they all will be created. """ acl.enforce('actions:create', context.ctx()) + definition = pecan.request.text scope = pecan.request.GET.get('scope', 'private') pecan.response.status = 201 @@ -115,8 +121,9 @@ class ActionsController(rest.RestController, hooks.HookController): with db_api.transaction(): db_acts = actions.create_actions(definition, scope=scope) - models_dicts = [db_act.to_dict() for db_act in db_acts] - action_list = [resources.Action.from_dict(act) for act in models_dicts] + action_list = [ + resources.Action.from_db_model(db_act) for db_act in db_acts + ] return resources.Actions(actions=action_list).to_json() @@ -125,6 +132,7 @@ class ActionsController(rest.RestController, hooks.HookController): def delete(self, identifier): """Delete the named action.""" acl.enforce('actions:delete', context.ctx()) + LOG.info("Delete action [identifier=%s]", identifier) with db_api.transaction(): diff --git a/mistral/api/controllers/v2/action_execution.py b/mistral/api/controllers/v2/action_execution.py index 70633beef..081dc9928 100644 --- a/mistral/api/controllers/v2/action_execution.py +++ b/mistral/api/controllers/v2/action_execution.py @@ -56,7 +56,7 @@ def _get_action_execution_resource_for_list(action_ex): # TODO(nmakhotkin): Get rid of using dicts for constructing resources. # TODO(nmakhotkin): Use db_model for this instead. - res = resources.ActionExecution.from_dict(action_ex.to_dict()) + res = resources.ActionExecution.from_db_model(action_ex) task_name = (action_ex.task_execution.name if action_ex.task_execution else None) @@ -142,14 +142,14 @@ class ActionExecutionsController(rest.RestController): "Please provide at least action name to run action." ) - action_ex = rpc.get_engine_client().start_action( + values = rpc.get_engine_client().start_action( name, action_input, description=description, **params ) - return resources.ActionExecution.from_dict(action_ex) + return resources.ActionExecution.from_dict(values) @rest_utils.wrap_wsme_controller_exception @wsme_pecan.wsexpose( diff --git a/mistral/api/controllers/v2/cron_trigger.py b/mistral/api/controllers/v2/cron_trigger.py index ef0219a2b..415bd7ce0 100644 --- a/mistral/api/controllers/v2/cron_trigger.py +++ b/mistral/api/controllers/v2/cron_trigger.py @@ -40,7 +40,7 @@ class CronTriggersController(rest.RestController): db_model = db_api.get_cron_trigger(name) - return resources.CronTrigger.from_dict(db_model.to_dict()) + return resources.CronTrigger.from_db_model(db_model) @rest_utils.wrap_wsme_controller_exception @wsme_pecan.wsexpose( @@ -71,7 +71,7 @@ class CronTriggersController(rest.RestController): workflow_id=values.get('workflow_id') ) - return resources.CronTrigger.from_dict(db_model.to_dict()) + return resources.CronTrigger.from_db_model(db_model) @rest_utils.wrap_wsme_controller_exception @wsme_pecan.wsexpose(None, wtypes.text, status_code=204) diff --git a/mistral/api/controllers/v2/environment.py b/mistral/api/controllers/v2/environment.py index 358c19a7d..29ea1cbdb 100644 --- a/mistral/api/controllers/v2/environment.py +++ b/mistral/api/controllers/v2/environment.py @@ -109,7 +109,7 @@ class EnvironmentController(rest.RestController): db_model = db_api.get_environment(name) - return resources.Environment.from_dict(db_model.to_dict()) + return resources.Environment.from_db_model(db_model) @rest_utils.wrap_wsme_controller_exception @wsme_pecan.wsexpose( @@ -130,7 +130,7 @@ class EnvironmentController(rest.RestController): db_model = db_api.create_environment(env.to_dict()) - return resources.Environment.from_dict(db_model.to_dict()) + return resources.Environment.from_db_model(db_model) @rest_utils.wrap_wsme_controller_exception @wsme_pecan.wsexpose(resources.Environment, body=resources.Environment) @@ -155,7 +155,7 @@ class EnvironmentController(rest.RestController): db_model = db_api.update_environment(env.name, env.to_dict()) - return resources.Environment.from_dict(db_model.to_dict()) + return resources.Environment.from_db_model(db_model) @rest_utils.wrap_wsme_controller_exception @wsme_pecan.wsexpose(None, wtypes.text, status_code=204) diff --git a/mistral/api/controllers/v2/event_trigger.py b/mistral/api/controllers/v2/event_trigger.py index 607414a61..ee8286867 100644 --- a/mistral/api/controllers/v2/event_trigger.py +++ b/mistral/api/controllers/v2/event_trigger.py @@ -43,7 +43,7 @@ class EventTriggersController(rest.RestController): db_model = db_api.get_event_trigger(id) - return resources.EventTrigger.from_dict(db_model.to_dict()) + return resources.EventTrigger.from_db_model(db_model) @rest_utils.wrap_wsme_controller_exception @wsme_pecan.wsexpose(resources.EventTrigger, body=resources.EventTrigger, @@ -73,7 +73,7 @@ class EventTriggersController(rest.RestController): workflow_params=values.get('workflow_params'), ) - return resources.EventTrigger.from_dict(db_model.to_dict()) + return resources.EventTrigger.from_db_model(db_model) @rest_utils.wrap_wsme_controller_exception @wsme_pecan.wsexpose(resources.EventTrigger, types.uuid, @@ -103,7 +103,7 @@ class EventTriggersController(rest.RestController): db_model = triggers.update_event_trigger(id, values) - return resources.EventTrigger.from_dict(db_model.to_dict()) + return resources.EventTrigger.from_db_model(db_model) @rest_utils.wrap_wsme_controller_exception @wsme_pecan.wsexpose(None, types.uuid, status_code=204) diff --git a/mistral/api/controllers/v2/execution.py b/mistral/api/controllers/v2/execution.py index 035803404..aa555ceeb 100644 --- a/mistral/api/controllers/v2/execution.py +++ b/mistral/api/controllers/v2/execution.py @@ -47,12 +47,12 @@ STATE_TYPES = wtypes.Enum( ) -def _get_execution_resource(ex): +def _get_execution_resource(wf_ex): # We need to refer to this lazy-load field explicitly in # order to make sure that it is correctly loaded. - hasattr(ex, 'output') + hasattr(wf_ex, 'output') - return resources.Execution.from_dict(ex.to_dict()) + return resources.Execution.from_db_model(wf_ex) # TODO(rakhmerov): Make sure to make all needed renaming on public API. @@ -77,7 +77,7 @@ class ExecutionsController(rest.RestController): # amount of DB queries and network traffic. hasattr(wf_ex, 'output') - return resources.Execution.from_dict(wf_ex.to_dict()) + return resources.Execution.from_db_model(wf_ex) @rest_utils.wrap_wsme_controller_exception @wsme_pecan.wsexpose( diff --git a/mistral/api/controllers/v2/member.py b/mistral/api/controllers/v2/member.py index b71b70d84..1822b380f 100644 --- a/mistral/api/controllers/v2/member.py +++ b/mistral/api/controllers/v2/member.py @@ -66,13 +66,13 @@ class MembersController(rest.RestController): member_id ) - member_dict = db_api.get_resource_member( + member_db = db_api.get_resource_member( self.resource_id, self.type, member_id - ).to_dict() + ) - return resources.Member.from_dict(member_dict) + return resources.Member.from_db_model(member_db) @rest_utils.wrap_pecan_controller_exception @auth_enable_check @@ -91,9 +91,10 @@ class MembersController(rest.RestController): self.resource_id, self.type ) + members = [ - resources.Member.from_dict(member.to_dict()) - for member in db_members + resources.Member.from_db_model(db_member) + for db_member in db_members ] return resources.Members(members=members) @@ -118,15 +119,15 @@ class MembersController(rest.RestController): ) if not member_info.member_id: - msg = "Member id must be provided." - raise exc.WorkflowException(msg) + raise exc.WorkflowException("Member id must be provided.") with db_api.transaction(): wf_db = db_api.get_workflow_definition(self.resource_id) if wf_db.scope != 'private': - msg = "Only private resource could be shared." - raise exc.WorkflowException(msg) + raise exc.WorkflowException( + "Only private resource could be shared." + ) resource_member = { 'resource_id': self.resource_id, @@ -137,7 +138,7 @@ class MembersController(rest.RestController): db_member = db_api.create_resource_member(resource_member) - return resources.Member.from_dict(db_member.to_dict()) + return resources.Member.from_db_model(db_member) @rest_utils.wrap_pecan_controller_exception @auth_enable_check @@ -165,7 +166,7 @@ class MembersController(rest.RestController): {'status': member_info.status} ) - return resources.Member.from_dict(db_member.to_dict()) + return resources.Member.from_db_model(db_member) @rest_utils.wrap_pecan_controller_exception @auth_enable_check diff --git a/mistral/api/controllers/v2/resources.py b/mistral/api/controllers/v2/resources.py index 0069b532a..22394bef4 100644 --- a/mistral/api/controllers/v2/resources.py +++ b/mistral/api/controllers/v2/resources.py @@ -95,16 +95,12 @@ class Workflow(resource.Resource): updated_at='1970-01-01T00:00:00.000000') @classmethod - def from_dict(cls, d): - e = cls() + def _set_input(cls, obj, wf_spec): input_list = [] - for key, val in d.items(): - if hasattr(e, key): - setattr(e, key, val) + if wf_spec: + input = wf_spec.get('input', []) - if 'spec' in d: - input = d.get('spec', {}).get('input', []) for param in input: if isinstance(param, dict): for k, v in param.items(): @@ -112,9 +108,21 @@ class Workflow(resource.Resource): else: input_list.append(param) - setattr(e, 'input', ", ".join(input_list) if input_list else '') + setattr(obj, 'input', ", ".join(input_list) if input_list else '') - return e + return obj + + @classmethod + def from_dict(cls, d): + obj = super(Workflow, cls).from_dict(d) + + return cls._set_input(obj, d.get('spec')) + + @classmethod + def from_db_model(cls, db_model): + obj = super(Workflow, cls).from_db_model(db_model) + + return cls._set_input(obj, db_model.spec) class Workflows(resource.ResourceList): diff --git a/mistral/api/controllers/v2/service.py b/mistral/api/controllers/v2/service.py index 9c47b2d07..727a37b55 100644 --- a/mistral/api/controllers/v2/service.py +++ b/mistral/api/controllers/v2/service.py @@ -58,10 +58,18 @@ class ServicesController(rest.RestController): try: for group in service_group: members = service_coordinator.get_members(group) - services_list.extend( - [resources.Service.from_dict( - {'type': group, 'name': member}) for member in members] - ) + + members_list = [ + resources.Service.from_dict( + { + 'type': group, + 'name': member + } + ) + for member in members + ] + + services_list.extend(members_list) except tooz.coordination.ToozError as e: # In the scenario of network interruption or manually shutdown # connection shutdown, ToozError will be raised. diff --git a/mistral/api/controllers/v2/task.py b/mistral/api/controllers/v2/task.py index 8c9e022e5..b682e897a 100644 --- a/mistral/api/controllers/v2/task.py +++ b/mistral/api/controllers/v2/task.py @@ -41,7 +41,8 @@ STATE_TYPES = wtypes.Enum(str, states.IDLE, states.RUNNING, states.SUCCESS, def _get_task_resource_with_result(task_ex): - task = resources.Task.from_dict(task_ex.to_dict()) + task = resources.Task.from_db_model(task_ex) + task.result = json.dumps(data_flow.get_task_execution_result(task_ex)) return task diff --git a/mistral/api/controllers/v2/workbook.py b/mistral/api/controllers/v2/workbook.py index bee4d9136..5f4c8d4b9 100644 --- a/mistral/api/controllers/v2/workbook.py +++ b/mistral/api/controllers/v2/workbook.py @@ -52,7 +52,7 @@ class WorkbooksController(rest.RestController, hooks.HookController): db_model = db_api.get_workbook(name) - return resources.Workbook.from_dict(db_model.to_dict()) + return resources.Workbook.from_db_model(db_model) @rest_utils.wrap_pecan_controller_exception @pecan.expose(content_type="text/plain") @@ -66,7 +66,7 @@ class WorkbooksController(rest.RestController, hooks.HookController): wb_db = workbooks.update_workbook_v2(definition) - return resources.Workbook.from_dict(wb_db.to_dict()).to_json() + return resources.Workbook.from_db_model(wb_db).to_json() @rest_utils.wrap_pecan_controller_exception @pecan.expose(content_type="text/plain") @@ -79,9 +79,10 @@ class WorkbooksController(rest.RestController, hooks.HookController): LOG.info("Create workbook [definition=%s]" % definition) wb_db = workbooks.create_workbook_v2(definition) + pecan.response.status = 201 - return resources.Workbook.from_dict(wb_db.to_dict()).to_json() + return resources.Workbook.from_db_model(wb_db).to_json() @rest_utils.wrap_wsme_controller_exception @wsme_pecan.wsexpose(None, wtypes.text, status_code=204) diff --git a/mistral/api/controllers/v2/workflow.py b/mistral/api/controllers/v2/workflow.py index c0202e883..19e04e2a3 100644 --- a/mistral/api/controllers/v2/workflow.py +++ b/mistral/api/controllers/v2/workflow.py @@ -85,7 +85,7 @@ class WorkflowsController(rest.RestController, hooks.HookController): db_model = db_api.get_workflow_definition(identifier) - return resources.Workflow.from_dict(db_model.to_dict()) + return resources.Workflow.from_db_model(db_model) @rest_utils.wrap_pecan_controller_exception @pecan.expose(content_type="text/plain") @@ -117,9 +117,8 @@ class WorkflowsController(rest.RestController, hooks.HookController): identifier=identifier ) - models_dicts = [db_wf.to_dict() for db_wf in db_wfs] workflow_list = [ - resources.Workflow.from_dict(wf) for wf in models_dicts + resources.Workflow.from_db_model(db_wf) for db_wf in db_wfs ] return (workflow_list[0].to_json() if identifier @@ -148,10 +147,9 @@ class WorkflowsController(rest.RestController, hooks.HookController): LOG.info("Create workflow(s) [definition=%s]", definition) db_wfs = workflows.create_workflows(definition, scope=scope) - models_dicts = [db_wf.to_dict() for db_wf in db_wfs] workflow_list = [ - resources.Workflow.from_dict(wf) for wf in models_dicts + resources.Workflow.from_db_model(db_wf) for db_wf in db_wfs ] return resources.Workflows(workflows=workflow_list).to_json() @@ -161,6 +159,7 @@ class WorkflowsController(rest.RestController, hooks.HookController): def delete(self, identifier): """Delete a workflow.""" acl.enforce('workflows:delete', context.ctx()) + LOG.info("Delete workflow [identifier=%s]", identifier) with db_api.transaction(): diff --git a/mistral/service/coordination.py b/mistral/service/coordination.py index 0c52c4fc9..d7ecde17e 100644 --- a/mistral/service/coordination.py +++ b/mistral/service/coordination.py @@ -144,6 +144,7 @@ class ServiceCoordinator(object): return [] get_members_req = self._coordinator.get_members(group_id) + try: members = get_members_req.get() diff --git a/mistral/tests/unit/api/test_resource_base.py b/mistral/tests/unit/api/test_resource_base.py new file mode 100644 index 000000000..783a33ada --- /dev/null +++ b/mistral/tests/unit/api/test_resource_base.py @@ -0,0 +1,74 @@ +# Copyright 2016 NEC Corporation. All rights reserved. +# +# 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 copy +import datetime + +from mistral.api.controllers.v2 import resources +from mistral.db.v2 import api as db_api +from mistral.tests.unit import base +from mistral import utils + + +WF_EXEC = { + 'id': 'c0f3be41-88b9-4c86-a669-83e77cd0a1b8', + 'spec': {}, + 'params': {'task': 'my_task1'}, + 'project_id': '', + 'scope': 'PUBLIC', + 'state': 'IDLE', + 'state_info': "Running...", + 'created_at': datetime.datetime(2016, 12, 1, 15, 0, 0), + 'updated_at': None, + 'context': None, + 'task_execution_id': None, + 'description': None, + 'output': None, + 'accepted': False, + 'some_invalid_field': "foobar" +} + + +class TestRestResource(base.DbTestCase): + def test_from_db_model(self): + wf_ex = db_api.create_workflow_execution(WF_EXEC) + + self.assertIsNotNone(wf_ex) + + wf_ex_resource = resources.Execution.from_db_model(wf_ex) + + self.assertIsNotNone(wf_ex_resource) + + expected = copy.copy(WF_EXEC) + + del expected['some_invalid_field'] + utils.datetime_to_str_in_dict(expected, 'created_at') + + self.assertDictEqual(expected, wf_ex.to_dict()) + + def test_from_dict(self): + wf_ex = db_api.create_workflow_execution(WF_EXEC) + + self.assertIsNotNone(wf_ex) + + wf_ex_resource = resources.Execution.from_dict(wf_ex.to_dict()) + + self.assertIsNotNone(wf_ex_resource) + + expected = copy.copy(WF_EXEC) + + del expected['some_invalid_field'] + utils.datetime_to_str_in_dict(expected, 'created_at') + + self.assertDictEqual(expected, wf_ex.to_dict()) diff --git a/mistral/tests/unit/api/v2/test_workflows.py b/mistral/tests/unit/api/v2/test_workflows.py index 1e4e1c363..eb64300a2 100644 --- a/mistral/tests/unit/api/v2/test_workflows.py +++ b/mistral/tests/unit/api/v2/test_workflows.py @@ -412,6 +412,9 @@ class TestWorkflowsController(base.APITest): self.assertEqual(200, resp.status_int) self.assertEqual(1, len(resp.json['workflows'])) + + print(resp.json['workflows'][0]) + self.assertDictEqual(WF, resp.json['workflows'][0]) @mock.patch.object(db_api, "get_workflow_definitions", MOCK_EMPTY) diff --git a/mistral/utils/__init__.py b/mistral/utils/__init__.py index d3a984de3..e5e77b9bd 100644 --- a/mistral/utils/__init__.py +++ b/mistral/utils/__init__.py @@ -474,6 +474,9 @@ def utc_now_sec(): def datetime_to_str(val, sep=' '): """Converts datetime value to string. + If the given value is not an instance of datetime then the method + returns the same value. + :param val: datetime value. :param sep: Separator between date and time. :return: Datetime as a string. diff --git a/mistral/utils/rest_utils.py b/mistral/utils/rest_utils.py index 59e913550..131c795be 100644 --- a/mistral/utils/rest_utils.py +++ b/mistral/utils/rest_utils.py @@ -196,11 +196,14 @@ def get_all(list_cls, cls, get_all_function, get_function, if hasattr(obj, f): data.append(getattr(obj, f)) - dict_data = dict(zip(fields, data)) + # TODO(rakhmerov): This still can be expensive + # due to creation of extra dictionary. + list_to_return.append( + cls.from_dict(dict(zip(fields, data))) + ) else: - dict_data = obj.to_dict() + list_to_return.append(obj) - list_to_return.append(cls.from_dict(dict_data)) else: db_list = get_all_function( limit=limit, @@ -213,10 +216,12 @@ def get_all(list_cls, cls, get_all_function, get_function, ) for data in db_list: - dict_data = (dict(zip(fields, data)) if fields else - data.to_dict()) - - list_to_return.append(cls.from_dict(dict_data)) + if fields: + list_to_return.append( + cls.from_dict(dict(zip(fields, data))) + ) + else: + list_to_return.append(cls.from_db_model(data)) return list_cls.convert_with_links( list_to_return,