Deprecate not returning a query builder from the DBAL adapter's modif…

…ier callback

CHANGELOG.md

@@ -3,6 +3,7 @@
 ## 4.6.0 (2024-??-??)
 
 - Add support for `ruflin/elastica` 8.x
+- Deprecate not returning a query builder from the modifier callback in `Pagerfanta\Doctrine\DBAL\QueryAdapter`
 
 ## 4.5.0 (2024-04-10)
 

composer.json

@@ -6,7 +6,8 @@
     "license": "MIT",
     "require": {
         "php": "^8.1",
-        "ext-json": "*"
+        "ext-json": "*",
+        "symfony/deprecation-contracts": "^2.1 || ^3.0"
     },
     "require-dev": {
         "doctrine/collections": "^1.8 || ^2.0",

docs/adapters.md

@@ -107,7 +107,9 @@ $adapter = new SingleTableQueryAdapter($query, 'p.id');
 
 The `QueryAdapter` is the main adapter for use with the DBAL package, you should use this on queries that have join statements.
 
-The class constructor requires a `Doctrine\DBAL\Query\QueryBuilder` and a callable which can be used to modify a clone of the `QueryBuilder` for a COUNT query. The callable should have a signature of `function (QueryBuilder $queryBuilder): void {}`.
+The class constructor requires a `Doctrine\DBAL\Query\QueryBuilder` and a callable which can be used to modify a clone of the query builder for a COUNT query. The callable should have a signature of `function (QueryBuilder $queryBuilder): QueryBuilder {}`.
+
+<div class="docs-note docs-note--deprecated-feature">Before Pagerfanta 4.6, a return from the callable was ignored and the query builder passed to the callable was returned. This approach is deprecated in favor of the callable returning a query builder object, be it the provided builder or a new instance. In Pagerfanta 5.0, this return will be required.</div>
 
 Below is an example of using the `QueryAdapter`.
 
@@ -129,9 +131,11 @@ $query = $connection->createQueryBuilder()
     ->select('p.*')
     ->from('posts', 'p');
 
-$countQueryBuilderModifier = static function (QueryBuilder $queryBuilder): void {
+$countQueryBuilderModifier = static function (QueryBuilder $queryBuilder): QueryBuilder {
     $queryBuilder->select('COUNT(DISTINCT p.id) AS total_results')
         ->setMaxResults(1);
+
+    return $queryBuilder;
 };
 
 $adapter = new QueryAdapter($query, $countQueryBuilderModifier);

lib/Adapter/Doctrine/DBAL/QueryAdapter.php

@@ -19,12 +19,12 @@ class QueryAdapter implements AdapterInterface
     /**
      * @var callable
      *
-     * @phpstan-var callable(QueryBuilder): void
+     * @phpstan-var callable(QueryBuilder): (QueryBuilder|void)
      */
     private $countQueryBuilderModifier;
 
     /**
-     * @phpstan-param callable(QueryBuilder): void $countQueryBuilderModifier
+     * @phpstan-param callable(QueryBuilder): (QueryBuilder|void) $countQueryBuilderModifier
      */
     public function __construct(QueryBuilder $queryBuilder, callable $countQueryBuilderModifier)
     {
@@ -63,7 +63,13 @@ private function prepareCountQueryBuilder(): QueryBuilder
         $qb = clone $this->queryBuilder;
         $callable = $this->countQueryBuilderModifier;
 
-        $callable($qb);
+        $newQb = $callable($qb);
+
+        if ($newQb instanceof QueryBuilder) {
+            return $newQb;
+        }
+
+        trigger_deprecation('pagerfanta/doctrine-dbal-adapter', '4.6', 'Not returning a "%s" from the query builder modifier in "%s" is deprecated. In 5.0, returning a query builder object will be required.');
 
         return $qb;
     }

lib/Adapter/Doctrine/DBAL/SingleTableQueryAdapter.php

@@ -28,7 +28,7 @@ private function createCountQueryModifier(string $countField): \Closure
     {
         $select = $this->createSelectForCountField($countField);
 
-        return static function (QueryBuilder $queryBuilder) use ($select): void {
+        return static function (QueryBuilder $queryBuilder) use ($select): QueryBuilder {
             $queryBuilder->select($select);
 
             if (method_exists($queryBuilder, 'resetOrderBy')) {
@@ -38,6 +38,8 @@ private function createCountQueryModifier(string $countField): \Closure
             }
 
             $queryBuilder->setMaxResults(1);
+
+            return $queryBuilder;
         };
     }
 

lib/Adapter/Doctrine/DBAL/Tests/QueryAdapterTest.php

@@ -4,6 +4,7 @@
 
 use Doctrine\DBAL\Query\QueryBuilder;
 use Pagerfanta\Doctrine\DBAL\QueryAdapter;
+use PHPUnit\Framework\Attributes\Group;
 
 final class QueryAdapterTest extends DBALTestCase
 {
@@ -17,6 +18,20 @@ protected function setUp(): void
         $this->qb->select('p.*')->from('posts', 'p');
     }
 
+    #[Group('legacy')]
+    public function testAdapterReturnsNumberOfResultsWhenCallbackUsesDeprecatedNoReturnSignature(): void
+    {
+        $adapter = new QueryAdapter(
+            $this->qb,
+            static function (QueryBuilder $qb): void {
+                $qb->select('COUNT(DISTINCT p.id) AS total_results')
+                    ->setMaxResults(1);
+            }
+        );
+
+        self::assertSame(50, $adapter->getNbResults());
+    }
+
     public function testAdapterReturnsNumberOfResults(): void
     {
         $adapter = $this->createAdapterToTestGetNbResults();
@@ -35,7 +50,7 @@ public function testResultCountStaysConsistentAfterSlicing(): void
 
     public function testGetSlice(): void
     {
-        $adapter = new QueryAdapter($this->qb, static function (QueryBuilder $qb): void {});
+        $adapter = new QueryAdapter($this->qb, static fn (QueryBuilder $qb): QueryBuilder => $qb);
 
         $offset = 30;
         $length = 10;
@@ -63,10 +78,7 @@ private function createAdapterToTestGetNbResults(): QueryAdapter
     {
         return new QueryAdapter(
             $this->qb,
-            static function (QueryBuilder $qb): void {
-                $qb->select('COUNT(DISTINCT p.id) AS total_results')
-                    ->setMaxResults(1);
-            }
+            static fn (QueryBuilder $qb): QueryBuilder => $qb->select('COUNT(DISTINCT p.id) AS total_results')->setMaxResults(1),
         );
     }
 }

lib/Adapter/Doctrine/DBAL/composer.json

@@ -7,7 +7,8 @@
     "require": {
         "php": "^8.1",
         "doctrine/dbal": "^3.5 || ^4.0",
-        "pagerfanta/core": "^3.7 || ^4.0"
+        "pagerfanta/core": "^3.7 || ^4.0",
+        "symfony/deprecation-contracts": "^2.1 || ^3.0"
     },
     "require-dev": {
         "phpunit/phpunit": "^10.5"