Work on deprecating ActionController::Base.relative_url_root

8 files changed, 70 insertions, 60 deletions

diff --git a/actionpack/lib/action_controller/metal/compatibility.rb b/actionpack/lib/action_controller/metal/compatibility.rb
index d1c86b296d..93f7b8ca49 100644
--- a/actionpack/lib/action_controller/metal/compatibility.rb
+++ b/actionpack/lib/action_controller/metal/compatibility.rb
@@ -7,13 +7,17 @@ module ActionController
     class ::ActionController::ActionControllerError < StandardError #:nodoc:
     end
 
+    module ClassMethods
+    end
+
     # Temporary hax
     included do
       ::ActionController::UnknownAction = ::AbstractController::ActionNotFound
       ::ActionController::DoubleRenderError = ::AbstractController::DoubleRenderError
 
-      cattr_accessor :relative_url_root
-      self.relative_url_root = ENV['RAILS_RELATIVE_URL_ROOT']
+      # ROUTES TODO: This should be handled by a middleware and route generation
+      # should be able to handle SCRIPT_NAME
+      self.config.relative_url_root = ENV['RAILS_RELATIVE_URL_ROOT']
 
       class << self
         delegate :default_charset=, :to => "ActionDispatch::Response"
@@ -47,6 +51,39 @@ module ActionController
       cattr_accessor :trusted_proxies
     end
 
+    def self.deprecated_config_accessor(option, message = nil)
+      deprecated_config_reader(option, message)
+      deprecated_config_writer(option, message)
+    end
+
+    def self.deprecated_config_reader(option, message = nil)
+      message ||= "Reading #{option} directly from ActionController::Base is deprecated. " \
+                  "Please read it from config.#{option}"
+
+      ClassMethods.class_eval <<-RUBY, __FILE__, __LINE__ + 1
+        def #{option}
+          ActiveSupport::Deprecation.warn #{message.inspect}
+          config.#{option}
+        end
+      RUBY
+    end
+
+    def self.deprecated_config_writer(option, message = nil)
+      message ||= "Setting #{option} directly on ActionController::Base is deprecated. " \
+                  "Please set it on config.action_controller.#{option}"
+
+      ClassMethods.class_eval <<-RUBY, __FILE__, __LINE__ + 1
+        def #{option}=(val)
+          ActiveSupport::Deprecation.warn #{message.inspect}
+          config.#{option} = val
+        end
+      RUBY
+    end
+
+    deprecated_config_writer :session_store
+    deprecated_config_writer :session_options
+    deprecated_config_accessor :relative_url_root, "relative_url_root is ineffective. Please stop using it"
+
     # For old tests
     def initialize_template_class(*) end
     def assign_shortcuts(*) end
diff --git a/actionpack/lib/action_controller/metal/session_management.rb b/actionpack/lib/action_controller/metal/session_management.rb
index 264250db1a..09ef9261a4 100644
--- a/actionpack/lib/action_controller/metal/session_management.rb
+++ b/actionpack/lib/action_controller/metal/session_management.rb
@@ -8,22 +8,6 @@ module ActionController #:nodoc:
     end
 
     module ClassMethods
-      # Set the session store to be used for keeping the session data between requests.
-      # By default, sessions are stored in browser cookies (<tt>:cookie_store</tt>),
-      # but you can also specify one of the other included stores (<tt>:active_record_store</tt>,
-      # <tt>:mem_cache_store</tt>, or your own custom class.
-      def session_store=(store)
-        ActiveSupport::Deprecation.warn "Setting session_store directly on ActionController::Base is deprecated. " \
-                                        "Please set it on config.action_controller.session_store"
-        config.session_store = store
-      end
-
-      def session_options=(opts)
-        ActiveSupport::Deprecation.warn "Setting seession_options directly on ActionController::Base is deprecated. " \
-                                        "Please set it on config.action_controller.session_options"
-        config.session_store = opts
-      end
-
       def session_options
         config.session_options
       end
diff --git a/actionpack/lib/action_controller/metal/url_for.rb b/actionpack/lib/action_controller/metal/url_for.rb
index 8a06f34d23..c890dc51d4 100644
--- a/actionpack/lib/action_controller/metal/url_for.rb
+++ b/actionpack/lib/action_controller/metal/url_for.rb
@@ -9,6 +9,10 @@ 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
+        :relative_url_root => config.relative_url_root,
         :_path_segments => request.symbolized_path_parameters
       )
     end
diff --git a/actionpack/lib/action_controller/url_rewriter.rb b/actionpack/lib/action_controller/url_rewriter.rb
index 807b21cd0e..973a6facd7 100644
--- a/actionpack/lib/action_controller/url_rewriter.rb
+++ b/actionpack/lib/action_controller/url_rewriter.rb
@@ -32,6 +32,7 @@ module ActionController
 
       # ROUTES TODO: Fix the tests
       segments = options.delete(:_path_segments)
