From 84af855f7fa422bec4adc81e5eff6e7020078101 Mon Sep 17 00:00:00 2001 From: Florian Klein Date: Thu, 23 Apr 2020 09:54:39 +0200 Subject: [PATCH 1/5] calculate diff correctly to verify up-to-date status --- .../Migrations/Tracking/TableManipulator.php | 10 +-- .../Migrations/Tracking/TableStatus.php | 30 -------- .../MigrationTableManipulatorTest.php | 4 - .../Tests/Tracking/TableManipulatorTest.php | 12 --- .../Tests/Tracking/TableStatusTest.php | 73 ------------------- 5 files changed, 2 insertions(+), 127 deletions(-) diff --git a/lib/Doctrine/Migrations/Tracking/TableManipulator.php b/lib/Doctrine/Migrations/Tracking/TableManipulator.php index 6dbbde1e8c..b014f54aa6 100644 --- a/lib/Doctrine/Migrations/Tracking/TableManipulator.php +++ b/lib/Doctrine/Migrations/Tracking/TableManipulator.php @@ -53,15 +53,9 @@ public function createMigrationTable() : bool } if ($this->migrationTableStatus->isCreated()) { - if (! $this->migrationTableStatus->isUpToDate()) { - $this->migrationTableUpdater->updateMigrationTable(); + $this->migrationTableUpdater->updateMigrationTable(); - $this->migrationTableStatus->setUpToDate(true); - - return true; - } - - return false; + return true; } $table = $this->migrationTable->getNewDBALTable(); diff --git a/lib/Doctrine/Migrations/Tracking/TableStatus.php b/lib/Doctrine/Migrations/Tracking/TableStatus.php index a3b781de4f..b84481b2f6 100644 --- a/lib/Doctrine/Migrations/Tracking/TableStatus.php +++ b/lib/Doctrine/Migrations/Tracking/TableStatus.php @@ -22,9 +22,6 @@ class TableStatus /** @var bool|null */ private $created; - /** @var bool|null */ - private $upToDate; - public function __construct( AbstractSchemaManager $schemaManager, TableDefinition $migrationTable @@ -48,31 +45,4 @@ public function isCreated() : bool return $this->created; } - - public function setUpToDate(bool $upToDate) : void - { - $this->upToDate = $upToDate; - } - - public function isUpToDate() : bool - { - if ($this->upToDate !== null) { - return $this->upToDate; - } - - $table = $this->schemaManager->listTableDetails($this->migrationTable->getName()); - - $this->upToDate = true; - - foreach ($this->migrationTable->getColumnNames() as $columnName) { - if ($table->hasColumn($columnName)) { - continue; - } - - $this->upToDate = false; - break; - } - - return $this->upToDate; - } } diff --git a/tests/Doctrine/Migrations/Tests/Functional/MigrationTableManipulatorTest.php b/tests/Doctrine/Migrations/Tests/Functional/MigrationTableManipulatorTest.php index 118eab593a..71f2b642a6 100644 --- a/tests/Doctrine/Migrations/Tests/Functional/MigrationTableManipulatorTest.php +++ b/tests/Doctrine/Migrations/Tests/Functional/MigrationTableManipulatorTest.php @@ -31,7 +31,6 @@ public function testCreateMigrationTable() : void self::assertTrue($schemaManager->tablesExist(['doctrine_migration_versions'])); self::assertTrue($this->trackingTableStatus->isCreated()); - self::assertTrue($this->trackingTableStatus->isUpToDate()); $table = $schemaManager->listTableDetails('doctrine_migration_versions'); @@ -58,12 +57,9 @@ public function testUpdateMigrationTable() : void } self::assertTrue($this->trackingTableStatus->isCreated()); - self::assertFalse($this->trackingTableStatus->isUpToDate()); self::assertTrue($this->trackingTableManipulator->createMigrationTable()); - self::assertTrue($this->trackingTableStatus->isUpToDate()); - $schemaManager = $this->connection->getSchemaManager(); $table = $schemaManager->listTableDetails('doctrine_migration_versions'); diff --git a/tests/Doctrine/Migrations/Tests/Tracking/TableManipulatorTest.php b/tests/Doctrine/Migrations/Tests/Tracking/TableManipulatorTest.php index 95971e75de..6089514d98 100644 --- a/tests/Doctrine/Migrations/Tests/Tracking/TableManipulatorTest.php +++ b/tests/Doctrine/Migrations/Tests/Tracking/TableManipulatorTest.php @@ -47,10 +47,6 @@ public function testCreateMigrationTableAlreadyCreated() : void ->method('isCreated') ->willReturn(true); - $this->migrationTableStatus->expects(self::once()) - ->method('isUpToDate') - ->willReturn(true); - self::assertFalse($this->migrationTableManipulator->createMigrationTable()); } @@ -78,17 +74,9 @@ public function testCreateMigrationTableNotUpToDate() : void ->method('isCreated') ->willReturn(true); - $this->migrationTableStatus->expects(self::once()) - ->method('isUpToDate') - ->willReturn(false); - $this->migrationTableUpdater->expects(self::once()) ->method('updateMigrationTable'); - $this->migrationTableStatus->expects(self::once()) - ->method('setUpToDate') - ->with(true); - self::assertTrue($migrationTableManipulator->createMigrationTable()); } diff --git a/tests/Doctrine/Migrations/Tests/Tracking/TableStatusTest.php b/tests/Doctrine/Migrations/Tests/Tracking/TableStatusTest.php index 67d393d92c..70a706f81d 100644 --- a/tests/Doctrine/Migrations/Tests/Tracking/TableStatusTest.php +++ b/tests/Doctrine/Migrations/Tests/Tracking/TableStatusTest.php @@ -57,79 +57,6 @@ public function testIsCreatedFalse() : void self::assertFalse($this->migrationTableStatus->isCreated()); } - public function testSetUpToDate() : void - { - $this->migrationTableStatus->setUpToDate(true); - - self::assertTrue($this->migrationTableStatus->isUpToDate()); - } - - public function testIsUpToDateTrue() : void - { - $this->migrationTable->expects(self::once()) - ->method('getName') - ->willReturn('table_name'); - - $table = $this->createMock(Table::class); - - $this->schemaManager->expects(self::once()) - ->method('listTableDetails') - ->with('table_name') - ->willReturn($table); - - $this->migrationTable->expects(self::once()) - ->method('getColumnNames') - ->willReturn([ - 'version', - 'executed_at', - ]); - - $table->expects(self::at(0)) - ->method('hasColumn') - ->with('version') - ->willReturn(true); - - $table->expects(self::at(1)) - ->method('hasColumn') - ->with('executed_at') - ->willReturn(true); - - self::assertTrue($this->migrationTableStatus->isUpToDate()); - } - - public function testIsUpToDateFalse() : void - { - $this->migrationTable->expects(self::once()) - ->method('getName') - ->willReturn('table_name'); - - $table = $this->createMock(Table::class); - - $this->schemaManager->expects(self::once()) - ->method('listTableDetails') - ->with('table_name') - ->willReturn($table); - - $this->migrationTable->expects(self::once()) - ->method('getColumnNames') - ->willReturn([ - 'version', - 'executed_at', - ]); - - $table->expects(self::at(0)) - ->method('hasColumn') - ->with('version') - ->willReturn(true); - - $table->expects(self::at(1)) - ->method('hasColumn') - ->with('executed_at') - ->willReturn(false); - - self::assertFalse($this->migrationTableStatus->isUpToDate()); - } - protected function setUp() : void { $this->schemaManager = $this->createMock(AbstractSchemaManager::class); From 239ffdb2cd3b77e5e522d5a22e90ad6f2d189600 Mon Sep 17 00:00:00 2001 From: Florian Klein Date: Thu, 23 Apr 2020 10:08:37 +0200 Subject: [PATCH 2/5] fix failing tests --- lib/Doctrine/Migrations/Tracking/TableManipulator.php | 4 +--- lib/Doctrine/Migrations/Tracking/TableUpdater.php | 4 +++- .../Migrations/Tests/Tracking/TableManipulatorTest.php | 3 ++- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/lib/Doctrine/Migrations/Tracking/TableManipulator.php b/lib/Doctrine/Migrations/Tracking/TableManipulator.php index b014f54aa6..e94327e5f4 100644 --- a/lib/Doctrine/Migrations/Tracking/TableManipulator.php +++ b/lib/Doctrine/Migrations/Tracking/TableManipulator.php @@ -53,9 +53,7 @@ public function createMigrationTable() : bool } if ($this->migrationTableStatus->isCreated()) { - $this->migrationTableUpdater->updateMigrationTable(); - - return true; + return $this->migrationTableUpdater->updateMigrationTable(); } $table = $this->migrationTable->getNewDBALTable(); diff --git a/lib/Doctrine/Migrations/Tracking/TableUpdater.php b/lib/Doctrine/Migrations/Tracking/TableUpdater.php index 66693acdf2..ce94331628 100644 --- a/lib/Doctrine/Migrations/Tracking/TableUpdater.php +++ b/lib/Doctrine/Migrations/Tracking/TableUpdater.php @@ -43,7 +43,7 @@ public function __construct( $this->platform = $platform; } - public function updateMigrationTable() : void + public function updateMigrationTable() : bool { $fromTable = $this->getFromTable(); $toTable = $this->migrationTable->getDBALTable(); @@ -66,6 +66,8 @@ public function updateMigrationTable() : void } $this->connection->commit(); + + return !empty($queries); } /** diff --git a/tests/Doctrine/Migrations/Tests/Tracking/TableManipulatorTest.php b/tests/Doctrine/Migrations/Tests/Tracking/TableManipulatorTest.php index 6089514d98..10b5e81a64 100644 --- a/tests/Doctrine/Migrations/Tests/Tracking/TableManipulatorTest.php +++ b/tests/Doctrine/Migrations/Tests/Tracking/TableManipulatorTest.php @@ -75,7 +75,8 @@ public function testCreateMigrationTableNotUpToDate() : void ->willReturn(true); $this->migrationTableUpdater->expects(self::once()) - ->method('updateMigrationTable'); + ->method('updateMigrationTable') + ->willReturn(true); self::assertTrue($migrationTableManipulator->createMigrationTable()); } From 7a48626c02894182d69d2605cbc41473b23a90a9 Mon Sep 17 00:00:00 2001 From: Florian Klein Date: Thu, 23 Apr 2020 10:14:23 +0200 Subject: [PATCH 3/5] fix style --- lib/Doctrine/Migrations/Tracking/TableUpdater.php | 2 +- tests/Doctrine/Migrations/Tests/Tracking/TableStatusTest.php | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/Doctrine/Migrations/Tracking/TableUpdater.php b/lib/Doctrine/Migrations/Tracking/TableUpdater.php index ce94331628..e9d8ee58b4 100644 --- a/lib/Doctrine/Migrations/Tracking/TableUpdater.php +++ b/lib/Doctrine/Migrations/Tracking/TableUpdater.php @@ -67,7 +67,7 @@ public function updateMigrationTable() : bool $this->connection->commit(); - return !empty($queries); + return 0 !== count($queries); } /** diff --git a/tests/Doctrine/Migrations/Tests/Tracking/TableStatusTest.php b/tests/Doctrine/Migrations/Tests/Tracking/TableStatusTest.php index 70a706f81d..637650f8dd 100644 --- a/tests/Doctrine/Migrations/Tests/Tracking/TableStatusTest.php +++ b/tests/Doctrine/Migrations/Tests/Tracking/TableStatusTest.php @@ -5,7 +5,6 @@ namespace Doctrine\Migrations\Tests\Tracking; use Doctrine\DBAL\Schema\AbstractSchemaManager; -use Doctrine\DBAL\Schema\Table; use Doctrine\Migrations\Tracking\TableDefinition; use Doctrine\Migrations\Tracking\TableStatus; use PHPUnit\Framework\MockObject\MockObject; From de540fdcfd590afdbed27d02cae3229efadab857 Mon Sep 17 00:00:00 2001 From: Florian Klein Date: Thu, 23 Apr 2020 11:00:34 +0200 Subject: [PATCH 4/5] fix style --- lib/Doctrine/Migrations/Tracking/TableUpdater.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/Doctrine/Migrations/Tracking/TableUpdater.php b/lib/Doctrine/Migrations/Tracking/TableUpdater.php index e9d8ee58b4..7a242e464c 100644 --- a/lib/Doctrine/Migrations/Tracking/TableUpdater.php +++ b/lib/Doctrine/Migrations/Tracking/TableUpdater.php @@ -11,6 +11,7 @@ use Doctrine\DBAL\Schema\Table; use Throwable; use function in_array; +use function count; /** * The TableUpdater class is responsible for updating the tracking table schema if it needs to be updated. @@ -67,7 +68,7 @@ public function updateMigrationTable() : bool $this->connection->commit(); - return 0 !== count($queries); + return count($queries) !== 0; } /** From 7abf8761f0723786b95acb493b69171d706710b1 Mon Sep 17 00:00:00 2001 From: Florian Klein Date: Thu, 23 Apr 2020 11:21:16 +0200 Subject: [PATCH 5/5] fix style --- lib/Doctrine/Migrations/Tracking/TableUpdater.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Doctrine/Migrations/Tracking/TableUpdater.php b/lib/Doctrine/Migrations/Tracking/TableUpdater.php index 7a242e464c..18a397649b 100644 --- a/lib/Doctrine/Migrations/Tracking/TableUpdater.php +++ b/lib/Doctrine/Migrations/Tracking/TableUpdater.php @@ -10,8 +10,8 @@ use Doctrine\DBAL\Schema\Schema; use Doctrine\DBAL\Schema\Table; use Throwable; -use function in_array; use function count; +use function in_array; /** * The TableUpdater class is responsible for updating the tracking table schema if it needs to be updated.