From d1e92e8dbbdc222da6f55e04046dfaf6fe88f09a Mon Sep 17 00:00:00 2001
From: Beth Skurrie <beth@bethesque.com>
Date: Fri, 18 Aug 2023 07:20:48 +1000
Subject: [PATCH 1/6] feat: improve performance of matrix when multiple
 selectors are specified

---
 lib/pact_broker/domain/version.rb             |   1 +
 lib/pact_broker/matrix/every_row.rb           |  21 ++++
 lib/pact_broker/matrix/query_builder.rb       |   8 +-
 lib/pact_broker/matrix/quick_row.rb           | 109 ++++++++++++++----
 lib/pact_broker/matrix/repository.rb          |   3 +-
 lib/pact_broker/matrix/resolved_selector.rb   |   4 +
 lib/pact_broker/test/test_data_builder.rb     |   8 ++
 spec/lib/pact_broker/matrix/every_row_spec.rb |   4 +-
 8 files changed, 128 insertions(+), 30 deletions(-)

diff --git a/lib/pact_broker/domain/version.rb b/lib/pact_broker/domain/version.rb
index 4bc44b31a..a8a5fbc95 100644
--- a/lib/pact_broker/domain/version.rb
+++ b/lib/pact_broker/domain/version.rb
@@ -213,6 +213,7 @@ def delete
         end
 
         # rubocop: disable Metrics/CyclomaticComplexity
+        # @param [PactBroker::Matrix::UnresolvedSelector] selector
         def for_selector(selector)
           query = self
           query = query.where_pacticipant_name(selector.pacticipant_name) if selector.pacticipant_name
diff --git a/lib/pact_broker/matrix/every_row.rb b/lib/pact_broker/matrix/every_row.rb
index 4c882b09f..b61d621de 100644
--- a/lib/pact_broker/matrix/every_row.rb
+++ b/lib/pact_broker/matrix/every_row.rb
@@ -5,6 +5,21 @@ module Matrix
     class EveryRow < PactBroker::Matrix::QuickRow
       set_dataset(Sequel.as(:pact_publications, :p))
 
