Skip to content

Commit

Permalink
phalcon#97 - Renaming column does not drops column on morphTable
Browse files Browse the repository at this point in the history
  • Loading branch information
@BeMySlaveDarlin
BeMySlaveDarlin committed Aug 9, 2021
1 parent 0e76b22 commit 1b6b57d
Show file tree
Hide file tree
Showing 3 changed files with 298 additions and 44 deletions.
135 changes: 135 additions & 0 deletions src/Db/FieldDefinition.php
View file
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 <team@phalcon.io>
*
* 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
View file
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

0 comments on commit 1b6b57d

Please sign in to comment.