Continued effort to deglobalize the router

10 files changed, 66 insertions, 79 deletions

diff --git a/actionpack/lib/action_controller.rb b/actionpack/lib/action_controller.rb
index 93d8fb612f..759e52b135 100644
--- a/actionpack/lib/action_controller.rb
+++ b/actionpack/lib/action_controller.rb
@@ -32,7 +32,7 @@ module ActionController
     autoload :SessionManagement
     autoload :Streaming
     autoload :Testing
-    # ROUTES TODO: Proxy UrlFor to Rails.application.routes.url_helpers
+    autoload :UrlFor
     autoload :Verification
   end
 
diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb
index 0777d9dc16..f8ddc8da09 100644
--- a/actionpack/lib/action_controller/base.rb
+++ b/actionpack/lib/action_controller/base.rb
@@ -9,7 +9,7 @@ module ActionController
     include ActionController::Helpers
     
     include ActionController::HideActions
-    # include ActionController::UrlFor
+    include ActionController::UrlFor
     include ActionController::Redirecting
     include ActionController::Rendering
     include ActionController::Renderers::All
@@ -82,12 +82,5 @@ module ActionController
       filter
     end
 
-  protected
-
-    # Overwrite url rewriter to use request.
-    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/metal/url_for.rb b/actionpack/lib/action_controller/metal/url_for.rb
new file mode 100644
index 0000000000..013834ddee
--- /dev/null
+++ b/actionpack/lib/action_controller/metal/url_for.rb
@@ -0,0 +1,21 @@
+module ActionController
+  module UrlFor
+    extend ActiveSupport::Concern
+
+    include ActionDispatch::Routing::UrlFor
+    include ActionController::RackDelegation
+
+    def merge_options(options)
+      super.reverse_merge(
+        :host => request.host_with_port,
+        :protocol => request.protocol,
+        :_path_segments => request.symbolized_path_parameters
+      )
+    end
+
+    def _router
+      raise "In order to use #url_for, you must include the helpers of a particular " \
+            "router. For instance, `include Rails.application.router.named_url_helpers"
+    end
+  end
+end
\ No newline at end of file
diff --git a/actionpack/lib/action_controller/railtie.rb b/actionpack/lib/action_controller/railtie.rb
index 015a8212c4..5a16d82e49 100644
--- a/actionpack/lib/action_controller/railtie.rb
+++ b/actionpack/lib/action_controller/railtie.rb
@@ -1,6 +1,7 @@
 require "action_controller"
 require "rails"
 require "action_view/railtie"
+require "active_support/core_ext/class/subclasses"
 
 module ActionController
   class Railtie < Rails::Railtie
diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb
index 5f8bc6f325..0fa2a8b90c 100644
--- a/actionpack/lib/action_controller/test_case.rb
+++ b/actionpack/lib/action_controller/test_case.rb
@@ -336,7 +336,7 @@ module ActionController
     private
       def build_request_uri(action, parameters)
         unless @request.env['REQUEST_URI']
-          options = @controller.__send__(:rewrite_options, parameters)
+          options = @controller.__send__(:merge_options, parameters)
           options.update(:only_path => true, :action => action)
 
           url = ActionController::UrlRewriter.new(@request, parameters)
diff --git a/actionpack/lib/action_controller/url_rewriter.rb b/actionpack/lib/action_controller/url_rewriter.rb
index 272465d4f5..807b21cd0e 100644
--- a/actionpack/lib/action_controller/url_rewriter.rb
+++ b/actionpack/lib/action_controller/url_rewriter.rb
@@ -26,8 +26,14 @@ module ActionController
 
     # ROUTES TODO: Class method code smell
     def self.rewrite(router, options, path_segments=nil)
+      handle_positional_args(options)
+
       rewritten_url = ""
 
+      # ROUTES TODO: Fix the tests
+      segments = options.delete(:_path_segments)
+      path_segments = path_segments ? path_segments.merge(segments || {}) : segments
+
       unless options[:only_path]
         rewritten_url << (options[:protocol] || "http")
         rewritten_url << "://" unless rewritten_url.match("://")
@@ -52,6 +58,21 @@ module ActionController
 
   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))}@"
diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb
index 8b761d393f..7bfe4fa2bf 100644
--- a/actionpack/lib/action_dispatch/routing/route_set.rb
+++ b/actionpack/lib/action_dispatch/routing/route_set.rb
@@ -194,24 +194,11 @@ module ActionDispatch
             # end
             @module.module_eval <<-END_EVAL, __FILE__, __LINE__ + 1
               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 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[:use_defaults] = false
