From 792bb9ae57636f04c356df132c1f36a6118800b9 Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Fri, 19 Jan 2018 11:01:06 +1100 Subject: [PATCH 1/5] refactor: move logic for determining absolute latest triggered webhook into database --- ...180119_update_latest_triggered_webhooks.rb | 63 +++++++++++++++++++ lib/pact_broker/webhooks/repository.rb | 3 - 2 files changed, 63 insertions(+), 3 deletions(-) create mode 100644 db/migrations/20180119_update_latest_triggered_webhooks.rb diff --git a/db/migrations/20180119_update_latest_triggered_webhooks.rb b/db/migrations/20180119_update_latest_triggered_webhooks.rb new file mode 100644 index 000000000..5f393c769 --- /dev/null +++ b/db/migrations/20180119_update_latest_triggered_webhooks.rb @@ -0,0 +1,63 @@ +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" + ) + end + + down do + create_or_replace_view(:latest_triggered_webhook_ids, + "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" + ) + + 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.created_at = ltwi.latest_triggered_webhook_created_at" + ) + + drop_view(:latest_triggered_webhook_creation_dates) + end +end \ No newline at end of file diff --git a/lib/pact_broker/webhooks/repository.rb b/lib/pact_broker/webhooks/repository.rb index 007eb01c3..c4ab20985 100644 --- a/lib/pact_broker/webhooks/repository.rb +++ b/lib/pact_broker/webhooks/repository.rb @@ -140,9 +140,6 @@ def find_latest_triggered_webhooks consumer, provider .where(consumer: consumer, provider: provider) .order(:id) .all - .group_by{|w| [w.consumer_id, w.provider_id, w.webhook_uuid]} - .values - .collect(&:last) end def fail_retrying_triggered_webhooks From 01fd2f27ae75efe4f586c6765bd7c9ba2cee14fe Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Sat, 20 Jan 2018 13:43:07 +1100 Subject: [PATCH 2/5] refactor: new line --- db/migrations/000046_recreate_latest_verifications.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/db/migrations/000046_recreate_latest_verifications.rb b/db/migrations/000046_recreate_latest_verifications.rb index 6ce2292b3..8aedcf009 100644 --- a/db/migrations/000046_recreate_latest_verifications.rb +++ b/db/migrations/000046_recreate_latest_verifications.rb @@ -11,7 +11,8 @@ "SELECT v.id, v.number, v.success, s.number as provider_version, v.build_url, v.pact_version_id, v.execution_date, v.created_at, v.provider_version_id, s.number as provider_version_number FROM verifications v INNER JOIN latest_verification_numbers lv - ON v.pact_version_id = lv.pact_version_id AND v.number = lv.latest_number + ON v.pact_version_id = lv.pact_version_id + AND v.number = lv.latest_number INNER JOIN versions s on v.provider_version_id = s.id" ) end From a7128ef2d96a710cf2f27fd34c42c2faabc3479d Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Sat, 20 Jan 2018 13:43:20 +1100 Subject: [PATCH 3/5] docs: updated developer documentation --- DEVELOPER_DOCUMENTATION.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DEVELOPER_DOCUMENTATION.md b/DEVELOPER_DOCUMENTATION.md index 425f6aaa3..1c24d3f37 100644 --- a/DEVELOPER_DOCUMENTATION.md +++ b/DEVELOPER_DOCUMENTATION.md @@ -46,7 +46,7 @@ Domain classes are found in `lib/pact_broker/domain`. Many of these classes are * `consumer version number` and `consumer version order` * `revision_number` -* `latest_pact_publications` - This view has the same columns as `all_pact_publications`, but it only contains the latest revision of the pact for each provider/consumer/version. It maps to what a user would consider the "pact" resource ie. `/pacts/provider/Provider/consumer/Consumer/version/1.2.3`. Previous revisions are not currently exposed via the API. +* `latest_pact_publications_by_consumer_versions` - This view has the same columns as `all_pact_publications`, but it only contains the latest revision of the pact for each provider/consumer/version. It maps to what a user would consider the "pact" resource ie. `/pacts/provider/Provider/consumer/Consumer/version/1.2.3`. Previous revisions are not currently exposed via the API. The `AllPactPublications` Sequel model in the code is what is used when querying data for displaying in a response, rather than the normalised separate PactPublication and PactVersion models. From aa60a8568c5b61cac1bce36aa184f73415e0d67d Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Thu, 25 Jan 2018 11:15:02 +1100 Subject: [PATCH 4/5] feat: add endpoints to get latest pacticipant version and latest tagged version --- lib/pact_broker/api.rb | 2 ++ lib/pact_broker/api/resources/index.rb | 15 ++++++-- lib/pact_broker/api/resources/version.rb | 10 ++++-- lib/pact_broker/versions/service.rb | 4 +++ .../hal_relation_proxy_app.rb | 36 +++++++++++++++++++ spec/service_consumers/pact_helper.rb | 22 +++++++++++- .../provider_states_for_pact_broker_client.rb | 27 ++++++++++++++ 7 files changed, 109 insertions(+), 7 deletions(-) create mode 100644 spec/service_consumers/hal_relation_proxy_app.rb diff --git a/lib/pact_broker/api.rb b/lib/pact_broker/api.rb index 4b9813a13..f33827a59 100644 --- a/lib/pact_broker/api.rb +++ b/lib/pact_broker/api.rb @@ -44,6 +44,8 @@ module PactBroker add ['pacticipants', :name], Api::Resources::Pacticipant, {resource_name: "pacticipant"} add ['pacticipants', :pacticipant_name, 'versions'], Api::Resources::Versions, {resource_name: "pacticipant_versions"} add ['pacticipants', :pacticipant_name, 'versions', :pacticipant_version_number], Api::Resources::Version, {resource_name: "pacticipant_version"} + add ['pacticipants', :pacticipant_name, 'versions', 'latest', :tag], Api::Resources::Version, {resource_name: "latest_tagged_pacticipant_version"} + add ['pacticipants', :pacticipant_name, 'versions', 'latest'], Api::Resources::Version, {resource_name: "latest_pacticipant_version"} add ['pacticipants', :pacticipant_name, 'versions', :pacticipant_version_number, 'tags', :tag_name], Api::Resources::Tag, {resource_name: "pacticipant_version_tag"} add ['pacticipants', :pacticipant_name, 'labels', :label_name], Api::Resources::Label, {resource_name: "pacticipant_label"} diff --git a/lib/pact_broker/api/resources/index.rb b/lib/pact_broker/api/resources/index.rb index 1db0da84c..608440e2b 100644 --- a/lib/pact_broker/api/resources/index.rb +++ b/lib/pact_broker/api/resources/index.rb @@ -50,11 +50,20 @@ def to_json 'pb:latest-provider-pacts-with-tag' => { href: base_url + '/pacts/provider/{provider}/latest/{tag}', - title: 'Latest pacts by provider with a specified tag', + title: 'Latest pacts by provider with the specified tag', templated: true }, - 'pb:webhooks' => - { + 'pb:latest-version' => { + href: base_url + '/pacticipants/{pacticipant}/versions/latest', + title: 'Latest pacticipant version', + templated: true + }, + 'pb:latest-tagged-version' => { + href: base_url + '/pacticipants/{pacticipant}/versions/latest/{tag}', + title: 'Latest pacticipant version with the specified tag', + templated: true + }, + 'pb:webhooks' => { href: base_url + '/webhooks', title: 'Webhooks', templated: false diff --git a/lib/pact_broker/api/resources/version.rb b/lib/pact_broker/api/resources/version.rb index e551a0125..dfa9943f3 100644 --- a/lib/pact_broker/api/resources/version.rb +++ b/lib/pact_broker/api/resources/version.rb @@ -32,11 +32,15 @@ def delete_resource private def version - @version ||= version_service.find_by_pacticipant_name_and_number path_info + if path_info[:tag] + @version ||= version_service.find_by_pacticpant_name_and_latest_tag(path_info[:pacticipant_name], path_info[:tag]) + elsif path_info[:number] + @version ||= version_service.find_by_pacticipant_name_and_number path_info + else + @version ||= version_service.find_latest_by_pacticpant_name path_info + end end - end end - end end diff --git a/lib/pact_broker/versions/service.rb b/lib/pact_broker/versions/service.rb index 5bc3d5ad3..89f7a7619 100644 --- a/lib/pact_broker/versions/service.rb +++ b/lib/pact_broker/versions/service.rb @@ -7,6 +7,10 @@ class Service extend PactBroker::Repositories + def self.find_latest_by_pacticpant_name params + version_repository.find_latest_by_pacticpant_name params.fetch(:pacticipant_name) + end + def self.find_by_pacticipant_name_and_number params version_repository.find_by_pacticipant_name_and_number params.fetch(:pacticipant_name), params.fetch(:pacticipant_version_number) end diff --git a/spec/service_consumers/hal_relation_proxy_app.rb b/spec/service_consumers/hal_relation_proxy_app.rb new file mode 100644 index 000000000..f2a91d876 --- /dev/null +++ b/spec/service_consumers/hal_relation_proxy_app.rb @@ -0,0 +1,36 @@ +class HalRelationProxyApp + + # This hash allows us to do a dodgy "generators" implementation + # It means we can use placeholder URLS for the relations in our consumer tests so that + # the consumer does not need to know the actual URLs. + PATH_REPLACEMENTS = { + '/HAL-REL-PLACEHOLDER-INDEX-PB-LATEST-TAGGED-VERSION-Condor-production' => + '/pacticipants/Condor/versions/latest/production', + '/HAL-REL-PLACEHOLDER-INDEX-PB-LATEST-VERSION-Condor' => + '/pacticipants/Condor/versions/latest' + } + + RESPONSE_BODY_REPLACEMENTS = { + } + + def initialize(app) + @app = app + end + + def call env + env_with_modified_path = env + PATH_REPLACEMENTS.each do | (find, replace) | + env_with_modified_path['PATH_INFO'] = env_with_modified_path['PATH_INFO'].gsub(find, replace) + end + + response = @app.call(env_with_modified_path) + + modified_response_body = response.last.join + RESPONSE_BODY_REPLACEMENTS.each do | (find, replace) | + modified_response_body = modified_response_body.gsub(find, replace) + end + + [response[0], response[1], [modified_response_body]] + end + +end diff --git a/spec/service_consumers/pact_helper.rb b/spec/service_consumers/pact_helper.rb index 3fd1badc9..4b5471119 100644 --- a/spec/service_consumers/pact_helper.rb +++ b/spec/service_consumers/pact_helper.rb @@ -6,12 +6,32 @@ PactBroker::DB.connection = PactBroker::Database.database = DB::PACT_BROKER_DB require 'spec/support/database_cleaner' -require 'pact_broker/api' +require 'pact_broker' +require 'pact_broker/app' +require_relative 'hal_relation_proxy_app' require_relative 'provider_states_for_pact_broker_client' +pact_broker = PactBroker::App.new { |c| c.database_connection = DB::PACT_BROKER_DB } +app_to_verify = HalRelationProxyApp.new(pact_broker) + +module Rack + module PactBroker + class DatabaseTransaction + def do_not_rollback? response + # Dodgey hack to stop exceptions raising a Rollback error while verifying + # Otherwise the provider states that deliberately raise exceptions + # end up raising exceptions that break the verification tests + true + end + end + end +end + Pact.service_provider "Pact Broker" do + app { HalRelationProxyApp.new(app_to_verify) } + honours_pact_with "Pact Broker Client" do pact_uri "https://raw.githubusercontent.com/pact-foundation/pact_broker-client/master/spec/pacts/pact_broker_client-pact_broker.json" end diff --git a/spec/service_consumers/provider_states_for_pact_broker_client.rb b/spec/service_consumers/provider_states_for_pact_broker_client.rb index a9ba3d01f..e78c37e1b 100644 --- a/spec/service_consumers/provider_states_for_pact_broker_client.rb +++ b/spec/service_consumers/provider_states_for_pact_broker_client.rb @@ -2,6 +2,33 @@ Pact.provider_states_for "Pact Broker Client" do + provider_state "the pb:latest-tagged-version relation exists in the index resource" do + no_op + end + + provider_state "'Condor' exists in the pact-broker with the latest tagged 'production' version 1.2.3" do + set_up do + TestDataBuilder.new + .create_consumer("Condor") + .create_consumer_version("1.2.3") + .create_consumer_version_tag("production") + .create_consumer_version("2.0.0") + end + end + + provider_state "the pb:latest-version relation exists in the index resource" do + no_op + end + + provider_state "'Condor' exists in the pact-broker with the latest version 1.2.3" do + set_up do + TestDataBuilder.new + .create_consumer("Condor") + .create_consumer_version("1.0.0") + .create_consumer_version("1.2.3") + end + end + provider_state "the pact for Foo Thing version 1.2.3 has been verified by Bar version 4.5.6" do set_up do TestDataBuilder.new From 8e4506225e1665a47ba65d3950c61997001fd309 Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Thu, 25 Jan 2018 12:06:55 +1100 Subject: [PATCH 5/5] feat: change URL for retrieving latest version so that it does not clash with a version called "latest" --- lib/pact_broker/api.rb | 4 ++-- lib/pact_broker/api/resources/index.rb | 4 ++-- lib/pact_broker/api/resources/version.rb | 2 +- spec/service_consumers/hal_relation_proxy_app.rb | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/pact_broker/api.rb b/lib/pact_broker/api.rb index f33827a59..add1f605f 100644 --- a/lib/pact_broker/api.rb +++ b/lib/pact_broker/api.rb @@ -44,8 +44,8 @@ module PactBroker add ['pacticipants', :name], Api::Resources::Pacticipant, {resource_name: "pacticipant"} add ['pacticipants', :pacticipant_name, 'versions'], Api::Resources::Versions, {resource_name: "pacticipant_versions"} add ['pacticipants', :pacticipant_name, 'versions', :pacticipant_version_number], Api::Resources::Version, {resource_name: "pacticipant_version"} - add ['pacticipants', :pacticipant_name, 'versions', 'latest', :tag], Api::Resources::Version, {resource_name: "latest_tagged_pacticipant_version"} - add ['pacticipants', :pacticipant_name, 'versions', 'latest'], Api::Resources::Version, {resource_name: "latest_pacticipant_version"} + add ['pacticipants', :pacticipant_name, 'latest-version', :tag], Api::Resources::Version, {resource_name: "latest_tagged_pacticipant_version"} + add ['pacticipants', :pacticipant_name, 'latest-version'], Api::Resources::Version, {resource_name: "latest_pacticipant_version"} add ['pacticipants', :pacticipant_name, 'versions', :pacticipant_version_number, 'tags', :tag_name], Api::Resources::Tag, {resource_name: "pacticipant_version_tag"} add ['pacticipants', :pacticipant_name, 'labels', :label_name], Api::Resources::Label, {resource_name: "pacticipant_label"} diff --git a/lib/pact_broker/api/resources/index.rb b/lib/pact_broker/api/resources/index.rb index 608440e2b..04156d270 100644 --- a/lib/pact_broker/api/resources/index.rb +++ b/lib/pact_broker/api/resources/index.rb @@ -54,12 +54,12 @@ def to_json templated: true }, 'pb:latest-version' => { - href: base_url + '/pacticipants/{pacticipant}/versions/latest', + href: base_url + '/pacticipants/{pacticipant}/latest-version', title: 'Latest pacticipant version', templated: true }, 'pb:latest-tagged-version' => { - href: base_url + '/pacticipants/{pacticipant}/versions/latest/{tag}', + href: base_url + '/pacticipants/{pacticipant}/latest-version/{tag}', title: 'Latest pacticipant version with the specified tag', templated: true }, diff --git a/lib/pact_broker/api/resources/version.rb b/lib/pact_broker/api/resources/version.rb index dfa9943f3..c87f7a8f1 100644 --- a/lib/pact_broker/api/resources/version.rb +++ b/lib/pact_broker/api/resources/version.rb @@ -34,7 +34,7 @@ def delete_resource def version if path_info[:tag] @version ||= version_service.find_by_pacticpant_name_and_latest_tag(path_info[:pacticipant_name], path_info[:tag]) - elsif path_info[:number] + elsif path_info[:pacticipant_version_number] @version ||= version_service.find_by_pacticipant_name_and_number path_info else @version ||= version_service.find_latest_by_pacticpant_name path_info diff --git a/spec/service_consumers/hal_relation_proxy_app.rb b/spec/service_consumers/hal_relation_proxy_app.rb index f2a91d876..095a543ad 100644 --- a/spec/service_consumers/hal_relation_proxy_app.rb +++ b/spec/service_consumers/hal_relation_proxy_app.rb @@ -5,9 +5,9 @@ class HalRelationProxyApp # the consumer does not need to know the actual URLs. PATH_REPLACEMENTS = { '/HAL-REL-PLACEHOLDER-INDEX-PB-LATEST-TAGGED-VERSION-Condor-production' => - '/pacticipants/Condor/versions/latest/production', + '/pacticipants/Condor/latest-version/production', '/HAL-REL-PLACEHOLDER-INDEX-PB-LATEST-VERSION-Condor' => - '/pacticipants/Condor/versions/latest' + '/pacticipants/Condor/latest-version' } RESPONSE_BODY_REPLACEMENTS = {