Make collection caching explicit.

Having collection caching that wraps templates and automatically tries
to infer if they are cachable proved to be too much of a hassle.

We'd rather have it be something you explicitly turn on.

This removes much of the code and docs to explain the previous automatic
behavior.

This change also removes scoped cache keys and passing cache_options.

2 files changed, 29 insertions, 58 deletions

diff --git a/actionview/test/template/render_test.rb b/actionview/test/template/render_test.rb
index bf811abdd0..3561114d90 100644
--- a/actionview/test/template/render_test.rb
+++ b/actionview/test/template/render_test.rb
@@ -628,56 +628,59 @@ class LazyViewRenderTest < ActiveSupport::TestCase
   end
 end
 
-class CachedCollectionViewRenderTest < CachedViewRenderTest
+class CachedCollectionViewRenderTest < ActiveSupport::TestCase
   class CachedCustomer < Customer; end
 
-  teardown do
-    ActionView::PartialRenderer.collection_cache.clear
-  end
+  include RenderTestCases
 
-  test "with custom key" do
-    customer = Customer.new("david")
-    key = cache_key([customer, 'key'], "test/_customer")
+  # Ensure view path cache is primed
+  setup do
+    view_paths = ActionController::Base.view_paths
+    assert_equal ActionView::OptimizedFileSystemResolver, view_paths.first.class
 
-    ActionView::PartialRenderer.collection_cache.write(key, 'Hello')
+    ActionView::PartialRenderer.collection_cache = ActiveSupport::Cache::MemoryStore.new
 
-    assert_equal "Hello",
-      @view.render(partial: "test/customer", collection: [customer], cache: ->(item) { [item, 'key'] })
+    setup_view(view_paths)
   end
 
-  test "with caching with custom key and rendering with different key" do
-    customer = Customer.new("david")
-    key = cache_key([customer, 'key'], "test/_customer")
+  teardown do
+    GC.start
+    I18n.reload!
+  end
 
-    ActionView::PartialRenderer.collection_cache.write(key, 'Hello')
+  test "collection caching does not cache by default" do
+    customer = Customer.new("david", 1)
+    key = cache_key(customer, "test/_customer")
 
-    assert_equal "Hello: david",
-      @view.render(partial: "test/customer", collection: [customer], cache: ->(item) { [item, 'another_key'] })
+    ActionView::PartialRenderer.collection_cache.write(key, 'Cached')
+
+    assert_not_equal "Cached",
+      @view.render(partial: "test/customer", collection: [customer])
   end
 
-  test "automatic caching with inferred cache name" do
-    customer = CachedCustomer.new("david")
-    key = cache_key(customer, "test/_cached_customer")
+  test "collection caching with partial that doesn't use fragment caching" do
+    customer = Customer.new("david", 1)
+    key = cache_key(customer, "test/_customer")
 
     ActionView::PartialRenderer.collection_cache.write(key, 'Cached')
 
     assert_equal "Cached",
-      @view.render(partial: "test/cached_customer", collection: [customer])
+      @view.render(partial: "test/customer", collection: [customer], cached: true)
   end
 
-  test "automatic caching with as name" do
-    customer = CachedCustomer.new("david")
-    key = cache_key(customer, "test/_cached_customer_as")
+  test "collection caching with cached true" do
+    customer = CachedCustomer.new("david", 1)
+    key = cache_key(customer, "test/_cached_customer")
 
     ActionView::PartialRenderer.collection_cache.write(key, 'Cached')
 
     assert_equal "Cached",
-      @view.render(partial: "test/cached_customer_as", collection: [customer], as: :buyer)
+      @view.render(partial: "test/cached_customer", collection: [customer], cached: true)
   end
 
   private
-    def cache_key(names, virtual_path)
+    def cache_key(*names, virtual_path)
       digest = ActionView::Digestor.digest name: virtual_path, finder: @view.lookup_context, dependencies: []
-      @view.fragment_cache_key([ *Array(names), digest ])
+      @view.fragment_cache_key([ *names, digest ])
     end
 end
diff --git a/actionview/test/template/template_test.rb b/actionview/test/template/template_test.rb
index 921011b073..533c1c3219 100644
--- a/actionview/test/template/template_test.rb
+++ b/actionview/test/template/template_test.rb
@@ -192,38 +192,6 @@ class TestERBTemplate < ActiveSupport::TestCase
     assert_match(/\xFC/, e.message)
   end
 
-  def test_not_eligible_for_collection_caching_without_cache_call
-    [
-      "<%= 'Hello' %>",
-      "<% cache_customer = 42 %>",
-      "<% cache customer.name do %><% end %>",
-      "<% my_cache customer do %><% end %>"
-    ].each do |body|
-      template = new_template(body, virtual_path: "test/foo/_customer")
-      assert_not template.eligible_for_collection_caching?, "Template #{body.inspect} should not be eligible for collection caching"
-    end
-  end
-
-  def test_eligible_for_collection_caching_with_cache_call_or_explicit
-    [
-      "<% cache customer do %><% end %>",
-      "<% cache(customer) do %><% end %>",
-      "<% cache( customer) do %><% end %>",
-      "<% cache( customer ) do %><% end %>",
-      "<%cache customer do %><% end %>",
-      "<%   cache   customer    do %><% end %>",
-      "  <% cache customer do %>\n<% end %>\n",
-      "<%# comment %><% cache customer do %><% end %>",
-      "<%# comment %>\n<% cache customer do %><% end %>",
-      "<%# comment\n line 2\n line 3 %>\n<% cache customer do %><% end %>",
-      "<%# comment 1 %>\n<%# comment 2 %>\n<% cache customer do %><% end %>",
-      "<%# comment 1 %>\n<%# Template Collection: customer %>\n<% my_cache customer do %><% end %>"
-    ].each do |body|
-      template = new_template(body, virtual_path: "test/foo/_customer")
-      assert template.eligible_for_collection_caching?, "Template #{body.inspect} should be eligible for collection caching"
-    end
-  end
-
   def with_external_encoding(encoding)
     old = Encoding.default_external
     Encoding::Converter.new old, encoding if old != encoding