feat: log the reason why a webhook has been triggered

bethesque · bethesque · commit 30522c0d52a6 · 2019-04-27T19:35:25.000+10:00
diff --git a/lib/pact_broker/pacts/repository.rb b/lib/pact_broker/pacts/repository.rb
@@ -258,9 +258,11 @@ def find_previous_distinct_pact pact
 
       def find_previous_pacts pact
         if pact.consumer_version_tag_names.any?
-          pact.consumer_version_tag_names.map { |tag| find_previous_pact(pact, tag) }
+          pact.consumer_version_tag_names.each_with_object({}) do |tag, tags_to_pacts|
+            tags_to_pacts[tag] = find_previous_pact(pact, tag)
+          end
         else
-          [find_previous_pact(pact, :untagged)]
+          { :untagged => find_previous_pact(pact, :untagged) }
         end
       end
 
diff --git a/lib/pact_broker/pacts/service.rb b/lib/pact_broker/pacts/service.rb
@@ -110,13 +110,9 @@ def find_distinct_pacts_between consumer, options
         distinct
       end
 
-      # TODO also take into account overridden revisions
-      def pact_is_new_or_pact_has_changed_since_previous_version? pact
-        pact_repository.find_previous_pacts(pact).any? { |previous_pact| previous_pact.nil? || pact.json_content != previous_pact.json_content}
-      end
-
       private
 
+      # Overwriting an existing pact with the same consumer/provider/consumer version number
       def update_pact params, existing_pact
         logger.info "Updating existing pact publication with params #{params.reject{ |k, v| k == :json_content}}"
         logger.debug "Content #{params[:json_content]}"
@@ -125,17 +121,12 @@ def update_pact params, existing_pact
         update_params = { pact_version_sha: pact_version_sha, json_content: json_content }
         updated_pact = pact_repository.update(existing_pact.id, update_params)
 
-        webhook_service.trigger_webhooks updated_pact, nil, PactBroker::Webhooks::WebhookEvent::CONTRACT_PUBLISHED
-        # TODO this should use the sha!
-        if existing_pact.json_content != updated_pact.json_content
-          webhook_service.trigger_webhooks updated_pact, nil, PactBroker::Webhooks::WebhookEvent::CONTRACT_CONTENT_CHANGED
-        else
-          logger.debug "Pact has not changed since previous version, not triggering webhooks for changed content"
-        end
+        webhook_trigger_service.trigger_webhooks_for_updated_pact(existing_pact, updated_pact)
 
         updated_pact
       end
 
+      # When no publication for the given consumer/provider/consumer version number exists
       def create_pact params, version, provider
         logger.info "Creating new pact publication with params #{params.reject{ |k, v| k == :json_content}}"
         logger.debug "Content #{params[:json_content]}"
@@ -148,7 +139,7 @@ def create_pact params, version, provider
           pact_version_sha: pact_version_sha,
           json_content: json_content
         )
-        trigger_webhooks pact
+        webhook_trigger_service.trigger_webhooks_for_new_pact pact
         pact
       end
 
@@ -159,16 +150,6 @@ def generate_sha(json_content)
       def add_interaction_ids(json_content)
         Content.from_json(json_content).with_ids.to_json
       end
-
-      def trigger_webhooks pact
-        # TODO add tests for this
-        webhook_service.trigger_webhooks pact, nil, PactBroker::Webhooks::WebhookEvent::CONTRACT_PUBLISHED
-        if pact_is_new_or_pact_has_changed_since_previous_version?(pact)
-          webhook_service.trigger_webhooks pact, nil, PactBroker::Webhooks::WebhookEvent::CONTRACT_CONTENT_CHANGED
-        else
-          logger.debug "Pact has not changed since previous version, not triggering webhooks for changed content"
-        end
-      end
     end
   end
 end
diff --git a/lib/pact_broker/services.rb b/lib/pact_broker/services.rb
@@ -66,5 +66,10 @@ def integration_service
       require 'pact_broker/integrations/service'
       Integrations::Service
     end
+
+    def webhook_trigger_service
+      require 'pact_broker/webhooks/trigger_service'
+      Webhooks::TriggerService
+    end
   end
 end
