Support explicit defintion of resouce name for collection caching.

If a template includes `# Template Collection: ...` anywhere in its
source, that name will be used as the cache name for the partial that is
rendered for the collection.

This allows users to enable collection caching even if the template
doesn't start with `<% cache ... do %>`.

Moreover, the `# Template Collection: ...` notation is recognized in all
template types (and template types other than ERB can define a
resource_cache_call_pattern method to allow the `cache ... do` pattern
to be recognized too).

5 files changed, 47 insertions, 9 deletions

diff --git a/actionview/CHANGELOG.md b/actionview/CHANGELOG.md
index 069d181674..1f6bb31cd4 100644
--- a/actionview/CHANGELOG.md
+++ b/actionview/CHANGELOG.md
@@ -1,3 +1,8 @@
+*   Allow defining explicit collection caching using a `# Template Collection: ...`
+    directive inside templates.
+
+    *Dov Murik*
+
 *   Asset helpers raise `ArgumentError` when `nil` is passed as a source.
 
     *Anton Kolomiychuk*
diff --git a/actionview/lib/action_view/helpers/cache_helper.rb b/actionview/lib/action_view/helpers/cache_helper.rb
index 8945575860..797d029317 100644
--- a/actionview/lib/action_view/helpers/cache_helper.rb
+++ b/actionview/lib/action_view/helpers/cache_helper.rb
@@ -137,6 +137,21 @@ module ActionView
       # The automatic cache multi read can be turned off like so:
       #
       #   <%= render @notifications, cache: false %>
+      #
+      # === Explicit Collection Caching
+      #
+      # If the partial template doesn't start with a clean cache call as
+      # mentioned above, you can still benefit from collection caching by
+      # adding a special comment format anywhere in the template, like:
+      #
+      #   <%# Template Collection: notification %>
+      #   <% my_helper_that_calls_cache(some_arg, notification) do %>
+      #     <%= notification.name %>
+      #   <% end %>
+      #
+      # The pattern used to match these is <tt>/# Template Collection: (\S+)/</tt>,
+      # so it's important that you type it out just so.
+      # You can only declare one collection in a partial template file.
       def cache(name = {}, options = {}, &block)
         if controller.respond_to?(:perform_caching) && controller.perform_caching
           safe_concat(fragment_for(cache_fragment_name(name, options), options, &block))
diff --git a/actionview/lib/action_view/template.rb b/actionview/lib/action_view/template.rb
index 377ceb534a..d8585514d5 100644
--- a/actionview/lib/action_view/template.rb
+++ b/actionview/lib/action_view/template.rb
@@ -130,7 +130,7 @@ module ActionView
       @source            = source
       @identifier        = identifier
       @handler           = handler
-      @cache_name        = extract_resource_cache_call_name
+      @cache_name        = extract_resource_cache_name
       @compiled          = false
       @original_encoding = nil
       @locals            = details[:locals] || []
@@ -351,9 +351,18 @@ module ActionView
         ActiveSupport::Notifications.instrument("#{action}.action_view", payload, &block)
       end
 
-      def extract_resource_cache_call_name
-        $1 if @handler.respond_to?(:resource_cache_call_pattern) &&
-          @source =~ @handler.resource_cache_call_pattern
+      EXPLICIT_COLLECTION = /# Template Collection: (?<resource_name>\w+)/
+
+      def extract_resource_cache_name
+        if match = @source.match(EXPLICIT_COLLECTION) || resource_cache_call_match
+          match[:resource_name]
+        end
+      end
+
+      def resource_cache_call_match
+        if @handler.respond_to?(:resource_cache_call_pattern)
+          @source.match(@handler.resource_cache_call_pattern)
+        end
       end
 
       def inferred_cache_name
diff --git a/actionview/lib/action_view/template/handlers/erb.rb b/actionview/lib/action_view/template/handlers/erb.rb
index da96347e4d..1f8459c24b 100644
--- a/actionview/lib/action_view/template/handlers/erb.rb
+++ b/actionview/lib/action_view/template/handlers/erb.rb
@@ -125,7 +125,7 @@ module ActionView
 
         # Returns Regexp to extract a cached resource's name from a cache call at the
         # first line of a template.
-        # The extracted cache name is expected in $1.
+        # The extracted cache name is captured as :resource_name.
         #
         #   <% cache notification do %> # => notification
         #
@@ -138,7 +138,14 @@ module ActionView
         #
         #   <% cache notification.event do %> # => nil
         def resource_cache_call_pattern
-          /\A(?:<%#.*%>)*\s*<%\s*cache\(?\s*(\w+)[\s\)]/m
+          /\A
+            (?:<%\#.*%>)*                 # optional initial comment
+            \s*                           # followed by optional spaces or newlines
+            <%\s*cache[\(\s]              # followed by an ERB call to cache
+            \s*                           # followed by optional spaces or newlines
+            (?<resource_name>\w+)         # capture the cache call argument as :resource_name
+            [\s\)]                        # followed by a space or close paren
+          /xm
         end
 
       private
diff --git a/actionview/test/template/template_test.rb b/actionview/test/template/template_test.rb
index d17034e88f..d3b51cd629 100644
--- a/actionview/test/template/template_test.rb
+++ b/actionview/test/template/template_test.rb
@@ -194,14 +194,15 @@ class TestERBTemplate < ActiveSupport::TestCase
     [
       "<%= 'Hello' %>",
       "<% cache_customer = 42 %>",
-      "<% cache customer.name do %><% end %>"
+      "<% 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
+  def test_eligible_for_collection_caching_with_cache_call_or_explicit
     [
       "<% cache customer do %><% end %>",
       "<% cache(customer) do %><% end %>",
@@ -213,7 +214,8 @@ class TestERBTemplate < ActiveSupport::TestCase
       "<%# 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<%# 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"