fix: do not allow the deployment of a provider version with no result…

…s when one of its consumers is already deployed (#486)

lib/pact_broker/matrix/integration.rb

@@ -65,7 +65,7 @@ def pacticipant_names
       end
 
       def to_s
-        "Integration between #{consumer_name} (id=#{consumer_id}) and #{provider_name} (id=#{provider_id})"
+        "Integration between #{consumer_name} (id=#{consumer_id}) and #{provider_name} (id=#{provider_id}) required=#{required?}"
       end
 
       def involves_consumer_with_id?(consumer_id)

lib/pact_broker/matrix/quick_row.rb

@@ -71,6 +71,16 @@ class QuickRow < Sequel::Model(Sequel.as(:latest_pact_publication_ids_for_consum
         select(*SELECT_ALL_COLUMN_ARGS)
         select(*SELECT_PACTICIPANT_IDS_ARGS)
 
+        def distinct_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)
+            .from_self(alias: :integrations)
+            .select(:consumer_id, :provider_id, Sequel[:consumers][:name].as(:consumer_name), Sequel[:providers][:name].as(:provider_name))
+            .join_consumers(:integrations, :consumers)
+            .join_providers(:integrations, :providers)
+        end
+
         def distinct_integrations selectors, infer_integrations
           query = if selectors.size == 1
                     pacticipant_ids_matching_one_selector_optimised(selectors)

lib/pact_broker/matrix/repository.rb

@@ -1,3 +1,4 @@
+require "pact_broker/logging"
 require "pact_broker/repositories/helpers"
 require "pact_broker/matrix/row"
 require "pact_broker/matrix/quick_row"
@@ -20,6 +21,7 @@ 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
 
@@ -59,7 +61,7 @@ def find_ids_for_pacticipant_names params
       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, infer_selectors_for_integrations?(options))
+        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)
@@ -81,17 +83,76 @@ def find_considered_and_ignored_rows(resolved_selectors, resolved_ignore_selecto
         return considered_rows, ignored_rows
       end
 
-      def find_integrations_for_specified_selectors(resolved_specified_selectors, infer_integrations)
+      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, options)
+        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, infer_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, options)
+        integrations = integrations_where_specified_selector_is_consumer(resolved_specified_selectors) +
+                        integrations_where_specified_selector_is_provider(resolved_specified_selectors, options)
+        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
+              )
+            end
+        end
+      end
+
+      def integrations_where_specified_selector_is_provider(resolved_specified_selectors, options)
+        integrations_involving_specified_providers = PactBroker::Integrations::Integration
+                                                      .where(provider_id: resolved_specified_selectors.collect(&:pacticipant_id))
+                                                      .eager(:consumer)
+                                                      .all
+
+        destination_selector = PactBroker::Matrix::UnresolvedSelector.new(options.slice(:latest, :tag, :branch, :environment_name).compact)
+
+        integrations_involving_specified_providers.collect do | integration |
+          required = PactBroker::Domain::Version.where(pacticipant: integration.consumer).for_selector(destination_selector).any?
+
+          Integration.from_hash(
+            consumer_id: integration.consumer.id,
+            consumer_name: integration.consumer.name,
+            provider_id: integration.provider.id,
+            provider_name: integration.provider.name,
+            required: required
+          )
+        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)

spec/fixtures/approvals/matrix_integration_ignore_spec.approved.json

@@ -90,10 +90,10 @@
       "PactBroker::Matrix::PactNotEverVerifiedByProvider"
     ]
   },
-  "PactBroker::Matrix::Service find when deploying a provider and ignoring a consumer with a missing verification from a provider does allows the provider to be deployed even without ignoring anything because there is no connection between that version of the provider and the consumer": {
-    "deployable": true,
+  "PactBroker::Matrix::Service find when deploying a provider and ignoring a consumer with a missing verification from a provider does not allows the provider to be deployed": {
+    "deployable": null,
     "reasons": [
-      "PactBroker::Matrix::NoDependenciesMissing"
+      "PactBroker::Matrix::PactNotEverVerifiedByProvider"
     ]
   },
   "PactBroker::Matrix::Service find when deploying a provider and ignoring a consumer with a failed verification from a provider deployment_status_summary is expected to be deployable": {

spec/fixtures/approvals/matrix_integration_spec.approved.json

@@ -156,12 +156,18 @@
       "PactBroker::Matrix::Successful"
     ]
   },
