Fixed the incorrect behaviour of limit() and page() methods

Previously:

  When klass.default_per_page is 25,

  response.limit(35).page(3)
  => :size is back to 25, :from is 25 * 2

  response.page(3).limit(35)
  => :size is 35, but :from is 25 * 2

After this patch:

  response.limit(35).page(3)
  => :size is 35, :from is 35 * 2

  response.page(3).limit(35)
  => :size is 35, :from is 35 * 2

Closes elastic#42

Related: elastic#72, elastic#55

Author: joker1007

Diff for: elasticsearch-model/lib/elasticsearch/model/response/pagination.rb

@@ -30,8 +30,12 @@ def #{::Kaminari.config.page_method_name}(num=nil)
                 @results  = nil
                 @records  = nil
                 @response = nil
-                self.search.definition.update size: klass.default_per_page,
-                                              from: klass.default_per_page * ([num.to_i, 1].max - 1)
+                @page     = [num.to_i, 1].max
+                @per_page ||= klass.default_per_page
+
+                self.search.definition.update size: @per_page,
+                                              from: @per_page * (@page - 1)
+
                 self
               end
             RUBY
@@ -69,7 +73,10 @@ def limit(value)
             @results  = nil
             @records  = nil
             @response = nil
-            search.definition.update :size => value
+            @per_page = value
+
+            search.definition.update :size => @per_page
+            search.definition.update :from => @per_page * (@page - 1) if @page
             self
           end
 
@@ -79,6 +86,7 @@ def offset(value)
             @results  = nil
             @records  = nil
             @response = nil
+            @page     = nil
             search.definition.update :from => value
             self
           end

Diff for: elasticsearch-model/test/integration/active_record_pagination_test.rb

@@ -102,16 +102,28 @@ class ::ArticleForPagination < ActiveRecord::Base
           assert_equal 12, records.size
         end
 
+        should "set the limit per request" do
+          records = ArticleForPagination.search('title:test').limit(50).page(2).records
+
+          assert_equal 18,  records.size
+          assert_equal 2,   records.current_page
+          assert_equal 1,   records.prev_page
+          assert_equal nil, records.next_page
+          assert_equal 2,   records.total_pages
+
+          assert records.last_page?, "Should be the last page"
+        end
+
         context "with specific model settings" do
           teardown do
             ArticleForPagination.instance_variable_set(:@_default_per_page, nil)
           end
-        end
 
-        should "respect paginates_per" do
-          ArticleForPagination.paginates_per 50
+          should "respect paginates_per" do
+            ArticleForPagination.paginates_per 50
 
-          assert_equal 50, ArticleForPagination.search('*').page(1).records.size
+            assert_equal 50, ArticleForPagination.search('*').page(1).records.size
+          end
         end
       end
 

Diff for: elasticsearch-model/test/unit/response_pagination_test.rb

@@ -103,6 +103,33 @@ def self.document_type; 'bar'; end
       end
     end
 
+    context "with the page() and limit() methods" do
+      setup do
+        @response.records
+        @response.results
+      end
+
+      should "set the values" do
+        @response.page(3).limit(35)
+        assert_equal 35, @response.search.definition[:size]
+        assert_equal 70, @response.search.definition[:from]
+      end
+
+      should "set the values when limit is called first" do
+        @response.limit(35).page(3)
+        assert_equal 35, @response.search.definition[:size]
+        assert_equal 70, @response.search.definition[:from]
+      end
+
+      should "reset the instance variables" do
+        @response.page(3).limit(35)
+
+        assert_nil @response.instance_variable_get(:@response)
+        assert_nil @response.instance_variable_get(:@records)
+        assert_nil @response.instance_variable_get(:@results)
+      end
+    end
+
     context "offset setter" do
       setup do
         @response.records