diff options
Diffstat (limited to 'actionpack')
| -rw-r--r-- | actionpack/CHANGELOG | 2 | ||||
| -rw-r--r-- | actionpack/lib/action_view.rb | 1 | ||||
| -rw-r--r-- | actionpack/lib/action_view/base.rb | 48 | ||||
| -rw-r--r-- | actionpack/lib/action_view/partial_template.rb | 61 | ||||
| -rw-r--r-- | actionpack/lib/action_view/partials.rb | 94 | ||||
| -rw-r--r-- | actionpack/lib/action_view/template.rb | 22 | ||||
| -rw-r--r-- | actionpack/lib/action_view/template_handler.rb | 2 | ||||
| -rw-r--r-- | actionpack/lib/action_view/template_handlers/compilable.rb | 4 | ||||
| -rw-r--r-- | actionpack/test/controller/custom_handler_test.rb | 6 | ||||
| -rw-r--r-- | actionpack/test/controller/layout_test.rb | 4 | ||||
| -rw-r--r-- | actionpack/test/controller/new_render_test.rb | 20 | ||||
| -rw-r--r-- | actionpack/test/fixtures/test/_customer_counter.erb | 1 | ||||
| -rw-r--r-- | actionpack/test/template/compiled_templates_test.rb | 24 | ||||
| -rw-r--r-- | actionpack/test/template/template_object_test.rb | 62 |
14 files changed, 211 insertions, 140 deletions
diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index 4413ed3477..782e364f7e 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -1,5 +1,7 @@ *SVN* +* Refactor partial rendering into a PartialTemplate class. [Pratik] + * Added that requests with JavaScript as the priority mime type in the accept header and no format extension in the parameters will be treated as though their format was :js when it comes to determining which template to render. This makes it possible for JS requests to automatically render action.js.rjs files without an explicit respond_to block [DHH] * Tests for distance_of_time_in_words with TimeWithZone instances. Closes #10914 [ernesto.jimenez] diff --git a/actionpack/lib/action_view.rb b/actionpack/lib/action_view.rb index c3b846eece..e20812d308 100644 --- a/actionpack/lib/action_view.rb +++ b/actionpack/lib/action_view.rb @@ -29,6 +29,7 @@ require 'action_view/template_handlers/rjs' require 'action_view/template_finder' require 'action_view/template' +require 'action_view/partial_template' require 'action_view/base' require 'action_view/partials' diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb index f5346bd480..143428c6d9 100644 --- a/actionpack/lib/action_view/base.rb +++ b/actionpack/lib/action_view/base.rb @@ -292,24 +292,22 @@ If you are rendering a subtemplate, you must now use controller-like partial syn # Renders the template present at <tt>template_path</tt> (relative to the view_paths array). # The hash in <tt>local_assigns</tt> is made available as local variables. - def render(options = {}, old_local_assigns = {}, &block) #:nodoc: + def render(options = {}, local_assigns = {}, &block) #:nodoc: if options.is_a?(String) - render_file(options, true, old_local_assigns) + render_file(options, true, local_assigns) elsif options == :update update_page(&block) elsif options.is_a?(Hash) options = options.reverse_merge(:locals => {}, :use_full_path => true) - if options[:layout] - path, partial_name = partial_pieces(options.delete(:layout)) - + if partial_layout = options.delete(:layout) if block_given? wrap_content_for_layout capture(&block) do - concat(render(options.merge(:partial => "#{path}/#{partial_name}")), block.binding) + concat(render(options.merge(:partial => partial_layout)), block.binding) end else wrap_content_for_layout render(options) do - render(options.merge(:partial => "#{path}/#{partial_name}")) + render(options.merge(:partial => partial_layout)) end end elsif options[:file] @@ -325,17 +323,8 @@ If you are rendering a subtemplate, you must now use controller-like partial syn end end - # Renders the +template+ which is given as a string as either erb or builder depending on <tt>template_extension</tt>. - # The hash in <tt>local_assigns</tt> is made available as local variables. def render_template(template) #:nodoc: - handler = template.handler - @current_render_extension = template.extension - - if handler.compilable? - compile_and_render_template(handler, template) - else - handler.render(template.source, template.locals) - end + template.render end # Returns true is the file may be rendered implicitly. @@ -369,7 +358,7 @@ If you are rendering a subtemplate, you must now use controller-like partial syn end end - private + private def wrap_content_for_layout(content) original_content_for_layout = @content_for_layout @content_for_layout = content @@ -388,24 +377,11 @@ If you are rendering a subtemplate, you must now use controller-like partial syn def assign_variables_from_controller @assigns.each { |key, value| instance_variable_set("@#{key}", value) } end - - # Render the provided template with the given local assigns. If the template has not been rendered with the provided - # local assigns yet, or if the template has been updated on disk, then the template will be compiled to a method. - # - # Either, but not both, of template and file_path may be nil. If file_path is given, the template - # will only be read if it has to be compiled. - # - def compile_and_render_template(handler, template) #:nodoc: - # compile the given template, if necessary - handler.compile_template(template) - - # Get the method name for this template and run it - method_name = @@method_names[template.method_key] - evaluate_assigns - - send(method_name, template.locals) do |*name| - instance_variable_get "@content_for_#{name.first || 'layout'}" - end + + def execute(template) + send(template.method, template.locals) do |*names| + instance_variable_get "@content_for_#{names.first || 'layout'}" + end end end end diff --git a/actionpack/lib/action_view/partial_template.rb b/actionpack/lib/action_view/partial_template.rb new file mode 100644 index 0000000000..a470c915cd --- /dev/null +++ b/actionpack/lib/action_view/partial_template.rb @@ -0,0 +1,61 @@ +module ActionView #:nodoc: + class PartialTemplate < Template #:nodoc: + + attr_reader :variable_name, :object + + def initialize(view, partial_path, object = nil, locals = {}) + @path, @variable_name = extract_partial_name_and_path(view, partial_path) + super(view, @path, true, locals) + add_object_to_local_assigns!(object) + + # This is needed here in order to compile template with knowledge of 'counter' + initialize_counter + + # Prepare early. This is a performance optimization for partial collections + prepare! + end + + def render + ActionController::Base.benchmark("Rendered #{@path}", Logger::DEBUG, false) do + @handler.render(self) + end + end + + def render_member(object) + @locals[@counter_name] += 1 + @locals[:object] = @locals[@variable_name] = object + render + end + + private + + def add_object_to_local_assigns!(object) + @locals[:object] ||= + @locals[@variable_name] ||= + if object.is_a?(ActionView::Base::ObjectWrapper) + object.value + else + object + end || @view.controller.instance_variable_get("@#{variable_name}") + end + + def extract_partial_name_and_path(view, partial_path) + path, partial_name = partial_pieces(view, partial_path) + [File.join(path, "_#{partial_name}"), partial_name.split('/').last.split('.').first.to_sym] + end + + def partial_pieces(view, partial_path) + if partial_path.include?('/') + return File.dirname(partial_path), File.basename(partial_path) + else + return view.controller.class.controller_path, partial_path + end + end + + def initialize_counter + @counter_name ||= "#{@variable_name}_counter".to_sym + @locals[@counter_name] = 0 + end + + end +end diff --git a/actionpack/lib/action_view/partials.rb b/actionpack/lib/action_view/partials.rb index e2aa98a319..7f1963fb29 100644 --- a/actionpack/lib/action_view/partials.rb +++ b/actionpack/lib/action_view/partials.rb @@ -103,23 +103,11 @@ module ActionView # As you can see, the :locals hash is shared between both the partial and its layout. module Partials private - def render_partial(partial_path, object_assigns = nil, local_assigns = nil) #:nodoc: + def render_partial(partial_path, object_assigns = nil, local_assigns = {}) #:nodoc: case partial_path when String, Symbol, NilClass - path, partial_name = partial_pieces(partial_path) - full_partial_path = File.join(path, "_#{partial_name}") - object = extracting_object(partial_name, object_assigns) - local_assigns = local_assigns ? local_assigns.clone : {} - add_counter_to_local_assigns!(partial_name, local_assigns) - add_object_to_local_assigns!(partial_name, local_assigns, object) - - if logger && logger.debug? - ActionController::Base.benchmark("Rendered #{full_partial_path}", Logger::DEBUG, false) do - render(full_partial_path, local_assigns) - end - else - render(full_partial_path, local_assigns) - end + # Render the template + ActionView::PartialTemplate.new(self, partial_path, object_assigns, local_assigns).render when ActionView::Helpers::FormBuilder builder_partial_path = partial_path.class.to_s.demodulize.underscore.sub(/_builder$/, '') render_partial(builder_partial_path, object_assigns, (local_assigns || {}).merge(builder_partial_path.to_sym => partial_path)) @@ -132,75 +120,21 @@ module ActionView "" end else - render_partial( - ActionController::RecordIdentifier.partial_path(partial_path), - partial_path, local_assigns) + render_partial(ActionController::RecordIdentifier.partial_path(partial_path), partial_path, local_assigns) end end - def render_partial_collection(partial_name, collection, partial_spacer_template = nil, local_assigns = nil) #:nodoc: - collection_of_partials = Array.new - counter_name = partial_counter_name(partial_name) + def render_partial_collection(partial_path, collection, partial_spacer_template = nil, local_assigns = {}) #:nodoc: + return " " if collection.empty? + local_assigns = local_assigns ? local_assigns.clone : {} - collection.each_with_index do |element, counter| - local_assigns[counter_name] = counter - collection_of_partials.push(render_partial(partial_name, element, local_assigns)) - end - - return " " if collection_of_partials.empty? - - if partial_spacer_template - spacer_path, spacer_name = partial_pieces(partial_spacer_template) - collection_of_partials.join(render("#{spacer_path}/_#{spacer_name}")) - else - collection_of_partials.join - end - end - - alias_method :render_collection_of_partials, :render_partial_collection - - def partial_pieces(partial_path) - if partial_path.include?('/') - return File.dirname(partial_path), File.basename(partial_path) - else - return controller.class.controller_path, partial_path - end - end - - def partial_counter_name(partial_name) - "#{partial_variable_name(partial_name)}_counter".intern - end - - def partial_variable_name(partial_name) - @@partial_variable_names ||= {} - @@partial_variable_names[partial_name] ||= - partial_name.split('/').last.split('.').first.intern - end - - def extracting_object(partial_name, object_assigns) - variable_name = partial_variable_name(partial_name) - if object_assigns.nil? - controller.instance_variable_get("@#{variable_name}") - else - object_assigns - end - end - - def add_counter_to_local_assigns!(partial_name, local_assigns) - counter_name = partial_counter_name(partial_name) - local_assigns[counter_name] = 1 unless local_assigns.has_key?(counter_name) - end - - def add_object_to_local_assigns!(partial_name, local_assigns, object) - variable_name = partial_variable_name(partial_name) - - local_assigns[:object] ||= - local_assigns[variable_name] ||= - if object.is_a?(ActionView::Base::ObjectWrapper) - object.value - else - object - end || controller.instance_variable_get("@#{variable_name}") + template = ActionView::PartialTemplate.new(self, partial_path, nil, local_assigns) + + spacer = partial_spacer_template ? render(:partial => partial_spacer_template) : '' + + collection.map do |element| + template.render_member(element) + end.join(spacer) end end end diff --git a/actionpack/lib/action_view/template.rb b/actionpack/lib/action_view/template.rb index a814d7b4e5..9f979e2e5d 100644 --- a/actionpack/lib/action_view/template.rb +++ b/actionpack/lib/action_view/template.rb @@ -2,7 +2,7 @@ module ActionView #:nodoc: class Template #:nodoc: attr_accessor :locals - attr_reader :handler, :path, :source, :extension, :filename, :path_without_extension + attr_reader :handler, :path, :source, :extension, :filename, :path_without_extension, :method def initialize(view, path_or_source, use_full_path, locals = {}, inline = false, inline_type = nil) @view = view @@ -19,6 +19,12 @@ module ActionView #:nodoc: @extension = inline_type end @locals = locals || {} + @handler = @view.class.handler_class_for_extension(@extension).new(@view) + end + + def render + prepare! + @handler.render(self) end def source @@ -29,13 +35,19 @@ module ActionView #:nodoc: @method_key ||= (@filename || @source) end - def handler - @handler ||= @view.class.handler_class_for_extension(@extension).new(@view) - end - def base_path_for_exception @finder.find_base_path_for("#{@path_without_extension}.#{@extension}") || @finder.view_paths.first end + + def prepare! + @view.send :evaluate_assigns + @view.current_render_extension = @extension + + if @handler.compilable? + @handler.compile_template(self) # compile the given template, if necessary + @method = @view.method_names[method_key] # Set the method name for this template and run it + end + end private diff --git a/actionpack/lib/action_view/template_handler.rb b/actionpack/lib/action_view/template_handler.rb index 687c0db4c6..ec407e3fb3 100644 --- a/actionpack/lib/action_view/template_handler.rb +++ b/actionpack/lib/action_view/template_handler.rb @@ -13,7 +13,7 @@ module ActionView @view = view end - def render(template, local_assigns) + def render(template) end def compile(template) diff --git a/actionpack/lib/action_view/template_handlers/compilable.rb b/actionpack/lib/action_view/template_handlers/compilable.rb index 110c90af0a..35c74f6c51 100644 --- a/actionpack/lib/action_view/template_handlers/compilable.rb +++ b/actionpack/lib/action_view/template_handlers/compilable.rb @@ -24,6 +24,10 @@ module ActionView true end end + + def render(template) + @view.send :execute, template + end # Compile and evaluate the template's code def compile_template(template) diff --git a/actionpack/test/controller/custom_handler_test.rb b/actionpack/test/controller/custom_handler_test.rb index 932b8c15c3..cdde00cccc 100644 --- a/actionpack/test/controller/custom_handler_test.rb +++ b/actionpack/test/controller/custom_handler_test.rb @@ -5,9 +5,9 @@ class CustomHandler < ActionView::TemplateHandler @view = view end - def render( template, local_assigns ) - [ template, - local_assigns, + def render( template ) + [ template.source, + template.locals, @view ] end end diff --git a/actionpack/test/controller/layout_test.rb b/actionpack/test/controller/layout_test.rb index 11ad4a20af..56d7f69774 100644 --- a/actionpack/test/controller/layout_test.rb +++ b/actionpack/test/controller/layout_test.rb @@ -35,8 +35,8 @@ class MabView < ActionView::TemplateHandler def initialize(view) end - def render(text, locals = {}) - text + def render(template) + template.source end end diff --git a/actionpack/test/controller/new_render_test.rb b/actionpack/test/controller/new_render_test.rb index 928c46cb6d..0f14de54b1 100644 --- a/actionpack/test/controller/new_render_test.rb +++ b/actionpack/test/controller/new_render_test.rb @@ -146,6 +146,14 @@ class NewRenderTestController < ActionController::Base def partial_collection render :partial => "customer", :collection => [ Customer.new("david"), Customer.new("mary") ] end + + def partial_collection_with_spacer + render :partial => "customer", :spacer_template => "partial_only", :collection => [ Customer.new("david"), Customer.new("mary") ] + end + + def partial_collection_with_counter + render :partial => "customer_counter", :collection => [ Customer.new("david"), Customer.new("mary") ] + end def partial_collection_with_locals render :partial => "customer_greeting", :collection => [ Customer.new("david"), Customer.new("mary") ], :locals => { :greeting => "Bonjour" } @@ -712,11 +720,21 @@ EOS get :partial_collection assert_equal "Hello: davidHello: mary", @response.body end - + + def test_partial_collection_with_counter + get :partial_collection_with_counter + assert_equal "david1mary2", @response.body + end + def test_partial_collection_with_locals get :partial_collection_with_locals assert_equal "Bonjour: davidBonjour: mary", @response.body end + + def test_partial_collection_with_spacer + get :partial_collection_with_spacer + assert_equal "Hello: davidonly partialHello: mary", @response.body + end def test_partial_collection_shorthand_with_locals get :partial_collection_shorthand_with_locals diff --git a/actionpack/test/fixtures/test/_customer_counter.erb b/actionpack/test/fixtures/test/_customer_counter.erb new file mode 100644 index 0000000000..3435979dba --- /dev/null +++ b/actionpack/test/fixtures/test/_customer_counter.erb @@ -0,0 +1 @@ +<%= customer_counter.name %><%= customer_counter_counter %>
\ No newline at end of file diff --git a/actionpack/test/template/compiled_templates_test.rb b/actionpack/test/template/compiled_templates_test.rb index ad002e4cce..b969575235 100644 --- a/actionpack/test/template/compiled_templates_test.rb +++ b/actionpack/test/template/compiled_templates_test.rb @@ -109,9 +109,9 @@ class CompiledTemplateTests < Test::Unit::TestCase # All templates are rendered at t+2 Time.expects(:now).times(windows ? 2 : 3).returns(t + 2.seconds) - v.send(:compile_and_render_template, @handler, ta) - v.send(:compile_and_render_template, @handler, tb) - v.send(:compile_and_render_template, @handler, ts) unless windows + v.send(:render_template, ta) + v.send(:render_template, tb) + v.send(:render_template, ts) unless windows a_n = v.method_names[@a] b_n = v.method_names[@b] s_n = v.method_names[@s] unless windows @@ -129,9 +129,9 @@ class CompiledTemplateTests < Test::Unit::TestCase assert !@handler.send(:compile_template?, ta) assert !@handler.send(:compile_template?, tb) assert !@handler.send(:compile_template?, ts) unless windows - v.send(:compile_and_render_template, @handler, ta) - v.send(:compile_and_render_template, @handler, tb) - v.send(:compile_and_render_template, @handler, ts) unless windows + v.send(:render_template, ta) + v.send(:render_template, tb) + v.send(:render_template, ts) unless windows # none of the files have changed since last compile assert @handler.compile_time[a_n] < t + 3.seconds assert @handler.compile_time[b_n] < t + 3.seconds @@ -154,9 +154,9 @@ class CompiledTemplateTests < Test::Unit::TestCase # Only the symlink template gets rendered at t+3 Time.stubs(:now).returns(t + 3.seconds) unless windows - v.send(:compile_and_render_template, @handler, ta) - v.send(:compile_and_render_template, @handler, tb) - v.send(:compile_and_render_template, @handler, ts) unless windows + v.send(:render_template, ta) + v.send(:render_template, tb) + v.send(:render_template, ts) unless windows # the symlink has changed since last compile assert @handler.compile_time[a_n] < t + 3.seconds assert @handler.compile_time[b_n] < t + 3.seconds @@ -179,9 +179,9 @@ class CompiledTemplateTests < Test::Unit::TestCase assert @handler.send(:compile_template?, ts) unless windows Time.expects(:now).times(windows ? 1 : 2).returns(t + 5.seconds) - v.send(:compile_and_render_template, @handler, ta) - v.send(:compile_and_render_template, @handler, tb) - v.send(:compile_and_render_template, @handler, ts) unless windows + v.send(:render_template, ta) + v.send(:render_template, tb) + v.send(:render_template, ts) unless windows # the file at the end of the symlink has changed since last compile # both the symlink and the file at the end of it should be recompiled assert @handler.compile_time[a_n] < t + 5.seconds diff --git a/actionpack/test/template/template_object_test.rb b/actionpack/test/template/template_object_test.rb new file mode 100644 index 0000000000..780df17f0c --- /dev/null +++ b/actionpack/test/template/template_object_test.rb @@ -0,0 +1,62 @@ +require 'abstract_unit' + +class TemplateObjectTest < Test::Unit::TestCase + LOAD_PATH_ROOT = File.join(File.dirname(__FILE__), '..', 'fixtures') + ActionView::TemplateFinder.process_view_paths(LOAD_PATH_ROOT) + + class TemplateTest < Test::Unit::TestCase + def setup + @view = ActionView::Base.new(LOAD_PATH_ROOT) + @path = "test/hello_world.erb" + end + + def test_should_create_valid_template + template = ActionView::Template.new(@view, @path, true) + + assert_kind_of ActionView::TemplateHandlers::ERB, template.handler + assert_equal "test/hello_world.erb", template.path + assert_nil template.instance_variable_get(:"@source") + assert_equal "erb", template.extension + end + + uses_mocha 'Template preparation tests' do + + def test_should_prepare_template_properly + template = ActionView::Template.new(@view, @path, true) + view = template.instance_variable_get(:"@view") + + view.expects(:evaluate_assigns) + template.handler.expects(:compile_template).with(template) + view.expects(:method_names).returns({}) + + template.prepare! + end + + end + end + + class PartialTemplateTest < Test::Unit::TestCase + def setup + @view = ActionView::Base.new(LOAD_PATH_ROOT) + @path = "test/partial_only" + end + + def test_should_create_valid_partial_template + template = ActionView::PartialTemplate.new(@view, @path, nil) + + assert_equal "test/_partial_only", template.path + assert_equal :partial_only, template.variable_name + + assert template.locals.has_key?(:object) + assert template.locals.has_key?(:partial_only) + end + + uses_mocha 'Partial template preparation tests' do + def test_should_prepare_on_initialization + ActionView::PartialTemplate.any_instance.expects(:prepare!) + template = ActionView::PartialTemplate.new(@view, @path, 1) + end + end + end + +end |