Skip to content

Commit fc2bff7

Browse files
kenjiskenjis
authored
Merge pull request #8282 from kenjis/fix-model-pk-with-cast
fix: Model handling of Entity $primaryKey casting
2 parents 7a38206 + 6df976b commit fc2bff7

File tree

6 files changed

+205
-32
lines changed

6 files changed

+205
-32
lines changed

‎phpstan-baseline.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2608,7 +2608,7 @@
26082608
];
26092609
$ignoreErrors[] = [
26102610
'message' => '#^Construct empty\\(\\) is not allowed\\. Use more strict comparison\\.$#',
2611-
'count' => 21,
2611+
'count' => 18,
26122612
'path' => __DIR__ . '/system/Model.php',
26132613
];
26142614
$ignoreErrors[] = [

‎system/Model.php‎

Lines changed: 16 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -534,6 +534,21 @@ protected function idValue($data)
534534
public function getIdValue($data)
535535
{
536536
if (is_object($data) && isset($data->{$this->primaryKey})) {
537+
// Get the raw primary key value of the Entity.
538+
if ($data instanceof Entity) {
539+
$cast = $data->cast();
540+
541+
// Disable Entity casting, because raw primary key value is needed for database.
542+
$data->cast(false);
543+
544+
$primaryKey = $data->{$this->primaryKey};
545+
546+
// Restore Entity casting setting.
547+
$data->cast($cast);
548+
549+
return $primaryKey;
550+
}
551+
537552
return $data->{$this->primaryKey};
538553
}
539554

@@ -796,37 +811,7 @@ public function update($id = null, $data = null): bool
796811
*/
797812
protected function objectToRawArray($data, bool $onlyChanged = true, bool $recursive = false): ?array
798813
{
799-
$properties = parent::objectToRawArray($data, $onlyChanged);
800-
801-
$primaryKey = null;
802-
803-
if ($data instanceof Entity) {
804-
$cast = $data->cast();
805-
806-
// Disable Entity casting, because raw primary key data is needed for database.
807-
$data->cast(false);
808-
809-
$primaryKey = $data->{$this->primaryKey};
810-
811-
// Restore Entity casting setting.
812-
$data->cast($cast);
813-
}
814-
815-
// Always grab the primary key otherwise updates will fail.
816-
if (
817-
// @TODO Should use `$data instanceof Entity`.
818-
method_exists($data, 'toRawArray')
819-
&& (
820-
! empty($properties)
821-
&& ! empty($this->primaryKey)
822-
&& ! in_array($this->primaryKey, $properties, true)
823-
&& ! empty($primaryKey)
824-
)
825-
) {
826-
$properties[$this->primaryKey] = $primaryKey;
827-
}
828-
829-
return $properties;
814+
return parent::objectToRawArray($data, $onlyChanged);
830815
}
831816

832817
/**

‎tests/_support/Entity/Cast/CastBinaryUUID.php‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
<?php
2+
3+
/**
4+
* This file is part of CodeIgniter 4 framework.
5+
*
6+
* (c) CodeIgniter Foundation <[email protected]>
7+
*
8+
* For the full copyright and license information, please view
9+
* the LICENSE file that was distributed with this source code.
10+
*/
11+
12+
namespace Tests\Support\Entity\Cast;
13+
14+
use CodeIgniter\Entity\Cast\BaseCast;
15+
16+
class CastBinaryUUID extends BaseCast
17+
{
18+
/**
19+
* Get
20+
*
21+
* @param string $binary Binary UUID
22+
*
23+
* @return string String UUID
24+
*/
25+
public static function get($binary, array $params = []): string
26+
{
27+
$string = unpack('h*', $binary);
28+
29+
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]);
30+
}
31+
32+
/**
33+
* Set
34+
*
35+
* @param string $string String UUID
36+
*
37+
* @return string Binary UUID
38+
*/
39+
public static function set($string, array $params = []): string
40+
{
41+
return pack('h*', str_replace('-', '', $string));
42+
}
43+
}

‎tests/_support/Entity/UUID.php‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
<?php
2+
3+
/**
4+
* This file is part of CodeIgniter 4 framework.
5+
*
6+
* (c) CodeIgniter Foundation <[email protected]>
7+
*
8+
* For the full copyright and license information, please view
9+
* the LICENSE file that was distributed with this source code.
10+
*/
11+
12+
namespace Tests\Support\Entity;
13+
14+
use CodeIgniter\Entity\Entity;
15+
use Tests\Support\Entity\Cast\CastBinaryUUID;
16+
17+
class UUID extends Entity
18+
{
19+
protected $casts = [
20+
'id' => 'uuid',
21+
];
22+
protected $castHandlers = [
23+
'uuid' => CastBinaryUUID::class,
24+
];
25+
}

