diff --git a/app/Models/Foundation/Summit/Registration/SponsorBadgeScan.php b/app/Models/Foundation/Summit/Registration/SponsorBadgeScan.php index d7340c63a..5984aeabc 100644 --- a/app/Models/Foundation/Summit/Registration/SponsorBadgeScan.php +++ b/app/Models/Foundation/Summit/Registration/SponsorBadgeScan.php @@ -92,6 +92,32 @@ class SponsorBadgeScan extends SponsorUserInfoGrant #[ORM\Column(name: 'Source', type: 'string', options: ['default' => self::Source_QR])] private $source; + /** + * Denormalized "::" identity of the physical + * scan this row represents, carrying a UNIQUE index (see the migration that + * adds SponsorBadgeScan_ScanDedupKey). That index - not the Redis dedup lock + * in SponsorUserInfoGrantService::addBadgeScanLocked - is what actually makes + * addBadgeScan idempotent: a lock with a TTL and no renewal cannot guarantee + * mutual exclusion (it can expire mid-transaction, and LockManagerService + * only logs the mismatch at release time), so the database has to be the + * authority on "one row per scan". + * + * The column has to live here rather than being an index over the tuple + * itself: SponsorUserInfoGrant/SponsorBadgeScan is a JOINED inheritance pair + * with SponsorID on the parent table and BadgeID/ScanDate on this one, and a + * UNIQUE index cannot span both tables. + * + * Nullable on purpose, and rows created before that migration keep NULL: + * MySQL allows any number of NULLs in a UNIQUE index, so pre-existing + * duplicates (which this bug already produced in production) neither block + * the index creation nor need deleting. Those historical rows stay covered by + * the explicit findExistingBadgeScan() check, which matches on the real + * columns; every new row gets a key and is covered by the index too. + * @var string|null + */ + #[ORM\Column(name: 'ScanDedupKey', type: 'string', nullable: true)] + private $scan_dedup_key; + /** * @var SponsorBadgeScanExtraQuestionAnswer[] */ @@ -169,6 +195,38 @@ public function setScanDate(\DateTime $scan_date): void $this->scan_date = $scan_date; } + /** + * Builds the value for the ScanDedupKey UNIQUE index from the tuple that + * identifies one physical scan. Uses the scan_date's epoch so the key is + * insensitive to how the DateTime was constructed (timezone, sub-second + * precision the DATETIME column would drop anyway) - the scanning app + * sends the timestamp as epoch seconds and resends it unchanged on a retry. + * @param Sponsor $sponsor + * @param SummitAttendeeBadge $badge + * @param \DateTime $scan_date + * @return string + */ + public static function buildDedupKey(Sponsor $sponsor, SummitAttendeeBadge $badge, \DateTime $scan_date): string + { + return sprintf('%d:%d:%d', $sponsor->getId(), $badge->getId(), $scan_date->getTimestamp()); + } + + /** + * @return string|null + */ + public function getScanDedupKey(): ?string + { + return $this->scan_dedup_key; + } + + /** + * @param string $scan_dedup_key + */ + public function setScanDedupKey(string $scan_dedup_key): void + { + $this->scan_dedup_key = $scan_dedup_key; + } + public function getAttendeeFirstName():?string{ $attendee = $this->getBadge()->getTicket()->getOwner(); return $attendee->hasMember() ? $attendee->getMember()->getFirstName() : $attendee->getFirstName(); diff --git a/app/Models/Foundation/Summit/Repositories/ISponsorUserInfoGrantRepository.php b/app/Models/Foundation/Summit/Repositories/ISponsorUserInfoGrantRepository.php index 715569b62..888221066 100644 --- a/app/Models/Foundation/Summit/Repositories/ISponsorUserInfoGrantRepository.php +++ b/app/Models/Foundation/Summit/Repositories/ISponsorUserInfoGrantRepository.php @@ -18,5 +18,14 @@ */ interface ISponsorUserInfoGrantRepository extends IBaseRepository { - + /** + * Looks up a previously persisted SponsorBadgeScan for the exact same + * (sponsor, badge, scan_date) tuple, used to make SponsorUserInfoGrantService::addBadgeScan + * idempotent against a client retry of the same scan (SUP-86b9fp53j). + * @param Sponsor $sponsor + * @param SummitAttendeeBadge $badge + * @param \DateTime $scan_date + * @return SponsorBadgeScan|null + */ + public function findExistingBadgeScan(Sponsor $sponsor, SummitAttendeeBadge $badge, \DateTime $scan_date): ?SponsorBadgeScan; } \ No newline at end of file diff --git a/app/Repositories/Summit/DoctrineSponsorUserInfoGrantRepository.php b/app/Repositories/Summit/DoctrineSponsorUserInfoGrantRepository.php index 67c7f7d51..2687609bb 100644 --- a/app/Repositories/Summit/DoctrineSponsorUserInfoGrantRepository.php +++ b/app/Repositories/Summit/DoctrineSponsorUserInfoGrantRepository.php @@ -15,8 +15,10 @@ use Doctrine\ORM\QueryBuilder; use models\summit\ISponsorUserInfoGrantRepository; use models\summit\Presentation; +use models\summit\Sponsor; use models\summit\SponsorBadgeScan; use models\summit\SponsorUserInfoGrant; +use models\summit\SummitAttendeeBadge; use models\summit\SummitEvent; use utils\DoctrineFilterMapping; use utils\DoctrineInstanceOfFilterMapping; @@ -124,4 +126,32 @@ protected function getBaseEntity() { return SponsorUserInfoGrant::class; } -} \ No newline at end of file + + /** + * Queries SponsorBadgeScan directly (not the generic filter/order pipeline + * above, which matches against the whole SponsorUserInfoGrant hierarchy and + * is meant for paged listing) for an exact (sponsor, badge, scan_date) match. + * Doctrine resolves the SponsorUserInfoGrant/SponsorBadgeScan joined-table + * inheritance transparently, so no manual join is needed here. + * @param Sponsor $sponsor + * @param SummitAttendeeBadge $badge + * @param \DateTime $scan_date + * @return SponsorBadgeScan|null + */ + public function findExistingBadgeScan(Sponsor $sponsor, SummitAttendeeBadge $badge, \DateTime $scan_date): ?SponsorBadgeScan + { + $query = $this->getEntityManager() + ->createQueryBuilder() + ->select("e") + ->from(SponsorBadgeScan::class, "e") + ->where("e.sponsor = :sponsor") + ->andWhere("e.badge = :badge") + ->andWhere("e.scan_date = :scan_date") + ->setParameter("sponsor", $sponsor) + ->setParameter("badge", $badge) + ->setParameter("scan_date", $scan_date) + ->setMaxResults(1); + + return $query->getQuery()->getOneOrNullResult(); + } +} diff --git a/app/Services/Model/Imp/SponsorUserInfoGrantService.php b/app/Services/Model/Imp/SponsorUserInfoGrantService.php index 811c8c5e5..027912a77 100644 --- a/app/Services/Model/Imp/SponsorUserInfoGrantService.php +++ b/app/Services/Model/Imp/SponsorUserInfoGrantService.php @@ -16,7 +16,8 @@ use App\Models\Foundation\Summit\Repositories\ISummitAttendeeBadgeRepository; use App\Services\Model\AbstractService; use App\Services\Model\ISponsorUserInfoGrantService; -use App\Utils\AES; +use App\Services\Utils\ILockManagerService; +use Doctrine\DBAL\Exception\UniqueConstraintViolationException; use Illuminate\Support\Facades\Log; use libs\utils\ITransactionService; use models\exceptions\EntityNotFoundException; @@ -58,12 +59,18 @@ final class SponsorUserInfoGrantService */ private $sponsor_repository; + /** + * @var ILockManagerService + */ + private $lock_service; + /** * @param ISponsorUserInfoGrantRepository $repository * @param ISummitAttendeeRepository $attendee_repository * @param ISummitAttendeeBadgeRepository $badge_repository * @param ISponsorRepository $sponsor_repository * @param ITransactionService $tx_service + * @param ILockManagerService $lock_service */ public function __construct ( @@ -71,7 +78,8 @@ public function __construct ISummitAttendeeRepository $attendee_repository, ISummitAttendeeBadgeRepository $badge_repository, ISponsorRepository $sponsor_repository, - ITransactionService $tx_service + ITransactionService $tx_service, + ILockManagerService $lock_service ) { parent::__construct($tx_service); @@ -79,6 +87,7 @@ public function __construct $this->attendee_repository = $attendee_repository; $this->badge_repository = $badge_repository; $this->sponsor_repository = $sponsor_repository; + $this->lock_service = $lock_service; } /** @@ -112,6 +121,28 @@ public function addGrant(Summit $summit, int $sponsor_id, Member $current_member }); } + /** + * Redis TTL (seconds) for the per-(sponsor, badge, scan_date) dedup lock + * below - a crash-safety ceiling (a process that dies mid-critical-section + * without releasing must not wedge that scan forever), not the + * acquire-contention timeout, which ILockManagerService governs on its + * own (LockManagerService::MaxRetries with backoff). + * + * This value is deliberately NOT a correctness parameter, and is kept + * short so an orphaned lock frees that scan quickly (acquireLock only + * waits ~0.7s before giving up, so a long-lived orphan would turn every + * retry of that one scan into a failure). It can't be one: the lock has + * no renewal and no fencing token, so it can expire while the transaction + * it wraps is still running - DoctrineTransactionService retries a root + * transaction up to MaxRetries = 10 on reconnectable errors with no + * backoff bounding the wall clock, and LockManagerService::releaseLock + * only logs 'lock was not held by this token at release time' when the + * TTL already lapsed. The SponsorBadgeScan.ScanDedupKey UNIQUE index is + * what actually guarantees one row per scan; the lock just keeps the + * common case from doing wasted work. + */ + private const BADGE_SCAN_LOCK_LIFETIME_SECONDS = 30; + /** * @param Summit $summit * @param Member $current_member @@ -121,137 +152,271 @@ public function addGrant(Summit $summit, int $sponsor_id, Member $current_member */ public function addBadgeScan(Summit $summit, Member $current_member, array $data): SponsorBadgeScan { - return $this->tx_service->transaction(function() use($summit, $current_member, $data){ - $raw_qr_code = $data['qr_code'] ?? null; - $raw_attendee_email = $data['attendee_email'] ?? null; - if(empty($raw_qr_code) && empty($raw_attendee_email)) - throw new ValidationException("Missing required parameters (qr_code or attendee_email)."); - $ticket_number = null; - $qr_code = null; - $source = null; - if(!empty($raw_qr_code)) { - $qr_code = SummitAttendeeBadge::decodeQRCodeFor($summit, $raw_qr_code); - $fields = SummitAttendeeBadge::parseQRCode($qr_code); - $prefix = $fields['prefix']; - if($summit->getBadgeQRPrefix() != $prefix) - throw new ValidationException + // Phase 1: parse the request and resolve the ticket/badge/sponsor it + // refers to. Entirely read-only (nothing is persisted here), so it + // runs outside any transaction - which is what lets the dedup lock + // below be acquired, keyed by (sponsor, badge, scan_date), BEFORE + // the transaction that actually creates the scan ever opens. + $raw_qr_code = $data['qr_code'] ?? null; + $raw_attendee_email = $data['attendee_email'] ?? null; + if(empty($raw_qr_code) && empty($raw_attendee_email)) + throw new ValidationException("Missing required parameters (qr_code or attendee_email)."); + $ticket_number = null; + $qr_code = null; + $source = null; + if(!empty($raw_qr_code)) { + $qr_code = SummitAttendeeBadge::decodeQRCodeFor($summit, $raw_qr_code); + $fields = SummitAttendeeBadge::parseQRCode($qr_code); + $prefix = $fields['prefix']; + if($summit->getBadgeQRPrefix() != $prefix) + throw new ValidationException + ( + sprintf ( - sprintf - ( - "%s qr code is not valid for summit %s.", - $qr_code, - $summit->getId() - ) - ); - $ticket_number = $fields['ticket_number']; - $source = SponsorBadgeScan::Source_QR; + "%s qr code is not valid for summit %s.", + $qr_code, + $summit->getId() + ) + ); + $ticket_number = $fields['ticket_number']; + $source = SponsorBadgeScan::Source_QR; + } + else if(!empty($raw_attendee_email)) { + $attendee = $this->attendee_repository->getBySummitAndEmail($summit, trim($raw_attendee_email)); + if(is_null($attendee)){ + throw new EntityNotFoundException("Attendee not found."); } - else if(!empty($raw_attendee_email)) { - $attendee = $this->attendee_repository->getBySummitAndEmail($summit, trim($raw_attendee_email)); - if(is_null($attendee)){ - throw new EntityNotFoundException("Attendee not found."); - } - $ticket = null; - foreach ($attendee->getTickets() as $t) { - if ($t->isActive() && $t->hasBadge()) { $ticket = $t; break; } - } - - if(is_null($ticket)){ - throw new EntityNotFoundException("Ticket not found."); - } - $ticket_number = $ticket->getNumber(); - $badge = $ticket->getBadge(); - // generate QR code on-demand if missing - $qr_code = $badge->generateQRCode(); - $qr_code = base64_encode($qr_code); - // normalize qr code - $qr_code = SummitAttendeeBadge::decodeQRCodeFor($summit, $qr_code); - $source = SponsorBadgeScan::Source_Attendee_Email; + $ticket = null; + foreach ($attendee->getTickets() as $t) { + if ($t->isActive() && $t->hasBadge()) { $ticket = $t; break; } } - $scan_date_epoch = intval($data['scan_date']); - $scan_date = new \DateTime("@$scan_date_epoch"); - $begin_date = $summit->getBeginDate(); - $end_date = $summit->getEndDate(); - - /* - if(!($scan_date >= $begin_date && $scan_date <= $end_date)) - throw new ValidationException("scan_date does not belong to summit period."); - */ - if(empty($ticket_number)){ - throw new ValidationException("Ticket not found."); + if(is_null($ticket)){ + throw new EntityNotFoundException("Ticket not found."); } - - $badge = $this->badge_repository->getBadgeByTicketNumber($ticket_number); - - if(is_null($badge)) - throw new EntityNotFoundException("badge not found."); - - // if we are and admin / show admin , then we need to provide the sponsor id - if($current_member->isAuthzFor($summit)){ - - Log::debug("SponsorUserInfoGrantService::addBadgeScan current member is an admin"); - - if (empty($data['sponsor_id'])) - throw new ValidationException("sponsor_id is required when current member is an admin."); - $sponsor_id = intval($data['sponsor_id']); - $sponsor = $this->sponsor_repository->getById($sponsor_id); - if(!$sponsor instanceof Sponsor){ - throw new EntityNotFoundException("Sponsor not found."); - } - if($sponsor->getSummitId() !== $summit->getId()){ - throw new ValidationException("Sponsor does not belong to this summit."); - } - Log::debug(sprintf("SponsorUserInfoGrantService::addBadgeScan selected sponsor %s (admin provided).", $sponsor->getId())); + $ticket_number = $ticket->getNumber(); + $badge = $ticket->getBadge(); + // generate QR code on-demand if missing + $qr_code = $badge->generateQRCode(); + $qr_code = base64_encode($qr_code); + // normalize qr code + $qr_code = SummitAttendeeBadge::decodeQRCodeFor($summit, $qr_code); + $source = SponsorBadgeScan::Source_Attendee_Email; + } + + $scan_date_epoch = intval($data['scan_date']); + $scan_date = new \DateTime("@$scan_date_epoch"); + + /* + $begin_date = $summit->getBeginDate(); + $end_date = $summit->getEndDate(); + + if(!($scan_date >= $begin_date && $scan_date <= $end_date)) + throw new ValidationException("scan_date does not belong to summit period."); + */ + if(empty($ticket_number)){ + throw new ValidationException("Ticket not found."); + } + + $badge = $this->badge_repository->getBadgeByTicketNumber($ticket_number); + + if(is_null($badge)) + throw new EntityNotFoundException("badge not found."); + + // if we are and admin / show admin , then we need to provide the sponsor id + if($current_member->isAuthzFor($summit)){ + + Log::debug("SponsorUserInfoGrantService::addBadgeScan current member is an admin"); + + if (empty($data['sponsor_id'])) + throw new ValidationException("sponsor_id is required when current member is an admin."); + $sponsor_id = intval($data['sponsor_id']); + $sponsor = $this->sponsor_repository->getById($sponsor_id); + if(!$sponsor instanceof Sponsor){ + throw new EntityNotFoundException("Sponsor not found."); + } + if($sponsor->getSummitId() !== $summit->getId()){ + throw new ValidationException("Sponsor does not belong to this summit."); } - else { - $member_sponsors = $current_member->getAccessibleSponsorsBySummit($summit); + Log::debug(sprintf("SponsorUserInfoGrantService::addBadgeScan selected sponsor %s (admin provided).", $sponsor->getId())); + } + else { + $member_sponsors = $current_member->getAccessibleSponsorsBySummit($summit); - if ($member_sponsors->isEmpty()) - throw new ValidationException("Current member does not have badge scan permissions for any sponsor of this summit."); + if ($member_sponsors->isEmpty()) + throw new ValidationException("Current member does not have badge scan permissions for any sponsor of this summit."); - if ($member_sponsors->count() === 1) { - $sponsor = $member_sponsors->first(); - Log::debug(sprintf("SponsorUserInfoGrantService::addBadgeScan selected sponsor %s (first).", $sponsor->getId())); - } else { - Log::debug("SponsorUserInfoGrantService::addBadgeScan current member is associated to multiple sponsors."); + if ($member_sponsors->count() === 1) { + $sponsor = $member_sponsors->first(); + Log::debug(sprintf("SponsorUserInfoGrantService::addBadgeScan selected sponsor %s (first).", $sponsor->getId())); + } else { + Log::debug("SponsorUserInfoGrantService::addBadgeScan current member is associated to multiple sponsors."); - if (empty($data['sponsor_id'])) - throw new ValidationException("sponsor_id is required when the member belongs to multiple sponsors."); + if (empty($data['sponsor_id'])) + throw new ValidationException("sponsor_id is required when the member belongs to multiple sponsors."); - $sponsor_id = intval($data['sponsor_id']); - $sponsor = $member_sponsors->filter(fn($s) => $s->getId() === $sponsor_id)->first(); + $sponsor_id = intval($data['sponsor_id']); + $sponsor = $member_sponsors->filter(fn($s) => $s->getId() === $sponsor_id)->first(); - if ($sponsor === false) - throw new ValidationException("Current member does not belong to the selected summit sponsor."); + if ($sponsor === false) + throw new ValidationException("Current member does not belong to the selected summit sponsor."); - Log::debug(sprintf("SponsorUserInfoGrantService::addBadgeScan selected sponsor %s (multiple).", $sponsor->getId())); + Log::debug(sprintf("SponsorUserInfoGrantService::addBadgeScan selected sponsor %s (multiple).", $sponsor->getId())); - } } + } - $scan = new SponsorBadgeScan(); - $scan->setScanDate($scan_date); - $scan->setQRCode($qr_code); - $scan->setUser($current_member); - $scan->setBadge($badge); - $scan->setSource($source); - $scan->setNotes(isset($data['notes'])? trim($data['notes']): ""); - - $sponsor->addUserInfoGrant($scan); - - // extra questions - $extra_questions = $data['extra_questions'] ?? []; + // Phase 2: create the scan, guarded against a concurrent duplicate + // for this same (sponsor, badge, scan_date) - see addBadgeScanLocked. + return $this->addBadgeScanLocked($sponsor, $badge, $scan_date, $scan_date_epoch, $qr_code, $source, $current_member, $data); + } - if (count($extra_questions)) { - $res = $scan->hadCompletedExtraQuestions($extra_questions); - if (!$res) { - throw new ValidationException("You neglected to fill in all mandatory questions for the badge scan."); + /** + * Creates (or, on a retry of the same scan, returns) the SponsorBadgeScan + * for the given (sponsor, badge, scan_date). SUP-86b9fp53j: the scanning + * app retries an upload whenever its own client-side timeout elapses, + * with no guarantee the original request didn't already reach this far + * and commit - two such requests reading "no existing scan yet" before + * either INSERTs is exactly how one physical badge scan ended up as two + * rows. A plain existence check right before the INSERT doesn't close + * that window under READ_COMMITTED (the isolation level + * ITransactionService::transaction defaults to): two concurrent + * transactions can both run the check before either commits. + * + * The invariant is enforced in two layers, and only the second one is + * authoritative: + * + * 1. ILockManagerService (Redis-backed; see SponsorUserSyncService and + * SummitOrderService for other callers) keyed by the same tuple + * serializes the common case, with the lock wrapping the whole + * transaction() call below and held past its COMMIT rather than + * released as soon as the row is attached in-memory - releasing any + * earlier would let a second request's existence check run (and find + * nothing) before the first request's INSERT is durable. This is an + * optimization: it keeps a retry from doing wasted work and from + * provoking the exception path below. + * + * 2. The SponsorBadgeScan.ScanDedupKey UNIQUE index is what actually + * guarantees one row per scan. The lock cannot: it has a TTL, no + * renewal and no fencing token, so it can lapse while the transaction + * it wraps is still running (DoctrineTransactionService retries a root + * transaction up to MaxRetries = 10 on reconnectable errors, with no + * backoff bounding the wall clock) and LockManagerService::releaseLock + * merely logs that it was no longer held. When that happens the INSERT + * is rejected by the index and the UniqueConstraintViolationException + * handler below resolves the retry to the row that won the race. + * + * Rows predating that index keep a NULL ScanDedupKey and are covered by + * the explicit existence check alone - see the column's own docblock. + * + * A lock the retries inside ILockManagerService::acquireLock can't get + * throws UnacquiredLockException, deliberately left to propagate (same + * as SponsorUserSyncService's own lock usage) rather than turned into a + * ValidationException: the scanning app's SyncService treats a non-4xx + * failure as transient and retries the scan on its own, which is the + * right outcome for lock contention - a ValidationException would mark + * it a permanent client error instead and stop retrying it. + * @param Sponsor $sponsor + * @param SummitAttendeeBadge $badge + * @param \DateTime $scan_date + * @param int $scan_date_epoch + * @param string $qr_code + * @param string $source + * @param Member $current_member + * @param array $data + * @return SponsorBadgeScan + * @throws \Exception + */ + private function addBadgeScanLocked( + Sponsor $sponsor, + SummitAttendeeBadge $badge, + \DateTime $scan_date, + int $scan_date_epoch, + string $qr_code, + string $source, + Member $current_member, + array $data + ): SponsorBadgeScan + { + $lock_name = sprintf('badge_scan.%d.%d.%d.lock', $sponsor->getId(), $badge->getId(), $scan_date_epoch); + $dedup_key = SponsorBadgeScan::buildDedupKey($sponsor, $badge, $scan_date); + + try { + return $this->lock_service->lock($lock_name, function() use($sponsor, $badge, $scan_date, $dedup_key, $qr_code, $source, $current_member, $data){ + return $this->tx_service->transaction(function() use($sponsor, $badge, $scan_date, $dedup_key, $qr_code, $source, $current_member, $data){ + $existing = $this->repository->findExistingBadgeScan($sponsor, $badge, $scan_date); + if(!is_null($existing)){ + Log::warning( + sprintf( + "SponsorUserInfoGrantService::addBadgeScan duplicate scan detected for sponsor %s badge %s scan_date %s - returning existing scan %s", + $sponsor->getId(), + $badge->getId(), + $scan_date->getTimestamp(), + $existing->getId() + ) + ); + return $existing; + } + + $scan = new SponsorBadgeScan(); + $scan->setScanDate($scan_date); + $scan->setQRCode($qr_code); + $scan->setUser($current_member); + $scan->setBadge($badge); + $scan->setSource($source); + $scan->setNotes(isset($data['notes'])? trim($data['notes']): ""); + // Populates the column carrying the UNIQUE index, which is what + // actually rejects a duplicate if the lock above failed to serialize + // this request - the check right above only closes the window it can see. + $scan->setScanDedupKey($dedup_key); + + $sponsor->addUserInfoGrant($scan); + + // extra questions + $extra_questions = $data['extra_questions'] ?? []; + + if (count($extra_questions)) { + $res = $scan->hadCompletedExtraQuestions($extra_questions); + if (!$res) { + throw new ValidationException("You neglected to fill in all mandatory questions for the badge scan."); + } + } + + return $scan; + }); + }, self::BADGE_SCAN_LOCK_LIFETIME_SECONDS); + } + catch(UniqueConstraintViolationException $ex){ + // SponsorBadgeScan_ScanDedupKey rejected the INSERT: another request for + // this same physical scan committed first, so the existence check above + // ran before that row was visible - either because the dedup lock's TTL + // lapsed mid-transaction (it has no renewal, and releaseLock only logs + // the mismatch) or because the two requests never contended on it at all. + // Either way the retry is satisfied by returning the row that won. + // + // Caught out here rather than inside the closures on purpose: a failed + // flush leaves the EntityManager closed and the connection rollback-only, + // so the winning row can only be re-read in a fresh transaction. Same + // placement as SummitService::addEventToMemberSchedule's own handling. + // DoctrineTransactionService::shouldReconnect() does not treat this as + // reconnectable, so it reaches us instead of being retried. + Log::warning( + sprintf( + "SponsorUserInfoGrantService::addBadgeScan unique violation on dedup key %s - a concurrent request won the race, resolving to the committed scan.", + $dedup_key + ) + ); + + return $this->tx_service->transaction(function() use($sponsor, $badge, $scan_date, $ex){ + $existing = $this->repository->findExistingBadgeScan($sponsor, $badge, $scan_date); + if(is_null($existing)){ + // Not our tuple - some other unique index on the scan or its answers + // rejected the write, and swallowing that would hide a real failure. + throw $ex; } - } - - return $scan; - }); + return $existing; + }); + } } /** diff --git a/database/migrations/model/Version20260910181020.php b/database/migrations/model/Version20260910181020.php new file mode 100644 index 000000000..ef11489a3 --- /dev/null +++ b/database/migrations/model/Version20260910181020.php @@ -0,0 +1,84 @@ +addSql(<<addSql(<<addSql(<<addSql(<<clearGroups(); + self::$member->add2Group($this->sponsor_group); + self::$em->persist(self::$member); + self::$em->flush(); + + $sponsor = self::$summit->getSummitSponsors()[0]; + $sponsor->addUser(self::$member); + self::$em->persist($sponsor); + self::$em->flush(); + + $params = [ + 'id' => self::$summit->getId(), + ]; + + $attendee = self::$summit->getAttendeeByMemberId(self::$defaultMember->getId()); + $badge = $attendee->getFirstTicket()->getBadge(); + + // Generated once and reused across both requests: a real retry + // resends the exact same body, it doesn't re-derive the QR code. + $data = [ + 'qr_code' => $badge->generateQRCode(), + 'scan_date' => 1572019200, + 'sponsor_id' => $sponsor->getId(), + ]; + $body = json_encode($data); + + $first_response = $this->action( + "POST", + "OAuth2SummitBadgeScanApiController@add", + $params, + [], + [], + [], + $this->getAuthHeaders(), + $body + ); + + $this->assertResponseStatus(201); + $first_scan = json_decode($first_response->getContent()); + $this->assertTrue(!is_null($first_scan)); + + $second_response = $this->action( + "POST", + "OAuth2SummitBadgeScanApiController@add", + $params, + [], + [], + [], + $this->getAuthHeaders(), + $body + ); + + $this->assertResponseStatus(201); + $second_scan = json_decode($second_response->getContent()); + $this->assertTrue(!is_null($second_scan)); + + $this->assertEquals($first_scan->id, $second_scan->id, + "a retry of the identical scan must return the same entity, not create a new one"); + + $count = self::$em->getRepository(\models\summit\SponsorBadgeScan::class) + ->count(['sponsor' => $sponsor, 'badge' => $badge]); + $this->assertEquals(1, $count, + "exactly one SponsorBadgeScan row must exist for this sponsor+badge+scan_date, not two"); + } + + /** + * A different scan_date for the same sponsor+badge must NOT be + * deduplicated - it's a genuine second scan (e.g. the sponsor scanned + * this attendee again later), not a retry of the same attempt. + */ + public function testAddBadgeScanWithDifferentScanDateIsNotDeduplicated(){ + self::$member->clearGroups(); + self::$member->add2Group($this->sponsor_group); + self::$em->persist(self::$member); + self::$em->flush(); + + $sponsor = self::$summit->getSummitSponsors()[0]; + $sponsor->addUser(self::$member); + self::$em->persist($sponsor); + self::$em->flush(); + + $params = [ + 'id' => self::$summit->getId(), + ]; + + $attendee = self::$summit->getAttendeeByMemberId(self::$defaultMember->getId()); + $badge = $attendee->getFirstTicket()->getBadge(); + $qr_code = $badge->generateQRCode(); + + $first_response = $this->action( + "POST", + "OAuth2SummitBadgeScanApiController@add", + $params, + [], + [], + [], + $this->getAuthHeaders(), + json_encode(['qr_code' => $qr_code, 'scan_date' => 1572019200, 'sponsor_id' => $sponsor->getId()]) + ); + $this->assertResponseStatus(201); + $first_scan = json_decode($first_response->getContent()); + + $second_response = $this->action( + "POST", + "OAuth2SummitBadgeScanApiController@add", + $params, + [], + [], + [], + $this->getAuthHeaders(), + json_encode(['qr_code' => $qr_code, 'scan_date' => 1572019260, 'sponsor_id' => $sponsor->getId()]) + ); + $this->assertResponseStatus(201); + $second_scan = json_decode($second_response->getContent()); + + $this->assertNotEquals($first_scan->id, $second_scan->id, + "a genuinely later scan of the same badge must not be collapsed into the earlier one"); + } + + /** + * The two tests above prove the end result (one row survives two + * identical POSTs), but a plain "check then insert" with no locking at + * all would pass them too, since PHPUnit calls are strictly sequential - + * they never actually overlap two in-flight requests. This test proves + * the lock itself is what SponsorUserInfoGrantService::addBadgeScan + * acquires: holding the exact lock name it should use externally, then + * calling the real service directly (bypassing HTTP, so the thrown + * exception type is visible), and asserting it fails to acquire the + * lock and gives up - the concurrency-closing mechanism this fix + * actually depends on for a genuine race, not just the happy path. + */ + public function testAddBadgeScanBlocksOnAConcurrentLockHolder(){ + self::$member->clearGroups(); + self::$member->add2Group($this->sponsor_group); + self::$em->persist(self::$member); + self::$em->flush(); + + $sponsor = self::$summit->getSummitSponsors()[0]; + $sponsor->addUser(self::$member); + self::$em->persist($sponsor); + self::$em->flush(); + + $attendee = self::$summit->getAttendeeByMemberId(self::$defaultMember->getId()); + $badge = $attendee->getFirstTicket()->getBadge(); + $qr_code = $badge->generateQRCode(); + $scan_date_epoch = 1572019200; + + // Same construction as SponsorUserInfoGrantService::addBadgeScanLocked's + // $lock_name - deliberately duplicated (not called via a shared + // constant) so this test also catches a future change to that + // format silently no longer matching what's held here. + $lock_name = sprintf('badge_scan.%d.%d.%d.lock', $sponsor->getId(), $badge->getId(), $scan_date_epoch); + + $lock_service = App::make(ILockManagerService::class); + $held_token = $lock_service->acquireLock($lock_name, 10); + + $data = [ + 'qr_code' => $qr_code, + 'scan_date' => $scan_date_epoch, + 'sponsor_id' => $sponsor->getId(), + ]; + + $service = App::make(ISponsorUserInfoGrantService::class); + + $threw = false; + try { + $service->addBadgeScan(self::$summit, self::$member, $data); + } catch (UnacquiredLockException $ex) { + $threw = true; + } finally { + $lock_service->releaseLock($lock_name, $held_token); + } + $this->assertTrue($threw, + "addBadgeScan must fail to acquire a lock already held under the exact name this test holds - ". + "either it isn't locking on (sponsor, badge, scan_date) at all, or the name format drifted"); + + // With the external holder gone, the same call now succeeds. + $scan = $service->addBadgeScan(self::$summit, self::$member, $data); + $this->assertNotNull($scan); + $this->assertEquals($sponsor->getId(), $scan->getSponsor()->getId()); + } + public function testAddBadgeScanByAttendeeEmail(){ self::$member->clearGroups(); self::$member->add2Group($this->sponsor_group); @@ -864,4 +1062,93 @@ public function testExportSummitBadgeScansWithAllReportSettingsRestriction(){ $this->assertResponseStatus(200); $this->assertNotEmpty($content); } + + /** + * The dedup lock cannot guarantee one row per scan on its own - it has a + * TTL, no renewal and no fencing token, so it can lapse while the + * transaction it wraps is still running. The SponsorBadgeScan.ScanDedupKey + * UNIQUE index is what does, so this asserts the index is actually there + * and rejecting: two rows carrying the same key must not both persist, + * whatever the service layer above happens to do. + */ + public function testScanDedupKeyUniqueIndexRejectsADuplicateRow(){ + $sponsor = self::$summit->getSummitSponsors()[0]; + $attendee = self::$summit->getAttendeeByMemberId(self::$defaultMember->getId()); + $badge = $attendee->getFirstTicket()->getBadge(); + $scan_date = new \DateTime("@1572019200"); + + $dedup_key = \models\summit\SponsorBadgeScan::buildDedupKey($sponsor, $badge, $scan_date); + + $first = new \models\summit\SponsorBadgeScan(); + $first->setScanDate($scan_date); + $first->setQRCode('dedup-index-test'); + $first->setUser(self::$member); + $first->setBadge($badge); + $first->setNotes(''); + $first->setScanDedupKey($dedup_key); + $sponsor->addUserInfoGrant($first); + self::$em->persist($first); + self::$em->flush(); + + // Byte-identical key, which is the whole point: a second physical row for + // one scan is what the production bug produced and what the index forbids. + $second = new \models\summit\SponsorBadgeScan(); + $second->setScanDate($scan_date); + $second->setQRCode('dedup-index-test'); + $second->setUser(self::$member); + $second->setBadge($badge); + $second->setNotes(''); + $second->setScanDedupKey($dedup_key); + $sponsor->addUserInfoGrant($second); + self::$em->persist($second); + + $threw = false; + try { + self::$em->flush(); + } catch (UniqueConstraintViolationException $ex) { + $threw = true; + } + + $this->assertTrue($threw, + "SponsorBadgeScan_ScanDedupKey must reject a second row with the same dedup key - ". + "without that index the lock's TTL is the only thing preventing duplicates, which it cannot be"); + } + + /** + * The index above only protects rows that actually carry a key, so this + * asserts the service populates it: a scan created through the real + * addBadgeScan path must come out with the ScanDedupKey its + * (sponsor, badge, scan_date) tuple implies. If this regressed, every new + * row would go in with NULL - which MySQL allows without limit in a UNIQUE + * index - and the duplicate protection would silently be gone. + */ + public function testAddBadgeScanPopulatesTheScanDedupKey(){ + self::$member->clearGroups(); + self::$member->add2Group($this->sponsor_group); + self::$em->persist(self::$member); + self::$em->flush(); + + $sponsor = self::$summit->getSummitSponsors()[0]; + $sponsor->addUser(self::$member); + self::$em->persist($sponsor); + self::$em->flush(); + + $attendee = self::$summit->getAttendeeByMemberId(self::$defaultMember->getId()); + $badge = $attendee->getFirstTicket()->getBadge(); + $scan_date_epoch = 1572019200; + + $service = App::make(ISponsorUserInfoGrantService::class); + $scan = $service->addBadgeScan(self::$summit, self::$member, [ + 'qr_code' => $badge->generateQRCode(), + 'scan_date' => $scan_date_epoch, + 'sponsor_id' => $sponsor->getId(), + ]); + + $this->assertNotNull($scan); + $this->assertEquals( + sprintf('%d:%d:%d', $sponsor->getId(), $badge->getId(), $scan_date_epoch), + $scan->getScanDedupKey(), + "addBadgeScan must stamp the dedup key on the new scan, otherwise the UNIQUE index protects nothing" + ); + } }