Skip to content

Commit

Permalink
Remove property-based schema comparison APIs
Browse files Browse the repository at this point in the history
  • Loading branch information
morozov committed Sep 5, 2022
1 parent 075848c commit e7b326c
Show file tree
Hide file tree
Showing 15 changed files with 107 additions and 443 deletions.
8 changes: 6 additions & 2 deletions UPGRADE.md
Original file line number Diff line number Diff line change
Expand Up @@ -537,9 +537,13 @@ Instead of returning an empty value, `Connection::lastInsertId()` throws an exce

The method `Comparator::compareSchemas()` cannot be called statically anymore.

## Removed `Comparator::compare()`
## Removed `Comparator` methods

The method `Comparator::compare()` has been removed, use `Comparator::compareSchemas()` instead.
The `Comparator::compare()` and `::diffColumn()` methods have been removed.

## Removed `ColumnDiff` methods

The `ColumnDiff::hasChanged()` method has been removed.

## Removed `TableGenerator` component

Expand Down
2 changes: 1 addition & 1 deletion phpcs.xml.dist
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@

<!-- see https://github.com/doctrine/dbal/issues/3377 -->
<rule ref="SlevomatCodingStandard.Operators.DisallowEqualOperators.DisallowedNotEqualOperator">
<exclude-pattern>src/Schema/Comparator.php</exclude-pattern>
<exclude-pattern>src/Schema/ColumnDiff.php</exclude-pattern>
</rule>

<!-- The SQLSRV_* functions are defined in the upper case by the sqlsrv extension and violate the standard
Expand Down
13 changes: 0 additions & 13 deletions psalm.xml.dist
Original file line number Diff line number Diff line change
Expand Up @@ -39,21 +39,8 @@
See https://github.com/doctrine/dbal/pull/4317
-->
<file name="tests/Functional/LegacyAPITest.php"/>
<!--
TODO: remove in 4.0.0
-->
<referencedMethod name="Doctrine\DBAL\Schema\ColumnDiff::hasChanged"/>
<referencedMethod name="Doctrine\DBAL\Schema\Comparator::diffColumn"/>
</errorLevel>
</DeprecatedMethod>
<DeprecatedProperty>
<errorLevel type="suppress">
<!--
TODO: remove in 4.0.0
-->
<referencedProperty name="Doctrine\DBAL\Schema\ColumnDiff::$changedProperties"/>
</errorLevel>
</DeprecatedProperty>
<DocblockTypeContradiction>
<errorLevel type="suppress">
<!--
Expand Down
70 changes: 39 additions & 31 deletions src/Schema/ColumnDiff.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,87 +4,95 @@

namespace Doctrine\DBAL\Schema;

use function in_array;

