Clean up the config object in ActionPack. Create config_accessor which just delegates to the config object, reducing the number of deprecations and add specific tests.

8 files changed, 93 insertions, 165 deletions

diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb
index 2e94a20d9d..092fe98588 100644
--- a/actionpack/lib/action_controller/base.rb
+++ b/actionpack/lib/action_controller/base.rb
@@ -65,8 +65,11 @@ module ActionController
       @subclasses ||= []
     end
 
+    # TODO Move this to the appropriate module
+    config_accessor :assets_dir, :asset_path, :javascripts_dir, :stylesheets_dir
+
     ActiveSupport.run_load_hooks(:action_controller, self)
   end
 end
 
-require "action_controller/deprecated/base"
+require "action_controller/deprecated/base"
\ No newline at end of file
diff --git a/actionpack/lib/action_controller/caching.rb b/actionpack/lib/action_controller/caching.rb
index 0da0ca1893..4105f9e14f 100644
--- a/actionpack/lib/action_controller/caching.rb
+++ b/actionpack/lib/action_controller/caching.rb
@@ -63,12 +63,10 @@ module ActionController #:nodoc:
     included do
       extend ConfigMethods
 
-      delegate :perform_caching, :perform_caching=, :to => :config
-      singleton_class.delegate :perform_caching, :perform_caching=, :to => :config
-      self.perform_caching = true
+      config_accessor :perform_caching
+      self.perform_caching = true if perform_caching.nil?
     end
 
-
     def caching_allowed?
       request.get? && response.status == 200
     end
diff --git a/actionpack/lib/action_controller/caching/pages.rb b/actionpack/lib/action_controller/caching/pages.rb
index 20df3a1a69..cefd1f48c0 100644
--- a/actionpack/lib/action_controller/caching/pages.rb
+++ b/actionpack/lib/action_controller/caching/pages.rb
@@ -44,8 +44,8 @@ module ActionController #:nodoc:
         # For Rails, this directory has already been set to Rails.public_path (which is usually set to <tt>Rails.root + "/public"</tt>). Changing
         # this setting can be useful to avoid naming conflicts with files in <tt>public/</tt>, but doing so will likely require configuring your
         # web server to look in the new location for cached files.
-        singleton_class.delegate :page_cache_directory, :page_cache_directory=, :to => :config
-        self.page_cache_directory = ''
+        config_accessor :page_cache_directory
+        self.page_cache_directory ||= ''
 
         ##
         # :singleton-method:
@@ -53,8 +53,8 @@ module ActionController #:nodoc:
         # order to make it easy for the cached files to be picked up properly by the web server. By default, this cache extension is <tt>.html</tt>.
         # If you want something else, like <tt>.php</tt> or <tt>.shtml</tt>, just set Base.page_cache_extension. In cases where a request already has an
         # extension, such as <tt>.xml</tt> or <tt>.rss</tt>, page caching will not add an extension. This allows it to work well with RESTful apps.
-        singleton_class.delegate :page_cache_extension, :page_cache_extension=, :to => :config
-        self.page_cache_extension = '.html'
+        config_accessor :page_cache_extension
+        self.page_cache_extension ||= '.html'
       end
 
       module ClassMethods
diff --git a/actionpack/lib/action_controller/deprecated/base.rb b/actionpack/lib/action_controller/deprecated/base.rb
index 57203ce95f..3975afcaf0 100644
--- a/actionpack/lib/action_controller/deprecated/base.rb
+++ b/actionpack/lib/action_controller/deprecated/base.rb
@@ -1,33 +1,16 @@
 module ActionController
   class Base
-    class << self
-      def deprecated_config_accessor(option, message = nil)
-        deprecated_config_reader(option, message)
-        deprecated_config_writer(option, message)
+    # Deprecated methods. Wrap them in a module so they can be overwritten by plugins
+    # (like the verify method.)
+    module DeprecatedBehavior #:nodoc:
+      def relative_url_root
+        ActiveSupport::Deprecation.warn "ActionController::Base.relative_url_root is ineffective. " <<
+          "Please stop using it.", caller
       end
 
