Caching refactoring

6 files changed, 98 insertions, 93 deletions

diff --git a/actionpack/lib/action_controller/caching.rb b/actionpack/lib/action_controller/caching.rb
index 63429e1cbe..083d6328af 100644
--- a/actionpack/lib/action_controller/caching.rb
+++ b/actionpack/lib/action_controller/caching.rb
@@ -3,26 +3,30 @@ 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
-  # around for subsequent requests. Action Controller affords you three approaches in varying levels of granularity: Page, Action, Fragment.
+  # Caching is a cheap way of speeding up slow applications by keeping the result of
+  # calculations, renderings, and database calls around for subsequent requests.
+  # Action Controller affords you three approaches in varying levels of granularity:
+  # Page, Action, Fragment.
   #
-  # You can read more about each approach and the sweeping assistance by clicking the modules below.
-  #
-  # Note: To turn off all caching and sweeping, set Base.perform_caching = false.
+  # You can read more about each approach and the sweeping assistance by clicking the
+  # modules below.
   #
+  # Note: To turn off all caching and sweeping, set
+  #   config.action_controller.perform_caching = false.
   #
   # == Caching stores
   #
-  # All the caching stores from ActiveSupport::Cache are available to be used as backends for Action Controller caching. This setting only
-  # affects action and fragment caching as page caching is always written to disk.
+  # All the caching stores from ActiveSupport::Cache are available to be used as backends
+  # for Action Controller caching. This setting only affects action and fragment caching
+  # as page caching is always written to disk.
   #
   # Configuration examples (MemoryStore is the default):
   #
-  #   ActionController::Base.cache_store = :memory_store
-  #   ActionController::Base.cache_store = :file_store, "/path/to/cache/directory"
-  #   ActionController::Base.cache_store = :drb_store, "druby://localhost:9192"
-  #   ActionController::Base.cache_store = :mem_cache_store, "localhost"
-  #   ActionController::Base.cache_store = MyOwnStore.new("parameter")
+  #   config.action_controller.cache_store = :memory_store
+  #   config.action_controller.cache_store = :file_store, "/path/to/cache/directory"
+  #   config.action_controller.cache_store = :drb_store, "druby://localhost:9192"
+  #   config.action_controller.cache_store = :mem_cache_store, "localhost"
+  #   config.action_controller.cache_store = MyOwnStore.new("parameter")
   module Caching
     extend ActiveSupport::Concern
 
@@ -46,8 +50,10 @@ module ActionController #:nodoc:
 
       @@perform_caching = true
       cattr_accessor :perform_caching
+    end
 
-      def self.cache_configured?
+    module ClassMethods
+      def cache_configured?
         perform_caching && cache_store
       end
     end
diff --git a/actionpack/lib/action_controller/caching/actions.rb b/actionpack/lib/action_controller/caching/actions.rb
index 05305d9a3f..35111a4b92 100644
--- a/actionpack/lib/action_controller/caching/actions.rb
+++ b/actionpack/lib/action_controller/caching/actions.rb
@@ -2,10 +2,12 @@ require 'set'
 
 module ActionController #:nodoc:
   module Caching
-    # Action caching is similar to page caching by the fact that the entire output of the response is
-    # cached, but unlike page caching, every request still goes through the Action Pack. The key benefit
-    # of this is that filters are run before the cache is served, which allows for authentication and other
-    # restrictions on whether someone is allowed to see the cache. Example:
+    # Action caching is similar to page caching by the fact that the entire
+    # output of the response is cached, but unlike page caching, every
+    # request still goes through the Action Pack. The key benefit
+    # of this is that filters are run before the cache is served, which
+    # allows for authentication and other restrictions on whether someone
+    # is allowed to see the cache. Example:
     #
     #   class ListsController < ApplicationController
     #     before_filter :authenticate, :except => :public
@@ -13,55 +15,65 @@ module ActionController #:nodoc:
     #     caches_action :index, :show, :feed
     #   end
     #
-    # In this example, the public action doesn't require authentication, so it's possible to use the faster
-    # page caching method. But both the show and feed action are to be shielded behind the authenticate
+    # In this example, the public action doesn't require authentication,
+    # so it's possible to use the faster page caching method. But both
+    # the show and feed action are to be shielded behind the authenticate
     # filter, so we need to implement those as action caches.
     #
-    # Action caching internally uses the fragment caching and an around filter to do the job. The fragment
-    # cache is named according to both the current host and the path. So a page that is accessed at
+    # Action caching internally uses the fragment caching and an around
+    # filter to do the job. The fragment cache is named according to both
+    # 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.
+    # "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
+    # 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 so are cached separately. Keep in mind when expiring an
-    # action cache that <tt>:action => 'lists'</tt> is not the same as
+    # are treated like separate requests and so are cached separately.
+    # Keep in mind when expiring an action cache that
+    # <tt>:action => 'lists'</tt> is not the same as
     # <tt>:action => 'list', :format => :xml</tt>.
     #
-    # You can set modify the default action cache path by passing a :cache_path option.  This will be
-    # passed directly to ActionCachePath.path_for.  This is handy for actions with multiple possible
-    # routes that should be cached differently.  If a block is given, it is called with the current
-    # controller instance.
+    # You can set modify the default action cache path by passing a
+    # :cache_path option.  This will be passed directly to
+    # ActionCachePath.path_for.  This is handy for actions with multiple
+    # possible routes that should be cached differently.  If a block is
+    # given, it is called with the current controller instance.
     #
-    # And you can also use :if (or :unless) to pass a Proc that specifies when the action should
-    # be cached.
+    # And you can also use :if (or :unless) to pass a Proc that
+    # specifies when the action should be cached.
     #
     # Finally, if you are using memcached, you can also pass :expires_in.
     #
     #   class ListsController < ApplicationController
     #     before_filter :authenticate, :except => :public
     #     caches_page   :public
-    #     caches_action :index, :if => proc { |c| !c.request.format.json? } # cache if is not a JSON request
-    #     caches_action :show, :cache_path => { :project => 1 }, :expires_in => 1.hour
-    #     caches_action :feed, :cache_path => proc { |controller|
-    #       controller.params[:user_id] ?
-    #         controller.send(:user_list_url, controller.params[:user_id], controller.params[:id]) :
-    #         controller.send(:list_url, controller.params[:id]) }
+    #     caches_action :index, :if => proc do |c|
+    #       !c.request.format.json?  # cache if is not a JSON request
+    #     end
+    #
+    #     caches_action :show, :cache_path => { :project => 1 },
+    #       :expires_in => 1.hour
+    #
+    #     caches_action :feed, :cache_path => proc do |controller|
+    #       if controller.params[:user_id]
+    #         controller.send(:user_list_url,
+    #           controller.params[:user_id], controller.params[:id])
+    #       else
+    #         controller.send(:list_url, controller.params[:id])
+    #       end
+    #     end
     #   end
     #
-    # If you pass :layout => false, it will only cache your action content. It is useful when your
-    # layout has dynamic information.
+    # If you pass :layout => false, it will only cache your action
+    # content. It is useful when your layout has dynamic information.
     #
     module Actions
       extend ActiveSupport::Concern
 
-      included do
-        attr_accessor :rendered_action_cache, :action_cache_path
-      end
-
       module ClassMethods
         # Declares that +actions+ should be cached.
         # See ActionController::Caching::Actions for details.
@@ -76,11 +88,7 @@ module ActionController #:nodoc:
       end
 
       def _render_cache_fragment(cache, extension, layout)
-        self.rendered_action_cache = true
-        response.content_type = Mime[extension].to_s if extension
-        options = { :text => cache }
-        options.merge!(:layout => true) if layout
-        render options
+        render :text => cache, :layout => layout, :content_type => Mime[extension || :html]
       end
 
       def _save_fragment(name, layout, options)
@@ -90,17 +98,17 @@ module ActionController #:nodoc:
         write_fragment(name, content, options)
       end
 
-      protected
-        def expire_action(options = {})
-          return unless cache_configured?
+    protected
+      def expire_action(options = {})
+        return unless cache_configured?
 
-          actions = options[:action]
-          if actions.is_a?(Array)
-            actions.each {|action| expire_action(options.merge(:action => action)) }
-          else
-            expire_fragment(ActionCachePath.path_for(self, options, false))
-          end
+        actions = options[:action]
+        if actions.is_a?(Array)
+          actions.each {|action| expire_action(options.merge(:action => action)) }
+        else
+          expire_fragment(ActionCachePath.new(self, options, false).path)
         end
+      end
 
       class ActionCacheFilter #:nodoc:
         def initialize(options, &block)
@@ -109,7 +117,12 @@ module ActionController #:nodoc:
         end
 
         def filter(controller)
-          path_options = @cache_path.respond_to?(:call) ? @cache_path.call(controller) : @cache_path
+          path_options = if @cache_path.respond_to?(:call)
+            controller.instance_exec(controller, &@cache_path)
+          else
+            @cache_path
+          end
+
           cache_path = ActionCachePath.new(controller, path_options || {})
 
           if cache = controller.read_fragment(cache_path.path, @store_options)
@@ -124,41 +137,25 @@ module ActionController #:nodoc:
       class ActionCachePath
         attr_reader :path, :extension
 
