From ef9d12ac6f38a5f117169b7c693336f29ab39ccb Mon Sep 17 00:00:00 2001 From: Asmir Mustafic Date: Sat, 14 Dec 2019 17:56:26 +0100 Subject: [PATCH] planning for single version returns migrations in order --- .../Migrations/MigrationPlanCalculator.php | 29 +++++-- phpstan.neon.dist | 3 + .../Tests/MigrationPlanCalculatorTest.php | 77 +++++++++++++++++-- .../Console/Command/ExecuteCommandTest.php | 53 ++++++------- 4 files changed, 120 insertions(+), 42 deletions(-) diff --git a/lib/Doctrine/Migrations/MigrationPlanCalculator.php b/lib/Doctrine/Migrations/MigrationPlanCalculator.php index 70a6805e6b..039f2ba74c 100644 --- 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,9 +16,13 @@ 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 @@ -25,7 +30,7 @@ * * @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 index 40660c1334..27f942fd1d 100644 --- 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 index 213e483b9e..a1743fdca2 100644 --- 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 index 454858c30b..63b832f3d5 100644 --- 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'])