perf(appointment): batch-fetch pending payments in expiry loop (fix N+1)
AppointmentExpiryService ran one findPendingByAppointment query per expiring booking. Add PaymentRepository::findPendingByAppointments (one IN query keyed by appointment id) and use it. Test covers expiry + payment cancellation for several appointments at once.
This commit is contained in:
@@ -29,12 +29,16 @@ class AppointmentExpiryService
|
|||||||
$expired[$appointment->getUuid()] = $appointment;
|
$expired[$appointment->getUuid()] = $appointment;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Fetch all pending payments for the expiring appointments in one query
|
||||||
|
// instead of one lookup per appointment (was N+1 in this scheduler loop).
|
||||||
|
$pendingByAppointment = $this->paymentRepo->findPendingByAppointments(array_values($expired));
|
||||||
|
|
||||||
$count = 0;
|
$count = 0;
|
||||||
foreach ($expired as $appointment) {
|
foreach ($expired as $appointment) {
|
||||||
$appointment->transitionTo(Appointment::STATUS_EXPIRED);
|
$appointment->transitionTo(Appointment::STATUS_EXPIRED);
|
||||||
$this->appointmentRepo->save($appointment, false);
|
$this->appointmentRepo->save($appointment, false);
|
||||||
|
|
||||||
$payment = $this->paymentRepo->findPendingByAppointment($appointment);
|
$payment = $pendingByAppointment[$appointment->getId()] ?? null;
|
||||||
if ($payment !== null) {
|
if ($payment !== null) {
|
||||||
$payment->setStatus(Payment::STATUS_CANCELED);
|
$payment->setStatus(Payment::STATUS_CANCELED);
|
||||||
$this->paymentRepo->save($payment, false);
|
$this->paymentRepo->save($payment, false);
|
||||||
|
|||||||
@@ -62,6 +62,36 @@ class PaymentRepository extends ServiceEntityRepository
|
|||||||
]);
|
]);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Batch variant of findPendingByAppointment: all pending payments for the
|
||||||
|
* given appointments in ONE query, keyed by appointment id. Avoids the N+1
|
||||||
|
* in AppointmentExpiryService when many bookings expire at once.
|
||||||
|
*
|
||||||
|
* @param Appointment[] $appointments
|
||||||
|
* @return array<int, Payment> appointment id => pending Payment
|
||||||
|
*/
|
||||||
|
public function findPendingByAppointments(array $appointments): array
|
||||||
|
{
|
||||||
|
if ($appointments === []) {
|
||||||
|
return [];
|
||||||
|
}
|
||||||
|
|
||||||
|
$payments = $this->createQueryBuilder('p')
|
||||||
|
->andWhere('p.appointment IN (:appointments)')
|
||||||
|
->andWhere('p.status = :status')
|
||||||
|
->setParameter('appointments', $appointments)
|
||||||
|
->setParameter('status', Payment::STATUS_PENDING)
|
||||||
|
->getQuery()
|
||||||
|
->getResult();
|
||||||
|
|
||||||
|
$byAppointment = [];
|
||||||
|
foreach ($payments as $payment) {
|
||||||
|
$byAppointment[$payment->getAppointment()->getId()] = $payment;
|
||||||
|
}
|
||||||
|
|
||||||
|
return $byAppointment;
|
||||||
|
}
|
||||||
|
|
||||||
public function save(Payment $entity, bool $flush = true): void
|
public function save(Payment $entity, bool $flush = true): void
|
||||||
{
|
{
|
||||||
$this->getEntityManager()->persist($entity);
|
$this->getEntityManager()->persist($entity);
|
||||||
|
|||||||
@@ -0,0 +1,52 @@
|
|||||||
|
<?php
|
||||||
|
|
||||||
|
namespace App\Tests\Appointment;
|
||||||
|
|
||||||
|
use App\Appointment\Entity\Appointment;
|
||||||
|
use App\Appointment\Service\AppointmentExpiryService;
|
||||||
|
use App\Doctor\Entity\Doctor;
|
||||||
|
use App\Payment\Entity\Payment;
|
||||||
|
use App\Tests\ApiTestCase;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Covers AppointmentExpiryService: stale pending bookings are expired and their
|
||||||
|
* pending payments cancelled. Also guards the N+1 fix (batch payment fetch) by
|
||||||
|
* exercising several appointments at once.
|
||||||
|
*/
|
||||||
|
class AppointmentExpiryServiceTest extends ApiTestCase
|
||||||
|
{
|
||||||
|
public function testExpiresStaleAndCancelsPendingPayments(): void
|
||||||
|
{
|
||||||
|
$owner = $this->createUser(['ROLE_DOCTOR']);
|
||||||
|
$doctor = new Doctor($owner, 'دکتر تست');
|
||||||
|
$this->em->persist($doctor);
|
||||||
|
|
||||||
|
$past = time() - 3600;
|
||||||
|
$appointments = [];
|
||||||
|
for ($i = 0; $i < 5; $i++) {
|
||||||
|
$patient = $this->createUser(['ROLE_USER']);
|
||||||
|
$appt = new Appointment($doctor, $patient, $past, $past + 900);
|
||||||
|
$this->em->persist($appt);
|
||||||
|
|
||||||
|
$payment = new Payment($patient, 100_000, 'mellat', 'appointment');
|
||||||
|
$payment->setAppointment($appt);
|
||||||
|
$this->em->persist($payment);
|
||||||
|
|
||||||
|
$appointments[] = [$appt, $payment];
|
||||||
|
}
|
||||||
|
$this->em->flush();
|
||||||
|
|
||||||
|
$service = static::getContainer()->get(AppointmentExpiryService::class);
|
||||||
|
$count = $service->expireStale();
|
||||||
|
|
||||||
|
$this->assertSame(5, $count);
|
||||||
|
|
||||||
|
$this->em->clear();
|
||||||
|
foreach ($appointments as [$appt, $payment]) {
|
||||||
|
$freshAppt = $this->em->getRepository(Appointment::class)->find($appt->getId());
|
||||||
|
$freshPay = $this->em->getRepository(Payment::class)->find($payment->getId());
|
||||||
|
$this->assertSame(Appointment::STATUS_EXPIRED, $freshAppt->getStatus());
|
||||||
|
$this->assertSame(Payment::STATUS_CANCELED, $freshPay->getStatus());
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user