Support backwards pagination with list's #auto_paging_each (#865)

* Support backwards pagination with list's `#auto_paging_each`

Previously, `#auto_paging_each` would always try to paginate forward,
even if it was clear based on the list's current filters that the user
had been intending to iterate backwards by specifying an `ending_before`
filter exclusively.

Here we implement backwards iteration by detecting this condition,
reversing the current list data, and making new requests for the
previous page (instead of the next one) as needed, which allows the user
to handle elements in reverse logical order.

Reversing the current page's list is intended as a minor user feature,
but may possibly be contentious. For background, when backwards
iterating in the API, results are still returned in "normal" order. So
if I specifying `ending_before=7`, the next page would look like `[4, 5,
6`] instead of `[6, 5, 4]`. In `#auto_paging_each` I reverse it to `[6,
5, 4]` so it feels to the user like they're handling elements in the
order they're iterating, which I think is okay. The reason it might be
contentious though is that it could be a tad confusing to someone who
already understands the normal `ending_before` ordering in the API.

Fixes #864.

* Allow `ending_before` and `starting_after` to remain in hydrated list object

lib/stripe/api_operations/list.rb

@@ -11,12 +11,7 @@ def list(filters = {}, opts = {})
 
         # set filters so that we can fetch the same limit, expansions, and
         # predicates when accessing the next and previous pages
-        #
-        # just for general cleanliness, remove any paging options
         obj.filters = filters.dup
-        obj.filters.delete(:ending_before)
-        obj.filters.delete(:starting_after)
-
         obj
       end
     end

lib/stripe/list_object.rb

@@ -59,8 +59,18 @@ def auto_paging_each(&blk)
 
       page = self
       loop do
-        page.each(&blk)
-        page = page.next_page
+        # Backward iterating activates if we have an `ending_before` constraint
+        # and _just_ an `ending_before` constraint. If `starting_after` was
+        # also used, we iterate forwards normally.
+        if filters.include?(:ending_before) &&
+           !filters.include?(:starting_after)
+          page.reverse_each(&blk)
+          page = page.previous_page
+        else
+          page.each(&blk)
+          page = page.next_page
+        end
+
         break if page.empty?
       end
     end
@@ -96,6 +106,8 @@ def next_page(params = {}, opts = {})
     # This method will try to respect the limit of the current page. If none
     # was given, the default limit will be fetched again.
     def previous_page(params = {}, opts = {})
+      return self.class.empty_list(opts) unless has_more
+
       first_id = data.first.id
 
       params = filters.merge(ending_before: first_id).merge(params)
@@ -107,5 +119,11 @@ def resource_url
       url ||
         raise(ArgumentError, "List object does not contain a 'url' field.")
     end
+
+    # Iterates through each resource in the page represented by the current
+    # `ListObject` in reverse.
+    def reverse_each(&blk)
+      data.reverse_each(&blk)
+    end
   end
 end

test/stripe/list_object_test.rb

@@ -25,20 +25,80 @@ class ListObjectTest < Test::Unit::TestCase
       assert_equal expected, list.each.to_a
     end
 
-    should "provide #auto_paging_each" do
+    should "provide #reverse_each" do
       arr = [
         { id: 1 },
         { id: 2 },
         { id: 3 },
       ]
+      expected = Util.convert_to_stripe_object(arr.reverse, {})
+      list = Stripe::ListObject.construct_from(data: arr)
+      assert_equal expected, list.reverse_each.to_a
+    end
+
+    should "provide #auto_paging_each that supports forward pagination" do
+      arr = [
+        { id: 1 },
+        { id: 2 },
+        { id: 3 },
+        { id: 4 },
+        { id: 5 },
+        { id: 6 },
+      ]
       expected = Util.convert_to_stripe_object(arr, {})
 
+      # Initial list object to page on. Notably, its last data element will be
+      # used as a cursor to fetch the next page.
       list = TestListObject.construct_from(data: [{ id: 1 }],
                                            has_more: true,
                                            url: "/things")
+      list.filters = { limit: 3 }
+
+      # The test will start with the synthetic list object above, and use it as
+      # a starting point to fetch two more pages. The second page indicates
+      # that there are no more elements by setting `has_more` to `false`, and
+      # iteration stops.
       stub_request(:get, "#{Stripe.api_base}/things")
