Revert "Removed #update_attribute method. New #update_column method."

This reverts commit 45c233ef.
Signed-off-by: NSantiago Pastorino <santiago@wyeworks.com>

activerecord/lib/active_record/associations/has_one_association.rb

@@ -34,8 +34,7 @@ def delete(method = options[:dependent])
             when :destroy
               target.destroy
             when :nullify
-              target.send("#{reflection.foreign_key}=", nil)
-              target.save(:validations => false)
+              target.update_attribute(reflection.foreign_key, nil)
           end
         end
       end

activerecord/lib/active_record/persistence.rb

@@ -104,17 +104,19 @@ def becomes(klass)
       became
     end
 
-    # Updates a single attribute of an object, without calling save.
+    # Updates a single attribute and saves the record.
+    # This is especially useful for boolean flags on existing records. Also note that
     #
     # * Validation is skipped.
-    # * Callbacks are skipped.
-    # * updated_at/updated_on column is not updated in any case.
+    # * Callbacks are invoked.
+    # * updated_at/updated_on column is updated if that column is available.
+    # * Updates all the attributes that are dirty in this object.
     #
-    def update_column(name, value)
+    def update_attribute(name, value)
       name = name.to_s
       raise ActiveRecordError, "#{name} is marked as readonly" if self.class.readonly_attributes.include?(name)
       send("#{name}=", value)
-      self.class.update_all({ name => value }, self.class.primary_key => id)
+      save(:validate => false)
     end
 
     # Updates the attributes of the model from the passed-in hash and saves the
@@ -154,7 +156,7 @@ def increment(attribute, by = 1)
     # Saving is not subjected to validation checks. Returns +true+ if the
     # record could be saved.
     def increment!(attribute, by = 1)
-      increment(attribute, by).update_column(attribute, self[attribute])
+      increment(attribute, by).update_attribute(attribute, self[attribute])
     end
 
     # Initializes +attribute+ to zero if +nil+ and subtracts the value passed as +by+ (default is 1).
@@ -171,7 +173,7 @@ def decrement(attribute, by = 1)
     # Saving is not subjected to validation checks. Returns +true+ if the
     # record could be saved.
     def decrement!(attribute, by = 1)
-      decrement(attribute, by).update_column(attribute, self[attribute])
+      decrement(attribute, by).update_attribute(attribute, self[attribute])
     end
 
     # Assigns to +attribute+ the boolean opposite of <tt>attribute?</tt>. So
@@ -188,7 +190,7 @@ def toggle(attribute)
     # Saving is not subjected to validation checks. Returns +true+ if the
     # record could be saved.
     def toggle!(attribute)
-      toggle(attribute).update_column(attribute, self[attribute])
+      toggle(attribute).update_attribute(attribute, self[attribute])
     end
 
     # Reloads the attributes of this object from the database.

activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb

@@ -604,7 +604,7 @@ def test_update_attributes_after_push_without_duplicate_join_table_rows
     project = SpecialProject.create("name" => "Special Project")
     assert developer.save
     developer.projects << project
-    developer.update_column("name", "Bruza")
+    developer.update_attribute("name", "Bruza")
     assert_equal 1, Developer.connection.select_value(<<-end_sql).to_i
       SELECT count(*) FROM developers_projects
       WHERE project_id = #{project.id}

activerecord/test/cases/associations/has_many_associations_test.rb

@@ -639,7 +639,7 @@ def test_deleting_updates_counter_cache_without_dependent_option
 
   def test_deleting_updates_counter_cache_with_dependent_delete_all
     post = posts(:welcome)
-    post.update_column(:taggings_with_delete_all_count, post.taggings_count)
+    post.update_attribute(:taggings_with_delete_all_count, post.taggings_count)
 
     assert_difference "post.reload.taggings_with_delete_all_count", -1 do
       post.taggings_with_delete_all.delete(post.taggings_with_delete_all.first)
@@ -648,7 +648,7 @@ def test_deleting_updates_counter_cache_with_dependent_delete_all
 
   def test_deleting_updates_counter_cache_with_dependent_destroy
     post = posts(:welcome)
-    post.update_column(:taggings_with_destroy_count, post.taggings_count)
+    post.update_attribute(:taggings_with_destroy_count, post.taggings_count)
 
     assert_difference "post.reload.taggings_with_destroy_count", -1 do
       post.taggings_with_destroy.delete(post.taggings_with_destroy.first)
@@ -787,7 +787,7 @@ def test_delete_all_association_with_primary_key_deletes_correct_records
     firm = Firm.find(:first)
     # break the vanilla firm_id foreign key
     assert_equal 2, firm.clients.count
