Skip to content
Permalink
Browse files

Mounted Rack apps should have default named routes based on app name

This fixes a regression in 4.2.0 from 4.1.8.

#17823 fixed a similar regression regarding _explicitly_ named routes for a mounted Rack app, but there was another regression for the default value.

With a route like:

    Rails.application.routes.draw do
      mount Mountable::Web, at: 'some_route'
    end

The "Prefix" column of rake routes gives the following:

- 4.1.8:         mountable_web
- 4.2.0.beta1-4: [nothing]
- 4.2.0.rc1:     [nothing]
- 4.2.0.rc2:     some_route   <- regression

This fixes the default to go back to being based off the name of the class like the docs specify: https://github.com/rails/rails/blob/785d04e3109f69d0b9b9f4732179592f0ef04e52/actionpack/lib/action_dispatch/routing/mapper.rb#L558-L560

Explicitly named routes still work correctly per #17823:

    Rails.application.routes.draw do
      mount Mountable::Web, at: 'some_route', as: 'named'
    end

- 4.1.8:         named
- 4.2.0.beta1-4: [nothing]
- 4.2.0.rc1:     [nothing]
- 4.2.0.rc2:     named
  • Loading branch information
tjschuck committed Dec 6, 2014
1 parent 785d04e commit ee65f48c2666a660cc48496c8bc9f63113a41e44
Showing with 43 additions and 14 deletions.
  1. +10 −3 actionpack/lib/action_dispatch/routing/mapper.rb
  2. +33 −11 actionpack/test/dispatch/routing/inspector_test.rb
@@ -579,15 +579,14 @@ def mount(app, options = nil)

raise "A rack application must be specified" unless path

rails_app = rails_app? app
options[:as] ||= app.railtie_name if rails_app
options[:as] ||= app_name(app)

target_as = name_for_action(options[:as], path)
options[:via] ||= :all

match(path, options.merge(:to => app, :anchor => false, :format => false))

define_generate_prefix(app, target_as) if rails_app
define_generate_prefix(app, target_as) if rails_app?(app)
self
end

@@ -612,6 +611,14 @@ def rails_app?(app)
app.is_a?(Class) && app < Rails::Railtie
end

def app_name(app)
if rails_app?(app)
app.railtie_name
elsif class_name = app.try(:name)
ActiveSupport::Inflector.underscore(class_name).tr("/", "_")
end
end

def define_generate_prefix(app, name)
_route = @set.named_routes.get name
_routes = @set
@@ -2,6 +2,11 @@
require 'rails/engine'
require 'action_dispatch/routing/inspector'

class MountedRackApp
def self.call(env)
end
end

module ActionDispatch
module Routing
class RoutesInspectorTest < ActiveSupport::TestCase
@@ -204,19 +209,36 @@ def test_rake_routes_shows_routes_with_dashes
], output
end

class RackApp
def self.call(env)
def test_rake_routes_shows_route_with_rack_app
output = draw do
get 'foo/:id' => MountedRackApp, :id => /[A-Z]\d{5}/
end

assert_equal [
"Prefix Verb URI Pattern Controller#Action",
" GET /foo/:id(.:format) MountedRackApp {:id=>/[A-Z]\\d{5}/}"
], output
end

def test_rake_routes_shows_route_with_rack_app
def test_rake_routes_shows_named_route_with_mounted_rack_app
output = draw do
get 'foo/:id' => RackApp, :id => /[A-Z]\d{5}/
mount MountedRackApp => '/foo'
end

assert_equal [
"Prefix Verb URI Pattern Controller#Action",
" GET /foo/:id(.:format) #{RackApp.name} {:id=>/[A-Z]\\d{5}/}"
" Prefix Verb URI Pattern Controller#Action",
"mounted_rack_app /foo MountedRackApp"
], output
end

def test_rake_routes_shows_overridden_named_route_with_mounted_rack_app_with_name
output = draw do
mount MountedRackApp => '/foo', as: 'blog'
end

assert_equal [
"Prefix Verb URI Pattern Controller#Action",
" blog /foo MountedRackApp"
], output
end

@@ -229,21 +251,21 @@ def inspect

output = draw do
scope :constraint => constraint.new do
mount RackApp => '/foo'
mount MountedRackApp => '/foo'
end
end

assert_equal [
"Prefix Verb URI Pattern Controller#Action",
" foo /foo #{RackApp.name} {:constraint=>( my custom constraint )}"
" Prefix Verb URI Pattern Controller#Action",
"mounted_rack_app /foo MountedRackApp {:constraint=>( my custom constraint )}"
], output
end

def test_rake_routes_dont_show_app_mounted_in_assets_prefix
output = draw do
get '/sprockets' => RackApp
get '/sprockets' => MountedRackApp
end
assert_no_match(/RackApp/, output.first)
assert_no_match(/MountedRackApp/, output.first)
assert_no_match(/\/sprockets/, output.first)
end

0 comments on commit ee65f48

Please sign in to comment.
You can’t perform that action at this time.