Permalink
Browse files

Use a BodyProxy instead of including a Module that responds to close.

Closes #4441 if Active Record is disabled assets are delivered correctly
  • Loading branch information...
1 parent 4ca3592 commit c159b01b85ac3955c53cd6b8a62d5d90ee973cfb @spastorino spastorino committed Jan 13, 2012
View
4 actionpack/CHANGELOG.md
@@ -1,5 +1,9 @@
## Rails 3.2.0 (unreleased) ##
+* Use a BodyProxy instead of including a Module that responds to
+ close. Closes #4441 if Active Record is disabled assets are delivered
+ correctly *Santiago Pastorino*
+
* Rails initialization with initialize_on_precompile = false should set assets_dir *Santiago Pastorino*
* Add font_path helper method *Santiago Pastorino*
View
28 actionpack/lib/action_dispatch/middleware/body_proxy.rb
@@ -0,0 +1,28 @@
+# Keep this file meanwhile https://github.com/rack/rack/pull/313 is not released
@spastorino
Ruby on Rails member
spastorino added a line comment Jan 13, 2012

is not included in a release :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+module ActionDispatch
+ class BodyProxy
+ def initialize(body, &block)
+ @body, @block, @closed = body, block, false
+ end
+
+ def respond_to?(*args)
+ super or @body.respond_to?(*args)
+ end
+
+ def close
+ return if @closed
+ @closed = true
+ @body.close if @body.respond_to? :close
@josevalim
Ruby on Rails member
josevalim added a line comment Jan 14, 2012

Bro, it needs to be like the pull request I sent to rack. It needs to be:

begin
  @body.close
ensure
  @block.call
end

The current way will always call the block even if return if @closed returns.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ ensure
+ @block.call
+ end
+
+ def closed?
+ @closed
+ end
+
+ def method_missing(*args, &block)
+ @body.__send__(*args, &block)
+ end
+ end
+end
View
17 actionpack/lib/action_dispatch/middleware/reloader.rb
@@ -1,3 +1,5 @@
+require 'action_dispatch/middleware/body_proxy'
+
module ActionDispatch
# ActionDispatch::Reloader provides prepare and cleanup callbacks,
# intended to assist with code reloading during development.
@@ -61,7 +63,7 @@ def call(env)
@validated = @condition.call
prepare!
response = @app.call(env)
- response[2].extend(module_hook)
+ response[2] = ActionDispatch::BodyProxy.new(response[2]) { cleanup! }
response
rescue Exception
cleanup!
@@ -83,18 +85,5 @@ def cleanup! #:nodoc:
def validated? #:nodoc:
@validated
end
-
- def module_hook #:nodoc:
- middleware = self
- Module.new do
- define_method :close do
- begin
- super() if defined?(super)
- ensure
- middleware.cleanup!
- end
- end
- end
- end
end
end
View
13 actionpack/test/dispatch/reloader_test.rb
@@ -43,6 +43,16 @@ def test_returned_body_object_always_responds_to_close
assert_respond_to body, :close
end
+ def test_returned_body_object_always_responds_to_close_even_if_called_twice
+ body = call_and_return_body
+ assert_respond_to body, :close
+ body.close
+
+ body = call_and_return_body
+ assert_respond_to body, :close
+ body.close
+ end
+
def test_condition_specifies_when_to_reload
i, j = 0, 0, 0, 0
Reloader.to_prepare { |*args| i += 1 }
@@ -154,7 +164,8 @@ def test_cleanup_callbacks_are_called_on_exceptions
private
def call_and_return_body(&block)
- @reloader ||= Reloader.new(block || proc {[200, {}, 'response']})
+ @response ||= 'response'
+ @reloader ||= Reloader.new(block || proc {[200, {}, @response]})
@reloader.call({'rack.input' => StringIO.new('')})[2]
end
end
View
2 railties/guides/source/3_2_release_notes.textile
@@ -215,6 +215,8 @@ In the example above, Posts controller will no longer automatically look up for
h4. Action Dispatch
+* Use a BodyProxy instead of including a Module that responds to close. Closes #4441 if Active Record is disabled assets are delivered correctly.
+
* Added <tt>ActionDispatch::RequestId</tt> middleware that'll make a unique X-Request-Id header available to the response and enables the <tt>ActionDispatch::Request#uuid</tt> method. This makes it easy to trace requests from end-to-end in the stack and to identify individual requests in mixed logs like Syslog.
* The <tt>ShowExceptions</tt> middleware now accepts a exceptions application that is responsible to render an exception when the application fails. The application is invoked with a copy of the exception in +env["action_dispatch.exception"]+ and with the <tt>PATH_INFO</tt> rewritten to the status code.

0 comments on commit c159b01

Please sign in to comment.