diogo
/
symfony
Archived
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

bug #39113 [DoctrineBridge] drop binary variants of UID types (nicolas-grekas) 

This PR was merged into the 5.2 branch.

Discussion
----------

[DoctrineBridge] drop binary variants of UID types

| Q             | A
| ------------- | ---
| Branch?       | 5.2
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | #39112
| License       | MIT
| Doc PR        | -

#39112 made me realize that when the DB engine doesn't have a native UUID/GUID type, there's no benefit to representing ULIDs as UUIDs.

This PR proposes to use the native GUID type for both UUIDs and ULIDs only when the DB engine provides such a type, and to use `BINARY(16)` when the DB engine doesn't have a native GUID type.

This leaves us in a situation where, whether the DB engine supports GUID natively or not, UUID and ULID are always stored in the most compact format.

This makes the "binary" variants useless.

MySQL 8 has [functions](https://mysqlserverteam.com/mysql-8-0-uuid-support/) to deal with binary GUID, and so does [SQLite](https://sqlite.org/src/file/ext/misc/uuid.c).

Commits
-------

bdfc20520e [DoctrineBridge] drop binary variants of UID types
This commit is contained in:
Fabien Potencier 2020-11-19 10:31:30 +01:00
parent de09329253 bdfc20520e
commit c13d5b5053
10 changed files with 19 additions and 391 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/Symfony/Bridge/Doctrine/CHANGELOG.md
View File

@ -4,7 +4,7 @@ CHANGELOG
5.2.0
-----
* added support for symfony/uid as `UlidType`, `UuidType`, `UlidBinaryType` and `UuidBinaryType` as Doctrine types
* added support for symfony/uid as `UlidType` and `UuidType` as Doctrine types
* added `UlidGenerator`, `UUidV1Generator`, `UuidV4Generator` and `UuidV6Generator`
5.0.0

10
src/Symfony/Bridge/Doctrine/DependencyInjection/CompilerPass/RegisterUidTypePass.php
View File

@ -11,9 +11,7 @@
namespace Symfony\Bridge\Doctrine\DependencyInjection\CompilerPass;
use Symfony\Bridge\Doctrine\Types\UlidBinaryType;
use Symfony\Bridge\Doctrine\Types\UlidType;
use Symfony\Bridge\Doctrine\Types\UuidBinaryType;
use Symfony\Bridge\Doctrine\Types\UuidType;
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
use Symfony\Component\DependencyInjection\ContainerBuilder;
@ -40,14 +38,6 @@ final class RegisterUidTypePass implements CompilerPassInterface
$typeDefinition['ulid'] = ['class' => UlidType::class];
}
if (!isset($typeDefinition['uuid_binary'])) {
$typeDefinition['uuid_binary'] = ['class' => UuidBinaryType::class];
}
if (!isset($typeDefinition['ulid_binary'])) {
$typeDefinition['ulid_binary'] = ['class' => UlidBinaryType::class];
}
$container->setParameter('doctrine.dbal.connection_factory.types', $typeDefinition);
}
}

114
src/Symfony/Bridge/Doctrine/Tests/Types/UlidBinaryTypeTest.php
View File

