Deprecation: @request will be removed after 1.2. Use the request method instead.

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@5201 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

12 files changed, 105 insertions, 112 deletions

diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG
index 72e1d79eff..f5bafbdd74 100644
--- a/actionpack/CHANGELOG
+++ b/actionpack/CHANGELOG
@@ -1,5 +1,7 @@
 *SVN*
 
+* Deprecation: @request will be removed after 1.2. Use the request method instead.  [Jeremy Kemper]
+
 * Make the :status parameter expand to the default message for that status code if it is an integer. Also support symbol statuses. [Jamis Buck]. Examples:
 
     head :status => 404        # expands to "404 Not Found"
diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb
index 94e2d6a25f..743c97863a 100755
--- a/actionpack/lib/action_controller/base.rb
+++ b/actionpack/lib/action_controller/base.rb
@@ -294,7 +294,7 @@ module ActionController #:nodoc:
 
     # Holds the request object that's primarily used to get environment variables through access like
     # <tt>request.env["REQUEST_URI"]</tt>.
-    attr_accessor :request
+    attr_internal :request
 
     # Holds a hash of all the GET, POST, and Url parameters passed to the action. Accessed like <tt>params["post_id"]</tt>
     # to get the post_id. No type casts are made, so all values are returned as strings.
@@ -1014,7 +1014,7 @@ module ActionController #:nodoc:
       end
 
       def assign_shortcuts(request, response)
-        @request, @_params, @cookies = request, request.parameters, request.cookies
+        @_request, @_params, @cookies = request, request.parameters, request.cookies
 
         @response         = response
         @response.session = request.session
@@ -1030,7 +1030,7 @@ module ActionController #:nodoc:
 
 
       # TODO: assigns cookies headers params request response template
-      DEPRECATED_INSTANCE_VARIABLES = %w(flash params session)
+      DEPRECATED_INSTANCE_VARIABLES = %w(flash params request session)
 
       # Gone after 1.2.
       def assign_deprecated_shortcuts(request, response)
@@ -1128,7 +1128,7 @@ module ActionController #:nodoc:
           %w(@assigns @performed_redirect @performed_render)
         else
           %w(@assigns @performed_redirect @performed_render
-             @request @response @_params @_session @session
+             @_request @request @response @_params @params @_session @session
              @cookies @template @request_origin @parent_controller)
         end
       end
diff --git a/actionpack/lib/action_controller/components.rb b/actionpack/lib/action_controller/components.rb
index c05b1eb068..0515461910 100644
--- a/actionpack/lib/action_controller/components.rb
+++ b/actionpack/lib/action_controller/components.rb
@@ -139,22 +139,22 @@ module ActionController #:nodoc:
             self.class
           end
         end
-        
+
         # Create a new request object based on the current request.
         # The new request inherits the session from the current request,
         # bypassing any session options set for the component controller's class
         def request_for_component(controller_name, options)
