github/rails
1
1
Fork 0
mirror of https://github.com/github/rails.git synced 2026-04-26 03:00:59 -04:00
Code Issues Packages Projects Releases Wiki Activity

Finally fix the bug where symbols and strings were not having the same behavior in the router.

Browse Source 
If you were using symbols before for methods like match/get/post/put/delete, it is likely that this commit will break your routes.
Everything should behave the same if you are using strings, if not, please open up a ticket.
This commit is contained in:
José Valim
2010-08-24 16:42:14 -03:00
parent 4a90ecb3ad
commit e197d6f34b
2 changed files with 85 additions and 74 deletions
Download Patch File Download Diff File Expand all files Collapse all files

115
actionpack/lib/action_dispatch/routing/mapper.rb
View File

@@ -44,7 +44,8 @@ module ActionDispatch
IGNORE_OPTIONS = [:to, :as, :via, :on, :constraints, :defaults, :only, :except, :anchor, :shallow, :shallow_path, :shallow_prefix]
def initialize(set, scope, path, options)
@set, @scope, @options = set, scope, options
@set, @scope = set, scope
@options = (@scope[:options] || {}).merge(options)
@path = normalize_path(path)
normalize_options!
end
@@ -57,18 +58,11 @@ module ActionDispatch
def normalize_options!
path_without_format = @path.sub(/\(\.:format\)$/, '')
@options = (@scope[:options] || {}).merge(@options)
if @scope[:as] && !@options[:as].blank?
@options[:as] = "#{@scope[:as]}_#{@options[:as]}"
elsif @scope[:as] && @options[:as] == ""
@options[:as] = @scope[:as].to_s
end
if using_match_shorthand?(path_without_format, @options)
to_shorthand = @options[:to].blank?
@options[:to] ||= path_without_format[1..-1].sub(%r{/([^/]*)$}, '#\1')
@options[:as] ||= path_without_format[1..-1].gsub("/", "_")
@options[:as] ||= Mapper.normalize_name(path_without_format)
end
@options.merge!(default_controller_and_action(to_shorthand))
@@ -80,8 +74,8 @@ module ActionDispatch
end
def normalize_path(path)
raise ArgumentError, "path is required" if @scope[:path].blank? && path.blank?
path = Mapper.normalize_path("#{@scope[:path]}/#{path}")
raise ArgumentError, "path is required" if path.blank?
path = Mapper.normalize_path(path)
if path.match(':controller')
raise ArgumentError, ":controller segment is not allowed within a namespace block" if @scope[:module]
@@ -93,7 +87,11 @@ module ActionDispatch
@options.reverse_merge!(:controller => /.+?/)
end
path
if path.include?(":format")
path
else
"#{path}(.:format)"
end
end
def app
@@ -216,6 +214,10 @@ module ActionDispatch
path
end
def self.normalize_name(name)
normalize_path(name)[1..-1].gsub("/", "_")
end
module Base
def initialize(set) #:nodoc:
@set = set
@@ -324,13 +326,7 @@ module ActionDispatch
ActiveSupport::Deprecation.warn ":name_prefix was deprecated in the new router syntax. Use :as instead.", caller
end
case args.first
when String
options[:path] = args.first
when Symbol
options[:controller] = args.first
end
options[:path] = args.first if args.first.is_a?(String)
recover = {}
options[:constraints] ||= {}
@@ -362,8 +358,9 @@ module ActionDispatch
@scope[:blocks] = recover[:block]
end
def controller(controller)
scope(controller.to_sym) { yield }
def controller(controller, options={})
options[:controller] = controller
scope(options) { yield }
end
def namespace(path, options = {})
@@ -444,9 +441,9 @@ module ActionDispatch
module Resources
# CANONICAL_ACTIONS holds all actions that does not need a prefix or
# a path appended since they fit properly in their scope level.
VALID_ON_OPTIONS = [:new, :collection, :member]
CANONICAL_ACTIONS = [:index, :create, :new, :show, :update, :destroy]
RESOURCE_OPTIONS = [:as, :controller, :path, :only, :except]
VALID_ON_OPTIONS = [:new, :collection, :member]
RESOURCE_OPTIONS = [:as, :controller, :path, :only, :except]
CANONICAL_ACTIONS = %w(index create new show update destroy)
class Resource #:nodoc:
DEFAULT_ACTIONS = [:index, :create, :new, :show, :update, :destroy, :edit]
@@ -700,41 +697,27 @@ module ActionDispatch
return member { match(*args) }
end
path = options.delete(:path)
action = args.first
if action.is_a?(Symbol) || (resource_method_scope? && action.to_s =~ /^[A-Za-z_]\w*$/)
path = path_for_action(action, path)
options[:action] ||= action
options[:as] = name_for_action(action, options[:as])
with_exclusive_scope do
return super(path, options)
end
elsif resource_method_scope?
path = path_for_custom_action
options[:as] = name_for_action(options[:as]) if options[:as]
args.push(options)
with_exclusive_scope do
scope(path) do
return super
end
end
end
if resource_scope?
raise ArgumentError, "can't define route directly in resource(s) scope"
end
args.push(options)
super
action = args.first
path = path_for_action(action, options.delete(:path))
if action.to_s =~ /^[\w\/]+$/
options[:action] ||= action unless action.to_s.include?("/")
options[:as] = name_for_action(action, options[:as])
else
options[:as] = name_for_action(options[:as])
end
super(path, options)
end
def root(options={})
if @scope[:scope_level] == :resources
with_scope_level(:nested) do
scope(parent_resource.path, :as => parent_resource.collection_name) do
with_scope_level(:root) do
scope(parent_resource.path) do
super(options)
end
end
@@ -871,7 +854,7 @@ module ActionDispatch
end
def canonical_action?(action, flag)
flag && CANONICAL_ACTIONS.include?(action)
flag && resource_method_scope? && CANONICAL_ACTIONS.include?(action.to_s)
end
def shallow_scoping?
@@ -882,18 +865,10 @@ module ActionDispatch
prefix = shallow_scoping? ?
"#{@scope[:shallow_path]}/#{parent_resource.path}/:id" : @scope[:path]
if canonical_action?(action, path.blank?)
"#{prefix}(.:format)"
path = if canonical_action?(action, path.blank?)
prefix.to_s
else
"#{prefix}/#{action_path(action, path)}(.:format)"
end
end
def path_for_custom_action
if shallow_scoping?
"#{@scope[:shallow_path]}/#{parent_resource.path}/:id"
else
@scope[:path]
"#{prefix}/#{action_path(action, path)}"
end
end
@@ -913,6 +888,7 @@ module ActionDispatch
def name_for_action(action, as=nil)
prefix = prefix_name_for_action(action, as)
prefix = Mapper.normalize_name(prefix) if prefix
name_prefix = @scope[:as]
if parent_resource
@@ -922,15 +898,18 @@ module ActionDispatch
name = case @scope[:scope_level]
when :collection
[name_prefix, collection_name]
[prefix, name_prefix, collection_name]
when :new
[:new, name_prefix, member_name]
[prefix, :new, name_prefix, member_name]
when :member
[prefix, shallow_scoping? ? @scope[:shallow_prefix] : name_prefix, member_name]
when :root
[name_prefix, collection_name, prefix]
else
[shallow_scoping? ? @scope[:shallow_prefix] : name_prefix, member_name]
[name_prefix, member_name, prefix]
end
name.unshift(prefix)
name.select(&:present?).join("_")
name.select(&:present?).join("_").presence
end
end

