Skip to content

Commit 5d54089

Browse files
vincentchalamondunglas
vincentchalamon
authored and
dunglas
committed
Fix #1246 (#1324)
1 parent 14300d0 commit 5d54089

File tree

12 files changed

+288
-78
lines changed

12 files changed

+288
-78
lines changed

‎features/bootstrap/FeatureContext.php

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\EmbeddedDummy;
3030
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\FileConfigDummy;
3131
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\Foo;
32+
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\FooDummy;
3233
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\Node;
3334
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\Person;
3435
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\PersonToPet;
@@ -142,6 +143,28 @@ public function thereAreFooObjectsWithFakeNames(int $nb)
142143
$this->manager->flush();
143144
}
144145

146+
/**
147+
* @Given there are :nb fooDummy objects with fake names
148+
*/
149+
public function thereAreFooDummyObjectsWithFakeNames($nb)
150+
{
151+
$names = ['Hawsepipe', 'Ephesian', 'Sthenelus', 'Separativeness', 'Balbo'];
152+
$dummies = ['Lorem', 'Dolor', 'Dolor', 'Sit', 'Amet'];
153+
154+
for ($i = 0; $i < $nb; ++$i) {
155+
$dummy = new Dummy();
156+
$dummy->setName($dummies[$i]);
157+
158+
$foo = new FooDummy();
159+
$foo->setName($names[$i]);
160+
$foo->setDummy($dummy);
161+
162+
$this->manager->persist($foo);
163+
}
164+
165+
$this->manager->flush();
166+
}
167+
145168
/**
146169
* @Given there is :nb dummy group objects
147170
*/

‎features/main/default_order.feature

Lines changed: 59 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ Feature: Default order
33
As a client software developer,
44
I need to be able to specify default order.
55

