Permalink
Browse files

Make action caching aware of different formats for the same action so…

… that, e.g. foo.xml is cached separately from foo.html. Implicitly set content type when reading in cached content with mime revealing extensions so the entire onous isn't on the webserver. PDI MORE CACHING TESTS [Marcel Molina Jr.]

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@4656 5ecf4fe2-1ee6-0310-87b1-e25e094e27de
  • Loading branch information...
Marcel Molina
Marcel Molina committed Aug 3, 2006
1 parent 58ebf30 commit d17bd6a0432dde66056e4dd417741731bcb9d323
View
@@ -1,5 +1,7 @@
*SVN*
+* Make action caching aware of different formats for the same action so that, e.g. foo.xml is cached separately from foo.html. Implicitly set content type when reading in cached content with mime revealing extensions so the entire onous isn't on the webserver. [Marcel Molina Jr.]
+
* Restrict Request Method hacking with ?_method to POST requests. [Rick Olson]
* Fix bug when passing multiple options to SimplyRestful, like :new => { :preview => :get, :draft => :get }. [Rick Olson, Josh Susser, Lars Pind]
@@ -155,6 +155,10 @@ def caching_allowed
# the current host and the path. So a page that is accessed at http://david.somewhere.com/lists/show/1 will result in a fragment named
# "david.somewhere.com/lists/show/1". This allows the cacher to differentiate between "david.somewhere.com/lists/" and
# "jamis.somewhere.com/lists/" -- which is a helpful way of assisting the subdomain-as-account-key pattern.
+ #
+ # Different representations of the same resource, e.g. <tt>http://david.somewhere.com/lists</tt> and <tt>http://david.somewhere.com/lists.xml</tt>
+ # are treated like separate requests and are so are cached separately. Keep in mine when expiring an action cache that <tt>:action => 'lists'</tt> is not the same
+ # as <tt>:action => 'list', :format => :xml</tt>.
module Actions
def self.included(base) #:nodoc:
base.extend(ClassMethods)
@@ -172,31 +176,85 @@ def expire_action(options = {})
return unless perform_caching
if options[:action].is_a?(Array)
options[:action].dup.each do |action|
- expire_fragment(url_for(options.merge({ :action => action })).split("://").last)
+ expire_fragment(ActionCachePath.path_for(self, options.merge({ :action => action })))
end
else
- expire_fragment(url_for(options).split("://").last)
+ expire_fragment(ActionCachePath.path_for(self, options))
end
end
- class ActionCacheFilter #:nodoc:
- def initialize(*actions)
+ class ActionCacheFilter #:nodoc:
+ def initialize(*actions, &block)
@actions = actions
end
def before(controller)
return unless @actions.include?(controller.action_name.intern)
- if cache = controller.read_fragment(controller.url_for.split("://").last)
+ action_cache_path = ActionCachePath.new(controller)
+ if cache = controller.read_fragment(action_cache_path.path)
controller.rendered_action_cache = true
+ set_content_type!(action_cache_path)
controller.send(:render_text, cache)
false
end
end
def after(controller)
return if !@actions.include?(controller.action_name.intern) || controller.rendered_action_cache
- controller.write_fragment(controller.url_for.split("://").last, controller.response.body)
+ controller.write_fragment(ActionCachePath.path_for(controller), controller.response.body)
+ end
+
+ private
+
+ def set_content_type!(action_cache_path)
+ if extention = action_cache_path.extension
+ content_type = Mime::EXTENSION_LOOKUP[extention]
+ action_cache_path.controller.headers['Content-Type'] = content_type.to_s
+ end
+ end
+
+ end
+
+ class ActionCachePath
+ attr_reader :controller, :options
+
+ class << self
+ def path_for(*args, &block)
+ new(*args).path
+ end
+ end
+
+ def initialize(controller, options = {})
+ @controller = controller
+ @options = options
end
+
+ def path
+ return @path if @path
+ @path = controller.url_for(options).split('://').last
+ normalize!
+ add_extension!
+ URI.unescape(@path)
+ end
+
+ def extension
+ @extension ||= extract_extension(controller.request.path)
+ end
+
+ private
+ def normalize!
+ @path << 'index' if @path.last == '/'
+ end
+
+ def add_extension!
+ @path << ".#{extension}" if extension
+ end
+
+ def extract_extension(file_path)
+ # Don't want just what comes after the last '.' to accomodate multi part extensions
+ # such as tar.gz.
+ file_path[/^[^.]+\.(.+)$/, 1]
+ end
end
end
@@ -0,0 +1,137 @@
+require 'fileutils'
+require File.dirname(__FILE__) + '/../abstract_unit'
+
+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.fragment_cache_store = :file_store, FILE_STORE_PATH
+
+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)
+ def path; '#{@mock_path}' end
+ self
+ EVAL
+ end
+end
+
+class ActionCacheTest < Test::Unit::TestCase
+ def setup
+ reset!
+ FileUtils.mkdir_p(FILE_STORE_PATH)
+ @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'
+ path_object = @path_class.new(@mock_controller)
+ 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
+
+ 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
@@ -23,7 +23,6 @@ class SubclassedController < ContainedNonEmptyController
end
end
class EmptyController < ActionController::Base
- include ActionController::Caching
end
class NonEmptyController < ActionController::Base
def public_action

0 comments on commit d17bd6a

Please sign in to comment.