diff --git a/db/migrations/20171117_create_webhook_events.rb b/db/migrations/20171117_create_webhook_events.rb new file mode 100644 index 000000000..84a633895 --- /dev/null +++ b/db/migrations/20171117_create_webhook_events.rb @@ -0,0 +1,14 @@ +require_relative 'migration_helper' + +Sequel.migration do + change do + create_table(:webhook_events, charset: 'utf8') do + primary_key :id + foreign_key :webhook_id, :webhooks, on_delete: :cascade + String :name + DateTime :created_at, null: false + DateTime :updated_at, null: false + index [:id, :name], unique: true, name: 'uq_webhook_id_name' + end + end +end diff --git a/db/migrations/20171118_create_webhook_events.rb b/db/migrations/20171118_create_webhook_events.rb new file mode 100644 index 000000000..acb4ea90c --- /dev/null +++ b/db/migrations/20171118_create_webhook_events.rb @@ -0,0 +1,18 @@ +require_relative 'migration_helper' + +Sequel.migration do + up do + from(:webhooks).each do | webhook | + from(:webhook_events).insert( + webhook_id: webhook[:id], + name: 'contract_changed', + created_at: DateTime.now, + updated_at: DateTime.now + ) + end + end + + down do + + end +end diff --git a/lib/pact_broker/api/contracts/webhook_contract.rb b/lib/pact_broker/api/contracts/webhook_contract.rb index b53d0b0f2..0d9dfabf8 100644 --- a/lib/pact_broker/api/contracts/webhook_contract.rb +++ b/lib/pact_broker/api/contracts/webhook_contract.rb @@ -5,7 +5,6 @@ module PactBroker module Api module Contracts class WebhookContract < Reform::Form - property :request validation do configure do @@ -13,6 +12,7 @@ class WebhookContract < Reform::Form end required(:request).filled + required(:events).maybe(min_size?: 1) end property :request do @@ -39,6 +39,15 @@ def valid_url?(value) required(:url).filled(:valid_url?) end end + + collection :events do + property :name + + validation do + required(:name).filled + end + end + end end end diff --git a/lib/pact_broker/api/decorators/webhook_decorator.rb b/lib/pact_broker/api/decorators/webhook_decorator.rb index d9b3b32a0..151acdc92 100644 --- a/lib/pact_broker/api/decorators/webhook_decorator.rb +++ b/lib/pact_broker/api/decorators/webhook_decorator.rb @@ -2,19 +2,24 @@ require 'pact_broker/api/decorators/webhook_request_decorator' require 'pact_broker/api/decorators/timestamps' require 'pact_broker/domain/webhook_request' +require 'pact_broker/webhooks/webhook_event' require 'pact_broker/api/decorators/basic_pacticipant_decorator' +require_relative 'pact_pacticipant_decorator' +require_relative 'pacticipant_decorator' module PactBroker module Api module Decorators class WebhookDecorator < BaseDecorator - property :request, :class => PactBroker::Domain::WebhookRequest, :extend => WebhookRequestDecorator + class WebhookEventDecorator < BaseDecorator + property :name + end - include Timestamps + property :request, :class => PactBroker::Domain::WebhookRequest, extend: WebhookRequestDecorator + collection :events, :class => PactBroker::Webhooks::WebhookEvent, extend: WebhookEventDecorator - property :consumer, :extend => PactBroker::Api::Decorators::BasicPacticipantDecorator, :embedded => true, writeable: false - property :provider, :extend => PactBroker::Api::Decorators::BasicPacticipantDecorator, :embedded => true, writeable: false + include Timestamps link :self do | options | { @@ -31,9 +36,26 @@ class WebhookDecorator < BaseDecorator } end + + link :'pb:consumer' do | options | + { + title: "Consumer", + name: represented.consumer.name, + href: pacticipant_url(options.fetch(:base_url), represented.consumer) + } + end + + link :'pb:provider' do | options | + { + title: "Provider", + name: represented.provider.name, + href: pacticipant_url(options.fetch(:base_url), represented.provider) + } + end + link :'pb:pact-webhooks' do | options | { - title: "All webhooks for the pact between #{represented.consumer.name} and #{represented.provider.name}", + title: "All webhooks for consumer #{represented.consumer.name} and provider #{represented.provider.name}", href: webhooks_for_pact_url(represented.consumer, represented.provider, options[:base_url]) } end diff --git a/lib/pact_broker/domain/webhook.rb b/lib/pact_broker/domain/webhook.rb index c044a4e7a..54611cf83 100644 --- a/lib/pact_broker/domain/webhook.rb +++ b/lib/pact_broker/domain/webhook.rb @@ -10,7 +10,7 @@ class Webhook include Messages include Logging - attr_accessor :uuid, :consumer, :provider, :request, :created_at, :updated_at + attr_accessor :uuid, :consumer, :provider, :request, :created_at, :updated_at, :events attr_reader :attributes def initialize attributes = {} @@ -19,6 +19,7 @@ def initialize attributes = {} @request = attributes[:request] @consumer = attributes[:consumer] @provider = attributes[:provider] + @events = attributes[:events] @created_at = attributes[:created_at] @updated_at = attributes[:updated_at] end diff --git a/lib/pact_broker/webhooks/repository.rb b/lib/pact_broker/webhooks/repository.rb index efaffa309..6c7bfac89 100644 --- a/lib/pact_broker/webhooks/repository.rb +++ b/lib/pact_broker/webhooks/repository.rb @@ -3,6 +3,7 @@ require 'pact_broker/domain/pacticipant' require 'pact_broker/db' require 'pact_broker/webhooks/webhook' +require 'pact_broker/webhooks/webhook_event' require 'pact_broker/webhooks/triggered_webhook' require 'pact_broker/webhooks/latest_triggered_webhook' require 'pact_broker/webhooks/execution' @@ -21,6 +22,9 @@ def create uuid, webhook, consumer, provider webhook.request.headers.each_pair do | name, value | db_webhook.add_header PactBroker::Webhooks::WebhookHeader.from_domain(name, value, db_webhook.id) end + (webhook.events || []).each do | webhook_event | + db_webhook.add_event(webhook_event) + end find_by_uuid db_webhook.uuid end @@ -32,9 +36,13 @@ def update_by_uuid uuid, webhook existing_webhook = Webhook.find(uuid: uuid) existing_webhook.update_from_domain(webhook).save existing_webhook.headers.collect(&:delete) + existing_webhook.events.collect(&:delete) webhook.request.headers.each_pair do | name, value | existing_webhook.add_header PactBroker::Webhooks::WebhookHeader.from_domain(name, value, existing_webhook.id) end + (webhook.events || []).each do | webhook_event | + existing_webhook.add_event(webhook_event) + end find_by_uuid uuid end diff --git a/lib/pact_broker/webhooks/webhook.rb b/lib/pact_broker/webhooks/webhook.rb index e1c17769d..4165ae60b 100644 --- a/lib/pact_broker/webhooks/webhook.rb +++ b/lib/pact_broker/webhooks/webhook.rb @@ -9,6 +9,8 @@ class Webhook < Sequel::Model set_primary_key :id associate(:many_to_one, :provider, :class => "PactBroker::Domain::Pacticipant", :key => :provider_id, :primary_key => :id) associate(:many_to_one, :consumer, :class => "PactBroker::Domain::Pacticipant", :key => :consumer_id, :primary_key => :id) + + one_to_many :events, :class => "PactBroker::Webhooks::WebhookEvent", :reciprocal => :webhook one_to_many :headers, :class => "PactBroker::Webhooks::WebhookHeader", :reciprocal => :webhook dataset_module do @@ -41,6 +43,7 @@ def to_domain uuid: uuid, consumer: consumer, provider: provider, + events: events, request: Domain::WebhookRequest.new(request_attributes), created_at: created_at, updated_at: updated_at) diff --git a/lib/pact_broker/webhooks/webhook_event.rb b/lib/pact_broker/webhooks/webhook_event.rb new file mode 100644 index 000000000..ebfecfa54 --- /dev/null +++ b/lib/pact_broker/webhooks/webhook_event.rb @@ -0,0 +1,16 @@ +require 'sequel' +require 'pact_broker/repositories/helpers' + +module PactBroker + module Webhooks + class WebhookEvent < Sequel::Model + + dataset_module do + include PactBroker::Repositories::Helpers + end + + end + + WebhookEvent.plugin :timestamps, update_on_create: true + end +end diff --git a/spec/features/create_webhook_spec.rb b/spec/features/create_webhook_spec.rb index c0de0e4ae..37325a8f1 100644 --- a/spec/features/create_webhook_spec.rb +++ b/spec/features/create_webhook_spec.rb @@ -3,7 +3,7 @@ describe "Creating a webhook" do before do - TestDataBuilder.new.create_pact_with_hierarchy("Some Consumer", "1", "Some Provider").and_return(:pact) + TestDataBuilder.new.create_pact_with_hierarchy("Some Consumer", "1", "Some Provider") end let(:path) { "/webhooks/provider/Some%20Provider/consumer/Some%20Consumer" } @@ -38,6 +38,9 @@ let(:webhook_hash) do { + events: [{ + name: 'something_happened' + }], request: { method: 'POST', url: 'http://example.org', diff --git a/spec/fixtures/webhook_valid.json b/spec/fixtures/webhook_valid.json index 487eff303..8f224ebc0 100644 --- a/spec/fixtures/webhook_valid.json +++ b/spec/fixtures/webhook_valid.json @@ -1,4 +1,7 @@ { + "events": [{ + "name": "something_happened" + }], "request": { "method": "POST", "url": "http://some.url", 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 d2d16413f..87b934ad9 100644 --- a/spec/lib/pact_broker/api/contracts/webhook_contract_spec.rb +++ b/spec/lib/pact_broker/api/contracts/webhook_contract_spec.rb @@ -37,6 +37,26 @@ def valid_webhook_with end end + context "with no events defined" do + let(:json) { {}.to_json } + + it "contains an error for missing request, I wish I could work out how not to have the second error" do + expect(subject.errors[:events]).to eq ["is missing", "size cannot be less than 1"] + end + end + + context "with empty events" do + let(:json) do + valid_webhook_with do |hash| + hash['events'] = [] + end + end + + it "contains an error for missing request" do + expect(subject.errors[:events]).to eq ["size cannot be less than 1"] + end + end + context "with no method" do let(:json) do valid_webhook_with do |hash| 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 714f60977..4855e168a 100644 --- a/spec/lib/pact_broker/api/decorators/webhook_decorator_spec.rb +++ b/spec/lib/pact_broker/api/decorators/webhook_decorator_spec.rb @@ -21,6 +21,7 @@ module Decorators let(:consumer) { Domain::Pacticipant.new(name: 'Consumer') } let(:provider) { Domain::Pacticipant.new(name: 'Provider') } + let(:event) { Webhooks::WebhookEvent.new(name: 'something_happened') } let(:created_at) { DateTime.now } let(:updated_at) { created_at + 1 } @@ -30,6 +31,7 @@ module Decorators uuid: 'some-uuid', consumer: consumer, provider: provider, + events: [event], created_at: created_at, updated_at: updated_at ) @@ -44,26 +46,14 @@ module Decorators expect(parsed_json[:request]).to eq request end - it 'includes an embedded consumer' do - expect(parsed_json[:_embedded][:consumer]).to eq ({ - name: 'Consumer', - _links: { - self: { - href: 'http://example.org/pacticipants/Consumer' - } - } - }) + 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' end - it 'includes an embedded provider' do - expect(parsed_json[:_embedded][:provider]).to eq ({ - name: 'Provider', - _links: { - self: { - href: 'http://example.org/pacticipants/Provider' - } - } - }) + it 'includes a link to the provider' do + expect(parsed_json[:_links][:'pb:provider'][:name]).to eq 'Provider' + expect(parsed_json[:_links][:'pb:provider'][:href]).to eq 'http://example.org/pacticipants/Provider' end it 'includes a link to itself' do @@ -83,6 +73,10 @@ module Decorators expect(parsed_json[:_links][:'pb:execute'][:href]).to eq 'http://example.org/webhooks/some-uuid/execute' end + it 'includes the events' do + expect(parsed_json[:events].first).to eq name: 'something_happened' + end + it 'includes timestamps' do expect(parsed_json[:createdAt]).to eq created_at.xmlschema expect(parsed_json[:updatedAt]).to eq updated_at.xmlschema @@ -97,7 +91,8 @@ module Decorators end describe 'from_json' do - let(:hash) { { request: request } } + let(:hash) { { request: request, events: [event] } } + let(:event) { {name: 'something_happened'} } let(:json) { hash.to_json } let(:webhook) { Domain::Webhook.new } let(:parsed_object) { subject.from_json(json) } @@ -117,6 +112,10 @@ module Decorators it 'parses the request body' do expect(parsed_object.request.body).to eq 'some' => 'body' end + + it 'parses the events' do + expect(parsed_object.events.first.name).to eq 'something_happened' + end end end end diff --git a/spec/lib/pact_broker/webhooks/repository_spec.rb b/spec/lib/pact_broker/webhooks/repository_spec.rb index bd3e2f05d..bdca3c1ec 100644 --- a/spec/lib/pact_broker/webhooks/repository_spec.rb +++ b/spec/lib/pact_broker/webhooks/repository_spec.rb @@ -18,22 +18,30 @@ module Webhooks password: 'password', body: body) end - let(:webhook) { Domain::Webhook.new(request: request)} + let(:event) do + PactBroker::Webhooks::WebhookEvent.new(name: 'something_happened') + end + let(:events) { [event]} + let(:webhook) { Domain::Webhook.new(request: request, events: events)} let(:test_data_builder) { TestDataBuilder.new } let(:consumer) { test_data_builder.create_pacticipant 'Consumer'; test_data_builder.pacticipant} let(:provider) { test_data_builder.create_pacticipant 'Provider'; test_data_builder.pacticipant} let(:uuid) { 'the-uuid' } let(:created_webhook_record) { ::DB::PACT_BROKER_DB[:webhooks].order(:id).last } let(:created_headers) { ::DB::PACT_BROKER_DB[:webhook_headers].where(webhook_id: created_webhook_record[:id]).order(:name).all } - let(:expected_webhook_record) { { - :uuid=>"the-uuid", - :method=>"post", - :url=>"http://example.org", - :username => 'username', - :password => "cGFzc3dvcmQ=", - :body=>body.to_json, - :consumer_id=> consumer.id, - :provider_id=> provider.id } } + let(:created_events) { ::DB::PACT_BROKER_DB[:webhook_events].where(webhook_id: created_webhook_record[:id]).order(:name).all } + let(:expected_webhook_record) do + { + uuid: "the-uuid", + method: "post", + url: "http://example.org", + username: 'username', + password: "cGFzc3dvcmQ=", + body: body.to_json, + consumer_id: consumer.id, + provider_id: provider.id + } + end describe "#create" do @@ -53,6 +61,10 @@ module Webhooks expect(created_headers.last[:value]).to eq "application/json" end + it "saves the webhook events" do + expect(subject.events.first[:name]).to eq "something_happened" + end + end describe "delete_by_uuid" do @@ -193,6 +205,7 @@ module Webhooks let(:td) { TestDataBuilder.new } let(:old_webhook_params) do { + events: [{ name: 'something' }], uuid: uuid, method: 'POST', url: 'http://example.org', @@ -210,13 +223,16 @@ module Webhooks headers: {'Content-Type' => 'text/plain'} } end + let(:new_event) do + PactBroker::Webhooks::WebhookEvent.new(name: 'something_else') + end before do td.create_consumer .create_provider .create_webhook(old_webhook_params) end let(:new_webhook) do - PactBroker::Domain::Webhook.new(request: PactBroker::Domain::WebhookRequest.new(new_webhook_params)) + PactBroker::Domain::Webhook.new(events: [new_event], request: PactBroker::Domain::WebhookRequest.new(new_webhook_params)) end subject { Repository.new.update_by_uuid uuid, new_webhook } @@ -230,6 +246,7 @@ module Webhooks expect(updated_webhook.request.headers).to eq 'Content-Type' => 'text/plain' expect(updated_webhook.request.username).to eq nil expect(updated_webhook.request.password).to eq nil + expect(updated_webhook.events.first.name).to eq 'something_else' end end diff --git a/spec/support/test_data_builder.rb b/spec/support/test_data_builder.rb index a7ba8c74f..479f9028a 100644 --- a/spec/support/test_data_builder.rb +++ b/spec/support/test_data_builder.rb @@ -200,9 +200,10 @@ def revise_pact json_content = nil def create_webhook params = {} uuid = params[:uuid] || PactBroker::Webhooks::Service.next_uuid - default_params = {method: 'POST', url: 'http://example.org', headers: {'Content-Type' => 'application/json'}} + events = (params[:events] || [{ name: 'pact_changed' }]).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), @consumer, @provider + @webhook = PactBroker::Webhooks::Repository.new.create uuid, PactBroker::Domain::Webhook.new(request: request, events: events), @consumer, @provider self end diff --git a/tasks/database.rb b/tasks/database.rb index c06b7d49b..77051f53c 100644 --- a/tasks/database.rb +++ b/tasks/database.rb @@ -9,7 +9,7 @@ module PactBroker module Database - TABLES = [:labels, :webhook_executions, :triggered_webhooks, :config, :pacts, :pact_version_contents, :tags, :verifications, :pact_publications, :pact_versions, :webhook_headers, :webhooks, :versions, :pacticipants].freeze + TABLES = [:labels, :webhook_events, :webhook_executions, :triggered_webhooks, :config, :pacts, :pact_version_contents, :tags, :verifications, :pact_publications, :pact_versions, :webhook_headers, :webhooks, :versions, :pacticipants].freeze extend self