Merge pull request #2552 from acelaya-forks/db-invokable-commands

Convert database commands into invokable commands

module/CLI/config/dependencies.config.php

@@ -32,6 +32,7 @@ return [
 
             RedirectRule\RedirectRuleHandler::class => InvokableFactory::class,
             Util\ProcessRunner::class => ConfigAbstractFactory::class,
+            Util\PhpProcessRunner::class => ConfigAbstractFactory::class,
 
             ApiKey\RoleResolver::class => ConfigAbstractFactory::class,
 
@@ -79,6 +80,7 @@ return [
 
     ConfigAbstractFactory::class => [
         Util\ProcessRunner::class => [SymfonyCli\Helper\ProcessHelper::class],
+        Util\PhpProcessRunner::class => [Util\ProcessRunner::class, PhpExecutableFinder::class],
         ApiKey\RoleResolver::class => [DomainService::class, UrlShortenerOptions::class],
 
         Command\ShortUrl\CreateShortUrlCommand::class => [
@@ -136,16 +138,11 @@ return [
 
         Command\Db\CreateDatabaseCommand::class => [
             LockFactory::class,
-            Util\ProcessRunner::class,
-            PhpExecutableFinder::class,
+            Util\PhpProcessRunner::class,
             'em',
             NoDbNameConnectionFactory::SERVICE_NAME,
         ],
-        Command\Db\MigrateDatabaseCommand::class => [
-            LockFactory::class,
-            Util\ProcessRunner::class,
-            PhpExecutableFinder::class,
-        ],
+        Command\Db\MigrateDatabaseCommand::class => [LockFactory::class, Util\PhpProcessRunner::class],
     ],
 
 ];

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

@@ -1,47 +0,0 @@
-<?php
-
-declare(strict_types=1);
-
-namespace Shlinkio\Shlink\CLI\Command\Db;
-
-use Shlinkio\Shlink\CLI\Command\Util\CommandUtils;
-use Shlinkio\Shlink\CLI\Command\Util\LockConfig;
-use Shlinkio\Shlink\CLI\Util\ProcessRunnerInterface;
-use Symfony\Component\Console\Command\Command;
-use Symfony\Component\Console\Input\InputInterface;
-use Symfony\Component\Console\Output\OutputInterface;
-use Symfony\Component\Console\Style\SymfonyStyle;
-use Symfony\Component\Lock\LockFactory;
-use Symfony\Component\Process\PhpExecutableFinder;
-
-abstract class AbstractDatabaseCommand extends Command
-{
-    private string $phpBinary;
-
-    public function __construct(
-        private readonly LockFactory $locker,
-        private readonly ProcessRunnerInterface $processRunner,
-        PhpExecutableFinder $phpFinder,
-    ) {
-        parent::__construct();
-        $this->phpBinary = $phpFinder->find(false) ?: 'php';
-    }
-
-    protected function runPhpCommand(OutputInterface $output, array $command): void
-    {
-        $command = [$this->phpBinary, ...$command, '--no-interaction'];
-        $this->processRunner->run($output, $command);
-    }
-
-    final protected function execute(InputInterface $input, OutputInterface $output): int
-    {
-        return CommandUtils::executeWithLock(
-            $this->locker,
-            LockConfig::blocking($this->getName() ?? static::class),
-            new SymfonyStyle($input, $output),
-            fn () => $this->lockedExecute($input, $output),
-        );
-    }
-
-    abstract protected function lockedExecute(InputInterface $input, OutputInterface $output): int;
-}

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

@@ -7,51 +7,54 @@ namespace Shlinkio\Shlink\CLI\Command\Db;
 use Doctrine\DBAL\Connection;
 use Doctrine\ORM\EntityManagerInterface;
 use Doctrine\ORM\Mapping\ClassMetadata;
+use Shlinkio\Shlink\CLI\Command\Util\CommandUtils;
+use Shlinkio\Shlink\CLI\Command\Util\LockConfig;
 use Shlinkio\Shlink\CLI\Util\ProcessRunnerInterface;
-use Symfony\Component\Console\Input\InputInterface;
-use Symfony\Component\Console\Output\OutputInterface;
+use Symfony\Component\Console\Attribute\AsCommand;
+use Symfony\Component\Console\Command\Command;
 use Symfony\Component\Console\Style\SymfonyStyle;
 use Symfony\Component\Lock\LockFactory;
-use Symfony\Component\Process\PhpExecutableFinder;
 use Throwable;
 
 use function array_map;
 use function Shlinkio\Shlink\Core\ArrayUtils\contains;
 use function Shlinkio\Shlink\Core\ArrayUtils\some;
 
-class CreateDatabaseCommand extends AbstractDatabaseCommand
+#[AsCommand(
+    name: CreateDatabaseCommand::NAME,
+    description: 'Creates the database needed for shlink to work. It will do nothing if the database already exists',
+    hidden: true,
+)]
+class CreateDatabaseCommand extends Command
 {
     private readonly Connection $regularConn;
 
     public const string NAME = 'db:create';
-    public const string DOCTRINE_SCRIPT = 'bin/doctrine';
-    public const string DOCTRINE_CREATE_SCHEMA_COMMAND = 'orm:schema-tool:create';
+    public const string SCRIPT = 'bin/doctrine';
+    public const string COMMAND = 'orm:schema-tool:create';
 
     public function __construct(
-        LockFactory $locker,
-        ProcessRunnerInterface $processRunner,
-        PhpExecutableFinder $phpFinder,
+        private readonly LockFactory $locker,
+        private readonly ProcessRunnerInterface $processRunner,
         private readonly EntityManagerInterface $em,
         private readonly Connection $noDbNameConn,
     ) {
         $this->regularConn = $this->em->getConnection();
-        parent::__construct($locker, $processRunner, $phpFinder);
+        parent::__construct();
     }
 
-    protected function configure(): void
+    public function __invoke(SymfonyStyle $io): int
     {
-        $this
-            ->setName(self::NAME)
-            ->setHidden()
-            ->setDescription(
-                'Creates the database needed for shlink to work. It will do nothing if the database already exists',
+        return CommandUtils::executeWithLock(
+            $this->locker,
+            LockConfig::blocking(self::NAME),
+            $io,
+            fn () => $this->executeCommand($io),
         );
     }
 
-    protected function lockedExecute(InputInterface $input, OutputInterface $output): int
+    private function executeCommand(SymfonyStyle $io): int
     {
-        $io = new SymfonyStyle($input, $output);
-
         if ($this->databaseTablesExist()) {
             $io->success('Database already exists. Run "db:migrate" command to make sure it is up to date.');
             return self::SUCCESS;
@@ -59,7 +62,7 @@ class CreateDatabaseCommand extends AbstractDatabaseCommand
 
         // Create database
         $io->writeln('<fg=blue>Creating database tables...</>');
-        $this->runPhpCommand($output, [self::DOCTRINE_SCRIPT, self::DOCTRINE_CREATE_SCHEMA_COMMAND]);
+        $this->processRunner->run($io, [self::SCRIPT, self::COMMAND, '--no-interaction']);
         $io->success('Database properly created!');
 
         return self::SUCCESS;

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

@@ -4,30 +4,46 @@ declare(strict_types=1);
 
 namespace Shlinkio\Shlink\CLI\Command\Db;
 
-use Symfony\Component\Console\Input\InputInterface;
-use Symfony\Component\Console\Output\OutputInterface;
+use Shlinkio\Shlink\CLI\Command\Util\CommandUtils;
+use Shlinkio\Shlink\CLI\Command\Util\LockConfig;
+use Shlinkio\Shlink\CLI\Util\ProcessRunnerInterface;
+use Symfony\Component\Console\Attribute\AsCommand;
+use Symfony\Component\Console\Command\Command;
 use Symfony\Component\Console\Style\SymfonyStyle;
+use Symfony\Component\Lock\LockFactory;
 
-class MigrateDatabaseCommand extends AbstractDatabaseCommand
+#[AsCommand(
+    name: MigrateDatabaseCommand::NAME,
+    description: 'Runs database migrations, which will ensure the shlink database is up to date',
+    hidden: true,
+)]
+class MigrateDatabaseCommand extends Command
 {
     public const string NAME = 'db:migrate';
-    public const string DOCTRINE_MIGRATIONS_SCRIPT = 'vendor/doctrine/migrations/bin/doctrine-migrations.php';
-    public const string DOCTRINE_MIGRATE_COMMAND = 'migrations:migrate';
+    public const string SCRIPT = 'vendor/doctrine/migrations/bin/doctrine-migrations.php';
+    public const string COMMAND = 'migrations:migrate';
 
-    protected function configure(): void
-    {
-        $this
-            ->setName(self::NAME)
-            ->setHidden()
-            ->setDescription('Runs database migrations, which will ensure the shlink database is up to date.');
+    public function __construct(
+        private readonly LockFactory $locker,
+        private readonly ProcessRunnerInterface $processRunner,
+    ) {
+        parent::__construct();
     }
 
-    protected function lockedExecute(InputInterface $input, OutputInterface $output): int
+    public function __invoke(SymfonyStyle $io): int
     {
-        $io = new SymfonyStyle($input, $output);
+        return CommandUtils::executeWithLock(
+            $this->locker,
+            LockConfig::blocking(self::NAME),
+            $io,
+            fn () => $this->executeCommand($io),
+        );
+    }
 
+    private function executeCommand(SymfonyStyle $io): int
+    {
         $io->writeln('<fg=blue>Migrating database...</>');
-        $this->runPhpCommand($output, [self::DOCTRINE_MIGRATIONS_SCRIPT, self::DOCTRINE_MIGRATE_COMMAND]);
+        $this->processRunner->run($io, [self::SCRIPT, self::COMMAND, '--no-interaction']);
         $io->success('Database properly migrated!');
 
         return self::SUCCESS;

module/CLI/src/Util/PhpProcessRunner.php

@@ -0,0 +1,26 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Shlinkio\Shlink\CLI\Util;
+
+use Symfony\Component\Console\Output\OutputInterface;
+use Symfony\Component\Process\PhpExecutableFinder;
+
+/**
+ * Wraps another process manager prefixing any command run with current PHP binary
+ */
+readonly class PhpProcessRunner implements ProcessRunnerInterface
+{
+    private string $phpBinary;
+
+    public function __construct(private ProcessRunnerInterface $wrappedProcessRunner, PhpExecutableFinder $phpFinder)
+    {
+        $this->phpBinary = $phpFinder->find(includeArgs: false) ?: 'php';
+    }
+
+    public function run(OutputInterface $output, array $cmd): void
+    {
+        $this->wrappedProcessRunner->run($output, [$this->phpBinary, ...$cmd]);
+    }
+}

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

@@ -25,7 +25,6 @@ use Symfony\Component\Console\Output\OutputInterface;
 use Symfony\Component\Console\Tester\CommandTester;
 use Symfony\Component\Lock\LockFactory;
 use Symfony\Component\Lock\SharedLockInterface;
-use Symfony\Component\Process\PhpExecutableFinder;
 
 class CreateDatabaseCommandTest extends TestCase
 {
@@ -44,9 +43,6 @@ class CreateDatabaseCommandTest extends TestCase
         $lock->method('acquire')->willReturn(true);
         $locker->method('createLock')->willReturn($lock);
 
-        $phpExecutableFinder = $this->createStub(PhpExecutableFinder::class);
-        $phpExecutableFinder->method('find')->willReturn('/usr/local/bin/php');
-
         $this->processHelper = $this->createMock(ProcessRunnerInterface::class);
         $this->schemaManager = $this->createMock(AbstractSchemaManager::class);
 
@@ -63,7 +59,7 @@ class CreateDatabaseCommandTest extends TestCase
         $noDbNameConn = $this->createStub(Connection::class);
         $noDbNameConn->method('createSchemaManager')->willReturn($this->schemaManager);
 
-        $command = new CreateDatabaseCommand($locker, $this->processHelper, $phpExecutableFinder, $em, $noDbNameConn);
+        $command = new CreateDatabaseCommand($locker, $this->processHelper, $em, $noDbNameConn);
         $this->commandTester = CliTestUtils::testerForCommand($command);
     }
 
@@ -112,9 +108,8 @@ class CreateDatabaseCommandTest extends TestCase
         $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), [
-            '/usr/local/bin/php',
-            CreateDatabaseCommand::DOCTRINE_SCRIPT,
-            CreateDatabaseCommand::DOCTRINE_CREATE_SCHEMA_COMMAND,
+            CreateDatabaseCommand::SCRIPT,
+            CreateDatabaseCommand::COMMAND,
             '--no-interaction',
         ]);
 

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

@@ -14,7 +14,6 @@ use Symfony\Component\Console\Output\OutputInterface;
 use Symfony\Component\Console\Tester\CommandTester;
 use Symfony\Component\Lock\LockFactory;
 use Symfony\Component\Lock\SharedLockInterface;
-use Symfony\Component\Process\PhpExecutableFinder;
 
 class MigrateDatabaseCommandTest extends TestCase
 {
@@ -28,12 +27,9 @@ class MigrateDatabaseCommandTest extends TestCase
         $lock->method('acquire')->willReturn(true);
         $locker->method('createLock')->willReturn($lock);
 
-        $phpExecutableFinder = $this->createStub(PhpExecutableFinder::class);
-        $phpExecutableFinder->method('find')->willReturn('/usr/local/bin/php');
-
         $this->processHelper = $this->createMock(ProcessRunnerInterface::class);
 
-        $command = new MigrateDatabaseCommand($locker, $this->processHelper, $phpExecutableFinder);
+        $command = new MigrateDatabaseCommand($locker, $this->processHelper);
         $this->commandTester = CliTestUtils::testerForCommand($command);
     }
 
@@ -41,9 +37,8 @@ class MigrateDatabaseCommandTest extends TestCase
     public function migrationsCommandIsRunWithProperVerbosity(): void
     {
         $this->processHelper->expects($this->once())->method('run')->with($this->isInstanceOf(OutputInterface::class), [
-            '/usr/local/bin/php',
-            MigrateDatabaseCommand::DOCTRINE_MIGRATIONS_SCRIPT,
-            MigrateDatabaseCommand::DOCTRINE_MIGRATE_COMMAND,
+            MigrateDatabaseCommand::SCRIPT,
+            MigrateDatabaseCommand::COMMAND,
             '--no-interaction',
         ]);
 

module/CLI/test/Util/PhpProcessRunnerTest.php

@@ -0,0 +1,40 @@
+<?php
+
+declare(strict_types=1);
+
+namespace ShlinkioTest\Shlink\CLI\Util;
+
+use PHPUnit\Framework\Attributes\Test;
+use PHPUnit\Framework\Attributes\TestWith;
+use PHPUnit\Framework\MockObject\MockObject;
+use PHPUnit\Framework\TestCase;
+use Shlinkio\Shlink\CLI\Util\PhpProcessRunner;
+use Shlinkio\Shlink\CLI\Util\ProcessRunnerInterface;
+use Symfony\Component\Console\Output\OutputInterface;
+use Symfony\Component\Process\PhpExecutableFinder;
+
+class PhpProcessRunnerTest extends TestCase
+{
+    private MockObject & ProcessRunnerInterface $wrapped;
+    private MockObject & PhpExecutableFinder $executableFinder;
+
+    protected function setUp(): void
+    {
+        $this->wrapped = $this->createMock(ProcessRunnerInterface::class);
+        $this->executableFinder = $this->createMock(PhpExecutableFinder::class);
+    }
+
+    #[Test]
+    #[TestWith([false, 'php'])]
+    #[TestWith(['/usr/local/bin/php', '/usr/local/bin/php'])]
+    public function commandsArePrefixedWithPhp(string|false $resolvedExecutable, string $expectedExecutable): void
+    {
+        $output = $this->createStub(OutputInterface::class);
+        $command = ['foo', 'bar', 'baz'];
+
+        $this->wrapped->expects($this->once())->method('run')->with($output, [$expectedExecutable, ...$command]);
+        $this->executableFinder->expects($this->once())->method('find')->with(false)->willReturn($resolvedExecutable);
+
+        new PhpProcessRunner($this->wrapped, $this->executableFinder)->run($output, $command);
+    }
+}