From f489ba7b88a579d80b85725931f02ec5c0e3bc0d Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Thu, 1 Oct 2020 17:26:32 +1000 Subject: [PATCH] feat: paginate pacticipant versions Closes: https://github.com/pact-foundation/pact_broker/issues/355 --- .../api/decorators/decorator_context.rb | 4 +-- .../api/decorators/pagination_links.rb | 34 +++++++++++++++++++ .../api/decorators/versions_decorator.rb | 6 +++- lib/pact_broker/api/pact_broker_urls.rb | 8 +++++ lib/pact_broker/api/resources/versions.rb | 13 ++++++- .../doc/views/pacticipant/versions.markdown | 9 +++++ lib/pact_broker/pacticipants/repository.rb | 11 +++--- lib/pact_broker/pacticipants/service.rb | 4 +-- spec/features/get_versions_spec.rb | 8 +++++ .../api/decorators/versions_decorator_spec.rb | 23 ++++++++----- .../pacticipants/repository_spec.rb | 9 ++++- 11 files changed, 108 insertions(+), 21 deletions(-) create mode 100644 lib/pact_broker/api/decorators/pagination_links.rb create mode 100644 lib/pact_broker/doc/views/pacticipant/versions.markdown diff --git a/lib/pact_broker/api/decorators/decorator_context.rb b/lib/pact_broker/api/decorators/decorator_context.rb index 19c3179b7..d86566423 100644 --- a/lib/pact_broker/api/decorators/decorator_context.rb +++ b/lib/pact_broker/api/decorators/decorator_context.rb @@ -2,14 +2,14 @@ module PactBroker module Api module Decorators class DecoratorContext < Hash - - attr_reader :base_url, :resource_url, :resource_title, :env + attr_reader :base_url, :resource_url, :resource_title, :env, :query_string def initialize base_url, resource_url, env, options = {} @base_url = self[:base_url] = base_url @resource_url = self[:resource_url] = resource_url @resource_title = self[:resource_title] = options[:resource_title] @env = self[:env] = env + @query_string = self[:query_string] = (env['QUERY_STRING'] && !env['QUERY_STRING'].empty? ? env['QUERY_STRING'] : nil) merge!(options) end diff --git a/lib/pact_broker/api/decorators/pagination_links.rb b/lib/pact_broker/api/decorators/pagination_links.rb new file mode 100644 index 000000000..16a8e0b68 --- /dev/null +++ b/lib/pact_broker/api/decorators/pagination_links.rb @@ -0,0 +1,34 @@ +require 'roar/decorator' +require 'roar/json/hal' + +module PactBroker + module Api + module Decorators + module PaginationLinks + include Roar::JSON::HAL + include Roar::JSON::HAL::Links + + link :next do | context | + if represented.respond_to?(:current_page) && + represented.respond_to?(:page_count) && + represented.current_page < represented.page_count + { + href: context[:resource_url] + "?pageSize=#{represented.page_size}&pageNumber=#{represented.current_page + 1}", + title: "Next page" + } + + end + end + + 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}", + title: "Previous page" + } + end + end + end + end + end +end diff --git a/lib/pact_broker/api/decorators/versions_decorator.rb b/lib/pact_broker/api/decorators/versions_decorator.rb index b074992e5..13149d88c 100644 --- a/lib/pact_broker/api/decorators/versions_decorator.rb +++ b/lib/pact_broker/api/decorators/versions_decorator.rb @@ -1,5 +1,6 @@ require_relative 'base_decorator' require_relative 'version_decorator' +require_relative 'pagination_links' module PactBroker module Api @@ -9,12 +10,15 @@ class VersionsDecorator < BaseDecorator collection :entries, as: :versions, embedded: true, :extend => PactBroker::Api::Decorators::VersionDecorator link :self do | context | + href = append_query_if_present(context[:resource_url], context[:query_string]) { - href: context[:resource_url], + href: href, title: "All application versions of #{context[:pacticipant_name]}" } end + include PaginationLinks + link :'pb:pacticipant' do | context | { href: pacticipant_url(context[:base_url], OpenStruct.new(name: context[:pacticipant_name])), diff --git a/lib/pact_broker/api/pact_broker_urls.rb b/lib/pact_broker/api/pact_broker_urls.rb index dccb837e7..5df64fad2 100644 --- a/lib/pact_broker/api/pact_broker_urls.rb +++ b/lib/pact_broker/api/pact_broker_urls.rb @@ -307,6 +307,14 @@ def url_encode param ERB::Util.url_encode param end + def append_query_if_present(url, query) + if query && query.size > 0 + url + "?#{query}" + else + url + end + end + private def representable_pact pact diff --git a/lib/pact_broker/api/resources/versions.rb b/lib/pact_broker/api/resources/versions.rb index 6736593b8..905851eee 100644 --- a/lib/pact_broker/api/resources/versions.rb +++ b/lib/pact_broker/api/resources/versions.rb @@ -23,12 +23,23 @@ def to_json end def versions - @versions ||= pacticipant_service.find_all_pacticipant_versions_in_reverse_order pacticipant_name + @versions ||= pacticipant_service.find_all_pacticipant_versions_in_reverse_order(pacticipant_name, pagination_options) end def policy_name :'versions::versions' end + + def pagination_options + if request.query['pageNumber'] || request.query['pageSize'] + { + page_number: request.query['pageNumber']&.to_i || 1, + page_size: request.query['pageSize']&.to_i || 100 + } + else + nil + end + end end end end diff --git a/lib/pact_broker/doc/views/pacticipant/versions.markdown b/lib/pact_broker/doc/views/pacticipant/versions.markdown new file mode 100644 index 000000000..3187fc5e3 --- /dev/null +++ b/lib/pact_broker/doc/views/pacticipant/versions.markdown @@ -0,0 +1,9 @@ +# Pacticipant versions + +Allowed methods: `GET` + +Path: `/pacticipants/{pacticipant}/versions` + +A list of pacticipant versions in order from newest to oldest. + +To paginate, append `?pageNumber=x&pageSize=x` and follow the `next` relation until it is no longer present. diff --git a/lib/pact_broker/pacticipants/repository.rb b/lib/pact_broker/pacticipants/repository.rb index cda771838..f38d3076f 100644 --- a/lib/pact_broker/pacticipants/repository.rb +++ b/lib/pact_broker/pacticipants/repository.rb @@ -8,6 +8,7 @@ module Pacticipants class Repository include PactBroker::Repositories::Helpers + include PactBroker::Repositories def find_by_name name pacticipants = PactBroker::Domain::Pacticipant.where(name_like(:name, name)).all @@ -35,11 +36,11 @@ def find options = {} query.order_ignore_case(Sequel[:pacticipants][:name]).eager(:labels).eager(:latest_version).all end - def find_all_pacticipant_versions_in_reverse_order name - PactBroker::Domain::Version.select_all_qualified - .join(:pacticipants, {id: :pacticipant_id}) - .where(name_like(:name, name)) - .reverse_order(:order) + def find_all_pacticipant_versions_in_reverse_order name, pagination_options = nil + pacticipant = pacticipant_repository.find_by_name!(name) + query = PactBroker::Domain::Version.where(pacticipant: pacticipant).reverse_order(:order) + query = query.paginate(pagination_options[:page_number], pagination_options[:page_size]) if pagination_options + query end def find_by_name_or_create name diff --git a/lib/pact_broker/pacticipants/service.rb b/lib/pact_broker/pacticipants/service.rb index 76438cb6d..f070a5b9b 100644 --- a/lib/pact_broker/pacticipants/service.rb +++ b/lib/pact_broker/pacticipants/service.rb @@ -52,8 +52,8 @@ def self.find options pacticipant_repository.find options end - def self.find_all_pacticipant_versions_in_reverse_order name - pacticipant_repository.find_all_pacticipant_versions_in_reverse_order(name) + def self.find_all_pacticipant_versions_in_reverse_order name, pagination_options = nil + pacticipant_repository.find_all_pacticipant_versions_in_reverse_order(name, pagination_options) end def self.find_pacticipant_repository_url_by_pacticipant_name name diff --git a/spec/features/get_versions_spec.rb b/spec/features/get_versions_spec.rb index f9dd6b6f7..f2e8c637a 100644 --- a/spec/features/get_versions_spec.rb +++ b/spec/features/get_versions_spec.rb @@ -21,6 +21,14 @@ it "returns a list of links to the versions" do expect(last_response_body[:_links][:"versions"].size).to eq 2 end + + context "with pagination options" do + subject { get(path, { 'pageSize' => '1', 'pageNumber' => '1' }) } + + it "paginates the response" do + expect(last_response_body[:_links][:"versions"].size).to eq 1 + end + end end context "when the pacticipant does not exist" do diff --git a/spec/lib/pact_broker/api/decorators/versions_decorator_spec.rb b/spec/lib/pact_broker/api/decorators/versions_decorator_spec.rb index 0191738ff..a58a9bd85 100644 --- a/spec/lib/pact_broker/api/decorators/versions_decorator_spec.rb +++ b/spec/lib/pact_broker/api/decorators/versions_decorator_spec.rb @@ -3,32 +3,38 @@ require 'pact_broker/domain/version' module PactBroker - module Api - module Decorators - describe VersionsDecorator do - let(:options) { {base_url: 'http://example.org', pacticipant_name: "Consumer" }} + let(:options) { { resource_url: 'http://versions', base_url: 'http://example.org', pacticipant_name: "Consumer", query_string: query_string}} + let(:query_string) { nil } + let(:versions) { [] } subject { JSON.parse VersionsDecorator.new(versions).to_json(user_options: options), symbolize_names: true } - context "with no versions" do - let(:versions) { [] } + context "with no query string" do + its([:_links, :self, :href]) { is_expected.to eq 'http://versions' } + end + context "with a query string" do + let(:query_string) { 'foo=bar' } + its([:_links, :self, :href]) { is_expected.to eq 'http://versions?foo=bar' } + end + + context "with no versions" do it "doesn't blow up" do subject end end context "with versions" do - let(:version) do + let!(:version) do TestDataBuilder.new .create_consumer("Consumer") .create_consumer_version("1.2.3") .create_consumer_version_tag("prod") - PactBroker::Versions::Repository.new.find_by_pacticipant_name_and_number "Consumer", "1.2.3" + .and_return(:consumer_version) end let(:versions) { [version] } @@ -37,7 +43,6 @@ module Decorators expect(subject[:_embedded][:versions].size).to eq 1 end end - end end end diff --git a/spec/lib/pact_broker/pacticipants/repository_spec.rb b/spec/lib/pact_broker/pacticipants/repository_spec.rb index f6eed0150..0e1845a6b 100644 --- a/spec/lib/pact_broker/pacticipants/repository_spec.rb +++ b/spec/lib/pact_broker/pacticipants/repository_spec.rb @@ -154,8 +154,15 @@ module Pacticipants it "returns all the application versions for the given consumer" do expect(subject.collect(&:number)).to eq ["4.5.6", "1.2.3"] end - end + context "with pagination options" do + subject { Repository.new.find_all_pacticipant_versions_in_reverse_order "Foo", page_number: 1, page_size: 1 } + + it "paginates the query" do + expect(subject.collect(&:number)).to eq ["4.5.6"] + end + end + end end end end