Move AC::UrlRewriter onto route set

7 files changed, 76 insertions, 90 deletions

diff --git a/actionpack/lib/action_controller.rb b/actionpack/lib/action_controller.rb
index 3b97f38498..536154fc6b 100644
--- a/actionpack/lib/action_controller.rb
+++ b/actionpack/lib/action_controller.rb
@@ -46,7 +46,6 @@ module ActionController
 
   eager_autoload do
     autoload :RecordIdentifier
-    autoload :UrlRewriter
 
     # TODO: Don't autoload exceptions, setup explicit
     # requires for files that need them
diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb
index c43e560ac1..072d289964 100644
--- a/actionpack/lib/action_controller/test_case.rb
+++ b/actionpack/lib/action_controller/test_case.rb
@@ -335,23 +335,22 @@ module ActionController
       @request.remote_addr = '208.77.188.166' # example.com
     end
 
-  private
-    def build_request_uri(action, parameters)
-      unless @request.env["PATH_INFO"]
-        options = @controller.__send__(:url_options).merge(parameters)
-        options.update(
-          :only_path => true,
-          :action => action,
-          :relative_url_root => nil,
-          :_path_segments => @request.symbolized_path_parameters)
-
-        rewriter = ActionController::UrlRewriter
-        url, query_string = rewriter.rewrite(@router, options).split("?", 2)
-
-        @request.env["SCRIPT_NAME"] = @controller.config.relative_url_root
-        @request.env["PATH_INFO"] = url
-        @request.env["QUERY_STRING"] = query_string || ""
+    private
+      def build_request_uri(action, parameters)
+        unless @request.env["PATH_INFO"]
+          options = @controller.__send__(:url_options).merge(parameters)
+          options.update(
+            :only_path => true,
+            :action => action,
+            :relative_url_root => nil,
+            :_path_segments => @request.symbolized_path_parameters)
+
+          url, query_string = @router.rewrite(options).split("?", 2)
+
+          @request.env["SCRIPT_NAME"] = @controller.config.relative_url_root
+          @request.env["PATH_INFO"] = url
+          @request.env["QUERY_STRING"] = query_string || ""
+        end
       end
     end
-  end
 end
diff --git a/actionpack/lib/action_controller/url_rewriter.rb b/actionpack/lib/action_controller/url_rewriter.rb
deleted file mode 100644
index b387d49593..0000000000
--- a/actionpack/lib/action_controller/url_rewriter.rb
+++ /dev/null
@@ -1,64 +0,0 @@
-require 'active_support/core_ext/hash/except'
-
-module ActionController
-  # Rewrites URLs for Base.redirect_to and Base.url_for in the controller.
-  module UrlRewriter #:nodoc:
-    RESERVED_OPTIONS = [:anchor, :params, :only_path, :host, :protocol, :port, :trailing_slash, :skip_relative_url_root]
-
-    # ROUTES TODO: Class method code smell
-    def self.rewrite(router, options)
-      handle_positional_args(options)
-
-      rewritten_url = ""
-
-      path_segments = options.delete(:_path_segments)
-
-      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
-
-      path_options = options.except(*RESERVED_OPTIONS)
-      path_options = yield(path_options) if block_given?
-      path = router.generate(path_options, path_segments || {})
-
-      # ROUTES TODO: This can be called directly, so script_name should probably be set in the router
-      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
-    end
-
-  protected
-
-    def self.handle_positional_args(options)
-      return unless args = options.delete(:_positional_args)
-
-      keys = options.delete(:_positional_keys)
-      keys -= options.keys if args.size < keys.size - 1 # take format into account
-
-      args = args.zip(keys).inject({}) do |h, (v, k)|
-        h[k] = v
-        h
-      end
-
-      # Tell url_for to skip default_url_options
-      options.merge!(args)
-    end
-
-    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
-
-  end
-end
diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb
index e99e39269b..15c7cd00ba 100644
--- a/actionpack/lib/action_dispatch/routing/route_set.rb
+++ b/actionpack/lib/action_dispatch/routing/route_set.rb
@@ -410,6 +410,38 @@ module ActionDispatch
         Generator.new(options, recall, @set, extras).generate
       end
 
