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.

7 files changed, 111 insertions, 37 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 '&lt;|&gt;', 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
diff --git a/railties/lib/rails/tasks/routes.rake b/railties/lib/rails/tasks/routes.rake
index 676b475640..1815c2fdc7 100644
--- a/railties/lib/rails/tasks/routes.rake
+++ b/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