Decouple the local cache strategy from MemCacheStore for reuse with other remote stores [#1653 state:resolved]

Signed-off-by: Joshua Peek <josh@joshpeek.com>

activesupport/lib/active_support/cache.rb

@@ -10,6 +10,10 @@ module ActiveSupport
     autoload :MemCacheStore, 'active_support/cache/mem_cache_store'
     autoload :CompressedMemCacheStore, 'active_support/cache/compressed_mem_cache_store'
 
+    module Strategy
+      autoload :LocalCache, 'active_support/cache/strategy/local_cache'
+    end
+
     # Creates a new CacheStore object according to the given options.
     #
     # If no arguments are passed to this method, then a new

activesupport/lib/active_support/cache/mem_cache_store.rb

@@ -23,24 +23,6 @@ module ActiveSupport
         DELETED     = "DELETED\r\n"
       end
 
-      # this allows caching of the fact that there is nothing in the remote cache
-      NULL = 'mem_cache_store:null'
-
-      THREAD_LOCAL_KEY = :mem_cache_store_cache
-
-      class LocalCache
-        def initialize(app)
-          @app = app
-        end
-
-        def call(env)
-          Thread.current[THREAD_LOCAL_KEY] = MemoryStore.new
-          @app.call(env)
-        ensure
-          Thread.current[THREAD_LOCAL_KEY] = nil
-        end
-      end
-
       attr_reader :addresses
 
       # Creates a new MemCacheStore object, with the given memcached server
@@ -57,22 +39,13 @@ module ActiveSupport
         addresses = ["localhost"] if addresses.empty?
         @addresses = addresses
         @data = MemCache.new(addresses, options)
+
+        extend Strategy::LocalCache
       end
 
       def read(key, options = nil) # :nodoc:
         super
-
-        value = local_cache && local_cache.read(key)
-        if value == NULL
-          nil
-        elsif value.nil?
-          value = @data.get(key, raw?(options))
-          local_cache.write(key, value || NULL) if local_cache
-          value
-        else
-          # forcing the value to be immutable
-          value.dup
-        end
+        @data.get(key, raw?(options))
       rescue MemCache::MemCacheError => e
         logger.error("MemCacheError (#{e}): #{e.message}")
         nil
@@ -91,7 +64,6 @@ module ActiveSupport
         # memcache-client will break the connection if you send it an integer
         # in raw mode, so we convert it to a string to be sure it continues working.
         value = value.to_s if raw?(options)
-        local_cache.write(key, value || NULL) if local_cache
         response = @data.send(method, key, value, expires_in(options), raw?(options))
         response == Response::STORED
       rescue MemCache::MemCacheError => e
@@ -101,7 +73,6 @@ module ActiveSupport
 
       def delete(key, options = nil) # :nodoc:
         super
-        local_cache.write(key, NULL) if local_cache
         response = @data.delete(key, expires_in(options))
         response == Response::DELETED
       rescue MemCache::MemCacheError => e
@@ -113,40 +84,22 @@ module ActiveSupport
         # Doesn't call super, cause exist? in memcache is in fact a read
         # But who cares? Reading is very fast anyway
         # Local cache is checked first, if it doesn't know then memcache itself is read from
-        value = local_cache.read(key) if local_cache
-        if value == NULL
-          false
-        elsif value
-          true
-        else
         !read(key, options).nil?
       end
-      end
 
       def increment(key, amount = 1) # :nodoc:
         log("incrementing", key, amount)
 
         response = @data.incr(key, amount)
-        unless response == Response::NOT_FOUND
-          local_cache.write(key, response.to_s) if local_cache
-          response
-        else
-          nil
-        end
+        response == Response::NOT_FOUND ? nil : response
       rescue MemCache::MemCacheError
         nil
       end
 
       def decrement(key, amount = 1) # :nodoc:
         log("decrement", key, amount)
-
         response = @data.decr(key, amount)
-        unless response == Response::NOT_FOUND
-          local_cache.write(key, response.to_s) if local_cache
-          response
-        else
-          nil
-        end
+        response == Response::NOT_FOUND ? nil : response
       rescue MemCache::MemCacheError
         nil
       end
@@ -159,7 +112,6 @@ module ActiveSupport
       end
 
       def clear
-        local_cache.clear if local_cache
         @data.flush_all
       end
 
@@ -168,10 +120,6 @@ module ActiveSupport
       end
 
       private
-        def local_cache
-          Thread.current[THREAD_LOCAL_KEY]
-        end
-
         def expires_in(options)
           (options && options[:expires_in]) || 0
         end

activesupport/lib/active_support/cache/strategy/local_cache.rb

@@ -0,0 +1,104 @@
+module ActiveSupport
+  module Cache
+    module Strategy
+      module LocalCache
+        # this allows caching of the fact that there is nothing in the remote cache
+        NULL = 'remote_cache_store:null'
+
+        def with_local_cache
+          Thread.current[thread_local_key] = MemoryStore.new
+          yield
+        ensure
+          Thread.current[thread_local_key] = nil
+        end
+
+        def middleware
+          @middleware ||= begin
+            klass = Class.new
+            klass.class_eval(<<-EOS, __FILE__, __LINE__)
+              def initialize(app)
+                @app = app
+              end
+
+              def call(env)
+                Thread.current[:#{thread_local_key}] = MemoryStore.new
+                @app.call(env)
+              ensure
+                Thread.current[:#{thread_local_key}] = nil
+              end
+            EOS
+            klass
+          end
+        end
+
+        def read(key, options = nil)
+          value = local_cache && local_cache.read(key)
+          if value == NULL
+            nil
+          elsif value.nil?
+            value = super
+            local_cache.write(key, value || NULL) if local_cache
+            value
+          else
+            # forcing the value to be immutable
+            value.dup
+          end
+        end
+
+        def write(key, value, options = nil)
+          value = value.to_s if respond_to?(:raw?) && raw?(options)
+          local_cache.write(key, value || NULL) if local_cache
+          super
+        end
+
+        def delete(key, options = nil)
+          local_cache.write(key, NULL) if local_cache
+          super
+        end
+
+        def exist(key, options = nil)
+          value = local_cache.read(key) if local_cache
+          if value == NULL
+            false
+          elsif value
+            true
+          else
+            super
+          end
+        end
+
+        def increment(key, amount = 1)
+          if value = super
+            local_cache.write(key, value.to_s) if local_cache
+            value
+          else
+            nil
+          end
+        end
+
+        def decrement(key, amount = 1)
+          if value = super
+            local_cache.write(key, value.to_s) if local_cache
+            value
+          else
+            nil
+          end
+        end
+
+        def clear
+          local_cache.clear if local_cache
+          super
+        end
+
+        private
+          def thread_local_key
+            @thread_local_key ||= "#{self.class.name.underscore}_local_cache".gsub("/", "_").to_sym
+          end
+
+          def local_cache
+            Thread.current[thread_local_key]
+          end
+      end
+    end
+  end
+end

activesupport/test/caching_test.rb

@@ -161,7 +161,7 @@ uses_memcached 'memcached backed store' do
     include CacheStoreBehavior
 
     def test_store_objects_should_be_immutable
-      with_local_cache do
+      @cache.with_local_cache do
         @cache.write('foo', 'bar')
         @cache.read('foo').gsub!(/.*/, 'baz')
         assert_equal 'bar', @cache.read('foo')
@@ -169,7 +169,7 @@ uses_memcached 'memcached backed store' do
     end
 
     def test_write_should_return_true_on_success
-      with_local_cache do
+      @cache.with_local_cache do
         result = @cache.write('foo', 'bar')
         assert_equal 'bar', @cache.read('foo') # make sure 'foo' was written
         assert result
@@ -177,7 +177,7 @@ uses_memcached 'memcached backed store' do
     end
 
     def test_local_writes_are_persistent_on_the_remote_cache
-      with_local_cache do
+      @cache.with_local_cache do
         @cache.write('foo', 'bar')
       end
 
@@ -185,7 +185,7 @@ uses_memcached 'memcached backed store' do
     end
 
     def test_clear_also_clears_local_cache
-      with_local_cache do
+      @cache.with_local_cache do
         @cache.write('foo', 'bar')
         @cache.clear
         assert_nil @cache.read('foo')
@@ -193,7 +193,7 @@ uses_memcached 'memcached backed store' do
     end
 
     def test_local_cache_of_read_and_write
-      with_local_cache do
+      @cache.with_local_cache do
         @cache.write('foo', 'bar')
         @data.flush_all # Clear remote cache
         assert_equal 'bar', @cache.read('foo')
@@ -201,7 +201,7 @@ uses_memcached 'memcached backed store' do
     end
 
     def test_local_cache_of_delete
-      with_local_cache do
+      @cache.with_local_cache do
         @cache.write('foo', 'bar')
         @cache.delete('foo')
         @data.flush_all # Clear remote cache
@@ -210,7 +210,7 @@ uses_memcached 'memcached backed store' do
     end
 
     def test_local_cache_of_exist
-      with_local_cache do
+      @cache.with_local_cache do
         @cache.write('foo', 'bar')
         @cache.instance_variable_set(:@data, nil)
         @data.flush_all # Clear remote cache
@@ -219,7 +219,7 @@ uses_memcached 'memcached backed store' do
     end
 
     def test_local_cache_of_increment
-      with_local_cache do
+      @cache.with_local_cache do
         @cache.write('foo', 1, :raw => true)
         @cache.increment('foo')
         @data.flush_all # Clear remote cache
@@ -228,7 +228,7 @@ uses_memcached 'memcached backed store' do
     end
 
     def test_local_cache_of_decrement
-      with_local_cache do
+      @cache.with_local_cache do
         @cache.write('foo', 1, :raw => true)
         @cache.decrement('foo')
         @data.flush_all # Clear remote cache
@@ -237,19 +237,21 @@ uses_memcached 'memcached backed store' do
     end
 
     def test_exist_with_nulls_cached_locally
-      with_local_cache do
+      @cache.with_local_cache do
         @cache.write('foo', 'bar')
         @cache.delete('foo')
         assert !@cache.exist?('foo')
       end
     end
 
-    private
-      def with_local_cache
-        Thread.current[ActiveSupport::Cache::MemCacheStore::THREAD_LOCAL_KEY] = ActiveSupport::Cache::MemoryStore.new
-        yield
-      ensure
-        Thread.current[ActiveSupport::Cache::MemCacheStore::THREAD_LOCAL_KEY] = nil
+    def test_middleware
+      app = lambda { |env|
+        result = @cache.write('foo', 'bar')
+        assert_equal 'bar', @cache.read('foo') # make sure 'foo' was written
+        assert result
+      }
+      app = @cache.middleware.new(app)
+      app.call({})
     end
   end
 

railties/lib/initializer.rb

@@ -414,8 +414,10 @@ Run `rake gems:install` to install the missing gems.
     def initialize_cache
       unless defined?(RAILS_CACHE)
         silence_warnings { Object.const_set "RAILS_CACHE", ActiveSupport::Cache.lookup_store(configuration.cache_store) }
-        if RAILS_CACHE.class.name == "ActiveSupport::Cache::MemCacheStore"
-          configuration.middleware.insert_after(:"ActionController::Failsafe", ActiveSupport::Cache::MemCacheStore::LocalCache)
+
+        if RAILS_CACHE.respond_to?(:middleware)
+          # Insert middleware to setup and teardown local cache for each request
+          configuration.middleware.insert_after(:"ActionController::Failsafe", RAILS_CACHE.middleware)
         end
       end
     end