Merge pull request #1797 from acelaya-forks/feature/improve-new-db-check

Feature/improve new db check

CHANGELOG.md

@@ -22,7 +22,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this
 * [#1790](https://github.com/shlinkio/shlink/issues/1790) Drop support for PHP 8.1.
 
 ### Fixed
-* *Nothing*
+* [#1413](https://github.com/shlinkio/shlink/issues/1413) Fix error when creating initial DB in Postgres in a cluster where a default `postgres` db does not exist or the credentials do not grant permissions to connect.
 
 
 ## [3.6.0] - 2023-05-24

module/CLI/src/Command/Db/CreateDatabaseCommand.php

@@ -15,6 +15,7 @@ use Symfony\Component\Console\Output\OutputInterface;
 use Symfony\Component\Console\Style\SymfonyStyle;
 use Symfony\Component\Lock\LockFactory;
 use Symfony\Component\Process\PhpExecutableFinder;
+use Throwable;
 
 use function Functional\contains;
 use function Functional\map;
@@ -53,9 +54,7 @@ class CreateDatabaseCommand extends AbstractDatabaseCommand
     {
         $io = new SymfonyStyle($input, $output);
 
-        $this->checkDbExists();
-
-        if ($this->schemaExists()) {
+        if ($this->databaseTablesExist()) {
             $io->success('Database already exists. Run "db:migrate" command to make sure it is up to date.');
             return ExitCode::EXIT_SUCCESS;
         }
@@ -68,30 +67,9 @@ class CreateDatabaseCommand extends AbstractDatabaseCommand
         return ExitCode::EXIT_SUCCESS;
     }
 
-    private function checkDbExists(): void
+    private function databaseTablesExist(): bool
     {
-        if ($this->regularConn->getDriver()->getDatabasePlatform() instanceof SqlitePlatform) {
-            return;
-        }
-
-        // In order to create the new database, we have to use a connection where the dbname was not set.
-        // Otherwise, it will fail to connect and will not be able to create the new database
-        $schemaManager = $this->noDbNameConn->createSchemaManager();
-        $databases = $schemaManager->listDatabases();
-        // We cannot use getDatabase() to get the database name here, because then the driver will try to connect, and
-        // it does not exist yet. We need to read from the raw params instead.
-        $shlinkDatabase = $this->regularConn->getParams()['dbname'] ?? null;
-
-        if ($shlinkDatabase !== null && ! contains($databases, $shlinkDatabase)) {
-            $schemaManager->createDatabase($shlinkDatabase);
-        }
-    }
-
-    private function schemaExists(): bool
-    {
-        $schemaManager = $this->regularConn->createSchemaManager();
-        $existingTables = $schemaManager->listTableNames();
-
+        $existingTables = $this->ensureDatabaseExistsAndGetTables();
         $allMetadata = $this->em->getMetadataFactory()->getAllMetadata();
         $shlinkTables = map($allMetadata, static fn (ClassMetadata $metadata) => $metadata->getTableName());
 
@@ -99,4 +77,25 @@ class CreateDatabaseCommand extends AbstractDatabaseCommand
         // Any other inconsistency will be taken care of by the migrations.
         return some($shlinkTables, static fn (string $shlinkTable) => contains($existingTables, $shlinkTable));
     }
+
+    private function ensureDatabaseExistsAndGetTables(): array
+    {
+        if ($this->regularConn->getDriver()->getDatabasePlatform() instanceof SqlitePlatform) {
+            return [];
+        }
+
+        try {
+            // Trying to list tables requires opening a connection to configured database.
+            // If it fails, it means it does not exist yet.
+            return $this->regularConn->createSchemaManager()->listTableNames();
+        } catch (Throwable) {
+            // We cannot use getDatabase() to get the database name here, because then the driver will try to connect.
+            // Instead, we read from the raw params.
+            $shlinkDatabase = $this->regularConn->getParams()['dbname'] ?? '';
+            // Create the database using a connection where the dbname was not set.
+            $this->noDbNameConn->createSchemaManager()->createDatabase($shlinkDatabase);
+
+            return [];
+        }
+    }
 }

module/CLI/test/Command/Db/CreateDatabaseCommandTest.php

@@ -12,6 +12,7 @@ use Doctrine\DBAL\Schema\AbstractSchemaManager;
 use Doctrine\ORM\EntityManagerInterface;
 use Doctrine\ORM\Mapping\ClassMetadata;
 use Doctrine\Persistence\Mapping\ClassMetadataFactory;
+use Exception;
 use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\Attributes\Test;
 use PHPUnit\Framework\MockObject\MockObject;
@@ -69,17 +70,14 @@ class CreateDatabaseCommandTest extends TestCase
     #[Test]
     public function successMessageIsPrintedIfDatabaseAlreadyExists(): void
     {
-        $shlinkDatabase = 'shlink_database';
-        $this->regularConn->expects($this->once())->method('getParams')->willReturn(['dbname' => $shlinkDatabase]);
+        $this->regularConn->expects($this->never())->method('getParams');
+        $this->driver->method('getDatabasePlatform')->willReturn($this->createMock(AbstractPlatform::class));
+
         $metadataMock = $this->createMock(ClassMetadata::class);
         $metadataMock->expects($this->once())->method('getTableName')->willReturn('foo_table');
         $this->metadataFactory->method('getAllMetadata')->willReturn([$metadataMock]);
-        $this->schemaManager->expects($this->once())->method('listDatabases')->willReturn(
-            ['foo', $shlinkDatabase, 'bar'],
-        );
         $this->schemaManager->expects($this->never())->method('createDatabase');
         $this->schemaManager->expects($this->once())->method('listTableNames')->willReturn(['foo_table', 'bar_table']);
-        $this->driver->method('getDatabasePlatform')->willReturn($this->createMock(AbstractPlatform::class));
 
         $this->commandTester->execute([]);
         $output = $this->commandTester->getDisplay();
@@ -90,15 +88,13 @@ class CreateDatabaseCommandTest extends TestCase
     #[Test]
     public function databaseIsCreatedIfItDoesNotExist(): void
     {
+        $this->driver->method('getDatabasePlatform')->willReturn($this->createMock(AbstractPlatform::class));
+
         $shlinkDatabase = 'shlink_database';
         $this->regularConn->expects($this->once())->method('getParams')->willReturn(['dbname' => $shlinkDatabase]);
         $this->metadataFactory->method('getAllMetadata')->willReturn([]);
-        $this->schemaManager->expects($this->once())->method('listDatabases')->willReturn(['foo', 'bar']);
         $this->schemaManager->expects($this->once())->method('createDatabase')->with($shlinkDatabase);
-        $this->schemaManager->expects($this->once())->method('listTableNames')->willReturn(
-            ['foo_table', 'bar_table'],
-        );
-        $this->driver->method('getDatabasePlatform')->willReturn($this->createMock(AbstractPlatform::class));
+        $this->schemaManager->expects($this->once())->method('listTableNames')->willThrowException(new Exception(''));
 
         $this->commandTester->execute([]);
     }
@@ -106,14 +102,12 @@ class CreateDatabaseCommandTest extends TestCase
     #[Test, DataProvider('provideEmptyDatabase')]
     public function tablesAreCreatedIfDatabaseIsEmpty(array $tables): void
     {
-        $shlinkDatabase = 'shlink_database';
-        $this->regularConn->expects($this->once())->method('getParams')->willReturn(['dbname' => $shlinkDatabase]);
+        $this->regularConn->expects($this->never())->method('getParams');
+        $this->driver->method('getDatabasePlatform')->willReturn($this->createMock(AbstractPlatform::class));
+
         $metadata = $this->createMock(ClassMetadata::class);
         $metadata->method('getTableName')->willReturn('shlink_table');
         $this->metadataFactory->method('getAllMetadata')->willReturn([$metadata]);
-        $this->schemaManager->expects($this->once())->method('listDatabases')->willReturn(
-            ['foo', $shlinkDatabase, 'bar'],
-        );
         $this->schemaManager->expects($this->never())->method('createDatabase');
         $this->schemaManager->expects($this->once())->method('listTableNames')->willReturn($tables);
         $this->processHelper->expects($this->once())->method('run')->with($this->isInstanceOf(OutputInterface::class), [
@@ -122,7 +116,6 @@ class CreateDatabaseCommandTest extends TestCase
             CreateDatabaseCommand::DOCTRINE_CREATE_SCHEMA_COMMAND,
             '--no-interaction',
         ]);
-        $this->driver->method('getDatabasePlatform')->willReturn($this->createMock(AbstractPlatform::class));
 
         $this->commandTester->execute([]);
         $output = $this->commandTester->getDisplay();
@@ -141,12 +134,12 @@ class CreateDatabaseCommandTest extends TestCase
     public function databaseCheckIsSkippedForSqlite(): void
     {
         $this->driver->method('getDatabasePlatform')->willReturn($this->createMock(SqlitePlatform::class));
-
         $this->regularConn->expects($this->never())->method('getParams');
-        $this->metadataFactory->expects($this->once())->method('getAllMetadata')->willReturn([]);
         $this->schemaManager->expects($this->never())->method('listDatabases');
         $this->schemaManager->expects($this->never())->method('createDatabase');
-        $this->schemaManager->expects($this->once())->method('listTableNames')->willReturn(['foo_table', 'bar_table']);
+        $this->schemaManager->expects($this->never())->method('listTableNames');
+
+        $this->metadataFactory->expects($this->once())->method('getAllMetadata')->willReturn([]);
 
         $this->commandTester->execute([]);
     }