From eb1997066e6a86efe0313112e1e32a024838cb1a Mon Sep 17 00:00:00 2001 From: hamed <15238-genius.ha@users.noreply.drupalcode.org> Date: Sun, 28 Jun 2026 19:29:06 +0330 Subject: [PATCH] perf(insurance): batch-fetch service items in coverage list (H8) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit listServiceCoverage called serviceItemRepo->find() once per coverage row (N+1). Collect the ids and fetch them in one findBy(['id' => $ids]), then map by id. Regression: tests/Insurance/ServiceCoverageNPlusOneTest (functional correctness — every row resolves the right service_item_uuid). Co-Authored-By: Claude Opus 4.8 --- docs/audit-backlog.md | 2 +- .../Controller/InsuranceController.php | 15 ++++- .../Insurance/ServiceCoverageNPlusOneTest.php | 57 +++++++++++++++++++ 3 files changed, 70 insertions(+), 4 deletions(-) create mode 100644 tests/Insurance/ServiceCoverageNPlusOneTest.php diff --git a/docs/audit-backlog.md b/docs/audit-backlog.md index eb074735..6b036c40 100644 --- a/docs/audit-backlog.md +++ b/docs/audit-backlog.md @@ -44,7 +44,7 @@ _None outstanding._ | ✅H5 | Insurance pricing/coverage modeled as raw int FKs (no FK/onDelete) → orphan rows on delete | src/Insurance/Entity/EntityInsurancePricing.php · TenantInsurance.php · TenantServiceCoverage.php | db-ondelete | **DONE (entity-owner path)** — `entity_id` is polymorphic (doctor\|clinic) so no DB FK is possible; added `TenantInsuranceCleanupService::purgeForEntity()` wired into doctor + clinic DELETE (purges tenant_insurances + pricing + coverage). `tests/Insurance/TenantInsuranceCleanupTest`. **Residual (→ M20-adjacent):** orphans when an *insurance category* itself is deleted (`insurance_id` ref) or a *service_item* is deleted (`service_item_id` ref) — different deletion paths, lower freq. | | ✅H6 | N+1: clinic doctors list lazy-loads `specialties` per doctor | src/Doctor/Repository/DoctorRepository.php (findByClinicWithFilters) | perf-nplus1 | **DONE** — `addSelect('s')` + `Paginator(fetchJoinCollection:true)`. Added `ApiTestCase::countQueries()` helper. `tests/Doctor/ClinicDoctorListNPlusOneTest` (query count constant vs doctor count; verified 4→10 without fix). | | H7 | N+1: comments list lazy-loads `likes`→`user`, `replies` (recursive), author realName | src/Rating/Controller/RatingController.php:278,352 | perf-nplus1 | Profiler GET comments → query count scales w/ comments | -| H8 | N+1: insurance service-coverage `serviceItemRepo->find()` per row in array_map | src/Insurance/Controller/InsuranceController.php:410 | perf-nplus1 | GET service-coverage → 1 find()/row | +| ✅H8 | N+1: insurance service-coverage `serviceItemRepo->find()` per row in array_map | src/Insurance/Controller/InsuranceController.php:406-420 | perf-nplus1 | **DONE** — batch `findBy(['id' => $ids])` + uuid map. Verified by functional correctness test (`tests/Insurance/ServiceCoverageNPlusOneTest`); query-count assertion was unreliable for this endpoint (identity-map), so correctness-tested instead. | | H9 | Unbounded list: `listClaims` loads ALL tenant claims, no LIMIT/pagination | src/Billing/Controller/BillingController.php:173 · ClaimRepository.php:64 | perf-pagination | GET claims high volume → must paginate | | H10 | Unbounded list: `wallet/transactions` loads ALL user transactions, no LIMIT | src/Settlement/Controller/SettlementController.php:89-94 | perf-pagination | GET wallet transactions → must paginate | diff --git a/src/Insurance/Controller/InsuranceController.php b/src/Insurance/Controller/InsuranceController.php index 9e4e488f..20099230 100644 --- a/src/Insurance/Controller/InsuranceController.php +++ b/src/Insurance/Controller/InsuranceController.php @@ -405,10 +405,19 @@ class InsuranceController extends BaseController $rows = $this->serviceCoverageRepo->findByContract($contract->getId()); - $data = array_map(function ($r) { + // 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(); + } + } + + $data = array_map(function ($r) use ($uuidById) { $row = $r->toArray(); - $item = $this->serviceItemRepo->find($r->getServiceItemId()); - $row['service_item_uuid'] = $item?->getUuid(); + $row['service_item_uuid'] = $uuidById[$r->getServiceItemId()] ?? null; return $row; }, $rows); diff --git a/tests/Insurance/ServiceCoverageNPlusOneTest.php b/tests/Insurance/ServiceCoverageNPlusOneTest.php new file mode 100644 index 00000000..58a97d85 --- /dev/null +++ b/tests/Insurance/ServiceCoverageNPlusOneTest.php @@ -0,0 +1,57 @@ +createUser(['ROLE_DOCTOR']); + $doctor = new Doctor($owner, 'دکتر تست'); + $this->em->persist($doctor); + $this->em->flush(); + + $tenant = new TenantInsurance(TenantInsurance::TYPE_DOCTOR, $doctor->getId(), 1); + $section = new ServiceSection(TenantInsurance::TYPE_DOCTOR, $doctor->getId(), 'بخش'); + $this->em->persist($tenant); + $this->em->persist($section); + $this->em->flush(); + + $expectedUuids = []; + for ($i = 0; $i < 4; $i++) { + $item = new ServiceItem($section, "خدمت $i"); + $this->em->persist($item); + $this->em->flush(); + $expectedUuids[] = $item->getUuid(); + $this->em->persist(new TenantServiceCoverage($tenant->getId(), $item->getId())); + } + $this->em->flush(); + + $body = $this->authJson( + 'GET', + '/api/v1/billing/tenant-insurances/' . $tenant->getUuid() . '/service-coverage', + $owner, + ); + + $this->assertSame(200, $this->responseCode()); + $rows = $body['data']['data'] ?? $body['data']; + $this->assertCount(4, $rows); + + $returnedUuids = array_column($rows, 'service_item_uuid'); + sort($expectedUuids); + sort($returnedUuids); + $this->assertSame($expectedUuids, $returnedUuids); + } +}