fix: Not allowed empty PK value for update/delete

Author: neznaika0

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];
         }
 

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<mixed>|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);
         }
 

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']],
         ];
     }
 }

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,
             ],
         ];
     }