Skip to content

Commit 64115e1

Browse files
authored
fix(odm): partial pagination limit the documents entering $facet (api-platform#7822)
1 parent 31289b8 commit 64115e1

2 files changed

Lines changed: 48 additions & 8 deletions

File tree

src/Doctrine/Odm/Extension/PaginationExtension.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,11 @@ public function applyToCollection(Builder $aggregationBuilder, string $resourceC
6767
*/
6868
$repository = $manager->getRepository($resourceClass);
6969

70+
// Limit the documents entering $facet to avoid buffering the whole result set
71+
if (!$doesCount && $limit > 0) {
72+
$aggregationBuilder->limit($offset + $limit);
73+
}
74+
7075
$facet = $aggregationBuilder->facet();
7176
$addFields = $aggregationBuilder->addFields();
7277

src/Doctrine/Odm/Tests/Extension/PaginationExtensionTest.php

Lines changed: 43 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,24 @@ public function testApplyToCollectionWithMaximumItemsPerPage(): void
259259
$extension->applyToCollection($aggregationBuilderProphecy->reveal(), 'Foo', (new GetCollection())->withPaginationEnabled(true)->withPaginationClientEnabled(true)->withPaginationMaximumItemsPerPage(80), $context);
260260
}
261261

262+
public function testApplyToCollectionWithPartialPagination(): void
263+
{
264+
$pagination = new Pagination([
265+
'partial' => true,
266+
'page_parameter_name' => '_page',
267+
]);
268+
269+
$aggregationBuilderProphecy = $this->mockAggregationBuilder(20, 20, true);
270+
271+
$context = ['filters' => ['_page' => 2, 'itemsPerPage' => 20]];
272+
273+
$extension = new PaginationExtension(
274+
$this->managerRegistryProphecy->reveal(),
275+
$pagination
276+
);
277+
$extension->applyToCollection($aggregationBuilderProphecy->reveal(), 'Foo', (new GetCollection())->withPaginationEnabled(true)->withPaginationPartial(true)->withPaginationClientEnabled(true)->withPaginationItemsPerPage(20), $context);
278+
}
279+
262280
public function testSupportsResult(): void
263281
{
264282
$pagination = new Pagination();
@@ -430,11 +448,14 @@ public function testGetResultWithExecuteOptions(): void
430448
$this->assertInstanceOf(PaginatorInterface::class, $result);
431449
}
432450

433-
private function mockAggregationBuilder(int $expectedOffset, int $expectedLimit): ObjectProphecy
451+
private function mockAggregationBuilder(int $expectedOffset, int $expectedLimit, bool $partial = false): ObjectProphecy
434452
{
435453
$countProphecy = $this->prophesize(Count::class);
436454
$countAggregationBuilderProphecy = $this->prophesize(Builder::class);
437-
$countAggregationBuilderProphecy->count('count')->shouldBeCalled()->willReturn($countProphecy->reveal());
455+
456+
if (!$partial) {
457+
$countAggregationBuilderProphecy->count('count')->shouldBeCalled()->willReturn($countProphecy->reveal());
458+
}
438459

439460
$repositoryProphecy = $this->prophesize(DocumentRepository::class);
440461

@@ -448,10 +469,17 @@ private function mockAggregationBuilder(int $expectedOffset, int $expectedLimit)
448469

449470
if ($expectedLimit > 0) {
450471
$resultsAggregationBuilderProphecy = $this->prophesize(Builder::class);
451-
$repositoryProphecy->createAggregationBuilder()->shouldBeCalled()->willReturn(
452-
$resultsAggregationBuilderProphecy->reveal(),
453-
$countAggregationBuilderProphecy->reveal()
454-
);
472+
473+
if ($partial) {
474+
$repositoryProphecy->createAggregationBuilder()->shouldBeCalled()->willReturn(
475+
$resultsAggregationBuilderProphecy->reveal()
476+
);
477+
} else {
478+
$repositoryProphecy->createAggregationBuilder()->shouldBeCalled()->willReturn(
479+
$resultsAggregationBuilderProphecy->reveal(),
480+
$countAggregationBuilderProphecy->reveal()
481+
);
482+
}
455483

456484
$skipProphecy = $this->prophesize(Skip::class);
457485
$skipProphecy->limit($expectedLimit)->shouldBeCalled()->willReturn($skipProphecy->reveal());
@@ -467,15 +495,22 @@ private function mockAggregationBuilder(int $expectedOffset, int $expectedLimit)
467495
$addFieldsProphecy->literal([])->shouldBeCalled()->willReturn($addFieldsProphecy->reveal());
468496
}
469497

470-
$facetProphecy->field('count')->shouldBeCalled()->willReturn($facetProphecy->reveal());
471-
$facetProphecy->pipeline($countProphecy)->shouldBeCalled()->willReturn($facetProphecy->reveal());
498+
if (!$partial) {
499+
$facetProphecy->field('count')->shouldBeCalled()->willReturn($facetProphecy->reveal());
500+
$facetProphecy->pipeline($countProphecy)->shouldBeCalled()->willReturn($facetProphecy->reveal());
501+
}
472502

473503
$addFieldsProphecy->field('__api_first_result__')->shouldBeCalled()->willReturn($addFieldsProphecy->reveal());
474504
$addFieldsProphecy->literal($expectedOffset)->shouldBeCalled()->willReturn($addFieldsProphecy->reveal());
475505
$addFieldsProphecy->field('__api_max_results__')->shouldBeCalled()->willReturn($addFieldsProphecy->reveal());
476506
$addFieldsProphecy->literal($expectedLimit)->shouldBeCalled()->willReturn($addFieldsProphecy->reveal());
477507

478508
$aggregationBuilderProphecy = $this->prophesize(Builder::class);
509+
510+
if ($partial && $expectedLimit > 0) {
511+
$aggregationBuilderProphecy->limit($expectedOffset + $expectedLimit)->shouldBeCalled();
512+
}
513+
479514
$aggregationBuilderProphecy->facet()->shouldBeCalled()->willReturn($facetProphecy->reveal());
480515
$aggregationBuilderProphecy->addFields()->shouldBeCalled()->willReturn($addFieldsProphecy->reveal());
481516

0 commit comments

Comments
 (0)