diff --git a/examples/list.py b/examples/list.py index f1214d76..5b916d9e 100644 --- a/examples/list.py +++ b/examples/list.py @@ -22,7 +22,7 @@ def run_list(opts): path_args = None if opts.data: path_args = common.get_data_option(opts) - for obj in cls.list(sess, path_args=path_args): + for obj in cls.list(sess, path_args=path_args, paginated=True): print(str(obj)) return diff --git a/openstack/network/v2/floating_ip.py b/openstack/network/v2/floating_ip.py index 9aa885d5..8d5a607b 100644 --- a/openstack/network/v2/floating_ip.py +++ b/openstack/network/v2/floating_ip.py @@ -50,11 +50,11 @@ class FloatingIP(resource.Resource): @classmethod def find_available(cls, session): - args = { + params = { 'port_id': '', 'fields': cls.id_attribute, } - info = cls.list(session, **args) + info = cls.list(session, params=params) try: return next(info) except StopIteration: diff --git a/openstack/resource.py b/openstack/resource.py index 3fb08253..2a3ccebe 100644 --- a/openstack/resource.py +++ b/openstack/resource.py @@ -36,7 +36,6 @@ import itertools import time import six -from six.moves.urllib import parse as url_parse from openstack import exceptions from openstack import utils @@ -813,29 +812,14 @@ class Resource(collections.MutableMapping): self.delete_by_id(session, self.id, path_args=self) @classmethod - def list(cls, session, limit=None, marker=None, path_args=None, - paginated=False, **params): - """Get a response that is a list of objects. + def list(cls, session, path_args=None, paginated=False, params=None): + """This method is a generator which yields resource objects. - This method starts at ``limit`` and ``marker`` (both defaulting to - None), advances the marker to the last item received in each response, - and continues making requests for more resources until no results - are returned. - - When no ``limit`` is provided, the server will return its maximum - amount of items per page. After that response, the limit will be - determined and sent in subsequent requests to the server. That limit - will be used to calculate when all items have been received. + This resource object list generator handles pagination and takes query + params for response filtering. :param session: The session to use for making this request. :type session: :class:`~openstack.session.Session` - :param limit: The maximum amount of results to retrieve. - The default is ``None``, which means no limit will be - set, and the underlying API will return its default - amount of responses. - :param marker: The position in the list to begin requests from. - The type of value to use for ``marker`` depends on - the API being called. :param dict path_args: A dictionary of arguments to construct a compound URL. See `How path_args are used`_ for details. @@ -845,8 +829,10 @@ class Resource(collections.MutableMapping): **When paginated is False only one page of data will be returned regardless of the API's support of pagination.** - :param dict params: Parameters to be passed into the underlying + :param dict params: Query parameters to be passed into the underlying :meth:`~openstack.session.Session.get` method. + Values that the server may support include `limit` + and `marker`. :return: A generator of :class:`Resource` objects. :raises: :exc:`~openstack.exceptions.MethodNotSupported` if @@ -856,9 +842,12 @@ class Resource(collections.MutableMapping): raise exceptions.MethodNotSupported(cls, 'list') more_data = True - + params = {} if params is None else params + url = cls._get_url(path_args) while more_data: - resp = cls.page(session, limit, marker, path_args, **params) + resp = session.get(url, service=cls.service, params=params).body + if cls.resources_key: + resp = resp[cls.resources_key] if not resp: more_data = False @@ -869,55 +858,16 @@ class Resource(collections.MutableMapping): yielded = 0 for data in resp: value = cls.existing(**data) - marker = value.id + new_marker = value.id yielded += 1 yield value - if not paginated or limit and yielded < limit: - more_data = False - - limit = yielded - - @classmethod - def page(cls, session, limit=None, marker=None, path_args=None, **params): - """Get a one page response. - - This method gets starting at ``marker`` with a maximum of ``limit`` - records. - - :param session: The session to use for making this request. - :type session: :class:`~openstack.session.Session` - :param limit: The maximum amount of results to retrieve. The default - is to retrieve as many results as the service allows. - :param marker: The position in the list to begin requests from. - The type of value to use for ``marker`` depends on - the API being called. - :param dict path_args: A dictionary of arguments to construct - a compound URL. - See `How path_args are used`_ for details. - :param dict params: Parameters to be passed into the underlying - :meth:`~openstack.session.Session.get` method. - - :return: A list of dictionaries returned in the response body. - """ - - filters = {} - - if limit: - filters['limit'] = limit - if marker: - filters['marker'] = marker - - url = cls._get_url(path_args) - if filters: - url = '%s?%s' % (url, url_parse.urlencode(filters)) - - resp = session.get(url, service=cls.service, params=params).body - - if cls.resources_key: - resp = resp[cls.resources_key] - - return resp + if not paginated: + return + if 'limit' in params and yielded < params['limit']: + return + params['limit'] = yielded + params['marker'] = new_marker @classmethod def find(cls, session, name_or_id, path_args=None, ignore_missing=True): @@ -943,35 +893,40 @@ class Resource(collections.MutableMapping): :raises: :class:`openstack.exceptions.ResourceNotFound` if nothing is found and ignore_missing is ``False``. """ - def get_one_match(data, attr): - if len(data) == 1: - result = cls.existing(**data[0]) - value = getattr(result, attr, False) - if value == name_or_id: - return result + def get_one_match(generator, attr, raise_exception): + try: + first = next(generator) + if any(generator): + if raise_exception: + msg = "More than one %s exists with the name '%s'." + msg = (msg % (cls.get_resource_name(), name_or_id)) + raise exceptions.DuplicateResource(msg) + return None + except StopIteration: + return None + result = cls.existing(**first) + value = getattr(result, attr, False) + if value == name_or_id: + return result return None try: if cls.allow_retrieve: return cls.get_by_id(session, name_or_id, path_args=path_args) - args = {cls.id_attribute: name_or_id} - info = cls.page(session, limit=2, path_args=path_args, **args) - result = get_one_match(info, cls.id_attribute) + params = {'limit': 2} + params[cls.id_attribute] = name_or_id + info = cls.list(session, path_args=path_args, params=params) + result = get_one_match(info, cls.id_attribute, False) if result is not None: return result except exceptions.HttpException: pass if cls.name_attribute: - params = {cls.name_attribute: name_or_id} - info = cls.page(session, limit=2, path_args=path_args, **params) - - if len(info) > 1: - msg = "More than one %s exists with the name '%s'." - msg = (msg % (cls.get_resource_name(), name_or_id)) - raise exceptions.DuplicateResource(msg) - - result = get_one_match(info, cls.name_attribute) + params = {'limit': 2} + params[cls.name_attribute] = name_or_id + info = cls.list(session, path_args=path_args, params=params) + result = get_one_match(info, cls.name_attribute, True) if result is not None: return result diff --git a/openstack/tests/unit/test_resource.py b/openstack/tests/unit/test_resource.py index 56725832..18ee823d 100644 --- a/openstack/tests/unit/test_resource.py +++ b/openstack/tests/unit/test_resource.py @@ -737,7 +737,6 @@ class ResourceTests(base.TestTransportBase): results = [fake_data.copy(), fake_data.copy(), fake_data.copy()] for i in range(len(results)): results[i]['id'] = fake_id + i - if resource_class.resources_key is not None: body = {resource_class.resources_key: self._get_expected_results()} @@ -745,19 +744,15 @@ class ResourceTests(base.TestTransportBase): else: body = self._get_expected_results() sentinel = [] - - marker = "marker=%d" % results[-1]['id'] - self.session.get.side_effect = [mock.Mock(body=body), mock.Mock(body=sentinel)] - objs = resource_class.list(self.session, limit=1, - path_args=fake_arguments, - paginated=True) - objs = list(objs) - self.assertIn(marker, self.session.get.call_args[0][0]) - self.assertEqual(3, len(objs)) + objs = list(resource_class.list(self.session, path_args=fake_arguments, + paginated=True)) + params = {'limit': 3, 'marker': results[-1]['id']} + self.assertEqual(params, self.session.get.call_args[1]['params']) + self.assertEqual(3, len(objs)) for obj in objs: self.assertIn(obj.id, range(fake_id, fake_id + 3)) self.assertEqual(fake_name, obj['name']) @@ -783,7 +778,7 @@ class ResourceTests(base.TestTransportBase): attrs = {"get.return_value": body} session = mock.Mock(**attrs) - list(FakeResource.list(session, limit=len(results) + 1, + list(FakeResource.list(session, params={'limit': len(results) + 1}, path_args=fake_arguments, paginated=paginated)) @@ -798,53 +793,26 @@ class ResourceTests(base.TestTransportBase): # When we call with paginated=False, make sure we made one call self._test_list_call_count(False) - @mock.patch("openstack.resource.Resource.page") - def test_determine_limit(self, fake_page): + def test_determine_limit(self): full_page = [fake_data.copy(), fake_data.copy(), fake_data.copy()] last_page = [fake_data.copy()] session = mock.MagicMock() - pages = [full_page, full_page, last_page] - fake_page.side_effect = pages + session.get = mock.MagicMock() + full_response = mock.MagicMock() + full_response.body = {FakeResource.resources_key: full_page} + last_response = mock.MagicMock() + last_response.body = {FakeResource.resources_key: last_page} + pages = [full_response, full_response, last_response] + session.get.side_effect = pages # Don't specify a limit. Resource.list will determine the limit # is 3 based on the first `full_page`. results = list(FakeResource.list(session, path_args=fake_arguments, paginated=True)) - self.assertEqual(fake_page.call_count, len(pages)) - self.assertEqual(len(results), sum([len(page) for page in pages])) - - def test_page(self): - session = mock.Mock() - session.get = mock.Mock() - records = [{'id': 'squid'}] - response = FakeResponse({FakeResource.resources_key: records}) - session.get.return_value = response - - objs = FakeResource.page(session, 1, None, path_args=fake_arguments) - - self.assertEqual(records, objs) - path = fake_path + '?limit=1' - session.get.assert_called_with(path, params={}, service=None) - - objs = FakeResource.page(session, None, 'a', path_args=fake_arguments) - - self.assertEqual(records, objs) - path = fake_path + '?marker=a' - session.get.assert_called_with(path, params={}, service=None) - - objs = FakeResource.page(session, None, None, path_args=fake_arguments) - - self.assertEqual(records, objs) - path = fake_path - session.get.assert_called_with(path, params={}, service=None) - - objs = FakeResource.page(session) - - self.assertEqual(records, objs) - path = fake_base_path - session.get.assert_called_with(path, params={}, service=None) + self.assertEqual(session.get.call_count, len(pages)) + self.assertEqual(len(full_page + full_page + last_page), len(results)) def test_attrs_name(self): obj = FakeResource() @@ -1200,9 +1168,8 @@ class TestFind(base.TestCase): self.assertEqual(self.NAME, result.name) self.assertEqual(self.PROP, result.prop) - p = {'name': self.NAME} - path = fake_path + "?limit=2" - self.mock_get.assert_any_call(path, params=p, service=None) + p = {'limit': 2, 'name': self.NAME} + self.assertEqual(p, self.mock_get.call_args[1]['params']) def test_id(self): self.mock_get.side_effect = [ @@ -1232,9 +1199,8 @@ class TestFind(base.TestCase): self.assertEqual(self.ID, result.id) self.assertEqual(self.PROP, result.prop) - p = {'id': self.ID} - path = fake_path + "?limit=2" - self.mock_get.assert_any_call(path, params=p, service=None) + p = {'id': self.ID, 'limit': 2} + self.assertEqual(p, self.mock_get.call_args[1]['params']) def test_dups(self): dup = {'id': 'Larry'}