Recyclable cache keys

actionmailer/test/caching_test.rb

@@ -21,10 +21,6 @@ def setup
     @mailer.perform_caching = true
     @mailer.cache_store = @store
   end
-
-  def test_fragment_cache_key
-    assert_equal "views/what a key", @mailer.fragment_cache_key("what a key")
-  end
 end
 
 class FragmentCachingTest < BaseCachingTest

actionpack/lib/abstract_controller/caching/fragments.rb

@@ -25,7 +25,10 @@ module Fragments
 
         self.fragment_cache_keys = []
 
-        helper_method :fragment_cache_key if respond_to?(:helper_method)
+        if respond_to?(:helper_method)
+          helper_method :fragment_cache_key
+          helper_method :combined_fragment_cache_key
+        end
       end
 
       module ClassMethods
@@ -62,17 +65,36 @@ def fragment_cache_key(value = nil, &key)
       # with the specified +key+ value. The key is expanded using
       # ActiveSupport::Cache.expand_cache_key.
       def fragment_cache_key(key)
+        ActiveSupport::Deprecation.warn(<<-MSG.squish)
+          Calling fragment_cache_key directly is deprecated and will be removed in Rails 6.0.
+          All fragment accessors now use the combined_fragment_cache_key method that retains the key as an array,
+          such that the caching stores can interrogate the parts for cache versions used in
+          recyclable cache keys.
+        MSG
+
         head = self.class.fragment_cache_keys.map { |k| instance_exec(&k) }
         tail = key.is_a?(Hash) ? url_for(key).split("://").last : key
         ActiveSupport::Cache.expand_cache_key([*head, *tail], :views)
       end
 
+      # Given a key (as described in +expire_fragment+), returns
+      # a key array suitable for use in reading, writing, or expiring a
+      # cached fragment. All keys begin with <tt>:views</tt>,
+      # followed by ENV["RAILS_CACHE_ID"] or ENV["RAILS_APP_VERSION"] if set,
+      # followed by any controller-wide key prefix values, ending
+      # with the specified +key+ value.
+      def combined_fragment_cache_key(key)
+        head = self.class.fragment_cache_keys.map { |k| instance_exec(&k) }
+        tail = key.is_a?(Hash) ? url_for(key).split("://").last : key
+        [ :views, (ENV["RAILS_CACHE_ID"] || ENV["RAILS_APP_VERSION"]), *head, *tail ].compact
+      end
+
       # Writes +content+ to the location signified by
       # +key+ (see +expire_fragment+ for acceptable formats).
       def write_fragment(key, content, options = nil)
         return content unless cache_configured?
 
-        key = fragment_cache_key(key)
+        key = combined_fragment_cache_key(key)
         instrument_fragment_cache :write_fragment, key do
           content = content.to_str
           cache_store.write(key, content, options)
@@ -85,7 +107,7 @@ def write_fragment(key, content, options = nil)
       def read_fragment(key, options = nil)
         return unless cache_configured?
 
-        key = fragment_cache_key(key)
+        key = combined_fragment_cache_key(key)
         instrument_fragment_cache :read_fragment, key do
           result = cache_store.read(key, options)
           result.respond_to?(:html_safe) ? result.html_safe : result
@@ -96,7 +118,7 @@ def read_fragment(key, options = nil)
       # +key+ exists (see +expire_fragment+ for acceptable formats).
       def fragment_exist?(key, options = nil)
         return unless cache_configured?
-        key = fragment_cache_key(key)
+        key = combined_fragment_cache_key(key)
 
         instrument_fragment_cache :exist_fragment?, key do
           cache_store.exist?(key, options)
@@ -123,7 +145,7 @@ def fragment_exist?(key, options = nil)
       # method (or <tt>delete_matched</tt>, for Regexp keys).
       def expire_fragment(key, options = nil)
         return unless cache_configured?
