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
  ...

2 files changed, 91 insertions, 81 deletions

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,