fix dbal pagination count with groupBy

garak · Jul 29, 2024 · 427ef3a · 427ef3a
1 parent 59ef316
commit 427ef3a
Show file tree

Hide file tree

Showing 9 changed files with 41 additions and 44 deletions.
diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon
@@ -1,10 +1,5 @@
 parameters:
 	ignoreErrors:
-		-
-			message: "#^Call to an undefined method Doctrine\\\\DBAL\\\\Query\\\\QueryBuilder\\:\\:resetQueryParts\\(\\)\\.$#"
-			count: 1
-			path: src/Knp/Component/Pager/Event/Subscriber/Paginate/Doctrine/DBALQueryBuilderSubscriber.php
-
 		-
 			message: "#^Method Knp\\\\Component\\\\Pager\\\\Event\\\\Subscriber\\\\Sortable\\\\ArraySubscriber\\:\\:sortFunction\\(\\) has parameter \\$object1 with no value type specified in iterable type array\\.$#"
 			count: 1

diff --git a/src/Knp/Component/Pager/Event/BeforeEvent.php b/src/Knp/Component/Pager/Event/BeforeEvent.php
@@ -2,6 +2,7 @@
 
 namespace Knp\Component\Pager\Event;
 
+use Doctrine\DBAL\Connection;
 use Knp\Component\Pager\ArgumentAccess\ArgumentAccessInterface;
 use Symfony\Contracts\EventDispatcher\Event;
 use Symfony\Contracts\EventDispatcher\EventDispatcherInterface;
