Skip to content

Commit

Permalink
Merge pull request #848 from alphagov/remove-cache
Browse files Browse the repository at this point in the history
Remove caching
  • Loading branch information
thomasleese authored Aug 31, 2018
2 parents 89ed041 + 72fe0a9 commit a0c99d4
Show file tree
Hide file tree
Showing 7 changed files with 6 additions and 431 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
# Unrelease

* Remove support for caching responses.

# 52.8.0

* Add support for the `unpublish-messages` endpoint in email-alert-api
Expand Down
1 change: 0 additions & 1 deletion gds-api-adapters.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ Gem::Specification.new do |s|
s.require_path = 'lib'
s.add_dependency 'addressable'
s.add_dependency 'link_header'
s.add_dependency 'lrucache', '~> 0.1.1'
s.add_dependency 'null_logger'
s.add_dependency 'plek', '>= 1.9.0'
s.add_dependency 'rack-cache'
Expand Down
76 changes: 2 additions & 74 deletions lib/gds_api/json_client.rb
Original file line number Diff line number Diff line change
@@ -1,53 +1,22 @@
require_relative 'response'
require_relative 'exceptions'
require_relative 'version'
require_relative 'null_cache'
require_relative 'govuk_headers'
require 'lrucache'
require 'rest-client'
require 'null_logger'

module GdsApi
class JsonClient
include GdsApi::ExceptionHandling

# Cache TTL will be overridden for a given request/response by the Expires
# header if it is included in the response.
#
# LRUCache doesn't respect a cache size of 0, and instead effectively
# creates a cache with a size of 1.
def self.cache(size = DEFAULT_CACHE_SIZE, ttl = DEFAULT_CACHE_TTL)
@cache ||= LRUCache.new(max_size: size, ttl: ttl)
end

# Set the caching implementation. Default is LRUCache. Can be Anything
# which responds to:
#
# [](key)
# []=(key, value)
# store(key, value, expiry_time=nil) - or a Ruby Time object
#
class << self
attr_writer :cache
end

attr_accessor :logger, :options, :cache
attr_accessor :logger, :options

def initialize(options = {})
if options[:disable_timeout] || options[:timeout].to_i < 0
raise "It is no longer possible to disable the timeout."
end

@logger = options[:logger] || NullLogger.instance
disable_cache = options[:disable_cache] || ENV.fetch("GDS_API_DISABLE_CACHE", false)

if disable_cache || options[:cache_size]&.zero?
@cache = NullCache.new
else
cache_size = options[:cache_size] || DEFAULT_CACHE_SIZE
cache_ttl = options[:cache_ttl] || DEFAULT_CACHE_TTL
@cache = JsonClient.cache(cache_size, cache_ttl)
end
@options = options
end

Expand All @@ -70,8 +39,6 @@ def self.json_body_headers
end

DEFAULT_TIMEOUT_IN_SECONDS = 4
DEFAULT_CACHE_SIZE = 100
DEFAULT_CACHE_TTL = 15 * 60 # 15 minutes

def get_raw!(url)
do_raw_request(:get, url)
Expand Down Expand Up @@ -142,7 +109,7 @@ def do_json_request(method, url, params = nil, additional_headers = {}, &create_
if params
additional_headers.merge!(self.class.json_body_headers)
end
response = do_request_with_cache(method, url, (params.to_json if params), additional_headers)
response = do_request(method, url, (params.to_json if params), additional_headers)
rescue RestClient::Exception => e
# Attempt to parse the body as JSON if possible
error_details = begin
Expand Down Expand Up @@ -201,45 +168,6 @@ def with_ssl_options(method_params)
)
end

def do_request_with_cache(method, url, params = nil, additional_headers = {})
# Only read GET requests from the cache: any other request methods should
# always be passed through. Note that this means HEAD requests won't get
# cached, but that would involve separating the cache by method and URL.
# Also, we don't generally make HEAD requests.
use_cache = (method == :get)

if use_cache
cached_response = @cache[url]
return cached_response if cached_response
end

response = do_request(method, url, params, additional_headers)

if use_cache
cache_time = response_cache_time(response)
# If cache_time is nil, this will fall back on @cache's default
@cache.store(url, response, cache_time)
end

response
end

# Return either a Time object representing the expiry time of the response
# or nil if no cache information is provided
def response_cache_time(response)
if response.headers[:cache_control]
cache_control = Rack::Cache::CacheControl.new(response.headers[:cache_control])

if cache_control.private? || cache_control.no_cache? || cache_control.no_store?
Time.now.utc
elsif cache_control.max_age
Time.now.utc + cache_control.max_age
end
elsif response.headers[:expires]
Time.httpdate response.headers[:expires]
end
end

def do_request(method, url, params = nil, additional_headers = {})
loggable = { request_uri: url, start_time: Time.now.to_f }
start_logging = loggable.merge(action: 'start')
Expand Down
11 changes: 0 additions & 11 deletions lib/gds_api/null_cache.rb

This file was deleted.

3 changes: 0 additions & 3 deletions lib/gds_api/test_helpers/json_client_helper.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1 @@
require 'gds_api/json_client'
require 'gds_api/null_cache'

GdsApi::JsonClient.cache = GdsApi::NullCache.new
32 changes: 0 additions & 32 deletions test/gds_api_base_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,8 @@ def base_url
end
end

def setup
@orig_cache = GdsApi::JsonClient.cache
end

def teardown
GdsApi::Base.default_options = nil
GdsApi::JsonClient.cache = @orig_cache
end

def test_should_construct_escaped_query_string
Expand Down Expand Up @@ -55,33 +50,6 @@ def test_should_accept_options_as_second_arg
assert_equal "bar", api.options[:foo]
end

def test_setting_cache_size_from_options
GdsApi::JsonClient.cache = false
api = ConcreteApi.new("https://foo", cache_size: 2)
assert_equal 2, api.client.cache.max_size
end

def test_setting_cache_size_from_default_options
GdsApi::JsonClient.cache = false
GdsApi::Base.default_options = { cache_size: 4 }
api = ConcreteApi.new("http://bar")
assert_equal 4, api.client.cache.max_size
end

def test_disabling_cache
# Ensure that there is a non-null cache by default
GdsApi::JsonClient.cache = true
api = ConcreteApi.new("http://bar", disable_cache: true)
assert api.client.cache.is_a? GdsApi::NullCache
end

def test_disabling_cache_old_style
# Ensure that there is a non-null cache by default
GdsApi::JsonClient.cache = true
api = ConcreteApi.new("http://bar", cache_size: 0)
assert api.client.cache.is_a? GdsApi::NullCache
end

def test_should_barf_if_not_given_valid_url
assert_raises GdsApi::Base::InvalidAPIURL do
ConcreteApi.new('invalid-url')
Expand Down
Loading

0 comments on commit a0c99d4

Please sign in to comment.