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 Apr 30, 2024
1 parent 7fb00fd commit fa707e2
Show file tree
Hide file tree
Showing 14 changed files with 249 additions and 8 deletions.
11 changes: 11 additions & 0 deletions src/Platforms/AbstractMySQLPlatform.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
use function sprintf;
use function str_replace;
use function strtolower;
use function version_compare;

/**
* Provides the base implementation for the lowest versions of supported MySQL-like database platforms.
Expand Down Expand Up @@ -839,4 +840,14 @@ private function indexAssetsByLowerCaseName(array $assets): array

return $result;
}

/**
* {@inheritDoc}
*
* @link https://dev.mysql.com/doc/refman/8.0/en/charset-unicode-utf8mb3.html
*/
public function informationSchemaUsesUtf8mb3(Connection $connection): bool

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

View check run for this annotation

Codecov / codecov/patch

src/Platforms/AbstractMySQLPlatform.php#L849

Added line #L849 was not covered by tests
{
return version_compare($connection->getServerVersion(), '8.0.30') >= 0;

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

View check run for this annotation

Codecov / codecov/patch

src/Platforms/AbstractMySQLPlatform.php#L851

Added line #L851 was not covered by tests
}
}
19 changes: 19 additions & 0 deletions src/Platforms/MariaDBPlatform.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace Doctrine\DBAL\Platforms;

use Doctrine\DBAL\Connection;
use Doctrine\DBAL\Platforms\Keywords\KeywordList;
use Doctrine\DBAL\Platforms\Keywords\MariaDBKeywords;
use Doctrine\DBAL\Schema\ForeignKeyConstraint;
Expand All @@ -14,6 +15,7 @@
use function array_merge;
use function count;
use function in_array;
use function version_compare;

/**
* Provides the behavior, features and SQL dialect of the MariaDB database platform of the oldest supported version.
Expand Down Expand Up @@ -162,4 +164,21 @@ protected function createReservedKeywordsList(): KeywordList
{
return new MariaDBKeywords();
}

/**
* {@inheritDoc}
*
* @link https://mariadb.com/kb/en/unicode/
* @link https://mariadb.com/kb/en/old-mode/
*/
public function informationSchemaUsesUtf8mb3(Connection $connection): bool

Check warning on line 174 in src/Platforms/MariaDBPlatform.php

View check run for this annotation

Codecov / codecov/patch

src/Platforms/MariaDBPlatform.php#L174

Added line #L174 was not covered by tests
{
if (version_compare($connection->getServerVersion(), '10.6.0') < 0) {
return false;

Check warning on line 177 in src/Platforms/MariaDBPlatform.php

View check run for this annotation

Codecov / codecov/patch

src/Platforms/MariaDBPlatform.php#L176-L177

Added lines #L176 - L177 were not covered by tests
}

$oldMode = (string) $connection->fetchOne('SELECT @old_mode');

Check warning on line 180 in src/Platforms/MariaDBPlatform.php

View check run for this annotation

Codecov / codecov/patch

src/Platforms/MariaDBPlatform.php#L180

Added line #L180 was not covered by tests

return ! str_contains($oldMode, 'UTF8_IS_UTF8MB3');

Check failure on line 182 in src/Platforms/MariaDBPlatform.php

View workflow job for this annotation

GitHub Actions / Coding Standards / Coding Standards (8.3)

Function str_contains() should not be referenced via a fallback global name, but via a use statement.

Check warning on line 182 in src/Platforms/MariaDBPlatform.php

View check run for this annotation

Codecov / codecov/patch

src/Platforms/MariaDBPlatform.php#L182

Added line #L182 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
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'));
}
}
84 changes: 84 additions & 0 deletions tests/Platforms/MySQL/ComparatorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,19 @@

namespace Doctrine\DBAL\Tests\Platforms\MySQL;

use Doctrine\DBAL\Connection;
use Doctrine\DBAL\Platforms\MySQL\CharsetMetadataProvider;
use Doctrine\DBAL\Platforms\MySQL\CharsetMetadataProvider\ConnectionCharsetMetadataProvider;
use Doctrine\DBAL\Platforms\MySQL\CollationMetadataProvider;
use Doctrine\DBAL\Platforms\MySQL\CollationMetadataProvider\ConnectionCollationMetadataProvider;
use Doctrine\DBAL\Platforms\MySQL\Comparator;
use Doctrine\DBAL\Platforms\MySQL\DefaultTableOptions;
use Doctrine\DBAL\Platforms\MySQLPlatform;
use Doctrine\DBAL\Schema\Column;
use Doctrine\DBAL\Schema\Table;
use Doctrine\DBAL\Tests\Schema\AbstractComparatorTestCase;
use Doctrine\DBAL\Types\StringType;
use PHPUnit\Framework\Attributes\DataProvider;

