diff options
author | Aaron Patterson <aaron.patterson@gmail.com> | 2014-05-29 16:02:16 -0700 |
---|---|---|
committer | Aaron Patterson <aaron.patterson@gmail.com> | 2014-05-29 16:02:16 -0700 |
commit | c767fbfc25561b1ba6a9c4ae040169826e5235b8 (patch) | |
tree | 1c0bd3ffc87cd5469a6e4e6f0fb48296cf94b15b /actionpack/lib/action_dispatch/routing | |
parent | aecf76fcde37ca31d5520f49f5e238a7161869a3 (diff) | |
parent | da2cf937aa68c2470c0d4a73c29fe3555b2667c0 (diff) | |
download | rails-c767fbfc25561b1ba6a9c4ae040169826e5235b8.tar.gz rails-c767fbfc25561b1ba6a9c4ae040169826e5235b8.tar.bz2 rails-c767fbfc25561b1ba6a9c4ae040169826e5235b8.zip |
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
...
Diffstat (limited to 'actionpack/lib/action_dispatch/routing')
-rw-r--r-- | actionpack/lib/action_dispatch/routing/mapper.rb | 165 | ||||
-rw-r--r-- | actionpack/lib/action_dispatch/routing/route_set.rb | 7 |
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, |