Add tests for streaming chunks + a few minor fixes.

ioquatix · ioquatix · commit 6ddf5d8a8924 · 2024-11-01T19:28:44.000+13:00
diff --git a/actionpack/lib/action_controller/metal/live.rb b/actionpack/lib/action_controller/metal/live.rb
@@ -171,12 +171,6 @@ def initialize(response)
         @ignore_disconnect = false
       end
 
-      # ActionDispatch::Response delegates #to_ary to the internal
-      # ActionDispatch::Response::Buffer, defining #to_ary is an indicator that the
-      # response body can be buffered and/or cached by Rack middlewares, this is not
-      # the case for Live responses so we undefine it for this Buffer subclass.
-      undef_method :to_ary
-
       def write(string)
         unless @response.committed?
           @response.headers["Cache-Control"] ||= "no-cache"
diff --git a/actionpack/lib/action_dispatch/http/response.rb b/actionpack/lib/action_dispatch/http/response.rb
@@ -105,10 +105,18 @@ def initialize(response, buf)
         @str_body = nil
       end
 
+      BODY_METHODS = { to_ary: true }
+
+      def respond_to?(method, include_private = false)
+        if BODY_METHODS.key?(method)
+          @buf.respond_to?(method)
+        else
+          super
+        end
+      end
+
       def to_ary
-        @buf.respond_to?(:to_ary) ?
-          @buf.to_ary :
-          @buf.each
+        @buf.to_ary
       end
 
       def body
@@ -499,6 +507,8 @@ def initialize(response)
         @response = response
       end
 
+      attr :response
+
       def close
         # Rack "close" maps to Response#abort, and **not** Response#close (which is used
         # when the controller's finished writing)
diff --git a/actionpack/test/controller/new_base/render_streaming_test.rb b/actionpack/test/controller/new_base/render_streaming_test.rb
@@ -45,44 +45,78 @@ def explicit_cache
 
   class StreamingTest < Rack::TestCase
     test "rendering with streaming enabled at the class level" do
+      env = Rack::MockRequest.env_for("/render_streaming/basic/hello_world")
+      status, headers, body = app.call(env)
+      assert_streaming!(status, headers, body)
+      assert_chunks ["Hello world", ", I'm here!"], body
+
       get "/render_streaming/basic/hello_world"
       assert_body "Hello world, I'm here!"
-      assert_streaming!
     end
 
     test "rendering with streaming given to render" do
+      env = Rack::MockRequest.env_for("/render_streaming/basic/explicit")
+      status, headers, body = app.call(env)
+      assert_streaming!(status, headers, body)
+      assert_chunks ["Hello world", ", I'm here!"], body
+
       get "/render_streaming/basic/explicit"
       assert_body "Hello world, I'm here!"
-      assert_streaming!
+      assert_cache_control!
     end
 
     test "rendering with streaming do not override explicit cache control given to render" do
+      env = Rack::MockRequest.env_for("/render_streaming/basic/explicit_cache")
+      status, headers, body = app.call(env)
+      assert_streaming!(status, headers, body)
+      assert_chunks ["Hello world", ", I'm here!"], body
+
       get "/render_streaming/basic/explicit_cache"
       assert_body "Hello world, I'm here!"
-      assert_streaming! "private"
+      assert_cache_control! "private"
     end
 
     test "rendering with streaming no layout" do
+      env = Rack::MockRequest.env_for("/render_streaming/basic/no_layout")
+      status, headers, body = app.call(env)
+      assert_streaming!(status, headers, body)
+      assert_chunks ["Hello world"], body
+
       get "/render_streaming/basic/no_layout"
       assert_body "Hello world"
-      assert_streaming!
+      assert_cache_control!
     end
 
     test "skip rendering with streaming at render level" do
+      env = Rack::MockRequest.env_for("/render_streaming/basic/skip")
+      status, _, body = app.call(env)
+      assert_equal 200, status
+      assert_chunks ["Hello world, I'm here!"], body
+
       get "/render_streaming/basic/skip"
       assert_body "Hello world, I'm here!"
     end
 
     test "rendering with layout exception" do
+      env = Rack::MockRequest.env_for("/render_streaming/basic/layout_exception")
+      status, headers, body = app.call(env)
+      assert_streaming!(status, headers, body)
+      assert_chunks ["<body class=\"", "\"><script>window.location = \"/500.html\"</script></html>"], body
+
       get "/render_streaming/basic/layout_exception"
       assert_body "<body class=\"\"><script>window.location = \"/500.html\"</script></html>"
-      assert_streaming!
+      assert_cache_control!
     end
 
     test "rendering with template exception" do
+      env = Rack::MockRequest.env_for("/render_streaming/basic/template_exception")
+      status, headers, body = app.call(env)
+      assert_streaming!(status, headers, body)
+      assert_chunks ["\"><script>window.location = \"/500.html\"</script></html>"], body
+
       get "/render_streaming/basic/template_exception"
       assert_body "\"><script>window.location = \"/500.html\"</script></html>"
-      assert_streaming!
+      assert_cache_control!
     end
 
     test "rendering with template exception logs the exception" do
@@ -98,9 +132,28 @@ class StreamingTest < Rack::TestCase
       end
     end
 
-    def assert_streaming!(cache = "no-cache")
-      assert_status 200
-      assert_equal cache, headers["cache-control"]
+    def assert_streaming!(status, headers, body)
+      assert_equal 200, status
+
+      # It should not have a content length
+      assert_nil headers["content-length"]
+
+      # The body should not respond to `#to_ary`
+      assert_not_respond_to body, :to_ary
+    end
+
+    def assert_cache_control!(value = "no-cache", headers: self.headers)
+      assert_equal value, headers["cache-control"]
+    end
+
+    def assert_chunks(expected, body)
+      index = 0
+      body.each do |chunk|
+        assert_equal expected[index], chunk
+        index += 1
+      end
+
+      assert_equal expected.size, index
     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
@@ -42,7 +42,7 @@ def log_error(exception)
     # 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.body] unless layout_name && template.supports_streaming?
+      return [super.body] unless template.supports_streaming?
 
       locals ||= {}
       layout   = find_layout(layout_name, locals.keys, [formats.first])
diff --git a/actionview/lib/action_view/template/raw_file.rb b/actionview/lib/action_view/template/raw_file.rb
@@ -20,6 +20,10 @@ def identifier
       def render(*args)
         ::File.read(@filename)
       end
+
+      def supports_streaming?
+        false
+      end
     end
   end
 end
