diff options
| -rw-r--r-- | actionpack/lib/action_dispatch/journey/nodes/node.rb | 4 | ||||
| -rw-r--r-- | actionpack/lib/action_dispatch/journey/path/pattern.rb | 27 | ||||
| -rw-r--r-- | actionpack/lib/action_dispatch/journey/router/strexp.rb | 15 | ||||
| -rw-r--r-- | actionpack/lib/action_dispatch/routing/mapper.rb | 165 | ||||
| -rw-r--r-- | actionpack/lib/action_dispatch/routing/route_set.rb | 7 | ||||
| -rw-r--r-- | actionpack/test/dispatch/mapper_test.rb | 4 | ||||
| -rw-r--r-- | actionpack/test/dispatch/routing_test.rb | 32 | ||||
| -rw-r--r-- | actionpack/test/journey/path/pattern_test.rb | 46 | ||||
| -rw-r--r-- | actionpack/test/journey/route_test.rb | 22 | ||||
| -rw-r--r-- | actionpack/test/journey/router/strexp_test.rb | 32 | ||||
| -rw-r--r-- | actionpack/test/journey/router_test.rb | 66 | ||||
| -rw-r--r-- | actionpack/test/journey/routes_test.rb | 10 |
12 files changed, 220 insertions, 210 deletions
diff --git a/actionpack/lib/action_dispatch/journey/nodes/node.rb b/actionpack/lib/action_dispatch/journey/nodes/node.rb index 935442ef66..bb01c087bc 100644 --- a/actionpack/lib/action_dispatch/journey/nodes/node.rb +++ b/actionpack/lib/action_dispatch/journey/nodes/node.rb @@ -93,6 +93,10 @@ module ActionDispatch class Star < Unary # :nodoc: def type; :STAR; end + + def name + left.name.tr '*:', '' + end end class Binary < Node # :nodoc: diff --git a/actionpack/lib/action_dispatch/journey/path/pattern.rb b/actionpack/lib/action_dispatch/journey/path/pattern.rb index cb0a02c298..3af940a02f 100644 --- a/actionpack/lib/action_dispatch/journey/path/pattern.rb +++ b/actionpack/lib/action_dispatch/journey/path/pattern.rb @@ -1,27 +1,20 @@ +require 'action_dispatch/journey/router/strexp' + module ActionDispatch module Journey # :nodoc: module Path # :nodoc: class Pattern # :nodoc: attr_reader :spec, :requirements, :anchored + def self.from_string string + new Journey::Router::Strexp.build(string, {}, ["/.?"], true) + end + def initialize(strexp) - parser = Journey::Parser.new - - @anchored = true - - case strexp - when String - @spec = parser.parse(strexp) - @requirements = {} - @separators = "/.?" - when Router::Strexp - @spec = parser.parse(strexp.path) - @requirements = strexp.requirements - @separators = strexp.separators.join - @anchored = strexp.anchor - else - raise ArgumentError, "Bad expression: #{strexp}" - end + @spec = strexp.ast + @requirements = strexp.requirements + @separators = strexp.separators.join + @anchored = strexp.anchor @names = nil @optional_names = nil diff --git a/actionpack/lib/action_dispatch/journey/router/strexp.rb b/actionpack/lib/action_dispatch/journey/router/strexp.rb index f97f1a223e..4b7738f335 100644 --- a/actionpack/lib/action_dispatch/journey/router/strexp.rb +++ b/actionpack/lib/action_dispatch/journey/router/strexp.rb @@ -6,18 +6,21 @@ module ActionDispatch alias :compile :new end - attr_reader :path, :requirements, :separators, :anchor + attr_reader :path, :requirements, :separators, :anchor, :ast - def initialize(path, requirements, separators, anchor = true) + def self.build(path, requirements, separators, anchor = true) + parser = Journey::Parser.new + ast = parser.parse path + new ast, path, requirements, separators, anchor + end + + def initialize(ast, path, requirements, separators, anchor = true) + @ast = ast @path = path @requirements = requirements @separators = separators @anchor = anchor end - - def names - @path.scan(/:\w+/).map { |s| s.tr(':', '') } - end end end end diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 4cf7adfb3c..879e8daa33 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -62,13 +62,12 @@ module ActionDispatch class Mapping #:nodoc: IGNORE_OPTIONS = [:to, :as, :via, :on, :constraints, :defaults, :only, :except, :anchor, :shallow, :shallow_path, :shallow_prefix, :format] ANCHOR_CHARACTERS_REGEX = %r{\A(\\A|\^)|(\\Z|\\z|\$)\Z} - WILDCARD_PATH = %r{\*([^/\)]+)\)?$} - attr_reader :scope, :path, :options, :requirements, :conditions, :defaults + attr_reader :scope, :options, :requirements, :conditions, :defaults attr_reader :to, :default_controller, :default_action def initialize(set, scope, path, options) - @set, @scope, @path = set, scope, path + @set, @scope = set, scope @requirements, @conditions, @defaults = {}, {}, {} options = scope[:options].merge(options) if scope[:options] @@ -76,10 +75,12 @@ module ActionDispatch @default_controller = options[:controller] || scope[:controller] @default_action = options[:action] || scope[:action] - @options = normalize_options!(options) - normalize_path! - normalize_requirements! - normalize_conditions! + path = normalize_path! path, options[:format] + ast = path_ast path + path_params = path_params ast + @options = normalize_options!(options, path_params, ast) + normalize_requirements!(path_params) + normalize_conditions!(path_params, path, ast) normalize_defaults! end @@ -89,35 +90,33 @@ module ActionDispatch private - def normalize_path! - raise ArgumentError, "path is required" if @path.blank? - @path = Mapper.normalize_path(@path) + def normalize_path!(path, format) + raise ArgumentError, "path is required" if path.blank? + path = Mapper.normalize_path(path) - if required_format? - @path = "#{@path}.:format" - elsif optional_format? - @path = "#{@path}(.:format)" + if format == true + "#{path}.:format" + elsif optional_format?(path, format) + "#{path}(.:format)" + else + path end end - def required_format? - options[:format] == true - end - - def optional_format? - options[:format] != false && !path.include?(':format') && !path.end_with?('/') + def optional_format?(path, format) + format != false && !path.include?(':format') && !path.end_with?('/') end - def normalize_options!(options) - path_without_format = path.sub(/\(\.:format\)$/, '') - + def normalize_options!(options, path_params, path_ast) # Add a constraint for wildcard route to make it non-greedy and match the # optional format part of the route by default - if path_without_format.match(WILDCARD_PATH) && options[:format] != false - options[$1.to_sym] ||= /.+?/ + if options[:format] != false + path_ast.grep(Journey::Nodes::Star) do |node| + options[node.name.to_sym] ||= /.+?/ + end end - if path_without_format.match(':controller') + if path_params.include?(:controller) raise ArgumentError, ":controller segment is not allowed within a namespace block" if scope[:module] # Add a default constraint for :controller path segments that matches namespaced @@ -127,12 +126,16 @@ module ActionDispatch options[:controller] ||= /.+?/ end - options.merge!(default_controller_and_action) + if to.respond_to? :call + options + else + options.merge!(default_controller_and_action(path_params)) + end end - def normalize_requirements! + def normalize_requirements!(path_params) constraints.each do |key, requirement| - next unless segment_keys.include?(key) || key == :controller + next unless path_params.include?(key) || key == :controller verify_regexp_requirement(requirement) if requirement.is_a?(Regexp) @requirements[key] = requirement end @@ -189,18 +192,19 @@ module ActionDispatch end end - def normalize_conditions! + def normalize_conditions!(path_params, path, ast) @conditions[:path_info] = path + @conditions[:parsed_path_info] = ast constraints.each do |key, condition| - unless segment_keys.include?(key) || key == :controller + unless path_params.include?(key) || key == :controller @conditions[key] = condition end end required_defaults = [] options.each do |key, required_default| - unless segment_keys.include?(key) || IGNORE_OPTIONS.include?(key) || Regexp === required_default + unless path_params.include?(key) || IGNORE_OPTIONS.include?(key) || Regexp === required_default required_defaults << key end end @@ -236,55 +240,61 @@ module ActionDispatch end end - def default_controller_and_action - if to.respond_to?(:call) - { } - else - if to.is_a?(String) - controller, action = to.split('#') - elsif to.is_a?(Symbol) - action = to.to_s - end - - controller ||= default_controller - action ||= default_action + def default_controller_and_action(path_params) + controller, action = get_controller_and_action(default_controller, + default_action, + to, + @scope[:module] + ) - if @scope[:module] && !controller.is_a?(Regexp) - if controller =~ %r{\A/} - controller = controller[1..-1] - else - controller = [@scope[:module], controller].compact.join("/").presence - end - end + hash = check_part(:controller, controller, path_params, {}) do |part| + translate_controller(part) { + message = "'#{part}' is not a supported controller name. This can lead to potential routing problems." + message << " See http://guides.rubyonrails.org/routing.html#specifying-a-controller-to-use" - if controller.is_a?(String) && controller =~ %r{\A/} - raise ArgumentError, "controller name should not start with a slash" - end + raise ArgumentError, message + } + end - controller = controller.to_s unless controller.is_a?(Regexp) - action = action.to_s unless action.is_a?(Regexp) + check_part(:action, action, path_params, hash) { |part| + part.is_a?(Regexp) ? part : part.to_s + } + end - if controller.blank? && segment_keys.exclude?(:controller) - message = "Missing :controller key on routes definition, please check your routes." + def check_part(name, part, path_params, hash) + if part + hash[name] = yield(part) + else + unless path_params.include?(name) + message = "Missing :#{name} key on routes definition, please check your routes." raise ArgumentError, message end + end + hash + end - if action.blank? && segment_keys.exclude?(:action) - message = "Missing :action key on routes definition, please check your routes." - raise ArgumentError, message - end + def get_controller_and_action(controller, action, to, modyoule) + case to + when Symbol then action = to.to_s + when /#/ then controller, action = to.split('#') + when String then controller = to + end - if controller.is_a?(String) && controller !~ /\A[a-z_0-9\/]*\z/ - message = "'#{controller}' is not a supported controller name. This can lead to potential routing problems." - message << " See http://guides.rubyonrails.org/routing.html#specifying-a-controller-to-use" - raise ArgumentError, message + if modyoule && !controller.is_a?(Regexp) + if controller =~ %r{\A/} + controller = controller[1..-1] + else + controller = [modyoule, controller].compact.join("/") end - - hash = {} - hash[:controller] = controller unless controller.blank? - hash[:action] = action unless action.blank? - hash end + [controller, action] + end + + def translate_controller(controller) + return controller if Regexp === controller + return controller.to_s if controller =~ /\A[a-z_0-9][a-z_0-9\/]*\z/ + + yield end def blocks @@ -307,16 +317,13 @@ module ActionDispatch end end - def segment_keys - @segment_keys ||= path_pattern.names.map{ |s| s.to_sym } - end - - def path_pattern - Journey::Path::Pattern.new(strexp) + def path_params(ast) + ast.grep(Journey::Nodes::Symbol).map { |n| n.name.to_sym } end - def strexp - Journey::Router::Strexp.compile(path, requirements, SEPARATORS) + def path_ast(path) + parser = Journey::Parser.new + parser.parse path end def dispatcher diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index 3ca5abf0fd..bdda802195 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -418,7 +418,9 @@ module ActionDispatch "http://guides.rubyonrails.org/routing.html#restricting-the-routes-created" end - path = build_path(conditions.delete(:path_info), requirements, SEPARATORS, anchor) + path = conditions.delete :path_info + ast = conditions.delete :parsed_path_info + path = build_path(path, ast, requirements, SEPARATORS, anchor) conditions = build_conditions(conditions, path.names.map { |x| x.to_sym }) route = @set.add_route(app, path, conditions, defaults, name) @@ -426,8 +428,9 @@ module ActionDispatch route end - def build_path(path, requirements, separators, anchor) + def build_path(path, ast, requirements, separators, anchor) strexp = Journey::Router::Strexp.new( + ast, path, requirements, SEPARATORS, diff --git a/actionpack/test/dispatch/mapper_test.rb b/actionpack/test/dispatch/mapper_test.rb index 58457b0c28..e3dcf9b88a 100644 --- a/actionpack/test/dispatch/mapper_test.rb +++ b/actionpack/test/dispatch/mapper_test.rb @@ -72,7 +72,7 @@ module ActionDispatch mapper = Mapper.new fakeset mapper.get '/*path/foo/:bar', :to => 'pages#show' assert_equal '/*path/foo/:bar(.:format)', fakeset.conditions.first[:path_info] - assert_nil fakeset.requirements.first[:path] + assert_equal(/.+?/, fakeset.requirements.first[:path]) end def test_map_wildcard_with_multiple_wildcard @@ -80,7 +80,7 @@ module ActionDispatch mapper = Mapper.new fakeset mapper.get '/*foo/*bar', :to => 'pages#show' assert_equal '/*foo/*bar(.:format)', fakeset.conditions.first[:path_info] - assert_nil fakeset.requirements.first[:foo] + assert_equal(/.+?/, fakeset.requirements.first[:foo]) assert_equal(/.+?/, fakeset.requirements.first[:bar]) end diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index c9777ae71f..d6477e19bb 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -99,6 +99,16 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end end + def test_namespace_without_controller_segment + draw do + namespace :admin do + get 'hello/:controllers/:action' + end + end + get '/admin/hello/foo/new' + assert_equal 'foo', @request.params["controllers"] + end + def test_session_singleton_resource draw do resource :session do @@ -3137,6 +3147,18 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest assert_equal '/foo', foo_root_path end + def test_namespace_as_controller + draw do + namespace :foo do + get '/', to: '/bar#index', as: 'root' + end + end + + get '/foo' + assert_equal 'bar#index', @response.body + assert_equal '/foo', foo_root_path + end + def test_trailing_slash draw do resources :streams @@ -3546,6 +3568,16 @@ class TestNamespaceWithControllerOption < ActionDispatch::IntegrationTest assert_match "'Admin::StorageFiles' is not a supported controller name", e.message end + + def test_warn_with_ruby_constant_syntax_no_colons + e = assert_raise(ArgumentError) do + draw do + resources :storage_files, :controller => 'Admin' + end + end + + assert_match "'Admin' is not a supported controller name", e.message + end end class TestDefaultScope < ActionDispatch::IntegrationTest diff --git a/actionpack/test/journey/path/pattern_test.rb b/actionpack/test/journey/path/pattern_test.rb index ce02104181..9dfdfc23ed 100644 --- a/actionpack/test/journey/path/pattern_test.rb +++ b/actionpack/test/journey/path/pattern_test.rb @@ -18,7 +18,7 @@ module ActionDispatch '/:controller/*foo/bar' => %r{\A/(#{x})/(.+)/bar\Z}, }.each do |path, expected| define_method(:"test_to_regexp_#{path}") do - strexp = Router::Strexp.new( + strexp = Router::Strexp.build( path, { :controller => /.+/ }, ["/", ".", "?"] @@ -41,7 +41,7 @@ module ActionDispatch '/:controller/*foo/bar' => %r{\A/(#{x})/(.+)/bar}, }.each do |path, expected| define_method(:"test_to_non_anchored_regexp_#{path}") do - strexp = Router::Strexp.new( + strexp = Router::Strexp.build( path, { :controller => /.+/ }, ["/", ".", "?"], @@ -65,7 +65,7 @@ module ActionDispatch '/:controller/*foo/bar' => %w{ controller foo }, }.each do |path, expected| define_method(:"test_names_#{path}") do - strexp = Router::Strexp.new( + strexp = Router::Strexp.build( path, { :controller => /.+/ }, ["/", ".", "?"] @@ -75,12 +75,8 @@ module ActionDispatch end end - def test_to_raise_exception_with_bad_expression - assert_raise(ArgumentError, "Bad expression: []") { Pattern.new [] } - end - def test_to_regexp_with_extended_group - strexp = Router::Strexp.new( + strexp = Router::Strexp.build( '/page/:name', { :name => / #ROFL @@ -101,13 +97,13 @@ module ActionDispatch ['/:foo(/:bar)', %w{ bar }], ['/:foo(/:bar)/:lol(/:baz)', %w{ bar baz }], ].each do |pattern, list| - path = Pattern.new pattern + path = Pattern.from_string pattern assert_equal list.sort, path.optional_names.sort end end def test_to_regexp_match_non_optional - strexp = Router::Strexp.new( + strexp = Router::Strexp.build( '/:name', { :name => /\d+/ }, ["/", ".", "?"] @@ -118,7 +114,7 @@ module ActionDispatch end def test_to_regexp_with_group - strexp = Router::Strexp.new( + strexp = Router::Strexp.build( '/page/:name', { :name => /(tender|love)/ }, ["/", ".", "?"] @@ -131,7 +127,7 @@ module ActionDispatch def test_ast_sets_regular_expressions requirements = { :name => /(tender|love)/, :value => /./ } - strexp = Router::Strexp.new( + strexp = Router::Strexp.build( '/page/:name/:value', requirements, ["/", ".", "?"] @@ -148,7 +144,7 @@ module ActionDispatch end def test_match_data_with_group - strexp = Router::Strexp.new( + strexp = Router::Strexp.build( '/page/:name', { :name => /(tender|love)/ }, ["/", ".", "?"] @@ -160,7 +156,7 @@ module ActionDispatch end def test_match_data_with_multi_group - strexp = Router::Strexp.new( + strexp = Router::Strexp.build( '/page/:name/:id', { :name => /t(((ender|love)))()/ }, ["/", ".", "?"] @@ -175,7 +171,7 @@ module ActionDispatch def test_star_with_custom_re z = /\d+/ - strexp = Router::Strexp.new( + strexp = Router::Strexp.build( '/page/*foo', { :foo => z }, ["/", ".", "?"] @@ -185,7 +181,7 @@ module ActionDispatch end def test_insensitive_regexp_with_group - strexp = Router::Strexp.new( + strexp = Router::Strexp.build( '/page/:name/aaron', { :name => /(tender|love)/i }, ["/", ".", "?"] @@ -197,7 +193,7 @@ module ActionDispatch end def test_to_regexp_with_strexp - strexp = Router::Strexp.new('/:controller', { }, ["/", ".", "?"]) + strexp = Router::Strexp.build('/:controller', { }, ["/", ".", "?"]) path = Pattern.new strexp x = %r{\A/([^/.?]+)\Z} @@ -205,20 +201,20 @@ module ActionDispatch end def test_to_regexp_defaults - path = Pattern.new '/:controller(/:action(/:id))' + path = Pattern.from_string '/:controller(/:action(/:id))' expected = %r{\A/([^/.?]+)(?:/([^/.?]+)(?:/([^/.?]+))?)?\Z} assert_equal expected, path.to_regexp end def test_failed_match - path = Pattern.new '/:controller(/:action(/:id(.:format)))' + path = Pattern.from_string '/:controller(/:action(/:id(.:format)))' uri = 'content' assert_not path =~ uri end def test_match_controller - path = Pattern.new '/:controller(/:action(/:id(.:format)))' + path = Pattern.from_string '/:controller(/:action(/:id(.:format)))' uri = '/content' match = path =~ uri @@ -230,7 +226,7 @@ module ActionDispatch end def test_match_controller_action - path = Pattern.new '/:controller(/:action(/:id(.:format)))' + path = Pattern.from_string '/:controller(/:action(/:id(.:format)))' uri = '/content/list' match = path =~ uri @@ -242,7 +238,7 @@ module ActionDispatch end def test_match_controller_action_id - path = Pattern.new '/:controller(/:action(/:id(.:format)))' + path = Pattern.from_string '/:controller(/:action(/:id(.:format)))' uri = '/content/list/10' match = path =~ uri @@ -254,7 +250,7 @@ module ActionDispatch end def test_match_literal - path = Path::Pattern.new "/books(/:action(.:format))" + path = Path::Pattern.from_string "/books(/:action(.:format))" uri = '/books' match = path =~ uri @@ -264,7 +260,7 @@ module ActionDispatch end def test_match_literal_with_action - path = Path::Pattern.new "/books(/:action(.:format))" + path = Path::Pattern.from_string "/books(/:action(.:format))" uri = '/books/list' match = path =~ uri @@ -274,7 +270,7 @@ module ActionDispatch end def test_match_literal_with_action_and_format - path = Path::Pattern.new "/books(/:action(.:format))" + path = Path::Pattern.from_string "/books(/:action(.:format))" uri = '/books/list.rss' match = path =~ uri diff --git a/actionpack/test/journey/route_test.rb b/actionpack/test/journey/route_test.rb index cbe6284714..21d867aca0 100644 --- a/actionpack/test/journey/route_test.rb +++ b/actionpack/test/journey/route_test.rb @@ -5,7 +5,7 @@ module ActionDispatch class TestRoute < ActiveSupport::TestCase def test_initialize app = Object.new - path = Path::Pattern.new '/:controller(/:action(/:id(.:format)))' + path = Path::Pattern.from_string '/:controller(/:action(/:id(.:format)))' defaults = {} route = Route.new("name", app, path, {}, defaults) @@ -16,7 +16,7 @@ module ActionDispatch def test_route_adds_itself_as_memo app = Object.new - path = Path::Pattern.new '/:controller(/:action(/:id(.:format)))' + path = Path::Pattern.from_string '/:controller(/:action(/:id(.:format)))' defaults = {} route = Route.new("name", app, path, {}, defaults) @@ -26,21 +26,21 @@ module ActionDispatch end def test_ip_address - path = Path::Pattern.new '/messages/:id(.:format)' + path = Path::Pattern.from_string '/messages/:id(.:format)' 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.new '/messages/:id(.:format)' + path = Path::Pattern.from_string '/messages/:id(.:format)' route = Route.new("name", nil, path, {}, { :controller => 'foo', :action => 'bar' }) assert_equal(//, route.ip) end def test_format_with_star - path = Path::Pattern.new '/:controller/*extra' + path = Path::Pattern.from_string '/:controller/*extra' route = Route.new("name", nil, path, {}, { :controller => 'foo', :action => 'bar' }) assert_equal '/foo/himom', route.format({ @@ -50,7 +50,7 @@ module ActionDispatch end def test_connects_all_match - path = Path::Pattern.new '/:controller(/:action(/:id(.:format)))' + path = Path::Pattern.from_string '/:controller(/:action(/:id(.:format)))' route = Route.new("name", nil, path, {:action => 'bar'}, { :controller => 'foo' }) assert_equal '/foo/bar/10', route.format({ @@ -61,21 +61,21 @@ module ActionDispatch end def test_extras_are_not_included_if_optional - path = Path::Pattern.new '/page/:id(/:action)' + path = Path::Pattern.from_string '/page/:id(/:action)' 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.new '(/sections/:section)/pages/:id' + path = Path::Pattern.from_string '(/sections/:section)/pages/:id' 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.new '(/sections/:section)/pages/:id' + path = Path::Pattern.from_string '(/sections/:section)/pages/:id' route = Route.new("name", nil, path, { }, { :action => 'show' }) assert_equal '/pages/10', route.format({:id => 10, :section => nil}) @@ -85,10 +85,10 @@ module ActionDispatch constraints = {:required_defaults => [:controller, :action]} defaults = {:controller=>"pages", :action=>"show"} - path = Path::Pattern.new "/page/:id(/:action)(.:format)" + path = Path::Pattern.from_string "/page/:id(/:action)(.:format)" specific = Route.new "name", nil, path, constraints, defaults - path = Path::Pattern.new "/:controller(/:action(/:id))(.:format)" + path = Path::Pattern.from_string "/:controller(/:action(/:id))(.:format)" generic = Route.new "name", nil, path, constraints knowledge = {:id=>20, :controller=>"pages", :action=>"show"} diff --git a/actionpack/test/journey/router/strexp_test.rb b/actionpack/test/journey/router/strexp_test.rb deleted file mode 100644 index 7ccdfb7b4d..0000000000 --- a/actionpack/test/journey/router/strexp_test.rb +++ /dev/null @@ -1,32 +0,0 @@ -require 'abstract_unit' - -module ActionDispatch - module Journey - class Router - class TestStrexp < ActiveSupport::TestCase - def test_many_names - exp = Strexp.new( - "/:controller(/:action(/:id(.:format)))", - {:controller=>/.+?/}, - ["/", ".", "?"], - true) - - assert_equal ["controller", "action", "id", "format"], exp.names - end - - def test_names - { - "/bar(.:format)" => %w{ format }, - ":format" => %w{ format }, - ":format-" => %w{ format }, - ":format0" => %w{ format0 }, - ":format1,:format2" => %w{ format1 format2 }, - }.each do |string, expected| - exp = Strexp.new(string, {}, ["/", ".", "?"]) - assert_equal expected, exp.names - end - end - end - end - end -end diff --git a/actionpack/test/journey/router_test.rb b/actionpack/test/journey/router_test.rb index f298808be8..e092432b01 100644 --- a/actionpack/test/journey/router_test.rb +++ b/actionpack/test/journey/router_test.rb @@ -32,7 +32,7 @@ module ActionDispatch def test_dashes router = Router.new(routes) - exp = Router::Strexp.new '/foo-bar-baz', {}, ['/.?'] + exp = Router::Strexp.build '/foo-bar-baz', {}, ['/.?'] path = Path::Pattern.new exp routes.add_route nil, path, {}, {:id => nil}, {} @@ -49,7 +49,7 @@ module ActionDispatch router = Router.new(routes) #match the escaped version of /ほげ - exp = Router::Strexp.new '/%E3%81%BB%E3%81%92', {}, ['/.?'] + exp = Router::Strexp.build '/%E3%81%BB%E3%81%92', {}, ['/.?'] path = Path::Pattern.new exp routes.add_route nil, path, {}, {:id => nil}, {} @@ -68,7 +68,7 @@ module ActionDispatch requirements = { :hello => /world/ } - exp = Router::Strexp.new '/foo(/:id)', {}, ['/.?'] + exp = Router::Strexp.build '/foo(/:id)', {}, ['/.?'] path = Path::Pattern.new exp routes.add_route nil, path, requirements, {:id => nil}, {} @@ -88,7 +88,7 @@ module ActionDispatch requirements = { :hello => /mom/ } - exp = Router::Strexp.new '/foo(/:id)', {}, ['/.?'] + exp = Router::Strexp.build '/foo(/:id)', {}, ['/.?'] path = Path::Pattern.new exp router.routes.add_route nil, path, requirements, {:id => nil}, {} @@ -115,7 +115,7 @@ module ActionDispatch def test_request_class_overrides_path_info router = Router.new(routes) - exp = Router::Strexp.new '/bar', {}, ['/.?'] + exp = Router::Strexp.build '/bar', {}, ['/.?'] path = Path::Pattern.new exp routes.add_route nil, path, {}, {}, {} @@ -133,8 +133,8 @@ module ActionDispatch def test_regexp_first_precedence add_routes @router, [ - Router::Strexp.new("/whois/:domain", {:domain => /\w+\.[\w\.]+/}, ['/', '.', '?']), - Router::Strexp.new("/whois/:id(.:format)", {}, ['/', '.', '?']) + Router::Strexp.build("/whois/:domain", {:domain => /\w+\.[\w\.]+/}, ['/', '.', '?']), + Router::Strexp.build("/whois/:id(.:format)", {}, ['/', '.', '?']) ] env = rails_env 'PATH_INFO' => '/whois/example.com' @@ -152,7 +152,7 @@ module ActionDispatch def test_required_parts_verified_are_anchored add_routes @router, [ - Router::Strexp.new("/foo/:id", { :id => /\d/ }, ['/', '.', '?'], false) + Router::Strexp.build("/foo/:id", { :id => /\d/ }, ['/', '.', '?'], false) ] assert_raises(ActionController::UrlGenerationError) do @@ -162,7 +162,7 @@ module ActionDispatch def test_required_parts_are_verified_when_building add_routes @router, [ - Router::Strexp.new("/foo/:id", { :id => /\d+/ }, ['/', '.', '?'], false) + Router::Strexp.build("/foo/:id", { :id => /\d+/ }, ['/', '.', '?'], false) ] path, _ = @formatter.generate(nil, { :id => '10' }, { }) @@ -175,7 +175,7 @@ module ActionDispatch def test_only_required_parts_are_verified add_routes @router, [ - Router::Strexp.new("/foo(/:id)", {:id => /\d/}, ['/', '.', '?'], false) + Router::Strexp.build("/foo(/:id)", {:id => /\d/}, ['/', '.', '?'], false) ] path, _ = @formatter.generate(nil, { :id => '10' }, { }) @@ -190,7 +190,7 @@ module ActionDispatch def test_knows_what_parts_are_missing_from_named_route route_name = "gorby_thunderhorse" - pattern = Router::Strexp.new("/foo/:id", { :id => /\d+/ }, ['/', '.', '?'], false) + pattern = Router::Strexp.build("/foo/:id", { :id => /\d+/ }, ['/', '.', '?'], false) path = Path::Pattern.new pattern @router.routes.add_route nil, path, {}, {}, route_name @@ -213,7 +213,7 @@ module ActionDispatch route_set = Routing::RouteSet.new mapper = Routing::Mapper.new route_set - strexp = Router::Strexp.new("/", {}, ['/', '.', '?'], false) + strexp = Router::Strexp.build("/", {}, ['/', '.', '?'], false) path = Path::Pattern.new strexp app = lambda { |env| [200, {}, ['success!']] } mapper.get '/weblog', :to => app @@ -225,7 +225,7 @@ module ActionDispatch end def test_defaults_merge_correctly - path = Path::Pattern.new '/foo(/:id)' + path = Path::Pattern.from_string '/foo(/:id)' @router.routes.add_route nil, path, {}, {:id => nil}, {} env = rails_env 'PATH_INFO' => '/foo/10' @@ -241,7 +241,7 @@ module ActionDispatch def test_recognize_with_unbound_regexp add_routes @router, [ - Router::Strexp.new("/foo", { }, ['/', '.', '?'], false) + Router::Strexp.build("/foo", { }, ['/', '.', '?'], false) ] env = rails_env 'PATH_INFO' => '/foo/bar' @@ -254,7 +254,7 @@ module ActionDispatch def test_bound_regexp_keeps_path_info add_routes @router, [ - Router::Strexp.new("/foo", { }, ['/', '.', '?'], true) + Router::Strexp.build("/foo", { }, ['/', '.', '?'], true) ] env = rails_env 'PATH_INFO' => '/foo' @@ -308,7 +308,7 @@ module ActionDispatch end def test_nil_path_parts_are_ignored - path = Path::Pattern.new "/:controller(/:action(.:format))" + path = Path::Pattern.from_string "/:controller(/:action(.:format))" @router.routes.add_route @app, path, {}, {}, {} params = { :controller => "tasks", :format => nil } @@ -321,7 +321,7 @@ module ActionDispatch def test_generate_slash params = [ [:controller, "tasks"], [:action, "show"] ] - str = Router::Strexp.new("/", Hash[params], ['/', '.', '?'], true) + str = Router::Strexp.build("/", Hash[params], ['/', '.', '?'], true) path = Path::Pattern.new str @router.routes.add_route @app, path, {}, {}, {} @@ -331,7 +331,7 @@ module ActionDispatch end def test_generate_calls_param_proc - path = Path::Pattern.new '/:controller(/:action)' + path = Path::Pattern.from_string '/:controller(/:action)' @router.routes.add_route @app, path, {}, {}, {} parameterized = [] @@ -348,7 +348,7 @@ module ActionDispatch end def test_generate_id - path = Path::Pattern.new '/:controller(/:action)' + path = Path::Pattern.from_string '/:controller(/:action)' @router.routes.add_route @app, path, {}, {}, {} path, params = @formatter.generate( @@ -358,7 +358,7 @@ module ActionDispatch end def test_generate_escapes - path = Path::Pattern.new '/:controller(/:action)' + path = Path::Pattern.from_string '/:controller(/:action)' @router.routes.add_route @app, path, {}, {}, {} path, _ = @formatter.generate(nil, @@ -369,7 +369,7 @@ module ActionDispatch end def test_generate_escapes_with_namespaced_controller - path = Path::Pattern.new '/:controller(/:action)' + path = Path::Pattern.from_string '/:controller(/:action)' @router.routes.add_route @app, path, {}, {}, {} path, _ = @formatter.generate( @@ -380,7 +380,7 @@ module ActionDispatch end def test_generate_extra_params - path = Path::Pattern.new '/:controller(/:action)' + path = Path::Pattern.from_string '/:controller(/:action)' @router.routes.add_route @app, path, {}, {}, {} path, params = @formatter.generate( @@ -394,7 +394,7 @@ module ActionDispatch end def test_generate_uses_recall_if_needed - path = Path::Pattern.new '/:controller(/:action(/:id))' + path = Path::Pattern.from_string '/:controller(/:action(/:id))' @router.routes.add_route @app, path, {}, {}, {} path, params = @formatter.generate( @@ -406,7 +406,7 @@ module ActionDispatch end def test_generate_with_name - path = Path::Pattern.new '/:controller(/:action)' + path = Path::Pattern.from_string '/:controller(/:action)' @router.routes.add_route @app, path, {}, {}, {} path, params = @formatter.generate( @@ -423,7 +423,7 @@ module ActionDispatch '/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.new "/:controller(/:action(/:id))" + path = Path::Pattern.from_string "/:controller(/:action(/:id))" app = Object.new route = @router.routes.add_route(app, path, {}, {}, {}) @@ -445,7 +445,7 @@ module ActionDispatch :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.new '/:segment/*splat' + path = Path::Pattern.from_string '/:segment/*splat' app = Object.new route = @router.routes.add_route(app, path, {}, {}, {}) @@ -463,7 +463,7 @@ module ActionDispatch end def test_namespaced_controller - strexp = Router::Strexp.new( + strexp = Router::Strexp.build( "/:controller(/:action(/:id))", { :controller => /.+?/ }, ["/", ".", "?"] @@ -489,7 +489,7 @@ module ActionDispatch end def test_recognize_literal - path = Path::Pattern.new "/books(/:action(.:format))" + path = Path::Pattern.from_string "/books(/:action(.:format))" app = Object.new route = @router.routes.add_route(app, path, {}, {:controller => 'books'}) @@ -506,7 +506,7 @@ module ActionDispatch end def test_recognize_head_request_as_get_route - path = Path::Pattern.new "/books(/:action(.:format))" + path = Path::Pattern.from_string "/books(/:action(.:format))" app = Object.new conditions = { :request_method => 'GET' @@ -525,7 +525,7 @@ module ActionDispatch end def test_recognize_cares_about_verbs - path = Path::Pattern.new "/books(/:action(.:format))" + path = Path::Pattern.from_string "/books(/:action(.:format))" app = Object.new conditions = { :request_method => 'GET' @@ -553,7 +553,11 @@ module ActionDispatch def add_routes router, paths paths.each do |path| - path = Path::Pattern.new path + if String === path + path = Path::Pattern.from_string path + else + path = Path::Pattern.new path + end 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 25e0321d31..a4efc82b8c 100644 --- a/actionpack/test/journey/routes_test.rb +++ b/actionpack/test/journey/routes_test.rb @@ -5,7 +5,7 @@ module ActionDispatch class TestRoutes < ActiveSupport::TestCase def test_clear routes = Routes.new - exp = Router::Strexp.new '/foo(/:id)', {}, ['/.?'] + exp = Router::Strexp.build '/foo(/:id)', {}, ['/.?'] path = Path::Pattern.new exp requirements = { :hello => /world/ } @@ -18,7 +18,7 @@ module ActionDispatch def test_ast routes = Routes.new - path = Path::Pattern.new '/hello' + path = Path::Pattern.from_string '/hello' routes.add_route nil, path, {}, {}, {} ast = routes.ast @@ -28,7 +28,7 @@ module ActionDispatch def test_simulator_changes routes = Routes.new - path = Path::Pattern.new '/hello' + path = Path::Pattern.from_string '/hello' routes.add_route nil, path, {}, {}, {} sim = routes.simulator @@ -40,8 +40,8 @@ module ActionDispatch #def add_route app, path, conditions, defaults, name = nil routes = Routes.new - one = Path::Pattern.new '/hello' - two = Path::Pattern.new '/aaron' + one = Path::Pattern.from_string '/hello' + two = Path::Pattern.from_string '/aaron' routes.add_route nil, one, {}, {}, 'aaron' routes.add_route nil, two, {}, {}, 'aaron' |