Permalink
Browse files

Merge pull request #8658 from senny/8649_unify_routes_output_for_cons…

…ole_and_web

Unify routes output for console and web, ensure engine routes are shown in html routes table.

Closes #8649.
  • Loading branch information...
2 parents 1e1c7af + 80795e0 commit a6cec6e8ed7cc158fa1bc3cc59a5c3fa392b2afe @carlosantoniodasilva carlosantoniodasilva committed Jan 5, 2013
View
4 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.:
View
27 actionpack/lib/action_dispatch/middleware/debug_exceptions.rb
@@ -2,7 +2,6 @@
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 @@ def render_exception(env, exception)
: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 @@ def stderr_logger
@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
View
4 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 %>
View
62 actionpack/lib/action_dispatch/routing/inspector.rb
@@ -61,21 +61,33 @@ def engine?
##
# 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 @@ def collect_engine_routes(route)
@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
View
22 actionpack/test/dispatch/debug_exceptions_test.rb
@@ -45,8 +45,17 @@ def call(env)
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 @@ def call(env)
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 '&lt;|&gt;', routing_table, "there should not be escaped html in the output"
+ end
+
test "rescue with diagnostics message" do
@app = DevelopmentApp
View
25 actionpack/test/dispatch/routing/inspector_test.rb
@@ -8,7 +8,6 @@ module Routing
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 @@ def setup
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 @@ def self.inspect
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 @@ def test_redirect
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
View
4 railties/lib/rails/tasks/routes.rake
@@ -2,6 +2,6 @@ desc 'Print out all defined routes in match order, with names. Target specific c
task routes: :environment do
all_routes = Rails.application.routes.routes
require 'action_dispatch/routing/inspector'
- inspector = ActionDispatch::Routing::RoutesInspector.new
- puts inspector.format(all_routes, ENV['CONTROLLER']).join "\n"
+ inspector = ActionDispatch::Routing::RoutesInspector.new(all_routes)
+ puts inspector.format(ActionDispatch::Routing::ConsoleFormatter.new, ENV['CONTROLLER'])
end

0 comments on commit a6cec6e

Please sign in to comment.