Merge pull request #4498 from carlosantoniodasilva/action-controller-refactor-3-2

Action controller refactor - deprecate Compatibility module

11 files changed, 87 insertions, 46 deletions

diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md
index 8e870448fd..81bf0c7dc4 100644
--- a/actionpack/CHANGELOG.md
+++ b/actionpack/CHANGELOG.md
@@ -1,5 +1,20 @@
 ## Rails 3.2.0 (unreleased) ##
 
+*   Add `config.action_dispatch.default_charset` to configure default charset for ActionDispatch::Response. *Carlos Antonio da Silva*
+
+*   Deprecate setting default charset at controller level, use the new `config.action_dispatch.default_charset` instead. *Carlos Antonio da Silva*
+
+*   Deprecate ActionController::UnknownAction in favour of AbstractController::ActionNotFound. *Carlos Antonio da Silva*
+
+*   Deprecate ActionController::DoubleRenderError in favour of AbstractController::DoubleRenderError. *Carlos Antonio da Silva*
+
+*   Deprecate method_missing handling for not found actions, use action_missing instead. *Carlos Antonio da Silva*
+
+*   Deprecate ActionController#performed?, check for response_body presence instead. *Carlos Antonio da Silva*
+
+*   Deprecate ActionController#rescue_action, ActionController#initialize_template_class, and ActionController#assign_shortcuts.
+    These methods were not being used internally anymore and are going to be removed in Rails 4. *Carlos Antonio da Silva*
+
 *   Add config.assets.logger to configure Sprockets logger *Rafael França*
 
 *   Use a BodyProxy instead of including a Module that responds to
@@ -26,7 +41,7 @@
           <%= f.button %>
         <% end %>
 
-*   Date helpers accept a new option, `:use_two_digit_numbers = true`, that renders select boxes for months and days with a leading zero without changing the respective values. 
+*   Date helpers accept a new option, `:use_two_digit_numbers = true`, that renders select boxes for months and days with a leading zero without changing the respective values.
     For example, this is useful for displaying ISO8601-style dates such as '2011-08-01'. *Lennart Fridén and Kim Persson*
 
 *   Make ActiveSupport::Benchmarkable a default module for ActionController::Base, so the #benchmark method is once again available in the controller context like it used to be *DHH*
diff --git a/actionpack/lib/action_controller/metal/compatibility.rb b/actionpack/lib/action_controller/metal/compatibility.rb
index 05dca445a4..7b73f86584 100644
--- a/actionpack/lib/action_controller/metal/compatibility.rb
+++ b/actionpack/lib/action_controller/metal/compatibility.rb
@@ -1,21 +1,22 @@
+require 'active_support/deprecation'
+
 module ActionController
   module Compatibility
     extend ActiveSupport::Concern
 
-    class ::ActionController::ActionControllerError < StandardError #:nodoc:
-    end
-
     # Temporary hax
     included do
-      ::ActionController::UnknownAction = ::AbstractController::ActionNotFound
-      ::ActionController::DoubleRenderError = ::AbstractController::DoubleRenderError
+      ::ActionController::UnknownAction = ActiveSupport::Deprecation::DeprecatedConstantProxy.new('ActionController::UnknownAction', '::AbstractController::ActionNotFound')
+      ::ActionController::DoubleRenderError = ActiveSupport::Deprecation::DeprecatedConstantProxy.new('ActionController::DoubleRenderError', '::AbstractController::DoubleRenderError')
 
       # 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"
+      def self.default_charset=(new_charset)
+        ActiveSupport::Deprecation.warn "Setting default charset at controller level" \
+          " is deprecated, please use `config.action_dispatch.default_charset` instead", caller
+        ActionDispatch::Response.default_charset = new_charset
       end
 
       self.protected_instance_variables = %w(
@@ -24,13 +25,19 @@ module ActionController
       )
 
       def rescue_action(env)
+        ActiveSupport::Deprecation.warn "Calling `rescue_action` is deprecated and will be removed in Rails 4.0.", caller
         raise env["action_dispatch.rescue.exception"]
       end
     end
 
     # For old tests
