From ba31d0ee5995049f8926502554e6075317a2edf8 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Fri, 16 Oct 2020 15:25:24 +0200 Subject: [PATCH] [DoctrinBridge] make Uid types stricter --- .../Tests/Types/UlidBinaryTypeTest.php | 16 +++---- .../Doctrine/Tests/Types/UlidTypeTest.php | 40 +++++++---------- .../Tests/Types/UuidBinaryTypeTest.php | 18 +++----- .../Doctrine/Tests/Types/UuidTypeTest.php | 37 +++++++--------- .../Doctrine/Types/AbstractBinaryUidType.php | 40 ++++++----------- .../Bridge/Doctrine/Types/AbstractUidType.php | 44 ++++++++----------- 6 files changed, 74 insertions(+), 121 deletions(-) diff --git a/src/Symfony/Bridge/Doctrine/Tests/Types/UlidBinaryTypeTest.php b/src/Symfony/Bridge/Doctrine/Tests/Types/UlidBinaryTypeTest.php index fce1aa6100953..8ea165b49769a 100644 --- a/src/Symfony/Bridge/Doctrine/Tests/Types/UlidBinaryTypeTest.php +++ b/src/Symfony/Bridge/Doctrine/Tests/Types/UlidBinaryTypeTest.php @@ -52,24 +52,18 @@ public function testUlidConvertsToDatabaseValue() $this->assertEquals($expected, $actual); } - public function testStringUlidConvertsToDatabaseValue() - { - $expected = Ulid::fromString(self::DUMMY_ULID)->toBinary(); - $actual = $this->type->convertToDatabaseValue(self::DUMMY_ULID, $this->platform); - - $this->assertEquals($expected, $actual); - } - - public function testInvalidUlidConversionForDatabaseValue() + public function testNotSupportedStringUlidConversionToDatabaseValue() { $this->expectException(ConversionException::class); - $this->type->convertToDatabaseValue('abcdefg', $this->platform); + $this->type->convertToDatabaseValue(self::DUMMY_ULID, $this->platform); } public function testNotSupportedTypeConversionForDatabaseValue() { - $this->assertNull($this->type->convertToDatabaseValue(new \stdClass(), $this->platform)); + $this->expectException(ConversionException::class); + + $this->type->convertToDatabaseValue(new \stdClass(), $this->platform); } public function testNullConversionForDatabaseValue() diff --git a/src/Symfony/Bridge/Doctrine/Tests/Types/UlidTypeTest.php b/src/Symfony/Bridge/Doctrine/Tests/Types/UlidTypeTest.php index 328df993b2690..43715bb2324a1 100644 --- a/src/Symfony/Bridge/Doctrine/Tests/Types/UlidTypeTest.php +++ b/src/Symfony/Bridge/Doctrine/Tests/Types/UlidTypeTest.php @@ -44,7 +44,7 @@ protected function setUp(): void $this->type = Type::getType('ulid'); } - public function testUlidConvertsToDatabaseValue(): void + public function testUlidConvertsToDatabaseValue() { $ulid = Ulid::fromString(self::DUMMY_ULID); @@ -54,7 +54,7 @@ public function testUlidConvertsToDatabaseValue(): void $this->assertEquals($expected, $actual); } - public function testUlidInterfaceConvertsToDatabaseValue(): void + public function testUlidInterfaceConvertsToDatabaseValue() { $ulid = $this->createMock(AbstractUid::class); @@ -68,34 +68,26 @@ public function testUlidInterfaceConvertsToDatabaseValue(): void $this->assertEquals('foo', $actual); } - public function testUlidStringConvertsToDatabaseValue(): void - { - $actual = $this->type->convertToDatabaseValue(self::DUMMY_ULID, $this->platform); - $ulid = Ulid::fromString(self::DUMMY_ULID); - - $expected = $ulid->toRfc4122(); - - $this->assertEquals($expected, $actual); - } - - public function testInvalidUlidConversionForDatabaseValue(): void + public function testNotSupportedUlidStringConversionToDatabaseValue() { $this->expectException(ConversionException::class); - $this->type->convertToDatabaseValue('abcdefg', $this->platform); + $this->type->convertToDatabaseValue(self::DUMMY_ULID, $this->platform); } public function testNotSupportedTypeConversionForDatabaseValue() { - $this->assertNull($this->type->convertToDatabaseValue(new \stdClass(), $this->platform)); + $this->expectException(ConversionException::class); + + $this->type->convertToDatabaseValue(new \stdClass(), $this->platform); } - public function testNullConversionForDatabaseValue(): void + public function testNullConversionForDatabaseValue() { $this->assertNull($this->type->convertToDatabaseValue(null, $this->platform)); } - public function testUlidInterfaceConvertsToPHPValue(): void + public function testUlidInterfaceConvertsToPHPValue() { $ulid = $this->createMock(AbstractUid::class); $actual = $this->type->convertToPHPValue($ulid, $this->platform); @@ -103,7 +95,7 @@ public function testUlidInterfaceConvertsToPHPValue(): void $this->assertSame($ulid, $actual); } - public function testUlidConvertsToPHPValue(): void + public function testUlidConvertsToPHPValue() { $ulid = $this->type->convertToPHPValue(self::DUMMY_ULID, $this->platform); @@ -111,36 +103,36 @@ public function testUlidConvertsToPHPValue(): void $this->assertEquals(self::DUMMY_ULID, $ulid->__toString()); } - public function testInvalidUlidConversionForPHPValue(): void + public function testInvalidUlidConversionForPHPValue() { $this->expectException(ConversionException::class); $this->type->convertToPHPValue('abcdefg', $this->platform); } - public function testNullConversionForPHPValue(): void + public function testNullConversionForPHPValue() { $this->assertNull($this->type->convertToPHPValue(null, $this->platform)); } - public function testReturnValueIfUlidForPHPValue(): void + public function testReturnValueIfUlidForPHPValue() { $ulid = new Ulid(); $this->assertSame($ulid, $this->type->convertToPHPValue($ulid, $this->platform)); } - public function testGetName(): void + public function testGetName() { $this->assertEquals('ulid', $this->type->getName()); } - public function testGetGuidTypeDeclarationSQL(): void + public function testGetGuidTypeDeclarationSQL() { $this->assertEquals('DUMMYVARCHAR()', $this->type->getSqlDeclaration(['length' => 36], $this->platform)); } - public function testRequiresSQLCommentHint(): void + public function testRequiresSQLCommentHint() { $this->assertTrue($this->type->requiresSQLCommentHint($this->platform)); } diff --git a/src/Symfony/Bridge/Doctrine/Tests/Types/UuidBinaryTypeTest.php b/src/Symfony/Bridge/Doctrine/Tests/Types/UuidBinaryTypeTest.php index 9e68b6caed3a6..7d38606cf9054 100644 --- a/src/Symfony/Bridge/Doctrine/Tests/Types/UuidBinaryTypeTest.php +++ b/src/Symfony/Bridge/Doctrine/Tests/Types/UuidBinaryTypeTest.php @@ -52,21 +52,11 @@ public function testUuidConvertsToDatabaseValue() $this->assertEquals($expected, $actual); } - public function testStringUuidConvertsToDatabaseValue() - { - $uuid = self::DUMMY_UUID; - - $expected = uuid_parse(self::DUMMY_UUID); - $actual = $this->type->convertToDatabaseValue($uuid, $this->platform); - - $this->assertEquals($expected, $actual); - } - - public function testInvalidUuidConversionForDatabaseValue() + public function testNotSupportedStringUuidConversionToDatabaseValue() { $this->expectException(ConversionException::class); - $this->type->convertToDatabaseValue('abcdefg', $this->platform); + $this->type->convertToDatabaseValue(self::DUMMY_UUID, $this->platform); } public function testNullConversionForDatabaseValue() @@ -90,7 +80,9 @@ public function testInvalidUuidConversionForPHPValue() public function testNotSupportedTypeConversionForDatabaseValue() { - $this->assertNull($this->type->convertToDatabaseValue(new \stdClass(), $this->platform)); + $this->expectException(ConversionException::class); + + $this->type->convertToDatabaseValue(new \stdClass(), $this->platform); } public function testNullConversionForPHPValue() diff --git a/src/Symfony/Bridge/Doctrine/Tests/Types/UuidTypeTest.php b/src/Symfony/Bridge/Doctrine/Tests/Types/UuidTypeTest.php index a61526cc3dc10..c5c6658dad7bd 100644 --- a/src/Symfony/Bridge/Doctrine/Tests/Types/UuidTypeTest.php +++ b/src/Symfony/Bridge/Doctrine/Tests/Types/UuidTypeTest.php @@ -44,7 +44,7 @@ protected function setUp(): void $this->type = Type::getType('uuid'); } - public function testUuidConvertsToDatabaseValue(): void + public function testUuidConvertsToDatabaseValue() { $uuid = Uuid::fromString(self::DUMMY_UUID); @@ -54,7 +54,7 @@ public function testUuidConvertsToDatabaseValue(): void $this->assertEquals($expected, $actual); } - public function testUuidInterfaceConvertsToDatabaseValue(): void + public function testUuidInterfaceConvertsToDatabaseValue() { $uuid = $this->createMock(AbstractUid::class); @@ -68,31 +68,26 @@ public function testUuidInterfaceConvertsToDatabaseValue(): void $this->assertEquals('foo', $actual); } - public function testUuidStringConvertsToDatabaseValue(): void - { - $actual = $this->type->convertToDatabaseValue(self::DUMMY_UUID, $this->platform); - - $this->assertEquals(self::DUMMY_UUID, $actual); - } - - public function testInvalidUuidConversionForDatabaseValue(): void + public function testNotSupportedUuidStringConversionToDatabaseValue() { $this->expectException(ConversionException::class); - $this->type->convertToDatabaseValue('abcdefg', $this->platform); + $this->type->convertToDatabaseValue(self::DUMMY_UUID, $this->platform); } public function testNotSupportedTypeConversionForDatabaseValue() { - $this->assertNull($this->type->convertToDatabaseValue(new \stdClass(), $this->platform)); + $this->expectException(ConversionException::class); + + $this->type->convertToDatabaseValue(new \stdClass(), $this->platform); } - public function testNullConversionForDatabaseValue(): void + public function testNullConversionForDatabaseValue() { $this->assertNull($this->type->convertToDatabaseValue(null, $this->platform)); } - public function testUuidInterfaceConvertsToPHPValue(): void + public function testUuidInterfaceConvertsToPHPValue() { $uuid = $this->createMock(AbstractUid::class); $actual = $this->type->convertToPHPValue($uuid, $this->platform); @@ -100,7 +95,7 @@ public function testUuidInterfaceConvertsToPHPValue(): void $this->assertSame($uuid, $actual); } - public function testUuidConvertsToPHPValue(): void + public function testUuidConvertsToPHPValue() { $uuid = $this->type->convertToPHPValue(self::DUMMY_UUID, $this->platform); @@ -108,36 +103,36 @@ public function testUuidConvertsToPHPValue(): void $this->assertEquals(self::DUMMY_UUID, $uuid->__toString()); } - public function testInvalidUuidConversionForPHPValue(): void + public function testInvalidUuidConversionForPHPValue() { $this->expectException(ConversionException::class); $this->type->convertToPHPValue('abcdefg', $this->platform); } - public function testNullConversionForPHPValue(): void + public function testNullConversionForPHPValue() { $this->assertNull($this->type->convertToPHPValue(null, $this->platform)); } - public function testReturnValueIfUuidForPHPValue(): void + public function testReturnValueIfUuidForPHPValue() { $uuid = Uuid::v4(); $this->assertSame($uuid, $this->type->convertToPHPValue($uuid, $this->platform)); } - public function testGetName(): void + public function testGetName() { $this->assertEquals('uuid', $this->type->getName()); } - public function testGetGuidTypeDeclarationSQL(): void + public function testGetGuidTypeDeclarationSQL() { $this->assertEquals('DUMMYVARCHAR()', $this->type->getSqlDeclaration(['length' => 36], $this->platform)); } - public function testRequiresSQLCommentHint(): void + public function testRequiresSQLCommentHint() { $this->assertTrue($this->type->requiresSQLCommentHint($this->platform)); } diff --git a/src/Symfony/Bridge/Doctrine/Types/AbstractBinaryUidType.php b/src/Symfony/Bridge/Doctrine/Types/AbstractBinaryUidType.php index 587f9a54f3f0c..9321d148ce3be 100644 --- a/src/Symfony/Bridge/Doctrine/Types/AbstractBinaryUidType.php +++ b/src/Symfony/Bridge/Doctrine/Types/AbstractBinaryUidType.php @@ -13,21 +13,19 @@ use Doctrine\DBAL\Platforms\AbstractPlatform; use Doctrine\DBAL\Types\ConversionException; -use Doctrine\DBAL\Types\GuidType; +use Doctrine\DBAL\Types\Type; use Symfony\Component\Uid\AbstractUid; -abstract class AbstractBinaryUidType extends GuidType +abstract class AbstractBinaryUidType extends Type { abstract protected function getUidClass(): string; public function getSQLDeclaration(array $fieldDeclaration, AbstractPlatform $platform): string { - return $platform->getBinaryTypeDeclarationSQL( - [ - 'length' => '16', - 'fixed' => true, - ] - ); + return $platform->getBinaryTypeDeclarationSQL([ + 'length' => '16', + 'fixed' => true, + ]); } /** @@ -37,21 +35,19 @@ public function getSQLDeclaration(array $fieldDeclaration, AbstractPlatform $pla */ public function convertToPHPValue($value, AbstractPlatform $platform): ?AbstractUid { - if (null === $value || '' === $value) { - return null; + if ($value instanceof AbstractUid || null === $value) { + return $value; } - if ($value instanceof AbstractUid) { - return $value; + if (!\is_string($value)) { + throw ConversionException::conversionFailedInvalidType($value, $this->getName(), ['null', 'string', AbstractUid::class]); } try { - $uuid = $this->getUidClass()::fromString($value); + return $this->getUidClass()::fromString($value); } catch (\InvalidArgumentException $e) { - throw ConversionException::conversionFailed($value, $this->getName()); + throw ConversionException::conversionFailed($value, $this->getName(), $e); } - - return $uuid; } /** @@ -61,23 +57,15 @@ public function convertToPHPValue($value, AbstractPlatform $platform): ?Abstract */ public function convertToDatabaseValue($value, AbstractPlatform $platform): ?string { - if (null === $value || '' === $value) { - return null; - } - if ($value instanceof AbstractUid) { return $value->toBinary(); } - if (!\is_string($value) && !(\is_object($value) && method_exists($value, '__toString'))) { + if (null === $value) { return null; } - try { - return $this->getUidClass()::fromString((string) $value)->toBinary(); - } catch (\InvalidArgumentException $e) { - throw ConversionException::conversionFailed($value, $this->getName()); - } + throw ConversionException::conversionFailedInvalidType($value, $this->getName(), ['null', AbstractUid::class]); } /** diff --git a/src/Symfony/Bridge/Doctrine/Types/AbstractUidType.php b/src/Symfony/Bridge/Doctrine/Types/AbstractUidType.php index 2286bb7268191..a14eb853e3868 100644 --- a/src/Symfony/Bridge/Doctrine/Types/AbstractUidType.php +++ b/src/Symfony/Bridge/Doctrine/Types/AbstractUidType.php @@ -13,13 +13,21 @@ use Doctrine\DBAL\Platforms\AbstractPlatform; use Doctrine\DBAL\Types\ConversionException; -use Doctrine\DBAL\Types\GuidType; +use Doctrine\DBAL\Types\Type; use Symfony\Component\Uid\AbstractUid; -abstract class AbstractUidType extends GuidType +abstract class AbstractUidType extends Type { abstract protected function getUidClass(): string; + /** + * {@inheritdoc} + */ + public function getSQLDeclaration(array $column, AbstractPlatform $platform): string + { + return $platform->getGuidTypeDeclarationSQL($column); + } + /** * {@inheritdoc} * @@ -27,21 +35,19 @@ abstract protected function getUidClass(): string; */ public function convertToPHPValue($value, AbstractPlatform $platform): ?AbstractUid { - if (null === $value || '' === $value) { - return null; + if ($value instanceof AbstractUid || null === $value) { + return $value; } - if ($value instanceof AbstractUid) { - return $value; + if (!\is_string($value)) { + throw ConversionException::conversionFailedInvalidType($value, $this->getName(), ['null', 'string', AbstractUid::class]); } try { - $uuid = $this->getUidClass()::fromString($value); + return $this->getUidClass()::fromString($value); } catch (\InvalidArgumentException $e) { - throw ConversionException::conversionFailed($value, $this->getName()); + throw ConversionException::conversionFailed($value, $this->getName(), $e); } - - return $uuid; } /** @@ -51,29 +57,15 @@ public function convertToPHPValue($value, AbstractPlatform $platform): ?Abstract */ public function convertToDatabaseValue($value, AbstractPlatform $platform): ?string { - if (null === $value || '' === $value) { - return null; - } - if ($value instanceof AbstractUid) { return $value->toRfc4122(); } - if (!\is_string($value) && !(\is_object($value) && method_exists($value, '__toString'))) { + if (null === $value) { return null; } - if ($this->getUidClass()::isValid((string) $value)) { - try { - $uuid = $this->getUidClass()::fromString($value); - - return $uuid->toRfc4122(); - } catch (\InvalidArgumentException $e) { - throw ConversionException::conversionFailed($value, $this->getName()); - } - } - - throw ConversionException::conversionFailed($value, $this->getName()); + throw ConversionException::conversionFailedInvalidType($value, $this->getName(), ['null', AbstractUid::class]); } /**