Improve detection of partial templates eligible for collection caching.

The regular expression which was used to detect partial templates that
begin with a `<% cache ... do %>` call missed some cases.  This commits
attempts to improve the detection for some cases such as multi-line
comments at the beginning of the template.  The different templates are
listed in two new unit test methods.

Note that specially crafted Ruby code can still evade such `cache`-call
detection: for example, a user might have its own method which itself
calls the Rails `cache` helper. In such a case, the template's code
doesn't start with a literal `cache` string and therefore will not be
eligible for collection caching.

3 files changed, 36 insertions, 1 deletions

diff --git a/actionview/CHANGELOG.md b/actionview/CHANGELOG.md
index 7b6008d5ed..fe47fb47fd 100644
--- a/actionview/CHANGELOG.md
+++ b/actionview/CHANGELOG.md
@@ -1,3 +1,8 @@
+*   Improve detection of partial templates eligible for collection caching,
+    now allowing multi-line comments at the beginning of the template file.
+
+    *Dov Murik*
+
 *   Raise an ArgumentError when a false value for `include_blank` is passed to a
     required select field (to comply with the HTML5 spec).
 
diff --git a/actionview/lib/action_view/template/handlers/erb.rb b/actionview/lib/action_view/template/handlers/erb.rb
index 88a8570706..da96347e4d 100644
--- a/actionview/lib/action_view/template/handlers/erb.rb
+++ b/actionview/lib/action_view/template/handlers/erb.rb
@@ -138,7 +138,7 @@ module ActionView
         #
         #   <% cache notification.event do %> # => nil
         def resource_cache_call_pattern
-          /\A(?:<%#.*%>\n?)?<% cache\(?\s*(\w+\.?)/
+          /\A(?:<%#.*%>)*\s*<%\s*cache\(?\s*(\w+)[\s\)]/m
         end
 
       private
diff --git a/actionview/test/template/template_test.rb b/actionview/test/template/template_test.rb
index aae6a9aa09..d17034e88f 100644
--- a/actionview/test/template/template_test.rb
+++ b/actionview/test/template/template_test.rb
@@ -190,6 +190,36 @@ 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 %>"
+    ].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
+    [
+      "<% 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 %>"
+    ].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