+      relative_url_root = options.delete(:relative_url_root).to_s
       path_segments = path_segments ? path_segments.merge(segments || {}) : segments
 
       unless options[:only_path]
@@ -49,7 +50,8 @@ module ActionController
       path_options = yield(path_options) if block_given?
       path = router.generate(path_options, path_segments || {})
 
-      rewritten_url << ActionController::Base.relative_url_root.to_s unless options[:skip_relative_url_root]
+      # ROUTES TODO: This can be called directly, so relative_url_root should probably be set in the router
+      rewritten_url << 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]
 
diff --git a/actionpack/lib/action_view/helpers/asset_tag_helper.rb b/actionpack/lib/action_view/helpers/asset_tag_helper.rb
index 96976ce45f..5f76ff456e 100644
--- a/actionpack/lib/action_view/helpers/asset_tag_helper.rb
+++ b/actionpack/lib/action_view/helpers/asset_tag_helper.rb
@@ -648,8 +648,8 @@ module ActionView
             source = rewrite_asset_path(source)
 
             if has_request && include_host
-              unless source =~ %r{^#{ActionController::Base.relative_url_root}/}
-                source = "#{ActionController::Base.relative_url_root}#{source}"
+              unless source =~ %r{^#{controller.config.relative_url_root}/}
+                source = "#{controller.config.relative_url_root}#{source}"
               end
             end
           end
diff --git a/actionpack/test/controller/url_for_test.rb b/actionpack/test/controller/url_for_test.rb
index a7b77edc6e..07809aa480 100644
--- a/actionpack/test/controller/url_for_test.rb
+++ b/actionpack/test/controller/url_for_test.rb
@@ -113,15 +113,13 @@ module AbstractController
       end
 
       def test_relative_url_root_is_respected
-        orig_relative_url_root = ActionController::Base.relative_url_root
-        ActionController::Base.relative_url_root = '/subdir'
+        # ROUTES TODO: Tests should not have to pass :relative_url_root directly. This
+        # should probably come from the router.
 
         add_host!
         assert_equal('https://www.basecamphq.com/subdir/c/a/i',
-          W.new.url_for(:controller => 'c', :action => 'a', :id => 'i', :protocol => 'https')
+          W.new.url_for(:controller => 'c', :action => 'a', :id => 'i', :protocol => 'https', :relative_url_root => '/subdir')
         )
-      ensure
-        ActionController::Base.relative_url_root = orig_relative_url_root
       end
 
       def test_named_routes
@@ -146,9 +144,6 @@ module AbstractController
       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
@@ -158,10 +153,8 @@ module AbstractController
           controller = kls.new
 
           assert_equal 'http://www.basecamphq.com/subdir/home/sweet/home/again',
-            controller.send(:home_url, :host => 'www.basecamphq.com', :user => 'again')
+            controller.send(:home_url, :host => 'www.basecamphq.com', :user => 'again', :relative_url_root => "/subdir")
         end
-      ensure
-        ActionController::Base.relative_url_root = orig_relative_url_root
       end
 
       def test_only_path
diff --git a/actionpack/test/dispatch/request_test.rb b/actionpack/test/dispatch/request_test.rb
index cc6acead6e..703f03fa96 100644
--- a/actionpack/test/dispatch/request_test.rb
+++ b/actionpack/test/dispatch/request_test.rb
@@ -1,14 +1,6 @@
 require 'abstract_unit'
 
 class RequestTest < ActiveSupport::TestCase
-  def setup
-    ActionController::Base.relative_url_root = nil
-  end
-
-  def teardown
-    ActionController::Base.relative_url_root = nil
-  end
-
   test "remote ip" do
     request = stub_request 'REMOTE_ADDR' => '1.2.3.4'
     assert_equal '1.2.3.4', request.remote_ip
diff --git a/actionpack/test/template/asset_tag_helper_test.rb b/actionpack/test/template/asset_tag_helper_test.rb
index 586de66714..ccc39e9af6 100644
--- a/actionpack/test/template/asset_tag_helper_test.rb
+++ b/actionpack/test/template/asset_tag_helper_test.rb
@@ -1,4 +1,13 @@
 require 'abstract_unit'
+require 'active_support/ordered_options'
+
+class FakeController
+  attr_accessor :request
+
+  def config
+    @config ||= ActiveSupport::InheritableOptions.new(ActionController::Metal.config)
+  end
+end
 
 class AssetTagHelperTest < ActionView::TestCase
   tests ActionView::Helpers::AssetTagHelper
@@ -32,8 +41,7 @@ class AssetTagHelperTest < ActionView::TestCase
       )
     end
 
-    @controller = Class.new do
-      attr_accessor :request
+    @controller = Class.new(FakeController) do
       def url_for(*args) "http://www.example.com" end
     end.new
 
@@ -372,11 +380,9 @@ class AssetTagHelperTest < ActionView::TestCase
   end
 
   def test_timebased_asset_id_with_relative_url_root
-      ActionController::Base.relative_url_root = "/collaboration/hieraki"
-      expected_time = File.stat(File.expand_path(File.dirname(__FILE__) + "/../fixtures/public/images/rails.png")).mtime.to_i.to_s
-      assert_equal %(<img alt="Rails" src="#{ActionController::Base.relative_url_root}/images/rails.png?#{expected_time}" />), image_tag("rails.png")
-    ensure
-      ActionController::Base.relative_url_root = ""
+    @controller.config.relative_url_root = "/collaboration/hieraki"
+    expected_time = File.stat(File.expand_path(File.dirname(__FILE__) + "/../fixtures/public/images/rails.png")).mtime.to_i.to_s
+    assert_equal %(<img alt="Rails" src="#{@controller.config.relative_url_root}/images/rails.png?#{expected_time}" />), image_tag("rails.png")
   end
 
   def test_should_skip_asset_id_on_complete_url
@@ -606,7 +612,7 @@ class AssetTagHelperTest < ActionView::TestCase
 
   def test_caching_javascript_include_tag_with_relative_url_root
     ENV["RAILS_ASSET_ID"] = ""
-    ActionController::Base.relative_url_root = "/collaboration/hieraki"
+    @controller.config.relative_url_root = "/collaboration/hieraki"
     ActionController::Base.perform_caching = true
 
     assert_dom_equal(
@@ -624,7 +630,6 @@ class AssetTagHelperTest < ActionView::TestCase
     assert File.exist?(File.join(ActionView::Helpers::AssetTagHelper::JAVASCRIPTS_DIR, 'money.js'))
 
   ensure
-    ActionController::Base.relative_url_root = nil
     FileUtils.rm_f(File.join(ActionView::Helpers::AssetTagHelper::JAVASCRIPTS_DIR, 'all.js'))
     FileUtils.rm_f(File.join(ActionView::Helpers::AssetTagHelper::JAVASCRIPTS_DIR, 'money.js'))
   end
@@ -821,7 +826,7 @@ class AssetTagHelperTest < ActionView::TestCase
 
   def test_caching_stylesheet_link_tag_with_relative_url_root
     ENV["RAILS_ASSET_ID"] = ""
-    ActionController::Base.relative_url_root = "/collaboration/hieraki"
+    @controller.config.relative_url_root = "/collaboration/hieraki"
     ActionController::Base.perform_caching = true
 
     assert_dom_equal(
@@ -841,7 +846,6 @@ class AssetTagHelperTest < ActionView::TestCase
 
     assert File.exist?(File.join(ActionView::Helpers::AssetTagHelper::STYLESHEETS_DIR, 'money.css'))
   ensure
-    ActionController::Base.relative_url_root = nil
     FileUtils.rm_f(File.join(ActionView::Helpers::AssetTagHelper::STYLESHEETS_DIR, 'all.css'))
     FileUtils.rm_f(File.join(ActionView::Helpers::AssetTagHelper::STYLESHEETS_DIR, 'money.css'))
   end
@@ -884,16 +888,14 @@ class AssetTagHelperNonVhostTest < ActionView::TestCase
 
   def setup
     super
-    ActionController::Base.relative_url_root = "/collaboration/hieraki"
-
-    @controller = Class.new do
-      attr_accessor :request
-
+    @controller = Class.new(FakeController) do
       def url_for(options)
         "http://www.example.com/collaboration/hieraki"
       end
     end.new
 
+    @controller.config.relative_url_root = "/collaboration/hieraki"
+
     @request = Class.new do
       def protocol
         'gopher://'
@@ -905,10 +907,6 @@ class AssetTagHelperNonVhostTest < ActionView::TestCase
     ActionView::Helpers::AssetTagHelper::reset_javascript_include_default
   end
 
-  def teardown
-    ActionController::Base.relative_url_root = nil
-  end
-
   def test_should_compute_proper_path
     assert_dom_equal(%(<link href="http://www.example.com/collaboration/hieraki" rel="alternate" title="RSS" type="application/rss+xml" />), auto_discovery_link_tag)
     assert_dom_equal(%(/collaboration/hieraki/javascripts/xmlhr.js), javascript_path("xmlhr"))