8000 bug #46985 [DoctrineBridge] Avoid calling `AbstractPlatform::hasNativ… · symfony/symfony@47cb70b · GitHub
[go: up one dir, main page]
Skip to content

Commit 47cb70b

Browse files
fabpotfabpot
committed
bug #46985 [DoctrineBridge] Avoid calling AbstractPlatform::hasNativeGuidType() (derrabus)
This PR was merged into the 5.4 branch. Discussion ---------- [DoctrineBridge] Avoid calling `AbstractPlatform::hasNativeGuidType()` | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Follows doctrine/dbal#5509, alternative to doctrine/dbal#5518 | License | MIT | Doc PR | N/A `AbstractPlatform::hasNativeGuidType()` has been deprecated in Doctrine DBAL. This PR inlines the logic of that method where we need it. Furthermore, I took the liberty to refactor the corresponding tests a little. We don't really need to mock `AbstractPlatform` because we can work with actual instances. This also allows us to test the behavior of our implementation on different platforms. Commits ------- 5d882ce Avoid calling AbstractPlatform::hasNativeGuidType()
2 parents 0ddf1c9 + 5d882ce commit 47cb70b

File tree

3 files changed

+93
-51
lines changed

3 files changed

+93
-51
lines changed

‎src/Symfony/Bridge/Doctrine/Tests/Types/UlidTypeTest.php

Lines changed: 31 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -12,20 +12,23 @@
1212
namespace Symfony\Bridge\Doctrine\Tests\Types;
1313

1414
use Doctrine\DBAL\Platforms\AbstractPlatform;
15+
use Doctrine\DBAL\Platforms\MySQLPlatform;
16+
use Doctrine\DBAL\Platforms\PostgreSQLPlatform;
17+
use Doctrine\DBAL\Platforms\SqlitePlatform;
1518
use Doctrine\DBAL\Types\ConversionException;
1619
use Doctrine\DBAL\Types\Type;
1720
use PHPUnit\Framework\TestCase;
1821
use Symfony\Bridge\Doctrine\Types\UlidType;
1922
use Symfony\Component\Uid\AbstractUid;
2023
use Symfony\Component\Uid\Ulid;
2124

25+
// DBAL 2 compatibility
26+
class_exists('Doctrine\DBAL\Platforms\PostgreSqlPlatform');
27+
2228
final class UlidTypeTest extends TestCase
2329
{
2430
private const DUMMY_ULID = '01EEDQEK6ZAZE93J8KG5B4MBJC';
2531

26-
/** @var AbstractPlatform */
27-
private $platform;
28-
2932
/** @var UlidType */
3033
private $type;
3134

@@ -40,14 +43,6 @@ public static function setUpBeforeClass(): void
4043

4144
protected function setUp(): void
4245
{
43-
$this->platform = $this->createMock(AbstractPlatform::class);
44-
$this->platform
45-
->method('hasNativeGuidType')
46-
->willReturn(true);
47-
$this->platform
48-
->method('getGuidTypeDeclarationSQL')
49-
->willReturn('DUMMYVARCHAR()');
50-
5146
$this->type = Type::getType('ulid');
5247
}
5348

@@ -56,7 +51,7 @@ public function testUlidConvertsToDatabaseValue()
5651
$ulid = Ulid::fromString(self::DUMMY_ULID);
5752

5853
$expected = $ulid->toRfc4122();
59-
$actual = $this->type->convertToDatabaseValue($ulid, $this->platform);
54+
$actual = $this->type->convertToDatabaseValue($ulid, new PostgreSQLPlatform());
6055
< 9E81 br>
6156
$this->assertEquals($expected, $actual);
6257
}
@@ -70,14 +65,14 @@ public function testUlidInterfaceConvertsToDatabaseValue()
7065
->method('toRfc4122')
7166
->willReturn('foo');
7267

73-
$actual = $this->type->convertToDatabaseValue($ulid, $this->platform);
68+
$actual = $this->type->convertToDatabaseValue($ulid, new PostgreSQLPlatform());
7469

7570
$this->assertEquals('foo', $actual);
7671
}
7772

