diff --git a/db/ddl_statements/latest_triggered_webhooks.rb b/db/ddl_statements/latest_triggered_webhooks.rb new file mode 100644 index 000000000..3a1fbbe71 --- /dev/null +++ b/db/ddl_statements/latest_triggered_webhooks.rb @@ -0,0 +1,50 @@ +# These views find the latest triggered webhook for each webhook/consumer/provider +# by finding the latest execution date for each webhook +# then taking the row with the max ID in case there there are two +# triggered webhooks for the same UUID and same creation date +# Not sure if this is strictly necessary to do the extra step, but better to be +# safe than sorry. +# I probably could just take the max ID for each webhook/consumer/provider, but +# something in my head says that +# relying on the primary key for order is not a good idea, even though +# according to the SQL it should be fine. +def latest_triggered_webhook_creation_dates_v2 + "select webhook_uuid, consumer_id, provider_id, max(created_at) as latest_triggered_webhook_created_at + from triggered_webhooks + group by webhook_uuid, consumer_id, provider_id" +end + +# Ignore ltwcd.latest_triggered_webhook_created_at, it's there because postgres doesn't allow you to modify +# the names and types of columns in a view +def latest_triggered_webhook_ids_v2 + "select tw.webhook_uuid, tw.consumer_id, tw.provider_id, ltwcd.latest_triggered_webhook_created_at, max(tw.id) as latest_triggered_webhook_id + from latest_triggered_webhook_creation_dates ltwcd + inner join triggered_webhooks tw + on tw.consumer_id = ltwcd.consumer_id + and tw.provider_id = ltwcd.provider_id + and tw.webhook_uuid = ltwcd.webhook_uuid + and tw.created_at = ltwcd.latest_triggered_webhook_created_at + group by tw.webhook_uuid, tw.consumer_id, tw.provider_id, ltwcd.latest_triggered_webhook_created_at" +end + +def latest_triggered_webhooks_v2 + "select tw.* + from triggered_webhooks tw + inner join latest_triggered_webhook_ids ltwi + on tw.consumer_id = ltwi.consumer_id + and tw.provider_id = ltwi.provider_id + and tw.webhook_uuid = ltwi.webhook_uuid + and tw.id = ltwi.latest_triggered_webhook_id" +end + +##### + +# screw dates, just use IDs. +def latest_triggered_webhooks_v3 + "select tw.* + from triggered_webhooks tw + inner join (select max(id) as max_id + from triggered_webhooks + group by webhook_uuid, consumer_id, provider_id, event_name) latest_ids + on latest_ids.max_id = tw.id" +end diff --git a/db/migrations/20180119_update_latest_triggered_webhooks.rb b/db/migrations/20180119_update_latest_triggered_webhooks.rb index 5f393c769..e4c052f82 100644 --- a/db/migrations/20180119_update_latest_triggered_webhooks.rb +++ b/db/migrations/20180119_update_latest_triggered_webhooks.rb @@ -1,44 +1,11 @@ +require_relative '../ddl_statements/latest_triggered_webhooks' + Sequel.migration do up do - # These views find the latest triggered webhook for each webhook/consumer/provider - # by finding the latest execution date for each webhook - # then taking the row with the max ID in case there there are two - # triggered webhooks for the same UUID and same creation date - # Not sure if this is strictly necessary to do the extra step, but better to be - # safe than sorry. - # I probably could just take the max ID for each webhook/consumer/provider, but - # something in my head says that - # relying on the primary key for order is not a good idea, even though - # according to the SQL it should be fine. - - create_or_replace_view(:latest_triggered_webhook_creation_dates, - "select webhook_uuid, consumer_id, provider_id, max(created_at) as latest_triggered_webhook_created_at - from triggered_webhooks - group by webhook_uuid, consumer_id, provider_id" - ) - - # Ignore ltwcd.latest_triggered_webhook_created_at, it's there because postgres doesn't allow you to modify - # the names and types of columns in a view - create_or_replace_view(:latest_triggered_webhook_ids, - "select tw.webhook_uuid, tw.consumer_id, tw.provider_id, ltwcd.latest_triggered_webhook_created_at, max(tw.id) as latest_triggered_webhook_id - from latest_triggered_webhook_creation_dates ltwcd - inner join triggered_webhooks tw - on tw.consumer_id = ltwcd.consumer_id - and tw.provider_id = ltwcd.provider_id - and tw.webhook_uuid = ltwcd.webhook_uuid - and tw.created_at = ltwcd.latest_triggered_webhook_created_at - group by tw.webhook_uuid, tw.consumer_id, tw.provider_id, ltwcd.latest_triggered_webhook_created_at" - ) - create_or_replace_view(:latest_triggered_webhooks, - "select tw.* - from triggered_webhooks tw - inner join latest_triggered_webhook_ids ltwi - on tw.consumer_id = ltwi.consumer_id - and tw.provider_id = ltwi.provider_id - and tw.webhook_uuid = ltwi.webhook_uuid - and tw.id = ltwi.latest_triggered_webhook_id" - ) + create_or_replace_view(:latest_triggered_webhook_creation_dates, latest_triggered_webhook_creation_dates_v2) + create_or_replace_view(:latest_triggered_webhook_ids, latest_triggered_webhook_ids_v2) + create_or_replace_view(:latest_triggered_webhooks, latest_triggered_webhooks_v2) end down do diff --git a/db/migrations/20200930_update_latest_triggered_webhooks.rb b/db/migrations/20200930_update_latest_triggered_webhooks.rb new file mode 100644 index 000000000..6ae2661c1 --- /dev/null +++ b/db/migrations/20200930_update_latest_triggered_webhooks.rb @@ -0,0 +1,14 @@ +require_relative '../ddl_statements/latest_triggered_webhooks' + +Sequel.migration do + up do + # TODO + # drop_view(:latest_triggered_webhook_ids) + # drop_view(:latest_triggered_webhook_creation_dates) + create_or_replace_view(:latest_triggered_webhooks, latest_triggered_webhooks_v3) + end + + down do + create_or_replace_view(:latest_triggered_webhooks, latest_triggered_webhooks_v2) + end +end diff --git a/lib/pact_broker/test/test_data_builder.rb b/lib/pact_broker/test/test_data_builder.rb index fc6feb866..94682b85d 100644 --- a/lib/pact_broker/test/test_data_builder.rb +++ b/lib/pact_broker/test/test_data_builder.rb @@ -293,8 +293,9 @@ def create_consumer_webhook parameters = {} def create_triggered_webhook params = {} params.delete(:comment) trigger_uuid = params[:trigger_uuid] || webhook_service.next_uuid + event_name = params.key?(:event_name) ? params[:event_name] : @webhook.events.first.name # could be nil, for backwards compatibility verification = @webhook.trigger_on_provider_verification_published? ? @verification : nil - @triggered_webhook = webhook_repository.create_triggered_webhook trigger_uuid, @webhook, @pact, verification, PactBroker::Webhooks::Service::RESOURCE_CREATION, @webhook.events.first.name + @triggered_webhook = webhook_repository.create_triggered_webhook trigger_uuid, @webhook, @pact, verification, PactBroker::Webhooks::Service::RESOURCE_CREATION, event_name @triggered_webhook.update(status: params[:status]) if params[:status] set_created_at_if_set params[:created_at], :triggered_webhooks, {id: @triggered_webhook.id} self diff --git a/spec/lib/pact_broker/webhooks/repository_spec.rb b/spec/lib/pact_broker/webhooks/repository_spec.rb index 32434bfef..fcaba845c 100644 --- a/spec/lib/pact_broker/webhooks/repository_spec.rb +++ b/spec/lib/pact_broker/webhooks/repository_spec.rb @@ -563,6 +563,26 @@ module Webhooks expect(subject.collect(&:trigger_uuid).sort).to eq ['332', '638'] end + context "when a webhook has been triggered by different events" do + before do + td.create_pact_with_hierarchy("Foo2", "1.0.0", "Bar2") + .create_webhook + .create_triggered_webhook(trigger_uuid: '333', event_name: 'foo') + .create_triggered_webhook(trigger_uuid: '555', event_name: 'foo') + .create_webhook_execution + .create_triggered_webhook(trigger_uuid: '444', event_name: 'bar') + .create_triggered_webhook(trigger_uuid: '777', event_name: 'bar') + .create_webhook_execution + .create_triggered_webhook(trigger_uuid: '111', event_name: nil) + .create_triggered_webhook(trigger_uuid: '888', event_name: nil) + .create_webhook_execution + end + + it "returns one for each event" do + expect(subject.collect(&:trigger_uuid).sort).to eq ['555', '777', '888'] + end + end + context "when there are two 'latest' triggered webhooks at the same time" do before do td.create_triggered_webhook(trigger_uuid: '888', created_at: DateTime.new(2018))