diff --git a/CHANGELOG.md b/CHANGELOG.md index 5a47e75d..3a3a6d9b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ - Fix the Twig view reusing the template from a previous render's options when no default template is configured - Fix the template views reusing the options from a previous render - Add support for `ruflin/elastica` 9.x +- [#66](https://github.com/BabDev/Pagerfanta/issues/66) Improved handling of zero-length slices in the pagination adapters ## 4.9.0 (2026-09-08) diff --git a/lib/Adapter/Doctrine/Collections/SelectableAdapter.php b/lib/Adapter/Doctrine/Collections/SelectableAdapter.php index 20669783..3e12c0b9 100644 --- a/lib/Adapter/Doctrine/Collections/SelectableAdapter.php +++ b/lib/Adapter/Doctrine/Collections/SelectableAdapter.php @@ -40,6 +40,10 @@ public function getNbResults(): int */ public function getSlice(int $offset, int $length): iterable { + if (0 === $length) { + return []; + } + return $this->selectable->matching($this->createCriteria($offset, $length)); } diff --git a/lib/Adapter/Doctrine/Collections/Tests/CollectionAdapterTest.php b/lib/Adapter/Doctrine/Collections/Tests/CollectionAdapterTest.php index cad5ccab..6c0205f5 100644 --- a/lib/Adapter/Doctrine/Collections/Tests/CollectionAdapterTest.php +++ b/lib/Adapter/Doctrine/Collections/Tests/CollectionAdapterTest.php @@ -38,4 +38,10 @@ public function testGetResultsShouldReturnTheCollectionSliceReturnValue(): void $this->assertSame(array_values(range(6, 17)), array_values($slice)); } + + public function testGetSliceWithZeroLength(): void + { + $this->assertSame([], $this->adapter->getSlice(0, 0)); + $this->assertSame([], $this->adapter->getSlice(5, 0)); + } } diff --git a/lib/Adapter/Doctrine/Collections/Tests/SelectableAdapterTest.php b/lib/Adapter/Doctrine/Collections/Tests/SelectableAdapterTest.php index a513e292..e68a0fe0 100644 --- a/lib/Adapter/Doctrine/Collections/Tests/SelectableAdapterTest.php +++ b/lib/Adapter/Doctrine/Collections/Tests/SelectableAdapterTest.php @@ -2,6 +2,7 @@ namespace Pagerfanta\Doctrine\Collections\Tests; +use Doctrine\Common\Collections\ArrayCollection; use Doctrine\Common\Collections\Collection; use Doctrine\Common\Collections\Criteria; use Doctrine\Common\Collections\Order; @@ -92,4 +93,12 @@ public function testGetSlice(): void $this->assertSame($slice, $this->adapter->getSlice(10, 20)); } + + public function testGetSliceWithZeroLength(): void + { + $adapter = new SelectableAdapter(new ArrayCollection(range(1, 10)), Criteria::create(true)); + + $this->assertSame([], [...$adapter->getSlice(0, 0)]); + $this->assertSame([], [...$adapter->getSlice(5, 0)]); + } } diff --git a/lib/Adapter/Doctrine/DBAL/Tests/QueryAdapterTest.php b/lib/Adapter/Doctrine/DBAL/Tests/QueryAdapterTest.php index d5087ce0..1d5367f5 100644 --- a/lib/Adapter/Doctrine/DBAL/Tests/QueryAdapterTest.php +++ b/lib/Adapter/Doctrine/DBAL/Tests/QueryAdapterTest.php @@ -61,6 +61,14 @@ public function testGetSlice(): void $this->assertSame($this->qb->executeQuery()->fetchAllAssociative(), $adapter->getSlice($offset, $length)); } + public function testGetSliceWithZeroLength(): void + { + $adapter = new QueryAdapter($this->qb, static fn (QueryBuilder $qb): QueryBuilder => $qb); + + $this->assertSame([], $adapter->getSlice(0, 0)); + $this->assertSame([], $adapter->getSlice(30, 0)); + } + public function testTheAdapterUsesAClonedQuery(): void { $adapter = $this->createAdapterToTestGetNbResults(); diff --git a/lib/Adapter/Doctrine/DBAL/Tests/SingleTableQueryAdapterTest.php b/lib/Adapter/Doctrine/DBAL/Tests/SingleTableQueryAdapterTest.php index 8353d78d..f5c49aee 100644 --- a/lib/Adapter/Doctrine/DBAL/Tests/SingleTableQueryAdapterTest.php +++ b/lib/Adapter/Doctrine/DBAL/Tests/SingleTableQueryAdapterTest.php @@ -56,4 +56,10 @@ public function testGetSlice(): void $this->assertSame($q->executeQuery()->fetchAllAssociative(), $this->adapter->getSlice($offset, $length)); } + + public function testGetSliceWithZeroLength(): void + { + $this->assertSame([], $this->adapter->getSlice(0, 0)); + $this->assertSame([], $this->adapter->getSlice(30, 0)); + } } diff --git a/lib/Adapter/Doctrine/MongoDBODM/AggregationAdapter.php b/lib/Adapter/Doctrine/MongoDBODM/AggregationAdapter.php index 4e35f016..ba64768e 100644 --- a/lib/Adapter/Doctrine/MongoDBODM/AggregationAdapter.php +++ b/lib/Adapter/Doctrine/MongoDBODM/AggregationAdapter.php @@ -41,6 +41,10 @@ public function getNbResults(): int */ public function getSlice(int $offset, int $length): iterable { + if (0 === $length) { + return []; + } + $aggregationBuilder = clone $this->aggregationBuilder; return $aggregationBuilder diff --git a/lib/Adapter/Doctrine/MongoDBODM/QueryAdapter.php b/lib/Adapter/Doctrine/MongoDBODM/QueryAdapter.php index accde02d..3d775b9c 100644 --- a/lib/Adapter/Doctrine/MongoDBODM/QueryAdapter.php +++ b/lib/Adapter/Doctrine/MongoDBODM/QueryAdapter.php @@ -40,6 +40,10 @@ public function getNbResults(): int */ public function getSlice(int $offset, int $length): iterable { + if (0 === $length) { + return []; + } + return $this->queryBuilder->limit($length) ->skip($offset) ->getQuery() diff --git a/lib/Adapter/Doctrine/MongoDBODM/Tests/AggregationAdapterTest.php b/lib/Adapter/Doctrine/MongoDBODM/Tests/AggregationAdapterTest.php index 3ed508a4..8fd7d01a 100644 --- a/lib/Adapter/Doctrine/MongoDBODM/Tests/AggregationAdapterTest.php +++ b/lib/Adapter/Doctrine/MongoDBODM/Tests/AggregationAdapterTest.php @@ -109,4 +109,14 @@ public function testGetSlice(): void $this->assertSame($slice, $this->adapter->getSlice($offset, $length)); } + + public function testGetSliceWithZeroLengthDoesNotExecuteTheAggregation(): void + { + // MongoDB rejects a $limit stage of 0, so the adapter must not run the aggregation + $this->aggregationBuilder->expects($this->never()) + ->method('skip'); + + $this->assertSame([], $this->adapter->getSlice(0, 0)); + $this->assertSame([], $this->adapter->getSlice(10, 0)); + } } diff --git a/lib/Adapter/Doctrine/MongoDBODM/Tests/QueryAdapterTest.php b/lib/Adapter/Doctrine/MongoDBODM/Tests/QueryAdapterTest.php index 4ef7d066..e484c543 100644 --- a/lib/Adapter/Doctrine/MongoDBODM/Tests/QueryAdapterTest.php +++ b/lib/Adapter/Doctrine/MongoDBODM/Tests/QueryAdapterTest.php @@ -88,4 +88,17 @@ public function testGetSlice(): void $this->assertSame($slice, $this->adapter->getSlice($offset, $length)); } + + public function testGetSliceWithZeroLengthDoesNotExecuteTheQuery(): void + { + // A limit of 0 means "no limit" to MongoDB, so the adapter must not run the query + $this->queryBuilder->expects($this->never()) + ->method('limit'); + + $this->queryBuilder->expects($this->never()) + ->method('getQuery'); + + $this->assertSame([], $this->adapter->getSlice(0, 0)); + $this->assertSame([], $this->adapter->getSlice(10, 0)); + } } diff --git a/lib/Adapter/Doctrine/PHPCRODM/QueryAdapter.php b/lib/Adapter/Doctrine/PHPCRODM/QueryAdapter.php index b908563e..84e1dcce 100644 --- a/lib/Adapter/Doctrine/PHPCRODM/QueryAdapter.php +++ b/lib/Adapter/Doctrine/PHPCRODM/QueryAdapter.php @@ -38,6 +38,10 @@ public function getNbResults(): int */ public function getSlice(int $offset, int $length): iterable { + if (0 === $length) { + return []; + } + return $this->queryBuilder->getQuery() ->setMaxResults($length) ->setFirstResult($offset) diff --git a/lib/Adapter/Doctrine/PHPCRODM/Tests/QueryAdapterTest.php b/lib/Adapter/Doctrine/PHPCRODM/Tests/QueryAdapterTest.php index 2c5413fe..20dbc832 100644 --- a/lib/Adapter/Doctrine/PHPCRODM/Tests/QueryAdapterTest.php +++ b/lib/Adapter/Doctrine/PHPCRODM/Tests/QueryAdapterTest.php @@ -73,4 +73,13 @@ public function testGetSlice(): void $this->assertSame($slice, $this->adapter->getSlice($offset, $length)); } + + public function testGetSliceWithZeroLengthDoesNotExecuteTheQuery(): void + { + $this->queryBuilder->expects($this->never()) + ->method('getQuery'); + + $this->assertSame([], $this->adapter->getSlice(0, 0)); + $this->assertSame([], $this->adapter->getSlice(10, 0)); + } } diff --git a/lib/Adapter/Elastica/Tests/ElasticaAdapterTest.php b/lib/Adapter/Elastica/Tests/ElasticaAdapterTest.php index 7cc2b9bd..ca6df835 100644 --- a/lib/Adapter/Elastica/Tests/ElasticaAdapterTest.php +++ b/lib/Adapter/Elastica/Tests/ElasticaAdapterTest.php @@ -68,6 +68,20 @@ public function testGetSlice(): void $this->assertSame($this->resultSet, $this->adapter->getResultSet()); } + /** + * A zero size is valid for Elasticsearch, and the search must still run so the result set (i.e. aggregations) is available. + */ + public function testGetSliceWithZeroLength(): void + { + $this->searchable->expects($this->once()) + ->method('search') + ->with($this->query, ['from' => 0, 'size' => 0, 'option1' => 'value1', 'option2' => 'value2']) + ->willReturn($this->resultSet); + + $this->assertSame($this->resultSet, $this->adapter->getSlice(0, 0)); + $this->assertSame($this->resultSet, $this->adapter->getResultSet()); + } + /** * Returns the number of results before search, use count() method if resultSet is empty. */ diff --git a/lib/Adapter/Solarium/Tests/SolariumAdapterTest.php b/lib/Adapter/Solarium/Tests/SolariumAdapterTest.php index d1bc61a3..ab58cc0a 100644 --- a/lib/Adapter/Solarium/Tests/SolariumAdapterTest.php +++ b/lib/Adapter/Solarium/Tests/SolariumAdapterTest.php @@ -104,6 +104,36 @@ public function testGetSlice(): void $this->assertSame($result, $adapter->getSlice(1, 200)); } + /** + * A zero row count is valid for Solr, and the query must still run so the result set (i.e. facets) is available. + */ + public function testGetSliceWithZeroLength(): void + { + $query = $this->createQueryMock(); + $query->expects($this->once()) + ->method('setStart') + ->with(0) + ->willReturnSelf(); + + $query->expects($this->once()) + ->method('setRows') + ->with(0) + ->willReturnSelf(); + + $result = $this->createResultMock(); + + $client = $this->createClientMock(); + $client->expects($this->once()) + ->method('select') + ->with($query) + ->willReturn($result); + + $adapter = new SolariumAdapter($client, $query); + + $this->assertSame($result, $adapter->getSlice(0, 0)); + $this->assertSame($result, $adapter->getResultSet()); + } + public function testGetSliceCannotUseACachedResultSet(): void { $query = $this->createQueryStub(); diff --git a/lib/Core/Adapter/ConcatenationAdapter.php b/lib/Core/Adapter/ConcatenationAdapter.php index ebf47beb..cad5b1be 100644 --- a/lib/Core/Adapter/ConcatenationAdapter.php +++ b/lib/Core/Adapter/ConcatenationAdapter.php @@ -106,6 +106,11 @@ public function getSlice(int $offset, int $length): iterable $fetchLength = $adapterNbResults - $fetchOffset; } + // The adapter has nothing to contribute to the requested range (i.e. it has no results) — skip it + if ($fetchLength <= 0) { + continue; + } + // Getting the subslice from the adapter and adding it to the result slice $fetchSlice = $adapter->getSlice($fetchOffset, $fetchLength); diff --git a/lib/Core/Tests/Adapter/ArrayAdapterTest.php b/lib/Core/Tests/Adapter/ArrayAdapterTest.php index a5fd079a..79f934ba 100644 --- a/lib/Core/Tests/Adapter/ArrayAdapterTest.php +++ b/lib/Core/Tests/Adapter/ArrayAdapterTest.php @@ -47,4 +47,10 @@ public function testGetSlice(int $offset, int $length): void { $this->assertSame(\array_slice($this->array, $offset, $length), $this->adapter->getSlice($offset, $length)); } + + public function testGetSliceWithZeroLength(): void + { + $this->assertSame([], $this->adapter->getSlice(0, 0)); + $this->assertSame([], $this->adapter->getSlice(50, 0)); + } } diff --git a/lib/Core/Tests/Adapter/ConcatenationAdapterTest.php b/lib/Core/Tests/Adapter/ConcatenationAdapterTest.php index 56a1c316..459d16c0 100644 --- a/lib/Core/Tests/Adapter/ConcatenationAdapterTest.php +++ b/lib/Core/Tests/Adapter/ConcatenationAdapterTest.php @@ -91,4 +91,34 @@ public function testGetResultsWithTraversableAdapter(): void $this->assertSame([2, 3], $adapter->getSlice(1, 2)); $this->assertSame([4, 5, 6], $adapter->getSlice(3, 3)); } + + public function testGetSliceWithZeroLength(): void + { + $adapter = new ConcatenationAdapter([ + new ArrayAdapter([1, 2, 3]), + new ArrayAdapter([4, 5, 6]), + ]); + + $this->assertSame([], $adapter->getSlice(0, 0)); + $this->assertSame([], $adapter->getSlice(3, 0)); + } + + public function testGetSliceDoesNotRequestAZeroLengthSliceFromAnAdapterWithoutResults(): void + { + $emptyAdapter = new CallbackAdapter( + static fn () => 0, + function (int $offset, int $length): iterable { + $this->fail(\sprintf('The adapter without results should not be sliced, requested offset %d and length %d.', $offset, $length)); + } + ); + + $adapter = new ConcatenationAdapter([ + new ArrayAdapter([1, 2, 3]), + $emptyAdapter, + new ArrayAdapter([4, 5, 6]), + ]); + + $this->assertSame([3, 4, 5], $adapter->getSlice(2, 3)); + $this->assertSame([1, 2, 3, 4, 5, 6], $adapter->getSlice(0, 10)); + } } diff --git a/lib/Core/Tests/Adapter/NullAdapterTest.php b/lib/Core/Tests/Adapter/NullAdapterTest.php index 001e7384..e9ba569c 100644 --- a/lib/Core/Tests/Adapter/NullAdapterTest.php +++ b/lib/Core/Tests/Adapter/NullAdapterTest.php @@ -42,6 +42,14 @@ public function testGetSliceShouldReturnANullArrayWithTheRemainCountWhenLengthIs $this->assertSame($this->createNullArray(3), $adapter->getSlice(30, 10)); } + public function testGetSliceShouldReturnAnEmptyArrayWithZeroLength(): void + { + $adapter = new NullAdapter(10); + + $this->assertSame([], $adapter->getSlice(0, 0)); + $this->assertSame([], $adapter->getSlice(5, 0)); + } + /** * @param int<0, max> $length * diff --git a/lib/Core/Tests/Adapter/TransformingAdapterTest.php b/lib/Core/Tests/Adapter/TransformingAdapterTest.php index 514fcccb..54f9a495 100644 --- a/lib/Core/Tests/Adapter/TransformingAdapterTest.php +++ b/lib/Core/Tests/Adapter/TransformingAdapterTest.php @@ -37,6 +37,11 @@ public function testGetSlice(): void $this->assertSame(['0 => 4', '1 => 5'], [...$this->adapter->getSlice(3, 2)]); } + public function testGetSliceWithZeroLength(): void + { + $this->assertSame([], [...$this->adapter->getSlice(0, 0)]); + } + public function testCreateFromInvokable(): void { $this->adapter = new TransformingAdapter( diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index f445f35b..03fe330d 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -84,12 +84,6 @@ parameters: count: 1 path: lib/Adapter/Solarium/SolariumAdapter.php - - - message: '#^Parameter \#2 \$length of method Pagerfanta\\Adapter\\AdapterInterface\\:\:getSlice\(\) expects int\<0, max\>, int given\.$#' - identifier: argument.type - count: 1 - path: lib/Core/Adapter/ConcatenationAdapter.php - - message: '#^Property Pagerfanta\\Adapter\\ConcatenationAdapter\\:\:\$adaptersNbResultsCache \(list\\>\|null\) does not accept non\-empty\-array\, int\<0, max\>\>\.$#' identifier: assign.propertyType