From acb7a7355055e04b9bb05fbba5f6590e57d681fa Mon Sep 17 00:00:00 2001 From: Naveed Massjouni Date: Fri, 18 Mar 2011 00:18:55 -0400 Subject: Support for markers for pagination as defined in the 1.1 spec. --- nova/api/openstack/common.py | 28 ++++++++++++++++++++++++++++ nova/api/openstack/servers.py | 8 +++++++- nova/tests/api/openstack/test_servers.py | 7 +++++++ 3 files changed, 42 insertions(+), 1 deletion(-) diff --git a/nova/api/openstack/common.py b/nova/api/openstack/common.py index b224cbfb4..8106f841b 100644 --- a/nova/api/openstack/common.py +++ b/nova/api/openstack/common.py @@ -55,6 +55,34 @@ def limited(items, request, max_limit=1000): return items[offset:range_end] +def limited_by_marker(items, request, max_limit=1000): + ''' Return a slice of items according to requested marker and limit. ''' + + marker = request.GET.get('marker') + + try: + limit = int(request.GET.get('limit', max_limit)) + except ValueError: + raise webob.exc.HTTPBadRequest(_('limit param must be an integer')) + + if limit < 0: + raise webob.exc.HTTPBadRequest(_('limit param must be positive')) + + limit = min(max_limit, limit or max_limit) + start_index = 0 + if marker != None: + found_it = False + for i, item in enumerate(items): + if str(item['id']) == marker: + start_index = i + found_it = True + break + if not found_it: + raise webob.exc.HTTPBadRequest(_('marker not found')) + range_end = start_index + limit + return items[start_index:range_end] + + def get_image_id_from_image_hash(image_service, context, image_hash): """Given an Image ID Hash, return an objectstore Image ID. diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index e3141934b..461bf5989 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -81,7 +81,7 @@ class Controller(wsgi.Controller): builder - the response model builder """ instance_list = self.compute_api.get_all(req.environ['nova.context']) - limited_list = common.limited(instance_list, req) + limited_list = self._limit_items(instance_list, req) builder = self._get_view_builder(req) servers = [builder.build(inst, is_detail)['server'] for inst in limited_list] @@ -528,6 +528,9 @@ class ControllerV10(Controller): def _get_addresses_view_builder(self, req): return nova.api.openstack.views.addresses.ViewBuilderV10(req) + def _limit_items(self, items, req): + return common.limited(items, req) + class ControllerV11(Controller): def _image_id_from_req_data(self, data): @@ -551,6 +554,9 @@ class ControllerV11(Controller): def _get_addresses_view_builder(self, req): return nova.api.openstack.views.addresses.ViewBuilderV11(req) + def _limit_items(self, items, req): + return common.limited_by_marker(items, req) + class ServerCreateRequestXMLDeserializer(object): """ diff --git a/nova/tests/api/openstack/test_servers.py b/nova/tests/api/openstack/test_servers.py index 6e78db9da..e1cadcef6 100644 --- a/nova/tests/api/openstack/test_servers.py +++ b/nova/tests/api/openstack/test_servers.py @@ -239,6 +239,13 @@ class ServersTest(test.TestCase): servers = json.loads(res.body)['servers'] self.assertEqual([s['id'] for s in servers], [1, 2]) + def test_get_servers_with_marker(self): + req = webob.Request.blank('/v1.1/servers?marker=2') + res = req.get_response(fakes.wsgi_app()) + print 'body:', res.body + servers = json.loads(res.body)['servers'] + self.assertEqual([s['id'] for s in servers], [2, 3, 4]) + def _setup_for_create_instance(self): """Shared implementation for tests below that create instance""" def instance_create(context, inst): -- cgit From 1ad0faf980ac89e904a246f1dfeddf51a21fd740 Mon Sep 17 00:00:00 2001 From: Naveed Massjouni Date: Thu, 24 Mar 2011 13:48:04 -0400 Subject: Paginated results should not include the item starting at marker. Improved implementation of common.limited_by_marker as suggested by Matt Dietz. Added flag osapi_max_limit. --- nova/api/openstack/common.py | 28 ++++++++++++++++------------ nova/flags.py | 2 ++ nova/tests/api/openstack/test_servers.py | 3 +-- 3 files changed, 19 insertions(+), 14 deletions(-) diff --git a/nova/api/openstack/common.py b/nova/api/openstack/common.py index f85d1df9d..f598ac824 100644 --- a/nova/api/openstack/common.py +++ b/nova/api/openstack/common.py @@ -20,9 +20,11 @@ from urlparse import urlparse import webob from nova import exception +from nova import flags +FLAGS = flags.FLAGS -def limited(items, request, max_limit=1000): +def limited(items, request, max_limit=FLAGS.osapi_max_limit): """ Return a slice of items according to requested offset and limit. @@ -56,10 +58,13 @@ def limited(items, request, max_limit=1000): return items[offset:range_end] -def limited_by_marker(items, request, max_limit=1000): - ''' Return a slice of items according to requested marker and limit. ''' +def limited_by_marker(items, request, max_limit=FLAGS.osapi_max_limit): + """Return a slice of items according to the requested marker and limit.""" - marker = request.GET.get('marker') + try: + marker = int(request.GET.get('marker', 0)) + except ValueError: + raise webob.exc.HTTPBadRequest(_('marker param must be an integer')) try: limit = int(request.GET.get('limit', max_limit)) @@ -69,17 +74,16 @@ def limited_by_marker(items, request, max_limit=1000): if limit < 0: raise webob.exc.HTTPBadRequest(_('limit param must be positive')) - limit = min(max_limit, limit or max_limit) + limit = min(max_limit, limit) start_index = 0 - if marker != None: - found_it = False + if marker: + start_index = -1 for i, item in enumerate(items): - if str(item['id']) == marker: - start_index = i - found_it = True + if item['id'] == marker: + start_index = i + 1 break - if not found_it: - raise webob.exc.HTTPBadRequest(_('marker not found')) + if start_index < 0: + raise webob.exc.HTTPBadRequest(_('marker [%s] not found' % marker)) range_end = start_index + limit return items[start_index:range_end] diff --git a/nova/flags.py b/nova/flags.py index 9123e9ac7..d1817dc3b 100644 --- a/nova/flags.py +++ b/nova/flags.py @@ -302,6 +302,8 @@ DEFINE_string('osapi_host', '$my_ip', 'ip of api server') DEFINE_string('osapi_scheme', 'http', 'prefix for openstack') DEFINE_integer('osapi_port', 8774, 'OpenStack API port') DEFINE_string('osapi_path', '/v1.0/', 'suffix for openstack') +DEFINE_integer('osapi_max_limit', 1000, + 'max number of items returned in a collection response') DEFINE_string('default_project', 'openstack', 'default project for openstack') DEFINE_string('default_image', 'ami-11111', diff --git a/nova/tests/api/openstack/test_servers.py b/nova/tests/api/openstack/test_servers.py index 09b08ce8d..cfed78b90 100644 --- a/nova/tests/api/openstack/test_servers.py +++ b/nova/tests/api/openstack/test_servers.py @@ -242,9 +242,8 @@ class ServersTest(test.TestCase): def test_get_servers_with_marker(self): req = webob.Request.blank('/v1.1/servers?marker=2') res = req.get_response(fakes.wsgi_app()) - print 'body:', res.body servers = json.loads(res.body)['servers'] - self.assertEqual([s['id'] for s in servers], [2, 3, 4]) + self.assertEqual([s['id'] for s in servers], [3, 4]) def _setup_for_create_instance(self): """Shared implementation for tests below that create instance""" -- cgit From c50e6c3879109d2e2e0c2f6b9c42195e9559993d Mon Sep 17 00:00:00 2001 From: Naveed Massjouni Date: Thu, 24 Mar 2011 16:18:50 -0400 Subject: Added test_get_servers_with_limit_and_marker to test pagination with marker and limit request params. --- nova/tests/api/openstack/test_servers.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/nova/tests/api/openstack/test_servers.py b/nova/tests/api/openstack/test_servers.py index cfed78b90..c3ece939e 100644 --- a/nova/tests/api/openstack/test_servers.py +++ b/nova/tests/api/openstack/test_servers.py @@ -245,6 +245,12 @@ class ServersTest(test.TestCase): servers = json.loads(res.body)['servers'] self.assertEqual([s['id'] for s in servers], [3, 4]) + def test_get_servers_with_limit_and_marker(self): + req = webob.Request.blank('/v1.1/servers?limit=2&marker=1') + res = req.get_response(fakes.wsgi_app()) + servers = json.loads(res.body)['servers'] + self.assertEqual([s['id'] for s in servers], [2, 3]) + def _setup_for_create_instance(self): """Shared implementation for tests below that create instance""" def instance_create(context, inst): -- cgit From a6174e64b541560989c305b50787c96fb5890679 Mon Sep 17 00:00:00 2001 From: Naveed Massjouni Date: Thu, 24 Mar 2011 16:31:04 -0400 Subject: Added test_get_servers_with_bad_limit, test_get_servers_with_bad_offset and test_get_servers_with_bad_marker. --- nova/tests/api/openstack/test_servers.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/nova/tests/api/openstack/test_servers.py b/nova/tests/api/openstack/test_servers.py index c3ece939e..c48cc5179 100644 --- a/nova/tests/api/openstack/test_servers.py +++ b/nova/tests/api/openstack/test_servers.py @@ -239,6 +239,18 @@ class ServersTest(test.TestCase): servers = json.loads(res.body)['servers'] self.assertEqual([s['id'] for s in servers], [1, 2]) + def test_get_servers_with_bad_limit(self): + req = webob.Request.blank('/v1.0/servers?limit=asdf&offset=1') + res = req.get_response(fakes.wsgi_app()) + self.assertEqual(res.status_int, 400) + self.assertTrue(res.body.find('limit param') > -1) + + def test_get_servers_with_bad_offset(self): + req = webob.Request.blank('/v1.0/servers?limit=2&offset=asdf') + res = req.get_response(fakes.wsgi_app()) + self.assertEqual(res.status_int, 400) + self.assertTrue(res.body.find('offset param') > -1) + def test_get_servers_with_marker(self): req = webob.Request.blank('/v1.1/servers?marker=2') res = req.get_response(fakes.wsgi_app()) @@ -251,6 +263,12 @@ class ServersTest(test.TestCase): servers = json.loads(res.body)['servers'] self.assertEqual([s['id'] for s in servers], [2, 3]) + def test_get_servers_with_bad_marker(self): + req = webob.Request.blank('/v1.1/servers?limit=2&marker=asdf') + res = req.get_response(fakes.wsgi_app()) + self.assertEqual(res.status_int, 400) + self.assertTrue(res.body.find('marker param') > -1) + def _setup_for_create_instance(self): """Shared implementation for tests below that create instance""" def instance_create(context, inst): -- cgit