From 7f917100cc25a7edbebf328da7064c3060ca7087 Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Tue, 10 Oct 2017 10:38:15 +1100 Subject: [PATCH] feat(matrix): add validation errors to matrix resource --- lib/pact_broker/api/resources/matrix.rb | 22 ++++++---- lib/pact_broker/matrix/service.rb | 33 ++++++++++++++- spec/features/get_matrix_spec.rb | 2 +- .../pact_broker/api/resources/matrix_spec.rb | 38 +++++++++++++++++ spec/lib/pact_broker/matrix/service_spec.rb | 42 +++++++++++++++++++ ...d_provider_version_to_verification_spec.rb | 2 +- .../provider_states_for_pact_broker_client.rb | 12 ++++++ 7 files changed, 139 insertions(+), 12 deletions(-) create mode 100644 spec/lib/pact_broker/api/resources/matrix_spec.rb create mode 100644 spec/lib/pact_broker/matrix/service_spec.rb diff --git a/lib/pact_broker/api/resources/matrix.rb b/lib/pact_broker/api/resources/matrix.rb index 598c07546..1f1956c0c 100644 --- a/lib/pact_broker/api/resources/matrix.rb +++ b/lib/pact_broker/api/resources/matrix.rb @@ -15,30 +15,34 @@ def allowed_methods ["GET"] end - def resource_exists? - true + def malformed_request? + error_messages = matrix_service.validate_selectors(selectors) + if error_messages.any? + set_json_validation_error_messages error_messages + true + else + false + end end def to_json - versions = version_service.find_versions_by_selector(version_selectors) - criteria = versions.each_with_object({}) { | version, hash | hash[version.pacticipant.name] = version.number } + criteria = selected_versions.each_with_object({}) { | version, hash | hash[version.pacticipant.name] = version.number } lines = matrix_service.find_compatible_pacticipant_versions(criteria) PactBroker::Api::Decorators::MatrixPactDecorator.new(lines).to_json(user_options: { base_url: base_url }) end def selectors - @selectors ||= CGI.parse(request.uri.query)['selector[]'] + @selectors ||= CGI.parse(CGI.unescape(request.uri.query))['selectors[]'] end def version_selectors @version_selectors ||= selectors.select{ | selector| selector.include?("/version/") } end - def pacticipant_selectors - @pacticipant_selectors ||= selectors.select{ | selector | selectors.include?("/version/")} + def selected_versions + @selected_versions ||= version_service.find_versions_by_selector(version_selectors) end - end end end -end \ No newline at end of file +end diff --git a/lib/pact_broker/matrix/service.rb b/lib/pact_broker/matrix/service.rb index bced812c7..019ed4225 100644 --- a/lib/pact_broker/matrix/service.rb +++ b/lib/pact_broker/matrix/service.rb @@ -3,9 +3,11 @@ module PactBroker module Matrix module Service - extend self + VERSION_SELECTOR_PATTERN = %r{(^[^/]+)/version/[^/]+$}.freeze + extend self extend PactBroker::Repositories + extend PactBroker::Services def find params matrix_repository.find params[:consumer_name], params[:provider_name] @@ -14,6 +16,35 @@ def find params def find_compatible_pacticipant_versions criteria matrix_repository.find_compatible_pacticipant_versions criteria end + + def validate_selectors selectors + error_messages = [] + selectors.each do | version_selector | + if !(version_selector =~ VERSION_SELECTOR_PATTERN) + error_messages << "Invalid version selector '#{version_selector}'. Format must be /version/" + end + end + + selectors.each do | version_selector | + if match = version_selector.match(VERSION_SELECTOR_PATTERN) + pacticipant_name = match[1] + unless pacticipant_service.find_pacticipant_by_name(pacticipant_name) + error_messages << "Pacticipant '#{pacticipant_name}' not found" + end + end + end + + if error_messages.empty? + selected_versions = version_service.find_versions_by_selector(selectors) + if selected_versions.any?(&:nil?) + selected_versions.each_with_index do | selected_version, i | + error_messages << "No pact or verification found for #{selectors[i]}" + end + end + end + + error_messages + end end end end diff --git a/spec/features/get_matrix_spec.rb b/spec/features/get_matrix_spec.rb index 433247282..a4e140e10 100644 --- a/spec/features/get_matrix_spec.rb +++ b/spec/features/get_matrix_spec.rb @@ -10,7 +10,7 @@ let(:path) { "/matrix" } let(:params) do { - selector: ['Consumer/version/1.0.0','Provider/version/4.5.6'] + selectors: ['Consumer/version/1.0.0','Provider/version/4.5.6'] } end let(:last_response_body) { JSON.parse(subject.body, symbolize_names: true) } diff --git a/spec/lib/pact_broker/api/resources/matrix_spec.rb b/spec/lib/pact_broker/api/resources/matrix_spec.rb new file mode 100644 index 000000000..54d93f87c --- /dev/null +++ b/spec/lib/pact_broker/api/resources/matrix_spec.rb @@ -0,0 +1,38 @@ +require 'pact_broker/api/resources/matrix' +require 'pact_broker/matrix/service' + +module PactBroker + module Api + module Resources + describe Matrix do + before do + allow(PactBroker::Matrix::Service).to receive(:validate_selectors).and_return(error_messages) + end + + let(:td) { TestDataBuilder.new } + let(:path) { "/matrix" } + let(:json_response_body) { JSON.parse(subject.body, symbolize_names: true) } + let(:params) { { selectors: ['Foo/version/1', 'Bar/version/2'] } } + let(:error_messages) { [] } + + subject { get path, params, {'Content-Type' => 'application/hal+json'}; last_response } + + it "validates the selectors" do + expect(PactBroker::Matrix::Service).to receive(:validate_selectors).with(['Foo/version/1', 'Bar/version/2']) + subject + end + + context "when a validation error occurs" do + let(:error_messages) { ['foo'] } + it "returns a 400 status" do + expect(subject.status).to eq 400 + end + + it "returns error messages" do + expect(json_response_body[:errors]).to eq ['foo'] + end + end + end + end + end +end diff --git a/spec/lib/pact_broker/matrix/service_spec.rb b/spec/lib/pact_broker/matrix/service_spec.rb new file mode 100644 index 000000000..2fee2dc25 --- /dev/null +++ b/spec/lib/pact_broker/matrix/service_spec.rb @@ -0,0 +1,42 @@ +require 'pact_broker/matrix/service' + +module PactBroker + module Matrix + describe Service do + describe "validate_selectors" do + let(:td) { TestDataBuilder.new } + + subject { Service.validate_selectors(selectors) } + + context "when a selector format is invalid" do + let(:selectors) { ["Foo/1"] } + + it "returns error messages" do + expect(subject.first).to eq "Invalid version selector 'Foo/1'. Format must be /version/" + end + end + + context "when one or more of the selectors does not match any known version" do + before do + td.create_pacticipant("Foo") + end + + let(:selectors) { ["Foo/version/1"] } + + it "returns error messages" do + expect(subject.first).to eq "No pact or verification found for Foo/version/1" + end + end + + context "when the pacticipant does not exist" do + let(:selectors) { ["Foo/version/1"] } + + it "returns error messages" do + expect(subject.first).to eq "Pacticipant 'Foo' not found" + end + end + end + end + end +end + diff --git a/spec/migrations/44_add_provider_version_to_verification_spec.rb b/spec/migrations/44_add_provider_version_to_verification_spec.rb index 817545638..3113638ba 100644 --- a/spec/migrations/44_add_provider_version_to_verification_spec.rb +++ b/spec/migrations/44_add_provider_version_to_verification_spec.rb @@ -42,7 +42,7 @@ it "sets the created_at date of the new version to the created_at of the verification" do subject - expect(PactBroker::Domain::Verification.first.provider_version.created_at).to eq now + expect(PactBroker::Domain::Verification.first.provider_version.created_at.to_datetime).to eq now end context "when the version already exists" do diff --git a/spec/service_consumers/provider_states_for_pact_broker_client.rb b/spec/service_consumers/provider_states_for_pact_broker_client.rb index 6ba5f25b1..57f9c51f6 100644 --- a/spec/service_consumers/provider_states_for_pact_broker_client.rb +++ b/spec/service_consumers/provider_states_for_pact_broker_client.rb @@ -2,6 +2,18 @@ Pact.provider_states_for "Pact Broker Client" do + provider_state "the pact for Foo version 1.2.3 has been verified by Bar version 4.5.6" do + set_up do + TestDataBuilder.new + .create_pact_with_hierarchy("Foo", "1.2.3", "Bar") + .create_verification(provider_version: "4.5.6") + .create_verification(provider_version: "7.8.9", number: 2) + .create_consumer_version("2.0.0") + .create_pact + .create_verification(provider_version: "4.5.6") + end + end + provider_state "the 'Pricing Service' does not exist in the pact-broker" do no_op end