From c40b1a4a67ccaa3be31edb13399d93da0c628567 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Mon, 7 Aug 2006 12:40:14 +0000 Subject: Deprecate direct usage of @params. Update ActionView::Base for instance var deprecation. git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@4715 5ecf4fe2-1ee6-0310-87b1-e25e094e27de --- actionpack/CHANGELOG | 2 +- actionpack/examples/address_book_controller.rb | 6 ++--- actionpack/examples/blog_controller.cgi | 2 +- actionpack/examples/debate_controller.cgi | 10 +++---- actionpack/lib/action_controller/base.rb | 20 +++++++------- actionpack/lib/action_controller/pagination.rb | 2 +- actionpack/lib/action_controller/rescue.rb | 4 +-- .../templates/rescues/_request_and_response.rhtml | 2 +- actionpack/lib/action_controller/verification.rb | 4 +-- actionpack/lib/action_view/base.rb | 3 ++- .../test/controller/action_pack_assertions_test.rb | 2 +- actionpack/test/controller/components_test.rb | 2 +- actionpack/test/controller/verification_test.rb | 8 +++--- .../template/deprecated_instance_variables_test.rb | 31 ++++++++++++++++++++++ 14 files changed, 66 insertions(+), 32 deletions(-) create mode 100644 actionpack/test/template/deprecated_instance_variables_test.rb diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index d8104951c7..d8e390c9d4 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -2,7 +2,7 @@ * Add support for the param_name parameter to the auto_complete_field helper. #5026 [david.a.williams@gmail.com] -* Deprecation! @session and @flash will be removed after 1.2. Use the session and flash methods instead. You'll get printed warnings during tests and logged warnings in dev mode when you access either instance variable directly. [Jeremy Kemper] +* Deprecation! @params, @session, @flash will be removed after 1.2. Use the corresponding instance methods instead. You'll get printed warnings during tests and logged warnings in dev mode when you access either instance variable directly. [Jeremy Kemper] * Make Routing noisy when an anchor regexp is assigned to a segment. #5674 [francois.beausoleil@gmail.com] diff --git a/actionpack/examples/address_book_controller.rb b/actionpack/examples/address_book_controller.rb index 01d498e1bc..9ca6612512 100644 --- a/actionpack/examples/address_book_controller.rb +++ b/actionpack/examples/address_book_controller.rb @@ -28,11 +28,11 @@ class AddressBookController < ActionController::Base end def person - @person = @address_book.find_person(@params["id"]) + @person = @address_book.find_person(params[:id]) end def create_person - @address_book.create_person(@params["person"]) + @address_book.create_person(params[:person]) redirect_to :action => "index" end @@ -49,4 +49,4 @@ begin AddressBookController.process_cgi(CGI.new) if $0 == __FILE__ rescue => e CGI.new.out { "#{e.class}: #{e.message}" } -end \ No newline at end of file +end diff --git a/actionpack/examples/blog_controller.cgi b/actionpack/examples/blog_controller.cgi index b0c14033a2..3868c306df 100755 --- a/actionpack/examples/blog_controller.cgi +++ b/actionpack/examples/blog_controller.cgi @@ -32,7 +32,7 @@ class BlogController < ActionController::Base end def create - @session["posts"].unshift(Post.new(@params["post"]["title"], @params["post"]["body"])) + @session["posts"].unshift(Post.new(params[:post][:title], params[:post][:body])) flash["alert"] = "New post added!" redirect_to :action => "index" end diff --git a/actionpack/examples/debate_controller.cgi b/actionpack/examples/debate_controller.cgi index b82ac6259d..dbe022568b 100755 --- a/actionpack/examples/debate_controller.cgi +++ b/actionpack/examples/debate_controller.cgi @@ -25,19 +25,19 @@ class DebateController < ActionController::Base end def topic - @topic = @debate.find_topic(@params["id"]) + @topic = @debate.find_topic(params[:id]) end # def new_topic() end <-- This is not needed as the template doesn't require any assigns def create_topic - @debate.create_topic(@params["topic"]) + @debate.create_topic(params[:topic]) redirect_to :action => "index" end def create_reply - @debate.create_reply(@params["reply"]) - redirect_to :action => "topic", :path_params => { "id" => @params["reply"]["topic_id"] } + @debate.create_reply(params[:reply]) + redirect_to :action => "topic", :path_params => { "id" => params[:reply][:topic_id] } end private @@ -54,4 +54,4 @@ begin DebateController.process_cgi(CGI.new) if $0 == __FILE__ rescue => e CGI.new.out { "#{e.class}: #{e.message}" } -end \ No newline at end of file +end diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index 03b89826ce..8823076fcc 100755 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -287,17 +287,17 @@ module ActionController #:nodoc: # Holds the request object that's primarily used to get environment variables through access like # request.env["REQUEST_URI"]. attr_accessor :request - + # Holds a hash of all the GET, POST, and Url parameters passed to the action. Accessed like params["post_id"] # to get the post_id. No type casts are made, so all values are returned as strings. - attr_accessor :params - + attr_internal :params + # Holds the response object that's primarily used to set additional HTTP headers through access like # response.headers["Cache-Control"] = "no-cache". Can also be used to access the final body HTML after a template # has been rendered through response.body -- useful for after_filters that wants to manipulate the output, # such as a OutputCompressionFilter. attr_accessor :response - + # Holds a hash of objects in the session. Accessed like session[:person] to get the object tied to the "person" # key. The session will hold any type of object as values, but the key should be a string or symbol. attr_internal :session @@ -932,7 +932,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 @@ -946,7 +946,7 @@ module ActionController #:nodoc: # TODO: assigns cookies headers params request response template - DEPRECATED_INSTANCE_VARIABLES = %w(flash session) + DEPRECATED_INSTANCE_VARIABLES = %w(flash params session) # Gone after 1.2. def assign_deprecated_shortcuts(request, response) @@ -1019,16 +1019,18 @@ module ActionController #:nodoc: end def add_class_variables_to_assigns - %w( template_root logger template_class ignore_missing_templates ).each do |cvar| + %w(template_root logger template_class ignore_missing_templates).each do |cvar| @assigns[cvar] = self.send(cvar) end end def protected_instance_variables if view_controller_internals - [ "@assigns", "@performed_redirect", "@performed_render" ] + %w(@assigns @performed_redirect @performed_render) else - [ "@assigns", "@performed_redirect", "@performed_render", "@request", "@response", "@session", "@cookies", "@template", "@request_origin", "@parent_controller" ] + %w(@assigns @performed_redirect @performed_render + @request @response @_params @_session @session + @cookies @template @request_origin @parent_controller) end end diff --git a/actionpack/lib/action_controller/pagination.rb b/actionpack/lib/action_controller/pagination.rb index f7cd74120e..54084c0361 100644 --- a/actionpack/lib/action_controller/pagination.rb +++ b/actionpack/lib/action_controller/pagination.rb @@ -191,7 +191,7 @@ module ActionController def paginator_and_collection_for(collection_id, options) #:nodoc: klass = options[:class_name].constantize - page = @params[options[:parameter]] + page = params[options[:parameter]] count = count_collection_for_pagination(klass, options) paginator = Paginator.new(self, count, options[:per_page], page) collection = find_collection_for_pagination(klass, options, paginator) diff --git a/actionpack/lib/action_controller/rescue.rb b/actionpack/lib/action_controller/rescue.rb index 9b57d35587..7cd05eab32 100644 --- a/actionpack/lib/action_controller/rescue.rb +++ b/actionpack/lib/action_controller/rescue.rb @@ -79,7 +79,7 @@ module ActionController #:nodoc: begin perform_action_without_rescue rescue Object => exception - if defined?(Breakpoint) && @params["BP-RETRY"] + if defined?(Breakpoint) && params["BP-RETRY"] msg = exception.backtrace.first if md = /^(.+?):(\d+)(?::in `(.+)')?$/.match(msg) then origin_file, origin_line = md[1], md[2].to_i @@ -87,7 +87,7 @@ module ActionController #:nodoc: set_trace_func(lambda do |type, file, line, method, context, klass| if file == origin_file and line == origin_line then set_trace_func(nil) - @params["BP-RETRY"] = false + params["BP-RETRY"] = false callstack = caller callstack.slice!(0) if callstack.first["rescue.rb"] diff --git a/actionpack/lib/action_controller/templates/rescues/_request_and_response.rhtml b/actionpack/lib/action_controller/templates/rescues/_request_and_response.rhtml index 81995320c2..82007ff5e6 100644 --- a/actionpack/lib/action_controller/templates/rescues/_request_and_response.rhtml +++ b/actionpack/lib/action_controller/templates/rescues/_request_and_response.rhtml @@ -11,7 +11,7 @@ <%= form_tag(@request.request_uri, "method" => @request.method) %> - <% for key, values in @params %> + <% for key, values in params %> <% next if key == "BP-RETRY" %> <% for value in Array(values) %> diff --git a/actionpack/lib/action_controller/verification.rb b/actionpack/lib/action_controller/verification.rb index 8e0b255f26..ebf30becac 100644 --- a/actionpack/lib/action_controller/verification.rb +++ b/actionpack/lib/action_controller/verification.rb @@ -76,8 +76,8 @@ module ActionController #:nodoc: def verify_action(options) #:nodoc: prereqs_invalid = - [*options[:params] ].find { |v| @params[v].nil? } || - [*options[:session]].find { |v| @session[v].nil? } || + [*options[:params] ].find { |v| params[v].nil? } || + [*options[:session]].find { |v| session[v].nil? } || [*options[:flash] ].find { |v| flash[v].nil? } if !prereqs_invalid && options[:method] diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb index 13b2ef4821..b38501359c 100644 --- a/actionpack/lib/action_view/base.rb +++ b/actionpack/lib/action_view/base.rb @@ -148,7 +148,8 @@ module ActionView #:nodoc: attr_accessor :base_path, :assigns, :template_extension attr_accessor :controller - attr_reader :logger, :params, :request, :response, :session, :headers, :flash + attr_reader :logger, :request, :response, :headers + attr_internal :flash, :params, :session # Specify trim mode for the ERB compiler. Defaults to '-'. # See ERB documentation for suitable values. diff --git a/actionpack/test/controller/action_pack_assertions_test.rb b/actionpack/test/controller/action_pack_assertions_test.rb index 0ef31cb851..56b1752392 100644 --- a/actionpack/test/controller/action_pack_assertions_test.rb +++ b/actionpack/test/controller/action_pack_assertions_test.rb @@ -54,7 +54,7 @@ class ActionPackAssertionsController < ActionController::Base end def render_based_on_parameters - render_text "Mr. #{@params["name"]}" + render_text "Mr. #{params[:name]}" end def render_url diff --git a/actionpack/test/controller/components_test.rb b/actionpack/test/controller/components_test.rb index 075bc98a28..57e3b21acf 100644 --- a/actionpack/test/controller/components_test.rb +++ b/actionpack/test/controller/components_test.rb @@ -46,7 +46,7 @@ end class CalleeController < ActionController::Base def being_called - render_text "#{@params["name"] || "Lady"} of the House, speaking" + render_text "#{params[:name] || "Lady"} of the House, speaking" end def blowing_up diff --git a/actionpack/test/controller/verification_test.rb b/actionpack/test/controller/verification_test.rb index dc95c4b8a1..a3a913d42f 100644 --- a/actionpack/test/controller/verification_test.rb +++ b/actionpack/test/controller/verification_test.rb @@ -34,15 +34,15 @@ class VerificationTest < Test::Unit::TestCase verify :only => :must_be_post, :method => :post, :render => { :status => 405, :text => "Must be post" }, :add_headers => { "Allow" => "POST" } def guarded_one - render :text => "#{@params["one"]}" + render :text => "#{params[:one]}" end def guarded_with_flash - render :text => "#{@params["one"]}" + render :text => "#{params[:one]}" end def guarded_two - render :text => "#{@params["one"]}:#{@params["two"]}" + render :text => "#{params[:one]}:#{params[:two]}" end def guarded_in_session @@ -70,7 +70,7 @@ class VerificationTest < Test::Unit::TestCase end def unguarded - render :text => "#{@params["one"]}" + render :text => "#{params[:one]}" end def two_redirects diff --git a/actionpack/test/template/deprecated_instance_variables_test.rb b/actionpack/test/template/deprecated_instance_variables_test.rb new file mode 100644 index 0000000000..86fc7d6139 --- /dev/null +++ b/actionpack/test/template/deprecated_instance_variables_test.rb @@ -0,0 +1,31 @@ +require File.dirname(__FILE__) + '/../abstract_unit' + +class DeprecatedInstanceVariablesTest < Test::Unit::TestCase + class Target < ActionController::Base + ActionController::Base::DEPRECATED_INSTANCE_VARIABLES.each do |var| + class_eval <<-end_eval + def old_#{var}; render :inline => '<%= @#{var}.inspect %>' end + def new_#{var}; render :inline => '<%= #{var}.inspect %>' end + end_eval + end + + def rescue_action(e) raise e end + end + + def setup + @request = ActionController::TestRequest.new + @response = ActionController::TestResponse.new + @controller = Target.new + end + + ActionController::Base::DEPRECATED_INSTANCE_VARIABLES.each do |var| + class_eval <<-end_eval, __FILE__, __LINE__ + def test_old_#{var}_is_deprecated + assert_deprecated('@#{var}') { get :old_#{var} } + end + def test_new_#{var}_isnt_deprecated + assert_not_deprecated { get :new_#{var} } + end + end_eval + end +end -- cgit v1.2.3