-    firm.clients.first.update_column(:firm_id, nil)
+    firm.clients.first.update_attribute(:firm_id, nil)
     assert_equal 1, firm.clients(true).count
     assert_equal 1, firm.clients_using_primary_key_with_delete_all.count
     old_record = firm.clients_using_primary_key_with_delete_all.first

activerecord/test/cases/associations/has_many_through_associations_test.rb

@@ -286,7 +286,7 @@ def test_update_counter_caches_on_delete
   def test_update_counter_caches_on_delete_with_dependent_destroy
     post = posts(:welcome)
     tag  = post.tags.create!(:name => 'doomed')
-    post.update_column(:tags_with_destroy_count, post.tags.count)
+    post.update_attribute(:tags_with_destroy_count, post.tags.count)
 
     assert_difference ['post.reload.taggings_count', 'post.reload.tags_with_destroy_count'], -1 do
       posts(:welcome).tags_with_destroy.delete(tag)
@@ -296,7 +296,7 @@ def test_update_counter_caches_on_delete_with_dependent_destroy
   def test_update_counter_caches_on_delete_with_dependent_nullify
     post = posts(:welcome)
     tag  = post.tags.create!(:name => 'doomed')
-    post.update_column(:tags_with_nullify_count, post.tags.count)
+    post.update_attribute(:tags_with_nullify_count, post.tags.count)
 
     assert_no_difference 'post.reload.taggings_count' do
       assert_difference 'post.reload.tags_with_nullify_count', -1 do

activerecord/test/cases/associations/has_one_through_associations_test.rb

@@ -90,12 +90,12 @@ def test_has_one_through_eager_loading_through_polymorphic
   def test_has_one_through_with_conditions_eager_loading
     # conditions on the through table
     assert_equal clubs(:moustache_club), Member.find(@member.id, :include => :favourite_club).favourite_club
-    memberships(:membership_of_favourite_club).update_column(:favourite, false)
+    memberships(:membership_of_favourite_club).update_attribute(:favourite, false)
     assert_equal nil,                    Member.find(@member.id, :include => :favourite_club).reload.favourite_club
 
     # conditions on the source table
     assert_equal clubs(:moustache_club), Member.find(@member.id, :include => :hairy_club).hairy_club
-    clubs(:moustache_club).update_column(:name, "Association of Clean-Shaven Persons")
+    clubs(:moustache_club).update_attribute(:name, "Association of Clean-Shaven Persons")
     assert_equal nil,                    Member.find(@member.id, :include => :hairy_club).reload.hairy_club
   end
 

activerecord/test/cases/associations/join_model_test.rb

@@ -161,7 +161,7 @@ def test_create_polymorphic_has_one_with_scope
 
   def test_delete_polymorphic_has_many_with_delete_all
     assert_equal 1, posts(:welcome).taggings.count
-    posts(:welcome).taggings.first.update_column :taggable_type, 'PostWithHasManyDeleteAll'
+    posts(:welcome).taggings.first.update_attribute :taggable_type, 'PostWithHasManyDeleteAll'
     post = find_post_with_dependency(1, :has_many, :taggings, :delete_all)
 
     old_count = Tagging.count
@@ -172,7 +172,7 @@ def test_delete_polymorphic_has_many_with_delete_all
 
   def test_delete_polymorphic_has_many_with_destroy
     assert_equal 1, posts(:welcome).taggings.count
-    posts(:welcome).taggings.first.update_column :taggable_type, 'PostWithHasManyDestroy'
+    posts(:welcome).taggings.first.update_attribute :taggable_type, 'PostWithHasManyDestroy'
     post = find_post_with_dependency(1, :has_many, :taggings, :destroy)
 
     old_count = Tagging.count
@@ -183,7 +183,7 @@ def test_delete_polymorphic_has_many_with_destroy
 
   def test_delete_polymorphic_has_many_with_nullify
     assert_equal 1, posts(:welcome).taggings.count
-    posts(:welcome).taggings.first.update_column :taggable_type, 'PostWithHasManyNullify'
+    posts(:welcome).taggings.first.update_attribute :taggable_type, 'PostWithHasManyNullify'
     post = find_post_with_dependency(1, :has_many, :taggings, :nullify)
 
     old_count = Tagging.count
@@ -194,7 +194,7 @@ def test_delete_polymorphic_has_many_with_nullify
 
   def test_delete_polymorphic_has_one_with_destroy
     assert posts(:welcome).tagging
-    posts(:welcome).tagging.update_column :taggable_type, 'PostWithHasOneDestroy'
+    posts(:welcome).tagging.update_attribute :taggable_type, 'PostWithHasOneDestroy'
     post = find_post_with_dependency(1, :has_one, :tagging, :destroy)
 
     old_count = Tagging.count
@@ -205,7 +205,7 @@ def test_delete_polymorphic_has_one_with_destroy
 
   def test_delete_polymorphic_has_one_with_nullify
     assert posts(:welcome).tagging
-    posts(:welcome).tagging.update_column :taggable_type, 'PostWithHasOneNullify'
+    posts(:welcome).tagging.update_attribute :taggable_type, 'PostWithHasOneNullify'
     post = find_post_with_dependency(1, :has_one, :tagging, :nullify)
 
     old_count = Tagging.count
@@ -707,7 +707,7 @@ def test_has_many_through_goes_through_all_sti_classes
     # create dynamic Post models to allow different dependency options
     def find_post_with_dependency(post_id, association, association_name, dependency)
       class_name = "PostWith#{association.to_s.classify}#{dependency.to_s.classify}"
-      Post.find(post_id).update_column :type, class_name
+      Post.find(post_id).update_attribute :type, class_name
       klass = Object.const_set(class_name, Class.new(ActiveRecord::Base))
       klass.set_table_name 'posts'
       klass.send(association, association_name, :as => :taggable, :dependent => dependency)

activerecord/test/cases/associations_test.rb

@@ -66,7 +66,7 @@ def test_loading_the_association_target_should_load_most_recent_attributes_for_c
     ship = Ship.create!(:name => "The good ship Dollypop")
     part = ship.parts.create!(:name => "Mast")
     part.mark_for_destruction
-    ShipPart.find(part.id).update_column(:name, 'Deck')
+    ShipPart.find(part.id).update_attribute(:name, 'Deck')
     ship.parts.send(:load_target)
     assert_equal 'Deck', ship.parts[0].name
   end
@@ -170,7 +170,7 @@ def test_save_on_parent_does_not_load_target
     david = developers(:david)
 
     assert !david.projects.loaded?
-    david.update_column(:created_at, Time.now)
+    david.update_attribute(:created_at, Time.now)
     assert !david.projects.loaded?
   end
 

activerecord/test/cases/base_test.rb

@@ -484,7 +484,7 @@ def test_non_valid_identifier_column_name
     weird.reload
     assert_equal 'value', weird.send('a$b')
 
-    weird.update_column('a$b', 'value2')
+    weird.update_attribute('a$b', 'value2')
     weird.reload
     assert_equal 'value2', weird.send('a$b')
   end

activerecord/test/cases/calculations_test.rb

@@ -311,8 +311,8 @@ def test_should_count_scoped_select
 
   def test_should_count_scoped_select_with_options
     Account.update_all("credit_limit = NULL")
-    Account.last.update_column('credit_limit', 49)
-    Account.first.update_column('credit_limit', 51)
+    Account.last.update_attribute('credit_limit', 49)
+    Account.first.update_attribute('credit_limit', 51)
 
     assert_equal 1, Account.scoped(:select => "credit_limit").count(:conditions => ['credit_limit >= 50'])
   end

activerecord/test/cases/dirty_test.rb

@@ -413,7 +413,7 @@ def test_save_should_not_save_serialized_attribute_with_partial_updates_if_not_p
     with_partial_updates(Topic) do
       Topic.create!(:author_name => 'Bill', :content => {:a => "a"})
       topic = Topic.select('id, author_name').first
-      topic.update_column :author_name, 'John'
+      topic.update_attribute :author_name, 'John'
       topic = Topic.first
       assert_not_nil topic.content
     end
@@ -487,8 +487,7 @@ def test_previous_changes
     assert !pirate.previous_changes.key?('created_on')
 
     pirate = Pirate.find_by_catchphrase("Ahoy!")
-    pirate.catchphrase = "Ninjas suck!"
-    pirate.save(:validations => false)
+    pirate.update_attribute(:catchphrase, "Ninjas suck!")
 
     assert_equal 2, pirate.previous_changes.size
     assert_equal ["Ahoy!", "Ninjas suck!"], pirate.previous_changes['catchphrase']

activerecord/test/cases/persistence_test.rb

@@ -327,62 +327,66 @@ def test_destroy_record_with_associations
     assert_raise(ActiveSupport::FrozenObjectError) { client.name = "something else" }
   end
 
-  def test_update_column
-    topic = Topic.find(1)
-    topic.update_column("approved", true)
-    assert topic.approved?
-    topic.reload
-    assert topic.approved?
+  def test_update_attribute
+    assert !Topic.find(1).approved?
+    Topic.find(1).update_attribute("approved", true)
+    assert Topic.find(1).approved?
 