-    def initialize_template_class(*) end
-    def assign_shortcuts(*) end
+    def initialize_template_class(*)
+      ActiveSupport::Deprecation.warn "Calling `initialize_template_class` is deprecated and has no effect anymore.", caller
+    end
+
+    def assign_shortcuts(*)
+      ActiveSupport::Deprecation.warn "Calling `assign_shortcuts` is deprecated and has no effect anymore.", caller
+    end
 
     def _normalize_options(options)
       options[:text] = nil if options.delete(:nothing) == true
@@ -44,6 +51,9 @@ module ActionController
     end
 
     def _handle_method_missing
+      ActiveSupport::Deprecation.warn "Using `method_missing` to handle non" \
+        " existing actions is deprecated and will be removed in Rails 4.0, " \
+        " please use `action_missing` instead.", caller
       method_missing(@_action_name.to_sym)
     end
 
@@ -52,6 +62,8 @@ module ActionController
     end
 
     def performed?
+      ActiveSupport::Deprecation.warn "Calling `performed?` is deprecated and will " \
+        "be removed in Rails 4.0. Please check for `response_body` presence instead.", caller
       response_body
     end
   end
diff --git a/actionpack/lib/action_dispatch/railtie.rb b/actionpack/lib/action_dispatch/railtie.rb
index a4f4825f92..2bfeeb348f 100644
--- a/actionpack/lib/action_dispatch/railtie.rb
+++ b/actionpack/lib/action_dispatch/railtie.rb
@@ -11,6 +11,7 @@ module ActionDispatch
     config.action_dispatch.ignore_accept_header = false
     config.action_dispatch.rescue_templates = { }
     config.action_dispatch.rescue_responses = { }
