diff --git a/lib/pact_broker/api/contracts/dry_validation_workarounds.rb b/lib/pact_broker/api/contracts/dry_validation_workarounds.rb new file mode 100644 index 000000000..c2487828a --- /dev/null +++ b/lib/pact_broker/api/contracts/dry_validation_workarounds.rb @@ -0,0 +1,38 @@ +module PactBroker + module Api + module Contracts + module DryValidationWorkarounds + extend self + + # I just cannot seem to get the validation to stop on the first error. + # If one rule fails, they all come back failed, and it's driving me nuts. + # Why on earth would I want that behaviour? + def select_first_message(messages) + messages.each_with_object({}) do | (key, value), new_messages | + new_messages[key] = [value.first] + end + end + + def flatten_array_of_hashes(array_of_hashes) + new_messages = array_of_hashes.collect do | index, hash | + hash.values.flatten.collect { | text | "#{text} at index #{index}"} + end.flatten + end + + def flatten_indexed_messages(messages) + if messages.values.any?{ | value | is_indexed_structure?(value) } + messages.each_with_object({}) do | (key, value), new_messages | + new_messages[key] = is_indexed_structure?(value) ? flatten_array_of_hashes(value) : value + end + else + messages + end + end + + def is_indexed_structure?(thing) + thing.is_a?(Hash) && thing.keys.first.is_a?(Integer) + end + end + end + end +end diff --git a/lib/pact_broker/api/contracts/verifiable_pacts_json_query_schema.rb b/lib/pact_broker/api/contracts/verifiable_pacts_json_query_schema.rb new file mode 100644 index 000000000..b86e1b20d --- /dev/null +++ b/lib/pact_broker/api/contracts/verifiable_pacts_json_query_schema.rb @@ -0,0 +1,28 @@ +require 'dry-validation' +require 'pact_broker/hash_refinements' +require 'pact_broker/api/contracts/dry_validation_workarounds' + +module PactBroker + module Api + module Contracts + class VerifiablePactsJSONQuerySchema + extend DryValidationWorkarounds + using PactBroker::HashRefinements + + SCHEMA = Dry::Validation.Schema do + optional(:providerVersionTags).maybe(:array?) + optional(:consumerVersionSelectors).each do + schema do + required(:tag).filled(:str?) + optional(:latest).filled(included_in?: [true, false]) + end + end + 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/contracts/verifiable_pacts_query_schema.rb b/lib/pact_broker/api/contracts/verifiable_pacts_query_schema.rb index e514f709b..90ce5c489 100644 --- a/lib/pact_broker/api/contracts/verifiable_pacts_query_schema.rb +++ b/lib/pact_broker/api/contracts/verifiable_pacts_query_schema.rb @@ -1,12 +1,15 @@ require 'dry-validation' +require 'pact_broker/api/contracts/dry_validation_workarounds' module PactBroker module Api module Contracts class VerifiablePactsQuerySchema + extend DryValidationWorkarounds + using PactBroker::HashRefinements + SCHEMA = Dry::Validation.Schema do optional(:provider_version_tags).maybe(:array?) - # optional(:exclude_other_pending).filled(included_in?: ["true", "false"]) optional(:consumer_version_selectors).each do schema do required(:tag).filled(:str?) @@ -16,24 +19,7 @@ class VerifiablePactsQuerySchema end def self.call(params) - select_first_message(flatten_index_messages(SCHEMA.call(params).messages(full: true))) - end - - def self.select_first_message(messages) - messages.each_with_object({}) do | (key, value), new_messages | - new_messages[key] = [value.first] - end - end - - def self.flatten_index_messages(messages) - if messages[:consumer_version_selectors] - new_messages = messages[:consumer_version_selectors].collect do | index, value | - value.values.flatten.collect { | text | "#{text} at index #{index}"} - end.flatten - messages.merge(consumer_version_selectors: new_messages) - else - messages - end + select_first_message(flatten_indexed_messages(SCHEMA.call(params&.symbolize_keys).messages(full: true))) end end end diff --git a/lib/pact_broker/api/decorators/verifiable_pacts_query_decorator.rb b/lib/pact_broker/api/decorators/verifiable_pacts_query_decorator.rb index 96774596f..03e106c99 100644 --- a/lib/pact_broker/api/decorators/verifiable_pacts_query_decorator.rb +++ b/lib/pact_broker/api/decorators/verifiable_pacts_query_decorator.rb @@ -1,25 +1,27 @@ require_relative 'base_decorator' require_relative 'verifiable_pact_decorator' require 'pact_broker/api/pact_broker_urls' +require 'pact_broker/hash_refinements' module PactBroker module Api module Decorators class VerifiablePactsQueryDecorator < BaseDecorator - collection :provider_version_tags + using PactBroker::HashRefinements - collection :consumer_version_selectors, class: OpenStruct do + collection :provider_version_tags, default: [] + + collection :consumer_version_selectors, default: [], class: OpenStruct do property :tag - property :latest, setter: ->(fragment:, represented:, **) { represented.latest = (fragment == 'true') } + property :latest, + setter: ->(fragment:, represented:, **) { + represented.latest = (fragment == 'true' || fragment == true) + } end - - def from_hash(*args) - # Should remember how to do this via Representable... - result = super - result.consumer_version_selectors = [] if result.consumer_version_selectors.nil? - result.provider_version_tags = [] if result.provider_version_tags.nil? - result + def from_hash(hash) + # This handles both the snakecase keys from the GET query and the camelcase JSON POST body + super(hash&.snakecase_keys) end end end diff --git a/lib/pact_broker/api/resources/base_resource.rb b/lib/pact_broker/api/resources/base_resource.rb index 49e7b6409..a5e22ed9b 100644 --- a/lib/pact_broker/api/resources/base_resource.rb +++ b/lib/pact_broker/api/resources/base_resource.rb @@ -88,7 +88,7 @@ def params end def params_with_string_keys - JSON.parse(request.body.to_s, {symbolize_names: false}.merge(PACT_PARSING_OPTIONS)) + @params_with_string_keys ||= JSON.parse(request.body.to_s, {symbolize_names: false}.merge(PACT_PARSING_OPTIONS)) end def pact_params 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 37850724b..c30711f80 100644 --- a/lib/pact_broker/api/resources/provider_pacts_for_verification.rb +++ b/lib/pact_broker/api/resources/provider_pacts_for_verification.rb @@ -2,14 +2,18 @@ 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' module PactBroker module Api module Resources class ProviderPactsForVerification < ProviderPacts - def initialize - super - @query = Rack::Utils.parse_nested_query(request.uri.query) + def allowed_methods + ["GET", "POST", "OPTIONS"] + end + + def content_types_accepted + [["application/json"]] end def malformed_request? @@ -21,9 +25,12 @@ def malformed_request? end end - private + def process_post + response.body = to_json + true + end - attr_reader :query + private def pacts pact_service.find_for_verification( @@ -43,12 +50,26 @@ def to_json def query_schema - PactBroker::Api::Contracts::VerifiablePactsQuerySchema + if request.get? + PactBroker::Api::Contracts::VerifiablePactsQuerySchema + else + PactBroker::Api::Contracts::VerifiablePactsJSONQuerySchema + end end def parsed_query_params @parsed_query_params ||= PactBroker::Api::Decorators::VerifiablePactsQueryDecorator.new(OpenStruct.new).from_hash(query) end + + def query + @query ||= begin + if request.get? + Rack::Utils.parse_nested_query(request.uri.query) + elsif request.post? + params_with_string_keys + end + end + end end end end diff --git a/lib/pact_broker/hash_refinements.rb b/lib/pact_broker/hash_refinements.rb index 4714f2d1e..1841622d9 100644 --- a/lib/pact_broker/hash_refinements.rb +++ b/lib/pact_broker/hash_refinements.rb @@ -1,6 +1,11 @@ +require 'pact_broker/string_refinements' + module PactBroker module HashRefinements + refine Hash do + using PactBroker::StringRefinements + def deep_merge(other_hash, &block) block_actual = Proc.new {|key, oldval, newval| newval = block.call(key, oldval, newval) if block_given? @@ -8,6 +13,49 @@ def deep_merge(other_hash, &block) } merge(other_hash, &block_actual) end + + def symbolize_keys + symbolize_keys_private(self) + end + + def snakecase_keys + snakecase_keys_private(self) + end + + private + + def snakecase_keys_private(params) + case params + when Hash + params.inject({}) do |result, (key, value)| + snake_key = case key + when String then key.snakecase + when Symbol then key.to_s.snakecase.to_sym + else + key + end + result.merge(snake_key => snakecase_keys_private(value)) + end + when Array + params.collect { |value| snakecase_keys_private(value) } + else + params + end + + end + + def symbolize_keys_private(params) + case params + when Hash + params.inject({}) do |result, (key, value)| + result.merge(key.to_sym => symbolize_keys_private(value)) + end + when Array + params.collect { |value| symbolize_keys_private(value) } + else + params + end + end end end end diff --git a/lib/pact_broker/string_refinements.rb b/lib/pact_broker/string_refinements.rb index a49e01fb3..fb79b7432 100644 --- a/lib/pact_broker/string_refinements.rb +++ b/lib/pact_broker/string_refinements.rb @@ -8,6 +8,41 @@ def not_blank? def blank? self.strip.size == 0 end + + # ripped from rubyworks/facets, thank you + def snakecase + gsub(/([A-Z]+)([A-Z][a-z])/,'\1_\2') + .gsub(/([a-z\d])([A-Z])/,'\1_\2') + .tr('-', '_') + .gsub(/\s/, '_') + .gsub(/__+/, '_') + .downcase + end + + # ripped from rubyworks/facets, thank you + def camelcase(*separators) + case separators.first + when Symbol, TrueClass, FalseClass, NilClass + first_letter = separators.shift + end + + separators = ['_', '\s'] if separators.empty? + + str = self.dup + + separators.each do |s| + str = str.gsub(/(?:#{s}+)([a-z])/){ $1.upcase } + end + + case first_letter + when :upper, true + str = str.gsub(/(\A|\s)([a-z])/){ $1 + $2.upcase } + when :lower, false + str = str.gsub(/(\A|\s)([A-Z])/){ $1 + $2.downcase } + end + + str + end end end -end \ No newline at end of file +end diff --git a/spec/features/get_provider_pacts_for_verification_spec.rb b/spec/features/get_provider_pacts_for_verification_spec.rb index 29fb62ec6..ac3fd854d 100644 --- a/spec/features/get_provider_pacts_for_verification_spec.rb +++ b/spec/features/get_provider_pacts_for_verification_spec.rb @@ -1,7 +1,13 @@ describe "Get provider pacts for verification" do let(:last_response_body) { JSON.parse(subject.body, symbolize_names: true) } let(:pacts) { last_response_body[:_embedded][:'pacts'] } - subject { get path; last_response } + let(:query) do + { + consumer_version_selectors: [ { tag: "prod", latest: "true" }] + } + end + + subject { get(path, query) } context "when the provider exists" do before do @@ -16,24 +22,53 @@ .create_pact end - context "with no tag specified" do - let(:path) { "/pacts/provider/Provider/for-verification" } + 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 2 + 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 - end - context "when the provider does not exist" do - let(:path) { "/pacts/provider/Provider" } + context "when using POST" do + let(:request_body) do + { + consumerVersionSelectors: [ { tag: "prod", latest: true }] + } + end - it "returns a 404 response" do - expect(subject).to be_a_404_response + let(:request_headers) do + { + 'CONTENT_TYPE' => 'application/json', + 'HTTP_ACCEPT' => 'application/hal+json' + } + end + + subject { post(path, request_body.to_json, request_headers) } + + 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 end end diff --git a/spec/integration/app_spec.rb b/spec/integration/app_spec.rb index d2d73fb4c..12aba7297 100644 --- a/spec/integration/app_spec.rb +++ b/spec/integration/app_spec.rb @@ -29,7 +29,7 @@ module PactBroker context "when Accept includes text/html" do let(:env) { {'HTTP_ACCEPT' => 'text/html'} } - subject { get path, '', env; last_response } + subject { get(path, '', env) } describe "a request for root" do let(:path) { '/' } diff --git a/spec/lib/pact_broker/api/contracts/verifiable_pacts_json_query_schema_spec.rb b/spec/lib/pact_broker/api/contracts/verifiable_pacts_json_query_schema_spec.rb new file mode 100644 index 000000000..c867554cc --- /dev/null +++ b/spec/lib/pact_broker/api/contracts/verifiable_pacts_json_query_schema_spec.rb @@ -0,0 +1,67 @@ +require 'pact_broker/api/contracts/verifiable_pacts_json_query_schema' + +module PactBroker + module Api + module Contracts + describe VerifiablePactsJSONQuerySchema do + let(:params) do + { + providerVersionTags: provider_version_tags, + consumerVersionSelectors: consumer_version_selectors + } + end + + let(:provider_version_tags) { %w[master] } + + let(:consumer_version_selectors) do + [{ + tag: "master", + latest: true + }] + end + + subject { VerifiablePactsJSONQuerySchema.(params) } + + context "when the params are valid" do + it "has no errors" do + expect(subject).to eq({}) + end + end + + context "when providerVersionTags is not an array" do + let(:provider_version_tags) { true } + + it { is_expected.to have_key(:providerVersionTags) } + end + + context "when consumerVersionSelectors is not an array" do + let(:consumer_version_selectors) { true } + + it { is_expected.to have_key(:consumerVersionSelectors) } + 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[:consumerVersionSelectors].first).to eq "tag is missing at index 0" + end + end + + context "when the consumerVersionSelectors is missing the latest" do + let(:consumer_version_selectors) do + [{ + tag: "master" + }] + end + + it "has no errors" do + expect(subject).to eq({}) + end + end + end + end + end +end 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 index e427156b3..1f2eb2240 100644 --- 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 @@ -4,7 +4,6 @@ module PactBroker module Api module Contracts describe VerifiablePactsQuerySchema do - let(:params) do { provider_version_tags: provider_version_tags, diff --git a/spec/lib/pact_broker/api/decorators/verifiable_pacts_query_decorator_spec.rb b/spec/lib/pact_broker/api/decorators/verifiable_pacts_query_decorator_spec.rb index ebeb4a328..387706cac 100644 --- a/spec/lib/pact_broker/api/decorators/verifiable_pacts_query_decorator_spec.rb +++ b/spec/lib/pact_broker/api/decorators/verifiable_pacts_query_decorator_spec.rb @@ -4,40 +4,72 @@ module PactBroker module Api module Decorators describe VerifiablePactsQueryDecorator do - let(:params) do - { - "provider_version_tags" => provider_version_tags, - "consumer_version_selectors" => consumer_version_selectors - } - end + let(:provider_version_tags) { %w[dev] } - let(:consumer_version_selectors) do - [{"tag" => "dev", "ignored" => "foo"}] - end subject { VerifiablePactsQueryDecorator.new(OpenStruct.new).from_hash(params) } - context "when latest is not specified" do - it "defaults to nil" do - expect(subject.consumer_version_selectors.first.latest).to be nil + context "when parsing JSON params" do + let(:params) do + { + "providerVersionTags" => provider_version_tags, + "consumerVersionSelectors" => consumer_version_selectors + } end - end - context "when latest is a string" do let(:consumer_version_selectors) do - [{"tag" => "dev", "latest" => "true"}] + [{"tag" => "dev", "ignored" => "foo", "latest" => true}] end - it "casts it to a boolean" do + context "when latest is not specified" do + let(:consumer_version_selectors) do + [{"tag" => "dev"}] + end + + it "defaults to nil" do + expect(subject.consumer_version_selectors.first.latest).to be nil + end + end + + it "parses the latest as a boolean" do expect(subject.consumer_version_selectors.first.latest).to be true end + + context "when there are no consumer_version_selectors" do + let(:params) { {} } + + it "returns an empty array" do + expect(subject.consumer_version_selectors).to eq [] + end + end + + context "when there are no provider_version_tags" do + let(:params) { {} } + + it "returns an empty array" do + expect(subject.provider_version_tags).to eq [] + end + end end - context "when there are no consumer_version_selectors" do - let(:params) { {} } + context "when parsing query string params" do + let(:params) do + { + "provider_version_tags" => provider_version_tags, + "consumer_version_selectors" => consumer_version_selectors + } + end - it "returns an empty array" do - expect(subject.consumer_version_selectors).to eq [] + let(:consumer_version_selectors) do + [{"tag" => "dev", "latest" => "true"}] + end + + it "parses the provider_version_tags" do + expect(subject.provider_version_tags).to eq provider_version_tags + end + + it "parses a string 'latest' to a boolean" do + expect(subject.consumer_version_selectors.first.latest).to be true 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 4cc2f8b47..ce175003a 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 @@ -14,13 +14,69 @@ module Resources let(:pacts) { double('pacts') } let(:path) { '/pacts/provider/Bar/for-verification' } let(:decorator) { instance_double('PactBroker::Api::Decorators::VerifiablePactsDecorator') } + let(:query) do + { + provider_version_tags: ['master'], + consumer_version_selectors: [ { tag: "dev", latest: "true" }] + } + end - subject { get(path, provider_version_tags: ['master'], consumer_version_selectors: [ { tag: "dev", latest: true}]) } + subject { get(path, query) } - 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"], [ OpenStruct.new(tag: "dev", latest: true)]) - subject + 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"], [OpenStruct.new(tag: "dev", latest: 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 + + describe "POST" do + let(:request_body) do + { + providerVersionTags: ['master'], + consumerVersionSelectors: [ { tag: "dev", latest: true }] + } + end + + let(:request_headers) do + { + 'CONTENT_TYPE' => 'application/json', + 'HTTP_ACCEPT' => 'application/hal+json' + } + 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("Bar", ["master"], [OpenStruct.new(tag: "dev", latest: true)]) + subject + end + + context "when there are validation errors" do + let(:request_body) do + { + providerVersionTags: true, + } + end + + it "returns the keys with the right case" do + expect(JSON.parse(subject.body)['errors']).to have_key('providerVersionTags') + end + end end it "sets the correct resource title" do diff --git a/spec/lib/pact_broker/hash_refinements_spec.rb b/spec/lib/pact_broker/hash_refinements_spec.rb index 69946d128..90cfa90f2 100644 --- a/spec/lib/pact_broker/hash_refinements_spec.rb +++ b/spec/lib/pact_broker/hash_refinements_spec.rb @@ -11,5 +11,29 @@ module PactBroker it "merges" do expect(a.deep_merge(b)).to eq expected end + + describe "snakecase_keys" do + let(:hash_1) do + { + "fooBar" => { + :meepMoop => "blahBlah", + "already_snake" => "" + } + } + end + + let(:expected) do + { + "foo_bar" => { + :meep_moop => "blahBlah", + "already_snake" => "" + } + } + end + + it "snake cases the keys" do + expect(hash_1.snakecase_keys).to eq expected + end + end end end