Merge pull request #236 from Ocramius/fix/allow-middleware-dispatch-t…

…o-behave-like-controller-dispatch

Fix: allow middleware dispatch to behave like controller dispatch

src/Controller/MiddlewareController.php

@@ -0,0 +1,122 @@
+<?php
+/**
+ * Zend Framework (http://framework.zend.com/)
+ *
+ * @link      http://github.com/zendframework/zf2 for the canonical source repository
+ * @copyright Copyright (c) 2005-2015 Zend Technologies USA Inc. (http://www.zend.com)
+ * @license   http://framework.zend.com/license/new-bsd New BSD License
+ */
+
+namespace Zend\Mvc\Controller;
+
+use Psr\Http\Message\ResponseInterface;
+use Psr\Http\Message\ServerRequestInterface;
+use Zend\EventManager\EventManagerInterface;
+use Zend\Http\Request;
+use Zend\Mvc\Exception\ReachedFinalHandlerException;
+use Zend\Mvc\Exception\RuntimeException;
+use Zend\Mvc\MvcEvent;
+use Zend\Psr7Bridge\Psr7ServerRequest;
+use Zend\Router\RouteMatch;
+use Zend\Stratigility\Delegate\CallableDelegateDecorator;
+use Zend\Stratigility\MiddlewarePipe;
+
+/**
+ * @internal don't use this in your codebase, or else @ocramius will hunt you down. This is just an internal
+ * @internal hack to make middleware trigger 'dispatch' events attached to the DispatchableInterface identifier.
+ *
+ * Specifically, it will receive a @see MiddlewarePipe, a @see ResponseInterface prototype, and then dispatch
+ * the pipe whilst still behaving like a normal controller. That is needed for any events attached to
+ * the @see \Zend\Stdlib\DispatchableInterface identifier to reach their listeners on any attached
+ * @see \Zend\EventManager\SharedEventManagerInterface
+ */
+final class MiddlewareController extends AbstractController
+{
+    /**
+     * @var MiddlewarePipe
+     */
+    private $pipe;
+
+    /**
+     * @var ResponseInterface
+     */
+    private $responsePrototype;
+
+    public function __construct(
+        MiddlewarePipe $pipe,
+        ResponseInterface $responsePrototype,
+        EventManagerInterface $eventManager,
+        MvcEvent $event
+    ) {
+        $this->eventIdentifier   = __CLASS__;
+        $this->pipe              = $pipe;
+        $this->responsePrototype = $responsePrototype;
+
+        $this->setEventManager($eventManager);
+        $this->setEvent($event);
+    }
+
+    /**
+     * {@inheritDoc}
+     *
+     * @throws RuntimeException
+     */
+    public function onDispatch(MvcEvent $e)
+    {
+        $routeMatch  = $e->getRouteMatch();
+        $psr7Request = $this->populateRequestParametersFromRoute(
+            $this->loadRequest()->withAttribute(RouteMatch::class, $routeMatch),
+            $routeMatch
+        );
+
+        $result = $this->pipe->process($psr7Request, new CallableDelegateDecorator(
+            function () {
+                throw ReachedFinalHandlerException::create();
+            },
+            $this->responsePrototype
+        ));
+
+        $e->setResult($result);
+
+        return $result;
+    }
+
+    /**
+     * @return \Zend\Diactoros\ServerRequest
+     *
+     * @throws RuntimeException
+     */
+    private function loadRequest()
+    {
+        $request = $this->request;
+
+        if (! $request instanceof Request) {
+            throw new RuntimeException(sprintf(
+                'Expected request to be a %s, %s given',
+                Request::class,
+                get_class($request)
+            ));
+        }
+
+        return Psr7ServerRequest::fromZend($request);
+    }
+
+    /**
+     * @param ServerRequestInterface $request
+     * @param RouteMatch|null $routeMatch
+     *
+     * @return ServerRequestInterface
+     */
+    private function populateRequestParametersFromRoute(ServerRequestInterface $request, RouteMatch $routeMatch = null)
+    {
+        if (! $routeMatch) {
+            return $request;
+        }
+
+        foreach ($routeMatch->getParams() as $key => $value) {
+            $request = $request->withAttribute($key, $value);
+        }
+
+        return $request;
+    }
+}

src/DispatchListener.php

