diff options
11 files changed, 113 insertions, 39 deletions
diff --git a/actionview/lib/action_view/base.rb b/actionview/lib/action_view/base.rb index 556a5ee502..be2a662adc 100644 --- a/actionview/lib/action_view/base.rb +++ b/actionview/lib/action_view/base.rb @@ -286,7 +286,7 @@ module ActionView #:nodoc: self.class end - def in_context(options, locals) + def in_rendering_context(options) old_view_renderer = @view_renderer old_lookup_context = @lookup_context diff --git a/actionview/lib/action_view/helpers/rendering_helper.rb b/actionview/lib/action_view/helpers/rendering_helper.rb index 7323963c72..7ead691113 100644 --- a/actionview/lib/action_view/helpers/rendering_helper.rb +++ b/actionview/lib/action_view/helpers/rendering_helper.rb @@ -27,7 +27,7 @@ module ActionView def render(options = {}, locals = {}, &block) case options when Hash - in_context(options, locals) do |renderer| + in_rendering_context(options) do |renderer| if block_given? view_renderer.render_partial(self, options.merge(partial: options[:layout]), &block) else diff --git a/actionview/lib/action_view/renderer/abstract_renderer.rb b/actionview/lib/action_view/renderer/abstract_renderer.rb index ae366ce54a..200dc3e10e 100644 --- a/actionview/lib/action_view/renderer/abstract_renderer.rb +++ b/actionview/lib/action_view/renderer/abstract_renderer.rb @@ -27,6 +27,46 @@ module ActionView raise NotImplementedError end + class RenderedCollection # :nodoc: + attr_reader :rendered_templates + + def initialize(rendered_templates, spacer) + @rendered_templates = rendered_templates + @spacer = spacer + end + + def body + @rendered_templates.map(&:body).join(@spacer.body).html_safe + end + + def format + rendered_templates.first.format + end + + class EmptyCollection + def format; nil; end + def body; nil; end + end + + EMPTY = EmptyCollection.new + end + + class RenderedTemplate # :nodoc: + attr_reader :body, :layout, :template + + def initialize(body, layout, template) + @body = body + @layout = layout + @template = template + end + + def format + template.formats.first + end + + EMPTY_SPACER = Struct.new(:body).new + end + private def extract_details(options) # :doc: @@ -49,5 +89,13 @@ module ActionView @lookup_context.formats = formats | @lookup_context.formats end + + def build_rendered_template(content, template, layout = nil) + RenderedTemplate.new content, layout, template + end + + def build_rendered_collection(templates, spacer) + RenderedCollection.new templates, spacer + end end end diff --git a/actionview/lib/action_view/renderer/partial_renderer.rb b/actionview/lib/action_view/renderer/partial_renderer.rb index f8a6f13ae9..4ae6f635ae 100644 --- a/actionview/lib/action_view/renderer/partial_renderer.rb +++ b/actionview/lib/action_view/renderer/partial_renderer.rb @@ -314,14 +314,6 @@ module ActionView template = nil end - @lookup_context.rendered_format ||= begin - if template && template.formats.first - template.formats.first - else - formats.first - end - end - if @collection render_collection(context, template) else @@ -334,10 +326,13 @@ module ActionView def render_collection(view, template) identifier = (template && template.identifier) || @path instrument(:collection, identifier: identifier, count: @collection.size) do |payload| - return nil if @collection.blank? + return RenderedCollection::EMPTY if @collection.blank? - if @options.key?(:spacer_template) - spacer = find_template(@options[:spacer_template], @locals.keys).render(view, @locals) + spacer = if @options.key?(:spacer_template) + spacer_template = find_template(@options[:spacer_template], @locals.keys) + build_rendered_template(spacer_template.render(view, @locals), spacer_template) + else + RenderedTemplate::EMPTY_SPACER end collection_body = if template @@ -347,7 +342,7 @@ module ActionView else collection_without_template(view) end - collection_body.join(spacer).html_safe + build_rendered_collection(collection_body, spacer) end end @@ -369,7 +364,7 @@ module ActionView content = layout.render(view, locals) { content } if layout payload[:cache_hit] = view.view_renderer.cache_hits[template.virtual_path] - content + build_rendered_template(content, template, layout) end end @@ -460,7 +455,7 @@ module ActionView content = template.render(view, locals) content = layout.render(view, locals) { content } if layout partial_iteration.iterate! - content + build_rendered_template(content, template, layout) end end @@ -482,7 +477,7 @@ module ActionView template = (cache[path] ||= find_template(path, keys + [as, counter, iteration])) content = template.render(view, locals) partial_iteration.iterate! - content + build_rendered_template(content, template) end end diff --git a/actionview/lib/action_view/renderer/partial_renderer/collection_caching.rb b/actionview/lib/action_view/renderer/partial_renderer/collection_caching.rb index 9f1de5a397..ed59033e27 100644 --- a/actionview/lib/action_view/renderer/partial_renderer/collection_caching.rb +++ b/actionview/lib/action_view/renderer/partial_renderer/collection_caching.rb @@ -40,7 +40,7 @@ module ActionView rendered_partials = @collection.empty? ? [] : yield index = 0 - fetch_or_cache_partial(cached_partials, order_by: keyed_collection.each_key) do + fetch_or_cache_partial(cached_partials, template, order_by: keyed_collection.each_key) do # This block is called once # for every cache miss while preserving order. rendered_partials[index].tap { index += 1 } @@ -81,11 +81,13 @@ module ActionView # # If the partial is not already cached it will also be # written back to the underlying cache store. - def fetch_or_cache_partial(cached_partials, order_by:) + def fetch_or_cache_partial(cached_partials, template, order_by:) order_by.map do |cache_key| - cached_partials.fetch(cache_key) do + if content = cached_partials[cache_key] + build_rendered_template(content, template) + else yield.tap do |rendered_partial| - collection_cache.write(cache_key, rendered_partial) + collection_cache.write(cache_key, rendered_partial.body) end end end diff --git a/actionview/lib/action_view/renderer/renderer.rb b/actionview/lib/action_view/renderer/renderer.rb index 3f3a97529d..485eb1a5b4 100644 --- a/actionview/lib/action_view/renderer/renderer.rb +++ b/actionview/lib/action_view/renderer/renderer.rb @@ -19,10 +19,14 @@ module ActionView # Main render entry point shared by Action View and Action Controller. def render(context, options) + render_to_object(context, options).body + end + + def render_to_object(context, options) # :nodoc: if options.key?(:partial) - render_partial(context, options) + render_partial_to_object(context, options) else - render_template(context, options) + render_template_to_object(context, options) end end @@ -41,16 +45,24 @@ module ActionView # Direct access to template rendering. def render_template(context, options) #:nodoc: - TemplateRenderer.new(@lookup_context).render(context, options) + render_template_to_object(context, options).body end # Direct access to partial rendering. def render_partial(context, options, &block) #:nodoc: - PartialRenderer.new(@lookup_context).render(context, options, block) + render_partial_to_object(context, options, &block).body end def cache_hits # :nodoc: @cache_hits ||= {} end + + def render_template_to_object(context, options) #:nodoc: + TemplateRenderer.new(@lookup_context).render(context, options) + end + + def render_partial_to_object(context, options, &block) #:nodoc: + PartialRenderer.new(@lookup_context).render(context, options, block) + end end end diff --git a/actionview/lib/action_view/renderer/streaming_template_renderer.rb b/actionview/lib/action_view/renderer/streaming_template_renderer.rb index f414620923..279ef3c680 100644 --- a/actionview/lib/action_view/renderer/streaming_template_renderer.rb +++ b/actionview/lib/action_view/renderer/streaming_template_renderer.rb @@ -44,7 +44,7 @@ module ActionView # object that responds to each. This object is initialized with a block # that knows how to render the template. def render_template(view, template, layout_name = nil, locals = {}) #:nodoc: - return [super] unless layout_name && template.supports_streaming? + return [super.body] unless layout_name && template.supports_streaming? locals ||= {} layout = layout_name && find_layout(layout_name, locals.keys, [formats.first]) diff --git a/actionview/lib/action_view/renderer/template_renderer.rb b/actionview/lib/action_view/renderer/template_renderer.rb index c36baeffcd..c17d6182e8 100644 --- a/actionview/lib/action_view/renderer/template_renderer.rb +++ b/actionview/lib/action_view/renderer/template_renderer.rb @@ -10,8 +10,6 @@ module ActionView prepend_formats(template.formats) - @lookup_context.rendered_format ||= (template.formats.first || formats.first) - render_template(context, template, options[:layout], options[:locals] || {}) end @@ -46,23 +44,24 @@ module ActionView # Renders the given template. A string representing the layout can be # supplied as well. def render_template(view, template, layout_name, locals) - render_with_layout(view, layout_name, locals) do |layout| + render_with_layout(view, layout_name, template, locals) do |layout| instrument(:template, identifier: template.identifier, layout: layout.try(:virtual_path)) do template.render(view, locals) { |*name| view._layout_for(*name) } end end end - def render_with_layout(view, path, locals) + def render_with_layout(view, path, template, locals) layout = path && find_layout(path, locals.keys, [formats.first]) content = yield(layout) - if layout + body = if layout view.view_flow.set(:layout, content) layout.render(view, locals) { |*name| view._layout_for(*name) } else content end + build_rendered_template(body, template, layout) end # This is the method which actually finds the layout using details in the lookup diff --git a/actionview/lib/action_view/rendering.rb b/actionview/lib/action_view/rendering.rb index b798e80b04..699ccb0ca8 100644 --- a/actionview/lib/action_view/rendering.rb +++ b/actionview/lib/action_view/rendering.rb @@ -109,10 +109,15 @@ module ActionView context = view_context context.assign assigns if assigns - lookup_context.rendered_format = nil if options[:formats] lookup_context.variants = variant if variant - context.view_renderer.render(context, options) + rendered_template = context.in_rendering_context(options) do |renderer| + renderer.render_to_object(context, options) + end + + lookup_context.rendered_format = rendered_template.format || lookup_context.formats.first + + rendered_template.body end # Assign the rendered format to look up context. diff --git a/actionview/test/actionpack/controller/render_test.rb b/actionview/test/actionpack/controller/render_test.rb index 727d3fbc1a..52c3c54d96 100644 --- a/actionview/test/actionpack/controller/render_test.rb +++ b/actionview/test/actionpack/controller/render_test.rb @@ -174,6 +174,10 @@ class TestController < ActionController::Base render inline: "<%= controller_name %>" end + def inline_rendered_format_without_format + render inline: "test" + end + # :ported: def render_custom_code render plain: "hello world", status: 404 @@ -659,6 +663,7 @@ class RenderTest < ActionController::TestCase get :hello_world_from_rxml_using_action, to: "test#hello_world_from_rxml_using_action" get :hello_world_from_rxml_using_template, to: "test#hello_world_from_rxml_using_template" get :hello_world_with_layout_false, to: "test#hello_world_with_layout_false" + get :inline_rendered_format_without_format, to: "test#inline_rendered_format_without_format" get :layout_overriding_layout, to: "test#layout_overriding_layout" get :layout_test, to: "test#layout_test" get :layout_test_with_different_layout, to: "test#layout_test_with_different_layout" @@ -1015,6 +1020,12 @@ class RenderTest < ActionController::TestCase assert_equal "<wrapper>\n<html>\n <p>Hello </p>\n<p>This is grand!</p>\n</html>\n</wrapper>\n", @response.body end + def test_rendered_format_without_format + get :inline_rendered_format_without_format + assert_equal "test", @response.body + assert_equal "text/html", @response.content_type + end + def test_partials_list get :partials_list assert_equal "goodbyeHello: davidHello: marygoodbye\n", @response.body diff --git a/actionview/test/template/render_test.rb b/actionview/test/template/render_test.rb index 5068e00c7d..ee8a110b44 100644 --- a/actionview/test/template/render_test.rb +++ b/actionview/test/template/render_test.rb @@ -69,11 +69,6 @@ module RenderTestCases assert_match "<error>No Comment</error>", @view.render(template: "comments/empty", formats: [:xml]) end - def test_rendered_format_without_format - @view.render(inline: "test") - assert_equal :html, @view.lookup_context.rendered_format - end - def test_render_partial_implicitly_use_format_of_the_rendered_template @view.lookup_context.formats = [:json] assert_equal "Hello world", @view.render(template: "test/one", formats: [:html]) @@ -743,10 +738,17 @@ class CachedCollectionViewRenderTest < ActiveSupport::TestCase end teardown do - GC.start I18n.reload! end + test "template body written to cache" do + customer = Customer.new("david", 1) + key = cache_key(customer, "test/_customer") + assert_nil ActionView::PartialRenderer.collection_cache.read(key) + @view.render(partial: "test/customer", collection: [customer], cached: true) + assert_equal "Hello: david", ActionView::PartialRenderer.collection_cache.read(key) + end + test "collection caching does not cache by default" do customer = Customer.new("david", 1) key = cache_key(customer, "test/_customer") |