Skip to content

Commit

Permalink
Don't clear the local cache on each requests with with_local_cache:
Browse files Browse the repository at this point in the history
- The `ActiveSupport::LocalCache::Middleware` clears the local cache
  after each requests, but when using the `with_local_cache` method,
  this is counter intuitive as this method is supposed to use a
  cache for the duration of the block:

  ```ruby
    Rails.cache.with_local_cache do
      get "/"
      # Before this patch: Cache is cleared here
    end
    # After this patch: Cache is cleared here
  ```
  • Loading branch information
Edouard-chin committed Feb 15, 2024
1 parent 6e7ef7d commit e89b5a6
Show file tree
Hide file tree
Showing 5 changed files with 146 additions and 3 deletions.
15 changes: 15 additions & 0 deletions activesupport/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,18 @@
* `ActiveSupport::Cache#with_local_cache` now prevents the local cache
from being cleared after each requests.

This is most useful for testing purposes to assert the state
of the cache without to temporary change the cache strategy.
For example:

```ruby
Rails.cache.with_local_cache do
get "/customers"

assert_equal("", Rails.cache.read("cache_key"))
end
```

* Include `IPAddr#prefix` when serializing an `IPAddr` using the
`ActiveSupport::MessagePack` serializer. This change is backward and forward
compatible — old payloads can still be read, and new payloads will be
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,12 @@ def fetch_entry(key) # :nodoc:

# Use a local cache for the duration of block.
def with_local_cache(&block)
clear_cache = middleware.clear_cache?
middleware.clear_cache = false

use_temporary_local_cache(LocalStore.new, &block)
ensure
middleware.clear_cache = clear_cache
end

# Middleware class can be inserted as a Rack handler to be local cache for the
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ def initialize(name, local_cache_key)
@name = name
@local_cache_key = local_cache_key
@app = nil
self.clear_cache = true
end

def new(app)
Expand All @@ -25,18 +26,28 @@ def new(app)
end

def call(env)
LocalCacheRegistry.set_cache_for(local_cache_key, LocalStore.new)
LocalCacheRegistry.cache_for(local_cache_key) ||
LocalCacheRegistry.set_cache_for(local_cache_key, LocalStore.new)

response = @app.call(env)
response[2] = ::Rack::BodyProxy.new(response[2]) do
LocalCacheRegistry.set_cache_for(local_cache_key, nil)
LocalCacheRegistry.set_cache_for(local_cache_key, nil) if clear_cache?
end
cleanup_on_body_close = true
response
rescue Rack::Utils::InvalidParameterError
[400, {}, []]
ensure
LocalCacheRegistry.set_cache_for(local_cache_key, nil) unless
cleanup_on_body_close
cleanup_on_body_close || !clear_cache?
end

def clear_cache=(value)
ActiveSupport::IsolatedExecutionState[:active_support_local_cache_middleware_clear] = value
end

def clear_cache?
ActiveSupport::IsolatedExecutionState[:active_support_local_cache_middleware_clear]
end
end
end
Expand Down
37 changes: 37 additions & 0 deletions activesupport/test/cache/local_cache_middleware_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,43 @@ def test_local_cache_cleared_on_throw
assert_throws(:warden) { middleware.call({}) }
assert_nil LocalCacheRegistry.cache_for(key)
end

def test_local_cache_not_cleared_on_close_when_clear_cache_is_false
key = "super awesome key"
assert_nil LocalCacheRegistry.cache_for key
middleware = Middleware.new("<3", key).new(->(env) {
assert LocalCacheRegistry.cache_for(key), "should have a cache"
[200, {}, []]
})
_, _, body = middleware.call({})
assert LocalCacheRegistry.cache_for(key), "should still have a cache"
body.each { }
assert LocalCacheRegistry.cache_for(key), "should still have a cache"

clear_cache = middleware.clear_cache?
middleware.clear_cache = false
body.close
assert LocalCacheRegistry.cache_for(key), "should still have a cache"
ensure
LocalCacheRegistry.set_cache_for(key, nil)
middleware.clear_cache = clear_cache
end

def test_local_cache_no_cleared_on_exception_when_clear_cache_is_false
key = "super awesome key"
assert_nil LocalCacheRegistry.cache_for key
middleware = Middleware.new("<3", key).new(->(env) {
assert LocalCacheRegistry.cache_for(key), "should have a cache"
raise
})
clear_cache = middleware.clear_cache?
middleware.clear_cache = false
assert_raises(RuntimeError) { middleware.call({}) }
assert LocalCacheRegistry.cache_for(key), "should still have a cache"
ensure
LocalCacheRegistry.set_cache_for(key, nil)
middleware.clear_cache = clear_cache
end
end
end
end
Expand Down
75 changes: 75 additions & 0 deletions railties/test/application/integration_test_case_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -73,4 +73,79 @@ def test_app_returns_action_dispatch_test_app_by_default
assert_match(/0 failures, 0 errors/, output)
end
end

class LocalCacheTest < ActiveSupport::TestCase
include ActiveSupport::Testing::Isolation, EnvHelpers

setup :build_app
teardown :teardown_app

test "with_local_cache isn't cleared between requests" do
app_file "config/routes.rb", <<-RUBY
Rails.application.routes.draw do
get :customer, to: "customer#show"
get :cached_customer, to: "customer#show_with_cache"
end
RUBY

app_file "app/controllers/customer_controller.rb", <<-RUBY
class CustomerController < ApplicationController
def show
Rails.cache.fetch("customer") { "David" }
head(:ok)
end
def show_with_cache
if customer = Rails.cache.read("customer")
render(plain: customer, status: :ok)
else
render(plain: "Cache miss", status: :bad_request)
end
end
end
RUBY

app_file "test/integration/customer_integration_test.rb", <<-RUBY
require "test_helper"
class CustomerIntegrationTest < ActionDispatch::IntegrationTest
def test_cache_is_cleared_on_each_request
get("/customer")
assert_response(:ok)
assert_nil(Rails.cache.read("customer"), "The cache was not cleared")
end
def test_cache_is_not_cleared_after_the_request
Rails.cache.with_local_cache do
get("/customer")
assert_response(:ok)
assert_equal("David", Rails.cache.read("customer"))
end
assert_nil(Rails.cache.read("customer"), "The cache was not cleared")
end
def test_cache_is_hit
Rails.cache.with_local_cache do
get("/customer")
assert_response(:ok)
assert_equal("David", Rails.cache.read("customer"))
get("/cached_customer")
assert_response(:ok)
assert_equal("David", response.body)
end
assert_nil(Rails.cache.read("customer"), "The cache was not cleared")
end
end
RUBY

with_rails_env("test") { rails("db:migrate") }
output = rails("test")
assert_match(/3 runs, 10 assertions, 0 failures, 0 errors/, output)
end
end
end

0 comments on commit e89b5a6

Please sign in to comment.