WIP: Super refactoring 8

gem/lib/pagy/backend.rb

@@ -1,8 +1,8 @@
 # See Pagy::Backend API documentation: https://ddnexus.github.io/pagy/docs/api/backend
 # frozen_string_literal: true
 
+require_relative 'helpers/url'
 require_relative 'loaders/backend'
-
 class Pagy
   # Define a few generic methods to paginate a collection out of the box,
   # or any collection by overriding any of the `pagy_*` methods in your controller.
@@ -29,16 +29,30 @@ def pagy_get_limit(vars)
     # Get the limit from the request
     # Overridable by the jsonapi extra
     def pagy_requested_limit(vars)
-      params[vars[:limit_sym] || DEFAULT[:limit_sym]]
+      if pagy_skip_jsonapi?(vars)
+        params[vars[:limit_sym] || DEFAULT[:limit_sym]]
+      else
+        params[:page][vars[:limit_sym] || DEFAULT[:limit_sym]]
+      end
     end
 
     # Get the page integer from the params
     # Overridable by the jsonapi extra
     def pagy_get_page(vars, force_integer: true)
-      page = params[vars[:page_sym] || DEFAULT[:page_sym]]
+      page = if pagy_skip_jsonapi?(vars)
+               params[vars[:page_sym] || DEFAULT[:page_sym]]
+             else
+               params[:page][vars[:page_sym] || DEFAULT[:page_sym]]
+             end
       force_integer ? (page || 1).to_i : page
     end
 
+    def pagy_skip_jsonapi?(vars)
+      return true unless (vars.key?(:jsonapi) ? vars[:jsonapi] : DEFAULT[:jsonapi]) && params[:page]
+
+      raise ReservedParamError, params[:page] unless params[:page].respond_to?(:fetch)
+    end
+
     include Loaders::Backend
   end
 end

gem/lib/pagy/exceptions.rb

@@ -22,4 +22,12 @@ class I18nError < StandardError; end
 
   # Generic internal error
   class InternalError < StandardError; end
+
+  # JsonApi :page param error
+  class ReservedParamError < StandardError
+    # Inform about the actual value
+    def initialize(value)
+      super("expected reserved :page param to be nil or Hash-like; got #{value.inspect}")
+    end
+  end
 end

gem/lib/pagy/helpers/url.rb

@@ -41,25 +41,19 @@ def escape(str)
     # It supports all Rack-based frameworks (Sinatra, Padrino, Rails, ...).
     # For non-rack environments you can just set the :url variable
     def pagy_page_url(pagy, page, absolute: false, fragment: nil, **_)
-      vars         = pagy.vars
-      query_params = vars[:url] ? {} : request.GET.clone(freeze: false)
-      query_params.merge!(vars[:params].transform_keys(&:to_s)) if vars[:params].is_a?(Hash)
-      pagy_set_query_params(page, vars, query_params)
-      query_params = vars[:params].(query_params) if vars[:params].is_a?(Proc)
-      query_string = "?#{QueryUtils.build_nested_query(query_params, nil,
-                                                       %i[page_sym limit_sym].map { |k| pagy.vars[k].to_s })}"
-      if vars[:url]
-        "#{vars[:url]}#{query_string}#{fragment}"
-      else
-        "#{request.base_url if absolute}#{vars[:request_path] || request.path}#{query_string}#{fragment}"
-      end
-    end
+      pagy_params, url      = (vars = pagy.vars).values_at(:params, :url)
+      page_name, limit_name = vars.values_at(:page_sym, :limit_sym).map(&:to_s)
+
+      query_params = url ? {} : request.GET.clone(freeze: false)
+      query_params.merge!(pagy_params.transform_keys(&:to_s)) if pagy_params.is_a?(Hash)
+      page_and_limit = { page_name => page }.tap { |h| h[limit_name] = vars[:limit] if vars[:limit_requestable] }
+      query_params.merge!(vars[:jsonapi] ? { 'page' => page_and_limit } : page_and_limit)
+
+      query_params = pagy_params.(query_params) if pagy_params.is_a?(Proc)
+      query_string = "?#{QueryUtils.build_nested_query(query_params, nil, [page_name, limit_name])}"
+      return "#{url}#{query_string}#{fragment}" if url
 
-    # Add the page and limit params
-    # Overridable by the jsonapi extra
-    def pagy_set_query_params(page, vars, query_params)
-      query_params[vars[:page_sym].to_s]  = page
-      query_params[vars[:limit_sym].to_s] = vars[:limit] if vars[:limit_requestable]
+      "#{request.base_url if absolute}#{vars[:request_path] || request.path}#{query_string}#{fragment}"
     end
   end
 end

