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

Commit cfbbbe9

Browse files
weierophinneyweierophinney
committed
Merge pull request #36 from alextech/hotfix/missing_parameters
Keep original query filters in collection self _links
2 parents 7bd1a64 + 2529382 commit cfbbbe9

File tree

6 files changed

+292
-6
lines changed

6 files changed

+292
-6
lines changed

‎CHANGELOG.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,8 @@ Versions prior to 0.4.0 were released as the package "weierophinney/hal".
2424

2525
### Fixed
2626

27-
- Nothing.
27+
- [#36](https://github.com/zendframework/zend-expressive-hal/pull/36)
28+
Fixes lost parameter queries in collection output.
2829

2930
## 1.0.0 - 2018-03-15
3031

‎src/ResourceGenerator/RouteBasedCollectionStrategy.php

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,8 +100,18 @@ protected function generateSelfLink(
100100
ResourceGenerator $resourceGenerator,
101101
ServerRequestInterface $request
102102
) {
103+
104+
$routeParams = $metadata->getRouteParams() ?? [];
105+
$queryStringArgs = array_merge($request->getQueryParams() ?? [], $metadata->getQueryStringArguments() ?? []);
106+
103107
return $resourceGenerator
104108
->getLinkGenerator()
105-
->fromRoute('self', $request, $metadata->getRoute());
109+
->fromRoute(
110+
'self',
111+
$request,
112+
$metadata->getRoute(),
113+
$routeParams,
114+
$queryStringArgs
115+
);
106116
}
107117
}

