From e4daa76f9a5e4f68ae652b8b6be6af7d3936cf54 Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Fri, 16 Jun 2023 13:55:32 +1000 Subject: [PATCH] feat: add contract_data_updated_at to integrations table to speed up dashboard query (#617) PACT-1070 --- ...d_integrations_contract_data_updated_at.rb | 13 ++++ ...t_integrations_contract_data_updated_at.rb | 11 ++++ lib/pact_broker/api.rb | 1 + ...ntract_data_updated_at_for_integrations.rb | 47 ++++++++++++++ lib/pact_broker/db/migrate_data.rb | 1 + lib/pact_broker/events/subscriber.rb | 16 +++-- lib/pact_broker/initializers/subscriptions.rb | 4 ++ .../integrations/event_listener.rb | 23 +++++++ lib/pact_broker/integrations/integration.rb | 5 +- lib/pact_broker/integrations/repository.rb | 9 +++ lib/pact_broker/integrations/service.rb | 7 ++ lib/pact_broker/test/test_data_builder.rb | 4 ++ ...h_verification_results_and_version_spec.rb | 18 ----- .../endpoints/publish_contracts_spec.rb | 6 ++ .../publish_verification_results_spec.rb | 52 +++++++++++++++ ...t_data_updated_at_for_integrations_spec.rb | 49 ++++++++++++++ .../integrations/repository_spec.rb | 65 +++++++++++++++++++ spec/support/rspec_matchers.rb | 13 ++++ 18 files changed, 321 insertions(+), 23 deletions(-) create mode 100644 db/migrations/20230615_add_integrations_contract_data_updated_at.rb create mode 100644 db/migrations/20230616_set_integrations_contract_data_updated_at.rb create mode 100644 lib/pact_broker/db/data_migrations/set_contract_data_updated_at_for_integrations.rb create mode 100644 lib/pact_broker/initializers/subscriptions.rb create mode 100644 lib/pact_broker/integrations/event_listener.rb create mode 100644 spec/integration/endpoints/publish_verification_results_spec.rb create mode 100644 spec/lib/pact_broker/db/data_migrations/set_contract_data_updated_at_for_integrations_spec.rb create mode 100644 spec/lib/pact_broker/integrations/repository_spec.rb diff --git a/db/migrations/20230615_add_integrations_contract_data_updated_at.rb b/db/migrations/20230615_add_integrations_contract_data_updated_at.rb new file mode 100644 index 000000000..6077b3a5d --- /dev/null +++ b/db/migrations/20230615_add_integrations_contract_data_updated_at.rb @@ -0,0 +1,13 @@ +Sequel.migration do + up do + alter_table(:integrations) do + add_column(:contract_data_updated_at, DateTime) + end + end + + down do + alter_table(:integrations) do + drop_column(:contract_data_updated_at) + end + end +end diff --git a/db/migrations/20230616_set_integrations_contract_data_updated_at.rb b/db/migrations/20230616_set_integrations_contract_data_updated_at.rb new file mode 100644 index 000000000..6f9c4ac9a --- /dev/null +++ b/db/migrations/20230616_set_integrations_contract_data_updated_at.rb @@ -0,0 +1,11 @@ +require "pact_broker/db/data_migrations/set_contract_data_updated_at_for_integrations" + +Sequel.migration do + up do + PactBroker::DB::DataMigrations::SetContractDataUpdatedAtForIntegrations.call(self) + end + + down do + + end +end diff --git a/lib/pact_broker/api.rb b/lib/pact_broker/api.rb index 56c635dad..299e3998d 100644 --- a/lib/pact_broker/api.rb +++ b/lib/pact_broker/api.rb @@ -5,6 +5,7 @@ require "pact_broker/api/contracts" require "pact_broker/application_context" require "pact_broker/feature_toggle" +require "pact_broker/initializers/subscriptions" module Webmachine class Request diff --git a/lib/pact_broker/db/data_migrations/set_contract_data_updated_at_for_integrations.rb b/lib/pact_broker/db/data_migrations/set_contract_data_updated_at_for_integrations.rb new file mode 100644 index 000000000..498579303 --- /dev/null +++ b/lib/pact_broker/db/data_migrations/set_contract_data_updated_at_for_integrations.rb @@ -0,0 +1,47 @@ +# Populate the newly created contract_data_updated_at date in the integrations table +# using the latest created_at date from the pact_publications or verifications tables. +module PactBroker + module DB + module DataMigrations + class SetContractDataUpdatedAtForIntegrations + def self.call(connection) + join = { + Sequel[:integrations][:consumer_id] => Sequel[:target][:consumer_id], + Sequel[:integrations][:provider_id] => Sequel[:target][:provider_id] + } + + max_created_at_for_each_integration = integrations_max_created_at(connection).from_self(alias: :target).select(:created_at).where(join) + + connection[:integrations] + .where(contract_data_updated_at: nil) + .update(contract_data_updated_at: max_created_at_for_each_integration) + end + + # @return [Sequel::Dataset] the overall max created_at from the union of the pact_publications and verifications tables, + # for each integration keyed by consumer_id/provider_id + def self.integrations_max_created_at(connection) + pact_publication_max_created_at(connection) + .union(verification_max_created_at(connection)) + .select_group(:consumer_id, :provider_id) + .select_append{ max(:created_at).as(:created_at) } + end + + # @return [Sequel::Dataset] the max created_at from the pact_publications table + # for each integration keyed by consumer_id/provider_id + def self.pact_publication_max_created_at(connection) + connection[:pact_publications] + .select_group(:consumer_id, :provider_id) + .select_append{ max(:created_at).as(:created_at) } + end + + # @return [Sequel::Dataset] the max created_at from the verifications table + # for each integration keyed by consumer_id/provider_id + def self.verification_max_created_at(connection) + connection[:verifications] + .select_group(:consumer_id, :provider_id) + .select_append{ max(:created_at).as(:created_at) } + end + end + end + end +end diff --git a/lib/pact_broker/db/migrate_data.rb b/lib/pact_broker/db/migrate_data.rb index 42bfba3d6..94ac3cb79 100644 --- a/lib/pact_broker/db/migrate_data.rb +++ b/lib/pact_broker/db/migrate_data.rb @@ -29,6 +29,7 @@ def self.call database_connection, _options = {} DataMigrations::CreateBranches.call(database_connection) DataMigrations::MigrateIntegrations.call(database_connection) DataMigrations::MigratePactVersionProviderTagSuccessfulVerifications.call(database_connection) + DataMigrations::SetContractDataUpdatedAtForIntegrations.call(database_connection) end end end diff --git a/lib/pact_broker/events/subscriber.rb b/lib/pact_broker/events/subscriber.rb index 5b144593c..ca3e2a961 100644 --- a/lib/pact_broker/events/subscriber.rb +++ b/lib/pact_broker/events/subscriber.rb @@ -32,11 +32,19 @@ module Events extend self def subscribe(*args) - result = nil - TemporaryListeners.subscribe(*args) do - result = yield + if block_given? + result = nil + TemporaryListeners.subscribe(*args) do + result = yield + end + result + else + Wisper.subscribe(*args) end - result + end + + def unsubscribe(*args) + Wisper.unsubscribe(*args) end end end diff --git a/lib/pact_broker/initializers/subscriptions.rb b/lib/pact_broker/initializers/subscriptions.rb new file mode 100644 index 000000000..7ac2bc32d --- /dev/null +++ b/lib/pact_broker/initializers/subscriptions.rb @@ -0,0 +1,4 @@ +require "pact_broker/events/subscriber" +require "pact_broker/integrations/event_listener" + +PactBroker::Events.subscribe(PactBroker::Integrations::EventListener.new) diff --git a/lib/pact_broker/integrations/event_listener.rb b/lib/pact_broker/integrations/event_listener.rb new file mode 100644 index 000000000..200b4173c --- /dev/null +++ b/lib/pact_broker/integrations/event_listener.rb @@ -0,0 +1,23 @@ +require "pact_broker/services" + +# Listens for events that happen in the Pact Broker that are relevant to the Integrations objects. + +module PactBroker + module Integrations + class EventListener + include PactBroker::Services + + # @param [Hash] params the params from the broadcast event + # @option params [PactBroker::Domain::Pact] :pact the newly published pact + def contract_published(params) + integration_service.handle_contract_data_published(params.fetch(:pact).consumer, params.fetch(:pact).provider) + end + + # @param [Hash] params the params from the broadcast event + # @option params [PactBroker::Domain::Verification] :verification the newly published verification + def provider_verification_published(params) + integration_service.handle_contract_data_published(params.fetch(:verification).consumer, params.fetch(:verification).provider) + end + end + end +end diff --git a/lib/pact_broker/integrations/integration.rb b/lib/pact_broker/integrations/integration.rb index cc288d701..6f95c0b6a 100644 --- a/lib/pact_broker/integrations/integration.rb +++ b/lib/pact_broker/integrations/integration.rb @@ -1,3 +1,4 @@ +require "pact_broker/dataset" require "pact_broker/verifications/pseudo_branch_status" require "pact_broker/domain/verification" require "pact_broker/webhooks/latest_triggered_webhook" @@ -6,7 +7,7 @@ module PactBroker module Integrations - class Integration < Sequel::Model(Sequel::Model.db[:integrations].select(:id, :consumer_id, :provider_id)) + class Integration < Sequel::Model(Sequel::Model.db[:integrations].select(:id, :consumer_id, :provider_id, :contract_data_updated_at)) set_primary_key :id plugin :insert_ignore, identifying_columns: [:consumer_id, :provider_id] associate(:many_to_one, :consumer, :class => "PactBroker::Domain::Pacticipant", :key => :consumer_id, :primary_key => :id) @@ -75,6 +76,8 @@ class Integration < Sequel::Model(Sequel::Model.db[:integrations].select(:id, :c end) dataset_module do + include PactBroker::Dataset + def including_pacticipant_id(pacticipant_id) where(consumer_id: pacticipant_id).or(provider_id: pacticipant_id) end diff --git a/lib/pact_broker/integrations/repository.rb b/lib/pact_broker/integrations/repository.rb index f176c08bb..ecac6853b 100644 --- a/lib/pact_broker/integrations/repository.rb +++ b/lib/pact_broker/integrations/repository.rb @@ -17,6 +17,15 @@ def create_for_pact(consumer_id, provider_id) def delete(consumer_id, provider_id) Integration.where(consumer_id: consumer_id, provider_id: provider_id).delete end + + # Sets the contract_data_updated_at for the integration(s) as specified by the consumer and provider + # @param [PactBroker::Domain::Pacticipant, nil] consumer the consumer for the integration, or nil if for a provider-only event (eg. Pactflow provider contract published) + # @param [PactBroker::Domain::Pacticipant] provider the provider for the integration + def set_contract_data_updated_at(consumer, provider) + Integration + .where({ consumer_id: consumer&.id, provider_id: provider.id }.compact ) + .update(contract_data_updated_at: Sequel.datetime_class.now) + end end end end diff --git a/lib/pact_broker/integrations/service.rb b/lib/pact_broker/integrations/service.rb index b446df206..3e172c9af 100644 --- a/lib/pact_broker/integrations/service.rb +++ b/lib/pact_broker/integrations/service.rb @@ -32,6 +32,13 @@ def self.find_all .sort { | a, b| Integration.compare_by_last_action_date(a, b) } end + # Callback to invoke when a consumer contract, verification result (or provider contract in Pactflow) is published + # @param [PactBroker::Domain::Pacticipant] consumer or nil + # @param [PactBroker::Domain::Pacticipant] provider + def self.handle_contract_data_published(consumer, provider) + integration_repository.set_contract_data_updated_at(consumer, provider) + end + def self.delete(consumer_name, provider_name) consumer = pacticipant_service.find_pacticipant_by_name!(consumer_name) provider = pacticipant_service.find_pacticipant_by_name!(provider_name) diff --git a/lib/pact_broker/test/test_data_builder.rb b/lib/pact_broker/test/test_data_builder.rb index 19a71fec0..43d6c9fb4 100644 --- a/lib/pact_broker/test/test_data_builder.rb +++ b/lib/pact_broker/test/test_data_builder.rb @@ -493,6 +493,10 @@ def and_return instance_variable_name instance_variable_get("@#{instance_variable_name}") end + def clear_now + @now = nil + end + def set_now date @now = date.to_date self diff --git a/spec/features/publish_verification_results_and_version_spec.rb b/spec/features/publish_verification_results_and_version_spec.rb index ecf4a75b6..ae3ae3735 100644 --- a/spec/features/publish_verification_results_and_version_spec.rb +++ b/spec/features/publish_verification_results_and_version_spec.rb @@ -49,22 +49,4 @@ expect(last_response.status).to be 200 expect(JSON.parse(subject.body)).to include JSON.parse(verification_content) end - - context "with a webhook configured", job: true do - before do - td.create_webhook( - method: "POST", - url: "http://example.org", - events: [{ name: PactBroker::Webhooks::WebhookEvent::VERIFICATION_PUBLISHED }] - ) - end - let!(:request) do - stub_request(:post, "http://example.org").to_return(:status => 200) - end - - it "executes the webhook" do - subject - expect(request).to have_been_made - end - end end diff --git a/spec/integration/endpoints/publish_contracts_spec.rb b/spec/integration/endpoints/publish_contracts_spec.rb index 6febe86ba..9bea43c4b 100644 --- a/spec/integration/endpoints/publish_contracts_spec.rb +++ b/spec/integration/endpoints/publish_contracts_spec.rb @@ -21,6 +21,7 @@ let(:rack_headers) { { "CONTENT_TYPE" => "application/json", "HTTP_ACCEPT" => "application/hal+json" } } let(:encoded_contract) { Base64.strict_encode64(contract) } let(:path) { "/contracts/publish" } + let(:contract) { td.fixed_json_content("Foo", "Bar", "1") } subject { post(path, request_body_hash.to_json, rack_headers) } @@ -30,4 +31,9 @@ its(:status) { is_expected.to eq 400 } its(:body) { is_expected.to include("non UTF-8 character") } end + + it "sets the contract_data_updated_at on the integration" do + subject + expect(PactBroker::Integrations::Integration.last.contract_data_updated_at).to_not be nil + end end diff --git a/spec/integration/endpoints/publish_verification_results_spec.rb b/spec/integration/endpoints/publish_verification_results_spec.rb new file mode 100644 index 000000000..dfc74bd3b --- /dev/null +++ b/spec/integration/endpoints/publish_verification_results_spec.rb @@ -0,0 +1,52 @@ +require "pact_broker/domain/verification" +require "timecop" + +describe "Publishing a pact verification" do + let(:path) { "/pacts/provider/Provider/consumer/Consumer/pact-version/#{pact.pact_version_sha}/verification-results" } + let(:verification_content) { load_fixture("verification.json") } + let(:parsed_response_body) { JSON.parse(subject.body) } + let(:pact) { td.pact } + let(:rack_env) do + { + "CONTENT_TYPE" => "application/json", + "HTTP_ACCEPT" => "application/hal+json", + "pactbroker.database_connector" => lambda { |&block| block.call } + } + end + + subject { post(path, verification_content, rack_env) } + + before do + Timecop.freeze(Date.today - 2) do + td.create_provider("Provider") + .create_consumer("Consumer") + .create_consumer_version("1.0.0") + .create_pact + .create_consumer_version("1.2.3") + .create_pact + .revise_pact + end + end + + it "updates the contract_data_updated_at on the integration" do + expect { subject }.to change { PactBroker::Integrations::Integration.last.contract_data_updated_at } + end + + context "with a webhook configured", job: true do + before do + td.create_webhook( + method: "POST", + url: "http://example.org", + events: [{ name: PactBroker::Webhooks::WebhookEvent::VERIFICATION_PUBLISHED }] + ) + end + let!(:request) do + stub_request(:post, "http://example.org").to_return(:status => 200) + end + + it "executes the webhook" do + subject + expect(request).to have_been_made + end + end +end diff --git a/spec/lib/pact_broker/db/data_migrations/set_contract_data_updated_at_for_integrations_spec.rb b/spec/lib/pact_broker/db/data_migrations/set_contract_data_updated_at_for_integrations_spec.rb new file mode 100644 index 000000000..f0b153d46 --- /dev/null +++ b/spec/lib/pact_broker/db/data_migrations/set_contract_data_updated_at_for_integrations_spec.rb @@ -0,0 +1,49 @@ +require "pact_broker/db/data_migrations/set_contract_data_updated_at_for_integrations" +require "timecop" +require "tzinfo" + +module PactBroker + module DB + module DataMigrations + describe SetContractDataUpdatedAtForIntegrations do + before do + td.clear_now # use timecop instead of the TestDataBuilder @now + + Timecop.freeze(day_1) do + td.publish_pact(consumer_name: "Foo", provider_name: "Bar", consumer_version_number: "1") + end + + Timecop.freeze(day_2) do + td.publish_pact(consumer_name: "Foo", provider_name: "Bar", consumer_version_number: "2") + end + + Timecop.freeze(day_3) do + td.create_verification(provider_version: "2") + end + + Timecop.freeze(day_4) do + td.publish_pact(consumer_name: "Cat", provider_name: "Dog", consumer_version_number: "2") + end + + db[:integrations].update(contract_data_updated_at: nil) + end + + let(:day_1) { td.in_utc{ DateTime.new(2023, 6, 11) } } + let(:day_2) { td.in_utc{ DateTime.new(2023, 6, 12) } } + let(:day_3) { td.in_utc{ DateTime.new(2023, 6, 13) } } + let(:day_4) { td.in_utc{ DateTime.new(2023, 6, 14) } } + + let(:db) { PactBroker::Domain::Version.db } + + subject { SetContractDataUpdatedAtForIntegrations.call(db) } + + it "sets the contract_data_updated_at to the latest of the pact publication and verification publication dates for that integration" do + subject + integrations = db[:integrations].order(:id) + expect(integrations.first[:contract_data_updated_at]).to be_date_time(day_3) + expect(integrations.last[:contract_data_updated_at]).to be_date_time(day_4) + end + end + end + end +end diff --git a/spec/lib/pact_broker/integrations/repository_spec.rb b/spec/lib/pact_broker/integrations/repository_spec.rb new file mode 100644 index 000000000..ff19b550f --- /dev/null +++ b/spec/lib/pact_broker/integrations/repository_spec.rb @@ -0,0 +1,65 @@ +require "pact_broker/integrations/repository" +require "timecop" + +module PactBroker + module Integrations + describe Repository do + describe "#set_contract_data_updated_at" do + before do + # A -> B + # Foo -> Bar + td.create_consumer("A") + .create_provider("B") + .create_integration + .create_consumer("Foo") + .create_provider("Bar") + .create_integration + end + + let(:then) { Date.today - 20 } + let(:now) { DateTime.new(2010, 11, 1, 1, 1, 1) } + let(:foo) { td.and_return(:consumer) } + let(:bar) { td.and_return(:provider) } + + subject do + Timecop.freeze(now) do + Repository.new.set_contract_data_updated_at(foo, bar) + end + end + + it "updates the contract_data_updated_at to now" do + subject + expect(Integration.last.contract_data_updated_at).to be_date_time(now) + end + + it "does not update the other integrations" do + expect { subject }.to_not change { Integration.first.contract_data_updated_at } + end + + context "with the consumer is nil (eg. when a provider contract is published in Pactflow)" do + before do + # A -> B + # Foo -> Bar + # A -> Bar + td.use_consumer("A") + .use_provider("Bar") + .create_integration + end + + subject do + Timecop.freeze(now) do + Repository.new.set_contract_data_updated_at(nil, bar) + end + end + + it "updates all the integrations for the provider" do + subject + integrations = Integration.select_all_qualified.including_pacticipant_id(bar.id) + expect(integrations.first.contract_data_updated_at).to be_date_time(now) + expect(integrations.last.contract_data_updated_at).to be_date_time(now) + end + end + end + end + end +end diff --git a/spec/support/rspec_matchers.rb b/spec/support/rspec_matchers.rb index b821c9ae9..f976d23a9 100644 --- a/spec/support/rspec_matchers.rb +++ b/spec/support/rspec_matchers.rb @@ -1,3 +1,5 @@ +require "pact_broker/api/decorators/format_date_time" + RSpec::Matchers.define :be_datey do |_expected| match do |actual| actual.instance_of?(DateTime) || actual.instance_of?(Time) @@ -7,3 +9,14 @@ "expected #{actual.inspect} to be an instance of DateTime or Time" end end + +# Need this because dates get loaded into models as strings when using MySQL +RSpec::Matchers.define :be_date_time do |expected| + match do |actual| + PactBroker::Api::Decorators::FormatDateTime.call(expected) == PactBroker::Api::Decorators::FormatDateTime.call(actual) + end + + failure_message do |actual| + "expected #{PactBroker::Api::Decorators::FormatDateTime.call(expected)} to equal #{PactBroker::Api::Decorators::FormatDateTime.call(actual)}" + end +end