fix find_in_batches for other pagination types (#2)

Author: spape

lib/dhs/concerns/record/batch.rb

@@ -22,17 +22,18 @@ def find_each(options = {})
       def find_in_batches(options = {})
         raise 'No block given' unless block_given?
         options = options.dup
-        start = options.delete(:start) || 1
-        batch_size = options.delete(:batch_size) || DHS::Pagination::Base::DEFAULT_LIMIT
+        start = options[:start] || self.pagination_class::DEFAULT_OFFSET
+        batch_size = options.delete(:batch_size) || self.pagination_class::DEFAULT_LIMIT
         loop do # as suggested by Matz
           options = options.dup
           options[:params] = (options[:params] || {}).merge(limit_key(:parameter) => batch_size, pagination_key(:parameter) => start)
           data = request(options)
-          batch_size = data._raw.dig(*limit_key(:body))
-          left = data._raw.dig(*total_key).to_i - data._raw.dig(*pagination_key(:body)).to_i - data._raw.dig(*limit_key(:body)).to_i
+          pagination = self.pagination(data)
+          batch_size = pagination.limit
+          left = pagination.pages_left
           yield new(data)
           break if left <= 0
-          start += batch_size
+          start = pagination.next_offset
         end
       end
     end

lib/dhs/version.rb

@@ -1,5 +1,5 @@
 # frozen_string_literal: true
 
 module DHS
-  VERSION = '1.0.1'
+  VERSION = '1.0.2'
 end

spec/record/find_each_spec.rb

@@ -29,11 +29,11 @@ class Record < DHS::Record
 
   context 'find_each' do
     it 'processes each record by fetching records in batches' do
-      stub_request(:get, "#{datastore}/feedbacks?limit=100&offset=1").to_return(status: 200, body: api_response((1..100).to_a, 1))
-      stub_request(:get, "#{datastore}/feedbacks?limit=100&offset=101").to_return(status: 200, body: api_response((101..200).to_a, 101))
-      stub_request(:get, "#{datastore}/feedbacks?limit=100&offset=201").to_return(status: 200, body: api_response((201..300).to_a, 201))
-      stub_request(:get, "#{datastore}/feedbacks?limit=100&offset=301").to_return(status: 200, body: api_response((301..400).to_a, 301))
-      stub_request(:get, "#{datastore}/feedbacks?limit=100&offset=401").to_return(status: 200, body: api_response((401..total).to_a, 401))
+      stub_request(:get, "#{datastore}/feedbacks?limit=100&offset=0").to_return(status: 200, body: api_response((1..100).to_a, 0))
+      stub_request(:get, "#{datastore}/feedbacks?limit=100&offset=100").to_return(status: 200, body: api_response((101..200).to_a, 100))
+      stub_request(:get, "#{datastore}/feedbacks?limit=100&offset=200").to_return(status: 200, body: api_response((201..300).to_a, 200))
+      stub_request(:get, "#{datastore}/feedbacks?limit=100&offset=300").to_return(status: 200, body: api_response((301..400).to_a, 300))
+      stub_request(:get, "#{datastore}/feedbacks?limit=100&offset=400").to_return(status: 200, body: api_response((401..total).to_a, 400))
       count = 0
       Record.find_each do |record|
         count += 1
@@ -45,7 +45,7 @@ class Record < DHS::Record
     end
 
     it 'passes options to the requests made' do
-      request = stub_request(:get, 'http://depay.fi/v2/feedbacks?limit=100&offset=1')
+      request = stub_request(:get, 'http://depay.fi/v2/feedbacks?limit=100&offset=0')
         .with(headers: { 'Authorization' => 'Bearer 123' })
         .to_return(body: {
           items: []

spec/record/find_in_batches_spec.rb

@@ -29,11 +29,11 @@ class Record < DHS::Record
 
   context 'find_batches' do
     it 'processes records in batches' do
-      stub_request(:get, "#{datastore}/feedbacks?limit=100&offset=1").to_return(status: 200, body: api_response((1..100).to_a, 1))
-      stub_request(:get, "#{datastore}/feedbacks?limit=100&offset=101").to_return(status: 200, body: api_response((101..200).to_a, 101))
-      stub_request(:get, "#{datastore}/feedbacks?limit=100&offset=201").to_return(status: 200, body: api_response((201..300).to_a, 201))
-      stub_request(:get, "#{datastore}/feedbacks?limit=100&offset=301").to_return(status: 200, body: api_response((301..400).to_a, 301))
-      stub_request(:get, "#{datastore}/feedbacks?limit=100&offset=401").to_return(status: 200, body: api_response((401..total).to_a, 401))
+      stub_request(:get, "#{datastore}/feedbacks?limit=100&offset=0").to_return(status: 200, body: api_response((1..100).to_a, 0))
+      stub_request(:get, "#{datastore}/feedbacks?limit=100&offset=100").to_return(status: 200, body: api_response((101..200).to_a, 100))
+      stub_request(:get, "#{datastore}/feedbacks?limit=100&offset=200").to_return(status: 200, body: api_response((201..300).to_a, 200))
+      stub_request(:get, "#{datastore}/feedbacks?limit=100&offset=300").to_return(status: 200, body: api_response((301..400).to_a, 300))
+      stub_request(:get, "#{datastore}/feedbacks?limit=100&offset=400").to_return(status: 200, body: api_response((401..total).to_a, 400))
       length = 0
       Record.find_in_batches do |records|
         length += records.length
@@ -44,11 +44,11 @@ class Record < DHS::Record
     end
 
     it 'adapts to backend max limit' do
-      stub_request(:get, "#{datastore}/feedbacks?limit=230&offset=1").to_return(status: 200, body: api_response((1..100).to_a, 1))
-      stub_request(:get, "#{datastore}/feedbacks?limit=100&offset=101").to_return(status: 200, body: api_response((101..200).to_a, 101))
-      stub_request(:get, "#{datastore}/feedbacks?limit=100&offset=201").to_return(status: 200, body: api_response((201..300).to_a, 201))
-      stub_request(:get, "#{datastore}/feedbacks?limit=100&offset=301").to_return(status: 200, body: api_response((301..400).to_a, 301))
-      stub_request(:get, "#{datastore}/feedbacks?limit=100&offset=401").to_return(status: 200, body: api_response((401..total).to_a, 401))
+      stub_request(:get, "#{datastore}/feedbacks?limit=230&offset=0").to_return(status: 200, body: api_response((1..100).to_a, 0))
+      stub_request(:get, "#{datastore}/feedbacks?limit=100&offset=100").to_return(status: 200, body: api_response((101..200).to_a, 100))
+      stub_request(:get, "#{datastore}/feedbacks?limit=100&offset=200").to_return(status: 200, body: api_response((201..300).to_a, 200))
+      stub_request(:get, "#{datastore}/feedbacks?limit=100&offset=300").to_return(status: 200, body: api_response((301..400).to_a, 300))
+      stub_request(:get, "#{datastore}/feedbacks?limit=100&offset=400").to_return(status: 200, body: api_response((401..total).to_a, 400))
       length = 0
       Record.find_in_batches(batch_size: 230) do |records|
         length += records.length
@@ -59,8 +59,8 @@ class Record < DHS::Record
     end
 
     it 'forwards offset' do
-      stub_request(:get, "#{datastore}/feedbacks?limit=100&offset=401").to_return(status: 200, body: api_response((401..total).to_a, 401))
-      Record.find_in_batches(start: 401) do |records|
+      stub_request(:get, "#{datastore}/feedbacks?limit=100&offset=400").to_return(status: 200, body: api_response((401..total).to_a, 400))
+      Record.find_in_batches(start: 400) do |records|
         expect(records.length).to eq(total - 400)
       end
     end
@@ -119,4 +119,42 @@ class Record < DHS::Record
       expect(length).to eq total
     end
   end
+
+  context 'different pagination strategy' do
+    before do
+      class Transaction < DHS::Record
+        endpoint 'https://api/transactions'
+        configuration limit_key: :items_on_page, pagination_strategy: :total_pages, pagination_key: :page, items_key: :transactions, total_key: :total_pages
+      end
+
+      stub_request(:get, 'https://api/transactions?items_on_page=50&page=1')
+        .to_return(body: {
+          page: 1,
+          total_pages: 2,
+          items_on_page: 50,
+          transactions: 50.times.map { |index| { id: index } }
+        }.to_json)
+
+      stub_request(:get, 'https://api/transactions?items_on_page=50&page=2')
+        .to_return(body: {
+          page: 2,
+          total_pages: 2,
+          items_on_page: 50,
+          transactions: 22.times.map { |index| { id: 50 + index } }
+        }.to_json)
+    end
+
+    it 'find in batches and paginates automatically even for different pagination strategies' do
+      total = 0
+      transactions = []
+
+      Transaction.find_in_batches(batch_size: 50) do |batch|
+        total += batch.length
+        transactions << batch
+      end
+
+      expect(total).to eq(72)
+      expect(transactions.flatten.as_json).to eq(72.times.map { |index| { id: index }.as_json })
+    end
+  end
 end