From b4c2afda7c00059c5bda6be765b46adc5f184d8c Mon Sep 17 00:00:00 2001 From: Nicolas Eeckeloo Date: Tue, 7 Jul 2015 11:42:53 +0200 Subject: [PATCH 1/3] Improve pagination link creation --- src/Plugin/Hal.php | 111 ++++++++++++++++++++++++--------------------- 1 file changed, 60 insertions(+), 51 deletions(-) diff --git a/src/Plugin/Hal.php b/src/Plugin/Hal.php index f66c3ff..075fc5e 100644 --- a/src/Plugin/Hal.php +++ b/src/Plugin/Hal.php @@ -466,7 +466,7 @@ public function getHydratorForEntity($entity) * * * $params = $e->getParams(); - * $params['routeOptions']['query'] = array('format' => 'json'); + * $params['routeOptions']['query'] = ['format' => 'json']; * * * @param Collection $halCollection @@ -957,76 +957,85 @@ protected function injectPaginationLinks(Collection $halCollection) $collection = $halCollection->getCollection(); $page = $halCollection->getPage(); $pageSize = $halCollection->getPageSize(); - $route = $halCollection->getCollectionRoute(); - $params = $halCollection->getCollectionRouteParams(); - $options = $halCollection->getCollectionRouteOptions(); $collection->setItemCountPerPage($pageSize); $collection->setCurrentPageNumber($page); - $count = count($collection); - if (!$count) { + $pageCount = count($collection); + if ($pageCount == 0) { return true; } - if ($page < 1 || $page > $count) { + if ($page < 1 || $page > $pageCount) { return new ApiProblem(409, 'Invalid page provided'); } - $links = $halCollection->getLinks(); - $next = ($page < $count) ? $page + 1 : false; - $prev = ($page > 1) ? $page - 1 : false; + $this->addSelfLink($halCollection); + $this->addFirstLink($halCollection); + $this->addLastLink($halCollection); + $this->addPrevLink($halCollection); + $this->addNextLink($halCollection); - // self link - $link = new Link('self'); - $link->setRoute($route); - $link->setRouteParams($params); - $link->setRouteOptions(ArrayUtils::merge($options, [ - 'query' => ['page' => $page], - ])); - $links->add($link, true); + return true; + } - // first link - $link = new Link('first'); - $link->setRoute($route); - $link->setRouteParams($params); - $link->setRouteOptions(ArrayUtils::merge($options, [ - 'query' => ['page' => null], - ])); - $links->add($link); + private function addSelfLink(Collection $halCollection) + { + $page = $halCollection->getPage(); + $link = $this->createPaginationLink('self', $halCollection, $page); + $halCollection->getLinks()->add($link, true); + } - // last link - $link = new Link('last'); - $link->setRoute($route); - $link->setRouteParams($params); - $link->setRouteOptions(ArrayUtils::merge($options, [ - 'query' => ['page' => $count], - ])); - $links->add($link); + private function addFirstLink(Collection $halCollection) + { + $link = $this->createPaginationLink('first', $halCollection); + $halCollection->getLinks()->add($link); + } + + private function addLastLink(Collection $halCollection) + { + $page = $halCollection->getCollection()->count(); + $link = $this->createPaginationLink('last', $halCollection, $page); + $halCollection->getLinks()->add($link); + } + + private function addPrevLink(Collection $halCollection) + { + $page = $halCollection->getPage(); + $prev = ($page > 1) ? $page - 1 : false; - // prev link if ($prev) { - $link = new Link('prev'); - $link->setRoute($route); - $link->setRouteParams($params); - $link->setRouteOptions(ArrayUtils::merge($options, [ - 'query' => ['page' => $prev], - ])); - $links->add($link); + $link = $this->createPaginationLink('prev', $halCollection, $prev); + $halCollection->getLinks()->add($link); } + } + + private function addNextLink(Collection $halCollection) + { + $page = $halCollection->getPage(); + $pageCount = $halCollection->getCollection()->count(); + $next = ($page < $pageCount) ? $page + 1 : false; - // next link if ($next) { - $link = new Link('next'); - $link->setRoute($route); - $link->setRouteParams($params); - $link->setRouteOptions(ArrayUtils::merge($options, [ - 'query' => ['page' => $next], - ])); - $links->add($link); + $link = $this->createPaginationLink('next', $halCollection, $next); + $halCollection->getLinks()->add($link); } + } - return true; + private function createPaginationLink($relation, Collection $halCollection, $page = null) + { + $route = $halCollection->getCollectionRoute(); + $params = $halCollection->getCollectionRouteParams(); + $options = $halCollection->getCollectionRouteOptions(); + + $link = new Link($relation); + $link->setRoute($route); + $link->setRouteParams($params); + $link->setRouteOptions(ArrayUtils::merge($options, [ + 'query' => ['page' => $page], + ])); + + return $link; } /** From 9fff422c3fef0ddf16353c039211ad9d721a13c2 Mon Sep 17 00:00:00 2001 From: Nicolas Eeckeloo Date: Tue, 14 Jul 2015 19:40:52 +0200 Subject: [PATCH 2/3] Add pagination injector --- src/Link/PaginationInjector.php | 122 ++++++++++++++++++++++ src/Link/PaginationInjectorInterface.php | 20 ++++ src/Plugin/Hal.php | 114 ++++++-------------- test/Link/PaginationInjectorTest.php | 127 +++++++++++++++++++++++ 4 files changed, 299 insertions(+), 84 deletions(-) create mode 100644 src/Link/PaginationInjector.php create mode 100644 src/Link/PaginationInjectorInterface.php create mode 100644 test/Link/PaginationInjectorTest.php diff --git a/src/Link/PaginationInjector.php b/src/Link/PaginationInjector.php new file mode 100644 index 0000000..c9695f7 --- /dev/null +++ b/src/Link/PaginationInjector.php @@ -0,0 +1,122 @@ +getCollection(); + if (!$collection instanceof Paginator) { + return false; + } + + $this->configureCollection($halCollection); + + $pageCount = count($collection); + if ($pageCount == 0) { + return true; + } + + $page = $halCollection->getPage(); + + if ($page < 1 || $page > $pageCount) { + return new ApiProblem(409, 'Invalid page provided'); + } + + $this->injectLinks($halCollection); + + return true; + } + + private function configureCollection(Collection $halCollection) + { + $collection = $halCollection->getCollection(); + $page = $halCollection->getPage(); + $pageSize = $halCollection->getPageSize(); + + $collection->setItemCountPerPage($pageSize); + $collection->setCurrentPageNumber($page); + } + + private function injectLinks(Collection $halCollection) + { + $this->injectSelfLink($halCollection); + $this->injectFirstLink($halCollection); + $this->injectLastLink($halCollection); + $this->injectPrevLink($halCollection); + $this->injectNextLink($halCollection); + } + + private function injectSelfLink(Collection $halCollection) + { + $page = $halCollection->getPage(); + $link = $this->createPaginationLink('self', $halCollection, $page); + $halCollection->getLinks()->add($link, true); + } + + private function injectFirstLink(Collection $halCollection) + { + $link = $this->createPaginationLink('first', $halCollection); + $halCollection->getLinks()->add($link); + } + + private function injectLastLink(Collection $halCollection) + { + $page = $halCollection->getCollection()->count(); + $link = $this->createPaginationLink('last', $halCollection, $page); + $halCollection->getLinks()->add($link); + } + + private function injectPrevLink(Collection $halCollection) + { + $page = $halCollection->getPage(); + $prev = ($page > 1) ? $page - 1 : false; + + if ($prev) { + $link = $this->createPaginationLink('prev', $halCollection, $prev); + $halCollection->getLinks()->add($link); + } + } + + private function injectNextLink(Collection $halCollection) + { + $page = $halCollection->getPage(); + $pageCount = $halCollection->getCollection()->count(); + $next = ($page < $pageCount) ? $page + 1 : false; + + if ($next) { + $link = $this->createPaginationLink('next', $halCollection, $next); + $halCollection->getLinks()->add($link); + } + } + + private function createPaginationLink($relation, Collection $halCollection, $page = null) + { + $options = ArrayUtils::merge( + $halCollection->getCollectionRouteOptions(), + ['query' => ['page' => $page]] + ); + + return Link::factory([ + 'rel' => $relation, + 'route' => [ + 'name' => $halCollection->getCollectionRoute(), + 'params' => $halCollection->getCollectionRouteParams(), + 'options' => $options, + ], + ]); + } +} diff --git a/src/Link/PaginationInjectorInterface.php b/src/Link/PaginationInjectorInterface.php new file mode 100644 index 0000000..9ba73f0 --- /dev/null +++ b/src/Link/PaginationInjectorInterface.php @@ -0,0 +1,20 @@ +paginationInjector instanceof PaginationInjectorInterface) { + $this->setPaginationInjector(new PaginationInjector()); + } + return $this->paginationInjector; + } + + /** + * @param PaginationInjectorInterface $injector + * @return self + */ + public function setPaginationInjector(PaginationInjectorInterface $injector) + { + $this->paginationInjector = $injector; + return $this; + } + /** * @param ServerUrl $helper * @return self @@ -950,92 +977,11 @@ public function injectSelfLink(LinkCollectionAwareInterface $resource, $route, $ * Generate HAL links for a paginated collection * * @param Collection $halCollection - * @return boolean + * @return boolean|ApiProblem */ protected function injectPaginationLinks(Collection $halCollection) { - $collection = $halCollection->getCollection(); - $page = $halCollection->getPage(); - $pageSize = $halCollection->getPageSize(); - - $collection->setItemCountPerPage($pageSize); - $collection->setCurrentPageNumber($page); - - $pageCount = count($collection); - if ($pageCount == 0) { - return true; - } - - if ($page < 1 || $page > $pageCount) { - return new ApiProblem(409, 'Invalid page provided'); - } - - $this->addSelfLink($halCollection); - $this->addFirstLink($halCollection); - $this->addLastLink($halCollection); - $this->addPrevLink($halCollection); - $this->addNextLink($halCollection); - - return true; - } - - private function addSelfLink(Collection $halCollection) - { - $page = $halCollection->getPage(); - $link = $this->createPaginationLink('self', $halCollection, $page); - $halCollection->getLinks()->add($link, true); - } - - private function addFirstLink(Collection $halCollection) - { - $link = $this->createPaginationLink('first', $halCollection); - $halCollection->getLinks()->add($link); - } - - private function addLastLink(Collection $halCollection) - { - $page = $halCollection->getCollection()->count(); - $link = $this->createPaginationLink('last', $halCollection, $page); - $halCollection->getLinks()->add($link); - } - - private function addPrevLink(Collection $halCollection) - { - $page = $halCollection->getPage(); - $prev = ($page > 1) ? $page - 1 : false; - - if ($prev) { - $link = $this->createPaginationLink('prev', $halCollection, $prev); - $halCollection->getLinks()->add($link); - } - } - - private function addNextLink(Collection $halCollection) - { - $page = $halCollection->getPage(); - $pageCount = $halCollection->getCollection()->count(); - $next = ($page < $pageCount) ? $page + 1 : false; - - if ($next) { - $link = $this->createPaginationLink('next', $halCollection, $next); - $halCollection->getLinks()->add($link); - } - } - - private function createPaginationLink($relation, Collection $halCollection, $page = null) - { - $route = $halCollection->getCollectionRoute(); - $params = $halCollection->getCollectionRouteParams(); - $options = $halCollection->getCollectionRouteOptions(); - - $link = new Link($relation); - $link->setRoute($route); - $link->setRouteParams($params); - $link->setRouteOptions(ArrayUtils::merge($options, [ - 'query' => ['page' => $page], - ])); - - return $link; + return $this->getPaginationInjector()->injectPaginationLinks($halCollection); } /** diff --git a/test/Link/PaginationInjectorTest.php b/test/Link/PaginationInjectorTest.php new file mode 100644 index 0000000..3f8bcf2 --- /dev/null +++ b/test/Link/PaginationInjectorTest.php @@ -0,0 +1,127 @@ +setCollectionRoute('foo'); + $halCollection->setPage($currentPage); + $halCollection->setPageSize(1); + + return $halCollection; + } + + public function testInjectPaginationLinksGivenIntermediatePageShouldInjectAllLinks() + { + $halCollection = $this->getHalCollection(5, 2); + + $injector = new PaginationInjector(); + $injector->injectPaginationLinks($halCollection); + + $links = $halCollection->getLinks(); + $this->assertTrue($links->has('self')); + $this->assertTrue($links->has('first')); + $this->assertTrue($links->has('last')); + $this->assertTrue($links->has('prev')); + $this->assertTrue($links->has('next')); + } + + public function testInjectPaginationLinksGivenFirstPageShouldInjectLinksExceptForPrevious() + { + $halCollection = $this->getHalCollection(5, 1); + + $injector = new PaginationInjector(); + $injector->injectPaginationLinks($halCollection); + + $links = $halCollection->getLinks(); + $this->assertTrue($links->has('self')); + $this->assertTrue($links->has('first')); + $this->assertTrue($links->has('last')); + $this->assertFalse($links->has('prev')); + $this->assertTrue($links->has('next')); + } + + public function testInjectPaginationLinksGivenLastPageShouldInjectLinksExceptForNext() + { + $halCollection = $this->getHalCollection(5, 5); + + $injector = new PaginationInjector(); + $injector->injectPaginationLinks($halCollection); + + $links = $halCollection->getLinks(); + $this->assertTrue($links->has('self')); + $this->assertTrue($links->has('first')); + $this->assertTrue($links->has('last')); + $this->assertTrue($links->has('prev')); + $this->assertFalse($links->has('next')); + } + + public function testInjectPaginationLinksGivenEmptyCollectionShouldNotInjectAnyLink() + { + $halCollection = $this->getHalCollection(0, 1); + + $injector = new PaginationInjector(); + $injector->injectPaginationLinks($halCollection); + + $links = $halCollection->getLinks(); + $this->assertFalse($links->has('self')); + $this->assertFalse($links->has('first')); + $this->assertFalse($links->has('last')); + $this->assertFalse($links->has('prev')); + $this->assertFalse($links->has('next')); + } + + public function testInjectPaginationLinksGivenPageGreaterThanPageCountShouldReturnApiProblem() + { + $halCollection = $this->getHalCollection(5, 6); + + $injector = new PaginationInjector(); + $result = $injector->injectPaginationLinks($halCollection); + + $this->assertInstanceOf('ZF\ApiProblem\ApiProblem', $result); + $this->assertEquals(409, $result->status); + } + + public function testInjectPaginationLinksGivenCollectionRouteNameShouldInjectLinksWithSameRoute() + { + $halCollection = $this->getHalCollection(5, 2); + + $injector = new PaginationInjector(); + $injector->injectPaginationLinks($halCollection); + + $collectionRoute = $halCollection->getCollectionRoute(); + + $links = $halCollection->getLinks(); + $this->assertEquals($collectionRoute, $links->get('self')->getRoute()); + $this->assertEquals($collectionRoute, $links->get('first')->getRoute()); + $this->assertEquals($collectionRoute, $links->get('last')->getRoute()); + $this->assertEquals($collectionRoute, $links->get('prev')->getRoute()); + $this->assertEquals($collectionRoute, $links->get('next')->getRoute()); + } +} From 627e80d2f134f0fde35143da7c7d8ce238b473e5 Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Thu, 16 Jul 2015 10:00:59 -0500 Subject: [PATCH 3/3] CS - normalize `!` conditions --- src/Link/PaginationInjector.php | 4 ++-- src/Plugin/Hal.php | 24 ++++++++++++------------ 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/Link/PaginationInjector.php b/src/Link/PaginationInjector.php index c9695f7..0ea6be5 100644 --- a/src/Link/PaginationInjector.php +++ b/src/Link/PaginationInjector.php @@ -19,14 +19,14 @@ class PaginationInjector implements PaginationInjectorInterface public function injectPaginationLinks(Collection $halCollection) { $collection = $halCollection->getCollection(); - if (!$collection instanceof Paginator) { + if (! $collection instanceof Paginator) { return false; } $this->configureCollection($halCollection); $pageCount = count($collection); - if ($pageCount == 0) { + if ($pageCount === 0) { return true; } diff --git a/src/Plugin/Hal.php b/src/Plugin/Hal.php index 0aabc8b..a907dc6 100644 --- a/src/Plugin/Hal.php +++ b/src/Plugin/Hal.php @@ -163,7 +163,7 @@ public function getController() */ public function getEventManager() { - if (!$this->events) { + if (! $this->events) { $this->setEventManager(new EventManager()); } return $this->events; @@ -228,7 +228,7 @@ public function getHydratorManager() */ public function getMetadataMap() { - if (!$this->metadataMap instanceof MetadataMap) { + if (! $this->metadataMap instanceof MetadataMap) { $this->setMetadataMap(new MetadataMap()); } return $this->metadataMap; @@ -251,7 +251,7 @@ public function setMetadataMap(MetadataMap $map) */ public function getPaginationInjector() { - if (!$this->paginationInjector instanceof PaginationInjectorInterface) { + if (! $this->paginationInjector instanceof PaginationInjectorInterface) { $this->setPaginationInjector(new PaginationInjector()); } return $this->paginationInjector; @@ -314,7 +314,7 @@ public function setLinkCollectionExtractor(LinkCollectionExtractorInterface $ext */ public function addHydrator($class, $hydrator) { - if (!$hydrator instanceof ExtractionInterface) { + if (! $hydrator instanceof ExtractionInterface) { $hydrator = $this->hydrators->get($hydrator); } @@ -620,7 +620,7 @@ public function renderEntity(Entity $halEntity, $renderEntity = true, $depth = 0 } } - if (!$renderEntity || ($maxDepth !== null && $depth > $maxDepth)) { + if (! $renderEntity || ($maxDepth !== null && $depth > $maxDepth)) { $entity = []; } @@ -805,7 +805,7 @@ public function createEntityFromMetadata($object, Metadata $metadata, $renderEmb $id = ($entityIdentifierName) ? $data[$entityIdentifierName]: null; - if (!$renderEmbeddedEntities) { + if (! $renderEmbeddedEntities) { $object = []; } @@ -815,7 +815,7 @@ public function createEntityFromMetadata($object, Metadata $metadata, $renderEmb $this->marshalMetadataLinks($metadata, $links); $forceSelfLink = $metadata->getForceSelfLink(); - if ($forceSelfLink && !$links->has('self')) { + if ($forceSelfLink && ! $links->has('self')) { $link = $this->marshalLinkFromMetadata($metadata, $object, $id, $metadata->getRouteIdentifierName()); $links->add($link); } @@ -873,7 +873,7 @@ public function createEntity($entity, $route, $routeIdentifierName) break; } $metadata = (!is_array($entity) && $metadataMap->has($entity)) ? $metadataMap->get($entity) : false; - if (!$metadata || ($metadata && $metadata->getForceSelfLink())) { + if (! $metadata || ($metadata && $metadata->getForceSelfLink())) { $this->injectSelfLink($halEntity, $route, $routeIdentifierName); } return $halEntity; @@ -893,12 +893,12 @@ public function createCollection($collection, $route = null) $collection = $this->createCollectionFromMetadata($collection, $metadataMap->get($collection)); } - if (!$collection instanceof Collection) { + if (! $collection instanceof Collection) { $collection = new Collection($collection); } $metadata = $metadataMap->get($collection); - if (!$metadata || ($metadata && $metadata->getForceSelfLink())) { + if (! $metadata || ($metadata && $metadata->getForceSelfLink())) { $this->injectSelfLink($collection, $route); } return $collection; @@ -922,7 +922,7 @@ public function createCollectionFromMetadata($object, Metadata $metadata) $this->marshalMetadataLinks($metadata, $links); $forceSelfLink = $metadata->getForceSelfLink(); - if ($forceSelfLink && !$links->has('self') + if ($forceSelfLink && ! $links->has('self') && ($metadata->hasUrl() || $metadata->hasRoute()) ) { $link = $this->marshalLinkFromMetadata($metadata, $object); @@ -1247,7 +1247,7 @@ protected function marshalLinkFromMetadata( return $link; } - if (!$metadata->hasRoute()) { + if (! $metadata->hasRoute()) { throw new Exception\RuntimeException(sprintf( 'Unable to create a self link for resource of type "%s"; metadata does not contain a route or a url', get_class($object)