Resolved code review feedback

DumitracheAdrian · May 4, 2024 · b8e3518 · b8e3518
1 parent 94bffd2
commit b8e3518
Show file tree

Hide file tree

Showing 13 changed files with 297 additions and 106 deletions.
diff --git a/app/src/Message/FailedMessage.php b/app/src/Message/FailedMessage.php
@@ -0,0 +1,9 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Message;
+
+class FailedMessage
+{
+}
diff --git a/config/packages/messenger.yaml b/config/packages/messenger.yaml
@@ -19,5 +19,6 @@ framework:
       Draw\Component\CronJob\Message\ExecuteCronJobMessage: 'async_high_priority'
       App\Message\NewTestDocumentMessage: ['sync', 'async']
       Draw\Component\Messenger\ManualTrigger\Message\ManuallyTriggeredInterface: 'async'
+      Draw\Component\Messenger\Message\RetryFailedMessageMessage: 'async'
       Draw\Bundle\UserBundle\Message\NewUserLockMessage: 'sync'
       Draw\Bundle\UserBundle\Message\UserLockDelayedActivationMessage: 'sync'
diff --git a/...framework-extra-bundle/DependencyInjection/Compiler/ConsolePathAwareCompilerPassTrait.php b/...framework-extra-bundle/DependencyInjection/Compiler/ConsolePathAwareCompilerPassTrait.php
@@ -0,0 +1,30 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Draw\Bundle\FrameworkExtraBundle\DependencyInjection\Compiler;
+
+use Symfony\Component\DependencyInjection\ContainerBuilder;
+use Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException;
+
+trait ConsolePathAwareCompilerPassTrait
+{
+    private function setConsolePathArgument(ContainerBuilder $container, string $definitionId): void
+    {
+        try {
+            $definition = $container->findDefinition($definitionId);
+        } catch (ServiceNotFoundException) {
+            return;
+        }
+
+        if (null === $symfonyConsolePath = $container->getParameter('draw.symfony_console_path')) {
+            $symfonyConsolePath = $container->getParameterBag()->resolveValue('%kernel.project_dir%/bin/console');
+        }
+
+        if (false === realpath($symfonyConsolePath)) {
+            throw new \RuntimeException('The draw_framework_extra.symfony_console_path value ['.$symfonyConsolePath.'] is invalid');
+        }
+
+        $definition->setArgument('$consolePath', realpath($symfonyConsolePath));
+    }
+}
diff --git a/packages/framework-extra-bundle/DependencyInjection/Compiler/MessengerBrokerCompilerPass.php b/packages/framework-extra-bundle/DependencyInjection/Compiler/MessengerBrokerCompilerPass.php
@@ -5,26 +5,13 @@
 use Draw\Component\Messenger\Broker\Command\StartMessengerBrokerCommand;
 use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
-use Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException;
 
 class MessengerBrokerCompilerPass implements CompilerPassInterface
 {
+    use ConsolePathAwareCompilerPassTrait;
+
     public function process(ContainerBuilder $container): void
     {
-        try {
-            $definition = $container->findDefinition(StartMessengerBrokerCommand::class);
-        } catch (ServiceNotFoundException) {
-            return;
-        }
-
-        if (null === $symfonyConsolePath = $container->getParameter('draw.symfony_console_path')) {
-            $symfonyConsolePath = $container->getParameterBag()->resolveValue('%kernel.project_dir%/bin/console');
-        }
-
-        if (false === realpath($symfonyConsolePath)) {
-            throw new \RuntimeException('The draw_framework_extra.symfony_console_path value ['.$symfonyConsolePath.'] is invalid');
-        }
-
-        $definition->setArgument('$consolePath', realpath($symfonyConsolePath));
+        $this->setConsolePathArgument($container, StartMessengerBrokerCommand::class);
     }
 }
