Merge pull request #20781 from dubek/collection-cache-explicit

Support explicit defintion of resource name for collection caching.

actionview/CHANGELOG.md

+*   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*

actionview/lib/action_view/helpers/cache_helper.rb

@@ -137,6 +137,21 @@ module CacheHelper
       # 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))

actionview/lib/action_view/template.rb

@@ -130,7 +130,7 @@ def initialize(source, identifier, handler, details)
       @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 @@ def instrument(action, &block)
         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

actionview/lib/action_view/template/handlers/erb.rb

@@ -125,7 +125,7 @@ def call(template)
 
         # 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 @@ def call(template)
         #
         #   <% 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

actionview/test/template/template_test.rb

@@ -194,14 +194,15 @@ def test_not_eligible_for_collection_caching_without_cache_call
     [
       "<%= '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 @@ def test_eligible_for_collection_caching_with_cache_call
       "<%# 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"