Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate rendering templates with . in the name #39164

Merged
merged 1 commit into from May 6, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 3 additions & 1 deletion actionpack/test/controller/render_test.rb
Expand Up @@ -365,7 +365,9 @@ def test_dynamic_render_with_absolute_path
def test_dynamic_render
assert File.exist?(File.expand_path("../../test/abstract_unit.rb", __dir__))
assert_raises ActionView::MissingTemplate do
get :dynamic_render, params: { id: '../\\../test/abstract_unit.rb' }
assert_deprecated do
get :dynamic_render, params: { id: '../\\../test/abstract_unit.rb' }
end
end
end

Expand Down
2 changes: 1 addition & 1 deletion actionpack/test/controller/renderer_test.rb
Expand Up @@ -109,7 +109,7 @@ def test_render_component
xml = "<p>Hello world!</p>\n"

assert_equal html, render["respond_to/using_defaults"]
assert_equal xml, render["respond_to/using_defaults.xml.builder"]
assert_equal xml, assert_deprecated { render["respond_to/using_defaults.xml.builder"] }
assert_equal xml, render["respond_to/using_defaults", formats: :xml]
end

Expand Down
9 changes: 9 additions & 0 deletions actionview/lib/action_view/template/resolver.rb
Expand Up @@ -227,6 +227,10 @@ def reject_files_external_to_app(files)
end

def find_template_paths_from_details(path, details)
if path.name.include?(".")
ActiveSupport::Deprecation.warn("Rendering actions with '.' in the name is deprecated: #{path}")
end

query = build_query(path, details)
find_template_paths(query)
end
Expand Down Expand Up @@ -331,6 +335,11 @@ def find_candidate_template_paths(path)
end

def find_template_paths_from_details(path, details)
if path.name.include?(".")
# Fall back to the unoptimized resolver, which will warn
return super
end

candidates = find_candidate_template_paths(path)

regex = build_regex(path, details)
Expand Down
Expand Up @@ -8,7 +8,7 @@ def setup
end

def test_should_have_no_virtual_path
templates = @root_resolver.find_all("hello_world.erb", "#{FIXTURE_LOAD_PATH}/test", false, locale: [], formats: [:html], variants: [], handlers: [:erb])
templates = @root_resolver.find_all("hello_world", "#{FIXTURE_LOAD_PATH}/test", false, locale: [], formats: [:html], variants: [], handlers: [:erb])
assert_equal 1, templates.size
assert_equal "Hello world!", templates[0].source
assert_nil templates[0].virtual_path
Expand Down
16 changes: 12 additions & 4 deletions actionview/test/template/render_test.rb
Expand Up @@ -200,7 +200,9 @@ def test_render_partial_from_default
def test_render_outside_path
assert File.exist?(File.expand_path("../../test/abstract_unit.rb", __dir__))
assert_raises ActionView::MissingTemplate do
@view.render(template: "../\\../test/abstract_unit.rb")
assert_deprecated do
@view.render(template: "../\\../test/abstract_unit.rb")
end
end
end

Expand Down Expand Up @@ -338,8 +340,10 @@ def test_render_partial_collection
end

def test_render_partial_collection_with_partial_name_containing_dot
assert_equal "Hello: davidHello: mary",
@view.render(partial: "test/customer.mobile", collection: [ Customer.new("david"), Customer.new("mary") ])
assert_deprecated do
assert_equal "Hello: davidHello: mary",
@view.render(partial: "test/customer.mobile", collection: [ Customer.new("david"), Customer.new("mary") ])
end
end

def test_render_partial_collection_as_by_string
Expand Down Expand Up @@ -579,7 +583,11 @@ def test_render_does_not_use_unregistered_extension_and_template_handler
def test_render_ignores_templates_with_malformed_template_handlers
%w(malformed malformed.erb malformed.html.erb malformed.en.html.erb).each do |name|
assert File.exist?(File.expand_path("#{FIXTURE_LOAD_PATH}/test/malformed/#{name}~")), "Malformed file (#{name}~) which should be ignored does not exists"
assert_raises(ActionView::MissingTemplate) { @view.render(template: "test/malformed/#{name}") }
assert_raises(ActionView::MissingTemplate) do
ActiveSupport::Deprecation.silence do
@view.render(template: "test/malformed/#{name}")
end
end
end
end

Expand Down
2 changes: 1 addition & 1 deletion actionview/test/template/resolver_shared_tests.rb
Expand Up @@ -169,7 +169,7 @@ def test_templates_sort_by_formats_html_first
def test_virtual_path_is_preserved_with_dot
with_file "test/hello_world.html.erb", "Hello html!"

template = context.find("hello_world.html", "test", false, [], {})
template = assert_deprecated { context.find("hello_world.html", "test", false, [], {}) }
assert_equal "test/hello_world.html", template.virtual_path

template = context.find("hello_world", "test", false, [], {})
Expand Down