From 91d274059536f09dffd87141e570c3eab9ebd9b4 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Thu, 12 Mar 2009 21:47:34 -0700 Subject: [PATCH] Return body parts directly to Rack rather than building a response string ourselves. Allows Rack middleware to orchestrate response building. --- actionpack/lib/action_controller/base.rb | 12 +++--- .../lib/action_controller/integration.rb | 10 +++-- actionpack/lib/action_controller/response.rb | 39 +++++++++++++------ .../lib/action_controller/test_process.rb | 4 +- actionpack/test/controller/rack_test.rb | 2 +- actionpack/test/controller/send_file_test.rb | 4 +- 6 files changed, 44 insertions(+), 27 deletions(-) diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index 0facf7066d..c6dd99e959 100644 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -984,6 +984,7 @@ module ActionController #:nodoc: # of sending it as the response body to the browser. def render_to_string(options = nil, &block) #:doc: render(options, &block) + response.body ensure response.content_type = nil erase_render_results @@ -1020,7 +1021,7 @@ module ActionController #:nodoc: # Clears the rendered results, allowing for another render to be performed. def erase_render_results #:nodoc: - response.body = nil + response.body = [] @performed_render = false end @@ -1247,13 +1248,12 @@ module ActionController #:nodoc: response.status = interpret_status(status || DEFAULT_RENDER_STATUS_CODE) if append_response - response.body ||= '' - response.body << text.to_s + response.body_parts << text.to_s else response.body = case text - when Proc then text - when nil then " " # Safari doesn't pass the headers of the return if the response is zero length - else text.to_s + when Proc then text + when nil then [" "] # Safari doesn't pass the headers of the return if the response is zero length + else [text.to_s] end end end diff --git a/actionpack/lib/action_controller/integration.rb b/actionpack/lib/action_controller/integration.rb index 1c05ab0bf6..4faa263e2d 100644 --- a/actionpack/lib/action_controller/integration.rb +++ b/actionpack/lib/action_controller/integration.rb @@ -332,11 +332,13 @@ module ActionController @cookies[name] = value end - @body = "" if body.is_a?(String) - @body << body + @body_parts = [body] + @body = body else - body.each { |part| @body << part } + @body_parts = [] + body.each { |part| @body_parts << part.to_s } + @body = @body_parts.join end if @controller = ActionController::Base.last_instantiation @@ -349,7 +351,7 @@ module ActionController @response = Response.new @response.status = status.to_s @response.headers.replace(@headers) - @response.body = @body + @response.body = @body_parts end # Decorate the response with the standard behavior of the diff --git a/actionpack/lib/action_controller/response.rb b/actionpack/lib/action_controller/response.rb index ccff473df0..c69223dd69 100644 --- a/actionpack/lib/action_controller/response.rb +++ b/actionpack/lib/action_controller/response.rb @@ -40,16 +40,30 @@ module ActionController # :nodoc: delegate :default_charset, :to => 'ActionController::Base' def initialize - @status = 200 + super @header = Rack::Utils::HeaderHash.new(DEFAULT_HEADERS) - - @writer = lambda { |x| @body << x } - @block = nil - - @body = "", @session, @assigns = [], [] end + def body + str = '' + each { |part| str << part.to_s } + str + end + + def body=(body) + @body = + if body.is_a?(String) + [body] + else + body + end + end + + def body_parts + @body + end + def location; headers['Location'] end def location=(url) headers['Location'] = url end @@ -152,7 +166,7 @@ module ActionController # :nodoc: @writer = lambda { |x| callback.call(x) } @body.call(self, self) elsif @body.is_a?(String) - @body.each_line(&callback) + callback.call(@body) else @body.each(&callback) end @@ -162,7 +176,8 @@ module ActionController # :nodoc: end def write(str) - @writer.call str.to_s + str = str.to_s + @writer.call str str end @@ -186,7 +201,7 @@ module ActionController # :nodoc: if request && request.etag_matches?(etag) self.status = '304 Not Modified' - self.body = '' + self.body = [] end set_conditional_cache_control! @@ -195,7 +210,7 @@ module ActionController # :nodoc: def nonempty_ok_response? ok = !status || status.to_s[0..2] == '200' - ok && body.is_a?(String) && !body.empty? + ok && !body_parts.respond_to?(:call) && body_parts.any? end def set_conditional_cache_control! @@ -216,8 +231,8 @@ module ActionController # :nodoc: headers.delete('Content-Length') elsif length = headers['Content-Length'] headers['Content-Length'] = length.to_s - elsif !body.respond_to?(:call) && (!status || status.to_s[0..2] != '304') - headers["Content-Length"] = (body.respond_to?(:bytesize) ? body.bytesize : body.size).to_s + elsif !body_parts.respond_to?(:call) && (!status || status.to_s[0..2] != '304') + headers["Content-Length"] = Rack::Utils.bytesize(body).to_s end end diff --git a/actionpack/lib/action_controller/test_process.rb b/actionpack/lib/action_controller/test_process.rb index dbaec00bee..9dd09c30b4 100644 --- a/actionpack/lib/action_controller/test_process.rb +++ b/actionpack/lib/action_controller/test_process.rb @@ -258,11 +258,11 @@ module ActionController #:nodoc: # Returns binary content (downloadable file), converted to a String def binary_content - raise "Response body is not a Proc: #{body.inspect}" unless body.kind_of?(Proc) + raise "Response body is not a Proc: #{body_parts.inspect}" unless body_parts.kind_of?(Proc) require 'stringio' sio = StringIO.new - body.call(self, sio) + body_parts.call(self, sio) sio.rewind sio.read diff --git a/actionpack/test/controller/rack_test.rb b/actionpack/test/controller/rack_test.rb index b550d3db78..89bf4fdacc 100644 --- a/actionpack/test/controller/rack_test.rb +++ b/actionpack/test/controller/rack_test.rb @@ -258,7 +258,7 @@ class RackResponseTest < BaseRackTest }, headers) parts = [] - body.each { |part| parts << part } + body.each { |part| parts << part.to_s } assert_equal ["0", "1", "2", "3", "4"], parts end end diff --git a/actionpack/test/controller/send_file_test.rb b/actionpack/test/controller/send_file_test.rb index a27e951929..3d1904fee9 100644 --- a/actionpack/test/controller/send_file_test.rb +++ b/actionpack/test/controller/send_file_test.rb @@ -44,12 +44,12 @@ class SendFileTest < ActionController::TestCase response = nil assert_nothing_raised { response = process('file') } assert_not_nil response - assert_kind_of Proc, response.body + assert_kind_of Proc, response.body_parts require 'stringio' output = StringIO.new output.binmode - assert_nothing_raised { response.body.call(response, output) } + assert_nothing_raised { response.body_parts.call(response, output) } assert_equal file_data, output.string end