Use recyclable cache keys (#29092)

6 files changed, 53 insertions, 28 deletions

diff --git a/actionpack/test/controller/caching_test.rb b/actionpack/test/controller/caching_test.rb
index fa8d9dc09a..dac5861c4b 100644
--- a/actionpack/test/controller/caching_test.rb
+++ b/actionpack/test/controller/caching_test.rb
@@ -26,10 +26,6 @@ class FragmentCachingMetalTest < ActionController::TestCase
     @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 @@ class FragmentCachingTestController < CachingController
 end
 
 class FragmentCachingTest < ActionController::TestCase
+  ModelWithKeyAndVersion = Struct.new(:cache_key, :cache_version)
+
   def setup
     super
     @store = ActiveSupport::Cache::MemoryStore.new
@@ -53,12 +51,25 @@ class FragmentCachingTest < ActionController::TestCase
     @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_deprecated do
+      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")
+    end
+  end
+
+  def test_combined_fragment_cache_key
+    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 +83,12 @@ class FragmentCachingTest < ActionController::TestCase
     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")
@@ -198,7 +215,7 @@ CACHED
     assert_equal expected_body, @response.body
 
     assert_equal "This bit's fragment cached",
-      @store.read("views/test.host/functional_caching/fragment_cached/#{template_digest("functional_caching/fragment_cached")}")
+      @store.read("views/functional_caching/fragment_cached:#{template_digest("functional_caching/fragment_cached")}/fragment")
   end
 
   def test_fragment_caching_in_partials
@@ -207,7 +224,7 @@ CACHED
     assert_match(/Old fragment caching in a partial/, @response.body)
 
     assert_match("Old fragment caching in a partial",
-      @store.read("views/test.host/functional_caching/html_fragment_cached_with_partial/#{template_digest("functional_caching/_partial")}"))
+      @store.read("views/functional_caching/_partial:#{template_digest("functional_caching/_partial")}/test.host/functional_caching/html_fragment_cached_with_partial"))
   end
 
   def test_skipping_fragment_cache_digesting
@@ -237,7 +254,7 @@ CACHED
     assert_match(/Some inline content/, @response.body)
     assert_match(/Some cached content/, @response.body)
     assert_match("Some cached content",
-      @store.read("views/test.host/functional_caching/inline_fragment_cached/#{template_digest("functional_caching/inline_fragment_cached")}"))
+      @store.read("views/functional_caching/inline_fragment_cached:#{template_digest("functional_caching/inline_fragment_cached")}/test.host/functional_caching/inline_fragment_cached"))
   end
 
   def test_fragment_cache_instrumentation
@@ -264,7 +281,7 @@ CACHED
     assert_equal expected_body, @response.body
 
     assert_equal "<p>ERB</p>",
-      @store.read("views/test.host/functional_caching/formatted_fragment_cached/#{template_digest("functional_caching/formatted_fragment_cached")}")
+      @store.read("views/functional_caching/formatted_fragment_cached:#{template_digest("functional_caching/formatted_fragment_cached")}/fragment")
   end
 
   def test_xml_formatted_fragment_caching
@@ -275,7 +292,7 @@ CACHED
     assert_equal expected_body, @response.body
 
     assert_equal "  <p>Builder</p>\n",
-      @store.read("views/test.host/functional_caching/formatted_fragment_cached/#{template_digest("functional_caching/formatted_fragment_cached")}")
+      @store.read("views/functional_caching/formatted_fragment_cached:#{template_digest("functional_caching/formatted_fragment_cached")}/fragment")
   end
 
   def test_fragment_caching_with_variant
@@ -286,7 +303,7 @@ CACHED
     assert_equal expected_body, @response.body
 
     assert_equal "<p>PHONE</p>",
-      @store.read("views/test.host/functional_caching/formatted_fragment_cached_with_variant/#{template_digest("functional_caching/formatted_fragment_cached_with_variant")}")
+      @store.read("views/functional_caching/formatted_fragment_cached_with_variant:#{template_digest("functional_caching/formatted_fragment_cached_with_variant")}/fragment")
   end
 
   private
@@ -412,7 +429,7 @@ class CollectionCacheTest < ActionController::TestCase
   def test_collection_fetches_cached_views
     get :index
     assert_equal 1, @controller.partial_rendered_times
-    assert_customer_cached "david/1", "david, 1"
+    assert_match "david, 1", ActionView::PartialRenderer.collection_cache.read("views/customers/_customer:7c228ab609f0baf0b1f2367469210937/david/1")
 
     get :index
     assert_equal 1, @controller.partial_rendered_times
