From 1858cc60701dcfb801b9634a1da0177eaeba3807 Mon Sep 17 00:00:00 2001 From: Piotr Chmolowski Date: Sat, 1 Mar 2014 18:59:56 +0100 Subject: 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 --- actionview/CHANGELOG.md | 9 +++ actionview/lib/action_view/digestor.rb | 64 +++++++++++++++++----- actionview/lib/action_view/helpers/cache_helper.rb | 2 +- .../fixtures/digestor/messages/new.html+iphone.erb | 15 +++++ actionview/test/template/digestor_test.rb | 35 +++++++++--- 5 files changed, 103 insertions(+), 22 deletions(-) create mode 100644 actionview/test/fixtures/digestor/messages/new.html+iphone.erb (limited to 'actionview') diff --git a/actionview/CHANGELOG.md b/actionview/CHANGELOG.md index 67b953a782..e46f55a875 100644 --- a/actionview/CHANGELOG.md +++ b/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. diff --git a/actionview/lib/action_view/digestor.rb b/actionview/lib/action_view/digestor.rb index 5570e2a8dc..da43fef2e3 100644 --- a/actionview/lib/action_view/digestor.rb +++ b/actionview/lib/action_view/digestor.rb @@ -9,23 +9,56 @@ module ActionView @@digest_monitor = Monitor.new class << self - def digest(name, format, finder, options = {}) + # Supported options: + # + # * name - Template name + # * format - Template format + # * variant - Variant of +format+ (optional) + # * finder - An instance of ActionView::LookupContext + # * dependencies - An array of dependent views + # * partial - 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 @@ module ActionView 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 @@ module ActionView 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 @@ module ActionView 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 @@ module ActionView 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("-") diff --git a/actionview/lib/action_view/helpers/cache_helper.rb b/actionview/lib/action_view/helpers/cache_helper.rb index b3af1d4da4..30e4e5e329 100644 --- a/actionview/lib/action_view/helpers/cache_helper.rb +++ b/actionview/lib/action_view/helpers/cache_helper.rb @@ -167,7 +167,7 @@ module ActionView 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 diff --git a/actionview/test/fixtures/digestor/messages/new.html+iphone.erb b/actionview/test/fixtures/digestor/messages/new.html+iphone.erb new file mode 100644 index 0000000000..791e1d36b4 --- /dev/null +++ b/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 +%> \ No newline at end of file diff --git a/actionview/test/template/digestor_test.rb b/actionview/test/template/digestor_test.rb index 779a7fb53c..2406a64310 100644 --- a/actionview/test/template/digestor_test.rb +++ b/actionview/test/template/digestor_test.rb @@ -26,7 +26,11 @@ class FixtureFinder 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 @@ class TemplateDigestorTest < ActionView::TestCase 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 @@ class TemplateDigestorTest < ActionView::TestCase 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 @@ class TemplateDigestorTest < ActionView::TestCase 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 -- cgit v1.2.3