use the mapper to build the routing table

We should build the routes using the user facing API which is `Mapper`.
This frees up the library internals to change as we see fit. IOW we
shouldn't be testing internals.

actionpack/test/journey/router_test.rb

@@ -4,7 +4,7 @@
 module ActionDispatch
   module Journey
     class TestRouter < ActiveSupport::TestCase
-      attr_reader :routes
+      attr_reader :routes, :mapper
 
       def setup
         @app       = Routing::RouteSet::Dispatcher.new({})
@@ -12,125 +12,35 @@ def setup
         @routes = @route_set.router.routes
         @router = @route_set.router
         @formatter = @route_set.formatter
-      end
-
-      class FakeRequestFeeler < ActionDispatch::Request
-        attr_writer :env
-        attr_accessor :called
-
-        def new env
-          self.env = env
-          self
-        end
-
-        def hello
-          self.called = true
-          'world'
-        end
+        @mapper = ActionDispatch::Routing::Mapper.new @route_set
       end
 
       def test_dashes
-        router = Router.new(routes)
-
-        path = Path::Pattern.build '/foo-bar-baz', {}, ['/.?'], true
-
-        add_route nil, path, {}, [], {:id => nil}, {}
+        mapper.get '/foo-bar-baz', to: 'foo#bar'
 
         env = rails_env 'PATH_INFO' => '/foo-bar-baz'
         called = false
-        router.recognize(env) do |r, params|
+        @router.recognize(env) do |r, params|
           called = true
         end
         assert called
       end
 
       def test_unicode
-        router = Router.new(routes)
+        mapper.get '/ほげ', to: 'foo#bar'
 
         #match the escaped version of /ほげ
-        path = Path::Pattern.build '/%E3%81%BB%E3%81%92', {}, ['/.?'], true
-
-        add_route nil, path, {}, [], {:id => nil}, {}
-
         env = rails_env 'PATH_INFO' => '/%E3%81%BB%E3%81%92'
         called = false
-        router.recognize(env) do |r, params|
+        @router.recognize(env) do |r, params|
           called = true
         end
         assert called
       end
 
-      def test_request_class_and_requirements_success
-        klass  = FakeRequestFeeler.new nil
-        router = Router.new(routes)
-
-        requirements = { :hello => /world/ }
-
-        path = Path::Pattern.build '/foo(/:id)', {}, ['/.?'], true
-
-        add_route nil, path, requirements, [], {:id => nil}, {}
-
-        env = rails_env({'PATH_INFO' => '/foo/10'}, klass)
-        router.recognize(env) do |r, params|
-          assert_equal({:id => '10'}, params)
-        end
-
-        assert klass.called, 'hello should have been called'
-        assert_equal env.env, klass.env
-      end
-
-      def test_request_class_and_requirements_fail
-        klass  = FakeRequestFeeler.new nil
-        router = Router.new(routes)
-
-        requirements = { :hello => /mom/ }
-
-        path = Path::Pattern.build '/foo(/:id)', {}, ['/.?'], true
-
-        add_route nil, path, requirements, [], {:id => nil}, {}
-
-        env = rails_env({'PATH_INFO' => '/foo/10'}, klass)
-        router.recognize(env) do |r, params|
-          flunk 'route should not be found'
-        end
-
-        assert klass.called, 'hello should have been called'
-        assert_equal env.env, klass.env
-      end
-
-      class CustomPathRequest < ActionDispatch::Request
-        def path_info
-          env['custom.path_info']
-        end
-
-        def path_info=(x)
-          env['custom.path_info'] = x
-        end
-      end
-
-      def test_request_class_overrides_path_info
-        router = Router.new(routes)
-
-        path = Path::Pattern.build '/bar', {}, ['/.?'], true
-
-        add_route nil, path, {}, [], {}, {}
-
-        env = rails_env({'PATH_INFO' => '/foo',
-                         'custom.path_info' => '/bar'}, CustomPathRequest)
-
-        recognized = false
-        router.recognize(env) do |r, params|
-          recognized = true
-        end
-
-        assert recognized, "route should have been recognized"
-      end
-
       def test_regexp_first_precedence
