From 6d703324304fc0c64a7e7fb8e209bf4b3f3b3562 Mon Sep 17 00:00:00 2001 From: Mateusz Zalewski Date: Tue, 20 Jun 2023 13:48:10 +0200 Subject: [PATCH 1/5] Webhooks final fix --- composer.json | 1 + config/packages/lock.yaml | 2 + config/packages/monolog.php | 25 +++++++++ config/services.php | 12 +++++ config/services/actions.php | 7 +++ config/services/command_handler.php | 2 + config/shop_routing.yml | 2 +- phpunit.xml.dist | 6 +++ src/Client/SaferpayClientBodyFactory.php | 2 +- src/Controller/Action/AssertAction.php | 2 + src/Controller/Action/PrepareAssertAction.php | 21 ++++++++ src/Controller/Action/WebhookAction.php | 33 +++++++++++- .../PaymentAlreadyProcessedException.php | 10 ++++ src/Payment/Command/AssertPaymentCommand.php | 7 ++- src/Payum/Action/AuthorizeAction.php | 8 +-- src/Payum/Action/CaptureAction.php | 4 +- src/Payum/Action/ResolveNextCommandAction.php | 2 +- src/Payum/Provider/TokenProvider.php | 1 + src/Processor/SaferpayPaymentProcessor.php | 54 +++++++++++++++++++ src/Provider/PaymentProvider.php | 7 +++ src/Provider/PaymentProviderInterface.php | 2 + .../WebhookRouteGeneratorInterface.php | 2 +- src/Routing/Generator/WebhookUrlGenerator.php | 4 +- symfony.lock | 12 +++++ 24 files changed, 213 insertions(+), 15 deletions(-) create mode 100644 config/packages/lock.yaml create mode 100644 config/packages/monolog.php create mode 100644 src/Exception/PaymentAlreadyProcessedException.php create mode 100644 src/Processor/SaferpayPaymentProcessor.php diff --git a/composer.json b/composer.json index 45ad221b..88411777 100644 --- a/composer.json +++ b/composer.json @@ -11,6 +11,7 @@ "sylius/mailer-bundle": "^1.8 || ^2.0", "sylius/sylius": "~1.10.0 || ~1.11.0 || ~1.12.0", "symfony/http-client": "5.4.* || ^6.0", + "symfony/lock": "5.4.*", "symfony/messenger": "5.4.* || ^6.0", "symfony/uid": "5.4.* || ^6.0", "symfony/webpack-encore-bundle": "^1.15" diff --git a/config/packages/lock.yaml b/config/packages/lock.yaml new file mode 100644 index 00000000..70f578a1 --- /dev/null +++ b/config/packages/lock.yaml @@ -0,0 +1,2 @@ +framework: + lock: ~ diff --git a/config/packages/monolog.php b/config/packages/monolog.php new file mode 100644 index 00000000..69dd710f --- /dev/null +++ b/config/packages/monolog.php @@ -0,0 +1,25 @@ +extension('monolog', [ + 'channels' => ['saferpay'], + 'handlers' => [ + 'main' => [ + 'type' => 'stream', + 'path' => '%kernel.logs_dir%/%kernel.environment%.log', + 'level' => 'debug', + 'channels' => ['!event', '!doctrine'], + ], + 'saferpay' => [ + 'type' => 'stream', + 'path' => '%kernel.logs_dir%/saferpay.log', + 'level' => 'debug', + 'channels' => ['saferpay'], + ], + ], + ]); +}; diff --git a/config/services.php b/config/services.php index a853b409..f8d86800 100644 --- a/config/services.php +++ b/config/services.php @@ -2,10 +2,12 @@ declare(strict_types=1); +use CommerceWeavers\SyliusSaferpayPlugin\Processor\SaferpayPaymentProcessor; use CommerceWeavers\SyliusSaferpayPlugin\Resolver\SaferpayApiBaseUrlResolver; use CommerceWeavers\SyliusSaferpayPlugin\Resolver\SaferpayApiBaseUrlResolverInterface; use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator; use function Symfony\Component\DependencyInjection\Loader\Configurator\param; +use function Symfony\Component\DependencyInjection\Loader\Configurator\service; return static function (ContainerConfigurator $containerConfigurator) { $containerConfigurator->import(__DIR__ . '/services/**/**'); @@ -19,4 +21,14 @@ param('commerce_weavers.saferpay.test_api_base_url'), ]) ; + + $services + ->set(SaferpayPaymentProcessor::class) + ->public() + ->args([ + service('lock.factory'), + service('doctrine.orm.entity_manager'), + service('monolog.logger.saferpay'), + ]) + ; }; diff --git a/config/services/actions.php b/config/services/actions.php index 72fe1d4e..10b31470 100644 --- a/config/services/actions.php +++ b/config/services/actions.php @@ -9,6 +9,7 @@ use CommerceWeavers\SyliusSaferpayPlugin\Controller\Action\WebhookAction; use CommerceWeavers\SyliusSaferpayPlugin\Payum\Factory\AssertFactoryInterface; use CommerceWeavers\SyliusSaferpayPlugin\Payum\Provider\TokenProviderInterface; +use CommerceWeavers\SyliusSaferpayPlugin\Processor\SaferpayPaymentProcessor; use CommerceWeavers\SyliusSaferpayPlugin\Provider\PaymentProviderInterface; use Sylius\Component\Resource\Metadata\MetadataInterface; use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator; @@ -27,6 +28,9 @@ ->args(['sylius.order']), service(PaymentProviderInterface::class), service(TokenProviderInterface::class), + service(SaferpayPaymentProcessor::class), + service('router'), + service('monolog.logger.saferpay'), ]) ->tag('controller.service_arguments') ; @@ -73,6 +77,9 @@ ->args([ service('payum'), service('sylius.command_bus'), + service('monolog.logger.saferpay'), + service(PaymentProviderInterface::class), + service(SaferpayPaymentProcessor::class), ]) ->tag('controller.service_arguments') ; diff --git a/config/services/command_handler.php b/config/services/command_handler.php index ca903dbc..dd9d8781 100644 --- a/config/services/command_handler.php +++ b/config/services/command_handler.php @@ -8,6 +8,8 @@ use CommerceWeavers\SyliusSaferpayPlugin\Payum\Factory\AssertFactoryInterface; use CommerceWeavers\SyliusSaferpayPlugin\Payum\Factory\CaptureFactoryInterface; use CommerceWeavers\SyliusSaferpayPlugin\Payum\Factory\ResolveNextCommandFactoryInterface; +use CommerceWeavers\SyliusSaferpayPlugin\Processor\SaferpayPaymentProcessor; +use CommerceWeavers\SyliusSaferpayPlugin\Provider\PaymentProviderInterface; use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator; use function Symfony\Component\DependencyInjection\Loader\Configurator\service; diff --git a/config/shop_routing.yml b/config/shop_routing.yml index 57d1434f..96e48a78 100644 --- a/config/shop_routing.yml +++ b/config/shop_routing.yml @@ -23,7 +23,7 @@ commerce_weavers_sylius_saferpay_prepare_capture: route: sylius_shop_order_after_pay commerce_weavers_sylius_saferpay_webhook: - path: /payment/saferpay/webhook/{payum_token} + path: /payment/saferpay/webhook/{payum_token}/{order_token} methods: [GET] defaults: _controller: CommerceWeavers\SyliusSaferpayPlugin\Controller\Action\WebhookAction diff --git a/phpunit.xml.dist b/phpunit.xml.dist index 1070b705..89755b77 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -22,5 +22,11 @@ + + + + + + diff --git a/src/Client/SaferpayClientBodyFactory.php b/src/Client/SaferpayClientBodyFactory.php index 1df54259..7a8f87dd 100644 --- a/src/Client/SaferpayClientBodyFactory.php +++ b/src/Client/SaferpayClientBodyFactory.php @@ -40,7 +40,7 @@ public function createForAuthorize(PaymentInterface $payment, TokenInterface $to $allowedPaymentMethods = $config['allowed_payment_methods'] ?? []; $webhookToken = $this->tokenProvider->provideForWebhook($payment, self::COMMERCE_WEAVERS_SYLIUS_SAFERPAY_WEBHOOK); - $notificationUrl = $this->webhookRouteGenerator->generate($webhookToken->getHash()); + $notificationUrl = $this->webhookRouteGenerator->generate($webhookToken->getHash(), $order->getTokenValue()); return array_merge($this->provideBodyRequestHeader($gatewayConfig), [ 'TerminalId' => $terminalId, diff --git a/src/Controller/Action/AssertAction.php b/src/Controller/Action/AssertAction.php index 33186b16..459fb079 100644 --- a/src/Controller/Action/AssertAction.php +++ b/src/Controller/Action/AssertAction.php @@ -5,11 +5,13 @@ namespace CommerceWeavers\SyliusSaferpayPlugin\Controller\Action; use CommerceWeavers\SyliusSaferpayPlugin\Payum\Factory\AssertFactoryInterface; +use CommerceWeavers\SyliusSaferpayPlugin\Processor\SaferpayPaymentProcessor; use Exception; use Payum\Core\Payum; use Payum\Core\Request\GetStatusInterface; use Sylius\Bundle\PayumBundle\Factory\GetStatusFactoryInterface; use Sylius\Bundle\PayumBundle\Factory\ResolveNextRouteFactoryInterface; +use Sylius\Component\Core\Model\PaymentInterface; use Symfony\Component\HttpFoundation\RedirectResponse; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Session\Session; diff --git a/src/Controller/Action/PrepareAssertAction.php b/src/Controller/Action/PrepareAssertAction.php index 602581ea..d5eb5538 100644 --- a/src/Controller/Action/PrepareAssertAction.php +++ b/src/Controller/Action/PrepareAssertAction.php @@ -5,11 +5,14 @@ namespace CommerceWeavers\SyliusSaferpayPlugin\Controller\Action; use CommerceWeavers\SyliusSaferpayPlugin\Payum\Provider\TokenProviderInterface; +use CommerceWeavers\SyliusSaferpayPlugin\Processor\SaferpayPaymentProcessor; use CommerceWeavers\SyliusSaferpayPlugin\Provider\PaymentProviderInterface; +use Psr\Log\LoggerInterface; use Sylius\Bundle\ResourceBundle\Controller\RequestConfigurationFactoryInterface; use Sylius\Component\Resource\Metadata\MetadataInterface; use Symfony\Component\HttpFoundation\RedirectResponse; use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\Routing\Generator\UrlGeneratorInterface; final class PrepareAssertAction { @@ -18,16 +21,34 @@ public function __construct( private MetadataInterface $orderMetadata, private PaymentProviderInterface $paymentProvider, private TokenProviderInterface $tokenProvider, + private SaferpayPaymentProcessor $saferpayPaymentProcessor, + private UrlGeneratorInterface $router, + private LoggerInterface $logger, ) { } public function __invoke(Request $request, string $tokenValue): RedirectResponse { + $this->logger->debug('Synchronous processing started'); + + try { + $payment = $this->paymentProvider->provideForOrder($tokenValue); + $this->saferpayPaymentProcessor->lock($payment); + } catch (\Exception) { + $this->logger->debug('Synchronous processing aborted - webhook handled the payment'); + + $request->getSession()->getFlashBag()->add('success', 'sylius.payment.completed'); + + return new RedirectResponse($this->router->generate('sylius_shop_order_thank_you')); + } + $requestConfiguration = $this->requestConfigurationFactory->create($this->orderMetadata, $request); $lastPayment = $this->paymentProvider->provideForAssert($tokenValue); $token = $this->tokenProvider->provideForAssert($lastPayment, $requestConfiguration); + $this->logger->debug('Synchronous processing PrepareAssertAction succeeded'); + return new RedirectResponse($token->getTargetUrl()); } } diff --git a/src/Controller/Action/WebhookAction.php b/src/Controller/Action/WebhookAction.php index 9a3bddc8..8b98ee44 100644 --- a/src/Controller/Action/WebhookAction.php +++ b/src/Controller/Action/WebhookAction.php @@ -4,11 +4,17 @@ namespace CommerceWeavers\SyliusSaferpayPlugin\Controller\Action; +use CommerceWeavers\SyliusSaferpayPlugin\Exception\PaymentAlreadyProcessedException; use CommerceWeavers\SyliusSaferpayPlugin\Payment\Command\AssertPaymentCommand; +use CommerceWeavers\SyliusSaferpayPlugin\Processor\SaferpayPaymentProcessor; +use CommerceWeavers\SyliusSaferpayPlugin\Provider\PaymentProviderInterface; use Payum\Core\Payum; +use Psr\Log\LoggerInterface; +use Sylius\Component\Core\Model\PaymentInterface; use Symfony\Component\HttpFoundation\JsonResponse; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; +use Symfony\Component\Messenger\Exception\HandlerFailedException; use Symfony\Component\Messenger\MessageBusInterface; final class WebhookAction @@ -16,15 +22,38 @@ final class WebhookAction public function __construct( private Payum $payum, private MessageBusInterface $commandBus, + private LoggerInterface $logger, + private PaymentProviderInterface $paymentProvider, + private SaferpayPaymentProcessor $saferpayPaymentProcessor, ) { } public function __invoke(Request $request): Response { + $this->logger->debug('Handling webhook started'); + + $orderToken = $request->attributes->get('order_token'); + $payment = $this->paymentProvider->provideForOrder($orderToken); + try { + $this->saferpayPaymentProcessor->lock($payment); + } catch (\Exception) { + $this->logger->debug('Webhook aborted - payment already processed'); + + return new JsonResponse(status: Response::HTTP_OK); + } + $token = $this->payum->getHttpRequestVerifier()->verify($request); - $this->commandBus->dispatch(new AssertPaymentCommand($token->getHash())); + try { + $this->commandBus->dispatch(new AssertPaymentCommand($token->getHash(), $orderToken)); + } catch (HandlerFailedException $exception) { + $this->logger->debug('Webhook failed: ', ['exception' => $exception->getMessage()]); + + return new JsonResponse(status: Response::HTTP_BAD_REQUEST); + } + + $this->logger->debug('Webhook handled successfully'); - return new JsonResponse(status: Response::HTTP_NO_CONTENT); + return new JsonResponse(status: Response::HTTP_OK); } } diff --git a/src/Exception/PaymentAlreadyProcessedException.php b/src/Exception/PaymentAlreadyProcessedException.php new file mode 100644 index 00000000..4abbb42a --- /dev/null +++ b/src/Exception/PaymentAlreadyProcessedException.php @@ -0,0 +1,10 @@ +payumToken; } + + public function getOrderToken(): string + { + return $this->orderToken; + } } diff --git a/src/Payum/Action/AuthorizeAction.php b/src/Payum/Action/AuthorizeAction.php index ee38e2cc..a0880650 100644 --- a/src/Payum/Action/AuthorizeAction.php +++ b/src/Payum/Action/AuthorizeAction.php @@ -38,9 +38,9 @@ public function execute(mixed $request): void $response = $this->saferpayClient->authorize($payment, $token); if ($response instanceof ErrorResponse) { - $payment->setDetails([ + $payment->setDetails(array_merge($payment->getDetails(), [ 'status' => StatusAction::STATUS_FAILED, - ]); + ])); return; } @@ -48,11 +48,11 @@ public function execute(mixed $request): void $redirectUrl = $response->getRedirectUrl(); $token->setAfterUrl($redirectUrl); - $payment->setDetails([ + $payment->setDetails(array_merge($payment->getDetails(), [ 'request_id' => $response->getResponseHeader()->getRequestId(), 'saferpay_token' => $response->getToken(), 'status' => StatusAction::STATUS_NEW, - ]); + ])); } public function supports($request): bool diff --git a/src/Payum/Action/CaptureAction.php b/src/Payum/Action/CaptureAction.php index 87a553ba..2a0cc76b 100644 --- a/src/Payum/Action/CaptureAction.php +++ b/src/Payum/Action/CaptureAction.php @@ -37,10 +37,10 @@ public function execute($request): void /** @var CaptureResponse|ErrorResponse $response */ $response = $this->saferpayClient->capture($payment); if ($response instanceof ErrorResponse) { - $payment->setDetails([ + $payment->setDetails(array_merge($payment->getDetails(), [ 'status' => StatusAction::STATUS_FAILED, 'transaction_id' => $response->getTransactionId(), - ]); + ])); return; } diff --git a/src/Payum/Action/ResolveNextCommandAction.php b/src/Payum/Action/ResolveNextCommandAction.php index f059e17d..084231b4 100644 --- a/src/Payum/Action/ResolveNextCommandAction.php +++ b/src/Payum/Action/ResolveNextCommandAction.php @@ -32,7 +32,7 @@ public function execute($request): void $token = $this->tokenProvider->provideForCommandHandler($payment); if ($this->statusChecker->isNew($payment)) { - $request->setNextCommand(new AssertPaymentCommand($token->getHash())); + $request->setNextCommand(new AssertPaymentCommand($token->getHash(), $payment->getOrder()->getTokenValue())); return; } diff --git a/src/Payum/Provider/TokenProvider.php b/src/Payum/Provider/TokenProvider.php index 806837fb..a3de5328 100644 --- a/src/Payum/Provider/TokenProvider.php +++ b/src/Payum/Provider/TokenProvider.php @@ -63,6 +63,7 @@ public function provideForWebhook(PaymentInterface $payment, string $webhookRout $this->getGatewayName($payment), $payment, $webhookRoute, + ['order_token' => $payment->getOrder()->getTokenValue()], ); } diff --git a/src/Processor/SaferpayPaymentProcessor.php b/src/Processor/SaferpayPaymentProcessor.php new file mode 100644 index 00000000..09cfa3b8 --- /dev/null +++ b/src/Processor/SaferpayPaymentProcessor.php @@ -0,0 +1,54 @@ +logger->debug('Trying to lock payment: ', ['id' => $payment->getId(), 'details' => $payment->getDetails()]); + $lock = $this->lockFactory->createLock('payment_processing'); + + try { + if (!$lock->acquire()) { + throw new \Exception('Payment already processed'); + } + } catch (LockConflictedException|LockAcquiringException) { + throw new \Exception('Payment already processed'); + } + + $paymentDetails = $payment->getDetails(); + + if ( + (isset($paymentDetails['processing']) && $paymentDetails['processing'] === true) || + (isset($paymentDetails['status']) && $paymentDetails['status'] !== 'NEW') + ) { + $this->logger->debug('Payment processing aborted: ', ['details' => $paymentDetails]); + + throw new \Exception('Payment already processed'); + } + + $this->logger->debug('Payment is being processed: ', ['details' => $paymentDetails]); + $payment->setDetails(array_merge($paymentDetails, ['processing' => true])); + $this->logger->debug('Payment has been processed: ', ['details' => $payment->getDetails()]); + $this->entityManager->flush(); + + $lock->release(); + } +} diff --git a/src/Provider/PaymentProvider.php b/src/Provider/PaymentProvider.php index 4eabd414..6e3e8e31 100644 --- a/src/Provider/PaymentProvider.php +++ b/src/Provider/PaymentProvider.php @@ -21,6 +21,13 @@ public function provideForAssert(string $orderTokenValue): PaymentInterface return $this->provideByOrderAndState($order, PaymentInterface::STATE_NEW); } + public function provideForOrder(string $orderTokenValue): PaymentInterface + { + $order = $this->orderProvider->provideForAssert($orderTokenValue); + + return $order->getLastPayment(); + } + public function provideForCapture(string $orderTokenValue): PaymentInterface { $order = $this->orderProvider->provideForCapture($orderTokenValue); diff --git a/src/Provider/PaymentProviderInterface.php b/src/Provider/PaymentProviderInterface.php index 5f4584ff..70a6a143 100644 --- a/src/Provider/PaymentProviderInterface.php +++ b/src/Provider/PaymentProviderInterface.php @@ -10,5 +10,7 @@ interface PaymentProviderInterface { public function provideForAssert(string $orderTokenValue): PaymentInterface; + public function provideForOrder(string $orderTokenValue): PaymentInterface; + public function provideForCapture(string $orderTokenValue): PaymentInterface; } diff --git a/src/Routing/Generator/WebhookRouteGeneratorInterface.php b/src/Routing/Generator/WebhookRouteGeneratorInterface.php index 1fda4fe3..a3be9206 100644 --- a/src/Routing/Generator/WebhookRouteGeneratorInterface.php +++ b/src/Routing/Generator/WebhookRouteGeneratorInterface.php @@ -6,5 +6,5 @@ interface WebhookRouteGeneratorInterface { - public function generate(string $payumToken): string; + public function generate(string $payumToken, string $tokenValue): string; } diff --git a/src/Routing/Generator/WebhookUrlGenerator.php b/src/Routing/Generator/WebhookUrlGenerator.php index d6d6e6a1..721676de 100644 --- a/src/Routing/Generator/WebhookUrlGenerator.php +++ b/src/Routing/Generator/WebhookUrlGenerator.php @@ -15,11 +15,11 @@ public function __construct(private RouterInterface $router) { } - public function generate(string $payumToken): string + public function generate(string $payumToken, string $tokenValue): string { return $this->router->generate( self::COMMERCE_WEAVERS_SYLIUS_SAFERPAY_WEBHOOK_ROUTE, - ['payum_token' => $payumToken], + ['payum_token' => $payumToken, 'order_token' => $tokenValue], UrlGeneratorInterface::ABSOLUTE_URL, ); } diff --git a/symfony.lock b/symfony.lock index 833abcf8..0166bfeb 100644 --- a/symfony.lock +++ b/symfony.lock @@ -281,6 +281,18 @@ "src/Kernel.php" ] }, + "symfony/lock": { + "version": "5.4", + "recipe": { + "repo": "github.com/symfony/recipes", + "branch": "main", + "version": "5.2", + "ref": "8e937ff2b4735d110af1770f242c1107fdab4c8e" + }, + "files": [ + "config/packages/lock.yaml" + ] + }, "symfony/mailer": { "version": "5.4", "recipe": { From 7e044d59e99b0751f57e73c3cd432cb929c6a074 Mon Sep 17 00:00:00 2001 From: Mateusz Zalewski Date: Tue, 20 Jun 2023 13:53:33 +0200 Subject: [PATCH 2/5] Minor fixes --- phpunit.xml.dist | 6 ------ src/Controller/Action/AssertAction.php | 2 -- src/Controller/Action/PrepareAssertAction.php | 3 ++- src/Controller/Action/WebhookAction.php | 6 +++--- src/Exception/PaymentAlreadyProcessedException.php | 1 - src/Payment/Command/AssertPaymentCommand.php | 7 +------ src/Payum/Action/ResolveNextCommandAction.php | 2 +- src/Processor/SaferpayPaymentProcessor.php | 9 ++++----- 8 files changed, 11 insertions(+), 25 deletions(-) diff --git a/phpunit.xml.dist b/phpunit.xml.dist index 89755b77..1070b705 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -22,11 +22,5 @@ - - - - - - diff --git a/src/Controller/Action/AssertAction.php b/src/Controller/Action/AssertAction.php index 459fb079..33186b16 100644 --- a/src/Controller/Action/AssertAction.php +++ b/src/Controller/Action/AssertAction.php @@ -5,13 +5,11 @@ namespace CommerceWeavers\SyliusSaferpayPlugin\Controller\Action; use CommerceWeavers\SyliusSaferpayPlugin\Payum\Factory\AssertFactoryInterface; -use CommerceWeavers\SyliusSaferpayPlugin\Processor\SaferpayPaymentProcessor; use Exception; use Payum\Core\Payum; use Payum\Core\Request\GetStatusInterface; use Sylius\Bundle\PayumBundle\Factory\GetStatusFactoryInterface; use Sylius\Bundle\PayumBundle\Factory\ResolveNextRouteFactoryInterface; -use Sylius\Component\Core\Model\PaymentInterface; use Symfony\Component\HttpFoundation\RedirectResponse; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Session\Session; diff --git a/src/Controller/Action/PrepareAssertAction.php b/src/Controller/Action/PrepareAssertAction.php index d5eb5538..0948f152 100644 --- a/src/Controller/Action/PrepareAssertAction.php +++ b/src/Controller/Action/PrepareAssertAction.php @@ -4,6 +4,7 @@ namespace CommerceWeavers\SyliusSaferpayPlugin\Controller\Action; +use CommerceWeavers\SyliusSaferpayPlugin\Exception\PaymentAlreadyProcessedException; use CommerceWeavers\SyliusSaferpayPlugin\Payum\Provider\TokenProviderInterface; use CommerceWeavers\SyliusSaferpayPlugin\Processor\SaferpayPaymentProcessor; use CommerceWeavers\SyliusSaferpayPlugin\Provider\PaymentProviderInterface; @@ -34,7 +35,7 @@ public function __invoke(Request $request, string $tokenValue): RedirectResponse try { $payment = $this->paymentProvider->provideForOrder($tokenValue); $this->saferpayPaymentProcessor->lock($payment); - } catch (\Exception) { + } catch (PaymentAlreadyProcessedException) { $this->logger->debug('Synchronous processing aborted - webhook handled the payment'); $request->getSession()->getFlashBag()->add('success', 'sylius.payment.completed'); diff --git a/src/Controller/Action/WebhookAction.php b/src/Controller/Action/WebhookAction.php index 8b98ee44..25ef7eca 100644 --- a/src/Controller/Action/WebhookAction.php +++ b/src/Controller/Action/WebhookAction.php @@ -10,7 +10,6 @@ use CommerceWeavers\SyliusSaferpayPlugin\Provider\PaymentProviderInterface; use Payum\Core\Payum; use Psr\Log\LoggerInterface; -use Sylius\Component\Core\Model\PaymentInterface; use Symfony\Component\HttpFoundation\JsonResponse; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; @@ -34,9 +33,10 @@ public function __invoke(Request $request): Response $orderToken = $request->attributes->get('order_token'); $payment = $this->paymentProvider->provideForOrder($orderToken); + try { $this->saferpayPaymentProcessor->lock($payment); - } catch (\Exception) { + } catch (PaymentAlreadyProcessedException) { $this->logger->debug('Webhook aborted - payment already processed'); return new JsonResponse(status: Response::HTTP_OK); @@ -45,7 +45,7 @@ public function __invoke(Request $request): Response $token = $this->payum->getHttpRequestVerifier()->verify($request); try { - $this->commandBus->dispatch(new AssertPaymentCommand($token->getHash(), $orderToken)); + $this->commandBus->dispatch(new AssertPaymentCommand($token->getHash())); } catch (HandlerFailedException $exception) { $this->logger->debug('Webhook failed: ', ['exception' => $exception->getMessage()]); diff --git a/src/Exception/PaymentAlreadyProcessedException.php b/src/Exception/PaymentAlreadyProcessedException.php index 4abbb42a..21a3dace 100644 --- a/src/Exception/PaymentAlreadyProcessedException.php +++ b/src/Exception/PaymentAlreadyProcessedException.php @@ -6,5 +6,4 @@ final class PaymentAlreadyProcessedException extends \RuntimeException { - } diff --git a/src/Payment/Command/AssertPaymentCommand.php b/src/Payment/Command/AssertPaymentCommand.php index b35da4e7..8b2f80a3 100644 --- a/src/Payment/Command/AssertPaymentCommand.php +++ b/src/Payment/Command/AssertPaymentCommand.php @@ -6,7 +6,7 @@ final class AssertPaymentCommand { - public function __construct(private string $payumToken, private string $orderToken) + public function __construct(private string $payumToken) { } @@ -14,9 +14,4 @@ public function getPayumToken(): string { return $this->payumToken; } - - public function getOrderToken(): string - { - return $this->orderToken; - } } diff --git a/src/Payum/Action/ResolveNextCommandAction.php b/src/Payum/Action/ResolveNextCommandAction.php index 084231b4..f059e17d 100644 --- a/src/Payum/Action/ResolveNextCommandAction.php +++ b/src/Payum/Action/ResolveNextCommandAction.php @@ -32,7 +32,7 @@ public function execute($request): void $token = $this->tokenProvider->provideForCommandHandler($payment); if ($this->statusChecker->isNew($payment)) { - $request->setNextCommand(new AssertPaymentCommand($token->getHash(), $payment->getOrder()->getTokenValue())); + $request->setNextCommand(new AssertPaymentCommand($token->getHash())); return; } diff --git a/src/Processor/SaferpayPaymentProcessor.php b/src/Processor/SaferpayPaymentProcessor.php index 09cfa3b8..88816eaf 100644 --- a/src/Processor/SaferpayPaymentProcessor.php +++ b/src/Processor/SaferpayPaymentProcessor.php @@ -4,6 +4,7 @@ namespace CommerceWeavers\SyliusSaferpayPlugin\Processor; +use CommerceWeavers\SyliusSaferpayPlugin\Exception\PaymentAlreadyProcessedException; use Doctrine\ORM\EntityManagerInterface; use Psr\Log\LoggerInterface; use Sylius\Component\Core\Model\PaymentInterface; @@ -27,10 +28,10 @@ public function lock(PaymentInterface $payment): void try { if (!$lock->acquire()) { - throw new \Exception('Payment already processed'); + throw new PaymentAlreadyProcessedException(); } } catch (LockConflictedException|LockAcquiringException) { - throw new \Exception('Payment already processed'); + throw new PaymentAlreadyProcessedException(); } $paymentDetails = $payment->getDetails(); @@ -41,12 +42,10 @@ public function lock(PaymentInterface $payment): void ) { $this->logger->debug('Payment processing aborted: ', ['details' => $paymentDetails]); - throw new \Exception('Payment already processed'); + throw new PaymentAlreadyProcessedException(); } - $this->logger->debug('Payment is being processed: ', ['details' => $paymentDetails]); $payment->setDetails(array_merge($paymentDetails, ['processing' => true])); - $this->logger->debug('Payment has been processed: ', ['details' => $payment->getDetails()]); $this->entityManager->flush(); $lock->release(); From b8cd71c081ab683254ec7f5d3eb19693f490d1c9 Mon Sep 17 00:00:00 2001 From: Mateusz Zalewski Date: Tue, 20 Jun 2023 14:05:00 +0200 Subject: [PATCH 3/5] Bump lock version --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 88411777..30593de3 100644 --- a/composer.json +++ b/composer.json @@ -11,7 +11,7 @@ "sylius/mailer-bundle": "^1.8 || ^2.0", "sylius/sylius": "~1.10.0 || ~1.11.0 || ~1.12.0", "symfony/http-client": "5.4.* || ^6.0", - "symfony/lock": "5.4.*", + "symfony/lock": "5.4.* || ^6.0", "symfony/messenger": "5.4.* || ^6.0", "symfony/uid": "5.4.* || ^6.0", "symfony/webpack-encore-bundle": "^1.15" From 5ebd51ff44715f4632324e0c4bcc9d5e36f657ff Mon Sep 17 00:00:00 2001 From: Mateusz Zalewski Date: Tue, 20 Jun 2023 14:38:18 +0200 Subject: [PATCH 4/5] Handle it properly on capture --- config/services/actions.php | 2 ++ spec/Client/SaferpayClientBodyFactorySpec.php | 3 ++- src/Controller/Action/PrepareAssertAction.php | 3 ++- src/Controller/Action/PrepareCaptureAction.php | 16 +++++++++++++++- src/Processor/SaferpayPaymentProcessor.php | 6 +++--- .../SaferpayPaymentProcessorInterface.php | 12 ++++++++++++ src/Provider/PaymentProvider.php | 8 ++------ 7 files changed, 38 insertions(+), 12 deletions(-) create mode 100644 src/Processor/SaferpayPaymentProcessorInterface.php diff --git a/config/services/actions.php b/config/services/actions.php index 10b31470..f440e45d 100644 --- a/config/services/actions.php +++ b/config/services/actions.php @@ -56,6 +56,8 @@ ->args(['sylius.order']), service(PaymentProviderInterface::class), service(TokenProviderInterface::class), + service('monolog.logger.saferpay'), + service('router'), ]) ->tag('controller.service_arguments') ; diff --git a/spec/Client/SaferpayClientBodyFactorySpec.php b/spec/Client/SaferpayClientBodyFactorySpec.php index fe1268d4..d2544824 100644 --- a/spec/Client/SaferpayClientBodyFactorySpec.php +++ b/spec/Client/SaferpayClientBodyFactorySpec.php @@ -57,13 +57,14 @@ function it_creates_body_for_authorize_request( $payment->getCurrencyCode()->willReturn('CHF'); $order->getNumber()->willReturn('000000001'); $order->getCustomer()->willReturn($customer); + $order->getTokenValue()->willReturn('TOKEN'); $customer->getEmail()->willReturn('test@example.com'); $token->getAfterUrl()->willReturn('https://example.com/after'); $tokenProvider->provideForWebhook($payment, 'commerce_weavers_sylius_saferpay_webhook')->willReturn($webhookToken); $webhookToken->getHash()->willReturn('webhook_hash'); - $webhookRouteGenerator->generate('webhook_hash')->willReturn('https://example.com/webhook'); + $webhookRouteGenerator->generate('webhook_hash', 'TOKEN')->willReturn('https://example.com/webhook'); $this->createForAuthorize($payment, $token)->shouldReturn([ 'RequestHeader' => [ diff --git a/src/Controller/Action/PrepareAssertAction.php b/src/Controller/Action/PrepareAssertAction.php index 0948f152..c0d1a63f 100644 --- a/src/Controller/Action/PrepareAssertAction.php +++ b/src/Controller/Action/PrepareAssertAction.php @@ -10,6 +10,7 @@ use CommerceWeavers\SyliusSaferpayPlugin\Provider\PaymentProviderInterface; use Psr\Log\LoggerInterface; use Sylius\Bundle\ResourceBundle\Controller\RequestConfigurationFactoryInterface; +use Sylius\Component\Core\Model\PaymentInterface; use Sylius\Component\Resource\Metadata\MetadataInterface; use Symfony\Component\HttpFoundation\RedirectResponse; use Symfony\Component\HttpFoundation\Request; @@ -30,7 +31,7 @@ public function __construct( public function __invoke(Request $request, string $tokenValue): RedirectResponse { - $this->logger->debug('Synchronous processing started'); + $this->logger->debug('PrepareAssertAction: Synchronous processing started'); try { $payment = $this->paymentProvider->provideForOrder($tokenValue); diff --git a/src/Controller/Action/PrepareCaptureAction.php b/src/Controller/Action/PrepareCaptureAction.php index 94993380..efbcbaad 100644 --- a/src/Controller/Action/PrepareCaptureAction.php +++ b/src/Controller/Action/PrepareCaptureAction.php @@ -4,12 +4,15 @@ namespace CommerceWeavers\SyliusSaferpayPlugin\Controller\Action; +use CommerceWeavers\SyliusSaferpayPlugin\Exception\PaymentAlreadyProcessedException; use CommerceWeavers\SyliusSaferpayPlugin\Payum\Provider\TokenProviderInterface; use CommerceWeavers\SyliusSaferpayPlugin\Provider\PaymentProviderInterface; +use Psr\Log\LoggerInterface; use Sylius\Bundle\ResourceBundle\Controller\RequestConfigurationFactoryInterface; use Sylius\Component\Resource\Metadata\MetadataInterface; use Symfony\Component\HttpFoundation\RedirectResponse; use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\Routing\Generator\UrlGeneratorInterface; final class PrepareCaptureAction { @@ -18,13 +21,24 @@ public function __construct( private MetadataInterface $orderMetadata, private PaymentProviderInterface $paymentProvider, private TokenProviderInterface $tokenProvider, + private LoggerInterface $logger, + private UrlGeneratorInterface $router, ) { } public function __invoke(Request $request, string $tokenValue): RedirectResponse { $requestConfiguration = $this->requestConfigurationFactory->create($this->orderMetadata, $request); - $lastPayment = $this->paymentProvider->provideForCapture($tokenValue); + + try { + $lastPayment = $this->paymentProvider->provideForCapture($tokenValue); + } catch (PaymentAlreadyProcessedException) { + $this->logger->debug('Synchronous processing aborted - webhook handled the payment'); + + $request->getSession()->getFlashBag()->add('success', 'sylius.payment.completed'); + + return new RedirectResponse($this->router->generate('sylius_shop_order_thank_you')); + } $token = $this->tokenProvider->provideForCapture($lastPayment, $requestConfiguration); diff --git a/src/Processor/SaferpayPaymentProcessor.php b/src/Processor/SaferpayPaymentProcessor.php index 88816eaf..27191f09 100644 --- a/src/Processor/SaferpayPaymentProcessor.php +++ b/src/Processor/SaferpayPaymentProcessor.php @@ -12,7 +12,7 @@ use Symfony\Component\Lock\Exception\LockConflictedException; use Symfony\Component\Lock\LockFactory; -final class SaferpayPaymentProcessor +final class SaferpayPaymentProcessor implements SaferpayPaymentProcessorInterface { public function __construct( private LockFactory $lockFactory, @@ -21,7 +21,7 @@ public function __construct( ) { } - public function lock(PaymentInterface $payment): void + public function lock(PaymentInterface $payment, string $targetState = 'NEW'): void { $this->logger->debug('Trying to lock payment: ', ['id' => $payment->getId(), 'details' => $payment->getDetails()]); $lock = $this->lockFactory->createLock('payment_processing'); @@ -38,7 +38,7 @@ public function lock(PaymentInterface $payment): void if ( (isset($paymentDetails['processing']) && $paymentDetails['processing'] === true) || - (isset($paymentDetails['status']) && $paymentDetails['status'] !== 'NEW') + (isset($paymentDetails['status']) && $paymentDetails['status'] !== $targetState) ) { $this->logger->debug('Payment processing aborted: ', ['details' => $paymentDetails]); diff --git a/src/Processor/SaferpayPaymentProcessorInterface.php b/src/Processor/SaferpayPaymentProcessorInterface.php new file mode 100644 index 00000000..82baad0b --- /dev/null +++ b/src/Processor/SaferpayPaymentProcessorInterface.php @@ -0,0 +1,12 @@ +getLastPayment($state); if (null === $payment) { - /** @var string $orderTokenValue */ - $orderTokenValue = $order->getTokenValue(); - - throw new NotFoundHttpException( - sprintf('Order with token "%s" does not have an active payment.', $orderTokenValue), - ); + throw new PaymentAlreadyProcessedException(); } return $payment; From b4394883e90d551f44d0c853f036ad5b7520f868 Mon Sep 17 00:00:00 2001 From: Mateusz Zalewski Date: Wed, 21 Jun 2023 17:04:53 +0200 Subject: [PATCH 5/5] Fix tests and coding standards --- config/services.php | 3 +- config/services/actions.php | 5 +- .../Action/PrepareAssertActionSpec.php | 50 +++++++++++++++++-- .../Action/PrepareCaptureActionSpec.php | 35 ++++++++++++- spec/Payum/Action/AuthorizeActionSpec.php | 6 ++- spec/Payum/Provider/TokenProviderSpec.php | 14 ++++-- spec/Provider/PaymentProviderSpec.php | 5 +- .../Generator/WebhookUrlGeneratorSpec.php | 6 +-- src/Client/SaferpayClientBodyFactory.php | 2 +- src/Controller/Action/PrepareAssertAction.php | 10 ++-- .../Action/PrepareCaptureAction.php | 5 +- src/Controller/Action/WebhookAction.php | 1 + src/Payum/Provider/TokenProvider.php | 2 +- src/Provider/PaymentProvider.php | 6 ++- 14 files changed, 125 insertions(+), 25 deletions(-) diff --git a/config/services.php b/config/services.php index f8d86800..2d7428f1 100644 --- a/config/services.php +++ b/config/services.php @@ -3,6 +3,7 @@ declare(strict_types=1); use CommerceWeavers\SyliusSaferpayPlugin\Processor\SaferpayPaymentProcessor; +use CommerceWeavers\SyliusSaferpayPlugin\Processor\SaferpayPaymentProcessorInterface; use CommerceWeavers\SyliusSaferpayPlugin\Resolver\SaferpayApiBaseUrlResolver; use CommerceWeavers\SyliusSaferpayPlugin\Resolver\SaferpayApiBaseUrlResolverInterface; use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator; @@ -23,7 +24,7 @@ ; $services - ->set(SaferpayPaymentProcessor::class) + ->set(SaferpayPaymentProcessorInterface::class, SaferpayPaymentProcessor::class) ->public() ->args([ service('lock.factory'), diff --git a/config/services/actions.php b/config/services/actions.php index f440e45d..c937fee4 100644 --- a/config/services/actions.php +++ b/config/services/actions.php @@ -10,6 +10,7 @@ use CommerceWeavers\SyliusSaferpayPlugin\Payum\Factory\AssertFactoryInterface; use CommerceWeavers\SyliusSaferpayPlugin\Payum\Provider\TokenProviderInterface; use CommerceWeavers\SyliusSaferpayPlugin\Processor\SaferpayPaymentProcessor; +use CommerceWeavers\SyliusSaferpayPlugin\Processor\SaferpayPaymentProcessorInterface; use CommerceWeavers\SyliusSaferpayPlugin\Provider\PaymentProviderInterface; use Sylius\Component\Resource\Metadata\MetadataInterface; use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator; @@ -28,7 +29,7 @@ ->args(['sylius.order']), service(PaymentProviderInterface::class), service(TokenProviderInterface::class), - service(SaferpayPaymentProcessor::class), + service(SaferpayPaymentProcessorInterface::class), service('router'), service('monolog.logger.saferpay'), ]) @@ -81,7 +82,7 @@ service('sylius.command_bus'), service('monolog.logger.saferpay'), service(PaymentProviderInterface::class), - service(SaferpayPaymentProcessor::class), + service(SaferpayPaymentProcessorInterface::class), ]) ->tag('controller.service_arguments') ; diff --git a/spec/Controller/Action/PrepareAssertActionSpec.php b/spec/Controller/Action/PrepareAssertActionSpec.php index e41a48eb..56dd6fd1 100644 --- a/spec/Controller/Action/PrepareAssertActionSpec.php +++ b/spec/Controller/Action/PrepareAssertActionSpec.php @@ -4,18 +4,24 @@ namespace spec\CommerceWeavers\SyliusSaferpayPlugin\Controller\Action; +use CommerceWeavers\SyliusSaferpayPlugin\Exception\PaymentAlreadyProcessedException; use CommerceWeavers\SyliusSaferpayPlugin\Payum\Provider\TokenProviderInterface; +use CommerceWeavers\SyliusSaferpayPlugin\Processor\SaferpayPaymentProcessorInterface; use CommerceWeavers\SyliusSaferpayPlugin\Provider\PaymentProviderInterface; use Payum\Core\Security\TokenInterface; use PhpSpec\ObjectBehavior; use Prophecy\Argument; +use Psr\Log\LoggerInterface; use Sylius\Bundle\ResourceBundle\Controller\RequestConfiguration; use Sylius\Bundle\ResourceBundle\Controller\RequestConfigurationFactoryInterface; use Sylius\Component\Core\Model\PaymentInterface; use Sylius\Component\Resource\Metadata\MetadataInterface; use Symfony\Component\HttpFoundation\RedirectResponse; use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpFoundation\Session\Flash\FlashBagInterface; +use Symfony\Component\HttpFoundation\Session\Session; use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; +use Symfony\Component\Routing\Generator\UrlGeneratorInterface; final class PrepareAssertActionSpec extends ObjectBehavior { @@ -25,29 +31,67 @@ function let( PaymentProviderInterface $paymentProvider, TokenProviderInterface $tokenProvider, RequestConfiguration $requestConfiguration, + SaferpayPaymentProcessorInterface $saferpayPaymentProcessor, + UrlGeneratorInterface $router, + LoggerInterface $logger, ): void { $requestConfigurationFactory->create($orderMetadata, Argument::type(Request::class))->willReturn($requestConfiguration); - $this->beConstructedWith($requestConfigurationFactory, $orderMetadata, $paymentProvider, $tokenProvider); + $this->beConstructedWith( + $requestConfigurationFactory, + $orderMetadata, + $paymentProvider, + $tokenProvider, + $saferpayPaymentProcessor, + $router, + $logger, + ); } function it_throws_an_exception_when_last_payment_for_given_order_token_value_does_not_exist( PaymentProviderInterface $paymentProvider, + PaymentInterface $payment, Request $request, ): void { - $paymentProvider->provideForAssert('TOKEN')->willThrow(NotFoundHttpException::class); + $paymentProvider->provideForOrder('TOKEN')->willReturn($payment); + $paymentProvider->provideForAssert('TOKEN')->willThrow(PaymentAlreadyProcessedException::class); - $this->shouldThrow(NotFoundHttpException::class)->during('__invoke', [$request, 'TOKEN']); + $this->shouldThrow(PaymentAlreadyProcessedException::class)->during('__invoke', [$request, 'TOKEN']); + } + + function it_returns_to_thank_you_page_if_payment_is_already_processed( + PaymentProviderInterface $paymentProvider, + SaferpayPaymentProcessorInterface $saferpayPaymentProcessor, + UrlGeneratorInterface $router, + Request $request, + PaymentInterface $payment, + Session $session, + FlashBagInterface $flashBag + ): void { + $paymentProvider->provideForOrder('TOKEN')->willReturn($payment); + $saferpayPaymentProcessor->lock($payment)->willThrow(PaymentAlreadyProcessedException::class); + + $request->getSession()->willReturn($session); + $session->getFlashBag()->willReturn($flashBag); + $flashBag->add('success', 'sylius.payment.completed')->shouldBeCalled(); + + $router->generate('sylius_shop_order_thank_you')->willReturn('https://thank-you.com'); + + $this($request, 'TOKEN')->shouldBeLike(new RedirectResponse('https://thank-you.com')); } function it_returns_redirect_response_to_target_url_from_token( PaymentProviderInterface $paymentProvider, + SaferpayPaymentProcessorInterface $saferpayPaymentProcessor, TokenProviderInterface $tokenProvider, RequestConfiguration $requestConfiguration, Request $request, PaymentInterface $payment, TokenInterface $token, ): void { + $paymentProvider->provideForOrder('TOKEN')->willReturn($payment); + $saferpayPaymentProcessor->lock($payment)->shouldBeCalled(); + $paymentProvider->provideForAssert('TOKEN')->willReturn($payment); $tokenProvider->provideForAssert($payment, $requestConfiguration)->willReturn($token); $token->getTargetUrl()->willReturn('/url'); diff --git a/spec/Controller/Action/PrepareCaptureActionSpec.php b/spec/Controller/Action/PrepareCaptureActionSpec.php index ded62922..b2572a17 100644 --- a/spec/Controller/Action/PrepareCaptureActionSpec.php +++ b/spec/Controller/Action/PrepareCaptureActionSpec.php @@ -4,18 +4,23 @@ namespace spec\CommerceWeavers\SyliusSaferpayPlugin\Controller\Action; +use CommerceWeavers\SyliusSaferpayPlugin\Exception\PaymentAlreadyProcessedException; use CommerceWeavers\SyliusSaferpayPlugin\Payum\Provider\TokenProviderInterface; use CommerceWeavers\SyliusSaferpayPlugin\Provider\PaymentProviderInterface; use Payum\Core\Security\TokenInterface; use PhpSpec\ObjectBehavior; use Prophecy\Argument; +use Psr\Log\LoggerInterface; use Sylius\Bundle\ResourceBundle\Controller\RequestConfiguration; use Sylius\Bundle\ResourceBundle\Controller\RequestConfigurationFactoryInterface; use Sylius\Component\Core\Model\PaymentInterface; use Sylius\Component\Resource\Metadata\MetadataInterface; use Symfony\Component\HttpFoundation\RedirectResponse; use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpFoundation\Session\Flash\FlashBagInterface; +use Symfony\Component\HttpFoundation\Session\Session; use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; +use Symfony\Component\Routing\Generator\UrlGeneratorInterface; final class PrepareCaptureActionSpec extends ObjectBehavior { @@ -25,10 +30,19 @@ function let( PaymentProviderInterface $paymentProvider, TokenProviderInterface $tokenProvider, RequestConfiguration $requestConfiguration, + LoggerInterface $logger, + UrlGeneratorInterface $router, ): void { $requestConfigurationFactory->create($orderMetadata, Argument::type(Request::class))->willReturn($requestConfiguration); - $this->beConstructedWith($requestConfigurationFactory, $orderMetadata, $paymentProvider, $tokenProvider); + $this->beConstructedWith( + $requestConfigurationFactory, + $orderMetadata, + $paymentProvider, + $tokenProvider, + $logger, + $router, + ); } function it_throws_an_exception_when_last_payment_for_given_order_token_value_does_not_exist( @@ -40,6 +54,25 @@ function it_throws_an_exception_when_last_payment_for_given_order_token_value_do $this->shouldThrow(NotFoundHttpException::class)->during('__invoke', [$request, 'TOKEN']); } + function it_returns_to_thank_you_page_if_payment_is_already_processed( + PaymentProviderInterface $paymentProvider, + UrlGeneratorInterface $router, + Request $request, + PaymentInterface $payment, + Session $session, + FlashBagInterface $flashBag + ): void { + $paymentProvider->provideForCapture('TOKEN')->willThrow(PaymentAlreadyProcessedException::class); + + $request->getSession()->willReturn($session); + $session->getFlashBag()->willReturn($flashBag); + $flashBag->add('success', 'sylius.payment.completed')->shouldBeCalled(); + + $router->generate('sylius_shop_order_thank_you')->willReturn('https://thank-you.com'); + + $this($request, 'TOKEN')->shouldBeLike(new RedirectResponse('https://thank-you.com')); + } + function it_returns_redirect_response_to_target_url_from_token( PaymentProviderInterface $paymentProvider, TokenProviderInterface $tokenProvider, diff --git a/spec/Payum/Action/AuthorizeActionSpec.php b/spec/Payum/Action/AuthorizeActionSpec.php index 61fba2a9..a332f88e 100644 --- a/spec/Payum/Action/AuthorizeActionSpec.php +++ b/spec/Payum/Action/AuthorizeActionSpec.php @@ -129,7 +129,11 @@ function it_marks_the_payment_as_failed_if_there_is_different_status_code_than_o $saferpayClient->authorize($payment, $token)->willReturn($errorResponse); $errorResponse->getStatusCode()->willReturn(402); - $payment->setDetails(['status' => StatusAction::STATUS_FAILED])->shouldBeCalled(); + $payment->getDetails()->willReturn(['processing' => true]); + $payment + ->setDetails(['processing' => true, 'status' => StatusAction::STATUS_FAILED]) + ->shouldBeCalled() + ; $this->execute($request->getWrappedObject()); } diff --git a/spec/Payum/Provider/TokenProviderSpec.php b/spec/Payum/Provider/TokenProviderSpec.php index 6a86c4a9..95d307e6 100644 --- a/spec/Payum/Provider/TokenProviderSpec.php +++ b/spec/Payum/Provider/TokenProviderSpec.php @@ -11,6 +11,7 @@ use Sylius\Bundle\PayumBundle\Model\GatewayConfigInterface; use Sylius\Bundle\ResourceBundle\Controller\Parameters; use Sylius\Bundle\ResourceBundle\Controller\RequestConfiguration; +use Sylius\Component\Core\Model\OrderInterface; use Sylius\Component\Core\Model\PaymentInterface; use Sylius\Component\Core\Model\PaymentMethodInterface; @@ -154,21 +155,28 @@ function it_provides_token_for_capture_with_route_as_array( function it_provides_token_for_webhook( Payum $payum, - RequestConfiguration $requestConfiguration, - Parameters $parameters, GenericTokenFactoryInterface $tokenFactory, PaymentInterface $payment, TokenInterface $token, PaymentMethodInterface $paymentMethod, GatewayConfigInterface $gatewayConfig, + OrderInterface $order, ): void { $payment->getMethod()->willReturn($paymentMethod); $paymentMethod->getGatewayConfig()->willReturn($gatewayConfig); $gatewayConfig->getGatewayName()->willReturn('saferpay'); + $payment->getOrder()->willReturn($order); + $order->getTokenValue()->willReturn('TOKEN'); + $payum->getTokenFactory()->willReturn($tokenFactory); $tokenFactory - ->createToken('saferpay', $payment->getWrappedObject(), 'commerce_weavers_sylius_saferpay_webhook') + ->createToken( + 'saferpay', + $payment->getWrappedObject(), + 'commerce_weavers_sylius_saferpay_webhook', + ['order_token' => 'TOKEN'], + ) ->willReturn($token) ; $this->provideForWebhook($payment, 'commerce_weavers_sylius_saferpay_webhook')->shouldReturn($token); diff --git a/spec/Provider/PaymentProviderSpec.php b/spec/Provider/PaymentProviderSpec.php index 049862e6..2f75212d 100644 --- a/spec/Provider/PaymentProviderSpec.php +++ b/spec/Provider/PaymentProviderSpec.php @@ -4,6 +4,7 @@ namespace spec\CommerceWeavers\SyliusSaferpayPlugin\Provider; +use CommerceWeavers\SyliusSaferpayPlugin\Exception\PaymentAlreadyProcessedException; use CommerceWeavers\SyliusSaferpayPlugin\Provider\OrderProviderInterface; use PhpSpec\ObjectBehavior; use Sylius\Component\Core\Model\OrderInterface; @@ -27,7 +28,7 @@ function it_throws_an_exception_when_last_payment_with_new_state_does_not_exist_ $order->getTokenValue()->willReturn('TOKEN'); $this - ->shouldThrow(new NotFoundHttpException('Order with token "TOKEN" does not have an active payment.')) + ->shouldThrow(PaymentAlreadyProcessedException::class) ->during('provideForAssert', ['TOKEN']) ; } @@ -41,7 +42,7 @@ function it_throws_an_exception_when_last_payment_with_new_state_does_not_exist_ $order->getTokenValue()->willReturn('TOKEN'); $this - ->shouldThrow(new NotFoundHttpException('Order with token "TOKEN" does not have an active payment.')) + ->shouldThrow(PaymentAlreadyProcessedException::class) ->during('provideForCapture', ['TOKEN']) ; } diff --git a/spec/Routing/Generator/WebhookUrlGeneratorSpec.php b/spec/Routing/Generator/WebhookUrlGeneratorSpec.php index a9239f1b..4baa54c0 100644 --- a/spec/Routing/Generator/WebhookUrlGeneratorSpec.php +++ b/spec/Routing/Generator/WebhookUrlGeneratorSpec.php @@ -18,10 +18,10 @@ function it_generates_webhook_url(RouterInterface $router): void { $router->generate( 'commerce_weavers_sylius_saferpay_webhook', - ['payum_token' => 'abc123'], + ['payum_token' => 'abc123', 'order_token' => 'TOKEN'], 0, - )->shouldBeCalled()->willReturn('/saferpay/webhook/abc123'); + )->shouldBeCalled()->willReturn('/saferpay/webhook/abc123/TOKEN'); - $this->generate('abc123')->shouldReturn('/saferpay/webhook/abc123'); + $this->generate('abc123', 'TOKEN')->shouldReturn('/saferpay/webhook/abc123/TOKEN'); } } diff --git a/src/Client/SaferpayClientBodyFactory.php b/src/Client/SaferpayClientBodyFactory.php index 7a8f87dd..cd8cb452 100644 --- a/src/Client/SaferpayClientBodyFactory.php +++ b/src/Client/SaferpayClientBodyFactory.php @@ -40,7 +40,7 @@ public function createForAuthorize(PaymentInterface $payment, TokenInterface $to $allowedPaymentMethods = $config['allowed_payment_methods'] ?? []; $webhookToken = $this->tokenProvider->provideForWebhook($payment, self::COMMERCE_WEAVERS_SYLIUS_SAFERPAY_WEBHOOK); - $notificationUrl = $this->webhookRouteGenerator->generate($webhookToken->getHash(), $order->getTokenValue()); + $notificationUrl = $this->webhookRouteGenerator->generate($webhookToken->getHash(), (string) $order->getTokenValue()); return array_merge($this->provideBodyRequestHeader($gatewayConfig), [ 'TerminalId' => $terminalId, diff --git a/src/Controller/Action/PrepareAssertAction.php b/src/Controller/Action/PrepareAssertAction.php index c0d1a63f..4962d170 100644 --- a/src/Controller/Action/PrepareAssertAction.php +++ b/src/Controller/Action/PrepareAssertAction.php @@ -6,14 +6,14 @@ use CommerceWeavers\SyliusSaferpayPlugin\Exception\PaymentAlreadyProcessedException; use CommerceWeavers\SyliusSaferpayPlugin\Payum\Provider\TokenProviderInterface; -use CommerceWeavers\SyliusSaferpayPlugin\Processor\SaferpayPaymentProcessor; +use CommerceWeavers\SyliusSaferpayPlugin\Processor\SaferpayPaymentProcessorInterface; use CommerceWeavers\SyliusSaferpayPlugin\Provider\PaymentProviderInterface; use Psr\Log\LoggerInterface; use Sylius\Bundle\ResourceBundle\Controller\RequestConfigurationFactoryInterface; -use Sylius\Component\Core\Model\PaymentInterface; use Sylius\Component\Resource\Metadata\MetadataInterface; use Symfony\Component\HttpFoundation\RedirectResponse; use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpFoundation\Session\Session; use Symfony\Component\Routing\Generator\UrlGeneratorInterface; final class PrepareAssertAction @@ -23,7 +23,7 @@ public function __construct( private MetadataInterface $orderMetadata, private PaymentProviderInterface $paymentProvider, private TokenProviderInterface $tokenProvider, - private SaferpayPaymentProcessor $saferpayPaymentProcessor, + private SaferpayPaymentProcessorInterface $saferpayPaymentProcessor, private UrlGeneratorInterface $router, private LoggerInterface $logger, ) { @@ -39,7 +39,9 @@ public function __invoke(Request $request, string $tokenValue): RedirectResponse } catch (PaymentAlreadyProcessedException) { $this->logger->debug('Synchronous processing aborted - webhook handled the payment'); - $request->getSession()->getFlashBag()->add('success', 'sylius.payment.completed'); + /** @var Session $session */ + $session = $request->getSession(); + $session->getFlashBag()->add('success', 'sylius.payment.completed'); return new RedirectResponse($this->router->generate('sylius_shop_order_thank_you')); } diff --git a/src/Controller/Action/PrepareCaptureAction.php b/src/Controller/Action/PrepareCaptureAction.php index efbcbaad..f40f6288 100644 --- a/src/Controller/Action/PrepareCaptureAction.php +++ b/src/Controller/Action/PrepareCaptureAction.php @@ -12,6 +12,7 @@ use Sylius\Component\Resource\Metadata\MetadataInterface; use Symfony\Component\HttpFoundation\RedirectResponse; use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpFoundation\Session\Session; use Symfony\Component\Routing\Generator\UrlGeneratorInterface; final class PrepareCaptureAction @@ -35,7 +36,9 @@ public function __invoke(Request $request, string $tokenValue): RedirectResponse } catch (PaymentAlreadyProcessedException) { $this->logger->debug('Synchronous processing aborted - webhook handled the payment'); - $request->getSession()->getFlashBag()->add('success', 'sylius.payment.completed'); + /** @var Session $session */ + $session = $request->getSession(); + $session->getFlashBag()->add('success', 'sylius.payment.completed'); return new RedirectResponse($this->router->generate('sylius_shop_order_thank_you')); } diff --git a/src/Controller/Action/WebhookAction.php b/src/Controller/Action/WebhookAction.php index 25ef7eca..c90cccd2 100644 --- a/src/Controller/Action/WebhookAction.php +++ b/src/Controller/Action/WebhookAction.php @@ -31,6 +31,7 @@ public function __invoke(Request $request): Response { $this->logger->debug('Handling webhook started'); + /** @var string $orderToken */ $orderToken = $request->attributes->get('order_token'); $payment = $this->paymentProvider->provideForOrder($orderToken); diff --git a/src/Payum/Provider/TokenProvider.php b/src/Payum/Provider/TokenProvider.php index a3de5328..0a51c3e2 100644 --- a/src/Payum/Provider/TokenProvider.php +++ b/src/Payum/Provider/TokenProvider.php @@ -63,7 +63,7 @@ public function provideForWebhook(PaymentInterface $payment, string $webhookRout $this->getGatewayName($payment), $payment, $webhookRoute, - ['order_token' => $payment->getOrder()->getTokenValue()], + ['order_token' => $payment->getOrder()?->getTokenValue()], ); } diff --git a/src/Provider/PaymentProvider.php b/src/Provider/PaymentProvider.php index dd4a3d51..15942d87 100644 --- a/src/Provider/PaymentProvider.php +++ b/src/Provider/PaymentProvider.php @@ -7,7 +7,6 @@ use CommerceWeavers\SyliusSaferpayPlugin\Exception\PaymentAlreadyProcessedException; use Sylius\Component\Core\Model\OrderInterface; use Sylius\Component\Core\Model\PaymentInterface; -use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; final class PaymentProvider implements PaymentProviderInterface { @@ -26,7 +25,10 @@ public function provideForOrder(string $orderTokenValue): PaymentInterface { $order = $this->orderProvider->provideForAssert($orderTokenValue); - return $order->getLastPayment(); + /** @var PaymentInterface $payment */ + $payment = $order->getLastPayment(); + + return $payment; } public function provideForCapture(string $orderTokenValue): PaymentInterface