From 5a41d00455f67a03e3cb77cd89c3b710a7c389d0 Mon Sep 17 00:00:00 2001 From: Dov Murik Date: Sat, 4 Jul 2015 22:34:00 -0400 Subject: [PATCH] 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). --- actionview/CHANGELOG.md | 5 +++++ .../lib/action_view/helpers/cache_helper.rb | 15 +++++++++++++++ actionview/lib/action_view/template.rb | 17 +++++++++++++---- .../lib/action_view/template/handlers/erb.rb | 11 +++++++++-- actionview/test/template/template_test.rb | 8 +++++--- 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 /# Template Collection: (\S+)/, + # 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: (?\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 + (?\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"