‎tests/_support/Models/UUIDPkeyModel.php‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
<?php
2+
3+
/**
4+
* This file is part of CodeIgniter 4 framework.
5+
*
6+
* (c) CodeIgniter Foundation <[email protected]>
7+
*
8+
* For the full copyright and license information, please view
9+
* the LICENSE file that was distributed with this source code.
10+
*/
11+
12+
namespace Tests\Support\Models;
13+
14+
use CodeIgniter\Model;
15+
use Tests\Support\Entity\UUID;
16+
17+
class UUIDPkeyModel extends Model
18+
{
19+
protected $table = 'uuid';
20+
protected $useAutoIncrement = false;
21+
protected $returnType = UUID::class;
22+
protected $allowedFields = [
23+
'value',
24+
];
25+
}

‎tests/system/Models/UpdateModelTest.php‎

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,15 @@
1414
use CodeIgniter\Database\Exceptions\DatabaseException;
1515
use CodeIgniter\Database\Exceptions\DataException;
1616
use CodeIgniter\Entity\Entity;
17+
use Config\Database;
1718
use InvalidArgumentException;
1819
use stdClass;
20+
use Tests\Support\Entity\UUID;
1921
use Tests\Support\Models\EventModel;
2022
use Tests\Support\Models\JobModel;
2123
use Tests\Support\Models\SecondaryModel;
2224
use Tests\Support\Models\UserModel;
25+
use Tests\Support\Models\UUIDPkeyModel;
2326
use Tests\Support\Models\ValidModel;
2427
use Tests\Support\Models\WithoutAutoIncrementModel;
2528

@@ -375,6 +378,98 @@ public function testUpdateWithEntityNoAllowedFields(): void
375378
$this->model->update($id, $entity);
376379
}
377380

381+
public function testUpdateEntityWithPrimaryKeyCast(): void
382+
{
383+
if (
384+
$this->db->DBDriver === 'OCI8'
385+
|| $this->db->DBDriver === 'Postgre'
386+
|| $this->db->DBDriver === 'SQLSRV'
387+
|| $this->db->DBDriver === 'SQLite3'
388+
) {
389+
$this->markTestSkipped($this->db->DBDriver . ' does not work with binary data as string data.');
390+
}
391+
392+
$this->createUuidTable();
393+
394+
$this->createModel(UUIDPkeyModel::class);
395+
396+
$entity = new UUID();
397+
$entity->id = '550e8400-e29b-41d4-a716-446655440000';
398+
$entity->value = 'test1';
399+
400+
$id = $this->model->insert($entity);
401+
$entity = $this->model->find($id);
402+
403+
$entity->value = 'id';
404+
$result = $this->model->save($entity);
405+
406+
$this->assertTrue($result);
407+
408+
$entity = $this->model->find($id);
409+
410+
$this->assertSame('id', $entity->value);
411+
}
412+
413+
public function testUpdateBatchEntityWithPrimaryKeyCast(): void
414+
{
415+
if (
416+
$this->db->DBDriver === 'OCI8'
417+
|| $this->db->DBDriver === 'Postgre'
418+
|| $this->db->DBDriver === 'SQLSRV'
419+
|| $this->db->DBDriver === 'SQLite3'
420+
) {
421+
$this->markTestSkipped($this->db->DBDriver . ' does not work with binary data as string data.');
422+
}
423+
424+
// See https://github.com/codeigniter4/CodeIgniter4/pull/8282#issuecomment-1836974182
425+
$this->markTestSkipped(
426+
'batchUpdate() is currently not working due to data type issues in the generated SQL statement.'
427+
);
428+
429+
$this->createUuidTable();
430+
431+
$this->createModel(UUIDPkeyModel::class);
432+
433+
$entity1 = new UUID();
434+
$entity1->id = '550e8400-e29b-41d4-a716-446655440000';
435+
$entity1->value = 'test1';
436+
$id1 = $this->model->insert($entity1);
437+
438+
$entity2 = new UUID();
439+
$entity2->id = 'bd59cff1-7a24-dde5-ac10-7b929db6da8c';
440+
$entity2->value = 'test2';
441+
$id2 = $this->model->insert($entity2);
442+
443+
$entity1 = $this->model->find($id1);
444+
$entity2 = $this->model->find($id2);
445+
446+
$entity1->value = 'update1';
447+
$entity2->value = 'update2';
448+
449+
$data = [
450+
$entity1,
451+
$entity2,
452+
];
453+
$this->model->updateBatch($data, 'id');
454+
455+
$this->seeInDatabase('uuid', [
456+
'value' => 'update1',
457+
]);
458+
$this->seeInDatabase('uuid', [
459+
'value' => 'update2',
460+
]);
461+
}
462+
463+
private function createUuidTable(): void
464+
{
465+
$forge = Database::forge($this->DBGroup);
466+
$forge->dropTable('uuid', true);
467+
$forge->addField([
468+
'id' => ['type' => 'BINARY', 'constraint' => 16],
469+
'value' => ['type' => 'VARCHAR', 'constraint' => 400, 'null' => true],
470+
])->addKey('id', true)->createTable('uuid', true);
471+
}
472+
378473
public function testUseAutoIncrementSetToFalseUpdate(): void
379474
{
380475
$key = 'key';

0 commit comments

Comments
 (0)