class ComparatorTest extends AbstractComparatorTestCase
{
Expand All @@ -22,4 +29,81 @@ protected function setUp(): void
new DefaultTableOptions('utf8mb4', 'utf8mb4_general_ci'),
);
}

#[DataProvider('utf8AndUtf8mb3MismatchesProvider')]
public function testUtf8AndUtf8mb3Mismatches(bool $useUtf8mb3, string $defaultCharset): void
{
$connection = $this->createMock(Connection::class);
$platform = new MySQLPlatform();

$utf8Comparator = new Comparator(
$platform,
new ConnectionCharsetMetadataProvider($connection, $useUtf8mb3),
new ConnectionCollationMetadataProvider($connection, $useUtf8mb3),
new DefaultTableOptions($defaultCharset, $defaultCharset . '_unicode_ci'),
);

$table1 = new Table('t1', [
new Column('c', new StringType(), [
'length' => 8,
'platformOptions' => ['collation' => 'utf8_unicode_ci'],
]),
]);
$table2 = new Table('t2', [
new Column('c', new StringType(), [
'length' => 8,
'platformOptions' => ['collation' => 'utf8mb3_unicode_ci'],
]),
]);
$table3 = new Table('t3', [
new Column('c', new StringType(), [
'length' => 8,
'platformOptions' => ['collation' => 'utf8mb4_unicode_ci'],
]),
]);

self::assertEmpty($utf8Comparator->compareTables($table1, $table2)->getModifiedColumns());
self::assertEmpty($utf8Comparator->compareTables($table2, $table1)->getModifiedColumns());
self::assertNotEmpty($utf8Comparator->compareTables($table1, $table3)->getModifiedColumns());
self::assertNotEmpty($utf8Comparator->compareTables($table3, $table1)->getModifiedColumns());
self::assertNotEmpty($utf8Comparator->compareTables($table2, $table3)->getModifiedColumns());
self::assertNotEmpty($utf8Comparator->compareTables($table3, $table2)->getModifiedColumns());

$table4 = new Table('t4', [
new Column('c', new StringType(), [
'length' => 8,
'platformOptions' => ['charset' => 'utf8'],
]),
]);
$table5 = new Table('t5', [
new Column('c', new StringType(), [
'length' => 8,
'platformOptions' => ['charset' => 'utf8mb3'],
]),
]);
$table6 = new Table('t6', [
new Column('c', new StringType(), [
'length' => 8,
'platformOptions' => ['charset' => 'utf8mb4'],
]),
]);

self::assertEmpty($utf8Comparator->compareTables($table4, $table5)->getModifiedColumns());
self::assertEmpty($utf8Comparator->compareTables($table5, $table4)->getModifiedColumns());
self::assertNotEmpty($utf8Comparator->compareTables($table4, $table6)->getModifiedColumns());
self::assertNotEmpty($utf8Comparator->compareTables($table6, $table4)->getModifiedColumns());
self::assertNotEmpty($utf8Comparator->compareTables($table5, $table6)->getModifiedColumns());
self::assertNotEmpty($utf8Comparator->compareTables($table6, $table5)->getModifiedColumns());
}

/** @return iterable<array{bool,string}> */
public static function utf8AndUtf8mb3MismatchesProvider(): iterable
{
yield [false, 'utf8'];
yield [true, 'utf8'];
yield [false, 'utf8mb3'];
yield [true, 'utf8mb3'];
yield [false, 'utf8mb4'];
yield [true, 'utf8mb4'];
}
}
10 changes: 10 additions & 0 deletions tests/Platforms/MySQL/MariaDBJsonComparatorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,22 @@ protected function setUp(): void
$this->comparator = new Comparator(
new MariaDBPlatform(),
new class implements CharsetMetadataProvider {
public function normalizeCharset(string $charset): string
{
return $charset;
}

public function getDefaultCharsetCollation(string $charset): ?string
{
return null;
}
},
new class implements CollationMetadataProvider {
public function normalizeCollation(string $collation): string
{
return $collation;
}

public function getCollationCharset(string $collation): ?string
{
return null;
Expand Down

0 comments on commit fa707e2

Please sign in to comment.