From 34f98592e04067bc4941196ccb26fe36415bc18e Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Tue, 6 Oct 2020 07:10:47 +1100 Subject: [PATCH] feat(wip pacts): add 'wip' column to verification results --- .../20201006_add_wip_to_verification.rb | 5 +++++ .../decorators/verifiable_pact_decorator.rb | 6 ++++-- lib/pact_broker/api/pact_broker_urls.rb | 16 +++++++++------ .../api/resources/verifications.rb | 12 +++++++++-- lib/pact_broker/verifications/service.rb | 3 ++- .../pact_and_verification_parameters.rb | 2 +- .../verifiable_pact_decorator_spec.rb | 9 +++++++-- .../pact_broker/api/pact_broker_urls_spec.rb | 11 +++++----- .../api/resources/verifications_spec.rb | 20 ++++++++++++++++--- .../pact_broker/verifications/service_spec.rb | 6 ++++-- spec/lib/pact_broker/webhooks/render_spec.rb | 2 +- 11 files changed, 67 insertions(+), 25 deletions(-) create mode 100644 db/migrations/20201006_add_wip_to_verification.rb diff --git a/db/migrations/20201006_add_wip_to_verification.rb b/db/migrations/20201006_add_wip_to_verification.rb new file mode 100644 index 000000000..935f45a60 --- /dev/null +++ b/db/migrations/20201006_add_wip_to_verification.rb @@ -0,0 +1,5 @@ +Sequel.migration do + change do + add_column(:verifications, :wip, TrueClass, default: false, null: false) + end +end diff --git a/lib/pact_broker/api/decorators/verifiable_pact_decorator.rb b/lib/pact_broker/api/decorators/verifiable_pact_decorator.rb index 1a52a5e0a..6c756316b 100644 --- a/lib/pact_broker/api/decorators/verifiable_pact_decorator.rb +++ b/lib/pact_broker/api/decorators/verifiable_pact_decorator.rb @@ -20,14 +20,16 @@ class VerifiablePactDecorator < BaseDecorator getter: -> (context) { context[:decorator].notices(context[:options][:user_options]) } def notices(user_options) - pact_url = pact_version_url(represented, user_options[:base_url]) + metadata = represented.wip ? { wip: true } : nil + pact_url = pact_version_url_with_metadata(represented, represented.wip, user_options[:base_url]) PactBroker::Pacts::BuildVerifiablePactNotices.call(represented, pact_url, user_options) end end link :self do | user_options | + metadata = represented.wip ? { wip: true } : nil { - href: pact_version_url(represented, user_options[:base_url]), + href: pact_version_url_with_metadata(represented, metadata, user_options[:base_url]), name: represented.name } end diff --git a/lib/pact_broker/api/pact_broker_urls.rb b/lib/pact_broker/api/pact_broker_urls.rb index 5df64fad2..e7f1a3423 100644 --- a/lib/pact_broker/api/pact_broker_urls.rb +++ b/lib/pact_broker/api/pact_broker_urls.rb @@ -58,19 +58,23 @@ def pact_version_url pact, base_url = '' "#{pactigration_base_url(base_url, pact)}/pact-version/#{pact.pact_version_sha}" end - def pact_version_url_with_metadata pact, base_url = '' - "#{pactigration_base_url(base_url, pact)}/pact-version/#{pact.pact_version_sha}/metadata/#{build_webhook_metadata(pact)}" + def pact_version_url_with_metadata pact, metadata, base_url = '' + if metadata && metadata.any? + "#{pact_version_url(pact, base_url)}/#{encode_metadata(metadata)}" + else + pact_version_url(pact, base_url) + end end - def build_webhook_metadata(pact) - encode_webhook_metadata(build_metadata_for_webhook_triggered_by_pact_publication(pact)) + def pact_version_url_with_webhook_metadata pact, base_url = '' + pact_version_url_with_metadata(pact, build_metadata_for_webhook_triggered_by_pact_publication(pact), base_url) end - def encode_webhook_metadata(metadata) + def encode_metadata(metadata) Base64.strict_encode64(Rack::Utils.build_nested_query(metadata)) end - def decode_webhook_metadata(metadata) + def decode_pact_metadata(metadata) if metadata begin Rack::Utils.parse_nested_query(Base64.strict_decode64(metadata)).each_with_object({}) do | (k, v), new_hash | diff --git a/lib/pact_broker/api/resources/verifications.rb b/lib/pact_broker/api/resources/verifications.rb index 0f4519d7a..5e6494dcf 100644 --- a/lib/pact_broker/api/resources/verifications.rb +++ b/lib/pact_broker/api/resources/verifications.rb @@ -48,7 +48,7 @@ def create_path end def from_json - verification = verification_service.create(next_verification_number, params(symbolize_names: false), pact, webhook_options) + verification = verification_service.create(next_verification_number, verification_params, pact, webhook_options) response.body = decorator_for(verification).to_json(decorator_options) true end @@ -72,7 +72,11 @@ def decorator_for model end def metadata - PactBrokerUrls.decode_webhook_metadata(identifier_from_path[:metadata]) + PactBrokerUrls.decode_pact_metadata(identifier_from_path[:metadata]) + end + + def wip? + metadata[:wip] == true end def webhook_options @@ -82,6 +86,10 @@ def webhook_options .with_webhook_context(metadata) } end + + def verification_params + params(symbolize_names: false).merge('wip' => wip?) + end end end end diff --git a/lib/pact_broker/verifications/service.rb b/lib/pact_broker/verifications/service.rb index 32c07925d..2946d8922 100644 --- a/lib/pact_broker/verifications/service.rb +++ b/lib/pact_broker/verifications/service.rb @@ -21,10 +21,11 @@ def next_number end def create next_verification_number, params, pact, webhook_options - logger.info "Creating verification #{next_verification_number} for pact_id=#{pact.id} from params #{params.reject{ |k,_| k == "testResults"}}" + logger.info "Creating verification #{next_verification_number} for pact_id=#{pact.id}", payload: params.reject{ |k,_| k == "testResults"} verification = PactBroker::Domain::Verification.new provider_version_number = params.fetch('providerApplicationVersion') PactBroker::Api::Decorators::VerificationDecorator.new(verification).from_hash(params) + verification.wip = params.fetch('wip') verification.number = next_verification_number verification = verification_repository.create(verification, provider_version_number, pact) diff --git a/lib/pact_broker/webhooks/pact_and_verification_parameters.rb b/lib/pact_broker/webhooks/pact_and_verification_parameters.rb index 337ae1121..31e6481e0 100644 --- a/lib/pact_broker/webhooks/pact_and_verification_parameters.rb +++ b/lib/pact_broker/webhooks/pact_and_verification_parameters.rb @@ -38,7 +38,7 @@ def initialize(pact, trigger_verification, webhook_context) def to_hash @hash ||= { - PACT_URL => pact ? PactBroker::Api::PactBrokerUrls.pact_version_url_with_metadata(pact, base_url) : "", + PACT_URL => pact ? PactBroker::Api::PactBrokerUrls.pact_version_url_with_webhook_metadata(pact, base_url) : "", VERIFICATION_RESULT_URL => verification_url, CONSUMER_VERSION_NUMBER => consumer_version_number, PROVIDER_VERSION_NUMBER => verification ? verification.provider_version_number : "", 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 4c0bdf634..f8e11cab9 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 @@ -5,7 +5,7 @@ module Api module Decorators describe VerifiablePactDecorator do before do - allow_any_instance_of(PactBroker::Api::PactBrokerUrls).to receive(:pact_version_url).and_return('http://pact') + 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') end @@ -61,7 +61,7 @@ module Decorators end it "creates the pact version url" do - expect(decorator).to receive(:pact_version_url).with(pact, 'http://example.org') + expect(decorator).to receive(:pact_version_url_with_metadata).with(pact, nil, 'http://example.org') subject end @@ -84,6 +84,11 @@ 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 cb51ed0ae..266216a0c 100644 --- a/spec/lib/pact_broker/api/pact_broker_urls_spec.rb +++ b/spec/lib/pact_broker/api/pact_broker_urls_spec.rb @@ -111,25 +111,26 @@ module Api end it "builds the webhook metadata" do - expect(PactBrokerUrls.decode_webhook_metadata(PactBrokerUrls.build_webhook_metadata(pact))).to eq (expected_metadata) + 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) end end - describe "decode_webhook_metadata" do + describe "decode_pact_metadata" do context "when the metadata is nil" do it "returns an empty hash" do - expect(PactBrokerUrls.decode_webhook_metadata(nil)).to eq({}) + expect(PactBrokerUrls.decode_pact_metadata(nil)).to eq({}) end end context "when the metadata is not valid base64" do it "returns an empty hash" do - expect(PactBrokerUrls.decode_webhook_metadata("foo==,")).to eq({}) + expect(PactBrokerUrls.decode_pact_metadata("foo==,")).to eq({}) end it "logs a warning" do expect(logger).to receive(:warn).with("Exception parsing webhook metadata: foo==,", ArgumentError) - PactBrokerUrls.decode_webhook_metadata("foo==,") + PactBrokerUrls.decode_pact_metadata("foo==,") end end end diff --git a/spec/lib/pact_broker/api/resources/verifications_spec.rb b/spec/lib/pact_broker/api/resources/verifications_spec.rb index 27f739501..8ec1237fc 100644 --- a/spec/lib/pact_broker/api/resources/verifications_spec.rb +++ b/spec/lib/pact_broker/api/resources/verifications_spec.rb @@ -24,7 +24,7 @@ module Resources before do allow(PactBroker::Verifications::Service).to receive(:create).and_return(verification) allow(PactBroker::Verifications::Service).to receive(:errors).and_return(double(:errors, messages: ['errors'], empty?: errors_empty)) - allow(PactBrokerUrls).to receive(:decode_webhook_metadata).and_return(parsed_metadata) + allow(PactBrokerUrls).to receive(:decode_pact_metadata).and_return(parsed_metadata) allow_any_instance_of(Verifications).to receive(:webhook_execution_configuration).and_return(webhook_execution_configuration) allow(webhook_execution_configuration).to receive(:with_webhook_context).and_return(webhook_execution_configuration) end @@ -65,7 +65,7 @@ module Resources end it "parses the webhook metadata" do - expect(PactBrokerUrls).to receive(:decode_webhook_metadata).with("abcd") + expect(PactBrokerUrls).to receive(:decode_pact_metadata).with("abcd") subject end @@ -85,7 +85,7 @@ module Resources it "stores the verification in the database" do expect(PactBroker::Verifications::Service).to receive(:create).with( next_verification_number, - hash_including('some' => 'params'), + hash_including('some' => 'params', 'wip' => false), pact, { webhook_execution_configuration: webhook_execution_configuration, @@ -104,6 +104,20 @@ module Resources it "returns the serialised verification in the response body" do expect(subject.body).to eq serialised_verification end + + context "when the verification is for a wip pact" do + let(:parsed_metadata) { { wip: true } } + + it "merges that into the verification params" do + expect(PactBroker::Verifications::Service).to receive(:create).with( + anything, + hash_including('wip' => true), + anything, + anything + ) + subject + end + end end context "when invalid parameters are used" do diff --git a/spec/lib/pact_broker/verifications/service_spec.rb b/spec/lib/pact_broker/verifications/service_spec.rb index 3d631dfd2..4f09f8dfb 100644 --- a/spec/lib/pact_broker/verifications/service_spec.rb +++ b/spec/lib/pact_broker/verifications/service_spec.rb @@ -23,7 +23,7 @@ module Verifications let(:options) { { webhook_execution_configuration: webhook_execution_configuration } } let(:webhook_execution_configuration) { instance_double(PactBroker::Webhooks::ExecutionConfiguration) } - let(:params) { {'success' => true, 'providerApplicationVersion' => '4.5.6'} } + let(:params) { { 'success' => true, 'providerApplicationVersion' => '4.5.6', 'wip' => true, 'testResults' => { 'some' => 'results' }} } let(:pact) do td.create_pact_with_hierarchy .create_provider_version('4.5.6') @@ -33,14 +33,16 @@ module Verifications let(:create_verification) { subject.create 3, params, pact, options } it "logs the creation" do - expect(logger).to receive(:info).with(/.*verification.*3.*success/) + expect(logger).to receive(:info).with(/.*verification.*3/, payload: {"providerApplicationVersion"=>"4.5.6", "success"=>true, "wip"=>true}) create_verification end it "sets the verification attributes" do verification = create_verification + expect(verification.wip).to be true expect(verification.success).to be true expect(verification.number).to eq 3 + expect(verification.test_results).to eq 'some' => 'results' end it "sets the pact content for the verification" do diff --git a/spec/lib/pact_broker/webhooks/render_spec.rb b/spec/lib/pact_broker/webhooks/render_spec.rb index eef5ff266..1e0e4a4ca 100644 --- a/spec/lib/pact_broker/webhooks/render_spec.rb +++ b/spec/lib/pact_broker/webhooks/render_spec.rb @@ -10,7 +10,7 @@ module Webhooks describe Render do describe "#call" do before do - allow(PactBroker::Api::PactBrokerUrls).to receive(:pact_version_url_with_metadata).and_return("http://foo") + allow(PactBroker::Api::PactBrokerUrls).to receive(:pact_version_url_with_webhook_metadata).and_return("http://foo") allow(PactBroker::Api::PactBrokerUrls).to receive(:verification_url) do | verification, base_url | expect(verification).to_not be nil "http://verification"