-        key = fragment_cache_key(key) unless key.is_a?(Regexp)
+        key = combined_fragment_cache_key(key) unless key.is_a?(Regexp)
 
         instrument_fragment_cache :expire_fragment, key do
           if key.is_a?(Regexp)
@@ -135,8 +157,7 @@ def expire_fragment(key, options = nil)
       end
 
       def instrument_fragment_cache(name, key) # :nodoc:
-        payload = instrument_payload(key)
-        ActiveSupport::Notifications.instrument("#{name}.#{instrument_name}", payload) { yield }
+        ActiveSupport::Notifications.instrument("#{name}.#{instrument_name}", instrument_payload(key)) { yield }
       end
     end
   end

actionpack/lib/action_controller/log_subscriber.rb

@@ -60,9 +60,9 @@ def unpermitted_parameters(event)
       class_eval <<-METHOD, __FILE__, __LINE__ + 1
         def #{method}(event)
           return unless logger.info? && ActionController::Base.enable_fragment_cache_logging
-          key_or_path = event.payload[:key] || event.payload[:path]
+          key         = ActiveSupport::Cache.expand_cache_key(event.payload[:key] || event.payload[:path])
           human_name  = #{method.to_s.humanize.inspect}
-          info("\#{human_name} \#{key_or_path} (\#{event.duration.round(1)}ms)")
+          info("\#{human_name} \#{key} (\#{event.duration.round(1)}ms)")
         end
       METHOD
     end

actionpack/test/controller/caching_test.rb

@@ -26,10 +26,6 @@ def setup
     @controller.request = @request
     @controller.response = @response
   end
-
-  def test_fragment_cache_key
-    assert_equal "views/what a key", @controller.fragment_cache_key("what a key")
-  end
 end
 
 class CachingController < ActionController::Base
@@ -43,6 +39,8 @@ def some_action; end
 end
 
 class FragmentCachingTest < ActionController::TestCase
+  ModelWithKeyAndVersion = Struct.new(:cache_key, :cache_version)
+
   def setup
     super
     @store = ActiveSupport::Cache::MemoryStore.new
@@ -53,12 +51,17 @@ def setup
     @controller.params = @params
     @controller.request = @request
     @controller.response = @response
+
+    @m1v1 = ModelWithKeyAndVersion.new("model/1", "1")
+    @m1v2 = ModelWithKeyAndVersion.new("model/1", "2")
+    @m2v1 = ModelWithKeyAndVersion.new("model/2", "1")
+    @m2v2 = ModelWithKeyAndVersion.new("model/2", "2")
   end
 
   def test_fragment_cache_key
-    assert_equal "views/what a key", @controller.fragment_cache_key("what a key")
-    assert_equal "views/test.host/fragment_caching_test/some_action",
-                  @controller.fragment_cache_key(controller: "fragment_caching_test", action: "some_action")
+    assert_equal [ :views, "what a key" ], @controller.combined_fragment_cache_key("what a key")
+    assert_equal [ :views, "test.host/fragment_caching_test/some_action" ],
+      @controller.combined_fragment_cache_key(controller: "fragment_caching_test", action: "some_action")
   end
 
   def test_read_fragment_with_caching_enabled
@@ -72,6 +75,12 @@ def test_read_fragment_with_caching_disabled
     assert_nil @controller.read_fragment("name")
   end
 
+  def test_read_fragment_with_versioned_model
+    @controller.write_fragment([ "stuff", @m1v1 ], "hello")
+    assert_equal "hello", @controller.read_fragment([ "stuff", @m1v1 ])
+    assert_nil @controller.read_fragment([ "stuff", @m1v2 ])
+  end
+
   def test_fragment_exist_with_caching_enabled
     @store.write("views/name", "value")
     assert @controller.fragment_exist?("name")
@@ -472,9 +481,9 @@ def setup
 
   def test_fragment_cache_key
     @controller.account_id = "123"
