From 5ae35254da325798d8478364741d1b953c16563d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20TAMARELLE?= Date: Wed, 18 Dec 2024 14:39:32 +0100 Subject: [PATCH 1/4] feat(mongodb): add pagination metadata to the aggregation results --- .../Odm/Extension/PaginationExtension.php | 13 +-- src/Doctrine/Odm/Paginator.php | 84 +++++-------------- .../Extension/PaginationExtensionTest.php | 37 ++++++-- src/Doctrine/Odm/Tests/stubs/AddFields.php | 22 +++++ 4 files changed, 83 insertions(+), 73 deletions(-) create mode 100644 src/Doctrine/Odm/Tests/stubs/AddFields.php diff --git a/src/Doctrine/Odm/Extension/PaginationExtension.php b/src/Doctrine/Odm/Extension/PaginationExtension.php index f1c7892f5ca..664e1dedc9b 100644 --- a/src/Doctrine/Odm/Extension/PaginationExtension.php +++ b/src/Doctrine/Odm/Extension/PaginationExtension.php @@ -66,9 +66,6 @@ public function applyToCollection(Builder $aggregationBuilder, string $resourceC $resultsAggregationBuilder = $repository->createAggregationBuilder()->skip($offset); if ($limit > 0) { $resultsAggregationBuilder->limit($limit); - } else { - // Results have to be 0 but MongoDB does not support a limit equal to 0. - $resultsAggregationBuilder->match()->field(Paginator::LIMIT_ZERO_MARKER_FIELD)->equals(Paginator::LIMIT_ZERO_MARKER); } $aggregationBuilder @@ -79,7 +76,13 @@ public function applyToCollection(Builder $aggregationBuilder, string $resourceC ->field('count')->pipeline( $repository->createAggregationBuilder() ->count('count') - ); + ) + // Store pagination metadata, read by the Paginator + // Using __ to avoid field names mapping + ->addFields() + ->field('__firstResult__')->literal($offset) + ->field('__maxResults__')->literal($limit) + ; } /** @@ -109,7 +112,7 @@ public function getResult(Builder $aggregationBuilder, string $resourceClass, ?O $attribute = $operation?->getExtraProperties()['doctrine_mongodb'] ?? []; $executeOptions = $attribute['execute_options'] ?? []; - return new Paginator($aggregationBuilder->execute($executeOptions), $manager->getUnitOfWork(), $resourceClass, $aggregationBuilder->getPipeline()); + return new Paginator($aggregationBuilder->execute($executeOptions), $manager->getUnitOfWork(), $resourceClass); } private function addCountToContext(Builder $aggregationBuilder, array $context): array diff --git a/src/Doctrine/Odm/Paginator.php b/src/Doctrine/Odm/Paginator.php index b39d6ee1b4f..20a2b88eda2 100644 --- a/src/Doctrine/Odm/Paginator.php +++ b/src/Doctrine/Odm/Paginator.php @@ -13,7 +13,6 @@ namespace ApiPlatform\Doctrine\Odm; -use ApiPlatform\Metadata\Exception\InvalidArgumentException; use ApiPlatform\State\Pagination\HasNextPagePaginatorInterface; use ApiPlatform\State\Pagination\PaginatorInterface; use Doctrine\ODM\MongoDB\Iterator\Iterator; @@ -27,10 +26,7 @@ */ final class Paginator implements \IteratorAggregate, PaginatorInterface, HasNextPagePaginatorInterface { - public const LIMIT_ZERO_MARKER_FIELD = '___'; - public const LIMIT_ZERO_MARKER = 'limit0'; - - private ?\ArrayIterator $iterator = null; + private readonly \ArrayIterator $iterator; private readonly int $firstResult; @@ -38,18 +34,27 @@ final class Paginator implements \IteratorAggregate, PaginatorInterface, HasNext private readonly int $totalItems; - public function __construct(private readonly Iterator $mongoDbOdmIterator, private readonly UnitOfWork $unitOfWork, private readonly string $resourceClass, private readonly array $pipeline) + private readonly int $count; + + public function __construct(Iterator $mongoDbOdmIterator, UnitOfWork $unitOfWork, string $resourceClass) { - $resultsFacetInfo = $this->getFacetInfo('results'); - $this->getFacetInfo('count'); - - /* - * Since the {@see \MongoDB\Driver\Cursor} class does not expose information about - * skip/limit parameters of the query, the values set in the facet stage are used instead. - */ - $this->firstResult = $this->getStageInfo($resultsFacetInfo, '$skip'); - $this->maxResults = $this->hasLimitZeroStage($resultsFacetInfo) ? 0 : $this->getStageInfo($resultsFacetInfo, '$limit'); - $this->totalItems = $mongoDbOdmIterator->toArray()[0]['count'][0]['count'] ?? 0; + $results = $mongoDbOdmIterator->toArray(); + + // The "results" facet contains the returned documents + if ($results[0]['results'] === []) { + $this->count = 0; + $this->iterator = new \ArrayIterator(); + } else { + $this->count = \count($results[0]['results']); + $this->iterator = new \ArrayIterator(array_map( + static fn($result): object => $unitOfWork->getOrCreateDocument($resourceClass, $result), + $results[0]['results'], + )); + } + + $this->totalItems = $results[0]['count'][0]['count'] ?? 0; + $this->firstResult = $results[0]['__firstResult__']; + $this->maxResults = $results[0]['__maxResults__']; } /** @@ -97,7 +102,7 @@ public function getTotalItems(): float */ public function getIterator(): \Traversable { - return $this->iterator ?? $this->iterator = new \ArrayIterator(array_map(fn ($result): object => $this->unitOfWork->getOrCreateDocument($this->resourceClass, $result), $this->mongoDbOdmIterator->toArray()[0]['results'])); + return $this->iterator; } /** @@ -105,7 +110,7 @@ public function getIterator(): \Traversable */ public function count(): int { - return is_countable($this->mongoDbOdmIterator->toArray()[0]['results']) ? \count($this->mongoDbOdmIterator->toArray()[0]['results']) : 0; + return $this->count; } /** @@ -115,47 +120,4 @@ public function hasNextPage(): bool { return $this->getLastPage() > $this->getCurrentPage(); } - - /** - * @throws InvalidArgumentException - */ - private function getFacetInfo(string $field): array - { - foreach ($this->pipeline as $indexStage => $infoStage) { - if (\array_key_exists('$facet', $infoStage)) { - if (!isset($this->pipeline[$indexStage]['$facet'][$field])) { - throw new InvalidArgumentException("\"$field\" facet was not applied to the aggregation pipeline."); - } - - return $this->pipeline[$indexStage]['$facet'][$field]; - } - } - - throw new InvalidArgumentException('$facet stage was not applied to the aggregation pipeline.'); - } - - /** - * @throws InvalidArgumentException - */ - private function getStageInfo(array $resultsFacetInfo, string $stage): int - { - foreach ($resultsFacetInfo as $resultFacetInfo) { - if (isset($resultFacetInfo[$stage])) { - return $resultFacetInfo[$stage]; - } - } - - throw new InvalidArgumentException("$stage stage was not applied to the facet stage of the aggregation pipeline."); - } - - private function hasLimitZeroStage(array $resultsFacetInfo): bool - { - foreach ($resultsFacetInfo as $resultFacetInfo) { - if (self::LIMIT_ZERO_MARKER === ($resultFacetInfo['$match'][self::LIMIT_ZERO_MARKER_FIELD] ?? null)) { - return true; - } - } - - return false; - } } diff --git a/src/Doctrine/Odm/Tests/Extension/PaginationExtensionTest.php b/src/Doctrine/Odm/Tests/Extension/PaginationExtensionTest.php index 7e28cfde308..cc1e88350ac 100644 --- a/src/Doctrine/Odm/Tests/Extension/PaginationExtensionTest.php +++ b/src/Doctrine/Odm/Tests/Extension/PaginationExtensionTest.php @@ -14,7 +14,6 @@ namespace ApiPlatform\Doctrine\Odm\Tests\Extension; use ApiPlatform\Doctrine\Odm\Extension\PaginationExtension; -use ApiPlatform\Doctrine\Odm\Paginator; use ApiPlatform\Doctrine\Odm\Tests\DoctrineMongoDbOdmSetup; use ApiPlatform\Doctrine\Odm\Tests\Fixtures\Document\Dummy; use ApiPlatform\Metadata\Exception\InvalidArgumentException; @@ -23,9 +22,9 @@ use ApiPlatform\State\Pagination\PaginatorInterface; use ApiPlatform\State\Pagination\PartialPaginatorInterface; use Doctrine\ODM\MongoDB\Aggregation\Builder; +use Doctrine\ODM\MongoDB\Aggregation\Stage\AddFields; use Doctrine\ODM\MongoDB\Aggregation\Stage\Count; use Doctrine\ODM\MongoDB\Aggregation\Stage\Facet; -use Doctrine\ODM\MongoDB\Aggregation\Stage\MatchStage as AggregationMatch; use Doctrine\ODM\MongoDB\Aggregation\Stage\Skip; use Doctrine\ODM\MongoDB\DocumentManager; use Doctrine\ODM\MongoDB\Iterator\Iterator; @@ -49,6 +48,9 @@ class PaginationExtensionTest extends TestCase */ protected function setUp(): void { + // This class is final, but we need to mock it + require_once __DIR__.'/../stubs/AddFields.php'; + $this->managerRegistryProphecy = $this->prophesize(ManagerRegistry::class); } @@ -322,11 +324,14 @@ public function testGetResult(): void $iteratorProphecy = $this->prophesize(Iterator::class); $iteratorProphecy->toArray()->willReturn([ [ + 'results' => [], 'count' => [ [ 'count' => 9, ], ], + '__firstResult__' => 3, + '__maxResults__' => 6, ], ]); @@ -344,6 +349,12 @@ public function testGetResult(): void ], ], ], + [ + '$addFields' => [ + '__firstResult__' => ['$literal' => 3], + '__maxResults__' => ['$literal' => 6], + ], + ], ]); $paginationExtension = new PaginationExtension( @@ -370,11 +381,14 @@ public function testGetResultWithExecuteOptions(): void $iteratorProphecy = $this->prophesize(Iterator::class); $iteratorProphecy->toArray()->willReturn([ [ + 'results' => [], 'count' => [ [ 'count' => 9, ], ], + '__firstResult__' => 3, + '__maxResults__' => 6, ], ]); @@ -392,6 +406,13 @@ public function testGetResultWithExecuteOptions(): void ], ], ], + [ + + '$addFields' => [ + '__firstResult__' => ['$literal' => 3], + '__maxResults__' => ['$literal' => 6], + ], + ], ]); $paginationExtension = new PaginationExtension( @@ -410,16 +431,17 @@ private function mockAggregationBuilder(int $expectedOffset, int $expectedLimit) $skipProphecy = $this->prophesize(Skip::class); if ($expectedLimit > 0) { $skipProphecy->limit($expectedLimit)->shouldBeCalled(); - } else { - $matchProphecy = $this->prophesize(AggregationMatch::class); - $matchProphecy->field(Paginator::LIMIT_ZERO_MARKER_FIELD)->shouldBeCalled()->willReturn($matchProphecy->reveal()); - $matchProphecy->equals(Paginator::LIMIT_ZERO_MARKER)->shouldBeCalled()->willReturn($matchProphecy->reveal()); - $skipProphecy->match()->shouldBeCalled()->willReturn($matchProphecy->reveal()); } $resultsAggregationBuilderProphecy = $this->prophesize(Builder::class); $resultsAggregationBuilderProphecy->skip($expectedOffset)->shouldBeCalled()->willReturn($skipProphecy->reveal()); + $addFieldsProphecy = $this->prophesize(AddFields::class); + $addFieldsProphecy->field('__firstResult__')->shouldBeCalled()->willReturn($addFieldsProphecy->reveal()); + $addFieldsProphecy->literal($expectedOffset)->shouldBeCalled()->willReturn($addFieldsProphecy->reveal()); + $addFieldsProphecy->field('__maxResults__')->shouldBeCalled()->willReturn($addFieldsProphecy->reveal()); + $addFieldsProphecy->literal($expectedLimit)->shouldBeCalled()->willReturn($addFieldsProphecy->reveal()); + $countProphecy = $this->prophesize(Count::class); $countAggregationBuilderProphecy = $this->prophesize(Builder::class); @@ -441,6 +463,7 @@ private function mockAggregationBuilder(int $expectedOffset, int $expectedLimit) $facetProphecy->pipeline($countProphecy)->shouldBeCalled()->willReturn($facetProphecy); $facetProphecy->field('count')->shouldBeCalled()->willReturn($facetProphecy); $facetProphecy->field('results')->shouldBeCalled()->willReturn($facetProphecy); + $facetProphecy->addFields()->shouldBeCalled()->willReturn($addFieldsProphecy->reveal()); $aggregationBuilderProphecy = $this->prophesize(Builder::class); $aggregationBuilderProphecy->facet()->shouldBeCalled()->willReturn($facetProphecy->reveal()); diff --git a/src/Doctrine/Odm/Tests/stubs/AddFields.php b/src/Doctrine/Odm/Tests/stubs/AddFields.php new file mode 100644 index 00000000000..bbee6d045c7 --- /dev/null +++ b/src/Doctrine/Odm/Tests/stubs/AddFields.php @@ -0,0 +1,22 @@ +} +*/ +class AddFields extends Operator +{ + /** @return AddFieldsStageExpression */ + public function getExpression(): array + { + return ['$addFields' => $this->expr->getExpression()]; + } +} From e13199184e9fac0f5e5cecc8e639ad5693e38666 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Fri, 17 Jan 2025 11:00:11 +0100 Subject: [PATCH 2/4] Fix PaginatorTest, CS and rename metadata fields for more unicity --- .../Odm/Extension/PaginationExtension.php | 5 +- src/Doctrine/Odm/Paginator.php | 24 ++- .../Extension/PaginationExtensionTest.php | 21 ++- src/Doctrine/Odm/Tests/PaginatorTest.php | 142 +++++------------- src/Doctrine/Odm/Tests/stubs/AddFields.php | 2 +- 5 files changed, 63 insertions(+), 131 deletions(-) diff --git a/src/Doctrine/Odm/Extension/PaginationExtension.php b/src/Doctrine/Odm/Extension/PaginationExtension.php index 664e1dedc9b..419e74aa7ec 100644 --- a/src/Doctrine/Odm/Extension/PaginationExtension.php +++ b/src/Doctrine/Odm/Extension/PaginationExtension.php @@ -80,9 +80,8 @@ public function applyToCollection(Builder $aggregationBuilder, string $resourceC // Store pagination metadata, read by the Paginator // Using __ to avoid field names mapping ->addFields() - ->field('__firstResult__')->literal($offset) - ->field('__maxResults__')->literal($limit) - ; + ->field('__api_first_result__')->literal($offset) + ->field('__api_max_results__')->literal($limit); } /** diff --git a/src/Doctrine/Odm/Paginator.php b/src/Doctrine/Odm/Paginator.php index 20a2b88eda2..30fafe39667 100644 --- a/src/Doctrine/Odm/Paginator.php +++ b/src/Doctrine/Odm/Paginator.php @@ -13,6 +13,7 @@ namespace ApiPlatform\Doctrine\Odm; +use ApiPlatform\Metadata\Exception\RuntimeException; use ApiPlatform\State\Pagination\HasNextPagePaginatorInterface; use ApiPlatform\State\Pagination\PaginatorInterface; use Doctrine\ODM\MongoDB\Iterator\Iterator; @@ -38,23 +39,30 @@ final class Paginator implements \IteratorAggregate, PaginatorInterface, HasNext public function __construct(Iterator $mongoDbOdmIterator, UnitOfWork $unitOfWork, string $resourceClass) { - $results = $mongoDbOdmIterator->toArray(); + $result = $mongoDbOdmIterator->toArray()[0]; + + if (array_diff_key(['results' => 1, 'count' => 1, '__api_first_result__' => 1, '__api_max_results__' => 1], $result)) { + throw new RuntimeException('The result of the query must contain only "__api_first_result__", "__api_max_results__", "results" and "count" fields.'); + } + + // The "count" facet contains the total number of documents, + // it is not set when the query does not return any document + $this->totalItems = $result['count'][0]['count'] ?? 0; // The "results" facet contains the returned documents - if ($results[0]['results'] === []) { + if ([] === $result['results']) { $this->count = 0; $this->iterator = new \ArrayIterator(); } else { - $this->count = \count($results[0]['results']); + $this->count = \count($result['results']); $this->iterator = new \ArrayIterator(array_map( - static fn($result): object => $unitOfWork->getOrCreateDocument($resourceClass, $result), - $results[0]['results'], + static fn ($result): object => $unitOfWork->getOrCreateDocument($resourceClass, $result), + $result['results'], )); } - $this->totalItems = $results[0]['count'][0]['count'] ?? 0; - $this->firstResult = $results[0]['__firstResult__']; - $this->maxResults = $results[0]['__maxResults__']; + $this->firstResult = $result['__api_first_result__']; + $this->maxResults = $result['__api_max_results__']; } /** diff --git a/src/Doctrine/Odm/Tests/Extension/PaginationExtensionTest.php b/src/Doctrine/Odm/Tests/Extension/PaginationExtensionTest.php index cc1e88350ac..8971e64df2b 100644 --- a/src/Doctrine/Odm/Tests/Extension/PaginationExtensionTest.php +++ b/src/Doctrine/Odm/Tests/Extension/PaginationExtensionTest.php @@ -330,8 +330,8 @@ public function testGetResult(): void 'count' => 9, ], ], - '__firstResult__' => 3, - '__maxResults__' => 6, + '__api_first_result__' => 3, + '__api_max_results__' => 6, ], ]); @@ -351,8 +351,8 @@ public function testGetResult(): void ], [ '$addFields' => [ - '__firstResult__' => ['$literal' => 3], - '__maxResults__' => ['$literal' => 6], + '__api_first_result__' => ['$literal' => 3], + '__api_max_results__' => ['$literal' => 6], ], ], ]); @@ -387,8 +387,8 @@ public function testGetResultWithExecuteOptions(): void 'count' => 9, ], ], - '__firstResult__' => 3, - '__maxResults__' => 6, + '__api_first_result__' => 3, + '__api_max_results__' => 6, ], ]); @@ -407,10 +407,9 @@ public function testGetResultWithExecuteOptions(): void ], ], [ - '$addFields' => [ - '__firstResult__' => ['$literal' => 3], - '__maxResults__' => ['$literal' => 6], + '__api_first_result__' => ['$literal' => 3], + '__api_max_results__' => ['$literal' => 6], ], ], ]); @@ -437,9 +436,9 @@ private function mockAggregationBuilder(int $expectedOffset, int $expectedLimit) $resultsAggregationBuilderProphecy->skip($expectedOffset)->shouldBeCalled()->willReturn($skipProphecy->reveal()); $addFieldsProphecy = $this->prophesize(AddFields::class); - $addFieldsProphecy->field('__firstResult__')->shouldBeCalled()->willReturn($addFieldsProphecy->reveal()); + $addFieldsProphecy->field('__api_first_result__')->shouldBeCalled()->willReturn($addFieldsProphecy->reveal()); $addFieldsProphecy->literal($expectedOffset)->shouldBeCalled()->willReturn($addFieldsProphecy->reveal()); - $addFieldsProphecy->field('__maxResults__')->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); diff --git a/src/Doctrine/Odm/Tests/PaginatorTest.php b/src/Doctrine/Odm/Tests/PaginatorTest.php index d6540a7f46a..bab185bc5c8 100644 --- a/src/Doctrine/Odm/Tests/PaginatorTest.php +++ b/src/Doctrine/Odm/Tests/PaginatorTest.php @@ -16,8 +16,10 @@ use ApiPlatform\Doctrine\Odm\Paginator; use ApiPlatform\Doctrine\Odm\Tests\Fixtures\Document\Dummy; use ApiPlatform\Metadata\Exception\InvalidArgumentException; +use ApiPlatform\Metadata\Exception\RuntimeException; use Doctrine\ODM\MongoDB\DocumentManager; use Doctrine\ODM\MongoDB\Iterator\Iterator; +use PHPUnit\Framework\Attributes\TestWith; use PHPUnit\Framework\TestCase; use Prophecy\PhpUnit\ProphecyTrait; @@ -36,62 +38,24 @@ public function testInitialize(int $firstResult, int $maxResults, int $totalItem $this->assertSame($hasNextPage, $paginator->hasNextPage()); } - public function testInitializeWithFacetStageNotApplied(): void - { - $this->expectException(InvalidArgumentException::class); - $this->expectExceptionMessage('$facet stage was not applied to the aggregation pipeline.'); - - $this->getPaginatorWithMissingStage(); - } - - public function testInitializeWithResultsFacetNotApplied(): void - { - $this->expectException(InvalidArgumentException::class); - $this->expectExceptionMessage('"results" facet was not applied to the aggregation pipeline.'); - - $this->getPaginatorWithMissingStage(true); - } - - public function testInitializeWithCountFacetNotApplied(): void - { - $this->expectException(InvalidArgumentException::class); - $this->expectExceptionMessage('"count" facet was not applied to the aggregation pipeline.'); - - $this->getPaginatorWithMissingStage(true, true); - } - - public function testInitializeWithSkipStageNotApplied(): void - { - $this->expectException(InvalidArgumentException::class); - $this->expectExceptionMessage('$skip stage was not applied to the facet stage of the aggregation pipeline.'); - - $this->getPaginatorWithMissingStage(true, true, true); - } - - public function testInitializeWithLimitStageNotApplied(): void - { - $this->expectException(InvalidArgumentException::class); - $this->expectExceptionMessage('$limit stage was not applied to the facet stage of the aggregation pipeline.'); - - $this->getPaginatorWithMissingStage(true, true, true, true); - } - - public function testInitializeWithLimitZeroStageApplied(): void + public function testInitializeWithNoCount(): void { - $paginator = $this->getPaginator(0, 5, 0, true); + $paginator = $this->getPaginatorWithNoCount(); $this->assertSame(1., $paginator->getCurrentPage()); $this->assertSame(1., $paginator->getLastPage()); - $this->assertSame(0., $paginator->getItemsPerPage()); + $this->assertSame(15., $paginator->getItemsPerPage()); } - public function testInitializeWithNoCount(): void + #[TestWith(['__api_first_result__'])] + #[TestWith(['__api_max_results__'])] + #[TestWith(['results'])] + #[TestWith(['count'])] + public function testInitializeWithMissingResultField(string $missingField): void { - $paginator = $this->getPaginatorWithNoCount(); + $this->expectException(RuntimeException::class); - $this->assertSame(1., $paginator->getCurrentPage()); - $this->assertSame(1., $paginator->getLastPage()); - $this->assertSame(15., $paginator->getItemsPerPage()); + $this->getPaginatorMissingResultField($missingField); } public function testGetIterator(): void @@ -101,30 +65,15 @@ public function testGetIterator(): void $this->assertSame($paginator->getIterator(), $paginator->getIterator(), 'Iterator should be cached'); } - private function getPaginator(int $firstResult = 1, int $maxResults = 15, int $totalItems = 42, bool $limitZero = false): Paginator + private function getPaginator(int $firstResult = 1, int $maxResults = 15, int $totalItems = 42): Paginator { $iterator = $this->prophesize(Iterator::class); - $pipeline = [ - [ - '$facet' => [ - 'results' => [ - ['$skip' => $firstResult], - $limitZero ? ['$match' => [Paginator::LIMIT_ZERO_MARKER_FIELD => Paginator::LIMIT_ZERO_MARKER]] : ['$limit' => $maxResults], - ], - 'count' => [ - ['$count' => 'count'], - ], - ], - ], - ]; $iterator->toArray()->willReturn([ [ - 'count' => [ - [ - 'count' => $totalItems, - ], - ], + 'count' => [['count' => $totalItems]], 'results' => [], + '__api_first_result__' => $firstResult, + '__api_max_results__' => $maxResults, ], ]); @@ -132,68 +81,45 @@ private function getPaginator(int $firstResult = 1, int $maxResults = 15, int $t $config = DoctrineMongoDbOdmSetup::createAttributeMetadataConfiguration([$fixturesPath], true); $documentManager = DocumentManager::create(null, $config); - return new Paginator($iterator->reveal(), $documentManager->getUnitOfWork(), Dummy::class, $pipeline); + return new Paginator($iterator->reveal(), $documentManager->getUnitOfWork(), Dummy::class); } - private function getPaginatorWithMissingStage(bool $facet = false, bool $results = false, bool $count = false, bool $maxResults = false): Paginator + private function getPaginatorWithNoCount(): Paginator { - $pipeline = []; - - if ($facet) { - $pipeline[] = [ - '$facet' => [], - ]; - } - - if ($results) { - $pipeline[0]['$facet']['results'] = []; - } - - if ($count) { - $pipeline[0]['$facet']['count'] = []; - } - - if ($maxResults) { - $pipeline[0]['$facet']['results'][] = ['$skip' => 42]; - } - $iterator = $this->prophesize(Iterator::class); + $iterator->toArray()->willReturn([ + [ + 'count' => [], + 'results' => [], + '__api_first_result__' => 1, + '__api_max_results__' => 15, + ], + ]); $fixturesPath = \dirname((string) (new \ReflectionClass(Dummy::class))->getFileName()); $config = DoctrineMongoDbOdmSetup::createAttributeMetadataConfiguration([$fixturesPath], true); $documentManager = DocumentManager::create(null, $config); - return new Paginator($iterator->reveal(), $documentManager->getUnitOfWork(), Dummy::class, $pipeline); + return new Paginator($iterator->reveal(), $documentManager->getUnitOfWork(), Dummy::class); } - private function getPaginatorWithNoCount($firstResult = 1, $maxResults = 15): Paginator + private function getPaginatorMissingResultField(string $missing): Paginator { $iterator = $this->prophesize(Iterator::class); - $pipeline = [ - [ - '$facet' => [ - 'results' => [ - ['$skip' => $firstResult], - ['$limit' => $maxResults], - ], - 'count' => [ - ['$count' => 'count'], - ], - ], - ], - ]; $iterator->toArray()->willReturn([ - [ - 'count' => [], + array_diff_key([ + 'count' => [['count' => 42]], 'results' => [], - ], + '__api_first_result__' => 1, + '__api_max_results__' => 15, + ], [$missing => 1]), ]); $fixturesPath = \dirname((string) (new \ReflectionClass(Dummy::class))->getFileName()); $config = DoctrineMongoDbOdmSetup::createAttributeMetadataConfiguration([$fixturesPath], true); $documentManager = DocumentManager::create(null, $config); - return new Paginator($iterator->reveal(), $documentManager->getUnitOfWork(), Dummy::class, $pipeline); + return new Paginator($iterator->reveal(), $documentManager->getUnitOfWork(), Dummy::class); } public static function initializeProvider(): array diff --git a/src/Doctrine/Odm/Tests/stubs/AddFields.php b/src/Doctrine/Odm/Tests/stubs/AddFields.php index bbee6d045c7..84813bcaad4 100644 --- a/src/Doctrine/Odm/Tests/stubs/AddFields.php +++ b/src/Doctrine/Odm/Tests/stubs/AddFields.php @@ -11,7 +11,7 @@ * * @psalm-import-type OperatorExpression from Expr * @psalm-type AddFieldsStageExpression = array{'$addFields': array} -*/ + */ class AddFields extends Operator { /** @return AddFieldsStageExpression */ From 2da79700dd99f6c3d3711d24409c4327a610ea68 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Fri, 17 Jan 2025 16:18:03 +0100 Subject: [PATCH 3/4] Remove AddFields stubs --- composer.json | 2 +- .../Extension/PaginationExtensionTest.php | 3 --- src/Doctrine/Odm/Tests/PaginatorTest.php | 1 - src/Doctrine/Odm/Tests/stubs/AddFields.php | 22 ------------------- 4 files changed, 1 insertion(+), 27 deletions(-) delete mode 100644 src/Doctrine/Odm/Tests/stubs/AddFields.php diff --git a/composer.json b/composer.json index 290828ac782..a1f2a03f540 100644 --- a/composer.json +++ b/composer.json @@ -124,7 +124,7 @@ "doctrine/common": "^3.2.2", "doctrine/dbal": "^4.0", "doctrine/doctrine-bundle": "^2.11", - "doctrine/mongodb-odm": "^2.6", + "doctrine/mongodb-odm": "^2.9.2", "doctrine/mongodb-odm-bundle": "^4.0 || ^5.0", "doctrine/orm": "^2.17 || ^3.0", "elasticsearch/elasticsearch": "^8.4", diff --git a/src/Doctrine/Odm/Tests/Extension/PaginationExtensionTest.php b/src/Doctrine/Odm/Tests/Extension/PaginationExtensionTest.php index 8971e64df2b..0d5323e6a4d 100644 --- a/src/Doctrine/Odm/Tests/Extension/PaginationExtensionTest.php +++ b/src/Doctrine/Odm/Tests/Extension/PaginationExtensionTest.php @@ -48,9 +48,6 @@ class PaginationExtensionTest extends TestCase */ protected function setUp(): void { - // This class is final, but we need to mock it - require_once __DIR__.'/../stubs/AddFields.php'; - $this->managerRegistryProphecy = $this->prophesize(ManagerRegistry::class); } diff --git a/src/Doctrine/Odm/Tests/PaginatorTest.php b/src/Doctrine/Odm/Tests/PaginatorTest.php index bab185bc5c8..e8746ea7e8a 100644 --- a/src/Doctrine/Odm/Tests/PaginatorTest.php +++ b/src/Doctrine/Odm/Tests/PaginatorTest.php @@ -15,7 +15,6 @@ use ApiPlatform\Doctrine\Odm\Paginator; use ApiPlatform\Doctrine\Odm\Tests\Fixtures\Document\Dummy; -use ApiPlatform\Metadata\Exception\InvalidArgumentException; use ApiPlatform\Metadata\Exception\RuntimeException; use Doctrine\ODM\MongoDB\DocumentManager; use Doctrine\ODM\MongoDB\Iterator\Iterator; diff --git a/src/Doctrine/Odm/Tests/stubs/AddFields.php b/src/Doctrine/Odm/Tests/stubs/AddFields.php deleted file mode 100644 index 84813bcaad4..00000000000 --- a/src/Doctrine/Odm/Tests/stubs/AddFields.php +++ /dev/null @@ -1,22 +0,0 @@ -} - */ -class AddFields extends Operator -{ - /** @return AddFieldsStageExpression */ - public function getExpression(): array - { - return ['$addFields' => $this->expr->getExpression()]; - } -} From b2c22120d843362a4b9ded0bd47c306d88a64c30 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Tue, 28 Jan 2025 21:19:11 +0100 Subject: [PATCH 4/4] Fix logic for $limit=0 --- .../Odm/Extension/PaginationExtension.php | 32 +++++------ .../Extension/PaginationExtensionTest.php | 56 +++++++++++-------- 2 files changed, 48 insertions(+), 40 deletions(-) diff --git a/src/Doctrine/Odm/Extension/PaginationExtension.php b/src/Doctrine/Odm/Extension/PaginationExtension.php index 419e74aa7ec..2cf644e5ac2 100644 --- 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 index 0d5323e6a4d..5205c8e83d4 100644 --- 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 */ private ObjectProphecy $managerRegistryProphecy; /** @@ -424,30 +425,11 @@ 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()); @@ -455,14 +437,40 @@ private function mockAggregationBuilder(int $expectedOffset, int $expectedLimit) $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; }