feat(badges): only cache successful badge responses from shields.io

Author: bethesque

lib/pact_broker/badges/cached_service.rb

@@ -12,6 +12,7 @@ module Service
       include PactBroker::Logging
 
       SPACE_DASH_UNDERSCORE = /[\s_\-]/
+      CACHE = {}
 
       def pact_verification_badge pact, label, initials, verification_status
         return static_svg(pact, verification_status) unless pact
@@ -23,6 +24,10 @@ def pact_verification_badge pact, label, initials, verification_status
         dynamic_svg(title, status, color) || static_svg(pact, verification_status)
       end
 
+      def clear_cache
+        CACHE.clear
+      end
+
       private
 
       def badge_title pact, label, initials
@@ -79,7 +84,7 @@ def dynamic_svg left_text, right_text, color
           response = do_request(uri)
           response.code == '200' ? response.body : nil
         rescue StandardError => e
-          log_error e, "Error retrieving badge from #{uri}"
+          logger.error "Error retrieving badge from #{uri} due to #{e.class} - #{e.message}"
           nil
         end
       end
@@ -95,11 +100,27 @@ def escape_text text
       end
 
       def do_request(uri)
-        request = Net::HTTP::Get.new(uri)
-        Net::HTTP.start(uri.hostname, uri.port,
-          use_ssl: uri.scheme == 'https', read_timeout: 1000) do |http|
-          http.request request
+        with_cache uri do
+          request = Net::HTTP::Get.new(uri)
+          Net::HTTP.start(uri.hostname, uri.port,
+              use_ssl: uri.scheme == 'https',
+              read_timeout: 3,
+              open_timeout: 1,
+              ssl_timeout: 1,
+              continue_timeout: 1) do |http|
+            http.request request
+          end
+        end
+      end
+
+      def with_cache uri
+        if !(response = CACHE[uri])
+          response = yield
+          if response.code == '200'
+            CACHE[uri] = response
+          end
         end
+        response
       end
 
       def static_svg pact, verification_status

lib/pact_broker/badges/service.rb

@@ -43,8 +43,8 @@ def verification_service
     end
 
     def badge_service
-      require 'pact_broker/badges/cached_service'
-      Badges::CachedService
+      require 'pact_broker/badges/service'
+      Badges::Service
     end
   end
 end

lib/pact_broker/services.rb

@@ -1,5 +1,5 @@
 require 'pact_broker/api/resources/badge'
-require 'pact_broker/badges/cached_service'
+require 'pact_broker/badges/service'
 
 module PactBroker
   module Api
@@ -11,7 +11,7 @@ module Resources
         before do
           allow(PactBroker::Pacts::Service).to receive(:find_latest_pact).and_return(pact)
           allow(PactBroker::Verifications::Service).to receive(:find_latest_verification_for).and_return(verification)
-          allow(PactBroker::Badges::CachedService).to receive(:pact_verification_badge).and_return("badge")
+          allow(PactBroker::Badges::Service).to receive(:pact_verification_badge).and_return("badge")
           allow(PactBroker::Verifications::Status).to receive(:new).and_return(verification_status)
         end
 
@@ -68,7 +68,7 @@ module Resources
           end
 
           it "creates a badge" do
-            expect(PactBroker::Badges::CachedService).to receive(:pact_verification_badge).with(pact, nil, false, :verified)
+            expect(PactBroker::Badges::Service).to receive(:pact_verification_badge).with(pact, nil, false, :verified)
             subject
           end
 
@@ -84,7 +84,7 @@ module Resources
             let(:params) { {label: 'consumer'} }
 
             it "creates a badge with the specified label" do
-              expect(PactBroker::Badges::CachedService).to receive(:pact_verification_badge).with(anything, 'consumer', anything, anything)
+              expect(PactBroker::Badges::Service).to receive(:pact_verification_badge).with(anything, 'consumer', anything, anything)
               subject
             end
           end
@@ -93,7 +93,7 @@ module Resources
             let(:params) { {initials: 'true'} }
 
             it "creates a badge with initials" do
-              expect(PactBroker::Badges::CachedService).to receive(:pact_verification_badge).with(anything, anything, true, anything)
+              expect(PactBroker::Badges::Service).to receive(:pact_verification_badge).with(anything, anything, true, anything)
               subject
             end
           end

spec/lib/pact_broker/api/resources/badge_spec.rb

@@ -20,12 +20,22 @@ module Service
           stub_request(:get, expected_url).to_return(:status => response_status, :body => "svg")
         end
 
-        let(:subject) { PactBroker::Badges::Service.pact_verification_badge pact, label, initials, verification_status }
+        subject { PactBroker::Badges::Service.pact_verification_badge pact, label, initials, verification_status }
+
+        before do
+          Service.clear_cache
+        end
 
         it "returns the svg file" do
            expect(subject).to eq "svg"
         end
 
+        it "caches the response" do
+          PactBroker::Badges::Service.pact_verification_badge pact, label, initials, verification_status
+          PactBroker::Badges::Service.pact_verification_badge pact, label, initials, verification_status
+          expect(http_request).to have_been_made.once
+        end
+
         context "when the label is not specified" do
           it "creates a badge with the consumer and provider names" do
             subject
@@ -221,6 +231,10 @@ module Service
           it "returns a static image" do
             expect(subject).to include ">pact</"
           end
+
+          it "does not cache the response" do
+            expect(Service::CACHE.size).to eq 0
+          end
         end
 
         context "when the shields_io_base_url is not configured" do

spec/lib/pact_broker/badges/cached_service_spec.rb

@@ -23,8 +23,8 @@
 
   config.before :each do
     PactBroker.reset_configuration
-    require 'pact_broker/badges/cached_service'
-    PactBroker::Badges::CachedService.clear_cache
+    require 'pact_broker/badges/service'
+    PactBroker::Badges::Service.clear_cache
   end
 
   config.include Rack::Test::Methods