diff options
| -rw-r--r-- | actionpack/lib/action_controller/metal/live.rb | 23 | ||||
| -rw-r--r-- | actionpack/lib/action_controller/test_case.rb | 28 | ||||
| -rw-r--r-- | actionpack/lib/action_dispatch/journey/formatter.rb | 4 | ||||
| -rw-r--r-- | actionpack/lib/action_dispatch/journey/gtg/transition_table.rb | 6 | ||||
| -rw-r--r-- | actionpack/test/controller/live_stream_test.rb | 48 | ||||
| -rw-r--r-- | railties/CHANGELOG.md | 9 | ||||
| -rw-r--r-- | railties/lib/rails/generators/rails/controller/controller_generator.rb | 4 | ||||
| -rw-r--r-- | railties/test/generators/plugin_generator_test.rb | 12 |
8 files changed, 89 insertions, 45 deletions
diff --git a/actionpack/lib/action_controller/metal/live.rb b/actionpack/lib/action_controller/metal/live.rb index 5ef4f6ccda..d60f1b0d44 100644 --- a/actionpack/lib/action_controller/metal/live.rb +++ b/actionpack/lib/action_controller/metal/live.rb @@ -107,8 +107,11 @@ module ActionController end class Buffer < ActionDispatch::Response::Buffer #:nodoc: + include MonitorMixin + def initialize(response) @error_callback = lambda { true } + @cv = new_cond super(response, SizedQueue.new(10)) end @@ -128,8 +131,17 @@ module ActionController end def close - super - @buf.push nil + synchronize do + super + @buf.push nil + @cv.broadcast + end + end + + def await_close + synchronize do + @cv.wait_until { @closed } + end end def on_error(&block) @@ -191,6 +203,7 @@ module ActionController t1 = Thread.current locals = t1.keys.map { |key| [key, t1[key]] } + error = nil # This processes the action in a child thread. It lets us return the # response code and headers back up the rack stack, and still process # the body in parallel with sending data to the client @@ -205,8 +218,9 @@ module ActionController begin super(name) rescue => e - @_response.status = 500 unless @_response.committed? - @_response.status = 400 if e.class == ActionController::BadRequest + unless @_response.committed? + error = e + end begin @_response.stream.write(ActionView::Base.streaming_completion_on_exception) if request.format == :html @_response.stream.call_on_error @@ -222,6 +236,7 @@ module ActionController } @_response.await_commit + raise error if error end def log_error(exception) diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index 8650b75400..33a5858766 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -258,6 +258,17 @@ module ActionController end end + class LiveTestResponse < Live::Response + def recycle! + @body = nil + initialize + end + + def body + @body ||= super + end + end + # Methods #destroy and #load! are overridden to avoid calling methods on the # @store object, which does not exist for the TestSession class. class TestSession < Rack::Session::Abstract::SessionHash #:nodoc: @@ -583,13 +594,14 @@ module ActionController end def setup_controller_request_and_response - @request = build_request - @response = build_response - @response.request = @request - @controller = nil unless defined? @controller + response_klass = TestResponse + if klass = self.class.controller_class + if klass < ActionController::Live + response_klass = LiveTestResponse + end unless @controller begin @controller = klass.new @@ -599,6 +611,10 @@ module ActionController end end + @request = build_request + @response = build_response response_klass + @response.request = @request + if @controller @controller.request = @request @controller.params = {} @@ -609,8 +625,8 @@ module ActionController TestRequest.new end - def build_response - TestResponse.new + def build_response(klass) + klass.new end included do diff --git a/actionpack/lib/action_dispatch/journey/formatter.rb b/actionpack/lib/action_dispatch/journey/formatter.rb index 4410c1b5d5..57f0963731 100644 --- a/actionpack/lib/action_dispatch/journey/formatter.rb +++ b/actionpack/lib/action_dispatch/journey/formatter.rb @@ -121,9 +121,9 @@ module ActionDispatch def possibles(cache, options, depth = 0) cache.fetch(:___routes) { [] } + options.find_all { |pair| cache.key?(pair) - }.map { |pair| + }.flat_map { |pair| possibles(cache[pair], options, depth + 1) - }.flatten(1) + } end # Returns +true+ if no missing keys are present, otherwise +false+. diff --git a/actionpack/lib/action_dispatch/journey/gtg/transition_table.rb b/actionpack/lib/action_dispatch/journey/gtg/transition_table.rb index 5a79059ed6..a5b19fcf06 100644 --- a/actionpack/lib/action_dispatch/journey/gtg/transition_table.rb +++ b/actionpack/lib/action_dispatch/journey/gtg/transition_table.rb @@ -120,11 +120,11 @@ module ActionDispatch end def transitions - @string_states.map { |from, hash| + @string_states.flat_map { |from, hash| hash.map { |s, to| [from, s, to] } - }.flatten(1) + @regexp_states.map { |from, hash| + } + @regexp_states.flat_map { |from, hash| hash.map { |s, to| [from, s, to] } - }.flatten(1) + } end private diff --git a/actionpack/test/controller/live_stream_test.rb b/actionpack/test/controller/live_stream_test.rb index 934915fc5b..eb8b2c9832 100644 --- a/actionpack/test/controller/live_stream_test.rb +++ b/actionpack/test/controller/live_stream_test.rb @@ -1,5 +1,6 @@ require 'abstract_unit' require 'active_support/concurrency/latch' +Thread.abort_on_exception = true module ActionController class SSETest < ActionController::TestCase @@ -43,9 +44,7 @@ module ActionController tests SSETestController def wait_for_response_stream_close - while !response.stream.closed? - sleep 0.01 - end + response.stream.await_close end def test_basic_sse @@ -91,6 +90,9 @@ module ActionController end class LiveStreamTest < ActionController::TestCase + class Exception < StandardError + end + class TestController < ActionController::Base include ActionController::Live @@ -153,11 +155,12 @@ module ActionController response.stream.close end + response.stream.write "" # make sure the response is committed raise 'An exception occurred...' end def exception_in_controller - raise 'Exception in controller' + raise Exception, 'Exception in controller' end def bad_request_error @@ -169,23 +172,15 @@ module ActionController response.stream.on_error do raise 'We need to go deeper.' end + response.stream.write '' response.stream.write params[:widget][:didnt_check_for_nil] end end tests TestController - class TestResponse < Live::Response - def recycle! - initialize - end - end - - def build_response - TestResponse.new - end - def assert_stream_closed + response.stream.await_close assert response.stream.closed?, 'stream should be closed' end @@ -257,24 +252,19 @@ module ActionController end def test_exception_handling_html - capture_log_output do |output| + assert_raises(ActionView::MissingTemplate) do get :exception_in_view - assert_match %r((window\.location = "/500\.html"</script></html>)$), response.body - assert_match 'Missing template test/doesntexist', output.rewind && output.read - assert_stream_closed end + assert_stream_closed end def test_exception_handling_plain_text - capture_log_output do |output| + assert_raises(ActionView::MissingTemplate) do get :exception_in_view, format: :json - assert_equal '', response.body - assert_match 'Missing template test/doesntexist', output.rewind && output.read - assert_stream_closed end end - def test_exception_callback + def test_exception_callback_when_committed capture_log_output do |output| get :exception_with_callback, format: 'text/event-stream' assert_equal %(data: "500 Internal Server Error"\n\n), response.body @@ -284,16 +274,18 @@ module ActionController end def test_exception_in_controller_before_streaming - response = get :exception_in_controller, format: 'text/event-stream' - assert_equal 500, response.status + assert_raises(ActionController::LiveStreamTest::Exception) do + get :exception_in_controller, format: 'text/event-stream' + end end def test_bad_request_in_controller_before_streaming - response = get :bad_request_error, format: 'text/event-stream' - assert_equal 400, response.status + assert_raises(ActionController::BadRequest) do + get :bad_request_error, format: 'text/event-stream' + end end - def test_exceptions_raised_handling_exceptions + def test_exceptions_raised_handling_exceptions_and_committed capture_log_output do |output| get :exception_in_exception_callback, format: 'text/event-stream' assert_equal '', response.body diff --git a/railties/CHANGELOG.md b/railties/CHANGELOG.md index 18f2546c73..9dce38fc93 100644 --- a/railties/CHANGELOG.md +++ b/railties/CHANGELOG.md @@ -1 +1,10 @@ +* Avoid namespacing routes inside engines. + + Mountable engines are namespaced by default so the generated routes + were too while they should not. + + Fixes #14079. + + *Yves Senn*, *Carlos Antonio da Silva*, *Robin Dupret* + Please check [4-1-stable](https://github.com/rails/rails/blob/4-1-stable/railties/CHANGELOG.md) for previous changes. diff --git a/railties/lib/rails/generators/rails/controller/controller_generator.rb b/railties/lib/rails/generators/rails/controller/controller_generator.rb index 33a0d81bf6..7588a558e7 100644 --- a/railties/lib/rails/generators/rails/controller/controller_generator.rb +++ b/railties/lib/rails/generators/rails/controller/controller_generator.rb @@ -27,11 +27,11 @@ module Rails # end # end def generate_routing_code(action) - depth = class_path.length + depth = regular_class_path.length # Create 'namespace' ladder # namespace :foo do # namespace :bar do - namespace_ladder = class_path.each_with_index.map do |ns, i| + namespace_ladder = regular_class_path.each_with_index.map do |ns, i| indent("namespace :#{ns} do\n", i * 2) end.join diff --git a/railties/test/generators/plugin_generator_test.rb b/railties/test/generators/plugin_generator_test.rb index 932cd75bcb..b2fc3a2a4f 100644 --- a/railties/test/generators/plugin_generator_test.rb +++ b/railties/test/generators/plugin_generator_test.rb @@ -355,6 +355,18 @@ class PluginGeneratorTest < Rails::Generators::TestCase FileUtils.rm gemfile_path end + def test_generating_controller_inside_mountable_engine + run_generator [destination_root, "--mountable"] + + capture(:stdout) do + `#{destination_root}/bin/rails g controller admin/dashboard foo` + end + + assert_file "config/routes.rb" do |contents| + assert_match(/namespace :admin/, contents) + assert_no_match(/namespace :bukkit/, contents) + end + end protected def action(*args, &block) |