From 2451177f37aa252513ac372d24cba6a3c44c054b Mon Sep 17 00:00:00 2001 From: Javan Makhmali Date: Tue, 14 Jun 2016 17:23:44 -0400 Subject: Fix digesting templates with identical logical names when requesting a format other than the first default --- actionview/lib/action_view/digestor.rb | 9 ++++++--- actionview/test/fixtures/digestor/comments/_comment.json.erb | 1 + actionview/test/template/digestor_test.rb | 12 +++++++++++- 3 files changed, 18 insertions(+), 4 deletions(-) create mode 100644 actionview/test/fixtures/digestor/comments/_comment.json.erb (limited to 'actionview') diff --git a/actionview/lib/action_view/digestor.rb b/actionview/lib/action_view/digestor.rb index b91e61da18..bc42b5dae1 100644 --- a/actionview/lib/action_view/digestor.rb +++ b/actionview/lib/action_view/digestor.rb @@ -15,7 +15,7 @@ module ActionView # * partial - Specifies whether the template is a partial def digest(name:, finder:, dependencies: []) dependencies ||= [] - cache_key = ([ name ].compact + dependencies).join('.') + cache_key = [ name, finder.rendered_format, dependencies ].flatten.compact.join('.') # this is a correctly done double-checked locking idiom # (Concurrent::Map's lookups have volatile semantics) @@ -39,8 +39,11 @@ module ActionView def tree(name, finder, partial = false, seen = {}) logical_name = name.gsub(%r|/_|, "/") - if finder.disable_cache { finder.exists?(logical_name, [], partial) } - template = finder.disable_cache { finder.find(logical_name, [], partial) } + format = finder.rendered_format + formats = finder.formats.without(format).unshift(format) + + if finder.disable_cache { finder.exists?(logical_name, [], partial, [], formats: formats) } + template = finder.disable_cache { finder.find(logical_name, [], partial, [], formats: formats) } if node = seen[template.identifier] # handle cycles in the tree node diff --git a/actionview/test/fixtures/digestor/comments/_comment.json.erb b/actionview/test/fixtures/digestor/comments/_comment.json.erb new file mode 100644 index 0000000000..696eb13917 --- /dev/null +++ b/actionview/test/fixtures/digestor/comments/_comment.json.erb @@ -0,0 +1 @@ +{"content": "Great story!"} diff --git a/actionview/test/template/digestor_test.rb b/actionview/test/template/digestor_test.rb index 4750d2a5a3..3dad70f464 100644 --- a/actionview/test/template/digestor_test.rb +++ b/actionview/test/template/digestor_test.rb @@ -18,6 +18,7 @@ class FixtureFinder < ActionView::LookupContext def initialize(details = {}) super(ActionView::PathSet.new(['digestor']), details, []) + @rendered_format = :html end end @@ -280,6 +281,12 @@ class TemplateDigestorTest < ActionView::TestCase end end + def test_different_formats + html_digest = digest("comments/_comment", format: :html) + json_digest = digest("comments/_comment", format: :json) + + assert_not_equal html_digest, json_digest + end private def assert_logged(message) @@ -309,8 +316,11 @@ class TemplateDigestorTest < ActionView::TestCase def digest(template_name, options = {}) options = options.dup + finder_options = options.extract!(:variants, :format) + + finder.variants = finder_options[:variants] || [] + finder.rendered_format = finder_options[:format] if finder_options[:format] - finder.variants = options.delete(:variants) || [] ActionView::Digestor.digest(name: template_name, finder: finder, dependencies: (options[:dependencies] || [])) end -- cgit v1.2.3 From 1717836e4fbc111d4b339aff875b2a3c301ab7e3 Mon Sep 17 00:00:00 2001 From: Javan Makhmali Date: Wed, 15 Jun 2016 16:25:58 -0400 Subject: Explicity find with the rendered format to handle searching multiple view paths correctly --- actionview/lib/action_view/digestor.rb | 4 +--- actionview/test/fixtures/digestor/api/comments/_comment.json.erb | 1 + actionview/test/fixtures/digestor/comments/_comment.json.erb | 1 - actionview/test/template/digestor_test.rb | 2 +- 4 files changed, 3 insertions(+), 5 deletions(-) create mode 100644 actionview/test/fixtures/digestor/api/comments/_comment.json.erb delete mode 100644 actionview/test/fixtures/digestor/comments/_comment.json.erb (limited to 'actionview') diff --git a/actionview/lib/action_view/digestor.rb b/actionview/lib/action_view/digestor.rb index bc42b5dae1..8d174955a1 100644 --- a/actionview/lib/action_view/digestor.rb +++ b/actionview/lib/action_view/digestor.rb @@ -38,9 +38,7 @@ module ActionView # Create a dependency tree for template named +name+. def tree(name, finder, partial = false, seen = {}) logical_name = name.gsub(%r|/_|, "/") - - format = finder.rendered_format - formats = finder.formats.without(format).unshift(format) + formats = [finder.rendered_format] if finder.disable_cache { finder.exists?(logical_name, [], partial, [], formats: formats) } template = finder.disable_cache { finder.find(logical_name, [], partial, [], formats: formats) } diff --git a/actionview/test/fixtures/digestor/api/comments/_comment.json.erb b/actionview/test/fixtures/digestor/api/comments/_comment.json.erb new file mode 100644 index 0000000000..696eb13917 --- /dev/null +++ b/actionview/test/fixtures/digestor/api/comments/_comment.json.erb @@ -0,0 +1 @@ +{"content": "Great story!"} diff --git a/actionview/test/fixtures/digestor/comments/_comment.json.erb b/actionview/test/fixtures/digestor/comments/_comment.json.erb deleted file mode 100644 index 696eb13917..0000000000 --- a/actionview/test/fixtures/digestor/comments/_comment.json.erb +++ /dev/null @@ -1 +0,0 @@ -{"content": "Great story!"} diff --git a/actionview/test/template/digestor_test.rb b/actionview/test/template/digestor_test.rb index 3dad70f464..a06f681eda 100644 --- a/actionview/test/template/digestor_test.rb +++ b/actionview/test/template/digestor_test.rb @@ -17,7 +17,7 @@ class FixtureFinder < ActionView::LookupContext FIXTURES_DIR = "#{File.dirname(__FILE__)}/../fixtures/digestor" def initialize(details = {}) - super(ActionView::PathSet.new(['digestor']), details, []) + super(ActionView::PathSet.new(['digestor', 'digestor/api']), details, []) @rendered_format = :html end end -- cgit v1.2.3 From 1ee4eebb2b2da07e49bb7d467f5f9d5497bcf718 Mon Sep 17 00:00:00 2001 From: Javan Makhmali Date: Wed, 15 Jun 2016 17:56:17 -0400 Subject: Fix finding templates for digesting for */* requests that render a non-default (html) template --- actionview/lib/action_view/digestor.rb | 9 ++++++--- .../digestor/api/comments/_comments.json.erb | 1 + .../fixtures/digestor/messages/thread.json.erb | 1 + actionview/test/template/digestor_test.rb | 22 ++++++++++++++++++++++ 4 files changed, 30 insertions(+), 3 deletions(-) create mode 100644 actionview/test/fixtures/digestor/api/comments/_comments.json.erb create mode 100644 actionview/test/fixtures/digestor/messages/thread.json.erb (limited to 'actionview') diff --git a/actionview/lib/action_view/digestor.rb b/actionview/lib/action_view/digestor.rb index 8d174955a1..f3c29d663c 100644 --- a/actionview/lib/action_view/digestor.rb +++ b/actionview/lib/action_view/digestor.rb @@ -38,10 +38,13 @@ module ActionView # Create a dependency tree for template named +name+. def tree(name, finder, partial = false, seen = {}) logical_name = name.gsub(%r|/_|, "/") - formats = [finder.rendered_format] - if finder.disable_cache { finder.exists?(logical_name, [], partial, [], formats: formats) } - template = finder.disable_cache { finder.find(logical_name, [], partial, [], formats: formats) } + options = {} + options[:formats] = [finder.rendered_format] if finder.rendered_format + + if finder.disable_cache { finder.exists?(logical_name, [], partial, [], options) } + template = finder.disable_cache { finder.find(logical_name, [], partial, [], options) } + finder.rendered_format ||= template.formats.first if node = seen[template.identifier] # handle cycles in the tree node diff --git a/actionview/test/fixtures/digestor/api/comments/_comments.json.erb b/actionview/test/fixtures/digestor/api/comments/_comments.json.erb new file mode 100644 index 0000000000..c28646a283 --- /dev/null +++ b/actionview/test/fixtures/digestor/api/comments/_comments.json.erb @@ -0,0 +1 @@ +<%= render partial: "comments/comment", collection: commentable.comments %> diff --git a/actionview/test/fixtures/digestor/messages/thread.json.erb b/actionview/test/fixtures/digestor/messages/thread.json.erb new file mode 100644 index 0000000000..e4c1ba97cd --- /dev/null +++ b/actionview/test/fixtures/digestor/messages/thread.json.erb @@ -0,0 +1 @@ +<%= render "comments/comments" %> diff --git a/actionview/test/template/digestor_test.rb b/actionview/test/template/digestor_test.rb index a06f681eda..e39d6ae810 100644 --- a/actionview/test/template/digestor_test.rb +++ b/actionview/test/template/digestor_test.rb @@ -22,6 +22,12 @@ class FixtureFinder < ActionView::LookupContext end end +class ActionView::Digestor::Node + def flatten + [self] + children.flat_map(&:flatten) + end +end + class TemplateDigestorTest < ActionView::TestCase def setup @cwd = Dir.pwd @@ -148,6 +154,17 @@ class TemplateDigestorTest < ActionView::TestCase assert_equal nested_deps, nested_dependencies("messages/show") end + def test_nested_template_deps_with_non_default_rendered_format + finder.rendered_format = nil + nested_deps = [{"comments/comments"=>["comments/comment"]}] + assert_equal nested_deps, nested_dependencies("messages/thread") + end + + def test_template_formats_of_nested_deps_with_non_default_rendered_format + finder.rendered_format = nil + assert_equal [:json, :json, :json], tree_template_formats("messages/thread") + end + def test_recursion_in_renders assert digest("level/recursion") # assert recursion is possible assert_not_nil digest("level/recursion") # assert digest is stored @@ -334,6 +351,11 @@ class TemplateDigestorTest < ActionView::TestCase tree.children.map(&:to_dep_map) end + def tree_template_formats(template_name) + tree = ActionView::Digestor.tree(template_name, finder) + tree.flatten.map(&:template).flat_map(&:formats) + end + def disable_resolver_caching old_caching, ActionView::Resolver.caching = ActionView::Resolver.caching, false yield -- cgit v1.2.3 From fe381d21f7bfd7c6727bba00f5c9e8fd6e2fc7b6 Mon Sep 17 00:00:00 2001 From: Javan Makhmali Date: Wed, 15 Jun 2016 18:00:34 -0400 Subject: Move and rename test --- actionview/test/template/digestor_test.rb | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) (limited to 'actionview') diff --git a/actionview/test/template/digestor_test.rb b/actionview/test/template/digestor_test.rb index e39d6ae810..dd8a6accf8 100644 --- a/actionview/test/template/digestor_test.rb +++ b/actionview/test/template/digestor_test.rb @@ -276,6 +276,13 @@ class TemplateDigestorTest < ActionView::TestCase assert_not_equal digest_phone, digest_fridge_phone end + def test_different_formats_with_same_logical_template_names_results_in_different_digests + html_digest = digest("comments/_comment", format: :html) + json_digest = digest("comments/_comment", format: :json) + + assert_not_equal html_digest, json_digest + end + def test_digest_cache_cleanup_with_recursion first_digest = digest("level/_recursion") second_digest = digest("level/_recursion") @@ -298,13 +305,6 @@ class TemplateDigestorTest < ActionView::TestCase end end - def test_different_formats - html_digest = digest("comments/_comment", format: :html) - json_digest = digest("comments/_comment", format: :json) - - assert_not_equal html_digest, json_digest - end - private def assert_logged(message) old_logger = ActionView::Base.logger -- cgit v1.2.3 From 57f87ae4806c5514df8c12e90501c0df1dbaff82 Mon Sep 17 00:00:00 2001 From: Javan Makhmali Date: Wed, 15 Jun 2016 18:24:26 -0400 Subject: Add test for nested html dependencies with same logical name as templates for other formats --- actionview/test/template/digestor_test.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'actionview') diff --git a/actionview/test/template/digestor_test.rb b/actionview/test/template/digestor_test.rb index dd8a6accf8..410f562f07 100644 --- a/actionview/test/template/digestor_test.rb +++ b/actionview/test/template/digestor_test.rb @@ -162,7 +162,11 @@ class TemplateDigestorTest < ActionView::TestCase def test_template_formats_of_nested_deps_with_non_default_rendered_format finder.rendered_format = nil - assert_equal [:json, :json, :json], tree_template_formats("messages/thread") + assert_equal [:json], tree_template_formats("messages/thread").uniq + end + + def test_template_formats_of_dependencies_with_same_logical_name_and_different_rendered_format + assert_equal [:html], tree_template_formats("messages/show").uniq end def test_recursion_in_renders @@ -353,7 +357,7 @@ class TemplateDigestorTest < ActionView::TestCase def tree_template_formats(template_name) tree = ActionView::Digestor.tree(template_name, finder) - tree.flatten.map(&:template).flat_map(&:formats) + tree.flatten.map(&:template).compact.flat_map(&:formats) end def disable_resolver_caching -- cgit v1.2.3