Skip to content

Commit

Permalink
feat(badges): only cache successful badge responses from shields.io
Browse files Browse the repository at this point in the history
  • Loading branch information
bethesque committed Sep 17, 2017
1 parent 6fc78ff commit e5f08ad
Show file tree
Hide file tree
Showing 7 changed files with 50 additions and 95 deletions.
32 changes: 0 additions & 32 deletions lib/pact_broker/badges/cached_service.rb

This file was deleted.

31 changes: 26 additions & 5 deletions lib/pact_broker/badges/service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions lib/pact_broker/services.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
10 changes: 5 additions & 5 deletions spec/lib/pact_broker/api/resources/badge_spec.rb
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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

Expand Down Expand Up @@ -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

Expand All @@ -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
Expand All @@ -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
Expand Down
48 changes: 0 additions & 48 deletions spec/lib/pact_broker/badges/cached_service_spec.rb

This file was deleted.

16 changes: 15 additions & 1 deletion spec/lib/pact_broker/badges/service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit e5f08ad

Please sign in to comment.