Skip to content

Commit

Permalink
Remove legacy processing and content_length
Browse files Browse the repository at this point in the history
  * convert_content_type! is handled by assign_default_content_type_and_charset!
  * set_content_length! should be handled by the endpoint server. Otherwise
    each middleware that modifies the body has to do the expensive work of
    recalculating content_length.
  * convert_language! appears to be legacy. There are no tests for this
  * convert_cookies! should be handled by the new HeaderHash in Rack
  * Use an integer for .status's internal representation to avoid needing to
    do String manipulation just to find out the status
  • Loading branch information
wycats committed Aug 2, 2009
1 parent 503ce1d commit b53f006
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 49 deletions.
50 changes: 10 additions & 40 deletions actionpack/lib/action_dispatch/http/response.rb
Expand Up @@ -46,18 +46,22 @@ def initialize
@header = Rack::Utils::HeaderHash.new
end

def status=(status)
@status = status.to_i
end

# The response code of the request
def response_code
status.to_s[0,3].to_i rescue 0
@status
end

# Returns a String to ensure compatibility with Net::HTTPResponse
def code
status.to_s.split(' ')[0]
@status.to_s
end

def message
status.to_s.split(' ',2)[1] || StatusCodes::STATUS_CODES[response_code]
StatusCodes::STATUS_CODES[@status]
end
alias_method :status_message, :message

Expand Down Expand Up @@ -143,10 +147,7 @@ def assign_default_content_type_and_charset!
def prepare!
assign_default_content_type_and_charset!
handle_conditional_get!
set_content_length!
convert_content_type!
convert_language!
convert_cookies!
self["Set-Cookie"] ||= ""
end

def each(&callback)
Expand Down Expand Up @@ -203,7 +204,7 @@ def handle_conditional_get!
self.etag = body

if request && request.etag_matches?(etag)
self.status = '304 Not Modified'
self.status = 304
self.body = []
end

Expand All @@ -214,7 +215,7 @@ def handle_conditional_get!
end

def nonempty_ok_response?
ok = !status || status.to_s[0..2] == '200'
ok = !@status || @status == 200
ok && string_body?
end

Expand All @@ -238,36 +239,5 @@ def set_conditional_cache_control!

headers["Cache-Control"] = options.join(", ")
end

def convert_content_type!
headers['Content-Type'] ||= "text/html"
headers['Content-Type'] += "; charset=" + headers.delete('charset') if headers['charset']
end

# Don't set the Content-Length for block-based bodies as that would mean
# reading it all into memory. Not nice for, say, a 2GB streaming file.
def set_content_length!
if status && status.to_s[0..2] == '204'
headers.delete('Content-Length')
elsif length = headers['Content-Length']
headers['Content-Length'] = length.to_s
elsif string_body? && (!status || status.to_s[0..2] != '304')
headers["Content-Length"] = Rack::Utils.bytesize(body).to_s
end
end

def convert_language!
headers["Content-Language"] = headers.delete("language") if headers["language"]
end

def convert_cookies!
headers['Set-Cookie'] =
if header = headers['Set-Cookie']
header = header.split("\n") if header.respond_to?(:to_str)
header.compact
else
[]
end
end
end
end
Expand Up @@ -120,8 +120,6 @@ def teardown
fixture = FIXTURE_PATH + "/mona_lisa.jpg"
params = { :uploaded_data => fixture_file_upload(fixture, "image/jpg") }
post '/read', params
expected_length = 'File: '.length + File.size(fixture)
assert_equal expected_length, response.content_length
end
end

Expand Down
6 changes: 2 additions & 4 deletions actionpack/test/dispatch/response_test.rb
Expand Up @@ -15,8 +15,7 @@ def setup
"Content-Type" => "text/html; charset=utf-8",
"Cache-Control" => "max-age=0, private, must-revalidate",
"ETag" => '"65a8e27d8879283831b664bd8b7f0ad4"',
"Set-Cookie" => "",
"Content-Length" => "13"
"Set-Cookie" => ""
}, headers)

parts = []
Expand All @@ -34,8 +33,7 @@ def setup
"Content-Type" => "text/html; charset=utf-8",
"Cache-Control" => "max-age=0, private, must-revalidate",
"ETag" => '"ebb5e89e8a94e9dd22abf5d915d112b2"',
"Set-Cookie" => "",
"Content-Length" => "8"
"Set-Cookie" => ""
}, headers)
end

Expand Down
3 changes: 0 additions & 3 deletions actionpack/test/new_base/base_test.rb
Expand Up @@ -34,23 +34,20 @@ class BaseTest < SimpleRouteCase
assert_body "success"
assert_status 200
assert_content_type "text/html; charset=utf-8"
assert_header "Content-Length", "7"
end

# :api: plugin
test "directly modifying response body" do
get "/dispatching/simple/modify_response_body"

assert_body "success"
assert_header "Content-Length", "7" # setting the body manually sets the content length
end

# :api: plugin
test "directly modifying response body twice" do
get "/dispatching/simple/modify_response_body_twice"

assert_body "success!"
assert_header "Content-Length", "8"
end

test "controller path" do
Expand Down

0 comments on commit b53f006

Please sign in to comment.