Skip to content

Commit

Permalink
Merge pull request #5684 from morozov/column-diff-private-properties
Browse files Browse the repository at this point in the history 
Mark ColumnDiff properties as private and read-only
  • Loading branch information
@morozov
morozov authored Sep 21, 2022
2 parents 76635d3 + 2373c42 commit 06243b7
Show file tree
Hide file tree
Showing 6 changed files with 43 additions and 43 deletions.
14 changes: 7 additions & 7 deletions src/Schema/ColumnDiff.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,23 +12,23 @@ class ColumnDiff
/**
* @internal The diff can be only instantiated by a {@see Comparator}.
*/
public function __construct(public Column $column, public Column $fromColumn)
public function __construct(private readonly Column $oldColumn, private readonly Column $newColumn)
{
}

public function getOldColumn(): Column
{
return $this->fromColumn;
return $this->oldColumn;
}

public function getNewColumn(): Column
{
return $this->column;
return $this->newColumn;
}

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

public function hasLengthChanged(): bool
Expand Down Expand Up @@ -75,8 +75,8 @@ public function hasNotNullChanged(): bool

public function hasDefaultChanged(): bool
{
$oldDefault = $this->fromColumn->getDefault();
$newDefault = $this->column->getDefault();
$oldDefault = $this->oldColumn->getDefault();
$newDefault = $this->newColumn->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.
Expand All @@ -103,6 +103,6 @@ public function hasCommentChanged(): bool

private function hasPropertyChanged(callable $property): bool
{
return $property($this->column) !== $property($this->fromColumn);
return $property($this->newColumn) !== $property($this->oldColumn);
}
}
2 changes: 1 addition & 1 deletion src/Schema/Comparator.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -213,8 +213,8 @@ public function diffTable(Table $fromTable, Table $toTable): ?TableDiff
}

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

$changes++;
Expand Down
8 changes: 4 additions & 4 deletions tests/Platforms/AbstractPlatformTestCase.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -317,8 +317,8 @@ public function testGetAlterTableSqlDispatchEvent(): void
$tableDiff->addedColumns['added'] = new Column('added', Type::getType('integer'), []);
$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'),
new Column('changed2', Type::getType('string'), ['length' => 255]),
);
$tableDiff->renamedColumns['renamed'] = new Column('renamed2', Type::getType('integer'));

Expand Down Expand Up @@ -562,12 +562,12 @@ public function testAlterTableChangeQuotedColumn(): void

$tableDiff = new TableDiff($table);
$tableDiff->changedColumns['select'] = new ColumnDiff(
$table->getColumn('select'),
new Column(
'select',
Type::getType('string'),
['length' => 255],
),
$table->getColumn('select'),
);

