planning for single version returns migrations in order

doctrine · Dec 21, 2019 · ef9d12a · ef9d12a
1 parent 682eadc
commit ef9d12a
Show file tree

Hide file tree

Showing 4 changed files with 120 additions and 42 deletions.
diff --git a/lib/Doctrine/Migrations/MigrationPlanCalculator.php b/lib/Doctrine/Migrations/MigrationPlanCalculator.php
@@ -4,6 +4,7 @@
 
 namespace Doctrine\Migrations;
 
+use Doctrine\Migrations\Exception\MigrationClassNotFound;
 use Doctrine\Migrations\Exception\NoMigrationsFoundWithCriteria;
 use Doctrine\Migrations\Exception\NoMigrationsToExecute;
 use Doctrine\Migrations\Metadata\AvailableMigration;
@@ -15,17 +16,21 @@
 use Doctrine\Migrations\Metadata\Storage\MetadataStorage;
 use Doctrine\Migrations\Version\Direction;
 use Doctrine\Migrations\Version\Version;
+use function array_diff;
 use function array_filter;
 use function array_map;
 use function array_reverse;
+use function count;
+use function in_array;
+use function reset;
 
 /**
  * The MigrationPlanCalculator is responsible for calculating the plan for migrating from the current
  * version to another version.
  *
  * @internal
  */
