Don't Prompt the User for Confirmation on the migrations:migrate Command When There is Nothing to Do

lib/Doctrine/DBAL/Migrations/Migration.php

@@ -118,12 +118,11 @@ public function setNoMigrationException($noMigrationException = false)
      * @param string  $to             The version to migrate to.
      * @param boolean $dryRun         Whether or not to make this a dry run and not execute anything.
      * @param boolean $timeAllQueries Measuring or not the execution time of each SQL query.
-     *
-     * @return array $sql     The array of migration sql statements
-     *
+     * @param callable|null $confirm A callback to confirm whether the migrations should be executed.
+     * @return array|false An array of migration sql statements or false if the confirm callback denied execution
      * @throws MigrationException
      */
-    public function migrate($to = null, $dryRun = false, $timeAllQueries = false)
+    public function migrate($to = null, $dryRun = false, $timeAllQueries = false, callable $confirm = null)
     {
         /**
          * If no version to migrate to is given we default to the last available one.
@@ -156,7 +155,11 @@ public function migrate($to = null, $dryRun = false, $timeAllQueries = false)
          * to signify that there is nothing left to do.
          */
         if ($from === $to && empty($migrationsToExecute) && !empty($migrations)) {
-            return [];
+            return $this->noMigrations();
+        }
+
+        if (!$dryRun && false === $this->migrationsCanExecute($confirm)) {
+            return false;
         }
 
         $output = $dryRun ? 'Executing dry run of migration' : 'Migrating';
@@ -168,6 +171,8 @@ public function migrate($to = null, $dryRun = false, $timeAllQueries = false)
          */
         if (empty($migrationsToExecute) && !$this->noMigrationException) {
             throw MigrationException::noMigrationsToExecute();
+        } elseif (empty($migrationsToExecute)) {
+            return $this->noMigrations();
         }
 
         $sql = [];
@@ -185,4 +190,15 @@ public function migrate($to = null, $dryRun = false, $timeAllQueries = false)
 
         return $sql;
     }
+
+    private function noMigrations()
+    {
+        $this->outputWriter->write('<comment>No migrations to execute.</comment>');
+        return [];
+    }
+
+    private function migrationsCanExecute(callable $confirm=null)
+    {
+        return null === $confirm ? true : $confirm();
+    }
 }

lib/Doctrine/DBAL/Migrations/Tools/Console/Command/MigrateCommand.php

@@ -83,7 +83,7 @@ protected function configure()
     public function execute(InputInterface $input, OutputInterface $output)
     {
         $configuration = $this->getMigrationConfiguration($input, $output);
-        $migration = new Migration($configuration);
+        $migration = $this->createMigration($configuration);
 
         $this->outputHeader($configuration, $output);
 
@@ -124,30 +124,36 @@ public function execute(InputInterface $input, OutputInterface $output)
         if ($path = $input->getOption('write-sql')) {
             $path = is_bool($path) ? getcwd() : $path;
             $migration->writeSqlFile($path, $version);
-        } else {
-            $dryRun = (boolean) $input->getOption('dry-run');
-
-            // warn the user if no dry run and interaction is on
-            if (! $dryRun) {
-                $question = 'WARNING! You are about to execute a database migration'
-                    . ' that could result in schema changes and data lost.'
-                    . ' Are you sure you wish to continue? (y/n)';
-                if (! $this->canExecute($question, $input, $output)) {
-                    $output->writeln('<error>Migration cancelled!</error>');
-
-                    return 1;
-                }
-            }
+            return 0;
+        }
 
-            $migration->setNoMigrationException($input->getOption('allow-no-migration'));
-            $sql = $migration->migrate($version, $dryRun, $timeAllqueries);
+        $dryRun = (boolean) $input->getOption('dry-run');
 
-            if (empty($sql)) {
-                $output->writeln('<comment>No migrations to execute.</comment>');
-            }
+        $migration->setNoMigrationException($input->getOption('allow-no-migration'));
+        $result = $migration->migrate($version, $dryRun, $timeAllqueries, function () use ($input, $output) {
+            $question = 'WARNING! You are about to execute a database migration'
+                . ' that could result in schema changes and data lost.'
+                . ' Are you sure you wish to continue? (y/n)';
+            return $this->canExecute($question, $input, $output);
+        });
+
+        if (false === $result) {
+            $output->writeln('<error>Migration cancelled!</error>');
+            return 1;
         }
     }
 
