From 45c9a4cf102852fbb781080585b6c20e76a9ed12 Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Sat, 4 Nov 2017 11:57:57 +1100 Subject: [PATCH] feat(matrix): allow the 'groupby' to determine the 'latest' row to be specified via the api --- .../api/decorators/matrix_decorator.rb | 8 +- .../api/decorators/matrix_text_decorator.rb | 5 +- lib/pact_broker/matrix/parse_query.rb | 6 + lib/pact_broker/matrix/repository.rb | 40 ++- .../lib/pact_broker/matrix/repository_spec.rb | 290 ++++++++++++++++-- spec/support/shared_examples_for_responses.rb | 19 ++ 6 files changed, 326 insertions(+), 42 deletions(-) diff --git a/lib/pact_broker/api/decorators/matrix_decorator.rb b/lib/pact_broker/api/decorators/matrix_decorator.rb index e402f17df..672b250e9 100644 --- a/lib/pact_broker/api/decorators/matrix_decorator.rb +++ b/lib/pact_broker/api/decorators/matrix_decorator.rb @@ -25,10 +25,6 @@ def to_hash(options) } end - private - - attr_reader :lines - def deployable return nil if lines.any?{ |line| line[:success].nil? } lines.any? && lines.all?{ |line| line[:success] } @@ -43,6 +39,10 @@ def reason end end + private + + attr_reader :lines + def matrix(lines, base_url) provider = nil consumer = nil diff --git a/lib/pact_broker/api/decorators/matrix_text_decorator.rb b/lib/pact_broker/api/decorators/matrix_text_decorator.rb index 27068e8de..80f6569f3 100644 --- a/lib/pact_broker/api/decorators/matrix_text_decorator.rb +++ b/lib/pact_broker/api/decorators/matrix_text_decorator.rb @@ -1,5 +1,7 @@ require 'ostruct' require 'pact_broker/api/pact_broker_urls' +require 'pact_broker/api/decorators/matrix_decorator' + require 'table_print' module PactBroker @@ -13,11 +15,12 @@ def initialize(lines) end def to_text(options) + json_decorator = PactBroker::Api::Decorators::MatrixDecorator.new(lines) data = lines.collect do | line | Line.new(line[:consumer_name], line[:consumer_version_number], line[:provider_name], line[:provider_version_number], line[:success]) end printer = TablePrint::Printer.new(data) - printer.table_print + "\n" + printer.table_print + "\n\nDeployable: #{json_decorator.deployable.inspect}\nReason: #{json_decorator.reason}\n" end private diff --git a/lib/pact_broker/matrix/parse_query.rb b/lib/pact_broker/matrix/parse_query.rb index 3f9cb29eb..698fd0f64 100644 --- a/lib/pact_broker/matrix/parse_query.rb +++ b/lib/pact_broker/matrix/parse_query.rb @@ -22,6 +22,12 @@ def self.call query if params.key?('success') && params['success'].is_a?(String) options[:success] = [params['success'] == '' ? nil : params['success'] == 'true'] end + if params.key?('scope') + options[:scope] = params['scope'] + end + if params.key?('groupby') + options[:groupby] = params['groupby'] + end return selectors, options end end diff --git a/lib/pact_broker/matrix/repository.rb b/lib/pact_broker/matrix/repository.rb index 1bee4b10f..848aab83f 100644 --- a/lib/pact_broker/matrix/repository.rb +++ b/lib/pact_broker/matrix/repository.rb @@ -6,15 +6,19 @@ class Repository include PactBroker::Repositories::Helpers include PactBroker::Repositories + # TODO SORT BY MOST RECENT FIRST + # TODO move latest verification logic in to database + + GROUP_BY_PROVIDER_VERSION_NUMBER = [:consumer_name, :consumer_version_number, :provider_name, :provider_version_number] + GROUP_BY_PROVIDER = [:consumer_name, :consumer_version_number, :provider_name] + GROUP_BY_PACT = [:consumer_name, :provider_name] + # Return the latest matrix row (pact/verification) for each consumer_version_number/provider_version_number def find selectors, options = {} # The group with the nil provider_version_numbers will be the results of the left outer join # that don't have verifications, so we need to include them all. lines = find_all(selectors) - .group_by{|line| [line[:consumer_name], line[:consumer_version_number], line[:provider_name], line[:provider_version_number]]} - .values - .collect{ | lines | lines.first[:provider_version_number].nil? ? lines : lines.last } - .flatten + lines = apply_scope(options, selectors, lines) if options.key?(:success) lines = lines.select{ |l| options[:success].include?(l[:success]) } @@ -23,6 +27,26 @@ def find selectors, options = {} lines end + def all_versions_specified? selectors + selectors.all?{ |s| s[:pacticipant_version_number] } + end + + def apply_scope options, selectors, lines + return lines unless options[:latestby] + + group_by_columns = case options[:latestby] + when 'cvp' then GROUP_BY_PROVIDER + when 'cp' then GROUP_BY_PACT + else + GROUP_BY_PROVIDER_VERSION_NUMBER + end + + lines.group_by{|line| group_by_columns.collect{|key| line[key] }} + .values + .collect{ | lines | lines.first[:provider_version_number].nil? ? lines : lines.last } + .flatten + end + def find_for_consumer_and_provider pacticipant_1_name, pacticipant_2_name selectors = [{ pacticipant_name: pacticipant_1_name }, { pacticipant_name: pacticipant_2_name }] find_all(selectors) @@ -30,7 +54,12 @@ def find_for_consumer_and_provider pacticipant_1_name, pacticipant_2_name end def find_compatible_pacticipant_versions selectors - find(selectors).select{ |line | line[:success] } + find(selectors) + .group_by{|line| GROUP_BY_PROVIDER_VERSION_NUMBER.collect{|key| line[key] }} + .values + .collect{ | lines | lines.first[:provider_version_number].nil? ? lines : lines.last } + .flatten + .select{|line| line[:success] } end ## @@ -60,7 +89,6 @@ def find_all selectors def look_up_versions_for_tags(selectors) selectors.collect do | selector | # resource validation currently stops tag being specified without latest=true - if selector[:tag] && selector[:latest] version = version_repository.find_by_pacticpant_name_and_latest_tag(selector[:pacticipant_name], selector[:tag]) # validation in resource should ensure we always have a version diff --git a/spec/lib/pact_broker/matrix/repository_spec.rb b/spec/lib/pact_broker/matrix/repository_spec.rb index f33f23886..416917345 100644 --- a/spec/lib/pact_broker/matrix/repository_spec.rb +++ b/spec/lib/pact_broker/matrix/repository_spec.rb @@ -11,7 +11,234 @@ def build_selectors(hash) end end + def shorten_row row + "#{row[:consumer_name]}#{row[:consumer_version_number]} #{row[:provider_name]}#{row[:provider_version_number] || '?'} n#{row[:number] || '?'}" + end + + def shorten_rows rows + rows.collect{ |r| shorten_row(r) } + end + + describe "find" do + before do + # A1 - B1 + # A1 - B1 r2 + # A1 - C1 + # A2 - B? + # A2 - C2 + td.create_pact_with_hierarchy("A", "1", "B") + .create_verification(provider_version: '1', success: false) + .create_verification(provider_version: '1', number: 2, success: true) + .create_verification(provider_version: '2', number: 3, success: true) + .create_provider("C") + .create_pact + .create_verification(provider_version: '1') + .create_consumer_version("2") + .create_pact + .create_verification(provider_version: '3') + .use_provider("B") + .create_pact + end + + subject { shorten_rows(Repository.new.find(selectors, options)) } + let(:options) { { latestby: latestby } } + let(:latestby) { nil } + let(:a1_b1_n1) { "A1 B1 n1" } + let(:a1_b1_n2) { "A1 B1 n2" } + let(:a1_b2_n3) { "A1 B2 n3" } + let(:a1_c1_n1) { "A1 C1 n1"} + + context "when just the consumer name is specified" do + let(:selectors) { build_selectors('A' => nil) } + + context "when no latestby is specified" do + it "returns all rows" do + expect(subject).to include a1_b1_n1 + expect(subject).to include a1_b1_n2 + expect(subject).to include a1_c1_n1 + expect(subject.size).to eq 6 + end + end + + context "when latestby=cvpv" do + let(:latestby) { 'cvpv' } + + it "returns the latest rows per consumer version/provider version" do + expect(subject).to_not include a1_b1_n1 + expect(subject).to include a1_b1_n2 + expect(subject).to include a1_c1_n1 + expect(subject.size).to eq 5 + end + end + + context "when latestby=cvp" do + let(:latestby) { 'cvp' } + + it "returns the latest row for each provider for each consumer version" do + expect(subject).to_not include a1_b1_n1 + expect(subject).to_not include a1_b1_n2 + expect(subject).to include a1_b2_n3 + expect(subject).to include a1_c1_n1 + expect(subject.size).to eq 4 + end + end + + context "when latestby=cp", pending: true do + let(:latestby) { 'cp' } + + it "returns the latest rows per consumer/provider" do + expect(subject).to include "A2 C3 n1" + expect(subject).to include "A2 B? n?" + expect(subject).to include a1_c1_n1 + expect(subject).to_not include a1_b2_n3 + expect(subject.size).to eq 2 + end + end + end + + context "when the consumer name/version are specified" do + let(:selectors) { build_selectors('A' => '1') } + + context "when no latestby is specified" do + it "returns all the rows for the consumer version" do + expect(subject.size).to eq 4 + end + end + + context "when latestby=cvpv" do + let(:latestby) { 'cvpv' } + + it "returns the latest verification for each provider version for the specified consumer version" do + expect(subject).to_not include a1_b1_n1 + expect(subject).to include a1_b1_n2 + expect(subject).to include a1_c1_n1 + expect(subject.size).to eq 3 + end + end + + context "when latestby=cvp" do + let(:latestby) { 'cvp' } + + it "returns the latest verifications for each provider for the specified consumer version" do + expect(subject).to_not include a1_b1_n1 + expect(subject).to_not include a1_b1_n2 + expect(subject).to include a1_b2_n3 + expect(subject).to include a1_c1_n1 + expect(subject.size).to eq 2 + end + end + + context "when latestby=cp" do + let(:latestby) { 'cp' } + + it "returns the same as latestby=cvp" do + expect(subject).to_not include a1_b1_n1 + expect(subject).to_not include a1_b1_n2 + expect(subject).to include a1_b2_n3 + expect(subject).to include a1_c1_n1 + expect(subject.size).to eq 2 + end + end + end + + context "when the consumer name/version and the provider name are specified" do + let(:selectors) { build_selectors('A' => '1', 'B' => nil) } + + context "when no latestby is specified" do + it "returns all the rows for the given consumer version and given provider" do + expect(subject).to include a1_b1_n1 + expect(subject).to include a1_b1_n2 + expect(subject).to include a1_b2_n3 + expect(subject).to_not include a1_c1_n1 + expect(subject.size).to eq 3 + end + end + + context "when latestby=cvpv" do + let(:latestby) { 'cvpv' } + + it "returns the latest verification for each provider version for the given consumer version" do + expect(subject).to_not include a1_b1_n1 + expect(subject).to include a1_b1_n2 + expect(subject).to include a1_b2_n3 + expect(subject).to_not include a1_c1_n1 + expect(subject.size).to eq 2 + end + end + + context "when latestby=cvp" do + let(:latestby) { 'cvp' } + + it "returns the latest verification for the given provider for the given consumer version" do + expect(subject).to_not include a1_b1_n1 + expect(subject).to_not include a1_b1_n2 + expect(subject).to include a1_b2_n3 + expect(subject).to_not include a1_c1_n1 + expect(subject.size).to eq 1 + end + end + + context "when latestby=cp" do + let(:latestby) { 'cp' } + + it "returns the same as latestby=cvp" do + expect(subject).to_not include a1_b1_n1 + expect(subject).to_not include a1_b1_n2 + expect(subject).to include a1_b2_n3 + expect(subject).to_not include a1_c1_n1 + expect(subject.size).to eq 1 + end + end + end + + context "when the consumer name/version and provider name/version are specified" do + let(:selectors) { build_selectors('A' => '1', 'B' => '1') } + + context "when no latestby is specified" do + it "returns all the rows for the given consumer/version and given provider/version" do + expect(subject).to include a1_b1_n1 + expect(subject).to include a1_b1_n2 + expect(subject).to_not include a1_b2_n3 + expect(subject).to_not include a1_c1_n1 + expect(subject.size).to eq 2 + end + end + + context "when latestby=cvpv" do + let(:latestby) { 'cvpv' } + + it "returns the latest verification for the given provider version for the given consumer version" do + expect(subject).to include a1_b1_n2 + expect(subject.size).to eq 1 + end + end + + context "when latestby=cvp" do + let(:latestby) { 'cvp' } + + it "returns the same as latestby=cvpv" do + expect(subject).to include a1_b1_n2 + expect(subject.size).to eq 1 + end + end + + context "when latestby=cp" do + let(:latestby) { 'cp' } + + it "returns the same as latestby=cvp" do + expect(subject).to include a1_b1_n2 + expect(subject.size).to eq 1 + end + end + end + end + describe "find" do + let(:options) { { scope: scope} } + let(:scope) { 'latest' } + + subject { Repository.new.find(selectors, options) } + context "when the provider version resource exists but there is no verification for that version" do before do # A/1.2.3 -> B @@ -24,8 +251,7 @@ def build_selectors(hash) .create_version("3.0.0") .create_pact end - - subject { Repository.new.find build_selectors("A" => "1.2.3", "B" => "2.0.0", "C" => "3.0.0") } + let(:selectors) { build_selectors("A" => "1.2.3", "B" => "2.0.0", "C" => "3.0.0") } it "returns a row for each pact" do expect(subject.size).to eq 2 @@ -46,8 +272,6 @@ def build_selectors(hash) context "when only 2 version selectors are specified" do let(:selectors) { build_selectors("A" => "1.2.3", "B" => "2.0.0") } - subject { Repository.new.find(selectors) } - it "only returns 1 row" do expect(subject.size).to eq 1 end @@ -66,7 +290,7 @@ def build_selectors(hash) .create_verification(provider_version: "4.5.6") end - subject { Repository.new.find build_selectors("A" => "1.2.3") } + let(:selectors) { build_selectors("A" => "1.2.3") } it "returns a row for each verification for that version" do expect(subject.size).to eq 2 @@ -84,7 +308,7 @@ def build_selectors(hash) .create_pact_with_hierarchy("X", "1.2.3", "Y") end - subject { Repository.new.find build_selectors("A" => nil) } + let(:selectors) { build_selectors("A" => nil) } it "returns a row for each verification for the pacticipant" do expect(subject.collect{|r| r[:consumer_name]}.uniq).to eq ["A"] @@ -105,7 +329,7 @@ def build_selectors(hash) .create_verification(provider_version: "6.7.8", number: 2) end - subject { Repository.new.find build_selectors("B" => "4.5.6") } + let(:selectors) { build_selectors("B" => "4.5.6") } it "returns a row for each verification for that version" do expect(subject.size).to eq 2 @@ -125,7 +349,11 @@ def build_selectors(hash) .create_verification(provider_version: "6.7.8", number: 2) end - subject { Repository.new.find build_selectors("B" => nil) } + before do + options.delete(:scope) + end + + let(:selectors) { build_selectors("B" => nil) } it "returns a row for each verification for that version" do expect(subject.size).to eq 3 @@ -148,7 +376,7 @@ def build_selectors(hash) .create_verification(provider_version: '1', success: false) end - subject { Repository.new.find build_selectors("B" => "1") } + let(:selectors) { build_selectors("B" => "1") } it "returns rows where the pacticipant is the consumer and rows where the pacticipant is the provider" do # A/1 and B/1 @@ -170,10 +398,10 @@ def build_selectors(hash) let(:selectors) { build_selectors("A" => nil, "B" => nil) } - subject { Repository.new.find(selectors, options) } - context "when the success option is not set" do - let(:options) { { } } + before do + options.delete(:success) + end it "returns all rows specified by the selectors" do expect(subject.size).to eq 3 @@ -181,7 +409,9 @@ def build_selectors(hash) end context "when the success option is true" do - let(:options) { { success: [true] } } + before do + options[:success] = [true] + end it "only includes successes" do expect(subject.first[:provider_version_number]).to eq "1.0.0" @@ -190,7 +420,9 @@ def build_selectors(hash) end context "when the success option is false" do - let(:options) { { success: [false] } } + before do + options[:success] = [false] + end it "only includes failures" do expect(subject.first[:provider_version_number]).to eq "2.0.0" @@ -199,7 +431,9 @@ def build_selectors(hash) end context "when the success option is nil" do - let(:options) { { success: [nil] } } + before do + options[:success] = [nil] + end it "only includes unverified rows" do expect(subject.first[:provider_version_number]).to eq nil @@ -208,7 +442,9 @@ def build_selectors(hash) end context "when multiple success options are specified" do - let(:options) { { success: [false, nil] } } + before do + options[:success] = [false, nil] + end it "returns all matching rows" do # postgres orders differently, and ruby array sort blows up with a nil string @@ -220,7 +456,7 @@ def build_selectors(hash) end end - context "when the latest tag is specified for a provider instead of a version" do + context "when the latest tag is specified for a provider" do before do td.create_pact_with_hierarchy("A", "1.2.3", "B") .create_verification(provider_version: "1.0.0") @@ -239,8 +475,6 @@ def build_selectors(hash) ] end - subject { Repository.new.find(selectors) } - it "returns the row for the version " do expect(subject.first).to include provider_version_number: "2.0.0" expect(subject.size).to eq 1 @@ -264,8 +498,6 @@ def build_selectors(hash) ] end - subject { Repository.new.find(selectors) } - it "returns the row for the version " do expect(subject.first).to include provider_version_number: "3.0.0" expect(subject.size).to eq 1 @@ -286,8 +518,6 @@ def build_selectors(hash) ] end - subject { Repository.new.find(selectors) } - it "returns no data - this may be confusing. Might need to re-think this logic." do expect(subject.size).to eq 0 end @@ -324,6 +554,8 @@ def build_selectors(hash) describe "#find_compatible_pacticipant_versions" do let(:td) { TestDataBuilder.new } + # subject { Repository.new.find_compatible_pacticipant_versions(selectors) } + subject { Repository.new.find(selectors, success: [true], scope: 'latest')} context "when compatible versions can be found" do before do @@ -345,8 +577,6 @@ def build_selectors(hash) let(:selectors){ build_selectors("A" => "1", "B" => "2", "C" => "2") } - subject { Repository.new.find_compatible_pacticipant_versions(selectors) } - it "returns matrix lines for each compatible version pair (A/1-B/2, B/2-C/2)" do expect(subject.first[:consumer_name]).to eq "A" expect(subject.first[:consumer_version_number]).to eq "1" @@ -368,11 +598,10 @@ def build_selectors(hash) context "when one or more pacticipants does not have a version specified" do let(:selectors){ build_selectors("A" => "1", "B" => "2", "C" => nil) } - subject { Repository.new.find_compatible_pacticipant_versions(selectors) } it "returns all the rows for that pacticipant" do - expect(subject[1]).to include(provider_name: "C", provider_version_number: "2") - expect(subject[2]).to include(provider_name: "C", provider_version_number: "3") + expect(subject).to include_hash_matching(provider_name: "C", provider_version_number: "2") + expect(subject).to include_hash_matching(provider_name: "C", provider_version_number: "3") expect(subject.size).to eq 3 end end @@ -380,8 +609,6 @@ def build_selectors(hash) context "none of the pacticipants have a version specified" do let(:selectors){ build_selectors("A" => nil, "B" => nil, "C" => nil) } - subject { Repository.new.find_compatible_pacticipant_versions(selectors) } - it "returns all the rows" do expect(subject.size).to eq 5 end @@ -405,7 +632,7 @@ def build_selectors(hash) end end - context "when there is more than one compatible verison pair and the last one is a failure" do + context "when there is more than one verification and the last one is a failure" do before do td.create_pact_with_hierarchy("X", "1", "Y") .create_verification(provider_version: "1") @@ -417,6 +644,7 @@ def build_selectors(hash) subject { Repository.new.find_compatible_pacticipant_versions(selectors) } it "does not return a matrix line" do + puts subject expect(subject.size).to eq 0 end end diff --git a/spec/support/shared_examples_for_responses.rb b/spec/support/shared_examples_for_responses.rb index 364e600e0..82461783f 100644 --- a/spec/support/shared_examples_for_responses.rb +++ b/spec/support/shared_examples_for_responses.rb @@ -42,11 +42,30 @@ end end +RSpec::Matchers.define :include_hashes_matching do |expected_array_of_hashes| + match do |array_of_hashes| + expected_array_of_hashes.each do | expected | + expect(array_of_hashes).to include_hash_matching(expected) + end + + expect(array_of_hashes.size).to eq expected_array_of_hashes.size + end + + def slice actual, keys + keys.each_with_object({}) { |k, hash| hash[k] = actual[k] if actual.has_key?(k) } + end +end + RSpec::Matchers.define :include_hash_matching do |expected| match do |array_of_hashes| + @array_of_hashes = array_of_hashes array_of_hashes.any? { |actual| slice(actual, expected.keys) == expected } end + failure_message do + "expected #{@array_of_hashes.inspect} to include #{expected.inspect}" + end + def slice actual, keys keys.each_with_object({}) { |k, hash| hash[k] = actual[k] if actual.has_key?(k) } end