Skip to content

Commit

Permalink
Avoid no-op database query in Model::destroy() with empty ids
Browse files Browse the repository at this point in the history
  • Loading branch information
spawnia committed Nov 19, 2020
1 parent 5ba7984 commit 17cf511
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 5 deletions.
15 changes: 10 additions & 5 deletions src/Illuminate/Database/Eloquent/Model.php
Expand Up @@ -1061,22 +1061,27 @@ protected function insertAndSetId(Builder $query, $attributes)
*/
public static function destroy($ids)
{
// We'll initialize a count here so we will return the total number of deletes
// for the operation. The developers can then check this number as a boolean
// type value or get this total count of records deleted for logging, etc.
$count = 0;

if ($ids instanceof BaseCollection) {
$ids = $ids->all();
}

$ids = is_array($ids) ? $ids : func_get_args();

// Avoid a no-op database query by bailing early
if (count($ids) === 0) {
return 0;
}

// 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();

// We'll initialize a count here so we will return the total number of deletes
// for the operation. The developers can then check this number as a boolean
// type value or get this total count of records deleted for logging, etc.
$count = 0;

foreach ($instance->whereIn($key, $ids)->get() as $model) {
if ($model->delete()) {
$count++;
Expand Down
21 changes: 21 additions & 0 deletions tests/Database/DatabaseEloquentModelTest.php
Expand Up @@ -293,6 +293,16 @@ public function testDestroyMethodCallsQueryBuilderCorrectlyWithCollection()
EloquentModelDestroyStub::destroy(new Collection([1, 2, 3]));
}

public function testDestroyMethodCallsQueryBuilderCorrectlyWithMultipleArgs()
{
EloquentModelDestroyStub::destroy(1, 2, 3);
}

public function testDestroyMethodCallsQueryBuilderCorrectlyWithEmptyIds()
{
EloquentModelEmptyDestroyStub::destroy([]);
}

public function testWithMethodCallsQueryBuilderCorrectly()
{
$result = EloquentModelWithStub::with('foo', 'bar');
Expand Down Expand Up @@ -2400,6 +2410,17 @@ public function newQuery()
}
}

class EloquentModelEmptyDestroyStub extends Model
{
public function newQuery()
{
$mock = m::mock(Builder::class);
$mock->shouldReceive('whereIn')->never();

return $mock;
}
}

class EloquentModelHydrateRawStub extends Model
{
public static function hydrate(array $items, $connection = null)
Expand Down

0 comments on commit 17cf511

Please sign in to comment.