Merge pull request #64 from Shopify/fix-rails-7-local-cache

Fix compatibility with Rails edge

.github/workflows/ci.yml

@@ -14,6 +14,9 @@ jobs:
           - name: 'Latest released'
             ruby: '3.0'
             gemfile: "Gemfile"
+          - name: 'Rails edge'
+            ruby: '3.0'
+            gemfile: "Gemfile.edge"
 
     name: ${{ matrix.entry.name }}
 

Gemfile.edge

@@ -0,0 +1,6 @@
+source 'https://rubygems.org'
+
+gemspec
+
+gem "rails", github: "rails/rails", branch: "main"
+gem "snappy"

lib/active_support/cache/memcached_store.rb

@@ -212,22 +212,82 @@ def reset #:nodoc:
         end
       end
 
-      protected
+      private
 
-      def read_entry(key, _options) # :nodoc:
-        handle_exceptions(return_value_on_error: nil) do
-          deserialize_entry(@connection.get(key))
+      if private_method_defined?(:read_serialized_entry)
+        class LocalStore < Strategy::LocalCache::LocalStore
+          def write_entry(_key, entry)
+            if entry.is_a?(Entry)
+              entry.dup_value! 
+            end
+            super
+          end
+
+          def fetch_entry(key)
+            entry = @data.fetch(key) do
+              new_entry = yield
+              if entry.is_a?(Entry)
+                new_entry.dup_value!
+              end
+              @data[key] = new_entry
+            end
+            entry = entry.dup
+
+            if entry.is_a?(Entry)
+              entry.dup_value!
+            end
+
+            entry
+          end
         end
-      end
 
-      def write_entry(key, entry, options) # :nodoc:
-        return true if read_only
-        method = options && options[:unless_exist] ? :add : :set
-        expires_in = expiration(options)
-        value = serialize_entry(entry, options)
-        handle_exceptions(return_value_on_error: false) do
-          @connection.send(method, key, value, expires_in)
-          true
+        module LocalCacheDup
+          def with_local_cache
+            use_temporary_local_cache(LocalStore.new) { yield }
+          end
+        end
+        prepend LocalCacheDup
+
+        def read_entry(key, **options) # :nodoc:
+          deserialize_entry(read_serialized_entry(key, **options))
+        end
+
+        def read_serialized_entry(key, **)
+          handle_exceptions(return_value_on_error: nil) do
+            @connection.get(key)
+          end
+        end
+
+        def write_entry(key, entry, **options) # :nodoc:
+          return true if read_only
+
+          write_serialized_entry(key, serialize_entry(entry, **options), **options)
+        end
+
+        def write_serialized_entry(key, value, **options)
+          method = options && options[:unless_exist] ? :add : :set
+          expires_in = expiration(options)
+          handle_exceptions(return_value_on_error: false) do
+            @connection.send(method, key, value, expires_in)
+            true
+          end
+        end
+      else
+        def read_entry(key, _options) # :nodoc:
+          handle_exceptions(return_value_on_error: nil) do
+            deserialize_entry(@connection.get(key))
+          end
+        end
+
+        def write_entry(key, entry, options) # :nodoc:
+          return true if read_only
+          method = options && options[:unless_exist] ? :add : :set
+          expires_in = expiration(options)
+          value = serialize_entry(entry, options)
+          handle_exceptions(return_value_on_error: false) do
+            @connection.send(method, key, value, expires_in)
+            true
+          end
         end
       end
 

test/support/local_cache_behavior.rb