self::assertStringContainsString(
Expand Down Expand Up @@ -1080,8 +1080,8 @@ public function testQuotesTableIdentifiersInAlterTableSQL(): void
$tableDiff = new TableDiff($table);
$tableDiff->addedColumns['bloo'] = new Column('bloo', Type::getType('integer'));
$tableDiff->changedColumns['bar'] = new ColumnDiff(
new Column('bar', Type::getType('integer'), ['notnull' => false]),
$table->getColumn('bar'),
new Column('bar', Type::getType('integer'), ['notnull' => false]),
);
$tableDiff->renamedColumns['id'] = new Column('war', Type::getType('integer'));
$tableDiff->removedColumns['baz'] = new Column('baz', Type::getType('integer'));
Expand All @@ -1106,12 +1106,12 @@ public function testAlterStringToFixedString(): void
$tableDiff = new TableDiff($table);

$tableDiff->changedColumns['name'] = new ColumnDiff(
$table->getColumn('name'),
new Column(
'name',
Type::getType('string'),
['fixed' => true, 'length' => 2],
),
$table->getColumn('name'),
);

$sql = $this->platform->getAlterTableSQL($tableDiff);
Expand Down
2 changes: 1 addition & 1 deletion tests/Platforms/DB2PlatformTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -501,7 +501,7 @@ public function testGeneratesAlterColumnSQL(
?string $expectedSQLClause,
): void {
$tableDiff = new TableDiff(new Table('foo'));
$tableDiff->changedColumns['bar'] = new ColumnDiff($newColumn, $oldColumn);
$tableDiff->changedColumns['bar'] = new ColumnDiff($oldColumn, $newColumn);

$expectedSQL = [];

Expand Down
44 changes: 22 additions & 22 deletions tests/Platforms/SQLServerPlatformTestCase.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -662,8 +662,8 @@ public function testAlterTableWithSchemaDropColumnComments(): void

$tableDiff = new TableDiff($table);
$tableDiff->changedColumns['quota'] = new ColumnDiff(
new Column('quota', Type::getType('integer'), []),
new Column('quota', Type::getType('integer'), ['comment' => 'A comment']),
new Column('quota', Type::getType('integer'), []),
);

$expectedSql = [
Expand All @@ -680,8 +680,8 @@ public function testAlterTableWithSchemaUpdateColumnComments(): void

$tableDiff = new TableDiff($table);
$tableDiff->changedColumns['quota'] = new ColumnDiff(
new Column('quota', Type::getType('integer'), ['comment' => 'B comment']),
new Column('quota', Type::getType('integer'), ['comment' => 'A comment']),
new Column('quota', Type::getType('integer'), ['comment' => 'B comment']),
);

$expectedSql = ["EXEC sp_updateextendedproperty N'MS_Description', N'B comment', "
Expand Down Expand Up @@ -796,60 +796,60 @@ public function testGeneratesAlterTableSQLWithColumnComments(): void

// Add comment to non-commented column.
$tableDiff->changedColumns['id'] = new ColumnDiff(
new Column('id', Type::getType('integer'), ['autoincrement' => true, 'comment' => 'primary']),
new Column('id', Type::getType('integer'), ['autoincrement' => true]),
new Column('id', Type::getType('integer'), ['autoincrement' => true, 'comment' => 'primary']),
);

// Change type to custom type from empty string commented column.
$tableDiff->changedColumns['comment_empty_string'] = new ColumnDiff(
new Column('comment_empty_string', Type::getType('json')),
new Column('comment_empty_string', Type::getType('integer'), ['comment' => '']),
new Column('comment_empty_string', Type::getType('json')),
);

// Change comment to empty comment from zero-string commented column.
$tableDiff->changedColumns['comment_string_0'] = new ColumnDiff(
new Column('comment_string_0', Type::getType('integer'), ['comment' => '']),
new Column('comment_string_0', Type::getType('integer'), ['comment' => '0']),
new Column('comment_string_0', Type::getType('integer'), ['comment' => '']),
);

// Remove comment from regular commented column.
$tableDiff->changedColumns['comment'] = new ColumnDiff(
new Column('comment', Type::getType('integer')),
new Column('comment', Type::getType('integer'), ['comment' => 'Doctrine 0wnz you!']),
new Column('comment', Type::getType('integer')),
);

// Change comment and change type to custom type from regular commented column.
$tableDiff->changedColumns['`comment_quoted`'] = new ColumnDiff(
new Column('`comment_quoted`', Type::getType('json'), ['comment' => 'Doctrine JSON.']),
new Column('`comment_quoted`', Type::getType('integer'), ['comment' => 'Doctrine 0wnz you!']),
new Column('`comment_quoted`', Type::getType('json'), ['comment' => 'Doctrine JSON.']),
);

// Remove comment and change type to custom type from regular commented column.
$tableDiff->changedColumns['create'] = new ColumnDiff(
new Column('create', Type::getType('json')),
new Column(
'create',
Type::getType('integer'),
['comment' => 'Doctrine 0wnz comments for reserved keyword columns!'],
),
new Column('create', Type::getType('json')),
);

// Add comment and change custom type to regular type from non-commented column.
$tableDiff->changedColumns['commented_type'] = new ColumnDiff(
new Column('commented_type', Type::getType('integer'), ['comment' => 'foo']),
new Column('commented_type', Type::getType('json')),
new Column('commented_type', Type::getType('integer'), ['comment' => 'foo']),
);

// Remove comment from commented custom type column.
$tableDiff->changedColumns['commented_type_with_comment'] = new ColumnDiff(
new Column('commented_type_with_comment', Type::getType('json')),
new Column('commented_type_with_comment', Type::getType('json'), ['comment' => 'Doctrine JSON type.']),
new Column('commented_type_with_comment', Type::getType('json')),
);

// Change comment from comment with string literal char column.
$tableDiff->changedColumns['comment_with_string_literal_char'] = new ColumnDiff(
new Column('comment_with_string_literal_char', Type::getType('string'), ['comment' => "'"]),
new Column('comment_with_string_literal_char', Type::getType('json'), ['comment' => "O'Reilly"]),
new Column('comment_with_string_literal_char', Type::getType('string'), ['comment' => "'"]),
);

self::assertEquals(
Expand Down Expand Up @@ -1033,17 +1033,17 @@ public function testChangeColumnsTypeWithDefaultValue(): void

$tableDiff = new TableDiff($table);
$tableDiff->changedColumns['col_int'] = new ColumnDiff(
new Column('col_int', Type::getType('integer'), ['default' => 666]),
new Column('col_int', Type::getType('smallint'), ['default' => 666]),
new Column('col_int', Type::getType('integer'), ['default' => 666]),
);

$tableDiff->changedColumns['col_string'] = new ColumnDiff(
new Column('col_string', Type::getType('string'), ['default' => 'foo']),
new Column('col_string', Type::getType('string'), [
'length' => 255,
'fixed' => true,
'default' => 'foo',
]),
new Column('col_string', Type::getType('string'), ['default' => 'foo']),
);

self::assertSame(
Expand Down Expand Up @@ -1201,11 +1201,11 @@ public static function getGeneratesIdentifierNamesInAlterTableSQL(): iterable
'mycolumn' => new ColumnDiff(
new Column('mycolumn', Type::getType('string'), [
'length' => 255,
'default' => 'bar',
'default' => 'foo',
]),
new Column('mycolumn', Type::getType('string'), [
'length' => 255,
'default' => 'foo',
'default' => 'bar',
]),
),
],
Expand Down Expand Up @@ -1239,11 +1239,11 @@ public static function getGeneratesIdentifierNamesInAlterTableSQL(): iterable
'mycolumn' => new ColumnDiff(
new Column('`mycolumn`', Type::getType('string'), [
'length' => 255,
'default' => 'bar',
'default' => 'foo',
]),
new Column('`mycolumn`', Type::getType('string'), [
'length' => 255,
'default' => 'foo',
'default' => 'bar',
]),
),
],
Expand Down Expand Up @@ -1277,11 +1277,11 @@ public static function getGeneratesIdentifierNamesInAlterTableSQL(): iterable
'select' => new ColumnDiff(
new Column('select', Type::getType('string'), [
'length' => 255,
'default' => 'bar',
'default' => 'foo',
]),
new Column('select', Type::getType('string'), [
'length' => 255,
'default' => 'foo',
'default' => 'bar',
]),
),
],
Expand Down Expand Up @@ -1315,11 +1315,11 @@ public static function getGeneratesIdentifierNamesInAlterTableSQL(): iterable
'select' => new ColumnDiff(
new Column('`select`', Type::getType('string'), [
'length' => 255,
'default' => 'bar',
'default' => 'foo',
]),
new Column('`select`', Type::getType('string'), [
'length' => 255,
'default' => 'foo',
'default' => 'bar',
]),
),
],
Expand Down Expand Up @@ -1584,8 +1584,8 @@ public function testAlterTableWithSchemaSameColumnComments(): void

$tableDiff = new TableDiff($table);
$tableDiff->changedColumns['quota'] = new ColumnDiff(
new Column('quota', Type::getType('integer'), ['comment' => 'A comment', 'notnull' => true]),
new Column('quota', Type::getType('integer'), ['comment' => 'A comment', 'notnull' => false]),
new Column('quota', Type::getType('integer'), ['comment' => 'A comment', 'notnull' => true]),
);

$expectedSql = ['ALTER TABLE testschema.mytable ALTER COLUMN quota INT NOT NULL'];
Expand Down
16 changes: 8 additions & 8 deletions tests/Schema/ComparatorTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ public function testCompareAutoIncrementChanged(): void
$column1 = new Column('foo', Type::getType('integer'), ['autoincrement' => true]);
$column2 = new Column('foo', Type::getType('integer'), ['autoincrement' => false]);

$diff = new ColumnDiff($column1, $column2);
$diff = new ColumnDiff($column2, $column1);

self::assertTrue($diff->hasAutoIncrementChanged());
}
Expand Down Expand Up @@ -187,7 +187,7 @@ public function testCompareChangedColumnsChangeType(): void
$column1 = new Column('id', Type::getType(Types::STRING));
$column2 = new Column('id', Type::getType(Types::INTEGER));

$diff12 = new ColumnDiff($column1, $column2);
$diff12 = new ColumnDiff($column2, $column1);
self::assertTrue($diff12->hasTypeChanged());

$diff11 = new ColumnDiff($column1, $column1);
Expand All @@ -204,7 +204,7 @@ public function testDifferentTypeInstancesOfTheSameType(): void
$column1 = new Column('id', $type1);
$column2 = new Column('id', $type2);

$diff = new ColumnDiff($column1, $column2);
$diff = new ColumnDiff($column2, $column1);
self::assertFalse($diff->hasTypeChanged());
}

