diff options
author | Carlos Antonio da Silva <carlosantoniodasilva@gmail.com> | 2013-01-05 05:41:13 -0800 |
---|---|---|
committer | Carlos Antonio da Silva <carlosantoniodasilva@gmail.com> | 2013-01-05 05:41:13 -0800 |
commit | a6cec6e8ed7cc158fa1bc3cc59a5c3fa392b2afe (patch) | |
tree | 8dc6b747221945e37a0ea1959a9c567271a16cdc /actionpack | |
parent | 1e1c7afe3fed844e1a11a75e125a0ac7af1d468b (diff) | |
parent | 80795e02ca7025651ee7abc2e85e123e5e0a7ad6 (diff) | |
download | rails-a6cec6e8ed7cc158fa1bc3cc59a5c3fa392b2afe.tar.gz rails-a6cec6e8ed7cc158fa1bc3cc59a5c3fa392b2afe.tar.bz2 rails-a6cec6e8ed7cc158fa1bc3cc59a5c3fa392b2afe.zip |
Merge pull request #8658 from senny/8649_unify_routes_output_for_console_and_web
Unify routes output for console and web, ensure engine routes are shown in html routes table.
Closes #8649.
Diffstat (limited to 'actionpack')
6 files changed, 109 insertions, 35 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 3e8441343f..8a9cd2d8ce 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -560,7 +560,9 @@ *Carlos Galdino + Rafael Mendonça França* -* Show routes in exception page while debugging a `RoutingError` in development. *Richard Schneeman and Mattt Thompson* +* Show routes in exception page while debugging a `RoutingError` in development. + + *Richard Schneeman + Mattt Thompson + Yves Senn* * Add `ActionController::Flash.add_flash_types` method to allow people to register their own flash types. e.g.: diff --git a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb index ac8f6af7fe..5c50f9ec6a 100644 --- a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb @@ -2,7 +2,6 @@ require 'action_dispatch/http/request' require 'action_dispatch/middleware/exception_wrapper' require 'action_dispatch/routing/inspector' - module ActionDispatch # This middleware is responsible for logging exceptions and # showing a debugging page in case the request is local. @@ -42,12 +41,11 @@ module ActionDispatch :application_trace => wrapper.application_trace, :framework_trace => wrapper.framework_trace, :full_trace => wrapper.full_trace, - :routes => formatted_routes(exception), + :routes_inspector => routes_inspector(exception), :source_extract => wrapper.source_extract, :line_number => wrapper.line_number, :file => wrapper.file ) - file = "rescues/#{wrapper.rescue_template}" body = template.render(:template => file, :layout => 'rescues/layout') render(wrapper.status_code, body) @@ -85,11 +83,28 @@ module ActionDispatch @stderr_logger ||= ActiveSupport::Logger.new($stderr) end - def formatted_routes(exception) + def routes_inspector(exception) return false unless @routes_app.respond_to?(:routes) if exception.is_a?(ActionController::RoutingError) || exception.is_a?(ActionView::Template::Error) - inspector = ActionDispatch::Routing::RoutesInspector.new - inspector.collect_routes(@routes_app.routes.routes) + ActionDispatch::Routing::RoutesInspector.new(@routes_app.routes.routes) + end + end + + class TableRoutesFormatter + def initialize(view) + @view = view + @buffer = [] + end + + def section(type, title, routes) + @buffer << %(<tr><th colspan="4">#{title}</th></tr>) + @buffer << @view.render(partial: "routes/route", collection: routes) + end + + def result + @view.raw @view.render(layout: "routes/route_wrapper") { + @view.raw @buffer.join("\n") + } end end end diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/routing_error.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/routing_error.erb index 77804a8cbb..9463da520b 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/routing_error.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/routing_error.erb @@ -15,6 +15,7 @@ <% end %> <%= render template: "rescues/_trace" %> +<% if @routes_inspector %> <h2> Routes </h2> @@ -23,6 +24,5 @@ Routes match in priority from top to bottom </p> -<%= render layout: "routes/route_wrapper" do %> - <%= render partial: "routes/route", collection: @routes %> + <%= @routes_inspector.format(ActionDispatch::DebugExceptions::TableRoutesFormatter.new(self)) %> <% end %> diff --git a/actionpack/lib/action_dispatch/routing/inspector.rb b/actionpack/lib/action_dispatch/routing/inspector.rb index 73b2b4ac1d..7ae1d2a0ee 100644 --- a/actionpack/lib/action_dispatch/routing/inspector.rb +++ b/actionpack/lib/action_dispatch/routing/inspector.rb @@ -61,21 +61,33 @@ module ActionDispatch ## # This class is just used for displaying route information when someone - # executes `rake routes`. People should not use this class. + # executes `rake routes` or looks at the RoutingError page. + # People should not use this class. class RoutesInspector # :nodoc: - def initialize - @engines = Hash.new + def initialize(routes) + @engines = {} + @routes = routes end - def format(all_routes, filter = nil) - if filter - all_routes = all_routes.select{ |route| route.defaults[:controller] == filter } + def format(formatter, filter = nil) + routes_to_display = filter_routes(filter) + + routes = collect_routes(routes_to_display) + formatter.section :application, 'Application routes', routes + + @engines.each do |name, routes| + formatter.section :engine, "Routes for #{name}", routes end - routes = collect_routes(all_routes) + formatter.result + end - formatted_routes(routes) + - formatted_routes_for_engines + def filter_routes(filter) + if filter + @routes.select { |route| route.defaults[:controller] == filter } + else + @routes + end end def collect_routes(routes) @@ -100,22 +112,32 @@ module ActionDispatch @engines[name] = collect_routes(routes.routes) end end + end - def formatted_routes_for_engines - @engines.map do |name, routes| - ["\nRoutes for #{name}:"] + formatted_routes(routes) - end.flatten + class ConsoleFormatter + def initialize + @buffer = [] end - def formatted_routes(routes) - name_width = routes.map{ |r| r[:name].length }.max - verb_width = routes.map{ |r| r[:verb].length }.max - path_width = routes.map{ |r| r[:path].length }.max + def result + @buffer.join("\n") + end - routes.map do |r| - "#{r[:name].rjust(name_width)} #{r[:verb].ljust(verb_width)} #{r[:path].ljust(path_width)} #{r[:reqs]}" - end + def section(type, title, routes) + @buffer << "\n#{title}:" unless type == :application + @buffer << draw_section(routes) end + + private + def draw_section(routes) + name_width = routes.map { |r| r[:name].length }.max + verb_width = routes.map { |r| r[:verb].length }.max + path_width = routes.map { |r| r[:path].length }.max + + routes.map do |r| + "#{r[:name].rjust(name_width)} #{r[:verb].ljust(verb_width)} #{r[:path].ljust(path_width)} #{r[:reqs]}" + end + end end end end diff --git a/actionpack/test/dispatch/debug_exceptions_test.rb b/actionpack/test/dispatch/debug_exceptions_test.rb index 39e791b4f4..1319eba9ac 100644 --- a/actionpack/test/dispatch/debug_exceptions_test.rb +++ b/actionpack/test/dispatch/debug_exceptions_test.rb @@ -45,8 +45,17 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest end end - ProductionApp = ActionDispatch::DebugExceptions.new(Boomer.new(false)) - DevelopmentApp = ActionDispatch::DebugExceptions.new(Boomer.new(true)) + def setup + app = ActiveSupport::OrderedOptions.new + app.config = ActiveSupport::OrderedOptions.new + app.config.assets = ActiveSupport::OrderedOptions.new + app.config.assets.prefix = '/sprockets' + Rails.stubs(:application).returns(app) + end + + RoutesApp = Struct.new(:routes).new(SharedTestRoutes) + ProductionApp = ActionDispatch::DebugExceptions.new(Boomer.new(false), RoutesApp) + DevelopmentApp = ActionDispatch::DebugExceptions.new(Boomer.new(true), RoutesApp) test 'skip diagnosis if not showing detailed exceptions' do @app = ProductionApp @@ -78,6 +87,15 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest assert boomer.closed, "Expected to close the response body" end + test 'displays routes in a table when a RoutingError occurs' do + @app = DevelopmentApp + get "/pass", {}, {'action_dispatch.show_exceptions' => true} + routing_table = body[/route_table.*<.table>/m] + assert_match '/:controller(/:action)(.:format)', routing_table + assert_match ':controller#:action', routing_table + assert_no_match '<|>', routing_table, "there should not be escaped html in the output" + end + test "rescue with diagnostics message" do @app = DevelopmentApp diff --git a/actionpack/test/dispatch/routing/inspector_test.rb b/actionpack/test/dispatch/routing/inspector_test.rb index c058bd4909..46e1cab30b 100644 --- a/actionpack/test/dispatch/routing/inspector_test.rb +++ b/actionpack/test/dispatch/routing/inspector_test.rb @@ -8,7 +8,6 @@ module ActionDispatch class RoutesInspectorTest < ActiveSupport::TestCase def setup @set = ActionDispatch::Routing::RouteSet.new - @inspector = ActionDispatch::Routing::RoutesInspector.new app = ActiveSupport::OrderedOptions.new app.config = ActiveSupport::OrderedOptions.new app.config.assets = ActiveSupport::OrderedOptions.new @@ -17,9 +16,10 @@ module ActionDispatch Rails.stubs(:env).returns("development") end - def draw(&block) + def draw(options = {}, &block) @set.draw(&block) - @inspector.format(@set.routes) + inspector = ActionDispatch::Routing::RoutesInspector.new(@set.routes) + inspector.format(ActionDispatch::Routing::ConsoleFormatter.new, options[:filter]).split("\n") end def test_displaying_routes_for_engines @@ -40,7 +40,8 @@ module ActionDispatch expected = [ "custom_assets GET /custom/assets(.:format) custom_assets#show", " blog /blog Blog::Engine", - "\nRoutes for Blog::Engine:", + "", + "Routes for Blog::Engine:", "cart GET /cart(.:format) cart#show" ] assert_equal expected, output @@ -165,6 +166,22 @@ module ActionDispatch assert_equal " bar GET /bar(.:format) redirect(307, path: /foo/bar)", output[1] assert_equal "foobar GET /foobar(.:format) redirect(301)", output[2] end + + def test_routes_can_be_filtered + output = draw(filter: 'posts') do + resources :articles + resources :posts + end + + assert_equal [" posts GET /posts(.:format) posts#index", + " POST /posts(.:format) posts#create", + " new_post GET /posts/new(.:format) posts#new", + "edit_post GET /posts/:id/edit(.:format) posts#edit", + " post GET /posts/:id(.:format) posts#show", + " PATCH /posts/:id(.:format) posts#update", + " PUT /posts/:id(.:format) posts#update", + " DELETE /posts/:id(.:format) posts#destroy"], output + end end end end |