From 4013ae87dd05f3e05e1bde4914d0634aaf7a84cb Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 1 Jun 2023 19:27:04 +0200 Subject: [PATCH] Change order to create initial database to avoid permission errors --- .../src/Command/Db/CreateDatabaseCommand.php | 51 +++++++++---------- .../Command/Db/CreateDatabaseCommandTest.php | 33 +++++------- 2 files changed, 38 insertions(+), 46 deletions(-) diff --git a/module/CLI/src/Command/Db/CreateDatabaseCommand.php b/module/CLI/src/Command/Db/CreateDatabaseCommand.php index f6df9b04..0fd1b18c 100644 --- a/module/CLI/src/Command/Db/CreateDatabaseCommand.php +++ b/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 []; + } + } } diff --git a/module/CLI/test/Command/Db/CreateDatabaseCommandTest.php b/module/CLI/test/Command/Db/CreateDatabaseCommandTest.php index 4b09ed7f..66f46db4 100644 --- a/module/CLI/test/Command/Db/CreateDatabaseCommandTest.php +++ b/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([]); }