diff --git a/CHANGELOG.md b/CHANGELOG.md index 538f3c30..c49e4f0b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this ### Fixed * [#1698](https://github.com/shlinkio/shlink/issues/1698) Fixed error 500 in `robots.txt`. * [#1688](https://github.com/shlinkio/shlink/issues/1688) Fixed huge performance degradation on `/tags/stats` endpoint. +* [#1693](https://github.com/shlinkio/shlink/issues/1693) Fixed Shlink thinking database already exists if it finds foreign tables. ## [3.5.1] - 2023-02-04 diff --git a/config/autoload/entity-manager.local.php.dist b/config/autoload/entity-manager.local.php.dist index 4a0750a5..dadc49af 100644 --- a/config/autoload/entity-manager.local.php.dist +++ b/config/autoload/entity-manager.local.php.dist @@ -6,12 +6,32 @@ return [ 'entity_manager' => [ 'connection' => [ + // MySQL 'user' => 'root', 'password' => 'root', 'driver' => 'pdo_mysql', 'host' => 'shlink_db_mysql', 'dbname' => 'shlink', +// 'dbname' => 'shlink_foo', 'charset' => 'utf8mb4', + + // Postgres +// 'user' => 'postgres', +// 'password' => 'root', +// 'driver' => 'pdo_pgsql', +// 'host' => 'shlink_db_postgres', +// 'dbname' => 'shlink_foo', +// 'charset' => 'utf8', + + // MSSQL +// 'user' => 'sa', +// 'password' => 'Passw0rd!', +// 'driver' => 'pdo_sqlsrv', +// 'host' => 'shlink_db_ms', +// 'dbname' => 'shlink_foo', +// 'driverOptions' => [ +// 'TrustServerCertificate' => 'true', +// ], ], ], diff --git a/config/constants.php b/config/constants.php index 5c891a34..cc802301 100644 --- a/config/constants.php +++ b/config/constants.php @@ -19,4 +19,3 @@ const DEFAULT_QR_CODE_FORMAT = 'png'; const DEFAULT_QR_CODE_ERROR_CORRECTION = 'l'; const DEFAULT_QR_CODE_ROUND_BLOCK_SIZE = true; const MIN_TASK_WORKERS = 4; -const MIGRATIONS_TABLE = 'migrations'; diff --git a/config/test/bootstrap_cli_tests.php b/config/test/bootstrap_cli_tests.php index c8c33721..c193b404 100644 --- a/config/test/bootstrap_cli_tests.php +++ b/config/test/bootstrap_cli_tests.php @@ -23,10 +23,10 @@ if (file_exists($covFile)) { } $testHelper->createTestDb( - ['bin/cli', 'db:create'], - ['bin/cli', 'db:migrate'], - ['bin/doctrine', 'orm:schema-tool:drop'], - ['bin/doctrine', 'dbal:run-sql'], + createDbCommand: ['bin/cli', 'db:create'], + migrateDbCommand: ['bin/cli', 'db:migrate'], + dropSchemaCommand: ['bin/doctrine', 'orm:schema-tool:drop'], + runSqlCommand: ['bin/doctrine', 'dbal:run-sql'], ); CliTest\CliTestCase::setSeedFixturesCallback( static fn () => $testHelper->seedFixtures($em, $config['data_fixtures'] ?? []), diff --git a/migrations.php b/migrations.php index 78369f6a..306c1c08 100644 --- a/migrations.php +++ b/migrations.php @@ -2,15 +2,13 @@ declare(strict_types=1); -use const Shlinkio\Shlink\MIGRATIONS_TABLE; - return [ 'migrations_paths' => [ 'ShlinkMigrations' => 'data/migrations', ], 'table_storage' => [ - 'table_name' => MIGRATIONS_TABLE, + 'table_name' => 'migrations', ], 'custom_template' => 'data/migrations_template.txt', diff --git a/module/CLI/config/dependencies.config.php b/module/CLI/config/dependencies.config.php index 177e4c8b..e5176f42 100644 --- a/module/CLI/config/dependencies.config.php +++ b/module/CLI/config/dependencies.config.php @@ -4,7 +4,6 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI; -use Doctrine\DBAL\Connection; use GeoIp2\Database\Reader; use Laminas\ServiceManager\AbstractFactory\ConfigAbstractFactory; use Laminas\ServiceManager\Factory\InvokableFactory; @@ -116,7 +115,7 @@ return [ LockFactory::class, Util\ProcessRunner::class, PhpExecutableFinder::class, - Connection::class, + 'em', NoDbNameConnectionFactory::SERVICE_NAME, ], Command\Db\MigrateDatabaseCommand::class => [ diff --git a/module/CLI/src/Command/Db/CreateDatabaseCommand.php b/module/CLI/src/Command/Db/CreateDatabaseCommand.php index 5cc6a184..95b08da2 100644 --- a/module/CLI/src/Command/Db/CreateDatabaseCommand.php +++ b/module/CLI/src/Command/Db/CreateDatabaseCommand.php @@ -6,6 +6,8 @@ namespace Shlinkio\Shlink\CLI\Command\Db; use Doctrine\DBAL\Connection; use Doctrine\DBAL\Platforms\SqlitePlatform; +use Doctrine\ORM\EntityManagerInterface; +use Doctrine\ORM\Mapping\ClassMetadata; use Shlinkio\Shlink\CLI\Util\ExitCodes; use Shlinkio\Shlink\CLI\Util\ProcessRunnerInterface; use Symfony\Component\Console\Input\InputInterface; @@ -15,12 +17,13 @@ use Symfony\Component\Lock\LockFactory; use Symfony\Component\Process\PhpExecutableFinder; use function Functional\contains; -use function Functional\filter; - -use const Shlinkio\Shlink\MIGRATIONS_TABLE; +use function Functional\map; +use function Functional\some; class CreateDatabaseCommand extends AbstractDatabaseCommand { + private readonly Connection $regularConn; + public const NAME = 'db:create'; public const DOCTRINE_SCRIPT = 'bin/doctrine'; public const DOCTRINE_CREATE_SCHEMA_COMMAND = 'orm:schema-tool:create'; @@ -29,9 +32,10 @@ class CreateDatabaseCommand extends AbstractDatabaseCommand LockFactory $locker, ProcessRunnerInterface $processRunner, PhpExecutableFinder $phpFinder, - private Connection $regularConn, - private Connection $noDbNameConn, + private readonly EntityManagerInterface $em, + private readonly Connection $noDbNameConn, ) { + $this->regularConn = $this->em->getConnection(); parent::__construct($locker, $processRunner, $phpFinder); } @@ -74,6 +78,8 @@ class CreateDatabaseCommand extends AbstractDatabaseCommand // 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)) { @@ -83,10 +89,14 @@ class CreateDatabaseCommand extends AbstractDatabaseCommand private function schemaExists(): bool { - // If at least one of the shlink tables exist, we will consider the database exists somehow. - // We exclude the migrations table, in case db:migrate was run first by mistake. - // Any other inconsistency will be taken care by the migrations. $schemaManager = $this->regularConn->createSchemaManager(); - return ! empty(filter($schemaManager->listTableNames(), fn (string $table) => $table !== MIGRATIONS_TABLE)); + $existingTables = $schemaManager->listTableNames(); + + $allMetadata = $this->em->getMetadataFactory()->getAllMetadata(); + $shlinkTables = map($allMetadata, static fn (ClassMetadata $metadata) => $metadata->getTableName()); + + // If at least one of the shlink tables exist, we will consider the database exists somehow. + // Any other inconsistency will be taken care of by the migrations. + return some($shlinkTables, static fn (string $shlinkTable) => contains($existingTables, $shlinkTable)); } } diff --git a/module/CLI/test/Command/Db/CreateDatabaseCommandTest.php b/module/CLI/test/Command/Db/CreateDatabaseCommandTest.php index 9358184b..4b09ed7f 100644 --- a/module/CLI/test/Command/Db/CreateDatabaseCommandTest.php +++ b/module/CLI/test/Command/Db/CreateDatabaseCommandTest.php @@ -9,6 +9,9 @@ use Doctrine\DBAL\Driver; use Doctrine\DBAL\Platforms\AbstractPlatform; use Doctrine\DBAL\Platforms\SqlitePlatform; use Doctrine\DBAL\Schema\AbstractSchemaManager; +use Doctrine\ORM\EntityManagerInterface; +use Doctrine\ORM\Mapping\ClassMetadata; +use Doctrine\Persistence\Mapping\ClassMetadataFactory; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\MockObject\MockObject; @@ -22,8 +25,6 @@ use Symfony\Component\Lock\LockFactory; use Symfony\Component\Lock\LockInterface; use Symfony\Component\Process\PhpExecutableFinder; -use const Shlinkio\Shlink\MIGRATIONS_TABLE; - class CreateDatabaseCommandTest extends TestCase { use CliTestUtilsTrait; @@ -31,6 +32,7 @@ class CreateDatabaseCommandTest extends TestCase private CommandTester $commandTester; private MockObject & ProcessRunnerInterface $processHelper; private MockObject & Connection $regularConn; + private MockObject & ClassMetadataFactory $metadataFactory; private MockObject & AbstractSchemaManager $schemaManager; private MockObject & Driver $driver; @@ -51,17 +53,16 @@ class CreateDatabaseCommandTest extends TestCase $this->regularConn->method('createSchemaManager')->willReturn($this->schemaManager); $this->driver = $this->createMock(Driver::class); $this->regularConn->method('getDriver')->willReturn($this->driver); + + $this->metadataFactory = $this->createMock(ClassMetadataFactory::class); + $em = $this->createMock(EntityManagerInterface::class); + $em->method('getConnection')->willReturn($this->regularConn); + $em->method('getMetadataFactory')->willReturn($this->metadataFactory); + $noDbNameConn = $this->createMock(Connection::class); $noDbNameConn->method('createSchemaManager')->withAnyParameters()->willReturn($this->schemaManager); - $command = new CreateDatabaseCommand( - $locker, - $this->processHelper, - $phpExecutableFinder, - $this->regularConn, - $noDbNameConn, - ); - + $command = new CreateDatabaseCommand($locker, $this->processHelper, $phpExecutableFinder, $em, $noDbNameConn); $this->commandTester = $this->testerForCommand($command); } @@ -70,6 +71,9 @@ class CreateDatabaseCommandTest extends TestCase { $shlinkDatabase = 'shlink_database'; $this->regularConn->expects($this->once())->method('getParams')->willReturn(['dbname' => $shlinkDatabase]); + $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'], ); @@ -88,10 +92,11 @@ class CreateDatabaseCommandTest extends TestCase { $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', MIGRATIONS_TABLE], + ['foo_table', 'bar_table'], ); $this->driver->method('getDatabasePlatform')->willReturn($this->createMock(AbstractPlatform::class)); @@ -103,6 +108,9 @@ class CreateDatabaseCommandTest extends TestCase { $shlinkDatabase = 'shlink_database'; $this->regularConn->expects($this->once())->method('getParams')->willReturn(['dbname' => $shlinkDatabase]); + $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'], ); @@ -126,7 +134,7 @@ class CreateDatabaseCommandTest extends TestCase public static function provideEmptyDatabase(): iterable { yield 'no tables' => [[]]; - yield 'migrations table' => [[MIGRATIONS_TABLE]]; + yield 'migrations table' => [['non_shlink_table']]; } #[Test] @@ -135,6 +143,7 @@ class CreateDatabaseCommandTest extends TestCase $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']);