From f9705583f2eb596b799619416e5a15352950a539 Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Thu, 29 Feb 2024 13:53:29 +1100 Subject: [PATCH] fix: improve performance of WIP pacts by using branch heads instead of calculating latest pact for branch --- .../pacts/pact_publication_dataset_module.rb | 34 +++++++++---------- ...act_publication_selector_dataset_module.rb | 2 +- .../pacts_for_verification_repository.rb | 2 +- .../pact_publication_dataset_module_spec.rb | 4 +-- 4 files changed, 21 insertions(+), 21 deletions(-) diff --git a/lib/pact_broker/pacts/pact_publication_dataset_module.rb b/lib/pact_broker/pacts/pact_publication_dataset_module.rb index 851a8637b..f8ef35ee6 100644 --- a/lib/pact_broker/pacts/pact_publication_dataset_module.rb +++ b/lib/pact_broker/pacts/pact_publication_dataset_module.rb @@ -118,6 +118,23 @@ def overall_latest_for_consumer_id_and_provider_id(consumer_id, provider_id) .limit(1) end + # Returns the pacts (if they exist) for all the branch heads. + # If the version for the branch head does not have a pact, then no pact is returned, + # (unlike latest_by_consumer_branch) + # This is much more performant than latest_by_consumer_branch and should be used + # for the 'pacts for verification' response + # @return [Dataset] + def for_all_branch_heads + base_query = self + base_query = base_query.join(:branch_heads, { Sequel[:bh][:version_id] => Sequel[:pact_publications][:consumer_version_id] }, { table_alias: :bh }) + + if no_columns_selected? + base_query = base_query.select_all_qualified.select_append(Sequel[:bh][:branch_name].as(:branch_name)) + end + + base_query.remove_overridden_revisions + end + # Return the pacts (if they exist) for the branch heads of the given branch names # This uses the new logic of finding the branch head and returning any associated pacts, # rather than the old logic of returning the pact for the latest version @@ -141,23 +158,6 @@ def for_branch_heads(branch_name) .remove_overridden_revisions_from_complete_query end - # Return the pacts (if they exist) for all the branch heads. - # @return [Sequel::Dataset] - def latest_for_all_consumer_branches - branch_head_join = { - Sequel[:pact_publications][:consumer_version_id] => Sequel[:branch_heads][:version_id], - } - - base_query = self - if no_columns_selected? - base_query = base_query.select_all_qualified.select_append(Sequel[:branch_heads][:branch_name].as(:branch_name)) - end - - base_query - .join(:branch_heads, branch_head_join) - .remove_overridden_revisions_from_complete_query - end - # The pact that belongs to the branch head. # May return nil if the branch head does not have a pact published for it. def latest_for_consumer_branch(branch_name) diff --git a/lib/pact_broker/pacts/pact_publication_selector_dataset_module.rb b/lib/pact_broker/pacts/pact_publication_selector_dataset_module.rb index 1e624215e..017821090 100644 --- a/lib/pact_broker/pacts/pact_publication_selector_dataset_module.rb +++ b/lib/pact_broker/pacts/pact_publication_selector_dataset_module.rb @@ -15,7 +15,7 @@ def for_provider_and_consumer_version_selector provider, selector # Do the "latest" logic last so that the provider/consumer criteria get included in the "latest" query before the join, rather than after query = query.latest_for_main_branches if selector.latest_for_main_branch? - query = query.latest_for_all_consumer_branches if selector.latest_for_each_branch? + query = query.for_all_branch_heads if selector.latest_for_each_branch? query = query.latest_for_consumer_branch(selector.branch) if selector.latest_for_branch? query = query.for_latest_consumer_versions_with_tag(selector.tag) if selector.latest_for_tag? query = query.overall_latest if selector.overall_latest? diff --git a/lib/pact_broker/pacts/pacts_for_verification_repository.rb b/lib/pact_broker/pacts/pacts_for_verification_repository.rb index 2c5ed84db..cb8533d9c 100644 --- a/lib/pact_broker/pacts/pacts_for_verification_repository.rb +++ b/lib/pact_broker/pacts/pacts_for_verification_repository.rb @@ -229,7 +229,7 @@ def find_wip_pact_versions_for_provider_by_provider_branch(provider_name, provid provider = pacticipant_repository.find_by_name(provider_name) wip_start_date = options.fetch(:include_wip_pacts_since) - potential_wip_by_consumer_branch = PactPublication.for_provider(provider).created_after(wip_start_date).latest_by_consumer_branch + potential_wip_by_consumer_branch = PactPublication.for_provider(provider).created_after(wip_start_date).for_all_branch_heads potential_wip_by_consumer_tag = PactPublication.for_provider(provider).created_after(wip_start_date).latest_by_consumer_tag log_debug_for_wip do diff --git a/spec/lib/pact_broker/pacts/pact_publication_dataset_module_spec.rb b/spec/lib/pact_broker/pacts/pact_publication_dataset_module_spec.rb index ac8b50883..2f496f05b 100644 --- a/spec/lib/pact_broker/pacts/pact_publication_dataset_module_spec.rb +++ b/spec/lib/pact_broker/pacts/pact_publication_dataset_module_spec.rb @@ -131,7 +131,7 @@ module Pacts end end - describe "latest_for_all_consumer_branches" do + describe "for_all_branch_heads" do before do td.create_consumer("Foo") .create_provider("Bar") @@ -150,7 +150,7 @@ module Pacts .create_pact end - subject { PactPublication.latest_for_all_consumer_branches } + subject { PactPublication.for_all_branch_heads } it "returns the pacts for all the branch heads" do all = subject.all_allowing_lazy_load.sort_by{ |pact_publication| pact_publication.consumer_version.order }