Skip to content

Commit 20291cc

Browse files
teohhanhuiteohhanhui
committed
Don't use fetchJoinCollection on Doctrine Paginator when not needed
Refactor QueryChecker etc.
1 parent 66ee1ba commit 20291cc

File tree

10 files changed

+969
-330
lines changed

10 files changed

+969
-330
lines changed

‎composer.json

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,6 @@
4444
"jangregor/phpstan-prophecy": "^0.3",
4545
"justinrainbow/json-schema": "^5.0",
4646
"nelmio/api-doc-bundle": "^2.13.3",
47-
"php-mock/php-mock-phpunit": "^2.0",
4847
"phpdocumentor/reflection-docblock": "^3.0 || ^4.0",
4948
"phpdocumentor/type-resolver": "^0.3 || ^0.4",
5049
"phpspec/prophecy": "^1.8",

‎phpstan.neon.dist

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,9 @@ parameters:
6767
path: %currentWorkingDirectory%/src/Test/DoctrineMongoDbOdmFilterTestCase.php
6868
- '#Method ApiPlatform\\Core\\(Serializer\\Abstract|JsonApi\\Serializer\\)ItemNormalizer::normalizeRelation\(\) should return array\|string but returns array\|bool\|float\|int\|string\.#'
6969
- '#Method ApiPlatform\\Core\\Util\\RequestParser::parseRequestParams\(\) should return array but returns array\|false\.#'
70+
-
71+
message: '#Method ApiPlatform\\Core\\Bridge\\Doctrine\\Orm\\Util\\QueryBuilderHelper::mapJoinAliases() should return array<string, array<string>\|string> but returns array<int|string, mixed>\.#'
72+
path: %currentWorkingDirectory%/src/Bridge/Doctrine/Orm/Util/QueryBuilderHelper.php
7073
- "#Call to method PHPUnit\\\\Framework\\\\Assert::assertSame\\(\\) with array\\('(collection_context|item_context|subresource_context)'\\) and array<Symfony\\\\Component\\\\VarDumper\\\\Cloner\\\\Data>\\|bool\\|float\\|int\\|string\\|null will always evaluate to false\\.#"
7174
# https://github.com/symfony/symfony/pull/30535
7275
-

‎src/Bridge/Doctrine/Orm/Extension/PaginationExtension.php

Lines changed: 42 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -151,8 +151,8 @@ public function getResult(QueryBuilder $queryBuilder, string $resourceClass = nu
151151
$query->setHint(CountWalker::HINT_DISTINCT, false);
152152
}
153153

154-
$doctrineOrmPaginator = new DoctrineOrmPaginator($query, $this->useFetchJoinCollection($queryBuilder, $resourceClass, $operationName));
155-
$doctrineOrmPaginator->setUseOutputWalkers($this->useOutputWalkers($queryBuilder));
154+
$doctrineOrmPaginator = new DoctrineOrmPaginator($query, $this->shouldDoctrinePaginatorFetchJoinCollection($queryBuilder, $resourceClass, $operationName));
155+
$doctrineOrmPaginator->setUseOutputWalkers($this->shouldDoctrinePaginatorUseOutputWalkers($queryBuilder, $resourceClass, $operationName));
156156

