From 812902380229cf5df1ebcf33ff0bf952892586ea Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Sat, 17 Jul 2021 12:16:51 +1000 Subject: [PATCH] feat: add detailed debug logging for WIP pact calculations --- lib/pact_broker/pacts/pact_publication.rb | 4 +- .../pacts/pact_publication_dataset_module.rb | 6 +- .../pacts_for_verification_repository.rb | 55 +++++++++++-------- lib/pact_broker/pacts/repository.rb | 4 +- lib/pact_broker/pacts/service.rb | 20 ++----- ..._pact_versions_for_provider_branch_spec.rb | 2 +- ...ind_wip_pact_versions_for_provider_spec.rb | 2 +- 7 files changed, 46 insertions(+), 47 deletions(-) diff --git a/lib/pact_broker/pacts/pact_publication.rb b/lib/pact_broker/pacts/pact_publication.rb index a2debcb17..86ab8e007 100644 --- a/lib/pact_broker/pacts/pact_publication.rb +++ b/lib/pact_broker/pacts/pact_publication.rb @@ -47,8 +47,8 @@ class PactPublication < Sequel::Model(:pact_publications) include PactPublicationWipDatasetModule end - def self.subtract(a, b) - b_ids = b.collect(&:id) + def self.subtract(a, *b) + b_ids = b.flat_map{ |pact_publications| pact_publications.collect(&:id) } a.reject{ |pact_publication| b_ids.include?(pact_publication.id) } end diff --git a/lib/pact_broker/pacts/pact_publication_dataset_module.rb b/lib/pact_broker/pacts/pact_publication_dataset_module.rb index 608039e21..4c0d373c8 100644 --- a/lib/pact_broker/pacts/pact_publication_dataset_module.rb +++ b/lib/pact_broker/pacts/pact_publication_dataset_module.rb @@ -186,9 +186,9 @@ def remove_overridden_revisions(pact_publications_alias = :pact_publications) end def remove_overridden_revisions_from_complete_query - from_self(alias: :p) - .select(Sequel[:p].*) - .remove_overridden_revisions(:p) + from_self(alias: :pact_publications) + .select(Sequel[:pact_publications].*) + .remove_overridden_revisions(:pact_publications) end def for_pact_version_id(pact_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 5ca15f441..7464d325e 100644 --- a/lib/pact_broker/pacts/pacts_for_verification_repository.rb +++ b/lib/pact_broker/pacts/pacts_for_verification_repository.rb @@ -35,7 +35,7 @@ def find(provider_name, consumer_version_selectors) # provider tags they are pending for. # Don't include pact publications that were created before the provider tag was first used # (that is, before the provider's git branch was created). - def find_wip provider_name, provider_version_branch, provider_tags_names = [], options = {} + def find_wip provider_name, provider_version_branch, provider_tags_names, specified_pact_version_shas, options = {} # TODO not sure about this if provider_tags_names.empty? && provider_version_branch == nil log_debug_for_wip do @@ -45,7 +45,7 @@ def find_wip provider_name, provider_version_branch, provider_tags_names = [], o end if provider_version_branch - return find_wip_pact_versions_for_provider_by_provider_branch(provider_name, provider_version_branch, options) + return find_wip_pact_versions_for_provider_by_provider_branch(provider_name, provider_version_branch, specified_pact_version_shas, options) end provider = pacticipant_repository.find_by_name(provider_name) @@ -57,14 +57,18 @@ def find_wip provider_name, provider_version_branch, provider_tags_names = [], o provider_tags_names, provider_tags, wip_start_date, - :latest_by_consumer_tag) + specified_pact_version_shas, + :latest_by_consumer_tag + ) wip_by_consumer_branches = find_wip_pact_versions_for_provider_by_provider_tags( provider, provider_tags_names, provider_tags, wip_start_date, - :latest_by_consumer_branch) + specified_pact_version_shas, + :latest_by_consumer_branch + ) deduplicate_verifiable_pacts(wip_by_consumer_tags + wip_by_consumer_branches).sort end @@ -199,21 +203,19 @@ def provider_tag_objects_for(provider, provider_tags_names) end # TODO ? find the WIP pacts by consumer branch - def find_wip_pact_versions_for_provider_by_provider_tags(provider, provider_tags_names, _provider_tags, wip_start_date, pact_publication_scope) + def find_wip_pact_versions_for_provider_by_provider_tags(provider, provider_tags_names, _provider_tags, wip_start_date, specified_pact_version_shas, pact_publication_scope) potential_wip_pacts_by_consumer_tag_query = PactPublication.for_provider(provider).created_after(wip_start_date).send(pact_publication_scope) log_debug_for_wip do log_pact_publications("Potential WIP pacts for provider tag(s) #{provider_tags_names.join(", ")} created after #{wip_start_date} by #{pact_publication_scope}", potential_wip_pacts_by_consumer_tag_query) end - potential_wip_pacts_by_consumer_tag = potential_wip_pacts_by_consumer_tag_query.all - tag_to_pact_publications = provider_tags_names.each_with_object({}) do | provider_tag_name, tag_to_pact_publication | tag_to_pact_publication[provider_tag_name] = remove_non_wip_for_tag( - potential_wip_pacts_by_consumer_tag, potential_wip_pacts_by_consumer_tag_query, provider, - provider_tag_name + provider_tag_name, + specified_pact_version_shas ) end @@ -233,7 +235,7 @@ def create_selectors_for_wip_pact(pact_publication) end end - def find_wip_pact_versions_for_provider_by_provider_branch(provider_name, provider_version_branch, options) + def find_wip_pact_versions_for_provider_by_provider_branch(provider_name, provider_version_branch, specified_pact_version_shas, options) provider = pacticipant_repository.find_by_name(provider_name) wip_start_date = options.fetch(:include_wip_pacts_since) @@ -248,13 +250,15 @@ def find_wip_pact_versions_for_provider_by_provider_branch(provider_name, provid wip_pact_publications_by_branch = remove_non_wip_for_branch( potential_wip_by_consumer_branch, provider, - provider_version_branch + provider_version_branch, + specified_pact_version_shas ) wip_pact_publications_by_tag = remove_non_wip_for_branch( potential_wip_by_consumer_tag, provider, - provider_version_branch + provider_version_branch, + specified_pact_version_shas ) verifiable_pacts = (wip_pact_publications_by_branch + wip_pact_publications_by_tag).collect do | pact_publication | @@ -292,30 +296,32 @@ def find_all_pact_versions_for_provider_with_consumer_version_tags provider_name end - def remove_non_wip_for_branch(pact_publications, provider, provider_version_branch) - verified_by_this_branch = pact_publications.successfully_verified_by_provider_branch_when_not_wip(provider.id, provider_version_branch) - verified_by_other_branch = pact_publications.successfully_verified_by_provider_another_branch_before_this_branch_first_created(provider.id, provider_version_branch) + def remove_non_wip_for_branch(pact_publications_query, provider, provider_version_branch, specified_pact_version_shas) + specified_explicitly = pact_publications_query.for_pact_version_sha(specified_pact_version_shas) + verified_by_this_branch = pact_publications_query.successfully_verified_by_provider_branch_when_not_wip(provider.id, provider_version_branch) + verified_by_other_branch = pact_publications_query.successfully_verified_by_provider_another_branch_before_this_branch_first_created(provider.id, provider_version_branch) log_debug_for_wip do + log_pact_publications("Ignoring pacts explicitly specified in the selectors", specified_explicitly) log_pact_publications("Ignoring pacts successfully verified by this provider branch when not WIP", verified_by_this_branch) log_pact_publications("Ignoring pacts successfully verified by another provider branch when not WIP", verified_by_other_branch) end - remaining_pact_publications = PactPublication.subtract(pact_publications.all, verified_by_this_branch.all) - PactPublication.subtract(remaining_pact_publications, verified_by_other_branch.all) + PactPublication.subtract(pact_publications_query.all, specified_explicitly.all, verified_by_this_branch.all, verified_by_other_branch.all) end - def remove_non_wip_for_tag(pact_publications, query, provider, tag) - verified_by_this_tag = query.successfully_verified_by_provider_tag_when_not_wip(provider.id, tag) - verified_by_another_tag = query.successfully_verified_by_provider_another_tag_before_this_tag_first_created(provider.id, tag) + def remove_non_wip_for_tag(pact_publications_query, provider, tag, specified_pact_version_shas) + specified_explicitly = pact_publications_query.for_pact_version_sha(specified_pact_version_shas) + verified_by_this_tag = pact_publications_query.successfully_verified_by_provider_tag_when_not_wip(provider.id, tag) + verified_by_another_tag = pact_publications_query.successfully_verified_by_provider_another_tag_before_this_tag_first_created(provider.id, tag) log_debug_for_wip do + log_pact_publications("Ignoring pacts explicitly specified in the selectors", specified_explicitly) log_pact_publications("Ignoring pacts successfully verified by this provider tag when not WIP", verified_by_this_tag) log_pact_publications("Ignoring pacts successfully verified by another provider tag when not WIP", verified_by_another_tag) end - pact_publications = PactPublication.subtract(pact_publications, verified_by_this_tag.all) - PactPublication.subtract(pact_publications, verified_by_another_tag.all) + PactPublication.subtract(pact_publications_query.all, specified_explicitly.all, verified_by_this_tag.all, verified_by_another_tag.all) end def scope_for(scope) @@ -323,7 +329,10 @@ def scope_for(scope) end def collect_consumer_name_and_version_number(pact_publications_query) - pact_publications_query.eager(:provider).eager(:consumer).eager(:consumer_version).order(:consumer_version_order).all.sort.collect{ |p| "#{p.consumer_name} #{p.consumer_version_number}" } + pact_publications_query.eager(:provider).eager(:consumer).eager(:consumer_version).order(:consumer_version_order).all.sort.collect do |p| + tag_suffix = p.values[:tag_name] ? " (tag #{p.values[:tag_name]})" : "" + "#{p.consumer_name} #{p.consumer_version_number}" + tag_suffix + end end def log_pact_publications(message, pact_publications_query) diff --git a/lib/pact_broker/pacts/repository.rb b/lib/pact_broker/pacts/repository.rb index 8fdec9b18..c5bd7e81d 100644 --- a/lib/pact_broker/pacts/repository.rb +++ b/lib/pact_broker/pacts/repository.rb @@ -160,8 +160,8 @@ def find_for_verification(provider_name, consumer_version_selectors) PactsForVerificationRepository.new.find(provider_name, consumer_version_selectors) end - def find_wip_pact_versions_for_provider provider_name, provider_version_branch, provider_tags_names = [], options = {} - PactsForVerificationRepository.new.find_wip(provider_name, provider_version_branch, provider_tags_names, options) + def find_wip_pact_versions_for_provider provider_name, provider_version_branch, provider_tags_names, specified_pact_version_shas, options = {} + PactsForVerificationRepository.new.find_wip(provider_name, provider_version_branch, provider_tags_names, specified_pact_version_shas, options) end def find_pact_versions_for_provider provider_name, tag = nil diff --git a/lib/pact_broker/pacts/service.rb b/lib/pact_broker/pacts/service.rb index 730856b39..76bea246a 100644 --- a/lib/pact_broker/pacts/service.rb +++ b/lib/pact_broker/pacts/service.rb @@ -116,33 +116,23 @@ def find_distinct_pacts_between consumer, options end def find_for_verification(provider_name, provider_version_branch, provider_version_tags, consumer_version_selectors, options) - verifiable_pacts_specified_in_request = pact_repository + explicitly_specified_verifiable_pacts = pact_repository .find_for_verification(provider_name, consumer_version_selectors) .collect do | selected_pact | + # Todo move this into the repository squash_pacts_for_verification(provider_version_tags, selected_pact, options[:include_pending_status]) end verifiable_wip_pacts = if options[:include_wip_pacts_since] - exclude_specified_pacts( - pact_repository.find_wip_pact_versions_for_provider(provider_name, provider_version_branch, provider_version_tags, options), - verifiable_pacts_specified_in_request) + specified_pact_version_shas = explicitly_specified_verifiable_pacts.collect(&:pact_version_sha) + pact_repository.find_wip_pact_versions_for_provider(provider_name, provider_version_branch, provider_version_tags, specified_pact_version_shas, options) else [] end - verifiable_pacts_specified_in_request + verifiable_wip_pacts + explicitly_specified_verifiable_pacts + verifiable_wip_pacts end - def exclude_specified_pacts(wip_pacts, specified_pacts) - wip_pacts.reject do | wip_pact | - specified_pacts.any? do | specified_pact | - wip_pact.pact_version_sha == specified_pact.pact_version_sha - end - end - end - - private :exclude_specified_pacts - # Overwriting an existing pact with the same consumer/provider/consumer version number def update_pact params, existing_pact logger.info "Updating existing pact publication with params #{params.reject{ |k, _v| k == :json_content}}" diff --git a/spec/lib/pact_broker/pacts/repository_find_wip_pact_versions_for_provider_branch_spec.rb b/spec/lib/pact_broker/pacts/repository_find_wip_pact_versions_for_provider_branch_spec.rb index 04c54bfaa..a36138724 100644 --- a/spec/lib/pact_broker/pacts/repository_find_wip_pact_versions_for_provider_branch_spec.rb +++ b/spec/lib/pact_broker/pacts/repository_find_wip_pact_versions_for_provider_branch_spec.rb @@ -10,7 +10,7 @@ module Pacts let(:options) { { include_wip_pacts_since: include_wip_pacts_since } } let(:include_wip_pacts_since) { (Date.today - 1).to_datetime } - subject { Repository.new.find_wip_pact_versions_for_provider("bar", provider_version_branch, provider_tags, options) } + subject { Repository.new.find_wip_pact_versions_for_provider("bar", provider_version_branch, provider_tags, [], options) } context "when there are no tags or branch" do let(:provider_tags) { [] } diff --git a/spec/lib/pact_broker/pacts/repository_find_wip_pact_versions_for_provider_spec.rb b/spec/lib/pact_broker/pacts/repository_find_wip_pact_versions_for_provider_spec.rb index 7192f56d8..8b431cfa7 100644 --- a/spec/lib/pact_broker/pacts/repository_find_wip_pact_versions_for_provider_spec.rb +++ b/spec/lib/pact_broker/pacts/repository_find_wip_pact_versions_for_provider_spec.rb @@ -9,7 +9,7 @@ module Pacts let(:options) { { include_wip_pacts_since: include_wip_pacts_since } } let(:include_wip_pacts_since) { (Date.today - 1).to_datetime } - subject { Repository.new.find_wip_pact_versions_for_provider("bar", provider_version_branch, provider_tags, options) } + subject { Repository.new.find_wip_pact_versions_for_provider("bar", provider_version_branch, provider_tags, [], options) } context "when there are no tags" do let(:provider_tags) { [] }