From cda80990c77abaed5c07cb147357a6aa616e88a2 Mon Sep 17 00:00:00 2001 From: Serreau Jovann Date: Sat, 4 Apr 2026 10:19:40 +0200 Subject: [PATCH] Remove pending orders sync from StripeSyncCommand and add pessimistic lock to webhook payment handler - Remove syncPendingOrders and its helpers (handleSucceeded, handleCancelled, handleFailed) from StripeSyncCommand - Clean up unused dependencies (BilletOrderService, MailerService, AuditService, BilletBuyer) - Add PESSIMISTIC_WRITE lock in handlePaymentIntentSucceeded to prevent duplicate ticket generation when Stripe sends concurrent webhook calls - Update tests to match simplified command Co-Authored-By: Claude Opus 4.6 (1M context) --- src/Command/StripeSyncCommand.php | 121 +--------- src/Controller/StripeWebhookController.php | 18 ++ tests/Command/StripeSyncCommandTest.php | 252 --------------------- 3 files changed, 19 insertions(+), 372 deletions(-) diff --git a/src/Command/StripeSyncCommand.php b/src/Command/StripeSyncCommand.php index 6a68708..2b56c8d 100644 --- a/src/Command/StripeSyncCommand.php +++ b/src/Command/StripeSyncCommand.php @@ -2,11 +2,7 @@ namespace App\Command; -use App\Entity\BilletBuyer; use App\Entity\User; -use App\Service\AuditService; -use App\Service\BilletOrderService; -use App\Service\MailerService; use App\Service\StripeService; use Doctrine\ORM\EntityManagerInterface; use Symfony\Component\Console\Attribute\AsCommand; @@ -17,16 +13,13 @@ use Symfony\Component\Console\Style\SymfonyStyle; #[AsCommand( name: 'app:stripe:sync', - description: 'Sync Stripe account status for all organizers and reconcile pending orders', + description: 'Sync Stripe account status for all organizers', )] class StripeSyncCommand extends Command { public function __construct( private EntityManagerInterface $em, private StripeService $stripeService, - private BilletOrderService $billetOrderService, - private MailerService $mailerService, - private AuditService $audit, ) { parent::__construct(); } @@ -36,7 +29,6 @@ class StripeSyncCommand extends Command $io = new SymfonyStyle($input, $output); $hasErrors = $this->syncAccounts($io); - $hasErrors = $this->syncPendingOrders($io) || $hasErrors; return $hasErrors ? Command::FAILURE : Command::SUCCESS; } @@ -101,115 +93,4 @@ class StripeSyncCommand extends Command return $errors > 0; } - private function syncPendingOrders(SymfonyStyle $io): bool - { - $pendingOrders = $this->em->getRepository(BilletBuyer::class)->findBy([ - 'status' => BilletBuyer::STATUS_PENDING, - ]); - - if (0 === \count($pendingOrders)) { - $io->info('No pending orders to reconcile.'); - - return false; - } - - $io->info(sprintf('Checking %d pending order(s) on Stripe...', \count($pendingOrders))); - - $processed = 0; - $errors = 0; - - foreach ($pendingOrders as $order) { - $paymentIntentId = $order->getStripeSessionId(); - - $stripeAccountId = $order->getEvent()?->getAccount()?->getStripeAccountId(); - - if (!$paymentIntentId || !$stripeAccountId) { - $io->text(sprintf(' [SKIP] Order #%s — no payment intent ID or Stripe account', $order->getOrderNumber())); - continue; - } - - try { - $paymentIntent = $this->stripeService->retrievePaymentIntent($paymentIntentId, $stripeAccountId); - $stripeStatus = $paymentIntent->status; - - match ($stripeStatus) { - 'succeeded' => $this->handleSucceeded($order, $paymentIntent, $io), - 'canceled' => $this->handleCancelled($order, $io), - 'requires_payment_method' => $this->handleFailed($order, $paymentIntent, $io), - default => $io->text(sprintf( - ' [PENDING] Order #%s — Stripe status: %s', - $order->getOrderNumber(), - $stripeStatus, - )), - }; - - ++$processed; - } catch (\Throwable $e) { - $io->text(sprintf( - ' [ERROR] Order #%s — %s', - $order->getOrderNumber(), - $e->getMessage(), - )); - ++$errors; - } - } - - $this->em->flush(); - - $io->success(sprintf('Orders: %d checked, %d error(s).', $processed, $errors)); - - return $errors > 0; - } - - private function handleSucceeded(BilletBuyer $order, \Stripe\PaymentIntent $paymentIntent, SymfonyStyle $io): void - { - $debtOrganizerId = $paymentIntent->metadata->debt_organizer_id ?? null; - if ($debtOrganizerId) { - $organizer = $this->em->getRepository(User::class)->find((int) $debtOrganizerId); - if ($organizer) { - $organizer->reduceDebt($paymentIntent->amount ?? 0); - } - } - - $this->billetOrderService->generateOrderTickets($order); - $this->billetOrderService->generateAndSendTickets($order); - $this->billetOrderService->notifyOrganizer($order); - - $io->text(sprintf(' [PAID] Order #%s — tickets generated and sent', $order->getOrderNumber())); - } - - private function handleCancelled(BilletBuyer $order, SymfonyStyle $io): void - { - $order->setStatus(BilletBuyer::STATUS_CANCELLED); - $this->em->flush(); - - $this->audit->log('payment_cancelled_sync', 'BilletBuyer', $order->getId(), [ - 'orderNumber' => $order->getOrderNumber(), - ]); - - $io->text(sprintf(' [CANCELLED] Order #%s', $order->getOrderNumber())); - } - - private function handleFailed(BilletBuyer $order, \Stripe\PaymentIntent $paymentIntent, SymfonyStyle $io): void - { - $errorMessage = $paymentIntent->last_payment_error->message ?? 'Paiement refuse'; - - $order->setStatus(BilletBuyer::STATUS_CANCELLED); - $this->em->flush(); - - $this->audit->log('payment_failed_sync', 'BilletBuyer', $order->getId(), [ - 'orderNumber' => $order->getOrderNumber(), - 'error' => $errorMessage, - ]); - - if ($order->getEmail()) { - $this->mailerService->sendEmail( - $order->getEmail(), - 'Echec de paiement - '.$order->getEvent()->getTitle(), - 'Votre paiement pour la commande '.$order->getOrderNumber().' a echoue : '.$errorMessage, - ); - } - - $io->text(sprintf(' [FAILED] Order #%s — %s', $order->getOrderNumber(), $errorMessage)); - } } diff --git a/src/Controller/StripeWebhookController.php b/src/Controller/StripeWebhookController.php index 1f0fdae..ad674ff 100644 --- a/src/Controller/StripeWebhookController.php +++ b/src/Controller/StripeWebhookController.php @@ -12,6 +12,7 @@ use App\Service\BilletOrderService; use App\Service\MailerService; use App\Service\PayoutPdfService; use App\Service\StripeService; +use Doctrine\DBAL\LockMode; use Doctrine\ORM\EntityManagerInterface; use Psr\Cache\CacheItemPoolInterface; use Symfony\Bundle\FrameworkBundle\Controller\AbstractController; @@ -157,6 +158,23 @@ class StripeWebhookController extends AbstractController return; } + $canProcess = $em->wrapInTransaction(function () use ($order, $em): bool { + $em->refresh($order, LockMode::PESSIMISTIC_WRITE); + + if (BilletBuyer::STATUS_PENDING !== $order->getStatus()) { + return false; + } + + $order->setStatus(BilletBuyer::STATUS_PAID); + $order->setPaidAt(new \DateTimeImmutable()); + + return true; + }); + + if (!$canProcess) { + return; + } + $debtOrganizerId = $paymentIntent->metadata->debt_organizer_id ?? null; if ($debtOrganizerId) { $organizer = $em->getRepository(User::class)->find((int) $debtOrganizerId); diff --git a/tests/Command/StripeSyncCommandTest.php b/tests/Command/StripeSyncCommandTest.php index f6d124b..ef29cfd 100644 --- a/tests/Command/StripeSyncCommandTest.php +++ b/tests/Command/StripeSyncCommandTest.php @@ -3,12 +3,7 @@ namespace App\Tests\Command; use App\Command\StripeSyncCommand; -use App\Entity\BilletBuyer; -use App\Entity\Event; use App\Entity\User; -use App\Service\AuditService; -use App\Service\BilletOrderService; -use App\Service\MailerService; use App\Service\StripeService; use Doctrine\ORM\EntityManagerInterface; use Doctrine\ORM\EntityRepository; @@ -19,29 +14,19 @@ use Symfony\Component\Console\Tester\CommandTester; class StripeSyncCommandTest extends TestCase { private StripeService $stripeService; - private BilletOrderService $billetOrderService; - private MailerService $mailerService; - private AuditService $audit; private EntityManagerInterface $em; private EntityRepository $userRepo; - private EntityRepository $buyerRepo; protected function setUp(): void { $this->stripeService = $this->createMock(StripeService::class); - $this->billetOrderService = $this->createMock(BilletOrderService::class); - $this->mailerService = $this->createMock(MailerService::class); - $this->audit = $this->createMock(AuditService::class); $this->userRepo = $this->createMock(EntityRepository::class); - $this->buyerRepo = $this->createMock(EntityRepository::class); - $this->buyerRepo->method('findBy')->willReturn([]); $this->em = $this->createMock(EntityManagerInterface::class); $this->em->method('getRepository')->willReturnCallback(function (string $class) { return match ($class) { User::class => $this->userRepo, - BilletBuyer::class => $this->buyerRepo, default => $this->createMock(EntityRepository::class), }; }); @@ -63,29 +48,11 @@ class StripeSyncCommandTest extends TestCase return $user; } - private function createPendingOrder(?string $paymentIntentId = null): BilletBuyer - { - $event = $this->createMock(Event::class); - $event->method('getTitle')->willReturn('Test Event'); - $event->method('getAccount')->willReturn($this->createOrganizer('acct_org')); - - $order = new BilletBuyer(); - $order->setStatus(BilletBuyer::STATUS_PENDING); - $order->setStripeSessionId($paymentIntentId); - $order->setEmail('buyer@test.fr'); - $order->setEvent($event); - - return $order; - } - private function createCommandTester(): CommandTester { $command = new StripeSyncCommand( $this->em, $this->stripeService, - $this->billetOrderService, - $this->mailerService, - $this->audit, ); $app = new Application(); @@ -94,23 +61,6 @@ class StripeSyncCommandTest extends TestCase return new CommandTester($app->find('app:stripe:sync')); } - private function setBuyerRepo(array $orders): void - { - $this->buyerRepo = $this->createMock(EntityRepository::class); - $this->buyerRepo->method('findBy')->willReturn($orders); - - $this->em = $this->createMock(EntityManagerInterface::class); - $this->em->method('getRepository')->willReturnCallback(function (string $class) { - return match ($class) { - User::class => $this->userRepo, - BilletBuyer::class => $this->buyerRepo, - default => $this->createMock(EntityRepository::class), - }; - }); - } - - // --- Account sync tests --- - public function testSyncUpdatesStripeStatus(): void { $user = $this->createOrganizer('acct_123'); @@ -195,206 +145,4 @@ class StripeSyncCommandTest extends TestCase self::assertTrue($userWithStripe->isStripeChargesEnabled()); self::assertFalse($userWithStripe->isStripePayoutsEnabled()); } - - // --- Pending orders sync tests --- - - public function testNoPendingOrders(): void - { - $this->userRepo->method('findAll')->willReturn([]); - - $tester = $this->createCommandTester(); - $tester->execute([]); - - self::assertStringContainsString('No pending orders', $tester->getDisplay()); - } - - public function testPendingOrderSucceeded(): void - { - $this->userRepo->method('findAll')->willReturn([]); - - $order = $this->createPendingOrder('pi_succeeded'); - $this->setBuyerRepo([$order]); - - $paymentIntent = \Stripe\PaymentIntent::constructFrom([ - 'id' => 'pi_succeeded', - 'status' => 'succeeded', - 'amount' => 5000, - 'metadata' => [], - ]); - - $this->stripeService->method('retrievePaymentIntent') - ->with('pi_succeeded') - ->willReturn($paymentIntent); - - $this->billetOrderService->expects(self::once())->method('generateOrderTickets')->with($order); - $this->billetOrderService->expects(self::once())->method('generateAndSendTickets')->with($order); - $this->billetOrderService->expects(self::once())->method('notifyOrganizer')->with($order); - - $tester = $this->createCommandTester(); - $tester->execute([]); - - self::assertStringContainsString('PAID', $tester->getDisplay()); - } - - public function testPendingOrderSucceededWithDebtOrganizer(): void - { - $organizer = $this->createOrganizer('acct_debt'); - $this->userRepo->method('findAll')->willReturn([]); - $this->userRepo->method('find')->with(42)->willReturn($organizer); - - $order = $this->createPendingOrder('pi_debt'); - $this->setBuyerRepo([$order]); - - $paymentIntent = \Stripe\PaymentIntent::constructFrom([ - 'id' => 'pi_debt', - 'status' => 'succeeded', - 'amount' => 3000, - 'metadata' => ['debt_organizer_id' => '42'], - ]); - - $this->stripeService->method('retrievePaymentIntent')->willReturn($paymentIntent); - - $tester = $this->createCommandTester(); - $tester->execute([]); - - self::assertStringContainsString('PAID', $tester->getDisplay()); - } - - public function testPendingOrderCanceled(): void - { - $this->userRepo->method('findAll')->willReturn([]); - - $order = $this->createPendingOrder('pi_canceled'); - $this->setBuyerRepo([$order]); - - $paymentIntent = \Stripe\PaymentIntent::constructFrom([ - 'id' => 'pi_canceled', - 'status' => 'canceled', - ]); - - $this->stripeService->method('retrievePaymentIntent')->willReturn($paymentIntent); - - $this->audit->expects(self::once())->method('log') - ->with('payment_cancelled_sync', 'BilletBuyer', self::anything(), self::anything()); - - $tester = $this->createCommandTester(); - $tester->execute([]); - - self::assertSame(BilletBuyer::STATUS_CANCELLED, $order->getStatus()); - self::assertStringContainsString('CANCELLED', $tester->getDisplay()); - } - - public function testPendingOrderFailed(): void - { - $this->userRepo->method('findAll')->willReturn([]); - - $order = $this->createPendingOrder('pi_failed'); - $this->setBuyerRepo([$order]); - - $paymentIntent = \Stripe\PaymentIntent::constructFrom([ - 'id' => 'pi_failed', - 'status' => 'requires_payment_method', - 'last_payment_error' => ['message' => 'Card declined'], - ]); - - $this->stripeService->method('retrievePaymentIntent')->willReturn($paymentIntent); - - $this->audit->expects(self::once())->method('log') - ->with('payment_failed_sync', 'BilletBuyer', self::anything(), self::anything()); - $this->mailerService->expects(self::once())->method('sendEmail'); - - $tester = $this->createCommandTester(); - $tester->execute([]); - - self::assertSame(BilletBuyer::STATUS_CANCELLED, $order->getStatus()); - self::assertStringContainsString('FAILED', $tester->getDisplay()); - } - - public function testPendingOrderSkippedWithoutPaymentIntentId(): void - { - $this->userRepo->method('findAll')->willReturn([]); - - $order = $this->createPendingOrder(null); - $this->setBuyerRepo([$order]); - - $this->stripeService->expects(self::never())->method('retrievePaymentIntent'); - - $tester = $this->createCommandTester(); - $tester->execute([]); - - self::assertStringContainsString('SKIP', $tester->getDisplay()); - } - - public function testPendingOrderStillPending(): void - { - $this->userRepo->method('findAll')->willReturn([]); - - $order = $this->createPendingOrder('pi_processing'); - $this->setBuyerRepo([$order]); - - $paymentIntent = \Stripe\PaymentIntent::constructFrom([ - 'id' => 'pi_processing', - 'status' => 'processing', - ]); - - $this->stripeService->method('retrievePaymentIntent')->willReturn($paymentIntent); - - $tester = $this->createCommandTester(); - $tester->execute([]); - - self::assertSame(BilletBuyer::STATUS_PENDING, $order->getStatus()); - self::assertStringContainsString('PENDING', $tester->getDisplay()); - } - - public function testPendingOrderStripeApiError(): void - { - $this->userRepo->method('findAll')->willReturn([]); - - $order = $this->createPendingOrder('pi_error'); - $this->setBuyerRepo([$order]); - - $this->stripeService->method('retrievePaymentIntent') - ->willThrowException(new \RuntimeException('Stripe API error')); - - $tester = $this->createCommandTester(); - $tester->execute([]); - - self::assertStringContainsString('ERROR', $tester->getDisplay()); - self::assertSame(1, $tester->getStatusCode()); - } - - public function testPendingOrderFailedWithoutEmail(): void - { - $this->userRepo->method('findAll')->willReturn([]); - - $organizer = $this->createOrganizer('acct_no_email'); - $event = $this->createMock(Event::class); - $event->method('getTitle')->willReturn('Test Event'); - $event->method('getAccount')->willReturn($organizer); - - $order = new BilletBuyer(); - $order->setStatus(BilletBuyer::STATUS_PENDING); - $order->setStripeSessionId('pi_no_email'); - $order->setEvent($event); - - $this->setBuyerRepo([$order]); - - $paymentIntent = \Stripe\PaymentIntent::constructFrom([ - 'id' => 'pi_no_email', - 'status' => 'requires_payment_method', - 'last_payment_error' => null, - ]); - - $this->stripeService->method('retrievePaymentIntent')->willReturn($paymentIntent); - - $this->audit->expects(self::once())->method('log') - ->with('payment_failed_sync', 'BilletBuyer', self::anything(), self::anything()); - $this->mailerService->expects(self::never())->method('sendEmail'); - - $tester = $this->createCommandTester(); - $tester->execute([]); - - self::assertSame(BilletBuyer::STATUS_CANCELLED, $order->getStatus()); - self::assertStringContainsString('FAILED', $tester->getDisplay()); - } }