diff --git a/UPGRADE_TO_2_0 b/UPGRADE_TO_2_0 index 86fb2bfdc..fa273e17c 100644 --- a/UPGRADE_TO_2_0 +++ b/UPGRADE_TO_2_0 @@ -4,11 +4,25 @@ # Upgrade from 2.0-ALPHA3 to 2.0-ALPHA4 +## Changes in Method Signatures + + * A bunch of Methods on both Doctrine\DBAL\Platforms\AbstractPlatform and Doctrine\DBAL\Schema\AbstractSchemaManager + have changed quite significantly by adopting the new Schema instance objects. + ## Renamed Methods * Doctrine\ORM\AbstractQuery::setExpireResultCache() -> expireResultCache() * Doctrine\ORM\Query::setExpireQueryCache() -> expireQueryCache() +## WARNING: Change in SchemaTool behaviour + + * "doctrine schema-tool --drop" now always drops the complete database instead of only those tables defined by the + current database model. The previous method had problems when foreign keys of orphaned tables pointed to + tables that were schedulded for deletion. + * Use "doctrine schema-tool --update" to get a save incremental update for your database schema without + deleting any unused tables, sequences or foreign keys. + * Use "doctrine schema-tool --complete-update" to do a full incremental update of your schema. + # Upgrade from 2.0-ALPHA2 to 2.0-ALPHA3 This section details the changes made to Doctrine 2.0-ALPHA3 to make it easier for you diff --git a/lib/Doctrine/DBAL/Schema/Schema.php b/lib/Doctrine/DBAL/Schema/Schema.php index 19a97917f..b058c13e5 100644 --- a/lib/Doctrine/DBAL/Schema/Schema.php +++ b/lib/Doctrine/DBAL/Schema/Schema.php @@ -266,23 +266,25 @@ class Schema extends AbstractAsset } /** - * @param Schema $schema + * @param Schema $toSchema * @param AbstractPlatform $platform */ - public function getMigrateToSql(Schema $schema, \Doctrine\DBAL\Platforms\AbstractPlatform $platform) + public function getMigrateToSql(Schema $toSchema, \Doctrine\DBAL\Platforms\AbstractPlatform $platform) { $comparator = new Comparator(); - $schemaDiff = $comparator->compare($this, $schema); + $schemaDiff = $comparator->compare($this, $toSchema); + return $schemaDiff->toSql($platform); } /** - * @param Schema $schema + * @param Schema $fromSchema * @param AbstractPlatform $platform */ - public function getMigrateFromSql(Schema $schema, \Doctrine\DBAL\Platforms\AbstractPlatform $platform) + public function getMigrateFromSql(Schema $fromSchema, \Doctrine\DBAL\Platforms\AbstractPlatform $platform) { $comparator = new Comparator(); - $schemaDiff = $comparator->compare($schema, $this); + $schemaDiff = $comparator->compare($fromSchema, $this); + return $schemaDiff->toSql($platform); } /** diff --git a/lib/Doctrine/DBAL/Schema/SchemaDiff.php b/lib/Doctrine/DBAL/Schema/SchemaDiff.php index a3d737fda..80b7aa76e 100644 --- a/lib/Doctrine/DBAL/Schema/SchemaDiff.php +++ b/lib/Doctrine/DBAL/Schema/SchemaDiff.php @@ -126,7 +126,7 @@ class SchemaDiff { $sql = array(); - if ($platform->supportsForeignKeyConstraints()) { + if ($platform->supportsForeignKeyConstraints() && $saveMode == false) { foreach ($this->orphanedForeignKeys AS $orphanedForeignKey) { $sql[] = $platform->getDropForeignKeySql($orphanedForeignKey, $orphanedForeignKey->getLocalTableName()); } @@ -156,7 +156,7 @@ class SchemaDiff ); } - if ($saveMode === true) { + if ($saveMode === false) { foreach ($this->removedTables AS $table) { $sql[] = $platform->getDropTableSql($table); } diff --git a/lib/Doctrine/DBAL/Schema/SchemaException.php b/lib/Doctrine/DBAL/Schema/SchemaException.php index b72833e64..eabe02e58 100644 --- a/lib/Doctrine/DBAL/Schema/SchemaException.php +++ b/lib/Doctrine/DBAL/Schema/SchemaException.php @@ -110,11 +110,6 @@ class SchemaException extends \Doctrine\DBAL\DBALException return new self("There exists no foreign key with the name '".$fkName."'.", self::FOREIGNKEY_DOESNT_EXIST); } - static public function invalidCaseModeGiven() - { - return new self("Invalid case mode given to Schema Asset."); - } - static public function namedForeignKeyRequired($localTable, $foreignKey) { return new self( diff --git a/lib/Doctrine/ORM/Tools/Cli/Tasks/SchemaToolTask.php b/lib/Doctrine/ORM/Tools/Cli/Tasks/SchemaToolTask.php index 9f8f89998..c2d154bc2 100644 --- a/lib/Doctrine/ORM/Tools/Cli/Tasks/SchemaToolTask.php +++ b/lib/Doctrine/ORM/Tools/Cli/Tasks/SchemaToolTask.php @@ -57,15 +57,23 @@ class SchemaToolTask extends AbstractTask 'If defined, --drop, --update and --re-create can not be requested on same task.' ), new Option( - 'drop', '', + 'drop', null, 'Drops the schema of EntityManager (drop tables on Database).' . PHP_EOL . - 'Defaults to "metadata" if only --drop is specified.' . PHP_EOL . + 'Beware that the complete database is dropped by this command, '.PHP_EOL. + 'even tables that are not relevant to your metadata model.' . PHP_EOL . 'If defined, --create, --update and --re-create can not be requested on same task.' ), new Option( - 'update', null, + 'update', null, 'Updates the schema in EntityManager (update tables on Database).' . PHP_EOL . - 'If defined, --create, --drop and --re-create can not be requested on same task.' + 'This command does a save update, which does not delete any tables, sequences or affected foreign keys.' . PHP_EOL . + 'If defined, --create, --drop and --complete-update --re-create can not be requested on same task.' + ), + new Option( + 'complete-update', null, + 'Updates the schema in EntityManager (update tables on Database).' . PHP_EOL . + 'Beware that all assets of the database which are not relevant to the current metadata are dropped by this command.'.PHP_EOL. + 'If defined, --create, --drop and --update --re-create can not be requested on same task.' ), new Option( 're-create', null, @@ -104,14 +112,20 @@ class SchemaToolTask extends AbstractTask $isCreate = isset($args['create']); $isDrop = isset($args['drop']); $isUpdate = isset($args['update']); + $isCompleteUpdate = isset($args['complete-update']); - if ($isUpdate && ($isCreate || $isDrop)) { - $printer->writeln("You can't use --update with --create or --drop", 'ERROR'); + if ($isUpdate && ($isCreate || $isDrop || $isCompleteUpdate)) { + $printer->writeln("You can't use --update with --create, --drop or --complete-update", 'ERROR'); return false; } - if ( ! ($isCreate || $isDrop || $isUpdate)) { - $printer->writeln('You must specify at a minimum one of the options (--create, --drop, --update, --re-create).', 'ERROR'); + if ($isCompleteUpdate && ($isCreate || $isDrop || $isUpdate)) { + $printer->writeln("You can't use --update with --create, --drop or --update", 'ERROR'); + return false; + } + + if ( ! ($isCreate || $isDrop || $isUpdate || $isCompleteUpdate)) { + $printer->writeln('You must specify at a minimum one of the options (--create, --drop, --update, --re-create, --complete-update).', 'ERROR'); return false; } @@ -140,6 +154,7 @@ class SchemaToolTask extends AbstractTask $isCreate = isset($args['create']); $isDrop = isset($args['drop']); $isUpdate = isset($args['update']); + $isCompleteUpdate = isset($args['complete-update']); $em = $this->getEntityManager(); $cmf = $em->getMetadataFactory(); @@ -162,20 +177,15 @@ class SchemaToolTask extends AbstractTask $tool = new SchemaTool($em); if ($isDrop) { - $dropMode = $args['drop']; - if(!in_array($dropMode, array('metadata', 'database'))) { - $dropMode = 'metadata'; - } - if (isset($args['dump-sql'])) { - foreach ($tool->getDropSchemaSql($classes, $dropMode) as $sql) { + foreach ($tool->getDropSchemaSql($classes) as $sql) { $printer->writeln($sql); } } else { $printer->writeln('Dropping database schema...', 'INFO'); try { - $tool->dropSchema($classes, $dropMode); + $tool->dropSchema($classes); $printer->writeln('Database schema dropped successfully.', 'INFO'); } catch (\Exception $ex) { throw new DoctrineException($ex); @@ -200,16 +210,17 @@ class SchemaToolTask extends AbstractTask } } - if ($isUpdate) { + if ($isUpdate || $isCompleteUpdate) { + $saveMode = $isUpdate?true:false; if (isset($args['dump-sql'])) { - foreach ($tool->getUpdateSchemaSql($classes) as $sql) { + foreach ($tool->getUpdateSchemaSql($classes, $saveMode) as $sql) { $printer->writeln($sql); } } else { $printer->writeln('Updating database schema...', 'INFO'); try { - $tool->updateSchema($classes); + $tool->updateSchema($classes, $saveMode); $printer->writeln('Database schema updated successfully.', 'INFO'); } catch (\Exception $ex) { throw new DoctrineException($ex); diff --git a/lib/Doctrine/ORM/Tools/SchemaTool.php b/lib/Doctrine/ORM/Tools/SchemaTool.php index c82ed633f..ccf1f6f25 100644 --- a/lib/Doctrine/ORM/Tools/SchemaTool.php +++ b/lib/Doctrine/ORM/Tools/SchemaTool.php @@ -517,9 +517,9 @@ class SchemaTool * @param array $classes * @return void */ - public function updateSchema(array $classes) + public function updateSchema(array $classes, $saveMode=false) { - $updateSchemaSql = $this->getUpdateSchemaSql($classes); + $updateSchemaSql = $this->getUpdateSchemaSql($classes, $saveMode); $conn = $this->_em->getConnection(); foreach ($updateSchemaSql as $sql) { @@ -534,7 +534,7 @@ class SchemaTool * @param array $classes The classes to consider. * @return array The sequence of SQL statements. */ - public function getUpdateSchemaSql(array $classes) + public function getUpdateSchemaSql(array $classes, $saveMode=false) { $sm = $this->_em->getConnection()->getSchemaManager(); @@ -544,7 +544,11 @@ class SchemaTool $comparator = new \Doctrine\DBAL\Schema\Comparator(); $schemaDiff = $comparator->compare($fromSchema, $toSchema); - return $schemaDiff->toSql($this->_platform); + if ($saveMode) { + return $schemaDiff->toSaveSql($this->_platform); + } else { + return $schemaDiff->toSql($this->_platform); + } } private function _getCommitOrder(array $classes) diff --git a/tests/Doctrine/Tests/DBAL/AllTests.php b/tests/Doctrine/Tests/DBAL/AllTests.php index a7de1bfd7..4976c235f 100644 --- a/tests/Doctrine/Tests/DBAL/AllTests.php +++ b/tests/Doctrine/Tests/DBAL/AllTests.php @@ -49,6 +49,7 @@ class AllTests $suite->addTestSuite('Doctrine\Tests\DBAL\Schema\SchemaTest'); $suite->addTestSuite('Doctrine\Tests\DBAL\Schema\Visitor\SchemaSqlCollectorTest'); $suite->addTestSuite('Doctrine\Tests\DBAL\Schema\ComparatorTest'); + $suite->addTestSuite('Doctrine\Tests\DBAL\Schema\SchemaDiffTest'); // Driver manager test $suite->addTestSuite('Doctrine\Tests\DBAL\DriverManagerTest'); diff --git a/tests/Doctrine/Tests/DBAL/Schema/SchemaDiffTest.php b/tests/Doctrine/Tests/DBAL/Schema/SchemaDiffTest.php new file mode 100644 index 000000000..ecffaf5d1 --- /dev/null +++ b/tests/Doctrine/Tests/DBAL/Schema/SchemaDiffTest.php @@ -0,0 +1,97 @@ +createSchemaDiff(); + $platform = $this->createPlatform(); + + $sql = $diff->toSql($platform); + + $expected = array('drop_orphan_fk', 'drop_seq', 'create_seq', 'drop_seq', 'create_seq', 'create_table', 'drop_table', 'alter_table'); + + $this->assertEquals($expected, $sql); + } + + public function testSchemaDiffToSaveSql() + { + $diff = $this->createSchemaDiff(); + $platform = $this->createPlatform(1, 0, 0); + + $sql = $diff->toSaveSql($platform); + + $expected = array('drop_seq', 'create_seq', 'create_seq', 'create_table', 'alter_table'); + + $this->assertEquals($expected, $sql); + } + + public function createPlatform($dropSequenceCount=2, $dropTableCount=1, $dropOrphanedFkCount=1) + { + $platform = $this->getMock('Doctrine\Tests\DBAL\Mocks\MockPlatform'); + $platform->expects($this->exactly($dropSequenceCount)) + ->method('getDropSequenceSql') + ->with($this->isInstanceOf('Doctrine\DBAL\Schema\Sequence')) + ->will($this->returnValue('drop_seq')); + $platform->expects($this->exactly(2)) + ->method('getCreateSequenceSql') + ->with($this->isInstanceOf('Doctrine\DBAL\Schema\Sequence')) + ->will($this->returnValue('create_seq')); + if ($dropTableCount > 0) { + $platform->expects($this->exactly($dropTableCount)) + ->method('getDropTableSql') + ->with($this->isInstanceof('Doctrine\DBAL\Schema\Table')) + ->will($this->returnValue('drop_table')); + } + $platform->expects($this->exactly(1)) + ->method('getCreateTableSql') + ->with($this->isInstanceof('Doctrine\DBAL\Schema\Table')) + ->will($this->returnValue(array('create_table'))); + $platform->expects($this->exactly(1)) + ->method('getAlterTableSql') + ->with($this->isInstanceOf('Doctrine\DBAL\Schema\TableDiff')) + ->will($this->returnValue(array('alter_table'))); + if ($dropOrphanedFkCount > 0) { + $platform->expects($this->exactly($dropOrphanedFkCount)) + ->method('getDropForeignKeySql') + ->with($this->isInstanceof('Doctrine\DBAL\Schema\ForeignKeyConstraint'), $this->equalTo('local_table')) + ->will($this->returnValue('drop_orphan_fk')); + } + $platform->expects($this->exactly(1)) + ->method('supportsSequences') + ->will($this->returnValue(true)); + $platform->expects($this->exactly(1)) + ->method('supportsForeignKeyConstraints') + ->will($this->returnValue(true)); + return $platform; + } + + public function createSchemaDiff() + { + $diff = new SchemaDiff(); + $diff->changedSequences['foo_seq'] = new Sequence('foo_seq'); + $diff->newSequences['bar_seq'] = new Sequence('bar_seq'); + $diff->removedSequences['baz_seq'] = new Sequence('baz_seq'); + $diff->newTables['foo_table'] = new Table('foo_table'); + $diff->removedTables['bar_table'] = new Table('bar_table'); + $diff->changedTables['baz_table'] = new TableDiff('baz_table'); + $fk = new \Doctrine\DBAL\Schema\ForeignKeyConstraint(array('id'), 'foreign_table', array('id')); + $fk->setLocalTable(new Table('local_table')); + $diff->orphanedForeignKeys[] = $fk; + return $diff; + } +} \ No newline at end of file diff --git a/tests/Doctrine/Tests/DBAL/Schema/SchemaTest.php b/tests/Doctrine/Tests/DBAL/Schema/SchemaTest.php index 05974a541..1890f6f2b 100644 --- a/tests/Doctrine/Tests/DBAL/Schema/SchemaTest.php +++ b/tests/Doctrine/Tests/DBAL/Schema/SchemaTest.php @@ -166,4 +166,24 @@ class SchemaTest extends \PHPUnit_Framework_TestCase $schema = new Schema(array(), array($sequence, $sequence)); } + + public function testFixSchema_AddExplicitIndexForForeignKey() + { + $schema = new Schema(); + $tableA = $schema->createTable('foo'); + $tableA->createColumn('id', 'integer'); + + $tableB = $schema->createTable('bar'); + $tableB->createColumn('id', 'integer'); + $tableB->createcolumn('foo_id', 'integer'); + $tableB->addForeignKeyConstraint($tableA, array('foo_id'), array('id')); + + $this->assertEquals(0, count($tableB->getIndexes())); + + $schema->visit(new \Doctrine\DBAL\Schema\Visitor\FixSchema(true)); + + $this->assertEquals(1, count($tableB->getIndexes())); + $index = current($tableB->getIndexes()); + $this->assertTrue($index->hasColumnAtPosition('foo_id', 0)); + } } \ No newline at end of file