Routes should respect namespace even if action is a regexp and just c…

…ontroller is given to to.

actionpack/lib/action_dispatch/routing/mapper.rb

@@ -35,11 +35,12 @@ def call(env)
       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 @@ def extract_path_and_options(args)
               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 @@ def extract_path_and_options(args)
               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 @@ def requirements
 
           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 @@ def default_controller
           def default_action
             if @options[:action]
               @options[:action]
+            elsif @scope[:action]
+              @scope[:action]
             end
           end
       end
@@ -424,7 +419,7 @@ def merge_module_scope(parent, child)
           end
 
           def merge_controller_scope(parent, child)
-            @scope[:module] ? "#{@scope[:module]}/#{child}" : child
+            child
           end
 
           def merge_path_names_scope(parent, child)

actionpack/test/dispatch/routing_test.rb

@@ -245,7 +245,8 @@ def self.matches?(request)
 
       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,22 +1160,33 @@ def test_index
     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'
       assert_equal 'account#overview', @response.body
     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'
       assert_equal 'account#shorthand', @response.body
     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