-        .with(query: { starting_after: "1" })
-        .to_return(body: JSON.generate(data: [{ id: 2 }, { id: 3 }], has_more: false))
+        .with(query: { starting_after: "1", limit: "3" })
+        .to_return(body: JSON.generate(data: [{ id: 2 }, { id: 3 }, { id: 4 }], has_more: true, url: "/things"))
+      stub_request(:get, "#{Stripe.api_base}/things")
+        .with(query: { starting_after: "4", limit: "3" })
+        .to_return(body: JSON.generate(data: [{ id: 5 }, { id: 6 }], has_more: false, url: "/things"))
+
+      assert_equal expected, list.auto_paging_each.to_a
+    end
+
+    should "provide #auto_paging_each that supports backward pagination with `ending_before`" do
+      arr = [
+        { id: 6 },
+        { id: 5 },
+        { id: 4 },
+        { id: 3 },
+        { id: 2 },
+        { id: 1 },
+      ]
+      expected = Util.convert_to_stripe_object(arr, {})
+
+      # Initial list object to page on. Notably, its first data element will be
+      # used as a cursor to fetch the next page.
+      list = TestListObject.construct_from(data: [{ id: 6 }],
+                                           has_more: true,
+                                           url: "/things")
+
+      # We also add an `ending_before` filter on the list to simulate backwards
+      # pagination.
+      list.filters = { ending_before: 7, limit: 3 }
+
+      # The test will start with the synthetic list object above, and use it as
+      # a starting point to fetch two more pages. The second page indicates
+      # that there are no more elements by setting `has_more` to `false`, and
+      # iteration stops.
+      stub_request(:get, "#{Stripe.api_base}/things")
+        .with(query: { ending_before: "6", limit: "3" })
+        .to_return(body: JSON.generate(data: [{ id: 3 }, { id: 4 }, { id: 5 }], has_more: true, url: "/things"))
+      stub_request(:get, "#{Stripe.api_base}/things")
+        .with(query: { ending_before: "3", limit: "3" })
+        .to_return(body: JSON.generate(data: [{ id: 1 }, { id: 2 }], has_more: false, url: "/things"))
 
       assert_equal expected, list.auto_paging_each.to_a
     end
@@ -97,7 +157,7 @@ class ListObjectTest < Test::Unit::TestCase
         .with(query: { "expand[]" => "data.source", "limit" => "3", "starting_after" => "1" })
         .to_return(body: JSON.generate(data: [{ id: 2 }], has_more: false))
       next_list = list.next_page
-      assert_equal({ expand: ["data.source"], limit: 3 }, next_list.filters)
+      assert_equal({ expand: ["data.source"], limit: 3, starting_after: 1 }, next_list.filters)
     end
 
     should "fetch an empty page through #next_page" do
@@ -114,23 +174,25 @@ class ListObjectTest < Test::Unit::TestCase
 
     should "fetch a next page through #previous_page" do
       list = TestListObject.construct_from(data: [{ id: 2 }],
+                                           has_more: true,
                                            url: "/things")
       stub_request(:get, "#{Stripe.api_base}/things")
         .with(query: { ending_before: "2" })
-        .to_return(body: JSON.generate(data: [{ id: 1 }]))
+        .to_return(body: JSON.generate(data: [{ id: 1 }], has_more: false))
       next_list = list.previous_page
       refute next_list.empty?
     end
 
     should "fetch a next page through #previous_page and respect limit" do
       list = TestListObject.construct_from(data: [{ id: 2 }],
+                                           has_more: true,
                                            url: "/things")
       list.filters = { expand: ["data.source"], limit: 3 }
       stub_request(:get, "#{Stripe.api_base}/things")
         .with(query: { "expand[]" => "data.source", "limit" => "3", "ending_before" => "2" })
-        .to_return(body: JSON.generate(data: [{ id: 1 }]))
+        .to_return(body: JSON.generate(data: [{ id: 1 }], has_more: false))
       next_list = list.previous_page
-      assert_equal({ expand: ["data.source"], limit: 3 }, next_list.filters)
+      assert_equal({ ending_before: 2, expand: ["data.source"], limit: 3 }, next_list.filters)
     end
   end
 end