From 32bbe1c3ba3c7369e7e2f92dbfe9de29280d373d Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Thu, 17 Dec 2020 21:14:16 +1100 Subject: [PATCH] feat: include the consumer version selectors in the metadata of the 'pact for verification' URL --- .../decorators/verifiable_pact_decorator.rb | 4 +- lib/pact_broker/api/pact_broker_urls.rb | 6 +- .../resources/metadata_resource_methods.rb | 3 +- lib/pact_broker/pacts/metadata.rb | 59 +++++++++++++-- lib/pact_broker/pacts/repository.rb | 2 +- lib/pact_broker/pacts/selector.rb | 9 ++- .../test/http_test_data_builder.rb | 20 +++-- script/reproduce-issue.rb | 5 +- .../verifiable_pact_decorator_spec.rb | 12 ++- .../pact_broker/api/pact_broker_urls_spec.rb | 8 +- spec/lib/pact_broker/pacts/metadata_spec.rb | 73 +++++++++++++++++++ .../repository_find_for_verification_spec.rb | 11 ++- 12 files changed, 173 insertions(+), 39 deletions(-) create mode 100644 spec/lib/pact_broker/pacts/metadata_spec.rb diff --git a/lib/pact_broker/api/decorators/verifiable_pact_decorator.rb b/lib/pact_broker/api/decorators/verifiable_pact_decorator.rb index 013956a52..9e46823bc 100644 --- a/lib/pact_broker/api/decorators/verifiable_pact_decorator.rb +++ b/lib/pact_broker/api/decorators/verifiable_pact_decorator.rb @@ -1,11 +1,13 @@ require_relative 'base_decorator' require 'pact_broker/api/pact_broker_urls' require 'pact_broker/pacts/build_verifiable_pact_notices' +require 'pact_broker/pacts/metadata' module PactBroker module Api module Decorators class VerifiablePactDecorator < BaseDecorator + include PactBroker::Pacts::Metadata property :shortDescription, getter: -> (context) { PactBroker::Pacts::VerifiablePactMessages.new(context[:represented], nil).pact_version_short_description } @@ -27,7 +29,7 @@ def notices(user_options) end link :self do | user_options | - metadata = represented.wip ? { wip: true } : nil + metadata = build_metadata_for_pact_for_verification(represented) { href: pact_version_url_with_metadata(represented, metadata, user_options[:base_url]), name: represented.name diff --git a/lib/pact_broker/api/pact_broker_urls.rb b/lib/pact_broker/api/pact_broker_urls.rb index 0cabda3cd..d7e8a7460 100644 --- a/lib/pact_broker/api/pact_broker_urls.rb +++ b/lib/pact_broker/api/pact_broker_urls.rb @@ -1,6 +1,8 @@ require 'erb' require 'pact_broker/pacts/metadata' require 'pact_broker/logging' +require 'base64' +require 'rack' module PactBroker module Api @@ -77,9 +79,7 @@ def encode_metadata(metadata) def decode_pact_metadata(metadata) if metadata && metadata != '' begin - Rack::Utils.parse_nested_query(Base64.strict_decode64(metadata)).each_with_object({}) do | (k, v), new_hash | - new_hash[k.to_sym] = v - end + Rack::Utils.parse_nested_query(Base64.strict_decode64(metadata)) rescue StandardError => e logger.warn("Exception parsing webhook metadata: #{metadata}", e) {} diff --git a/lib/pact_broker/api/resources/metadata_resource_methods.rb b/lib/pact_broker/api/resources/metadata_resource_methods.rb index 4ff1031dc..3ddb0f5e3 100644 --- a/lib/pact_broker/api/resources/metadata_resource_methods.rb +++ b/lib/pact_broker/api/resources/metadata_resource_methods.rb @@ -1,4 +1,5 @@ require 'pact_broker/hash_refinements' +require 'pact_broker/pacts/metadata' module PactBroker module Api @@ -15,7 +16,7 @@ def maybe_params_with_consumer_version_number end def metadata - @metadata ||= PactBrokerUrls.decode_pact_metadata(identifier_from_path[:metadata]) + @metadata ||= PactBroker::Pacts::Metadata.parse_metadata(PactBrokerUrls.decode_pact_metadata(identifier_from_path[:metadata])) end end end diff --git a/lib/pact_broker/pacts/metadata.rb b/lib/pact_broker/pacts/metadata.rb index a2c139fe0..652ba366f 100644 --- a/lib/pact_broker/pacts/metadata.rb +++ b/lib/pact_broker/pacts/metadata.rb @@ -3,6 +3,15 @@ module Pacts module Metadata extend self + MAPPINGS = [ + [:consumer_version_tags, "cvt"], + [:consumer_version_number, "cvn"], + [:wip, "w"], + [:consumer_version_selectors, "s"], + [:tag, "t"], + [:latest, "l"] + ] + # When verifying a pact at /.../latest/TAG, this stores the # tag and the current consumer version number in the # metadata parameter of the URL for publishing the verification results. @@ -11,12 +20,12 @@ module Metadata def build_metadata_for_latest_pact(pact, selection_parameters) if selection_parameters[:tag] { - consumer_version_tags: [selection_parameters[:tag]], - consumer_version_number: pact.consumer_version_number + "cvt" => [selection_parameters[:tag]], + "cvn" => pact.consumer_version_number } else { - consumer_version_number: pact.consumer_version_number + "cvn" => pact.consumer_version_number } end end @@ -28,12 +37,50 @@ def build_metadata_for_latest_pact(pact, selection_parameters) # go back to the correct consumer version number (eg for git statuses) def build_metadata_for_webhook_triggered_by_pact_publication(pact) metadata = { - consumer_version_number: pact.consumer_version_number, - consumer_version_tags: pact.consumer_version_tag_names + "cvn" => pact.consumer_version_number, + "cvt" => pact.consumer_version_tag_names } - metadata[:wip] = "true" + metadata["w"] = "true" metadata end + + def build_metadata_for_pact_for_verification(verifiable_pact) + # todo put in tags + if verifiable_pact.wip + { + "w" => true + } + else + { + "s" => verifiable_pact.selectors.collect do | selector | + { + "t" => selector.tag, + "l" => selector.latest, + "cvn" => selector.consumer_version.number + }.compact + end + } + end + end + + def parse_metadata(metadata) + parse_object(metadata) + end + + def parse_object(object) + case object + when Hash then parse_hash(object) + when Array then object.collect{|i| parse_object(i) } + else object + end + end + + def parse_hash(hash) + hash.each_with_object({}) do | (key, value), new_hash | + long_key = MAPPINGS.find{ |mapping| mapping.last == key }&.first + new_hash[long_key || key] = parse_object(value) + end + end end end end diff --git a/lib/pact_broker/pacts/repository.rb b/lib/pact_broker/pacts/repository.rb index faf32f1ee..71d1eb19f 100644 --- a/lib/pact_broker/pacts/repository.rb +++ b/lib/pact_broker/pacts/repository.rb @@ -168,7 +168,7 @@ def find_all_pact_versions_for_provider_with_consumer_version_tags provider_name .values .collect do | pact_publications | latest_pact_publication = pact_publications.sort_by{ |p| p.values.fetch(:consumer_version_order) }.last - SelectedPact.new(latest_pact_publication.to_domain, Selectors.new(selector)) + SelectedPact.new(latest_pact_publication.to_domain, Selectors.new(selector).resolve(latest_pact_publication.consumer_version)) end end diff --git a/lib/pact_broker/pacts/selector.rb b/lib/pact_broker/pacts/selector.rb index da28e1115..feb8a7af0 100644 --- a/lib/pact_broker/pacts/selector.rb +++ b/lib/pact_broker/pacts/selector.rb @@ -134,11 +134,12 @@ def latest? end class ResolvedSelector < Selector - attr_reader :consumer_version - def initialize(options = {}, consumer_version) - super(options) - @consumer_version = consumer_version + super(options.merge(consumer_version: consumer_version)) + end + + def consumer_version + self[:consumer_version] end def == other diff --git a/lib/pact_broker/test/http_test_data_builder.rb b/lib/pact_broker/test/http_test_data_builder.rb index 420a5792f..8f5ce9bd8 100644 --- a/lib/pact_broker/test/http_test_data_builder.rb +++ b/lib/pact_broker/test/http_test_data_builder.rb @@ -88,14 +88,18 @@ def get_pacts_for_verification(provider: last_provider_name, provider_version_ta end def print_pacts_for_verification - pacts = @pacts_for_verification_response.body["_embedded"]["pacts"] - puts "Pacts for verification (#{pacts.count}):" - pacts.each do | pact | - puts({ - "url" => pact["_links"]["self"]["href"], - "wip" => pact["verificationProperties"]["wip"], - "pending" => pact["verificationProperties"]["pending"] - }.to_yaml) + pacts = @pacts_for_verification_response.body.dig("_embedded", "pacts") + if pacts + puts "Pacts for verification (#{pacts.count}):" + pacts.each do | pact | + puts({ + "url" => pact["_links"]["self"]["href"], + "wip" => pact["verificationProperties"]["wip"], + "pending" => pact["verificationProperties"]["pending"] + }.to_yaml) + end + else + puts @pacts_for_verification_response.body end self end diff --git a/script/reproduce-issue.rb b/script/reproduce-issue.rb index 23ad6c112..997d9e64d 100755 --- a/script/reproduce-issue.rb +++ b/script/reproduce-issue.rb @@ -10,9 +10,12 @@ td = PactBroker::Test::HttpTestDataBuilder.new(base_url, { }) td.delete_integration(consumer: "MyConsumer", provider: "MyProvider") .publish_pact(consumer: "MyConsumer", consumer_version: "1", provider: "MyProvider", content_id: "111", tag: "main") + .publish_pact(consumer: "MyConsumer", consumer_version: "2", provider: "MyProvider", content_id: "222", tag: "main") + .publish_pact(consumer: "MyConsumer", consumer_version: "3", provider: "MyProvider", content_id: "111", tag: "feat/a") .get_pacts_for_verification( provider_version_tag: "main", - consumer_version_selectors: [{ tag: "main", latest: true }]) + consumer_version_selectors: [{ tag: "main" }, { tag: "feat/a", latest: true }]) + .verify_pact(success: true, provider_version_tag: "main", provider_version: "2" ) rescue StandardError => e diff --git a/spec/lib/pact_broker/api/decorators/verifiable_pact_decorator_spec.rb b/spec/lib/pact_broker/api/decorators/verifiable_pact_decorator_spec.rb index f8e11cab9..0fa48169d 100644 --- a/spec/lib/pact_broker/api/decorators/verifiable_pact_decorator_spec.rb +++ b/spec/lib/pact_broker/api/decorators/verifiable_pact_decorator_spec.rb @@ -8,6 +8,7 @@ module Decorators allow_any_instance_of(PactBroker::Api::PactBrokerUrls).to receive(:pact_version_url_with_metadata).and_return('http://pact') allow(PactBroker::Pacts::BuildVerifiablePactNotices).to receive(:call).and_return(notices) allow_any_instance_of(PactBroker::Pacts::VerifiablePactMessages).to receive(:pact_version_short_description).and_return('short desc') + allow_any_instance_of(VerifiablePactDecorator).to receive(:build_metadata_for_pact_for_verification).and_return(metadata) end let(:pending_reason) { "the pending reason" } @@ -45,13 +46,15 @@ module Decorators name: "name", provider_name: "Bar", pending_provider_tags: pending_provider_tags, - consumer_tags: consumer_tags) + consumer_tags: consumer_tags, + selectors: PactBroker::Pacts::Selectors.new([PactBroker::Pacts::ResolvedSelector.new({ latest: true }, double('version', number: "2", id: 1))])) end let(:pending_provider_tags) { %w[dev] } let(:consumer_tags) { %w[dev] } let(:options) { { user_options: { base_url: 'http://example.org', include_pending_status: include_pending_status } } } let(:include_pending_status) { true } let(:wip){ false } + let(:metadata) { double('metadata') } let(:json) { decorator.to_json(options) } subject { JSON.parse(json) } @@ -61,7 +64,7 @@ module Decorators end it "creates the pact version url" do - expect(decorator).to receive(:pact_version_url_with_metadata).with(pact, nil, 'http://example.org') + expect(decorator).to receive(:pact_version_url_with_metadata).with(pact, metadata, 'http://example.org') subject end @@ -84,11 +87,6 @@ module Decorators it "includes the wip flag" do expect(subject['verificationProperties']['wip']).to be true end - - it "includes it in the metadata" do - expect(decorator).to receive(:pact_version_url_with_metadata).with(pact, { wip: true }, 'http://example.org') - subject - end end end end diff --git a/spec/lib/pact_broker/api/pact_broker_urls_spec.rb b/spec/lib/pact_broker/api/pact_broker_urls_spec.rb index 541b18fc5..b1feab59c 100644 --- a/spec/lib/pact_broker/api/pact_broker_urls_spec.rb +++ b/spec/lib/pact_broker/api/pact_broker_urls_spec.rb @@ -107,13 +107,9 @@ module Api end describe "webhook metadata" do - let(:expected_metadata) do - { consumer_version_number: "123/456", consumer_version_tags: %w[dev], wip: "true" } - end - it "builds the webhook metadata" do - encoded_metadata = PactBrokerUrls.encode_metadata(PactBrokerUrls.build_metadata_for_webhook_triggered_by_pact_publication(pact)) - expect(PactBrokerUrls.decode_pact_metadata(encoded_metadata)).to eq (expected_metadata) + encoded_metadata = PactBrokerUrls.encode_metadata("some" => "metadata") + expect(PactBrokerUrls.decode_pact_metadata(encoded_metadata)).to eq "some" => "metadata" end end diff --git a/spec/lib/pact_broker/pacts/metadata_spec.rb b/spec/lib/pact_broker/pacts/metadata_spec.rb new file mode 100644 index 000000000..95f848c39 --- /dev/null +++ b/spec/lib/pact_broker/pacts/metadata_spec.rb @@ -0,0 +1,73 @@ +require 'pact_broker/pacts/metadata' + +module PactBroker + module Pacts + module Metadata + describe "#build_metadata_for_pact_for_verification" do + let(:selectors) do + Selectors.new([ResolvedSelector.new({ latest: true, consumer: "consumer", tag: "tag" }, consumer_version)]) + end + let(:consumer_version) { double('version', number: "2") } + let(:verifiable_pact) { double('PactBroker::Pacts::VerifiablePact', wip: wip, selectors: selectors) } + let(:wip) { false } + + subject { Metadata.build_metadata_for_pact_for_verification(verifiable_pact) } + + it "builds the metadata with the resolved selectors" do + expect(subject).to eq({ + "s" => [ + { + "l" => true, + "t" => "tag", + "cvn" => "2" + } + ] + }) + end + + context "when wip is true" do + let(:wip) { true } + + it { is_expected.to eq "w" => true } + + end + end + + describe "parse_metadata" do + let(:incoming_metadata) do + { + "cvn" => "2", + "cvt" => ["tag"], + "w" => true, + "s" => [ + { + "l" => true, + "t" => "tag", + "cvn" => "2" + } + ] + } + end + + let(:parsed_metadata) do + { + :consumer_version_number => "2", + :consumer_version_tags => ["tag"], + :wip => true, + :consumer_version_selectors => [ + { + :latest => true, + :tag => "tag", + :consumer_version_number => "2" + } + ] + } + end + + it "expands the key names" do + expect(Metadata.parse_metadata(incoming_metadata)).to eq parsed_metadata + end + end + end + end +end diff --git a/spec/lib/pact_broker/pacts/repository_find_for_verification_spec.rb b/spec/lib/pact_broker/pacts/repository_find_for_verification_spec.rb index ba4ef5908..7b66e2fc1 100644 --- a/spec/lib/pact_broker/pacts/repository_find_for_verification_spec.rb +++ b/spec/lib/pact_broker/pacts/repository_find_for_verification_spec.rb @@ -145,14 +145,23 @@ def find_by_consumer_name_and_consumer_version_number(consumer_name, consumer_ve .create_consumer_version_tag("prod") .create_pact_with_hierarchy("Foo", "3", "Bar") .create_consumer_version_tag("prod") + .create_consumer_version("4") + .create_consumer_version_tag("prod") + .republish_same_pact end + let(:consumer_version_selectors) do Selectors.new(Selector.all_for_tag_and_consumer('prod', 'Foo')) end it "returns all the pacts with that tag for that consumer" do expect(subject.size).to eq 3 - expect(find_by_consumer_version_number("foo-latest-prod-version").selectors).to eq [Selector.all_for_tag_and_consumer('prod', 'Foo')] + expect(find_by_consumer_version_number("foo-latest-prod-version").selectors).to eq [Selector.all_for_tag_and_consumer('prod', 'Foo').resolve(PactBroker::Domain::Version.for("Foo", "foo-latest-prod-version"))] + end + + it "uses the latest consumer verison number as the resolved version when the same pact content is selected multiple times" do + expect(find_by_consumer_version_number("3")).to be nil + expect(find_by_consumer_version_number("4").selectors).to eq [Selector.all_for_tag_and_consumer('prod', 'Foo').resolve(PactBroker::Domain::Version.for("Foo", "4"))] end end end