From bfa8075f659f6fe18eb2442f2ae0722d0fd1c2ff Mon Sep 17 00:00:00 2001 From: William Wolf Date: Thu, 15 Sep 2011 10:00:38 -0400 Subject: added build_list to servers controllers and view builder and kept all old tests passing --- nova/api/openstack/servers.py | 27 +++++++++++++++++++++++---- nova/api/openstack/views/servers.py | 22 +++++++++++++++++++++- nova/tests/api/openstack/test_servers.py | 2 ++ 3 files changed, 46 insertions(+), 5 deletions(-) diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index 5affd1f33..02bce1b50 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -75,6 +75,9 @@ class Controller(object): def _build_view(self, req, instance, is_detail=False): raise NotImplementedError() + def _build_list(self, req, instances, is_detail=False): + raise NotImplementedError() + def _limit_items(self, items, req): raise NotImplementedError() @@ -130,10 +133,7 @@ class Controller(object): search_opts=search_opts) limited_list = self._limit_items(instance_list, req) - servers = [self._build_view(req, inst, is_detail)['server'] - for inst in limited_list] - - return dict(servers=servers) + return self._build_list(req, limited_list, is_detail=is_detail) @scheduler_api.redirect_handler def show(self, req, id): @@ -589,6 +589,11 @@ class ControllerV10(Controller): builder = nova.api.openstack.views.servers.ViewBuilderV10(addresses) return builder.build(instance, is_detail=is_detail) + def _build_list(self, req, instances, is_detail=False): + addresses = nova.api.openstack.views.addresses.ViewBuilderV10() + builder = nova.api.openstack.views.servers.ViewBuilderV10(addresses) + return builder.build_list(instances, is_detail=is_detail) + def _limit_items(self, items, req): return common.limited(items, req) @@ -682,6 +687,20 @@ class ControllerV11(Controller): return builder.build(instance, is_detail=is_detail) + def _build_list(self, req, instances, is_detail=False): + params = common.get_pagination_params(req) + project_id = getattr(req.environ['nova.context'], 'project_id', '') + base_url = req.application_url + flavor_builder = nova.api.openstack.views.flavors.ViewBuilderV11( + base_url, project_id) + image_builder = nova.api.openstack.views.images.ViewBuilderV11( + base_url, project_id) + addresses_builder = nova.api.openstack.views.addresses.ViewBuilderV11() + builder = nova.api.openstack.views.servers.ViewBuilderV11( + addresses_builder, flavor_builder, image_builder, + base_url, project_id) + return builder.build_list(instances, is_detail=is_detail) + def _action_change_password(self, input_dict, req, id): context = req.environ['nova.context'] if (not 'changePassword' in input_dict diff --git a/nova/api/openstack/views/servers.py b/nova/api/openstack/views/servers.py index 473dc9e7e..1228c81f7 100644 --- a/nova/api/openstack/views/servers.py +++ b/nova/api/openstack/views/servers.py @@ -40,7 +40,7 @@ class ViewBuilder(object): def __init__(self, addresses_builder): self.addresses_builder = addresses_builder - def build(self, inst, is_detail): + def build(self, inst, is_detail=False): """Return a dict that represenst a server.""" if inst.get('_is_precooked', False): server = dict(server=inst) @@ -54,6 +54,16 @@ class ViewBuilder(object): return server + def build_list(self, server_objs, is_detail=False, **kwargs): + limit = kwargs.get('limit', None) + servers = [] + servers_links = [] + + for server_obj in server_objs: + servers.append(self.build(server_obj, is_detail)['server']) + + return dict(servers=servers) + def _build_simple(self, inst): """Return a simple model of a server.""" return dict(server=dict(id=inst['id'], name=inst['display_name'])) @@ -205,6 +215,16 @@ class ViewBuilderV11(ViewBuilder): response["links"] = links + def build_list(self, server_objs, is_detail=False, **kwargs): + limit = kwargs.get('limit', None) + servers = [] + servers_links = [] + + for server_obj in server_objs: + servers.append(self.build(server_obj, is_detail)['server']) + + return dict(servers=servers) + def generate_href(self, server_id): """Create an url that refers to a specific server id.""" return os.path.join(self.base_url, self.project_id, diff --git a/nova/tests/api/openstack/test_servers.py b/nova/tests/api/openstack/test_servers.py index ee7927c64..b269b3961 100644 --- a/nova/tests/api/openstack/test_servers.py +++ b/nova/tests/api/openstack/test_servers.py @@ -2393,10 +2393,12 @@ class ServersTest(test.TestCase): def test_get_all_server_details_v1_0(self): req = webob.Request.blank('/v1.0/servers/detail') res = req.get_response(fakes.wsgi_app()) + print res.body self.assertEqual(res.status_int, 200) res_dict = json.loads(res.body) for i, s in enumerate(res_dict['servers']): + print i, ' --:-- ', s self.assertEqual(s['id'], i) self.assertEqual(s['hostId'], '') self.assertEqual(s['name'], 'server%d' % i) -- cgit From 41e8bbd9fd9daa669e0becd762c3c0e4639287ea Mon Sep 17 00:00:00 2001 From: William Wolf Date: Thu, 15 Sep 2011 10:21:33 -0400 Subject: added servers_links in v1.1 with tests --- nova/api/openstack/servers.py | 2 +- nova/api/openstack/views/servers.py | 19 +++++++- nova/tests/api/openstack/test_servers.py | 78 ++++++++++++++++++++++++++++++++ 3 files changed, 97 insertions(+), 2 deletions(-) diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index 02bce1b50..30933ca34 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -699,7 +699,7 @@ class ControllerV11(Controller): builder = nova.api.openstack.views.servers.ViewBuilderV11( addresses_builder, flavor_builder, image_builder, base_url, project_id) - return builder.build_list(instances, is_detail=is_detail) + return builder.build_list(instances, is_detail=is_detail, **params) def _action_change_password(self, input_dict, req, id): context = req.environ['nova.context'] diff --git a/nova/api/openstack/views/servers.py b/nova/api/openstack/views/servers.py index 1228c81f7..52e0850fb 100644 --- a/nova/api/openstack/views/servers.py +++ b/nova/api/openstack/views/servers.py @@ -217,13 +217,30 @@ class ViewBuilderV11(ViewBuilder): def build_list(self, server_objs, is_detail=False, **kwargs): limit = kwargs.get('limit', None) + print "BUILD LIST 11", limit servers = [] servers_links = [] for server_obj in server_objs: servers.append(self.build(server_obj, is_detail)['server']) - return dict(servers=servers) + if (len(servers) and limit) and (limit == len(servers)): + print "LIMIT SET" + next_link = self.generate_next_link(servers[-1]['id'], + limit, is_detail) + servers_links = [dict(rel='next', href=next_link)] + + reval = dict(servers=servers) + if len(servers_links) > 0: + reval['servers_links'] = servers_links + + return reval + + def generate_next_link(self, server_id, limit, is_detail=False): + """ Return an href string with proper limit and marker params""" + return "%s?limit=%s&marker=%s" % ( + os.path.join(self.base_url, self.project_id, "servers"), + limit, server_id) def generate_href(self, server_id): """Create an url that refers to a specific server id.""" diff --git a/nova/tests/api/openstack/test_servers.py b/nova/tests/api/openstack/test_servers.py index b269b3961..065c3d80f 100644 --- a/nova/tests/api/openstack/test_servers.py +++ b/nova/tests/api/openstack/test_servers.py @@ -1150,6 +1150,48 @@ class ServersTest(test.TestCase): self.assertEqual(res.status_int, 400) self.assertTrue('limit' in res.body) + def test_get_servers_with_limit_v1_1(self): + req = webob.Request.blank('/v1.1/fake/servers?limit=3') + res = req.get_response(fakes.wsgi_app()) + servers = json.loads(res.body)['servers'] + servers_links = json.loads(res.body)['servers_links'] + print res + print servers_links + self.assertEqual([s['id'] for s in servers], [0, 1, 2]) + self.assertEqual(servers_links[0]['rel'], 'next') + self.assertEqual(servers_links[0]['href'], + 'http://localhost/v1.1/fake/servers?limit=3&marker=2') + + req = webob.Request.blank('/v1.1/fake/servers?limit=aaa') + res = req.get_response(fakes.wsgi_app()) + self.assertEqual(res.status_int, 400) + self.assertTrue('limit' in res.body) + + def test_get_server_details_with_limit_v1_1(self): + req = webob.Request.blank('/v1.1/fake/servers/detail?limit=3') + res = req.get_response(fakes.wsgi_app()) + servers = json.loads(res.body)['servers'] + servers_links = json.loads(res.body)['servers_links'] + print res + print servers_links + self.fail() + self.assertEqual([s['id'] for s in servers], [0, 1, 2]) + self.assertEqual(servers_links[0]['rel'], 'next') + self.assertEqual(servers_links[0]['href'], + 'http://localhost/v1.1/fake/servers?limit=3&marker=2') + + req = webob.Request.blank('/v1.1/fake/servers/detail?limit=aaa') + res = req.get_response(fakes.wsgi_app()) + self.assertEqual(res.status_int, 400) + self.assertTrue('limit' in res.body) + + def test_get_servers_with_too_big_limit_v1_1(self): + req = webob.Request.blank('/v1.1/fake/servers?limit=30') + res = req.get_response(fakes.wsgi_app()) + res_dict = json.loads(res.body) + print res + self.assertTrue('servers_links' not in res_dict) + def test_get_servers_with_offset(self): req = webob.Request.blank('/v1.0/servers?offset=2') res = req.get_response(fakes.wsgi_app()) @@ -2428,6 +2470,42 @@ class ServersTest(test.TestCase): } req = webob.Request.blank('/v1.1/fake/servers/detail') res = req.get_response(fakes.wsgi_app()) + print res.body + self.fail() + res_dict = json.loads(res.body) + + for i, s in enumerate(res_dict['servers']): + self.assertEqual(s['id'], i) + self.assertEqual(s['hostId'], '') + self.assertEqual(s['name'], 'server%d' % i) + self.assertEqual(s['image'], expected_image) + self.assertEqual(s['flavor'], expected_flavor) + self.assertEqual(s['status'], 'BUILD') + self.assertEqual(s['metadata']['seq'], str(i)) + + def test_get_all_server_details_v1_1_with_limit(self): + expected_flavor = { + "id": "1", + "links": [ + { + "rel": "bookmark", + "href": 'http://localhost/fake/flavors/1', + }, + ], + } + expected_image = { + "id": "10", + "links": [ + { + "rel": "bookmark", + "href": 'http://localhost/fake/images/10', + }, + ], + } + req = webob.Request.blank('/v1.1/fake/servers/detail') + res = req.get_response(fakes.wsgi_app()) + print res.body + self.fail() res_dict = json.loads(res.body) for i, s in enumerate(res_dict['servers']): -- cgit From 16f3e0b4f1212ab8d34a68ad763ff6c870dbbe73 Mon Sep 17 00:00:00 2001 From: William Wolf Date: Thu, 15 Sep 2011 11:32:37 -0400 Subject: added xml support for servers_list in response with tests --- nova/api/openstack/schemas/v1.1/servers_index.rng | 3 + nova/api/openstack/servers.py | 10 ++- nova/tests/api/openstack/test_servers.py | 105 ++++++++++++++-------- 3 files changed, 79 insertions(+), 39 deletions(-) diff --git a/nova/api/openstack/schemas/v1.1/servers_index.rng b/nova/api/openstack/schemas/v1.1/servers_index.rng index 768f0912d..023e4b66a 100644 --- a/nova/api/openstack/schemas/v1.1/servers_index.rng +++ b/nova/api/openstack/schemas/v1.1/servers_index.rng @@ -9,4 +9,7 @@ + + + diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index 30933ca34..de72537a8 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -946,18 +946,22 @@ class ServerXMLSerializer(wsgi.XMLDictSerializer): 'security_group') group_elem.set('name', group['name']) - for link in server_dict.get('links', []): - elem = etree.SubElement(server_elem, + self._populate_links(server_elem, server_dict.get('links', [])) + + def _populate_links(self, parent, links): + for link in links: + elem = etree.SubElement(parent, '{%s}link' % xmlutil.XMLNS_ATOM) elem.set('rel', link['rel']) elem.set('href', link['href']) - return server_elem def index(self, servers_dict): servers = etree.Element('servers', nsmap=self.NSMAP) for server_dict in servers_dict['servers']: server = etree.SubElement(servers, 'server') self._populate_server(server, server_dict, False) + + self._populate_links(servers, servers_dict.get('servers_links', [])) return self._to_xml(servers) def detail(self, servers_dict): diff --git a/nova/tests/api/openstack/test_servers.py b/nova/tests/api/openstack/test_servers.py index 065c3d80f..8ab753906 100644 --- a/nova/tests/api/openstack/test_servers.py +++ b/nova/tests/api/openstack/test_servers.py @@ -1174,7 +1174,6 @@ class ServersTest(test.TestCase): servers_links = json.loads(res.body)['servers_links'] print res print servers_links - self.fail() self.assertEqual([s['id'] for s in servers], [0, 1, 2]) self.assertEqual(servers_links[0]['rel'], 'next') self.assertEqual(servers_links[0]['href'], @@ -2471,41 +2470,6 @@ class ServersTest(test.TestCase): req = webob.Request.blank('/v1.1/fake/servers/detail') res = req.get_response(fakes.wsgi_app()) print res.body - self.fail() - res_dict = json.loads(res.body) - - for i, s in enumerate(res_dict['servers']): - self.assertEqual(s['id'], i) - self.assertEqual(s['hostId'], '') - self.assertEqual(s['name'], 'server%d' % i) - self.assertEqual(s['image'], expected_image) - self.assertEqual(s['flavor'], expected_flavor) - self.assertEqual(s['status'], 'BUILD') - self.assertEqual(s['metadata']['seq'], str(i)) - - def test_get_all_server_details_v1_1_with_limit(self): - expected_flavor = { - "id": "1", - "links": [ - { - "rel": "bookmark", - "href": 'http://localhost/fake/flavors/1', - }, - ], - } - expected_image = { - "id": "10", - "links": [ - { - "rel": "bookmark", - "href": 'http://localhost/fake/images/10', - }, - ], - } - req = webob.Request.blank('/v1.1/fake/servers/detail') - res = req.get_response(fakes.wsgi_app()) - print res.body - self.fail() res_dict = json.loads(res.body) for i, s in enumerate(res_dict['servers']): @@ -4180,6 +4144,7 @@ class ServerXMLSerializationTest(test.TestCase): TIMESTAMP = "2010-10-11T10:30:22Z" SERVER_HREF = 'http://localhost/v1.1/servers/123' + SERVER_NEXT = 'http://localhost/v1.1/servers?limit=%s&marker=%s' SERVER_BOOKMARK = 'http://localhost/servers/123' IMAGE_BOOKMARK = 'http://localhost/images/5' FLAVOR_BOOKMARK = 'http://localhost/flavors/1' @@ -4598,6 +4563,74 @@ class ServerXMLSerializationTest(test.TestCase): for key, value in link.items(): self.assertEqual(link_nodes[i].get(key), value) + def test_index_with_servers_links(self): + serializer = servers.ServerXMLSerializer() + + expected_server_href = 'http://localhost/v1.1/servers/1' + expected_server_next = self.SERVER_NEXT % (2, 2) + expected_server_bookmark = 'http://localhost/servers/1' + expected_server_href_2 = 'http://localhost/v1.1/servers/2' + expected_server_bookmark_2 = 'http://localhost/servers/2' + fixture = {"servers": [ + { + "id": 1, + "name": "test_server", + 'links': [ + { + 'href': expected_server_href, + 'rel': 'self', + }, + { + 'href': expected_server_bookmark, + 'rel': 'bookmark', + }, + ], + }, + { + "id": 2, + "name": "test_server_2", + 'links': [ + { + 'href': expected_server_href_2, + 'rel': 'self', + }, + { + 'href': expected_server_bookmark_2, + 'rel': 'bookmark', + }, + ], + }, + ], + "servers_links": [ + { + 'rel': 'next', + 'href': expected_server_next, + }, + ]} + + output = serializer.serialize(fixture, 'index') + print output + root = etree.XML(output) + xmlutil.validate_schema(root, 'servers_index') + server_elems = root.findall('{0}server'.format(NS)) + self.assertEqual(len(server_elems), 2) + for i, server_elem in enumerate(server_elems): + server_dict = fixture['servers'][i] + for key in ['name', 'id']: + self.assertEqual(server_elem.get(key), str(server_dict[key])) + + link_nodes = server_elem.findall('{0}link'.format(ATOMNS)) + self.assertEqual(len(link_nodes), 2) + for i, link in enumerate(server_dict['links']): + for key, value in link.items(): + self.assertEqual(link_nodes[i].get(key), value) + + # Check servers_links + servers_links = root.findall('{0}link'.format(ATOMNS)) + for i, link in enumerate(fixture['servers_links']): + for key, value in link.items(): + self.assertEqual(servers_links[i].get(key), value) + def test_detail(self): serializer = servers.ServerXMLSerializer() -- cgit From 98693520d262dec4849c2aa71a090acfd37c73dd Mon Sep 17 00:00:00 2001 From: William Wolf Date: Thu, 15 Sep 2011 11:52:12 -0400 Subject: pep8 issues --- nova/api/openstack/views/servers.py | 1 - 1 file changed, 1 deletion(-) diff --git a/nova/api/openstack/views/servers.py b/nova/api/openstack/views/servers.py index 52e0850fb..8f54d58dd 100644 --- a/nova/api/openstack/views/servers.py +++ b/nova/api/openstack/views/servers.py @@ -233,7 +233,6 @@ class ViewBuilderV11(ViewBuilder): reval = dict(servers=servers) if len(servers_links) > 0: reval['servers_links'] = servers_links - return reval def generate_next_link(self, server_id, limit, is_detail=False): -- cgit From f4a453cb99bf76e17c4ec89c850f65f6c704b4fe Mon Sep 17 00:00:00 2001 From: William Wolf Date: Thu, 15 Sep 2011 14:04:50 -0400 Subject: get rid of debugs --- nova/api/openstack/views/servers.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/nova/api/openstack/views/servers.py b/nova/api/openstack/views/servers.py index 8f54d58dd..dafa2d463 100644 --- a/nova/api/openstack/views/servers.py +++ b/nova/api/openstack/views/servers.py @@ -217,7 +217,6 @@ class ViewBuilderV11(ViewBuilder): def build_list(self, server_objs, is_detail=False, **kwargs): limit = kwargs.get('limit', None) - print "BUILD LIST 11", limit servers = [] servers_links = [] @@ -225,7 +224,6 @@ class ViewBuilderV11(ViewBuilder): servers.append(self.build(server_obj, is_detail)['server']) if (len(servers) and limit) and (limit == len(servers)): - print "LIMIT SET" next_link = self.generate_next_link(servers[-1]['id'], limit, is_detail) servers_links = [dict(rel='next', href=next_link)] -- cgit From b7aff10d1aa76b764ab354cc04ae03ba8bf678e3 Mon Sep 17 00:00:00 2001 From: William Wolf Date: Thu, 15 Sep 2011 14:09:00 -0400 Subject: cleanup prints in tests --- nova/tests/api/openstack/test_servers.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/nova/tests/api/openstack/test_servers.py b/nova/tests/api/openstack/test_servers.py index 8ab753906..8eac58bc5 100644 --- a/nova/tests/api/openstack/test_servers.py +++ b/nova/tests/api/openstack/test_servers.py @@ -2434,12 +2434,10 @@ class ServersTest(test.TestCase): def test_get_all_server_details_v1_0(self): req = webob.Request.blank('/v1.0/servers/detail') res = req.get_response(fakes.wsgi_app()) - print res.body self.assertEqual(res.status_int, 200) res_dict = json.loads(res.body) for i, s in enumerate(res_dict['servers']): - print i, ' --:-- ', s self.assertEqual(s['id'], i) self.assertEqual(s['hostId'], '') self.assertEqual(s['name'], 'server%d' % i) -- cgit From fb8302c48ce41f31946045a19abb66fce516f5c7 Mon Sep 17 00:00:00 2001 From: William Wolf Date: Thu, 15 Sep 2011 14:10:26 -0400 Subject: cleanup prints in tests --- nova/tests/api/openstack/test_servers.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/nova/tests/api/openstack/test_servers.py b/nova/tests/api/openstack/test_servers.py index 8eac58bc5..1f774f8d3 100644 --- a/nova/tests/api/openstack/test_servers.py +++ b/nova/tests/api/openstack/test_servers.py @@ -1155,8 +1155,6 @@ class ServersTest(test.TestCase): res = req.get_response(fakes.wsgi_app()) servers = json.loads(res.body)['servers'] servers_links = json.loads(res.body)['servers_links'] - print res - print servers_links self.assertEqual([s['id'] for s in servers], [0, 1, 2]) self.assertEqual(servers_links[0]['rel'], 'next') self.assertEqual(servers_links[0]['href'], @@ -1172,8 +1170,6 @@ class ServersTest(test.TestCase): res = req.get_response(fakes.wsgi_app()) servers = json.loads(res.body)['servers'] servers_links = json.loads(res.body)['servers_links'] - print res - print servers_links self.assertEqual([s['id'] for s in servers], [0, 1, 2]) self.assertEqual(servers_links[0]['rel'], 'next') self.assertEqual(servers_links[0]['href'], @@ -1188,7 +1184,6 @@ class ServersTest(test.TestCase): req = webob.Request.blank('/v1.1/fake/servers?limit=30') res = req.get_response(fakes.wsgi_app()) res_dict = json.loads(res.body) - print res self.assertTrue('servers_links' not in res_dict) def test_get_servers_with_offset(self): @@ -1991,7 +1986,6 @@ class ServersTest(test.TestCase): req.headers["content-type"] = "application/json" res = req.get_response(fakes.wsgi_app()) - print res self.assertEqual(res.status_int, 202) server = json.loads(res.body)['server'] self.assertEqual(1, server['id']) -- cgit From 35bf3c005538c243351a89332f0866836f788d29 Mon Sep 17 00:00:00 2001 From: William Wolf Date: Thu, 15 Sep 2011 15:03:53 -0400 Subject: pass-through all other parameters in next links as well --- nova/api/openstack/servers.py | 7 ++++++- nova/api/openstack/views/servers.py | 10 +++++----- nova/tests/api/openstack/test_servers.py | 32 ++++++++++++++++++++++++++++---- 3 files changed, 39 insertions(+), 10 deletions(-) diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index de72537a8..966cd306a 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -688,7 +688,12 @@ class ControllerV11(Controller): return builder.build(instance, is_detail=is_detail) def _build_list(self, req, instances, is_detail=False): - params = common.get_pagination_params(req) + params = req.GET.copy() + pagination_params = common.get_pagination_params(req) + # Update params with int() values from pagination params + for key, val in pagination_params.iteritems(): + params[key] = val + project_id = getattr(req.environ['nova.context'], 'project_id', '') base_url = req.application_url flavor_builder = nova.api.openstack.views.flavors.ViewBuilderV11( diff --git a/nova/api/openstack/views/servers.py b/nova/api/openstack/views/servers.py index dafa2d463..0037fd64e 100644 --- a/nova/api/openstack/views/servers.py +++ b/nova/api/openstack/views/servers.py @@ -19,6 +19,7 @@ import datetime import hashlib import os +import urllib from nova import exception from nova.api.openstack import common @@ -225,7 +226,7 @@ class ViewBuilderV11(ViewBuilder): if (len(servers) and limit) and (limit == len(servers)): next_link = self.generate_next_link(servers[-1]['id'], - limit, is_detail) + kwargs, is_detail) servers_links = [dict(rel='next', href=next_link)] reval = dict(servers=servers) @@ -233,11 +234,10 @@ class ViewBuilderV11(ViewBuilder): reval['servers_links'] = servers_links return reval - def generate_next_link(self, server_id, limit, is_detail=False): + def generate_next_link(self, server_id, params, is_detail=False): """ Return an href string with proper limit and marker params""" - return "%s?limit=%s&marker=%s" % ( - os.path.join(self.base_url, self.project_id, "servers"), - limit, server_id) + params['marker'] = server_id + return "%s?%s" % (self.base_url, urllib.urlencode(params)) def generate_href(self, server_id): """Create an url that refers to a specific server id.""" diff --git a/nova/tests/api/openstack/test_servers.py b/nova/tests/api/openstack/test_servers.py index 1f774f8d3..9a45bfd45 100644 --- a/nova/tests/api/openstack/test_servers.py +++ b/nova/tests/api/openstack/test_servers.py @@ -20,6 +20,7 @@ import base64 import datetime import json import unittest +import urlparse from lxml import etree from xml.dom import minidom @@ -1157,8 +1158,11 @@ class ServersTest(test.TestCase): servers_links = json.loads(res.body)['servers_links'] self.assertEqual([s['id'] for s in servers], [0, 1, 2]) self.assertEqual(servers_links[0]['rel'], 'next') - self.assertEqual(servers_links[0]['href'], - 'http://localhost/v1.1/fake/servers?limit=3&marker=2') + + + qs = urlparse.urlparse(servers_links[0]['href']).query + params = urlparse.parse_qs(qs) + self.assertDictMatch({'limit': ['3'], 'marker': ['2']}, params) req = webob.Request.blank('/v1.1/fake/servers?limit=aaa') res = req.get_response(fakes.wsgi_app()) @@ -1172,8 +1176,28 @@ class ServersTest(test.TestCase): servers_links = json.loads(res.body)['servers_links'] self.assertEqual([s['id'] for s in servers], [0, 1, 2]) self.assertEqual(servers_links[0]['rel'], 'next') - self.assertEqual(servers_links[0]['href'], - 'http://localhost/v1.1/fake/servers?limit=3&marker=2') + + qs = urlparse.urlparse(servers_links[0]['href']).query + params = urlparse.parse_qs(qs) + self.assertDictMatch({'limit': ['3'], 'marker': ['2']}, params) + + req = webob.Request.blank('/v1.1/fake/servers/detail?limit=aaa') + res = req.get_response(fakes.wsgi_app()) + self.assertEqual(res.status_int, 400) + self.assertTrue('limit' in res.body) + + def test_get_server_details_with_limit_and_other_params_v1_1(self): + req = webob.Request.blank('/v1.1/fake/servers/detail?limit=3&blah=2') + res = req.get_response(fakes.wsgi_app()) + servers = json.loads(res.body)['servers'] + servers_links = json.loads(res.body)['servers_links'] + self.assertEqual([s['id'] for s in servers], [0, 1, 2]) + self.assertEqual(servers_links[0]['rel'], 'next') + + qs = urlparse.urlparse(servers_links[0]['href']).query + params = urlparse.parse_qs(qs) + self.assertDictMatch({'limit': ['3'], 'blah': ['2'], 'marker': ['2']}, + params) req = webob.Request.blank('/v1.1/fake/servers/detail?limit=aaa') res = req.get_response(fakes.wsgi_app()) -- cgit From 3b2e71962dfab872134e69a76dddeef73aedbcbf Mon Sep 17 00:00:00 2001 From: William Wolf Date: Thu, 15 Sep 2011 15:18:25 -0400 Subject: pep8 fixes --- nova/tests/api/openstack/test_servers.py | 1 - 1 file changed, 1 deletion(-) diff --git a/nova/tests/api/openstack/test_servers.py b/nova/tests/api/openstack/test_servers.py index 9a45bfd45..a05a253dc 100644 --- a/nova/tests/api/openstack/test_servers.py +++ b/nova/tests/api/openstack/test_servers.py @@ -1159,7 +1159,6 @@ class ServersTest(test.TestCase): self.assertEqual([s['id'] for s in servers], [0, 1, 2]) self.assertEqual(servers_links[0]['rel'], 'next') - qs = urlparse.urlparse(servers_links[0]['href']).query params = urlparse.parse_qs(qs) self.assertDictMatch({'limit': ['3'], 'marker': ['2']}, params) -- cgit From 5feb413f7698633c6a598ec2899772269a96b690 Mon Sep 17 00:00:00 2001 From: William Wolf Date: Mon, 19 Sep 2011 11:26:14 -0400 Subject: make our own function instead of using urllib.urlencode since we apparently don't suppor urlencoded strings yet --- nova/api/openstack/common.py | 10 ++++++++++ nova/api/openstack/views/servers.py | 2 +- nova/tests/api/openstack/test_servers.py | 6 +++--- 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/nova/api/openstack/common.py b/nova/api/openstack/common.py index ca7848678..64d7dded1 100644 --- a/nova/api/openstack/common.py +++ b/nova/api/openstack/common.py @@ -259,6 +259,16 @@ def check_img_metadata_quota_limit(context, metadata): headers={'Retry-After': 0}) +def dict_to_query_str(params): + # TODO: we should just use urllib.urlencode instead of this + # But currently we don't work with urlencoded url's + param_str = "" + for key, val in params.iteritems(): + param_str = param_str + '='.join([str(key), str(val)]) + '&' + + return param_str.rstrip('&') + + class MetadataXMLDeserializer(wsgi.XMLDeserializer): def extract_metadata(self, metadata_node): diff --git a/nova/api/openstack/views/servers.py b/nova/api/openstack/views/servers.py index 0037fd64e..49e7b6645 100644 --- a/nova/api/openstack/views/servers.py +++ b/nova/api/openstack/views/servers.py @@ -237,7 +237,7 @@ class ViewBuilderV11(ViewBuilder): def generate_next_link(self, server_id, params, is_detail=False): """ Return an href string with proper limit and marker params""" params['marker'] = server_id - return "%s?%s" % (self.base_url, urllib.urlencode(params)) + return "%s?%s" % (self.base_url, common.dict_to_query_str(params)) def generate_href(self, server_id): """Create an url that refers to a specific server id.""" diff --git a/nova/tests/api/openstack/test_servers.py b/nova/tests/api/openstack/test_servers.py index bf8bbb2b8..160335d80 100644 --- a/nova/tests/api/openstack/test_servers.py +++ b/nova/tests/api/openstack/test_servers.py @@ -1188,7 +1188,7 @@ class ServersTest(test.TestCase): self.assertTrue('limit' in res.body) def test_get_server_details_with_limit_and_other_params_v1_1(self): - req = webob.Request.blank('/v1.1/fake/servers/detail?limit=3&blah=2') + req = webob.Request.blank('/v1.1/fake/servers/detail?limit=3&blah=2:t') res = req.get_response(fakes.wsgi_app()) servers = json.loads(res.body)['servers'] servers_links = json.loads(res.body)['servers_links'] @@ -1197,8 +1197,8 @@ class ServersTest(test.TestCase): qs = urlparse.urlparse(servers_links[0]['href']).query params = urlparse.parse_qs(qs) - self.assertDictMatch({'limit': ['3'], 'blah': ['2'], 'marker': ['2']}, - params) + self.assertDictMatch({'limit': ['3'], 'blah': ['2:t'], + 'marker': ['2']}, params) req = webob.Request.blank('/v1.1/fake/servers/detail?limit=aaa') res = req.get_response(fakes.wsgi_app()) -- cgit From c7f04b68ba752766fa2fc1edb68e6fb0219606e3 Mon Sep 17 00:00:00 2001 From: William Wolf Date: Mon, 19 Sep 2011 16:03:31 -0400 Subject: remove urllib import --- nova/api/openstack/views/servers.py | 1 - 1 file changed, 1 deletion(-) diff --git a/nova/api/openstack/views/servers.py b/nova/api/openstack/views/servers.py index 49e7b6645..bb8bb0972 100644 --- a/nova/api/openstack/views/servers.py +++ b/nova/api/openstack/views/servers.py @@ -19,7 +19,6 @@ import datetime import hashlib import os -import urllib from nova import exception from nova.api.openstack import common -- cgit From a0e705359353cb6a9b0c3fc8178e714e4350c585 Mon Sep 17 00:00:00 2001 From: William Wolf Date: Mon, 19 Sep 2011 21:53:21 -0400 Subject: oops, add project_id and 'servers' to next links --- nova/api/openstack/views/servers.py | 4 +++- nova/tests/api/openstack/test_servers.py | 15 +++++++++------ 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/nova/api/openstack/views/servers.py b/nova/api/openstack/views/servers.py index bb8bb0972..61480da70 100644 --- a/nova/api/openstack/views/servers.py +++ b/nova/api/openstack/views/servers.py @@ -236,7 +236,9 @@ class ViewBuilderV11(ViewBuilder): def generate_next_link(self, server_id, params, is_detail=False): """ Return an href string with proper limit and marker params""" params['marker'] = server_id - return "%s?%s" % (self.base_url, common.dict_to_query_str(params)) + return "%s?%s" % ( + os.path.join(self.base_url, self.project_id, "servers"), + common.dict_to_query_str(params)) def generate_href(self, server_id): """Create an url that refers to a specific server id.""" diff --git a/nova/tests/api/openstack/test_servers.py b/nova/tests/api/openstack/test_servers.py index 160335d80..b3e4ad0cf 100644 --- a/nova/tests/api/openstack/test_servers.py +++ b/nova/tests/api/openstack/test_servers.py @@ -1161,8 +1161,9 @@ class ServersTest(test.TestCase): self.assertEqual([s['id'] for s in servers], [0, 1, 2]) self.assertEqual(servers_links[0]['rel'], 'next') - qs = urlparse.urlparse(servers_links[0]['href']).query - params = urlparse.parse_qs(qs) + href_parts = urlparse.urlparse(servers_links[0]['href']) + self.assertEqual('/v1.1/fake/servers', href_parts.path) + params = urlparse.parse_qs(href_parts.query) self.assertDictMatch({'limit': ['3'], 'marker': ['2']}, params) req = webob.Request.blank('/v1.1/fake/servers?limit=aaa') @@ -1178,8 +1179,9 @@ class ServersTest(test.TestCase): self.assertEqual([s['id'] for s in servers], [0, 1, 2]) self.assertEqual(servers_links[0]['rel'], 'next') - qs = urlparse.urlparse(servers_links[0]['href']).query - params = urlparse.parse_qs(qs) + href_parts = urlparse.urlparse(servers_links[0]['href']) + self.assertEqual('/v1.1/fake/servers', href_parts.path) + params = urlparse.parse_qs(href_parts.query) self.assertDictMatch({'limit': ['3'], 'marker': ['2']}, params) req = webob.Request.blank('/v1.1/fake/servers/detail?limit=aaa') @@ -1195,8 +1197,9 @@ class ServersTest(test.TestCase): self.assertEqual([s['id'] for s in servers], [0, 1, 2]) self.assertEqual(servers_links[0]['rel'], 'next') - qs = urlparse.urlparse(servers_links[0]['href']).query - params = urlparse.parse_qs(qs) + href_parts = urlparse.urlparse(servers_links[0]['href']) + self.assertEqual('/v1.1/fake/servers', href_parts.path) + params = urlparse.parse_qs(href_parts.query) self.assertDictMatch({'limit': ['3'], 'blah': ['2:t'], 'marker': ['2']}, params) -- cgit From 2fdc37c21ee9c6533cf7452e4347a9fa9212c31d Mon Sep 17 00:00:00 2001 From: William Wolf Date: Tue, 20 Sep 2011 14:43:50 -0400 Subject: pep8 fixes --- nova/tests/api/openstack/test_servers.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/nova/tests/api/openstack/test_servers.py b/nova/tests/api/openstack/test_servers.py index cccbafcde..81ec64359 100644 --- a/nova/tests/api/openstack/test_servers.py +++ b/nova/tests/api/openstack/test_servers.py @@ -1162,7 +1162,7 @@ class ServersTest(test.TestCase): self.assertEqual(servers_links[0]['rel'], 'next') href_parts = urlparse.urlparse(servers_links[0]['href']) - self.assertEqual('/v1.1/fake/servers', href_parts.path) + self.assertEqual('/v1.1/fake/servers', href_parts.path) params = urlparse.parse_qs(href_parts.query) self.assertDictMatch({'limit': ['3'], 'marker': ['2']}, params) @@ -1180,7 +1180,7 @@ class ServersTest(test.TestCase): self.assertEqual(servers_links[0]['rel'], 'next') href_parts = urlparse.urlparse(servers_links[0]['href']) - self.assertEqual('/v1.1/fake/servers', href_parts.path) + self.assertEqual('/v1.1/fake/servers', href_parts.path) params = urlparse.parse_qs(href_parts.query) self.assertDictMatch({'limit': ['3'], 'marker': ['2']}, params) @@ -1198,7 +1198,7 @@ class ServersTest(test.TestCase): self.assertEqual(servers_links[0]['rel'], 'next') href_parts = urlparse.urlparse(servers_links[0]['href']) - self.assertEqual('/v1.1/fake/servers', href_parts.path) + self.assertEqual('/v1.1/fake/servers', href_parts.path) params = urlparse.parse_qs(href_parts.query) self.assertDictMatch({'limit': ['3'], 'blah': ['2:t'], 'marker': ['2']}, params) -- cgit