Skip to content

Commit

Permalink
Optimized Cache::Entry should support old dalli_store values
Browse files Browse the repository at this point in the history
Same bug as rails#42559, but a very different fix due to rails#42025. To recap, the issue:

- While using the `dalli_store`, you set any value in the Rails cache with no expiry.
- You change to the `mem_cache_store`.
- You upgrade to Rails 7.
- You try to read the same cache key you set in step 1, and it crashes on read.

rails#42025 was backward compatible with entries written using the `mem_cache_store`, but *not* the `dalli_store` which did not use `ActiveSupport::Cache::Entry`. This PR attempts to fix that.
  • Loading branch information
ghiculescu committed Jun 22, 2021
1 parent 935cf8d commit 25b47c2
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 2 deletions.
10 changes: 8 additions & 2 deletions activesupport/lib/active_support/cache.rb
Expand Up @@ -844,14 +844,20 @@ module Loader
extend self

def load(payload)
if payload.start_with?(MARK_70_UNCOMPRESSED)
if !payload.is_a?(String)
ActiveSupport::Cache::Store.logger&.warn %{Payload wasn't a string, was #{payload.class.name} - couldn't unmarshal, so returning nil."}

return nil
elsif payload.start_with?(MARK_70_UNCOMPRESSED)
members = Marshal.load(payload.byteslice(1..-1))
elsif payload.start_with?(MARK_70_COMPRESSED)
members = Marshal.load(Zlib::Inflate.inflate(payload.byteslice(1..-1)))
elsif payload.start_with?(MARK_61)
return Marshal.load(payload)
else
raise ArgumentError, %{Invalid cache prefix: #{payload.byteslice(0).inspect}, expected "\\x00" or "\\x01"}
ActiveSupport::Cache::Store.logger&.warn %{Invalid cache prefix: #{payload.byteslice(0).inspect}, expected "\\x00" or "\\x01"}

return nil
end
Entry.unpack(members)
end
Expand Down
7 changes: 7 additions & 0 deletions activesupport/test/cache/behaviors/local_cache_behavior.rb
Expand Up @@ -228,4 +228,11 @@ def test_local_race_condition_protection
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
36 changes: 36 additions & 0 deletions activesupport/test/cache/stores/mem_cache_store_test.rb
Expand Up @@ -232,6 +232,42 @@ def test_large_object_with_default_compression_settings
assert_compressed(LARGE_OBJECT)
end

def test_can_load_raw_values_from_dalli_store
key = "test-with-value-the-way-the-dalli-store-did"

@cache.instance_variable_get(:@data).with { |c| c.set(@cache.send(:normalize_key, key, nil), "value", 0, compress: false) }
assert_nil @cache.read(key)
assert_equal "value", @cache.fetch(key) { "value" }
end

def test_can_load_raw_falsey_values_from_dalli_store
key = "test-with-false-value-the-way-the-dalli-store-did"

@cache.instance_variable_get(:@data).with { |c| c.set(@cache.send(:normalize_key, key, nil), false, 0, compress: false) }
assert_nil @cache.read(key)
assert_equal false, @cache.fetch(key) { false }
end

def test_can_load_raw_values_from_dalli_store_with_local_cache
key = "test-with-value-the-way-the-dalli-store-did-with-local-cache"

@cache.instance_variable_get(:@data).with { |c| c.set(@cache.send(:normalize_key, key, nil), "value", 0, compress: false) }
@cache.with_local_cache do
assert_nil @cache.read(key)
assert_equal "value", @cache.fetch(key) { "value" }
end
end

def test_can_load_raw_falsey_values_from_dalli_store_with_local_cache
key = "test-with-false-value-the-way-the-dalli-store-did-with-local-cache"

@cache.instance_variable_get(:@data).with { |c| c.set(@cache.send(:normalize_key, key, nil), false, 0, compress: false) }
@cache.with_local_cache do
assert_nil @cache.read(key)
assert_equal false, @cache.fetch(key) { false }
end
end

private
def random_string(length)
(0...length).map { (65 + rand(26)).chr }.join
Expand Down

0 comments on commit 25b47c2

Please sign in to comment.