mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
Change the behavior of route defaults
This commit changes route defaults so that explicit defaults are no longer required where the key is not part of the path. For example: resources :posts, bucket_type: 'posts' will be required whenever constructing the url from a hash such as a functional test or using url_for directly. However using the explicit form alters the behavior so it's not required: resources :projects, defaults: { bucket_type: 'projects' } This changes existing behavior slightly in that any routes which only differ in their defaults will match the first route rather than the closest match. Closes #8814
This commit is contained in:
parent
90d2802b71
commit
f1d8f2af72
7 changed files with 99 additions and 11 deletions
|
@ -1,5 +1,21 @@
|
||||||
## Rails 4.0.0 (unreleased) ##
|
## Rails 4.0.0 (unreleased) ##
|
||||||
|
|
||||||
|
* Change the behavior of route defaults so that explicit defaults are no longer
|
||||||
|
required where the key is not part of the path. For example:
|
||||||
|
|
||||||
|
resources :posts, bucket_type: 'posts'
|
||||||
|
|
||||||
|
will be required whenever constructing the url from a hash such as a functional
|
||||||
|
test or using url_for directly. However using the explicit form alters the
|
||||||
|
behavior so it's not required:
|
||||||
|
|
||||||
|
resources :projects, defaults: { bucket_type: 'projects' }
|
||||||
|
|
||||||
|
This changes existing behavior slightly in that any routes which only differ
|
||||||
|
in their defaults will match the first route rather than the closest match.
|
||||||
|
|
||||||
|
*Andrew White*
|
||||||
|
|
||||||
* Add support for routing constraints other than Regexp and String.
|
* Add support for routing constraints other than Regexp and String.
|
||||||
For example this now allows the use of arrays like this:
|
For example this now allows the use of arrays like this:
|
||||||
|
|
||||||
|
|
|
@ -45,7 +45,7 @@ module ActionDispatch
|
||||||
end
|
end
|
||||||
|
|
||||||
def required_keys
|
def required_keys
|
||||||
path.required_names.map { |x| x.to_sym } + required_defaults.keys
|
required_parts + required_defaults.keys
|
||||||
end
|
end
|
||||||
|
|
||||||
def score(constraints)
|
def score(constraints)
|
||||||
|
@ -79,10 +79,13 @@ module ActionDispatch
|
||||||
@required_parts ||= path.required_names.map { |n| n.to_sym }
|
@required_parts ||= path.required_names.map { |n| n.to_sym }
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def required_default?(key)
|
||||||
|
(constraints[:required_defaults] || []).include?(key)
|
||||||
|
end
|
||||||
|
|
||||||
def required_defaults
|
def required_defaults
|
||||||
@required_defaults ||= begin
|
@required_defaults ||= @defaults.dup.delete_if do |k,_|
|
||||||
matches = parts
|
parts.include?(k) || !required_default?(k)
|
||||||
@defaults.dup.delete_if { |k,_| matches.include?(k) }
|
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
|
@ -61,8 +61,8 @@ module ActionDispatch
|
||||||
normalize_path!
|
normalize_path!
|
||||||
normalize_options!
|
normalize_options!
|
||||||
normalize_requirements!
|
normalize_requirements!
|
||||||
normalize_defaults!
|
|
||||||
normalize_conditions!
|
normalize_conditions!
|
||||||
|
normalize_defaults!
|
||||||
end
|
end
|
||||||
|
|
||||||
def to_route
|
def to_route
|
||||||
|
@ -186,6 +186,13 @@ module ActionDispatch
|
||||||
@conditions[key] = condition
|
@conditions[key] = condition
|
||||||
end
|
end
|
||||||
|
|
||||||
|
@conditions[:required_defaults] = []
|
||||||
|
options.each do |key, required_default|
|
||||||
|
next if segment_keys.include?(key) || IGNORE_OPTIONS.include?(key)
|
||||||
|
next if Regexp === required_default
|
||||||
|
@conditions[:required_defaults] << key
|
||||||
|
end
|
||||||
|
|
||||||
via_all = options.delete(:via) if options[:via] == :all
|
via_all = options.delete(:via) if options[:via] == :all
|
||||||
|
|
||||||
if !via_all && options[:via].blank?
|
if !via_all && options[:via].blank?
|
||||||
|
|
|
@ -421,7 +421,7 @@ module ActionDispatch
|
||||||
end
|
end
|
||||||
|
|
||||||
conditions.keep_if do |k, _|
|
conditions.keep_if do |k, _|
|
||||||
k == :action || k == :controller ||
|
k == :action || k == :controller || k == :required_defaults ||
|
||||||
@request_class.public_method_defined?(k) || path_values.include?(k)
|
@request_class.public_method_defined?(k) || path_values.include?(k)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -931,3 +931,34 @@ class AnonymousControllerTest < ActionController::TestCase
|
||||||
assert_equal 'anonymous', @response.body
|
assert_equal 'anonymous', @response.body
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
class RoutingDefaultsTest < ActionController::TestCase
|
||||||
|
def setup
|
||||||
|
@controller = Class.new(ActionController::Base) do
|
||||||
|
def post
|
||||||
|
render :text => request.fullpath
|
||||||
|
end
|
||||||
|
|
||||||
|
def project
|
||||||
|
render :text => request.fullpath
|
||||||
|
end
|
||||||
|
end.new
|
||||||
|
|
||||||
|
@routes = ActionDispatch::Routing::RouteSet.new.tap do |r|
|
||||||
|
r.draw do
|
||||||
|
get '/posts/:id', :to => 'anonymous#post', :bucket_type => 'post'
|
||||||
|
get '/projects/:id', :to => 'anonymous#project', :defaults => { :bucket_type => 'project' }
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
def test_route_option_can_be_passed_via_process
|
||||||
|
get :post, :id => 1, :bucket_type => 'post'
|
||||||
|
assert_equal '/posts/1', @response.body
|
||||||
|
end
|
||||||
|
|
||||||
|
def test_route_default_is_not_required_for_building_request_uri
|
||||||
|
get :project, :id => 2
|
||||||
|
assert_equal '/projects/2', @response.body
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
|
@ -3300,3 +3300,31 @@ class TestPortConstraints < ActionDispatch::IntegrationTest
|
||||||
assert_response :success
|
assert_response :success
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
class TestRouteDefaults < ActionDispatch::IntegrationTest
|
||||||
|
stub_controllers do |routes|
|
||||||
|
Routes = routes
|
||||||
|
Routes.draw do
|
||||||
|
resources :posts, bucket_type: 'post'
|
||||||
|
resources :projects, defaults: { bucket_type: 'project' }
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
def app
|
||||||
|
Routes
|
||||||
|
end
|
||||||
|
|
||||||
|
include Routes.url_helpers
|
||||||
|
|
||||||
|
def test_route_options_are_required_for_url_for
|
||||||
|
assert_raises(ActionController::UrlGenerationError) do
|
||||||
|
assert_equal '/posts/1', url_for(controller: 'posts', action: 'show', id: 1, only_path: true)
|
||||||
|
end
|
||||||
|
|
||||||
|
assert_equal '/posts/1', url_for(controller: 'posts', action: 'show', id: 1, bucket_type: 'post', only_path: true)
|
||||||
|
end
|
||||||
|
|
||||||
|
def test_route_defaults_are_not_required_for_url_for
|
||||||
|
assert_equal '/projects/1', url_for(controller: 'projects', action: 'show', id: 1, only_path: true)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
|
@ -6,18 +6,18 @@ module ActionDispatch
|
||||||
def test_initialize
|
def test_initialize
|
||||||
app = Object.new
|
app = Object.new
|
||||||
path = Path::Pattern.new '/:controller(/:action(/:id(.:format)))'
|
path = Path::Pattern.new '/:controller(/:action(/:id(.:format)))'
|
||||||
defaults = Object.new
|
defaults = {}
|
||||||
route = Route.new("name", app, path, {}, defaults)
|
route = Route.new("name", app, path, {}, defaults)
|
||||||
|
|
||||||
assert_equal app, route.app
|
assert_equal app, route.app
|
||||||
assert_equal path, route.path
|
assert_equal path, route.path
|
||||||
assert_equal defaults, route.defaults
|
assert_same defaults, route.defaults
|
||||||
end
|
end
|
||||||
|
|
||||||
def test_route_adds_itself_as_memo
|
def test_route_adds_itself_as_memo
|
||||||
app = Object.new
|
app = Object.new
|
||||||
path = Path::Pattern.new '/:controller(/:action(/:id(.:format)))'
|
path = Path::Pattern.new '/:controller(/:action(/:id(.:format)))'
|
||||||
defaults = Object.new
|
defaults = {}
|
||||||
route = Route.new("name", app, path, {}, defaults)
|
route = Route.new("name", app, path, {}, defaults)
|
||||||
|
|
||||||
route.ast.grep(Nodes::Terminal).each do |node|
|
route.ast.grep(Nodes::Terminal).each do |node|
|
||||||
|
@ -82,11 +82,14 @@ module ActionDispatch
|
||||||
end
|
end
|
||||||
|
|
||||||
def test_score
|
def test_score
|
||||||
|
constraints = {:required_defaults => [:controller, :action]}
|
||||||
|
defaults = {:controller=>"pages", :action=>"show"}
|
||||||
|
|
||||||
path = Path::Pattern.new "/page/:id(/:action)(.:format)"
|
path = Path::Pattern.new "/page/:id(/:action)(.:format)"
|
||||||
specific = Route.new "name", nil, path, {}, {:controller=>"pages", :action=>"show"}
|
specific = Route.new "name", nil, path, constraints, defaults
|
||||||
|
|
||||||
path = Path::Pattern.new "/:controller(/:action(/:id))(.:format)"
|
path = Path::Pattern.new "/:controller(/:action(/:id))(.:format)"
|
||||||
generic = Route.new "name", nil, path, {}
|
generic = Route.new "name", nil, path, constraints
|
||||||
|
|
||||||
knowledge = {:id=>20, :controller=>"pages", :action=>"show"}
|
knowledge = {:id=>20, :controller=>"pages", :action=>"show"}
|
||||||
|
|
||||||
|
|
Loading…
Reference in a new issue