Add LRU cache for HTTP response caching

lib/puppet_forge/lru_cache.rb

@@ -0,0 +1,78 @@
+require 'digest'
+
+module PuppetForge
+  # Implements a simple LRU cache. This is used internally by the
+  # {PuppetForge::V3::Base} class to cache API responses.
+  class LruCache
+    # Takes a list of strings (or objects that respond to #to_s) and
+    # returns a SHA256 hash of the strings joined with colons. This is
+    # a convenience method for generating cache keys. Cache keys do not
+    # have to be SHA256 hashes, but they must be unique.
+    def self.new_key(*string_args)
+      Digest::SHA256.hexdigest(string_args.map(&:to_s).join(':'))
+    end
+
+    # @return [Integer] the maximum number of items to cache.
+    attr_reader :max_size
+
+    # @param max_size [Integer] the maximum number of items to cache. This can
+    #   be overridden by setting the PUPPET_FORGE_MAX_CACHE_SIZE environment
+    #   variable.
+    def initialize(max_size = 30)
+      raise ArgumentError, "max_size must be a positive integer" unless max_size.is_a?(Integer) && max_size > 0
+
+      @max_size = ENV['PUPPET_FORGE_MAX_CACHE_SIZE'] ? ENV['PUPPET_FORGE_MAX_CACHE_SIZE'].to_i : max_size
+      @cache = {}
+      @lru = []
+      @semaphore = Mutex.new
+    end
+
+    # Retrieves a value from the cache.
+    # @param key [Object] the key to look up in the cache
+    # @return [Object] the cached value for the given key, or nil if
+    #   the key is not present in the cache.
+    def get(key)
+      if cache.key?(key)
+        semaphore.synchronize do
+          # If the key is present, move it to the front of the LRU
+          # list.
+          lru.delete(key)
+          lru.unshift(key)
+        end
+        cache[key]
+      end
+    end
+
+    # Adds a value to the cache.
+    # @param key [Object] the key to add to the cache
+    # @param value [Object] the value to add to the cache
+    def put(key, value)
+      semaphore.synchronize do
+        if cache.key?(key)
+          # If the key is already present, delete it from the LRU list.
+          lru.delete(key)
+        elsif cache.size >= max_size
+          # If the cache is full, remove the least recently used item.
+          cache.delete(lru.pop)
+        end
+        # Add the key to the front of the LRU list and add the value
+        # to the cache.
+        lru.unshift(key)
+        cache[key] = value
+      end
+    end
+
+    # Clears the cache.
+    def clear
+      semaphore.synchronize do
+        cache.clear
+        lru.clear
+      end
+    end
+
+    private
+
+    # Makes testing easier as these can be accessed directly with #send.
+    attr_reader :cache, :lru, :semaphore
+  end
+end

lib/puppet_forge/v3/base.rb

@@ -4,6 +4,7 @@
 
 require 'puppet_forge/lazy_accessors'
 require 'puppet_forge/lazy_relations'
+require 'puppet_forge/lru_cache'
 
 module PuppetForge
   module V3
@@ -53,8 +54,22 @@ def api_version
           API_VERSION
         end
 
+        # @private
+        def lru_cache
+          @lru_cache ||= PuppetForge::LruCache.new
+        end
+
+        # @private
+        def lru_cache_key(*args)
+          PuppetForge::LruCache.new_key(*args)
+        end
+
         # @private
         def request(resource, item = nil, params = {}, reset_connection = false, conn_opts = {})
+          cache_key = lru_cache_key(resource, item, params)
+          cached = lru_cache.get(cache_key)
+          return cached unless cached.nil?
+
           conn(reset_connection, conn_opts) if reset_connection
           unless conn.url_prefix.to_s =~ /^#{PuppetForge.host}/
             conn.url_prefix = "#{PuppetForge.host}"
