fix: correct the logic for determining the deployment status in the matrix resource

When there are multiple integrations, and one integration returns a matching matrix row for the can-i-deploy query, while another other doesn't, the deployable status should not be true.

Closes: pact-foundation/pact_broker-client#33

Author: bethesque

lib/pact_broker/api/decorators/matrix_decorator.rb

@@ -7,8 +7,8 @@ module Decorators
       class MatrixDecorator
         include PactBroker::Api::PactBrokerUrls
 
-        def initialize(lines)
-          @lines = lines
+        def initialize(query_results_with_deployment_status_summary)
+          @query_results_with_deployment_status_summary = query_results_with_deployment_status_summary
         end
 
         def to_json(options)
@@ -21,32 +21,27 @@ def to_hash(options)
               deployable: deployable,
               reason: reason
             },
-            matrix: matrix(lines, options[:user_options][:base_url])
-          }
+            matrix: matrix(options[:user_options][:base_url])
+          }.tap do | hash |
+            hash[:summary].merge!(query_results_with_deployment_status_summary.deployment_status_summary.counts)
+          end
+
         end
 
         def deployable
-          return nil if lines.empty?
-          return nil if lines.any?{ |line| line.success.nil? }
-          lines.any? && lines.all?{ |line| line.success }
+          query_results_with_deployment_status_summary.deployment_status_summary.deployable?
         end
 
         def reason
-          return "No results matched the given query" if lines.empty?
-          case deployable
-          when true then "All verification results are published and successful"
-          when false then "One or more verifications have failed"
-          else
-            "Missing one or more verification results"
-          end
+          query_results_with_deployment_status_summary.deployment_status_summary.reasons.join("\n")
         end
 
         private
 
-        attr_reader :lines
+        attr_reader :query_results_with_deployment_status_summary
 
-        def matrix(lines, base_url)
-          lines.collect do | line |
+        def matrix(base_url)
+          query_results_with_deployment_status_summary.rows.collect do | line |
             provider = OpenStruct.new(name: line.provider_name)
             consumer = OpenStruct.new(name: line.consumer_name)
             consumer_version = OpenStruct.new(number: line.consumer_version_number, pacticipant: consumer)

lib/pact_broker/api/resources/relationships.rb

@@ -22,9 +22,7 @@ def to_csv
         def pacts
           pact_service.find_latest_pacts
         end
-
       end
     end
-
   end
-end
+end

lib/pact_broker/matrix/deployment_status_summary.rb

@@ -0,0 +1,89 @@
+module PactBroker
+  module Matrix
+    class DeploymentStatusSummary
+      attr_reader :rows, :resolved_selectors, :integrations
+
+      def initialize(rows, resolved_selectors, integrations)
+        @rows = rows
+        @resolved_selectors = resolved_selectors
+        @integrations = integrations
+      end
+
+      def counts
+        {
+          success: rows.count{ |row| row.success },
+          failed: rows.count { |row| row.success == false },
+          unknown: integrations_without_a_row.count + rows.count { |row| row.success.nil? }
+        }
+      end
+
+      def deployable?
+        return nil if rows.empty?
+        return nil if rows.any?{ |row| row.success.nil? }
+        return nil if integrations_without_a_row.any?
+        rows.all?{ |row| row.success }
+      end
+
+      def reasons
+        @reasons ||= begin
+          reasons = []
+          if rows.empty?
+            reasons << "No results matched the given query"
+          else
+            reasons.concat(missing_reasons)
+            reasons.concat(failure_messages)
+            reasons.concat(unverified_messages)
+            reasons.concat(success_messages)
+          end
+          reasons
+        end
+      end
+
+      def unverified_messages
+        if rows.any?{ |row| row.success.nil? }
+          ["Missing one or more verification results"]
+        else
+          []
+        end
+      end
+
+      def failure_messages
+        if rows.any?{ |row| row.success == false }
+          ["One or more verifications have failed"]
+        else
+          []
+        end
+      end
+
+      def success_messages
+        if rows.all?{ |row| row.success } && integrations_without_a_row.empty?
+          ["All verification results are published and successful"]
+        else
+          []
+        end
+      end
+
+      def integrations_without_a_row
+        @integrations_without_a_row ||= begin
+          integrations.select do | relationship |
+            !rows.find do | row |
+              row.consumer_id == relationship.consumer_id && row.provider_id == relationship.provider_id
+            end
+          end
+        end
+      end
+
+      def missing_reasons
+        integrations_without_a_row.collect do | missing_relationship|
+          consumer_version_desc = "#{missing_relationship.consumer_name} (#{resolved_version_for(missing_relationship.consumer_id)})"
+          provider_version_desc = "#{missing_relationship.provider_name} (#{resolved_version_for(missing_relationship.provider_id)})"
+          "There is no verified pact between #{consumer_version_desc} and #{provider_version_desc}"
+        end
+      end
+
+      def resolved_version_for(pacticipant_id)
+        resolved_selectors.find{ | s| s[:pacticipant_id] == pacticipant_id }[:pacticipant_version_number]
+      end
+    end
+  end
+end