-    topic.update_column(:approved, false)
-    assert !topic.approved?
-    topic.reload
-    assert !topic.approved?
+    Topic.find(1).update_attribute(:approved, false)
+    assert !Topic.find(1).approved?
   end
 
-  def test_update_column_with_model_having_primary_key_other_than_id
+  def test_update_attribute_for_readonly_attribute
     minivan = Minivan.find('m1')
-    new_name = 'sebavan'
-
-    minivan.update_column(:name, new_name)
-    assert_equal new_name, minivan.name
-  end
+    assert_raises(ActiveRecord::ActiveRecordError) { minivan.update_attribute(:color, 'black') }
+  end
+
+  # This test is correct, but it is hard to fix it since
+  # update_attribute trigger simply call save! that triggers
+  # all callbacks.
+  # def test_update_attribute_with_one_changed_and_one_updated
+  #   t = Topic.order('id').limit(1).first
+  #   title, author_name = t.title, t.author_name
+  #   t.author_name = 'John'
+  #   t.update_attribute(:title, 'super_title')
+  #   assert_equal 'John', t.author_name
+  #   assert_equal 'super_title', t.title
+  #   assert t.changed?, "topic should have changed"
+  #   assert t.author_name_changed?, "author_name should have changed"
+  #   assert !t.title_changed?, "title should not have changed"
+  #   assert_nil t.title_change, 'title change should be nil'
+  #   assert_equal ['author_name'], t.changed
+  #
+  #   t.reload
+  #   assert_equal 'David', t.author_name
+  #   assert_equal 'super_title', t.title
+  # end
+
+  def test_update_attribute_with_one_updated
+    t = Topic.first
+    title = t.title
+    t.update_attribute(:title, 'super_title')
+    assert_equal 'super_title', t.title
+    assert !t.changed?, "topic should not have changed"
+    assert !t.title_changed?, "title should not have changed"
+    assert_nil t.title_change, 'title change should be nil'
 
-  def test_update_column_for_readonly_attribute
-    minivan = Minivan.find('m1')
-    prev_color = minivan.color
-    assert_raises(ActiveRecord::ActiveRecordError) { minivan.update_column(:color, 'black') }
-    assert_equal prev_color, minivan.color
+    t.reload
+    assert_equal 'super_title', t.title
   end
 
-  def test_update_column_should_not_modify_updated_at
+  def test_update_attribute_for_updated_at_on
     developer = Developer.find(1)
     prev_month = Time.now.prev_month
 
-    developer.update_column(:updated_at, prev_month)
+    developer.update_attribute(:updated_at, prev_month)
     assert_equal prev_month, developer.updated_at
 
-    developer.update_column(:salary, 80001)
-    assert_equal prev_month, developer.updated_at
+    developer.update_attribute(:salary, 80001)
+    assert_not_equal prev_month, developer.updated_at
 
     developer.reload
-    assert_equal prev_month, developer.updated_at
-  end
-
-  def test_update_column_with_one_changed_and_one_updated
-    t = Topic.order('id').limit(1).first
-    title, author_name = t.title, t.author_name
-    t.author_name = 'John'
-    t.update_column(:title, 'super_title')
-    assert_equal 'John', t.author_name
-    assert_equal 'super_title', t.title
-    assert t.changed?, "topic should have changed"
-    assert t.author_name_changed?, "author_name should have changed"
-    assert t.title_changed?, "title should have changed"
-
-    t.reload
-    assert_equal author_name, t.author_name
-    assert_equal 'super_title', t.title
+    assert_not_equal prev_month, developer.updated_at
   end
 
   def test_update_attributes

activerecord/test/cases/timestamp_test.rb

@@ -113,8 +113,7 @@ def test_touching_a_record_with_a_belongs_to_that_uses_a_counter_cache_should_up
 
     pet = Pet.first
     owner = pet.owner
-    owner.happy_at = 3.days.ago
-    owner.save
+    owner.update_attribute(:happy_at, 3.days.ago)
     previously_owner_updated_at = owner.updated_at
 
     pet.name = "I'm a parrot"
@@ -132,9 +131,8 @@ def test_touching_a_record_touches_parent_record_and_grandparent_record
     toy = Toy.first
     pet = toy.pet
     owner = pet.owner
-    time = 3.days.ago
 
-    owner.update_column(:updated_at, time)
+    owner.update_attribute(:updated_at, (time = 3.days.ago))
     toy.touch
     owner.reload