Merge pull request #1458 from openaustralia/feature/allow_webhook_to_include_run_stats

Feature: allow webhook to include run stats

Author: ianheggie-oaf

app/views/documentation/webhooks.html.haml

@@ -24,6 +24,18 @@
   You can send information through the request
   by putting it in the webhook URL,
   either as the path or as GET parameters.
+  The following UPPERCASE strings will be replaced with the current run details in the url:
+
+  * ADDED: Records added count
+  * AUTO: "true" if an automatic daily run, otherwise "false"
+  * COMMIT: Full git revision
+  * OUTCOME: "success" if run finished successfully, otherwise "failed"
+  * REMOVED: Records removed count
+  * REVISION: Abbreviated git revision (e.g. "c9fab2s7")
+  * RUN_TIME: Seconds taken to run (eg "603" is 10 minutes and 3 seconds)
+  * SCRAPER: Full scraper name, eg "owner-name/repo-name"
+  * STATUS_CODE: Process exit code (0 means success)
+
   If there’s something specific you’d like included in the webhook payload
   then please [ask a question on our help forum](#{host_origin("help")}/).
 

app/workers/deliver_webhook_worker.rb

@@ -13,7 +13,7 @@ def perform(webhook_delivery_id)
     # TODO: As soon as we've upgraded to a recent version of Ubuntu switch the SSL
     # verification back on. It's not crazy-super important that we verify SSL with
     # webhooks but it's sure a good idea.
-    connection = Faraday.new(webhook_delivery.webhook.url, ssl: { verify: false })
+    connection = Faraday.new(substituted_url(webhook_delivery), ssl: { verify: false })
     begin
       response = connection.post
       webhook_delivery.update(
@@ -26,4 +26,25 @@ def perform(webhook_delivery_id)
       Rails.logger.error("Webhook delivery failure on scraper #{webhook_delivery.run.scraper.full_name}: #{e}")
     end
   end
+
+  private
+
+  def substituted_url(webhook_delivery)
+    url = webhook_delivery.webhook.url
+    run = webhook_delivery.run
+    {
+      "ADDED" => run.records_added,
+      "AUTO" => run.auto?,
+      "COMMIT" => run.git_revision,
+      "OUTCOME" => run.finished_successfully? ? "success" : "failed",
+      "REMOVED" => run.records_removed,
+      "REVISION" => run.git_revision[0..7],
+      "RUN_TIME" => run.wall_time.to_i,
+      "SCRAPER" => run.scraper.full_name,
+      "STATUS_CODE" => run.status_code
+    }.each do |name, value|
+      url = url.gsub(name, value.to_s)
+    end
+    url
+  end
 end

spec/lib/morph/language_spec.rb

@@ -4,6 +4,11 @@
 require "spec_helper"
 
 describe Morph::Language do
+  before do
+    # Clear any files from manual testing
+    Dir.glob("default_files/*/template/data.sqlite").each { |file| File.delete(file) }
+  end
+
   let(:ruby) { described_class.new(:ruby) }
   let(:python) { described_class.new(:python) }
   let(:php) { described_class.new(:php) }

spec/spec_helper.rb

@@ -71,6 +71,12 @@
 # Commented out for the benefit of zeus
 # require 'rspec/autorun'
 
+# Commented out so we can run docker tests
+# require 'webmock/rspec'
+
+require "webmock"
+require "webmock/rspec/matchers"
+
 # Requires supporting ruby files with custom matchers and macros, etc.
 # in spec/support/ and its subdirectories.
 Dir[Rails.root.join("spec/support/**/*.rb")].sort.each { |f| require f }
@@ -132,6 +138,11 @@
   config.include RetryHelper
   config.include CaptureHelper
   config.include ToolAvailability
+  # Include helpers like 'webmock/rspec' does without WebMock.enable! / disable which breaks VCR
+  config.include WebMock::API
+  config.include WebMock::Matchers
+
+  WebMock::AssertionFailure.error_class = RSpec::Expectations::ExpectationNotMetError
 
   config.before(:suite) do
     Searchkick.disable_callbacks
@@ -145,11 +156,22 @@
   # For tests marked as :docker tests don't use VCR
   config.around do |ex|
     if ex.metadata.key?(:docker)
+      # Disable VCR and WebMock checks so docker can retrieve images
+      VCR.turned_off do
+        WebMock.allow_net_connect! # Just to be sure!
+        ex.run
+      end
+    elsif ex.metadata.key?(:webmock)
+      # WebMock checks all requests have been stubbed, and
+      # clean up WebMock stubs afterwards
       VCR.turned_off do
-        WebMock.allow_net_connect!
+        WebMock.disallow_net_connect!
+        # Do NOT surround with WebMock.enable! / disable as it breaks some docker and VCR specs!
         ex.run
+        WebMock.reset!
       end
     else
+      # VCR checks all requests are enclosed in a VCR.use_cassette call
       ex.run
     end
   end

spec/workers/deliver_webhook_worker_spec.rb

@@ -3,18 +3,99 @@
 
 require "spec_helper"
 
-describe DeliverWebhookWorker, :vcr do
+describe DeliverWebhookWorker do
   let(:scraper) { create(:scraper) }
-  let(:run) { create(:run) }
+  let(:run) do
+    create(:run,
+           git_revision: "c9fabbc7a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6",
+           records_added: 100,
+           records_removed: 200,
+           status_code: 3,
+           started_at: 400.4.seconds.ago,
+           finished_at: 0.1.seconds.ago,
+           auto: true)
+  end
+
+  describe "test to actual site" do
+    it "works" do
+      VCR.use_cassette("webhook_delivery") do
+        webhook = Webhook.create!(scraper: scraper, url: "http://requestb.in/x3pcr8x3")
+        webhook_delivery = webhook.deliveries.create!(run: run)
+        described_class.new.perform(webhook_delivery.id)
+        webhook_delivery.reload
+        expect(webhook_delivery.response_code).to eq(200)
+        expect(webhook_delivery.sent_at).to be_within(1.minute).of(DateTime.now)
+      end
+    end
+  end
 
-  it "works" do
-    VCR.use_cassette("webhook_delivery") do
-      webhook = Webhook.create!(scraper: scraper, url: "http://requestb.in/x3pcr8x3")
+  describe "error handling", :webmock do
+    it "records response code and time on success" do
+      webhook = Webhook.create!(scraper: scraper, url: "https://example.com/hook")
       webhook_delivery = webhook.deliveries.create!(run: run)
+
+      stub_request(:post, "https://example.com/hook").to_return(status: 200)
+
       described_class.new.perform(webhook_delivery.id)
+
       webhook_delivery.reload
-      expect(webhook_delivery.response_code).to be(200)
+      expect(webhook_delivery.response_code).to eq(200)
       expect(webhook_delivery.sent_at).to be_within(1.minute).of(DateTime.now)
     end
+
+    it "logs connection failures without raising" do
+      webhook = Webhook.create!(scraper: scraper, url: "http://example.com/hook")
+      webhook_delivery = webhook.deliveries.create!(run: run)
+
+      stub_request(:post, "http://example.com/hook").to_raise(Faraday::ConnectionFailed.new("Connection refused"))
+
+      allow(Rails.logger).to receive(:error)
+
+      expect do
+        described_class.new.perform(webhook_delivery.id)
+      end.not_to raise_error
+
+      expect(Rails.logger).to have_received(:error).with(/Webhook delivery failure/)
+    end
+  end
+
+  describe "URL substitution", :webmock do
+    it "substitutes all placeholders correctly" do
+      substitutions = {
+        "ADDED" => run.records_added,
+        "AUTO" => run.auto,
+        "COMMIT" => run.git_revision,
+        "OUTCOME" => "failed",
+        "REMOVED" => run.records_removed,
+        "REVISION" => "c9fabbc7",
+        "RUN_TIME" => run.wall_time.to_i,
+        "SCRAPER" => run.scraper.full_name,
+        "STATUS_CODE" => run.status_code
+      }
+
+      url_template = "https://example.com/?#{substitutions.keys.map { |k| "#{k.downcase}=#{k}" }.join('&')}"
+      expected_url = "https://example.com/?#{substitutions.map { |k, v| "#{k.downcase}=#{v}" }.join('&')}"
+
+      webhook = Webhook.create!(scraper: scraper, url: url_template)
+      webhook_delivery = webhook.deliveries.create!(run: run)
+
+      stub = stub_request(:post, expected_url).to_return(status: 200)
+
+      described_class.new.perform(webhook_delivery.id)
+
+      expect(stub).to have_been_requested.once
+    end
+
+    it "uses 'success' for OUTCOME when run succeeds" do
+      run.update!(status_code: 0)
+      webhook = Webhook.create!(scraper: scraper, url: "https://example.com/?outcome=OUTCOME")
+      webhook_delivery = webhook.deliveries.create!(run: run)
+
+      stub = stub_request(:post, "https://example.com/?outcome=success").to_return(status: 200)
+
+      described_class.new.perform(webhook_delivery.id)
+
+      expect(stub).to have_been_requested.once
+    end
   end
 end