Fix logic for $limit=0

api-platform · Jan 28, 2025 · abfa2e9 · abfa2e9
1 parent 2da7970
commit abfa2e9
Show file tree

Hide file tree

Showing 2 changed files with 48 additions and 40 deletions.
diff --git a/src/Doctrine/Odm/Extension/PaginationExtension.php b/src/Doctrine/Odm/Extension/PaginationExtension.php
@@ -63,25 +63,25 @@ public function applyToCollection(Builder $aggregationBuilder, string $resourceC
          * @var DocumentRepository
          */
         $repository = $manager->getRepository($resourceClass);
-        $resultsAggregationBuilder = $repository->createAggregationBuilder()->skip($offset);
+
+        $facet = $aggregationBuilder->facet();
+        $addFields = $aggregationBuilder->addFields();
+
+        // Get the results slice, from $offset to $offset + $limit
+        // MongoDB does not support $limit: O, so we return an empty array directly
         if ($limit > 0) {
-            $resultsAggregationBuilder->limit($limit);
+            $facet->field('results')->pipeline($repository->createAggregationBuilder()->skip($offset)->limit($limit));
+        } else {
+            $addFields->field('results')->literal([]);
         }
 
-        $aggregationBuilder
-            ->facet()
-            ->field('results')->pipeline(
-                $resultsAggregationBuilder
-            )
-            ->field('count')->pipeline(
-                $repository->createAggregationBuilder()
-                    ->count('count')
-            )
-            // Store pagination metadata, read by the Paginator
-            // Using __ to avoid field names mapping
-            ->addFields()
-                ->field('__api_first_result__')->literal($offset)
-                ->field('__api_max_results__')->literal($limit);
+        // Count the total number of items
+        $facet->field('count')->pipeline($repository->createAggregationBuilder()->count('count'));
+
+        // Store pagination metadata, read by the Paginator
+        // Using __ to avoid field names mapping
+        $addFields->field('__api_first_result__')->literal($offset);
+        $addFields->field('__api_max_results__')->literal($limit);
     }
 
     /**

diff --git a/src/Doctrine/Odm/Tests/Extension/PaginationExtensionTest.php b/src/Doctrine/Odm/Tests/Extension/PaginationExtensionTest.php
@@ -41,6 +41,7 @@ class PaginationExtensionTest extends TestCase
 {
     use ProphecyTrait;
 
+    /** @var ObjectProphecy<ManagerRegistry> */
     private ObjectProphecy $managerRegistryProphecy;
 
     /**
@@ -424,45 +425,52 @@ public function testGetResultWithExecuteOptions(): void
 
     private function mockAggregationBuilder(int $expectedOffset, int $expectedLimit): ObjectProphecy
     {
-        $skipProphecy = $this->prophesize(Skip::class);
-        if ($expectedLimit > 0) {
-            $skipProphecy->limit($expectedLimit)->shouldBeCalled();
-        }
-
-        $resultsAggregationBuilderProphecy = $this->prophesize(Builder::class);
-        $resultsAggregationBuilderProphecy->skip($expectedOffset)->shouldBeCalled()->willReturn($skipProphecy->reveal());
-
-        $addFieldsProphecy = $this->prophesize(AddFields::class);
-        $addFieldsProphecy->field('__api_first_result__')->shouldBeCalled()->willReturn($addFieldsProphecy->reveal());
-        $addFieldsProphecy->literal($expectedOffset)->shouldBeCalled()->willReturn($addFieldsProphecy->reveal());
-        $addFieldsProphecy->field('__api_max_results__')->shouldBeCalled()->willReturn($addFieldsProphecy->reveal());
-        $addFieldsProphecy->literal($expectedLimit)->shouldBeCalled()->willReturn($addFieldsProphecy->reveal());
-
         $countProphecy = $this->prophesize(Count::class);
-
         $countAggregationBuilderProphecy = $this->prophesize(Builder::class);
         $countAggregationBuilderProphecy->count('count')->shouldBeCalled()->willReturn($countProphecy->reveal());
 
         $repositoryProphecy = $this->prophesize(DocumentRepository::class);
-        $repositoryProphecy->createAggregationBuilder()->shouldBeCalled()->willReturn(
-            $resultsAggregationBuilderProphecy->reveal(),
-            $countAggregationBuilderProphecy->reveal()
-        );
 
         $objectManagerProphecy = $this->prophesize(DocumentManager::class);
         $objectManagerProphecy->getRepository('Foo')->shouldBeCalled()->willReturn($repositoryProphecy->reveal());
 
         $this->managerRegistryProphecy->getManagerForClass('Foo')->shouldBeCalled()->willReturn($objectManagerProphecy->reveal());
 
         $facetProphecy = $this->prophesize(Facet::class);
-        $facetProphecy->pipeline($skipProphecy)->shouldBeCalled()->willReturn($facetProphecy);
-        $facetProphecy->pipeline($countProphecy)->shouldBeCalled()->willReturn($facetProphecy);
-        $facetProphecy->field('count')->shouldBeCalled()->willReturn($facetProphecy);
-        $facetProphecy->field('results')->shouldBeCalled()->willReturn($facetProphecy);
-        $facetProphecy->addFields()->shouldBeCalled()->willReturn($addFieldsProphecy->reveal());
+        $addFieldsProphecy = $this->prophesize(AddFields::class);
+
+        if ($expectedLimit > 0) {
+            $resultsAggregationBuilderProphecy = $this->prophesize(Builder::class);
+            $repositoryProphecy->createAggregationBuilder()->shouldBeCalled()->willReturn(
+                $resultsAggregationBuilderProphecy->reveal(),
+                $countAggregationBuilderProphecy->reveal()
+            );
+
+            $skipProphecy = $this->prophesize(Skip::class);
+            $skipProphecy->limit($expectedLimit)->shouldBeCalled()->willReturn($skipProphecy->reveal());
+            $resultsAggregationBuilderProphecy->skip($expectedOffset)->shouldBeCalled()->willReturn($skipProphecy->reveal());
+            $facetProphecy->field('results')->shouldBeCalled()->willReturn($facetProphecy);
+            $facetProphecy->pipeline($skipProphecy)->shouldBeCalled()->willReturn($facetProphecy->reveal());
+        } else {
+            $repositoryProphecy->createAggregationBuilder()->shouldBeCalled()->willReturn(
+                $countAggregationBuilderProphecy->reveal()
+            );
+
+            $addFieldsProphecy->field('results')->shouldBeCalled()->willReturn($addFieldsProphecy->reveal());
+            $addFieldsProphecy->literal([])->shouldBeCalled()->willReturn($addFieldsProphecy->reveal());
+        }
+
+        $facetProphecy->field('count')->shouldBeCalled()->willReturn($facetProphecy->reveal());
+        $facetProphecy->pipeline($countProphecy)->shouldBeCalled()->willReturn($facetProphecy->reveal());
+
+        $addFieldsProphecy->field('__api_first_result__')->shouldBeCalled()->willReturn($addFieldsProphecy->reveal());
+        $addFieldsProphecy->literal($expectedOffset)->shouldBeCalled()->willReturn($addFieldsProphecy->reveal());
+        $addFieldsProphecy->field('__api_max_results__')->shouldBeCalled()->willReturn($addFieldsProphecy->reveal());
+        $addFieldsProphecy->literal($expectedLimit)->shouldBeCalled()->willReturn($addFieldsProphecy->reveal());
 
         $aggregationBuilderProphecy = $this->prophesize(Builder::class);
         $aggregationBuilderProphecy->facet()->shouldBeCalled()->willReturn($facetProphecy->reveal());
+        $aggregationBuilderProphecy->addFields()->shouldBeCalled()->willReturn($addFieldsProphecy->reveal());
 
         return $aggregationBuilderProphecy;
     }