From b95a82eeb8cb4dcebf5f5af260f9d44854ead91b Mon Sep 17 00:00:00 2001
From: Domizio Demichelis <dd.nexus@gmail.com>
Date: Mon, 13 Jan 2025 18:37:38 +0700
Subject: [PATCH] WIP: Super refactoring 8
---
gem/lib/pagy/backend.rb | 20 ++++++-
gem/lib/pagy/exceptions.rb | 8 +++
gem/lib/pagy/extras/jsonapi.rb | 70 -----------------------
gem/lib/pagy/helpers/url.rb | 28 ++++-----
gem/lib/pagy/loaders/backend.rb | 3 +-
gem/lib/pagy/mixins/headers.rb | 3 +-
gem/lib/pagy/{helpers => mixins}/links.rb | 3 +-
test/pagy/extras/jsonapi_test.rb | 30 ++++------
test/pagy/extras/jsonapi_test.rb.yaml | 26 ++++-----
9 files changed, 66 insertions(+), 125 deletions(-)
delete mode 100644 gem/lib/pagy/extras/jsonapi.rb
rename gem/lib/pagy/{helpers => mixins}/links.rb (91%)
diff --git a/gem/lib/pagy/backend.rb b/gem/lib/pagy/backend.rb
index 25256dd0e..6b807ab04 100644
--- a/gem/lib/pagy/backend.rb
+++ b/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
diff --git a/gem/lib/pagy/exceptions.rb b/gem/lib/pagy/exceptions.rb
index d35a14de4..a30a851a1 100644
--- a/gem/lib/pagy/exceptions.rb
+++ b/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
diff --git a/gem/lib/pagy/extras/jsonapi.rb b/gem/lib/pagy/extras/jsonapi.rb
deleted file mode 100644
index 7067c5015..000000000
--- a/gem/lib/pagy/extras/jsonapi.rb
+++ /dev/null
@@ -1,70 +0,0 @@
-# See the Pagy documentation: https://ddnexus.github.io/pagy/docs/extras/jsonapi
-# frozen_string_literal: true
-
-require_relative '../helpers/url'
-require_relative '../helpers/links'
-
-class Pagy
- DEFAULT[:jsonapi] = true
-
- # Add a specialized backend method compliant with JSON:API
- module JsonApiExtra
- # 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
-
- # Module overriding Backend
- module BackendOverride
- private
-
- include Url
- include Links
-
- # Return the jsonapi links
- alias pagy_jsonapi_links pagy_links
-
- # Should skip the jsonapi
- def pagy_skip_jsonapi?(vars)
- return true if vars[:jsonapi] == false || (vars[:jsonapi].nil? && DEFAULT[:jsonapi] == false)
- # check the reserved :page param
- raise ReservedParamError, params[:page] unless params[:page].respond_to?(:fetch) || params[:page].nil?
- end
-
- # Override the Backend method
- def pagy_get_page(vars, force_integer: true)
- return super if pagy_skip_jsonapi?(vars) || params[:page].nil?
-
- page = params[:page][vars[:page_sym] || DEFAULT[:page_sym]]
- force_integer ? (page || 1).to_i : page
- end
-
- # Override the backend method
- def pagy_requested_limit(vars)
- return super if pagy_skip_jsonapi?(vars)
- return unless params[:page]
-
- params[:page][vars[:limit_sym] || DEFAULT[:limit_sym]]
- end
- end
- # :nocov:
- Backend.prepend BackendOverride
- # :nocov:
-
- # Module overriding UrlHelper
- module UrlOverride
- # Override Url method
- def pagy_set_query_params(page, vars, query_params)
- return super unless vars[:jsonapi]
-
- query_params['page'] ||= {}
- query_params['page'][vars[:page_sym].to_s] = page
- query_params['page'][vars[:limit_sym].to_s] = vars[:limit] if vars[:limit_requestable]
- end
- end
- Url.prepend UrlOverride
- end
-end
diff --git a/gem/lib/pagy/helpers/url.rb b/gem/lib/pagy/helpers/url.rb
index 39183ccd9..258bf8cae 100644
--- a/gem/lib/pagy/helpers/url.rb
+++ b/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
+ pagy_params = (vars = pagy.vars)[:params]
+ page_name, limit_name = vars.values_at(:page_sym, :limit_sym).map(&:to_s)
+
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
+ 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 "#{vars[:url]}#{query_string}#{fragment}" if vars[: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
diff --git a/gem/lib/pagy/loaders/backend.rb b/gem/lib/pagy/loaders/backend.rb
index 7cf085d89..422d673f1 100644
--- a/gem/lib/pagy/loaders/backend.rb
+++ b/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
diff --git a/gem/lib/pagy/mixins/headers.rb b/gem/lib/pagy/mixins/headers.rb
index f104dc58d..19de95c2f 100644
--- a/gem/lib/pagy/mixins/headers.rb
+++ b/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)
diff --git a/gem/lib/pagy/helpers/links.rb b/gem/lib/pagy/mixins/links.rb
similarity index 91%
rename from gem/lib/pagy/helpers/links.rb
rename to gem/lib/pagy/mixins/links.rb
index 411118e51..4250a6f1b 100644
--- a/gem/lib/pagy/helpers/links.rb
+++ b/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|
diff --git a/test/pagy/extras/jsonapi_test.rb b/test/pagy/extras/jsonapi_test.rb
index 4f8546ccb..6f26079d1 100644
--- a/test/pagy/extras/jsonapi_test.rb
+++ b/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
diff --git a/test/pagy/extras/jsonapi_test.rb.yaml b/test/pagy/extras/jsonapi_test.rb.yaml
index c6f209bcf..b6f81e350 100644
--- a/test/pagy/extras/jsonapi_test.rb.yaml
+++ b/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"