From 1b3cc72358392d45fee819bed1923ef7a045a092 Mon Sep 17 00:00:00 2001 From: romanetar Date: Mon, 19 Oct 2026 17:37:56 +0200 Subject: [PATCH] fix(orders): return 412 instead of 500 when a concurrent reserve loses the SummitAttendee_Email_SummitID race ReserveOrderTask finds-or-creates the attendee by (summit, email) with no serialization, so parallel reserves from the same buyer fail on the root flush with a 1062 duplicate entry and the API answered 500. Catch UniqueConstraintViolationException outside the transaction closure (the flush happens after it returns), translate violations naming SummitAttendee_Email_SummitID into a ValidationException and rethrow any other index violation unchanged. --- app/Services/Model/Imp/SummitOrderService.php | 20 +++++- tests/SummitOrderServiceTest.php | 62 +++++++++++++++++++ 2 files changed, 80 insertions(+), 2 deletions(-) diff --git a/app/Services/Model/Imp/SummitOrderService.php b/app/Services/Model/Imp/SummitOrderService.php index 9f490a955..783dfa067 100644 --- a/app/Services/Model/Imp/SummitOrderService.php +++ b/app/Services/Model/Imp/SummitOrderService.php @@ -41,6 +41,7 @@ use App\Services\Model\Strategies\TicketFinder\ITicketFinderStrategyFactory; use App\Services\Utils\CSVReader; use App\Services\Utils\ILockManagerService; +use Doctrine\DBAL\Exception\UniqueConstraintViolationException; use Exception; use Illuminate\Http\UploadedFile; use Illuminate\Support\Facades\App; @@ -432,7 +433,7 @@ public function run(array $formerState): array { $this->formerState = $formerState; - return $this->tx_service->transaction(function () { + $reserve = function () { $owner_email = $this->payload['owner_email']; $owner_first_name = $this->payload['owner_first_name']; @@ -685,7 +686,22 @@ public function run(array $formerState): array // ApplyPromoCodeTask — which now runs after ReserveOrderTask — can read // the state populated by PreProcessReservationTask. return $this->formerState; - }); + }; + + try { + return $this->tx_service->transaction($reserve); + } catch (UniqueConstraintViolationException $ex) { + // raised by the root flush (after the closure returns), so it can't be caught inside the transaction. + // a concurrent reserve for the same attendee email won the race on the (SummitID, Email) unique index + if (!str_contains($ex->getMessage(), 'SummitAttendee_Email_SummitID')) throw $ex; + Log::warning(sprintf("ReserveOrderTask::run duplicate attendee on concurrent reserve: %s", $ex->getMessage())); + throw new ValidationException( + sprintf( + "A registration for %s is already being processed, please refresh and check your orders.", + $this->payload['owner_email'] + ) + ); + } } public function undo() diff --git a/tests/SummitOrderServiceTest.php b/tests/SummitOrderServiceTest.php index c6ccd2872..d9ab03acb 100644 --- a/tests/SummitOrderServiceTest.php +++ b/tests/SummitOrderServiceTest.php @@ -28,9 +28,12 @@ use App\Services\Model\ICompanyService; use App\Services\Model\IMemberService; use App\Services\Model\ISummitOrderService; +use App\Services\Model\ReserveOrderTask; use App\Services\Model\Strategies\TicketFinder\ITicketFinderStrategyFactory; use App\Services\Model\SummitOrderService; use App\Services\Utils\ILockManagerService; +use Doctrine\DBAL\Driver\PDO\Exception as PDODriverException; +use Doctrine\DBAL\Exception\UniqueConstraintViolationException; use Illuminate\Support\Facades\App; use Illuminate\Support\Facades\Cache; use Illuminate\Support\Facades\Config; @@ -669,6 +672,65 @@ public function testReserveAsGuestWithMultipleTicketsAutoAssignsFirstTicket() $this->assertEquals([$guest_email], $assigned); } + private function buildReserveOrderTaskThrowing(string $index_name, string $owner_email): ReserveOrderTask + { + $tx_service = Mockery::mock(ITransactionService::class); + // the violation is raised by the root flush after the closure returns, so the + // transaction call itself is what throws + $tx_service->shouldReceive('transaction')->andThrow( + new UniqueConstraintViolationException( + new PDODriverException( + sprintf("SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry '1-%s' for key '%s'", $owner_email, $index_name), + '23000', + 1062 + ), + null + ) + ); + + return new ReserveOrderTask( + null, + self::$summit, + ["owner_email" => $owner_email], + Mockery::mock(IBuildDefaultPaymentGatewayProfileStrategy::class), + Mockery::mock(IMemberRepository::class), + Mockery::mock(ISummitAttendeeRepository::class), + Mockery::mock(ISummitAttendeeTicketRepository::class), + Mockery::mock(ICompanyRepository::class), + Mockery::mock(ICompanyService::class), + $tx_service + ); + } + + /** + * A concurrent reserve for the same attendee email loses the race on the + * (SummitID, Email) unique index: it must be rejected as a 412 ValidationException. + */ + public function testReserveOrderTaskTranslatesDuplicateAttendeeToValidationException() + { + $email = sprintf('dup-%s@test.com', uniqid()); + $task = $this->buildReserveOrderTaskThrowing('SummitAttendee_Email_SummitID', $email); + + try { + $task->run([]); + $this->fail('Expected ValidationException was not thrown'); + } catch (ValidationException $ex) { + $this->assertStringContainsString($email, $ex->getMessage()); + $this->assertStringContainsString('already being processed', $ex->getMessage()); + } + } + + /** + * A unique violation on any other index must keep propagating unchanged. + */ + public function testReserveOrderTaskRethrowsOtherUniqueConstraintViolations() + { + $task = $this->buildReserveOrderTaskThrowing('SomeOther_Unique_Index', 'other@test.com'); + + $this->expectException(UniqueConstraintViolationException::class); + $task->run([]); + } + /** * ReserveTicketsTask rejects a reservation on a sold-out ticket type, and the * saga compensation leaves no order behind.