From 72381f98444a9b2d47890a7bca7d3b67da0d5f36 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 1 Jun 2023 19:27:04 +0200 Subject: [PATCH 1/4] 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([]); } From 7cff11080d1d41c4638b0980935b9a570ef31b0e Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 4 Jun 2023 08:57:07 +0200 Subject: [PATCH 2/4] Update changelog --- CHANGELOG.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ebc43101..91fd6070 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,23 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com), and this project adheres to [Semantic Versioning](https://semver.org). +## [3.6.1] - 2023-05-04 +### Added +* *Nothing* + +### Changed +* *Nothing* + +### Deprecated +* *Nothing* + +### Removed +* *Nothing* + +### Fixed +* [#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 ### Added * [#1148](https://github.com/shlinkio/shlink/issues/1148) Add support to delete short URL visits. From e50c21440f12b656be31ddd48f54dd935477e66f Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 4 Jun 2023 09:07:41 +0200 Subject: [PATCH 3/4] Define default values for env vars used in rr prod config --- CHANGELOG.md | 1 + config/roadrunner/.rr.yml | 6 +++--- docker/docker-entrypoint.sh | 8 -------- 3 files changed, 4 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 91fd6070..4df74b5a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this ### Fixed * [#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. +* [#1803](https://github.com/shlinkio/shlink/issues/1803) Fix default RoadRunner port when not using docker image. ## [3.6.0] - 2023-05-24 diff --git a/config/roadrunner/.rr.yml b/config/roadrunner/.rr.yml index 8d1344d7..b6783c28 100644 --- a/config/roadrunner/.rr.yml +++ b/config/roadrunner/.rr.yml @@ -7,18 +7,18 @@ server: command: 'php -dopcache.enable_cli=1 -dopcache.validate_timestamps=0 ../../bin/roadrunner-worker.php' http: - address: '0.0.0.0:${PORT}' + address: '0.0.0.0:${PORT:-8080}' middleware: ['static'] static: dir: '../../public' forbid: ['.php', '.htaccess'] pool: - num_workers: ${WEB_WORKER_NUM} + num_workers: ${WEB_WORKER_NUM:-0} jobs: timeout: 300 # 5 minutes pool: - num_workers: ${TASK_WORKER_NUM} + num_workers: ${TASK_WORKER_NUM:-0} consume: ['shlink'] pipelines: shlink: diff --git a/docker/docker-entrypoint.sh b/docker/docker-entrypoint.sh index a2daec3d..2058c44c 100644 --- a/docker/docker-entrypoint.sh +++ b/docker/docker-entrypoint.sh @@ -20,14 +20,6 @@ if [ "${ENABLE_PERIODIC_VISIT_LOCATE}" = "true" ] && [ "${SHLINK_USER_ID}" = "ro /usr/sbin/crond & fi -# RoadRunner config needs these to have been set, so falling back to default values if not set yet -if [ "$SHLINK_RUNTIME" == 'rr' ]; then - export PORT="${PORT:-"8080"}" - # Default to 0 so that RoadRunner decides the number of workers based on the amount of logical CPUs - export WEB_WORKER_NUM="${WEB_WORKER_NUM:-"0"}" - export TASK_WORKER_NUM="${TASK_WORKER_NUM:-"0"}" -fi - if [ "$SHLINK_RUNTIME" == 'openswoole' ]; then # When restarting the container, openswoole might think it is already in execution # This forces the app to be started every second until the exit code is 0 From 575e6bf707a1aba50a61cd6959db85b17ec32d70 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 4 Jun 2023 09:13:37 +0200 Subject: [PATCH 4/4] Downgrade PHPUnit to avoid infection error --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index a3494086..eb8aac8e 100644 --- a/composer.json +++ b/composer.json @@ -72,7 +72,7 @@ "phpstan/phpstan-phpunit": "^1.3", "phpstan/phpstan-symfony": "^1.2", "phpunit/php-code-coverage": "^10.0", - "phpunit/phpunit": "^10.1", + "phpunit/phpunit": "~10.1.0", "roave/security-advisories": "dev-master", "shlinkio/php-coding-standard": "~2.3.0", "shlinkio/shlink-test-utils": "^3.6",