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

ISSUE-97: Renaming column does not drops column on morphTable #123

Merged
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
135 changes: 135 additions & 0 deletions src/Db/FieldDefinition.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
<?php

/**
* This file is part of the Phalcon Migrations.
*
* (c) Phalcon Team <[email protected]>
*
* 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();
}
}
88 changes: 44 additions & 44 deletions src/Mvc/Model/Migration.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,14 @@
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;
use Phalcon\Events\Manager as EventsManager;
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;
Expand Down Expand Up @@ -434,6 +434,7 @@ public function morphTable(string $tableName, array $definition): void
}

$fields = [];
$previousField = null;
/** @var ColumnInterface $tableColumn */
foreach ($definition['columns'] as $tableColumn) {
/**
Expand All @@ -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()
Expand All @@ -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()
Expand All @@ -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()
Expand Down Expand Up @@ -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';
}
Expand Down Expand Up @@ -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)) {
Expand All @@ -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
Expand All @@ -852,7 +852,7 @@ function ($value) {
*
* @return AbstractAdapter
*/
public function getConnection()
public function getConnection(): AbstractAdapter
{
return self::$connection;
}
Expand Down Expand Up @@ -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;
Expand Down
Loading