From 59c2862d6561b4a2fed807f5e120dc50867aae1b Mon Sep 17 00:00:00 2001 From: Lucas Alvares Gomes Date: Mon, 23 Sep 2013 11:32:09 +0100 Subject: [PATCH] Collection named based on resource type To be more consistent with other existing OpenStack APIs, the collection name is now based on the requested resource type. Also, the link to the next subset of the collection is now an attribute on the root document of the request body. Change-Id: Ie0f99d975b691aad7cd39fddd7d141f95c7912f8 Closes-Bug: #1227928 --- ironic/api/controllers/v1/chassis.py | 19 +++++------ ironic/api/controllers/v1/collection.py | 42 ++++++++++++------------- ironic/api/controllers/v1/node.py | 19 +++++------ ironic/api/controllers/v1/port.py | 10 +++--- ironic/tests/api/test_chassis.py | 30 ++++++++---------- ironic/tests/api/test_nodes.py | 26 +++++++-------- ironic/tests/api/test_ports.py | 16 +++++----- 7 files changed, 77 insertions(+), 85 deletions(-) diff --git a/ironic/api/controllers/v1/chassis.py b/ironic/api/controllers/v1/chassis.py index 73f32c22d1..30eafea421 100644 --- a/ironic/api/controllers/v1/chassis.py +++ b/ironic/api/controllers/v1/chassis.py @@ -91,15 +91,17 @@ class Chassis(base.APIBase): class ChassisCollection(collection.Collection): """API representation of a collection of chassis.""" - items = [Chassis] + chassis = [Chassis] "A list containing chassis objects" + def __init__(self, **kwargs): + self._type = 'chassis' + @classmethod def convert_with_links(cls, chassis, limit, **kwargs): collection = ChassisCollection() - collection.type = 'chassis' - collection.items = [Chassis.convert_with_links(ch) for ch in chassis] - collection.links = collection.make_links(limit, 'chassis', **kwargs) + collection.chassis = [Chassis.convert_with_links(ch) for ch in chassis] + collection.next = collection.get_next(limit, **kwargs) return collection @@ -200,10 +202,9 @@ class ChassisController(rest.RestController): sort_key=sort_key, sort_dir=sort_dir) collection = node.NodeCollection() - collection.type = 'node' - collection.items = [node.Node.convert_with_links(n) for n in nodes] + collection.nodes = [node.Node.convert_with_links(n) for n in nodes] resource_url = '/'.join(['chassis', chassis_uuid, 'nodes']) - collection.links = collection.make_links(limit, resource_url, - sort_key=sort_key, - sort_dir=sort_dir) + collection.next = collection.get_next(limit, url=resource_url, + sort_key=sort_key, + sort_dir=sort_dir) return collection diff --git a/ironic/api/controllers/v1/collection.py b/ironic/api/controllers/v1/collection.py index 1ce88c96da..10f2ff52d4 100644 --- a/ironic/api/controllers/v1/collection.py +++ b/ironic/api/controllers/v1/collection.py @@ -25,29 +25,27 @@ from ironic.api.controllers.v1 import link class Collection(base.APIBase): - links = [link.Link] - "A list containing a link to retrieve the next subset of the collection" + next = wtypes.text + "A link to retrieve the next subset of the collection" - type = wtypes.text - "The type of the collection" - - def _check_items(self): - if not hasattr(self, 'items') or self.items == wtypes.Unset: - raise AttributeError(_("Collection items are uninitialized")) + @property + def collection(self): + return getattr(self, self._type) def has_next(self, limit): - self._check_items() - return len(self.items) and len(self.items) == limit + """Return whether collection has more items.""" + return len(self.collection) and len(self.collection) == limit - def make_links(self, limit, res_name, **kwargs): - self._check_items() - links = [] - if self.has_next(limit): - q_args = ''.join(['%s=%s&' % (key, kwargs[key]) for key in kwargs]) - next_args = '?%(args)slimit=%(limit)d&marker=%(marker)s' % { - 'args': q_args, 'limit': limit, - 'marker': self.items[-1].uuid} - links = [link.Link.make_link('next', pecan.request.host_url, - res_name, next_args) - ] - return links + def get_next(self, limit, url=None, **kwargs): + """Return a link to the next subset of the collection.""" + if not self.has_next(limit): + return wtypes.Unset + + resource_url = url or self._type + q_args = ''.join(['%s=%s&' % (key, kwargs[key]) for key in kwargs]) + next_args = '?%(args)slimit=%(limit)d&marker=%(marker)s' % { + 'args': q_args, 'limit': limit, + 'marker': self.collection[-1].uuid} + + return link.Link.make_link('next', pecan.request.host_url, + resource_url, next_args).href diff --git a/ironic/api/controllers/v1/node.py b/ironic/api/controllers/v1/node.py index b87108a118..54c8a22e75 100644 --- a/ironic/api/controllers/v1/node.py +++ b/ironic/api/controllers/v1/node.py @@ -245,15 +245,17 @@ class Node(base.APIBase): class NodeCollection(collection.Collection): """API representation of a collection of nodes.""" - items = [Node] + nodes = [Node] "A list containing nodes objects" + def __init__(self, **kwargs): + self._type = 'nodes' + @classmethod def convert_with_links(cls, nodes, limit, **kwargs): collection = NodeCollection() - collection.type = 'node' - collection.items = [Node.convert_with_links(n) for n in nodes] - collection.links = collection.make_links(limit, 'nodes', **kwargs) + collection.nodes = [Node.convert_with_links(n) for n in nodes] + collection.next = collection.get_next(limit, **kwargs) return collection @@ -425,10 +427,9 @@ class NodesController(rest.RestController): sort_key=sort_key, sort_dir=sort_dir) collection = port.PortCollection() - collection.type = 'port' - collection.items = [port.Port.convert_with_links(n) for n in ports] + collection.ports = [port.Port.convert_with_links(n) for n in ports] resource_url = '/'.join(['nodes', node_uuid, 'ports']) - collection.links = collection.make_links(limit, resource_url, - sort_key=sort_key, - sort_dir=sort_dir) + collection.next = collection.get_next(limit, url=resource_url, + sort_key=sort_key, + sort_dir=sort_dir) return collection diff --git a/ironic/api/controllers/v1/port.py b/ironic/api/controllers/v1/port.py index d4b6052b09..807a827ebc 100644 --- a/ironic/api/controllers/v1/port.py +++ b/ironic/api/controllers/v1/port.py @@ -75,15 +75,17 @@ class Port(base.APIBase): class PortCollection(collection.Collection): """API representation of a collection of ports.""" - items = [Port] + ports = [Port] "A list containing ports objects" + def __init__(self, **kwargs): + self._type = 'ports' + @classmethod def convert_with_links(cls, ports, limit, **kwargs): collection = PortCollection() - collection.type = 'port' - collection.items = [Port.convert_with_links(p) for p in ports] - collection.links = collection.make_links(limit, 'ports', **kwargs) + collection.ports = [Port.convert_with_links(p) for p in ports] + collection.next = collection.get_next(limit, **kwargs) return collection diff --git a/ironic/tests/api/test_chassis.py b/ironic/tests/api/test_chassis.py index eb09d9fba2..6a852c7d9d 100644 --- a/ironic/tests/api/test_chassis.py +++ b/ironic/tests/api/test_chassis.py @@ -25,13 +25,13 @@ class TestListChassis(base.FunctionalTest): def test_empty(self): data = self.get_json('/chassis') - self.assertEqual([], data['items']) + self.assertEqual([], data['chassis']) def test_one(self): ndict = dbutils.get_test_chassis() chassis = self.dbapi.create_chassis(ndict) data = self.get_json('/chassis') - self.assertEqual(chassis['uuid'], data['items'][0]["uuid"]) + self.assertEqual(chassis['uuid'], data['chassis'][0]["uuid"]) def test_many(self): ch_list = [] @@ -41,9 +41,9 @@ class TestListChassis(base.FunctionalTest): chassis = self.dbapi.create_chassis(ndict) ch_list.append(chassis['uuid']) data = self.get_json('/chassis') - self.assertEqual(len(ch_list), len(data['items'])) + self.assertEqual(len(ch_list), len(data['chassis'])) - uuids = [n['uuid'] for n in data['items']] + uuids = [n['uuid'] for n in data['chassis']] self.assertEqual(ch_list.sort(), uuids.sort()) def test_links(self): @@ -63,12 +63,10 @@ class TestListChassis(base.FunctionalTest): ch = self.dbapi.create_chassis(ndict) chassis.append(ch['uuid']) data = self.get_json('/chassis/?limit=3') - self.assertEqual(data['type'], 'chassis') - self.assertEqual(len(data['items']), 3) + self.assertEqual(len(data['chassis']), 3) - next_marker = data['items'][-1]['uuid'] - next_link = [l['href'] for l in data['links'] if l['rel'] == 'next'][0] - self.assertIn(next_marker, next_link) + next_marker = data['chassis'][-1]['uuid'] + self.assertIn(next_marker, data['next']) def test_nodes_subresource_link(self): ndict = dbutils.get_test_chassis() @@ -87,15 +85,13 @@ class TestListChassis(base.FunctionalTest): self.dbapi.create_node(ndict) data = self.get_json('/chassis/%s/nodes' % cdict['uuid']) - self.assertEqual(data['type'], 'node') - self.assertEqual(len(data['items']), 2) - self.assertEqual(len(data['links']), 0) + self.assertEqual(len(data['nodes']), 2) + self.assertNotIn('next', data.keys()) # Test collection pagination data = self.get_json('/chassis/%s/nodes?limit=1' % cdict['uuid']) - self.assertEqual(data['type'], 'node') - self.assertEqual(len(data['items']), 1) - self.assertEqual(len(data['links']), 1) + self.assertEqual(len(data['nodes']), 1) + self.assertIn('next', data.keys()) class TestPatch(base.FunctionalTest): @@ -222,8 +218,8 @@ class TestPost(base.FunctionalTest): self.post_json('/chassis', cdict) result = self.get_json('/chassis') self.assertEqual(cdict['description'], - result['items'][0]['description']) - self.assertTrue(uuidutils.is_uuid_like(result['items'][0]['uuid'])) + result['chassis'][0]['description']) + self.assertTrue(uuidutils.is_uuid_like(result['chassis'][0]['uuid'])) class TestDelete(base.FunctionalTest): diff --git a/ironic/tests/api/test_nodes.py b/ironic/tests/api/test_nodes.py index 7513f81f02..3372646487 100644 --- a/ironic/tests/api/test_nodes.py +++ b/ironic/tests/api/test_nodes.py @@ -31,13 +31,13 @@ class TestListNodes(base.FunctionalTest): def test_empty(self): data = self.get_json('/nodes') - self.assertEqual([], data['items']) + self.assertEqual([], data['nodes']) def test_one(self): ndict = dbutils.get_test_node() node = self.dbapi.create_node(ndict) data = self.get_json('/nodes') - self.assertEqual(node['uuid'], data['items'][0]["uuid"]) + self.assertEqual(node['uuid'], data['nodes'][0]["uuid"]) def test_many(self): nodes = [] @@ -47,9 +47,9 @@ class TestListNodes(base.FunctionalTest): node = self.dbapi.create_node(ndict) nodes.append(node['uuid']) data = self.get_json('/nodes') - self.assertEqual(len(nodes), len(data['items'])) + self.assertEqual(len(nodes), len(data['nodes'])) - uuids = [n['uuid'] for n in data['items']] + uuids = [n['uuid'] for n in data['nodes']] self.assertEqual(nodes.sort(), uuids.sort()) def test_links(self): @@ -69,12 +69,10 @@ class TestListNodes(base.FunctionalTest): node = self.dbapi.create_node(ndict) nodes.append(node['uuid']) data = self.get_json('/nodes/?limit=3') - self.assertEqual(data['type'], 'node') - self.assertEqual(len(data['items']), 3) + self.assertEqual(len(data['nodes']), 3) - next_marker = data['items'][-1]['uuid'] - next_link = [l['href'] for l in data['links'] if l['rel'] == 'next'][0] - self.assertIn(next_marker, next_link) + next_marker = data['nodes'][-1]['uuid'] + self.assertIn(next_marker, data['next']) def test_ports_subresource_link(self): ndict = dbutils.get_test_node() @@ -93,15 +91,13 @@ class TestListNodes(base.FunctionalTest): self.dbapi.create_port(pdict) data = self.get_json('/nodes/%s/ports' % ndict['uuid']) - self.assertEqual(data['type'], 'port') - self.assertEqual(len(data['items']), 2) - self.assertEqual(len(data['links']), 0) + self.assertEqual(len(data['ports']), 2) + self.assertNotIn('next', data.keys()) # Test collection pagination data = self.get_json('/nodes/%s/ports?limit=1' % ndict['uuid']) - self.assertEqual(data['type'], 'port') - self.assertEqual(len(data['items']), 1) - self.assertEqual(len(data['links']), 1) + self.assertEqual(len(data['ports']), 1) + self.assertIn('next', data.keys()) def test_state(self): ndict = dbutils.get_test_node() diff --git a/ironic/tests/api/test_ports.py b/ironic/tests/api/test_ports.py index 3885c45a2b..74dc76eec7 100644 --- a/ironic/tests/api/test_ports.py +++ b/ironic/tests/api/test_ports.py @@ -25,13 +25,13 @@ class TestListPorts(base.FunctionalTest): def test_empty(self): data = self.get_json('/ports') - self.assertEqual([], data['items']) + self.assertEqual([], data['ports']) def test_one(self): ndict = dbutils.get_test_port() port = self.dbapi.create_port(ndict) data = self.get_json('/ports') - self.assertEqual(port['uuid'], data['items'][0]["uuid"]) + self.assertEqual(port['uuid'], data['ports'][0]["uuid"]) def test_many(self): ports = [] @@ -41,9 +41,9 @@ class TestListPorts(base.FunctionalTest): port = self.dbapi.create_port(ndict) ports.append(port['uuid']) data = self.get_json('/ports') - self.assertEqual(len(ports), len(data['items'])) + self.assertEqual(len(ports), len(data['ports'])) - uuids = [n['uuid'] for n in data['items']] + uuids = [n['uuid'] for n in data['ports']] self.assertEqual(ports.sort(), uuids.sort()) def test_links(self): @@ -63,12 +63,10 @@ class TestListPorts(base.FunctionalTest): port = self.dbapi.create_port(ndict) ports.append(port['uuid']) data = self.get_json('/ports/?limit=3') - self.assertEqual(data['type'], 'port') - self.assertEqual(len(data['items']), 3) + self.assertEqual(len(data['ports']), 3) - next_marker = data['items'][-1]['uuid'] - next_link = [l['href'] for l in data['links'] if l['rel'] == 'next'][0] - self.assertIn(next_marker, next_link) + next_marker = data['ports'][-1]['uuid'] + self.assertIn(next_marker, data['next']) class TestPatch(base.FunctionalTest):