-                  options.merge!(args)
+                options =  #{hash_access_method}(args.extract_options!)
+
+                if args.any?
+                  options[:_positional_args] = args
+                  options[:_positional_keys] = #{route.segment_keys.inspect}
                 end
 
                 url_for(options)
diff --git a/actionpack/lib/action_dispatch/routing/url_for.rb b/actionpack/lib/action_dispatch/routing/url_for.rb
index 64c695fe13..1d6da817ed 100644
--- a/actionpack/lib/action_dispatch/routing/url_for.rb
+++ b/actionpack/lib/action_dispatch/routing/url_for.rb
@@ -85,8 +85,6 @@ module ActionDispatch
       extend ActiveSupport::Concern
 
       included do
-        # ActionDispatch::Routing::Routes.install_helpers(self)
-
         # Including in a class uses an inheritable hash. Modules get a plain hash.
         if respond_to?(:class_attribute)
           class_attribute :default_url_options
@@ -97,23 +95,16 @@ module ActionDispatch
         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)
-        # ROUTES TODO: This should probably be an instance method
         self.class.default_url_options
       end
 
-      def rewrite_options(options) #:nodoc:
-        if options.delete(:use_defaults) != false && (defaults = default_url_options(options))
+      def url_options
+        self.class.default_url_options.merge
+      end
+
+      def merge_options(options) #:nodoc:
+        if options.delete(:use_defaults) != false && respond_to?(:default_url_options) && (defaults = default_url_options(options))
           defaults.merge(options)
         else
           options
@@ -149,19 +140,11 @@ module ActionDispatch
           when String
             options
           when Hash
-            _url_rewriter.rewrite(_router, rewrite_options(options))
+            ActionController::UrlRewriter.rewrite(_router, merge_options(options))
           else
             polymorphic_url(options)
         end
       end
-
-    protected
-
-      # ROUTES TODO: Figure out why _url_rewriter is sometimes the class and
-      # sometimes an instance.
-      def _url_rewriter
-        ActionController::UrlRewriter
-      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 25006dda76..436ee212c8 100644
--- a/actionpack/test/controller/base_test.rb
+++ b/actionpack/test/controller/base_test.rb
@@ -220,6 +220,7 @@ class EmptyUrlOptionsTest < ActionController::TestCase
 
   def test_named_routes_with_path_without_doing_a_request_first
     @controller = EmptyController.new
+    @controller.request = @request
 
     with_routing do |set|
       set.draw do |map|
diff --git a/actionpack/test/controller/routing_test.rb b/actionpack/test/controller/routing_test.rb
index f390bbdc89..f2fccfbcfc 100644
--- a/actionpack/test/controller/routing_test.rb
+++ b/actionpack/test/controller/routing_test.rb
@@ -52,29 +52,11 @@ class UriReservedCharactersRoutingTest < Test::Unit::TestCase
 end
 
 class MockController
-  attr_accessor :routes
-
-  def initialize(routes)
-    self.routes = routes
-  end
-
   def url_for(options)
-    only_path = options.delete(:only_path)
-
-    port        = options.delete(:port) || 80
-    port_string = port == 80 ? '' : ":#{port}"
-
-    protocol = options.delete(:protocol) || "http"
-    host     = options.delete(:host) || "test.host"
-    anchor   = "##{options.delete(:anchor)}" if options.key?(:anchor)
+    options[:protocol] ||= "http"
+    options[:host] ||= "test.host"
 
-    path = routes.generate(options)
-
-    only_path ? "#{path}#{anchor}" : "#{protocol}://#{host}#{port_string}#{path}#{anchor}"
-  end
-
-  def request
-    @request ||= ActionController::TestRequest.new
+    super(options)
   end
 end
 
@@ -268,9 +250,9 @@ class LegacyRouteSetTests < Test::Unit::TestCase
   end
 
   def setup_for_named_route
-    klass = Class.new(MockController)
-    rs.install_helpers(klass)
-    klass.new(rs)
+    inst = MockController.clone.new
+    inst.class.send(:include, rs.named_url_helpers)
+    inst
   end
 
   def test_named_route_without_hash
@@ -759,9 +741,7 @@ class RouteSetTest < ActiveSupport::TestCase
       map.users '/admin/users', :controller => 'admin/users', :action => 'index'
     end
 
-    klass = Class.new(MockController)
-    set.install_helpers(klass)
-    klass.new(set)
+    MockController.clone.new.tap { |inst| inst.class.send(:include, set.named_url_helpers)}
   end
 
   def test_named_route_hash_access_method