7873
public function testUlidStringConvertsToDatabaseValue()
7974
{
80-
$actual = $this->type->convertToDatabaseValue(self::DUMMY_ULID, $this->platform);
75+
$actual = $this->type->convertToDatabaseValue(self::DUMMY_ULID, new PostgreSQLPlatform());
8176
$ulid = Ulid::fromString(self::DUMMY_ULID);
8277

8378
$expected = $ulid->toRfc4122();
@@ -89,25 +84,25 @@ public function testNotSupportedTypeConversionForDatabaseValue()
8984
{
9085
$this->expectException(ConversionException::class);
9186

92-
$this->type->convertToDatabaseValue(new \stdClass(), $this->platform);
87+
$this->type->convertToDatabaseValue(new \stdClass(), new SqlitePlatform());
9388
}
9489

9590
public function testNullConversionForDatabaseValue()
9691
{
97-
$this->assertNull($this->type->convertToDatabaseValue(null, $this->platform));
92+
$this->assertNull($this->type->convertToDatabaseValue(null, new SqlitePlatform()));
9893
}
9994

10095
public function testUlidInterfaceConvertsToPHPValue()
10196
{
10297
$ulid = $this->createMock(AbstractUid::class);
103-
$actual = $this->type->convertToPHPValue($ulid, $this->platform);
98+
$actual = $this->type->convertToPHPValue($ulid, new SqlitePlatform());
10499

105100
$this->assertSame($ulid, $actual);
106101
}
107102

108103
public function testUlidConvertsToPHPValue()
109104
{
110-
$ulid = $this->type->convertToPHPValue(self::DUMMY_ULID, $this->platform);
105+
$ulid = $this->type->convertToPHPValue(self::DUMMY_ULID, new SqlitePlatform());
111106

112107
$this->assertInstanceOf(Ulid::class, $ulid);
113108
$this->assertEquals(self::DUMMY_ULID, $ulid->__toString());
@@ -117,33 +112,45 @@ public function testInvalidUlidConversionForPHPValue()
117112
{
118113
$this->expectException(ConversionException::class);
119114

120-
$this->type->convertToPHPValue('abcdefg', $this->platform);
115+
$this->type->convertToPHPValue('abcdefg', new SqlitePlatform());
121116
}
122117

123118
public function testNullConversionForPHPValue()
124119
{
125-
$this->assertNull($this->type->convertToPHPValue(null, $this->platform));
120+
$this->assertNull($this->type->convertToPHPValue(null, new SqlitePlatform()));
126121
}
127122

128123
public function testReturnValueIfUlidForPHPValue()
129124
{
130125
$ulid = new Ulid();
131126

132-
$this->assertSame($ulid, $this->type->convertToPHPValue($ulid, $this->platform));
127+
$this->assertSame($ulid, $this->type->convertToPHPValue($ulid, new SqlitePlatform()));
133128
}
134129

135130
public function testGetName()
136131
{
137132
$this->assertEquals('ulid', $this->type->getName());
138133
}
139134

140-
public function testGetGuidTypeDeclarationSQL()
135+
/**
136+
* @dataProvider provideSqlDeclarations
137+
*/
138+
public function testGetGuidTypeDeclarationSQL(AbstractPlatform $platform, string $expectedDeclaration)
139+
{
140+
$this->assertEquals($expectedDeclaration, $this->type->getSqlDeclaration(['length' => 36], $platform));
141+
}
142+
143+
public function provideSqlDeclarations(): array
141144
{
142-
$this->assertEquals('DUMMYVARCHAR()', $this->type->getSqlDeclaration(['length' => 36], $this->platform));
145+
return [
146+
[new PostgreSQLPlatform(), 'UUID'],
147+
[new SqlitePlatform(), 'BLOB'],
148+
[new MySQLPlatform(), 'BINARY(16)'],
149+
];
143150
}
144151

145152
public function testRequiresSQLCommentHint()
146153
{
147-
$this->assertTrue($this->type->requiresSQLCommentHint($this->platform));
154+
$this->assertTrue($this->type->requiresSQLCommentHint(new SqlitePlatform()));
148155
}
149156
}

‎src/Symfony/Bridge/Doctrine/Tests/Types/UuidTypeTest.php

Lines changed: 47 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -12,20 +12,24 @@
1212
namespace Symfony\Bridge\Doctrine\Tests\Types;
1313

1414
use Doctrine\DBAL\Platforms\AbstractPlatform;
15+
use Doctrine\DBAL\Platforms\MySQLPlatform;
16+
use Doctrine\DBAL\Platforms\PostgreSQLPlatform;
17+
use Doctrine\DBAL\Platforms\SqlitePlatform;
1518
use Doctrine\DBAL\Types\ConversionException;
1619
use Doctrine\DBAL\Types\Type;
1720
use PHPUnit\Framework\TestCase;
1821
use Symfony\Bridge\Doctrine\Types\UuidType;
1922
use Symfony\Component\Uid\AbstractUid;
2023
use Symfony\Component\Uid\Uuid;
2124

25+
// DBAL 2 compatibility
26+
class_exists('Doctrine\DBAL\Platforms\MySqlPlatform');
27+
class_exists('Doctrine\DBAL\Platforms\PostgreSqlPlatform');
28+
2229
final class UuidTypeTest extends TestCase
2330
{
2431
private const DUMMY_UUID = '9f755235-5a2d-4aba-9605-e9962b312e50';
2532

26-
/** @var AbstractPlatform */
27-
private $platform;
28-
2933
/** @var UuidType */
3034
private $type;
3135

@@ -40,14 +44,6 @@ public static function setUpBeforeClass(): void
4044

4145
protected function setUp(): void
4246
{
43-
$this->platform = $this->createMock(AbstractPlatform::class);
44-
$this->platform
45-
->method('hasNativeGuidType')
46-
->willReturn(true);
47-
$this->platform
48-
->method('getGuidTypeDeclarationSQL')
49-
->willReturn('DUMMYVARCHAR()');
50-
5147
$this->type = Type::getType('uuid');
5248
}
5349

@@ -56,12 +52,12 @@ public function testUuidConvertsToDatabaseValue()
5652
$uuid = Uuid::fromString(self::DUMMY_UUID);
5753

5854
$expected = $uuid->__toString();
59-
$actual = $this->type->convertToDatabaseValue($uuid, $this->platform);
55+
$actual = $this->type->convertToDatabaseValue($uuid, new PostgreSQLPlatform());
6056

6157
$this->assertEquals($expected, $actual);
6258
}
6359

64-
public function testUuidInterfaceConvertsToDatabaseValue()
60+
public function testUuidInterfaceConvertsToNativeUidDatabaseValue()
6561
{
6662
$uuid = $this->createMock(AbstractUid::class);
6763

@@ -70,14 +66,28 @@ public function testUuidInterfaceConvertsToDatabaseValue()
7066
->method('toRfc4122')
7167
->willReturn('foo');
7268

73-
$actual = $this->type->convertToDatabaseValue($uuid, $this->platform);
69+
$actual = $this->type->convertToDatabaseValue($uuid, new PostgreSQLPlatform());
70+
71+
$this->assertEquals('foo', $actual);
72+
}
73+
74+
public function testUuidInterfaceConvertsToBinaryDatabaseValue()
75+
{
76+
$uuid = $this->createMock(AbstractUid::class);
77+
78+
$uuid
79+
->expects($this->once())
80+
->method('toBinary')
81+
->willReturn('foo');
82+
83+
$actual = $this->type->convertToDatabaseValue($uuid, new MySQLPlatform());
7484

7585
$this->assertEquals('foo', $actual);
7686
}
7787

7888
public function testUuidStringConvertsToDatabaseValue()
7989
{
80-
$actual = $this->type->convertToDatabaseValue(self::DUMMY_UUID, $this->platform);
90+
$actual = $this->type->convertToDatabaseValue(self::DUMMY_UUID, new PostgreSQLPlatform());
8191

8292
$this->assertEquals(self::DUMMY_UUID, $actual);
8393
}
@@ -86,25 +96,25 @@ public function testNotSupportedTypeConversionForDatabaseValue()
8696
{
8797
$this->expectException(ConversionException::class);
8898

89-
$this->type->convertToDatabaseValue(new \stdClass(), $this->platform);
99+
$this->type->convertToDatabaseValue(new \stdClass(), new SqlitePlatform());
90100
}
91101

92102
public function testNullConversionForDatabaseValue()
93103
{
94-
$this->assertNull($this->type->convertToDatabaseValue(null, $this->platform));
104+
$this->assertNull($this->type->convertToDatabaseValue(null, new SqlitePlatform()));
95105
}
96106

97107
public function testUuidInterfaceConvertsToPHPValue()
98108
{
99109
$uuid = $this->createMock(AbstractUid::class);
100-
$actual = $this->type->convertToPHPValue($uuid, $this->platform);
110+
$actual = $this->type->convertToPHPValue($uuid, new SqlitePlatform());
101111

102112
$this->assertSame($uuid, $actual);
103113
}
104114

105115
public function testUuidConvertsToPHPValue()
106116
{
107-
$uuid = $this->type->convertToPHPValue(self::DUMMY_UUID, $this->platform);
117+
$uuid = $this->type->convertToPHPValue(self::DUMMY_UUID, new SqlitePlatform());
108118

109119
$this->assertInstanceOf(Uuid::class, $uuid);
110120
$this->assertEquals(self::DUMMY_UUID, $uuid->__toString());
@@ -114,33 +124,45 @@ public function testInvalidUuidConversionForPHPValue()
114124
{
115125
$this->expectException(ConversionException::class);
116126

117-
$this->type->convertToPHPValue('abcdefg', $this->platform);
127+
$this->type->convertToPHPValue('abcdefg', new SqlitePlatform());
118128
}
119129

120130
public function testNullConversionForPHPValue()
121131
{
122-
$this->assertNull($this->type->convertToPHPValue(null, $this->platform));
132+
$this->assertNull($this->type->convertToPHPValue(null, new SqlitePlatform()));
123133
}
124134

125135
public function testReturnValueIfUuidForPHPValue()
126136
{
127137
$uuid = Uuid::v4();
128138

129-
$this->assertSame($uuid, $this->type->convertToPHPValue($uuid, $this->platform));
139+
$this->assertSame($uuid, $this->type->convertToPHPValue($uuid, new SqlitePlatform()));
130140
}
131141

132142
public function testGetName()
133143
{
134144
$this->assertEquals('uuid', $this->type->getName());
135145
}
136146

137-
public function testGetGuidTypeDeclarationSQL()
147+
/**
148+
* @dataProvider provideSqlDeclarations
149+
*/
150+
public function testGetGuidTypeDeclarationSQL(AbstractPlatform $platform, string $expectedDeclaration)
151+
{
152+
$this->assertEquals($expectedDeclaration, $this->type->getSqlDeclaration(['length' => 36], $platform));
153+
}
154+
155+
public function provideSqlDeclarations(): array
138156
{
139-
$this->assertEquals('DUMMYVARCHAR()', $this->type->getSqlDeclaration(['length' => 36], $this->platform));
157+
return [
158+
[new PostgreSQLPlatform(), 'UUID'],
159+
[new SqlitePlatform(), 'BLOB'],
160+
[new MySQLPlatform(), 'BINARY(16)'],
161+
];
140162
}
141163

142164
public function testRequiresSQLCommentHint()
143165
{
144-
$this->assertTrue($this->type->requiresSQLCommentHint($this->platform));
166+
$this->assertTrue($this->type->requiresSQLCommentHint(new SqlitePlatform()));
145167
}
146168
}

‎src/Symfony/Bridge/Doctrine/Types/AbstractUidType.php

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,17 @@
1818

1919
abstract class AbstractUidType extends Type
2020
{
21+
/**
22+
* @return class-string<AbstractUid>
23+
*/
2124
abstract protected function getUidClass(): string;
2225

2326
/**
2427
* {@inheritdoc}
2528
*/
2629
public function getSQLDeclaration(array $column, AbstractPlatform $platform): string
2730
{
28-
if ($platform->hasNativeGuidType()) {
31+
if ($this->hasNativeGuidType($platform)) {
2932
return $platform->getGuidTypeDeclarationSQL CCF4 ($column);
3033
}
3134

@@ -64,7 +67,7 @@ public function convertToPHPValue($value, AbstractPlatform $platform): ?Abstract
6467
*/
6568
public function convertToDatabaseValue($value, AbstractPlatform $platform): ?string
6669
{
67-
$toString = $platform->hasNativeGuidType() ? 'toRfc4122' : 'toBinary';
70+
$toString = $this->hasNativeGuidType($platform) ? 'toRfc4122' : 'toBinary';
6871

6972
if ($value instanceof AbstractUid) {
7073
return $value->$toString();
@@ -92,4 +95,14 @@ public function requiresSQLCommentHint(AbstractPlatform $platform): bool
9295
{
9396
return true;
9497
}
98+
99+
private function hasNativeGuidType(AbstractPlatform $platform): bool
100+
{
101+
// Compatibility with DBAL < 3.4
102+
$method = \method_exists($platform, 'getStringTypeDeclarationSQL')
103+
? 'getStringTypeDeclarationSQL'
104+
: 'getVarcharTypeDeclarationSQL';
105+
106+
return $platform->getGuidTypeDeclarationSQL([]) !== $platform->$method(['fixed' => true, 'length' => 36]);
107+
}
95108
}

0 commit comments

Comments
 (0)
0