diff --git a/openstack/resource2.py b/openstack/resource2.py index 258e196cf..716b31604 100644 --- a/openstack/resource2.py +++ b/openstack/resource2.py @@ -216,6 +216,8 @@ class Resource(object): resource_key = None #: Plural form of key for resource. resources_key = None + #: Key used for pagination links + pagination_key = None #: The ID of this resource. id = Body("id") @@ -726,45 +728,111 @@ class Resource(object): if not cls.allow_list: raise exceptions.MethodNotSupported(cls, "list") - more_data = True query_params = cls._query_mapping._transpose(params) uri = cls.base_path % params - while more_data: - resp = session.get(uri, - headers={"Accept": "application/json"}, - params=query_params) - resp = resp.json() + limit = query_params.get('limit') + + # Track the total number of resources yielded so we can paginate + # swift objects + total_yielded = 0 + while uri: + # Copy query_params due to weird mock unittest interactions + response = session.get( + uri, + headers={"Accept": "application/json"}, + params=query_params.copy()) + exceptions.raise_from_response(response) + data = response.json() + + # Discard any existing pagination keys + query_params.pop('marker', None) + query_params.pop('limit', None) + if cls.resources_key: - resp = resp[cls.resources_key] + resources = data[cls.resources_key] + else: + resources = data - if not resp: - more_data = False + if not isinstance(resources, list): + resources = [resources] - # Keep track of how many items we've yielded. If we yielded - # less than our limit, we don't need to do an extra request - # to get back an empty data set, which acts as a sentinel. + # Keep track of how many items we've yielded. The server should + # handle this, but it's easy for us to as well. yielded = 0 - new_marker = None - for data in resp: + marker = None + for raw_resource in resources: # Do not allow keys called "self" through. Glance chose # to name a key "self", so we need to pop it out because # we can't send it through cls.existing and into the # Resource initializer. "self" is already the first # argument and is practically a reserved word. - data.pop("self", None) + raw_resource.pop("self", None) - value = cls.existing(**data) - new_marker = value.id - yielded += 1 + value = cls.existing(**raw_resource) + marker = value.id yield value + yielded += 1 + total_yielded += 1 - if not paginated: + # If a limit was given by the user and we have not returned + # as many records as the limit, then it stands to reason that + # there are no more records to return and we don't need to do + # anything else. + if limit and yielded < limit: return - if "limit" in query_params and yielded < query_params["limit"]: + + if resources and paginated: + uri, next_params = cls._get_next_link( + uri, response, data, marker, limit, total_yielded) + query_params.update(next_params) + else: return - query_params["limit"] = yielded - query_params["marker"] = new_marker + + @classmethod + def _get_next_link(cls, uri, response, data, marker, limit, total_yielded): + next_link = None + params = {} + if isinstance(data, dict): + pagination_key = cls.pagination_key + + if not pagination_key and 'links' in data: + # api-wg guidelines are for a links dict in the main body + pagination_key == 'links' + if not pagination_key and cls.resources_key: + # Nova has a {key}_links dict in the main body + pagination_key = '{key}_links'.format(key=cls.resources_key) + if pagination_key: + links = data.get(pagination_key, {}) + for item in links: + if item.get('rel') == 'next' and 'href' in item: + next_link = item['href'] + break + # Glance has a next field in the main body + next_link = next_link or data.get('next') + if not next_link and 'next' in response.links: + # RFC5988 specifies Link headers and requests parses them if they + # are there. We prefer link dicts in resource body, but if those + # aren't there and Link headers are, use them. + next_link = response.links['next']['uri'] + # Swift provides a count of resources in a header and a list body + if not next_link and cls.pagination_key: + total_count = response.headers.get(cls.pagination_key) + if total_count: + total_count = int(total_count) + if total_count > total_yielded: + params['marker'] = marker + if limit: + params['limit'] = limit + next_link = uri + # If we still have no link, and limit was given and is non-zero, + # and the number of records yielded equals the limit, then the user + # is playing pagination ball so we should go ahead and try once more. + if not next_link and limit: + next_link = uri + params['marker'] = marker + params['limit'] = limit + return next_link, params @classmethod def _get_one_match(cls, name_or_id, results): diff --git a/openstack/tests/unit/meter/v2/test_sample.py b/openstack/tests/unit/meter/v2/test_sample.py index ee2588512..8f7faf729 100644 --- a/openstack/tests/unit/meter/v2/test_sample.py +++ b/openstack/tests/unit/meter/v2/test_sample.py @@ -65,6 +65,7 @@ class TestSample(testtools.TestCase): sess = mock.Mock() resp = mock.Mock() resp.json = mock.Mock(return_value=[SAMPLE]) + resp.status_code = 200 sess.get = mock.Mock(return_value=resp) found = sample.Sample.list(sess, counter_name='name_of_meter') diff --git a/openstack/tests/unit/network/v2/test_floating_ip.py b/openstack/tests/unit/network/v2/test_floating_ip.py index 0005f13d7..0ae513989 100644 --- a/openstack/tests/unit/network/v2/test_floating_ip.py +++ b/openstack/tests/unit/network/v2/test_floating_ip.py @@ -73,6 +73,7 @@ class TestFloatingIP(testtools.TestCase): fake_response = mock.Mock() body = {floating_ip.FloatingIP.resources_key: [data]} fake_response.json = mock.Mock(return_value=body) + fake_response.status_code = 200 mock_session.get = mock.Mock(return_value=fake_response) result = floating_ip.FloatingIP.find_available(mock_session) @@ -88,6 +89,7 @@ class TestFloatingIP(testtools.TestCase): fake_response = mock.Mock() body = {floating_ip.FloatingIP.resources_key: []} fake_response.json = mock.Mock(return_value=body) + fake_response.status_code = 200 mock_session.get = mock.Mock(return_value=fake_response) self.assertIsNone(floating_ip.FloatingIP.find_available(mock_session)) diff --git a/openstack/tests/unit/test_resource2.py b/openstack/tests/unit/test_resource2.py index fdfc449e9..a69b88dc0 100644 --- a/openstack/tests/unit/test_resource2.py +++ b/openstack/tests/unit/test_resource2.py @@ -938,6 +938,7 @@ class TestResourceActions(base.TestCase): class Test(resource2.Resource): service = self.service_name base_path = self.base_path + resources_key = 'resources' allow_create = True allow_get = True allow_head = True @@ -1096,7 +1097,9 @@ class TestResourceActions(base.TestCase): # the generator. Wrap calls to self.sot.list in a `list` # and then test the results as a list of responses. def test_list_empty_response(self): - mock_response = FakeResponse([]) + mock_response = mock.Mock() + mock_response.status_code = 200 + mock_response.json.return_value = {"resources": []} self.session.get.return_value = mock_response @@ -1112,8 +1115,9 @@ class TestResourceActions(base.TestCase): def test_list_one_page_response_paginated(self): id_value = 1 mock_response = mock.Mock() - mock_response.json.side_effect = [[{"id": id_value}], - []] + mock_response.status_code = 200 + mock_response.links = {} + mock_response.json.return_value = {"resources": [{"id": id_value}]} self.session.get.return_value = mock_response @@ -1123,17 +1127,15 @@ class TestResourceActions(base.TestCase): self.assertEqual(1, len(results)) - # Look at the `params` argument to each of the get calls that - # were made. - self.session.get.call_args_list[0][1]["params"] = {} - self.session.get.call_args_list[1][1]["params"] = {"marker": id_value} + self.assertEqual(1, len(self.session.get.call_args_list)) self.assertEqual(id_value, results[0].id) self.assertIsInstance(results[0], self.test_class) def test_list_one_page_response_not_paginated(self): id_value = 1 mock_response = mock.Mock() - mock_response.json.return_value = [{"id": id_value}] + mock_response.status_code = 200 + mock_response.json.return_value = {"resources": [{"id": id_value}]} self.session.get.return_value = mock_response @@ -1156,6 +1158,7 @@ class TestResourceActions(base.TestCase): id_value = 1 mock_response = mock.Mock() + mock_response.status_code = 200 mock_response.json.return_value = {key: [{"id": id_value}]} self.session.get.return_value = mock_response @@ -1173,11 +1176,85 @@ class TestResourceActions(base.TestCase): self.assertEqual(id_value, results[0].id) self.assertIsInstance(results[0], self.test_class) + def test_list_response_paginated_without_links(self): + ids = [1, 2] + mock_response = mock.Mock() + mock_response.status_code = 200 + mock_response.links = {} + mock_response.json.return_value = { + "resources": [{"id": ids[0]}], + "resources_links": [{ + "href": "https://example.com/next-url", + "rel": "next", + }] + } + mock_response2 = mock.Mock() + mock_response2.status_code = 200 + mock_response2.links = {} + mock_response2.json.return_value = { + "resources": [{"id": ids[1]}], + } + + self.session.get.side_effect = [mock_response, mock_response2] + + results = list(self.sot.list(self.session, paginated=True)) + + self.assertEqual(2, len(results)) + self.assertEqual(ids[0], results[0].id) + self.assertEqual(ids[1], results[1].id) + self.assertEqual( + mock.call('base_path', + headers={'Accept': 'application/json'}, params={}), + self.session.get.mock_calls[0]) + self.assertEqual( + mock.call('https://example.com/next-url', + headers={'Accept': 'application/json'}, params={}), + self.session.get.mock_calls[1]) + self.assertEqual(2, len(self.session.get.call_args_list)) + self.assertIsInstance(results[0], self.test_class) + + def test_list_response_paginated_with_links(self): + ids = [1, 2] + mock_response = mock.Mock() + mock_response.status_code = 200 + mock_response.links = {} + mock_response.json.side_effect = [ + { + "resources": [{"id": ids[0]}], + "resources_links": [{ + "href": "https://example.com/next-url", + "rel": "next", + }] + }, { + "resources": [{"id": ids[1]}], + }] + + self.session.get.return_value = mock_response + + results = list(self.sot.list(self.session, paginated=True)) + + self.assertEqual(2, len(results)) + self.assertEqual(ids[0], results[0].id) + self.assertEqual(ids[1], results[1].id) + self.assertEqual( + mock.call('base_path', + headers={'Accept': 'application/json'}, params={}), + self.session.get.mock_calls[0]) + self.assertEqual( + mock.call('https://example.com/next-url', + headers={'Accept': 'application/json'}, params={}), + self.session.get.mock_calls[2]) + self.assertEqual(2, len(self.session.get.call_args_list)) + self.assertIsInstance(results[0], self.test_class) + def test_list_multi_page_response_not_paginated(self): ids = [1, 2] mock_response = mock.Mock() - mock_response.json.side_effect = [[{"id": ids[0]}], - [{"id": ids[1]}]] + mock_response.status_code = 200 + mock_response.json.side_effect = [ + {"resources": [{"id": ids[0]}]}, + {"resources": [{"id": ids[1]}]}, + ] self.session.get.return_value = mock_response @@ -1194,10 +1271,16 @@ class TestResourceActions(base.TestCase): uri_param = "uri param!" mock_response = mock.Mock() - mock_response.json.side_effect = [[{"id": id}], - []] + mock_response.status_code = 200 + mock_response.links = {} + mock_response.json.return_value = {"resources": [{"id": id}]} - self.session.get.return_value = mock_response + mock_empty = mock.Mock() + mock_empty.status_code = 200 + mock_empty.links = {} + mock_empty.json.return_value = {"resources": []} + + self.session.get.side_effect = [mock_response, mock_empty] class Test(self.test_class): _query_mapping = resource2.QueryParameters(query_param=qp_name) @@ -1217,19 +1300,33 @@ class TestResourceActions(base.TestCase): Test.base_path % {"something": uri_param}) def test_list_multi_page_response_paginated(self): - # This tests our ability to stop making calls once - # we've received all of the data. However, this tests - # the case that we always receive full pages of data - # and then the signal that there is no more data - an empty list. - # In this case, we need to make one extra request beyond - # the end of data to ensure we've received it all. ids = [1, 2] resp1 = mock.Mock() - resp1.json.return_value = [{"id": ids[0]}] + resp1.status_code = 200 + resp1.links = {} + resp1.json.return_value = { + "resources": [{"id": ids[0]}], + "resources_links": [{ + "href": "https://example.com/next-url", + "rel": "next", + }], + } resp2 = mock.Mock() - resp2.json.return_value = [{"id": ids[1]}] + resp2.status_code = 200 + resp2.links = {} + resp2.json.return_value = { + "resources": [{"id": ids[1]}], + "resources_links": [{ + "href": "https://example.com/next-url", + "rel": "next", + }], + } resp3 = mock.Mock() - resp3.json.return_value = [] + resp3.status_code = 200 + resp3.links = {} + resp3.json.return_value = { + "resources": [] + } self.session.get.side_effect = [resp1, resp2, resp3] @@ -1245,26 +1342,113 @@ class TestResourceActions(base.TestCase): result1 = next(results) self.assertEqual(result1.id, ids[1]) self.session.get.assert_called_with( - self.base_path, + 'https://example.com/next-url', headers={"Accept": "application/json"}, - params={"limit": 1, "marker": 1}) + params={}) self.assertRaises(StopIteration, next, results) self.session.get.assert_called_with( - self.base_path, + 'https://example.com/next-url', headers={"Accept": "application/json"}, - params={"limit": 1, "marker": 2}) + params={}) def test_list_multi_page_early_termination(self): # This tests our ability to be somewhat smart when evaluating - # the contents of the responses. When we receive a full page - # of data, we can be smart about terminating our responses - # once we see that we've received a page with less data than - # expected, saving one request. + # the contents of the responses. When we request a limit and + # receive less than that limit and there are no next links, + # we can be pretty sure there are no more pages. + ids = [1, 2] + resp1 = mock.Mock() + resp1.status_code = 200 + resp1.json.return_value = { + "resources": [{"id": ids[0]}, {"id": ids[1]}], + } + + self.session.get.return_value = resp1 + + results = self.sot.list(self.session, limit=3, paginated=True) + + result0 = next(results) + self.assertEqual(result0.id, ids[0]) + result1 = next(results) + self.assertEqual(result1.id, ids[1]) + self.session.get.assert_called_with( + self.base_path, + headers={"Accept": "application/json"}, + params={"limit": 3}) + + # Ensure we're done after those two items + self.assertRaises(StopIteration, next, results) + + # Ensure we only made one calls to get this done + self.assertEqual(1, len(self.session.get.call_args_list)) + + def test_list_multi_page_inferred_additional(self): + # If we explicitly request a limit and we receive EXACTLY that + # amount of results and there is no next link, we make one additional + # call to check to see if there are more records and the service is + # just sad. + # NOTE(mordred) In a perfect world we would not do this. But it's 2018 + # and I don't think anyone has any illusions that we live in a perfect + # world anymore. ids = [1, 2, 3] resp1 = mock.Mock() + resp1.status_code = 200 + resp1.links = {} + resp1.json.return_value = { + "resources": [{"id": ids[0]}, {"id": ids[1]}], + } + resp2 = mock.Mock() + resp2.status_code = 200 + resp2.links = {} + resp2.json.return_value = {"resources": [{"id": ids[2]}]} + + self.session.get.side_effect = [resp1, resp2] + + results = self.sot.list(self.session, limit=2, paginated=True) + + # Get the first page's two items + result0 = next(results) + self.assertEqual(result0.id, ids[0]) + result1 = next(results) + self.assertEqual(result1.id, ids[1]) + self.session.get.assert_called_with( + self.base_path, + headers={"Accept": "application/json"}, + params={"limit": 2}) + + result2 = next(results) + self.assertEqual(result2.id, ids[2]) + self.session.get.assert_called_with( + self.base_path, + headers={"Accept": "application/json"}, + params={'limit': 2, 'marker': 2}) + + # Ensure we're done after those three items + self.assertRaises(StopIteration, next, results) + + # Ensure we only made two calls to get this done + self.assertEqual(2, len(self.session.get.call_args_list)) + + def test_list_multi_page_header_count(self): + class Test(self.test_class): + resources_key = None + pagination_key = 'X-Container-Object-Count' + self.sot = Test() + + # Swift returns a total number of objects in a header and we compare + # that against the total number returned to know if we need to fetch + # more objects. + ids = [1, 2, 3] + resp1 = mock.Mock() + resp1.status_code = 200 + resp1.links = {} + resp1.headers = {'X-Container-Object-Count': 3} resp1.json.return_value = [{"id": ids[0]}, {"id": ids[1]}] resp2 = mock.Mock() + resp2.status_code = 200 + resp2.links = {} + resp2.headers = {'X-Container-Object-Count': 3} resp2.json.return_value = [{"id": ids[2]}] self.session.get.side_effect = [resp1, resp2] @@ -1281,13 +1465,58 @@ class TestResourceActions(base.TestCase): headers={"Accept": "application/json"}, params={}) - # Second page only has one item result2 = next(results) self.assertEqual(result2.id, ids[2]) self.session.get.assert_called_with( self.base_path, headers={"Accept": "application/json"}, - params={"limit": 2, "marker": 2}) + params={'marker': 2}) + + # Ensure we're done after those three items + self.assertRaises(StopIteration, next, results) + + # Ensure we only made two calls to get this done + self.assertEqual(2, len(self.session.get.call_args_list)) + + def test_list_multi_page_link_header(self): + # Swift returns a total number of objects in a header and we compare + # that against the total number returned to know if we need to fetch + # more objects. + ids = [1, 2, 3] + resp1 = mock.Mock() + resp1.status_code = 200 + resp1.links = { + 'next': {'uri': 'https://example.com/next-url', 'rel': 'next'}} + resp1.headers = {} + resp1.json.return_value = { + "resources": [{"id": ids[0]}, {"id": ids[1]}], + } + resp2 = mock.Mock() + resp2.status_code = 200 + resp2.links = {} + resp2.headers = {} + resp2.json.return_value = {"resources": [{"id": ids[2]}]} + + self.session.get.side_effect = [resp1, resp2] + + results = self.sot.list(self.session, paginated=True) + + # Get the first page's two items + result0 = next(results) + self.assertEqual(result0.id, ids[0]) + result1 = next(results) + self.assertEqual(result1.id, ids[1]) + self.session.get.assert_called_with( + self.base_path, + headers={"Accept": "application/json"}, + params={}) + + result2 = next(results) + self.assertEqual(result2.id, ids[2]) + self.session.get.assert_called_with( + 'https://example.com/next-url', + headers={"Accept": "application/json"}, + params={}) # Ensure we're done after those three items self.assertRaises(StopIteration, next, results)