diff --git a/...le/DependencyInjection/Compiler/MessengerRetryFailedMessageMessageHandlerCompilerPass.php b/...le/DependencyInjection/Compiler/MessengerRetryFailedMessageMessageHandlerCompilerPass.php
@@ -0,0 +1,19 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Draw\Bundle\FrameworkExtraBundle\DependencyInjection\Compiler;
+
+use Draw\Component\Messenger\MessageHandler\RetryFailedMessageMessageHandler;
+use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
+use Symfony\Component\DependencyInjection\ContainerBuilder;
+
+class MessengerRetryFailedMessageMessageHandlerCompilerPass implements CompilerPassInterface
+{
+    use ConsolePathAwareCompilerPassTrait;
+
+    public function process(ContainerBuilder $container): void
+    {
+        $this->setConsolePathArgument($container, RetryFailedMessageMessageHandler::class);
+    }
+}
diff --git a/packages/framework-extra-bundle/DrawFrameworkExtraBundle.php b/packages/framework-extra-bundle/DrawFrameworkExtraBundle.php
@@ -9,6 +9,7 @@
 use Draw\Bundle\FrameworkExtraBundle\DependencyInjection\Compiler\JmsDoctrineObjectConstructionCompilerPass;
 use Draw\Bundle\FrameworkExtraBundle\DependencyInjection\Compiler\LoggerDecoratorPass;
 use Draw\Bundle\FrameworkExtraBundle\DependencyInjection\Compiler\MessengerBrokerCompilerPass;
+use Draw\Bundle\FrameworkExtraBundle\DependencyInjection\Compiler\MessengerRetryFailedMessageMessageHandlerCompilerPass;
 use Draw\Bundle\FrameworkExtraBundle\DependencyInjection\Compiler\MessengerTransportNamesCompilerPass;
 use Draw\Bundle\FrameworkExtraBundle\DependencyInjection\Compiler\TagIfExpressionCompilerPass;
 use Draw\Bundle\FrameworkExtraBundle\DependencyInjection\Compiler\UserCheckerDecoratorPass;
@@ -19,6 +20,7 @@
 use Draw\Component\Mailer\EmailWriter\EmailWriterInterface;
 use Draw\Component\Messenger\Broker\Broker;
 use Draw\Component\Messenger\Expirable\Command\PurgeExpiredMessageCommand;
+use Draw\Component\Messenger\MessageHandler\RetryFailedMessageMessageHandler;
 use Draw\Component\OpenApi\OpenApi;
 use Draw\Component\Security\Core\User\EventDrivenUserChecker;
 use Draw\Component\Security\Http\Authenticator\JwtAuthenticator;
@@ -45,6 +47,10 @@ public function build(ContainerBuilder $container): void
             $container->addCompilerPass(new MessengerBrokerCompilerPass());
         }
 
+        if (class_exists(RetryFailedMessageMessageHandler::class)) {
+            $container->addCompilerPass(new MessengerRetryFailedMessageMessageHandlerCompilerPass());
+        }
+
         if (class_exists(AddNewestInstanceRoleCommandOptionPass::class)) {
             $container->addCompilerPass(new AddNewestInstanceRoleCommandOptionPass());
         }

diff --git a/packages/framework-extra-bundle/Tests/DrawFrameworkExtraBundleTest.php b/packages/framework-extra-bundle/Tests/DrawFrameworkExtraBundleTest.php
@@ -9,6 +9,7 @@
 use Draw\Bundle\FrameworkExtraBundle\DependencyInjection\Compiler\JmsDoctrineObjectConstructionCompilerPass;
 use Draw\Bundle\FrameworkExtraBundle\DependencyInjection\Compiler\LoggerDecoratorPass;
 use Draw\Bundle\FrameworkExtraBundle\DependencyInjection\Compiler\MessengerBrokerCompilerPass;
+use Draw\Bundle\FrameworkExtraBundle\DependencyInjection\Compiler\MessengerRetryFailedMessageMessageHandlerCompilerPass;
 use Draw\Bundle\FrameworkExtraBundle\DependencyInjection\Compiler\MessengerTransportNamesCompilerPass;
 use Draw\Bundle\FrameworkExtraBundle\DependencyInjection\Compiler\TagIfExpressionCompilerPass;
 use Draw\Bundle\FrameworkExtraBundle\DependencyInjection\Compiler\UserCheckerDecoratorPass;
@@ -37,7 +38,7 @@ public function testBuild(): void
         $containerBuilder = $this->createMock(ContainerBuilder::class);
 
         $containerBuilder
