feat(webhook status): ensure triggered webhook and webhook execution objects are saved to database even when webhook fails and response code is 500

Author: bethesque

lib/pact_broker/api/decorators/pact_webhooks_status_decorator.rb

@@ -60,29 +60,40 @@ class PactWebhooksStatusDecorator < BaseDecorator
         end
 
         link :'pb:pact-version' do | context |
-          {
-            href: pact_url(context[:base_url], pact),
-            title: "Pact",
-            name: pact.name
-          }
+          if pact
+            {
+              href: pact_url(context[:base_url], pact),
+              title: "Pact",
+              name: pact.name
+            }
+          else
+            nil
+          end
         end
 
         link :'pb:consumer' do | context |
           {
-            href: pacticipant_url(context[:base_url], OpenStruct.new(name: context[:consumer_name])),
+            href: pacticipant_url(context[:base_url], fake_consumer(context)),
             title: "Consumer",
             name: context[:consumer_name]
           }
         end
 
         link :'pb:provider' do | context |
           {
-            href: pacticipant_url(context[:base_url], OpenStruct.new(name: context[:provider_name])),
+            href: pacticipant_url(context[:base_url], fake_provider(context)),
             title: "Provider",
             name: context[:provider_name]
           }
         end
 
+        link :'pb:pact-webhooks' do | context |
+          {
+            title: "Webhooks for the pact between #{context[:consumer_name]} and #{context[:provider_name]}",
+            href: webhooks_for_pact_url(fake_consumer(context), fake_provider(context), context.fetch(:base_url))
+          }
+        end
+
         def summary
           counts = represented.group_by(&:status).each_with_object({}) do | (status, triggered_webhooks), counts |
             counts[status] = triggered_webhooks.count
@@ -97,6 +108,14 @@ def triggered_webhooks_with_error_logs
         def pact
           represented.any? ? represented.first.pact_publication : nil
         end
+
+        def fake_consumer context
+          OpenStruct.new(name: context[:consumer_name])
+        end
+
+        def fake_provider context
+          OpenStruct.new(name: context[:provider_name])
+        end
       end
     end
   end

lib/pact_broker/api/decorators/webhook_decorator.rb

@@ -24,6 +24,13 @@ class WebhookDecorator < BaseDecorator
 
         end
 
+        link :'pb:execute' do | options |
+          {
+            title: "Test the execution of the webhook by sending a POST request to this URL",
+            href: webhook_execution_url(represented, options[:base_url])
+          }
+        end
+
         link :'pb:pact-webhooks' do | options |
           {
             title: "All webhooks for the pact between #{represented.consumer.name} and #{represented.provider.name}",
@@ -38,13 +45,6 @@ class WebhookDecorator < BaseDecorator
           }
         end
 
-        link :'pb:execute' do | options |
-          {
-            title: "Test the execution of the webhook by sending a POST request to this URL",
-            href: webhook_execution_url(represented, options[:base_url])
-          }
-
-        end
 
       end
     end

lib/pact_broker/api/resources/webhook_execution.rb

@@ -1,5 +1,6 @@
 require 'pact_broker/services'
 require 'pact_broker/api/decorators/webhook_execution_result_decorator'
+require 'pact_broker/constants'
 
 module PactBroker
   module Api
@@ -15,7 +16,12 @@ def process_post
           webhook_execution_result = webhook_service.execute_webhook_now webhook, pact
           response.headers['Content-Type'] = 'application/hal+json;charset=utf-8'
           response.body = post_response_body webhook_execution_result
-          webhook_execution_result.success? ? true : 500
+          if webhook_execution_result.success?
+            true
+          else
+            response.headers[PactBroker::DO_NOT_ROLLBACK] = 'true'
+            500
+          end
         end
 
         def resource_exists?

lib/pact_broker/constants.rb

@@ -1,5 +1,5 @@
 module PactBroker
 
   CONSUMER_VERSION_HEADER = 'X-Pact-Consumer-Version'.freeze
-
+  DO_NOT_ROLLBACK = 'X-Pact-Broker-Do-Not-Rollback'.freeze
 end

lib/pact_broker/webhooks/service.rb

@@ -62,6 +62,7 @@ def self.execute_webhook_now webhook, pact
         else
           webhook_repository.update_triggered_webhook_status triggered_webhook, TriggeredWebhook::STATUS_FAILURE
         end
+        webhook_execution_result
       end
 
       def self.execute_triggered_webhook_now triggered_webhook

lib/rack/pact_broker/database_transaction.rb

@@ -1,4 +1,4 @@
-require 'pact_broker/version'
+require 'pact_broker/constants'
 require 'sequel'
 
 module Rack
@@ -33,10 +33,16 @@ def call_with_transaction env
         response = nil
         @database_connection.transaction do
           response = @app.call(env)
-          raise Sequel::Rollback if response.first == 500
+          if response.first == 500
+            raise Sequel::Rollback unless do_not_rollback?(response)
+          end
         end
         response
       end
