PAYOSWXP-156: webhooks: add sort-order for handlers to prevent wrong …

…order
PAYONE-GmbH · Sep 16, 2024 · 1e26985 · 1e26985
1 parent 725b5a1
commit 1e26985
Show file tree

Hide file tree

Showing 7 changed files with 133 additions and 56 deletions.
diff --git a/src/DependencyInjection/webhooks.xml b/src/DependencyInjection/webhooks.xml
@@ -10,33 +10,36 @@
             <argument key="$logger" type="service" id="monolog.logger.payone" />
         </service>
 
-        <service id="PayonePayment\Payone\Webhook\Handler\TransactionStatusWebhookHandler">
-            <argument type="service" id="PayonePayment\Components\TransactionStatus\TransactionStatusService"/>
+        <service id="PayonePayment\Payone\Webhook\Handler\WebhookLogHandler">
             <argument type="service" id="PayonePayment\Components\DataHandler\Transaction\TransactionDataHandlerInterface" />
+            <argument type="service" id="PayonePayment\Components\DataHandler\WebhookLog\WebhookLogDataHandlerInterface" />
             <argument type="service" id="monolog.logger.payone" />
-            <argument type="service" id="PayonePayment\Components\AutomaticCaptureService\AutomaticCaptureServiceInterface" />
 
-            <tag name="payone.webhook.handler" />
+            <tag name="payone.webhook.handler" priority="1000" />
         </service>
 
-        <service id="PayonePayment\Payone\Webhook\Handler\WebhookLogHandler">
+        <service id="PayonePayment\Payone\Webhook\Handler\TransactionStatusWebhookHandler">
+            <argument type="service" id="PayonePayment\Components\TransactionStatus\TransactionStatusService"/>
             <argument type="service" id="PayonePayment\Components\DataHandler\Transaction\TransactionDataHandlerInterface" />
-            <argument type="service" id="PayonePayment\Components\DataHandler\WebhookLog\WebhookLogDataHandlerInterface" />
             <argument type="service" id="monolog.logger.payone" />
 
-            <tag name="payone.webhook.handler" />
+            <tag name="payone.webhook.handler" priority="300" />
+        </service>
+
+        <service id="PayonePayment\Payone\Webhook\Handler\PaymentStatusHandler" autowire="true">
+            <tag name="payone.webhook.handler" priority="200" />
+        </service>
+
+        <service id="PayonePayment\Payone\Webhook\Handler\AutoCaptureHandler" autowire="true">
+            <tag name="payone.webhook.handler" priority="100" />
         </service>
 
         <service id="PayonePayment\Payone\Webhook\Handler\NotificationForwardHandler">
             <argument type="service" id="payone_payment_notification_target.repository"/>
             <argument type="service" id="PayonePayment\Components\DataHandler\Transaction\TransactionDataHandlerInterface" />
             <argument type="service" id="messenger.bus.shopware"/>
 
-            <tag name="payone.webhook.handler" />
-        </service>
-
-        <service id="PayonePayment\Payone\Webhook\Handler\PaymentStatusHandler" autowire="true">
-            <tag name="payone.webhook.handler" />
+            <tag name="payone.webhook.handler" priority="0" />
         </service>
 
         <service id="PayonePayment\Payone\Webhook\MessageBus\MessageHandler\NotificationForwardHandler" autowire="true">

diff --git a/src/Payone/Webhook/Handler/AutoCaptureHandler.php b/src/Payone/Webhook/Handler/AutoCaptureHandler.php
@@ -0,0 +1,40 @@
+<?php
+
+declare(strict_types=1);
+
+namespace PayonePayment\Payone\Webhook\Handler;
+
+use PayonePayment\Components\AutomaticCaptureService\AutomaticCaptureServiceInterface;
+use PayonePayment\Components\DataHandler\Transaction\TransactionDataHandlerInterface;
+use PayonePayment\Components\TransactionStatus\TransactionStatusService;
+use PayonePayment\Struct\PaymentTransaction;
+use Shopware\Core\System\SalesChannel\SalesChannelContext;
+use Symfony\Component\HttpFoundation\Request;
+
+class AutoCaptureHandler implements WebhookHandlerInterface
+{
+    public function __construct(
+        private readonly TransactionDataHandlerInterface $transactionDataHandler,
+        private readonly AutomaticCaptureServiceInterface $automaticCaptureService
+    ) {
+    }
+
+    public function process(SalesChannelContext $salesChannelContext, Request $request): void
+    {
+        $paymentTransaction = $this->transactionDataHandler->getPaymentTransactionByPayoneTransactionId(
+            $salesChannelContext->getContext(),
+            $request->request->getInt('txid')
+        );
+
+        if (!$paymentTransaction instanceof PaymentTransaction) {
+            return;
+        }
+
+        $this->automaticCaptureService->captureIfPossible($paymentTransaction, $salesChannelContext);
+    }
+
+    public function supports(SalesChannelContext $salesChannelContext, array $data): bool
+    {
+        return isset($data['txid']) && ($data['txaction'] ?? null) === TransactionStatusService::ACTION_APPOINTED;
+    }
+}
diff --git a/src/Payone/Webhook/Handler/TransactionStatusWebhookHandler.php b/src/Payone/Webhook/Handler/TransactionStatusWebhookHandler.php
@@ -4,7 +4,6 @@
 
 namespace PayonePayment\Payone\Webhook\Handler;
 
