-
Notifications
You must be signed in to change notification settings - Fork 13
Refactor routing and dispatch logic #48
Refactor routing and dispatch logic #48
Conversation
This patch accomplishes several things. First, it extracts the logic for producing a 405 response to new middleware, `Zend\Expressive\Router\MethodNotAllowedMiddleware`. This middleware checks for a route result that is due to method failure, and, in such a case, uses the composed response prototype. Second, the `RouteMiddleware` was modified as follows: - it no longer requires a response prototype - it _always_ injects the route result as a request attribute - it no longer produces a 405 response on its own Third, the `Route` class was modified to implement the PSR-15 `MiddlewareInterface`. When processed, it proxies to the composed middleware instance. Fourth, the `RouteResult` was modified to implement the PSR-15 `MiddlewareInterface`. When processed, if it is a route failure, it delegates to the provided handler. If it is a successful route match, it proxies to the matched `Route` instance. The `getMatchedMiddleware()` method was removed, as it's essentially unnecessary. Fifth, the `DispatchMiddleware` was modified to process the `RouteResult` instance pulled from the request, instead of retrieving its composed middleware.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
src/MethodNotAllowedMiddleware.php
Outdated
|
||
public function process(ServerRequestInterface $request, RequestHandlerInterface $handler) : ResponseInterface | ||
{ | ||
$routeResult = $request->getAttribute(RouteResult::class, false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The second param false
can be removed, we are not using that is false
, the same result we get with null
(default value).
Since the default is to return a `null`, which evaluates as false-y for purposes of conditionals, we can omit it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
test/DispatchMiddlewareTest.php
Outdated
use Prophecy\Prophecy\ObjectProphecy; | ||
use Psr\Http\Message\ResponseInterface; | ||
use Psr\Http\Message\ServerRequestInterface; | ||
use Psr\Http\Server\MiddlewareInterface; | ||
use Psr\Http\Server\RequestHandlerInterface; | ||
use Zend\Expressive\Router\DispatchMiddleware; | ||
use Zend\Expressive\Router\Route; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not used anymore, same goes for the $responsePrototype property.
use Prophecy\Prophecy\ObjectProphecy; | ||
use Psr\Http\Message\ResponseInterface; | ||
use Psr\Http\Message\ServerRequestInterface; | ||
use Psr\Http\Server\MiddlewareInterface; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unused.
This patch accomplishes several things.
First, it extracts the logic for producing a 405 response to new middleware,
Zend\Expressive\Router\MethodNotAllowedMiddleware
. This middleware checks for a route result that is due to method failure, and, in such a case, uses the composed response prototype. This change will allow users to substitute their ownMethodNotAllowedMiddleware
, allowing for templated solutions, or to omit it entirely, allowing fallback to the 404 handler.Second, the
RouteMiddleware
was modified as follows:These changes mean the middleware does only what it says (routing).
Third, the
Route
class was modified to implement the PSR-15MiddlewareInterface
. When processed, it proxies to the composed middleware instance.Fourth, the
RouteResult
was modified to implement the PSR-15MiddlewareInterface
. When processed, if it is a route failure, it delegates to the provided handler. If it is a successful route match, it proxies to the matchedRoute
instance. ThegetMatchedMiddleware()
method was removed, as it's essentially unnecessary; processing the route result accomplishes the same thing.Fifth, the
DispatchMiddleware
was modified to process theRouteResult
instance pulled from the request, instead of retrieving its composed middleware to process.