/**
* Represents the change of a column.
*/
class ColumnDiff
{
/**
* @internal The diff can be only instantiated by a {@see Comparator}.
*
* @param array<string> $changedProperties
*/
public function __construct(
public Column $column,
/**
* @deprecated Use {@see hasTypeChanged()}, {@see hasLengthChanged()}, {@see hasPrecisionChanged()},
* {@see hasScaleChanged()}, {@see hasUnsignedChanged()}, {@see hasFixedChanged()}, {@see hasNotNullChanged()},
* {@see hasDefaultChanged()}, {@see hasAutoIncrementChanged()} or {@see hasCommentChanged()} instead.
*/
public array $changedProperties,
public Column $fromColumn,
) {
public function __construct(public Column $column, public Column $fromColumn)
{
}

public function hasTypeChanged(): bool
{
return $this->hasChanged('type');
return $this->column->getType()::class !== $this->fromColumn->getType()::class;
}

public function hasLengthChanged(): bool
{
return $this->hasChanged('length');
return $this->hasPropertyChanged(static function (Column $column): ?int {
return $column->getLength();
});
}

public function hasPrecisionChanged(): bool
{
return $this->hasChanged('precision');
return $this->hasPropertyChanged(static function (Column $column): ?int {
return $column->getPrecision();
});
}

public function hasScaleChanged(): bool
{
return $this->hasChanged('scale');
return $this->hasPropertyChanged(static function (Column $column): int {
return $column->getScale();
});
}

public function hasUnsignedChanged(): bool
{
return $this->hasChanged('unsigned');
return $this->hasPropertyChanged(static function (Column $column): bool {
return $column->getUnsigned();
});
}

public function hasFixedChanged(): bool
{
return $this->hasChanged('fixed');
return $this->hasPropertyChanged(static function (Column $column): bool {
return $column->getFixed();
});
}

public function hasNotNullChanged(): bool
{
return $this->hasChanged('notnull');
return $this->hasPropertyChanged(static function (Column $column): bool {
return $column->getNotnull();
});
}

public function hasDefaultChanged(): bool
{
return $this->hasChanged('default');
$oldDefault = $this->fromColumn->getDefault();
$newDefault = $this->column->getDefault();

// Null values need to be checked additionally as they tell whether to create or drop a default value.
// null != 0, null != false, null != '' etc. This affects platform's table alteration SQL generation.
if (($newDefault === null) xor ($oldDefault === null)) {
return true;
}

return $newDefault != $oldDefault;
}

public function hasAutoIncrementChanged(): bool
{
return $this->hasChanged('autoincrement');
return $this->hasPropertyChanged(static function (Column $column): bool {
return $column->getAutoincrement();
});
}

public function hasCommentChanged(): bool
{
return $this->hasChanged('comment');
return $this->hasPropertyChanged(static function (Column $column): string {
return $column->getComment();
});
}

/**
* @deprecated Use {@see hasTypeChanged()}, {@see hasLengthChanged()}, {@see hasPrecisionChanged()},
* {@see hasScaleChanged()}, {@see hasUnsignedChanged()}, {@see hasFixedChanged()}, {@see hasNotNullChanged()},
* {@see hasDefaultChanged()}, {@see hasAutoIncrementChanged()} or {@see hasCommentChanged()} instead.
*/
public function hasChanged(string $propertyName): bool
private function hasPropertyChanged(callable $property): bool
{
return in_array($propertyName, $this->changedProperties, true);
return $property($this->column) !== $property($this->fromColumn);
}
}
99 changes: 0 additions & 99 deletions src/Schema/Comparator.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,8 @@

use Doctrine\DBAL\Exception;
use Doctrine\DBAL\Platforms\AbstractPlatform;
use Doctrine\DBAL\Types;
use Doctrine\Deprecations\Deprecation;

use function array_intersect_key;
use function array_keys;
use function array_map;
use function array_unique;
use function assert;
use function count;
use function strtolower;
Expand Down Expand Up @@ -221,7 +216,6 @@ public function diffTable(Table $fromTable, Table $toTable): ?TableDiff

$tableDifferences->changedColumns[$column->getName()] = new ColumnDiff(
$toColumn,
$this->diffColumn($column, $toColumn),
$column,
);

Expand Down Expand Up @@ -419,99 +413,6 @@ protected function columnsEqual(Column $column1, Column $column2): bool
return $this->platform->columnsEqual($column1, $column2);
}

/**
* Returns the difference between the columns
*
* 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.
*
* @return array<int, string>
*/
public function diffColumn(Column $column1, Column $column2): array
{
Deprecation::triggerIfCalledFromOutside(
'doctrine/dbal',
'https://github.com/doctrine/dbal/pull/5650',
'%s is deprecated. Use diffTable() instead.',
__METHOD__,
);

$properties1 = $column1->toArray();
$properties2 = $column2->toArray();

$changedProperties = [];

if ($properties1['type']::class !== $properties2['type']::class) {
$changedProperties[] = 'type';
}

foreach (['notnull', 'unsigned', 'autoincrement'] as $property) {
if ($properties1[$property] === $properties2[$property]) {
continue;
}

$changedProperties[] = $property;
}

// Null values need to be checked additionally as they tell whether to create or drop a default value.
// null != 0, null != false, null != '' etc. This affects platform's table alteration SQL generation.
if (
($properties1['default'] === null) !== ($properties2['default'] === null)
|| $properties1['default'] != $properties2['default']
) {
$changedProperties[] = 'default';
}

if (
($properties1['type'] instanceof Types\StringType && ! $properties1['type'] instanceof Types\GuidType) ||
$properties1['type'] instanceof Types\BinaryType
) {
if (
(isset($properties1['length']) !== isset($properties2['length']))
|| (isset($properties1['length']) && isset($properties2['length'])
&& $properties1['length'] !== $properties2['length'])
) {
$changedProperties[] = 'length';
}

if ($properties1['fixed'] !== $properties2['fixed']) {
$changedProperties[] = 'fixed';
}
} elseif ($properties1['type'] instanceof Types\DecimalType) {
if ($properties1['precision'] !== $properties2['precision']) {
$changedProperties[] = 'precision';
}

if ($properties1['scale'] !== $properties2['scale']) {
$changedProperties[] = 'scale';
}
}

// A null value and an empty string are actually equal for a comment so they should not trigger a change.
if (
$properties1['comment'] !== $properties2['comment'] &&
! ($properties1['comment'] === null && $properties2['comment'] === '') &&
! ($properties2['comment'] === null && $properties1['comment'] === '')
) {
$changedProperties[] = 'comment';
}

$platformOptions1 = $column1->getPlatformOptions();
$platformOptions2 = $column2->getPlatformOptions();

foreach (array_keys(array_intersect_key($platformOptions1, $platformOptions2)) as $key) {
if ($properties1[$key] === $properties2[$key]) {
continue;
}

$changedProperties[] = $key;
}

return array_unique($changedProperties);
}

