Merge "Implement iterator paging"

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)

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)

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: []})