From 1c0530c033f20ed67ea25d77bb25b49ef505b331 Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Tue, 31 Dec 2019 15:08:18 +1100 Subject: [PATCH] refactor: dry up the pacticipant and pacticipant version collection code into QueryIds --- lib/pact_broker/matrix/query_builder.rb | 90 +++++++++---------------- lib/pact_broker/matrix/query_ids.rb | 40 +++++++++++ lib/pact_broker/matrix/quick_row.rb | 80 +++++++++++----------- 3 files changed, 113 insertions(+), 97 deletions(-) create mode 100644 lib/pact_broker/matrix/query_ids.rb diff --git a/lib/pact_broker/matrix/query_builder.rb b/lib/pact_broker/matrix/query_builder.rb index 782efa465..d7abab262 100644 --- a/lib/pact_broker/matrix/query_builder.rb +++ b/lib/pact_broker/matrix/query_builder.rb @@ -1,47 +1,35 @@ module PactBroker module Matrix class QueryBuilder - def self.provider_or_provider_version_matches(selectors, qualifier = nil) - Sequel.|(*provider_or_provider_version_criteria(selectors, qualifier)) + def self.provider_or_provider_version_matches(query_ids, qualifier = nil) + Sequel.|(*provider_or_provider_version_criteria(query_ids, qualifier)) end - def self.provider_or_provider_version_matches_or_pact_unverified(selectors, qualifier = nil) - ors = provider_or_provider_version_criteria(selectors, qualifier) - all_provider_ids = selectors.collect{ |s| s[:pacticipant_id] } + def self.provider_or_provider_version_matches_or_pact_unverified(query_ids, qualifier = nil) + ors = provider_or_provider_version_criteria(query_ids, qualifier) + ors << { - qualify(:lp, :provider_id) => all_provider_ids, + qualify(:lp, :provider_id) => query_ids.all_pacticipant_ids, qualify(qualifier, :provider_version_id) => nil } Sequel.|(*ors) end - def self.provider_or_provider_version_criteria(selectors, qualifier = nil) - most_specific_criteria = selectors.collect(&:most_specific_criterion) - # the pacticipant version ids for selectors where pacticipant version id was the most specific criterion - pacticipant_version_ids = collect_ids(most_specific_criteria, :pacticipant_version_id) - # the pacticipant ids for the selectors where the pacticipant id was most specific criterion - pacticipant_ids = collect_ids(most_specific_criteria, :pacticipant_id) - + def self.provider_or_provider_version_criteria(query_ids, qualifier = nil) ors = [] - ors << { qualify(qualifier, :provider_version_id) => pacticipant_version_ids } if pacticipant_version_ids.any? - ors << { qualify(qualifier, :provider_id) => pacticipant_ids } if pacticipant_ids.any? + ors << { qualify(qualifier, :provider_version_id) => query_ids.pacticipant_version_ids } if query_ids.pacticipant_version_ids.any? + ors << { qualify(qualifier, :provider_id) => query_ids.pacticipant_ids } if query_ids.pacticipant_ids.any? ors end - def self.consumer_in_pacticipant_ids(selectors) - { consumer_id: selectors.collect(&:pacticipant_id) } + def self.consumer_in_pacticipant_ids(query_ids) + { consumer_id: query_ids.all_pacticipant_ids } end - def self.consumer_or_consumer_version_matches(selectors, qualifier) - most_specific_criteria = selectors.collect(&:most_specific_criterion) - # the pacticipant version ids for selectors where pacticipant version id was the most specific criterion - pacticipant_version_ids = collect_ids(most_specific_criteria, :pacticipant_version_id) - # the pacticipant ids for the selectors where the pacticipant id was most specific criterion - pacticipant_ids = collect_ids(most_specific_criteria, :pacticipant_id) - + def self.consumer_or_consumer_version_matches(query_ids, qualifier) ors = [] - ors << { qualify(qualifier, :consumer_version_id) => pacticipant_version_ids } if pacticipant_version_ids.any? - ors << { qualify(qualifier, :consumer_id) => pacticipant_ids } if pacticipant_ids.any? + ors << { qualify(qualifier, :consumer_version_id) => query_ids.pacticipant_version_ids } if query_ids.pacticipant_version_ids.any? + ors << { qualify(qualifier, :consumer_id) => query_ids.pacticipant_ids } if query_ids.pacticipant_ids.any? Sequel.|(*ors) end @@ -53,45 +41,29 @@ def self.consumer_or_consumer_version_matches(selectors, qualifier) # implied selectors as well. # This extra filter makes sure that every row that is returned actually matches one of the specified # selectors. - def self.either_consumer_or_provider_was_specified_in_query(selectors, qualifier = nil) - specified_pacticipant_ids = selectors.select(&:specified?).collect(&:pacticipant_id) + def self.either_consumer_or_provider_was_specified_in_query(query_ids, qualifier = nil) Sequel.|({ - qualify(qualifier, :consumer_id) => specified_pacticipant_ids + qualify(qualifier, :consumer_id) => query_ids.specified_pacticipant_ids } , { - qualify(qualifier, :provider_id) => specified_pacticipant_ids + qualify(qualifier, :provider_id) => query_ids.specified_pacticipant_ids }) end - def self.consumer_or_consumer_version_or_provider_or_provider_or_provider_version_match(s) - consumer_or_consumer_version_match = s[:pacticipant_version_id] ? { Sequel[:lp][:consumer_version_id] => s[:pacticipant_version_id] } : { Sequel[:lp][:consumer_id] => s[:pacticipant_id] } - provider_or_provider_version_match = s[:pacticipant_version_id] ? { Sequel[:lv][:provider_version_id] => s[:pacticipant_version_id] } : { Sequel[:lp][:provider_id] => s[:pacticipant_id] } - Sequel.|(consumer_or_consumer_version_match , provider_or_provider_version_match) - end - - def self.all_pacticipant_ids selectors - selectors.collect(&:pacticipant_id) - end - - def self.collect_ids(hashes, key) - hashes.collect{ |s| s[key] }.flatten.compact - end - - def self.collect_the_ids selectors - most_specific_criteria = selectors.collect(&:most_specific_criterion) - pacticipant_version_ids = collect_ids(most_specific_criteria, :pacticipant_version_id) - pacticipant_ids = collect_ids(most_specific_criteria, :pacticipant_id) - all_pacticipant_ids = selectors.collect(&:pacticipant_id) - - specified_pacticipant_ids = selectors.select(&:specified?).collect(&:pacticipant_id) + # QueryIds is built from a single selector, so there is only one pacticipant_id or pacticipant_version_id + def self.consumer_or_consumer_version_or_provider_or_provider_or_provider_version_match(query_ids) + ors = if query_ids.pacticipant_version_id + [ + { Sequel[:lp][:consumer_version_id] => query_ids.pacticipant_version_id }, + { Sequel[:lv][:provider_version_id] => query_ids.pacticipant_version_id } + ] + else + [ + { Sequel[:lp][:consumer_id] => query_ids.pacticipant_id }, + { Sequel[:lp][:provider_id] => query_ids.pacticipant_id } + ] + end - { - consumer_version_ids: pacticipant_version_ids, - provider_version_ids: pacticipant_version_ids, - consumer_ids: pacticipant_ids, - provider_ids: pacticipant_ids, - all_pacticipant_ids: all_pacticipant_ids, - specified_pacticipant_ids: specified_pacticipant_ids - } + Sequel.|(*ors) end def self.qualify(qualifier, column) diff --git a/lib/pact_broker/matrix/query_ids.rb b/lib/pact_broker/matrix/query_ids.rb new file mode 100644 index 000000000..a4d7e3a94 --- /dev/null +++ b/lib/pact_broker/matrix/query_ids.rb @@ -0,0 +1,40 @@ +module PactBroker + module Matrix + class QueryIds + attr_reader :all_pacticipant_ids, :specified_pacticipant_ids, :pacticipant_ids, :pacticipant_version_ids + + # pacticipant_version_ids - the pacticipant version ids from the selectors where the pacticipant version id is the most specific criterion + # pacticipant_ids - the pacticipant ids from the selectors where the pacticipant id is the most specific criterion + # all_pacticipant_ids - the pacticipant ids from all the selectors, regardless of whether or not a pacticipant version has also been specified + # specified_pacticipant_ids the IDs of the pacticipants that were specified in the can-i-deploy query + def initialize(all_pacticipant_ids, specified_pacticipant_ids, pacticipant_ids, pacticipant_version_ids) + @all_pacticipant_ids = all_pacticipant_ids + @specified_pacticipant_ids = specified_pacticipant_ids + @pacticipant_ids = pacticipant_ids + @pacticipant_version_ids = pacticipant_version_ids + @all_pacticipant_ids = all_pacticipant_ids + end + + def self.from_selectors(selectors) + most_specific_criteria = selectors.collect(&:most_specific_criterion) + all_pacticipant_ids = selectors.collect(&:pacticipant_id) + specified_pacticipant_ids = selectors.select(&:specified?).collect(&:pacticipant_id) + pacticipant_version_ids = collect_ids(most_specific_criteria, :pacticipant_version_id) + pacticipant_ids = collect_ids(most_specific_criteria, :pacticipant_id) + QueryIds.new(all_pacticipant_ids, specified_pacticipant_ids, pacticipant_ids, pacticipant_version_ids) + end + + def self.collect_ids(hashes, key) + hashes.collect{ |s| s[key] }.flatten.compact + end + + def pacticipant_id + pacticipant_ids.first + end + + def pacticipant_version_id + pacticipant_version_ids.first + end + end + end +end diff --git a/lib/pact_broker/matrix/quick_row.rb b/lib/pact_broker/matrix/quick_row.rb index 8b73bcc05..83c0ff711 100644 --- a/lib/pact_broker/matrix/quick_row.rb +++ b/lib/pact_broker/matrix/quick_row.rb @@ -10,6 +10,7 @@ require 'pact_broker/domain/verification' require 'pact_broker/pacts/pact_publication' require 'pact_broker/tags/tag_with_latest_flag' +require 'pact_broker/matrix/query_ids' # The difference between `join_verifications_for` and `join_verifications` is that # the left outer join is done on a pre-filtered dataset in `join_verifications_for`, @@ -92,19 +93,45 @@ def distinct_integrations selectors def matching_selectors selectors if selectors.size == 1 - matching_one_selector(selectors.first) + matching_one_selector(selectors) else matching_multiple_selectors(selectors) end end + def order_by_names_ascending_most_recent_first + from_self. + order( + Sequel.asc(:consumer_name), + Sequel.desc(:consumer_version_order), + Sequel.asc(:provider_name), + Sequel.desc(:provider_version_order), + Sequel.desc(:verification_id)) + end + + def eager_all_the_things + eager(:consumer) + .eager(:provider) + .eager(:consumer_version) + .eager(:provider_version) + .eager(:verification) + .eager(:pact_publication) + .eager(:pact_version) + end + + def default_scope + select_all_columns.join_verifications.join_pacticipants_and_pacticipant_versions.from_self + end + + # PRIVATE METHODS + # When we have one selector, we need to join ALL the verifications to find out # what integrations exist - def matching_one_selector(selector) + def matching_one_selector(selectors) join_verifications .join_pacticipants_and_pacticipant_versions .where { - QueryBuilder.consumer_or_consumer_version_or_provider_or_provider_or_provider_version_match(selector) + QueryBuilder.consumer_or_consumer_version_or_provider_or_provider_or_provider_version_match(QueryIds.from_selectors(selectors)) } end @@ -118,24 +145,29 @@ def matching_one_selector(selector) # and THEN join them to the pacts, so that we get a row for the pact with null provider version # and verification fields. def matching_multiple_selectors(selectors) - join_verifications_for(selectors) + query_ids = QueryIds.from_selectors(selectors) + join_verifications_for(query_ids) .join_pacticipants_and_pacticipant_versions .where { Sequel.&( - QueryBuilder.consumer_or_consumer_version_matches(selectors, :lp), - QueryBuilder.provider_or_provider_version_matches_or_pact_unverified(selectors, :lv), - QueryBuilder.either_consumer_or_provider_was_specified_in_query(selectors, :lp) + QueryBuilder.consumer_or_consumer_version_matches(query_ids, :lp), + QueryBuilder.provider_or_provider_version_matches_or_pact_unverified(query_ids, :lv), + QueryBuilder.either_consumer_or_provider_was_specified_in_query(query_ids, :lp) ) } end - def verifications_for(selectors) + def join_verifications_for(query_ids) + left_outer_join(verifications_for(query_ids), LP_LV_JOIN, { table_alias: :lv } ) + end + + def verifications_for(query_ids) db[LV] .select(:verification_id, :provider_version_id, :pact_version_id, :provider_id) .where { Sequel.&( - QueryBuilder.consumer_in_pacticipant_ids(selectors), - QueryBuilder.provider_or_provider_version_matches(selectors) + QueryBuilder.consumer_in_pacticipant_ids(query_ids), + QueryBuilder.provider_or_provider_version_matches(query_ids) ) } end @@ -163,37 +195,9 @@ def join_provider_versions left_outer_join(:versions, PROVIDER_VERSION_JOIN, { table_alias: :pv } ) end - def join_verifications_for(selectors) - left_outer_join(verifications_for(selectors), LP_LV_JOIN, { table_alias: :lv } ) - end - def join_verifications left_outer_join(LV, LP_LV_JOIN, { table_alias: :lv } ) end - - def order_by_names_ascending_most_recent_first - from_self. - order( - Sequel.asc(:consumer_name), - Sequel.desc(:consumer_version_order), - Sequel.asc(:provider_name), - Sequel.desc(:provider_version_order), - Sequel.desc(:verification_id)) - end - - def eager_all_the_things - eager(:consumer) - .eager(:provider) - .eager(:consumer_version) - .eager(:provider_version) - .eager(:verification) - .eager(:pact_publication) - .eager(:pact_version) - end - - def default_scope - select_all_columns.join_verifications.join_pacticipants_and_pacticipant_versions.from_self - end end # end dataset_module def success