From 924aaae93358ef78cc3f585565f606968f78bb0c Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Fri, 2 Feb 2018 09:35:19 +1100 Subject: [PATCH] feat(dashboard api): fix query for displaying dashboard with tags --- db/migrations/20180201_create_head_matrix.rb | 33 +++++ ...0180202_create_materialized_head_matrix.rb | 36 +++++ lib/pact_broker/api/resources/dashboard.rb | 2 +- lib/pact_broker/domain/index_item.rb | 2 +- lib/pact_broker/index/service.rb | 63 ++++----- lib/pact_broker/matrix/head_row.rb | 10 ++ lib/pact_broker/matrix/repository.rb | 4 +- lib/pact_broker/matrix/row.rb | 14 ++ spec/lib/pact_broker/index/service_spec.rb | 77 +++++++++- .../20180201_create_head_matrix_spec.rb | 132 ++++++++++++++++++ spec/support/test_data_builder.rb | 3 + 11 files changed, 333 insertions(+), 43 deletions(-) create mode 100644 db/migrations/20180201_create_head_matrix.rb create mode 100644 db/migrations/20180202_create_materialized_head_matrix.rb create mode 100644 lib/pact_broker/matrix/head_row.rb create mode 100644 spec/migrations/20180201_create_head_matrix_spec.rb diff --git a/db/migrations/20180201_create_head_matrix.rb b/db/migrations/20180201_create_head_matrix.rb new file mode 100644 index 000000000..2914e7060 --- /dev/null +++ b/db/migrations/20180201_create_head_matrix.rb @@ -0,0 +1,33 @@ +Sequel.migration do + up do + # a row for each of the latest pact publications, + # and a row for each of the latest tagged pact publications + create_view(:head_matrix, + "SELECT matrix.*, hpp.tag_name as consumer_tag_name + FROM latest_matrix_for_consumer_version_and_provider_version matrix + INNER JOIN head_pact_publications hpp + ON matrix.consumer_id = hpp.consumer_id + AND matrix.provider_id = hpp.provider_id + AND matrix.consumer_version_order = hpp.consumer_version_order + INNER JOIN latest_verification_id_for_consumer_version_and_provider AS lv + ON ((matrix.consumer_version_id = lv.consumer_version_id) + AND (matrix.provider_id = lv.provider_id) + AND ((matrix.verification_id = lv.latest_verification_id))) + + UNION + + SELECT matrix.*, hpp.tag_name as consumer_tag_name + FROM latest_matrix_for_consumer_version_and_provider_version matrix + INNER JOIN head_pact_publications hpp + ON matrix.consumer_id = hpp.consumer_id + AND matrix.provider_id = hpp.provider_id + AND matrix.consumer_version_order = hpp.consumer_version_order + where verification_id is null + " + ) + end + + down do + drop_view(:head_matrix) + end +end diff --git a/db/migrations/20180202_create_materialized_head_matrix.rb b/db/migrations/20180202_create_materialized_head_matrix.rb new file mode 100644 index 000000000..b52b05129 --- /dev/null +++ b/db/migrations/20180202_create_materialized_head_matrix.rb @@ -0,0 +1,36 @@ +Sequel.migration do + up do + create_table(:materialized_head_matrix, charset: 'utf8') do + Integer :consumer_id, null: false + String :consumer_name, null: false + Integer :consumer_version_id, null: false + String :consumer_version_number, null: false + Integer :consumer_version_order, null: false + Integer :pact_publication_id, null: false + Integer :pact_version_id, null: false + String :pact_version_sha, null: false + Integer :pact_revision_number, null: false + DateTime :pact_created_at, null: false + Integer :provider_id, null: false + String :provider_name, null: false + Integer :provider_version_id + String :provider_version_number + Integer :provider_version_order + Integer :verification_id + Boolean :success + Integer :verification_number + DateTime :verification_executed_at + String :verification_build_url + String :consumer_tag_name + index [:consumer_id], name: 'ndx_mhm_consumer_id' + index [:provider_id], name: 'ndx_mhm_provider_id' + index [:consumer_version_order], name: 'ndx_mhm_cv_ord' + end + + from(:materialized_head_matrix).insert(from(:head_matrix).select_all) + end + + down do + drop_table(:materialized_head_matrix) + end +end diff --git a/lib/pact_broker/api/resources/dashboard.rb b/lib/pact_broker/api/resources/dashboard.rb index 216e1c613..3150ccb6c 100644 --- a/lib/pact_broker/api/resources/dashboard.rb +++ b/lib/pact_broker/api/resources/dashboard.rb @@ -30,7 +30,7 @@ def to_text private def index_items - index_service.find_index_items + index_service.find_index_items(tags: true) end end end diff --git a/lib/pact_broker/domain/index_item.rb b/lib/pact_broker/domain/index_item.rb index 078a38e8a..f1eb5a7d1 100644 --- a/lib/pact_broker/domain/index_item.rb +++ b/lib/pact_broker/domain/index_item.rb @@ -128,7 +128,7 @@ def <=> other end def to_s - "Pact between #{consumer_name} and #{provider_name}" + "Pact between #{consumer_name} #{consumer_version_number} and #{provider_name} #{provider_version_number}" end def to_a diff --git a/lib/pact_broker/index/service.rb b/lib/pact_broker/index/service.rb index 888efc486..184b4286c 100644 --- a/lib/pact_broker/index/service.rb +++ b/lib/pact_broker/index/service.rb @@ -3,6 +3,7 @@ require 'pact_broker/domain/index_item' require 'pact_broker/matrix/latest_row' require 'pact_broker/matrix/actual_latest_row' +require 'pact_broker/matrix/head_row' module PactBroker @@ -13,42 +14,41 @@ 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 - if !options[:tags] - rows = PactBroker::Matrix::ActualLatestRow + 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) - .all + + if !options[:tags] + rows = rows.where(consumer_tag_name: nil).all + overall_latest_publication_ids = rows.collect(&:pact_publication_id) end if options[:tags] - tagged_rows = PactBroker::Matrix::Row - .select_all_qualified - .select_append(Sequel[:head_pact_publications][:tag_name]) - .join(:head_pact_publications, {consumer_id: :consumer_id, provider_id: :provider_id, consumer_version_order: :consumer_version_order}) - .eager(:latest_triggered_webhooks) - .eager(:webhooks) - .order(:consumer_name, :provider_name) - .eager(:consumer_version_tags) - .eager(:provider_version_tags) - - if options[:tags].is_a?(Array) - tagged_rows = tagged_rows.where(Sequel[:head_pact_publications][:tag_name] => options[:tags]).or(Sequel[:head_pact_publications][:tag_name] => nil) - end - - tagged_rows = tagged_rows.all - .group_by(&:pact_publication_id) - .values - .collect{|group| [group.last, group.collect{|r| r[:tag_name]}.compact] } - .collect{ |(row, tag_names)| row.consumer_head_tag_names = tag_names; row } - - rows = tagged_rows + if options[:tags].is_a?(Array) + rows = rows.where(consumer_tag_name: options[:tags]).or(consumer_tag_name: nil) + end + + rows = rows.all + overall_latest_publication_ids = rows.select{|r| !r[:consumer_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_tag_name]}.compact] } + .collect{ |(row, tag_names)| row.consumer_head_tag_names = tag_names; row } end index_items = [] @@ -56,20 +56,19 @@ def self.find_index_items options = {} tag_names = [] if options[:tags] tag_names = row.consumer_version_tags.collect(&:name) - if options[:tags].is_a?(Array) - tag_names = tag_names & options[:tags] - end end - previous_index_item_for_same_consumer_and_provider = index_items.last && index_items.last.consumer_name == row.consumer_name && index_items.last.provider_name == row.provider_name - index_items << PactBroker::Domain::IndexItem.create(row.consumer, row.provider, + + index_items << PactBroker::Domain::IndexItem.create( + row.consumer, + row.provider, row.pact, - !previous_index_item_for_same_consumer_and_provider, + overall_latest_publication_ids.include?(row.pact_publication_id), row.latest_verification, row.webhooks, row.latest_triggered_webhooks, - tag_names, + row.consumer_head_tag_names, row.provider_version_tags.select(&:latest?) - ) + ) end index_items diff --git a/lib/pact_broker/matrix/head_row.rb b/lib/pact_broker/matrix/head_row.rb new file mode 100644 index 000000000..0558001b4 --- /dev/null +++ b/lib/pact_broker/matrix/head_row.rb @@ -0,0 +1,10 @@ +require 'pact_broker/matrix/row' + +module PactBroker + module Matrix + # A row for each of the overall latest pacts, and a row for each of the latest tagged pacts + class HeadRow < Row + set_dataset(:materialized_head_matrix) + end + end +end diff --git a/lib/pact_broker/matrix/repository.rb b/lib/pact_broker/matrix/repository.rb index a2637b9d0..46d948a2e 100644 --- a/lib/pact_broker/matrix/repository.rb +++ b/lib/pact_broker/matrix/repository.rb @@ -1,6 +1,7 @@ require 'pact_broker/repositories/helpers' require 'pact_broker/matrix/row' require 'pact_broker/matrix/latest_row' +require 'pact_broker/matrix/head_row' require 'pact_broker/error' module PactBroker @@ -19,8 +20,7 @@ class Repository GROUP_BY_PACT = [:consumer_name, :provider_name] def refresh params - PactBroker::Matrix::Row.refresh(params) - PactBroker::Matrix::ActualLatestRow.refresh(params) + PactBroker::Matrix::HeadRow.refresh(params) end # Return the latest matrix row (pact/verification) for each consumer_version_number/provider_version_number diff --git a/lib/pact_broker/matrix/row.rb b/lib/pact_broker/matrix/row.rb index 51de12204..144330c45 100644 --- a/lib/pact_broker/matrix/row.rb +++ b/lib/pact_broker/matrix/row.rb @@ -192,6 +192,20 @@ def compare_number_desc number1, number2 -1 end end + + # For some reason, with MySQL, the success column value + # comes back as an integer rather than a boolean + # for the latest_matrix view (but not the matrix view!) + # Maybe something to do with the union? + # Haven't investigated as this is an easy enough fix. + def success + value = super + value.nil? ? nil : value == true || value == 1 + end + + def values + super.merge(success: success) + end end end end diff --git a/spec/lib/pact_broker/index/service_spec.rb b/spec/lib/pact_broker/index/service_spec.rb index 52655e1f7..8a05027ad 100644 --- a/spec/lib/pact_broker/index/service_spec.rb +++ b/spec/lib/pact_broker/index/service_spec.rb @@ -113,16 +113,79 @@ module Index end context "when there are multiple verifications for the latest consumer version" do - before do - td.create_pact_with_hierarchy("Foo", "1", "Bar") - .create_verification(provider_version: "1.0.0") - .create_verification(provider_version: "2.0.0", number: 2) + + context "with no tags" do + before do + td.create_pact_with_hierarchy("Foo", "1", "Bar") + .create_verification(provider_version: "1.0.0") + .create_verification(provider_version: "2.0.0", number: 2) + end + + let(:options) { {} } + + it "only returns the row for the latest provider version" do + expect(rows.count).to eq 1 + end end - let(:options) { {} } + context "with tags=true" do + before do + td.create_pact_with_hierarchy("Foo", "1", "Bar") + .create_consumer_version("2") + .create_consumer_version_tag("prod") + .create_consumer_version_tag("master") + .create_pact + .revise_pact + .create_verification(provider_version: "1.0.0") + .create_verification(provider_version: "2.0.0", number: 2) + end - it "only returns the row for the latest provider version" do - expect(rows.count).to eq 1 + let(:options) { {tags: true} } + + it "only returns the row for the latest provider version" do + expect(rows.size).to eq 1 + expect(rows.first.tag_names.sort).to eq ["master","prod"] + expect(rows.first.provider_version_number).to eq "2.0.0" + end + end + + context "with tags=true" do + before do + td.create_pact_with_hierarchy("Foo", "1.0.0", "Bar") + .create_verification(provider_version: "4.5.6") + .create_consumer_version("2.0.0") + .create_consumer_version_tag("dev") + .create_pact + .revise_pact + .create_consumer_version("2.1.0") + .create_consumer_version_tag("prod") + .create_pact + .revise_pact + .create_verification(provider_version: "4.5.6", number: 1) + .create_verification(provider_version: "4.5.7", number: 2) + .create_verification(provider_version: "4.5.8", number: 3) + .create_verification(provider_version: "4.5.9", number: 4) + .create_provider("Wiffle") + .create_pact + end + + let(:options) { {tags: true} } + + it "returns a row for each of the head pacts" do + expect(rows.size).to eq 3 + + expect(rows[0].latest?).to be true + expect(rows[0].provider_name).to eq "Bar" + expect(rows[0].tag_names).to eq ["prod"] + expect(rows[0].provider_version_number).to eq "4.5.9" + + expect(rows[2].latest?).to be false + expect(rows[2].provider_name).to eq "Bar" + expect(rows[2].tag_names).to eq ["dev"] + + expect(rows[1].latest?).to be true + expect(rows[1].provider_name).to eq "Wiffle" + end end end end diff --git a/spec/migrations/20180201_create_head_matrix_spec.rb b/spec/migrations/20180201_create_head_matrix_spec.rb new file mode 100644 index 000000000..54f39cae1 --- /dev/null +++ b/spec/migrations/20180201_create_head_matrix_spec.rb @@ -0,0 +1,132 @@ +describe "latest tagged matrix", migration: true do + let(:td) { TestDataBuilder.new } + + before do + PactBroker::Database.migrate + end + + subject { database[:head_matrix].all } + + context "with a single consumer version tag on the latest version" do + before do + td.create_pact_with_hierarchy("Foo", "1", "Bar") + .create_consumer_version_tag("prod") + .create_consumer_version("2") + .create_consumer_version_tag("prod") + .create_pact + .revise_pact + end + + context "without a verification" do + it "returns a line with no tag and a line with a tag" do + expect(subject).to contain_hash( + consumer_tag_name: nil, + consumer_version_number: "2" + ) + expect(subject).to contain_hash( + consumer_tag_name: "prod", + consumer_version_number: "2" + ) + end + end + + context "with a verification" do + before do + td.create_verification(provider_version: "3") + end + it "returns the verification details" do + expect(subject).to contain_hash( + consumer_tag_name: nil, + consumer_version_number: "2", + provider_version_number: "3" + ) + expect(subject).to contain_hash( + consumer_tag_name: "prod", + consumer_version_number: "2", + provider_version_number: "3" + ) + end + end + end + context "with two tags on the latest version" do + before do + td.create_pact_with_hierarchy("Foo", "1", "Bar") + .create_consumer_version_tag("prod") + .create_consumer_version_tag("master") + .create_consumer_version("2") + .create_consumer_version_tag("prod") + .create_consumer_version_tag("master") + .create_pact + .revise_pact + end + + it "returns a line with no tag, and a line for each tag" do + expect(subject).to contain_hash( + consumer_tag_name: nil, + consumer_version_number: "2" + ) + expect(subject).to contain_hash( + consumer_tag_name: "prod", + consumer_version_number: "2" + ) + + expect(subject).to contain_hash( + consumer_tag_name: "master", + consumer_version_number: "2" + ) + end + + context "with a verification" do + before do + td.create_verification(provider_version: "3") + end + it "returns the verification details" do + expect(subject).to contain_hash( + consumer_tag_name: nil, + consumer_version_number: "2", + provider_version_number: "3" + ) + expect(subject).to contain_hash( + consumer_tag_name: "prod", + consumer_version_number: "2", + provider_version_number: "3" + ) + + expect(subject).to contain_hash( + consumer_tag_name: "master", + consumer_version_number: "2", + provider_version_number: "3" + ) + end + end + end + context "with different versions for overall latest, and two other tags" do + before do + td.create_pact_with_hierarchy("Foo", "1", "Bar") + .create_consumer_version_tag("prod") + .create_consumer_version_tag("master") + .create_consumer_version("2") + .create_consumer_version_tag("master") + .create_pact + .revise_pact + .create_consumer_version("3") + .create_pact + end + + it "returns a line for each" do + expect(subject).to contain_hash( + consumer_tag_name: nil, + consumer_version_number: "3" + ) + expect(subject).to contain_hash( + consumer_tag_name: "prod", + consumer_version_number: "1" + ) + + expect(subject).to contain_hash( + consumer_tag_name: "master", + consumer_version_number: "2" + ) + end + end +end diff --git a/spec/support/test_data_builder.rb b/spec/support/test_data_builder.rb index adee888ed..64c34213b 100644 --- a/spec/support/test_data_builder.rb +++ b/spec/support/test_data_builder.rb @@ -22,6 +22,7 @@ require 'pact_broker/tags/repository' require 'pact_broker/webhooks/repository' require 'pact_broker/certificates/certificate' +require 'pact_broker/matrix/row' require 'ostruct' class TestDataBuilder @@ -49,6 +50,8 @@ def refresh_matrix params[:consumer_name] = consumer.name if consumer params[:provider_name] = provider.name if provider matrix_service.refresh(params) + # Row is not used in production code, but the tests depend on it + PactBroker::Matrix::Row.refresh(params) end end