aboutsummaryrefslogtreecommitdiffstats
path: root/actionpack
diff options
context:
space:
mode:
authorJeremy Kemper <jeremy@bitsweat.net>2006-12-19 20:25:52 +0000
committerJeremy Kemper <jeremy@bitsweat.net>2006-12-19 20:25:52 +0000
commite56bd3a121d1b9ae1d89a335cc39ff9aef36789e (patch)
treea40e38ab24cca3d63a74a0bab181bfecaee2be68 /actionpack
parent7277c518912e08d125cef0f0a890a72bd7283cf5 (diff)
downloadrails-e56bd3a121d1b9ae1d89a335cc39ff9aef36789e.tar.gz
rails-e56bd3a121d1b9ae1d89a335cc39ff9aef36789e.tar.bz2
rails-e56bd3a121d1b9ae1d89a335cc39ff9aef36789e.zip
Only cache GET requests with a 200 OK response. Closes #6514, #6743.
git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@5755 5ecf4fe2-1ee6-0310-87b1-e25e094e27de
Diffstat (limited to 'actionpack')
-rw-r--r--actionpack/CHANGELOG2
-rw-r--r--actionpack/lib/action_controller/caching.rb2
-rw-r--r--actionpack/test/controller/caching_test.rb119
3 files changed, 94 insertions, 29 deletions
diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG
index 32cc8e0316..8050fb4722 100644
--- a/actionpack/CHANGELOG
+++ b/actionpack/CHANGELOG
@@ -1,5 +1,7 @@
*SVN*
+* Only cache GET requests with a 200 OK response. #6514, #6743 [RSL, anamba]
+
* Add a 'referer' attribute to TestRequest. [Jamis Buck]
* Ensure render :json => ... skips the layout. Closes #6808 [Josh Peek]
diff --git a/actionpack/lib/action_controller/caching.rb b/actionpack/lib/action_controller/caching.rb
index 28dc45139a..c7f3e95617 100644
--- a/actionpack/lib/action_controller/caching.rb
+++ b/actionpack/lib/action_controller/caching.rb
@@ -135,7 +135,7 @@ module ActionController #:nodoc:
private
def caching_allowed
- !request.post? && response.headers['Status'] && response.headers['Status'].to_i < 400
+ request.get? && response.headers['Status'].to_i == 200
end
end
diff --git a/actionpack/test/controller/caching_test.rb b/actionpack/test/controller/caching_test.rb
index 1935cf5aa5..07e16896bd 100644
--- a/actionpack/test/controller/caching_test.rb
+++ b/actionpack/test/controller/caching_test.rb
@@ -5,8 +5,29 @@ CACHE_DIR = 'test_cache'
# Don't change '/../temp/' cavalierly or you might hoze something you don't want hozed
FILE_STORE_PATH = File.join(File.dirname(__FILE__), '/../temp/', CACHE_DIR)
ActionController::Base.perform_caching = true
+ActionController::Base.page_cache_directory = FILE_STORE_PATH
ActionController::Base.fragment_cache_store = :file_store, FILE_STORE_PATH
+class PageCachingTestController < ActionController::Base
+ caches_page :ok, :no_content, :found, :not_found
+
+ def ok
+ head :ok
+ end
+
+ def no_content
+ head :no_content
+ end
+
+ def found
+ redirect_to :action => 'ok'
+ end
+
+ def not_found
+ head :not_found
+ end
+end
+
class PageCachingTest < Test::Unit::TestCase
def setup
ActionController::Routing::Routes.draw do |map|
@@ -14,11 +35,23 @@ class PageCachingTest < Test::Unit::TestCase
map.resources :posts
map.connect ':controller/:action/:id'
end
-
+
@request = ActionController::TestRequest.new
+ @request.host = 'hostname.com'
+
+ @response = ActionController::TestResponse.new
+ @controller = PageCachingTestController.new
+
@params = {:controller => 'posts', :action => 'index', :only_path => true, :skip_relative_url_root => true}
@rewriter = ActionController::UrlRewriter.new(@request, @params)
- end
+
+ FileUtils.rm_rf(File.dirname(FILE_STORE_PATH))
+ FileUtils.mkdir_p(FILE_STORE_PATH)
+ end
+
+ def teardown
+ FileUtils.rm_rf(File.dirname(FILE_STORE_PATH))
+ end
def test_page_caching_resources_saves_to_correct_path_with_extension_even_if_default_route
@params[:format] = 'rss'
@@ -26,35 +59,67 @@ class PageCachingTest < Test::Unit::TestCase
@params[:format] = nil
assert_equal '/', @rewriter.rewrite(@params)
end
+
+ def test_should_cache_get_with_ok_status
+ get :ok
+ assert_response :ok
+ assert_page_cached :ok, "get with ok status should have been cached"
+ end
+
+ [:ok, :no_content, :found, :not_found].each do |status|
+ [:get, :post, :put, :delete].each do |method|
+ unless method == :get and status == :ok
+ define_method "test_shouldnt_cache_#{method}_with_#{status}_status" do
+ @request.env['REQUEST_METHOD'] = method.to_s.upcase
+ process status
+ assert_response status
+ assert_page_not_cached status, "#{method} with #{status} status shouldn't have been cached"
+ end
+ end
+ end
+ end
+
+ private
+ def assert_page_cached(action, message = "#{action} should have been cached")
+ assert page_cached?(action), message
+ end
+
+ def assert_page_not_cached(action, message = "#{action} shouldn't have been cached")
+ assert !page_cached?(action), message
+ end
+
+ def page_cached?(action)
+ File.exist? "#{FILE_STORE_PATH}/page_caching_test/#{action}.html"
+ end
end
class ActionCachingTestController < ActionController::Base
caches_action :index
-
+
def index
@cache_this = Time.now.to_f.to_s
render :text => @cache_this
end
-
+
def expire
expire_action :controller => 'action_caching_test', :action => 'index'
render :nothing => true
end
-
+
end
class ActionCachingMockController
attr_accessor :mock_url_for
attr_accessor :mock_path
-
+
def initialize
yield self if block_given?
end
-
+
def url_for(*args)
@mock_url_for
end
-
+
def request
mocked_path = @mock_path
Object.new.instance_eval(<<-EVAL)
@@ -71,62 +136,62 @@ class ActionCacheTest < Test::Unit::TestCase
@path_class = ActionController::Caching::Actions::ActionCachePath
@mock_controller = ActionCachingMockController.new
end
-
+
def teardown
FileUtils.rm_rf(File.dirname(FILE_STORE_PATH))
end
-
+
def test_simple_action_cache
get :index
cached_time = content_to_cache
assert_equal cached_time, @response.body
reset!
-
+
get :index
assert_equal cached_time, @response.body
end
-
+
def test_cache_expiration
get :index
cached_time = content_to_cache
reset!
-
+
get :index
assert_equal cached_time, @response.body
reset!
get :expire
reset!
-
+
get :index
new_cached_time = content_to_cache
assert_not_equal cached_time, @response.body
reset!
-
+
get :index
assert_response :success
assert_equal new_cached_time, @response.body
end
-
+
def test_cache_is_scoped_by_subdomain
@request.host = 'jamis.hostname.com'
get :index
jamis_cache = content_to_cache
-
+
@request.host = 'david.hostname.com'
get :index
david_cache = content_to_cache
assert_not_equal jamis_cache, @response.body
-
+
@request.host = 'jamis.hostname.com'
get :index
assert_equal jamis_cache, @response.body
-
+
@request.host = 'david.hostname.com'
get :index
assert_equal david_cache, @response.body
end
-
+
def test_xml_version_of_resource_is_treated_as_different_cache
@mock_controller.mock_url_for = 'http://example.org/posts/'
@mock_controller.mock_path = '/posts/index.xml'
@@ -134,32 +199,30 @@ class ActionCacheTest < Test::Unit::TestCase
assert_equal 'xml', path_object.extension
assert_equal 'example.org/posts/index.xml', path_object.path
end
-
+
def test_empty_path_is_normalized
@mock_controller.mock_url_for = 'http://example.org/'
@mock_controller.mock_path = '/'
assert_equal 'example.org/index', @path_class.path_for(@mock_controller)
end
-
+
def test_file_extensions
- get :index, :id => 'kitten.jpg'
+ get :index, :id => 'kitten.jpg'
get :index, :id => 'kitten.jpg'
assert_response :success
end
-
+
private
-
def content_to_cache
assigns(:cache_this)
end
-
+
def reset!
@request = ActionController::TestRequest.new
@response = ActionController::TestResponse.new
@controller = ActionCachingTestController.new
@request.host = 'hostname.com'
end
-
-end \ No newline at end of file
+end