From 105bea2fae848df64f7049f167f80bb7a41a2a70 Mon Sep 17 00:00:00 2001 From: meyerbaptiste Date: Tue, 2 Feb 2021 17:37:51 +0100 Subject: [PATCH 1/2] Revert "fix: only display hydra:next when the item total is strictly greater than the number of items per page (#3967)" This reverts commit 9cecfab38e7e47b7894be61463c7b0b373635663. --- src/Hydra/Serializer/PartialCollectionViewNormalizer.php | 2 +- tests/Hydra/Serializer/PartialCollectionViewNormalizerTest.php | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Hydra/Serializer/PartialCollectionViewNormalizer.php b/src/Hydra/Serializer/PartialCollectionViewNormalizer.php index 3e0bae6eb08..fcde0825c7d 100644 --- a/src/Hydra/Serializer/PartialCollectionViewNormalizer.php +++ b/src/Hydra/Serializer/PartialCollectionViewNormalizer.php @@ -113,7 +113,7 @@ public function normalize($object, $format = null, array $context = []) $data['hydra:view']['hydra:previous'] = IriHelper::createIri($parsed['parts'], $parsed['parameters'], $this->pageParameterName, $currentPage - 1.); } - if (null !== $lastPage && $currentPage < $lastPage || null === $lastPage && $pageTotalItems > $itemsPerPage) { + if (null !== $lastPage && $currentPage < $lastPage || null === $lastPage && $pageTotalItems >= $itemsPerPage) { $data['hydra:view']['hydra:next'] = IriHelper::createIri($parsed['parts'], $parsed['parameters'], $this->pageParameterName, $currentPage + 1.); } } diff --git a/tests/Hydra/Serializer/PartialCollectionViewNormalizerTest.php b/tests/Hydra/Serializer/PartialCollectionViewNormalizerTest.php index 94d9cea5852..5531da57351 100644 --- a/tests/Hydra/Serializer/PartialCollectionViewNormalizerTest.php +++ b/tests/Hydra/Serializer/PartialCollectionViewNormalizerTest.php @@ -79,6 +79,7 @@ public function testNormalizePartialPaginator() '@id' => '/?_page=3', '@type' => 'hydra:PartialCollectionView', 'hydra:previous' => '/?_page=2', + 'hydra:next' => '/?_page=4', ], ], $this->normalizePaginator(true) From 725e66a8b8ae4cefeee4f49bfb461b12d42a1372 Mon Sep 17 00:00:00 2001 From: meyerbaptiste Date: Tue, 2 Feb 2021 17:39:25 +0100 Subject: [PATCH 2/2] Fix partial pagination which no longer returns the "hydra:next" property --- features/hydra/collection.feature | 28 ++++--- .../PartialCollectionViewNormalizer.php | 74 +++++++++++-------- .../PartialCollectionViewNormalizerTest.php | 49 +++++++++++- 3 files changed, 106 insertions(+), 45 deletions(-) diff --git a/features/hydra/collection.feature b/features/hydra/collection.feature index ee92c4002e7..aebca7165d7 100644 --- a/features/hydra/collection.feature +++ b/features/hydra/collection.feature @@ -203,9 +203,13 @@ Feature: Collections support "hydra:next": {"pattern": "^/dummies\\?partial=1&page=8$"}, "hydra:previous": {"pattern": "^/dummies\\?partial=1&page=6$"} }, - "additionalProperties": false + "required": ["@id", "@type", "hydra:next", "hydra:previous"], + "additionalProperties": false, + "maxProperties": 4 } - } + }, + "required": ["@context", "@id", "@type", "hydra:member", "hydra:view", "hydra:search"], + "maxProperties": 6 } """ @@ -275,16 +279,16 @@ Feature: Collections support And the response should be in JSON And the header "Content-Type" should be equal to "application/ld+json; charset=utf-8" And the JSON should be valid according to this schema: - """ - { - "@id":"/dummies?page=3", - "@type":"hydra:PartialCollectionView", - "hydra:first":"/dummies?page=1", - "hydra:last":"/dummies?page=10", - "hydra:previous":"/dummies?page=2", - "hydra:next":"/dummies?page=4" - } - """ + """ + { + "@id":"/dummies?page=3", + "@type":"hydra:PartialCollectionView", + "hydra:first":"/dummies?page=1", + "hydra:last":"/dummies?page=10", + "hydra:previous":"/dummies?page=2", + "hydra:next":"/dummies?page=4" + } + """ Scenario: Filter with exact match When I send a "GET" request to "/dummies?id=8" Then the response status code should be 200 diff --git a/src/Hydra/Serializer/PartialCollectionViewNormalizer.php b/src/Hydra/Serializer/PartialCollectionViewNormalizer.php index fcde0825c7d..f6a18586b22 100644 --- a/src/Hydra/Serializer/PartialCollectionViewNormalizer.php +++ b/src/Hydra/Serializer/PartialCollectionViewNormalizer.php @@ -62,7 +62,7 @@ public function normalize($object, $format = null, array $context = []) } $currentPage = $lastPage = $itemsPerPage = $pageTotalItems = null; - if ($paginated = $object instanceof PartialPaginatorInterface) { + if ($paginated = ($object instanceof PartialPaginatorInterface)) { if ($object instanceof PaginatorInterface) { $paginated = 1. !== $lastPage = $object->getLastPage(); } else { @@ -81,41 +81,20 @@ public function normalize($object, $format = null, array $context = []) return $data; } + $cursorPaginationAttribute = null; $metadata = isset($context['resource_class']) && null !== $this->resourceMetadataFactory ? $this->resourceMetadataFactory->create($context['resource_class']) : null; $isPaginatedWithCursor = $paginated && null !== $metadata && null !== $cursorPaginationAttribute = $metadata->getCollectionOperationAttribute($context['collection_operation_name'] ?? $context['subresource_operation_name'], 'pagination_via_cursor', null, true); - $data['hydra:view'] = [ - '@id' => IriHelper::createIri($parsed['parts'], $parsed['parameters'], $this->pageParameterName, $paginated && !$isPaginatedWithCursor ? $currentPage : null), - '@type' => 'hydra:PartialCollectionView', - ]; + $data['hydra:view'] = ['@id' => null, '@type' => 'hydra:PartialCollectionView']; if ($isPaginatedWithCursor) { - $objects = iterator_to_array($object); - $firstObject = current($objects); - $lastObject = end($objects); - - $data['hydra:view']['@id'] = IriHelper::createIri($parsed['parts'], $parsed['parameters']); - - if (false !== $lastObject && isset($cursorPaginationAttribute)) { - $data['hydra:view']['hydra:next'] = IriHelper::createIri($parsed['parts'], array_merge($parsed['parameters'], $this->cursorPaginationFields($cursorPaginationAttribute, 1, $lastObject))); - } - - if (false !== $firstObject && isset($cursorPaginationAttribute)) { - $data['hydra:view']['hydra:previous'] = IriHelper::createIri($parsed['parts'], array_merge($parsed['parameters'], $this->cursorPaginationFields($cursorPaginationAttribute, -1, $firstObject))); - } - } elseif ($paginated) { - if (null !== $lastPage) { - $data['hydra:view']['hydra:first'] = IriHelper::createIri($parsed['parts'], $parsed['parameters'], $this->pageParameterName, 1.); - $data['hydra:view']['hydra:last'] = IriHelper::createIri($parsed['parts'], $parsed['parameters'], $this->pageParameterName, $lastPage); - } + return $this->populateDataWithCursorBasedPagination($data, $parsed, $object, $cursorPaginationAttribute); + } - if (1. !== $currentPage) { - $data['hydra:view']['hydra:previous'] = IriHelper::createIri($parsed['parts'], $parsed['parameters'], $this->pageParameterName, $currentPage - 1.); - } + $data['hydra:view']['@id'] = IriHelper::createIri($parsed['parts'], $parsed['parameters'], $this->pageParameterName, $paginated ? $currentPage : null); - if (null !== $lastPage && $currentPage < $lastPage || null === $lastPage && $pageTotalItems >= $itemsPerPage) { - $data['hydra:view']['hydra:next'] = IriHelper::createIri($parsed['parts'], $parsed['parameters'], $this->pageParameterName, $currentPage + 1.); - } + if ($paginated) { + return $this->populateDataWithPagination($data, $parsed, $currentPage, $lastPage, $itemsPerPage, $pageTotalItems); } return $data; @@ -164,4 +143,41 @@ private function cursorPaginationFields(array $fields, int $direction, $object) return $paginationFilters; } + + private function populateDataWithCursorBasedPagination(array $data, array $parsed, \Traversable $object, $cursorPaginationAttribute): array + { + $objects = iterator_to_array($object); + $firstObject = current($objects); + $lastObject = end($objects); + + $data['hydra:view']['@id'] = IriHelper::createIri($parsed['parts'], $parsed['parameters']); + + if (false !== $lastObject && \is_array($cursorPaginationAttribute)) { + $data['hydra:view']['hydra:next'] = IriHelper::createIri($parsed['parts'], array_merge($parsed['parameters'], $this->cursorPaginationFields($cursorPaginationAttribute, 1, $lastObject))); + } + + if (false !== $firstObject && \is_array($cursorPaginationAttribute)) { + $data['hydra:view']['hydra:previous'] = IriHelper::createIri($parsed['parts'], array_merge($parsed['parameters'], $this->cursorPaginationFields($cursorPaginationAttribute, -1, $firstObject))); + } + + return $data; + } + + private function populateDataWithPagination(array $data, array $parsed, ?float $currentPage, ?float $lastPage, ?float $itemsPerPage, ?float $pageTotalItems): array + { + if (null !== $lastPage) { + $data['hydra:view']['hydra:first'] = IriHelper::createIri($parsed['parts'], $parsed['parameters'], $this->pageParameterName, 1.); + $data['hydra:view']['hydra:last'] = IriHelper::createIri($parsed['parts'], $parsed['parameters'], $this->pageParameterName, $lastPage); + } + + if (1. !== $currentPage) { + $data['hydra:view']['hydra:previous'] = IriHelper::createIri($parsed['parts'], $parsed['parameters'], $this->pageParameterName, $currentPage - 1.); + } + + if ((null !== $lastPage && $currentPage < $lastPage) || (null === $lastPage && $pageTotalItems >= $itemsPerPage)) { + $data['hydra:view']['hydra:next'] = IriHelper::createIri($parsed['parts'], $parsed['parameters'], $this->pageParameterName, $currentPage + 1.); + } + + return $data; + } } diff --git a/tests/Hydra/Serializer/PartialCollectionViewNormalizerTest.php b/tests/Hydra/Serializer/PartialCollectionViewNormalizerTest.php index 5531da57351..d6ebe2b4da6 100644 --- a/tests/Hydra/Serializer/PartialCollectionViewNormalizerTest.php +++ b/tests/Hydra/Serializer/PartialCollectionViewNormalizerTest.php @@ -17,6 +17,8 @@ use ApiPlatform\Core\DataProvider\PartialPaginatorInterface; use ApiPlatform\Core\Hydra\Serializer\PartialCollectionViewNormalizer; use ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface; +use ApiPlatform\Core\Metadata\Resource\ResourceMetadata; +use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\SoMany; use ApiPlatform\Core\Tests\ProphecyTrait; use PHPUnit\Framework\TestCase; use Prophecy\Argument; @@ -86,7 +88,24 @@ public function testNormalizePartialPaginator() ); } - private function normalizePaginator($partial = false) + public function testNormalizeWithCursorBasedPagination(): void + { + self::assertEquals( + [ + 'foo' => 'bar', + 'hydra:totalItems' => 40, + 'hydra:view' => [ + '@id' => '/', + '@type' => 'hydra:PartialCollectionView', + 'hydra:previous' => '/?id%5Bgt%5D=1', + 'hydra:next' => '/?id%5Blt%5D=2', + ], + ], + $this->normalizePaginator(false, true) + ); + } + + private function normalizePaginator(bool $partial = false, bool $cursor = false) { $paginatorProphecy = $this->prophesize($partial ? PartialPaginatorInterface::class : PaginatorInterface::class); $paginatorProphecy->getCurrentPage()->willReturn(3)->shouldBeCalled(); @@ -103,11 +122,33 @@ private function normalizePaginator($partial = false) $decoratedNormalizerProphecy = $this->prophesize(NormalizerInterface::class); $decoratedNormalizerProphecy->normalize(Argument::type($partial ? PartialPaginatorInterface::class : PaginatorInterface::class), null, Argument::type('array'))->willReturn($decoratedNormalize)->shouldBeCalled(); - $resourceMetadataFactory = $this->prophesize(ResourceMetadataFactoryInterface::class); - $normalizer = new PartialCollectionViewNormalizer($decoratedNormalizerProphecy->reveal(), '_page', 'pagination', $resourceMetadataFactory->reveal()); + $resourceMetadataFactoryProphecy = null; + + if ($cursor) { + $firstSoMany = new SoMany(); + $firstSoMany->id = 1; + $firstSoMany->content = 'SoMany #1'; + + $lastSoMany = new SoMany(); + $lastSoMany->id = 2; + $lastSoMany->content = 'SoMany #2'; + + $paginatorProphecy->rewind()->willReturn()->shouldBeCalledOnce(); + $paginatorProphecy->valid()->willReturn(true, true, false)->shouldBeCalledTimes(3); + $paginatorProphecy->key()->willReturn(1, 2)->shouldBeCalledTimes(2); + $paginatorProphecy->current()->willReturn($firstSoMany, $lastSoMany)->shouldBeCalledTimes(2); + $paginatorProphecy->next()->willReturn()->shouldBeCalledTimes(2); + + $soManyMetadata = new ResourceMetadata(null, null, null, null, ['get' => ['pagination_via_cursor' => [['field' => 'id', 'direction' => 'desc']]]]); + + $resourceMetadataFactoryProphecy = $this->prophesize(ResourceMetadataFactoryInterface::class); + $resourceMetadataFactoryProphecy->create(SoMany::class)->willReturn($soManyMetadata)->shouldBeCalledOnce(); + } + + $normalizer = new PartialCollectionViewNormalizer($decoratedNormalizerProphecy->reveal(), '_page', 'pagination', $resourceMetadataFactoryProphecy ? $resourceMetadataFactoryProphecy->reveal() : null); - return $normalizer->normalize($paginatorProphecy->reveal()); + return $normalizer->normalize($paginatorProphecy->reveal(), null, ['resource_class' => SoMany::class, 'collection_operation_name' => 'get']); } public function testSupportsNormalization()