Merge pull request #559 from wjordan/fetch_nil

Support caching nil values in the fetch operation

History.md

@@ -5,6 +5,7 @@ Dalli Changelog
 ==========
 
 - Support rcvbuff and sndbuff byte configuration. (btatnall)
+- Add `:cache_nils` option to support nil values in `DalliStore#fetch` and `Dalli::Client#fetch` (wjordan, #559)
 
 2.7.4
 ==========

README.md

@@ -188,7 +188,9 @@ If serving compressed data using nginx's HttpMemcachedModule, set `memcached_gzi
 
 **sndbuf**: In bytes, set the socket SO_SNDBUF. Defaults to operating system default.
 
-**rcvbuf**: In bytes, set the socket SO_RCVBUF. Defatuls to operating system default.
+**rcvbuf**: In bytes, set the socket SO_RCVBUF. Defaults to operating system default.
+
+**cache_nils**: Boolean. If true Dalli will not treat cached `nil` values as 'not found' for `#fetch` operations. Default is false.
 
 Features and Changes
 ------------------------

lib/active_support/cache/dalli_store.rb

@@ -82,31 +82,41 @@ def with(&block)
         @data.with(&block)
       end
 
+      # Fetch the value associated with the key.
+      # If a value is found, then it is returned.
+      #
+      # If a value is not found and no block is given, then nil is returned.
+      #
+      # If a value is not found (or if the found value is nil and :cache_nils is false)
+      # and a block is given, the block will be invoked and its return value
+      # written to the cache and returned.
       def fetch(name, options=nil)
         options ||= {}
+        options[:cache_nils] = true if @options[:cache_nils]
         namespaced_name = namespaced_key(name, options)
-
+        not_found = options[:cache_nils] ? Dalli::Server::NOT_FOUND : nil
         if block_given?
+          entry = not_found
           unless options[:force]
             entry = instrument(:read, namespaced_name, options) do |payload|
               read_entry(namespaced_name, options).tap do |result|
                 if payload
                   payload[:super_operation] = :fetch
-                  payload[:hit] = !result.nil?
+                  payload[:hit] = result != not_found
                 end
               end
             end
           end
 
-          if !entry.nil?
-            instrument(:fetch_hit, name, options) { |payload| }
-            entry
-          else
+          if entry == not_found
             result = instrument(:generate, name, options) do |payload|
               yield
             end
             write(name, result, options)
             result
+          else
+            instrument(:fetch_hit, name, options) { |payload|}
+            entry
           end
         else
           read(name, options)

lib/dalli/client.rb

@@ -27,6 +27,7 @@ class Client
     # - :compress - defaults to false, if true Dalli will compress values larger than 1024 bytes before sending them to memcached.
     # - :serializer - defaults to Marshal
     # - :compressor - defaults to zlib
+    # - :cache_nils - defaults to false, if true Dalli will not treat cached nil values as 'not found' for #fetch operations.
     #
     def initialize(servers=nil, options={})
       @servers = normalize_servers(servers || ENV["MEMCACHE_SERVERS"] || '127.0.0.1:11211')
@@ -52,8 +53,9 @@ def multi
 
     ##
     # Get the value associated with the key.
+    # If a value is not found, then +nil+ is returned.
     def get(key, options=nil)
-      perform(:get, key)
+      perform(:get, key, options)
     end
 
     ##
@@ -71,10 +73,24 @@ def get_multi(*keys)
       end
     end
 
+    CACHE_NILS = {cache_nils: true}.freeze
+
+    # Fetch the value associated with the key.
+    # If a value is found, then it is returned.
+    #
+    # If a value is not found and no block is given, then nil is returned.
+    #
+    # If a value is not found (or if the found value is nil and :cache_nils is false)
+    # and a block is given, the block will be invoked and its return value
+    # written to the cache and returned.
     def fetch(key, ttl=nil, options=nil)
       ttl ||= @options[:expires_in].to_i
+      options = options.nil? ? CACHE_NILS : options.merge(CACHE_NILS) if @options[:cache_nils]
       val = get(key, options)
-      if val.nil? && block_given?
+      not_found = @options[:cache_nils] ?
+        val == Dalli::Server::NOT_FOUND :
+        val.nil?
+      if not_found && block_given?
         val = yield
         add(key, val, ttl, options)
       end

lib/dalli/server.rb

@@ -267,10 +267,10 @@ def multi?
       Thread.current[:dalli_multi]
     end
 
-    def get(key)
+    def get(key, options=nil)
       req = [REQUEST, OPCODES[:get], key.bytesize, 0, 0, 0, key.bytesize, 0, 0, key].pack(FORMAT[:get])
       write(req)
-      generic_response(true)
+      generic_response(true, !!(options && options.is_a?(Hash) && options[:cache_nils]))
     end
 
     def send_multiget(keys)
@@ -492,11 +492,15 @@ def sanitize_ttl(ttl)
       end
     end
 
-    def generic_response(unpack=false)
+    # Implements the NullObject pattern to store an application-defined value for 'Key not found' responses.
+    class NilObject; end
+    NOT_FOUND = NilObject.new
+
+    def generic_response(unpack=false, cache_nils=false)
       (extras, _, status, count) = read_header.unpack(NORMAL_HEADER)
       data = read(count) if count > 0
       if status == 1
-        nil
+        cache_nils ? NOT_FOUND : nil
       elsif status == 2 || status == 5
         false # Not stored, normal status for add operation
       elsif status != 0

test/helper.rb

@@ -48,6 +48,16 @@ def with_activesupport
     yield
   end
 
+  def with_activesupport_memcached
+    port = 19987
+    with_activesupport do
+      memcached_persistent(port) do
+        connect(port)
+        yield
+      end
+    end
+  end
+
   def with_actionpack
     require 'action_dispatch'
     require 'action_controller'

test/memcached_mock.rb

@@ -78,8 +78,8 @@ def find_memcached
       raise Errno::ENOENT, "Unable to find memcached 1.4+ locally"
     end
 
-    def memcached_persistent(port=21345)
-      dc = start_and_flush_with_retry(port, '', {})
+    def memcached_persistent(port=21345, options={})
+      dc = start_and_flush_with_retry(port, '', options)
       yield dc, port if block_given?
     end
 

test/test_active_support.rb

@@ -47,25 +47,64 @@ def cache_key
       end
     end
 
-    it 'support fetch' do
-      with_activesupport do
-        memcached_persistent(@port) do
-          connect(@port)
-          dvalue = @dalli.fetch('someotherkeywithoutspaces', :expires_in => 1.second) { 123 }
-          assert_equal 123, dvalue
+    describe 'fetch' do
+      it 'support expires_in' do
+        with_activesupport_memcached do
+            dvalue = @dalli.fetch('someotherkeywithoutspaces', :expires_in => 1.second) { 123 }
+            assert_equal 123, dvalue
+        end
+      end
 
+      it 'support object' do
+        with_activesupport_memcached do
           o = Object.new
           o.instance_variable_set :@foo, 'bar'
-          dvalue = @dalli.fetch(rand_key, :raw => true) { o }
+          dvalue = @dalli.fetch(rand_key) { o }
           assert_equal o, dvalue
+        end
+      end
 
-          dvalue = @dalli.fetch(rand_key) { o }
+      it 'support object with raw' do
+        with_activesupport_memcached do
+          o = Object.new
+          o.instance_variable_set :@foo, 'bar'
+          dvalue = @dalli.fetch(rand_key, :raw => true) { o }
           assert_equal o, dvalue
+        end
+      end
 
+      it 'support false value' do
+        with_activesupport_memcached do
           @dalli.write('false', false)
           dvalue = @dalli.fetch('false') { flunk }
           assert_equal false, dvalue
+        end
+      end
+
+      it 'support nil value when cache_nils: true' do
+        @dalli = ActiveSupport::Cache.lookup_store(:dalli_store, "localhost:#{@port}", cache_nils: true)
+        @dalli.clear
+
+        memcached_persistent(@port) do
+          @dalli.write('nil', nil)
+          dvalue = @dalli.fetch('nil') { flunk }
+          assert_equal nil, dvalue
+        end
+
+        @dalli = ActiveSupport::Cache.lookup_store(:dalli_store, "localhost:#{@port}", cache_nils: false)
+        @dalli.clear
+
+        memcached_persistent(@port) do
+          @dalli.write('nil', nil)
+          executed = false
+          dvalue = @dalli.fetch('nil') { executed = true; 'bar' }
+          assert_equal true, executed
+          assert_equal 'bar', dvalue
+        end
+      end
 
+      it 'support object with cache_key' do
+        with_activesupport_memcached do
           user = MockUser.new
           @dalli.write(user.cache_key, false)
           dvalue = @dalli.fetch(user) { flunk }

test/test_dalli.rb

@@ -216,10 +216,25 @@
         dc.set("fetch_key", false)
         res = dc.fetch("fetch_key") { flunk "fetch block called" }
         assert_equal false, res
+      end
+    end
+
+    it "support the fetch operation with nil values when cache_nils: true" do
+      memcached_persistent(21345, cache_nils: true) do |dc|
+        dc.flush
 
         dc.set("fetch_key", nil)
-        res = dc.fetch("fetch_key") { "bob" }
-        assert_equal 'bob', res
+        res = dc.fetch("fetch_key") { flunk "fetch block called" }
+        assert_equal nil, res
+      end
+
+      memcached_persistent(21345, cache_nils: false) do |dc|
+        dc.flush
+        dc.set("fetch_key", nil)
+        executed = false
+        res = dc.fetch("fetch_key") { executed = true; 'bar' }
+        assert_equal 'bar', res
+        assert_equal true, executed
       end
     end