-          request         = @request.dup
-          request.session = @request.session
-        
-          request.instance_variable_set(
+          new_request         = request.dup
+          new_request.session = request.session
+
+          new_request.instance_variable_set(
             :@parameters,
             (options[:params] || {}).with_indifferent_access.update(
               "controller" => controller_name, "action" => options[:action], "id" => options[:id]
             )
           )
-          
-          request
+
+          new_request
         end
 
         def component_logging(options)
diff --git a/actionpack/lib/action_controller/templates/rescues/diagnostics.rhtml b/actionpack/lib/action_controller/templates/rescues/diagnostics.rhtml
index fa48b62f6f..e9faacef09 100644
--- a/actionpack/lib/action_controller/templates/rescues/diagnostics.rhtml
+++ b/actionpack/lib/action_controller/templates/rescues/diagnostics.rhtml
@@ -1,7 +1,7 @@
 <h1>
   <%=h @exception.class.to_s %>
-  <% if @request.parameters['controller'] %>
-    in <%=h @request.parameters['controller'].humanize %>Controller<% if @request.parameters['action'] %>#<%=h @request.parameters['action'] %><% end %>
+  <% if request.parameters['controller'] %>
+    in <%=h request.parameters['controller'].humanize %>Controller<% if request.parameters['action'] %>#<%=h request.parameters['action'] %><% end %>
   <% end %>
 </h1>
 <pre><%=h @exception.clean_message %></pre>
diff --git a/actionpack/lib/action_controller/verification.rb b/actionpack/lib/action_controller/verification.rb
index d9b10cebca..7cdf658437 100644
--- a/actionpack/lib/action_controller/verification.rb
+++ b/actionpack/lib/action_controller/verification.rb
@@ -85,7 +85,7 @@ module ActionController #:nodoc:
       
       if !prereqs_invalid && options[:method]
         prereqs_invalid ||= 
-          [*options[:method]].all? { |v| @request.method != v.to_sym }
+          [*options[:method]].all? { |v| request.method != v.to_sym }
       end
       
       prereqs_invalid ||= (request.xhr? != options[:xhr]) unless options[:xhr].nil?
diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb
index 5d1bf57c94..53811d8e9d 100644
--- a/actionpack/lib/action_view/base.rb
+++ b/actionpack/lib/action_view/base.rb
@@ -147,8 +147,8 @@ module ActionView #:nodoc:
     attr_accessor :base_path, :assigns, :template_extension
     attr_accessor :controller
 
-    attr_reader :logger, :request, :response, :headers
-    attr_internal :flash, :params, :session
+    attr_reader :logger, :response, :headers
+    attr_internal *ActionController::Base::DEPRECATED_INSTANCE_VARIABLES
 
     # Specify trim mode for the ERB compiler. Defaults to '-'.
     # See ERB documentation for suitable values.
@@ -440,11 +440,11 @@ module ActionView #:nodoc:
         if template_requires_setup?(extension)
           body = case extension.to_sym
             when :rxml
-              "@controller.response.content_type ||= 'application/xml'\n" +
+              "controller.response.content_type ||= 'application/xml'\n" +
               "xml = Builder::XmlMarkup.new(:indent => 2)\n" +
               template
             when :rjs
-              "@controller.response.content_type ||= 'text/javascript'\n" +
+              "controller.response.content_type ||= 'text/javascript'\n" +
               "update_page do |page|\n#{template}\nend"
           end
         else
@@ -525,4 +525,4 @@ module ActionView #:nodoc:
   end
 end
 
-require 'action_view/template_error'
\ No newline at end of file
+require 'action_view/template_error'
diff --git a/actionpack/test/controller/action_pack_assertions_test.rb b/actionpack/test/controller/action_pack_assertions_test.rb
index 6e1ccba50e..1b4e0b5aca 100644
--- a/actionpack/test/controller/action_pack_assertions_test.rb
+++ b/actionpack/test/controller/action_pack_assertions_test.rb
@@ -11,7 +11,7 @@ class ActionPackAssertionsController < ActionController::Base
 
   # a standard template
   def hello_xml_world() render "test/hello_xml_world"; end
- 
+
   # a redirect to an internal location
   def redirect_internal() redirect_to "/nothing"; end
 
@@ -22,10 +22,10 @@ class ActionPackAssertionsController < ActionController::Base
   def redirect_to_path() redirect_to '/some/path' end
 
   def redirect_to_named_route() redirect_to route_one_url end
-  
+
   # a redirect to an external location
   def redirect_external() redirect_to_url "http://www.rubyonrails.org"; end
-  
+
   # a 404
   def response404() render_text "", "404 AWOL"; end
 
@@ -70,52 +70,52 @@ class ActionPackAssertionsController < ActionController::Base
     session['xmas'] = 'turkey'
     render_text "ho ho ho"
   end
-  
+
   # raises exception on get requests
   def raise_on_get
-    raise "get" if @request.get?
-    render_text "request method: #{@request.env['REQUEST_METHOD']}"
+    raise "get" if request.get?
+    render_text "request method: #{request.env['REQUEST_METHOD']}"
   end
 
   # raises exception on post requests
   def raise_on_post
-    raise "post" if @request.post?
-    render_text "request method: #{@request.env['REQUEST_METHOD']}"
-  end       
-  
+    raise "post" if request.post?
+    render_text "request method: #{request.env['REQUEST_METHOD']}"
+  end
+
   def get_valid_record
-    @record = Class.new do       
+    @record = Class.new do
       def valid?
         true
       end
 
       def errors
-        Class.new do 
-           def full_messages; []; end          
+        Class.new do
+           def full_messages; []; end
         end.new
-      end    
-    
+      end
+
     end.new
-        
-    render :nothing => true    
+
+    render :nothing => true
   end
 
 
   def get_invalid_record
-    @record = Class.new do 
-      
+    @record = Class.new do
+
       def valid?
         false
       end
-      
+
       def errors
-        Class.new do 
-           def full_messages; ['...stuff...']; end          
+        Class.new do
+           def full_messages; ['...stuff...']; end
         end.new
       end
-    end.new                
-    
-    render :nothing => true    
+    end.new
+
+    render :nothing => true
   end
 
   # 911
@@ -145,20 +145,20 @@ end
 # ---------------------------------------------------------------------------
 
 
-# tell the controller where to find its templates but start from parent 
-# directory of test_request_response to simulate the behaviour of a 
+# tell the controller where to find its templates but start from parent
+# directory of test_request_response to simulate the behaviour of a
 # production environment
 ActionPackAssertionsController.template_root = File.dirname(__FILE__) + "/../fixtures/"
 
 
 # a test case to exercise the new capabilities TestRequest & TestResponse
 class ActionPackAssertionsControllerTest < Test::Unit::TestCase
-  # let's get this party started  
+  # let's get this party started
   def setup
     @controller = ActionPackAssertionsController.new
     @request, @response = ActionController::TestRequest.new, ActionController::TestResponse.new
   end
- 
+
   # -- assertion-based testing ------------------------------------------------
 
   def test_assert_tag_and_url_for
@@ -172,7 +172,7 @@ class ActionPackAssertionsControllerTest < Test::Unit::TestCase
     assert_deprecated_assertion { assert_session_has 'xmas' }
     assert_deprecated_assertion { assert_session_has_no 'halloween' }
   end
-  
+
   # test the get method, make sure the request really was a get
   def test_get
     assert_raise(RuntimeError) { get :raise_on_get }
@@ -186,7 +186,7 @@ class ActionPackAssertionsControllerTest < Test::Unit::TestCase
     post :raise_on_get
     assert_equal @response.body, 'request method: POST'
   end
-  
+
 #   the following test fails because the request_method is now cached on the request instance
 #   test the get/post switch within one test action
 #   def test_get_post_switch
@@ -212,7 +212,7 @@ class ActionPackAssertionsControllerTest < Test::Unit::TestCase
     assert_deprecated_assertion { assert_template_has_no 'maple syrup' }
     assert_deprecated_assertion { assert_template_has_no 'howdy' }
   end
-  
+
   # test the redirection assertions
   def test_assert_redirect
     process :redirect_internal
@@ -234,7 +234,7 @@ class ActionPackAssertionsControllerTest < Test::Unit::TestCase
       assert_redirect_url_match 'rails.org'
     end
   end
-  
+
   # test the redirection pattern matching on a pattern
   def test_assert_redirect_url_match_pattern
     process :redirect_external
@@ -312,22 +312,22 @@ class ActionPackAssertionsControllerTest < Test::Unit::TestCase
     assert_deprecated_assertion { assert_flash_has_no 'hello' }
     assert_deprecated_assertion { assert_flash_has_no 'qwerty' }
   end
-  
-  # test the assert_rendered_file 
+
+  # test the assert_rendered_file
   def test_assert_rendered_file
     assert_deprecated(/render/) { process :hello_world }
     assert_deprecated_assertion { assert_rendered_file 'test/hello_world' }
     assert_deprecated_assertion { assert_rendered_file 'hello_world' }
   end
-  
+
   # test the assert_success assertion
   def test_assert_success
     process :nothing
     assert_deprecated_assertion { assert_success }
   end
-  
+
   # -- standard request/response object testing --------------------------------
- 
+
   # ensure our session is working properly
   def test_session_objects
     process :session_stuffing
@@ -335,20 +335,20 @@ class ActionPackAssertionsControllerTest < Test::Unit::TestCase
     assert_deprecated_assertion { assert_session_equal 'turkey', 'xmas' }
     assert !@response.has_session_object?('easter')
   end
-  
+
   # make sure that the template objects exist
   def test_template_objects_alive
     process :assign_this
     assert !@response.has_template_object?('hi')
     assert @response.has_template_object?('howdy')
   end
-  
+
   # make sure we don't have template objects when we shouldn't
   def test_template_object_missing
     process :nothing
     assert_nil @response.template_objects['howdy']
   end
-  
+
   def test_assigned_equal
     process :assign_this
     assert_deprecated_assertion { assert_assigned_equal "ho", :howdy }
@@ -376,7 +376,7 @@ class ActionPackAssertionsControllerTest < Test::Unit::TestCase
     assert !@response.has_flash_with_contents?
     assert_nil @response.flash['hello']
   end
-  
+
   # examine that the flash objects are what we expect
   def test_flash_equals
     process :flash_me
@@ -384,8 +384,8 @@ class ActionPackAssertionsControllerTest < Test::Unit::TestCase
       assert_flash_equal 'my name is inigo montoya...', 'hello'
     end
   end
-  
-  # check if we were rendered by a file-based template? 
+
+  # check if we were rendered by a file-based template?
   def test_rendered_action
     process :nothing
     assert !@response.rendered_with_file?
@@ -394,7 +394,7 @@ class ActionPackAssertionsControllerTest < Test::Unit::TestCase
     assert @response.rendered_with_file?
     assert 'hello_world', @response.rendered_file
   end
-  
+
   # check the redirection location
   def test_redirection_location
     process :redirect_internal
@@ -406,20 +406,20 @@ class ActionPackAssertionsControllerTest < Test::Unit::TestCase
     process :nothing
     assert_nil @response.redirect_url
   end
-  
- 
-  # check server errors 
+
+
+  # check server errors
   def test_server_error_response_code
     process :response500
     assert @response.server_error?
-    
+
     process :response599
     assert @response.server_error?
-    
+
     process :response404
     assert !@response.server_error?
   end
-  
+
   # check a 404 response code
   def test_missing_response_code
     process :response404
@@ -435,7 +435,7 @@ class ActionPackAssertionsControllerTest < Test::Unit::TestCase
     assert !@response.redirect_url_match?("phpoffrails")
     assert !@response.redirect_url_match?(/perloffrails/)
   end
-  
+
   # check for a redirection
   def test_redirection
     process :redirect_internal
@@ -447,19 +447,19 @@ class ActionPackAssertionsControllerTest < Test::Unit::TestCase
     process :nothing
     assert !@response.redirect?
   end
-  
+
   # check a successful response code
   def test_successful_response_code
     process :nothing
     assert @response.success?
-  end 
-  
+  end
+
   # a basic check to make sure we have a TestResponse object
   def test_has_response
     process :nothing
     assert_kind_of ActionController::TestResponse, @response
   end
-  
+
   def test_render_based_on_parameters
     process :render_based_on_parameters, "name" => "David"
     assert_equal "Mr. David", @response.body
@@ -491,7 +491,7 @@ class ActionPackAssertionsControllerTest < Test::Unit::TestCase
   def test_follow_redirect
     process :redirect_to_action
     assert_redirected_to :action => "flash_me"
-    
+
     follow_redirect
     assert_equal 1, @request.parameters["id"].to_i
 
@@ -534,23 +534,23 @@ class ActionPackAssertionsControllerTest < Test::Unit::TestCase
     @controller = Admin::InnerModuleController.new
     get :redirect_to_absolute_controller
     assert_redirected_to :controller => 'content'
-    
+
     get :redirect_to_fellow_controller
     assert_redirected_to :controller => 'admin/user'
-  end                 
-  
+  end
+
   def test_assert_valid
     get :get_valid_record
-    assert_valid assigns('record')    
-  end                
-  
+    assert_valid assigns('record')
+  end
+
   def test_assert_valid_failing
     get :get_invalid_record
-    
+
     begin
-      assert_valid assigns('record')    
+      assert_valid assigns('record')
       assert false
-    rescue Test::Unit::AssertionFailedError => e             
+    rescue Test::Unit::AssertionFailedError => e
     end
   end
 
@@ -560,7 +560,7 @@ class ActionPackAssertionsControllerTest < Test::Unit::TestCase
     end
 end
 
-class ActionPackHeaderTest < Test::Unit::TestCase  
+class ActionPackHeaderTest < Test::Unit::TestCase
   def setup
     @controller = ActionPackAssertionsController.new
     @request, @response = ActionController::TestRequest.new, ActionController::TestResponse.new
@@ -577,7 +577,7 @@ class ActionPackHeaderTest < Test::Unit::TestCase
     assert_equal('application/pdf; charset=utf-8', @controller.headers['Content-Type'])
   end
 
-  
+
   def test_render_text_with_custom_content_type
     get :render_text_with_custom_content_type
     assert_equal 'application/rss+xml; charset=utf-8', @response.headers['Content-Type']
diff --git a/actionpack/test/controller/new_render_test.rb b/actionpack/test/controller/new_render_test.rb
index b2b40b7636..729119d9fb 100644
--- a/actionpack/test/controller/new_render_test.rb
+++ b/actionpack/test/controller/new_render_test.rb
@@ -422,18 +422,26 @@ class NewRenderTest < Test::Unit::TestCase
     ActionController::Base.protected_variables_cache = nil
 
     get :hello_world
-    assert_nil(assigns["request"])
+    assert !assigns.include?('request'), 'request should not be in assigns'
 
     ActionController::Base.view_controller_internals = true
     ActionController::Base.protected_variables_cache = nil
 
     get :hello_world
-    assert_kind_of ActionController::AbstractRequest, assigns["request"]
+    assert assigns.include?('request'), 'request should be in assigns'
+    assert_deprecated 'request' do
+      assert_kind_of ActionController::AbstractRequest, assigns['request']
+    end
+    assert_not_deprecated do
+      assert_kind_of ActionController::AbstractRequest, @response.template.request
+      assert_kind_of ActionController::AbstractRequest, assigns['_request']
+    end
 
+  ensure
     ActionController::Base.view_controller_internals = view_internals_old_value
     ActionController::Base.protected_variables_cache = nil
   end
-  
+
   def test_render_xml
     get :render_xml_hello
     assert_equal "<html>\n  <p>Hello David</p>\n<p>This is grand!</p>\n</html>\n", @response.body
diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb
index 23f90c6243..6e37a7f142 100644
--- a/actionpack/test/controller/render_test.rb
+++ b/actionpack/test/controller/render_test.rb
@@ -168,25 +168,6 @@ class RenderTest < Test::Unit::TestCase
     assert_raises(ActionController::UnknownAction, "No action responded to [determine_layout]") { get :determine_layout }
   end
 
-  def test_access_to_request_in_view
-    view_internals_old_value = ActionController::Base.view_controller_internals
-
-    ActionController::Base.view_controller_internals = false
-    ActionController::Base.protected_variables_cache = nil
-
-    get :hello_world
-    assert_nil assigns["request"]
-
-    ActionController::Base.view_controller_internals = true
-    ActionController::Base.protected_variables_cache = nil
-
-    get :hello_world
-    assert_kind_of ActionController::AbstractRequest,  assigns["request"]
-
-    ActionController::Base.view_controller_internals = view_internals_old_value
-    ActionController::Base.protected_variables_cache = nil
-  end
-
   def test_render_xml
     assert_deprecated_render { get :render_xml_hello }
     assert_equal "<html>\n  <p>Hello David</p>\n<p>This is grand!</p>\n</html>\n", @response.body
diff --git a/actionpack/test/controller/verification_test.rb b/actionpack/test/controller/verification_test.rb
index c5ff175d1b..05012cf724 100644
--- a/actionpack/test/controller/verification_test.rb
+++ b/actionpack/test/controller/verification_test.rb
@@ -59,15 +59,15 @@ class VerificationTest < Test::Unit::TestCase
     end
 
     def guarded_by_method
-      render :text => "#{@request.method}"
+      render :text => "#{request.method}"
     end
     
     def guarded_by_xhr
-      render :text => "#{@request.xhr?}"
+      render :text => "#{request.xhr?}"
     end
     
     def guarded_by_not_xhr
-      render :text => "#{@request.xhr?}"
+      render :text => "#{request.xhr?}"
     end
 
     def unguarded
diff --git a/actionpack/test/fixtures/deprecated_instance_variables/_request_ivar.rhtml b/actionpack/test/fixtures/deprecated_instance_variables/_request_ivar.rhtml
new file mode 100644
index 0000000000..a1680c23d5
--- /dev/null
+++ b/actionpack/test/fixtures/deprecated_instance_variables/_request_ivar.rhtml
@@ -0,0 +1 @@
+<%= @request.method %>
diff --git a/actionpack/test/fixtures/deprecated_instance_variables/_request_method.rhtml b/actionpack/test/fixtures/deprecated_instance_variables/_request_method.rhtml
new file mode 100644
index 0000000000..0c74cf1c1b
--- /dev/null
+++ b/actionpack/test/fixtures/deprecated_instance_variables/_request_method.rhtml
@@ -0,0 +1 @@
+<%= request.method %>