From 81a0dbddd993a2d160bbe74b9e923c01d3299e14 Mon Sep 17 00:00:00 2001 From: Farzad Ghanei Date: Fri, 16 Oct 2015 10:06:12 +0800 Subject: [PATCH 1/4] Allow calls to addSql in postUp and postDown methods Both preUp/preDown and up/down methods could call addSql() to register a SQL command to be executed as part of the migration, this not available for postUp/postDown yet. By extracting the logic of executing the SQL queue (populated by schema change or addSql) to a private method (executeRegisteredSql) which could be called multiple times, and call this method after postUp/postDown hooks. Right now calling addSql() in postUp/postDown has no effect. This could be helpful where the migration changes the schema (using DDL), but there is a need for DML as well which relies on the new schema. So the DML commands can be added to postUp/postDown using addSql() method. --- lib/Doctrine/DBAL/Migrations/Version.php | 73 ++++++++++++++---------- 1 file changed, 44 insertions(+), 29 deletions(-) diff --git a/lib/Doctrine/DBAL/Migrations/Version.php b/lib/Doctrine/DBAL/Migrations/Version.php index 45e086dc4a..2e0e726231 100644 --- a/lib/Doctrine/DBAL/Migrations/Version.php +++ b/lib/Doctrine/DBAL/Migrations/Version.php @@ -264,43 +264,20 @@ public function execute($direction, $dryRun = false, $timeAllQueries = false) $this->migration->$direction($toSchema); $this->addSql($fromSchema->getMigrateToSql($toSchema, $this->platform)); - if (! $dryRun) { - if (!empty($this->sql)) { - foreach ($this->sql as $key => $query) { - $queryStart = microtime(true); - - if ( ! isset($this->params[$key])) { - $this->outputWriter->write(' -> ' . $query); - $this->connection->exec($query); - } else { - $this->outputWriter->write(sprintf(' - %s (with parameters)', $query)); - $this->connection->executeQuery($query, $this->params[$key], $this->types[$key]); - } - - $this->outputQueryTime($queryStart, $timeAllQueries); - } - } else { - $this->outputWriter->write(sprintf( - 'Migration %s was executed but did not result in any SQL statements.', - $this->version - )); - } + $this->executeRegisteredSql(); + $this->state = self::STATE_POST; + $this->migration->{'post' . ucfirst($direction)}($toSchema); + $this->executeRegisteredSql(); + + if (! $dryRun) { if ($direction === self::DIRECTION_UP) { $this->markMigrated(); } else { $this->markNotMigrated(); } - - } else { - foreach ($this->sql as $query) { - $this->outputWriter->write(' -> ' . $query); - } } - $this->state = self::STATE_POST; - $this->migration->{'post' . ucfirst($direction)}($toSchema); - $migrationEnd = microtime(true); $this->time = round($migrationEnd - $migrationStart, 2); if ($direction === self::DIRECTION_UP) { @@ -392,4 +369,42 @@ public function __toString() { return $this->version; } + + private function executeRegisteredSql($dryRun = false, $timeAllQueries = false) + { + if (! $dryRun) { + if (!empty($this->sql)) { + foreach ($this->sql as $key => $query) { + $queryStart = microtime(true); + + if ( ! isset($this->params[$key])) { + $this->outputWriter->write(' -> ' . $query); + $this->connection->exec($query); + } else { + $this->outputWriter->write(sprintf(' - %s (with parameters)', $query)); + $this->connection->executeQuery($query, $this->params[$key], $this->types[$key]); + } + + $this->outputQueryTime($queryStart, $timeAllQueries); + } + } else { + $this->outputWriter->write(sprintf( + 'Migration %s was executed but did not result in any SQL statements.', + $this->version + )); + } + } else { + foreach ($this->sql as $query) { + $this->outputWriter->write(' -> ' . $query); + } + } + $this->resetRegisteredSql(); + } + + private function resetRegisteredSql() + { + $this->sql = []; + $this->params = []; + $this->types = []; + } } From ac698ec409996b0fd4ed5870f6707c86e3e1ec4c Mon Sep 17 00:00:00 2001 From: Farzad Ghanei Date: Fri, 16 Oct 2015 11:03:05 +0800 Subject: [PATCH 2/4] Pass dryRun and timeAllQueries parameters to extracted method After extracting the logic of executing the SQL queue to a separte private method, forgot to pass the options of dry run and time all queries to the method. --- lib/Doctrine/DBAL/Migrations/Version.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Doctrine/DBAL/Migrations/Version.php b/lib/Doctrine/DBAL/Migrations/Version.php index 2e0e726231..24335650fc 100644 --- a/lib/Doctrine/DBAL/Migrations/Version.php +++ b/lib/Doctrine/DBAL/Migrations/Version.php @@ -264,11 +264,11 @@ public function execute($direction, $dryRun = false, $timeAllQueries = false) $this->migration->$direction($toSchema); $this->addSql($fromSchema->getMigrateToSql($toSchema, $this->platform)); - $this->executeRegisteredSql(); + $this->executeRegisteredSql($dryRun, $timeAllQueries); $this->state = self::STATE_POST; $this->migration->{'post' . ucfirst($direction)}($toSchema); - $this->executeRegisteredSql(); + $this->executeRegisteredSql($dryRun, $timeAllQueries); if (! $dryRun) { if ($direction === self::DIRECTION_UP) { From dbd47fdd6fbbf6c741a93d7c8260afc0d1055745 Mon Sep 17 00:00:00 2001 From: Farzad Ghanei Date: Fri, 16 Oct 2015 17:53:35 +0800 Subject: [PATCH 3/4] Add a functional test to use addSql() calls in postUp --- .../Tests/Functional/FunctionalTest.php | 23 ++++++++++++++ .../Functional/MigrateAddSqlPostUpTest.php | 31 +++++++++++++++++++ 2 files changed, 54 insertions(+) create mode 100644 tests/Doctrine/DBAL/Migrations/Tests/Stub/Functional/MigrateAddSqlPostUpTest.php diff --git a/tests/Doctrine/DBAL/Migrations/Tests/Functional/FunctionalTest.php b/tests/Doctrine/DBAL/Migrations/Tests/Functional/FunctionalTest.php index 658019eb11..fcd37f15c1 100644 --- a/tests/Doctrine/DBAL/Migrations/Tests/Functional/FunctionalTest.php +++ b/tests/Doctrine/DBAL/Migrations/Tests/Functional/FunctionalTest.php @@ -178,6 +178,29 @@ public function testAddSql() $this->assertFalse($schema->hasTable('test_add_sql_table')); } + public function testAddSqlInPostUp() + { + $this->config->registerMigration(1, 'Doctrine\DBAL\Migrations\Tests\Stub\Functional\MigrateAddSqlPostUpTest'); + $tableName = \Doctrine\DBAL\Migrations\Tests\Stub\Functional\MigrateAddSqlPostUpTest::TABLE_NAME; + + $migration = new \Doctrine\DBAL\Migrations\Migration($this->config); + $migration->migrate(1); + + $migrations = $this->config->getMigrations(); + $this->assertTrue($migrations[1]->isMigrated()); + + $schema = $this->config->getConnection()->getSchemaManager()->createSchema(); + $this->assertTrue($schema->hasTable($tableName)); + $check = $this->config->getConnection()->fetchAll("select * from $tableName"); + $this->assertNotEmpty($check); + $this->assertEquals('test', $check[0]['test']); + + $migration->migrate(0); + $this->assertFalse($migrations[1]->isMigrated()); + $schema = $this->config->getConnection()->getSchemaManager()->createSchema(); + $this->assertFalse($schema->hasTable($tableName)); + } + public function testVersionInDatabaseWithoutRegisteredMigrationStillMigrates() { $this->config->registerMigration(1, 'Doctrine\DBAL\Migrations\Tests\Stub\Functional\MigrateAddSqlTest'); diff --git a/tests/Doctrine/DBAL/Migrations/Tests/Stub/Functional/MigrateAddSqlPostUpTest.php b/tests/Doctrine/DBAL/Migrations/Tests/Stub/Functional/MigrateAddSqlPostUpTest.php new file mode 100644 index 0000000000..9a22d0e71d --- /dev/null +++ b/tests/Doctrine/DBAL/Migrations/Tests/Stub/Functional/MigrateAddSqlPostUpTest.php @@ -0,0 +1,31 @@ +createTable(self::TABLE_NAME); + $table->addColumn('test', Type::STRING, ['length' => 64]); + } + + public function postUp(Schema $schema) + { + $this->addSql( + sprintf("INSERT INTO %s (test) values (?)", self::TABLE_NAME), + ['test'] + ); + } + + public function down(Schema $schema) + { + $schema->dropTable(self::TABLE_NAME); + } +} From d393217244b059be8be98250c6f65d2044434996 Mon Sep 17 00:00:00 2001 From: mike Date: Sun, 18 Oct 2015 18:33:07 +0200 Subject: [PATCH 4/4] Testing All methods for addSql --- .../Tests/Functional/FunctionalTest.php | 21 ++++--- .../MigrateAddSqlPostAndPreUpAndDownTest.php | 60 +++++++++++++++++++ .../Functional/MigrateAddSqlPostUpTest.php | 31 ---------- 3 files changed, 73 insertions(+), 39 deletions(-) create mode 100644 tests/Doctrine/DBAL/Migrations/Tests/Stub/Functional/MigrateAddSqlPostAndPreUpAndDownTest.php delete mode 100644 tests/Doctrine/DBAL/Migrations/Tests/Stub/Functional/MigrateAddSqlPostUpTest.php diff --git a/tests/Doctrine/DBAL/Migrations/Tests/Functional/FunctionalTest.php b/tests/Doctrine/DBAL/Migrations/Tests/Functional/FunctionalTest.php index fcd37f15c1..a2cf866ac6 100644 --- a/tests/Doctrine/DBAL/Migrations/Tests/Functional/FunctionalTest.php +++ b/tests/Doctrine/DBAL/Migrations/Tests/Functional/FunctionalTest.php @@ -180,8 +180,10 @@ public function testAddSql() public function testAddSqlInPostUp() { - $this->config->registerMigration(1, 'Doctrine\DBAL\Migrations\Tests\Stub\Functional\MigrateAddSqlPostUpTest'); - $tableName = \Doctrine\DBAL\Migrations\Tests\Stub\Functional\MigrateAddSqlPostUpTest::TABLE_NAME; + $this->config->registerMigration(1, 'Doctrine\DBAL\Migrations\Tests\Stub\Functional\MigrateAddSqlPostAndPreUpAndDownTest'); + $tableName = \Doctrine\DBAL\Migrations\Tests\Stub\Functional\MigrateAddSqlPostAndPreUpAndDownTest::TABLE_NAME; + + $this->config->getConnection()->executeQuery(sprintf("CREATE TABLE IF NOT EXISTS %s (test INT)", $tableName)); $migration = new \Doctrine\DBAL\Migrations\Migration($this->config); $migration->migrate(1); @@ -189,16 +191,19 @@ public function testAddSqlInPostUp() $migrations = $this->config->getMigrations(); $this->assertTrue($migrations[1]->isMigrated()); - $schema = $this->config->getConnection()->getSchemaManager()->createSchema(); - $this->assertTrue($schema->hasTable($tableName)); - $check = $this->config->getConnection()->fetchAll("select * from $tableName"); + $check = $this->config->getConnection()->fetchColumn("select SUM(test) as sum from $tableName"); + $this->assertNotEmpty($check); - $this->assertEquals('test', $check[0]['test']); + $this->assertEquals(6, $check); $migration->migrate(0); $this->assertFalse($migrations[1]->isMigrated()); - $schema = $this->config->getConnection()->getSchemaManager()->createSchema(); - $this->assertFalse($schema->hasTable($tableName)); + $check = $this->config->getConnection()->fetchColumn("select SUM(test) as sum from $tableName"); + $this->assertNotEmpty($check); + $this->assertEquals(21, $check); + + + $this->config->getConnection()->executeQuery(sprintf("DROP TABLE %s ", $tableName)); } public function testVersionInDatabaseWithoutRegisteredMigrationStillMigrates() diff --git a/tests/Doctrine/DBAL/Migrations/Tests/Stub/Functional/MigrateAddSqlPostAndPreUpAndDownTest.php b/tests/Doctrine/DBAL/Migrations/Tests/Stub/Functional/MigrateAddSqlPostAndPreUpAndDownTest.php new file mode 100644 index 0000000000..1aa756f81e --- /dev/null +++ b/tests/Doctrine/DBAL/Migrations/Tests/Stub/Functional/MigrateAddSqlPostAndPreUpAndDownTest.php @@ -0,0 +1,60 @@ +addSql( + sprintf("INSERT INTO %s (test) values (?)", self::TABLE_NAME), + [1] + ); + } + + public function up(Schema $schema) + { + $this->addSql( + sprintf("INSERT INTO %s (test) values (?)", self::TABLE_NAME), + [2] + ); + } + + public function postUp(Schema $schema) + { + $this->addSql( + sprintf("INSERT INTO %s (test) values (?)", self::TABLE_NAME), + [3] + ); + } + + public function preDown(Schema $schema) + { + $this->addSql( + sprintf("INSERT INTO %s (test) values (?)", self::TABLE_NAME), + [4] + ); + } + + public function down(Schema $schema) + { + $this->addSql( + sprintf("INSERT INTO %s (test) values (?)", self::TABLE_NAME), + [5] + ); + } + + public function postDown(Schema $schema) + { + $this->addSql( + sprintf("INSERT INTO %s (test) values (?)", self::TABLE_NAME), + [6] + ); + } +} diff --git a/tests/Doctrine/DBAL/Migrations/Tests/Stub/Functional/MigrateAddSqlPostUpTest.php b/tests/Doctrine/DBAL/Migrations/Tests/Stub/Functional/MigrateAddSqlPostUpTest.php deleted file mode 100644 index 9a22d0e71d..0000000000 --- a/tests/Doctrine/DBAL/Migrations/Tests/Stub/Functional/MigrateAddSqlPostUpTest.php +++ /dev/null @@ -1,31 +0,0 @@ -createTable(self::TABLE_NAME); - $table->addColumn('test', Type::STRING, ['length' => 64]); - } - - public function postUp(Schema $schema) - { - $this->addSql( - sprintf("INSERT INTO %s (test) values (?)", self::TABLE_NAME), - ['test'] - ); - } - - public function down(Schema $schema) - { - $schema->dropTable(self::TABLE_NAME); - } -}