From e22a5969ad2c3bdd4915e8c9e7e22cb0b0cfd09a Mon Sep 17 00:00:00 2001 From: Jeffrey Zhang Date: Tue, 23 Sep 2014 22:54:32 +0800 Subject: [PATCH] Fix no links property in pool list response The response in pool list should contain a link property. Closes-Bug: #1372931 Change-Id: If486013252a9d92c8fbb679402468f094c7da90f --- tests/functional/wsgi/v1_1/test_pools.py | 4 +- zaqar/queues/api/v1/response.py | 52 +++++++++++++++++++ zaqar/queues/api/v1_1/response.py | 25 +++++++-- zaqar/queues/storage/mongodb/pools.py | 12 +++-- zaqar/queues/storage/pooling.py | 14 +++-- zaqar/queues/storage/sqlalchemy/pools.py | 11 +++- zaqar/queues/transport/wsgi/v1_0/pools.py | 42 ++++++++++----- zaqar/queues/transport/wsgi/v1_1/pools.py | 40 +++++++++----- zaqar/tests/queues/storage/base.py | 23 +++++--- .../queues/transport/wsgi/v1/test_pools.py | 41 ++++++++++++--- .../queues/transport/wsgi/v1_1/test_pools.py | 40 ++++++++++++-- 11 files changed, 245 insertions(+), 59 deletions(-) diff --git a/tests/functional/wsgi/v1_1/test_pools.py b/tests/functional/wsgi/v1_1/test_pools.py index 853661d5b..6f1b07752 100644 --- a/tests/functional/wsgi/v1_1/test_pools.py +++ b/tests/functional/wsgi/v1_1/test_pools.py @@ -86,7 +86,7 @@ class TestPools(base.V1_1FunctionalTestBase): # Test existence result = self.client.get('/'+pool_name+'?detailed=true') self.assertEqual(result.status_code, 200) - self.assertSchema(result.json(), 'pool-detail') + self.assertSchema(result.json(), 'pool_detail') @ddt.data( { @@ -142,7 +142,7 @@ class TestPools(base.V1_1FunctionalTestBase): result = self.client.get() self.assertEqual(result.status_code, 200) - self.assertSchema(result.json(), 'pool-list') + self.assertSchema(result.json(), 'pool_list') @ddt.data( { diff --git a/zaqar/queues/api/v1/response.py b/zaqar/queues/api/v1/response.py index 1f979c2e9..c4543ea11 100644 --- a/zaqar/queues/api/v1/response.py +++ b/zaqar/queues/api/v1/response.py @@ -143,6 +143,58 @@ class ResponseSchema(api.Api): 'required': ['messages'], 'additionalProperties': False }, + + 'pool_list': { + 'type': 'object', + 'properties': { + 'links': { + 'type': 'array', + 'items': { + 'type': 'object', + 'properties': { + 'rel': { + 'type': 'string' + }, + 'href': { + 'type': 'string', + 'pattern': '^/v1/pools\?' + } + }, + 'required': ['rel', 'href'], + 'additionalProperties': False + } + }, + 'pools': { + 'type': 'array', + 'items': { + 'type': 'object', + 'properties': { + 'href': { + 'type': 'string', + 'pattern': '^/v1/' + 'pools/[a-zA-Z0-9_-]{1,64}$' + }, + 'weight': { + 'type': 'number', + 'minimum': -1 + }, + 'uri': { + 'type': 'string' + }, + 'options': { + 'type': 'object', + 'additionalProperties': True + } + }, + 'required': ['href', 'weight', 'uri'], + 'additionalProperties': False, + }, + } + }, + 'required': ['links', 'pools'], + 'additionalProperties': False + }, + 'message_list': { 'type': 'object', 'properties': { diff --git a/zaqar/queues/api/v1_1/response.py b/zaqar/queues/api/v1_1/response.py index 99f451f44..717ea4827 100644 --- a/zaqar/queues/api/v1_1/response.py +++ b/zaqar/queues/api/v1_1/response.py @@ -163,9 +163,26 @@ class ResponseSchema(api.Api): 'additionalProperties': False }, - 'pool-list': { + 'pool_list': { 'type': 'object', 'properties': { + 'links': { + 'type': 'array', + 'items': { + 'type': 'object', + 'properties': { + 'rel': { + 'type': 'string' + }, + 'href': { + 'type': 'string', + 'pattern': '^/v1\.1/pools\?' + } + }, + 'required': ['rel', 'href'], + 'additionalProperties': False + } + }, 'pools': { 'type': 'array', 'items': { @@ -174,7 +191,7 @@ class ResponseSchema(api.Api): 'href': { 'type': 'string', 'pattern': '^/v1\.1/' - 'pools/[a-zA-Z0-9_-]{1,64}$' + 'pools/[a-zA-Z0-9_-]{1,64}$' }, 'weight': { 'type': 'number', @@ -196,7 +213,7 @@ class ResponseSchema(api.Api): }, } }, - 'required': ['pools'], + 'required': ['links', 'pools'], 'additionalProperties': False }, @@ -229,7 +246,7 @@ class ResponseSchema(api.Api): } } }, - 'pool-detail': { + 'pool_detail': { 'type': 'object', 'properties': { 'uri': { diff --git a/zaqar/queues/storage/mongodb/pools.py b/zaqar/queues/storage/mongodb/pools.py index f7dfca204..c1cccce12 100644 --- a/zaqar/queues/storage/mongodb/pools.py +++ b/zaqar/queues/storage/mongodb/pools.py @@ -60,9 +60,15 @@ class PoolsController(base.PoolsBase): query['n'] = {'$gt': marker} cursor = self._col.find(query, fields=_field_spec(detailed), - limit=limit) - normalizer = functools.partial(_normalize, detailed=detailed) - return utils.HookedCursor(cursor, normalizer) + limit=limit).sort('n') + marker_name = {} + + def normalizer(pool): + marker_name['next'] = pool['n'] + return _normalize(pool, detailed=detailed) + + yield utils.HookedCursor(cursor, normalizer) + yield marker_name and marker_name['next'] @utils.raises_conn_error def get(self, name, detailed=False): diff --git a/zaqar/queues/storage/pooling.py b/zaqar/queues/storage/pooling.py index 2c4095dc6..d69fc4504 100644 --- a/zaqar/queues/storage/pooling.py +++ b/zaqar/queues/storage/pooling.py @@ -72,9 +72,10 @@ class DataDriver(storage.DataDriverBase): self._pool_catalog = Catalog(conf, cache, control) def is_alive(self): + cursor = self._pool_catalog._pools_ctrl.list(limit=0) + pools = next(cursor) return all(self._pool_catalog.get_driver(pool['name']).is_alive() - for pool in - self._pool_catalog._pools_ctrl.list(limit=0)) + for pool in pools) def _health(self): KPI = {} @@ -82,15 +83,17 @@ class DataDriver(storage.DataDriverBase): # reachable or not KPI['catalog_reachable'] = self.is_alive() + cursor = self._pool_catalog._pools_ctrl.list(limit=0) # Messages of each pool - for pool in self._pool_catalog._pools_ctrl.list(): + for pool in next(cursor): driver = self._pool_catalog.get_driver(pool['name']) KPI[pool['name']] = driver._health() return KPI def gc(self): - for pool in self._pool_catalog._pools_ctrl.list(): + cursor = self._pool_catalog._pools_ctrl.list(limit=0) + for pool in next(cursor): driver = self._pool_catalog.get_driver(pool['name']) driver.gc() @@ -158,7 +161,8 @@ class QueueController(RoutingController): limit=storage.DEFAULT_QUEUES_PER_PAGE, detailed=False): def all_pages(): - for pool in self._pool_catalog._pools_ctrl.list(limit=0): + cursor = self._pool_catalog._pools_ctrl.list(limit=0) + for pool in next(cursor): yield next(self._pool_catalog.get_driver(pool['name']) .queue_controller.list( project=project, diff --git a/zaqar/queues/storage/sqlalchemy/pools.py b/zaqar/queues/storage/sqlalchemy/pools.py index 17cd0f320..0fb8463d6 100644 --- a/zaqar/queues/storage/sqlalchemy/pools.py +++ b/zaqar/queues/storage/sqlalchemy/pools.py @@ -54,8 +54,15 @@ class PoolsController(base.PoolsBase): stmt = stmt.limit(limit) cursor = self._conn.execute(stmt) - normalizer = functools.partial(_normalize, detailed=detailed) - return (normalizer(v) for v in cursor) + marker_name = {} + + def it(): + for cur in cursor: + marker_name['next'] = cur[0] + yield _normalize(cur, detailed=detailed) + + yield it() + yield marker_name and marker_name['next'] @utils.raises_conn_error def get_group(self, group=None, detailed=False): diff --git a/zaqar/queues/transport/wsgi/v1_0/pools.py b/zaqar/queues/transport/wsgi/v1_0/pools.py index bb8089a4b..ac25ea3fd 100644 --- a/zaqar/queues/transport/wsgi/v1_0/pools.py +++ b/zaqar/queues/transport/wsgi/v1_0/pools.py @@ -60,16 +60,21 @@ class Listing(object): self._ctrl = pools_controller def on_get(self, request, response, project_id): - """Returns a pool listing as objects embedded in an array: + """Returns a pool listing as objects embedded in an object: :: - [ - {"href": "", "weight": 100, "uri": ""}, - ... - ] + { + "pools": [ + {"href": "", "weight": 100, "uri": ""}, + ... + ], + "links": [ + {"href": "", "rel": "next"} + ] + } - :returns: HTTP | [200, 204] + :returns: HTTP | 200 """ LOG.debug(u'LIST pools') @@ -79,14 +84,25 @@ class Listing(object): request.get_param_as_int('limit', store=store) request.get_param_as_bool('detailed', store=store) - results = {} - results['pools'] = list(self._ctrl.list(**store)) - for entry in results['pools']: - entry['href'] = request.path + '/' + entry.pop('name') + cursor = self._ctrl.list(**store) - if not results['pools']: - response.status = falcon.HTTP_204 - return + pools = list(next(cursor)) + + results = {} + + if pools: + store['marker'] = next(cursor) + + for entry in pools: + entry['href'] = request.path + '/' + entry.pop('name') + + results['links'] = [ + { + 'rel': 'next', + 'href': request.path + falcon.to_query_str(store) + } + ] + results['pools'] = pools response.content_location = request.relative_uri response.body = transport_utils.to_json(results) diff --git a/zaqar/queues/transport/wsgi/v1_1/pools.py b/zaqar/queues/transport/wsgi/v1_1/pools.py index 0342c6a27..e4235dc1f 100644 --- a/zaqar/queues/transport/wsgi/v1_1/pools.py +++ b/zaqar/queues/transport/wsgi/v1_1/pools.py @@ -62,16 +62,21 @@ class Listing(object): self._ctrl = pools_controller def on_get(self, request, response, project_id): - """Returns a pool listing as objects embedded in an array: + """Returns a pool listing as objects embedded in an object: :: - [ - {"href": "", "weight": 100, "uri": ""}, - ... - ] + { + "pools": [ + {"href": "", "weight": 100, "uri": ""}, + ... + ], + "links": [ + {"href": "", "rel": "next"} + ] + } - :returns: HTTP | [200, 204] + :returns: HTTP | 200 """ LOG.debug(u'LIST pools') @@ -81,15 +86,26 @@ class Listing(object): request.get_param_as_int('limit', store=store) request.get_param_as_bool('detailed', store=store) + cursor = self._ctrl.list(**store) + pools = list(next(cursor)) + results = {} - results['pools'] = list(self._ctrl.list(**store)) - for entry in results['pools']: - entry['href'] = request.path + '/' + entry.pop('name') - if not results['pools']: - response.status = falcon.HTTP_204 - return + if pools: + store['marker'] = next(cursor) + for entry in pools: + entry['href'] = request.path + '/' + entry.pop('name') + + results['links'] = [ + { + 'rel': 'next', + 'href': request.path + falcon.to_query_str(store) + } + ] + results['pools'] = pools + + response.content_location = request.relative_uri response.body = transport_utils.to_json(results) response.status = falcon.HTTP_200 diff --git a/zaqar/tests/queues/storage/base.py b/zaqar/tests/queues/storage/base.py index 34119d4a9..50482da4f 100644 --- a/zaqar/tests/queues/storage/base.py +++ b/zaqar/tests/queues/storage/base.py @@ -990,7 +990,8 @@ class PoolsControllerTest(ControllerBaseTest): def test_drop_all_leads_to_empty_listing(self): self.pools_controller.drop_all() cursor = self.pools_controller.list() - self.assertRaises(StopIteration, next, cursor) + pools = next(cursor) + self.assertRaises(StopIteration, next, pools) def test_listing_simple(self): # NOTE(cpp-cabrera): base entry interferes with listing results @@ -1021,7 +1022,15 @@ class PoolsControllerTest(ControllerBaseTest): return n, w, u - res = list(self.pools_controller.list()) + def get_res(**kwargs): + cursor = self.pools_controller.list(**kwargs) + res = list(next(cursor)) + marker = next(cursor) + # TODO(jeffrey4l): marker should exist + self.assertTrue(marker) + return res + + res = get_res() self.assertEqual(len(res), 10) for entry in res: n, w, u = _pool(entry['name']) @@ -1029,19 +1038,19 @@ class PoolsControllerTest(ControllerBaseTest): self._pool_expects(entry, n, w, u) self.assertNotIn('options', entry) - res = list(self.pools_controller.list(limit=5)) + res = get_res(limit=5) self.assertEqual(len(res), 5) - res = list(self.pools_controller.list(limit=0)) + res = get_res(limit=0) self.assertEqual(len(res), 15) next_name = marker + 'n' self.pools_controller.create(next_name, 123, '123', options={}) - res = next(self.pools_controller.list(marker=marker)) - self._pool_expects(res, next_name, 123, '123') + res = get_res(marker=marker) + self._pool_expects(res[0], next_name, 123, '123') self.pools_controller.delete(next_name) - res = list(self.pools_controller.list(detailed=True)) + res = get_res(detailed=True) self.assertEqual(len(res), 10) for entry in res: n, w, u = _pool(entry['name']) diff --git a/zaqar/tests/queues/transport/wsgi/v1/test_pools.py b/zaqar/tests/queues/transport/wsgi/v1/test_pools.py index 8b34dec22..45d6dd010 100644 --- a/zaqar/tests/queues/transport/wsgi/v1/test_pools.py +++ b/zaqar/tests/queues/transport/wsgi/v1/test_pools.py @@ -18,7 +18,6 @@ import uuid import ddt import falcon - from zaqar.openstack.common import jsonutils from zaqar import tests as testing from zaqar.tests.queues.transport.wsgi import base @@ -230,10 +229,13 @@ class PoolsBaseTest(base.V1Base): body=jsonutils.dumps({'weight': 1})) self.assertEqual(self.srmock.status, falcon.HTTP_404) - def test_empty_listing_returns_204(self): + def test_empty_listing(self): self.simulate_delete(self.pool) - self.simulate_get(self.url_prefix + '/pools') - self.assertEqual(self.srmock.status, falcon.HTTP_204) + result = self.simulate_get(self.url_prefix + '/pools') + results = jsonutils.loads(result[0]) + self.assertEqual(self.srmock.status, falcon.HTTP_200) + self.assertTrue(len(results['pools']) == 0) + self.assertIn('links', results) def _listing_test(self, count=10, limit=10, marker=None, detailed=False): @@ -242,7 +244,7 @@ class PoolsBaseTest(base.V1Base): self.simulate_delete(self.pool) query = '?limit={0}&detailed={1}'.format(limit, detailed) if marker: - query += '&marker={2}'.format(marker) + query += '&marker={0}'.format(marker) with pools(self, count, self.doc['uri']) as expected: result = self.simulate_get(self.url_prefix + '/pools', @@ -251,9 +253,36 @@ class PoolsBaseTest(base.V1Base): results = jsonutils.loads(result[0]) self.assertIsInstance(results, dict) self.assertIn('pools', results) + self.assertIn('links', results) pool_list = results['pools'] + + link = results['links'][0] + self.assertEqual('next', link['rel']) + href = falcon.uri.parse_query_string(link['href']) + self.assertIn('marker', href) + self.assertEqual(href['limit'], str(limit)) + self.assertEqual(href['detailed'], str(detailed).lower()) + + next_query_string = ('?marker={marker}&limit={limit}' + '&detailed={detailed}').format(**href) + next_result = self.simulate_get(link['href'].split('?')[0], + query_string=next_query_string) + self.assertEqual(self.srmock.status, falcon.HTTP_200) + + next_pool = jsonutils.loads(next_result[0]) + next_pool_list = next_pool['pools'] + + self.assertIn('links', next_pool) + if limit < count: + self.assertEqual(len(next_pool_list), + min(limit, count-limit)) + else: + # NOTE(jeffrey4l): when limit >= count, there will be no + # pools in the 2nd page. + self.assertTrue(len(next_pool_list) == 0) + self.assertEqual(len(pool_list), min(limit, count)) - for s in pool_list: + for s in pool_list + next_pool_list: # NOTE(flwang): It can't assumed that both sqlalchemy and # mongodb can return query result with the same order. Just # like the order they're inserted. Actually, sqlalchemy can't diff --git a/zaqar/tests/queues/transport/wsgi/v1_1/test_pools.py b/zaqar/tests/queues/transport/wsgi/v1_1/test_pools.py index afc3204bf..1a93ba8e5 100644 --- a/zaqar/tests/queues/transport/wsgi/v1_1/test_pools.py +++ b/zaqar/tests/queues/transport/wsgi/v1_1/test_pools.py @@ -233,10 +233,13 @@ class PoolsBaseTest(base.V1_1Base): body=jsonutils.dumps({'weight': 1})) self.assertEqual(self.srmock.status, falcon.HTTP_404) - def test_empty_listing_returns_204(self): + def test_empty_listing(self): self.simulate_delete(self.pool) - self.simulate_get(self.url_prefix + '/pools') - self.assertEqual(self.srmock.status, falcon.HTTP_204) + result = self.simulate_get(self.url_prefix + '/pools') + results = jsonutils.loads(result[0]) + self.assertEqual(self.srmock.status, falcon.HTTP_200) + self.assertTrue(len(results['pools']) == 0) + self.assertIn('links', results) def _listing_test(self, count=10, limit=10, marker=None, detailed=False): @@ -245,7 +248,7 @@ class PoolsBaseTest(base.V1_1Base): self.simulate_delete(self.pool) query = '?limit={0}&detailed={1}'.format(limit, detailed) if marker: - query += '&marker={2}'.format(marker) + query += '&marker={0}'.format(marker) with pools(self, count, self.doc['uri'], 'my-group') as expected: result = self.simulate_get(self.url_prefix + '/pools', @@ -254,9 +257,36 @@ class PoolsBaseTest(base.V1_1Base): results = jsonutils.loads(result[0]) self.assertIsInstance(results, dict) self.assertIn('pools', results) + self.assertIn('links', results) pool_list = results['pools'] + + link = results['links'][0] + self.assertEqual('next', link['rel']) + href = falcon.uri.parse_query_string(link['href']) + self.assertIn('marker', href) + self.assertEqual(href['limit'], str(limit)) + self.assertEqual(href['detailed'], str(detailed).lower()) + + next_query_string = ('?marker={marker}&limit={limit}' + '&detailed={detailed}').format(**href) + next_result = self.simulate_get(link['href'].split('?')[0], + query_string=next_query_string) + self.assertEqual(self.srmock.status, falcon.HTTP_200) + + next_pool = jsonutils.loads(next_result[0]) + next_pool_list = next_pool['pools'] + + self.assertIn('links', next_pool) + if limit < count: + self.assertEqual(len(next_pool_list), + min(limit, count-limit)) + else: + # NOTE(jeffrey4l): when limit >= count, there will be no + # pools in the 2nd page. + self.assertTrue(len(next_pool_list) == 0) + self.assertEqual(len(pool_list), min(limit, count)) - for s in pool_list: + for s in pool_list + next_pool_list: # NOTE(flwang): It can't assumed that both sqlalchemy and # mongodb can return query result with the same order. Just # like the order they're inserted. Actually, sqlalchemy can't