-        class << self
-          def path_for(controller, options, infer_extension = true)
-            new(controller, options, infer_extension).path
-          end
-        end
-
         # If +infer_extension+ is true, the cache path extension is looked up 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
-            extract_extension(controller.request)
-            options = options.reverse_merge(:format => @extension) if options.is_a?(Hash)
+            @extension = controller.params[:format]
+            options.reverse_merge!(:format => @extension) if options.is_a?(Hash)
           end
 
-          path = controller.url_for(options).split('://').last
-          normalize!(path)
-          add_extension!(path, @extension)
-          @path = URI.unescape(path)
+          path = controller.url_for(options).split(%r{://}).last
+          @path = normalize!(path)
         end
 
       private
         def normalize!(path)
           path << 'index' if path[-1] == ?/
-        end
-
-        def add_extension!(path, extension)
           path << ".#{extension}" if extension and !path.ends_with?(extension)
-        end
-
-        def extract_extension(request)
-          # Don't want just what comes after the last '.' to accommodate multi part extensions
-          # such as tar.gz.
-          @extension = request.path[/^[^.]+\.(.+)$/, 1] || request.cache_format
+          URI.unescape(path)
         end
       end
     end
diff --git a/actionpack/lib/action_controller/caching/sweeping.rb b/actionpack/lib/action_controller/caching/sweeping.rb
index c1be264ffb..871f41bfdd 100644
--- a/actionpack/lib/action_controller/caching/sweeping.rb
+++ b/actionpack/lib/action_controller/caching/sweeping.rb
@@ -70,7 +70,7 @@ module ActionController #:nodoc:
         protected
           # gets the action cache path for the given options.
           def action_path_for(options)
-            ActionController::Caching::Actions::ActionCachePath.path_for(controller, options)
+            Actions::ActionCachePath.new(controller, options).path
           end
 
           # Retrieve instance variables set in the controller.
diff --git a/actionpack/lib/action_controller/metal/compatibility.rb b/actionpack/lib/action_controller/metal/compatibility.rb
index 22f9ab219c..c251d79f4e 100644
--- a/actionpack/lib/action_controller/metal/compatibility.rb
+++ b/actionpack/lib/action_controller/metal/compatibility.rb
@@ -25,9 +25,11 @@ module ActionController
 
       # cattr_reader :protected_instance_variables
       cattr_accessor :protected_instance_variables
-      self.protected_instance_variables = %w(@assigns @performed_redirect @performed_render @variables_added @request_origin @url @parent_controller
-                                          @action_name @before_filter_chain_aborted @action_cache_path @_headers @_params
-                                          @_flash @_response)
+      self.protected_instance_variables = %w(@assigns @performed_redirect @performed_render
+                                             @variables_added @request_origin @url
+                                             @parent_controller @action_name
+                                             @before_filter_chain_aborted @_headers @_params
+                                             @_flash @_response)
 
       # Indicates whether or not optimise the generated named
       # route helper methods
diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb
index bb99fac5e0..75be2cc260 100755
--- a/actionpack/lib/action_dispatch/http/request.rb
+++ b/actionpack/lib/action_dispatch/http/request.rb
@@ -98,7 +98,7 @@ module ActionDispatch
     end
 
     def forgery_whitelisted?
-      method == :get || xhr? || !(!content_type.nil? && content_type.verify_request?)
+      method == :get || xhr? || content_type.nil? || !content_type.verify_request?
     end
 
     def media_type
@@ -205,10 +205,6 @@ module ActionDispatch
       end
     end
 
-    def cache_format
-      parameters[:format]
-    end
-
     # Returns true if the request's "X-Requested-With" header contains
     # "XMLHttpRequest". (The Prototype Javascript library sends this header with
     # every Ajax request.)
diff --git a/actionpack/test/controller/caching_test.rb b/actionpack/test/controller/caching_test.rb
index df2dee8228..3ce90b6ccf 100644
--- a/actionpack/test/controller/caching_test.rb
+++ b/actionpack/test/controller/caching_test.rb
@@ -228,12 +228,16 @@ class ActionCachingMockController
     @mock_url_for
   end
 
+  def params
+    request.parameters
+  end
+
   def request
     mocked_path = @mock_path
     Object.new.instance_eval(<<-EVAL)
       def path; '#{@mock_path}' end
       def format; 'all' end
-      def cache_format; nil end
+      def parameters; {:format => nil}; end
       self
     EVAL
   end
@@ -466,7 +470,7 @@ class ActionCacheTest < ActionController::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.new(@mock_controller, {}).path
   end
 
   def test_file_extensions