fix: trigger one webhook for each pact publication that the verified …

…content belongs to when using the 'pacts for verification' API (#378)

lib/pact_broker/api/resources/verifications.rb

@@ -50,7 +50,7 @@ def create_path
         end
 
         def from_json
-          verification = verification_service.create(next_verification_number, verification_params, pact, metadata, webhook_options)
+          verification = verification_service.create(next_verification_number, verification_params, pact, event_context, webhook_options)
           response.body = decorator_for(verification).to_json(decorator_options)
           true
         end
@@ -81,11 +81,14 @@ def wip?
           metadata[:wip] == 'true'
         end
 
+        def event_context
+          metadata
+        end
+
         def webhook_options
           {
             database_connector: database_connector,
             webhook_execution_configuration: webhook_execution_configuration
-                                      .with_webhook_context(metadata)
           }
         end
 

lib/pact_broker/api/resources/webhook_execution.rb

@@ -26,7 +26,7 @@ def allowed_methods
         end
 
         def process_post
-          webhook_execution_result = webhook_service.test_execution(webhook, webhook_execution_configuration)
+          webhook_execution_result = webhook_service.test_execution(webhook, webhook_execution_configuration.webhook_context, webhook_execution_configuration)
           response.headers['Content-Type'] = 'application/hal+json;charset=utf-8'
           response.body = post_response_body(webhook_execution_result)
           true

lib/pact_broker/domain/webhook.rb

@@ -52,15 +52,15 @@ def request_description
         request && request.description
       end
 
-      def execute pact, verification, options
-        logger.info "Executing #{self} webhook_context=#{options.fetch(:webhook_context)}"
-        webhook_request = request.build(template_parameters(pact, verification, options))
+      def execute pact, verification, event_context, options
+        logger.info "Executing #{self} event_context=#{event_context}"
+        webhook_request = request.build(template_parameters(pact, verification, event_context, options))
         http_response, error = execute_request(webhook_request)
 
         PactBroker::Webhooks::WebhookExecutionResult.new(
           webhook_request.http_request,
           http_response,
-          generate_logs(webhook_request, http_response, error, options[:webhook_context], options.fetch(:logging_options)),
+          generate_logs(webhook_request, http_response, error, event_context, options.fetch(:logging_options)),
           error
         )
       end
@@ -106,18 +106,18 @@ def execute_request(webhook_request)
         return http_response, error
       end
 
-      def template_parameters(pact, verification, options)
-        PactBroker::Webhooks::PactAndVerificationParameters.new(pact, verification, options.fetch(:webhook_context)).to_hash
+      def template_parameters(pact, verification, event_context, options)
+        PactBroker::Webhooks::PactAndVerificationParameters.new(pact, verification, event_context).to_hash
       end
 
-      def generate_logs(webhook_request, http_response, error, webhook_context, logging_options)
+      def generate_logs(webhook_request, http_response, error, event_context, logging_options)
         webhook_request_logger = PactBroker::Webhooks::WebhookRequestLogger.new(logging_options)
         webhook_request_logger.log(
           uuid,
           webhook_request,
           http_response,
           error,
-          webhook_context
+          event_context
         )
       end
     end

lib/pact_broker/hash_refinements.rb

@@ -30,6 +30,10 @@ def slice(*keys)
         keys.each_with_object(Hash.new) { |k, hash| hash[k] = self[k] if has_key?(k) }
       end
 
+      def without(*keys)
+        reject { |k,_| keys.include?(k) }
+      end
+
       def camelcase_keys
         camelcase_keys_private(self)
       end

lib/pact_broker/webhooks/pact_and_verification_parameters.rb

@@ -13,6 +13,7 @@ class PactAndVerificationParameters
       BITBUCKET_VERIFICATION_STATUS = 'pactbroker.bitbucketVerificationStatus'
       CONSUMER_LABELS = 'pactbroker.consumerLabels'
       PROVIDER_LABELS = 'pactbroker.providerLabels'
+      EVENT_NAME = 'pactbroker.eventName'
 
       ALL = [
         CONSUMER_NAME,
@@ -26,7 +27,8 @@ class PactAndVerificationParameters
         GITHUB_VERIFICATION_STATUS,
         BITBUCKET_VERIFICATION_STATUS,
         CONSUMER_LABELS,
-        PROVIDER_LABELS
+        PROVIDER_LABELS,
+        EVENT_NAME
       ]
 
       def initialize(pact, trigger_verification, webhook_context)
@@ -49,7 +51,8 @@ def to_hash
           GITHUB_VERIFICATION_STATUS => github_verification_status,
           BITBUCKET_VERIFICATION_STATUS => bitbucket_verification_status,
           CONSUMER_LABELS => pacticipant_labels(pact && pact.consumer),
-          PROVIDER_LABELS => pacticipant_labels(pact && pact.provider)
+          PROVIDER_LABELS => pacticipant_labels(pact && pact.provider),
+          EVENT_NAME => event_name
         }
       end
 