6-
@createSchema @dropSchema
6+
@createSchema
77
Scenario: Override custom order
88
Given there are 5 foo objects with fake names
99
When I send a "GET" request to "/foos?itemsPerPage=10"
@@ -60,3 +60,61 @@ Feature: Default order
6060
}
6161
}
6262
"""
63+
64+
@dropSchema
65+
Scenario: Override custom order by association
66+
Given there are 5 fooDummy objects with fake names
67+
When I send a "GET" request to "/foo_dummies?itemsPerPage=10"
68+
Then the response status code should be 200
69+
And the response should be in JSON
70+
And the header "Content-Type" should be equal to "application/ld+json; charset=utf-8"
71+
And the JSON should be equal to:
72+
"""
73+
{
74+
"@context": "/contexts/FooDummy",
75+
"@id": "/foo_dummies",
76+
"@type": "hydra:Collection",
77+
"hydra:member": [
78+
{
79+
"@id": "/foo_dummies/5",
80+
"@type": "FooDummy",
81+
"id": 5,
82+
"name": "Balbo",
83+
"dummy": "/dummies/5"
84+
},
85+
{
86+
"@id": "/foo_dummies/2",
87+
"@type": "FooDummy",
88+
"id": 2,
89+
"name": "Ephesian",
90+
"dummy": "/dummies/2"
91+
},
92+
{
93+
"@id": "/foo_dummies/3",
94+
"@type": "FooDummy",
95+
"id": 3,
96+
"name": "Sthenelus",
97+
"dummy": "/dummies/3"
98+
},
99+
{
100+
"@id": "/foo_dummies/1",
101+
"@type": "FooDummy",
102+
"id": 1,
103+
"name": "Hawsepipe",
104+
"dummy": "/dummies/1"
105+
},
106+
{
107+
"@id": "/foo_dummies/4",
108+
"@type": "FooDummy",
109+
"id": 4,
110+
"name": "Separativeness",
111+
"dummy": "/dummies/4"
112+
}
113+
],
114+
"hydra:totalItems": 5,
115+
"hydra:view": {
116+
"@id": "/foo_dummies?itemsPerPage=10",
117+
"@type": "hydra:PartialCollectionView"
118+
}
119+
}
120+
"""

‎phpstan.neon

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,5 +11,4 @@ parameters:
1111
- '#Call to an undefined method Doctrine\\Common\\Persistence\\Mapping\\ClassMetadata::getAssociationMappings\(\)#'
1212

1313
# False positives
14-
- '#Parameter \#2 \$dqlPart of method Doctrine\\ORM\\QueryBuilder::add\(\) expects Doctrine\\ORM\\Query\\Expr\\Base, Doctrine\\ORM\\Query\\Expr\\Join\[\] given#' # Fixed in Doctrine's master
1514
- '#Call to an undefined method Doctrine\\Common\\Persistence\\ObjectManager::getConnection\(\)#'

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

Lines changed: 9 additions & 5 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\EagerLoadingTrait;
17+
use ApiPlatform\Core\Bridge\Doctrine\Orm\Util\QueryBuilderHelper;
1718
use ApiPlatform\Core\Bridge\Doctrine\Orm\Util\QueryNameGeneratorInterface;
1819
use ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface;
1920
use Doctrine\ORM\Query\Expr\Join;
@@ -119,12 +120,15 @@ private function getQueryBuilderWithNewAliases(QueryBuilder $queryBuilder, Query
119120

120121
//Change join aliases
121122
foreach ($joinParts[$originAlias] as $joinPart) {
123+
/** @var Join $joinPart */
124+
$joinString = str_replace($aliases, $replacements, $joinPart->getJoin());
125+
$pos = strpos($joinString, '.');
126+
$alias = substr($joinString, 0, $pos);
127+
$association = substr($joinString, $pos + 1);
128+
$condition = str_replace($aliases, $replacements, $joinPart->getCondition());
129+
$newAlias = QueryBuilderHelper::addJoinOnce($queryBuilderClone, $queryNameGenerator, $alias, $association, $joinPart->getJoinType(), $joinPart->getConditionType(), $condition);
122130
$aliases[] = "{$joinPart->getAlias()}.";
123-
$alias = $queryNameGenerator->generateJoinAlias($joinPart->getAlias());
124-
$replacements[] = "$alias.";
125-
$join = new Join($joinPart->getJoinType(), str_replace($aliases, $replacements, $joinPart->getJoin()), $alias, $joinPart->getConditionType(), str_replace($aliases, $replacements, $joinPart->getCondition()), $joinPart->getIndexBy());
126-
127-
$queryBuilderClone->add('join', [$join], true);
131+
$replacements[] = "$newAlias.";
128132
}
129133

130134
$queryBuilderClone->add('where', str_replace($aliases, $replacements, (string) $wherePart));

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

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

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

16+
use ApiPlatform\Core\Bridge\Doctrine\Orm\Util\QueryBuilderHelper;
1617
use ApiPlatform\Core\Bridge\Doctrine\Orm\Util\QueryNameGeneratorInterface;
1718
use ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface;
1819
use Doctrine\ORM\QueryBuilder;
@@ -47,10 +48,18 @@ public function applyToCollection(QueryBuilder $queryBuilder, QueryNameGenerator
4748
if (null !== $defaultOrder) {
4849
foreach ($defaultOrder as $field => $order) {
4950
if (is_int($field)) {
51+
// Default direction
5052
$field = $order;
5153
$order = 'ASC';
5254
}
53-
$queryBuilder->addOrderBy('o.'.$field, $order);
55+
if (false === ($pos = strpos($field, '.'))) {
56+
// Configure default filter with property
57+
$field = 'o.'.$field;
58+
} else {
59+
$alias = QueryBuilderHelper::addJoinOnce($queryBuilder, $queryNameGenerator, 'o', substr($field, 0, $pos));
60+
$field = sprintf('%s.%s', $alias, substr($field, $pos + 1));
61+
}
62+
$queryBuilder->addOrderBy($field, $order);
5463
}
5564

5665
return;

‎src/Bridge/Doctrine/Orm/Filter/AbstractFilter.php

Lines changed: 2 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,12 @@
1313

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

16-
use ApiPlatform\Core\Bridge\Doctrine\Orm\Util\QueryChecker;
16+
use ApiPlatform\Core\Bridge\Doctrine\Orm\Util\QueryBuilderHelper;
1717
use ApiPlatform\Core\Bridge\Doctrine\Orm\Util\QueryNameGeneratorInterface;
1818
use ApiPlatform\Core\Exception\InvalidArgumentException;
1919
use ApiPlatform\Core\Util\RequestParser;
2020
use Doctrine\Common\Persistence\ManagerRegistry;
2121
use Doctrine\Common\Persistence\Mapping\ClassMetadata;
22-
use Doctrine\ORM\Query\Expr\Join;
2322
use Doctrine\ORM\QueryBuilder;
2423
use Psr\Log\LoggerInterface;
2524
use Psr\Log\NullLogger;
@@ -332,7 +331,7 @@ protected function addJoinsForNestedProperty(string $property, string $rootAlias
332331
$parentAlias = $rootAlias;
333332

334333
foreach ($propertyParts['associations'] as $association) {
335-
$alias = $this->addJoinOnce($queryBuilder, $queryNameGenerator, $parentAlias, $association);
334+
$alias = QueryBuilderHelper::addJoinOnce($queryBuilder, $queryNameGenerator, $parentAlias, $association);
336335
$parentAlias = $alias;
337336
}
338337

@@ -342,61 +341,4 @@ protected function addJoinsForNestedProperty(string $property, string $rootAlias
342341

343342
return [$alias, $propertyParts['field'], $propertyParts['associations']];
344343
}
345-
346-
/**
347-
* Get the existing join from queryBuilder DQL parts.
348-
*
349-
* @param QueryBuilder $queryBuilder
350-
* @param string $alias
351-
* @param string $association the association field
352-
*
353-
* @return Join|null
354-
*/
355-
private function getExistingJoin(QueryBuilder $queryBuilder, string $alias, string $association)
356-
{
357-
$parts = $queryBuilder->getDQLPart('join');
358-
359-
if (!isset($parts['o'])) {
360-
return null;
361-
}
362-
363-
foreach ($parts['o'] as $join) {
364-
if (sprintf('%s.%s', $alias, $association) === $join->getJoin()) {
365-
return $join;
366-
}
367-
}
368-
369-
return null;
370-
}
371-
372-
/**
373-
* Adds a join to the queryBuilder if none exists.
374-
*
375-
* @param QueryBuilder $queryBuilder
376-
* @param QueryNameGeneratorInterface $queryNameGenerator
377-
* @param string $alias
378-
* @param string $association the association field
379-
*
380-
* @return string the new association alias
381-
*/
382-
protected function addJoinOnce(QueryBuilder $queryBuilder, QueryNameGeneratorInterface $queryNameGenerator, string $alias, string $association): string
383-
{
384-
$join = $this->getExistingJoin($queryBuilder, $alias, $association);
385-
386-
if (null === $join) {
387-
$associationAlias = $queryNameGenerator->generateJoinAlias($association);
388-
389-
if (true === QueryChecker::hasLeftJoin($queryBuilder)) {
390-
$queryBuilder
391-
->leftJoin(sprintf('%s.%s', $alias, $association), $associationAlias);
392-
} else {
393-
$queryBuilder
394-
->innerJoin(sprintf('%s.%s', $alias, $association), $associationAlias);
395-
}
396-
} else {
397-
$associationAlias = $join->getAlias();
398-
}
399-
400-
return $associationAlias;
401-
}
402344
}

‎src/Bridge/Doctrine/Orm/Filter/SearchFilter.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
namespace ApiPlatform\Core\Bridge\Doctrine\Orm\Filter;
1515

1616
use ApiPlatform\Core\Api\IriConverterInterface;
17+
use ApiPlatform\Core\Bridge\Doctrine\Orm\Util\QueryBuilderHelper;
1718
use ApiPlatform\Core\Bridge\Doctrine\Orm\Util\QueryNameGeneratorInterface;
1819
use ApiPlatform\Core\Exception\InvalidArgumentException;
1920
use Doctrine\Common\Persistence\ManagerRegistry;
@@ -251,7 +252,7 @@ protected function filterProperty(string $property, $value, QueryBuilder $queryB
251252
$valueParameter = $queryNameGenerator->generateParameterName($association);
252253

253254
if ($metadata->isCollectionValuedAssociation($association)) {
254-
$associationAlias = $this->addJoinOnce($queryBuilder, $queryNameGenerator, $alias, $association);
255+
$associationAlias = QueryBuilderHelper::addJoinOnce($queryBuilder, $queryNameGenerator, $alias, $association);
255256
$associationField = 'id';
256257
} else {
257258
$associationAlias = $alias;

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

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
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\Query\Expr\Join;
17+
use Doctrine\ORM\QueryBuilder;
18+
19+
/**
20+
* @author Vincent Chalamon <[email protected]>
21+
*
22+
* @internal
23+
*/
24+
final class QueryBuilderHelper
25+
{
26+
private function __construct()
27+
{
28+
}
29+
30+
/**
31+
* Adds a join to the queryBuilder if none exists.
32+
*/
33+
public static function addJoinOnce(QueryBuilder $queryBuilder, QueryNameGeneratorInterface $queryNameGenerator, string $alias, string $association, string $joinType = null, string $conditionType = null, string $condition = null): string
34+
{
35+
$join = self::getExistingJoin($queryBuilder, $alias, $association);
36+
37+
if (null !== $join) {
38+
return $join->getAlias();
39+
}
40+
41+
$associationAlias = $queryNameGenerator->generateJoinAlias($association);
42+
$query = "$alias.$association";
43+
44+
if (Join::LEFT_JOIN === $joinType || QueryChecker::hasLeftJoin($queryBuilder)) {
45+
$queryBuilder->leftJoin($query, $associationAlias, $conditionType, $condition);
46+
} else {
47+
$queryBuilder->innerJoin($query, $associationAlias, $conditionType, $condition);
48+
}
49+
50+
return $associationAlias;
51+
}
52+
53+
/**
54+
* Get the existing join from queryBuilder DQL parts.
55+
*
56+
* @return Join|null
57+
*/
58+
private static function getExistingJoin(QueryBuilder $queryBuilder, string $alias, string $association)
59+
{
60+
$parts = $queryBuilder->getDQLPart('join');
61+
62+
if (!isset($parts['o'])) {
63+
return null;
64+
}
65+
66+
foreach ($parts['o'] as $join) {
67+
/** @var Join $join */
68+
if (sprintf('%s.%s', $alias, $association) === $join->getJoin()) {
69+
return $join;
70+
}
71+
}
72+
73+
return null;
74+
}
75+
}

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -269,8 +269,8 @@ public function testCompositeIdentifiers()
269269
->setParameter('foo', 1);
270270

271271
$queryNameGenerator = $this->prophesize(QueryNameGeneratorInterface::class);
272-
$queryNameGenerator->generateJoinAlias('item')->shouldBeCalled()->willReturn('item_2');
273-
$queryNameGenerator->generateJoinAlias('label')->shouldBeCalled()->willReturn('label_2');
272+
$queryNameGenerator->generateJoinAlias('compositeItem')->shouldBeCalled()->willReturn('item_2');
273+
$queryNameGenerator->generateJoinAlias('compositeLabel')->shouldBeCalled()->willReturn('label_2');
274274
$queryNameGenerator->generateJoinAlias('o')->shouldBeCalled()->willReturn('o_2');
275275

276276
$filterEagerLoadingExtension = new FilterEagerLoadingExtension($resourceMetadataFactoryProphecy->reveal(), true);
@@ -325,8 +325,8 @@ public function testFetchEagerWithNoForceEager()
325325
->setParameter('foo', 1);
326326

327327
$queryNameGenerator = $this->prophesize(QueryNameGeneratorInterface::class);
328-
$queryNameGenerator->generateJoinAlias('item')->shouldBeCalled()->willReturn('item_2');
329-
$queryNameGenerator->generateJoinAlias('label')->shouldBeCalled()->willReturn('label_2');
328+
$queryNameGenerator->generateJoinAlias('compositeItem')->shouldBeCalled()->willReturn('item_2');
329+
$queryNameGenerator->generateJoinAlias('compositeLabel')->shouldBeCalled()->willReturn('label_2');
330330
$queryNameGenerator->generateJoinAlias('o')->shouldBeCalled()->willReturn('o_2');
331331

332332
$queryNameGenerator->generateJoinAlias('foo')->shouldBeCalled()->willReturn('foo_2');
@@ -388,8 +388,8 @@ public function testCompositeIdentifiersWithoutAssociation()
388388
->setParameter('bar', 2);
389389

390390
$queryNameGenerator = $this->prophesize(QueryNameGeneratorInterface::class);
391-
$queryNameGenerator->generateJoinAlias('item')->shouldBeCalled()->willReturn('item_2');
392-
$queryNameGenerator->generateJoinAlias('label')->shouldBeCalled()->willReturn('label_2');
391+
$queryNameGenerator->generateJoinAlias('compositeItem')->shouldBeCalled()->willReturn('item_2');
392+
$queryNameGenerator->generateJoinAlias('compositeLabel')->shouldBeCalled()->willReturn('label_2');
393393
$queryNameGenerator->generateJoinAlias('o')->shouldBeCalled()->willReturn('o_2');
394394

395395
$filterEagerLoadingExtension = new FilterEagerLoadingExtension($resourceMetadataFactoryProphecy->reveal(), true);

0 commit comments

Comments
 (0)