kotovalexarian-likes-github/rails--rails
1
0
Fork 0
mirror of https://github.com/rails/rails.git synced 2022-11-09 12:12:34 -05:00
Code Releases Activity

extract required_defaults from the conditions hash before constructing the route

Browse source 
this way we can remove the strange "respond_to?" conditional in the
`matches?` loop
This commit is contained in:
Aaron Patterson 2015-06-08 17:18:06 -07:00
parent ec69a45f5b
commit 8037d7eacd
6 changed files with 56 additions and 56 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

7
actionpack/lib/action_dispatch/journey/route.rb
View file

@ -11,7 +11,7 @@ module ActionDispatch
##
# +path+ is a path constraint.
# +constraints+ is a hash of constraints to be applied to this route.
def initialize(name, app, path, constraints, defaults = {})
def initialize(name, app, path, constraints, required_defaults, defaults)
@name = name
@app = app
@path = path
@ -19,6 +19,7 @@ module ActionDispatch
@constraints = constraints
@defaults = defaults
@required_defaults = nil
@_required_defaults = required_defaults || []
@required_parts = nil
@parts = nil
@decorated_ast = nil
@ -73,7 +74,7 @@ module ActionDispatch
end
def required_default?(key)
(constraints[:required_defaults] || []).include?(key)
@_required_defaults.include?(key)
end
def required_defaults
@ -92,8 +93,6 @@ module ActionDispatch
def matches?(request)
constraints.all? do |method, value|
next true unless request.respond_to?(method)
case value
when Regexp, String
value === request.send(method).to_s

4
actionpack/lib/action_dispatch/journey/routes.rb
View file

@ -63,8 +63,8 @@ module ActionDispatch
end
# Add a route to the routing table.
def add_route(app, path, conditions, defaults, name = nil)
route = Route.new(name, app, path, conditions, defaults)
def add_route(app, path, conditions, required_defaults, defaults, name = nil)
route = Route.new(name, app, path, conditions, required_defaults, defaults)
route.precedence = routes.length
routes << route

5
actionpack/lib/action_dispatch/routing/route_set.rb
View file

@ -511,10 +511,11 @@ module ActionDispatch
path = conditions.delete :path_info
ast = conditions.delete :parsed_path_info
required_defaults = conditions.delete :required_defaults
path = build_path(path, ast, requirements, anchor)
conditions = build_conditions(conditions, path.names.map(&:to_sym))
route = @set.add_route(app, path, conditions, defaults, name)
route = @set.add_route(app, path, conditions, required_defaults, defaults, name)
named_routes[name] = route if name
route
end
@ -563,7 +564,7 @@ module ActionDispatch
end
conditions.keep_if do |k, _|
k == :action || k == :controller || k == :required_defaults ||
k == :action || k == :controller ||
request_class.public_method_defined?(k) || path_values.include?(k)
end
end

26
actionpack/test/journey/route_test.rb
View file

