[RFR] Issues/1246

features/bootstrap/FeatureContext.php

@@ -29,6 +29,7 @@
 use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\EmbeddedDummy;
 use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\FileConfigDummy;
 use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\Foo;
+use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\FooDummy;
 use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\Node;
 use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\Person;
 use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\PersonToPet;
@@ -142,6 +143,28 @@ public function thereAreFooObjectsWithFakeNames(int $nb)
         $this->manager->flush();
     }
 
+    /**
+     * @Given there are :nb fooDummy objects with fake names
+     */
+    public function thereAreFooDummyObjectsWithFakeNames($nb)
+    {
+        $names = ['Hawsepipe', 'Ephesian', 'Sthenelus', 'Separativeness', 'Balbo'];
+        $dummies = ['Lorem', 'Dolor', 'Dolor', 'Sit', 'Amet'];
+
+        for ($i = 0; $i < $nb; ++$i) {
+            $dummy = new Dummy();
+            $dummy->setName($dummies[$i]);
+
+            $foo = new FooDummy();
+            $foo->setName($names[$i]);
+            $foo->setDummy($dummy);
+
+            $this->manager->persist($foo);
+        }
+
+        $this->manager->flush();
+    }
+
     /**
      * @Given there is :nb dummy group objects
      */

features/main/default_order.feature

@@ -3,7 +3,7 @@ Feature: Default order
   As a client software developer,
   I need to be able to specify default order.
 
-  @createSchema @dropSchema
+  @createSchema
   Scenario: Override custom order
     Given there are 5 foo objects with fake names
     When I send a "GET" request to "/foos?itemsPerPage=10"
