refactor: Add new hasRationalPrimaryKey()

Author: neznaika0

system/BaseModel.php

@@ -759,7 +759,7 @@ public function save($row): bool
 
     /**
      * This method is called on save to determine if entry have to be updated.
-     * If this method returns false insert operation will be executed
+     * If this method returns `false` insert operation will be executed.
      *
      * @param object|row_array $row
      */
@@ -966,11 +966,7 @@ public function insertBatch(?array $set = null, ?bool $escape = null, int $batch
      */
     public function update($id = null, $row = null): bool
     {
-        if (is_bool($id)) {
-            throw new InvalidArgumentException('update(): argument #1 ($id) should not be boolean.');
-        }
-
-        if (is_numeric($id) || is_string($id)) {
+        if ($this->hasRationalPrimaryKey($id)) {
             $id = [$id];
         }
 
@@ -1097,11 +1093,7 @@ public function updateBatch(?array $set = null, ?string $index = null, int $batc
      */
     public function delete($id = null, bool $purge = false)
     {
-        if (is_bool($id)) {
-            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 ($this->hasRationalPrimaryKey($id)) {
             $id = [$id];
         }
 
@@ -1896,4 +1888,24 @@ protected function convertToReturnType(array $row, string $returnType): array|ob
 
         return $this->converter->reconstruct($returnType, $row);
     }
+
+    /**
+     * Checking that the ID has a rational value.
+     *
+     * Standard type checks in PHP do not fully verify the validity of ID as a primary key.
+     * Especially the values 0, '0', '' are rarely used or are invalid.
+     * Example, if you add an entry with ID=0, it will be an error for `$this->insertID`.
+     *
+     * @param array<int|string, int|string>|float|int|string|null $id
+     *
+     * @throws InvalidArgumentException
+     */
+    protected function hasRationalPrimaryKey($id): bool
+    {
+        if (is_bool($id)) {
+            throw new InvalidArgumentException('The ID value should not be boolean.');
+        }
+
+        return ! in_array($id, [0, '0', '', 0.0], true) && (is_numeric($id) || is_string($id));
+    }
 }

system/Model.php

@@ -381,7 +381,7 @@ protected function doUpdate($id = null, $row = null): bool
 
         $builder = $this->builder();
 
-        if (! in_array($id, [null, '', 0, '0', []], true)) {
+        if ((is_array($id) && $id !== []) || $this->hasRationalPrimaryKey($id)) {
             $builder = $builder->whereIn($this->table . '.' . $this->primaryKey, $id);
         }
 
@@ -409,7 +409,7 @@ protected function doDelete($id = null, bool $purge = false)
         $set     = [];
         $builder = $this->builder();
 
-        if (! in_array($id, [null, '', 0, '0', []], true)) {
+        if ((is_array($id) && $id !== []) || $this->hasRationalPrimaryKey($id)) {
             $builder = $builder->whereIn($this->primaryKey, $id);
         }
 

tests/system/Models/MiscellaneousModelTest.php

@@ -14,7 +14,9 @@
 namespace CodeIgniter\Models;
 
 use CodeIgniter\Database\Exceptions\DataException;
+use CodeIgniter\Exceptions\InvalidArgumentException;
 use CodeIgniter\I18n\Time;
+use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\Attributes\Group;
 use Tests\Support\Models\EntityModel;
 use Tests\Support\Models\JobModel;
@@ -122,4 +124,46 @@ public function testEmptyDataInTransformDataToArray(): void
         $method = self::getPrivateMethodInvoker($this->model, 'transformDataToArray');
         $method([], 'insert');
     }
+
+    public function testHasRationalPrimaryKeyWithBoolean(): void
+    {
+        $this->expectException(InvalidArgumentException::class);
+        $this->expectExceptionMessage('The ID value should not be boolean.');
+
+        $this->createModel(JobModel::class);
+        $method = self::getPrivateMethodInvoker($this->model, 'hasRationalPrimaryKey');
+        $method(true); // @phpstan-ignore argument.type
+    }
+
+    /**
+     * @param float|int|string|null $value
+     */
+    #[DataProvider('provideHasRationalPrimaryKey')]
+    public function testHasRationalPrimaryKey($value, bool $expected): void
+    {
+        $this->createModel(JobModel::class);
+        $method = self::getPrivateMethodInvoker($this->model, 'hasRationalPrimaryKey');
+        $this->assertSame($method($value), $expected);
+    }
+
+    /**
+     * @return list<list<bool|float|int|string|null>>
+     */
+    public static function provideHasRationalPrimaryKey(): iterable
+    {
+        return [
+            [0, false],
+            ['0', false],
+            [0.0, false],
+            ['', false],
+            [null, false],
+            ['001', true],
+            [' ', true],
+            [' 0 ', true],
+            [1, true],
+            ['1', true],
+            ['00000000-0000-0000-0000-000000000000', true],
+            ['codeigniter', true],
+        ];
+    }
 }

tests/system/Models/UpdateModelTest.php

@@ -581,7 +581,7 @@ public static function provideUpdateThrowDatabaseExceptionWithoutWhereClause():
             [
                 false,
                 InvalidArgumentException::class,
-                'update(): argument #1 ($id) should not be boolean.',
+                'The ID value should not be boolean.',
             ],
         ];
     }