157157
if (null === $this->requestStack) {
158158
$isPartialEnabled = $this->pagination->isPartialEnabled($resourceClass, $operationName, $context);
@@ -276,64 +276,82 @@ private function getPaginationParameter(Request $request, string $parameterName,
276276
}
277277

278278
/**
279-
* Determines whether the Paginator should fetch join collections, if the root entity uses composite identifiers it should not.
280-
*
281-
* @see https://github.com/doctrine/doctrine2/issues/2910
279+
* Determines the value of the $fetchJoinCollection argument passed to the Doctrine ORM Paginator.
282280
*/
283-
private function useFetchJoinCollection(QueryBuilder $queryBuilder, string $resourceClass = null, string $operationName = null): bool
281+
private function shouldDoctrinePaginatorFetchJoinCollection(QueryBuilder $queryBuilder, string $resourceClass = null, string $operationName = null): bool
284282
{
283+
if (null !== $resourceClass) {
284+
$resourceMetadata = $this->resourceMetadataFactory->create($resourceClass);
285+
286+
if (null !== $fetchJoinCollection = $resourceMetadata->getCollectionOperationAttribute($operationName, 'pagination_fetch_join_collection', null, true)) {
287+
return $fetchJoinCollection;
288+
}
289+
}
290+
291+
/*
292+
* "Cannot count query which selects two FROM components, cannot make distinction"
293+
*
294+
* @see https://github.com/doctrine/orm/blob/v2.6.3/lib/Doctrine/ORM/Tools/Pagination/WhereInWalker.php#L81
295+
* @see https://github.com/doctrine/doctrine2/issues/2910
296+
*/
285297
if (QueryChecker::hasRootEntityWithCompositeIdentifier($queryBuilder, $this->managerRegistry)) {
286298
return false;
287299
}
288300

289-
if (null === $resourceClass) {
301+
if (QueryChecker::hasJoinedToManyAssociation($queryBuilder, $this->managerRegistry)) {
290302
return true;
291303
}
292304

293-
$resourceMetadata = $this->resourceMetadataFactory->create($resourceClass);
294-
295-
return $resourceMetadata->getCollectionOperationAttribute($operationName, 'pagination_fetch_join_collection', true, true);
305+
// disable $fetchJoinCollection by default (performance)
306+
return false;
296307
}
297308

298309
/**
299-
* Determines whether output walkers should be used.
310+
* Determines whether the Doctrine ORM Paginator should use output walkers.
300311
*/
301-
private function useOutputWalkers(QueryBuilder $queryBuilder): bool
312+
private function shouldDoctrinePaginatorUseOutputWalkers(QueryBuilder $queryBuilder, string $resourceClass = null, string $operationName = null): bool
302313
{
314+
if (null !== $resourceClass) {
315+
$resourceMetadata = $this->resourceMetadataFactory->create($resourceClass);
316+
317+
if (null !== $useOutputWalkers = $resourceMetadata->getCollectionOperationAttribute($operationName, 'pagination_use_output_walkers', null, true)) {
318+
return $useOutputWalkers;
319+
}
320+
}
321+
303322
/*
304323
* "Cannot count query that uses a HAVING clause. Use the output walkers for pagination"
305324
*
306-
* @see https://github.com/doctrine/doctrine2/blob/900b55d16afdcdeb5100d435a7166d3a425b9873/lib/Doctrine/ORM/Tools/Pagination/CountWalker.php#L50
325+
* @see https://github.com/doctrine/orm/blob/v2.6.3/lib/Doctrine/ORM/Tools/Pagination/CountWalker.php#L56
307326
*/
308327
if (QueryChecker::hasHavingClause($queryBuilder)) {
309328
return true;
310329
}
311330

312331
/*
313-
* "Paginating an entity with foreign key as identifier only works when using the Output Walkers. Call Paginator#setUseOutputWalkers(true) before iterating the paginator."
332+
* "Cannot count query which selects two FROM components, cannot make distinction"
314333
*
315-
* @see https://github.com/doctrine/doctrine2/blob/900b55d16afdcdeb5100d435a7166d3a425b9873/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryWalker.php#L87
334+
* @see https://github.com/doctrine/orm/blob/v2.6.3/lib/Doctrine/ORM/Tools/Pagination/CountWalker.php#L64
316335
*/
317-
if (QueryChecker::hasRootEntityWithForeignKeyIdentifier($queryBuilder, $this->managerRegistry)) {
336+
if (QueryChecker::hasRootEntityWithCompositeIdentifier($queryBuilder, $this->managerRegistry)) {
318337
return true;
319338
}
320339

321340
/*
322-
* "Cannot select distinct identifiers from query with LIMIT and ORDER BY on a column from a fetch joined to-many association. Use output walkers."
341+
* "Paginating an entity with foreign key as identifier only works when using the Output Walkers. Call Paginator#setUseOutputWalkers(true) before iterating the paginator."
323342
*
324-
* @see https://github.com/doctrine/doctrine2/blob/900b55d16afdcdeb5100d435a7166d3a425b9873/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryWalker.php#L149
343+
* @see https://github.com/doctrine/orm/blob/v2.6.3/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryWalker.php#L77
325344
*/
326-
if (
327-
QueryChecker::hasMaxResults($queryBuilder) &&
328-
QueryChecker::hasOrderByOnToManyJoin($queryBuilder, $this->managerRegistry)
329-
) {
345+
if (QueryChecker::hasRootEntityWithForeignKeyIdentifier($queryBuilder, $this->managerRegistry)) {
330346
return true;
331347
}
332348

333349
/*
334-
* When using composite identifiers pagination will need Output walkers
350+
* "Cannot select distinct identifiers from query with LIMIT and ORDER BY on a column from a fetch joined to-many association. Use output walkers."
351+
*
352+
* @see https://github.com/doctrine/orm/blob/v2.6.3/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryWalker.php#L150
335353
*/
336-
if (QueryChecker::hasRootEntityWithCompositeIdentifier($queryBuilder, $this->managerRegistry)) {
354+
if (QueryChecker::hasMaxResults($queryBuilder) && QueryChecker::hasOrderByOnFetchJoinedToManyAssociation($queryBuilder, $this->managerRegistry)) {
337355
return true;
338356
}
339357

‎src/Bridge/Doctrine/Orm/Util/QueryBuilderHelper.php

Lines changed: 147 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

1414
namespace ApiPlatform\Core\Bridge\Doctrine\Orm\Util;
1515

16+
use Doctrine\Common\Persistence\ManagerRegistry;
1617
use Doctrine\ORM\Query\Expr\Join;
1718
use Doctrine\ORM\QueryBuilder;
1819

@@ -28,7 +29,7 @@ private function __construct()
2829
}
2930

3031
/**
31-
* Adds a join to the queryBuilder if none exists.
32+
* Adds a join to the QueryBuilder if none exists.
3233
*/
3334
public static function addJoinOnce(QueryBuilder $queryBuilder, QueryNameGeneratorInterface $queryNameGenerator, string $alias, string $association, string $joinType = null, string $conditionType = null, string $condition = null, string $originAlias = null): string
3435
{
@@ -51,7 +52,113 @@ public static function addJoinOnce(QueryBuilder $queryBuilder, QueryNameGenerato
5152
}
5253

5354
/**
54-
* Get the existing join from queryBuilder DQL parts.
55+
* Gets the entity class name by an alias used in the QueryBuilder.
56+
*/
57+
public static function getEntityClassByAlias(string $alias, QueryBuilder $queryBuilder, ManagerRegistry $managerRegistry): string
58+
{
59+
if (!\in_array($alias, $queryBuilder->getAllAliases(), true)) {
60+
throw new \LogicException(sprintf('The alias "%s" does not exist in the QueryBuilder.', $alias));
61+
}
62+
63+
$rootAliasMap = self::mapRootAliases($queryBuilder->getRootAliases(), $queryBuilder->getRootEntities());
64+
65+
if (isset($rootAliasMap[$alias])) {
66+
return $rootAliasMap[$alias];
67+
}
68+
69+
$metadata = null;
70+
71+
foreach (self::traverseJoins($alias, $queryBuilder, $managerRegistry) as [$currentMetadata]) {
72+
$metadata = $currentMetadata;
73+
}
74+
75+
if (null === $metadata) {
76+
throw new \LogicException(sprintf('The alias "%s" does not exist in the QueryBuilder.', $alias));
77+
}
78+
79+
return $metadata->getName();
80+
}
81+
82+
/**
83+
* Finds the root alias for an alias used in the QueryBuilder.
84+
*/
85+
public static function findRootAlias(string $alias, QueryBuilder $queryBuilder): string
86+
{
87+
if (\in_array($alias, $queryBuilder->getRootAliases(), true)) {
88+
return $alias;
89+
}
90+
91+
foreach ($queryBuilder->getDQLPart('join') as $rootAlias => $joins) {
92+
foreach ($joins as $join) {
93+
if ($alias === $join->getAlias()) {
94+
return $rootAlias;
95+
}
96+
}
97+
}
98+
99+
throw new \LogicException(sprintf('The alias "%s" does not exist in the QueryBuilder.', $alias));
100+
}
101+
102+
/**
103+
* Traverses through the joins for an alias used in the QueryBuilder.
104+
*
105+
* @return \Generator<string, array>
106+
*/
107+
public static function traverseJoins(string $alias, QueryBuilder $queryBuilder, ManagerRegistry $managerRegistry): \Generator
108+
{
109+
$rootAliasMap = self::mapRootAliases($queryBuilder->getRootAliases(), $queryBuilder->getRootEntities());
110+
111+
$joinParts = $queryBuilder->getDQLPart('join');
112+
$rootAlias = self::findRootAlias($alias, $queryBuilder);
113+
114+
$joinAliasMap = self::mapJoinAliases($joinParts[$rootAlias]);
115+
116+
$aliasMap = array_merge($rootAliasMap, $joinAliasMap);
117+
118+
$apexEntityClass = null;
119+
$associationStack = [];
120+
$aliasStack = [];
121+
$currentAlias = $alias;
122+
123+
while (null === $apexEntityClass) {
124+
if (!isset($aliasMap[$currentAlias])) {
125+
throw new \LogicException(sprintf('Unknown alias "%s".', $currentAlias));
126+
}
127+
128+
if (\is_string($aliasMap[$currentAlias])) {
129+
$aliasStack[] = $currentAlias;
130+
$apexEntityClass = $aliasMap[$currentAlias];
131+
} else {
132+
[$parentAlias, $association] = $aliasMap[$currentAlias];
133+
134+
$associationStack[] = $association;
135+
$aliasStack[] = $currentAlias;
136+
$currentAlias = $parentAlias;
137+
}
138+
}
139+
140+
$entityClass = $apexEntityClass;
141+
142+
while (null !== ($alias = array_pop($aliasStack))) {
143+
$metadata = $managerRegistry
144+
->getManagerForClass($entityClass)
145+
->getClassMetadata($entityClass);
146+
147+
$association = array_pop($associationStack);
148+
149+
yield $alias => [
150+
$metadata,
151+
$association,
152+
];
153+
154+
if (null !== $association) {
155+
$entityClass = $metadata->getAssociationTargetClass($association);
156+
}
157+
}
158+
}
159+
160+
/**
161+
* Gets the existing join from QueryBuilder DQL parts.
55162
*/
56163
private static function getExistingJoin(QueryBuilder $queryBuilder, string $alias, string $association, string $originAlias = null): ?Join
57164
{
@@ -71,4 +178,42 @@ private static function getExistingJoin(QueryBuilder $queryBuilder, string $alia
71178

72179
return null;
73180
}
181+
182+
/**
183+
* Maps the root aliases to root entity classes.
184+
*
185+
* @return array<string, string>
186+
*/
187+
private static function mapRootAliases(array $rootAliases, array $rootEntities): array
188+
{
189+
$aliasMap = array_combine($rootAliases, $rootEntities);
190+
if (false === $aliasMap) {
191+
throw new \LogicException('Number of root aliases and root entities do not match.');
192+
}
193+
194+
return $aliasMap;
195+
}
196+
197+
/**
198+
* Maps the join aliases to the parent alias and association, or the entity class.
199+
*
200+
* @return array<string, string[]|string>
201+
*/
202+
private static function mapJoinAliases(iterable $joins): array
203+
{
204+
$aliasMap = [];
205+
206+
foreach ($joins as $join) {
207+
$alias = $join->getAlias();
208+
$relationship = $join->getJoin();
209+
210+
if (false !== strpos($relationship, '.')) {
211+
$aliasMap[$alias] = explode('.', $relationship);
212+
} else {
213+
$aliasMap[$alias] = $relationship;
214+
}
215+
}
216+
217+
return $aliasMap;
218+
}
74219
}

0 commit comments

Comments
 (0)