From 67305ee5430c06807d29ef358083d3266e4e9185 Mon Sep 17 00:00:00 2001 From: Christoph Krapp Date: Tue, 28 May 2024 23:47:20 +0200 Subject: [PATCH 1/3] Fix test names to reflect their actual purpose Both tests did expect a name change to result in an empty table diff in the past. When index renaming support was introduced the name of the test was not changed to reflect the new expectation. Signed-off-by: Christoph Krapp --- tests/Schema/AbstractComparatorTestCase.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Schema/AbstractComparatorTestCase.php b/tests/Schema/AbstractComparatorTestCase.php index a087202628f..0c89f7541ab 100644 --- a/tests/Schema/AbstractComparatorTestCase.php +++ b/tests/Schema/AbstractComparatorTestCase.php @@ -652,7 +652,7 @@ public function testCompareColumnCompareCaseInsensitive(): void self::assertFalse($tableDiff); } - public function testCompareIndexBasedOnPropertiesNotName(): void + public function testDetectIndexNameChange(): void { $tableA = new Table('foo'); $tableA->addColumn('id', Types::INTEGER); @@ -672,7 +672,7 @@ public function testCompareIndexBasedOnPropertiesNotName(): void ); } - public function testCompareForeignKeyBasedOnPropertiesNotName(): void + public function testDetectForeignKeyNameChange(): void { $tableA = new Table('foo'); $tableA->addColumn('id', Types::INTEGER); From 57c98dadc96f13683f611f9138afec4282b6b784 Mon Sep 17 00:00:00 2001 From: Christoph Krapp Date: Tue, 28 May 2024 23:39:47 +0200 Subject: [PATCH 2/3] Fix fk name change detection in schema comparator As index renaming support was introduced a while back do the same for foreign key name changes. Signed-off-by: Christoph Krapp --- src/Schema/Comparator.php | 4 ++++ tests/Schema/AbstractComparatorTestCase.php | 14 +++++++++++--- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/src/Schema/Comparator.php b/src/Schema/Comparator.php index 28e7f2f73b2..804c981702b 100644 --- a/src/Schema/Comparator.php +++ b/src/Schema/Comparator.php @@ -555,6 +555,10 @@ private function detectRenamedIndexes(array &$addedIndexes, array &$removedIndex */ public function diffForeignKey(ForeignKeyConstraint $key1, ForeignKeyConstraint $key2) { + if (strtolower($key1->getName()) !== strtolower($key2->getName())) { + return true; + } + if ( array_map('strtolower', $key1->getUnquotedLocalColumns()) !== array_map('strtolower', $key2->getUnquotedLocalColumns()) diff --git a/tests/Schema/AbstractComparatorTestCase.php b/tests/Schema/AbstractComparatorTestCase.php index 0c89f7541ab..656dec9c252 100644 --- a/tests/Schema/AbstractComparatorTestCase.php +++ b/tests/Schema/AbstractComparatorTestCase.php @@ -682,9 +682,17 @@ public function testDetectForeignKeyNameChange(): void $tableB->addColumn('ID', Types::INTEGER); $tableB->addForeignKeyConstraint('bar', ['id'], ['id'], [], 'bar_constraint'); - $tableDiff = $this->comparator->diffTable($tableA, $tableB); - - self::assertFalse($tableDiff); + $fkA = new ForeignKeyConstraint(['id'], 'bar', ['id'], 'foo_constraint'); + $fkA->setLocalTable($tableA); + $fkB = new ForeignKeyConstraint(['id'], 'bar', ['id'], 'bar_constraint'); + $fkB->setLocalTable($tableB); + $tableDiff = new TableDiff( + tableName: 'foo', + fromTable: $tableA, + addedForeignKeys: [$fkB], + removedForeignKeys: [$fkA], + ); + self::assertEquals($tableDiff, $this->comparator->compareTables($tableA, $tableB)); } public function testCompareForeignKeyRestrictNoActionAreTheSame(): void From d041e232279ac2fb4d19ccdda38b8bab46813735 Mon Sep 17 00:00:00 2001 From: uncaught Date: Fri, 6 Sep 2024 11:33:31 +0200 Subject: [PATCH 3/3] Allow to opt-out of foreign key name comparison (fixes #6518) --- src/Configuration.php | 18 ++++++++++++++++ src/Connection.php | 2 ++ src/Platforms/AbstractPlatform.php | 14 +++++++++++++ src/Schema/Comparator.php | 5 ++++- tests/Schema/AbstractComparatorTestCase.php | 23 +++++++++++++++------ 5 files changed, 55 insertions(+), 7 deletions(-) diff --git a/src/Configuration.php b/src/Configuration.php index 5cdae81354d..d89c549937f 100644 --- a/src/Configuration.php +++ b/src/Configuration.php @@ -64,6 +64,14 @@ class Configuration */ private bool $disableTypeComments = false; + /** + * Whether changes in the foreign key names should lead to a schema change. + * If you opt-out of this, you need to handle name changes of foreign keys yourself. + * Databases created based on the current schema might have different foreign key names + * than those migrated from older schemas if you turn this off. + */ + private bool $compareForeignKeyNames = true; + private ?SchemaManagerFactory $schemaManagerFactory = null; public function __construct() @@ -262,4 +270,14 @@ public function setDisableTypeComments(bool $disableTypeComments): self return $this; } + + public function getCompareForeignKeyNames(): bool + { + return $this->compareForeignKeyNames; + } + + public function setCompareForeignKeyNames(bool $compareForeignKeyNames): void + { + $this->compareForeignKeyNames = $compareForeignKeyNames; + } } diff --git a/src/Connection.php b/src/Connection.php index b9756706061..28ea8cc2220 100644 --- a/src/Connection.php +++ b/src/Connection.php @@ -211,6 +211,7 @@ public function __construct( $this->platform = $params['platform']; $this->platform->setEventManager($this->_eventManager); $this->platform->setDisableTypeComments($config->getDisableTypeComments()); + $this->platform->setCompareForeignKeyNames($config->getCompareForeignKeyNames()); } $this->_expr = $this->createExpressionBuilder(); @@ -318,6 +319,7 @@ public function getDatabasePlatform() $this->platform = $this->detectDatabasePlatform(); $this->platform->setEventManager($this->_eventManager); $this->platform->setDisableTypeComments($this->_config->getDisableTypeComments()); + $this->platform->setCompareForeignKeyNames($this->_config->getCompareForeignKeyNames()); } return $this->platform; diff --git a/src/Platforms/AbstractPlatform.php b/src/Platforms/AbstractPlatform.php index 928a5a02121..6d9a8a53fd6 100644 --- a/src/Platforms/AbstractPlatform.php +++ b/src/Platforms/AbstractPlatform.php @@ -107,12 +107,26 @@ abstract class AbstractPlatform private bool $disableTypeComments = false; + private bool $compareForeignKeyNames = true; + /** @internal */ final public function setDisableTypeComments(bool $value): void { $this->disableTypeComments = $value; } + /** @internal */ + final public function setCompareForeignKeyNames(bool $compare): void + { + $this->compareForeignKeyNames = $compare; + } + + /** @internal */ + public function getCompareForeignKeyNames(): bool + { + return $this->compareForeignKeyNames; + } + /** * Sets the EventManager used by the Platform. * diff --git a/src/Schema/Comparator.php b/src/Schema/Comparator.php index 804c981702b..f8c3e0d6e12 100644 --- a/src/Schema/Comparator.php +++ b/src/Schema/Comparator.php @@ -555,7 +555,10 @@ private function detectRenamedIndexes(array &$addedIndexes, array &$removedIndex */ public function diffForeignKey(ForeignKeyConstraint $key1, ForeignKeyConstraint $key2) { - if (strtolower($key1->getName()) !== strtolower($key2->getName())) { + if ( + (!$this->platform || $this->platform->getCompareForeignKeyNames()) + && strtolower($key1->getName()) !== strtolower($key2->getName()) + ) { return true; } diff --git a/tests/Schema/AbstractComparatorTestCase.php b/tests/Schema/AbstractComparatorTestCase.php index 656dec9c252..a8224f41765 100644 --- a/tests/Schema/AbstractComparatorTestCase.php +++ b/tests/Schema/AbstractComparatorTestCase.php @@ -2,6 +2,7 @@ namespace Doctrine\DBAL\Tests\Schema; +use Doctrine\DBAL\Platforms\AbstractPlatform; use Doctrine\DBAL\Schema\Column; use Doctrine\DBAL\Schema\ColumnDiff; use Doctrine\DBAL\Schema\Comparator; @@ -686,15 +687,25 @@ public function testDetectForeignKeyNameChange(): void $fkA->setLocalTable($tableA); $fkB = new ForeignKeyConstraint(['id'], 'bar', ['id'], 'bar_constraint'); $fkB->setLocalTable($tableB); - $tableDiff = new TableDiff( - tableName: 'foo', - fromTable: $tableA, - addedForeignKeys: [$fkB], - removedForeignKeys: [$fkA], - ); + $tableDiff = new TableDiff('foo', [], [], [], [], [], [], $tableA, [$fkB], [], [$fkA]); self::assertEquals($tableDiff, $this->comparator->compareTables($tableA, $tableB)); } + public function testCompareForeignKeyNameChangeDisabled(): void + { + $fk1 = new ForeignKeyConstraint(['foo'], 'bar', ['baz'], 'fk1'); + $fk2 = new ForeignKeyConstraint(['foo'], 'bar', ['baz'], 'fk2'); + + //Not disabled: + self::assertTrue($this->comparator->diffForeignKey($fk1, $fk2)); + + //Disabled foreign key name comparison: + $platform = $this->createMock(AbstractPlatform::class); + $platform->method('getCompareForeignKeyNames')->willReturn(false); + $comparatorDisabledFkName = new Comparator($platform); + self::assertFalse($comparatorDisabledFkName->diffForeignKey($fk1, $fk2)); + } + public function testCompareForeignKeyRestrictNoActionAreTheSame(): void { $fk1 = new ForeignKeyConstraint(['foo'], 'bar', ['baz'], 'fk1', ['onDelete' => 'NO ACTION']);