Merge pull request #2145 from soyuka/fix-2109

OrderFilter with nullable relation uses LEFT JOIN fix #2109

Author: soyuka

src/Bridge/Doctrine/Orm/Extension/FilterExtension.php

@@ -16,6 +16,7 @@
 use ApiPlatform\Core\Api\FilterCollection;
 use ApiPlatform\Core\Api\FilterLocatorTrait;
 use ApiPlatform\Core\Bridge\Doctrine\Orm\Filter\FilterInterface;
+use ApiPlatform\Core\Bridge\Doctrine\Orm\Filter\OrderFilter;
 use ApiPlatform\Core\Bridge\Doctrine\Orm\Util\QueryNameGeneratorInterface;
 use ApiPlatform\Core\Exception\InvalidArgumentException;
 use ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface;
@@ -60,12 +61,25 @@ public function applyToCollection(QueryBuilder $queryBuilder, QueryNameGenerator
             return;
         }
 
+        $orderFilter = null;
+
         foreach ($resourceFilters as $filterId) {
             $filter = $this->getFilter($filterId);
             if ($filter instanceof FilterInterface) {
+                // Apply the OrderFilter after every other filter to avoid an edge case where OrderFilter would do a LEFT JOIN instead of an INNER JOIN
+                if ($filter instanceof OrderFilter) {
+                    $orderFilter = $filter;
+                    continue;
+                }
+
                 $context['filters'] = $context['filters'] ?? [];
                 $filter->apply($queryBuilder, $queryNameGenerator, $resourceClass, $operationName, $context);
             }
         }
+
+        if (null !== $orderFilter) {
+            $context['filters'] = $context['filters'] ?? [];
+            $orderFilter->apply($queryBuilder, $queryNameGenerator, $resourceClass, $operationName, $context);
+        }
     }
 }

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

@@ -269,7 +269,7 @@ protected function extractProperties(Request $request/*, string $resourceClass*/
      *               the second element is the $field name
      *               the third element is the $associations array
      */
-    protected function addJoinsForNestedProperty(string $property, string $rootAlias, QueryBuilder $queryBuilder, QueryNameGeneratorInterface $queryNameGenerator/*, string $resourceClass*/): array
+    protected function addJoinsForNestedProperty(string $property, string $rootAlias, QueryBuilder $queryBuilder, QueryNameGeneratorInterface $queryNameGenerator/*, string $resourceClass, string $joinType*/): array
     {
         if (\func_num_args() > 4) {
             $resourceClass = func_get_arg(4);
@@ -283,12 +283,18 @@ protected function addJoinsForNestedProperty(string $property, string $rootAlias
             $resourceClass = null;
         }
 
+        if (\func_num_args() > 5) {
+            $joinType = func_get_arg(5);
+        } else {
+            $joinType = null;
+        }
+
         $propertyParts = $this->splitPropertyParts($property, $resourceClass);
         $parentAlias = $rootAlias;
         $alias = null;
 
         foreach ($propertyParts['associations'] as $association) {
-            $alias = QueryBuilderHelper::addJoinOnce($queryBuilder, $queryNameGenerator, $parentAlias, $association);
+            $alias = QueryBuilderHelper::addJoinOnce($queryBuilder, $queryNameGenerator, $parentAlias, $association, $joinType);
             $parentAlias = $alias;
         }
 

src/Bridge/Doctrine/Orm/Filter/OrderFilter.php

@@ -15,6 +15,7 @@
 
 use ApiPlatform\Core\Bridge\Doctrine\Orm\Util\QueryNameGeneratorInterface;
 use Doctrine\Common\Persistence\ManagerRegistry;
+use Doctrine\ORM\Query\Expr\Join;
 use Doctrine\ORM\QueryBuilder;
 use Psr\Log\LoggerInterface;
 use Symfony\Component\HttpFoundation\Request;
@@ -146,7 +147,7 @@ protected function filterProperty(string $property, $direction, QueryBuilder $qu
         $field = $property;
 
         if ($this->isPropertyNested($property, $resourceClass)) {
-            list($alias, $field) = $this->addJoinsForNestedProperty($property, $alias, $queryBuilder, $queryNameGenerator, $resourceClass);
+            list($alias, $field) = $this->addJoinsForNestedProperty($property, $alias, $queryBuilder, $queryNameGenerator, $resourceClass, Join::LEFT_JOIN);
         }
 
         if (null !== $nullsComparison = $this->properties[$property]['nulls_comparison'] ?? null) {

tests/Bridge/Doctrine/Orm/Filter/OrderFilterTest.php

@@ -255,7 +255,7 @@ public function provideApplyTestData(): array
                         'relatedDummy.symfony' => 'desc',
                     ],
                 ],
-                sprintf('SELECT o FROM %s o INNER JOIN o.relatedDummy relatedDummy_a1 ORDER BY o.id ASC, o.name DESC, relatedDummy_a1.symfony DESC', Dummy::class),
+                sprintf('SELECT o FROM %s o LEFT JOIN o.relatedDummy relatedDummy_a1 ORDER BY o.id ASC, o.name DESC, relatedDummy_a1.symfony DESC', Dummy::class),
                 null,
                 $orderFilterFactory,
             ],
@@ -354,6 +354,17 @@ public function provideApplyTestData(): array
                 null,
                 $orderFilterFactory,
             ],
+            'not nullable relation will be a LEFT JOIN' => [
+                [
+                    'relatedDummy.name' => 'ASC',
+                ],
+                [
+                    'order' => ['relatedDummy.name' => 'ASC'],
+                ],
+                sprintf('SELECT o FROM %s o LEFT JOIN o.relatedDummy relatedDummy_a1 ORDER BY relatedDummy_a1.name ASC', Dummy::class),
+                null,
+                $orderFilterFactory,
+            ],
         ];
     }
 }