Skip to content

Commit

Permalink
clear cache on body close so that cache remains during rendering
Browse files Browse the repository at this point in the history
fixes #13547

The body may use the local cache during rendering.  `call`ing the app
doesn't mean that rendering is finished, so we need to wait until
`close` is called on the body.
  • Loading branch information
tenderlove committed Jan 8, 2014
1 parent 2875b4a commit e176353
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 2 deletions.
10 changes: 8 additions & 2 deletions activesupport/lib/active_support/cache/strategy/local_cache.rb
Original file line number Original file line Diff line number Diff line change
@@ -1,5 +1,6 @@
require 'active_support/core_ext/object/duplicable' require 'active_support/core_ext/object/duplicable'
require 'active_support/core_ext/string/inflections' require 'active_support/core_ext/string/inflections'
require 'rack/body_proxy'

This comment has been minimized.

Copy link
@arthurnn

arthurnn Feb 23, 2014

Member

AS::Cache::Strategy::LocalCache now is rack dependent. Not sure if thats good =(



module ActiveSupport module ActiveSupport
module Cache module Cache
Expand Down Expand Up @@ -83,9 +84,14 @@ def new(app)


def call(env) def call(env)
LocalCacheRegistry.set_cache_for(local_cache_key, LocalStore.new) LocalCacheRegistry.set_cache_for(local_cache_key, LocalStore.new)
@app.call(env) response = @app.call(env)
ensure response[2] = ::Rack::BodyProxy.new(response[2]) do
LocalCacheRegistry.set_cache_for(local_cache_key, nil)
end
response
rescue Exception
LocalCacheRegistry.set_cache_for(local_cache_key, nil) LocalCacheRegistry.set_cache_for(local_cache_key, nil)
raise
end end
end end


Expand Down
37 changes: 37 additions & 0 deletions activesupport/test/caching_test.rb
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -3,6 +3,42 @@
require 'active_support/cache' require 'active_support/cache'
require 'dependencies_test_helpers' require 'dependencies_test_helpers'


module ActiveSupport
module Cache
module Strategy
module LocalCache
class MiddlewareTest < ActiveSupport::TestCase
def test_local_cache_cleared_on_close
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'
body.close
assert_nil LocalCacheRegistry.cache_for(key)
end

def test_local_cache_cleared_on_exception
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
})
assert_raises(RuntimeError) { middleware.call({}) }
assert_nil LocalCacheRegistry.cache_for(key)
end
end
end
end
end
end

class CacheKeyTest < ActiveSupport::TestCase class CacheKeyTest < ActiveSupport::TestCase
def test_entry_legacy_optional_ivars def test_entry_legacy_optional_ivars
legacy = Class.new(ActiveSupport::Cache::Entry) do legacy = Class.new(ActiveSupport::Cache::Entry) do
Expand Down Expand Up @@ -577,6 +613,7 @@ def test_middleware
result = @cache.write('foo', 'bar') result = @cache.write('foo', 'bar')
assert_equal 'bar', @cache.read('foo') # make sure 'foo' was written assert_equal 'bar', @cache.read('foo') # make sure 'foo' was written
assert result assert result
[200, {}, []]
} }
app = @cache.middleware.new(app) app = @cache.middleware.new(app)
app.call({}) app.call({})
Expand Down

0 comments on commit e176353

Please sign in to comment.