@@ -60,3 +60,61 @@ Feature: Default order
       }
     }
     """
+
+  @dropSchema
+  Scenario: Override custom order by association
+    Given there are 5 fooDummy objects with fake names
+    When I send a "GET" request to "/foo_dummies?itemsPerPage=10"
+    Then the response status code should be 200
+    And the response should be in JSON
+    And the header "Content-Type" should be equal to "application/ld+json; charset=utf-8"
+    And the JSON should be equal to:
+    """
+    {
+      "@context": "/contexts/FooDummy",
+      "@id": "/foo_dummies",
+      "@type": "hydra:Collection",
+      "hydra:member": [
+        {
+          "@id": "/foo_dummies/5",
+          "@type": "FooDummy",
+          "id": 5,
+          "name": "Balbo",
+          "dummy": "/dummies/5"
+        },
+        {
+          "@id": "/foo_dummies/2",
+          "@type": "FooDummy",
+          "id": 2,
+          "name": "Ephesian",
+          "dummy": "/dummies/2"
+        },
+        {
+          "@id": "/foo_dummies/3",
+          "@type": "FooDummy",
+          "id": 3,
+          "name": "Sthenelus",
+          "dummy": "/dummies/3"
+        },
+        {
+          "@id": "/foo_dummies/1",
+          "@type": "FooDummy",
+          "id": 1,
+          "name": "Hawsepipe",
+          "dummy": "/dummies/1"
+        },
+        {
+          "@id": "/foo_dummies/4",
+          "@type": "FooDummy",
+          "id": 4,
+          "name": "Separativeness",
+          "dummy": "/dummies/4"
+        }
+      ],
+      "hydra:totalItems": 5,
+      "hydra:view": {
+        "@id": "/foo_dummies?itemsPerPage=10",
+        "@type": "hydra:PartialCollectionView"
+      }
+    }
+    """

phpstan.neon

@@ -11,5 +11,4 @@ parameters:
         - '#Call to an undefined method Doctrine\\Common\\Persistence\\Mapping\\ClassMetadata::getAssociationMappings\(\)#'
 
         # False positives
-        - '#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
         - '#Call to an undefined method Doctrine\\Common\\Persistence\\ObjectManager::getConnection\(\)#'

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

@@ -14,6 +14,7 @@
 namespace ApiPlatform\Core\Bridge\Doctrine\Orm\Extension;
 
 use ApiPlatform\Core\Bridge\Doctrine\Orm\Util\EagerLoadingTrait;
+use ApiPlatform\Core\Bridge\Doctrine\Orm\Util\QueryBuilderHelper;
 use ApiPlatform\Core\Bridge\Doctrine\Orm\Util\QueryNameGeneratorInterface;
 use ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface;
 use Doctrine\ORM\Query\Expr\Join;
@@ -119,12 +120,15 @@ private function getQueryBuilderWithNewAliases(QueryBuilder $queryBuilder, Query
 
         //Change join aliases
         foreach ($joinParts[$originAlias] as $joinPart) {
+            /** @var Join $joinPart */
+            $joinString = str_replace($aliases, $replacements, $joinPart->getJoin());
+            $pos = strpos($joinString, '.');
+            $alias = substr($joinString, 0, $pos);
+            $association = substr($joinString, $pos + 1);
+            $condition = str_replace($aliases, $replacements, $joinPart->getCondition());
+            $newAlias = QueryBuilderHelper::addJoinOnce($queryBuilderClone, $queryNameGenerator, $alias, $association, $joinPart->getJoinType(), $joinPart->getConditionType(), $condition);
             $aliases[] = "{$joinPart->getAlias()}.";
-            $alias = $queryNameGenerator->generateJoinAlias($joinPart->getAlias());
-            $replacements[] = "$alias.";
-            $join = new Join($joinPart->getJoinType(), str_replace($aliases, $replacements, $joinPart->getJoin()), $alias, $joinPart->getConditionType(), str_replace($aliases, $replacements, $joinPart->getCondition()), $joinPart->getIndexBy());
-
-            $queryBuilderClone->add('join', [$join], true);
+            $replacements[] = "$newAlias.";
         }
 
         $queryBuilderClone->add('where', str_replace($aliases, $replacements, (string) $wherePart));

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

@@ -13,6 +13,7 @@
 
 namespace ApiPlatform\Core\Bridge\Doctrine\Orm\Extension;
 
+use ApiPlatform\Core\Bridge\Doctrine\Orm\Util\QueryBuilderHelper;
 use ApiPlatform\Core\Bridge\Doctrine\Orm\Util\QueryNameGeneratorInterface;
 use ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface;
 use Doctrine\ORM\QueryBuilder;
@@ -47,10 +48,18 @@ public function applyToCollection(QueryBuilder $queryBuilder, QueryNameGenerator
             if (null !== $defaultOrder) {
                 foreach ($defaultOrder as $field => $order) {
                     if (is_int($field)) {
+                        // Default direction
                         $field = $order;
                         $order = 'ASC';
                     }
-                    $queryBuilder->addOrderBy('o.'.$field, $order);
+                    if (false === ($pos = strpos($field, '.'))) {
+                        // Configure default filter with property
+                        $field = 'o.'.$field;
+                    } else {
+                        $alias = QueryBuilderHelper::addJoinOnce($queryBuilder, $queryNameGenerator, 'o', substr($field, 0, $pos));
+                        $field = sprintf('%s.%s', $alias, substr($field, $pos + 1));
+                    }
+                    $queryBuilder->addOrderBy($field, $order);
                 }
 
                 return;

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

@@ -13,13 +13,12 @@
 
 namespace ApiPlatform\Core\Bridge\Doctrine\Orm\Filter;
 
-use ApiPlatform\Core\Bridge\Doctrine\Orm\Util\QueryChecker;
+use ApiPlatform\Core\Bridge\Doctrine\Orm\Util\QueryBuilderHelper;
 use ApiPlatform\Core\Bridge\Doctrine\Orm\Util\QueryNameGeneratorInterface;
 use ApiPlatform\Core\Exception\InvalidArgumentException;
 use ApiPlatform\Core\Util\RequestParser;
 use Doctrine\Common\Persistence\ManagerRegistry;
 use Doctrine\Common\Persistence\Mapping\ClassMetadata;
-use Doctrine\ORM\Query\Expr\Join;
 use Doctrine\ORM\QueryBuilder;
 use Psr\Log\LoggerInterface;
 use Psr\Log\NullLogger;
@@ -332,7 +331,7 @@ protected function addJoinsForNestedProperty(string $property, string $rootAlias
         $parentAlias = $rootAlias;
 
         foreach ($propertyParts['associations'] as $association) {
-            $alias = $this->addJoinOnce($queryBuilder, $queryNameGenerator, $parentAlias, $association);
+            $alias = QueryBuilderHelper::addJoinOnce($queryBuilder, $queryNameGenerator, $parentAlias, $association);
             $parentAlias = $alias;
         }
 
@@ -342,61 +341,4 @@ protected function addJoinsForNestedProperty(string $property, string $rootAlias
 
         return [$alias, $propertyParts['field'], $propertyParts['associations']];
     }
-
-    /**
-     * Get the existing join from queryBuilder DQL parts.
-     *
-     * @param QueryBuilder $queryBuilder
-     * @param string       $alias
-     * @param string       $association  the association field
-     *
-     * @return Join|null
-     */
-    private function getExistingJoin(QueryBuilder $queryBuilder, string $alias, string $association)
-    {
-        $parts = $queryBuilder->getDQLPart('join');
-
-        if (!isset($parts['o'])) {
-            return null;
-        }
-
-        foreach ($parts['o'] as $join) {
-            if (sprintf('%s.%s', $alias, $association) === $join->getJoin()) {
-                return $join;
-            }
-        }
-
-        return null;
-    }
-
-    /**
-     * Adds a join to the queryBuilder if none exists.
-     *
-     * @param QueryBuilder                $queryBuilder
-     * @param QueryNameGeneratorInterface $queryNameGenerator
-     * @param string                      $alias
-     * @param string                      $association        the association field
-     *
-     * @return string the new association alias
-     */
-    protected function addJoinOnce(QueryBuilder $queryBuilder, QueryNameGeneratorInterface $queryNameGenerator, string $alias, string $association): string
-    {
-        $join = $this->getExistingJoin($queryBuilder, $alias, $association);
-
-        if (null === $join) {
-            $associationAlias = $queryNameGenerator->generateJoinAlias($association);
-
-            if (true === QueryChecker::hasLeftJoin($queryBuilder)) {
-                $queryBuilder
-                    ->leftJoin(sprintf('%s.%s', $alias, $association), $associationAlias);
-            } else {
-                $queryBuilder
-                    ->innerJoin(sprintf('%s.%s', $alias, $association), $associationAlias);
-            }
-        } else {
-            $associationAlias = $join->getAlias();
-        }
-
-        return $associationAlias;
-    }
 }

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

@@ -14,6 +14,7 @@
 namespace ApiPlatform\Core\Bridge\Doctrine\Orm\Filter;
 
 use ApiPlatform\Core\Api\IriConverterInterface;
+use ApiPlatform\Core\Bridge\Doctrine\Orm\Util\QueryBuilderHelper;
 use ApiPlatform\Core\Bridge\Doctrine\Orm\Util\QueryNameGeneratorInterface;
 use ApiPlatform\Core\Exception\InvalidArgumentException;
 use Doctrine\Common\Persistence\ManagerRegistry;
@@ -251,7 +252,7 @@ protected function filterProperty(string $property, $value, QueryBuilder $queryB
         $valueParameter = $queryNameGenerator->generateParameterName($association);
 
         if ($metadata->isCollectionValuedAssociation($association)) {
-            $associationAlias = $this->addJoinOnce($queryBuilder, $queryNameGenerator, $alias, $association);
+            $associationAlias = QueryBuilderHelper::addJoinOnce($queryBuilder, $queryNameGenerator, $alias, $association);
             $associationField = 'id';
         } else {
             $associationAlias = $alias;

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

@@ -0,0 +1,75 @@
+<?php
+
+/*
+ * This file is part of the API Platform project.
+ *
+ * (c) Kévin Dunglas <[email protected]>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+declare(strict_types=1);
+
+namespace ApiPlatform\Core\Bridge\Doctrine\Orm\Util;
+
+use Doctrine\ORM\Query\Expr\Join;
+use Doctrine\ORM\QueryBuilder;
+
+/**
+ * @author Vincent Chalamon <[email protected]>
+ *
+ * @internal
+ */
+final class QueryBuilderHelper
+{
+    private function __construct()
+    {
+    }
+
+    /**
+     * Adds a join to the queryBuilder if none exists.
+     */
+    public static function addJoinOnce(QueryBuilder $queryBuilder, QueryNameGeneratorInterface $queryNameGenerator, string $alias, string $association, string $joinType = null, string $conditionType = null, string $condition = null): string
+    {
+        $join = self::getExistingJoin($queryBuilder, $alias, $association);
+
+        if (null !== $join) {
+            return $join->getAlias();
+        }
+
+        $associationAlias = $queryNameGenerator->generateJoinAlias($association);
+        $query = "$alias.$association";
+
+        if (Join::LEFT_JOIN === $joinType || QueryChecker::hasLeftJoin($queryBuilder)) {
+            $queryBuilder->leftJoin($query, $associationAlias, $conditionType, $condition);
+        } else {
+            $queryBuilder->innerJoin($query, $associationAlias, $conditionType, $condition);
+        }
+
+        return $associationAlias;
+    }
+
+    /**
+     * Get the existing join from queryBuilder DQL parts.
+     *
+     * @return Join|null
+     */
+    private static function getExistingJoin(QueryBuilder $queryBuilder, string $alias, string $association)
+    {
+        $parts = $queryBuilder->getDQLPart('join');
+
+        if (!isset($parts['o'])) {
+            return null;
+        }
+
+        foreach ($parts['o'] as $join) {
+            /** @var Join $join */
+            if (sprintf('%s.%s', $alias, $association) === $join->getJoin()) {
+                return $join;
+            }
+        }
+
+        return null;
+    }
+}

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

@@ -269,8 +269,8 @@ public function testCompositeIdentifiers()
             ->setParameter('foo', 1);
 
         $queryNameGenerator = $this->prophesize(QueryNameGeneratorInterface::class);
-        $queryNameGenerator->generateJoinAlias('item')->shouldBeCalled()->willReturn('item_2');
-        $queryNameGenerator->generateJoinAlias('label')->shouldBeCalled()->willReturn('label_2');
+        $queryNameGenerator->generateJoinAlias('compositeItem')->shouldBeCalled()->willReturn('item_2');
+        $queryNameGenerator->generateJoinAlias('compositeLabel')->shouldBeCalled()->willReturn('label_2');
         $queryNameGenerator->generateJoinAlias('o')->shouldBeCalled()->willReturn('o_2');
 
         $filterEagerLoadingExtension = new FilterEagerLoadingExtension($resourceMetadataFactoryProphecy->reveal(), true);
@@ -325,8 +325,8 @@ public function testFetchEagerWithNoForceEager()
             ->setParameter('foo', 1);
 
         $queryNameGenerator = $this->prophesize(QueryNameGeneratorInterface::class);
-        $queryNameGenerator->generateJoinAlias('item')->shouldBeCalled()->willReturn('item_2');
-        $queryNameGenerator->generateJoinAlias('label')->shouldBeCalled()->willReturn('label_2');
+        $queryNameGenerator->generateJoinAlias('compositeItem')->shouldBeCalled()->willReturn('item_2');
+        $queryNameGenerator->generateJoinAlias('compositeLabel')->shouldBeCalled()->willReturn('label_2');
         $queryNameGenerator->generateJoinAlias('o')->shouldBeCalled()->willReturn('o_2');
 
         $queryNameGenerator->generateJoinAlias('foo')->shouldBeCalled()->willReturn('foo_2');
@@ -388,8 +388,8 @@ public function testCompositeIdentifiersWithoutAssociation()
             ->setParameter('bar', 2);
 
         $queryNameGenerator = $this->prophesize(QueryNameGeneratorInterface::class);
-        $queryNameGenerator->generateJoinAlias('item')->shouldBeCalled()->willReturn('item_2');
-        $queryNameGenerator->generateJoinAlias('label')->shouldBeCalled()->willReturn('label_2');
+        $queryNameGenerator->generateJoinAlias('compositeItem')->shouldBeCalled()->willReturn('item_2');
+        $queryNameGenerator->generateJoinAlias('compositeLabel')->shouldBeCalled()->willReturn('label_2');
         $queryNameGenerator->generateJoinAlias('o')->shouldBeCalled()->willReturn('o_2');
 
         $filterEagerLoadingExtension = new FilterEagerLoadingExtension($resourceMetadataFactoryProphecy->reveal(), true);