From 90d2802b71a6e89aedfe40564a37bd35f777e541 Mon Sep 17 00:00:00 2001 From: Andrew White Date: Tue, 15 Jan 2013 13:38:10 +0000 Subject: Add support for other types of routing constraints This now allows the use of arrays like this: get '/foo/:action', to: 'foo', constraints: { subdomain: %w[www admin] } or constraints where the request method returns an Fixnum like this: get '/foo', to: 'foo#index', constraints: { port: 8080 } Note that this only applies to constraints on the request - path constraints still need to be specified as Regexps as the various constraints are compiled into a single Regexp. --- actionpack/CHANGELOG.md | 15 ++ actionpack/lib/action_dispatch/journey/route.rb | 29 ++- actionpack/lib/action_dispatch/journey/router.rb | 6 +- actionpack/lib/action_dispatch/routing/mapper.rb | 252 ++++++++++++++--------- actionpack/test/dispatch/routing_test.rb | 48 +++++ 5 files changed, 237 insertions(+), 113 deletions(-) diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 141d1aef60..6785e7ffef 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,5 +1,20 @@ ## Rails 4.0.0 (unreleased) ## +* Add support for routing constraints other than Regexp and String. + For example this now allows the use of arrays like this: + + get '/foo/:action', to: 'foo', constraints: { subdomain: %w[www admin] } + + or constraints where the request method returns an Fixnum like this: + + get '/foo', to: 'foo#index', constraints: { port: 8080 } + + Note that this only applies to constraints on the request - path constraints + still need to be specified as Regexps as the various constraints are compiled + into a single Regexp. + + *Andrew White* + * Fix a bug in integration tests where setting the port via a url passed to the process method was ignored when constructing the request environment. diff --git a/actionpack/lib/action_dispatch/journey/route.rb b/actionpack/lib/action_dispatch/journey/route.rb index d3988cf31e..b08ac4b4eb 100644 --- a/actionpack/lib/action_dispatch/journey/route.rb +++ b/actionpack/lib/action_dispatch/journey/route.rb @@ -1,7 +1,7 @@ module ActionDispatch module Journey # :nodoc: class Route # :nodoc: - attr_reader :app, :path, :verb, :defaults, :ip, :name + attr_reader :app, :path, :defaults, :name attr_reader :constraints alias :conditions :constraints @@ -12,15 +12,11 @@ module ActionDispatch # +path+ is a path constraint. # +constraints+ is a hash of constraints to be applied to this route. def initialize(name, app, path, constraints, defaults = {}) - constraints = constraints.dup @name = name @app = app @path = path - @verb = constraints[:request_method] || // - @ip = constraints.delete(:ip) || // @constraints = constraints - @constraints.keep_if { |_,v| Regexp === v || String === v } @defaults = defaults @required_defaults = nil @required_parts = nil @@ -89,6 +85,29 @@ module ActionDispatch @defaults.dup.delete_if { |k,_| matches.include?(k) } end end + + def matches?(request) + constraints.all? do |method, value| + next true unless request.respond_to?(method) + + case value + when Regexp, String + value === request.send(method).to_s + when Array + value.include?(request.send(method)) + else + value === request.send(method) + end + end + end + + def ip + constraints[:ip] || // + end + + def verb + constraints[:request_method] || // + end end end end diff --git a/actionpack/lib/action_dispatch/journey/router.rb b/actionpack/lib/action_dispatch/journey/router.rb index 1fc45a2109..31868b1814 100644 --- a/actionpack/lib/action_dispatch/journey/router.rb +++ b/actionpack/lib/action_dispatch/journey/router.rb @@ -131,11 +131,7 @@ module ActionDispatch } routes.concat get_routes_as_head(routes) - routes.sort_by!(&:precedence).select! { |r| - r.constraints.all? { |k, v| v === req.send(k) } && - r.verb === req.request_method - } - routes.reject! { |r| req.ip && !(r.ip === req.ip) } + routes.sort_by!(&:precedence).select! { |r| r.matches?(req) } routes.map! { |r| match_data = r.path.match(req.path_info) diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 1046a7d75a..43d87e9c0b 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -8,6 +8,8 @@ require 'action_dispatch/routing/redirection' module ActionDispatch module Routing class Mapper + URL_OPTIONS = [:protocol, :subdomain, :domain, :host, :port] + class Constraints #:nodoc: def self.new(app, constraints, request = Rack::Request) if constraints.any? @@ -45,37 +47,68 @@ module ActionDispatch end class Mapping #:nodoc: - IGNORE_OPTIONS = [:to, :as, :via, :on, :constraints, :defaults, :only, :except, :anchor, :shallow, :shallow_path, :shallow_prefix] + 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} SHORTHAND_REGEX = %r{/[\w/]+$} WILDCARD_PATH = %r{\*([^/\)]+)\)?$} - def initialize(set, scope, path, options) - @set, @scope = set, scope - @segment_keys = nil - @options = (@scope[:options] || {}).merge(options) - @path = normalize_path(path) - normalize_options! + attr_reader :scope, :path, :options, :requirements, :conditions, :defaults - via_all = @options.delete(:via) if @options[:via] == :all + def initialize(set, scope, path, options) + @set, @scope, @path, @options = set, scope, path, options + @requirements, @conditions, @defaults = {}, {}, {} - if !via_all && request_method_condition.empty? - msg = "You should not use the `match` method in your router without specifying an HTTP method.\n" \ - "If you want to expose your action to GET, use `get` in the router:\n\n" \ - " Instead of: match \"controller#action\"\n" \ - " Do: get \"controller#action\"" - raise msg - end + normalize_path! + normalize_options! + normalize_requirements! + normalize_defaults! + normalize_conditions! end def to_route - [ app, conditions, requirements, defaults, @options[:as], @options[:anchor] ] + [ app, conditions, requirements, defaults, options[:as], options[:anchor] ] end private + def normalize_path! + 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)" + end + end + + def required_format? + options[:format] == true + end + + def optional_format? + options[:format] != false && !path.include?(':format') && !path.end_with?('/') + end + def normalize_options! - path_without_format = @path.sub(/\(\.:format\)$/, '') + @options.reverse_merge!(scope[:options]) if scope[:options] + path_without_format = path.sub(/\(\.:format\)$/, '') + + # 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] ||= /.+?/ + end + + if path_without_format.match(':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 + # controllers with default routes like :controller/:action/:id(.:format), e.g: + # GET /admin/products/show/1 + # => { controller: 'admin/products', action: 'show', id: '1' } + @options[:controller] ||= /.+?/ + end if using_match_shorthand?(path_without_format, @options) to_shorthand = @options[:to].blank? @@ -83,87 +116,96 @@ module ActionDispatch end @options.merge!(default_controller_and_action(to_shorthand)) + end - requirements.each do |name, requirement| - # segment_keys.include?(k.to_s) || k == :controller - next unless Regexp === requirement && !constraints[name] + # match "account/overview" + def using_match_shorthand?(path, options) + path && (options[:to] || options[:action]).nil? && path =~ SHORTHAND_REGEX + end + + def normalize_format! + if options[:format] == true + options[:format] = /.+/ + elsif options[:format] == false + options.delete(:format) + end + end + + def normalize_requirements! + constraints.each do |key, requirement| + next unless segment_keys.include?(key) || key == :controller if requirement.source =~ ANCHOR_CHARACTERS_REGEX raise ArgumentError, "Regexp anchor characters are not allowed in routing requirements: #{requirement.inspect}" end + if requirement.multiline? - raise ArgumentError, "Regexp multiline option not allowed in routing requirements: #{requirement.inspect}" + raise ArgumentError, "Regexp multiline option is not allowed in routing requirements: #{requirement.inspect}" end - end - if @options[:constraints].is_a?(Hash) - (@options[:defaults] ||= {}).reverse_merge!(defaults_from_constraints(@options[:constraints])) + @requirements[key] = requirement end - end - # match "account/overview" - def using_match_shorthand?(path, options) - path && (options[:to] || options[:action]).nil? && path =~ SHORTHAND_REGEX + if options[:format] == true + @requirements[:format] = /.+/ + elsif Regexp === options[:format] + @requirements[:format] = options[:format] + elsif String === options[:format] + @requirements[:format] = Regexp.compile(options[:format]) + end end - def normalize_path(path) - raise ArgumentError, "path is required" if path.blank? - path = Mapper.normalize_path(path) - - if path.match(':controller') - raise ArgumentError, ":controller segment is not allowed within a namespace block" if @scope[:module] + def normalize_defaults! + @defaults.merge!(scope[:defaults]) if scope[:defaults] + @defaults.merge!(options[:defaults]) if options[:defaults] - # Add a default constraint for :controller path segments that matches namespaced - # controllers with default routes like :controller/:action/:id(.:format), e.g: - # GET /admin/products/show/1 - # => { controller: 'admin/products', action: 'show', id: '1' } - @options[:controller] ||= /.+?/ + options.each do |key, default| + next if Regexp === default || IGNORE_OPTIONS.include?(key) + @defaults[key] = default end - # Add a constraint for wildcard route to make it non-greedy and match the - # optional format part of the route by default - if path.match(WILDCARD_PATH) && @options[:format] != false - @options[$1.to_sym] ||= /.+?/ + if options[:constraints].is_a?(Hash) + options[:constraints].each do |key, default| + next unless URL_OPTIONS.include?(key) && (String === default || Fixnum === default) + @defaults[key] ||= default + end end - if @options[:format] == false - @options.delete(:format) - path - elsif path.include?(":format") || path.end_with?('/') - path - elsif @options[:format] == true - "#{path}.:format" - else - "#{path}(.:format)" + if Regexp === options[:format] + @defaults[:format] = nil + elsif String === options[:format] + @defaults[:format] = options[:format] end end - def app - Constraints.new( - to.respond_to?(:call) ? to : Routing::RouteSet::Dispatcher.new(:defaults => defaults), - blocks, - @set.request_class - ) - end + def normalize_conditions! + @conditions.merge!(:path_info => path) - def conditions - { :path_info => @path }.merge!(constraints).merge!(request_method_condition) - end + constraints.each do |key, condition| + next if segment_keys.include?(key) || key == :controller + @conditions[key] = condition + end - def requirements - @requirements ||= (@options[:constraints].is_a?(Hash) ? @options[:constraints] : {}).tap do |requirements| - requirements.reverse_merge!(@scope[:constraints]) if @scope[:constraints] - @options.each { |k, v| requirements[k] ||= v if v.is_a?(Regexp) } + via_all = options.delete(:via) if options[:via] == :all + + if !via_all && options[:via].blank? + msg = "You should not use the `match` method in your router without specifying an HTTP method.\n" \ + "If you want to expose your action to GET, use `get` in the router:\n\n" \ + " Instead of: match \"controller#action\"\n" \ + " Do: get \"controller#action\"" + raise msg end - end - def defaults - @defaults ||= (@options[:defaults] || {}).tap do |defaults| - defaults.reverse_merge!(@scope[:defaults]) if @scope[:defaults] - @options.each { |k, v| defaults[k] = v unless v.is_a?(Regexp) || IGNORE_OPTIONS.include?(k.to_sym) } + if via = options[:via] + list = Array(via).map { |m| m.to_s.dasherize.upcase } + @conditions.merge!(:request_method => list) end end + def app + Constraints.new(endpoint, blocks, @set.request_class) + end + def default_controller_and_action(to_shorthand=nil) if to.respond_to?(:call) { } @@ -188,11 +230,11 @@ module ActionDispatch controller = controller.to_s unless controller.is_a?(Regexp) action = action.to_s unless action.is_a?(Regexp) - if controller.blank? && segment_keys.exclude?("controller") + if controller.blank? && segment_keys.exclude?(:controller) raise ArgumentError, "missing :controller" end - if action.blank? && segment_keys.exclude?("action") + if action.blank? && segment_keys.exclude?(:action) raise ArgumentError, "missing :action" end @@ -204,50 +246,55 @@ module ActionDispatch end def blocks - constraints = @options[:constraints] - if constraints.present? && !constraints.is_a?(Hash) - [constraints] + if options[:constraints].present? && !options[:constraints].is_a?(Hash) + [options[:constraints]] else - @scope[:blocks] || [] + scope[:blocks] || [] end end def constraints - @constraints ||= requirements.reject { |k, v| segment_keys.include?(k.to_s) || k == :controller } - end + @constraints ||= {}.tap do |constraints| + constraints.merge!(scope[:constraints]) if scope[:constraints] - def request_method_condition - if via = @options[:via] - list = Array(via).map { |m| m.to_s.dasherize.upcase } - { :request_method => list } - else - { } + options.except(*IGNORE_OPTIONS).each do |key, option| + constraints[key] = option if Regexp === option + end + + constraints.merge!(options[:constraints]) if options[:constraints].is_a?(Hash) end end def segment_keys - return @segment_keys if @segment_keys + @segment_keys ||= path_pattern.names.map{ |s| s.to_sym } + end + + def path_pattern + Journey::Path::Pattern.new(strexp) + end - @segment_keys = Journey::Path::Pattern.new( - Journey::Router::Strexp.compile(@path, requirements, SEPARATORS) - ).names + def strexp + Journey::Router::Strexp.compile(path, requirements, SEPARATORS) + end + + def endpoint + to.respond_to?(:call) ? to : dispatcher + end + + def dispatcher + Routing::RouteSet::Dispatcher.new(:defaults => defaults) end def to - @options[:to] + options[:to] end def default_controller - @options[:controller] || @scope[:controller] + options[:controller] || scope[:controller] end def default_action - @options[:action] || @scope[:action] - end - - def defaults_from_constraints(constraints) - url_keys = [:protocol, :subdomain, :domain, :host, :port] - constraints.select { |k, v| url_keys.include?(k) && (v.is_a?(String) || v.is_a?(Fixnum)) } + options[:action] || scope[:action] end end @@ -641,7 +688,11 @@ module ActionDispatch options[:constraints] ||= {} if options[:constraints].is_a?(Hash) - (options[:defaults] ||= {}).reverse_merge!(defaults_from_constraints(options[:constraints])) + defaults = options[:constraints].select do + |k, v| URL_OPTIONS.include?(k) && (v.is_a?(String) || v.is_a?(Fixnum)) + end + + (options[:defaults] ||= {}).reverse_merge!(defaults) else block, options[:constraints] = options[:constraints], {} end @@ -846,11 +897,6 @@ module ActionDispatch def override_keys(child) #:nodoc: child.key?(:only) || child.key?(:except) ? [:only, :except] : [] end - - def defaults_from_constraints(constraints) - url_keys = [:protocol, :subdomain, :domain, :host, :port] - constraints.select { |k, v| url_keys.include?(k) && (v.is_a?(String) || v.is_a?(Fixnum)) } - end end # Resource routing allows you to quickly declare all of the common routes diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index cb5299e8d3..2d34a0f6b1 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -3252,3 +3252,51 @@ class TestOptionalRootSegments < ActionDispatch::IntegrationTest assert_equal '/page/1', root_path(:page => '1') end end + +class TestPortConstraints < ActionDispatch::IntegrationTest + Routes = ActionDispatch::Routing::RouteSet.new.tap do |app| + app.draw do + ok = lambda { |env| [200, { 'Content-Type' => 'text/plain' }, []] } + + get '/integer', to: ok, constraints: { :port => 8080 } + get '/string', to: ok, constraints: { :port => '8080' } + get '/array', to: ok, constraints: { :port => [8080] } + get '/regexp', to: ok, constraints: { :port => /8080/ } + end + end + + include Routes.url_helpers + def app; Routes end + + def test_integer_port_constraints + get 'http://www.example.com/integer' + assert_response :not_found + + get 'http://www.example.com:8080/integer' + assert_response :success + end + + def test_string_port_constraints + get 'http://www.example.com/string' + assert_response :not_found + + get 'http://www.example.com:8080/string' + assert_response :success + end + + def test_array_port_constraints + get 'http://www.example.com/array' + assert_response :not_found + + get 'http://www.example.com:8080/array' + assert_response :success + end + + def test_regexp_port_constraints + get 'http://www.example.com/regexp' + assert_response :not_found + + get 'http://www.example.com:8080/regexp' + assert_response :success + end +end -- cgit v1.2.3