From 58b040964f0a61254e38d6aa05d59e2c45f2c374 Mon Sep 17 00:00:00 2001
From: Aaron Lipman <alipman88@gmail.com>
Date: Sun, 29 Dec 2019 14:34:46 -0500
Subject: [PATCH] Enforce fresh ETag header after collection changes

Add ActiveRecord::Relation#cache_key_with_version. This method will be
used by ActionController::ConditionalGet to ensure that when collection
cache versioning is enabled, requests using ConditionalGet don't return
the same ETag header after a collection is modified.

Prior to the introduction of collection cache versioning in
4f2ac80d4cdb01c4d3c1765637bed76cc91c1e35, all collection cache keys
included a version. However, with cache versioning enabled, collection
cache keys remain constant. In turn, ETag headers remain constant,
rendering them ineffective.

This commit takes the cache_key_with_version method used for individual
Active Record objects (from aa8749eb52d7919a438940c9218cad98d892f9ad),
and adds it to collections.
---
 activerecord/CHANGELOG.md                            |  8 ++++++++
 activerecord/lib/active_record/relation.rb           |  9 +++++++++
 activerecord/test/cases/collection_cache_key_test.rb | 10 ++++++++++
 3 files changed, 27 insertions(+)

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index bf15824c0b..aa953b0d64 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,11 @@
+*   Enforce fresh ETag header after a collection's contents change by adding
+    ActiveRecord::Relation#cache_key_with_version. This method will be used by
+    ActionController::ConditionalGet to ensure that when collection cache versioning
+    is enabled, requests using ConditionalGet don't return the same ETag header
+    after a collection is modified. Fixes #38078.
+
+    *Aaron Lipman*
+
 *   Skip test database when running `db:create` or `db:drop` in development
     with `DATABASE_URL` set.
 
diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb
index a475a19a87..78463f115f 100644
--- a/activerecord/lib/active_record/relation.rb
+++ b/activerecord/lib/active_record/relation.rb
@@ -385,6 +385,15 @@ module ActiveRecord
     end
     private :compute_cache_version
 
+    # 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
+
     # Scope all queries to the current scope.
     #
     #   Comment.where(post_id: 1).scoping do
diff --git a/activerecord/test/cases/collection_cache_key_test.rb b/activerecord/test/cases/collection_cache_key_test.rb
index f07f3c42e6..65a3843b26 100644
--- a/activerecord/test/cases/collection_cache_key_test.rb
+++ b/activerecord/test/cases/collection_cache_key_test.rb
@@ -198,6 +198,16 @@ module ActiveRecord
       end
     end
 
+    test "cache_key_with_version contains key and version regardless of collection_cache_versioning setting" do
+      key_with_version_1 = Developer.all.cache_key_with_version
+      assert_match(/\Adevelopers\/query-(\h+)-(\d+)-(\d+)\z/, key_with_version_1)
+
+      with_collection_cache_versioning do
+        key_with_version_2 = Developer.all.cache_key_with_version
+        assert_equal(key_with_version_1, key_with_version_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