Expand All @@ -221,7 +221,7 @@ public function testOverriddenType(): void
$column1 = new Column('id', $integerType);
$column2 = new Column('id', $overriddenStringType);

$diff = new ColumnDiff($column1, $column2);
$diff = new ColumnDiff($column2, $column1);
self::assertFalse($diff->hasTypeChanged());
}

Expand Down Expand Up @@ -951,8 +951,8 @@ public function testCompareChangedColumn(): void
$tableDiff = $expected->changedTables['foo'] = new TableDiff($tableFoo);

$tableDiff->changedColumns['id'] = new ColumnDiff(
$table->getColumn('id'),
$tableFoo->getColumn('id'),
$table->getColumn('id'),
);

self::assertEquals($expected, $this->comparator->compareSchemas($oldSchema, $newSchema));
Expand All @@ -974,8 +974,8 @@ public function testCompareChangedBinaryColumn(): void
$tableDiff = $expected->changedTables['foo'] = new TableDiff($tableFoo);

$tableDiff->changedColumns['id'] = new ColumnDiff(
$table->getColumn('id'),
$tableFoo->getColumn('id'),
$table->getColumn('id'),
);

self::assertEquals($expected, $this->comparator->compareSchemas($oldSchema, $newSchema));
Expand Down Expand Up @@ -1041,8 +1041,8 @@ public function testCompareColumnComments(string $comment1, string $comment2, bo
$column1 = new Column('foo', Type::getType('integer'), ['comment' => $comment1]);
$column2 = new Column('foo', Type::getType('integer'), ['comment' => $comment2]);

$diff1 = new ColumnDiff($column1, $column2);
$diff2 = new ColumnDiff($column2, $column1);
$diff1 = new ColumnDiff($column2, $column1);
$diff2 = new ColumnDiff($column1, $column2);

self::assertSame(! $equals, $diff1->hasCommentChanged());
self::assertSame(! $equals, $diff2->hasCommentChanged());
Expand Down

0 comments on commit 06243b7

Please sign in to comment.