+      class Verification < Sequel::Model(:verifications)
+        dataset_module do
+          select(:select_verification_columns, Sequel[:verifications][:id].as(:verification_id), :provider_version_id, Sequel[:verifications][:created_at].as(:provider_version_created_at), Sequel[:verifications][:pact_version_id])
+          select(:select_pact_version_id, Sequel[:verifications][:pact_version_id])
+
+          def select_distinct_pact_version_id
+            select_pact_version_id.distinct
+          end
+
+          def join_versions(versions_dataset)
+            join(versions_dataset, { Sequel[:verifications][:provider_version_id] => Sequel[:versions][:id] }, table_alias: :versions)
+          end
+        end
+      end
+
       P_V_JOIN = { Sequel[:p][:pact_version_id] => Sequel[:v][:pact_version_id] }
 
       PACT_COLUMNS = [
@@ -29,14 +44,20 @@ class EveryRow < PactBroker::Matrix::QuickRow
       ALL_COLUMNS = PACT_COLUMNS + VERIFICATION_COLUMNS
 
       SELECT_ALL_COLUMN_ARGS = [:select_all_columns] + ALL_COLUMNS
+      SELECT_PACT_COLUMNS_ARGS = [:select_pact_columns] + PACT_COLUMNS
 
       dataset_module do
         select(*SELECT_ALL_COLUMN_ARGS)
+        select(*SELECT_PACT_COLUMNS_ARGS)
 
         def join_verifications
           left_outer_join(:verifications, P_V_JOIN, { table_alias: :v } )
         end
 
+        def verification_model
+          EveryRow::Verification
+        end
+
         def inner_join_verifications
           join(:verifications, P_V_JOIN, { table_alias: :v } )
         end
diff --git a/lib/pact_broker/matrix/query_builder.rb b/lib/pact_broker/matrix/query_builder.rb
index f00fb28a3..8620eca4b 100644
--- a/lib/pact_broker/matrix/query_builder.rb
+++ b/lib/pact_broker/matrix/query_builder.rb
@@ -46,11 +46,11 @@ def self.consumer_or_consumer_version_matches(query_ids, qualifier)
         Sequel.|(*ors)
       end
 
-      # Some selecters are specified in the query, others are implied (when only one pacticipant is specified,
-      # the integrations are automatically worked out, and the selectors for these are of type :implied )
+      # Some selecters are specified in the query, others are inferred (when only one pacticipant is specified,
+      # the integrations are automatically worked out, and the selectors for these are of type :inferred )
       # When there are 3 pacticipants that each have dependencies on each other (A->B, A->C, B->C), the query
-      # to deploy C (implied A, implied B, specified C) was returning the A->B row because it matched the
-      # implied selectors as well.
+      # to deploy C (inferred A, inferred B, specified C) was returning the A->B row because it matched the
+      # inferred selectors as well.
       # This extra filter makes sure that every row that is returned actually matches one of the specified
       # selectors.
       def self.either_consumer_or_provider_was_specified_in_query(query_ids, qualifier = nil)
diff --git a/lib/pact_broker/matrix/quick_row.rb b/lib/pact_broker/matrix/quick_row.rb
index f7785990f..65b6d9edf 100644
--- a/lib/pact_broker/matrix/quick_row.rb
+++ b/lib/pact_broker/matrix/quick_row.rb
@@ -27,8 +27,8 @@ module PactBroker
   module Matrix
     # TODO rename this to just Row
     # rubocop: disable Metrics/ClassLength
-    class QuickRow < Sequel::Model(Sequel.as(:latest_pact_publication_ids_for_consumer_versions, :p))
 
+    class QuickRow < Sequel::Model(Sequel.as(:latest_pact_publication_ids_for_consumer_versions, :p))
       # Tables
       LV = :latest_verification_id_for_pact_version_and_provider_version
       LP = :latest_pact_publication_ids_for_consumer_versions
@@ -69,6 +69,7 @@ class QuickRow < Sequel::Model(Sequel.as(:latest_pact_publication_ids_for_consum
       # cachable select arguments
       SELECT_ALL_COLUMN_ARGS = [:select_all_columns] + ALL_COLUMNS
       SELECT_PACTICIPANT_IDS_ARGS = [:select_pacticipant_ids, Sequel[:p][:consumer_id], Sequel[:p][:provider_id]]
+      SELECT_PACT_COLUMNS_ARGS = [:select_pact_columns] + PACT_COLUMNS
 
       EAGER_LOADED_RELATIONSHIPS_FOR_VERSION = { current_deployed_versions: :environment, current_supported_released_versions: :environment, branch_versions: [:branch_head, :version, branch: :pacticipant] }
 
@@ -82,12 +83,34 @@ class QuickRow < Sequel::Model(Sequel.as(:latest_pact_publication_ids_for_consum
       associate(:one_to_many, :consumer_version_tags, :class => "PactBroker::Tags::TagWithLatestFlag", primary_key: :consumer_version_id, key: :version_id)
       associate(:one_to_many, :provider_version_tags, :class => "PactBroker::Tags::TagWithLatestFlag", primary_key: :provider_version_id, key: :version_id)
 
+      class Verification < Sequel::Model(Sequel.as(:latest_verification_id_for_pact_version_and_provider_version, :v))
+        dataset_module do
+          select(*([:select_verification_columns] + QuickRow::VERIFICATION_COLUMNS + [Sequel[:v][:pact_version_id]]))
+          select(:select_pact_version_id, Sequel[:v][:pact_version_id])
+
+          def select_distinct_pact_version_id
+            select_pact_version_id.distinct
+          end
+
+          def join_versions(versions_dataset)
+            join(versions_dataset, { Sequel[:v][:provider_version_id] => Sequel[:versions][:id] }, table_alias: :versions)
+          end
+        end
+      end
+
       dataset_module do
         include PactBroker::Dataset
 
         select(*SELECT_ALL_COLUMN_ARGS)
+        select(*SELECT_PACT_COLUMNS_ARGS)
         select(*SELECT_PACTICIPANT_IDS_ARGS)
+        select(:select_pacticipant_and_pact_version_ids, Sequel[:p][:consumer_id], Sequel[:p][:provider_id], Sequel[:p][:pact_version_id])
+
+        def select_distinct_pacticipant_and_pact_version_ids
+          select_pacticipant_and_pact_version_ids.distinct
+        end
 
+        # @param [PactBroker::Matrix::ResolvedSelector] selector
         def distinct_integrations_for_selector_as_consumer(selector)
           select(:consumer_id, :provider_id)
             .distinct
@@ -98,19 +121,23 @@ def distinct_integrations_for_selector_as_consumer(selector)
             .join_providers(:integrations, :providers)
         end
 
-        def distinct_integrations selectors, infer_integrations
+        # @param [Array<PactBroker::Matrix::ResolvedSelector>] selectors
+        def distinct_integrations(selectors)
           query = if selectors.size == 1
                     pacticipant_ids_matching_one_selector_optimised(selectors)
                   else
-                    query = select_pacticipant_ids.distinct
-                    if infer_integrations
-                      query.matching_any_of_multiple_selectors(selectors)
+                    if selectors.all?(&:only_pacticipant_name_specified?)
+                      matching_multiple_selectors_without_joining_verifications(selectors)
+                        .select_pacticipant_ids
+                        .distinct
                     else
-                      if selectors.all?(&:only_pacticipant_name_specified?)
-                        query.matching_multiple_selectors_without_joining_verifications(selectors)
-                      else
-                        query.matching_multiple_selectors_joining_verifications(selectors)
-                      end
+                      matching_multiple_selectors_joining_verifications(
+                          selectors,
+                          pact_columns: :select_distinct_pacticipant_and_pact_version_ids,
+                          verifications_columns: :select_distinct_pact_version_id
+                        )
+                        .select_pacticipant_ids
+                        .distinct
                     end
                   end
 
@@ -125,11 +152,16 @@ def distinct_integrations selectors, infer_integrations
             .join_providers(:pacticipant_ids, :p)
         end
 
+        # The matrix query used to determine the final dataset
+        # @param [Array<PactBroker::Matrix::ResolvedSelector>] selectors
         def matching_selectors selectors
           if selectors.size == 1
-            matching_one_selector(selectors)
+            select_all_columns.matching_one_selector(selectors)
           else
-            matching_multiple_selectors_joining_verifications(selectors)
+            matching_multiple_selectors_joining_verifications(
+              selectors,
+              pact_columns: :select_pact_columns,
+              verifications_columns: :select_verification_columns)
           end
         end
 
@@ -172,7 +204,6 @@ def matching_one_selector(selectors)
             }
 
           rows_where_selector_matches_provider_cols = inner_join_verifications_matching_one_selector_provider_or_provider_version(query_ids)
-
           rows_where_selector_matches_consumer_cols.union(rows_where_selector_matches_provider_cols)
         end
 
@@ -208,17 +239,41 @@ def distinct_pacticipant_ids_where_provider_or_provider_version_matches(query_id
         # Instead, we need to filter the verifications dataset down to only the ones specified in the selectors first,
         # and THEN join them to the pacts, so that we get a row for the pact with null provider version
         # and verification fields.
+        # @param [Array<PactBroker::Matrix::ResolvedSelector>] selectors
+        def matching_multiple_selectors_joining_verifications(selectors, pact_columns:, verifications_columns: )
+          pact_publications = pact_publications_matching_selectors_as_consumer(selectors, pact_columns: pact_columns).from_self(alias: :p)
+          verifications = verifications_matching_selectors_as_provider(selectors, verifications_columns: verifications_columns)
+          specified_pacticipant_ids = selectors.select(&:specified?).collect(&:pacticipant_id).uniq
+
+          pact_publications
+            .left_outer_join(verifications, { Sequel[:p][:pact_version_id] => Sequel[:v][:pact_version_id] }, { table_alias: :v })
+            .where(consumer_id: specified_pacticipant_ids).or(provider_id: specified_pacticipant_ids)
+        end
 
-        def matching_multiple_selectors_joining_verifications(selectors)
-          query_ids = QueryIds.from_selectors(selectors)
-          join_verifications_for(query_ids)
-            .where {
-              Sequel.&(
-                QueryBuilder.consumer_or_consumer_version_matches(query_ids, :p),
-                QueryBuilder.provider_or_provider_version_matches_or_pact_unverified(query_ids, :v, :p),
-                QueryBuilder.either_consumer_or_provider_was_specified_in_query(query_ids, :p)
-              )
-            }
+        # @param [Array<PactBroker::Matrix::ResolvedSelector>] selectors
+        def pact_publications_matching_selectors_as_consumer(selectors, pact_columns:)
+          unresolved_selectors = selectors.collect(&:original_selector).uniq
+          versions = unresolved_selectors.collect{ | selector | PactBroker::Domain::Version.select(Sequel[:versions][:id]).for_selector(selector).select(:id) }.reduce(&:union)
+          pacticipant_ids = selectors.collect(&:pacticipant_id).uniq
+          versions_join = { Sequel[:p][:consumer_version_id] => Sequel[:versions][:id] }
+          self.model.from_self(alias: :p).send(pact_columns).join(versions, versions_join, table_alias: :versions).where(provider_id: pacticipant_ids)
+        end
+
+
+        # @param [Array<PactBroker::Matrix::ResolvedSelector>] selectors
+        def verifications_matching_selectors_as_provider(selectors, verifications_columns: )
+          unresolved_selectors = selectors.collect(&:original_selector).uniq
+          versions = unresolved_selectors.collect{ | selector | PactBroker::Domain::Version.select(Sequel[:versions][:id]).for_selector(selector).select(:id) }.reduce(&:union)
+          pacticipant_ids = selectors.collect(&:pacticipant_id).uniq
+          verification_model
+            .send(verifications_columns)
+            .join_versions(versions)
+            .where(consumer_id: pacticipant_ids)
+        end
+
+
+        def verification_model
+          QuickRow::Verification
         end
 
         def matching_multiple_selectors_without_joining_verifications(selectors)
@@ -353,6 +408,14 @@ def to_s
         "#{consumer_name} v#{consumer_version_number} #{provider_name} #{provider_version_number} #{success}"
       end
 
+      def consumer_deets
+        "#{consumer_name} v#{consumer_version_number} #{provider_name}"
+      end
+
+      def provider_deets
+        "#{provider_name} #{provider_version_number} #{success}"
+      end
+
       def compare_number_desc number1, number2
         if number1 && number2
           number2 <=> number1
diff --git a/lib/pact_broker/matrix/repository.rb b/lib/pact_broker/matrix/repository.rb
index a58bee761..19cbc000f 100644
--- a/lib/pact_broker/matrix/repository.rb
+++ b/lib/pact_broker/matrix/repository.rb
@@ -195,7 +195,8 @@ def apply_latestby options, lines
       # rubocop: enable Metrics/CyclomaticComplexity
 
       def query_matrix selectors, options
-        query = base_model(options).select_all_columns
+        query = base_model(options)
+                  #.select_all_columns
                   .matching_selectors(selectors)
                   .order_by_last_action_date
 
diff --git a/lib/pact_broker/matrix/resolved_selector.rb b/lib/pact_broker/matrix/resolved_selector.rb
index 3ebb17ccf..c6d11d05d 100644
--- a/lib/pact_broker/matrix/resolved_selector.rb
+++ b/lib/pact_broker/matrix/resolved_selector.rb
@@ -187,6 +187,10 @@ def consider?
         !ignore?
       end
 
+      def original_selector
+        self[:original_selector]
+      end
+
       # rubocop: disable Metrics/CyclomaticComplexity, Metrics/MethodLength
       def description
         if latest_tagged? && pacticipant_version_number
diff --git a/lib/pact_broker/test/test_data_builder.rb b/lib/pact_broker/test/test_data_builder.rb
index 43d6c9fb4..8e37ee21b 100644
--- a/lib/pact_broker/test/test_data_builder.rb
+++ b/lib/pact_broker/test/test_data_builder.rb
@@ -386,6 +386,14 @@ def create_webhook_execution params = {}
         self
       end
 
+      # @param [Hash] parameters
+      # @option parameters [String] :provider_version
+      # @option parameters [Array<String>] :tag_names
+      # @option parameters [String] :branch
+      # @option parameters [Boolean] :success
+      # @option parameters [Integer] :number
+      # @option parameters [Boolean] :wip
+      # @option parameters [Boolean] :test_results
       def create_verification parameters = {}
         # This should use the verification service. what a mess
         parameters.delete(:comment)
diff --git a/spec/lib/pact_broker/matrix/every_row_spec.rb b/spec/lib/pact_broker/matrix/every_row_spec.rb
index 5480a4401..f18001690 100644
--- a/spec/lib/pact_broker/matrix/every_row_spec.rb
+++ b/spec/lib/pact_broker/matrix/every_row_spec.rb
@@ -19,11 +19,11 @@ module Matrix
         end
 
         let(:selector_1) do
-          PactBroker::Matrix::ResolvedSelector.for_pacticipant(foo, {}, :specified, false)
+          PactBroker::Matrix::ResolvedSelector.for_pacticipant(foo, PactBroker::Matrix::UnresolvedSelector.new(pacticipant_name: "Foo"), :specified, false)
         end
 
         let(:selector_2) do
-          PactBroker::Matrix::ResolvedSelector.for_pacticipant(bar, {}, :specified, false)
+          PactBroker::Matrix::ResolvedSelector.for_pacticipant(bar, PactBroker::Matrix::UnresolvedSelector.new(pacticipant_name: "Bar"), :specified, false)
         end
 
         let(:selectors) { [selector_1, selector_2] }

From dc63658db223cb5b74f52fca2b0f79b31058fb56 Mon Sep 17 00:00:00 2001
From: Beth Skurrie <beth.skurrie@smartbear.com>
Date: Sat, 19 Aug 2023 15:04:06 +1000
Subject: [PATCH 2/6] refactor: refactor the duck out of the matrix code, boy
 oh boy did that need it

---
 .../matrix/integrations_repository.rb         | 122 +++++++
 lib/pact_broker/matrix/quick_row.rb           |  13 +-
 lib/pact_broker/matrix/repository.rb          | 317 +++---------------
 lib/pact_broker/matrix/resolved_selector.rb   |   4 +-
 .../matrix/resolved_selector_builder.rb       |  84 +++++
 .../matrix/resolved_selectors_builder.rb      |  39 +++
 lib/pact_broker/matrix/row_ignorer.rb         |  36 ++
 lib/pact_broker/matrix/selector_ignorer.rb    |  59 ++++
 lib/pact_broker/matrix/selector_resolver.rb   | 130 +++++++
 lib/pact_broker/repositories.rb               |  10 +
 10 files changed, 533 insertions(+), 281 deletions(-)
 create mode 100644 lib/pact_broker/matrix/integrations_repository.rb
 create mode 100644 lib/pact_broker/matrix/resolved_selector_builder.rb
 create mode 100644 lib/pact_broker/matrix/resolved_selectors_builder.rb
 create mode 100644 lib/pact_broker/matrix/row_ignorer.rb
 create mode 100644 lib/pact_broker/matrix/selector_ignorer.rb
 create mode 100644 lib/pact_broker/matrix/selector_resolver.rb

diff --git a/lib/pact_broker/matrix/integrations_repository.rb b/lib/pact_broker/matrix/integrations_repository.rb
new file mode 100644
index 000000000..fa659e16c
--- /dev/null
+++ b/lib/pact_broker/matrix/integrations_repository.rb
@@ -0,0 +1,122 @@
+# A "find only" repository for the PactBroker::Matrix::Integration object.
+# The PactBroker::Matrix::Integration object is not a Sequel Model like the PactBroker::Integrations::Integration - it is built from the
+# matrix data specifically for a given matrix query, and as well as the consumer/provider attributes, it also
+# knows whether or not that particular depdency is required in the context of the specific matrix query.
+# eg. a HTTP consumer will always require that a provider is deployed, but a provider can be deployed if the consumer does not exist
+# in the given environment yet.
+
+module PactBroker
+  module Matrix
+    class IntegrationsRepository
+
+      # This is parameterised to Pactflow can have its own base model
+      # @param [Class<QuickRow>] base_model_for_integrations
+      def initialize(base_model_for_integrations)
+        @base_model_for_integrations = base_model_for_integrations
+      end
+
+      # Find all the Integrations required for this query, using the options to determine whether to find
+      # the inferred integrations or not.
+      # @param [Array<PactBroker::Matrix::ResolvedSelector>] resolved_specified_selectors
+      # @param [Boolean] infer_selectors_for_integrations
+      # @return [Array<PactBroker::Matrix::Integration>]
+      def find_integrations_for_specified_selectors(resolved_specified_selectors, infer_selectors_for_integrations)
+        if infer_selectors_for_integrations
+          find_integrations_for_specified_selectors_with_inferred_integrations(resolved_specified_selectors)
+        else
+          find_integrations_for_specified_selectors_only(resolved_specified_selectors)
+        end
+      end
+
+      private
+
+      attr_reader :base_model_for_integrations
+
+      # Find the Integrations only for the selectors specifed in the query (don't infer any others)
+      # @param [Array<PactBroker::Matrix::ResolvedSelector>] resolved_specified_selectors
+      # @return [Array<PactBroker::Matrix::Integration>]
+      def find_integrations_for_specified_selectors_only(resolved_specified_selectors)
+        specified_pacticipant_names = resolved_specified_selectors.collect(&:pacticipant_name)
+        base_model_for_integrations
+          .distinct_integrations(resolved_specified_selectors)
+          .all
+          .collect(&:to_hash)
+          .collect do | integration_hash |
+            required = is_a_row_for_this_integration_required?(specified_pacticipant_names, integration_hash[:consumer_name])
+            Integration.from_hash(integration_hash.merge(required: required))
+          end
+      end
+
+      # Find the Integrations for the specified selectors and infer the other Integrations
+      # @param [Array<PactBroker::Matrix::ResolvedSelector>] resolved_specified_selectors
+      # @return [Array<PactBroker::Matrix::Integration>]
+      def find_integrations_for_specified_selectors_with_inferred_integrations(resolved_specified_selectors)
+        integrations = integrations_where_specified_selector_is_consumer(resolved_specified_selectors) +
+                        integrations_where_specified_selector_is_provider(resolved_specified_selectors)
+        deduplicate_integrations(integrations)
+      end
+
+      # Find all the providers for the consumers specified in the query. Takes into account the consumer versions specified in the query,
+      # not just the consumer names.
+      # @param [Array<PactBroker::Matrix::ResolvedSelector>] the resolved selectors that were specified in the query
+      # @return [Array<PactBroker::Matrix::Integration>]
+      def integrations_where_specified_selector_is_consumer(resolved_specified_selectors)
+        resolved_specified_selectors.flat_map do | selector |
+          # Could optimise this to all in one query, but it's a small gain
+          base_model_for_integrations
+            .distinct_integrations_for_selector_as_consumer(selector)
+            .all
+            .collect do | integration |
+              Integration.from_hash(
+                consumer_id: integration[:consumer_id],
+                consumer_name: integration[:consumer_name],
+                provider_id: integration[:provider_id],
+                provider_name: integration[:provider_name],
+                required: true # consumer requires the provider to be present
+              )
+            end
+        end
+      end
+
+      # Why does the consumer equivalent of this method use the QuickRow distinct_integrations_for_selector_as_consumer
+      # while this method uses the Integration?
+      # Find all the consumers for the providers specified in the query. Does not take into consideration the provider version (not sure why).
+      # @param [Array<PactBroker::Matrix::ResolvedSelector>] the resolved selectors that were specified in the query
+      # @return [Array<PactBroker::Matrix::Integration>]
+      def integrations_where_specified_selector_is_provider(resolved_specified_selectors)
+        integrations_involving_specified_providers = PactBroker::Integrations::Integration
+                                                      .where(provider_id: resolved_specified_selectors.collect(&:pacticipant_id))
+                                                      .eager(:consumer, :provider)
+                                                      .all
+
+        integrations_involving_specified_providers.collect do | integration |
+          Integration.from_hash(
+            consumer_id: integration.consumer.id,
+            consumer_name: integration.consumer.name,
+            provider_id: integration.provider.id,
+            provider_name: integration.provider.name,
+            required: false # provider does not require the consumer to be present
+          )
+        end
+      end
+
+      # Deduplicate a list of Integrations
+      # @param [Array<PactBroker::Matrix::Integration>] integrations
+      # @return [Array<PactBroker::Matrix::Integration>]
+      def deduplicate_integrations(integrations)
+        integrations
+          .group_by{ | integration| [integration.consumer_id, integration.provider_id] }
+          .values
+          .collect { | duplicate_integrations | duplicate_integrations.find(&:required?) || duplicate_integrations.first }
+      end
+
+      # If a specified pacticipant is a consumer, then its provider is required to be deployed
+      # to the same environment before the consumer can be deployed.
+      # If a specified pacticipant is a provider only, then it may be deployed
+      # without the consumer being present, but cannot break an existing consumer.
+      def is_a_row_for_this_integration_required?(specified_pacticipant_names, consumer_name)
+        specified_pacticipant_names.include?(consumer_name)
+      end
+    end
+  end
+end
diff --git a/lib/pact_broker/matrix/quick_row.rb b/lib/pact_broker/matrix/quick_row.rb
index 65b6d9edf..16afcad99 100644
--- a/lib/pact_broker/matrix/quick_row.rb
+++ b/lib/pact_broker/matrix/quick_row.rb
@@ -65,8 +65,7 @@ class QuickRow < Sequel::Model(Sequel.as(:latest_pact_publication_ids_for_consum
 
       ALL_COLUMNS = PACT_COLUMNS + VERIFICATION_COLUMNS
 
-
-      # cachable select arguments
+      # cacheable select arguments
       SELECT_ALL_COLUMN_ARGS = [:select_all_columns] + ALL_COLUMNS
       SELECT_PACTICIPANT_IDS_ARGS = [:select_pacticipant_ids, Sequel[:p][:consumer_id], Sequel[:p][:provider_id]]
       SELECT_PACT_COLUMNS_ARGS = [:select_pact_columns] + PACT_COLUMNS
@@ -85,6 +84,7 @@ class QuickRow < Sequel::Model(Sequel.as(:latest_pact_publication_ids_for_consum
 
       class Verification < Sequel::Model(Sequel.as(:latest_verification_id_for_pact_version_and_provider_version, :v))
         dataset_module do
+          # declaring the selects this way makes them cacheable
           select(*([:select_verification_columns] + QuickRow::VERIFICATION_COLUMNS + [Sequel[:v][:pact_version_id]]))
           select(:select_pact_version_id, Sequel[:v][:pact_version_id])
 
@@ -121,6 +121,7 @@ def distinct_integrations_for_selector_as_consumer(selector)
             .join_providers(:integrations, :providers)
         end
 
+        # Find all the integrations (consumer/provider pairs) that involve the given selectors.
         # @param [Array<PactBroker::Matrix::ResolvedSelector>] selectors
         def distinct_integrations(selectors)
           query = if selectors.size == 1
@@ -251,10 +252,11 @@ def matching_multiple_selectors_joining_verifications(selectors, pact_columns:,
         end
 
         # @param [Array<PactBroker::Matrix::ResolvedSelector>] selectors
-        def pact_publications_matching_selectors_as_consumer(selectors, pact_columns:)
-          unresolved_selectors = selectors.collect(&:original_selector).uniq
+        def pact_publications_matching_selectors_as_consumer(resolved_selectors, pact_columns:)
+          # get the UnresolvedSelector objects back out of the resolved_selectors because the Version.for_selector() method uses the UnresolvedSelector
+          unresolved_selectors = resolved_selectors.collect(&:original_selector).uniq
           versions = unresolved_selectors.collect{ | selector | PactBroker::Domain::Version.select(Sequel[:versions][:id]).for_selector(selector).select(:id) }.reduce(&:union)
-          pacticipant_ids = selectors.collect(&:pacticipant_id).uniq
+          pacticipant_ids = resolved_selectors.collect(&:pacticipant_id).uniq
           versions_join = { Sequel[:p][:consumer_version_id] => Sequel[:versions][:id] }
           self.model.from_self(alias: :p).send(pact_columns).join(versions, versions_join, table_alias: :versions).where(provider_id: pacticipant_ids)
         end
@@ -262,6 +264,7 @@ def pact_publications_matching_selectors_as_consumer(selectors, pact_columns:)
 
         # @param [Array<PactBroker::Matrix::ResolvedSelector>] selectors
         def verifications_matching_selectors_as_provider(selectors, verifications_columns: )
+          # get the UnresolvedSelector objects back out of the resolved_selectors because the Version.for_selector() method uses the UnresolvedSelector
           unresolved_selectors = selectors.collect(&:original_selector).uniq
           versions = unresolved_selectors.collect{ | selector | PactBroker::Domain::Version.select(Sequel[:versions][:id]).for_selector(selector).select(:id) }.reduce(&:union)
           pacticipant_ids = selectors.collect(&:pacticipant_id).uniq
diff --git a/lib/pact_broker/matrix/repository.rb b/lib/pact_broker/matrix/repository.rb
index 19cbc000f..3f008c11e 100644
--- a/lib/pact_broker/matrix/repository.rb
+++ b/lib/pact_broker/matrix/repository.rb
@@ -1,26 +1,18 @@
-require "pact_broker/logging"
-require "pact_broker/repositories/helpers"
 require "pact_broker/matrix/quick_row"
 require "pact_broker/matrix/every_row"
-require "pact_broker/error"
 require "pact_broker/matrix/query_results"
 require "pact_broker/matrix/integration"
 require "pact_broker/matrix/query_results_with_deployment_status_summary"
-require "pact_broker/matrix/resolved_selector"
 require "pact_broker/matrix/unresolved_selector"
 require "pact_broker/verifications/latest_verification_id_for_pact_version_and_provider_version"
+require "pact_broker/matrix/integrations_repository"
+require "pact_broker/matrix/resolved_selectors_builder"
+require "pact_broker/matrix/row_ignorer"
 
 module PactBroker
   module Matrix
-
-    class Error < PactBroker::Error; end
-
     class Repository
-      include PactBroker::Repositories::Helpers
       include PactBroker::Repositories
-      include PactBroker::Logging
-
-      # TODO move latest verification logic in to database
 
       # Used when using table_print to output query results
       TP_COLS = [ :consumer_version_number, :pact_revision_number, :provider_version_number, :verification_number]
@@ -29,43 +21,29 @@ class Repository
       GROUP_BY_PROVIDER = [:consumer_name, :consumer_version_number, :provider_name]
       GROUP_BY_PACT = [:consumer_name, :provider_name]
 
-      # rubocop: disable Metrics/CyclomaticComplexity
-      def find_ids_for_pacticipant_names params
-        criteria  = {}
-
-        if params[:consumer_name] || params[:provider_name]
-          if params[:consumer_name]
-            pacticipant = PactBroker::Domain::Pacticipant.where(name_like(:name, params[:consumer_name])).single_record
-            criteria[:consumer_id] = pacticipant.id if pacticipant
-          end
-
-          if params[:provider_name]
-            pacticipant = PactBroker::Domain::Pacticipant.where(name_like(:name, params[:provider_name])).single_record
-            criteria[:provider_id] = pacticipant.id if pacticipant
-          end
-        end
-
-        if params[:pacticipant_name]
-          pacticipant = PactBroker::Domain::Pacticipant.where(name_like(:name, params[:pacticipant_name])).single_record
-          criteria[:pacticipant_id] = pacticipant.id if pacticipant
-        end
-
-        criteria[:tag_name] = params[:tag_name] if params[:tag_name].is_a?(String) # Could be a sym from resource parameters in api.rb
-        criteria
-      end
-      # rubocop: enable Metrics/CyclomaticComplexity
-
-      # Return the latest matrix row (pact/verification) for each consumer_version_number/provider_version_number
-      def find specified_selectors, options = {}
-        resolved_ignore_selectors = resolve_ignore_selectors(options)
-        resolved_specified_selectors = resolve_versions_and_add_ids(specified_selectors, :specified, resolved_ignore_selectors)
-        integrations = find_integrations_for_specified_selectors(resolved_specified_selectors, options)
-        resolved_selectors = add_any_inferred_selectors(resolved_specified_selectors, resolved_ignore_selectors, integrations, options)
-        considered_rows, ignored_rows = find_considered_and_ignored_rows(resolved_selectors, resolved_ignore_selectors, options)
-        QueryResults.new(considered_rows.sort, ignored_rows.sort, specified_selectors, options, resolved_selectors, resolved_ignore_selectors, integrations)
-      end
-
-      def find_for_consumer_and_provider pacticipant_1_name, pacticipant_2_name
+      # THE METHOD for querying the Matrix
+      # @param [Array<PactBroker::Matrix::UnresolvedSelector>] unresolved_specified_selectors
+      # @param [Hash] options
+      def find(unresolved_specified_selectors, options = {})
+        infer_selectors = infer_selectors_for_integrations?(options)
+        resolved_selectors_builder = ResolvedSelectorsBuilder.new
+        resolved_selectors_builder.resolve_selectors(unresolved_specified_selectors, options.fetch(:ignore_selectors, []))
+        integrations = matrix_integration_repository.find_integrations_for_specified_selectors(resolved_selectors_builder.specified_selectors, infer_selectors)
+        resolved_selectors_builder.resolve_inferred_selectors(integrations, options) if infer_selectors
+
+        considered_rows, ignored_rows = find_considered_and_ignored_rows(resolved_selectors_builder.all_selectors, resolved_selectors_builder.ignore_selectors, options)
+        QueryResults.new(
+          considered_rows.sort,
+          ignored_rows.sort,
+          unresolved_specified_selectors,
+          options,
+          resolved_selectors_builder.all_selectors,
+          resolved_selectors_builder.ignore_selectors,
+          integrations
+        )
+      end
+
+      def find_for_consumer_and_provider(pacticipant_1_name, pacticipant_2_name)
         selectors = [ UnresolvedSelector.new(pacticipant_name: pacticipant_1_name), UnresolvedSelector.new(pacticipant_name: pacticipant_2_name)]
         options = { latestby: "cvpv" }
         find(selectors, options)
@@ -73,90 +51,24 @@ def find_for_consumer_and_provider pacticipant_1_name, pacticipant_2_name
 
       private
 
-      def find_considered_and_ignored_rows(resolved_selectors, resolved_ignore_selectors, options)
-        rows = query_matrix(resolved_selectors, options)
+      # If the user has specified --to TAG or --to-environment ENVIRONMENT in the CLI
+      # (or nothing, which to defaults to latest=true - "with the latest version of the other integrated applications")
+      # we need to work out what the integrations are between the specified selectors and the other pacticipant versions
+      # in the target environment/branches/tags.
+      # @param [Hash] options the matrix options
+      # @return [Boolean]
+      def infer_selectors_for_integrations?(options)
+        options[:latest] || options[:tag] || options[:branch] || options[:environment_name] || options[:main_branch]
+      end
+
+      def find_considered_and_ignored_rows(all_resolved_selectors, resolved_ignore_selectors, options)
+        rows = query_matrix(all_resolved_selectors, options)
         rows = apply_latestby(options, rows)
         rows = apply_success_filter(rows, options)
-        considered_rows, ignored_rows = split_rows_into_considered_and_ignored(rows, resolved_ignore_selectors)
+        considered_rows, ignored_rows = RowIgnorer.split_rows_into_considered_and_ignored(rows, resolved_ignore_selectors)
         return considered_rows, ignored_rows
       end
 
-      def find_integrations_for_specified_selectors(resolved_specified_selectors, options)
-        if infer_selectors_for_integrations?(options)
-          find_integrations_for_specified_selectors_with_inferred_integrations(resolved_specified_selectors)
-        else
-          find_integrations_for_specified_selectors_only(resolved_specified_selectors)
-        end
-      end
-
-      def find_integrations_for_specified_selectors_only(resolved_specified_selectors)
-        specified_pacticipant_names = resolved_specified_selectors.collect(&:pacticipant_name)
-        base_model_for_integrations
-          .distinct_integrations(resolved_specified_selectors, false)
-          .collect(&:to_hash)
-          .collect do | integration_hash |
-            required = is_a_row_for_this_integration_required?(specified_pacticipant_names, integration_hash[:consumer_name])
-            Integration.from_hash(integration_hash.merge(required: required))
-          end
-      end
-
-      def find_integrations_for_specified_selectors_with_inferred_integrations(resolved_specified_selectors)
-        integrations = integrations_where_specified_selector_is_consumer(resolved_specified_selectors) +
-                        integrations_where_specified_selector_is_provider(resolved_specified_selectors)
-        deduplicate_integrations(integrations)
-      end
-
-      def integrations_where_specified_selector_is_consumer(resolved_specified_selectors)
-        resolved_specified_selectors.flat_map do | selector |
-          base_model_for_integrations
-            .distinct_integrations_for_selector_as_consumer(selector)
-            .all
-            .collect do | integration |
-              Integration.from_hash(
-                consumer_id: integration[:consumer_id],
-                consumer_name: integration[:consumer_name],
-                provider_id: integration[:provider_id],
-                provider_name: integration[:provider_name],
-                required: true # synchronous consumer requires the provider to be present
-              )
-            end
-        end
-      end
-
-      # Why does the consumer equivalent of this method use the QuickRow distinct_integrations_for_selector_as_consumer
-      # while this method uses the Integration?
-      def integrations_where_specified_selector_is_provider(resolved_specified_selectors)
-        integrations_involving_specified_providers = PactBroker::Integrations::Integration
-                                                      .where(provider_id: resolved_specified_selectors.collect(&:pacticipant_id))
-                                                      .eager(:consumer, :provider)
-                                                      .all
-
-        integrations_involving_specified_providers.collect do | integration |
-          Integration.from_hash(
-            consumer_id: integration.consumer.id,
-            consumer_name: integration.consumer.name,
-            provider_id: integration.provider.id,
-            provider_name: integration.provider.name,
-            required: false # synchronous provider does not require the consumer to be present
-          )
-        end
-      end
-
-      def deduplicate_integrations(integrations)
-        integrations
-          .group_by{ | integration| [integration.consumer_id, integration.provider_id] }
-          .values
-          .collect { | duplicate_integrations | duplicate_integrations.find(&:required?) || duplicate_integrations.first }
-      end
-
-      def add_any_inferred_selectors(resolved_specified_selectors, resolved_ignore_selectors, integrations, options)
-        if infer_selectors_for_integrations?(options)
-          resolved_specified_selectors + inferred_selectors(resolved_specified_selectors, resolved_ignore_selectors, integrations, options)
-        else
-          resolved_specified_selectors
-        end
-      end
-
       def apply_success_filter(rows_with_latest_by_applied, options)
         # This needs to be done after the latestby, so can't be done in the db unless
         # the latestby logic is moved to the db
@@ -167,17 +79,9 @@ def apply_success_filter(rows_with_latest_by_applied, options)
         end
       end
 
-      # If a specified pacticipant is a consumer, then its provider is required to be deployed
-      # to the same environment before the consumer can be deployed.
-      # If a specified pacticipant is a provider only, then it may be deployed
-      # without the consumer being present.
-      def is_a_row_for_this_integration_required?(specified_pacticipant_names, consumer_name)
-        specified_pacticipant_names.include?(consumer_name)
-      end
-
       # rubocop: disable Metrics/CyclomaticComplexity
       # It would be nicer to do this in the SQL, but it requires time that I don't have at the moment
-      def apply_latestby options, lines
+      def apply_latestby(options, lines)
         return lines unless options[:latestby]
         group_by_columns = case options[:latestby]
                            when "cvpv" then GROUP_BY_PROVIDER_VERSION_NUMBER
@@ -194,10 +98,9 @@ def apply_latestby options, lines
       end
       # rubocop: enable Metrics/CyclomaticComplexity
 
-      def query_matrix selectors, options
+      def query_matrix(all_resolved_selectors, options)
         query = base_model(options)
-                  #.select_all_columns
-                  .matching_selectors(selectors)
+                  .matching_selectors(all_resolved_selectors)
                   .order_by_last_action_date
 
         query = query.limit(options[:limit]) if options[:limit]
@@ -207,142 +110,6 @@ def query_matrix selectors, options
       def base_model(options = {})
         options[:latestby] ? QuickRow : EveryRow
       end
-
-      def resolve_ignore_selectors(options)
-        resolve_versions_and_add_ids(options.fetch(:ignore_selectors, []), :ignored)
-      end
-
-      # To make it easy for pf to override
-      def base_model_for_integrations
-        QuickRow
-      end
-
-      # Find the version number for selectors with the latest and/or tag specified
-      def resolve_versions_and_add_ids(unresolved_selectors, selector_type, resolved_ignore_selectors = [])
-        names = unresolved_selectors.collect(&:pacticipant_name)
-        pacticipants = PactBroker::Domain::Pacticipant.where(name: names).to_a.group_by(&:name).transform_values(&:first)
-        unresolved_selectors.collect do | unresolved_selector |
-          pacticipant = pacticipants[unresolved_selector.pacticipant_name]
-          if pacticipant
-            versions = find_versions_for_selector(unresolved_selector)
-            build_resolved_selectors(pacticipant, versions, unresolved_selector, selector_type, resolved_ignore_selectors)
-          else
-            selector_for_non_existing_pacticipant(unresolved_selector, selector_type)
-          end
-        end.flatten
-      end
-
-      def find_versions_for_selector(selector)
-        # For selectors that just set the pacticipant name, there's no need to resolve the version -
-        # only the pacticipant ID will be used in the query
-        return nil if selector.all_for_pacticipant?
-        versions = version_repository.find_versions_for_selector(selector)
-
-        if selector.latest
-          [versions.first]
-        else
-          versions.empty? ? [nil] : versions
-        end
-      end
-
-      # When a single selector specifies multiple versions (eg. "all prod pacts"), this expands
-      # the single selector into one selector for each version.
-      def build_resolved_selectors(pacticipant, versions, unresolved_selector, selector_type, resolved_ignore_selectors)
-        if versions
-          one_of_many = versions.compact.size > 1
-          versions.collect do | version |
-            if version
-              selector_for_found_version(pacticipant, version, unresolved_selector, selector_type, one_of_many, resolved_ignore_selectors)
-            else
-              selector_for_non_existing_version(pacticipant, unresolved_selector, selector_type, resolved_ignore_selectors)
-            end
-          end
-        else
-          selector_for_all_versions_of_a_pacticipant(pacticipant, unresolved_selector, selector_type, resolved_ignore_selectors)
-        end
-      end
-
-      # The user has specified --to TAG or --to-environment ENVIRONMENT in the CLI
-      # (or nothing, which to defaults to latest=true - "with the latest version of the other integrated applications")
-      def infer_selectors_for_integrations?(options)
-        options[:latest] || options[:tag] || options[:branch] || options[:environment_name] || options[:main_branch]
-      end
-
-      # When only one selector is specified, (eg. checking to see if Foo version 2 can be deployed to prod),
-      # need to look up all integrated pacticipants, and determine their relevant (eg. latest prod) versions
-      def inferred_selectors(resolved_specified_selectors, resolved_ignore_selectors, integrations, options)
-        all_pacticipant_names = integrations.collect(&:pacticipant_names).flatten.uniq
-        specified_names = resolved_specified_selectors.collect{ |s| s[:pacticipant_name] }
-        inferred_pacticipant_names = all_pacticipant_names - specified_names
-        build_inferred_selectors(inferred_pacticipant_names, resolved_ignore_selectors, options)
-      end
-
-      def build_inferred_selectors(inferred_pacticipant_names, resolved_ignore_selectors, options)
-        selectors = inferred_pacticipant_names.collect do | pacticipant_name |
-          selector = UnresolvedSelector.new(pacticipant_name: pacticipant_name)
-          selector.tag = options[:tag] if options[:tag]
-          selector.branch = options[:branch] if options[:branch]
-          selector.main_branch = options[:main_branch] if options[:main_branch]
-          selector.latest = options[:latest] if options[:latest]
-          selector.environment_name = options[:environment_name] if options[:environment_name]
-          selector
-        end
-        resolve_versions_and_add_ids(selectors, :inferred, resolved_ignore_selectors)
-      end
-
-      def selector_for_non_existing_version(pacticipant, unresolved_selector, selector_type, resolved_ignore_selectors)
-        ignore = resolved_ignore_selectors.any? do | s |
-          s.pacticipant_id == pacticipant.id && s.only_pacticipant_name_specified?
-        end
-        ResolvedSelector.for_pacticipant_and_non_existing_version(pacticipant, unresolved_selector, selector_type, ignore)
-      end
-
-      # rubocop: disable Metrics/ParameterLists
-      def selector_for_found_version(pacticipant, version, unresolved_selector, selector_type, one_of_many, resolved_ignore_selectors)
-        ignore = resolved_ignore_selectors.any? do | s |
-          s.pacticipant_id == pacticipant.id && (s.only_pacticipant_name_specified? || s.pacticipant_version_id == version.id)
-        end
-        ResolvedSelector.for_pacticipant_and_version(pacticipant, version, unresolved_selector, selector_type, ignore, one_of_many)
-      end
-      # rubocop: enable Metrics/ParameterLists
-
-      def selector_for_all_versions_of_a_pacticipant(pacticipant, unresolved_selector, selector_type, resolved_ignore_selectors)
-        # Doesn't make sense to ignore this, as you can't have a can-i-deploy query
-        # for "all versions of a pacticipant". But whatever.
-        ignore = resolved_ignore_selectors.any? do | s |
-          s.pacticipant_id == pacticipant.id && s.only_pacticipant_name_specified?
-        end
-        ResolvedSelector.for_pacticipant(pacticipant, unresolved_selector, selector_type, ignore)
-      end
-
-      # only relevant for ignore selectors, validation stops this happening for the normal
-      # selectors
-      def selector_for_non_existing_pacticipant(unresolved_selector, selector_type)
-        ResolvedSelector.for_non_existing_pacticipant(unresolved_selector, selector_type, false)
-      end
-
-      def split_rows_into_considered_and_ignored(rows, resolved_ignore_selectors)
-        if resolved_ignore_selectors.any?
-          considered, ignored = [], []
-          rows.each do | row |
-            if ignore_row?(resolved_ignore_selectors, row)
-              ignored << row
-            else
-              considered << row
-            end
-          end
-          return considered, ignored
-        else
-          return rows, []
-        end
-      end
-
-      def ignore_row?(resolved_ignore_selectors, row)
-        resolved_ignore_selectors.any? do | s |
-          s.pacticipant_id == row.consumer_id  && (s.only_pacticipant_name_specified? || s.pacticipant_version_id == row.consumer_version_id) ||
-            s.pacticipant_id == row.provider_id  && (s.only_pacticipant_name_specified? || s.pacticipant_version_id == row.provider_version_id)
-        end
-      end
     end
   end
 end
diff --git a/lib/pact_broker/matrix/resolved_selector.rb b/lib/pact_broker/matrix/resolved_selector.rb
index c6d11d05d..63846451e 100644
--- a/lib/pact_broker/matrix/resolved_selector.rb
+++ b/lib/pact_broker/matrix/resolved_selector.rb
@@ -4,7 +4,9 @@
 # This is created from either specified or inferred data, based on the user's query
 # eg.
 # can-i-deploy --pacticipant Foo --version 1 (this is a specified selector)
-#              --to prod (this is used to create inferred selectors)
+#              --to prod (this is used to create inferred selectors, one for each integrated pacticipant in that environment)
+# When an UnresolvedSelector specifies multiple application versions (eg. { tag: "prod" }) then a ResolvedSelector
+# is created for every Version object found for the original selector.
 
 module PactBroker
   module Matrix
diff --git a/lib/pact_broker/matrix/resolved_selector_builder.rb b/lib/pact_broker/matrix/resolved_selector_builder.rb
new file mode 100644
index 000000000..5b48e4770
--- /dev/null
+++ b/lib/pact_broker/matrix/resolved_selector_builder.rb
@@ -0,0 +1,84 @@
+# Builds a PactBroker::Matrix::UnresolvedSelector based on the given
+# UnresolvedSelector, selector type, Pacticipant and Version objects,
+# using the selector_ignorer to work out if the built ResolvedSelector
+# should be marked as ignored or not.
+
+module PactBroker
+  module Matrix
+    class ResolvedSelectorBuilder
+
+      attr_accessor :pacticipant, :versions
+
+      # @param [PactBroker::Matrix::UnresolvedSelector] unresolved_selector
+      # @param [Symbol] selector_type :specified or :inferred
+      # @param [PactBroker::Matrix::Ignorer] selector_ignorer
+      def initialize(unresolved_selector, selector_type, selector_ignorer)
+        @unresolved_selector = unresolved_selector
+        @selector_type = selector_type
+        @selector_ignorer = selector_ignorer
+      end
+
+      def build
+        if pacticipant && versions
+          build_resolved_selectors_for_versions(pacticipant, versions, unresolved_selector, selector_type)
+        elsif pacticipant
+          selector_for_all_versions_of_a_pacticipant(pacticipant, unresolved_selector, selector_type)
+        else
+          build_selector_for_non_existing_pacticipant(unresolved_selector, selector_type)
+        end
+      end
+
+      private
+
+      attr_reader :unresolved_selector, :selector_type, :selector_ignorer
+
+      # When a single selector specifies multiple versions (eg. "all prod pacts"), this expands
+      # the single selector into one selector for each version.
+      def build_resolved_selectors_for_versions(pacticipant, versions, unresolved_selector, selector_type)
+        one_of_many = versions.compact.size > 1
+        versions.collect do | version |
+          if version
+            selector_for_found_version(pacticipant, version, unresolved_selector, selector_type, one_of_many)
+          else
+            selector_for_non_existing_version(pacticipant, unresolved_selector, selector_type)
+          end
+        end
+      end
+
+      def selector_for_non_existing_version(pacticipant, unresolved_selector, selector_type)
+        ignore = selector_ignorer.ignore_pacticipant?(pacticipant)
+        ResolvedSelector.for_pacticipant_and_non_existing_version(pacticipant, unresolved_selector, selector_type, ignore)
+      end
+
+      # rubocop: disable Metrics/ParameterLists
+      def selector_for_found_version(pacticipant, version, unresolved_selector, selector_type, one_of_many)
+        ResolvedSelector.for_pacticipant_and_version(
+          pacticipant,
+          version,
+          unresolved_selector,
+          selector_type,
+          selector_ignorer.ignore_pacticipant_version?(pacticipant, version),
+          one_of_many
+        )
+      end
+      # rubocop: enable Metrics/ParameterLists
+
+      def selector_for_all_versions_of_a_pacticipant(pacticipant, unresolved_selector, selector_type)
+        # Doesn't make sense to ignore this, as you can't have a can-i-deploy query
+        # for "all versions of a pacticipant". But whatever.
+        ResolvedSelector.for_pacticipant(
+          pacticipant,
+          unresolved_selector,
+          selector_type,
+          selector_ignorer.ignore_pacticipant?(pacticipant)
+        )
+      end
+
+      # only relevant for ignore selectors, validation stops this happening for the normal
+      # selectors
+      def build_selector_for_non_existing_pacticipant(unresolved_selector, selector_type)
+        ResolvedSelector.for_non_existing_pacticipant(unresolved_selector, selector_type, false)
+      end
+    end
+  end
+end
\ No newline at end of file
diff --git a/lib/pact_broker/matrix/resolved_selectors_builder.rb b/lib/pact_broker/matrix/resolved_selectors_builder.rb
new file mode 100644
index 000000000..53f58e038
--- /dev/null
+++ b/lib/pact_broker/matrix/resolved_selectors_builder.rb
@@ -0,0 +1,39 @@
+require "pact_broker/matrix/selector_resolver"
+
+# Builds the array of ResolvedSelector objects using the
+# ignore selectors, the specified selectors, and the inferred integrations.
+
+module PactBroker
+  module Matrix
+    class ResolvedSelectorsBuilder
+      attr_reader :ignore_selectors, :specified_selectors, :inferred_selectors
+
+      def initialize
+        @inferred_selectors = []
+      end
+
+      # @param [Array<PactBroker::Matrix::UnresolvedSelector>]
+      # @param [Hash] options
+      def resolve_selectors(unresolved_specified_selectors, unresolved_ignore_selectors)
+        # must do this first because we need the ignore selectors to resolve the specified selectors
+        @ignore_selectors = SelectorResolver.resolved_ignore_selectors(unresolved_ignore_selectors)
+        @specified_selectors = SelectorResolver.resolve_specified_selectors(unresolved_specified_selectors, ignore_selectors)
+      end
+
+      # Use the given Integrations to work out what the selectors are for the versions that the versions for the specified
+      # selectors should be deployed with.
+      # eg. For `can-i-deploy --pacticipant Foo --version adfjkwejr --to-environment prod`, work out the selectors for the integrated application
+      # versions in the prod environment.
+      # @param [Array<PactBroker::Matrix::Integration>] integrations
+      def resolve_inferred_selectors(integrations, options)
+        @inferred_selectors = SelectorResolver.resolve_inferred_selectors(specified_selectors, ignore_selectors, integrations, options)
+      end
+
+      # All the resolved selectors to be used in the matrix query, specified and inferred (if any)
+      # @return [Array<PactBroker::Matrix::ResolvedSelector>]
+      def all_selectors
+        specified_selectors + inferred_selectors
+      end
+    end
+  end
+end
diff --git a/lib/pact_broker/matrix/row_ignorer.rb b/lib/pact_broker/matrix/row_ignorer.rb
new file mode 100644
index 000000000..c763deaa6
--- /dev/null
+++ b/lib/pact_broker/matrix/row_ignorer.rb
@@ -0,0 +1,36 @@
+module PactBroker
+  module Matrix
+    class RowIgnorer
+
+      class << self
+        # Splits the matrix rows into considered rows and ignored rows, based on the
+        # ignore selectors specified by the user in the can-i-deploy command (eg. --ignore SomeProviderThatIsNotReadyYet).
+        # @param [Array<QuickRow, EveryRow>] rows
+        # @param [<PactBroker::Matrix::ResolvedSelector>] resolved_ignore_selectors
+        # @return [Array<QuickRow, EveryRow>] considered_rows, [Array<QuickRow, EveryRow>] ignored_rows
+        def split_rows_into_considered_and_ignored(rows, resolved_ignore_selectors)
+          if resolved_ignore_selectors.any?
+            considered, ignored = [], []
+            rows.each do | row |
+              if ignore_row?(resolved_ignore_selectors, row)
+                ignored << row
+              else
+                considered << row
+              end
+            end
+            return considered, ignored
+          else
+            return rows, []
+          end
+        end
+
+        def ignore_row?(resolved_ignore_selectors, row)
+          resolved_ignore_selectors.any? do | s |
+            s.pacticipant_id == row.consumer_id  && (s.only_pacticipant_name_specified? || s.pacticipant_version_id == row.consumer_version_id) ||
+              s.pacticipant_id == row.provider_id  && (s.only_pacticipant_name_specified? || s.pacticipant_version_id == row.provider_version_id)
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/lib/pact_broker/matrix/selector_ignorer.rb b/lib/pact_broker/matrix/selector_ignorer.rb
new file mode 100644
index 000000000..e438ced77
--- /dev/null
+++ b/lib/pact_broker/matrix/selector_ignorer.rb
@@ -0,0 +1,59 @@
+# This class determines whether or not a resolved selector (both specified and inferred)
+# that is about to be created should be marked as "ignored".
+# It uses the ignore selectors are provided in the can-i-deploy CLI command like so:
+#   can-i-deploy --pacticipant Foo --version 234243 --to-environment prod --ignore SomeProviderThatIsNotReadyYet [--version SomeOptionalVersion]
+# The ignored flag on the ResolvedSelector is used to determine whether or not a failing/missing row
+# in the can-i-deploy matrix should be ignored.
+# This allows can-i-deploy to pass successfully when a dependency is known to be not ready,
+# but the developer wants to deploy the application anyway.
+
+# The only reason why we need to resolve the ignore selectors is that we check in PactBroker::Matrix::DeploymentStatusSummary
+# whether or not the pacticipant or version they specify actually exist.
+# We could actually have performed the ignore checks just using the name and version number.
+
+module PactBroker
+  module Matrix
+    class SelectorIgnorer
+
+      # @param [Array<PactBroker::Matrix::UnresolvedSelector>] resolved_ignore_selectors
+      def initialize(resolved_ignore_selectors)
+        @resolved_ignore_selectors = resolved_ignore_selectors
+      end
+
+      # Whether the pacticipant should be ignored if the verification results are missing/failed.
+      # @param [PactBroker::Domain::Pacticipant] pacticipant
+      # @return [Boolean]
+      def ignore_pacticipant?(pacticipant)
+        resolved_ignore_selectors.any? do | s |
+          s.pacticipant_id == pacticipant.id && s.only_pacticipant_name_specified?
+        end
+      end
+
+      # Whether the pacticipant version should be ignored if the verification results are missing/failed.
+      # @param [PactBroker::Domain::Pacticipant] pacticipant
+      # @param [PactBroker::Domain::Version] version
+      # @return [Boolean]
+      def ignore_pacticipant_version?(pacticipant, version)
+        resolved_ignore_selectors.any? do | s |
+          s.pacticipant_id == pacticipant.id && (s.only_pacticipant_name_specified? || s.pacticipant_version_id == version.id)
+        end
+      end
+
+      private
+
+      attr_reader :resolved_ignore_selectors
+    end
+
+    # Used when resolving the ignore selecors in the first place - the process for resolving normal selectors
+    # and ignore selectors is almost the same, but it makes no sense to ignore an ignore selector.
+    class NilSelectorIgnorer
+      def ignore_pacticipant?(*)
+        false
+      end
+
+      def ignore_pacticipant_version?(*)
+        false
+      end
+    end
+  end
+end
diff --git a/lib/pact_broker/matrix/selector_resolver.rb b/lib/pact_broker/matrix/selector_resolver.rb
new file mode 100644
index 000000000..63c15943c
--- /dev/null
+++ b/lib/pact_broker/matrix/selector_resolver.rb
@@ -0,0 +1,130 @@
+require "pact_broker/repositories"
+require "pact_broker/matrix/resolved_selector"
+require "pact_broker/matrix/resolved_selector_builder"
+require "pact_broker/matrix/selector_ignorer"
+
+
+# Take the selectors and options provided by the user (eg. [{ pacticipant_name: "Foo", pacticipant_version_number: "1" }], { to_environment: "prod" })
+# that use pacticipant/version/branch/environment names,
+# and look up the IDs of all the objects, and return them as ResolvedSelector objects.
+# For unresolved selectors that specify a collection of versions (eg. { branch: "main" }) a ResolvedSelector
+# will be returned for every pacticipant version found. This will eventually be used in the can-i-deploy
+# logic in PactBroker::Matrix::DeploymentStatusSummary to work out if there are any missing verifications.
+
+module PactBroker
+  module Matrix
+    class SelectorResolver
+      class << self
+        include PactBroker::Repositories
+
+        # Resolve any ignore selectors used in the can-i-deploy command e.g `--ignore SomeProviderThatIsNotReadyYet`
+        # @param [Array<PactBroker::Matrix::UnresolvedSelector>] unresolved_ignore_selectors
+        # @return [Array<PactBroker::Matrix::ResolvedSelector>]
+        def resolved_ignore_selectors(unresolved_ignore_selectors)
+          # When resolving the ignore_selectors, use the NilSelectorIgnorer because it doesn't make sense to ignore
+          # the ignore selectors.
+          resolve_versions_and_add_ids(unresolved_ignore_selectors, :ignored, NilSelectorIgnorer.new)
+        end
+
+        # Resolve the selectors that were specified in the can-i-deploy command eg. `--pacticipant Foo --version 43434`
+        # There may be one or multiple.
+        # @param [Array<PactBroker::Matrix::UnresolvedSelector>] unresolved_specified_selectors
+        # @param [Array<PactBroker::Matrix::ResolvedSelector>] resolved_ignore_selectors previously resolved selectors for the versions to ignore
+        # @return [Array<PactBroker::Matrix::ResolvedSelector>]
+        def resolve_specified_selectors(unresolved_specified_selectors, resolved_ignore_selectors)
+          resolve_versions_and_add_ids(unresolved_specified_selectors, :specified, SelectorIgnorer.new(resolved_ignore_selectors))
+        end
+
+        # When the can-i-deploy command uses any of the `--to` options (eg. `--to-environment ENV` or `--to TAG`)
+        # we need to create the inferred selectors for the pacticipant versions in that environment/with that tag/branch.
+        # eg. if A -> B, and the CLI command is `can-i-deploy --pacticipant A --version 3434 --to-environment prod`,
+        # then we need to make the inferred selector for pacticipant B with the version that is in prod.
+        # @param [Array<PactBroker::Matrix::ResolvedSelector>] resolved_specified_selectors
+        # @param [Array<PactBroker::Matrix::ResolvedSelector>] resolved_ignore_selectors
+        # @param [Array<PactBroker::Matrix::Integration>] integrations
+        # @param [Hash] options
+        # @return [Array<PactBroker::Matrix::ResolvedSelector>]
+        def resolve_inferred_selectors(resolved_specified_selectors, resolved_ignore_selectors, integrations, options)
+          all_pacticipant_names = integrations.collect(&:pacticipant_names).flatten.uniq
+          specified_names = resolved_specified_selectors.collect{ |s| s[:pacticipant_name] }
+          inferred_pacticipant_names = all_pacticipant_names - specified_names
+          unresolved_selectors = build_unresolved_selectors_for_inferred_pacticipants(inferred_pacticipant_names, options)
+          resolve_versions_and_add_ids(unresolved_selectors, :inferred, SelectorIgnorer.new(resolved_ignore_selectors))
+        end
+
+        # Find the IDs of every pacticipant and version in the UnresolvedSelectors, and return them as ResolvedSelectors,
+        # expanding selectors for multiple versions.
+        # This gets called first for the ignore selectors, then the specified selectors, and then the inferred selectors.
+        # When it gets called for the first time for the ignore selectors, they will be passed in as the unresolved_selectors, and the resolved_ignore_selectors
+        # will be empty.
+        # The next times it is called with the specified selectors and the inferred selectors, the previously resolved ignore selectors will be passed in
+        # as resolved_ignore_selectors so we can work out which of those selectors needs to be ignored.
+        #
+        # @param [Array<PactBroker::Matrix::UnresolvedSelector>] unresolved_selectors
+        # @param [Symbol] selector_type which may be :specified or :inferred
+        # @param [SelectorIgnorer] selector_ignorer
+        # @return [Array<PactBroker::Matrix::ResolvedSelector>]
+        def resolve_versions_and_add_ids(unresolved_selectors, selector_type, selector_ignorer)
+          pacticipants_hash = find_pacticipants_for_selectors(unresolved_selectors)
+          unresolved_selectors.collect do | unresolved_selector |
+            build_selectors(pacticipants_hash, unresolved_selector, selector_type, selector_ignorer)
+          end.flatten
+        end
+
+        private :resolve_versions_and_add_ids
+
+        # Return a Hash of the pacticipant names used in the selectors, where the key is the name and the value is the pacticipant
+        # @return [Hash<String, PactBroker::Domain::Pacticipant>]
+        def find_pacticipants_for_selectors(unresolved_selectors)
+          names = unresolved_selectors.collect(&:pacticipant_name)
+          PactBroker::Domain::Pacticipant.where(name: names).all.group_by(&:name).transform_values(&:first)
+        end
+
+        private :find_pacticipants_for_selectors
+
+        def build_selectors(pacticipants_hash, unresolved_selector, selector_type, selector_ignorer)
+          selector_builder = ResolvedSelectorBuilder.new(unresolved_selector, selector_type, selector_ignorer)
+          selector_builder.pacticipant = pacticipants_hash[unresolved_selector.pacticipant_name]
+          if selector_builder.pacticipant
+            versions = find_versions_for_selector(unresolved_selector)
+            selector_builder.versions = versions
+          end
+          selector_builder.build
+        end
+
+        # Find the pacticipant versions for the unresolved selector.
+        # @param [PactBroker::Matrix::UnresolvedSelector] unresolved_selector
+        def find_versions_for_selector(unresolved_selector)
+          # For selectors that just set the pacticipant name, there's no need to resolve the version -
+          # only the pacticipant ID will be used in the query
+          return nil if unresolved_selector.all_for_pacticipant?
+          versions = version_repository.find_versions_for_selector(unresolved_selector)
+
+          if unresolved_selector.latest
+            [versions.first]
+          else
+            versions.empty? ? [nil] : versions
+          end
+        end
+
+        private :find_versions_for_selector
+
+        # Build an unresolved selector for the integrations that we have inferred for the target environment/branch/tag
+        # @param [Array<String>] inferred_pacticipant_names the names of the pacticipants that we have determined to be integrated with the versions for the specified selectors
+        def build_unresolved_selectors_for_inferred_pacticipants(inferred_pacticipant_names, options)
+          inferred_pacticipant_names.collect do | pacticipant_name |
+            selector = UnresolvedSelector.new(pacticipant_name: pacticipant_name)
+            selector.tag = options[:tag] if options[:tag]
+            selector.branch = options[:branch] if options[:branch]
+            selector.main_branch = options[:main_branch] if options[:main_branch]
+            selector.latest = options[:latest] if options[:latest]
+            selector.environment_name = options[:environment_name] if options[:environment_name]
+            selector
+          end
+        end
+
+        private :build_unresolved_selectors_for_inferred_pacticipants
+      end
+    end
+  end
+end
diff --git a/lib/pact_broker/repositories.rb b/lib/pact_broker/repositories.rb
index 3bec3c87b..8c1c0729a 100644
--- a/lib/pact_broker/repositories.rb
+++ b/lib/pact_broker/repositories.rb
@@ -55,6 +55,10 @@ def integration_repository
       get_repository(:integration_repository)
     end
 
+    def matrix_integration_repository
+      get_repository(:matrix_integration_repository)
+    end
+
     # rubocop: disable Metrics/MethodLength
     def register_default_repositories
       register_repository(:pacticipant_repository) do
@@ -106,6 +110,12 @@ def register_default_repositories
         PactBroker::Integrations::Repository.new
       end
 
+      register_repository(:matrix_integration_repository) do
+        require "pact_broker/matrix/integrations_repository"
+        require "pact_broker/matrix/quick_row"
+        PactBroker::Matrix::IntegrationsRepository.new(PactBroker::Matrix::QuickRow)
+      end
+
       # rubocop: enable Metrics/MethodLength
     end
   end

From 51157e78e15f1a4b2dc2d658bf9a54822954b927 Mon Sep 17 00:00:00 2001
From: Beth Skurrie <beth.skurrie@smartbear.com>
Date: Sat, 19 Aug 2023 15:59:42 +1000
Subject: [PATCH 3/6] refactor: remove unused matrix code

---
 lib/pact_broker/matrix/every_row.rb           |  18 +--
 .../matrix/integrations_repository.rb         |   4 +-
 lib/pact_broker/matrix/query_builder.rb       |  41 +-----
 lib/pact_broker/matrix/query_ids.rb           |  10 +-
 lib/pact_broker/matrix/quick_row.rb           | 129 +++++-------------
 spec/lib/pact_broker/matrix/every_row_spec.rb |  29 ----
 6 files changed, 47 insertions(+), 184 deletions(-)

diff --git a/lib/pact_broker/matrix/every_row.rb b/lib/pact_broker/matrix/every_row.rb
index b61d621de..332138358 100644
--- a/lib/pact_broker/matrix/every_row.rb
+++ b/lib/pact_broker/matrix/every_row.rb
@@ -14,7 +14,7 @@ def select_distinct_pact_version_id
             select_pact_version_id.distinct
           end
 
-          def join_versions(versions_dataset)
+          def join_versions_dataset(versions_dataset)
             join(versions_dataset, { Sequel[:verifications][:provider_version_id] => Sequel[:versions][:id] }, table_alias: :versions)
           end
         end
@@ -58,10 +58,7 @@ def verification_model
           EveryRow::Verification
         end
 
-        def inner_join_verifications
-          join(:verifications, P_V_JOIN, { table_alias: :v } )
-        end
-
+        # TODO refactor this to get rid of QueryIds
         def inner_join_verifications_matching_one_selector_provider_or_provider_version(query_ids)
           verifications = db[:verifications]
             .select(*JOINED_VERIFICATION_COLUMNS)
@@ -71,17 +68,6 @@ def inner_join_verifications_matching_one_selector_provider_or_provider_version(
 
           join(verifications, P_V_JOIN, { table_alias: :v } )
         end
-
-        def verifications_for(query_ids)
-          db[:verifications]
-            .select(*JOINED_VERIFICATION_COLUMNS)
-            .where {
-              Sequel.&(
-                QueryBuilder.consumer_in_pacticipant_ids(query_ids),
-                QueryBuilder.provider_or_provider_version_matches(query_ids)
-              )
-            }
-        end
       end
     end
   end
diff --git a/lib/pact_broker/matrix/integrations_repository.rb b/lib/pact_broker/matrix/integrations_repository.rb
index fa659e16c..325d8f195 100644
--- a/lib/pact_broker/matrix/integrations_repository.rb
+++ b/lib/pact_broker/matrix/integrations_repository.rb
@@ -64,7 +64,7 @@ def integrations_where_specified_selector_is_consumer(resolved_specified_selecto
         resolved_specified_selectors.flat_map do | selector |
           # Could optimise this to all in one query, but it's a small gain
           base_model_for_integrations
-            .distinct_integrations_for_selector_as_consumer(selector)
+            .integrations_for_selector_as_consumer(selector)
             .all
             .collect do | integration |
               Integration.from_hash(
@@ -78,7 +78,7 @@ def integrations_where_specified_selector_is_consumer(resolved_specified_selecto
         end
       end
 
-      # Why does the consumer equivalent of this method use the QuickRow distinct_integrations_for_selector_as_consumer
+      # Why does the consumer equivalent of this method use the QuickRow integrations_for_selector_as_consumer
       # while this method uses the Integration?
       # Find all the consumers for the providers specified in the query. Does not take into consideration the provider version (not sure why).
       # @param [Array<PactBroker::Matrix::ResolvedSelector>] the resolved selectors that were specified in the query
diff --git a/lib/pact_broker/matrix/query_builder.rb b/lib/pact_broker/matrix/query_builder.rb
index 8620eca4b..d5cd878f9 100644
--- a/lib/pact_broker/matrix/query_builder.rb
+++ b/lib/pact_broker/matrix/query_builder.rb
@@ -7,26 +7,10 @@ def self.provider_or_provider_version_matches(query_ids, provider_version_qualif
 
       def self.provider_matches(query_ids, qualifier)
         {
-          qualify(qualifier, :provider_id) => query_ids.pacticipant_ids,
+          qualify(qualifier, :provider_id) => query_ids.pacticipant_ids
         }
       end
 
-      def self.provider_or_provider_version_matches_or_pact_unverified(query_ids, provider_version_qualifier = nil, provider_qualifier = nil)
-        ors = provider_or_provider_version_criteria(query_ids, provider_version_qualifier, provider_qualifier)
-
-        # If we have specified any versions, then we need to add an
-        # "OR (provider matches these IDs and provider version is null)"
-        # so that we get a line with blank verification details.
-        if query_ids.pacticipant_version_ids.any?
-          ors << {
-            qualify(provider_qualifier, :provider_id) => query_ids.all_pacticipant_ids,
-            qualify(provider_version_qualifier, :provider_version_id) => nil
-          }
-        end
-
-        Sequel.|(*ors)
-      end
-
       def self.provider_or_provider_version_criteria(query_ids, provider_version_qualifier = nil, provider_qualifier = nil)
         ors = []
         ors << { qualify(provider_version_qualifier, :provider_version_id) => query_ids.pacticipant_version_ids } if query_ids.pacticipant_version_ids.any?
@@ -34,8 +18,10 @@ def self.provider_or_provider_version_criteria(query_ids, provider_version_quali
         ors
       end
 
-      def self.consumer_in_pacticipant_ids(query_ids)
-        { consumer_id: query_ids.all_pacticipant_ids }
+      def self.consumer_matches(query_ids, qualifier)
+        {
+          qualify(qualifier, :consumer_id) => query_ids.pacticipant_ids
+        }
       end
 
       def self.consumer_or_consumer_version_matches(query_ids, qualifier)
@@ -61,23 +47,6 @@ def self.either_consumer_or_provider_was_specified_in_query(query_ids, qualifier
         })
       end
 
-      # QueryIds is built from a single selector, so there is only one pacticipant_id or pacticipant_version_id
-      def self.consumer_or_consumer_version_or_provider_or_provider_or_provider_version_match(query_ids, pacts_qualifier = :p, verifications_qualifier = :v)
-        ors = if query_ids.pacticipant_version_id
-                [
-                  { Sequel[pacts_qualifier][:consumer_version_id] => query_ids.pacticipant_version_id },
-                  { Sequel[verifications_qualifier][:provider_version_id] => query_ids.pacticipant_version_id }
-                ]
-              else
-                [
-                  { Sequel[pacts_qualifier][:consumer_id] => query_ids.pacticipant_id },
-                  { Sequel[pacts_qualifier][:provider_id] => query_ids.pacticipant_id }
-                ]
-              end
-
-        Sequel.|(*ors)
-      end
-
       def self.qualify(qualifier, column)
         if qualifier
           Sequel[qualifier][column]
diff --git a/lib/pact_broker/matrix/query_ids.rb b/lib/pact_broker/matrix/query_ids.rb
index 1286f50c5..f12522d55 100644
--- a/lib/pact_broker/matrix/query_ids.rb
+++ b/lib/pact_broker/matrix/query_ids.rb
@@ -1,3 +1,5 @@
+# This is left over from the old way of querying the matrix, and should eventually be deleted entirely
+
 module PactBroker
   module Matrix
     class QueryIds
@@ -27,14 +29,6 @@ def self.from_selectors(selectors)
       def self.collect_ids(hashes, key)
         hashes.collect{ |s| s[key] }.flatten.compact
       end
-
-      def pacticipant_id
-        pacticipant_ids.first
-      end
-
-      def pacticipant_version_id
-        pacticipant_version_ids.first
-      end
     end
   end
 end
diff --git a/lib/pact_broker/matrix/quick_row.rb b/lib/pact_broker/matrix/quick_row.rb
index 16afcad99..5678eaaf1 100644
--- a/lib/pact_broker/matrix/quick_row.rb
+++ b/lib/pact_broker/matrix/quick_row.rb
@@ -17,12 +17,6 @@
 # It is called the QuickRow because the initial implementation was called the Row, and this is an optimised
 # version. It needs to be renamed back to Row now that the old Row class has been deleted.
 
-# The difference between `join_verifications_for` and `join_verifications` is that
-# the left outer join is done on a pre-filtered dataset in `join_verifications_for`,
-# so that we get a row with null verification fields for a pact that has been verified
-# by a *different* version of the provider we're interested in,
-# rather than being excluded from the dataset altogether.
-
 module PactBroker
   module Matrix
     # TODO rename this to just Row
@@ -92,7 +86,7 @@ def select_distinct_pact_version_id
             select_pact_version_id.distinct
           end
 
-          def join_versions(versions_dataset)
+          def join_versions_dataset(versions_dataset)
             join(versions_dataset, { Sequel[:v][:provider_version_id] => Sequel[:versions][:id] }, table_alias: :versions)
           end
         end
@@ -111,7 +105,8 @@ def select_distinct_pacticipant_and_pact_version_ids
         end
 
         # @param [PactBroker::Matrix::ResolvedSelector] selector
-        def distinct_integrations_for_selector_as_consumer(selector)
+        # @return [Sequel::Dataset] for rows with consumer_id, consumer_name, provider_id and provider_name
+        def integrations_for_selector_as_consumer(selector)
           select(:consumer_id, :provider_id)
             .distinct
             .where({ consumer_id: selector.pacticipant_id, consumer_version_id: selector.pacticipant_version_id }.compact)
@@ -123,24 +118,9 @@ def distinct_integrations_for_selector_as_consumer(selector)
 
         # Find all the integrations (consumer/provider pairs) that involve the given selectors.
         # @param [Array<PactBroker::Matrix::ResolvedSelector>] selectors
+        # @return [Sequel::Dataset] for rows with consumer_id, consumer_name, provider_id and provider_name
         def distinct_integrations(selectors)
-          query = if selectors.size == 1
-                    pacticipant_ids_matching_one_selector_optimised(selectors)
-                  else
-                    if selectors.all?(&:only_pacticipant_name_specified?)
-                      matching_multiple_selectors_without_joining_verifications(selectors)
-                        .select_pacticipant_ids
-                        .distinct
-                    else
-                      matching_multiple_selectors_joining_verifications(
-                          selectors,
-                          pact_columns: :select_distinct_pacticipant_and_pact_version_ids,
-                          verifications_columns: :select_distinct_pact_version_id
-                        )
-                        .select_pacticipant_ids
-                        .distinct
-                    end
-                  end
+          query = build_query_for_pacticipant_ids(selectors)
 
           query.from_self(alias: :pacticipant_ids)
             .select(
@@ -195,8 +175,26 @@ def default_scope
 
         # PRIVATE METHODS
 
+        # @param [Array<PactBroker::Matrix::ResolvedSelector>] selectors
+        def build_query_for_pacticipant_ids(selectors)
+          if selectors.all?(&:only_pacticipant_name_specified?)
+            matching_selectors_for_pacticipants_only(selectors)
+              .select_pacticipant_ids
+              .distinct
+          else
+            matching_multiple_selectors_joining_verifications(
+                selectors,
+                pact_columns: :select_distinct_pacticipant_and_pact_version_ids,
+                verifications_columns: :select_distinct_pact_version_id
+              )
+              .select_pacticipant_ids
+              .distinct
+          end
+        end
+
         # When we have one selector, we need to join ALL the verifications to find out
         # what integrations exist
+        # TODO refactor this to get rid of QueryIds
         def matching_one_selector(selectors)
           query_ids = QueryIds.from_selectors(selectors)
           rows_where_selector_matches_consumer_cols = join_verifications
@@ -208,29 +206,6 @@ def matching_one_selector(selectors)
           rows_where_selector_matches_consumer_cols.union(rows_where_selector_matches_provider_cols)
         end
 
-        def pacticipant_ids_matching_one_selector_optimised(selectors)
-          query_ids = QueryIds.from_selectors(selectors)
-          distinct_pacticipant_ids_where_consumer_or_consumer_version_matches(query_ids)
-            .union(distinct_pacticipant_ids_where_provider_or_provider_version_matches(query_ids), all: true)
-        end
-
-        def distinct_pacticipant_ids_where_consumer_or_consumer_version_matches(query_ids)
-          select_pacticipant_ids
-            .distinct
-            .where {
-              QueryBuilder.consumer_or_consumer_version_matches(query_ids, :p)
-            }
-        end
-
-        def distinct_pacticipant_ids_where_provider_or_provider_version_matches(query_ids)
-          select_pacticipant_ids
-            .distinct
-            .inner_join_verifications
-            .where {
-              QueryBuilder.provider_or_provider_version_matches(query_ids, :v, :v)
-            }
-        end
-
         # When the user has specified multiple selectors, we only want to join the verifications for
         # the specified selectors. This is because of the behaviour of the left outer join.
         # Imagine a pact has been verified by a provider version that was NOT specified in the selectors.
@@ -241,6 +216,9 @@ def distinct_pacticipant_ids_where_provider_or_provider_version_matches(query_id
         # and THEN join them to the pacts, so that we get a row for the pact with null provider version
         # and verification fields.
         # @param [Array<PactBroker::Matrix::ResolvedSelector>] selectors
+        # @param [Symbol] pact_columns the method to call on the QuickRow/EveryRow model to get the right columns required for the particular query
+        # @param [Symbol] verifications_columns the method to call on the QuickRow::Verifications/EveryRow::Verifications model to get the right columns required for the particular query
+        # @return [Sequel::Dataset<QuickRow>]
         def matching_multiple_selectors_joining_verifications(selectors, pact_columns:, verifications_columns: )
           pact_publications = pact_publications_matching_selectors_as_consumer(selectors, pact_columns: pact_columns).from_self(alias: :p)
           verifications = verifications_matching_selectors_as_provider(selectors, verifications_columns: verifications_columns)
@@ -252,6 +230,8 @@ def matching_multiple_selectors_joining_verifications(selectors, pact_columns:,
         end
 
         # @param [Array<PactBroker::Matrix::ResolvedSelector>] selectors
+        # @param [Symbol] pact_columns the method to call on the Model class to get the right columns required for the particular query
+        # @return [Sequel::Dataset<QuickRow>]
         def pact_publications_matching_selectors_as_consumer(resolved_selectors, pact_columns:)
           # get the UnresolvedSelector objects back out of the resolved_selectors because the Version.for_selector() method uses the UnresolvedSelector
           unresolved_selectors = resolved_selectors.collect(&:original_selector).uniq
@@ -263,6 +243,8 @@ def pact_publications_matching_selectors_as_consumer(resolved_selectors, pact_co
 
 
         # @param [Array<PactBroker::Matrix::ResolvedSelector>] selectors
+        # @param [Symbol] verifications_columns the method to call on the QuickRow::Verifications/EveryRow::Verifications model to get the right columns required for the particular query
+        # @return [Sequel::Dataset<QuickRow>]
         def verifications_matching_selectors_as_provider(selectors, verifications_columns: )
           # get the UnresolvedSelector objects back out of the resolved_selectors because the Version.for_selector() method uses the UnresolvedSelector
           unresolved_selectors = selectors.collect(&:original_selector).uniq
@@ -270,47 +252,31 @@ def verifications_matching_selectors_as_provider(selectors, verifications_column
           pacticipant_ids = selectors.collect(&:pacticipant_id).uniq
           verification_model
             .send(verifications_columns)
-            .join_versions(versions)
+            .join_versions_dataset(versions)
             .where(consumer_id: pacticipant_ids)
         end
 
-
+        # Allow this to be overriden in EveryRow
         def verification_model
           QuickRow::Verification
         end
 
-        def matching_multiple_selectors_without_joining_verifications(selectors)
+        # TODO refactor this to get rid of QueryIds
+        def matching_selectors_for_pacticipants_only(selectors)
           # There are no versions specified in these selectors, so we can do the whole
           # query based on the consumer/provider IDs, which we have in the pact_publication
           # table without having to do a join.
           query_ids = QueryIds.from_selectors(selectors)
           where {
             Sequel.&(
-              QueryBuilder.consumer_or_consumer_version_matches(query_ids, :p),
+              QueryBuilder.consumer_matches(query_ids, :p),
               QueryBuilder.provider_matches(query_ids, :p),
               QueryBuilder.either_consumer_or_provider_was_specified_in_query(query_ids, :p)
             )
           }
         end
 
-        def matching_any_of_multiple_selectors(selectors)
-          query_ids = QueryIds.from_selectors(selectors)
-          join_verifications_for(query_ids)
-            .where {
-              Sequel.&(
-                Sequel.|(
-                  QueryBuilder.consumer_or_consumer_version_matches(query_ids, :p),
-                  QueryBuilder.provider_or_provider_version_matches_or_pact_unverified(query_ids, :v, :p),
-                ),
-                QueryBuilder.either_consumer_or_provider_was_specified_in_query(query_ids, :p)
-              )
-            }
-        end
-
-        def join_verifications_for(query_ids)
-          left_outer_join(verifications_for(query_ids), LP_LV_JOIN, { table_alias: :v } )
-        end
-
+        # TODO refactor this to get rid of QueryIds
         def inner_join_verifications_matching_one_selector_provider_or_provider_version(query_ids)
           verifications = db[LV]
             .select(*JOINED_VERIFICATION_COLUMNS)
@@ -321,17 +287,6 @@ def inner_join_verifications_matching_one_selector_provider_or_provider_version(
           join(verifications, LP_LV_JOIN, { table_alias: :v } )
         end
 
-        def verifications_for(query_ids)
-          db[LV]
-            .select(*JOINED_VERIFICATION_COLUMNS)
-            .where {
-              Sequel.&(
-                QueryBuilder.consumer_in_pacticipant_ids(query_ids),
-                QueryBuilder.provider_or_provider_version_matches(query_ids)
-              )
-            }
-        end
-
         def join_consumers qualifier = :p, table_alias = :consumers
           join(
             :pacticipants,
@@ -348,21 +303,9 @@ def join_providers qualifier = :p, table_alias = :providers
           )
         end
 
-        def join_consumer_versions
-          join(:versions, CONSUMER_VERSION_JOIN, { table_alias: :cv })
-        end
-
-        def join_provider_versions
-          left_outer_join(:versions, PROVIDER_VERSION_JOIN, { table_alias: :pv } )
-        end
-
         def join_verifications
           left_outer_join(LV, LP_LV_JOIN, { table_alias: :v } )
         end
-
-        def inner_join_verifications
-          join(LV, LP_LV_JOIN, { table_alias: :v } )
-        end
       end # end dataset_module
 
       def pact_version_sha
diff --git a/spec/lib/pact_broker/matrix/every_row_spec.rb b/spec/lib/pact_broker/matrix/every_row_spec.rb
index f18001690..cd21bc8c0 100644
--- a/spec/lib/pact_broker/matrix/every_row_spec.rb
+++ b/spec/lib/pact_broker/matrix/every_row_spec.rb
@@ -99,35 +99,6 @@ module Matrix
           expect(subject.all?(&:has_verification?)).to be true
         end
       end
-
-      describe "join_verifications_for" do
-        before do
-          td.create_pact_with_verification("Foo", "1", "Bar", "2")
-            .create_provider("Wiffle")
-            .create_pact
-            .create_verification(provider_version: "5")
-        end
-
-        let(:query_ids) do
-          double("query_ids",
-            all_pacticipant_ids: [foo.id, bar.id],
-            pacticipant_version_ids: [],
-            pacticipant_ids: [foo.id, bar.id]
-          )
-        end
-
-        subject do
-          EveryRow
-            .select_all_columns
-            .join_verifications_for(query_ids)
-            .all
-        end
-
-        it "pre-filters the verifications before joining them" do
-          expect(subject.size).to eq 2
-          expect(subject.find{ |r| r.provider_id == wiffle.id && !r.has_verification? }).to_not be nil
-        end
-      end
     end
   end
 end

From 64a01a0a054ad177cc8faa89a34bc183e16973f3 Mon Sep 17 00:00:00 2001
From: Beth Skurrie <beth.skurrie@smartbear.com>
Date: Sat, 19 Aug 2023 16:00:11 +1000
Subject: [PATCH 4/6] style: rubocop

---
 lib/pact_broker/matrix/resolved_selector_builder.rb | 2 --
 1 file changed, 2 deletions(-)

diff --git a/lib/pact_broker/matrix/resolved_selector_builder.rb b/lib/pact_broker/matrix/resolved_selector_builder.rb
index 5b48e4770..5fc3783e9 100644
--- a/lib/pact_broker/matrix/resolved_selector_builder.rb
+++ b/lib/pact_broker/matrix/resolved_selector_builder.rb
@@ -50,7 +50,6 @@ def selector_for_non_existing_version(pacticipant, unresolved_selector, selector
         ResolvedSelector.for_pacticipant_and_non_existing_version(pacticipant, unresolved_selector, selector_type, ignore)
       end
 
-      # rubocop: disable Metrics/ParameterLists
       def selector_for_found_version(pacticipant, version, unresolved_selector, selector_type, one_of_many)
         ResolvedSelector.for_pacticipant_and_version(
           pacticipant,
@@ -61,7 +60,6 @@ def selector_for_found_version(pacticipant, version, unresolved_selector, select
           one_of_many
         )
       end
-      # rubocop: enable Metrics/ParameterLists
 
       def selector_for_all_versions_of_a_pacticipant(pacticipant, unresolved_selector, selector_type)
         # Doesn't make sense to ignore this, as you can't have a can-i-deploy query

From 219a289fa9dc737061530bcb49e8833ac90933b6 Mon Sep 17 00:00:00 2001
From: Beth Skurrie <beth.skurrie@smartbear.com>
Date: Sat, 19 Aug 2023 16:02:51 +1000
Subject: [PATCH 5/6] docs: comment

---
 lib/pact_broker/matrix/quick_row.rb | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/pact_broker/matrix/quick_row.rb b/lib/pact_broker/matrix/quick_row.rb
index 5678eaaf1..3c57a0cc7 100644
--- a/lib/pact_broker/matrix/quick_row.rb
+++ b/lib/pact_broker/matrix/quick_row.rb
@@ -206,6 +206,7 @@ def matching_one_selector(selectors)
           rows_where_selector_matches_consumer_cols.union(rows_where_selector_matches_provider_cols)
         end
 
+
         # When the user has specified multiple selectors, we only want to join the verifications for
         # the specified selectors. This is because of the behaviour of the left outer join.
         # Imagine a pact has been verified by a provider version that was NOT specified in the selectors.
@@ -215,6 +216,7 @@ def matching_one_selector(selectors)
         # Instead, we need to filter the verifications dataset down to only the ones specified in the selectors first,
         # and THEN join them to the pacts, so that we get a row for the pact with null provider version
         # and verification fields.
+        # IDEA FOR OPTIMISATION - would it work to limit the pact_publications query and the verifications query to the limit of the overall query?
         # @param [Array<PactBroker::Matrix::ResolvedSelector>] selectors
         # @param [Symbol] pact_columns the method to call on the QuickRow/EveryRow model to get the right columns required for the particular query
         # @param [Symbol] verifications_columns the method to call on the QuickRow::Verifications/EveryRow::Verifications model to get the right columns required for the particular query

From 30fdd0b59a39768921ab9dd67b069129081cedc2 Mon Sep 17 00:00:00 2001
From: Beth Skurrie <beth.skurrie@smartbear.com>
Date: Mon, 21 Aug 2023 10:17:21 +1000
Subject: [PATCH 6/6] docs: comment

---
 lib/pact_broker/matrix/resolved_selector_builder.rb | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/lib/pact_broker/matrix/resolved_selector_builder.rb b/lib/pact_broker/matrix/resolved_selector_builder.rb
index 5fc3783e9..5295b740f 100644
--- a/lib/pact_broker/matrix/resolved_selector_builder.rb
+++ b/lib/pact_broker/matrix/resolved_selector_builder.rb
@@ -34,6 +34,10 @@ def build
 
       # When a single selector specifies multiple versions (eg. "all prod pacts"), this expands
       # the single selector into one selector for each version.
+      # When a pacticipant is found, but there are no versions matching the selector,
+      # the versions array will be have a single item which is nil (`[nil]`).
+      # See PactBroker::Matrix::SelectorResolver#find_versions_for_selector
+      # There may be a better way to pass in this information.
       def build_resolved_selectors_for_versions(pacticipant, versions, unresolved_selector, selector_type)
         one_of_many = versions.compact.size > 1
         versions.collect do | version |
@@ -62,8 +66,6 @@ def selector_for_found_version(pacticipant, version, unresolved_selector, select
       end
 
       def selector_for_all_versions_of_a_pacticipant(pacticipant, unresolved_selector, selector_type)
-        # Doesn't make sense to ignore this, as you can't have a can-i-deploy query
-        # for "all versions of a pacticipant". But whatever.
         ResolvedSelector.for_pacticipant(
           pacticipant,
           unresolved_selector,