Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Cleanup application after #close has been called on the Rack response…

… body, not when AC::Reload#call is done.

The Rack body might lazily evaluate its output, which is for example the case
if one calls 'render :text => lambda { ... }'. The code which lazily evaluates
the output might use other application classes. So we will want to defer
cleanup until the Rack request is completely finished.

Signed-off-by: Michael Koziarski <michael@koziarski.com>
  • Loading branch information...
commit bc2c4a45959be21e6314fba7876b32c1f04cd08a 1 parent 29c5985
@FooBarWidget FooBarWidget authored NZKoz committed
View
37 actionpack/lib/action_controller/reloader.rb
@@ -1,14 +1,45 @@
module ActionController
class Reloader
+ class BodyWrapper
+ def initialize(body)
+ @body = body
+ end
+
+ def close
+ @body.close if @body.respond_to?(:close)
+ ensure
+ Dispatcher.cleanup_application
+ end
+
+ def method_missing(*args, &block)
+ @body.send(*args, &block)
+ end
+
+ def respond_to?(symbol, include_private = false)
+ symbol == :close || @body.respond_to?(symbol, include_private)
+ end
+ end
+
def initialize(app)
@app = app
end
def call(env)
Dispatcher.reload_application
- @app.call(env)
- ensure
- Dispatcher.cleanup_application
+ status, headers, body = @app.call(env)
+ # We do not want to call 'cleanup_application' in an ensure block
+ # because the returned Rack response body may lazily generate its data. This
+ # is for example the case if one calls
+ #
+ # render :text => lambda { ... code here which refers to application models ... }
+ #
+ # in an ActionController.
+ #
+ # Instead, we will want to cleanup the application code after the request is
+ # completely finished. So we wrap the body in a BodyWrapper class so that
+ # when the Rack handler calls #close during the end of the request, we get to
+ # run our cleanup code.
+ [status, headers, BodyWrapper.new(body)]
end
end
end
View
97 actionpack/test/controller/reloader_test.rb
@@ -0,0 +1,97 @@
+require 'abstract_unit'
+
+class ReloaderTests < ActiveSupport::TestCase
+ Reloader = ActionController::Reloader
+ Dispatcher = ActionController::Dispatcher
+
+ class MyBody < Array
+ def initialize(&block)
+ @on_close = block
+ end
+
+ def foo
+ "foo"
+ end
+
+ def bar
+ "bar"
+ end
+
+ def close
+ @on_close.call if @on_close
+ end
+ end
+
+ def setup_and_return_body(app = lambda { })
+ Dispatcher.expects(:reload_application)
+ reloader = Reloader.new(app)
+ headers, status, body = reloader.call({ })
+ body
+ end
+
+ def test_it_reloads_the_application_before_the_request
+ Dispatcher.expects(:reload_application)
+ reloader = Reloader.new(lambda {
+ [200, { "Content-Type" => "text/html" }, [""]]
+ })
+ reloader.call({ })
+ end
+
+ def test_returned_body_object_always_responds_to_close
+ body = setup_and_return_body(lambda {
+ [200, { "Content-Type" => "text/html" }, [""]]
+ })
+ assert body.respond_to?(:close)
+ end
+
+ def test_returned_body_object_behaves_like_underlying_object
+ body = setup_and_return_body(lambda {
+ b = MyBody.new
+ b << "hello"
+ b << "world"
+ [200, { "Content-Type" => "text/html" }, b]
+ })
+ assert_equal 2, body.size
+ assert_equal "hello", body[0]
+ assert_equal "world", body[1]
+ assert_equal "foo", body.foo
+ assert_equal "bar", body.bar
+ end
+
+ def test_it_calls_close_on_underlying_object_when_close_is_called_on_body
+ close_called = false
+ body = setup_and_return_body(lambda {
+ b = MyBody.new do
+ close_called = true
+ end
+ [200, { "Content-Type" => "text/html" }, b]
+ })
+ body.close
+ assert close_called
+ end
+
+ def test_returned_body_object_responds_to_all_methods_supported_by_underlying_object
+ body = setup_and_return_body(lambda {
+ [200, { "Content-Type" => "text/html" }, MyBody.new]
+ })
+ assert body.respond_to?(:size)
+ assert body.respond_to?(:each)
+ assert body.respond_to?(:foo)
+ assert body.respond_to?(:bar)
+ end
+
+ def test_it_doesnt_clean_up_the_application_after_call
+ Dispatcher.expects(:cleanup_application).never
+ body = setup_and_return_body(lambda {
+ [200, { "Content-Type" => "text/html" }, MyBody.new]
+ })
+ end
+
+ def test_it_cleans_up_the_application_when_close_is_called_on_body
+ Dispatcher.expects(:cleanup_application)
+ body = setup_and_return_body(lambda {
+ [200, { "Content-Type" => "text/html" }, MyBody.new]
+ })
+ body.close
+ end
+end

2 comments on commit bc2c4a4

@NZKoz
Owner

Thanks for the report, I've reopened the original one.

I believe this is going to need to be reverted.

Please sign in to comment.
Something went wrong with that request. Please try again.