Add cache_key_with_version and use it in ActiveSupport::Cache.expand_cache_key

This retains the existing behavior of
ActiveSupport::Cache.expand_cache_key (as used by etaging) where the
cache key includes the version.

4 files changed, 50 insertions, 28 deletions

diff --git a/activerecord/lib/active_record/integration.rb b/activerecord/lib/active_record/integration.rb
index ed652e26aa..36ce49d518 100644
--- a/activerecord/lib/active_record/integration.rb
+++ b/activerecord/lib/active_record/integration.rb
@@ -7,8 +7,8 @@ module ActiveRecord
     included do
       ##
       # :singleton-method:
-      # Indicates the format used to generate the timestamp in the cache key.
-      # Accepts any of the symbols in <tt>Time::DATE_FORMATS</tt>.
+      # Indicates the format used to generate the timestamp in the cache key, if
+      # versioning is off. Accepts any of the symbols in <tt>Time::DATE_FORMATS</tt>.
       #
       # This is +:usec+, by default.
       class_attribute :cache_timestamp_format, instance_writer: false
@@ -51,24 +51,16 @@ module ActiveRecord
       id && id.to_s # Be sure to stringify the id for routes
     end
 
-    # Returns a cache key that can be used to identify this record.
+    # Returns a stable cache key that can be used to identify this record.
     #
     #   Product.new.cache_key     # => "products/new"
-    #   Product.find(5).cache_key # => "products/5" (updated_at not available)
-    #   Person.find(5).cache_key  # => "people/5-20071224150000" (updated_at available)
-    #
-    # You can also pass a list of named timestamps, and the newest in the list will be
-    # used to generate the key:
-    #
-    #   Person.find(5).cache_key(:updated_at, :last_reviewed_at)
+    #   Product.find(5).cache_key # => "products/5"
     #
-    # 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.
+    # If ActiveRecord::Base.cache_versioning is turned off, as it was in Rails 5.1 and earlier,
+    # the cache key will also include a version.
     #
-    #   Product.cache_versioning = true
-    #   Product.new.cache_key     # => "products/new"
-    #   Person.find(5).cache_key  # => "people/5" (even if updated_at available)
+    #   Product.cache_versioning = false
+    #   Person.find(5).cache_key  # => "people/5-20071224150000" (updated_at available)
     def cache_key(*timestamp_names)
       if new_record?
         "#{model_name.cache_key}/new"
@@ -77,6 +69,11 @@ module ActiveRecord
           "#{model_name.cache_key}/#{id}"
         else
           timestamp = if timestamp_names.any?
+            ActiveSupport::Deprecation.warn(<<-MSG.squish)
+              Specifying a timestamp name for #cache_key has been deprecated in favor of
+              the explicit #cache_version method that can be overwritten.
+            MSG
+
             max_updated_column_timestamp(timestamp_names)
           else
             max_updated_column_timestamp
@@ -99,9 +96,21 @@ module ActiveRecord
     # 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
+      if cache_versioning && timestamp = try(:updated_at)
+        updated_at.utc.to_s(:usec)
+      end
+    end
+
+    # Returns a cache key along with the version.
+    def cache_key_with_version
+      if version = cache_version
+        "#{cache_key}-#{version}"
+      else
+        cache_key
+      end
     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
diff --git a/activerecord/test/cases/cache_key_test.rb b/activerecord/test/cases/cache_key_test.rb
index f74cb18244..7b8264e6e8 100644
--- a/activerecord/test/cases/cache_key_test.rb
+++ b/activerecord/test/cases/cache_key_test.rb
@@ -39,5 +39,13 @@ module ActiveRecord
       assert CacheMeWithVersion.create.cache_version.present?
       assert_not CacheMe.create.cache_version.present?
     end
+
+    test "cache_key_with_version always has both key and version" do
+      r1 = CacheMeWithVersion.create
+      assert_equal "active_record/cache_key_test/cache_me_with_versions/#{r1.id}-#{r1.updated_at.to_s(:usec)}", r1.cache_key_with_version
+
+      r2 = CacheMe.create
+      assert_equal "active_record/cache_key_test/cache_mes/#{r2.id}-#{r2.updated_at.to_s(:usec)}", r2.cache_key_with_version
+    end
   end
 end
diff --git a/activerecord/test/cases/integration_test.rb b/activerecord/test/cases/integration_test.rb
index 7ffa86c42f..9104976126 100644
--- a/activerecord/test/cases/integration_test.rb
+++ b/activerecord/test/cases/integration_test.rb
@@ -168,14 +168,18 @@ class IntegrationTest < ActiveRecord::TestCase
   end
 
   def test_named_timestamps_for_cache_key
-    owner = owners(:blackbeard)
-    assert_equal "owners/#{owner.id}-#{owner.happy_at.utc.to_s(:usec)}", owner.cache_key(:updated_at, :happy_at)
+    assert_deprecated do
+      owner = owners(:blackbeard)
+      assert_equal "owners/#{owner.id}-#{owner.happy_at.utc.to_s(:usec)}", owner.cache_key(:updated_at, :happy_at)
+    end
   end
 
   def test_cache_key_when_named_timestamp_is_nil
-    owner = owners(:blackbeard)
-    owner.happy_at = nil
-    assert_equal "owners/#{owner.id}", owner.cache_key(:happy_at)
+    assert_deprecated do
+      owner = owners(:blackbeard)
+      owner.happy_at = nil
+      assert_equal "owners/#{owner.id}", owner.cache_key(:happy_at)
+    end
   end
 
   def test_cache_key_is_stable_with_versioning_on
@@ -213,13 +217,13 @@ class IntegrationTest < ActiveRecord::TestCase
     Developer.cache_versioning = true
 
     developer = Developer.first
-    first_key = developer.cache_key(:updated_at)
+    first_key = developer.cache_key_with_version
 
     travel 10.seconds do
       developer.touch
     end
 
-    second_key = developer.cache_key(:updated_at)
+    second_key = developer.cache_key_with_version
 
     assert_not_equal first_key, second_key
   ensure
diff --git a/activesupport/lib/active_support/cache.rb b/activesupport/lib/active_support/cache.rb
index 258140fe1d..cea603e1b3 100644
--- a/activesupport/lib/active_support/cache.rb
+++ b/activesupport/lib/active_support/cache.rb
@@ -88,10 +88,11 @@ module ActiveSupport
       private
         def retrieve_cache_key(key)
           case
-          when key.respond_to?(:cache_key) then key.cache_key
-          when key.is_a?(Array)            then key.map { |element| retrieve_cache_key(element) }.to_param
-          when key.respond_to?(:to_a)      then retrieve_cache_key(key.to_a)
-          else                                  key.to_param
+          when key.respond_to?(:cache_key_with_version) then key.cache_key_with_version
+          when key.respond_to?(:cache_key)              then key.cache_key
+          when key.is_a?(Array)                         then key.map { |element| retrieve_cache_key(element) }.to_param
+          when key.respond_to?(:to_a)                   then retrieve_cache_key(key.to_a)
+          else                                               key.to_param
           end.to_s
         end