From 8919c8254a53d253306dd165f874d761a408914d Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Thu, 1 Mar 2018 17:15:56 -0600 Subject: [PATCH 1/3] Deprecates RouteResult::getMatchedMiddleware() Per the [Expressive 2.2 roadmap](https://discourse.zendframework.com/t/roadmap-expressive-2-2/504), marking this method deprecated, as it is removed in version 3. --- src/RouteResult.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/RouteResult.php b/src/RouteResult.php index a27739e..e3b91a3 100644 --- a/src/RouteResult.php +++ b/src/RouteResult.php @@ -147,6 +147,10 @@ public function getMatchedRouteName() /** * Retrieve the matched middleware, if possible. * + * @deprecated since 2.4.0; to remove in 3.0.0. Retrieve using + * `getMatchedRoute()->getMiddleware()` if access is required. In version 3, + * RouteResult will implement the PSR-15 MiddlewareInterface, and proxy to the + * matched middleware if present, and to the handler argument otherwise. * @return false|callable|string|MiddlewareInterface|array Returns false if * the result represents a failure; otherwise, a callable, a string * service name, a MiddlewareInterface instance, or array of any of From b4cb6918f1a1d390bed62a1f8f19931a3fc0dbf1 Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Thu, 1 Mar 2018 17:30:59 -0600 Subject: [PATCH 2/3] Deprecates passing non-MiddlewareInterface arguments to Route constructor `Route` will only allow PSR-15 middleware in version 3. --- src/Route.php | 9 ++++++ test/DispatchMiddlewareTest.php | 15 ++++++++++ test/RouteTest.php | 52 +++++++++++++++++++++++++++++++-- 3 files changed, 73 insertions(+), 3 deletions(-) diff --git a/src/Route.php b/src/Route.php index fa27834..8eb2916 100644 --- a/src/Route.php +++ b/src/Route.php @@ -82,6 +82,15 @@ public function __construct($path, $middleware, $methods = self::HTTP_METHOD_ANY throw new Exception\InvalidArgumentException('Invalid path; must be a string'); } + if (! $middleware instanceof MiddlewareInterface) { + trigger_error(sprintf( + '%1$s will not accept anything other than objects implementing the MiddlewareInterface' + . ' starting in version 3.0.0. Please update your code to create %1$s instances' + . ' using MiddlewareInterface instances.', + __CLASS__ + ), E_USER_DEPRECATED); + } + if (! is_callable($middleware) && ! $middleware instanceof MiddlewareInterface && ! is_string($middleware) diff --git a/test/DispatchMiddlewareTest.php b/test/DispatchMiddlewareTest.php index f65b8ac..02722eb 100644 --- a/test/DispatchMiddlewareTest.php +++ b/test/DispatchMiddlewareTest.php @@ -22,6 +22,9 @@ class DispatchMiddlewareTest extends TestCase { + /** @var null|callable */ + private $errorHandler; + /** @var HandlerInterface|ObjectProphecy */ private $handler; @@ -35,6 +38,14 @@ public function setUp() $this->middleware = new DispatchMiddleware(); } + public function tearDown() + { + if ($this->errorHandler) { + restore_error_handler(); + $this->errorHandler = null; + } + } + public function testInvokesDelegateIfRequestDoesNotContainRouteResult() { $expected = $this->prophesize(ResponseInterface::class)->reveal(); @@ -85,6 +96,10 @@ public function invalidMiddleware() */ public function testInvalidRoutedMiddlewareInRouteResultResultsInException($middleware) { + $this->errorHandler = set_error_handler(function ($errno, $errstr) { + return true; + }, E_USER_DEPRECATED); + $this->handler->{HANDLER_METHOD}()->shouldNotBeCalled(); $routeResult = RouteResult::fromRoute(new Route('/', $middleware)); $this->request->getAttribute(RouteResult::class, false)->willReturn($routeResult); diff --git a/test/RouteTest.php b/test/RouteTest.php index c861fcb..b7b3fed 100644 --- a/test/RouteTest.php +++ b/test/RouteTest.php @@ -9,6 +9,7 @@ use Fig\Http\Message\RequestMethodInterface as RequestMethod; use PHPUnit\Framework\TestCase; +use Prophecy\Prophecy\ObjectProphecy; use Webimpress\HttpMiddlewareCompatibility\MiddlewareInterface; use Zend\Expressive\Router\Exception\InvalidArgumentException; use Zend\Expressive\Router\Route; @@ -18,15 +19,32 @@ */ class RouteTest extends TestCase { + /** @var null|callable */ + private $errorHandler; + /** - * @var callable + * @var MiddlewareInterface|ObjectProphecy */ private $noopMiddleware; public function setUp() { - $this->noopMiddleware = function ($req, $res, $next) { - }; + $this->noopMiddleware = $this->prophesize(MiddlewareInterface::class)->reveal(); + } + + public function tearDown() + { + if ($this->errorHandler) { + restore_error_handler(); + $this->errorHandler = null; + } + } + + public function registerDeprecationErrorHandler() + { + $this->errorHandler = set_error_handler(function ($errno, $errstr) { + return true; + }, E_USER_DEPRECATED); } public function testRoutePathIsRetrievable() @@ -43,6 +61,7 @@ public function testRouteMiddlewareIsRetrievable() public function testRouteMiddlewareMayBeANonCallableString() { + $this->registerDeprecationErrorHandler(); $route = new Route('/foo', 'Application\Middleware\HelloWorld'); $this->assertSame('Application\Middleware\HelloWorld', $route->getMiddleware()); } @@ -130,6 +149,7 @@ public function testThrowsExceptionDuringConstructionIfPathIsNotString() public function testThrowsExceptionDuringConstructionOnInvalidMiddleware() { + $this->registerDeprecationErrorHandler(); $this->expectException(InvalidArgumentException::class); $this->expectExceptionMessage('Invalid middleware'); @@ -226,6 +246,7 @@ public function testAllowsHttpInteropMiddleware() */ public function testAllowsNonCallableArraysAsMiddleware() { + $this->registerDeprecationErrorHandler(); $middleware = ['Non', 'Callable', 'Middleware']; $route = new Route('/test', $middleware, Route::HTTP_METHOD_ANY); $this->assertSame($middleware, $route->getMiddleware()); @@ -251,9 +272,34 @@ public function invalidMiddleware() */ public function testConstructorRaisesExceptionForInvalidMiddleware($middleware) { + $this->registerDeprecationErrorHandler(); $this->expectException(InvalidArgumentException::class); $this->expectExceptionMessage('Invalid middleware'); new Route('/test', $middleware); } + + public function validNonInteropMiddleware() + { + yield 'string-callable' => ['sprintf']; + yield 'array' => [['sprintf']]; + yield 'array-callable' => [[$this, 'setUp']]; + } + + /** + * @dataProvider validNonInteropMiddleware + * @param mixed $middleware + */ + public function testPassingNonInteropMiddlewareToConstructorTriggersDeprecationNotice($middleware) + { + $spy = (object) ['found' => false]; + $this->errorHandler = set_error_handler(function ($errno, $errstr) use ($spy) { + $spy->found = true; + return true; + }, E_USER_DEPRECATED); + + new Route('/test', $middleware); + + $this->assertTrue($spy->found); + } } From 80b5f000a3f7739d7127befd02b45f17ee507a7f Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Thu, 1 Mar 2018 17:34:34 -0600 Subject: [PATCH 3/3] Adds CHANGELOG entries for #56 --- CHANGELOG.md | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9bc13f9..1d1ef9f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,37 @@ All notable changes to this project will be documented in this file, in reverse chronological order by release. +## 2.4.0 - TBD + +### Added + +- Nothing. + +### Changed + +- Nothing. + +### Deprecated + +- [#56](https://github.com/zendframework/zend-expressive-router/pull/56) + deprecates the method `Zend\Expressive\RouteResult::getMatchedMiddleware()`, + as it will be removed in version 3. If you need access to the middleware, + use `getMatchedRoute()->getMiddleware()`. (In version 3, the `RouteResult` + _is_ middleware, and will proxy to it.) + +- [#56](https://github.com/zendframework/zend-expressive-router/pull/56) + deprecates passing non-MiddlewareInterface instances to the constructor of + `Zend\Expressive\Route`. The class now triggers a deprecation notice when this + occurs, indicating the changes the developer needs to make. + +### Removed + +- Nothing. + +### Fixed + +- Nothing. + ## 2.3.0 - 2018-02-01 ### Added