Fix rolling back columns with indices for SQLite and SQL Server

.github/workflows/ci.yml

@@ -172,7 +172,7 @@ jobs:
 
     - name: Run PHPUnit
       env:
-        SQLSRV_DSN: 'sqlserver://(localdb)\MSSQLLocalDB/phinx'
+        SQLSRV_DSN: 'sqlsrv://(localdb)\MSSQLLocalDB/phinx'
         CODECOVERAGE: 1
       run: |
           vendor/bin/phpunit --verbose --coverage-clover=coverage.xml

src/Phinx/Db/Plan/Plan.php

@@ -190,19 +190,6 @@ protected function resolveConflicts(): void
             }
         }
 
-        // Columns that are dropped will automatically cause the indexes to be dropped as well
-        foreach ($this->columnRemoves as $columnRemove) {
-            foreach ($columnRemove->getActions() as $action) {
-                if ($action instanceof RemoveColumn) {
-                    [$this->indexes] = $this->forgetDropIndex(
-                        $action->getTable(),
-                        [$action->getColumn()->getName()],
-                        $this->indexes
-                    );
-                }
-            }
-        }
-
         // Renaming a column and then changing the renamed column is something people do,
         // but it is a conflicting action. Luckily solving the conflict can be done by moving
         // the ChangeColumn action to another AlterTable.

tests/Phinx/Migration/ManagerTest.php

@@ -5,6 +5,7 @@
 use InvalidArgumentException;
 use Phinx\Config\Config;
 use Phinx\Console\Command\AbstractCommand;
+use Phinx\Db\Adapter\AdapterInterface;
 use Phinx\Migration\Manager;
 use RuntimeException;
 use Symfony\Component\Console\Input\ArrayInput;
@@ -120,6 +121,38 @@ public function getConfigArray()
         ];
     }
 
+    /**
+     * Prepares an environment for cross DBMS functional tests.
+     *
+     * @param array $paths The paths config to override.
+     * @return \Phinx\Db\Adapter\AdapterInterface
+     */
+    protected function prepareEnvironment(array $paths = []): AdapterInterface
+    {
+        $configArray = $this->getConfigArray();
+
+        // override paths as needed
+        if ($paths) {
+            $configArray['paths'] = $paths + $configArray['paths'];
+        }
+        $configArray['environments']['production'] = DB_CONFIG;
+        $this->manager->setConfig(new Config($configArray));
+
+        $adapter = $this->manager->getEnvironment('production')->getAdapter();
+
+        // ensure the database is empty
+        if (DB_CONFIG['adapter'] === 'pgsql') {
+            $adapter->dropSchema('public');
+            $adapter->createSchema('public');
+        } elseif (DB_CONFIG['name'] !== ':memory:') {
+            $adapter->dropDatabase(DB_CONFIG['name']);
+            $adapter->createDatabase(DB_CONFIG['name']);
+        }
+        $adapter->disconnect();
+
+        return $adapter;
+    }
+
     public function testInstantiation()
     {
         $this->assertInstanceOf(
@@ -5509,23 +5542,11 @@ public function testGettingAnInvalidEnvironment()
 
     public function testReversibleMigrationsWorkAsExpected()
     {
-        if (!defined('MYSQL_DB_CONFIG')) {
-            $this->markTestSkipped('Mysql tests disabled.');
-        }
-        $configArray = $this->getConfigArray();
-        $adapter = $this->manager->getEnvironment('production')->getAdapter();
-
-        // override the migrations directory to use the reversible migrations
-        $configArray['paths']['migrations'] = $this->getCorrectedPath(__DIR__ . '/_files/reversiblemigrations');
-        $config = new Config($configArray);
-
-        // ensure the database is empty
-        $adapter->dropDatabase(MYSQL_DB_CONFIG['name']);
-        $adapter->createDatabase(MYSQL_DB_CONFIG['name']);
-        $adapter->disconnect();
+        $adapter = $this->prepareEnvironment([
+            'migrations' => $this->getCorrectedPath(__DIR__ . '/_files/reversiblemigrations'),
+        ]);
 
         // migrate to the latest version
-        $this->manager->setConfig($config);
         $this->manager->migrate('production');
 
         // ensure up migrations worked
@@ -5539,8 +5560,8 @@ public function testReversibleMigrationsWorkAsExpected()
         $this->assertTrue($adapter->hasTable('change_direction_test'));
         $this->assertTrue($adapter->hasColumn('change_direction_test', 'subthing'));
         $this->assertEquals(
-            count($adapter->fetchAll('SELECT * FROM change_direction_test WHERE subthing IS NOT NULL')),
-            2
+            2,
+            count($adapter->fetchAll('SELECT * FROM change_direction_test WHERE subthing IS NOT NULL'))
         );
 
         // revert all changes to the first
@@ -5554,6 +5575,12 @@ public function testReversibleMigrationsWorkAsExpected()
         $this->assertTrue($adapter->hasColumn('users', 'bio'));
         $this->assertFalse($adapter->hasForeignKey('user_logins', ['user_id']));
         $this->assertFalse($adapter->hasTable('change_direction_test'));
+
+        // revert all changes
+        $this->manager->rollback('production', '0');
+
+        $this->assertFalse($adapter->hasTable('info'));
+        $this->assertFalse($adapter->hasTable('users'));
     }
 
     public function testReversibleMigrationWithIndexConflict()

tests/Phinx/Migration/_files/reversiblemigrations/20170824134305_direction_aware_reversible_up.php

@@ -22,10 +22,10 @@ public function change()
                     'thing' => 'two',
                 ],
                 [
-                    'thing' => 'fox_socks',
+                    'thing' => 'fox-socks',
                 ],
                 [
-                    'thing' => 'mouse_box',
+                    'thing' => 'mouse-box',
                 ],
             ])->save();
         }

