Merge pull request #34378 'collection-cache-versioning'

Signed-off-by: Kasper Timm Hansen <kaspth@gmail.com>

5 files changed, 93 insertions, 15 deletions

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 8daa6c0ce5..59669ca26e 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,14 @@
+*   Add `ActiveRecord::Relation#cache_version` to support recyclable cache keys via
+    the versioned entries in `ActiveSupport::Cache`. This also means that
+    `ActiveRecord::Relation#cache_key` will now return a stable key that does not
+    include the max timestamp or count any more.
+
+    NOTE: This feature is turned off by default, and `cache_key` will still return
+    cache keys with timestamps until you set `ActiveRecord::Base.collection_cache_versioning = true`.
+    That's the setting for all new apps on Rails 6.0+
+
+    *Lachlan Sylvester*
+
 *   Fix dirty tracking for `touch` to track saved changes.
 
     Fixes #33429.
diff --git a/activerecord/lib/active_record/integration.rb b/activerecord/lib/active_record/integration.rb
index b769541e95..c745bc1330 100644
--- a/activerecord/lib/active_record/integration.rb
+++ b/activerecord/lib/active_record/integration.rb
@@ -22,6 +22,14 @@ module ActiveRecord
       #
       # This is +true+, by default on Rails 5.2 and above.
       class_attribute :cache_versioning, instance_writer: false, default: false
+
+      ##
+      # :singleton-method:
+      # Indicates whether to use a stable #cache_key method that is accompanied
+      # by a changing version in the #cache_version method on collections.
+      #
+      # This is +false+, by default until Rails 6.1.
+      class_attribute :collection_cache_versioning, instance_writer: false, default: false
     end
 
     # Returns a +String+, which Action Pack uses for constructing a URL to this
diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb
index cd62b0b881..8eb71e6454 100644
--- a/activerecord/lib/active_record/relation.rb
+++ b/activerecord/lib/active_record/relation.rb
@@ -291,27 +291,23 @@ module ActiveRecord
       limit_value ? records.many? : size > 1
     end
 
-    # Returns a cache key that can be used to identify the records fetched by
-    # this query. The cache key is built with a fingerprint of the sql query,
-    # the number of records matched by the query and a timestamp of the last
-    # updated record. When a new record comes to match the query, or any of
-    # the existing records is updated or deleted, the cache key changes.
+    # Returns a stable cache key that can be used to identify this query.
+    # The cache key is built with a fingerprint of the SQL query.
     #
-    #   Product.where("name like ?", "%Cosmic Encounter%").cache_key
-    #   # => "products/query-1850ab3d302391b85b8693e941286659-1-20150714212553907087000"
+    #    Product.where("name like ?", "%Cosmic Encounter%").cache_key
+    #    # => "products/query-1850ab3d302391b85b8693e941286659"
     #
-    # If the collection is loaded, the method will iterate through the records
-    # to generate the timestamp, otherwise it will trigger one SQL query like:
+    # If ActiveRecord::Base.collection_cache_versioning is turned off, as it was
+    # in Rails 6.0 and earlier, the cache key will also include a version.
     #
-    #    SELECT COUNT(*), MAX("products"."updated_at") FROM "products" WHERE (name like '%Cosmic Encounter%')
+    #    ActiveRecord::Base.collection_cache_versioning = false
+    #    Product.where("name like ?", "%Cosmic Encounter%").cache_key
+    #    # => "products/query-1850ab3d302391b85b8693e941286659-1-20150714212553907087000"
     #
     # You can also pass a custom timestamp column to fetch the timestamp of the
     # last updated record.
     #
     #   Product.where("name like ?", "%Game%").cache_key(:last_reviewed_at)
-    #
-    # You can customize the strategy to generate the key on a per model basis
-    # overriding ActiveRecord::Base#collection_cache_key.
     def cache_key(timestamp_column = :updated_at)
       @cache_keys ||= {}
       @cache_keys[timestamp_column] ||= @klass.collection_cache_key(self, timestamp_column)
