From b76f9c4e54936e0355a2b5c3a8f58fdd712d668d Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Thu, 13 Feb 2020 13:00:08 +1100 Subject: [PATCH] feat(create-webhook): support creating webhooks for contract_published, provider_verification_succeeded and provider_verification_failed events, and allow description to be set --- .../Pact Broker Client - Pact Broker.md | 75 +++++++++++++++++- lib/pact_broker/client/cli/broker.rb | 42 ++++++---- lib/pact_broker/client/cli/custom_thor.rb | 4 + lib/pact_broker/client/webhooks/create.rb | 2 +- .../cli/broker_run_webhook_commands_spec.rb | 6 +- .../pacts/pact_broker_client-pact_broker.json | 79 ++++++++++++++++++- .../service_providers/webhooks_create_spec.rb | 38 +++++++-- 7 files changed, 216 insertions(+), 30 deletions(-) diff --git a/doc/pacts/markdown/Pact Broker Client - Pact Broker.md b/doc/pacts/markdown/Pact Broker Client - Pact Broker.md index 5104d834..ad8fe51c 100644 --- a/doc/pacts/markdown/Pact Broker Client - Pact Broker.md +++ b/doc/pacts/markdown/Pact Broker Client - Pact Broker.md @@ -52,6 +52,8 @@ * [A request to create a webhook with a non-JSON body for a consumer and provider](#a_request_to_create_a_webhook_with_a_non-JSON_body_for_a_consumer_and_provider_given_the_'Pricing_Service'_and_'Condor'_already_exist_in_the_pact-broker) given the 'Pricing Service' and 'Condor' already exist in the pact-broker +* [A request to create a webhook with every possible event type](#a_request_to_create_a_webhook_with_every_possible_event_type_given_the_'Pricing_Service'_and_'Condor'_already_exist_in_the_pact-broker) given the 'Pricing Service' and 'Condor' already exist in the pact-broker + * [A request to get the Pricing Service](#a_request_to_get_the_Pricing_Service_given_the_'Pricing_Service'_already_exists_in_the_pact-broker) given the 'Pricing Service' already exists in the pact-broker * [A request to get the Pricing Service](#a_request_to_get_the_Pricing_Service_given_the_'Pricing_Service'_does_not_exist_in_the_pact-broker) given the 'Pricing Service' does not exist in the pact-broker @@ -734,6 +736,7 @@ Upon receiving **a request to create a global webhook with a JSON body** from Pa "Accept": "application/hal+json" }, "body": { + "description": "a webhook", "events": [ { "name": "contract_content_changed" @@ -783,6 +786,7 @@ Given **'Condor' does not exist in the pact-broker**, upon receiving **a request "Accept": "application/hal+json" }, "body": { + "description": "a webhook", "events": [ { "name": "contract_content_changed" @@ -818,12 +822,13 @@ Given **the 'Pricing Service' and 'Condor' already exist in the pact-broker**, u ```json { "method": "put", - "path": "/webhooks/9999", + "path": "/webhooks/696c5f93-1b7f-44bc-8d03-59440fcaa9a0", "headers": { "Content-Type": "application/json", "Accept": "application/hal+json" }, "body": { + "description": "a webhook", "events": [ { "name": "contract_content_changed" @@ -879,6 +884,7 @@ Given **the 'Pricing Service' and 'Condor' already exist in the pact-broker**, u "Accept": "application/hal+json" }, "body": { + "description": "a webhook", "events": [ { "name": "contract_content_changed" @@ -931,6 +937,7 @@ Given **the 'Pricing Service' and 'Condor' already exist in the pact-broker**, u "Accept": "application/hal+json" }, "body": { + "description": "a webhook", "events": [ { "name": "contract_content_changed" @@ -980,6 +987,7 @@ Upon receiving **a request to create a webhook with a JSON body for a consumer t "Accept": "application/hal+json" }, "body": { + "description": "a webhook", "events": [ { "name": "contract_content_changed" @@ -1031,6 +1039,7 @@ Given **the 'Pricing Service' and 'Condor' already exist in the pact-broker**, u "Accept": "application/hal+json" }, "body": { + "description": "a webhook", "events": [ { "name": "contract_content_changed" @@ -1083,6 +1092,7 @@ Given **the 'Pricing Service' and 'Condor' already exist in the pact-broker**, u "Accept": "application/hal+json" }, "body": { + "description": "a webhook", "events": [ { "name": "contract_content_changed" @@ -1119,6 +1129,68 @@ Pact Broker will respond with: } } ``` + +Given **the 'Pricing Service' and 'Condor' already exist in the pact-broker**, upon receiving **a request to create a webhook with every possible event type** from Pact Broker Client, with +```json +{ + "method": "post", + "path": "/webhooks/provider/Pricing%20Service/consumer/Condor", + "headers": { + "Content-Type": "application/json", + "Accept": "application/hal+json" + }, + "body": { + "description": "a webhook", + "events": [ + { + "name": "contract_content_changed" + }, + { + "name": "contract_published" + }, + { + "name": "provider_verification_published" + }, + { + "name": "provider_verification_succeeded" + }, + { + "name": "provider_verification_failed" + } + ], + "request": { + "url": "https://webhook", + "method": "POST", + "headers": { + "Foo": "bar", + "Bar": "foo" + }, + "body": { + "some": "body" + }, + "username": "username", + "password": "password" + } + } +} +``` +Pact Broker will respond with: +```json +{ + "status": 201, + "headers": { + "Content-Type": "application/hal+json;charset=utf-8" + }, + "body": { + "_links": { + "self": { + "href": "http://localhost:1234/some-url", + "title": "A title" + } + } + } +} +``` Given **the 'Pricing Service' already exists in the pact-broker**, upon receiving **a request to get the Pricing Service** from Pact Broker Client, with ```json @@ -1739,6 +1811,7 @@ Given **the 'Pricing Service' and 'Condor' already exist in the pact-broker**, u "Accept": "application/hal+json" }, "body": { + "description": "a webhook", "events": [ { "name": "contract_content_changed" diff --git a/lib/pact_broker/client/cli/broker.rb b/lib/pact_broker/client/cli/broker.rb index 106399c1..898d42b5 100644 --- a/lib/pact_broker/client/cli/broker.rb +++ b/lib/pact_broker/client/cli/broker.rb @@ -194,11 +194,21 @@ def pact_broker_client_options client_options end - def run_webhook_commands webhook_url - require 'pact_broker/client/webhooks/create' + def parse_webhook_events + events = [] + events << 'contract_content_changed' if options.contract_content_changed + events << 'contract_published' if options.contract_published + events << 'provider_verification_published' if options.provider_verification_published + events << 'provider_verification_succeeded' if options.provider_verification_succeeded + events << 'provider_verification_failed' if options.provider_verification_failed + events + end - if !(options.contract_content_changed || options.provider_verification_published) - raise PactBroker::Client::Error.new("You must select at least one of --contract-content-changed or --provider-verification-published") + def parse_webhook_options(webhook_url) + events = parse_webhook_events + + if events.size == 0 + raise WebhookCreationError.new("You must specify at least one of --contract-content-changed, --contract-published, --provider-verification-published, --provider-verification-succeeded or --provider-verification-failed") end username = options.user ? options.user.split(":", 2).first : nil @@ -212,15 +222,11 @@ def run_webhook_commands webhook_url begin body = File.read(filepath) rescue StandardError => e - raise PactBroker::Client::Error.new("Couldn't read data from file \"#{filepath}\" due to #{e.class} #{e.message}") + raise WebhookCreationError.new("Couldn't read data from file \"#{filepath}\" due to #{e.class} #{e.message}") end end - events = [] - events << 'contract_content_changed' if options.contract_content_changed - events << 'provider_verification_published' if options.provider_verification_published - - params = { + { uuid: options.uuid, http_method: options.request, url: webhook_url, @@ -233,13 +239,15 @@ def run_webhook_commands webhook_url events: events } - begin - result = PactBroker::Client::Webhooks::Create.call(params, options.broker_base_url, pact_broker_client_options) - $stdout.puts result.message - exit(1) unless result.success - rescue PactBroker::Client::Error => e - raise WebhookCreationError, "#{e.class} - #{e.message}" - end + end + + def run_webhook_commands webhook_url + require 'pact_broker/client/webhooks/create' + result = PactBroker::Client::Webhooks::Create.call(parse_webhook_options(webhook_url), options.broker_base_url, pact_broker_client_options) + $stdout.puts result.message + exit(1) unless result.success + rescue PactBroker::Client::Error => e + raise WebhookCreationError, "#{e.class} - #{e.message}" end end end diff --git a/lib/pact_broker/client/cli/custom_thor.rb b/lib/pact_broker/client/cli/custom_thor.rb index 177f5284..5f9e2938 100644 --- a/lib/pact_broker/client/cli/custom_thor.rb +++ b/lib/pact_broker/client/cli/custom_thor.rb @@ -86,8 +86,12 @@ def self.shared_options_for_webhook_commands method_option :broker_username, desc: "Pact Broker basic auth username" method_option :broker_password, aliases: "-p", desc: "Pact Broker basic auth password" method_option :broker_token, aliases: "-k", desc: "Pact Broker bearer token" + method_option :description, desc: "The description of the webhook" method_option :contract_content_changed, type: :boolean, desc: "Trigger this webhook when the pact content changes" + method_option :contract_published, type: :boolean, desc: "Trigger this webhook when a pact is published" method_option :provider_verification_published, type: :boolean, desc: "Trigger this webhook when a provider verification result is published" + method_option :provider_verification_failed, type: :boolean, desc: "Trigger this webhook when a failed provider verification result is published" + method_option :provider_verification_succeeded, type: :boolean, desc: "Trigger this webhook when a successful provider verification result is published" method_option :verbose, aliases: "-v", type: :boolean, default: false, required: false, desc: "Verbose output. Default: false" end end diff --git a/lib/pact_broker/client/webhooks/create.rb b/lib/pact_broker/client/webhooks/create.rb index 03406c5b..94c7fb11 100644 --- a/lib/pact_broker/client/webhooks/create.rb +++ b/lib/pact_broker/client/webhooks/create.rb @@ -71,7 +71,7 @@ def request_body username: params.username, password: params.password } - } + }.tap { |req| req[:description] = params.description if params.description } end def request_body_with_optional_consumer_and_provider diff --git a/spec/lib/pact_broker/client/cli/broker_run_webhook_commands_spec.rb b/spec/lib/pact_broker/client/cli/broker_run_webhook_commands_spec.rb index e9b7bdf7..cbee72c2 100644 --- a/spec/lib/pact_broker/client/cli/broker_run_webhook_commands_spec.rb +++ b/spec/lib/pact_broker/client/cli/broker_run_webhook_commands_spec.rb @@ -78,7 +78,7 @@ module CLI end it "raises an error" do - expect { subject }.to raise_error PactBroker::Client::Error, /You must select at least one/ + expect { subject }.to raise_error WebhookCreationError, /You must specify at least one/ end end @@ -159,8 +159,8 @@ module CLI let(:data) { "@doesnotexist.json" } - it "raises a PactBroker::Client::Error" do - expect { subject }.to raise_error PactBroker::Client::Error, /Couldn't read data from file/ + it "raises a WebhookCreationError" do + expect { subject }.to raise_error WebhookCreationError, /Couldn't read data from file/ end end diff --git a/spec/pacts/pact_broker_client-pact_broker.json b/spec/pacts/pact_broker_client-pact_broker.json index 05aa9db8..18581aab 100644 --- a/spec/pacts/pact_broker_client-pact_broker.json +++ b/spec/pacts/pact_broker_client-pact_broker.json @@ -1260,6 +1260,7 @@ "Accept": "application/hal+json" }, "body": { + "description": "a webhook", "events": [ { "name": "contract_content_changed" @@ -1304,6 +1305,74 @@ } } }, + { + "description": "a request to create a webhook with every possible event type", + "providerState": "the 'Pricing Service' and 'Condor' already exist in the pact-broker", + "request": { + "method": "post", + "path": "/webhooks/provider/Pricing%20Service/consumer/Condor", + "headers": { + "Content-Type": "application/json", + "Accept": "application/hal+json" + }, + "body": { + "description": "a webhook", + "events": [ + { + "name": "contract_content_changed" + }, + { + "name": "contract_published" + }, + { + "name": "provider_verification_published" + }, + { + "name": "provider_verification_succeeded" + }, + { + "name": "provider_verification_failed" + } + ], + "request": { + "url": "https://webhook", + "method": "POST", + "headers": { + "Foo": "bar", + "Bar": "foo" + }, + "body": { + "some": "body" + }, + "username": "username", + "password": "password" + } + } + }, + "response": { + "status": 201, + "headers": { + "Content-Type": "application/hal+json;charset=utf-8" + }, + "body": { + "_links": { + "self": { + "href": "http://localhost:1234/some-url", + "title": "A title" + } + } + }, + "matchingRules": { + "$.body._links.self.href": { + "match": "regex", + "regex": "http:\\/\\/.*" + }, + "$.body._links.self.title": { + "match": "type" + } + } + } + }, { "description": "a request to create a webhook with a non-JSON body for a consumer and provider", "providerState": "the 'Pricing Service' and 'Condor' already exist in the pact-broker", @@ -1315,6 +1384,7 @@ "Accept": "application/hal+json" }, "body": { + "description": "a webhook", "events": [ { "name": "contract_content_changed" @@ -1368,6 +1438,7 @@ "Accept": "application/hal+json" }, "body": { + "description": "a webhook", "events": [ { "name": "contract_content_changed" @@ -1421,6 +1492,7 @@ "Accept": "application/hal+json" }, "body": { + "description": "a webhook", "events": [ { "name": "contract_content_changed" @@ -1488,6 +1560,7 @@ "Accept": "application/hal+json" }, "body": { + "description": "a webhook", "events": [ { "name": "contract_content_changed" @@ -1545,6 +1618,7 @@ "Accept": "application/hal+json" }, "body": { + "description": "a webhook", "events": [ { "name": "contract_content_changed" @@ -1601,6 +1675,7 @@ "Accept": "application/hal+json" }, "body": { + "description": "a webhook", "events": [ { "name": "contract_content_changed" @@ -1658,6 +1733,7 @@ "Accept": "application/hal+json" }, "body": { + "description": "a webhook", "events": [ { "name": "contract_content_changed" @@ -1737,12 +1813,13 @@ "providerState": "the 'Pricing Service' and 'Condor' already exist in the pact-broker", "request": { "method": "put", - "path": "/webhooks/9999", + "path": "/webhooks/696c5f93-1b7f-44bc-8d03-59440fcaa9a0", "headers": { "Content-Type": "application/json", "Accept": "application/hal+json" }, "body": { + "description": "a webhook", "events": [ { "name": "contract_content_changed" diff --git a/spec/service_providers/webhooks_create_spec.rb b/spec/service_providers/webhooks_create_spec.rb index d0831004..274c9825 100644 --- a/spec/service_providers/webhooks_create_spec.rb +++ b/spec/service_providers/webhooks_create_spec.rb @@ -6,8 +6,12 @@ include_context "pact broker" include PactBrokerPactHelperMethods + let(:event_names) { %w{contract_content_changed contract_published provider_verification_published provider_verification_succeeded provider_verification_failed} } + let(:params) do { + description: "a webhook", + events: %w{contract_content_changed}, http_method: "POST", url: "https://webhook", headers: { "Foo" => "bar", "Bar" => "foo"}, @@ -15,8 +19,7 @@ password: "password", body: body, consumer: "Condor", - provider: "Pricing Service", - events: ["contract_content_changed"] + provider: "Pricing Service" }.tap { |it| Pact::Fixture.add_fixture(:create_webhook_params, it) } end @@ -24,10 +27,9 @@ let(:request_body) do { + "description" => "a webhook", "events" => [ - { - "name" => "contract_content_changed" - } + "name" => "contract_content_changed" ], "request" => { "url" => "https://webhook", @@ -84,6 +86,27 @@ end end + context "when a valid webhook with every possible event type is sumbitted" do + before do + params.merge!(events: event_names) + request_body.merge!("events" => event_names.map{ |event_name| { "name" => event_name } }) + + pact_broker + .given("the 'Pricing Service' and 'Condor' already exist in the pact-broker") + .upon_receiving("a request to create a webhook with every possible event type") + .with( + method: :post, + path: '/webhooks/provider/Pricing%20Service/consumer/Condor', + headers: post_request_headers, + body: request_body) + .will_respond_with(success_response) + end + + it "returns a CommandResult with success = true" do + expect(subject.success).to be true + end + end + context "when a valid webhook with an XML body is submitted" do before do request_body["request"]["body"] = body @@ -255,7 +278,7 @@ context "when a uuid is specified" do before do - params.merge!(uuid: '9999') + params.merge!(uuid: uuid) request_body["provider"] = { "name" => "Pricing Service" } request_body["consumer"] = { "name" => "Condor" } mock_pact_broker_index_with_webhook_relation(self) @@ -265,11 +288,12 @@ .given("the 'Pricing Service' and 'Condor' already exist in the pact-broker") .with( method: :put, - path: '/webhooks/9999', + path: "/webhooks/#{uuid}", headers: put_request_headers, body: request_body) .will_respond_with(success_response) end + let(:uuid) { '696c5f93-1b7f-44bc-8d03-59440fcaa9a0' } it "returns a CommandResult with success = true" do expect(subject).to be_a PactBroker::Client::CommandResult