Ensure returning affected objects for class level `update` and `destroy`

Class level `update` and `destroy` are using `find` in the internal, so
it will raise `RecordNotFound` if given ids cannot find an object even
though the method already affect (update or destroy) to any objects.
These methods should return affected objects even in that case.

2 files changed, 14 insertions, 12 deletions

diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb
index 4a5ccdc597..d1cb3783e7 100644
--- a/activerecord/lib/active_record/persistence.rb
+++ b/activerecord/lib/active_record/persistence.rb
@@ -99,7 +99,7 @@ module ActiveRecord
       # for updating all records in a single query.
       def update(id = :all, attributes)
         if id.is_a?(Array)
-          id.map.with_index { |one_id, idx| update(one_id, attributes[idx]) }
+          id.map.with_index { |one_id, idx| update(one_id, attributes[idx]) }.compact
         elsif id == :all
           all.each { |record| record.update(attributes) }
         else
@@ -112,6 +112,7 @@ module ActiveRecord
           object.update(attributes)
           object
         end
+      rescue RecordNotFound
       end
 
       # Destroy an object (or multiple objects) that has the given id. The object is instantiated first,
@@ -135,10 +136,11 @@ module ActiveRecord
       #   Todo.destroy(todos)
       def destroy(id)
         if id.is_a?(Array)
-          id.map { |one_id| destroy(one_id) }
+          id.map { |one_id| destroy(one_id) }.compact
         else
           find(id).destroy
         end
+      rescue RecordNotFound
       end
 
       # Deletes the row with a primary key matching the +id+ argument, using a
diff --git a/activerecord/test/cases/persistence_test.rb b/activerecord/test/cases/persistence_test.rb
index 170fd02b6f..6cbe18cc8c 100644
--- a/activerecord/test/cases/persistence_test.rb
+++ b/activerecord/test/cases/persistence_test.rb
@@ -70,10 +70,10 @@ class PersistenceTest < ActiveRecord::TestCase
   end
 
   def test_update_many
-    topic_data = { 1 => { "content" => "1 updated" }, 2 => { "content" => "2 updated" } }
+    topic_data = { 1 => { "content" => "1 updated" }, 2 => { "content" => "2 updated" }, nil => {} }
     updated = Topic.update(topic_data.keys, topic_data.values)
 
-    assert_equal 2, updated.size
+    assert_equal [1, 2], updated.map(&:id)
     assert_equal "1 updated", Topic.find(1).content
     assert_equal "2 updated", Topic.find(2).content
   end
@@ -81,9 +81,8 @@ class PersistenceTest < ActiveRecord::TestCase
   def test_class_level_update_is_affected_by_scoping
     topic_data = { 1 => { "content" => "1 updated" }, 2 => { "content" => "2 updated" } }
 
-    assert_raise(ActiveRecord::RecordNotFound) do
-      Topic.where("1=0").scoping { Topic.update(topic_data.keys, topic_data.values) }
-    end
+    assert_equal [], Topic.where("1=0").scoping { Topic.update(topic_data.keys, topic_data.values) }
+
     assert_not_equal "1 updated", Topic.find(1).content
     assert_not_equal "2 updated", Topic.find(2).content
   end
@@ -175,7 +174,7 @@ class PersistenceTest < ActiveRecord::TestCase
     clients = Client.all.merge!(order: "id").find([2, 3])
 
     assert_difference("Client.count", -2) do
-      destroyed = Client.destroy([2, 3]).sort_by(&:id)
+      destroyed = Client.destroy([2, 3, nil]).sort_by(&:id)
       assert_equal clients, destroyed
       assert destroyed.all?(&:frozen?), "destroyed clients should be frozen"
     end
@@ -917,7 +916,9 @@ class PersistenceTest < ActiveRecord::TestCase
     should_be_destroyed_reply = Reply.create("title" => "hello", "content" => "world")
     Topic.find(1).replies << should_be_destroyed_reply
 
-    Topic.destroy(1)
+    topic = Topic.destroy(1)
+    assert topic.destroyed?
+
     assert_raise(ActiveRecord::RecordNotFound) { Topic.find(1) }
     assert_raise(ActiveRecord::RecordNotFound) { Reply.find(should_be_destroyed_reply.id) }
   end
@@ -926,9 +927,8 @@ class PersistenceTest < ActiveRecord::TestCase
     should_not_be_destroyed_reply = Reply.create("title" => "hello", "content" => "world")
     Topic.find(1).replies << should_not_be_destroyed_reply
 
-    assert_raise(ActiveRecord::RecordNotFound) do
-      Topic.where("1=0").scoping { Topic.destroy(1) }
-    end
+    assert_nil Topic.where("1=0").scoping { Topic.destroy(1) }
+
     assert_nothing_raised { Topic.find(1) }
     assert_nothing_raised { Reply.find(should_not_be_destroyed_reply.id) }
   end