lib/pact_broker/matrix/integration.rb

@@ -0,0 +1,54 @@
+#
+# Represents the integration relationship between a consumer and a provider
+#
+module PactBroker
+  module Matrix
+    class Integration
+
+      attr_reader :consumer_name, :consumer_id, :provider_name, :provider_id
+
+      def initialize consumer_id, consumer_name, provider_id, provider_name
+        @consumer_id = consumer_id
+        @consumer_name = consumer_name
+        @provider_id = provider_id
+        @provider_name = provider_name
+      end
+
+      def self.from_hash hash
+        new(
+          hash.fetch(:consumer_id),
+          hash.fetch(:consumer_name),
+          hash.fetch(:provider_id),
+          hash.fetch(:provider_name)
+        )
+      end
+
+      def == other
+        consumer_id == other.consumer_id && provider_id == other.provider_id
+      end
+
+      def <=> other
+        comparison = consumer_name <=> other.consumer_name
+        return comparison if comparison != 0
+        provider_name <=> other.provider_name
+      end
+
+      def to_hash
+        {
+          consumer_name: consumer_name,
+          consumer_id: consumer_id,
+          provider_name: provider_name,
+          provider_id: provider_id,
+        }
+      end
+
+      def pacticipant_names
+        [consumer_name, provider_name]
+      end
+
+      def to_s
+        "Relationship between #{consumer_name} (id=#{consumer_id}) and #{provider_name} (id=#{provider_id})"
+      end
+    end
+  end
+end

lib/pact_broker/matrix/parse_query.rb

@@ -29,7 +29,7 @@ def self.call query
           options[:limit] = params['limit']
         end
         if params.key?('latest') && params['latest'] != ''
-          options[:latest] = params['latest']
+          options[:latest] = params['latest'] == 'true'
         end
         if params.key?('tag') && params['tag'] != ''
           options[:tag] = params['tag']

lib/pact_broker/matrix/query_results.rb

@@ -0,0 +1,18 @@
+module PactBroker
+  module Matrix
+    class QueryResults < Array
+      attr_reader :selectors, :options, :resolved_selectors
+
+      def initialize rows, selectors, options, resolved_selectors
+        super(rows)
+        @selectors = selectors
+        @resolved_selectors = resolved_selectors
+        @options = options
+      end
+
+      def rows
+        to_a
+      end
+    end
+  end
+end

lib/pact_broker/matrix/query_results_with_deployment_status_summary.rb

@@ -0,0 +1,14 @@
+require 'pact_broker/matrix/query_results'
+
+module PactBroker
+  module Matrix
+    class QueryResultsWithDeploymentStatusSummary < QueryResults
+      attr_reader :deployment_status_summary
+
+      def initialize rows, selectors, options, resolved_selectors, deployment_status_summary
+        super(rows, selectors, options, resolved_selectors)
+        @deployment_status_summary = deployment_status_summary
+      end
+    end
+  end
+end

lib/pact_broker/matrix/repository.rb

@@ -2,7 +2,9 @@
 require 'pact_broker/matrix/row'
 require 'pact_broker/matrix/head_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'
 
 module PactBroker
   module Matrix
@@ -65,7 +67,8 @@ def find_ids_for_pacticipant_names params
 
       # Return the latest matrix row (pact/verification) for each consumer_version_number/provider_version_number
       def find selectors, options = {}
-        lines = query_matrix(resolve_selectors(selectors, options), options)
+        resolved_selectors = resolve_selectors(selectors, options)
+        lines = query_matrix(resolved_selectors, options)
         lines = apply_latestby(options, selectors, lines)
 
         # This needs to be done after the latestby, so can't be done in the db unless
@@ -74,9 +77,31 @@ def find selectors, options = {}
           lines = lines.select{ |l| options[:success].include?(l.success) }
         end
 
