Action Caching speedup. #8231 [skaes]

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@6867 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

actionpack/CHANGELOG

@@ -1,5 +1,7 @@
 *SVN*
 
+* Action Caching speedup.  #8231 [skaes]
+
 * Wordsmith resources documentation.  #8484 [marclove]
 
 * Fix syntax error in code example for routing documentation. #8377. [norbert]

actionpack/lib/action_controller/caching.rb

@@ -1,5 +1,6 @@
 require 'fileutils'
 require 'uri'
+require 'set'
 
 module ActionController #:nodoc:
   # Caching is a cheap way of speeding up slow applications by keeping the result of calculations, renderings, and database calls
@@ -177,7 +178,10 @@ module ActionController #:nodoc:
     module Actions
       def self.included(base) #:nodoc:
         base.extend(ClassMethods)
-        base.send(:attr_accessor, :rendered_action_cache)
+          base.class_eval do
+            attr_accessor :rendered_action_cache, :action_cache_path
+            alias_method_chain :protected_instance_variables, :action_caching
+          end
       end
 
       module ClassMethods
@@ -189,6 +193,10 @@ module ActionController #:nodoc:
         end
       end
 
+      def protected_instance_variables_with_action_caching
+        protected_instance_variables_without_action_caching + %w(@action_cache_path)
+      end
+
       def expire_action(options = {})
         return unless perform_caching
         if options[:action].is_a?(Array)
@@ -202,74 +210,64 @@ module ActionController #:nodoc:
 
       class ActionCacheFilter #:nodoc:
         def initialize(*actions, &block)
-          @actions = actions
-          @options = @actions.last.is_a?(Hash) ? @actions.pop : {}
+          @options = actions.last.is_a?(Hash) ? actions.pop : {}
+          @actions = Set.new actions
         end
 
         def before(controller)
           return unless @actions.include?(controller.action_name.intern)
-          action_cache_path = ActionCachePath.new(controller, path_options_for(controller))
-          if cache = controller.read_fragment(action_cache_path.path)
+          cache_path = ActionCachePath.new(controller, path_options_for(controller, @options))
+          if cache = controller.read_fragment(cache_path.path)
             controller.rendered_action_cache = true
-            set_content_type!(action_cache_path)
+            set_content_type!(controller, cache_path)
             controller.send(:render_text, cache)
             false
+          else
+            controller.action_cache_path = cache_path
           end
         end
 
         def after(controller)
           return if !@actions.include?(controller.action_name.intern) || controller.rendered_action_cache
-          controller.write_fragment(ActionCachePath.path_for(controller, path_options_for(controller)), controller.response.body)
+          controller.write_fragment(controller.action_cache_path.path, 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.response.content_type = content_type.to_s
-            end
+          def set_content_type!(controller, extension)
+            controller.response.content_type = Mime::EXTENSION_LOOKUP[extension].to_s if extension
           end
           
-          def path_options_for(controller)
-            (@options[:cache_path].respond_to?(:call) ? @options[:cache_path].call(controller) : @options[:cache_path]) || {}
+          def path_options_for(controller, options)
+            ((path_options = options[:cache_path]).respond_to?(:call) ? path_options.call(controller) : path_options) || {}
           end
           
       end
       
       class ActionCachePath
-        attr_reader :controller, :options
+        attr_reader :path, :extension
         
         class << self
-          def path_for(*args, &block)
-            new(*args).path
+          def path_for(controller, options)
+            new(controller, options).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)
+          @extension = extract_extension(controller.request.path)
+          path = controller.url_for(options).split('://').last
+          normalize!(path)
+          add_extension!(path, @extension)
+          @path = URI.unescape(path)
         end
         
         private
-          def normalize!
-            @path << 'index' if @path.last == '/'
+          def normalize!(path)
+            path << 'index' if path[-1] == ?/
           end
         
-          def add_extension!
-            @path << ".#{extension}" if extension
+          def add_extension!(path, extension)
+            path << ".#{extension}" if extension
           end
           
           def extract_extension(file_path)

actionpack/test/controller/caching_test.rb

@@ -224,7 +224,7 @@ class ActionCacheTest < Test::Unit::TestCase
   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)
+    path_object = @path_class.new(@mock_controller, {})
     assert_equal 'xml', path_object.extension
     assert_equal 'example.org/posts/index.xml', path_object.path
   end
@@ -233,7 +233,7 @@ class ActionCacheTest < Test::Unit::TestCase
     @mock_controller.mock_url_for = 'http://example.org/'
     @mock_controller.mock_path    = '/'
 
-    assert_equal 'example.org/index', @path_class.path_for(@mock_controller)
+    assert_equal 'example.org/index', @path_class.path_for(@mock_controller, {})
   end
 
   def test_file_extensions