From e5f08adc40d7933e9fbcef7717639a570e6d0324 Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Sun, 17 Sep 2017 16:25:51 +1000 Subject: [PATCH] feat(badges): only cache successful badge responses from shields.io --- lib/pact_broker/badges/cached_service.rb | 32 ------------- lib/pact_broker/badges/service.rb | 31 ++++++++++-- lib/pact_broker/services.rb | 4 +- .../pact_broker/api/resources/badge_spec.rb | 10 ++-- .../pact_broker/badges/cached_service_spec.rb | 48 ------------------- spec/lib/pact_broker/badges/service_spec.rb | 16 ++++++- spec/spec_helper.rb | 4 +- 7 files changed, 50 insertions(+), 95 deletions(-) delete mode 100644 lib/pact_broker/badges/cached_service.rb delete mode 100644 spec/lib/pact_broker/badges/cached_service_spec.rb diff --git a/lib/pact_broker/badges/cached_service.rb b/lib/pact_broker/badges/cached_service.rb deleted file mode 100644 index 8e04f8443..000000000 --- a/lib/pact_broker/badges/cached_service.rb +++ /dev/null @@ -1,32 +0,0 @@ -require 'pact_broker/logging' -require 'pact_broker/badges/service' - -module PactBroker - module Badges - module CachedService - - extend self - include PactBroker::Logging - extend PactBroker::Services - - CACHE = {} - private_constant :CACHE - - def pact_verification_badge pact, label, initials, verification_status - badge_key = key(pact, label, initials, verification_status) - CACHE[badge_key] ||= PactBroker::Badges::Service.pact_verification_badge(pact, label, initials, verification_status) - end - - def clear_cache - CACHE.clear - end - - private - - def key pact, label, initials, verification_status - pact_name = pact ? "#{pact.consumer.name}-#{pact.provider.name}" : "nil" - "#{pact_name}-#{label}-#{initials}-#{verification_status}" - end - end - end -end diff --git a/lib/pact_broker/badges/service.rb b/lib/pact_broker/badges/service.rb index c5df4c021..95625ea48 100644 --- a/lib/pact_broker/badges/service.rb +++ b/lib/pact_broker/badges/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 diff --git a/lib/pact_broker/services.rb b/lib/pact_broker/services.rb index d20727300..c7fd8355e 100644 --- a/lib/pact_broker/services.rb +++ b/lib/pact_broker/services.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 diff --git a/spec/lib/pact_broker/api/resources/badge_spec.rb b/spec/lib/pact_broker/api/resources/badge_spec.rb index eb0ca5bf5..a515edb4e 100644 --- a/spec/lib/pact_broker/api/resources/badge_spec.rb +++ b/spec/lib/pact_broker/api/resources/badge_spec.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 diff --git a/spec/lib/pact_broker/badges/cached_service_spec.rb b/spec/lib/pact_broker/badges/cached_service_spec.rb deleted file mode 100644 index 064f2b3ee..000000000 --- a/spec/lib/pact_broker/badges/cached_service_spec.rb +++ /dev/null @@ -1,48 +0,0 @@ -require 'pact_broker/badges/cached_service' -require 'pact_broker/badges/service' - -module PactBroker - module Badges - describe CachedService do - - let(:consumer) { double('consumer', name: 'foo') } - let(:provider) { double('provider', name: 'bar') } - let(:pact) { double('pact', consumer: consumer, provider: provider) } - let(:label) { 'consumer' } - let(:initials) { false } - let(:verification_status) { 'status' } - - describe "#pact_verification_badge" do - - before do - allow(Service).to receive(:pact_verification_badge).and_return('badge') - end - - subject { CachedService.pact_verification_badge pact, label, initials, verification_status } - - it "returns the badge" do - expect(subject).to eq 'badge' - end - - context "when the badge is not in the cache" do - before do - stub_const('PactBroker::Badges::CachedService::CACHE', {}) - end - - it "retrieves the badge from the Badges::Service" do - expect(Service).to receive(:pact_verification_badge) - subject - end - end - - context "when the badge is in the cache" do - it "returns the cached badge" do - expect(Service).to receive(:pact_verification_badge).once - CachedService.pact_verification_badge pact, label, initials, verification_status - CachedService.pact_verification_badge pact, label, initials, verification_status - end - end - end - end - end -end diff --git a/spec/lib/pact_broker/badges/service_spec.rb b/spec/lib/pact_broker/badges/service_spec.rb index 479929774..2cf243f80 100644 --- a/spec/lib/pact_broker/badges/service_spec.rb +++ b/spec/lib/pact_broker/badges/service_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