fix: correctly handle template parameters in URL when rendering webho…

…ok resource
pact-foundation · Jun 9, 2018 · a4b69db · a4b69db
1 parent 076afe6
commit a4b69db
Show file tree

Hide file tree

Showing 5 changed files with 38 additions and 4 deletions.
diff --git a/lib/pact_broker/api/contracts/webhook_contract.rb b/lib/pact_broker/api/contracts/webhook_contract.rb
@@ -1,6 +1,7 @@
 require 'reform'
 require 'reform/form'
 require 'pact_broker/webhooks/check_host_whitelist'
+require 'pact_broker/webhooks/render'
 
 module PactBroker
   module Api
@@ -106,7 +107,7 @@ def host_whitelist
               end
 
               def parse_uri(uri_string, placeholder = 'placeholder')
-                URI(uri_string.gsub(/\$\{pactbroker\.[^\}]+\}/, placeholder))
+                URI(uri_string.gsub(PactBroker::Webhooks::Render::TEMPLATE_PARAMETER_REGEXP, placeholder))
               end
             end
 

diff --git a/lib/pact_broker/domain/webhook_request.rb b/lib/pact_broker/domain/webhook_request.rb
@@ -45,7 +45,7 @@ def initialize attributes = {}
       end
 
       def description
-        "#{method.upcase} #{URI(url).host}"
+        "#{method.upcase} #{URI(url.gsub(PactBroker::Webhooks::Render::TEMPLATE_PARAMETER_REGEXP, 'placeholder')).host}"
       end
 
       def display_password

diff --git a/lib/pact_broker/webhooks/render.rb b/lib/pact_broker/webhooks/render.rb
@@ -1,6 +1,9 @@
 module PactBroker
   module Webhooks
     class Render
+
+      TEMPLATE_PARAMETER_REGEXP = /\$\{pactbroker\.[^\}]+\}/
+
       def self.call(template, pact, verification = nil, &escaper)
         base_url = PactBroker.configuration.base_url
         params = {

diff --git a/spec/lib/pact_broker/domain/webhook_request_spec.rb b/spec/lib/pact_broker/domain/webhook_request_spec.rb
@@ -43,6 +43,14 @@ module Domain
         it "returns a brief description of the HTTP request" do
           expect(subject.description).to eq 'POST example.org'
         end
+
+        context "when the URL has a template parameter in it" do
+          let(:url) { "http://foo/commits/${pactbroker.consumerVersionNumber}" }
+
+          it "doesn't explode" do
+            expect(subject.description).to eq 'POST foo'
+          end
+        end
       end
 
       describe "display_password" do

diff --git a/spec/lib/pact_broker/webhooks/render_spec.rb b/spec/lib/pact_broker/webhooks/render_spec.rb
@@ -10,7 +10,25 @@ module Webhooks
         end
 
         let(:pact) do
-          instance_double("pact", consumer_version_number: "1.2.3+foo", consumer_name: "Foo", provider_name: "Bar")
+          instance_double("pact", consumer_version_number: "1.2.3+foo", consumer_name: "Foo", provider_name: "Bar", latest_verification: nil)
+        end
+
+        let(:pact_with_no_verification) { pact }
+
+        let(:pact_with_successful_verification) do
+          instance_double("pact",
+            consumer_version_number: "1.2.3+foo",
+            consumer_name: "Foo",
+            provider_name: "Bar",
+            latest_verification: verification)
+        end
+
+        let(:pact_with_failed_verification) do
+          instance_double("pact",
+            consumer_version_number: "1.2.3+foo",
+            consumer_name: "Foo",
+            provider_name: "Bar",
+            latest_verification: failed_verification)
         end
 
         let(:verification) do
@@ -21,6 +39,7 @@ module Webhooks
           instance_double("verification", provider_version_number: "3", success: false)
         end
 
+        let(:nil_pact) { nil }
         let(:nil_verification) { nil }
 
         subject { Render.call(template, pact, verification) }
@@ -34,7 +53,10 @@ module Webhooks
           ["${pactbroker.providerName}", "Bar", :pact, :verification],
           ["${pactbroker.githubVerificationStatus}", "success", :pact, :verification],
           ["${pactbroker.githubVerificationStatus}", "failure", :pact, :failed_verification],
-          ["${pactbroker.githubVerificationStatus}", "", :pact, :nil_verification]
+          ["${pactbroker.githubVerificationStatus}", "", :nil_pact, :nil_verification],
+          ["${pactbroker.githubVerificationStatus}", "pending", :pact_with_no_verification, :nil_verification],
+          ["${pactbroker.githubVerificationStatus}", "success", :pact_with_successful_verification, :nil_verification],
+          ["${pactbroker.githubVerificationStatus}", "failure", :pact_with_failed_verification, :nil_verification]
         ]
 
         TEST_CASES.each do | (template, expected_output, pact_var_name, verification_var_name) |