Tweak out url_for uses :script_name and add some tests

6 files changed, 71 insertions, 24 deletions

diff --git a/actionpack/lib/action_controller/metal/url_for.rb b/actionpack/lib/action_controller/metal/url_for.rb
index 89ab0b4753..f6b6cb2ff4 100644
--- a/actionpack/lib/action_controller/metal/url_for.rb
+++ b/actionpack/lib/action_controller/metal/url_for.rb
@@ -9,12 +9,8 @@ module ActionController
       super.reverse_merge(
         :host => request.host_with_port,
         :protocol => request.protocol,
-        # ROUTES TODO: relative_url_root should be middleware
-        # and the generator should take SCRIPT_NAME into
-        # consideration
-        :script_name => request.env["SCRIPT_NAME"],
         :_path_segments => request.symbolized_path_parameters
-      )
+      ).merge(:script_name => request.script_name)
     end
 
     def _router
diff --git a/actionpack/lib/action_controller/railtie.rb b/actionpack/lib/action_controller/railtie.rb
index e638be5251..e9edf80451 100644
--- a/actionpack/lib/action_controller/railtie.rb
+++ b/actionpack/lib/action_controller/railtie.rb
@@ -51,12 +51,6 @@ module ActionController
       ac.stylesheets_dir = paths.public.stylesheets.to_a.first
       ac.secret = app.config.cookie_secret
 
-      if ac.relative_url_root
-        ActiveSupport::Deprecation.warn "config.action_controller.relative_url_root " \
-          "is no longer effective. Please set it in the router as " \
-          "routes.draw(:script_name => #{ac.relative_url_root.inspect})"
-      end
-
       ActionController::Base.config.replace(ac)
     end
 
diff --git a/actionpack/lib/action_controller/url_rewriter.rb b/actionpack/lib/action_controller/url_rewriter.rb
index 8821892b3a..981865517f 100644
--- a/actionpack/lib/action_controller/url_rewriter.rb
+++ b/actionpack/lib/action_controller/url_rewriter.rb
@@ -32,7 +32,6 @@ module ActionController
 
       # ROUTES TODO: Fix the tests
       segments = options.delete(:_path_segments)
-      relative_url_root = options.delete(:script_name).to_s
       path_segments = path_segments ? path_segments.merge(segments || {}) : segments
 
       unless options[:only_path]
@@ -50,8 +49,7 @@ module ActionController
       path_options = yield(path_options) if block_given?
       path = router.generate(path_options, path_segments || {})
 
-      # ROUTES TODO: This can be called directly, so relative_url_root should probably be set in the router
-      rewritten_url << relative_url_root
+      # 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]
 
diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb
index 26189329c2..a53d8067dd 100644
--- a/actionpack/lib/action_dispatch/routing/route_set.rb
+++ b/actionpack/lib/action_dispatch/routing/route_set.rb
@@ -209,29 +209,27 @@ module ActionDispatch
           end
       end
 
-      attr_accessor :routes, :named_routes
+      attr_accessor :routes, :named_routes, :mount_point
       attr_accessor :disable_clear_and_finalize, :resources_path_names
-      attr_accessor :script_name
 
       def self.default_resources_path_names
         { :new => 'new', :edit => 'edit' }
       end
 
-      def initialize
+      def initialize(options = {})
         self.routes = []
         self.named_routes = NamedRouteCollection.new
         self.resources_path_names = self.class.default_resources_path_names.dup
         self.controller_namespaces = Set.new
+        self.mount_point = options[:mount_point]
 
         @disable_clear_and_finalize = false
         clear!
       end
 
-      def draw(options = {}, &block)
+      def draw(&block)
         clear! unless @disable_clear_and_finalize
 
-        @script_name = options[:script_name]
-
         mapper = Mapper.new(self)
         if block.arity == 1
           mapper.instance_exec(DeprecatedMapper.new(self), &block)
@@ -341,6 +339,7 @@ module ActionDispatch
 
       def generate(options, recall = {}, method = :generate)
         options, recall = options.dup, recall.dup
+        script_name = options.delete(:script_name) || mount_point
         named_route = options.delete(:use_route)
 
         options = options_as_params(options)
@@ -406,6 +405,7 @@ module ActionDispatch
           [path, params.keys]
         elsif path
           path << "?#{params.to_query}" if params.any?
+          path = "#{script_name}#{path}"
           path
         else
           raise ActionController::RoutingError, "No route matches #{options.inspect}"
diff --git a/actionpack/lib/action_dispatch/routing/url_for.rb b/actionpack/lib/action_dispatch/routing/url_for.rb
index afbb2ecf16..be5edcc120 100644
--- a/actionpack/lib/action_dispatch/routing/url_for.rb
+++ b/actionpack/lib/action_dispatch/routing/url_for.rb
@@ -102,14 +102,15 @@ module ActionDispatch
 
       def url_options
         @url_options ||= begin
-          opts = self.class.default_url_options
-          opts.merge(:script_name => _router.script_name) if respond_to?(:_router)
-          opts
+          # self.class does not respond to default_url_options when the helpers are extended
+          # onto a singleton
+          self.class.respond_to?(:default_url_options) ? self.class.default_url_options : {}
         end
       end
 
       def url_options=(options)
-        @url_options = options
+        defaults = self.class.respond_to?(:default_url_options) ? self.class.default_url_options : {}
+        @url_options = defaults.merge(options)
       end
 
       # def merge_options(options) #:nodoc:
diff --git a/actionpack/test/dispatch/url_generation_test.rb b/actionpack/test/dispatch/url_generation_test.rb
new file mode 100644
index 0000000000..b55e001fa7
--- /dev/null
+++ b/actionpack/test/dispatch/url_generation_test.rb
@@ -0,0 +1,58 @@
+require 'abstract_unit'
+
+module TestUrlGeneration
+  class WithoutMountpoint < ActiveSupport::TestCase
+    setup do
+      app = lambda { |env| [200, {}, "Hello"] }
+      @router = ActionDispatch::Routing::RouteSet.new
+      @router.draw do
+        match "/foo", :to => app, :as => :foo
+      end
+
+      singleton_class.send(:include, @router.url_helpers)
+    end
+
+    test "generating URLS normally" do
+      assert_equal "/foo", foo_path
+    end
+
+    test "accepting a :script_name option" do
+      assert_equal "/bar/foo", foo_path(:script_name => "/bar")
+    end
+  end
+
+  class WithMountPoint < ActionDispatch::IntegrationTest
+    Router = ActionDispatch::Routing::RouteSet.new(:mount_point => "/baz")
+    Router.draw { match "/foo", :to => "my_route_generating#index", :as => :foo }
+
+    class ::MyRouteGeneratingController < ActionController::Base
+      include Router.url_helpers
+      def index
+        render :text => foo_path
+      end
+    end
+
+    include Router.url_helpers
+
+    def _router
+      Router
+    end
+
+    def app
+      Router
+    end
+
+    test "using the default :script_name option" do
+      assert_equal "/baz/foo", foo_path
+    end
+
+    test "overriding the default :script_name option" do
+      assert_equal "/bar/foo", foo_path(:script_name => "/bar")
+    end
+
+    test "the request's SCRIPT_NAME takes precedence over the router's" do
+      get "/foo", {}, 'SCRIPT_NAME' => "/new"
+      assert_equal "/new/foo", response.body
+    end
+  end
+end
\ No newline at end of file