Merge branch 'mapper'

* mapper: (34 commits)
  no more is_a checks on instantiation
  Path::Pattern is instantiated internally, so make the contructor require a strexp object
  Strexp#names is only used in a test, so rm
  pass the parsed path from mapper to the Strexp
  add an alternate constructor to Strexp that takes a string
  ask the strexp for the ast
  remove dead code
  disconnect path from the instance
  reuse the ast we already made
  use a parser to extract the group parts from the path
  pass the parsed parameters through the methods so we don't reparse or require caching code
  "controllers" should be a valid path name
  controllers with slash names are also not supported, so we can reuse the message
  only validate controllers
  golf down a bit
  only error handling between controller and action is the same
  add a test for controllers without colons
  move nil check to a method that yields to a block if the value is not nil
  translate action / controller to the desired object
  only one nil check on the action variable
  ...

actionpack/lib/action_dispatch/journey/nodes/node.rb

@@ -93,6 +93,10 @@ def type; :GROUP; end
 
       class Star < Unary # :nodoc:
         def type; :STAR; end
+
+        def name
+          left.name.tr '*:', ''
+        end
       end
 
       class Binary < Node # :nodoc:

actionpack/lib/action_dispatch/journey/path/pattern.rb

+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

actionpack/lib/action_dispatch/journey/router/strexp.rb

@@ -6,18 +6,21 @@ class << self
           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

actionpack/lib/action_dispatch/routing/mapper.rb

@@ -62,13 +62,12 @@ def constraint_args(constraint, request)
       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 @@ def initialize(set, scope, path, options)
           @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 @@ def to_route
 
         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 @@ def normalize_options!(options)
               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 @@ def verify_callable_constraint(callable_constraint)
             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 @@ def app
             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 @@ def constraints
             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

actionpack/lib/action_dispatch/routing/route_set.rb

@@ -418,7 +418,9 @@ def add_route(app, conditions = {}, requirements = {}, defaults = {}, name = nil
             "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 @@ def add_route(app, conditions = {}, requirements = {}, defaults = {}, name = nil
         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,

actionpack/test/dispatch/mapper_test.rb

@@ -72,7 +72,7 @@ def test_map_wildcard_with_other_element
         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 @@ def test_map_wildcard_with_multiple_wildcard
         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
 

actionpack/test/dispatch/routing_test.rb

@@ -99,6 +99,16 @@ def test_namespace_with_controller_segment
     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 @@ def test_absolute_controller_namespace
     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 @@ def test_warn_with_ruby_constant_syntax_namespaced_controller_option
 
     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

actionpack/test/journey/path/pattern_test.rb

@@ -18,7 +18,7 @@ class TestPattern < ActiveSupport::TestCase
           '/: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 @@ class TestPattern < ActiveSupport::TestCase
           '/: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 @@ class TestPattern < ActiveSupport::TestCase
           '/: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 @@ class TestPattern < ActiveSupport::TestCase
           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 @@ def test_optional_names
             ['/: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 @@ def test_to_regexp_match_non_optional
         end
 
         def test_to_regexp_with_group
-          strexp = Router::Strexp.new(
+          strexp = Router::Strexp.build(
             '/page/:name',
             { :name => /(tender|love)/ },
             ["/", ".", "?"]
@@ -131,7 +127,7 @@ def test_to_regexp_with_group
 
         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 @@ def test_ast_sets_regular_expressions
         end
 
         def test_match_data_with_group
-          strexp = Router::Strexp.new(
+          strexp = Router::Strexp.build(
             '/page/:name',
             { :name => /(tender|love)/ },
             ["/", ".", "?"]
@@ -160,7 +156,7 @@ def test_match_data_with_group
         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 @@ def test_match_data_with_multi_group
 
         def test_star_with_custom_re
           z = /\d+/
-          strexp = Router::Strexp.new(
+          strexp = Router::Strexp.build(
             '/page/*foo',
             { :foo => z },
             ["/", ".", "?"]
@@ -185,7 +181,7 @@ def test_star_with_custom_re
         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 @@ def test_insensitive_regexp_with_group
         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 @@ def test_to_regexp_with_strexp
         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 @@ def test_match_controller
         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 @@ def test_match_controller_action
         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 @@ def test_match_controller_action_id
         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 @@ def test_match_literal
         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 @@ def test_match_literal_with_action
         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

actionpack/test/journey/route_test.rb

@@ -5,7 +5,7 @@ module Journey
     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 @@ def test_initialize
 
       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 @@ def test_route_adds_itself_as_memo
       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 @@ def test_format_with_star
       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 @@ def test_connects_all_match
       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 @@ def test_score
         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"}

actionpack/test/journey/router/strexp_test.rb

-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

actionpack/test/journey/router_test.rb

@@ -32,7 +32,7 @@ def ip; env['REMOTE_ADDR']; end
       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 @@ def test_unicode
         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 @@ def test_request_class_and_requirements_success
 
         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 @@ def test_request_class_and_requirements_fail
 
         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 @@ def path_info=(x)
       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 @@ def test_request_class_overrides_path_info
 
       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 @@ def test_regexp_first_precedence
 
       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 @@ def test_required_parts_verified_are_anchored
 
       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 @@ def test_required_parts_are_verified_when_building
 
       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 @@ def test_only_required_parts_are_verified
 
       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 @@ def test_clear_trailing_slash_from_script_name_on_root_unanchored_routes
         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 @@ def test_clear_trailing_slash_from_script_name_on_root_unanchored_routes
       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 @@ def test_defaults_merge_correctly
 
       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 @@ def test_recognize_with_unbound_regexp
 
       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 @@ def test_recall_should_be_used_when_scoring
       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 @@ def test_nil_path_parts_are_ignored
       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 @@ def test_generate_slash
       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 @@ def test_generate_calls_param_proc
       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 @@ def test_generate_id
       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 @@ def test_generate_escapes
       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 @@ def test_generate_escapes_with_namespaced_controller
       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 @@ def test_generate_extra_params
       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 @@ def test_generate_uses_recall_if_needed
       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 @@ 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.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 @@ 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.new '/:segment/*splat'
+          path  = Path::Pattern.from_string '/:segment/*splat'
           app   = Object.new
           route = @router.routes.add_route(app, path, {}, {}, {})
 
@@ -463,7 +463,7 @@ def test_generate_with_name
       end
 
       def test_namespaced_controller
-        strexp = Router::Strexp.new(
+        strexp = Router::Strexp.build(
           "/:controller(/:action(/:id))",
           { :controller => /.+?/ },
           ["/", ".", "?"]
@@ -489,7 +489,7 @@ def test_namespaced_controller
       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 @@ def test_recognize_literal
       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 @@ def test_recognize_head_request_as_get_route
       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 @@ def test_recognize_cares_about_verbs
 
       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

actionpack/test/journey/routes_test.rb

@@ -5,7 +5,7 @@ module Journey
     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 @@ def test_clear
 
       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 @@ def test_ast
 
       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 @@ def test_first_name_wins
         #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'