From 97d04c5ef53e6e44c4e6cad169f19f52229c0c15 Mon Sep 17 00:00:00 2001 From: smarcet Date: Fri, 20 Feb 2026 13:47:58 -0300 Subject: [PATCH 01/17] feat: fix N+1 problem --- .../Summit/DoctrineSummitEventRepository.php | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/app/Repositories/Summit/DoctrineSummitEventRepository.php b/app/Repositories/Summit/DoctrineSummitEventRepository.php index 2963f3924..0464fdfd9 100644 --- a/app/Repositories/Summit/DoctrineSummitEventRepository.php +++ b/app/Repositories/Summit/DoctrineSummitEventRepository.php @@ -744,6 +744,24 @@ public function getAllByPage(PagingInfo $paging_info, Filter $filter = null, Ord if (isset($byId[$id])) $data[] = $byId[$id]; } + // Batch-load toMany collections (level 1) and nested relations (level 2+) + if (!empty($expands) && !empty($data)) { + $this->batchLoadExpandedRelations( + $em, + $data, + $expands, + SummitEvent::class, + self::$expandFieldMap, + [ + // Presentation.speakers returns PresentationSpeakerAssignment items; + // the serializer calls $assignment->getSpeaker() to get the actual speaker. + 'speakers' => fn($assignment) => method_exists($assignment, 'getSpeaker') + ? $assignment->getSpeaker() + : $assignment, + ] + ); + } + if ($shuffleResults) shuffle($data); $end = time() - $start; From e4d015cbb31c15ab1157b6c59686ef1d587eee9c Mon Sep 17 00:00:00 2001 From: smarcet Date: Fri, 20 Feb 2026 13:47:58 -0300 Subject: [PATCH 02/17] feat: fix N+1 problem --- .../OAuth2SummitEventsApiController.php | 51 +- ...etrieveAllSummitEventsBySummitStrategy.php | 4 +- .../RetrieveAllSummitEventsStrategy.php | 4 +- .../events/RetrieveSummitEventsStrategy.php | 8 +- .../PresentationCategorySerializer.php | 2 +- .../Repositories/ISummitEventRepository.php | 2 +- app/Repositories/DoctrineRepository.php | 3 + .../Summit/DoctrineSummitEventRepository.php | 37 +- app/libs/Utils/Doctrine/GraphLoaderTrait.php | 591 ++++++++++++++-- .../GraphLoaderNestedExpandTest.php | 667 ++++++++++++++++++ .../SummitEventExpandBatchLoadingTest.php | 648 +++++++++++++++++ .../Repositories/SummitEventExpandMapTest.php | 109 +++ ...ummitEventNestedExpandBatchLoadingTest.php | 241 +++++++ 13 files changed, 2296 insertions(+), 71 deletions(-) create mode 100644 tests/Unit/Repositories/GraphLoaderNestedExpandTest.php create mode 100644 tests/Unit/Repositories/SummitEventExpandBatchLoadingTest.php create mode 100644 tests/Unit/Repositories/SummitEventExpandMapTest.php create mode 100644 tests/Unit/Repositories/SummitEventNestedExpandBatchLoadingTest.php diff --git a/app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitEventsApiController.php b/app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitEventsApiController.php index b386bde85..9a4ef5585 100644 --- a/app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitEventsApiController.php +++ b/app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitEventsApiController.php @@ -174,12 +174,13 @@ public function getEvents($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]); + $expand = SerializerUtils::getExpand(); + $response = $strategy->getEvents(['summit_id' => $summit_id, 'expand' => $expand]); return $this->ok ( $response->toArray ( - SerializerUtils::getExpand(), + $expand, SerializerUtils::getFields(), SerializerUtils::getRelations(), [ @@ -234,12 +235,13 @@ public function getEventsCSV($summit_id) $this->event_repository, $this->resource_server_context ); - $response = $strategy->getEvents(['summit_id' => $summit_id]); + $expand = SerializerUtils::getExpand(); + $response = $strategy->getEvents(['summit_id' => $summit_id, 'expand' => $expand]); $filename = "activities-" . date('Ymd'); $list = $response->toArray ( - SerializerUtils::getExpand(), + $expand, SerializerUtils::getFields(), ['none'], [ @@ -328,18 +330,20 @@ public function getScheduledEvents($summit_id) $summit = SummitFinderStrategyFactory::build($this->getRepository(), $this->getResourceServerContext())->find($summit_id); if (is_null($summit)) return $this->error404(); + $expand = SerializerUtils::getExpand(); $params = [ 'summit_id' => $summit_id, 'summit' => $summit, 'published' => true, - 'current_user' => $current_user + 'current_user' => $current_user, + 'expand' => $expand, ]; $strategy = new RetrievePublishedSummitEventsBySummitStrategy($this->repository, $this->event_repository, $this->resource_server_context); $response = $strategy->getEvents($params); return $this->ok($response->toArray ( - SerializerUtils::getExpand(), + $expand, SerializerUtils::getFields(), SerializerUtils::getRelations(), $params, @@ -430,13 +434,14 @@ public function getAllEvents() $current_user = $this->resource_server_context->getCurrentUser(true); return $this->withReplica(function() use($current_user){ + $expand = SerializerUtils::getExpand(); $strategy = new RetrieveAllSummitEventsStrategy($this->event_repository); - $response = $strategy->getEvents(); + $response = $strategy->getEvents(['expand' => $expand]); return $this->ok ( $response->toArray ( - SerializerUtils::getExpand(), + $expand, SerializerUtils::getFields(), SerializerUtils::getRelations(), [ @@ -482,8 +487,9 @@ public function getAllPresentations($summit_id) $current_user = $this->resource_server_context->getCurrentUser(true); return $this->withReplica(function() use($current_user, $summit_id){ + $expand = SerializerUtils::getExpand(); $strategy = new RetrieveAllSummitPresentationsStrategy($this->repository, $this->event_repository, $this->resource_server_context); - $response = $strategy->getEvents(['summit_id' => intval($summit_id)]); + $response = $strategy->getEvents(['summit_id' => intval($summit_id), 'expand' => $expand]); $params = [ 'current_user' => $current_user, ]; @@ -491,7 +497,7 @@ public function getAllPresentations($summit_id) ( $response->toArray ( - SerializerUtils::getExpand(), + $expand, SerializerUtils::getFields(), SerializerUtils::getRelations(), $params, @@ -536,6 +542,7 @@ public function getAllVoteablePresentations($summit_id) $summit = SummitFinderStrategyFactory::build($this->repository, $this->resource_server_context)->find($summit_id); if (is_null($summit)) throw new EntityNotFoundException; + $expand = SerializerUtils::getExpand(); $strategy = new RetrieveAllSummitVoteablePresentationsStrategy ( $this->repository, @@ -543,7 +550,7 @@ public function getAllVoteablePresentations($summit_id) $this->resource_server_context ); - $response = $strategy->getEvents(['summit_id' => intval($summit_id)]); + $response = $strategy->getEvents(['summit_id' => intval($summit_id), 'expand' => $expand]); $params = [ 'current_user' => $this->resource_server_context->getCurrentUser(true), @@ -554,7 +561,7 @@ public function getAllVoteablePresentations($summit_id) ( $response->toArray ( - SerializerUtils::getExpand(), + $expand, SerializerUtils::getFields(), SerializerUtils::getRelations(), $params, @@ -599,13 +606,14 @@ public function getAllVoteablePresentationsV2($summit_id) $summit = SummitFinderStrategyFactory::build($this->repository, $this->resource_server_context)->find($summit_id); if (is_null($summit)) throw new EntityNotFoundException; + $expand = SerializerUtils::getExpand(); $strategy = new RetrieveAllSummitVoteablePresentationsStrategy ( $this->repository, $this->event_repository, $this->resource_server_context ); - $response = $strategy->getEvents(['summit_id' => intval($summit_id)]); + $response = $strategy->getEvents(['summit_id' => intval($summit_id), 'expand' => $expand]); $params = [ 'current_user' => $this->resource_server_context->getCurrentUser(true), @@ -628,7 +636,7 @@ public function getAllVoteablePresentationsV2($summit_id) ( $response->toArray ( - SerializerUtils::getExpand(), + $expand, SerializerUtils::getFields(), SerializerUtils::getRelations(), $params, @@ -667,6 +675,7 @@ public function getAllVoteablePresentationsV2CSV($summit_id) $summit = SummitFinderStrategyFactory::build($this->repository, $this->resource_server_context)->find($summit_id); if (is_null($summit)) throw new EntityNotFoundException; + $expand = SerializerUtils::getExpand(); $strategy = new RetrieveAllSummitVoteablePresentationsStrategyCSV ( $this->repository, @@ -674,7 +683,7 @@ public function getAllVoteablePresentationsV2CSV($summit_id) $this->resource_server_context ); - $response = $strategy->getEvents(['summit_id' => intval($summit_id)]); + $response = $strategy->getEvents(['summit_id' => intval($summit_id), 'expand' => $expand]); $params = [ 'current_user' => $this->resource_server_context->getCurrentUser(true), @@ -697,7 +706,7 @@ public function getAllVoteablePresentationsV2CSV($summit_id) $filename = "voteable-presentations-" . date('Ymd'); $list = $response->toArray ( - SerializerUtils::getExpand(), + $expand, SerializerUtils::getFields(), ['none'], $params, @@ -784,13 +793,14 @@ public function getAllScheduledEvents() $current_user = $this->resource_server_context->getCurrentUser(true); return $this->withReplica(function () use ($current_user) { + $expand = SerializerUtils::getExpand(); $strategy = new RetrieveAllPublishedSummitEventsStrategy($this->event_repository); - $response = $strategy->getEvents(); + $response = $strategy->getEvents(['expand' => $expand]); return $this->ok ( $response->toArray ( - SerializerUtils::getExpand(), + $expand, SerializerUtils::getFields(), SerializerUtils::getRelations(), [ @@ -1970,13 +1980,14 @@ public function getUnpublishedEvents($summit_id) } return $this->withReplica(function() use($summit_id, $serializer_type){ + $expand = SerializerUtils::getExpand(); $strategy = new RetrieveAllUnPublishedSummitEventsStrategy($this->repository, $this->event_repository, $this->resource_server_context); - $response = $strategy->getEvents(['summit_id' => $summit_id]); + $response = $strategy->getEvents(['summit_id' => $summit_id, 'expand' => $expand]); return $this->ok($response->toArray ( - SerializerUtils::getExpand(), + $expand, SerializerUtils::getFields(), SerializerUtils::getRelations(), [], diff --git a/app/Http/Controllers/Apis/Protected/Summit/Strategies/events/RetrieveAllSummitEventsBySummitStrategy.php b/app/Http/Controllers/Apis/Protected/Summit/Strategies/events/RetrieveAllSummitEventsBySummitStrategy.php index 88e826c33..958fd1bd1 100644 --- a/app/Http/Controllers/Apis/Protected/Summit/Strategies/events/RetrieveAllSummitEventsBySummitStrategy.php +++ b/app/Http/Controllers/Apis/Protected/Summit/Strategies/events/RetrieveAllSummitEventsBySummitStrategy.php @@ -105,9 +105,9 @@ protected function buildFilter(){ * @param Order|null $order * @return PagingResponse */ - public function retrieveEventsFromSource(PagingInfo $paging_info, Filter $filter = null, Order $order = null): PagingResponse + public function retrieveEventsFromSource(PagingInfo $paging_info, Filter $filter = null, Order $order = null, array $expands = []): PagingResponse { - return $this->events_repository->getAllByPage($paging_info, $filter, $order); + return $this->events_repository->getAllByPage($paging_info, $filter, $order, $expands); } } \ No newline at end of file diff --git a/app/Http/Controllers/Apis/Protected/Summit/Strategies/events/RetrieveAllSummitEventsStrategy.php b/app/Http/Controllers/Apis/Protected/Summit/Strategies/events/RetrieveAllSummitEventsStrategy.php index 3d73bf208..9da5dda2f 100644 --- a/app/Http/Controllers/Apis/Protected/Summit/Strategies/events/RetrieveAllSummitEventsStrategy.php +++ b/app/Http/Controllers/Apis/Protected/Summit/Strategies/events/RetrieveAllSummitEventsStrategy.php @@ -46,9 +46,9 @@ public function __construct(ISummitEventRepository $event_repository) * @param Order|null $order * @return PagingResponse */ - public function retrieveEventsFromSource(PagingInfo $paging_info, Filter $filter = null, Order $order = null) + public function retrieveEventsFromSource(PagingInfo $paging_info, Filter $filter = null, Order $order = null, array $expands = []) { - return $this->event_repository->getAllByPage($paging_info, $filter, $order); + return $this->event_repository->getAllByPage($paging_info, $filter, $order, $expands); } } \ No newline at end of file diff --git a/app/Http/Controllers/Apis/Protected/Summit/Strategies/events/RetrieveSummitEventsStrategy.php b/app/Http/Controllers/Apis/Protected/Summit/Strategies/events/RetrieveSummitEventsStrategy.php index 5d6716fc3..86232d0f8 100644 --- a/app/Http/Controllers/Apis/Protected/Summit/Strategies/events/RetrieveSummitEventsStrategy.php +++ b/app/Http/Controllers/Apis/Protected/Summit/Strategies/events/RetrieveSummitEventsStrategy.php @@ -55,9 +55,13 @@ public function getEvents(array $params = []) list($page, $per_page) = $this->getPaginationParams(); + // Parse expand parameter into array for repository batch-loading + $expandStr = $params['expand'] ?? ''; + $expands = !empty($expandStr) ? array_map('trim', explode(',', $expandStr)) : []; + return $this->retrieveEventsFromSource ( - new PagingInfo($page, $per_page), $this->buildFilter(), $this->buildOrder() + new PagingInfo($page, $per_page), $this->buildFilter(), $this->buildOrder(), $expands ); } @@ -152,7 +156,7 @@ protected function buildOrder() * @param Order|null $order * @return PagingResponse */ - abstract public function retrieveEventsFromSource(PagingInfo $paging_info, Filter $filter = null, Order $order = null); + abstract public function retrieveEventsFromSource(PagingInfo $paging_info, Filter $filter = null, Order $order = null, array $expands = []); /** * @return array diff --git a/app/ModelSerializers/Summit/Presentation/PresentationCategorySerializer.php b/app/ModelSerializers/Summit/Presentation/PresentationCategorySerializer.php index b1ace8379..3063b5be1 100644 --- a/app/ModelSerializers/Summit/Presentation/PresentationCategorySerializer.php +++ b/app/ModelSerializers/Summit/Presentation/PresentationCategorySerializer.php @@ -105,7 +105,7 @@ public function serialize($expand = null, array $fields = [], array $relations = $allowed_access_levels[] = intval($access_level->getId()); } - $values['allowed_access_levels'] = $allowed_access_levels; + $values['.'] = $allowed_access_levels; } if(in_array('proposed_schedule_allowed_locations', $relations)) { diff --git a/app/Models/Foundation/Summit/Repositories/ISummitEventRepository.php b/app/Models/Foundation/Summit/Repositories/ISummitEventRepository.php index 3eaf2ff71..eced15f4e 100644 --- a/app/Models/Foundation/Summit/Repositories/ISummitEventRepository.php +++ b/app/Models/Foundation/Summit/Repositories/ISummitEventRepository.php @@ -36,7 +36,7 @@ public function getPublishedOnSameTimeFrame(IPublishableEvent $event): array; * @param Order|null $order * @return PagingResponse */ - public function getAllByPage(PagingInfo $paging_info, Filter $filter = null, Order $order = null); + public function getAllByPage(PagingInfo $paging_info, Filter $filter = null, Order $order = null, array $expands = []); /** * @param PagingInfo $paging_info diff --git a/app/Repositories/DoctrineRepository.php b/app/Repositories/DoctrineRepository.php index 62542e841..23c4454db 100644 --- a/app/Repositories/DoctrineRepository.php +++ b/app/Repositories/DoctrineRepository.php @@ -25,6 +25,7 @@ use Doctrine\ORM\Tools\Pagination\Paginator; use Doctrine\Common\Collections\AbstractLazyCollection; use Doctrine\Common\Collections\Selectable; +use App\libs\Utils\Doctrine\GraphLoaderTrait; use Illuminate\Support\Facades\Log; use LaravelDoctrine\ORM\Facades\Registry; use models\utils\IBaseRepository; @@ -40,6 +41,8 @@ */ abstract class DoctrineRepository extends EntityRepository implements IBaseRepository { + use GraphLoaderTrait; + protected $fetchJoinCollection = true; public function __construct(EntityManagerInterface $em, ClassMetadata $class) diff --git a/app/Repositories/Summit/DoctrineSummitEventRepository.php b/app/Repositories/Summit/DoctrineSummitEventRepository.php index 0464fdfd9..4b62f5777 100644 --- a/app/Repositories/Summit/DoctrineSummitEventRepository.php +++ b/app/Repositories/Summit/DoctrineSummitEventRepository.php @@ -58,6 +58,27 @@ final class DoctrineSummitEventRepository SummitGroupEvent::ClassName, ]; + /** + * Maps serializer expand names to Doctrine field names — only for mismatches. + * The trait's ClassMetadata auto-detection handles association type and entity ownership. + */ + private static array $expandFieldMap = [ + 'track' => 'category', + 'creator' => 'created_by', + 'current_attendance' => 'attendance_metrics', + 'slides' => 'materials', + 'videos' => 'materials', + 'media_uploads' => 'materials', + 'links' => 'materials', + 'extra_questions' => 'extra_question_answers', + 'public_comments' => 'comments', + ]; + + public static function getExpandFieldMap(): array + { + return self::$expandFieldMap; + } + private function ensureJoin(QueryBuilder $qb, string $alias): void { @@ -701,7 +722,7 @@ public function getAllIdsByPage(PagingInfo $paging_info, Filter $filter = null, * @param Order|null $order * @return PagingResponse */ - public function getAllByPage(PagingInfo $paging_info, Filter $filter = null, Order $order = null) + public function getAllByPage(PagingInfo $paging_info, Filter $filter = null, Order $order = null, array $expands = []) { $start = time(); @@ -720,7 +741,21 @@ public function getAllByPage(PagingInfo $paging_info, Filter $filter = null, Ord ->where('e.id IN (:ids)') ->setParameter('ids', $ids); + $em = $this->getEntityManager(); + // Fetch-join requested toOne associations into the hydration query + if (!empty($expands)) { + $query = $this->addExpandFetchJoins( + $em, + $query, + $expands, + 'e', + SummitEvent::class, + self::$expandFieldMap, + [Presentation::class => 'p'], + ['type'] // already inner-joined + ); + } $rows = $query->getQuery()->getResult(); $byId = []; diff --git a/app/libs/Utils/Doctrine/GraphLoaderTrait.php b/app/libs/Utils/Doctrine/GraphLoaderTrait.php index 439d5f726..7a56bcf8f 100644 --- a/app/libs/Utils/Doctrine/GraphLoaderTrait.php +++ b/app/libs/Utils/Doctrine/GraphLoaderTrait.php @@ -13,13 +13,22 @@ */ use Doctrine\ORM\EntityManagerInterface; use Doctrine\ORM\Mapping\ClassMetadata; -use Doctrine\ORM\Query; +use Doctrine\ORM\QueryBuilder; +use Illuminate\Support\Facades\Log; /** * Generic helpers to load a root entity plus a flexible set of relations. - * - Direct ToOne relations are joined in the root query. - * - Each requested top-level ToMany relation is loaded in a separate query. - * - Nested ToOne under a top-level collection (dot-notation) are joined in that same per-collection query. + * + * Single-entity path (loadGraphBy): + * - Direct ToOne relations are joined in the root query. + * - Each requested top-level ToMany relation is loaded in a separate query. + * - Nested relations (dot-notation) are joined at any depth. + * + * Batch/list path (addExpandFetchJoins + batchLoadExpandedRelations): + * - ToOne associations are fetch-joined in the hydration query. + * - ToMany collections are batch-loaded per collection. + * - Nested relations (dot-notation) are recursively batch-loaded at any depth. + * - Uses ClassMetadata to auto-detect association types and entity ownership. */ trait GraphLoaderTrait { @@ -34,11 +43,95 @@ protected function normalizeRelations(array $relations): array )))); } + // ----------------------------------------------------------------------- + // Internal DQL helpers + // ----------------------------------------------------------------------- + + /** + * Build DQL LEFT JOIN chains from dot-notation paths. + * Returns [$selects, $joins] arrays ready to be appended to a DQL query. + * + * For paths like ['affiliations', 'affiliations.organization'], builds: + * LEFT JOIN parent.affiliations alias_affiliations + * LEFT JOIN alias_affiliations.organization alias_affiliations_organization + * + * Deduplicates joins automatically (safe to call with overlapping paths). + */ + private function buildNestedJoinChain(string $parentAlias, array $nestedPaths): array + { + $selects = []; + $joins = []; + $aliasMap = []; + + foreach ($nestedPaths as $path) { + $segments = explode('.', $path); + $currentPrefix = ''; + $currentParent = $parentAlias; + + foreach ($segments as $segment) { + $currentPrefix = $currentPrefix === '' ? $segment : $currentPrefix . '.' . $segment; + + if (isset($aliasMap[$currentPrefix])) { + $currentParent = $aliasMap[$currentPrefix]; + continue; + } + + $alias = $parentAlias . '_' . str_replace('.', '_', $currentPrefix); + $joins[] = "LEFT JOIN {$currentParent}.{$segment} {$alias}"; + $selects[] = $alias; + $aliasMap[$currentPrefix] = $alias; + $currentParent = $alias; + } + } + + return [$selects, $joins]; + } + + /** + * Resolve a serializer expand name to a Doctrine field name and find which + * entity class in the hierarchy owns it. + * + * @return array{string, string, int}|null [ownerClass, doctrineField, assocType] or null + */ + private function resolveExpandAssociation( + string $expandName, + ClassMetadata $baseMeta, + string $baseEntityClass, + array $subMetas, + array $expandFieldMap + ): ?array { + $doctrineField = $expandFieldMap[$expandName] ?? $expandName; + + if (isset($baseMeta->associationMappings[$doctrineField])) { + return [ + $baseEntityClass, + $doctrineField, + $baseMeta->associationMappings[$doctrineField]['type'], + ]; + } + + foreach ($subMetas as $subClass => $subMeta) { + if (isset($subMeta->associationMappings[$doctrineField])) { + return [ + $subClass, + $doctrineField, + $subMeta->associationMappings[$doctrineField]['type'], + ]; + } + } + + return null; + } + + // ----------------------------------------------------------------------- + // Single-entity loading path + // ----------------------------------------------------------------------- + /** * Partition requested relations into: * - $toOneDirect: direct ToOne on the root entity * - $topCollections: top-level ToMany relations on the root entity - * - $nestedByCollection: nested ToOne under a top-level collection (dot-notation) + * - $nestedByCollection: nested paths under a top-level collection (dot-notation, any depth) * * @return array{toOneDirect: string[], topCollections: string[], nestedByCollection: array} */ @@ -53,33 +146,27 @@ protected function partitionRelations(ClassMetadata $meta, array $relations): ar $top = $parts[0]; if (!isset($meta->associationMappings[$top])) { - // Not a mapped association on the root entity; skip. continue; } $type = $meta->associationMappings[$top]['type'] ?? null; if (count($parts) === 1) { - // Direct relation on root if (in_array($type, [ClassMetadata::MANY_TO_ONE, ClassMetadata::ONE_TO_ONE], true)) { $toOneDirect[] = $top; } else { - // ONE_TO_MANY or MANY_TO_MANY $topCollections[] = $top; } } else { - // Dot-notation: top is a collection; the rest are assumed ToOne - $topCollections[] = $top; - $nestedByCollection[$top] = array_merge( - $nestedByCollection[$top] ?? [], - array_slice($parts, 1) - ); + // Dot-notation: top is a collection; store the remaining path as a single string + $topCollections[] = $top; + $remainingPath = implode('.', array_slice($parts, 1)); + $nestedByCollection[$top][] = $remainingPath; } } - // De-duplicate - $toOneDirect = array_values(array_unique($toOneDirect)); - $topCollections = array_values(array_unique($topCollections)); + $toOneDirect = array_values(array_unique($toOneDirect)); + $topCollections = array_values(array_unique($topCollections)); foreach ($nestedByCollection as $k => $arr) { $nestedByCollection[$k] = array_values(array_unique($arr)); } @@ -98,7 +185,7 @@ protected function loadRootWithToOne( EntityManagerInterface $em, string $entityClass, array $toOneDirect, - callable $whereConfigurator // function(QueryBuilder $qb, string $rootAlias): void + callable $whereConfigurator ): ?object { $qb = $em->createQueryBuilder(); $qb->select('r') @@ -109,24 +196,23 @@ protected function loadRootWithToOne( $qb->leftJoin("r.$rel", $alias)->addSelect($alias); } - // Let caller apply WHERE / params $whereConfigurator($qb, 'r'); return $qb->getQuery()->getOneOrNullResult(); } /** - * For a given root id, load exactly one top-level collection and optional nested ToOne under items of that collection. - * Example: root -> badgeFeatureTypes (collection) -> file (ToOne) + * For a given root id, load exactly one top-level collection and optional + * nested relations under items of that collection (supports multi-level dot-paths). * * This hydrates into the current UnitOfWork; no need to assign the result. */ - protected function hydrateCollectionWithNestedToOne( + protected function hydrateCollectionWithNested( EntityManagerInterface $em, string $entityClass, string|int $rootId, string $collection, - array $nestedToOne = [] + array $nestedPaths = [] ): void { $rootAlias = 'r2'; $colAlias = 'c2_' . $collection; @@ -134,10 +220,10 @@ protected function hydrateCollectionWithNestedToOne( $selects = [$rootAlias, $colAlias]; $joins = ["LEFT JOIN $rootAlias.$collection $colAlias"]; - foreach ($nestedToOne as $nestedRel) { - $nestedAlias = $colAlias . '_' . $nestedRel; - $joins[] = "LEFT JOIN $colAlias.$nestedRel $nestedAlias"; - $selects[] = $nestedAlias; + if (!empty($nestedPaths)) { + [$nestedSelects, $nestedJoins] = $this->buildNestedJoinChain($colAlias, $nestedPaths); + $selects = array_merge($selects, $nestedSelects); + $joins = array_merge($joins, $nestedJoins); } $dql = sprintf( @@ -155,8 +241,9 @@ protected function hydrateCollectionWithNestedToOne( } /** - * High-level convenience: load a root entity by an arbitrary field using a where configurator, + * High-level convenience: load a root entity by an arbitrary field, * plus a flexible graph (ToOne direct + one query per requested top-level collection). + * Supports multi-level nested relations via dot-notation. * * Returns the root entity or null. */ @@ -164,45 +251,465 @@ protected function loadGraphBy( EntityManagerInterface $em, string $entityClass, array $relations, - callable $whereConfigurator // function(QueryBuilder $qb, string $rootAlias): void + callable $whereConfigurator ): ?object { $meta = $em->getClassMetadata($entityClass); $relations = $this->normalizeRelations($relations); $partitions = $this->partitionRelations($meta, $relations); - // 1) Root + direct ToOne $root = $this->loadRootWithToOne( - $em, - $entityClass, - $partitions['toOneDirect'], - $whereConfigurator + $em, $entityClass, $partitions['toOneDirect'], $whereConfigurator ); if (!$root) { return null; } - // 2) Per collection query (+ optional nested ToOne) $idField = $meta->getSingleIdentifierFieldName(); $getter = 'get' . ucfirst($idField); $rootId = $root->$getter(); foreach ($partitions['topCollections'] as $collection) { - // Defensive: ensure mapping still exists if (!isset($meta->associationMappings[$collection])) { continue; } $nested = $partitions['nestedByCollection'][$collection] ?? []; - $this->hydrateCollectionWithNestedToOne( - $em, - $entityClass, - $rootId, - $collection, - $nested + $this->hydrateCollectionWithNested( + $em, $entityClass, $rootId, $collection, $nested ); } return $root; } + + // ----------------------------------------------------------------------- + // Batch/list loading path + // ----------------------------------------------------------------------- + + /** + * Batch-load a single collection (+ optional nested relations at any depth) + * for multiple root IDs at once. Results hydrate into the UnitOfWork. + */ + protected function batchHydrateCollections( + EntityManagerInterface $em, + string $entityClass, + array $rootIds, + string $collection, + array $nestedPaths = [] + ): void { + if (empty($rootIds)) return; + + $rootAlias = 'r2'; + $colAlias = 'c2_' . $collection; + + $selects = [$rootAlias, $colAlias]; + $joins = ["LEFT JOIN $rootAlias.$collection $colAlias"]; + + if (!empty($nestedPaths)) { + [$nestedSelects, $nestedJoins] = $this->buildNestedJoinChain($colAlias, $nestedPaths); + $selects = array_merge($selects, $nestedSelects); + $joins = array_merge($joins, $nestedJoins); + } + + $dql = sprintf( + 'SELECT DISTINCT %s FROM %s %s %s WHERE %s.id IN (:ids)', + implode(', ', $selects), + $entityClass, + $rootAlias, + implode(' ', $joins), + $rootAlias + ); + + try { + $em->createQuery($dql) + ->setParameter('ids', $rootIds) + ->getResult(); + } catch (\Exception $ex) { + Log::warning("GraphLoaderTrait::batchHydrateCollections failed for {$entityClass}::{$collection}", [ + 'error' => $ex->getMessage(), + ]); + } + } + + /** + * Adds LEFT JOIN + addSelect for each requested toOne expand, + * using ClassMetadata to auto-detect association type and owner entity. + * + * @param EntityManagerInterface $em + * @param QueryBuilder $query + * @param string[] $expands Serializer expand names from the API + * @param string $rootAlias Root entity alias in the query + * @param string $baseEntityClass Root entity FQCN + * @param array $expandFieldMap Maps serializer name => Doctrine field (only for mismatches) + * @param array $subclassAliases Maps entity FQCN => query alias (e.g., Presentation::class => 'p') + * @param array $skipFields Doctrine fields already joined in the query + * @return QueryBuilder + */ + protected function addExpandFetchJoins( + EntityManagerInterface $em, + QueryBuilder $query, + array $expands, + string $rootAlias, + string $baseEntityClass, + array $expandFieldMap = [], + array $subclassAliases = [], + array $skipFields = [] + ): QueryBuilder { + $baseMeta = $em->getClassMetadata($baseEntityClass); + $subMetas = []; + foreach ($baseMeta->subClasses as $subClass) { + $subMetas[$subClass] = $em->getClassMetadata($subClass); + } + + $joined = []; + foreach ($expands as $expand) { + if (str_contains($expand, '.')) continue; + + $resolved = $this->resolveExpandAssociation( + $expand, $baseMeta, $baseEntityClass, $subMetas, $expandFieldMap + ); + if ($resolved === null) continue; + + [$ownerClass, $doctrineField, $assocType] = $resolved; + + if (in_array($doctrineField, $skipFields, true)) continue; + if (!in_array($assocType, [ClassMetadata::MANY_TO_ONE, ClassMetadata::ONE_TO_ONE], true)) continue; + + $joinKey = $ownerClass . '::' . $doctrineField; + if (isset($joined[$joinKey])) continue; + $joined[$joinKey] = true; + + $alias = 'exp_' . $doctrineField; + if (in_array($alias, $query->getAllAliases(), true)) continue; + + $root = $subclassAliases[$ownerClass] ?? $rootAlias; + $query->leftJoin("{$root}.{$doctrineField}", $alias)->addSelect($alias); + } + + return $query; + } + + /** + * Main entry point for batch loading expanded relations after the hydration query. + * Handles toMany collections (level 1) and nested relations (level 2+) at any depth. + * + * Uses ClassMetadata to auto-detect association types and entity ownership + * in the class hierarchy. + * + * @param EntityManagerInterface $em + * @param object[] $entities The hydrated root entities + * @param string[] $expands Serializer expand names (may contain dots for nesting) + * @param string $baseEntityClass Root entity FQCN + * @param array $expandFieldMap Maps serializer name => Doctrine field (only for mismatches) + * @param array $childEntityResolvers Maps expand path => callable(collectionItem): ?object + * For unwrapping collection items (e.g., assignment => speaker). + * Keys use expand dot-paths: 'speakers' (level 1), + * 'speakers.wrapped_items' (level 2+). + * @param array $nestedFieldOverrides Maps expand dot-path => Doctrine field name + * for nested levels where serializer names differ. + * E.g., 'speakers.speaker_affiliations' => 'affiliations' + */ + protected function batchLoadExpandedRelations( + EntityManagerInterface $em, + array $entities, + array $expands, + string $baseEntityClass, + array $expandFieldMap = [], + array $childEntityResolvers = [], + array $nestedFieldOverrides = [] + ): void { + if (empty($entities) || empty($expands)) return; + + $baseMeta = $em->getClassMetadata($baseEntityClass); + $subMetas = []; + foreach ($baseMeta->subClasses as $subClass) { + $subMetas[$subClass] = $em->getClassMetadata($subClass); + } + + // Separate immediate (level 1) from nested (dot-notation) expands + $immediateExpands = []; + $nestedExpands = []; + + foreach ($expands as $expand) { + if (str_contains($expand, '.')) { + $nestedExpands[] = $expand; + // Ensure the top-level collection is also batch-loaded + $topName = explode('.', $expand)[0]; + $immediateExpands[] = $topName; + } else { + $immediateExpands[] = $expand; + } + } + $immediateExpands = array_values(array_unique($immediateExpands)); + + // Classify immediate expands and collect toMany batches + $toManyBatches = []; // entityClass => [doctrineField => true] + + foreach ($immediateExpands as $expand) { + $resolved = $this->resolveExpandAssociation( + $expand, $baseMeta, $baseEntityClass, $subMetas, $expandFieldMap + ); + if ($resolved === null) continue; + + [$ownerClass, $doctrineField, $assocType] = $resolved; + + // Only batch-load toMany (toOne already handled by addExpandFetchJoins) + if (in_array($assocType, [ClassMetadata::MANY_TO_ONE, ClassMetadata::ONE_TO_ONE], true)) continue; + + $toManyBatches[$ownerClass][$doctrineField] = true; + } + + // Partition entity IDs by class + $idsByClass = [$baseEntityClass => []]; + foreach (array_keys($subMetas) as $subClass) { + $idsByClass[$subClass] = []; + } + foreach ($entities as $entity) { + $id = $entity->getId(); + $idsByClass[$baseEntityClass][] = $id; + foreach (array_keys($subMetas) as $subClass) { + if ($entity instanceof $subClass) { + $idsByClass[$subClass][] = $id; + } + } + } + + // Execute batch toMany queries + foreach ($toManyBatches as $entityClass => $fields) { + $ids = $idsByClass[$entityClass] ?? []; + if (empty($ids)) continue; + + foreach (array_keys($fields) as $field) { + try { + $em->createQueryBuilder() + ->select('root', 'col') + ->from($entityClass, 'root') + ->leftJoin("root.{$field}", 'col') + ->where('root.id IN (:ids)') + ->setParameter('ids', $ids) + ->getQuery() + ->getResult(); + } catch (\Exception $ex) { + Log::warning("GraphLoaderTrait::batchLoadExpandedRelations failed for {$entityClass}::{$field}", [ + 'error' => $ex->getMessage(), + ]); + } + } + } + + // Handle nested relations (level 2+, recursive) + if (!empty($nestedExpands)) { + $this->batchLoadNestedExpands( + $em, $entities, $nestedExpands, $baseEntityClass, + $expandFieldMap, $childEntityResolvers, $nestedFieldOverrides + ); + } + } + + /** + * Recursively batch-loads nested (dot-notation) relations at any depth. + * + * Given expands like ['speakers.member', 'speakers.affiliations', 'speakers.affiliations.organization']: + * 1. Groups by top-level name ('speakers') + * 2. Collects child entities from the 'speakers' collection + * 3. Batch-loads immediate nested relations ('member', 'affiliations') + * 4. Recursively processes deeper paths ('affiliations.organization') + * + * Both $nestedFieldOverrides and $childEntityResolvers use the full expand + * dot-path as the key (e.g., 'speakers.affiliations' or 'speakers.affiliations.organization'). + * At the first recursion level, simple names are also checked as a fallback. + */ + private function batchLoadNestedExpands( + EntityManagerInterface $em, + array $entities, + array $nestedExpands, + string $baseEntityClass, + array $expandFieldMap, + array $childEntityResolvers, + array $nestedFieldOverrides, + int $maxDepth = 10, + string $pathPrefix = '' + ): void { + if (empty($entities) || empty($nestedExpands) || $maxDepth <= 0) return; + + // Group by top-level expand name + $groups = []; + foreach ($nestedExpands as $expand) { + $parts = explode('.', $expand, 2); + $topName = $parts[0]; + $remaining = $parts[1] ?? null; + + if (!isset($groups[$topName])) { + $groups[$topName] = []; + } + if ($remaining !== null) { + $groups[$topName][] = $remaining; + } + } + + // Get metadata for base and subclasses + $baseMeta = $em->getClassMetadata($baseEntityClass); + $subMetas = []; + foreach ($baseMeta->subClasses as $subClass) { + $subMetas[$subClass] = $em->getClassMetadata($subClass); + } + + foreach ($groups as $topName => $remainingPaths) { + // Build full expand path for override/resolver lookups + $fullTopPath = $pathPrefix . $topName; + + // Resolve Doctrine field name for the top-level expand + // Check expandFieldMap first (level 1), then nestedFieldOverrides by path (level 2+) + $doctrineField = $expandFieldMap[$topName] + ?? $nestedFieldOverrides[$fullTopPath] + ?? $topName; + + // Find which entity class owns this association + $ownerClass = null; + if (isset($baseMeta->associationMappings[$doctrineField])) { + $ownerClass = $baseEntityClass; + } else { + foreach ($subMetas as $subClass => $subMeta) { + if (isset($subMeta->associationMappings[$doctrineField])) { + $ownerClass = $subClass; + break; + } + } + } + + if ($ownerClass === null) continue; + + // Collect child entities from the collection + // Check full path first (level 2+), then simple name (level 1 compat) + $resolver = $childEntityResolvers[$fullTopPath] + ?? $childEntityResolvers[$topName] + ?? null; + $getter = 'get' . ucfirst($doctrineField); + $childEntities = []; + + foreach ($entities as $rootEntity) { + if (!method_exists($rootEntity, $getter)) continue; + + try { + $collection = $rootEntity->$getter(); + } catch (\Exception $ex) { + continue; + } + + if ($collection === null) continue; + + $items = is_iterable($collection) ? $collection : [$collection]; + foreach ($items as $item) { + $child = $resolver ? $resolver($item) : $item; + if ($child !== null && is_object($child)) { + $oid = spl_object_id($child); + $childEntities[$oid] = $child; + } + } + } + + if (empty($childEntities)) continue; + + // Determine child entity class and metadata + $firstChild = reset($childEntities); + $childClass = get_class($firstChild); + + try { + $childMeta = $em->getClassMetadata($childClass); + } catch (\Exception $ex) { + Log::warning("GraphLoaderTrait::batchLoadNestedExpands cannot get metadata for {$childClass}", [ + 'error' => $ex->getMessage(), + ]); + continue; + } + + // Collect unique child IDs + $childIds = []; + foreach ($childEntities as $child) { + if (method_exists($child, 'getId') && $child->getId() !== null) { + $childIds[] = $child->getId(); + } + } + $childIds = array_values(array_unique($childIds)); + if (empty($childIds)) continue; + + // Extract immediate names (this level) and deeper paths (next levels) + $immediateNames = []; + $deeperPaths = []; + + foreach ($remainingPaths as $path) { + $pathParts = explode('.', $path); + $immediateNames[] = $pathParts[0]; + if (count($pathParts) > 1) { + $deeperPaths[] = $path; + } + } + $immediateNames = array_values(array_unique($immediateNames)); + + // Classify immediate names using child's ClassMetadata + $nestedToOne = []; + $nestedToMany = []; + + foreach ($immediateNames as $nestedName) { + // Resolve Doctrine field name using full expand path + $fullNestedPath = $fullTopPath . '.' . $nestedName; + $nestedDocField = $nestedFieldOverrides[$fullNestedPath] ?? $nestedName; + + if (!isset($childMeta->associationMappings[$nestedDocField])) continue; + + $assocType = $childMeta->associationMappings[$nestedDocField]['type']; + if (in_array($assocType, [ClassMetadata::MANY_TO_ONE, ClassMetadata::ONE_TO_ONE], true)) { + $nestedToOne[] = $nestedDocField; + } else { + $nestedToMany[] = $nestedDocField; + } + } + + // Batch fetch-join toOne relations in a single query + $nestedToOne = array_values(array_unique($nestedToOne)); + if (!empty($nestedToOne)) { + try { + $qb = $em->createQueryBuilder() + ->select('child') + ->from($childClass, 'child') + ->where('child.id IN (:ids)') + ->setParameter('ids', $childIds); + + foreach ($nestedToOne as $rel) { + $alias = 'n_' . $rel; + $qb->leftJoin("child.{$rel}", $alias)->addSelect($alias); + } + + $qb->getQuery()->getResult(); + } catch (\Exception $ex) { + Log::warning("GraphLoaderTrait::batchLoadNestedExpands toOne failed for {$childClass}", [ + 'error' => $ex->getMessage(), + 'relations' => $nestedToOne, + ]); + } + } + + // Batch-load each toMany collection separately + $nestedToMany = array_values(array_unique($nestedToMany)); + foreach ($nestedToMany as $collectionField) { + $this->batchHydrateCollections($em, $childClass, $childIds, $collectionField); + } + + // Recurse for deeper levels (level 3+) + if (!empty($deeperPaths)) { + $this->batchLoadNestedExpands( + $em, + array_values($childEntities), + $deeperPaths, + $childClass, + [], // No expandFieldMap at deeper levels (use nestedFieldOverrides instead) + $childEntityResolvers, + $nestedFieldOverrides, + $maxDepth - 1, + $fullTopPath . '.' + ); + } + } + } } diff --git a/tests/Unit/Repositories/GraphLoaderNestedExpandTest.php b/tests/Unit/Repositories/GraphLoaderNestedExpandTest.php new file mode 100644 index 000000000..497cc4658 --- /dev/null +++ b/tests/Unit/Repositories/GraphLoaderNestedExpandTest.php @@ -0,0 +1,667 @@ +id = $id; + $this->track = $track; + } + + public function getId(): int { return $this->id; } + public function getCategory(): ?StubLevel1Entity { return $this->track; } +} + +class StubLevel1Entity +{ + private int $id; + private array $subtracks; + + public function __construct(int $id, array $subtracks = []) + { + $this->id = $id; + $this->subtracks = $subtracks; + } + + public function getId(): int { return $this->id; } + public function getSubtracks(): array { return $this->subtracks; } +} + +class StubLevel2Entity +{ + private int $id; + private array $levels; + + public function __construct(int $id, array $levels = []) + { + $this->id = $id; + $this->levels = $levels; + } + + public function getId(): int { return $this->id; } + public function getAllowedAccessLevels(): array { return $this->levels; } +} + +class StubLevel3Entity +{ + private int $id; + private ?StubLevel4Entity $summit; + + public function __construct(int $id, ?StubLevel4Entity $summit = null) + { + $this->id = $id; + $this->summit = $summit; + } + + public function getId(): int { return $this->id; } + public function getSummit(): ?StubLevel4Entity { return $this->summit; } +} + +class StubLevel4Entity +{ + private int $id; + public function __construct(int $id) { $this->id = $id; } + public function getId(): int { return $this->id; } +} + +// Wrapper to exercise collection items that need unwrapping (like PresentationSpeakerAssignment) +class StubWrappedItem +{ + private StubLevel2Entity $inner; + public function __construct(StubLevel2Entity $inner) { $this->inner = $inner; } + public function getInner(): StubLevel2Entity { return $this->inner; } +} + +// --------------------------------------------------------------------------- +// Test double that exposes the trait's protected methods +// --------------------------------------------------------------------------- + +class GraphLoaderTestDouble +{ + use GraphLoaderTrait; + + public function exposedBatchLoadExpandedRelations( + EntityManagerInterface $em, + array $entities, + array $expands, + string $baseEntityClass, + array $expandFieldMap = [], + array $childEntityResolvers = [], + array $nestedFieldOverrides = [] + ): void { + $this->batchLoadExpandedRelations( + $em, $entities, $expands, $baseEntityClass, + $expandFieldMap, $childEntityResolvers, $nestedFieldOverrides + ); + } +} + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +/** + * Unit test for GraphLoaderTrait's nested expand resolution at 3+ levels, + * including path-based nestedFieldOverrides and childEntityResolvers. + * + * Uses stub entities and mocked Doctrine EntityManager — no database needed. + */ +class GraphLoaderNestedExpandTest extends TestCase +{ + private GraphLoaderTestDouble $loader; + + /** @var list */ + private array $recordedBatchQueries = []; + + /** @var list */ + private array $recordedFetchJoinQueries = []; + + protected function setUp(): void + { + parent::setUp(); + $this->loader = new GraphLoaderTestDouble(); + $this->recordedBatchQueries = []; + $this->recordedFetchJoinQueries = []; + } + + protected function tearDown(): void + { + Mockery::close(); + parent::tearDown(); + } + + // ------------------------------------------------------------------ + // Mock helpers + // ------------------------------------------------------------------ + + /** + * Build a ClassMetadata mock with the given association mappings. + * + * @param array $associations fieldName => ClassMetadata association type constant + * @param string[] $subClasses + */ + private function buildMetadata(array $associations, array $subClasses = []): ClassMetadata + { + $meta = Mockery::mock(ClassMetadata::class)->makePartial(); + $meta->subClasses = $subClasses; + $meta->associationMappings = []; + foreach ($associations as $field => $type) { + $meta->associationMappings[$field] = ['type' => $type]; + } + return $meta; + } + + /** + * Create a permissive QueryBuilder mock that records joins for inspection. + */ + private function buildQueryBuilder(): QueryBuilder + { + $qb = Mockery::mock(QueryBuilder::class); + + // Collect leftJoin calls for later assertion + $qb->shouldReceive('select')->andReturnSelf(); + $qb->shouldReceive('from')->andReturnSelf(); + $qb->shouldReceive('addSelect')->andReturnSelf(); + $qb->shouldReceive('where')->andReturnSelf(); + $qb->shouldReceive('setParameter')->andReturnSelf(); + $qb->shouldReceive('leftJoin')->andReturnSelf(); + $qb->shouldReceive('getAllAliases')->andReturn([]); + + $query = Mockery::mock(Query::class)->makePartial(); + $query->shouldReceive('getResult')->andReturn([]); + $query->shouldReceive('getOneOrNullResult')->andReturn(null); + $qb->shouldReceive('getQuery')->andReturn($query); + + return $qb; + } + + /** + * Create a recording QueryBuilder mock that captures batch query details. + */ + private function buildRecordingQueryBuilder(string $fromClass): QueryBuilder + { + $test = $this; + $currentFrom = $fromClass; + $currentJoins = []; + $currentIds = null; + + $qb = Mockery::mock(QueryBuilder::class); + $qb->shouldReceive('select')->andReturnSelf(); + $qb->shouldReceive('addSelect')->andReturnSelf(); + $qb->shouldReceive('where')->andReturnSelf(); + $qb->shouldReceive('getAllAliases')->andReturn([]); + + $qb->shouldReceive('from')->andReturnUsing(function ($class) use ($qb, &$currentFrom) { + $currentFrom = $class; + return $qb; + }); + + $qb->shouldReceive('leftJoin')->andReturnUsing(function ($join) use ($qb, &$currentJoins) { + $currentJoins[] = $join; + return $qb; + }); + + // Store IDs but defer recording to getQuery() time, because some + // call patterns (toOne fetch-join) call setParameter before leftJoin. + $qb->shouldReceive('setParameter')->andReturnUsing(function ($key, $ids) use ($qb, &$currentIds) { + if ($key === 'ids' && is_array($ids)) { + $currentIds = $ids; + } + return $qb; + }); + + $query = Mockery::mock(Query::class)->makePartial(); + $query->shouldReceive('getResult')->andReturn([]); + + // Record the batch query at getQuery() time when all state is available + $qb->shouldReceive('getQuery')->andReturnUsing( + function () use ($query, &$currentFrom, &$currentJoins, &$currentIds, $test) { + if ($currentIds !== null && !empty($currentJoins)) { + $field = ''; + $parts = explode('.', $currentJoins[0]); + $field = $parts[1] ?? $currentJoins[0]; + + $test->recordedBatchQueries[] = [ + 'class' => $currentFrom, + 'field' => $field, + 'ids' => $currentIds, + 'joins' => $currentJoins, + ]; + } + $currentJoins = []; + $currentIds = null; + return $query; + } + ); + + return $qb; + } + + /** + * Build an EntityManager mock that returns the given metadata map and recording query builders. + * + * @param array $metadataMap entityClass => ClassMetadata + */ + private function buildEntityManager(array $metadataMap): EntityManagerInterface + { + $em = Mockery::mock(EntityManagerInterface::class); + + $em->shouldReceive('getClassMetadata')->andReturnUsing( + fn(string $class) => $metadataMap[$class] ?? $this->buildMetadata([]) + ); + + $em->shouldReceive('createQueryBuilder')->andReturnUsing( + fn() => $this->buildRecordingQueryBuilder('') + ); + + // batchHydrateCollections uses raw DQL via createQuery (not QueryBuilder). + // Must mock Query (not AbstractQuery) to satisfy the return type. + // Also records batch queries so assertions can inspect them. + $test = $this; + $em->shouldReceive('createQuery')->andReturnUsing(function ($dql) use ($test) { + // Parse entity class and field from DQL pattern: + // "SELECT DISTINCT ... FROM {class} r2 LEFT JOIN r2.{field} ..." + $class = ''; + $field = ''; + if (preg_match('/FROM\s+(\S+)\s+/', $dql, $m)) { + $class = $m[1]; + } + if (preg_match('/LEFT JOIN\s+\w+\.(\S+)\s+/', $dql, $m)) { + $field = $m[1]; + } + + $query = Mockery::mock(Query::class)->makePartial(); + $query->shouldReceive('setParameter')->andReturnUsing( + function ($key, $ids) use ($query, $class, $field, $test) { + if ($key === 'ids' && is_array($ids)) { + $test->recordedBatchQueries[] = [ + 'class' => $class, + 'field' => $field, + 'ids' => $ids, + 'joins' => [], + ]; + } + return $query; + } + ); + $query->shouldReceive('getResult')->andReturn([]); + $query->shouldReceive('getOneOrNullResult')->andReturn(null); + return $query; + }); + + return $em; + } + + // ------------------------------------------------------------------ + // Tests: 3+ level nesting with auto-detection (no overrides) + // ------------------------------------------------------------------ + + /** + * @test + * Verifies that 4-level nested expands are processed recursively. + * + * Path: track → track.subtracks → track.subtracks.allowed_access_levels + * → track.subtracks.allowed_access_levels.summit + * + * Doctrine fields match expand names except level 1: track → category (via expandFieldMap). + */ + public function testFourLevelNestedExpandsWithFieldMap(): void + { + // Build entity graph: root → category → subtrack → access_level → summit + $summit = new StubLevel4Entity(400); + $level = new StubLevel3Entity(300, $summit); + $sub = new StubLevel2Entity(200, [$level]); + $track = new StubLevel1Entity(100, [$sub]); + $root = new StubRootEntity(1, $track); + + // Metadata: StubRootEntity has category (toOne) + $rootMeta = $this->buildMetadata([ + 'category' => ClassMetadata::MANY_TO_ONE, + ]); + + // StubLevel1Entity has subtracks (toMany) + $l1Meta = $this->buildMetadata([ + 'subtracks' => ClassMetadata::ONE_TO_MANY, + ]); + + // StubLevel2Entity has allowedAccessLevels (toMany) + $l2Meta = $this->buildMetadata([ + 'allowedAccessLevels' => ClassMetadata::MANY_TO_MANY, + ]); + + // StubLevel3Entity has summit (toOne) + $l3Meta = $this->buildMetadata([ + 'summit' => ClassMetadata::MANY_TO_ONE, + ]); + + $em = $this->buildEntityManager([ + StubRootEntity::class => $rootMeta, + StubLevel1Entity::class => $l1Meta, + StubLevel2Entity::class => $l2Meta, + StubLevel3Entity::class => $l3Meta, + StubLevel4Entity::class => $this->buildMetadata([]), + ]); + + $expands = [ + 'track', + 'track.subtracks', + 'track.subtracks.allowedAccessLevels', + 'track.subtracks.allowedAccessLevels.summit', + ]; + + $this->loader->exposedBatchLoadExpandedRelations( + $em, + [$root], + $expands, + StubRootEntity::class, + ['track' => 'category'], // level 1 field map + ); + + // Verify batch queries were generated at each level: + // Level 2: batch-load subtracks on StubLevel1Entity + // Level 3: batch-load allowedAccessLevels on StubLevel2Entity + // Level 4: batch fetch-join summit on StubLevel3Entity + $this->assertGreaterThanOrEqual( + 3, + count($this->recordedBatchQueries), + sprintf( + "Expected at least 3 batch queries for 4-level nesting. Got %d: %s", + count($this->recordedBatchQueries), + json_encode(array_map(fn($q) => $q['class'] . '::' . $q['field'], $this->recordedBatchQueries)) + ) + ); + + // Verify each level was touched with correct entity classes + $queriedClasses = array_map(fn($q) => $q['class'], $this->recordedBatchQueries); + $this->assertContains(StubLevel1Entity::class, $queriedClasses, 'Should batch-load on Level1 (subtracks)'); + $this->assertContains(StubLevel2Entity::class, $queriedClasses, 'Should batch-load on Level2 (allowedAccessLevels)'); + $this->assertContains(StubLevel3Entity::class, $queriedClasses, 'Should batch-load on Level3 (summit)'); + } + + // ------------------------------------------------------------------ + // Tests: path-based nestedFieldOverrides + // ------------------------------------------------------------------ + + /** + * @test + * Verifies path-based nestedFieldOverrides resolve correctly at levels 2, 3, and 4. + * + * Expand names differ from Doctrine fields at every nested level: + * track.sub_items → Doctrine field: subtracks (override: track.sub_items) + * track.sub_items.levels → Doctrine field: allowedAccessLevels (override: track.sub_items.levels) + * track.sub_items.levels.event → Doctrine field: summit (override: track.sub_items.levels.event) + */ + public function testPathBasedNestedFieldOverridesAtAllLevels(): void + { + $summit = new StubLevel4Entity(400); + $level = new StubLevel3Entity(300, $summit); + $sub = new StubLevel2Entity(200, [$level]); + $track = new StubLevel1Entity(100, [$sub]); + $root = new StubRootEntity(1, $track); + + $rootMeta = $this->buildMetadata(['category' => ClassMetadata::MANY_TO_ONE]); + $l1Meta = $this->buildMetadata(['subtracks' => ClassMetadata::ONE_TO_MANY]); + $l2Meta = $this->buildMetadata(['allowedAccessLevels' => ClassMetadata::MANY_TO_MANY]); + $l3Meta = $this->buildMetadata(['summit' => ClassMetadata::MANY_TO_ONE]); + + $em = $this->buildEntityManager([ + StubRootEntity::class => $rootMeta, + StubLevel1Entity::class => $l1Meta, + StubLevel2Entity::class => $l2Meta, + StubLevel3Entity::class => $l3Meta, + StubLevel4Entity::class => $this->buildMetadata([]), + ]); + + // Expand names intentionally DIFFER from Doctrine fields at every nested level + $expands = [ + 'track', + 'track.sub_items', + 'track.sub_items.levels', + 'track.sub_items.levels.event', + ]; + + $this->loader->exposedBatchLoadExpandedRelations( + $em, + [$root], + $expands, + StubRootEntity::class, + ['track' => 'category'], // level 1 field map + [], // no resolvers + [ + // path-based overrides: expand path → Doctrine field + 'track.sub_items' => 'subtracks', + 'track.sub_items.levels' => 'allowedAccessLevels', + 'track.sub_items.levels.event' => 'summit', + ] + ); + + $this->assertGreaterThanOrEqual( + 3, + count($this->recordedBatchQueries), + sprintf( + "Expected at least 3 batch queries with path-based overrides. Got %d: %s", + count($this->recordedBatchQueries), + json_encode(array_map(fn($q) => $q['class'] . '::' . $q['field'], $this->recordedBatchQueries)) + ) + ); + + $queriedClasses = array_map(fn($q) => $q['class'], $this->recordedBatchQueries); + $this->assertContains(StubLevel1Entity::class, $queriedClasses, 'Override track.sub_items → subtracks should resolve'); + $this->assertContains(StubLevel2Entity::class, $queriedClasses, 'Override track.sub_items.levels → allowedAccessLevels should resolve'); + $this->assertContains(StubLevel3Entity::class, $queriedClasses, 'Override track.sub_items.levels.event → summit should resolve'); + } + + // ------------------------------------------------------------------ + // Tests: path-based childEntityResolvers at deeper levels + // ------------------------------------------------------------------ + + /** + * @test + * Verifies that childEntityResolvers work at level 2+ using path-based keys. + * + * Level 1 collection returns wrapped items (StubWrappedItem) that need + * unwrapping via a resolver to get the real entity (StubLevel2Entity). + * This simulates PresentationSpeakerAssignment → PresentationSpeaker. + */ + public function testPathBasedChildEntityResolverAtLevel2(): void + { + $level = new StubLevel3Entity(300); + $inner = new StubLevel2Entity(200, [$level]); + $wrapped = new StubWrappedItem($inner); + + // Use real StubLevel1Entity (not Mockery mock) so get_class() returns + // a name that matches the metadata map for recursive resolution. + $track = new StubLevel1Entity(100, [$wrapped]); + $root = new StubRootEntity(1, $track); + + $rootMeta = $this->buildMetadata(['category' => ClassMetadata::MANY_TO_ONE]); + $l1Meta = $this->buildMetadata(['subtracks' => ClassMetadata::ONE_TO_MANY]); + $l2Meta = $this->buildMetadata(['allowedAccessLevels' => ClassMetadata::MANY_TO_MANY]); + $l3Meta = $this->buildMetadata(['summit' => ClassMetadata::MANY_TO_ONE]); + + $em = $this->buildEntityManager([ + StubRootEntity::class => $rootMeta, + StubLevel1Entity::class => $l1Meta, + StubLevel2Entity::class => $l2Meta, + StubLevel3Entity::class => $l3Meta, + ]); + + $resolverCalled = false; + + $this->loader->exposedBatchLoadExpandedRelations( + $em, + [$root], + ['track', 'track.subtracks', 'track.subtracks.allowedAccessLevels'], + StubRootEntity::class, + ['track' => 'category'], + [ + // Path-based resolver at level 2: unwrap StubWrappedItem → StubLevel2Entity + 'track.subtracks' => function ($item) use (&$resolverCalled) { + $resolverCalled = true; + return $item instanceof StubWrappedItem ? $item->getInner() : $item; + }, + ] + ); + + $this->assertTrue($resolverCalled, 'Path-based resolver at track.subtracks should have been called'); + + // Verify that the unwrapped entity (StubLevel2Entity) was used for deeper queries + $queriedClasses = array_map(fn($q) => $q['class'], $this->recordedBatchQueries); + $this->assertContains( + StubLevel2Entity::class, + $queriedClasses, + 'After unwrapping, batch query should target StubLevel2Entity' + ); + } + + // ------------------------------------------------------------------ + // Tests: expand with no matching association is silently skipped + // ------------------------------------------------------------------ + + /** + * @test + * Verifies that unknown nested expands are silently skipped without errors. + */ + public function testUnknownNestedExpandsAreSkipped(): void + { + $track = new StubLevel1Entity(100); + $root = new StubRootEntity(1, $track); + + $rootMeta = $this->buildMetadata(['category' => ClassMetadata::MANY_TO_ONE]); + $l1Meta = $this->buildMetadata([]); // no associations at all + + $em = $this->buildEntityManager([ + StubRootEntity::class => $rootMeta, + StubLevel1Entity::class => $l1Meta, + ]); + + // This should not throw — unknown nested expand is skipped + $this->loader->exposedBatchLoadExpandedRelations( + $em, + [$root], + ['track', 'track.nonexistent_field', 'track.nonexistent_field.deeper'], + StubRootEntity::class, + ['track' => 'category'], + ); + + // No batch queries for level 2+ (nothing to resolve) + $level2Queries = array_filter( + $this->recordedBatchQueries, + fn($q) => $q['class'] === StubLevel1Entity::class + ); + $this->assertEmpty($level2Queries, 'Unknown fields should produce no batch queries'); + } + + // ------------------------------------------------------------------ + // Tests: maxDepth safety limit + // ------------------------------------------------------------------ + + /** + * @test + * Verifies recursion stops at maxDepth (no infinite loop on deep paths). + */ + public function testRecursionRespectsMaxDepth(): void + { + // Create a self-referencing chain that would recurse infinitely + // without maxDepth protection. In practice, the recursion stops + // because entities at each level are finite, but this tests the safety limit. + $track = new StubLevel1Entity(100); + $root = new StubRootEntity(1, $track); + + $rootMeta = $this->buildMetadata(['category' => ClassMetadata::MANY_TO_ONE]); + $l1Meta = $this->buildMetadata(['subtracks' => ClassMetadata::ONE_TO_MANY]); + + $em = $this->buildEntityManager([ + StubRootEntity::class => $rootMeta, + StubLevel1Entity::class => $l1Meta, + ]); + + // Deeply nested path — should not cause infinite recursion + $expands = [ + 'track', + 'track.subtracks', + 'track.subtracks.subtracks', + 'track.subtracks.subtracks.subtracks', + 'track.subtracks.subtracks.subtracks.subtracks', + ]; + + // Should complete without error (maxDepth = 10 by default) + $this->loader->exposedBatchLoadExpandedRelations( + $em, + [$root], + $expands, + StubRootEntity::class, + ['track' => 'category'], + ); + + // Just verify it didn't hang or crash + $this->assertTrue(true, 'Deep nesting should complete without error'); + } + + // ------------------------------------------------------------------ + // Tests: simple name fallback for resolvers (backward compat) + // ------------------------------------------------------------------ + + /** + * @test + * Verifies that childEntityResolvers with simple name keys (no dots) + * still work at level 1 for backward compatibility. + */ + public function testSimpleNameResolverFallbackAtLevel1(): void + { + $level = new StubLevel3Entity(300); + $inner = new StubLevel2Entity(200, [$level]); + $wrapped = new StubWrappedItem($inner); + + // Use real StubLevel1Entity so get_class() matches the metadata map + $track = new StubLevel1Entity(100, [$wrapped]); + $root = new StubRootEntity(1, $track); + + $rootMeta = $this->buildMetadata(['category' => ClassMetadata::MANY_TO_ONE]); + $l1Meta = $this->buildMetadata(['subtracks' => ClassMetadata::ONE_TO_MANY]); + $l2Meta = $this->buildMetadata(['allowedAccessLevels' => ClassMetadata::MANY_TO_MANY]); + + $em = $this->buildEntityManager([ + StubRootEntity::class => $rootMeta, + StubLevel1Entity::class => $l1Meta, + StubLevel2Entity::class => $l2Meta, + ]); + + $resolverCalled = false; + + $this->loader->exposedBatchLoadExpandedRelations( + $em, + [$root], + ['track', 'track.subtracks', 'track.subtracks.allowedAccessLevels'], + StubRootEntity::class, + ['track' => 'category'], + [ + // Simple name (no dot) — should still work via fallback + 'subtracks' => function ($item) use (&$resolverCalled) { + $resolverCalled = true; + return $item instanceof StubWrappedItem ? $item->getInner() : $item; + }, + ] + ); + + $this->assertTrue($resolverCalled, 'Simple-name resolver fallback should work at level 1'); + } +} diff --git a/tests/Unit/Repositories/SummitEventExpandBatchLoadingTest.php b/tests/Unit/Repositories/SummitEventExpandBatchLoadingTest.php new file mode 100644 index 000000000..28bd3bd32 --- /dev/null +++ b/tests/Unit/Repositories/SummitEventExpandBatchLoadingTest.php @@ -0,0 +1,648 @@ +repository = app(ISummitEventRepository::class); + } + + protected function tearDown(): void + { + self::clearSummitTestData(); + parent::tearDown(); + } + + /** + * Attaches a DebugStack to the Doctrine connection and returns it. + */ + private function attachQueryLogger(): DebugStack + { + $em = Registry::getManager(SilverstripeBaseModel::EntityManager); + $debugStack = new DebugStack(); + $em->getConnection()->getConfiguration()->setSQLLogger($debugStack); + return $debugStack; + } + + /** + * Detaches the query logger. + */ + private function detachQueryLogger(): void + { + $em = Registry::getManager(SilverstripeBaseModel::EntityManager); + $em->getConnection()->getConfiguration()->setSQLLogger(null); + } + + /** + * Clears the Doctrine identity map so entities must be re-loaded from DB. + */ + private function clearIdentityMap(): void + { + $em = Registry::getManager(SilverstripeBaseModel::EntityManager); + $em->clear(); + } + + /** + * Builds a filter scoped to the test summit. + */ + private function buildSummitFilter(): Filter + { + $filter = new Filter(); + $filter->addFilterCondition(FilterParser::buildFilter('summit_id', '==', self::$summit->getId())); + return $filter; + } + + /** + * Classifies a recorded SQL query by the relation/table it targets. + */ + private function classifyQuery(string $sql): string + { + $sql = strtolower($sql); + + if (str_contains($sql, 'count(') && str_contains($sql, 'summitevent')) + return 'count'; + + if (str_contains($sql, 'summitevent') && !str_contains($sql, 'left join') && str_contains($sql, 'limit')) + return 'id_list'; + + if (str_contains($sql, 'summitevent') && str_contains($sql, 'in (')) + return 'hydration'; + + if (str_contains($sql, 'tag') || str_contains($sql, 'taggedrelation')) + return 'tags'; + + if (str_contains($sql, 'speaker') || str_contains($sql, 'presentationspeaker')) + return 'speakers'; + + if (str_contains($sql, 'presentationmaterial') || str_contains($sql, 'presentationmediaupload') + || str_contains($sql, 'presentationslide') || str_contains($sql, 'presentationvideo') + || str_contains($sql, 'presentationlink')) + return 'materials'; + + if (str_contains($sql, 'summitabstractlocation') || str_contains($sql, 'summitvenue')) + return 'location'; + + if (str_contains($sql, 'presentationcategory')) + return 'track'; + + if (str_contains($sql, 'summiteventtype') || str_contains($sql, 'presentationtype')) + return 'type'; + + if (str_contains($sql, 'selectionplan')) + return 'selection_plan'; + + if (str_contains($sql, 'presentationaction')) + return 'actions'; + + if (str_contains($sql, 'presentationcomment') || str_contains($sql, 'summitpresentationcomment')) + return 'comments'; + + if (str_contains($sql, 'extraquestionanswer')) + return 'extra_questions'; + + if (str_contains($sql, 'attendancemetric') || str_contains($sql, 'summiteventattendancemetric')) + return 'attendance'; + + if (str_contains($sql, 'allowedtickettype') || str_contains($sql, 'summit_event_allowed_ticket_types')) + return 'allowed_ticket_types'; + + if (str_contains($sql, 'rsvptemplate')) + return 'rsvp_template'; + + if (str_contains($sql, 'sponsor')) + return 'sponsors'; + + if (str_contains($sql, 'summiteventfeedback')) + return 'feedback'; + + if (str_contains($sql, 'member')) + return 'member'; + + return 'other'; + } + + /** + * Groups recorded queries by category and returns [category => count]. + */ + private function categorizeQueries(DebugStack $debugStack): array + { + $categories = []; + foreach ($debugStack->queries as $query) { + $category = $this->classifyQuery($query['sql']); + $categories[$category] = ($categories[$category] ?? 0) + 1; + } + ksort($categories); + return $categories; + } + + /** + * Formats a category breakdown as a readable table string. + */ + private function formatBreakdown(array $categories, int $total): string + { + $lines = []; + foreach ($categories as $cat => $count) { + $lines[] = sprintf(" %-22s %3d queries", $cat, $count); + } + $lines[] = sprintf(" %-22s %3d queries", 'TOTAL', $total); + return implode("\n", $lines); + } + + /** + * Touches ALL relations on each entity to force lazy-load queries. + * This mirrors what the serializer does when all expands are requested. + * + * @param SummitEvent[] $entities + */ + private function touchAllRelations(array $entities): void + { + foreach ($entities as $entity) { + // SummitEvent toOne + $entity->getLocation(); + $entity->getCategory(); + $entity->getType(); + $entity->getCreatedBy(); + $entity->getUpdatedBy(); + $entity->getRSVPTemplate(); + + // SummitEvent toMany + $entity->getTags()->count(); + $entity->getSponsors()->count(); + $entity->getFeedback()->count(); + $entity->getAttendance()->count(); + $entity->getAllowedTicketTypes()->count(); + + // Presentation-specific + if ($entity instanceof Presentation) { + // toOne + $entity->getModerator(); + $entity->getSelectionPlan(); + + // toMany + $entity->getSpeakers()->count(); + $entity->getMaterials()->count(); + $entity->getComments()->count(); + $entity->getAllExtraQuestionAnswers()->count(); + } + } + } + + /** + * Touches specific relations on each entity for isolated per-expand tests. + * + * @param SummitEvent[] $entities + * @param string[] $relations + */ + private function touchRelations(array $entities, array $relations): void + { + foreach ($entities as $entity) { + foreach ($relations as $rel) { + switch ($rel) { + // SummitEvent toOne + case 'location': + $entity->getLocation(); + break; + case 'track': + $entity->getCategory(); + break; + case 'type': + $entity->getType(); + break; + case 'created_by': + case 'creator': + $entity->getCreatedBy(); + break; + case 'updated_by': + $entity->getUpdatedBy(); + break; + case 'rsvp_template': + $entity->getRSVPTemplate(); + break; + + // SummitEvent toMany + case 'tags': + $entity->getTags()->count(); + break; + case 'sponsors': + $entity->getSponsors()->count(); + break; + case 'feedback': + $entity->getFeedback()->count(); + break; + case 'current_attendance': + $entity->getAttendance()->count(); + break; + case 'allowed_ticket_types': + $entity->getAllowedTicketTypes()->count(); + break; + + // Presentation toOne + case 'moderator': + if ($entity instanceof Presentation) $entity->getModerator(); + break; + case 'selection_plan': + if ($entity instanceof Presentation) $entity->getSelectionPlan(); + break; + + // Presentation toMany + case 'speakers': + if ($entity instanceof Presentation) $entity->getSpeakers()->count(); + break; + case 'slides': + case 'videos': + case 'media_uploads': + case 'links': + if ($entity instanceof Presentation) $entity->getMaterials()->count(); + break; + case 'actions': + if ($entity instanceof Presentation) { + // Access the raw collection to trigger lazy-load + $entity->getPresentationActions(); + } + break; + case 'extra_questions': + if ($entity instanceof Presentation) $entity->getAllExtraQuestionAnswers()->count(); + break; + case 'public_comments': + if ($entity instanceof Presentation) $entity->getComments()->count(); + break; + } + } + } + } + + /** + * Runs a query scenario: fetches page, touches relations, returns query count and breakdown. + * + * @return array{count: int, categories: array, items: SummitEvent[]} + */ + private function runScenario(PagingInfo $paging, Filter $filter, array $expands, array $touchRelations): array + { + $this->clearIdentityMap(); + $debug = $this->attachQueryLogger(); + + $response = $this->repository->getAllByPage($paging, $filter, null, $expands); + $items = $response->getItems(); + $this->touchRelations($items, $touchRelations); + + $count = count($debug->queries); + $categories = $this->categorizeQueries($debug); + $this->detachQueryLogger(); + + return ['count' => $count, 'categories' => $categories, 'items' => $items]; + } + + /** + * Like runScenario but touches ALL relations (not a specific list). + */ + private function runFullScenario(PagingInfo $paging, Filter $filter, array $expands): array + { + $this->clearIdentityMap(); + $debug = $this->attachQueryLogger(); + + $response = $this->repository->getAllByPage($paging, $filter, null, $expands); + $items = $response->getItems(); + $this->touchAllRelations($items); + + $count = count($debug->queries); + $categories = $this->categorizeQueries($debug); + $this->detachQueryLogger(); + + return ['count' => $count, 'categories' => $categories, 'items' => $items]; + } + + // ----------------------------------------------------------------------- + // Tests + // ----------------------------------------------------------------------- + + /** + * @test + * Proves that batch loading with ALL expands from the association map + * significantly reduces total SQL queries compared to lazy loading. + * Outputs a per-category breakdown for full visibility. + */ + public function testBatchLoadingWithAllExpandsReducesQueryCount(): void + { + $paging = new PagingInfo(1, 20); + $filter = $this->buildSummitFilter(); + + // --- Run 1: WITHOUT expands — touch ALL relations (lazy loading) --- + $without = $this->runFullScenario($paging, $filter, []); + + $this->assertNotEmpty($without['items'], 'Test data should produce results'); + $entityCount = count($without['items']); + $presentationCount = count(array_filter($without['items'], fn($e) => $e instanceof Presentation)); + + // --- Run 2: WITH ALL expands (batch loading) --- + $with = $this->runFullScenario($paging, $filter, self::ALL_EXPANDS); + + // --- Build report --- + $reduction = $without['count'] > 0 ? (1 - ($with['count'] / $without['count'])) * 100 : 0; + + $report = sprintf( + "\n" . + "=== Expand Batch Loading: ALL Expands Report ===\n" . + "Entities: %d total (%d Presentations, %d SummitEvents)\n" . + "Expands: %d total (%s)\n" . + "\n" . + "--- WITHOUT expands (lazy loading) ---\n%s\n" . + "\n" . + "--- WITH ALL expands (batch loading) ---\n%s\n" . + "\n" . + "=== RESULT: %d -> %d queries (%.1f%% reduction) ===\n", + $entityCount, + $presentationCount, + $entityCount - $presentationCount, + count(self::ALL_EXPANDS), + implode(', ', self::ALL_EXPANDS), + $this->formatBreakdown($without['categories'], $without['count']), + $this->formatBreakdown($with['categories'], $with['count']), + $without['count'], + $with['count'], + $reduction + ); + + fwrite(STDERR, $report); + + // --- Assertions --- + $this->assertSame( + count($without['items']), + count($with['items']), + 'Both runs must return the same number of entities' + ); + + $this->assertLessThan( + $without['count'], + $with['count'], + sprintf( + 'Batch loading must use fewer queries. Without: %d, With: %d', + $without['count'], + $with['count'] + ) + ); + + $this->assertGreaterThan( + 30.0, + $reduction, + sprintf('Query reduction must be > 30%%. Got %.1f%%', $reduction) + ); + } + + /** + * @test + * Tests EVERY expand from the association map individually. + * For each one: measures lazy vs batch query count and the per-relation delta. + */ + public function testPerExpandQueryReductionForAllExpands(): void + { + $paging = new PagingInfo(1, 20); + $filter = $this->buildSummitFilter(); + + $this->assertNotEmpty( + $this->runScenario($paging, $filter, [], [])['items'], + 'Test data should produce results' + ); + + $report = sprintf( + "\n=== Per-Expand Query Reduction (ALL %d expands) ===\n" . + "%-20s | %6s | %6s | %6s\n" . + "%s\n", + count(self::ALL_EXPANDS), + 'Expand', 'Lazy', 'Batch', 'Saved', + str_repeat('-', 50) + ); + + $totalSaved = 0; + foreach (self::ALL_EXPANDS as $expandName) { + // Lazy: no expand, but touch the relation + $lazy = $this->runScenario($paging, $filter, [], [$expandName]); + + // Batch: with expand, touch the same relation + $batch = $this->runScenario($paging, $filter, [$expandName], [$expandName]); + + $saved = $lazy['count'] - $batch['count']; + $totalSaved += $saved; + + $report .= sprintf( + "%-20s | %6d | %6d | %6d\n", + $expandName, + $lazy['count'], + $batch['count'], + $saved + ); + + // Each expand must not significantly increase query count. + // Allow +1 because the batch query itself fires even when the getter + // method uses a custom DQL (e.g. getPresentationActions filters by + // selection plan and may not trigger standard lazy-load). + $this->assertLessThanOrEqual( + $lazy['count'] + 1, + $batch['count'], + sprintf( + 'Expand "%s" must not add more than 1 query. Lazy: %d, Batch: %d', + $expandName, + $lazy['count'], + $batch['count'] + ) + ); + } + + $report .= sprintf("%s\n%-20s | %6s | %6s | %6d\n", + str_repeat('-', 50), + 'TOTAL SAVED', '', '', $totalSaved + ); + + fwrite(STDERR, $report . "\n"); + + // At least some expands should produce savings + $this->assertGreaterThan( + 0, + $totalSaved, + 'At least some expands should reduce query count' + ); + } + + /** + * @test + * Proves query count scales with page size under lazy loading (N+1 pattern) + * but stays nearly constant with batch loading. Uses ALL expands. + */ + public function testQueryCountScalingWithPageSize(): void + { + $filter = $this->buildSummitFilter(); + $pageSizes = [5, 10, 20]; + + $report = sprintf( + "\n=== Query Scaling by Page Size (ALL %d expands) ===\n" . + "%4s | %6s | %6s | %8s\n" . + "%s\n", + count(self::ALL_EXPANDS), + 'Size', 'Lazy', 'Batch', 'Saved', + str_repeat('-', 36) + ); + + $previousLazyCount = 0; + foreach ($pageSizes as $size) { + $paging = new PagingInfo(1, $size); + + $lazy = $this->runFullScenario($paging, $filter, []); + $batch = $this->runFullScenario($paging, $filter, self::ALL_EXPANDS); + + $saved = $lazy['count'] - $batch['count']; + $report .= sprintf( + "%4d | %6d | %6d | %8d\n", + $size, + $lazy['count'], + $batch['count'], + $saved + ); + + // Lazy-load query count should grow with page size (N+1 pattern) + if ($previousLazyCount > 0) { + $this->assertGreaterThan( + $previousLazyCount, + $lazy['count'], + sprintf( + 'Lazy-load queries must grow with page size. Size %d: %d queries, previous: %d', + $size, + $lazy['count'], + $previousLazyCount + ) + ); + } + + // Batch loading must always use fewer queries + $this->assertLessThan( + $lazy['count'], + $batch['count'], + sprintf( + 'Batch loading must use fewer queries at page size %d. Lazy: %d, Batch: %d', + $size, + $lazy['count'], + $batch['count'] + ) + ); + + $previousLazyCount = $lazy['count']; + } + + fwrite(STDERR, $report . "\n"); + } + + /** + * @test + */ + public function testEmptyExpandsProduceSameQueryCountAsNoExpands(): void + { + $paging = new PagingInfo(1, 10); + $filter = $this->buildSummitFilter(); + + $without = $this->runScenario($paging, $filter, [], []); + $empty = $this->runScenario($paging, $filter, [], []); + + $this->assertSame( + $without['count'], + $empty['count'], + 'Empty expands should not add any extra queries' + ); + } + + /** + * @test + * Verifies batch-loaded entities return identical data as lazy-loaded ones + * across ALL touchable relations. + */ + public function testBatchLoadedEntitiesReturnSameDataAsLazyLoaded(): void + { + $paging = new PagingInfo(1, 10); + $filter = $this->buildSummitFilter(); + + // Without expands (lazy) + $this->clearIdentityMap(); + $responseWithout = $this->repository->getAllByPage($paging, $filter, null, []); + $dataWithout = $responseWithout->getItems(); + + $idsWithout = array_map(fn($e) => $e->getId(), $dataWithout); + $tagCountsWithout = array_map(fn($e) => $e->getTags()->count(), $dataWithout); + $sponsorCountsWithout = array_map(fn($e) => $e->getSponsors()->count(), $dataWithout); + $speakerCountsWithout = array_map(function ($e) { + return $e instanceof Presentation ? $e->getSpeakers()->count() : 0; + }, $dataWithout); + $materialCountsWithout = array_map(function ($e) { + return $e instanceof Presentation ? $e->getMaterials()->count() : 0; + }, $dataWithout); + $commentCountsWithout = array_map(function ($e) { + return $e instanceof Presentation ? $e->getComments()->count() : 0; + }, $dataWithout); + + // With ALL expands (batch) + $this->clearIdentityMap(); + $responseWith = $this->repository->getAllByPage($paging, $filter, null, self::ALL_EXPANDS); + $dataWith = $responseWith->getItems(); + + $idsWith = array_map(fn($e) => $e->getId(), $dataWith); + $tagCountsWith = array_map(fn($e) => $e->getTags()->count(), $dataWith); + $sponsorCountsWith = array_map(fn($e) => $e->getSponsors()->count(), $dataWith); + $speakerCountsWith = array_map(function ($e) { + return $e instanceof Presentation ? $e->getSpeakers()->count() : 0; + }, $dataWith); + $materialCountsWith = array_map(function ($e) { + return $e instanceof Presentation ? $e->getMaterials()->count() : 0; + }, $dataWith); + $commentCountsWith = array_map(function ($e) { + return $e instanceof Presentation ? $e->getComments()->count() : 0; + }, $dataWith); + + $this->assertSame($idsWithout, $idsWith, 'Entity IDs should match'); + $this->assertSame($tagCountsWithout, $tagCountsWith, 'Tag counts should match'); + $this->assertSame($sponsorCountsWithout, $sponsorCountsWith, 'Sponsor counts should match'); + $this->assertSame($speakerCountsWithout, $speakerCountsWith, 'Speaker counts should match'); + $this->assertSame($materialCountsWithout, $materialCountsWith, 'Material counts should match'); + $this->assertSame($commentCountsWithout, $commentCountsWith, 'Comment counts should match'); + } +} diff --git a/tests/Unit/Repositories/SummitEventExpandMapTest.php b/tests/Unit/Repositories/SummitEventExpandMapTest.php new file mode 100644 index 000000000..4ee0b73c4 --- /dev/null +++ b/tests/Unit/Repositories/SummitEventExpandMapTest.php @@ -0,0 +1,109 @@ +map = DoctrineSummitEventRepository::getExpandFieldMap(); + } + + /** + * Every entry in the field map must have a non-empty Doctrine field name. + */ + public function testAllFieldMapEntriesHaveNonEmptyValues(): void + { + foreach ($this->map as $expandName => $doctrineField) { + $this->assertIsString($doctrineField, "Field map entry '{$expandName}' must be a string"); + $this->assertNotEmpty($doctrineField, "Field map entry '{$expandName}' must not be empty"); + } + } + + /** + * The field map must only contain entries where the serializer name + * differs from the Doctrine field name. If they're the same, the entry is redundant. + */ + public function testFieldMapOnlyContainsMismatches(): void + { + foreach ($this->map as $expandName => $doctrineField) { + $this->assertNotSame( + $expandName, + $doctrineField, + "Field map entry '{$expandName}' => '{$doctrineField}' is redundant — name matches Doctrine field" + ); + } + } + + /** + * Known mismatches must be present in the field map. + */ + public function testKnownMismatchesArePresent(): void + { + $expectedMismatches = [ + 'track' => 'category', + 'creator' => 'created_by', + 'current_attendance' => 'attendance_metrics', + 'slides' => 'materials', + 'videos' => 'materials', + 'media_uploads' => 'materials', + 'links' => 'materials', + 'extra_questions' => 'extra_question_answers', + 'public_comments' => 'comments', + ]; + + foreach ($expectedMismatches as $expandName => $expectedField) { + $this->assertArrayHasKey( + $expandName, + $this->map, + "Field map is missing known mismatch: {$expandName}" + ); + $this->assertSame( + $expectedField, + $this->map[$expandName], + "Field map entry '{$expandName}' should map to '{$expectedField}'" + ); + } + } + + /** + * The material-type expands should all map to the same 'materials' field. + */ + public function testMaterialsFieldDeduplication(): void + { + $materialExpands = ['slides', 'videos', 'media_uploads', 'links']; + $fields = []; + foreach ($materialExpands as $expand) { + $this->assertArrayHasKey($expand, $this->map, "Field map missing material expand: {$expand}"); + $fields[] = $this->map[$expand]; + } + $this->assertCount(1, array_unique($fields), "All material-type expands should map to the same 'materials' field"); + $this->assertSame('materials', $fields[0]); + } + + /** + * Creator and created_by should resolve to the same Doctrine field. + * 'creator' is in the field map; 'created_by' matches its Doctrine name so it's not. + */ + public function testCreatorMapsToCreatedBy(): void + { + $this->assertArrayHasKey('creator', $this->map); + $this->assertSame('created_by', $this->map['creator']); + } + + /** + * Empty field map filter should produce no entries. + */ + public function testEmptyExpandsProduceNoEntries(): void + { + $expands = []; + $matched = array_filter($this->map, fn($key) => in_array($key, $expands), ARRAY_FILTER_USE_KEY); + $this->assertEmpty($matched); + } +} diff --git a/tests/Unit/Repositories/SummitEventNestedExpandBatchLoadingTest.php b/tests/Unit/Repositories/SummitEventNestedExpandBatchLoadingTest.php new file mode 100644 index 000000000..89c28e779 --- /dev/null +++ b/tests/Unit/Repositories/SummitEventNestedExpandBatchLoadingTest.php @@ -0,0 +1,241 @@ +repository = app(ISummitEventRepository::class); + } + + protected function tearDown(): void + { + self::clearSummitTestData(); + parent::tearDown(); + } + + private function attachQueryLogger(): DebugStack + { + $em = Registry::getManager(SilverstripeBaseModel::EntityManager); + $debugStack = new DebugStack(); + $em->getConnection()->getConfiguration()->setSQLLogger($debugStack); + return $debugStack; + } + + private function detachQueryLogger(): void + { + $em = Registry::getManager(SilverstripeBaseModel::EntityManager); + $em->getConnection()->getConfiguration()->setSQLLogger(null); + } + + private function clearIdentityMap(): void + { + $em = Registry::getManager(SilverstripeBaseModel::EntityManager); + $em->clear(); + } + + private function buildSummitFilter(): Filter + { + $filter = new Filter(); + $filter->addFilterCondition(FilterParser::buildFilter('summit_id', '==', self::$summit->getId())); + return $filter; + } + + /** + * Touches level 1 + level 2 relations on speakers to trigger lazy-load queries. + * + * @param SummitEvent[] $entities + * @param bool $touchNested Whether to also touch second-level speaker relations + */ + private function touchSpeakerRelations(array $entities, bool $touchNested = false): void + { + foreach ($entities as $entity) { + if (!($entity instanceof Presentation)) continue; + + $speakers = $entity->getSpeakers(); + // Touch level 1: speakers collection + $speakers->count(); + + if ($touchNested) { + // Touch level 2: each speaker's relations + foreach ($speakers as $speaker) { + $speaker->getMember(); // toOne + $speaker->getAffiliations()->count(); // toMany + } + } + } + } + + /** + * Runs a scenario and returns query count. + * + * @return array{count: int, items: SummitEvent[]} + */ + private function runScenario(PagingInfo $paging, Filter $filter, array $expands, bool $touchNested): array + { + $this->clearIdentityMap(); + $debug = $this->attachQueryLogger(); + + $response = $this->repository->getAllByPage($paging, $filter, null, $expands); + $items = $response->getItems(); + $this->touchSpeakerRelations($items, $touchNested); + + $count = count($debug->queries); + $this->detachQueryLogger(); + + return ['count' => $count, 'items' => $items]; + } + + /** + * @test + * Proves that nested expands (speakers.member, speakers.affiliations) + * reduce query count compared to lazy loading at level 2. + */ + public function testNestedExpandReducesQueryCountForSpeakerRelations(): void + { + $paging = new PagingInfo(1, 20); + $filter = $this->buildSummitFilter(); + + // Level 1 only: speakers loaded, but nested relations lazy-loaded + $level1Only = $this->runScenario( + $paging, $filter, + ['speakers'], + true // touch nested = trigger lazy loads for member/affiliations + ); + + $this->assertNotEmpty($level1Only['items'], 'Test data should produce results'); + $presentationCount = count(array_filter($level1Only['items'], fn($e) => $e instanceof Presentation)); + $this->assertGreaterThan(0, $presentationCount, 'Should have presentations with speakers'); + + // Level 1 + Level 2: speakers + nested relations batch-loaded + $withNested = $this->runScenario( + $paging, $filter, + ['speakers', 'speakers.member', 'speakers.affiliations'], + true // touch same nested relations — should already be loaded + ); + + $report = sprintf( + "\n=== Nested Expand: Speaker Relations ===\n" . + "Entities: %d presentations\n" . + "Level 1 only (speakers): %d queries\n" . + "Level 1 + 2 (speakers.member,.affil): %d queries\n" . + "Saved: %d queries\n", + $presentationCount, + $level1Only['count'], + $withNested['count'], + $level1Only['count'] - $withNested['count'] + ); + + fwrite(STDERR, $report); + + // Nested batch loading must use fewer or equal queries + $this->assertLessThanOrEqual( + $level1Only['count'], + $withNested['count'], + sprintf( + 'Nested batch loading must not increase queries. L1: %d, L1+L2: %d', + $level1Only['count'], + $withNested['count'] + ) + ); + } + + /** + * @test + * Proves level-1-only expands produce no overhead from nested loading code. + */ + public function testLevel1OnlyExpandsHaveNoNestedOverhead(): void + { + $paging = new PagingInfo(1, 10); + $filter = $this->buildSummitFilter(); + + // With level 1 expands only (no dot-notation) + $level1 = $this->runScenario($paging, $filter, ['speakers', 'tags'], false); + + // Same expands again — should be identical query count + $level1Again = $this->runScenario($paging, $filter, ['speakers', 'tags'], false); + + $this->assertSame( + $level1['count'], + $level1Again['count'], + 'Level 1 only expands should produce consistent query count (no nested overhead)' + ); + } + + /** + * @test + * Proves data integrity: nested batch-loaded speaker data matches lazy-loaded data. + */ + public function testNestedBatchLoadedDataMatchesLazyLoaded(): void + { + $paging = new PagingInfo(1, 10); + $filter = $this->buildSummitFilter(); + + // Lazy: level 1 only, touch nested + $this->clearIdentityMap(); + $response1 = $this->repository->getAllByPage($paging, $filter, null, ['speakers']); + $items1 = $response1->getItems(); + + $speakerData1 = []; + foreach ($items1 as $entity) { + if (!($entity instanceof Presentation)) continue; + foreach ($entity->getSpeakers() as $speaker) { + $speakerData1[$speaker->getId()] = [ + 'member_id' => $speaker->getMember() ? $speaker->getMember()->getId() : null, + 'affiliations_count' => $speaker->getAffiliations()->count(), + ]; + } + } + + // Batch: level 1 + level 2 + $this->clearIdentityMap(); + $response2 = $this->repository->getAllByPage($paging, $filter, null, [ + 'speakers', 'speakers.member', 'speakers.affiliations' + ]); + $items2 = $response2->getItems(); + + $speakerData2 = []; + foreach ($items2 as $entity) { + if (!($entity instanceof Presentation)) continue; + foreach ($entity->getSpeakers() as $speaker) { + $speakerData2[$speaker->getId()] = [ + 'member_id' => $speaker->getMember() ? $speaker->getMember()->getId() : null, + 'affiliations_count' => $speaker->getAffiliations()->count(), + ]; + } + } + + $this->assertSame( + $speakerData1, + $speakerData2, + 'Nested batch-loaded speaker data must match lazy-loaded data' + ); + } +} From fd1871e672a8535a7221bb7a7db7535638cc9722 Mon Sep 17 00:00:00 2001 From: smarcet Date: Sat, 23 May 2026 00:13:40 -0300 Subject: [PATCH 03/17] fix(serializer): restore allowed_access_levels key in PresentationCategorySerializer (typo '.') --- .../Summit/Presentation/PresentationCategorySerializer.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/ModelSerializers/Summit/Presentation/PresentationCategorySerializer.php b/app/ModelSerializers/Summit/Presentation/PresentationCategorySerializer.php index 3063b5be1..b1ace8379 100644 --- a/app/ModelSerializers/Summit/Presentation/PresentationCategorySerializer.php +++ b/app/ModelSerializers/Summit/Presentation/PresentationCategorySerializer.php @@ -105,7 +105,7 @@ public function serialize($expand = null, array $fields = [], array $relations = $allowed_access_levels[] = intval($access_level->getId()); } - $values['.'] = $allowed_access_levels; + $values['allowed_access_levels'] = $allowed_access_levels; } if(in_array('proposed_schedule_allowed_locations', $relations)) { From 483fb0e4eea7e502b9f667ddb5884c352865fbaa Mon Sep 17 00:00:00 2001 From: smarcet Date: Sat, 23 May 2026 00:13:55 -0300 Subject: [PATCH 04/17] feat(repository): extend DoctrineRepository::getAllByPage with N+1 batch-load support - Add $expands parameter to DoctrineRepository::getAllByPage - Add addExpandFetchJoins call (toOne FETCH JOINs before pagination) - Add batchLoadExpandedRelations call (toMany IN-clause batching after hydration) - Add protected static getExpandFieldMap() hook (returns [] by default, subclasses override) - Use static::getExpandFieldMap() for late static binding so subclass maps resolve correctly - Make DoctrineSummitEventRepository::getExpandFieldMap() public static to allow static call from tests - Propagate array $expands = [] signature to all concrete getAllByPage overrides and interfaces --- .../Main/Repositories/IMemberRepository.php | 2 +- app/Models/Utils/IBaseRepository.php | 2 +- app/Repositories/DoctrineRepository.php | 34 +++++++++++++++++-- ...octrineEndPointRateLimitByIPRepository.php | 2 +- .../Summit/DoctrineMemberRepository.php | 2 +- .../Summit/DoctrineSpeakerRepository.php | 2 +- ...rineSponsorExtraQuestionTypeRepository.php | 2 +- ...DoctrineSummitAttendeeTicketRepository.php | 2 +- .../Summit/DoctrineSummitEventRepository.php | 1 - ...SummitOrderExtraQuestionTypeRepository.php | 2 +- 10 files changed, 40 insertions(+), 11 deletions(-) diff --git a/app/Models/Foundation/Main/Repositories/IMemberRepository.php b/app/Models/Foundation/Main/Repositories/IMemberRepository.php index 94b1802cc..6bf71cbc4 100644 --- a/app/Models/Foundation/Main/Repositories/IMemberRepository.php +++ b/app/Models/Foundation/Main/Repositories/IMemberRepository.php @@ -49,7 +49,7 @@ public function getByFullName(string $fullname):?Member; * @param Order|null $order * @return PagingResponse */ - public function getAllByPage(PagingInfo $paging_info, Filter $filter = null, Order $order = null); + public function getAllByPage(PagingInfo $paging_info, Filter $filter = null, Order $order = null, array $expands = []); /** diff --git a/app/Models/Utils/IBaseRepository.php b/app/Models/Utils/IBaseRepository.php index ef00d2c61..1f3929811 100644 --- a/app/Models/Utils/IBaseRepository.php +++ b/app/Models/Utils/IBaseRepository.php @@ -63,7 +63,7 @@ public function getAll(); * @param Order|null $order * @return PagingResponse */ - public function getAllByPage(PagingInfo $paging_info, Filter $filter = null, Order $order = null); + public function getAllByPage(PagingInfo $paging_info, Filter $filter = null, Order $order = null, array $expands = []); /** * @param PagingInfo $paging_info diff --git a/app/Repositories/DoctrineRepository.php b/app/Repositories/DoctrineRepository.php index 23c4454db..7fd9b30d2 100644 --- a/app/Repositories/DoctrineRepository.php +++ b/app/Repositories/DoctrineRepository.php @@ -268,9 +268,11 @@ public function getParametrizedAllIdsByPage(callable $fnQuery,PagingInfo $paging * @param Order|null $order * @return PagingResponse */ - public function getAllByPage(PagingInfo $paging_info, Filter $filter = null, Order $order = null){ + public function getAllByPage(PagingInfo $paging_info, Filter $filter = null, Order $order = null, array $expands = []){ - $query = $this->getEntityManager() + $em = $this->getEntityManager(); + + $query = $em ->createQueryBuilder() ->select("e") ->from($this->getBaseEntity(), "e"); @@ -289,6 +291,18 @@ public function getAllByPage(PagingInfo $paging_info, Filter $filter = null, Ord $order->apply2Query($query, $this->getOrderMappings($filter)); } + // Fetch-join requested toOne associations into the hydration query + if (!empty($expands)) { + $query = $this->addExpandFetchJoins( + $em, + $query, + $expands, + 'e', + $this->getBaseEntity(), + static::getExpandFieldMap() + ); + } + $query= $query ->setFirstResult($paging_info->getOffset()) ->setMaxResults($paging_info->getPerPage()); @@ -300,6 +314,17 @@ public function getAllByPage(PagingInfo $paging_info, Filter $filter = null, Ord foreach($paginator as $entity) array_push($data, $entity); + // Batch-load toMany collections (level 1) and nested relations (level 2+) + if (!empty($expands) && !empty($data)) { + $this->batchLoadExpandedRelations( + $em, + $data, + $expands, + $this->getBaseEntity(), + static::getExpandFieldMap() + ); + } + return new PagingResponse ( $total, @@ -310,6 +335,11 @@ public function getAllByPage(PagingInfo $paging_info, Filter $filter = null, Ord ); } + protected static function getExpandFieldMap(): array + { + return []; + } + /** * @param Filter|null $filter * @param Order|null $order diff --git a/app/Repositories/ResourceServer/DoctrineEndPointRateLimitByIPRepository.php b/app/Repositories/ResourceServer/DoctrineEndPointRateLimitByIPRepository.php index e0e932494..fd6347849 100644 --- a/app/Repositories/ResourceServer/DoctrineEndPointRateLimitByIPRepository.php +++ b/app/Repositories/ResourceServer/DoctrineEndPointRateLimitByIPRepository.php @@ -125,7 +125,7 @@ public function getAll() * @param Order|null $order * @return PagingResponse */ - public function getAllByPage(PagingInfo $paging_info, Filter $filter = null, Order $order = null) + public function getAllByPage(PagingInfo $paging_info, Filter $filter = null, Order $order = null, array $expands = []) { // TODO: Implement getAllByPage() method. } diff --git a/app/Repositories/Summit/DoctrineMemberRepository.php b/app/Repositories/Summit/DoctrineMemberRepository.php index 8bc166e14..1c81a667c 100644 --- a/app/Repositories/Summit/DoctrineMemberRepository.php +++ b/app/Repositories/Summit/DoctrineMemberRepository.php @@ -496,7 +496,7 @@ public function getByEmail($email): ?Member * @param Order|null $order * @return PagingResponse */ - public function getAllByPage(PagingInfo $paging_info, Filter $filter = null, Order $order = null) + public function getAllByPage(PagingInfo $paging_info, Filter $filter = null, Order $order = null, array $expands = []) { return $this->getParametrizedAllByPage(function () { return $this->getEntityManager()->createQueryBuilder() diff --git a/app/Repositories/Summit/DoctrineSpeakerRepository.php b/app/Repositories/Summit/DoctrineSpeakerRepository.php index 272f7c86e..fd96eb7aa 100644 --- a/app/Repositories/Summit/DoctrineSpeakerRepository.php +++ b/app/Repositories/Summit/DoctrineSpeakerRepository.php @@ -987,7 +987,7 @@ public function getSpeakersBySummitAndOnSchedule(Summit $summit, PagingInfo $pag * @param Order|null $order * @return PagingResponse */ - public function getAllByPage(PagingInfo $paging_info, Filter $filter = null, Order $order = null) + public function getAllByPage(PagingInfo $paging_info, Filter $filter = null, Order $order = null, array $expands = []) { $extra_filters = ''; diff --git a/app/Repositories/Summit/DoctrineSponsorExtraQuestionTypeRepository.php b/app/Repositories/Summit/DoctrineSponsorExtraQuestionTypeRepository.php index 1bca6bab1..d604cc69a 100644 --- a/app/Repositories/Summit/DoctrineSponsorExtraQuestionTypeRepository.php +++ b/app/Repositories/Summit/DoctrineSponsorExtraQuestionTypeRepository.php @@ -87,7 +87,7 @@ protected function getBaseEntity() * @param Order|null $order * @return PagingResponse */ - public function getAllByPage(PagingInfo $paging_info, Filter $filter = null, Order $order = null): PagingResponse + public function getAllByPage(PagingInfo $paging_info, Filter $filter = null, Order $order = null, array $expands = []): PagingResponse { return $this->getParametrizedAllByPage(function () { return $this->getEntityManager()->createQueryBuilder() diff --git a/app/Repositories/Summit/DoctrineSummitAttendeeTicketRepository.php b/app/Repositories/Summit/DoctrineSummitAttendeeTicketRepository.php index 7c6a8e0a8..017281728 100644 --- a/app/Repositories/Summit/DoctrineSummitAttendeeTicketRepository.php +++ b/app/Repositories/Summit/DoctrineSummitAttendeeTicketRepository.php @@ -750,7 +750,7 @@ public function getAllTicketsIdsByOrder(int $order_id, PagingInfo $paging_info): * @return mixed|PagingResponse * @throws \Doctrine\DBAL\Exception */ - public function getAllByPage(PagingInfo $paging_info, Filter $filter = null, Order $order = null) + public function getAllByPage(PagingInfo $paging_info, Filter $filter = null, Order $order = null, array $expands = []) { $start = time(); diff --git a/app/Repositories/Summit/DoctrineSummitEventRepository.php b/app/Repositories/Summit/DoctrineSummitEventRepository.php index 4b62f5777..f6fa870a0 100644 --- a/app/Repositories/Summit/DoctrineSummitEventRepository.php +++ b/app/Repositories/Summit/DoctrineSummitEventRepository.php @@ -22,7 +22,6 @@ use Doctrine\ORM\Tools\Pagination\Paginator; use Illuminate\Support\Facades\Log; use models\main\Tag; -use models\summit\IOrderConstants; use models\summit\ISummitCategoryChangeStatus; use models\summit\ISummitEventRepository; use models\summit\Presentation; diff --git a/app/Repositories/Summit/DoctrineSummitOrderExtraQuestionTypeRepository.php b/app/Repositories/Summit/DoctrineSummitOrderExtraQuestionTypeRepository.php index ce5cbc468..cef2013e4 100644 --- a/app/Repositories/Summit/DoctrineSummitOrderExtraQuestionTypeRepository.php +++ b/app/Repositories/Summit/DoctrineSummitOrderExtraQuestionTypeRepository.php @@ -71,7 +71,7 @@ protected function getBaseEntity() * @param Order|null $order * @return PagingResponse */ - public function getAllByPage(PagingInfo $paging_info, Filter $filter = null, Order $order = null): PagingResponse + public function getAllByPage(PagingInfo $paging_info, Filter $filter = null, Order $order = null, array $expands = []): PagingResponse { return $this->getParametrizedAllByPage(function () { return $this->getEntityManager()->createQueryBuilder() From fd8eaa4e1d70870668c875fa859c5474996536e9 Mon Sep 17 00:00:00 2001 From: smarcet Date: Sat, 23 May 2026 00:39:58 -0300 Subject: [PATCH 05/17] chore(repository): replace time() with microtime(true) and add per-phase timing in getAllByPage Adds ms-precision breakdown: count query, id query, hydration query, batchLoad. Was using integer time() which masks sub-second differences. --- .../Summit/DoctrineSummitEventRepository.php | 25 +++++++++++++------ 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/app/Repositories/Summit/DoctrineSummitEventRepository.php b/app/Repositories/Summit/DoctrineSummitEventRepository.php index f6fa870a0..e1daced60 100644 --- a/app/Repositories/Summit/DoctrineSummitEventRepository.php +++ b/app/Repositories/Summit/DoctrineSummitEventRepository.php @@ -724,14 +724,22 @@ public function getAllIdsByPage(PagingInfo $paging_info, Filter $filter = null, public function getAllByPage(PagingInfo $paging_info, Filter $filter = null, Order $order = null, array $expands = []) { - $start = time(); + $start = microtime(true); Log::debug("DoctrineSummitEventRepository::getAllByPage"); $shuffleResults = !is_null($order) && $order->hasOrder("page_random"); if ($shuffleResults) $order->removeOrder("page_random"); + + $t0 = microtime(true); $total = $this->getFastCount($filter, $order); + Log::debug("DoctrineSummitEventRepository::getAllByPage count", ['ms' => round((microtime(true) - $t0) * 1000), 'total' => $total]); + + $t0 = microtime(true); $ids = $this->getAllIdsByPage($paging_info, $filter, $order); - Log::debug("DoctrineSummitEventRepository::getAllByPage ids", ['ids' => $ids]); - $query = $this->getEntityManager()->createQueryBuilder() + Log::debug("DoctrineSummitEventRepository::getAllByPage ids", ['ms' => round((microtime(true) - $t0) * 1000), 'ids' => $ids]); + + $em = $this->getEntityManager(); + + $query = $em->createQueryBuilder() ->select('e, p , et, et2') ->from($this->getBaseEntity(), "e") ->innerJoin("e.type", "et")->addSelect("et") @@ -740,8 +748,6 @@ public function getAllByPage(PagingInfo $paging_info, Filter $filter = null, Ord ->where('e.id IN (:ids)') ->setParameter('ids', $ids); - $em = $this->getEntityManager(); - // Fetch-join requested toOne associations into the hydration query if (!empty($expands)) { $query = $this->addExpandFetchJoins( @@ -756,7 +762,10 @@ public function getAllByPage(PagingInfo $paging_info, Filter $filter = null, Ord ); } + $t0 = microtime(true); $rows = $query->getQuery()->getResult(); + Log::debug("DoctrineSummitEventRepository::getAllByPage hydration", ['ms' => round((microtime(true) - $t0) * 1000)]); + $byId = []; foreach ($rows as $row) { $event = $row instanceof SummitEvent @@ -780,6 +789,7 @@ public function getAllByPage(PagingInfo $paging_info, Filter $filter = null, Ord // Batch-load toMany collections (level 1) and nested relations (level 2+) if (!empty($expands) && !empty($data)) { + $t0 = microtime(true); $this->batchLoadExpandedRelations( $em, $data, @@ -794,12 +804,13 @@ public function getAllByPage(PagingInfo $paging_info, Filter $filter = null, Ord : $assignment, ] ); + Log::debug("DoctrineSummitEventRepository::getAllByPage batchLoad", ['ms' => round((microtime(true) - $t0) * 1000)]); } if ($shuffleResults) shuffle($data); - $end = time() - $start; - Log::debug("DoctrineSummitEventRepository::getAllByPage", ['seconds'=>$end]); + $end = round((microtime(true) - $start) * 1000); + Log::debug("DoctrineSummitEventRepository::getAllByPage total", ['ms' => $end]); return new PagingResponse ( From 4857e00ff2c5ff4a4669540d2ce75b3f02672ac3 Mon Sep 17 00:00:00 2001 From: smarcet Date: Sat, 23 May 2026 00:51:29 -0300 Subject: [PATCH 06/17] fix(repository): remove redundant et2 join from hydration query in getAllByPage SummitEventType uses JOINED inheritance with a ClassName discriminator so Doctrine already hydrates the correct PresentationType subclass automatically via innerJoin e.type. The explicit leftJoin(PresentationType::class, et2) in the hydration query was redundant and caused Doctrine to emit PresentationType as a separate root entity in getResult(), silently dropping those events from the byId map and logging unexpected hydration row warnings. The et2 join is kept in getFastCount and getAllIdsByPage where it is needed for type_allows_attendee_vote and type_allows_custom_ordering filter predicates. --- app/Repositories/Summit/DoctrineSummitEventRepository.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/Repositories/Summit/DoctrineSummitEventRepository.php b/app/Repositories/Summit/DoctrineSummitEventRepository.php index e1daced60..b767521b7 100644 --- a/app/Repositories/Summit/DoctrineSummitEventRepository.php +++ b/app/Repositories/Summit/DoctrineSummitEventRepository.php @@ -740,10 +740,9 @@ public function getAllByPage(PagingInfo $paging_info, Filter $filter = null, Ord $em = $this->getEntityManager(); $query = $em->createQueryBuilder() - ->select('e, p , et, et2') + ->select('e, p, et') ->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") ->where('e.id IN (:ids)') ->setParameter('ids', $ids); From d655630d07c2a27a02a356c9396cc813de24d2e2 Mon Sep 17 00:00:00 2001 From: smarcet Date: Sat, 23 May 2026 01:01:30 -0300 Subject: [PATCH 07/17] fix(serializer): eliminate speaker lazy-loads and enable presentation cache in getEvents Three causes of ~1100ms serializer overhead: 1. Presentation::getSpeakers() used matching() on an EXTRA_LAZY collection which always issues a DB query regardless of whether batchLoadExpandedRelations had already pre-loaded the assignments. Replaced with toArray() + PHP usort so that an already-initialized collection (batch-loaded) is sorted in memory with zero DB round-trips. Falls back to a single full-load query on cold collections. 2. getSpeaker() on each PresentationSpeakerAssignment triggered one lazy-load per speaker because batchLoadExpandedRelations only pre-fetched the assignments, not the speaker FK. Injecting speakers.speaker into the batch expands causes batchLoadNestedExpands to load all unique PresentationSpeaker entities in one IN-clause query, so getSpeaker() returns the already-initialized identity-map entry. 3. getEvents() did not pass use_cache=true to the serializer params, so PresentationSerializer skipped its Redis cache entirely. Adding it means warm requests serialize from cache (Redis GET + JSON decode) instead of re-running the full field-mapping and expand loop per presentation. --- .../Summit/OAuth2SummitEventsApiController.php | 5 +++-- .../Summit/Events/Presentations/Presentation.php | 12 ++++++------ .../Summit/DoctrineSummitEventRepository.php | 9 ++++++++- 3 files changed, 17 insertions(+), 9 deletions(-) diff --git a/app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitEventsApiController.php b/app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitEventsApiController.php index 9a4ef5585..0941de2bf 100644 --- a/app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitEventsApiController.php +++ b/app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitEventsApiController.php @@ -184,8 +184,9 @@ public function getEvents($summit_id) SerializerUtils::getFields(), SerializerUtils::getRelations(), [ - 'current_user' => $current_user - ], + 'current_user' => $current_user, + 'use_cache' => true, + ], $this->getSerializerType() ) ); diff --git a/app/Models/Foundation/Summit/Events/Presentations/Presentation.php b/app/Models/Foundation/Summit/Events/Presentations/Presentation.php index 91ce9ae73..19e6392b5 100644 --- a/app/Models/Foundation/Summit/Events/Presentations/Presentation.php +++ b/app/Models/Foundation/Summit/Events/Presentations/Presentation.php @@ -412,12 +412,12 @@ public function getClassName(): string */ public function getSpeakers() { - $criteria = Criteria::create(); - $criteria->orderBy(['order' => 'ASC']); - $res = $this->speakers->matching($criteria); - return $res->map(function ($entity) { - return $entity->getSpeaker(); - }); + // toArray() uses in-memory data when the collection is already initialized (e.g. by + // batchLoadExpandedRelations), avoiding the per-call DB round-trip that EXTRA_LAZY + // + matching() always issues regardless of collection state. + $items = $this->speakers->toArray(); + usort($items, fn($a, $b) => $a->getOrder() <=> $b->getOrder()); + return new ArrayCollection(array_map(fn($e) => $e->getSpeaker(), $items)); } /** diff --git a/app/Repositories/Summit/DoctrineSummitEventRepository.php b/app/Repositories/Summit/DoctrineSummitEventRepository.php index b767521b7..899b84789 100644 --- a/app/Repositories/Summit/DoctrineSummitEventRepository.php +++ b/app/Repositories/Summit/DoctrineSummitEventRepository.php @@ -789,10 +789,17 @@ public function getAllByPage(PagingInfo $paging_info, Filter $filter = null, Ord // Batch-load toMany collections (level 1) and nested relations (level 2+) if (!empty($expands) && !empty($data)) { $t0 = microtime(true); + // When speakers are requested, also pre-load the speaker FK on each assignment so + // that Presentation::getSpeakers() → getSpeaker() returns an initialized entity + // instead of triggering one lazy load per speaker. + $batchExpands = $expands; + if (in_array('speakers', $batchExpands) && !in_array('speakers.speaker', $batchExpands)) { + $batchExpands[] = 'speakers.speaker'; + } $this->batchLoadExpandedRelations( $em, $data, - $expands, + $batchExpands, SummitEvent::class, self::$expandFieldMap, [ From 302a28deb8ca392e25e340736f36f7162537fcec Mon Sep 17 00:00:00 2001 From: smarcet Date: Sat, 23 May 2026 01:04:56 -0300 Subject: [PATCH 08/17] 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 29c5540a7..f31fbde16 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 967d7374f1ddce057fe5a14cdf5cbd62faf85cb2 Mon Sep 17 00:00:00 2001 From: smarcet Date: Sat, 23 May 2026 01:24:50 -0300 Subject: [PATCH 09/17] chore(debug): add serializer timing logs to getEvents and PagingResponse::toArray Adds: - OAuth2SummitEventsApiController::getEvents: logs total serializer ms after toArray() - PagingResponse::toArray: logs per-item entity id + ms to identify slow outliers Remove before merging to main. --- .../OAuth2SummitEventsApiController.php | 24 +++++++++---------- app/Http/Utils/PagingResponse.php | 3 +++ 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitEventsApiController.php b/app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitEventsApiController.php index 0941de2bf..110d7c373 100644 --- a/app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitEventsApiController.php +++ b/app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitEventsApiController.php @@ -176,20 +176,20 @@ public function getEvents($summit_id) $strategy = new RetrieveAllSummitEventsBySummitStrategy($this->repository, $this->event_repository, $this->resource_server_context); $expand = SerializerUtils::getExpand(); $response = $strategy->getEvents(['summit_id' => $summit_id, 'expand' => $expand]); - return $this->ok + $t0 = microtime(true); + $data = $response->toArray ( - $response->toArray - ( - $expand, - SerializerUtils::getFields(), - SerializerUtils::getRelations(), - [ - 'current_user' => $current_user, - 'use_cache' => true, - ], - $this->getSerializerType() - ) + $expand, + SerializerUtils::getFields(), + SerializerUtils::getRelations(), + [ + 'current_user' => $current_user, + 'use_cache' => true, + ], + $this->getSerializerType() ); + Log::debug('OAuth2SummitEventsApiController::getEvents serializer', ['ms' => round((microtime(true) - $t0) * 1000)]); + return $this->ok($data); }); }); diff --git a/app/Http/Utils/PagingResponse.php b/app/Http/Utils/PagingResponse.php index 1cf9a2f87..dd62240e3 100644 --- a/app/Http/Utils/PagingResponse.php +++ b/app/Http/Utils/PagingResponse.php @@ -111,7 +111,10 @@ public function toArray($expand = null, array $fields = [], array $relations = [ { if($i instanceof IEntity) { + $t0 = microtime(true); + $id = method_exists($i, 'getId') ? $i->getId() : null; $i = SerializerRegistry::getInstance()->getSerializer($i, $serializer_type)->serialize($expand, $fields, $relations, $params); + \Illuminate\Support\Facades\Log::debug('PagingResponse::toArray item', ['id' => $id, 'ms' => round((microtime(true) - $t0) * 1000)]); } $items[] = $i; } From ca2ca3b49d3684b8d0160bccdd869cafb7b3cadc Mon Sep 17 00:00:00 2001 From: smarcet Date: Sat, 23 May 2026 01:38:43 -0300 Subject: [PATCH 10/17] fix(serializer): force-initialize EXTRA_LAZY collections and pre-load speaker Member MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two remaining N+1 sources on the cold-cache path: 1. GraphLoaderTrait::batchHydrateCollections ran the fetch-join DQL correctly but Doctrine's ObjectHydrator does not always mark EXTRA_LAZY PersistentCollections as initialized. Subsequent iteration (e.g. getSponsors(), getTags()) therefore still fired one SELECT per entity per collection. After getResult() returns, explicitly call setInitialized(true) on each root entity's collection via ClassMetadata::reflFields — valid because the DQL loaded ALL elements for these root IDs into the UnitOfWork. 2. PresentationSpeaker::getFirstName()/getLastName() fall back to $this->member->getFirstName() when the speaker's own field is empty, triggering one Member lazy-load per speaker. Outlier events (7570: 462ms, 7573: 322ms) had many such speakers. Adding speakers.speaker.member to the internal batch expands causes batchLoadNestedExpands to load all Member entities in one IN-clause query. --- .../Summit/DoctrineSummitEventRepository.php | 12 ++++++++++-- app/libs/Utils/Doctrine/GraphLoaderTrait.php | 18 ++++++++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/app/Repositories/Summit/DoctrineSummitEventRepository.php b/app/Repositories/Summit/DoctrineSummitEventRepository.php index 899b84789..eba8943dd 100644 --- a/app/Repositories/Summit/DoctrineSummitEventRepository.php +++ b/app/Repositories/Summit/DoctrineSummitEventRepository.php @@ -793,8 +793,16 @@ public function getAllByPage(PagingInfo $paging_info, Filter $filter = null, Ord // that Presentation::getSpeakers() → getSpeaker() returns an initialized entity // instead of triggering one lazy load per speaker. $batchExpands = $expands; - if (in_array('speakers', $batchExpands) && !in_array('speakers.speaker', $batchExpands)) { - $batchExpands[] = 'speakers.speaker'; + if (in_array('speakers', $batchExpands)) { + // Pre-load the speaker entity on each assignment so getSpeaker() hits + // the identity map instead of lazy-loading per assignment. + if (!in_array('speakers.speaker', $batchExpands)) + $batchExpands[] = 'speakers.speaker'; + // Pre-load the Member on each PresentationSpeaker so that + // getFirstName()/getLastName() fallback to member->getXxx() doesn't + // trigger one lazy-load per speaker when the speaker's own field is empty. + if (!in_array('speakers.speaker.member', $batchExpands)) + $batchExpands[] = 'speakers.speaker.member'; } $this->batchLoadExpandedRelations( $em, diff --git a/app/libs/Utils/Doctrine/GraphLoaderTrait.php b/app/libs/Utils/Doctrine/GraphLoaderTrait.php index 7a56bcf8f..0cf48aab3 100644 --- a/app/libs/Utils/Doctrine/GraphLoaderTrait.php +++ b/app/libs/Utils/Doctrine/GraphLoaderTrait.php @@ -325,6 +325,24 @@ protected function batchHydrateCollections( $em->createQuery($dql) ->setParameter('ids', $rootIds) ->getResult(); + + // Doctrine's ObjectHydrator populates EXTRA_LAZY collections during hydration + // but does not always mark them as initialized, so subsequent iteration still + // fires a per-entity SELECT. Force-initialize each collection now that we know + // all its elements for these root IDs have been loaded into the UnitOfWork. + $meta = $em->getClassMetadata($entityClass); + $reflField = $meta->reflFields[$collection] ?? null; + if ($reflField) { + $uow = $em->getUnitOfWork(); + foreach ($rootIds as $id) { + $entity = $uow->tryGetById([$meta->getSingleIdentifierFieldName() => $id], $meta); + if (!$entity) continue; + $coll = $reflField->getValue($entity); + if ($coll instanceof \Doctrine\ORM\PersistentCollection && !$coll->isInitialized()) { + $coll->setInitialized(true); + } + } + } } catch (\Exception $ex) { Log::warning("GraphLoaderTrait::batchHydrateCollections failed for {$entityClass}::{$collection}", [ 'error' => $ex->getMessage(), From 72b3063999d28dcf1d1bdff830329280e245e079 Mon Sep 17 00:00:00 2001 From: smarcet Date: Sat, 23 May 2026 01:46:30 -0300 Subject: [PATCH 11/17] fix(graph-loader): use em->find() instead of tryGetById for EXTRA_LAZY force-init MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit tryGetById() was returning false for all entities (API signature mismatch with Doctrine version in use), so setInitialized(true) was never called and the EXTRA_LAZY collections remained uninitialized — iteration still fired per-entity DB queries on every request for non-Presentation events (sponsors, tags). em->find() with an entity already in the identity map (guaranteed after the main hydration query runs) costs zero DB queries and correctly returns the entity so we can force-initialize its PersistentCollection. --- app/libs/Utils/Doctrine/GraphLoaderTrait.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/libs/Utils/Doctrine/GraphLoaderTrait.php b/app/libs/Utils/Doctrine/GraphLoaderTrait.php index 0cf48aab3..3b6604116 100644 --- a/app/libs/Utils/Doctrine/GraphLoaderTrait.php +++ b/app/libs/Utils/Doctrine/GraphLoaderTrait.php @@ -330,12 +330,13 @@ protected function batchHydrateCollections( // but does not always mark them as initialized, so subsequent iteration still // fires a per-entity SELECT. Force-initialize each collection now that we know // all its elements for these root IDs have been loaded into the UnitOfWork. + // Use $em->find() — if the entity is already in the identity map (it will be, + // since the main hydration query ran first) this costs zero DB queries. $meta = $em->getClassMetadata($entityClass); $reflField = $meta->reflFields[$collection] ?? null; if ($reflField) { - $uow = $em->getUnitOfWork(); foreach ($rootIds as $id) { - $entity = $uow->tryGetById([$meta->getSingleIdentifierFieldName() => $id], $meta); + $entity = $em->find($entityClass, $id); if (!$entity) continue; $coll = $reflField->getValue($entity); if ($coll instanceof \Doctrine\ORM\PersistentCollection && !$coll->isInitialized()) { From a447c2880a0adb5ddc3ba1e569a81741d79842ff Mon Sep 17 00:00:00 2001 From: smarcet Date: Sat, 23 May 2026 01:54:30 -0300 Subject: [PATCH 12/17] fix(graph-loader): correct tryGetById call for EXTRA_LAZY collection force-init Previous attempt passed ClassMetadata as second arg (must be FQCN string) and em->find() for JOINED-inheritance entities was issuing SELECT queries even for in-identity-map entities, making times worse. Correct call: tryGetById(scalar_id, meta->rootEntityName) - meta->rootEntityName is the root class FQCN string for the hierarchy - Doctrine hashes identity map keys as implode(' ', (array)$id) so passing the scalar id directly is correct - Zero DB queries: reads only the UnitOfWork identity map --- app/libs/Utils/Doctrine/GraphLoaderTrait.php | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/app/libs/Utils/Doctrine/GraphLoaderTrait.php b/app/libs/Utils/Doctrine/GraphLoaderTrait.php index 3b6604116..4bb3bf5db 100644 --- a/app/libs/Utils/Doctrine/GraphLoaderTrait.php +++ b/app/libs/Utils/Doctrine/GraphLoaderTrait.php @@ -330,13 +330,17 @@ protected function batchHydrateCollections( // but does not always mark them as initialized, so subsequent iteration still // fires a per-entity SELECT. Force-initialize each collection now that we know // all its elements for these root IDs have been loaded into the UnitOfWork. - // Use $em->find() — if the entity is already in the identity map (it will be, - // since the main hydration query ran first) this costs zero DB queries. + // tryGetById() reads the UnitOfWork identity map with zero DB queries. + // Key details: second arg must be the root entity class name string (not + // ClassMetadata), and the id must be a scalar — Doctrine hashes it with + // implode(' ', (array)$id), so passing the scalar directly is correct. $meta = $em->getClassMetadata($entityClass); $reflField = $meta->reflFields[$collection] ?? null; if ($reflField) { + $uow = $em->getUnitOfWork(); + $rootEntityName = $meta->rootEntityName; foreach ($rootIds as $id) { - $entity = $em->find($entityClass, $id); + $entity = $uow->tryGetById($id, $rootEntityName); if (!$entity) continue; $coll = $reflField->getValue($entity); if ($coll instanceof \Doctrine\ORM\PersistentCollection && !$coll->isInitialized()) { From 81615e1429a2ddcf2cb80d069aeb7e8dd07278dd Mon Sep 17 00:00:00 2001 From: smarcet Date: Sat, 23 May 2026 02:08:11 -0300 Subject: [PATCH 13/17] fix(serializer): pre-load PresentationSpeaker + Member in single DQL query MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The generic nested-expand recursion (speakers.speaker / speakers.speaker.member) could not batch-load the speaker FK because the speakers resolver ($assignment->getSpeaker()) is invoked during the iteration step to gather child entities — that step itself fires one EXTRA_LAZY lazy-load per assignment before the batch-load can run. The recursion then never finds a 'speaker' field on PresentationSpeaker and silently no-ops. Replace the broken nested batch with a single direct DQL: SELECT s, m FROM PresentationSpeakerAssignment a JOIN a.speaker s LEFT JOIN s.member m WHERE a.presentation IN (:ids) This loads ALL PresentationSpeaker entities AND their Member entities for the requested presentations in one query, regardless of how many speakers each has. After this preload runs: - $assignment->getSpeaker() hits the identity map (zero queries) - $speaker->getMember() returns an already-initialized entity - PresentationSpeaker::getFirstName()/getLastName() fallback to $this->member->getFirstName() costs nothing Removed the now-useless speakers.speaker + speakers.speaker.member additions to $batchExpands since they only contributed wasted N lazy-loads via the resolver. --- .../Summit/DoctrineSummitEventRepository.php | 46 ++++++++++++------- 1 file changed, 30 insertions(+), 16 deletions(-) diff --git a/app/Repositories/Summit/DoctrineSummitEventRepository.php b/app/Repositories/Summit/DoctrineSummitEventRepository.php index eba8943dd..ec3bc98e9 100644 --- a/app/Repositories/Summit/DoctrineSummitEventRepository.php +++ b/app/Repositories/Summit/DoctrineSummitEventRepository.php @@ -789,25 +789,10 @@ public function getAllByPage(PagingInfo $paging_info, Filter $filter = null, Ord // Batch-load toMany collections (level 1) and nested relations (level 2+) if (!empty($expands) && !empty($data)) { $t0 = microtime(true); - // When speakers are requested, also pre-load the speaker FK on each assignment so - // that Presentation::getSpeakers() → getSpeaker() returns an initialized entity - // instead of triggering one lazy load per speaker. - $batchExpands = $expands; - if (in_array('speakers', $batchExpands)) { - // Pre-load the speaker entity on each assignment so getSpeaker() hits - // the identity map instead of lazy-loading per assignment. - if (!in_array('speakers.speaker', $batchExpands)) - $batchExpands[] = 'speakers.speaker'; - // Pre-load the Member on each PresentationSpeaker so that - // getFirstName()/getLastName() fallback to member->getXxx() doesn't - // trigger one lazy-load per speaker when the speaker's own field is empty. - if (!in_array('speakers.speaker.member', $batchExpands)) - $batchExpands[] = 'speakers.speaker.member'; - } $this->batchLoadExpandedRelations( $em, $data, - $batchExpands, + $expands, SummitEvent::class, self::$expandFieldMap, [ @@ -819,6 +804,35 @@ public function getAllByPage(PagingInfo $paging_info, Filter $filter = null, Ord ] ); Log::debug("DoctrineSummitEventRepository::getAllByPage batchLoad", ['ms' => round((microtime(true) - $t0) * 1000)]); + + // Pre-load PresentationSpeaker + Member in a single DQL query when speakers + // are requested. The generic nested-expand recursion can't do this for the + // speakers path because its resolver ($assignment->getSpeaker()) short-circuits + // before the toOne batch-load step. Without this, $assignment->getSpeaker() + // fires one lazy-load per assignment (EXTRA_LAZY ManyToOne), and the speaker + // serializer's getFirstName()/getLastName() falls back to $this->member->getX() + // for another lazy-load per speaker. Loading all three (assignment.speaker + // + speaker.member) in one query collapses those N+1 chains into one query. + if (in_array('speakers', $expands)) { + $t1 = microtime(true); + $presIds = []; + foreach ($data as $event) { + if ($event instanceof Presentation) $presIds[] = $event->getId(); + } + if (!empty($presIds)) { + try { + $em->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', $presIds)->getResult(); + } catch (\Exception $ex) { + Log::warning("DoctrineSummitEventRepository::getAllByPage speakers+members preload failed", ['error' => $ex->getMessage()]); + } + } + Log::debug("DoctrineSummitEventRepository::getAllByPage speakerMemberPreload", ['ms' => round((microtime(true) - $t1) * 1000)]); + } } if ($shuffleResults) shuffle($data); From b3b9eb2d7063d0da5edd017d579b442fc4ae1ef4 Mon Sep 17 00:00:00 2001 From: smarcet Date: Sat, 23 May 2026 02:12:27 -0300 Subject: [PATCH 14/17] feat(serializer): add Redis cache to SummitEventSerializer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Non-Presentation events (SummitEvent, SummitGroupEvent, SummitEventWithFile) had no caching at all and went through full serialization (~60-170ms per event) on every request. Mirror the PresentationSerializer cache pattern: - Cache key includes static::class so each subclass (SummitGroupEventSerializer, SummitEventWithFileSerializer) gets its own namespace and they don't collide - last_edited timestamp in the key gives automatic invalidation on entity update without needing explicit Cache::forget anywhere - Skipped when $this instanceof PresentationSerializer — Presentations already have their own, more specific cache at the presentation level, and double caching would just waste Redis space - 1200s TTL matches PresentationSerializer Expected: SummitGroupEvent and plain-SummitEvent rows drop from 60-170ms to ~6-12ms on warm cache (matching Presentation cache behavior). --- .../Summit/SummitEventSerializer.php | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/app/ModelSerializers/Summit/SummitEventSerializer.php b/app/ModelSerializers/Summit/SummitEventSerializer.php index 0000549c3..035c1f58d 100644 --- a/app/ModelSerializers/Summit/SummitEventSerializer.php +++ b/app/ModelSerializers/Summit/SummitEventSerializer.php @@ -12,6 +12,8 @@ * limitations under the License. **/ +use Illuminate\Support\Facades\Cache; +use Illuminate\Support\Facades\Log; use Libs\ModelSerializers\AbstractSerializer; use models\summit\SummitEvent; @@ -21,6 +23,8 @@ */ class SummitEventSerializer extends SilverStripeSerializer { + const CacheTTL = 1200; + protected static $array_mappings = [ 'Title' => 'title:json_string', @@ -136,6 +140,30 @@ public function serialize $event = $this->object; if (!$event instanceof SummitEvent) return []; + // Cache opt-in via params. Skipped for PresentationSerializer (and its + // subclasses) because PresentationSerializer has its own, more specific + // cache at the presentation level. static::class is included in the key + // so each SummitEvent subclass (SummitGroupEventSerializer, ...) caches + // independently and doesn't collide with siblings. + $use_cache = ($params['use_cache'] ?? false) + && !($this instanceof PresentationSerializer); + $cache_key = null; + if ($use_cache) { + $cache_key = sprintf( + "public_summit_event_%s_%s_%s_%s_%s_%s", + static::class, + $event->getId(), + $event->getLastEditedUTC()?->getTimestamp() ?? 0, + $expand ?? "", + implode(",", $fields), + implode(",", $relations) + ); + if (Cache::has($cache_key)) { + Log::debug(sprintf("SummitEventSerializer::serialize cache hit for event %s", $event->getId())); + return json_decode(Cache::get($cache_key), true); + } + } + $values = parent::serialize($expand, $fields, $relations, $params); if (in_array('sponsors', $relations)) @@ -337,6 +365,10 @@ public function serialize } } + if ($use_cache && $cache_key !== null) { + Cache::put($cache_key, json_encode($values), self::CacheTTL); + } + return $values; } } \ No newline at end of file From 4b009bd99a84a715d42b6483e33de699172703e4 Mon Sep 17 00:00:00 2001 From: smarcet Date: Sat, 23 May 2026 02:24:53 -0300 Subject: [PATCH 15/17] fix(serializer): include media_uploads serializer type in cache key, skip re-serialization on hit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The cache-hit path was re-serializing the full media_uploads collection every time because getMediaUploadsSerializerType() depends on the current user role and the cached value could have been written by a request with a different role. Trace 885fa70613349e23d4f6719e36b8a513 from dev showed all 10 events as cache HITS yet still totaled 1274ms — the re-serialization of media_uploads (10+ items per event with sub-serializer calls and possible lazy-loads) was the remaining cost. Fix: include getMediaUploadsSerializerType() in the cache key. Now each user role gets its own cache entry containing fully-serialized media_uploads, and the cache-hit path just decodes JSON and returns — no per-hit re-serialization. --- .../Presentation/PresentationSerializer.php | 32 ++++--------------- 1 file changed, 6 insertions(+), 26 deletions(-) diff --git a/app/ModelSerializers/Summit/Presentation/PresentationSerializer.php b/app/ModelSerializers/Summit/Presentation/PresentationSerializer.php index f31fbde16..c24ccd2ac 100644 --- a/app/ModelSerializers/Summit/Presentation/PresentationSerializer.php +++ b/app/ModelSerializers/Summit/Presentation/PresentationSerializer.php @@ -106,12 +106,17 @@ public function serialize($expand = null, array $fields = [], array $relations = // 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. + // Include media_uploads serializer type so different user roles (Public/Admin) get + // their own cache entry — this lets us cache the full serialized media_uploads array + // and skip per-cache-hit re-serialization, which was the dominant cost on the cache-hit + // path for presentations with many media uploads. $key = sprintf ( - "public_presentation_%s_%s_%s_%s_%s", + "public_presentation_%s_%s_%s_%s_%s_%s", $presentation->getId(), $presentation->getLastEditedUTC()?->getTimestamp() ?? 0, + $this->getMediaUploadsSerializerType(), $expand ?? "", implode(",",$fields), implode(",", $relations) @@ -122,31 +127,6 @@ public function serialize($expand = null, array $fields = [], array $relations = if($use_cache && Cache::has($key)){ $values = json_decode(Cache::get($key), true); Log::debug(sprintf("PresentationSerializer::serialize cache hit for presentation %s", $presentation->getId())); - if (!empty($expand)) { - foreach (explode(',', $expand) as $relation) { - $relation = trim($relation); - switch ($relation) { - case 'media_uploads': - { - $media_uploads = []; - - foreach ($presentation->getMediaUploads() as $mediaUpload) { - $media_uploads[] = SerializerRegistry::getInstance()->getSerializer - ( - $mediaUpload, $this->getMediaUploadsSerializerType() - )->serialize - ( - AbstractSerializer::filterExpandByPrefix($expand, $relation), - AbstractSerializer::filterFieldsByPrefix($fields, $relation), - AbstractSerializer::filterFieldsByPrefix($relations, $relation), - ); - } - - $values['media_uploads'] = $media_uploads; - } - } - } - } return $values; } From 54af823efa1f3d6b04bc6367f25ed0f2736b90a7 Mon Sep 17 00:00:00 2001 From: smarcet Date: Sat, 23 May 2026 02:33:31 -0300 Subject: [PATCH 16/17] chore(debug): remove per-item and per-phase diagnostic logging The diagnostic logging added for finding the cache bottleneck was itself adding significant overhead per request (15-30 extra Log::debug writes for a 10-item page, each with JSON-encoded context). On a verbose-logging dev server this could easily add 50-200ms of pure logging overhead per request. Removed: - PagingResponse::toArray per-item id/ms log - OAuth2SummitEventsApiController::getEvents serializer ms log - DoctrineSummitEventRepository::getAllByPage per-phase logs (count, ids, hydration, batchLoad, speakerMemberPreload); kept just the final total ms log - PresentationSerializer cache-hit log (fires on every cache hit) - SummitEventSerializer cache-hit log (fires on every cache hit) Behaviour is unchanged; only the logging is removed. --- .../OAuth2SummitEventsApiController.php | 24 +++++++++---------- app/Http/Utils/PagingResponse.php | 3 --- .../Presentation/PresentationSerializer.php | 4 +--- .../Summit/SummitEventSerializer.php | 1 - .../Summit/DoctrineSummitEventRepository.php | 14 +---------- 5 files changed, 14 insertions(+), 32 deletions(-) diff --git a/app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitEventsApiController.php b/app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitEventsApiController.php index 110d7c373..0941de2bf 100644 --- a/app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitEventsApiController.php +++ b/app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitEventsApiController.php @@ -176,20 +176,20 @@ public function getEvents($summit_id) $strategy = new RetrieveAllSummitEventsBySummitStrategy($this->repository, $this->event_repository, $this->resource_server_context); $expand = SerializerUtils::getExpand(); $response = $strategy->getEvents(['summit_id' => $summit_id, 'expand' => $expand]); - $t0 = microtime(true); - $data = $response->toArray + return $this->ok ( - $expand, - SerializerUtils::getFields(), - SerializerUtils::getRelations(), - [ - 'current_user' => $current_user, - 'use_cache' => true, - ], - $this->getSerializerType() + $response->toArray + ( + $expand, + SerializerUtils::getFields(), + SerializerUtils::getRelations(), + [ + 'current_user' => $current_user, + 'use_cache' => true, + ], + $this->getSerializerType() + ) ); - Log::debug('OAuth2SummitEventsApiController::getEvents serializer', ['ms' => round((microtime(true) - $t0) * 1000)]); - return $this->ok($data); }); }); diff --git a/app/Http/Utils/PagingResponse.php b/app/Http/Utils/PagingResponse.php index dd62240e3..1cf9a2f87 100644 --- a/app/Http/Utils/PagingResponse.php +++ b/app/Http/Utils/PagingResponse.php @@ -111,10 +111,7 @@ public function toArray($expand = null, array $fields = [], array $relations = [ { if($i instanceof IEntity) { - $t0 = microtime(true); - $id = method_exists($i, 'getId') ? $i->getId() : null; $i = SerializerRegistry::getInstance()->getSerializer($i, $serializer_type)->serialize($expand, $fields, $relations, $params); - \Illuminate\Support\Facades\Log::debug('PagingResponse::toArray item', ['id' => $id, 'ms' => round((microtime(true) - $t0) * 1000)]); } $items[] = $i; } diff --git a/app/ModelSerializers/Summit/Presentation/PresentationSerializer.php b/app/ModelSerializers/Summit/Presentation/PresentationSerializer.php index c24ccd2ac..1160abdb7 100644 --- a/app/ModelSerializers/Summit/Presentation/PresentationSerializer.php +++ b/app/ModelSerializers/Summit/Presentation/PresentationSerializer.php @@ -125,9 +125,7 @@ public function serialize($expand = null, array $fields = [], array $relations = $use_cache = $params['use_cache'] ?? false; if($use_cache && Cache::has($key)){ - $values = json_decode(Cache::get($key), true); - Log::debug(sprintf("PresentationSerializer::serialize cache hit for presentation %s", $presentation->getId())); - return $values; + return json_decode(Cache::get($key), true); } $values = parent::serialize($expand, $fields, $relations, $params); diff --git a/app/ModelSerializers/Summit/SummitEventSerializer.php b/app/ModelSerializers/Summit/SummitEventSerializer.php index 035c1f58d..8bdb65474 100644 --- a/app/ModelSerializers/Summit/SummitEventSerializer.php +++ b/app/ModelSerializers/Summit/SummitEventSerializer.php @@ -159,7 +159,6 @@ public function serialize implode(",", $relations) ); if (Cache::has($cache_key)) { - Log::debug(sprintf("SummitEventSerializer::serialize cache hit for event %s", $event->getId())); return json_decode(Cache::get($cache_key), true); } } diff --git a/app/Repositories/Summit/DoctrineSummitEventRepository.php b/app/Repositories/Summit/DoctrineSummitEventRepository.php index ec3bc98e9..31e075171 100644 --- a/app/Repositories/Summit/DoctrineSummitEventRepository.php +++ b/app/Repositories/Summit/DoctrineSummitEventRepository.php @@ -725,17 +725,11 @@ public function getAllByPage(PagingInfo $paging_info, Filter $filter = null, Ord { $start = microtime(true); - Log::debug("DoctrineSummitEventRepository::getAllByPage"); $shuffleResults = !is_null($order) && $order->hasOrder("page_random"); if ($shuffleResults) $order->removeOrder("page_random"); - $t0 = microtime(true); $total = $this->getFastCount($filter, $order); - Log::debug("DoctrineSummitEventRepository::getAllByPage count", ['ms' => round((microtime(true) - $t0) * 1000), 'total' => $total]); - - $t0 = microtime(true); $ids = $this->getAllIdsByPage($paging_info, $filter, $order); - Log::debug("DoctrineSummitEventRepository::getAllByPage ids", ['ms' => round((microtime(true) - $t0) * 1000), 'ids' => $ids]); $em = $this->getEntityManager(); @@ -761,9 +755,7 @@ public function getAllByPage(PagingInfo $paging_info, Filter $filter = null, Ord ); } - $t0 = microtime(true); $rows = $query->getQuery()->getResult(); - Log::debug("DoctrineSummitEventRepository::getAllByPage hydration", ['ms' => round((microtime(true) - $t0) * 1000)]); $byId = []; foreach ($rows as $row) { @@ -788,7 +780,6 @@ public function getAllByPage(PagingInfo $paging_info, Filter $filter = null, Ord // Batch-load toMany collections (level 1) and nested relations (level 2+) if (!empty($expands) && !empty($data)) { - $t0 = microtime(true); $this->batchLoadExpandedRelations( $em, $data, @@ -803,7 +794,6 @@ public function getAllByPage(PagingInfo $paging_info, Filter $filter = null, Ord : $assignment, ] ); - Log::debug("DoctrineSummitEventRepository::getAllByPage batchLoad", ['ms' => round((microtime(true) - $t0) * 1000)]); // Pre-load PresentationSpeaker + Member in a single DQL query when speakers // are requested. The generic nested-expand recursion can't do this for the @@ -814,7 +804,6 @@ public function getAllByPage(PagingInfo $paging_info, Filter $filter = null, Ord // for another lazy-load per speaker. Loading all three (assignment.speaker // + speaker.member) in one query collapses those N+1 chains into one query. if (in_array('speakers', $expands)) { - $t1 = microtime(true); $presIds = []; foreach ($data as $event) { if ($event instanceof Presentation) $presIds[] = $event->getId(); @@ -831,14 +820,13 @@ public function getAllByPage(PagingInfo $paging_info, Filter $filter = null, Ord Log::warning("DoctrineSummitEventRepository::getAllByPage speakers+members preload failed", ['error' => $ex->getMessage()]); } } - Log::debug("DoctrineSummitEventRepository::getAllByPage speakerMemberPreload", ['ms' => round((microtime(true) - $t1) * 1000)]); } } if ($shuffleResults) shuffle($data); $end = round((microtime(true) - $start) * 1000); - Log::debug("DoctrineSummitEventRepository::getAllByPage total", ['ms' => $end]); + Log::debug("DoctrineSummitEventRepository::getAllByPage", ['ms' => $end]); return new PagingResponse ( From d3bed78b96260a4c06be819dc32cfa3ed4db9883 Mon Sep 17 00:00:00 2001 From: smarcet Date: Sat, 23 May 2026 02:45:25 -0300 Subject: [PATCH 17/17] fix(serializer): collapse Cache::has+Cache::get into single Cache::get for cache lookup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously each cache hit fired two Redis round-trips (EXISTS then GET). On a network where Redis RTT is ~5ms each, that's ~10ms per cache lookup. For a 10-item page where every event is a cache hit, that's ~100ms of pure Redis network overhead. Replace with a single Cache::get and a null check — Redis returns null when the key does not exist, so we get the same semantics in one round-trip. --- .../Summit/Presentation/PresentationSerializer.php | 7 +++++-- app/ModelSerializers/Summit/SummitEventSerializer.php | 5 +++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/app/ModelSerializers/Summit/Presentation/PresentationSerializer.php b/app/ModelSerializers/Summit/Presentation/PresentationSerializer.php index 1160abdb7..7a017668f 100644 --- a/app/ModelSerializers/Summit/Presentation/PresentationSerializer.php +++ b/app/ModelSerializers/Summit/Presentation/PresentationSerializer.php @@ -124,8 +124,11 @@ public function serialize($expand = null, array $fields = [], array $relations = $use_cache = $params['use_cache'] ?? false; - if($use_cache && Cache::has($key)){ - return json_decode(Cache::get($key), true); + if($use_cache){ + $cached = Cache::get($key); + if ($cached !== null) { + return json_decode($cached, true); + } } $values = parent::serialize($expand, $fields, $relations, $params); diff --git a/app/ModelSerializers/Summit/SummitEventSerializer.php b/app/ModelSerializers/Summit/SummitEventSerializer.php index 8bdb65474..336b71c3d 100644 --- a/app/ModelSerializers/Summit/SummitEventSerializer.php +++ b/app/ModelSerializers/Summit/SummitEventSerializer.php @@ -158,8 +158,9 @@ public function serialize implode(",", $fields), implode(",", $relations) ); - if (Cache::has($cache_key)) { - return json_decode(Cache::get($cache_key), true); + $cached = Cache::get($cache_key); + if ($cached !== null) { + return json_decode($cached, true); } }