From c12ba13c4b5d3176d2cc413c4a123680b32cc47c Mon Sep 17 00:00:00 2001 From: smarcet Date: Thu, 8 Oct 2026 02:16:33 -0300 Subject: [PATCH 1/4] test(registration): reproduce double slot release on order cancel and revoke Service-level tests with a mocked payment gateway covering the five release patterns seen on 2026-10-07: cancel twice, cancel on a paid order, cancel while the PaymentIntent is in flight or unreadable, late payment_failed and succeeded webhooks on a cancelled order, and the revoke/confirm crons acting on an order the widget cancelled after the listing. Plus LawPayApi::isDeclined for VOIDED and FAILED charges. --- tests/SummitOrderServiceTest.php | 466 ++++++++++++++++++++ tests/Unit/Services/LawPayApiStatusTest.php | 74 ++++ 2 files changed, 540 insertions(+) create mode 100644 tests/Unit/Services/LawPayApiStatusTest.php diff --git a/tests/SummitOrderServiceTest.php b/tests/SummitOrderServiceTest.php index c6ccd2872..2f7cbff27 100644 --- a/tests/SummitOrderServiceTest.php +++ b/tests/SummitOrderServiceTest.php @@ -30,10 +30,14 @@ use App\Services\Model\ISummitOrderService; use App\Services\Model\Strategies\TicketFinder\ITicketFinderStrategyFactory; use App\Services\Model\SummitOrderService; +use App\Services\Apis\CartAlreadyPaidException; +use App\Services\Apis\IPaymentGatewayAPI; use App\Services\Utils\ILockManagerService; use Illuminate\Support\Facades\App; use Illuminate\Support\Facades\Cache; use Illuminate\Support\Facades\Config; +use Illuminate\Support\Facades\DB; +use Illuminate\Support\Facades\Log; use Illuminate\Support\Facades\Queue; use LaravelDoctrine\ORM\Facades\EntityManager; use libs\utils\ITransactionService; @@ -45,11 +49,13 @@ use models\main\ITagRepository; use App\Models\Foundation\Summit\PromoCodes\PromoCodesConstants; use models\summit\SpeakersRegistrationDiscountCode; +use models\summit\IOrderConstants; use models\summit\ISummitAttendeeRepository; use models\summit\ISummitAttendeeTicketRepository; use models\summit\ISummitRegistrationPromoCodeRepository; use models\summit\ISummitRepository; use models\summit\ISummitTicketTypeRepository; +use models\summit\PaymentGatewayProfile; use models\summit\Summit; use models\summit\SummitAttendee; use models\summit\SummitAttendeeBadge; @@ -2065,4 +2071,464 @@ public function testRequestRefundOrderRollsBackEntireChainWhenOneTicketIsFree() $this->assertEquals(0, $reFetchedRefundable->getRefundedRequests()->count()); $this->assertEquals(0, $reFetchedFree->getRefundedRequests()->count()); } + + // --------------------------------------------------------------------- + // Slot release idempotency: a reservation slot must be released exactly + // once per order, only while the order holds it, and only after the + // payment gateway confirmed the PaymentIntent can no longer be charged. + // --------------------------------------------------------------------- + + /** + * Builds the service with every real dependency except the payment gateway. + * The test summit has no active payment profile, so Summit::getPaymentGateWayPerApp + * falls back to the default-profile strategy, which is where the mock is injected. + */ + private function buildOrderServiceWithGateway(?IPaymentGatewayAPI $gateway): SummitOrderService + { + $strategy = Mockery::mock(IBuildDefaultPaymentGatewayProfileStrategy::class); + if (is_null($gateway)) { + $strategy->shouldReceive('build')->andReturnNull(); + } else { + $profile = Mockery::mock(PaymentGatewayProfile::class); + $profile->shouldReceive('buildPaymentGatewayApi')->andReturn($gateway); + $strategy->shouldReceive('build')->andReturn($profile); + } + + return new SummitOrderService( + App::make(ISummitTicketTypeRepository::class), + App::make(IMemberRepository::class), + App::make(ISummitRegistrationPromoCodeRepository::class), + App::make(\models\summit\ISummitPromoCodeMemberReservationRepository::class), + App::make(ISummitAttendeeRepository::class), + App::make(ISummitOrderRepository::class), + App::make(ISummitAttendeeTicketRepository::class), + App::make(ISummitAttendeeBadgeRepository::class), + App::make(ISummitRepository::class), + App::make(ISummitAttendeeBadgePrintRuleRepository::class), + App::make(IMemberService::class), + $strategy, + Mockery::mock(IFileUploadStrategy::class), + Mockery::mock(IFileDownloadStrategy::class), + App::make(ICompanyRepository::class), + App::make(ITagRepository::class), + App::make(ISummitRefundRequestRepository::class), + App::make(ICompanyService::class), + App::make(ITicketFinderStrategyFactory::class), + App::make(ITransactionService::class), + App::make(ILockManagerService::class) + ); + } + + /** + * A gateway mock with Stripe-like status semantics. Tests add the + * getCartStatus / abandonCart / isSuccessFullPayment expectations they need. + */ + private function mockGateway(): Mockery\MockInterface + { + $gateway = Mockery::mock(IPaymentGatewayAPI::class); + $gateway->shouldReceive('canAbandon')->andReturnUsing( + fn(string $status) => in_array($status, ['requires_payment_method', 'requires_confirmation', 'requires_action']) + ); + $gateway->shouldReceive('isSucceeded')->andReturnUsing(fn(string $status) => $status === 'succeeded'); + $gateway->shouldReceive('isDeclined')->andReturnUsing(fn(string $status) => $status === 'canceled'); + $gateway->shouldReceive('getPaymentDetailsInfo')->andReturn([]); + return $gateway; + } + + private function createCappedTicketType(int $cap): SummitTicketType + { + $type = new SummitTicketType(); + $type->setName('SLOT RELEASE TICKET TYPE ' . uniqid()); + $type->setCost(100); + $type->setCurrency('USD'); + $type->setQuantity2Sell($cap); + $type->setAudience(SummitTicketType::Audience_All); + self::$summit->addTicketType($type); + self::$em->persist(self::$summit); + self::$em->flush(); + return $type; + } + + /** + * An order holding exactly one slot of $type, counted the same way + * ReserveTicketsTask counts it (SummitTicketType::sell). + */ + private function createOrderHoldingSlot(SummitTicketType $type, ?string $cart_id): SummitOrder + { + $order = new SummitOrder(); + $order->setSummit(self::$summit); + $order->setOwner(self::$defaultMember); + $order->generateNumber(); + $order->generateHash(); + if (!is_null($cart_id)) { + $order->setPaymentGatewayCartId($cart_id); + } + + $ticket = new SummitAttendeeTicket(); + $ticket->setTicketType($type); + $order->addTicket($ticket); + $ticket->activate(); + $ticket->generateNumber(); + $ticket->generateQRCode(); + + $type->sell(1); + self::$summit->addOrder($order); + self::$em->persist($order); + self::$em->flush(); + return $order; + } + + private function quantitySoldFromDb(int $type_id): int + { + self::$em->clear(); + return self::$em->find(SummitTicketType::class, $type_id)->getQuantitySold(); + } + + private function orderFromDb(int $order_id): SummitOrder + { + self::$em->clear(); + return self::$em->find(SummitOrder::class, $order_id); + } + + /** + * Simulates another PHP process cancelling $order and releasing its slot while + * this process still holds the stale order entity in its identity map (what the + * widget DELETE does to an order the cron loop listed a moment earlier). Only the + * order stays stale: the ticket type is refreshed so the DB and memory counters + * agree and a phantom release is observable as an extra decrement. + */ + private function cancelOrderFromAnotherProcess(SummitOrder $order, SummitTicketType $type): void + { + DB::update('UPDATE SummitOrder SET Status = ? WHERE ID = ?', [IOrderConstants::CancelledStatus, $order->getId()]); + DB::update('UPDATE SummitAttendeeTicket SET Status = ? WHERE OrderID = ?', [IOrderConstants::CancelledStatus, $order->getId()]); + DB::update('UPDATE SummitTicketType SET QuantitySold = QuantitySold - 1 WHERE ID = ?', [$type->getId()]); + self::$em->refresh($type); + } + + public function test_cancel_twice_restores_once() + { + $type = $this->createCappedTicketType(10); + $order_a = $this->createOrderHoldingSlot($type, null); + $this->createOrderHoldingSlot($type, null); + $this->assertEquals(2, $type->getQuantitySold()); + $service = $this->buildOrderServiceWithGateway($this->mockGateway()); + + $service->cancel(self::$summit, $order_a->getHash()); + $service->cancel(self::$summit, $order_a->getHash()); + + $this->assertEquals(1, $this->quantitySoldFromDb($type->getId()), 'A second cancel of the same order must not release a second slot'); + } + + public function test_cancel_on_paid_order_throws_and_keeps_quantity_sold() + { + $type = $this->createCappedTicketType(10); + $paid_order = $this->createOrderHoldingSlot($type, 'pi_paid_' . uniqid()); + $paid_order->setPaid(); + self::$em->flush(); + $this->createOrderHoldingSlot($type, null); + $service = $this->buildOrderServiceWithGateway($this->mockGateway()); + + try { + $service->cancel(self::$summit, $paid_order->getHash()); + $this->fail('Expected ValidationException was not thrown'); + } catch (ValidationException $ex) { + $this->addToAssertionCount(1); + } + + $this->assertEquals(2, $this->quantitySoldFromDb($type->getId()), 'Cancelling a paid order must not release its slot'); + $this->assertEquals(IOrderConstants::PaidStatus, $this->orderFromDb($paid_order->getId())->getStatus()); + } + + public function test_cancel_abandonable_intent_abandons_then_restores() + { + $type = $this->createCappedTicketType(10); + $cart_id = 'pi_abandon_' . uniqid(); + $order = $this->createOrderHoldingSlot($type, $cart_id); + $abandoned = false; + $gateway = $this->mockGateway(); + $gateway->shouldReceive('getCartStatus')->with($cart_id)->andReturn('requires_payment_method'); + $gateway->shouldReceive('abandonCart')->with($cart_id)->andReturnUsing(function () use (&$abandoned) { + $abandoned = true; + }); + $service = $this->buildOrderServiceWithGateway($gateway); + + $service->cancel(self::$summit, $order->getHash()); + + $this->assertTrue($abandoned, 'cancel() must cancel the PaymentIntent before releasing the slot'); + $this->assertEquals(0, $this->quantitySoldFromDb($type->getId())); + $this->assertEquals(IOrderConstants::CancelledStatus, $this->orderFromDb($order->getId())->getStatus()); + } + + /** + * Anti-regression (passes before the fix): a PaymentIntent that is already + * canceled still holds the slot on our side, so cancel() releases it without + * trying to abandon the intent again. + */ + public function test_cancel_canceled_intent_restores_without_abandon() + { + $type = $this->createCappedTicketType(10); + $cart_id = 'pi_canceled_' . uniqid(); + $order = $this->createOrderHoldingSlot($type, $cart_id); + $gateway = $this->mockGateway(); + $gateway->shouldReceive('getCartStatus')->with($cart_id)->andReturn('canceled'); + $gateway->shouldReceive('abandonCart')->never(); + $service = $this->buildOrderServiceWithGateway($gateway); + + $service->cancel(self::$summit, $order->getHash()); + + $this->assertEquals(0, $this->quantitySoldFromDb($type->getId())); + $this->assertEquals(IOrderConstants::CancelledStatus, $this->orderFromDb($order->getId())->getStatus()); + } + + public function test_cancel_processing_intent_leaves_order_unchanged() + { + $type = $this->createCappedTicketType(10); + $cart_id = 'pi_processing_' . uniqid(); + $order = $this->createOrderHoldingSlot($type, $cart_id); + $gateway = $this->mockGateway(); + $gateway->shouldReceive('getCartStatus')->with($cart_id)->andReturn('processing'); + $gateway->shouldReceive('abandonCart')->never(); + $service = $this->buildOrderServiceWithGateway($gateway); + + $service->cancel(self::$summit, $order->getHash()); + + $this->assertEquals(1, $this->quantitySoldFromDb($type->getId()), 'A payment in flight must keep its slot'); + $this->assertEquals(IOrderConstants::ReservedStatus, $this->orderFromDb($order->getId())->getStatus()); + } + + public function test_cancel_unreadable_status_leaves_order_unchanged() + { + $type = $this->createCappedTicketType(10); + $cart_id = 'pi_unreadable_' . uniqid(); + $order = $this->createOrderHoldingSlot($type, $cart_id); + $gateway = $this->mockGateway(); + $gateway->shouldReceive('getCartStatus')->with($cart_id)->andReturnNull(); + $gateway->shouldReceive('abandonCart')->never(); + $service = $this->buildOrderServiceWithGateway($gateway); + + $service->cancel(self::$summit, $order->getHash()); + + $this->assertEquals(1, $this->quantitySoldFromDb($type->getId()), 'An unreadable gateway status must never release a slot'); + $this->assertEquals(IOrderConstants::ReservedStatus, $this->orderFromDb($order->getId())->getStatus()); + } + + public function test_cancel_abandon_failure_leaves_order_unchanged() + { + $type = $this->createCappedTicketType(10); + $cart_id = 'pi_abandon_fail_' . uniqid(); + $order = $this->createOrderHoldingSlot($type, $cart_id); + $gateway = $this->mockGateway(); + $gateway->shouldReceive('getCartStatus')->with($cart_id)->andReturn('requires_payment_method'); + $gateway->shouldReceive('abandonCart')->with($cart_id)->andThrow(new \RuntimeException('gateway unavailable')); + $service = $this->buildOrderServiceWithGateway($gateway); + + $service->cancel(self::$summit, $order->getHash()); + + $this->assertEquals(1, $this->quantitySoldFromDb($type->getId()), 'A failed abandon must not release the slot'); + $this->assertEquals(IOrderConstants::ReservedStatus, $this->orderFromDb($order->getId())->getStatus()); + } + + public function test_cancel_abandon_already_paid_marks_order_paid_without_restore() + { + $type = $this->createCappedTicketType(10); + $cart_id = 'pi_race_' . uniqid(); + $order = $this->createOrderHoldingSlot($type, $cart_id); + $gateway = $this->mockGateway(); + // the intent was paid between the status read and the cancel attempt + $gateway->shouldReceive('getCartStatus')->with($cart_id)->andReturn('requires_payment_method', 'succeeded'); + $gateway->shouldReceive('abandonCart')->with($cart_id)->andThrow(new CartAlreadyPaidException('already paid')); + $service = $this->buildOrderServiceWithGateway($gateway); + + $service->cancel(self::$summit, $order->getHash()); + + $this->assertEquals(1, $this->quantitySoldFromDb($type->getId()), 'A paid intent keeps its slot'); + $this->assertEquals(IOrderConstants::PaidStatus, $this->orderFromDb($order->getId())->getStatus()); + } + + public function test_cancel_without_gateway_configured_throws_and_keeps_quantity_sold() + { + $type = $this->createCappedTicketType(10); + $order = $this->createOrderHoldingSlot($type, 'pi_no_gateway_' . uniqid()); + $service = $this->buildOrderServiceWithGateway(null); + + try { + $service->cancel(self::$summit, $order->getHash()); + $this->fail('Expected ValidationException was not thrown'); + } catch (ValidationException $ex) { + $this->addToAssertionCount(1); + } + + $this->assertEquals(1, $this->quantitySoldFromDb($type->getId())); + $this->assertEquals(IOrderConstants::ReservedStatus, $this->orderFromDb($order->getId())->getStatus()); + } + + public function test_payment_failed_on_cancelled_order_keeps_cancelled() + { + $type = $this->createCappedTicketType(10); + $cart_id = 'pi_failed_late_' . uniqid(); + $order = $this->createOrderHoldingSlot($type, $cart_id); + $order->setCancelled(); + $type->restore(1); + self::$em->flush(); + $gateway = $this->mockGateway(); + $gateway->shouldReceive('isSuccessFullPayment')->andReturn(false); + $gateway->shouldReceive('getPaymentError')->andReturn('Your card was declined.'); + $service = $this->buildOrderServiceWithGateway($gateway); + + $service->processPayment(['cart_id' => $cart_id], self::$summit); + + $db_order = $this->orderFromDb($order->getId()); + $this->assertEquals(IOrderConstants::CancelledStatus, $db_order->getStatus(), 'A late payment_failed must not move a cancelled order back to Error'); + $this->assertEquals('Your card was declined.', $db_order->getLastError()); + } + + /** + * Anti-regression (passes before the fix): Error is the only route by which a + * failed order reaches the revoke cron, so Confirmed -> Error must stay open. + */ + public function test_payment_failed_on_confirmed_order_moves_to_error() + { + $type = $this->createCappedTicketType(10); + $cart_id = 'pi_failed_confirmed_' . uniqid(); + $order = $this->createOrderHoldingSlot($type, $cart_id); + $order->setConfirmed(); + self::$em->flush(); + $gateway = $this->mockGateway(); + $gateway->shouldReceive('isSuccessFullPayment')->andReturn(false); + $gateway->shouldReceive('getPaymentError')->andReturn('Your card was declined.'); + $service = $this->buildOrderServiceWithGateway($gateway); + + $service->processPayment(['cart_id' => $cart_id], self::$summit); + + $this->assertEquals(IOrderConstants::ErrorStatus, $this->orderFromDb($order->getId())->getStatus()); + $this->assertEquals(1, $this->quantitySoldFromDb($type->getId()), 'An Error order still holds its slot until the cron releases it'); + } + + public function test_succeeded_on_cancelled_order_keeps_cancelled_and_logs_error() + { + $type = $this->createCappedTicketType(10); + $cart_id = 'pi_succeeded_late_' . uniqid(); + $order = $this->createOrderHoldingSlot($type, $cart_id); + $order->setCancelled(); + $type->restore(1); + self::$em->flush(); + $this->createOrderHoldingSlot($type, null); + $gateway = $this->mockGateway(); + $gateway->shouldReceive('isSuccessFullPayment')->andReturn(true); + $service = $this->buildOrderServiceWithGateway($gateway); + Log::spy(); + + $service->processPayment(['cart_id' => $cart_id], self::$summit); + + $this->assertEquals(IOrderConstants::CancelledStatus, $this->orderFromDb($order->getId())->getStatus(), 'A cancelled order must never be revived by a late succeeded webhook'); + $this->assertEquals(1, $this->quantitySoldFromDb($type->getId())); + Log::shouldHaveReceived('error')->withArgs(function ($message) use ($order, $cart_id) { + return is_string($message) + && str_contains($message, (string)$order->getId()) + && str_contains($message, $cart_id); + })->once(); + } + + public function test_revoke_after_widget_cancel_does_not_restore() + { + $type = $this->createCappedTicketType(10); + $order_a = $this->createOrderHoldingSlot($type, 'pi_revoke_a_' . uniqid()); + $this->createOrderHoldingSlot($type, 'pi_revoke_b_' . uniqid()); + $gateway = $this->mockGateway(); + $gateway->shouldReceive('getCartStatus')->andReturn('requires_payment_method'); + $gateway->shouldReceive('abandonCart')->andReturnNull(); + $service = $this->buildOrderServiceWithGateway($gateway); + + // the widget cancels A after the cron listed it but before its turn in the loop + $this->cancelOrderFromAnotherProcess($order_a, $type); + $service->processOrder2Revoke($order_a->getId()); + + $this->assertEquals(1, $this->quantitySoldFromDb($type->getId()), 'The cron must not release a slot the widget already released'); + } + + public function test_revoke_error_order_with_canceled_intent_moves_to_cancelled() + { + $type = $this->createCappedTicketType(10); + $cart_id = 'pi_zombie_' . uniqid(); + $order = $this->createOrderHoldingSlot($type, $cart_id); + $this->createOrderHoldingSlot($type, null); + // the incident shape: tickets Cancelled and slot released, order flipped back to Error + $order->setCancelled(); + $type->restore(1); + self::$em->flush(); + DB::update('UPDATE SummitOrder SET Status = ? WHERE ID = ?', [IOrderConstants::ErrorStatus, $order->getId()]); + self::$em->refresh($order); + $this->assertEquals(IOrderConstants::ErrorStatus, $order->getStatus()); + $gateway = $this->mockGateway(); + $gateway->shouldReceive('getCartStatus')->with($cart_id)->andReturn('canceled'); + $gateway->shouldReceive('abandonCart')->never(); + $service = $this->buildOrderServiceWithGateway($gateway); + + $service->processOrder2Revoke($order->getId()); + + $this->assertEquals(IOrderConstants::CancelledStatus, $this->orderFromDb($order->getId())->getStatus(), 'An Error order whose intent is canceled must end Cancelled, not stay Error forever'); + $this->assertEquals(1, $this->quantitySoldFromDb($type->getId()), 'Tickets already Cancelled must not be released again'); + } + + public function test_revoke_unreadable_status_skips_order() + { + $type = $this->createCappedTicketType(10); + $cart_id = 'pi_revoke_null_' . uniqid(); + $order = $this->createOrderHoldingSlot($type, $cart_id); + $gateway = $this->mockGateway(); + $gateway->shouldReceive('getCartStatus')->with($cart_id)->andReturnNull(); + $gateway->shouldReceive('abandonCart')->never(); + $service = $this->buildOrderServiceWithGateway($gateway); + + $service->processOrder2Revoke($order->getId()); + + $this->assertEquals(1, $this->quantitySoldFromDb($type->getId()), 'The cron must not release a slot while the PaymentIntent is still chargeable'); + $this->assertEquals(IOrderConstants::ReservedStatus, $this->orderFromDb($order->getId())->getStatus()); + } + + /** + * Guard for the refresh fix (passes before it): an order deleted between the + * cron listing and its turn must be skipped, not crash the whole run. + */ + public function test_revoke_deleted_order_returns_silently() + { + $type = $this->createCappedTicketType(10); + $order = $this->createOrderHoldingSlot($type, null); + $order_id = $order->getId(); + self::$summit->removeOrder($order); + self::$em->flush(); + $service = $this->buildOrderServiceWithGateway($this->mockGateway()); + + $service->processOrder2Revoke($order_id); + + self::$em->clear(); + $this->assertNull(self::$em->find(SummitOrder::class, $order_id)); + } + + public function test_confirm_cron_after_widget_cancel_does_not_restore() + { + $type = $this->createCappedTicketType(10); + // B is created first so the cron processes it before A + $cart_b = 'pi_confirm_b_' . uniqid(); + $cart_a = 'pi_confirm_a_' . uniqid(); + $order_b = $this->createOrderHoldingSlot($type, $cart_b); + $order_a = $this->createOrderHoldingSlot($type, $cart_a); + $order_b->setConfirmed(); + $order_a->setConfirmed(); + self::$em->flush(); + $this->assertEquals(2, $type->getQuantitySold()); + $gateway = $this->mockGateway(); + // while the cron is on B, the widget cancels A (already listed, still stale in memory) + $gateway->shouldReceive('getCartStatus')->with($cart_b)->andReturnUsing(function () use ($order_a, $type) { + $this->cancelOrderFromAnotherProcess($order_a, $type); + return 'processing'; + }); + $gateway->shouldReceive('getCartStatus')->with($cart_a)->andReturn('canceled'); + $service = $this->buildOrderServiceWithGateway($gateway); + + $service->confirmOrdersOlderThanNMinutes(0); + + $this->assertEquals(1, $this->quantitySoldFromDb($type->getId()), 'The confirm cron must not release a slot the widget already released'); + $this->assertEquals(IOrderConstants::ConfirmedStatus, $this->orderFromDb($order_b->getId())->getStatus()); + } } diff --git a/tests/Unit/Services/LawPayApiStatusTest.php b/tests/Unit/Services/LawPayApiStatusTest.php new file mode 100644 index 000000000..840c86dc9 --- /dev/null +++ b/tests/Unit/Services/LawPayApiStatusTest.php @@ -0,0 +1,74 @@ +instance('app', $container); + $container->instance('log', new class { + public function __call($name, $args) { /* swallow */ } + }); + \Illuminate\Support\Facades\Facade::setFacadeApplication($container); + } + + protected function tearDown(): void + { + \Illuminate\Support\Facades\Facade::clearResolvedInstances(); + \Illuminate\Support\Facades\Facade::setFacadeApplication(null); + parent::tearDown(); + } + + private function buildApi(): LawPayApi + { + return new LawPayApi([ + 'secret_key' => 'test_secret', + 'public_key' => 'test_public', + 'account_id' => 'test_account', + 'test_mode_enabled' => true, + ]); + } + + public function test_is_declined_voided_and_failed_return_true(): void + { + $api = $this->buildApi(); + + $this->assertTrue($api->isDeclined(ILawPayApiChargeStatus::Voided)); + $this->assertTrue($api->isDeclined(ILawPayApiChargeStatus::Failed)); + } + + public function test_is_declined_other_statuses_return_false(): void + { + $api = $this->buildApi(); + + $this->assertFalse($api->isDeclined(ILawPayApiChargeStatus::Pending)); + $this->assertFalse($api->isDeclined(ILawPayApiChargeStatus::Authorized)); + $this->assertFalse($api->isDeclined(ILawPayApiChargeStatus::Completed)); + } +} From 6a40ec2a5e1c44aa431a43e9774ee5642410cadd Mon Sep 17 00:00:00 2001 From: smarcet Date: Thu, 8 Oct 2026 02:31:46 -0300 Subject: [PATCH 2/4] fix(registration): make order slot release idempotent and gateway-state aware During the 2026-10-07 blast 21 ticket slots were released for orders that no longer held them, overselling a capped ticket type (416 paid on a cap of 405). Every release path was non-idempotent: cancel() released without checking the order status or cancelling the PaymentIntent, setPaymentError() moved Cancelled orders back to Error so the revoke cron released them again, both crons re-used stale identity-map entities and released on an unreadable gateway status, calculateTicketsAndPromoCodesToReturn() counted tickets already Cancelled, and setPaid() revived Cancelled orders without re-selling. A slot is now released exactly once, only while the order holds it, and only after the gateway confirmed the PaymentIntent can no longer be charged (abandoned here or already declined). Paid orders refuse cancel (412), Cancelled orders are a no-op, unreadable or in-flight gateway states hold the slot for the cron, and a successful payment on a Cancelled order is logged at ERROR instead of reviving it. The crons reload orders with refresh (null guarded) and skip Paid/Cancelled ones. LawPayApi::isDeclined now reports VOIDED and FAILED charges so LawPay orders keep releasing through the same path. --- .../Summit/Registration/SummitOrder.php | 19 +- app/Repositories/DoctrineRepository.php | 2 +- .../Apis/PaymentGateways/LawPayApi.php | 5 +- app/Services/Model/Imp/SummitOrderService.php | 214 +++++++++++++++--- 4 files changed, 206 insertions(+), 34 deletions(-) diff --git a/app/Models/Foundation/Summit/Registration/SummitOrder.php b/app/Models/Foundation/Summit/Registration/SummitOrder.php index 15f46f8a0..1b460945f 100644 --- a/app/Models/Foundation/Summit/Registration/SummitOrder.php +++ b/app/Models/Foundation/Summit/Registration/SummitOrder.php @@ -340,6 +340,10 @@ public function setPaid(array $payload = null):void return; } + // a cancelled order already released its slots; reviving it would oversell + if ($this->isCancelled()) + throw new ValidationException(sprintf("Order %s is cancelled and can not be marked as paid.", $this->id)); + $this->setPaidStatus(); if(!$this->tickets->count()) @@ -360,8 +364,15 @@ public function setPaid(array $payload = null):void public function setPaymentError(?string $error): void { if (empty($error)) return; - $this->status = IOrderConstants::ErrorStatus; $this->last_error = $error; + // only an order that still holds its slots can move to Error (Error is how it reaches the revoke cron); + // a Paid or Cancelled order keeps its status, otherwise a late payment_failed re-opens a released order + $allowed_statuses = [IOrderConstants::ReservedStatus, IOrderConstants::ConfirmedStatus, IOrderConstants::ErrorStatus]; + if (!in_array($this->status, $allowed_statuses)) { + Log::warning(sprintf("SummitOrder::setPaymentError order %s is %s, keeping status (error: %s).", $this->id, $this->status, $error)); + return; + } + $this->status = IOrderConstants::ErrorStatus; } public function setConfirmed() @@ -395,6 +406,12 @@ public function calculateTicketsAndPromoCodesToReturn(): array foreach ($this->tickets as $ticket) { + // a cancelled ticket already returned its slot: counting it again is a phantom release + if ($ticket->isCancelled()) { + Log::debug(sprintf("SummitOrder::calculateTicketsAndPromoCodesToReturn order %s ticket %s is already cancelled, skipping", $this->id, $ticket->getId())); + continue; + } + if (!isset($tickets_to_return[$ticket->getTicketTypeId()])) $tickets_to_return[$ticket->getTicketTypeId()] = 0; diff --git a/app/Repositories/DoctrineRepository.php b/app/Repositories/DoctrineRepository.php index ecea26e6b..c4f3e7ed6 100644 --- a/app/Repositories/DoctrineRepository.php +++ b/app/Repositories/DoctrineRepository.php @@ -454,7 +454,7 @@ public function find($id, $lockMode = null, $lockVersion = null, $refresh = fals { $em = $this->getEntityManager(); $res = $em->find($this->getEntityName(), $id, $lockMode, $lockVersion); - if($refresh) + if($refresh && !is_null($res)) $em->refresh($res); return $res; } diff --git a/app/Services/Apis/PaymentGateways/LawPayApi.php b/app/Services/Apis/PaymentGateways/LawPayApi.php index 2c855f55d..2005aff40 100644 --- a/app/Services/Apis/PaymentGateways/LawPayApi.php +++ b/app/Services/Apis/PaymentGateways/LawPayApi.php @@ -547,6 +547,9 @@ public function getPaymentDetailsInfo(string $cart_id): ?array public function isDeclined(string $status): bool { - return false; + return in_array($status, [ + ILawPayApiChargeStatus::Voided, + ILawPayApiChargeStatus::Failed, + ]); } } \ No newline at end of file diff --git a/app/Services/Model/Imp/SummitOrderService.php b/app/Services/Model/Imp/SummitOrderService.php index 9f490a955..1026db2a6 100644 --- a/app/Services/Model/Imp/SummitOrderService.php +++ b/app/Services/Model/Imp/SummitOrderService.php @@ -35,6 +35,8 @@ use App\Models\Foundation\Summit\Repositories\ISummitAttendeeBadgeRepository; use App\Models\Foundation\Summit\Repositories\ISummitOrderRepository; use App\Models\Foundation\Summit\Repositories\ISummitRefundRequestRepository; +use App\Services\Apis\CartAlreadyPaidException; +use App\Services\Apis\IPaymentGatewayAPI; use App\Services\FileSystem\IFileDownloadStrategy; use App\Services\FileSystem\IFileUploadStrategy; use App\Services\Model\dto\ExternalUserDTO; @@ -2809,16 +2811,152 @@ public function cancel(Summit $summit, string $order_hash): SummitOrder if (is_null($order) || !$order instanceof SummitOrder || $summit->getId() != $order->getSummitId()) throw new EntityNotFoundException("order not found"); - list($tickets_to_return, $promo_codes_to_return) = $order->calculateTicketsAndPromoCodesToReturn(); + if ($order->isPaid()) + throw new ValidationException(sprintf("Order %s is already paid and can not be cancelled.", $order->getId())); - $this->restoreTicketsPromoCodes($summit, $tickets_to_return, $promo_codes_to_return, $order->getOwner()); + if ($order->isCancelled()) { + Log::warning(sprintf("SummitOrderService::cancel order %s is already cancelled, nothing to release.", $order->getId())); + return $order; + } + + $cart_id = $order->getPaymentGatewayCartId(); + if (!empty($cart_id)) { + $payment_gateway = $summit->getPaymentGateWayPerApp + ( + IPaymentConstants::ApplicationTypeRegistration, + $this->default_payment_gateway_strategy + ); - $order->setCancelled(); + if (is_null($payment_gateway)) + throw new ValidationException(sprintf("Payment configuration is not set for summit %s.", $summit->getId())); + + $action = $this->resolveOrderReleaseAction($order, $payment_gateway, 'cancel'); + + if ($action === self::ReleaseAction_Succeeded) { + Log::warning(sprintf("SummitOrderService::cancel order %s was already paid at the gateway, marking as paid instead of cancelling.", $order->getId())); + $this->markOrderPaidFromGateway($order, $payment_gateway); + return $order; + } + + if ($action === self::ReleaseAction_Hold) { + Log::warning(sprintf("SummitOrderService::cancel order %s keeps its slots, the payment intent can still be charged or its status is unknown.", $order->getId())); + return $order; + } + } + + $this->releaseOrderSlots($summit, $order); return $order; }); } + private const ReleaseAction_Succeeded = 'succeeded'; + private const ReleaseAction_Release = 'release'; + private const ReleaseAction_Hold = 'hold'; + + /** + * Decides, against the payment gateway, what a release path may do with an order + * that still holds its slots. Every gateway call happens here, BEFORE any entity + * mutation, so a caller that swallows exceptions never commits a half-done release. + * + * - ReleaseAction_Succeeded: the payment intent was paid, the caller marks the order paid and keeps the slots + * - ReleaseAction_Release: the payment intent can no longer be charged (no intent, abandoned here, or already declined), the caller releases the slots + * - ReleaseAction_Hold: status unreadable, payment in flight, or gateway failure; the caller leaves the order untouched and the cron retries + * + * @param SummitOrder $order + * @param IPaymentGatewayAPI $payment_gateway + * @param string $caller + * @return string + * @throws ValidationException when the gateway itself raises one + */ + private function resolveOrderReleaseAction(SummitOrder $order, IPaymentGatewayAPI $payment_gateway, string $caller): string + { + $cart_id = $order->getPaymentGatewayCartId(); + if (empty($cart_id)) return self::ReleaseAction_Release; + + $status = $this->readCartStatus($cart_id, $payment_gateway, $caller); + if (is_null($status)) return self::ReleaseAction_Hold; + + Log::debug(sprintf("SummitOrderService::%s order %s cart %s status %s", $caller, $order->getId(), $cart_id, $status)); + + if ($payment_gateway->isSucceeded($status)) return self::ReleaseAction_Succeeded; + if ($payment_gateway->isDeclined($status)) return self::ReleaseAction_Release; + + if (!$payment_gateway->canAbandon($status)) { + Log::warning(sprintf("SummitOrderService::%s order %s can not be cancelled, external status %s, holding slots.", $caller, $order->getId(), $status)); + return self::ReleaseAction_Hold; + } + + try { + $payment_gateway->abandonCart($cart_id); + return self::ReleaseAction_Release; + } catch (CartAlreadyPaidException $ex) { + // the intent moved between the status read and the cancel attempt: re-read once + Log::warning(sprintf("SummitOrderService::%s order %s cart %s changed state while cancelling (%s), re-reading status.", $caller, $order->getId(), $cart_id, $ex->getMessage())); + $status = $this->readCartStatus($cart_id, $payment_gateway, $caller); + if (!is_null($status) && $payment_gateway->isSucceeded($status)) return self::ReleaseAction_Succeeded; + if (!is_null($status) && $payment_gateway->isDeclined($status)) return self::ReleaseAction_Release; + return self::ReleaseAction_Hold; + } catch (ValidationException $ex) { + throw $ex; + } catch (\Throwable $ex) { + Log::warning(sprintf("SummitOrderService::%s order %s could not abandon cart %s, holding slots.", $caller, $order->getId(), $cart_id)); + Log::warning($ex); + return self::ReleaseAction_Hold; + } + } + + /** + * @param string $cart_id + * @param IPaymentGatewayAPI $payment_gateway + * @param string $caller + * @return string|null null when the status can not be read, which must never release a slot + * @throws ValidationException + */ + private function readCartStatus(string $cart_id, IPaymentGatewayAPI $payment_gateway, string $caller): ?string + { + try { + $status = $payment_gateway->getCartStatus($cart_id); + if (is_null($status)) + Log::warning(sprintf("SummitOrderService::%s cart %s status is unreadable, holding slots.", $caller, $cart_id)); + return $status; + } catch (ValidationException $ex) { + throw $ex; + } catch (\Throwable $ex) { + Log::warning(sprintf("SummitOrderService::%s cart %s status read failed, holding slots.", $caller, $cart_id)); + Log::warning($ex); + return null; + } + } + + /** + * Releases every slot the order still holds (tickets already Cancelled are skipped + * by calculateTicketsAndPromoCodesToReturn) and cancels the order and its tickets + * in the same transaction. + * @param Summit $summit + * @param SummitOrder $order + */ + private function releaseOrderSlots(Summit $summit, SummitOrder $order): void + { + list($tickets_to_return, $promo_codes_to_return) = $order->calculateTicketsAndPromoCodesToReturn(); + + $this->restoreTicketsPromoCodes($summit, $tickets_to_return, $promo_codes_to_return, $order->getOwner()); + + $order->setCancelled(); + } + + /** + * @param SummitOrder $order + * @param IPaymentGatewayAPI $payment_gateway + * @throws ValidationException + */ + private function markOrderPaidFromGateway(SummitOrder $order, IPaymentGatewayAPI $payment_gateway): void + { + $order->setPaid($payment_gateway->getPaymentDetailsInfo($order->getPaymentGatewayCartId())); + // invoke now to avoid delays + $this->processInvitation($order); + } + /** * @param Summit $summit * @param array $tickets_to_return @@ -2843,7 +2981,8 @@ private function restoreTicketsPromoCodes(Summit $summit, array $tickets_to_retu ); foreach ($tickets_to_return as $ticket_type_id => $qty) { - $ticket_type = $this->ticket_type_repository->getByIdExclusiveLock($ticket_type_id); + // refresh: the type may already be in the identity map with a stale counter (cron loop) + $ticket_type = $this->ticket_type_repository->getByIdExclusiveLock($ticket_type_id, true); if (!$ticket_type instanceof SummitTicketType) continue; Log::debug(sprintf("SummitOrderService::restoreTicketsPromoCodes compensating ticket type %s on %s usages", $ticket_type_id, $qty)); try { @@ -2911,6 +3050,20 @@ public function processPayment(array $payload, ?Summit $summit = null): void if ($payment_gateway->isSuccessFullPayment($payload)) { Log::debug("SummitOrderService::processPayment: payment is successful"); + if ($order->isCancelled()) { + // unreachable by construction (a slot is released only after the intent was abandoned), + // so a paid cancelled order is an anomaly: never revive it, never sell again, hand it to a human + Log::error + ( + sprintf + ( + "SummitOrderService::processPayment order %s (cart_id %s) is Cancelled but the gateway reported a successful payment. The slot was already released, the order is NOT revived, manual refund required.", + $order->getId(), + $payload['cart_id'] + ) + ); + return; + } $order->setPaid($payload); return; } @@ -2937,8 +3090,14 @@ public function confirmOrdersOlderThanNMinutes(int $minutes, int $max = 100): vo try { if (!$order instanceof SummitOrder) return; - $order = $this->order_repository->getByIdExclusiveLock($order->getId()); + // refresh: the entity comes from the listing query's identity map and may have been + // cancelled or paid by another process (widget DELETE, webhook) since then + $order = $this->order_repository->getByIdExclusiveLock($order->getId(), true); if (!$order instanceof SummitOrder) return; + if ($order->isPaid() || $order->isCancelled()) { + Log::debug(sprintf("SummitOrderService::confirmOrdersOlderThanNMinutes order %s is %s, skipping", $order->getId(), $order->getStatus())); + return; + } Log::debug(sprintf("SummitOrderService::confirmOrdersOlderThanNMinutes processing order %s", $order->getId())); $summit = $order->getSummit(); $payment_gateway = $summit->getPaymentGateWayPerApp @@ -2953,7 +3112,7 @@ public function confirmOrdersOlderThanNMinutes(int $minutes, int $max = 100): vo $cart_id = $order->getPaymentGatewayCartId(); if (!empty($cart_id)) { - $status = $payment_gateway->getCartStatus($cart_id); + $status = $this->readCartStatus($cart_id, $payment_gateway, 'confirmOrdersOlderThanNMinutes'); if (!is_null($status)) { Log::debug @@ -2991,11 +3150,7 @@ public function confirmOrdersOlderThanNMinutes(int $minutes, int $max = 100): vo ) ); - list($tickets_to_return, $promo_codes_to_return) = $order->calculateTicketsAndPromoCodesToReturn(); - - $this->restoreTicketsPromoCodes($summit, $tickets_to_return, $promo_codes_to_return, $order->getOwner()); - - $order->setCancelled(); + $this->releaseOrderSlots($summit, $order); } } @@ -3077,8 +3232,14 @@ public function processOrder2Revoke(int $order_id): void try { - $order = $this->order_repository->getByIdExclusiveLock($order_id); + // refresh: the entity comes from the listing query's identity map and may have been + // cancelled or paid by another process (widget DELETE, webhook) since then + $order = $this->order_repository->getByIdExclusiveLock($order_id, true); if (!$order instanceof SummitOrder) return; + if ($order->isPaid() || $order->isCancelled()) { + Log::debug(sprintf("SummitOrderService::processOrder2Revoke order %s is %s, skipping", $order->getId(), $order->getStatus())); + return; + } $summit = $order->getSummit(); Log::debug(sprintf("SummitOrderService::processOrder2Revoke processing order %s summit %s", $order->getId(), $summit->getId())); $payment_gateway = $summit->getPaymentGateWayPerApp @@ -3094,29 +3255,20 @@ public function processOrder2Revoke(int $order_id): void Log::warning(sprintf("SummitOrderService::processOrder2Revoke cancelling order reservation %s create at %s", $order->getNumber(), $order->getCreated()->format("Y-m-d h:i:sa"))); - $cart_id = $order->getPaymentGatewayCartId(); - if (!empty($cart_id)) { + $action = $this->resolveOrderReleaseAction($order, $payment_gateway, 'processOrder2Revoke'); - $status = $payment_gateway->getCartStatus($cart_id); - if (!is_null($status)) { - if (!$payment_gateway->canAbandon($status)) { - Log::warning(sprintf("SummitOrderService::processOrder2Revoke reservation %s created at %s can not be cancelled external status %s", $order->getId(), $order->getCreated()->format("Y-m-d h:i:sa"), $status)); - if ($payment_gateway->isSucceeded($status)) { - $order->setPaid($payment_gateway->getPaymentDetailsInfo($cart_id)); - // invoke now to avoid delays - $this->processInvitation($order); - } - return; - } - $payment_gateway->abandonCart($cart_id); - } + if ($action === self::ReleaseAction_Succeeded) { + Log::warning(sprintf("SummitOrderService::processOrder2Revoke order %s was paid at the gateway, marking as paid.", $order->getId())); + $this->markOrderPaidFromGateway($order, $payment_gateway); + return; } - list($tickets_to_return, $promo_codes_to_return) = $order->calculateTicketsAndPromoCodesToReturn(); - - $this->restoreTicketsPromoCodes($summit, $tickets_to_return, $promo_codes_to_return, $order->getOwner()); + if ($action === self::ReleaseAction_Hold) { + Log::warning(sprintf("SummitOrderService::processOrder2Revoke order %s keeps its slots for now, will retry on next run.", $order->getId())); + return; + } - $order->setCancelled(); + $this->releaseOrderSlots($summit, $order); Log::warning(sprintf("SummitOrderService::processOrder2Revoke order %s got cancelled", $order->getId())); } catch (\Exception $ex) { From 2bd85b9382d06c9f8ecb9b040e29b46131bf8482 Mon Sep 17 00:00:00 2001 From: smarcet Date: Thu, 8 Oct 2026 10:44:27 -0300 Subject: [PATCH 3/4] test(registration): make confirm-cron stale-order test independent of query order getAllConfirmedOlderThanXMinutes has no ORDER BY, so the cron may visit either order first. The gateway mock now cancels whichever order was not queried first and the assertion follows the processed order instead of assuming B runs before A. --- tests/SummitOrderServiceTest.php | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/tests/SummitOrderServiceTest.php b/tests/SummitOrderServiceTest.php index 2f7cbff27..585582ec5 100644 --- a/tests/SummitOrderServiceTest.php +++ b/tests/SummitOrderServiceTest.php @@ -2508,7 +2508,6 @@ public function test_revoke_deleted_order_returns_silently() public function test_confirm_cron_after_widget_cancel_does_not_restore() { $type = $this->createCappedTicketType(10); - // B is created first so the cron processes it before A $cart_b = 'pi_confirm_b_' . uniqid(); $cart_a = 'pi_confirm_a_' . uniqid(); $order_b = $this->createOrderHoldingSlot($type, $cart_b); @@ -2518,17 +2517,32 @@ public function test_confirm_cron_after_widget_cancel_does_not_restore() self::$em->flush(); $this->assertEquals(2, $type->getQuantitySold()); $gateway = $this->mockGateway(); - // while the cron is on B, the widget cancels A (already listed, still stale in memory) - $gateway->shouldReceive('getCartStatus')->with($cart_b)->andReturnUsing(function () use ($order_a, $type) { - $this->cancelOrderFromAnotherProcess($order_a, $type); + $processed_order_id = null; + // getAllConfirmedOlderThanXMinutes has no ORDER BY, so the cron may visit A or B first. + // While the cron processes the first listed order, the widget cancels the other one, + // which is already listed but still stale in memory. + $gateway->shouldReceive('getCartStatus')->andReturnUsing(function ($cart_id) use ( + $cart_b, + $order_a, + $order_b, + $type, + &$processed_order_id + ) { + if ($cart_id === $cart_b) { + $processed_order_id = $order_b->getId(); + $this->cancelOrderFromAnotherProcess($order_a, $type); + } else { + $processed_order_id = $order_a->getId(); + $this->cancelOrderFromAnotherProcess($order_b, $type); + } return 'processing'; }); - $gateway->shouldReceive('getCartStatus')->with($cart_a)->andReturn('canceled'); $service = $this->buildOrderServiceWithGateway($gateway); $service->confirmOrdersOlderThanNMinutes(0); + $this->assertNotNull($processed_order_id, 'The cron must have queried the gateway for one of the two orders'); $this->assertEquals(1, $this->quantitySoldFromDb($type->getId()), 'The confirm cron must not release a slot the widget already released'); - $this->assertEquals(IOrderConstants::ConfirmedStatus, $this->orderFromDb($order_b->getId())->getStatus()); + $this->assertEquals(IOrderConstants::ConfirmedStatus, $this->orderFromDb($processed_order_id)->getStatus()); } } From 930267c5b8249b2c26d2621c5586dce377e62363 Mon Sep 17 00:00:00 2001 From: smarcet Date: Thu, 8 Oct 2026 11:10:53 -0300 Subject: [PATCH 4/4] docs(registration): document 412 on the cancel order endpoint cancel() now answers 412 when the order is already paid or when the summit has no payment configuration, matching the sibling endpoints in this controller. --- .../Apis/Protected/Summit/OAuth2SummitOrdersApiController.php | 1 + 1 file changed, 1 insertion(+) diff --git a/app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitOrdersApiController.php b/app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitOrdersApiController.php index f19bec49c..c43de619d 100644 --- a/app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitOrdersApiController.php +++ b/app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitOrdersApiController.php @@ -442,6 +442,7 @@ public function getMyTicketByOrderHash($summit_id, $hash) responses: [ new OA\Response(response: Response::HTTP_NO_CONTENT, description: 'Order cancelled successfully'), new OA\Response(response: Response::HTTP_NOT_FOUND, description: 'Summit or order not found'), + new OA\Response(response: Response::HTTP_PRECONDITION_FAILED, description: 'Validation Error (order already paid, or payment configuration not set)'), ] )] public function cancel($summit_id, $hash)