-    assert_equal "views/v1/123/what a key", @controller.fragment_cache_key("what a key")
+    assert_equal [ :views, "v1", "123", "what a key" ], @controller.combined_fragment_cache_key("what a key")
 
     @controller.account_id = nil
-    assert_equal "views/v1//what a key", @controller.fragment_cache_key("what a key")
+    assert_equal [ :views, "v1", "what a key" ], @controller.combined_fragment_cache_key("what a key")
   end
 end

actionview/lib/action_view/helpers/cache_helper.rb

@@ -217,10 +217,15 @@ def cache_fragment_name(name = {}, skip_digest: nil, virtual_path: nil)
 
       def fragment_name_with_digest(name, virtual_path)
         virtual_path ||= @virtual_path
+
         if virtual_path
           name = controller.url_for(name).split("://").last if name.is_a?(Hash)
-          digest = Digestor.digest name: virtual_path, finder: lookup_context, dependencies: view_cache_dependencies
-          [ name, digest ]
+
+          if digest = Digestor.digest(name: virtual_path, finder: lookup_context, dependencies: view_cache_dependencies).presence
+            [ "#{virtual_path}:#{digest}", name ]
+          else
+            [ virtual_path, name ]
+          end
         else
           name
         end

actionview/lib/action_view/renderer/partial_renderer/collection_caching.rb

@@ -38,7 +38,7 @@ def collection_by_cache_keys
       end
 
       def expanded_cache_key(key)
-        key = @view.fragment_cache_key(@view.cache_fragment_name(key, virtual_path: @template.virtual_path))
+        key = @view.combined_fragment_cache_key(@view.cache_fragment_name(key, virtual_path: @template.virtual_path))
         key.frozen? ? key.dup : key # #read_multi & #write may require mutability, Dalli 2.6.0.
       end
 

actionview/test/activerecord/relation_cache_test.rb

@@ -10,7 +10,7 @@ def setup
 
   def test_cache_relation_other
     cache(Project.all) { concat("Hello World") }
-    assert_equal "Hello World", controller.cache_store.read("views/projects-#{Project.count}/")
+    assert_equal "Hello World", controller.cache_store.read("views/path/projects-#{Project.count}")
   end
 
   def view_cache_dependencies; end

actionview/test/template/log_subscriber_test.rb

@@ -39,7 +39,7 @@ def set_cache_controller
 
   def set_view_cache_dependencies
     def @view.view_cache_dependencies; []; end
-    def @view.fragment_cache_key(*); "ahoy `controller` dependency"; end
+    def @view.combined_fragment_cache_key(*); "ahoy `controller` dependency"; end
   end
 
   def test_render_file_template

actionview/test/template/render_test.rb

@@ -10,8 +10,8 @@ def setup_view(paths)
     @view = Class.new(ActionView::Base) do
       def view_cache_dependencies; end
 
-      def fragment_cache_key(key)
-        ActiveSupport::Cache.expand_cache_key(key, :views)
+      def combined_fragment_cache_key(key)
+        [ :views, key ]
       end
     end.new(paths, @assigns)
 
@@ -718,6 +718,6 @@ class CachedCustomer < Customer; end
   private
     def cache_key(*names, virtual_path)
       digest = ActionView::Digestor.digest name: virtual_path, finder: @view.lookup_context, dependencies: []
-      @view.fragment_cache_key([ *names, digest ])
+      @view.combined_fragment_cache_key([ "#{virtual_path}:#{digest}", *names ])
     end
 end

activerecord/lib/active_record/integration.rb

@@ -13,6 +13,15 @@ module Integration
       # This is +:usec+, by default.
       class_attribute :cache_timestamp_format, instance_writer: false
       self.cache_timestamp_format = :usec
