Fix detection of use of output walkers in pagination (#1616) (#1623)

Co-authored-by: Fran Moreno <[email protected]>

src/Util/SmartPaginatorFactory.php

@@ -13,6 +13,7 @@
 
 namespace Sonata\DoctrineORMAdminBundle\Util;
 
+use Doctrine\ORM\QueryBuilder;
 use Doctrine\ORM\Tools\Pagination\CountWalker;
 use Doctrine\ORM\Tools\Pagination\Paginator;
 use Sonata\DoctrineORMAdminBundle\Datagrid\ProxyQuery;
@@ -55,13 +56,91 @@ public static function create(ProxyQuery $proxyQuery, array $hints = []): Pagina
         // To stay safe fetch join only when we have single primary key and joins
         $paginator = new Paginator($query, $hasSingleIdentifierName && $hasJoins);
 
-        $hasHavingPart = null !== $queryBuilder->getDQLPart('having');
-
         // it is only safe to disable output walkers for really simple queries
-        if (!$hasHavingPart && $hasSingleIdentifierName) {
+        if (self::canDisableOutPutWalkers($queryBuilder)) {
             $paginator->setUseOutputWalkers(false);
         }
 
         return $paginator;
     }
+
+    /**
+     * @see https://github.com/doctrine/orm/issues/8278#issue-705517756
+     */
+    private static function canDisableOutPutWalkers(QueryBuilder $queryBuilder): bool
+    {
+        // does not support queries using HAVING
+        if (null !== $queryBuilder->getDQLPart('having')) {
+            return false;
+        }
+
+        $fromParts = $queryBuilder->getDQLPart('from');
+
+        // does not support queries using multiple entities in FROM
+        if (1 !== \count($fromParts)) {
+            return false;
+        }
+
+        $fromPart = current($fromParts);
+
+        $classMetadata = $queryBuilder
+            ->getEntityManager()
+            ->getClassMetadata($fromPart->getFrom());
+
+        $identifierFieldNames = $classMetadata->getIdentifierFieldNames();
+
+        // does not support entities using a composite identifier
+        if (1 !== \count($identifierFieldNames)) {
+            return false;
+        }
+
+        $identifierName = current($identifierFieldNames);
+
+        // does not support entities using a foreign key as identifier
+        if ($classMetadata->hasAssociation($identifierName)) {
+            return false;
+        }
+
+        // does not support queries using a field from a toMany relation in the ORDER BY clause
+        if (self::hasOrderByWithToManyAssociation($queryBuilder)) {
+            return false;
+        }
+
+        return true;
+    }
+
+    private static function hasOrderByWithToManyAssociation(QueryBuilder $queryBuilder): bool
+    {
+        $joinParts = $queryBuilder->getDQLPart('join');
+
+        if (0 === \count($joinParts)) {
+            return false;
+        }
+
+        $orderByParts = $queryBuilder->getDQLPart('orderBy');
+
+        if (0 === \count($orderByParts)) {
+            return false;
+        }
+
+        $joinAliases = [];
+
+        foreach ($joinParts as $joinPart) {
+            foreach ($joinPart as $join) {
+                $joinAliases[] = $join->getAlias();
+            }
+        }
+
+        foreach ($orderByParts as $orderByPart) {
+            foreach ($orderByPart->getParts() as $part) {
+                foreach ($joinAliases as $joinAlias) {
+                    if (0 === strpos($part, $joinAlias.'.')) {
+                        return true;
+                    }
+                }
+            }
+        }
+
+        return false;
+    }
 }

tests/App/Entity/ProductAttribute.php

@@ -0,0 +1,51 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the Sonata Project package.
+ *
+ * (c) Thomas Rabaix <[email protected]>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Sonata\DoctrineORMAdminBundle\Tests\App\Entity;
+
+use Doctrine\ORM\Mapping as ORM;
+
+/** @ORM\Entity */
+class ProductAttribute
+{
+    /**
+     * @ORM\Id
+     * @ORM\ManyToOne(targetEntity="Product")
+     *
+     * @var Product
+     */
+    private $product;
+
+    /**
+     * @ORM\Column(type="string")
+     *
+     * @var string
+     */
+    private $name;
+
+    public function __construct(Product $product, string $name)
+    {
+        $this->product = $product;
+        $this->name = $name;
+    }
+
+    public function getProduct(): Product
+    {
+        return $this->product;
+    }
+
+    public function getName(): ?string
+    {
+        return $this->name;
+    }
+}

tests/Util/SmartPaginatorFactoryTest.php

@@ -17,8 +17,10 @@
 use Doctrine\ORM\Tools\Pagination\CountWalker;
 use PHPUnit\Framework\TestCase;
 use Sonata\DoctrineORMAdminBundle\Datagrid\ProxyQuery;
+use Sonata\DoctrineORMAdminBundle\Tests\App\Entity\Address;
 use Sonata\DoctrineORMAdminBundle\Tests\App\Entity\Author;
 use Sonata\DoctrineORMAdminBundle\Tests\App\Entity\Item;
+use Sonata\DoctrineORMAdminBundle\Tests\App\Entity\ProductAttribute;
 use Sonata\DoctrineORMAdminBundle\Tests\Fixtures\TestEntityManagerFactory;
 use Sonata\DoctrineORMAdminBundle\Util\SmartPaginatorFactory;
 
@@ -129,6 +131,48 @@ public function getQueriesForOutputWalker(): iterable
                 ->leftJoin('item.product', 'product'),
             null,
         ];
+
+        yield 'With order by not from join field' => [
+            TestEntityManagerFactory::create()
+                ->createQueryBuilder()
+                ->from(Author::class, 'author')
+                ->leftJoin('author.books', 'book')
+                ->orderBy('author.name'),
+            false,
+        ];
+
+        yield 'With order by not from join field using an alias contained in order by' => [
+            TestEntityManagerFactory::create()
+                ->createQueryBuilder()
+                ->from(Author::class, 'author')
+                ->leftJoin('author.books', 'a')
+                ->orderBy('author.name'),
+            false,
+        ];
+
+        yield 'With order by from join field' => [
+            TestEntityManagerFactory::create()
+                ->createQueryBuilder()
+                ->from(Author::class, 'author')
+                ->leftJoin('author.books', 'book')
+                ->orderBy('book.title'),
+            null,
+        ];
+
+        yield 'With foreign key as identifier' => [
+            TestEntityManagerFactory::create()
+                ->createQueryBuilder()
+                ->from(ProductAttribute::class, 'productAttribute'),
+            null,
+        ];
+
+        yield 'With multiple FROM' => [
+            TestEntityManagerFactory::create()
+                ->createQueryBuilder()
+                ->from(Author::class, 'author')
+                ->from(Address::class, 'address'),
+            null,
+        ];
     }
 
     /**