Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Refactor partial rendering into a PartialTemplate class. [Pratik]

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@8976 5ecf4fe2-1ee6-0310-87b1-e25e094e27de
  • Loading branch information...
commit 51b6619d4e78d9948952dab4709274ab57f41206 1 parent 10d9d0b
@NZKoz NZKoz authored
View
2  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]
View
1  actionpack/lib/action_view.rb
@@ -29,6 +29,7 @@
require 'action_view/template_finder'
require 'action_view/template'
+require 'action_view/partial_template'
require 'action_view/base'
require 'action_view/partials'
View
48 actionpack/lib/action_view/base.rb
@@ -292,24 +292,22 @@ def render_file(template_path, use_full_path = true, local_assigns = {}) #:nodoc
# 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 @@ def render(options = {}, old_local_assigns = {}, &block) #:nodoc:
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 @@ def template_format
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 @@ def evaluate_assigns
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
View
61 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
View
94 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 @@ def render_partial(partial_path, object_assigns = nil, local_assigns = nil) #:no
""
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
View
22 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 @@ def initialize(view, path_or_source, use_full_path, locals = {}, inline = false,
@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 @@ def method_key
@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
View
2  actionpack/lib/action_view/template_handler.rb
@@ -13,7 +13,7 @@ def initialize(view)
@view = view
end
- def render(template, local_assigns)
+ def render(template)
end
def compile(template)
View
4 actionpack/lib/action_view/template_handlers/compilable.rb
@@ -24,6 +24,10 @@ def compilable?
true
end
end
+
+ def render(template)
+ @view.send :execute, template
+ end
# Compile and evaluate the template's code
def compile_template(template)
View
6 actionpack/test/controller/custom_handler_test.rb
@@ -5,9 +5,9 @@ def initialize( view )
@view = view
end
- def render( template, local_assigns )
- [ template,
- local_assigns,
+ def render( template )
+ [ template.source,
+ template.locals,
@view ]
end
end
View
4 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
View
20 actionpack/test/controller/new_render_test.rb
@@ -146,6 +146,14 @@ def partial_with_form_builder_subclass
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 @@ def test_partial_collection
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
View
1  actionpack/test/fixtures/test/_customer_counter.erb
@@ -0,0 +1 @@
+<%= customer_counter.name %><%= customer_counter_counter %>
View
24 actionpack/test/template/compiled_templates_test.rb
@@ -109,9 +109,9 @@ def test_compile_time
# 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 @@ def test_compile_time
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 @@ def test_compile_time
# 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 @@ def test_compile_time
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
View
62 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
Please sign in to comment.
Something went wrong with that request. Please try again.