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

8 files changed, 104 insertions, 9 deletions

diff --git a/api-ref/source/v1/parameters.yaml b/api-ref/source/v1/parameters.yaml
index 95350e8..1c1d8a8 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 423dd88..4a4b179 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 c9750c3..4ff10e5 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 f79331d..89a4f62 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 33af720..8563ffc 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 1399ffc..0efc789 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 f033f74..9b8be90 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 3ead2a1..f5c6e74 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)