From 7d0fe1ea2e396e14ac2aa5cd9e28993a88b51ff1 Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Tue, 8 Sep 2020 10:59:48 +1000 Subject: [PATCH] feat(pacts for verification): do not require environment variable feature toggle to enable feature Removes support for the GET method on the pacts for verification resource. --- .../verifiable_pacts_query_schema.rb | 37 ------- lib/pact_broker/api/resources/index.rb | 15 +-- .../provider_pacts_for_verification.rb | 9 +- .../provider-pacts-for-verification.markdown | 80 +++++++++++++++ ...et_provider_pacts_for_verification_spec.rb | 18 ---- .../verifiable_pacts_query_schema_spec.rb | 97 ------------------- .../provider_pacts_for_verification_spec.rb | 44 ++------- 7 files changed, 97 insertions(+), 203 deletions(-) delete mode 100644 lib/pact_broker/api/contracts/verifiable_pacts_query_schema.rb create mode 100644 lib/pact_broker/doc/views/provider-pacts-for-verification.markdown delete mode 100644 spec/lib/pact_broker/api/contracts/verifiable_pacts_query_schema_spec.rb diff --git a/lib/pact_broker/api/contracts/verifiable_pacts_query_schema.rb b/lib/pact_broker/api/contracts/verifiable_pacts_query_schema.rb deleted file mode 100644 index 18498a6c2..000000000 --- a/lib/pact_broker/api/contracts/verifiable_pacts_query_schema.rb +++ /dev/null @@ -1,37 +0,0 @@ -require 'dry-validation' -require 'pact_broker/api/contracts/dry_validation_workarounds' -require 'pact_broker/api/contracts/dry_validation_predicates' - -module PactBroker - module Api - module Contracts - class VerifiablePactsQuerySchema - extend DryValidationWorkarounds - using PactBroker::HashRefinements - - SCHEMA = Dry::Validation.Schema do - configure do - predicates(DryValidationPredicates) - config.messages_file = File.expand_path("../../../locale/en.yml", __FILE__) - end - optional(:provider_version_tags).maybe(:array?) - optional(:consumer_version_selectors).each do - schema do - required(:tag).filled(:str?) - optional(:latest).filled(included_in?: ["true", "false"]) - optional(:fallback_tag).filled(:str?) - optional(:consumer).filled(:str?, :not_blank?) - end - end - optional(:include_pending_status).filled(included_in?: ["true", "false"]) - optional(:include_wip_pacts_since).filled(:date?) - end - - def self.call(params) - select_first_message(flatten_indexed_messages(SCHEMA.call(params&.symbolize_keys).messages(full: true))) - end - - end - end - end -end diff --git a/lib/pact_broker/api/resources/index.rb b/lib/pact_broker/api/resources/index.rb index 5414bc5a4..401c5d675 100644 --- a/lib/pact_broker/api/resources/index.rb +++ b/lib/pact_broker/api/resources/index.rb @@ -133,13 +133,14 @@ def links }] } - if PactBroker.feature_enabled?(:pacts_for_verification) - links_hash['beta:provider-pacts-for-verification'] = { - href: base_url + '/pacts/provider/{provider}/for-verification', - title: 'Pact versions to be verified for the specified provider', - templated: true - } - end + + links_hash['pb:provider-pacts-for-verification'] = { + href: base_url + '/pacts/provider/{provider}/for-verification', + title: 'Pact versions to be verified for the specified provider', + templated: true + } + + links_hash['beta:provider-pacts-for-verification'] = links_hash['pb:provider-pacts-for-verification'] links_hash end diff --git a/lib/pact_broker/api/resources/provider_pacts_for_verification.rb b/lib/pact_broker/api/resources/provider_pacts_for_verification.rb index e57ca211a..d1534c86a 100644 --- a/lib/pact_broker/api/resources/provider_pacts_for_verification.rb +++ b/lib/pact_broker/api/resources/provider_pacts_for_verification.rb @@ -1,6 +1,5 @@ require 'pact_broker/api/resources/provider_pacts' require 'pact_broker/api/decorators/verifiable_pacts_decorator' -require 'pact_broker/api/contracts/verifiable_pacts_query_schema' require 'pact_broker/api/decorators/verifiable_pacts_query_decorator' require 'pact_broker/api/contracts/verifiable_pacts_json_query_schema' require 'pact_broker/hash_refinements' @@ -12,7 +11,7 @@ class ProviderPactsForVerification < ProviderPacts using PactBroker::HashRefinements def allowed_methods - ["GET", "POST", "OPTIONS"] + ["POST", "OPTIONS"] end def content_types_accepted @@ -56,11 +55,7 @@ def to_json end def query_schema - if request.get? - PactBroker::Api::Contracts::VerifiablePactsQuerySchema - else - PactBroker::Api::Contracts::VerifiablePactsJSONQuerySchema - end + PactBroker::Api::Contracts::VerifiablePactsJSONQuerySchema end def parsed_query_params diff --git a/lib/pact_broker/doc/views/provider-pacts-for-verification.markdown b/lib/pact_broker/doc/views/provider-pacts-for-verification.markdown new file mode 100644 index 000000000..d09851e5b --- /dev/null +++ b/lib/pact_broker/doc/views/provider-pacts-for-verification.markdown @@ -0,0 +1,80 @@ +# Provider pacts for verification + +Path: `/pacts/provider/{provider}/for-verification` + +Allowed methods: `POST` + +Content type: `application/hal+json` + +Returns a deduplicated list of pacts to be verified by the specified provider. + +### Body + +Example: This data structure represents the way a user might specify "I want to verify the latest 'master' pact, all 'prod' pacts, and when I publish the verification results, I'm going to tag the provider version with 'master'" + + +```json +{ + "consumerVersionSelectors": [ + { + "tag": "master", + "latest": true + },{ + "tag": "prod" + } + ], + "providerVersionTags": ["master"], + "includePendingStatus": true, + "includeWipPactsSince": "2020-01-01" +} +``` + +`consumerVersionSelectors.tag`: the tag name(s) of the consumer versions to get the pacts for. + +`consumerVersionSelectors.fallbackTag`: the name of the tag to fallback to if the specified `tag` does not exist. This is useful when the consumer and provider use matching branch names to coordinate the development of new features. + +`consumerVersionSelectors.latest`: true. If the latest flag is omitted, *all* the pacts with the specified tag will be returned. (This might seem a bit weird, but it's done this way to match the syntax used for the matrix query params. See https://docs.pact.io/selectors) + +`consumerVersionSelectors.consumer`: allows a selector to only be applied to a certain consumer. This is used when there is an API that has multiple consumers, one of which is a deployed service, and one of which is a mobile consumer. The deployed service only needs the latest production pact verified, where as the mobile consumer may want all the production pacts verified. + +`providerVersionTags`: the tag name(s) for the provider application version that will be published with the verification results. This is used by the Broker to determine whether or not a particular pact is in pending state or not. This parameter can be specified multiple times. + +`includePendingStatus`: true|false (default false). When true, a pending boolean will be added to the verificationProperties in the response, and an extra message will appear in the notices array to indicate why this pact is/is not in pending state. This will allow your code to handle the response based on only what is present in the response, and not have to do ifs based on the user's options together with the response. As requested in the "pacts for verification" issue, please print out these messages in the tests if possible. If not possible, perhaps create a separate task which will list the pact URLs and messages for debugging purposes. + +`includeWipPactsSince`: Date string. The date from which to include the "work in progress" pacts. See https://docs.pact.io/wip for more information on work in progress pacts. + +### Response body + +`pending` flag and the "pending reason" notice will only be included if `includePendingStatus` is set to true. + +```json +{ + "_embedded": { + "pacts": [ + { + "verificationProperties": { + "notices": [ + { + "text": "This pact is being verified because it is the pact for the latest version of Foo tagged with 'dev'", + "when": "before_verification" + } + ], + "pending": false + }, + "_links": { + "self": { + "href": "http://localhost:9292/pacts/provider/Bar/consumer/Foo/pact-version/0e3369199f4008231946e0245474537443ccda2a", + "name": "Pact between Foo (v1.0.0) and Bar" + } + } + } + ] + }, + "_links": { + "self": { + "href": "http://localhost:9292/pacts/provider/Bar/for-verification", + "title": "Pacts to be verified" + } + } +} +``` diff --git a/spec/features/get_provider_pacts_for_verification_spec.rb b/spec/features/get_provider_pacts_for_verification_spec.rb index ac3fd854d..febb2af41 100644 --- a/spec/features/get_provider_pacts_for_verification_spec.rb +++ b/spec/features/get_provider_pacts_for_verification_spec.rb @@ -24,24 +24,6 @@ let(:path) { "/pacts/provider/Provider/for-verification" } - context "when using GET" do - it "returns a 200 HAL JSON response" do - expect(subject).to be_a_hal_json_success_response - end - - it "returns a list of links to the pacts" do - expect(pacts.size).to eq 1 - end - - context "when the provider does not exist" do - let(:path) { "/pacts/provider/ProviderThatDoesNotExist/for-verification" } - - it "returns a 404 response" do - expect(subject).to be_a_404_response - end - end - end - context "when using POST" do let(:request_body) do { diff --git a/spec/lib/pact_broker/api/contracts/verifiable_pacts_query_schema_spec.rb b/spec/lib/pact_broker/api/contracts/verifiable_pacts_query_schema_spec.rb deleted file mode 100644 index 5f7022117..000000000 --- a/spec/lib/pact_broker/api/contracts/verifiable_pacts_query_schema_spec.rb +++ /dev/null @@ -1,97 +0,0 @@ -require 'pact_broker/api/contracts/verifiable_pacts_query_schema' - -module PactBroker - module Api - module Contracts - describe VerifiablePactsQuerySchema do - let(:params) do - { - provider_version_tags: provider_version_tags, - consumer_version_selectors: consumer_version_selectors - } - end - - let(:provider_version_tags) { %w[master] } - - let(:consumer_version_selectors) do - [{ - tag: "master", - latest: "true" - }] - end - - subject { VerifiablePactsQuerySchema.(params) } - - context "when the params are valid" do - it "has no errors" do - expect(subject).to eq({}) - end - end - - context "when provider_version_tags is not an array" do - let(:provider_version_tags) { "foo" } - - it { is_expected.to have_key(:provider_version_tags) } - end - - context "when the consumer_version_selector is missing a tag" do - let(:consumer_version_selectors) do - [{}] - end - - it "flattens the messages" do - expect(subject[:consumer_version_selectors].first).to eq "tag is missing at index 0" - end - end - - context "when the consumer_version_selectors is missing the latest" do - let(:consumer_version_selectors) do - [{ - tag: "master" - }] - end - - it { is_expected.to be_empty } - end - - context "when include_wip_pacts_since key exists" do - let(:include_wip_pacts_since) { nil } - let(:params) do - { - include_wip_pacts_since: include_wip_pacts_since - } - end - - context "when it is nil" do - it { is_expected.to have_key(:include_wip_pacts_since) } - end - - context "when it is not a date" do - let(:include_wip_pacts_since) { "foo" } - - it { is_expected.to have_key(:include_wip_pacts_since) } - end - - context "when it is a valid date" do - let(:include_wip_pacts_since) { "2013-02-13T20:04:45.000+11:00" } - - it { is_expected.to_not have_key(:include_wip_pacts_since) } - end - end - - context "when a blank consumer name is specified" do - let(:consumer_version_selectors) do - [{ - tag: "feat-x", - consumer: "" - }] - end - - it "has an error" do - expect(subject[:consumer_version_selectors].first).to include "blank" - end - end - end - end - end -end diff --git a/spec/lib/pact_broker/api/resources/provider_pacts_for_verification_spec.rb b/spec/lib/pact_broker/api/resources/provider_pacts_for_verification_spec.rb index b5b35a882..c04412cce 100644 --- a/spec/lib/pact_broker/api/resources/provider_pacts_for_verification_spec.rb +++ b/spec/lib/pact_broker/api/resources/provider_pacts_for_verification_spec.rb @@ -23,35 +23,7 @@ module Resources } end - subject { get(path, query) } - - describe "GET" do - it "finds the pacts for verification by the provider" do - # Naughty not mocking out the query parsing... - expect(PactBroker::Pacts::Service).to receive(:find_for_verification).with( - "Bar", - ["master"], - PactBroker::Pacts::Selectors.new([PactBroker::Pacts::Selector.latest_for_tag("dev")]), - { - include_wip_pacts_since: DateTime.parse('2018-01-01'), - include_pending_status: true - } - ) - subject - end - - context "when there are validation errors" do - let(:query) do - { - provider_version_tags: true, - } - end - - it "returns the keys with the right case" do - expect(JSON.parse(subject.body)['errors']).to have_key('provider_version_tags') - end - end - end + subject { post(path, request_body.to_json, request_headers) } describe "POST" do let(:request_body) do @@ -70,8 +42,6 @@ module Resources } end - subject { post(path, request_body.to_json, request_headers) } - it "finds the pacts for verification by the provider" do # Naughty not mocking out the query parsing... expect(PactBroker::Pacts::Service).to receive(:find_for_verification).with( @@ -97,14 +67,14 @@ module Resources expect(JSON.parse(subject.body)['errors']).to have_key('providerVersionTags') end end - end - it "uses the correct options for the decorator" do - expect(decorator).to receive(:to_json) do | options | - expect(options[:user_options][:title]).to eq "Pacts to be verified by provider Bar" - expect(options[:user_options][:include_pending_status]).to eq true + it "uses the correct options for the decorator" do + expect(decorator).to receive(:to_json) do | options | + expect(options[:user_options][:title]).to eq "Pacts to be verified by provider Bar" + expect(options[:user_options][:include_pending_status]).to eq true + end + subject end - subject end end end