Optimize destroy method

laravel · Jan 18, 2023 · 21e985d · 21e985d
1 parent 7906436
commit 21e985d
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 6 deletions.
diff --git a/src/Illuminate/Database/Eloquent/Model.php b/src/Illuminate/Database/Eloquent/Model.php
@@ -1347,11 +1347,9 @@ public static function destroy($ids)
         // We will actually pull the models from the database table and call delete on
         // each of them individually so that their events get fired properly with a
         // correct set of attributes in case the developers wants to check these.
-        $key = ($instance = new static)->getKeyName();
-
         $count = 0;
 
-        foreach ($instance->whereIn($key, $ids)->get() as $model) {
+        foreach ((new static)->whereKey($ids)->get() as $model) {
             if ($model->delete()) {
                 $count++;
             }

diff --git a/tests/Database/DatabaseEloquentModelTest.php b/tests/Database/DatabaseEloquentModelTest.php
@@ -2823,7 +2823,7 @@ class EloquentModelDestroyStub extends Model
     public function newQuery()
     {
         $mock = m::mock(Builder::class);
-        $mock->shouldReceive('whereIn')->once()->with('id', [1, 2, 3])->andReturn($mock);
+        $mock->shouldReceive('whereKey')->once()->with([1, 2, 3])->andReturn($mock);
         $mock->shouldReceive('get')->once()->andReturn([$model = m::mock(stdClass::class)]);
         $model->shouldReceive('delete')->once();
 

diff --git a/tests/Integration/Database/EloquentDeleteTest.php b/tests/Integration/Database/EloquentDeleteTest.php
@@ -110,18 +110,31 @@ public function testDestroy()
         PostStringyKey::deleting(fn ($model) => $_SERVER['destroy']['deleting'][] = $model->my_id);
         PostStringyKey::deleted(fn ($model) => $_SERVER['destroy']['deleted'][] = $model->my_id);
 
+        // In case 0 ids are matched out of 2:
+        PostStringyKey::query()->getConnection()->flushQueryLog();
         $_SERVER['destroy'] = [];
-        PostStringyKey::destroy(1, 2, 3, 4);
+        $count = PostStringyKey::destroy(33, 44);
+        $this->assertEquals(0, $count);
+        $logs = PostStringyKey::query()->getConnection()->getQueryLog();
+        $this->assertCount(1, $logs);
+        $this->assertEmpty($_SERVER['destroy']);
+
+        // In case 2 ids are matched out of 4:
+        PostStringyKey::query()->getConnection()->flushQueryLog();
+        $_SERVER['destroy'] = [];
+        $count = PostStringyKey::destroy(1, 2, 3, 4);
 
         $this->assertEquals([1, 2], $_SERVER['destroy']['retrieved']);
         $this->assertEquals([1, 2], $_SERVER['destroy']['deleting']);
         $this->assertEquals([1, 2], $_SERVER['destroy']['deleted']);
 
+        $this->assertEquals(2, $count);
+
         $logs = PostStringyKey::query()->getConnection()->getQueryLog();
 
         $this->assertEquals(0, PostStringyKey::query()->count());
 
-        $this->assertStringStartsWith('select * from "my_posts" where "my_id" in (', str_replace(['`', '[', ']'], '"', $logs[0]['query']));
+        $this->assertStringStartsWith('select * from "my_posts" where "my_posts"."my_id" in (', str_replace(['`', '[', ']'], '"', $logs[0]['query']));
 
         $this->assertStringStartsWith('delete from "my_posts" where "my_id" = ', str_replace(['`', '[', ']'], '"', $logs[1]['query']));
         $this->assertEquals([1], $logs[1]['bindings']);
@@ -132,6 +145,16 @@ public function testDestroy()
         // Total of 3 queries.
         $this->assertCount(3, $logs);
 
+        PostStringyKey::query()->getConnection()->flushQueryLog();
+        $_SERVER['destroy'] = [];
+        $count = PostStringyKey::destroy([]);
+        $logs = PostStringyKey::query()->getConnection()->getQueryLog();
+
+        // no queries, no model events:
+        $this->assertEmpty($logs);
+        $this->assertEmpty($_SERVER['destroy']);
+        $this->assertEquals(0, $count);
+
         PostStringyKey::reguard();
         unset($_SERVER['destroy']);
         Schema::drop('my_posts');