From 1b6b57d29a323e4e7e4bc8c97e484125f40dde6f Mon Sep 17 00:00:00 2001 From: Aziz Date: Mon, 9 Aug 2021 21:30:40 +0300 Subject: [PATCH] #97 - Renaming column does not drops column on morphTable --- src/Db/FieldDefinition.php | 135 ++++++++++++++++++++++++++ src/Mvc/Model/Migration.php | 88 ++++++++--------- tests/unit/Db/FieldDefinitionTest.php | 119 +++++++++++++++++++++++ 3 files changed, 298 insertions(+), 44 deletions(-) create mode 100644 src/Db/FieldDefinition.php create mode 100644 tests/unit/Db/FieldDefinitionTest.php diff --git a/src/Db/FieldDefinition.php b/src/Db/FieldDefinition.php new file mode 100644 index 0000000..9f568b3 --- /dev/null +++ b/src/Db/FieldDefinition.php @@ -0,0 +1,135 @@ + + * + * For the full copyright and license information, please view + * the LICENSE file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace Phalcon\Migrations\Db; + +use Phalcon\Db\ColumnInterface; + +class FieldDefinition +{ + /** + * @var string + */ + private $name; + + /** + * @var ColumnInterface + */ + private $currentColumn; + + /** + * @var FieldDefinition|null + */ + private $previousField; + + /** + * @var FieldDefinition|null + */ + private $nextField; + + public function __construct( + ColumnInterface $column, + ?FieldDefinition $previousField = null, + ?FieldDefinition $nextField = null + ) { + $this->name = $column->getName(); + $this->currentColumn = $column; + $this->previousField = $previousField; + $this->nextField = $nextField; + } + + public function setPrevious(?FieldDefinition $field = null): void + { + $this->previousField = $field; + } + + public function setNext(?FieldDefinition $field = null): void + { + $this->nextField = $field; + } + + public function getName(): string + { + return $this->name; + } + + public function getColumn(): ColumnInterface + { + return $this->currentColumn; + } + + public function getPrevious(): ?FieldDefinition + { + return $this->previousField ?? null; + } + + public function getNext(): ?FieldDefinition + { + return $this->nextField ?? null; + } + + /** + * @param FieldDefinition[] $externalFieldset Set of another field definitions to compare field between + * + * @return self|null + */ + public function getPairedDefinition(array $externalFieldset): ?FieldDefinition + { + if (isset($externalFieldset[$this->getName()])) { + return $externalFieldset[$this->getName()]; + } + + $possiblePairedField = null; + if (null !== $this->previousField) { + $prevField = $externalFieldset[$this->previousField->getName()] ?? null; + if (null !== $prevField) { + $possiblePairedField = $prevField->getNext(); + } + } + if (null === $possiblePairedField && null !== $this->nextField) { + $nextField = $externalFieldset[$this->nextField->getName()] ?? null; + if (null !== $nextField) { + $possiblePairedField = $nextField->getPrevious(); + } + } + + if (null === $possiblePairedField) { + return null; + } + + if ($this->isChangedData($possiblePairedField)) { + return null; + } + + return $possiblePairedField; + } + + public function isChanged(FieldDefinition $comparingFieldDefinition): bool + { + return $this->isChangedName($comparingFieldDefinition) || $this->isChangedData($comparingFieldDefinition); + } + + public function isChangedName(FieldDefinition $comparingFieldDefinition): bool + { + return $this->currentColumn->getName() !== $comparingFieldDefinition->getColumn()->getName(); + } + + public function isChangedData(FieldDefinition $comparingFieldDefinition): bool + { + return $this->currentColumn->getType() !== $comparingFieldDefinition->getColumn()->getType() || + $this->currentColumn->getSize() !== $comparingFieldDefinition->getColumn()->getSize() || + $this->currentColumn->isNotNull() !== $comparingFieldDefinition->getColumn()->isNotNull() || + $this->currentColumn->getDefault() !== $comparingFieldDefinition->getColumn()->getDefault() || + $this->currentColumn->isUnsigned() !== $comparingFieldDefinition->getColumn()->isUnsigned(); + } +} diff --git a/src/Mvc/Model/Migration.php b/src/Mvc/Model/Migration.php index fe00143..669e095 100644 --- a/src/Mvc/Model/Migration.php +++ b/src/Mvc/Model/Migration.php @@ -20,7 +20,6 @@ use Phalcon\Db\Adapter\AbstractAdapter; use Phalcon\Db\Adapter\Pdo\Mysql as PdoMysql; use Phalcon\Db\ColumnInterface; -use Phalcon\Db\Enum; use Phalcon\Db\Exception as DbException; use Phalcon\Db\IndexInterface; use Phalcon\Db\ReferenceInterface; @@ -28,6 +27,7 @@ use Phalcon\Migrations\Db\Adapter\Pdo\PdoPostgresql; use Phalcon\Migrations\Db\Dialect\DialectMysql; use Phalcon\Migrations\Db\Dialect\DialectPostgresql; +use Phalcon\Migrations\Db\FieldDefinition; use Phalcon\Migrations\Exception\Db\UnknownColumnTypeException; use Phalcon\Migrations\Exception\RuntimeException; use Phalcon\Migrations\Generator\Snippet; @@ -434,6 +434,7 @@ public function morphTable(string $tableName, array $definition): void } $fields = []; + $previousField = null; /** @var ColumnInterface $tableColumn */ foreach ($definition['columns'] as $tableColumn) { /** @@ -443,28 +444,42 @@ public function morphTable(string $tableName, array $definition): void throw new DbException('Table must have at least one column'); } - /** @var ColumnInterface[] $fields */ - $fields[$tableColumn->getName()] = $tableColumn; + $field = new FieldDefinition($tableColumn); + $field->setPrevious($previousField); + if (null !== $previousField) { + $previousField->setNext($field); + } + $previousField = $field; + + $fields[$field->getName()] = $field; } if ($tableExists) { $localFields = []; - /** @var ColumnInterface[] $description */ + $previousField = null; $description = self::$connection->describeColumns($tableName, $defaultSchema); - foreach ($description as $field) { - /** @var ColumnInterface[] $localFields */ + /** @var ColumnInterface $localColumn */ + foreach ($description as $localColumn) { + $field = new FieldDefinition($localColumn); + $field->setPrevious($previousField); + if (null !== $previousField) { + $previousField->setNext($field); + } + $previousField = $field; + $localFields[$field->getName()] = $field; } - foreach ($fields as $fieldName => $column) { - if (!isset($localFields[$fieldName])) { + foreach ($fields as $fieldDefinition) { + $localFieldDefinition = $fieldDefinition->getPairedDefinition($localFields); + if (null === $localFieldDefinition) { try { - self::$connection->addColumn($tableName, $tableSchema, $column); + self::$connection->addColumn($tableName, $tableSchema, $fieldDefinition->getColumn()); } catch (Throwable $exception) { throw new RuntimeException( sprintf( "Failed to add column '%s' in table '%s'. In '%s' migration. DB error: %s", - $column->getName(), + $fieldDefinition->getName(), $tableName, get_called_class(), $exception->getMessage() @@ -475,35 +490,19 @@ public function morphTable(string $tableName, array $definition): void continue; } - $changed = false; - if ($localFields[$fieldName]->getType() !== $column->getType()) { - $changed = true; - } - - if ($localFields[$fieldName]->getSize() !== $column->getSize()) { - $changed = true; - } - - if ($localFields[$fieldName]->isNotNull() !== $column->isNotNull()) { - $changed = true; - } - - if ($localFields[$fieldName]->getDefault() !== $column->getDefault()) { - $changed = true; - } - - if ($localFields[$fieldName]->isUnsigned() !== $column->isUnsigned()) { - $changed = true; - } - - if ($changed === true) { + if ($fieldDefinition->isChanged($localFieldDefinition)) { try { - self::$connection->modifyColumn($tableName, $tableSchema, $column, $column); + self::$connection->modifyColumn( + $tableName, + $tableSchema, + $fieldDefinition->getColumn(), + $localFieldDefinition->getColumn() + ); } catch (Throwable $exception) { throw new RuntimeException( sprintf( "Failed to modify column '%s' in table '%s'. In '%s' migration. DB error: %s", - $column->getName(), + $fieldDefinition->getName(), $tableName, get_called_class(), $exception->getMessage() @@ -513,18 +512,19 @@ public function morphTable(string $tableName, array $definition): void } } - foreach ($localFields as $fieldName => $localField) { - if (!isset($fields[$fieldName])) { + foreach ($localFields as $fieldDefinition) { + $newFieldDefinition = $fieldDefinition->getPairedDefinition($fields); + if (null === $newFieldDefinition) { try { /** * TODO: Check, why schemaName is empty string. */ - self::$connection->dropColumn($tableName, '', $fieldName); + self::$connection->dropColumn($tableName, '', $fieldDefinition->getName()); } catch (Throwable $exception) { throw new RuntimeException( sprintf( "Failed to drop column '%s' in table '%s'. In '%s' migration. DB error: %s", - $fieldName, + $fieldDefinition->getName(), $tableName, get_called_class(), $exception->getMessage() @@ -782,7 +782,7 @@ public function batchInsert(string $tableName, $fields, int $size = 1024): void $batchHandler = fopen($migrationData, 'r'); while (($line = fgetcsv($batchHandler)) !== false) { $values = array_map( - function ($value) { + static function ($value) { if (null === $value || $value === 'NULL') { return 'NULL'; } @@ -820,7 +820,7 @@ function ($value) { * * @param string $tableName */ - public function batchDelete(string $tableName) + public function batchDelete(string $tableName): void { $migrationData = self::$migrationPath . $this->version . '/' . $tableName . '.dat'; if (!file_exists($migrationData)) { @@ -833,7 +833,7 @@ public function batchDelete(string $tableName) $batchHandler = fopen($migrationData, 'r'); while (($line = fgetcsv($batchHandler)) !== false) { $values = array_map( - function ($value) { + static function ($value) { return null === $value ? null : stripslashes($value); }, $line @@ -852,7 +852,7 @@ function ($value) { * * @return AbstractAdapter */ - public function getConnection() + public function getConnection(): AbstractAdapter { return self::$connection; } @@ -890,11 +890,11 @@ public static function resolveDbSchema(Config $config): ?string } $adapter = strtolower($config->get('adapter')); - if (self::DB_ADAPTER_POSTGRESQL == $adapter) { + if (self::DB_ADAPTER_POSTGRESQL === $adapter) { return 'public'; } - if (self::DB_ADAPTER_SQLITE == $adapter) { + if (self::DB_ADAPTER_SQLITE === $adapter) { // SQLite only supports the current database, unless one is // attached. This is not the case, so don't return a schema. return null; diff --git a/tests/unit/Db/FieldDefinitionTest.php b/tests/unit/Db/FieldDefinitionTest.php new file mode 100644 index 0000000..0ba1ea4 --- /dev/null +++ b/tests/unit/Db/FieldDefinitionTest.php @@ -0,0 +1,119 @@ + + * + * For the full copyright and license information, please view + * the LICENSE file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace Phalcon\Migrations\Tests\Unit\Db; + +use Codeception\Test\Unit; +use Phalcon\Db\Column; +use Phalcon\Migrations\Db\FieldDefinition; + +final class FieldDefinitionTest extends Unit +{ + public const OLD_COLUMN_NAME = 'login'; + public const NEW_COLUMN_NAME = 'username'; + public const COLUMN_DEF = [ + 'type' => Column::TYPE_VARCHAR, + 'notNull' => true, + 'size' => 2047, + 'after' => 'id', + ]; + public const PREV_COLUMN = 'id'; + public const ID_COLUMN_DEF = [ + 'type' => Column::TYPE_INTEGER, + 'notNull' => true, + 'autoIncrement' => true, + 'size' => 11, + 'first' => true, + ]; + public const NEXT_COLUMN = 'password'; + public const PASSWORD_COLUMN_DEF = [ + 'type' => Column::TYPE_VARCHAR, + 'notNull' => true, + 'size' => 2047, + ]; + + public function testCreate(): void + { + $column = new Column(self::OLD_COLUMN_NAME, self::COLUMN_DEF); + $fieldDefinition = new FieldDefinition($column); + + $this->assertSame($column->getName(), $fieldDefinition->getName()); + } + + public function testSetPrevAndNext(): void + { + $column = new Column(self::OLD_COLUMN_NAME, self::COLUMN_DEF); + $fieldDefinition = new FieldDefinition($column); + + $prevColumn = new Column(self::PREV_COLUMN, self::ID_COLUMN_DEF); + $prevFieldDefinition = new FieldDefinition($prevColumn); + $fieldDefinition->setPrevious($prevFieldDefinition); + + $nextColumn = new Column(self::NEXT_COLUMN, self::PASSWORD_COLUMN_DEF); + $nextFieldDefinition = new FieldDefinition($nextColumn); + $fieldDefinition->setNext($nextFieldDefinition); + + $this->assertSame($prevColumn->getName(), $fieldDefinition->getPrevious()->getColumn()->getName()); + $this->assertSame($nextColumn->getName(), $fieldDefinition->getNext()->getColumn()->getName()); + } + + public function testNameChanged(): void + { + $localFields = []; + $column = new Column(self::OLD_COLUMN_NAME, self::COLUMN_DEF); + $fieldDefinition = new FieldDefinition($column); + $prevFieldDefinition = $this->createPrev($fieldDefinition); + $nextFieldDefinition = $this->createNext($fieldDefinition); + + $localFields[$fieldDefinition->getName()] = $fieldDefinition; + $localFields[$prevFieldDefinition->getName()] = $prevFieldDefinition; + $localFields[$nextFieldDefinition->getName()] = $nextFieldDefinition; + + $fields = []; + $columnChanged = new Column(self::NEW_COLUMN_NAME, self::COLUMN_DEF); + $fieldDefinitionChanged = new FieldDefinition($columnChanged); + $prevFieldDefinitionChanged = $this->createPrev($fieldDefinitionChanged); + $nextFieldDefinitionChanged = $this->createNext($fieldDefinitionChanged); + + $fields[$fieldDefinitionChanged->getName()] = $fieldDefinitionChanged; + $fields[$prevFieldDefinitionChanged->getName()] = $prevFieldDefinitionChanged; + $fields[$nextFieldDefinitionChanged->getName()] = $nextFieldDefinitionChanged; + + $pairedDefinition = $fieldDefinition->getPairedDefinition($fields); + $localPairedDefinition = $fieldDefinitionChanged->getPairedDefinition($localFields); + + $this->assertFalse($pairedDefinition->isChangedData($localPairedDefinition)); + $this->assertTrue($pairedDefinition->isChangedName($localPairedDefinition)); + $this->assertTrue($pairedDefinition->isChanged($localPairedDefinition)); + } + + private function createPrev(FieldDefinition $fieldDefinition): FieldDefinition + { + $prevColumn = new Column(self::PREV_COLUMN, self::ID_COLUMN_DEF); + $prevFieldDefinition = new FieldDefinition($prevColumn); + $fieldDefinition->setPrevious($prevFieldDefinition); + $prevFieldDefinition->setNext($fieldDefinition); + + return $prevFieldDefinition; + } + + private function createNext(FieldDefinition $fieldDefinition): FieldDefinition + { + $nextColumn = new Column(self::NEXT_COLUMN, self::PASSWORD_COLUMN_DEF); + $nextFieldDefinition = new FieldDefinition($nextColumn); + $fieldDefinition->setNext($nextFieldDefinition); + $nextFieldDefinition->setPrevious($fieldDefinition); + + return $nextFieldDefinition; + } +}