From 6c280f3398966ffba45069500ff43d632513fe44 Mon Sep 17 00:00:00 2001 From: Carl Lerche Date: Fri, 30 Apr 2010 16:40:42 -0700 Subject: RouteSet does not raise ActionController::RoutingError when no routes match anymore. Instead, it follows the X-Cascade convention. ShowExceptions checks for X-Cascade so that the routing error page can still be displayed. --- .../action_dispatch/middleware/show_exceptions.rb | 12 ++++++++- .../lib/action_dispatch/routing/route_set.rb | 5 ---- actionpack/test/controller/rescue_test.rb | 6 +++-- actionpack/test/dispatch/routing_test.rb | 30 ++++++++++++++-------- 4 files changed, 35 insertions(+), 18 deletions(-) diff --git a/actionpack/lib/action_dispatch/middleware/show_exceptions.rb b/actionpack/lib/action_dispatch/middleware/show_exceptions.rb index 43440e5f7c..12a93d6a24 100644 --- a/actionpack/lib/action_dispatch/middleware/show_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/show_exceptions.rb @@ -45,7 +45,17 @@ module ActionDispatch end def call(env) - @app.call(env) + status, headers, body = @app.call(env) + + # Only this middleware cares about RoutingError. So, let's just raise + # it here. + # TODO: refactor this middleware to handle the X-Cascade scenario without + # having to raise an exception. + if headers['X-Cascade'] == 'pass' + raise ActionController::RoutingError, "No route matches #{env['PATH_INFO'].inspect}" + end + + [status, headers, body] rescue Exception => exception raise exception if env['action_dispatch.show_exceptions'] == false render_exception(env, exception) diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index fdbff74071..0d071dd7fe 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -6,10 +6,6 @@ require 'action_dispatch/routing/deprecated_mapper' module ActionDispatch module Routing class RouteSet #:nodoc: - NotFound = lambda { |env| - raise ActionController::RoutingError, "No route matches #{env['PATH_INFO'].inspect}" - } - PARAMETERS_KEY = 'action_dispatch.request.path_parameters' class Dispatcher #:nodoc: @@ -224,7 +220,6 @@ module ActionDispatch def finalize! return if @finalized @finalized = true - @set.add_route(NotFound) @set.freeze end diff --git a/actionpack/test/controller/rescue_test.rb b/actionpack/test/controller/rescue_test.rb index dd991898a8..0f64b77647 100644 --- a/actionpack/test/controller/rescue_test.rb +++ b/actionpack/test/controller/rescue_test.rb @@ -326,7 +326,8 @@ class RescueTest < ActionController::IntegrationTest end test 'rescue routing exceptions' do - @app = ActionDispatch::Rescue.new(SharedTestRoutes) do + raiser = proc { |env| raise ActionController::RoutingError, "Did not handle the request" } + @app = ActionDispatch::Rescue.new(raiser) do rescue_from ActionController::RoutingError, lambda { |env| [200, {"Content-Type" => "text/html"}, ["Gotcha!"]] } end @@ -335,7 +336,8 @@ class RescueTest < ActionController::IntegrationTest end test 'unrescued exception' do - @app = ActionDispatch::Rescue.new(SharedTestRoutes) + raiser = proc { |env| raise ActionController::RoutingError, "Did not handle the request" } + @app = ActionDispatch::Rescue.new(raiser) assert_raise(ActionController::RoutingError) { get '/b00m' } end diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index b508996467..651a7a6be0 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -392,12 +392,14 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest get '/admin', {}, {'REMOTE_ADDR' => '192.168.1.100'} assert_equal 'queenbee#index', @response.body - assert_raise(ActionController::RoutingError) { get '/admin', {}, {'REMOTE_ADDR' => '10.0.0.100'} } + get '/admin', {}, {'REMOTE_ADDR' => '10.0.0.100'} + assert_equal 'pass', @response.headers['X-Cascade'] get '/admin/accounts', {}, {'REMOTE_ADDR' => '192.168.1.100'} assert_equal 'queenbee#accounts', @response.body - assert_raise(ActionController::RoutingError) { get '/admin/accounts', {}, {'REMOTE_ADDR' => '10.0.0.100'} } + get '/admin/accounts', {}, {'REMOTE_ADDR' => '10.0.0.100'} + assert_equal 'pass', @response.headers['X-Cascade'] end end @@ -648,10 +650,14 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest assert_equal 'comments#index', @response.body assert_equal '/posts/1/comments', post_comments_path(:post_id => 1) - assert_raise(ActionController::RoutingError) { post '/posts' } - assert_raise(ActionController::RoutingError) { put '/posts/1' } - assert_raise(ActionController::RoutingError) { delete '/posts/1' } - assert_raise(ActionController::RoutingError) { delete '/posts/1/comments' } + post '/posts' + assert_equal 'pass', @response.headers['X-Cascade'] + put '/posts/1' + assert_equal 'pass', @response.headers['X-Cascade'] + delete '/posts/1' + assert_equal 'pass', @response.headers['X-Cascade'] + delete '/posts/1/comments' + assert_equal 'pass', @response.headers['X-Cascade'] end end @@ -775,7 +781,8 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest get '/articles/rails/1' assert_equal 'articles#with_id', @response.body - assert_raise(ActionController::RoutingError) { get '/articles/123/1' } + get '/articles/123/1' + assert_equal 'pass', @response.headers['X-Cascade'] assert_equal '/articles/rails/1', article_with_title_path(:title => 'rails', :id => 1) end @@ -953,19 +960,22 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest def test_resource_constraints with_test_routes do - assert_raise(ActionController::RoutingError) { get '/products/1' } + get '/products/1' + assert_equal 'pass', @response.headers['X-Cascade'] get '/products' assert_equal 'products#index', @response.body get '/products/0001' assert_equal 'products#show', @response.body - assert_raise(ActionController::RoutingError) { get '/products/1/images' } + get '/products/1/images' + assert_equal 'pass', @response.headers['X-Cascade'] get '/products/0001/images' assert_equal 'images#index', @response.body get '/products/0001/images/1' assert_equal 'images#show', @response.body - assert_raise(ActionController::RoutingError) { get '/dashboard', {}, {'REMOTE_ADDR' => '10.0.0.100'} } + get '/dashboard', {}, {'REMOTE_ADDR' => '10.0.0.100'} + assert_equal 'pass', @response.headers['X-Cascade'] get '/dashboard', {}, {'REMOTE_ADDR' => '192.168.1.100'} assert_equal 'dashboards#show', @response.body end -- cgit v1.2.3