Merge pull request #809 from doctrine/fix-dry-run-write-sql

Fix for referencing tables created during a dry run

lib/Doctrine/Migrations/Migrator.php

@@ -179,6 +179,13 @@ private function executeMigration(
             foreach ($migrationsToExecute as $version) {
                 $versionExecutionResult = $version->execute($direction, $migratorConfiguration);
 
+                // capture the to Schema for the migration so we have the ability to use
+                // it as the from Schema for the next migration when we are running a dry run
+                // $toSchema may be null in the case of skipped migrations
+                if (! $versionExecutionResult->isSkipped()) {
+                    $migratorConfiguration->setFromSchema($versionExecutionResult->getToSchema());
+                }
+
                 $sql[$version->getVersion()] = $versionExecutionResult->getSql();
                 $time                       += $versionExecutionResult->getTime();
             }

lib/Doctrine/Migrations/MigratorConfiguration.php

@@ -4,6 +4,8 @@
 
 namespace Doctrine\Migrations;
 
+use Doctrine\DBAL\Schema\Schema;
+
 /**
  * The MigratorConfiguration class is responsible for defining the configuration for a migration.
  *
@@ -26,6 +28,9 @@ class MigratorConfiguration
     /** @var bool */
     private $allOrNothing = false;
 
+    /** @var Schema|null */
+    private $fromSchema;
+
     public function isDryRun() : bool
     {
         return $this->dryRun;
@@ -73,4 +78,16 @@ public function setAllOrNothing(bool $allOrNothing) : self
 
         return $this;
     }
+
+    public function getFromSchema() : ?Schema
+    {
+        return $this->fromSchema;
+    }
+
+    public function setFromSchema(Schema $fromSchema) : self
+    {
+        $this->fromSchema = $fromSchema;
+
+        return $this;
+    }
 }

lib/Doctrine/Migrations/Version/ExecutionResult.php

@@ -4,6 +4,8 @@
 
 namespace Doctrine\Migrations\Version;
 
+use Doctrine\DBAL\Schema\Schema;
+use RuntimeException;
 use Throwable;
 use function count;
 
@@ -38,6 +40,9 @@ class ExecutionResult
     /** @var Throwable|null */
     private $exception;
 
