Skip to content

Commit d13fee1

Browse files
soyukasoyuka
authored
Merge pull request #1087 from soyuka/should-eager-load
Refactor should eager load
2 parents 2ec8626 + 6b12c5b commit d13fee1

File tree

4 files changed

+91
-75
lines changed

4 files changed

+91
-75
lines changed

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

Lines changed: 6 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
namespace ApiPlatform\Core\Bridge\Doctrine\Orm\Extension;
1515

1616
use ApiPlatform\Core\Bridge\Doctrine\Orm\Util\QueryNameGeneratorInterface;
17+
use ApiPlatform\Core\Bridge\Doctrine\Orm\Util\ShouldEagerLoad;
1718
use ApiPlatform\Core\Exception\PropertyNotFoundException;
1819
use ApiPlatform\Core\Exception\ResourceClassNotFoundException;
1920
use ApiPlatform\Core\Exception\RuntimeException;
@@ -35,6 +36,8 @@
3536
*/
3637
final class EagerLoadingExtension implements QueryCollectionExtensionInterface, QueryItemExtensionInterface
3738
{
39+
use ShouldEagerLoad;
40+
3841
private $propertyNameCollectionFactory;
3942
private $propertyMetadataFactory;
4043
private $resourceMetadataFactory;
@@ -65,7 +68,7 @@ public function applyToCollection(QueryBuilder $queryBuilder, QueryNameGenerator
6568
$options = ['collection_operation_name' => $operationName];
6669
}
6770

68-
$forceEager = $this->isForceEager($resourceClass, $options);
71+
$forceEager = $this->shouldOperationForceEager($resourceClass, $options);
6972

7073
$groups = $this->getSerializerGroups($resourceClass, $options, 'normalization_context');
7174

@@ -84,7 +87,7 @@ public function applyToItem(QueryBuilder $queryBuilder, QueryNameGeneratorInterf
8487
$options = ['item_operation_name' => $operationName];
8588
}
8689

87-
$forceEager = $this->isForceEager($resourceClass, $options);
90+
$forceEager = $this->shouldOperationForceEager($resourceClass, $options);
8891

8992
if (isset($context['groups'])) {
9093
$groups = ['serializer_groups' => $context['groups']];
@@ -131,6 +134,7 @@ private function joinRelations(QueryBuilder $queryBuilder, QueryNameGeneratorInt
131134
continue;
132135
}
133136

137+
// We don't want to interfere with doctrine on this association
134138
if (false === $forceEager && ClassMetadataInfo::FETCH_EAGER !== $mapping['fetch']) {
135139
continue;
136140
}
@@ -232,27 +236,4 @@ private function getSerializerGroups(string $resourceClass, array $options, stri
232236

233237
return ['serializer_groups' => $context['groups']];
234238
}
235-
236-
/**
237-
* Does an operation force eager?
238-
*
239-
* @param string $resourceClass
240-
* @param array $options
241-
*
242-
* @return bool
243-
*/
244-
private function isForceEager(string $resourceClass, array $options): bool
245-
{
246-
$resourceMetadata = $this->resourceMetadataFactory->create($resourceClass);
247-
248-
if (isset($options['collection_operation_name'])) {
249-
$forceEager = $resourceMetadata->getCollectionOperationAttribute($options['collection_operation_name'], 'force_eager', null, true);
250-
} elseif (isset($options['item_operation_name'])) {
251-
$forceEager = $resourceMetadata->getItemOperationAttribute($options['item_operation_name'], 'force_eager', null, true);
252-
} else {
253-
$forceEager = $resourceMetadata->getAttribute('force_eager');
254-
}
255-
256-
return is_bool($forceEager) ? $forceEager : $this->forceEager;
257-
}
258239
}

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

Lines changed: 4 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,8 @@
1414
namespace ApiPlatform\Core\Bridge\Doctrine\Orm\Extension;
1515

1616
use ApiPlatform\Core\Bridge\Doctrine\Orm\Util\QueryNameGeneratorInterface;
17+
use ApiPlatform\Core\Bridge\Doctrine\Orm\Util\ShouldEagerLoad;
1718
use ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface;
18-
use Doctrine\ORM\EntityManager;
19-
use Doctrine\ORM\Mapping\ClassMetadataInfo;
2019
use Doctrine\ORM\Query\Expr\Join;
2120
use Doctrine\ORM\QueryBuilder;
2221

@@ -26,6 +25,8 @@
2625
*/
2726
final class FilterEagerLoadingExtension implements QueryCollectionExtensionInterface
2827
{
28+
use ShouldEagerLoad;
29+
2930
private $resourceMetadataFactory;
3031
private $forceEager;
3132

@@ -43,7 +44,7 @@ public function applyToCollection(QueryBuilder $queryBuilder, QueryNameGenerator
4344
$em = $queryBuilder->getEntityManager();
4445
$classMetadata = $em->getClassMetadata($resourceClass);
4546

46-
if (!$this->hasFetchEagerAssociation($em, $classMetadata) && (false === $this->forceEager || false === $this->isForceEager($resourceClass, ['collection_operation_name' => $operationName]))) {
47+
if (!$this->shouldOperationForceEager($resourceClass, ['collection_operation_name' => $operationName]) && !$this->hasFetchEagerAssociation($em, $classMetadata)) {
4748
return;
4849
}
4950

@@ -129,48 +130,4 @@ private function getQueryBuilderWithNewAliases(QueryBuilder $queryBuilder, Query
129130

130131
return $queryBuilderClone;
131132
}
132-
133-
/**
134-
* Does an operation force eager?
135-
*
136-
* @param string $resourceClass
137-
* @param array $options
138-
*
139-
* @return bool
140-
*/
141-
private function isForceEager(string $resourceClass, array $options): bool
142-
{
143-
$resourceMetadata = $this->resourceMetadataFactory->create($resourceClass);
144-
145-
if (isset($options['collection_operation_name'])) {
146-
$forceEager = $resourceMetadata->getCollectionOperationAttribute($options['collection_operation_name'], 'force_eager', null, true);
147-
} else {
148-
$forceEager = $resourceMetadata->getAttribute('force_eager');
149-
}
150-
151-
return is_bool($forceEager) ? $forceEager : $this->forceEager;
152-
}
153-
154-
private function hasFetchEagerAssociation(EntityManager $em, ClassMetadataInfo $classMetadata, &$checked = [])
155-
{
156-
$checked[] = $classMetadata->name;
157-
158-
foreach ($classMetadata->associationMappings as $mapping) {
159-
if (ClassMetadataInfo::FETCH_EAGER === $mapping['fetch']) {
160-
return true;
161-
}
162-
163-
$related = $em->getClassMetadata($mapping['targetEntity']);
164-
165-
if (in_array($related->name, $checked, true)) {
166-
continue;
167-
}
168-
169-
if (true === $this->hasFetchEagerAssociation($em, $related, $checked)) {
170-
return true;
171-
}
172-
}
173-
174-
return false;
175-
}
176133
}

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

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the API Platform project.
5+
*
6+
* (c) Kévin Dunglas <[email protected]>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
declare(strict_types=1);
13+
14+
namespace ApiPlatform\Core\Bridge\Doctrine\Orm\Util;
15+
16+
use Doctrine\ORM\EntityManager;
17+
use Doctrine\ORM\Mapping\ClassMetadataInfo;
18+
19+
/**
20+
* @author Antoine Bluchet <[email protected]>
21+
*/
22+
trait ShouldEagerLoad
23+
{
24+
/**
25+
* Checks if an operation has a `force_eager` attribute.
26+
*
27+
* @param string $resourceClass
28+
* @param array $options
29+
*
30+
* @return bool
31+
*/
32+
private function shouldOperationForceEager(string $resourceClass, array $options): bool
33+
{
34+
$resourceMetadata = $this->resourceMetadataFactory->create($resourceClass);
35+
36+
if (isset($options['collection_operation_name'])) {
37+
$forceEager = $resourceMetadata->getCollectionOperationAttribute($options['collection_operation_name'], 'force_eager', null, true);
38+
} elseif (isset($options['item_operation_name'])) {
39+
$forceEager = $resourceMetadata->getItemOperationAttribute($options['item_operation_name'], 'force_eager', null, true);
40+
} else {
41+
$forceEager = $resourceMetadata->getAttribute('force_eager');
42+
}
43+
44+
return is_bool($forceEager) ? $forceEager : $this->forceEager;
45+
}
46+
47+
/**
48+
* Checkes if the class has an associationMapping with FETCH=EAGER.
49+
*
50+
* @param EntityManager $em
51+
* @param ClassMetadataInfo $classMetadata
52+
* @param array $checked array cache of tested metadata classes
53+
*/
54+
private function hasFetchEagerAssociation(EntityManager $em, ClassMetadataInfo $classMetadata, &$checked = [])
55+
{
56+
$checked[] = $classMetadata->name;
57+
58+
foreach ($classMetadata->associationMappings as $mapping) {
59+
if (ClassMetadataInfo::FETCH_EAGER === $mapping['fetch']) {
60+
return true;
61+
}
62+
63+
$related = $em->getClassMetadata($mapping['targetEntity']);
64+
65+
if (in_array($related->name, $checked, true)) {
66+
continue;
67+
}
68+
69+
if (true === $this->hasFetchEagerAssociation($em, $related, $checked)) {
70+
return true;
71+
}
72+
}
73+
74+
return false;
75+
}
76+
}

‎tests/Bridge/Doctrine/Orm/Extension/EagerLoadingExtensionTest.php

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -352,8 +352,10 @@ public function testMaxDepthReached()
352352

353353
public function testForceEager()
354354
{
355+
$resourceMetadata = new ResourceMetadata();
356+
$resourceMetadata = $resourceMetadata->withAttributes(['normalization_context' => ['groups' => 'foobar']]);
355357
$resourceMetadataFactoryProphecy = $this->prophesize(ResourceMetadataFactoryInterface::class);
356-
$resourceMetadataFactoryProphecy->create(Dummy::class)->willReturn(new ResourceMetadata());
358+
$resourceMetadataFactoryProphecy->create(Dummy::class)->willReturn($resourceMetadata);
357359

358360
$propertyNameCollectionFactoryProphecy = $this->prophesize(PropertyNameCollectionFactoryInterface::class);
359361
$propertyNameCollectionFactoryProphecy->create(UnknownDummy::class)->willReturn(new PropertyNameCollection(['id']))->shouldBeCalled();
@@ -365,8 +367,8 @@ public function testForceEager()
365367
$idPropertyMetadata = new PropertyMetadata();
366368
$idPropertyMetadata = $idPropertyMetadata->withIdentifier(true);
367369

368-
$propertyMetadataFactoryProphecy->create(UnknownDummy::class, 'id', [])->willReturn($idPropertyMetadata)->shouldBeCalled();
369-
$propertyMetadataFactoryProphecy->create(Dummy::class, 'relation', [])->willReturn($relationPropertyMetadata)->shouldBeCalled();
370+
$propertyMetadataFactoryProphecy->create(UnknownDummy::class, 'id', ['serializer_groups' => 'foobar'])->willReturn($idPropertyMetadata)->shouldBeCalled();
371+
$propertyMetadataFactoryProphecy->create(Dummy::class, 'relation', ['serializer_groups' => 'foobar'])->willReturn($relationPropertyMetadata)->shouldBeCalled();
370372

371373
$queryBuilderProphecy = $this->prophesize(QueryBuilder::class);
372374

0 commit comments

Comments
 (0)