Skip to content

Commit

Permalink
feat(verification webhooks): alter logic to select only the relevant …
Browse files Browse the repository at this point in the history
…webhooks when the pact has changed
  • Loading branch information
bethesque committed Nov 24, 2017
1 parent 9fe8d47 commit ec18943
Show file tree
Hide file tree
Showing 9 changed files with 58 additions and 19 deletions.
2 changes: 1 addition & 1 deletion db/migrations/20171118_create_webhook_events.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from(:webhooks).each do | webhook |
from(:webhook_events).insert(
webhook_id: webhook[:id],
name: 'contract_changed',
name: 'contract_content_changed',
created_at: DateTime.now,
updated_at: DateTime.now
)
Expand Down
9 changes: 9 additions & 0 deletions lib/pact_broker/webhooks/repository.rb
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,15 @@ def find_by_consumer_and_provider consumer, provider
Webhook.where(consumer_id: consumer.id, provider_id: provider.id).collect(&:to_domain)
end

def find_by_consumer_and_provider_and_event_name consumer, provider, event_name
Webhook
.select_all_qualified
.where(consumer_id: consumer.id, provider_id: provider.id)
.join(:webhook_events, { webhook_id: :id })
.where(Sequel[:webhook_events][:name] => event_name)
.collect(&:to_domain)
end

def find_by_consumer_and_provider_existing_at consumer, provider, date_time
Webhook.where(consumer_id: consumer.id, provider_id: provider.id)
.where(Sequel.lit("created_at < ?", date_time))
Expand Down
7 changes: 4 additions & 3 deletions lib/pact_broker/webhooks/service.rb
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
require 'pact_broker/repositories'
require 'pact_broker/logging'
require 'pact_broker/webhooks/job'
require 'base64'
require 'securerandom'
require 'pact_broker/webhooks/job'
require 'pact_broker/webhooks/triggered_webhook'
require 'pact_broker/webhooks/status'
require 'pact_broker/webhooks/webhook_event'

module PactBroker

Expand Down Expand Up @@ -83,8 +84,8 @@ def self.find_by_consumer_and_provider consumer, provider
webhook_repository.find_by_consumer_and_provider consumer, provider
end

def self.execute_webhooks pact
webhooks = webhook_repository.find_by_consumer_and_provider pact.consumer, pact.provider
def self.execute_webhooks pact, event_name = PactBroker::Webhooks::WebhookEvent::DEFAULT_EVENT_NAME
webhooks = webhook_repository.find_by_consumer_and_provider_and_event_name pact.consumer, pact.provider, event_name

if webhooks.any?
run_later(webhooks, pact)
Expand Down
6 changes: 5 additions & 1 deletion lib/pact_broker/webhooks/webhook_event.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,11 @@ module PactBroker
module Webhooks
class WebhookEvent < Sequel::Model

DEFAULT_EVENT_NAME = 'contract_changed'
CONTRACT_CONTENT_CHANGED = 'contract_content_changed'
CONTRACT_VERIFIABLE_CONTENT_CHANGED = 'contract_verifiable_content_changed'
VERIFICATION_PUBLISHED = 'verification_published'
VERIFICATION_STATUS_CHANGED = 'verification_status_changed'
DEFAULT_EVENT_NAME = CONTRACT_CONTENT_CHANGED

dataset_module do
include PactBroker::Repositories::Helpers
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ module Decorators

it "defaults to a single contract_changed event for backwards compatibility" do
expect(parsed_object.events.size).to eq 1
expect(parsed_object.events.first.name).to eq 'contract_changed'
expect(parsed_object.events.first.name).to eq PactBroker::Webhooks::WebhookEvent::DEFAULT_EVENT_NAME
end
end
end
Expand Down
17 changes: 8 additions & 9 deletions spec/lib/pact_broker/api/resources/pact_webhooks_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,14 @@ module Resources
let(:headers) { {'CONTENT_TYPE' => 'application/json'} }
let(:webhook) { double('webhook')}
let(:saved_webhook) { double('saved_webhook')}
let(:provider) { instance_double(PactBroker::Domain::Pacticipant)}
let(:consumer) { instance_double(PactBroker::Domain::Pacticipant)}
let(:provider) { instance_double(PactBroker::Domain::Pacticipant) }
let(:consumer) { instance_double(PactBroker::Domain::Pacticipant) }
let(:webhook_decorator) { instance_double(Decorators::WebhookDecorator, from_json: webhook) }

before do
allow(PactBroker::Pacticipants::Service).to receive(:find_pacticipant_by_name).with("Some Provider").and_return(provider)
allow(PactBroker::Pacticipants::Service).to receive(:find_pacticipant_by_name).with("Some Consumer").and_return(consumer)
allow(Decorators::WebhookDecorator).to receive(:new).and_return(webhook_decorator)
end

describe "GET" do
Expand Down Expand Up @@ -83,6 +85,7 @@ module Resources

context "when the provider is not found" do
let(:provider) { nil }