diff --git a/lib/pact_broker/webhooks/trigger_service.rb b/lib/pact_broker/webhooks/trigger_service.rb
@@ -0,0 +1,68 @@
+require 'pact_broker/services'
+
+module PactBroker
+  module Webhooks
+    module TriggerService
+
+      extend self
+      extend PactBroker::Repositories
+      extend PactBroker::Services
+      include PactBroker::Logging
+
+      def trigger_webhooks_for_new_pact pact
+        webhook_service.trigger_webhooks pact, nil, PactBroker::Webhooks::WebhookEvent::CONTRACT_PUBLISHED
+        if pact_is_new_or_newly_tagged_or_pact_has_changed_since_previous_version?(pact)
+          webhook_service.trigger_webhooks pact, nil, PactBroker::Webhooks::WebhookEvent::CONTRACT_CONTENT_CHANGED
+        else
+          logger.debug "Pact content has not changed since previous version, not triggering webhooks for changed content"
+        end
+      end
+
+      def trigger_webhooks_for_updated_pact(existing_pact, updated_pact)
+        webhook_service.trigger_webhooks updated_pact, nil, PactBroker::Webhooks::WebhookEvent::CONTRACT_PUBLISHED
+        # TODO this should use the sha!
+        if existing_pact.pact_version_sha != updated_pact.pact_version_sha
+          logger.debug "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
+        else
+          logger.debug "Pact content has not changed since previous revision, not triggering webhooks for changed content"
+        end
+      end
+
+      private
+
+      def pact_is_new_or_newly_tagged_or_pact_has_changed_since_previous_version? pact
+        changed_pacts = pact_repository
+          .find_previous_pacts(pact)
+          .reject { |_, previous_pact| !sha_changed_or_no_previous_version?(previous_pact, pact) }
+        print_debug_messages(changed_pacts)
+        changed_pacts.any?
+      end
+
+      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 print_debug_messages(changed_pacts)
+        if changed_pacts.any?
+          messages = changed_pacts.collect do |tag, previous_pact|
+            if tag == :untagged
+              if previous_pact
+                "pact content has changed since previous untagged version"
+              else
+                "first time untagged pact published"
+              end
+            else
+              if previous_pact
+                "pact content has changed since the last consumer version tagged with #{tag}"
+              else
+                "first time pact published with consumer version tagged #{tag}"
+              end
+            end
+          end
+          logger.debug("Webhook triggered for the following reasons: #{messages.join(',')}" )
+        end
+      end
+    end
+  end
+end
diff --git a/spec/lib/pact_broker/pacts/service_spec.rb b/spec/lib/pact_broker/pacts/service_spec.rb
@@ -14,6 +14,7 @@ module Pacts
 
         before do
           allow(described_class).to receive(:webhook_service).and_return(webhook_service)
+          allow(described_class).to receive(:webhook_trigger_service).and_return(webhook_trigger_service)
           allow(pacticipant_repository).to receive(:find_by_name_or_create).with(params[:consumer_name]).and_return(consumer)
           allow(pacticipant_repository).to receive(:find_by_name_or_create).with(params[:provider_name]).and_return(provider)
           allow(version_repository).to receive(:find_by_pacticipant_id_and_number_or_create).and_return(version)
@@ -22,9 +23,12 @@ module Pacts
           allow(pact_repository).to receive(:update).and_return(new_pact)
           allow(pact_repository).to receive(:find_previous_pacts).and_return(previous_pacts)
           allow(webhook_service).to receive(:trigger_webhooks)
+          allow(webhook_trigger_service).to receive(:trigger_webhooks_for_new_pact)
+          allow(webhook_trigger_service).to receive(:trigger_webhooks_for_updated_pact)
         end
 
         let(:webhook_service) { class_double("PactBroker::Webhooks::Service").as_stubbed_const }
+        let(:webhook_trigger_service) { class_double("PactBroker::Webhooks::TriggerService").as_stubbed_const }
         let(:consumer) { double('consumer', id: 1) }
         let(:provider) { double('provider', id: 2) }
         let(:version) { double('version', id: 3, pacticipant_id: 1) }
@@ -52,7 +56,6 @@ module Pacts
 
         subject { Service.create_or_update_pact(params) }
 
-
         context "when no pact exists with the same params" do
           it "creates the sha before adding the interaction ids" do
             expect(PactBroker::Pacts::GenerateSha).to receive(:call).ordered
@@ -65,8 +68,8 @@ module Pacts
             subject
           end
 
