Permalink
Browse files

Move ETag and ConditionalGet logic from AD::Response to the middlewar…

…e stack.
  • Loading branch information...
1 parent 50215f9 commit 74dd8a3681c6984ea35c879f88c6a87521b58ec2 @josevalim josevalim committed Sep 22, 2010
@@ -50,8 +50,7 @@ def initialize(*)
if cache_control = self["Cache-Control"]
cache_control.split(/,\s*/).each do |segment|
first, last = segment.split("=")
- last ||= true
- @cache_control[first.to_sym] = last
+ @cache_control[first.to_sym] = last || true
end
end
end
@@ -88,28 +87,9 @@ def etag=(etag)
def handle_conditional_get!
if etag? || last_modified? || !@cache_control.empty?
set_conditional_cache_control!
- elsif nonempty_ok_response?
- self.etag = body
-
- if request && request.etag_matches?(etag)
- self.status = 304
- self.body = []
- end
-
- set_conditional_cache_control!
- else
- headers["Cache-Control"] = "no-cache"
end
end
- def nonempty_ok_response?
- @status == 200 && string_body?
- end
-
- def string_body?
- !@blank && @body.respond_to?(:all?) && @body.all? { |part| part.is_a?(String) }
- end
-
DEFAULT_CACHE_CONTROL = "max-age=0, private, must-revalidate"
def set_conditional_cache_control!
@@ -132,7 +132,7 @@ def location=(url)
# information.
attr_accessor :charset, :content_type
- CONTENT_TYPE = "Content-Type"
+ CONTENT_TYPE = "Content-Type"
cattr_accessor(:default_charset) { "utf-8" }
@@ -1,46 +0,0 @@
-require 'abstract_unit'
-
-module Etags
- class BasicController < ActionController::Base
- self.view_paths = [ActionView::FixtureResolver.new(
- "etags/basic/base.html.erb" => "Hello from without_layout.html.erb",
- "layouts/etags.html.erb" => "teh <%= yield %> tagz"
- )]
-
- def without_layout
- render :action => "base"
- end
-
- def with_layout
- render :action => "base", :layout => "etags"
- end
- end
-
- class EtagTest < Rack::TestCase
- describe "Rendering without any special etag options returns an etag that is an MD5 hash of its text"
-
- test "an action without a layout" do
- get "/etags/basic/without_layout"
-
- body = "Hello from without_layout.html.erb"
- assert_body body
- assert_header "Etag", etag_for(body)
- assert_status 200
- end
-
- test "an action with a layout" do
- get "/etags/basic/with_layout"
-
- body = "teh Hello from without_layout.html.erb tagz"
- assert_body body
- assert_header "Etag", etag_for(body)
- assert_status 200
- end
-
- private
-
- def etag_for(text)
- %("#{Digest::MD5.hexdigest(text)}")
- end
- end
-end
@@ -99,11 +99,6 @@ def render_hello_world_with_last_modified_set
render :template => "test/hello_world"
end
- def render_hello_world_with_etag_set
- response.etag = "hello_world"
- render :template => "test/hello_world"
- end
-
# :ported: compatibility
def render_hello_world_with_forward_slash
render :template => "/test/hello_world"
@@ -1386,119 +1381,6 @@ def test_expires_now
end
end
-
-class EtagRenderTest < ActionController::TestCase
- tests TestController
-
- def setup
- super
- @request.host = "www.nextangle.com"
- @expected_bang_etag = etag_for(expand_key([:foo, 123]))
- end
-
- def test_render_blank_body_shouldnt_set_etag
- get :blank_response
- assert !@response.etag?
- end
-
- def test_render_200_should_set_etag
- get :render_hello_world_from_variable
- assert_equal etag_for("hello david"), @response.headers['ETag']
- assert_equal "max-age=0, private, must-revalidate", @response.headers['Cache-Control']
- end
-
- def test_render_against_etag_request_should_304_when_match
- @request.if_none_match = etag_for("hello david")
- get :render_hello_world_from_variable
- assert_equal 304, @response.status.to_i
- assert @response.body.empty?
- end
-
- def test_render_against_etag_request_should_have_no_content_length_when_match
- @request.if_none_match = etag_for("hello david")
- get :render_hello_world_from_variable
- assert !@response.headers.has_key?("Content-Length")
- end
-
- def test_render_against_etag_request_should_200_when_no_match
- @request.if_none_match = etag_for("hello somewhere else")
- get :render_hello_world_from_variable
- assert_equal 200, @response.status.to_i
- assert !@response.body.empty?
- end
-
- def test_render_should_not_set_etag_when_last_modified_has_been_specified
- get :render_hello_world_with_last_modified_set
- assert_equal 200, @response.status.to_i
- assert_not_nil @response.last_modified
- assert_nil @response.etag
- assert_present @response.body
- end
-
- def test_render_with_etag
- get :render_hello_world_from_variable
- expected_etag = etag_for('hello david')
- assert_equal expected_etag, @response.headers['ETag']
- @response = ActionController::TestResponse.new
-
- @request.if_none_match = expected_etag
- get :render_hello_world_from_variable
- assert_equal 304, @response.status.to_i
-
- @response = ActionController::TestResponse.new
- @request.if_none_match = "\"diftag\""
- get :render_hello_world_from_variable
- assert_equal 200, @response.status.to_i
- end
-
- def render_with_404_shouldnt_have_etag
- get :render_custom_code
- assert_nil @response.headers['ETag']
- end
-
- def test_etag_should_not_be_changed_when_already_set
- get :render_hello_world_with_etag_set
- assert_equal etag_for("hello_world"), @response.headers['ETag']
- end
-
- 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
-
- def test_etag_with_bang_should_set_etag
- get :conditional_hello_with_bangs
- assert_equal @expected_bang_etag, @response.headers["ETag"]
- assert_response :success
- end
-
- def test_etag_with_bang_should_obey_if_none_match
- @request.if_none_match = @expected_bang_etag
- get :conditional_hello_with_bangs
- assert_response :not_modified
- end
-
- def test_etag_with_public_true_should_set_header
- get :conditional_hello_with_public_header
- assert_equal "public", @response.headers['Cache-Control']
- end
-
- def test_etag_with_public_true_should_set_header_and_retain_other_headers
- get :conditional_hello_with_public_header_and_expires_at
- assert_equal "max-age=60, public", @response.headers['Cache-Control']
- end
-
- protected
- def etag_for(text)
- %("#{Digest::MD5.hexdigest(text)}")
- end
-
- def expand_key(args)
- ActiveSupport::Cache.expand_cache_key(args)
- end
-end
-
class LastModifiedRenderTest < ActionController::TestCase
tests TestController
@@ -11,9 +11,7 @@ def setup
status, headers, body = @response.to_a
assert_equal 200, status
assert_equal({
- "Content-Type" => "text/html; charset=utf-8",
- "Cache-Control" => "max-age=0, private, must-revalidate",
- "ETag" => '"65a8e27d8879283831b664bd8b7f0ad4"'
+ "Content-Type" => "text/html; charset=utf-8"
}, headers)
parts = []
@@ -27,9 +25,7 @@ def setup
status, headers, body = @response.to_a
assert_equal 200, status
assert_equal({
- "Content-Type" => "text/html; charset=utf-8",
- "Cache-Control" => "max-age=0, private, must-revalidate",
- "ETag" => '"ebb5e89e8a94e9dd22abf5d915d112b2"'
+ "Content-Type" => "text/html; charset=utf-8"
}, headers)
end
@@ -41,8 +37,7 @@ def setup
status, headers, body = @response.to_a
assert_equal 200, status
assert_equal({
- "Content-Type" => "text/html; charset=utf-8",
- "Cache-Control" => "no-cache"
+ "Content-Type" => "text/html; charset=utf-8"
}, headers)
parts = []
@@ -145,8 +145,8 @@ def default_middleware_stack
rack_cache = config.action_controller.perform_caching && config.action_dispatch.rack_cache
require "action_dispatch/http/rack_cache" if rack_cache
+ middleware.use ::Rack::Cache, rack_cache if rack_cache
- middleware.use ::Rack::Cache, rack_cache if rack_cache
middleware.use ::ActionDispatch::Static, config.static_asset_paths if config.serve_static_assets
middleware.use ::Rack::Lock if !config.allow_concurrency
middleware.use ::Rack::Runtime
@@ -165,6 +165,8 @@ def default_middleware_stack
middleware.use ::ActionDispatch::ParamsParser
middleware.use ::Rack::MethodOverride
middleware.use ::ActionDispatch::Head
+ middleware.use ::Rack::ConditionalGet
+ middleware.use ::Rack::ETag, "no-cache"
middleware.use ::ActionDispatch::BestStandardsSupport, config.action_dispatch.best_standards_support if config.action_dispatch.best_standards_support
end
end
@@ -36,6 +36,8 @@ def app
"ActionDispatch::ParamsParser",
"Rack::MethodOverride",
"ActionDispatch::Head",
+ "Rack::ConditionalGet",
+ "Rack::ETag",
"ActionDispatch::BestStandardsSupport"
], middleware
end
@@ -45,27 +47,7 @@ def app
boot!
- assert_equal [
- "Rack::Cache",
- "ActionDispatch::Static",
- "Rack::Lock",
- "ActiveSupport::Cache::Strategy::LocalCache",
- "Rack::Runtime",
- "Rails::Rack::Logger",
- "ActionDispatch::ShowExceptions",
- "ActionDispatch::RemoteIp",
- "Rack::Sendfile",
- "ActionDispatch::Callbacks",
- "ActiveRecord::ConnectionAdapters::ConnectionManagement",
- "ActiveRecord::QueryCache",
- "ActionDispatch::Cookies",
- "ActionDispatch::Session::CookieStore",
- "ActionDispatch::Flash",
- "ActionDispatch::ParamsParser",
- "Rack::MethodOverride",
- "ActionDispatch::Head",
- "ActionDispatch::BestStandardsSupport"
- ], middleware
+ assert_equal "Rack::Cache", middleware.first
end
test "removing Active Record omits its middleware" do
@@ -129,6 +111,46 @@ def app
assert_equal "Rack::Config", middleware.first
end
+ # ConditionalGet + Etag
+ test "conditional get + etag middlewares handle http caching based on body" do
+ make_basic_app
+
+ class ::OmgController < ActionController::Base
+ def index
+ if params[:nothing]
+ render :text => ""
+ else
+ render :text => "OMG"
+ end
+ end
+ end
+
+ etag = "5af83e3196bf99f440f31f2e1a6c9afe".inspect
+
+ get "/"
+ assert_equal 200, last_response.status
+ assert_equal "OMG", last_response.body
+ assert_equal "text/html; charset=utf-8", last_response.headers["Content-Type"]
+ assert_equal "max-age=0, private, must-revalidate", last_response.headers["Cache-Control"]
+ assert_equal etag, last_response.headers["Etag"]
+
+ get "/", {}, "HTTP_IF_NONE_MATCH" => etag
+ assert_equal 304, last_response.status
+ assert_equal "", last_response.body
+ assert_equal nil, last_response.headers["Content-Type"]
+ assert_equal "max-age=0, private, must-revalidate", last_response.headers["Cache-Control"]
+ assert_equal etag, last_response.headers["Etag"]
+
+ get "/?nothing=true"
+ puts last_response.body
+ assert_equal 200, last_response.status
+ assert_equal "", last_response.body
+ assert_equal "text/html; charset=utf-8", last_response.headers["Content-Type"]
+ assert_equal "no-cache", last_response.headers["Cache-Control"]
+ assert_equal nil, last_response.headers["Etag"]
+ end
+
+ # Show exceptions middleware
test "show exceptions middleware filter backtrace before logging" do
my_middleware = Struct.new(:app) do
def call(env)

0 comments on commit 74dd8a3

Please sign in to comment.