Skip to content
This repository has been archived by the owner on Jan 29, 2020. It is now read-only.

Commit

Permalink
Merge branch 'hotfix/36'
Browse files Browse the repository at this point in the history
Close #36
  • Loading branch information
weierophinney committed Mar 28, 2018
2 parents 7bd1a64 + f4d975a commit 69d2c62
Show file tree
Hide file tree
Showing 6 changed files with 295 additions and 7 deletions.
7 changes: 5 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ All notable changes to this project will be documented in this file, in reverse

Versions prior to 0.4.0 were released as the package "weierophinney/hal".

## 1.0.1 - TBD
## 1.0.1 - 2018-03-28

### Added

Expand All @@ -24,7 +24,10 @@ Versions prior to 0.4.0 were released as the package "weierophinney/hal".

### Fixed

- Nothing.
- [#36](https://github.com/zendframework/zend-expressive-hal/pull/36)
fixes an issue whereby query string arguments were not being added to
links generated for a resource. It now correctly merges those specified in
metadata with those from the request when generating links.

## 1.0.0 - 2018-03-15

Expand Down
12 changes: 11 additions & 1 deletion src/ResourceGenerator/RouteBasedCollectionStrategy.php
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,18 @@ protected function generateSelfLink(
ResourceGenerator $resourceGenerator,
ServerRequestInterface $request
) {

$routeParams = $metadata->getRouteParams() ?? [];
$queryStringArgs = array_merge($request->getQueryParams() ?? [], $metadata->getQueryStringArguments() ?? []);

return $resourceGenerator
->getLinkGenerator()
->fromRoute('self', $request, $metadata->getRoute());
->fromRoute(
'self',
$request,
$metadata->getRoute(),
$routeParams,
$queryStringArgs
);
}
}
11 changes: 9 additions & 2 deletions src/ResourceGenerator/UrlBasedCollectionStrategy.php
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ protected function generateLinkForPage(
) : Link {
$paginationParam = $metadata->getPaginationParam();
$paginationType = $metadata->getPaginationParamType();
$url = $metadata->getUrl();
$url = $metadata->getUrl() . '?' . http_build_query($request->getQueryParams());

switch ($paginationType) {
case Metadata\AbstractCollectionMetadata::TYPE_PLACEHOLDER:
Expand Down Expand Up @@ -101,7 +101,14 @@ protected function generateSelfLink(
ResourceGenerator $resourceGenerator,
ServerRequestInterface $request
) {
return new Link('self', $metadata->getUrl());

$queryStringArgs = $request->getQueryParams();
$url = $metadata->getUrl();
if ($queryStringArgs !== null) {
$url .= '?' . http_build_query($queryStringArgs);
}

return new Link('self', $url);
}

private function stripUrlFragment(string $url) : string
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,9 @@ public function createLinkGenerator($request)
->fromRoute(
'self',
$request->reveal(),
'collection'
'collection',
[],
[]
)
->willReturn(new Link('self', '/api/collection'));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class RouteBasedCollectionWithRouteParamsTest extends TestCase
{
use Assertions;

public function testUsesRouteParamsSpecifiedInMetadataWhenGeneratingLinkHref()
public function testUsesRouteParamsAndQueriesWithPaginatorSpecifiedInMetadataWhenGeneratingLinkHref()
{
$request = $this->prophesize(ServerRequestInterface::class);
$request->getAttribute('p', 1)->willReturn(3);
Expand Down Expand Up @@ -109,6 +109,81 @@ public function testUsesRouteParamsSpecifiedInMetadataWhenGeneratingLinkHref()
$this->assertLink('last', '/api/foo/1234/p/5?sort=ASC', $last);
}

public function testUsesRouteParamsAndQueriesSpecifiedInMetadataWhenGeneratingLinkHref()
{
$request = $this->prophesize(ServerRequestInterface::class);
$request->getAttribute('param_1', 1)->willReturn(3);
$request->getQueryParams()->willReturn([
'query_1' => 'value_1',
'query_2' => 'value_2',
]);

$metadataMap = $this->prophesize(MetadataMap::class);

$resourceMetadata = new RouteBasedResourceMetadata(
TestAsset\FooBar::class,
'foo-bar',
ObjectPropertyHydrator::class,
'id',
'bar_id',
['foo_id' => 1234]
);

$metadataMap->has(TestAsset\FooBar::class)->willReturn(true);
$metadataMap->get(TestAsset\FooBar::class)->willReturn($resourceMetadata);

$collectionMetadata = new RouteBasedCollectionMetadata(
\ArrayObject::class,
'foo-bar',
'foo-bar',
'p',
RouteBasedCollectionMetadata::TYPE_PLACEHOLDER,
[],
['query_2' => 'overridden_2']
);
$linkGenerator = $this->prophesize(LinkGenerator::class);
$linkGenerator->fromRoute(
'self',
$request->reveal(),
'foo-bar',
[],
['query_1' => 'value_1', 'query_2' => 'overridden_2']
)->willReturn(new Link('self', '/api/foo/1234/p/3?query1=value_1&query_2=overridden_2'));

$metadataMap->has(\ArrayObject::class)->willReturn(true);
$metadataMap->get(\ArrayObject::class)->willReturn($collectionMetadata);

$hydrators = $this->prophesize(ContainerInterface::class);
$hydrators->get(ObjectPropertyHydrator::class)->willReturn(new ObjectPropertyHydrator());

$collection = new \ArrayObject($this->createCollectionItems(
$linkGenerator,
$request
));

$generator = new ResourceGenerator(
$metadataMap->reveal(),
$hydrators->reveal(),
$linkGenerator->reveal()
);

$generator->addStrategy(
RouteBasedResourceMetadata::class,
ResourceGenerator\RouteBasedResourceStrategy::class
);

$generator->addStrategy(
RouteBasedCollectionMetadata::class,
ResourceGenerator\RouteBasedCollectionStrategy::class
);

$resource = $generator->fromObject($collection, $request->reveal());

$this->assertInstanceOf(HalResource::class, $resource);
$self = $this->getLinkByRel('self', $resource);
$this->assertLink('self', '/api/foo/1234/p/3?query1=value_1&query_2=overridden_2', $self);
}

private function createLinkGeneratorProphecy($linkGenerator, $request, string $rel, int $page)
{
$linkGenerator->fromRoute(
Expand Down
191 changes: 191 additions & 0 deletions test/ResourceGenerator/UrlBasedCollectionWithRouteParamsTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,191 @@
<?php
/**
* @see https://github.com/zendframework/zend-expressive-hal for the canonical source repository
* @copyright Copyright (c) 2017 Zend Technologies USA Inc. (http://www.zend.com)
* @license https://github.com/zendframework/zend-expressive-hal/blob/master/LICENSE.md New BSD License
*/

namespace ZendTest\Expressive\Hal\ResourceGenerator;

use PHPUnit\Framework\TestCase;
use Psr\Container\ContainerInterface;
use Psr\Http\Message\ServerRequestInterface;
use Zend\Expressive\Hal\HalResource;
use Zend\Expressive\Hal\Link;
use Zend\Expressive\Hal\LinkGenerator;
use Zend\Expressive\Hal\Metadata\MetadataMap;
use Zend\Expressive\Hal\Metadata\RouteBasedResourceMetadata;
use Zend\Expressive\Hal\Metadata\UrlBasedCollectionMetadata;
use Zend\Expressive\Hal\ResourceGenerator;
use Zend\Hydrator\ObjectProperty as ObjectPropertyHydrator;
use Zend\Paginator\Adapter\ArrayAdapter;
use Zend\Paginator\Paginator;
use ZendTest\Expressive\Hal\Assertions;
use ZendTest\Expressive\Hal\TestAsset;

class UrlBasedCollectionWithRouteParamsTest extends TestCase
{
use Assertions;

public function testUsesQueriesWithPaginatorSpecifiedInMetadataWhenGeneratingLinkHref()
{
$request = $this->prophesize(ServerRequestInterface::class);
$request->getQueryParams()->willReturn([
'query_1' => 'value_1',
'p' => 3,
'sort' => 'ASC',
]);

$linkGenerator = $this->prophesize(LinkGenerator::class);

$metadataMap = $this->prophesize(MetadataMap::class);

$resourceMetadata = new RouteBasedResourceMetadata(
TestAsset\FooBar::class,
'foo-bar',
ObjectPropertyHydrator::class,
'id',
'bar_id',
['foo_id' => 1234]
);

$metadataMap->has(TestAsset\FooBar::class)->willReturn(true);
$metadataMap->get(TestAsset\FooBar::class)->willReturn($resourceMetadata);

$collectionMetadata = new UrlBasedCollectionMetadata(
Paginator::class,
'foo-bar',
'http://test.local/collection/',
'p',
'query'
);

$metadataMap->has(Paginator::class)->willReturn(true);
$metadataMap->get(Paginator::class)->willReturn($collectionMetadata);

$hydrators = $this->prophesize(ContainerInterface::class);
$hydrators->get(ObjectPropertyHydrator::class)->willReturn(new ObjectPropertyHydrator());

$collection = new Paginator(new ArrayAdapter($this->createCollectionItems($linkGenerator, $request)));
$collection->setItemCountPerPage(3);

$generator = new ResourceGenerator(
$metadataMap->reveal(),
$hydrators->reveal(),
$linkGenerator->reveal()
);

$generator->addStrategy(
RouteBasedResourceMetadata::class,
ResourceGenerator\RouteBasedResourceStrategy::class
);

$generator->addStrategy(
UrlBasedCollectionMetadata::class,
ResourceGenerator\UrlBasedCollectionStrategy::class
);

$resource = $generator->fromObject($collection, $request->reveal());

$this->assertInstanceOf(HalResource::class, $resource);
$self = $this->getLinkByRel('self', $resource);
$this->assertLink('self', 'http://test.local/collection/?query_1=value_1&p=3&sort=ASC', $self);
$first = $this->getLinkByRel('first', $resource);
$this->assertLink('first', 'http://test.local/collection/?query_1=value_1&p=1&sort=ASC', $first);
$prev = $this->getLinkByRel('prev', $resource);
$this->assertLink('prev', 'http://test.local/collection/?query_1=value_1&p=2&sort=ASC', $prev);
$next = $this->getLinkByRel('next', $resource);
$this->assertLink('next', 'http://test.local/collection/?query_1=value_1&p=4&sort=ASC', $next);
$last = $this->getLinkByRel('last', $resource);
$this->assertLink('last', 'http://test.local/collection/?query_1=value_1&p=5&sort=ASC', $last);
}

public function testUsesQueriesSpecifiedInMetadataWhenGeneratingLinkHref()
{
$request = $this->prophesize(ServerRequestInterface::class);
$request->getQueryParams()->willReturn([
'query_1' => 'value_1',
'query_2' => 'value_2',
]);

$metadataMap = $this->prophesize(MetadataMap::class);

$resourceMetadata = new RouteBasedResourceMetadata(
TestAsset\FooBar::class,
'foo-bar',
ObjectPropertyHydrator::class,
'id',
'bar_id',
['foo_id' => 1234]
);

$metadataMap->has(TestAsset\FooBar::class)->willReturn(true);
$metadataMap->get(TestAsset\FooBar::class)->willReturn($resourceMetadata);

$collectionMetadata = new UrlBasedCollectionMetadata(
\ArrayObject::class,
'foo-bar',
'http://test.local/collection/',
'p',
'query'
);
$linkGenerator = $this->prophesize(LinkGenerator::class);

$metadataMap->has(\ArrayObject::class)->willReturn(true);
$metadataMap->get(\ArrayObject::class)->willReturn($collectionMetadata);

$hydrators = $this->prophesize(ContainerInterface::class);
$hydrators->get(ObjectPropertyHydrator::class)->willReturn(new ObjectPropertyHydrator());

$collection = new \ArrayObject();

$generator = new ResourceGenerator(
$metadataMap->reveal(),
$hydrators->reveal(),
$linkGenerator->reveal()
);

$generator->addStrategy(
RouteBasedResourceMetadata::class,
ResourceGenerator\RouteBasedResourceStrategy::class
);

$generator->addStrategy(
UrlBasedCollectionMetadata::class,
ResourceGenerator\UrlBasedCollectionStrategy::class
);

$resource = $generator->fromObject($collection, $request->reveal());

$this->assertInstanceOf(HalResource::class, $resource);
$self = $this->getLinkByRel('self', $resource);
$this->assertLink('self', 'http://test.local/collection/?query_1=value_1&query_2=value_2', $self);
}

private function createCollectionItems($linkGenerator, $request) : array
{
$instance = new TestAsset\FooBar;
$instance->foo = 'BAR';
$instance->bar = 'BAZ';

$items = [];
for ($i = 1; $i < 15; $i += 1) {
$next = clone $instance;
$next->id = $i;
$items[] = $next;

$linkGenerator
->fromRoute(
'self',
$request->reveal(),
'foo-bar',
[
'foo_id' => 1234,
'bar_id' => $i,
]
)
->willReturn(new Link('self', '/api/foo/1234/bar/' . $i));
}
return $items;
}
}

0 comments on commit 69d2c62

Please sign in to comment.