From 025c691536b22cc3f0ba802f6c303dd6f955c1c3 Mon Sep 17 00:00:00 2001 From: Piotr Chmolowski Date: Sat, 8 Mar 2014 23:09:54 +0100 Subject: [PATCH] Ensure LookupContext in Digestor selects correct variant Related to: #14242 #14243 14293 Variants passed to LookupContext#find() seem to be ignored, so I've used the setter instead: `finder.variants = [ variant ]`. I've also added some more test cases for variants. Hopefully this time passing tests will mean it actually works. --- actionpack/test/controller/caching_test.rb | 21 +++++++++++++++++++ ...ragment_cached_with_variant.html+phone.erb | 3 +++ actionview/lib/action_view/digestor.rb | 5 ++++- .../lib/action_view/helpers/cache_helper.rb | 18 ++++++++++++---- .../fixtures/test/hello_world.html+phone.erb | 1 + .../fixtures/test/hello_world.text+phone.erb | 1 + actionview/test/template/digestor_test.rb | 8 ++++--- .../test/template/lookup_context_test.rb | 14 +++++++++++++ 8 files changed, 63 insertions(+), 8 deletions(-) create mode 100644 actionpack/test/fixtures/functional_caching/formatted_fragment_cached_with_variant.html+phone.erb create mode 100644 actionview/test/fixtures/test/hello_world.html+phone.erb create mode 100644 actionview/test/fixtures/test/hello_world.text+phone.erb diff --git a/actionpack/test/controller/caching_test.rb b/actionpack/test/controller/caching_test.rb index 591d0eccc3..0ba8045cb5 100644 --- a/actionpack/test/controller/caching_test.rb +++ b/actionpack/test/controller/caching_test.rb @@ -164,6 +164,13 @@ class FunctionalCachingController < CachingController end end + def formatted_fragment_cached_with_variant + respond_to do |format| + format.html.phone + format.html + end + end + def fragment_cached_without_digest end end @@ -242,6 +249,20 @@ CACHED @store.read("views/test.host/functional_caching/formatted_fragment_cached/#{template_digest("functional_caching/formatted_fragment_cached", "xml")}") end + + def test_fragment_caching_with_variant + @request.variant = :phone + + get :formatted_fragment_cached_with_variant, :format => "html", :variant => :phone + assert_response :success + expected_body = "\n

PHONE

\n\n" + + assert_equal expected_body, @response.body + + assert_equal "

PHONE

", + @store.read("views/test.host/functional_caching/formatted_fragment_cached_with_variant/#{template_digest("functional_caching/formatted_fragment_cached_with_variant", :html, :phone)}") + end + private def template_digest(name, format, variant = nil) ActionView::Digestor.digest(name: name, format: format, variant: variant, finder: @controller.lookup_context) diff --git a/actionpack/test/fixtures/functional_caching/formatted_fragment_cached_with_variant.html+phone.erb b/actionpack/test/fixtures/functional_caching/formatted_fragment_cached_with_variant.html+phone.erb new file mode 100644 index 0000000000..e523b74ae3 --- /dev/null +++ b/actionpack/test/fixtures/functional_caching/formatted_fragment_cached_with_variant.html+phone.erb @@ -0,0 +1,3 @@ + +<%= cache do %>

PHONE

<% end %> + diff --git a/actionview/lib/action_view/digestor.rb b/actionview/lib/action_view/digestor.rb index da43fef2e3..4e3773abe7 100644 --- a/actionview/lib/action_view/digestor.rb +++ b/actionview/lib/action_view/digestor.rb @@ -126,7 +126,10 @@ module ActionView end def template - @template ||= finder.find(logical_name, [], partial?, formats: [ format ], variants: [ variant ]) + @template ||= begin + finder.variants = [ variant ] + finder.find(logical_name, [], partial?, formats: [ format ]) + end end def source diff --git a/actionview/lib/action_view/helpers/cache_helper.rb b/actionview/lib/action_view/helpers/cache_helper.rb index 30e4e5e329..3177d18c4d 100644 --- a/actionview/lib/action_view/helpers/cache_helper.rb +++ b/actionview/lib/action_view/helpers/cache_helper.rb @@ -165,10 +165,20 @@ module ActionView def fragment_name_with_digest(name) #:nodoc: if @virtual_path - [ - *Array(name.is_a?(Hash) ? controller.url_for(name).split("://").last : name), - Digestor.digest(name: @virtual_path, format: formats.last.to_sym, variant: request.variant, finder: lookup_context, dependencies: view_cache_dependencies) - ] + variant = request.variant.is_a?(Array) ? request.variant.first : request.variant + + options = { + name: @virtual_path, + format: formats.last.to_sym, + variant: variant, + finder: lookup_context, + dependencies: view_cache_dependencies + } + + names = Array(name.is_a?(Hash) ? controller.url_for(name).split("://").last : name) + digest = Digestor.digest(options) + + [*names, digest] else name end diff --git a/actionview/test/fixtures/test/hello_world.html+phone.erb b/actionview/test/fixtures/test/hello_world.html+phone.erb new file mode 100644 index 0000000000..b4f236f878 --- /dev/null +++ b/actionview/test/fixtures/test/hello_world.html+phone.erb @@ -0,0 +1 @@ +Hello phone! \ No newline at end of file diff --git a/actionview/test/fixtures/test/hello_world.text+phone.erb b/actionview/test/fixtures/test/hello_world.text+phone.erb new file mode 100644 index 0000000000..611e2ee442 --- /dev/null +++ b/actionview/test/fixtures/test/hello_world.text+phone.erb @@ -0,0 +1 @@ +Hello texty phone! \ No newline at end of file diff --git a/actionview/test/template/digestor_test.rb b/actionview/test/template/digestor_test.rb index 2406a64310..0580758dab 100644 --- a/actionview/test/template/digestor_test.rb +++ b/actionview/test/template/digestor_test.rb @@ -15,10 +15,12 @@ end class FixtureFinder FIXTURES_DIR = "#{File.dirname(__FILE__)}/../fixtures/digestor" - attr_reader :details + attr_reader :details + attr_accessor :variants def initialize - @details = {} + @details = {} + @variants = [] end def details_key @@ -28,7 +30,7 @@ class FixtureFinder def find(logical_name, keys, partial, options) partial_name = partial ? logical_name.gsub(%r|/([^/]+)$|, '/_\1') : logical_name format = options[:formats].first.to_s - format += "+#{options[:variants].first}" if options[:variants].any? + format += "+#{@variants.first}" if @variants.any? FixtureTemplate.new("digestor/#{partial_name}.#{format}.erb") end diff --git a/actionview/test/template/lookup_context_test.rb b/actionview/test/template/lookup_context_test.rb index ce9485e146..4f7823045e 100644 --- a/actionview/test/template/lookup_context_test.rb +++ b/actionview/test/template/lookup_context_test.rb @@ -93,6 +93,20 @@ class LookupContextTest < ActiveSupport::TestCase assert_equal "Hey verden", template.source end + test "find templates with given variants" do + @lookup_context.formats = [:html] + @lookup_context.variants = [:phone] + + template = @lookup_context.find("hello_world", %w(test)) + assert_equal "Hello phone!", template.source + + @lookup_context.variants = [:phone] + @lookup_context.formats = [:text] + + template = @lookup_context.find("hello_world", %w(test)) + assert_equal "Hello texty phone!", template.source + end + test "found templates respects given formats if one cannot be found from template or handler" do ActionView::Template::Handlers::Builder.expects(:default_format).returns(nil) @lookup_context.formats = [:text]