From 2ee497943d2eb32e7dedaf25ac97be5a0c1a8ded Mon Sep 17 00:00:00 2001 From: kenjis Date: Fri, 1 Dec 2023 17:42:04 +0900 Subject: [PATCH 1/7] test: add test for Model and Entity with cast primaryKey --- tests/_support/Entity/Cast/CastUUID.php | 43 +++++++++++++++++++++++++ tests/_support/Entity/UUID.php | 25 ++++++++++++++ tests/_support/Models/UUIDPkeyModel.php | 25 ++++++++++++++ tests/system/Models/UpdateModelTest.php | 36 +++++++++++++++++++++ 4 files changed, 129 insertions(+) create mode 100644 tests/_support/Entity/Cast/CastUUID.php create mode 100644 tests/_support/Entity/UUID.php create mode 100644 tests/_support/Models/UUIDPkeyModel.php diff --git a/tests/_support/Entity/Cast/CastUUID.php b/tests/_support/Entity/Cast/CastUUID.php new file mode 100644 index 000000000000..42cbb23b2670 --- /dev/null +++ b/tests/_support/Entity/Cast/CastUUID.php @@ -0,0 +1,43 @@ + + * + * 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 CastUUID 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)); + } +} diff --git a/tests/_support/Entity/UUID.php b/tests/_support/Entity/UUID.php new file mode 100644 index 000000000000..1c124656458f --- /dev/null +++ b/tests/_support/Entity/UUID.php @@ -0,0 +1,25 @@ + + * + * 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\CastUUID; + +class UUID extends Entity +{ + protected $casts = [ + 'id' => 'uuid', + ]; + protected $castHandlers = [ + 'uuid' => CastUUID::class, + ]; +} diff --git a/tests/_support/Models/UUIDPkeyModel.php b/tests/_support/Models/UUIDPkeyModel.php new file mode 100644 index 000000000000..be4a3c750093 --- /dev/null +++ b/tests/_support/Models/UUIDPkeyModel.php @@ -0,0 +1,25 @@ + + * + * 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', + ]; +} diff --git a/tests/system/Models/UpdateModelTest.php b/tests/system/Models/UpdateModelTest.php index 78ee2c945631..c18a30f7fc9a 100644 --- a/tests/system/Models/UpdateModelTest.php +++ b/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,39 @@ public function testUpdateWithEntityNoAllowedFields(): void $this->model->update($id, $entity); } + public function testUpdateEntityWithPrimaryKeyCast(): void + { + $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); + } + + 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'; From b8330e904ac467fff90d4b4e82640ab5dfa1c52b Mon Sep 17 00:00:00 2001 From: kenjis Date: Fri, 1 Dec 2023 18:58:12 +0900 Subject: [PATCH 2/7] fix: processing in Model of Entity for which primary key is cast --- system/Model.php | 47 ++++++++++++++++------------------------------- 1 file changed, 16 insertions(+), 31 deletions(-) diff --git a/system/Model.php b/system/Model.php index 3a055531cb5b..0a8cd9dbec59 100644 --- a/system/Model.php +++ b/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); } /** From 7f7be9a2ed59f4c5ab48805f83222182c2ce510e Mon Sep 17 00:00:00 2001 From: kenjis Date: Fri, 1 Dec 2023 19:05:58 +0900 Subject: [PATCH 3/7] chore: update phpstan-baseline --- phpstan-baseline.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpstan-baseline.php b/phpstan-baseline.php index 1322685d6fd2..039eedaaccdc 100644 --- a/phpstan-baseline.php +++ b/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[] = [ From c0a86fd6424938b506257e9f739b992853817125 Mon Sep 17 00:00:00 2001 From: kenjis Date: Sat, 2 Dec 2023 06:53:59 +0900 Subject: [PATCH 4/7] test: skip test on OCI8, Postgre, SQLSRV --- tests/system/Models/UpdateModelTest.php | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/system/Models/UpdateModelTest.php b/tests/system/Models/UpdateModelTest.php index c18a30f7fc9a..89d3768dc8d1 100644 --- a/tests/system/Models/UpdateModelTest.php +++ b/tests/system/Models/UpdateModelTest.php @@ -380,6 +380,14 @@ public function testUpdateWithEntityNoAllowedFields(): void public function testUpdateEntityWithPrimaryKeyCast(): void { + if ( + $this->db->DBDriver === 'OCI8' + || $this->db->DBDriver === 'Postgre' + || $this->db->DBDriver === 'SQLSRV' + ) { + $this->markTestSkipped($this->db->DBDriver . ' does not work with binary data as string data.'); + } + $this->createUuidTable(); $this->createModel(UUIDPkeyModel::class); From 87a706ca6a228d074488629c596b66d63df993c3 Mon Sep 17 00:00:00 2001 From: kenjis Date: Sat, 2 Dec 2023 07:23:51 +0900 Subject: [PATCH 5/7] test: skip SQLite3 SQLite3::escapeString() is not binary safe. https://www.php.net/manual/en/sqlite3.escapestring.php --- tests/system/Models/UpdateModelTest.php | 46 +++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/tests/system/Models/UpdateModelTest.php b/tests/system/Models/UpdateModelTest.php index 89d3768dc8d1..0a2013b52a49 100644 --- a/tests/system/Models/UpdateModelTest.php +++ b/tests/system/Models/UpdateModelTest.php @@ -384,6 +384,7 @@ public function testUpdateEntityWithPrimaryKeyCast(): void $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.'); } @@ -409,6 +410,51 @@ public function testUpdateEntityWithPrimaryKeyCast(): void $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.'); + } + + $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); From c357e4c43889b5fde01ec60b46d8aaacfbe1bac3 Mon Sep 17 00:00:00 2001 From: kenjis Date: Sat, 2 Dec 2023 10:25:46 +0900 Subject: [PATCH 6/7] test: skip test for batchUpdate() that does not work --- tests/system/Models/UpdateModelTest.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/system/Models/UpdateModelTest.php b/tests/system/Models/UpdateModelTest.php index 0a2013b52a49..4e30763a4073 100644 --- a/tests/system/Models/UpdateModelTest.php +++ b/tests/system/Models/UpdateModelTest.php @@ -421,6 +421,11 @@ public function testUpdateBatchEntityWithPrimaryKeyCast(): void $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); From 6df976b54556405873909a19d6d89b6d06294644 Mon Sep 17 00:00:00 2001 From: kenjis Date: Sat, 2 Dec 2023 18:52:17 +0900 Subject: [PATCH 7/7] test: fix test Entity name --- .../_support/Entity/Cast/{CastUUID.php => CastBinaryUUID.php} | 2 +- tests/_support/Entity/UUID.php | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) rename tests/_support/Entity/Cast/{CastUUID.php => CastBinaryUUID.php} (96%) diff --git a/tests/_support/Entity/Cast/CastUUID.php b/tests/_support/Entity/Cast/CastBinaryUUID.php similarity index 96% rename from tests/_support/Entity/Cast/CastUUID.php rename to tests/_support/Entity/Cast/CastBinaryUUID.php index 42cbb23b2670..ddf5a80c202f 100644 --- a/tests/_support/Entity/Cast/CastUUID.php +++ b/tests/_support/Entity/Cast/CastBinaryUUID.php @@ -13,7 +13,7 @@ use CodeIgniter\Entity\Cast\BaseCast; -class CastUUID extends BaseCast +class CastBinaryUUID extends BaseCast { /** * Get diff --git a/tests/_support/Entity/UUID.php b/tests/_support/Entity/UUID.php index 1c124656458f..d88fa25505f9 100644 --- a/tests/_support/Entity/UUID.php +++ b/tests/_support/Entity/UUID.php @@ -12,7 +12,7 @@ namespace Tests\Support\Entity; use CodeIgniter\Entity\Entity; -use Tests\Support\Entity\Cast\CastUUID; +use Tests\Support\Entity\Cast\CastBinaryUUID; class UUID extends Entity { @@ -20,6 +20,6 @@ class UUID extends Entity 'id' => 'uuid', ]; protected $castHandlers = [ - 'uuid' => CastUUID::class, + 'uuid' => CastBinaryUUID::class, ]; }