Permalink
Browse files

Merge branch 'update_columns'

  • Loading branch information...
2 parents 2d6af1d + 2d9e5a2 commit 864b49d42f887b2453a3d90807569c8ca73af807 @rafaelfranca rafaelfranca committed Jul 24, 2012
@@ -1,5 +1,17 @@
## Rails 4.0.0 (unreleased) ##
+* Deprecate `update_column` method in favor of `update_columns`.
+
+ *Rafael Mendonça França*
+
+* Added an `update_columns` method. This new method updates the given attributes on an object,
+ without calling save, hence skipping validations and callbacks.
+ Example:
+
+ User.first.update_columns({:name => "sebastian", :age => 25}) # => true
@fxn
fxn Jul 30, 2012 Ruby on Rails member

Stylistic point seen on passing, curlies in rightmost arguments are not normally used.

+
+ *Sebastian Martinez + Rafael Mendonça França*
+
* Removed `:finder_sql` and `:counter_sql` collection association options. Please
use scopes instead.
@@ -36,7 +36,7 @@ def delete(method = options[:dependent])
when :destroy
target.destroy
when :nullify
- target.update_column(reflection.foreign_key, nil)
+ target.update_columns(reflection.foreign_key => nil)
end
end
end
@@ -167,22 +167,6 @@ def becomes(klass)
became
end
- # Updates a single attribute of an object, without calling save.
- #
- # * Validation is skipped.
- # * Callbacks are skipped.
- # * updated_at/updated_on column is not updated if that column is available.
- #
- # Raises an +ActiveRecordError+ when called on new objects, or when the +name+
- # attribute is marked as readonly.
- def update_column(name, value)
- name = name.to_s
- verify_readonly_attribute(name)
- raise ActiveRecordError, "can not update on a new record object" unless persisted?
- raw_write_attribute(name, value)
- self.class.where(self.class.primary_key => id).update_all(name => value) == 1
- end
-
# Updates the attributes of the model from the passed-in hash and saves the
# record, all wrapped in a transaction. If the object is invalid, the saving
# will fail and false will be returned.
@@ -211,6 +195,45 @@ def update_attributes!(attributes, options = {})
end
end
+ # Updates a single attribute of an object, without calling save.
+ #
+ # * Validation is skipped.
+ # * Callbacks are skipped.
+ # * updated_at/updated_on column is not updated if that column is available.
+ #
+ # Raises an +ActiveRecordError+ when called on new objects, or when the +name+
+ # attribute is marked as readonly.
+ def update_column(name, value)
+ msg = "update_column is deprecated and will be removed in 4.1. Please use update_columns. " \
+ "E.g. update_columns(foo: 'bar')"
+
+ ActiveSupport::Deprecation.warn(msg)
+
+ update_columns(name => value)
+ end
+
+ # Updates the attributes from the passed-in hash, without calling save.
+ #
+ # * Validation is skipped.
+ # * Callbacks are skipped.
+ # * updated_at/updated_on column is not updated if that column is available.
+ #
+ # Raises an +ActiveRecordError+ when called on new objects, or when at least
+ # one of the attributes is marked as readonly.
+ def update_columns(attributes)
+ raise ActiveRecordError, "can not update on a new record object" unless persisted?
+
+ attributes.each_key do |key|
+ raise ActiveRecordError, "#{key.to_s} is marked as readonly" if self.class.readonly_attributes.include?(key.to_s)
+ end
+
+ attributes.each do |k,v|
+ raw_write_attribute(k,v)
+ end
+
+ self.class.where(self.class.primary_key => id).update_all(attributes) == 1
+ end
+
# Initializes +attribute+ to zero if +nil+ and adds the value passed as +by+ (default is 1).
# The increment is performed directly on the underlying attribute, no setter is invoked.
# Only makes sense for number-based attributes. Returns +self+.
@@ -225,7 +248,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_columns(attribute => self[attribute])
end
# Initializes +attribute+ to zero if +nil+ and subtracts the value passed as +by+ (default is 1).
@@ -242,7 +265,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_columns(attribute => self[attribute])
end
# Assigns to +attribute+ the boolean opposite of <tt>attribute?</tt>. So
@@ -259,7 +282,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_columns(attribute => self[attribute])
end
# Reloads the attributes of this object from the database.
@@ -298,12 +298,12 @@ def test_belongs_to_counter_after_update_attributes
assert_equal 1, Topic.find(topic.id)[:replies_count]
end
- def test_belongs_to_counter_when_update_column
+ def test_belongs_to_counter_when_update_columns
topic = Topic.create!(:title => "37s")
topic.replies.create!(:title => "re: 37s", :content => "rails")
assert_equal 1, Topic.find(topic.id)[:replies_count]
- topic.update_column(:content, "rails is wonderfull")
+ topic.update_columns(content: "rails is wonderfull")
assert_equal 1, Topic.find(topic.id)[:replies_count]
end
@@ -582,7 +582,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_columns("name" => "Bruza")
assert_equal 1, Developer.connection.select_value(<<-end_sql).to_i
SELECT count(*) FROM developers_projects
WHERE project_id = #{project.id}
@@ -634,7 +634,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_columns(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)
@@ -643,7 +643,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_columns(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)
@@ -813,7 +813,7 @@ def test_delete_all_association_with_primary_key_deletes_correct_records
firm = Firm.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_columns(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
@@ -327,7 +327,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_columns(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)
@@ -337,7 +337,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_columns(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
@@ -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.scoped(:includes => :favourite_club).find(@member.id).favourite_club
- memberships(:membership_of_favourite_club).update_column(:favourite, false)
+ memberships(:membership_of_favourite_club).update_columns(favourite: false)
assert_equal nil, Member.scoped(:includes => :favourite_club).find(@member.id).reload.favourite_club
# conditions on the source table
assert_equal clubs(:moustache_club), Member.scoped(:includes => :hairy_club).find(@member.id).hairy_club
- clubs(:moustache_club).update_column(:name, "Association of Clean-Shaven Persons")
+ clubs(:moustache_club).update_columns(name: "Association of Clean-Shaven Persons")
assert_equal nil, Member.scoped(:includes => :hairy_club).find(@member.id).reload.hairy_club
end
@@ -175,7 +175,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_columns taggable_type: 'PostWithHasManyDeleteAll'
post = find_post_with_dependency(1, :has_many, :taggings, :delete_all)
old_count = Tagging.count
@@ -186,7 +186,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_columns taggable_type: 'PostWithHasManyDestroy'
post = find_post_with_dependency(1, :has_many, :taggings, :destroy)
old_count = Tagging.count
@@ -197,7 +197,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_columns taggable_type: 'PostWithHasManyNullify'
post = find_post_with_dependency(1, :has_many, :taggings, :nullify)
old_count = Tagging.count
@@ -208,7 +208,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_columns taggable_type: 'PostWithHasOneDestroy'
post = find_post_with_dependency(1, :has_one, :tagging, :destroy)
old_count = Tagging.count
@@ -219,7 +219,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_columns taggable_type: 'PostWithHasOneNullify'
post = find_post_with_dependency(1, :has_one, :tagging, :nullify)
old_count = Tagging.count
@@ -734,7 +734,7 @@ def test_has_many_with_pluralize_table_names_false
# 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_columns type: class_name
klass = Object.const_set(class_name, Class.new(ActiveRecord::Base))
klass.table_name = 'posts'
klass.send(association, association_name, :as => :taggable, :dependent => dependency)
@@ -67,7 +67,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_columns(name: 'Deck')
ship.parts.send(:load_target)
assert_equal 'Deck', ship.parts[0].name
end
@@ -176,7 +176,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_columns(created_at: Time.now)
assert !david.projects.loaded?
end
@@ -609,7 +609,7 @@ def test_non_valid_identifier_column_name
assert_equal 'value', weird.send('a$b')
assert_equal 'value', weird.read_attribute('a$b')
- weird.update_column('a$b', 'value2')
+ weird.update_columns('a$b' => 'value2')
weird.reload
assert_equal 'value2', weird.send('a$b')
assert_equal 'value2', weird.read_attribute('a$b')
@@ -1896,7 +1896,7 @@ def test_cache_key_format_for_existing_record_with_updated_at
def test_cache_key_format_for_existing_record_with_nil_updated_at
dev = Developer.first
- dev.update_column(:updated_at, nil)
+ dev.update_columns(updated_at: nil)
assert_match(/\/#{dev.id}$/, dev.cache_key)
end
@@ -331,8 +331,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_columns('credit_limit' => 49)
+ Account.first.update_columns('credit_limit' => 51)
assert_equal 1, Account.scoped(:select => "credit_limit").where('credit_limit >= 50').count
end
@@ -424,7 +424,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_columns author_name: 'John'
topic = Topic.first
assert_not_nil topic.content
end
Oops, something went wrong.

6 comments on commit 864b49d

@josevalim
Member

There is a deprecation warning in 3.2.7 asking people to use update_column instead of update_attribute. It is not acceptable for us to ask users to change it again on Rails 4. We need to either add update_columns to 3.2.7 and update the warning or keep update_column on master.

@josevalim
Member

Given that 3.2.7 is already out, I would even advocate to not deprecate this.

@steveklabnik
Member

IIRC, the warning message has already been changed to say update_columns, and 3.2.8 (which is coming soonish, right @tenderlove?) will have the fixed message, so it's only for this one minor revision.

@rafaelfranca
Member

I already reverted the deprecation. I don't think that we should suggest the people to change again. Lets wait to deprecate and remove update_column in a next major release.

@steveklabnik
Member

Is it better to have them change twice over the course of a year, or to just say "hey, we screwed up, if you just changed this, please change it again right now?" After all, if they just modified it, the work is fresh in their minds, so it should be much easier to change again. If we wait a long time, more update_columns will just creep into codebases.

@rafaelfranca
Member

So we just don't remove it. The implementation is very simple, doesn't hurt to maintain and is using update_columns. I really like to remove it to make the API cleaner, but I don't want to do this in a minor release and see the people angry because they changed to update_column and have to do again using update_columns.

Anyway I don't think that here is the best place to discuss this.

Please sign in to comment.