Skip to content

Commit

Permalink
Fix: #6146 - utf8/utf8mb3 alias causes redundant ALTER TABLE changes
Browse files Browse the repository at this point in the history
  • Loading branch information
fisharebest committed Jun 4, 2024
1 parent cbd0e9a commit fb2b723
Show file tree
Hide file tree
Showing 14 changed files with 314 additions and 8 deletions.
10 changes: 10 additions & 0 deletions src/Platforms/AbstractMySQLPlatform.php
Original file line number Diff line number Diff line change
Expand Up @@ -839,4 +839,14 @@ private function indexAssetsByLowerCaseName(array $assets): array

return $result;
}

/**
* INFORMATION_SCHEMA.CHARACTER_SETS will contain either 'utf8' or 'utf8mb3' but not both.
*/
public function informationSchemaUsesUtf8mb3(Connection $connection): bool

Check warning on line 846 in src/Platforms/AbstractMySQLPlatform.php

View check run for this annotation

Codecov / codecov/patch

src/Platforms/AbstractMySQLPlatform.php#L846

Added line #L846 was not covered by tests
{
$sql = "SELECT character_set_name FROM INFORMATION_SCHEMA.CHARACTER_SETS WHERE character_set_name = 'utf8mb3'";

Check warning on line 848 in src/Platforms/AbstractMySQLPlatform.php

View check run for this annotation

Codecov / codecov/patch

src/Platforms/AbstractMySQLPlatform.php#L848

Added line #L848 was not covered by tests

return $connection->fetchOne($sql) === 'utf8mb3';

Check warning on line 850 in src/Platforms/AbstractMySQLPlatform.php

View check run for this annotation

Codecov / codecov/patch

src/Platforms/AbstractMySQLPlatform.php#L850

Added line #L850 was not covered by tests
}
}
2 changes: 2 additions & 0 deletions src/Platforms/MySQL/CharsetMetadataProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,7 @@
/** @internal */
interface CharsetMetadataProvider
{
public function normalizeCharset(string $charset): string;

public function getDefaultCharsetCollation(string $charset): ?string;
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@ public function __construct(private readonly CharsetMetadataProvider $charsetMet
{
}

public function normalizeCharset(string $charset): string

Check warning on line 21 in src/Platforms/MySQL/CharsetMetadataProvider/CachingCharsetMetadataProvider.php

View check run for this annotation

Codecov / codecov/patch

src/Platforms/MySQL/CharsetMetadataProvider/CachingCharsetMetadataProvider.php#L21

Added line #L21 was not covered by tests
{
return $this->charsetMetadataProvider->normalizeCharset($charset);

Check warning on line 23 in src/Platforms/MySQL/CharsetMetadataProvider/CachingCharsetMetadataProvider.php

View check run for this annotation

Codecov / codecov/patch

src/Platforms/MySQL/CharsetMetadataProvider/CachingCharsetMetadataProvider.php#L23

Added line #L23 was not covered by tests
}

public function getDefaultCharsetCollation(string $charset): ?string
{
if (array_key_exists($charset, $this->cache)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,28 @@
/** @internal */
final class ConnectionCharsetMetadataProvider implements CharsetMetadataProvider
{
public function __construct(private readonly Connection $connection)
public function __construct(private readonly Connection $connection, private bool $useUtf8mb3)
{
}

public function normalizeCharset(string $charset): string
{
if ($this->useUtf8mb3 && $charset === 'utf8') {
return 'utf8mb3';
}

if (! $this->useUtf8mb3 && $charset === 'utf8mb3') {
return 'utf8';
}

return $charset;
}

/** @throws Exception */
public function getDefaultCharsetCollation(string $charset): ?string
{
$charset = $this->normalizeCharset($charset);

$collation = $this->connection->fetchOne(
<<<'SQL'
SELECT DEFAULT_COLLATE_NAME
Expand Down
2 changes: 2 additions & 0 deletions src/Platforms/MySQL/CollationMetadataProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,7 @@
/** @internal */
interface CollationMetadataProvider
{
public function normalizeCollation(string $collation): string;

public function getCollationCharset(string $collation): ?string;
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@ public function __construct(private readonly CollationMetadataProvider $collatio
{
}

public function normalizeCollation(string $collation): string

Check warning on line 21 in src/Platforms/MySQL/CollationMetadataProvider/CachingCollationMetadataProvider.php

View check run for this annotation

Codecov / codecov/patch

src/Platforms/MySQL/CollationMetadataProvider/CachingCollationMetadataProvider.php#L21

Added line #L21 was not covered by tests
{
return $this->collationMetadataProvider->normalizeCollation($collation);

Check warning on line 23 in src/Platforms/MySQL/CollationMetadataProvider/CachingCollationMetadataProvider.php

View check run for this annotation

Codecov / codecov/patch

src/Platforms/MySQL/CollationMetadataProvider/CachingCollationMetadataProvider.php#L23

Added line #L23 was not covered by tests
}

public function getCollationCharset(string $collation): ?string
{
if (array_key_exists($collation, $this->cache)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,34 @@
use Doctrine\DBAL\Exception;
use Doctrine\DBAL\Platforms\MySQL\CollationMetadataProvider;

use function str_starts_with;
use function substr;

/** @internal */
final class ConnectionCollationMetadataProvider implements CollationMetadataProvider
{
public function __construct(private readonly Connection $connection)
public function __construct(private readonly Connection $connection, private bool $useUtf8mb3)
{
}

public function normalizeCollation(string $collation): string
{
if ($this->useUtf8mb3 && str_starts_with($collation, 'utf8_')) {
return 'utf8mb3' . substr($collation, 4);
}

if (! $this->useUtf8mb3 && str_starts_with($collation, 'utf8mb3_')) {
return 'utf8' . substr($collation, 7);
}

return $collation;
}

/** @throws Exception */
public function getCollationCharset(string $collation): ?string
{
$collation = $this->normalizeCollation($collation);

$charset = $this->connection->fetchOne(
<<<'SQL'
SELECT CHARACTER_SET_NAME
Expand Down
18 changes: 14 additions & 4 deletions src/Platforms/MySQL/Comparator.php
Original file line number Diff line number Diff line change
Expand Up @@ -82,10 +82,20 @@ private function normalizeTable(Table $table): Table
*/
private function normalizeOptions(array $options): array
{
if (isset($options['charset']) && ! isset($options['collation'])) {
$options['collation'] = $this->charsetMetadataProvider->getDefaultCharsetCollation($options['charset']);
} elseif (isset($options['collation']) && ! isset($options['charset'])) {
$options['charset'] = $this->collationMetadataProvider->getCollationCharset($options['collation']);
if (isset($options['charset'])) {
$options['charset'] = $this->charsetMetadataProvider->normalizeCharset($options['charset']);

if (! isset($options['collation'])) {
$options['collation'] = $this->charsetMetadataProvider->getDefaultCharsetCollation($options['charset']);
}
}

if (isset($options['collation'])) {
$options['collation'] = $this->collationMetadataProvider->normalizeCollation($options['collation']);

if (! isset($options['charset'])) {
$options['charset'] = $this->collationMetadataProvider->getCollationCharset($options['collation']);
}
}

return $options;
Expand Down
6 changes: 4 additions & 2 deletions src/Schema/MySQLSchemaManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -316,13 +316,15 @@ protected function _getPortableTableForeignKeyDefinition(array $tableForeignKey)
/** @throws Exception */
public function createComparator(): Comparator
{
$useUtf8mb3 = $this->platform->informationSchemaUsesUtf8mb3($this->connection);

Check warning on line 319 in src/Schema/MySQLSchemaManager.php

View check run for this annotation

Codecov / codecov/patch

src/Schema/MySQLSchemaManager.php#L319

Added line #L319 was not covered by tests

return new MySQL\Comparator(
$this->platform,
new CachingCharsetMetadataProvider(
new ConnectionCharsetMetadataProvider($this->connection),
new ConnectionCharsetMetadataProvider($this->connection, $useUtf8mb3),

Check warning on line 324 in src/Schema/MySQLSchemaManager.php

View check run for this annotation

Codecov / codecov/patch

src/Schema/MySQLSchemaManager.php#L324

Added line #L324 was not covered by tests
),
new CachingCollationMetadataProvider(
new ConnectionCollationMetadataProvider($this->connection),
new ConnectionCollationMetadataProvider($this->connection, $useUtf8mb3),

Check warning on line 327 in src/Schema/MySQLSchemaManager.php

View check run for this annotation

Codecov / codecov/patch

src/Schema/MySQLSchemaManager.php#L327

Added line #L327 was not covered by tests
),
$this->getDefaultTableOptions(),
);
Expand Down
85 changes: 85 additions & 0 deletions tests/Functional/Driver/DBAL6146Test.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
<?php

declare(strict_types=1);

namespace Doctrine\DBAL\Tests\Functional\Driver;

use Doctrine\DBAL\Schema\Column;
use Doctrine\DBAL\Schema\Schema;
use Doctrine\DBAL\Schema\Table;
use Doctrine\DBAL\Tests\FunctionalTestCase;
use Doctrine\DBAL\Tests\TestUtil;
use Doctrine\DBAL\Types\StringType;
use PHPUnit\Framework\Attributes\DataProvider;

class DBAL6146Test extends FunctionalTestCase
{
protected function setUp(): void
{
parent::setUp();

if (TestUtil::isDriverOneOf('pdo_mysql', 'mysqli')) {
return;
}

self::markTestSkipped('This test requires the pdo_mysql or the mysqli driver.');
}

/** @return iterable<array{array<string,string>,array<string,string>}> */
public static function equivalentCharsetAndCollationProvider(): iterable
{
yield [[], []];
yield [['charset' => 'utf8'], ['charset' => 'utf8']];
yield [['charset' => 'utf8'], ['charset' => 'utf8mb3']];
yield [['charset' => 'utf8mb3'], ['charset' => 'utf8']];
yield [['charset' => 'utf8mb3'], ['charset' => 'utf8mb3']];
yield [['collation' => 'utf8_unicode_ci'], ['collation' => 'utf8_unicode_ci']];
yield [['collation' => 'utf8_unicode_ci'], ['collation' => 'utf8mb3_unicode_ci']];
yield [['collation' => 'utf8mb3_unicode_ci'], ['collation' => 'utf8mb3_unicode_ci']];
yield [['collation' => 'utf8mb3_unicode_ci'], ['collation' => 'utf8_unicode_ci']];
yield [
['charset' => 'utf8', 'collation' => 'utf8_unicode_ci'],
['charset' => 'utf8', 'collation' => 'utf8_unicode_ci'],
];

yield [
['charset' => 'utf8', 'collation' => 'utf8_unicode_ci'],
['charset' => 'utf8mb3', 'collation' => 'utf8mb3_unicode_ci'],
];

yield [
['charset' => 'utf8mb3', 'collation' => 'utf8mb3_unicode_ci'],
['charset' => 'utf8', 'collation' => 'utf8_unicode_ci'],
];

yield [
['charset' => 'utf8mb3', 'collation' => 'utf8mb3_unicode_ci'],
['charset' => 'utf8mb3', 'collation' => 'utf8mb3_unicode_ci'],
];
}

/**
* @param array<string,string> $options1
* @param array<string,string> $options2
*/
#[DataProvider('equivalentCharsetAndCollationProvider')]
public function testThereAreNoRedundantAlterTableStatements(array $options1, array $options2): void
{
$column1 = new Column('bar', new StringType(), ['length' => 32, 'platformOptions' => $options1]);
$table1 = new Table(name: 'foo6146', columns: [$column1]);

$column2 = new Column('bar', new StringType(), ['length' => 32, 'platformOptions' => $options2]);
$table2 = new Table(name: 'foo6146', columns: [$column2]);

$this->dropAndCreateTable($table1);

$schemaManager = $this->connection->createSchemaManager();
$oldSchema = $schemaManager->introspectSchema();
$newSchema = new Schema([$table2]);
$comparator = $schemaManager->createComparator();
$schemaDiff = $comparator->compareSchemas($oldSchema, $newSchema);
$alteredTables = $schemaDiff->getAlteredTables();

self::assertEmpty($alteredTables);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
<?php

declare(strict_types=1);

namespace Doctrine\DBAL\Tests\Platforms\MySQL\CollationMetadataProvider;

use Doctrine\DBAL\Connection;
use Doctrine\DBAL\Platforms\MySQL\CharsetMetadataProvider\ConnectionCharsetMetadataProvider;
use PHPUnit\Framework\TestCase;

class ConnectionCharsetMetadataProviderTest extends TestCase
{
public function testNormalizeCharset(): void
{
$connection = $this->createMock(Connection::class);

$utf8Provider = new ConnectionCharsetMetadataProvider($connection, false);

self::assertSame('utf8', $utf8Provider->normalizeCharset('utf8'));
self::assertSame('utf8', $utf8Provider->normalizeCharset('utf8mb3'));
self::assertSame('foobar', $utf8Provider->normalizeCharset('foobar'));

$utf8mb3Provider = new ConnectionCharsetMetadataProvider($connection, true);

self::assertSame('utf8mb3', $utf8mb3Provider->normalizeCharset('utf8'));
self::assertSame('utf8mb3', $utf8mb3Provider->normalizeCharset('utf8mb3'));
self::assertSame('foobar', $utf8mb3Provider->normalizeCharset('foobar'));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
<?php

declare(strict_types=1);

namespace Doctrine\DBAL\Tests\Platforms\MySQL\CollationMetadataProvider;

use Doctrine\DBAL\Connection;
use Doctrine\DBAL\Platforms\MySQL\CollationMetadataProvider\ConnectionCollationMetadataProvider;
use PHPUnit\Framework\TestCase;

class ConnectionCollationMetadataProviderTest extends TestCase
{
public function testNormalizeCcollation(): void
{
$connection = $this->createMock(Connection::class);

$utf8Provider = new ConnectionCollationMetadataProvider($connection, false);

self::assertSame('utf8_unicode_ci', $utf8Provider->normalizeCollation('utf8_unicode_ci'));
self::assertSame('utf8_unicode_ci', $utf8Provider->normalizeCollation('utf8mb3_unicode_ci'));
self::assertSame('foobar_unicode_ci', $utf8Provider->normalizeCollation('foobar_unicode_ci'));

$utf8mb3Provider = new ConnectionCollationMetadataProvider($connection, true);

self::assertSame('utf8mb3_unicode_ci', $utf8mb3Provider->normalizeCollation('utf8_unicode_ci'));
self::assertSame('utf8mb3_unicode_ci', $utf8mb3Provider->normalizeCollation('utf8mb3_unicode_ci'));
self::assertSame('foobar_unicode_ci', $utf8mb3Provider->normalizeCollation('foobar_unicode_ci'));
}
}
Loading

0 comments on commit fb2b723

Please sign in to comment.