Merge pull request #21 from camcima/hotfix/table-drops

camcima · web-flow · commit 34b9969bb5fa · 2017-11-02T02:46:24.000-04:00
DROP first
diff --git a/src/Differ.php b/src/Differ.php
@@ -38,7 +38,10 @@ public function diffDatabases(Database $fromDatabase, Database $toDatabase, arra
             if ($fromTable->generateCreationScript(true) !== $toTable->generateCreationScript(true)) {
                 $changedTable = new ChangedTable($fromTable, $toTable);
                 $this->diffChangedTable($changedTable);
-                $databaseDiff->addChangedTable($changedTable);
+
+                if (!empty($changedTable->generateAlterScript())) {
+                    $databaseDiff->addChangedTable($changedTable);
+                }
             }
         }
 
diff --git a/src/Model/ChangedTable.php b/src/Model/ChangedTable.php
@@ -303,30 +303,31 @@ public function addChangedForeignKey(ForeignKey $changedForeignKey)
      */
     public function generateAlterScript()
     {
+        $tableDrops = [];
         $tableChanges = [];
 
         if ($this->deletedPrimaryKey || (!empty($this->fromTable->getPrimaryKeys()) && !empty($this->changedPrimaryKeys))) {
-            $tableChanges[] = 'DROP PRIMARY KEY';
+            $tableDrops[] = 'DROP PRIMARY KEY';
         }
 
         foreach ($this->deletedForeignKeys as $deletedForeignKey) {
-            $tableChanges[] = sprintf('DROP FOREIGN KEY `%s`', $deletedForeignKey->getName());
+            $tableDrops[] = sprintf('DROP FOREIGN KEY `%s`', $deletedForeignKey->getName());
         }
 
         foreach ($this->changedForeignKeys as $changedForeignKey) {
-            $tableChanges[] = sprintf('DROP FOREIGN KEY `%s`', $changedForeignKey->getName());
+            $tableDrops[] = sprintf('DROP FOREIGN KEY `%s`', $changedForeignKey->getName());
         }
 
         foreach ($this->deletedIndexes as $deletedIndex) {
-            $tableChanges[] = sprintf('DROP INDEX `%s`', $deletedIndex->getName());
+            $tableDrops[] = sprintf('DROP INDEX `%s`', $deletedIndex->getName());
         }
 
         foreach ($this->changedIndexes as $changedIndex) {
-            $tableChanges[] = sprintf('DROP INDEX `%s`', $changedIndex->getName());
+            $tableDrops[] = sprintf('DROP INDEX `%s`', $changedIndex->getName());
         }
 
         foreach ($this->deletedColumns as $deletedColumn) {
-            $tableChanges[] = sprintf('DROP COLUMN `%s`', $deletedColumn->getName());
+            $tableDrops[] = sprintf('DROP COLUMN `%s`', $deletedColumn->getName());
         }
 
         $columnStatements = [];
@@ -390,9 +391,17 @@ public function generateAlterScript()
             $tableChanges[] = sprintf('COMMENT=\'%s\'', str_replace('\'', '\'\'', $this->toTable->getComment()));
         }
 
-        $alterScript = sprintf('ALTER TABLE `%s`%s  %s;', $this->getName(), PHP_EOL, implode(',' . PHP_EOL . '  ', $tableChanges));
+        $alterScripts = [];
 