-      def deprecated_config_reader(option, message = nil)
-        message ||= "Reading #{option} directly from ActionController::Base is deprecated. " \
-                    "Please read it from config.#{option}"
-
-        self.class_eval <<-RUBY, __FILE__, __LINE__ + 1
-          def #{option}
-            ActiveSupport::Deprecation.warn #{message.inspect}, caller
-            config.#{option}
-          end
-        RUBY
-      end
-
-      def deprecated_config_writer(option, message = nil)
-        message ||= "Setting #{option} directly on ActionController::Base is deprecated. " \
-                    "Please set it on config.action_controller.#{option}"
-
-        self.class_eval <<-RUBY, __FILE__, __LINE__ + 1
-          def #{option}=(val)
-            ActiveSupport::Deprecation.warn #{message.inspect}, caller
-            config.#{option} = val
-          end
-        RUBY
+      def relative_url_root=
+        ActiveSupport::Deprecation.warn "ActionController::Base.relative_url_root= is ineffective. " <<
+          "Please stop using it.", caller
       end
 
       def consider_all_requests_local
@@ -125,9 +108,7 @@ module ActionController
       def use_accept_header=(val)
         use_accept_header
       end
-    end
 
-    module DeprecatedBehavior
       # This method has been moved to ActionDispatch::Request.filter_parameters
       def filter_parameter_logging(*args, &block)
         ActiveSupport::Deprecation.warn("Setting filter_parameter_logging in ActionController is deprecated and has no longer effect, please set 'config.filter_parameters' in config/application.rb instead", caller)
@@ -146,17 +127,6 @@ module ActionController
 
     extend DeprecatedBehavior
 
-    deprecated_config_writer :session_options
-    deprecated_config_writer :session_store
-
-    deprecated_config_accessor :assets_dir
-    deprecated_config_accessor :asset_path
-    deprecated_config_accessor :helpers_path
-    deprecated_config_accessor :javascripts_dir
-    deprecated_config_accessor :page_cache_directory
-    deprecated_config_accessor :relative_url_root, "relative_url_root is ineffective. Please stop using it"
-    deprecated_config_accessor :stylesheets_dir
-
     delegate :consider_all_requests_local, :consider_all_requests_local=,
              :allow_concurrency, :allow_concurrency=, :to => :"self.class"
   end
diff --git a/actionpack/lib/action_controller/metal/compatibility.rb b/actionpack/lib/action_controller/metal/compatibility.rb
index 02722360f1..d49465fa0b 100644
--- a/actionpack/lib/action_controller/metal/compatibility.rb
+++ b/actionpack/lib/action_controller/metal/compatibility.rb
@@ -21,8 +21,8 @@ module ActionController
         delegate :default_charset=, :to => "ActionDispatch::Response"
       end
 
