From 2b6d2d20374130da469ece24842ce3b681d3b788 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 11 Feb 2019 17:24:24 -0800 Subject: Turn lookup context in to a stack, push and pop if formats change This commit keeps a stack of lookup contexts on the ActionView::Base instance. If a format is passed to render, we instantiate a new lookup context and push it on the stack, that way any child calls to "render" will use the same format information as the parent. This also isolates "sibling" calls to render (multiple calls to render in the same template). Fixes #35222 #34138 --- .../lib/action_dispatch/middleware/debug_view.rb | 4 +- actionview/lib/action_view/base.rb | 54 ++++++++++++++++------ .../lib/action_view/helpers/rendering_helper.rb | 10 ++-- actionview/lib/action_view/lookup_context.rb | 7 +++ .../lib/action_view/renderer/partial_renderer.rb | 2 - actionview/lib/action_view/rendering.rb | 4 +- .../lib/action_view/template/handlers/erb/erubi.rb | 2 +- actionview/test/template/log_subscriber_test.rb | 3 +- actionview/test/template/render_test.rb | 7 ++- 9 files changed, 64 insertions(+), 29 deletions(-) diff --git a/actionpack/lib/action_dispatch/middleware/debug_view.rb b/actionpack/lib/action_dispatch/middleware/debug_view.rb index f16484d1ea..43c0a84504 100644 --- a/actionpack/lib/action_dispatch/middleware/debug_view.rb +++ b/actionpack/lib/action_dispatch/middleware/debug_view.rb @@ -11,8 +11,8 @@ module ActionDispatch def initialize(assigns) paths = [RESCUES_TEMPLATE_PATH] - renderer = ActionView::Renderer.new ActionView::LookupContext.new(paths) - super(renderer, assigns) + lookup_context = ActionView::LookupContext.new(paths) + super(lookup_context, assigns) end def compiled_method_container diff --git a/actionview/lib/action_view/base.rb b/actionview/lib/action_view/base.rb index 420136d6de..37ecd84792 100644 --- a/actionview/lib/action_view/base.rb +++ b/actionview/lib/action_view/base.rb @@ -196,10 +196,9 @@ module ActionView #:nodoc: end end - attr_reader :view_renderer + attr_reader :view_renderer, :lookup_context attr_internal :config, :assigns - delegate :lookup_context, to: :view_renderer delegate :formats, :formats=, :locale, :locale=, :view_paths, :view_paths=, to: :lookup_context def assign(new_assigns) # :nodoc: @@ -208,12 +207,17 @@ module ActionView #:nodoc: # :stopdoc: - def self.build_renderer(context, controller, formats) - lookup_context = context.is_a?(ActionView::LookupContext) ? - context : ActionView::LookupContext.new(context) - lookup_context.formats = formats if formats - lookup_context.prefixes = controller._prefixes if controller - ActionView::Renderer.new(lookup_context) + def self.build_lookup_context(context, controller, formats) + case context + when ActionView::Renderer + context.lookup_context + when Array + ActionView::LookupContext.new(context) + when nil + ActionView::LookupContext.new([]) + else + raise NotImplementedError, context.class.name + end end def self.empty @@ -225,14 +229,14 @@ module ActionView #:nodoc: end def self.with_context(context, assigns = {}, controller = nil) - new ActionView::Renderer.new(context), assigns, controller + new context, assigns, controller end NULL = Object.new # :startdoc: - def initialize(renderer = nil, assigns = {}, controller = nil, formats = NULL) #:nodoc: + def initialize(lookup_context = nil, assigns = {}, controller = nil, formats = NULL) #:nodoc: @_config = ActiveSupport::InheritableOptions.new if formats == NULL @@ -243,16 +247,19 @@ module ActionView #:nodoc: eowarn end - if renderer.is_a?(ActionView::Renderer) - @view_renderer = renderer + case lookup_context + when ActionView::LookupContext + @lookup_context = lookup_context else ActiveSupport::Deprecation.warn <<~eowarn - ActionView::Base instances should be constructed with a view renderer, + ActionView::Base instances should be constructed with a lookup context, assigments, and a controller. eowarn - @view_renderer = self.class.build_renderer(renderer, controller, formats) + @lookup_context = self.class.build_lookup_context(lookup_context, controller, formats) end + @view_renderer = ActionView::Renderer.new @lookup_context + @cache_hit = {} assign(assigns) assign_controller(controller) @@ -275,6 +282,25 @@ module ActionView #:nodoc: msg end + def in_context(options, locals) + old_view_renderer = @view_renderer + old_lookup_context = @lookup_context + + if !lookup_context.html_fallback_for_js && options[:formats] + formats = Array(options[:formats]) + if formats == [:js] + formats << :html + end + @lookup_context = lookup_context.with_prepended_formats(formats) + @view_renderer = ActionView::Renderer.new @lookup_context + end + + yield @view_renderer + ensure + @view_renderer = old_view_renderer + @lookup_context = old_lookup_context + end + ActiveSupport.run_load_hooks(:action_view, self) end end diff --git a/actionview/lib/action_view/helpers/rendering_helper.rb b/actionview/lib/action_view/helpers/rendering_helper.rb index 1e12aa2736..7323963c72 100644 --- a/actionview/lib/action_view/helpers/rendering_helper.rb +++ b/actionview/lib/action_view/helpers/rendering_helper.rb @@ -27,10 +27,12 @@ module ActionView def render(options = {}, locals = {}, &block) case options when Hash - if block_given? - view_renderer.render_partial(self, options.merge(partial: options[:layout]), &block) - else - view_renderer.render(self, options) + in_context(options, locals) do |renderer| + if block_given? + view_renderer.render_partial(self, options.merge(partial: options[:layout]), &block) + else + view_renderer.render(self, options) + end end else view_renderer.render_partial(self, partial: options, locals: locals, &block) diff --git a/actionview/lib/action_view/lookup_context.rb b/actionview/lib/action_view/lookup_context.rb index 61fe11cf45..125ab4dbe3 100644 --- a/actionview/lib/action_view/lookup_context.rb +++ b/actionview/lib/action_view/lookup_context.rb @@ -260,6 +260,13 @@ module ActionView @digest_cache ||= DetailsKey.digest_cache(@details) end + def with_prepended_formats(formats) + details = @details.dup + details[:formats] = formats + + self.class.new(@view_paths, details, @prefixes) + end + def initialize_details(target, details) registered_details.each do |k| target[k] = details[k] || Accessors::DEFAULT_PROCS[k].call diff --git a/actionview/lib/action_view/renderer/partial_renderer.rb b/actionview/lib/action_view/renderer/partial_renderer.rb index 801916954f..800a62eaa1 100644 --- a/actionview/lib/action_view/renderer/partial_renderer.rb +++ b/actionview/lib/action_view/renderer/partial_renderer.rb @@ -379,8 +379,6 @@ module ActionView @locals = options[:locals] || {} @details = extract_details(options) - prepend_formats(options[:formats]) - partial = options[:partial] if String === partial diff --git a/actionview/lib/action_view/rendering.rb b/actionview/lib/action_view/rendering.rb index da92ce1f5e..b798e80b04 100644 --- a/actionview/lib/action_view/rendering.rb +++ b/actionview/lib/action_view/rendering.rb @@ -82,7 +82,7 @@ module ActionView # # Override this method in a module to change the default behavior. def view_context - view_context_class.new(view_renderer, view_assigns, self) + view_context_class.new(lookup_context, view_assigns, self) end # Returns an object that is able to render templates. @@ -112,7 +112,7 @@ module ActionView lookup_context.rendered_format = nil if options[:formats] lookup_context.variants = variant if variant - view_renderer.render(context, options) + context.view_renderer.render(context, options) end # Assign the rendered format to look up context. diff --git a/actionview/lib/action_view/template/handlers/erb/erubi.rb b/actionview/lib/action_view/template/handlers/erb/erubi.rb index 15ca202024..20510c3062 100644 --- a/actionview/lib/action_view/template/handlers/erb/erubi.rb +++ b/actionview/lib/action_view/template/handlers/erb/erubi.rb @@ -26,7 +26,7 @@ module ActionView view = Class.new(ActionView::Base) { include action_view_erb_handler_context._routes.url_helpers class_eval("define_method(:_template) { |local_assigns, output_buffer| #{src} }", @filename || "(erubi)", 0) - }.with_context(action_view_erb_handler_context) + }.empty view.run(:_template, {}, ActionView::OutputBuffer.new) end diff --git a/actionview/test/template/log_subscriber_test.rb b/actionview/test/template/log_subscriber_test.rb index 83bb651ea3..85735139c1 100644 --- a/actionview/test/template/log_subscriber_test.rb +++ b/actionview/test/template/log_subscriber_test.rb @@ -16,8 +16,7 @@ class AVLogSubscriberTest < ActiveSupport::TestCase view_paths = ActionController::Base.view_paths lookup_context = ActionView::LookupContext.new(view_paths, {}, ["test"]) - renderer = ActionView::Renderer.new(lookup_context) - @view = ActionView::Base.with_empty_template_cache.new(renderer, {}) + @view = ActionView::Base.with_empty_template_cache.new(lookup_context, {}) ActionView::LogSubscriber.attach_to :action_view diff --git a/actionview/test/template/render_test.rb b/actionview/test/template/render_test.rb index e36babfd25..88047fd920 100644 --- a/actionview/test/template/render_test.rb +++ b/actionview/test/template/render_test.rb @@ -20,7 +20,10 @@ module RenderTestCases controller = TestController.new - @controller_view = controller.view_context_class.with_empty_template_cache.new(controller.view_renderer, controller.view_assigns, controller) + @controller_view = controller.view_context_class.with_empty_template_cache.new( + controller.lookup_context, + controller.view_assigns, + controller) # Reload and register danish language for testing I18n.backend.store_translations "da", {} @@ -83,7 +86,7 @@ module RenderTestCases def test_render_partial_use_last_prepended_format_for_partials_with_the_same_names @view.lookup_context.formats = [:html] - assert_equal "\nHTML Template, but JSON partial", @view.render(template: "test/change_priority") + assert_equal "\nHTML Template, but HTML partial", @view.render(template: "test/change_priority") end def test_render_template_with_a_missing_partial_of_another_format -- cgit v1.2.3