@@ -0,0 +1,209 @@
+
+module LocalCacheBehavior
+  def test_local_writes_are_persistent_on_the_remote_cache
+    retval = @cache.with_local_cache do
+      @cache.write("foo", "bar")
+    end
+    assert retval
+    assert_equal "bar", @cache.read("foo")
+  end
+
+  def test_clear_also_clears_local_cache
+    @cache.with_local_cache do
+      @cache.write("foo", "bar")
+      @cache.clear
+      assert_nil @cache.read("foo")
+    end
+
+    assert_nil @cache.read("foo")
+  end
+
+  def test_cleanup_clears_local_cache_but_not_remote_cache
+    begin
+      @cache.cleanup
+    rescue NotImplementedError
+      skip
+    end
+
+    @cache.with_local_cache do
+      @cache.write("foo", "bar")
+      assert_equal "bar", @cache.read("foo")
+
+      @cache.send(:bypass_local_cache) { @cache.write("foo", "baz") }
+      assert_equal "bar", @cache.read("foo")
+
+      @cache.cleanup
+      assert_equal "baz", @cache.read("foo")
+    end
+  end
+
+  def test_local_cache_of_write
+    @cache.with_local_cache do
+      @cache.write("foo", "bar")
+      @peek.delete("foo")
+      assert_equal "bar", @cache.read("foo")
+    end
+  end
+
+  def test_local_cache_of_read_returns_a_copy_of_the_entry
+    skip if ActiveSupport.gem_version < Gem::Version.new('6.1')
+
+    @cache.with_local_cache do
+      @cache.write(:foo, type: "bar")
+      value = @cache.read(:foo)
+      assert_equal("bar", value.delete(:type))
+      assert_equal({ type: "bar" }, @cache.read(:foo))
+    end
+  end
+
+  def test_local_cache_of_read
+    @cache.write("foo", "bar")
+    @cache.with_local_cache do
+      assert_equal "bar", @cache.read("foo")
+    end
+  end
+
+  def test_local_cache_of_read_nil
+    @cache.with_local_cache do
+      assert_nil @cache.read("foo")
+      @cache.send(:bypass_local_cache) { @cache.write "foo", "bar" }
+      assert_nil @cache.read("foo")
+    end
+  end
+
+  def test_local_cache_of_write_nil
+    @cache.with_local_cache do
+      assert @cache.write("foo", nil)
+      assert_nil @cache.read("foo")
+      @peek.write("foo", "bar")
+      assert_nil @cache.read("foo")
+    end
+  end
+
+  def test_local_cache_of_write_with_unless_exist
+    @cache.with_local_cache do
+      @cache.write("foo", "bar")
+      @cache.write("foo", "baz", unless_exist: true)
+      assert_equal @peek.read("foo"), @cache.read("foo")
+    end
+  end
+
+  def test_local_cache_of_delete
+    @cache.with_local_cache do
+      @cache.write("foo", "bar")
+      @cache.delete("foo")
+      assert_nil @cache.read("foo")
+    end
+  end
+
+  def test_local_cache_of_delete_matched
+    begin
+      @cache.delete_matched("*")
+    rescue NotImplementedError
+      skip
+    end
+
+    @cache.with_local_cache do
+      @cache.write("foo", "bar")
+      @cache.write("fop", "bar")
+      @cache.write("bar", "foo")
+      @cache.delete_matched("fo*")
+      assert_not @cache.exist?("foo")
+      assert_not @cache.exist?("fop")
+      assert_equal "foo", @cache.read("bar")
+    end
+  end
+
+  def test_local_cache_of_exist
+    @cache.with_local_cache do
+      @cache.write("foo", "bar")
+      @peek.delete("foo")
+      assert @cache.exist?("foo")
+    end
+  end
+
+  def test_local_cache_of_increment
+    @cache.with_local_cache do
+      @cache.write("foo", 1, raw: true)
+      @peek.write("foo", 2, raw: true)
+      @cache.increment("foo")
+      assert_equal 3, Integer(@cache.read("foo", raw: true))
+    end
+  end
+
+  def test_local_cache_of_decrement
+    @cache.with_local_cache do
+      @cache.write("foo", 1, raw: true)
+      @peek.write("foo", 3, raw: true)
+
+      @cache.decrement("foo")
+      assert_equal 2, Integer(@cache.read("foo", raw: true))
+    end
+  end
+
+  def test_local_cache_of_fetch_multi
+    @cache.with_local_cache do
+      @cache.fetch_multi("foo", "bar") { |_key| true }
+      @peek.delete("foo")
+      @peek.delete("bar")
+      assert_equal true, @cache.read("foo")
+      assert_equal true, @cache.read("bar")
+    end
+  end
+
+  def test_local_cache_of_read_multi
+    @cache.with_local_cache do
+      @cache.write("foo", "foo", raw: true)
+      @cache.write("bar", "bar", raw: true)
+      values = @cache.read_multi("foo", "bar", raw: true)
+      assert_equal "foo", @cache.read("foo", raw: true)
+      assert_equal "bar", @cache.read("bar", raw: true)
+      assert_equal "foo", values["foo"]
+      assert_equal "bar", values["bar"]
+    end
+  end
+
+  def test_initial_object_mutation_after_write
+    skip if ActiveSupport.gem_version < Gem::Version.new('6.1')
+
+    @cache.with_local_cache do
+      initial = +"bar"
+      @cache.write("foo", initial)
+      initial << "baz"
+      assert_equal "bar", @cache.read("foo")
+    end
+  end
+
+  def test_initial_object_mutation_after_fetch
+    skip if ActiveSupport.gem_version < Gem::Version.new('6.1')
+
+    @cache.with_local_cache do
+      initial = +"bar"
+      @cache.fetch("foo") { initial }
+      initial << "baz"
+      assert_equal "bar", @cache.read("foo")
+      assert_equal "bar", @cache.fetch("foo")
+    end
+  end
+
+  def test_local_race_condition_protection
+    @cache.with_local_cache do
+      time = Time.now
+      @cache.write("foo", "bar", expires_in: 60)
+      Time.stub(:now, time + 61) do
+        result = @cache.fetch("foo", race_condition_ttl: 10) do
+          assert_equal "bar", @cache.read("foo")
+          "baz"
+        end
+        assert_equal "baz", result
+      end
+    end
+  end
+
+  def test_local_cache_should_read_and_write_false
+    @cache.with_local_cache do
+      assert @cache.write("foo", false)
+      assert_equal false, @cache.read("foo")
+    end
+  end
+end

test/test_helper.rb

@@ -9,5 +9,6 @@
 require 'active_support/test_case'
 
 require_relative 'support/rails'
+require_relative 'support/local_cache_behavior'
 
 require 'memcached_store'

test/test_memcached_store.rb

@@ -2,9 +2,12 @@
 require 'logger'
 
 class TestMemcachedStore < ActiveSupport::TestCase
+  include LocalCacheBehavior
+
   setup do
     @cache = ActiveSupport::Cache.lookup_store(:memcached_store, expires_in: 60, support_cas: true)
     @cache.clear
+    @peek = ActiveSupport::Cache.lookup_store(:memcached_store, expires_in: 60, support_cas: true)
 
     # Enable ActiveSupport notifications. Can be disabled in Rails 5.
     Thread.current[:instrument_cache_store] = true
@@ -778,7 +781,12 @@ def test_raw_option_not_needed_on_read
   end
 
   def test_uncompress_regression
-    value = "bar" * ActiveSupport::Cache::Entry::DEFAULT_COMPRESS_LIMIT
+    limit = if defined? ActiveSupport::Cache::Entry::DEFAULT_COMPRESS_LIMIT
+      ActiveSupport::Cache::Entry::DEFAULT_COMPRESS_LIMIT
+    else
+      ActiveSupport::Cache::DEFAULT_COMPRESS_LIMIT
+    end
+    value = "bar" * limit
     Zlib::Deflate.expects(:deflate).never
     Zlib::Inflate.expects(:inflate).never