Determine limit based on page size

Without a limit parameter being provided, Resource.list required making
one call past the end of data in order to know it was done -- when it
received an empty response body.

This change figures out the limit based on the count of items returned,
then uses that in subsequent requests. The bigger gain here is that we
now know how many items constitutes a full page, so we can stop making
requests for more pages once we've received a smaller page than the
known limit.

Change-Id: I5b7739f14a8934b6c7a53987a2aec0f251fb240b

openstack/resource.py

@@ -804,6 +804,11 @@ class Resource(collections.MutableMapping):
         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.
+
         :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.
@@ -837,11 +842,6 @@ class Resource(collections.MutableMapping):
         while more_data:
             resp = cls.page(session, limit, marker, path_args, **params)
 
-            # 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
 
@@ -858,6 +858,8 @@ class Resource(collections.MutableMapping):
             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.

openstack/tests/unit/object_store/v1/test_proxy.py

@@ -122,10 +122,11 @@ class Test_containers(TestObjectStoreProxy, base.TestTransportBase):
     @httpretty.activate
     def test_containers_with_marker(self):
         marker = six.text_type("container2")
-        marker_param = "?marker=%s" % marker
+        marker_param = "marker=%s" % marker
 
         self.stub_url(httpretty.GET,
-                      path=[container.Container.base_path + marker_param],
+                      path=[container.Container.base_path + "?" +
+                            marker_param],
                       json=self.containers_body)
 
         count = 0
@@ -302,11 +303,12 @@ class Test_objects(TestObjectStoreProxy, base.TestTransportBase):
     @httpretty.activate
     def test_objects_with_marker(self):
         marker = six.text_type("object2")
-        marker_param = "?marker=%s" % marker
+        marker_param = "marker=%s" % marker
 
         self.stub_url(httpretty.GET,
                       path=[obj.Object.base_path %
-                            {"container": self.container_name} + marker_param],
+                            {"container": self.container_name} + "?" +
+                            marker_param],
                       json=self.objects_body)
 
         count = 0

openstack/tests/unit/test_resource.py

@@ -764,6 +764,23 @@ 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):
+        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
+
+        # 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()