-          it "triggers webhooks for contract publications" do
-            expect(webhook_service).to receive(:trigger_webhooks).with(new_pact, nil, PactBroker::Webhooks::WebhookEvent::CONTRACT_PUBLISHED)
+          it "triggers webhooks" do
+            expect(webhook_trigger_service).to receive(:trigger_webhooks_for_new_pact).with(new_pact)
             subject
           end
         end
@@ -85,8 +88,8 @@ module Pacts
             subject
           end
 
-          it "triggers webhooks for contract publications" do
-            expect(webhook_service).to receive(:trigger_webhooks).with(new_pact, nil, PactBroker::Webhooks::WebhookEvent::CONTRACT_PUBLISHED)
+          it "triggers webhooks" do
+            expect(webhook_trigger_service).to receive(:trigger_webhooks_for_updated_pact).with(existing_pact, new_pact)
             subject
           end
         end
@@ -111,91 +114,6 @@ module Pacts
         end
       end
 
-      describe "#pact_is_new_or_pact_has_changed_since_previous_version?" do
-        let(:json_content) { { 'some' => 'json'}.to_json }
-        let(:pact) { instance_double(PactBroker::Domain::Pact, json_content: json_content)}
-
-        subject { Service.pact_is_new_or_pact_has_changed_since_previous_version? pact }
-
-        context "when consumer version is untagged" do
-          before do
-            allow(pact).to receive(:consumer_version_tag_names).and_return([]);
-            allow_any_instance_of(Pacts::Repository).to receive(:find_previous_pact).with(pact, :untagged).and_return(previous_pact)
-          end
-
-          context "when a previous pact is found" do
-            let(:previous_pact) { instance_double(PactBroker::Domain::Pact, json_content: previous_json_content)}
-            let(:previous_json_content) { {'some' => 'json'}.to_json }
-
-            context "when the json_content is the same" do
-              it "returns false" do
-                expect(subject).to be_falsey
-              end
-            end
-
-            context "when the json_content is not the same" do
-              let(:previous_json_content) { {'some-other' => 'json'}.to_json }
-              it "returns truthy" do
-                expect(subject).to be_truthy
-              end
-            end
-          end
-
-          context "when a previous pact is not found" do
-            let(:previous_pact) { nil }
-
-            it "returns true" do
-              expect(subject).to be_truthy
-            end
-          end
-        end
-
-        context "when consumer version has two tags" do
-          before do
-            allow(pact).to receive(:consumer_version_tag_names).and_return(['tag_1', 'tag_2']);
-            allow_any_instance_of(Pacts::Repository).to receive(:find_previous_pact).with(pact, 'tag_1').and_return(previous_pact_tag_1)
-            allow_any_instance_of(Pacts::Repository).to receive(:find_previous_pact).with(pact, 'tag_2').and_return(previous_pact_tag_2)
-          end
-
-          context "when a previous pact is found for both tags" do
-            let(:previous_pact_tag_1) { instance_double(PactBroker::Domain::Pact, json_content: previous_json_content_tag_1)}
-            let(:previous_json_content_tag_1) { {'some' => 'json'}.to_json }
-
-            let(:previous_pact_tag_2) { instance_double(PactBroker::Domain::Pact, json_content: previous_json_content_tag_2)}
-            let(:previous_json_content_tag_2) { {'some' => 'json'}.to_json }
-
-            context "when the json_content of both previous pacts and new pact is the same" do
-              it "returns false" do
-                expect(subject).to be_falsey
-              end
-            end
-
-            context "when the json_content of first previous pact is not the same" do
-              let(:previous_json_content_tag_1) { {'some-other' => 'json'}.to_json }
-              it "returns truthy" do
-                expect(subject).to be_truthy
-              end
-            end
-
-            context "when the json_content of second previous pact not the same" do
-              let(:previous_json_content_tag_2) { {'some-other' => 'json'}.to_json }
-              it "returns truthy" do
-                expect(subject).to be_truthy
-              end
-            end
-          end
-
-          context "when no previous pacts are found" do
-            let(:previous_pact_tag_1) { nil }
-            let(:previous_pact_tag_2) { nil }
-
-            it "returns true" do
-              expect(subject).to be_truthy
-            end
-          end
-        end
-      end
-
       describe "delete" do
         before do
           td.create_pact_with_hierarchy
diff --git a/spec/lib/pact_broker/webhooks/trigger_service_spec.rb b/spec/lib/pact_broker/webhooks/trigger_service_spec.rb
