diff --git a/README.rst b/README.rst index 89a0b66c..fc974fb2 100644 --- a/README.rst +++ b/README.rst @@ -148,6 +148,21 @@ Delete a migration job: Note: only completed, failed or cancelled jobs can be deleted. +The following Coriolis APIs support pagination: +* transfers +* transfer executions +* deployments +* endpoint instances (only marker and limit parameters) + +Pagination parameters: +* ``sort_key`` - sort key, repeatable. `created_at` and `id` are used by default. +* ``sort_dir`` - sort direction, repeatable. `asc` or `desc` (default). +* ``marker`` - the last seen ID, omitted from the results. +* ``limit`` - the maximum number of records to retrieve. + +Example: + + GET http://server:7667/v1/transfers?marker=a7061715-e56c-470c-a6ac-80bb02f1f198&limit=2&sort_key=id&sort_dir=asc API Documentation ----------------- diff --git a/coriolis/api/common.py b/coriolis/api/common.py index 5ff97ae3..5582fea2 100644 --- a/coriolis/api/common.py +++ b/coriolis/api/common.py @@ -10,3 +10,36 @@ def get_paging_params(req): if limit is not None: limit = utils.parse_int_value(limit) return marker, limit + + +def get_sort_params(req, + default_keys=('created_at', 'id'), + default_dirs=('desc', 'desc')): + """Retrieves sort keys/directions parameters. + + Processes the parameters to create a list of sort keys and sort directions + that correspond to the 'sort_key' and 'sort_dir' parameter values. These + sorting parameters can be specified multiple times in order to generate + the list of sort keys and directions. + + The input parameters are not modified. + + :param req: coriolis.api.wsgi.Request object + :param default_keys: default sort key values, added to the list if no + 'sort_key' parameters are supplied + :param default_dirs: default sort dir values, added to the list if no + 'sort_dir' parameters are supplied + :returns: list of sort keys, list of sort dirs + """ + params = req.params.copy() + sort_keys = [] + sort_dirs = [] + while 'sort_key' in params: + sort_keys.append(params.pop('sort_key').strip()) + while 'sort_dir' in params: + sort_dirs.append(params.pop('sort_dir').strip()) + if len(sort_keys) == 0 and default_keys: + sort_keys.extend(default_keys) + if len(sort_dirs) == 0 and default_dirs: + sort_dirs.extend(default_dirs) + return sort_keys, sort_dirs diff --git a/coriolis/api/v1/deployments.py b/coriolis/api/v1/deployments.py index 14c0382a..2f62ddd7 100644 --- a/coriolis/api/v1/deployments.py +++ b/coriolis/api/v1/deployments.py @@ -4,6 +4,7 @@ from oslo_log import log as logging from webob import exc +from coriolis.api import common from coriolis.api.v1 import utils as api_utils from coriolis.api.v1.views import deployment_view from coriolis.api import wsgi as api_wsgi @@ -43,11 +44,17 @@ def _list(self, req): context.can(deployment_policies.get_deployments_policy_label("list")) include_task_info = api_utils.get_bool_url_arg( req, "include_task_info", default=False) + + marker, limit = common.get_paging_params(req) + sort_keys, sort_dirs = common.get_sort_params(req) + return deployment_view.collection( self._deployment_api.get_deployments( context, include_tasks=include_task_info, - include_task_info=include_task_info + include_task_info=include_task_info, + marker=marker, limit=limit, + sort_keys=sort_keys, sort_dirs=sort_dirs, )) def index(self, req): diff --git a/coriolis/api/v1/transfer_tasks_executions.py b/coriolis/api/v1/transfer_tasks_executions.py index 9241b88f..6fcb5eef 100644 --- a/coriolis/api/v1/transfer_tasks_executions.py +++ b/coriolis/api/v1/transfer_tasks_executions.py @@ -1,6 +1,7 @@ # Copyright 2016 Cloudbase Solutions Srl # All Rights Reserved. +from coriolis.api import common from coriolis.api.v1.views import transfer_tasks_execution_view from coriolis.api import wsgi as api_wsgi from coriolis import exception @@ -31,9 +32,14 @@ def index(self, req, transfer_id): context.can( executions_policies.get_transfer_executions_policy_label("list")) + marker, limit = common.get_paging_params(req) + sort_keys, sort_dirs = common.get_sort_params(req) + return transfer_tasks_execution_view.collection( self._transfer_tasks_execution_api.get_executions( - context, transfer_id, include_tasks=False)) + context, transfer_id, include_tasks=False, + marker=marker, limit=limit, + sort_keys=sort_keys, sort_dirs=sort_dirs)) def detail(self, req, transfer_id): context = req.environ["coriolis.context"] diff --git a/coriolis/api/v1/transfers.py b/coriolis/api/v1/transfers.py index 013ac0d4..92df1fde 100644 --- a/coriolis/api/v1/transfers.py +++ b/coriolis/api/v1/transfers.py @@ -1,6 +1,7 @@ # Copyright 2016 Cloudbase Solutions Srl # All Rights Reserved. +from coriolis.api import common from coriolis.api.v1 import utils as api_utils from coriolis.api.v1.views import transfer_tasks_execution_view from coriolis.api.v1.views import transfer_view @@ -49,11 +50,16 @@ def _list(self, req): context.can(transfer_policies.get_transfers_policy_label("list")) include_task_info = api_utils.get_bool_url_arg( req, "include_task_info", default=False) + marker, limit = common.get_paging_params(req) + sort_keys, sort_dirs = common.get_sort_params(req) return transfer_view.collection( self._transfer_api.get_transfers( context, include_tasks_executions=include_task_info, - include_task_info=include_task_info)) + include_task_info=include_task_info, + marker=marker, limit=limit, + sort_keys=sort_keys, sort_dirs=sort_dirs, + )) def index(self, req): return self._list(req) diff --git a/coriolis/conductor/rpc/client.py b/coriolis/conductor/rpc/client.py index d77fb7f4..a927aab3 100644 --- a/coriolis/conductor/rpc/client.py +++ b/coriolis/conductor/rpc/client.py @@ -143,11 +143,20 @@ def execute_transfer_tasks(self, ctxt, transfer_id, shutdown_instances=shutdown_instances, auto_deploy=auto_deploy) def get_transfer_tasks_executions(self, ctxt, transfer_id, - include_tasks=False): + include_tasks=False, + marker=None, + limit=None, + sort_keys=None, + sort_dirs=None): return self._call( ctxt, 'get_transfer_tasks_executions', transfer_id=transfer_id, - include_tasks=include_tasks) + include_tasks=include_tasks, + marker=marker, + limit=limit, + sort_keys=sort_keys, + sort_dirs=sort_dirs, + ) def get_transfer_tasks_execution(self, ctxt, transfer_id, execution_id, include_task_info=False): @@ -197,11 +206,18 @@ def create_instances_transfer(self, ctxt, skip_os_morphing=skip_os_morphing) def get_transfers(self, ctxt, include_tasks_executions=False, - include_task_info=False): + include_task_info=False, + marker=None, limit=None, + sort_keys=None, sort_dirs=None): return self._call( ctxt, 'get_transfers', include_tasks_executions=include_tasks_executions, - include_task_info=include_task_info) + include_task_info=include_task_info, + marker=marker, + limit=limit, + sort_keys=sort_keys, + sort_dirs=sort_dirs, + ) def get_transfer(self, ctxt, transfer_id, include_task_info=False): return self._call( @@ -217,10 +233,17 @@ def delete_transfer_disks(self, ctxt, transfer_id): ctxt, 'delete_transfer_disks', transfer_id=transfer_id) def get_deployments(self, ctxt, include_tasks=False, - include_task_info=False): + include_task_info=False, + marker=None, limit=None, + sort_keys=None, sort_dirs=None): return self._call( ctxt, 'get_deployments', include_tasks=include_tasks, - include_task_info=include_task_info) + include_task_info=include_task_info, + marker=marker, + limit=limit, + sort_keys=sort_keys, + sort_dirs=sort_dirs, + ) def get_deployment(self, ctxt, deployment_id, include_task_info=False): return self._call( diff --git a/coriolis/conductor/rpc/server.py b/coriolis/conductor/rpc/server.py index 6db7d51c..98bec5e1 100644 --- a/coriolis/conductor/rpc/server.py +++ b/coriolis/conductor/rpc/server.py @@ -1152,10 +1152,19 @@ def execute_transfer_tasks(self, ctxt, transfer_id, shutdown_instances, @transfer_synchronized def get_transfer_tasks_executions(self, ctxt, transfer_id, include_tasks=False, - include_task_info=False): + include_task_info=False, + marker=None, + limit=None, + sort_keys=None, + sort_dirs=None): return db_api.get_transfer_tasks_executions( ctxt, transfer_id, include_tasks, - include_task_info=include_task_info, to_dict=True) + include_task_info=include_task_info, + marker=marker, + limit=limit, + sort_keys=sort_keys, + sort_dirs=sort_dirs, + to_dict=True) @tasks_execution_synchronized def get_transfer_tasks_execution(self, ctxt, transfer_id, execution_id, @@ -1206,10 +1215,17 @@ def _get_transfer_tasks_execution(ctxt, transfer_id, execution_id, @staticmethod def get_transfers(ctxt, include_tasks_executions=False, - include_task_info=False): + include_task_info=False, + marker=None, limit=None, + sort_keys=None, sort_dirs=None): return db_api.get_transfers( ctxt, include_tasks_executions=include_tasks_executions, - include_task_info=include_task_info, to_dict=True) + include_task_info=include_task_info, + marker=marker, + limit=limit, + sort_keys=sort_keys, + sort_dirs=sort_dirs, + to_dict=True) @transfer_synchronized def get_transfer(self, ctxt, transfer_id, include_task_info=False): @@ -1365,10 +1381,16 @@ def _get_transfer(self, ctxt, transfer_id, include_task_info=False, return transfer @staticmethod - def get_deployments(ctxt, include_tasks, include_task_info=False): + def get_deployments(ctxt, include_tasks, include_task_info=False, + marker=None, limit=None, + sort_keys=None, sort_dirs=None): return db_api.get_deployments( ctxt, include_tasks, include_task_info=include_task_info, + marker=marker, + limit=limit, + sort_keys=sort_keys, + sort_dirs=sort_dirs, to_dict=True) @deployment_synchronized diff --git a/coriolis/db/api.py b/coriolis/db/api.py index 816031a4..52f674bb 100644 --- a/coriolis/db/api.py +++ b/coriolis/db/api.py @@ -7,6 +7,7 @@ from oslo_db import api as db_api from oslo_db import options as db_options from oslo_db.sqlalchemy import enginefacade +from oslo_db.sqlalchemy import utils as sqlalchemy_utils from oslo_log import log as logging from oslo_utils import timeutils from sqlalchemy import func @@ -275,7 +276,12 @@ def delete_endpoint(context, endpoint_id): @enginefacade.reader def get_transfer_tasks_executions(context, transfer_id, include_tasks=False, - include_task_info=False, to_dict=False): + include_task_info=False, + marker=None, + limit=None, + sort_keys: list[str] | None = None, + sort_dirs: list[str] | None = None, + to_dict=False): q = _soft_delete_aware_query(context, models.TasksExecution) q = q.join(models.Transfer) if include_task_info: @@ -285,8 +291,26 @@ def get_transfer_tasks_executions(context, transfer_id, include_tasks=False, if is_user_context(context): q = q.filter(models.Transfer.project_id == context.project_id) - db_result = q.filter( - models.Transfer.id == transfer_id).all() + q = q.filter(models.Transfer.id == transfer_id) + + sort_keys, sort_dirs = process_sort_params( + sort_keys, + sort_dirs, + ) + if marker: + try: + marker = get_transfer_tasks_execution( + context, transfer_id, marker) + except exception.NotFound: + raise exception.MarkerNotFound(marker=marker) + q = sqlalchemy_utils.paginate_query( + q, models.TasksExecution, limit, + sort_keys=sort_keys, + sort_dirs=sort_dirs, + marker=marker, + ) + + db_result = q.all() if to_dict: return [e.to_dict() for e in db_result] return db_result @@ -430,6 +454,10 @@ def get_transfers(context, transfer_scenario=None, include_tasks_executions=False, include_task_info=False, + marker=None, + limit=None, + sort_keys: list[str] | None = None, + sort_dirs: list[str] | None = None, to_dict=False): q = _soft_delete_aware_query(context, models.Transfer) if include_tasks_executions: @@ -442,6 +470,23 @@ def get_transfers(context, if is_user_context(context): q = q.filter( models.Transfer.project_id == context.project_id) + + sort_keys, sort_dirs = process_sort_params( + sort_keys, + sort_dirs, + ) + if marker: + try: + marker = get_transfer(context, marker) + except exception.NotFound: + raise exception.MarkerNotFound(marker=marker) + q = sqlalchemy_utils.paginate_query( + q, models.Transfer, limit, + sort_keys=sort_keys, + sort_dirs=sort_dirs, + marker=marker, + ) + db_result = q.all() if to_dict: return [ @@ -539,6 +584,10 @@ def get_transfer_deployments(context, transfer_id): def get_deployments(context, include_tasks=False, include_task_info=False, + marker=None, + limit=None, + sort_keys: list[str] | None = None, + sort_dirs: list[str] | None = None, to_dict=False): q = _soft_delete_aware_query(context, models.Deployment) if include_tasks: @@ -548,10 +597,26 @@ def get_deployments(context, if include_task_info: q = q.options(orm.undefer('info')) - args = {} if is_user_context(context): - args["project_id"] = context.project_id - result = q.filter_by(**args).all() + q = q.filter_by(project_id=context.project_id) + + sort_keys, sort_dirs = process_sort_params( + sort_keys, + sort_dirs, + ) + if marker: + try: + marker = get_deployment(context, marker) + except exception.NotFound: + raise exception.MarkerNotFound(marker=marker) + q = sqlalchemy_utils.paginate_query( + q, models.Deployment, limit, + sort_keys=sort_keys, + sort_dirs=sort_dirs, + marker=marker, + ) + + result = q.all() if to_dict: return [i.to_dict( include_task_info=include_task_info, @@ -1504,3 +1569,78 @@ def update_minion_pool(context, minion_pool_id, updated_values): # the oslo_db library uses this method for both the `created_at` and # `updated_at` fields setattr(lifecycle, 'updated_at', timeutils.utcnow()) + + +def process_sort_params( + sort_keys, + sort_dirs, + default_keys=None, + default_dir='desc', +): + """Process the sort parameters to include default keys. + + Creates a list of sort keys and a list of sort directions. Adds the default + keys to the end of the list if they are not already included. + + When adding the default keys to the sort keys list, the associated + direction is: + 1) The first element in the 'sort_dirs' list (if specified), else + 2) 'default_dir' value (Note that 'asc' is the default value since this is + the default in sqlalchemy.utils.paginate_query) + + :param sort_keys: List of sort keys to include in the processed list + :param sort_dirs: List of sort directions to include in the processed list + :param default_keys: List of sort keys that need to be included in the + processed list, they are added at the end of the list if not already + specified. + :param default_dir: Sort direction associated with each of the default + keys that are not supplied, used when they are added to the processed + list + :returns: list of sort keys, list of sort directions + :raise exception.InvalidInput: If more sort directions than sort keys + are specified or if an invalid sort direction is specified + """ + if default_keys is None: + default_keys = ['created_at', 'id'] + + # Determine direction to use for when adding default keys + if sort_dirs and len(sort_dirs): + default_dir_value = sort_dirs[0] + else: + default_dir_value = default_dir + + # Create list of keys (do not modify the input list) + if sort_keys: + result_keys = list(sort_keys) + else: + result_keys = [] + + # If a list of directions is not provided, use the default sort direction + # for all provided keys. + if sort_dirs: + result_dirs = [] + # Verify sort direction + for sort_dir in sort_dirs: + if sort_dir not in ('asc', 'desc'): + msg = (f"Unknown sort direction: {sort_dir}, " + "must be 'desc' or 'asc'.") + raise exception.InvalidInput(reason=msg) + result_dirs.append(sort_dir) + else: + result_dirs = [default_dir_value for _sort_key in result_keys] + + # Ensure that the key and direction length match + while len(result_dirs) < len(result_keys): + result_dirs.append(default_dir_value) + # Unless more direction are specified, which is an error + if len(result_dirs) > len(result_keys): + msg = "Sort direction array size exceeds sort key array size." + raise exception.InvalidInput(reason=msg) + + # Ensure defaults are included + for key in default_keys: + if key not in result_keys: + result_keys.append(key) + result_dirs.append(default_dir_value) + + return result_keys, result_dirs diff --git a/coriolis/deployments/api.py b/coriolis/deployments/api.py index 8cc21eec..695665fd 100644 --- a/coriolis/deployments/api.py +++ b/coriolis/deployments/api.py @@ -26,9 +26,13 @@ def cancel(self, ctxt, deployment_id, force): self._rpc_client.cancel_deployment(ctxt, deployment_id, force) def get_deployments(self, ctxt, include_tasks=False, - include_task_info=False): + include_task_info=False, + marker=None, limit=None, + sort_keys=None, sort_dirs=None): return self._rpc_client.get_deployments( - ctxt, include_tasks, include_task_info=include_task_info) + ctxt, include_tasks, include_task_info=include_task_info, + marker=marker, limit=limit, + sort_keys=sort_keys, sort_dirs=sort_dirs) def get_deployment(self, ctxt, deployment_id, include_task_info=False): return self._rpc_client.get_deployment( diff --git a/coriolis/exception.py b/coriolis/exception.py index 86671502..73f38488 100644 --- a/coriolis/exception.py +++ b/coriolis/exception.py @@ -289,6 +289,12 @@ class NotFound(CoriolisException): safe = True +class MarkerNotFound(NotFound): + message = _( + "Could not find database record " + "identified by marker: %(marker)s") + + class RegionNotFound(NotFound): message = _("The specified Coriolis region(s) could not be found.") diff --git a/coriolis/tests/api/test_common.py b/coriolis/tests/api/test_common.py new file mode 100644 index 00000000..1d4a46b7 --- /dev/null +++ b/coriolis/tests/api/test_common.py @@ -0,0 +1,43 @@ +# Copyright 2026 Cloudbase Solutions Srl +# All Rights Reserved. + +import webob + +from coriolis.api import common +from coriolis.tests import test_base + + +class ApiCommonTestCase(test_base.CoriolisBaseTestCase): + def test_get_paging_params(self): + req = webob.Request.blank('/some-path?marker=fake-marker&limit=10') + + marker, limit = common.get_paging_params(req) + + self.assertEqual("fake-marker", marker) + self.assertEqual(10, limit) + + def test_get_paging_params_unspecified(self): + req = webob.Request.blank('/some-path') + + marker, limit = common.get_paging_params(req) + + self.assertIsNone(marker) + self.assertIsNone(limit) + + def test_get_sort_params(self): + req = webob.Request.blank( + '/some-path?' + 'sort_key=key0&sort_dir=dir0&sort_key=key1&sort_dir=dir1') + + sort_keys, sort_dirs = common.get_sort_params(req) + + self.assertEqual(["key0", "key1"], sort_keys) + self.assertEqual(["dir0", "dir1"], sort_dirs) + + def test_get_sort_params_unspecified(self): + req = webob.Request.blank('/some-path?') + + sort_keys, sort_dirs = common.get_sort_params(req) + + self.assertEqual(["created_at", "id"], sort_keys) + self.assertEqual(["desc", "desc"], sort_dirs) diff --git a/coriolis/tests/api/v1/test_transfer_tasks_executions.py b/coriolis/tests/api/v1/test_transfer_tasks_executions.py index fd57a007..0bf6fabb 100644 --- a/coriolis/tests/api/v1/test_transfer_tasks_executions.py +++ b/coriolis/tests/api/v1/test_transfer_tasks_executions.py @@ -73,17 +73,28 @@ def test_show_not_found( mock_context, transfer_id, id) mock_single.assert_not_called() + @mock.patch("coriolis.api.common.get_paging_params") + @mock.patch("coriolis.api.common.get_sort_params") @mock.patch.object(transfer_tasks_execution_view, 'collection') @mock.patch.object(api.API, 'get_executions') def test_index( self, mock_get_executions, - mock_collection + mock_collection, + mock_get_sort_params, + mock_get_paging_params, ): mock_req = mock.Mock() mock_context = mock.Mock() mock_req.environ = {'coriolis.context': mock_context} transfer_id = mock.sentinel.transfer_id + mock_get_sort_params.return_value = ( + mock.sentinel.sort_keys, + mock.sentinel.sort_dirs) + mock_get_paging_params.return_value = ( + mock.sentinel.marker, + mock.sentinel.limit, + ) result = self.transfer_api.index(mock_req, transfer_id) @@ -95,7 +106,12 @@ def test_index( mock_context.can.assert_called_once_with( "migration:transfer_executions:list") mock_get_executions.assert_called_once_with( - mock_context, transfer_id, include_tasks=False) + mock_context, transfer_id, include_tasks=False, + marker=mock.sentinel.marker, + limit=mock.sentinel.limit, + sort_keys=mock.sentinel.sort_keys, + sort_dirs=mock.sentinel.sort_dirs, + ) mock_collection.assert_called_once_with( mock_get_executions.return_value) diff --git a/coriolis/tests/api/v1/test_transfers.py b/coriolis/tests/api/v1/test_transfers.py index 7cc50acb..25fdfea9 100644 --- a/coriolis/tests/api/v1/test_transfers.py +++ b/coriolis/tests/api/v1/test_transfers.py @@ -84,6 +84,8 @@ def test_show_no_transfer( include_task_info=mock_get_bool_url_arg.return_value) mock_single.assert_not_called() + @mock.patch("coriolis.api.common.get_paging_params") + @mock.patch("coriolis.api.common.get_sort_params") @mock.patch.object(transfer_view, 'collection') @mock.patch.object(api.API, 'get_transfers') @mock.patch.object(api_utils, 'get_bool_url_arg') @@ -92,10 +94,19 @@ def test_list( mock_get_bool_url_arg, mock_get_transfers, mock_collection, + mock_get_sort_params, + mock_get_paging_params, ): mock_req = mock.Mock() mock_context = mock.Mock() mock_req.environ = {'coriolis.context': mock_context} + mock_get_sort_params.return_value = ( + mock.sentinel.sort_keys, + mock.sentinel.sort_dirs) + mock_get_paging_params.return_value = ( + mock.sentinel.marker, + mock.sentinel.limit, + ) mock_get_bool_url_arg.side_effect = [False, False] @@ -116,7 +127,11 @@ def test_list( mock_get_transfers.assert_called_once_with( mock_context, include_tasks_executions=False, - include_task_info=False + include_task_info=False, + marker=mock.sentinel.marker, + limit=mock.sentinel.limit, + sort_keys=mock.sentinel.sort_keys, + sort_dirs=mock.sentinel.sort_dirs, ) mock_collection.assert_called_once_with( mock_get_transfers.return_value) diff --git a/coriolis/tests/conductor/rpc/test_client.py b/coriolis/tests/conductor/rpc/test_client.py index 2fc6084b..1b884128 100644 --- a/coriolis/tests/conductor/rpc/test_client.py +++ b/coriolis/tests/conductor/rpc/test_client.py @@ -37,6 +37,10 @@ def setUp(self): super(ConductorClientTestCase, self).setUp() self.client = client.ConductorClient() + self._mock_pagination_args = dict( + marker="mock_marker", limit=5, + sort_keys=["mock_column"], sort_dirs=["desc"]) + def test_create_endpoint(self): args = { "name": "mock_name", @@ -161,7 +165,8 @@ def test_execute_transfer_tasks(self): def test_get_transfer_tasks_executions(self): args = { "transfer_id": "mock_transfer_id", - "include_tasks": False + "include_tasks": False, + **self._mock_pagination_args, } self._test(self.client.get_transfer_tasks_executions, args) @@ -205,6 +210,7 @@ def test_get_transfers(self): args = { "include_tasks_executions": False, "include_task_info": False, + **self._mock_pagination_args, } self._test(self.client.get_transfers, args) diff --git a/coriolis/tests/conductor/rpc/test_server.py b/coriolis/tests/conductor/rpc/test_server.py index c37d6eef..096cd33a 100644 --- a/coriolis/tests/conductor/rpc/test_server.py +++ b/coriolis/tests/conductor/rpc/test_server.py @@ -37,6 +37,9 @@ def setUp(self): self.server = server.ConductorServerEndpoint() self._licensing_client = mock.Mock() self.server._licensing_client = self._licensing_client + self._mock_pagination_args = dict( + marker="mock_marker", limit=5, + sort_keys=["mock_column"], sort_dirs=["desc"]) @mock.patch.object( rpc_worker_client.WorkerClient, "from_service_definition" @@ -1447,7 +1450,8 @@ def test_get_transfer_tasks_executions( mock.sentinel.context, mock.sentinel.transfer_id, mock.sentinel.execution_id, - include_task_info=False + include_task_info=False, + **self._mock_pagination_args, ) self.assertEqual( @@ -1459,7 +1463,8 @@ def test_get_transfer_tasks_executions( mock.sentinel.transfer_id, mock.sentinel.execution_id, include_task_info=False, - to_dict=True + **self._mock_pagination_args, + to_dict=True, ) @mock.patch.object(db_api, "get_transfer_tasks_execution") @@ -1675,7 +1680,8 @@ def test_get_transfers(self, mock_get_transfers): result = self.server.get_transfers( mock.sentinel.context, include_tasks_executions=False, - include_task_info=False + include_task_info=False, + **self._mock_pagination_args, ) self.assertEqual( @@ -1686,7 +1692,8 @@ def test_get_transfers(self, mock_get_transfers): mock.sentinel.context, include_tasks_executions=False, include_task_info=False, - to_dict=True + to_dict=True, + **self._mock_pagination_args, ) @mock.patch.object(server.ConductorServerEndpoint, '_get_transfer') diff --git a/coriolis/tests/db/test_api.py b/coriolis/tests/db/test_api.py index 33e90c2d..088010a8 100644 --- a/coriolis/tests/db/test_api.py +++ b/coriolis/tests/db/test_api.py @@ -327,6 +327,75 @@ def test__soft_delete_aware_query_context_show_deleted(self): self.assertIsNotNone(result.deleted_at) +class DBAPISortParamsTestCase(BaseDBAPITestCase): + def test_invalid_sort_dirs(self): + self.assertRaises( + exception.InvalidInput, + api.process_sort_params, + sort_keys=["created_at", "id"], + sort_dirs=["asc", "descending"], + ) + + def test_too_many_sort_dirs(self): + self.assertRaises( + exception.InvalidInput, + api.process_sort_params, + sort_keys=["id"], + sort_dirs=["asc", "asc"], + ) + + def test_unmodified_input(self): + sort_keys = ["created_at", "id"] + sort_dirs = ["desc", "desc"] + + ret_keys, ret_dirs = api.process_sort_params( + sort_keys, sort_dirs) + + self.assertEqual(sort_keys, ret_keys) + self.assertEqual(sort_dirs, ret_dirs) + + def test_unmatched_input(self): + sort_keys = ["created_at", "id"] + sort_dirs = ["asc"] + exp_dirs = ["asc", "asc"] + + ret_keys, ret_dirs = api.process_sort_params( + sort_keys, sort_dirs) + + self.assertEqual(sort_keys, ret_keys) + self.assertEqual(exp_dirs, ret_dirs) + + def test_default_keys_appended(self): + sort_keys = ["created_at"] + sort_dirs = ["asc"] + exp_sort_keys = ["created_at", "id"] + exp_dirs = ["asc", "asc"] + + ret_keys, ret_dirs = api.process_sort_params( + sort_keys, sort_dirs, + default_keys=["id"], + default_dir="asc", + ) + + self.assertEqual(exp_sort_keys, ret_keys) + self.assertEqual(exp_dirs, ret_dirs) + + def test_default_keys_without_input(self): + sort_keys = None + sort_dirs = None + exp_sort_keys = ["id"] + exp_dirs = ["asc"] + + ret_keys, ret_dirs = api.process_sort_params( + sort_keys, sort_dirs, + default_keys=["id"], + default_dir="asc", + ) + + self.assertEqual(exp_sort_keys, ret_keys) + self.assertEqual(exp_dirs, ret_dirs) + + class EndpointDBAPITestCase(BaseDBAPITestCase): @classmethod diff --git a/coriolis/tests/integration/base.py b/coriolis/tests/integration/base.py index dc5fee95..0fd65553 100644 --- a/coriolis/tests/integration/base.py +++ b/coriolis/tests/integration/base.py @@ -101,12 +101,13 @@ def _create_transfer(self, src_id, dst_id, instances): return transfer - def _ignoreExc(self, func): + @staticmethod + def _ignoreExc(func, ignored_exc=Exception): """Wrap the given function, ignoring exceptions.""" def f(*args, **kwargs): try: return func(*args, **kwargs) - except Exception as ex: + except ignored_exc as ex: LOG.warn("Exception encountered: %s", ex) return f diff --git a/coriolis/tests/integration/test_pagination.py b/coriolis/tests/integration/test_pagination.py new file mode 100755 index 00000000..fefa125a --- /dev/null +++ b/coriolis/tests/integration/test_pagination.py @@ -0,0 +1,297 @@ +# Copyright 2026 Cloudbase Solutions Srl +# All Rights Reserved. + +"""API pagination tests.""" + +import datetime +import operator +import uuid + +from oslo_utils import timeutils + +from coriolis import constants +from coriolis import context as coriolis_context +from coriolis.db import api as db_api +from coriolis.db.sqlalchemy import models +from coriolis import exception +from coriolis.tests.integration import base + + +class PaginationTest(base.CoriolisIntegrationTestBase): + FAKE_USER_ID = "fake-user-id" + FAKE_PROJECT_ID = "fake-project-id" + + TRANSFER_COUNT = 5 + EXECUTIONS_PER_TRANSFER = 5 + DEPLOYMENTS_PER_TRANSFER = 5 + + @classmethod + def setUpClass(cls): + super().setUpClass() + + cls._ctx = coriolis_context.RequestContext( + user=cls.FAKE_USER_ID, + project_id=cls.FAKE_PROJECT_ID) + cls._admin_ctx = coriolis_context.get_admin_context() + + cls._setup_mocks() + + @classmethod + def _create_db_transfer( + cls, + origin_endpoint_id: str, + destination_endpoint_id: str, + instances: list[str] | None = None, + **kwargs, + ) -> models.Transfer: + kwargs["id"] = kwargs.get("id", str(uuid.uuid4())) + kwargs["instances"] = instances or [] + kwargs["origin_endpoint_id"] = origin_endpoint_id + kwargs["destination_endpoint_id"] = destination_endpoint_id + kwargs["info"] = {instance: { + 'volumes_info': []} for instance in kwargs["instances"]} + transfer = models.Transfer(**kwargs) + db_api.add_transfer(cls._ctx, transfer) + cls.addClassCleanup( + cls._ignoreExc(db_api.delete_transfer, exception.NotFound), + cls._admin_ctx, transfer.id) + return transfer + + @classmethod + def _create_db_execution( + cls, + transfer: models.Transfer, + **kwargs, + ) -> models.TasksExecution: + kwargs["action_id"] = transfer.id + kwargs["status"] = kwargs.get( + "status", + constants.EXECUTION_STATUS_UNEXECUTED) + kwargs["type"] = kwargs.get( + "type", + constants.EXECUTION_TYPE_TRANSFER_EXECUTION) + execution = models.TasksExecution(**kwargs) + # "add_transfer_tasks_execution" expects "action" to be set, + # despite not being declared by the model. + execution.action = transfer + db_api.add_transfer_tasks_execution(cls._admin_ctx, execution) + cls.addClassCleanup( + cls._ignoreExc(db_api.delete_transfer_tasks_execution, + exception.NotFound), + cls._admin_ctx, execution.id) + return execution + + @classmethod + def _create_db_endpoint( + cls, + **kwargs, + ) -> models.Endpoint: + endpoint_id = kwargs.get("id", str(uuid.uuid4())) + kwargs["id"] = endpoint_id + kwargs["name"] = kwargs.get("name", f"test-endpoint-{endpoint_id}") + kwargs["type"] = kwargs.get("type", "openstack") + endpoint = models.Endpoint( + **kwargs) + db_api.add_endpoint(cls._ctx, endpoint) + cls.addClassCleanup( + cls._ignoreExc(db_api.delete_endpoint, exception.NotFound), + cls._admin_ctx, endpoint.id) + return endpoint + + @classmethod + def _create_db_deployment( + cls, + transfer_id, + origin_endpoint_id: str, + destination_endpoint_id: str, + **kwargs, + ) -> models.Deployment: + kwargs["id"] = kwargs.get("id", str(uuid.uuid4())) + kwargs["transfer_id"] = transfer_id + kwargs["origin_endpoint_id"] = origin_endpoint_id + kwargs["destination_endpoint_id"] = destination_endpoint_id + deployment = models.Deployment( + **kwargs) + db_api.add_deployment(cls._ctx, deployment) + cls.addClassCleanup( + cls._ignoreExc(db_api.delete_deployment, exception.NotFound), + cls._admin_ctx, deployment.id) + return deployment + + @classmethod + def _setup_mocks(cls): + # Note that we're using an admin context when performing cleanups. + # In case of already deleted records we'll get a "NotFound" error + # instead of "NotAuthorized". + cls._src_endpoint = cls._create_db_endpoint() + cls._dst_endpoint = cls._create_db_endpoint() + + cls._transfers = [] + cls._executions = {} + cls._deployments = {} + cls._all_deployments = [] + for transfer_idx in range(cls.TRANSFER_COUNT): + # For testing purposes, we'll set the "created_at" field + # explicitly, adding a small time delta between records. + transfer = cls._create_db_transfer( + origin_endpoint_id=cls._src_endpoint.id, + destination_endpoint_id=cls._dst_endpoint.id, + created_at=timeutils.utcnow() + datetime.timedelta( + seconds=transfer_idx)) + cls._transfers.append(transfer) + + cls._executions[transfer.id] = [] + for execution_idx in range(cls.EXECUTIONS_PER_TRANSFER): + execution = cls._create_db_execution( + transfer=transfer, + created_at=timeutils.utcnow() + datetime.timedelta( + seconds=execution_idx)) + cls._executions[transfer.id].append(execution) + + cls._deployments[transfer.id] = [] + for deployment_idx in range(cls.DEPLOYMENTS_PER_TRANSFER): + deployment = cls._create_db_deployment( + transfer_id=transfer.id, + origin_endpoint_id=cls._src_endpoint.id, + destination_endpoint_id=cls._dst_endpoint.id, + created_at=timeutils.utcnow() + datetime.timedelta( + seconds=deployment_idx)) + cls._deployments[transfer.id].append(deployment) + cls._all_deployments.append(deployment) + + @staticmethod + def _get_record_summary(record): + # Extract a few fields from the db records and entries returned by + # the API so that we can compare them. We don't intend to validate + # *all* fields, just the ones that are relevant for pagination. + created_at = record.created_at + if isinstance(created_at, str): + created_at = datetime.datetime.fromisoformat(created_at) + # The service may not have microsecond level precision + # and we need to compare records. + created_at = created_at.replace(microsecond=0) + return { + "id": record.id, + "created_at": created_at, + } + + def test_transfer_execution_list(self): + executions = self._client.transfer_executions.list( + self._transfers[0].id) + ret_exec_summary = [self._get_record_summary(e) for e in executions] + + exp_exec = self._executions[self._transfers[0].id] + sorted_exp_exec = sorted( + exp_exec, + key=operator.attrgetter('created_at'), + reverse=True) + exp_sorted_exec_summary = [ + self._get_record_summary(e) for e in sorted_exp_exec] + + self.assertEqual(exp_sorted_exec_summary, ret_exec_summary) + + def test_transfer_execution_list_pagination(self): + # Get the first 2 entries, sorted by ID in ascending order. + executions = self._client.transfer_executions.list( + self._transfers[0].id, + limit=2, + sort_keys=['id'], + sort_dirs=['asc']) + ret_exec_summary = [self._get_record_summary(e) for e in executions] + + exp_exec = self._executions[self._transfers[0].id] + sorted_exp_exec = sorted( + exp_exec, + key=operator.attrgetter('id')) + exp_sorted_exec_summary = [ + self._get_record_summary(e) for e in sorted_exp_exec][:2] + self.assertEqual(exp_sorted_exec_summary, ret_exec_summary) + + # Get the next 2 entries. + next_executions = self._client.transfer_executions.list( + self._transfers[0].id, + limit=2, + marker=executions[-1].id, + sort_keys=['id'], + sort_dirs=['asc']) + ret_exec_summary = [ + self._get_record_summary(e) + for e in next_executions] + + exp_sorted_exec_summary = [ + self._get_record_summary(e) for e in sorted_exp_exec][2:4] + self.assertEqual(exp_sorted_exec_summary, ret_exec_summary) + + def test_deployment_list(self): + deployments = self._client.deployments.list() + ret_depl_summary = [self._get_record_summary(d) for d in deployments] + + exp_sorted_depl_summary = [ + self._get_record_summary(d) for d in self._all_deployments] + exp_sorted_depl_summary = sorted( + exp_sorted_depl_summary, + key=lambda x: (x["created_at"], x["id"]), + reverse=True) + self.assertEqual(exp_sorted_depl_summary, ret_depl_summary) + + def test_deployment_list_pagination(self): + # Get the first 2 entries, sorted by ID in ascending order. + deployments = self._client.deployments.list( + limit=2, + sort_keys=['id'], + sort_dirs=['asc']) + ret_depl_summary = [self._get_record_summary(d) for d in deployments] + + exp_sorted_depl_summary = [ + self._get_record_summary(d) for d in self._all_deployments] + exp_sorted_depl_summary = sorted( + exp_sorted_depl_summary, + key=lambda x: x["id"]) + self.assertEqual(exp_sorted_depl_summary[:2], ret_depl_summary) + + # Get the next 2 entries. + deployments = self._client.deployments.list( + limit=2, + sort_keys=['id'], + sort_dirs=['asc'], + marker=ret_depl_summary[-1]["id"], + ) + ret_depl_summary = [self._get_record_summary(d) for d in deployments] + self.assertEqual(exp_sorted_depl_summary[2:4], ret_depl_summary) + + def test_transfer_list(self): + transfers = self._client.transfers.list() + ret_transfer_summary = [self._get_record_summary(t) for t in transfers] + + exp_sorted_transfer_summary = [ + self._get_record_summary(d) for d in self._transfers] + exp_sorted_transfer_summary = sorted( + exp_sorted_transfer_summary, + key=lambda x: (x["created_at"], x["id"]), + reverse=True) + self.assertEqual(exp_sorted_transfer_summary, ret_transfer_summary) + + def test_transfer_list_pagination(self): + # Get the first 2 entries, sorted by ID in ascending order. + transfers = self._client.transfers.list( + limit=2, + sort_keys=['id'], + sort_dirs=['asc']) + ret_transfer_summary = [self._get_record_summary(t) for t in transfers] + + exp_sorted_transfer_summary = [ + self._get_record_summary(d) for d in self._transfers] + exp_sorted_transfer_summary = sorted( + exp_sorted_transfer_summary, + key=lambda x: x["id"]) + self.assertEqual(exp_sorted_transfer_summary[:2], ret_transfer_summary) + + # Get the next 2 entries. + transfers = self._client.transfers.list( + limit=2, + sort_keys=['id'], + sort_dirs=['asc'], + marker=transfers[-1].id) + ret_transfer_summary = [self._get_record_summary(t) for t in transfers] + self.assertEqual( + exp_sorted_transfer_summary[2:4], ret_transfer_summary) diff --git a/coriolis/tests/transfer_tasks_executions/test_api.py b/coriolis/tests/transfer_tasks_executions/test_api.py index 116c774c..36192446 100644 --- a/coriolis/tests/transfer_tasks_executions/test_api.py +++ b/coriolis/tests/transfer_tasks_executions/test_api.py @@ -48,13 +48,23 @@ def test_cancel(self): self.ctxt, self.transfer_id, self.execution_id, force)) def test_get_executions(self): - include_tasks = mock.sentinel.include_tasks - - result = self.api.get_executions(self.ctxt, self.transfer_id, - include_tasks) + result = self.api.get_executions( + self.ctxt, self.transfer_id, + mock.sentinel.include_tasks, + mock.sentinel.marker, + mock.sentinel.limit, + mock.sentinel.sort_keys, + mock.sentinel.sort_dirs, + ) self.rpc_client.get_transfer_tasks_executions.assert_called_once_with( - self.ctxt, self.transfer_id, include_tasks) + self.ctxt, self.transfer_id, + mock.sentinel.include_tasks, + mock.sentinel.marker, + mock.sentinel.limit, + mock.sentinel.sort_keys, + mock.sentinel.sort_dirs, + ) self.assertEqual( result, self.rpc_client.get_transfer_tasks_executions.return_value) diff --git a/coriolis/tests/transfers/test_api.py b/coriolis/tests/transfers/test_api.py index 308d2b75..9992e9eb 100644 --- a/coriolis/tests/transfers/test_api.py +++ b/coriolis/tests/transfers/test_api.py @@ -17,6 +17,9 @@ def setUp(self): self.api._rpc_client = self.rpc_client self.ctxt = mock.sentinel.ctxt self.transfer_id = mock.sentinel.transfer_id + self._mock_pagination_args = dict( + marker="mock_marker", limit=5, + sort_keys=["mock_column"], sort_dirs=["desc"]) def test_create(self): origin_endpoint_id = mock.sentinel.origin_endpoint_id @@ -66,10 +69,14 @@ def test_delete(self): def test_get_transfers(self): result = self.api.get_transfers( - self.ctxt, include_tasks_executions=False, include_task_info=False) + self.ctxt, include_tasks_executions=False, include_task_info=False, + **self._mock_pagination_args, + ) self.rpc_client.get_transfers.assert_called_once_with( - self.ctxt, False, include_task_info=False) + self.ctxt, False, include_task_info=False, + **self._mock_pagination_args, + ) self.assertEqual(result, self.rpc_client.get_transfers.return_value) def test_get_transfer(self): diff --git a/coriolis/transfer_tasks_executions/api.py b/coriolis/transfer_tasks_executions/api.py index 6e738599..7158f93e 100644 --- a/coriolis/transfer_tasks_executions/api.py +++ b/coriolis/transfer_tasks_executions/api.py @@ -20,9 +20,12 @@ def cancel(self, ctxt, transfer_id, execution_id, force): self._rpc_client.cancel_transfer_tasks_execution( ctxt, transfer_id, execution_id, force) - def get_executions(self, ctxt, transfer_id, include_tasks=False): + def get_executions(self, ctxt, transfer_id, include_tasks=False, + marker=None, limit=None, + sort_keys=None, sort_dirs=None): return self._rpc_client.get_transfer_tasks_executions( - ctxt, transfer_id, include_tasks) + ctxt, transfer_id, include_tasks, marker, limit, + sort_keys, sort_dirs) def get_execution(self, ctxt, transfer_id, execution_id): return self._rpc_client.get_transfer_tasks_execution( diff --git a/coriolis/transfers/api.py b/coriolis/transfers/api.py index 9d56ed9c..31ff9623 100644 --- a/coriolis/transfers/api.py +++ b/coriolis/transfers/api.py @@ -32,10 +32,14 @@ def delete(self, ctxt, transfer_id): self._rpc_client.delete_transfer(ctxt, transfer_id) def get_transfers(self, ctxt, include_tasks_executions=False, - include_task_info=False): + include_task_info=False, + marker=None, limit=None, + sort_keys=None, sort_dirs=None): return self._rpc_client.get_transfers( ctxt, include_tasks_executions, - include_task_info=include_task_info) + include_task_info=include_task_info, + marker=marker, limit=limit, + sort_keys=sort_keys, sort_dirs=sort_dirs) def get_transfer(self, ctxt, transfer_id, include_task_info=False): return self._rpc_client.get_transfer(