From 8065ae3be1a1c904b2ac0d6d0a17596c2ec0be18 Mon Sep 17 00:00:00 2001 From: hamed <15238-genius.ha@users.noreply.drupalcode.org> Date: Sat, 18 Jul 2026 14:06:38 +0330 Subject: [PATCH] test: enable Doctrine profiling in the test env and fix the N+1 it exposed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit APP_DEBUG=0 in .env means doctrine.dbal.profiling, which defaults to %kernel.debug%, was off in tests too, so doctrine.debug_data_holder was never registered. Every test calling countQueries() errored out — all four N+1 regression tests had been dead for as long as they have existed. Turning profiling on for when@test brings the harness back. Three of the four passed immediately. The fourth was a real N+1: the service-coverage endpoint batch-fetched its ServiceItem entities to avoid one find() per row, but ServiceItem maps staffMembers as fetch: EAGER, so hydrating N items fired N extra collection loads and the batch bought nothing. Six coverage rows cost 11 queries where one row cost 6. ServiceItemRepository::findUuidsByIds() returns the id => uuid map as a scalar query, so no entity is hydrated and no eager collection is touched. Also adds the query-count assertion for next_available_at that could not be written while the harness was broken. Confirmed it fails against the previous per-day implementation (40 queries for 2 locations, 113 for 6) and passes now. Suite: 411 tests, 2 failures — both pre-existing and unrelated (LowTierFixesTest, PatientWalletSessionSettleTest). Co-Authored-By: Claude Opus 4.8 (1M context) --- config/packages/doctrine.yaml | 4 ++ .../Repository/ServiceItemRepository.php | 26 +++++++++++++ .../Controller/InsuranceController.php | 13 ++----- .../Appointment/BookingLocationsScanTest.php | 39 ++++++++++++++++--- 4 files changed, 68 insertions(+), 14 deletions(-) diff --git a/config/packages/doctrine.yaml b/config/packages/doctrine.yaml index c126e408..939b3c82 100644 --- a/config/packages/doctrine.yaml +++ b/config/packages/doctrine.yaml @@ -26,6 +26,10 @@ when@test: doctrine: dbal: dbname_suffix: '_test%env(default::TEST_TOKEN)%' + # APP_DEBUG=0 در .env پروژه است، پس profiling که پیش‌فرضش %kernel.debug% + # است خاموش می‌ماند و سرویس doctrine.debug_data_holder ساخته نمی‌شود. + # تست‌های N+1 برای شمارش کوئری به آن نیاز دارند. + profiling: true when@prod: doctrine: diff --git a/src/ClinicService/Repository/ServiceItemRepository.php b/src/ClinicService/Repository/ServiceItemRepository.php index 9720fa0b..7d998481 100644 --- a/src/ClinicService/Repository/ServiceItemRepository.php +++ b/src/ClinicService/Repository/ServiceItemRepository.php @@ -109,6 +109,32 @@ class ServiceItemRepository extends ServiceEntityRepository ->getResult(); } + /** + * نگاشت id → uuid برای مجموعه‌ای از خدمات. + * + * عمداً entity هیدریت نمی‌کند: ServiceItem رابطهٔ staffMembers را EAGER دارد، + * پس هر entity یک کوئری اضافه برای بارگذاری کارکنانش می‌زند و فهرستی که فقط + * uuid می‌خواهد به N+1 می‌افتد. + * + * @param int[] $ids + * @return array + */ + public function findUuidsByIds(array $ids): array + { + if ($ids === []) { + return []; + } + + $rows = $this->createQueryBuilder('i') + ->select('i.id AS id, i.uuid AS uuid') + ->where('i.id IN (:ids)') + ->setParameter('ids', $ids) + ->getQuery() + ->getScalarResult(); + + return array_column($rows, 'uuid', 'id'); + } + public function save(ServiceItem $item): void { $this->getEntityManager()->persist($item); diff --git a/src/Insurance/Controller/InsuranceController.php b/src/Insurance/Controller/InsuranceController.php index e2d7dd8b..4a2f410b 100644 --- a/src/Insurance/Controller/InsuranceController.php +++ b/src/Insurance/Controller/InsuranceController.php @@ -493,15 +493,10 @@ class InsuranceController extends BaseController $rows = $this->serviceCoverageRepo->findByContract($contract->getId()); - // Batch-fetch the referenced service items once instead of one find() - // per coverage row (N+1). - $itemIds = array_values(array_unique(array_map(fn($r) => $r->getServiceItemId(), $rows))); - $uuidById = []; - if ($itemIds !== []) { - foreach ($this->serviceItemRepo->findBy(['id' => $itemIds]) as $item) { - $uuidById[$item->getId()] = $item->getUuid(); - } - } + // یک کوئری اسکالر برای همهٔ uuidها. هیدریت‌کردن entity کافی نیست: رابطهٔ + // EAGER staffMembers روی ServiceItem به ازای هر ردیف یک کوئری اضافه می‌زند. + $itemIds = array_values(array_unique(array_map(fn($r) => $r->getServiceItemId(), $rows))); + $uuidById = $this->serviceItemRepo->findUuidsByIds($itemIds); $data = array_map(function ($r) use ($uuidById) { $row = $r->toArray(); diff --git a/tests/Appointment/BookingLocationsScanTest.php b/tests/Appointment/BookingLocationsScanTest.php index 8e587065..28ddda77 100644 --- a/tests/Appointment/BookingLocationsScanTest.php +++ b/tests/Appointment/BookingLocationsScanTest.php @@ -12,11 +12,9 @@ use App\Tests\ApiTestCase; * `next_available_at` scans ahead for the first free slot per location. * * The scan prefetches the schedule, holidays, overrides and taken appointments - * once per location and resolves the rest in memory, so its cost does not grow - * with how far ahead the first opening is. A query-count assertion would be the - * natural guard, but countQueries() needs `doctrine.debug_data_holder`, which - * this test environment does not expose — the existing N+1 tests fail on that - * same missing service. This covers the observable contract instead. + * once per location and resolves the rest in memory, so its cost must not grow + * with the number of days walked or slots inspected — only with the number of + * locations, by a small constant. */ class BookingLocationsScanTest extends ApiTestCase { @@ -73,6 +71,37 @@ class BookingLocationsScanTest extends ApiTestCase return $doctor; } + public function testQueryCountGrowsOnlyPerLocation(): void + { + // Keep one kernel so the shared query logger stays consistent. + $this->client->disableReboot(); + + $few = $this->makeDoctorWithSchedules(1); + $many = $this->makeDoctorWithSchedules(5); + + $qFew = $this->countQueries(fn () => $this->client->request( + 'GET', '/api/v1/appointment-booking-locations/' . $few->getUuid() + )); + self::assertSame(200, $this->responseCode()); + + $qMany = $this->countQueries(fn () => $this->client->request( + 'GET', '/api/v1/appointment-booking-locations/' . $many->getUuid() + )); + self::assertSame(200, $this->responseCode()); + + // 2 locations -> 6 locations. Each extra one costs a fixed handful of + // queries (its schedule, holidays, overrides, blocking intervals, + // address). The regression this guards against is a per-day or per-slot + // query, which on a schedule active every day would add hundreds. + $extraLocations = 4; + $budgetEach = 8; + self::assertLessThanOrEqual( + $qFew + $extraLocations * $budgetEach, + $qMany, + "next_available_at scales badly: $qFew queries for 2 locations, $qMany for 6" + ); + } + public function testNextAvailableIsReportedPerLocation(): void { $doctor = $this->makeDoctorWithSchedules(1);