@ -1,114 +0,0 @@
<?php
/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/
namespace Symfony\Bridge\Doctrine\Tests\Types;
use Doctrine\DBAL\Platforms\AbstractPlatform;
use Doctrine\DBAL\Types\ConversionException;
use Doctrine\DBAL\Types\Type;
use PHPUnit\Framework\TestCase;
use Symfony\Bridge\Doctrine\Types\UlidBinaryType;
use Symfony\Component\Uid\Ulid;
class UlidBinaryTypeTest extends TestCase
{
private const DUMMY_ULID = '01EEDQEK6ZAZE93J8KG5B4MBJC';
private $platform;
/** @var UlidBinaryType */
private $type;
public static function setUpBeforeClass(): void
{
Type::addType('ulid_binary', UlidBinaryType::class);
}
protected function setUp(): void
{
$this->platform = $this->createMock(AbstractPlatform::class);
$this->platform
->method('getBinaryTypeDeclarationSQL')
->willReturn('DUMMYBINARY(16)');
$this->type = Type::getType('ulid_binary');
}
public function testUlidConvertsToDatabaseValue()
{
$uuid = Ulid::fromString(self::DUMMY_ULID);
$expected = $uuid->toBinary();
$actual = $this->type->convertToDatabaseValue($uuid, $this->platform);
$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 testNotSupportedTypeConversionForDatabaseValue()
{
$this->expectException(ConversionException::class);
$this->type->convertToDatabaseValue(new \stdClass(), $this->platform);
}
public function testNullConversionForDatabaseValue()
{
$this->assertNull($this->type->convertToDatabaseValue(null, $this->platform));
}
public function testUlidConvertsToPHPValue()
{
$uuid = $this->type->convertToPHPValue(self::DUMMY_ULID, $this->platform);
$this->assertEquals(self::DUMMY_ULID, $uuid->__toString());
}
public function testInvalidUlidConversionForPHPValue()
{
$this->expectException(ConversionException::class);
$this->type->convertToPHPValue('abcdefg', $this->platform);
}
public function testNullConversionForPHPValue()
{
$this->assertNull($this->type->convertToPHPValue(null, $this->platform));
}
public function testReturnValueIfUlidForPHPValue()
{
$uuid = new Ulid();
$this->assertSame($uuid, $this->type->convertToPHPValue($uuid, $this->platform));
}
public function testGetName()
{
$this->assertEquals('ulid_binary', $this->type->getName());
}
public function testGetGuidTypeDeclarationSQL()
{
$this->assertEquals('DUMMYBINARY(16)', $this->type->getSqlDeclaration(['length' => 36], $this->platform));
}
public function testRequiresSQLCommentHint()
{
$this->assertTrue($this->type->requiresSQLCommentHint($this->platform));
}
}

3
src/Symfony/Bridge/Doctrine/Tests/Types/UlidTypeTest.php
View File

@ -37,6 +37,9 @@ final class UlidTypeTest extends TestCase
protected function setUp(): void
{
$this->platform = $this->createMock(AbstractPlatform::class);
$this->platform
->method('hasNativeGuidType')
->willReturn(true);
$this->platform
->method('getGuidTypeDeclarationSQL')
->willReturn('DUMMYVARCHAR()');

123
src/Symfony/Bridge/Doctrine/Tests/Types/UuidBinaryTypeTest.php
View File

@ -1,123 +0,0 @@
<?php
/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/
namespace Symfony\Bridge\Doctrine\Tests\Types;
use Doctrine\DBAL\Platforms\AbstractPlatform;
use Doctrine\DBAL\Types\ConversionException;
use Doctrine\DBAL\Types\Type;
use PHPUnit\Framework\TestCase;
use Symfony\Bridge\Doctrine\Types\UuidBinaryType;
use Symfony\Component\Uid\Uuid;
class UuidBinaryTypeTest extends TestCase
{
private const DUMMY_UUID = '9f755235-5a2d-4aba-9605-e9962b312e50';
private $platform;
/** @var UuidBinaryType */
private $type;
public static function setUpBeforeClass(): void
{
Type::addType('uuid_binary', UuidBinaryType::class);
}
protected function setUp(): void
{
$this->platform = $this->createMock(AbstractPlatform::class);
$this->platform
->method('getBinaryTypeDeclarationSQL')
->willReturn('DUMMYBINARY(16)');
$this->type = Type::getType('uuid_binary');
}
public function testUuidConvertsToDatabaseValue()
{
$uuid = Uuid::fromString(self::DUMMY_UUID);
$expected = uuid_parse(self::DUMMY_UUID);
$actual = $this->type->convertToDatabaseValue($uuid, $this->platform);
$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()
{
$this->expectException(ConversionException::class);
$this->type->convertToDatabaseValue('abcdefg', $this->platform);
}
public function testNullConversionForDatabaseValue()
{
$this->assertNull($this->type->convertToDatabaseValue(null, $this->platform));
}
public function testUuidConvertsToPHPValue()
{
$uuid = $this->type->convertToPHPValue(uuid_parse(self::DUMMY_UUID), $this->platform);
$this->assertEquals(self::DUMMY_UUID, $uuid->__toString());
}
public function testInvalidUuidConversionForPHPValue()
{
$this->expectException(ConversionException::class);
$this->type->convertToPHPValue('abcdefg', $this->platform);
}
public function testNotSupportedTypeConversionForDatabaseValue()
{
$this->expectException(ConversionException::class);
$this->type->convertToDatabaseValue(new \stdClass(), $this->platform);
}
public function testNullConversionForPHPValue()
{
$this->assertNull($this->type->convertToPHPValue(null, $this->platform));
}
public function testReturnValueIfUuidForPHPValue()
{
$uuid = Uuid::v4();
$this->assertSame($uuid, $this->type->convertToPHPValue($uuid, $this->platform));
}
public function testGetName()
{
$this->assertEquals('uuid_binary', $this->type->getName());
}
public function testGetGuidTypeDeclarationSQL()
{
$this->assertEquals('DUMMYBINARY(16)', $this->type->getSqlDeclaration(['length' => 36], $this->platform));
}
public function testRequiresSQLCommentHint()
{
$this->assertTrue($this->type->requiresSQLCommentHint($this->platform));
}
}

3
src/Symfony/Bridge/Doctrine/Tests/Types/UuidTypeTest.php
View File

@ -37,6 +37,9 @@ final class UuidTypeTest extends TestCase
protected function setUp(): void
{
$this->platform = $this->createMock(AbstractPlatform::class);
$this->platform
->method('hasNativeGuidType')
->willReturn(true);
$this->platform
->method('getGuidTypeDeclarationSQL')
->willReturn('DUMMYVARCHAR()');

86
src/Symfony/Bridge/Doctrine/Types/AbstractBinaryUidType.php
View File

@ -1,86 +0,0 @@
<?php
/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/
namespace Symfony\Bridge\Doctrine\Types;
use Doctrine\DBAL\Platforms\AbstractPlatform;
use Doctrine\DBAL\Types\ConversionException;
use Doctrine\DBAL\Types\Type;
use Symfony\Component\Uid\AbstractUid;
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,
]);
}
/**
* {@inheritdoc}
*
* @throws ConversionException
*/
public function convertToPHPValue($value, AbstractPlatform $platform): ?AbstractUid
{
if ($value instanceof AbstractUid || null === $value) {
return $value;
}
if (!\is_string($value)) {
throw ConversionException::conversionFailedInvalidType($value, $this->getName(), ['null', 'string', AbstractUid::class]);
}
try {
return $this->getUidClass()::fromString($value);
} catch (\InvalidArgumentException $e) {
throw ConversionException::conversionFailed($value, $this->getName(), $e);
}
}
/**
* {@inheritdoc}
*
* @throws ConversionException
*/
public function convertToDatabaseValue($value, AbstractPlatform $platform): ?string
{
if ($value instanceof AbstractUid) {
return $value->toBinary();
}
if (null === $value || '' === $value) {
return null;
}
if (!\is_string($value)) {
throw ConversionException::conversionFailedInvalidType($value, $this->getName(), ['null', 'string', AbstractUid::class]);
}
try {
return $this->getUidClass()::fromString($value)->toBinary();
} catch (\InvalidArgumentException $e) {
throw ConversionException::conversionFailed($value, $this->getName());
}
}
/**
* {@inheritdoc}
*/
public function requiresSQLCommentHint(AbstractPlatform $platform): bool
{
return true;
}
}