@@ -11,14 +12,11 @@
  */
 final class BeforeEvent extends Event
 {
-    private EventDispatcherInterface $eventDispatcher;
-
-    private ArgumentAccessInterface $argumentAccess;
-
-    public function __construct(EventDispatcherInterface $eventDispatcher, ArgumentAccessInterface $argumentAccess)
-    {
-        $this->eventDispatcher = $eventDispatcher;
-        $this->argumentAccess = $argumentAccess;
+    public function __construct(
+        private EventDispatcherInterface $eventDispatcher,
+        private ArgumentAccessInterface $argumentAccess,
+        private ?Connection $connection = null
+    ) {
     }
 
     public function getEventDispatcher(): EventDispatcherInterface
@@ -30,4 +28,9 @@ public function getArgumentAccess(): ArgumentAccessInterface
     {
         return $this->argumentAccess;
     }
+
+    public function getConnection(): ?Connection
+    {
+        return $this->connection;
+    }
 }
diff --git a/src/Knp/Component/Pager/Event/Subscriber/Paginate/Doctrine/DBALQueryBuilderSubscriber.php b/src/Knp/Component/Pager/Event/Subscriber/Paginate/Doctrine/DBALQueryBuilderSubscriber.php
@@ -2,6 +2,7 @@
 
 namespace Knp\Component\Pager\Event\Subscriber\Paginate\Doctrine;
 
+use Doctrine\DBAL\Connection;
 use Doctrine\DBAL\Query\QueryBuilder;
 use Knp\Component\Pager\Event\ItemsEvent;
 use Symfony\Component\EventDispatcher\EventSubscriberInterface;
@@ -13,26 +14,21 @@
  */
 class DBALQueryBuilderSubscriber implements EventSubscriberInterface
 {
+    public function __construct(private Connection $connection)
+    {
+    }
+
     public function items(ItemsEvent $event): void
     {
         if ($event->target instanceof QueryBuilder) {
             $target = $event->target;
-
-            // count results
-            $qb = clone $target;
-
-            //reset count orderBy since it can break query and slow it down
-            if (method_exists($qb, 'resetOrderBy')) {
-                $qb->resetOrderBy();
-            } else {
-                $qb->resetQueryParts(['orderBy']);
-            }
-
-            // get the query
-            $sql = $qb->getSQL();
 
-            $qb
-                ->select('count(*) as cnt')
+            $qb = $this
+                ->connection
+                ->createQueryBuilder()
+                ->select('COUNT(*)')
+                ->from('(' . $target->getSQL() . ')', 'tmp')
+                ->setParameters($target->getParameters(), $target->getParameterTypes())
             ;
 
             $compat = $qb->executeQuery();

diff --git a/src/Knp/Component/Pager/Event/Subscriber/Paginate/PaginationSubscriber.php b/src/Knp/Component/Pager/Event/Subscriber/Paginate/PaginationSubscriber.php
@@ -39,7 +39,9 @@ public function before(BeforeEvent $event): void
         $dispatcher->addSubscriber(new Doctrine\ODM\PHPCR\QueryBuilderSubscriber);
         $dispatcher->addSubscriber(new Doctrine\ODM\PHPCR\QuerySubscriber);
         $dispatcher->addSubscriber(new Doctrine\CollectionSubscriber);
-        $dispatcher->addSubscriber(new Doctrine\DBALQueryBuilderSubscriber);
+        if (null !== $connection = $event->getConnection()) {
+            $dispatcher->addSubscriber(new Doctrine\DBALQueryBuilderSubscriber($connection));
+        }
         $dispatcher->addSubscriber(new PropelQuerySubscriber);
         $dispatcher->addSubscriber(new SolariumQuerySubscriber());
         $dispatcher->addSubscriber(new ElasticaQuerySubscriber());

diff --git a/src/Knp/Component/Pager/Paginator.php b/src/Knp/Component/Pager/Paginator.php
@@ -2,6 +2,7 @@
 
 namespace Knp\Component\Pager;
 
+use Doctrine\DBAL\Connection;
 use Knp\Component\Pager\ArgumentAccess\ArgumentAccessInterface;
 use Knp\Component\Pager\Exception\PageLimitInvalidException;
 use Knp\Component\Pager\Exception\PageNumberInvalidException;
@@ -17,8 +18,6 @@
  */
 final class Paginator implements PaginatorInterface
 {
-    private EventDispatcherInterface $eventDispatcher;
-
     /**
      * Default options of paginator
      *
@@ -35,12 +34,11 @@ final class Paginator implements PaginatorInterface
         self::DEFAULT_LIMIT => self::DEFAULT_LIMIT_VALUE,
     ];
 
-    private ArgumentAccessInterface $argumentAccess;
-
-    public function __construct(EventDispatcherInterface $eventDispatcher, ArgumentAccessInterface $argumentAccess)
-    {
-        $this->eventDispatcher = $eventDispatcher;
-        $this->argumentAccess = $argumentAccess;
+    public function __construct(
+        private EventDispatcherInterface $eventDispatcher,
+        private ArgumentAccessInterface $argumentAccess,
+        private ?Connection $connection = null
+    ) {
     }
 
     /**
@@ -89,7 +87,7 @@ public function paginate($target, int $page = 1, int $limit = null, array $optio
         }
 
         // before pagination start
-        $beforeEvent = new Event\BeforeEvent($this->eventDispatcher, $this->argumentAccess);
+        $beforeEvent = new Event\BeforeEvent($this->eventDispatcher, $this->argumentAccess, $this->connection);
         $this->eventDispatcher->dispatch($beforeEvent, 'knp_pager.before');
         // items
         $itemsEvent = new Event\ItemsEvent($offset, $limit);

diff --git a/tests/Test/Fixture/Document/Image.php b/tests/Test/Fixture/Document/Image.php
@@ -2,7 +2,6 @@
 
 namespace Test\Fixture\Document;
 
-use Doctrine\ODM\PHPCR\Mapping\Annotations as PHPCRAnnotations;
 use Doctrine\ODM\MongoDB\Mapping\Annotations as ODM;
 
 /**

diff --git a/tests/Test/Pager/Subscriber/Paginate/Doctrine/DBALQueryBuilderTest.php b/tests/Test/Pager/Subscriber/Paginate/Doctrine/DBALQueryBuilderTest.php
@@ -14,7 +14,7 @@ final class DBALQueryBuilderTest extends BaseTestCaseORM
     public function shouldPaginateSimpleDoctrineQuery(): void
     {
         $this->populate();
-        $p = $this->getPaginatorInstance();
+        $p = $this->getPaginatorInstance(null, null, $this->em->getConnection());
 
         $qb = new QueryBuilder($this->em->getConnection());
         $qb->select('*')

diff --git a/tests/Test/Pager/Subscriber/Paginate/PaginationSubscriberTest.php b/tests/Test/Pager/Subscriber/Paginate/PaginationSubscriberTest.php
@@ -16,7 +16,7 @@ final class PaginationSubscriberTest extends BaseTestCase
     public function shouldRegisterExpectedSubscribersOnlyOnce(): void
     {
         $dispatcher = $this->getMockBuilder(EventDispatcherInterface::class)->getMock();
-        $dispatcher->expects($this->exactly(13))->method('addSubscriber');
+        $dispatcher->expects($this->exactly(12))->method('addSubscriber');
 
         $subscriber = new PaginationSubscriber;
 

diff --git a/tests/Test/Tool/BaseTestCase.php b/tests/Test/Tool/BaseTestCase.php
@@ -2,6 +2,7 @@
 
 namespace Test\Tool;
 
+use Doctrine\DBAL\Connection;
 use Knp\Component\Pager\ArgumentAccess\ArgumentAccessInterface;
 use Knp\Component\Pager\ArgumentAccess\RequestArgumentAccess;
 use Knp\Component\Pager\Event\Subscriber\Paginate\PaginationSubscriber;
@@ -17,8 +18,11 @@
  */
 abstract class BaseTestCase extends TestCase
 {
-    protected function getPaginatorInstance(?RequestStack $requestStack = null, ?EventDispatcher $dispatcher = null): Paginator
-    {
+    protected function getPaginatorInstance(
+        ?RequestStack $requestStack = null,
+        ?EventDispatcher $dispatcher = null,
+        ?Connection $connection = null
+    ): Paginator {
         if (null === $dispatcher) {
             $dispatcher = new EventDispatcher();
             $dispatcher->addSubscriber(new PaginationSubscriber());
@@ -30,7 +34,7 @@ protected function getPaginatorInstance(?RequestStack $requestStack = null, ?Eve
             $accessor = $this->createMock(ArgumentAccessInterface::class);
         }
 
-        return new Paginator($dispatcher, $accessor);
+        return new Paginator($dispatcher, $accessor, $connection);
     }
 
     protected function createRequestStack(array $params): RequestStack