-        add_routes @router, [
-          Path::Pattern.build("/whois/:domain", {:domain => /\w+\.[\w\.]+/}, ['/', '.', '?'], true),
-          Path::Pattern.build("/whois/:id(.:format)", {}, ['/', '.', '?'], true)
-        ]
+        mapper.get "/whois/:domain", :domain => /\w+\.[\w\.]+/, to: "foo#bar"
+        mapper.get "/whois/:id(.:format)", to: "foo#baz"
 
         env = rails_env 'PATH_INFO' => '/whois/example.com'
 
@@ -142,25 +52,21 @@ def test_regexp_first_precedence
 
         r = list.first
 
-        assert_equal '/whois/:domain', r.path.spec.to_s
+        assert_equal '/whois/:domain(.:format)', r.path.spec.to_s
       end
 
       def test_required_parts_verified_are_anchored
-        add_routes @router, [
-          Path::Pattern.build("/foo/:id", { :id => /\d/ }, ['/', '.', '?'], false)
-        ]
+        mapper.get "/foo/:id", :id => /\d/, anchor: false, to: "foo#bar"
 
         assert_raises(ActionController::UrlGenerationError) do
-          @formatter.generate(nil, { :id => '10' }, { })
+          @formatter.generate(nil, { :controller => "foo", :action => "bar", :id => '10' }, { })
         end
       end
 
       def test_required_parts_are_verified_when_building
-        add_routes @router, [
-          Path::Pattern.build("/foo/:id", { :id => /\d+/ }, ['/', '.', '?'], false)
-        ]
+        mapper.get "/foo/:id", :id => /\d+/, anchor: false, to: "foo#bar"
 
-        path, _ = @formatter.generate(nil, { :id => '10' }, { })
+        path, _ = @formatter.generate(nil, { :controller => "foo", :action => "bar", :id => '10' }, { })
         assert_equal '/foo/10', path
 
         assert_raises(ActionController::UrlGenerationError) do
@@ -169,17 +75,15 @@ def test_required_parts_are_verified_when_building
       end
 
       def test_only_required_parts_are_verified
-        add_routes @router, [
-          Path::Pattern.build("/foo(/:id)", {:id => /\d/}, ['/', '.', '?'], false)
-        ]
+        mapper.get "/foo(/:id)", :id => /\d/, :to => "foo#bar"
 
-        path, _ = @formatter.generate(nil, { :id => '10' }, { })
+        path, _ = @formatter.generate(nil, { :controller => "foo", :action => "bar", :id => '10' }, { })
         assert_equal '/foo/10', path
 
-        path, _ = @formatter.generate(nil, { }, { })
+        path, _ = @formatter.generate(nil, { :controller => "foo", :action => "bar" }, { })
         assert_equal '/foo', path
 
-        path, _ = @formatter.generate(nil, { :id => 'aa' }, { })
+        path, _ = @formatter.generate(nil, { :controller => "foo", :action => "bar", :id => 'aa' }, { })
         assert_equal '/foo/aa', path
       end
 
@@ -206,7 +110,7 @@ def test_does_not_include_missing_keys_message
       end
 
       def test_X_Cascade
-        add_routes @router, [ "/messages(.:format)" ]
+        mapper.get "/messages(.:format)", to: "foo#bar"
         resp = @router.serve(rails_env({ 'REQUEST_METHOD' => 'GET', 'PATH_INFO' => '/lol' }))
         assert_equal ['Not Found'], resp.last
         assert_equal 'pass', resp[1]['X-Cascade']
@@ -227,24 +131,21 @@ def test_clear_trailing_slash_from_script_name_on_root_unanchored_routes
       end
 
       def test_defaults_merge_correctly
-        path  = Path::Pattern.from_string '/foo(/:id)'
-        add_route nil, path, {}, [], {:id => nil}, {}
+        mapper.get '/foo(/:id)', to: "foo#bar", id: nil
 
         env = rails_env 'PATH_INFO' => '/foo/10'
         @router.recognize(env) do |r, params|
-          assert_equal({:id => '10'}, params)
+          assert_equal({:id => '10', :controller => "foo", :action => "bar"}, params)
         end
 
         env = rails_env 'PATH_INFO' => '/foo'
         @router.recognize(env) do |r, params|
