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

Add a middleware dispatch #32

Merged
merged 3 commits into from
Oct 12, 2015

Conversation

ezimuel
Copy link
Contributor

@ezimuel ezimuel commented Oct 1, 2015

NOTE: This PR is a WIP, please don't merge it.

This PR adds the middleware support to MVC. We want to be able to dispatch a "middleware" parameter as callable or service, using the signature:

(
  Psr\Http\Message\ServerRequestInterface $request,
  Psr\Http\Message\ResponseInterface $response
)

To do:

  • Fix and complete the unit test

$return = $middleware(Psr7Request::fromZend($request), Psr7Response::fromZend($response));
return $this->complete(Psr7Response::toZend($return), $e);
}

Copy link
Member

Choose a reason for hiding this comment

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

I would argue we should separate this logic into a different listener, which we would then add as a default listener in Zend\Mvc\Application; it would execute at higher priority than this one (or same priority, but register prior to this one). If no middleware route match parameter is found, it would simply return, which would cause the original dispatch listener to take over.

Copy link
Member

Choose a reason for hiding this comment

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

I'd write the end logic as this:

if (! $return instanceof \Psr\Http\Message\ResponseInterface) {
    $e->setResult($return);
    return $return;
}
$response = Psr7Response::toZend($return);
$e->setResult($response);
return $response;

which might fix the testing issue you're seeing.

@ezimuel
Copy link
Contributor Author

ezimuel commented Oct 1, 2015

@weierophinney I created a MiddlewareListener with the logic suggested but I'm having issue to trigger this events. The unit tests are failing, the DispatchListener is executed before the MiddlewareListener even if I used a priority of 1000 here.

*/
public function attach(EventManagerInterface $events)
{
$this->listeners[] = $events->attach(MvcEvent::EVENT_DISPATCH, [$this, 'onDispatch'], 1000);
Copy link
Member

Choose a reason for hiding this comment

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

We'll likely take this down to 1 once we solve the issue.

@ezimuel
Copy link
Contributor Author

ezimuel commented Oct 5, 2015

@weierophinney I fixed the unit test issue. The test runs with two errors on ZendTest\Mvc\Service\TranslatorServiceFactoryTest. I guess that is caused for the refactor of zend-eventmanager.

@weierophinney
Copy link
Member

You're using dev-develop of the ModuleManager, which is currently setup to use dev-develop of the servicemanager — which is the source of that issue. You can ignore it for now.

@weierophinney weierophinney added this to the 3.0.0 milestone Oct 5, 2015
}
}

if (! $return instanceof \Psr\Http\Message\ResponseInterface) {
Copy link
Contributor

Choose a reason for hiding this comment

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

move to use statement, I created PR to your branch for it ezimuel#1

@Maks3w Maks3w added the WIP label Oct 9, 2015
@weierophinney weierophinney removed the WIP label Oct 12, 2015
@weierophinney weierophinney changed the title [WIP] Add a middleware dispatch Add a middleware dispatch Oct 12, 2015
@weierophinney weierophinney merged commit 5c41367 into zendframework:develop Oct 12, 2015
weierophinney added a commit that referenced this pull request Oct 12, 2015
[WIP] Add a middleware dispatch
weierophinney added a commit that referenced this pull request Oct 12, 2015
weierophinney added a commit that referenced this pull request Oct 12, 2015
@samsonasik samsonasik mentioned this pull request Oct 16, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants