From dbe9376d3ec939bee89bd3caa085d4a81841f16b Mon Sep 17 00:00:00 2001
From: Beth Skurrie <beth.skurrie@smartbear.com>
Date: Mon, 19 Jun 2023 13:31:25 +1000
Subject: [PATCH 1/3] feat: add pagination and filtering for integrations
 endpoint

PACT-1070
---
 .../api/decorators/integration_decorator.rb   |  1 +
 .../api/decorators/integrations_decorator.rb  |  3 +
 .../api/resources/filter_methods.rb           | 15 +++++
 lib/pact_broker/api/resources/integrations.rb | 22 +++++--
 lib/pact_broker/dataset.rb                    | 15 +++++
 lib/pact_broker/integrations/integration.rb   | 10 ++++
 lib/pact_broker/integrations/repository.rb    | 13 ++++
 lib/pact_broker/integrations/service.rb       | 18 +-----
 lib/pact_broker/pacticipants/repository.rb    |  2 +-
 spec/features/get_integrations_spec.rb        | 44 +++++++++++---
 .../api/resources/integrations_spec.rb        | 59 +++++++++++++++++++
 .../integrations/integration_spec.rb          | 39 ++++++++++++
 .../integrations/repository_spec.rb           | 33 +++++++++++
 .../pact_broker/integrations/service_spec.rb  | 49 ---------------
 14 files changed, 246 insertions(+), 77 deletions(-)
 create mode 100644 lib/pact_broker/api/resources/filter_methods.rb
 create mode 100644 spec/lib/pact_broker/api/resources/integrations_spec.rb

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..b3f996a10 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,12 @@ 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..177ff61ef 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))
+          .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..de8275643
--- /dev/null
+++ b/spec/lib/pact_broker/api/resources/integrations_spec.rb
@@ -0,0 +1,59 @@
+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") }
 

From 4f99c328a84544bf3d4cc30f5710df3d84c0366c Mon Sep 17 00:00:00 2001
From: Beth Skurrie <beth.skurrie@smartbear.com>
Date: Mon, 19 Jun 2023 13:37:36 +1000
Subject: [PATCH 2/3] style: whitespace

---
 lib/pact_broker/dataset.rb                              | 2 --
 spec/lib/pact_broker/api/resources/integrations_spec.rb | 1 -
 2 files changed, 3 deletions(-)

diff --git a/lib/pact_broker/dataset.rb b/lib/pact_broker/dataset.rb
index b3f996a10..97ad6ea22 100644
--- a/lib/pact_broker/dataset.rb
+++ b/lib/pact_broker/dataset.rb
@@ -92,8 +92,6 @@ def postgres?
     def escape_wildcards(value)
       value.gsub("_", "\\_").gsub("%", "\\%")
     end
-
     private :escape_wildcards
-
   end
 end
diff --git a/spec/lib/pact_broker/api/resources/integrations_spec.rb b/spec/lib/pact_broker/api/resources/integrations_spec.rb
index de8275643..4cb8939f3 100644
--- a/spec/lib/pact_broker/api/resources/integrations_spec.rb
+++ b/spec/lib/pact_broker/api/resources/integrations_spec.rb
@@ -27,7 +27,6 @@ module Resources
 
           subject { get(path, params, rack_headers) }
 
-
           it "validates the query params" do
             expect(PactBroker::Api::Contracts::PaginationQueryParamsSchema).to receive(:call).with(params)
             subject

From dc1b62b0d7cad33c7cb5f4c5b68a5a071022df52 Mon Sep 17 00:00:00 2001
From: Beth Skurrie <beth.skurrie@smartbear.com>
Date: Tue, 20 Jun 2023 09:12:12 +1000
Subject: [PATCH 3/3] chore: sort nulls last for postgres

---
 lib/pact_broker/integrations/repository.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/pact_broker/integrations/repository.rb b/lib/pact_broker/integrations/repository.rb
index 177ff61ef..e790a32da 100644
--- a/lib/pact_broker/integrations/repository.rb
+++ b/lib/pact_broker/integrations/repository.rb
@@ -12,7 +12,7 @@ def find(filter_options = {}, pagination_options = {}, eager_load_associations =
         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))
+          .order(Sequel.desc(:contract_data_updated_at, nulls: :last))
           .all_with_pagination_options(pagination_options)
       end