From 7e432bada15c899c1584a35bf9e0943bc67ade75 Mon Sep 17 00:00:00 2001 From: Sergei Morozov Date: Thu, 30 Jun 2022 13:37:57 -0700 Subject: [PATCH] Infer omitted column charset from collation --- .../MySQL/CollationMetadataProvider.php | 13 ++++++ .../CachingCollationMetadataProvider.php | 35 +++++++++++++++ .../ConnectionCollationMetadataProvider.php | 45 +++++++++++++++++++ src/Platforms/MySQL/Comparator.php | 41 ++++++++++++----- src/Schema/MySQLSchemaManager.php | 9 +++- .../Schema/MySQL/ComparatorTest.php | 22 +++++++++ .../AbstractMySQLPlatformTestCase.php | 18 +++++++- .../CachingCollationMetadataProviderTest.php | 37 +++++++++++++++ tests/Platforms/MySQL/ComparatorTest.php | 11 ++++- tests/Schema/Platforms/MySQLSchemaTest.php | 11 ++++- 10 files changed, 227 insertions(+), 15 deletions(-) create mode 100644 src/Platforms/MySQL/CollationMetadataProvider.php create mode 100644 src/Platforms/MySQL/CollationMetadataProvider/CachingCollationMetadataProvider.php create mode 100644 src/Platforms/MySQL/CollationMetadataProvider/ConnectionCollationMetadataProvider.php create mode 100644 tests/Platforms/MySQL/CollationMetadataProvider/CachingCollationMetadataProviderTest.php diff --git a/src/Platforms/MySQL/CollationMetadataProvider.php b/src/Platforms/MySQL/CollationMetadataProvider.php new file mode 100644 index 00000000000..4aa0faf6c31 --- /dev/null +++ b/src/Platforms/MySQL/CollationMetadataProvider.php @@ -0,0 +1,13 @@ + */ + private $cache = []; + + public function __construct(CollationMetadataProvider $collationMetadataProvider) + { + $this->collationMetadataProvider = $collationMetadataProvider; + } + + public function getCollationCharset(string $collation): ?string + { + if (array_key_exists($collation, $this->cache)) { + return $this->cache[$collation]; + } + + return $this->cache[$collation] = $this->collationMetadataProvider->getCollationCharset($collation); + } +} diff --git a/src/Platforms/MySQL/CollationMetadataProvider/ConnectionCollationMetadataProvider.php b/src/Platforms/MySQL/CollationMetadataProvider/ConnectionCollationMetadataProvider.php new file mode 100644 index 00000000000..401c940e003 --- /dev/null +++ b/src/Platforms/MySQL/CollationMetadataProvider/ConnectionCollationMetadataProvider.php @@ -0,0 +1,45 @@ +connection = $connection; + } + + /** + * @throws Exception + */ + public function getCollationCharset(string $collation): ?string + { + $charset = $this->connection->fetchOne( + <<<'SQL' +SELECT CHARACTER_SET_NAME +FROM information_schema.COLLATIONS +WHERE COLLATION_NAME = ?; +SQL + , + [$collation] + ); + + if ($charset !== false) { + return $charset; + } + + return null; + } +} diff --git a/src/Platforms/MySQL/Comparator.php b/src/Platforms/MySQL/Comparator.php index 81c54dbc4f0..74e7fe3baf9 100644 --- a/src/Platforms/MySQL/Comparator.php +++ b/src/Platforms/MySQL/Comparator.php @@ -18,12 +18,17 @@ */ class Comparator extends BaseComparator { + /** @var CollationMetadataProvider */ + private $collationMetadataProvider; + /** * @internal The comparator can be only instantiated by a schema manager. */ - public function __construct(AbstractMySQLPlatform $platform) + public function __construct(AbstractMySQLPlatform $platform, CollationMetadataProvider $collationMetadataProvider) { parent::__construct($platform); + + $this->collationMetadataProvider = $collationMetadataProvider; } /** @@ -39,28 +44,44 @@ public function diffTable(Table $fromTable, Table $toTable) private function normalizeColumns(Table $table): Table { - $defaults = array_intersect_key($table->getOptions(), [ + $tableOptions = array_intersect_key($table->getOptions(), [ 'charset' => null, 'collation' => null, ]); - if ($defaults === []) { - return $table; - } - $table = clone $table; foreach ($table->getColumns() as $column) { - $options = $column->getPlatformOptions(); - $diff = array_diff_assoc($options, $defaults); + $originalOptions = $column->getPlatformOptions(); + $normalizedOptions = $this->normalizeOptions($originalOptions); - if ($diff === $options) { + $overrideOptions = array_diff_assoc($normalizedOptions, $tableOptions); + + if ($overrideOptions === $originalOptions) { continue; } - $column->setPlatformOptions($diff); + $column->setPlatformOptions($overrideOptions); } return $table; } + + /** + * @param array $options + * + * @return array + */ + private function normalizeOptions(array $options): array + { + if (isset($options['collation']) && ! isset($options['charset'])) { + $charset = $this->collationMetadataProvider->getCollationCharset($options['collation']); + + if ($charset !== null) { + $options['charset'] = $charset; + } + } + + return $options; + } } diff --git a/src/Schema/MySQLSchemaManager.php b/src/Schema/MySQLSchemaManager.php index 7c175bb60bf..81ba4cab638 100644 --- a/src/Schema/MySQLSchemaManager.php +++ b/src/Schema/MySQLSchemaManager.php @@ -5,6 +5,8 @@ use Doctrine\DBAL\Platforms\AbstractMySQLPlatform; use Doctrine\DBAL\Platforms\MariaDb1027Platform; use Doctrine\DBAL\Platforms\MySQL; +use Doctrine\DBAL\Platforms\MySQL\CollationMetadataProvider\CachingCollationMetadataProvider; +use Doctrine\DBAL\Platforms\MySQL\CollationMetadataProvider\ConnectionCollationMetadataProvider; use Doctrine\DBAL\Types\Type; use function array_change_key_case; @@ -363,7 +365,12 @@ public function listTableDetails($name) public function createComparator(): Comparator { - return new MySQL\Comparator($this->getDatabasePlatform()); + return new MySQL\Comparator( + $this->getDatabasePlatform(), + new CachingCollationMetadataProvider( + new ConnectionCollationMetadataProvider($this->_conn) + ) + ); } /** diff --git a/tests/Functional/Schema/MySQL/ComparatorTest.php b/tests/Functional/Schema/MySQL/ComparatorTest.php index 110e69060af..f816c7ba0c5 100644 --- a/tests/Functional/Schema/MySQL/ComparatorTest.php +++ b/tests/Functional/Schema/MySQL/ComparatorTest.php @@ -138,6 +138,28 @@ public function testChangeColumnCollation(): void ComparatorTestUtils::assertDiffNotEmpty($this->connection, $this->comparator, $table); } + public function testImplicitColumnCharset(): void + { + $table = new Table('comparator_test'); + $table->addColumn('name', Types::STRING, [ + 'length' => 32, + 'platformOptions' => ['collation' => 'utf8mb4_unicode_ci'], + ]); + $this->dropAndCreateTable($table); + + self::assertFalse(ComparatorTestUtils::diffFromActualToDesiredTable( + $this->schemaManager, + $this->comparator, + $table + )); + + self::assertFalse(ComparatorTestUtils::diffFromDesiredToActualTable( + $this->schemaManager, + $this->comparator, + $table + )); + } + /** * @return array{Table,Column} * diff --git a/tests/Platforms/AbstractMySQLPlatformTestCase.php b/tests/Platforms/AbstractMySQLPlatformTestCase.php index 1cce3cbf0f4..d131bd411d4 100644 --- a/tests/Platforms/AbstractMySQLPlatformTestCase.php +++ b/tests/Platforms/AbstractMySQLPlatformTestCase.php @@ -4,6 +4,7 @@ use Doctrine\DBAL\Platforms\AbstractPlatform; use Doctrine\DBAL\Platforms\MySQL; +use Doctrine\DBAL\Platforms\MySQL\CollationMetadataProvider; use Doctrine\DBAL\Platforms\MySQLPlatform; use Doctrine\DBAL\Schema\Comparator; use Doctrine\DBAL\Schema\ForeignKeyConstraint; @@ -795,7 +796,12 @@ public function testIgnoresDifferenceInDefaultValuesForUnsupportedColumnTypes(): $diffTable->changeColumn('def_blob', ['default' => null]); $diffTable->changeColumn('def_blob_null', ['default' => null]); - self::assertFalse((new MySQL\Comparator($this->platform))->diffTable($table, $diffTable)); + $comparator = new MySQL\Comparator( + $this->platform, + $this->createMock(CollationMetadataProvider::class) + ); + + self::assertFalse($comparator->diffTable($table, $diffTable)); } /** @@ -1052,7 +1058,15 @@ public static function comparatorProvider(): iterable ]; yield 'MySQL comparator' => [ - new MySQL\Comparator(new MySQLPlatform()), + new MySQL\Comparator( + new MySQLPlatform(), + new class implements CollationMetadataProvider { + public function getCollationCharset(string $collation): ?string + { + return null; + } + } + ), ]; } } diff --git a/tests/Platforms/MySQL/CollationMetadataProvider/CachingCollationMetadataProviderTest.php b/tests/Platforms/MySQL/CollationMetadataProvider/CachingCollationMetadataProviderTest.php new file mode 100644 index 00000000000..5d0c47233e6 --- /dev/null +++ b/tests/Platforms/MySQL/CollationMetadataProvider/CachingCollationMetadataProviderTest.php @@ -0,0 +1,37 @@ +createMock(CollationMetadataProvider::class); + $underlyingProvider->expects(self::once()) + ->method('getCollationCharset') + ->with($collation) + ->willReturn($charset); + + $cachingProvider = new CachingCollationMetadataProvider($underlyingProvider); + self::assertSame($charset, $cachingProvider->getCollationCharset($collation)); + self::assertSame($charset, $cachingProvider->getCollationCharset($collation)); + } + + /** + * @return iterable + */ + public static function charsetAndCollationProvider(): iterable + { + yield 'found' => ['utf8mb4_unicode_ci', 'utf8mb4']; + yield 'not found' => ['utf8mb5_unicode_ci', null]; + } +} diff --git a/tests/Platforms/MySQL/ComparatorTest.php b/tests/Platforms/MySQL/ComparatorTest.php index 54dea32dc80..e677dd68e5c 100644 --- a/tests/Platforms/MySQL/ComparatorTest.php +++ b/tests/Platforms/MySQL/ComparatorTest.php @@ -2,6 +2,7 @@ namespace Doctrine\DBAL\Tests\Platforms\MySQL; +use Doctrine\DBAL\Platforms\MySQL\CollationMetadataProvider; use Doctrine\DBAL\Platforms\MySQL\Comparator; use Doctrine\DBAL\Platforms\MySQLPlatform; use Doctrine\DBAL\Tests\Schema\ComparatorTest as BaseComparatorTest; @@ -10,6 +11,14 @@ class ComparatorTest extends BaseComparatorTest { protected function setUp(): void { - $this->comparator = new Comparator(new MySQLPlatform()); + $this->comparator = new Comparator( + new MySQLPlatform(), + new class implements CollationMetadataProvider { + public function getCollationCharset(string $collation): ?string + { + return null; + } + } + ); } } diff --git a/tests/Schema/Platforms/MySQLSchemaTest.php b/tests/Schema/Platforms/MySQLSchemaTest.php index 73236f3e2e9..69145229f0d 100644 --- a/tests/Schema/Platforms/MySQLSchemaTest.php +++ b/tests/Schema/Platforms/MySQLSchemaTest.php @@ -4,6 +4,7 @@ use Doctrine\DBAL\Platforms\AbstractPlatform; use Doctrine\DBAL\Platforms\MySQL; +use Doctrine\DBAL\Platforms\MySQL\CollationMetadataProvider; use Doctrine\DBAL\Platforms\MySQLPlatform; use Doctrine\DBAL\Schema\Comparator; use Doctrine\DBAL\Schema\Table; @@ -99,7 +100,15 @@ public static function comparatorProvider(): iterable ]; yield 'MySQL comparator' => [ - new MySQL\Comparator(new MySQLPlatform()), + new MySQL\Comparator( + new MySQLPlatform(), + new class implements CollationMetadataProvider { + public function getCollationCharset(string $collation): ?string + { + return null; + } + } + ), ]; } }