@@ -116,6 +119,10 @@ def provider_version_tags
       def pacticipant_labels pacticipant
         pacticipant && pacticipant.labels ? pacticipant.labels.collect(&:name).join(", ") : ""
       end
+
+      def event_name
+        webhook_context.fetch(:event_name)
+      end
     end
   end
-end
+end

lib/pact_broker/webhooks/service.rb

@@ -87,7 +87,7 @@ def self.find_all
         webhook_repository.find_all
       end
 
-      def self.test_execution webhook, execution_configuration
+      def self.test_execution webhook, event_context, execution_configuration
         merged_options = execution_configuration.with_failure_log_message("Webhook execution failed").to_hash
 
         verification = nil
@@ -96,7 +96,7 @@ def self.test_execution webhook, execution_configuration
         end
 
         pact = pact_service.search_for_latest_pact(consumer_name: webhook.consumer_name, provider_name: webhook.provider_name) || PactBroker::Pacts::PlaceholderPact.new
-        webhook.execute(pact, verification, merged_options)
+        webhook.execute(pact, verification, event_context.merge(event_name: "test"), merged_options)
       end
 
       def self.execute_triggered_webhook_now triggered_webhook, webhook_execution_configuration_hash
@@ -126,18 +126,19 @@ def self.trigger_webhooks pact, verification, event_name, event_context, options
 
         if webhooks.any?
           webhook_execution_configuration = options.fetch(:webhook_execution_configuration).with_webhook_context(event_name: event_name)
-          run_later(webhooks, pact, verification, event_name, options.merge(webhook_execution_configuration: webhook_execution_configuration))
+          # bit messy to merge in base_url here, but easier than a big refactor
+          base_url = options.fetch(:webhook_execution_configuration).webhook_context.fetch(:base_url)
+          run_later(webhooks, pact, verification, event_name, event_context.merge(event_name: event_name, base_url: base_url), options.merge(webhook_execution_configuration: webhook_execution_configuration))
         else
           logger.info "No enabled webhooks found for consumer \"#{pact.consumer.name}\" and provider \"#{pact.provider.name}\" and event #{event_name}"
         end
       end
 
-      def self.run_later webhooks, pact, verification, event_name, options
+      def self.run_later webhooks, pact, verification, event_name, event_context, options
         trigger_uuid = next_uuid
         webhooks.each do | webhook |
           begin
-            webhook_context = options.fetch(:webhook_execution_configuration).webhook_context
-            triggered_webhook = webhook_repository.create_triggered_webhook(trigger_uuid, webhook, pact, verification, RESOURCE_CREATION, event_name, webhook_context)
+            triggered_webhook = webhook_repository.create_triggered_webhook(trigger_uuid, webhook, pact, verification, RESOURCE_CREATION, event_name, event_context)
             logger.info "Scheduling job for webhook with uuid #{webhook.uuid}"
             logger.debug "Schedule webhook with options #{options}"
             job_data = { triggered_webhook: triggered_webhook }.deep_merge(options)

lib/pact_broker/webhooks/trigger_service.rb

@@ -1,4 +1,5 @@
 require 'pact_broker/services'
+require 'pact_broker/hash_refinements'
 
 module PactBroker
   module Webhooks
@@ -8,6 +9,7 @@ module TriggerService
       extend PactBroker::Repositories
       extend PactBroker::Services
       include PactBroker::Logging