tests/Phinx/Migration/_files/reversiblemigrations/20170831121929_direction_aware_reversible_down.php

@@ -15,13 +15,18 @@ public function change()
             ->update();
 
         if ($this->isMigratingUp()) {
-            $this->execute("UPDATE change_direction_test
-                SET subthing = SUBSTRING(thing, LOCATE('_', thing) + 1),
-                    thing = LEFT(thing, LOCATE('_', thing) - 1)
-                WHERE thing LIKE '%\\\\_%'");
+            $query = $this->getQueryBuilder();
+            $query
+                ->update('change_direction_test')
+                ->set(['subthing' => $query->identifier('thing')])
+                ->where(['thing LIKE' => '%-%'])
+                ->execute();
         } else {
-            $this->execute("UPDATE change_direction_test
-                SET thing = CONCAT_WS('_', thing, subthing)");
+            $this
+                ->getQueryBuilder()
+                ->update('change_direction_test')
+                ->set(['subthing' => null])
+                ->execute();
         }
     }
 }

tests/Phinx/Migration/_files/reversiblemigrations/20190928220334_add_column_index_fk.php

@@ -33,21 +33,30 @@ class AddColumnIndexFk extends AbstractMigration
      */
     public function change()
     {
-        $this->table('statuses')
+        $table = $this->table('statuses')
             ->addColumn('user_id', Column::INTEGER, [
                 'null' => true,
                 'limit' => 20,
                 'signed' => false,
             ])
             ->addIndex(['user_id'], [
-                    'name' => 'statuses_users_id',
-                    'unique' => false,
-                ])
-            ->addForeignKey('user_id', 'users', 'id', [
-                    'constraint' => 'statuses_users_id',
-                    'update' => ForeignKey::RESTRICT,
-                    'delete' => ForeignKey::RESTRICT,
-                ])
-            ->update();
+                'name' => 'statuses_users_id',
+                'unique' => false,
+            ]);
+
+        if ($this->getAdapter()->getConnection()->getAttribute(\PDO::ATTR_DRIVER_NAME) === 'sqlite') {
+            $table->addForeignKey('user_id', 'users', 'id', [
+                'update' => ForeignKey::NO_ACTION,
+                'delete' => ForeignKey::NO_ACTION,
+            ]);
+        } else {
+            $table->addForeignKey('user_id', 'users', 'id', [
+                'constraint' => 'statuses_users_id',
+                'update' => ForeignKey::NO_ACTION,
+                'delete' => ForeignKey::NO_ACTION,
+            ]);
+        }
+
+        $table->update();
     }
 }

tests/phpunit-bootstrap.php

@@ -25,16 +25,20 @@
 
 if (getenv('SQLITE_DSN')) {
     define('SQLITE_DB_CONFIG', Util::parseDsn(getenv('SQLITE_DSN')));
+    define('DB_CONFIG', SQLITE_DB_CONFIG);
 }
 
 if (getenv('MYSQL_DSN')) {
     define('MYSQL_DB_CONFIG', Util::parseDsn(getenv('MYSQL_DSN')));
+    define('DB_CONFIG', MYSQL_DB_CONFIG);
 }
 
 if (getenv('PGSQL_DSN')) {
     define('PGSQL_DB_CONFIG', Util::parseDsn(getenv('PGSQL_DSN')));
+    define('DB_CONFIG', PGSQL_DB_CONFIG);
 }
 
 if (getenv('SQLSRV_DSN')) {
     define('SQLSRV_DB_CONFIG', Util::parseDsn(getenv('SQLSRV_DSN')));
+    define('DB_CONFIG', SQLSRV_DB_CONFIG);
 }