Tighten up the AV::Base constructor

The AV::Base constructor was too complicated, and this commit tightens
up the parameters it will take.  At runtime, AV::Base is most commonly
constructed here:

  94d54fa4ab/actionview/lib/action_view/rendering.rb (L72-L74)

This provides an AV::Renderer instance, a hash of assignments, and a
controller instance.  Since this is the common case for construction, we
should remove logic from the constructor that handles other cases.  This
commit introduces special constructors for those other cases.
Interestingly, most code paths that construct AV::Base "strangely" are
tests.
This commit is contained in:
Aaron Patterson 2019-01-29 15:17:52 -08:00
parent 94d54fa4ab
commit e17fe52e0e
No known key found for this signature in database
GPG Key ID: 953170BCB4FFAFC6
14 changed files with 61 additions and 26 deletions

View File

@ -75,7 +75,7 @@ module ActionController
# Provides a proxy to access helper methods from outside the view.
def helpers
@helper_proxy ||= begin
proxy = ActionView::Base.new
proxy = ActionView::Base.empty
proxy.config = config.inheritable_copy
proxy.extend(_helpers)
end

View File

@ -10,7 +10,9 @@ module ActionDispatch
RESCUES_TEMPLATE_PATH = File.expand_path("templates", __dir__)
def initialize(assigns)
super([RESCUES_TEMPLATE_PATH], assigns)
paths = [RESCUES_TEMPLATE_PATH]
renderer = ActionView::Renderer.new ActionView::LookupContext.new(paths)
super(renderer, assigns)
end
def debug_params(params)

View File

@ -3,6 +3,7 @@
require "active_support/core_ext/module/attr_internal"
require "active_support/core_ext/module/attribute_accessors"
require "active_support/ordered_options"
require "active_support/deprecation"
require "action_view/log_subscriber"
require "action_view/helpers"
require "action_view/context"
@ -187,7 +188,7 @@ module ActionView #:nodoc:
end
end
attr_accessor :view_renderer
attr_reader :view_renderer
attr_internal :config, :assigns
delegate :lookup_context, to: :view_renderer
@ -197,17 +198,49 @@ module ActionView #:nodoc:
@_assigns = new_assigns.each { |key, value| instance_variable_set("@#{key}", value) }
end
def initialize(context = nil, assigns = {}, controller = nil, formats = nil) #: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)
end
def self.empty
with_view_paths([])
end
def self.with_view_paths(view_paths, assigns = {}, controller = nil)
with_context ActionView::LookupContext.new(view_paths), assigns, controller
end
def self.with_context(context, assigns = {}, controller = nil)
new ActionView::Renderer.new(context), assigns, controller
end
NULL = Object.new
# :startdoc:
def initialize(renderer, assigns = {}, controller = nil, formats = NULL) #:nodoc:
@_config = ActiveSupport::InheritableOptions.new
if context.is_a?(ActionView::Renderer)
@view_renderer = context
unless formats == NULL
ActiveSupport::Deprecation.warn <<~eowarn
Passing formats to ActionView::Base.new is deprecated
eowarn
end
if renderer.is_a?(ActionView::Renderer)
@view_renderer = renderer
else
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
@view_renderer = ActionView::Renderer.new(lookup_context)
ActiveSupport::Deprecation.warn <<~eowarn
ActionView::Base instances should be constructed with a view renderer,
assigments, and a controller.
eowarn
@view_renderer = self.class.build_renderer(renderer, controller, formats)
end
@cache_hit = {}

View File

@ -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)
}.new(action_view_erb_handler_context)
}.with_context(action_view_erb_handler_context)
view.run(:_template, {}, ActionView::OutputBuffer.new)
end

View File

@ -48,7 +48,7 @@ module RenderERBUtils
@view ||= begin
path = ActionView::FileSystemResolver.new(FIXTURE_LOAD_PATH)
view_paths = ActionView::PathSet.new([path])
ActionView::Base.new(view_paths)
ActionView::Base.with_view_paths(view_paths)
end
end
@ -61,7 +61,7 @@ module RenderERBUtils
ActionView::Template::Handlers::ERB,
{})
template.render(ActionView::Base.new, {}).strip
template.render(ActionView::Base.empty, {}).strip
end
end

