Skip to content

Commit

Permalink
Remove ColumnDiff::$changedProperties
Browse files Browse the repository at this point in the history
  • Loading branch information
@morozov
morozov committed Sep 3, 2022
1 parent 35192e8 commit 343526d
Show file tree
Hide file tree
Showing 13 changed files with 101 additions and 411 deletions.
2 changes: 1 addition & 1 deletion phpcs.xml.dist
View file
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
63 changes: 39 additions & 24 deletions src/Schema/ColumnDiff.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,80 +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,
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) !== ($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();
});
}

/**
* @internal
*/
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);
}
}
89 changes: 0 additions & 89 deletions src/Schema/Comparator.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,8 @@

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

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 @@ -220,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 @@ -418,90 +413,6 @@ public 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.
*
* @return array<int, string>
*/
public function diffColumn(Column $column1, Column $column2): array
{
$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
View file
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
View file
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
View file
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
51 changes: 0 additions & 51 deletions tests/Platforms/AbstractPlatformTestCase.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -229,53 +229,6 @@ public function getGenerateConstraintForeignKeySql(ForeignKeyConstraint $fk): st
);
}

/** @return string[] */
abstract public function getGenerateAlterTableSql(): array;

public function testGeneratesTableAlterationSql(): void
{
$expectedSql = $this->getGenerateAlterTableSql();

$table = new Table('mytable');
$table->addColumn('id', 'integer', ['autoincrement' => true]);
$table->addColumn('foo', 'integer');
$table->addColumn('bar', 'string', ['length' => 32]);
$table->addColumn('bloo', 'boolean');
$table->setPrimaryKey(['id']);

$tableDiff = new TableDiff('mytable');
$tableDiff->fromTable = $table;
$tableDiff->newName = 'userlist';
$tableDiff->addedColumns['quota'] = new Column('quota', Type::getType('integer'), ['notnull' => false]);
$tableDiff->removedColumns['foo'] = new Column('foo', Type::getType('integer'));
$tableDiff->changedColumns['bar'] = new ColumnDiff(
new Column(
'baz',
Type::getType('string'),
[
'length' => 255,
'default' => 'def',
],
),
['type', 'notnull', 'default'],
$table->getColumn('bar'),
);

$tableDiff->changedColumns['bloo'] = new ColumnDiff(
new Column(
'bloo',
Type::getType('boolean'),
['default' => false],
),
['type', 'notnull', 'default'],
$table->getColumn('bloo'),
);

$sql = $this->platform->getAlterTableSQL($tableDiff);

self::assertEquals($expectedSql, $sql);
}

public function testGetCustomColumnDeclarationSql(): void
{
self::assertEquals(
Expand Down Expand Up @@ -366,7 +319,6 @@ public function testGetAlterTableSqlDispatchEvent(): void
$tableDiff->removedColumns['removed'] = new Column('removed', Type::getType('integer'), []);
$tableDiff->changedColumns['changed'] = new ColumnDiff(
new Column('changed2', Type::getType('string'), ['length' => 255]),
[],
$table->getColumn('changed'),
);
$tableDiff->renamedColumns['renamed'] = new Column('renamed2', Type::getType('integer'));
Expand Down Expand Up @@ -617,7 +569,6 @@ public function testAlterTableChangeQuotedColumn(): void
Type::getType('string'),
['length' => 255],
),
['type'],
$table->getColumn('select'),
);

Expand Down Expand Up @@ -1131,7 +1082,6 @@ public function testQuotesTableIdentifiersInAlterTableSQL(): void
$tableDiff->addedColumns['bloo'] = new Column('bloo', Type::getType('integer'));
$tableDiff->changedColumns['bar'] = new ColumnDiff(
new Column('bar', Type::getType('integer'), ['notnull' => false]),
['notnull'],
$table->getColumn('bar'),
);
$tableDiff->renamedColumns['id'] = new Column('war', Type::getType('integer'));
Expand Down Expand Up @@ -1163,7 +1113,6 @@ public function testAlterStringToFixedString(): void
Type::getType('string'),
['fixed' => true, 'length' => 2],
),
['fixed'],
$table->getColumn('name'),
);

Expand Down
Loading

0 comments on commit 343526d

Please sign in to comment.