From 679eec1927d2d6250cd012e3fa42b54d8d868cff Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Mon, 5 Feb 2018 08:40:17 +1100 Subject: [PATCH] fix(matrix): ensure matrix for latest consumer version/provider version shows correct results when a pact is published, published again for the same version with different content, then published again for the same version with the original content. --- ...0180204_fix_latest_matrix_for_cv_and_pv.rb | 59 +++++++++++++++++++ lib/pact_broker/index/service.rb | 3 - lib/pact_broker/matrix/repository.rb | 7 ++- lib/pact_broker/matrix/row.rb | 3 + lib/pact_broker/pacts/pact_version.rb | 3 +- .../lib/pact_broker/matrix/repository_spec.rb | 41 +++++++++++++ spec/support/test_data_builder.rb | 4 +- 7 files changed, 113 insertions(+), 7 deletions(-) create mode 100644 db/migrations/20180204_fix_latest_matrix_for_cv_and_pv.rb diff --git a/db/migrations/20180204_fix_latest_matrix_for_cv_and_pv.rb b/db/migrations/20180204_fix_latest_matrix_for_cv_and_pv.rb new file mode 100644 index 000000000..013b4eefe --- /dev/null +++ b/db/migrations/20180204_fix_latest_matrix_for_cv_and_pv.rb @@ -0,0 +1,59 @@ +Sequel.migration do + up do + # Removes 'overwritten' pacts and verifications from the matrix + # (ie. only show latest pact revision for each consumer version and + # latest verification for each provider version) + # Must include lines where verification_id is null so that we don't + # lose the unverified pacts. + # In this view there will be one row for each consumer version/provider version + # This view used to be (stupidly) called latest_matrix + create_or_replace_view(:latest_matrix_for_consumer_version_and_provider_version, + " + SELECT matrix.* FROM matrix + inner join latest_pact_publication_revision_numbers lr + on matrix.consumer_id = lr.consumer_id + and matrix.provider_id = lr.provider_id + and matrix.consumer_version_order = lr.consumer_version_order + and matrix.pact_revision_number = lr.latest_revision_number + INNER JOIN latest_verification_id_for_consumer_version_and_provider_version AS lv + ON ((matrix.consumer_version_id = lv.consumer_version_id) + AND (matrix.provider_version_id = lv.provider_version_id) + AND ((matrix.verification_id = lv.latest_verification_id))) + + UNION + + select matrix.* from matrix + inner join latest_pact_publication_revision_numbers lr + on matrix.consumer_id = lr.consumer_id + and matrix.provider_id = lr.provider_id + and matrix.consumer_version_order = lr.consumer_version_order + and matrix.pact_revision_number = lr.latest_revision_number + where verification_id is null + " + ) + from(:materialized_head_matrix).delete + from(:materialized_head_matrix).insert(from(:head_matrix).select_all) + end + + down do + # revert to dodgey definition + create_or_replace_view(:latest_matrix_for_consumer_version_and_provider_version, + "SELECT matrix.* FROM matrix + INNER JOIN latest_verification_id_for_consumer_version_and_provider_version AS lv + ON ((matrix.consumer_version_id = lv.consumer_version_id) + AND (matrix.provider_version_id = lv.provider_version_id) + AND ((matrix.verification_id = lv.latest_verification_id))) + + UNION + + select matrix.* from matrix + inner join latest_pact_publication_revision_numbers lr + on matrix.consumer_id = lr.consumer_id + and matrix.provider_id = lr.provider_id + and matrix.consumer_version_order = lr.consumer_version_order + and matrix.pact_revision_number = lr.latest_revision_number + where verification_id is null + " + ) + end +end \ No newline at end of file diff --git a/lib/pact_broker/index/service.rb b/lib/pact_broker/index/service.rb index 184b4286c..2e71da2e1 100644 --- a/lib/pact_broker/index/service.rb +++ b/lib/pact_broker/index/service.rb @@ -14,9 +14,6 @@ class Service extend PactBroker::Services extend PactBroker::Logging - # Used when using table_print to output query results - TP_COLS = [:consumer_name, :consumer_version_number, :consumer_version_id, :provider_name, :provider_id, :provider_version_number] - def self.find_index_items options = {} rows = [] overall_latest_publication_ids = nil diff --git a/lib/pact_broker/matrix/repository.rb b/lib/pact_broker/matrix/repository.rb index 46d948a2e..d2bd98879 100644 --- a/lib/pact_broker/matrix/repository.rb +++ b/lib/pact_broker/matrix/repository.rb @@ -20,16 +20,17 @@ class Repository GROUP_BY_PACT = [:consumer_name, :provider_name] def refresh params + PactBroker::Matrix::Row.refresh(params) PactBroker::Matrix::HeadRow.refresh(params) end # Return the latest matrix row (pact/verification) for each consumer_version_number/provider_version_number def find selectors, options = {} - # The group with the nil provider_version_numbers will be the results of the left outer join - # that don't have verifications, so we need to include them all. lines = query_matrix(resolve_selectors(selectors, options), options) lines = apply_latestby(options, selectors, lines) + # This needs to be done after the latestby, so can't be done in the db unless + # the latestby logic is moved to the db if options.key?(:success) lines = lines.select{ |l| options[:success].include?(l.success) } end @@ -45,6 +46,8 @@ def apply_latestby options, selectors, lines when 'cp' then GROUP_BY_PACT end + # The group with the nil provider_version_numbers will be the results of the left outer join + # that don't have verifications, so we need to include them all. lines.group_by{|line| group_by_columns.collect{|key| line.send(key) }} .values .collect{ | lines | lines.first.provider_version_number.nil? ? lines : lines.first } diff --git a/lib/pact_broker/matrix/row.rb b/lib/pact_broker/matrix/row.rb index 144330c45..13a0a3dc2 100644 --- a/lib/pact_broker/matrix/row.rb +++ b/lib/pact_broker/matrix/row.rb @@ -6,6 +6,9 @@ module PactBroker module Matrix class Row < Sequel::Model(:materialized_matrix) + # Used when using table_print to output query results + TP_COLS = [:consumer_version_number, :consumer_version_id, :pact_revision_number, :provider_id, :provider_version_number, :verification_number] + associate(:one_to_many, :latest_triggered_webhooks, :class => "PactBroker::Webhooks::LatestTriggeredWebhook", primary_key: :pact_publication_id, key: :pact_publication_id) 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) diff --git a/lib/pact_broker/pacts/pact_version.rb b/lib/pact_broker/pacts/pact_version.rb index 6b1fb4043..30f22a1d7 100644 --- a/lib/pact_broker/pacts/pact_version.rb +++ b/lib/pact_broker/pacts/pact_version.rb @@ -22,7 +22,8 @@ def latest_consumer_version end def latest_pact_publication - PactBroker::Pacts::LatestPactPublicationsByConsumerVersion.where(pact_version_id: id).order(:consumer_version_order).last + # This verification might be for an overwritten revision, so must use AllPactPublications + PactBroker::Pacts::AllPactPublications.where(pact_version_id: id).order(:consumer_version_order).last end def consumer_versions diff --git a/spec/lib/pact_broker/matrix/repository_spec.rb b/spec/lib/pact_broker/matrix/repository_spec.rb index c2be8fa7f..25302af90 100644 --- a/spec/lib/pact_broker/matrix/repository_spec.rb +++ b/spec/lib/pact_broker/matrix/repository_spec.rb @@ -248,6 +248,47 @@ def shorten_rows rows end end + describe "find" do + describe "when a pact for a particular consumer version is published, then re-published with different content, then published again with the original content" do + before do + first_pact = td.create_pact_with_hierarchy("billy", "1", "bobby").and_return(:pact) + td.create_verification(provider_version: "1") + .revise_pact + .revise_pact(first_pact.json_content) + end + + let(:selectors) { build_selectors('billy' => nil, 'bobby' => nil) } + + subject { Repository.new.find(selectors, options) } + + context "when latestby: cvpv" do + let(:options) { { latestby: 'cvpv' } } + + it "only includes the row for the latest revision" do + expect(subject.size).to eq 1 + expect(subject).to contain_hash(pact_revision_number: 3) + end + end + + context "when latestby: cvp" do + let(:options) { { latestby: 'cvp' } } + + it "only includes the row for the latest revision" do + expect(subject.size).to eq 1 + expect(subject).to contain_hash(pact_revision_number: 3) + end + end + + context "when latestby: nil" do + let(:options) { {} } + + it "includes all the rows" do + expect(subject.size).to eq 3 + end + end + end + end + describe "find" do let(:options) { {} } diff --git a/spec/support/test_data_builder.rb b/spec/support/test_data_builder.rb index 64c34213b..b6731c939 100644 --- a/spec/support/test_data_builder.rb +++ b/spec/support/test_data_builder.rb @@ -206,14 +206,16 @@ def create_pact params = {} self end - def create_same_pact params = {} + def republish_same_pact params = {} last_pact_version = PactBroker::Pacts::PactVersion.order(:id).last create_pact json_content: last_pact_version.content + self end def revise_pact json_content = nil json_content = json_content ? json_content : {random: rand}.to_json @pact = PactBroker::Pacts::Repository.new.update(@pact.id, json_content: json_content) + refresh_matrix self end