44
actionpack/test/dispatch/routing_test.rb
View File

@@ -40,6 +40,13 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest
get :new, :path => "build"
post :create, :path => "create", :as => ""
put :update
get :remove, :action => :destroy, :as => :remove
end
scope "pagemark", :controller => "pagemarks", :as => :pagemark do
get "new", :path => "build"
post "create", :as => ""
put "update"
get "remove", :action => :destroy, :as => :remove
end
@@ -203,11 +210,12 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest
end
resources :customers do
get "recent" => "customers#recent", :on => :collection
get "profile" => "customers#profile", :on => :member
get :recent, :on => :collection
get "profile", :on => :member
get "secret/profile" => "customers#secret", :on => :member
post "preview" => "customers#preview", :as => :another_preview, :on => :new
resource :avatar do
get "thumbnail(.:format)" => "avatars#thumbnail", :as => :thumbnail, :on => :member
get "thumbnail" => "avatars#thumbnail", :as => :thumbnail, :on => :member
end
resources :invoices do
get "outstanding" => "invoices#outstanding", :as => :outstanding, :on => :collection
@@ -648,15 +656,15 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest
with_test_routes do
get '/bookmark/build'
assert_equal 'bookmarks#new', @response.body
assert_equal '/bookmark/build', new_bookmark_path
assert_equal '/bookmark/build', bookmark_new_path
post '/bookmark/create'
assert_equal 'bookmarks#create', @response.body
assert_equal '/bookmark/create', bookmark_path
put '/bookmark'
put '/bookmark/update'
assert_equal 'bookmarks#update', @response.body
assert_equal '/bookmark', update_bookmark_path
assert_equal '/bookmark/update', bookmark_update_path
get '/bookmark/remove'
assert_equal 'bookmarks#destroy', @response.body
@@ -664,6 +672,26 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest
end
end
def test_pagemarks
with_test_routes do
get '/pagemark/build'
assert_equal 'pagemarks#new', @response.body
assert_equal '/pagemark/build', pagemark_new_path
post '/pagemark/create'
assert_equal 'pagemarks#create', @response.body
assert_equal '/pagemark/create', pagemark_path
put '/pagemark/update'
assert_equal 'pagemarks#update', @response.body
assert_equal '/pagemark/update', pagemark_update_path
get '/pagemark/remove'
assert_equal 'pagemarks#destroy', @response.body
assert_equal '/pagemark/remove', pagemark_remove_path
end
end
def test_admin
with_test_routes do
get '/admin', {}, {'REMOTE_ADDR' => '192.168.1.100'}
@@ -1564,6 +1592,7 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest
with_test_routes do
assert_equal '/customers/recent', recent_customers_path
assert_equal '/customers/1/profile', profile_customer_path(:id => '1')
assert_equal '/customers/1/secret/profile', secret_profile_customer_path(:id => '1')
assert_equal '/customers/new/preview', another_preview_new_customer_path
assert_equal '/customers/1/avatar/thumbnail.jpg', thumbnail_customer_avatar_path(:customer_id => '1', :format => :jpg)
assert_equal '/customers/1/invoices/outstanding', outstanding_customer_invoices_path(:customer_id => '1')
@@ -1577,6 +1606,9 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest
get '/customers/1/invoices/overdue'
assert_equal 'invoices#overdue', @response.body
get '/customers/1/secret/profile'
assert_equal 'customers#secret', @response.body
end
end