-final class MigrationPlanCalculator
+class MigrationPlanCalculator
 {
     /** @var MigrationRepository */
     private $migrationRepository;
@@ -44,11 +49,23 @@ public function __construct(MigrationRepository $migrationRepository, MetadataSt
      */
     public function getPlanForVersions(array $versions, string $direction) : MigrationPlanList
     {
-        $planItems = array_map(function (Version $version) use ($direction) : MigrationPlan {
-            $migration = $this->migrationRepository->getMigration($version);
-
-            return new MigrationPlan($migration->getVersion(), $migration->getMigration(), $direction);
-        }, $versions);
+        $migrationsToCheck   = $this->arrangeMigrationsForDirection($direction, $this->migrationRepository->getMigrations());
+        $availableMigrations = array_filter($migrationsToCheck, static function (AvailableMigration $availableMigration) use ($versions) : bool {
+            // in_array third parameter is intentionally false to force object to string casting
+            return in_array($availableMigration->getVersion(), $versions, false);
+        });
+
+        $planItems = array_map(static function (AvailableMigration $availableMigration) use ($direction) : MigrationPlan {
+            return new MigrationPlan($availableMigration->getVersion(), $availableMigration->getMigration(), $direction);
+        }, $availableMigrations);
+
+        if (count($planItems) !== count($versions)) {
+            $plannedVersions = array_map(static function (MigrationPlan $migrationPlan) : Version {
+                return $migrationPlan->getVersion();
+            }, $planItems);
+            $diff            = array_diff($versions, $plannedVersions);
+            throw MigrationClassNotFound::new((string) reset($diff));
+        }
 
         return new MigrationPlanList($planItems, $direction);
     }

diff --git a/phpstan.neon.dist b/phpstan.neon.dist
@@ -12,6 +12,9 @@ parameters:
         # Ignore proxy manager magic
         - '~ProxyManager\\Proxy\\VirtualProxyInterface~'
         - '~Variable method call on Doctrine\\Migrations\\AbstractMigration~'
+        -
+            message: '~^Call to function in_array\(\) requires parameter #3 to be true\.$~'
+            path: %currentWorkingDirectory%/lib/Doctrine/Migrations/MigrationPlanCalculator.php
         -
             message: '~^Variable property access on mixed\.$~'
             path: %currentWorkingDirectory%/lib/Doctrine/Migrations/Configuration/Loader/XmlFileLoader.php

diff --git a/tests/Doctrine/Migrations/Tests/MigrationPlanCalculatorTest.php b/tests/Doctrine/Migrations/Tests/MigrationPlanCalculatorTest.php
@@ -5,6 +5,7 @@
 namespace Doctrine\Migrations\Tests;
 
 use Doctrine\Migrations\AbstractMigration;
+use Doctrine\Migrations\Exception\MigrationClassNotFound;
 use Doctrine\Migrations\Exception\NoMigrationsToExecute;
 use Doctrine\Migrations\Metadata\AvailableMigration;
 use Doctrine\Migrations\Metadata\AvailableMigrationsList;
@@ -52,10 +53,8 @@ public function testPlanForVersionsWhenNoMigrations() : void
 
         $this->migrationRepository
             ->expects(self::any())
-            ->method('getMigration')
-            ->willReturnCallback(static function (Version $version) use ($migrationList) {
-                return $migrationList->getMigration($version);
-            });
+            ->method('getMigrations')
+            ->willReturn($migrationList);
 
         $plan = $this->migrationPlanCalculator->getPlanForVersions([new Version('C')], Direction::UP);
 
@@ -65,11 +64,77 @@ public function testPlanForVersionsWhenNoMigrations() : void
         self::assertEquals(new Version('C'), $plan->getFirst()->getVersion());
     }
 
+    public function testPlanForMultipleVersionsAreSortedUp() : void
+    {
+        $m1 = new AvailableMigration(new Version('A'), $this->abstractMigration);
+        $m2 = new AvailableMigration(new Version('B'), $this->abstractMigration);
+        $m3 = new AvailableMigration(new Version('C'), $this->abstractMigration);
+
+        $migrationList = new AvailableMigrationsList([$m1, $m2, $m3]);
+        $this->migrationRepository
+            ->expects(self::any())
+            ->method('getMigrations')
+            ->willReturn($migrationList);
+
+        $plan = $this->migrationPlanCalculator->getPlanForVersions([new Version('C'), new Version('A')], Direction::UP);
+
+        self::assertCount(2, $plan);
+        self::assertSame(Direction::UP, $plan->getDirection());
+        self::assertSame(Direction::UP, $plan->getFirst()->getDirection());
+
+        self::assertEquals(new Version('A'), $plan->getFirst()->getVersion());
+        self::assertEquals(new Version('C'), $plan->getLast()->getVersion());
+    }
+
+    public function testPlanForMultipleVersionsAreSortedDown() : void
+    {
+        $m1 = new AvailableMigration(new Version('A'), $this->abstractMigration);
+        $m2 = new AvailableMigration(new Version('B'), $this->abstractMigration);
+        $m3 = new AvailableMigration(new Version('C'), $this->abstractMigration);
+
+        $migrationList = new AvailableMigrationsList([$m1, $m2, $m3]);
+        $this->migrationRepository
+            ->expects(self::any())
+            ->method('getMigrations')
+            ->willReturn($migrationList);
+
+        $plan = $this->migrationPlanCalculator->getPlanForVersions([new Version('C'), new Version('A')], Direction::UP);
+
+        self::assertCount(2, $plan);
+        self::assertSame(Direction::UP, $plan->getDirection());
+        self::assertSame(Direction::UP, $plan->getFirst()->getDirection());
+
+        self::assertEquals(new Version('A'), $plan->getFirst()->getVersion());
+        self::assertEquals(new Version('C'), $plan->getLast()->getVersion());
+    }
+
+    public function testPlanForNoMigration() : void
+    {
+        $this->expectException(MigrationClassNotFound::class);
+        $this->expectExceptionMessage('Migration class "B" was not found?');
+
+        $this->migrationRepository
+            ->expects(self::once())
+            ->method('getMigrations')
+            ->willReturn(new AvailableMigrationsList([]));
+
+        $plan = $this->migrationPlanCalculator->getPlanForVersions([new Version('B')], Direction::UP);
+
+        self::assertCount(0, $plan);
+        self::assertSame(Direction::UP, $plan->getDirection());
+    }
+
     public function testPlanForNoVersions() : void
     {
+        $m1 = new AvailableMigration(new Version('A'), $this->abstractMigration);
+        $m2 = new AvailableMigration(new Version('B'), $this->abstractMigration);
+        $m3 = new AvailableMigration(new Version('C'), $this->abstractMigration);
+
+        $migrationList = new AvailableMigrationsList([$m1, $m2, $m3]);
         $this->migrationRepository
-            ->expects(self::never())
-            ->method('getMigration');
+            ->expects(self::any())
+            ->method('getMigrations')
+            ->willReturn($migrationList);
 
         $plan = $this->migrationPlanCalculator->getPlanForVersions([], Direction::UP);
 

diff --git a/tests/Doctrine/Migrations/Tests/Tools/Console/Command/ExecuteCommandTest.php b/tests/Doctrine/Migrations/Tests/Tools/Console/Command/ExecuteCommandTest.php
@@ -7,16 +7,15 @@
 use Doctrine\Migrations\AbstractMigration;
 use Doctrine\Migrations\Configuration\Configuration;
 use Doctrine\Migrations\DependencyFactory;
-use Doctrine\Migrations\Metadata\AvailableMigration;
+use Doctrine\Migrations\Metadata\MigrationPlan;
 use Doctrine\Migrations\Metadata\MigrationPlanList;
-use Doctrine\Migrations\Metadata\Storage\MetadataStorage;
 use Doctrine\Migrations\Metadata\Storage\TableMetadataStorageConfiguration;
 use Doctrine\Migrations\MigrationPlanCalculator;
-use Doctrine\Migrations\MigrationRepository;
 use Doctrine\Migrations\Migrator;
 use Doctrine\Migrations\MigratorConfiguration;
 use Doctrine\Migrations\QueryWriter;
 use Doctrine\Migrations\Tools\Console\Command\ExecuteCommand;
+use Doctrine\Migrations\Version\Direction;
 use Doctrine\Migrations\Version\Version;
 use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
@@ -41,8 +40,8 @@ class ExecuteCommandTest extends TestCase
     /** @var MockObject */
     private $queryWriter;
 
-    /** @var MigrationRepository|MockObject */
-    private $migrationRepository;
+    /** @var MigrationPlanCalculator|MockObject */
+    private $planCalculator;
 
     /**
      * @param mixed $arg
@@ -110,18 +109,14 @@ public function testExecute() : void
     public function testExecuteMultiple() : void
     {
         $migration = $this->createMock(AbstractMigration::class);
-        $m1        = new AvailableMigration(new Version('1'), $migration);
+        $p1        = new MigrationPlan(new Version('1'), $migration, Direction::UP);
+        $pl        = new MigrationPlanList([$p1], Direction::UP);
 
-        $expectedMigrations = ['1', '2'];
-        $i                  = 0;
-        $this->migrationRepository
-            ->expects(self::exactly(2))
-            ->method('getMigration')
-            ->willReturnCallback(static function (Version $version) use ($m1, $expectedMigrations, &$i) : AvailableMigration {
-                self::assertSame($expectedMigrations[$i++], (string) $version);
-
-                return $m1;
-            });
+        $this->planCalculator
+            ->expects(self::once())
+            ->method('getPlanForVersions')
+            ->with([new Version('1'), new Version('2')])
+            ->willReturn($pl);
 
         $this->executeCommand->expects(self::once())
             ->method('canExecute')
@@ -174,22 +169,24 @@ protected function setUp() : void
             ->setMethodsExcept(['getConsoleInputMigratorConfigurationFactory'])
             ->getMock();
 
-        $storage = $this->createMock(MetadataStorage::class);
-
         $this->migrator = $this->createMock(Migrator::class);
 
         $this->queryWriter = $this->createMock(QueryWriter::class);
 
         $migration = $this->createMock(AbstractMigration::class);
-        $m1        = new AvailableMigration(new Version('1'), $migration);
 
-        $this->migrationRepository = $this->createMock(MigrationRepository::class);
-        $this->migrationRepository
-            ->expects(self::atLeast(1))
-            ->method('getMigration')
-            ->willReturn($m1);
+        $p1 = new MigrationPlan(new Version('1'), $migration, Direction::UP);
+        $pl = new MigrationPlanList([$p1], Direction::UP);
+
+        $this->planCalculator = $this->getMockBuilder(MigrationPlanCalculator::class)
+            ->disableOriginalConstructor()
+            ->onlyMethods(['getPlanForVersions'])
+            ->getMock();
 
-        $planCalculator = new MigrationPlanCalculator($this->migrationRepository, $storage);
+        $this->planCalculator
+            ->expects(self::once())
+            ->method('getPlanForVersions')
+            ->willReturn($pl);
 
         $configuration = new Configuration();
         $configuration->setMetadataStorageConfiguration(new TableMetadataStorageConfiguration());
@@ -205,16 +202,12 @@ protected function setUp() : void
 
         $this->dependencyFactory->expects(self::once())
             ->method('getMigrationPlanCalculator')
-            ->willReturn($planCalculator);
+            ->willReturn($this->planCalculator);
 
         $this->dependencyFactory->expects(self::any())
             ->method('getQueryWriter')
             ->willReturn($this->queryWriter);
 
-        $this->dependencyFactory->expects(self::any())
-            ->method('getMigrationRepository')
-            ->willReturn($this->migrationRepository);
-
         $this->executeCommand = $this->getMockBuilder(ExecuteCommand::class)
             ->setConstructorArgs([null, $this->dependencyFactory])
             ->onlyMethods(['canExecute'])