From abb0a1c695af2839518b23ddc6689d0cecfeb486 Mon Sep 17 00:00:00 2001 From: Candy Goodison Date: Fri, 4 Aug 2023 16:34:18 +1000 Subject: [PATCH] feat: add pagination parameter validation for paginated endpoints. (#626) PACT-1101 --- lib/pact_broker/api/resources/branch_versions.rb | 12 ++++++++++++ lib/pact_broker/api/resources/dashboard.rb | 10 ++++++++++ lib/pact_broker/api/resources/pacticipants.rb | 16 ++++++++++++++-- lib/pact_broker/api/resources/versions.rb | 12 ++++++++++++ .../pact_broker/api/resources/dashboard_spec.rb | 6 ++++++ .../api/resources/pacticipants_spec.rb | 11 +++++++++++ spec/support/shared_examples_for_responses.rb | 13 +++++++++++++ 7 files changed, 78 insertions(+), 2 deletions(-) diff --git a/lib/pact_broker/api/resources/branch_versions.rb b/lib/pact_broker/api/resources/branch_versions.rb index 74ebef48f..6bb1b10ea 100644 --- a/lib/pact_broker/api/resources/branch_versions.rb +++ b/lib/pact_broker/api/resources/branch_versions.rb @@ -18,6 +18,10 @@ def allowed_methods ["GET", "OPTIONS"] end + def malformed_request? + super || request.get? && validation_errors_for_schema?(schema, request.query) + end + def resource_exists? !!branch end @@ -41,6 +45,14 @@ def branch def resource_title "Versions for branch #{branch.name} of #{branch.pacticipant.name}" end + + private + + def schema + if request.get? + PactBroker::Api::Contracts::PaginationQueryParamsSchema + end + end end end end diff --git a/lib/pact_broker/api/resources/dashboard.rb b/lib/pact_broker/api/resources/dashboard.rb index 1f9eee10e..c474d33ce 100644 --- a/lib/pact_broker/api/resources/dashboard.rb +++ b/lib/pact_broker/api/resources/dashboard.rb @@ -20,6 +20,10 @@ def allowed_methods ["GET", "OPTIONS"] end + def malformed_request? + super || (request.get? && validation_errors_for_schema?(schema, request.query)) + end + def to_json decorator_class(:dashboard_decorator).new(index_items).to_json(**decorator_options) end @@ -34,6 +38,12 @@ def policy_name private + def schema + if request.get? + PactBroker::Api::Contracts::PaginationQueryParamsSchema + end + end + def index_items index_service.find_index_items_for_api(**identifier_from_path.merge(pagination_options)) end diff --git a/lib/pact_broker/api/resources/pacticipants.rb b/lib/pact_broker/api/resources/pacticipants.rb index 05fcad0f2..92ee8126d 100644 --- a/lib/pact_broker/api/resources/pacticipants.rb +++ b/lib/pact_broker/api/resources/pacticipants.rb @@ -25,7 +25,15 @@ def allowed_methods end def malformed_request? - super || (request.post? && validation_errors_for_schema?) + if super + true + elsif request.post? && validation_errors_for_schema? + true + elsif request.get? && validation_errors_for_schema?(schema, request.query) + true + else + false + end end def request_body_required? @@ -68,7 +76,11 @@ def policy_name private def schema - PactBroker::Api::Contracts::PacticipantCreateSchema + if request.get? + PactBroker::Api::Contracts::PaginationQueryParamsSchema + else + PactBroker::Api::Contracts::PacticipantCreateSchema + end end def eager_load_associations diff --git a/lib/pact_broker/api/resources/versions.rb b/lib/pact_broker/api/resources/versions.rb index 4101103e1..e03b2d76d 100644 --- a/lib/pact_broker/api/resources/versions.rb +++ b/lib/pact_broker/api/resources/versions.rb @@ -18,6 +18,10 @@ def allowed_methods ["GET", "OPTIONS"] end + def malformed_request? + super || request.get? && validation_errors_for_schema?(schema, request.query) + end + def resource_exists? !!pacticipant end @@ -33,6 +37,14 @@ def versions def policy_name :'versions::versions' end + + private + + def schema + if request.get? + PactBroker::Api::Contracts::PaginationQueryParamsSchema + end + 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 56937e29c..5e669cd81 100644 --- a/spec/lib/pact_broker/api/resources/dashboard_spec.rb +++ b/spec/lib/pact_broker/api/resources/dashboard_spec.rb @@ -30,6 +30,12 @@ module Resources expect(response_body_hash["items"].size).to eq 1 end end + + context "with invalid pagination" do + subject { get(path, { pageNumber: -1, pageSize: -1 }) } + + it_behaves_like "an invalid pagination params response" + end end end end diff --git a/spec/lib/pact_broker/api/resources/pacticipants_spec.rb b/spec/lib/pact_broker/api/resources/pacticipants_spec.rb index be48bb37d..af646fb3d 100644 --- a/spec/lib/pact_broker/api/resources/pacticipants_spec.rb +++ b/spec/lib/pact_broker/api/resources/pacticipants_spec.rb @@ -40,6 +40,17 @@ module Resources and_return(pacticipants) expect(subject.status).to eq 200 end + + context "with invalid pagination params" do + let(:query) do + { + "pageSize" => "0", + "pageNumber" => "0", + } + end + + it_behaves_like "an invalid pagination params response" + end end describe "POST" do diff --git a/spec/support/shared_examples_for_responses.rb b/spec/support/shared_examples_for_responses.rb index 3c5538e20..6f8377bf3 100644 --- a/spec/support/shared_examples_for_responses.rb +++ b/spec/support/shared_examples_for_responses.rb @@ -28,6 +28,19 @@ end end +shared_examples_for "an invalid pagination params response" do + let(:response_body_hash) { JSON.parse(subject.body, symbolize_names: true) } + + it "returns a a 400 status code" do + expect(subject.status).to eq 400 + 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 + end +end + require "rspec/expectations" RSpec::Matchers.define :be_a_hal_json_success_response do