/**
* Finds the difference between the indexes $index1 and $index2.
*
Expand Down
5 changes: 0 additions & 5 deletions tests/Functional/Schema/SQLServerSchemaManagerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -100,12 +100,10 @@ public function testDefaultConstraints(): void
[
'df_integer' => new ColumnDiff(
new Column('df_integer', Type::getType('integer'), ['default' => 0]),
['default'],
new Column('df_integer', Type::getType('integer'), ['default' => 666]),
),
'df_string_2' => new ColumnDiff(
new Column('df_string_2', Type::getType('string'), ['length' => 32]),
['default'],
new Column('df_string_2', Type::getType('string'), [
'length' => 32,
'default' => 'Doctrine rocks!!!',
Expand All @@ -116,15 +114,13 @@ public function testDefaultConstraints(): void
'length' => 50,
'default' => 'another default value',
]),
['length'],
new Column('df_string_3', Type::getType('string'), [
'length' => 50,
'default' => 'another default value',
]),
),
'df_boolean' => new ColumnDiff(
new Column('df_boolean', Type::getType('boolean'), ['default' => false]),
['default'],
new Column('df_boolean', Type::getType('boolean'), ['default' => true]),
),
],
Expand Down Expand Up @@ -162,7 +158,6 @@ public function testDefaultConstraints(): void
[
'df_integer' => new ColumnDiff(
new Column('df_integer', Type::getType('integer'), ['default' => 666]),
['default'],
new Column('df_integer', Type::getType('integer'), ['default' => 0]),
),
],
Expand Down
2 changes: 0 additions & 2 deletions tests/Functional/Schema/SchemaManagerFunctionalTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -844,7 +844,6 @@ public function testChangeColumnsTypeWithDefaultValue(): void
$tableDiff->fromTable = $table;
$tableDiff->changedColumns['col_int'] = new ColumnDiff(
new Column('col_int', Type::getType('integer'), ['default' => 666]),
['type'],
new Column('col_int', Type::getType('smallint'), ['default' => 666]),
);

Expand All @@ -854,7 +853,6 @@ public function testChangeColumnsTypeWithDefaultValue(): void
'fixed' => true,
'default' => 'foo',
]),
['fixed'],
new Column('col_string', Type::getType('string'), [
'length' => 3,
'default' => 'foo',
Expand Down
12 changes: 0 additions & 12 deletions tests/Platforms/AbstractMySQLPlatformTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -54,18 +54,6 @@ public function getGenerateTableWithMultiColumnUniqueIndexSql(): array
];
}

/**
* {@inheritDoc}
*/
public function getGenerateAlterTableSql(): array
{
return [
'ALTER TABLE mytable RENAME TO userlist, ADD quota INT DEFAULT NULL, DROP foo, '
. "CHANGE bar baz VARCHAR(255) DEFAULT 'def' NOT NULL, "
. 'CHANGE bloo bloo TINYINT(1) DEFAULT 0 NOT NULL',
];
}

public function testGeneratesSqlSnippets(): void
{
self::assertEquals('RLIKE', $this->platform->getRegexpExpression());
Expand Down
Loading

0 comments on commit e7b326c

Please sign in to comment.