@@ -444,14 +461,8 @@ class CollectionCacheTest < ActionController::TestCase
 
   def test_caching_with_callable_cache_key
     get :index_with_callable_cache_key
-    assert_customer_cached "cached_david", "david, 1"
+    assert_match "david, 1", ActionView::PartialRenderer.collection_cache.read("views/customers/_customer:7c228ab609f0baf0b1f2367469210937/cached_david")
   end
-
-  private
-    def assert_customer_cached(key, content)
-      assert_match content,
-        ActionView::PartialRenderer.collection_cache.read("views/#{key}/7c228ab609f0baf0b1f2367469210937")
-    end
 end
 
 class FragmentCacheKeyTestController < CachingController
@@ -470,11 +481,21 @@ class FragmentCacheKeyTest < ActionController::TestCase
     @controller.cache_store = @store
   end
 
-  def test_fragment_cache_key
+  def test_combined_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
+
+  def test_combined_fragment_cache_key_with_envs
+    ENV["RAILS_APP_VERSION"] = "55"
+    assert_equal [ :views, "55", "v1", "what a key" ], @controller.combined_fragment_cache_key("what a key")
+
+    ENV["RAILS_CACHE_ID"] = "66"
+    assert_equal [ :views, "66", "v1", "what a key" ], @controller.combined_fragment_cache_key("what a key")
+  ensure
+    ENV["RAILS_CACHE_ID"] = ENV["RAILS_APP_VERSION"] = nil
   end
 end
diff --git a/actionpack/test/fixtures/functional_caching/formatted_fragment_cached.html.erb b/actionpack/test/fixtures/functional_caching/formatted_fragment_cached.html.erb
index 9b88fa1f5a..dfcd423978 100644
--- a/actionpack/test/fixtures/functional_caching/formatted_fragment_cached.html.erb
+++ b/actionpack/test/fixtures/functional_caching/formatted_fragment_cached.html.erb
@@ -1,3 +1,3 @@
 <body>
-<%= cache do %><p>ERB</p><% end %>
+<%= cache("fragment") do %><p>ERB</p><% end %>
 </body>
diff --git a/actionpack/test/fixtures/functional_caching/formatted_fragment_cached.xml.builder b/actionpack/test/fixtures/functional_caching/formatted_fragment_cached.xml.builder
index efdcc28e0f..6599579740 100644
--- a/actionpack/test/fixtures/functional_caching/formatted_fragment_cached.xml.builder
+++ b/actionpack/test/fixtures/functional_caching/formatted_fragment_cached.xml.builder
@@ -1,5 +1,5 @@
 xml.body do
-  cache do
+  cache("fragment") do
     xml.p "Builder"
   end
 end
diff --git a/actionpack/test/fixtures/functional_caching/formatted_fragment_cached_with_variant.html+phone.erb b/actionpack/test/fixtures/functional_caching/formatted_fragment_cached_with_variant.html+phone.erb
index e523b74ae3..abf7017ce6 100644
--- a/actionpack/test/fixtures/functional_caching/formatted_fragment_cached_with_variant.html+phone.erb
+++ b/actionpack/test/fixtures/functional_caching/formatted_fragment_cached_with_variant.html+phone.erb
@@ -1,3 +1,3 @@
 <body>
-<%= cache do %><p>PHONE</p><% end %>
+<%= cache("fragment") do %><p>PHONE</p><% end %>
 </body>
diff --git a/actionpack/test/fixtures/functional_caching/fragment_cached.html.erb b/actionpack/test/fixtures/functional_caching/fragment_cached.html.erb
index fa5e6bd318..1148d83ad7 100644
--- a/actionpack/test/fixtures/functional_caching/fragment_cached.html.erb
+++ b/actionpack/test/fixtures/functional_caching/fragment_cached.html.erb
@@ -1,3 +1,3 @@
 Hello
-<%= cache do %>This bit's fragment cached<% end %>
+<%= cache "fragment" do %>This bit's fragment cached<% end %>
 <%= 'Ciao' %>
diff --git a/actionpack/test/lib/controller/fake_models.rb b/actionpack/test/lib/controller/fake_models.rb
index b768553e7a..ff37d85ed8 100644
--- a/actionpack/test/lib/controller/fake_models.rb
+++ b/actionpack/test/lib/controller/fake_models.rb
@@ -26,6 +26,10 @@ Customer = Struct.new(:name, :id) do
   def persisted?
     id.present?
   end
+
+  def cache_key
+    "#{name}/#{id}"
+  end
 end
 
 Post = Struct.new(:title, :author_name, :body, :secret, :persisted, :written_on, :cost) do