diff --git a/openstack/network/v2/floatingip.py b/openstack/network/v2/floatingip.py index 9aa76797..fa2499fe 100644 --- a/openstack/network/v2/floatingip.py +++ b/openstack/network/v2/floatingip.py @@ -46,6 +46,8 @@ class FloatingIP(resource.Resource): 'fields': cls.id_attribute, } info = cls.list(session, **args) - if len(info) > 0: - return info[0] - raise exceptions.ResourceNotFound("No available floating ips exist.") + try: + return next(info) + except StopIteration: + msg = "No available floating ips exist." + raise exceptions.ResourceNotFound(msg) diff --git a/openstack/resource.py b/openstack/resource.py index 0968a542..e53ac14a 100644 --- a/openstack/resource.py +++ b/openstack/resource.py @@ -425,33 +425,49 @@ class Resource(collections.MutableMapping): @classmethod def list(cls, session, limit=None, marker=None, path_args=None, **params): - """Get a list of resources as an array of objects.""" + """Return a generator that will page through results of GET requests. - # NOTE(jamielennox): Is it possible we can return a generator from here - # and allow us to keep paging rather than limit and marker? + This method starts at `limit` and `marker` (both defaulting to None), + advances the marker to the last item received, and continues paging + until no results are returned. + """ if not cls.allow_list: raise exceptions.MethodNotSupported('list') - filters = {} + more_data = True - if limit: - filters['limit'] = limit - if marker: - filters['marker'] = marker + while more_data: + filters = {} - if path_args: - url = cls.base_path % path_args - else: - url = cls.base_path - if filters: - url = '%s?%s' % (url, url_parse.urlencode(filters)) + if limit: + filters['limit'] = limit + if marker: + filters['marker'] = marker - resp = session.get(url, service=cls.service, params=params).body + if path_args: + url = cls.base_path % path_args + else: + url = cls.base_path + if filters: + url = '%s?%s' % (url, url_parse.urlencode(filters)) - if cls.resources_key: - resp = resp[cls.resources_key] + resp = session.get(url, service=cls.service, params=params).body - return [cls.existing(**data) for data in resp] + if cls.resources_key: + resp = resp[cls.resources_key] + + # TODO(briancurtin): Although there are a few different ways + # across services, we can know from a response if it's the end + # without doing an extra request to get an empty response. + # Resources should probably carry something like a `_should_page` + # method to handle their service's pagination style. + if not resp: + more_data = False + + for data in resp: + value = cls.existing(**data) + marker = value.id + yield value @classmethod def find(cls, session, name_or_id, path_args=None): @@ -463,20 +479,36 @@ class Resource(collections.MutableMapping): 'path_args': path_args, } info = cls.list(session, **args) - if len(info) == 1: - return info[0] + # If there is exactly one result available, return it. + result = None + try: + result = next(info) + next(info) + except StopIteration: + if result is not None: + return result except exceptions.HttpException: pass + if cls.name_attribute: params = {cls.name_attribute: name_or_id, 'fields': cls.id_attribute} info = cls.list(session, path_args=path_args, **params) - if len(info) == 1: - return info[0] - if len(info) > 1: + result = None + # Take the first value as our result. If another value is, + # available then raise DuplicateResource. + try: + result = next(info) + next(info) msg = "More than one %s exists with the name '%s'." msg = (msg % (cls.get_resource_name(), name_or_id)) raise exceptions.DuplicateResource(msg) + except StopIteration: + # We got here because `info` either gave us the result + # or it was empty. + if result is not None: + return result + msg = ("No %s with a name or ID of '%s' exists." % (cls.get_resource_name(), name_or_id)) raise exceptions.ResourceNotFound(msg) diff --git a/openstack/tests/test_resource.py b/openstack/tests/test_resource.py index 02501b70..02533a0c 100644 --- a/openstack/tests/test_resource.py +++ b/openstack/tests/test_resource.py @@ -259,14 +259,19 @@ class ResourceTests(base.TestTransportBase): for i in range(len(results)): results[i]['id'] = fake_id + i + marker = "marker=%d" % results[-1]['id'] + self.stub_url(httpretty.GET, + path=[fake_path + "?" + marker], + json={fake_resources: []}, + match_querystring=True) self.stub_url(httpretty.GET, path=[fake_path], json={fake_resources: results}) - objs = FakeResource.list(self.session, marker='x', + objs = FakeResource.list(self.session, limit=1, path_args=fake_arguments) - - self.assertIn('marker=x', httpretty.last_request().path) + objs = list(objs) + self.assertIn(marker, httpretty.last_request().path) self.assertEqual(3, len(objs)) for obj in objs: @@ -325,18 +330,19 @@ class TestFind(base.TestCase): self.assertEqual(self.ID, result.id) p = {'fields': 'id', 'name': self.NAME} - self.mock_get.assert_called_with(fake_path, params=p, service=None) + self.mock_get.assert_any_call(fake_path, params=p, service=None) def test_id(self): - resp = FakeResponse({FakeResource.resources_key: [self.matrix]}) - self.mock_get.return_value = resp + self.mock_get.side_effect = [ + FakeResponse({FakeResource.resources_key: [self.matrix]}) + ] result = FakeResource.find(self.mock_session, self.ID, path_args=fake_arguments) self.assertEqual(self.ID, result.id) p = {'fields': 'id', 'id': self.ID} - self.mock_get.assert_called_with(fake_path, params=p, service=None) + self.mock_get.assert_any_call(fake_path, params=p, service=None) def test_nameo(self): self.mock_get.side_effect = [ @@ -351,7 +357,7 @@ class TestFind(base.TestCase): FakeResource.name_attribute = 'name' self.assertEqual(self.ID, result.id) p = {'fields': 'id', 'nameo': self.NAME} - self.mock_get.assert_called_with(fake_path, params=p, service=None) + self.mock_get.assert_any_call(fake_path, params=p, service=None) def test_dups(self): dup = {'id': 'Larry'} @@ -363,8 +369,9 @@ class TestFind(base.TestCase): def test_id_attribute_find(self): floater = {'ip_address': "127.0.0.1"} - resp = FakeResponse({FakeResource.resources_key: [floater]}) - self.mock_get.return_value = resp + self.mock_get.side_effect = [ + FakeResponse({FakeResource.resources_key: [floater]}) + ] FakeResource.id_attribute = 'ip_address' result = FakeResource.find(self.mock_session, "127.0.0.1", @@ -373,7 +380,7 @@ class TestFind(base.TestCase): FakeResource.id_attribute = 'id' p = {'fields': 'ip_address', 'ip_address': "127.0.0.1"} - self.mock_get.assert_called_with(fake_path, params=p, service=None) + self.mock_get.assert_any_call(fake_path, params=p, service=None) def test_nada(self): resp = FakeResponse({FakeResource.resources_key: []})