Merge pull request #35265 from rails/return-rendered-templates

Return rendered template objects from renderers

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
 

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

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

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 = if @options.key?(:spacer_template)
-            spacer = find_template(@options[:spacer_template], @locals.keys).render(view, @locals)
+            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
 

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

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

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])

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

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.

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

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")