From 27adcd1c1a5cc566cfa8c5f8268b65ef01a7e865 Mon Sep 17 00:00:00 2001 From: Yehuda Katz Date: Tue, 11 Aug 2009 15:02:05 -0700 Subject: [PATCH] Clean up ActionView some: * Call _evaluate_assigns_and_ivars at the two entry points so we don't have to do a check at every render. * Make template.render viable without having to go through a wrapper method * Remove old TemplateHandler#render(template, local_assigns) path so we don't have to set self.template every time we render a template. * Move Template rescuing code to Template#render so it gets caught every time. * Pull in some tests from Pratik that test render @object in ActionView --- actionpack/lib/action_view/base.rb | 16 ----- actionpack/lib/action_view/render/partials.rb | 6 +- .../lib/action_view/render/rendering.rb | 61 ++++++------------- .../lib/action_view/template/handler.rb | 2 +- .../lib/action_view/template/template.rb | 12 +++- actionpack/test/template/render_test.rb | 34 +++++++---- 6 files changed, 56 insertions(+), 75 deletions(-) diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb index c171a5a8f572f..edfd1fd71c705 100644 --- a/actionpack/lib/action_view/base.rb +++ b/actionpack/lib/action_view/base.rb @@ -255,15 +255,6 @@ def view_paths=(paths) @view_paths = self.class.process_view_paths(paths) end - def with_template(current_template) - _evaluate_assigns_and_ivars - last_template, self.template = template, current_template - last_formats, self.formats = formats, current_template.formats - yield - ensure - self.template, self.formats = last_template, last_formats - end - def punctuate_body!(part) flush_output_buffer response.body_parts << part @@ -272,18 +263,11 @@ def punctuate_body!(part) # Evaluates the local assigns and controller ivars, pushes them to the view. def _evaluate_assigns_and_ivars #:nodoc: - @assigns_added ||= _copy_ivars_from_controller - end - - private - - def _copy_ivars_from_controller #:nodoc: if @controller variables = @controller.instance_variable_names variables -= @controller.protected_instance_variables if @controller.respond_to?(:protected_instance_variables) variables.each { |name| instance_variable_set(name, @controller.instance_variable_get(name)) } end - true end end diff --git a/actionpack/lib/action_view/render/partials.rb b/actionpack/lib/action_view/render/partials.rb index 83175ab4cfa31..e287021eb1181 100644 --- a/actionpack/lib/action_view/render/partials.rb +++ b/actionpack/lib/action_view/render/partials.rb @@ -265,7 +265,9 @@ def render_template(template, object = @object) @locals[:object] = @locals[template.variable_name] = object @locals[@options[:as]] = object if @options[:as] - content = @view._render_single_template(template, @locals, &@block) + content = template.render(@view, @locals) do |*names| + @view._layout_for(names, &@block) + end return content if @block || !@options[:layout] find_template(@options[:layout]).render(@view, @locals) { content } end @@ -302,7 +304,7 @@ def partial_path(object = @object) end def render_partial(options) - @assigns_added = false + _evaluate_assigns_and_ivars # TODO: Handle other details here. self.formats = options[:_details][:formats] if options[:_details] _render_partial(options) diff --git a/actionpack/lib/action_view/render/rendering.rb b/actionpack/lib/action_view/render/rendering.rb index c7afc56e3b56f..e505fe6c8cbe0 100644 --- a/actionpack/lib/action_view/render/rendering.rb +++ b/actionpack/lib/action_view/render/rendering.rb @@ -12,8 +12,6 @@ module Rendering # as the locals hash. def render(options = {}, locals = {}, &block) #:nodoc: case options - when String, NilClass - _render_partial(:partial => options, :locals => locals || {}) when Hash layout = options[:layout] @@ -35,26 +33,8 @@ def render(options = {}, locals = {}, &block) #:nodoc: end when :update update_page(&block) - end - end - - def _render_content(content, layout, locals) - return content unless layout - - locals ||= {} - - if controller && layout - @_layout = layout.identifier - logger.info("Rendering template within #{layout.identifier}") if logger - end - - begin - old_content, @_content_for[:layout] = @_content_for[:layout], content - - @cached_content_for_layout = @_content_for[:layout] - _render_single_template(layout, locals) - ensure - @_content_for[:layout] = old_content + else + _render_partial(:partial => options, :locals => locals) end end @@ -108,30 +88,17 @@ def _layout_for(names, &block) end end - def _render_single_template(template, locals = {}, &block) - with_template(template) do - template.render(self, locals) do |*names| - _layout_for(names, &block) - end - end - rescue Exception => e - if e.is_a?(TemplateError) - e.sub_template_of(template) - raise e - else - raise TemplateError.new(template, assigns, e) - end - end - def _render_inline(inline, layout, options) handler = Template.handler_class_for_extension(options[:type] || "erb") template = Template.new(options[:inline], "inline #{options[:inline].inspect}", handler, {}) - content = _render_single_template(template, options[:locals] || {}) - layout ? _render_content(content, layout, options[:locals]) : content + locals = options[:locals] || {} + content = template.render(self, locals) + content = layout.render(self, locals) { content } if layout + content end def _render_text(text, layout, options) - layout ? _render_content(text, layout, options[:locals]) : text + text = layout.render(self, options[:locals]) { text } if layout end # This is the API to render a ViewContext's template from a controller. @@ -141,7 +108,7 @@ def _render_text(text, layout, options) # _layout:: The layout, if any, to wrap the Template in # _partial:: true if the template is a partial def render_template(options) - @assigns_added = nil + _evaluate_assigns_and_ivars template, layout, partial = options.values_at(:_template, :_layout, :_partial) _render_template(template, layout, options, partial) end @@ -158,10 +125,18 @@ def _render_template(template, layout = nil, options = {}, partial = nil) content = if partial _render_partial_object(template, options) else - _render_single_template(template, locals) + template.render(self, locals) end - _render_content(content, layout, locals) + @cached_content_for_layout = content + @_content_for[:layout] = content + + if layout + @_layout = layout.identifier + logger.info("Rendering template within #{layout.identifier}") if logger + content = layout.render(self, locals) + end + content end end end \ No newline at end of file diff --git a/actionpack/lib/action_view/template/handler.rb b/actionpack/lib/action_view/template/handler.rb index 3071c78174198..7311e5e12fa2a 100644 --- a/actionpack/lib/action_view/template/handler.rb +++ b/actionpack/lib/action_view/template/handler.rb @@ -26,7 +26,7 @@ class TemplateHandler self.default_format = Mime::HTML def self.call(template) - "#{name}.new(self).render(template, local_assigns)" + raise "Need to implement #{self.class.name}#call(template)" end def initialize(view = nil) diff --git a/actionpack/lib/action_view/template/template.rb b/actionpack/lib/action_view/template/template.rb index 33d3f79ad3c87..f6270e5616755 100644 --- a/actionpack/lib/action_view/template/template.rb +++ b/actionpack/lib/action_view/template/template.rb @@ -26,9 +26,17 @@ def initialize(source, identifier, handler, details) @details[:formats] = Array.wrap(format.to_sym) end - def render(view, locals, &blk) + def render(view, locals, &block) method_name = compile(locals, view) - view.send(method_name, locals, &blk) + block ||= proc {|*names| view._layout_for(names) } + view.send(method_name, locals, &block) + rescue Exception => e + if e.is_a?(TemplateError) + e.sub_template_of(self) + raise e + else + raise TemplateError.new(self, view.assigns, e) + end end # TODO: Figure out how to abstract this diff --git a/actionpack/test/template/render_test.rb b/actionpack/test/template/render_test.rb index 7f30ae88a1967..50074b3b9d67d 100644 --- a/actionpack/test/template/render_test.rb +++ b/actionpack/test/template/render_test.rb @@ -2,10 +2,14 @@ require 'abstract_unit' require 'controller/fake_models' +class TestController < ActionController::Base +end + module RenderTestCases def setup_view(paths) @assigns = { :secret => 'in the sauce' } @view = ActionView::Base.new(paths, @assigns) + @controller_view = ActionView::Base.for_controller(TestController.new) # Reload and register danish language for testing I18n.reload! @@ -158,6 +162,25 @@ def test_render_partial_with_empty_array_should_return_nil assert_nil @view.render(:partial => []) end + def test_render_partial_using_string + assert_equal "Hello: Anonymous", @controller_view.render('customer') + end + + def test_render_partial_with_locals_using_string + assert_equal "Hola: david", @controller_view.render('customer_greeting', :greeting => 'Hola', :customer_greeting => Customer.new("david")) + end + + def test_render_partial_using_object + assert_equal "Hello: lifo", + @controller_view.render(Customer.new("lifo"), :greeting => "Hello") + end + + def test_render_partial_using_collection + customers = [ Customer.new("Amazon"), Customer.new("Yahoo") ] + assert_equal "Hello: AmazonHello: Yahoo", + @controller_view.render(customers, :greeting => "Hello") + end + # TODO: The reason for this test is unclear, improve documentation def test_render_partial_and_fallback_to_layout assert_equal "Before (Josh)\n\nAfter", @view.render(:partial => "test/layout_for_partial", :locals => { :name => "Josh" }) @@ -196,17 +219,6 @@ def test_render_inline_with_locals_and_compilable_custom_type assert_equal 'source: "Hello, <%= name %>!"', @view.render(:inline => "Hello, <%= name %>!", :locals => { :name => "Josh" }, :type => :foo) end - class LegacyHandler < ActionView::TemplateHandler - def render(template, local_assigns) - "source: #{template.source}; locals: #{local_assigns.inspect}" - end - end - - def test_render_legacy_handler_with_custom_type - ActionView::Template.register_template_handler :foo, LegacyHandler - assert_equal 'source: Hello, <%= name %>!; locals: {:name=>"Josh"}', @view.render(:inline => "Hello, <%= name %>!", :locals => { :name => "Josh" }, :type => :foo) - end - def test_render_ignores_templates_with_malformed_template_handlers %w(malformed malformed.erb malformed.html.erb malformed.en.html.erb).each do |name| assert_raise(ActionView::MissingTemplate) { @view.render(:file => "test/malformed/#{name}") }