From 05cc14d39997cf13f43298852824fbca3c6a3ddf Mon Sep 17 00:00:00 2001 From: Xinran Date: Tue, 24 Oct 2017 10:13:48 +0800 Subject: [PATCH] Pagination support for server list API We should support to return a list of servers according to users' requirements. In this patch, marker, limit, sort_key and sort_dir were added in server list API. - marker is used to display a list of servers after marker - limit is used to determinate the maximum number of servers to display - sort_key is used to sort the returned server list by specified key value - sort_dir is used to select a sort direction DocImpact APIImpact Change-Id: Id70e965794c82a0a29e53d4364f65b0f39042c7c Closes-Bug: #1726665 --- api-ref/source/v1/parameters.yaml | 36 +++++++++++++++++++++++++ api-ref/source/v1/servers.inc | 8 ++++++ mogan/api/controllers/v1/servers.py | 22 +++++++++++++-- mogan/api/controllers/v1/utils.py | 1 - mogan/db/api.py | 3 ++- mogan/db/sqlalchemy/api.py | 24 +++++++++++++++-- mogan/objects/server.py | 8 +++++- mogan/tests/unit/objects/test_server.py | 11 ++++++-- 8 files changed, 104 insertions(+), 9 deletions(-) diff --git a/api-ref/source/v1/parameters.yaml b/api-ref/source/v1/parameters.yaml index 95350e88..1c1d8a85 100644 --- a/api-ref/source/v1/parameters.yaml +++ b/api-ref/source/v1/parameters.yaml @@ -122,12 +122,48 @@ image_query: in: query required: false type: string +limit: + description: | + Requests a page size of items. Returns a number of items up to a limit value. + Use the ``limit`` parameter to make an initial limited request and use the ID + of the last-seen item from the response as the ``marker`` parameter value in a + subsequent limited request. + in: query + required: false + type: integer +marker: + description: | + The ID of the last-seen item. Use the ``limit`` parameter to make an initial limited + request and use the ID of the last-seen item from the response as the ``marker`` + parameter value in a subsequent limited request. + in: query + required: false + type: string server_name_query: description: | Filters the server list by name. Users can filter by prefix of server's name. in: query required: false type: string +sort_dir: + description: | + Sort direction. A valid value is ``asc`` (ascending) or ``desc`` (descending). + Default is ``asc``. You can specify multiple pairs of sort key and sort direction + query parameters. If you omit the sort direction in a pair, the API uses the natural + sorting direction of the direction of the server ``sort_key`` attribute. + in: query + required: false + type: string +sort_key: + description: | + Sorts the response by the this attribute value. + Default is ``id``. You can specify multiple pairs of sort key and + sort direction query parameters. If you omit the sort direction in + a pair, the API uses the natural sorting direction of the server + attribute that is provided as the ``sort_key``. + in: query + required: false + type: string status_query: description: | Filters the server list by the server's status. diff --git a/api-ref/source/v1/servers.inc b/api-ref/source/v1/servers.inc index 423dd887..4a4b1794 100644 --- a/api-ref/source/v1/servers.inc +++ b/api-ref/source/v1/servers.inc @@ -157,6 +157,10 @@ Request - flavor_name: flavor_name_query - image_uuid: image_query - ip: fixed_ip_query + - limit: limit + - marker: marker + - sort_key: sort_key + - sort_dir: sort_dir - all_tenants: all_tenants - fields: fields @@ -202,6 +206,10 @@ Request - flavor_name: flavor_name_query - image_uuid: image_query - ip: fixed_ip_query + - limit: limit + - marker: marker + - sort_key: sort_key + - sort_dir: sort_dir - all_tenants: all_tenants diff --git a/mogan/api/controllers/v1/servers.py b/mogan/api/controllers/v1/servers.py index c9750c39..4ff10e54 100644 --- a/mogan/api/controllers/v1/servers.py +++ b/mogan/api/controllers/v1/servers.py @@ -568,12 +568,20 @@ class ServerController(ServerControllerBase): def _get_server_collection(self, name=None, status=None, flavor_uuid=None, flavor_name=None, image_uuid=None, ip=None, + limit=None, marker=None, + sort_key=None, sort_dir=None, all_tenants=None, fields=None): context = pecan.request.context project_only = True if context.is_admin and all_tenants: project_only = False + limit = api_utils.validate_limit(limit) + + marker_obj = None + if marker: + marker_obj = objects.Server.get(pecan.request.context, marker) + filters = {} if name: filters['name'] = name @@ -587,6 +595,8 @@ class ServerController(ServerControllerBase): filters['image_uuid'] = image_uuid servers = objects.Server.list(pecan.request.context, + limit, marker_obj, + sort_key=sort_key, sort_dir=sort_dir, project_only=project_only, filters=filters) if ip: @@ -617,10 +627,12 @@ class ServerController(ServerControllerBase): @expose.expose(ServerCollection, wtypes.text, wtypes.text, types.uuid, wtypes.text, types.uuid, wtypes.text, - types.listtype, types.boolean) + int, types.uuid, types.listtype, wtypes.text, + wtypes.text, types.boolean) def get_all(self, name=None, status=None, flavor_uuid=None, flavor_name=None, image_uuid=None, ip=None, - fields=None, all_tenants=None): + limit=None, marker=None, fields=None, sort_key='id', + sort_dir='asc', all_tenants=None): """Retrieve a list of server. :param fields: Optional, a list with a specified set of fields @@ -635,6 +647,8 @@ class ServerController(ServerControllerBase): return self._get_server_collection(name, status, flavor_uuid, flavor_name, image_uuid, ip, + limit, marker, + sort_key, sort_dir, all_tenants=all_tenants, fields=fields) @@ -654,9 +668,11 @@ class ServerController(ServerControllerBase): @expose.expose(ServerCollection, wtypes.text, wtypes.text, types.uuid, wtypes.text, types.uuid, wtypes.text, + int, types.uuid, wtypes.text, wtypes.text, types.boolean) def detail(self, name=None, status=None, flavor_uuid=None, flavor_name=None, image_uuid=None, ip=None, + limit=None, marker=None, sort_key='id', sort_dir='asc', all_tenants=None): """Retrieve detail of a list of servers.""" # /detail should only work against collections @@ -668,6 +684,8 @@ class ServerController(ServerControllerBase): return self._get_server_collection(name, status, flavor_uuid, flavor_name, image_uuid, ip, + limit, marker, + sort_key, sort_dir, all_tenants=all_tenants) @policy.authorize_wsgi("mogan:server", "create", False) diff --git a/mogan/api/controllers/v1/utils.py b/mogan/api/controllers/v1/utils.py index f79331d3..89a4f628 100644 --- a/mogan/api/controllers/v1/utils.py +++ b/mogan/api/controllers/v1/utils.py @@ -31,7 +31,6 @@ JSONPATCH_EXCEPTIONS = (jsonpatch.JsonPatchException, def validate_limit(limit): if limit is None: return CONF.api.max_limit - if limit <= 0: raise wsme.exc.ClientSideError(_("Limit must be positive")) diff --git a/mogan/db/api.py b/mogan/db/api.py index 33af7204..8563ffcf 100644 --- a/mogan/db/api.py +++ b/mogan/db/api.py @@ -72,7 +72,8 @@ class Connection(object): """Get server by name.""" @abc.abstractmethod - def server_get_all(self, context, project_only, filters=None): + def server_get_all(self, context, project_only, limit=None, marker=None, + sort_key=None, sort_dir=None, filters=None): """Get all servers.""" @abc.abstractmethod diff --git a/mogan/db/sqlalchemy/api.py b/mogan/db/sqlalchemy/api.py index 1399ffcd..0efc7897 100644 --- a/mogan/db/sqlalchemy/api.py +++ b/mogan/db/sqlalchemy/api.py @@ -84,6 +84,24 @@ def model_query(context, model, *args, **kwargs): return query +def _paginate_query(context, model, limit=None, marker=None, sort_key=None, + sort_dir=None, query=None): + if not query: + query = model_query(context, model) + sort_keys = ['id'] + if sort_key and sort_key not in sort_keys: + sort_keys.insert(0, sort_key) + try: + query = sqlalchemyutils.paginate_query(query, model, limit, sort_keys, + marker=marker, + sort_dir=sort_dir) + except db_exc.InvalidSortKey: + raise exception.InvalidParameterValue( + _('The sort_key value "%(key)s" is an invalid field for sorting') + % {'key': sort_key}) + return query.all() + + def add_identity_filter(query, value): """Adds an identity filter to a query. @@ -253,11 +271,13 @@ class Connection(api.Connection): except NoResultFound: raise exception.ServerNotFound(server=server_id) - def server_get_all(self, context, project_only, filters=None): + def server_get_all(self, context, project_only, limit=None, marker=None, + sort_key=None, sort_dir=None, filters=None): query = model_query(context, models.Server, project_only=project_only) query = self._add_servers_filters(context, query, filters) - return query.all() + return _paginate_query(context, models.Server, limit, marker, + sort_key, sort_dir, query) @oslo_db_api.retry_on_deadlock def server_destroy(self, context, server_id): diff --git a/mogan/objects/server.py b/mogan/objects/server.py index f033f740..9b8be90e 100644 --- a/mogan/objects/server.py +++ b/mogan/objects/server.py @@ -133,10 +133,16 @@ class Server(base.MoganObject, object_base.VersionedObjectDictCompat): return data @classmethod - def list(cls, context, project_only=False, filters=None): + def list(cls, context, limit=None, marker=None, sort_key=None, + sort_dir=None, project_only=False, + filters=None): """Return a list of Server objects.""" db_servers = cls.dbapi.server_get_all(context, project_only=project_only, + limit=limit, + marker=marker, + sort_key=sort_key, + sort_dir=sort_dir, filters=filters) return Server._from_db_object_list(db_servers, cls, context) diff --git a/mogan/tests/unit/objects/test_server.py b/mogan/tests/unit/objects/test_server.py index 3ead2a13..f5c6e749 100644 --- a/mogan/tests/unit/objects/test_server.py +++ b/mogan/tests/unit/objects/test_server.py @@ -47,9 +47,16 @@ class TestServerObject(base.DbTestCase): mock_server_get_all.return_value = [self.fake_server] project_only = False filters = None - servers = objects.Server.list(self.context, project_only, filters) + marker = None + limit = None + sort_key = None + sort_dir = None + servers = objects.Server.list(self.context, limit, marker, + sort_key, sort_dir, + project_only, filters) mock_server_get_all.assert_called_once_with( - self.context, project_only, filters) + self.context, project_only, limit, marker, sort_key, sort_dir, + filters) self.assertIsInstance(servers[0], objects.Server) self.assertEqual(self.context, servers[0]._context)