From 7da81b26d66e5a879f039d5048667dc2eb92f183 Mon Sep 17 00:00:00 2001 From: smarcet Date: Sat, 23 May 2026 01:04:56 -0300 Subject: [PATCH 01/27] fix(serializer): add last_edited timestamp to presentation cache key for automatic invalidation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../Summit/Presentation/PresentationSerializer.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) 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) From 34d39eba600ad63feec288fc5e6de8619dd78924 Mon Sep 17 00:00:00 2001 From: smarcet Date: Sat, 23 May 2026 13:37:25 -0300 Subject: [PATCH 02/27] chore(routes): enable server.timing.doctrine on getEvents for profiling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- routes/api_v1.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routes/api_v1.php b/routes/api_v1.php index fad69a60c4..f8eb5abe71 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' => ['auth.user', 'server.timing.doctrine'], 'uses' => 'OAuth2SummitEventsApiController@getEvents']); Route::group(['prefix' => 'csv'], function () { Route::get('', ['middleware' => 'auth.user', 'uses' => 'OAuth2SummitEventsApiController@getEventsCSV']); From 07882a0d910565ab62f01755ff40d38cb84f86e8 Mon Sep 17 00:00:00 2001 From: smarcet Date: Sat, 23 May 2026 14:52:56 -0300 Subject: [PATCH 03/27] chore(profiling): expose per-phase request breakdown via Server-Timing header MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../OAuth2SummitEventsApiController.php | 23 ++++++++------- app/Http/Middleware/ServerTimingDoctrine.php | 29 +++++++++++++++---- routes/api_v1.php | 2 +- 3 files changed, 37 insertions(+), 17 deletions(-) diff --git a/app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitEventsApiController.php b/app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitEventsApiController.php index b386bde853..e6dbb4816c 100644 --- a/app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitEventsApiController.php +++ b/app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitEventsApiController.php @@ -170,24 +170,27 @@ private function getSerializerType(): string */ public function getEvents($summit_id) { + \Illuminate\Support\Facades\Session::put('timing.controller_start', microtime(true)); return $this->processRequest(function () use ($summit_id) { $current_user = $this->resource_server_context->getCurrentUser(true); return $this->withReplica(function() use ($summit_id, $current_user) { $strategy = new RetrieveAllSummitEventsBySummitStrategy($this->repository, $this->event_repository, $this->resource_server_context); $response = $strategy->getEvents(['summit_id' => $summit_id]); - return $this->ok + \Illuminate\Support\Facades\Session::put('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() ); + \Illuminate\Support\Facades\Session::put('timing.serializer_end', microtime(true)); + $result = $this->ok($data); + \Illuminate\Support\Facades\Session::put('timing.controller_end', microtime(true)); + return $result; }); }); diff --git a/app/Http/Middleware/ServerTimingDoctrine.php b/app/Http/Middleware/ServerTimingDoctrine.php index ecdf4a49f2..a319593f1b 100644 --- a/app/Http/Middleware/ServerTimingDoctrine.php +++ b/app/Http/Middleware/ServerTimingDoctrine.php @@ -88,15 +88,32 @@ public function debug($m, array $c = []):void { $this->log('debug', $m, $c); } $response = $next($request); } - $totalMs = (microtime(true) - $start) * 1000.0; + $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); - $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; + + // Read controller-level timing markers (set by the controller method). + // If the controller didn't set them, these phases are reported as 0. + $cStart = Session::has("timing.controller_start") ? (float) Session::get("timing.controller_start") : null; + $cEnd = Session::has("timing.controller_end") ? (float) Session::get("timing.controller_end") : null; + $sStart = Session::has("timing.serializer_start") ? (float) Session::get("timing.serializer_start") : null; + $sEnd = Session::has("timing.serializer_end") ? (float) Session::get("timing.serializer_end") : null; + + $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; + + // Clear so they don't leak into a recycled worker's next request. + Session::forget(['timing.controller_start','timing.controller_end','timing.serializer_start','timing.serializer_end']); + $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,serializer;dur=%.1f,post;dur=%.1f,app;dur=%.1f,total;dur=%.1f', + $bootMs, $preMs, $controllerMs, $dbMs, $serializerMs, $postMs, $appMs, $totalMs + ) + ); $response->headers->set('Timing-Allow-Origin', '*'); return $response; diff --git a/routes/api_v1.php b/routes/api_v1.php index f8eb5abe71..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', 'server.timing.doctrine'], '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']); From d09ff466b9ea0988d6b1aa0c3fa7f808a1416335 Mon Sep 17 00:00:00 2001 From: smarcet Date: Sat, 23 May 2026 15:10:16 -0300 Subject: [PATCH 04/27] fix(profiling): accurate DB time measurement via DBAL Driver Middleware MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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'. --- .../Doctrine/QueryTimingCollector.php | 29 +++++++ .../Doctrine/QueryTimingMiddleware.php | 77 ++++++++++++++++ app/Http/Middleware/ServerTimingDoctrine.php | 87 +++---------------- config/doctrine.php | 3 + 4 files changed, 121 insertions(+), 75 deletions(-) create mode 100644 app/Http/Middleware/Doctrine/QueryTimingCollector.php create mode 100644 app/Http/Middleware/Doctrine/QueryTimingMiddleware.php 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); - - 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); - } - - // --- DBAL 3.x: Logging\Middleware (PSR-3) --- - } elseif (class_exists(\Doctrine\DBAL\Logging\Middleware::class) && method_exists($cfg, 'setMiddlewares')) { - - - $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])); + // 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. + \App\Http\Middleware\Doctrine\QueryTimingCollector::reset(); - try { - /** @var Response $response */ - $response = $next($request); - } finally { - $dbMs = $collector->dbMs; - $cfg->setMiddlewares($prev); - } + /** @var Response $response */ + $response = $next($request); - // --- Fallback - } else { - /** @var Response $response */ - $response = $next($request); - } + $dbMs = \App\Http\Middleware\Doctrine\QueryTimingCollector::$totalMs; + $dbCount = \App\Http\Middleware\Doctrine\QueryTimingCollector::$count; $end = microtime(true); $totalMs = ($end - $start) * 1000.0; @@ -110,8 +47,8 @@ public function debug($m, array $c = []):void { $this->log('debug', $m, $c); } $response->headers->set('Server-Timing', sprintf( - 'boot;dur=%.1f,pre;dur=%.1f,controller;dur=%.1f,db;dur=%.1f,serializer;dur=%.1f,post;dur=%.1f,app;dur=%.1f,total;dur=%.1f', - $bootMs, $preMs, $controllerMs, $dbMs, $serializerMs, $postMs, $appMs, $totalMs + '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', '*'); 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, ]), ] ], From 3c042976141787e0e071a2a2987e25f769b1a4c2 Mon Sep 17 00:00:00 2001 From: smarcet Date: Sat, 23 May 2026 15:31:50 -0300 Subject: [PATCH 05/27] chore(profiling): log top-repeating SQL patterns to identify exact N+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. --- .../Doctrine/QueryTimingCollector.php | 56 ++++++++++++++++++- .../Doctrine/QueryTimingMiddleware.php | 16 ++++-- app/Http/Middleware/ServerTimingDoctrine.php | 15 +++++ 3 files changed, 81 insertions(+), 6 deletions(-) diff --git a/app/Http/Middleware/Doctrine/QueryTimingCollector.php b/app/Http/Middleware/Doctrine/QueryTimingCollector.php index f20c70b866..9f34dabe34 100644 --- a/app/Http/Middleware/Doctrine/QueryTimingCollector.php +++ b/app/Http/Middleware/Doctrine/QueryTimingCollector.php @@ -15,15 +15,67 @@ class QueryTimingCollector public static float $totalMs = 0.0; public static int $count = 0; - public static function record(float $startedAt): void + /** @var array */ + public static array $patterns = []; + + public static function record(float $startedAt, ?string $sql = null): void { - self::$totalMs += (microtime(true) - $startedAt) * 1000.0; + $ms = (microtime(true) - $startedAt) * 1000.0; + self::$totalMs += $ms; self::$count++; + + if ($sql !== null) { + $pattern = self::normalize($sql); + if (!isset(self::$patterns[$pattern])) { + self::$patterns[$pattern] = ['count' => 0, 'totalMs' => 0.0, 'sample' => $sql]; + } + self::$patterns[$pattern]['count']++; + self::$patterns[$pattern]['totalMs'] += $ms; + } } public static function reset(): void { self::$totalMs = 0.0; self::$count = 0; + self::$patterns = []; + } + + /** + * Returns the top N most-repeated SQL patterns, sorted by count descending. + * + * @return array + */ + public static function topPatterns(int $limit = 10): array + { + $rows = []; + foreach (self::$patterns as $pattern => $stats) { + if ($stats['count'] < 2) continue; // only interested in repeats + $rows[] = [ + 'pattern' => $pattern, + 'count' => $stats['count'], + 'totalMs' => round($stats['totalMs'], 1), + 'sample' => $stats['sample'], + ]; + } + usort($rows, fn($a, $b) => $b['count'] <=> $a['count']); + return array_slice($rows, 0, $limit); + } + + /** + * Replace numeric and quoted literals with ? so that "SELECT x WHERE id = 7570" + * and "SELECT x WHERE id = 7571" map to the same pattern, surfacing N+1s. + */ + private static function normalize(string $sql): string + { + // Collapse positional and named params + $s = preg_replace('/\?|:[a-zA-Z_][a-zA-Z0-9_]*/', '?', $sql); + // Collapse quoted strings + $s = preg_replace("/'[^']*'/", '?', $s); + // Collapse numbers + $s = preg_replace('/\b\d+\b/', '?', $s); + // Collapse runs of whitespace + $s = preg_replace('/\s+/', ' ', $s); + return trim($s); } } diff --git a/app/Http/Middleware/Doctrine/QueryTimingMiddleware.php b/app/Http/Middleware/Doctrine/QueryTimingMiddleware.php index c0581efac8..101a27b310 100644 --- a/app/Http/Middleware/Doctrine/QueryTimingMiddleware.php +++ b/app/Http/Middleware/Doctrine/QueryTimingMiddleware.php @@ -43,7 +43,7 @@ public function query(string $sql): DBALResult try { return parent::query($sql); } finally { - QueryTimingCollector::record($start); + QueryTimingCollector::record($start, $sql); } } @@ -53,25 +53,33 @@ public function exec(string $sql): int try { return parent::exec($sql); } finally { - QueryTimingCollector::record($start); + QueryTimingCollector::record($start, $sql); } } public function prepare(string $sql): DBALStatement { - return new QueryTimingStatement(parent::prepare($sql)); + return new QueryTimingStatement(parent::prepare($sql), $sql); } } class QueryTimingStatement extends AbstractStatementMiddleware { + private string $sql; + + public function __construct($wrapped, string $sql) + { + parent::__construct($wrapped); + $this->sql = $sql; + } + public function execute($params = null): DBALResult { $start = microtime(true); try { return parent::execute($params); } finally { - QueryTimingCollector::record($start); + QueryTimingCollector::record($start, $this->sql); } } } diff --git a/app/Http/Middleware/ServerTimingDoctrine.php b/app/Http/Middleware/ServerTimingDoctrine.php index b22831dfcd..d55453e724 100644 --- a/app/Http/Middleware/ServerTimingDoctrine.php +++ b/app/Http/Middleware/ServerTimingDoctrine.php @@ -3,6 +3,7 @@ namespace App\Http\Middleware; use Closure; +use Illuminate\Support\Facades\Log; use Illuminate\Support\Facades\Session; use Symfony\Component\HttpFoundation\Response; @@ -53,6 +54,20 @@ public function handle($request, Closure $next): Response ); $response->headers->set('Timing-Allow-Origin', '*'); + // If this request fired N+1s, log the top repeating SQL patterns so we + // can target the actual lazy loads. Only logs when there are repeats + // (count >= 2) — typical normal requests stay quiet. + if ($dbCount >= 20) { + $top = \App\Http\Middleware\Doctrine\QueryTimingCollector::topPatterns(8); + foreach ($top as $row) { + Log::warning('N+1 candidate', [ + 'count' => $row['count'], + 'totalMs' => $row['totalMs'], + 'sample' => mb_substr($row['sample'], 0, 240), + ]); + } + } + return $response; } } From 9a9e23e1e98c7fd142b10d02b9fee7780f69d63e Mon Sep 17 00:00:00 2001 From: smarcet Date: Sat, 23 May 2026 15:39:20 -0300 Subject: [PATCH 06/27] fix(member): memoize belongsToGroup() per-instance to eliminate N+1 of 84 queries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- app/Models/Foundation/Main/Member.php | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) 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; } From c6fa4d38815fa4461fd25997c8342d611ff3dbc7 Mon Sep 17 00:00:00 2001 From: smarcet Date: Sat, 23 May 2026 15:48:26 -0300 Subject: [PATCH 07/27] fix(repository): preload PresentationSpeaker + Member in single DQL query MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../Summit/DoctrineSummitEventRepository.php | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/app/Repositories/Summit/DoctrineSummitEventRepository.php b/app/Repositories/Summit/DoctrineSummitEventRepository.php index 2963f39247..f0a57e67a6 100644 --- a/app/Repositories/Summit/DoctrineSummitEventRepository.php +++ b/app/Repositories/Summit/DoctrineSummitEventRepository.php @@ -744,6 +744,32 @@ public function getAllByPage(PagingInfo $paging_info, Filter $filter = null, Ord if (isset($byId[$id])) $data[] = $byId[$id]; } + // 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 { + $this->getEntityManager()->createQuery( + 'SELECT 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(); + } catch (\Exception $ex) { + Log::warning('DoctrineSummitEventRepository::getAllByPage speaker+member preload failed', [ + 'error' => $ex->getMessage(), + ]); + } + } + if ($shuffleResults) shuffle($data); $end = time() - $start; From b3ba4826d32ead511081959c5d27786fd2fd9244 Mon Sep 17 00:00:00 2001 From: smarcet Date: Sat, 23 May 2026 15:57:47 -0300 Subject: [PATCH 08/27] fix(repository): include root alias 'a' in speaker+member preload SELECT 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. --- app/Repositories/Summit/DoctrineSummitEventRepository.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/Repositories/Summit/DoctrineSummitEventRepository.php b/app/Repositories/Summit/DoctrineSummitEventRepository.php index f0a57e67a6..ae163e230c 100644 --- a/app/Repositories/Summit/DoctrineSummitEventRepository.php +++ b/app/Repositories/Summit/DoctrineSummitEventRepository.php @@ -758,7 +758,7 @@ public function getAllByPage(PagingInfo $paging_info, Filter $filter = null, Ord if (!empty($presentationIds)) { try { $this->getEntityManager()->createQuery( - 'SELECT s, m FROM ' . \App\Models\Foundation\Summit\Speakers\PresentationSpeakerAssignment::class . ' a ' . + '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)' From 098d513596e74e6be3c80f16742c25bdd708ff73 Mon Sep 17 00:00:00 2001 From: smarcet Date: Sat, 23 May 2026 16:04:44 -0300 Subject: [PATCH 09/27] chore(debug): instrument preload to confirm what it loads 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. --- .../Summit/DoctrineSummitEventRepository.php | 37 ++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/app/Repositories/Summit/DoctrineSummitEventRepository.php b/app/Repositories/Summit/DoctrineSummitEventRepository.php index ae163e230c..24bedbea93 100644 --- a/app/Repositories/Summit/DoctrineSummitEventRepository.php +++ b/app/Repositories/Summit/DoctrineSummitEventRepository.php @@ -757,12 +757,47 @@ public function getAllByPage(PagingInfo $paging_info, Filter $filter = null, Ord } if (!empty($presentationIds)) { try { - $this->getEntityManager()->createQuery( + $em = $this->getEntityManager(); + $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(); + + // Diagnostic: confirm what was actually loaded. + $speakerIds = []; + $memberIds = []; + $speakerInitialized = 0; + $memberInitialized = 0; + foreach ($assignments as $a) { + if (method_exists($a, 'getSpeaker')) { + $s = $a->getSpeaker(); + if ($s) { + $speakerIds[$s->getId()] = true; + if (!($s instanceof \Doctrine\Persistence\Proxy) || $s->__isInitialized()) { + $speakerInitialized++; + } + if (method_exists($s, 'getMember')) { + $m = $s->getMember(); + if ($m) { + $memberIds[$m->getId()] = true; + if (!($m instanceof \Doctrine\Persistence\Proxy) || $m->__isInitialized()) { + $memberInitialized++; + } + } + } + } + } + } + Log::warning('preload diagnostic', [ + 'presentationIds' => count($presentationIds), + 'assignmentsLoaded' => count($assignments), + 'uniqueSpeakers' => count($speakerIds), + 'speakersInitialized'=> $speakerInitialized, + 'uniqueMembers' => count($memberIds), + 'membersInitialized' => $memberInitialized, + ]); } catch (\Exception $ex) { Log::warning('DoctrineSummitEventRepository::getAllByPage speaker+member preload failed', [ 'error' => $ex->getMessage(), From 92596db198f678dfdb939f1669cb4222235a208a Mon Sep 17 00:00:00 2001 From: smarcet Date: Sat, 23 May 2026 18:27:42 -0300 Subject: [PATCH 10/27] chore(debug): capture Member query params to identify which Members are 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/QueryTimingCollector.php | 18 +++++++++++++++++- .../Doctrine/QueryTimingMiddleware.php | 11 ++++++++++- app/Http/Middleware/ServerTimingDoctrine.php | 6 ++++++ 3 files changed, 33 insertions(+), 2 deletions(-) diff --git a/app/Http/Middleware/Doctrine/QueryTimingCollector.php b/app/Http/Middleware/Doctrine/QueryTimingCollector.php index 9f34dabe34..eabdd0bb0f 100644 --- a/app/Http/Middleware/Doctrine/QueryTimingCollector.php +++ b/app/Http/Middleware/Doctrine/QueryTimingCollector.php @@ -18,7 +18,15 @@ class QueryTimingCollector /** @var array */ public static array $patterns = []; - public static function record(float $startedAt, ?string $sql = null): void + /** + * Sample of full SQL strings (with their bound values where possible). + * Capped at a reasonable size to avoid memory issues. + * + * @var array + */ + public static array $memberQueries = []; + + public static function record(float $startedAt, ?string $sql = null, ?array $params = null): void { $ms = (microtime(true) - $startedAt) * 1000.0; self::$totalMs += $ms; @@ -31,6 +39,13 @@ public static function record(float $startedAt, ?string $sql = null): void } self::$patterns[$pattern]['count']++; self::$patterns[$pattern]['totalMs'] += $ms; + + // Capture FROM-Member SELECT queries so we can see exactly which + // Member IDs are being loaded and from which code path. + if (count(self::$memberQueries) < 100 && stripos($sql, 'FROM Member') !== false) { + $paramsStr = $params ? '[' . implode(',', array_map(fn($v) => is_scalar($v) ? (string)$v : gettype($v), $params)) . ']' : ''; + self::$memberQueries[] = $paramsStr ?: 'no-params'; + } } } @@ -39,6 +54,7 @@ public static function reset(): void self::$totalMs = 0.0; self::$count = 0; self::$patterns = []; + self::$memberQueries = []; } /** diff --git a/app/Http/Middleware/Doctrine/QueryTimingMiddleware.php b/app/Http/Middleware/Doctrine/QueryTimingMiddleware.php index 101a27b310..d8c864fc8d 100644 --- a/app/Http/Middleware/Doctrine/QueryTimingMiddleware.php +++ b/app/Http/Middleware/Doctrine/QueryTimingMiddleware.php @@ -73,13 +73,22 @@ public function __construct($wrapped, string $sql) $this->sql = $sql; } + private array $boundParams = []; + + public function bindValue($param, $value, $type = \Doctrine\DBAL\ParameterType::STRING): bool + { + $this->boundParams[$param] = $value; + return parent::bindValue($param, $value, $type); + } + public function execute($params = null): DBALResult { $start = microtime(true); try { return parent::execute($params); } finally { - QueryTimingCollector::record($start, $this->sql); + $effective = $params ?? $this->boundParams; + QueryTimingCollector::record($start, $this->sql, is_array($effective) ? $effective : null); } } } diff --git a/app/Http/Middleware/ServerTimingDoctrine.php b/app/Http/Middleware/ServerTimingDoctrine.php index d55453e724..f86fad7222 100644 --- a/app/Http/Middleware/ServerTimingDoctrine.php +++ b/app/Http/Middleware/ServerTimingDoctrine.php @@ -66,6 +66,12 @@ public function handle($request, Closure $next): Response 'sample' => mb_substr($row['sample'], 0, 240), ]); } + // Dump Member query params so we can see which IDs are being loaded + // and correlate to the code path (created_by, updated_by, speaker.member, ...). + $memberQueries = \App\Http\Middleware\Doctrine\QueryTimingCollector::$memberQueries; + if (!empty($memberQueries)) { + Log::warning('member queries', ['count' => count($memberQueries), 'params' => $memberQueries]); + } } return $response; From 0aab8d16938a3bdfa80b7e1d24fe2013026b4436 Mon Sep 17 00:00:00 2001 From: smarcet Date: Sat, 23 May 2026 18:57:48 -0300 Subject: [PATCH 11/27] fix(debug): strip backticks before matching Member table name Doctrine wraps table identifiers in backticks (FROM `Member` t0) so the plain 'FROM Member' check missed every query. Strip backticks first. --- app/Http/Middleware/Doctrine/QueryTimingCollector.php | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/app/Http/Middleware/Doctrine/QueryTimingCollector.php b/app/Http/Middleware/Doctrine/QueryTimingCollector.php index eabdd0bb0f..2c6ba4f38d 100644 --- a/app/Http/Middleware/Doctrine/QueryTimingCollector.php +++ b/app/Http/Middleware/Doctrine/QueryTimingCollector.php @@ -42,9 +42,13 @@ public static function record(float $startedAt, ?string $sql = null, ?array $par // Capture FROM-Member SELECT queries so we can see exactly which // Member IDs are being loaded and from which code path. - if (count(self::$memberQueries) < 100 && stripos($sql, 'FROM Member') !== false) { - $paramsStr = $params ? '[' . implode(',', array_map(fn($v) => is_scalar($v) ? (string)$v : gettype($v), $params)) . ']' : ''; - self::$memberQueries[] = $paramsStr ?: 'no-params'; + // Doctrine wraps identifiers in backticks, so strip them before matching. + if (count(self::$memberQueries) < 100) { + $stripped = str_replace('`', '', $sql); + if (stripos($stripped, 'FROM Member') !== false) { + $paramsStr = $params ? '[' . implode(',', array_map(fn($v) => is_scalar($v) ? (string)$v : gettype($v), $params)) . ']' : ''; + self::$memberQueries[] = $paramsStr ?: 'no-params'; + } } } } From 9883ff4d3d2497ebacad9afcf35483d66f60310c Mon Sep 17 00:00:00 2001 From: smarcet Date: Sat, 23 May 2026 19:48:39 -0300 Subject: [PATCH 12/27] =?UTF-8?q?fix(auth):=20memoize=20getCurrentUser()?= =?UTF-8?q?=20=E2=80=94=20eliminates=20~98=20redundant=20Member=20SELECTs?= =?UTF-8?q?=20per=20request?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- app/Models/OAuth2/ResourceServerContext.php | 29 ++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/app/Models/OAuth2/ResourceServerContext.php b/app/Models/OAuth2/ResourceServerContext.php index bfc46a25df..d4ad3fd90b 100644 --- a/app/Models/OAuth2/ResourceServerContext.php +++ b/app/Models/OAuth2/ResourceServerContext.php @@ -159,6 +159,20 @@ public function updateAuthContextVar(string $varName, $value):void { } } + /** + * 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(). + * + * Caches the resolved Member regardless of \$synch_groups / \$update_member_fields + * arguments — the side-effects (group sync, event dispatch, field updates) + * are idempotent per request and only need to run once. + */ + private ?Member $cachedCurrentUser = null; + private bool $cachedCurrentUserResolved = false; + /** * @param bool $synch_groups * @param bool $update_member_fields @@ -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; + } + $member = null; // try to get by external id $user_external_id = $this->getAuthContextVar(IResourceServerContext::UserId); @@ -176,7 +194,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 +250,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 +288,9 @@ 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; + return $this->cachedCurrentUser = $resolved; } /** From 50432315d8b840c49892aa8ac5abec00547807df Mon Sep 17 00:00:00 2001 From: smarcet Date: Sat, 23 May 2026 20:00:48 -0300 Subject: [PATCH 13/27] fix(repository): batch-preload SummitSelectedPresentation, eliminates 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. --- .../Events/Presentations/Presentation.php | 66 ++++++++++++++----- .../Summit/DoctrineSummitEventRepository.php | 38 +++++++++++ 2 files changed, 86 insertions(+), 18 deletions(-) diff --git a/app/Models/Foundation/Summit/Events/Presentations/Presentation.php b/app/Models/Foundation/Summit/Events/Presentations/Presentation.php index 91ce9ae739..26ff8efd3e 100644 --- a/app/Models/Foundation/Summit/Events/Presentations/Presentation.php +++ b/app/Models/Foundation/Summit/Events/Presentations/Presentation.php @@ -950,20 +950,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 +1001,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 +1023,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 diff --git a/app/Repositories/Summit/DoctrineSummitEventRepository.php b/app/Repositories/Summit/DoctrineSummitEventRepository.php index 24bedbea93..7ea3d6eb19 100644 --- a/app/Repositories/Summit/DoctrineSummitEventRepository.php +++ b/app/Repositories/Summit/DoctrineSummitEventRepository.php @@ -758,6 +758,44 @@ public function getAllByPage(PagingInfo $paging_info, Filter $filter = null, Ord 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, 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; + } + 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 ' . From e49226ae2d58e2b2eb5abe06f6a8e8ecdc20b990 Mon Sep 17 00:00:00 2001 From: smarcet Date: Sun, 24 May 2026 21:45:25 -0300 Subject: [PATCH 14/27] chore(debug): instrument SummitSelectedPresentation preload to find why 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). --- .../Summit/Events/Presentations/Presentation.php | 7 ++++++- .../Summit/DoctrineSummitEventRepository.php | 12 ++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/app/Models/Foundation/Summit/Events/Presentations/Presentation.php b/app/Models/Foundation/Summit/Events/Presentations/Presentation.php index 26ff8efd3e..df5780b038 100644 --- a/app/Models/Foundation/Summit/Events/Presentations/Presentation.php +++ b/app/Models/Foundation/Summit/Events/Presentations/Presentation.php @@ -979,9 +979,14 @@ public function getSelectionStatus() return $this->memoizedSelectionStatus; } - if ($this->preloadedSessionSelections !== null) { + $preloadIsSet = isset($this->preloadedSessionSelections); + if ($preloadIsSet) { $session_sel = $this->preloadedSessionSelections; } else { + \Illuminate\Support\Facades\Log::warning('getSelectionStatus cache MISS', [ + 'pid' => $this->id, + 'class' => get_class($this), + ]); $session_sel = $this->createQuery("SELECT sp from models\summit\SummitSelectedPresentation sp JOIN sp.list l JOIN sp.presentation p diff --git a/app/Repositories/Summit/DoctrineSummitEventRepository.php b/app/Repositories/Summit/DoctrineSummitEventRepository.php index 7ea3d6eb19..981f254581 100644 --- a/app/Repositories/Summit/DoctrineSummitEventRepository.php +++ b/app/Repositories/Summit/DoctrineSummitEventRepository.php @@ -785,11 +785,23 @@ public function getAllByPage(PagingInfo $paging_info, Filter $filter = null, Ord $pid = $sp->getPresentation()->getId(); $byPresentation[$pid][] = $sp; } + $fed = 0; + $skipped = 0; foreach ($data as $event) { if ($event instanceof Presentation && method_exists($event, 'setPreloadedSessionSelections')) { $event->setPreloadedSessionSelections($byPresentation[$event->getId()] ?? []); + $fed++; + } else { + $skipped++; } } + Log::warning('selection-status preload diagnostic', [ + 'presentationIds' => count($presentationIds), + 'selectionsLoaded' => count($selections), + 'fed' => $fed, + 'skipped' => $skipped, + 'eventClasses' => array_map(fn($e) => get_class($e), array_slice($data, 0, 3)), + ]); } catch (\Exception $ex) { Log::warning('DoctrineSummitEventRepository::getAllByPage selection-status preload failed', [ 'error' => $ex->getMessage(), From bec63ec642ca807460b615a72961d4509d3ec0da Mon Sep 17 00:00:00 2001 From: smarcet Date: Sun, 24 May 2026 22:11:36 -0300 Subject: [PATCH 15/27] fix(speaker): pre-populate getPresentationAssignmentOrder cache from batch-loaded assignments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../Summit/Speakers/PresentationSpeaker.php | 20 +++++++++++++++++++ .../Summit/DoctrineSummitEventRepository.php | 12 +++++++++++ 2 files changed, 32 insertions(+) diff --git a/app/Models/Foundation/Summit/Speakers/PresentationSpeaker.php b/app/Models/Foundation/Summit/Speakers/PresentationSpeaker.php index 84fa700b78..8cbda63801 100644 --- a/app/Models/Foundation/Summit/Speakers/PresentationSpeaker.php +++ b/app/Models/Foundation/Summit/Speakers/PresentationSpeaker.php @@ -1659,12 +1659,32 @@ 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; + } + /** * @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/Repositories/Summit/DoctrineSummitEventRepository.php b/app/Repositories/Summit/DoctrineSummitEventRepository.php index 981f254581..c5edcfe4cb 100644 --- a/app/Repositories/Summit/DoctrineSummitEventRepository.php +++ b/app/Repositories/Summit/DoctrineSummitEventRepository.php @@ -815,6 +815,18 @@ public function getAllByPage(PagingInfo $paging_info, Filter $filter = null, Ord '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()); + } + } + // Diagnostic: confirm what was actually loaded. $speakerIds = []; $memberIds = []; From 9a4001517d644859fa14bc8cfca4594c423d401a Mon Sep 17 00:00:00 2001 From: smarcet Date: Sun, 24 May 2026 22:21:33 -0300 Subject: [PATCH 16/27] fix(repository): preload location (FETCH JOIN), tags and materials in /events hydration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../Summit/DoctrineSummitEventRepository.php | 39 ++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/app/Repositories/Summit/DoctrineSummitEventRepository.php b/app/Repositories/Summit/DoctrineSummitEventRepository.php index c5edcfe4cb..16e302eb8b 100644 --- a/app/Repositories/Summit/DoctrineSummitEventRepository.php +++ b/app/Repositories/Summit/DoctrineSummitEventRepository.php @@ -712,11 +712,15 @@ 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') ->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') ->where('e.id IN (:ids)') ->setParameter('ids', $ids); @@ -744,6 +748,23 @@ 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(), + ]); + } + } + // 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: @@ -827,6 +848,22 @@ public function getAllByPage(PagingInfo $paging_info, Filter $filter = null, Ord } } + // 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(), + ]); + } + // Diagnostic: confirm what was actually loaded. $speakerIds = []; $memberIds = []; From 1376db2586a07ade03b531db8158986e897fe21d Mon Sep 17 00:00:00 2001 From: smarcet Date: Sun, 24 May 2026 22:31:15 -0300 Subject: [PATCH 17/27] fix(repository): preload sponsors batch + fetch-join PresentationCategory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../Summit/DoctrineSummitEventRepository.php | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/app/Repositories/Summit/DoctrineSummitEventRepository.php b/app/Repositories/Summit/DoctrineSummitEventRepository.php index 16e302eb8b..6b64bb3337 100644 --- a/app/Repositories/Summit/DoctrineSummitEventRepository.php +++ b/app/Repositories/Summit/DoctrineSummitEventRepository.php @@ -712,7 +712,7 @@ 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, loc') + ->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") @@ -721,6 +721,8 @@ public function getAllByPage(PagingInfo $paging_info, Filter $filter = null, Ord // 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); @@ -763,6 +765,19 @@ public function getAllByPage(PagingInfo $paging_info, Filter $filter = null, Ord '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: From e919d1c0a89ef49eb0f366358d7240e5ad03a2bd Mon Sep 17 00:00:00 2001 From: smarcet Date: Sun, 24 May 2026 22:45:26 -0300 Subject: [PATCH 18/27] chore(cleanup): remove diagnostic logging used to profile the /events 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. --- .../Doctrine/QueryTimingCollector.php | 76 +------------------ .../Doctrine/QueryTimingMiddleware.php | 25 +----- app/Http/Middleware/ServerTimingDoctrine.php | 21 ----- .../Events/Presentations/Presentation.php | 7 +- .../Summit/DoctrineSummitEventRepository.php | 45 ----------- 5 files changed, 7 insertions(+), 167 deletions(-) diff --git a/app/Http/Middleware/Doctrine/QueryTimingCollector.php b/app/Http/Middleware/Doctrine/QueryTimingCollector.php index 2c6ba4f38d..f20c70b866 100644 --- a/app/Http/Middleware/Doctrine/QueryTimingCollector.php +++ b/app/Http/Middleware/Doctrine/QueryTimingCollector.php @@ -15,87 +15,15 @@ class QueryTimingCollector public static float $totalMs = 0.0; public static int $count = 0; - /** @var array */ - public static array $patterns = []; - - /** - * Sample of full SQL strings (with their bound values where possible). - * Capped at a reasonable size to avoid memory issues. - * - * @var array - */ - public static array $memberQueries = []; - - public static function record(float $startedAt, ?string $sql = null, ?array $params = null): void + public static function record(float $startedAt): void { - $ms = (microtime(true) - $startedAt) * 1000.0; - self::$totalMs += $ms; + self::$totalMs += (microtime(true) - $startedAt) * 1000.0; self::$count++; - - if ($sql !== null) { - $pattern = self::normalize($sql); - if (!isset(self::$patterns[$pattern])) { - self::$patterns[$pattern] = ['count' => 0, 'totalMs' => 0.0, 'sample' => $sql]; - } - self::$patterns[$pattern]['count']++; - self::$patterns[$pattern]['totalMs'] += $ms; - - // Capture FROM-Member SELECT queries so we can see exactly which - // Member IDs are being loaded and from which code path. - // Doctrine wraps identifiers in backticks, so strip them before matching. - if (count(self::$memberQueries) < 100) { - $stripped = str_replace('`', '', $sql); - if (stripos($stripped, 'FROM Member') !== false) { - $paramsStr = $params ? '[' . implode(',', array_map(fn($v) => is_scalar($v) ? (string)$v : gettype($v), $params)) . ']' : ''; - self::$memberQueries[] = $paramsStr ?: 'no-params'; - } - } - } } public static function reset(): void { self::$totalMs = 0.0; self::$count = 0; - self::$patterns = []; - self::$memberQueries = []; - } - - /** - * Returns the top N most-repeated SQL patterns, sorted by count descending. - * - * @return array - */ - public static function topPatterns(int $limit = 10): array - { - $rows = []; - foreach (self::$patterns as $pattern => $stats) { - if ($stats['count'] < 2) continue; // only interested in repeats - $rows[] = [ - 'pattern' => $pattern, - 'count' => $stats['count'], - 'totalMs' => round($stats['totalMs'], 1), - 'sample' => $stats['sample'], - ]; - } - usort($rows, fn($a, $b) => $b['count'] <=> $a['count']); - return array_slice($rows, 0, $limit); - } - - /** - * Replace numeric and quoted literals with ? so that "SELECT x WHERE id = 7570" - * and "SELECT x WHERE id = 7571" map to the same pattern, surfacing N+1s. - */ - private static function normalize(string $sql): string - { - // Collapse positional and named params - $s = preg_replace('/\?|:[a-zA-Z_][a-zA-Z0-9_]*/', '?', $sql); - // Collapse quoted strings - $s = preg_replace("/'[^']*'/", '?', $s); - // Collapse numbers - $s = preg_replace('/\b\d+\b/', '?', $s); - // Collapse runs of whitespace - $s = preg_replace('/\s+/', ' ', $s); - return trim($s); } } diff --git a/app/Http/Middleware/Doctrine/QueryTimingMiddleware.php b/app/Http/Middleware/Doctrine/QueryTimingMiddleware.php index d8c864fc8d..c0581efac8 100644 --- a/app/Http/Middleware/Doctrine/QueryTimingMiddleware.php +++ b/app/Http/Middleware/Doctrine/QueryTimingMiddleware.php @@ -43,7 +43,7 @@ public function query(string $sql): DBALResult try { return parent::query($sql); } finally { - QueryTimingCollector::record($start, $sql); + QueryTimingCollector::record($start); } } @@ -53,42 +53,25 @@ public function exec(string $sql): int try { return parent::exec($sql); } finally { - QueryTimingCollector::record($start, $sql); + QueryTimingCollector::record($start); } } public function prepare(string $sql): DBALStatement { - return new QueryTimingStatement(parent::prepare($sql), $sql); + return new QueryTimingStatement(parent::prepare($sql)); } } class QueryTimingStatement extends AbstractStatementMiddleware { - private string $sql; - - public function __construct($wrapped, string $sql) - { - parent::__construct($wrapped); - $this->sql = $sql; - } - - private array $boundParams = []; - - public function bindValue($param, $value, $type = \Doctrine\DBAL\ParameterType::STRING): bool - { - $this->boundParams[$param] = $value; - return parent::bindValue($param, $value, $type); - } - public function execute($params = null): DBALResult { $start = microtime(true); try { return parent::execute($params); } finally { - $effective = $params ?? $this->boundParams; - QueryTimingCollector::record($start, $this->sql, is_array($effective) ? $effective : null); + QueryTimingCollector::record($start); } } } diff --git a/app/Http/Middleware/ServerTimingDoctrine.php b/app/Http/Middleware/ServerTimingDoctrine.php index f86fad7222..b22831dfcd 100644 --- a/app/Http/Middleware/ServerTimingDoctrine.php +++ b/app/Http/Middleware/ServerTimingDoctrine.php @@ -3,7 +3,6 @@ namespace App\Http\Middleware; use Closure; -use Illuminate\Support\Facades\Log; use Illuminate\Support\Facades\Session; use Symfony\Component\HttpFoundation\Response; @@ -54,26 +53,6 @@ public function handle($request, Closure $next): Response ); $response->headers->set('Timing-Allow-Origin', '*'); - // If this request fired N+1s, log the top repeating SQL patterns so we - // can target the actual lazy loads. Only logs when there are repeats - // (count >= 2) — typical normal requests stay quiet. - if ($dbCount >= 20) { - $top = \App\Http\Middleware\Doctrine\QueryTimingCollector::topPatterns(8); - foreach ($top as $row) { - Log::warning('N+1 candidate', [ - 'count' => $row['count'], - 'totalMs' => $row['totalMs'], - 'sample' => mb_substr($row['sample'], 0, 240), - ]); - } - // Dump Member query params so we can see which IDs are being loaded - // and correlate to the code path (created_by, updated_by, speaker.member, ...). - $memberQueries = \App\Http\Middleware\Doctrine\QueryTimingCollector::$memberQueries; - if (!empty($memberQueries)) { - Log::warning('member queries', ['count' => count($memberQueries), 'params' => $memberQueries]); - } - } - return $response; } } diff --git a/app/Models/Foundation/Summit/Events/Presentations/Presentation.php b/app/Models/Foundation/Summit/Events/Presentations/Presentation.php index df5780b038..26ff8efd3e 100644 --- a/app/Models/Foundation/Summit/Events/Presentations/Presentation.php +++ b/app/Models/Foundation/Summit/Events/Presentations/Presentation.php @@ -979,14 +979,9 @@ public function getSelectionStatus() return $this->memoizedSelectionStatus; } - $preloadIsSet = isset($this->preloadedSessionSelections); - if ($preloadIsSet) { + if ($this->preloadedSessionSelections !== null) { $session_sel = $this->preloadedSessionSelections; } else { - \Illuminate\Support\Facades\Log::warning('getSelectionStatus cache MISS', [ - 'pid' => $this->id, - 'class' => get_class($this), - ]); $session_sel = $this->createQuery("SELECT sp from models\summit\SummitSelectedPresentation sp JOIN sp.list l JOIN sp.presentation p diff --git a/app/Repositories/Summit/DoctrineSummitEventRepository.php b/app/Repositories/Summit/DoctrineSummitEventRepository.php index 6b64bb3337..42cfc70971 100644 --- a/app/Repositories/Summit/DoctrineSummitEventRepository.php +++ b/app/Repositories/Summit/DoctrineSummitEventRepository.php @@ -821,23 +821,11 @@ public function getAllByPage(PagingInfo $paging_info, Filter $filter = null, Ord $pid = $sp->getPresentation()->getId(); $byPresentation[$pid][] = $sp; } - $fed = 0; - $skipped = 0; foreach ($data as $event) { if ($event instanceof Presentation && method_exists($event, 'setPreloadedSessionSelections')) { $event->setPreloadedSessionSelections($byPresentation[$event->getId()] ?? []); - $fed++; - } else { - $skipped++; } } - Log::warning('selection-status preload diagnostic', [ - 'presentationIds' => count($presentationIds), - 'selectionsLoaded' => count($selections), - 'fed' => $fed, - 'skipped' => $skipped, - 'eventClasses' => array_map(fn($e) => get_class($e), array_slice($data, 0, 3)), - ]); } catch (\Exception $ex) { Log::warning('DoctrineSummitEventRepository::getAllByPage selection-status preload failed', [ 'error' => $ex->getMessage(), @@ -879,39 +867,6 @@ public function getAllByPage(PagingInfo $paging_info, Filter $filter = null, Ord ]); } - // Diagnostic: confirm what was actually loaded. - $speakerIds = []; - $memberIds = []; - $speakerInitialized = 0; - $memberInitialized = 0; - foreach ($assignments as $a) { - if (method_exists($a, 'getSpeaker')) { - $s = $a->getSpeaker(); - if ($s) { - $speakerIds[$s->getId()] = true; - if (!($s instanceof \Doctrine\Persistence\Proxy) || $s->__isInitialized()) { - $speakerInitialized++; - } - if (method_exists($s, 'getMember')) { - $m = $s->getMember(); - if ($m) { - $memberIds[$m->getId()] = true; - if (!($m instanceof \Doctrine\Persistence\Proxy) || $m->__isInitialized()) { - $memberInitialized++; - } - } - } - } - } - } - Log::warning('preload diagnostic', [ - 'presentationIds' => count($presentationIds), - 'assignmentsLoaded' => count($assignments), - 'uniqueSpeakers' => count($speakerIds), - 'speakersInitialized'=> $speakerInitialized, - 'uniqueMembers' => count($memberIds), - 'membersInitialized' => $memberInitialized, - ]); } catch (\Exception $ex) { Log::warning('DoctrineSummitEventRepository::getAllByPage speaker+member preload failed', [ 'error' => $ex->getMessage(), From f6d3997b0aeb3fccd4fe7596615254430edd63bb Mon Sep 17 00:00:00 2001 From: smarcet Date: Sun, 24 May 2026 22:48:27 -0300 Subject: [PATCH 19/27] docs(adr): document /events N+1 elimination work 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. --- ...02-events-endpoint-n-plus-1-elimination.md | 380 ++++++++++++++++++ 1 file changed, 380 insertions(+) create mode 100644 adr/002-events-endpoint-n-plus-1-elimination.md 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..6bb2938e40 --- /dev/null +++ b/adr/002-events-endpoint-n-plus-1-elimination.md @@ -0,0 +1,380 @@ +# 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` + +## 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. From 56610bb31cc459f1ac32c3b8a81bf18d60c429c4 Mon Sep 17 00:00:00 2001 From: smarcet Date: Sun, 24 May 2026 23:22:01 -0300 Subject: [PATCH 20/27] fix(events): correct SummitSelectedPresentation preload DQL and cache 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. --- app/Models/OAuth2/ResourceServerContext.php | 4 ++++ app/Repositories/Summit/DoctrineSummitEventRepository.php | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/app/Models/OAuth2/ResourceServerContext.php b/app/Models/OAuth2/ResourceServerContext.php index d4ad3fd90b..e291c3e0d9 100644 --- a/app/Models/OAuth2/ResourceServerContext.php +++ b/app/Models/OAuth2/ResourceServerContext.php @@ -126,6 +126,8 @@ public function getCurrentUserId() public function setAuthorizationContext(array $auth_context) { $this->auth_context = $auth_context; + $this->cachedCurrentUser = null; + $this->cachedCurrentUserResolved = false; } /** @@ -156,6 +158,8 @@ 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; } } diff --git a/app/Repositories/Summit/DoctrineSummitEventRepository.php b/app/Repositories/Summit/DoctrineSummitEventRepository.php index 42cfc70971..93498f5b89 100644 --- a/app/Repositories/Summit/DoctrineSummitEventRepository.php +++ b/app/Repositories/Summit/DoctrineSummitEventRepository.php @@ -801,9 +801,9 @@ public function getAllByPage(PagingInfo $paging_info, Filter $filter = null, Ord // Filters match getSelectionStatus()'s constants exactly. try { $selections = $em->createQuery( - 'SELECT sp, p FROM ' . SummitSelectedPresentation::class . ' sp ' . + 'SELECT sp FROM ' . SummitSelectedPresentation::class . ' sp ' . + 'JOIN FETCH sp.presentation p ' . 'JOIN sp.list l ' . - 'JOIN sp.presentation p ' . 'WHERE p.id IN (:ids) ' . 'AND sp.collection = :collection ' . 'AND l.list_type = :list_type ' . From 1506fd1b8bbbb718940f6d561f1358ff33148610 Mon Sep 17 00:00:00 2001 From: smarcet Date: Sun, 24 May 2026 23:31:47 -0300 Subject: [PATCH 21/27] fix(events): low-severity findings + unit tests from PR #549 review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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() --- ...02-events-endpoint-n-plus-1-elimination.md | 2 +- .../Summit/Speakers/PresentationSpeaker.php | 10 ++ tests/PresentationSpeakerCacheTest.php | 143 ++++++++++++++++++ tests/ResourceServerContextTest.php | 29 ++++ 4 files changed, 183 insertions(+), 1 deletion(-) create mode 100644 tests/PresentationSpeakerCacheTest.php diff --git a/adr/002-events-endpoint-n-plus-1-elimination.md b/adr/002-events-endpoint-n-plus-1-elimination.md index 6bb2938e40..82deff69e4 100644 --- a/adr/002-events-endpoint-n-plus-1-elimination.md +++ b/adr/002-events-endpoint-n-plus-1-elimination.md @@ -1,4 +1,4 @@ -# ADR-0001: Eliminate N+1 Queries in `/events` Endpoint +# ADR-0002: Eliminate N+1 Queries in `/events` Endpoint - **Status:** Accepted - **Date:** 2026-05-25 diff --git a/app/Models/Foundation/Summit/Speakers/PresentationSpeaker.php b/app/Models/Foundation/Summit/Speakers/PresentationSpeaker.php index 8cbda63801..8d460c052b 100644 --- a/app/Models/Foundation/Summit/Speakers/PresentationSpeaker.php +++ b/app/Models/Foundation/Summit/Speakers/PresentationSpeaker.php @@ -1675,6 +1675,16 @@ public function setPreloadedAssignmentOrder(int $presentationId, ?int $order): v $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 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 From ad726c415518c2d50a3643bbe6c893a8fb803094 Mon Sep 17 00:00:00 2001 From: smarcet Date: Sun, 24 May 2026 23:40:40 -0300 Subject: [PATCH 22/27] fix(trait): restore optional afterQuery hook in _getAll 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. --- .../Protected/Summit/Traits/ParametrizedGetAll.php | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) 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 From 0707b346645e797fc1c68cc6d91eb3498409234e Mon Sep 17 00:00:00 2001 From: smarcet Date: Mon, 25 May 2026 21:00:27 -0300 Subject: [PATCH 23/27] fix(timing): replace Session with request attributes for sub-phase timings 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. --- .../Summit/OAuth2SummitEventsApiController.php | 13 +++++++------ app/Http/Middleware/ServerTimingDoctrine.php | 15 ++++++--------- 2 files changed, 13 insertions(+), 15 deletions(-) diff --git a/app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitEventsApiController.php b/app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitEventsApiController.php index e6dbb4816c..d95e00b905 100644 --- a/app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitEventsApiController.php +++ b/app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitEventsApiController.php @@ -170,13 +170,14 @@ private function getSerializerType(): string */ public function getEvents($summit_id) { - \Illuminate\Support\Facades\Session::put('timing.controller_start', microtime(true)); - 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]); - \Illuminate\Support\Facades\Session::put('timing.serializer_start', microtime(true)); + $req->attributes->set('timing.serializer_start', microtime(true)); $data = $response->toArray ( SerializerUtils::getExpand(), @@ -187,9 +188,9 @@ public function getEvents($summit_id) ], $this->getSerializerType() ); - \Illuminate\Support\Facades\Session::put('timing.serializer_end', microtime(true)); + $req->attributes->set('timing.serializer_end', microtime(true)); $result = $this->ok($data); - \Illuminate\Support\Facades\Session::put('timing.controller_end', microtime(true)); + $req->attributes->set('timing.controller_end', microtime(true)); return $result; }); diff --git a/app/Http/Middleware/ServerTimingDoctrine.php b/app/Http/Middleware/ServerTimingDoctrine.php index b22831dfcd..eaf04f617f 100644 --- a/app/Http/Middleware/ServerTimingDoctrine.php +++ b/app/Http/Middleware/ServerTimingDoctrine.php @@ -3,7 +3,6 @@ namespace App\Http\Middleware; use Closure; -use Illuminate\Support\Facades\Session; use Symfony\Component\HttpFoundation\Response; class ServerTimingDoctrine @@ -30,21 +29,19 @@ public function handle($request, Closure $next): Response $bootMs = defined('LARAVEL_START') ? max(($start - LARAVEL_START) * 1000.0, 0.0) : 0.0; $appMs = max($totalMs - $dbMs, 0.0); - // Read controller-level timing markers (set by the controller method). + // 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. - $cStart = Session::has("timing.controller_start") ? (float) Session::get("timing.controller_start") : null; - $cEnd = Session::has("timing.controller_end") ? (float) Session::get("timing.controller_end") : null; - $sStart = Session::has("timing.serializer_start") ? (float) Session::get("timing.serializer_start") : null; - $sEnd = Session::has("timing.serializer_end") ? (float) Session::get("timing.serializer_end") : null; + $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; $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; - // Clear so they don't leak into a recycled worker's next request. - Session::forget(['timing.controller_start','timing.controller_end','timing.serializer_start','timing.serializer_end']); - $response->headers->set('Server-Timing', 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', From a117437ce2b14fc5279967be6d14867fcd10a8db Mon Sep 17 00:00:00 2001 From: smarcet Date: Mon, 25 May 2026 21:03:22 -0300 Subject: [PATCH 24/27] fix(timing): use import instead of fully qualified class for QueryTimingCollector --- app/Http/Middleware/ServerTimingDoctrine.php | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/app/Http/Middleware/ServerTimingDoctrine.php b/app/Http/Middleware/ServerTimingDoctrine.php index eaf04f617f..723a514358 100644 --- a/app/Http/Middleware/ServerTimingDoctrine.php +++ b/app/Http/Middleware/ServerTimingDoctrine.php @@ -2,6 +2,7 @@ namespace App\Http\Middleware; +use App\Http\Middleware\Doctrine\QueryTimingCollector; use Closure; use Symfony\Component\HttpFoundation\Response; @@ -16,13 +17,13 @@ public function handle($request, Closure $next): Response // 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. - \App\Http\Middleware\Doctrine\QueryTimingCollector::reset(); + QueryTimingCollector::reset(); /** @var Response $response */ $response = $next($request); - $dbMs = \App\Http\Middleware\Doctrine\QueryTimingCollector::$totalMs; - $dbCount = \App\Http\Middleware\Doctrine\QueryTimingCollector::$count; + $dbMs = QueryTimingCollector::$totalMs; + $dbCount = QueryTimingCollector::$count; $end = microtime(true); $totalMs = ($end - $start) * 1000.0; From 96e036ad333acacd80c4d25a7dcca0f567afb2fd Mon Sep 17 00:00:00 2001 From: smarcet Date: Mon, 25 May 2026 21:08:11 -0300 Subject: [PATCH 25/27] fix(auth): track side-effects independently in getCurrentUser cache 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. --- app/Models/OAuth2/ResourceServerContext.php | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/app/Models/OAuth2/ResourceServerContext.php b/app/Models/OAuth2/ResourceServerContext.php index e291c3e0d9..39cacbc082 100644 --- a/app/Models/OAuth2/ResourceServerContext.php +++ b/app/Models/OAuth2/ResourceServerContext.php @@ -128,6 +128,8 @@ public function setAuthorizationContext(array $auth_context) $this->auth_context = $auth_context; $this->cachedCurrentUser = null; $this->cachedCurrentUserResolved = false; + $this->groupsSynched = false; + $this->fieldsSynched = false; } /** @@ -160,6 +162,8 @@ public function updateAuthContextVar(string $varName, $value):void { $this->auth_context[$varName] = $value; $this->cachedCurrentUser = null; $this->cachedCurrentUserResolved = false; + $this->groupsSynched = false; + $this->fieldsSynched = false; } } @@ -170,12 +174,14 @@ public function updateAuthContextVar(string $varName, $value):void { * this cache, profiling /events showed the same Member SELECT firing 98+ * times via getByExternalId(). * - * Caches the resolved Member regardless of \$synch_groups / \$update_member_fields - * arguments — the side-effects (group sync, event dispatch, field updates) - * are idempotent per request and only need to run once. + * 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 @@ -186,6 +192,13 @@ 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; } @@ -294,6 +307,8 @@ public function getCurrentUser(bool $synch_groups = true, bool $update_member_fi }); $this->cachedCurrentUserResolved = true; + $this->groupsSynched = $synch_groups; + $this->fieldsSynched = $update_member_fields; return $this->cachedCurrentUser = $resolved; } From f29b535ebad0ed6469ec39fe6dde1c384f137556 Mon Sep 17 00:00:00 2001 From: smarcet Date: Tue, 26 May 2026 09:34:21 -0300 Subject: [PATCH 26/27] fix(presentation): reset memoizedSelectionStatus on all state-mutating 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. --- .../Foundation/Summit/Events/Presentations/Presentation.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/Models/Foundation/Summit/Events/Presentations/Presentation.php b/app/Models/Foundation/Summit/Events/Presentations/Presentation.php index 26ff8efd3e..969bcb9513 100644 --- a/app/Models/Foundation/Summit/Events/Presentations/Presentation.php +++ b/app/Models/Foundation/Summit/Events/Presentations/Presentation.php @@ -927,6 +927,7 @@ public function getSelectedPresentations() */ public function setSelectedPresentations($selected_presentations) { + $this->memoizedSelectionStatus = null; $this->selected_presentations = $selected_presentations; } @@ -1078,6 +1079,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()) { @@ -1089,6 +1091,7 @@ public function setSelectionPlan($selection_plan) public function clearSelectionPlan() { + $this->memoizedSelectionStatus = null; $this->selection_plan = null; } @@ -2181,6 +2184,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()) { From 1bfc58c45fe38e356aa836371b2926ec04b318b4 Mon Sep 17 00:00:00 2001 From: smarcet Date: Tue, 26 May 2026 10:22:33 -0300 Subject: [PATCH 27/27] fix(cache): bump last_edited on association-only mutations to bust PresentationSerializer cache MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../Summit/Events/Presentations/Presentation.php | 8 ++++++++ app/Models/Foundation/Summit/Events/SummitEvent.php | 2 ++ 2 files changed, 10 insertions(+) diff --git a/app/Models/Foundation/Summit/Events/Presentations/Presentation.php b/app/Models/Foundation/Summit/Events/Presentations/Presentation.php index 969bcb9513..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; } 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(); } /**