diff --git a/lib/pact_broker/api/decorators/integration_decorator.rb b/lib/pact_broker/api/decorators/integration_decorator.rb index ec31fa827..7510ba1c2 100644 --- a/lib/pact_broker/api/decorators/integration_decorator.rb +++ b/lib/pact_broker/api/decorators/integration_decorator.rb @@ -40,3 +40,4 @@ class IntegrationDecorator < BaseDecorator end end end + diff --git a/lib/pact_broker/api/decorators/integrations_decorator.rb b/lib/pact_broker/api/decorators/integrations_decorator.rb index cbcdb3f8b..9b3a87021 100644 --- a/lib/pact_broker/api/decorators/integrations_decorator.rb +++ b/lib/pact_broker/api/decorators/integrations_decorator.rb @@ -1,5 +1,6 @@ require_relative "base_decorator" require_relative "integration_decorator" +require "pact_broker/api/decorators/pagination_links" module PactBroker module Api @@ -13,6 +14,8 @@ class IntegrationsDecorator < BaseDecorator title: "All integrations" } end + + include PactBroker::Api::Decorators::PaginationLinks end end end diff --git a/lib/pact_broker/api/resources/filter_methods.rb b/lib/pact_broker/api/resources/filter_methods.rb new file mode 100644 index 000000000..3461c6daf --- /dev/null +++ b/lib/pact_broker/api/resources/filter_methods.rb @@ -0,0 +1,15 @@ +module PactBroker + module Api + module Resources + module FilterMethods + def filter_options + if request.query.has_key?("q") + { query_string: request.query["q"] } + else + {} + end + end + end + end + end +end diff --git a/lib/pact_broker/api/resources/integrations.rb b/lib/pact_broker/api/resources/integrations.rb index 4f83e8bb3..7f1d12a22 100644 --- a/lib/pact_broker/api/resources/integrations.rb +++ b/lib/pact_broker/api/resources/integrations.rb @@ -1,11 +1,17 @@ require "pact_broker/api/resources/base_resource" require "pact_broker/api/renderers/integrations_dot_renderer" require "pact_broker/api/decorators/integrations_decorator" +require "pact_broker/api/resources/filter_methods" +require "pact_broker/api/resources/pagination_methods" +require "pact_broker/api/contracts/pagination_query_params_schema" module PactBroker module Api module Resources class Integrations < BaseResource + include PaginationMethods + include FilterMethods + def content_types_provided [ ["text/vnd.graphviz", :to_dot], @@ -17,18 +23,20 @@ def allowed_methods ["GET", "OPTIONS", "DELETE"] end + def malformed_request? + super || (request.get? && validation_errors_for_schema?(schema, request.query)) + end + def to_dot + integrations = integration_service.find_all(filter_options, pagination_options) PactBroker::Api::Renderers::IntegrationsDotRenderer.call(integrations) end def to_json + integrations = integration_service.find_all(filter_options, pagination_options, decorator_class(:integrations_decorator).eager_load_associations) decorator_class(:integrations_decorator).new(integrations).to_json(**decorator_options) end - def integrations - @integrations ||= integration_service.find_all - end - def delete_resource integration_service.delete_all true @@ -37,6 +45,12 @@ def delete_resource def policy_name :'integrations::integrations' end + + def schema + if request.get? + PactBroker::Api::Contracts::PaginationQueryParamsSchema + end + end end end end diff --git a/lib/pact_broker/dataset.rb b/lib/pact_broker/dataset.rb index fd461bf93..97ad6ea22 100644 --- a/lib/pact_broker/dataset.rb +++ b/lib/pact_broker/dataset.rb @@ -5,6 +5,14 @@ module PactBroker module Dataset + + # Return a dataset that only includes the rows where the specified column + # includes the given query string. + # @return [Sequel::Dataset] + def filter(column_name, query_string) + where(Sequel.ilike(column_name, "%" + escape_wildcards(query_string) + "%")) + end + def name_like column_name, value if PactBroker.configuration.use_case_sensitive_resource_names if mysql? @@ -80,5 +88,10 @@ def mysql? def postgres? Sequel::Model.db.adapter_scheme.to_s =~ /postgres/ end + + def escape_wildcards(value) + value.gsub("_", "\\_").gsub("%", "\\%") + end + private :escape_wildcards end end diff --git a/lib/pact_broker/integrations/integration.rb b/lib/pact_broker/integrations/integration.rb index 6f95c0b6a..78c6750ab 100644 --- a/lib/pact_broker/integrations/integration.rb +++ b/lib/pact_broker/integrations/integration.rb @@ -78,6 +78,12 @@ class Integration < Sequel::Model(Sequel::Model.db[:integrations].select(:id, :c dataset_module do include PactBroker::Dataset + def filter_by_pacticipant(query_string) + matching_pacticipants = PactBroker::Domain::Pacticipant.filter(:name, query_string) + pacticipants_join = Sequel.|({ Sequel[:integrations][:consumer_id] => Sequel[:p][:id] }, { Sequel[:integrations][:provider_id] => Sequel[:p][:id] }) + join(matching_pacticipants, pacticipants_join, table_alias: :p) + end + def including_pacticipant_id(pacticipant_id) where(consumer_id: pacticipant_id).or(provider_id: pacticipant_id) end @@ -123,6 +129,10 @@ def provider_name def pacticipant_ids [consumer_id, provider_id] end + + def to_s + "Integration: consumer #{associations[:consumer]&.name || consumer_id}/provider #{associations[:provider]&.name || provider_id}" + end end end end diff --git a/lib/pact_broker/integrations/repository.rb b/lib/pact_broker/integrations/repository.rb index ecac6853b..e790a32da 100644 --- a/lib/pact_broker/integrations/repository.rb +++ b/lib/pact_broker/integrations/repository.rb @@ -1,8 +1,21 @@ require "pact_broker/integrations/integration" +require "pact_broker/repositories/scopes" module PactBroker module Integrations class Repository + + include PactBroker::Repositories::Scopes + + def find(filter_options = {}, pagination_options = {}, eager_load_associations = []) + query = scope_for(PactBroker::Integrations::Integration).select_all_qualified + query = query.filter_by_pacticipant(filter_options[:query_string]) if filter_options[:query_string] + query + .eager(*eager_load_associations) + .order(Sequel.desc(:contract_data_updated_at, nulls: :last)) + .all_with_pagination_options(pagination_options) + end + def create_for_pact(consumer_id, provider_id) if Integration.where(consumer_id: consumer_id, provider_id: provider_id).empty? Integration.new( diff --git a/lib/pact_broker/integrations/service.rb b/lib/pact_broker/integrations/service.rb index 3e172c9af..e82539163 100644 --- a/lib/pact_broker/integrations/service.rb +++ b/lib/pact_broker/integrations/service.rb @@ -14,22 +14,8 @@ class Service include PactBroker::Logging extend PactBroker::Repositories::Scopes - def self.find_all - # The only reason the pact_version needs to be loaded is that - # the Verification::PseudoBranchStatus uses it to determine if - # the pseudo branch is 'stale'. - # Because this is the status for a pact, and not a pseudo branch, - # the status can never be 'stale', - # so it would be better to create a Verification::PactStatus class - # that doesn't have the 'stale' logic in it. - # Then we can remove the eager loading of the pact_version - scope_for(PactBroker::Integrations::Integration) - .eager(:consumer) - .eager(:provider) - .eager(:latest_pact) # latest_pact eager loader is custom, can't take any more options - .eager(:latest_verification) - .all - .sort { | a, b| Integration.compare_by_last_action_date(a, b) } + def self.find_all(filter_options = {}, pagination_options = {}, eager_load_associations = []) + integration_repository.find(filter_options, pagination_options, eager_load_associations) end # Callback to invoke when a consumer contract, verification result (or provider contract in Pactflow) is published diff --git a/lib/pact_broker/pacticipants/repository.rb b/lib/pact_broker/pacticipants/repository.rb index 71907a293..514561b7e 100644 --- a/lib/pact_broker/pacticipants/repository.rb +++ b/lib/pact_broker/pacticipants/repository.rb @@ -40,7 +40,7 @@ def find_all(options = {}, pagination_options = {}, eager_load_associations = [] def find(options = {}, pagination_options = {}, eager_load_associations = []) query = scope_for(PactBroker::Domain::Pacticipant).select_all_qualified - query = query.where(Sequel.ilike(:name, "%#{options[:query_string].gsub("_", "\\_")}%")) if options[:query_string] + query = query.filter(:name, options[:query_string]) if options[:query_string] query = query.label(options[:label_name]) if options[:label_name] query.order_ignore_case(Sequel[:pacticipants][:name]).eager(*eager_load_associations).all_with_pagination_options(pagination_options) end diff --git a/spec/features/get_integrations_spec.rb b/spec/features/get_integrations_spec.rb index b15b99b82..04b01482c 100644 --- a/spec/features/get_integrations_spec.rb +++ b/spec/features/get_integrations_spec.rb @@ -1,17 +1,47 @@ -describe "Get integrations dot file" do +describe "Get integrations" do before do - td.create_pact_with_hierarchy("Foo", "1", "Bar") - .create_verification(provider_version: "2") + td.create_consumer("Foo") + .create_provider("Bar") + .create_integration + .create_consumer("Apple") + .create_provider("Pear") + .create_integration + .create_consumer("Dog") + .create_provider("Cat") + .create_integration end let(:path) { "/integrations" } - let(:response_body_hash) { JSON.parse(subject.body, symbolize_names: true) } - - subject { get path, nil, {"HTTP_ACCEPT" => "application/hal+json" } } + let(:query) { nil } + let(:response_body_hash) { JSON.parse(subject.body) } + subject { get path, query, {"HTTP_ACCEPT" => "application/hal+json" } } it { is_expected.to be_a_hal_json_success_response } it "returns a json body with embedded integrations" do - expect(JSON.parse(subject.body)["_embedded"]["integrations"]).to be_a(Array) + expect(response_body_hash["_embedded"]["integrations"]).to be_a(Array) + end + + context "with pagination options" do + let(:query) { { "pageSize" => "2", "pageNumber" => "1" } } + + it_behaves_like "a paginated response" + end + + context "with a query string" do + let(:query) { { "q" => "pp" } } + + it "returns only the integrations with a consumer or provider name including the given string" do + expect(response_body_hash["_embedded"]["integrations"]).to contain_exactly(hash_including("consumer" => hash_including("name" => "Apple"))) + end + end + + context "as a dot file" do + subject { get path, query, {"HTTP_ACCEPT" => "text/vnd.graphviz" } } + + it "returns a dot file" do + expect(subject.body).to include "digraph" + expect(subject.body).to include "Foo -> Bar" + end end end diff --git a/spec/lib/pact_broker/api/resources/integrations_spec.rb b/spec/lib/pact_broker/api/resources/integrations_spec.rb new file mode 100644 index 000000000..4cb8939f3 --- /dev/null +++ b/spec/lib/pact_broker/api/resources/integrations_spec.rb @@ -0,0 +1,58 @@ +require "pact_broker/api/resources/integrations" + +module PactBroker + module Api + module Resources + describe Integrations do + describe "GET" do + before do + allow_any_instance_of(described_class).to receive(:integration_service).and_return(integration_service) + allow(integration_service).to receive(:find_all).and_return(integrations) + allow_any_instance_of(described_class).to receive(:decorator_class).and_return(decorator_class) + allow_any_instance_of(described_class).to receive_message_chain(:decorator_class, :eager_load_associations).and_return(eager_load_associations) + allow(PactBroker::Api::Contracts::PaginationQueryParamsSchema).to receive(:call).and_return(errors) + end + + let(:integration_service) { class_double("PactBroker::Integrations::Service").as_stubbed_const } + let(:integrations) { double("integrations") } + let(:decorator_class) { double("decorator class", new: decorator) } + let(:decorator) { double("decorator", to_json: json) } + let(:json) { "some json" } + let(:rack_headers) { { "HTTP_ACCEPT" => "application/hal+json" } } + let(:eager_load_associations) { [:foo, :bar] } + let(:errors) { {} } + + let(:path) { "/integrations" } + let(:params) { { "pageNumber" => "1", "pageSize" => "2" } } + + subject { get(path, params, rack_headers) } + + it "validates the query params" do + expect(PactBroker::Api::Contracts::PaginationQueryParamsSchema).to receive(:call).with(params) + subject + end + + it "finds the integrations" do + allow(integration_service).to receive(:find_all).with({}, { page_number: 1, page_size: 2 }, eager_load_associations) + subject + end + + its(:status) { is_expected.to eq 200 } + + it "renders the integrations" do + expect(decorator_class).to receive(:new).with(integrations) + expect(decorator).to receive(:to_json).with(user_options: instance_of(Decorators::DecoratorContext)) + expect(subject.body).to eq json + end + + context "with invalid query params" do + let(:errors) { { "some" => ["errors"]} } + + its(:status) { is_expected.to eq 400 } + its(:body) { is_expected.to match "some.*errors" } + end + end + end + end + end +end diff --git a/spec/lib/pact_broker/integrations/integration_spec.rb b/spec/lib/pact_broker/integrations/integration_spec.rb index 030ae2428..c5ec2d1cb 100644 --- a/spec/lib/pact_broker/integrations/integration_spec.rb +++ b/spec/lib/pact_broker/integrations/integration_spec.rb @@ -3,6 +3,45 @@ module PactBroker module Integrations describe Integration do + describe "filter" do + before do + td.create_consumer("Foo") + .create_provider("Bar") + .create_integration + .create_consumer("Cat") + .create_provider("Dog") + .create_integration + .create_consumer("Y") + .create_provider("Z") + .create_integration + end + + subject { Integration.select_all_qualified.filter_by_pacticipant(query_string).all } + + context "with a filter matching the consumer" do + let(:query_string) { "oo" } + + it { is_expected.to contain_exactly(have_attributes(consumer_name: "Foo", provider_name: "Bar")) } + end + + context "with a filter matching the provider" do + let(:query_string) { "ar" } + + it { is_expected.to contain_exactly(have_attributes(consumer_name: "Foo", provider_name: "Bar")) } + end + + context "with a filter matching both consumer and provider" do + let(:query_string) { "o" } + + it "returns the matching integrations" do + expect(subject).to contain_exactly( + have_attributes(consumer_name: "Foo", provider_name: "Bar"), + have_attributes(consumer_name: "Cat", provider_name: "Dog") + ) + end + end + end + describe "relationships" do before do td.set_now(DateTime.new(2018, 1, 7)) diff --git a/spec/lib/pact_broker/integrations/repository_spec.rb b/spec/lib/pact_broker/integrations/repository_spec.rb index ff19b550f..8d97dfae6 100644 --- a/spec/lib/pact_broker/integrations/repository_spec.rb +++ b/spec/lib/pact_broker/integrations/repository_spec.rb @@ -4,6 +4,39 @@ module PactBroker module Integrations describe Repository do + describe "find" do + before do + Timecop.freeze(Date.today - 5) do + td.publish_pact(consumer_name: "Foo", provider_name: "Bar", consumer_version_number: "1") + end + + Timecop.freeze(Date.today - 4) do + td.create_verification(provider_version: "2") + end + + Timecop.freeze(Date.today - 3) do + td.publish_pact(consumer_name: "Apple", provider_name: "Pear", consumer_version_number: "1") + end + + Timecop.freeze(Date.today - 2) do + td.create_verification(provider_version: "2") + end + + # No contract data date + td.create_consumer("Dog") + .create_provider("Cat") + .create_integration + end + + subject { Repository.new.find } + + it "it orders by most recent event" do + expect(subject[0]).to have_attributes(consumer_name: "Apple") + expect(subject[1]).to have_attributes(consumer_name: "Foo") + expect(subject[2]).to have_attributes(consumer_name: "Dog") + end + end + describe "#set_contract_data_updated_at" do before do # A -> B diff --git a/spec/lib/pact_broker/integrations/service_spec.rb b/spec/lib/pact_broker/integrations/service_spec.rb index eda4e02a4..475fcd292 100644 --- a/spec/lib/pact_broker/integrations/service_spec.rb +++ b/spec/lib/pact_broker/integrations/service_spec.rb @@ -3,55 +3,6 @@ module PactBroker module Integrations describe Service do - describe "find_all" do - before do - allow(PactBroker::Integrations::Integration).to receive(:eager).and_return(dataset) - allow(dataset).to receive(:eager).and_return(dataset) - allow(dataset).to receive(:all).and_return(integrations) - end - - let(:dataset) { double("integrations") } - let(:integrations) { [ integration_1, integration_2 ] } - let(:latest_pact_or_verification_publication_date_1) { DateTime.new(2019, 1, 1) } - let(:latest_pact_or_verification_publication_date_2) { DateTime.new(2019, 2, 1) } - let(:integration_1) do - int = Integration.new - allow(int).to receive(:consumer_name).and_return("consumer 1") - allow(int).to receive(:provider_name).and_return("provider 1") - allow(int).to receive(:latest_pact_or_verification_publication_date).and_return(latest_pact_or_verification_publication_date_1) - int - end - - let(:integration_2) do - int = Integration.new - allow(int).to receive(:consumer_name).and_return("consumer 2") - allow(int).to receive(:provider_name).and_return("provider 2") - allow(int).to receive(:latest_pact_or_verification_publication_date).and_return(latest_pact_or_verification_publication_date_2) - int - end - - it "sorts the integrations with the most recently active first so that the UI doesn't need to do it" do - expect(Service.find_all.first).to be integration_2 - end - - context "when an integration has no publication date" do - let(:latest_pact_or_verification_publication_date_1) { nil } - - it "sorts the integration with the nil date last" do - expect(Service.find_all.first).to be integration_2 - end - end - - context "when all integrations have no publication date" do - let(:latest_pact_or_verification_publication_date_1) { nil } - let(:latest_pact_or_verification_publication_date_2) { nil } - - it "sorts the integrations by name" do - expect(Service.find_all.first).to be integration_1 - end - end - end - describe "#delete" do subject { Service.delete("Foo", "Bar") }