+      RESERVED_OPTIONS = [:anchor, :params, :only_path, :host, :protocol, :port, :trailing_slash, :skip_relative_url_root]
+
+      # TODO: Terrible method name
+      def rewrite(options)
+        handle_positional_args(options)
+
+        rewritten_url = ""
+
+        path_segments = options.delete(:_path_segments)
+
+        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
+
+        path_options = options.except(*RESERVED_OPTIONS)
+        path_options = yield(path_options) if block_given?
+        path = generate(path_options, path_segments || {})
+
+        # ROUTES TODO: This can be called directly, so script_name should probably be set in the router
+        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
+      end
+
       def call(env)
         @set.call(env)
       end
@@ -435,6 +467,30 @@ module ActionDispatch
 
         raise ActionController::RoutingError, "No route matches #{path.inspect}"
       end
+
+      private
+        def handle_positional_args(options)
+          return unless args = options.delete(:_positional_args)
+
+          keys = options.delete(:_positional_keys)
+          keys -= options.keys if args.size < keys.size - 1 # take format into account
+
+          args = args.zip(keys).inject({}) do |h, (v, k)|
+            h[k] = v
+            h
+          end
+
+          # Tell url_for to skip default_url_options
+          options.merge!(args)
+        end
+
+        def rewrite_authentication(options)
+          if options[:user] && options[:password]
+            "#{Rack::Utils.escape(options.delete(:user))}:#{Rack::Utils.escape(options.delete(:password))}@"
+          else
+            ""
+          end
+        end
     end
   end
 end
diff --git a/actionpack/lib/action_dispatch/routing/url_for.rb b/actionpack/lib/action_dispatch/routing/url_for.rb
index 4629e7e002..0ba04316d7 100644
--- a/actionpack/lib/action_dispatch/routing/url_for.rb
+++ b/actionpack/lib/action_dispatch/routing/url_for.rb
@@ -131,7 +131,7 @@ module ActionDispatch
         when String
           options
         when nil, Hash
-          ActionController::UrlRewriter.rewrite(_router, url_options.merge(options || {}))
+          _router.rewrite(url_options.merge(options || {}))
         else
           polymorphic_url(options)
         end
diff --git a/actionpack/test/controller/caching_test.rb b/actionpack/test/controller/caching_test.rb
index 2be91893d4..08efd5a48c 100644
--- a/actionpack/test/controller/caching_test.rb
+++ b/actionpack/test/controller/caching_test.rb
@@ -64,7 +64,6 @@ class PageCachingTest < ActionController::TestCase
     @controller.cache_store = :file_store, FILE_STORE_PATH
 
     @params = {:controller => 'posts', :action => 'index', :only_path => true, :skip_relative_url_root => true}
-    @rewriter = ActionController::UrlRewriter
 
     FileUtils.rm_rf(File.dirname(FILE_STORE_PATH))
     FileUtils.mkdir_p(FILE_STORE_PATH)
@@ -82,9 +81,9 @@ class PageCachingTest < ActionController::TestCase
         match '/', :to => 'posts#index', :as => :main
       end
       @params[:format] = 'rss'
-      assert_equal '/posts.rss', @rewriter.rewrite(@router, @params)
+      assert_equal '/posts.rss', @router.rewrite(@params)
       @params[:format] = nil
-      assert_equal '/', @rewriter.rewrite(@router, @params)
+      assert_equal '/', @router.rewrite(@params)
     end
   end
 
diff --git a/actionpack/test/controller/url_rewriter_test.rb b/actionpack/test/controller/url_rewriter_test.rb
index 3bc8bec3fb..cf775846df 100644
--- a/actionpack/test/controller/url_rewriter_test.rb
+++ b/actionpack/test/controller/url_rewriter_test.rb
@@ -1,8 +1,6 @@
 require 'abstract_unit'
 require 'controller/fake_controllers'
 
-ActionController::UrlRewriter
-
 class UrlRewriterTests < ActionController::TestCase
   class Rewriter
     def initialize(request)
@@ -13,8 +11,7 @@ class UrlRewriterTests < ActionController::TestCase
     end
 
     def rewrite(router, options)
-      options = @options.merge(options)
-      ActionController::UrlRewriter.rewrite(router, options)
+      router.rewrite(@options.merge(options))
     end
   end