+      using PactBroker::HashRefinements
 
       def trigger_webhooks_for_new_pact(pact, event_context, webhook_options)
         webhook_service.trigger_webhooks pact, nil, PactBroker::Webhooks::WebhookEvent::CONTRACT_PUBLISHED, event_context, webhook_options
@@ -30,31 +32,33 @@ def trigger_webhooks_for_updated_pact(existing_pact, updated_pact, event_context
       end
 
       def trigger_webhooks_for_verification_results_publication(pact, verification, event_context, webhook_options)
-        if verification.success
-          webhook_service.trigger_webhooks(
-            pact,
-            verification,
-            PactBroker::Webhooks::WebhookEvent::VERIFICATION_SUCCEEDED,
-            event_context,
-            webhook_options
-          )
-        else
+        expand_events(event_context).each do | reconstituted_event_context |
+          if verification.success
+            webhook_service.trigger_webhooks(
+              pact,
+              verification,
+              PactBroker::Webhooks::WebhookEvent::VERIFICATION_SUCCEEDED,
+              reconstituted_event_context,
+              webhook_options
+            )
+          else
+            webhook_service.trigger_webhooks(
+              pact,
+              verification,
+              PactBroker::Webhooks::WebhookEvent::VERIFICATION_FAILED,
+              reconstituted_event_context,
+              webhook_options
+            )
+          end
+
           webhook_service.trigger_webhooks(
             pact,
             verification,
-            PactBroker::Webhooks::WebhookEvent::VERIFICATION_FAILED,
-            event_context,
+            PactBroker::Webhooks::WebhookEvent::VERIFICATION_PUBLISHED,
+            reconstituted_event_context,
             webhook_options
           )
         end
-
-        webhook_service.trigger_webhooks(
-          pact,
-          verification,
-          PactBroker::Webhooks::WebhookEvent::VERIFICATION_PUBLISHED,
-          event_context,
-          webhook_options
-        )
       end
 
       private
@@ -71,6 +75,32 @@ def sha_changed_or_no_previous_version?(previous_pact, new_pact)
         previous_pact.nil? || new_pact.pact_version_sha != previous_pact.pact_version_sha
       end
 
+      def merge_consumer_version_selectors(consumer_version_number, selectors, event_context)
+        event_context.merge(
+          consumer_version_number: consumer_version_number,
+          consumer_version_tags: selectors.collect{ | selector | selector[:tag] }.compact.uniq
+        )
+      end
+
+      # Now that we de-duplicate the pact contents when verifying though the 'pacts for verification' API,
+      # we no longer get a webhook triggered for the verification results publication of each indiviual
+      # consumer version tag, meaning that only the most recent commit will get the updated verification status.
+      # To fix this, each URL of the pacts returned by the 'pacts for verification' API now contains the
+      # relevant selectors in the metadata, so that when the verification results are published,
+      # we can parse that metadata, and trigger one webhook for each consumer version like we used to.
+      # Actually, we used to trigger one webhook per tag, but given that the most likely use of the
+      # verification published webhook is for reporting git statuses,
+      # it makes more sense to trigger per consumer version number (ie. commit).
+      def expand_events(event_context)
+        triggers = if event_context[:consumer_version_selectors].is_a?(Array)
+          event_context[:consumer_version_selectors]
+            .group_by{ | selector | selector[:consumer_version_number] }
+            .collect { | consumer_version_number, selectors | merge_consumer_version_selectors(consumer_version_number, selectors, event_context.without(:consumer_version_selectors)) }
+        else
+          [event_context]
+        end
+      end
+
       def print_debug_messages(changed_pacts)
         if changed_pacts.any?
           messages = changed_pacts.collect do |tag, previous_pact|

lib/pact_broker/webhooks/triggered_webhook.rb

@@ -1,13 +1,15 @@
 require 'sequel'
 require 'pact_broker/repositories/helpers'
 require 'pact_broker/webhooks/execution'
+require 'pact_broker/hash_refinements'
 
 # Represents the relationship between a webhook and the event and object
 # that caused it to be triggered. eg a pact publication
 
 module PactBroker
   module Webhooks
     class TriggeredWebhook < Sequel::Model(:triggered_webhooks)
+      using PactBroker::HashRefinements
       plugin :timestamps, update_on_create: true
       plugin :serialization, :json, :event_context
 
@@ -60,7 +62,7 @@ def execute options
         # getting a random 'no method to_domain for null' error
         # not sure on which object, so splitting this out into two lines
         pact = pact_publication.to_domain
-        webhook.to_domain.execute(pact, verification, options)
+        webhook.to_domain.execute(pact, verification, event_context.symbolize_keys, options)
       end
 
       def consumer_name

spec/lib/pact_broker/api/resources/verifications_spec.rb

@@ -81,11 +81,6 @@ module Resources
               expect(subject.headers['Location']).to eq("http://example.org/pacts/provider/Provider/consumer/Consumer/pact-version/1234/verification-results/2")
             end
 
-            it "merges the upstream verification metdata into the webhook context" do
-              expect(webhook_execution_configuration).to receive(:with_webhook_context).with(parsed_metadata)
-              subject
-            end
-
             it "stores the verification in the database" do
               expect(PactBroker::Verifications::Service).to receive(:create).with(
                 next_verification_number,

spec/lib/pact_broker/api/resources/webhook_execution_spec.rb

@@ -30,7 +30,8 @@ module Resources
             let(:pact) { instance_double("PactBroker::Domain::Pact") }
             let(:consumer_name) { "foo" }
             let(:provider_name) { "bar" }
-            let(:webhook_execution_configuration) { instance_double(PactBroker::Webhooks::ExecutionConfiguration) }
+            let(:webhook_execution_configuration) { instance_double(PactBroker::Webhooks::ExecutionConfiguration, webhook_context: event_context) }
+            let(:event_context) { { some: "data" } }
 
             before do
               allow(PactBroker::Webhooks::Service).to receive(:test_execution).and_return(execution_result)
@@ -39,7 +40,7 @@ module Resources
             end
 
             it "executes the webhook" do
-              expect(PactBroker::Webhooks::Service).to receive(:test_execution).with(webhook, webhook_execution_configuration)
+              expect(PactBroker::Webhooks::Service).to receive(:test_execution).with(webhook, event_context, webhook_execution_configuration)
               subject
             end
 

spec/lib/pact_broker/domain/webhook_spec.rb

@@ -12,9 +12,9 @@ module Domain
       let(:webhook_template_parameters_hash) { { 'foo' => 'bar' } }
       let(:http_request) { double('http request') }
       let(:http_response) { double('http response') }
-      let(:webhook_context) { { some: 'things' } }
+      let(:event_context) { { some: 'things' } }
       let(:logging_options) { { other: 'options' } }
-      let(:options) { { webhook_context: webhook_context, logging_options: logging_options } }
+      let(:options) { { logging_options: logging_options } }
       let(:pact) { double('pact') }
       let(:verification) { double('verification') }
       let(:logger) { double('logger').as_null_object }
@@ -61,11 +61,11 @@ module Domain
 
         let(:webhook_request_logger) { instance_double(PactBroker::Webhooks::WebhookRequestLogger, log: "logs") }
 
-        let(:execute) { subject.execute pact, verification, options }
+        let(:execute) { subject.execute(pact, verification, event_context, options) }
 
         it "creates the template parameters" do
           expect(PactBroker::Webhooks::PactAndVerificationParameters).to receive(:new).with(
-            pact, verification, webhook_context
+            pact, verification, event_context
           )
           execute
         end
@@ -81,7 +81,7 @@ module Domain
         end
 
         it "generates the execution logs" do
-          expect(webhook_request_logger).to receive(:log).with(uuid, webhook_request, http_response, nil, webhook_context)
+          expect(webhook_request_logger).to receive(:log).with(uuid, webhook_request, http_response, nil, event_context)
           execute
         end
 
@@ -106,7 +106,7 @@ module Domain
           end
 
           it "generates the execution logs" do
-            expect(webhook_request_logger).to receive(:log).with(uuid, webhook_request, nil, instance_of(error_class), webhook_context)
+            expect(webhook_request_logger).to receive(:log).with(uuid, webhook_request, nil, instance_of(error_class), event_context)
             execute
           end