From 200a8fb05081d3628cb9a04b9f018ebd89293f53 Mon Sep 17 00:00:00 2001 From: Sergei Morozov Date: Sun, 16 Oct 2022 15:54:04 -0700 Subject: [PATCH 1/2] Add Comparator::compareTables(), deprecate Comparator::diffTable() --- UPGRADE.md | 30 +++++++++++++ psalm.xml.dist | 4 ++ src/Schema/Comparator.php | 46 +++++++++++--------- src/Schema/TableDiff.php | 19 ++++++++ tests/Platforms/AbstractPlatformTestCase.php | 8 ++++ 5 files changed, 86 insertions(+), 21 deletions(-) diff --git a/UPGRADE.md b/UPGRADE.md index e2c06fca04a..e062bffbafd 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -121,6 +121,36 @@ The "unique" and "check" column properties have been deprecated. Use unique cons Relying on the default precision and scale of decimal columns provided by the DBAL is deprecated. When declaring decimal columns, specify the precision and scale explicitly. +## Deprecated `Comparator::diffTable()` method. + +The `Comparator::diffTable()` method has been deprecated in favor of `Comparator::compareTables()` +and `TableDiff::isEmpty()`. + +Instead of having to check whether the diff is equal to the boolean `false`, you can optionally check +if the returned table diff is empty. + +### Before + +```php +$diff = $comparator->diffTable($oldTable, $newTable); + +// mandatory check +if ($diff !== false) { + // we have a diff +} +``` + +### After + +```php +$diff = $comparator->compareTables($oldTable, $newTable); + +// optional check +if (! $diff->isEmpty()) { + // we have a diff +} +``` + ## Deprecated not passing `$fromColumn` to the `TableDiff` constructor. Not passing `$fromColumn` to the `TableDiff` constructor has been deprecated. diff --git a/psalm.xml.dist b/psalm.xml.dist index c6c7505e2d8..38704769a8a 100644 --- a/psalm.xml.dist +++ b/psalm.xml.dist @@ -446,6 +446,10 @@ TODO: remove in 4.0.0 --> + + diff --git a/src/Schema/Comparator.php b/src/Schema/Comparator.php index d0bb6d2e3fd..01c0abcdd0c 100644 --- a/src/Schema/Comparator.php +++ b/src/Schema/Comparator.php @@ -280,14 +280,37 @@ public function diffSequence(Sequence $sequence1, Sequence $sequence2) * * If there are no differences this method returns the boolean false. * + * @deprecated Use {@see compareTables()} and, optionally, {@see TableDiff::isEmpty()} instead. + * * @return TableDiff|false * * @throws Exception */ public function diffTable(Table $fromTable, Table $toTable) { - $hasChanges = false; + Deprecation::triggerIfCalledFromOutside( + 'doctrine/dbal', + 'https://github.com/doctrine/dbal/pull/5770', + '%s is deprecated. Use compareTables() instead.', + __METHOD__, + ); + + $diff = $this->compareTables($fromTable, $toTable); + if ($diff->isEmpty()) { + return false; + } + + return $diff; + } + + /** + * Compares the tables and returns the difference between them. + * + * @throws Exception + */ + public function compareTables(Table $fromTable, Table $toTable): TableDiff + { $addedColumns = []; $modifiedColumns = []; $droppedColumns = []; @@ -308,8 +331,6 @@ public function diffTable(Table $fromTable, Table $toTable) } $addedColumns[$columnName] = $column; - - $hasChanges = true; } /* See if there are any removed columns in "to" table */ @@ -318,7 +339,6 @@ public function diffTable(Table $fromTable, Table $toTable) if (! $toTable->hasColumn($columnName)) { $droppedColumns[$columnName] = $column; - $hasChanges = true; continue; } @@ -341,8 +361,6 @@ public function diffTable(Table $fromTable, Table $toTable) $changedProperties, $column, ); - - $hasChanges = true; } $renamedColumns = $this->detectRenamedColumns($addedColumns, $droppedColumns); @@ -357,8 +375,6 @@ public function diffTable(Table $fromTable, Table $toTable) } $addedIndexes[$indexName] = $index; - - $hasChanges = true; } /* See if there are any removed indexes in "to" table */ @@ -370,7 +386,6 @@ public function diffTable(Table $fromTable, Table $toTable) ) { $droppedIndexes[$indexName] = $index; - $hasChanges = true; continue; } @@ -383,8 +398,6 @@ public function diffTable(Table $fromTable, Table $toTable) } $modifiedIndexes[$indexName] = $toTableIndex; - - $hasChanges = true; } $renamedIndexes = $this->detectRenamedIndexes($addedIndexes, $droppedIndexes); @@ -400,7 +413,6 @@ public function diffTable(Table $fromTable, Table $toTable) if (strtolower($fromConstraint->getName()) === strtolower($toConstraint->getName())) { $modifiedForeignKeys[] = $toConstraint; - $hasChanges = true; unset($fromForeignKeys[$fromKey], $toForeignKeys[$toKey]); } } @@ -409,18 +421,10 @@ public function diffTable(Table $fromTable, Table $toTable) foreach ($fromForeignKeys as $fromConstraint) { $droppedForeignKeys[] = $fromConstraint; - - $hasChanges = true; } foreach ($toForeignKeys as $toConstraint) { $addedForeignKeys[] = $toConstraint; - - $hasChanges = true; - } - - if (! $hasChanges) { - return false; } return new TableDiff( @@ -598,7 +602,7 @@ public function columnsEqual(Column $column1, Column $column2): bool * If there are differences this method returns the changed properties as a * string array, otherwise an empty array gets returned. * - * @deprecated Use {@see diffTable()} instead. + * @deprecated Use {@see columnsEqual()} instead. * * @return string[] */ diff --git a/src/Schema/TableDiff.php b/src/Schema/TableDiff.php index 22ee6b21ec6..58128d75b3e 100644 --- a/src/Schema/TableDiff.php +++ b/src/Schema/TableDiff.php @@ -7,6 +7,7 @@ use function array_filter; use function array_values; +use function count; /** * Table Diff. @@ -339,4 +340,22 @@ static function ($removedForeignKey) use ($foreignKey): bool { }, ); } + + /** + * Returns whether the diff is empty (contains no changes). + */ + public function isEmpty(): bool + { + return count($this->addedColumns) === 0 + && count($this->changedColumns) === 0 + && count($this->removedColumns) === 0 + && count($this->renamedColumns) === 0 + && count($this->addedIndexes) === 0 + && count($this->changedIndexes) === 0 + && count($this->removedIndexes) === 0 + && count($this->renamedIndexes) === 0 + && count($this->addedForeignKeys) === 0 + && count($this->changedForeignKeys) === 0 + && count($this->removedForeignKeys) === 0; + } } diff --git a/tests/Platforms/AbstractPlatformTestCase.php b/tests/Platforms/AbstractPlatformTestCase.php index 6741c818ab8..1e649537a22 100644 --- a/tests/Platforms/AbstractPlatformTestCase.php +++ b/tests/Platforms/AbstractPlatformTestCase.php @@ -1382,6 +1382,14 @@ public function requiresSQLCommentHint(AbstractPlatform $platform): bool self::assertTrue($this->platform->isCommentedDoctrineType($type)); } + public function testEmptyTableDiff(): void + { + $diff = new TableDiff('test'); + + self::assertTrue($diff->isEmpty()); + self::assertSame([], $this->platform->getAlterTableSQL($diff)); + } + public function tearDown(): void { if (! isset($this->backedUpType)) { From acc2c8a9b511e8063fb79ea4b514e91ed31c4669 Mon Sep 17 00:00:00 2001 From: Sergei Morozov Date: Mon, 17 Oct 2022 13:17:23 -0700 Subject: [PATCH 2/2] Add SchemaDiff::isEmpty() --- src/Schema/SchemaDiff.php | 25 ++++++++++++++++++-- tests/Platforms/AbstractPlatformTestCase.php | 9 +++++++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/src/Schema/SchemaDiff.php b/src/Schema/SchemaDiff.php index 82bc0c2f753..a6a866579cf 100644 --- a/src/Schema/SchemaDiff.php +++ b/src/Schema/SchemaDiff.php @@ -5,7 +5,9 @@ use Doctrine\DBAL\Platforms\AbstractPlatform; use Doctrine\Deprecations\Deprecation; +use function array_filter; use function array_merge; +use function count; /** * Differences between two schemas. @@ -120,8 +122,12 @@ public function __construct( $alteredSequences = [], $droppedSequences = [] ) { - $this->newTables = $newTables; - $this->changedTables = $changedTables; + $this->newTables = $newTables; + + $this->changedTables = array_filter($changedTables, static function (TableDiff $diff): bool { + return ! $diff->isEmpty(); + }); + $this->removedTables = $removedTables; $this->fromSchema = $fromSchema; $this->newNamespaces = $createdSchemas; @@ -179,6 +185,21 @@ public function getDroppedSequences(): array return $this->removedSequences; } + /** + * Returns whether the diff is empty (contains no changes). + */ + public function isEmpty(): bool + { + return count($this->newNamespaces) === 0 + && count($this->removedNamespaces) === 0 + && count($this->newTables) === 0 + && count($this->changedTables) === 0 + && count($this->removedTables) === 0 + && count($this->newSequences) === 0 + && count($this->changedSequences) === 0 + && count($this->removedSequences) === 0; + } + /** * The to save sql mode ensures that the following things don't happen: * diff --git a/tests/Platforms/AbstractPlatformTestCase.php b/tests/Platforms/AbstractPlatformTestCase.php index 1e649537a22..1fff8734a01 100644 --- a/tests/Platforms/AbstractPlatformTestCase.php +++ b/tests/Platforms/AbstractPlatformTestCase.php @@ -14,6 +14,7 @@ use Doctrine\DBAL\Schema\Comparator; use Doctrine\DBAL\Schema\ForeignKeyConstraint; use Doctrine\DBAL\Schema\Index; +use Doctrine\DBAL\Schema\SchemaDiff; use Doctrine\DBAL\Schema\Table; use Doctrine\DBAL\Schema\TableDiff; use Doctrine\DBAL\Schema\UniqueConstraint; @@ -1390,6 +1391,14 @@ public function testEmptyTableDiff(): void self::assertSame([], $this->platform->getAlterTableSQL($diff)); } + public function testEmptySchemaDiff(): void + { + $diff = new SchemaDiff(); + + self::assertTrue($diff->isEmpty()); + self::assertSame([], $this->platform->getAlterSchemaSQL($diff)); + } + public function tearDown(): void { if (! isset($this->backedUpType)) {