diff --git a/lib/pact_broker/matrix/integration.rb b/lib/pact_broker/matrix/integration.rb index b51d4d893..b119a32f4 100644 --- a/lib/pact_broker/matrix/integration.rb +++ b/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) diff --git a/lib/pact_broker/matrix/quick_row.rb b/lib/pact_broker/matrix/quick_row.rb index a872470ac..2c69b31fc 100644 --- a/lib/pact_broker/matrix/quick_row.rb +++ b/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) diff --git a/lib/pact_broker/matrix/repository.rb b/lib/pact_broker/matrix/repository.rb index ac26ac369..d4090deb4 100644 --- a/lib/pact_broker/matrix/repository.rb +++ b/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,10 +83,18 @@ 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]) @@ -92,6 +102,57 @@ def find_integrations_for_specified_selectors(resolved_specified_selectors, infe 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) diff --git a/spec/fixtures/approvals/matrix_integration_ignore_spec.approved.json b/spec/fixtures/approvals/matrix_integration_ignore_spec.approved.json index 0d8bf76ba..dacc1ff19 100644 --- a/spec/fixtures/approvals/matrix_integration_ignore_spec.approved.json +++ b/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": { diff --git a/spec/fixtures/approvals/matrix_integration_spec.approved.json b/spec/fixtures/approvals/matrix_integration_spec.approved.json index 906ce32dd..4e3c1a705 100644 --- a/spec/fixtures/approvals/matrix_integration_spec.approved.json +++ b/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" + ] } } diff --git a/spec/lib/pact_broker/matrix/integration_ignore_spec.rb b/spec/lib/pact_broker/matrix/integration_ignore_spec.rb index 6d9cc2b76..ba4e97e2f 100644 --- a/spec/lib/pact_broker/matrix/integration_ignore_spec.rb +++ b/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 diff --git a/spec/lib/pact_broker/matrix/integration_spec.rb b/spec/lib/pact_broker/matrix/integration_spec.rb index a06f800d5..9c058eedf 100644 --- a/spec/lib/pact_broker/matrix/integration_spec.rb +++ b/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 { diff --git a/spec/lib/pact_broker/matrix/repository_spec.rb b/spec/lib/pact_broker/matrix/repository_spec.rb index 3e41bc1c3..60f69e626 100644 --- a/spec/lib/pact_broker/matrix/repository_spec.rb +++ b/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) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index d2f8d2c72..162e40966 100644 --- a/spec/spec_helper.rb +++ b/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 diff --git a/spec/support/approvals.rb b/spec/support/approvals.rb index cd38e138d..23be617ea 100644 --- a/spec/support/approvals.rb +++ b/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 |