Skip to content

Commit

Permalink
Add comparison of foreign key names with option to opt-out of this (f…
Browse files Browse the repository at this point in the history
…ixes #6518).
  • Loading branch information
uncaught committed Sep 6, 2024
1 parent 0e81fa2 commit d7c47bf
Show file tree
Hide file tree
Showing 20 changed files with 97 additions and 22 deletions.
19 changes: 19 additions & 0 deletions src/Configuration.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,15 @@ class Configuration

private ?SchemaManagerFactory $schemaManagerFactory = null;

/**
* Whether changes in the foreign key names should be compared.
* 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.
* This could lead to incompatible migrations that try to drop non-existent foreign keys.
*/
private bool $compareForeignKeyNames = false;

public function __construct()
{
$this->schemaAssetsFilter = static function (): bool {
Expand Down Expand Up @@ -153,4 +162,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;
}
}
4 changes: 3 additions & 1 deletion src/Platforms/MySQL/Comparator.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace Doctrine\DBAL\Platforms\MySQL;

use Doctrine\DBAL\Configuration;
use Doctrine\DBAL\Platforms\AbstractMySQLPlatform;
use Doctrine\DBAL\Schema\Comparator as BaseComparator;
use Doctrine\DBAL\Schema\Table;
Expand All @@ -23,11 +24,12 @@ class Comparator extends BaseComparator
/** @internal The comparator can be only instantiated by a schema manager. */
public function __construct(
AbstractMySQLPlatform $platform,
Configuration $configuration,
private readonly CharsetMetadataProvider $charsetMetadataProvider,
private readonly CollationMetadataProvider $collationMetadataProvider,
private readonly DefaultTableOptions $defaultTableOptions,
) {
parent::__construct($platform);
parent::__construct($platform, $configuration);
}

public function compareTables(Table $oldTable, Table $newTable): TableDiff
Expand Down
10 changes: 7 additions & 3 deletions src/Platforms/SQLServer/Comparator.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace Doctrine\DBAL\Platforms\SQLServer;

use Doctrine\DBAL\Configuration;
use Doctrine\DBAL\Platforms\SQLServerPlatform;
use Doctrine\DBAL\Schema\Comparator as BaseComparator;
use Doctrine\DBAL\Schema\Table;
Expand All @@ -17,9 +18,12 @@
class Comparator extends BaseComparator
{
/** @internal The comparator can be only instantiated by a schema manager. */
public function __construct(SQLServerPlatform $platform, private readonly string $databaseCollation)
{
parent::__construct($platform);
public function __construct(
SQLServerPlatform $platform,
Configuration $configuration,
private readonly string $databaseCollation,
) {
parent::__construct($platform, $configuration);
}

public function compareTables(Table $oldTable, Table $newTable): TableDiff
Expand Down
5 changes: 3 additions & 2 deletions src/Platforms/SQLite/Comparator.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace Doctrine\DBAL\Platforms\SQLite;

use Doctrine\DBAL\Configuration;
use Doctrine\DBAL\Platforms\SQLitePlatform;
use Doctrine\DBAL\Schema\Comparator as BaseComparator;
use Doctrine\DBAL\Schema\Table;
Expand All @@ -19,9 +20,9 @@
class Comparator extends BaseComparator
{
/** @internal The comparator can be only instantiated by a schema manager. */
public function __construct(SQLitePlatform $platform)
public function __construct(SQLitePlatform $platform, Configuration $configuration)
{
parent::__construct($platform);
parent::__construct($platform, $configuration);
}

public function compareTables(Table $oldTable, Table $newTable): TableDiff
Expand Down
2 changes: 1 addition & 1 deletion src/Schema/AbstractSchemaManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -842,7 +842,7 @@ private function getDatabase(string $methodName): string

public function createComparator(): Comparator
{
return new Comparator($this->platform);
return new Comparator($this->platform, $this->connection->getConfiguration());

Check warning on line 845 in src/Schema/AbstractSchemaManager.php

View check run for this annotation

Codecov / codecov/patch

src/Schema/AbstractSchemaManager.php#L845

Added line #L845 was not covered by tests
}

/**
Expand Down
14 changes: 12 additions & 2 deletions src/Schema/Comparator.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace Doctrine\DBAL\Schema;

use Doctrine\DBAL\Configuration;
use Doctrine\DBAL\Platforms\AbstractPlatform;

use function array_map;
Expand All @@ -17,8 +18,10 @@
class Comparator
{
/** @internal The comparator can be only instantiated by a schema manager. */
public function __construct(private readonly AbstractPlatform $platform)
{
public function __construct(
private readonly AbstractPlatform $platform,
private readonly Configuration $configuration,
) {
}

/**
Expand Down Expand Up @@ -389,6 +392,13 @@ private function detectRenamedIndexes(array &$addedIndexes, array &$removedIndex

protected function diffForeignKey(ForeignKeyConstraint $key1, ForeignKeyConstraint $key2): bool
{
if (
$this->configuration->getCompareForeignKeyNames()
&& strtolower($key1->getName()) !== strtolower($key2->getName())
) {
return true;
}

if (
array_map('strtolower', $key1->getUnquotedLocalColumns())
!== array_map('strtolower', $key2->getUnquotedLocalColumns())
Expand Down
1 change: 1 addition & 0 deletions src/Schema/MySQLSchemaManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,7 @@ public function createComparator(): Comparator
{
return new MySQL\Comparator(
$this->platform,
$this->connection->getConfiguration(),

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

View check run for this annotation

Codecov / codecov/patch

src/Schema/MySQLSchemaManager.php#L321

Added line #L321 was not covered by tests
new CachingCharsetMetadataProvider(
new ConnectionCharsetMetadataProvider($this->connection),
),
Expand Down
3 changes: 2 additions & 1 deletion src/Schema/SQLServerSchemaManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace Doctrine\DBAL\Schema;

use Doctrine\DBAL\Configuration;
use Doctrine\DBAL\Exception;
use Doctrine\DBAL\Platforms\SQLServer;
use Doctrine\DBAL\Platforms\SQLServerPlatform;
Expand Down Expand Up @@ -264,7 +265,7 @@ protected function _getPortableViewDefinition(array $view): View
/** @throws Exception */
public function createComparator(): Comparator
{
return new SQLServer\Comparator($this->platform, $this->getDatabaseCollation());
return new SQLServer\Comparator($this->platform, new Configuration(), $this->getDatabaseCollation());
}

/** @throws Exception */
Expand Down
3 changes: 2 additions & 1 deletion src/Schema/SQLiteSchemaManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace Doctrine\DBAL\Schema;

use Doctrine\DBAL\Configuration;
use Doctrine\DBAL\Exception;
use Doctrine\DBAL\Platforms\SQLite;
use Doctrine\DBAL\Platforms\SQLitePlatform;
Expand Down Expand Up @@ -498,7 +499,7 @@ private function getForeignKeyDetails(string $table): array

public function createComparator(): Comparator
{
return new SQLite\Comparator($this->platform);
return new SQLite\Comparator($this->platform, new Configuration());

Check warning on line 502 in src/Schema/SQLiteSchemaManager.php

View check run for this annotation

Codecov / codecov/patch

src/Schema/SQLiteSchemaManager.php#L502

Added line #L502 was not covered by tests
}

protected function selectTableNames(string $databaseName): Result
Expand Down
3 changes: 2 additions & 1 deletion tests/Functional/Schema/ComparatorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace Doctrine\DBAL\Tests\Functional\Schema;

use Doctrine\DBAL\Configuration;
use Doctrine\DBAL\Platforms\AbstractMySQLPlatform;
use Doctrine\DBAL\Platforms\MariaDBPlatform;
use Doctrine\DBAL\Schema\AbstractSchemaManager;
Expand Down Expand Up @@ -53,7 +54,7 @@ public function testDefaultValueComparison(string $type, mixed $value): void
public function testRenameColumnComparison(): void
{
$platform = $this->connection->getDatabasePlatform();
$comparator = new Comparator($platform);
$comparator = new Comparator($platform, new Configuration());

$table = new Table('rename_table');
$table->addColumn('test', Types::STRING, ['default' => 'baz', 'length' => 20]);
Expand Down
2 changes: 2 additions & 0 deletions tests/Platforms/AbstractMySQLPlatformTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace Doctrine\DBAL\Tests\Platforms;

use Doctrine\DBAL\Configuration;
use Doctrine\DBAL\Exception\InvalidColumnDeclaration;
use Doctrine\DBAL\Platforms\AbstractMySQLPlatform;
use Doctrine\DBAL\Platforms\MySQL;
Expand Down Expand Up @@ -689,6 +690,7 @@ protected function createComparator(): Comparator
{
return new MySQL\Comparator(
$this->platform,
new Configuration(),
self::createStub(CharsetMetadataProvider::class),
self::createStub(CollationMetadataProvider::class),
new DefaultTableOptions('utf8mb4', 'utf8mb4_general_ci'),
Expand Down
3 changes: 2 additions & 1 deletion tests/Platforms/AbstractPlatformTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace Doctrine\DBAL\Tests\Platforms;

use Doctrine\DBAL\Configuration;
use Doctrine\DBAL\Exception;
use Doctrine\DBAL\Exception\InvalidColumnDeclaration;
use Doctrine\DBAL\Platforms\AbstractPlatform;
Expand Down Expand Up @@ -40,7 +41,7 @@ protected function setUp(): void

protected function createComparator(): Comparator
{
return new Comparator($this->platform);
return new Comparator($this->platform, new Configuration());
}

public function testQuoteIdentifier(): void
Expand Down
6 changes: 4 additions & 2 deletions tests/Platforms/MySQL/ComparatorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace Doctrine\DBAL\Tests\Platforms\MySQL;

use Doctrine\DBAL\Configuration;
use Doctrine\DBAL\Platforms\MySQL\CharsetMetadataProvider;
use Doctrine\DBAL\Platforms\MySQL\CollationMetadataProvider;
use Doctrine\DBAL\Platforms\MySQL\Comparator;
Expand All @@ -13,10 +14,11 @@

class ComparatorTest extends AbstractComparatorTestCase
{
protected function setUp(): void
protected function createComparator(?Configuration $configuration = null): Comparator
{
$this->comparator = new Comparator(
return new Comparator(
new MySQLPlatform(),
$configuration ?? new Configuration(),
self::createStub(CharsetMetadataProvider::class),
self::createStub(CollationMetadataProvider::class),
new DefaultTableOptions('utf8mb4', 'utf8mb4_general_ci'),
Expand Down
2 changes: 2 additions & 0 deletions tests/Platforms/MySQL/MariaDBJsonComparatorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace Doctrine\DBAL\Tests\Platforms\MySQL;

use Doctrine\DBAL\Configuration;
use Doctrine\DBAL\Platforms\MariaDBPlatform;
use Doctrine\DBAL\Platforms\MySQL\CharsetMetadataProvider;
use Doctrine\DBAL\Platforms\MySQL\CollationMetadataProvider;
Expand All @@ -27,6 +28,7 @@ protected function setUp(): void
{
$this->comparator = new Comparator(
new MariaDBPlatform(),
new Configuration(),
new class implements CharsetMetadataProvider {
public function getDefaultCharsetCollation(string $charset): ?string
{
Expand Down
5 changes: 3 additions & 2 deletions tests/Platforms/SQLServer/ComparatorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,15 @@

namespace Doctrine\DBAL\Tests\Platforms\SQLServer;

use Doctrine\DBAL\Configuration;
use Doctrine\DBAL\Platforms\SQLServer\Comparator;
use Doctrine\DBAL\Platforms\SQLServerPlatform;
use Doctrine\DBAL\Tests\Schema\AbstractComparatorTestCase;

class ComparatorTest extends AbstractComparatorTestCase
{
protected function setUp(): void
protected function createComparator(?Configuration $configuration = null): Comparator
{
$this->comparator = new Comparator(new SQLServerPlatform(), '');
return new Comparator(new SQLServerPlatform(), $configuration ?? new Configuration(), '');
}
}
3 changes: 2 additions & 1 deletion tests/Platforms/SQLServerPlatformTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace Doctrine\DBAL\Tests\Platforms;

use Doctrine\DBAL\Configuration;
use Doctrine\DBAL\Exception;
use Doctrine\DBAL\Exception\InvalidColumnDeclaration;
use Doctrine\DBAL\LockMode;
Expand Down Expand Up @@ -32,7 +33,7 @@ public function createPlatform(): AbstractPlatform

protected function createComparator(): Comparator
{
return new SQLServer\Comparator($this->platform, '');
return new SQLServer\Comparator($this->platform, new Configuration(), '');
}

public function getGenerateTableSql(): string
Expand Down
5 changes: 3 additions & 2 deletions tests/Platforms/SQLite/ComparatorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,16 @@

namespace Doctrine\DBAL\Tests\Platforms\SQLite;

use Doctrine\DBAL\Configuration;
use Doctrine\DBAL\Platforms\SQLite\Comparator;
use Doctrine\DBAL\Platforms\SQLitePlatform;
use Doctrine\DBAL\Tests\Schema\AbstractComparatorTestCase;

class ComparatorTest extends AbstractComparatorTestCase
{
protected function setUp(): void
protected function createComparator(?Configuration $configuration = null): Comparator
{
$this->comparator = new Comparator(new SQLitePlatform());
return new Comparator(new SQLitePlatform(), $configuration ?? new Configuration());
}

public function testCompareChangedBinaryColumn(): void
Expand Down
3 changes: 2 additions & 1 deletion tests/Platforms/SQLitePlatformTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace Doctrine\DBAL\Tests\Platforms;

use Doctrine\DBAL\Configuration;
use Doctrine\DBAL\Exception;
use Doctrine\DBAL\Platforms\AbstractPlatform;
use Doctrine\DBAL\Platforms\SQLite;
Expand All @@ -30,7 +31,7 @@ public function createPlatform(): AbstractPlatform

protected function createComparator(): Comparator
{
return new SQLite\Comparator($this->platform);
return new SQLite\Comparator($this->platform, new Configuration());
}

public function getGenerateTableSql(): string
Expand Down
24 changes: 23 additions & 1 deletion tests/Schema/AbstractComparatorTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace Doctrine\DBAL\Tests\Schema;

use Doctrine\DBAL\Configuration;
use Doctrine\DBAL\Schema\AbstractAsset;
use Doctrine\DBAL\Schema\Column;
use Doctrine\DBAL\Schema\ColumnDiff;
Expand All @@ -29,6 +30,13 @@ abstract class AbstractComparatorTestCase extends TestCase
{
protected Comparator $comparator;

protected function setUp(): void
{
$this->comparator = $this->createComparator();
}

abstract protected function createComparator(?Configuration $configuration = null): Comparator;

public function testCompareSame1(): void
{
$schema1 = new Schema([
Expand Down Expand Up @@ -365,7 +373,7 @@ public function testCompareIndexBasedOnPropertiesNotName(): void
);
}

public function testCompareForeignKeyBasedOnPropertiesNotName(): void
public function testDetectForeignKeyNameChange(): void
{
$tableA = new Table('foo');
$tableA->addColumn('id', Types::INTEGER);
Expand All @@ -379,6 +387,20 @@ public function testCompareForeignKeyBasedOnPropertiesNotName(): void
new TableDiff($tableA),
$this->comparator->compareTables($tableA, $tableB),
);

// With enabled name comparison:
$configWithEnabledFkNames = new Configuration();
$configWithEnabledFkNames->setCompareForeignKeyNames(true);
$comparatorEnabledNameComparison = $this->createComparator($configWithEnabledFkNames);

self::assertEquals(
new TableDiff(
oldTable: $tableA,
addedForeignKeys: [new ForeignKeyConstraint(['id'], 'bar', ['id'], 'bar_constraint')],
droppedForeignKeys: [new ForeignKeyConstraint(['id'], 'bar', ['id'], 'foo_constraint')],
),
$comparatorEnabledNameComparison->compareTables($tableA, $tableB),
);
}

public function testDetectRenameColumn(): void
Expand Down
Loading

0 comments on commit d7c47bf

Please sign in to comment.