fix: Model handling of Entity $primaryKey casting

phpstan-baseline.php

@@ -2608,7 +2608,7 @@
 ];
 $ignoreErrors[] = [
 	'message' => '#^Construct empty\\(\\) is not allowed\\. Use more strict comparison\\.$#',
-	'count' => 21,
+	'count' => 18,
 	'path' => __DIR__ . '/system/Model.php',
 ];
 $ignoreErrors[] = [

system/Model.php

@@ -534,6 +534,21 @@ protected function idValue($data)
     public function getIdValue($data)
     {
         if (is_object($data) && isset($data->{$this->primaryKey})) {
+            // Get the raw primary key value of the Entity.
+            if ($data instanceof Entity) {
+                $cast = $data->cast();
+
+                // Disable Entity casting, because raw primary key value is needed for database.
+                $data->cast(false);
+
+                $primaryKey = $data->{$this->primaryKey};
+
+                // Restore Entity casting setting.
+                $data->cast($cast);
+
+                return $primaryKey;
+            }
+
             return $data->{$this->primaryKey};
         }
 
@@ -796,37 +811,7 @@ public function update($id = null, $data = null): bool
      */
     protected function objectToRawArray($data, bool $onlyChanged = true, bool $recursive = false): ?array
     {
-        $properties = parent::objectToRawArray($data, $onlyChanged);
-
-        $primaryKey = null;
-
-        if ($data instanceof Entity) {
-            $cast = $data->cast();
-
-            // Disable Entity casting, because raw primary key data is needed for database.
-            $data->cast(false);
-
-            $primaryKey = $data->{$this->primaryKey};
-
-            // Restore Entity casting setting.
-            $data->cast($cast);
-        }
-
-        // Always grab the primary key otherwise updates will fail.
-        if (
-            // @TODO Should use `$data instanceof Entity`.
-            method_exists($data, 'toRawArray')
-            && (
-                ! empty($properties)
-                && ! empty($this->primaryKey)
-                && ! in_array($this->primaryKey, $properties, true)
-                && ! empty($primaryKey)
-            )
-        ) {
-            $properties[$this->primaryKey] = $primaryKey;
-        }
-
-        return $properties;
+        return parent::objectToRawArray($data, $onlyChanged);
     }
 
     /**

tests/_support/Entity/Cast/CastBinaryUUID.php

@@ -0,0 +1,43 @@
+<?php
+
+/**
+ * This file is part of CodeIgniter 4 framework.
+ *
+ * (c) CodeIgniter Foundation <[email protected]>
+ *
+ * For the full copyright and license information, please view
+ * the LICENSE file that was distributed with this source code.
+ */
+
+namespace Tests\Support\Entity\Cast;
+
+use CodeIgniter\Entity\Cast\BaseCast;
+
+class CastBinaryUUID extends BaseCast
+{
+    /**
+     * Get
+     *
+     * @param string $binary Binary UUID
+     *
+     * @return string String UUID
+     */
+    public static function get($binary, array $params = []): string
+    {
+        $string = unpack('h*', $binary);
+
+        return preg_replace('/([0-9a-f]{8})([0-9a-f]{4})([0-9a-f]{4})([0-9a-f]{4})([0-9a-f]{12})/', '$1-$2-$3-$4-$5', $string[1]);
+    }
+
+    /**
+     * Set
+     *
+     * @param string $string String UUID
+     *
+     * @return string Binary UUID
+     */
+    public static function set($string, array $params = []): string
+    {
+        return pack('h*', str_replace('-', '', $string));
+    }
+}

tests/_support/Entity/UUID.php

@@ -0,0 +1,25 @@
+<?php
+
+/**
+ * This file is part of CodeIgniter 4 framework.
+ *
+ * (c) CodeIgniter Foundation <[email protected]>
+ *
+ * For the full copyright and license information, please view
+ * the LICENSE file that was distributed with this source code.
+ */
+
+namespace Tests\Support\Entity;
+
+use CodeIgniter\Entity\Entity;
+use Tests\Support\Entity\Cast\CastBinaryUUID;
+
+class UUID extends Entity
+{
+    protected $casts = [
+        'id' => 'uuid',
+    ];
+    protected $castHandlers = [
+        'uuid' => CastBinaryUUID::class,
+    ];
+}

tests/_support/Models/UUIDPkeyModel.php

@@ -0,0 +1,25 @@
+<?php
+
+/**
+ * This file is part of CodeIgniter 4 framework.
+ *
+ * (c) CodeIgniter Foundation <[email protected]>
+ *
+ * For the full copyright and license information, please view
+ * the LICENSE file that was distributed with this source code.
+ */
+
+namespace Tests\Support\Models;
+
+use CodeIgniter\Model;
+use Tests\Support\Entity\UUID;
+
+class UUIDPkeyModel extends Model
+{
+    protected $table            = 'uuid';
+    protected $useAutoIncrement = false;
+    protected $returnType       = UUID::class;
+    protected $allowedFields    = [
+        'value',
+    ];
+}

tests/system/Models/UpdateModelTest.php

@@ -14,12 +14,15 @@
 use CodeIgniter\Database\Exceptions\DatabaseException;
 use CodeIgniter\Database\Exceptions\DataException;
 use CodeIgniter\Entity\Entity;
+use Config\Database;
 use InvalidArgumentException;
 use stdClass;
+use Tests\Support\Entity\UUID;
 use Tests\Support\Models\EventModel;
 use Tests\Support\Models\JobModel;
 use Tests\Support\Models\SecondaryModel;
 use Tests\Support\Models\UserModel;
+use Tests\Support\Models\UUIDPkeyModel;
 use Tests\Support\Models\ValidModel;
 use Tests\Support\Models\WithoutAutoIncrementModel;
 
@@ -375,6 +378,98 @@ public function testUpdateWithEntityNoAllowedFields(): void
         $this->model->update($id, $entity);
     }
 
+    public function testUpdateEntityWithPrimaryKeyCast(): void
+    {
+        if (
+            $this->db->DBDriver === 'OCI8'
+            || $this->db->DBDriver === 'Postgre'
+            || $this->db->DBDriver === 'SQLSRV'
+            || $this->db->DBDriver === 'SQLite3'
+        ) {
+            $this->markTestSkipped($this->db->DBDriver . ' does not work with binary data as string data.');
+        }
+
+        $this->createUuidTable();
+
+        $this->createModel(UUIDPkeyModel::class);
+
+        $entity        = new UUID();
+        $entity->id    = '550e8400-e29b-41d4-a716-446655440000';
+        $entity->value = 'test1';
+
+        $id     = $this->model->insert($entity);
+        $entity = $this->model->find($id);
+
+        $entity->value = 'id';
+        $result        = $this->model->save($entity);
+
+        $this->assertTrue($result);
+
+        $entity = $this->model->find($id);
+
+        $this->assertSame('id', $entity->value);
+    }
+
+    public function testUpdateBatchEntityWithPrimaryKeyCast(): void
+    {
+        if (
+            $this->db->DBDriver === 'OCI8'
+            || $this->db->DBDriver === 'Postgre'
+            || $this->db->DBDriver === 'SQLSRV'
+            || $this->db->DBDriver === 'SQLite3'
+        ) {
+            $this->markTestSkipped($this->db->DBDriver . ' does not work with binary data as string data.');
+        }
+
+        // See https://github.com/codeigniter4/CodeIgniter4/pull/8282#issuecomment-1836974182
+        $this->markTestSkipped(
+            'batchUpdate() is currently not working due to data type issues in the generated SQL statement.'
+        );
+
+        $this->createUuidTable();
+
+        $this->createModel(UUIDPkeyModel::class);
+
+        $entity1        = new UUID();
+        $entity1->id    = '550e8400-e29b-41d4-a716-446655440000';
+        $entity1->value = 'test1';
+        $id1            = $this->model->insert($entity1);
+
+        $entity2        = new UUID();
+        $entity2->id    = 'bd59cff1-7a24-dde5-ac10-7b929db6da8c';
+        $entity2->value = 'test2';
+        $id2            = $this->model->insert($entity2);
+
+        $entity1 = $this->model->find($id1);
+        $entity2 = $this->model->find($id2);
+
+        $entity1->value = 'update1';
+        $entity2->value = 'update2';
+
+        $data = [
+            $entity1,
+            $entity2,
+        ];
+        $this->model->updateBatch($data, 'id');
+
+        $this->seeInDatabase('uuid', [
+            'value' => 'update1',
+        ]);
+        $this->seeInDatabase('uuid', [
+            'value' => 'update2',
+        ]);
+    }
+
+    private function createUuidTable(): void
+    {
+        $forge = Database::forge($this->DBGroup);
+        $forge->dropTable('uuid', true);
+        $forge->addField([
+            'id'    => ['type' => 'BINARY', 'constraint' => 16],
+            'value' => ['type' => 'VARCHAR', 'constraint' => 400, 'null' => true],
+        ])->addKey('id', true)->createTable('uuid', true);
+    }
+
     public function testUseAutoIncrementSetToFalseUpdate(): void
     {
         $key = 'key';