From 7fe98a7d460094255762b057d564679dbb7ad716 Mon Sep 17 00:00:00 2001
From: Beth Skurrie <beth@bethesque.com>
Date: Mon, 12 Apr 2021 08:32:54 +1000
Subject: [PATCH] fix(verifications): gracefully handle a verification number
 in the URL that is not an integer

---
 lib/pact_broker/api/resources/verification.rb | 12 ++-
 .../api/resources/verification_spec.rb        | 83 +++++++++++++++++--
 2 files changed, 87 insertions(+), 8 deletions(-)

diff --git a/lib/pact_broker/api/resources/verification.rb b/lib/pact_broker/api/resources/verification.rb
index 5cbfd979e..d82096342 100644
--- a/lib/pact_broker/api/resources/verification.rb
+++ b/lib/pact_broker/api/resources/verification.rb
@@ -24,9 +24,11 @@ def allowed_methods
         end
 
         def resource_exists?
-          if identifier_from_path[:verification_number] == "all"
+          if verification_number == "all"
             set_json_error_message("To see all the verifications for a pact, use the Matrix page")
             false
+          elsif !verification_number_is_integer?
+            false
           else
             !!verification
           end
@@ -66,6 +68,14 @@ def decorator_for model
         def extended_decorator_for model
           decorator_class(:extended_verification_decorator).new(model)
         end
+
+        def verification_number
+          identifier_from_path[:verification_number]
+        end
+
+        def verification_number_is_integer?
+          verification_number =~ /^\d+$/
+        end
       end
     end
   end
diff --git a/spec/lib/pact_broker/api/resources/verification_spec.rb b/spec/lib/pact_broker/api/resources/verification_spec.rb
index ffb0f2a45..4b9167b77 100644
--- a/spec/lib/pact_broker/api/resources/verification_spec.rb
+++ b/spec/lib/pact_broker/api/resources/verification_spec.rb
@@ -1,15 +1,84 @@
-require 'pact_broker/api/resources/verification'
-
 module PactBroker
   module Api
     module Resources
       describe Verification do
-        context "when someone tries to get all the verifications for a pact" do
-          subject { get("/pacts/provider/Bar/consumer/Foo/pact-version/1/verification-results/all") }
+        before do
+          allow_any_instance_of(described_class).to receive(:verification_service).and_return(verification_service)
+          allow(verification_service).to receive(:find).and_return(verification)
+          allow(PactBroker::Api::Decorators::VerificationDecorator).to receive(:new).and_return(decorator)
+        end
+
+        let(:verification) { instance_double("PactBroker::Domain::Verification") }
+        let(:parsed_verification) { double("parsed verification") }
+        let(:verification_service) { class_double("PactBroker::Verifications::Service").as_stubbed_const }
+        let(:path) { "/pacts/provider/Bar/consumer/Foo/pact-version/1/verification-results/2" }
+
+        let(:rack_headers) do
+          {
+            "HTTP_ACCEPT" => "application/hal+json"
+          }
+        end
+
+        let(:decorator) do
+          instance_double("PactBroker::Api::Decorators::VerificationDecorator",
+            to_json: "response",
+            from_json: parsed_verification
+          )
+        end
+
+        describe "GET" do
+          let(:identifier_params) { { consumer_name: "Foo", provider_name: "Bar", pact_version_sha: "1", verification_number: "2" } }
+
+          subject { get(path, nil, rack_headers) }
+
+          it "finds the Verification" do
+            expect(PactBroker::Verifications::Service).to receive(:find).with(hash_including(identifier_params))
+            subject
+          end
+
+          context "when the verification does not exist" do
+            let(:verification) { nil }
+
+            it { is_expected.to be_a_404_response }
+          end
+
+          context "when someone tries to get all the verifications for a pact" do
+            let(:path) { "/pacts/provider/Bar/consumer/Foo/pact-version/1/verification-results/all" }
+
+            it "does not attempt to find the verification" do
+              expect(PactBroker::Verifications::Service).to_not receive(:find)
+              subject
+            end
+
+            it "tells them to use the matrix" do
+              expect(subject.status).to eq 404
+              expect(subject.body).to include "Matrix"
+            end
+          end
+
+          context "when the verification number specified is not a number" do
+            let(:path) { "/pacts/provider/Bar/consumer/Foo/pact-version/1/verification-results/5*5" }
+
+            it "does not attempt to find the verification" do
+              expect(PactBroker::Verifications::Service).to_not receive(:find)
+              subject
+            end
+
+            its(:status) { is_expected.to eq 404 }
+          end
+
+          context "when the Verification exists" do
+            it "generates a JSON representation of the Verification" do
+              expect(PactBroker::Api::Decorators::VerificationDecorator).to receive(:new).with(verification)
+              expect(decorator).to receive(:to_json).with(user_options: hash_including(base_url: "http://example.org"))
+              subject
+            end
+
+            it { is_expected.to be_a_hal_json_success_response }
 
-          it "tells them to use the matrix" do
-            expect(subject.status).to eq 404
-            expect(subject.body).to include "Matrix"
+            it "includes the JSON representation in the response body" do
+              expect(subject.body).to eq "response"
+            end
           end
         end
       end