extract required_defaults from the conditions hash before constructing the route

this way we can remove the strange "respond_to?" conditional in the
`matches?` loop

6 files changed, 56 insertions, 56 deletions

diff --git a/actionpack/lib/action_dispatch/journey/route.rb b/actionpack/lib/action_dispatch/journey/route.rb
index 4698ff8cc7..cbc985640a 100644
--- a/actionpack/lib/action_dispatch/journey/route.rb
+++ b/actionpack/lib/action_dispatch/journey/route.rb
@@ -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
diff --git a/actionpack/lib/action_dispatch/journey/routes.rb b/actionpack/lib/action_dispatch/journey/routes.rb
index 6325dfa3dd..5990964b57 100644
--- a/actionpack/lib/action_dispatch/journey/routes.rb
+++ b/actionpack/lib/action_dispatch/journey/routes.rb
@@ -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
diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb
index d0d8ded515..1e950d13f8 100644
--- a/actionpack/lib/action_dispatch/routing/route_set.rb
+++ b/actionpack/lib/action_dispatch/routing/route_set.rb
@@ -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
diff --git a/actionpack/test/journey/route_test.rb b/actionpack/test/journey/route_test.rb
index 0980df06e8..eff96a0abc 100644
--- a/actionpack/test/journey/route_test.rb
+++ b/actionpack/test/journey/route_test.rb
@@ -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"}
 
diff --git a/actionpack/test/journey/router_test.rb b/actionpack/test/journey/router_test.rb
index a134e343cc..802fb93c69 100644
--- a/actionpack/test/journey/router_test.rb
+++ b/actionpack/test/journey/router_test.rb
@@ -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
 
diff --git a/actionpack/test/journey/routes_test.rb b/actionpack/test/journey/routes_test.rb
index 2a1c1c337e..b9dac8751c 100644
--- a/actionpack/test/journey/routes_test.rb
+++ b/actionpack/test/journey/routes_test.rb
@@ -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