From 3e07f320c10b53ec21b947d949d0063e724c5fd8 Mon Sep 17 00:00:00 2001 From: Jonathan del Strother Date: Thu, 6 Mar 2008 11:50:44 +0000 Subject: Improve ActionCaching's format-handling Make ActionCaching more aware of different mimetype formats. It will now use request.format to look up the cache type, in addition to the path extension. When expiring caches, the request format no longer affects which cache is expired. Signed-off-by: Pratik Naik --- actionpack/CHANGELOG | 2 + .../lib/action_controller/caching/actions.rb | 41 ++++++++++++----- actionpack/test/controller/caching_test.rb | 53 ++++++++++++++++++++-- 3 files changed, 79 insertions(+), 17 deletions(-) (limited to 'actionpack') diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index 3096716dba..8d1acb265f 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -1,5 +1,7 @@ *Edge* +* Make caching more aware of mime types. Ensure request format is not considered while expiring cache. [Jonathan del Strother] + * Drop ActionController::Base.allow_concurrency flag [Josh Peek] * More efficient concat and capture helpers. Remove ActionView::Base.erb_variable. [Jeremy Kemper] diff --git a/actionpack/lib/action_controller/caching/actions.rb b/actionpack/lib/action_controller/caching/actions.rb index c4b0a97a33..65a36f7f98 100644 --- a/actionpack/lib/action_controller/caching/actions.rb +++ b/actionpack/lib/action_controller/caching/actions.rb @@ -67,10 +67,10 @@ module ActionController #:nodoc: if options[:action].is_a?(Array) options[:action].dup.each do |action| - expire_fragment(ActionCachePath.path_for(self, options.merge({ :action => action }))) + expire_fragment(ActionCachePath.path_for(self, options.merge({ :action => action }), false)) end else - expire_fragment(ActionCachePath.path_for(self, options)) + expire_fragment(ActionCachePath.path_for(self, options, false)) end end @@ -125,16 +125,24 @@ module ActionController #:nodoc: attr_reader :path, :extension class << self - def path_for(controller, options) - new(controller, options).path + def path_for(controller, options, infer_extension=true) + new(controller, options, infer_extension).path end end - - def initialize(controller, options = {}) - @extension = extract_extension(controller.request.path) + + # When true, infer_extension will look up the cache path extension from the request's path & format. + # This is desirable when reading and writing the cache, but not when expiring the cache - expire_action should expire the same files regardless of the request format. + def initialize(controller, options = {}, infer_extension=true) + if infer_extension and options.is_a? Hash + request_extension = extract_extension(controller.request) + options = options.reverse_merge(:format => request_extension) + end path = controller.url_for(options).split('://').last normalize!(path) - add_extension!(path, @extension) + if infer_extension + @extension = request_extension + add_extension!(path, @extension) + end @path = URI.unescape(path) end @@ -144,13 +152,22 @@ module ActionController #:nodoc: end def add_extension!(path, extension) - path << ".#{extension}" if extension + path << ".#{extension}" if extension and !path.ends_with?(extension) end - - def extract_extension(file_path) + + def extract_extension(request) # Don't want just what comes after the last '.' to accommodate multi part extensions # such as tar.gz. - file_path[/^[^.]+\.(.+)$/, 1] + extension = request.path[/^[^.]+\.(.+)$/, 1] + + # If there's no extension in the path, check request.format + if extension.nil? + extension = request.format.to_sym.to_s + if extension=='all' + extension = nil + end + end + extension end end end diff --git a/actionpack/test/controller/caching_test.rb b/actionpack/test/controller/caching_test.rb index 4fb2397e5b..89e12ddae3 100644 --- a/actionpack/test/controller/caching_test.rb +++ b/actionpack/test/controller/caching_test.rb @@ -189,6 +189,10 @@ class ActionCachingTestController < ActionController::Base expire_action :controller => 'action_caching_test', :action => 'index' render :nothing => true end + def expire_xml + expire_action :controller => 'action_caching_test', :action => 'index', :format => 'xml' + render :nothing => true + end end class MockTime < Time @@ -214,6 +218,7 @@ class ActionCachingMockController mocked_path = @mock_path Object.new.instance_eval(<<-EVAL) def path; '#{@mock_path}' end + def format; 'all' end self EVAL end @@ -327,6 +332,20 @@ class ActionCacheTest < Test::Unit::TestCase assert_equal new_cached_time, @response.body end + def test_cache_expiration_isnt_affected_by_request_format + get :index + cached_time = content_to_cache + reset! + + @request.set_REQUEST_URI "/action_caching_test/expire.xml" + get :expire, :format => :xml + reset! + + get :index + new_cached_time = content_to_cache + assert_not_equal cached_time, @response.body + end + def test_cache_is_scoped_by_subdomain @request.host = 'jamis.hostname.com' get :index @@ -371,11 +390,35 @@ class ActionCacheTest < Test::Unit::TestCase 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 + with_routing do |set| + ActionController::Routing::Routes.draw do |map| + map.connect ':controller/:action.:format' + map.connect ':controller/:action' + end + + get :index, :format => 'xml' + cached_time = content_to_cache + assert_equal cached_time, @response.body + assert fragment_exist?('hostname.com/action_caching_test/index.xml') + reset! + + get :index, :format => 'xml' + assert_equal cached_time, @response.body + assert_equal 'application/xml', @response.content_type + reset! + + @request.env['HTTP_ACCEPT'] = "application/xml" + get :index + assert_equal cached_time, @response.body + assert_equal 'application/xml', @response.content_type + reset! + + get :expire_xml + reset! + + get :index, :format => 'xml' + assert_not_equal cached_time, @response.body + end end def test_correct_content_type_is_returned_for_cache_hit -- cgit v1.2.3