-        return $alterScript;
+        if (!empty($tableDrops)) {
+            $alterScripts[] = sprintf('ALTER TABLE `%s`%s  %s;', $this->getName(), PHP_EOL, implode(',' . PHP_EOL . '  ', $tableDrops));
+        }
+
+        if (!empty($tableChanges)) {
+            $alterScripts[] = sprintf('ALTER TABLE `%s`%s  %s;', $this->getName(), PHP_EOL, implode(',' . PHP_EOL . '  ', $tableChanges));
+        }
+
+        return implode(PHP_EOL, $alterScripts);
     }
 
     /**
diff --git a/src/RegExpPattern.php b/src/RegExpPattern.php
@@ -35,7 +35,7 @@ class RegExpPattern
     public static function tables()
     {
         $pattern = '/(?<creationScript>CREATE\s+TABLE\s+(?<ifNotExists>IF NOT EXISTS)?\s*`(?<tableName>\S+)`\s+';
-        $pattern .= '\((?<tableDefinition>[^\/]+)\)';
+        $pattern .= '\((?<tableDefinition>[^\/]+?)\)';
         $pattern .= '(';
         $pattern .= '(?:\s+ENGINE\s*=\s*(?<engine>[^;\s]+))?\s*';
         $pattern .= '|';
diff --git a/tests/DifferTest.php b/tests/DifferTest.php
@@ -101,4 +101,17 @@ public function testIsGeneratingMigrationScript()
 
         $this->assertEquals($this->getDatabaseFixture('sakila_migration.sql'), $result);
     }
+
+    public function testIsDiffingDeletedForeignKeyColumn()
+    {
+        $parser = new Parser();
+
+        $fromDatabase = $parser->parseDatabase($this->getDatabaseFixture('fk_deleted_column1.sql'));
+        $toDatabase = $parser->parseDatabase($this->getDatabaseFixture('fk_deleted_column2.sql'));
+
+        $differ = new Differ();
+        $databaseDiff = $differ->diffDatabases($fromDatabase, $toDatabase);
+
+        $this->assertEquals($this->getDatabaseFixture('fk_deleted_column_migration.sql'), $differ->generateMigrationScript($databaseDiff));
+    }
 }
diff --git a/tests/fixtures/fk_deleted_column1.sql b/tests/fixtures/fk_deleted_column1.sql
@@ -0,0 +1,33 @@
+-- --------------------------------------------------------
+-- Host:                         127.0.0.1
+-- Server version:               5.7.10-log - MySQL Community Server (GPL)
+-- Server OS:                    Win64
+-- HeidiSQL Version:             9.4.0.5174
+-- --------------------------------------------------------
+
+/*!40101 SET @OLD_CHARACTER_SET_CLIENT=@@CHARACTER_SET_CLIENT */;
+/*!40101 SET NAMES utf8 */;
+/*!50503 SET NAMES utf8mb4 */;
+/*!40014 SET @OLD_FOREIGN_KEY_CHECKS=@@FOREIGN_KEY_CHECKS, FOREIGN_KEY_CHECKS=0 */;
+/*!40101 SET @OLD_SQL_MODE=@@SQL_MODE, SQL_MODE='NO_AUTO_VALUE_ON_ZERO' */;
+
+-- Dumping structure for table sakila.table_1
+CREATE TABLE IF NOT EXISTS `table_1` (
+  `id` int(11) NOT NULL AUTO_INCREMENT,
+  `foreignId` int(11) DEFAULT NULL,
+  PRIMARY KEY (`id`),
+  KEY `FK_table_1_table_2` (`foreignId`),
+  CONSTRAINT `FK_table_1_table_2` FOREIGN KEY (`foreignId`) REFERENCES `table_2` (`id`) ON DELETE NO ACTION ON UPDATE NO ACTION
+) ENGINE=InnoDB DEFAULT CHARSET=latin1 COMMENT='.';
+
+-- Data exporting was unselected.
+-- Dumping structure for table sakila.table_2
+CREATE TABLE IF NOT EXISTS `table_2` (
+  `id` int(11) NOT NULL AUTO_INCREMENT,
+  PRIMARY KEY (`id`)
+) ENGINE=InnoDB DEFAULT CHARSET=latin1 COMMENT='.';
+
+-- Data exporting was unselected.
+/*!40101 SET SQL_MODE=IFNULL(@OLD_SQL_MODE, '') */;
+/*!40014 SET FOREIGN_KEY_CHECKS=IF(@OLD_FOREIGN_KEY_CHECKS IS NULL, 1, @OLD_FOREIGN_KEY_CHECKS) */;
+/*!40101 SET CHARACTER_SET_CLIENT=@OLD_CHARACTER_SET_CLIENT */;
diff --git a/tests/fixtures/fk_deleted_column2.sql b/tests/fixtures/fk_deleted_column2.sql
@@ -0,0 +1,17 @@
+-- Host: 127.0.0.1    Database: test_2
+
+DROP TABLE IF EXISTS `table_1`;
+CREATE TABLE `table_1` (
+  `id` int(11) NOT NULL AUTO_INCREMENT,
+  `foreignId2` int(11) DEFAULT NULL,
+  PRIMARY KEY (`id`),
+  KEY `FK_table_1_table_2` (`foreignId2`),
+  CONSTRAINT `FK_table_1_table_2` FOREIGN KEY (`foreignId2`) REFERENCES `table_2` (`id`) ON DELETE NO ACTION ON UPDATE NO ACTION
+) ENGINE=InnoDB DEFAULT CHARSET=latin1 COMMENT='.';
+
+
+DROP TABLE IF EXISTS `table_2`;
+CREATE TABLE `table_2` (
+  `id` int(11) NOT NULL AUTO_INCREMENT,
+  PRIMARY KEY (`id`)
+) ENGINE=InnoDB DEFAULT CHARSET=latin1 COMMENT='.';
diff --git a/tests/fixtures/fk_deleted_column_migration.sql b/tests/fixtures/fk_deleted_column_migration.sql
@@ -0,0 +1,23 @@
+# Disable Foreign Keys Check
+SET FOREIGN_KEY_CHECKS = 0;
+SET SQL_MODE = '';
+
+# Deleted Tables
+
+# Changed Tables
+
+-- changed table `table_1`
+
+ALTER TABLE `table_1`
+  DROP FOREIGN KEY `FK_table_1_table_2`,
+  DROP INDEX `FK_table_1_table_2`,
+  DROP COLUMN `foreignId`;
+ALTER TABLE `table_1`
+  ADD COLUMN `foreignId2` int(11) DEFAULT NULL AFTER `id`,
+  ADD KEY `FK_table_1_table_2` (`foreignId2`),
+  ADD CONSTRAINT `FK_table_1_table_2` FOREIGN KEY (`foreignId2`) REFERENCES `table_2` (`id`) ON DELETE NO ACTION ON UPDATE NO ACTION;
+
+# New Tables
+
+# Disable Foreign Keys Check
+SET FOREIGN_KEY_CHECKS = 1;
diff --git a/tests/fixtures/sakila_migration.sql b/tests/fixtures/sakila_migration.sql
@@ -15,7 +15,8 @@ DROP TABLE `test1`;
 ALTER TABLE `test2`
   DROP PRIMARY KEY,
   DROP FOREIGN KEY `FK__test1`,
