From ae76b2ab8dbeade33fdbbcf6824f3b70802d9447 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Tue, 16 Nov 2010 21:31:54 +0100 Subject: [PATCH] DDC-853, DDC-629 - Fix drop schema always dropping everything at the cost of potential failures when dropping due to foreign keys. Added a full-database drop mode that resembles the old behavior. --- .../Command/SchemaTool/DropCommand.php | 24 +++++- lib/Doctrine/ORM/Tools/SchemaTool.php | 76 +++++++++++-------- .../ORM/Functional/SchemaTool/DDC214Test.php | 21 ++--- .../ORM/Functional/Ticket/DDC735Test.php | 3 +- .../ORM/Functional/Ticket/DDC809Test.php | 18 ++--- .../ORM/Functional/Ticket/DDC832Test.php | 5 ++ 6 files changed, 95 insertions(+), 52 deletions(-) diff --git a/lib/Doctrine/ORM/Tools/Console/Command/SchemaTool/DropCommand.php b/lib/Doctrine/ORM/Tools/Console/Command/SchemaTool/DropCommand.php index df6167d64..f72cca1fe 100644 --- a/lib/Doctrine/ORM/Tools/Console/Command/SchemaTool/DropCommand.php +++ b/lib/Doctrine/ORM/Tools/Console/Command/SchemaTool/DropCommand.php @@ -60,6 +60,10 @@ class DropCommand extends AbstractCommand 'force', null, InputOption::PARAMETER_NONE, "Don't ask for the deletion of the database, but force the operation to run." ), + new InputOption( + 'full-database', null, InputOption::PARAMETER_NONE, + 'Instead of using the Class Metadata to detect the database table schema, drop ALL assets that the database contains.' + ), )) ->setHelp(<<getOption('full-database')); + if ($input->getOption('dump-sql') === true) { - $sqls = $schemaTool->getDropSchemaSql($metadatas); + if ($isFullDatabaseDrop) { + $sqls = $schemaTool->getDropDatabaseSQL(); + } else { + $sqls = $schemaTool->getDropSchemaSQL($metadatas); + } $output->write(implode(';' . PHP_EOL, $sqls) . PHP_EOL); } else if ($input->getOption('force') === true) { $output->write('Dropping database schema...' . PHP_EOL); - $schemaTool->dropSchema($metadatas); + if ($isFullDatabaseDrop) { + $schemaTool->dropDatabase(); + } else { + $schemaTool->dropSchema($metadatas); + } $output->write('Database schema dropped successfully!' . PHP_EOL); } else { $output->write('ATTENTION: This operation should not be executed in an production enviroment.' . PHP_EOL . PHP_EOL); - $sqls = $schemaTool->getDropSchemaSql($metadatas); + if ($isFullDatabaseDrop) { + $sqls = $schemaTool->getDropDatabaseSQL(); + } else { + $sqls = $schemaTool->getDropSchemaSQL($metadatas); + } if (count($sqls)) { $output->write('Schema-Tool would execute ' . count($sqls) . ' queries to drop the database.' . PHP_EOL); diff --git a/lib/Doctrine/ORM/Tools/SchemaTool.php b/lib/Doctrine/ORM/Tools/SchemaTool.php index cee2fe294..0da4be1f3 100644 --- a/lib/Doctrine/ORM/Tools/SchemaTool.php +++ b/lib/Doctrine/ORM/Tools/SchemaTool.php @@ -509,12 +509,26 @@ class SchemaTool } /** - * Gets the SQL needed to drop the database schema for the given classes. + * Drops all elements in the database of the current connection. + * + * @return void + */ + public function dropDatabase() + { + $dropSchemaSql = $this->getDropDatabaseSQL(); + $conn = $this->_em->getConnection(); + + foreach ($dropSchemaSql as $sql) { + $conn->executeQuery($sql); + } + } + + /** + * Gets the SQL needed to drop the database schema for the connections database. * - * @param array $classes * @return array */ - public function getDropSchemaSql(array $classes) + public function getDropDatabaseSQL() { $sm = $this->_em->getConnection()->getSchemaManager(); $schema = $sm->createSchema(); @@ -526,39 +540,31 @@ class SchemaTool } /** - * Drop all tables of the database connection. * + * @param array $classes * @return array */ - private function _getDropSchemaTablesDatabaseMode($classes) + public function getDropSchemaSQL(array $classes) { - $conn = $this->_em->getConnection(); + $sm = $this->_em->getConnection()->getSchemaManager(); + + $sql = array(); + $orderedTables = array(); - $sm = $conn->getSchemaManager(); - /* @var $sm \Doctrine\DBAL\Schema\AbstractSchemaManager */ - - $allTables = $sm->listTables(); - - $orderedTables = $this->_getDropSchemaTablesMetadataMode($classes); - foreach($allTables AS $tableName) { - if(!in_array($tableName, $orderedTables)) { - $orderedTables[] = $tableName; + foreach ($classes AS $class) { + if ($class->isIdGeneratorSequence() && $class->name == $class->rootEntityName && $this->_platform->supportsSequences()) { + $sql[] = $this->_platform->getDropSequenceSQL($class->sequenceGeneratorDefinition['sequenceName']); } } - return $orderedTables; - } - - private function _getDropSchemaTablesMetadataMode(array $classes) - { - $orderedTables = array(); - $commitOrder = $this->_getCommitOrder($classes); $associationTables = $this->_getAssociationTables($commitOrder); // Drop association tables first foreach ($associationTables as $associationTable) { - $orderedTables[] = $associationTable; + if (!in_array($associationTable, $orderedTables)) { + $orderedTables[] = $associationTable; + } } // Drop tables in reverse commit order @@ -570,17 +576,27 @@ class SchemaTool continue; } - $orderedTables[] = $class->getTableName(); + if (!in_array($class->getTableName(), $orderedTables)) { + $orderedTables[] = $class->getTableName(); + } } - //TODO: Drop other schema elements, like sequences etc. + $dropTablesSql = array(); + foreach ($orderedTables AS $tableName) { + /* @var $sm \Doctrine\DBAL\Schema\AbstractSchemaManager */ + $foreignKeys = $sm->listTableForeignKeys($tableName); + foreach ($foreignKeys AS $foreignKey) { + $sql[] = $this->_platform->getDropForeignKeySQL($foreignKey, $tableName); + } + $dropTablesSql[] = $this->_platform->getDropTableSQL($tableName); + } - return $orderedTables; + return array_merge($sql, $dropTablesSql); } /** * Updates the database schema of the given classes by comparing the ClassMetadata - * instances to the current database schema that is inspected. + * ins$tableNametances to the current database schema that is inspected. * * @param array $classes * @return void @@ -628,7 +644,7 @@ class SchemaTool $calc->addClass($class); foreach ($class->associationMappings as $assoc) { - if ($assoc->isOwningSide) { + if ($assoc['isOwningSide']) { $targetClass = $this->_em->getClassMetadata($assoc['targetEntity']); if ( ! $calc->hasClass($targetClass->name)) { @@ -650,8 +666,8 @@ class SchemaTool foreach ($classes as $class) { foreach ($class->associationMappings as $assoc) { - if ($assoc->isOwningSide && $assoc['type'] == ClassMetadata::MANY_TO_MANY) { - $associationTables[] = $assoc->joinTable['name']; + if ($assoc['isOwningSide'] && $assoc['type'] == ClassMetadata::MANY_TO_MANY) { + $associationTables[] = $assoc['joinTable']['name']; } } } diff --git a/tests/Doctrine/Tests/ORM/Functional/SchemaTool/DDC214Test.php b/tests/Doctrine/Tests/ORM/Functional/SchemaTool/DDC214Test.php index a6682b6a8..e4e5e69a0 100644 --- a/tests/Doctrine/Tests/ORM/Functional/SchemaTool/DDC214Test.php +++ b/tests/Doctrine/Tests/ORM/Functional/SchemaTool/DDC214Test.php @@ -12,6 +12,9 @@ require_once __DIR__ . '/../../../TestInit.php'; */ class DDC214Test extends \Doctrine\Tests\OrmFunctionalTestCase { + private $classes = array(); + private $schemaTool = null; + public function setUp() { parent::setUp(); @@ -20,6 +23,7 @@ class DDC214Test extends \Doctrine\Tests\OrmFunctionalTestCase if (strpos($conn->getDriver()->getName(), "sqlite") !== false) { $this->markTestSkipped('SQLite does not support ALTER TABLE statements.'); } + $this->schemaTool = new Tools\SchemaTool($this->_em); } /** @@ -27,7 +31,7 @@ class DDC214Test extends \Doctrine\Tests\OrmFunctionalTestCase */ public function testCmsAddressModel() { - $classes = array( + $this->classes = array( 'Doctrine\Tests\Models\CMS\CmsUser', 'Doctrine\Tests\Models\CMS\CmsPhonenumber', 'Doctrine\Tests\Models\CMS\CmsAddress', @@ -35,7 +39,7 @@ class DDC214Test extends \Doctrine\Tests\OrmFunctionalTestCase 'Doctrine\Tests\Models\CMS\CmsArticle' ); - $this->assertCreatedSchemaNeedsNoUpdates($classes); + $this->assertCreatedSchemaNeedsNoUpdates($this->classes); } /** @@ -43,7 +47,7 @@ class DDC214Test extends \Doctrine\Tests\OrmFunctionalTestCase */ public function testCompanyModel() { - $classes = array( + $this->classes = array( 'Doctrine\Tests\Models\Company\CompanyPerson', 'Doctrine\Tests\Models\Company\CompanyEmployee', 'Doctrine\Tests\Models\Company\CompanyManager', @@ -54,7 +58,7 @@ class DDC214Test extends \Doctrine\Tests\OrmFunctionalTestCase 'Doctrine\Tests\Models\Company\CompanyCar' ); - $this->assertCreatedSchemaNeedsNoUpdates($classes); + $this->assertCreatedSchemaNeedsNoUpdates($this->classes); } public function assertCreatedSchemaNeedsNoUpdates($classes) @@ -64,19 +68,18 @@ class DDC214Test extends \Doctrine\Tests\OrmFunctionalTestCase $classMetadata[] = $this->_em->getClassMetadata($class); } - $schemaTool = new Tools\SchemaTool($this->_em); - $schemaTool->dropSchema($classMetadata); - $schemaTool->createSchema($classMetadata); + $this->schemaTool->dropDatabase(); + $this->schemaTool->createSchema($classMetadata); $sm = $this->_em->getConnection()->getSchemaManager(); $fromSchema = $sm->createSchema(); - $toSchema = $schemaTool->getSchemaFromMetadata($classMetadata); + $toSchema = $this->schemaTool->getSchemaFromMetadata($classMetadata); $comparator = new \Doctrine\DBAL\Schema\Comparator(); $schemaDiff = $comparator->compare($fromSchema, $toSchema); $sql = $schemaDiff->toSql($this->_em->getConnection()->getDatabasePlatform()); - $this->assertEquals(0, count($sql)); + $this->assertEquals(0, count($sql), "SQL: " . implode(PHP_EOL, $sql)); } } \ No newline at end of file diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC735Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC735Test.php index 76d3720c9..77ef6e148 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC735Test.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC735Test.php @@ -35,6 +35,7 @@ class DDC735Test extends \Doctrine\Tests\OrmFunctionalTestCase $this->assertEquals(1, count($product->getReviews())); // Remove the review + $reviewId = $review->getId(); $product->removeReview($review); $this->_em->flush(); @@ -48,7 +49,7 @@ class DDC735Test extends \Doctrine\Tests\OrmFunctionalTestCase $this->assertEquals(0, count($product->getReviews()), 'count($reviews) should still be 0 after the refresh'); // Review should also not be available anymore - $this->assertNull($this->_em->find(__NAMESPACE__.'\DDC735Review', $review->getId())); + $this->assertNull($this->_em->find(__NAMESPACE__.'\DDC735Review', $reviewId)); } } diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC809Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC809Test.php index c0dcba90e..8ab65d8c6 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC809Test.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC809Test.php @@ -29,15 +29,15 @@ class DDC809Test extends \Doctrine\Tests\OrmFunctionalTestCase $conn->insert('variant_test', array('variant_id' => 545208)); $conn->insert('variant_test', array('variant_id' => 545209)); - $conn->insert('variant_specification_value_test', array('variant_id' => 545208, 'specification_value_id' => 94606)); - $conn->insert('variant_specification_value_test', array('variant_id' => 545208, 'specification_value_id' => 94607)); - $conn->insert('variant_specification_value_test', array('variant_id' => 545208, 'specification_value_id' => 94609)); - $conn->insert('variant_specification_value_test', array('variant_id' => 545208, 'specification_value_id' => 94711)); + $conn->insert('var_spec_value_test', array('variant_id' => 545208, 'specification_value_id' => 94606)); + $conn->insert('var_spec_value_test', array('variant_id' => 545208, 'specification_value_id' => 94607)); + $conn->insert('var_spec_value_test', array('variant_id' => 545208, 'specification_value_id' => 94609)); + $conn->insert('var_spec_value_test', array('variant_id' => 545208, 'specification_value_id' => 94711)); - $conn->insert('variant_specification_value_test', array('variant_id' => 545209, 'specification_value_id' => 94589)); - $conn->insert('variant_specification_value_test', array('variant_id' => 545209, 'specification_value_id' => 94593)); - $conn->insert('variant_specification_value_test', array('variant_id' => 545209, 'specification_value_id' => 94606)); - $conn->insert('variant_specification_value_test', array('variant_id' => 545209, 'specification_value_id' => 94607)); + $conn->insert('var_spec_value_test', array('variant_id' => 545209, 'specification_value_id' => 94589)); + $conn->insert('var_spec_value_test', array('variant_id' => 545209, 'specification_value_id' => 94593)); + $conn->insert('var_spec_value_test', array('variant_id' => 545209, 'specification_value_id' => 94606)); + $conn->insert('var_spec_value_test', array('variant_id' => 545209, 'specification_value_id' => 94607)); } /** @@ -72,7 +72,7 @@ class DDC809Variant /** * @ManyToMany(targetEntity="DDC809SpecificationValue", inversedBy="Variants") - * @JoinTable(name="variant_specification_value_test", + * @JoinTable(name="var_spec_value_test", * joinColumns={ * @JoinColumn(name="variant_id", referencedColumnName="variant_id") * }, diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC832Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC832Test.php index c598a8194..5e112ffb7 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC832Test.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC832Test.php @@ -13,6 +13,11 @@ class DDC832Test extends \Doctrine\Tests\OrmFunctionalTestCase public function setUp() { parent::setUp(); + $platform = $this->_em->getConnection()->getDatabasePlatform(); + if ($platform->getName() == "oracle") { + $this->markTestSkipped('Doesnt run on Oracle.'); + } + try { $this->_schemaTool->createSchema(array( $this->_em->getClassMetadata(__NAMESPACE__ . '\DDC832JoinedIndex'),