Scenario collection endpoint leaks embedded buckets (N+1 + contract violation) #68

New issue

Closed

opened 2026-07-07 10:12:00 +02:00 by myrmidex · 0 comments

myrmidex commented

2026-07-07 10:12:00 +02:00

Owner

Bug

GET /api/scenarios (the collection) embeds each scenario's buckets array, when only the item representation (GET /api/scenarios/{id}, #64) should. This is both a contract violation (the lean list shape documented on Scenario::$buckets) and an N+1: every scenario lazy-loads its bucket collection on every list request.

Root cause

GetCollection() on Scenario (src/Entity/Scenario.php) has no normalizationContext. In API Platform, an operation without a normalization group context serializes all readable properties, ignoring the #[Groups] annotations entirely (groups only filter when the context names which groups to use). So buckets — grouped scenario:read, intended item-only — leaks into the collection.

How it went unnoticed

The guard test App\Tests\Functional\ScenarioApiTest::testGetCollectionDoesNotEmbedBuckets (added in #64, 6fd2243) has been failing since it was written — confirmed by running it against the #64 version of the entity: it fails identically. The full backend suite evidently wasn't run clean since #64 (the container-dev tooling was broken; tests ran ad-hoc). Surfaced during #54's finish-phase full-suite run: 260 tests, this 1 failure.

Fix

Give GetCollection an explicit lean normalizationContext (a scenario:summary group covering name + description only), so buckets stays confined to the item op's scenario:read. The item op's context must include the summary group too so name/description still serialize there.

Collection: normalizationContext: ['groups' => ['scenario:summary']]
Item: ['groups' => ['scenario:summary', 'scenario:read', 'bucket:read']]
name/description: add scenario:summary (keep readable on both); buckets: stays scenario:read only.

Testing

testGetCollectionDoesNotEmbedBuckets goes green (the guard).
testGetItemEmbedsItsBuckets + the item field tests stay green (item still embeds).
Full backend suite back to 260/260.

Notes

No migration (serialization-only change). May need cache:clear for API Platform metadata.
Pure backend; unrelated to #54 (frontend add-bucket), during whose finish phase it was found.

## Bug `GET /api/scenarios` (the collection) embeds each scenario's `buckets` array, when only the item representation (`GET /api/scenarios/{id}`, #64) should. This is both a contract violation (the lean list shape documented on `Scenario::$buckets`) and an N+1: every scenario lazy-loads its bucket collection on every list request. ## Root cause `GetCollection()` on `Scenario` (`src/Entity/Scenario.php`) has **no `normalizationContext`**. In API Platform, an operation without a normalization group context serializes **all** readable properties, ignoring the `#[Groups]` annotations entirely (groups only filter when the context names which groups to use). So `buckets` — grouped `scenario:read`, intended item-only — leaks into the collection. ## How it went unnoticed The guard test `App\Tests\Functional\ScenarioApiTest::testGetCollectionDoesNotEmbedBuckets` (added in #64, `6fd2243`) **has been failing since it was written** — confirmed by running it against the #64 version of the entity: it fails identically. The full backend suite evidently wasn't run clean since #64 (the container-dev tooling was broken; tests ran ad-hoc). Surfaced during #54's finish-phase full-suite run: 260 tests, this 1 failure. ## Fix Give `GetCollection` an explicit lean `normalizationContext` (a `scenario:summary` group covering `name` + `description` only), so `buckets` stays confined to the item op's `scenario:read`. The item op's context must include the summary group too so `name`/`description` still serialize there. - Collection: `normalizationContext: ['groups' => ['scenario:summary']]` - Item: `['groups' => ['scenario:summary', 'scenario:read', 'bucket:read']]` - `name`/`description`: add `scenario:summary` (keep readable on both); `buckets`: stays `scenario:read` only. ## Testing - `testGetCollectionDoesNotEmbedBuckets` goes green (the guard). - `testGetItemEmbedsItsBuckets` + the item field tests stay green (item still embeds). - Full backend suite back to 260/260. ## Notes - No migration (serialization-only change). May need `cache:clear` for API Platform metadata. - Pure backend; unrelated to #54 (frontend add-bucket), during whose finish phase it was found.