From d4015a7f0609f12a5f7be4c794e5c5eaf2c748d5 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 25 Feb 2019 15:14:53 -0800 Subject: Pass locals in to the template object on construction This commit ensures that locals are passed in to the template objects when they are constructed, then removes the `locals=` mutator on the template object. This means we don't need to mutate Template objects with locals in the `decorate` method. --- actionview/lib/action_view/template.rb | 14 ++++++++++---- actionview/lib/action_view/template/resolver.rb | 19 +++++++++++-------- actionview/lib/action_view/testing/resolvers.rb | 7 ++++--- actionview/test/abstract_unit.rb | 2 +- .../test/actionpack/controller/view_paths_test.rb | 5 +++-- actionview/test/template/template_test.rb | 14 ++++++-------- 6 files changed, 35 insertions(+), 26 deletions(-) (limited to 'actionview') diff --git a/actionview/lib/action_view/template.rb b/actionview/lib/action_view/template.rb index f67c90de76..1070128d00 100644 --- a/actionview/lib/action_view/template.rb +++ b/actionview/lib/action_view/template.rb @@ -122,23 +122,28 @@ module ActionView extend Template::Handlers - attr_accessor :locals, :virtual_path + attr_accessor :virtual_path attr_reader :source, :identifier, :handler, :original_encoding, :updated_at - attr_reader :variable, :format, :variant + attr_reader :variable, :format, :variant, :locals - def initialize(source, identifier, handler, format: nil, variant: nil, **details) + def initialize(source, identifier, handler, format: nil, variant: nil, locals: nil, **details) unless format ActiveSupport::Deprecation.warn "ActionView::Template#initialize requires a format parameter" format = :html end + unless locals + ActiveSupport::Deprecation.warn "ActionView::Template#initialize requires a locals parameter" + locals = [] + end + @source = source @identifier = identifier @handler = handler @compiled = false @original_encoding = nil - @locals = details[:locals] || [] + @locals = locals @virtual_path = details[:virtual_path] @variable = if @virtual_path @@ -153,6 +158,7 @@ module ActionView @compile_mutex = Mutex.new end + deprecate def locals=(_); end deprecate def formats=(_); end deprecate def formats; Array(format); end deprecate def variants=(_); end diff --git a/actionview/lib/action_view/template/resolver.rb b/actionview/lib/action_view/template/resolver.rb index 6f0bf4187e..312299b755 100644 --- a/actionview/lib/action_view/template/resolver.rb +++ b/actionview/lib/action_view/template/resolver.rb @@ -143,14 +143,18 @@ module ActionView # Normalizes the arguments and passes it on to find_templates. def find_all(name, prefix = nil, partial = false, details = {}, key = nil, locals = []) + locals = locals.map(&:to_s).sort!.freeze + cached(key, [name, prefix, partial], details, locals) do - find_templates(name, prefix, partial, details) + find_templates(name, prefix, partial, details, locals) end end def find_all_anywhere(name, prefix, partial = false, details = {}, key = nil, locals = []) + locals = locals.map(&:to_s).sort!.freeze + cached(key, [name, prefix, partial], details, locals) do - find_templates(name, prefix, partial, details, true) + find_templates(name, prefix, partial, details, true, locals) end end @@ -165,7 +169,7 @@ module ActionView # This is what child classes implement. No defaults are needed # because Resolver guarantees that the arguments are present and # normalized. - def find_templates(name, prefix, partial, details, outside_app_allowed = false) + def find_templates(name, prefix, partial, details, outside_app_allowed = false, locals = []) raise NotImplementedError, "Subclasses must implement a find_templates(name, prefix, partial, details, outside_app_allowed = false) method" end @@ -180,7 +184,6 @@ module ActionView # resolver is fresher before returning it. def cached(key, path_info, details, locals) name, prefix, partial = path_info - locals = locals.map(&:to_s).sort! if key @cache.cache(key, name, prefix, partial, locals) do @@ -195,7 +198,6 @@ module ActionView def decorate(templates, path_info, details, locals) cached = nil templates.each do |t| - t.locals = locals t.virtual_path ||= (cached ||= build_path(*path_info)) end end @@ -213,12 +215,12 @@ module ActionView private - def find_templates(name, prefix, partial, details, outside_app_allowed = false) + def find_templates(name, prefix, partial, details, outside_app_allowed = false, locals) path = Path.build(name, prefix, partial) - query(path, details, details[:formats], outside_app_allowed) + query(path, details, details[:formats], outside_app_allowed, locals) end - def query(path, details, formats, outside_app_allowed) + def query(path, details, formats, outside_app_allowed, locals) template_paths = find_template_paths_from_details(path, details) template_paths = reject_files_external_to_app(template_paths) unless outside_app_allowed @@ -229,6 +231,7 @@ module ActionView virtual_path: path.virtual, format: format, variant: variant, + locals: locals, updated_at: mtime(template) ) end diff --git a/actionview/lib/action_view/testing/resolvers.rb b/actionview/lib/action_view/testing/resolvers.rb index 275e2e9182..2305fc9b81 100644 --- a/actionview/lib/action_view/testing/resolvers.rb +++ b/actionview/lib/action_view/testing/resolvers.rb @@ -23,7 +23,7 @@ module ActionView #:nodoc: private - def query(path, exts, _, _) + def query(path, exts, _, _, locals) query = +"" EXTENSIONS.each_key do |ext| query << "(" << exts[ext].map { |e| e && Regexp.escape(".#{e}") }.join("|") << "|)" @@ -39,6 +39,7 @@ module ActionView #:nodoc: virtual_path: path.virtual, format: format, variant: variant, + locals: locals, updated_at: updated_at ) end @@ -48,9 +49,9 @@ module ActionView #:nodoc: end class NullResolver < PathResolver - def query(path, exts, _, _) + def query(path, exts, _, _, locals) 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)] + [ActionView::Template.new("Template generated by Null Resolver", path.virtual, handler, virtual_path: path.virtual, format: format, variant: variant, locals: locals)] end end end diff --git a/actionview/test/abstract_unit.rb b/actionview/test/abstract_unit.rb index e0a1f59755..fe317bf39e 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) + format: :html, locals: []) view = ActionView::Base.with_empty_template_cache template.render(view.empty, {}).strip diff --git a/actionview/test/actionpack/controller/view_paths_test.rb b/actionview/test/actionpack/controller/view_paths_test.rb index a7a58a78cc..4449d08496 100644 --- a/actionview/test/actionpack/controller/view_paths_test.rb +++ b/actionview/test/actionpack/controller/view_paths_test.rb @@ -143,8 +143,9 @@ class ViewLoadPathsTest < ActionController::TestCase "Decorated body", template.identifier, template.handler, - virtual_path: template.virtual_path, - format: template.format + virtual_path: template.virtual_path, + format: template.format, + locals: template.locals ) end end diff --git a/actionview/test/template/template_test.rb b/actionview/test/template/template_test.rb index c74305fb42..5a24aea173 100644 --- a/actionview/test/template/template_test.rb +++ b/actionview/test/template/template_test.rb @@ -39,7 +39,8 @@ class TestERBTemplate < ActiveSupport::TestCase "partial", ERBHandler, virtual_path: "partial", - format: :html + format: :html, + locals: [] ) end @@ -57,7 +58,7 @@ class TestERBTemplate < ActiveSupport::TestCase end def new_template(body = "<%= hello %>", details = {}) - details = { format: :html }.merge details + details = { format: :html, locals: [] }.merge details ActionView::Template.new(body.dup, "hello template", details.fetch(:handler) { ERBHandler }, { virtual_path: "hello" }.merge!(details)) end @@ -103,8 +104,7 @@ class TestERBTemplate < ActiveSupport::TestCase end def test_locals - @template = new_template("<%= my_local %>") - @template.locals = [:my_local] + @template = new_template("<%= my_local %>", locals: [:my_local]) assert_equal "I am a local", render(my_local: "I am a local") end @@ -122,16 +122,14 @@ class TestERBTemplate < ActiveSupport::TestCase end def test_refresh_with_templates - @template = new_template("Hello", virtual_path: "test/foo/bar") - @template.locals = [:key] + @template = new_template("Hello", virtual_path: "test/foo/bar", locals: [:key]) assert_called_with(@context.lookup_context, :find_template, ["bar", %w(test/foo), false, [:key]], returns: "template") do assert_equal "template", @template.refresh(@context) end end def test_refresh_with_partials - @template = new_template("Hello", virtual_path: "test/_foo") - @template.locals = [:key] + @template = new_template("Hello", virtual_path: "test/_foo", locals: [:key]) assert_called_with(@context.lookup_context, :find_template, ["foo", %w(test), true, [:key]], returns: "partial") do assert_equal "partial", @template.refresh(@context) end -- cgit v1.2.3 From 9a343d148b96874a231b87906c9d6499b5e0b64a Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 25 Feb 2019 16:33:05 -0800 Subject: Don't mutate `virtual_path`, remove `decorate` `virtual_path` is calculated in the constructor when the Template object is allocated. We don't actually need to set it in the `decorate` method. That means we can remove the decorate method all together. --- actionview/lib/action_view/template.rb | 5 ++--- actionview/lib/action_view/template/resolver.rb | 16 ++-------------- 2 files changed, 4 insertions(+), 17 deletions(-) (limited to 'actionview') diff --git a/actionview/lib/action_view/template.rb b/actionview/lib/action_view/template.rb index 1070128d00..09476ceadc 100644 --- a/actionview/lib/action_view/template.rb +++ b/actionview/lib/action_view/template.rb @@ -122,10 +122,8 @@ module ActionView extend Template::Handlers - attr_accessor :virtual_path - attr_reader :source, :identifier, :handler, :original_encoding, :updated_at - attr_reader :variable, :format, :variant, :locals + attr_reader :variable, :format, :variant, :locals, :virtual_path def initialize(source, identifier, handler, format: nil, variant: nil, locals: nil, **details) unless format @@ -158,6 +156,7 @@ module ActionView @compile_mutex = Mutex.new end + deprecate def virtual_path=(_); end deprecate def locals=(_); end deprecate def formats=(_); end deprecate def formats; Array(format); end diff --git a/actionview/lib/action_view/template/resolver.rb b/actionview/lib/action_view/template/resolver.rb index 312299b755..9ca028840f 100644 --- a/actionview/lib/action_view/template/resolver.rb +++ b/actionview/lib/action_view/template/resolver.rb @@ -187,18 +187,10 @@ module ActionView if key @cache.cache(key, name, prefix, partial, locals) do - decorate(yield, path_info, details, locals) + yield end else - decorate(yield, path_info, details, locals) - end - end - - # Ensures all the resolver information is set in the template. - def decorate(templates, path_info, details, locals) - cached = nil - templates.each do |t| - t.virtual_path ||= (cached ||= build_path(*path_info)) + yield end end end @@ -432,9 +424,5 @@ module ActionView def self.instances [new(""), new("/")] end - - def decorate(*) - super.each { |t| t.virtual_path = nil } - end end end -- cgit v1.2.3 From 3a8d5dac9aa82d9d775c04d70230a2e0b404e162 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 25 Feb 2019 16:39:13 -0800 Subject: Expand key word args for ActionView::Template --- actionview/lib/action_view/template.rb | 6 +++--- actionview/test/template/template_test.rb | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) (limited to 'actionview') diff --git a/actionview/lib/action_view/template.rb b/actionview/lib/action_view/template.rb index 09476ceadc..06d3de17b3 100644 --- a/actionview/lib/action_view/template.rb +++ b/actionview/lib/action_view/template.rb @@ -125,7 +125,7 @@ module ActionView attr_reader :source, :identifier, :handler, :original_encoding, :updated_at attr_reader :variable, :format, :variant, :locals, :virtual_path - def initialize(source, identifier, handler, format: nil, variant: nil, locals: nil, **details) + def initialize(source, identifier, handler, format: nil, variant: nil, locals: nil, virtual_path: nil, updated_at: Time.now) unless format ActiveSupport::Deprecation.warn "ActionView::Template#initialize requires a format parameter" format = :html @@ -142,7 +142,7 @@ module ActionView @compiled = false @original_encoding = nil @locals = locals - @virtual_path = details[:virtual_path] + @virtual_path = virtual_path @variable = if @virtual_path base = @virtual_path[-1] == "/" ? "" : File.basename(@virtual_path) @@ -150,7 +150,7 @@ module ActionView $1.to_sym end - @updated_at = details[:updated_at] || Time.now + @updated_at = updated_at @format = format @variant = variant @compile_mutex = Mutex.new diff --git a/actionview/test/template/template_test.rb b/actionview/test/template/template_test.rb index 5a24aea173..71fb99115b 100644 --- a/actionview/test/template/template_test.rb +++ b/actionview/test/template/template_test.rb @@ -59,7 +59,7 @@ class TestERBTemplate < ActiveSupport::TestCase def new_template(body = "<%= hello %>", details = {}) details = { format: :html, locals: [] }.merge details - ActionView::Template.new(body.dup, "hello template", details.fetch(:handler) { ERBHandler }, { virtual_path: "hello" }.merge!(details)) + ActionView::Template.new(body.dup, "hello template", details.delete(:handler) || ERBHandler, { virtual_path: "hello" }.merge!(details)) end def render(locals = {}) -- cgit v1.2.3 From 0e13941dbe9c4840175e5c6f5b2c3f572249a4ec Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 25 Feb 2019 16:42:45 -0800 Subject: `original_encoding` isn't used, so deprecate it and remove the ivar --- actionview/lib/action_view/file_template.rb | 4 ++-- actionview/lib/action_view/template.rb | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) (limited to 'actionview') diff --git a/actionview/lib/action_view/file_template.rb b/actionview/lib/action_view/file_template.rb index cdc077a01a..dea02176eb 100644 --- a/actionview/lib/action_view/file_template.rb +++ b/actionview/lib/action_view/file_template.rb @@ -22,11 +22,11 @@ module ActionView # to ensure that references to the template object can be marshalled as well. This means forgoing # the marshalling of the compiler mutex and instantiating that again on unmarshalling. def marshal_dump # :nodoc: - [ @identifier, @handler, @compiled, @original_encoding, @locals, @virtual_path, @updated_at, @format, @variant ] + [ @identifier, @handler, @compiled, @locals, @virtual_path, @updated_at, @format, @variant ] end def marshal_load(array) # :nodoc: - @identifier, @handler, @compiled, @original_encoding, @locals, @virtual_path, @updated_at, @format, @variant = *array + @identifier, @handler, @compiled, @locals, @virtual_path, @updated_at, @format, @variant = *array @compile_mutex = Mutex.new end end diff --git a/actionview/lib/action_view/template.rb b/actionview/lib/action_view/template.rb index 06d3de17b3..7f29dedd7c 100644 --- a/actionview/lib/action_view/template.rb +++ b/actionview/lib/action_view/template.rb @@ -140,7 +140,6 @@ module ActionView @identifier = identifier @handler = handler @compiled = false - @original_encoding = nil @locals = locals @virtual_path = virtual_path @@ -156,6 +155,7 @@ module ActionView @compile_mutex = Mutex.new end + deprecate :original_encoding deprecate def virtual_path=(_); end deprecate def locals=(_); end deprecate def formats=(_); end @@ -266,11 +266,11 @@ module ActionView # to ensure that references to the template object can be marshalled as well. This means forgoing # the marshalling of the compiler mutex and instantiating that again on unmarshalling. def marshal_dump # :nodoc: - [ @source, @identifier, @handler, @compiled, @original_encoding, @locals, @virtual_path, @updated_at, @format, @variant ] + [ @source, @identifier, @handler, @compiled, @locals, @virtual_path, @updated_at, @format, @variant ] end def marshal_load(array) # :nodoc: - @source, @identifier, @handler, @compiled, @original_encoding, @locals, @virtual_path, @updated_at, @format, @variant = *array + @source, @identifier, @handler, @compiled, @locals, @virtual_path, @updated_at, @format, @variant = *array @compile_mutex = Mutex.new end -- cgit v1.2.3 From ebb4e015c9a8b1307d5f3564978f6e24ac2b7c4a Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Tue, 26 Feb 2019 10:05:02 -0800 Subject: Update actionview/lib/action_view/template/resolver.rb Co-Authored-By: tenderlove --- actionview/lib/action_view/template/resolver.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionview') diff --git a/actionview/lib/action_view/template/resolver.rb b/actionview/lib/action_view/template/resolver.rb index 9ca028840f..b346acf7e0 100644 --- a/actionview/lib/action_view/template/resolver.rb +++ b/actionview/lib/action_view/template/resolver.rb @@ -146,7 +146,7 @@ module ActionView locals = locals.map(&:to_s).sort!.freeze cached(key, [name, prefix, partial], details, locals) do - find_templates(name, prefix, partial, details, locals) + find_templates(name, prefix, partial, details, false, locals) end end -- cgit v1.2.3 From 082130d1d43620ca29c2f2f169a5059fa0065a8c Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Tue, 26 Feb 2019 10:07:46 -0800 Subject: Remove unused method / fix documentation --- actionview/lib/action_view/template/resolver.rb | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) (limited to 'actionview') diff --git a/actionview/lib/action_view/template/resolver.rb b/actionview/lib/action_view/template/resolver.rb index b346acf7e0..1c463efbb2 100644 --- a/actionview/lib/action_view/template/resolver.rb +++ b/actionview/lib/action_view/template/resolver.rb @@ -170,12 +170,7 @@ module ActionView # because Resolver guarantees that the arguments are present and # normalized. def find_templates(name, prefix, partial, details, outside_app_allowed = false, locals = []) - raise NotImplementedError, "Subclasses must implement a find_templates(name, prefix, partial, details, outside_app_allowed = false) method" - end - - # Helpers that builds a path. Useful for building virtual paths. - def build_path(name, prefix, partial) - Path.build(name, prefix, partial) + raise NotImplementedError, "Subclasses must implement a find_templates(name, prefix, partial, details, outside_app_allowed = false, locals = []) method" end # Handles templates caching. If a key is given and caching is on -- cgit v1.2.3