-          assert_equal({:id => nil}, params)
+          assert_equal({:id => nil, :controller => "foo", :action => "bar"}, params)
         end
       end
 
       def test_recognize_with_unbound_regexp
-        add_routes @router, [
-          Path::Pattern.build("/foo", { }, ['/', '.', '?'], false)
-        ]
+        mapper.get "/foo", anchor: false, to: "foo#bar"
 
         env = rails_env 'PATH_INFO' => '/foo/bar'
 
@@ -255,9 +156,7 @@ def test_recognize_with_unbound_regexp
       end
 
       def test_bound_regexp_keeps_path_info
-        add_routes @router, [
-          Path::Pattern.build("/foo", { }, ['/', '.', '?'], true)
-        ]
+        mapper.get "/foo", to: "foo#bar"
 
         env = rails_env 'PATH_INFO' => '/foo'
 
@@ -270,12 +169,14 @@ def test_bound_regexp_keeps_path_info
       end
 
       def test_path_not_found
-        add_routes @router, [
+        [
           "/messages(.:format)",
           "/messages/new(.:format)",
           "/messages/:id/edit(.:format)",
           "/messages/:id(.:format)"
-        ]
+        ].each do |path|
+          mapper.get path, to: "foo#bar"
+        end
         env = rails_env 'PATH_INFO' => '/messages/unknown/path'
         yielded = false
 
@@ -286,32 +187,29 @@ def test_path_not_found
       end
 
       def test_required_part_in_recall
-        add_routes @router, [ "/messages/:a/:b" ]
+        mapper.get "/messages/:a/:b", to: "foo#bar"
 
-        path, _ = @formatter.generate(nil, { :a => 'a' }, { :b => 'b' })
+        path, _ = @formatter.generate(nil, { :controller => "foo", :action => "bar", :a => 'a' }, { :b => 'b' })
         assert_equal "/messages/a/b", path
       end
 
       def test_splat_in_recall
-        add_routes @router, [ "/*path" ]
+        mapper.get "/*path", to: "foo#bar"
 
-        path, _ = @formatter.generate(nil, { }, { :path => 'b' })
+        path, _ = @formatter.generate(nil, { :controller => "foo", :action => "bar" }, { :path => 'b' })
         assert_equal "/b", path
       end
 
       def test_recall_should_be_used_when_scoring
-        add_routes @router, [
-          "/messages/:action(/:id(.:format))",
-          "/messages/:id(.:format)"
-        ]
+        mapper.get "/messages/:action(/:id(.:format))", to: 'foo#bar'
+        mapper.get "/messages/:id(.:format)", to: 'bar#baz'
 
-        path, _ = @formatter.generate(nil, { :id => 10 }, { :action => 'index' })
+        path, _ = @formatter.generate(nil, { :controller => "foo", :id => 10 }, { :action => 'index' })
         assert_equal "/messages/index/10", path
       end
 
       def test_nil_path_parts_are_ignored
-        path  = Path::Pattern.from_string "/:controller(/:action(.:format))"
-        add_route @app, path, {}, [], {}, {}
+        mapper.get "/:controller(/:action(.:format))", to: "tasks#lol"
 
         params = { :controller => "tasks", :format => nil }
         extras = { :action => 'lol' }
@@ -323,17 +221,14 @@ def test_nil_path_parts_are_ignored
       def test_generate_slash
         params = [ [:controller, "tasks"],
                    [:action, "show"] ]
-        path = Path::Pattern.build("/", Hash[params], ['/', '.', '?'], true)
-
-        add_route @app, path, {}, [], {}, {}
+        mapper.get "/", Hash[params]
 
         path, _ = @formatter.generate(nil, Hash[params], {})
         assert_equal '/', path
       end
 
       def test_generate_calls_param_proc
