diff options
author | Xinran <xin-ran.wang@intel.com> | 2017-10-24 10:13:48 +0800 |
---|---|---|
committer | Xinran WANG <xin-ran.wang@intel.com> | 2017-10-26 07:40:05 +0000 |
commit | 05cc14d39997cf13f43298852824fbca3c6a3ddf (patch) | |
tree | aed20b7c8bce49e8b2aeca7bc6b525288eaa33e4 | |
parent | e2b2d7b42cec8d2a34b479f473358ef32214e847 (diff) |
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
Notes
Notes (review):
Code-Review+2: Zhenguo Niu <Niu.ZGlinux@gmail.com>
Code-Review+2: ShaoHe Feng <shaohe.feng@intel.com>
Workflow+1: ShaoHe Feng <shaohe.feng@intel.com>
Verified+2: Zuul
Submitted-by: Zuul
Submitted-at: Sat, 28 Oct 2017 16:55:05 +0000
Reviewed-on: https://review.openstack.org/514506
Project: openstack/mogan
Branch: refs/heads/master
-rw-r--r-- | api-ref/source/v1/parameters.yaml | 36 | ||||
-rw-r--r-- | api-ref/source/v1/servers.inc | 8 | ||||
-rw-r--r-- | mogan/api/controllers/v1/servers.py | 22 | ||||
-rw-r--r-- | mogan/api/controllers/v1/utils.py | 1 | ||||
-rw-r--r-- | mogan/db/api.py | 3 | ||||
-rw-r--r-- | mogan/db/sqlalchemy/api.py | 24 | ||||
-rw-r--r-- | mogan/objects/server.py | 8 | ||||
-rw-r--r-- | 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 95350e8..1c1d8a8 100644 --- a/api-ref/source/v1/parameters.yaml +++ b/api-ref/source/v1/parameters.yaml | |||
@@ -122,12 +122,48 @@ image_query: | |||
122 | in: query | 122 | in: query |
123 | required: false | 123 | required: false |
124 | type: string | 124 | type: string |
125 | limit: | ||
126 | description: | | ||
127 | Requests a page size of items. Returns a number of items up to a limit value. | ||
128 | Use the ``limit`` parameter to make an initial limited request and use the ID | ||
129 | of the last-seen item from the response as the ``marker`` parameter value in a | ||
130 | subsequent limited request. | ||
131 | in: query | ||
132 | required: false | ||
133 | type: integer | ||
134 | marker: | ||
135 | description: | | ||
136 | The ID of the last-seen item. Use the ``limit`` parameter to make an initial limited | ||
137 | request and use the ID of the last-seen item from the response as the ``marker`` | ||
138 | parameter value in a subsequent limited request. | ||
139 | in: query | ||
140 | required: false | ||
141 | type: string | ||
125 | server_name_query: | 142 | server_name_query: |
126 | description: | | 143 | description: | |
127 | Filters the server list by name. Users can filter by prefix of server's name. | 144 | Filters the server list by name. Users can filter by prefix of server's name. |
128 | in: query | 145 | in: query |
129 | required: false | 146 | required: false |
130 | type: string | 147 | type: string |
148 | sort_dir: | ||
149 | description: | | ||
150 | Sort direction. A valid value is ``asc`` (ascending) or ``desc`` (descending). | ||
151 | Default is ``asc``. You can specify multiple pairs of sort key and sort direction | ||
152 | query parameters. If you omit the sort direction in a pair, the API uses the natural | ||
153 | sorting direction of the direction of the server ``sort_key`` attribute. | ||
154 | in: query | ||
155 | required: false | ||
156 | type: string | ||
157 | sort_key: | ||
158 | description: | | ||
159 | Sorts the response by the this attribute value. | ||
160 | Default is ``id``. You can specify multiple pairs of sort key and | ||
161 | sort direction query parameters. If you omit the sort direction in | ||
162 | a pair, the API uses the natural sorting direction of the server | ||
163 | attribute that is provided as the ``sort_key``. | ||
164 | in: query | ||
165 | required: false | ||
166 | type: string | ||
131 | status_query: | 167 | status_query: |
132 | description: | | 168 | description: | |
133 | Filters the server list by the server's status. | 169 | 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 | |||
157 | - flavor_name: flavor_name_query | 157 | - flavor_name: flavor_name_query |
158 | - image_uuid: image_query | 158 | - image_uuid: image_query |
159 | - ip: fixed_ip_query | 159 | - ip: fixed_ip_query |
160 | - limit: limit | ||
161 | - marker: marker | ||
162 | - sort_key: sort_key | ||
163 | - sort_dir: sort_dir | ||
160 | - all_tenants: all_tenants | 164 | - all_tenants: all_tenants |
161 | - fields: fields | 165 | - fields: fields |
162 | 166 | ||
@@ -202,6 +206,10 @@ Request | |||
202 | - flavor_name: flavor_name_query | 206 | - flavor_name: flavor_name_query |
203 | - image_uuid: image_query | 207 | - image_uuid: image_query |
204 | - ip: fixed_ip_query | 208 | - ip: fixed_ip_query |
209 | - limit: limit | ||
210 | - marker: marker | ||
211 | - sort_key: sort_key | ||
212 | - sort_dir: sort_dir | ||
205 | - all_tenants: all_tenants | 213 | - all_tenants: all_tenants |
206 | 214 | ||
207 | 215 | ||
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): | |||
568 | def _get_server_collection(self, name=None, status=None, | 568 | def _get_server_collection(self, name=None, status=None, |
569 | flavor_uuid=None, flavor_name=None, | 569 | flavor_uuid=None, flavor_name=None, |
570 | image_uuid=None, ip=None, | 570 | image_uuid=None, ip=None, |
571 | limit=None, marker=None, | ||
572 | sort_key=None, sort_dir=None, | ||
571 | all_tenants=None, fields=None): | 573 | all_tenants=None, fields=None): |
572 | context = pecan.request.context | 574 | context = pecan.request.context |
573 | project_only = True | 575 | project_only = True |
574 | if context.is_admin and all_tenants: | 576 | if context.is_admin and all_tenants: |
575 | project_only = False | 577 | project_only = False |
576 | 578 | ||
579 | limit = api_utils.validate_limit(limit) | ||
580 | |||
581 | marker_obj = None | ||
582 | if marker: | ||
583 | marker_obj = objects.Server.get(pecan.request.context, marker) | ||
584 | |||
577 | filters = {} | 585 | filters = {} |
578 | if name: | 586 | if name: |
579 | filters['name'] = name | 587 | filters['name'] = name |
@@ -587,6 +595,8 @@ class ServerController(ServerControllerBase): | |||
587 | filters['image_uuid'] = image_uuid | 595 | filters['image_uuid'] = image_uuid |
588 | 596 | ||
589 | servers = objects.Server.list(pecan.request.context, | 597 | servers = objects.Server.list(pecan.request.context, |
598 | limit, marker_obj, | ||
599 | sort_key=sort_key, sort_dir=sort_dir, | ||
590 | project_only=project_only, | 600 | project_only=project_only, |
591 | filters=filters) | 601 | filters=filters) |
592 | if ip: | 602 | if ip: |
@@ -617,10 +627,12 @@ class ServerController(ServerControllerBase): | |||
617 | 627 | ||
618 | @expose.expose(ServerCollection, wtypes.text, wtypes.text, | 628 | @expose.expose(ServerCollection, wtypes.text, wtypes.text, |
619 | types.uuid, wtypes.text, types.uuid, wtypes.text, | 629 | types.uuid, wtypes.text, types.uuid, wtypes.text, |
620 | types.listtype, types.boolean) | 630 | int, types.uuid, types.listtype, wtypes.text, |
631 | wtypes.text, types.boolean) | ||
621 | def get_all(self, name=None, status=None, | 632 | def get_all(self, name=None, status=None, |
622 | flavor_uuid=None, flavor_name=None, image_uuid=None, ip=None, | 633 | flavor_uuid=None, flavor_name=None, image_uuid=None, ip=None, |
623 | fields=None, all_tenants=None): | 634 | limit=None, marker=None, fields=None, sort_key='id', |
635 | sort_dir='asc', all_tenants=None): | ||
624 | """Retrieve a list of server. | 636 | """Retrieve a list of server. |
625 | 637 | ||
626 | :param fields: Optional, a list with a specified set of fields | 638 | :param fields: Optional, a list with a specified set of fields |
@@ -635,6 +647,8 @@ class ServerController(ServerControllerBase): | |||
635 | return self._get_server_collection(name, status, | 647 | return self._get_server_collection(name, status, |
636 | flavor_uuid, flavor_name, | 648 | flavor_uuid, flavor_name, |
637 | image_uuid, ip, | 649 | image_uuid, ip, |
650 | limit, marker, | ||
651 | sort_key, sort_dir, | ||
638 | all_tenants=all_tenants, | 652 | all_tenants=all_tenants, |
639 | fields=fields) | 653 | fields=fields) |
640 | 654 | ||
@@ -654,9 +668,11 @@ class ServerController(ServerControllerBase): | |||
654 | 668 | ||
655 | @expose.expose(ServerCollection, wtypes.text, wtypes.text, | 669 | @expose.expose(ServerCollection, wtypes.text, wtypes.text, |
656 | types.uuid, wtypes.text, types.uuid, wtypes.text, | 670 | types.uuid, wtypes.text, types.uuid, wtypes.text, |
671 | int, types.uuid, wtypes.text, wtypes.text, | ||
657 | types.boolean) | 672 | types.boolean) |
658 | def detail(self, name=None, status=None, | 673 | def detail(self, name=None, status=None, |
659 | flavor_uuid=None, flavor_name=None, image_uuid=None, ip=None, | 674 | flavor_uuid=None, flavor_name=None, image_uuid=None, ip=None, |
675 | limit=None, marker=None, sort_key='id', sort_dir='asc', | ||
660 | all_tenants=None): | 676 | all_tenants=None): |
661 | """Retrieve detail of a list of servers.""" | 677 | """Retrieve detail of a list of servers.""" |
662 | # /detail should only work against collections | 678 | # /detail should only work against collections |
@@ -668,6 +684,8 @@ class ServerController(ServerControllerBase): | |||
668 | return self._get_server_collection(name, status, | 684 | return self._get_server_collection(name, status, |
669 | flavor_uuid, flavor_name, | 685 | flavor_uuid, flavor_name, |
670 | image_uuid, ip, | 686 | image_uuid, ip, |
687 | limit, marker, | ||
688 | sort_key, sort_dir, | ||
671 | all_tenants=all_tenants) | 689 | all_tenants=all_tenants) |
672 | 690 | ||
673 | @policy.authorize_wsgi("mogan:server", "create", False) | 691 | @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, | |||
31 | def validate_limit(limit): | 31 | def validate_limit(limit): |
32 | if limit is None: | 32 | if limit is None: |
33 | return CONF.api.max_limit | 33 | return CONF.api.max_limit |
34 | |||
35 | if limit <= 0: | 34 | if limit <= 0: |
36 | raise wsme.exc.ClientSideError(_("Limit must be positive")) | 35 | raise wsme.exc.ClientSideError(_("Limit must be positive")) |
37 | 36 | ||
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): | |||
72 | """Get server by name.""" | 72 | """Get server by name.""" |
73 | 73 | ||
74 | @abc.abstractmethod | 74 | @abc.abstractmethod |
75 | def server_get_all(self, context, project_only, filters=None): | 75 | def server_get_all(self, context, project_only, limit=None, marker=None, |
76 | sort_key=None, sort_dir=None, filters=None): | ||
76 | """Get all servers.""" | 77 | """Get all servers.""" |
77 | 78 | ||
78 | @abc.abstractmethod | 79 | @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): | |||
84 | return query | 84 | return query |
85 | 85 | ||
86 | 86 | ||
87 | def _paginate_query(context, model, limit=None, marker=None, sort_key=None, | ||
88 | sort_dir=None, query=None): | ||
89 | if not query: | ||
90 | query = model_query(context, model) | ||
91 | sort_keys = ['id'] | ||
92 | if sort_key and sort_key not in sort_keys: | ||
93 | sort_keys.insert(0, sort_key) | ||
94 | try: | ||
95 | query = sqlalchemyutils.paginate_query(query, model, limit, sort_keys, | ||
96 | marker=marker, | ||
97 | sort_dir=sort_dir) | ||
98 | except db_exc.InvalidSortKey: | ||
99 | raise exception.InvalidParameterValue( | ||
100 | _('The sort_key value "%(key)s" is an invalid field for sorting') | ||
101 | % {'key': sort_key}) | ||
102 | return query.all() | ||
103 | |||
104 | |||
87 | def add_identity_filter(query, value): | 105 | def add_identity_filter(query, value): |
88 | """Adds an identity filter to a query. | 106 | """Adds an identity filter to a query. |
89 | 107 | ||
@@ -253,11 +271,13 @@ class Connection(api.Connection): | |||
253 | except NoResultFound: | 271 | except NoResultFound: |
254 | raise exception.ServerNotFound(server=server_id) | 272 | raise exception.ServerNotFound(server=server_id) |
255 | 273 | ||
256 | def server_get_all(self, context, project_only, filters=None): | 274 | def server_get_all(self, context, project_only, limit=None, marker=None, |
275 | sort_key=None, sort_dir=None, filters=None): | ||
257 | query = model_query(context, models.Server, | 276 | query = model_query(context, models.Server, |
258 | project_only=project_only) | 277 | project_only=project_only) |
259 | query = self._add_servers_filters(context, query, filters) | 278 | query = self._add_servers_filters(context, query, filters) |
260 | return query.all() | 279 | return _paginate_query(context, models.Server, limit, marker, |
280 | sort_key, sort_dir, query) | ||
261 | 281 | ||
262 | @oslo_db_api.retry_on_deadlock | 282 | @oslo_db_api.retry_on_deadlock |
263 | def server_destroy(self, context, server_id): | 283 | 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): | |||
133 | return data | 133 | return data |
134 | 134 | ||
135 | @classmethod | 135 | @classmethod |
136 | def list(cls, context, project_only=False, filters=None): | 136 | def list(cls, context, limit=None, marker=None, sort_key=None, |
137 | sort_dir=None, project_only=False, | ||
138 | filters=None): | ||
137 | """Return a list of Server objects.""" | 139 | """Return a list of Server objects.""" |
138 | db_servers = cls.dbapi.server_get_all(context, | 140 | db_servers = cls.dbapi.server_get_all(context, |
139 | project_only=project_only, | 141 | project_only=project_only, |
142 | limit=limit, | ||
143 | marker=marker, | ||
144 | sort_key=sort_key, | ||
145 | sort_dir=sort_dir, | ||
140 | filters=filters) | 146 | filters=filters) |
141 | return Server._from_db_object_list(db_servers, cls, context) | 147 | return Server._from_db_object_list(db_servers, cls, context) |
142 | 148 | ||
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): | |||
47 | mock_server_get_all.return_value = [self.fake_server] | 47 | mock_server_get_all.return_value = [self.fake_server] |
48 | project_only = False | 48 | project_only = False |
49 | filters = None | 49 | filters = None |
50 | servers = objects.Server.list(self.context, project_only, filters) | 50 | marker = None |
51 | limit = None | ||
52 | sort_key = None | ||
53 | sort_dir = None | ||
54 | servers = objects.Server.list(self.context, limit, marker, | ||
55 | sort_key, sort_dir, | ||
56 | project_only, filters) | ||
51 | mock_server_get_all.assert_called_once_with( | 57 | mock_server_get_all.assert_called_once_with( |
52 | self.context, project_only, filters) | 58 | self.context, project_only, limit, marker, sort_key, sort_dir, |
59 | filters) | ||
53 | self.assertIsInstance(servers[0], objects.Server) | 60 | self.assertIsInstance(servers[0], objects.Server) |
54 | self.assertEqual(self.context, servers[0]._context) | 61 | self.assertEqual(self.context, servers[0]._context) |
55 | 62 | ||