-  "PactBroker::Matrix::Service find when there is a consumer with two providers, and only one of them has a verification, and the consumer and the verified provider are explicitly specified should allow the consumer to be deployed": {
+  "PactBroker::Matrix::Service find when there is a consumer with two providers, and only one of them has a verification, and the consumer and the verified provider are explicitly specified allows the consumer to be deployed": {
     "deployable": true,
     "reasons": [
       "PactBroker::Matrix::NoEnvironmentSpecified",
       "PactBroker::Matrix::SelectorWithoutPacticipantVersionNumberSpecified",
       "PactBroker::Matrix::Successful"
     ]
+  },
+  "PactBroker::Matrix::Service find when a provider version has no verification results with the consumer version already in the environment does not allow the provider to be deployed": {
+    "deployable": null,
+    "reasons": [
+      "PactBroker::Matrix::PactNotEverVerifiedByProvider"
+    ]
   }
 }

spec/lib/pact_broker/matrix/integration_ignore_spec.rb

@@ -199,9 +199,9 @@ module Matrix
 
             let(:ignore_selectors) { [] }
 
-            it "does allows the provider to be deployed even without ignoring anything because there is no connection between that version of the provider and the consumer" do
-              expect(subject.deployment_status_summary).to be_deployable
-              expect(subject.deployment_status_summary.reasons.collect(&:class)).to include(PactBroker::Matrix::NoDependenciesMissing)
+            it "does not allows the provider to be deployed" do
+              expect(subject.deployment_status_summary).to_not be_deployable
+              expect(subject.deployment_status_summary.reasons.collect(&:class)).to include(PactBroker::Matrix::PactNotEverVerifiedByProvider)
             end
           end
 

spec/lib/pact_broker/matrix/integration_spec.rb

@@ -426,13 +426,36 @@ module Matrix
             ]
           end
 
-          let(:options) { { latestby: "cvpv"} }
+          let(:options) { { latestby: "cvpv" } }
 
-          it "should allow the consumer to be deployed" do
+          it "allows the consumer to be deployed" do
             expect(subject.deployment_status_summary).to be_deployable
           end
         end
 
+        # https://github.com/pact-foundation/pact_broker/issues/485
+        describe "when a provider version has no verification results with the consumer version already in the environment" do
+          before do
+            td.create_pact_with_verification("Foo", "1", "Bar", "1")
+              .create_pact_with_verification("NotInTest", "1", "Bar", "1")
+              .create_consumer_version_tag("test")
+              .create_provider_version("2")
+          end
+
+          let(:selectors) do
+            [
+              UnresolvedSelector.new(pacticipant_name: "Bar", pacticipant_version_number: "2"),
+            ]
+          end
+
+          let(:options) { { latestby: "cvp", tag: "test", latest: true } }
+
+          it "does not allow the provider to be deployed" do
+            expect(subject.deployment_status_summary).to_not be_deployable
+            expect(subject.deployment_status_summary.reasons.collect(&:class)).to eq [PactBroker::Matrix::PactNotEverVerifiedByProvider]
+          end
+        end
+
         describe "when verification results are published missing tests for some interactions" do
           let(:pact_content) do
             {

spec/lib/pact_broker/matrix/repository_spec.rb

@@ -4,8 +4,6 @@
 module PactBroker
   module Matrix
     describe Repository do
-      let(:td) { TestDataBuilder.new}
-
       def build_selectors(hash)
         hash.collect do | key, value |
           UnresolvedSelector.new(pacticipant_name: key, pacticipant_version_number: value)

spec/spec_helper.rb

@@ -61,5 +61,5 @@ def app
 
 if ENV["DEBUG"] == "true"
   SemanticLogger.add_appender(io: $stdout)
-  SemanticLogger.default_level = :info
+  SemanticLogger.default_level = :debug
 end

spec/support/approvals.rb

@@ -30,12 +30,11 @@ def print_diff(exception)
 
 module MatrixQueryContentForApproval
   def matrix_query_content_for_approval(result)
-    content = {
+    {
       "deployable" => result.deployment_status_summary.deployable?,
       "reasons" => result.deployment_status_summary.reasons.collect(&:class).collect(&:name).sort
     }
   end
-
 end
 
 RSpec.configure do | config |