[2.0] DDC-169 - Some final cleanups for the DBAL refactoring, changed SchemaTool CLI API slightly.

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

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);
     }
 
     /**

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);
             }

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(

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', '<metadata|database>', 
+                '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 ($isCompleteUpdate && ($isCreate || $isDrop || $isUpdate)) {
+            $printer->writeln("You can't use --update with --create, --drop or --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 ( ! ($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);

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)

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');

tests/Doctrine/Tests/DBAL/Schema/SchemaDiffTest.php

+<?php
+
+namespace Doctrine\Tests\DBAL\Schema;
+
+require_once __DIR__ . '/../../TestInit.php';
+
+use Doctrine\DBAL\Schema\Schema,
+    Doctrine\DBAL\Schema\Table,
+    Doctrine\DBAL\Schema\Column,
+    Doctrine\DBAL\Schema\Index,
+    Doctrine\DBAL\Schema\Sequence,
+    Doctrine\DBAL\Schema\SchemaDiff,
+    Doctrine\DBAL\Schema\TableDiff,
+    Doctrine\DBAL\Schema\Comparator,
+    Doctrine\DBAL\Types\Type;
+
+class SchemaDiffTest extends \PHPUnit_Framework_TestCase
+{
+    public function testSchemaDiffToSql()
+    {
+        $diff = $this->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

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