diff options
| -rw-r--r-- | actionpack/lib/action_dispatch/journey/routes.rb | 10 | ||||
| -rw-r--r-- | actionpack/lib/action_dispatch/routing/mapper.rb | 81 | ||||
| -rw-r--r-- | actionpack/lib/action_dispatch/routing/route_set.rb | 50 | ||||
| -rw-r--r-- | actionpack/test/dispatch/mapper_test.rb | 23 | ||||
| -rw-r--r-- | actionpack/test/journey/router_test.rb | 58 | ||||
| -rw-r--r-- | actionpack/test/journey/routes_test.rb | 34 |
6 files changed, 131 insertions, 125 deletions
diff --git a/actionpack/lib/action_dispatch/journey/routes.rb b/actionpack/lib/action_dispatch/journey/routes.rb index 5990964b57..dacb0ccf48 100644 --- a/actionpack/lib/action_dispatch/journey/routes.rb +++ b/actionpack/lib/action_dispatch/journey/routes.rb @@ -62,9 +62,13 @@ module ActionDispatch end end - # Add a route to the routing table. - def add_route(app, path, conditions, required_defaults, defaults, name = nil) - route = Route.new(name, app, path, conditions, required_defaults, defaults) + def add_route(name, mapping) + route = Route.new(name, + mapping.application, + mapping.path, + mapping.conditions, + mapping.required_defaults, + mapping.defaults) route.precedence = routes.length routes << route diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 4411625e51..d80faf7423 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -59,16 +59,17 @@ module ActionDispatch class Mapping #:nodoc: ANCHOR_CHARACTERS_REGEX = %r{\A(\\A|\^)|(\\Z|\\z|\$)\Z} - attr_reader :requirements, :conditions, :defaults + attr_reader :requirements, :defaults attr_reader :to, :default_controller, :default_action + attr_reader :required_defaults, :ast - def self.build(scope, set, ast, controller, default_action, to, via, formatted, options_constraints, options) + def self.build(scope, set, ast, controller, default_action, to, via, formatted, options_constraints, anchor, options) options = scope[:options].merge(options) if scope[:options] defaults = (scope[:defaults] || {}).dup scope_constraints = scope[:constraints] || {} - new set, ast, defaults, controller, default_action, scope[:module], to, formatted, scope_constraints, scope[:blocks] || [], via, options_constraints, options + new set, ast, defaults, controller, default_action, scope[:module], to, formatted, scope_constraints, scope[:blocks] || [], via, options_constraints, anchor, options end def self.check_via(via) @@ -99,13 +100,15 @@ module ActionDispatch format != false && !path.include?(':format') && !path.end_with?('/') end - def initialize(set, ast, defaults, controller, default_action, modyoule, to, formatted, scope_constraints, blocks, via, options_constraints, options) + def initialize(set, ast, defaults, controller, default_action, modyoule, to, formatted, scope_constraints, blocks, via, options_constraints, anchor, options) @defaults = defaults @set = set @to = to @default_controller = controller @default_action = default_action + @ast = ast + @anchor = anchor path_params = ast.find_all(&:symbol?).map(&:to_sym) @@ -136,16 +139,67 @@ module ActionDispatch @conditions = Hash[conditions] @defaults = formats[:defaults].merge(@defaults).merge(normalize_defaults(options)) - @conditions[:required_defaults] = (split_options[:required_defaults] || []).map(&:first) + @required_defaults = (split_options[:required_defaults] || []).map(&:first) unless via == [:all] @conditions[:request_method] = via.map { |m| m.to_s.dasherize.upcase } end end - def to_route - [ app(@blocks), conditions, requirements, defaults ] + def application + app(@blocks) end + def path + build_path @ast, requirements, @anchor + end + + def conditions + build_conditions @conditions, @set.request_class + end + + def build_conditions(current_conditions, request_class) + conditions = current_conditions.dup + + # Rack-Mount requires that :request_method be a regular expression. + # :request_method represents the HTTP verb that matches this route. + # + # Here we munge values before they get sent on to rack-mount. + verbs = conditions[:request_method] || [] + unless verbs.empty? + conditions[:request_method] = %r[^#{verbs.join('|')}$] + end + + conditions.keep_if do |k, _| + request_class.public_method_defined?(k) + end + end + private :build_conditions + + def build_path(ast, requirements, anchor) + pattern = Journey::Path::Pattern.new(ast, requirements, SEPARATORS, anchor) + + builder = Journey::GTG::Builder.new ast + + # Get all the symbol nodes followed by literals that are not the + # dummy node. + symbols = ast.grep(Journey::Nodes::Symbol).find_all { |n| + builder.followpos(n).first.literal? + } + + # Get all the symbol nodes preceded by literals. + symbols.concat ast.find_all(&:literal?).map { |n| + builder.followpos(n).first + }.find_all(&:symbol?) + + symbols.each { |x| + x.regexp = /(?:#{Regexp.union(x.regexp, '-')})+/ + } + + pattern + end + private :build_path + + private def add_wildcard_options(options, formatted, path_ast) # Add a constraint for wildcard route to make it non-greedy and match the @@ -1553,7 +1607,8 @@ to this: route_options[:as] = _path _path = option_path end - process_path(route_options, controller, _path, _path, to, via, formatted, anchor, options_constraints) + to = get_to_from_path(_path, to, route_options[:action]) + decomposed_match(_path, controller, route_options, _path, to, via, formatted, anchor, options_constraints) end path_types.fetch(Symbol, []).each do |action| @@ -1575,11 +1630,6 @@ to this: end end - def process_path(options, controller, path, option_path, to, via, formatted, ancho, options_constraintsr) - to = get_to_from_path(path, to, options[:action]) - decomposed_match(path, controller, options, option_path, to, via, formatted, ancho, options_constraintsr) - end - def using_match_shorthand?(path) path =~ %r{^/?[-\w]+/[-\w/]+$} end @@ -1622,9 +1672,8 @@ to this: path = Mapping.normalize_path URI.parser.escape(path), formatted ast = Journey::Parser.parse path - mapping = Mapping.build(@scope, @set, ast, controller, default_action, to, via, formatted, options_constraints, options) - app, conditions, requirements, defaults = mapping.to_route - @set.add_route(app, conditions, ast, requirements, defaults, as, anchor) + mapping = Mapping.build(@scope, @set, ast, controller, default_action, to, via, formatted, options_constraints, anchor, options) + @set.add_route(mapping, ast, as, anchor) end def root(path, options={}) diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index 04d8013768..4f698c84ab 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -514,7 +514,7 @@ module ActionDispatch routes.empty? end - def add_route(app, conditions, path_ast, requirements, defaults, name, anchor) + def add_route(mapping, path_ast, name, anchor) raise ArgumentError, "Invalid route name: '#{name}'" unless name.blank? || name.to_s.match(/^[_a-z]\w*$/i) if name && named_routes[name] @@ -525,57 +525,11 @@ module ActionDispatch "http://guides.rubyonrails.org/routing.html#restricting-the-routes-created" end - required_defaults = conditions.delete :required_defaults - path = build_path(path_ast, requirements, anchor) - conditions = build_conditions(conditions) - - route = @set.add_route(app, path, conditions, required_defaults, defaults, name) + route = @set.add_route(name, mapping) named_routes[name] = route if name route end - def build_path(ast, requirements, anchor) - pattern = Journey::Path::Pattern.new(ast, requirements, SEPARATORS, anchor) - - builder = Journey::GTG::Builder.new ast - - # Get all the symbol nodes followed by literals that are not the - # dummy node. - symbols = ast.grep(Journey::Nodes::Symbol).find_all { |n| - builder.followpos(n).first.literal? - } - - # Get all the symbol nodes preceded by literals. - symbols.concat ast.find_all(&:literal?).map { |n| - builder.followpos(n).first - }.find_all(&:symbol?) - - symbols.each { |x| - x.regexp = /(?:#{Regexp.union(x.regexp, '-')})+/ - } - - pattern - end - private :build_path - - def build_conditions(current_conditions) - conditions = current_conditions.dup - - # Rack-Mount requires that :request_method be a regular expression. - # :request_method represents the HTTP verb that matches this route. - # - # Here we munge values before they get sent on to rack-mount. - verbs = conditions[:request_method] || [] - unless verbs.empty? - conditions[:request_method] = %r[^#{verbs.join('|')}$] - end - - conditions.keep_if do |k, _| - request_class.public_method_defined?(k) - end - end - private :build_conditions - class Generator PARAMETERIZE = lambda do |name, value| if name == :controller diff --git a/actionpack/test/dispatch/mapper_test.rb b/actionpack/test/dispatch/mapper_test.rb index 289fb69b8b..eca4ca8e0e 100644 --- a/actionpack/test/dispatch/mapper_test.rb +++ b/actionpack/test/dispatch/mapper_test.rb @@ -4,20 +4,10 @@ module ActionDispatch module Routing class MapperTest < ActiveSupport::TestCase class FakeSet < ActionDispatch::Routing::RouteSet - def initialize - @my_routes = [] - super - end - def resources_path_names {} end - def add_route(*args) - @my_routes << args - super - end - def request_class ActionDispatch::Request end @@ -27,11 +17,11 @@ module ActionDispatch end def defaults - @my_routes.map { |x| x[4] } + routes.map(&:defaults) end def conditions - @my_routes.map { |x| x[1] } + routes.map(&:constraints) end def requirements @@ -92,16 +82,15 @@ module ActionDispatch end assert_equal({:omg=>:awesome, :controller=>"posts", :action=>"index"}, fakeset.defaults.first) - assert_equal ["GET"], fakeset.conditions.first[:request_method] + assert_equal(/^GET$/, fakeset.conditions.first[:request_method]) end def test_mapping_requirements options = { } scope = Mapper::Scope.new({}) ast = Journey::Parser.parse '/store/:name(*rest)' - m = Mapper::Mapping.build(scope, FakeSet.new, ast, 'foo', 'bar', nil, [:get], nil, {}, options) - _, _, requirements, _ = m.to_route - assert_equal(/.+?/, requirements[:rest]) + m = Mapper::Mapping.build(scope, FakeSet.new, ast, 'foo', 'bar', nil, [:get], nil, {}, true, options) + assert_equal(/.+?/, m.requirements[:rest]) end def test_via_scope @@ -110,7 +99,7 @@ module ActionDispatch mapper.scope(via: :put) do mapper.match '/', :to => 'posts#index', :as => :main end - assert_equal ["PUT"], fakeset.conditions.first[:request_method] + assert_equal(/^PUT$/, fakeset.conditions.first[:request_method]) end def test_map_slash diff --git a/actionpack/test/journey/router_test.rb b/actionpack/test/journey/router_test.rb index 427d63d75d..bddfbe7ef0 100644 --- a/actionpack/test/journey/router_test.rb +++ b/actionpack/test/journey/router_test.rb @@ -33,7 +33,7 @@ module ActionDispatch path = Path::Pattern.build '/foo-bar-baz', {}, ['/.?'], true - routes.add_route nil, path, {}, [], {:id => nil}, {} + add_route nil, path, {}, [], {:id => nil}, {} env = rails_env 'PATH_INFO' => '/foo-bar-baz' called = false @@ -49,7 +49,7 @@ module ActionDispatch #match the escaped version of /ほげ path = Path::Pattern.build '/%E3%81%BB%E3%81%92', {}, ['/.?'], true - routes.add_route nil, path, {}, [], {:id => nil}, {} + add_route nil, path, {}, [], {:id => nil}, {} env = rails_env 'PATH_INFO' => '/%E3%81%BB%E3%81%92' called = false @@ -67,7 +67,7 @@ module ActionDispatch path = Path::Pattern.build '/foo(/:id)', {}, ['/.?'], true - routes.add_route nil, path, requirements, [], {:id => nil}, {} + add_route nil, path, requirements, [], {:id => nil}, {} env = rails_env({'PATH_INFO' => '/foo/10'}, klass) router.recognize(env) do |r, params| @@ -86,7 +86,7 @@ module ActionDispatch path = Path::Pattern.build '/foo(/:id)', {}, ['/.?'], true - router.routes.add_route nil, path, requirements, [], {:id => nil}, {} + add_route nil, path, requirements, [], {:id => nil}, {} env = rails_env({'PATH_INFO' => '/foo/10'}, klass) router.recognize(env) do |r, params| @@ -112,7 +112,7 @@ module ActionDispatch path = Path::Pattern.build '/bar', {}, ['/.?'], true - routes.add_route nil, path, {}, [], {}, {} + add_route nil, path, {}, [], {}, {} env = rails_env({'PATH_INFO' => '/foo', 'custom.path_info' => '/bar'}, CustomPathRequest) @@ -185,7 +185,7 @@ module ActionDispatch def test_knows_what_parts_are_missing_from_named_route route_name = "gorby_thunderhorse" path = Path::Pattern.build("/foo/:id", { :id => /\d+/ }, ['/', '.', '?'], false) - @router.routes.add_route nil, path, {}, [], {}, route_name + add_route nil, path, {}, [], {}, route_name error = assert_raises(ActionController::UrlGenerationError) do @formatter.generate(route_name, { }, { }) @@ -227,7 +227,7 @@ module ActionDispatch def test_defaults_merge_correctly path = Path::Pattern.from_string '/foo(/:id)' - @router.routes.add_route nil, path, {}, [], {:id => nil}, {} + add_route nil, path, {}, [], {:id => nil}, {} env = rails_env 'PATH_INFO' => '/foo/10' @router.recognize(env) do |r, params| @@ -310,7 +310,7 @@ module ActionDispatch def test_nil_path_parts_are_ignored path = Path::Pattern.from_string "/:controller(/:action(.:format))" - @router.routes.add_route @app, path, {}, [], {}, {} + add_route @app, path, {}, [], {}, {} params = { :controller => "tasks", :format => nil } extras = { :action => 'lol' } @@ -324,7 +324,7 @@ module ActionDispatch [:action, "show"] ] path = Path::Pattern.build("/", Hash[params], ['/', '.', '?'], true) - @router.routes.add_route @app, path, {}, [], {}, {} + add_route @app, path, {}, [], {}, {} path, _ = @formatter.generate(nil, Hash[params], {}) assert_equal '/', path @@ -332,7 +332,7 @@ module ActionDispatch def test_generate_calls_param_proc path = Path::Pattern.from_string '/:controller(/:action)' - @router.routes.add_route @app, path, {}, [], {}, {} + add_route @app, path, {}, [], {}, {} parameterized = [] params = [ [:controller, "tasks"], @@ -349,7 +349,7 @@ module ActionDispatch def test_generate_id path = Path::Pattern.from_string '/:controller(/:action)' - @router.routes.add_route @app, path, {}, [], {}, {} + add_route @app, path, {}, [], {}, {} path, params = @formatter.generate( nil, {:id=>1, :controller=>"tasks", :action=>"show"}, {}) @@ -359,7 +359,7 @@ module ActionDispatch def test_generate_escapes path = Path::Pattern.from_string '/:controller(/:action)' - @router.routes.add_route @app, path, {}, [], {}, {} + add_route @app, path, {}, [], {}, {} path, _ = @formatter.generate(nil, { :controller => "tasks", @@ -370,7 +370,7 @@ module ActionDispatch def test_generate_escapes_with_namespaced_controller path = Path::Pattern.from_string '/:controller(/:action)' - @router.routes.add_route @app, path, {}, [], {}, {} + add_route @app, path, {}, [], {}, {} path, _ = @formatter.generate( nil, { :controller => "admin/tasks", @@ -381,7 +381,7 @@ module ActionDispatch def test_generate_extra_params path = Path::Pattern.from_string '/:controller(/:action)' - @router.routes.add_route @app, path, {}, [], {}, {} + add_route @app, path, {}, [], {}, {} path, params = @formatter.generate( nil, { :id => 1, @@ -395,7 +395,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, {}, [], {}, {} + add_route @app, path, {}, [], {}, {} primarty_parameters = { :id => 1, :controller => "tasks", @@ -422,7 +422,7 @@ module ActionDispatch def test_generate_uses_recall_if_needed path = Path::Pattern.from_string '/:controller(/:action(/:id))' - @router.routes.add_route @app, path, {}, [], {}, {} + add_route @app, path, {}, [], {}, {} path, params = @formatter.generate( nil, @@ -434,7 +434,7 @@ module ActionDispatch def test_generate_with_name path = Path::Pattern.from_string '/:controller(/:action)' - @router.routes.add_route @app, path, {}, [], {}, "tasks" + add_route @app, path, {}, [], {}, "tasks" path, params = @formatter.generate( "tasks", @@ -452,7 +452,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 = add_route(app, path, {}, [], {}, {}) env = rails_env 'PATH_INFO' => request_path called = false @@ -474,7 +474,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 = add_route(app, path, {}, [], {}, {}) env = rails_env 'PATH_INFO' => request_path called = false @@ -497,7 +497,7 @@ module ActionDispatch true ) app = Object.new - route = @router.routes.add_route(app, path, {}, [], {}, {}) + route = add_route(app, path, {}, [], {}, {}) env = rails_env 'PATH_INFO' => '/admin/users/show/10' called = false @@ -518,7 +518,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 = add_route(app, path, {}, [], {:controller => 'books'}) env = rails_env 'PATH_INFO' => '/books/list.rss' expected = { :controller => 'books', :action => 'list', :format => 'rss' } @@ -536,7 +536,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, [], {}) + add_route(app, path, conditions, [], {}) env = rails_env( 'PATH_INFO' => '/books/list.rss', @@ -557,7 +557,7 @@ module ActionDispatch conditions = { :request_method => 'GET' } - @router.routes.add_route(app, path, conditions, [], {}) + add_route(app, path, conditions, [], {}) env = rails_env 'PATH_INFO' => '/books/list.rss', "REQUEST_METHOD" => "HEAD" @@ -574,7 +574,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, [], {}) + add_route(app, path, conditions, [], {}) env = rails_env 'PATH_INFO' => '/books/list.rss', "REQUEST_METHOD" => "POST" @@ -589,7 +589,7 @@ module ActionDispatch conditions = conditions.dup conditions[:request_method] = 'POST' - post = @router.routes.add_route(app, path, conditions, [], {}) + post = add_route(app, path, conditions, [], {}) called = false @router.recognize(env) do |r, params| @@ -609,7 +609,7 @@ module ActionDispatch else path end - router.routes.add_route @app, path, {}, [], {}, {} + add_route @app, path, {}, [], {}, {} end end @@ -636,6 +636,12 @@ module ActionDispatch "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 diff --git a/actionpack/test/journey/routes_test.rb b/actionpack/test/journey/routes_test.rb index 6853cefc01..01566f0148 100644 --- a/actionpack/test/journey/routes_test.rb +++ b/actionpack/test/journey/routes_test.rb @@ -3,16 +3,24 @@ require 'abstract_unit' module ActionDispatch module Journey class TestRoutes < ActiveSupport::TestCase - setup do + attr_reader :routes + + def setup @routes = Routes.new + 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 - routes = Routes.new path = Path::Pattern.build '/foo(/:id)', {}, ['/.?'], true requirements = { :hello => /world/ } - routes.add_route nil, path, requirements, [], {:id => nil}, {} + add_route nil, path, requirements, [], {:id => nil}, {} assert_not routes.empty? assert_equal 1, routes.length @@ -22,29 +30,27 @@ module ActionDispatch end def test_ast - routes = Routes.new path = Path::Pattern.from_string '/hello' - routes.add_route nil, path, {}, [], {}, {} + add_route nil, path, {}, [], {}, {} ast = routes.ast - routes.add_route nil, path, {}, [], {}, {} + add_route nil, path, {}, [], {}, {} assert_not_equal ast, routes.ast end def test_simulator_changes - routes = Routes.new path = Path::Pattern.from_string '/hello' - routes.add_route nil, path, {}, [], {}, {} + add_route nil, path, {}, [], {}, {} sim = routes.simulator - routes.add_route nil, path, {}, [], {}, {} + 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 = add_route nil, path, {}, [], {}, {} assert_equal [anchored_route], @routes.anchored_routes assert_equal [], @routes.custom_routes @@ -52,19 +58,17 @@ module ActionDispatch "/hello/:who", { who: /\d/ }, ['/', '.', '?'], false ) - custom_route = @routes.add_route nil, path, {}, [], {}, {} + custom_route = add_route nil, path, {}, [], {}, {} assert_equal [custom_route], @routes.custom_routes assert_equal [anchored_route], @routes.anchored_routes end def test_first_name_wins - routes = Routes.new - one = Path::Pattern.from_string '/hello' two = Path::Pattern.from_string '/aaron' - routes.add_route nil, one, {}, [], {}, 'aaron' - routes.add_route nil, two, {}, [], {}, 'aaron' + add_route nil, one, {}, [], {}, 'aaron' + add_route nil, two, {}, [], {}, 'aaron' assert_equal '/hello', routes.named_routes['aaron'].path.spec.to_s end |