diff --git a/adr/002-events-endpoint-n-plus-1-elimination.md b/adr/002-events-endpoint-n-plus-1-elimination.md new file mode 100644 index 0000000000..82deff69e4 --- /dev/null +++ b/adr/002-events-endpoint-n-plus-1-elimination.md @@ -0,0 +1,380 @@ +# ADR-0002: Eliminate N+1 Queries in `/events` Endpoint + +- **Status:** Accepted +- **Date:** 2026-05-25 +- **Endpoint:** `GET /api/v1/summits/{id}/events` +- **Branch:** `hotfix/cache-optimizations` + +## Context + +The `/events` endpoint was averaging ~1.5 s server-side for a typical 10-event +page with the standard expand set (`speakers,type,created_by,track,sponsors, +selection_plan,location,tags,media_uploads,media_uploads.media_upload_type`). +Empirically the DB phase alone was inconsistent (sub-second up to 1.1 s) and +the rest of the request was opaque — we did not know how much time was spent +in the framework boot, the OAuth middleware, the controller body, the +serializer chain, or response wrapping. + +A first attempt at a generic `GraphLoader`-based eager-loader caused +regressions because it ran on every request regardless of whether the +existing caching would have handled the load anyway. That branch was +reverted and replaced with this work, which is **profiling-driven and +surgical**: every change targets a specific N+1 pattern that was +demonstrated to fire in production logs. + +## Decision + +Two distinct workstreams: + +1. **Profiling infrastructure** — make per-request work *measurable* before + making any code changes. +2. **Targeted N+1 fixes** — each one a small, isolated change keyed to a + real pattern observed in the profiler output. + +### 1. Profiling infrastructure + +#### 1.1 `Server-Timing` HTTP header (`ServerTimingDoctrine` middleware) + +Reordered the route's middleware so `server.timing.doctrine` runs first +(before `auth.user`), and extended it to emit a per-phase breakdown that +Chrome DevTools renders natively in the Network tab → Timing → Server +Timing section: + +``` +Server-Timing: boot;dur=…, pre;dur=…, controller;dur=…, + db;dur=…;desc="N queries", serializer;dur=…, + post;dur=…, app;dur=…, total;dur=… +``` + +| Phase | Measures | +| ------------ | -------------------------------------------------------------- | +| `boot` | `LARAVEL_START` → `server.timing.doctrine` middleware start | +| `pre` | middleware start → controller entry (`auth.user`, etc.) | +| `controller` | full controller body (`processRequest` / `withReplica` closure) | +| `db` | aggregate SQL time across the request | +| `serializer` | the `$response->toArray()` call only | +| `post` | controller return → middleware exit | +| `app` | `total − db` (cross-check vs `controller`) | +| `total` | middleware entry → middleware exit | + +The controller body marks each transition by writing `microtime(true)` to +the session under keys `timing.controller_start`, `timing.serializer_start`, +`timing.serializer_end`, `timing.controller_end`. + +#### 1.2 Accurate DB measurement: `QueryTimingMiddleware` + +Doctrine 3.x deprecated `Configuration::setSQLLogger`, and its replacement +`Doctrine\DBAL\Logging\Middleware` does **not** pass query duration in its +PSR-3 context, so the previous `ServerTimingDoctrine` reported `db = 0.4 ms` +no matter what. We replaced it with a proper **DBAL Driver Middleware** +(`App\Http\Middleware\Doctrine\QueryTimingMiddleware`) that wraps the driver +chain (Driver → Connection → Statement) and times every `query()` / +`exec()` / `Statement::execute()`. Totals accumulate into +`QueryTimingCollector` (static, request-scoped, reset by +`ServerTimingDoctrine` at the top of each request). Per-query overhead is +two `microtime(true)` calls. Registered globally in `config/doctrine.php` +so it always runs. + +This is the foundation everything else builds on — without accurate db ms + +count, we could not distinguish "the database is slow" from "the serializer +fires lazy loads we cannot see". + +### 2. Profiling-driven fixes + +For each fix below: the *Symptom* is the exact SQL pattern that fired in the +production logs; the *Root cause* is why; the *Fix* is the minimal change; +the *Impact* is the measured reduction in query count and/or time. + +--- + +#### Fix 1 — `Member::belongsToGroup()` per-instance memoization + +**Symptom:** `SELECT COUNT(MemberID) FROM Group_Members JOIN Group …` fired +**84 times** per request, all for the same `Member` (the authenticated +user). + +**Root cause:** `PresentationSerializer::getMediaUploadsSerializerType()` +calls `$currentUser->isAdmin()` and `$presentation->memberCanEdit($currentUser)`, +both of which internally invoke `belongsToGroup($code)`. Each call +re-executed a raw SQL `SELECT COUNT(MemberID)` against `Group_Members`. With +~8 group codes checked per presentation and 10 presentations on the page, +that's 80 redundant queries against an answer that never changes within a +request. + +**Fix:** Added `private array $groupMembershipCache = []` (unannotated, so +Doctrine ignores it) and cache the result keyed by trimmed group code. +First call per code hits the DB; every subsequent call within the request +returns the cached boolean. + +**File:** `app/Models/Foundation/Main/Member.php` + +**Impact:** 84 → ~8 queries (~76 saved, ~85 ms of DB time saved per request). + +--- + +#### Fix 2 — `ResourceServerContext::getCurrentUser()` request-scoped cache + +**Symptom:** `SELECT * FROM Member WHERE …` fired **98 of 100 times** for +the *same* Member ID (the current user). + +**Root cause:** `getCurrentUser()` is called many times per request by the +serializer chain (per-event `getSerializerType()`, per-presentation +`getMediaUploadsSerializerType()`, etc.). Each call runs +`$member_repository->getByExternalId(intval($user_external_id))` plus +optional group sync, hitting the DB every time. + +**Fix:** Added `private ?Member $cachedCurrentUser` and a resolved-flag, +populated at every return point. The authenticated user does not change +within a single request, so the same `Member` is the correct answer every +time. Side effects (group sync, `MemberAssocSummitOrders::dispatch`, field +updates) run only on the first call — they're idempotent per request anyway. + +**File:** `app/Models/OAuth2/ResourceServerContext.php` + +**Impact:** This was the single largest win. Total query count: **220 → +117 (-103)**. Serializer time alone dropped ~370 ms because every redundant +`getCurrentUser()` call had been wrapped in a transaction whose overhead +compounded. Also collapsed the `SET TRANSACTION ISOLATION LEVEL` +overhead from 42 → ~3 (each transaction caused a new isolation level +declaration on connection acquisition). + +--- + +#### Fix 3 — Batch preload `PresentationSpeakerAssignment + PresentationSpeaker + Member` + +**Symptom:** `SELECT * FROM Member WHERE id = ?` fired 56 times and +`SELECT … FROM Presentation_Speakers WHERE PresentationID = ? AND +PresentationSpeakerID = ?` fired 19 times. + +**Root cause:** Two separate lazy-load chains: + +1. `PresentationSpeaker::getFirstName()` and `getLastName()` fall back to + `$this->member->getFirstName()` when the speaker's own field is empty. + Member is `ManyToOne(fetch="EXTRA_LAZY")` → one DB load per speaker. +2. `PresentationSpeaker::getPresentationAssignmentOrder($presentation)` + did `$this->presentations->matching($criteria)->first()`. On + EXTRA_LAZY collections, `matching()` **always** fires SQL regardless + of whether the assignment is already in the identity map. + +**Fix:** In `DoctrineSummitEventRepository::getAllByPage`, after the main +hydration, run **one** DQL that fetch-joins all three: + +```dql +SELECT a, s, m FROM PresentationSpeakerAssignment a +JOIN a.speaker s +LEFT JOIN s.member m +WHERE a.presentation IN (:ids) +``` + +The `a, s, m` selection (root + fetch-joined associations) is what Doctrine +requires for a true fetch join — `SELECT s, m` alone fails with +*"Cannot select entity through identification variables without choosing +at least one root entity alias"*. After the query, every speaker has its +member already initialized in the UnitOfWork. + +For Fix #2 above (composite-key lookup), we added +`PresentationSpeaker::setPreloadedAssignmentOrder(int $pid, ?int $order)` +and a `$preloadedAssignmentOrders` array on the entity (unannotated). The +repository iterates the assignments it just loaded and pushes each +`(presentation_id → order)` pair into the corresponding speaker. +`getPresentationAssignmentOrder()` now reads from this cache and only +falls back to the `matching()` DQL when the cache is unset (e.g., for code +paths that don't go through `getAllByPage`). + +**Files:** +- `app/Repositories/Summit/DoctrineSummitEventRepository.php` +- `app/Models/Foundation/Summit/Speakers/PresentationSpeaker.php` + +**Impact:** 75 queries collapsed into 1; ~216 ms of DB time saved. + +--- + +#### Fix 4 — Batch preload `SummitSelectedPresentation` + memoize `getSelectionStatus()` + +**Symptom:** A DQL `SELECT sp FROM SummitSelectedPresentation sp JOIN sp.list +l JOIN sp.presentation p WHERE p.id = ? AND sp.collection = ? AND +l.list_type = ? AND l.list_class = ?` fired 20 times. + +**Root cause:** `Presentation::getSelectionStatus()` ran the DQL per +presentation, and the serializer accessed `selection_status` once per +presentation per request. + +**Fix:** Two parts: + +1. `Presentation` gains a transient `$preloadedSessionSelections` array and + `setPreloadedSessionSelections(array)`. `getSelectionStatus()` uses + these rows if set, otherwise falls through to the original DQL. + The computed status is also memoized in `$memoizedSelectionStatus` so + repeated `getSelectionStatus()` calls cost nothing after the first. + +2. `DoctrineSummitEventRepository::getAllByPage` runs one batch DQL with + the same filters (`collection=Selected, list_type=Group, list_class=Session`) + but `WHERE p.id IN (:ids)`, groups results by presentation id, and + feeds each Presentation via the setter. + +**Files:** +- `app/Models/Foundation/Summit/Events/Presentations/Presentation.php` +- `app/Repositories/Summit/DoctrineSummitEventRepository.php` + +**Impact:** 20 → 1 query. + +--- + +#### Fix 5 — Fetch-join `Location` and `PresentationCategory` in main hydration + +**Symptom:** +- 10 queries selecting `SummitAbstractLocation` (per-event lazy load). +- 5 queries selecting `PresentationCategory` (per-event lazy load). + +**Root cause:** The original hydration query selected +`e, p, et, et2` — only the event, the Presentation subclass row, and the +type/PresentationType. `location` and `category` were EXTRA_LAZY `ManyToOne` +associations that the serializer would dereference per event. + +**Fix:** Added `LEFT JOIN e.location loc + addSelect('loc')` and +`LEFT JOIN p.category cat + addSelect('cat')` to the main query. Doctrine's +JOINED inheritance handles `SummitVenueRoom` etc. subclasses automatically. + +**File:** `app/Repositories/Summit/DoctrineSummitEventRepository.php` + +**Impact:** 15 queries removed in a single hydration step. The JOIN tree +grew but the total request time stayed flat compared with leaving the +lazy loads in place (per-query latency + identity-map maintenance cost +matched the JOIN cost). + +--- + +#### Fix 6 — Batch preload `Tag`, `Sponsor`, `PresentationMaterial` collections + +**Symptom:** Three near-identical per-entity lazy loads: +- 10× `SELECT … FROM Tag t INNER JOIN SummitEvent_Tags WHERE SummitEventID = ?` +- 10× `SELECT … FROM Company c INNER JOIN SummitEvent_Sponsors WHERE …` (sponsors) +- 10× `SELECT … FROM PresentationMaterial WHERE PresentationID = ?` + +**Root cause:** Each is an EXTRA_LAZY collection on the event/presentation +that the serializer iterates. EXTRA_LAZY + iteration triggers a per-entity +full-collection load. + +**Fix:** Three fetch-join batch queries after the main hydration: + +```dql +SELECT e, t FROM SummitEvent e LEFT JOIN e.tags t WHERE e.id IN (:ids) +SELECT e, s FROM SummitEvent e LEFT JOIN e.sponsors s WHERE e.id IN (:ids) +SELECT p, m FROM Presentation p LEFT JOIN p.materials m WHERE p.id IN (:presentationIds) +``` + +The fetch-join pattern (root + collection alias both in SELECT) is what +Doctrine requires to populate the inverse-side collection on the parent +entities. Once those collections are populated, subsequent serializer +iterations read from memory. + +**File:** `app/Repositories/Summit/DoctrineSummitEventRepository.php` + +**Impact:** 30 queries collapsed into 3. + +--- + +#### Fix 7 — Remove redundant `et2` JOIN in main hydration + +**Symptom:** Log warnings `DoctrineSummitEventRepository::getAllByPage +unexpected hydration row {"type":"PresentationType"}` fired multiple times +per request — confirming events with `PresentationType` discriminators were +being returned as separate root entities and silently dropped from the +result map (so the page returned fewer items than `per_page`). + +**Root cause:** The main hydration explicitly joined +`LEFT JOIN PresentationType et2 WITH et.id = et2.id` and selected `et2`, +which made `et2` appear as a separate root entity in `getResult()`. +Doctrine's JOINED inheritance on `SummitEventType` already hydrates the +correct subclass via the `ClassName` discriminator column when you do +`INNER JOIN e.type et` — the extra `et2` JOIN was redundant. + +**Fix:** Dropped `et2` from the main hydration's SELECT and JOIN list. +Kept it in the `getFastCount` and `getAllIdsByPage` queries where it +**is** used (filter predicates reference `et2.allow_attendee_vote`). + +**File:** `app/Repositories/Summit/DoctrineSummitEventRepository.php` + +**Impact:** Correctness fix — pages with presentations now return the +correct number of items. No additional perf win but it stopped a class of +silent data loss. + +## Consequences + +### Performance — measured on dev (`api2.dev.fnopen.com`) + +Same endpoint, same summit, same expand set, warm OPcache: + +| Metric | Baseline (main) | After all fixes | Δ | +| --------------- | --------------- | --------------- | -------- | +| Queries | 298 | 47 | **-84%** | +| DB time | ~410 ms | ~175 ms | -57% | +| Serializer time | ~640 ms | ~50 ms | **-92%** | +| Total (server) | ~1500 ms | ~340 ms | **-77%** | +| Speed vs prod | baseline (1.4 s) | ~4× faster | | + +### What we kept + +- `ServerTimingDoctrine` middleware + `Server-Timing` header — useful for + ongoing visibility; Chrome DevTools renders it for free. +- `QueryTimingMiddleware` and `QueryTimingCollector` — provides accurate + per-request SQL time and count without depending on Doctrine's + deprecated `SQLLogger`. +- All entity-level caches (`Member::$groupMembershipCache`, + `ResourceServerContext::$cachedCurrentUser`, + `Presentation::$preloadedSessionSelections + $memoizedSelectionStatus`, + `PresentationSpeaker::$preloadedAssignmentOrders`). These are + per-instance and per-request — Doctrine discards them on entity + re-hydration, so there is no stale-cache risk across requests. + +### What we did *not* fix (and why) + +- **`Presentation::getSpeakers()` matching() — 10 queries.** Calls + `$this->speakers->matching($criteria)` which on `EXTRA_LAZY` always + fires SQL regardless of identity-map state. Fixing this requires + changing the entity method to use `toArray() + PHP usort` in memory. + A previous attempt at this caused regressions in other code paths + that depend on the lazy semantics. Out of scope for this fix. +- **The remaining ~4 `Member` SELECTs.** These come from non-current-user + Member references (e.g., another event's `created_by`) and are not + worth batch-loading individually. +- **Boot phase ~300 ms.** Laravel framework boot, route resolution, + OAuth middleware initialization. Tuning this is a separate concern + (config cache, route cache, OPcache preload, fewer service providers) + and orthogonal to the N+1 work. + +### Risks + +- The fetch-join batch queries make the hydration phase produce wider + result sets. If a presentation page ever has very large + `tags`/`sponsors`/`materials` collections per event, the batch query + could materialize a Cartesian-ish row set. Empirically not a problem + for the typical 10 events × ~5 tags/sponsors page. +- The transient cache properties on entities (`$cachedCurrentUser`, + `$preloadedSessionSelections`, etc.) are correct only as long as the + entity instance is request-scoped. Doctrine re-hydrates entities on + fresh requests so the cache resets naturally, but if any code path + starts persisting these entities across requests (e.g., long-lived + workers reusing the EntityManager without `clear()`), the cache must + be invalidated explicitly. None of the current code does this. + +## Methodology — for next time + +The order that produced these results matters: + +1. **Measure first.** A generic eager-loader implemented before profiling + made things worse. Once we shipped `Server-Timing + QueryTimingMiddleware` + and saw real per-phase numbers, every subsequent fix was targeted. +2. **Add a SQL pattern logger temporarily.** Bucketing queries by + normalized SQL (numeric and quoted literals replaced with `?`) made it + trivial to identify which N+1 to attack next. This was removed in the + final cleanup commit. +3. **One pattern per commit.** Each fix is independently revertable. + When something didn't work as expected (e.g., the SP preload silently + failed because of a DQL semantical error), the bisect was a one-commit + step. +4. **Keep the entity-level cache pattern uniform.** Every fix uses the + same shape: a private, unannotated property; a public setter called + by the batch loader; a check in the getter; a fall-through to the + original code path so out-of-band callers still work. This keeps the + blast radius small. diff --git a/app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitEventsApiController.php b/app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitEventsApiController.php index b386bde853..d95e00b905 100644 --- a/app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitEventsApiController.php +++ b/app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitEventsApiController.php @@ -170,24 +170,28 @@ private function getSerializerType(): string */ public function getEvents($summit_id) { - return $this->processRequest(function () use ($summit_id) { + $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) { + 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]); - return $this->ok + $req->attributes->set('timing.serializer_start', microtime(true)); + $data = $response->toArray ( - $response->toArray - ( - SerializerUtils::getExpand(), - SerializerUtils::getFields(), - SerializerUtils::getRelations(), - [ - 'current_user' => $current_user + SerializerUtils::getExpand(), + SerializerUtils::getFields(), + SerializerUtils::getRelations(), + [ + 'current_user' => $current_user ], - $this->getSerializerType() - ) + $this->getSerializerType() ); + $req->attributes->set('timing.serializer_end', microtime(true)); + $result = $this->ok($data); + $req->attributes->set('timing.controller_end', microtime(true)); + return $result; }); }); diff --git a/app/Http/Controllers/Apis/Protected/Summit/Traits/ParametrizedGetAll.php b/app/Http/Controllers/Apis/Protected/Summit/Traits/ParametrizedGetAll.php index 72548bbdfa..90cb4ac01f 100644 --- a/app/Http/Controllers/Apis/Protected/Summit/Traits/ParametrizedGetAll.php +++ b/app/Http/Controllers/Apis/Protected/Summit/Traits/ParametrizedGetAll.php @@ -82,7 +82,8 @@ public function _getAll callable $defaultOrderRules = null, callable $defaultPageSize = null, callable $queryCallable = null, - array $serializerParams = [] + array $serializerParams = [], + callable $afterQuery = null ) { return $this->processRequest(function () use ( @@ -94,7 +95,8 @@ public function _getAll $defaultOrderRules, $defaultPageSize, $queryCallable, - $serializerParams + $serializerParams, + $afterQuery ) { $values = Request::all(); @@ -139,6 +141,14 @@ public function _getAll $applyExtraFilters ); $dbEnd = (microtime(true)-$dbStart)*1000; + + // Optional post-load hook so callers can pre-populate caches / + // batch-load related entities before serialization runs. Receives + // the PagingResponse so it can inspect items. + if (is_callable($afterQuery)) { + $afterQuery($data); + } + $transformStart = microtime(true); $serializerParams['filter'] = $filter; $res = $data->toArray diff --git a/app/Http/Middleware/Doctrine/QueryTimingCollector.php b/app/Http/Middleware/Doctrine/QueryTimingCollector.php new file mode 100644 index 0000000000..f20c70b866 --- /dev/null +++ b/app/Http/Middleware/Doctrine/QueryTimingCollector.php @@ -0,0 +1,29 @@ +em = Registry::getManager(SilverstripeBaseModel::EntityManager); - } - public function handle($request, Closure $next): Response { $start = microtime(true); - $conn = $this->em->getConnection(); - $cfg = $conn->getConfiguration(); - - $dbMs = 0.0; - // --- DBAL 2.x: DebugStack --- - if (class_exists(\Doctrine\DBAL\Logging\DebugStack::class) && method_exists($cfg, 'setSQLLogger')) { - $debugStack = new \Doctrine\DBAL\Logging\DebugStack(); - $prevLogger = $cfg->getSQLLogger() ?? null; - $cfg->setSQLLogger($debugStack); + // Reset the request-scoped SQL timing accumulator. The actual per-query + // timing is done by QueryTimingMiddleware, a DBAL Driver Middleware + // registered globally in config/doctrine.php. That gives accurate + // per-statement durations under DBAL 3.x prepared statements, which + // the deprecated SQLLogger / Logging\Middleware paths do not. + QueryTimingCollector::reset(); - try { - /** @var Response $response */ - $response = $next($request); - } finally { - foreach ($debugStack->queries as $q) { - $dbMs += isset($q['executionMS']) ? (float) $q['executionMS'] : 0.0; - } - $cfg->setSQLLogger($prevLogger); - } + /** @var Response $response */ + $response = $next($request); - // --- DBAL 3.x: Logging\Middleware (PSR-3) --- - } elseif (class_exists(\Doctrine\DBAL\Logging\Middleware::class) && method_exists($cfg, 'setMiddlewares')) { + $dbMs = QueryTimingCollector::$totalMs; + $dbCount = QueryTimingCollector::$count; + $end = microtime(true); + $totalMs = ($end - $start) * 1000.0; + $bootMs = defined('LARAVEL_START') ? max(($start - LARAVEL_START) * 1000.0, 0.0) : 0.0; + $appMs = max($totalMs - $dbMs, 0.0); - $collector = new class implements LoggerInterface { - public float $dbMs = 0.0; - public function log($level, $message, array $context = []): void { - if (isset($context['duration_ms'])) { - $this->dbMs += (float) $context['duration_ms']; - } elseif (isset($context['executionMS'])) { - $this->dbMs += (float) $context['executionMS']; - } elseif (isset($context['duration'])) { - $this->dbMs += (float) $context['duration']; - } - } - public function emergency($m, array $c = []):void { $this->log('emergency', $m, $c); } - public function alert($m, array $c = []):void { $this->log('alert', $m, $c); } - public function critical($m, array $c = []):void { $this->log('critical', $m, $c); } - public function error($m, array $c = []):void { $this->log('error', $m, $c); } - public function warning($m, array $c = []):void { $this->log('warning', $m, $c); } - public function notice($m, array $c = []) :void { $this->log('notice', $m, $c); } - public function info($m, array $c = []):void { $this->log('info', $m, $c); } - public function debug($m, array $c = []):void { $this->log('debug', $m, $c); } - }; - - $mw = new \Doctrine\DBAL\Logging\Middleware($collector); - $prev = method_exists($cfg, 'getMiddlewares') ? $cfg->getMiddlewares() : []; - $cfg->setMiddlewares(array_merge($prev, [$mw])); - - try { - /** @var Response $response */ - $response = $next($request); - } finally { - $dbMs = $collector->dbMs; - $cfg->setMiddlewares($prev); - } + // Read controller-level timing markers (set by the controller via $request->attributes). + // If the controller didn't set them, these phases are reported as 0. + $attrs = $request->attributes; + $cStart = $attrs->has("timing.controller_start") ? (float) $attrs->get("timing.controller_start") : null; + $cEnd = $attrs->has("timing.controller_end") ? (float) $attrs->get("timing.controller_end") : null; + $sStart = $attrs->has("timing.serializer_start") ? (float) $attrs->get("timing.serializer_start") : null; + $sEnd = $attrs->has("timing.serializer_end") ? (float) $attrs->get("timing.serializer_end") : null; - // --- Fallback - } else { - /** @var Response $response */ - $response = $next($request); - } + $preMs = ($cStart !== null) ? max(($cStart - $start) * 1000.0, 0.0) : 0.0; + $controllerMs = ($cStart !== null && $cEnd !== null) ? max(($cEnd - $cStart) * 1000.0, 0.0) : 0.0; + $serializerMs = ($sStart !== null && $sEnd !== null) ? max(($sEnd - $sStart) * 1000.0, 0.0) : 0.0; + $postMs = ($cEnd !== null) ? max(($end - $cEnd) * 1000.0, 0.0) : 0.0; - $totalMs = (microtime(true) - $start) * 1000.0; - $bootMs = defined('LARAVEL_START') ? max(($start - LARAVEL_START) * 1000.0, 0.0) : 0.0; - $appMs = max($totalMs - $dbMs, 0.0); - $dbMs = Session::has("db_time") ? (float) Session::get("db_time") : $dbMs; - $transformMs = Session::has("transform_time") ? (float) Session::get("transform_time") : 0.0; - $encodeMs = Session::has("encode_time") ? (float) Session::get("encode_time") : 0.0; $response->headers->set('Server-Timing', - sprintf('boot;dur=%.1f,db;dur=%.1f,transform;dur=%.1f,encode;dur=%.1f,app;dur=%.1f,total;dur=%.1f', - $bootMs,$dbMs,$transformMs,$encodeMs,$appMs,$totalMs)); + sprintf( + 'boot;dur=%.1f,pre;dur=%.1f,controller;dur=%.1f,db;dur=%.1f;desc="%d queries",serializer;dur=%.1f,post;dur=%.1f,app;dur=%.1f,total;dur=%.1f', + $bootMs, $preMs, $controllerMs, $dbMs, $dbCount, $serializerMs, $postMs, $appMs, $totalMs + ) + ); $response->headers->set('Timing-Allow-Origin', '*'); return $response; diff --git a/app/ModelSerializers/Summit/Presentation/PresentationSerializer.php b/app/ModelSerializers/Summit/Presentation/PresentationSerializer.php index 29c5540a71..f31fbde16a 100644 --- a/app/ModelSerializers/Summit/Presentation/PresentationSerializer.php +++ b/app/ModelSerializers/Summit/Presentation/PresentationSerializer.php @@ -104,11 +104,14 @@ public function serialize($expand = null, array $fields = [], array $relations = $presentation = $this->object; if(!$presentation instanceof Presentation) return []; + // Include last_edited timestamp so a presentation update naturally busts the cache + // without needing an explicit Cache::forget — the old key just ages out via TTL. $key = sprintf ( - "public_presentation_%s_%s_%s_%s", + "public_presentation_%s_%s_%s_%s_%s", $presentation->getId(), + $presentation->getLastEditedUTC()?->getTimestamp() ?? 0, $expand ?? "", implode(",",$fields), implode(",", $relations) diff --git a/app/Models/Foundation/Main/Member.php b/app/Models/Foundation/Main/Member.php index 9fcfb8b0a8..eb418c4931 100644 --- a/app/Models/Foundation/Main/Member.php +++ b/app/Models/Foundation/Main/Member.php @@ -1041,12 +1041,29 @@ public function getGroupByCode(string $code): ?Group return $res === false ? null : $res; } + /** + * Request-scoped cache for group membership lookups. The current-user + * Member instance is shared across the whole request, and the same group + * codes are checked many times by serializers (isAdmin, memberCanEdit, + * etc.) — typical /events request fires this ~80 times for ~8 unique codes. + * Property is unannotated so Doctrine ignores it; resets naturally when + * the entity is re-hydrated on a new request. + * + * @var array + */ + private array $groupMembershipCache = []; + /** * @param string $code * @return bool */ public function belongsToGroup(string $code): bool { + $code = trim($code); + if (array_key_exists($code, $this->groupMembershipCache)) { + return $this->groupMembershipCache[$code]; + } + try { $sql = <<prepareRawSQL($sql, [ 'member_id' => $this->getId(), - 'code' => trim($code), + 'code' => $code, ]); $res = $stmt->executeQuery(); $res = $res->fetchFirstColumn(); - return intval($res[0]) > 0; + return $this->groupMembershipCache[$code] = (intval($res[0]) > 0); } catch (\Exception $ex) { } - return false; + return $this->groupMembershipCache[$code] = false; } diff --git a/app/Models/Foundation/Summit/Events/Presentations/Presentation.php b/app/Models/Foundation/Summit/Events/Presentations/Presentation.php index 91ce9ae739..b867e90703 100644 --- a/app/Models/Foundation/Summit/Events/Presentations/Presentation.php +++ b/app/Models/Foundation/Summit/Events/Presentations/Presentation.php @@ -456,11 +456,13 @@ public function addSpeaker(PresentationSpeaker $speaker) $order = $this->getSpeakerAssignmentsMaxOrder(); $speaker_assignment = new PresentationSpeakerAssignment($this, $speaker, $order + 1); $this->speakers->add($speaker_assignment); + $this->updateLastEdited(); } public function clearSpeakers() { $this->speakers->clear(); + $this->updateLastEdited(); } /** @@ -474,6 +476,7 @@ public function removeSpeaker(PresentationSpeaker $speaker) if ($speaker_assignment === false) return; $this->speakers->removeElement($speaker_assignment); self::resetOrderForSelectable($this->speakers, PresentationSpeakerAssignment::class); + $this->updateLastEdited(); } /** @@ -621,6 +624,7 @@ public function removeVideo(PresentationVideo $video) { $this->materials->removeElement($video); $video->unsetPresentation(); + $this->updateLastEdited(); } /** @@ -630,6 +634,7 @@ public function removeSlide(PresentationSlide $slide) { $this->materials->removeElement($slide); $slide->unsetPresentation(); + $this->updateLastEdited(); } /** @@ -639,6 +644,7 @@ public function removeLink(PresentationLink $link) { $this->materials->removeElement($link); $link->unsetPresentation(); + $this->updateLastEdited(); } /** @@ -648,6 +654,7 @@ public function removeMediaUpload(PresentationMediaUpload $mediaUpload) { $this->materials->removeElement($mediaUpload); $mediaUpload->unsetPresentation(); + $this->updateLastEdited(); } /** @@ -911,6 +918,7 @@ public function addMaterial(PresentationMaterial $material) $this->materials->add($material); $material->setPresentation($this); $material->setOrder($this->getMaterialsMaxOrder() + 1); + $this->updateLastEdited(); return $this; } @@ -927,6 +935,7 @@ public function getSelectedPresentations() */ public function setSelectedPresentations($selected_presentations) { + $this->memoizedSelectionStatus = null; $this->selected_presentations = $selected_presentations; } @@ -950,20 +959,50 @@ public function setAttendingMedia($attending_media) * @return string * @throws ValidationException */ + /** + * Request-scoped preload cache populated by DoctrineSummitEventRepository::getAllByPage + * via setPreloadedSessionSelections(). When set, getSelectionStatus() uses these + * pre-fetched rows instead of firing its own DQL — eliminates one query per + * Presentation on /events listings. Doctrine ignores the unannotated property. + * + * @var SummitSelectedPresentation[]|null + */ + private ?array $preloadedSessionSelections = null; + + /** @var string|null memoized result of getSelectionStatus() within a request */ + private ?string $memoizedSelectionStatus = null; + + /** + * @param SummitSelectedPresentation[] $selections rows already filtered for + * (collection=Selected, list_type=Group, list_class=Session) + */ + public function setPreloadedSessionSelections(array $selections): void + { + $this->preloadedSessionSelections = $selections; + $this->memoizedSelectionStatus = null; + } + public function getSelectionStatus() { + if ($this->memoizedSelectionStatus !== null) { + return $this->memoizedSelectionStatus; + } - $session_sel = $this->createQuery("SELECT sp from models\summit\SummitSelectedPresentation sp - JOIN sp.list l - JOIN sp.presentation p - WHERE p.id = :presentation_id - AND sp.collection = :collection - AND l.list_type = :list_type - AND l.list_class = :list_class") - ->setParameter('presentation_id', $this->id) - ->setParameter('collection', SummitSelectedPresentation::CollectionSelected) - ->setParameter('list_type', SummitSelectedPresentationList::Group) - ->setParameter('list_class', SummitSelectedPresentationList::Session)->getResult(); + if ($this->preloadedSessionSelections !== null) { + $session_sel = $this->preloadedSessionSelections; + } else { + $session_sel = $this->createQuery("SELECT sp from models\summit\SummitSelectedPresentation sp + JOIN sp.list l + JOIN sp.presentation p + WHERE p.id = :presentation_id + AND sp.collection = :collection + AND l.list_type = :list_type + AND l.list_class = :list_class") + ->setParameter('presentation_id', $this->id) + ->setParameter('collection', SummitSelectedPresentation::CollectionSelected) + ->setParameter('list_type', SummitSelectedPresentationList::Group) + ->setParameter('list_class', SummitSelectedPresentationList::Session)->getResult(); + } // Error out if a talk has more than one selection if (count($session_sel) > 1) { @@ -971,20 +1010,20 @@ public function getSelectionStatus() } if ($this->isPublished()) { - return Presentation::SelectionStatus_Accepted; + return $this->memoizedSelectionStatus = Presentation::SelectionStatus_Accepted; } // from here we need to be sure that the selection period is going on or it is over $selection_plan = $this->getSelectionPlan(); if (is_null($selection_plan)) { - return Presentation::SelectionStatus_Pending; + return $this->memoizedSelectionStatus = Presentation::SelectionStatus_Pending; } if (!$selection_plan->hasSelectionPeriodDefined()) { - return Presentation::SelectionStatus_Pending; + return $this->memoizedSelectionStatus = Presentation::SelectionStatus_Pending; } if ($selection_plan->isSelectionNotYetStarted()) { - return Presentation::SelectionStatus_Pending; + return $this->memoizedSelectionStatus = Presentation::SelectionStatus_Pending; } $selection = null; @@ -993,14 +1032,14 @@ public function getSelectionStatus() } if (!$selection) { - return Presentation::SelectionStatus_Unaccepted; + return $this->memoizedSelectionStatus = Presentation::SelectionStatus_Unaccepted; } if ($selection->getOrder() <= $this->getCategory()->getSessionCount()) { - return Presentation::SelectionStatus_Accepted; + return $this->memoizedSelectionStatus = Presentation::SelectionStatus_Accepted; } - return Presentation::SelectionStatus_Alternate; + return $this->memoizedSelectionStatus = Presentation::SelectionStatus_Alternate; } public function getRank(): ?int @@ -1048,6 +1087,7 @@ public function getSelectionPlan(): ?SelectionPlan */ public function setSelectionPlan($selection_plan) { + $this->memoizedSelectionStatus = null; $oldSelectionPlan = $this->selection_plan; // if selection plan changes if (!is_null($oldSelectionPlan) && $oldSelectionPlan->getId() != $selection_plan->getId()) { @@ -1059,6 +1099,7 @@ public function setSelectionPlan($selection_plan) public function clearSelectionPlan() { + $this->memoizedSelectionStatus = null; $this->selection_plan = null; } @@ -2151,6 +2192,7 @@ public function unCastAttendeeVote(SummitAttendee $attendee): void */ public function setCategory(PresentationCategory $category) { + $this->memoizedSelectionStatus = null; // check if we change the category $oldCategory = $this->category; if (!is_null($oldCategory) && $oldCategory->getId() != $category->getId()) { diff --git a/app/Models/Foundation/Summit/Events/SummitEvent.php b/app/Models/Foundation/Summit/Events/SummitEvent.php index 1e4ad79798..9b29214377 100644 --- a/app/Models/Foundation/Summit/Events/SummitEvent.php +++ b/app/Models/Foundation/Summit/Events/SummitEvent.php @@ -821,11 +821,13 @@ public function addTag(Tag $tag) { if ($this->tags->contains($tag)) return; $this->tags->add($tag); + $this->updateLastEdited(); } public function clearTags() { $this->tags->clear(); + $this->updateLastEdited(); } /** diff --git a/app/Models/Foundation/Summit/Speakers/PresentationSpeaker.php b/app/Models/Foundation/Summit/Speakers/PresentationSpeaker.php index 84fa700b78..8d460c052b 100644 --- a/app/Models/Foundation/Summit/Speakers/PresentationSpeaker.php +++ b/app/Models/Foundation/Summit/Speakers/PresentationSpeaker.php @@ -1659,12 +1659,42 @@ public function getAllModeratedPresentations($published_ones = true) })->toArray(); } + /** + * Request-scoped cache of (presentation_id => order) populated by + * DoctrineSummitEventRepository::getAllByPage when assignments are batch-loaded. + * When set, getPresentationAssignmentOrder() reads from here instead of + * firing a per-call DB query via EXTRA_LAZY matching(). Unannotated so + * Doctrine ignores it. + * + * @var array + */ + private array $preloadedAssignmentOrders = []; + + public function setPreloadedAssignmentOrder(int $presentationId, ?int $order): void + { + $this->preloadedAssignmentOrders[$presentationId] = $order; + } + + public function clearPreloadedAssignmentOrder(int $presentationId): void + { + unset($this->preloadedAssignmentOrders[$presentationId]); + } + + public function clearAllPreloadedAssignmentOrders(): void + { + $this->preloadedAssignmentOrders = []; + } + /** * @param Presentation $presentation * @return int|null */ public function getPresentationAssignmentOrder(Presentation $presentation): ?int { + $pid = $presentation->getId(); + if (array_key_exists($pid, $this->preloadedAssignmentOrders)) { + return $this->preloadedAssignmentOrders[$pid]; + } $criteria = Criteria::create(); $criteria->where(Criteria::expr()->eq('presentation', $presentation)); $res = $this->presentations->matching($criteria)->first(); diff --git a/app/Models/OAuth2/ResourceServerContext.php b/app/Models/OAuth2/ResourceServerContext.php index bfc46a25df..39cacbc082 100644 --- a/app/Models/OAuth2/ResourceServerContext.php +++ b/app/Models/OAuth2/ResourceServerContext.php @@ -126,6 +126,10 @@ public function getCurrentUserId() public function setAuthorizationContext(array $auth_context) { $this->auth_context = $auth_context; + $this->cachedCurrentUser = null; + $this->cachedCurrentUserResolved = false; + $this->groupsSynched = false; + $this->fieldsSynched = false; } /** @@ -156,9 +160,29 @@ private function getAuthContextVar(string $varName) public function updateAuthContextVar(string $varName, $value):void { if(isset($this->auth_context[$varName])){ $this->auth_context[$varName] = $value; + $this->cachedCurrentUser = null; + $this->cachedCurrentUserResolved = false; + $this->groupsSynched = false; + $this->fieldsSynched = false; } } + /** + * Request-scoped cache. The current authenticated user does not change + * within a single request, and getCurrentUser() is called many times by + * serializers (per-event, per-sub-serializer permission checks). Without + * this cache, profiling /events showed the same Member SELECT firing 98+ + * times via getByExternalId(). + * + * Side-effects ($synch_groups, $update_member_fields) are tracked separately + * so a first call with false does not permanently suppress them — a later + * call with true will still run the missing side-effect exactly once. + */ + private ?Member $cachedCurrentUser = null; + private bool $cachedCurrentUserResolved = false; + private bool $groupsSynched = false; + private bool $fieldsSynched = false; + /** * @param bool $synch_groups * @param bool $update_member_fields @@ -167,6 +191,17 @@ public function updateAuthContextVar(string $varName, $value):void { */ public function getCurrentUser(bool $synch_groups = true, bool $update_member_fields = true): ?Member { + if ($this->cachedCurrentUserResolved) { + if ($synch_groups && !$this->groupsSynched && $this->cachedCurrentUser !== null) { + $member = $this->cachedCurrentUser; + $this->cachedCurrentUser = $this->tx_service->transaction( + fn() => $this->checkGroups($member) + ); + $this->groupsSynched = true; + } + return $this->cachedCurrentUser; + } + $member = null; // try to get by external id $user_external_id = $this->getAuthContextVar(IResourceServerContext::UserId); @@ -176,7 +211,8 @@ public function getCurrentUser(bool $synch_groups = true, bool $update_member_fi $user_email_verified = boolval($this->getAuthContextVar(IResourceServerContext::UserEmailVerified)); if (is_null($user_external_id)) { - return null; + $this->cachedCurrentUserResolved = true; + return $this->cachedCurrentUser = null; } // first we check by external id $member = $this->tx_service->transaction(function () use ($user_external_id) { @@ -231,10 +267,11 @@ public function getCurrentUser(bool $synch_groups = true, bool $update_member_fi if (is_null($member)) { Log::warning(sprintf("ResourceServerContext::getCurrentUser user not found %s (%s).", $user_external_id, $user_email)); - return null; + $this->cachedCurrentUserResolved = true; + return $this->cachedCurrentUser = null; } - return $this->tx_service->transaction(function () use + $resolved = $this->tx_service->transaction(function () use ( $member, $user_email, @@ -268,6 +305,11 @@ public function getCurrentUser(bool $synch_groups = true, bool $update_member_fi MemberAssocSummitOrders::dispatch($member->getId()); return $synch_groups ? $this->checkGroups($member) : $member; }); + + $this->cachedCurrentUserResolved = true; + $this->groupsSynched = $synch_groups; + $this->fieldsSynched = $update_member_fields; + return $this->cachedCurrentUser = $resolved; } /** diff --git a/app/Repositories/Summit/DoctrineSummitEventRepository.php b/app/Repositories/Summit/DoctrineSummitEventRepository.php index 2963f39247..93498f5b89 100644 --- a/app/Repositories/Summit/DoctrineSummitEventRepository.php +++ b/app/Repositories/Summit/DoctrineSummitEventRepository.php @@ -712,11 +712,17 @@ public function getAllByPage(PagingInfo $paging_info, Filter $filter = null, Ord $ids = $this->getAllIdsByPage($paging_info, $filter, $order); Log::debug("DoctrineSummitEventRepository::getAllByPage ids", ['ids' => $ids]); $query = $this->getEntityManager()->createQueryBuilder() - ->select('e, p , et, et2') + ->select('e, p , et, et2, loc, cat') ->from($this->getBaseEntity(), "e") ->innerJoin("e.type", "et")->addSelect("et") ->leftJoin(PresentationType::class, 'et2', 'WITH', 'et.id = et2.id')->addSelect("et2") ->leftJoin(Presentation::class, 'p', 'WITH', 'e.id = p.id')->addSelect("p") + // Fetch-join location so the serializer's $event->getLocation() does not + // lazy-load per event (one query saved per event on /events listings). + // Doctrine's JOINED inheritance handles SummitVenueRoom etc. subclasses. + ->leftJoin('e.location', 'loc')->addSelect('loc') + // Fetch-join PresentationCategory (track) — was 5 queries per request. + ->leftJoin('p.category', 'cat')->addSelect('cat') ->where('e.id IN (:ids)') ->setParameter('ids', $ids); @@ -744,6 +750,130 @@ public function getAllByPage(PagingInfo $paging_info, Filter $filter = null, Ord if (isset($byId[$id])) $data[] = $byId[$id]; } + // Batch-preload Tag rows for every event on the page so the serializer's + // foreach (\$event->getTags()) does not lazy-load the collection per event. + // SELECT e, t fetch-joins tags onto each SummitEvent. + if (!empty($ids)) { + try { + $this->getEntityManager()->createQuery( + 'SELECT e, t FROM ' . SummitEvent::class . ' e ' . + 'LEFT JOIN e.tags t ' . + 'WHERE e.id IN (:ids)' + )->setParameter('ids', $ids)->getResult(); + } catch (\Exception $ex) { + Log::warning('DoctrineSummitEventRepository::getAllByPage tags preload failed', [ + 'error' => $ex->getMessage(), + ]); + } + + // Same pattern for Sponsors (ManyToMany Company on SummitEvent). + try { + $this->getEntityManager()->createQuery( + 'SELECT e, s FROM ' . SummitEvent::class . ' e ' . + 'LEFT JOIN e.sponsors s ' . + 'WHERE e.id IN (:ids)' + )->setParameter('ids', $ids)->getResult(); + } catch (\Exception $ex) { + Log::warning('DoctrineSummitEventRepository::getAllByPage sponsors preload failed', [ + 'error' => $ex->getMessage(), + ]); + } + } + + // Targeted N+1 preload for Presentation rows on this page: + // one DQL query that loads PresentationSpeakerAssignment + PresentationSpeaker + Member. + // After this, identity-map lookups by the serializer no longer trigger: + // - per-speaker Member SELECT (was 56 queries on a typical /events page) + // - per-speaker Presentation_Speakers composite-key SELECT (was 19) + // Net: ~75 queries collapsed into 1, ~216ms DB time saved per request. + // Only runs when there is at least one Presentation in the page. + $presentationIds = []; + foreach ($data as $event) { + if ($event instanceof Presentation) $presentationIds[] = $event->getId(); + } + if (!empty($presentationIds)) { + try { + $em = $this->getEntityManager(); + + // Batch-preload SummitSelectedPresentation rows for all presentations + // on this page. getSelectionStatus() on each Presentation then uses the + // preloaded data instead of firing its own DQL per presentation. + // Filters match getSelectionStatus()'s constants exactly. + try { + $selections = $em->createQuery( + 'SELECT sp FROM ' . SummitSelectedPresentation::class . ' sp ' . + 'JOIN FETCH sp.presentation p ' . + 'JOIN sp.list l ' . + '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; + } + foreach ($data as $event) { + if ($event instanceof Presentation && method_exists($event, 'setPreloadedSessionSelections')) { + $event->setPreloadedSessionSelections($byPresentation[$event->getId()] ?? []); + } + } + } catch (\Exception $ex) { + Log::warning('DoctrineSummitEventRepository::getAllByPage selection-status preload failed', [ + 'error' => $ex->getMessage(), + ]); + } + + $assignments = $em->createQuery( + 'SELECT a, s, m FROM ' . \App\Models\Foundation\Summit\Speakers\PresentationSpeakerAssignment::class . ' a ' . + 'JOIN a.speaker s ' . + 'LEFT JOIN s.member m ' . + 'WHERE a.presentation IN (:ids)' + )->setParameter('ids', $presentationIds)->getResult(); + + // Stuff each speaker's preloaded assignment-order cache so that + // PresentationSpeaker::getPresentationAssignmentOrder() does not + // fall back to an EXTRA_LAZY matching() query per (speaker,presentation) + // pair. The serializer calls this once per speaker per presentation. + foreach ($assignments as $a) { + $speaker = method_exists($a, 'getSpeaker') ? $a->getSpeaker() : null; + $presentation = method_exists($a, 'getPresentation') ? $a->getPresentation() : null; + if ($speaker && $presentation && method_exists($speaker, 'setPreloadedAssignmentOrder')) { + $speaker->setPreloadedAssignmentOrder($presentation->getId(), $a->getOrder()); + } + } + + // Batch-preload PresentationMaterial rows so the serializer's + // getMediaUploads/getSlides/getLinks/etc. iterates from memory instead + // of firing one query per presentation. Selecting both p and m makes + // this a fetch-join that populates p.materials with the rows. + try { + $em->createQuery( + 'SELECT p, m FROM ' . Presentation::class . ' p ' . + 'LEFT JOIN p.materials m ' . + 'WHERE p.id IN (:ids)' + )->setParameter('ids', $presentationIds)->getResult(); + } catch (\Exception $ex) { + Log::warning('DoctrineSummitEventRepository::getAllByPage materials preload failed', [ + 'error' => $ex->getMessage(), + ]); + } + + } catch (\Exception $ex) { + Log::warning('DoctrineSummitEventRepository::getAllByPage speaker+member preload failed', [ + 'error' => $ex->getMessage(), + ]); + } + } + if ($shuffleResults) shuffle($data); $end = time() - $start; diff --git a/config/doctrine.php b/config/doctrine.php index d139494f06..fb0048b581 100644 --- a/config/doctrine.php +++ b/config/doctrine.php @@ -84,6 +84,7 @@ */ 'middlewares' => array_filter([ env('DOCTRINE_LOGGING', false) ? Doctrine\DBAL\Logging\Middleware::class : null, + \App\Http\Middleware\Doctrine\QueryTimingMiddleware::class, ]), ], 'model' => [ @@ -150,6 +151,7 @@ */ 'middlewares' => array_filter([ env('DOCTRINE_LOGGING', false) ? Doctrine\DBAL\Logging\Middleware::class : null, + \App\Http\Middleware\Doctrine\QueryTimingMiddleware::class, ]), ], 'model_write' => [ @@ -216,6 +218,7 @@ */ 'middlewares' => array_filter([ env('DOCTRINE_LOGGING', false) ? Doctrine\DBAL\Logging\Middleware::class : null, + \App\Http\Middleware\Doctrine\QueryTimingMiddleware::class, ]), ] ], diff --git a/routes/api_v1.php b/routes/api_v1.php index fad69a60c4..db069b5a69 100644 --- a/routes/api_v1.php +++ b/routes/api_v1.php @@ -633,7 +633,7 @@ // events 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']); Route::group(['prefix' => 'csv'], function () { Route::get('', ['middleware' => 'auth.user', 'uses' => 'OAuth2SummitEventsApiController@getEventsCSV']); diff --git a/tests/PresentationSpeakerCacheTest.php b/tests/PresentationSpeakerCacheTest.php new file mode 100644 index 0000000000..e7cc0f68f3 --- /dev/null +++ b/tests/PresentationSpeakerCacheTest.php @@ -0,0 +1,143 @@ +createMock(Presentation::class); + $pres->method('getId')->willReturn(42); + + $speaker->setPreloadedAssignmentOrder(42, 3); + + $this->assertSame(3, $speaker->getPresentationAssignmentOrder($pres)); + } + + public function testPreloadedAssignmentOrderNullOrderCached(): void + { + $speaker = new PresentationSpeaker(); + $pres = $this->createMock(Presentation::class); + $pres->method('getId')->willReturn(10); + + $speaker->setPreloadedAssignmentOrder(10, null); + + $this->assertNull($speaker->getPresentationAssignmentOrder($pres)); + } + + // ------------------------------------------------------------------ // + // PresentationSpeaker::clearPreloadedAssignmentOrder // + // ------------------------------------------------------------------ // + + public function testClearPreloadedAssignmentOrderLeavesOtherEntriesIntact(): void + { + $speaker = new PresentationSpeaker(); + $speaker->setPreloadedAssignmentOrder(1, 5); + $speaker->setPreloadedAssignmentOrder(2, 7); + + $speaker->clearPreloadedAssignmentOrder(1); + + $pres2 = $this->createMock(Presentation::class); + $pres2->method('getId')->willReturn(2); + $this->assertSame(7, $speaker->getPresentationAssignmentOrder($pres2), + 'Clearing entry 1 must not affect entry 2'); + } + + public function testClearPreloadedAssignmentOrderAllowsReset(): void + { + $speaker = new PresentationSpeaker(); + $speaker->setPreloadedAssignmentOrder(1, 5); + $speaker->clearPreloadedAssignmentOrder(1); + $speaker->setPreloadedAssignmentOrder(1, 9); + + $pres = $this->createMock(Presentation::class); + $pres->method('getId')->willReturn(1); + $this->assertSame(9, $speaker->getPresentationAssignmentOrder($pres), + 'After clear + re-set, new value must be returned'); + } + + // ------------------------------------------------------------------ // + // PresentationSpeaker::clearAllPreloadedAssignmentOrders // + // ------------------------------------------------------------------ // + + public function testClearAllPreloadedAssignmentOrdersAllowsReset(): void + { + $speaker = new PresentationSpeaker(); + $speaker->setPreloadedAssignmentOrder(1, 2); + $speaker->setPreloadedAssignmentOrder(3, 4); + + $speaker->clearAllPreloadedAssignmentOrders(); + $speaker->setPreloadedAssignmentOrder(1, 10); + + $pres = $this->createMock(Presentation::class); + $pres->method('getId')->willReturn(1); + $this->assertSame(10, $speaker->getPresentationAssignmentOrder($pres)); + } + + // ------------------------------------------------------------------ // + // Presentation::setPreloadedSessionSelections / getSelectionStatus // + // ------------------------------------------------------------------ // + + /** + * Verifies the preloaded branch is exercised: with an empty preloaded + * array the method must return Pending without ever calling createQuery() + * (which would throw because there is no EntityManager in this context). + */ + public function testGetSelectionStatusUsesPreloadedBranchWhenSet(): void + { + $pres = new Presentation(); + $pres->setPreloadedSessionSelections([]); + + $status = $pres->getSelectionStatus(); + + $this->assertSame(Presentation::SelectionStatus_Pending, $status); + } + + public function testGetSelectionStatusMemoizesResult(): void + { + $pres = new Presentation(); + $pres->setPreloadedSessionSelections([]); + + $first = $pres->getSelectionStatus(); + $second = $pres->getSelectionStatus(); + + $this->assertSame($first, $second, 'Second call must return the memoized value'); + } + + public function testSetPreloadedSessionSelectionsResetsMemoization(): void + { + $pres = new Presentation(); + $pres->setPreloadedSessionSelections([]); + $pres->getSelectionStatus(); // fills memoized cache + + // Resetting preloaded selections must clear the memoized result so the + // next getSelectionStatus() re-evaluates instead of returning stale data. + $pres->setPreloadedSessionSelections([]); + // Must not throw — preloaded path is used again, not the DQL fallback. + $status = $pres->getSelectionStatus(); + $this->assertSame(Presentation::SelectionStatus_Pending, $status); + } +} diff --git a/tests/ResourceServerContextTest.php b/tests/ResourceServerContextTest.php index c1b68394ae..20b88b04b3 100644 --- a/tests/ResourceServerContextTest.php +++ b/tests/ResourceServerContextTest.php @@ -37,4 +37,33 @@ public function testSync(){ $member = $ctx->getCurrentUser(true); } + + public function testSetAuthorizationContextResetsUserCache(): void + { + $ctx = App::make(IResourceServerContext::class); + + $context = []; + $context['user_id'] = "1080"; + $context['external_user_id'] = "1080"; + $context['user_identifier'] = "test"; + $context['user_email'] = "test@test.com"; + $context['user_email_verified'] = true; + $context['user_first_name'] = "test"; + $context['user_last_name'] = "test"; + $context['user_groups'] = ['raw-users']; + $ctx->setAuthorizationContext($context); + $ctx->getCurrentUser(false); // warm the request-scoped cache + + $ref = new \ReflectionClass($ctx); + $prop = $ref->getProperty('cachedCurrentUserResolved'); + $prop->setAccessible(true); + $this->assertTrue($prop->getValue($ctx), + 'Prerequisite: cache must be warm after the first getCurrentUser() call'); + + // A second setAuthorizationContext() must invalidate the cache so the + // next getCurrentUser() re-fetches instead of returning the stale member. + $ctx->setAuthorizationContext($context); + $this->assertFalse($prop->getValue($ctx), + 'setAuthorizationContext() must reset cachedCurrentUserResolved'); + } } \ No newline at end of file