+    /**
+     * Create a new migration instance to execute the migrations.
+     *
+     * @param $configuration The configuration with which the migrations will be executed
+     * @return Migration a new migration instance
+     */
+    protected function createMigration(Configuration $configuration)
+    {
+        return new Migration($configuration);
+    }
+
     /**
      * @param string $question
      * @param InputInterface $input

tests/Doctrine/DBAL/Migrations/Tests/MigrationTest.php

@@ -44,6 +44,7 @@ function realpath($path)
 use Doctrine\DBAL\Migrations\Migration;
 use Doctrine\DBAL\Migrations\MigrationException;
 use Doctrine\DBAL\Migrations\OutputWriter;
+use Doctrine\DBAL\Migrations\Tests\Stub\Functional\MigrateNotTouchingTheSchema;
 use \Mockery as m;
 use org\bovigo\vfs\vfsStream;
 use org\bovigo\vfs\vfsStreamFile;
@@ -54,14 +55,16 @@ function realpath($path)
  */
 class MigrationTest extends MigrationTestCase
 {
+    private $conn;
     /** @var Configuration */
     private $config;
 
     protected $output;
 
     protected function setUp()
     {
-        $this->config = new Configuration($this->getSqliteConnection());
+        $this->conn = $this->getSqliteConnection();
+        $this->config = new Configuration($this->conn);
         $this->config->setMigrationsDirectory(__DIR__ . DIRECTORY_SEPARATOR . 'Stub/migration-empty-folder');
         $this->config->setMigrationsNamespace('DoctrineMigrations\\');
     }
@@ -90,10 +93,18 @@ public function testMigrateWithNoMigrationsThrowsException()
 
     public function testMigrateWithNoMigrationsDontThrowsExceptionIfContiniousIntegrationOption()
     {
+        $messages = [];
+        $output = new OutputWriter(function ($msg) use (&$messages) {
+            $messages[] = $msg;
+        });
+        $this->config->setOutputWriter($output);
         $migration = new Migration($this->config);
 
         $migration->setNoMigrationException(true);
         $migration->migrate();
+
+        $this->assertCount(2, $messages, 'should output header and no migrations message');
+        $this->assertContains('No migrations', $messages[1]);
     }
 
     /**
@@ -186,4 +197,63 @@ public function testWriteSqlFileShouldUseStandardCommentMarkerInSql()
         $this->assertRegExp('/^\s*-- Version 1/m', $sqlMigrationFile->getContent());
         $this->assertNotRegExp('/^\s*#/m', $sqlMigrationFile->getContent());
     }
+
+    public function testMigrateWithMigrationsAndAddTheCurrentVersionOutputsANoMigrationsMessage()
+    {
+        $messages = [];
+        $output = new OutputWriter(function ($msg) use (&$messages) {
+            $messages[] = $msg;
+        });
+        $this->config->setOutputWriter($output);
+        $this->config->setMigrationsDirectory(__DIR__.'/Stub/migrations-empty-folder');
+        $this->config->setMigrationsNamespace('DoctrineMigrations\\');
+        $this->config->registerMigration('20160707000000', MigrateNotTouchingTheSchema::class);
+        $this->config->createMigrationTable();
+        $this->conn->insert($this->config->getMigrationsTableName(), [
+            'version' => '20160707000000',
+        ]);
+
+        $migration = new Migration($this->config);
+
+        $migration->migrate();
+
+        $this->assertCount(1, $messages, 'should output the no migrations message');
+        $this->assertContains('No migrations', $messages[0]);
+    }
+
+    public function testMigrateReturnsFalseWhenTheConfirmationIsDeclined()
+    {
+        $this->config->setMigrationsDirectory(__DIR__.'/Stub/migrations-empty-folder');
+        $this->config->setMigrationsNamespace('DoctrineMigrations\\');
+        $this->config->registerMigration('20160707000000', MigrateNotTouchingTheSchema::class);
+        $this->config->createMigrationTable();
+        $called = false;
+        $migration = new Migration($this->config);
+
+        $result = $migration->migrate(null, false, false, function () use (&$called) {
+            $called = true;
+            return false;
+        });
+
+        $this->assertFalse($result);
+        $this->assertTrue($called, 'should have called the confirmation callback');
+    }
+
+    public function testMigrateWithDryRunDoesNotCallTheConfirmationCallback()
+    {
+        $this->config->setMigrationsDirectory(__DIR__.'/Stub/migrations-empty-folder');
+        $this->config->setMigrationsNamespace('DoctrineMigrations\\');
+        $this->config->registerMigration('20160707000000', MigrateNotTouchingTheSchema::class);
+        $this->config->createMigrationTable();
+        $called = false;
+        $migration = new Migration($this->config);
+
+        $result = $migration->migrate(null, true, false, function () use (&$called) {
+            $called = true;
+            return false;
+        });
+
+        $this->assertFalse($called);
+        $this->assertEquals(['20160707000000' => ['SELECT 1']], $result);
+    }
 }

tests/Doctrine/DBAL/Migrations/Tests/Tools/Console/Command/DialogSupport.php

@@ -0,0 +1,31 @@
+<?php
+
+namespace Doctrine\DBAL\Migrations\Tests\Tools\Console\Command;
+
+use Symfony\Component\Console\Application;
+use Symfony\Component\Console\Helper\QuestionHelper;
+use Symfony\Component\Console\Helper\DialogHelper;
+
+trait DialogSupport
+{
+    protected $questions, $isDialogHelper;
+
+    protected function configureDialogs(Application $app)
+    {
+        if (class_exists(QuestionHelper::class)) {
+            $this->isDialogHelper = false;
+            $this->questions = $this->getMock(QuestionHelper::class);
+        } else {
+            $this->isDialogHelper = true;
+            $this->questions = $this->getMock(DialogHelper::class);
+        }
+        $app->getHelperSet()->set($this->questions, $this->isDialogHelper ? 'dialog' : 'question');
+    }
+
+    protected function willAskConfirmationAndReturn($bool)
+    {
+        $this->questions->expects($this->once())
+            ->method($this->isDialogHelper ? 'askConfirmation' : 'ask')
+            ->willReturn($bool);
+    }
+}

tests/Doctrine/DBAL/Migrations/Tests/Tools/Console/Command/ExecuteCommandTest.php

@@ -10,9 +10,11 @@
 
 class ExecuteCommandTest extends CommandTestCase
 {
+    use DialogSupport;
+
     const VERSION = '20160705000000';
 
-    private $version, $questions, $isDialogHelper;
+    private $version;
 
     public function testWriteSqlCommandOutputsSqlFileToTheCurrentWorkingDirectory()
     {
@@ -94,14 +96,7 @@ protected function setUp()
             ->with(self::VERSION)
             ->willReturn($this->version);
 
-        if (class_exists(QuestionHelper::class)) {
-            $this->isDialogHelper = false;
-            $this->questions = $this->getMock(QuestionHelper::class);
-        } else {
-            $this->isDialogHelper = true;
-            $this->questions = $this->getMock(DialogHelper::class);
-        }
-        $this->app->getHelperSet()->set($this->questions, $this->isDialogHelper ? 'dialog' : 'question');
+        $this->configureDialogs($this->app);
     }
 
     protected function createCommand()
@@ -121,11 +116,4 @@ private function mockWithoutConstructor($cls)
             ->disableOriginalConstructor()
             ->getMock();
     }
-
-    private function willAskConfirmationAndReturn($bool)
-    {
-        $this->questions->expects($this->once())
-            ->method($this->isDialogHelper ? 'askConfirmation' : 'ask')
-            ->willReturn($bool);
-    }
 }