diff --git a/system/BaseModel.php b/system/BaseModel.php index 8e81863adfb9..c225eb97be2e 100644 --- a/system/BaseModel.php +++ b/system/BaseModel.php @@ -1104,7 +1104,7 @@ public function delete($id = null, bool $purge = false) throw new InvalidArgumentException('delete(): argument #1 ($id) should not be boolean.'); } - if (! in_array($id, [null, 0, '0'], true) && (is_numeric($id) || is_string($id))) { + if (is_numeric($id) || is_string($id)) { $id = [$id]; } diff --git a/system/Model.php b/system/Model.php index 60759b9e201f..d02e17439f84 100644 --- a/system/Model.php +++ b/system/Model.php @@ -443,7 +443,7 @@ protected function doUpdate($id = null, $row = null): bool $builder = $this->builder(); - if (! in_array($id, [null, '', 0, '0', []], true)) { + if ($this->isValidID($id)) { $builder = $builder->whereIn($this->table . '.' . $this->primaryKey, $id); } @@ -461,6 +461,25 @@ protected function doUpdate($id = null, $row = null): bool return $builder->update(); } + /** + * Make sure that the primary keys are set and non-empty + * for $this->delete() and $this->update(). + * + * @param int|list|string|null $id + */ + protected function isValidID($id): bool + { + if (is_array($id) && $id !== []) { + foreach ($id as $valueId) { + if (is_array($valueId) || ! $this->isValidID($valueId)) { + return false; + } + } + } + + return ! in_array($id, [null, '', 0, '0', []], true); + } + /** * Compiles an update string and runs the query * This method works only with dbCalls. @@ -496,7 +515,7 @@ protected function doDelete($id = null, bool $purge = false) $set = []; $builder = $this->builder(); - if (! in_array($id, [null, '', 0, '0', []], true)) { + if ($this->isValidID($id)) { $builder = $builder->whereIn($this->primaryKey, $id); } diff --git a/tests/system/Models/DeleteModelTest.php b/tests/system/Models/DeleteModelTest.php index 0294ab32aed9..84ca922ffb83 100644 --- a/tests/system/Models/DeleteModelTest.php +++ b/tests/system/Models/DeleteModelTest.php @@ -273,6 +273,10 @@ public static function emptyPkValues(): iterable [0], [null], ['0'], + // @todo Fail only testDontThrowExceptionWhenSoftDeleteConditionIsSetWithEmptyValue() + // [''], + // [[]], + // [[15 => 150, '_id_' => '200', 20 => '0']], ]; } } diff --git a/tests/system/Models/UpdateModelTest.php b/tests/system/Models/UpdateModelTest.php index 1dc7249c3efd..b9b139861917 100644 --- a/tests/system/Models/UpdateModelTest.php +++ b/tests/system/Models/UpdateModelTest.php @@ -553,10 +553,14 @@ public function testUpdateWithSetAndEscape(): void * @param false|null $id */ #[DataProvider('provideUpdateThrowDatabaseExceptionWithoutWhereClause')] - public function testUpdateThrowDatabaseExceptionWithoutWhereClause($id, string $exception, string $exceptionMessage): void + public function testUpdateThrowDatabaseExceptionWithoutWhereClause($id, string $exception): void { $this->expectException($exception); - $this->expectExceptionMessage($exceptionMessage); + $this->expectExceptionMessage( + $exception === DatabaseException::class + ? 'Updates are not allowed unless they contain a "where" or "like" clause.' + : 'update(): argument #1 ($id) should not be boolean.', + ); // $useSoftDeletes = false $this->createModel(JobModel::class); @@ -570,12 +574,34 @@ public static function provideUpdateThrowDatabaseExceptionWithoutWhereClause(): [ null, DatabaseException::class, - 'Updates are not allowed unless they contain a "where" or "like" clause.', ], [ false, InvalidArgumentException::class, - 'update(): argument #1 ($id) should not be boolean.', + ], + [ + '', + DatabaseException::class, + ], + [ + 0, + DatabaseException::class, + ], + [ + '0', + DatabaseException::class, + ], + [ + [], + DatabaseException::class, + ], + [ + [15 => 150, '_id_' => '200', 20 => '0'], + DatabaseException::class, + ], + [ + [0 => '150', [1 => 200]], + DatabaseException::class, ], ]; }