perf(secretary,billing): kill N+1 in secretary + claims lists (M8, M9)

M8: DoctorSecretary::toArray() lazy-loaded secretary/doctor/clinic per row;
fetch-join them in findByDoctorScope/findByClinic (shared listWithRelations()).

M9: enrichClaims() lazy-loaded each claim's items collection and called
insuranceRepo->find() per claim. Fetch-join items in findByTenant (Paginator,
fetchJoinCollection) and batch-fetch insurance names once.

Regressions (query count constant vs row count): SecretaryListNPlusOneTest,
ClaimsListNPlusOneTest.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
hamed
2026-06-28 20:43:35 +03:30
co-authored by Claude Opus 4.8
parent 61ac775175
commit ccb71e4371
6 changed files with 141 additions and 14 deletions
+2 -2
View File
@@ -61,8 +61,8 @@ _None outstanding._
| ✅M5 | IDOR read: AppointmentSettings list endpoints leak any doctor's config — `listOverrides`, `listHolidays`, `availableLocations` (no ownership on {doctorUuid}) | src/Appointment/Controller/AppointmentSettingsController.php:150,256,349 | security-idor | **DONE** — ownership added to listOverrides/listHolidays/availableLocations. `tests/Appointment/AppointmentSettingsListOwnershipTest` |
| ✅M6 | OTP send-code has no per-mobile/per-uuid cap, only per-IP (5/hr) → SMS flood from rotating IPs | src/Auth/Controller/AuthController.php:136-153 · OtpService.php:59-77 · rate_limiter.yaml:4-7 | security-ratelimit | **DONE** — per-mobile limiter (5/hr) added alongside per-IP. `tests/Auth/SendCodeMobileRateLimitTest` (6 reqs / 6 IPs → 6th 429). |
| ✅M7 | Refresh token not rotated on use (same raw token 30d), never re-checks user status | src/Auth/Controller/AuthController.php:477-480 · TokenService.php:33-45 | security-auth | **DONE** — single-use rotation (revoke old + issue new) + suspended-user (status!=1) rejected. `tests/Auth/RefreshTokenRotationTest`. |
| M8 | N+1: secretary list lazy-loads secretary/doctor ManyToOne per row | src/Secretary/Controller/SecretaryController.php:192,213 | perf-nplus1 | Profiler secretary list → ~2 queries/row |
| M9 | N+1: billing claims lazy `items` + `insuranceRepo->find()` per claim in enrichClaims | src/Billing/Controller/BillingController.php:~49,68 | perf-nplus1 | GET claims → items+insurance query/claim |
| M8 | N+1: secretary list lazy-loads secretary/doctor ManyToOne per row | src/Secretary/Controller/SecretaryController.php:192,213 | perf-nplus1 | **DONE** — fetch-join secretary/doctor/clinic. `tests/Secretary/SecretaryListNPlusOneTest` |
| M9 | N+1: billing claims lazy `items` + `insuranceRepo->find()` per claim in enrichClaims | src/Billing/Controller/BillingController.php:~49,68 | perf-nplus1 | **DONE** — fetch-join items (Paginator) + batch insurance names. `tests/Billing/ClaimsListNPlusOneTest` |
| ✅M10 | Unbounded list: `listMine` settlements `findByUser` no limit | src/Settlement/Controller/SettlementController.php:193 | perf-pagination | **DONE** — page/limit + countByUser + data.meta. `tests/Settlement/SettlementListPaginationTest` |
| ✅M11 | Unbounded list: admin `pendingComments` `findPending` no limit | src/Rating/Controller/RatingController.php:350 | perf-pagination | **DONE** — findPending(limit,offset)+countPending+meta. `tests/Rating/CommentPaginationTest::testAdminPendingListPaginates` |
| ✅M12 | Unbounded list: public `listComments` per-doctor no limit | src/Rating/Controller/RatingController.php:270 | perf-pagination | **DONE** — Paginator(fetchJoinCollection) page/limit + countApprovedRootsByDoctor + meta. `tests/Rating/CommentPaginationTest::testPublicListPaginates` |
+11 -2
View File
@@ -63,9 +63,18 @@ class BillingController extends BaseController
$sessionDates = $this->sessionRepo->datesForIds($uniqueSessionIds);
$patientInfo = $this->sessionRepo->patientInfoForIds($uniqueSessionIds);
return array_map(function (Claim $claim) use ($itemDetails, $sessionDates, $patientInfo) {
// Batch-fetch insurance names instead of one find() per claim (N+1).
$insuranceIds = array_values(array_unique(array_map(fn(Claim $c) => $c->getInsuranceId(), $claims)));
$insuranceNames = [];
if ($insuranceIds !== []) {
foreach ($this->insuranceRepo->findBy(['id' => $insuranceIds]) as $ins) {
$insuranceNames[$ins->getId()] = $ins->getName();
}
}
return array_map(function (Claim $claim) use ($itemDetails, $sessionDates, $patientInfo, $insuranceNames) {
$data = $claim->toArray();
$data['insurance_name'] = $this->insuranceRepo->find($claim->getInsuranceId())?->getName();
$data['insurance_name'] = $insuranceNames[$claim->getInsuranceId()] ?? null;
$patientName = null;
$patientMobile = null;
+10 -4
View File
@@ -4,6 +4,7 @@ namespace App\Billing\Repository;
use App\Billing\Entity\Claim;
use Doctrine\Bundle\DoctrineBundle\Repository\ServiceEntityRepository;
use Doctrine\ORM\Tools\Pagination\Paginator;
use Doctrine\Persistence\ManagerRegistry;
class ClaimRepository extends ServiceEntityRepository
@@ -24,12 +25,17 @@ class ClaimRepository extends ServiceEntityRepository
*/
public function findByTenant(string $entityType, int $entityId, array $filters = [], int $page = 1, int $limit = 50): array
{
return $this->tenantQb($entityType, $entityId, $filters)
// fetch-join items so enrichClaims()/toArray() don't lazy-load the
// items collection per claim (N+1). fetchJoinCollection keeps the LIMIT
// paginating by claim.
$qb = $this->tenantQb($entityType, $entityId, $filters)
->addSelect('i')
->leftJoin('c.items', 'i')
->orderBy('c.id', 'DESC')
->setFirstResult(($page - 1) * $limit)
->setMaxResults($limit)
->getQuery()
->getResult();
->setMaxResults($limit);
return iterator_to_array(new Paginator($qb, fetchJoinCollection: true));
}
public function countByTenant(string $entityType, int $entityId, array $filters = []): int
@@ -38,13 +38,27 @@ class DoctorSecretaryRepository extends ServiceEntityRepository
return $this->findBy(['doctor' => $doctor], ['createdAt' => 'DESC']);
}
/** Base query with secretary/doctor/clinic fetch-joined for toArray() (no N+1). */
private function listWithRelations(): \Doctrine\ORM\QueryBuilder
{
return $this->createQueryBuilder('s')
->addSelect('sec', 'doc', 'cl')
->leftJoin('s.secretary', 'sec')
->leftJoin('s.doctor', 'doc')
->leftJoin('s.clinic', 'cl')
->orderBy('s.createdAt', 'DESC');
}
/** منشی ها مطب شخصی یک دکتر (owner_type='doctor') */
public function findByDoctorScope(Doctor $doctor): array
{
return $this->findBy(
['doctor' => $doctor, 'ownerType' => DoctorSecretary::OWNER_DOCTOR],
['createdAt' => 'DESC']
);
return $this->listWithRelations()
->where('s.doctor = :doctor')
->andWhere('s.ownerType = :type')
->setParameter('doctor', $doctor)
->setParameter('type', DoctorSecretary::OWNER_DOCTOR)
->getQuery()
->getResult();
}
/** رابطه منشی در scope مطب شخصی */
@@ -111,12 +125,11 @@ class DoctorSecretaryRepository extends ServiceEntityRepository
/** همه منشی ها کلینیک (owner_type='clinic') */
public function findByClinic(Clinic $clinic): array
{
return $this->createQueryBuilder('s')
return $this->listWithRelations()
->where('s.clinic = :clinic')
->andWhere('s.ownerType = :type')
->setParameter('clinic', $clinic)
->setParameter('type', DoctorSecretary::OWNER_CLINIC)
->orderBy('s.createdAt', 'DESC')
->getQuery()
->getResult();
}
+54
View File
@@ -0,0 +1,54 @@
<?php
namespace App\Tests\Billing;
use App\Billing\Entity\Claim;
use App\Billing\Entity\ClaimItem;
use App\Doctor\Entity\Doctor;
use App\Insurance\Entity\Insurance;
use App\Insurance\Enum\InsuranceType;
use App\Tests\ApiTestCase;
/**
* GET /billing/claims must batch items + insurance names, not lazy-load per
* claim. Query count stays constant with claim count.
*/
class ClaimsListNPlusOneTest extends ApiTestCase
{
/** @return \App\Auth\Entity\User */
private function makeTenantWithClaims(int $count)
{
$owner = $this->createUser(['ROLE_DOCTOR']);
$doctor = new Doctor($owner, 'دکتر');
$this->em->persist($doctor);
$insurance = new Insurance('بیمه', InsuranceType::Basic);
$this->em->persist($insurance);
$this->em->flush();
for ($i = 0; $i < $count; $i++) {
$claim = new Claim('doctor', $doctor->getId(), $insurance->getId(), 'base');
$item = new ClaimItem($claim, 1, 100_000);
$claim->addItem($item);
$claim->submit();
$this->em->persist($claim);
$this->em->persist($item);
}
$this->em->flush();
return $owner;
}
public function testQueryCountDoesNotGrowWithClaimCount(): void
{
$this->client->disableReboot();
$ownerS = $this->makeTenantWithClaims(1);
$ownerL = $this->makeTenantWithClaims(5);
$this->em->clear();
$qSmall = $this->countQueries(fn () => $this->authJson('GET', '/api/v1/billing/claims', $ownerS));
$qLarge = $this->countQueries(fn () => $this->authJson('GET', '/api/v1/billing/claims', $ownerL));
$this->assertLessThanOrEqual($qSmall + 1, $qLarge, "N+1: query count grew from $qSmall to $qLarge");
}
}
@@ -0,0 +1,45 @@
<?php
namespace App\Tests\Secretary;
use App\Doctor\Entity\Doctor;
use App\Secretary\Entity\DoctorSecretary;
use App\Tests\ApiTestCase;
/**
* The per-doctor secretary list must fetch-join secretary/doctor/clinic, not
* lazy-load them per row. Query count stays constant with secretary count.
*/
class SecretaryListNPlusOneTest extends ApiTestCase
{
/** @return array{0: \App\Auth\Entity\User, 1: Doctor} */
private function makeDoctorWithSecretaries(int $count): array
{
$owner = $this->createUser(['ROLE_DOCTOR']);
$doctor = new Doctor($owner, 'دکتر');
$this->em->persist($doctor);
$this->em->flush();
for ($i = 0; $i < $count; $i++) {
$secUser = $this->createUser(['ROLE_SECRETARY']);
$this->em->persist(new DoctorSecretary($doctor, $secUser, DoctorSecretary::OWNER_DOCTOR));
}
$this->em->flush();
return [$owner, $doctor];
}
public function testQueryCountDoesNotGrowWithSecretaryCount(): void
{
$this->client->disableReboot();
[$ownerS, $small] = $this->makeDoctorWithSecretaries(1);
[$ownerL, $large] = $this->makeDoctorWithSecretaries(5);
$this->em->clear();
$qSmall = $this->countQueries(fn () => $this->authJson('GET', '/api/v1/secretaries/' . $small->getUuid(), $ownerS));
$qLarge = $this->countQueries(fn () => $this->authJson('GET', '/api/v1/secretaries/' . $large->getUuid(), $ownerL));
$this->assertLessThanOrEqual($qSmall + 1, $qLarge, "N+1: query count grew from $qSmall to $qLarge");
}
}