diff --git a/app/config/config_test.yml b/app/config/config_test.yml index d3abd4319..3ad233351 100644 --- a/app/config/config_test.yml +++ b/app/config/config_test.yml @@ -5,6 +5,7 @@ imports: parameters: fosuser_registration: true + wallabag_dbname_suffix: '_test' framework: test: ~ @@ -23,7 +24,7 @@ web_profiler: doctrine: dbal: - dbname_suffix: '_test' # for MySQL and PostgreSQL + dbname_suffix: '%wallabag_dbname_suffix%' # for MySQL and PostgreSQL use_savepoints: true orm: diff --git a/app/config/parameters_test.yml b/app/config/parameters_test.yml index 307d33e7a..e9a3b6c80 100644 --- a/app/config/parameters_test.yml +++ b/app/config/parameters_test.yml @@ -1,2 +1,2 @@ parameters: - database_path: "%kernel.project_dir%/data/db/wallabag_test.sqlite" + database_path: "%kernel.project_dir%/data/db/wallabag%wallabag_dbname_suffix%.sqlite" diff --git a/app/config/services.yml b/app/config/services.yml index d29ef44b2..2618bd8c0 100644 --- a/app/config/services.yml +++ b/app/config/services.yml @@ -109,6 +109,9 @@ services: Doctrine\ORM\EntityManagerInterface: alias: doctrine.orm.entity_manager + Doctrine\Migrations\Metadata\Storage\TableMetadataStorageConfiguration: + alias: doctrine.migrations.storage.table_storage + Doctrine\Persistence\ManagerRegistry: alias: doctrine @@ -260,7 +263,6 @@ services: Wallabag\Command\InstallCommand: arguments: $databaseDriver: '%database_driver%' - $databaseName: '%database_name%' $defaultSettings: '%wallabag.default_internal_settings%' $defaultIgnoreOriginInstanceRules: '%wallabag.default_ignore_origin_instance_rules%' diff --git a/app/config/tests/parameters_test.sqlite.yml b/app/config/tests/parameters_test.sqlite.yml index c168aefa6..3082dffed 100644 --- a/app/config/tests/parameters_test.sqlite.yml +++ b/app/config/tests/parameters_test.sqlite.yml @@ -1,2 +1,2 @@ parameters: - env(DATABASE_URL): sqlite:///%kernel.project_dir%/data/db/wallabag_test.sqlite?charset=utf8 + env(DATABASE_URL): sqlite:///%kernel.project_dir%/data/db/wallabag%wallabag_dbname_suffix%.sqlite?charset=utf8 diff --git a/src/Command/InstallCommand.php b/src/Command/InstallCommand.php index cbfcad442..8eb2f4c93 100644 --- a/src/Command/InstallCommand.php +++ b/src/Command/InstallCommand.php @@ -7,6 +7,7 @@ use Doctrine\DBAL\Exception\DriverException; use Doctrine\DBAL\Platforms\MySQLPlatform; use Doctrine\DBAL\Platforms\PostgreSQLPlatform; use Doctrine\DBAL\Platforms\SqlitePlatform; +use Doctrine\Migrations\Metadata\Storage\TableMetadataStorageConfiguration; use Doctrine\ORM\EntityManagerInterface; use FOS\UserBundle\Event\UserEvent; use FOS\UserBundle\FOSUserEvents; @@ -35,34 +36,28 @@ class InstallCommand extends Command 'curl_exec', 'curl_multi_init', ]; - private bool $runOtherCommands = true; private EntityManagerInterface $entityManager; private EventDispatcherInterface $dispatcher; private UserManagerInterface $userManager; + private TableMetadataStorageConfiguration $tableMetadataStorageConfiguration; private string $databaseDriver; - private string $databaseName; private array $defaultSettings; private array $defaultIgnoreOriginInstanceRules; - public function __construct(EntityManagerInterface $entityManager, EventDispatcherInterface $dispatcher, UserManagerInterface $userManager, string $databaseDriver, string $databaseName, array $defaultSettings, array $defaultIgnoreOriginInstanceRules) + public function __construct(EntityManagerInterface $entityManager, EventDispatcherInterface $dispatcher, UserManagerInterface $userManager, TableMetadataStorageConfiguration $tableMetadataStorageConfiguration, string $databaseDriver, array $defaultSettings, array $defaultIgnoreOriginInstanceRules) { $this->entityManager = $entityManager; $this->dispatcher = $dispatcher; $this->userManager = $userManager; + $this->tableMetadataStorageConfiguration = $tableMetadataStorageConfiguration; $this->databaseDriver = $databaseDriver; - $this->databaseName = $databaseName; $this->defaultSettings = $defaultSettings; $this->defaultIgnoreOriginInstanceRules = $defaultIgnoreOriginInstanceRules; parent::__construct(); } - public function disableRunOtherCommands(): void - { - $this->runOtherCommands = false; - } - protected function configure() { $this @@ -127,7 +122,7 @@ class InstallCommand extends Command $conn->connect(); } catch (\Exception $e) { if (!str_contains($e->getMessage(), 'Unknown database') - && !str_contains($e->getMessage(), 'database "' . $this->databaseName . '" does not exist')) { + && !str_contains($e->getMessage(), 'database "' . $conn->getDatabase() . '" does not exist')) { $fulfilled = false; $status = 'ERROR!'; $help = 'Can\'t connect to the database: ' . $e->getMessage(); @@ -198,13 +193,21 @@ class InstallCommand extends Command { $this->io->section('Step 2 of 4: Setting up database.'); + $conn = $this->entityManager->getConnection(); + $databasePlatform = $conn->isConnected() ? $conn->getDatabasePlatform() : null; + // user want to reset everything? Don't care about what is already here if (true === $this->defaultInput->getOption('reset')) { $this->io->text('Dropping database, creating database and schema, clearing the cache'); + $this->runCommand('doctrine:schema:drop', ['--force' => true, '--full-database' => true]); + + if (!$databasePlatform instanceof PostgreSQLPlatform) { + $this->runCommand('doctrine:database:drop', ['--force' => true]); + $this->runCommand('doctrine:database:create'); + } + $this - ->runCommand('doctrine:database:drop', ['--force' => true]) - ->runCommand('doctrine:database:create') ->runCommand('doctrine:migrations:migrate', ['--no-interaction' => true]) ->runCommand('cache:clear') ; @@ -231,19 +234,20 @@ class InstallCommand extends Command if ($this->io->confirm('It appears that your database already exists. Would you like to reset it?', false)) { $this->io->text('Dropping database, creating database and schema...'); - $this - ->runCommand('doctrine:database:drop', ['--force' => true]) - ->runCommand('doctrine:database:create') - ->runCommand('doctrine:migrations:migrate', ['--no-interaction' => true]) - ; + $this->runCommand('doctrine:schema:drop', ['--force' => true, '--full-database' => true]); + + if (!$databasePlatform instanceof PostgreSQLPlatform) { + $this->runCommand('doctrine:database:drop', ['--force' => true]); + $this->runCommand('doctrine:database:create'); + } + + $this->runCommand('doctrine:migrations:migrate', ['--no-interaction' => true]); } elseif ($this->isSchemaPresent()) { if ($this->io->confirm('Seems like your database contains schema. Do you want to reset it?', false)) { $this->io->text('Dropping schema and creating schema...'); - $this - ->runCommand('doctrine:schema:drop', ['--force' => true]) - ->runCommand('doctrine:migrations:migrate', ['--no-interaction' => true]) - ; + $this->dropWallabagSchemaOnly(); + $this->runCommand('doctrine:migrations:migrate', ['--no-interaction' => true]); } } else { $this->io->text('Creating schema...'); @@ -333,10 +337,6 @@ class InstallCommand extends Command */ private function runCommand($command, $parameters = []) { - if (!$this->runOtherCommands) { - return $this; - } - $parameters = array_merge( ['command' => $command], $parameters, @@ -376,7 +376,13 @@ class InstallCommand extends Command private function isDatabasePresent() { $connection = $this->entityManager->getConnection(); - $databaseName = $connection->getDatabase(); + $params = $connection->getParams(); + + if ($connection->getDatabasePlatform() instanceof SqlitePlatform) { + $databaseName = $params['path']; + } else { + $databaseName = $params['dbname']; + } try { $schemaManager = $connection->createSchemaManager(); @@ -396,8 +402,6 @@ class InstallCommand extends Command // custom verification for sqlite, since `getListDatabasesSQL` doesn't work for sqlite if ($connection->getDatabasePlatform() instanceof SqlitePlatform) { - $params = $connection->getParams(); - if (isset($params['path']) && file_exists($params['path'])) { return true; } @@ -416,14 +420,21 @@ class InstallCommand extends Command /** * Check if the schema is already created. - * If we found at least one table, it means the schema exists. - * - * @return bool + * We use the Doctrine Migrations table for the check. */ - private function isSchemaPresent() + private function isSchemaPresent(): bool { $schemaManager = $this->entityManager->getConnection()->createSchemaManager(); - return \count($schemaManager->listTableNames()) > 0 ? true : false; + return $schemaManager->tablesExist([$this->tableMetadataStorageConfiguration->getTableName()]); + } + + private function dropWallabagSchemaOnly(): void + { + $this->runCommand('doctrine:schema:drop', ['--force' => true]); + + $connection = $this->entityManager->getConnection(); + $databasePlatform = $connection->getDatabasePlatform(); + $connection->executeQuery('DROP TABLE ' . $databasePlatform->quoteIdentifier($this->tableMetadataStorageConfiguration->getTableName()) . ';'); } } diff --git a/tests/Command/InstallCommandTest.php b/tests/Command/InstallCommandTest.php index 4f4f3c790..9e89e30c1 100644 --- a/tests/Command/InstallCommandTest.php +++ b/tests/Command/InstallCommandTest.php @@ -8,6 +8,7 @@ use Doctrine\DBAL\Platforms\MySQLPlatform; use Doctrine\DBAL\Platforms\PostgreSQLPlatform; use Doctrine\DBAL\Platforms\SqlitePlatform; use Doctrine\Persistence\ManagerRegistry; +use GuzzleHttp\Psr7\Uri; use Symfony\Bundle\FrameworkBundle\Console\Application; use Symfony\Component\Console\Command\LazyCommand; use Symfony\Component\Console\Input\ArrayInput; @@ -36,50 +37,57 @@ class InstallCommandTest extends WallabagTestCase /** @var Connection $connection */ $connection = $this->getTestClient()->getContainer()->get(ManagerRegistry::class)->getConnection(); - if ($connection->getDatabasePlatform() instanceof PostgreSQLPlatform) { - /* - * LOG: statement: CREATE DATABASE "wallabag" - * ERROR: source database "template1" is being accessed by other users - * DETAIL: There is 1 other session using the database. - * STATEMENT: CREATE DATABASE "wallabag" - * FATAL: database "wallabag" does not exist - * - * http://stackoverflow.com/a/14374832/569101 - */ - $this->markTestSkipped('PostgreSQL spotted: can\'t find a good way to drop current database, skipping.'); - } + + $originalDatabaseUrl = $this->getTestClient()->getContainer()->getParameter('env(DATABASE_URL)'); + $dbnameSuffix = $this->getTestClient()->getContainer()->getParameter('wallabag_dbname_suffix'); + $tmpDatabaseName = 'wallabag_' . bin2hex(random_bytes(5)); if ($connection->getDatabasePlatform() instanceof SqlitePlatform) { - // Environnement variable useful only for sqlite to avoid the error "attempt to write a readonly database" - // We can't define always this environnement variable because pdo_mysql seems to use it - // and we have the error: - // SQLSTATE[42000]: Syntax error or access violation: 1064 You have an error in your SQL syntax; - // check the manual that corresponds to your MariaDB server version for the right syntax to use - // near '/tmp/wallabag_testTYj1kp' at line 1 - $databasePath = tempnam(sys_get_temp_dir(), 'wallabag_test'); - putenv("DATABASE_URL=sqlite:///$databasePath?charset=utf8"); - - // The environnement has been changed, recreate the client in order to update connection - parent::setUp(); + $tmpDatabaseUrl = str_replace('wallabag' . $dbnameSuffix . '.sqlite', $tmpDatabaseName . $dbnameSuffix . '.sqlite', $originalDatabaseUrl); + } else { + $tmpDatabaseUrl = (string) (new Uri($originalDatabaseUrl))->withPath($tmpDatabaseName); } - $this->resetDatabase($this->getTestClient()); + putenv("DATABASE_URL=$tmpDatabaseUrl"); + + if ($connection->getDatabasePlatform() instanceof PostgreSQLPlatform) { + // PostgreSQL requires that the database exists before connecting to it + $tmpTestDatabaseName = $tmpDatabaseName . $dbnameSuffix; + $connection->executeQuery('CREATE DATABASE ' . $tmpTestDatabaseName); + } + + // The environnement has been changed, recreate the client in order to update connection + $this->getNewClient(); } protected function tearDown(): void { $databaseUrl = getenv('DATABASE_URL'); - $databasePath = parse_url($databaseUrl, \PHP_URL_PATH); - // Remove the real environnement variable - putenv('DATABASE_URL'); - if ($databasePath && file_exists($databasePath)) { - unlink($databasePath); + /** @var Connection $connection */ + $connection = $this->getTestClient()->getContainer()->get(ManagerRegistry::class)->getConnection(); + + if ($connection->getDatabasePlatform() instanceof SqlitePlatform) { + // Remove the real environnement variable + putenv('DATABASE_URL'); + + $databasePath = parse_url($databaseUrl, \PHP_URL_PATH); + + if (file_exists($databasePath)) { + unlink($databasePath); + } } else { + $testDatabaseName = $connection->getDatabase(); + $connection->close(); + + // Remove the real environnement variable + putenv('DATABASE_URL'); + // Create a new client to avoid the error: // Transaction commit failed because the transaction has been marked for rollback only. - $client = $this->getNewClient(); - $this->resetDatabase($client); + $this->getNewClient(); + + $this->getTestClient()->getContainer()->get(ManagerRegistry::class)->getConnection()->executeQuery('DROP DATABASE ' . $testDatabaseName); } parent::tearDown(); @@ -87,12 +95,9 @@ class InstallCommandTest extends WallabagTestCase public function testRunInstallCommand() { - $command = $this->getCommand(); + $this->setupDatabase(); - // enable calling other commands for MySQL only because rollback isn't supported - if (!$this->getTestClient()->getContainer()->get(ManagerRegistry::class)->getConnection()->getDatabasePlatform() instanceof MySQLPlatform) { - $command->disableRunOtherCommands(); - } + $command = $this->getCommand(); $tester = new CommandTester($command); $tester->setInputs([ @@ -112,12 +117,9 @@ class InstallCommandTest extends WallabagTestCase public function testRunInstallCommandWithReset() { - if ($this->getTestClient()->getContainer()->get(ManagerRegistry::class)->getConnection()->getDatabasePlatform() instanceof MySQLPlatform) { - $this->markTestSkipped('Rollback are not properly handled for MySQL, skipping.'); - } + $this->setupDatabase(); $command = $this->getCommand(); - $command->disableRunOtherCommands(); $tester = new CommandTester($command); $tester->setInputs([ @@ -140,10 +142,10 @@ class InstallCommandTest extends WallabagTestCase $this->assertStringContainsString('Dropping database, creating database and schema, clearing the cache', $tester->getDisplay()); } - public function testRunInstallCommandWithDatabaseRemoved() + public function testRunInstallCommandWithNonExistingDatabase() { - if ($this->getTestClient()->getContainer()->get(ManagerRegistry::class)->getConnection()->getDatabasePlatform() instanceof MySQLPlatform) { - $this->markTestSkipped('Rollback are not properly handled for MySQL, skipping.'); + if ($this->getTestClient()->getContainer()->get(ManagerRegistry::class)->getConnection()->getDatabasePlatform() instanceof PostgreSQLPlatform) { + $this->markTestSkipped('PostgreSQL spotted: PostgreSQL requires that the database exists before connecting to it, skipping.'); } // skipped SQLite check when database is removed because while testing for the connection, @@ -154,15 +156,6 @@ class InstallCommandTest extends WallabagTestCase $application = new Application($this->getTestClient()->getKernel()); - // drop database first, so the install command won't ask to reset things - $command = $application->find('doctrine:database:drop'); - $command->run(new ArrayInput([ - '--force' => true, - ]), new NullOutput()); - - // start a new application to avoid lagging connexion to pgsql - $this->getNewClient(); - $command = $this->getCommand(); $tester = new CommandTester($command); @@ -185,12 +178,9 @@ class InstallCommandTest extends WallabagTestCase public function testRunInstallCommandChooseResetSchema() { - if ($this->getTestClient()->getContainer()->get(ManagerRegistry::class)->getConnection()->getDatabasePlatform() instanceof MySQLPlatform) { - $this->markTestSkipped('Rollback are not properly handled for MySQL, skipping.'); - } + $this->setupDatabase(); $command = $this->getCommand(); - $command->disableRunOtherCommands(); $tester = new CommandTester($command); $tester->setInputs([ @@ -210,29 +200,8 @@ class InstallCommandTest extends WallabagTestCase public function testRunInstallCommandChooseNothing() { - /* - * [PHPUnit\Framework\Error\Warning (2)] - * filemtime(): stat failed for /home/runner/work/wallabag/wallabag/var/cache/tes_/ContainerNVNxA24/appAppKernelTestDebugContainer.php - * - * I don't know from where the "/tes_/" come from, it should be "/test/" instead ... - */ - if ($this->getTestClient()->getContainer()->get(ManagerRegistry::class)->getConnection()->getDatabasePlatform() instanceof MySQLPlatform) { - $this->markTestSkipped('That test is failing when using MySQL when clearing the cache (see code comment)'); - } - $application = new Application($this->getTestClient()->getKernel()); - // drop database first, so the install command won't ask to reset things - $command = $application->find('doctrine:database:drop'); - $command->run(new ArrayInput([ - '--force' => true, - ]), new NullOutput()); - - $this->getTestClient()->getContainer()->get(ManagerRegistry::class)->getConnection()->close(); - - $command = $application->find('doctrine:database:create'); - $command->run(new ArrayInput([]), new NullOutput()); - $command = $this->getCommand(); $tester = new CommandTester($command); @@ -247,17 +216,24 @@ class InstallCommandTest extends WallabagTestCase $this->assertStringContainsString('Administration setup.', $tester->getDisplay()); $this->assertStringContainsString('Config setup.', $tester->getDisplay()); - $this->assertStringContainsString('Creating schema', $tester->getDisplay()); + $databasePlatform = $this->getTestClient()->getContainer()->get(ManagerRegistry::class)->getConnection()->getDatabasePlatform(); + + if ($databasePlatform instanceof SqlitePlatform || $databasePlatform instanceof PostgreSQLPlatform) { + // SQLite and PostgreSQL always have the database created, so we create the schema only + $this->assertStringContainsString('Creating schema', $tester->getDisplay()); + } + + if ($databasePlatform instanceof MySQLPlatform) { + // MySQL can start with a non-existing database, so we create both the database and the schema + $this->assertStringContainsString('Creating database and schema', $tester->getDisplay()); + } } public function testRunInstallCommandNoInteraction() { - if ($this->getTestClient()->getContainer()->get(ManagerRegistry::class)->getConnection()->getDatabasePlatform() instanceof MySQLPlatform) { - $this->markTestSkipped('Rollback are not properly handled for MySQL, skipping.'); - } + $this->setupDatabase(); $command = $this->getCommand(); - $command->disableRunOtherCommands(); $tester = new CommandTester($command); $tester->execute([], [ @@ -284,4 +260,36 @@ class InstallCommandTest extends WallabagTestCase return $command; } + + private function setupDatabase() + { + $application = new Application($this->getTestClient()->getKernel()); + $application->setAutoExit(false); + + $application->run(new ArrayInput([ + 'command' => 'doctrine:database:create', + '--no-interaction' => true, + '--env' => 'test', + ]), new NullOutput()); + + $application->run(new ArrayInput([ + 'command' => 'doctrine:migrations:migrate', + '--no-interaction' => true, + '--env' => 'test', + ]), new NullOutput()); + + $application->run(new ArrayInput([ + 'command' => 'doctrine:fixtures:load', + '--no-interaction' => true, + '--env' => 'test', + ]), new NullOutput()); + + /* + * Recreate client to avoid error: + * + * [Doctrine\DBAL\ConnectionException] + * Transaction commit failed because the transaction has been marked for rollback only. + */ + $this->getNewClient(); + } } diff --git a/tests/WallabagTestCase.php b/tests/WallabagTestCase.php index c2df2ce35..8f70a115e 100644 --- a/tests/WallabagTestCase.php +++ b/tests/WallabagTestCase.php @@ -3,11 +3,8 @@ namespace Tests\Wallabag; use Doctrine\ORM\EntityManagerInterface; -use Symfony\Bundle\FrameworkBundle\Console\Application; use Symfony\Bundle\FrameworkBundle\KernelBrowser; use Symfony\Bundle\FrameworkBundle\Test\WebTestCase; -use Symfony\Component\Console\Input\ArrayInput; -use Symfony\Component\Console\Output\NullOutput; use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface; use Wallabag\Entity\User; @@ -39,40 +36,6 @@ abstract class WallabagTestCase extends WebTestCase return $this->client; } - public function resetDatabase(KernelBrowser $client) - { - $application = new Application($client->getKernel()); - $application->setAutoExit(false); - - $application->run(new ArrayInput([ - 'command' => 'doctrine:schema:drop', - '--no-interaction' => true, - '--force' => true, - '--full-database' => true, - '--env' => 'test', - ]), new NullOutput()); - - $application->run(new ArrayInput([ - 'command' => 'doctrine:migrations:migrate', - '--no-interaction' => true, - '--env' => 'test', - ]), new NullOutput()); - - $application->run(new ArrayInput([ - 'command' => 'doctrine:fixtures:load', - '--no-interaction' => true, - '--env' => 'test', - ]), new NullOutput()); - - /* - * Recreate client to avoid error: - * - * [Doctrine\DBAL\ConnectionException] - * Transaction commit failed because the transaction has been marked for rollback only. - */ - $this->client = $this->getNewClient(); - } - public function getEntityManager() { return $this->client->getContainer()->get(EntityManagerInterface::class);