From 871209e1dab94cbdf4eb56f2b48cd276c9a8e1b1 Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Thu, 29 Feb 2024 17:24:07 +1100 Subject: [PATCH] fix: optimise WIP pacts by using branch/tag heads (#668) --- .../pacts/pact_publication_dataset_module.rb | 30 +++++++++++- .../pacts_for_verification_repository.rb | 4 +- .../pact_publication_dataset_module_spec.rb | 47 +++++++++++++++++++ 3 files changed, 78 insertions(+), 3 deletions(-) diff --git a/lib/pact_broker/pacts/pact_publication_dataset_module.rb b/lib/pact_broker/pacts/pact_publication_dataset_module.rb index f8ef35ee6..d6c8f6a5c 100644 --- a/lib/pact_broker/pacts/pact_publication_dataset_module.rb +++ b/lib/pact_broker/pacts/pact_publication_dataset_module.rb @@ -199,7 +199,10 @@ def old_latest_for_consumer_branch(branch_name) # The latest pact publication for each tag # This uses the old logic of "the latest pact for a version that has a tag" (which always returns a pact) # rather than "the pact for the latest version with a tag" - # Need to see about updating this. + # + # For 'pacts for verification' this has been replaced by for_all_tag_heads + # This should only be used for the UI + # @return [Sequel::Dataset] def latest_by_consumer_tag tags_join = { Sequel[:pact_publications][:consumer_version_id] => Sequel[:tags][:version_id], @@ -228,6 +231,7 @@ def latest_by_consumer_tag # This uses the old logic of "the latest pact for a version that has a tag" (which always returns a pact) # rather than "the pact for the latest version with a tag" # Need to see about updating this. + # @return [Sequel::Dataset] def latest_for_consumer_tag(tag_name) tags_join = { Sequel[:pact_publications][:consumer_version_id] => Sequel[:tags][:version_id], @@ -278,6 +282,30 @@ def for_latest_consumer_versions_with_tag(tag_name) .remove_overridden_revisions_from_complete_query end + # The pacts for the latest versions for each tag. + # Will not return a pact if the pact is no longer published for a particular tag + # NEW LOGIC + # @return [Sequel::Dataset] + def for_all_tag_heads + head_tags = PactBroker::Domain::Tag + .select_group(:pacticipant_id, :name) + .select_append{ max(version_order).as(:latest_version_order) } + + head_tags_join = { + Sequel[:pact_publications][:consumer_id] => Sequel[:head_tags][:pacticipant_id], + Sequel[:pact_publications][:consumer_version_order] => Sequel[:head_tags][:latest_version_order] + } + + base_query = self + if no_columns_selected? + base_query = base_query.select_all_qualified.select_append(Sequel[:head_tags][:name].as(:tag_name)) + end + + base_query + .join(head_tags, head_tags_join, { table_alias: :head_tags }) + .remove_overridden_revisions_from_complete_query + end + def in_environments currently_deployed_join = { Sequel[:pact_publications][:consumer_version_id] => Sequel[:currently_deployed_version_ids][:version_id] diff --git a/lib/pact_broker/pacts/pacts_for_verification_repository.rb b/lib/pact_broker/pacts/pacts_for_verification_repository.rb index cb8533d9c..5c226812b 100644 --- a/lib/pact_broker/pacts/pacts_for_verification_repository.rb +++ b/lib/pact_broker/pacts/pacts_for_verification_repository.rb @@ -64,7 +64,7 @@ def find_wip(provider_name, provider_version_branch, provider_tags_names, explic provider_tags_names, wip_start_date, explicitly_specified_verifiable_pacts, - :latest_by_consumer_tag + :for_all_tag_heads ) wip_by_consumer_branches = find_wip_pact_versions_for_provider_by_provider_tags( @@ -72,7 +72,7 @@ def find_wip(provider_name, provider_version_branch, provider_tags_names, explic provider_tags_names, wip_start_date, explicitly_specified_verifiable_pacts, - :latest_by_consumer_branch + :for_all_branch_heads ) deduplicate_verifiable_pacts(wip_by_consumer_tags + wip_by_consumer_branches).sort 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 2f496f05b..d9b892e59 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 @@ -244,6 +244,53 @@ module Pacts end end + describe "for_all_tag_heads" do + before do + td.create_consumer("Foo") + .create_provider("Bar") + .create_consumer_version("1", tag_names: ["main"]) + .create_pact + .create_consumer_version("2", tag_names: ["feat/x"]) + .create_pact + .create_consumer_version("3", tag_names: ["main"], comment: "latest") + .create_pact + .create_consumer_version("4", tag_names: ["feat/x"], comment: "latest") + .create_pact + .create_consumer("FooZ") + .create_consumer_version("6", tag_names: ["main"], comment: "Different consumer") + .create_pact + .create_consumer_version("7", comment: "No branch") + .create_pact + .create_consumer_version("8", tag_names: ["main"], comment: "No pact") + end + + subject { PactPublication.for_all_tag_heads.all_allowing_lazy_load } + + let(:foo) { PactBroker::Domain::Pacticipant.where(name: "Foo").single_record } + let(:bar) { PactBroker::Domain::Pacticipant.where(name: "Bar").single_record } + let(:foo_z) { PactBroker::Domain::Pacticipant.where(name: "FooZ").single_record } + + it "returns the pacts belonging to the latest tagged version for each tag" do + expect(subject.size).to eq 2 + subject.collect(&:values) + + expect(subject.find { |pp| pp.consumer_id == foo.id && pp[:tag_name] == "main" }.consumer_version.number).to eq "3" + expect(subject.find { |pp| pp.consumer_id == foo.id && pp[:tag_name] == "feat/x" }.consumer_version.number).to eq "4" + end + + it "does not return extra columns" do + expect(subject.first.values.keys.sort).to eq (PactPublication.columns + [:tag_name]).sort + end + + context "when columns are already selected" do + subject { PactPublication.select(Sequel[:pact_publications][:id]).latest_by_consumer_tag } + + it "does not override them" do + expect(subject.all.first.values.keys).to eq [:id] + end + end + end + describe "overall_latest" do before do td.create_consumer("Foo")