@ -7,7 +7,7 @@ module ActionDispatch
app = Object.new
path = Path::Pattern.from_string '/:controller(/:action(/:id(.:format)))'
defaults = {}
route = Route.new("name", app, path, {}, defaults)
route = Route.new("name", app, path, {}, [], defaults)
assert_equal app, route.app
assert_equal path, route.path
@ -18,7 +18,7 @@ module ActionDispatch
app = Object.new
path = Path::Pattern.from_string '/:controller(/:action(/:id(.:format)))'
defaults = {}
route = Route.new("name", app, path, {}, defaults)
route = Route.new("name", app, path, {}, [], defaults)
route.ast.grep(Nodes::Terminal).each do |node|
assert_equal route, node.memo
@ -29,27 +29,27 @@ module ActionDispatch
strexp = Router::Strexp.build(':name', { name: /love/ }, ['/'])
path = Path::Pattern.new strexp
defaults = { name: 'tender' }
route = Route.new('name', nil, path, nil, defaults)
route = Route.new('name', nil, path, nil, [], defaults)
assert_equal(/love/, route.requirements[:name])
end
def test_ip_address
path = Path::Pattern.from_string '/messages/:id(.:format)'
route = Route.new("name", nil, path, {:ip => '192.168.1.1'},
route = Route.new("name", nil, path, {:ip => '192.168.1.1'}, [],
{ :controller => 'foo', :action => 'bar' })
assert_equal '192.168.1.1', route.ip
end
def test_default_ip
path = Path::Pattern.from_string '/messages/:id(.:format)'
route = Route.new("name", nil, path, {},
route = Route.new("name", nil, path, {}, [],
{ :controller => 'foo', :action => 'bar' })
assert_equal(//, route.ip)
end
def test_format_with_star
path = Path::Pattern.from_string '/:controller/*extra'
route = Route.new("name", nil, path, {},
route = Route.new("name", nil, path, {}, [],
{ :controller => 'foo', :action => 'bar' })
assert_equal '/foo/himom', route.format({
:controller => 'foo',
@ -59,7 +59,7 @@ module ActionDispatch
def test_connects_all_match
path = Path::Pattern.from_string '/:controller(/:action(/:id(.:format)))'
route = Route.new("name", nil, path, {:action => 'bar'}, { :controller => 'foo' })
route = Route.new("name", nil, path, {:action => 'bar'}, [], { :controller => 'foo' })
assert_equal '/foo/bar/10', route.format({
:controller => 'foo',
@ -70,34 +70,34 @@ module ActionDispatch
def test_extras_are_not_included_if_optional
path = Path::Pattern.from_string '/page/:id(/:action)'
route = Route.new("name", nil, path, { }, { :action => 'show' })
route = Route.new("name", nil, path, { }, [], { :action => 'show' })
assert_equal '/page/10', route.format({ :id => 10 })
end
def test_extras_are_not_included_if_optional_with_parameter
path = Path::Pattern.from_string '(/sections/:section)/pages/:id'
route = Route.new("name", nil, path, { }, { :action => 'show' })
route = Route.new("name", nil, path, { }, [], { :action => 'show' })
assert_equal '/pages/10', route.format({:id => 10})
end
def test_extras_are_not_included_if_optional_parameter_is_nil
path = Path::Pattern.from_string '(/sections/:section)/pages/:id'
route = Route.new("name", nil, path, { }, { :action => 'show' })
route = Route.new("name", nil, path, { }, [], { :action => 'show' })
assert_equal '/pages/10', route.format({:id => 10, :section => nil})
end
def test_score
constraints = {:required_defaults => [:controller, :action]}
constraints = {}
defaults = {:controller=>"pages", :action=>"show"}
path = Path::Pattern.from_string "/page/:id(/:action)(.:format)"
specific = Route.new "name", nil, path, constraints, defaults
specific = Route.new "name", nil, path, constraints, [:controller, :action], defaults
path = Path::Pattern.from_string "/:controller(/:action(/:id))(.:format)"
generic = Route.new "name", nil, path, constraints
generic = Route.new "name", nil, path, constraints, [], {}
knowledge = {:id=>20, :controller=>"pages", :action=>"show"}

52
actionpack/test/journey/router_test.rb
View file

@ -35,7 +35,7 @@ module ActionDispatch
exp = Router::Strexp.build '/foo-bar-baz', {}, ['/.?']
path = Path::Pattern.new exp
routes.add_route nil, path, {}, {:id => nil}, {}
routes.add_route nil, path, {}, [], {:id => nil}, {}
env = rails_env 'PATH_INFO' => '/foo-bar-baz'
called = false
@ -52,7 +52,7 @@ module ActionDispatch
exp = Router::Strexp.build '/%E3%81%BB%E3%81%92', {}, ['/.?']
path = Path::Pattern.new exp
routes.add_route nil, path, {}, {:id => nil}, {}
routes.add_route nil, path, {}, [], {:id => nil}, {}
env = rails_env 'PATH_INFO' => '/%E3%81%BB%E3%81%92'
called = false
@ -71,7 +71,7 @@ module ActionDispatch
exp = Router::Strexp.build '/foo(/:id)', {}, ['/.?']
path = Path::Pattern.new exp
routes.add_route nil, path, requirements, {:id => nil}, {}
routes.add_route nil, path, requirements, [], {:id => nil}, {}
env = rails_env({'PATH_INFO' => '/foo/10'}, klass)
router.recognize(env) do |r, params|
@ -91,7 +91,7 @@ module ActionDispatch
exp = Router::Strexp.build '/foo(/:id)', {}, ['/.?']
path = Path::Pattern.new exp
router.routes.add_route nil, path, requirements, {:id => nil}, {}
router.routes.add_route nil, path, requirements, [], {:id => nil}, {}
env = rails_env({'PATH_INFO' => '/foo/10'}, klass)
router.recognize(env) do |r, params|
@ -118,7 +118,7 @@ module ActionDispatch
exp = Router::Strexp.build '/bar', {}, ['/.?']
path = Path::Pattern.new exp
routes.add_route nil, path, {}, {}, {}
routes.add_route nil, path, {}, [], {}, {}
env = rails_env({'PATH_INFO' => '/foo',
'custom.path_info' => '/bar'}, CustomPathRequest)
@ -192,7 +192,7 @@ module ActionDispatch
route_name = "gorby_thunderhorse"
pattern = Router::Strexp.build("/foo/:id", { :id => /\d+/ }, ['/', '.', '?'], false)
path = Path::Pattern.new pattern
@router.routes.add_route nil, path, {}, {}, route_name
@router.routes.add_route nil, path, {}, [], {}, route_name
error = assert_raises(ActionController::UrlGenerationError) do
@formatter.generate(route_name, { }, { })
@ -234,7 +234,7 @@ module ActionDispatch
def test_defaults_merge_correctly
path = Path::Pattern.from_string '/foo(/:id)'
@router.routes.add_route nil, path, {}, {:id => nil}, {}
@router.routes.add_route nil, path, {}, [], {:id => nil}, {}
env = rails_env 'PATH_INFO' => '/foo/10'
@router.recognize(env) do |r, params|
@ -317,7 +317,7 @@ module ActionDispatch
def test_nil_path_parts_are_ignored
path = Path::Pattern.from_string "/:controller(/:action(.:format))"
@router.routes.add_route @app, path, {}, {}, {}
@router.routes.add_route @app, path, {}, [], {}, {}
params = { :controller => "tasks", :format => nil }
extras = { :action => 'lol' }
@ -332,7 +332,7 @@ module ActionDispatch
str = Router::Strexp.build("/", Hash[params], ['/', '.', '?'], true)
path = Path::Pattern.new str
@router.routes.add_route @app, path, {}, {}, {}
@router.routes.add_route @app, path, {}, [], {}, {}
path, _ = @formatter.generate(nil, Hash[params], {})
assert_equal '/', path
@ -340,7 +340,7 @@ module ActionDispatch
def test_generate_calls_param_proc
path = Path::Pattern.from_string '/:controller(/:action)'
@router.routes.add_route @app, path, {}, {}, {}
@router.routes.add_route @app, path, {}, [], {}, {}
parameterized = []
params = [ [:controller, "tasks"],
@ -357,7 +357,7 @@ module ActionDispatch
def test_generate_id
path = Path::Pattern.from_string '/:controller(/:action)'
@router.routes.add_route @app, path, {}, {}, {}
@router.routes.add_route @app, path, {}, [], {}, {}
path, params = @formatter.generate(
nil, {:id=>1, :controller=>"tasks", :action=>"show"}, {})
@ -367,7 +367,7 @@ module ActionDispatch
def test_generate_escapes
path = Path::Pattern.from_string '/:controller(/:action)'
@router.routes.add_route @app, path, {}, {}, {}
@router.routes.add_route @app, path, {}, [], {}, {}
path, _ = @formatter.generate(nil,
{ :controller => "tasks",
@ -378,7 +378,7 @@ module ActionDispatch
def test_generate_escapes_with_namespaced_controller
path = Path::Pattern.from_string '/:controller(/:action)'
@router.routes.add_route @app, path, {}, {}, {}
@router.routes.add_route @app, path, {}, [], {}, {}
path, _ = @formatter.generate(
nil, { :controller => "admin/tasks",
@ -389,7 +389,7 @@ module ActionDispatch
def test_generate_extra_params
path = Path::Pattern.from_string '/:controller(/:action)'
@router.routes.add_route @app, path, {}, {}, {}
@router.routes.add_route @app, path, {}, [], {}, {}
path, params = @formatter.generate(
nil, { :id => 1,
@ -403,7 +403,7 @@ module ActionDispatch
def test_generate_missing_keys_no_matches_different_format_keys
path = Path::Pattern.from_string '/:controller/:action/:name'
@router.routes.add_route @app, path, {}, {}, {}
@router.routes.add_route @app, path, {}, [], {}, {}
primarty_parameters = {
:id => 1,
:controller => "tasks",
@ -430,7 +430,7 @@ module ActionDispatch
def test_generate_uses_recall_if_needed
path = Path::Pattern.from_string '/:controller(/:action(/:id))'
@router.routes.add_route @app, path, {}, {}, {}
@router.routes.add_route @app, path, {}, [], {}, {}
path, params = @formatter.generate(
nil,
@ -442,7 +442,7 @@ module ActionDispatch
def test_generate_with_name
path = Path::Pattern.from_string '/:controller(/:action)'
@router.routes.add_route @app, path, {}, {}, "tasks"
@router.routes.add_route @app, path, {}, [], {}, "tasks"
path, params = @formatter.generate(
"tasks",
@ -460,7 +460,7 @@ module ActionDispatch
define_method("test_recognize_#{expected.keys.map(&:to_s).join('_')}") do
path = Path::Pattern.from_string "/:controller(/:action(/:id))"
app = Object.new
route = @router.routes.add_route(app, path, {}, {}, {})
route = @router.routes.add_route(app, path, {}, [], {}, {})
env = rails_env 'PATH_INFO' => request_path
called = false
@ -482,7 +482,7 @@ module ActionDispatch
define_method("test_recognize_#{name}") do
path = Path::Pattern.from_string '/:segment/*splat'
app = Object.new
route = @router.routes.add_route(app, path, {}, {}, {})
route = @router.routes.add_route(app, path, {}, [], {}, {})
env = rails_env 'PATH_INFO' => request_path
called = false
@ -505,7 +505,7 @@ module ActionDispatch
)
path = Path::Pattern.new strexp
app = Object.new
route = @router.routes.add_route(app, path, {}, {}, {})
route = @router.routes.add_route(app, path, {}, [], {}, {})
env = rails_env 'PATH_INFO' => '/admin/users/show/10'
called = false
@ -526,7 +526,7 @@ module ActionDispatch
def test_recognize_literal
path = Path::Pattern.from_string "/books(/:action(.:format))"
app = Object.new
route = @router.routes.add_route(app, path, {}, {:controller => 'books'})
route = @router.routes.add_route(app, path, {}, [], {:controller => 'books'})
env = rails_env 'PATH_INFO' => '/books/list.rss'
expected = { :controller => 'books', :action => 'list', :format => 'rss' }
@ -544,7 +544,7 @@ module ActionDispatch
path = Path::Pattern.from_string "/books(/:action(.:format))"
app = Object.new
conditions = { request_method: 'HEAD' }
@router.routes.add_route(app, path, conditions, {})
@router.routes.add_route(app, path, conditions, [], {})
env = rails_env(
'PATH_INFO' => '/books/list.rss',
@ -565,7 +565,7 @@ module ActionDispatch
conditions = {
:request_method => 'GET'
}
@router.routes.add_route(app, path, conditions, {})
@router.routes.add_route(app, path, conditions, [], {})
env = rails_env 'PATH_INFO' => '/books/list.rss',
"REQUEST_METHOD" => "HEAD"
@ -582,7 +582,7 @@ module ActionDispatch
path = Path::Pattern.from_string "/books(/:action(.:format))"
app = Object.new
conditions = { request_method: 'GET' }
@router.routes.add_route(app, path, conditions, {})
@router.routes.add_route(app, path, conditions, [], {})
env = rails_env 'PATH_INFO' => '/books/list.rss',
"REQUEST_METHOD" => "POST"
@ -597,7 +597,7 @@ module ActionDispatch
conditions = conditions.dup
conditions[:request_method] = 'POST'
post = @router.routes.add_route(app, path, conditions, {})
post = @router.routes.add_route(app, path, conditions, [], {})
called = false
@router.recognize(env) do |r, params|
@ -617,7 +617,7 @@ module ActionDispatch
else
path = Path::Pattern.new path
end
router.routes.add_route @app, path, {}, {}, {}
router.routes.add_route @app, path, {}, [], {}, {}
end
end

18
actionpack/test/journey/routes_test.rb
View file

@ -13,7 +13,7 @@ module ActionDispatch
path = Path::Pattern.new exp
requirements = { :hello => /world/ }
routes.add_route nil, path, requirements, {:id => nil}, {}
routes.add_route nil, path, requirements, [], {:id => nil}, {}
assert_not routes.empty?
assert_equal 1, routes.length
@ -26,9 +26,9 @@ module ActionDispatch
routes = Routes.new
path = Path::Pattern.from_string '/hello'
routes.add_route nil, path, {}, {}, {}
routes.add_route nil, path, {}, [], {}, {}
ast = routes.ast
routes.add_route nil, path, {}, {}, {}
routes.add_route nil, path, {}, [], {}, {}
assert_not_equal ast, routes.ast
end
@ -36,16 +36,16 @@ module ActionDispatch
routes = Routes.new
path = Path::Pattern.from_string '/hello'
routes.add_route nil, path, {}, {}, {}
routes.add_route nil, path, {}, [], {}, {}
sim = routes.simulator
routes.add_route nil, path, {}, {}, {}
routes.add_route nil, path, {}, [], {}, {}
assert_not_equal sim, routes.simulator
end
def test_partition_route
path = Path::Pattern.from_string '/hello'
anchored_route = @routes.add_route nil, path, {}, {}, {}
anchored_route = @routes.add_route nil, path, {}, [], {}, {}
assert_equal [anchored_route], @routes.anchored_routes
assert_equal [], @routes.custom_routes
@ -54,7 +54,7 @@ module ActionDispatch
)
path = Path::Pattern.new strexp
custom_route = @routes.add_route nil, path, {}, {}, {}
custom_route = @routes.add_route nil, path, {}, [], {}, {}
assert_equal [custom_route], @routes.custom_routes
assert_equal [anchored_route], @routes.anchored_routes
end
@ -65,8 +65,8 @@ module ActionDispatch
one = Path::Pattern.from_string '/hello'
two = Path::Pattern.from_string '/aaron'
routes.add_route nil, one, {}, {}, 'aaron'
routes.add_route nil, two, {}, {}, 'aaron'
routes.add_route nil, one, {}, [], {}, 'aaron'
routes.add_route nil, two, {}, [], {}, 'aaron'
assert_equal '/hello', routes.named_routes['aaron'].path.spec.to_s
end