Skip to content
Browse files

Variants in ActionView::Digestor

Take variants into account when calculating template digests in
ActionView::Digest.

Digestor#digest now takes a hash as an argument to support variants and
allow more flexibility in the future. Old-style arguments have been
deprecated.

Fixes #14242
  • Loading branch information...
1 parent 8ed0f54 commit 1858cc60701dcfb801b9634a1da0177eaeba3807 @pch pch committed Mar 1, 2014
View
4 actionpack/test/controller/caching_test.rb
@@ -243,8 +243,8 @@ def test_xml_formatted_fragment_caching
end
private
- def template_digest(name, format)
- ActionView::Digestor.digest(name, format, @controller.lookup_context)
+ def template_digest(name, format, variant = nil)
+ ActionView::Digestor.digest(name: name, format: format, variant: variant, finder: @controller.lookup_context)
end
end
View
9 actionview/CHANGELOG.md
@@ -5,4 +5,13 @@
*Sergey Prikhodko*
+* Take variants into account when calculating template digests in ActionView::Digestor.
+
+ The arguments to ActionView::Digestor#digest are now being passed as a hash
+ to support variants and allow more flexibility in the future. The support for
+ regular (required) arguments is deprecated and will be removed in Rails 5.0 or later.
+
+ *Piotr Chmolowski, Łukasz Strzałkowski*
+
+
Please check [4-1-stable](https://github.com/rails/rails/blob/4-1-stable/actionview/CHANGELOG.md) for previous changes.
View
64 actionview/lib/action_view/digestor.rb
@@ -9,23 +9,56 @@ class Digestor
@@digest_monitor = Monitor.new
class << self
- def digest(name, format, finder, options = {})
+ # Supported options:
+ #
+ # * <tt>name</tt> - Template name
+ # * <tt>format</tt> - Template format
+ # * <tt>variant</tt> - Variant of +format+ (optional)
+ # * <tt>finder</tt> - An instance of ActionView::LookupContext
+ # * <tt>dependencies</tt> - An array of dependent views
+ # * <tt>partial</tt> - Specifies whether the template is a partial
+ def digest(*args)
+ options = _setup_options(*args)
+
+ name = options[:name]
+ format = options[:format]
+ variant = options[:variant]
+ finder = options[:finder]
+
details_key = finder.details_key.hash
dependencies = Array.wrap(options[:dependencies])
- cache_key = ([name, details_key, format] + dependencies).join('.')
+ cache_key = ([name, details_key, format, variant].compact + dependencies).join('.')
# this is a correctly done double-checked locking idiom
# (ThreadSafe::Cache's lookups have volatile semantics)
@@cache[cache_key] || @@digest_monitor.synchronize do
@@cache.fetch(cache_key) do # re-check under lock
- compute_and_store_digest(cache_key, name, format, finder, options)
+ compute_and_store_digest(cache_key, options)
end
end
end
+ def _setup_options(*args)
+ unless args.first.is_a?(Hash)
+ ActiveSupport::Deprecation.warn("Arguments to ActionView::Digestor should be provided as a hash. The support for regular arguments will be removed in Rails 5.0 or later")
+
+ {
+ name: args.first,
+ format: args.second,
+ finder: args.third,
+ }.merge(args.fourth || {})
+ else
+ options = args.first
+ options.assert_valid_keys(:name, :format, :variant, :finder, :dependencies, :partial)
+
+ options
+ end
+ end
+
private
- def compute_and_store_digest(cache_key, name, format, finder, options) # called under @@digest_monitor lock
- klass = if options[:partial] || name.include?("/_")
+
+ def compute_and_store_digest(cache_key, options) # called under @@digest_monitor lock
+ klass = if options[:partial] || options[:name].include?("/_")
# Prevent re-entry or else recursive templates will blow the stack.
# There is no need to worry about other threads seeing the +false+ value,
# as they will then have to wait for this thread to let go of the @@digest_monitor lock.
@@ -35,20 +68,25 @@ def compute_and_store_digest(cache_key, name, format, finder, options) # called
Digestor
end
- digest = klass.new(name, format, finder, options).digest
+ digest = klass.new(options).digest
# Store the actual digest if config.cache_template_loading is true
@@cache[cache_key] = stored_digest = digest if ActionView::Resolver.caching?
digest
ensure
# something went wrong or ActionView::Resolver.caching? is false, make sure not to corrupt the @@cache
- @@cache.delete_pair(cache_key, false) if pre_stored && !stored_digest
+ @@cache.delete_pair(cache_key, false) if pre_stored && !stored_digest
end
end
- attr_reader :name, :format, :finder, :options
+ attr_reader :name, :format, :variant, :finder, :options
+
+ def initialize(*args)
+ @options = self.class._setup_options(*args)
- def initialize(name, format, finder, options={})
- @name, @format, @finder, @options = name, format, finder, options
+ @name = @options.delete(:name)
+ @format = @options.delete(:format)
+ @variant = @options.delete(:variant)
+ @finder = @options.delete(:finder)
end
def digest
@@ -68,7 +106,7 @@ def dependencies
def nested_dependencies
dependencies.collect do |dependency|
- dependencies = PartialDigestor.new(dependency, format, finder).nested_dependencies
+ dependencies = PartialDigestor.new(name: dependency, format: format, finder: finder).nested_dependencies
dependencies.any? ? { dependency => dependencies } : dependency
end
end
@@ -88,7 +126,7 @@ def partial?
end
def template
- @template ||= finder.find(logical_name, [], partial?, formats: [ format ])
+ @template ||= finder.find(logical_name, [], partial?, formats: [ format ], variants: [ variant ])
end
def source
@@ -97,7 +135,7 @@ def source
def dependency_digest
template_digests = dependencies.collect do |template_name|
- Digestor.digest(template_name, format, finder, partial: true)
+ Digestor.digest(name: template_name, format: format, finder: finder, partial: true)
end
(template_digests + injected_dependencies).join("-")
View
2 actionview/lib/action_view/helpers/cache_helper.rb
@@ -167,7 +167,7 @@ def fragment_name_with_digest(name) #:nodoc:
if @virtual_path
[
*Array(name.is_a?(Hash) ? controller.url_for(name).split("://").last : name),
- Digestor.digest(@virtual_path, formats.last.to_sym, lookup_context, dependencies: view_cache_dependencies)
+ Digestor.digest(name: @virtual_path, format: formats.last.to_sym, variant: request.variant, finder: lookup_context, dependencies: view_cache_dependencies)
]
else
name
View
15 actionview/test/fixtures/digestor/messages/new.html+iphone.erb
@@ -0,0 +1,15 @@
+<%# Template Dependency: messages/message %>
+
+<%= render "header" %>
+<%= render "comments/comments" %>
+
+<%= render "messages/actions/move" %>
+
+<%= render @message.history.events %>
+
+<%# render "something_missing" %>
+<%# render "something_missing_1" %>
+
+<%
+ # Template Dependency: messages/form
+%>
View
35 actionview/test/template/digestor_test.rb
@@ -26,7 +26,11 @@ def details_key
end
def find(logical_name, keys, partial, options)
- FixtureTemplate.new("digestor/#{partial ? logical_name.gsub(%r|/([^/]+)$|, '/_\1') : logical_name}.#{options[:formats].first}.erb")
+ partial_name = partial ? logical_name.gsub(%r|/([^/]+)$|, '/_\1') : logical_name
+ format = options[:formats].first.to_s
+ format += "+#{options[:variants].first}" if options[:variants].any?
+
+ FixtureTemplate.new("digestor/#{partial_name}.#{format}.erb")
end
end
@@ -194,6 +198,13 @@ def test_old_style_hash_in_render_invocation
end
end
+ def test_variants
+ assert_digest_difference("messages/new", false, variant: :iphone) do
+ change_template("messages/new", :iphone)
+ change_template("messages/_header", :iphone)
+ end
+ end
+
def test_dependencies_via_options_results_in_different_digest
digest_plain = digest("comments/_comment")
digest_fridge = digest("comments/_comment", dependencies: ["fridge"])
@@ -242,6 +253,11 @@ def test_digest_cache_cleanup_with_recursion_and_template_caching_off
ActionView::Resolver.caching = resolver_before
end
+ def test_arguments_deprecation
+ assert_deprecated(/should be provided as a hash/) { ActionView::Digestor.digest('messages/show', :html, finder) }
+ assert_deprecated(/should be provided as a hash/) { ActionView::Digestor.new('messages/show', :html, finder) }
+ end
+
private
def assert_logged(message)
old_logger = ActionView::Base.logger
@@ -258,26 +274,29 @@ def assert_logged(message)
end
end
- def assert_digest_difference(template_name, persistent = false)
- previous_digest = digest(template_name)
+ def assert_digest_difference(template_name, persistent = false, options = {})
+ previous_digest = digest(template_name, options)
ActionView::Digestor.cache.clear unless persistent
yield
- assert previous_digest != digest(template_name), "digest didn't change"
+ assert previous_digest != digest(template_name, options), "digest didn't change"
ActionView::Digestor.cache.clear
end
- def digest(template_name, options={})
- ActionView::Digestor.digest(template_name, :html, finder, options)
+ def digest(template_name, options = {})
+ options = options.dup
+ ActionView::Digestor.digest({ name: template_name, format: :html, finder: finder }.merge(options))
end
def finder
@finder ||= FixtureFinder.new
end
- def change_template(template_name)
- File.open("digestor/#{template_name}.html.erb", "w") do |f|
+ def change_template(template_name, variant = nil)
+ variant = "+#{variant}" if variant.present?
+
+ File.open("digestor/#{template_name}.html#{variant}.erb", "w") do |f|
f.write "\nTHIS WAS CHANGED!"
end
end

0 comments on commit 1858cc6

Please sign in to comment.
Something went wrong with that request. Please try again.