From c71089fe9902a35a510c44417c0f673b1a71b925 Mon Sep 17 00:00:00 2001 From: Voon Siong Wong Date: Fri, 10 Nov 2023 11:06:38 +1100 Subject: [PATCH] feat: suppport `page` + `size` as pagination params (#642) This is required for compatibility with SmartBear pagination standards. PACT-1460 --- .../pagination_query_params_schema.rb | 5 +++ .../api/decorators/branch_decorator.rb | 2 +- .../api/decorators/pagination_links.rb | 4 +- .../api/resources/pagination_methods.rb | 8 +++- spec/features/get_branch_versions_spec.rb | 4 +- spec/features/get_integrations_spec.rb | 2 +- .../features/get_pacticipant_branches_spec.rb | 4 +- spec/features/get_pacticipants_spec.rb | 4 +- spec/features/get_versions_spec.rb | 2 +- ...cticipant_branches_decorator.approved.json | 2 +- .../pagination_query_params_schema_spec.rb | 42 +++++++++++++++---- .../api/resources/dashboard_spec.rb | 4 +- .../api/resources/integrations_spec.rb | 2 +- .../api/resources/pacticipants_spec.rb | 8 ++-- spec/support/shared_examples_for_responses.rb | 4 +- 15 files changed, 67 insertions(+), 30 deletions(-) diff --git a/lib/pact_broker/api/contracts/pagination_query_params_schema.rb b/lib/pact_broker/api/contracts/pagination_query_params_schema.rb index bf2761b3d..7f3e69a48 100644 --- a/lib/pact_broker/api/contracts/pagination_query_params_schema.rb +++ b/lib/pact_broker/api/contracts/pagination_query_params_schema.rb @@ -5,8 +5,13 @@ module Api module Contracts class PaginationQueryParamsSchema < BaseContract params do + # legacy format optional(:pageNumber).maybe(:integer).value(gteq?: 1) optional(:pageSize).maybe(:integer).value(gteq?: 1) + + # desired format + optional(:page).maybe(:integer).value(gteq?: 1) + optional(:size).maybe(:integer).value(gteq?: 1) end end end diff --git a/lib/pact_broker/api/decorators/branch_decorator.rb b/lib/pact_broker/api/decorators/branch_decorator.rb index 915a12975..a53f4d2db 100644 --- a/lib/pact_broker/api/decorators/branch_decorator.rb +++ b/lib/pact_broker/api/decorators/branch_decorator.rb @@ -18,7 +18,7 @@ class BranchDecorator < BaseDecorator link "pb:latest-version" do | user_options | { title: "Latest version for branch", - href: branch_versions_url(represented, user_options.fetch(:base_url)) + "?pageSize=1" + href: branch_versions_url(represented, user_options.fetch(:base_url)) + "?size=1" } end diff --git a/lib/pact_broker/api/decorators/pagination_links.rb b/lib/pact_broker/api/decorators/pagination_links.rb index eba47eeae..39b00091a 100644 --- a/lib/pact_broker/api/decorators/pagination_links.rb +++ b/lib/pact_broker/api/decorators/pagination_links.rb @@ -24,7 +24,7 @@ module PaginationLinks represented.respond_to?(:page_count) && represented.current_page < represented.page_count { - href: context[:resource_url] + "?pageSize=#{represented.page_size}&pageNumber=#{represented.current_page + 1}", + href: context[:resource_url] + "?size=#{represented.page_size}&page=#{represented.current_page + 1}", title: "Next page" } @@ -34,7 +34,7 @@ module PaginationLinks link :previous do | context | if represented.respond_to?(:first_page?) && !represented.first_page? { - href: context[:resource_url] + "?pageSize=#{represented.page_size}&pageNumber=#{represented.current_page - 1}", + href: context[:resource_url] + "?size=#{represented.page_size}&page=#{represented.current_page - 1}", title: "Previous page" } end diff --git a/lib/pact_broker/api/resources/pagination_methods.rb b/lib/pact_broker/api/resources/pagination_methods.rb index 069f71853..a8c2f4a9f 100644 --- a/lib/pact_broker/api/resources/pagination_methods.rb +++ b/lib/pact_broker/api/resources/pagination_methods.rb @@ -2,8 +2,14 @@ module PactBroker module Api module Resources module PaginationMethods + # rubocop: disable Metrics/CyclomaticComplexity def pagination_options - if request.query["pageNumber"] || request.query["pageSize"] + if request.query["page"] || request.query["size"] + { + page_number: request.query["page"]&.to_i || 1, + page_size: request.query["size"]&.to_i || 100 + } + elsif request.query["pageNumber"] || request.query["pageSize"] { page_number: request.query["pageNumber"]&.to_i || 1, page_size: request.query["pageSize"]&.to_i || 100 diff --git a/spec/features/get_branch_versions_spec.rb b/spec/features/get_branch_versions_spec.rb index 31774d407..9884cab86 100644 --- a/spec/features/get_branch_versions_spec.rb +++ b/spec/features/get_branch_versions_spec.rb @@ -27,9 +27,9 @@ end context "with pagination options" do - subject { get(path, { "pageSize" => "2", "pageNumber" => "1" }) } + subject { get(path, { "size" => "2", "page" => "1" }) } - it "only returns the number of items specified in the pageSize" do + it "only returns the number of items specified in the size" do expect(JSON.parse(subject.body).dig("_embedded", "versions").size).to eq 2 end diff --git a/spec/features/get_integrations_spec.rb b/spec/features/get_integrations_spec.rb index 04b01482c..c2389301f 100644 --- a/spec/features/get_integrations_spec.rb +++ b/spec/features/get_integrations_spec.rb @@ -23,7 +23,7 @@ end context "with pagination options" do - let(:query) { { "pageSize" => "2", "pageNumber" => "1" } } + let(:query) { { "size" => "2", "page" => "1" } } it_behaves_like "a paginated response" end diff --git a/spec/features/get_pacticipant_branches_spec.rb b/spec/features/get_pacticipant_branches_spec.rb index 4fb6b4b2b..56fcf41e6 100644 --- a/spec/features/get_pacticipant_branches_spec.rb +++ b/spec/features/get_pacticipant_branches_spec.rb @@ -21,9 +21,9 @@ it_behaves_like "a page" context "with pagination options" do - subject { get(path, { "pageSize" => "2", "pageNumber" => "1" }) } + subject { get(path, { "size" => "2", "number" => "1" }) } - it "only returns the number of items specified in the pageSize" do + it "only returns the number of items specified in the size" do expect(response_body_hash[:_links][:"pb:branches"].size).to eq 2 end diff --git a/spec/features/get_pacticipants_spec.rb b/spec/features/get_pacticipants_spec.rb index 97232bc97..4f10667ca 100644 --- a/spec/features/get_pacticipants_spec.rb +++ b/spec/features/get_pacticipants_spec.rb @@ -23,9 +23,9 @@ end context "with pagination options" do - subject { get(path, { "pageSize" => "2", "pageNumber" => "1" }) } + subject { get(path, { "size" => "2", "page" => "1" }) } - it "only returns the number of items specified in the pageSize" do + it "only returns the number of items specified in the page" do expect(response_body_hash[:_links][:"pacticipants"].size).to eq 2 end diff --git a/spec/features/get_versions_spec.rb b/spec/features/get_versions_spec.rb index f0a6ff1c2..e2c24fb8d 100644 --- a/spec/features/get_versions_spec.rb +++ b/spec/features/get_versions_spec.rb @@ -26,7 +26,7 @@ end context "with pagination options" do - subject { get(path, { "pageSize" => "1", "pageNumber" => "1" }) } + subject { get(path, { "size" => "1", "page" => "1" }) } it "paginates the response" do expect(last_response_body[:_links][:"versions"].size).to eq 1 diff --git a/spec/fixtures/approvals/pacticipant_branches_decorator.approved.json b/spec/fixtures/approvals/pacticipant_branches_decorator.approved.json index def84d52f..6f719defe 100644 --- a/spec/fixtures/approvals/pacticipant_branches_decorator.approved.json +++ b/spec/fixtures/approvals/pacticipant_branches_decorator.approved.json @@ -11,7 +11,7 @@ }, "pb:latest-version": { "title": "Latest version for branch", - "href": "http://example.org/pacticipants/Foo/branches/main/versions?pageSize=1" + "href": "http://example.org/pacticipants/Foo/branches/main/versions?size=1" } } } diff --git a/spec/lib/pact_broker/api/contracts/pagination_query_params_schema_spec.rb b/spec/lib/pact_broker/api/contracts/pagination_query_params_schema_spec.rb index 7e7afc6ae..1277f5352 100644 --- a/spec/lib/pact_broker/api/contracts/pagination_query_params_schema_spec.rb +++ b/spec/lib/pact_broker/api/contracts/pagination_query_params_schema_spec.rb @@ -19,25 +19,51 @@ module Contracts context "with values that are not numeric" do let(:params) do { - "pageNumber" => "a", - "pageSize" => "3.2" + "page" => "a", + "size" => "3.2" } end - its([:pageNumber]) { is_expected.to contain_exactly(match("integer"))} - its([:pageSize]) { is_expected.to contain_exactly(match("integer"))} + its([:page]) { is_expected.to contain_exactly(match("integer"))} + its([:size]) { is_expected.to contain_exactly(match("integer"))} end context "with values that are 0" do let(:params) do { - "pageNumber" => "-0", - "pageSize" => "-0" + "page" => "-0", + "size" => "-0" } end - its([:pageNumber]) { is_expected.to contain_exactly(match(/greater.*1/))} - its([:pageSize]) { is_expected.to contain_exactly(match(/greater.*1/))} + its([:page]) { is_expected.to contain_exactly(match(/greater.*1/))} + its([:size]) { is_expected.to contain_exactly(match(/greater.*1/))} + end + + context "legacy format" do + context "with values that are not numeric" do + let(:params) do + { + "pageNumber" => "a", + "pageSize" => "3.2" + } + end + + its([:pageNumber]) { is_expected.to contain_exactly(match("integer"))} + its([:pageSize]) { is_expected.to contain_exactly(match("integer"))} + end + + context "with values that are 0" do + let(:params) do + { + "pageNumber" => "-0", + "pageSize" => "-0" + } + end + + its([:pageNumber]) { is_expected.to contain_exactly(match(/greater.*1/))} + its([:pageSize]) { is_expected.to contain_exactly(match(/greater.*1/))} + end end end end diff --git a/spec/lib/pact_broker/api/resources/dashboard_spec.rb b/spec/lib/pact_broker/api/resources/dashboard_spec.rb index 5e669cd81..cb50ac5b7 100644 --- a/spec/lib/pact_broker/api/resources/dashboard_spec.rb +++ b/spec/lib/pact_broker/api/resources/dashboard_spec.rb @@ -24,7 +24,7 @@ module Resources end context "with pagination" do - subject { get(path, { pageNumber: 1, pageSize: 1 }) } + subject { get(path, { page: 1, size: 1 }) } it "only returns the items for the page" do expect(response_body_hash["items"].size).to eq 1 @@ -32,7 +32,7 @@ module Resources end context "with invalid pagination" do - subject { get(path, { pageNumber: -1, pageSize: -1 }) } + subject { get(path, { page: -1, size: -1 }) } it_behaves_like "an invalid pagination params response" end diff --git a/spec/lib/pact_broker/api/resources/integrations_spec.rb b/spec/lib/pact_broker/api/resources/integrations_spec.rb index 3aacb1be5..30bf76e20 100644 --- a/spec/lib/pact_broker/api/resources/integrations_spec.rb +++ b/spec/lib/pact_broker/api/resources/integrations_spec.rb @@ -23,7 +23,7 @@ module Resources let(:errors) { {} } let(:path) { "/integrations" } - let(:params) { { "pageNumber" => "1", "pageSize" => "2" } } + let(:params) { { "page" => "1", "size" => "2" } } subject { get(path, params, rack_headers) } diff --git a/spec/lib/pact_broker/api/resources/pacticipants_spec.rb b/spec/lib/pact_broker/api/resources/pacticipants_spec.rb index 6120aacb8..14a30a6a6 100644 --- a/spec/lib/pact_broker/api/resources/pacticipants_spec.rb +++ b/spec/lib/pact_broker/api/resources/pacticipants_spec.rb @@ -7,8 +7,8 @@ module Resources describe "GET" do let(:query) do { - "pageSize" => "10", - "pageNumber" => "1", + "size" => "10", + "page" => "1", "q" => "search" } end @@ -44,8 +44,8 @@ module Resources context "with invalid pagination params" do let(:query) do { - "pageSize" => "0", - "pageNumber" => "0", + "size" => "0", + "page" => "0", } end diff --git a/spec/support/shared_examples_for_responses.rb b/spec/support/shared_examples_for_responses.rb index 7e7a713a5..02885ee93 100644 --- a/spec/support/shared_examples_for_responses.rb +++ b/spec/support/shared_examples_for_responses.rb @@ -45,8 +45,8 @@ end it "includes the parameter validation errors" do - expect(response_body_hash[:errors].has_key?(:pageNumber)).to be_truthy - expect(response_body_hash[:errors].has_key?(:pageSize)).to be_truthy + expect(response_body_hash[:errors].has_key?(:page)).to be_truthy + expect(response_body_hash[:errors].has_key?(:size)).to be_truthy end end