From 2c0981ae0c520c415d00173d248d99900652c3f7 Mon Sep 17 00:00:00 2001 From: TheMilek Date: Tue, 21 Apr 2026 11:31:55 +0200 Subject: [PATCH 1/2] Use state machine in PaymentWebhookController --- config/services/controller/shop.xml | 2 + .../Shop/PaymentWebhookController.php | 77 ++++++- .../Shop/PaymentWebhookControllerTest.php | 199 ++++++++++++++++++ 3 files changed, 272 insertions(+), 6 deletions(-) create mode 100644 tests/Unit/Controller/Shop/PaymentWebhookControllerTest.php diff --git a/config/services/controller/shop.xml b/config/services/controller/shop.xml index abf08caa..5d8cb8cf 100644 --- a/config/services/controller/shop.xml +++ b/config/services/controller/shop.xml @@ -64,6 +64,8 @@ + + diff --git a/src/Controller/Shop/PaymentWebhookController.php b/src/Controller/Shop/PaymentWebhookController.php index f3ab58a8..e1cf2533 100644 --- a/src/Controller/Shop/PaymentWebhookController.php +++ b/src/Controller/Shop/PaymentWebhookController.php @@ -13,12 +13,15 @@ namespace Sylius\MolliePlugin\Controller\Shop; +use Doctrine\ORM\EntityManagerInterface; use Mollie\Api\Exceptions\ApiException; use Mollie\Api\Resources\Payment; use Mollie\Api\Types\PaymentStatus; +use Sylius\Abstraction\StateMachine\StateMachineInterface; use Sylius\Component\Core\Repository\PaymentRepositoryInterface; use Sylius\Component\Order\Repository\OrderRepositoryInterface; use Sylius\Component\Payment\Model\PaymentInterface; +use Sylius\Component\Payment\PaymentTransitions; use Sylius\MolliePlugin\Client\MollieApiClient; use Sylius\MolliePlugin\Entity\OrderInterface; use Sylius\MolliePlugin\Logger\MollieLoggerActionInterface; @@ -33,8 +36,10 @@ public function __construct( private readonly MollieApiClient $mollieApiClient, private readonly MollieApiClientKeyResolverInterface $apiClientKeyResolver, private readonly OrderRepositoryInterface $orderRepository, - private readonly PaymentRepositoryInterface $paymentRepository, + private readonly ?PaymentRepositoryInterface $paymentRepository = null, private readonly ?MollieLoggerActionInterface $logger = null, + private readonly ?StateMachineInterface $stateMachine = null, + private readonly ?EntityManagerInterface $entityManager = null, ) { if (null === $this->logger) { trigger_deprecation( @@ -44,6 +49,26 @@ public function __construct( self::class, ); } + + if (null === $this->stateMachine || null === $this->entityManager) { + trigger_deprecation( + 'sylius/mollie-plugin', + '3.3', + 'Not passing StateMachineInterface and EntityManagerInterface to %s is deprecated and will be required from 4.0. ' . + 'State changes currently fall back to direct Payment::setState() which bypasses state machine guards and after-callbacks (e.g. auto-creation of a new payment on fail/cancel).', + self::class, + ); + } + + if (null !== $this->paymentRepository) { + trigger_deprecation( + 'sylius/mollie-plugin', + '3.3', + 'Passing PaymentRepositoryInterface to %s is deprecated and will be removed in 4.0. ' . + 'It is only used by the setState() fallback path which is itself deprecated — prefer StateMachineInterface.', + self::class, + ); + } } public function __invoke(Request $request): Response @@ -71,24 +96,64 @@ public function __invoke(Request $request): Response } $payment = $order->getLastPayment(); - $status = $this->getStatus($molliePayment); + if (null === $payment) { + return new JsonResponse(Response::HTTP_OK); + } + + if (null !== $this->stateMachine && null !== $this->entityManager) { + $this->applyTransition($payment, $molliePayment->status); + } else { + $this->applyLegacyState($payment, $molliePayment); + } + + return new JsonResponse(Response::HTTP_OK); + } + + private function applyTransition(PaymentInterface $payment, string $mollieStatus): void + { + $transition = $this->mapMolliePaymentStatusToTransition($mollieStatus); + if (null === $transition) { + return; + } + + if (!$this->stateMachine->can($payment, PaymentTransitions::GRAPH, $transition)) { + return; + } + + $this->stateMachine->apply($payment, PaymentTransitions::GRAPH, $transition); + $this->entityManager->flush(); + } + + private function applyLegacyState(PaymentInterface $payment, Payment $molliePayment): void + { + $status = $this->mapMolliePaymentStatusToState($molliePayment); if ($payment->getState() !== $status && PaymentInterface::STATE_UNKNOWN !== $status) { $payment->setState($status); $this->paymentRepository->add($payment); } + } - return new JsonResponse(Response::HTTP_OK); + private function mapMolliePaymentStatusToTransition(string $status): ?string + { + return match ($status) { + PaymentStatus::STATUS_PENDING, PaymentStatus::STATUS_OPEN => PaymentTransitions::TRANSITION_PROCESS, + PaymentStatus::STATUS_AUTHORIZED => PaymentTransitions::TRANSITION_AUTHORIZE, + PaymentStatus::STATUS_PAID => PaymentTransitions::TRANSITION_COMPLETE, + PaymentStatus::STATUS_CANCELED, PaymentStatus::STATUS_EXPIRED => PaymentTransitions::TRANSITION_CANCEL, + PaymentStatus::STATUS_FAILED => PaymentTransitions::TRANSITION_FAIL, + default => null, + }; } - private function getStatus(Payment $molliePayment): string + private function mapMolliePaymentStatusToState(Payment $molliePayment): string { return match ($molliePayment->status) { PaymentStatus::STATUS_PENDING, PaymentStatus::STATUS_OPEN => PaymentInterface::STATE_PROCESSING, PaymentStatus::STATUS_AUTHORIZED => PaymentInterface::STATE_AUTHORIZED, PaymentStatus::STATUS_PAID => PaymentInterface::STATE_COMPLETED, - PaymentStatus::STATUS_CANCELED => PaymentInterface::STATE_CANCELLED, - PaymentStatus::STATUS_EXPIRED, PaymentStatus::STATUS_FAILED => PaymentInterface::STATE_FAILED, + PaymentStatus::STATUS_CANCELED, PaymentStatus::STATUS_EXPIRED => PaymentInterface::STATE_CANCELLED, + PaymentStatus::STATUS_FAILED => PaymentInterface::STATE_FAILED, default => PaymentInterface::STATE_UNKNOWN, }; } diff --git a/tests/Unit/Controller/Shop/PaymentWebhookControllerTest.php b/tests/Unit/Controller/Shop/PaymentWebhookControllerTest.php new file mode 100644 index 00000000..e7c22e82 --- /dev/null +++ b/tests/Unit/Controller/Shop/PaymentWebhookControllerTest.php @@ -0,0 +1,199 @@ +mollieApiClient = $this->createMock(MollieApiClient::class); + $this->apiClientKeyResolver = $this->createMock(MollieApiClientKeyResolverInterface::class); + $this->orderRepository = $this->createMock(OrderRepositoryInterface::class); + $this->paymentRepository = $this->createMock(PaymentRepositoryInterface::class); + $this->logger = $this->createMock(MollieLoggerActionInterface::class); + $this->stateMachine = $this->createMock(StateMachineInterface::class); + $this->entityManager = $this->createMock(EntityManagerInterface::class); + + $this->paymentEndpoint = $this->createMock(PaymentEndpoint::class); + $this->mollieApiClient->payments = $this->paymentEndpoint; + + $this->mollieApiClient->method('getApiKey')->willReturn('test_key'); + $this->apiClientKeyResolver->method('getClientWithKey')->willReturn($this->mollieApiClient); + } + + public function testItReturnsOkWhenOrderIsNotFound(): void + { + $controller = $this->createController(); + + $request = new Request(['id' => 'tr_abc', 'orderId' => '42']); + $this->paymentEndpoint->expects(self::once())->method('get')->willReturn($this->makeMolliePayment(PaymentStatus::STATUS_PAID)); + $this->orderRepository->expects(self::once())->method('findOneBy')->with(['id' => '42'])->willReturn(null); + $this->stateMachine->expects(self::never())->method('apply'); + + $response = $controller->__invoke($request); + + self::assertSame(Response::HTTP_OK, $response->getStatusCode()); + } + + public function testItReturnsOkWhenOrderHasNoPayment(): void + { + $controller = $this->createController(); + + $request = new Request(['id' => 'tr_abc', 'orderId' => '42']); + $order = $this->createMock(OrderInterface::class); + $order->method('getLastPayment')->willReturn(null); + $this->paymentEndpoint->expects(self::once())->method('get')->willReturn($this->makeMolliePayment(PaymentStatus::STATUS_PAID)); + $this->orderRepository->method('findOneBy')->willReturn($order); + $this->stateMachine->expects(self::never())->method('apply'); + + $response = $controller->__invoke($request); + + self::assertSame(Response::HTTP_OK, $response->getStatusCode()); + } + + public function testItAppliesCompleteTransitionForPaidMolliePayment(): void + { + $controller = $this->createController(); + + $request = new Request(['id' => 'tr_abc', 'orderId' => '42']); + $payment = $this->createMock(CorePaymentInterface::class); + $order = $this->createMock(OrderInterface::class); + $order->method('getLastPayment')->willReturn($payment); + + $this->paymentEndpoint->method('get')->willReturn($this->makeMolliePayment(PaymentStatus::STATUS_PAID)); + $this->orderRepository->method('findOneBy')->willReturn($order); + + $this->stateMachine->expects(self::once()) + ->method('can') + ->with($payment, PaymentTransitions::GRAPH, PaymentTransitions::TRANSITION_COMPLETE) + ->willReturn(true); + $this->stateMachine->expects(self::once()) + ->method('apply') + ->with($payment, PaymentTransitions::GRAPH, PaymentTransitions::TRANSITION_COMPLETE); + $this->entityManager->expects(self::once())->method('flush'); + + $response = $controller->__invoke($request); + + self::assertSame(Response::HTTP_OK, $response->getStatusCode()); + } + + public function testItSkipsTransitionWhenStateMachineRejectsIt(): void + { + $controller = $this->createController(); + + $request = new Request(['id' => 'tr_abc', 'orderId' => '42']); + $payment = $this->createMock(CorePaymentInterface::class); + $order = $this->createMock(OrderInterface::class); + $order->method('getLastPayment')->willReturn($payment); + + $this->paymentEndpoint->method('get')->willReturn($this->makeMolliePayment(PaymentStatus::STATUS_PAID)); + $this->orderRepository->method('findOneBy')->willReturn($order); + + $this->stateMachine->method('can')->willReturn(false); + $this->stateMachine->expects(self::never())->method('apply'); + $this->entityManager->expects(self::never())->method('flush'); + + $response = $controller->__invoke($request); + + self::assertSame(Response::HTTP_OK, $response->getStatusCode()); + } + + /** + * @group legacy + */ + public function testItFallsBackToSetStateWhenStateMachineIsNotProvided(): void + { + $controller = new PaymentWebhookController( + $this->mollieApiClient, + $this->apiClientKeyResolver, + $this->orderRepository, + $this->paymentRepository, + $this->logger, + ); + + $request = new Request(['id' => 'tr_abc', 'orderId' => '42']); + $payment = $this->createMock(CorePaymentInterface::class); + $payment->method('getState')->willReturn(PaymentInterface::STATE_NEW); + $payment->expects(self::once())->method('setState')->with(PaymentInterface::STATE_COMPLETED); + + $order = $this->createMock(OrderInterface::class); + $order->method('getLastPayment')->willReturn($payment); + + $this->paymentEndpoint->method('get')->willReturn($this->makeMolliePayment(PaymentStatus::STATUS_PAID)); + $this->orderRepository->method('findOneBy')->willReturn($order); + $this->paymentRepository->expects(self::once())->method('add')->with($payment); + + $response = $controller->__invoke($request); + + self::assertSame(Response::HTTP_OK, $response->getStatusCode()); + } + + private function createController(): PaymentWebhookController + { + return new PaymentWebhookController( + $this->mollieApiClient, + $this->apiClientKeyResolver, + $this->orderRepository, + $this->paymentRepository, + $this->logger, + $this->stateMachine, + $this->entityManager, + ); + } + + private function makeMolliePayment(string $status): Payment + { + $payment = new Payment($this->mollieApiClient); + $payment->id = 'tr_abc'; + $payment->status = $status; + + return $payment; + } +} From 289db82c2291ef5f94ce91fe279dd6173f93888c Mon Sep 17 00:00:00 2001 From: TheMilek Date: Tue, 21 Apr 2026 16:12:04 +0200 Subject: [PATCH 2/2] Add comment about 200 responses --- src/Controller/Shop/PaymentWebhookController.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/Controller/Shop/PaymentWebhookController.php b/src/Controller/Shop/PaymentWebhookController.php index e1cf2533..4e19d3eb 100644 --- a/src/Controller/Shop/PaymentWebhookController.php +++ b/src/Controller/Shop/PaymentWebhookController.php @@ -71,6 +71,13 @@ public function __construct( } } + /** + * Every exit path returns 200 because Mollie keeps retrying the webhook on any + * non-2xx response — "for any other response we keep trying" + * (https://docs.mollie.com/docs/accepting-payments). When we cannot meaningfully + * act (unknown Mollie id, unknown order, order has no payment, unsupported + * status), a retry would not change the outcome, so we acknowledge and move on. + */ public function __invoke(Request $request): Response { $this->mollieApiClient->setApiKey($this->apiClientKeyResolver->getClientWithKey()->getApiKey());