From 11ebed60293df204cff7c210593fb4b18a9763be Mon Sep 17 00:00:00 2001 From: Lance Ivy Date: Sat, 23 May 2009 19:47:40 -0700 Subject: [PATCH] deprecate attr_accessible and attr_protected --- activerecord/lib/active_record/base.rb | 7 ++- activerecord/test/cases/base_test.rb | 63 +------------------------ activerecord/test/cases/finder_test.rb | 26 ++-------- activerecord/test/cases/validations_test.rb | 24 --------- activerecord/test/models/company.rb | 1 - activerecord/test/models/company_in_module.rb | 1 - activerecord/test/models/reply.rb | 4 +- 7 files changed, 13 insertions(+), 113 deletions(-) diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index b459079..e08f513 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -1040,6 +1040,7 @@ module ActiveRecord #:nodoc: # To start from an all-closed default and enable attributes as needed, # have a look at +attr_accessible+. def attr_protected(*attributes) + ActiveSupport::Deprecation.warn("ActiveRecord::Base.attr_protected has been deprecated. Please specify assignable attributes at call time instead.", caller) write_inheritable_attribute(:attr_protected, Set.new(attributes.map {|a| a.to_s}) + (protected_attributes || [])) end @@ -1076,6 +1077,7 @@ module ActiveRecord #:nodoc: # customer.credit_rating = "Average" # customer.credit_rating # => "Average" def attr_accessible(*attributes) + ActiveSupport::Deprecation.warn("ActiveRecord::Base.attr_protected has been deprecated. Please specify assignable attributes at call time instead.", caller) write_inheritable_attribute(:attr_accessible, Set.new(attributes.map(&:to_s)) + (accessible_attributes || [])) end @@ -1912,7 +1914,7 @@ module ActiveRecord #:nodoc: # # if args[0].is_a?(Hash) # attributes = args[0].with_indifferent_access - # allowed_attributes = nil + # allowed_attributes = args[1] # find_attributes = attributes.slice(*attribute_names) # else # find_attributes = attributes = construct_attributes_from_arguments(attribute_names, args) @@ -1939,7 +1941,7 @@ module ActiveRecord #:nodoc: if args[0].is_a?(Hash) attributes = args[0].with_indifferent_access - allowed_attributes = nil + allowed_attributes = args[1] # or nil find_attributes = attributes.slice(*attribute_names) else find_attributes = attributes = construct_attributes_from_arguments(attribute_names, args) @@ -2768,6 +2770,7 @@ module ActiveRecord #:nodoc: end def attributes=(new_attributes, guard_protected_attributes = true) + ActiveSupport::Deprecation.warn("ActiveRecord::Base#attributes= has been deprecated. Please use assign() instead.", caller) allowed_attributes = guard_protected_attributes ? nil : new_attributes.keys assign(new_attributes, allowed_attributes) end diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index 7c84fd4..2b803c4 100755 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -40,25 +40,19 @@ class TestOracleDefault < ActiveRecord::Base; end class LoosePerson < ActiveRecord::Base self.table_name = 'people' self.abstract_class = true - attr_protected :credit_rating, :administrator end class LooseDescendant < LoosePerson - attr_protected :phone_number end class LooseDescendantSecond< LoosePerson - attr_protected :phone_number - attr_protected :name end class TightPerson < ActiveRecord::Base self.table_name = 'people' - attr_accessible :name, :address end class TightDescendant < TightPerson - attr_accessible :phone_number end class ReadonlyTitlePost < Post @@ -68,13 +62,6 @@ end class Booleantest < ActiveRecord::Base; end class Task < ActiveRecord::Base - attr_protected :starting -end - -class TopicWithProtectedContentAndAccessibleAuthorName < ActiveRecord::Base - self.table_name = 'topics' - attr_accessible :author_name - attr_protected :content end class BasicsTest < ActiveRecord::TestCase @@ -909,19 +896,6 @@ class BasicsTest < ActiveRecord::TestCase assert_raise(ActiveRecord::RecordInvalid) { reply.update_attributes!(:title => nil, :content => "Have a nice evening") } end - def test_mass_assignment_should_raise_exception_if_accessible_and_protected_attribute_writers_are_both_used - topic = TopicWithProtectedContentAndAccessibleAuthorName.new - assert_raise(RuntimeError) { topic.assign({ "author_name" => "me" }) } - assert_raise(RuntimeError) { topic.assign({ "content" => "stuff" }) } - end - - def test_mass_assignment_protection - firm = Firm.new - firm.assign({ "name" => "Next Angle", "rating" => 5 }) - assert_equal "Next Angle", firm.name - assert_equal 1, firm.rating - end - def test_mass_assignment_with_allowed_attributes firm = Firm.new firm.assign({ "name" => "Next Angle", "rating" => 5 }, ["name"]) @@ -929,13 +903,6 @@ class BasicsTest < ActiveRecord::TestCase assert_equal 1, firm.rating end - def test_mass_assignment_with_allowed_attributes_ignores_attr_protected - firm = Firm.new - firm.assign({ "name" => "Next Angle", "rating" => 5 }, ["name", "rating"]) - assert_equal "Next Angle", firm.name - assert_equal 5, firm.rating - end - def test_mass_assignment_on_initialization_with_allowed_attributes firm = Firm.new({ "name" => "Next Angle", "rating" => 5 }, ["name", "rating"]) assert_equal "Next Angle", firm.name @@ -952,7 +919,7 @@ class BasicsTest < ActiveRecord::TestCase end end - def test_customized_primary_key_remains_protected + def test_customized_primary_key_remains_default_protected subscriber = Subscriber.new subscriber.assign(:nick => 'webster123', :name => 'nice try') assert_nil subscriber.id @@ -961,7 +928,7 @@ class BasicsTest < ActiveRecord::TestCase assert_nil keyboard.id end - def test_customized_primary_key_remains_protected_when_referred_to_as_id + def test_customized_primary_key_remains_default_protected_when_referred_to_as_id subscriber = Subscriber.new subscriber.assign(:id => 'webster123', :name => 'nice try') assert_nil subscriber.id @@ -998,23 +965,6 @@ class BasicsTest < ActiveRecord::TestCase assert !reply.approved? end - def test_mass_assignment_protection_inheritance - assert_nil LoosePerson.accessible_attributes - assert_equal Set.new([ 'credit_rating', 'administrator' ]), LoosePerson.protected_attributes - - assert_nil LooseDescendant.accessible_attributes - assert_equal Set.new([ 'credit_rating', 'administrator', 'phone_number' ]), LooseDescendant.protected_attributes - - assert_nil LooseDescendantSecond.accessible_attributes - assert_equal Set.new([ 'credit_rating', 'administrator', 'phone_number', 'name' ]), LooseDescendantSecond.protected_attributes, 'Running attr_protected twice in one class should merge the protections' - - assert_nil TightPerson.protected_attributes - assert_equal Set.new([ 'name', 'address' ]), TightPerson.accessible_attributes - - assert_nil TightDescendant.protected_attributes - assert_equal Set.new([ 'name', 'address', 'phone_number' ]), TightDescendant.accessible_attributes - end - def test_readonly_attributes assert_equal Set.new([ 'title' , 'comments_count' ]), ReadonlyTitlePost.readonly_attributes @@ -1169,15 +1119,6 @@ class BasicsTest < ActiveRecord::TestCase assert_equal Time.local(2004, 6, 24, 16, 24, 0), topic.written_on end - def test_multiparameter_mass_assignment_protector - task = Task.new - time = Time.mktime(2000, 1, 1, 1) - task.starting = time - attributes = { "starting(1i)" => "2004", "starting(2i)" => "6", "starting(3i)" => "24" } - task.assign(attributes) - assert_equal time, task.starting - end - def test_multiparameter_assignment_of_aggregation customer = Customer.new address = Address.new("The Street", "The City", "The Country") diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index 037b67e..6a9683e 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -833,39 +833,23 @@ class FinderTest < ActiveRecord::TestCase end def test_find_or_initialize_from_one_attribute_should_not_set_attribute_even_when_protected - c = Company.find_or_initialize_by_name({:name => "Fortune 1000", :rating => 1000}) + c = Company.find_or_initialize_by_name({:name => "Fortune 1000", :rating => 1000}, [:name]) assert_equal "Fortune 1000", c.name assert_not_equal 1000, c.rating assert c.valid? assert c.new_record? end - def test_find_or_create_from_one_attribute_should_set_not_attribute_even_when_protected - c = Company.find_or_create_by_name({:name => "Fortune 1000", :rating => 1000}) + def test_find_or_create_from_one_attribute_should_not_set_attribute_even_when_protected + c = Company.find_or_create_by_name({:name => "Fortune 1000", :rating => 1000}, [:name]) assert_equal "Fortune 1000", c.name assert_not_equal 1000, c.rating assert c.valid? assert !c.new_record? end - def test_find_or_initialize_from_one_attribute_should_set_attribute_even_when_protected - c = Company.find_or_initialize_by_name_and_rating("Fortune 1000", 1000) - assert_equal "Fortune 1000", c.name - assert_equal 1000, c.rating - assert c.valid? - assert c.new_record? - end - - def test_find_or_create_from_one_attribute_should_set_attribute_even_when_protected - c = Company.find_or_create_by_name_and_rating("Fortune 1000", 1000) - assert_equal "Fortune 1000", c.name - assert_equal 1000, c.rating - assert c.valid? - assert !c.new_record? - end - def test_find_or_initialize_should_set_protected_attributes_if_given_as_block - c = Company.find_or_initialize_by_name(:name => "Fortune 1000") { |f| f.rating = 1000 } + c = Company.find_or_initialize_by_name({:name => "Fortune 1000"}, [:name]) { |f| f.rating = 1000 } assert_equal "Fortune 1000", c.name assert_equal 1000.to_f, c.rating.to_f assert c.valid? @@ -873,7 +857,7 @@ class FinderTest < ActiveRecord::TestCase end def test_find_or_create_should_set_protected_attributes_if_given_as_block - c = Company.find_or_create_by_name(:name => "Fortune 1000") { |f| f.rating = 1000 } + c = Company.find_or_create_by_name({:name => "Fortune 1000"}, [:name]) { |f| f.rating = 1000 } assert_equal "Fortune 1000", c.name assert_equal 1000.to_f, c.rating.to_f assert c.valid? diff --git a/activerecord/test/cases/validations_test.rb b/activerecord/test/cases/validations_test.rb index c20f5ae..9cc3be8 100644 --- a/activerecord/test/cases/validations_test.rb +++ b/activerecord/test/cases/validations_test.rb @@ -24,12 +24,6 @@ class Topic end end -class ProtectedPerson < ActiveRecord::Base - set_table_name 'people' - attr_accessor :addon - attr_protected :first_name -end - class UniqueReply < Reply validates_uniqueness_of :content, :scope => 'parent_id' end @@ -154,24 +148,6 @@ class ValidationsTest < ActiveRecord::TestCase end end - def test_create_with_exceptions_using_scope_for_protected_attributes - assert_nothing_raised do - ProtectedPerson.with_scope( :create => { :first_name => "Mary" } ) do - person = ProtectedPerson.create! :addon => "Addon" - assert_equal person.first_name, "Mary", "scope should ignore attr_protected" - end - end - end - - def test_create_with_exceptions_using_scope_and_empty_attributes - assert_nothing_raised do - ProtectedPerson.with_scope( :create => { :first_name => "Mary" } ) do - person = ProtectedPerson.create! - assert_equal person.first_name, "Mary", "should be ok when no attributes are passed to create!" - end - end - end - def test_single_error_per_attr_iteration r = Reply.new r.save diff --git a/activerecord/test/models/company.rb b/activerecord/test/models/company.rb index eb68153..c5d79d1 100644 --- a/activerecord/test/models/company.rb +++ b/activerecord/test/models/company.rb @@ -3,7 +3,6 @@ class AbstractCompany < ActiveRecord::Base end class Company < AbstractCompany - attr_protected :rating set_sequence_name :companies_nonstd_seq validates_presence_of :name diff --git a/activerecord/test/models/company_in_module.rb b/activerecord/test/models/company_in_module.rb index 3c34efb..0f33d16 100644 --- a/activerecord/test/models/company_in_module.rb +++ b/activerecord/test/models/company_in_module.rb @@ -3,7 +3,6 @@ require 'active_support/core_ext/object/misc' module MyApplication module Business class Company < ActiveRecord::Base - attr_protected :rating end class Firm < Company diff --git a/activerecord/test/models/reply.rb b/activerecord/test/models/reply.rb index 1c990ac..4c24f15 100644 --- a/activerecord/test/models/reply.rb +++ b/activerecord/test/models/reply.rb @@ -9,8 +9,6 @@ class Reply < Topic validate :errors_on_empty_content validate_on_create :title_is_wrong_create - attr_accessible :title, :author_name, :author_email_address, :written_on, :content, :last_read - def validate errors.add("title", "Empty") unless attribute_present? "title" end @@ -42,4 +40,4 @@ module Web class Reply < Web::Topic belongs_to :topic, :foreign_key => "parent_id", :counter_cache => true, :class_name => 'Web::Topic' end -end \ No newline at end of file +end -- 1.6.0.4