@@ -76,6 +76,10 @@ public function attach(EventManagerInterface $events, $priority = 1)
      */
     public function onDispatch(MvcEvent $e)
     {
+        if (null !== $e->getResult()) {
+            return;
+        }
+
         $routeMatch        = $e->getRouteMatch();
         $controllerName    = $routeMatch instanceof RouteMatch
             ? $routeMatch->getParam('controller', 'not-found')

src/MiddlewareListener.php

@@ -18,7 +18,7 @@
 use Zend\EventManager\EventManagerInterface;
 use Zend\Mvc\Exception\InvalidMiddlewareException;
 use Zend\Mvc\Exception\ReachedFinalHandlerException;
-use Zend\Psr7Bridge\Psr7ServerRequest as Psr7Request;
+use Zend\Mvc\Controller\MiddlewareController;
 use Zend\Psr7Bridge\Psr7Response;
 use Zend\Router\RouteMatch;
 use Zend\Stratigility\Delegate\CallableDelegateDecorator;
@@ -45,6 +45,10 @@ public function attach(EventManagerInterface $events, $priority = 1)
      */
     public function onDispatch(MvcEvent $event)
     {
+        if (null !== $event->getResult()) {
+            return;
+        }
+
         $routeMatch = $event->getRouteMatch();
         $middleware = $routeMatch->getParam('middleware', false);
         if (false === $middleware) {
@@ -78,16 +82,12 @@ public function onDispatch(MvcEvent $event)
 
         $caughtException = null;
         try {
-            $psr7Request = Psr7Request::fromZend($request)->withAttribute(RouteMatch::class, $routeMatch);
-            foreach ($routeMatch->getParams() as $key => $value) {
-                $psr7Request = $psr7Request->withAttribute($key, $value);
-            }
-            $return = $pipe->process($psr7Request, new CallableDelegateDecorator(
-                function (PsrServerRequestInterface $request, PsrResponseInterface $response) {
-                    throw ReachedFinalHandlerException::create();
-                },
-                $psr7ResponsePrototype
-            ));
+            $return = (new MiddlewareController(
+                $pipe,
+                $psr7ResponsePrototype,
+                $application->getServiceManager()->get('EventManager'),
+                $event
+            ))->dispatch($request, $response);
         } catch (\Throwable $ex) {
             $caughtException = $ex;
         } catch (\Exception $ex) {  // @TODO clean up once PHP 7 requirement is enforced
@@ -107,6 +107,8 @@ function (PsrServerRequestInterface $request, PsrResponseInterface $response) {
             }
         }
 
+        $event->setError('');
+
         if (! $return instanceof PsrResponseInterface) {
             $event->setResult($return);
             return $return;

test/Controller/MiddlewareControllerTest.php

@@ -0,0 +1,147 @@
+<?php
+/**
+ * Zend Framework (http://framework.zend.com/)
+ *
+ * @link      http://github.com/zendframework/zf2 for the canonical source repository
+ * @copyright Copyright (c) 2005-2015 Zend Technologies USA Inc. (http://www.zend.com)
+ * @license   http://framework.zend.com/license/new-bsd New BSD License
+ */
+
+namespace ZendTest\Mvc\Controller;
+
+use PHPUnit\Framework\TestCase;
+use Psr\Http\Message\ResponseInterface;
+use Zend\EventManager\EventManager;
+use Zend\EventManager\EventManagerInterface;
+use Zend\Http\Request;
+use Zend\Http\Response;
+use Zend\Mvc\Controller\AbstractController;
+use Zend\Mvc\Controller\MiddlewareController;
+use Zend\Mvc\Exception\RuntimeException;
+use Zend\Mvc\MvcEvent;
+use Zend\Stdlib\DispatchableInterface;
+use Zend\Stdlib\RequestInterface;
+use Zend\Stratigility\MiddlewarePipe;
+
+/**
+ * @covers \Zend\Mvc\Controller\MiddlewareController
+ */
+class MiddlewareControllerTest extends TestCase
+{
+    /**
+     * @var MiddlewarePipe|\PHPUnit_Framework_MockObject_MockObject
+     */
+    private $pipe;
+
+    /**
+     * @var ResponseInterface|\PHPUnit_Framework_MockObject_MockObject
+     */
+    private $responsePrototype;
+
+    /**
+     * @var EventManagerInterface
+     */
+    private $eventManager;
+
+    /**
+     * @var AbstractController|\PHPUnit_Framework_MockObject_MockObject
+     */
+    private $controller;
+
+    /**
+     * @var MvcEvent
+     */
+    private $event;
+
+    /**
+     * {@inheritDoc}
+     */
+    protected function setUp()
+    {
+        $this->pipe              = $this->createMock(MiddlewarePipe::class);
+        $this->responsePrototype = $this->createMock(ResponseInterface::class);
+        $this->eventManager      = $this->createMock(EventManagerInterface::class);
+        $this->event             = new MvcEvent();
+        $this->eventManager      = new EventManager();
+
+        $this->controller = new MiddlewareController(
+            $this->pipe,
+            $this->responsePrototype,
+            $this->eventManager,
+            $this->event
+        );
+    }
+
+    public function testWillAssignCorrectEventManagerIdentifiers()
+    {
+        $identifiers = $this->eventManager->getIdentifiers();
+
+        self::assertContains(MiddlewareController::class, $identifiers);
+        self::assertContains(AbstractController::class, $identifiers);
+        self::assertContains(DispatchableInterface::class, $identifiers);
+    }
+
+    public function testWillDispatchARequestAndResponseWithAGivenPipe()
+    {
+        $request          = new Request();
+        $response         = new Response();
+        $result           = $this->createMock(ResponseInterface::class);
+        /* @var $dispatchListener callable|\PHPUnit_Framework_MockObject_MockObject */
+        $dispatchListener = $this->getMockBuilder(\stdClass::class)->setMethods(['__invoke'])->getMock();
+
+        $this->eventManager->attach(MvcEvent::EVENT_DISPATCH, $dispatchListener, 100);
+        $this->eventManager->attach(MvcEvent::EVENT_DISPATCH_ERROR, function () {
+            self::fail('No dispatch error expected');
+        }, 100);
+
+        $dispatchListener
+            ->expects(self::once())
+            ->method('__invoke')
+            ->with(self::callback(function (MvcEvent $event) use ($request, $response) {
+                self::assertSame($this->event, $event);
+                self::assertSame(MvcEvent::EVENT_DISPATCH, $event->getName());
+                self::assertSame($this->controller, $event->getTarget());
+                self::assertSame($request, $event->getRequest());
+                self::assertSame($response, $event->getResponse());
+
+                return true;
+            }));
+
+        $this->pipe->expects(self::once())->method('process')->willReturn($result);
+
+        $controllerResult = $this->controller->dispatch($request, $response);
+
+        self::assertSame($result, $controllerResult);
+        self::assertSame($result, $this->event->getResult());
+    }
+
+    public function testWillRefuseDispatchingInvalidRequestTypes()
+    {
+        /* @var $request RequestInterface */
+        $request          = $this->createMock(RequestInterface::class);
+        $response         = new Response();
+        /* @var $dispatchListener callable|\PHPUnit_Framework_MockObject_MockObject */
+        $dispatchListener = $this->getMockBuilder(\stdClass::class)->setMethods(['__invoke'])->getMock();
+
+        $this->eventManager->attach(MvcEvent::EVENT_DISPATCH, $dispatchListener, 100);
+
+        $dispatchListener
+            ->expects(self::once())
+            ->method('__invoke')
+            ->with(self::callback(function (MvcEvent $event) use ($request, $response) {
+                self::assertSame($this->event, $event);
+                self::assertSame(MvcEvent::EVENT_DISPATCH, $event->getName());
+                self::assertSame($this->controller, $event->getTarget());
+                self::assertSame($request, $event->getRequest());
+                self::assertSame($response, $event->getResponse());
+
+                return true;
+            }));
+
+        $this->pipe->expects(self::never())->method('process');
+
+        $this->expectException(RuntimeException::class);
+
+        $this->controller->dispatch($request, $response);
+    }
+}

test/DispatchListenerTest.php

@@ -19,6 +19,8 @@
 use Zend\Mvc\MvcEvent;
 use Zend\Router\RouteMatch;
 use Zend\ServiceManager\ServiceManager;
+use Zend\Stdlib\ResponseInterface;
+use Zend\View\Model\ModelInterface;
 
 class DispatchListenerTest extends TestCase
 {
@@ -83,4 +85,49 @@ public function testUnlocatableControllerViaAbstractFactory()
         $this->assertArrayHasKey('error', $log);
         $this->assertSame('error-controller-not-found', $log['error']);
     }
+
+    /**
+     * @dataProvider alreadySetMvcEventResultProvider
+     *
+     * @param mixed $alreadySetResult
+     */
+    public function testWillNotDispatchWhenAnMvcEventResultIsAlreadySet($alreadySetResult)
+    {
+        $event = $this->createMvcEvent('path');
+
+        $event->setResult($alreadySetResult);
+
+        $listener = new DispatchListener(new ControllerManager(new ServiceManager(), ['abstract_factories' => [
+            Controller\TestAsset\UnlocatableControllerLoaderAbstractFactory::class,
+        ]]));
+
+        $event->getApplication()->getEventManager()->attach(MvcEvent::EVENT_DISPATCH_ERROR, function () {
+            self::fail('No dispatch failures should be raised - dispatch should be skipped');
+        });
+
+        $listener->onDispatch($event);
+
+        self::assertSame($alreadySetResult, $event->getResult(), 'The event result was not replaced');
+    }
+
+    /**
+     * @return mixed[][]
+     */
+    public function alreadySetMvcEventResultProvider()
+    {
+        return [
+            [123],
+            [true],
+            [false],
+            [[]],
+            [new \stdClass()],
+            [$this],
+            [$this->createMock(ModelInterface::class)],
+            [$this->createMock(ResponseInterface::class)],
+            [$this->createMock(Response::class)],
+            [['view model data' => 'as an array']],
+            [['foo' => new \stdClass()]],
+            ['a response string'],
+        ];
+    }
 }