-use PayonePayment\Components\AutomaticCaptureService\AutomaticCaptureServiceInterface;
 use PayonePayment\Components\DataHandler\Transaction\TransactionDataHandlerInterface;
 use PayonePayment\Components\TransactionStatus\TransactionStatusServiceInterface;
 use PayonePayment\Struct\PaymentTransaction;
@@ -17,8 +16,7 @@ class TransactionStatusWebhookHandler implements WebhookHandlerInterface
     public function __construct(
         private readonly TransactionStatusServiceInterface $transactionStatusService,
         private readonly TransactionDataHandlerInterface $transactionDataHandler,
-        private readonly LoggerInterface $logger,
-        private readonly AutomaticCaptureServiceInterface $automaticCaptureService
+        private readonly LoggerInterface $logger
     ) {
     }
 
@@ -54,17 +52,6 @@ public function process(SalesChannelContext $salesChannelContext, Request $reque
 
         $this->transactionDataHandler->saveTransactionData($paymentTransaction, $salesChannelContext->getContext(), $payoneTransactionData);
         $this->transactionStatusService->transitionByConfigMapping($salesChannelContext, $paymentTransaction, $data);
-
-        // Reload the paymentTransaction for automatic capture
-        /** @var PaymentTransaction|null $paymentTransaction */
-        $paymentTransaction = $this->transactionDataHandler->getPaymentTransactionByPayoneTransactionId(
-            $salesChannelContext->getContext(),
-            (int) $data['txid']
-        );
-
-        if ($paymentTransaction) {
-            $this->automaticCaptureService->captureIfPossible($paymentTransaction, $salesChannelContext);
-        }
     }
 
     private function utf8EncodeRecursive(array $transactionData): array

