From 7df39b8b337322c417c00830d463e8b7a96bc7f9 Mon Sep 17 00:00:00 2001 From: Dan Manges Date: Mon, 9 Mar 2009 00:05:50 -0500 Subject: [PATCH] rename ActiveRecord::Base#attributes= to merge_attributes --- .../lib/active_record/attribute_methods.rb | 2 +- activerecord/lib/active_record/base.rb | 36 +++++++------ .../lib/active_record/nested_attributes.rb | 2 +- .../active_record/serializers/json_serializer.rb | 2 +- .../active_record/serializers/xml_serializer.rb | 2 +- activerecord/test/cases/attribute_methods_test.rb | 2 +- activerecord/test/cases/base_test.rb | 54 +++++++++++-------- activerecord/test/cases/dirty_test.rb | 2 +- activerecord/test/cases/nested_attributes_test.rb | 20 ++++---- 9 files changed, 67 insertions(+), 55 deletions(-) diff --git a/activerecord/lib/active_record/attribute_methods.rb b/activerecord/lib/active_record/attribute_methods.rb index 3ffc489..3ad4a81 100644 --- a/activerecord/lib/active_record/attribute_methods.rb +++ b/activerecord/lib/active_record/attribute_methods.rb @@ -224,7 +224,7 @@ module ActiveRecord # Allows access to the object attributes, which are held in the @attributes hash, as though they # were first-class methods. So a Person class with a name attribute can use Person#name and # Person#name= and never directly use the attributes hash -- except for multiple assigns with - # ActiveRecord#attributes=. A Milestone class can also ask Milestone#completed? to test that + # ActiveRecord#merge_attributes. A Milestone class can also ask Milestone#completed? to test that # the completed attribute is not +nil+ or 0. # # It's also possible to instantiate related objects, so a Client class belonging to the clients diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 60a1221..6cad272 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -127,7 +127,7 @@ module ActiveRecord #:nodoc: end # Raised when an error occurred while doing a mass assignment to an attribute through the - # attributes= method. The exception has an +attribute+ property that is the name of the + # merge_attributes method. The exception has an +attribute+ property that is the name of the # offending attribute. class AttributeAssignmentError < ActiveRecordError attr_reader :exception, :attribute @@ -380,9 +380,9 @@ module ActiveRecord #:nodoc: # nothing was found, please check its documentation for further details. # * StatementInvalid - The database server rejected the SQL statement. The precise error is added in the message. # * MultiparameterAssignmentErrors - Collection of errors that occurred during a mass assignment using the - # attributes= method. The +errors+ property of this exception contains an array of AttributeAssignmentError + # merge_attributes method. The +errors+ property of this exception contains an array of AttributeAssignmentError # objects that should be inspected to determine which attributes triggered the errors. - # * AttributeAssignmentError - An error occurred while doing a mass assignment through the attributes= method. + # * AttributeAssignmentError - An error occurred while doing a mass assignment through the merge_attributes method. # You can inspect the +attribute+ property of the exception object to determine which attribute triggered the error. # # *Note*: The attributes listed are class-level attributes (accessible from both the class and instance level). @@ -1007,7 +1007,7 @@ module ActiveRecord #:nodoc: # Attributes named in this macro are protected from mass-assignment, # such as new(attributes), # update_attributes(attributes), or - # attributes=(attributes). + # merge_attributes(attributes). # # Mass-assignment to these attributes will simply be ignored, to assign # to them you can use direct writer methods. This is meant to protect @@ -1020,7 +1020,7 @@ module ActiveRecord #:nodoc: # # customer = Customer.new("name" => David, "credit_rating" => "Excellent") # customer.credit_rating # => nil - # customer.attributes = { "description" => "Jolly fellow", "credit_rating" => "Superb" } + # customer.merge_attributes { "description" => "Jolly fellow", "credit_rating" => "Superb" } # customer.credit_rating # => nil # # customer.credit_rating = "Average" @@ -1040,7 +1040,7 @@ module ActiveRecord #:nodoc: # Specifies a white list of model attributes that can be set via # mass-assignment, such as new(attributes), # update_attributes(attributes), or - # attributes=(attributes) + # merge_attributes(attributes) # # This is the opposite of the +attr_protected+ macro: Mass-assignment # will only set attributes in this list, to assign to the rest of @@ -1056,7 +1056,7 @@ module ActiveRecord #:nodoc: # # customer = Customer.new(:name => "David", :nickname => "Dave", :credit_rating => "Excellent") # customer.credit_rating # => nil - # customer.attributes = { :name => "Jolly fellow", :credit_rating => "Superb" } + # customer.merge_attributes :name => "Jolly fellow", :credit_rating => "Superb" # customer.credit_rating # => nil # # customer.credit_rating = "Average" @@ -1909,7 +1909,7 @@ module ActiveRecord #:nodoc: # record = find(:first, options) # # if record.nil? - # record = self.new { |r| r.send(:attributes=, attributes, guard_protected_attributes) } + # record = self.new { |r| r.send(:merge_attributes, attributes, guard_protected_attributes) } # yield(record) if block_given? # record.save # record @@ -1935,7 +1935,7 @@ module ActiveRecord #:nodoc: record = find(:first, options) if record.nil? - record = self.new { |r| r.send(:attributes=, attributes, guard_protected_attributes) } + record = self.new { |r| r.send(:merge_attributes, attributes, guard_protected_attributes) } #{'yield(record) if block_given?'} #{'record.save' if instantiator == :create} record @@ -2436,7 +2436,7 @@ module ActiveRecord #:nodoc: @attributes_cache = {} @new_record = true ensure_proper_type - self.attributes = attributes unless attributes.nil? + self.merge_attributes attributes unless attributes.nil? self.class.send(:scope, :create).each { |att,value| self.send("#{att}=", value) } if self.class.send(:scoped?, :create) result = yield self if block_given? callback(:after_initialize) if respond_to_without_attributes?(:after_initialize) @@ -2624,13 +2624,13 @@ module ActiveRecord #:nodoc: # Updates all the attributes from the passed-in Hash and saves the record. If the object is invalid, the saving will # fail and false will be returned. def update_attributes(attributes) - self.attributes = attributes + self.merge_attributes attributes save end # Updates an object just like Base.update_attributes but calls save! instead of save so an exception is raised if the record is invalid. def update_attributes!(attributes) - self.attributes = attributes + self.merge_attributes attributes save! end @@ -2724,13 +2724,13 @@ module ActiveRecord #:nodoc: # end # # user = User.new - # user.attributes = { :username => 'Phusion', :is_admin => true } + # user.merge_attributes :username => 'Phusion', :is_admin => true # user.username # => "Phusion" # user.is_admin? # => false # - # user.send(:attributes=, { :username => 'Phusion', :is_admin => true }, false) + # user.merge_attributes({ :username => 'Phusion', :is_admin => true }, false) # user.is_admin? # => true - def attributes=(new_attributes, guard_protected_attributes = true) + def merge_attributes(new_attributes, guard_protected_attributes = true) return if new_attributes.nil? attributes = new_attributes.dup attributes.stringify_keys! @@ -2749,7 +2749,11 @@ module ActiveRecord #:nodoc: assign_multiparameter_attributes(multi_parameter_attributes) end - + def attributes=(new_attributes, guard_protected_attributes = true) #:nodoc: + ActiveSupport::Deprecation.warn "ActiveRecord::Base#attributes= is deprecated; use ActiveRecord::Base#merge_attributes instead." + merge_attributes new_attributes, guard_protected_attributes + end + # Returns a hash of all the attributes with their names as keys and the values of the attributes as values. def attributes self.attribute_names.inject({}) do |attrs, name| diff --git a/activerecord/lib/active_record/nested_attributes.rb b/activerecord/lib/active_record/nested_attributes.rb index e3122d1..4b0ec50 100644 --- a/activerecord/lib/active_record/nested_attributes.rb +++ b/activerecord/lib/active_record/nested_attributes.rb @@ -309,7 +309,7 @@ module ActiveRecord if has_delete_flag?(attributes) && allow_destroy record.mark_for_destruction else - record.attributes = attributes.except(*UNASSIGNABLE_KEYS) + record.merge_attributes attributes.except(*UNASSIGNABLE_KEYS) end end diff --git a/activerecord/lib/active_record/serializers/json_serializer.rb b/activerecord/lib/active_record/serializers/json_serializer.rb index 1fd65ed..0bb1b7b 100644 --- a/activerecord/lib/active_record/serializers/json_serializer.rb +++ b/activerecord/lib/active_record/serializers/json_serializer.rb @@ -80,7 +80,7 @@ module ActiveRecord #:nodoc: end def from_json(json) - self.attributes = ActiveSupport::JSON.decode(json) + self.merge_attributes ActiveSupport::JSON.decode(json) self end diff --git a/activerecord/lib/active_record/serializers/xml_serializer.rb b/activerecord/lib/active_record/serializers/xml_serializer.rb index 4749823..291194d 100644 --- a/activerecord/lib/active_record/serializers/xml_serializer.rb +++ b/activerecord/lib/active_record/serializers/xml_serializer.rb @@ -157,7 +157,7 @@ module ActiveRecord #:nodoc: end def from_xml(xml) - self.attributes = Hash.from_xml(xml).values.first + self.merge_attributes Hash.from_xml(xml).values.first self end end diff --git a/activerecord/test/cases/attribute_methods_test.rb b/activerecord/test/cases/attribute_methods_test.rb index 17ed302..0e8e090 100644 --- a/activerecord/test/cases/attribute_methods_test.rb +++ b/activerecord/test/cases/attribute_methods_test.rb @@ -274,7 +274,7 @@ class AttributeMethodsTest < ActiveRecord::TestCase privatize("title=(value)") assert_raise(ActiveRecord::UnknownAttributeError) { topic = @target.new(:title => "Rants about pants") } - assert_raise(ActiveRecord::UnknownAttributeError) { @target.new.attributes = { :title => "Ants in pants" } } + assert_raise(ActiveRecord::UnknownAttributeError) { @target.new.merge_attributes :title => "Ants in pants" } end private diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index 748a64a..831a535 100755 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -86,12 +86,20 @@ class BasicsTest < ActiveRecord::TestCase def test_set_attributes topic = Topic.find(1) - topic.attributes = { "title" => "Budget", "author_name" => "Jason" } + topic.merge_attributes "title" => "Budget", "author_name" => "Jason" topic.save assert_equal("Budget", topic.title) assert_equal("Jason", topic.author_name) assert_equal(topics(:first).author_email_address, Topic.find(1).author_email_address) end + + def test_attributes_setter_is_deprecated + assert_deprecated('ActiveRecord::Base#attributes=') do + topic = Topic.find(1) + topic.attributes = {"title" => "New Title"} + assert_equal "New Title", topic.title + end + end def test_integers_as_nil test = AutoId.create('value' => '') @@ -201,7 +209,7 @@ class BasicsTest < ActiveRecord::TestCase def test_save_null_string_attributes topic = Topic.find(1) - topic.attributes = { "title" => "null", "author_name" => "null" } + topic.merge_attributes "title" => "null", "author_name" => "null" topic.save! topic.reload assert_equal("null", topic.title) @@ -232,7 +240,7 @@ class BasicsTest < ActiveRecord::TestCase topic = Topic.new(new_topic) assert_equal new_topic[:title], topic.title - topic.attributes= new_topic_values + topic.merge_attributes new_topic_values assert_equal new_topic_values[:title], topic.title end @@ -788,7 +796,7 @@ class BasicsTest < ActiveRecord::TestCase Topic.default_timezone = :utc attributes = { "bonus_time" => "5:42:00AM" } topic = Topic.find(1) - topic.attributes = attributes + topic.merge_attributes attributes assert_equal Time.utc(2000, 1, 1, 5, 42, 0), topic.bonus_time Topic.default_timezone = :local end @@ -910,13 +918,13 @@ class BasicsTest < ActiveRecord::TestCase def test_mass_assignment_should_raise_exception_if_accessible_and_protected_attribute_writers_are_both_used topic = TopicWithProtectedContentAndAccessibleAuthorName.new - assert_raise(RuntimeError) { topic.attributes = { "author_name" => "me" } } - assert_raise(RuntimeError) { topic.attributes = { "content" => "stuff" } } + assert_raise(RuntimeError) { topic.merge_attributes "author_name" => "me" } + assert_raise(RuntimeError) { topic.merge_attributes "content" => "stuff" } end def test_mass_assignment_protection firm = Firm.new - firm.attributes = { "name" => "Next Angle", "rating" => 5 } + firm.merge_attributes "name" => "Next Angle", "rating" => 5 assert_equal 1, firm.rating end @@ -950,13 +958,13 @@ class BasicsTest < ActiveRecord::TestCase firm = Firm.new assert_raise(ActiveRecord::UnknownAttributeError) do - firm.attributes = { "id" => 5, "type" => "Client", "i_dont_even_exist" => 20 } + firm.merge_attributes "id" => 5, "type" => "Client", "i_dont_even_exist" => 20 end end def test_mass_assignment_protection_on_defaults firm = Firm.new - firm.attributes = { "id" => 5, "type" => "Client" } + firm.merge_attributes "id" => 5, "type" => "Client" assert_nil firm.id assert_equal "Firm", firm[:type] end @@ -1006,7 +1014,7 @@ class BasicsTest < ActiveRecord::TestCase def test_multiparameter_attributes_on_date attributes = { "last_read(1i)" => "2004", "last_read(2i)" => "6", "last_read(3i)" => "24" } topic = Topic.find(1) - topic.attributes = attributes + topic.merge_attributes attributes # note that extra #to_date call allows test to pass for Oracle, which # treats dates/times the same assert_date_from_db Date.new(2004, 6, 24), topic.last_read.to_date @@ -1015,7 +1023,7 @@ class BasicsTest < ActiveRecord::TestCase def test_multiparameter_attributes_on_date_with_empty_date attributes = { "last_read(1i)" => "2004", "last_read(2i)" => "6", "last_read(3i)" => "" } topic = Topic.find(1) - topic.attributes = attributes + topic.merge_attributes attributes # note that extra #to_date call allows test to pass for Oracle, which # treats dates/times the same assert_date_from_db Date.new(2004, 6, 1), topic.last_read.to_date @@ -1024,7 +1032,7 @@ class BasicsTest < ActiveRecord::TestCase def test_multiparameter_attributes_on_date_with_all_empty attributes = { "last_read(1i)" => "", "last_read(2i)" => "", "last_read(3i)" => "" } topic = Topic.find(1) - topic.attributes = attributes + topic.merge_attributes attributes assert_nil topic.last_read end @@ -1034,7 +1042,7 @@ class BasicsTest < ActiveRecord::TestCase "written_on(4i)" => "16", "written_on(5i)" => "24", "written_on(6i)" => "00" } topic = Topic.find(1) - topic.attributes = attributes + topic.merge_attributes attributes assert_equal Time.local(2004, 6, 24, 16, 24, 0), topic.written_on end @@ -1044,7 +1052,7 @@ class BasicsTest < ActiveRecord::TestCase "written_on(4i)" => "16", "written_on(5i)" => "24", "written_on(6i)" => "00" } topic = Topic.find(1) - topic.attributes = attributes + topic.merge_attributes attributes # testing against to_s(:db) representation because either a Time or a DateTime might be returned, depending on platform assert_equal "1850-06-24 16:24:00", topic.written_on.to_s(:db) end @@ -1056,7 +1064,7 @@ class BasicsTest < ActiveRecord::TestCase "written_on(4i)" => "16", "written_on(5i)" => "24", "written_on(6i)" => "00" } topic = Topic.find(1) - topic.attributes = attributes + topic.merge_attributes attributes assert_equal Time.utc(2004, 6, 24, 16, 24, 0), topic.written_on ensure ActiveRecord::Base.default_timezone = :local @@ -1071,7 +1079,7 @@ class BasicsTest < ActiveRecord::TestCase "written_on(4i)" => "16", "written_on(5i)" => "24", "written_on(6i)" => "00" } topic = Topic.find(1) - topic.attributes = attributes + topic.merge_attributes attributes assert_equal Time.utc(2004, 6, 24, 23, 24, 0), topic.written_on assert_equal Time.utc(2004, 6, 24, 16, 24, 0), topic.written_on.time assert_equal Time.zone, topic.written_on.time_zone @@ -1089,7 +1097,7 @@ class BasicsTest < ActiveRecord::TestCase "written_on(4i)" => "16", "written_on(5i)" => "24", "written_on(6i)" => "00" } topic = Topic.find(1) - topic.attributes = attributes + topic.merge_attributes attributes assert_equal Time.local(2004, 6, 24, 16, 24, 0), topic.written_on assert_equal false, topic.written_on.respond_to?(:time_zone) ensure @@ -1106,7 +1114,7 @@ class BasicsTest < ActiveRecord::TestCase "written_on(4i)" => "16", "written_on(5i)" => "24", "written_on(6i)" => "00" } topic = Topic.find(1) - topic.attributes = attributes + topic.merge_attributes attributes assert_equal Time.utc(2004, 6, 24, 16, 24, 0), topic.written_on assert_equal false, topic.written_on.respond_to?(:time_zone) ensure @@ -1125,7 +1133,7 @@ class BasicsTest < ActiveRecord::TestCase "bonus_time(4i)" => "16", "bonus_time(5i)" => "24" } topic = Topic.find(1) - topic.attributes = attributes + topic.merge_attributes attributes assert_equal Time.utc(2000, 1, 1, 16, 24, 0), topic.bonus_time assert topic.bonus_time.utc? ensure @@ -1140,7 +1148,7 @@ class BasicsTest < ActiveRecord::TestCase "written_on(4i)" => "16", "written_on(5i)" => "24", "written_on(6i)" => "" } topic = Topic.find(1) - topic.attributes = attributes + topic.merge_attributes attributes assert_equal Time.local(2004, 6, 24, 16, 24, 0), topic.written_on end @@ -1149,7 +1157,7 @@ class BasicsTest < ActiveRecord::TestCase time = Time.mktime(2000, 1, 1, 1) task.starting = time attributes = { "starting(1i)" => "2004", "starting(2i)" => "6", "starting(3i)" => "24" } - task.attributes = attributes + task.merge_attributes attributes assert_equal time, task.starting end @@ -1157,7 +1165,7 @@ class BasicsTest < ActiveRecord::TestCase customer = Customer.new address = Address.new("The Street", "The City", "The Country") attributes = { "address(1)" => address.street, "address(2)" => address.city, "address(3)" => address.country } - customer.attributes = attributes + customer.merge_attributes attributes assert_equal address, customer.address end @@ -1169,7 +1177,7 @@ class BasicsTest < ActiveRecord::TestCase "bonus_time" => "5:42:00AM" } topic = Topic.find(1) - topic.attributes = attributes + topic.merge_attributes attributes assert_equal Time.local(2000, 1, 1, 5, 42, 0), topic.bonus_time end diff --git a/activerecord/test/cases/dirty_test.rb b/activerecord/test/cases/dirty_test.rb index ac95bac..67b6e3b 100644 --- a/activerecord/test/cases/dirty_test.rb +++ b/activerecord/test/cases/dirty_test.rb @@ -179,7 +179,7 @@ class DirtyTest < ActiveRecord::TestCase # Coming from web form. params = {:topic => {:approved => 1}} # In the controller. - topic.attributes = params[:topic] + topic.merge_attributes params[:topic] assert topic.approved? assert !topic.approved_changed? end diff --git a/activerecord/test/cases/nested_attributes_test.rb b/activerecord/test/cases/nested_attributes_test.rb index cd6277c..eee4608 100644 --- a/activerecord/test/cases/nested_attributes_test.rb +++ b/activerecord/test/cases/nested_attributes_test.rb @@ -174,7 +174,7 @@ class TestNestedAttributesOnAHasOneAssociation < ActiveRecord::TestCase def test_should_not_destroy_the_associated_model_until_the_parent_is_saved assert_no_difference('Ship.count') do - @pirate.attributes = { :ship_attributes => { :id => @ship.id, :_delete => '1' } } + @pirate.merge_attributes :ship_attributes => { :id => @ship.id, :_delete => '1' } end assert_difference('Ship.count', -1) do @pirate.save @@ -293,7 +293,7 @@ class TestNestedAttributesOnABelongsToAssociation < ActiveRecord::TestCase def test_should_not_destroy_the_associated_model_until_the_parent_is_saved assert_no_difference('Pirate.count') do - @ship.attributes = { :pirate_attributes => { :id => @ship.pirate.id, '_delete' => true } } + @ship.merge_attributes :pirate_attributes => { :id => @ship.pirate.id, '_delete' => true } end assert_difference('Pirate.count', -1) { @ship.save } end @@ -329,7 +329,7 @@ module NestedAttributesOnACollectionAssociationTests end def test_should_take_a_hash_and_assign_the_attributes_to_the_associated_models - @pirate.attributes = @alternate_params + @pirate.merge_attributes @alternate_params assert_equal 'Grace OMalley', @pirate.send(@association_name).first.name assert_equal 'Privateers Greed', @pirate.send(@association_name).last.name end @@ -338,21 +338,21 @@ module NestedAttributesOnACollectionAssociationTests @child_1.stubs(:id).returns('ABC1X') @child_2.stubs(:id).returns('ABC2X') - @pirate.attributes = { + @pirate.merge_attributes( association_getter => [ { :id => @child_1.id, :name => 'Grace OMalley' }, { :id => @child_2.id, :name => 'Privateers Greed' } ] - } + ) assert_equal ['Grace OMalley', 'Privateers Greed'], [@child_1.name, @child_2.name] end def test_should_automatically_build_new_associated_models_for_each_entry_in_a_hash_where_the_id_is_missing @pirate.send(@association_name).destroy_all - @pirate.reload.attributes = { + @pirate.reload.merge_attributes( association_getter => { 'foo' => { :name => 'Grace OMalley' }, 'bar' => { :name => 'Privateers Greed' }} - } + ) assert @pirate.send(@association_name).first.new_record? assert_equal 'Grace OMalley', @pirate.send(@association_name).first.name @@ -369,12 +369,12 @@ module NestedAttributesOnACollectionAssociationTests def test_should_ignore_new_associated_records_with_truthy_delete_attribute @pirate.send(@association_name).destroy_all - @pirate.reload.attributes = { + @pirate.reload.merge_attributes( association_getter => { 'foo' => { :name => 'Grace OMalley' }, 'bar' => { :name => 'Privateers Greed', '_delete' => '1' } } - } + ) assert_equal 1, @pirate.send(@association_name).length assert_equal 'Grace OMalley', @pirate.send(@association_name).first.name @@ -383,7 +383,7 @@ module NestedAttributesOnACollectionAssociationTests def test_should_ignore_new_associated_records_if_a_reject_if_proc_returns_false @alternate_params[association_getter]['baz'] = {} assert_no_difference("@pirate.send(@association_name).length") do - @pirate.attributes = @alternate_params + @pirate.merge_attributes @alternate_params end end -- 1.6.1.2