perf(events): eliminate /events N+1 queries — 298 → 47 queries, ~1500ms → ~340ms#549
Open
smarcet wants to merge 27 commits into
Open
perf(events): eliminate /events N+1 queries — 298 → 47 queries, ~1500ms → ~340ms#549smarcet wants to merge 27 commits into
smarcet wants to merge 27 commits into
Conversation
…for automatic invalidation Without this, a presentation update left stale cached data for up to 20 minutes (CacheTTL=1200s). Including the last_edited Unix timestamp in the key means any update to the presentation changes the key, so the old entry is silently ignored and ages out via TTL — no explicit Cache::forget needed anywhere.
Adds the existing ServerTimingDoctrine middleware to the /events endpoint so the response carries a Server-Timing HTTP header. Chrome DevTools renders it natively in the Network tab → Timing → Server Timing section. This is profiling-only — no behaviour change. Lets us see the breakdown of boot / db / app / total time per request on main and on any branch, so we can identify which phase is actually slow before writing any optimization.
…g header Adds these phases to the Server-Timing response header on /events: boot — LARAVEL_START to server.timing.doctrine middleware start pre — middleware start to controller method entry (auth.user etc.) controller — controller method body total db — Doctrine SQL time (already existed) serializer — $response->toArray() call only post — controller return to middleware end (response wrapping) app — total - db (existing) total — middleware start to middleware end Implementation: - Reorder route middleware so server.timing.doctrine runs BEFORE auth.user; this lets it measure auth.user as part of 'pre' time. - Controller writes 4 microtime(true) markers to Session at controller_start, serializer_start, serializer_end, controller_end. - Middleware reads markers, computes deltas, emits in header, then clears them so they don't leak to recycled workers. Profiling-only — no behavior change. Chrome DevTools Network tab → Timing → Server Timing renders each metric natively.
The previous Server-Timing 'db' metric reported ~0.4ms because:
- DBAL 2.x SQLLogger / DebugStack is deprecated and not invoked by DBAL 3.x
prepared statements (the normal query path).
- DBAL 3.x Logging\Middleware logs queries but does not include duration in
the PSR-3 context, so the previous collector saw no duration to sum.
Replace with a proper DBAL Driver Middleware that times each query at the
statement-execution level:
- QueryTimingCollector — static request-scoped accumulator (totalMs + count).
- QueryTimingMiddleware (and inner Driver/Connection/Statement wrappers) —
times every query(), exec(), and prepared-Statement::execute(). Per-query
overhead is two microtime(true) calls.
Registered globally in config/doctrine.php for all three connections. The
request lifecycle middleware (ServerTimingDoctrine) now just resets the
collector at the start of each request and reads totalMs / count at the end.
Also adds the query count to the Server-Timing header as a desc on 'db'.
…1 sources
298 queries per /events request confirms a real N+1 problem in the serializer,
but we don't know WHICH 30-per-event lazy loads are firing. Without that,
any batch-load fix is guesswork.
Enhance QueryTimingCollector to also bucket queries by normalized SQL pattern
(numeric literals + quoted strings + params collapsed to '?'). At the end of
any request that ran >= 20 queries, log the top 8 patterns (count + total ms +
sample SQL) at WARNING level so they show up in laravel.log.
Example expected output:
N+1 candidate {"count":30,"totalMs":42.1,"sample":"SELECT t0.id ... FROM PresentationSpeaker t0 WHERE t0.id = ?"}
N+1 candidate {"count":30,"totalMs":35.4,"sample":"SELECT t0.id ... FROM Member t0 WHERE t0.id = ?"}
This identifies exactly which lazy associations need to be batch-loaded.
…f 84 queries
Profiling /events on dev showed 'SELECT COUNT(MemberID)' firing 84 times per
request — the largest single N+1 source. Traced to:
PresentationSerializer::getMediaUploadsSerializerType()
-> $currentUser->isAdmin() (calls belongsToGroup)
-> $presentation->memberCanEdit($currentUser) (calls belongsToGroup multiple times)
These check the same ~8 group codes against the SAME current-user Member
instance for every presentation on the page. Each call ran a raw SELECT
COUNT(MemberID) against Group_Members.
Memoize results on the Member instance ($groupMembershipCache, unannotated
so Doctrine ignores it). The cache is per-instance and per-request — naturally
discarded when Doctrine re-hydrates the entity on the next request.
Expected impact: 84 queries -> ~8 queries on /events (one per unique group
code), saving ~85ms of DB time per request. No behaviour change.
…uery Profiling /events showed two remaining N+1 patterns directly tied to speakers: - Member SELECT by id: 56 queries / 190ms per request - Presentation_Speakers composite-key lookup: 19 queries / 26ms per request Both fire from the serializer chain (PresentationSpeaker::getFirstName() / getLastName() falling back to $this->member, and $speaker->getPresentationAssignmentOrder() looking up the assignment per (presentation, speaker)). Add a single targeted DQL right before getAllByPage returns the PagingResponse: SELECT s, m FROM PresentationSpeakerAssignment a JOIN a.speaker s LEFT JOIN s.member m WHERE a.presentation IN (:ids) That warms Doctrine's identity map with every speaker + member that the serializer is about to demand, so each subsequent getSpeaker() / getMember() call hits the identity map (zero DB). Expected impact: 75 queries collapsed into 1, ~216ms of DB time saved per /events request that contains presentations. No behaviour change — the query is read-only and side-effects only the UnitOfWork.
DQL requires that the FROM clause's root alias appears in SELECT. Previous query 'SELECT s, m FROM PresentationSpeakerAssignment a JOIN a.speaker s ...' failed with a semantical error and the preload silently no-op'd (the catch block logged the warning but the calling code continued). Change to 'SELECT a, s, m' so Doctrine hydrates the assignments, speakers, and members all into the UnitOfWork. As a bonus this also pre-populates the PresentationSpeakerAssignment entities by ID, so the per-speaker getPresentationAssignmentOrder() lookup (Presentation_Speakers composite-key pattern, 19 queries) also hits identity map.
After fixing the DQL syntax, query count only dropped 220->208 instead of the predicted ~145. Either the preload returns 0 assignments, or it loads assignments but their speaker/member fields remain proxies (so the serializer still triggers per-call lazy initialization). Add a one-shot diagnostic log right after the preload that counts: - how many assignments came back - how many unique speaker entities are reachable - how many of those speakers are initialized (not proxies) - same for members Will revert this once we know which case we're in.
…re being loaded Preload diagnostic confirmed speakers+members ARE being loaded into identity map (12 unique members, all initialized). Yet 56 Member SELECTs still fire. The 56 must therefore come from OTHER code paths (created_by, updated_by, ...). Track FROM-Member SQL queries with their bound params (up to 100 per request). Log them when the total db query count exceeds 20. The params will let us correlate Member IDs across queries and identify whether they're for: - the current_user (group/permission checks) - per-event created_by / updated_by - per-presentation creator / moderator - speakers (should be 0 if preload is working)
Doctrine wraps table identifiers in backticks (FROM `Member` t0) so the plain 'FROM Member' check missed every query. Strip backticks first.
… SELECTs per request Profiling /events captured 100 Member SELECT params; 98 of them were the SAME Member ID (the current authenticated user). Root cause: ResourceServerContext:: getCurrentUser() runs $member_repository->getByExternalId() on every single call, and serializers call it many times per request (PresentationSerializer:: getMediaUploadsSerializerType(), getSerializerType(), permission checks). Cache the resolved Member instance on the ResourceServerContext (request-scoped service). The authenticated user does not change mid-request, so the same Member is the right answer every time. The side-effects (group sync, event dispatch, field updates) only fire on the first call — which is idempotent behaviour per request anyway. Expected impact: ~98 Member queries eliminated. DB time saved depends on the network RTT to the DB but typically ~100-200ms on /events.
… 20 queries
Presentation::getSelectionStatus() ran a JOIN DQL per presentation
('SELECT sp from SummitSelectedPresentation JOIN sp.list JOIN sp.presentation
WHERE p.id = :id'). Profiling /events showed it firing 20 times per request
(once per presentation, ~twice on average).
Two changes:
1. Presentation gains $preloadedSessionSelections (transient, Doctrine-ignored).
When set by a caller via setPreloadedSessionSelections(), getSelectionStatus()
uses these rows instead of firing the DQL. Result is also memoized in
$memoizedSelectionStatus so repeated calls within a request are free.
2. DoctrineSummitEventRepository::getAllByPage adds one batch query for ALL
presentations on the page, filtered with the same constants getSelectionStatus
uses (collection=Selected, list_type=Group, list_class=Session). Results are
grouped by presentation id and pushed onto each Presentation instance.
Expected: 20 queries -> 1, plus memoization saves repeat calls within the
serializer.
…hy it has no effect 20 SummitSelectedPresentation queries still fire per request despite the preload running with no warning. Add two diagnostic logs to find where the chain breaks: 1. Repository: log how many Presentation entities received the cache via setPreloadedSessionSelections, plus the concrete class names of the first few events (in case Doctrine returns a different class than the imported models\summit\Presentation). 2. Presentation::getSelectionStatus: log a cache MISS for any presentation that falls through to the DQL fallback. If the repo logs 'fed=10' but getSelectionStatus logs 10 cache MISS, the property is being lost between setter and getter (likely a Doctrine proxy hydration issue or a different Presentation instance).
…batch-loaded assignments PresentationSpeaker::getPresentationAssignmentOrder() ran a per-call DB query via the EXTRA_LAZY $this->presentations->matching() pattern — 19 queries per /events request, one per (speaker, presentation) pair the serializer touches. Two changes: 1. PresentationSpeaker gains $preloadedAssignmentOrders (transient, unannotated so Doctrine ignores it) and setPreloadedAssignmentOrder(pid, order). getPresentationAssignmentOrder checks this cache first and only falls through to the original matching() query when the cache is unset. 2. DoctrineSummitEventRepository::getAllByPage iterates the assignments it already loaded for the speaker+member preload and pushes each (pid, order) pair into the corresponding speaker. Zero extra queries. Expected: 19 queries collapsed to 0, total query count 98 -> ~79.
… /events hydration Three remaining per-event N+1 patterns (10 queries each) collapsed in a single commit: 1. Location — was lazy-loaded per event in the serializer. Add LEFT JOIN e.location loc to the main hydration query with addSelect, so Doctrine eagerly hydrates the SummitAbstractLocation (and its JOINED subclasses like SummitVenueRoom) for every event in the same SQL round-trip. 2. Tags — was a per-event lazy load on the EXTRA_LAZY ManyToMany. Add a single 'SELECT e, t FROM SummitEvent e LEFT JOIN e.tags t WHERE e.id IN (:ids)' batch query after the main hydration, fetch-joining tags so each event's tag collection is populated. 3. PresentationMaterial — same pattern for the materials OneToMany on Presentation. Single 'SELECT p, m FROM Presentation p LEFT JOIN p.materials m WHERE p.id IN (:ids)' covers media_uploads/slides/links/ videos/etc. (they all live in the materials table). Expected: 30 queries removed, total drops from 79 to ~49 per /events request.
…gory
Two more N+1 patterns from /events:
1. Sponsors (ManyToMany Company on SummitEvent) — 10 queries per request, one
per event when the serializer iterates $event->getSponsors(). Same
fetch-join batch pattern we used for tags: SELECT e, s FROM SummitEvent e
LEFT JOIN e.sponsors s WHERE e.id IN (:ids).
2. PresentationCategory (track on Presentation) — 5 queries per request from
per-event lazy loads. Add LEFT JOIN p.category cat + addSelect('cat') to
the main hydration query so it's eagerly hydrated alongside the Presentation
subclass data.
Expected: 15 more queries removed, total drops from 61 to ~46.
… N+1 hunt The profiling work that drove the /events optimization is done. Remove the temporary diagnostic logging while keeping the actual fixes and the Server-Timing header (which remains useful for ongoing visibility). Removed: - N+1 candidate pattern logger in ServerTimingDoctrine ($dbCount>=20 trigger) - 'member queries' params dump in ServerTimingDoctrine - 'preload diagnostic' speaker+member count dump in repository - 'selection-status preload diagnostic' dump in repository - 'getSelectionStatus cache MISS' log in Presentation entity - $patterns / $memberQueries / topPatterns()/normalize() in QueryTimingCollector - bindValue() override and SQL param capture in QueryTimingMiddleware Kept: - Server-Timing HTTP header with boot/pre/controller/db/serializer/post/total - QueryTimingMiddleware DBAL Driver Middleware (accurate db ms + count) - QueryTimingCollector (now just totalMs + count) - Every actual fix from the optimization series.
Record context, profiling methodology, every fix (with symptom/root cause/ impact), what was kept, what was intentionally not fixed, and the order that produced the result. Future readers should be able to follow the same approach without rediscovering the dead ends.
📝 WalkthroughWalkthroughThis PR eliminates N+1 query issues in the GET /api/v1/summits/{id}/events endpoint by adding per-request DB timing instrumentation, refactoring Server-Timing emission and controller timing markers, introducing request-scoped entity memoization, and adding repository-level batch preloads; an ADR documents the fixes and measured outcomes. ChangesN+1 Query Elimination for Events Endpoint
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
📘 OpenAPI / Swagger preview ➡️ https://OpenStackweb.github.io/summit-api/openapi/pr-549/ This page is automatically updated on each push to this PR. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR is a profiling-driven performance pass on GET /api/v1/summits/{id}/events, aiming to eliminate major N+1 query patterns and add lightweight timing instrumentation to make request phases (boot/controller/db/serializer) measurable.
Changes:
- Added
Server-Timinginstrumentation and accurate per-request SQL timing/count via a DBAL Driver Middleware. - Introduced request-scoped memoization/caches for repeated auth/group checks (
getCurrentUser(),belongsToGroup(), selection status, assignment order). - Added repository-level batch preloads / fetch-joins to avoid per-entity lazy-load queries during serialization (speakers/members, selections, tags/sponsors/materials, location/category).
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| routes/api_v1.php | Adds server.timing.doctrine middleware to the /events route. |
| config/doctrine.php | Registers the new DBAL QueryTimingMiddleware for Doctrine connections. |
| app/Repositories/Summit/DoctrineSummitEventRepository.php | Adds fetch-joins and batch preload queries to collapse N+1s during event hydration/serialization. |
| app/ModelSerializers/Summit/Presentation/PresentationSerializer.php | Extends public presentation cache key to include last-edited timestamp. |
| app/Models/OAuth2/ResourceServerContext.php | Adds request-scoped current-user caching to reduce repeated Member lookups/transactions. |
| app/Models/Foundation/Summit/Speakers/PresentationSpeaker.php | Adds preloaded assignment-order cache to avoid EXTRA_LAZY matching() queries. |
| app/Models/Foundation/Summit/Events/Presentations/Presentation.php | Adds selection-status preload + memoization to avoid per-presentation DQL. |
| app/Models/Foundation/Main/Member.php | Adds per-instance group membership memoization for repeated permission checks. |
| app/Http/Middleware/ServerTimingDoctrine.php | Reworks Server-Timing header emission to use the new query timing collector and controller markers. |
| app/Http/Middleware/Doctrine/QueryTimingMiddleware.php | New DBAL Driver Middleware to time query/exec/statement execution. |
| app/Http/Middleware/Doctrine/QueryTimingCollector.php | New static request-scoped accumulator for SQL timing and query count. |
| app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitEventsApiController.php | Adds controller/serializer phase markers for Server-Timing. |
| adr/002-events-endpoint-n-plus-1-elimination.md | Adds ADR documenting the approach, measurements, and decisions. |
Comments suppressed due to low confidence (4)
app/Models/OAuth2/ResourceServerContext.php:186
- This early-return cache is checked before reading the current
user_external_idfrom the auth context. SinceIResourceServerContextis bound as a singleton, in long-lived runtimes a new request can update the authorization context but still hit this cached branch and return the prior request's user. The cache should be keyed/validated against the currentuser_external_id(and ideally the option flags) before returning.
public function getCurrentUser(bool $synch_groups = true, bool $update_member_fields = true): ?Member
{
if ($this->cachedCurrentUserResolved) {
return $this->cachedCurrentUser;
}
app/Models/OAuth2/ResourceServerContext.php:199
- When caching the
nullreturn for missinguser_external_id, make sure any new cache metadata you add (e.g., which external id/options the cache corresponds to) is set consistently here too; otherwise subsequent calls may incorrectly reuse the cached value for a different auth context/options combination.
if (is_null($user_external_id)) {
$this->cachedCurrentUserResolved = true;
return $this->cachedCurrentUser = null;
}
app/Models/OAuth2/ResourceServerContext.php:255
- This cached
nullreturn for a missing local Member should also set/validate any cache key metadata (currentuser_external_idand option flags). Otherwise, later calls with a different auth context/options may still short-circuit to this cached result.
if (is_null($member)) {
Log::warning(sprintf("ResourceServerContext::getCurrentUser user not found %s (%s).", $user_external_id, $user_email));
$this->cachedCurrentUserResolved = true;
return $this->cachedCurrentUser = null;
}
app/Models/OAuth2/ResourceServerContext.php:293
- The cached result is stored unconditionally here. To preserve
getCurrentUser($synch_groups, $update_member_fields)semantics and avoid cross-request leakage in long-lived runtimes, store enough metadata alongside the cached Member (e.g., currentuser_external_idand the option flags) and validate it in the early-return branch before reusing the cache.
$resolved = $this->tx_service->transaction(function () use
(
$member,
$user_email,
$user_first_name,
$user_last_name,
$user_external_id,
$user_email_verified,
$synch_groups,
$update_member_fields
) {
if($update_member_fields) {
// update member fields
if (!empty($user_email)) {
Log::debug(sprintf("ResourceServerContext::getCurrentUser setting email for member %s", $member->getId()));
$member->setEmail($user_email);
}
if (!empty($user_first_name)) {
Log::debug(sprintf("ResourceServerContext::getCurrentUser setting first name for member %s", $member->getId()));
$member->setFirstName($user_first_name);
}
if (!empty($user_last_name)) {
Log::debug(sprintf("ResourceServerContext::getCurrentUser setting last name for member %s", $member->getId()));
$member->setLastName($user_last_name);
}
}
$member->setUserExternalId($user_external_id);
$member->setEmailVerified($user_email_verified);
MemberAssocSummitOrders::dispatch($member->getId());
return $synch_groups ? $this->checkGroups($member) : $member;
});
$this->cachedCurrentUserResolved = true;
return $this->cachedCurrentUser = $resolved;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+803
to
+823
| $selections = $em->createQuery( | ||
| 'SELECT sp, p FROM ' . SummitSelectedPresentation::class . ' sp ' . | ||
| 'JOIN sp.list l ' . | ||
| 'JOIN sp.presentation p ' . | ||
| 'WHERE p.id IN (:ids) ' . | ||
| 'AND sp.collection = :collection ' . | ||
| 'AND l.list_type = :list_type ' . | ||
| 'AND l.list_class = :list_class' | ||
| ) | ||
| ->setParameter('ids', $presentationIds) | ||
| ->setParameter('collection', SummitSelectedPresentation::CollectionSelected) | ||
| ->setParameter('list_type', SummitSelectedPresentationList::Group) | ||
| ->setParameter('list_class', SummitSelectedPresentationList::Session) | ||
| ->getResult(); | ||
|
|
||
| // Group by presentation id and feed each Presentation entity. | ||
| $byPresentation = []; | ||
| foreach ($selections as $sp) { | ||
| $pid = $sp->getPresentation()->getId(); | ||
| $byPresentation[$pid][] = $sp; | ||
| } |
Comment on lines
162
to
+186
| @@ -167,6 +181,10 @@ public function updateAuthContextVar(string $varName, $value):void { | |||
| */ | |||
| public function getCurrentUser(bool $synch_groups = true, bool $update_member_fields = true): ?Member | |||
| { | |||
| if ($this->cachedCurrentUserResolved) { | |||
| return $this->cachedCurrentUser; | |||
| } | |||
Comment on lines
+1
to
+6
| # ADR-0001: Eliminate N+1 Queries in `/events` Endpoint | ||
|
|
||
| - **Status:** Accepted | ||
| - **Date:** 2026-05-25 | ||
| - **Endpoint:** `GET /api/v1/summits/{id}/events` | ||
| - **Branch:** `hotfix/cache-optimizations` |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/Models/Foundation/Summit/Speakers/PresentationSpeaker.php (1)
1662-1691:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winClear cached assignment orders after in-request reordering/removal.
After
preloadedAssignmentOrders[$presentationId]is set, this method never re-checks the live assignment. If the same request later reorders speakers or removes the assignment, serialization can still emit the old order.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/Models/Foundation/Summit/Speakers/PresentationSpeaker.php` around lines 1662 - 1691, The cached map preloadedAssignmentOrders can become stale after in-request reorders/removals; add explicit invalidation and use it. Add methods clearPreloadedAssignmentOrder(int $presentationId): void and clearAllPreloadedAssignmentOrders(): void on this class to unset entries in the preloadedAssignmentOrders array, and update any mutators that change assignments (e.g., methods that reorder or remove presentation assignments or caller sites that previously called setPreloadedAssignmentOrder) to call clearPreloadedAssignmentOrder(presentationId) (or clearAllPreloadedAssignmentOrders() when batch changes occur) so getPresentationAssignmentOrder() will re-query the live association when needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@adr/002-events-endpoint-n-plus-1-elimination.md`:
- Line 1: Update the ADR title to match the file identity by changing "ADR-0001"
to "ADR-002" so the document header and filename
(adr/002-events-endpoint-n-plus-1-elimination.md) and any internal references
align; edit the first line (the H1 title) to read "ADR-002: Eliminate N+1
Queries in `/events` Endpoint" and verify any in-file references to the ADR
number are also updated to ADR-002.
- Around line 43-47: The fenced code block showing the Server-Timing header is
missing a language tag causing markdownlint MD040; update the opening fence for
that block (the triple-backticks before the "Server-Timing: boot;dur=…" example)
to include the "http" language tag so the block becomes ```http and the linter
passes.
In
`@app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitEventsApiController.php`:
- Around line 173-193: The controller OAuth2SummitEventsApiController@getEvents
currently writes timing markers to Session keys; change those Session::put calls
(e.g., 'timing.controller_start', 'timing.serializer_start',
'timing.serializer_end', 'timing.controller_end') to store markers on the
current request object (use $request->attributes->set(...) or the equivalent
available in the controller) so markers are request-scoped, and pass the same
request through to any nested closures; update the ServerTimingDoctrine
middleware to read timing markers from $request->attributes->get(...) instead of
Session and remove any Session::forget(...) cleanup so timing is no longer tied
to session state.
In `@app/Models/Foundation/Summit/Events/Presentations/Presentation.php`:
- Around line 953-974: The memoized selection status ($memoizedSelectionStatus)
must be cleared whenever selection-related state mutators run; update
setSelectedPresentations(), setSelectionPlan(), clearSelectionPlan(), and
setCategory() to reset $this->memoizedSelectionStatus = null (in addition to any
existing state changes and the existing setPreloadedSessionSelections behavior)
so that subsequent getSelectionStatus() will recompute using the new selection
state; reference the private property names preloadedSessionSelections and
memoizedSelectionStatus and the methods setSelectedPresentations,
setSelectionPlan, clearSelectionPlan, and setCategory when applying the change.
In `@app/Models/OAuth2/ResourceServerContext.php`:
- Around line 162-174: The cachedCurrentUser boolean currently prevents later
calls to getCurrentUser(fromToken, $synch_groups, $update_member_fields) from
performing side-effects (checkGroups/field updates) if the first call passed
false for those flags; fix by changing caching to separate the resolved Member
instance from whether side-effects have been performed: keep cachedCurrentUser
(Member) but add a cachedCurrentUserSideEffectsPerformed flag (or track
permittedFlags bitmask) and in getCurrentUser ensure that even if
cachedCurrentUser is set, you still run checkGroups() / refreshMemberFields()
when a later call requests them and only mark side-effects performed once;
update all uses in getCurrentUser and any helper methods (references:
getCurrentUser, cachedCurrentUser, cachedCurrentUserResolved) so side-effects
are idempotent and not skipped due to cache population order.
In `@routes/api_v1.php`:
- Line 636: The middleware order for the events GET route is inverted compared
to the tickets GET route which leads to inconsistent profiling; update the
events route declaration that references
OAuth2SummitEventsApiController@getEvents to use the same middleware ordering as
the tickets route (i.e., ['auth.user', 'server.timing.doctrine']) or, if the
difference is intentional, add a comment explaining why the ordering differs so
timing measurements remain comparable and maintainers understand the choice.
---
Outside diff comments:
In `@app/Models/Foundation/Summit/Speakers/PresentationSpeaker.php`:
- Around line 1662-1691: The cached map preloadedAssignmentOrders can become
stale after in-request reorders/removals; add explicit invalidation and use it.
Add methods clearPreloadedAssignmentOrder(int $presentationId): void and
clearAllPreloadedAssignmentOrders(): void on this class to unset entries in the
preloadedAssignmentOrders array, and update any mutators that change assignments
(e.g., methods that reorder or remove presentation assignments or caller sites
that previously called setPreloadedAssignmentOrder) to call
clearPreloadedAssignmentOrder(presentationId) (or
clearAllPreloadedAssignmentOrders() when batch changes occur) so
getPresentationAssignmentOrder() will re-query the live association when needed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 41d6871b-5f86-4f1d-a8bc-ac23b46d252b
📒 Files selected for processing (13)
adr/002-events-endpoint-n-plus-1-elimination.mdapp/Http/Controllers/Apis/Protected/Summit/OAuth2SummitEventsApiController.phpapp/Http/Middleware/Doctrine/QueryTimingCollector.phpapp/Http/Middleware/Doctrine/QueryTimingMiddleware.phpapp/Http/Middleware/ServerTimingDoctrine.phpapp/ModelSerializers/Summit/Presentation/PresentationSerializer.phpapp/Models/Foundation/Main/Member.phpapp/Models/Foundation/Summit/Events/Presentations/Presentation.phpapp/Models/Foundation/Summit/Speakers/PresentationSpeaker.phpapp/Models/OAuth2/ResourceServerContext.phpapp/Repositories/Summit/DoctrineSummitEventRepository.phpconfig/doctrine.phproutes/api_v1.php
Comment on lines
+43
to
+47
| ``` | ||
| Server-Timing: boot;dur=…, pre;dur=…, controller;dur=…, | ||
| db;dur=…;desc="N queries", serializer;dur=…, | ||
| post;dur=…, app;dur=…, total;dur=… | ||
| ``` |
There was a problem hiding this comment.
Add a language tag to the fenced example block.
Line 43 opens a fenced block without a language, which triggers markdownlint MD040.
📝 Proposed fix
-```
+```http
Server-Timing: boot;dur=…, pre;dur=…, controller;dur=…,
db;dur=…;desc="N queries", serializer;dur=…,
post;dur=…, app;dur=…, total;dur=…</details>
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.22.1)</summary>
[warning] 43-43: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @adr/002-events-endpoint-n-plus-1-elimination.md around lines 43 - 47, The
fenced code block showing the Server-Timing header is missing a language tag
causing markdownlint MD040; update the opening fence for that block (the
triple-backticks before the "Server-Timing: boot;dur=…" example) to include the
"http" language tag so the block becomes ```http and the linter passes.
</details>
<!-- fingerprinting:phantom:poseidon:hawk -->
<!-- This is an auto-generated comment by CodeRabbit -->
| Route::group(array('prefix' => 'events'), function () { | ||
|
|
||
| Route::get('', ['middleware' => 'auth.user', 'uses' => 'OAuth2SummitEventsApiController@getEvents']); | ||
| Route::get('', ['middleware' => ['server.timing.doctrine', 'auth.user'], 'uses' => 'OAuth2SummitEventsApiController@getEvents']); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Find all routes using server.timing.doctrine middleware and check order consistency
# Search for routes with server.timing.doctrine middleware
rg -n -C2 "server\.timing\.doctrine" --type=php routes/Repository: OpenStackweb/summit-api
Length of output: 1228
Align server.timing.doctrine middleware ordering for consistent profiling
routes/api_v1.phpuses['server.timing.doctrine', 'auth.user']for the events GET route, but['auth.user', 'server.timing.doctrine']for the tickets GET route; standardize the order (or document why they intentionally differ) so timing measurements are comparable.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@routes/api_v1.php` at line 636, The middleware order for the events GET route
is inverted compared to the tickets GET route which leads to inconsistent
profiling; update the events route declaration that references
OAuth2SummitEventsApiController@getEvents to use the same middleware ordering as
the tickets route (i.e., ['auth.user', 'server.timing.doctrine']) or, if the
difference is intentional, add a comment explaining why the ordering differs so
timing measurements remain comparable and maintainers understand the choice.
… reset on auth context change - DoctrineSummitEventRepository: change 'SELECT sp, p' to 'SELECT sp ... JOIN FETCH sp.presentation p' so getResult() returns SummitSelectedPresentation[] instead of mixed arrays. The prior query caused $sp->getPresentation() to fail on every request, silently falling through the try/catch and leaving the getSelectionStatus() N+1 optimization inactive. - ResourceServerContext: reset cachedCurrentUser/cachedCurrentUserResolved in setAuthorizationContext() and updateAuthContextVar() so a context change mid-request (or between requests in tests) does not return a stale member.
- adr/002: fix heading ADR-0001 → ADR-0002 to match filename - PresentationSpeaker: add clearPreloadedAssignmentOrder(id) and clearAllPreloadedAssignmentOrders() so write paths can invalidate the preloaded assignment-order cache - PresentationSpeakerCacheTest (8 tests, no DB): covers cache hit, null order, clear-single, clear-all, and Presentation preloaded selection-status path (memoization + reset) - ResourceServerContextTest: add setAuthorizationContextResetsUserCache asserting cachedCurrentUserResolved is cleared by setAuthorizationContext()
|
📘 OpenAPI / Swagger preview ➡️ https://OpenStackweb.github.io/summit-api/openapi/pr-549/ This page is automatically updated on each push to this PR. |
The afterQuery callable hook was reverted out of the trait during cleanup, which broke the attendees controller's batch preload (it still passes a closure as the 10th arg). Restore the param + invocation between data load and toArray so OAuth2SummitAttendeesApiController::getAttendeesBySummit can warm Summit::preloadSpeakersByMemberIds + the 4 fetch-join preloads (notes / tickets+badges / tags / member) it needs.
smarcet
added a commit
that referenced
this pull request
May 25, 2026
- adr/002: fix heading ADR-0001 → ADR-0002 to match filename - PresentationSpeaker: add clearPreloadedAssignmentOrder(id) and clearAllPreloadedAssignmentOrders() so write paths can invalidate the preloaded assignment-order cache - PresentationSpeakerCacheTest (8 tests, no DB): covers cache hit, null order, clear-single, clear-all, and Presentation preloaded selection-status path (memoization + reset) - ResourceServerContextTest: add setAuthorizationContextResetsUserCache asserting cachedCurrentUserResolved is cleared by setAuthorizationContext()
|
📘 OpenAPI / Swagger preview ➡️ https://OpenStackweb.github.io/summit-api/openapi/pr-549/ This page is automatically updated on each push to this PR. |
…mings Session is unavailable on stateless api routes (no StartSession middleware), so timing.controller_start/end and timing.serializer_start/end were always null, causing pre/controller/serializer/post phases to report 0 ms. Replace Session::put/get with $request->attributes->set/get, which is request-scoped and available on all route groups without a session driver.
A first call with synch_groups=false permanently suppressed group sync for the rest of the request, because the cache early-returned without checking whether checkGroups() had been run yet. Add $groupsSynched / $fieldsSynched flags. On cache hit, if the caller requests synch_groups=true and checkGroups() has not been run yet, execute it in a transaction and update the cached member. Record which side-effects were performed at first resolution so they run at most once. Both flags are reset alongside the member cache in setAuthorizationContext and updateAuthContextVar.
…g methods setSelectionPlan, clearSelectionPlan, setCategory, and setSelectedPresentations can all change the result of getSelectionStatus() but did not invalidate the memo. A write path that called getSelectionStatus() before mutation (e.g. for validation) and again after (e.g. in the response serializer) would return a stale status for the remainder of the request. setPreloadedSessionSelections already reset the memo; the four mutators now do the same.
…esentationSerializer cache Doctrine's PreUpdate lifecycle callback only fires when scalar columns on the Presentation row change. Adding or removing speakers, materials (slides, videos, links, media uploads), or tags modifies join tables without touching the Presentation row, so last_edited was never updated and the cache key never changed — stale serialized data was served for the full 1200s TTL. Call updateLastEdited() in: - Presentation: addSpeaker, removeSpeaker, clearSpeakers - Presentation: addMaterial (covers addSlide/addVideo/addLink/addMediaUpload) - Presentation: removeSlide, removeVideo, removeLink, removeMediaUpload - SummitEvent: addTag, clearTags Guards (early-return on no-op) are respected — updateLastEdited only fires when a real change is made.
|
📘 OpenAPI / Swagger preview ➡️ https://OpenStackweb.github.io/summit-api/openapi/pr-549/ This page is automatically updated on each push to this PR. |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/Models/Foundation/Summit/Events/Presentations/Presentation.php (1)
459-480:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReorder-only mutations still leave the serializer cache stale.
These
updateLastEdited()calls cover add/remove flows, butupdateSpeakerOrder()andrecalculateMaterialOrder()still do not bumplast_edited. With the serializer cache now keyed bylast_edited, a reorder can keep serving the old speaker/material order until some unrelated edit happens.♻️ Follow-up outside this hunk
public function updateSpeakerOrder(PresentationSpeaker $speaker, int $order) { ... self::recalculateOrderForSelectable($this->speakers, $speaker_assignment, $order, PresentationSpeakerAssignment::class); $this->updateLastEdited(); } public function recalculateMaterialOrder(PresentationMaterial $material, $new_order) { self::recalculateOrderForSelectable($this->materials, $material, $new_order); $this->updateLastEdited(); }Also applies to: 916-922
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/Models/Foundation/Summit/Events/Presentations/Presentation.php` around lines 459 - 480, Reorder-only changes (speaker/material order) do not update last_edited, so the serializer cache keyed by last_edited remains stale; add calls to updateLastEdited() at the end of the reorder paths — specifically inside updateSpeakerOrder (after self::recalculateOrderForSelectable(...)) and inside recalculateMaterialOrder (after self::recalculateOrderForSelectable(...)) so any reordering bumps last_edited; also audit any other reorder helper like recalculateOrderForSelectable/recalculateOrderForSelectable usages and ensure updateLastEdited() is invoked after those reorder operations (e.g., the methods referenced in the follow-up snippet).
♻️ Duplicate comments (1)
app/Models/OAuth2/ResourceServerContext.php (1)
194-202:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHonor
fieldsSynchedon cached hits too.
fieldsSynchedis now tracked, but the cached path only replayscheckGroups(). A firstgetCurrentUser(false, false)still prevents later calls from refreshing member profile fields, so the method remains call-order dependent.Also applies to: 310-311
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/Models/OAuth2/ResourceServerContext.php` around lines 194 - 202, Cached-path only replays checkGroups(), ignoring fieldsSynched so an initial getCurrentUser(false, false) can block later field refreshes; update the cached hit branch that checks $this->cachedCurrentUserResolved to also honor $this->fieldsSynched: when $synch_fields is true and !$this->fieldsSynched and $this->cachedCurrentUser !== null, run the field sync inside the same transaction (use $this->tx_service->transaction(fn() => $this->checkFields($member))) and set $this->fieldsSynched = true, mirroring how groups are handled (references: $this->cachedCurrentUserResolved, $this->cachedCurrentUser, $this->groupsSynched, $this->fieldsSynched, $this->tx_service->transaction, checkGroups(), checkFields()); apply the same change to the other cached-hit branch around the later occurrence noted (lines 310-311 equivalent).
🧹 Nitpick comments (1)
app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitEventsApiController.php (1)
173-194: ⚡ Quick winKeep timing end markers in
finallyblocks.If
getEvents(),toArray(), or$this->ok($data)throws,timing.serializer_end/timing.controller_endnever get written, and the newServer-Timingbreakdown collapses those phases to0on failures. Wrap both scopes intry/finallyso error responses are still measurable.📏 Minimal follow-up
$req = request(); $req->attributes->set('timing.controller_start', microtime(true)); return $this->processRequest(function () use ($summit_id, $req) { $current_user = $this->resource_server_context->getCurrentUser(true); return $this->withReplica(function() use ($summit_id, $current_user, $req) { - $strategy = new RetrieveAllSummitEventsBySummitStrategy($this->repository, $this->event_repository, $this->resource_server_context); - $response = $strategy->getEvents(['summit_id' => $summit_id]); - $req->attributes->set('timing.serializer_start', microtime(true)); - $data = $response->toArray(...); - $req->attributes->set('timing.serializer_end', microtime(true)); - $result = $this->ok($data); - $req->attributes->set('timing.controller_end', microtime(true)); - return $result; + try { + $strategy = new RetrieveAllSummitEventsBySummitStrategy($this->repository, $this->event_repository, $this->resource_server_context); + $response = $strategy->getEvents(['summit_id' => $summit_id]); + $req->attributes->set('timing.serializer_start', microtime(true)); + try { + $data = $response->toArray(...); + } finally { + $req->attributes->set('timing.serializer_end', microtime(true)); + } + return $this->ok($data); + } finally { + $req->attributes->set('timing.controller_end', microtime(true)); + } }); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitEventsApiController.php` around lines 173 - 194, The timing end markers "timing.serializer_end" and "timing.controller_end" can be skipped if exceptions are thrown in RetrieveAllSummitEventsBySummitStrategy->getEvents, the response->toArray call, or $this->ok($data); wrap the serializer phase and the outer controller phase in try/finally blocks inside the processRequest closure so that the finally clauses always set the respective request attributes (use $req->attributes->set(...)) even on error; keep current calls to getEvents(), toArray(...), and $this->ok(...) inside the try blocks and move the timing.serializer_end and timing.controller_end attribute writes into their corresponding finally blocks.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/Models/Foundation/Summit/Events/Presentations/Presentation.php`:
- Around line 938-939: The mutators that change selection state (they currently
set $memoizedSelectionStatus = null and update $selected_presentations) must
also clear the preload cache by setting $preloadedSessionSelections = null so
getSelectionStatus() won't use stale preloaded rows; update the same pattern
wherever selection/plan/category changes (the other similar blocks around the
methods that update selection state and currently touch $memoizedSelectionStatus
and $selected_presentations) and ensure getSelectionStatus() continues to prefer
preloaded data only when $preloadedSessionSelections is intentionally populated.
---
Outside diff comments:
In `@app/Models/Foundation/Summit/Events/Presentations/Presentation.php`:
- Around line 459-480: Reorder-only changes (speaker/material order) do not
update last_edited, so the serializer cache keyed by last_edited remains stale;
add calls to updateLastEdited() at the end of the reorder paths — specifically
inside updateSpeakerOrder (after self::recalculateOrderForSelectable(...)) and
inside recalculateMaterialOrder (after self::recalculateOrderForSelectable(...))
so any reordering bumps last_edited; also audit any other reorder helper like
recalculateOrderForSelectable/recalculateOrderForSelectable usages and ensure
updateLastEdited() is invoked after those reorder operations (e.g., the methods
referenced in the follow-up snippet).
---
Duplicate comments:
In `@app/Models/OAuth2/ResourceServerContext.php`:
- Around line 194-202: Cached-path only replays checkGroups(), ignoring
fieldsSynched so an initial getCurrentUser(false, false) can block later field
refreshes; update the cached hit branch that checks
$this->cachedCurrentUserResolved to also honor $this->fieldsSynched: when
$synch_fields is true and !$this->fieldsSynched and $this->cachedCurrentUser !==
null, run the field sync inside the same transaction (use
$this->tx_service->transaction(fn() => $this->checkFields($member))) and set
$this->fieldsSynched = true, mirroring how groups are handled (references:
$this->cachedCurrentUserResolved, $this->cachedCurrentUser,
$this->groupsSynched, $this->fieldsSynched, $this->tx_service->transaction,
checkGroups(), checkFields()); apply the same change to the other cached-hit
branch around the later occurrence noted (lines 310-311 equivalent).
---
Nitpick comments:
In
`@app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitEventsApiController.php`:
- Around line 173-194: The timing end markers "timing.serializer_end" and
"timing.controller_end" can be skipped if exceptions are thrown in
RetrieveAllSummitEventsBySummitStrategy->getEvents, the response->toArray call,
or $this->ok($data); wrap the serializer phase and the outer controller phase in
try/finally blocks inside the processRequest closure so that the finally clauses
always set the respective request attributes (use $req->attributes->set(...))
even on error; keep current calls to getEvents(), toArray(...), and
$this->ok(...) inside the try blocks and move the timing.serializer_end and
timing.controller_end attribute writes into their corresponding finally blocks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 19b6f2d5-52f0-4270-8422-51f76c2a6478
📒 Files selected for processing (11)
adr/002-events-endpoint-n-plus-1-elimination.mdapp/Http/Controllers/Apis/Protected/Summit/OAuth2SummitEventsApiController.phpapp/Http/Controllers/Apis/Protected/Summit/Traits/ParametrizedGetAll.phpapp/Http/Middleware/ServerTimingDoctrine.phpapp/Models/Foundation/Summit/Events/Presentations/Presentation.phpapp/Models/Foundation/Summit/Events/SummitEvent.phpapp/Models/Foundation/Summit/Speakers/PresentationSpeaker.phpapp/Models/OAuth2/ResourceServerContext.phpapp/Repositories/Summit/DoctrineSummitEventRepository.phptests/PresentationSpeakerCacheTest.phptests/ResourceServerContextTest.php
✅ Files skipped from review due to trivial changes (1)
- adr/002-events-endpoint-n-plus-1-elimination.md
Comment on lines
+938
to
939
| $this->memoizedSelectionStatus = null; | ||
| $this->selected_presentations = $selected_presentations; |
There was a problem hiding this comment.
Clear the preload cache when selection state changes.
getSelectionStatus() always prefers $preloadedSessionSelections when it is non-null, so these mutators still recompute against stale preloaded rows after a selection, plan, or category change. Reset that cache alongside $memoizedSelectionStatus.
🧹 Minimal fix pattern
public function setSelectedPresentations($selected_presentations)
{
+ $this->preloadedSessionSelections = null;
$this->memoizedSelectionStatus = null;
$this->selected_presentations = $selected_presentations;
}
public function setSelectionPlan($selection_plan)
{
+ $this->preloadedSessionSelections = null;
$this->memoizedSelectionStatus = null;
...
}
public function clearSelectionPlan()
{
+ $this->preloadedSessionSelections = null;
$this->memoizedSelectionStatus = null;
$this->selection_plan = null;
}
public function setCategory(PresentationCategory $category)
{
+ $this->preloadedSessionSelections = null;
$this->memoizedSelectionStatus = null;
...
}Also applies to: 1090-1103, 2195-2205
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/Models/Foundation/Summit/Events/Presentations/Presentation.php` around
lines 938 - 939, The mutators that change selection state (they currently set
$memoizedSelectionStatus = null and update $selected_presentations) must also
clear the preload cache by setting $preloadedSessionSelections = null so
getSelectionStatus() won't use stale preloaded rows; update the same pattern
wherever selection/plan/category changes (the other similar blocks around the
methods that update selection state and currently touch $memoizedSelectionStatus
and $selected_presentations) and ensure getSelectionStatus() continues to prefer
preloaded data only when $preloadedSessionSelections is intentionally populated.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ref: https://app.clickup.com/t/86ba466d4
Summary
Profiling-driven N+1 elimination on
GET /api/v1/summits/{id}/events.Full design notes, every fix (symptom / root cause / impact), and what was intentionally NOT fixed:
adr/002-events-endpoint-n-plus-1-elimination.md.What changed
Profiling infrastructure (kept in main):
ServerTimingDoctrinemiddleware emits a per-phaseServer-Timingresponse header — Chrome DevTools renders it natively in the Network → Timing panel.QueryTimingMiddlewareprovides accurate per-request SQL time + query count (Doctrine 3.x deprecatedSQLLoggerno longer works for prepared statements).Surgical fixes — each one keyed to a measured pattern:
Member::belongsToGroup()per-instance memoizationModels/Foundation/Main/Member.phpResourceServerContext::getCurrentUser()request-scoped cacheModels/OAuth2/ResourceServerContext.phpPresentationSpeakerAssignment + Speaker + MemberpreloadRepositories/Summit/DoctrineSummitEventRepository.phpPresentationSpeaker::getPresentationAssignmentOrder()preloaded cacheModels/Foundation/Summit/Speakers/PresentationSpeaker.phpSummitSelectedPresentationpreload + memoizegetSelectionStatus()Models/Foundation/Summit/Events/Presentations/Presentation.phplocation+categoryin main hydrationRepositories/Summit/DoctrineSummitEventRepository.phpTag/Sponsor/PresentationMaterialpreloadsRepositories/Summit/DoctrineSummitEventRepository.phpet2JOIN from hydration (also fixed silent row-drop bug)Repositories/Summit/DoctrineSummitEventRepository.phpMethodology
Server-Timinginstrumentation + accurate DB timing FIRST (a previous generic-eager-loader attempt without measurements caused regressions; the branch was reverted).?). Dumped top patterns to laravel.log whendb_count >= 20.Server-Timing+QueryTimingMiddleware.Verification
Tested on
api2.dev.fnopen.comover many runs with the same expand set used by the admin UI:expand=speakers,type,created_by,track,sponsors,selection_plan,location,tags,media_uploads,media_uploads.media_upload_typeFinal steady-state Server-Timing:
What was intentionally NOT fixed
Presentation::getSpeakers() matching()— 10 queries. Requires entity method change (toArray() + PHP usort); previous attempt caused regressions.created_by).SET TRANSACTIONstatements (~2ms total). Connection lifecycle, not application code.Risks / things to watch
per_page=10× handful of tags/sponsors per event, but worth re-profiling if pagination defaults grow.$cachedCurrentUser,$preloadedSessionSelections, etc.) are correct only as long as instances are request-scoped. Long-lived workers reusing an EM withoutclear()would need explicit invalidation. None of the current code does this.Summary by CodeRabbit
Bug Fixes
Performance
User-visible Behavior
Documentation