Merge RackProcess#normalize_headers logic into AbstractResponse#prepare!

actionpack/lib/action_controller/rack_process.rb

@@ -143,23 +143,26 @@ def session_options_with_string_keys
   end
 
   class RackResponse < AbstractResponse #:nodoc:
-    attr_accessor :status
-
     def initialize(request)
-      @request = request
+      @cgi = request.cgi
       @writer = lambda { |x| @body << x }
       @block = nil
       super()
     end
 
+    # Retrieve status from instance variable if has already been delete
+    def status
+      @status || super
+    end
+
     def out(output = $stdout, &block)
       @block = block
-      normalize_headers(@headers)
-      if [204, 304].include?(@status.to_i)
-        @headers.delete "Content-Type"
-        [status, @headers.to_hash, []]
+      @status = headers.delete("Status")
+      if [204, 304].include?(status.to_i)
+        headers.delete("Content-Type")
+        [status, headers.to_hash, []]
       else
-        [status, @headers.to_hash, self]
+        [status, headers.to_hash, self]
       end
     end
     alias to_a out
@@ -191,43 +194,57 @@ def empty?
       @block == nil && @body.empty?
     end
 
-    private
-      def normalize_headers(options = "text/html")
-        if options.is_a?(String)
-          headers['Content-Type']     = options unless headers['Content-Type']
-        else
-          headers['Content-Length']   = options.delete('Content-Length').to_s if options['Content-Length']
+    def prepare!
+      super
 
-          headers['Content-Type']     = options.delete('type') || "text/html"
-          headers['Content-Type']    += "; charset=" + options.delete('charset') if options['charset']
+      convert_language!
+      convert_expires!
+      set_status!
+      set_cookies!
+    end
 
-          headers['Content-Language'] = options.delete('language') if options['language']
-          headers['Expires']          = options.delete('expires') if options['expires']
+    private
+      def convert_language!
+        headers["Content-Language"] = headers.delete("language") if headers["language"]
+      end
 
-          @status = options.delete('Status') || "200 OK"
+      def convert_expires!
+        headers["Expires"] = headers.delete("") if headers["expires"]
+      end
 
-          # Convert 'cookie' header to 'Set-Cookie' headers.
-          # Because Set-Cookie header can appear more the once in the response body,
-          # we store it in a line break separated string that will be translated to
-          # multiple Set-Cookie header by the handler.
-          if cookie = options.delete('cookie')
-            cookies = []
+      def convert_content_type!
+        super
+        headers['Content-Type'] = headers.delete('type') || "text/html"
+        headers['Content-Type'] += "; charset=" + headers.delete('charset') if headers['charset']
+      end
 
-            case cookie
-              when Array then cookie.each { |c| cookies << c.to_s }
-              when Hash  then cookie.each { |_, c| cookies << c.to_s }
-              else            cookies << cookie.to_s
-            end
+      def set_content_length!
+        super
+        headers["Content-Length"] = headers["Content-Length"].to_s if headers["Content-Length"]
+      end
 
-            @request.cgi.output_cookies.each { |c| cookies << c.to_s } if @request.cgi.output_cookies
+      def set_status!
+        self.status ||= "200 OK"
+      end
 
-            headers['Set-Cookie'] = [headers['Set-Cookie'], cookies].flatten.compact
+      def set_cookies!
+        # Convert 'cookie' header to 'Set-Cookie' headers.
+        # Because Set-Cookie header can appear more the once in the response body,
+        # we store it in a line break separated string that will be translated to
+        # multiple Set-Cookie header by the handler.
+        if cookie = headers.delete('cookie')
+          cookies = []
+
+          case cookie
+            when Array then cookie.each { |c| cookies << c.to_s }
+            when Hash  then cookie.each { |_, c| cookies << c.to_s }
+            else            cookies << cookie.to_s
           end
 
-          options.each { |k,v| headers[k] = v }
-        end
+          @cgi.output_cookies.each { |c| cookies << c.to_s } if @cgi.output_cookies
 
-        ""
+          headers['Set-Cookie'] = [headers['Set-Cookie'], cookies].flatten.compact
+        end
       end
   end
 

actionpack/test/controller/rack_test.rb

@@ -236,10 +236,17 @@ def setup
 
   def test_simple_output
     @response.body = "Hello, World!"
+    @response.prepare!
 
     status, headers, body = @response.out(@output)
     assert_equal "200 OK", status
-    assert_equal({"Content-Type" => "text/html", "Cache-Control" => "no-cache", "Set-Cookie" => []}, headers)
+    assert_equal({
+      "Content-Type" => "text/html",
+      "Cache-Control" => "private, max-age=0, must-revalidate",
+      "ETag" => '"65a8e27d8879283831b664bd8b7f0ad4"',
+      "Set-Cookie" => [],
+      "Content-Length" => "13"
+    }, headers)
 
     parts = []
     body.each { |part| parts << part }
@@ -250,6 +257,7 @@ def test_streaming_block
     @response.body = Proc.new do |response, output|
       5.times { |n| output.write(n) }
     end
+    @response.prepare!
 
     status, headers, body = @response.out(@output)
     assert_equal "200 OK", status
@@ -265,13 +273,16 @@ def test_set_session_cookie
     @request.cgi.send :instance_variable_set, '@output_cookies', [cookie]
 
     @response.body = "Hello, World!"
+    @response.prepare!
 
     status, headers, body = @response.out(@output)
     assert_equal "200 OK", status
     assert_equal({
       "Content-Type" => "text/html",
-      "Cache-Control" => "no-cache",
-      "Set-Cookie" => ["name=Josh; path="]
+      "Cache-Control" => "private, max-age=0, must-revalidate",
+      "ETag" => '"65a8e27d8879283831b664bd8b7f0ad4"',
+      "Set-Cookie" => ["name=Josh; path="],
+      "Content-Length" => "13"
     }, headers)
 
     parts = []
@@ -285,18 +296,18 @@ def setup
     super
     @response = ActionController::RackResponse.new(@request)
     @output = StringIO.new('')
-    @response.headers['Status'] = 200
+    @response.headers['Status'] = "200 OK"
   end
 
   def test_content_type
     [204, 304].each do |c|
-      @response.headers['Status'] = c
-      assert !response_headers.has_key?("Content-Type")
+      @response.headers['Status'] = c.to_s
+      assert !response_headers.has_key?("Content-Type"), "#{c} should not have Content-Type header"
     end
 
     [200, 302, 404, 500].each do |c|
-      @response.headers['Status'] = c
-      assert response_headers.has_key?("Content-Type")
+      @response.headers['Status'] = c.to_s
+      assert response_headers.has_key?("Content-Type"), "#{c} did not have Content-Type header"
     end
   end
 
@@ -305,8 +316,8 @@ def test_status
   end
 
   private
-
-  def response_headers
-    @response.out(@output)[1]
-  end
+    def response_headers
+      @response.prepare!
+      @response.out(@output)[1]
+    end
 end