From now on, parameters defined in default_url_options can be absent from named routes.

This allows the following setup to work:

  # app/controllers/application_controller.rb
  class ApplicationController
    def default_url_options(options=nil)
      { :locale => I18n.locale }
    end
  end

  # From your views and controllers:
  I18n.locale                 #=> :en
  users_url                   #=> "/en/users"
  users_url(:pl)              #=> "/pl/users"
  user_url(1)                 #=> "/en/users/1"
  user_url(:pl, 1)            #=> "/pl/users/1"
  user_url(1, :locale => :pl) #=> "/pl/users/1"

If you provide all expected parameters, it still works as previously.
But if any parameter is missing, it tries to assign all possible ones
with the hash returned in default_url_options or the one passed straight
to the named route method.

Beware that default_url_options in ApplicationController is not shared
with ActionMailer, so you are required to always give the locale in your
email views.

4 files changed, 93 insertions, 65 deletions

diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb
index 0d2ffc6d69..7752642a7b 100644
--- a/actionpack/lib/action_dispatch/routing/route_set.rb
+++ b/actionpack/lib/action_dispatch/routing/route_set.rb
@@ -74,9 +74,8 @@ module ActionDispatch
           @routes = {}
           @helpers = []
 
-          @module ||= Module.new
-          @module.instance_methods.each do |selector|
-            @module.class_eval { remove_method selector }
+          @module ||= Module.new do
+            instance_methods.each { |selector| remove_method(selector) }
           end
         end
 
@@ -168,25 +167,56 @@ module ActionDispatch
             selector = url_helper_name(name, kind)
             hash_access_method = hash_access_name(name, kind)
 
-            # We use module_eval to avoid leaks
+            # We use module_eval to avoid leaks.
+            #
+            # def users_url(*args)
+            #   if args.empty? || Hash === args.first
+            #     options = hash_for_users_url(args.first || {})
+            #   else
+            #     options = hash_for_users_url(args.extract_options!)
+            #     default = default_url_options(options) if self.respond_to?(:default_url_options, true)
+            #     options = (default ||= {}).merge(options)
+            #
+            #     keys = []
+            #     keys -= options.keys unless keys.size == args.size
+            #
+            #     args = args.zip(keys).inject({}) do |h, (v, k)|
+            #       h[k] = v
+            #       h
+            #     end
+            #
+            #     # Tell url_for to skip default_url_options
+            #     options[:use_defaults] = false
+            #     options.merge!(args)
+            #   end
+            # 
+            #   url_for(options)
+            # end
             @module.module_eval <<-END_EVAL, __FILE__, __LINE__ + 1