15
src/Symfony/Bridge/Doctrine/Types/AbstractUidType.php
View File

@ -25,7 +25,14 @@ abstract class AbstractUidType extends Type
*/
public function getSQLDeclaration(array $column, AbstractPlatform $platform): string
{
return $platform->getGuidTypeDeclarationSQL($column);
if ($platform->hasNativeGuidType()) {
return $platform->getGuidTypeDeclarationSQL($column);
}
return $platform->getBinaryTypeDeclarationSQL([
'length' => '16',
'fixed' => true,
]);
}
/**
@ -57,8 +64,10 @@ abstract class AbstractUidType extends Type
*/
public function convertToDatabaseValue($value, AbstractPlatform $platform): ?string
{
$toString = $platform->hasNativeGuidType() ? 'toRfc4122' : 'toBinary';
if ($value instanceof AbstractUid) {
return $value->toRfc4122();
return $value->$toString();
}
if (null === $value || '' === $value) {
@ -70,7 +79,7 @@ abstract class AbstractUidType extends Type
}
try {
return $this->getUidClass()::fromString($value)->toRfc4122();
return $this->getUidClass()::fromString($value)->$toString();
} catch (\InvalidArgumentException $e) {
throw ConversionException::conversionFailed($value, $this->getName());
}

27
src/Symfony/Bridge/Doctrine/Types/UlidBinaryType.php
View File

@ -1,27 +0,0 @@
<?php
/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/
namespace Symfony\Bridge\Doctrine\Types;
use Symfony\Component\Uid\Ulid;
final class UlidBinaryType extends AbstractBinaryUidType
{
public function getName(): string
{
return 'ulid_binary';
}
protected function getUidClass(): string
{
return Ulid::class;
}
}

27
src/Symfony/Bridge/Doctrine/Types/UuidBinaryType.php
View File

@ -1,27 +0,0 @@
<?php
/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/
namespace Symfony\Bridge\Doctrine\Types;
use Symfony\Component\Uid\Uuid;
final class UuidBinaryType extends AbstractBinaryUidType
{
public function getName(): string
{
return 'uuid_binary';
}
protected function getUidClass(): string
{
return Uuid::class;
}
}