@@ -69,7 +84,9 @@ def request(resource, item = nil, params = {}, reset_connection = false, conn_op
           # The API expects a space separated string. This allows the user to invoke it with a more natural feeling array.
           params['endorsements'] = params['endorsements'].join(' ') if params['endorsements'].is_a? Array
 
-          PuppetForge::V3::Base.conn.get uri_path, params
+          result = PuppetForge::V3::Base.conn.get uri_path, params
+          lru_cache.put(cache_key, result)
+          result
         end
 
         # @private

spec/spec_helper.rb

@@ -12,7 +12,10 @@
 
 module StubbingFaraday
 
-  def stub_api_for(klass, base_url = "http://api.example.com")
+  def stub_api_for(klass, base_url = "http://api.example.com", lru_cache: false)
+    unless lru_cache # Disable LRU cache by default
+      allow(klass).to receive(:lru_cache).and_return(instance_double('PuppetForge::LruCache', get: nil, put: nil, clear: nil))
+    end
     allow(klass).to receive(:conn) do
       Faraday.new :url => base_url do |builder|
         builder.response(:json, :content_type => /\bjson$/, :parser_options => { :symbolize_names => true })

spec/unit/forge/lru_cache_spec.rb

@@ -0,0 +1,127 @@
+require 'spec_helper'
+
+describe PuppetForge::LruCache do
+  it 'creates a cache key from a list of strings' do
+    expect { subject.class.new_key('foo', 'bar', 'baz') }.not_to raise_error 
+  end
+
+  it 'creates a new instance' do
+    expect { PuppetForge::LruCache.new(1) }.not_to raise_error
+  end
+
+  it 'raises an error if max_size is not a positive integer' do
+    expect { PuppetForge::LruCache.new(-1) }.to raise_error(ArgumentError)
+    expect { PuppetForge::LruCache.new(0) }.to raise_error(ArgumentError)
+    expect { PuppetForge::LruCache.new(1.5) }.to raise_error(ArgumentError)
+  end
+
+  it 'defaults to a max_size of 30' do
+    expect(PuppetForge::LruCache.new.max_size).to eq(30)
+  end
+
+  it 'allows max_size to be set via the max_size parameter' do
+    expect(PuppetForge::LruCache.new(42).max_size).to eq(42)
+  end
+
+  it 'provides a #get method' do
+    expect(PuppetForge::LruCache.new).to respond_to(:get)
+  end
+
+  it 'provides a #put method' do
+    expect(PuppetForge::LruCache.new).to respond_to(:put)
+  end
+
+  it 'provides a #clear method' do
+    expect(PuppetForge::LruCache.new).to respond_to(:clear)
+  end
+
+  context 'with environment variables' do
+    around(:each) do |example|
+      @old_max_size = ENV['PUPPET_FORGE_MAX_CACHE_SIZE']
+      ENV['PUPPET_FORGE_MAX_CACHE_SIZE'] = '42'
+      example.run
+      ENV['PUPPET_FORGE_MAX_CACHE_SIZE'] = @old_max_size
+    end
+
+    it 'uses the value of the PUPPET_FORGE_MAX_CACHE_SIZE environment variable if present' do
+      expect(PuppetForge::LruCache.new.max_size).to eq(42)
+    end
+  end
+
+  context '#get' do
+    it 'returns nil if the key is not present in the cache' do
+      expect(PuppetForge::LruCache.new.get('foo')).to be_nil
+    end
+
+    it 'returns the cached value for the given key' do
+      cache = PuppetForge::LruCache.new
+      cache.put('foo', 'bar')
+      expect(cache.get('foo')).to eq('bar')
+    end
+
+    it 'moves the key to the front of the LRU list' do
+      cache = PuppetForge::LruCache.new
+      cache.put('foo', 'bar')
+      cache.put('baz', 'qux')
+      cache.get('foo')
+      expect(cache.send(:lru)).to eq(['foo', 'baz'])
+    end
+
+    # The below test is non-deterministic but I'm not sure how to unit
+    # test thread-safety.
+    # it 'is thread-safe' do
+    #   cache = PuppetForge::LruCache.new
+    #   cache.put('foo', 'bar')
+    #   cache.put('baz', 'qux')
+    #   threads = []
+    #   threads << Thread.new { 100.times { cache.get('foo') } }
+    #   threads << Thread.new { 100.times { cache.get('baz') } }
+    #   threads.each(&:join)
+    #   expect(cache.send(:lru)).to eq(['baz', 'foo'])
+    # end
+  end
+
+  context '#put' do
+    it 'adds the key to the front of the LRU list' do
+      cache = PuppetForge::LruCache.new
+      cache.put('foo', 'bar')
+      expect(cache.send(:lru)).to eq(['foo'])
+    end
+
+    it 'adds the value to the cache' do
+      cache = PuppetForge::LruCache.new
+      cache.put('foo', 'bar')
+      expect(cache.send(:cache)).to eq({ 'foo' => 'bar' })
+    end
+
+    it 'removes the least recently used item if the cache is full' do
+      cache = PuppetForge::LruCache.new(2)
+      cache.put('foo', 'bar')
+      cache.put('baz', 'qux')
+      cache.put('quux', 'corge')
+      expect(cache.send(:lru)).to eq(['quux', 'baz'])
+    end
+
+    # The below test is non-deterministic but I'm not sure how to unit
+    # test thread-safety.
+    # it 'is thread-safe' do
+    #   cache = PuppetForge::LruCache.new
+    #   threads = []
+    #   threads << Thread.new { 100.times { cache.put('foo', 'bar') } }
+    #   threads << Thread.new { 100.times { cache.put('baz', 'qux') } }
+    #   threads.each(&:join)
+    #   expect(cache.send(:lru)).to eq(['baz', 'foo'])
+    # end
+  end
+
+  context '#clear' do
+    it 'clears the cache' do
+      cache = PuppetForge::LruCache.new
+      cache.put('foo', 'bar')
+      cache.put('baz', 'qux')
+      cache.clear
+      expect(cache.send(:lru).empty?).to be_truthy
+      expect(cache.send(:cache).empty?).to be_truthy
+    end
+  end
+end

spec/unit/forge/v3/base_spec.rb

@@ -61,6 +61,7 @@
   describe 'the host url setting' do
     context 'without a path prefix' do
       before(:each) do
+        PuppetForge::V3::Base.lru_cache.clear # We test the cache later, so clear it now
         @orig_host = PuppetForge.host
         PuppetForge.host = 'https://api.example.com'
 
@@ -83,10 +84,25 @@
         base = PuppetForge::V3::Base.find 'puppet'
         expect(base.username).to eq('foo')
       end
+
+      it 'caches responses' do
+        stub_api_for(PuppetForge::V3::Base, lru_cache: true) do |stubs|
+          stub_fixture(stubs, :get, '/v3/bases/puppet')
+        end
+        allow(PuppetForge::V3::Base.lru_cache).to receive(:put).and_call_original
+        allow(PuppetForge::V3::Base.lru_cache).to receive(:get).and_call_original
+
+        PuppetForge::V3::Base.find 'puppet'
+        PuppetForge::V3::Base.find 'puppet'
+        PuppetForge::V3::Base.find 'puppet'
+        expect(PuppetForge::V3::Base.lru_cache).to have_received(:put).once
+        expect(PuppetForge::V3::Base.lru_cache).to have_received(:get).exactly(3).times
+      end
     end
 
     context 'with a path prefix' do
       before(:each) do
+        PuppetForge::V3::Base.lru_cache.clear # We test the cache later, so clear it now
         @orig_host = PuppetForge.host
         PuppetForge.host = 'https://api.example.com/uri/prefix'
 
@@ -109,6 +125,20 @@
         base = PuppetForge::V3::Base.find 'puppet'
         expect(base.username).to eq('bar')
       end
+
+      it 'caches responses' do
+        stub_api_for(PuppetForge::V3::Base, PuppetForge.host, lru_cache: true) do |stubs|
+          stub_fixture(stubs, :get, '/uri/prefix/v3/bases/puppet')
+        end
+        allow(PuppetForge::V3::Base.lru_cache).to receive(:put).and_call_original
+        allow(PuppetForge::V3::Base.lru_cache).to receive(:get).and_call_original
+
+        PuppetForge::V3::Base.find 'puppet'
+        PuppetForge::V3::Base.find 'puppet'
+        PuppetForge::V3::Base.find 'puppet'
+        expect(PuppetForge::V3::Base.lru_cache).to have_received(:put).once
+        expect(PuppetForge::V3::Base.lru_cache).to have_received(:get).exactly(3).times
+      end
     end
   end
 end