Merge pull request #127 from BeMySlaveDarlin/issue/126-fix-update-col…

…umn-name #126 - Fix update column name alg
phalcon · Dec 7, 2021 · afbd64c · afbd64c
2 parents 4a57782 + 34e6d6e
commit afbd64c
Show file tree

Hide file tree

Showing 6 changed files with 47 additions and 27 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,6 +1,7 @@
 # [2.2.3](https://github.com/phalcon/migrations/releases/tag/v2.2.3) (2021-08-11)
 - Changed column modification behaviour during table morph ([#97](https://github.com/phalcon/migrations/issues/97))
 - Updated composer packages ([#124](https://github.com/phalcon/migrations/pull/124))
+- Changed column modification behaviour during table morph ([#126](https://github.com/phalcon/migrations/issues/126))
 
 # [2.2.2](https://github.com/phalcon/migrations/releases/tag/v2.2.2) (2021-08-08)
 - Integrated nette/php-generator, changed algorithm of migrations generation ([#90](https://github.com/phalcon/migrations/issues/90))

diff --git a/psalm.xml.dist b/psalm.xml.dist
@@ -15,5 +15,6 @@
     <issueHandlers>
         <MethodSignatureMismatch errorLevel="info" />
         <InvalidArgument errorLevel="info" />
+        <PossiblyUndefinedArrayOffset errorLevel="info" />
     </issueHandlers>
 </psalm>
diff --git a/src/Db/Adapter/Pdo/PdoPostgresql.php b/src/Db/Adapter/Pdo/PdoPostgresql.php
@@ -118,8 +118,8 @@ public function describeIndexes(string $table, string $schema = null): array
         foreach ($indexes as $name => $index) {
             $indexObjects[$name] = new Index(
                 $name,
-                $index['columns'] ?? [],
-                $index['type'] ?? ''
+                $index['columns'],
+                $index['type']
             );
         }
 

diff --git a/src/Db/FieldDefinition.php b/src/Db/FieldDefinition.php
@@ -126,10 +126,16 @@ public function isChangedName(FieldDefinition $comparingFieldDefinition): bool
 
     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();
+        $paramsToCheck = get_class_methods(ColumnInterface::class);
+        unset($paramsToCheck['getName']);
+
+        $comparingFieldColumn = $comparingFieldDefinition->getColumn();
+        foreach ($paramsToCheck as $method) {
+            if ($this->currentColumn->$method() !== $comparingFieldColumn->$method()) {
+                return true;
+            }
+        }
+
+        return false;
     }
 }
diff --git a/tests/mysql/MigrationsCest.php b/tests/mysql/MigrationsCest.php
@@ -445,7 +445,7 @@ public function updateColumnUnsigned(MysqlTester $mysqlTester): void
 
         $columns = $mysqlTester->getPhalconDb()->describeColumns($tableName);
 
-        $mysqlTester->asserttrue($columns[0]->isUnsigned());
+        $mysqlTester->assertTrue($columns[0]->isUnsigned());
     }
 
     public function nullableTimestamp(MysqlTester $I): void

diff --git a/tests/unit/Db/FieldDefinitionTest.php b/tests/unit/Db/FieldDefinitionTest.php
@@ -19,14 +19,22 @@
 
 final class FieldDefinitionTest extends Unit
 {
-    public const OLD_COLUMN_NAME = 'login';
-    public const NEW_COLUMN_NAME = 'username';
+    public const COLUMN_NAME = 'login';
     public const COLUMN_DEF = [
         'type' => Column::TYPE_VARCHAR,
         'notNull' => true,
         'size' => 2047,
         'after' => 'id',
     ];
+
+    public const NEW_COLUMN_NAME = 'username';
+    public const NEW_COLUMN_DEF = [
+        'type' => Column::TYPE_VARCHAR,
+        'notNull' => true,
+        'size' => 4096,
+        'after' => 'id',
+    ];
+
     public const PREV_COLUMN = 'id';
     public const ID_COLUMN_DEF = [
         'type' => Column::TYPE_INTEGER,
@@ -35,6 +43,7 @@ final class FieldDefinitionTest extends Unit
         'size' => 11,
         'first' => true,
     ];
+
     public const NEXT_COLUMN = 'password';
     public const PASSWORD_COLUMN_DEF = [
         'type' => Column::TYPE_VARCHAR,
@@ -44,15 +53,15 @@ final class FieldDefinitionTest extends Unit
 
     public function testCreate(): void
     {
-        $column = new Column(self::OLD_COLUMN_NAME, self::COLUMN_DEF);
+        $column = new Column(self::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);
+        $column = new Column(self::COLUMN_NAME, self::COLUMN_DEF);
         $fieldDefinition = new FieldDefinition($column);
 
         $prevColumn = new Column(self::PREV_COLUMN, self::ID_COLUMN_DEF);
@@ -69,32 +78,35 @@ public function testSetPrevAndNext(): void
 
     public function testNameChanged(): void
     {
-        $localFields = [];
-        $column = new Column(self::OLD_COLUMN_NAME, self::COLUMN_DEF);
+        $column = new Column(self::COLUMN_NAME, self::COLUMN_DEF);
+        $columnChanged = new Column(self::NEW_COLUMN_NAME, self::COLUMN_DEF);
+
         $fieldDefinition = new FieldDefinition($column);
+        $fieldDefinitionChanged = new FieldDefinition($columnChanged);
+
         $prevFieldDefinition = $this->createPrev($fieldDefinition);
         $nextFieldDefinition = $this->createNext($fieldDefinition);
 
+        $localFields = [];
         $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);
+        $pairedDefinition = $fieldDefinitionChanged->getPairedDefinition($localFields);
 
-        $fields[$fieldDefinitionChanged->getName()] = $fieldDefinitionChanged;
-        $fields[$prevFieldDefinitionChanged->getName()] = $prevFieldDefinitionChanged;
-        $fields[$nextFieldDefinitionChanged->getName()] = $nextFieldDefinitionChanged;
+        $this->assertNull($pairedDefinition);
+    }
 
-        $pairedDefinition = $fieldDefinition->getPairedDefinition($fields);
-        $localPairedDefinition = $fieldDefinitionChanged->getPairedDefinition($localFields);
+    public function testIsChangedData(): void
+    {
+        $column = new Column(self::COLUMN_NAME, self::COLUMN_DEF);
+        $fieldDefinition = new FieldDefinition($column);
+
+        $columnChanged = new Column(self::NEW_COLUMN_NAME, self::NEW_COLUMN_DEF);
+        $fieldDefinitionChanged = new FieldDefinition($columnChanged);
 
-        $this->assertFalse($pairedDefinition->isChangedData($localPairedDefinition));
-        $this->assertTrue($pairedDefinition->isChangedName($localPairedDefinition));
-        $this->assertTrue($pairedDefinition->isChanged($localPairedDefinition));
+        $this->assertFalse($fieldDefinition->isChangedData($fieldDefinition));
+        $this->assertTrue($fieldDefinition->isChangedData($fieldDefinitionChanged));
     }
 
     private function createPrev(FieldDefinition $fieldDefinition): FieldDefinition