diff --git a/lib/pact_broker/api/contracts/webhook_contract.rb b/lib/pact_broker/api/contracts/webhook_contract.rb index 463bd9b65..aab3181c5 100644 --- a/lib/pact_broker/api/contracts/webhook_contract.rb +++ b/lib/pact_broker/api/contracts/webhook_contract.rb @@ -26,10 +26,45 @@ def validate(*) config.messages_file = File.expand_path("../../../locale/en.yml", __FILE__) end + optional(:consumer) + optional(:provider) required(:request).filled optional(:events).maybe(min_size?: 1) end + property :consumer do + property :name + + validation do + configure do + config.messages_file = File.expand_path("../../../locale/en.yml", __FILE__) + + def pacticipant_exists?(name) + !!PactBroker::Pacticipants::Service.find_pacticipant_by_name(name) + end + end + + required(:name).filled(:pacticipant_exists?) + end + + end + + property :provider do + property :name + + validation do + configure do + config.messages_file = File.expand_path("../../../locale/en.yml", __FILE__) + + def pacticipant_exists?(name) + !!PactBroker::Pacticipants::Service.find_pacticipant_by_name(name) + end + end + + required(:name).filled(:pacticipant_exists?) + end + end + property :request do property :url property :http_method diff --git a/lib/pact_broker/api/decorators/webhook_decorator.rb b/lib/pact_broker/api/decorators/webhook_decorator.rb index de79b3fde..538e5841a 100644 --- a/lib/pact_broker/api/decorators/webhook_decorator.rb +++ b/lib/pact_broker/api/decorators/webhook_decorator.rb @@ -16,6 +16,14 @@ class WebhookEventDecorator < BaseDecorator property :name end + property :consumer, :class => PactBroker::Domain::Pacticipant do + property :name + end + + property :provider, :class => PactBroker::Domain::Pacticipant do + property :name + end + property :request, :class => PactBroker::Domain::WebhookRequest, extend: WebhookRequestDecorator collection :events, :class => PactBroker::Webhooks::WebhookEvent, extend: WebhookEventDecorator diff --git a/lib/pact_broker/locale/en.yml b/lib/pact_broker/locale/en.yml index c75047a8e..c982aae08 100644 --- a/lib/pact_broker/locale/en.yml +++ b/lib/pact_broker/locale/en.yml @@ -8,6 +8,7 @@ en: name_in_path_matches_name_in_pact?: "does not match %{left} name in path ('%{right}')." valid_consumer_version_number?: "Consumer version number '%{value}' cannot be parsed to a version number. The expected format (unless this configuration has been overridden) is a semantic version. eg. 1.3.0 or 2.0.4.rc1" non_templated_host?: "cannot have a template parameter in the host" + pacticipant_exists?: "does not match an existing pacticipant" pact_broker: messages: diff --git a/spec/fixtures/webhook_valid_with_pacticipants.json b/spec/fixtures/webhook_valid_with_pacticipants.json new file mode 100644 index 000000000..5a8688823 --- /dev/null +++ b/spec/fixtures/webhook_valid_with_pacticipants.json @@ -0,0 +1,23 @@ +{ + "events": [{ + "name": "something_happened" + }], + "consumer": { + "name": "Foo" + }, + "provider": { + "name": "Bar" + }, + "request": { + "method": "POST", + "url": "HTTPS://some.url", + "username": "username", + "password": "password", + "headers": { + "Accept": "application/json" + }, + "body": { + "a" : "body" + } + } +} diff --git a/spec/lib/pact_broker/api/contracts/webhook_contract_spec.rb b/spec/lib/pact_broker/api/contracts/webhook_contract_spec.rb index 761b790e9..210124fc3 100644 --- a/spec/lib/pact_broker/api/contracts/webhook_contract_spec.rb +++ b/spec/lib/pact_broker/api/contracts/webhook_contract_spec.rb @@ -6,14 +6,16 @@ module PactBroker module Api module Contracts describe WebhookContract do - let(:json) { load_fixture 'webhook_valid.json' } + let(:json) { load_fixture 'webhook_valid_with_pacticipants.json' } let(:hash) { JSON.parse(json) } let(:webhook) { PactBroker::Api::Decorators::WebhookDecorator.new(Domain::Webhook.new).from_json(json) } let(:subject) { WebhookContract.new(webhook) } let(:matching_hosts) { ['foo'] } + let(:consumer) { double("consumer") } + let(:provider) { double("provider") } def valid_webhook_with - hash = load_json_fixture 'webhook_valid.json' + hash = load_json_fixture 'webhook_valid_with_pacticipants.json' yield hash hash.to_json end @@ -23,6 +25,8 @@ def valid_webhook_with PactBroker.configuration.webhook_http_method_whitelist = webhook_http_method_whitelist PactBroker.configuration.webhook_host_whitelist = webhook_host_whitelist allow(PactBroker::Webhooks::CheckHostWhitelist).to receive(:call).and_return(whitelist_matches) + allow(PactBroker::Pacticipants::Service).to receive(:find_pacticipant_by_name).with("Foo").and_return(consumer) + allow(PactBroker::Pacticipants::Service).to receive(:find_pacticipant_by_name).with("Bar").and_return(provider) subject.validate(hash) end @@ -36,6 +40,98 @@ def valid_webhook_with end end + context "with a nil consumer name" do + let(:json) do + valid_webhook_with do |hash| + hash['consumer']['name'] = nil + end + end + + it "contains an error" do + expect(subject.errors[:'consumer.name']).to eq ["can't be blank"] + end + end + + context "with no consumer name key" do + let(:json) do + valid_webhook_with do |hash| + hash['consumer'].delete('name') + end + end + + # I'd prefer this to be "is missing". Isn't the whole point of dry validation + # that you can distingush between keys being missing and values being missing? FFS. + it "contains an error" do + expect(subject.errors[:'consumer.name']).to eq ["can't be blank"] + end + end + + context "with no consumer" do + let(:json) do + valid_webhook_with do |hash| + hash.delete('consumer') + end + end + + it "contains no errors" do + expect(subject.errors).to be_empty + end + end + + context "with a consumer name that doesn't match any existing consumer" do + let(:consumer) { nil } + + it "contains no errors" do + expect(subject.errors[:'consumer.name']).to eq ["does not match an existing pacticipant"] + end + end + + context "with a nil provider name" do + let(:json) do + valid_webhook_with do |hash| + hash['provider']['name'] = nil + end + end + + it "contains an error" do + expect(subject.errors[:'provider.name']).to eq ["can't be blank"] + end + end + + context "with no provider name key" do + let(:json) do + valid_webhook_with do |hash| + hash['provider'].delete('name') + end + end + + # I'd prefer this to be "is missing". Isn't the whole point of dry validation + # that you can distingush between keys being missing and values being missing? FFS. + it "contains an error" do + expect(subject.errors[:'provider.name']).to eq ["can't be blank"] + end + end + + context "with no provider" do + let(:json) do + valid_webhook_with do |hash| + hash.delete('provider') + end + end + + it "contains no errors" do + expect(subject.errors).to be_empty + end + end + + context "with a provider name that doesn't match any existing provider" do + let(:provider) { nil } + + it "contains no errors" do + expect(subject.errors[:'provider.name']).to eq ["does not match an existing pacticipant"] + end + end + context "with no request defined" do let(:json) { {}.to_json } diff --git a/spec/lib/pact_broker/api/decorators/webhook_decorator_spec.rb b/spec/lib/pact_broker/api/decorators/webhook_decorator_spec.rb index c347c3268..c14c5bd45 100644 --- a/spec/lib/pact_broker/api/decorators/webhook_decorator_spec.rb +++ b/spec/lib/pact_broker/api/decorators/webhook_decorator_spec.rb @@ -46,6 +46,14 @@ module Decorators expect(parsed_json[:request]).to eq request end + it 'includes the consumer' do + expect(parsed_json[:consumer]).to eq name: "Consumer" + end + + it 'includes the provider' do + expect(parsed_json[:provider]).to eq name: "Provider" + end + it 'includes a link to the consumer' do expect(parsed_json[:_links][:'pb:consumer'][:name]).to eq 'Consumer' expect(parsed_json[:_links][:'pb:consumer'][:href]).to eq 'http://example.org/pacticipants/Consumer'