View File

@ -77,7 +77,7 @@ class ViewLoadPathsTest < ActionController::TestCase
end
def test_template_appends_view_path_correctly
@controller.instance_variable_set :@template, ActionView::Base.new(TestController.view_paths, {}, @controller)
@controller.instance_variable_set :@template, ActionView::Base.with_view_paths(TestController.view_paths, {}, @controller)
class_view_paths = TestController.view_paths
@controller.append_view_path "foo"
@ -89,7 +89,7 @@ class ViewLoadPathsTest < ActionController::TestCase
end
def test_template_prepends_view_path_correctly
@controller.instance_variable_set :@template, ActionView::Base.new(TestController.view_paths, {}, @controller)
@controller.instance_variable_set :@template, ActionView::Base.with_view_paths(TestController.view_paths, {}, @controller)
class_view_paths = TestController.view_paths
@controller.prepend_view_path "baz"

View File

@ -19,7 +19,7 @@ class MultifetchCacheTest < ActiveRecordTestCase
def combined_fragment_cache_key(key)
[ :views, key ]
end
end.new(view_paths, {})
end.with_view_paths(view_paths, {})
end
def test_only_preloading_for_records_that_miss_the_cache

View File

@ -5,7 +5,7 @@ require "abstract_unit"
class CaptureHelperTest < ActionView::TestCase
def setup
super
@av = ActionView::Base.new
@av = ActionView::Base.empty
@view_flow = ActionView::OutputFlow.new
end

View File

@ -72,13 +72,13 @@ class CompiledTemplatesTest < ActiveSupport::TestCase
def render_with_cache(*args)
view_paths = ActionController::Base.view_paths
ActionView::Base.new(view_paths, {}).render(*args)
ActionView::Base.with_view_paths(view_paths, {}).render(*args)
end
def render_without_cache(*args)
path = ActionView::FileSystemResolver.new(FIXTURE_LOAD_PATH)
view_paths = ActionView::PathSet.new([path])
ActionView::Base.new(view_paths, {}).render(*args)
ActionView::Base.with_view_paths(view_paths, {}).render(*args)
end
def modify_template(template, content)

View File

@ -15,7 +15,7 @@ module RenderTestCases
def combined_fragment_cache_key(key)
[ :views, key ]
end
end.new(paths, @assigns)
end.with_view_paths(paths, @assigns)
@controller_view = TestController.new.view_context

View File

@ -9,7 +9,7 @@ class SetupFiberedBase < ActiveSupport::TestCase
def setup
view_paths = ActionController::Base.view_paths
@assigns = { secret: "in the sauce", name: nil }
@view = ActionView::Base.new(view_paths, @assigns)
@view = ActionView::Base.with_view_paths(view_paths, @assigns)
@controller_view = TestController.new.view_context
end

View File

@ -52,7 +52,7 @@ module ActionView
end
test "retrieve non existing config values" do
assert_nil ActionView::Base.new.config.something_odd
assert_nil ActionView::Base.empty.config.something_odd
end
test "works without testing a helper module" do

View File

@ -36,7 +36,7 @@ class TranslationHelperTest < ActiveSupport::TestCase
}
}
)
@view = ::ActionView::Base.new(ActionController::Base.view_paths, {})
@view = ::ActionView::Base.with_view_paths(ActionController::Base.view_paths, {})
end
teardown do

View File

@ -150,8 +150,8 @@ module RailsGuides
puts "Generating #{guide} as #{output_file}"
layout = @kindle ? "kindle/layout" : "layout"
view = ActionView::Base.new(
@source_dir,
view = ActionView::Base.with_view_paths(
[@source_dir],
edge: @edge,
version: @version,
mobi: "kindle/#{mobi}",