diff --git a/tests/Payone/Webhook/Handler/AutoCaptureHandlerTest.php b/tests/Payone/Webhook/Handler/AutoCaptureHandlerTest.php
@@ -0,0 +1,68 @@
+<?php
+
+declare(strict_types=1);
+
+namespace PayonePayment\Payone\Webhook\Handler;
+
+use PayonePayment\Components\AutomaticCaptureService\AutomaticCaptureServiceInterface;
+use PayonePayment\Components\DataHandler\Transaction\TransactionDataHandlerInterface;
+use PayonePayment\Components\TransactionStatus\TransactionStatusService;
+use PayonePayment\Struct\PaymentTransaction;
+use PHPUnit\Framework\TestCase;
+use Shopware\Core\System\SalesChannel\SalesChannelContext;
+use Symfony\Component\HttpFoundation\Request;
+
+/**
+ * @covers \PayonePayment\Payone\Webhook\Handler\AutoCaptureHandler
+ */
+class AutoCaptureHandlerTest extends TestCase
+{
+    public function testSupports(): void
+    {
+        $handler = new AutoCaptureHandler(
+            $this->createMock(TransactionDataHandlerInterface::class),
+            $this->createMock(AutomaticCaptureServiceInterface::class),
+        );
+
+        $salesChannelMock = $this->createMock(SalesChannelContext::class);
+
+        static::assertTrue($handler->supports($salesChannelMock, ['txid' => 1, 'txaction' => 'appointed']), 'supports should return true, cause of valid txid nd correct status');
+
+        static::assertFalse($handler->supports($salesChannelMock, ['txaction' => 'appointed']), 'supports should return false, cause of missing txid');
+        static::assertFalse($handler->supports($salesChannelMock, ['txid' => 1]), 'supports should return false, cause of missing status');
+        static::assertFalse($handler->supports($salesChannelMock, []), 'supports should return false, cause of missing data');
+        static::assertFalse($handler->supports($salesChannelMock, []), 'supports should return false, cause of missing data');
+
+        static::assertFalse($handler->supports($salesChannelMock, ['txid' => 1, 'txaction' => TransactionStatusService::ACTION_CAPTURE]), 'supports should return false, cause of wrong status');
+        static::assertFalse($handler->supports($salesChannelMock, ['txid' => 1, 'txaction' => TransactionStatusService::ACTION_COMPLETED]), 'supports should return false, cause of wrong status');
+        static::assertFalse($handler->supports($salesChannelMock, ['txid' => 1, 'txaction' => TransactionStatusService::ACTION_FAILED]), 'supports should return false, cause of wrong status');
+    }
+
+    public function testIsCaptureGotExecuted(): void
+    {
+        $dataHandler = $this->createMock(TransactionDataHandlerInterface::class);
+        $dataHandler->method('getPaymentTransactionByPayoneTransactionId')->willReturn($this->createMock(PaymentTransaction::class));
+        $captureService = $this->createMock(AutomaticCaptureServiceInterface::class);
+        $captureService->expects(static::once())->method('captureIfPossible');
+
+        $request = new Request();
+        $request->request->set('txid', 123);
+
+        (new AutoCaptureHandler($dataHandler, $captureService))
+            ->process($this->createMock(SalesChannelContext::class), $request);
+    }
+
+    public function testIsCaptureGotNotExecutedIfTransactionIsNotFound(): void
+    {
+        $dataHandler = $this->createMock(TransactionDataHandlerInterface::class);
+        $dataHandler->method('getPaymentTransactionByPayoneTransactionId')->willReturn(null);
+        $captureService = $this->createMock(AutomaticCaptureServiceInterface::class);
+        $captureService->expects(static::never())->method('captureIfPossible');
+
+        $request = new Request();
+        $request->request->set('txid', 123);
+
+        (new AutoCaptureHandler($dataHandler, $captureService))
+            ->process($this->createMock(SalesChannelContext::class), $request);
+    }
+}
diff --git a/tests/Payone/Webhook/Handler/TransactionStatusWebhookHandlerTest.php b/tests/Payone/Webhook/Handler/TransactionStatusWebhookHandlerTest.php
@@ -4,7 +4,6 @@
 
 namespace PayonePayment\Payone\Webhook\Handler;
 
-use PayonePayment\Components\AutomaticCaptureService\AutomaticCaptureServiceInterface;
 use PayonePayment\Components\DataHandler\Transaction\TransactionDataHandlerInterface;
 use PayonePayment\Constants;
 use PayonePayment\PaymentHandler\PayoneCreditCardPaymentHandler;
@@ -46,16 +45,12 @@ public function testItAppointsCreditCardWithoutMapping(): void
         ];
 
         $transactionDataHandler = $this->createMock(TransactionDataHandlerInterface::class);
-        $transactionDataHandler->expects(static::exactly(2))->method('getPaymentTransactionByPayoneTransactionId')->willReturn($paymentTransaction);
+        $transactionDataHandler->expects(static::exactly(1))->method('getPaymentTransactionByPayoneTransactionId')->willReturn($paymentTransaction);
         $transactionDataHandler->expects(static::once())->method('getTransactionDataFromWebhook')->willReturn($transactionData);
 
-        $automaticCaptureService = $this->createMock(AutomaticCaptureServiceInterface::class);
-        $automaticCaptureService->expects(static::once())->method('captureIfPossible');
-
         $transactionStatusHandler = TransactionStatusWebhookHandlerFactory::createHandler(
             $transactionStatusService,
-            $transactionDataHandler,
-            $automaticCaptureService
+            $transactionDataHandler
         );
 
         $transactionStatusHandler->process(
@@ -97,16 +92,12 @@ public function testItAppointsCreditCardWithMapping(): void
         ];
 
         $transactionDataHandler = $this->createMock(TransactionDataHandlerInterface::class);
-        $transactionDataHandler->expects(static::exactly(2))->method('getPaymentTransactionByPayoneTransactionId')->willReturn($paymentTransaction);
+        $transactionDataHandler->expects(static::exactly(1))->method('getPaymentTransactionByPayoneTransactionId')->willReturn($paymentTransaction);
         $transactionDataHandler->expects(static::once())->method('getTransactionDataFromWebhook')->willReturn($transactionData);
 
