hamedandClaude Opus 4.8
aa87b4a9cb
A non-unique index on (doctor_id, slot_start) plus a count-then-insert check left a TOCTOU race: two concurrent requests could both pass isSlotTaken and both insert. wrapInTransaction alone doesn't stop the phantom under InnoDB REPEATABLE-READ. Add a nullable, unique active_slot_key on Appointment = "doctorId:slotStart" while the booking occupies the slot (pending/confirmed — in lockstep with isSlotTaken); NULL once expired/completed/no_show/cancelled (NULLs don't collide in a MySQL unique index, so released slots rebook freely). bookAtomically now: catches the unique violation -> SlotTakenException, and expires lapsed pendings in-transaction so the ~1-min window before the expiry cron doesn't wrongly block rebooking. All three booking paths (online / my / admin) routed through it. Migration backfills one row per (doctor, slot) — the latest id — so the index builds even on dirty historical data without destructively cancelling bookings. (Backfill surfaced a real pre-existing double-booked slot in dev data.) Regression: tests/Appointment/SlotUniquenessTest. Adjusted the expiry-service test fixture to use distinct slots (one live booking per slot is now enforced). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
79 lines
2.4 KiB
PHP
79 lines
2.4 KiB
PHP
<?php
|
|
|
|
namespace App\Tests\Appointment;
|
|
|
|
use App\Appointment\Entity\Appointment;
|
|
use App\Appointment\Repository\SlotTakenException;
|
|
use App\Doctor\Entity\Doctor;
|
|
use App\Tests\ApiTestCase;
|
|
use Doctrine\DBAL\Exception\UniqueConstraintViolationException;
|
|
|
|
/**
|
|
* A doctor's (doctor_id, slot_start) can be held by at most one live booking.
|
|
* The unique active_slot_key is the race backstop behind bookAtomically:
|
|
* even if two concurrent transactions both pass the isSlotTaken re-check, only
|
|
* one INSERT can win. Cancelling/expiring a booking releases the slot.
|
|
*/
|
|
class SlotUniquenessTest extends ApiTestCase
|
|
{
|
|
private function makeDoctor(): Doctor
|
|
{
|
|
$doctor = new Doctor($this->createUser(['ROLE_DOCTOR']), 'دکتر تست');
|
|
$this->em->persist($doctor);
|
|
$this->em->flush();
|
|
|
|
return $doctor;
|
|
}
|
|
|
|
private function newBooking(Doctor $doctor, int $start): Appointment
|
|
{
|
|
return new Appointment($doctor, $this->createUser(), $start, $start + 1_800);
|
|
}
|
|
|
|
public function testTwoLiveBookingsOnSameSlotViolateUniqueKey(): void
|
|
{
|
|
$doctor = $this->makeDoctor();
|
|
$start = time() + 86_400;
|
|
|
|
$this->em->persist($this->newBooking($doctor, $start));
|
|
$this->em->flush();
|
|
|
|
$this->em->persist($this->newBooking($doctor, $start));
|
|
$this->expectException(UniqueConstraintViolationException::class);
|
|
$this->em->flush();
|
|
}
|
|
|
|
public function testCancelledBookingFreesTheSlot(): void
|
|
{
|
|
$doctor = $this->makeDoctor();
|
|
$start = time() + 86_400;
|
|
|
|
$first = $this->newBooking($doctor, $start);
|
|
$this->em->persist($first);
|
|
$this->em->flush();
|
|
|
|
$first->transitionTo(Appointment::STATUS_CANCELLED_BY_USER);
|
|
$this->em->flush();
|
|
|
|
$second = $this->newBooking($doctor, $start);
|
|
$this->em->persist($second);
|
|
$this->em->flush();
|
|
|
|
$this->assertNotNull($second->getId());
|
|
}
|
|
|
|
public function testBookAtomicallyRejectsAlreadyTakenSlot(): void
|
|
{
|
|
$doctor = $this->makeDoctor();
|
|
$start = time() + 86_400;
|
|
$repo = $this->em->getRepository(Appointment::class);
|
|
|
|
$first = $this->newBooking($doctor, $start);
|
|
$first->markPendingWithTtl(Appointment::PAYMENT_TTL);
|
|
$repo->bookAtomically($first);
|
|
|
|
$this->expectException(SlotTakenException::class);
|
|
$repo->bookAtomically($this->newBooking($doctor, $start));
|
|
}
|
|
}
|