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

Add route match params to request attributes #210

Conversation

asgrim
Copy link
Contributor

@asgrim asgrim commented Dec 12, 2016

Proposed change copies the RouteMatch parameters into the PSR-7 \Zend\Diactoros\ServerRequest so that they may be referred to in a middleware that has been invoked by the MiddlewareListener. Currently, if using a middleware in this way, I don't see any other way to access the matched route information (specifically, matched route parameters from a segment route, e.g. /:uuid matches, but I can't access the uuid parameter anywhere in the middleware action).

@asgrim asgrim force-pushed the add-route-match-params-to-request-attributes branch from 9abe66e to 79fe956 Compare December 13, 2016 17:38
@akrabat
Copy link
Contributor

akrabat commented Dec 19, 2016

Looks good to me.

Copy link
Member

@michalbundyra michalbundyra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think would be also nice inject whole RouteMatch, not only individual matched params (as we have it in expressive).

@@ -59,7 +59,11 @@ public function onDispatch(MvcEvent $event)

$caughtException = null;
try {
$return = $middleware(Psr7Request::fromZend($request), Psr7Response::fromZend($response));
$psr7Request = Psr7Request::fromZend($request);
foreach ($routeMatch->getParams() as $key => $value) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think will be nice to inject also while RouteMatch (as we have it in expressive: https://github.com/zendframework/zend-expressive/blob/master/src/Application.php#L431-L435), not just individual matched params.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, added

$listener = new MiddlewareListener();
$return = $listener->onDispatch($event);
self::assertInstanceOf(Response::class, $return);
self::assertSame($matchedRouteParam, $return->getBody());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use $this-> (instead of self::) in 2 above assertions to keep consistency with other tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated

Copy link
Member

@weierophinney weierophinney left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Patch looks quite reasonable, and makes the support within middleware more on par with that in MVC controllers. 👍

@akrabat akrabat merged commit 05f3e6f into zendframework:master Dec 20, 2016
akrabat added a commit that referenced this pull request Dec 20, 2016
…-attributes

Add route match params to request attributes
akrabat added a commit that referenced this pull request Dec 20, 2016
akrabat added a commit that referenced this pull request Dec 20, 2016
akrabat added a commit that referenced this pull request Dec 20, 2016
@akrabat akrabat added this to the 3.0.4 milestone Dec 20, 2016
akrabat added a commit that referenced this pull request Dec 20, 2016
Added
-----

- Nothing.

Deprecated
----------

- Nothing.

Removed
-------

- Nothing.

Fixed
-----

- [#210](#210) copies the
  `RouteMatch` and its parameters to the PSR-7 `ServerRequest` object so
  that they are available to middleware.
@asgrim asgrim deleted the add-route-match-params-to-request-attributes branch December 20, 2016 15:46
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants