Remove duplicated url_for code and move methods shared between ActionMailer and ActionController up to AbstractController.

10 files changed, 488 insertions, 477 deletions

diff --git a/actionpack/lib/abstract_controller.rb b/actionpack/lib/abstract_controller.rb
index 237ab577ba..efc35a7e56 100644
--- a/actionpack/lib/abstract_controller.rb
+++ b/actionpack/lib/abstract_controller.rb
@@ -15,4 +15,5 @@ module AbstractController
   autoload :LocalizedCache
   autoload :Logger
   autoload :Rendering
+  autoload :UrlFor
 end
diff --git a/actionpack/lib/abstract_controller/url_for.rb b/actionpack/lib/abstract_controller/url_for.rb
new file mode 100644
index 0000000000..6b7d2b1f34
--- /dev/null
+++ b/actionpack/lib/abstract_controller/url_for.rb
@@ -0,0 +1,156 @@
+module AbstractController
+  # In <b>routes.rb</b> one defines URL-to-controller mappings, but the reverse
+  # is also possible: an URL can be generated from one of your routing definitions.
+  # URL generation functionality is centralized in this module.
+  #
+  # See AbstractController::Routing and AbstractController::Resources for general
+  # information about routing and routes.rb.
+  #
+  # <b>Tip:</b> If you need to generate URLs from your models or some other place,
+  # then AbstractController::UrlFor is what you're looking for. Read on for
+  # an introduction.
+  #
+  # == URL generation from parameters
+  #
+  # As you may know, some functions - such as AbstractController::Base#url_for
+  # and ActionView::Helpers::UrlHelper#link_to, can generate URLs given a set
+  # of parameters. For example, you've probably had the chance to write code
+  # like this in one of your views:
+  #
+  #   <%= link_to('Click here', :controller => 'users',
+  #           :action => 'new', :message => 'Welcome!') %>
+  #
+  #   #=> Generates a link to: /users/new?message=Welcome%21
+  #
+  # link_to, and all other functions that require URL generation functionality,
+  # actually use AbstractController::UrlFor under the hood. And in particular,
+  # they use the AbstractController::UrlFor#url_for method. One can generate
+  # the same path as the above example by using the following code:
+  #
+  #   include UrlFor
+  #   url_for(:controller => 'users',
+  #           :action => 'new',
+  #           :message => 'Welcome!',
+  #           :only_path => true)
+  #   # => "/users/new?message=Welcome%21"
+  #
+  # Notice the <tt>:only_path => true</tt> part. This is because UrlFor has no
+  # information about the website hostname that your Rails app is serving. So if you
+  # want to include the hostname as well, then you must also pass the <tt>:host</tt>
+  # argument:
+  #
+  #   include UrlFor
+  #   url_for(:controller => 'users',
+  #           :action => 'new',
+  #           :message => 'Welcome!',
+  #           :host => 'www.example.com')        # Changed this.
+  #   # => "http://www.example.com/users/new?message=Welcome%21"
+  #
+  # By default, all controllers and views have access to a special version of url_for,
+  # that already knows what the current hostname is. So if you use url_for in your
+  # controllers or your views, then you don't need to explicitly pass the <tt>:host</tt>
+  # argument.
+  #
+  # For convenience reasons, mailers provide a shortcut for AbstractController::UrlFor#url_for.
+  # So within mailers, you only have to type 'url_for' instead of 'AbstractController::UrlFor#url_for'
+  # in full. However, mailers don't have hostname information, and what's why you'll still
+  # have to specify the <tt>:host</tt> argument when generating URLs in mailers.
+  #
+  #
+  # == URL generation for named routes
+  #
+  # UrlFor also allows one to access methods that have been auto-generated from
+  # named routes. For example, suppose that you have a 'users' resource in your
+  # <b>routes.rb</b>:
+  #
+  #   map.resources :users
+  #
+  # This generates, among other things, the method <tt>users_path</tt>. By default,
+  # this method is accessible from your controllers, views and mailers. If you need
+  # to access this auto-generated method from other places (such as a model), then
+  # you can do that by including AbstractController::UrlFor in your class:
+  #
+  #   class User < ActiveRecord::Base
+  #     include AbstractController::UrlFor
+  #
+  #     def base_uri
+  #       user_path(self)
+  #     end
+  #   end
+  #
+  #   User.find(1).base_uri # => "/users/1"
+  #
+  module UrlFor
+    extend ActiveSupport::Concern
+
+    included do
+      ActionController::Routing::Routes.install_helpers(self)
+      extlib_inheritable_accessor :default_url_options,
+                                  :instance_writer => false, :instance_reader => false
+      self.default_url_options ||= {}
+    end
+
+    # Overwrite to implement a number of default options that all url_for-based methods will use. The default options should come in
+    # the form of a hash, just like the one you would use for url_for directly. Example:
+    #
+    #   def default_url_options(options)
+    #     { :project => @project.active? ? @project.url_name : "unknown" }
+    #   end
+    #
+    # As you can infer from the example, this is mostly useful for situations where you want to centralize dynamic decisions about the
+    # urls as they stem from the business domain. Please note that any individual url_for call can always override the defaults set
+    # by this method.
+    def default_url_options(options = nil)
+      self.class.default_url_options
+    end
+
+    def rewrite_options(options) #:nodoc:
+      if options.delete(:use_defaults) != false && (defaults = default_url_options(options))
+        defaults.merge(options)
+      else
+        options
+      end
+    end
+
+    # Generate a url based on the options provided, default_url_options and the
+    # routes defined in routes.rb.  The following options are supported:
+    #
+    # * <tt>:only_path</tt> - If true, the relative url is returned. Defaults to +false+.
+    # * <tt>:protocol</tt> - The protocol to connect to. Defaults to 'http'.
+    # * <tt>:host</tt> - Specifies the host the link should be targeted at.
+    #   If <tt>:only_path</tt> is false, this option must be
+    #   provided either explicitly, or via +default_url_options+.
+    # * <tt>:port</tt> - Optionally specify the port to connect to.
+    # * <tt>:anchor</tt> - An anchor name to be appended to the path.
+    # * <tt>:skip_relative_url_root</tt> - If true, the url is not constructed using the
+    #   +relative_url_root+ set in AbstractController::Base.relative_url_root.
+    # * <tt>:trailing_slash</tt> - If true, adds a trailing slash, as in "/archive/2009/"
+    #
+    # Any other key (<tt>:controller</tt>, <tt>:action</tt>, etc.) given to
+    # +url_for+ is forwarded to the Routes module.
+    #
+    # Examples:
+    #
+    #    url_for :controller => 'tasks', :action => 'testing', :host=>'somehost.org', :port=>'8080'    # => 'http://somehost.org:8080/tasks/testing'
+    #    url_for :controller => 'tasks', :action => 'testing', :host=>'somehost.org', :anchor => 'ok', :only_path => true    # => '/tasks/testing#ok'
+    #    url_for :controller => 'tasks', :action => 'testing', :trailing_slash=>true  # => 'http://somehost.org/tasks/testing/'
+    #    url_for :controller => 'tasks', :action => 'testing', :host=>'somehost.org', :number => '33'  # => 'http://somehost.org/tasks/testing?number=33'
+    def url_for(options = {})
+      options ||= {}
+      case options
+        when String
+          options
+        when Hash
+          _url_rewriter.rewrite(rewrite_options(options))
+        else
+          polymorphic_url(options)
+      end
+    end
+
+  protected
+
+    def _url_rewriter
+      ActionController::UrlRewriter
+    end
+  end
+end
diff --git a/actionpack/lib/action_controller.rb b/actionpack/lib/action_controller.rb
index e6cd055a59..8271f6f3e7 100644
--- a/actionpack/lib/action_controller.rb
+++ b/actionpack/lib/action_controller.rb
@@ -49,7 +49,6 @@ module ActionController
   eager_autoload do
     autoload :RecordIdentifier
     autoload :UrlRewriter
-    autoload :UrlWriter,                'action_controller/url_rewriter'
 
     # TODO: Don't autoload exceptions, setup explicit
     # requires for files that need them
diff --git a/actionpack/lib/action_controller/metal/head.rb b/actionpack/lib/action_controller/metal/head.rb
index c82d9cf369..37be8b3999 100644
--- a/actionpack/lib/action_controller/metal/head.rb
+++ b/actionpack/lib/action_controller/metal/head.rb
@@ -1,6 +1,7 @@
 module ActionController
   module Head
-    include UrlFor
+    extend ActiveSupport::Concern
+    include ActionController::UrlFor
 
     # Return a response that has no content (merely headers). The options
     # argument is interpreted to be a hash of header names and values.
diff --git a/actionpack/lib/action_controller/metal/redirecting.rb b/actionpack/lib/action_controller/metal/redirecting.rb
index 7a2f9a6fc5..7b277c0ae0 100644
--- a/actionpack/lib/action_controller/metal/redirecting.rb
+++ b/actionpack/lib/action_controller/metal/redirecting.rb
@@ -9,7 +9,9 @@ module ActionController
 
   module Redirecting
     extend ActiveSupport::Concern
+
     include AbstractController::Logger
+    include ActionController::UrlFor
 
     # Redirects the browser to the target specified in +options+. This parameter can take one of three forms:
     #
diff --git a/actionpack/lib/action_controller/metal/url_for.rb b/actionpack/lib/action_controller/metal/url_for.rb
index 8c3810ebcb..73feacb872 100644
--- a/actionpack/lib/action_controller/metal/url_for.rb
+++ b/actionpack/lib/action_controller/metal/url_for.rb
@@ -2,40 +2,14 @@ module ActionController
   module UrlFor
     extend ActiveSupport::Concern
 
-    include RackDelegation
+    include AbstractController::UrlFor
+    include ActionController::RackDelegation
 
-    # Overwrite to implement a number of default options that all url_for-based methods will use. The default options should come in
-    # the form of a hash, just like the one you would use for url_for directly. Example:
-    #
-    #   def default_url_options(options)
-    #     { :project => @project.active? ? @project.url_name : "unknown" }
-    #   end
-    #
-    # As you can infer from the example, this is mostly useful for situations where you want to centralize dynamic decisions about the
-    # urls as they stem from the business domain. Please note that any individual url_for call can always override the defaults set
-    # by this method.
-    def default_url_options(options = nil)
-    end
-
-    def rewrite_options(options) #:nodoc:
-      if defaults = default_url_options(options)
-        defaults.merge(options)
-      else
-        options
-      end
-    end
+  protected
 
-    def url_for(options = {})
-      options ||= {}
-      case options
-        when String
-          options
-        when Hash
-          @url ||= UrlRewriter.new(request, params)
-          @url.rewrite(rewrite_options(options))
-        else
-          polymorphic_url(options)
-      end
+    def _url_rewriter
+      return ActionController::UrlRewriter unless request
+      @_url_rewriter ||= ActionController::UrlRewriter.new(request, params)
     end
   end
 end
diff --git a/actionpack/lib/action_controller/url_rewriter.rb b/actionpack/lib/action_controller/url_rewriter.rb
index 80285b07f5..933a1fa8f9 100644
--- a/actionpack/lib/action_controller/url_rewriter.rb
+++ b/actionpack/lib/action_controller/url_rewriter.rb
@@ -1,153 +1,21 @@
-module ActionController
-  # In <b>routes.rb</b> one defines URL-to-controller mappings, but the reverse
-  # is also possible: an URL can be generated from one of your routing definitions.
-  # URL generation functionality is centralized in this module.
-  #
-  # See ActionController::Routing and ActionController::Resources for general
-  # information about routing and routes.rb.
-  #
-  # <b>Tip:</b> If you need to generate URLs from your models or some other place,
-  # then ActionController::UrlWriter is what you're looking for. Read on for
-  # an introduction.
-  #
-  # == URL generation from parameters
-  #
-  # As you may know, some functions - such as ActionController::Base#url_for
-  # and ActionView::Helpers::UrlHelper#link_to, can generate URLs given a set
-  # of parameters. For example, you've probably had the chance to write code
-  # like this in one of your views:
-  #
-  #   <%= link_to('Click here', :controller => 'users',
-  #           :action => 'new', :message => 'Welcome!') %>
-  #
-  #   #=> Generates a link to: /users/new?message=Welcome%21
-  #
-  # link_to, and all other functions that require URL generation functionality,
-  # actually use ActionController::UrlWriter under the hood. And in particular,
-  # they use the ActionController::UrlWriter#url_for method. One can generate
-  # the same path as the above example by using the following code:
-  #
-  #   include UrlWriter
-  #   url_for(:controller => 'users',
-  #           :action => 'new',
-  #           :message => 'Welcome!',
-  #           :only_path => true)
-  #   # => "/users/new?message=Welcome%21"
-  #
-  # Notice the <tt>:only_path => true</tt> part. This is because UrlWriter has no
-  # information about the website hostname that your Rails app is serving. So if you
-  # want to include the hostname as well, then you must also pass the <tt>:host</tt>
-  # argument:
-  #
-  #   include UrlWriter
-  #   url_for(:controller => 'users',
-  #           :action => 'new',
-  #           :message => 'Welcome!',
-  #           :host => 'www.example.com')        # Changed this.
-  #   # => "http://www.example.com/users/new?message=Welcome%21"
-  #
-  # By default, all controllers and views have access to a special version of url_for,
-  # that already knows what the current hostname is. So if you use url_for in your
-  # controllers or your views, then you don't need to explicitly pass the <tt>:host</tt>
-  # argument.
-  #
-  # For convenience reasons, mailers provide a shortcut for ActionController::UrlWriter#url_for.
-  # So within mailers, you only have to type 'url_for' instead of 'ActionController::UrlWriter#url_for'
-  # in full. However, mailers don't have hostname information, and what's why you'll still
-  # have to specify the <tt>:host</tt> argument when generating URLs in mailers.
-  #
-  #
-  # == URL generation for named routes
-  #
-  # UrlWriter also allows one to access methods that have been auto-generated from
-  # named routes. For example, suppose that you have a 'users' resource in your
-  # <b>routes.rb</b>:
-  #
-  #   map.resources :users
-  #
-  # This generates, among other things, the method <tt>users_path</tt>. By default,
-  # this method is accessible from your controllers, views and mailers. If you need
-  # to access this auto-generated method from other places (such as a model), then
-  # you can do that by including ActionController::UrlWriter in your class:
-  #
-  #   class User < ActiveRecord::Base
-  #     include ActionController::UrlWriter
-  #
-  #     def base_uri
-  #       user_path(self)
-  #     end
-  #   end
-  #
-  #   User.find(1).base_uri # => "/users/1"
-  module UrlWriter
-    def self.included(base) #:nodoc:
-      ActionController::Routing::Routes.install_helpers(base)
-      base.mattr_accessor :default_url_options
-
-      # The default options for urls written by this writer. Typically a <tt>:host</tt> pair is provided.
-      base.default_url_options ||= {}
-    end
-
-    # Generate a url based on the options provided, default_url_options and the
-    # routes defined in routes.rb.  The following options are supported:
-    #
-    # * <tt>:only_path</tt> - If true, the relative url is returned. Defaults to +false+.
-    # * <tt>:protocol</tt> - The protocol to connect to. Defaults to 'http'.
-    # * <tt>:host</tt> - Specifies the host the link should be targeted at.
-    #   If <tt>:only_path</tt> is false, this option must be
-    #   provided either explicitly, or via +default_url_options+.
-    # * <tt>:port</tt> - Optionally specify the port to connect to.
-    # * <tt>:anchor</tt> - An anchor name to be appended to the path.
-    # * <tt>:skip_relative_url_root</tt> - If true, the url is not constructed using the
-    #   +relative_url_root+ set in ActionController::Base.relative_url_root.
-    # * <tt>:trailing_slash</tt> - If true, adds a trailing slash, as in "/archive/2009/"
-    #
-    # Any other key (<tt>:controller</tt>, <tt>:action</tt>, etc.) given to
-    # +url_for+ is forwarded to the Routes module.
-    #
-    # Examples:
-    #
-    #    url_for :controller => 'tasks', :action => 'testing', :host=>'somehost.org', :port=>'8080'    # => 'http://somehost.org:8080/tasks/testing'
-    #    url_for :controller => 'tasks', :action => 'testing', :host=>'somehost.org', :anchor => 'ok', :only_path => true    # => '/tasks/testing#ok'
-    #    url_for :controller => 'tasks', :action => 'testing', :trailing_slash=>true  # => 'http://somehost.org/tasks/testing/'
-    #    url_for :controller => 'tasks', :action => 'testing', :host=>'somehost.org', :number => '33'  # => 'http://somehost.org/tasks/testing?number=33'
-    def url_for(options)
-      options = self.class.default_url_options.merge(options)
-
-      url = ''
-
-      unless options.delete(:only_path)
-        url << (options.delete(:protocol) || 'http')
-        url << '://' unless url.match("://")
-
-        raise "Missing host to link to! Please provide :host parameter or set default_url_options[:host]" unless options[:host]
-
-        url << options.delete(:host)
-        url << ":#{options.delete(:port)}" if options.key?(:port)
-      else
-        # Delete the unused options to prevent their appearance in the query string.
-        [:protocol, :host, :port, :skip_relative_url_root].each { |k| options.delete(k) }
-      end
-      trailing_slash = options.delete(:trailing_slash) if options.key?(:trailing_slash)
-      url << ActionController::Base.relative_url_root.to_s unless options[:skip_relative_url_root]
-      anchor = "##{Rack::Utils.escape options.delete(:anchor).to_param.to_s}" if options[:anchor]
-      generated = Routing::Routes.generate(options, {})
-      url << (trailing_slash ? generated.sub(/\?|\z/) { "/" + $& } : generated)
-      url << anchor if anchor
-
-      url
-    end
-  end
+require 'active_support/core_ext/hash/except'
 
+module ActionController
   # Rewrites URLs for Base.redirect_to and Base.url_for in the controller.
   class UrlRewriter #:nodoc:
     RESERVED_OPTIONS = [:anchor, :params, :only_path, :host, :protocol, :port, :trailing_slash, :skip_relative_url_root]
+
     def initialize(request, parameters)
       @request, @parameters = request, parameters
     end
 
     def rewrite(options = {})
-      rewrite_url(options)
+      options[:host]     ||= @request.host_with_port
+      options[:protocol] ||= @request.protocol
+
+      self.class.rewrite(options, @request.symbolized_path_parameters) do |options|
+        process_path_options(options)
+      end
     end
 
     def to_str
@@ -156,49 +24,53 @@ module ActionController
 
     alias_method :to_s, :to_str
 
-    private
-      # Given a path and options, returns a rewritten URL string
-      def rewrite_url(options)
-        rewritten_url = ""
-
-        unless options[:only_path]
-          rewritten_url << (options[:protocol] || @request.protocol)
-          rewritten_url << "://" unless rewritten_url.match("://")
-          rewritten_url << rewrite_authentication(options)
-          rewritten_url << (options[:host] || @request.host_with_port)
-          rewritten_url << ":#{options.delete(:port)}" if options.key?(:port)
-        end
-
-        path = rewrite_path(options)
-        rewritten_url << ActionController::Base.relative_url_root.to_s unless options[:skip_relative_url_root]
-        rewritten_url << (options[:trailing_slash] ? path.sub(/\?|\z/) { "/" + $& } : path)
-        rewritten_url << "##{Rack::Utils.escape(options[:anchor].to_param.to_s)}" if options[:anchor]
-
-        rewritten_url
+    def self.rewrite(options, path_segments=nil)
+      rewritten_url = ""
+
+      unless options[:only_path]
+        rewritten_url << (options[:protocol] || "http")
+        rewritten_url << "://" unless rewritten_url.match("://")
+        rewritten_url << rewrite_authentication(options)
+
+        raise "Missing host to link to! Please provide :host parameter or set default_url_options[:host]" unless options[:host]
+
+        rewritten_url << options[:host]
+        rewritten_url << ":#{options.delete(:port)}" if options.key?(:port)
       end
 
-      # Given a Hash of options, generates a route
-      def rewrite_path(options)
-        options = options.symbolize_keys
-        options.update(options[:params].symbolize_keys) if options[:params]
+      path_options = options.except(*RESERVED_OPTIONS)
+      path_options = yield(path_options) if block_given?
+      path = Routing::Routes.generate(path_options, path_segments || {})
+
+      rewritten_url << ActionController::Base.relative_url_root.to_s unless options[:skip_relative_url_root]
+      rewritten_url << (options[:trailing_slash] ? path.sub(/\?|\z/) { "/" + $& } : path)
+      rewritten_url << "##{Rack::Utils.escape(options[:anchor].to_param.to_s)}" if options[:anchor]
 
-        if (overwrite = options.delete(:overwrite_params))
-          options.update(@parameters.symbolize_keys)
-          options.update(overwrite.symbolize_keys)
-        end
+      rewritten_url
+    end
 
-        RESERVED_OPTIONS.each { |k| options.delete(k) }
+  protected
 
-        # Generates the query string, too
-        Routing::Routes.generate(options, @request.symbolized_path_parameters)
+    def self.rewrite_authentication(options)
+      if options[:user] && options[:password]
+        "#{Rack::Utils.escape(options.delete(:user))}:#{Rack::Utils.escape(options.delete(:password))}@"
+      else
+        ""
       end
+    end
+
+    # Given a Hash of options, generates a route
+    def process_path_options(options)
+      options = options.symbolize_keys
+      options.update(options[:params].symbolize_keys) if options[:params]
 
-      def rewrite_authentication(options)
-        if options[:user] && options[:password]
-          "#{Rack::Utils.escape(options.delete(:user))}:#{Rack::Utils.escape(options.delete(:password))}@"
-        else
-          ""
-        end
+      if (overwrite = options.delete(:overwrite_params))
+        options.update(@parameters.symbolize_keys)
+        options.update(overwrite.symbolize_keys)
       end
+
+      options
+    end
+
   end
 end
diff --git a/actionpack/test/abstract/url_for_test.rb b/actionpack/test/abstract/url_for_test.rb
new file mode 100644
index 0000000000..e5570349b8
--- /dev/null
+++ b/actionpack/test/abstract/url_for_test.rb
@@ -0,0 +1,272 @@
+require 'abstract_unit'
+
+module AbstractController
+  module Testing
+
+    class UrlForTests < ActionController::TestCase
+      class W
+        include AbstractController::UrlFor
+      end
+
+      def teardown
+        W.default_url_options.clear
+      end
+
+      def add_host!
+        W.default_url_options[:host] = 'www.basecamphq.com'
+      end
+
+      def test_exception_is_thrown_without_host
+        assert_raise RuntimeError do
+          W.new.url_for :controller => 'c', :action => 'a', :id => 'i'
+        end
+      end
+
+      def test_anchor
+        assert_equal('/c/a#anchor',
+          W.new.url_for(:only_path => true, :controller => 'c', :action => 'a', :anchor => 'anchor')
+        )
+      end
+
+      def test_anchor_should_call_to_param
+        assert_equal('/c/a#anchor',
+          W.new.url_for(:only_path => true, :controller => 'c', :action => 'a', :anchor => Struct.new(:to_param).new('anchor'))
+        )
+      end
+
+      def test_anchor_should_be_cgi_escaped
+        assert_equal('/c/a#anc%2Fhor',
+          W.new.url_for(:only_path => true, :controller => 'c', :action => 'a', :anchor => Struct.new(:to_param).new('anc/hor'))
+        )
+      end
+
+      def test_default_host
+        add_host!
+        assert_equal('http://www.basecamphq.com/c/a/i',
+          W.new.url_for(:controller => 'c', :action => 'a', :id => 'i')
+        )
+      end
+
+      def test_host_may_be_overridden
+        add_host!
+        assert_equal('http://37signals.basecamphq.com/c/a/i',
+          W.new.url_for(:host => '37signals.basecamphq.com', :controller => 'c', :action => 'a', :id => 'i')
+        )
+      end
+
+      def test_port
+        add_host!
+        assert_equal('http://www.basecamphq.com:3000/c/a/i',
+          W.new.url_for(:controller => 'c', :action => 'a', :id => 'i', :port => 3000)
+        )
+      end
+
+      def test_protocol
+        add_host!
+        assert_equal('https://www.basecamphq.com/c/a/i',
+          W.new.url_for(:controller => 'c', :action => 'a', :id => 'i', :protocol => 'https')
+        )
+      end
+
+      def test_protocol_with_and_without_separator
+        add_host!
+        assert_equal('https://www.basecamphq.com/c/a/i',
+          W.new.url_for(:controller => 'c', :action => 'a', :id => 'i', :protocol => 'https')
+        )
+        assert_equal('https://www.basecamphq.com/c/a/i',
+          W.new.url_for(:controller => 'c', :action => 'a', :id => 'i', :protocol => 'https://')
+        )
+      end
+
+      def test_trailing_slash
+        add_host!
+        options = {:controller => 'foo', :trailing_slash => true, :action => 'bar', :id => '33'}
+        assert_equal('http://www.basecamphq.com/foo/bar/33/', W.new.url_for(options) )
+      end
+
+      def test_trailing_slash_with_protocol
+        add_host!
+        options = { :trailing_slash => true,:protocol => 'https', :controller => 'foo', :action => 'bar', :id => '33'}
+        assert_equal('https://www.basecamphq.com/foo/bar/33/', W.new.url_for(options) )
+        assert_equal 'https://www.basecamphq.com/foo/bar/33/?query=string', W.new.url_for(options.merge({:query => 'string'}))
+      end
+
+      def test_trailing_slash_with_only_path
+        options = {:controller => 'foo', :trailing_slash => true}
+        assert_equal '/foo/', W.new.url_for(options.merge({:only_path => true}))
+        options.update({:action => 'bar', :id => '33'})
+        assert_equal '/foo/bar/33/', W.new.url_for(options.merge({:only_path => true}))
+        assert_equal '/foo/bar/33/?query=string', W.new.url_for(options.merge({:query => 'string',:only_path => true}))
+      end
+
+      def test_trailing_slash_with_anchor
+        options = {:trailing_slash => true, :controller => 'foo', :action => 'bar', :id => '33', :only_path => true, :anchor=> 'chapter7'}
+        assert_equal '/foo/bar/33/#chapter7', W.new.url_for(options)
+        assert_equal '/foo/bar/33/?query=string#chapter7', W.new.url_for(options.merge({:query => 'string'}))
+      end
+
+      def test_trailing_slash_with_params
+        url = W.new.url_for(:trailing_slash => true, :only_path => true, :controller => 'cont', :action => 'act', :p1 => 'cafe', :p2 => 'link')
+        params = extract_params(url)
+        assert_equal params[0], { :p1 => 'cafe' }.to_query
+        assert_equal params[1], { :p2 => 'link' }.to_query
+      end
+
+      def test_relative_url_root_is_respected
+        orig_relative_url_root = ActionController::Base.relative_url_root
+        ActionController::Base.relative_url_root = '/subdir'
+
+        add_host!
+        assert_equal('https://www.basecamphq.com/subdir/c/a/i',
+          W.new.url_for(:controller => 'c', :action => 'a', :id => 'i', :protocol => 'https')
+        )
+      ensure
+        ActionController::Base.relative_url_root = orig_relative_url_root
+      end
+
+      def test_named_routes
+        with_routing do |set|
+          set.draw do |map|
+            match 'this/is/verbose', :to => 'home#index', :as => :no_args
+            match 'home/sweet/home/:user', :to => 'home#index', :as => :home
+          end
+
+          # We need to create a new class in order to install the new named route.
+          kls = Class.new { include AbstractController::UrlFor }
+          controller = kls.new
+          assert controller.respond_to?(:home_url)
+          assert_equal 'http://www.basecamphq.com/home/sweet/home/again',
+            controller.send(:home_url, :host => 'www.basecamphq.com', :user => 'again')
+
+          assert_equal("/home/sweet/home/alabama", controller.send(:home_path, :user => 'alabama', :host => 'unused'))
+          assert_equal("http://www.basecamphq.com/home/sweet/home/alabama", controller.send(:home_url, :user => 'alabama', :host => 'www.basecamphq.com'))
+          assert_equal("http://www.basecamphq.com/this/is/verbose", controller.send(:no_args_url, :host=>'www.basecamphq.com'))
+        end
+      end
+
+      def test_relative_url_root_is_respected_for_named_routes
+        orig_relative_url_root = ActionController::Base.relative_url_root
+        ActionController::Base.relative_url_root = '/subdir'
+
+        with_routing do |set|
+          set.draw do |map|
+            match '/home/sweet/home/:user', :to => 'home#index', :as => :home
+          end
+
+          kls = Class.new { include AbstractController::UrlFor }
+          controller = kls.new
+
+          assert_equal 'http://www.basecamphq.com/subdir/home/sweet/home/again',
+            controller.send(:home_url, :host => 'www.basecamphq.com', :user => 'again')
+        end
+      ensure
+        ActionController::Base.relative_url_root = orig_relative_url_root
+      end
+
+      def test_only_path
+        with_routing do |set|
+          set.draw do |map|
+            match 'home/sweet/home/:user', :to => 'home#index', :as => :home
+            match ':controller/:action/:id'
+          end
+
+          # We need to create a new class in order to install the new named route.
+          kls = Class.new { include AbstractController::UrlFor }
+          controller = kls.new
+          assert controller.respond_to?(:home_url)
+          assert_equal '/brave/new/world',
+            controller.send(:url_for, :controller => 'brave', :action => 'new', :id => 'world', :only_path => true)
+
+          assert_equal("/home/sweet/home/alabama", controller.send(:home_url, :user => 'alabama', :host => 'unused', :only_path => true))
+          assert_equal("/home/sweet/home/alabama", controller.send(:home_path, 'alabama'))
+        end
+      end
+
+      def test_one_parameter
+        assert_equal('/c/a?param=val',
+          W.new.url_for(:only_path => true, :controller => 'c', :action => 'a', :param => 'val')
+        )
+      end
+
+      def test_two_parameters
+        url = W.new.url_for(:only_path => true, :controller => 'c', :action => 'a', :p1 => 'X1', :p2 => 'Y2')
+        params = extract_params(url)
+        assert_equal params[0], { :p1 => 'X1' }.to_query
+        assert_equal params[1], { :p2 => 'Y2' }.to_query
+      end
+
+      def test_hash_parameter
+        url = W.new.url_for(:only_path => true, :controller => 'c', :action => 'a', :query => {:name => 'Bob', :category => 'prof'})
+        params = extract_params(url)
+        assert_equal params[0], { 'query[category]' => 'prof' }.to_query
+        assert_equal params[1], { 'query[name]'     => 'Bob'  }.to_query
+      end
+
+      def test_array_parameter
+        url = W.new.url_for(:only_path => true, :controller => 'c', :action => 'a', :query => ['Bob', 'prof'])
+        params = extract_params(url)
+        assert_equal params[0], { 'query[]' => 'Bob'  }.to_query
+        assert_equal params[1], { 'query[]' => 'prof' }.to_query
+      end
+
+      def test_hash_recursive_parameters
+        url = W.new.url_for(:only_path => true, :controller => 'c', :action => 'a', :query => {:person => {:name => 'Bob', :position => 'prof'}, :hobby => 'piercing'})
+        params = extract_params(url)
+        assert_equal params[0], { 'query[hobby]'            => 'piercing' }.to_query
+        assert_equal params[1], { 'query[person][name]'     => 'Bob'      }.to_query
+        assert_equal params[2], { 'query[person][position]' => 'prof'     }.to_query
+      end
+
+      def test_hash_recursive_and_array_parameters
+        url = W.new.url_for(:only_path => true, :controller => 'c', :action => 'a', :id => 101, :query => {:person => {:name => 'Bob', :position => ['prof', 'art director']}, :hobby => 'piercing'})
+        assert_match %r(^/c/a/101), url
+        params = extract_params(url)
+        assert_equal params[0], { 'query[hobby]'              => 'piercing'     }.to_query
+        assert_equal params[1], { 'query[person][name]'       => 'Bob'          }.to_query
+        assert_equal params[2], { 'query[person][position][]' => 'art director' }.to_query
+        assert_equal params[3], { 'query[person][position][]' => 'prof'         }.to_query
+      end
+
+      def test_path_generation_for_symbol_parameter_keys
+        assert_generates("/image", :controller=> :image)
+      end
+
+      def test_named_routes_with_nil_keys
+        with_routing do |set|
+          set.draw do |map|
+            match 'posts.:format', :to => 'posts#index', :as => :posts
+            match '/', :to => 'posts#index', :as => :main
+          end
+
+          # We need to create a new class in order to install the new named route.
+          kls = Class.new { include AbstractController::UrlFor }
+          kls.default_url_options[:host] = 'www.basecamphq.com'
+
+          controller = kls.new
+          params = {:action => :index, :controller => :posts, :format => :xml}
+          assert_equal("http://www.basecamphq.com/posts.xml", controller.send(:url_for, params))
+          params[:format] = nil
+          assert_equal("http://www.basecamphq.com/", controller.send(:url_for, params))
+        end
+      end
+
+      def test_multiple_includes_maintain_distinct_options
+        first_class = Class.new { include AbstractController::UrlFor }
+        second_class = Class.new { include AbstractController::UrlFor }
+
+        first_host, second_host = 'firsthost.com', 'secondhost.com'
+
+        first_class.default_url_options[:host] = first_host
+        second_class.default_url_options[:host] = second_host
+
+        assert_equal first_class.default_url_options[:host], first_host
+        assert_equal second_class.default_url_options[:host], second_host
+      end
+
+      private
+        def extract_params(url)
+          url.split('?', 2).last.split('&').sort
+        end
+    end
+  end
+end
\ No newline at end of file
diff --git a/actionpack/test/controller/base_test.rb b/actionpack/test/controller/base_test.rb
index 65118f9bc9..92d2e75c20 100644
--- a/actionpack/test/controller/base_test.rb
+++ b/actionpack/test/controller/base_test.rb
@@ -205,7 +205,6 @@ class EnsureNamedRoutesWorksTicket22BugTest < ActionController::TestCase
       set.draw do |map|
         resources :things
       end
-      EmptyController.send :include, ActionController::UrlWriter
 
       assert_equal '/things', EmptyController.new.send(:things_path)
     end
diff --git a/actionpack/test/controller/url_rewriter_test.rb b/actionpack/test/controller/url_rewriter_test.rb
index 139d91f8ac..c2b8cd85d8 100644
--- a/actionpack/test/controller/url_rewriter_test.rb
+++ b/actionpack/test/controller/url_rewriter_test.rb
@@ -100,268 +100,3 @@ class UrlRewriterTests < ActionController::TestCase
   end
 end
 
-class UrlWriterTests < ActionController::TestCase
-  class W
-    include ActionController::UrlWriter
-  end
-
-  def teardown
-    W.default_url_options.clear
-  end
-
-  def add_host!
-    W.default_url_options[:host] = 'www.basecamphq.com'
-  end
-
-  def test_exception_is_thrown_without_host
-    assert_raise RuntimeError do
-      W.new.url_for :controller => 'c', :action => 'a', :id => 'i'
-    end
-  end
-
-  def test_anchor
-    assert_equal('/c/a#anchor',
-      W.new.url_for(:only_path => true, :controller => 'c', :action => 'a', :anchor => 'anchor')
-    )
-  end
-
-  def test_anchor_should_call_to_param
-    assert_equal('/c/a#anchor',
-      W.new.url_for(:only_path => true, :controller => 'c', :action => 'a', :anchor => Struct.new(:to_param).new('anchor'))
-    )
-  end
-
-  def test_anchor_should_be_cgi_escaped
-    assert_equal('/c/a#anc%2Fhor',
-      W.new.url_for(:only_path => true, :controller => 'c', :action => 'a', :anchor => Struct.new(:to_param).new('anc/hor'))
-    )
-  end
-
-  def test_default_host
-    add_host!
-    assert_equal('http://www.basecamphq.com/c/a/i',
-      W.new.url_for(:controller => 'c', :action => 'a', :id => 'i')
-    )
-  end
-
-  def test_host_may_be_overridden
-    add_host!
-    assert_equal('http://37signals.basecamphq.com/c/a/i',
-      W.new.url_for(:host => '37signals.basecamphq.com', :controller => 'c', :action => 'a', :id => 'i')
-    )
-  end
-
-  def test_port
-    add_host!
-    assert_equal('http://www.basecamphq.com:3000/c/a/i',
-      W.new.url_for(:controller => 'c', :action => 'a', :id => 'i', :port => 3000)
-    )
-  end
-
-  def test_protocol
-    add_host!
-    assert_equal('https://www.basecamphq.com/c/a/i',
-      W.new.url_for(:controller => 'c', :action => 'a', :id => 'i', :protocol => 'https')
-    )
-  end
-
-  def test_protocol_with_and_without_separator
-    add_host!
-    assert_equal('https://www.basecamphq.com/c/a/i',
-      W.new.url_for(:controller => 'c', :action => 'a', :id => 'i', :protocol => 'https')
-    )
-    assert_equal('https://www.basecamphq.com/c/a/i',
-      W.new.url_for(:controller => 'c', :action => 'a', :id => 'i', :protocol => 'https://')
-    )
-  end
-
-  def test_trailing_slash
-    add_host!
-    options = {:controller => 'foo', :trailing_slash => true, :action => 'bar', :id => '33'}
-    assert_equal('http://www.basecamphq.com/foo/bar/33/', W.new.url_for(options) )
-  end
-
-  def test_trailing_slash_with_protocol
-    add_host!
-    options = { :trailing_slash => true,:protocol => 'https', :controller => 'foo', :action => 'bar', :id => '33'}
-    assert_equal('https://www.basecamphq.com/foo/bar/33/', W.new.url_for(options) )
-    assert_equal 'https://www.basecamphq.com/foo/bar/33/?query=string', W.new.url_for(options.merge({:query => 'string'}))
-  end
-
-  def test_trailing_slash_with_only_path
-    options = {:controller => 'foo', :trailing_slash => true}
-    assert_equal '/foo/', W.new.url_for(options.merge({:only_path => true}))
-    options.update({:action => 'bar', :id => '33'})
-    assert_equal '/foo/bar/33/', W.new.url_for(options.merge({:only_path => true}))
-    assert_equal '/foo/bar/33/?query=string', W.new.url_for(options.merge({:query => 'string',:only_path => true}))
-  end
-
-  def test_trailing_slash_with_anchor
-    options = {:trailing_slash => true, :controller => 'foo', :action => 'bar', :id => '33', :only_path => true, :anchor=> 'chapter7'}
-    assert_equal '/foo/bar/33/#chapter7', W.new.url_for(options)
-    assert_equal '/foo/bar/33/?query=string#chapter7', W.new.url_for(options.merge({:query => 'string'}))
-  end
-
-  def test_trailing_slash_with_params
-    url = W.new.url_for(:trailing_slash => true, :only_path => true, :controller => 'cont', :action => 'act', :p1 => 'cafe', :p2 => 'link')
-    params = extract_params(url)
-    assert_equal params[0], { :p1 => 'cafe' }.to_query
-    assert_equal params[1], { :p2 => 'link' }.to_query
-  end
-
-  def test_relative_url_root_is_respected
-    orig_relative_url_root = ActionController::Base.relative_url_root
-    ActionController::Base.relative_url_root = '/subdir'
-
-    add_host!
-    assert_equal('https://www.basecamphq.com/subdir/c/a/i',
-      W.new.url_for(:controller => 'c', :action => 'a', :id => 'i', :protocol => 'https')
-    )
-  ensure
-    ActionController::Base.relative_url_root = orig_relative_url_root
-  end
-
-  def test_named_routes
-    with_routing do |set|
-      set.draw do |map|
-        match 'this/is/verbose', :to => 'home#index', :as => :no_args
-        match 'home/sweet/home/:user', :to => 'home#index', :as => :home
-      end
-
-      # We need to create a new class in order to install the new named route.
-      kls = Class.new { include ActionController::UrlWriter }
-      controller = kls.new
-      assert controller.respond_to?(:home_url)
-      assert_equal 'http://www.basecamphq.com/home/sweet/home/again',
-        controller.send(:home_url, :host => 'www.basecamphq.com', :user => 'again')
-
-      assert_equal("/home/sweet/home/alabama", controller.send(:home_path, :user => 'alabama', :host => 'unused'))
-      assert_equal("http://www.basecamphq.com/home/sweet/home/alabama", controller.send(:home_url, :user => 'alabama', :host => 'www.basecamphq.com'))
-      assert_equal("http://www.basecamphq.com/this/is/verbose", controller.send(:no_args_url, :host=>'www.basecamphq.com'))
-    end
-  end
-
-  def test_relative_url_root_is_respected_for_named_routes
-    orig_relative_url_root = ActionController::Base.relative_url_root
-    ActionController::Base.relative_url_root = '/subdir'
-
-    with_routing do |set|
-      set.draw do |map|
-        match '/home/sweet/home/:user', :to => 'home#index', :as => :home
-      end
-
-      kls = Class.new { include ActionController::UrlWriter }
-      controller = kls.new
-
-      assert_equal 'http://www.basecamphq.com/subdir/home/sweet/home/again',
-        controller.send(:home_url, :host => 'www.basecamphq.com', :user => 'again')
-    end
-  ensure
-    ActionController::Base.relative_url_root = orig_relative_url_root
-  end
-
-  def test_only_path
-    with_routing do |set|
-      set.draw do |map|
-        match 'home/sweet/home/:user', :to => 'home#index', :as => :home
-        match ':controller/:action/:id'
-      end
-
-      # We need to create a new class in order to install the new named route.
-      kls = Class.new { include ActionController::UrlWriter }
-      controller = kls.new
-      assert controller.respond_to?(:home_url)
-      assert_equal '/brave/new/world',
-        controller.send(:url_for, :controller => 'brave', :action => 'new', :id => 'world', :only_path => true)
-
-      assert_equal("/home/sweet/home/alabama", controller.send(:home_url, :user => 'alabama', :host => 'unused', :only_path => true))
-      assert_equal("/home/sweet/home/alabama", controller.send(:home_path, 'alabama'))
-    end
-  end
-
-  def test_one_parameter
-    assert_equal('/c/a?param=val',
-      W.new.url_for(:only_path => true, :controller => 'c', :action => 'a', :param => 'val')
-    )
-  end
-
-  def test_two_parameters
-    url = W.new.url_for(:only_path => true, :controller => 'c', :action => 'a', :p1 => 'X1', :p2 => 'Y2')
-    params = extract_params(url)
-    assert_equal params[0], { :p1 => 'X1' }.to_query
-    assert_equal params[1], { :p2 => 'Y2' }.to_query
-  end
-
-  def test_hash_parameter
-    url = W.new.url_for(:only_path => true, :controller => 'c', :action => 'a', :query => {:name => 'Bob', :category => 'prof'})
-    params = extract_params(url)
-    assert_equal params[0], { 'query[category]' => 'prof' }.to_query
-    assert_equal params[1], { 'query[name]'     => 'Bob'  }.to_query
-  end
-
-  def test_array_parameter
-    url = W.new.url_for(:only_path => true, :controller => 'c', :action => 'a', :query => ['Bob', 'prof'])
-    params = extract_params(url)
-    assert_equal params[0], { 'query[]' => 'Bob'  }.to_query
-    assert_equal params[1], { 'query[]' => 'prof' }.to_query
-  end
-
-  def test_hash_recursive_parameters
-    url = W.new.url_for(:only_path => true, :controller => 'c', :action => 'a', :query => {:person => {:name => 'Bob', :position => 'prof'}, :hobby => 'piercing'})
-    params = extract_params(url)
-    assert_equal params[0], { 'query[hobby]'            => 'piercing' }.to_query
-    assert_equal params[1], { 'query[person][name]'     => 'Bob'      }.to_query
-    assert_equal params[2], { 'query[person][position]' => 'prof'     }.to_query
-  end
-
-  def test_hash_recursive_and_array_parameters
-    url = W.new.url_for(:only_path => true, :controller => 'c', :action => 'a', :id => 101, :query => {:person => {:name => 'Bob', :position => ['prof', 'art director']}, :hobby => 'piercing'})
-    assert_match %r(^/c/a/101), url
-    params = extract_params(url)
-    assert_equal params[0], { 'query[hobby]'              => 'piercing'     }.to_query
-    assert_equal params[1], { 'query[person][name]'       => 'Bob'          }.to_query
-    assert_equal params[2], { 'query[person][position][]' => 'art director' }.to_query
-    assert_equal params[3], { 'query[person][position][]' => 'prof'         }.to_query
-  end
-
-  def test_path_generation_for_symbol_parameter_keys
-    assert_generates("/image", :controller=> :image)
-  end
-
-  def test_named_routes_with_nil_keys
-    with_routing do |set|
-      set.draw do |map|
-        match 'posts.:format', :to => 'posts#index', :as => :posts
-        match '/', :to => 'posts#index', :as => :main
-      end
-
-      # We need to create a new class in order to install the new named route.
-      kls = Class.new { include ActionController::UrlWriter }
-      kls.default_url_options[:host] = 'www.basecamphq.com'
-
-      controller = kls.new
-      params = {:action => :index, :controller => :posts, :format => :xml}
-      assert_equal("http://www.basecamphq.com/posts.xml", controller.send(:url_for, params))
-      params[:format] = nil
-      assert_equal("http://www.basecamphq.com/", controller.send(:url_for, params))
-    end
-  end
-
-  def test_multiple_includes_maintain_distinct_options
-    first_class = Class.new { include ActionController::UrlWriter }
-    second_class = Class.new { include ActionController::UrlWriter }
-
-    first_host, second_host = 'firsthost.com', 'secondhost.com'
-
-    first_class.default_url_options[:host] = first_host
-    second_class.default_url_options[:host] = second_host
-
-    assert_equal first_class.default_url_options[:host], first_host
-    assert_equal second_class.default_url_options[:host], second_host
-  end
-
-  private
-    def extract_params(url)
-      url.split('?', 2).last.split('&').sort
-    end
-end