Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove property-based schema comparison APIs #5649

Merged
merged 1 commit into from
Sep 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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