perf(insurance): batch-fetch service items in coverage list (H8)

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 <noreply@anthropic.com>
2026-06-28 19:29:06 +03:30
parent 6b12f3ddb9
commit eb1997066e
3 changed files with 70 additions and 4 deletions
@@ -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 |

@@ -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);

@@ -0,0 +1,57 @@
+<?php
+
+namespace App\Tests\Insurance;
+
+use App\ClinicService\Entity\ServiceItem;
+use App\ClinicService\Entity\ServiceSection;
+use App\Doctor\Entity\Doctor;
+use App\Insurance\Entity\TenantInsurance;
+use App\Insurance\Entity\TenantServiceCoverage;
+use App\Tests\ApiTestCase;
+
+/**
+ * The service-coverage list batch-fetches the referenced service items in one
+ * query instead of find()-per-row. This guards the refactor's correctness: every
+ * row must still carry the right service_item_uuid.
+ */
+class ServiceCoverageNPlusOneTest extends ApiTestCase
+{
+    public function testEveryRowResolvesItsServiceItemUuid(): void
+    {
+        $owner  = $this->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);
+    }
+}