From c27997f95660f9353ea8a69dca81b38d9303d061 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 28 Apr 2017 13:51:36 +0200 Subject: [PATCH 01/32] `MiddlewareListener` should cancel any previous dispatch errors set in the given `MvcEvent` --- test/MiddlewareListenerTest.php | 40 ++++++++++++++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/test/MiddlewareListenerTest.php b/test/MiddlewareListenerTest.php index 9a70cc31e..29f76ab93 100644 --- a/test/MiddlewareListenerTest.php +++ b/test/MiddlewareListenerTest.php @@ -16,6 +16,7 @@ use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; use Zend\Diactoros\Response\HtmlResponse; +use Zend\Diactoros\Response as DiactorosResponse; use Zend\EventManager\EventManager; use Zend\Http\Request; use Zend\Http\Response; @@ -265,7 +266,6 @@ public function testCanLoadFromAbstractFactory() $listener = new MiddlewareListener(); $return = $listener->onDispatch($event); - $this->assertInstanceOf(Response::class, $return); $this->assertInstanceOf(Response::class, $return); $this->assertSame(200, $return->getStatusCode()); @@ -340,4 +340,42 @@ public function testNullMiddlewareThrowsInvalidMiddlewareException() $return = $listener->onDispatch($event); $this->assertEquals('FAILED', $return); } + + public function testValidMiddlewareDispatchCancelsPreviousDispatchFailures() + { + $middlewareName = uniqid('middleware', true); + /* @var $routeMatch RouteMatch|\PHPUnit_Framework_MockObject_MockObject */ + $routeMatch = $this->createMock(RouteMatch::class); + $response = new DiactorosResponse(); + /* @var $application Application|\PHPUnit_Framework_MockObject_MockObject */ + $application = $this->createMock(Application::class); + $serviceManager = $this->createMock(ServiceManager::class); + $eventManager = new EventManager(); + $middleware = $this->getMockBuilder(\stdClass::class)->setMethods(['__invoke'])->getMock(); + + $application->expects(self::any())->method('getRequest')->willReturn(new Request()); + $application->expects(self::any())->method('getEventManager')->willReturn($eventManager); + $application->expects(self::any())->method('getServiceManager')->willReturn($serviceManager); + $application->expects(self::any())->method('getResponse')->willReturn(new Response()); + $middleware->expects(self::once())->method('__invoke')->willReturn($response); + + $serviceManager->expects(self::any())->method('has')->with($middlewareName)->willReturn(true); + $serviceManager->expects(self::any())->method('get')->with($middlewareName)->willReturn($middleware); + $routeMatch->expects(self::any())->method('getParam')->with('middleware')->willReturn($middlewareName); + $routeMatch->expects(self::any())->method('getParams')->willReturn([]); + + $event = new MvcEvent(); + + $event->setRequest(new Request()); + $event->setApplication($application); + $event->setError(Application::ERROR_CONTROLLER_CANNOT_DISPATCH); + $event->setRouteMatch($routeMatch); + + $listener = new MiddlewareListener(); + $result = $listener->onDispatch($event); + + self::assertInstanceOf(Response::class, $result); + self::assertInstanceOf(Response::class, $event->getResult()); + self::assertNull($event->getError(), 'Previously set MVC errors are canceled by a successful dispatch'); + } } From 5a914bdf01154ec5051165d6418e4052c30b1335 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 28 Apr 2017 13:58:09 +0200 Subject: [PATCH 02/32] Removing route match mock - not needed --- test/MiddlewareListenerTest.php | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/test/MiddlewareListenerTest.php b/test/MiddlewareListenerTest.php index 29f76ab93..05969a401 100644 --- a/test/MiddlewareListenerTest.php +++ b/test/MiddlewareListenerTest.php @@ -27,6 +27,7 @@ use Zend\Mvc\MvcEvent; use Zend\Router\RouteMatch; use Zend\ServiceManager\ServiceManager; +use Zend\View\Model\ModelInterface; class MiddlewareListenerTest extends TestCase { @@ -344,8 +345,7 @@ public function testNullMiddlewareThrowsInvalidMiddlewareException() public function testValidMiddlewareDispatchCancelsPreviousDispatchFailures() { $middlewareName = uniqid('middleware', true); - /* @var $routeMatch RouteMatch|\PHPUnit_Framework_MockObject_MockObject */ - $routeMatch = $this->createMock(RouteMatch::class); + $routeMatch = new RouteMatch(['middleware' => $middlewareName]); $response = new DiactorosResponse(); /* @var $application Application|\PHPUnit_Framework_MockObject_MockObject */ $application = $this->createMock(Application::class); @@ -361,8 +361,6 @@ public function testValidMiddlewareDispatchCancelsPreviousDispatchFailures() $serviceManager->expects(self::any())->method('has')->with($middlewareName)->willReturn(true); $serviceManager->expects(self::any())->method('get')->with($middlewareName)->willReturn($middleware); - $routeMatch->expects(self::any())->method('getParam')->with('middleware')->willReturn($middlewareName); - $routeMatch->expects(self::any())->method('getParams')->willReturn([]); $event = new MvcEvent(); From ecab36da6ba4ea3a2ced3d7f45d25db50a820a44 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 28 Apr 2017 13:59:49 +0200 Subject: [PATCH 03/32] Canceling previous `MvcEvent` errors when successfully dispatching a middleware --- src/MiddlewareListener.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/MiddlewareListener.php b/src/MiddlewareListener.php index 3c3aaab62..0442dac01 100644 --- a/src/MiddlewareListener.php +++ b/src/MiddlewareListener.php @@ -107,6 +107,8 @@ function (PsrServerRequestInterface $request, PsrResponseInterface $response) { } } + $event->setError(''); + if (! $return instanceof PsrResponseInterface) { $event->setResult($return); return $return; From cb4d7eebd6cba85919ee6b2f43ce50ff432de94e Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 28 Apr 2017 14:07:27 +0200 Subject: [PATCH 04/32] Correcting assertion, since `MvcEvent#getError()` is always a `string` --- test/MiddlewareListenerTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/MiddlewareListenerTest.php b/test/MiddlewareListenerTest.php index 05969a401..4f606e856 100644 --- a/test/MiddlewareListenerTest.php +++ b/test/MiddlewareListenerTest.php @@ -374,6 +374,6 @@ public function testValidMiddlewareDispatchCancelsPreviousDispatchFailures() self::assertInstanceOf(Response::class, $result); self::assertInstanceOf(Response::class, $event->getResult()); - self::assertNull($event->getError(), 'Previously set MVC errors are canceled by a successful dispatch'); + self::assertEmpty($event->getError(), 'Previously set MVC errors are canceled by a successful dispatch'); } } From 0af1831ef0b3f2dd7e395e70bc49d459e1dff52a Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 28 Apr 2017 14:09:21 +0200 Subject: [PATCH 05/32] Verifying that the `MiddlewareListener` will relay anything the middleware produces, regardless of its shape/type, unless it is a PSR7-response --- test/MiddlewareListenerTest.php | 63 +++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/test/MiddlewareListenerTest.php b/test/MiddlewareListenerTest.php index 4f606e856..931004b47 100644 --- a/test/MiddlewareListenerTest.php +++ b/test/MiddlewareListenerTest.php @@ -376,4 +376,67 @@ public function testValidMiddlewareDispatchCancelsPreviousDispatchFailures() self::assertInstanceOf(Response::class, $event->getResult()); self::assertEmpty($event->getError(), 'Previously set MVC errors are canceled by a successful dispatch'); } + + /** + * @dataProvider possibleMiddlewareNonPsr7ResponseReturnValues + * + * @param mixed $middlewareResult + */ + public function testMiddlewareDispatchWillRetrieveAnyCallableReturnValue($middlewareResult) + { + $middlewareName = uniqid('middleware', true); + $routeMatch = new RouteMatch(['middleware' => $middlewareName]); + /* @var $application Application|\PHPUnit_Framework_MockObject_MockObject */ + $application = $this->createMock(Application::class); + $serviceManager = $this->createMock(ServiceManager::class); + $eventManager = new EventManager(); + $middleware = $this->getMockBuilder(\stdClass::class)->setMethods(['__invoke'])->getMock(); + + $application->expects(self::any())->method('getRequest')->willReturn(new Request()); + $application->expects(self::any())->method('getEventManager')->willReturn($eventManager); + $application->expects(self::any())->method('getServiceManager')->willReturn($serviceManager); + $application->expects(self::any())->method('getResponse')->willReturn(new Response()); + $middleware->expects(self::once())->method('__invoke')->willReturn($middlewareResult); + + $serviceManager->expects(self::any())->method('has')->with($middlewareName)->willReturn(true); + $serviceManager->expects(self::any())->method('get')->with($middlewareName)->willReturn($middleware); + + $eventManager->attach(MvcEvent::EVENT_DISPATCH_ERROR, function () { + self::fail('No dispatch error should have been raised'); + }); + + $event = new MvcEvent(); + + $event->setRequest(new Request()); + $event->setApplication($application); + $event->setError(Application::ERROR_CONTROLLER_CANNOT_DISPATCH); + $event->setRouteMatch($routeMatch); + + $listener = new MiddlewareListener(); + $result = $listener->onDispatch($event); + + self::assertSame($middlewareResult, $result); + self::assertSame($middlewareResult, $event->getResult()); + self::assertEmpty($event->getError(), 'No errors raised when the return type is unknown'); + } + + /** + * @return mixed[][] + */ + public function possibleMiddlewareNonPsr7ResponseReturnValues() + { + return [ + [123], + [true], + [false], + [[]], + [new \stdClass()], + [$this], + [$this->createMock(ModelInterface::class)], + [$this->createMock(Response::class)], + [['view model data' => 'as an array']], + [['foo' => new \stdClass()]], + ['a response string'], + ]; + } } From 606ce9424cc9c99a91ad538a100b74b834d2f8b5 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 28 Apr 2017 16:54:41 +0200 Subject: [PATCH 06/32] Using a real service manager in the `MiddlewareListenerTest` - no test is mocking anything particular at this point --- test/MiddlewareListenerTest.php | 58 +++++++++++++++++++++++---------- 1 file changed, 41 insertions(+), 17 deletions(-) diff --git a/test/MiddlewareListenerTest.php b/test/MiddlewareListenerTest.php index 931004b47..dbcbd6c33 100644 --- a/test/MiddlewareListenerTest.php +++ b/test/MiddlewareListenerTest.php @@ -50,17 +50,21 @@ public function createMvcEvent($middlewareMatched, $middleware = null) $this->routeMatch->getParam('middleware', false)->willReturn($middlewareMatched); $this->routeMatch->getParams()->willReturn([]); - $eventManager = new EventManager(); - - $serviceManager = $this->prophesize(ContainerInterface::class); - $serviceManager->has($middlewareMatched)->willReturn(true); - $serviceManager->get($middlewareMatched)->willReturn($middleware); + $eventManager = new EventManager(); + $serviceManager = new ServiceManager([ + 'factories' => [ + 'EventManager' => function () { + return new EventManager(); + }, + ], + 'services' => [ + $middlewareMatched => $middleware, + ], + ]); $application = $this->prophesize(Application::class); $application->getEventManager()->willReturn($eventManager); - $application->getServiceManager()->will(function () use ($serviceManager) { - return $serviceManager->reveal(); - }); + $application->getServiceManager()->willReturn($serviceManager); $application->getResponse()->willReturn($response); $event = new MvcEvent(); @@ -83,6 +87,7 @@ public function testSuccessfullyDispatchesMiddleware() $application = $event->getApplication(); $application->getEventManager()->attach(MvcEvent::EVENT_DISPATCH_ERROR, function ($e) { + die(var_dump($e->getParam('exception')->getMessage())); $this->fail(sprintf('dispatch.error triggered when it should not be: %s', var_export($e->getError(), 1))); }); @@ -249,6 +254,12 @@ public function testCanLoadFromAbstractFactory() $serviceManager = new ServiceManager(); $serviceManager->addAbstractFactory(TestAsset\MiddlewareAbstractFactory::class); + $serviceManager->setFactory( + 'EventManager', + function () { + return new EventManager(); + } + ); $application = $this->prophesize(Application::class); $application->getEventManager()->willReturn($eventManager); @@ -349,9 +360,18 @@ public function testValidMiddlewareDispatchCancelsPreviousDispatchFailures() $response = new DiactorosResponse(); /* @var $application Application|\PHPUnit_Framework_MockObject_MockObject */ $application = $this->createMock(Application::class); - $serviceManager = $this->createMock(ServiceManager::class); $eventManager = new EventManager(); $middleware = $this->getMockBuilder(\stdClass::class)->setMethods(['__invoke'])->getMock(); + $serviceManager = new ServiceManager([ + 'factories' => [ + 'EventManager' => function () { + return new EventManager(); + }, + ], + 'services' => [ + $middlewareName => $middleware, + ], + ]); $application->expects(self::any())->method('getRequest')->willReturn(new Request()); $application->expects(self::any())->method('getEventManager')->willReturn($eventManager); @@ -359,9 +379,6 @@ public function testValidMiddlewareDispatchCancelsPreviousDispatchFailures() $application->expects(self::any())->method('getResponse')->willReturn(new Response()); $middleware->expects(self::once())->method('__invoke')->willReturn($response); - $serviceManager->expects(self::any())->method('has')->with($middlewareName)->willReturn(true); - $serviceManager->expects(self::any())->method('get')->with($middlewareName)->willReturn($middleware); - $event = new MvcEvent(); $event->setRequest(new Request()); @@ -388,9 +405,18 @@ public function testMiddlewareDispatchWillRetrieveAnyCallableReturnValue($middle $routeMatch = new RouteMatch(['middleware' => $middlewareName]); /* @var $application Application|\PHPUnit_Framework_MockObject_MockObject */ $application = $this->createMock(Application::class); - $serviceManager = $this->createMock(ServiceManager::class); $eventManager = new EventManager(); $middleware = $this->getMockBuilder(\stdClass::class)->setMethods(['__invoke'])->getMock(); + $serviceManager = new ServiceManager([ + 'factories' => [ + 'EventManager' => function () { + return new EventManager(); + }, + ], + 'services' => [ + $middlewareName => $middleware, + ], + ]); $application->expects(self::any())->method('getRequest')->willReturn(new Request()); $application->expects(self::any())->method('getEventManager')->willReturn($eventManager); @@ -398,10 +424,8 @@ public function testMiddlewareDispatchWillRetrieveAnyCallableReturnValue($middle $application->expects(self::any())->method('getResponse')->willReturn(new Response()); $middleware->expects(self::once())->method('__invoke')->willReturn($middlewareResult); - $serviceManager->expects(self::any())->method('has')->with($middlewareName)->willReturn(true); - $serviceManager->expects(self::any())->method('get')->with($middlewareName)->willReturn($middleware); - - $eventManager->attach(MvcEvent::EVENT_DISPATCH_ERROR, function () { + $eventManager->attach(MvcEvent::EVENT_DISPATCH_ERROR, function ($e) { + var_dump($e->getParam('exception')->getMessage()); self::fail('No dispatch error should have been raised'); }); From 1ab5b13827e944b8a985b2a1c8e999ca7883f393 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 28 Apr 2017 16:55:01 +0200 Subject: [PATCH 07/32] Removed traditional `var_dump()` statement --- test/MiddlewareListenerTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/test/MiddlewareListenerTest.php b/test/MiddlewareListenerTest.php index dbcbd6c33..8df78b444 100644 --- a/test/MiddlewareListenerTest.php +++ b/test/MiddlewareListenerTest.php @@ -425,7 +425,6 @@ public function testMiddlewareDispatchWillRetrieveAnyCallableReturnValue($middle $middleware->expects(self::once())->method('__invoke')->willReturn($middlewareResult); $eventManager->attach(MvcEvent::EVENT_DISPATCH_ERROR, function ($e) { - var_dump($e->getParam('exception')->getMessage()); self::fail('No dispatch error should have been raised'); }); From 71066ad7ca60251105505fb85fc028519cd11003 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 28 Apr 2017 16:55:38 +0200 Subject: [PATCH 08/32] Creating an intermediate `MiddlewareController` that makes any middleware act like a normal controller --- src/Controller/MiddlewareController.php | 83 +++++++++++++++++++++++++ 1 file changed, 83 insertions(+) create mode 100644 src/Controller/MiddlewareController.php diff --git a/src/Controller/MiddlewareController.php b/src/Controller/MiddlewareController.php new file mode 100644 index 000000000..b08f37a19 --- /dev/null +++ b/src/Controller/MiddlewareController.php @@ -0,0 +1,83 @@ +eventIdentifier = __CLASS__; + $this->middleware = $middleware; + + $this->setEventManager($eventManager); + $this->setEvent($event); + } + + /** + * {@inheritDoc} + * @throws \Zend\Mvc\Exception\RuntimeException + */ + public function onDispatch(MvcEvent $e) + { + $request = $this->request; + $response = $this->response; + + if (! $request instanceof Request) { + throw new RuntimeException(sprintf( + 'Expected request to be a %s, %s given', + Request::class, + get_class($request) + )); + } + + if (! $response instanceof Response) { + throw new RuntimeException(sprintf( + 'Expected response to be a %s, %s given', + Response::class, + get_class($response) + )); + } + + $routeMatch = $e->getRouteMatch(); + $psr7Request = Psr7ServerRequest::fromZend($request)->withAttribute(RouteMatch::class, $routeMatch); + + if ($routeMatch) { + foreach ($routeMatch->getParams() as $key => $value) { + $psr7Request = $psr7Request->withAttribute($key, $value); + } + } + + $result = \call_user_func($this->middleware, $psr7Request, Psr7Response::fromZend($response)); + + $e->setResult($result); + + return $result; + } +} From a6d23ff801baf3ae7b637a7eb20f6629adb2240f Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 28 Apr 2017 16:56:12 +0200 Subject: [PATCH 09/32] Using the `MiddlewareListener` within the context of a `zend-mvc` dispatch --- src/MiddlewareListener.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/MiddlewareListener.php b/src/MiddlewareListener.php index 0442dac01..f8ec148dc 100644 --- a/src/MiddlewareListener.php +++ b/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; @@ -78,6 +78,11 @@ public function onDispatch(MvcEvent $event) $caughtException = null; try { + $return = (new MiddlewareController( + $middleware, + $application->getServiceManager()->get('EventManager'), + $event + ))->dispatch($request, $response); $psr7Request = Psr7Request::fromZend($request)->withAttribute(RouteMatch::class, $routeMatch); foreach ($routeMatch->getParams() as $key => $value) { $psr7Request = $psr7Request->withAttribute($key, $value); From 3c3136a9da1be3fe548ca1177381918fc6ec6db7 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 28 Apr 2017 17:02:06 +0200 Subject: [PATCH 10/32] A shared listener should be attached to middleware, so that the entire useless view layer can do its terrible view model magic --- test/MiddlewareListenerTest.php | 47 +++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/test/MiddlewareListenerTest.php b/test/MiddlewareListenerTest.php index 8df78b444..1e7a3633f 100644 --- a/test/MiddlewareListenerTest.php +++ b/test/MiddlewareListenerTest.php @@ -18,6 +18,7 @@ use Zend\Diactoros\Response\HtmlResponse; use Zend\Diactoros\Response as DiactorosResponse; use Zend\EventManager\EventManager; +use Zend\EventManager\SharedEventManager; use Zend\Http\Request; use Zend\Http\Response; use Zend\Mvc\Application; @@ -27,6 +28,7 @@ use Zend\Mvc\MvcEvent; use Zend\Router\RouteMatch; use Zend\ServiceManager\ServiceManager; +use Zend\Stdlib\DispatchableInterface; use Zend\View\Model\ModelInterface; class MiddlewareListenerTest extends TestCase @@ -462,4 +464,49 @@ public function possibleMiddlewareNonPsr7ResponseReturnValues() ['a response string'], ]; } + + + public function testValidMiddlewareFiresDispatchableInterfaceEventListeners() + { + $middlewareName = uniqid('middleware', true); + $routeMatch = new RouteMatch(['middleware' => $middlewareName]); + $response = new DiactorosResponse(); + /* @var $application Application|\PHPUnit_Framework_MockObject_MockObject */ + $application = $this->createMock(Application::class); + $sharedManager = new SharedEventManager(); + /* @var $sharedListener callable|\PHPUnit_Framework_MockObject_MockObject */ + $sharedListener = $this->getMockBuilder(\stdClass::class)->setMethods(['__invoke'])->getMock(); + $eventManager = new EventManager(); + $middleware = $this->getMockBuilder(\stdClass::class)->setMethods(['__invoke'])->getMock(); + $serviceManager = new ServiceManager([ + 'factories' => [ + 'EventManager' => function () use ($sharedManager) { + return new EventManager($sharedManager); + }, + ], + 'services' => [ + $middlewareName => $middleware, + ], + ]); + + $application->expects(self::any())->method('getRequest')->willReturn(new Request()); + $application->expects(self::any())->method('getEventManager')->willReturn($eventManager); + $application->expects(self::any())->method('getServiceManager')->willReturn($serviceManager); + $application->expects(self::any())->method('getResponse')->willReturn(new Response()); + $middleware->expects(self::once())->method('__invoke')->willReturn($response); + + $event = new MvcEvent(); + + $event->setRequest(new Request()); + $event->setApplication($application); + $event->setError(Application::ERROR_CONTROLLER_CANNOT_DISPATCH); + $event->setRouteMatch($routeMatch); + + $listener = new MiddlewareListener(); + + $sharedManager->attach(DispatchableInterface::class, MvcEvent::EVENT_DISPATCH, $sharedListener); + $sharedListener->expects(self::once())->method('__invoke')->with($event); + + $listener->onDispatch($event); + } } From a46ae7eac366e6dc48b9671321d6eb7ce066d94d Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 28 Apr 2017 17:02:35 +0200 Subject: [PATCH 11/32] Added missing docblock for a test class property --- test/View/RouteNotFoundStrategyTest.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/View/RouteNotFoundStrategyTest.php b/test/View/RouteNotFoundStrategyTest.php index c83a85600..4073002ac 100644 --- a/test/View/RouteNotFoundStrategyTest.php +++ b/test/View/RouteNotFoundStrategyTest.php @@ -23,6 +23,11 @@ class RouteNotFoundStrategyTest extends TestCase { use EventListenerIntrospectionTrait; + /** + * @var RouteNotFoundStrategy + */ + private $strategy; + public function setUp() { $this->strategy = new RouteNotFoundStrategy(); From a37be06c7c9d9f2f4cc831edfbc3dfd9a06477ce Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Mon, 1 May 2017 19:45:32 +0200 Subject: [PATCH 12/32] The `MiddlewareListener` should bail out if an MVC result is already present in the dispatched event --- test/MiddlewareListenerTest.php | 64 +++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/test/MiddlewareListenerTest.php b/test/MiddlewareListenerTest.php index 1e7a3633f..9c06f2bc7 100644 --- a/test/MiddlewareListenerTest.php +++ b/test/MiddlewareListenerTest.php @@ -509,4 +509,68 @@ public function testValidMiddlewareFiresDispatchableInterfaceEventListeners() $listener->onDispatch($event); } + + /** + * @dataProvider alreadySetMvcEventResultProvider + * + * @param mixed $alreadySetResult + */ + public function testWillNotDispatchWhenAnMvcEventResultIsAlreadySet($alreadySetResult) + { + $middlewareName = uniqid('middleware', true); + $routeMatch = new RouteMatch(['middleware' => $middlewareName]); + /* @var $application Application|\PHPUnit_Framework_MockObject_MockObject */ + $application = $this->createMock(Application::class); + $middleware = $this->getMockBuilder(\stdClass::class)->setMethods(['__invoke'])->getMock(); + $serviceManager = new ServiceManager([ + 'factories' => [ + 'EventManager' => function () { + return new EventManager(); + }, + ], + 'services' => [ + $middlewareName => $middleware, + ], + ]); + + $application->expects(self::any())->method('getRequest')->willReturn(new Request()); + $application->expects(self::any())->method('getEventManager')->willReturn(new EventManager()); + $application->expects(self::any())->method('getServiceManager')->willReturn($serviceManager); + $application->expects(self::any())->method('getResponse')->willReturn(new Response()); + $middleware->expects(self::never())->method('__invoke'); + + $event = new MvcEvent(); + + $event->setResult($alreadySetResult); // a result is already there - listener should bail out early + $event->setRequest(new Request()); + $event->setApplication($application); + $event->setError(Application::ERROR_CONTROLLER_CANNOT_DISPATCH); + $event->setRouteMatch($routeMatch); + + $listener = new MiddlewareListener(); + + $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(Response::class)], + [['view model data' => 'as an array']], + [['foo' => new \stdClass()]], + ['a response string'], + ]; + } } From 0ce89d1489dc0aa7e08a9c408924914d5760972b Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Mon, 1 May 2017 19:47:07 +0200 Subject: [PATCH 13/32] Skipping dispatch if an `MvcEvent#getResult()` is not null (fixes tests) --- src/MiddlewareListener.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/MiddlewareListener.php b/src/MiddlewareListener.php index f8ec148dc..d5c6dc48e 100644 --- a/src/MiddlewareListener.php +++ b/src/MiddlewareListener.php @@ -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) { From 533a489d30fbfc1defcb27176ff429bc18c6db41 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Mon, 1 May 2017 19:53:30 +0200 Subject: [PATCH 14/32] Verifying that the `MiddlewareListener` triggers no error events on skipped dispatch --- test/MiddlewareListenerTest.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/test/MiddlewareListenerTest.php b/test/MiddlewareListenerTest.php index 9c06f2bc7..f94117fdd 100644 --- a/test/MiddlewareListenerTest.php +++ b/test/MiddlewareListenerTest.php @@ -521,6 +521,7 @@ public function testWillNotDispatchWhenAnMvcEventResultIsAlreadySet($alreadySetR $routeMatch = new RouteMatch(['middleware' => $middlewareName]); /* @var $application Application|\PHPUnit_Framework_MockObject_MockObject */ $application = $this->createMock(Application::class); + $eventManager = new EventManager(); $middleware = $this->getMockBuilder(\stdClass::class)->setMethods(['__invoke'])->getMock(); $serviceManager = new ServiceManager([ 'factories' => [ @@ -534,7 +535,7 @@ public function testWillNotDispatchWhenAnMvcEventResultIsAlreadySet($alreadySetR ]); $application->expects(self::any())->method('getRequest')->willReturn(new Request()); - $application->expects(self::any())->method('getEventManager')->willReturn(new EventManager()); + $application->expects(self::any())->method('getEventManager')->willReturn($eventManager); $application->expects(self::any())->method('getServiceManager')->willReturn($serviceManager); $application->expects(self::any())->method('getResponse')->willReturn(new Response()); $middleware->expects(self::never())->method('__invoke'); @@ -549,6 +550,10 @@ public function testWillNotDispatchWhenAnMvcEventResultIsAlreadySet($alreadySetR $listener = new MiddlewareListener(); + $eventManager->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'); From e78b531c9456fc08fa3cb6aff267bb1c6c578892 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Mon, 1 May 2017 19:56:56 +0200 Subject: [PATCH 15/32] The dispatch listener should not interfere when `MvcEvent#getResult()` already yields a non-null result --- test/DispatchListenerTest.php | 47 +++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/test/DispatchListenerTest.php b/test/DispatchListenerTest.php index cc93d7ddc..590917e5c 100644 --- a/test/DispatchListenerTest.php +++ b/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'], + ]; + } } From 2d2f887b43dbf40bcfcb03c4f4b5e00eb95ab6c9 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Mon, 1 May 2017 19:57:23 +0200 Subject: [PATCH 16/32] Skipping dispatch when a non-null `MvcEvent#getResult()` is detected --- src/DispatchListener.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/DispatchListener.php b/src/DispatchListener.php index d79881529..f6b1dab69 100644 --- a/src/DispatchListener.php +++ b/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') From 396a65059455179966bc157495e785ac099f2674 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Mon, 1 May 2017 20:15:10 +0200 Subject: [PATCH 17/32] Corrected rebase conflicts - moved middleware pipe dispatch into the controller wrapper --- src/Controller/MiddlewareController.php | 35 ++++++++++++++++++++----- src/MiddlewareListener.php | 13 ++------- test/MiddlewareListenerTest.php | 4 ++- 3 files changed, 33 insertions(+), 19 deletions(-) diff --git a/src/Controller/MiddlewareController.php b/src/Controller/MiddlewareController.php index b08f37a19..060982d90 100644 --- a/src/Controller/MiddlewareController.php +++ b/src/Controller/MiddlewareController.php @@ -9,14 +9,19 @@ namespace Zend\Mvc\Controller; +use Psr\Http\Message\ResponseInterface; +use Psr\Http\Message\ServerRequestInterface; use Zend\EventManager\EventManager; use Zend\Http\Request; use Zend\Http\Response; +use Zend\Mvc\Exception\ReachedFinalHandlerException; use Zend\Mvc\Exception\RuntimeException; use Zend\Mvc\MvcEvent; use Zend\Psr7Bridge\Psr7Response; use Zend\Psr7Bridge\Psr7ServerRequest; use Zend\Router\RouteMatch; +use Zend\Stratigility\Delegate\CallableDelegateDecorator; +use Zend\Stratigility\MiddlewarePipe; /** * Note: I'm a terrible person @@ -27,17 +32,28 @@ final class MiddlewareController extends AbstractController { /** - * @var callable + * @var MiddlewarePipe */ - private $middleware; + private $pipe; - public function __construct(callable $middleware, EventManager $eventManager, MvcEvent $event) - { - $this->eventIdentifier = __CLASS__; - $this->middleware = $middleware; + /** + * @var ResponseInterface + */ + private $responsePrototype; + + public function __construct( + MiddlewarePipe $pipe, + ResponseInterface $responsePrototype, + EventManager $eventManager, + MvcEvent $event + ) { + $this->eventIdentifier = __CLASS__; + $this->pipe = $pipe; + $this->responsePrototype = $responsePrototype; $this->setEventManager($eventManager); $this->setEvent($event); + } /** @@ -74,7 +90,12 @@ public function onDispatch(MvcEvent $e) } } - $result = \call_user_func($this->middleware, $psr7Request, Psr7Response::fromZend($response)); + $result = $this->pipe->process($psr7Request, new CallableDelegateDecorator( + function (ServerRequestInterface $request, ResponseInterface $response) { + throw ReachedFinalHandlerException::create(); + }, + $this->responsePrototype + )); $e->setResult($result); diff --git a/src/MiddlewareListener.php b/src/MiddlewareListener.php index d5c6dc48e..f8c0f1285 100644 --- a/src/MiddlewareListener.php +++ b/src/MiddlewareListener.php @@ -83,20 +83,11 @@ public function onDispatch(MvcEvent $event) $caughtException = null; try { $return = (new MiddlewareController( - $middleware, + $pipe, + $psr7ResponsePrototype, $application->getServiceManager()->get('EventManager'), $event ))->dispatch($request, $response); - $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 - )); } catch (\Throwable $ex) { $caughtException = $ex; } catch (\Exception $ex) { // @TODO clean up once PHP 7 requirement is enforced diff --git a/test/MiddlewareListenerTest.php b/test/MiddlewareListenerTest.php index f94117fdd..af77a52e2 100644 --- a/test/MiddlewareListenerTest.php +++ b/test/MiddlewareListenerTest.php @@ -89,7 +89,6 @@ public function testSuccessfullyDispatchesMiddleware() $application = $event->getApplication(); $application->getEventManager()->attach(MvcEvent::EVENT_DISPATCH_ERROR, function ($e) { - die(var_dump($e->getParam('exception')->getMessage())); $this->fail(sprintf('dispatch.error triggered when it should not be: %s', var_export($e->getError(), 1))); }); @@ -163,6 +162,7 @@ public function testSuccessfullyDispatchesPipeOfCallableAndHttpInteropStyleMiddl $eventManager = new EventManager(); $serviceManager = $this->prophesize(ContainerInterface::class); + $serviceManager->get('EventManager')->willReturn($eventManager); $serviceManager->has('firstMiddleware')->willReturn(true); $serviceManager->get('firstMiddleware')->willReturn(function ($request, $response, $next) { $this->assertInstanceOf(ServerRequestInterface::class, $request); @@ -303,6 +303,8 @@ public function testMiddlewareWithNothingPipedReachesFinalHandlerException() }); $application->getResponse()->willReturn($response); + $serviceManager->get('EventManager')->willReturn($eventManager); + $event = new MvcEvent(); $event->setRequest(new Request()); $event->setResponse($response); From 50ec228d55a1b0f809e20e8b350ac17d8c3a900d Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Mon, 1 May 2017 20:15:31 +0200 Subject: [PATCH 18/32] Optimised imports --- src/Controller/MiddlewareController.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Controller/MiddlewareController.php b/src/Controller/MiddlewareController.php index 060982d90..23cbe500d 100644 --- a/src/Controller/MiddlewareController.php +++ b/src/Controller/MiddlewareController.php @@ -17,7 +17,6 @@ use Zend\Mvc\Exception\ReachedFinalHandlerException; use Zend\Mvc\Exception\RuntimeException; use Zend\Mvc\MvcEvent; -use Zend\Psr7Bridge\Psr7Response; use Zend\Psr7Bridge\Psr7ServerRequest; use Zend\Router\RouteMatch; use Zend\Stratigility\Delegate\CallableDelegateDecorator; From 16a77c3c47af573848338a14e2a4ab787a6ce7e6 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Mon, 1 May 2017 20:16:34 +0200 Subject: [PATCH 19/32] Removing unused callable parameters --- src/Controller/MiddlewareController.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Controller/MiddlewareController.php b/src/Controller/MiddlewareController.php index 23cbe500d..1029fc7b3 100644 --- a/src/Controller/MiddlewareController.php +++ b/src/Controller/MiddlewareController.php @@ -10,7 +10,6 @@ namespace Zend\Mvc\Controller; use Psr\Http\Message\ResponseInterface; -use Psr\Http\Message\ServerRequestInterface; use Zend\EventManager\EventManager; use Zend\Http\Request; use Zend\Http\Response; @@ -90,7 +89,7 @@ public function onDispatch(MvcEvent $e) } $result = $this->pipe->process($psr7Request, new CallableDelegateDecorator( - function (ServerRequestInterface $request, ResponseInterface $response) { + function () { throw ReachedFinalHandlerException::create(); }, $this->responsePrototype From e4824a3646195a020d075d335a367a42a0ada8cc Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Mon, 1 May 2017 20:19:46 +0200 Subject: [PATCH 20/32] Splitting request attribute population into a private method --- src/Controller/MiddlewareController.php | 31 +++++++++++++++++++------ 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/src/Controller/MiddlewareController.php b/src/Controller/MiddlewareController.php index 1029fc7b3..58eb40963 100644 --- a/src/Controller/MiddlewareController.php +++ b/src/Controller/MiddlewareController.php @@ -10,6 +10,7 @@ namespace Zend\Mvc\Controller; use Psr\Http\Message\ResponseInterface; +use Psr\Http\Message\ServerRequestInterface; use Zend\EventManager\EventManager; use Zend\Http\Request; use Zend\Http\Response; @@ -80,13 +81,10 @@ public function onDispatch(MvcEvent $e) } $routeMatch = $e->getRouteMatch(); - $psr7Request = Psr7ServerRequest::fromZend($request)->withAttribute(RouteMatch::class, $routeMatch); - - if ($routeMatch) { - foreach ($routeMatch->getParams() as $key => $value) { - $psr7Request = $psr7Request->withAttribute($key, $value); - } - } + $psr7Request = $this->populateRequestParametersFromRoute( + Psr7ServerRequest::fromZend($request)->withAttribute(RouteMatch::class, $routeMatch), + $routeMatch + ); $result = $this->pipe->process($psr7Request, new CallableDelegateDecorator( function () { @@ -99,4 +97,23 @@ function () { return $result; } + + /** + * @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; + } } From 7d33ff791bf4f9109e602ac7f0864873991afd8d Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Mon, 1 May 2017 20:22:33 +0200 Subject: [PATCH 21/32] Splitting request checks into their own method --- src/Controller/MiddlewareController.php | 44 +++++++++++++------------ 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/src/Controller/MiddlewareController.php b/src/Controller/MiddlewareController.php index 58eb40963..317085763 100644 --- a/src/Controller/MiddlewareController.php +++ b/src/Controller/MiddlewareController.php @@ -57,32 +57,14 @@ public function __construct( /** * {@inheritDoc} - * @throws \Zend\Mvc\Exception\RuntimeException + * + * @throws RuntimeException */ public function onDispatch(MvcEvent $e) { - $request = $this->request; - $response = $this->response; - - if (! $request instanceof Request) { - throw new RuntimeException(sprintf( - 'Expected request to be a %s, %s given', - Request::class, - get_class($request) - )); - } - - if (! $response instanceof Response) { - throw new RuntimeException(sprintf( - 'Expected response to be a %s, %s given', - Response::class, - get_class($response) - )); - } - $routeMatch = $e->getRouteMatch(); $psr7Request = $this->populateRequestParametersFromRoute( - Psr7ServerRequest::fromZend($request)->withAttribute(RouteMatch::class, $routeMatch), + $this->loadRequest()->withAttribute(RouteMatch::class, $routeMatch), $routeMatch ); @@ -98,6 +80,26 @@ function () { 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 From 617070071d34eef05bda312faedb7a1a4ec06d02 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Mon, 1 May 2017 20:22:43 +0200 Subject: [PATCH 22/32] Removed unused imports --- src/Controller/MiddlewareController.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Controller/MiddlewareController.php b/src/Controller/MiddlewareController.php index 317085763..5a039254d 100644 --- a/src/Controller/MiddlewareController.php +++ b/src/Controller/MiddlewareController.php @@ -13,7 +13,6 @@ use Psr\Http\Message\ServerRequestInterface; use Zend\EventManager\EventManager; use Zend\Http\Request; -use Zend\Http\Response; use Zend\Mvc\Exception\ReachedFinalHandlerException; use Zend\Mvc\Exception\RuntimeException; use Zend\Mvc\MvcEvent; From 98c6d39572479f20d6c98df0f8fc00a805a6e2c4 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Mon, 1 May 2017 20:24:30 +0200 Subject: [PATCH 23/32] Removing test that specifies that allowing anything as the return type of a middleware is OK This is likely going to land into a separate dispatch listener instead --- test/MiddlewareListenerTest.php | 71 --------------------------------- 1 file changed, 71 deletions(-) diff --git a/test/MiddlewareListenerTest.php b/test/MiddlewareListenerTest.php index af77a52e2..dc643f8e7 100644 --- a/test/MiddlewareListenerTest.php +++ b/test/MiddlewareListenerTest.php @@ -10,7 +10,6 @@ namespace ZendTest\Mvc; use Interop\Container\ContainerInterface; -use Interop\Http\ServerMiddleware\DelegateInterface; use Interop\Http\ServerMiddleware\MiddlewareInterface; use PHPUnit\Framework\TestCase; use Psr\Http\Message\ResponseInterface; @@ -398,76 +397,6 @@ public function testValidMiddlewareDispatchCancelsPreviousDispatchFailures() self::assertEmpty($event->getError(), 'Previously set MVC errors are canceled by a successful dispatch'); } - /** - * @dataProvider possibleMiddlewareNonPsr7ResponseReturnValues - * - * @param mixed $middlewareResult - */ - public function testMiddlewareDispatchWillRetrieveAnyCallableReturnValue($middlewareResult) - { - $middlewareName = uniqid('middleware', true); - $routeMatch = new RouteMatch(['middleware' => $middlewareName]); - /* @var $application Application|\PHPUnit_Framework_MockObject_MockObject */ - $application = $this->createMock(Application::class); - $eventManager = new EventManager(); - $middleware = $this->getMockBuilder(\stdClass::class)->setMethods(['__invoke'])->getMock(); - $serviceManager = new ServiceManager([ - 'factories' => [ - 'EventManager' => function () { - return new EventManager(); - }, - ], - 'services' => [ - $middlewareName => $middleware, - ], - ]); - - $application->expects(self::any())->method('getRequest')->willReturn(new Request()); - $application->expects(self::any())->method('getEventManager')->willReturn($eventManager); - $application->expects(self::any())->method('getServiceManager')->willReturn($serviceManager); - $application->expects(self::any())->method('getResponse')->willReturn(new Response()); - $middleware->expects(self::once())->method('__invoke')->willReturn($middlewareResult); - - $eventManager->attach(MvcEvent::EVENT_DISPATCH_ERROR, function ($e) { - self::fail('No dispatch error should have been raised'); - }); - - $event = new MvcEvent(); - - $event->setRequest(new Request()); - $event->setApplication($application); - $event->setError(Application::ERROR_CONTROLLER_CANNOT_DISPATCH); - $event->setRouteMatch($routeMatch); - - $listener = new MiddlewareListener(); - $result = $listener->onDispatch($event); - - self::assertSame($middlewareResult, $result); - self::assertSame($middlewareResult, $event->getResult()); - self::assertEmpty($event->getError(), 'No errors raised when the return type is unknown'); - } - - /** - * @return mixed[][] - */ - public function possibleMiddlewareNonPsr7ResponseReturnValues() - { - return [ - [123], - [true], - [false], - [[]], - [new \stdClass()], - [$this], - [$this->createMock(ModelInterface::class)], - [$this->createMock(Response::class)], - [['view model data' => 'as an array']], - [['foo' => new \stdClass()]], - ['a response string'], - ]; - } - - public function testValidMiddlewareFiresDispatchableInterfaceEventListeners() { $middlewareName = uniqid('middleware', true); From 8442764dea5877f0ec598e0900468f34adecae8f Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Mon, 1 May 2017 20:29:14 +0200 Subject: [PATCH 24/32] Documenting the reason for the `MiddlewareController` to exist --- src/Controller/MiddlewareController.php | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/Controller/MiddlewareController.php b/src/Controller/MiddlewareController.php index 5a039254d..0501cbf50 100644 --- a/src/Controller/MiddlewareController.php +++ b/src/Controller/MiddlewareController.php @@ -22,10 +22,13 @@ use Zend\Stratigility\MiddlewarePipe; /** - * Note: I'm a terrible person - * * @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 { From 29d8e5b8ed622c4d121de4a57aa8de96a902bbac Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Mon, 1 May 2017 20:53:50 +0200 Subject: [PATCH 25/32] Basic test coverage for the `MiddlewareController` --- test/Controller/MiddlewareControllerTest.php | 118 +++++++++++++++++++ 1 file changed, 118 insertions(+) create mode 100644 test/Controller/MiddlewareControllerTest.php diff --git a/test/Controller/MiddlewareControllerTest.php b/test/Controller/MiddlewareControllerTest.php new file mode 100644 index 000000000..809aa5e9a --- /dev/null +++ b/test/Controller/MiddlewareControllerTest.php @@ -0,0 +1,118 @@ +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); + $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()); + } +} From 145e30076770004fa58dfa6ae13875a137ed12e5 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Mon, 1 May 2017 20:58:21 +0200 Subject: [PATCH 26/32] Dispatching a `MiddlewareController` with an invalid request type should raise a `RuntimeException` --- test/Controller/MiddlewareControllerTest.php | 33 ++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/test/Controller/MiddlewareControllerTest.php b/test/Controller/MiddlewareControllerTest.php index 809aa5e9a..d80d1f80a 100644 --- a/test/Controller/MiddlewareControllerTest.php +++ b/test/Controller/MiddlewareControllerTest.php @@ -20,9 +20,11 @@ use Zend\Http\Response; use Zend\Mvc\Controller\AbstractController; use Zend\Mvc\Controller\MiddlewareController; +use Zend\Mvc\Exception\RuntimeException; use Zend\Mvc\InjectApplicationEventInterface; use Zend\Mvc\MvcEvent; use Zend\Stdlib\DispatchableInterface; +use Zend\Stdlib\RequestInterface; use Zend\Stratigility\MiddlewarePipe; /** @@ -88,6 +90,7 @@ 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); @@ -115,4 +118,34 @@ public function testWillDispatchARequestAndResponseWithAGivenPipe() 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); + } } From 4628c44a0b20a11095afa466102a09853a05b649 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Mon, 1 May 2017 20:58:32 +0200 Subject: [PATCH 27/32] Removed unused imports --- test/Controller/MiddlewareControllerTest.php | 4 ---- 1 file changed, 4 deletions(-) diff --git a/test/Controller/MiddlewareControllerTest.php b/test/Controller/MiddlewareControllerTest.php index d80d1f80a..6ea27ca86 100644 --- a/test/Controller/MiddlewareControllerTest.php +++ b/test/Controller/MiddlewareControllerTest.php @@ -11,17 +11,13 @@ use PHPUnit\Framework\TestCase; use Psr\Http\Message\ResponseInterface; -use ReflectionProperty; use Zend\EventManager\EventManager; -use Zend\EventManager\EventManagerAwareInterface; use Zend\EventManager\EventManagerInterface; -use Zend\EventManager\ResponseCollection; 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\InjectApplicationEventInterface; use Zend\Mvc\MvcEvent; use Zend\Stdlib\DispatchableInterface; use Zend\Stdlib\RequestInterface; From b7f52a9b81ff8ed6f12b5d28a9b20b62e060fa49 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Mon, 1 May 2017 20:58:49 +0200 Subject: [PATCH 28/32] Corrected `EventManagerInterface` type-hint --- src/Controller/MiddlewareController.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Controller/MiddlewareController.php b/src/Controller/MiddlewareController.php index 0501cbf50..d931a5c75 100644 --- a/src/Controller/MiddlewareController.php +++ b/src/Controller/MiddlewareController.php @@ -11,7 +11,7 @@ use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; -use Zend\EventManager\EventManager; +use Zend\EventManager\EventManagerInterface; use Zend\Http\Request; use Zend\Mvc\Exception\ReachedFinalHandlerException; use Zend\Mvc\Exception\RuntimeException; @@ -45,7 +45,7 @@ final class MiddlewareController extends AbstractController public function __construct( MiddlewarePipe $pipe, ResponseInterface $responsePrototype, - EventManager $eventManager, + EventManagerInterface $eventManager, MvcEvent $event ) { $this->eventIdentifier = __CLASS__; From a1227914d33709aac24019a3806fe37c7ad1229d Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Mon, 1 May 2017 21:01:21 +0200 Subject: [PATCH 29/32] CS (whitespace) --- src/Controller/MiddlewareController.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Controller/MiddlewareController.php b/src/Controller/MiddlewareController.php index d931a5c75..e052ae639 100644 --- a/src/Controller/MiddlewareController.php +++ b/src/Controller/MiddlewareController.php @@ -54,7 +54,6 @@ public function __construct( $this->setEventManager($eventManager); $this->setEvent($event); - } /** From fdf3cb802ad9850fc2e6e4bd16773f21448a8e0c Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Mon, 1 May 2017 15:04:50 -0500 Subject: [PATCH 30/32] Formatted class docblock `@internal` paragraphs --- src/Controller/MiddlewareController.php | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/Controller/MiddlewareController.php b/src/Controller/MiddlewareController.php index e052ae639..d62867538 100644 --- a/src/Controller/MiddlewareController.php +++ b/src/Controller/MiddlewareController.php @@ -22,13 +22,16 @@ 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. + * @internal don't use this in your codebase, or else @ocramius will hunt you + * down. This is just an 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 + * Specifically, it will receive a @see MiddlewarePipe and 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 { From cf5c4c5fdc4ed8e524d79b4ea4fb9ec6ed956182 Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Mon, 1 May 2017 15:08:47 -0500 Subject: [PATCH 31/32] Updated license docblocks to reflect correct year ranges following updates --- src/Controller/MiddlewareController.php | 8 +++----- src/DispatchListener.php | 8 +++----- src/MiddlewareListener.php | 8 +++----- test/Controller/MiddlewareControllerTest.php | 8 +++----- test/DispatchListenerTest.php | 8 +++----- test/MiddlewareListenerTest.php | 8 +++----- test/View/RouteNotFoundStrategyTest.php | 8 +++----- 7 files changed, 21 insertions(+), 35 deletions(-) diff --git a/src/Controller/MiddlewareController.php b/src/Controller/MiddlewareController.php index d62867538..bbab337e6 100644 --- a/src/Controller/MiddlewareController.php +++ b/src/Controller/MiddlewareController.php @@ -1,10 +1,8 @@ Date: Mon, 1 May 2017 15:18:37 -0500 Subject: [PATCH 32/32] Added CHANGELOG for #236 --- CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f3f6e14fa..fd33ca37d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,13 @@ All notable changes to this project will be documented in this file, in reverse it will be marshaled into a `Zend\Stratigility\MiddlewarePipe` instance, using the same rules as if you specified a single middleware. +- [#236](https://github.com/zendframework/zend-mvc/pull/236) adds the ability to + attach dispatch listeners to middleware when using the `MiddlewareListener`. + Attach shared events to the class identifier `Zend\Mvc\Controller\MiddlewareController`. + This feature helps ensure that listeners that should run for every controller + (e.g., authentication or authorization listeners) will run even for + middleware. + ### Deprecated - Nothing.