From e1740462b5046cdc6e053f9ef8d78ecee22772e1 Mon Sep 17 00:00:00 2001 From: hamed <15238-genius.ha@users.noreply.drupalcode.org> Date: Sun, 28 Jun 2026 17:10:32 +0330 Subject: [PATCH] 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. --- .../Service/AppointmentExpiryService.php | 6 ++- src/Payment/Repository/PaymentRepository.php | 30 +++++++++++ .../AppointmentExpiryServiceTest.php | 52 +++++++++++++++++++ 3 files changed, 87 insertions(+), 1 deletion(-) create mode 100644 tests/Appointment/AppointmentExpiryServiceTest.php diff --git a/src/Appointment/Service/AppointmentExpiryService.php b/src/Appointment/Service/AppointmentExpiryService.php index 639ba3b0..d903b014 100644 --- a/src/Appointment/Service/AppointmentExpiryService.php +++ b/src/Appointment/Service/AppointmentExpiryService.php @@ -29,12 +29,16 @@ class AppointmentExpiryService $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; foreach ($expired as $appointment) { $appointment->transitionTo(Appointment::STATUS_EXPIRED); $this->appointmentRepo->save($appointment, false); - $payment = $this->paymentRepo->findPendingByAppointment($appointment); + $payment = $pendingByAppointment[$appointment->getId()] ?? null; if ($payment !== null) { $payment->setStatus(Payment::STATUS_CANCELED); $this->paymentRepo->save($payment, false); diff --git a/src/Payment/Repository/PaymentRepository.php b/src/Payment/Repository/PaymentRepository.php index 283c0e54..73dedf4d 100644 --- a/src/Payment/Repository/PaymentRepository.php +++ b/src/Payment/Repository/PaymentRepository.php @@ -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 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 { $this->getEntityManager()->persist($entity); diff --git a/tests/Appointment/AppointmentExpiryServiceTest.php b/tests/Appointment/AppointmentExpiryServiceTest.php new file mode 100644 index 00000000..060aaea5 --- /dev/null +++ b/tests/Appointment/AppointmentExpiryServiceTest.php @@ -0,0 +1,52 @@ +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()); + } + } +}