+    /** @var Schema|null */
+    private $toSchema;
+
     /**
      * @param string[] $sql
      * @param mixed[]  $params
@@ -152,4 +157,18 @@ public function getException() : ?Throwable
     {
         return $this->exception;
     }
+
+    public function setToSchema(Schema $toSchema) : void
+    {
+        $this->toSchema = $toSchema;
+    }
+
+    public function getToSchema() : Schema
+    {
+        if ($this->toSchema === null) {
+            throw new RuntimeException('Cannot call getToSchema() when toSchema is null.');
+        }
+
+        return $this->toSchema;
+    }
 }

lib/Doctrine/Migrations/Version/Executor.php

@@ -5,6 +5,7 @@
 namespace Doctrine\Migrations\Version;
 
 use Doctrine\DBAL\Connection;
+use Doctrine\DBAL\Schema\Schema;
 use Doctrine\Migrations\AbstractMigration;
 use Doctrine\Migrations\Configuration\Configuration;
 use Doctrine\Migrations\Events;
@@ -192,7 +193,7 @@ private function executeMigration(
 
         $version->setState(State::PRE);
 
-        $fromSchema = $this->schemaProvider->createFromSchema();
+        $fromSchema = $this->getFromSchema($migratorConfiguration);
 
         $migration->{'pre' . ucfirst($direction)}($fromSchema);
 
@@ -206,6 +207,8 @@ private function executeMigration(
 
         $toSchema = $this->schemaProvider->createToSchema($fromSchema);
 
+        $versionExecutionResult->setToSchema($toSchema);
+
         $migration->$direction($toSchema);
 
         foreach ($this->schemaProvider->getSqlDiffToMigrate($fromSchema, $toSchema) as $sql) {
@@ -368,4 +371,14 @@ private function outputSqlQuery(int $idx, string $query) : void
             $params
         )));
     }
+
+    private function getFromSchema(MigratorConfiguration $migratorConfiguration) : Schema
+    {
+        // if we're in a dry run, use the from Schema instead of reading the schema from the database
+        if ($migratorConfiguration->isDryRun() && $migratorConfiguration->getFromSchema() !== null) {
+            return $migratorConfiguration->getFromSchema();
+        }
+
+        return $this->schemaProvider->createFromSchema();
+    }
 }

tests/Doctrine/Migrations/Tests/Functional/FunctionalTest.php

@@ -19,6 +19,8 @@
 use Doctrine\Migrations\Stopwatch;
 use Doctrine\Migrations\Tests\MigrationTestCase;
 use Doctrine\Migrations\Tests\Stub\EventVerificationListener;
+use Doctrine\Migrations\Tests\Stub\Functional\DryRun\DryRun1;
+use Doctrine\Migrations\Tests\Stub\Functional\DryRun\DryRun2;
 use Doctrine\Migrations\Tests\Stub\Functional\MigrateAddSqlPostAndPreUpAndDownTest;
 use Doctrine\Migrations\Tests\Stub\Functional\MigrateAddSqlTest;
 use Doctrine\Migrations\Tests\Stub\Functional\MigrateNotTouchingTheSchema;
@@ -187,6 +189,26 @@ public function testDryRunMigration() : void
         self::assertFalse($migrations['3']->isMigrated());
     }
 
+    public function testDryRunWithTableCreatedWithSchemaInFirstMigration() : void
+    {
+        $this->config->registerMigration('1', DryRun1::class);
+        $this->config->registerMigration('2', DryRun2::class);
+
+        $migratorConfiguration = (new MigratorConfiguration())
+            ->setDryRun(true);
+
+        $migrator = $this->createTestMigrator($this->config);
+        $migrator->migrate('2', $migratorConfiguration);
+
+        $schema = $migratorConfiguration->getFromSchema();
+
+        self::assertInstanceOf(Schema::class, $schema);
+        self::assertTrue($schema->hasTable('foo'));
+
+        $table = $schema->getTable('foo');
+        self::assertTrue($table->hasColumn('bar'));
+    }
+
     public function testMigrateDownSeveralSteps() : void
     {
         $this->config->registerMigration('1', MigrationMigrateUp::class);

tests/Doctrine/Migrations/Tests/Stub/Functional/DryRun/DryRun1.php

@@ -0,0 +1,22 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\Migrations\Tests\Stub\Functional\DryRun;
+
+use Doctrine\DBAL\Schema\Schema;
+use Doctrine\Migrations\AbstractMigration;
+
+class DryRun1 extends AbstractMigration
+{
+    public function up(Schema $schema) : void
+    {
+        $table = $schema->createTable('foo');
+        $table->addColumn('id', 'integer');
+    }
+
+    public function down(Schema $schema) : void
+    {
+        $schema->dropTable('foo');
+    }
+}

tests/Doctrine/Migrations/Tests/Stub/Functional/DryRun/DryRun2.php

@@ -0,0 +1,23 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\Migrations\Tests\Stub\Functional\DryRun;
+
+use Doctrine\DBAL\Schema\Schema;
+use Doctrine\Migrations\AbstractMigration;
+
+class DryRun2 extends AbstractMigration
+{
+    public function up(Schema $schema) : void
+    {
+        $table = $schema->getTable('foo');
+        $table->addColumn('bar', 'string', ['notnull' => false]);
+    }
+
+    public function down(Schema $schema) : void
+    {
+        $table = $schema->getTable('foo');
+        $table->dropColumn('bar');
+    }
+}

tests/Doctrine/Migrations/Tests/Version/ExecutionResultTest.php

@@ -4,9 +4,11 @@
 
 namespace Doctrine\Migrations\Tests\Version;
 
+use Doctrine\DBAL\Schema\Schema;
 use Doctrine\Migrations\Version\ExecutionResult;
 use InvalidArgumentException;
 use PHPUnit\Framework\TestCase;
+use RuntimeException;
 
 class ExecutionResultTest extends TestCase
 {
@@ -95,6 +97,23 @@ public function testException() : void
         self::assertSame($exception, $this->versionExecutionResult->getException());
     }
 
+    public function testToSchema() : void
+    {
+        $toSchema = $this->createMock(Schema::class);
+
+        $this->versionExecutionResult->setToSchema($toSchema);
+
+        self::assertSame($toSchema, $this->versionExecutionResult->getToSchema());
+    }
+
+    public function testToSchemaThrowsRuntimExceptionWhenToSchemaIsNull() : void
+    {
+        self::expectException(RuntimeException::class);
+        self::expectExceptionMessage('Cannot call getToSchema() when toSchema is null.');
+
+        $this->versionExecutionResult->getToSchema();
+    }
+
     protected function setUp() : void
     {
         $this->versionExecutionResult = new ExecutionResult(