Merge pull request #1399 from shlinkio/feature/improve-db-create

Feature/improve db create

CHANGELOG.md

@@ -11,6 +11,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this
 ### Changed
 * [#1359](https://github.com/shlinkio/shlink/issues/1359) Hidden database commands.
 * [#1385](https://github.com/shlinkio/shlink/issues/1385) Prevented a big error message from being logged when using Shlink without mercure.
+* [#1398](https://github.com/shlinkio/shlink/issues/1398) Increased required mutation score for unit tests to 85%.
 
 ### Deprecated
 * [#1340](https://github.com/shlinkio/shlink/issues/1340) Deprecated webhooks. New events will only be added to other real-time updates approaches, and webhooks will be completely removed in Shlink 4.0.0.
@@ -19,7 +20,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this
 * *Nothing*
 
 ### Fixed
-* *Nothing*
+* [#1397](https://github.com/shlinkio/shlink/issues/1397) Fixed `db:create` command always reporting the schema exists if the `db:migrate` command has been run before by mistake.
 
 
 ## [3.0.3] - 2022-02-19

composer.json

@@ -64,7 +64,7 @@
         "cebe/php-openapi": "^1.5",
         "devster/ubench": "^2.1",
         "dms/phpunit-arraysubset-asserts": "^0.3.0",
-        "infection/infection": "^0.26",
+        "infection/infection": "^0.26.5",
         "openswoole/ide-helper": "~4.9.1",
         "phpspec/prophecy-phpunit": "^2.0",
         "phpstan/phpstan": "^1.2",
@@ -139,7 +139,7 @@
         "test:api": "bin/test/run-api-tests.sh",
         "test:api:ci": "GENERATE_COVERAGE=yes composer test:api",
         "infect:ci:base": "infection --threads=4 --log-verbosity=default --only-covered --only-covering-test-cases --skip-initial-tests",
-        "infect:ci:unit": "@infect:ci:base --coverage=build/coverage-unit --min-msi=80",
+        "infect:ci:unit": "@infect:ci:base --coverage=build/coverage-unit --min-msi=85",
         "infect:ci:db": "@infect:ci:base --coverage=build/coverage-db --min-msi=95 --configuration=infection-db.json",
         "infect:ci:api": "@infect:ci:base --coverage=build/coverage-api --min-msi=80 --configuration=infection-api.json",
         "infect:ci": "@parallel infect:ci:unit infect:ci:db infect:ci:api",

config/constants.php

@@ -19,3 +19,4 @@ 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';

migrations.php

@@ -2,13 +2,15 @@
 
 declare(strict_types=1);
 
+use const Shlinkio\Shlink\MIGRATIONS_TABLE;
+
 return [
 
     'migrations_paths' => [
         'ShlinkMigrations' => 'data/migrations',
     ],
     'table_storage' => [
-        'table_name' => 'migrations',
+        'table_name' => MIGRATIONS_TABLE,
     ],
     'custom_template' => 'data/migrations_template.txt',
 

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

@@ -5,6 +5,7 @@ declare(strict_types=1);
 namespace Shlinkio\Shlink\CLI\Command\Db;
 
 use Doctrine\DBAL\Connection;
+use Doctrine\DBAL\Platforms\SqlitePlatform;
 use Shlinkio\Shlink\CLI\Util\ExitCodes;
 use Shlinkio\Shlink\CLI\Util\ProcessRunnerInterface;
 use Symfony\Component\Console\Input\InputInterface;
@@ -14,6 +15,9 @@ use Symfony\Component\Lock\LockFactory;
 use Symfony\Component\Process\PhpExecutableFinder;
 
 use function Functional\contains;
+use function Functional\filter;
+
+use const Shlinkio\Shlink\MIGRATIONS_TABLE;
 
 class CreateDatabaseCommand extends AbstractDatabaseCommand
 {
@@ -62,7 +66,7 @@ class CreateDatabaseCommand extends AbstractDatabaseCommand
 
     private function checkDbExists(): void
     {
-        if ($this->regularConn->getDatabasePlatform()->getName() === 'sqlite') {
+        if ($this->regularConn->getDatabasePlatform() instanceof SqlitePlatform) {
             return;
         }
 
@@ -80,8 +84,9 @@ class CreateDatabaseCommand extends AbstractDatabaseCommand
     private function schemaExists(): bool
     {
         // If at least one of the shlink tables exist, we will consider the database exists somehow.
-        // Any inconsistency should be taken care by the migrations
+        // 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($schemaManager->listTableNames());
+        return ! empty(filter($schemaManager->listTableNames(), fn (string $table) => $table !== MIGRATIONS_TABLE));
     }
 }

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

@@ -6,6 +6,7 @@ namespace ShlinkioTest\Shlink\CLI\Command\Db;
 
 use Doctrine\DBAL\Connection;
 use Doctrine\DBAL\Platforms\AbstractPlatform;
+use Doctrine\DBAL\Platforms\SqlitePlatform;
 use Doctrine\DBAL\Schema\AbstractSchemaManager;
 use PHPUnit\Framework\TestCase;
 use Prophecy\Argument;
@@ -19,6 +20,8 @@ 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;
@@ -27,7 +30,6 @@ class CreateDatabaseCommandTest extends TestCase
     private ObjectProphecy $processHelper;
     private ObjectProphecy $regularConn;
     private ObjectProphecy $schemaManager;
-    private ObjectProphecy $databasePlatform;
 
     public function setUp(): void
     {
@@ -43,11 +45,10 @@ class CreateDatabaseCommandTest extends TestCase
 
         $this->processHelper = $this->prophesize(ProcessRunnerInterface::class);
         $this->schemaManager = $this->prophesize(AbstractSchemaManager::class);
-        $this->databasePlatform = $this->prophesize(AbstractPlatform::class);
 
         $this->regularConn = $this->prophesize(Connection::class);
         $this->regularConn->createSchemaManager()->willReturn($this->schemaManager->reveal());
-        $this->regularConn->getDatabasePlatform()->willReturn($this->databasePlatform->reveal());
+        $this->regularConn->getDatabasePlatform()->willReturn($this->prophesize(AbstractPlatform::class)->reveal());
         $noDbNameConn = $this->prophesize(Connection::class);
         $noDbNameConn->createSchemaManager()->willReturn($this->schemaManager->reveal());
 
@@ -90,7 +91,7 @@ class CreateDatabaseCommandTest extends TestCase
         $listDatabases = $this->schemaManager->listDatabases()->willReturn(['foo', 'bar']);
         $createDatabase = $this->schemaManager->createDatabase($shlinkDatabase)->will(function (): void {
         });
-        $listTables = $this->schemaManager->listTableNames()->willReturn(['foo_table', 'bar_table']);
+        $listTables = $this->schemaManager->listTableNames()->willReturn(['foo_table', 'bar_table', MIGRATIONS_TABLE]);
 
         $this->commandTester->execute([]);
 
@@ -100,15 +101,18 @@ class CreateDatabaseCommandTest extends TestCase
         $listTables->shouldHaveBeenCalledOnce();
     }
 
-    /** @test */
-    public function tablesAreCreatedIfDatabaseIsEmpty(): void
+    /**
+     * @test
+     * @dataProvider provideEmptyDatabase
+     */
+    public function tablesAreCreatedIfDatabaseIsEmpty(array $tables): void
     {
         $shlinkDatabase = 'shlink_database';
         $getDatabase = $this->regularConn->getDatabase()->willReturn($shlinkDatabase);
         $listDatabases = $this->schemaManager->listDatabases()->willReturn(['foo', $shlinkDatabase, 'bar']);
         $createDatabase = $this->schemaManager->createDatabase($shlinkDatabase)->will(function (): void {
         });
-        $listTables = $this->schemaManager->listTableNames()->willReturn([]);
+        $listTables = $this->schemaManager->listTableNames()->willReturn($tables);
         $runCommand = $this->processHelper->run(Argument::type(OutputInterface::class), [
             '/usr/local/bin/php',
             CreateDatabaseCommand::DOCTRINE_SCRIPT,
@@ -128,10 +132,16 @@ class CreateDatabaseCommandTest extends TestCase
         $runCommand->shouldHaveBeenCalledOnce();
     }
 
+    public function provideEmptyDatabase(): iterable
+    {
+        yield 'no tables' => [[]];
+        yield 'migrations table' => [[MIGRATIONS_TABLE]];
+    }
+
     /** @test */
     public function databaseCheckIsSkippedForSqlite(): void
     {
-        $this->databasePlatform->getName()->willReturn('sqlite');
+        $this->regularConn->getDatabasePlatform()->willReturn($this->prophesize(SqlitePlatform::class)->reveal());
 
         $shlinkDatabase = 'shlink_database';
         $getDatabase = $this->regularConn->getDatabase()->willReturn($shlinkDatabase);