From e96544f69673269a2bac7cb5ea94ea24e732e557 Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Fri, 15 Mar 2019 20:01:59 +1100 Subject: [PATCH] feat(matrix): improve reasons in response when pacticipant cannot be deployed --- MATRIX.md | 4 + .../matrix/deployment_status_summary.rb | 96 ++++++-- lib/pact_broker/matrix/integration.rb | 10 +- lib/pact_broker/matrix/query_results.rb | 5 +- ..._results_with_deployment_status_summary.rb | 4 +- lib/pact_broker/matrix/repository.rb | 98 ++++---- lib/pact_broker/matrix/resolved_selector.rb | 95 ++++++- lib/pact_broker/matrix/row.rb | 8 + lib/pact_broker/matrix/service.rb | 18 +- .../api/decorators/matrix_decorator_spec.rb | 3 +- .../matrix/deployment_status_summary_spec.rb | 125 ++++++++-- .../pact_broker/matrix/integration_spec.rb | 233 ++++++++++++++++++ .../repository_find_integrations_spec.rb | 51 ---- .../lib/pact_broker/matrix/repository_spec.rb | 9 +- spec/lib/pact_broker/matrix/service_spec.rb | 92 ------- 15 files changed, 578 insertions(+), 273 deletions(-) create mode 100644 MATRIX.md create mode 100644 spec/lib/pact_broker/matrix/integration_spec.rb delete mode 100644 spec/lib/pact_broker/matrix/repository_find_integrations_spec.rb diff --git a/MATRIX.md b/MATRIX.md new file mode 100644 index 000000000..f9472b8fc --- /dev/null +++ b/MATRIX.md @@ -0,0 +1,4 @@ +# Interpreting the Matrix results + +* If there is a row with a blank provider version, it's because the pact for that consumer version hasn't been verified by that provider (the result of a left outer join). +* If there is no row, it's because it has been verified, but not by the provider version you've specified. diff --git a/lib/pact_broker/matrix/deployment_status_summary.rb b/lib/pact_broker/matrix/deployment_status_summary.rb index 3c16381d1..8b99cecac 100644 --- a/lib/pact_broker/matrix/deployment_status_summary.rb +++ b/lib/pact_broker/matrix/deployment_status_summary.rb @@ -22,29 +22,52 @@ def counts end def deployable? - # return nil if rows.empty? + return false if specified_selectors_that_do_not_exist.any? return nil if rows.any?{ |row| row.success.nil? } return nil if required_integrations_without_a_row.any? - rows.all?{ |row| row.success } + rows.all?{ |row| row.success } # true if rows is empty end def reasons - @reasons ||= begin - reasons = [] - reasons.concat(missing_reasons) - reasons.concat(failure_messages) - reasons.concat(unverified_messages) - reasons.concat(success_messages) - reasons + error_messages.any? ? error_messages : success_messages + end + + def error_messages + @error_messages ||= begin + messages = [] + messages.concat(specified_selectors_do_not_exist_messages) + if messages.empty? + messages.concat(missing_reasons) + messages.concat(failure_messages) + messages.concat(unverified_messages) + end + messages.uniq end end + def specified_selectors_that_do_not_exist + resolved_selectors.select(&:specified_version_that_does_not_exist?) + end + + def specified_selectors_do_not_exist_messages + specified_selectors_that_do_not_exist.collect(&:version_does_not_exist_description) + end + def unverified_messages if rows.any?{ |row| row.success.nil? } - ["Missing one or more verification results"] + rows.collect do | row | + missing_verified_pact_reason(row) + + # selectors = selectors_without_a_version_for(row) + # if selectors.any? + # selectors.collect(&:version_does_not_exist_description) + # else + # ["Missing one or more required verification results"] + # end + end else [] - end + end.flatten.uniq end def failure_messages @@ -57,10 +80,14 @@ def failure_messages def success_messages if rows.all?{ |row| row.success } && required_integrations_without_a_row.empty? - ["All verification results are published and successful"] + if rows.any? + ["All required verification results are published and successful"] + else + ["There are no missing dependencies"] + end else [] - end + end.flatten.uniq end # For deployment, the consumer requires the provider, @@ -77,20 +104,45 @@ def required_integrations_without_a_row end def missing_reasons - required_integrations_without_a_row.collect do | missing_relationship| - consumer_version_desc = "#{missing_relationship.consumer_name} (#{resolved_version_for(missing_relationship.consumer_id)})" - provider_version_desc = "#{missing_relationship.provider_name} (#{resolved_version_for(missing_relationship.provider_id)})" - "There is no verified pact between #{consumer_version_desc} and #{provider_version_desc}" + required_integrations_without_a_row.collect do | integration | + relevant_selectors_without_a_version = selectors_without_a_version_for(integration) + # if relevant_selectors_without_a_version.any? + # missing_specified_version_reasons(relevant_selectors_without_a_version) + # else + missing_verified_pact_reason(integration) + # end + end.flatten + end + + def selectors_without_a_version_for(integration) + selectors_with_non_existing_versions.select do | selector | + integration.involves_pacticipant_with_name?(selector.pacticipant_name) end end - def resolved_version_for(pacticipant_id) - resolved_selector = resolved_selectors.find{ | s| s[:pacticipant_id] == pacticipant_id } + def selectors_with_non_existing_versions + @selectors_with_non_existing_versions ||= resolved_selectors.select(&:latest_tagged_version_that_does_not_exist?) + end + + def missing_specified_version_reasons(selectors) + selectors.collect(&:version_does_not_exist_description) + end + + def missing_verified_pact_reason(integration) + "There is no verified pact between #{description_for_selector(integration.consumer_name)} and #{description_for_selector(integration.provider_name)}" + # "There is no verification by #{description_for_selector(integration.provider_name)} for the pact for #{description_for_selector(integration.consumer_name)}" + end + + def description_for_selector(pacticipant_name) + resolved_selector = resolved_selectors.find{ | s| s.pacticipant_name == pacticipant_name } if resolved_selector - resolved_selector[:pacticipant_version_number] + resolved_selector.description else - logger.warn "Could not find the resolved version for pacticipant_id #{pacticipant_id} from integrations #{integrations.collect(&:to_s).join(", ")} in resolved selectors #{resolved_selectors.inspect}" - "unresolved version" + # This happens when the user has not specified a version of the provider (eg no 'latest' and/or 'tag') + # so the "add inferred selectors" code has not run + # AND no versions of the provider exist (ie. it has never verified the pact). + logger.warn "Could not find the resolved version for pacticipant_name #{pacticipant_name} from integrations #{integrations.collect(&:to_s).join(", ")} in resolved selectors #{resolved_selectors.inspect}" + "#{pacticipant_name} (unresolved version)" end end end diff --git a/lib/pact_broker/matrix/integration.rb b/lib/pact_broker/matrix/integration.rb index d78e6a11d..73a2baf6f 100644 --- a/lib/pact_broker/matrix/integration.rb +++ b/lib/pact_broker/matrix/integration.rb @@ -57,7 +57,7 @@ def pacticipant_names end def to_s - "Relationship between #{consumer_name} (id=#{consumer_id}) and #{provider_name} (id=#{provider_id})" + "Integration between #{consumer_name} (id=#{consumer_id}) and #{provider_name} (id=#{provider_id})" end def involves_consumer_with_id?(consumer_id) @@ -75,6 +75,14 @@ def involves_provider_with_name?(provider_name) def involves_consumer_with_name?(consumer_name) self.consumer_name == consumer_name end + + def pacticipant_names + [consumer_name, provider_name] + end + + def involves_pacticipant_with_name?(pacticipant_name) + pacticipant_names.include?(pacticipant_name) + end end end end diff --git a/lib/pact_broker/matrix/query_results.rb b/lib/pact_broker/matrix/query_results.rb index ff5e28074..c77320d62 100644 --- a/lib/pact_broker/matrix/query_results.rb +++ b/lib/pact_broker/matrix/query_results.rb @@ -1,13 +1,14 @@ module PactBroker module Matrix class QueryResults < Array - attr_reader :selectors, :options, :resolved_selectors + attr_reader :selectors, :options, :resolved_selectors, :integrations - def initialize rows, selectors, options, resolved_selectors + def initialize rows, selectors, options, resolved_selectors, integrations super(rows) @selectors = selectors @resolved_selectors = resolved_selectors @options = options + @integrations = integrations end def rows diff --git a/lib/pact_broker/matrix/query_results_with_deployment_status_summary.rb b/lib/pact_broker/matrix/query_results_with_deployment_status_summary.rb index a27a79b9e..0a4b1d2de 100644 --- a/lib/pact_broker/matrix/query_results_with_deployment_status_summary.rb +++ b/lib/pact_broker/matrix/query_results_with_deployment_status_summary.rb @@ -5,8 +5,8 @@ module Matrix class QueryResultsWithDeploymentStatusSummary < QueryResults attr_reader :deployment_status_summary - def initialize rows, selectors, options, resolved_selectors, deployment_status_summary - super(rows, selectors, options, resolved_selectors) + def initialize rows, selectors, options, resolved_selectors, integrations, deployment_status_summary + super(rows, selectors, options, resolved_selectors, integrations) @deployment_status_summary = deployment_status_summary end end diff --git a/lib/pact_broker/matrix/repository.rb b/lib/pact_broker/matrix/repository.rb index 920da74a2..4a835a5a8 100644 --- a/lib/pact_broker/matrix/repository.rb +++ b/lib/pact_broker/matrix/repository.rb @@ -60,7 +60,9 @@ def find specified_selectors, options = {} lines = lines.select{ |l| options[:success].include?(l.success) } end - QueryResults.new(lines.sort, specified_selectors, options, resolved_selectors) + integrations = find_integrations_for_specified_selectors(resolved_selectors.select(&:specified?)) + + QueryResults.new(lines.sort, specified_selectors, options, resolved_selectors, integrations) end def find_for_consumer_and_provider pacticipant_1_name, pacticipant_2_name @@ -73,11 +75,13 @@ def find_compatible_pacticipant_versions selectors find(selectors, latestby: 'cvpv').select{|line| line.success } end - def find_integrations(pacticipant_names) - selectors = pacticipant_names.collect{ | pacticipant_name | add_ids_to_selector(pacticipant_name: pacticipant_name) } + # If one pacticipant is specified, find all the integrations that involve that pacticipant + # If two or more are specified, just return the integrations that involve the specified pacticipants + def find_integrations_for_specified_selectors(resolved_specified_selectors) + specified_pacticipant_names = resolved_specified_selectors.collect(&:pacticipant_name) Row .select(:consumer_name, :consumer_id, :provider_name, :provider_id) - .matching_selectors(selectors) + .matching_selectors(resolved_specified_selectors) .distinct .all .collect do |row | @@ -85,12 +89,21 @@ def find_integrations(pacticipant_names) end .uniq .collect do | hash | - Integration.from_hash(hash.merge(required: pacticipant_names.include?(hash[:consumer_name]))) + required = is_a_row_for_this_integration_required?(specified_pacticipant_names, hash[:consumer_name]) + Integration.from_hash(hash.merge(required: required)) end end private + # If one of the specified pacticipants is a consumer, then that provider is required to be deployed + # to the same environment before the consumer can be deployed. + # If one of the specified pacticipants is a provider, then the provider may be deployed + # without the consumer being present. + def is_a_row_for_this_integration_required?(specified_pacticipant_names, consumer_name) + specified_pacticipant_names.include?(consumer_name) + end + def apply_latestby options, selectors, lines return lines unless options[:latestby] group_by_columns = case options[:latestby] @@ -121,7 +134,7 @@ def remove_overwritten_revisions lines end def query_matrix selectors, options - query = view_for(options).select_all.matching_selectors(selectors) + query = Row.select_all.matching_selectors(selectors) query = query.limit(options[:limit]) if options[:limit] query .order_by_names_ascending_most_recent_first @@ -130,12 +143,8 @@ def query_matrix selectors, options .all end - def view_for(options) - Row - end - def resolve_selectors(specified_selectors, options) - resolved_specified_selectors = resolve_versions_and_add_ids(specified_selectors, options) + resolved_specified_selectors = resolve_versions_and_add_ids(specified_selectors, :specified) if options[:latest] || options[:tag] add_inferred_selectors(resolved_specified_selectors, options) else @@ -144,42 +153,40 @@ def resolve_selectors(specified_selectors, options) end # Find the version number for selectors with the latest and/or tag specified - def resolve_versions_and_add_ids(selectors, options, required = true) + def resolve_versions_and_add_ids(selectors, selector_type) selectors.collect do | selector | pacticipant = PactBroker::Domain::Pacticipant.find(name: selector[:pacticipant_name]) + versions = find_versions_for_selector(selector) + build_selectors_for_pacticipant_and_versions(pacticipant, versions, selector, selector_type) + end.flatten + end - versions = find_versions_for_selector(selector, required) - - if versions - versions.collect do | version | - if version - selector_for_version(pacticipant, version) - else - selector_for_non_existing_version(pacticipant) - end + def build_selectors_for_pacticipant_and_versions(pacticipant, versions, original_selector, selector_type) + if versions + versions.collect do | version | + if version + selector_for_version(pacticipant, version, original_selector, selector_type) + else + selector_for_non_existing_version(pacticipant, original_selector, selector_type) end - else - selector_without_version(pacticipant) end - end.flatten + else + selector_without_version(pacticipant, selector_type) + end end - def find_versions_for_selector(selector, required) + def find_versions_for_selector(selector) if selector[:tag] && selector[:latest] version = version_repository.find_by_pacticipant_name_and_latest_tag(selector[:pacticipant_name], selector[:tag]) - # raise Error.new("No version of #{selector[:pacticipant_name]} found with tag #{selector[:tag]}") if required && !version [version] elsif selector[:latest] version = version_repository.find_latest_by_pacticpant_name(selector[:pacticipant_name]) - # raise Error.new("No version of #{selector[:pacticipant_name]} found") if required && !version [version] elsif selector[:tag] versions = version_repository.find_by_pacticipant_name_and_tag(selector[:pacticipant_name], selector[:tag]) - # raise Error.new("No version of #{selector[:pacticipant_name]} found with tag #{selector[:tag]}") if required && versions.empty? versions.any? ? versions : [nil] elsif selector[:pacticipant_version_number] version = version_repository.find_by_pacticipant_name_and_number(selector[:pacticipant_name], selector[:pacticipant_version_number]) - # raise Error.new("No version #{selector[:pacticipant_version_number]} of #{selector[:pacticipant_name]} found") if required && !version [version] else nil @@ -203,24 +210,17 @@ def add_ids_to_selector(selector) selector end - # eg. when checking to see if Foo version 2 can be deployed to prod, - # need to look up all the 'partner' pacticipants, and determine their latest prod versions + # When only one selector is specified, (eg. checking to see if Foo version 2 can be deployed to prod), + # need to look up all integrated pacticipants, and determine their relevant (eg. latest prod) versions def add_inferred_selectors(resolved_specified_selectors, options) - integrations = find_integrations(resolved_specified_selectors.collect{|s| s[:pacticipant_name]}) + integrations = find_integrations_for_specified_selectors(resolved_specified_selectors) all_pacticipant_names = integrations.collect(&:pacticipant_names).flatten.uniq specified_names = resolved_specified_selectors.collect{ |s| s[:pacticipant_name] } inferred_pacticipant_names = all_pacticipant_names - specified_names - # Inferred providers are required for a consumer to be deployed - required_inferred_pacticipant_names = inferred_pacticipant_names.select{ | n | integrations.any?{ |i| i.involves_provider_with_name?(n) } } - # Inferred consumers are NOT required for a provider to be deployed - optional_inferred_pacticipant_names = inferred_pacticipant_names - required_inferred_pacticipant_names - - resolved_specified_selectors + - build_inferred_selectors(required_inferred_pacticipant_names, options, true) + - build_inferred_selectors(optional_inferred_pacticipant_names, options, false) + resolved_specified_selectors + build_inferred_selectors(inferred_pacticipant_names, options) end - def build_inferred_selectors(inferred_pacticipant_names, options, required) + def build_inferred_selectors(inferred_pacticipant_names, options) selectors = inferred_pacticipant_names.collect do | pacticipant_name | selector = { pacticipant_name: pacticipant_name @@ -229,26 +229,26 @@ def build_inferred_selectors(inferred_pacticipant_names, options, required) selector[:latest] = options[:latest] if options[:latest] selector end - resolve_versions_and_add_ids(selectors, options, required) + resolve_versions_and_add_ids(selectors, :inferred) end def all_pacticipant_names_in_specified_matrix(selectors) - find_integrations(selectors.collect{|s| s[:pacticipant_name]}) + find_integrations_for_specified_selectors(selectors) .collect(&:pacticipant_names) .flatten .uniq end - def selector_for_non_existing_version(pacticipant) - ResolvedSelector.for_pacticipant_and_non_existing_version(pacticipant) + def selector_for_non_existing_version(pacticipant, original_selector, selector_type) + ResolvedSelector.for_pacticipant_and_non_existing_version(pacticipant, original_selector, selector_type) end - def selector_for_version(pacticipant, version) - ResolvedSelector.for_pacticipant_and_version(pacticipant, version) + def selector_for_version(pacticipant, version, original_selector, selector_type) + ResolvedSelector.for_pacticipant_and_version(pacticipant, version, original_selector, selector_type) end - def selector_without_version(pacticipant) - ResolvedSelector.for_pacticipant(pacticipant) + def selector_without_version(pacticipant, selector_type) + ResolvedSelector.for_pacticipant(pacticipant, selector_type) end end end diff --git a/lib/pact_broker/matrix/resolved_selector.rb b/lib/pact_broker/matrix/resolved_selector.rb index d84ddeacf..6974a44bf 100644 --- a/lib/pact_broker/matrix/resolved_selector.rb +++ b/lib/pact_broker/matrix/resolved_selector.rb @@ -8,29 +8,36 @@ def initialize(params) merge!(params) end - def self.for_pacticipant(pacticipant) + def self.for_pacticipant(pacticipant, type) ResolvedSelector.new( pacticipant_id: pacticipant.id, - pacticipant_name: pacticipant.name + pacticipant_name: pacticipant.name, + type: type ) end - def self.for_pacticipant_and_version(pacticipant, version) + def self.for_pacticipant_and_version(pacticipant, version, original_selector, type) ResolvedSelector.new( pacticipant_id: pacticipant.id, pacticipant_name: pacticipant.name, pacticipant_version_id: version.id, - pacticipant_version_number: version.number + pacticipant_version_number: version.number, + latest: original_selector[:latest], + tag: original_selector[:tag], + type: type ) end # An ID of -1 will not match any rows, which is what we want - def self.for_pacticipant_and_non_existing_version(pacticipant) + def self.for_pacticipant_and_non_existing_version(pacticipant, original_selector, type) ResolvedSelector.new( pacticipant_id: pacticipant.id, pacticipant_name: pacticipant.name, pacticipant_version_id: NULL_VERSION_ID, - pacticipant_version_number: "" + pacticipant_version_number: original_selector[:pacticipant_version_number], + latest: original_selector[:latest], + tag: original_selector[:tag], + type: type ) end @@ -50,10 +57,78 @@ def pacticipant_version_number self[:pacticipant_version_number] end - def version_not_found? - pacticipant_version_id == NULL_VERSION_ID + def latest? + self[:latest] + end + + def tag + self[:tag] + end + + def latest_tagged? + latest? && tag + end + + def version_does_not_exist? + !version_exists? + end + + def latest_tagged_version_that_does_not_exist? + version_does_not_exist? && latest_tagged? + end + + def specified_version_that_does_not_exist? + specified? && version_does_not_exist? + end + + def version_exists? + pacticipant_version_id != NULL_VERSION_ID end - end + # Did the user specify this selector in the HTTP query? + def specified? + self[:type] == :specified + end + + # Was this selector inferred based on the HTTP query? + #(ie. the integrations were calculated because only on selector was specified) + def inferred? + self[:type] == :inferred + end + + def description + if latest_tagged? && pacticipant_version_number + "the latest version of #{pacticipant_name} with tag #{tag} (#{pacticipant_version_number})" + elsif latest_tagged? + "the latest version of #{pacticipant_name} with tag #{tag} (it does not exist)" + elsif latest? && pacticipant_version_number + "the latest version of #{pacticipant_name} (#{pacticipant_version_number})" + elsif latest? + "the latest version of #{pacticipant_name} (it does not exist)" + elsif tag && pacticipant_version_number + "a version of #{pacticipant_name} with tag #{tag} (#{pacticipant_version_number})" + elsif tag + "a version of #{pacticipant_name} with tag #{tag} (it does not exist)" + elsif pacticipant_version_number + "version #{pacticipant_version_number} of #{pacticipant_name}" + else + "all versions of #{pacticipant_name}" + end + end + + def version_does_not_exist_description + if version_does_not_exist? + if tag + "No version with tag #{tag} exists for #{pacticipant_name}" + elsif pacticipant_version_number + "No version with number #{pacticipant_version_number} exists for #{pacticipant_name}" + else + "No versions exist for #{pacticipant_name}" + end + else + nil + end + end + end end -end \ No newline at end of file +end diff --git a/lib/pact_broker/matrix/row.rb b/lib/pact_broker/matrix/row.rb index 634c4508d..cb91a6d7b 100644 --- a/lib/pact_broker/matrix/row.rb +++ b/lib/pact_broker/matrix/row.rb @@ -197,6 +197,14 @@ def values def eql?(obj) (obj.class == model) && (obj.values == values) end + + def pacticipant_names + [consumer_name, provider_name] + end + + def involves_pacticipant_with_name?(pacticipant_name) + pacticipant_name.include?(pacticipant_name) + end end end end diff --git a/lib/pact_broker/matrix/service.rb b/lib/pact_broker/matrix/service.rb index 7b590a0a8..ccaa82aab 100644 --- a/lib/pact_broker/matrix/service.rb +++ b/lib/pact_broker/matrix/service.rb @@ -12,10 +12,8 @@ module Service def find selectors, options = {} query_results = matrix_repository.find selectors, options - pacticipant_names = selectors.collect{ | s| s[:pacticipant_name] } - integrations = matrix_repository.find_integrations(pacticipant_names) - deployment_status_summary = DeploymentStatusSummary.new(query_results.rows, query_results.resolved_selectors, integrations) - QueryResultsWithDeploymentStatusSummary.new(query_results.rows, query_results.selectors, query_results.options, query_results.resolved_selectors, deployment_status_summary) + deployment_status_summary = DeploymentStatusSummary.new(query_results.rows, query_results.resolved_selectors, query_results.integrations) + QueryResultsWithDeploymentStatusSummary.new(query_results.rows, query_results.selectors, query_results.options, query_results.resolved_selectors, query_results.integrations, deployment_status_summary) end def find_for_consumer_and_provider params @@ -67,18 +65,6 @@ def validate_selectors selectors end end - if error_messages.empty? - selectors.each do | s | - if s[:pacticipant_version_number] - version = version_service.find_by_pacticipant_name_and_number(pacticipant_name: s[:pacticipant_name], pacticipant_version_number: s[:pacticipant_version_number]) - error_messages << "No pact or verification found for #{s[:pacticipant_name]} version #{s[:pacticipant_version_number]}" if version.nil? - elsif s[:tag] - version = version_service.find_by_pacticipant_name_and_latest_tag(s[:pacticipant_name], s[:tag]) - error_messages << "No version of #{s[:pacticipant_name]} found with tag #{s[:tag]}" if version.nil? - end - end - end - if selectors.size == 0 error_messages << "Please provide 1 or more version selectors." end diff --git a/spec/lib/pact_broker/api/decorators/matrix_decorator_spec.rb b/spec/lib/pact_broker/api/decorators/matrix_decorator_spec.rb index 756a32bd8..645f0a9f0 100644 --- a/spec/lib/pact_broker/api/decorators/matrix_decorator_spec.rb +++ b/spec/lib/pact_broker/api/decorators/matrix_decorator_spec.rb @@ -101,8 +101,9 @@ module Decorators } end - let(:query_results){ PactBroker::Matrix::QueryResultsWithDeploymentStatusSummary.new([row_1, row_2], selectors, options, resolved_selectors, deployment_status_summary)} + let(:query_results){ PactBroker::Matrix::QueryResultsWithDeploymentStatusSummary.new([row_1, row_2], selectors, options, resolved_selectors, integrations, deployment_status_summary)} let(:selectors) { nil } + let(:integrations){ [] } let(:options) { nil } let(:resolved_selectors) { nil } let(:counts) { { success: 1 } } diff --git a/spec/lib/pact_broker/matrix/deployment_status_summary_spec.rb b/spec/lib/pact_broker/matrix/deployment_status_summary_spec.rb index dc4b8640a..279748002 100644 --- a/spec/lib/pact_broker/matrix/deployment_status_summary_spec.rb +++ b/spec/lib/pact_broker/matrix/deployment_status_summary_spec.rb @@ -2,6 +2,7 @@ require 'pact_broker/matrix/row' require 'pact_broker/matrix/query_results' require 'pact_broker/matrix/integration' +require 'pact_broker/matrix/resolved_selector' module PactBroker module Matrix @@ -21,7 +22,8 @@ module Matrix consumer_id: 1, provider_name: "Bar", provider_id: 2, - success: row_1_success + success: row_1_success, + pacticipant_names: %w{Foo Bar} ) end @@ -31,7 +33,8 @@ module Matrix consumer_id: 1, provider_name: "Baz", provider_id: 3, - success: true + success: true, + pacticipant_names: %w{Foo Baz} ) end @@ -44,17 +47,42 @@ module Matrix ] end + let(:foo) { double('foo', id: 1, name: "Foo") } + let(:bar) { double('bar', id: 2, name: "Bar") } + let(:baz) { double('baz', id: 3, name: "Baz") } + let(:foo_version) { double('foo version', number: "ddec8101dabf4edf9125a69f9a161f0f294af43c", id: 10)} + let(:bar_version) { double('bar version', number: "14131c5da3abf323ccf410b1b619edac76231243", id: 10)} + let(:baz_version) { double('baz version', number: "4ee06460f10e8207ad904fa9fa6c4842e462ab59", id: 10)} + let(:resolved_selectors) do [ - { - pacticipant_id: 1, pacticipant_name: "Foo", pacticipant_version_number: "ddec8101dabf4edf9125a69f9a161f0f294af43c" - }, - { - pacticipant_id: 2, pacticipant_name: "Bar", pacticipant_version_number: "14131c5da3abf323ccf410b1b619edac76231243" - }, - { - pacticipant_id: 3, pacticipant_name: "Baz", pacticipant_version_number: "4ee06460f10e8207ad904fa9fa6c4842e462ab59" - } + double('foo selector', + pacticipant_id: 1, + pacticipant_name: "Foo", + pacticipant_version_number: "ddec8101dabf4edf9125a69f9a161f0f294af43c", + pacticipant_version_id: 10, + latest_tagged_version_that_does_not_exist?: false, + specified_version_that_does_not_exist?: false, + description: "version ddec8101dabf4edf9125a69f9a161f0f294af43c of Foo" + ), + double('bar selector', + pacticipant_id: 2, + pacticipant_name: "Bar", + pacticipant_version_number: "14131c5da3abf323ccf410b1b619edac76231243", + pacticipant_version_id: 11, + latest_tagged_version_that_does_not_exist?: false, + specified_version_that_does_not_exist?: false, + description: "version 14131c5da3abf323ccf410b1b619edac76231243 of Bar" + ), + double('baz selector', + pacticipant_id: 3, + pacticipant_name: "Baz", + pacticipant_version_number: "4ee06460f10e8207ad904fa9fa6c4842e462ab59", + pacticipant_version_id: 12, + latest_tagged_version_that_does_not_exist?: false, + specified_version_that_does_not_exist?: false, + description: "version 4ee06460f10e8207ad904fa9fa6c4842e462ab59 of Baz" + ), ] end @@ -62,7 +90,7 @@ module Matrix context "when there is a row for all integrations" do its(:deployable?) { is_expected.to be true } - its(:reasons) { is_expected.to eq ["All verification results are published and successful"] } + its(:reasons) { is_expected.to eq ["All required verification results are published and successful"] } its(:counts) { is_expected.to eq success: 2, failed: 0, unknown: 0 } end @@ -72,8 +100,8 @@ module Matrix its(:deployable?) { is_expected.to be nil } its(:reasons) do is_expected.to eq [ - "There is no verified pact between Foo (ddec8101dabf4edf9125a69f9a161f0f294af43c) and Bar (14131c5da3abf323ccf410b1b619edac76231243)", - "There is no verified pact between Foo (ddec8101dabf4edf9125a69f9a161f0f294af43c) and Baz (4ee06460f10e8207ad904fa9fa6c4842e462ab59)", + "There is no verified pact between version ddec8101dabf4edf9125a69f9a161f0f294af43c of Foo and version 14131c5da3abf323ccf410b1b619edac76231243 of Bar", + "There is no verified pact between version ddec8101dabf4edf9125a69f9a161f0f294af43c of Foo and version 4ee06460f10e8207ad904fa9fa6c4842e462ab59 of Baz", ] end its(:counts) { is_expected.to eq success: 0, failed: 0, unknown: 2 } @@ -83,7 +111,12 @@ module Matrix let(:row_1_success) { nil } its(:deployable?) { is_expected.to be nil } - its(:reasons) { is_expected.to eq ["Missing one or more verification results"] } + its(:reasons) do + is_expected.to eq [ + "There is no verified pact between version ddec8101dabf4edf9125a69f9a161f0f294af43c of Foo and version 14131c5da3abf323ccf410b1b619edac76231243 of Bar", + "There is no verified pact between version ddec8101dabf4edf9125a69f9a161f0f294af43c of Foo and version 4ee06460f10e8207ad904fa9fa6c4842e462ab59 of Baz", + ] + end its(:counts) { is_expected.to eq success: 1, failed: 0, unknown: 1 } end @@ -99,11 +132,11 @@ module Matrix let(:rows) { [row_1] } its(:deployable?) { is_expected.to be nil } - its(:reasons) { is_expected.to eq ["There is no verified pact between Foo (ddec8101dabf4edf9125a69f9a161f0f294af43c) and Baz (4ee06460f10e8207ad904fa9fa6c4842e462ab59)"] } + its(:reasons) { is_expected.to eq ["There is no verified pact between version ddec8101dabf4edf9125a69f9a161f0f294af43c of Foo and version 4ee06460f10e8207ad904fa9fa6c4842e462ab59 of Baz"] } its(:counts) { is_expected.to eq success: 1, failed: 0, unknown: 1 } end - context "when there is a consumer row missing and only the provider was specified in the query" do + context "when there is a consumer integration missing and only the provider was specified in the query" do let(:rows) { [row_1] } let(:integrations) do [ @@ -113,11 +146,11 @@ module Matrix end its(:deployable?) { is_expected.to be true } - its(:reasons) { is_expected.to eq ["All verification results are published and successful"] } + its(:reasons) { is_expected.to eq ["All required verification results are published and successful"] } its(:counts) { is_expected.to eq success: 1, failed: 0, unknown: 0 } end - context "when there is a consumer relationship missing and only the consumer was specified in the query" do + context "when there is a provider integration missing and only the consumer was specified in the query" do let(:rows) { [row_1] } let(:integrations) do @@ -127,26 +160,66 @@ module Matrix ] end - let(:rows) { [row_1] } - its(:deployable?) { is_expected.to be nil } - its(:reasons) { is_expected.to eq ["There is no verified pact between Foo (ddec8101dabf4edf9125a69f9a161f0f294af43c) and Baz (4ee06460f10e8207ad904fa9fa6c4842e462ab59)"] } + its(:reasons) { is_expected.to eq ["There is no verified pact between version ddec8101dabf4edf9125a69f9a161f0f294af43c of Foo and version 4ee06460f10e8207ad904fa9fa6c4842e462ab59 of Baz"] } its(:counts) { is_expected.to eq success: 1, failed: 0, unknown: 1 } end + context "when there is a provider integration missing because the provider version does not exist" do + let(:rows) { [] } + let(:integrations) do + [ + Integration.new(1, "Foo", 2, "Bar", true) + ] + end + + let(:resolved_selectors) do + [ + double('foo selector', + pacticipant_id: 1, + pacticipant_name: "Foo", + pacticipant_version_number: "ddec8101dabf4edf9125a69f9a161f0f294af43c", + pacticipant_version_id: 10, + latest_tagged_version_that_does_not_exist?: false, + specified_version_that_does_not_exist?: false, + description: 'verison foo'), + double('bar selector', + pacticipant_id: 2, + pacticipant_name: "Bar", + pacticipant_version_number: "", + pacticipant_version_id: 11, + latest_tagged_version_that_does_not_exist?: true, + involves_pacticipant_with_name?: true, + version_does_not_exist_description: "description", + specified_version_that_does_not_exist?: false, + description: 'bar version'), + ] + end + + its(:deployable?) { is_expected.to be nil } + its(:reasons) { is_expected.to eq ["There is no verified pact between verison foo and bar version"] } + its(:counts) { is_expected.to eq success: 0, failed: 0, unknown: 1 } + end + context "when there is something unexpected about the data and the resolved selector cannot be found" do let(:rows) { [row_1] } let(:resolved_selectors) do [ - { - pacticipant_id: 3, pacticipant_version_number: "4ee06460f10e8207ad904fa9fa6c4842e462ab59" - } + double('selector', + pacticipant_id: 3, + pacticipant_name: "Foo", + pacticipant_version_number: "4ee06460f10e8207ad904fa9fa6c4842e462ab59", + pacticipant_version_id: 10, + latest_tagged_version_that_does_not_exist?: false, + specified_version_that_does_not_exist?: false, + description: "version 4ee06460f10e8207ad904fa9fa6c4842e462ab59 of Foo" + ) ] end its(:deployable?) { is_expected.to be nil } - its(:reasons) { is_expected.to eq ["There is no verified pact between Foo (unresolved version) and Baz (4ee06460f10e8207ad904fa9fa6c4842e462ab59)"] } + its(:reasons) { is_expected.to eq ["There is no verified pact between version 4ee06460f10e8207ad904fa9fa6c4842e462ab59 of Foo and Baz (unresolved version)"] } it "logs a warning" do expect(logger).to receive(:warn).with(/Could not find the resolved version/) diff --git a/spec/lib/pact_broker/matrix/integration_spec.rb b/spec/lib/pact_broker/matrix/integration_spec.rb new file mode 100644 index 000000000..3f0178f25 --- /dev/null +++ b/spec/lib/pact_broker/matrix/integration_spec.rb @@ -0,0 +1,233 @@ +require 'pact_broker/matrix/service' + +module PactBroker + module Matrix + describe Service do + let(:td) { TestDataBuilder.new } + describe "find" do + subject { Service.find(selectors, options) } + + let(:options) { {} } + + describe "find" do + let(:selectors) do + [ { pacticipant_name: "foo" } ] + end + + let(:options) do + { latest: true, tag: "prod" } + end + + before do + td.create_pact_with_hierarchy("foo", "1", "bar") + .create_verification(provider_version: "2", tag_names: ["prod"]) + end + + it "returns a QueryResultsWithDeploymentStatusSummary" do + expect(subject.rows).to be_a(Array) + expect(subject.selectors).to be selectors + expect(subject.options).to be options + expect(subject.resolved_selectors).to be_a(Array) + expect(subject.resolved_selectors.count).to eq 2 + expect(subject.integrations.count).to eq 1 + expect(subject.deployment_status_summary).to be_a(DeploymentStatusSummary) + end + end + + describe "when deploying a version of a provider with multiple versions of a consumer in production that is missing a verification for the latest prod version" do + before do + td.create_pact_with_hierarchy("Foo", "1", "Bar") + .create_consumer_version_tag("prod") + .create_verification(provider_version: "10") + .create_consumer_version("2", tag_names: ["prod"]) + .create_pact + end + + let(:selectors) { [{ pacticipant_name: "Bar", pacticipant_version_number: "10" }]} + let(:options) { { tag: "prod" } } + + it "does not allow the consumer to be deployed" do + expect(subject.deployment_status_summary.deployable?).to_not be true + end + end + + describe "when deploying a consumer that has not been verified by any providers" do + before do + td.create_pact_with_hierarchy("Foo", "1", "Bar") + .create_provider("Baz") + .create_pact + end + let(:selectors) do + [ { pacticipant_name: "Foo", pacticipant_version_number: "1" } ] + end + + it "returns 2 integrations" do + expect(subject.integrations.size).to eq 2 + end + + it "but cannot resolve selectors for the providers" do + expect(subject.resolved_selectors.size).to eq 1 + end + + it "does not allow the consumer to be deployed" do + expect(subject.deployment_status_summary.deployable?).to_not be true + end + end + + describe "when deploying a consumer that has two providers in prod, but it is not verified by one of the prod provider versions, pact_broker-client issue #33" do + before do + td.create_pact_with_hierarchy("Foo", "3.0.0", "Bar") + .create_verification(provider_version: "10.0.0", tag_names: ["prod"]) + .create_provider("Baz") + .create_pact + .create_verification(provider_version: "20", tag_names:["prod"]) + .create_consumer_version("2.0.0") + .use_provider("Bar") + .create_pact + .create_verification(provider_version: "11.0.0", tag_names: ["prod"]) + end + + let(:selectors) do + [ { pacticipant_name: "Foo", pacticipant_version_number: "3.0.0" } ] + end + + let(:options) { {latest: true, tag: "prod"} } + + it "returns 2 integrations" do + expect(subject.integrations.size).to eq 2 + end + + it "returns 1 row" do + expect(subject.rows.size).to eq 1 + end + + it "does not allow the consumer to be deployed" do + expect(subject.deployment_status_summary.deployable?).to_not be true + end + end + + describe "when deploying an old version of a consumer that has added a new provider since that version" do + before do + td.create_pact_with_hierarchy("Foo", "1", "Bar") + .create_verification(provider_version: "2") + .create_consumer_version("2") + .create_pact + .create_verification(provider_version: "3") + .create_provider("Wiffle") + .create_pact + .create_verification(provider_version: "10") + end + + let(:selectors) do + [ { pacticipant_name: "Foo", pacticipant_version_number: "1" } ] + end + + it "allows the old version of the consumer to be deployed" do + expect(subject.deployment_status_summary.deployable?).to be true + end + end + + describe "when the specified version does not exist" do + before do + td.create_pact_with_hierarchy("Foo", "1", "Bar") + end + + let(:selectors) do + [ { pacticipant_name: "Bar", pacticipant_version_number: "5" } ] + end + + it "does not allow the provider to be deployed" do + expect(subject.deployment_status_summary.deployable?).to_not be true + end + end + + describe "when deploying a provider to prod for the first time and the consumer is not yet deployed" do + before do + td.create_pact_with_hierarchy("Foo", "1", "Bar") + .create_verification(provider_version: "2") + end + + let(:selectors) do + [ { pacticipant_name: "Bar", pacticipant_version_number: "2" } ] + end + + let(:options) do + { latest: true, tag: "prod" } + end + + subject { Service.find(selectors, options) } + + it "allows the provider to be deployed" do + expect(subject.deployment_status_summary.deployable?).to be true + end + end + + describe "when deploying a consumer to prod for the first time and the provider is not yet deployed" do + before do + td.create_pact_with_hierarchy("Foo", "1", "Bar") + .create_verification(provider_version: "2") + end + + let(:selectors) do + [ { pacticipant_name: "Foo", pacticipant_version_number: "1" } ] + end + + let(:options) do + { latest: true, tag: "prod" } + end + + it "does not allow the consumer to be deployed" do + expect(subject.deployment_status_summary.deployable?).to_not be true + end + end + + describe "when deploying an app that is both a consumer and a provider to prod for the first time and the downstream provider is not yet deployed" do + before do + td.create_pact_with_hierarchy("Foo", "1", "Bar") + .create_verification(provider_version: "2") + .use_consumer("Bar") + .use_consumer_version("2") + .create_provider("Baz") + .create_pact + end + + let(:selectors) do + [ { pacticipant_name: "Bar", pacticipant_version_number: "2" } ] + end + + let(:options) do + { latest: true, tag: "prod" } + end + + it "does not allow the app to be deployed" do + expect(subject.deployment_status_summary.deployable?).to_not be true + end + end + + describe "when deploying an app that is both a consumer and a provider to prod for the first time and the downstream provider has been deployed" do + before do + td.create_pact_with_hierarchy("Foo", "1", "Bar") + .create_verification(provider_version: "2") + .use_consumer("Bar") + .use_consumer_version("2") + .create_provider("Baz") + .create_pact + .create_verification(provider_version: "4", tag_names: "prod") + end + + let(:selectors) do + [ { pacticipant_name: "Bar", pacticipant_version_number: "2" } ] + end + + let(:options) do + { latest: true, tag: "prod" } + end + + it "allows the app to be deployed" do + expect(subject.deployment_status_summary.deployable?).to be true + end + end + end + end + end +end \ No newline at end of file diff --git a/spec/lib/pact_broker/matrix/repository_find_integrations_spec.rb b/spec/lib/pact_broker/matrix/repository_find_integrations_spec.rb deleted file mode 100644 index 84bfebfb7..000000000 --- a/spec/lib/pact_broker/matrix/repository_find_integrations_spec.rb +++ /dev/null @@ -1,51 +0,0 @@ -require 'pact_broker/matrix/repository' - -module PactBroker - module Matrix - describe Repository do - let(:td) { TestDataBuilder.new} - - describe "find_integrations" do - before do - td.create_pact_with_hierarchy("foo", "1", "bar") - .create_provider("baz") - .create_pact - .use_consumer("baz") - .create_consumer_version("3") - .create_provider("wiffle") - .create_pact - end - - subject { Repository.new.find_integrations(["foo"]).sort } - - context "with only one pacticipant name" do - it "returns all the integrations that the pacticipant with the given name has" do - expect(subject.first.consumer_name).to eq "foo" - expect(subject.first.provider_name).to eq "bar" - expect(subject.last.consumer_name).to eq "foo" - expect(subject.last.provider_name).to eq "baz" - expect(subject.size).to eq 2 - end - end - - context "with the names of two pacticipants that are integrated" do - subject { Repository.new.find_integrations(["foo", "bar"]).sort } - - it "returns only that integration" do - expect(subject.first.consumer_name).to eq "foo" - expect(subject.first.provider_name).to eq "bar" - expect(subject.size).to eq 1 - end - end - - context "with the names of two pacticipants that aren't integrated" do - subject { Repository.new.find_integrations(["foo", "wiffle"]).sort } - - it "returns an empty array" do - expect(subject).to eq [] - end - end - end - end - end -end diff --git a/spec/lib/pact_broker/matrix/repository_spec.rb b/spec/lib/pact_broker/matrix/repository_spec.rb index a90c49790..248019f29 100644 --- a/spec/lib/pact_broker/matrix/repository_spec.rb +++ b/spec/lib/pact_broker/matrix/repository_spec.rb @@ -991,6 +991,8 @@ def shorten_rows rows .create_pact .create_verification(provider_version: "10.0.0", tag_names: ["prod"]) .create_provider("baz") + .create_pact + .create_verification(provider_version: "9.0.0") .create_consumer_version("2.0.0") .create_pact .create_verification(provider_version: "20.0.0", tag_names: ["prod"]) @@ -998,12 +1000,17 @@ def shorten_rows rows let(:selectors) { [{ pacticipant_name: "foo", pacticipant_version_number: "1.0.0" }] } let(:options) { {latestby: "cvp", latest: true, tag: "prod"} } + let(:results) { Repository.new.find(selectors, options) } - subject { shorten_rows(Repository.new.find(selectors, options)) } + subject { shorten_rows(results) } it "only returns a row for the foo pact version that has been verified by the current production version of bar" do expect(subject).to eq ["foo1.0.0 bar10.0.0 n1"] end + + it "returns 2 integrations" do + expect(results.integrations.size).to eq 2 + end end end end diff --git a/spec/lib/pact_broker/matrix/service_spec.rb b/spec/lib/pact_broker/matrix/service_spec.rb index 572f77bac..80fc8067f 100644 --- a/spec/lib/pact_broker/matrix/service_spec.rb +++ b/spec/lib/pact_broker/matrix/service_spec.rb @@ -5,74 +5,6 @@ module Matrix describe Service do let(:td) { TestDataBuilder.new } - describe "find integration test" do - let(:selectors) do - [ { pacticipant_name: "foo" } ] - end - - let(:options) do - { latest: true, tag: "prod" } - end - - before do - td.create_pact_with_hierarchy("foo", "1", "bar") - .create_verification(provider_version: "2", tag_names: ["prod"]) - end - - subject { Service.find(selectors, options) } - - it "returns a QueryResultsWithDeploymentStatusSummary" do - expect(subject.rows).to be_a(Array) - expect(subject.selectors).to be selectors - expect(subject.options).to be options - expect(subject.resolved_selectors).to be_a(Array) - expect(subject.resolved_selectors.count).to eq 2 - expect(subject.deployment_status_summary).to be_a(DeploymentStatusSummary) - end - end - - describe "integration - when deploying a provider to prod for the first time and the consumer is not yet deployed" do - before do - td.create_pact_with_hierarchy("Foo", "1", "Bar") - .create_verification(provider_version: "2") - end - - let(:selectors) do - [ { pacticipant_name: "Bar", pacticipant_version_number: "2" } ] - end - - let(:options) do - { latest: true, tag: "prod" } - end - - subject { Service.find(selectors, options) } - - it "allows the provider to be deployed" do - expect(subject.deployment_status_summary.deployable?).to be true - end - end - - describe "integration - when deploying a consumer to prod for the first time and the provider is not yet deployed" do - before do - td.create_pact_with_hierarchy("Foo", "1", "Bar") - .create_verification(provider_version: "2") - end - - let(:selectors) do - [ { pacticipant_name: "Foo", pacticipant_version_number: "1" } ] - end - - let(:options) do - { latest: true, tag: "prod" } - end - - subject { Service.find(selectors, options) } - - it "does not allow the provider to be deployed" do - expect(subject.deployment_status_summary.deployable?).to_not be true - end - end - describe "validate_selectors" do subject { Service.validate_selectors(selectors) } @@ -85,21 +17,6 @@ module Matrix end end - context "when one or more of the selectors does not match any known version" do - before do - td.create_pacticipant("Foo") - .create_version("1") - .create_pacticipant("Bar") - .create_version("2") - end - - let(:selectors) { [{ pacticipant_name: "Foo", pacticipant_version_number: "1" }, { pacticipant_name: "Bar", pacticipant_version_number: "1" }] } - - it "returns error messages" do - expect(subject).to eq ["No pact or verification found for Bar version 1"] - end - end - context "when the pacticipant does not exist" do let(:selectors) { [{ pacticipant_name: "Foo", pacticipant_version_number: "1" }] } @@ -147,15 +64,6 @@ module Matrix expect(subject).to eq [] end end - - context "when there is not a version for the tag" do - - let(:selectors) { [{ pacticipant_name: "Foo", latest: true, tag: "wiffle" }, { pacticipant_name: "Bar", pacticipant_version_number: "2" }] } - - it "returns an error message" do - expect(subject).to eq ["No version of Foo found with tag wiffle"] - end - end end context "when the latest is used as well as a version" do