feat: improve performance of matrix when multiple selectors are speci…

…fied (#631)

CC-30188

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

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_dataset(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,18 +44,21 @@ 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 inner_join_verifications
-          join(:verifications, P_V_JOIN, { table_alias: :v } )
+        def verification_model
+          EveryRow::Verification
         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)
@@ -50,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

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
+            .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 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

lib/pact_broker/matrix/query_builder.rb

@@ -7,35 +7,21 @@ 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?
         ors << { qualify(provider_qualifier, :provider_id) => query_ids.pacticipant_ids } if query_ids.pacticipant_ids.any?
         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)
@@ -46,11 +32,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)
@@ -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]

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