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

Refactor routing and dispatch logic #48

Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions src/DispatchMiddleware.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,7 @@
* Checks for a composed route result in the request. If none is provided,
* delegates request processing to the handler.
*
* Otherwise, it pulls the middleware from the route result and processes it
* with the provided request and handler.
* Otherwise, it delegates processing to the route result.
*/
class DispatchMiddleware implements MiddlewareInterface
{
Expand All @@ -32,7 +31,6 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface
return $handler->handle($request);
}

$middleware = $routeResult->getMatchedMiddleware();
return $middleware->process($request, $handler);
return $routeResult->process($request, $handler);
}
}
54 changes: 54 additions & 0 deletions src/MethodNotAllowedMiddleware.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
<?php
/**
* @see https://github.com/zendframework/zend-expressive-router for the canonical source repository
* @copyright Copyright (c) 2018 Zend Technologies USA Inc. (https://www.zend.com)
* @license https://github.com/zendframework/zend-expressive-router/blob/master/LICENSE.md New BSD License
*/

declare(strict_types=1);

namespace Zend\Expressive\Router;

use Fig\Http\Message\StatusCodeInterface as StatusCode;
use Psr\Http\Message\ResponseInterface;
use Psr\Http\Message\ServerRequestInterface;
use Psr\Http\Server\MiddlewareInterface;
use Psr\Http\Server\RequestHandlerInterface;

/**
* Emit a 405 Method Not Allowed response
*
* If the request composes a route result, and the route result represents a
* failure due to request method, this middleware will emit a 405 response,
* along with an Allow header indicating allowed methods, as reported by the
* route result.
*
* If no route result is composed, and/or it's not the result of a method
* failure, it passes handling to the provided handler.
*/
class MethodNotAllowedMiddleware implements MiddlewareInterface
{
/**
* Response prototype for 405 responses.
*
* @var ResponseInterface
*/
private $responsePrototype;

public function __construct(ResponseInterface $responsePrototype)
{
$this->responsePrototype = $responsePrototype;
}

public function process(ServerRequestInterface $request, RequestHandlerInterface $handler) : ResponseInterface
{
$routeResult = $request->getAttribute(RouteResult::class, false);
Copy link
Member

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).

if (! $routeResult || ! $routeResult->isMethodFailure()) {
return $handler->handle($request);
}

return $this->responsePrototype
->withStatus(StatusCode::STATUS_METHOD_NOT_ALLOWED)
->withHeader('Allow', implode(',', $routeResult->getAllowedMethods()));
}
}
13 changes: 12 additions & 1 deletion src/Route.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,10 @@
namespace Zend\Expressive\Router;

use Fig\Http\Message\RequestMethodInterface as RequestMethod;
use Psr\Http\Message\ResponseInterface;
use Psr\Http\Message\ServerRequestInterface;
use Psr\Http\Server\MiddlewareInterface;
use Psr\Http\Server\RequestHandlerInterface;

