diff options
17 files changed, 44 insertions, 62 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index bbb0ec575e..a7b75456a2 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,4 +1,7 @@ ## Rails 6.0.0.alpha (Unreleased) ## +* Deprecate `update_attributes`/`!` in favor of `update`/`!`. + + *Eddie Lebow* * Add ActiveRecord::Base.create_or_find_by/! to deal with the SELECT/INSERT race condition in ActiveRecord::Base.find_or_create_by/! by leaning on unique constraints in the database. diff --git a/activerecord/lib/active_record/internal_metadata.rb b/activerecord/lib/active_record/internal_metadata.rb index 5a65edf27e..3626a13d7c 100644 --- a/activerecord/lib/active_record/internal_metadata.rb +++ b/activerecord/lib/active_record/internal_metadata.rb @@ -17,7 +17,7 @@ module ActiveRecord end def []=(key, value) - find_or_initialize_by(key: key).update_attributes!(value: value) + find_or_initialize_by(key: key).update!(value: value) end def [](key) diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index a45d011d75..88b971327b 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -418,6 +418,7 @@ module ActiveRecord end alias update_attributes update + deprecate :update_attributes # Updates its receiver just like #update but calls #save! instead # of +save+, so an exception is raised if the record is invalid and saving will fail. @@ -431,6 +432,7 @@ module ActiveRecord end alias update_attributes! update! + deprecate :update_attributes! # Equivalent to <code>update_columns(name => value)</code>. def update_column(name, value) diff --git a/activerecord/test/cases/adapter_test.rb b/activerecord/test/cases/adapter_test.rb index 0092146e09..4c41a68407 100644 --- a/activerecord/test/cases/adapter_test.rb +++ b/activerecord/test/cases/adapter_test.rb @@ -20,7 +20,7 @@ module ActiveRecord b = Book.create(name: "my \x00 book") b.reload assert_equal "my \x00 book", b.name - b.update_attributes(name: "my other \x00 book") + b.update(name: "my other \x00 book") b.reload assert_equal "my other \x00 book", b.name end diff --git a/activerecord/test/cases/adapters/mysql2/transaction_test.rb b/activerecord/test/cases/adapters/mysql2/transaction_test.rb index f921515c10..30455fbde5 100644 --- a/activerecord/test/cases/adapters/mysql2/transaction_test.rb +++ b/activerecord/test/cases/adapters/mysql2/transaction_test.rb @@ -46,7 +46,7 @@ module ActiveRecord Sample.transaction do s1.lock! barrier.wait - s2.update_attributes value: 1 + s2.update value: 1 end end @@ -54,7 +54,7 @@ module ActiveRecord Sample.transaction do s2.lock! barrier.wait - s1.update_attributes value: 2 + s1.update value: 2 end ensure thread.join diff --git a/activerecord/test/cases/adapters/postgresql/transaction_test.rb b/activerecord/test/cases/adapters/postgresql/transaction_test.rb index 9821b103df..8e952a9408 100644 --- a/activerecord/test/cases/adapters/postgresql/transaction_test.rb +++ b/activerecord/test/cases/adapters/postgresql/transaction_test.rb @@ -76,7 +76,7 @@ module ActiveRecord Sample.transaction do s1.lock! barrier.wait - s2.update_attributes value: 1 + s2.update value: 1 end end @@ -84,7 +84,7 @@ module ActiveRecord Sample.transaction do s2.lock! barrier.wait - s1.update_attributes value: 2 + s1.update value: 2 end ensure thread.join diff --git a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb index d9e05cf7e2..5f771fe85f 100644 --- a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb @@ -662,7 +662,7 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase assert_includes developer.sym_special_projects, sp end - def test_update_attributes_after_push_without_duplicate_join_table_rows + def test_update_columns_after_push_without_duplicate_join_table_rows developer = Developer.new("name" => "Kano") project = SpecialProject.create("name" => "Special Project") assert developer.save diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 9ee42cef0f..72fae0fe84 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -1211,20 +1211,20 @@ class HasManyAssociationsTest < ActiveRecord::TestCase end end - def test_calling_update_attributes_on_id_changes_the_counter_cache + def test_calling_update_on_id_changes_the_counter_cache topic = Topic.order("id ASC").first original_count = topic.replies.to_a.size assert_equal original_count, topic.replies_count first_reply = topic.replies.first - first_reply.update_attributes(parent_id: nil) + first_reply.update(parent_id: nil) assert_equal original_count - 1, topic.reload.replies_count - first_reply.update_attributes(parent_id: topic.id) + first_reply.update(parent_id: topic.id) assert_equal original_count, topic.reload.replies_count end - def test_calling_update_attributes_changing_ids_doesnt_change_counter_cache + def test_calling_update_changing_ids_doesnt_change_counter_cache topic1 = Topic.find(1) topic2 = Topic.find(3) original_count1 = topic1.replies.to_a.size @@ -1233,11 +1233,11 @@ class HasManyAssociationsTest < ActiveRecord::TestCase reply1 = topic1.replies.first reply2 = topic2.replies.first - reply1.update_attributes(parent_id: topic2.id) + reply1.update(parent_id: topic2.id) assert_equal original_count1 - 1, topic1.reload.replies_count assert_equal original_count2 + 1, topic2.reload.replies_count - reply2.update_attributes(parent_id: topic1.id) + reply2.update(parent_id: topic1.id) assert_equal original_count1, topic1.reload.replies_count assert_equal original_count2, topic2.reload.replies_count end diff --git a/activerecord/test/cases/autosave_association_test.rb b/activerecord/test/cases/autosave_association_test.rb index 09684520d1..b8243d148a 100644 --- a/activerecord/test/cases/autosave_association_test.rb +++ b/activerecord/test/cases/autosave_association_test.rb @@ -50,7 +50,7 @@ class TestAutosaveAssociationsInGeneral < ActiveRecord::TestCase } u = person.create!(first_name: "cool") - u.update_attributes!(first_name: "nah") # still valid because validation only applies on 'create' + u.update!(first_name: "nah") # still valid because validation only applies on 'create' assert_predicate reference.create!(person: u), :persisted? end diff --git a/activerecord/test/cases/batches_test.rb b/activerecord/test/cases/batches_test.rb index ad701fb200..c8163901c6 100644 --- a/activerecord/test/cases/batches_test.rb +++ b/activerecord/test/cases/batches_test.rb @@ -508,7 +508,7 @@ class EachTest < ActiveRecord::TestCase def test_in_batches_relations_update_all_should_not_affect_matching_records_in_other_batches Post.update_all(author_id: 0) person = Post.last - person.update_attributes(author_id: 1) + person.update(author_id: 1) Post.in_batches(of: 2) do |batch| batch.where("author_id >= 1").update_all("author_id = author_id + 1") diff --git a/activerecord/test/cases/json_shared_test_cases.rb b/activerecord/test/cases/json_shared_test_cases.rb index c60a276850..9b79803503 100644 --- a/activerecord/test/cases/json_shared_test_cases.rb +++ b/activerecord/test/cases/json_shared_test_cases.rb @@ -101,7 +101,7 @@ module JSONSharedTestCases x = klass.where(payload: nil).first assert_nil(x) - json.update_attributes(payload: nil) + json.update(payload: nil) x = klass.where(payload: nil).first assert_equal(json.reload, x) end diff --git a/activerecord/test/cases/persistence_test.rb b/activerecord/test/cases/persistence_test.rb index d4a4cd564a..e4a65a48ca 100644 --- a/activerecord/test/cases/persistence_test.rb +++ b/activerecord/test/cases/persistence_test.rb @@ -525,7 +525,7 @@ class PersistenceTest < ActiveRecord::TestCase def test_update_does_not_run_sql_if_record_has_not_changed topic = Topic.create(title: "Another New Topic") assert_queries(0) { assert topic.update(title: "Another New Topic") } - assert_queries(0) { assert topic.update_attributes(title: "Another New Topic") } + assert_queries(0) { assert topic.update(title: "Another New Topic") } end def test_delete @@ -934,42 +934,33 @@ class PersistenceTest < ActiveRecord::TestCase topic.reload assert_not_predicate topic, :approved? assert_equal "The First Topic", topic.title - end - - def test_update_attributes - topic = Topic.find(1) - assert_not_predicate topic, :approved? - assert_equal "The First Topic", topic.title - - topic.update_attributes("approved" => true, "title" => "The First Topic Updated") - topic.reload - assert_predicate topic, :approved? - assert_equal "The First Topic Updated", topic.title - - topic.update_attributes(approved: false, title: "The First Topic") - topic.reload - assert_not_predicate topic, :approved? - assert_equal "The First Topic", topic.title error = assert_raise(ActiveRecord::RecordNotUnique, ActiveRecord::StatementInvalid) do - topic.update_attributes(id: 3, title: "Hm is it possible?") + topic.update(id: 3, title: "Hm is it possible?") end assert_not_nil error.cause assert_not_equal "Hm is it possible?", Topic.find(3).title - topic.update_attributes(id: 1234) + topic.update(id: 1234) assert_nothing_raised { topic.reload } assert_equal topic.title, Topic.find(1234).title end - def test_update_attributes_parameters + def test_update_attributes + topic = Topic.find(1) + assert_deprecated do + topic.update_attributes("title" => "The First Topic Updated") + end + end + + def test_update_parameters topic = Topic.find(1) assert_nothing_raised do - topic.update_attributes({}) + topic.update({}) end assert_raises(ArgumentError) do - topic.update_attributes(nil) + topic.update(nil) end end @@ -995,24 +986,10 @@ class PersistenceTest < ActiveRecord::TestCase end def test_update_attributes! - Reply.validates_presence_of(:title) reply = Reply.find(2) - assert_equal "The Second Topic of the day", reply.title - assert_equal "Have a nice day", reply.content - - reply.update_attributes!("title" => "The Second Topic of the day updated", "content" => "Have a nice evening") - reply.reload - assert_equal "The Second Topic of the day updated", reply.title - assert_equal "Have a nice evening", reply.content - - reply.update_attributes!(title: "The Second Topic of the day", content: "Have a nice day") - reply.reload - assert_equal "The Second Topic of the day", reply.title - assert_equal "Have a nice day", reply.content - - assert_raise(ActiveRecord::RecordInvalid) { reply.update_attributes!(title: nil, content: "Have a nice evening") } - ensure - Reply.clear_validators! + assert_deprecated do + reply.update_attributes!("title" => "The Second Topic of the day updated") + end end def test_destroyed_returns_boolean diff --git a/activerecord/test/cases/query_cache_test.rb b/activerecord/test/cases/query_cache_test.rb index 8d37f5ea95..f63f6295d6 100644 --- a/activerecord/test/cases/query_cache_test.rb +++ b/activerecord/test/cases/query_cache_test.rb @@ -366,7 +366,7 @@ class QueryCacheTest < ActiveRecord::TestCase post = Post.first Post.transaction do - post.update_attributes(title: "rollback") + post.update(title: "rollback") assert_equal 1, Post.where(title: "rollback").to_a.count raise ActiveRecord::Rollback end @@ -379,7 +379,7 @@ class QueryCacheTest < ActiveRecord::TestCase begin Post.transaction do - post.update_attributes(title: "rollback") + post.update(title: "rollback") assert_equal 1, Post.where(title: "rollback").to_a.count raise "broken" end diff --git a/activerecord/test/cases/validations/length_validation_test.rb b/activerecord/test/cases/validations/length_validation_test.rb index 73422a31cd..62cd89041a 100644 --- a/activerecord/test/cases/validations/length_validation_test.rb +++ b/activerecord/test/cases/validations/length_validation_test.rb @@ -56,7 +56,7 @@ class LengthValidationTest < ActiveRecord::TestCase assert owner.save pet_count = Pet.count - assert_not owner.update_attributes pets_attributes: [ { _destroy: 1, id: pet.id } ] + assert_not owner.update pets_attributes: [ { _destroy: 1, id: pet.id } ] assert_not_predicate owner, :valid? assert_predicate owner.errors[:pets], :any? assert_equal pet_count, Pet.count diff --git a/activerecord/test/cases/validations/uniqueness_validation_test.rb b/activerecord/test/cases/validations/uniqueness_validation_test.rb index 1fa94a5b75..941aed5402 100644 --- a/activerecord/test/cases/validations/uniqueness_validation_test.rb +++ b/activerecord/test/cases/validations/uniqueness_validation_test.rb @@ -62,7 +62,7 @@ class TopicWithAfterCreate < Topic after_create :set_author def set_author - update_attributes!(author_name: "#{title} #{id}") + update!(author_name: "#{title} #{id}") end end diff --git a/activerecord/test/models/comment.rb b/activerecord/test/models/comment.rb index 5ab433f2d9..f0f0576709 100644 --- a/activerecord/test/models/comment.rb +++ b/activerecord/test/models/comment.rb @@ -76,7 +76,7 @@ class CommentThatAutomaticallyAltersPostBody < Comment belongs_to :post, class_name: "PostThatLoadsCommentsInAnAfterSaveHook", foreign_key: :post_id after_save do |comment| - comment.post.update_attributes(body: "Automatically altered") + comment.post.update(body: "Automatically altered") end end @@ -87,6 +87,6 @@ end class CommentWithAfterCreateUpdate < Comment after_create do - update_attributes(body: "bar") + update(body: "bar") end end diff --git a/guides/source/active_record_callbacks.md b/guides/source/active_record_callbacks.md index 630dafe632..4f54b4c206 100644 --- a/guides/source/active_record_callbacks.md +++ b/guides/source/active_record_callbacks.md @@ -264,7 +264,7 @@ The whole callback chain is wrapped in a transaction. If any callback raises an throw :abort ``` -WARNING. Any exception that is not `ActiveRecord::Rollback` or `ActiveRecord::RecordInvalid` will be re-raised by Rails after the callback chain is halted. Raising an exception other than `ActiveRecord::Rollback` or `ActiveRecord::RecordInvalid` may break code that does not expect methods like `save` and `update_attributes` (which normally try to return `true` or `false`) to raise an exception. +WARNING. Any exception that is not `ActiveRecord::Rollback` or `ActiveRecord::RecordInvalid` will be re-raised by Rails after the callback chain is halted. Raising an exception other than `ActiveRecord::Rollback` or `ActiveRecord::RecordInvalid` may break code that does not expect methods like `save` and `update` (which normally try to return `true` or `false`) to raise an exception. Relational Callbacks -------------------- |