From 2f128a82e66f181577ff77d83d4ca02659aa8a8d Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 25 Feb 2019 11:53:55 -0800 Subject: Always pass a format to the ActionView::Template constructor This means we can eliminate nil checks and remove some mutations from the `decorate` method. --- actionpack/test/dispatch/debug_exceptions_test.rb | 2 +- actionview/lib/action_view/template.rb | 7 +++++-- actionview/lib/action_view/template/resolver.rb | 18 +++++++++++++----- actionview/lib/action_view/testing/resolvers.rb | 4 ++-- actionview/test/abstract_unit.rb | 2 +- actionview/test/template/template_test.rb | 6 ++++-- 6 files changed, 26 insertions(+), 13 deletions(-) diff --git a/actionpack/test/dispatch/debug_exceptions_test.rb b/actionpack/test/dispatch/debug_exceptions_test.rb index 6914fb66f9..c33b0e1c14 100644 --- a/actionpack/test/dispatch/debug_exceptions_test.rb +++ b/actionpack/test/dispatch/debug_exceptions_test.rb @@ -39,7 +39,7 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest def call(env) env["action_dispatch.show_detailed_exceptions"] = @detailed req = ActionDispatch::Request.new(env) - template = ActionView::Template.new(File.read(__FILE__), __FILE__, ActionView::Template::Handlers::Raw.new, {}) + template = ActionView::Template.new(File.read(__FILE__), __FILE__, ActionView::Template::Handlers::Raw.new, format: :html) case req.path when "/pass" diff --git a/actionview/lib/action_view/template.rb b/actionview/lib/action_view/template.rb index 133a316405..9e8f17d746 100644 --- a/actionview/lib/action_view/template.rb +++ b/actionview/lib/action_view/template.rb @@ -128,8 +128,11 @@ module ActionView attr_reader :variable - def initialize(source, identifier, handler, details) - format = details[:format] || (handler.default_format if handler.respond_to?(:default_format)) + def initialize(source, identifier, handler, format: nil, **details) + unless format + ActiveSupport::Deprecation.warn "ActionView::Template#initialize requires a format parameter" + format = :html + end @source = source @identifier = identifier diff --git a/actionview/lib/action_view/template/resolver.rb b/actionview/lib/action_view/template/resolver.rb index 3b4594942b..8232494746 100644 --- a/actionview/lib/action_view/template/resolver.rb +++ b/actionview/lib/action_view/template/resolver.rb @@ -196,7 +196,6 @@ module ActionView cached = nil templates.each do |t| t.locals = locals - t.formats = details[:formats] || [:html] if t.formats.empty? t.variants = details[:variants] || [] if t.variants.empty? t.virtual_path ||= (cached ||= build_path(*path_info)) end @@ -225,7 +224,7 @@ module ActionView template_paths = reject_files_external_to_app(template_paths) unless outside_app_allowed template_paths.map do |template| - handler, format, variant = extract_handler_and_format_and_variant(template) + handler, format, variant = extract_handler_and_format_and_variant(template, formats.first) FileTemplate.new(File.expand_path(template), handler, virtual_path: path.virtual, @@ -292,7 +291,7 @@ module ActionView # Extract handler, formats and variant from path. If a format cannot be found neither # from the path, or the handler, we should return the array of formats given # to the resolver. - def extract_handler_and_format_and_variant(path) + def extract_handler_and_format_and_variant(path, query_format) pieces = File.basename(path).split(".") pieces.shift @@ -300,9 +299,18 @@ module ActionView handler = Template.handler_for_extension(extension) format, variant = pieces.last.split(EXTENSIONS[:variants], 2) if pieces.last - format &&= Template::Types[format] + format = if format + Template::Types[format] + else + if handler.respond_to?(:default_format) # default_format can return nil + handler.default_format + else + query_format + end + end - [handler, format, variant] + # Template::Types[format] and handler.default_format can return nil + [handler, format || query_format, variant] end end diff --git a/actionview/lib/action_view/testing/resolvers.rb b/actionview/lib/action_view/testing/resolvers.rb index d6203b95c5..275e2e9182 100644 --- a/actionview/lib/action_view/testing/resolvers.rb +++ b/actionview/lib/action_view/testing/resolvers.rb @@ -34,7 +34,7 @@ module ActionView #:nodoc: @hash.each do |_path, array| source, updated_at = array next unless query.match?(_path) - handler, format, variant = extract_handler_and_format_and_variant(_path) + handler, format, variant = extract_handler_and_format_and_variant(_path, :html) templates << Template.new(source, _path, handler, virtual_path: path.virtual, format: format, @@ -49,7 +49,7 @@ module ActionView #:nodoc: class NullResolver < PathResolver def query(path, exts, _, _) - handler, format, variant = extract_handler_and_format_and_variant(path) + handler, format, variant = extract_handler_and_format_and_variant(path, :html) [ActionView::Template.new("Template generated by Null Resolver", path.virtual, handler, virtual_path: path.virtual, format: format, variant: variant)] end end diff --git a/actionview/test/abstract_unit.rb b/actionview/test/abstract_unit.rb index b649b3c9dd..e0a1f59755 100644 --- a/actionview/test/abstract_unit.rb +++ b/actionview/test/abstract_unit.rb @@ -60,7 +60,7 @@ module RenderERBUtils string.strip, "test template", ActionView::Template.handler_for_extension(:erb), - {}) + format: :html) view = ActionView::Base.with_empty_template_cache template.render(view.empty, {}).strip diff --git a/actionview/test/template/template_test.rb b/actionview/test/template/template_test.rb index a069c8f2d0..36caef28c2 100644 --- a/actionview/test/template/template_test.rb +++ b/actionview/test/template/template_test.rb @@ -38,7 +38,8 @@ class TestERBTemplate < ActiveSupport::TestCase "<%= @virtual_path %>", "partial", ERBHandler, - virtual_path: "partial" + virtual_path: "partial", + format: :html ) end @@ -55,7 +56,8 @@ class TestERBTemplate < ActiveSupport::TestCase end end - def new_template(body = "<%= hello %>", details = { format: :html }) + def new_template(body = "<%= hello %>", details = {}) + details = { format: :html }.merge details ActionView::Template.new(body.dup, "hello template", details.fetch(:handler) { ERBHandler }, { virtual_path: "hello" }.merge!(details)) end -- cgit v1.2.3 From bc74959aad2073b294f537b0e928e0bbc23abfea Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 25 Feb 2019 11:58:19 -0800 Subject: Dereference the format type before template construction The format should always be exactly one symbol. Now we don't need to check whether or not the format is a `Type` in the constructor. --- actionview/lib/action_view/template.rb | 2 +- actionview/lib/action_view/template/resolver.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/actionview/lib/action_view/template.rb b/actionview/lib/action_view/template.rb index 9e8f17d746..ffc3a7362e 100644 --- a/actionview/lib/action_view/template.rb +++ b/actionview/lib/action_view/template.rb @@ -149,7 +149,7 @@ module ActionView end @updated_at = details[:updated_at] || Time.now - @formats = Array(format).map { |f| f.respond_to?(:ref) ? f.ref : f } + @formats = Array(format) @variants = [details[:variant]] @compile_mutex = Mutex.new end diff --git a/actionview/lib/action_view/template/resolver.rb b/actionview/lib/action_view/template/resolver.rb index 8232494746..220794e443 100644 --- a/actionview/lib/action_view/template/resolver.rb +++ b/actionview/lib/action_view/template/resolver.rb @@ -300,7 +300,7 @@ module ActionView handler = Template.handler_for_extension(extension) format, variant = pieces.last.split(EXTENSIONS[:variants], 2) if pieces.last format = if format - Template::Types[format] + Template::Types[format]&.ref else if handler.respond_to?(:default_format) # default_format can return nil handler.default_format -- cgit v1.2.3 From a213c4829ab0d87b2a7a74df0e35513c6ea6bdea Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 25 Feb 2019 12:03:45 -0800 Subject: remove the formats writer on templates --- actionview/lib/action_view/template.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/actionview/lib/action_view/template.rb b/actionview/lib/action_view/template.rb index ffc3a7362e..8b1c3199d2 100644 --- a/actionview/lib/action_view/template.rb +++ b/actionview/lib/action_view/template.rb @@ -122,11 +122,11 @@ module ActionView extend Template::Handlers - attr_accessor :locals, :formats, :variants, :virtual_path + attr_accessor :locals, :variants, :virtual_path attr_reader :source, :identifier, :handler, :original_encoding, :updated_at - attr_reader :variable + attr_reader :variable, :formats def initialize(source, identifier, handler, format: nil, **details) unless format @@ -154,6 +154,10 @@ module ActionView @compile_mutex = Mutex.new end + def formats= + end + deprecate :formats= + # Returns whether the underlying handler supports streaming. If so, # a streaming buffer *may* be passed when it starts rendering. def supports_streaming? -- cgit v1.2.3 From 12ff6064d169366e554b11a25813d82701435253 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Mon, 25 Feb 2019 12:33:29 -0800 Subject: Update actionview/lib/action_view/template.rb Co-Authored-By: tenderlove --- actionview/lib/action_view/template.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/actionview/lib/action_view/template.rb b/actionview/lib/action_view/template.rb index 8b1c3199d2..8ba03027d3 100644 --- a/actionview/lib/action_view/template.rb +++ b/actionview/lib/action_view/template.rb @@ -154,7 +154,7 @@ module ActionView @compile_mutex = Mutex.new end - def formats= + def formats=(_) end deprecate :formats= -- cgit v1.2.3