Permalink
Browse files

Move etagging down to response, so renders with layouts dont screw it…

… up [DHH]

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@6165 5ecf4fe2-1ee6-0310-87b1-e25e094e27de
  • Loading branch information...
1 parent aad7fbd commit 7ec0204ecd7557a63267ff64cc412338176a0805 @dhh dhh committed Feb 19, 2007
View
17 actionpack/lib/action_controller/base.rb
@@ -7,7 +7,6 @@
require 'action_controller/status_codes'
require 'drb'
require 'set'
-require 'digest/md5'
module ActionController #:nodoc:
class ActionControllerError < StandardError #:nodoc:
@@ -474,6 +473,9 @@ def process(request, response, method = :perform_action, *arguments) #:nodoc:
send(method, *arguments)
assign_default_content_type_and_charset
+
+ response.request = request
+ response.prepare!
response
ensure
process_cleanup
@@ -876,20 +878,7 @@ def render_text(text = nil, status = nil, append_response = false) #:nodoc:
response.body << text
else
response.body = text
-
- if text.is_a?(String)
- if response.headers['Status'][0..2] == '200' && !response.body.empty?
- response.headers['Etag'] = %("#{Digest::MD5.hexdigest(text)}")
-
- if request.headers['HTTP_IF_NONE_MATCH'] == response.headers['Etag']
- response.headers['Status'] = "304 Not Modified"
- response.body = ''
- end
- end
- end
end
-
- response.body
end
def render_javascript(javascript, status = nil, append_response = true) #:nodoc:
View
22 actionpack/lib/action_controller/cgi_process.rb
@@ -184,9 +184,6 @@ def initialize(cgi)
end
def out(output = $stdout)
- convert_content_type!
- set_content_length!
-
output.binmode if output.respond_to?(:binmode)
output.sync = false if output.respond_to?(:sync=)
@@ -209,24 +206,5 @@ def out(output = $stdout)
# lost connection to parent process, ignore output
end
end
-
- private
- def convert_content_type!
- if content_type = @headers.delete("Content-Type")
- @headers["type"] = content_type
- end
- if content_type = @headers.delete("Content-type")
- @headers["type"] = content_type
- end
- if content_type = @headers.delete("content-type")
- @headers["type"] = content_type
- end
- 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!
- @headers["Content-Length"] = @body.size unless @body.respond_to?(:call)
- end
end
end
View
53 actionpack/lib/action_controller/response.rb
@@ -1,35 +1,74 @@
+require 'digest/md5'
+
module ActionController
class AbstractResponse #:nodoc:
DEFAULT_HEADERS = { "Cache-Control" => "no-cache" }
+ attr_accessor :request
attr_accessor :body, :headers, :session, :cookies, :assigns, :template, :redirected_to, :redirected_to_method_params, :layout
def initialize
@body, @headers, @session, @assigns = "", DEFAULT_HEADERS.merge("cookie" => []), [], []
end
def content_type=(mime_type)
- @headers["Content-Type"] = charset ? "#{mime_type}; charset=#{charset}" : mime_type
+ self.headers["Content-Type"] = charset ? "#{mime_type}; charset=#{charset}" : mime_type
end
def content_type
- content_type = String(@headers["Content-Type"]).split(";")[0]
+ content_type = String(headers["Content-Type"] || headers["type"]).split(";")[0]
content_type.blank? ? nil : content_type
end
def charset=(encoding)
- @headers["Content-Type"] = "#{content_type || "text/html"}; charset=#{encoding}"
+ self.headers["Content-Type"] = "#{content_type || "text/html"}; charset=#{encoding}"
end
def charset
- charset = String(@headers["Content-Type"]).split(";")[1]
+ charset = String(headers["Content-Type"] || headers["type"]).split(";")[1]
charset.blank? ? nil : charset.strip.split("=")[1]
end
def redirect(to_url, permanently = false)
- @headers["Status"] = "302 Found" unless @headers["Status"] == "301 Moved Permanently"
- @headers["Location"] = to_url
+ self.headers["Status"] = "302 Found" unless headers["Status"] == "301 Moved Permanently"
+ self.headers["Location"] = to_url
+
+ self.body = "<html><body>You are being <a href=\"#{to_url}\">redirected</a>.</body></html>"
+ end
- @body = "<html><body>You are being <a href=\"#{to_url}\">redirected</a>.</body></html>"
+ def prepare!
+ handle_conditional_get!
+ convert_content_type!
+ set_content_length!
end
+
+ private
+ def handle_conditional_get!
+ if body.is_a?(String) && headers['Status'][0..2] == '200' && !body.empty?
+ self.headers['Etag'] ||= %("#{Digest::MD5.hexdigest(body)}")
+
+ if request.headers['HTTP_IF_NONE_MATCH'] == headers['Etag']
+ self.headers['Status'] = '304 Not Modified'
+ self.body = ''
+ end
+ end
+ end
+
+ def convert_content_type!
+ if content_type = headers.delete("Content-Type")
+ self.headers["type"] = content_type
+ end
+ if content_type = headers.delete("Content-type")
+ self.headers["type"] = content_type
+ end
+ if content_type = headers.delete("content-type")
+ self.headers["type"] = content_type
+ end
+ 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!
+ self.headers["Content-Length"] = body.size unless body.respond_to?(:call)
+ end
end
end
View
1 actionpack/test/controller/render_test.rb
@@ -337,6 +337,7 @@ def test_etag_should_not_be_changed_when_already_set
def test_etag_should_govern_renders_with_layouts_too
get :builder_layout_test
+ assert_equal "<wrapper>\n<html>\n <p>Hello </p>\n<p>This is grand!</p>\n</html>\n</wrapper>\n", @response.body
assert_equal etag_for("<wrapper>\n<html>\n <p>Hello </p>\n<p>This is grand!</p>\n</html>\n</wrapper>\n"), @response.headers['Etag']
end

0 comments on commit 7ec0204

Please sign in to comment.