gem/lib/pagy/loaders/backend.rb

@@ -23,7 +23,8 @@ def pagy_load_backend(...)
                                 pagy_meilisearch:         PAGY_PATH.join('mixins/meilisearch').to_s,
                                 pagy_metadata:            PAGY_PATH.join('mixins/metadata').to_s,
                                 pagy_offset:              PAGY_PATH.join('mixins/offset').to_s,
-                                pagy_searchkick:          PAGY_PATH.join('mixins/searchkick').to_s }.freeze
+                                pagy_searchkick:          PAGY_PATH.join('mixins/searchkick').to_s,
+                                pagy_links:               PAGY_PATH.join('mixins/links') }.freeze
 
       BACKEND_METHOD_MIXINS.each_key do |method|
         class_eval "alias #{method} pagy_load_backend", __FILE__, __LINE__  # alias pagy_* pagy_load_backend

gem/lib/pagy/mixins/headers.rb

@@ -2,7 +2,7 @@
 # frozen_string_literal: true
 
 require_relative '../helpers/url'
-require_relative '../helpers/links'
+require_relative 'links'
 
 class Pagy
   DEFAULT[:headers] = { page:  'current-page',
@@ -14,7 +14,6 @@ class Pagy
     private
 
     include Url
-    include Links
 
     # Merge the pagy headers into the response.headers
     def pagy_headers_merge(pagy)

gem/lib/pagy/helpers/links.rb → gem/lib/pagy/mixins/links.rb

@@ -1,7 +1,8 @@
 # frozen_string_literal: true
 
+# require_relative
 class Pagy
-  module Links
+  Backend.class_eval do
     def pagy_links(pagy, **)
       url_str = pagy_page_url(pagy, PAGE_TOKEN, **)
       { first: url_str.sub(PAGE_TOKEN, '1') }.tap do |links|

test/pagy/extras/jsonapi_test.rb

@@ -1,26 +1,22 @@
 # frozen_string_literal: true
 
 require_relative '../../test_helper'
-require 'pagy/extras/jsonapi'
 
 require_relative '../../mock_helpers/collection'
 require_relative '../../files/models'
 require_relative '../../mock_helpers/app'
 
 Pagy::DEFAULT[:limit_requestable] = true
+Pagy::DEFAULT[:jsonapi]           = true
 
-describe 'pagy/mixins/jsonapi' do
+describe 'jsonapi' do
   before do
     @collection = MockCollection.new
   end
 
-  it 'defaults to true' do
-    _(Pagy::DEFAULT[:jsonapi]).must_equal true
-  end
-
   it 'raises PageParamError with page number' do
     app = MockApp.new(params: { page: 2 })
-    _ { _pagy, _records = app.send(:pagy_offset, @collection) }.must_raise Pagy::JsonApiExtra::ReservedParamError
+    _ { _pagy, _records = app.send(:pagy_offset, @collection) }.must_raise Pagy::ReservedParamError
   end
 
   describe 'JsonApi' do
@@ -50,13 +46,11 @@
       Pagy::DEFAULT[:jsonapi] = true
     end
     it 'skips the :jsonapi with page:3' do
-      Pagy::DEFAULT[:jsonapi] = false
       app = MockApp.new(params: { page: 3 })
-      pagy, _records = app.send(:pagy_offset, @collection, limit_requestable: false)
+      pagy, _records = app.send(:pagy_offset, @collection, jsonapi: false, limit_requestable: false)
       _(app.send(:pagy_page_url, pagy, 2)).must_equal '/foo?page=2'
-      pagy, _records = app.send(:pagy_offset, @collection)
+      pagy, _records = app.send(:pagy_offset, @collection, jsonapi: false)
       _(app.send(:pagy_page_url, pagy, 2)).must_equal '/foo?page=2&limit=20'
-      Pagy::DEFAULT[:jsonapi] = true
     end
   end
   describe 'JsonApi with custom named params' do
@@ -72,35 +66,35 @@
       _(app.send(:pagy_page_url, pagy, 4)).must_rematch :url
     end
   end
-  describe '#pagy_jsonapi_links' do
+  describe '#pagy_links' do
     it 'returns the ordered links' do
       app = MockApp.new(params: { page: { number: 3, size: 10 } })
       pagy, _records = app.send(:pagy_offset, @collection, page_sym: :number, limit_sym: :size)
-      result = app.send(:pagy_jsonapi_links, pagy)
+      result = app.send(:pagy_links, pagy)
       _(result.keys).must_equal %i[first prev next last]
       _(result).must_rematch :result
     end
     it 'sets the prev value to null when the link is unavailable' do
       app = MockApp.new(params: { page: { page: 1 } })
       pagy, _records = app.send(:pagy_offset, @collection)
-      result = app.send(:pagy_jsonapi_links, pagy)
+      result = app.send(:pagy_links, pagy)
       _(result[:prev]).must_be_nil
     end
     it 'sets the next value to null when the link is unavailable' do
       app = MockApp.new(params: { page: { page: 50 } })
       pagy, _records = app.send(:pagy_offset, @collection)
-      result = app.send(:pagy_jsonapi_links, pagy)
+      result = app.send(:pagy_links, pagy)
       _(result[:next]).must_be_nil
     end
   end
-  describe '#pagy_jsonapi_links (keyset)' do
+  describe '#pagy_links (keyset)' do
     it 'returns the ordered links' do
       app = MockApp.new(params: { page: { latest: 'WzIwXQ', size: 10 } })
       pagy, _records = app.send(:pagy_keyset,
                                 Pet.order(:id),
                                 page_sym:  :latest,
                                 limit_sym: :size)
-      result = app.send(:pagy_jsonapi_links, pagy)
+      result = app.send(:pagy_links, pagy)
       _(result.keys).must_equal %i[first next]
       _(result).must_rematch :keyset_result
     end
@@ -111,7 +105,7 @@
                                 Pet.order(:id),
                                 page_sym:  :latest,
                                 limit_sym: :size)