-  DROP INDEX `FK__test1`,
+  DROP INDEX `FK__test1`;
+ALTER TABLE `test2`
   CHANGE COLUMN `id` `id` int(10) NOT NULL AUTO_INCREMENT FIRST,
   CHANGE COLUMN `fk` `fk` int(10) AFTER `id`,
   CHANGE COLUMN `val` `val` decimal(11,3) NOT NULL AFTER `fk`,

Original file line number	Diff line number	Diff line change
`@@ -38,7 +38,10 @@ public function diffDatabases(Database $fromDatabase, Database $toDatabase, arra`
`38`	`38`	`if ($fromTable->generateCreationScript(true) !== $toTable->generateCreationScript(true)) {`
`39`	`39`	`$changedTable = new ChangedTable($fromTable, $toTable);`
`40`	`40`	`$this->diffChangedTable($changedTable);`
`41`		`- $databaseDiff->addChangedTable($changedTable);`
	`41`	`+`
	`42`	`+ if (!empty($changedTable->generateAlterScript())) {`
	`43`	`+ $databaseDiff->addChangedTable($changedTable);`
	`44`	`+ }`
`42`	`45`	`}`
`43`	`46`	`}`
`44`	`47`
Original file line number	Diff line number	Diff line change
`@@ -303,30 +303,31 @@ public function addChangedForeignKey(ForeignKey $changedForeignKey)`
`303`	`303`	`*/`
`304`	`304`	`public function generateAlterScript()`
`305`	`305`	`{`
	`306`	`+ $tableDrops = [];`
`306`	`307`	`$tableChanges = [];`
`307`	`308`
`308`	`309`	`if ($this->deletedPrimaryKey \|\| (!empty($this->fromTable->getPrimaryKeys()) && !empty($this->changedPrimaryKeys))) {`
`309`		`- $tableChanges[] = 'DROP PRIMARY KEY';`
	`310`	`+ $tableDrops[] = 'DROP PRIMARY KEY';`
