diff options
| -rw-r--r-- | actionpack/lib/action_dispatch/routing/mapper.rb | 63 | ||||
| -rw-r--r-- | actionpack/test/dispatch/routing_test.rb | 18 |
2 files changed, 44 insertions, 37 deletions
diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 430f6fc5a3..6a4b57e3f9 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -35,11 +35,12 @@ module ActionDispatch end class Mapping #:nodoc: - IGNORE_OPTIONS = [:to, :as, :controller, :action, :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] def initialize(set, scope, args) @set, @scope = set, scope @path, @options = extract_path_and_options(args) + normalize_options! end def to_route @@ -57,14 +58,6 @@ module ActionDispatch path = args.first end - if @scope[:module] && options[:to] && !options[:to].is_a?(Proc) - if options[:to].to_s.include?("#") - options[:to] = "#{@scope[:module]}/#{options[:to]}" - elsif @scope[:controller].nil? - options[:to] = "#{@scope[:module]}##{options[:to]}" - end - end - if path.match(':controller') raise ArgumentError, ":controller segment is not allowed within a namespace block" if @scope[:module] @@ -75,15 +68,19 @@ module ActionDispatch options.reverse_merge!(:controller => /.+?/) end - path = normalize_path(path) - path_without_format = path.sub(/\(\.:format\)$/, '') + [ normalize_path(path), options ] + end - if using_match_shorthand?(path_without_format, options) - options[:to] ||= path_without_format[1..-1].sub(%r{/([^/]*)$}, '#\1') - options[:as] ||= path_without_format[1..-1].gsub("/", "_") + def normalize_options! + path_without_format = @path.sub(/\(\.:format\)$/, '') + + if using_match_shorthand?(path_without_format, @options) + to_shorthand = @options[:to].blank? + @options[:to] ||= path_without_format[1..-1].sub(%r{/([^/]*)$}, '#\1') + @options[:as] ||= path_without_format[1..-1].gsub("/", "_") end - [ path, options ] + @options.merge!(default_controller_and_action(to_shorthand)) end # match "account" => "account#index" @@ -122,44 +119,40 @@ module ActionDispatch def defaults @defaults ||= (@options[:defaults] || {}).tap do |defaults| - defaults.merge!(default_controller_and_action) 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) } end end - def default_controller_and_action + def default_controller_and_action(to_shorthand=nil) if to.respond_to?(:call) { } else - defaults = case to - when String + if to.is_a?(String) controller, action = to.split('#') - { :controller => controller, :action => action } - when Symbol - { :action => to.to_s } - else - {} + elsif to.is_a?(Symbol) + action = to.to_s end - defaults[:controller] ||= default_controller - defaults[:action] ||= default_action + controller ||= default_controller + action ||= default_action - defaults.delete(:controller) if defaults[:controller].blank? || defaults[:controller].is_a?(Regexp) - defaults.delete(:action) if defaults[:action].blank? || defaults[:action].is_a?(Regexp) + unless controller.is_a?(Regexp) || to_shorthand + controller = [@scope[:module], controller].compact.join("/").presence + end - defaults[:controller] = defaults[:controller].to_s if defaults.key?(:controller) - defaults[:action] = defaults[:action].to_s if defaults.key?(:action) + controller = controller.to_s unless controller.is_a?(Regexp) + action = action.to_s unless action.is_a?(Regexp) - if defaults[:controller].blank? && segment_keys.exclude?("controller") + if controller.blank? && segment_keys.exclude?("controller") raise ArgumentError, "missing :controller" end - if defaults[:action].blank? && segment_keys.exclude?("action") + if action.blank? && segment_keys.exclude?("action") raise ArgumentError, "missing :action" end - defaults + { :controller => controller, :action => action } end end @@ -207,6 +200,8 @@ module ActionDispatch def default_action if @options[:action] @options[:action] + elsif @scope[:action] + @scope[:action] end end end @@ -424,7 +419,7 @@ module ActionDispatch end def merge_controller_scope(parent, child) - @scope[:module] ? "#{@scope[:module]}/#{child}" : child + child end def merge_path_names_scope(parent, child) diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index 2a014bf976..4808663aa9 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -245,7 +245,8 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest namespace :account do match 'shorthand' - match 'description', :to => "description", :as => "description" + match 'description', :to => :description, :as => "description" + match ':action/callback', :action => /twitter|github/, :to => "callbacks", :as => :callback resource :subscription, :credit, :credit_card root :to => "account#index" @@ -1159,7 +1160,7 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end end - def test_convention_match_with_no_scope + def test_match_shorthand_with_no_scope with_test_routes do assert_equal '/account/overview', account_overview_path get '/account/overview' @@ -1167,7 +1168,7 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end end - def test_convention_match_inside_namespace + def test_match_shorthand_inside_namespace with_test_routes do assert_equal '/account/shorthand', account_shorthand_path get '/account/shorthand' @@ -1175,6 +1176,17 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end end + def test_scoped_controller_with_namespace_and_action + with_test_routes do + assert_equal '/account/twitter/callback', account_callback_path("twitter") + get '/account/twitter/callback' + assert_equal 'account/callbacks#twitter', @response.body + + get '/account/whatever/callback' + assert_equal 'Not Found', @response.body + end + end + def test_convention_match_nested_and_with_leading_slash with_test_routes do assert_equal '/account/nested/overview', account_nested_overview_path |