From b1e0db9ec44d01a224c1cf5b5ff9c8364ebd1ff6 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Wed, 21 Jul 2021 16:50:28 +0200 Subject: [PATCH] Fix compatibility with Rails edge --- .github/workflows/ci.yml | 3 + Gemfile.edge | 6 + lib/active_support/cache/memcached_store.rb | 86 ++++++-- test/support/local_cache_behavior.rb | 209 ++++++++++++++++++++ test/test_helper.rb | 1 + test/test_memcached_store.rb | 10 +- 6 files changed, 301 insertions(+), 14 deletions(-) create mode 100644 Gemfile.edge create mode 100644 test/support/local_cache_behavior.rb diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fa2948a..d99ef66 100644 --- a/.github/workflows/ci.yml +++ b/.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 }} diff --git a/Gemfile.edge b/Gemfile.edge new file mode 100644 index 0000000..0e032a4 --- /dev/null +++ b/Gemfile.edge @@ -0,0 +1,6 @@ +source 'https://rubygems.org' + +gemspec + +gem "rails", github: "rails/rails", branch: "main" +gem "snappy" diff --git a/lib/active_support/cache/memcached_store.rb b/lib/active_support/cache/memcached_store.rb index aef2e1b..c8ddc21 100644 --- a/lib/active_support/cache/memcached_store.rb +++ b/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 diff --git a/test/support/local_cache_behavior.rb b/test/support/local_cache_behavior.rb new file mode 100644 index 0000000..9603a69 --- /dev/null +++ b/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 diff --git a/test/test_helper.rb b/test/test_helper.rb index 4ae4291..0803877 100644 --- a/test/test_helper.rb +++ b/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' diff --git a/test/test_memcached_store.rb b/test/test_memcached_store.rb index d18800e..9263d4d 100644 --- a/test/test_memcached_store.rb +++ b/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