-      # cattr_reader :protected_instance_variables
-      cattr_accessor :protected_instance_variables
+      # TODO: Update protected instance variables list
+      config_accessor :protected_instance_variables
       self.protected_instance_variables = %w(@assigns @performed_redirect @performed_render
                                              @variables_added @request_origin @url
                                              @parent_controller @action_name
diff --git a/actionpack/lib/action_controller/metal/helpers.rb b/actionpack/lib/action_controller/metal/helpers.rb
index 82bedc3fad..1110d7c117 100644
--- a/actionpack/lib/action_controller/metal/helpers.rb
+++ b/actionpack/lib/action_controller/metal/helpers.rb
@@ -52,8 +52,8 @@ module ActionController
     include AbstractController::Helpers
 
     included do
-      class_attribute :helpers_path
-      self.helpers_path = []
+      config_accessor :helpers_path
+      self.helpers_path ||= []
     end
 
     module ClassMethods
diff --git a/actionpack/lib/action_controller/metal/request_forgery_protection.rb b/actionpack/lib/action_controller/metal/request_forgery_protection.rb
index 39a809657b..2ba0d6e5cd 100644
--- a/actionpack/lib/action_controller/metal/request_forgery_protection.rb
+++ b/actionpack/lib/action_controller/metal/request_forgery_protection.rb
@@ -4,6 +4,45 @@ module ActionController #:nodoc:
   class InvalidAuthenticityToken < ActionControllerError #:nodoc:
   end
 
+  # Protecting controller actions from CSRF attacks by ensuring that all forms are coming from the current
+  # web application, not a forged link from another site, is done by embedding a token based on a random
+  # string stored in the session (which an attacker wouldn't know) in all forms and Ajax requests generated
+  # by Rails and then verifying the authenticity of that token in the controller.  Only HTML/JavaScript
+  # requests are checked, so this will not protect your XML API (presumably you'll have a different
+  # authentication scheme there anyway).  Also, GET requests are not protected as these should be
+  # idempotent anyway.
+  #
+  # This is turned on with the <tt>protect_from_forgery</tt> method, which will check the token and raise an
+  # ActionController::InvalidAuthenticityToken if it doesn't match what was expected. You can customize the
+  # error message in production by editing public/422.html.  A call to this method in ApplicationController is
+  # generated by default in post-Rails 2.0 applications.
+  #
+  # The token parameter is named <tt>authenticity_token</tt> by default. If you are generating an HTML form
+  # manually (without the use of Rails' <tt>form_for</tt>, <tt>form_tag</tt> or other helpers), you have to
+  # include a hidden field named like that and set its value to what is returned by
+  # <tt>form_authenticity_token</tt>.
+  #
+  # Request forgery protection is disabled by default in test environment. If you are upgrading from Rails
+  # 1.x, add this to config/environments/test.rb:
+  #
+  #   # Disable request forgery protection in test environment
+  #   config.action_controller.allow_forgery_protection = false
+  #
+  # == Learn more about CSRF (Cross-Site Request Forgery) attacks
+  #
+  # Here are some resources:
+  # * http://isc.sans.org/diary.html?storyid=1750
+  # * http://en.wikipedia.org/wiki/Cross-site_request_forgery
+  #
+  # Keep in mind, this is NOT a silver-bullet, plug 'n' play, warm security blanket for your rails application.
+  # There are a few guidelines you should follow:
+  #
+  # * Keep your GET requests safe and idempotent.  More reading material:
+  #   * http://www.xml.com/pub/a/2002/04/24/deviant.html
+  #   * http://www.w3.org/Protocols/rfc2616/rfc2616-sec9.html#sec9.1.1
+  #   * Make sure the session cookies that Rails creates are non-persistent.  Check in Firefox and look
+  #     for "Expires: at end of session"
+  #
   module RequestForgeryProtection
     extend ActiveSupport::Concern
 
@@ -12,54 +51,17 @@ module ActionController #:nodoc:
     included do
       # Sets the token parameter name for RequestForgery. Calling +protect_from_forgery+
       # sets it to <tt>:authenticity_token</tt> by default.
-      config.request_forgery_protection_token ||= :authenticity_token
+      config_accessor :request_forgery_protection_token
+      self.request_forgery_protection_token ||= :authenticity_token
 
       # Controls whether request forgergy protection is turned on or not. Turned off by default only in test mode.
-      config.allow_forgery_protection ||= true
+      config_accessor :allow_forgery_protection
+      self.allow_forgery_protection = true if allow_forgery_protection.nil?
 
       helper_method :form_authenticity_token
       helper_method :protect_against_forgery?
     end
 
-    # Protecting controller actions from CSRF attacks by ensuring that all forms are coming from the current
-    # web application, not a forged link from another site, is done by embedding a token based on a random
-    # string stored in the session (which an attacker wouldn't know) in all forms and Ajax requests generated
-    # by Rails and then verifying the authenticity of that token in the controller.  Only HTML/JavaScript
-    # requests are checked, so this will not protect your XML API (presumably you'll have a different
-    # authentication scheme there anyway).  Also, GET requests are not protected as these should be
-    # idempotent anyway.
-    #
-    # This is turned on with the <tt>protect_from_forgery</tt> method, which will check the token and raise an
-    # ActionController::InvalidAuthenticityToken if it doesn't match what was expected. You can customize the
-    # error message in production by editing public/422.html.  A call to this method in ApplicationController is
-    # generated by default in post-Rails 2.0 applications.
-    #
-    # The token parameter is named <tt>authenticity_token</tt> by default. If you are generating an HTML form
-    # manually (without the use of Rails' <tt>form_for</tt>, <tt>form_tag</tt> or other helpers), you have to
-    # include a hidden field named like that and set its value to what is returned by
-    # <tt>form_authenticity_token</tt>.
-    #
-    # Request forgery protection is disabled by default in test environment. If you are upgrading from Rails
-    # 1.x, add this to config/environments/test.rb:
-    #
-    #   # Disable request forgery protection in test environment
-    #   config.action_controller.allow_forgery_protection = false
-    #
-    # == Learn more about CSRF (Cross-Site Request Forgery) attacks
-    #
-    # Here are some resources:
-    # * http://isc.sans.org/diary.html?storyid=1750
-    # * http://en.wikipedia.org/wiki/Cross-site_request_forgery
-    #
-    # Keep in mind, this is NOT a silver-bullet, plug 'n' play, warm security blanket for your rails application.
-    # There are a few guidelines you should follow:
-    #
-    # * Keep your GET requests safe and idempotent.  More reading material:
-    #   * http://www.xml.com/pub/a/2002/04/24/deviant.html
-    #   * http://www.w3.org/Protocols/rfc2616/rfc2616-sec9.html#sec9.1.1
-    #   * Make sure the session cookies that Rails creates are non-persistent.  Check in Firefox and look
-    #     for "Expires: at end of session"
-    #
     module ClassMethods
       # Turn on request forgery protection. Bear in mind that only non-GET, HTML/JavaScript requests are checked.
       #
@@ -79,22 +81,6 @@ module ActionController #:nodoc:
         self.request_forgery_protection_token ||= :authenticity_token
         before_filter :verify_authenticity_token, options
       end
-
-      def request_forgery_protection_token
-        config.request_forgery_protection_token
-      end
-
-      def request_forgery_protection_token=(val)
-        config.request_forgery_protection_token = val
-      end
-
-      def allow_forgery_protection
-        config.allow_forgery_protection
-      end
-
-      def allow_forgery_protection=(val)
-        config.allow_forgery_protection = val
-      end
     end
 
     protected
@@ -104,22 +90,6 @@ module ActionController #:nodoc:
         before_filter :verify_authenticity_token, options
       end
 
-      def request_forgery_protection_token
-        config.request_forgery_protection_token
-      end
-
-      def request_forgery_protection_token=(val)
-        config.request_forgery_protection_token = val
-      end
-
-      def allow_forgery_protection
-        config.allow_forgery_protection
-      end
-
-      def allow_forgery_protection=(val)
-        config.allow_forgery_protection = val
-      end
-
       # The actual before_filter that is used.  Modify this to change how you handle unverified requests.
       def verify_authenticity_token
         verified_request? || raise(ActionController::InvalidAuthenticityToken)
@@ -146,7 +116,7 @@ module ActionController #:nodoc:
       end
 
       def protect_against_forgery?
-        config.allow_forgery_protection
+        allow_forgery_protection
       end
   end
 end
diff --git a/actionpack/lib/action_controller/railtie.rb b/actionpack/lib/action_controller/railtie.rb
index b029434004..0e3cdffadc 100644
--- a/actionpack/lib/action_controller/railtie.rb
+++ b/actionpack/lib/action_controller/railtie.rb
@@ -13,64 +13,51 @@ module ActionController
   class Railtie < Rails::Railtie
     config.action_controller = ActiveSupport::OrderedOptions.new
 
-    ad = config.action_dispatch
-    config.action_controller.singleton_class.send(:define_method, :session) do
-      ActiveSupport::Deprecation.warn "config.action_controller.session has been " \
-        "renamed to config.action_dispatch.session.", caller
-      ad.session
-    end
+    config.action_controller.singleton_class.tap do |d|
+      d.send(:define_method, :session) do
+        ActiveSupport::Deprecation.warn "config.action_controller.session has been deprecated. " <<
+          "Please use Rails.application.config.session_store instead.", caller
+      end
 
-    config.action_controller.singleton_class.send(:define_method, :session=) do |val|
-      ActiveSupport::Deprecation.warn "config.action_controller.session has been " \
-        "renamed to config.action_dispatch.session.", caller
-      ad.session = val
-    end
+      d.send(:define_method, :session=) do |val|
+        ActiveSupport::Deprecation.warn "config.action_controller.session= has been deprecated. " <<
+          "Please use config.session_store(name, options) instead.", caller
+      end
 
-    config.action_controller.singleton_class.send(:define_method, :session_store) do
-      ActiveSupport::Deprecation.warn "config.action_controller.session_store has been " \
-        "renamed to config.action_dispatch.session_store.", caller
-      ad.session_store
-    end
+      d.send(:define_method, :session_store) do
+        ActiveSupport::Deprecation.warn "config.action_controller.session_store has been deprecated. " <<
+          "Please use Rails.application.config.session_store instead.", caller
+      end
 
-    config.action_controller.singleton_class.send(:define_method, :session_store=) do |val|
-      ActiveSupport::Deprecation.warn "config.action_controller.session_store has been " \
-        "renamed to config.action_dispatch.session_store.", caller
-      ad.session_store = val
+      d.send(:define_method, :session_store=) do |val|
+        ActiveSupport::Deprecation.warn "config.action_controller.session_store= has been deprecated. " <<
+          "Please use config.session_store(name, options) instead.", caller
+      end
     end
 
     log_subscriber :action_controller, ActionController::Railties::LogSubscriber.new
 
-    initializer "action_controller.logger" do
-      ActiveSupport.on_load(:action_controller) { self.logger ||= Rails.logger }
-    end
-
-    initializer "action_controller.page_cache_directory" do
-      ActiveSupport.on_load(:action_controller) do
-        self.page_cache_directory = Rails.public_path
-      end
-    end
-
     initializer "action_controller.set_configs" do |app|
       paths = app.config.paths
       ac = app.config.action_controller
 
-      ac.assets_dir      = paths.public.to_a.first
-      ac.javascripts_dir = paths.public.javascripts.to_a.first
-      ac.stylesheets_dir = paths.public.stylesheets.to_a.first
+      ac.assets_dir           ||= paths.public.to_a.first
+      ac.javascripts_dir      ||= paths.public.javascripts.to_a.first
+      ac.stylesheets_dir      ||= paths.public.stylesheets.to_a.first
+      ac.page_cache_directory ||= paths.public.to_a.first
+      ac.helpers_path         ||= paths.app.helpers.to_a
 
       ActiveSupport.on_load(:action_controller) do
         self.config.merge!(ac)
       end
     end
 
-    initializer "action_controller.initialize_framework_caches" do
-      ActiveSupport.on_load(:action_controller) { self.cache_store ||= RAILS_CACHE }
+    initializer "action_controller.logger" do
+      ActiveSupport.on_load(:action_controller) { self.logger ||= Rails.logger }
     end
 
-    initializer "action_controller.set_helpers_path" do |app|
-      ActiveSupport.on_load(:action_controller) do
-        self.helpers_path = app.config.paths.app.helpers.to_a
-      end
+    initializer "action_controller.initialize_framework_caches" do
+      ActiveSupport.on_load(:action_controller) { self.cache_store ||= RAILS_CACHE }
     end
 
     initializer "action_controller.url_helpers" do |app|