From c062b2e4adc668f601103ae14b1b8a541538bda5 Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Wed, 23 May 2018 13:50:05 +1000 Subject: [PATCH 1/7] chore: add script to run broker with self signed certificate --- script/run-with-ssl.rb | 44 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 script/run-with-ssl.rb diff --git a/script/run-with-ssl.rb b/script/run-with-ssl.rb new file mode 100644 index 000000000..6cec7c587 --- /dev/null +++ b/script/run-with-ssl.rb @@ -0,0 +1,44 @@ +if __FILE__ == $0 + require 'pact_broker' + + DATABASE_CREDENTIALS = {adapter: "sqlite", database: "pact_broker_ssl_database.sqlite3", :encoding => 'utf8'} + + app = PactBroker::App.new do | config | + config.logger = ::Logger.new($stdout) + config.logger.level = ::Logger::DEBUG + config.database_connection = Sequel.connect(DATABASE_CREDENTIALS) + end + + SSL_KEY = 'spec/fixtures/certificates/key.pem' + SSL_CERT = 'spec/fixtures/certificates/cert.pem' + + trap(:INT) do + @server.shutdown + exit + end + + def webrick_opts port + certificate = OpenSSL::X509::Certificate.new(File.read(SSL_CERT)) + cert_name = certificate.subject.to_a.collect{|a| a[0..1] } + { + Port: port, + Host: "0.0.0.0", + AccessLog: [], + SSLCertificate: certificate, + SSLPrivateKey: OpenSSL::PKey::RSA.new(File.read(SSL_KEY)), + SSLEnable: true, + SSLCertName: cert_name + } + end + + require 'webrick' + require 'webrick/https' + require 'rack' + require 'rack/handler/webrick' + + opts = webrick_opts(4444) + + Rack::Handler::WEBrick.run(app, opts) do |server| + @server = server + end +end From eb67511c795fddffea9f365ac9da3b4294e7941f Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Wed, 23 May 2018 13:41:14 +1000 Subject: [PATCH 2/7] feat: create view for latest verifications for consumer version tags --- .../20180311_optimise_head_matrix.rb | 1 + ...verifications_for_consumer_version_tags.rb | 24 ++++++++++ lib/pact_broker/matrix/row.rb | 14 ++++++ ...t_verification_for_consumer_version_tag.rb | 9 ++++ spec/lib/pact_broker/matrix/row_spec.rb | 46 +++++++++++++++++-- 5 files changed, 91 insertions(+), 3 deletions(-) create mode 100644 db/migrations/20180523_create_latest_verifications_for_consumer_version_tags.rb create mode 100644 lib/pact_broker/verifications/latest_verification_for_consumer_version_tag.rb diff --git a/db/migrations/20180311_optimise_head_matrix.rb b/db/migrations/20180311_optimise_head_matrix.rb index 529f347db..3ff853dfe 100644 --- a/db/migrations/20180311_optimise_head_matrix.rb +++ b/db/migrations/20180311_optimise_head_matrix.rb @@ -16,6 +16,7 @@ ) # Add provider_version_order to original definition + # The most recent verification for each pact version v = :verifications create_or_replace_view(:latest_verifications, from(v) diff --git a/db/migrations/20180523_create_latest_verifications_for_consumer_version_tags.rb b/db/migrations/20180523_create_latest_verifications_for_consumer_version_tags.rb new file mode 100644 index 000000000..ade9fcef1 --- /dev/null +++ b/db/migrations/20180523_create_latest_verifications_for_consumer_version_tags.rb @@ -0,0 +1,24 @@ +Sequel.migration do + up do + # The latest verification id for each consumer version tag + create_view(:latest_verifications_ids_for_consumer_version_tags, + "select t.name as consumer_version_tag_name, max(lv.id) as latest_verification_id + from verifications lv + join latest_pact_publications_by_consumer_versions lpp + on lv.pact_version_id = lpp.pact_version_id + join tags t on lpp.consumer_version_id = t.version_id + group by t.name") + + # The latest verification for each consumer version tag + create_view(:latest_verifications_for_consumer_version_tags, + "select v.*, lv.consumer_version_tag_name + from verifications v + join latest_verifications_ids_for_consumer_version_tags lv + on lv.latest_verification_id = v.id") + end + + down do + drop_view(:latest_verifications_for_consumer_version_tags) + drop_view(:latest_verifications_ids_for_consumer_version_tags) + end +end diff --git a/lib/pact_broker/matrix/row.rb b/lib/pact_broker/matrix/row.rb index 1d9063a80..a19f9e702 100644 --- a/lib/pact_broker/matrix/row.rb +++ b/lib/pact_broker/matrix/row.rb @@ -2,9 +2,11 @@ require 'pact_broker/webhooks/latest_triggered_webhook' require 'pact_broker/tags/tag_with_latest_flag' require 'pact_broker/logging' +require 'pact_broker/verifications/latest_verification_for_consumer_version_tag' module PactBroker module Matrix + class Row < Sequel::Model(:materialized_matrix) # Used when using table_print to output query results @@ -14,6 +16,18 @@ class Row < Sequel::Model(:materialized_matrix) associate(:one_to_many, :webhooks, :class => "PactBroker::Webhooks::Webhook", primary_key: [:consumer_id, :provider_id], key: [:consumer_id, :provider_id]) associate(:one_to_many, :consumer_version_tags, :class => "PactBroker::Tags::TagWithLatestFlag", primary_key: :consumer_version_id, key: :version_id) associate(:one_to_many, :provider_version_tags, :class => "PactBroker::Tags::TagWithLatestFlag", primary_key: :provider_version_id, key: :version_id) + associate(:many_to_one, :verification, :class => "PactBroker::Domain::Verification", primary_key: :id, key: :verification_id) + + one_to_one :latest_verification_for_consumer_version_tag, primary_keys: [], key: [], :eager_loader=>(proc do |eo_opts| + tag_to_row = eo_opts[:rows].each_with_object({}) { | row, map | row.consumer_version_tags.each{ | tag | map[tag.name] = row } } + eo_opts[:rows].each{|row| row.associations[:latest_verification_for_consumer_version_tag] = nil} + + PactBroker::Verifications::LatestVerificationForConsumerVersionTag.each do | verification | + if tag_to_row[verification.consumer_version_tag_name] + tag_to_row[verification.consumer_version_tag_name].associations[:latest_verification_for_consumer_version_tag] = verification + end + end + end) dataset_module do include PactBroker::Repositories::Helpers diff --git a/lib/pact_broker/verifications/latest_verification_for_consumer_version_tag.rb b/lib/pact_broker/verifications/latest_verification_for_consumer_version_tag.rb new file mode 100644 index 000000000..2c07b4541 --- /dev/null +++ b/lib/pact_broker/verifications/latest_verification_for_consumer_version_tag.rb @@ -0,0 +1,9 @@ +require 'pact_broker/domain/verification' + +module PactBroker + module Verifications + class LatestVerificationForConsumerVersionTag < PactBroker::Domain::Verification + set_dataset(:latest_verifications_for_consumer_version_tags) + end + end +end \ No newline at end of file diff --git a/spec/lib/pact_broker/matrix/row_spec.rb b/spec/lib/pact_broker/matrix/row_spec.rb index 75a364d26..d59d45ac8 100644 --- a/spec/lib/pact_broker/matrix/row_spec.rb +++ b/spec/lib/pact_broker/matrix/row_spec.rb @@ -3,14 +3,54 @@ module PactBroker module Matrix describe Row do - describe "refresh", migration: true do - before do - PactBroker::Database.migrate + let(:td) { TestDataBuilder.new } + + describe "latest_verification_for_consumer_version_tag" do + context "when the pact with a given tag has been verified" do + before do + td.create_pact_with_hierarchy("Foo", "1", "Bar") + .create_consumer_version_tag("prod") + .create_verification(provider_version: "10") + .create_consumer_version("2") + .create_consumer_version_tag("prod") + .create_pact + .create_verification(provider_version: "11", number: 1) + .create_verification(provider_version: "12", number: 2) + end + + subject { Row.eager(:consumer_version_tags).eager(:latest_verification_for_consumer_version_tag).order(:pact_publication_id, :verification_id).all } + + it "returns its most recent verification" do + expect(subject.last.provider_version_number).to eq "12" + expect(subject.last.latest_verification_for_consumer_version_tag.provider_version.number).to eq "12" + end + end + + context "when the most recent pact with a given tag has not been verified, but a previous version with the same tag has" do + before do + td.create_pact_with_hierarchy("Foo", "1", "Bar") + .create_consumer_version_tag("prod") + .create_verification(provider_version: "10") + .create_verification(provider_version: "11", number: 2, comment: "this is the latest verification for a pact with cv tag prod") + .create_consumer_version("2") + .create_consumer_version_tag("prod") + .create_pact + end + + subject { Row.eager(:consumer_version_tags).eager(:latest_verification_for_consumer_version_tag).order(:pact_publication_id).all } + + it "returns the most recent verification for the previous version with the same tag" do + expect(subject.last.verification_id).to be nil # this pact version has not been verified directly + expect(subject.last.latest_verification_for_consumer_version_tag.provider_version.number).to eq "11" + end end + end + describe "refresh", migration: true do let(:td) { TestDataBuilder.new(auto_refresh_matrix: false) } before do + PactBroker::Database.migrate td.create_pact_with_hierarchy("Foo", "1", "Bar") end From 524e08d0b68b74021d0260e5f82cdc1c99af74e9 Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Fri, 25 May 2018 14:13:46 +1000 Subject: [PATCH 3/7] feat: optimise queries for index page with tags --- ...verifications_for_consumer_version_tags.rb | 17 ++-- lib/pact_broker/domain/index_item.rb | 2 +- lib/pact_broker/index/service.rb | 96 +++---------------- lib/pact_broker/matrix/aggregated_row.rb | 57 +++++++++++ lib/pact_broker/matrix/head_row.rb | 21 ++++ lib/pact_broker/matrix/row.rb | 12 +-- .../pact_broker/matrix/aggregated_row_spec.rb | 80 ++++++++++++++++ spec/lib/pact_broker/matrix/head_row_spec.rb | 55 +++++++++++ spec/lib/pact_broker/matrix/row_spec.rb | 41 -------- spec/support/rspec_match_hash.rb | 31 +++--- 10 files changed, 253 insertions(+), 159 deletions(-) create mode 100644 lib/pact_broker/matrix/aggregated_row.rb create mode 100644 spec/lib/pact_broker/matrix/aggregated_row_spec.rb diff --git a/db/migrations/20180523_create_latest_verifications_for_consumer_version_tags.rb b/db/migrations/20180523_create_latest_verifications_for_consumer_version_tags.rb index ade9fcef1..0c41e371d 100644 --- a/db/migrations/20180523_create_latest_verifications_for_consumer_version_tags.rb +++ b/db/migrations/20180523_create_latest_verifications_for_consumer_version_tags.rb @@ -2,19 +2,22 @@ up do # The latest verification id for each consumer version tag create_view(:latest_verifications_ids_for_consumer_version_tags, - "select t.name as consumer_version_tag_name, max(lv.id) as latest_verification_id - from verifications lv + "select pv.pacticipant_id as provider_id, lpp.consumer_id, t.name as consumer_version_tag_name, max(v.id) as latest_verification_id + from verifications v join latest_pact_publications_by_consumer_versions lpp - on lv.pact_version_id = lpp.pact_version_id - join tags t on lpp.consumer_version_id = t.version_id - group by t.name") + on v.pact_version_id = lpp.pact_version_id + join tags t + on lpp.consumer_version_id = t.version_id + join versions pv + on v.provider_version_id = pv.id + group by pv.pacticipant_id, lpp.consumer_id, t.name") # The latest verification for each consumer version tag create_view(:latest_verifications_for_consumer_version_tags, - "select v.*, lv.consumer_version_tag_name + "select v.*, lv.provider_id, lv.consumer_id, lv.consumer_version_tag_name from verifications v join latest_verifications_ids_for_consumer_version_tags lv - on lv.latest_verification_id = v.id") + on lv.latest_verification_id = v.id") end down do diff --git a/lib/pact_broker/domain/index_item.rb b/lib/pact_broker/domain/index_item.rb index f1eb5a7d1..3a010fab6 100644 --- a/lib/pact_broker/domain/index_item.rb +++ b/lib/pact_broker/domain/index_item.rb @@ -102,7 +102,7 @@ def latest_verification_successful? end def pact_changed_since_last_verification? - latest_verification.pact_version_sha != latest_pact.pact_version_sha + latest_verification.pact_version_id != latest_pact.pact_version_id end def latest_verification_provider_version_number diff --git a/lib/pact_broker/index/service.rb b/lib/pact_broker/index/service.rb index 77f27faac..30ff4b460 100644 --- a/lib/pact_broker/index/service.rb +++ b/lib/pact_broker/index/service.rb @@ -2,6 +2,7 @@ require 'pact_broker/logging' require 'pact_broker/domain/index_item' require 'pact_broker/matrix/head_row' +require 'pact_broker/matrix/aggregated_row' module PactBroker @@ -13,107 +14,38 @@ class Service extend PactBroker::Logging def self.find_index_items options = {} - rows = [] - overall_latest_publication_ids = nil - rows = PactBroker::Matrix::HeadRow .select_all_qualified .eager(:latest_triggered_webhooks) .eager(:webhooks) .order(:consumer_name, :provider_name) - .eager(:consumer_version_tags) - .eager(:provider_version_tags) + .eager(:verification) if !options[:tags] - rows = rows.where(consumer_version_tag_name: nil).all - overall_latest_publication_ids = rows.collect(&:pact_publication_id) - end - - if options[:tags] + rows = rows.where(consumer_version_tag_name: nil) + else if options[:tags].is_a?(Array) rows = rows.where(consumer_version_tag_name: options[:tags]).or(consumer_version_tag_name: nil) end - - rows = rows.all - overall_latest_publication_ids = rows.select{|r| !r[:consumer_version_tag_name] }.collect(&:pact_publication_id).uniq - - # Smoosh all the rows with matching pact publications together - # and collect their consumer_head_tag_names - rows = rows - .group_by(&:pact_publication_id) - .values - .collect{|group| [group.last, group.collect{|r| r[:consumer_version_tag_name]}.compact] } - .collect{ |(row, tag_names)| row.consumer_head_tag_names = tag_names; row } + rows = rows.eager(:consumer_version_tags) + .eager(:provider_version_tags) + .eager(:latest_verification_for_consumer_version_tag) end + rows = rows.all.group_by(&:pact_publication_id).values.collect{ | rows| Matrix::AggregatedRow.new(rows) } - index_items = [] - rows.sort.each do | row | - tag_names = [] - if options[:tags] - tag_names = row.consumer_version_tags.collect(&:name) - end - - overall_latest = overall_latest_publication_ids.include?(row.pact_publication_id) - latest_verification = if overall_latest - verification_repository.find_latest_verification_for row.consumer_name, row.provider_name - else - tag_names.collect do | tag_name | - verification_repository.find_latest_verification_for row.consumer_name, row.provider_name, tag_name - end.compact.sort do | v1, v2 | - # Some provider versions have nil orders, not sure why - # Sort by execution_date instead of order - v1.execution_date <=> v2.execution_date - end.last - end - - index_items << PactBroker::Domain::IndexItem.create( + rows.sort.collect do | row | + PactBroker::Domain::IndexItem.create( row.consumer, row.provider, row.pact, - overall_latest, - latest_verification, + row.overall_latest?, + options[:tags] ? row.latest_verification : row.verification, row.webhooks, row.latest_triggered_webhooks, - row.consumer_head_tag_names, - row.provider_version_tags.select(&:latest?) + options[:tags] ? row.consumer_head_tag_names : [], + options[:tags] ? row.provider_version_tags.select(&:latest?) : [] ) end - - index_items - end - - def self.tags_for(pact, options) - if options[:tags] == true - tag_service.find_all_tag_names_for_pacticipant(pact.consumer_name) - elsif options[:tags].is_a?(Array) - options[:tags] - else - [] - end - end - - def self.build_index_item_rows(pact, tags) - index_items = [build_latest_pact_index_item(pact, tags)] - tags.each do | tag | - index_items << build_index_item_for_tagged_pact(pact, tag) - end - index_items.compact - end - - def self.build_latest_pact_index_item pact, tags - latest_verification = verification_service.find_latest_verification_for(pact.consumer, pact.provider) - webhooks = webhook_service.find_by_consumer_and_provider pact.consumer, pact.provider - triggered_webhooks = webhook_service.find_latest_triggered_webhooks pact.consumer, pact.provider - tag_names = pact.consumer_version_tag_names.select{ |name| tags.include?(name) } - PactBroker::Domain::IndexItem.create pact.consumer, pact.provider, pact, true, latest_verification, webhooks, triggered_webhooks, tag_names - end - - def self.build_index_item_for_tagged_pact latest_pact, tag - pact = pact_service.find_latest_pact consumer_name: latest_pact.consumer_name, provider_name: latest_pact.provider_name, tag: tag - return nil unless pact - return nil if pact.id == latest_pact.id - verification = verification_repository.find_latest_verification_for pact.consumer_name, pact.provider_name, tag - PactBroker::Domain::IndexItem.create pact.consumer, pact.provider, pact, false, verification, [], [], [tag] end end end diff --git a/lib/pact_broker/matrix/aggregated_row.rb b/lib/pact_broker/matrix/aggregated_row.rb new file mode 100644 index 000000000..c6c45f16c --- /dev/null +++ b/lib/pact_broker/matrix/aggregated_row.rb @@ -0,0 +1,57 @@ +require 'pact_broker/verifications/repository' + +# A collection of matrix rows with the same pact publication id +# It's basically a normalised view of a denormalised view :( + +module PactBroker + module Matrix + class AggregatedRow + extend Forwardable + + delegate [:consumer, :consumer_name, :consumer_version, :consumer_version_number, :consumer_version_order, :consumer_version_tags] => :first_row + delegate [:provider, :provider_name, :provider_version, :provider_version_number, :provider_version_order, :provider_version_tags] => :first_row + delegate [:pact, :pact_version, :pact_revision_number, :webhooks, :latest_triggered_webhooks, :'<=>'] => :first_row + delegate [:verification_id, :verification] => :first_row + + + def initialize matrix_rows + @matrix_rows = matrix_rows + @first_row = matrix_rows.first + end + + def overall_latest? + !!matrix_rows.find{ | row| row.consumer_version_tag_name.nil? } + end + + def latest_verification + @latest_verification ||= begin + verification = matrix_rows.collect do | row| + row.verification || row.latest_verification_for_consumer_version_tag + end.compact.sort{ |v1, v2| v1.id <=> v2.id}.last + + if !verification && overall_latest? + PactBroker::Verifications::Repository.new.find_latest_verification_for(consumer_name, provider_name) + else + verification + end + end + end + + def consumer_head_tag_names + matrix_rows.collect(&:consumer_version_tag_name).compact + end + + private + + attr_reader :matrix_rows + + def first_row + @first_row + end + + def consumer_version_tag_names + matrix_rows.collect(&:consumer_version_tag_name) + end + end + end +end diff --git a/lib/pact_broker/matrix/head_row.rb b/lib/pact_broker/matrix/head_row.rb index 816d6447e..cbf6462b3 100644 --- a/lib/pact_broker/matrix/head_row.rb +++ b/lib/pact_broker/matrix/head_row.rb @@ -7,6 +7,27 @@ module Matrix class HeadRow < Row set_dataset(:materialized_head_matrix) + # one_to_one :latest_verification_for_consumer_version_tag, + # :class => "PactBroker::Verifications::LatestVerificationForConsumerVersionTag", + # primary_key: [:provider_id, :consumer_id, :consumer_version_tag_name], key: [:provider_id, :consumer_id, :consumer_version_tag_name] + + # Loading the latest_verification_for_consumer_version_tag objects this way is quicker than + # doing it using an inbult relation with primary_key/key, if we are loading the relation for + # the entire HeadRow table + # Using the inbuilt relation puts constraints on the columns that are not necessary and slow + # the query down. + one_to_one :latest_verification_for_consumer_version_tag, :class => "PactBroker::Verifications::LatestVerificationForConsumerVersionTag", primary_keys: [], key: [], :eager_loader=>(proc do |eo_opts| + tag_to_row = eo_opts[:rows].each_with_object({}) { | row, map | map[[row.provider_id, row.consumer_id, row.consumer_version_tag_name]] = row } + eo_opts[:rows].each{|row| row.associations[:latest_verification_for_consumer_version_tag] = nil} + + PactBroker::Verifications::LatestVerificationForConsumerVersionTag.each do | verification | + key = [verification.provider_id, verification.consumer_id, verification.consumer_version_tag_name] + if tag_to_row[key] + tag_to_row[key].associations[:latest_verification_for_consumer_version_tag] = verification + end + end + end) + dataset_module do include PactBroker::Repositories::Helpers include PactBroker::Logging diff --git a/lib/pact_broker/matrix/row.rb b/lib/pact_broker/matrix/row.rb index a19f9e702..bdf3a286a 100644 --- a/lib/pact_broker/matrix/row.rb +++ b/lib/pact_broker/matrix/row.rb @@ -17,17 +17,7 @@ class Row < Sequel::Model(:materialized_matrix) associate(:one_to_many, :consumer_version_tags, :class => "PactBroker::Tags::TagWithLatestFlag", primary_key: :consumer_version_id, key: :version_id) associate(:one_to_many, :provider_version_tags, :class => "PactBroker::Tags::TagWithLatestFlag", primary_key: :provider_version_id, key: :version_id) associate(:many_to_one, :verification, :class => "PactBroker::Domain::Verification", primary_key: :id, key: :verification_id) - - one_to_one :latest_verification_for_consumer_version_tag, primary_keys: [], key: [], :eager_loader=>(proc do |eo_opts| - tag_to_row = eo_opts[:rows].each_with_object({}) { | row, map | row.consumer_version_tags.each{ | tag | map[tag.name] = row } } - eo_opts[:rows].each{|row| row.associations[:latest_verification_for_consumer_version_tag] = nil} - - PactBroker::Verifications::LatestVerificationForConsumerVersionTag.each do | verification | - if tag_to_row[verification.consumer_version_tag_name] - tag_to_row[verification.consumer_version_tag_name].associations[:latest_verification_for_consumer_version_tag] = verification - end - end - end) + # associate(:many_to_one, :pact_version, :class => "PactBroker::Pacts::PactVersion", primary_key: :id, key: :pact_version_id) dataset_module do include PactBroker::Repositories::Helpers diff --git a/spec/lib/pact_broker/matrix/aggregated_row_spec.rb b/spec/lib/pact_broker/matrix/aggregated_row_spec.rb new file mode 100644 index 000000000..43dd2dd20 --- /dev/null +++ b/spec/lib/pact_broker/matrix/aggregated_row_spec.rb @@ -0,0 +1,80 @@ +require 'pact_broker/matrix/aggregated_row' + +module PactBroker + module Matrix + describe AggregatedRow do + describe "latest_verification" do + let(:row_1) do + instance_double('PactBroker::Matrix::HeadRow', + consumer_name: "Foo", + provider_name: "Bar", + verification: verification_1, + latest_verification_for_consumer_version_tag: tag_verification_1, + consumer_version_tag_name: consumer_version_tag_name_1) + end + let(:row_2) do + instance_double('PactBroker::Matrix::HeadRow', + verification: verification_2, + latest_verification_for_consumer_version_tag: tag_verification_2, + consumer_version_tag_name: consumer_version_tag_name_2) + end + let(:verification_1) { instance_double('PactBroker::Domain::Verification', id: 1) } + let(:verification_2) { instance_double('PactBroker::Domain::Verification', id: 2) } + let(:tag_verification_1) { instance_double('PactBroker::Domain::Verification', id: 3) } + let(:tag_verification_2) { instance_double('PactBroker::Domain::Verification', id: 4) } + let(:consumer_version_tag_name_1) { 'master' } + let(:consumer_version_tag_name_2) { 'prod' } + let(:rows) { [row_1, row_2] } + let(:aggregated_row) { AggregatedRow.new(rows) } + + subject { aggregated_row.latest_verification } + + context "when the rows have verifications" do + it "returns the verification with the largest id" do + expect(subject).to be verification_2 + end + end + + context "when the rows do not have verifications, but there are a previous verifications for a pacts with the same tag" do + let(:verification_1) { nil } + let(:verification_2) { nil } + + it "returns the verification for the previous pact that has the largest id" do + expect(subject).to be tag_verification_2 + end + end + + context "when there is no verification for any of the rows or any of the pacts with the same tag" do + before do + allow_any_instance_of(PactBroker::Verifications::Repository).to receive(:find_latest_verification_for).and_return(overall_latest_verification) + end + + let(:overall_latest_verification) { instance_double('PactBroker::Domain::Verification', id: 5) } + let(:verification_1) { nil } + let(:verification_2) { nil } + let(:tag_verification_1) { nil } + let(:tag_verification_2) { nil } + + context "when one of the rows is the overall latest" do + let(:consumer_version_tag_name_1) { nil } + + it "looks up the overall latest verification" do + expect_any_instance_of(PactBroker::Verifications::Repository).to receive(:find_latest_verification_for).with("Foo", "Bar") + subject + end + + it "returns the overall latest verification" do + expect(subject).to be overall_latest_verification + end + end + + context "when none of the rows is not the overall latest (they are all the latest with a tag)" do + it "returns nil" do + expect(subject).to be nil + end + end + end + end + end + end +end diff --git a/spec/lib/pact_broker/matrix/head_row_spec.rb b/spec/lib/pact_broker/matrix/head_row_spec.rb index 5f16aa9fa..b8f47ef9c 100644 --- a/spec/lib/pact_broker/matrix/head_row_spec.rb +++ b/spec/lib/pact_broker/matrix/head_row_spec.rb @@ -3,6 +3,61 @@ module PactBroker module Matrix describe HeadRow do + let(:td) { TestDataBuilder.new } + + describe "latest_verification_for_consumer_version_tag" do + context "when the pact with a given tag has been verified" do + before do + td.create_pact_with_hierarchy("Foo", "1", "Bar") + .create_consumer_version_tag("prod") + .create_verification(provider_version: "10") + .create_consumer_version("2", comment: "latest prod version for Foo") + .create_consumer_version_tag("prod") + .create_pact + .create_verification(provider_version: "11") + .create_verification(provider_version: "12", number: 2) + .create_consumer("Wiffle") + .create_consumer_version("30", comment: "latest prod version for Wiffle") + .create_consumer_version_tag("prod") + .create_pact + .create_verification(provider_version: "12") + .create_provider("Meep") + .create_pact + .create_verification(provider_version: "40") + end + + subject { HeadRow.eager(:consumer_version_tags).eager(:latest_verification_for_consumer_version_tag).order(:pact_publication_id, :verification_id).exclude(consumer_version_tag_name: nil).all } + + it "returns its most recent verification" do + cols = [:consumer_name, :consumer_version_number, :consumer_version_tag_name, :provider_name, :provider_version_number] + rows = subject.collect{ | row | cols.collect{ | col | row[col]} } + + expect(subject.size).to eq 3 + expect(rows).to include ["Foo", "2", "prod", "Bar", "12"] + expect(rows).to include ["Wiffle", "30", "prod", "Bar", "12"] + expect(rows).to include ["Wiffle", "30", "prod", "Meep", "40"] + end + end + + context "when the most recent pact with a given tag has not been verified, but a previous version with the same tag has" do + before do + td.create_pact_with_hierarchy("Foo", "1", "Bar") + .create_consumer_version_tag("prod") + .create_verification(provider_version: "10") + .create_verification(provider_version: "11", number: 2, comment: "this is the latest verification for a pact with cv tag prod") + .create_consumer_version("2") + .create_consumer_version_tag("prod") + .create_pact + end + + subject { HeadRow.eager(:consumer_version_tags).eager(:latest_verification_for_consumer_version_tag).order(:pact_publication_id).all } + + it "returns the most recent verification for the previous version with the same tag" do + expect(subject.last.verification_id).to be nil # this pact version has not been verified directly + expect(subject.last.latest_verification_for_consumer_version_tag.provider_version.number).to eq "11" + end + end + end describe "refresh", migration: true do before do PactBroker::Database.migrate diff --git a/spec/lib/pact_broker/matrix/row_spec.rb b/spec/lib/pact_broker/matrix/row_spec.rb index d59d45ac8..b2df06730 100644 --- a/spec/lib/pact_broker/matrix/row_spec.rb +++ b/spec/lib/pact_broker/matrix/row_spec.rb @@ -5,47 +5,6 @@ module Matrix describe Row do let(:td) { TestDataBuilder.new } - describe "latest_verification_for_consumer_version_tag" do - context "when the pact with a given tag has been verified" do - before do - td.create_pact_with_hierarchy("Foo", "1", "Bar") - .create_consumer_version_tag("prod") - .create_verification(provider_version: "10") - .create_consumer_version("2") - .create_consumer_version_tag("prod") - .create_pact - .create_verification(provider_version: "11", number: 1) - .create_verification(provider_version: "12", number: 2) - end - - subject { Row.eager(:consumer_version_tags).eager(:latest_verification_for_consumer_version_tag).order(:pact_publication_id, :verification_id).all } - - it "returns its most recent verification" do - expect(subject.last.provider_version_number).to eq "12" - expect(subject.last.latest_verification_for_consumer_version_tag.provider_version.number).to eq "12" - end - end - - context "when the most recent pact with a given tag has not been verified, but a previous version with the same tag has" do - before do - td.create_pact_with_hierarchy("Foo", "1", "Bar") - .create_consumer_version_tag("prod") - .create_verification(provider_version: "10") - .create_verification(provider_version: "11", number: 2, comment: "this is the latest verification for a pact with cv tag prod") - .create_consumer_version("2") - .create_consumer_version_tag("prod") - .create_pact - end - - subject { Row.eager(:consumer_version_tags).eager(:latest_verification_for_consumer_version_tag).order(:pact_publication_id).all } - - it "returns the most recent verification for the previous version with the same tag" do - expect(subject.last.verification_id).to be nil # this pact version has not been verified directly - expect(subject.last.latest_verification_for_consumer_version_tag.provider_version.number).to eq "11" - end - end - end - describe "refresh", migration: true do let(:td) { TestDataBuilder.new(auto_refresh_matrix: false) } diff --git a/spec/support/rspec_match_hash.rb b/spec/support/rspec_match_hash.rb index c5a7046d8..0acad199a 100644 --- a/spec/support/rspec_match_hash.rb +++ b/spec/support/rspec_match_hash.rb @@ -4,26 +4,23 @@ match do |actual| contains_hash?(expected, actual) end -end + failure_message do |actual| + "expected #{actual.class} to include #{expected.class}\n" + formatted_diffs + end -def contains_hash?(expected, actual) - if actual.is_a?(Array) - actual.any? && actual.any?{|actual_item| contains_hash?(expected, actual_item)} - else - expected.all? do |key, value| - unordered_match(actual[key], value) - end + def formatted_diffs + @diffs.collect{ | diff| Pact::Matchers::UnixDiffFormatter.call(diff) }.join("\n") end -end -def unordered_match(expected, actual) - case - when [expected, actual].all?{|val| val.is_a? Array } - expected.all?{|el| actual.include? el } - when [expected, actual].all?{|val| val.is_a? Hash } - contains_hash?(expected, actual) - else - expected == actual + def contains_hash?(expected, actual) + if actual.is_a?(Array) + actual.any? && actual.any?{|actual_item| contains_hash?(expected, actual_item)} + else + @diffs ||= [] + diff = Pact::Matchers.diff(expected, actual.to_hash) + @diffs << diff + diff.empty? + end end end From ee4e93d8cb0d97ae211c1ae4c0b282c335bb0815 Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Fri, 25 May 2018 19:03:03 +1000 Subject: [PATCH 4/7] chore: comments, rename file --- lib/pact_broker/matrix/head_row.rb | 1 + lib/pact_broker/matrix/row.rb | 1 - .../domain/{index_items_spec.rb => index_item_spec.rb} | 0 3 files changed, 1 insertion(+), 1 deletion(-) rename spec/lib/pact_broker/domain/{index_items_spec.rb => index_item_spec.rb} (100%) diff --git a/lib/pact_broker/matrix/head_row.rb b/lib/pact_broker/matrix/head_row.rb index cbf6462b3..3c65d006e 100644 --- a/lib/pact_broker/matrix/head_row.rb +++ b/lib/pact_broker/matrix/head_row.rb @@ -16,6 +16,7 @@ class HeadRow < Row # the entire HeadRow table # Using the inbuilt relation puts constraints on the columns that are not necessary and slow # the query down. + # This relation relies on consumer_version_tags already being loaded one_to_one :latest_verification_for_consumer_version_tag, :class => "PactBroker::Verifications::LatestVerificationForConsumerVersionTag", primary_keys: [], key: [], :eager_loader=>(proc do |eo_opts| tag_to_row = eo_opts[:rows].each_with_object({}) { | row, map | map[[row.provider_id, row.consumer_id, row.consumer_version_tag_name]] = row } eo_opts[:rows].each{|row| row.associations[:latest_verification_for_consumer_version_tag] = nil} diff --git a/lib/pact_broker/matrix/row.rb b/lib/pact_broker/matrix/row.rb index bdf3a286a..d401c8c8a 100644 --- a/lib/pact_broker/matrix/row.rb +++ b/lib/pact_broker/matrix/row.rb @@ -17,7 +17,6 @@ class Row < Sequel::Model(:materialized_matrix) associate(:one_to_many, :consumer_version_tags, :class => "PactBroker::Tags::TagWithLatestFlag", primary_key: :consumer_version_id, key: :version_id) associate(:one_to_many, :provider_version_tags, :class => "PactBroker::Tags::TagWithLatestFlag", primary_key: :provider_version_id, key: :version_id) associate(:many_to_one, :verification, :class => "PactBroker::Domain::Verification", primary_key: :id, key: :verification_id) - # associate(:many_to_one, :pact_version, :class => "PactBroker::Pacts::PactVersion", primary_key: :id, key: :pact_version_id) dataset_module do include PactBroker::Repositories::Helpers diff --git a/spec/lib/pact_broker/domain/index_items_spec.rb b/spec/lib/pact_broker/domain/index_item_spec.rb similarity index 100% rename from spec/lib/pact_broker/domain/index_items_spec.rb rename to spec/lib/pact_broker/domain/index_item_spec.rb From e366af496ae375de8f56cfdb81d1681190606cb4 Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Fri, 25 May 2018 20:50:10 +1000 Subject: [PATCH 5/7] feat: include more columns in latest_verifications_for_consumer_version_tags to avoid having to do extra queries for pact_versions and provider_versions --- ...verifications_for_consumer_version_tags.rb | 37 +++++++++++++++---- lib/pact_broker/index/service.rb | 5 ++- lib/pact_broker/matrix/aggregated_row.rb | 27 +++++++++++--- lib/pact_broker/matrix/head_row.rb | 1 + lib/pact_broker/matrix/row.rb | 29 ++++----------- ...t_verification_for_consumer_version_tag.rb | 16 +++++++- 6 files changed, 79 insertions(+), 36 deletions(-) diff --git a/db/migrations/20180523_create_latest_verifications_for_consumer_version_tags.rb b/db/migrations/20180523_create_latest_verifications_for_consumer_version_tags.rb index 0c41e371d..21c04e9ae 100644 --- a/db/migrations/20180523_create_latest_verifications_for_consumer_version_tags.rb +++ b/db/migrations/20180523_create_latest_verifications_for_consumer_version_tags.rb @@ -2,7 +2,11 @@ up do # The latest verification id for each consumer version tag create_view(:latest_verifications_ids_for_consumer_version_tags, - "select pv.pacticipant_id as provider_id, lpp.consumer_id, t.name as consumer_version_tag_name, max(v.id) as latest_verification_id + "select + pv.pacticipant_id as provider_id, + lpp.consumer_id, + t.name as consumer_version_tag_name, + max(v.id) as latest_verification_id from verifications v join latest_pact_publications_by_consumer_versions lpp on v.pact_version_id = lpp.pact_version_id @@ -12,12 +16,31 @@ on v.provider_version_id = pv.id group by pv.pacticipant_id, lpp.consumer_id, t.name") - # The latest verification for each consumer version tag - create_view(:latest_verifications_for_consumer_version_tags, - "select v.*, lv.provider_id, lv.consumer_id, lv.consumer_version_tag_name - from verifications v - join latest_verifications_ids_for_consumer_version_tags lv - on lv.latest_verification_id = v.id") + # The most recent verification for each consumer/consumer version tag/provider + latest_verifications = from(:verifications) + .select( + Sequel[:lv][:consumer_id], + Sequel[:lv][:provider_id], + Sequel[:lv][:consumer_version_tag_name], + Sequel[:pv][:sha].as(:pact_version_sha), + Sequel[:prv][:number].as(:provider_version_number), + Sequel[:prv][:order].as(:provider_version_order), + ) + .select_append{ verifications.* } + .join(:latest_verifications_ids_for_consumer_version_tags, + { + Sequel[:verifications][:id] => Sequel[:lv][:latest_verification_id], + }, { table_alias: :lv }) + .join(:versions, + { + Sequel[:verifications][:provider_version_id] => Sequel[:prv][:id] + }, { table_alias: :prv }) + .join(:pact_versions, + { + Sequel[:verifications][:pact_version_id] => Sequel[:pv][:id] + }, { table_alias: :pv }) + + create_or_replace_view(:latest_verifications_for_consumer_version_tags, latest_verifications) end down do diff --git a/lib/pact_broker/index/service.rb b/lib/pact_broker/index/service.rb index 30ff4b460..db02bad8c 100644 --- a/lib/pact_broker/index/service.rb +++ b/lib/pact_broker/index/service.rb @@ -19,7 +19,7 @@ def self.find_index_items options = {} .eager(:latest_triggered_webhooks) .eager(:webhooks) .order(:consumer_name, :provider_name) - .eager(:verification) + #.eager(verification: [:provider_version, :pact_version]) if !options[:tags] rows = rows.where(consumer_version_tag_name: nil) @@ -34,12 +34,13 @@ def self.find_index_items options = {} rows = rows.all.group_by(&:pact_publication_id).values.collect{ | rows| Matrix::AggregatedRow.new(rows) } rows.sort.collect do | row | + # TODO simplify PactBroker::Domain::IndexItem.create( row.consumer, row.provider, row.pact, row.overall_latest?, - options[:tags] ? row.latest_verification : row.verification, + row.latest_verification, row.webhooks, row.latest_triggered_webhooks, options[:tags] ? row.consumer_head_tag_names : [], diff --git a/lib/pact_broker/matrix/aggregated_row.rb b/lib/pact_broker/matrix/aggregated_row.rb index c6c45f16c..0bc5b78e2 100644 --- a/lib/pact_broker/matrix/aggregated_row.rb +++ b/lib/pact_broker/matrix/aggregated_row.rb @@ -2,7 +2,7 @@ # A collection of matrix rows with the same pact publication id # It's basically a normalised view of a denormalised view :( - +# A pact publication may be the overall latest, and/or the latest for a tag module PactBroker module Matrix class AggregatedRow @@ -13,7 +13,6 @@ class AggregatedRow delegate [:pact, :pact_version, :pact_revision_number, :webhooks, :latest_triggered_webhooks, :'<=>'] => :first_row delegate [:verification_id, :verification] => :first_row - def initialize matrix_rows @matrix_rows = matrix_rows @first_row = matrix_rows.first @@ -23,20 +22,26 @@ def overall_latest? !!matrix_rows.find{ | row| row.consumer_version_tag_name.nil? } end + # If this comes back nil, it won't be "cached", but it's a reasonably + # quick query, so it's probably ok. def latest_verification @latest_verification ||= begin verification = matrix_rows.collect do | row| - row.verification || row.latest_verification_for_consumer_version_tag - end.compact.sort{ |v1, v2| v1.id <=> v2.id}.last + row.verification || latest_verification_for_consumer_version_tag(row) + end.compact.sort{ |v1, v2| v1.id <=> v2.id }.last if !verification && overall_latest? - PactBroker::Verifications::Repository.new.find_latest_verification_for(consumer_name, provider_name) + overall_latest_verification else verification end end end + # The list of tag names for which this pact publication is the most recent with that tag + # There could, however, be a later consumer version that does't have a pact (perhaps because it was deleted) + # that has the same tag. + # TODO show a warning when the data is "corrupted" as above. def consumer_head_tag_names matrix_rows.collect(&:consumer_version_tag_name).compact end @@ -45,6 +50,18 @@ def consumer_head_tag_names attr_reader :matrix_rows + def latest_verification_for_consumer_version_tag row + row.latest_verification_for_consumer_version_tag if row.consumer_version_tag_name + end + + def overall_latest_verification + # This causes the + # SELECT "latest_verifications".* FROM "latest_verifications" + # query in the logs for the tagged index. + # Given it only happens rarely, it's ok to not lazy load it. + PactBroker::Verifications::Repository.new.find_latest_verification_for(consumer_name, provider_name) + end + def first_row @first_row end diff --git a/lib/pact_broker/matrix/head_row.rb b/lib/pact_broker/matrix/head_row.rb index 3c65d006e..c2e8e8001 100644 --- a/lib/pact_broker/matrix/head_row.rb +++ b/lib/pact_broker/matrix/head_row.rb @@ -21,6 +21,7 @@ class HeadRow < Row tag_to_row = eo_opts[:rows].each_with_object({}) { | row, map | map[[row.provider_id, row.consumer_id, row.consumer_version_tag_name]] = row } eo_opts[:rows].each{|row| row.associations[:latest_verification_for_consumer_version_tag] = nil} + # Need the all then the each to ensure the eager loading works PactBroker::Verifications::LatestVerificationForConsumerVersionTag.each do | verification | key = [verification.provider_id, verification.consumer_id, verification.consumer_version_tag_name] if tag_to_row[key] diff --git a/lib/pact_broker/matrix/row.rb b/lib/pact_broker/matrix/row.rb index d401c8c8a..045f52f70 100644 --- a/lib/pact_broker/matrix/row.rb +++ b/lib/pact_broker/matrix/row.rb @@ -16,7 +16,6 @@ class Row < Sequel::Model(:materialized_matrix) associate(:one_to_many, :webhooks, :class => "PactBroker::Webhooks::Webhook", primary_key: [:consumer_id, :provider_id], key: [:consumer_id, :provider_id]) associate(:one_to_many, :consumer_version_tags, :class => "PactBroker::Tags::TagWithLatestFlag", primary_key: :consumer_version_id, key: :version_id) associate(:one_to_many, :provider_version_tags, :class => "PactBroker::Tags::TagWithLatestFlag", primary_key: :provider_version_id, key: :version_id) - associate(:many_to_one, :verification, :class => "PactBroker::Domain::Verification", primary_key: :id, key: :verification_id) dataset_module do include PactBroker::Repositories::Helpers @@ -114,20 +113,6 @@ def [] key end end - # tags for which this pact publication is the latest of that tag - # this is set in the code rather than the database - def consumer_head_tag_names - @consumer_head_tag_names ||= [] - end - - def consumer_head_tag_names= consumer_head_tag_names - @consumer_head_tag_names = consumer_head_tag_names - end - - # def latest_triggered_webhooks - # @latest_triggered_webhooks ||= [] - # end - def summary "#{consumer_name}#{consumer_version_number} #{provider_name}#{provider_version_number || '?'} (r#{pact_revision_number}n#{verification_number || '?'})" end @@ -141,17 +126,18 @@ def provider end def consumer_version - @consumer_version ||= OpenStruct.new(number: consumer_version_number, id: consumer_version_id, pacticipant: consumer) + @consumer_version ||= OpenStruct.new(number: consumer_version_number, order: consumer_version_order, id: consumer_version_id, pacticipant: consumer) end def provider_version if provider_version_number - @provider_version ||= OpenStruct.new(number: provider_version_number, id: provider_version_id, pacticipant: provider) + @provider_version ||= OpenStruct.new(number: provider_version_number, order: provider_version_order, id: provider_version_id, pacticipant: provider) end end def pact - @pact ||= OpenStruct.new(consumer: consumer, + @pact ||= OpenStruct.new( + consumer: consumer, provider: provider, consumer_version: consumer_version, consumer_version_number: consumer_version_number, @@ -161,7 +147,7 @@ def pact ) end - def latest_verification + def verification if verification_executed_at @latest_verification ||= OpenStruct.new( id: verification_id, @@ -170,10 +156,12 @@ def latest_verification execution_date: verification_executed_at, created_at: verification_executed_at, provider_version_number: provider_version_number, + provider_version_order: provider_version_order, build_url: verification_build_url, provider_version: provider_version, consumer_name: consumer_name, - provider_name: provider_name + provider_name: provider_name, + pact_version_sha: pact_version_sha ) end end @@ -190,7 +178,6 @@ def <=> other ] comparisons.find{|c| c != 0 } || 0 - end def compare_name_asc name1, name2 diff --git a/lib/pact_broker/verifications/latest_verification_for_consumer_version_tag.rb b/lib/pact_broker/verifications/latest_verification_for_consumer_version_tag.rb index 2c07b4541..b4398d329 100644 --- a/lib/pact_broker/verifications/latest_verification_for_consumer_version_tag.rb +++ b/lib/pact_broker/verifications/latest_verification_for_consumer_version_tag.rb @@ -4,6 +4,20 @@ module PactBroker module Verifications class LatestVerificationForConsumerVersionTag < PactBroker::Domain::Verification set_dataset(:latest_verifications_for_consumer_version_tags) + + def pact_version_sha + # Don't need to load the pact_version as we do in the superclass, + # as pact_version_sha is included in the view for convenience + values[:pact_version_sha] + end + + def provider_version_number + values[:provider_version_number] + end + + def provider_version_order + values[:provider_version_order] + end end end -end \ No newline at end of file +end From 13b7c6e3aa780940b163f927b32c883bab3dce8a Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Sat, 26 May 2018 20:32:55 +1000 Subject: [PATCH 6/7] feat: load latest verification for consumer/provider via relationship rather than repository --- ...verifications_for_consumer_version_tags.rb | 6 +-- ...verifications_for_consumer_and_provider.rb | 46 +++++++++++++++++++ lib/pact_broker/index/service.rb | 2 - lib/pact_broker/matrix/aggregated_row.rb | 7 +-- lib/pact_broker/matrix/row.rb | 5 ++ ..._verification_for_consumer_and_provider.rb | 26 +++++++++++ ...t_verification_for_consumer_version_tag.rb | 4 +- .../pact_broker/matrix/aggregated_row_spec.rb | 13 +++--- spec/lib/pact_broker/matrix/row_spec.rb | 21 +++++++++ 9 files changed, 111 insertions(+), 19 deletions(-) create mode 100644 db/migrations/20180524_create_latest_verifications_for_consumer_and_provider.rb create mode 100644 lib/pact_broker/verifications/latest_verification_for_consumer_and_provider.rb diff --git a/db/migrations/20180523_create_latest_verifications_for_consumer_version_tags.rb b/db/migrations/20180523_create_latest_verifications_for_consumer_version_tags.rb index 21c04e9ae..b93cead9d 100644 --- a/db/migrations/20180523_create_latest_verifications_for_consumer_version_tags.rb +++ b/db/migrations/20180523_create_latest_verifications_for_consumer_version_tags.rb @@ -1,7 +1,7 @@ Sequel.migration do up do # The latest verification id for each consumer version tag - create_view(:latest_verifications_ids_for_consumer_version_tags, + create_view(:latest_verification_ids_for_consumer_version_tags, "select pv.pacticipant_id as provider_id, lpp.consumer_id, @@ -27,7 +27,7 @@ Sequel[:prv][:order].as(:provider_version_order), ) .select_append{ verifications.* } - .join(:latest_verifications_ids_for_consumer_version_tags, + .join(:latest_verification_ids_for_consumer_version_tags, { Sequel[:verifications][:id] => Sequel[:lv][:latest_verification_id], }, { table_alias: :lv }) @@ -45,6 +45,6 @@ down do drop_view(:latest_verifications_for_consumer_version_tags) - drop_view(:latest_verifications_ids_for_consumer_version_tags) + drop_view(:latest_verification_ids_for_consumer_version_tags) end end diff --git a/db/migrations/20180524_create_latest_verifications_for_consumer_and_provider.rb b/db/migrations/20180524_create_latest_verifications_for_consumer_and_provider.rb new file mode 100644 index 000000000..eb7dbe4a5 --- /dev/null +++ b/db/migrations/20180524_create_latest_verifications_for_consumer_and_provider.rb @@ -0,0 +1,46 @@ +Sequel.migration do + up do + # The latest verification id for each consumer version tag + create_view(:latest_verification_ids_for_consumer_and_provider, + "select + pv.pacticipant_id as provider_id, + lpp.consumer_id, + max(v.id) as latest_verification_id + from verifications v + join latest_pact_publications_by_consumer_versions lpp + on v.pact_version_id = lpp.pact_version_id + join versions pv + on v.provider_version_id = pv.id + group by pv.pacticipant_id, lpp.consumer_id") + + # The most recent verification for each consumer/consumer version tag/provider + latest_verifications = from(:verifications) + .select( + Sequel[:lv][:consumer_id], + Sequel[:lv][:provider_id], + Sequel[:pv][:sha].as(:pact_version_sha), + Sequel[:prv][:number].as(:provider_version_number), + Sequel[:prv][:order].as(:provider_version_order), + ) + .select_append{ verifications.* } + .join(:latest_verification_ids_for_consumer_and_provider, + { + Sequel[:verifications][:id] => Sequel[:lv][:latest_verification_id], + }, { table_alias: :lv }) + .join(:versions, + { + Sequel[:verifications][:provider_version_id] => Sequel[:prv][:id] + }, { table_alias: :prv }) + .join(:pact_versions, + { + Sequel[:verifications][:pact_version_id] => Sequel[:pv][:id] + }, { table_alias: :pv }) + + create_or_replace_view(:latest_verifications_for_consumer_and_provider, latest_verifications) + end + + down do + drop_view(:latest_verifications_for_consumer_and_provider) + drop_view(:latest_verification_ids_for_consumer_and_provider) + end +end diff --git a/lib/pact_broker/index/service.rb b/lib/pact_broker/index/service.rb index db02bad8c..71eb77a23 100644 --- a/lib/pact_broker/index/service.rb +++ b/lib/pact_broker/index/service.rb @@ -18,8 +18,6 @@ def self.find_index_items options = {} .select_all_qualified .eager(:latest_triggered_webhooks) .eager(:webhooks) - .order(:consumer_name, :provider_name) - #.eager(verification: [:provider_version, :pact_version]) if !options[:tags] rows = rows.where(consumer_version_tag_name: nil) diff --git a/lib/pact_broker/matrix/aggregated_row.rb b/lib/pact_broker/matrix/aggregated_row.rb index 0bc5b78e2..14d8aceb3 100644 --- a/lib/pact_broker/matrix/aggregated_row.rb +++ b/lib/pact_broker/matrix/aggregated_row.rb @@ -55,11 +55,8 @@ def latest_verification_for_consumer_version_tag row end def overall_latest_verification - # This causes the - # SELECT "latest_verifications".* FROM "latest_verifications" - # query in the logs for the tagged index. - # Given it only happens rarely, it's ok to not lazy load it. - PactBroker::Verifications::Repository.new.find_latest_verification_for(consumer_name, provider_name) + # not eager loaded because it shouldn't be called that often + first_row.latest_verification_for_consumer_and_provider end def first_row diff --git a/lib/pact_broker/matrix/row.rb b/lib/pact_broker/matrix/row.rb index 045f52f70..cbfc92755 100644 --- a/lib/pact_broker/matrix/row.rb +++ b/lib/pact_broker/matrix/row.rb @@ -3,6 +3,7 @@ require 'pact_broker/tags/tag_with_latest_flag' require 'pact_broker/logging' require 'pact_broker/verifications/latest_verification_for_consumer_version_tag' +require 'pact_broker/verifications/latest_verification_for_consumer_and_provider' module PactBroker module Matrix @@ -17,6 +18,10 @@ class Row < Sequel::Model(:materialized_matrix) associate(:one_to_many, :consumer_version_tags, :class => "PactBroker::Tags::TagWithLatestFlag", primary_key: :consumer_version_id, key: :version_id) associate(:one_to_many, :provider_version_tags, :class => "PactBroker::Tags::TagWithLatestFlag", primary_key: :provider_version_id, key: :version_id) + many_to_one :latest_verification_for_consumer_and_provider, + :class => "PactBroker::Verifications::LatestVerificationForConsumerAndProvider", + primary_key: [:provider_id, :consumer_id], key: [:provider_id, :consumer_id] + dataset_module do include PactBroker::Repositories::Helpers include PactBroker::Logging diff --git a/lib/pact_broker/verifications/latest_verification_for_consumer_and_provider.rb b/lib/pact_broker/verifications/latest_verification_for_consumer_and_provider.rb new file mode 100644 index 000000000..e52f8d806 --- /dev/null +++ b/lib/pact_broker/verifications/latest_verification_for_consumer_and_provider.rb @@ -0,0 +1,26 @@ +require 'pact_broker/domain/verification' + +module PactBroker + module Verifications + + include PactBroker::Repositories::Helpers + + class LatestVerificationForConsumerAndProvider < PactBroker::Domain::Verification + set_dataset(:latest_verifications_for_consumer_and_provider) + + # Don't need to load the pact_version as we do in the superclass, + # as pact_version_sha is included in the view for convenience + def pact_version_sha + values[:pact_version_sha] + end + + def provider_version_number + values[:provider_version_number] + end + + def provider_version_order + values[:provider_version_order] + end + end + end +end diff --git a/lib/pact_broker/verifications/latest_verification_for_consumer_version_tag.rb b/lib/pact_broker/verifications/latest_verification_for_consumer_version_tag.rb index b4398d329..de8f1a098 100644 --- a/lib/pact_broker/verifications/latest_verification_for_consumer_version_tag.rb +++ b/lib/pact_broker/verifications/latest_verification_for_consumer_version_tag.rb @@ -5,9 +5,9 @@ module Verifications class LatestVerificationForConsumerVersionTag < PactBroker::Domain::Verification set_dataset(:latest_verifications_for_consumer_version_tags) + # Don't need to load the pact_version as we do in the superclass, + # as pact_version_sha is included in the view for convenience def pact_version_sha - # Don't need to load the pact_version as we do in the superclass, - # as pact_version_sha is included in the view for convenience values[:pact_version_sha] end diff --git a/spec/lib/pact_broker/matrix/aggregated_row_spec.rb b/spec/lib/pact_broker/matrix/aggregated_row_spec.rb index 43dd2dd20..6092684a7 100644 --- a/spec/lib/pact_broker/matrix/aggregated_row_spec.rb +++ b/spec/lib/pact_broker/matrix/aggregated_row_spec.rb @@ -45,11 +45,6 @@ module Matrix end context "when there is no verification for any of the rows or any of the pacts with the same tag" do - before do - allow_any_instance_of(PactBroker::Verifications::Repository).to receive(:find_latest_verification_for).and_return(overall_latest_verification) - end - - let(:overall_latest_verification) { instance_double('PactBroker::Domain::Verification', id: 5) } let(:verification_1) { nil } let(:verification_2) { nil } let(:tag_verification_1) { nil } @@ -57,10 +52,14 @@ module Matrix context "when one of the rows is the overall latest" do let(:consumer_version_tag_name_1) { nil } + let(:overall_latest_verification) { instance_double('PactBroker::Domain::Verification', id: 1) } + before do + allow(row_1).to receive(:latest_verification_for_consumer_and_provider).and_return(overall_latest_verification) + end it "looks up the overall latest verification" do - expect_any_instance_of(PactBroker::Verifications::Repository).to receive(:find_latest_verification_for).with("Foo", "Bar") - subject + expect(row_1).to receive(:latest_verification_for_consumer_and_provider) + subject end it "returns the overall latest verification" do diff --git a/spec/lib/pact_broker/matrix/row_spec.rb b/spec/lib/pact_broker/matrix/row_spec.rb index b2df06730..8dc08f936 100644 --- a/spec/lib/pact_broker/matrix/row_spec.rb +++ b/spec/lib/pact_broker/matrix/row_spec.rb @@ -5,6 +5,27 @@ module Matrix describe Row do let(:td) { TestDataBuilder.new } + describe "latest_verification_for_consumer_and_provider" do + before do + td.create_pact_with_hierarchy("Foo", "1", "Bar") + .create_verification(provider_version: "9") + .create_consumer_version("2") + .create_consumer_version_tag("prod") + .create_pact + .create_verification(provider_version: "10") + .create_consumer("Wiffle") + .create_consumer_version("4") + .create_pact + .create_verification(provider_version: "11") + end + + subject { Row.where(consumer_name: "Foo", provider_name: "Bar").all.collect(&:latest_verification_for_consumer_and_provider) } + + it "returns the latest verification for the consumer and provider" do + expect(subject.collect(&:provider_version_number)).to eq ["10", "10"] + end + end + describe "refresh", migration: true do let(:td) { TestDataBuilder.new(auto_refresh_matrix: false) } From c94a9923b73ca4b2b2caca099ae47b90e6ba5833 Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Sat, 26 May 2018 20:47:24 +1000 Subject: [PATCH 7/7] chore: use pact_version_sha instead of pact_version_id in index item - revert to it's previous logic --- lib/pact_broker/domain/index_item.rb | 2 +- lib/pact_broker/index/service.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/pact_broker/domain/index_item.rb b/lib/pact_broker/domain/index_item.rb index 3a010fab6..f1eb5a7d1 100644 --- a/lib/pact_broker/domain/index_item.rb +++ b/lib/pact_broker/domain/index_item.rb @@ -102,7 +102,7 @@ def latest_verification_successful? end def pact_changed_since_last_verification? - latest_verification.pact_version_id != latest_pact.pact_version_id + latest_verification.pact_version_sha != latest_pact.pact_version_sha end def latest_verification_provider_version_number diff --git a/lib/pact_broker/index/service.rb b/lib/pact_broker/index/service.rb index 71eb77a23..6176ea4e0 100644 --- a/lib/pact_broker/index/service.rb +++ b/lib/pact_broker/index/service.rb @@ -32,7 +32,7 @@ def self.find_index_items options = {} rows = rows.all.group_by(&:pact_publication_id).values.collect{ | rows| Matrix::AggregatedRow.new(rows) } rows.sort.collect do | row | - # TODO simplify + # TODO simplify. Do we really need 3 layers of abstraction? PactBroker::Domain::IndexItem.create( row.consumer, row.provider,