diff --git a/mistral/db/v2/api.py b/mistral/db/v2/api.py index 6bdf74f6a..cb4c164e2 100644 --- a/mistral/db/v2/api.py +++ b/mistral/db/v2/api.py @@ -174,7 +174,7 @@ def load_action_definition(name): return IMPL.load_action_definition(name) -def get_action_definitions(limit=None, marker=None, sort_keys=['name'], +def get_action_definitions(limit=None, marker=None, sort_keys=None, sort_dirs=None, **kwargs): return IMPL.get_action_definitions( limit=limit, @@ -255,7 +255,7 @@ def load_workflow_execution(name): return IMPL.load_workflow_execution(name) -def get_workflow_executions(limit=None, marker=None, sort_keys=['created_at'], +def get_workflow_executions(limit=None, marker=None, sort_keys=None, sort_dirs=None, **kwargs): return IMPL.get_workflow_executions( limit=limit, @@ -301,7 +301,7 @@ def load_task_execution(id): return IMPL.load_task_execution(id) -def get_task_executions(limit=None, marker=None, sort_keys=['created_at'], +def get_task_executions(limit=None, marker=None, sort_keys=None, sort_dirs=None, **kwargs): return IMPL.get_task_executions( limit=limit, @@ -447,7 +447,7 @@ def load_environment(name): return IMPL.load_environment(name) -def get_environments(limit=None, marker=None, sort_keys=['name'], +def get_environments(limit=None, marker=None, sort_keys=None, sort_dirs=None, **kwargs): return IMPL.get_environments( diff --git a/mistral/db/v2/sqlalchemy/api.py b/mistral/db/v2/sqlalchemy/api.py index be46ae977..9329c19de 100644 --- a/mistral/db/v2/sqlalchemy/api.py +++ b/mistral/db/v2/sqlalchemy/api.py @@ -178,7 +178,9 @@ def _paginate_query(model, limit=None, marker=None, sort_keys=None, sort_keys = sort_keys if sort_keys else [] - if 'id' not in sort_keys: + # We should add sorting by id only if we use pagination. Otherwise + # we can omit it to increase the performance. + if (marker or limit) and 'id' not in sort_keys: sort_keys.append('id') sort_dirs.append('asc') if sort_dirs else None @@ -224,28 +226,6 @@ def _get_collection(model, insecure=False, limit=None, marker=None, return query.all() -def _get_collection_sorted_by_name(model, insecure=False, fields=None, - sort_keys=['name'], **kwargs): - return _get_collection( - model=model, - insecure=insecure, - sort_keys=sort_keys, - fields=fields, - **kwargs - ) - - -def _get_collection_sorted_by_time(model, insecure=False, fields=None, - sort_keys=['created_at'], **kwargs): - return _get_collection( - model=model, - insecure=insecure, - sort_keys=sort_keys, - fields=fields, - **kwargs - ) - - def _get_db_object_by_name(model, name, filter_=None, order_by=None): query = _secure_query(model) @@ -404,7 +384,7 @@ def load_workbook(name, session=None): @b.session_aware() def get_workbooks(session=None, **kwargs): - return _get_collection_sorted_by_name(models.Workbook, **kwargs) + return _get_collection(models.Workbook, **kwargs) @b.session_aware() @@ -511,15 +491,13 @@ def load_workflow_definition(name, namespace='', session=None): @b.session_aware() -def get_workflow_definitions(sort_keys=['created_at'], fields=None, - session=None, **kwargs): +def get_workflow_definitions(fields=None, session=None, **kwargs): if fields and 'input' in fields: fields.remove('input') fields.append('spec') - return _get_collection_sorted_by_name( + return _get_collection( model=models.WorkflowDefinition, - sort_keys=sort_keys, fields=fields, **kwargs ) @@ -655,10 +633,7 @@ def load_action_definition(name, session=None): @b.session_aware() def get_action_definitions(session=None, **kwargs): - return _get_collection_sorted_by_name( - model=models.ActionDefinition, - **kwargs - ) + return _get_collection(model=models.ActionDefinition, **kwargs) @b.session_aware() @@ -781,7 +756,7 @@ def delete_action_executions(session=None, **kwargs): def _get_action_executions(**kwargs): - return _get_collection_sorted_by_time(models.ActionExecution, **kwargs) + return _get_collection(models.ActionExecution, **kwargs) # Workflow executions. @@ -816,10 +791,7 @@ def ensure_workflow_execution_exists(id, session=None): @b.session_aware() def get_workflow_executions(session=None, **kwargs): - return _get_collection_sorted_by_time( - models.WorkflowExecution, - **kwargs - ) + return _get_collection(models.WorkflowExecution, **kwargs) @b.session_aware() @@ -994,7 +966,7 @@ def delete_task_executions(session=None, **kwargs): def _get_task_executions(**kwargs): - return _get_collection_sorted_by_time(models.TaskExecution, **kwargs) + return _get_collection(models.TaskExecution, **kwargs) # Delayed calls. @@ -1073,10 +1045,7 @@ def get_delayed_call(id, session=None): @b.session_aware() def get_delayed_calls(session=None, **kwargs): - return _get_collection( - model=models.DelayedCall, - **kwargs - ) + return _get_collection(model=models.DelayedCall, **kwargs) @b.session_aware() @@ -1160,12 +1129,8 @@ def load_cron_trigger(identifier, session=None): @b.session_aware() -def get_cron_triggers(insecure=False, session=None, **kwargs): - return _get_collection_sorted_by_name( - models.CronTrigger, - insecure=insecure, - **kwargs - ) +def get_cron_triggers(session=None, **kwargs): + return _get_collection(models.CronTrigger, **kwargs) @b.session_aware() @@ -1284,7 +1249,7 @@ def load_environment(name, session=None): @b.session_aware() def get_environments(session=None, **kwargs): - return _get_collection_sorted_by_name(models.Environment, **kwargs) + return _get_collection(models.Environment, **kwargs) @b.session_aware() @@ -1514,12 +1479,8 @@ def load_event_trigger(id, insecure=False, session=None): @b.session_aware() -def get_event_triggers(insecure=False, session=None, **kwargs): - return _get_collection_sorted_by_time( - model=models.EventTrigger, - insecure=insecure, - **kwargs - ) +def get_event_triggers(session=None, **kwargs): + return _get_collection(model=models.EventTrigger, **kwargs) @b.session_aware() diff --git a/mistral/tests/unit/db/v2/test_sqlalchemy_db_api.py b/mistral/tests/unit/db/v2/test_sqlalchemy_db_api.py index 07a88644d..ae86b1df8 100644 --- a/mistral/tests/unit/db/v2/test_sqlalchemy_db_api.py +++ b/mistral/tests/unit/db/v2/test_sqlalchemy_db_api.py @@ -145,8 +145,8 @@ class WorkbookTest(SQLAlchemyTest): fetched = db_api.get_workbooks() self.assertEqual(2, len(fetched)) - self.assertEqual(created0, fetched[0]) - self.assertEqual(created1, fetched[1]) + self._assert_single_item(fetched, name=created0['name']) + self._assert_single_item(fetched, name=created1['name']) def test_filter_workbooks_by_equal_value(self): db_api.create_workbook(WORKBOOKS[0]) @@ -203,8 +203,8 @@ class WorkbookTest(SQLAlchemyTest): fetched = db_api.get_workbooks(**_filter) self.assertEqual(2, len(fetched)) - self.assertEqual(created0, fetched[0]) - self.assertEqual(created1, fetched[1]) + self._assert_single_item(fetched, name=created0['name']) + self._assert_single_item(fetched, name=created1['name']) def test_filter_workbooks_by_less_than_value(self): created0 = db_api.create_workbook(WORKBOOKS[0]) @@ -232,8 +232,8 @@ class WorkbookTest(SQLAlchemyTest): fetched = db_api.get_workbooks(**_filter) self.assertEqual(2, len(fetched)) - self.assertEqual(created0, fetched[0]) - self.assertEqual(created1, fetched[1]) + self._assert_single_item(fetched, name=created0['name']) + self._assert_single_item(fetched, name=created1['name']) def test_filter_workbooks_by_values_in_list(self): created0 = db_api.create_workbook(WORKBOOKS[0]) @@ -489,8 +489,8 @@ class WorkflowDefinitionTest(SQLAlchemyTest): fetched = db_api.get_workflow_definitions(**_filter) self.assertEqual(2, len(fetched)) - self.assertEqual(created0, fetched[0]) - self.assertEqual(created1, fetched[1]) + self._assert_single_item(fetched, name=created0['name']) + self._assert_single_item(fetched, name=created1['name']) def test_filter_workflow_definition_by_less_than_value(self): created0 = db_api.create_workflow_definition(WF_DEFINITIONS[0]) @@ -518,8 +518,8 @@ class WorkflowDefinitionTest(SQLAlchemyTest): fetched = db_api.get_workflow_definitions(**_filter) self.assertEqual(2, len(fetched)) - self.assertEqual(created0, fetched[0]) - self.assertEqual(created1, fetched[1]) + self._assert_single_item(fetched, name=created0['name']) + self._assert_single_item(fetched, name=created1['name']) def test_filter_workflow_definition_by_values_in_list(self): created0 = db_api.create_workflow_definition(WF_DEFINITIONS[0]) @@ -782,8 +782,8 @@ class WorkflowDefinitionTest(SQLAlchemyTest): fetched = db_api.get_workflow_definitions() self.assertEqual(2, len(fetched)) - self.assertEqual(created0, fetched[0]) - self.assertEqual(created1, fetched[1]) + self._assert_single_item(fetched, name=created0['name']) + self._assert_single_item(fetched, name=created1['name']) def test_delete_workflow_definition(self): created0 = db_api.create_workflow_definition(WF_DEFINITIONS[0]) @@ -996,8 +996,8 @@ class ActionDefinitionTest(SQLAlchemyTest): fetched = db_api.get_action_definitions(**_filter) self.assertEqual(2, len(fetched)) - self.assertEqual(created0, fetched[0]) - self.assertEqual(created1, fetched[1]) + self._assert_single_item(fetched, name=created0['name']) + self._assert_single_item(fetched, name=created1['name']) def test_filter_action_definitions_by_greater_than_value(self): created0 = db_api.create_action_definition(ACTION_DEFINITIONS[0]) @@ -1028,9 +1028,9 @@ class ActionDefinitionTest(SQLAlchemyTest): fetched = db_api.get_action_definitions(**_filter) self.assertEqual(3, len(fetched)) - self.assertEqual(created0, fetched[0]) - self.assertEqual(created1, fetched[1]) - self.assertEqual(created2, fetched[2]) + self._assert_single_item(fetched, name=created0['name']) + self._assert_single_item(fetched, name=created1['name']) + self._assert_single_item(fetched, name=created2['name']) def test_filter_action_definitions_by_less_than_value(self): created0 = db_api.create_action_definition(ACTION_DEFINITIONS[0]) @@ -1045,8 +1045,8 @@ class ActionDefinitionTest(SQLAlchemyTest): fetched = db_api.get_action_definitions(**_filter) self.assertEqual(2, len(fetched)) - self.assertEqual(created0, fetched[0]) - self.assertEqual(created1, fetched[1]) + self._assert_single_item(fetched, name=created0['name']) + self._assert_single_item(fetched, name=created1['name']) def test_filter_action_definitions_by_less_than_equal_value(self): created0 = db_api.create_action_definition(ACTION_DEFINITIONS[0]) @@ -1061,9 +1061,9 @@ class ActionDefinitionTest(SQLAlchemyTest): fetched = db_api.get_action_definitions(**_filter) self.assertEqual(3, len(fetched)) - self.assertEqual(created0, fetched[0]) - self.assertEqual(created1, fetched[1]) - self.assertEqual(created2, fetched[2]) + self._assert_single_item(fetched, name=created0['name']) + self._assert_single_item(fetched, name=created1['name']) + self._assert_single_item(fetched, name=created2['name']) def test_filter_action_definitions_by_values_in_list(self): created0 = db_api.create_action_definition(ACTION_DEFINITIONS[0]) @@ -1079,8 +1079,8 @@ class ActionDefinitionTest(SQLAlchemyTest): fetched = db_api.get_action_definitions(**_filter) self.assertEqual(2, len(fetched)) - self.assertEqual(created0, fetched[0]) - self.assertEqual(created1, fetched[1]) + self._assert_single_item(fetched, name=created0['name']) + self._assert_single_item(fetched, name=created1['name']) def test_filter_action_definitions_by_values_notin_list(self): created0 = db_api.create_action_definition(ACTION_DEFINITIONS[0]) @@ -1095,7 +1095,7 @@ class ActionDefinitionTest(SQLAlchemyTest): fetched = db_api.get_action_definitions(**_filter) self.assertEqual(1, len(fetched)) - self.assertEqual(created2, fetched[0]) + self._assert_single_item(fetched, name=created2['name']) def test_filter_action_definitions_by_multiple_columns(self): created0 = db_api.create_action_definition(ACTION_DEFINITIONS[0]) @@ -1204,8 +1204,8 @@ class ActionDefinitionTest(SQLAlchemyTest): fetched = db_api.get_action_definitions(is_system=True) self.assertEqual(2, len(fetched)) - self.assertEqual(created0, fetched[0]) - self.assertEqual(created1, fetched[1]) + self._assert_single_item(fetched, name=created0['name']) + self._assert_single_item(fetched, name=created1['name']) def test_delete_action_definition_with_name(self): created = db_api.create_action_definition(ACTION_DEFINITIONS[0]) @@ -1606,8 +1606,8 @@ class WorkflowExecutionTest(SQLAlchemyTest): fetched = db_api.get_workflow_executions(**_filter) self.assertEqual(2, len(fetched)) - self.assertEqual(created0, fetched[0]) - self.assertEqual(created1, fetched[1]) + self._assert_single_item(fetched, state=created0['state']) + self._assert_single_item(fetched, state=created1['state']) def test_filter_workflow_execution_by_less_than_value(self): with db_api.transaction(): @@ -1637,8 +1637,8 @@ class WorkflowExecutionTest(SQLAlchemyTest): fetched = db_api.get_workflow_executions(**_filter) self.assertEqual(2, len(fetched)) - self.assertEqual(created0, fetched[0]) - self.assertEqual(created1, fetched[1]) + self._assert_single_item(fetched, state=created0['state']) + self._assert_single_item(fetched, state=created1['state']) def test_filter_workflow_execution_by_values_in_list(self): with db_api.transaction(): @@ -1966,8 +1966,8 @@ class TaskExecutionTest(SQLAlchemyTest): ) self.assertEqual(2, len(fetched)) - self.assertEqual(created0, fetched[0]) - self.assertEqual(created1, fetched[1]) + self._assert_single_item(fetched, name=created0['name']) + self._assert_single_item(fetched, name=created1['name']) def test_filter_task_execution_by_equal_value(self): created, _ = self._create_task_executions() @@ -2020,8 +2020,8 @@ class TaskExecutionTest(SQLAlchemyTest): fetched = db_api.get_task_executions(**_filter) self.assertEqual(2, len(fetched)) - self.assertEqual(created0, fetched[0]) - self.assertEqual(created1, fetched[1]) + self._assert_single_item(fetched, name=created0['name']) + self._assert_single_item(fetched, name=created1['name']) def test_filter_task_execution_by_less_than_value(self): created0, created1 = self._create_task_executions() @@ -2047,8 +2047,8 @@ class TaskExecutionTest(SQLAlchemyTest): fetched = db_api.get_task_executions(**_filter) self.assertEqual(2, len(fetched)) - self.assertEqual(created0, fetched[0]) - self.assertEqual(created1, fetched[1]) + self._assert_single_item(fetched, name=created0['name']) + self._assert_single_item(fetched, name=created1['name']) def test_filter_task_execution_by_values_in_list(self): created, _ = self._create_task_executions() @@ -2329,8 +2329,8 @@ class CronTriggerTest(SQLAlchemyTest): fetched = db_api.get_cron_triggers(pattern='* * * * *') self.assertEqual(2, len(fetched)) - self.assertEqual(created0, fetched[0]) - self.assertEqual(created1, fetched[1]) + self._assert_single_item(fetched, name=created0['name']) + self._assert_single_item(fetched, name=created1['name']) def test_get_cron_trigger(self): cron_trigger = db_api.create_cron_trigger(CRON_TRIGGERS[0]) @@ -2531,8 +2531,8 @@ class EnvironmentTest(SQLAlchemyTest): fetched = db_api.get_environments() self.assertEqual(2, len(fetched)) - self.assertEqual(created0, fetched[0]) - self.assertEqual(created1, fetched[1]) + self._assert_single_item(fetched, name=created0['name']) + self._assert_single_item(fetched, name=created1['name']) def test_delete_environment(self): created = db_api.create_environment(ENVIRONMENTS[0]) diff --git a/mistral/utils/rest_utils.py b/mistral/utils/rest_utils.py index e721afaa1..4408847cc 100644 --- a/mistral/utils/rest_utils.py +++ b/mistral/utils/rest_utils.py @@ -127,7 +127,7 @@ def filters_to_dict(**kwargs): def get_all(list_cls, cls, get_all_function, get_function, resource_function=None, marker=None, limit=None, - sort_keys='created_at', sort_dirs='asc', fields='', + sort_keys=None, sort_dirs=None, fields=None, all_projects=False, **filters): """Return a list of cls. @@ -143,11 +143,11 @@ def get_all(list_cls, cls, get_all_function, get_function, :param limit: Optional. Maximum number of resources to return in a single result. Default value is None for backward compatibility. - :param sort_keys: Optional. Columns to sort results by. - Default: created_at. - :param sort_dirs: Optional. Directions to sort corresponding to + :param sort_keys: Optional. List of columns to sort results by. + Default: ['created_at']. + :param sort_dirs: Optional. List of directions to sort corresponding to sort_keys, "asc" or "desc" can be chosen. - Default: asc. + Default: ['asc']. :param fields: Optional. A specified list of fields of the resource to be returned. 'id' will be included automatically in fields if it's provided, since it will be used when @@ -155,6 +155,10 @@ def get_all(list_cls, cls, get_all_function, get_function, :param filters: Optional. A specified dictionary of filters to match. :param all_projects: Optional. Get resources of all projects. """ + sort_keys = ['created_at'] if sort_keys is None else sort_keys + sort_dirs = ['asc'] if sort_dirs is None else sort_dirs + fields = [] if fields is None else fields + if fields and 'id' not in fields: fields.insert(0, 'id')