-      result = app.send(:pagy_jsonapi_links, pagy)
+      result = app.send(:pagy_links, pagy)
       _(result).must_rematch :keyset_result
       _(result[:next]).must_be_nil
     end

test/pagy/extras/jsonapi_test.rb.yaml

@@ -1,22 +1,22 @@
 ---
-pagy/mixins/jsonapi___pagy_jsonapi_links_(keyset)_test_0002_sets_the_next_value_to_null_when_the_link_is_unavailable:
-  :keyset_result:
-    :first: "/foo?page%5Bsize%5D=50&page%5Blatest%5D=1"
-pagy/mixins/jsonapi___pagy_jsonapi_links_(keyset)_test_0001_returns_the_ordered_links:
-  :keyset_result:
-    :first: "/foo?page%5Blatest%5D=1&page%5Bsize%5D=10"
-    :next: "/foo?page%5Blatest%5D=WzMwXQ&page%5Bsize%5D=10"
-pagy/mixins/jsonapi__JsonApi_with_custom_named_params_test_0002_sets_custom_named_params:
+jsonapi__JsonApi_with_custom_named_params_test_0002_sets_custom_named_params:
   :url: "/foo?page%5Bnumber%5D=4&page%5Bsize%5D=10"
-pagy/mixins/jsonapi___pagy_jsonapi_links_test_0001_returns_the_ordered_links:
+jsonapi___pagy_links_test_0001_returns_the_ordered_links:
   :result:
     :first: "/foo?page%5Bnumber%5D=1&page%5Bsize%5D=10"
     :prev: "/foo?page%5Bnumber%5D=2&page%5Bsize%5D=10"
     :next: "/foo?page%5Bnumber%5D=4&page%5Bsize%5D=10"
     :last: "/foo?page%5Bnumber%5D=100&page%5Bsize%5D=10"
-pagy/mixins/jsonapi__JsonApi_test_0002_uses_the__jsonapi_with_page_3:
-  :url_1: "/foo?page%5Bpage%5D=2"
-  :url_2: "/foo?page%5Bpage%5D=2&page%5Blimit%5D=20"
-pagy/mixins/jsonapi__JsonApi_test_0001_uses_the__jsonapi_with_page_nil:
+jsonapi__JsonApi_test_0001_uses_the__jsonapi_with_page_nil:
   :url_1: "/foo?page%5Bpage%5D=1"
   :url_2: "/foo?page%5Bpage%5D=1&page%5Blimit%5D=20"
+jsonapi__JsonApi_test_0002_uses_the__jsonapi_with_page_3:
+  :url_1: "/foo?page%5Bpage%5D=2"
+  :url_2: "/foo?page%5Bpage%5D=2&page%5Blimit%5D=20"
+jsonapi___pagy_links_(keyset)_test_0001_returns_the_ordered_links:
+  :keyset_result:
+    :first: "/foo?page%5Blatest%5D=1&page%5Bsize%5D=10"
+    :next: "/foo?page%5Blatest%5D=WzMwXQ&page%5Bsize%5D=10"
+jsonapi___pagy_links_(keyset)_test_0002_sets_the_next_value_to_null_when_the_link_is_unavailable:
+  :keyset_result:
+    :first: "/foo?page%5Blatest%5D=1&page%5Bsize%5D=50"