-              def #{selector}(*args)                                                        # def users_url(*args)
-                                                                                            #
-                opts = if args.empty? || Hash === args.first                                #   opts = if args.empty? || Hash === args.first
-                  args.first || {}                                                          #     args.first || {}
-                else                                                                        #   else
-                  options = args.extract_options!                                           #     options = args.extract_options!
-                  args = args.zip(#{route.segment_keys.inspect}).inject({}) do |h, (v, k)|  #     args = args.zip([]).inject({}) do |h, (v, k)|
-                    h[k] = v                                                                #       h[k] = v
-                    h                                                                       #       h
-                  end                                                                       #     end
-                  options.merge(args)                                                       #     options.merge(args)
-                end                                                                         #   end
-                                                                                            #
-                url_for(#{hash_access_method}(opts))                                        #   url_for(hash_for_users_url(opts))
-                                                                                            #
-              end                                                                           # end
-              protected :#{selector}                                                        # protected :users_url
+              def #{selector}(*args)
+                if args.empty? || Hash === args.first 
+                  options = #{hash_access_method}(args.first || {})
+                else
+                  options = #{hash_access_method}(args.extract_options!)
+                  default = default_url_options(options) if self.respond_to?(:default_url_options, true)
+                  options = (default ||= {}).merge(options)
+
+                  keys = #{route.segment_keys.inspect}
+                  keys -= options.keys unless keys.size == args.size
+
+                  args = args.zip(keys).inject({}) do |h, (v, k)|
+                    h[k] = v
+                    h
+                  end
+
+                  # Tell url_for to skip default_url_options
+                  options[:use_defaults] = false
+                  options.merge!(args)
+                end
+
+                url_for(options)
+              end
+              protected :#{selector}
             END_EVAL
             helpers << selector
           end
diff --git a/actionpack/lib/action_view/helpers/url_helper.rb b/actionpack/lib/action_view/helpers/url_helper.rb
index 710178905a..14628c5404 100644
--- a/actionpack/lib/action_view/helpers/url_helper.rb
+++ b/actionpack/lib/action_view/helpers/url_helper.rb
@@ -11,6 +11,11 @@ module ActionView
     module UrlHelper
       include JavaScriptHelper
 
+      # Need to map default url options to controller one.
+      def default_url_options(*args) #:nodoc:
+        @controller.send(:default_url_options, *args)
+      end
+
       # Returns the URL for the set of +options+ provided. This takes the
       # same options as +url_for+ in Action Controller (see the
       # documentation for <tt>ActionController::Base#url_for</tt>). Note that by default
diff --git a/actionpack/test/activerecord/polymorphic_routes_test.rb b/actionpack/test/activerecord/polymorphic_routes_test.rb
index ad744421db..ea82758cf5 100644
--- a/actionpack/test/activerecord/polymorphic_routes_test.rb
+++ b/actionpack/test/activerecord/polymorphic_routes_test.rb
@@ -26,7 +26,7 @@ class Series < ActiveRecord::Base
 end
 
 class PolymorphicRoutesTest < ActionController::TestCase
-  include ActionController::UrlWriter
+  include ActionController::UrlFor
   self.default_url_options[:host] = 'example.com'
 
   def setup
diff --git a/actionpack/test/controller/base_test.rb b/actionpack/test/controller/base_test.rb
index 92d2e75c20..3d43688902 100644
--- a/actionpack/test/controller/base_test.rb
+++ b/actionpack/test/controller/base_test.rb
@@ -6,6 +6,7 @@ require 'pp' # require 'pp' early to prevent hidden_methods from not picking up
 module Submodule
   class ContainedEmptyController < ActionController::Base
   end
+
   class ContainedNonEmptyController < ActionController::Base
     def public_action
       render :nothing => true
@@ -20,12 +21,15 @@ module Submodule
     end
     hide_action :another_hidden_action
   end
+
   class SubclassedController < ContainedNonEmptyController
     hide_action :public_action # Hiding it here should not affect the superclass.
   end
 end
+
 class EmptyController < ActionController::Base
 end
+
 class NonEmptyController < ActionController::Base
   def public_action
     render :nothing => true
@@ -37,7 +41,6 @@ class NonEmptyController < ActionController::Base
 end
 
 class MethodMissingController < ActionController::Base
-
   hide_action :shouldnt_be_called
   def shouldnt_be_called
     raise "NO WAY!"
@@ -48,16 +51,15 @@ protected
   def method_missing(selector)
     render :text => selector.to_s
   end
-
 end
 
 class DefaultUrlOptionsController < ActionController::Base
-  def default_url_options_action
-    render :nothing => true
+  def from_view
+    render :inline => "<%= #{params[:route]} %>"
   end
 
   def default_url_options(options = nil)
-    { :host => 'www.override.com', :action => 'new', :bacon => 'chunky' }
+    { :host => 'www.override.com', :action => 'new', :locale => 'en' }
   end
 end
 
@@ -68,6 +70,7 @@ class ControllerClassTests < Test::Unit::TestCase
     assert_equal 'submodule/contained_empty', Submodule::ContainedEmptyController.controller_path
     assert_equal Submodule::ContainedEmptyController.controller_path, Submodule::ContainedEmptyController.new.controller_path
   end
+
   def test_controller_name
     assert_equal 'empty', EmptyController.controller_name
     assert_equal 'contained_empty', Submodule::ContainedEmptyController.controller_name
@@ -86,41 +89,16 @@ class ControllerInstanceTests < Test::Unit::TestCase
 
   def test_action_methods
     @empty_controllers.each do |c|
-      hide_mocha_methods_from_controller(c)
       assert_equal Set.new, c.class.__send__(:action_methods), "#{c.controller_path} should be empty!"
     end
+
     @non_empty_controllers.each do |c|
-      hide_mocha_methods_from_controller(c)
       assert_equal Set.new(%w(public_action)), c.class.__send__(:action_methods), "#{c.controller_path} should not be empty!"
     end
   end
-
-  protected
-    # Mocha adds some public instance methods to Object that would be
-    # considered actions, so explicitly hide_action them.
-    def hide_mocha_methods_from_controller(controller)
-      mocha_methods = [
-        :expects, :mocha, :mocha_inspect, :reset_mocha, :stubba_object,
-        :stubba_method, :stubs, :verify, :__metaclass__, :__is_a__, :to_matcher,
-      ]
-      controller.class.__send__(:hide_action, *mocha_methods)
-    end
 end
 
-
 class PerformActionTest < ActionController::TestCase
-  class MockLogger
-    attr_reader :logged
-
-    def initialize
-      @logged = []
-    end
-
-    def method_missing(method, *args)
-      @logged << args.first.to_s
-    end
-  end
-
   def use_controller(controller_class)
     @controller = controller_class.new
 
@@ -128,9 +106,8 @@ class PerformActionTest < ActionController::TestCase
     # a more accurate simulation of what happens in "real life".
     @controller.logger = Logger.new(nil)
 
-    @request    = ActionController::TestRequest.new
-    @response   = ActionController::TestResponse.new
-
+    @request     = ActionController::TestRequest.new
+    @response    = ActionController::TestResponse.new
     @request.host = "www.nextangle.com"
 
     rescue_action_in_public!
@@ -145,8 +122,7 @@ class PerformActionTest < ActionController::TestCase
 
   def test_method_missing_is_not_an_action_name
     use_controller MethodMissingController
-
-    assert ! @controller.__send__(:action_method?, 'method_missing')
+    assert !@controller.__send__(:action_method?, 'method_missing')
 
     get :method_missing
     assert_response :success
@@ -172,16 +148,35 @@ class DefaultUrlOptionsTest < ActionController::TestCase
   def test_default_url_options_are_used_if_set
     with_routing do |set|
       set.draw do |map|
-        match 'default_url_options', :to => 'default_url_options#default_url_options_action', :as => :default_url_options
+        match 'from_view', :to => 'default_url_options#from_view', :as => :from_view
         match ':controller/:action'
       end
 
-      get :default_url_options_action # Make a dummy request so that the controller is initialized properly.
+      get :from_view, :route => "from_view_url"
 
-      assert_equal 'http://www.override.com/default_url_options/new?bacon=chunky', @controller.url_for(:controller => 'default_url_options')
-      assert_equal 'http://www.override.com/default_url_options?bacon=chunky', @controller.send(:default_url_options_url)
+      assert_equal 'http://www.override.com/from_view?locale=en', @response.body
+      assert_equal 'http://www.override.com/from_view?locale=en', @controller.send(:from_view_url)
+      assert_equal 'http://www.override.com/default_url_options/new?locale=en', @controller.url_for(:controller => 'default_url_options')
     end
   end
+
+  def test_default_url_options_are_used_in_non_positional_parameters
+    with_routing do |set|
+      set.draw do |map|
+        scope("/:locale") do
+          resources :descriptions
+        end
+        match ':controller/:action'
+      end
+
+      get :from_view, :route => "description_path(1)"
+
+      assert_equal '/en/descriptions/1', @response.body
+      assert_equal '/en/descriptions/1', @controller.send(:description_path, 1)
+      assert_equal '/pl/descriptions/1', @controller.send(:description_path, 1, :locale => "pl")
+    end
+  end
+
 end
 
 class EmptyUrlOptionsTest < ActionController::TestCase
@@ -197,10 +192,8 @@ class EmptyUrlOptionsTest < ActionController::TestCase
     get :public_action
     assert_equal "http://www.example.com/non_empty/public_action", @controller.url_for
   end
-end
 
-class EnsureNamedRoutesWorksTicket22BugTest < ActionController::TestCase
-  def test_named_routes_still_work
+  def test_named_routes_with_path_without_doing_a_request_first
     with_routing do |set|
       set.draw do |map|
         resources :things