+
+      ##
+      # :singleton-method:
+      # Indicates whether to use a stable #cache_key method that is accompanied
+      # by a changing version in the #cache_version method.
+      #
+      # This is +false+, by default until Rails 6.0.
+      class_attribute :cache_versioning, instance_writer: false
+      self.cache_versioning = false
     end
 
     # Returns a +String+, which Action Pack uses for constructing a URL to this
@@ -52,25 +61,47 @@ def to_param
     # used to generate the key:
     #
     #   Person.find(5).cache_key(:updated_at, :last_reviewed_at)
+    #
+    # If ActiveRecord::Base.cache_versioning is turned on, no version will be included
+    # in the cache key. The version will instead be supplied by #cache_version. This
+    # separation enables recycling of cache keys.
+    #
+    #   Product.cache_versioning = true
+    #   Product.new.cache_key     # => "products/new"
+    #   Person.find(5).cache_key  # => "people/5" (even if updated_at available)
     def cache_key(*timestamp_names)
       if new_record?
         "#{model_name.cache_key}/new"
       else
-        timestamp = if timestamp_names.any?
-          max_updated_column_timestamp(timestamp_names)
+        if cache_version && timestamp_names.none?
+          "#{model_name.cache_key}/#{id}"
         else
-          max_updated_column_timestamp
-        end
+          timestamp = if timestamp_names.any?
+            max_updated_column_timestamp(timestamp_names)
+          else
+            max_updated_column_timestamp
+          end
 
-        if timestamp
-          timestamp = timestamp.utc.to_s(cache_timestamp_format)
-          "#{model_name.cache_key}/#{id}-#{timestamp}"
-        else
-          "#{model_name.cache_key}/#{id}"
+          if timestamp
+            timestamp = timestamp.utc.to_s(cache_timestamp_format)
+            "#{model_name.cache_key}/#{id}-#{timestamp}"
+          else
+            "#{model_name.cache_key}/#{id}"
+          end
         end
       end
     end
 
+    # Returns a cache version that can be used together with the cache key to form
+    # a recyclable caching scheme. By default, the #updated_at column is used for the
+    # cache_version, but this method can be overwritten to return something else.
+    #
+    # Note, this method will return nil if ActiveRecord::Base.cache_versioning is set to
+    # +false+ (which it is by default until Rails 6.0).
+    def cache_version
+      try(:updated_at).try(:to_i) if cache_versioning
+    end
+
     module ClassMethods
       # Defines your model's +to_param+ method to generate "pretty" URLs
       # using +method_name+, which can be any attribute or method that

activerecord/test/cases/cache_key_test.rb

@@ -4,15 +4,23 @@ module ActiveRecord
   class CacheKeyTest < ActiveRecord::TestCase
     self.use_transactional_tests = false
 
-    class CacheMe < ActiveRecord::Base; end
+    class CacheMe < ActiveRecord::Base
+      self.cache_versioning = false
+    end
+
+    class CacheMeWithVersion < ActiveRecord::Base
+      self.cache_versioning = true
+    end
 
     setup do
       @connection = ActiveRecord::Base.connection
-      @connection.create_table(:cache_mes) { |t| t.timestamps }
+      @connection.create_table(:cache_mes)              { |t| t.timestamps }
+      @connection.create_table(:cache_me_with_versions) { |t| t.timestamps }
     end
 
     teardown do
       @connection.drop_table :cache_mes, if_exists: true
+      @connection.drop_table :cache_me_with_versions, if_exists: true
     end
 
     test "cache_key format is not too precise" do
@@ -21,5 +29,15 @@ class CacheMe < ActiveRecord::Base; end
 
       assert_equal key, record.reload.cache_key
     end
+
+    test "cache_key has no version when versioning is on" do
+      record = CacheMeWithVersion.create
+      assert_equal "cache_me_with_versions/#{record.id}", record.cache_key
+    end
+
+    test "cache_version is only there when versioning is on" do
+      assert CacheMeWithVersion.create.cache_version.present?
+      assert_not CacheMe.create.cache_version.present?
+    end
   end
 end