-        lines.sort
+        QueryResults.new(lines.sort, selectors, options, resolved_selectors)
+      end
+
+      def find_for_consumer_and_provider pacticipant_1_name, pacticipant_2_name
+        selectors = [{ pacticipant_name: pacticipant_1_name }, { pacticipant_name: pacticipant_2_name }]
+        options = { latestby: 'cvpv' }
+        find(selectors, options)
+      end
+
+      def find_compatible_pacticipant_versions selectors
+        find(selectors, latestby: 'cvpv').select{|line| line.success }
+      end
+
+      def find_integrations(pacticipant_names)
+        selectors = pacticipant_names.collect{ | pacticipant_name | add_ids(pacticipant_name: pacticipant_name) }
+        Row
+          .select(:consumer_name, :consumer_id, :provider_name, :provider_id)
+          .matching_selectors(selectors)
+          .distinct
+          .all
+          .collect{ |row | Integration.from_hash(row.to_hash) }.uniq
       end
 
+      private
+
       def apply_latestby options, selectors, lines
         return lines unless options[:latestby]
         group_by_columns = case options[:latestby]
@@ -106,16 +131,6 @@ def remove_overwritten_revisions lines
         latest_revisions
       end
 
-      def find_for_consumer_and_provider pacticipant_1_name, pacticipant_2_name
-        selectors = [{ pacticipant_name: pacticipant_1_name }, { pacticipant_name: pacticipant_2_name }]
-        options = { latestby: 'cvpv' }
-        find(selectors, options)
-      end
-
-      def find_compatible_pacticipant_versions selectors
-        find(selectors, latestby: 'cvpv').select{|line| line.success }
-      end
-
       def query_matrix selectors, options
         query = view_for(options).select_all.matching_selectors(selectors)
         query = query.limit(options[:limit]) if options[:limit]
@@ -195,7 +210,7 @@ def add_ids(selector)
       # eg. when checking to see if Foo version 2 can be deployed to prod,
       # need to look up all the 'partner' pacticipants, and determine their latest prod versions
       def apply_latest_and_tag_to_inferred_selectors(selectors, options)
-        all_pacticipant_names = all_pacticipant_names_in_specified_matrix(selectors, options)
+        all_pacticipant_names = all_pacticipant_names_in_specified_matrix(selectors)
         specified_names = selectors.collect{ |s| s[:pacticipant_name] }
         inferred_names = all_pacticipant_names - specified_names
 
@@ -211,12 +226,9 @@ def apply_latest_and_tag_to_inferred_selectors(selectors, options)
         selectors + look_up_version_numbers(inferred_selectors, options)
       end
 
-      def all_pacticipant_names_in_specified_matrix(selectors, options)
-        query = view_for(options).select(:consumer_name, :provider_name)
-        query = query.matching_selectors(selectors)
-        query
-          .all
-          .collect{ | row | [row.consumer_name, row.provider_name] }
+      def all_pacticipant_names_in_specified_matrix(selectors)
+        find_integrations(selectors.collect{|s| s[:pacticipant_name]})
+          .collect(&:pacticipant_names)
           .flatten
           .uniq
       end

lib/pact_broker/matrix/service.rb

@@ -1,5 +1,6 @@
 require 'pact_broker/repositories'
 require 'pact_broker/matrix/row'
+require 'pact_broker/matrix/deployment_status_summary'
 
 module PactBroker
   module Matrix
@@ -17,12 +18,18 @@ def refresh_tags params, &block
         matrix_repository.refresh_tags(params, &block)
       end
 
-      def find criteria, options = {}
-        matrix_repository.find criteria, options
+      def find selectors, options = {}
+        query_results = matrix_repository.find selectors, options
+        pacticipant_names = selectors.collect{ | s| s[:pacticipant_name] }
+        integrations = matrix_repository.find_integrations(pacticipant_names)
+        deployment_status_summary = DeploymentStatusSummary.new(query_results.rows, query_results.resolved_selectors, integrations)
+        QueryResultsWithDeploymentStatusSummary.new(query_results.rows, query_results.selectors, query_results.options, query_results.resolved_selectors, deployment_status_summary)
       end
 
       def find_for_consumer_and_provider params
-        matrix_repository.find_for_consumer_and_provider params[:consumer_name], params[:provider_name]
+        selectors = [{ pacticipant_name: params[:consumer_name] }, { pacticipant_name: params[:provider_name] }]
+        options = { latestby: 'cvpv' }
+        find(selectors, options)
       end
 
       def find_for_consumer_and_provider_with_tags params