From a51043cb5ada8dfe078c92942c4eee73a8311f97 Mon Sep 17 00:00:00 2001 From: Oleg Andreyev Date: Mon, 7 Mar 2022 11:29:25 +0200 Subject: [PATCH 1/3] Added condition to check that we have an active connection --- Collector/MigrationsCollector.php | 41 +++++++++++++++++-------------- 1 file changed, 23 insertions(+), 18 deletions(-) diff --git a/Collector/MigrationsCollector.php b/Collector/MigrationsCollector.php index b846a212..c8ea9bc0 100644 --- a/Collector/MigrationsCollector.php +++ b/Collector/MigrationsCollector.php @@ -10,6 +10,10 @@ use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpKernel\DataCollector\DataCollector; +use Throwable; + +use function count; +use function get_class; class MigrationsCollector extends DataCollector { @@ -21,17 +25,22 @@ class MigrationsCollector extends DataCollector public function __construct(DependencyFactory $dependencyFactory, MigrationsFlattener $migrationsFlattener) { $this->dependencyFactory = $dependencyFactory; - $this->flattener = $migrationsFlattener; + $this->flattener = $migrationsFlattener; } - public function collect(Request $request, Response $response, \Throwable $exception = null) + public function collect(Request $request, Response $response, ?Throwable $exception = null): void { - if (!empty($this->data)) { + if (! empty($this->data)) { + return; + } + + $connection = $this->dependencyFactory->getConnection(); + if (! $connection->isConnected()) { return; } $metadataStorage = $this->dependencyFactory->getMetadataStorage(); - $planCalculator = $this->dependencyFactory->getMigrationPlanCalculator(); + $planCalculator = $this->dependencyFactory->getMigrationPlanCalculator(); try { $executedMigrations = $metadataStorage->getExecutedMigrations(); @@ -46,33 +55,29 @@ public function collect(Request $request, Response $response, \Throwable $except $availableMigrations = $planCalculator->getMigrations(); - $this->data['available_migrations_count'] = count($availableMigrations); - $unavailableMigrations = $executedMigrations->unavailableSubset($availableMigrations); + $this->data['available_migrations_count'] = count($availableMigrations); + $unavailableMigrations = $executedMigrations->unavailableSubset($availableMigrations); $this->data['unavailable_migrations_count'] = count($unavailableMigrations); - $newMigrations = $availableMigrations->newSubset($executedMigrations); - $this->data['new_migrations'] = $this->flattener->flattenAvailableMigrations($newMigrations); + $newMigrations = $availableMigrations->newSubset($executedMigrations); + $this->data['new_migrations'] = $this->flattener->flattenAvailableMigrations($newMigrations); $this->data['executed_migrations'] = $this->flattener->flattenExecutedMigrations($executedMigrations, $availableMigrations); $this->data['storage'] = get_class($metadataStorage); - $configuration = $this->dependencyFactory->getConfiguration(); - $storage = $configuration->getMetadataStorageConfiguration(); + $configuration = $this->dependencyFactory->getConfiguration(); + $storage = $configuration->getMetadataStorageConfiguration(); if ($storage instanceof TableMetadataStorageConfiguration) { - $this->data['table'] = $storage->getTableName(); + $this->data['table'] = $storage->getTableName(); $this->data['column'] = $storage->getVersionColumnName(); } - $connection = $this->dependencyFactory->getConnection(); $this->data['driver'] = get_class($connection->getDriver()); - $this->data['name'] = $connection->getDatabase(); + $this->data['name'] = $connection->getDatabase(); $this->data['namespaces'] = $configuration->getMigrationDirectories(); } - /** - * @return string - */ - public function getName() + public function getName(): string { return 'doctrine_migrations'; } @@ -82,7 +87,7 @@ public function getData() return $this->data; } - public function reset() + public function reset(): void { $this->data = []; } From 6b589492aa4008d1f709fb54beb4250e8e954c00 Mon Sep 17 00:00:00 2001 From: Oleg Andreyev Date: Tue, 22 Mar 2022 20:23:59 +0200 Subject: [PATCH 2/3] removed return type --- Collector/MigrationsCollector.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Collector/MigrationsCollector.php b/Collector/MigrationsCollector.php index c8ea9bc0..19ad7324 100644 --- a/Collector/MigrationsCollector.php +++ b/Collector/MigrationsCollector.php @@ -28,7 +28,7 @@ public function __construct(DependencyFactory $dependencyFactory, MigrationsFlat $this->flattener = $migrationsFlattener; } - public function collect(Request $request, Response $response, ?Throwable $exception = null): void + public function collect(Request $request, Response $response, ?Throwable $exception = null) { if (! empty($this->data)) { return; From b0662f39e3cb5d9c1153ca4900a87f0226677311 Mon Sep 17 00:00:00 2001 From: Oleg Andreyev Date: Tue, 22 Mar 2022 20:41:43 +0200 Subject: [PATCH 3/3] added test + comment on why checking connection --- Collector/MigrationsCollector.php | 3 ++ Tests/Collector/MigrationsCollectorTest.php | 50 +++++++++++++++++++++ 2 files changed, 53 insertions(+) create mode 100644 Tests/Collector/MigrationsCollectorTest.php diff --git a/Collector/MigrationsCollector.php b/Collector/MigrationsCollector.php index 19ad7324..ccb8e346 100644 --- a/Collector/MigrationsCollector.php +++ b/Collector/MigrationsCollector.php @@ -35,6 +35,9 @@ public function collect(Request $request, Response $response, ?Throwable $except } $connection = $this->dependencyFactory->getConnection(); + // before collecting migrations, we need to make sure that Application has + // at least opened a connection, otherwise it's not guaranteed that connection + // is even configured if (! $connection->isConnected()) { return; } diff --git a/Tests/Collector/MigrationsCollectorTest.php b/Tests/Collector/MigrationsCollectorTest.php new file mode 100644 index 00000000..2b15d9f2 --- /dev/null +++ b/Tests/Collector/MigrationsCollectorTest.php @@ -0,0 +1,50 @@ +createMock(DependencyFactory::class); + $migrationsFlattener = $this->createMock(MigrationsFlattener::class); + $request = $this->createMock(Request::class); + $response = $this->createMock(Response::class); + $metadataStorage = $this->createMock(MetadataStorage::class); + $connection = $this->createMock(Connection::class); + + $dependencyFactory + ->method('getConnection') + ->willReturn($connection); + + $connection + ->method('isConnected') + ->willReturn(false); + + $dependencyFactory + ->expects($this->never()) + ->method('getMetadataStorage'); + + $dependencyFactory + ->expects($this->never()) + ->method('getMigrationPlanCalculator'); + + $metadataStorage + ->expects($this->never()) + ->method('getExecutedMigrations'); + + $target = new MigrationsCollector($dependencyFactory, $migrationsFlattener); + $target->collect($request, $response); + } +}