@@ -321,6 +317,31 @@ module ActiveRecord
       query_signature = ActiveSupport::Digest.hexdigest(to_sql)
       key = "#{klass.model_name.cache_key}/query-#{query_signature}"
 
+      if cache_version(timestamp_column)
+        key
+      else
+        "#{key}-#{compute_cache_version(timestamp_column)}"
+      end
+    end
+
+    # Returns a cache version that can be used together with the cache key to form
+    # a recyclable caching scheme. The cache version is built with the number of records
+    # matching the query, and the timestamp of the last updated record. When a new record
+    # comes to match the query, or any of the existing records is updated or deleted,
+    # the cache version changes.
+    #
+    # If the collection is loaded, the method will iterate through the records
+    # to generate the timestamp, otherwise it will trigger one SQL query like:
+    #
+    #    SELECT COUNT(*), MAX("products"."updated_at") FROM "products" WHERE (name like '%Cosmic Encounter%')
+    def cache_version(timestamp_column = :updated_at)
+      if collection_cache_versioning
+        @cache_versions ||= {}
+        @cache_versions[timestamp_column] ||= compute_cache_version(timestamp_column)
+      end
+    end
+
+    def compute_cache_version(timestamp_column) # :nodoc:
       if loaded? || distinct_value
         size = records.size
         if size > 0
@@ -356,9 +377,9 @@ module ActiveRecord
       end
 
       if timestamp
-        "#{key}-#{size}-#{timestamp.utc.to_s(cache_timestamp_format)}"
+        "#{size}-#{timestamp.utc.to_s(cache_timestamp_format)}"
       else
-        "#{key}-#{size}"
+        "#{size}"
       end
     end
 
diff --git a/activerecord/test/cases/collection_cache_key_test.rb b/activerecord/test/cases/collection_cache_key_test.rb
index 483383257b..f07f3c42e6 100644
--- a/activerecord/test/cases/collection_cache_key_test.rb
+++ b/activerecord/test/cases/collection_cache_key_test.rb
@@ -171,5 +171,39 @@ module ActiveRecord
 
       assert_match(/\Adevelopers\/query-(\h+)-(\d+)-(\d+)\z/, developers.cache_key)
     end
+
+    test "cache_key should be stable when using collection_cache_versioning" do
+      with_collection_cache_versioning do
+        developers = Developer.where(salary: 100000)
+
+        assert_match(/\Adevelopers\/query-(\h+)\z/, developers.cache_key)
+
+        /\Adevelopers\/query-(\h+)\z/ =~ developers.cache_key
+
+        assert_equal ActiveSupport::Digest.hexdigest(developers.to_sql), $1
+      end
+    end
+
+    test "cache_version for relation" do
+      with_collection_cache_versioning do
+        developers = Developer.where(salary: 100000).order(updated_at: :desc)
+        last_developer_timestamp = developers.first.updated_at
+
+        assert_match(/(\d+)-(\d+)\z/, developers.cache_version)
+
+        /(\d+)-(\d+)\z/ =~ developers.cache_version
+
+        assert_equal developers.count.to_s, $1
+        assert_equal last_developer_timestamp.to_s(ActiveRecord::Base.cache_timestamp_format), $2
+      end
+    end
+
+    def with_collection_cache_versioning(value = true)
+      @old_collection_cache_versioning = ActiveRecord::Base.collection_cache_versioning
+      ActiveRecord::Base.collection_cache_versioning = value
+      yield
+    ensure
+      ActiveRecord::Base.collection_cache_versioning = @old_collection_cache_versioning
+    end
   end
 end
diff --git a/railties/lib/rails/application/configuration.rb b/railties/lib/rails/application/configuration.rb
index b79dbdbc6f..d743c1c0d9 100644
--- a/railties/lib/rails/application/configuration.rb
+++ b/railties/lib/rails/application/configuration.rb
@@ -142,6 +142,10 @@ module Rails
             active_storage.queues.analysis = :active_storage_analysis
             active_storage.queues.purge    = :active_storage_purge
           end
+
+          if respond_to?(:active_record)
+            active_record.collection_cache_versioning = true
+          end
         else
           raise "Unknown version #{target_version.to_s.inspect}"
         end