Merge pull request #12 from venyii/fix-validation-lock

Prevent locking database if validation error occurred in version command
gruberro · Nov 20, 2017 · e7a4b90 · e7a4b90
2 parents bb2de92 + 02227c6
commit e7a4b90
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 2 deletions.
diff --git a/lib/Console/Command/VersionsCommand.php b/lib/Console/Command/VersionsCommand.php
@@ -89,8 +89,6 @@ protected function execute(Console\Input\InputInterface $input, Console\Output\O
         $migrations = $this->getMigrations($directories);
         $db = $this->connect($input->getOption('server'), $input->getArgument('database'));
 
-        $this->acquireLock($db);
-
         $add = $input->getOption('add');
         $delete = $input->getOption('delete');
         $all = $input->getOption('all');
@@ -108,6 +106,8 @@ protected function execute(Console\Input\InputInterface $input, Console\Output\O
             throw new Console\Exception\RuntimeException("Specify --all or a single migration id");
         }
 
+        $this->acquireLock($db);
+
         try {
             if ($all) {
                 $id = null;

diff --git a/tests/Console/Command/VersionsCommandTest.php b/tests/Console/Command/VersionsCommandTest.php
@@ -212,6 +212,36 @@ public function testExecuteWithInvalidMigrationDirectory()
         );
     }
 
+    public function testExecuteWithValidationErrorDoesNotLockDatabase()
+    {
+        $application = new Application();
+        $application->add(new MongoDbMigrations\Console\Command\VersionsCommand());
+
+        $command = $application->find('php-mongodb-migrations:version');
+        $commandTester = new CommandTester($command);
+        $hasValidationException = false;
+
+        try {
+            $commandTester->execute(
+                [
+                    'command' => $command->getName(),
+                    'database' => $this->getTestDatabaseName(),
+                ]
+            );
+        } catch (\RuntimeException $e) {
+            $hasValidationException = true;
+            $this->assertSame('Specify --all or a single migration id', $e->getMessage());
+        }
+
+        $this->assertTrue($hasValidationException, 'Expected a validation exception to be thrown');
+
+        $databaseMigrationsLockCollection = $this->getTestDatabase()->selectCollection('DATABASE_MIGRATIONS_LOCK');
+        $this->assertNull(
+            $databaseMigrationsLockCollection->findOne(['locked' => ['$exists' => true]]),
+            'The database lock must not be acquired, even if the command fails!'
+        );
+    }
+
     public function testExecuteASingleIdOnly()
     {
         $application = new Application();