-        path  = Path::Pattern.from_string '/:controller(/:action)'
-        add_route @app, path, {}, [], {}, {}
+        mapper.get '/:controller(/:action)', to: "foo#bar"
 
         parameterized = []
         params = [ [:controller, "tasks"],
@@ -349,8 +244,7 @@ def test_generate_calls_param_proc
       end
 
       def test_generate_id
-        path  = Path::Pattern.from_string '/:controller(/:action)'
-        add_route @app, path, {}, [], {}, {}
+        mapper.get '/:controller(/:action)', to: 'foo#bar'
 
         path, params = @formatter.generate(
           nil, {:id=>1, :controller=>"tasks", :action=>"show"}, {})
@@ -359,8 +253,7 @@ def test_generate_id
       end
 
       def test_generate_escapes
-        path  = Path::Pattern.from_string '/:controller(/:action)'
-        add_route @app, path, {}, [], {}, {}
+        mapper.get '/:controller(/:action)', to: "foo#bar"
 
         path, _ = @formatter.generate(nil,
           { :controller        => "tasks",
@@ -370,8 +263,7 @@ def test_generate_escapes
       end
 
       def test_generate_escapes_with_namespaced_controller
-        path  = Path::Pattern.from_string '/:controller(/:action)'
-        add_route @app, path, {}, [], {}, {}
+        mapper.get '/:controller(/:action)', to: "foo#bar"
 
         path, _ = @formatter.generate(
           nil, { :controller        => "admin/tasks",
@@ -381,8 +273,7 @@ def test_generate_escapes_with_namespaced_controller
       end
 
       def test_generate_extra_params
-        path  = Path::Pattern.from_string '/:controller(/:action)'
-        add_route @app, path, {}, [], {}, {}
+        mapper.get '/:controller(/:action)', to: "foo#bar"
 
         path, params = @formatter.generate(
           nil, { :id                => 1,
@@ -395,8 +286,7 @@ def test_generate_extra_params
       end
 
       def test_generate_missing_keys_no_matches_different_format_keys
-        path  = Path::Pattern.from_string '/:controller/:action/:name'
-        add_route @app, path, {}, [], {}, {}
+        mapper.get '/:controller/:action/:name', to: "foo#bar"
         primarty_parameters = {
           :id                => 1,
           :controller        => "tasks",
@@ -422,8 +312,7 @@ def test_generate_missing_keys_no_matches_different_format_keys
       end
 
       def test_generate_uses_recall_if_needed
-        path  = Path::Pattern.from_string '/:controller(/:action(/:id))'
-        add_route @app, path, {}, [], {}, {}
+        mapper.get '/:controller(/:action(/:id))', to: "foo#bar"
 
         path, params = @formatter.generate(
           nil,
@@ -434,8 +323,7 @@ def test_generate_uses_recall_if_needed
       end
 
       def test_generate_with_name
-        path  = Path::Pattern.from_string '/:controller(/:action)'
-        add_route @app, path, {}, [], {}, "tasks"
+        mapper.get '/:controller(/:action)', to: 'foo#bar', as: 'tasks'
 
         path, params = @formatter.generate(
           "tasks",
@@ -451,16 +339,15 @@ def test_generate_with_name
         '/content/show/10'  => { :controller => 'content', :action => 'show', :id => "10" },
       }.each do |request_path, expected|
         define_method("test_recognize_#{expected.keys.map(&:to_s).join('_')}") do
-          path  = Path::Pattern.from_string "/:controller(/:action(/:id))"
-          app   = Object.new
-          route = add_route(app, path, {}, [], {}, {})
+          mapper.get "/:controller(/:action(/:id))", to: 'foo#bar'
+          route = @routes.first
 
           env = rails_env 'PATH_INFO' => request_path
           called   = false
 
           @router.recognize(env) do |r, params|
             assert_equal route, r
-            assert_equal(expected, params)
+            assert_equal({ :action => "bar" }.merge(expected), params)
             called = true
           end
 
@@ -473,16 +360,15 @@ def test_generate_with_name
         :splat   => ['/segment/a/b%20c+d', { :segment => 'segment', :splat => 'a/b c+d' }]
       }.each do |name, (request_path, expected)|
         define_method("test_recognize_#{name}") do
-          path  = Path::Pattern.from_string '/:segment/*splat'
-          app   = Object.new
-          route = add_route(app, path, {}, [], {}, {})
+          mapper.get '/:segment/*splat', to: 'foo#bar'
 
           env = rails_env 'PATH_INFO' => request_path
           called   = false
+          route = @routes.first
 
           @router.recognize(env) do |r, params|
             assert_equal route, r
-            assert_equal(expected, params)
+            assert_equal(expected.merge(:controller=>"foo", :action=>"bar"), params)
             called = true
           end
 
@@ -491,14 +377,8 @@ def test_generate_with_name
       end
 
       def test_namespaced_controller
-        path = Path::Pattern.build(
-          "/:controller(/:action(/:id))",
-          { :controller => /.+?/ },
-          ["/", ".", "?"],
-          true
-        )
-        app   = Object.new
-        route = add_route(app, path, {}, [], {}, {})
+        mapper.get "/:controller(/:action(/:id))", { :controller => /.+?/ }
+        route = @routes.first
 
         env = rails_env 'PATH_INFO' => '/admin/users/show/10'
         called   = false
@@ -517,9 +397,8 @@ def test_namespaced_controller
       end
 
       def test_recognize_literal
-        path   = Path::Pattern.from_string "/books(/:action(.:format))"
-        app    = Object.new
-        route  = add_route(app, path, {}, [], {:controller => 'books'})
+        mapper.get "/books(/:action(.:format))", controller: "books"
+        route = @routes.first
 
         env    = rails_env 'PATH_INFO' => '/books/list.rss'
         expected = { :controller => 'books', :action => 'list', :format => 'rss' }
@@ -534,10 +413,7 @@ def test_recognize_literal
       end
 
       def test_recognize_head_route
-        path   = Path::Pattern.from_string "/books(/:action(.:format))"
-        app    = Object.new
-        conditions = { request_method: 'HEAD' }
-        add_route(app, path, conditions, [], {})
+        mapper.match "/books(/:action(.:format))", via: 'head', to: 'foo#bar'
 
         env = rails_env(
           'PATH_INFO' => '/books/list.rss',
@@ -553,12 +429,7 @@ def test_recognize_head_route
       end
 
       def test_recognize_head_request_as_get_route
-        path   = Path::Pattern.from_string "/books(/:action(.:format))"
-        app    = Object.new
-        conditions = {
-          :request_method => 'GET'
-        }
-        add_route(app, path, conditions, [], {})
+        mapper.get "/books(/:action(.:format))", to: 'foo#bar'
 
         env = rails_env 'PATH_INFO' => '/books/list.rss',
                         "REQUEST_METHOD"    => "HEAD"
@@ -571,11 +442,8 @@ def test_recognize_head_request_as_get_route
         assert called
       end
 
-      def test_recognize_cares_about_verbs
-        path   = Path::Pattern.from_string "/books(/:action(.:format))"
-        app    = Object.new
-        conditions = { request_method: 'GET' }
-        add_route(app, path, conditions, [], {})
+      def test_recognize_cares_about_get_verbs
+        mapper.match "/books(/:action(.:format))", to: "foo#bar", via: :get
 
         env = rails_env 'PATH_INFO' => '/books/list.rss',
                         "REQUEST_METHOD" => "POST"
@@ -586,21 +454,48 @@ def test_recognize_cares_about_verbs
         end
 
         assert_not called
+      end
 
-        conditions = conditions.dup
-        conditions[:request_method] = 'POST'
+      def test_recognize_cares_about_post_verbs
+        mapper.match "/books(/:action(.:format))", to: "foo#bar", via: :post
 
-        post = add_route(app, path, conditions, [], {})
+        env = rails_env 'PATH_INFO' => '/books/list.rss',
+                        "REQUEST_METHOD" => "POST"
 
         called = false
         @router.recognize(env) do |r, params|
-          assert_equal post, r
           called = true
         end
 
         assert called
       end
 
+      def test_multi_verb_recognition
+        mapper.match "/books(/:action(.:format))", to: "foo#bar", via: [:post, :get]
+
+        %w( POST GET ).each do |verb|
+          env = rails_env 'PATH_INFO' => '/books/list.rss',
+            "REQUEST_METHOD" => verb
+
+          called = false
+          @router.recognize(env) do |r, params|
+            called = true
+          end
+
+          assert called
+        end
+
+        env = rails_env 'PATH_INFO' => '/books/list.rss',
+          "REQUEST_METHOD" => 'PUT'
+
+        called = false
+        @router.recognize(env) do |r, params|
+          called = true
+        end
+
+        assert_not called
+      end
+
       private
 
       def add_routes router, paths, anchor = true
@@ -637,12 +532,6 @@ def rack_env env
           "CONTENT_LENGTH"    => "0"
         }.merge env
       end
-
-      MyMapping = Struct.new(:application, :path, :conditions, :required_defaults, :defaults)
-
-      def add_route(app, path, conditions, required_defaults, defaults, name = nil)
-        @routes.add_route(name, MyMapping.new(app, path, conditions, required_defaults, defaults))
-      end
     end
   end
 end

actionpack/test/journey/routes_test.rb

@@ -3,27 +3,19 @@
 module ActionDispatch
   module Journey
     class TestRoutes < ActiveSupport::TestCase
-      attr_reader :routes
+      attr_reader :routes, :mapper
 
       def setup
         @route_set  = ActionDispatch::Routing::RouteSet.new
         @routes = @route_set.router.routes
         @router = @route_set.router
+        @mapper = ActionDispatch::Routing::Mapper.new @route_set
         super
       end
 
-      MyMapping = Struct.new(:application, :path, :conditions, :required_defaults, :defaults)
-
-      def add_route(app, path, conditions, required_defaults, defaults, name = nil)
-        @routes.add_route(name, MyMapping.new(app, path, conditions, required_defaults, defaults))
-      end
-
       def test_clear
-        path    = Path::Pattern.build '/foo(/:id)', {}, ['/.?'], true
-        requirements = { :hello => /world/ }
-
-        add_route nil, path, requirements, [], {:id => nil}, {}
-        assert_not routes.empty?
+        mapper.get "/foo(/:id)", to: "foo#bar", as: 'aaron'
+        assert_not_predicate routes, :empty?
         assert_equal 1, routes.length
 
         routes.clear
@@ -32,41 +24,32 @@ def test_clear
       end
 
       def test_ast
-        path   = Path::Pattern.from_string '/hello'
-
-        add_route nil, path, {}, [], {}, {}
+        mapper.get "/foo(/:id)", to: "foo#bar", as: 'aaron'
         ast = routes.ast
-        add_route nil, path, {}, [], {}, {}
+        mapper.get "/foo(/:id)", to: "foo#bar", as: 'gorby'
         assert_not_equal ast, routes.ast
       end
 
       def test_simulator_changes
-        path   = Path::Pattern.from_string '/hello'
-
-        add_route nil, path, {}, [], {}, {}
+        mapper.get "/foo(/:id)", to: "foo#bar", as: 'aaron'
         sim = routes.simulator
-        add_route nil, path, {}, [], {}, {}
+        mapper.get "/foo(/:id)", to: "foo#bar", as: 'gorby'
         assert_not_equal sim, routes.simulator
       end
 
       def test_partition_route
-        path   = Path::Pattern.from_string '/hello'
+        mapper.get "/foo(/:id)", to: "foo#bar", as: 'aaron'
 
-        anchored_route = add_route nil, path, {}, [], {}, {}
-        assert_equal [anchored_route], @routes.anchored_routes
-        assert_equal [], @routes.custom_routes
+        assert_equal 1, @routes.anchored_routes.length
+        assert_predicate @routes.custom_routes, :empty?
 
-        path = Path::Pattern.build(
-          "/hello/:who", { who: /\d/ }, ['/', '.', '?'], false
-        )
+        mapper.get "/hello/:who", to: "foo#bar", as: 'bar', who: /\d/
 
-        custom_route = add_route nil, path, {}, [], {}, {}
-        assert_equal [custom_route], @routes.custom_routes
-        assert_equal [anchored_route], @routes.anchored_routes
+        assert_equal 1, @routes.custom_routes.length
+        assert_equal 1, @routes.anchored_routes.length
       end
 
       def test_first_name_wins
-        mapper = ActionDispatch::Routing::Mapper.new @route_set
         mapper.get "/hello", to: "foo#bar", as: 'aaron'
         assert_raise(ArgumentError) do
           mapper.get "/aaron", to: "foo#bar", as: 'aaron'