+
+      def do_not_rollback? response
+        response[1].delete(::PactBroker::DO_NOT_ROLLBACK)
+      end
     end
   end
 end

spec/features/execute_webhook_spec.rb

@@ -0,0 +1,74 @@
+require 'support/test_data_builder'
+require 'webmock/rspec'
+require 'rack/pact_broker/database_transaction'
+
+describe "Execute a webhook" do
+
+  let(:td) { TestDataBuilder.new }
+
+  before do
+    td.create_pact_with_hierarchy("Some Consumer", "1", "Some Provider")
+      .create_webhook(method: 'POST')
+  end
+
+  let(:path) { "/webhooks/#{td.webhook.uuid}/execute" }
+  let(:response_body) { JSON.parse(last_response.body, symbolize_names: true)}
+
+  subject { post path; last_response }
+
+  context "when the execution is successful" do
+    let!(:request) do
+      stub_request(:post, /http/).to_return(:status => 200)
+    end
+
+    it "performs the HTTP request" do
+      subject
+      expect(request).to have_been_made
+    end
+
+    it "returns a 200 response" do
+      puts subject.body
+      expect(subject.status).to be 200
+    end
+
+    it "saves a TriggeredWebhook" do
+      expect { subject }.to change { PactBroker::Webhooks::TriggeredWebhook.count }.by(1)
+    end
+
+    it "saves a WebhookExecution" do
+      expect { subject }.to change { PactBroker::Webhooks::Execution.count }.by(1)
+    end
+  end
+
+  context "when an error occurs", no_db_clean: true do
+    let(:app) { Rack::PactBroker::DatabaseTransaction.new(PactBroker::API, PactBroker::DB.connection) }
+
+    let!(:request) do
+      stub_request(:post, /http/).to_raise(Errno::ECONNREFUSED)
+    end
+
+    before do
+      PactBroker::Database.truncate
+      td.create_pact_with_hierarchy("Some Consumer", "1", "Some Provider")
+        .create_webhook(method: 'POST')
+    end
+
+    after do
+      PactBroker::Database.truncate
+    end
+
+    subject { post path; last_response }
+
+    it "returns a 500 response" do
+      expect(subject.status).to be 500
+    end
+
+    it "saves a TriggeredWebhook" do
+      expect { subject }.to change { PactBroker::Webhooks::TriggeredWebhook.count }.by(1)
+    end
+
+    it "saves a WebhookExecution" do
+      expect { subject }.to change { PactBroker::Webhooks::Execution.count }.by(1)
+    end
+  end
+end

spec/lib/pact_broker/api/decorators/pact_webhooks_status_decorator_spec.rb

@@ -36,9 +36,10 @@ module Decorators
         let(:retrying) { false }
         let(:status) { PactBroker::Webhooks::TriggeredWebhook::STATUS_SUCCESS }
         let(:logs_url) { "http://example.org/webhooks/4321/trigger/1234/logs" }
+        let(:triggered_webhooks) { [triggered_webhook] }
 
         let(:json) do
-          PactWebhooksStatusDecorator.new([triggered_webhook]).to_json(user_options: user_options)
+          PactWebhooksStatusDecorator.new(triggered_webhooks).to_json(user_options: user_options)
         end
 
         subject { JSON.parse json, symbolize_names: true }
@@ -112,6 +113,14 @@ module Decorators
             expect(subject[:summary]).to eq({successful: 0, failed: 0, notRun: 1})
           end
         end
+
+        context "when there are no triggered webhooks" do
+          let(:triggered_webhooks) { [] }
+
+          it "doesn't blow up" do
+            subject
+          end
+        end
       end
     end
   end

spec/lib/rack/pact_broker/database_transaction_spec.rb

@@ -13,8 +13,10 @@ module PactBroker
         ::PactBroker::Database.truncate
       end
 
+      let(:headers) { {} }
+
       let(:api) do
-        ->(env) { ::PactBroker::Domain::Pacticipant.create(name: 'Foo'); [500, {}, []] }
+        ->(env) { ::PactBroker::Domain::Pacticipant.create(name: 'Foo'); [500, headers, []] }
       end
 
       let(:app) do
@@ -39,6 +41,15 @@ module PactBroker
           end
         end
       end
+
+      context "when there is an error but the resource sets the no rollback header" do
+        let(:headers) { {::PactBroker::DO_NOT_ROLLBACK => 'true'} }
+        let(:http_method) { :post }
+
+        it "does not roll back" do
+          expect { subject }.to change { ::PactBroker::Domain::Pacticipant.count }.by(1)
+        end
+      end
     end
   end
 end

tasks/database.rb

@@ -77,7 +77,12 @@ def recreate
     def truncate
       TABLES.each do | table_name |
         if database.table_exists?(table_name)
-          database[table_name].delete
+          begin
+            database[table_name].delete
+          rescue SQLite3::ConstraintException => e
+            puts "Could not delete the following records from #{table_name}: #{database[table_name].select_all}"
+            raise e
+          end
         end
       end
     end