Skip to content

Commit

Permalink
Added support for regexp flags like ignoring case in the :requirement…
Browse files Browse the repository at this point in the history
…s part of routes declarations (closes #11421) [NeilW]

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@9115 5ecf4fe2-1ee6-0310-87b1-e25e094e27de
  • Loading branch information
dhh committed Mar 28, 2008
1 parent 9300ebd commit db3a60e
Show file tree
Hide file tree
Showing 6 changed files with 190 additions and 7 deletions.
2 changes: 2 additions & 0 deletions actionpack/CHANGELOG
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
*SVN*

* Added support for regexp flags like ignoring case in the :requirements part of routes declarations #11421 [NeilW]

* Fixed that ActionController::Base#read_multipart would fail if boundary was exactly 10240 bytes #10886 [ariejan]

* Fixed HTML::Tokenizer (used in sanitize helper) didn't handle unclosed CDATA tags #10071 [esad, packagethief]
Expand Down
18 changes: 18 additions & 0 deletions actionpack/lib/action_controller/routing.rb
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,24 @@ module ActionController
# map.geocode 'geocode/:postalcode', :controller => 'geocode',
# :action => 'show', :requirements => { :postalcode => /\d{5}(-\d{4})?/ }
#
# Formats can include the 'ignorecase' and 'extended syntax' regular
# expression modifiers:
#
# map.geocode 'geocode/:postalcode', :controller => 'geocode',
# :action => 'show', :postalcode => /hx\d\d\s\d[a-z]{2}/i
#
# map.geocode 'geocode/:postalcode', :controller => 'geocode',
# :action => 'show',:requirements => {
# :postalcode => /# Postcode format
# \d{5} #Prefix
# (-\d{4})? #Suffix
# /x
# }
#
# Using the multiline match modifier will raise an ArgumentError.
# Encoding regular expression modifiers are silently ignored. The
# match will always use the default encoding or ASCII.
#
# == Route globbing
#
# Specifying <tt>*[string]</tt> as part of a rule like:
Expand Down
9 changes: 8 additions & 1 deletion actionpack/lib/action_controller/routing/builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ def interval_regexp
Regexp.new "(.*?)(#{separators.source}|$)"
end

def multiline_regexp?(expression)
expression.options & Regexp::MULTILINE == Regexp::MULTILINE
end

# Accepts a "route path" (a string defining a route), and returns the array
# of segments that corresponds to it. Note that the segment array is only
# partially initialized--the defaults and requirements, for instance, need
Expand Down Expand Up @@ -99,6 +103,9 @@ def assign_route_options(segments, defaults, requirements)
if requirement.source =~ %r{\A(\\A|\^)|(\\Z|\\z|\$)\Z}
raise ArgumentError, "Regexp anchor characters are not allowed in routing requirements: #{requirement.inspect}"
end
if multiline_regexp?(requirement)
raise ArgumentError, "Regexp multiline option not allowed in routing requirements: #{requirement.inspect}"
end
segment.regexp = requirement
else
route_requirements[key] = requirement
Expand Down Expand Up @@ -194,4 +201,4 @@ def build(path, options)
end
end
end
end
end
4 changes: 2 additions & 2 deletions actionpack/lib/action_controller/routing/route.rb
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ def generation_extraction
def generation_requirements
requirement_conditions = requirements.collect do |key, req|
if req.is_a? Regexp
value_regexp = Regexp.new "\\A#{req.source}\\Z"
value_regexp = Regexp.new "\\A#{req.to_s}\\Z"
"hash[:#{key}] && #{value_regexp.inspect} =~ options[:#{key}]"
else
"hash[:#{key}] == #{req.inspect}"
Expand Down Expand Up @@ -237,4 +237,4 @@ def requirement_for(key)

end
end
end
end
18 changes: 16 additions & 2 deletions actionpack/lib/action_controller/routing/segments.rb
Original file line number Diff line number Diff line change
Expand Up @@ -171,10 +171,19 @@ def string_structure(prior_segments)
end

def value_regexp
Regexp.new "\\A#{regexp.source}\\Z" if regexp
Regexp.new "\\A#{regexp.to_s}\\Z" if regexp
end

def regexp_chunk
regexp ? "(#{regexp.source})" : "([^#{Routing::SEPARATORS.join}]+)"
if regexp
if regexp_has_modifiers?
"(#{regexp.to_s})"
else
"(#{regexp.source})"
end
else
"([^#{Routing::SEPARATORS.join}]+)"
end
end

def build_pattern(pattern)
Expand All @@ -183,6 +192,7 @@ def build_pattern(pattern)
pattern = "#{chunk}#{pattern}"
optional? ? Regexp.optionalize(pattern) : pattern
end

def match_extraction(next_capture)
# All non code-related keys (such as :id, :slug) are URI-unescaped as
# path parameters.
Expand All @@ -201,6 +211,10 @@ def optionality_implied?
[:action, :id].include? key
end

def regexp_has_modifiers?
regexp.options & (Regexp::IGNORECASE | Regexp::EXTENDED) != 0
end

end

class ControllerSegment < DynamicSegment #:nodoc:
Expand Down
146 changes: 144 additions & 2 deletions actionpack/test/controller/routing_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -936,6 +936,16 @@ def test_optionality_implied
a_segment.key = :action
assert a_segment.optionality_implied?
end

def test_modifiers_must_be_handled_sensibly
a_segment = ROUTING::DynamicSegment.new
a_segment.regexp = /david|jamis/i
assert_equal "((?i-mx:david|jamis))stuff", a_segment.build_pattern('stuff')
a_segment.regexp = /david|jamis/x
assert_equal "((?x-mi:david|jamis))stuff", a_segment.build_pattern('stuff')
a_segment.regexp = /david|jamis/
assert_equal "(david|jamis)stuff", a_segment.build_pattern('stuff')
end
end

class ControllerSegmentTest < Test::Unit::TestCase
Expand Down Expand Up @@ -1723,7 +1733,7 @@ def test_route_requirements_with_anchor_chars_are_invalid
end
end
end

def test_non_path_route_requirements_match_all
set.draw do |map|
map.connect 'page/37s', :controller => 'pages', :action => 'show', :name => /(jamis|david)/
Expand Down Expand Up @@ -2110,6 +2120,138 @@ def test_setting_root_in_namespace_using_string
end
end
end

def test_route_requirements_with_unsupported_regexp_options_must_error
assert_raises ArgumentError do
set.draw do |map|
map.connect 'page/:name', :controller => 'pages',
:action => 'show',
:requirements => {:name => /(david|jamis)/m}
end
end
end

def test_route_requirements_with_supported_options_must_not_error
assert_nothing_raised do
set.draw do |map|
map.connect 'page/:name', :controller => 'pages',
:action => 'show',
:requirements => {:name => /(david|jamis)/i}
end
end
assert_nothing_raised do
set.draw do |map|
map.connect 'page/:name', :controller => 'pages',
:action => 'show',
:requirements => {:name => / # Desperately overcommented regexp
( #Either
david #The Creator
| #Or
jamis #The Deployer
)/x}
end
end
end

def test_route_requirement_recognize_with_ignore_case
set.draw do |map|
map.connect 'page/:name', :controller => 'pages',
:action => 'show',
:requirements => {:name => /(david|jamis)/i}
end
assert_equal({:controller => 'pages', :action => 'show', :name => 'jamis'}, set.recognize_path('/page/jamis'))
assert_raises ActionController::RoutingError do
set.recognize_path('/page/davidjamis')
end
assert_equal({:controller => 'pages', :action => 'show', :name => 'DAVID'}, set.recognize_path('/page/DAVID'))
end

def test_route_requirement_generate_with_ignore_case
set.draw do |map|
map.connect 'page/:name', :controller => 'pages',
:action => 'show',
:requirements => {:name => /(david|jamis)/i}
end
url = set.generate({:controller => 'pages', :action => 'show', :name => 'david'})
assert_equal "/page/david", url
assert_raises ActionController::RoutingError do
url = set.generate({:controller => 'pages', :action => 'show', :name => 'davidjamis'})
end
url = set.generate({:controller => 'pages', :action => 'show', :name => 'JAMIS'})
assert_equal "/page/JAMIS", url
end

def test_route_requirement_recognize_with_extended_syntax
set.draw do |map|
map.connect 'page/:name', :controller => 'pages',
:action => 'show',
:requirements => {:name => / # Desperately overcommented regexp
( #Either
david #The Creator
| #Or
jamis #The Deployer
)/x}
end
assert_equal({:controller => 'pages', :action => 'show', :name => 'jamis'}, set.recognize_path('/page/jamis'))
assert_equal({:controller => 'pages', :action => 'show', :name => 'david'}, set.recognize_path('/page/david'))
assert_raises ActionController::RoutingError do
set.recognize_path('/page/david #The Creator')
end
assert_raises ActionController::RoutingError do
set.recognize_path('/page/David')
end
end

def test_route_requirement_generate_with_extended_syntax
set.draw do |map|
map.connect 'page/:name', :controller => 'pages',
:action => 'show',
:requirements => {:name => / # Desperately overcommented regexp
( #Either
david #The Creator
| #Or
jamis #The Deployer
)/x}
end
url = set.generate({:controller => 'pages', :action => 'show', :name => 'david'})
assert_equal "/page/david", url
assert_raises ActionController::RoutingError do
url = set.generate({:controller => 'pages', :action => 'show', :name => 'davidjamis'})
end
assert_raises ActionController::RoutingError do
url = set.generate({:controller => 'pages', :action => 'show', :name => 'JAMIS'})
end
end

def test_route_requirement_generate_with_xi_modifiers
set.draw do |map|
map.connect 'page/:name', :controller => 'pages',
:action => 'show',
:requirements => {:name => / # Desperately overcommented regexp
( #Either
david #The Creator
| #Or
jamis #The Deployer
)/xi}
end
url = set.generate({:controller => 'pages', :action => 'show', :name => 'JAMIS'})
assert_equal "/page/JAMIS", url
end

def test_route_requirement_recognize_with_xi_modifiers
set.draw do |map|
map.connect 'page/:name', :controller => 'pages',
:action => 'show',
:requirements => {:name => / # Desperately overcommented regexp
( #Either
david #The Creator
| #Or
jamis #The Deployer
)/xi}
end
assert_equal({:controller => 'pages', :action => 'show', :name => 'JAMIS'}, set.recognize_path('/page/JAMIS'))
end


end

Expand Down Expand Up @@ -2190,7 +2332,7 @@ def test_routing_helper_module
ActionController::Routing::Routes.install_helpers c
assert c.ancestors.include?(h)
end

end

uses_mocha 'route loading' do
Expand Down

0 comments on commit db3a60e

Please sign in to comment.