Fix requirements regexp for path segments

Signed-off-by: Michael Koziarski <michael@koziarski.com>
[#1772 state:committed]

2 files changed, 36 insertions, 16 deletions

diff --git a/actionpack/lib/action_controller/routing/segments.rb b/actionpack/lib/action_controller/routing/segments.rb
index a5e3b95f31..129e87c139 100644
--- a/actionpack/lib/action_controller/routing/segments.rb
+++ b/actionpack/lib/action_controller/routing/segments.rb
@@ -191,23 +191,19 @@ module ActionController
       end
 
       def regexp_chunk
-        if regexp
-          if regexp_has_modifiers?
-            "(#{regexp.to_s})"
-          else
-            "(#{regexp.source})"
-          end
-        else
-          "([^#{Routing::SEPARATORS.join}]+)"
-        end
+        regexp ? regexp_string : default_regexp_chunk
+      end
+
+      def regexp_string
+        regexp_has_modifiers? ? "(#{regexp.to_s})" : "(#{regexp.source})"
+      end
+
+      def default_regexp_chunk
+        "([^#{Routing::SEPARATORS.join}]+)"
       end
 
       def number_of_captures
-        if regexp
-          regexp.number_of_captures + 1
-        else
-          1
-        end
+        regexp ? regexp.number_of_captures + 1 : 1
       end
 
       def build_pattern(pattern)
@@ -285,8 +281,8 @@ module ActionController
         "params[:#{key}] = PathSegment::Result.new_escaped((match[#{next_capture}]#{" || " + default.inspect if default}).split('/'))#{" if match[" + next_capture + "]" if !default}"
       end
 
-      def regexp_chunk
-        regexp || "(.*)"
+      def default_regexp_chunk
+        "(.*)"
       end
 
       def number_of_captures
diff --git a/actionpack/test/controller/routing_test.rb b/actionpack/test/controller/routing_test.rb
index d3fe62e28e..13ba0c30dd 100644
--- a/actionpack/test/controller/routing_test.rb
+++ b/actionpack/test/controller/routing_test.rb
@@ -340,6 +340,30 @@ class ControllerSegmentTest < Test::Unit::TestCase
   end
 end
 
+class PathSegmentTest < Test::Unit::TestCase
+  def segment(options = {})
+    unless @segment
+      @segment = ROUTING::PathSegment.new(:path, options)
+    end
+    @segment
+  end
+
+  def test_regexp_chunk_should_return_string
+    segment = segment(:regexp => /[a-z]+/)
+    assert_kind_of String, segment.regexp_chunk
+  end
+
+  def test_regexp_chunk_should_be_wrapped_with_parenthesis
+    segment = segment(:regexp => /[a-z]+/)
+    assert_equal "([a-z]+)", segment.regexp_chunk
+  end
+
+  def test_regexp_chunk_should_respect_options
+    segment = segment(:regexp => /[a-z]+/i)
+    assert_equal "((?i-mx:[a-z]+))", segment.regexp_chunk
+  end
+end
+
 class RouteBuilderTest < Test::Unit::TestCase
   def builder
     @builder ||= ROUTING::RouteBuilder.new