`310`	`311`	`}`
`311`	`312`
`312`	`313`	`foreach ($this->deletedForeignKeys as $deletedForeignKey) {`
`313`		- $tableChanges[] = sprintf('DROP FOREIGN KEY `%s`', $deletedForeignKey->getName());
	`314`	+ $tableDrops[] = sprintf('DROP FOREIGN KEY `%s`', $deletedForeignKey->getName());
`314`	`315`	`}`
`315`	`316`
`316`	`317`	`foreach ($this->changedForeignKeys as $changedForeignKey) {`
`317`		- $tableChanges[] = sprintf('DROP FOREIGN KEY `%s`', $changedForeignKey->getName());
	`318`	+ $tableDrops[] = sprintf('DROP FOREIGN KEY `%s`', $changedForeignKey->getName());
`318`	`319`	`}`
`319`	`320`
`320`	`321`	`foreach ($this->deletedIndexes as $deletedIndex) {`
`321`		- $tableChanges[] = sprintf('DROP INDEX `%s`', $deletedIndex->getName());
	`322`	+ $tableDrops[] = sprintf('DROP INDEX `%s`', $deletedIndex->getName());
`322`	`323`	`}`
`323`	`324`
`324`	`325`	`foreach ($this->changedIndexes as $changedIndex) {`
`325`		- $tableChanges[] = sprintf('DROP INDEX `%s`', $changedIndex->getName());
	`326`	+ $tableDrops[] = sprintf('DROP INDEX `%s`', $changedIndex->getName());
`326`	`327`	`}`
`327`	`328`
`328`	`329`	`foreach ($this->deletedColumns as $deletedColumn) {`
`329`		- $tableChanges[] = sprintf('DROP COLUMN `%s`', $deletedColumn->getName());
	`330`	+ $tableDrops[] = sprintf('DROP COLUMN `%s`', $deletedColumn->getName());
`330`	`331`	`}`
`331`	`332`
`332`	`333`	`$columnStatements = [];`
`@@ -390,9 +391,17 @@ public function generateAlterScript()`
`390`	`391`	`$tableChanges[] = sprintf('COMMENT=\'%s\'', str_replace('\'', '\'\'', $this->toTable->getComment()));`
`391`	`392`	`}`
`392`	`393`
`393`		- $alterScript = sprintf('ALTER TABLE `%s`%s %s;', $this->getName(), PHP_EOL, implode(',' . PHP_EOL . ' ', $tableChanges));
	`394`	`+ $alterScripts = [];`
`394`	`395`
`395`		`- return $alterScript;`
	`396`	`+ if (!empty($tableDrops)) {`
	`397`	+ $alterScripts[] = sprintf('ALTER TABLE `%s`%s %s;', $this->getName(), PHP_EOL, implode(',' . PHP_EOL . ' ', $tableDrops));
	`398`	`+ }`
	`399`	`+`
	`400`	`+ if (!empty($tableChanges)) {`
	`401`	+ $alterScripts[] = sprintf('ALTER TABLE `%s`%s %s;', $this->getName(), PHP_EOL, implode(',' . PHP_EOL . ' ', $tableChanges));
	`402`	`+ }`
	`403`	`+`
	`404`	`+ return implode(PHP_EOL, $alterScripts);`
`396`	`405`	`}`
`397`	`406`
`398`	`407`	`/**`
Original file line number	Diff line number	Diff line change
`@@ -35,7 +35,7 @@ class RegExpPattern`
`35`	`35`	`public static function tables()`
`36`	`36`	`{`
`37`	`37`	$pattern = '/(?<creationScript>CREATE\s+TABLE\s+(?<ifNotExists>IF NOT EXISTS)?\s*`(?<tableName>\S+)`\s+';
`38`		`- $pattern .= '\((?<tableDefinition>[^\/]+)\)';`
	`38`	`+ $pattern .= '\((?<tableDefinition>[^\/]+?)\)';`
`39`	`39`	`$pattern .= '(';`
`40`	`40`	`$pattern .= '(?:\s+ENGINE\s=\s(?<engine>[^;\s]+))?\s*';`
`41`	`41`	`$pattern .= '\|';`