it "returns a 404 status" do
subject
expect(last_response.status).to eq 404
Expand Down Expand Up @@ -142,10 +145,10 @@ module Resources
context "with valid attributes" do

let(:webhook_response_json) { {some: 'webhook'}.to_json }
let(:decorator) { instance_double(Decorators::WebhookDecorator) }

before do
allow_any_instance_of(Decorators::WebhookDecorator).to receive(:to_json).and_return(webhook_response_json)
allow(webhook_decorator).to receive(:to_json).and_return(webhook_response_json)
end

it "saves the webhook" do
Expand All @@ -169,9 +172,8 @@ module Resources
end

it "generates the JSON response body" do
allow(Decorators::WebhookDecorator).to receive(:new).and_call_original #Deserialise
expect(Decorators::WebhookDecorator).to receive(:new).with(saved_webhook).and_return(decorator) #Serialize
expect(decorator).to receive(:to_json).with(user_options: { base_url: 'http://example.org' })
expect(Decorators::WebhookDecorator).to receive(:new).with(saved_webhook).and_return(webhook_decorator)
expect(webhook_decorator).to receive(:to_json).with(user_options: { base_url: 'http://example.org' })
subject
end

Expand All @@ -180,10 +182,7 @@ module Resources
expect(last_response.body).to eq webhook_response_json
end
end

end

end
end

end
23 changes: 23 additions & 0 deletions spec/lib/pact_broker/webhooks/repository_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,29 @@ module Webhooks
end
end

describe "find_by_consumer_and_provider_and_event_name" do
let(:test_data_builder) { TestDataBuilder.new }
subject { Repository.new.find_by_consumer_and_provider_and_event_name test_data_builder.consumer, test_data_builder.provider, 'something_happened' }

context "when a webhook exists with a matching consumer and provider and event name" do

before do
test_data_builder
.create_consumer("Consumer")
.create_provider("Another Provider")
.create_webhook
.create_provider("Provider")
.create_webhook(uuid: '1', events: [{ name: 'something_happened' }])
.create_webhook(uuid: '2', events: [{ name: 'something_happened' }])
.create_webhook(uuid: '3', events: [{ name: 'something_else_happened' }])
end

it "returns an array of webhooks" do
expect(subject.collect(&:uuid).sort).to eq ['1', '2']
end
end
end

describe "create_triggered_webhook" do
before do
td.create_consumer
Expand Down
8 changes: 5 additions & 3 deletions spec/lib/pact_broker/webhooks/service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,15 @@ module Webhooks
let(:triggered_webhook) { instance_double(PactBroker::Webhooks::TriggeredWebhook) }

before do
allow_any_instance_of(PactBroker::Webhooks::Repository).to receive(:find_by_consumer_and_provider).and_return(webhooks)
allow_any_instance_of(PactBroker::Webhooks::Repository).to receive(:find_by_consumer_and_provider_and_event_name).and_return(webhooks)
allow_any_instance_of(PactBroker::Webhooks::Repository).to receive(:create_triggered_webhook).and_return(triggered_webhook)
allow(Job).to receive(:perform_async)
end

subject { Service.execute_webhooks pact }

it "finds the webhooks" do
expect_any_instance_of(PactBroker::Webhooks::Repository).to receive(:find_by_consumer_and_provider).with(consumer, provider)
expect_any_instance_of(PactBroker::Webhooks::Repository).to receive(:find_by_consumer_and_provider_and_event_name).with(consumer, provider, PactBroker::Webhooks::WebhookEvent::DEFAULT_EVENT_NAME)
subject
end

Expand Down Expand Up @@ -126,12 +126,14 @@ module Webhooks
to_return(:status => 200)
end

let(:events) { [{ name: PactBroker::Webhooks::WebhookEvent::DEFAULT_EVENT_NAME }] }

let(:pact) do
td.create_consumer
.create_provider
.create_consumer_version
.create_pact
.create_webhook(method: 'GET', url: 'http://example.org')
.create_webhook(method: 'GET', url: 'http://example.org', events: events)
.and_return(:pact)
end

Expand Down
3 changes: 2 additions & 1 deletion spec/support/test_data_builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,8 @@ def revise_pact json_content = nil

def create_webhook params = {}
uuid = params[:uuid] || PactBroker::Webhooks::Service.next_uuid
events = (params[:events] || [{ name: 'pact_changed' }]).collect{ |e| PactBroker::Webhooks::WebhookEvent.new(e) }
event_params = params[:events] || [{ name: PactBroker::Webhooks::WebhookEvent::DEFAULT_EVENT_NAME }]
events = event_params.collect{ |e| PactBroker::Webhooks::WebhookEvent.new(e) }
default_params = { method: 'POST', url: 'http://example.org', headers: {'Content-Type' => 'application/json'}}
request = PactBroker::Domain::WebhookRequest.new(default_params.merge(params))
@webhook = PactBroker::Webhooks::Repository.new.create uuid, PactBroker::Domain::Webhook.new(request: request, events: events), @consumer, @provider
Expand Down

0 comments on commit ec18943

Please sign in to comment.