‎src/ResourceGenerator/UrlBasedCollectionStrategy.php

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ protected function generateLinkForPage(
7171
) : Link {
7272
$paginationParam = $metadata->getPaginationParam();
7373
$paginationType = $metadata->getPaginationParamType();
74-
$url = $metadata->getUrl();
74+
$url = $metadata->getUrl() . '?' . http_build_query($request->getQueryParams());
7575

7676
switch ($paginationType) {
7777
case Metadata\AbstractCollectionMetadata::TYPE_PLACEHOLDER:
@@ -101,7 +101,14 @@ protected function generateSelfLink(
101101
ResourceGenerator $resourceGenerator,
102102
ServerRequestInterface $request
103103
) {
104-
return new Link('self', $metadata->getUrl());
104+
105+
$queryStringArgs = $request->getQueryParams();
106+
$url = $metadata->getUrl();
107+
if ($queryStringArgs !== null) {
108+
$url .= '?' . http_build_query($queryStringArgs);
109+
}
110+
111+
return new Link('self', $url);
105112
}
106113

107114
private function stripUrlFragment(string $url) : string

‎test/ResourceGenerator/NestedCollectionResourceGenerationTest.php

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,9 @@ public function createLinkGenerator($request)
153153
->fromRoute(
154154
'self',
155155
$request->reveal(),
156-
'collection'
156+
'collection',
157+
[],
158+
[]
157159
)
158160
->willReturn(new Link('self', '/api/collection'));
159161

‎test/ResourceGenerator/RouteBasedCollectionWithRouteParamsTest.php

Lines changed: 76 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ class RouteBasedCollectionWithRouteParamsTest extends TestCase
2929
{
3030
use Assertions;
3131

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

112+
public function testUsesRouteParamsAndQueriesSpecifiedInMetadataWhenGeneratingLinkHref()
113+
{
114+
$request = $this->prophesize(ServerRequestInterface::class);
115+
$request->getAttribute('param_1', 1)->willReturn(3);
116+
$request->getQueryParams()->willReturn([
117+
'query_1' => 'value_1',
118+
'query_2' => 'value_2',
119+
]);
120+
121+
$metadataMap = $this->prophesize(MetadataMap::class);
122+
123+
$resourceMetadata = new RouteBasedResourceMetadata(
124+
TestAsset\FooBar::class,
125+
'foo-bar',
126+
ObjectPropertyHydrator::class,
127+
'id',
128+
'bar_id',
129+
['foo_id' => 1234]
130+
);
131+
132+
$metadataMap->has(TestAsset\FooBar::class)->willReturn(true);
133+
$metadataMap->get(TestAsset\FooBar::class)->willReturn($resourceMetadata);
134+
135+
$collectionMetadata = new RouteBasedCollectionMetadata(
136+
\ArrayObject::class,
137+
'foo-bar',
138+
'foo-bar',
139+
'p',
140+
RouteBasedCollectionMetadata::TYPE_PLACEHOLDER,
141+
[],
142+
['query_2' => 'overridden_2']
143+
);
144+
$linkGenerator = $this->prophesize(LinkGenerator::class);
145+
$linkGenerator->fromRoute(
146+
'self',
147+
$request->reveal(),
148+
'foo-bar',
149+
[],
150+
['query_1' => 'value_1', 'query_2' => 'overridden_2']
151+
)->willReturn(new Link('self', '/api/foo/1234/p/3?query1=value_1&query_2=overridden_2'));
152+
153+
$metadataMap->has(\ArrayObject::class)->willReturn(true);
154+
$metadataMap->get(\ArrayObject::class)->willReturn($collectionMetadata);
155+
156+
$hydrators = $this->prophesize(ContainerInterface::class);
157+
$hydrators->get(ObjectPropertyHydrator::class)->willReturn(new ObjectPropertyHydrator());
158+
159+
$collection = new \ArrayObject($this->createCollectionItems(
160+
$linkGenerator,
161+
$request
162+
));
163+
164+
$generator = new ResourceGenerator(
165+
$metadataMap->reveal(),
166+
$hydrators->reveal(),
167+
$linkGenerator->reveal()
168+
);
169+
170+
$generator->addStrategy(
171+
RouteBasedResourceMetadata::class,
172+
ResourceGenerator\RouteBasedResourceStrategy::class
173+
);
174+
175+
$generator->addStrategy(
176+
RouteBasedCollectionMetadata::class,
177+
ResourceGenerator\RouteBasedCollectionStrategy::class
178+
);
179+
180+
$resource = $generator->fromObject($collection, $request->reveal());
181+
182+
$this->assertInstanceOf(HalResource::class, $resource);
183+
$self = $this->getLinkByRel('self', $resource);
184+
$this->assertLink('self', '/api/foo/1234/p/3?query1=value_1&query_2=overridden_2', $self);
185+
}
186+
112187
private function createLinkGeneratorProphecy($linkGenerator, $request, string $rel, int $page)
113188
{
114189
$linkGenerator->fromRoute(

‎test/ResourceGenerator/UrlBasedCollectionWithRouteParamsTest.php

Lines changed: 191 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,191 @@
1+
<?php
2+
/**
3+
* @see https://github.com/zendframework/zend-expressive-hal for the canonical source repository
4+
* @copyright Copyright (c) 2017 Zend Technologies USA Inc. (http://www.zend.com)
5+
* @license https://github.com/zendframework/zend-expressive-hal/blob/master/LICENSE.md New BSD License
6+
*/
7+
8+
namespace ZendTest\Expressive\Hal\ResourceGenerator;
9+
10+
use PHPUnit\Framework\TestCase;
11+
use Psr\Container\ContainerInterface;
12+
use Psr\Http\Message\ServerRequestInterface;
13+
use Zend\Expressive\Hal\HalResource;
14+
use Zend\Expressive\Hal\Link;
15+
use Zend\Expressive\Hal\LinkGenerator;
16+
use Zend\Expressive\Hal\Metadata\MetadataMap;
17+
use Zend\Expressive\Hal\Metadata\RouteBasedResourceMetadata;
18+
use Zend\Expressive\Hal\Metadata\UrlBasedCollectionMetadata;
19+
use Zend\Expressive\Hal\ResourceGenerator;
20+
use Zend\Hydrator\ObjectProperty as ObjectPropertyHydrator;
21+
use Zend\Paginator\Adapter\ArrayAdapter;
22+
use Zend\Paginator\Paginator;
23+
use ZendTest\Expressive\Hal\Assertions;
24+
use ZendTest\Expressive\Hal\TestAsset;
25+
26+
class UrlBasedCollectionWithRouteParamsTest extends TestCase
27+
{
28+
use Assertions;
29+
30+
public function testUsesQueriesWithPaginatorSpecifiedInMetadataWhenGeneratingLinkHref()
31+
{
32+
$request = $this->prophesize(ServerRequestInterface::class);
33+
$request->getQueryParams()->willReturn([
34+
'query_1' => 'value_1',
35+
'p' => 3,
36+
'sort' => 'ASC',
37+
]);
38+
39+
$linkGenerator = $this->prophesize(LinkGenerator::class);
40+
41+
$metadataMap = $this->prophesize(MetadataMap::class);
42+
43+
$resourceMetadata = new RouteBasedResourceMetadata(
44+
TestAsset\FooBar::class,
45+
'foo-bar',
46+
ObjectPropertyHydrator::class,
47+
'id',
48+
'bar_id',
49+
['foo_id' => 1234]
50+
);
51+
52+
$metadataMap->has(TestAsset\FooBar::class)->willReturn(true);
53+
$metadataMap->get(TestAsset\FooBar::class)->willReturn($resourceMetadata);
54+
55+
$collectionMetadata = new UrlBasedCollectionMetadata(
56+
Paginator::class,
57+
'foo-bar',
58+
'http://test.local/collection/',
59+
'p',
60+
'query'
61+
);
62+
63+
$metadataMap->has(Paginator::class)->willReturn(true);
64+
$metadataMap->get(Paginator::class)->willReturn($collectionMetadata);
65+
66+
$hydrators = $this->prophesize(ContainerInterface::class);
67+
$hydrators->get(ObjectPropertyHydrator::class)->willReturn(new ObjectPropertyHydrator());
68+
69+
$collection = new Paginator(new ArrayAdapter($this->createCollectionItems($linkGenerator, $request)));
70+
$collection->setItemCountPerPage(3);
71+
72+
$generator = new ResourceGenerator(
73+
$metadataMap->reveal(),
74+
$hydrators->reveal(),
75+
$linkGenerator->reveal()
76+
);
77+
78+
$generator->addStrategy(
79+
RouteBasedResourceMetadata::class,
80+
ResourceGenerator\RouteBasedResourceStrategy::class
81+
);
82+
83+
$generator->addStrategy(
84+
UrlBasedCollectionMetadata::class,
85+
ResourceGenerator\UrlBasedCollectionStrategy::class
86+
);
87+
88+
$resource = $generator->fromObject($collection, $request->reveal());
89+
90+
$this->assertInstanceOf(HalResource::class, $resource);
91+
$self = $this->getLinkByRel('self', $resource);
92+
$this->assertLink('self', 'http://test.local/collection/?query_1=value_1&p=3&sort=ASC', $self);
93+
$first = $this->getLinkByRel('first', $resource);
94+
$this->assertLink('first', 'http://test.local/collection/?query_1=value_1&p=1&sort=ASC', $first);
95+
$prev = $this->getLinkByRel('prev', $resource);
96+
$this->assertLink('prev', 'http://test.local/collection/?query_1=value_1&p=2&sort=ASC', $prev);
97+
$next = $this->getLinkByRel('next', $resource);
98+
$this->assertLink('next', 'http://test.local/collection/?query_1=value_1&p=4&sort=ASC', $next);
99+
$last = $this->getLinkByRel('last', $resource);
100+
$this->assertLink('last', 'http://test.local/collection/?query_1=value_1&p=5&sort=ASC', $last);
101+
}
102+
103+
public function testUsesQueriesSpecifiedInMetadataWhenGeneratingLinkHref()
104+
{
105+
$request = $this->prophesize(ServerRequestInterface::class);
106+
$request->getQueryParams()->willReturn([
107+
'query_1' => 'value_1',
108+
'query_2' => 'value_2',
109+
]);
110+
111+
$metadataMap = $this->prophesize(MetadataMap::class);
112+
113+
$resourceMetadata = new RouteBasedResourceMetadata(
114+
TestAsset\FooBar::class,
115+
'foo-bar',
116+
ObjectPropertyHydrator::class,
117+
'id',
118+
'bar_id',
119+
['foo_id' => 1234]
120+
);
121+
122+
$metadataMap->has(TestAsset\FooBar::class)->willReturn(true);
123+
$metadataMap->get(TestAsset\FooBar::class)->willReturn($resourceMetadata);
124+
125+
$collectionMetadata = new UrlBasedCollectionMetadata(
126+
\ArrayObject::class,
127+
'foo-bar',
128+
'http://test.local/collection/',
129+
'p',
130+
'query'
131+
);
132+
$linkGenerator = $this->prophesize(LinkGenerator::class);
133+
134+
$metadataMap->has(\ArrayObject::class)->willReturn(true);
135+
$metadataMap->get(\ArrayObject::class)->willReturn($collectionMetadata);
136+
137+
$hydrators = $this->prophesize(ContainerInterface::class);
138+
$hydrators->get(ObjectPropertyHydrator::class)->willReturn(new ObjectPropertyHydrator());
139+
140+
$collection = new \ArrayObject();
141+
142+
$generator = new ResourceGenerator(
143+
$metadataMap->reveal(),
144+
$hydrators->reveal(),
145+
$linkGenerator->reveal()
146+
);
147+
148+
$generator->addStrategy(
149+
RouteBasedResourceMetadata::class,
150+
ResourceGenerator\RouteBasedResourceStrategy::class
151+
);
152+
153+
$generator->addStrategy(
154+
UrlBasedCollectionMetadata::class,
155+
ResourceGenerator\UrlBasedCollectionStrategy::class
156+
);
157+
158+
$resource = $generator->fromObject($collection, $request->reveal());
159+
160+
$this->assertInstanceOf(HalResource::class, $resource);
161+
$self = $this->getLinkByRel('self', $resource);
162+
$this->assertLink('self', 'http://test.local/collection/?query_1=value_1&query_2=value_2', $self);
163+
}
164+
165+
private function createCollectionItems($linkGenerator, $request) : array
166+
{
167+
$instance = new TestAsset\FooBar;
168+
$instance->foo = 'BAR';
169+
$instance->bar = 'BAZ';
170+
171+
$items = [];
172+
for ($i = 1; $i < 15; $i += 1) {
173+
$next = clone $instance;
174+
$next->id = $i;
175+
$items[] = $next;
176+
177+
$linkGenerator
178+
->fromRoute(
179+
'self',
180+
$request->reveal(),
181+
'foo-bar',
182+
[
183+
'foo_id' => 1234,
184+
'bar_id' => $i,
185+
]
186+
)
187+
->willReturn(new Link('self', '/api/foo/1234/bar/' . $i));
188+
}
189+
return $items;
190+
}
191+
}

0 commit comments

Comments
 (0)