-            ->expects(static::exactly(10))
+            ->expects(static::exactly(11))
             ->method('addCompilerPass')
             ->with(
                 ...static::withConsecutive(
@@ -61,6 +62,11 @@ public function testBuild(): void
                         PassConfig::TYPE_BEFORE_OPTIMIZATION,
                         0,
                     ],
+                    [
+                        static::isInstanceOf(MessengerRetryFailedMessageMessageHandlerCompilerPass::class),
+                        PassConfig::TYPE_BEFORE_OPTIMIZATION,
+                        0,
+                    ],
                     [
                         static::isInstanceOf(AddNewestInstanceRoleCommandOptionPass::class),
                         PassConfig::TYPE_BEFORE_OPTIMIZATION,

diff --git a/packages/messenger/MessageHandler/RetryFailedMessageMessageHandler.php b/packages/messenger/MessageHandler/RetryFailedMessageMessageHandler.php
@@ -5,36 +5,29 @@
 namespace Draw\Component\Messenger\MessageHandler;
 
 use Draw\Component\Messenger\Message\RetryFailedMessageMessage;
-use Symfony\Bundle\FrameworkBundle\Console\Application;
-use Symfony\Component\Console\Input\ArrayInput;
-use Symfony\Component\Console\Output\NullOutput;
-use Symfony\Component\HttpKernel\KernelInterface;
+use Draw\Contracts\Process\ProcessFactoryInterface;
 use Symfony\Component\Messenger\Attribute\AsMessageHandler;
 
 class RetryFailedMessageMessageHandler
 {
     public function __construct(
-        private KernelInterface $kernel,
+        private ProcessFactoryInterface $processFactory,
+        private string $consolePath,
     ) {
     }
 
     #[AsMessageHandler]
     public function handleRetryFailedMessageMessage(RetryFailedMessageMessage $message): void
     {
-        $application = new Application($this->kernel);
-        $application->setAutoExit(false);
-
-        $application->run(
-            new ArrayInput(
+        $this->processFactory
+            ->create(
                 [
-                    'command' => 'messenger:failed:retry',
-                    'id' => [
-                        $message->getMessage()->getId(),
-                    ],
-                    '--force' => true,
+                    $this->consolePath,
+                    'messenger:failed:retry',
+                    $message->getMessage()->getId(),
+                    '--force',
                 ]
-            ),
-            new NullOutput()
-        );
+            )
+            ->mustRun();
     }
 }
diff --git a/packages/messenger/Tests/MessageHandler/RetryFailedMessageMessageHandlerTest.php b/packages/messenger/Tests/MessageHandler/RetryFailedMessageMessageHandlerTest.php
@@ -0,0 +1,64 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Draw\Component\Messenger\Tests\MessageHandler;
+
+use App\Entity\MessengerMessage;
+use Draw\Component\Messenger\Message\RetryFailedMessageMessage;
+use Draw\Component\Messenger\MessageHandler\RetryFailedMessageMessageHandler;
+use Draw\Contracts\Process\ProcessFactoryInterface;
+use PHPUnit\Framework\Attributes\CoversClass;
+use PHPUnit\Framework\MockObject\MockObject;
+use PHPUnit\Framework\TestCase;
+use Ramsey\Uuid\Uuid;
+use Symfony\Component\Process\Process;
+
+#[
+    CoversClass(RetryFailedMessageMessage::class),
+    CoversClass(RetryFailedMessageMessageHandler::class),
+]
+class RetryFailedMessageMessageHandlerTest extends TestCase
+{
+    private const CONSOLE_PATH = 'bin/console';
+
+    private RetryFailedMessageMessageHandler $handler;
+
+    private ProcessFactoryInterface&MockObject $processFactory;
+
+    protected function setUp(): void
+    {
+        parent::setUp();
+
+        $this->handler = new RetryFailedMessageMessageHandler(
+            $this->processFactory = $this->createMock(ProcessFactoryInterface::class),
+            self::CONSOLE_PATH
+        );
+    }
+
+    public function testHandleRetryFailedMessageMessage(): void
+    {
+        $this->processFactory
+            ->expects(static::once())
+            ->method('create')
+            ->with(
+                [
+                    self::CONSOLE_PATH,
+                    'messenger:failed:retry',
+                    $messageId = Uuid::uuid6()->toString(),
+                    '--force',
+                ]
+            )
+            ->willReturn($process = $this->createMock(Process::class));
+
+        $process
+            ->expects(static::once())
+            ->method('mustRun');
+
+        $this->handler->handleRetryFailedMessageMessage(
+            new RetryFailedMessageMessage(
+                (new MessengerMessage())->setId($messageId)
+            )
+        );
+    }
+}
diff --git a/tests/SonataIntegrationBundle/Messenger/Action/RetryFailedMessageActionTest.php b/tests/SonataIntegrationBundle/Messenger/Action/RetryFailedMessageActionTest.php
@@ -0,0 +1,50 @@
+<?php
+
+declare(strict_types=1);
+
+namespace SonataIntegrationBundle\Messenger\Action;
+
+use App\Entity\MessengerMessage;
+use App\Message\FailedMessage;
+use App\Tests\SonataIntegrationBundle\WebTestCaseTrait;
+use Draw\Bundle\TesterBundle\PHPUnit\Extension\SetUpAutowire\AutowiredInterface;
+use Draw\Bundle\TesterBundle\PHPUnit\Extension\SetUpAutowire\AutowireService;
+use Symfony\Bundle\FrameworkBundle\Test\WebTestCase;
+use Symfony\Component\Messenger\MessageBusInterface;
+use Symfony\Component\Messenger\Stamp\TransportNamesStamp;
+
+class RetryFailedMessageActionTest extends WebTestCase implements AutowiredInterface
+{
+    use WebTestCaseTrait;
+
+    #[AutowireService]
+    private MessageBusInterface $messageBus;
+
+    public function testRetry(): void
+    {
+        $this->messageBus->dispatch(
+            new FailedMessage(),
+            [
+                new TransportNamesStamp('failed'),
+            ]
+        );
+
+        $this->login('[email protected]');
+
+        static::assertInstanceOf(
+            MessengerMessage::class,
+            $failedMessage = $this->entityManager
+                ->getRepository(MessengerMessage::class)
+                ->findOneBy(['queueName' => 'failed'])
+        );
+        static::assertSame(FailedMessage::class, $failedMessage->getMessageClass());
+
+        static::$client->request('GET', sprintf('/admin/app/messengermessage/%s/retry', $failedMessage->getId()));
+
+        static::assertResponseStatusCodeSame(302);
+
+        static::$client->followRedirect();
+
+        static::assertSelectorTextContains('.alert-success', 'Retry message successfully dispatched.');
+    }
+}
diff --git a/tests/SonataIntegrationBundle/User/Action/UnlockTraitUserActionTest.php b/tests/SonataIntegrationBundle/User/Action/UnlockTraitUserActionTest.php
@@ -0,0 +1,56 @@
+<?php
+
+namespace App\Tests\SonataIntegrationBundle\User\Action;
+
+use App\Tests\SonataIntegrationBundle\WebTestCaseTrait;
+use Draw\Bundle\TesterBundle\PHPUnit\Extension\SetUpAutowire\AutowiredInterface;
+use Draw\Bundle\UserBundle\Entity\UserLock;
+use Symfony\Bundle\FrameworkBundle\Test\WebTestCase;
+
+class UnlockTraitUserActionTest extends WebTestCase implements AutowiredInterface
+{
+    use WebTestCaseTrait;
+
+    public function testUnlock(): void
+    {
+        $this->login('[email protected]');
+
+        $user = $this->getUser('[email protected]');
+
+        $userLock = $user->getLocks()['manual-lock'];
+
+        $userLock->setUnlockUntil(null);
+
+        static::$client->request('GET', sprintf('/admin/app/user/%s/unlock', $user->getId()));
+
+        static::assertResponseStatusCodeSame(302);
+        static::assertResponseHeaderSame(
+            'Location',
+            sprintf('/admin/app/user/%s/show', $user->getId())
+        );
+
+        static::$client->followRedirect();
+
+        static::assertResponseIsSuccessful();
+
+        static::assertSelectorTextContains('.alert-success', 'The user lock has been unlock for the next 24 hour');
+
+        $userLock = $this->entityManager->find(
+            UserLock::class,
+            $userLock->getId()
+        );
+
+        static::assertEqualsWithDelta(new \DateTimeImmutable('+ 24 hours'), $userLock->getUnlockUntil(), 2);
+    }
+
+    public function testNoAccess(): void
+    {
+        $this->login('[email protected]');
+
+        $user = $this->getUser('[email protected]');
+
+        static::$client->request('GET', sprintf('/admin/app/user/%s/unlock', $user->getId()));
+
+        static::assertResponseStatusCodeSame(403);
+    }
+}