move file sending to the response object

Just a slight refactor that delegates file sending to the response
object.  This gives us the advantage that if a webserver (in the future)
provides a response object that knows how to do accelerated file
serving, it can implement this method.

6 files changed, 50 insertions, 30 deletions

diff --git a/actionpack/lib/abstract_controller/rendering.rb b/actionpack/lib/abstract_controller/rendering.rb
index 78b43f2fbe..824a383f79 100644
--- a/actionpack/lib/abstract_controller/rendering.rb
+++ b/actionpack/lib/abstract_controller/rendering.rb
@@ -22,13 +22,13 @@ module AbstractController
     # :api: public
     def render(*args, &block)
       options = _normalize_render(*args, &block)
-      self.response_body = render_to_body(options)
+      rendered_body = render_to_body(options)
       if options[:html]
         _set_html_content_type
       else
         _set_rendered_content_type rendered_format
       end
-      self.response_body
+      self.response_body = rendered_body
     end
 
     # Raw rendering of a template to a string.
diff --git a/actionpack/lib/action_controller/metal.rb b/actionpack/lib/action_controller/metal.rb
index beeaae9d0c..94ec62ec6f 100644
--- a/actionpack/lib/action_controller/metal.rb
+++ b/actionpack/lib/action_controller/metal.rb
@@ -174,7 +174,11 @@ module ActionController
 
     def response_body=(body)
       body = [body] unless body.nil? || body.respond_to?(:each)
-      response.body = body
+      response.reset_body!
+      body.each { |part|
+        next if part.empty?
+        response.write part
+      }
       super
     end
 
diff --git a/actionpack/lib/action_controller/metal/data_streaming.rb b/actionpack/lib/action_controller/metal/data_streaming.rb
index e6d7f958bb..957e7a3019 100644
--- a/actionpack/lib/action_controller/metal/data_streaming.rb
+++ b/actionpack/lib/action_controller/metal/data_streaming.rb
@@ -72,31 +72,7 @@ module ActionController #:nodoc:
 
         self.status = options[:status] || 200
         self.content_type = options[:content_type] if options.key?(:content_type)
-        self.response_body = FileBody.new(path)
-      end
-
-      # Avoid having to pass an open file handle as the response body.
-      # Rack::Sendfile will usually intercept the response and uses
-      # the path directly, so there is no reason to open the file.
-      class FileBody #:nodoc:
-        attr_reader :to_path
-
-        def initialize(path)
-          @to_path = path
-        end
-
-        def body
-          File.binread(to_path)
-        end
-
-        # Stream the file's contents if Rack::Sendfile isn't present.
-        def each
-          File.open(to_path, 'rb') do |file|
-            while chunk = file.read(16384)
-              yield chunk
-            end
-          end
-        end
+        response.send_file path
       end
 
       # Sends the given binary data to the browser. This method is similar to
diff --git a/actionpack/lib/action_dispatch/http/response.rb b/actionpack/lib/action_dispatch/http/response.rb
index a27ff67114..d4a534d892 100644
--- a/actionpack/lib/action_dispatch/http/response.rb
+++ b/actionpack/lib/action_dispatch/http/response.rb
@@ -39,7 +39,7 @@ module ActionDispatch # :nodoc:
       end
 
       def []=(k,v)
-        if @response.committed?
+        if @response.sending? || @response.sent?
           raise ActionDispatch::IllegalStateError, 'header already sent'
         end
 
@@ -279,6 +279,10 @@ module ActionDispatch # :nodoc:
       @stream.body
     end
 
+    def write(string)
+      @stream.write string
+    end
+
     EMPTY = " "
 
     # Allows you to manually set or override the response body.
@@ -294,6 +298,40 @@ module ActionDispatch # :nodoc:
       end
     end
 
+    # Avoid having to pass an open file handle as the response body.
+    # Rack::Sendfile will usually intercept the response and uses
+    # the path directly, so there is no reason to open the file.
+    class FileBody #:nodoc:
+      attr_reader :to_path
+
+      def initialize(path)
+        @to_path = path
+      end
+
+      def body
+        File.binread(to_path)
+      end
+
+      # Stream the file's contents if Rack::Sendfile isn't present.
+      def each
+        File.open(to_path, 'rb') do |file|
+          while chunk = file.read(16384)
+            yield chunk
+          end
+        end
+      end
+    end
+
+    # Send the file stored at +path+ as the response body.
+    def send_file(path)
+      commit!
+      @stream = FileBody.new(path)
+    end
+
+    def reset_body!
+      @stream = build_buffer(self, [])
+    end
+
     def body_parts
       parts = []
       @stream.each { |x| parts << x }
diff --git a/actionpack/test/controller/send_file_test.rb b/actionpack/test/controller/send_file_test.rb
index c712c75c88..3224d84b58 100644
--- a/actionpack/test/controller/send_file_test.rb
+++ b/actionpack/test/controller/send_file_test.rb
@@ -195,7 +195,7 @@ class SendFileTest < ActionController::TestCase
   %w(file data).each do |method|
     define_method "test_send_#{method}_status" do
       @controller.options = { :stream => false, :status => 500 }
-      assert_nothing_raised { assert_not_nil process(method) }
+      assert_not_nil process(method)
       assert_equal 500, @response.status
     end
 
diff --git a/actionpack/test/dispatch/live_response_test.rb b/actionpack/test/dispatch/live_response_test.rb
index 1c128365c7..55becc1c91 100644
--- a/actionpack/test/dispatch/live_response_test.rb
+++ b/actionpack/test/dispatch/live_response_test.rb
@@ -83,6 +83,8 @@ module ActionController
 
       def test_headers_cannot_be_written_after_close
         @response.stream.close
+        # we can add data until it's actually written, which happens on `each`
+        @response.each { |x| }
 
         e = assert_raises(ActionDispatch::IllegalStateError) do
           @response.headers['Content-Length'] = "zomg"