From 274e145686e607538554e6875ea51d4ea9b46036 Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Tue, 13 Apr 2021 09:01:18 +1000 Subject: [PATCH] fix: select pact publication with matching consumer version when triggering webhook for verification --- lib/pact_broker/webhooks/trigger_service.rb | 23 ++++++++++--- .../webhooks/trigger_service_spec.rb | 33 +++++++++++++------ 2 files changed, 42 insertions(+), 14 deletions(-) diff --git a/lib/pact_broker/webhooks/trigger_service.rb b/lib/pact_broker/webhooks/trigger_service.rb index 62dea4a96..0122db774 100644 --- a/lib/pact_broker/webhooks/trigger_service.rb +++ b/lib/pact_broker/webhooks/trigger_service.rb @@ -22,7 +22,6 @@ def trigger_webhooks_for_new_pact(pact, event_context, webhook_options) def trigger_webhooks_for_updated_pact(existing_pact, updated_pact, event_context, webhook_options) webhook_service.trigger_webhooks updated_pact, nil, PactBroker::Webhooks::WebhookEvent::CONTRACT_PUBLISHED, event_context, webhook_options - # TODO this should use the sha! if existing_pact.pact_version_sha != updated_pact.pact_version_sha logger.info "Existing pact for version #{existing_pact.consumer_version_number} has been updated with new content, triggering webhooks for changed content" webhook_service.trigger_webhooks updated_pact, nil, PactBroker::Webhooks::WebhookEvent::CONTRACT_CONTENT_CHANGED, event_context, webhook_options @@ -33,9 +32,12 @@ def trigger_webhooks_for_updated_pact(existing_pact, updated_pact, event_context def trigger_webhooks_for_verification_results_publication(pact, verification, event_context, webhook_options) expand_events(event_context).each do | reconstituted_event_context | + # The pact passed in is the most recent one with the matching SHA. + # Find the pact with the right consumer version number + pact_for_triggered_webhook = find_pact_for_verification_triggered_webhook(pact, reconstituted_event_context) if verification.success webhook_service.trigger_webhooks( - pact, + pact_for_triggered_webhook, verification, PactBroker::Webhooks::WebhookEvent::VERIFICATION_SUCCEEDED, reconstituted_event_context, @@ -43,7 +45,7 @@ def trigger_webhooks_for_verification_results_publication(pact, verification, ev ) else webhook_service.trigger_webhooks( - pact, + pact_for_triggered_webhook, verification, PactBroker::Webhooks::WebhookEvent::VERIFICATION_FAILED, reconstituted_event_context, @@ -52,7 +54,7 @@ def trigger_webhooks_for_verification_results_publication(pact, verification, ev end webhook_service.trigger_webhooks( - pact, + pact_for_triggered_webhook, verification, PactBroker::Webhooks::WebhookEvent::VERIFICATION_PUBLISHED, reconstituted_event_context, @@ -101,6 +103,19 @@ def expand_events(event_context) end end + def find_pact_for_verification_triggered_webhook(pact, reconstituted_event_context) + if reconstituted_event_context[:consumer_version_number] + find_pact_params = { + consumer_name: pact.consumer_name, + provider_name: pact.provider_name, + consumer_version_number: reconstituted_event_context[:consumer_version_number] + } + pact_service.find_pact(find_pact_params) || pact + else + pact + end + end + def print_debug_messages(changed_pacts) if changed_pacts.any? messages = changed_pacts.collect do |tag, previous_pact| diff --git a/spec/lib/pact_broker/webhooks/trigger_service_spec.rb b/spec/lib/pact_broker/webhooks/trigger_service_spec.rb index 713379354..f5ab73446 100644 --- a/spec/lib/pact_broker/webhooks/trigger_service_spec.rb +++ b/spec/lib/pact_broker/webhooks/trigger_service_spec.rb @@ -3,9 +3,10 @@ module PactBroker module Webhooks describe TriggerService do - let(:pact) { double("pact", pact_version_sha: pact_version_sha) } + let(:pact) { double("pact", pact_version_sha: pact_version_sha, consumer_name: "foo", provider_name: "bar") } let(:pact_version_sha) { "111" } let(:pact_repository) { double("pact_repository", find_previous_pacts: previous_pacts) } + let(:pact_service) { class_double("PactBroker::Pacts::Service").as_stubbed_const } let(:webhook_service) { double("webhook_service", trigger_webhooks: nil) } let(:previous_pact) { double("previous_pact", pact_version_sha: previous_pact_version_sha) } let(:previous_pact_version_sha) { "111" } @@ -16,6 +17,7 @@ module Webhooks before do allow(TriggerService).to receive(:pact_repository).and_return(pact_repository) + allow(TriggerService).to receive(:pact_service).and_return(pact_service) allow(TriggerService).to receive(:webhook_service).and_return(webhook_service) allow(TriggerService).to receive(:logger).and_return(logger) end @@ -139,6 +141,10 @@ module Webhooks end describe "#trigger_webhooks_for_verification_results_publication" do + before do + allow(pact_service).to receive(:find_pact).and_return(pact_for_consumer_version_1, pact_for_consumer_version_2) + # allow(pact_service).to receive(:find_pact).with(hash_including(consumer_version_number: "2")).and_return(pact_for_consumer_version_2) + end let(:verification) { double("verification", success: success) } let(:success) { true } # See lib/pact_broker/pacts/metadata.rb build_metadata_for_pact_for_verification @@ -153,21 +159,28 @@ module Webhooks end let(:expected_event_context_1) { { consumer_version_number: "1", consumer_version_tags: ["prod", "main"], other: "foo" } } let(:expected_event_context_2) { { consumer_version_number: "2", consumer_version_tags: ["feat/2"], other: "foo" } } + let(:pact_for_consumer_version_1) { double('pact_for_consumer_version_1') } + let(:pact_for_consumer_version_2) { double('pact_for_consumer_version_2') } subject { TriggerService.trigger_webhooks_for_verification_results_publication(pact, verification, event_context, webhook_options) } - context "when the verification is successful" do + it "find the pact publication for each consumer version number" do + expect(pact_service).to receive(:find_pact).with(hash_including(consumer_version_number: "1")).and_return(pact_for_consumer_version_1) + expect(pact_service).to receive(:find_pact).with(hash_including(consumer_version_number: "2")).and_return(pact_for_consumer_version_2) + subject + end + context "when the verification is successful" do context "when there are consumer_version_selectors in the event_context" do it "triggers a provider_verification_succeeded webhook for each consumer version (ie. commit)" do - expect(webhook_service).to receive(:trigger_webhooks).with(pact, verification, PactBroker::Webhooks::WebhookEvent::VERIFICATION_SUCCEEDED, expected_event_context_1, webhook_options) - expect(webhook_service).to receive(:trigger_webhooks).with(pact, verification, PactBroker::Webhooks::WebhookEvent::VERIFICATION_SUCCEEDED, expected_event_context_2, webhook_options) + expect(webhook_service).to receive(:trigger_webhooks).with(pact_for_consumer_version_1, verification, PactBroker::Webhooks::WebhookEvent::VERIFICATION_SUCCEEDED, expected_event_context_1, webhook_options) + expect(webhook_service).to receive(:trigger_webhooks).with(pact_for_consumer_version_2, verification, PactBroker::Webhooks::WebhookEvent::VERIFICATION_SUCCEEDED, expected_event_context_2, webhook_options) subject end it "triggers a provider_verification_published webhook for each consumer version (ie. commit)" do - expect(webhook_service).to receive(:trigger_webhooks).with(pact, verification, PactBroker::Webhooks::WebhookEvent::VERIFICATION_PUBLISHED, expected_event_context_1, webhook_options) - expect(webhook_service).to receive(:trigger_webhooks).with(pact, verification, PactBroker::Webhooks::WebhookEvent::VERIFICATION_PUBLISHED, expected_event_context_2, webhook_options) + expect(webhook_service).to receive(:trigger_webhooks).with(pact_for_consumer_version_1, verification, PactBroker::Webhooks::WebhookEvent::VERIFICATION_PUBLISHED, expected_event_context_1, webhook_options) + expect(webhook_service).to receive(:trigger_webhooks).with(pact_for_consumer_version_2, verification, PactBroker::Webhooks::WebhookEvent::VERIFICATION_PUBLISHED, expected_event_context_2, webhook_options) subject end end @@ -188,14 +201,14 @@ module Webhooks context "when there are consumer_version_selectors in the event_context" do it "triggers a provider_verification_failed webhook for each consumer version (ie. commit)" do - expect(webhook_service).to receive(:trigger_webhooks).with(pact, verification, PactBroker::Webhooks::WebhookEvent::VERIFICATION_FAILED, expected_event_context_1, webhook_options) - expect(webhook_service).to receive(:trigger_webhooks).with(pact, verification, PactBroker::Webhooks::WebhookEvent::VERIFICATION_FAILED, expected_event_context_2, webhook_options) + expect(webhook_service).to receive(:trigger_webhooks).with(pact_for_consumer_version_1, verification, PactBroker::Webhooks::WebhookEvent::VERIFICATION_FAILED, expected_event_context_1, webhook_options) + expect(webhook_service).to receive(:trigger_webhooks).with(pact_for_consumer_version_2, verification, PactBroker::Webhooks::WebhookEvent::VERIFICATION_FAILED, expected_event_context_2, webhook_options) subject end it "triggeres a provider_verification_published webhook for each consumer version (ie. commit)" do - expect(webhook_service).to receive(:trigger_webhooks).with(pact, verification, PactBroker::Webhooks::WebhookEvent::VERIFICATION_PUBLISHED, expected_event_context_1, webhook_options) - expect(webhook_service).to receive(:trigger_webhooks).with(pact, verification, PactBroker::Webhooks::WebhookEvent::VERIFICATION_PUBLISHED, expected_event_context_2, webhook_options) + expect(webhook_service).to receive(:trigger_webhooks).with(pact_for_consumer_version_1, verification, PactBroker::Webhooks::WebhookEvent::VERIFICATION_PUBLISHED, expected_event_context_1, webhook_options) + expect(webhook_service).to receive(:trigger_webhooks).with(pact_for_consumer_version_2, verification, PactBroker::Webhooks::WebhookEvent::VERIFICATION_PUBLISHED, expected_event_context_2, webhook_options) subject end end