zendframework · weierophinney · Mar 28, 2018 · Mar 28, 2018 · Mar 28, 2018
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -24,7 +24,8 @@ 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 lost parameter queries in collection output.
 
 ## 1.0.0 - 2018-03-15
 

diff --git a/src/ResourceGenerator/RouteBasedCollectionStrategy.php b/src/ResourceGenerator/RouteBasedCollectionStrategy.php
@@ -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
+            );
     }
 }
diff --git a/src/ResourceGenerator/UrlBasedCollectionStrategy.php b/src/ResourceGenerator/UrlBasedCollectionStrategy.php
@@ -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:
@@ -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

diff --git a/test/ResourceGenerator/NestedCollectionResourceGenerationTest.php b/test/ResourceGenerator/NestedCollectionResourceGenerationTest.php
@@ -153,7 +153,9 @@ public function createLinkGenerator($request)
             ->fromRoute(
                 'self',
                 $request->reveal(),
-                'collection'
+                'collection',
+                [],
+                []
             )
             ->willReturn(new Link('self', '/api/collection'));
 

diff --git a/test/ResourceGenerator/RouteBasedCollectionWithRouteParamsTest.php b/test/ResourceGenerator/RouteBasedCollectionWithRouteParamsTest.php
@@ -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);
@@ -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(

diff --git a/test/ResourceGenerator/UrlBasedCollectionWithRouteParamsTest.php b/test/ResourceGenerator/UrlBasedCollectionWithRouteParamsTest.php
@@ -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;
+    }
+}