Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix dbal pagination count with groupBy #337

Merged
merged 1 commit into from
Jul 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions phpstan-baseline.neon
Original file line number Diff line number Diff line change
@@ -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
Expand Down
19 changes: 11 additions & 8 deletions src/Knp/Component/Pager/Event/BeforeEvent.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand All @@ -30,4 +28,9 @@ public function getArgumentAccess(): ArgumentAccessInterface
{
return $this->argumentAccess;
}

public function getConnection(): ?Connection
{
return $this->connection;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
16 changes: 7 additions & 9 deletions src/Knp/Component/Pager/Paginator.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -17,8 +18,6 @@
*/
final class Paginator implements PaginatorInterface
{
private EventDispatcherInterface $eventDispatcher;

/**
* Default options of paginator
*
Expand All @@ -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
) {
}

/**
Expand Down Expand Up @@ -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);
Expand Down
1 change: 0 additions & 1 deletion tests/Test/Fixture/Document/Image.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

namespace Test\Fixture\Document;

use Doctrine\ODM\PHPCR\Mapping\Annotations as PHPCRAnnotations;
use Doctrine\ODM\MongoDB\Mapping\Annotations as ODM;

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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('*')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
10 changes: 7 additions & 3 deletions tests/Test/Tool/BaseTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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());
Expand All @@ -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
Expand Down