Permalink
Browse files

Ref #381. Ported propelorm/Propel#618

  • Loading branch information...
marcj committed May 14, 2013
1 parent 02a87a9 commit 4dfdfe16842e953e824fb55690fad63c4b7224ea
@@ -11,6 +11,7 @@
namespace Propel\Generator\Reverse;
use Propel\Generator\Config\GeneratorConfigInterface;
+use Propel\Generator\Model\VendorInfo;
use Propel\Runtime\Connection\ConnectionInterface;
/**
@@ -122,9 +122,7 @@ public function parse(Database $database)
$this->addIndexes($table);
$this->addPrimaryKey($table);
- if ($this->addVendorInfo) {
- $this->addTableVendorInfo($table);
- }
+ $this->addTableVendorInfo($table);
}
return count($tables);
@@ -401,6 +399,10 @@ protected function addTableVendorInfo(Table $table)
{
$stmt = $this->dbh->query("SHOW TABLE STATUS LIKE '" . $table->getName() . "'");
$row = $stmt->fetch(\PDO::FETCH_ASSOC);
+ if (!$this->addVendorInfo) {
+ // since we depend on `Engine` in the MysqlPlatform, we always have to extract this vendor information
+ $row = array('Engine' => $row['Engine']);
+ }
$vi = $this->getNewVendorInfoObject($row);
$table->addVendorInfo($vi);
}
@@ -0,0 +1,78 @@
+<?php
+
+namespace Propel\Tests\Helpers;
+
+use Propel\Generator\Config\QuickGeneratorConfig;
+use Propel\Generator\Model\Database;
+use Propel\Generator\Model\Diff\DatabaseComparator;
+use Propel\Generator\Platform\MysqlPlatform;
+use Propel\Generator\Reverse\MysqlSchemaParser;
+use Propel\Generator\Util\SqlParser;
+use Propel\Runtime\Propel;
+
+class PlatformDatabaseBuildTimeBase extends \PHPUnit_Framework_TestCase
+{
+
+ /**
+ * @var Database
+ */
+ public $database;
+
+ /**
+ * @var MysqlSchemaParser
+ */
+ public $parser;
+
+ /**
+ * @var MysqlPlatform
+ */
+ public $platform;
+
+ /**
+ * @var PDO
+ */
+ public $con;
+
+ protected function setUp()
+ {
+ include(__DIR__ . '/../../../Fixtures/reverse/mysql/build/conf/reverse-bookstore-conf.php');
+
+ $this->con = Propel::getConnection('reverse-bookstore');
+
+ $this->parser = new MysqlSchemaParser($this->con);
+ $this->platform = new MysqlPlatform();
+
+ $this->parser->setGeneratorConfig(new QuickGeneratorConfig());
+ $this->parser->setPlatform($this->platform);
+ parent::setUp();
+ }
+
+ public function readDatabase()
+ {
+ $this->database = new Database();
+ $this->database->setPlatform($this->platform);
+ $this->parser->parse($this->database);
+ }
+
+ /**
+ * Detects the differences between current connected database and $pDatabase
+ * and updates the schema. This does not DROP tables.
+ *
+ * @param Database $pDatabase
+ */
+ public function updateSchema($pDatabase)
+ {
+ $diff = DatabaseComparator::computeDiff($this->database, $pDatabase);
+ $sql = $this->database->getPlatform()->getModifyDatabaseDDL($diff);
+
+ $statements = SqlParser::parseString($sql);
+ foreach ($statements as $statement) {
+ if (strpos($statement, 'DROP') === 0) {
+ // drop statements cause errors since the table doesn't exist
+ continue;
+ }
+ $stmt = $this->con->prepare($statement);
+ $stmt->execute();
+ }
+ }
+}
@@ -0,0 +1,188 @@
+<?php
+
+namespace Propel\Tests\Issues;
+
+use Propel\Generator\Model\Diff\DatabaseComparator;
+use Propel\Generator\Util\QuickBuilder;
+use Propel\Tests\Helpers\PlatformDatabaseBuildTimeBase;
+
+/**
+ * This test proves the bug described in https://github.com/propelorm/Propel/issues/617.
+ * Since the build property `addVendorInfo` is per default not set (= false), the `MysqlSchemaParser` **did**
+ * not return the `Engine` of the table. Since we depend on that information in `MysqlPlatform`,
+ * we really need that kind of information.
+ *
+ */
+class Issue617Test extends PlatformDatabaseBuildTimeBase
+{
+ /**
+ * Contains the builder instance of the updated schema (removed FK)
+ * @var PropelQuickBuilder
+ */
+ private $updatedBuilder;
+
+ protected function setUp()
+ {
+ parent::setUp();
+ $this->removeTables();
+ }
+
+ protected function tearDown()
+ {
+ $this->removeTables();
+ parent::tearDown();
+ }
+
+ /**
+ * Remove issue617 tables.
+ */
+ public function removeTables()
+ {
+ $this->con->query('DROP TABLE IF EXISTS `issue617_user`');
+ $this->con->query('DROP TABLE IF EXISTS `issue617_group`');
+ }
+
+ /**
+ * Setups the initial schema.
+ */
+ private function setupInitSchema(){
+
+ /*
+ * Create issue617 tables with foreign keys
+ */
+ $schema = '
+<database name="bookstore">
+<table name="issue617_user">
+ <vendor type="mysql">
+ <parameter name="Engine" value="InnoDB"/>
+ <parameter name="Charset" value="utf8"/>
+ </vendor>
+ <column name="id" type="INTEGER" required="true" primaryKey="true" autoIncrement="true" />
+ <column name="full_name" type="VARCHAR" size="50" required="true" />
+
+ <!-- this column (and FK) will be removed from schema, but not from DB on migrate -->
+ <column name="group_id" type="INTEGER" />
+ <foreign-key foreignTable="issue617_group" onDelete="setnull">
+ <reference local="group_id" foreign="id" />
+ </foreign-key>
+</table>
+
+<table name="issue617_group">
+ <vendor type="mysql">
+ <parameter name="Engine" value="InnoDB"/>
+ <parameter name="Charset" value="utf8"/>
+ </vendor>
+ <column name="id" type="INTEGER" required="true" primaryKey="true" autoIncrement="true" />
+ <column name="name" type="VARCHAR" size="50" required="true" />
+</table>
+</database>
+';
+
+ $builder = new QuickBuilder();
+ $builder->setPlatform($this->database->getPlatform());
+ $builder->setSchema($schema);
+
+ $diff = DatabaseComparator::computeDiff($this->database, $builder->getDatabase());
+ $sql = $this->database->getPlatform()->getModifyDatabaseDDL($diff);
+
+ $expected = '
+CREATE TABLE `issue617_user`
+(
+ `id` INTEGER NOT NULL AUTO_INCREMENT,
+ `full_name` VARCHAR(50) NOT NULL,
+ `group_id` INTEGER,
+ PRIMARY KEY (`id`),
+ INDEX `issue617_user_FI_1` (`group_id`),
+ CONSTRAINT `issue617_user_FK_1`
+ FOREIGN KEY (`group_id`)
+ REFERENCES `issue617_group` (`id`)
+ ON DELETE SET NULL
+) ENGINE=InnoDB CHARACTER SET=\'utf8\';
+
+CREATE TABLE `issue617_group`
+(
+ `id` INTEGER NOT NULL AUTO_INCREMENT,
+ `name` VARCHAR(50) NOT NULL,
+ PRIMARY KEY (`id`)
+) ENGINE=InnoDB CHARACTER SET=\'utf8\';
+';
+
+ $this->assertContains($expected, $sql);
+ $this->updateSchema($builder->getDatabase());
+
+ }
+
+ /**
+ * Drop the foreign key in the `_user` table and check whether it generates
+ * the correct `DROP` SQL.
+ */
+ private function dropForeignKey(){
+ $this->readDatabase();
+ $updatedSchema = '
+<database name="reverse-bookstore">
+<table name="issue617_user">
+ <vendor type="mysql">
+ <parameter name="Engine" value="InnoDB"/>
+ <parameter name="Charset" value="utf8"/>
+ </vendor>
+ <column name="id" type="INTEGER" required="true" primaryKey="true" autoIncrement="true" />
+ <column name="full_name" type="VARCHAR" size="50" required="true" />
+</table>
+
+<table name="issue617_group">
+ <vendor type="mysql">
+ <parameter name="Engine" value="InnoDB"/>
+ <parameter name="Charset" value="utf8"/>
+ </vendor>
+ <column name="id" type="INTEGER" required="true" primaryKey="true" autoIncrement="true" />
+ <column name="name" type="VARCHAR" size="50" required="true" />
+</table>
+</database>
+';
+
+ $this->updatedBuilder = new QuickBuilder();
+ $this->updatedBuilder->setPlatform($this->database->getPlatform());
+ $this->updatedBuilder->setSchema($updatedSchema);
+
+ $diff = DatabaseComparator::computeDiff($this->database, $this->updatedBuilder->getDatabase());
+ $sql = $this->database->getPlatform()->getModifyDatabaseDDL($diff);
+
+ $expected = '
+ALTER TABLE `issue617_user` DROP FOREIGN KEY `issue617_user_FK_1`;
+
+DROP INDEX `issue617_user_FI_1` ON `issue617_user`;
+
+ALTER TABLE `issue617_user` DROP `group_id`;
+';
+
+ $this->assertContains($expected, $sql);
+ $this->updateSchema($this->updatedBuilder->getDatabase());
+ }
+
+ /*
+ * Checks if FKs are really deleted.
+ */
+ private function checkDeletedFk(){
+ $this->readDatabase();
+ $diff = DatabaseComparator::computeDiff($this->database, $this->updatedBuilder->getDatabase());
+ $sql = $this->database->getPlatform()->getModifyDatabaseDDL($diff);
+
+ $expected = 'issue617_user';
+
+ $this->assertNotContains($expected, $sql);
+ }
+
+ /**
+ * Checks if a changed schema with removed FK does really delete the FK.
+ * Based on a real use-case, reverse classes and `computeDiff`.
+ */
+ public function testDropForeignKey()
+ {
+ $this->readDatabase();
+
+ $this->setupInitSchema();
+ $this->dropForeignKey();
+ $this->checkDeletedFk();
+
+ }
+}
@@ -71,6 +71,15 @@ propel.database.password =
#
# -------------------------------------------------------------------
+# samePhpName
+# If true, the reverse task will set the phpName attribute for the
+# tables and columns to be the same as SQL name.
+#
+# addVendorInfo
+# If true, the reverse task will add all vendor specific information
+# to the database schema. Under `mysql` the `Engine` vendor information
+# is always added.
+#
propel.samePhpName = false
propel.addVendorInfo = false

0 comments on commit 4dfdfe1

Please sign in to comment.