diff options
author | eileencodes <eileencodes@gmail.com> | 2015-12-09 07:51:02 -0500 |
---|---|---|
committer | eileencodes <eileencodes@gmail.com> | 2015-12-09 08:19:07 -0500 |
commit | b05801754f6423a1d90954ef3f6e2f5dc55c6320 (patch) | |
tree | b909dbee3aef87372f4d1c7a2ce39b5976fd8e85 | |
parent | 92a9744f8132f46b0e5f49fa5b4d8c66b4e861cb (diff) | |
download | rails-b05801754f6423a1d90954ef3f6e2f5dc55c6320.tar.gz rails-b05801754f6423a1d90954ef3f6e2f5dc55c6320.tar.bz2 rails-b05801754f6423a1d90954ef3f6e2f5dc55c6320.zip |
Fix `make_response!` when called by `serve` in `RouteSet`
All of our tests were testing the `ActionController::Live` behavior in a
standalone environment, without going through the router or behaving
like a real application.
This resulted in `ActionController::Live` throwing the exception
`undefined method 'request' for #<ActionDispatch::Request:0x00000003ad1148>`
because `make_response!` was expecting a response instead of a request.
The expectation of a response came from `set_response!` in non-router
tests setting the response and passing it to `make_response!`. In the
case of an application we would hit `serve` in `RouteSet` first which
would send us to `make_response!` with a request sent instead of a
response.
The changes here remove `set_response!` so `make_response!` always
receives a request.
Thanks to KalabiYau for help with the investigation and solution.
Fixes #22524
[Eileen M. Uchitelle & KalabiYau]
-rw-r--r-- | actionpack/lib/action_controller/metal/live.rb | 7 | ||||
-rw-r--r-- | actionpack/test/controller/live_stream_test.rb | 39 |
2 files changed, 40 insertions, 6 deletions
diff --git a/actionpack/lib/action_controller/metal/live.rb b/actionpack/lib/action_controller/metal/live.rb index 6804e577f0..e3c540bf5f 100644 --- a/actionpack/lib/action_controller/metal/live.rb +++ b/actionpack/lib/action_controller/metal/live.rb @@ -36,8 +36,7 @@ module ActionController extend ActiveSupport::Concern module ClassMethods - def make_response!(response) - request = response.request + def make_response!(request) if request.get_header("HTTP_VERSION") == "HTTP/1.0" super else @@ -287,9 +286,5 @@ module ActionController super response.close if response end - - def set_response!(response) - @_response = self.class.make_response! response - end end end diff --git a/actionpack/test/controller/live_stream_test.rb b/actionpack/test/controller/live_stream_test.rb index 843dafac06..aab2d9545d 100644 --- a/actionpack/test/controller/live_stream_test.rb +++ b/actionpack/test/controller/live_stream_test.rb @@ -442,3 +442,42 @@ module ActionController end end end + +class LiveStreamRouterTest < ActionDispatch::IntegrationTest + class TestController < ActionController::Base + include ActionController::Live + + def index + response.headers['Content-Type'] = 'text/event-stream' + sse = SSE.new(response.stream) + sse.write("{\"name\":\"John\"}") + sse.write({ name: "Ryan" }) + ensure + sse.close + end + end + + def self.call(env) + routes.call(env) + end + + def self.routes + @routes ||= ActionDispatch::Routing::RouteSet.new + end + + routes.draw do + get '/test' => 'live_stream_router_test/test#index' + end + + def app + self.class + end + + test "streaming served through the router" do + get "/test" + + assert_response :ok + assert_match(/data: {\"name\":\"John\"}/, response.body) + assert_match(/data: {\"name\":\"Ryan\"}/, response.body) + end +end |