/**
* Value object representing a single route.
Expand All @@ -27,7 +30,7 @@
* be provided after instantiation via the "options" property and related
* setOptions() method.
*/
class Route
class Route implements MiddlewareInterface
{
public const HTTP_METHOD_ANY = null;
public const HTTP_METHOD_SEPARATOR = ':';
Expand Down Expand Up @@ -98,6 +101,14 @@ public function __construct(
&& ! in_array(RequestMethod::METHOD_OPTIONS, $this->methods, true);
}

/**
* Proxies to the middleware composed during instantiation.
*/
public function process(ServerRequestInterface $request, RequestHandlerInterface $handler) : ResponseInterface
{
return $this->middleware->process($request, $handler);
}

public function getPath() : string
{
return $this->path;
Expand Down
39 changes: 11 additions & 28 deletions src/RouteMiddleware.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@

namespace Zend\Expressive\Router;

use Fig\Http\Message\StatusCodeInterface as StatusCode;
use Psr\Http\Message\ResponseInterface;
use Psr\Http\Message\ServerRequestInterface;
use Psr\Http\Server\MiddlewareInterface;
Expand All @@ -18,52 +17,36 @@
/**
* Default routing middleware.
*
* Uses the composed router to match against the incoming request.
* Uses the composed router to match against the incoming request, and
* injects the request passed to the handler with the `RouteResult` instance
* returned (using the `RouteResult` class name as the attribute name).
*
* When routing failure occurs, if the failure is due to HTTP method, uses
* the composed response prototype to generate a 405 response; otherwise,
* it delegates to the next middleware.
*
* If routing succeeds, injects the route result into the request (under the
* RouteResult class name), as well as any matched parameters, before
* delegating to the next middleware.
* If routing succeeds, injects the request passed to the handler with any
* matched parameters as well.
*/
class RouteMiddleware implements MiddlewareInterface
{
/**
* Response prototype for 405 responses.
*
* @var ResponseInterface
*/
private $responsePrototype;

/**
* @var RouterInterface
*/
protected $router;

public function __construct(RouterInterface $router, ResponseInterface $responsePrototype)
public function __construct(RouterInterface $router)
{
$this->router = $router;
$this->responsePrototype = $responsePrototype;
}

public function process(ServerRequestInterface $request, RequestHandlerInterface $handler) : ResponseInterface
{
$result = $this->router->match($request);

if ($result->isFailure()) {
if ($result->isMethodFailure()) {
return $this->responsePrototype->withStatus(StatusCode::STATUS_METHOD_NOT_ALLOWED)
->withHeader('Allow', implode(',', $result->getAllowedMethods()));
}
return $handler->handle($request);
}

// Inject the actual route result, as well as individual matched parameters.
$request = $request->withAttribute(RouteResult::class, $result);
foreach ($result->getMatchedParams() as $param => $value) {
$request = $request->withAttribute($param, $value);

if ($result->isSuccess()) {
foreach ($result->getMatchedParams() as $param => $value) {
$request = $request->withAttribute($param, $value);
}
}

return $handler->handle($request);
Expand Down
41 changes: 21 additions & 20 deletions src/RouteResult.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@

namespace Zend\Expressive\Router;

use Psr\Http\Message\ResponseInterface;
use Psr\Http\Message\ServerRequestInterface;
use Psr\Http\Server\MiddlewareInterface;
use Psr\Http\Server\RequestHandlerInterface;

/**
* Value object representing the results of routing.
Expand All @@ -31,7 +34,7 @@
* RouteResult instances are consumed by the Application in the routing
* middleware.
*/
class RouteResult
class RouteResult implements MiddlewareInterface
{
/**
* @var array
Expand Down Expand Up @@ -102,6 +105,23 @@ public static function fromRouteFailure(?array $methods) : self
return $result;
}

/**
* Process the result as middleware.
*
* If the result represents a failure, it passes handling to the handler.
*
* Otherwise, it processes the composed middleware using the provide request
* and handler.
*/
public function process(ServerRequestInterface $request, RequestHandlerInterface $handler) : ResponseInterface
{
if ($this->isFailure()) {
return $handler->handle($request);
}

return $this->getMatchedRoute()->process($request, $handler);
}

/**
* Does the result represent successful routing?
*/
Expand Down Expand Up @@ -142,25 +162,6 @@ public function getMatchedRouteName()
return $this->matchedRouteName;
}

/**
* Retrieve the matched middleware, if possible.
*
* @return false|MiddlewareInterface Returns false if the result
* represents a failure; otherwise, a MiddlewareInterface instance.
*/
public function getMatchedMiddleware()
{
if ($this->isFailure()) {
return false;
}

if (! $this->matchedMiddleware && $this->route) {
$this->matchedMiddleware = $this->route->getMiddleware();
}

return $this->matchedMiddleware;
}

/**
* Returns the matched params.
*
Expand Down
21 changes: 11 additions & 10 deletions test/DispatchMiddlewareTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@
namespace ZendTest\Expressive\Router;

use PHPUnit\Framework\TestCase;
use Prophecy\Argument;
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;
Copy link
Member

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.

Expand Down Expand Up @@ -51,18 +51,19 @@ public function testInvokesHandlerIfRequestDoesNotContainRouteResult()
$this->assertSame($this->response, $response);
}

public function testInvokesMatchedMiddlewareWhenRouteResult()
public function testInvokesRouteResultWhenPresent()
{
$this->handler->handle()->shouldNotBeCalled();

$routedMiddleware = $this->prophesize(MiddlewareInterface::class);
$routedMiddleware
->process($this->request->reveal(), $this->handler->reveal())
$this->handler->handle(Argument::any())->shouldNotBeCalled();

$routeResult = $this->prophesize(RouteResult::class);
$routeResult
->process(
Argument::that([$this->request, 'reveal']),
Argument::that([$this->handler, 'reveal'])
)
->willReturn($this->response);

$routeResult = RouteResult::fromRoute(new Route('/', $routedMiddleware->reveal()));

$this->request->getAttribute(RouteResult::class, false)->willReturn($routeResult);
$this->request->getAttribute(RouteResult::class, false)->will([$routeResult, 'reveal']);

$response = $this->middleware->process($this->request->reveal(), $this->handler->reveal());

Expand Down
93 changes: 93 additions & 0 deletions test/MethodNotAllowedMiddlewareTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
<?php
/**
* @see https://github.com/zendframework/zend-expressive-router for the canonical source repository
* @copyright Copyright (c) 2018 Zend Technologies USA Inc. (https://www.zend.com)
* @license https://github.com/zendframework/zend-expressive-router/blob/master/LICENSE.md New BSD License
*/

declare(strict_types=1);

namespace ZendTest\Expressive\Router;

use Fig\Http\Message\StatusCodeInterface as StatusCode;
use PHPUnit\Framework\TestCase;
use Prophecy\Argument;
use Prophecy\Prophecy\ObjectProphecy;
use Psr\Http\Message\ResponseInterface;
use Psr\Http\Message\ServerRequestInterface;
use Psr\Http\Server\MiddlewareInterface;
Copy link
Member

Choose a reason for hiding this comment

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

Unused.

use Psr\Http\Server\RequestHandlerInterface;
use Zend\Expressive\Router\MethodNotAllowedMiddleware;
use Zend\Expressive\Router\RouteResult;

class MethodNotAllowedMiddlewareTest extends TestCase
{
/** @var RequestHandlerInterface|ObjectProphecy */
private $handler;

/** @var MethodNotAllowedMiddleware */
private $middleware;

/** @var ServerRequestInterface|ObjectProphecy */
private $request;

/** @var ResponseInterface|ObjectProphecy */
private $response;

public function setUp()
{
$this->handler = $this->prophesize(RequestHandlerInterface::class);
$this->request = $this->prophesize(ServerRequestInterface::class);
$this->response = $this->prophesize(ResponseInterface::class);
$this->middleware = new MethodNotAllowedMiddleware($this->response->reveal());
}

public function testDelegatesToHandlerIfNoRouteResultPresentInRequest()
{
$this->request->getAttribute(RouteResult::class, false)->willReturn(false);
$this->handler->handle(Argument::that([$this->request, 'reveal']))->will([$this->response, 'reveal']);

$this->response->withStatus(Argument::any())->shouldNotBeCalled();
$this->response->withHeader('Allow', Argument::any())->shouldNotBeCalled();

$this->assertSame(
$this->response->reveal(),
$this->middleware->process($this->request->reveal(), $this->handler->reveal())
);
}

public function testDelegatesToHandlerIfRouteResultNotAMethodFailure()
{
$result = $this->prophesize(RouteResult::class);
$result->isMethodFailure()->willReturn(false);

$this->request->getAttribute(RouteResult::class, false)->will([$result, 'reveal']);
$this->handler->handle(Argument::that([$this->request, 'reveal']))->will([$this->response, 'reveal']);

$this->response->withStatus(Argument::any())->shouldNotBeCalled();
$this->response->withHeader('Allow', Argument::any())->shouldNotBeCalled();

$this->assertSame(
$this->response->reveal(),
$this->middleware->process($this->request->reveal(), $this->handler->reveal())
);
}

public function testReturns405ResponseWithAllowHeaderIfResultDueToMethodFailure()
{
$result = $this->prophesize(RouteResult::class);
$result->isMethodFailure()->willReturn(true);
$result->getAllowedMethods()->willReturn(['GET', 'POST']);

$this->request->getAttribute(RouteResult::class, false)->will([$result, 'reveal']);
$this->handler->handle(Argument::that([$this->request, 'reveal']))->shouldNotBeCalled();

$this->response->withStatus(StatusCode::STATUS_METHOD_NOT_ALLOWED)->will([$this->response, 'reveal']);
$this->response->withHeader('Allow', 'GET,POST')->will([$this->response, 'reveal']);

$this->assertSame(
$this->response->reveal(),
$this->middleware->process($this->request->reveal(), $this->handler->reveal())
);
}
}
5 changes: 1 addition & 4 deletions test/PathBasedRoutingMiddlewareTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,7 @@ public function setUp()
{
$this->router = $this->prophesize(RouterInterface::class);
$this->response = $this->prophesize(ResponseInterface::class);
$this->middleware = new PathBasedRoutingMiddleware(
$this->router->reveal(),
$this->response->reveal()
);
$this->middleware = new PathBasedRoutingMiddleware($this->router->reveal());

$this->request = $this->prophesize(ServerRequestInterface::class);
$this->handler = $this->prophesize(RequestHandlerInterface::class);
Expand Down
Loading