-        $automaticCaptureService = $this->createMock(AutomaticCaptureServiceInterface::class);
-        $automaticCaptureService->expects(static::once())->method('captureIfPossible');
-
         $transactionStatusHandler = TransactionStatusWebhookHandlerFactory::createHandler(
             $transactionStatusService,
-            $transactionDataHandler,
-            $automaticCaptureService
+            $transactionDataHandler
         );
 
         $transactionStatusHandler->process(
@@ -148,16 +139,12 @@ public function testItAppointsCreditCardWithSpecificMapping(): void
         ];
 
         $transactionDataHandler = $this->createMock(TransactionDataHandlerInterface::class);
-        $transactionDataHandler->expects(static::exactly(2))->method('getPaymentTransactionByPayoneTransactionId')->willReturn($paymentTransaction);
+        $transactionDataHandler->expects(static::exactly(1))->method('getPaymentTransactionByPayoneTransactionId')->willReturn($paymentTransaction);
         $transactionDataHandler->expects(static::once())->method('getTransactionDataFromWebhook')->willReturn($transactionData);
 
-        $automaticCaptureService = $this->createMock(AutomaticCaptureServiceInterface::class);
-        $automaticCaptureService->expects(static::once())->method('captureIfPossible');
-
         $transactionStatusHandler = TransactionStatusWebhookHandlerFactory::createHandler(
             $transactionStatusService,
-            $transactionDataHandler,
-            $automaticCaptureService
+            $transactionDataHandler
         );
 
         $transactionStatusHandler->process(

diff --git a/tests/Payone/Webhook/Processor/WebhookProcessorTest.php b/tests/Payone/Webhook/Processor/WebhookProcessorTest.php
@@ -4,7 +4,6 @@
 
 namespace PayonePayment\Payone\Webhook\Processor;
 
-use PayonePayment\Components\AutomaticCaptureService\AutomaticCaptureServiceInterface;
 use PayonePayment\Components\DataHandler\Transaction\TransactionDataHandlerInterface;
 use PayonePayment\Components\TransactionStatus\TransactionStatusService;
 use PayonePayment\Constants;
@@ -213,16 +212,12 @@ protected function getWebhookProcessor(string $transitionName, array $transactio
         $paymentTransaction = PaymentTransaction::fromOrderTransaction($orderTransactionEntity, $orderEntity);
 
         $transactionDataHandler = $this->createMock(TransactionDataHandlerInterface::class);
-        $transactionDataHandler->expects(static::exactly(2))->method('getPaymentTransactionByPayoneTransactionId')->willReturn($paymentTransaction);
+        $transactionDataHandler->expects(static::exactly(1))->method('getPaymentTransactionByPayoneTransactionId')->willReturn($paymentTransaction);
         $transactionDataHandler->expects(static::once())->method('getTransactionDataFromWebhook')->willReturn($transactionData);
 
-        $automaticCaptureService = $this->createMock(AutomaticCaptureServiceInterface::class);
-        $automaticCaptureService->expects(static::once())->method('captureIfPossible');
-
         $transactionStatusHandler = TransactionStatusWebhookHandlerFactory::createHandler(
             $transactionStatusService,
-            $transactionDataHandler,
-            $automaticCaptureService
+            $transactionDataHandler
         );
 
         return new WebhookProcessor(

diff --git a/tests/TestCaseBase/Factory/TransactionStatusWebhookHandlerFactory.php b/tests/TestCaseBase/Factory/TransactionStatusWebhookHandlerFactory.php
@@ -4,7 +4,6 @@
 
 namespace PayonePayment\TestCaseBase\Factory;
 
-use PayonePayment\Components\AutomaticCaptureService\AutomaticCaptureServiceInterface;
 use PayonePayment\Components\Currency\CurrencyPrecision;
 use PayonePayment\Components\DataHandler\Transaction\TransactionDataHandlerInterface;
 use PayonePayment\Components\TransactionStatus\TransactionStatusService;
@@ -29,13 +28,11 @@ class TransactionStatusWebhookHandlerFactory
     public static function createHandler(
         TransactionStatusServiceInterface $transactionStatusService,
         TransactionDataHandlerInterface $transactionDataHandler,
-        AutomaticCaptureServiceInterface $automaticCaptureService
     ): TransactionStatusWebhookHandler {
         return new TransactionStatusWebhookHandler(
             $transactionStatusService,
             $transactionDataHandler,
-            new NullLogger(),
-            $automaticCaptureService
+            new NullLogger()
         );
     }