+    config.action_dispatch.default_charset = nil
 
     config.action_dispatch.rack_cache = {
       :metastore => "rails:/",
@@ -21,7 +22,7 @@ module ActionDispatch
     initializer "action_dispatch.configure" do |app|
       ActionDispatch::Http::URL.tld_length = app.config.action_dispatch.tld_length
       ActionDispatch::Request.ignore_accept_header = app.config.action_dispatch.ignore_accept_header
-      ActionDispatch::Response.default_charset = app.config.encoding
+      ActionDispatch::Response.default_charset = app.config.action_dispatch.default_charset || app.config.encoding
 
       ActionDispatch::ExceptionWrapper.rescue_responses.merge!(config.action_dispatch.rescue_responses)
       ActionDispatch::ExceptionWrapper.rescue_templates.merge!(config.action_dispatch.rescue_templates)
diff --git a/actionpack/test/controller/base_test.rb b/actionpack/test/controller/base_test.rb
index f2b054c849..76e5786e70 100644
--- a/actionpack/test/controller/base_test.rb
+++ b/actionpack/test/controller/base_test.rb
@@ -161,7 +161,9 @@ class PerformActionTest < ActionController::TestCase
 
   def test_get_on_priv_should_show_selector
     use_controller MethodMissingController
-    get :shouldnt_be_called
+    assert_deprecated /Using `method_missing` to handle .* use `action_missing` instead/ do
+      get :shouldnt_be_called
+    end
     assert_response :success
     assert_equal 'shouldnt_be_called', @response.body
   end
@@ -170,21 +172,25 @@ class PerformActionTest < ActionController::TestCase
     use_controller MethodMissingController
     assert !@controller.__send__(:action_method?, 'method_missing')
 
-    get :method_missing
+    assert_deprecated /Using `method_missing` to handle .* use `action_missing` instead/ do
+      get :method_missing
+    end
     assert_response :success
     assert_equal 'method_missing', @response.body
   end
 
   def test_method_missing_should_recieve_symbol
     use_controller AnotherMethodMissingController
-    get :some_action
+    assert_deprecated /Using `method_missing` to handle .* use `action_missing` instead/ do
+      get :some_action
+    end
     assert_kind_of NameError, @controller._exception
   end
 
   def test_get_on_hidden_should_fail
     use_controller NonEmptyController
-    assert_raise(ActionController::UnknownAction) { get :hidden_action }
-    assert_raise(ActionController::UnknownAction) { get :another_hidden_action }
+    assert_raise(AbstractController::ActionNotFound) { get :hidden_action }
+    assert_raise(AbstractController::ActionNotFound) { get :another_hidden_action }
   end
 end
 
diff --git a/actionpack/test/controller/caching_test.rb b/actionpack/test/controller/caching_test.rb
index 34a38a5567..443b56830a 100644
--- a/actionpack/test/controller/caching_test.rb
+++ b/actionpack/test/controller/caching_test.rb
@@ -686,8 +686,6 @@ class FragmentCachingTest < ActionController::TestCase
     @controller.params = @params
     @controller.request = @request
     @controller.response = @response
-    @controller.send(:initialize_template_class, @response)
-    @controller.send(:assign_shortcuts, @request, @response)
   end
 
   def test_fragment_cache_key
diff --git a/actionpack/test/controller/content_type_test.rb b/actionpack/test/controller/content_type_test.rb
index d51882066d..d0dabb29ca 100644
--- a/actionpack/test/controller/content_type_test.rb
+++ b/actionpack/test/controller/content_type_test.rb
@@ -70,12 +70,16 @@ class ContentTypeTest < ActionController::TestCase
   end
 
   def test_render_changed_charset_default
-    OldContentTypeController.default_charset = "utf-16"
-    get :render_defaults
-    assert_equal "utf-16", @response.charset
-    assert_equal Mime::HTML, @response.content_type
-  ensure
-    OldContentTypeController.default_charset = "utf-8"
+    assert_deprecated /Setting default charset at controller.*config\.action_dispatch\.default_charset/ do
+      begin
+        OldContentTypeController.default_charset = "utf-16"
+        get :render_defaults
+        assert_equal "utf-16", @response.charset
+        assert_equal Mime::HTML, @response.content_type
+      ensure
+        OldContentTypeController.default_charset = "utf-8"
+      end
+    end
   end
 
   # :ported:
@@ -107,12 +111,16 @@ class ContentTypeTest < ActionController::TestCase
   end
 
   def test_nil_default_for_erb
-    OldContentTypeController.default_charset = nil
-    get :render_default_for_erb
-    assert_equal Mime::HTML, @response.content_type
-    assert_nil @response.charset, @response.headers.inspect
-  ensure
-    OldContentTypeController.default_charset = "utf-8"
+    assert_deprecated /Setting default charset at controller.*config\.action_dispatch\.default_charset/ do
+      begin
+        OldContentTypeController.default_charset = nil
+        get :render_default_for_erb
+        assert_equal Mime::HTML, @response.content_type
+        assert_nil @response.charset, @response.headers.inspect
+      ensure
+        OldContentTypeController.default_charset = "utf-8"
+      end
+    end
   end
 
   def test_default_for_erb
diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb
index f42a04d670..28f6bc88e0 100644
--- a/actionpack/test/controller/render_test.rb
+++ b/actionpack/test/controller/render_test.rb
@@ -54,7 +54,7 @@ class TestController < ActionController::Base
 
   def conditional_hello_with_record
     record = Struct.new(:updated_at, :cache_key).new(Time.now.utc.beginning_of_day, "foo/123")
-    
+
     if stale?(record)
       render :action => 'hello_world'
     end
@@ -893,12 +893,12 @@ class RenderTest < ActionController::TestCase
 
   # :ported:
   def test_attempt_to_access_object_method
-    assert_raise(ActionController::UnknownAction, "No action responded to [clone]") { get :clone }
+    assert_raise(AbstractController::ActionNotFound, "No action responded to [clone]") { get :clone }
   end
 
   # :ported:
   def test_private_methods
-    assert_raise(ActionController::UnknownAction, "No action responded to [determine_layout]") { get :determine_layout }
+    assert_raise(AbstractController::ActionNotFound, "No action responded to [determine_layout]") { get :determine_layout }
   end
 
   # :ported:
@@ -1098,15 +1098,15 @@ class RenderTest < ActionController::TestCase
 
   # :ported:
   def test_double_render
-    assert_raise(ActionController::DoubleRenderError) { get :double_render }
+    assert_raise(AbstractController::DoubleRenderError) { get :double_render }
   end
 
   def test_double_redirect
-    assert_raise(ActionController::DoubleRenderError) { get :double_redirect }
+    assert_raise(AbstractController::DoubleRenderError) { get :double_redirect }
   end
 
   def test_render_and_redirect
-    assert_raise(ActionController::DoubleRenderError) { get :render_and_redirect }
+    assert_raise(AbstractController::DoubleRenderError) { get :render_and_redirect }
   end
 
   # specify the one exception to double render rule - render_to_string followed by render
diff --git a/actionpack/test/controller/view_paths_test.rb b/actionpack/test/controller/view_paths_test.rb
index f5ac886c20..04f550ae1e 100644
--- a/actionpack/test/controller/view_paths_test.rb
+++ b/actionpack/test/controller/view_paths_test.rb
@@ -22,16 +22,9 @@ class ViewLoadPathsTest < ActionController::TestCase
   end
 
   def setup
-    # TestController.view_paths = nil
-
     @request  = ActionController::TestRequest.new
     @response = ActionController::TestResponse.new
-
     @controller = TestController.new
-    # Following is needed in order to setup @controller.template object properly
-    @controller.send :assign_shortcuts, @request, @response
-    @controller.send :initialize_template_class, @response
-
     @paths = TestController.view_paths
   end
 
diff --git a/actionpack/test/dispatch/debug_exceptions_test.rb b/actionpack/test/dispatch/debug_exceptions_test.rb
index f3dc160d7d..c3a565990e 100644
--- a/actionpack/test/dispatch/debug_exceptions_test.rb
+++ b/actionpack/test/dispatch/debug_exceptions_test.rb
@@ -24,7 +24,7 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest
       when "/pass"
         [404, { "X-Cascade" => "pass" }, self]
       when "/not_found"
-        raise ActionController::UnknownAction
+        raise AbstractController::ActionNotFound
       when "/runtime_error"
         raise RuntimeError
       when "/method_not_allowed"
@@ -83,7 +83,7 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest
 
     get "/not_found", {}, {'action_dispatch.show_exceptions' => true}
     assert_response 404
-    assert_match(/#{ActionController::UnknownAction.name}/, body)
+    assert_match(/#{AbstractController::ActionNotFound.name}/, body)
 
     get "/method_not_allowed", {}, {'action_dispatch.show_exceptions' => true}
     assert_response 405
diff --git a/actionpack/test/dispatch/show_exceptions_test.rb b/actionpack/test/dispatch/show_exceptions_test.rb
index e9504f3524..4a6d5ddbf7 100644
--- a/actionpack/test/dispatch/show_exceptions_test.rb
+++ b/actionpack/test/dispatch/show_exceptions_test.rb
@@ -7,7 +7,7 @@ class ShowExceptionsTest < ActionDispatch::IntegrationTest
       req = ActionDispatch::Request.new(env)
       case req.path
       when "/not_found"
-        raise ActionController::UnknownAction
+        raise AbstractController::ActionNotFound
       when "/method_not_allowed"
         raise ActionController::MethodNotAllowed
       when "/not_found_original_exception"
diff --git a/railties/test/application/configuration_test.rb b/railties/test/application/configuration_test.rb
index cedf5fe5d5..eb16713455 100644
--- a/railties/test/application/configuration_test.rb
+++ b/railties/test/application/configuration_test.rb
@@ -286,6 +286,14 @@ module ApplicationTests
       assert_equal res, last_response.body # value should be unchanged
     end
 
+    test "sets ActionDispatch::Response.default_charset" do
+      make_basic_app do |app|
+        app.config.action_dispatch.default_charset = "utf-16"
+      end
+
+      assert_equal "utf-16", ActionDispatch::Response.default_charset
+    end
+
     test "sets all Active Record models to whitelist all attributes by default" do
       add_to_config <<-RUBY
         config.active_record.whitelist_attributes = true