test: enable Doctrine profiling in the test env and fix the N+1 it exposed
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) <noreply@anthropic.com>
This commit is contained in:
@@ -26,6 +26,10 @@ when@test:
|
|||||||
doctrine:
|
doctrine:
|
||||||
dbal:
|
dbal:
|
||||||
dbname_suffix: '_test%env(default::TEST_TOKEN)%'
|
dbname_suffix: '_test%env(default::TEST_TOKEN)%'
|
||||||
|
# APP_DEBUG=0 در .env پروژه است، پس profiling که پیشفرضش %kernel.debug%
|
||||||
|
# است خاموش میماند و سرویس doctrine.debug_data_holder ساخته نمیشود.
|
||||||
|
# تستهای N+1 برای شمارش کوئری به آن نیاز دارند.
|
||||||
|
profiling: true
|
||||||
|
|
||||||
when@prod:
|
when@prod:
|
||||||
doctrine:
|
doctrine:
|
||||||
|
|||||||
@@ -109,6 +109,32 @@ class ServiceItemRepository extends ServiceEntityRepository
|
|||||||
->getResult();
|
->getResult();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* نگاشت id → uuid برای مجموعهای از خدمات.
|
||||||
|
*
|
||||||
|
* عمداً entity هیدریت نمیکند: ServiceItem رابطهٔ staffMembers را EAGER دارد،
|
||||||
|
* پس هر entity یک کوئری اضافه برای بارگذاری کارکنانش میزند و فهرستی که فقط
|
||||||
|
* uuid میخواهد به N+1 میافتد.
|
||||||
|
*
|
||||||
|
* @param int[] $ids
|
||||||
|
* @return array<int, string>
|
||||||
|
*/
|
||||||
|
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
|
public function save(ServiceItem $item): void
|
||||||
{
|
{
|
||||||
$this->getEntityManager()->persist($item);
|
$this->getEntityManager()->persist($item);
|
||||||
|
|||||||
@@ -493,15 +493,10 @@ class InsuranceController extends BaseController
|
|||||||
|
|
||||||
$rows = $this->serviceCoverageRepo->findByContract($contract->getId());
|
$rows = $this->serviceCoverageRepo->findByContract($contract->getId());
|
||||||
|
|
||||||
// Batch-fetch the referenced service items once instead of one find()
|
// یک کوئری اسکالر برای همهٔ uuidها. هیدریتکردن entity کافی نیست: رابطهٔ
|
||||||
// per coverage row (N+1).
|
// EAGER staffMembers روی ServiceItem به ازای هر ردیف یک کوئری اضافه میزند.
|
||||||
$itemIds = array_values(array_unique(array_map(fn($r) => $r->getServiceItemId(), $rows)));
|
$itemIds = array_values(array_unique(array_map(fn($r) => $r->getServiceItemId(), $rows)));
|
||||||
$uuidById = [];
|
$uuidById = $this->serviceItemRepo->findUuidsByIds($itemIds);
|
||||||
if ($itemIds !== []) {
|
|
||||||
foreach ($this->serviceItemRepo->findBy(['id' => $itemIds]) as $item) {
|
|
||||||
$uuidById[$item->getId()] = $item->getUuid();
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
$data = array_map(function ($r) use ($uuidById) {
|
$data = array_map(function ($r) use ($uuidById) {
|
||||||
$row = $r->toArray();
|
$row = $r->toArray();
|
||||||
|
|||||||
@@ -12,11 +12,9 @@ use App\Tests\ApiTestCase;
|
|||||||
* `next_available_at` scans ahead for the first free slot per location.
|
* `next_available_at` scans ahead for the first free slot per location.
|
||||||
*
|
*
|
||||||
* The scan prefetches the schedule, holidays, overrides and taken appointments
|
* 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
|
* once per location and resolves the rest in memory, so its cost must not grow
|
||||||
* with how far ahead the first opening is. A query-count assertion would be the
|
* with the number of days walked or slots inspected — only with the number of
|
||||||
* natural guard, but countQueries() needs `doctrine.debug_data_holder`, which
|
* locations, by a small constant.
|
||||||
* this test environment does not expose — the existing N+1 tests fail on that
|
|
||||||
* same missing service. This covers the observable contract instead.
|
|
||||||
*/
|
*/
|
||||||
class BookingLocationsScanTest extends ApiTestCase
|
class BookingLocationsScanTest extends ApiTestCase
|
||||||
{
|
{
|
||||||
@@ -73,6 +71,37 @@ class BookingLocationsScanTest extends ApiTestCase
|
|||||||
return $doctor;
|
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
|
public function testNextAvailableIsReportedPerLocation(): void
|
||||||
{
|
{
|
||||||
$doctor = $this->makeDoctorWithSchedules(1);
|
$doctor = $this->makeDoctorWithSchedules(1);
|
||||||
|
|||||||
Reference in New Issue
Block a user