Skip to content

Commit

Permalink
Update for Responder → RequestHandler
Browse files Browse the repository at this point in the history
  • Loading branch information
@trowski
trowski committed Mar 19, 2018
1 parent 5bdf5d9 commit 3fd3e2e
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 56 deletions.
52 changes: 27 additions & 25 deletions src/Router.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace Amp\Http\Server;

use Amp\Failure;
use Amp\Http\Server\RequestHandler\CallableRequestHandler;
use Amp\Http\Status;
use Amp\Promise;
use cash\LRUCache;
Expand All @@ -11,7 +12,7 @@
use function Amp\call;
use function FastRoute\simpleDispatcher;

final class Router implements Responder, ServerObserver {
final class Router implements RequestHandler, ServerObserver {
const DEFAULT_CACHE_SIZE = 512;

/** @var bool */
Expand All @@ -23,7 +24,7 @@ final class Router implements Responder, ServerObserver {
/** @var ErrorHandler */
private $errorHandler;

/** @var Responder|null */
/** @var RequestHandler|null */
private $fallback;

/** @var \SplObjectStorage */
Expand Down Expand Up @@ -62,7 +63,7 @@ public function __construct(int $cacheSize = self::DEFAULT_CACHE_SIZE) {
*
* @return Promise<\Amp\Http\Server\Response>
*/
public function respond(Request $request): Promise {
public function handleRequest(Request $request): Promise {
$method = $request->getMethod();
$path = $request->getUri()->getPath();

Expand All @@ -76,17 +77,17 @@ public function respond(Request $request): Promise {
switch ($match[0]) {
case Dispatcher::FOUND:
/**
* @var Responder $responder
* @var RequestHandler $requestHandler
* @var string[] $routeArgs
*/
list(, $responder, $routeArgs) = $match;
list(, $requestHandler, $routeArgs) = $match;
$request->setAttribute(self::class, $routeArgs);

return $responder->respond($request);
return $requestHandler->handleRequest($request);

case Dispatcher::NOT_FOUND:
if ($this->fallback !== null) {
return $this->fallback->respond($request);
return $this->fallback->handleRequest($request);
}

return $this->makeNotFoundResponse($request);
Expand Down Expand Up @@ -174,7 +175,7 @@ public function prefix(string $prefix) {
/**
* Define an application route.
*
* Matched URI route arguments are made available to responders as a request attribute
* Matched URI route arguments are made available to request handlers as a request attribute
* which may be accessed with:
*
* $request->getAttribute(Router::class)
Expand All @@ -185,12 +186,12 @@ public function prefix(string $prefix) {
*
* @param string $method The HTTP method verb for which this route applies.
* @param string $uri The string URI.
* @param Responder $responder Responder invoked on a route match.
* @param RequestHandler $requestHandler Request handler invoked on a route match.
* @param Middleware[] ...$middlewares
*
* @throws \Error If the server has started, or if $method is empty.
*/
public function addRoute(string $method, string $uri, Responder $responder, Middleware ...$middlewares) {
public function addRoute(string $method, string $uri, RequestHandler $requestHandler, Middleware ...$middlewares) {
if ($this->running) {
throw new \Error(
"Cannot add routes once the server has started"
Expand All @@ -203,8 +204,8 @@ public function addRoute(string $method, string $uri, Responder $responder, Midd
);
}

if ($responder instanceof ServerObserver) {
$this->observers->attach($responder);
if ($requestHandler instanceof ServerObserver) {
$this->observers->attach($requestHandler);
}

if (!empty($middlewares)) {
Expand All @@ -214,14 +215,15 @@ public function addRoute(string $method, string $uri, Responder $responder, Midd
}
}

$responder = Middleware\stack($responder, ...$middlewares);
$requestHandler = Middleware\stack($requestHandler, ...$middlewares);
}

$this->routes[] = [$method, \ltrim($uri, "/"), $responder];
$this->routes[] = [$method, \ltrim($uri, "/"), $requestHandler];
}

/**
* Specifies a set of middlewares that is applied to every route, but will not be applied to the fallback responder.
* Specifies a set of middlewares that is applied to every route, but will not be applied to the fallback request
* handler.
*
* All middlewares are called in the order they're passed, so the first middleware is the outer middleware.
*
Expand All @@ -241,20 +243,20 @@ public function stack(Middleware ...$middlewares) {
}

/**
* Specifies an instance of Responder that is used if no routes match.
* Specifies an instance of RequestHandler that is used if no routes match.
*
* If no fallback is given, a 404 response is returned from `respond()` when no matching routes are found.
*
* @param Responder $responder
* @param RequestHandler $requestHandler
*
* @throws \Error If the server has started.
*/
public function setFallback(Responder $responder) {
public function setFallback(RequestHandler $requestHandler) {
if ($this->running) {
throw new \Error("Cannot add fallback responder after the server has started");
throw new \Error("Cannot add fallback request handler after the server has started");
}

$this->fallback = $responder;
$this->fallback = $requestHandler;
}

public function onStart(Server $server): Promise {
Expand All @@ -275,14 +277,14 @@ public function onStart(Server $server): Promise {
$logger = $server->getLogger();

$this->routeDispatcher = simpleDispatcher(function (RouteCollector $rc) use ($allowedMethods, $logger) {
foreach ($this->routes as list($method, $uri, $responder)) {
foreach ($this->routes as list($method, $uri, $requestHandler)) {
if (!\in_array($method, $allowedMethods, true)) {
$logger->alert(
"Router URI '$uri' uses method '$method' that is not in the list of allowed methods"
);
}

$responder = Middleware\stack($responder, ...$this->middlewares);
$requestHandler = Middleware\stack($requestHandler, ...$this->middlewares);
$uri = $this->prefix . $uri;

// Special-case, otherwise we redirect just to the same URI again
Expand All @@ -294,8 +296,8 @@ public function onStart(Server $server): Promise {
$canonicalUri = \substr($uri, 0, -2);
$redirectUri = \substr($uri, 0, -1);

$rc->addRoute($method, $canonicalUri, $responder);
$rc->addRoute($method, $redirectUri, new CallableResponder(static function (Request $request): Response {
$rc->addRoute($method, $canonicalUri, $requestHandler);
$rc->addRoute($method, $redirectUri, new CallableRequestHandler(static function (Request $request): Response {
$uri = $request->getUri();
$path = \rtrim($uri->getPath(), '/');

Expand All @@ -311,7 +313,7 @@ public function onStart(Server $server): Promise {
], "Canonical resource location: {$path}");
}));
} else {
$rc->addRoute($method, $uri, $responder);
$rc->addRoute($method, $uri, $requestHandler);
}
}
});
Expand Down
64 changes: 33 additions & 31 deletions test/RouterTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,15 @@

namespace Amp\Http\Server\Router\Test;

use Amp\ByteStream\Message;
use Amp\ByteStream\Payload;
use Amp\Failure;
use Amp\Http\Server\CallableResponder;
use Amp\Http\Server\Client;
use Amp\Http\Server\RequestHandler\CallableRequestHandler;
use Amp\Http\Server\Driver\Client;
use Amp\Http\Server\DefaultErrorHandler;
use Amp\Http\Server\Middleware;
use Amp\Http\Server\Options;
use Amp\Http\Server\Request;
use Amp\Http\Server\Responder;
use Amp\Http\Server\RequestHandler;
use Amp\Http\Server\Response;
use Amp\Http\Server\Router;
use Amp\Http\Server\Server;
Expand All @@ -25,7 +25,7 @@ public function mockServer(): Server {
$options = new Options;

$mock = $this->getMockBuilder(Server::class)
->setConstructorArgs([$this->createMock(Responder::class), $options, $this->createMock(PsrLogger::class)])
->setConstructorArgs([[], $this->createMock(RequestHandler::class), $options, $this->createMock(PsrLogger::class)])
->getMock();

$mock->method("getOptions")
Expand All @@ -43,7 +43,7 @@ public function mockServer(): Server {
*/
public function testRouteThrowsOnEmptyMethodString() {
$router = new Router;
$router->addRoute("", "/uri", new CallableResponder(function () {}));
$router->addRoute("", "/uri", new CallableRequestHandler(function () {}));
}

public function testUpdateFailsIfStartedWithoutAnyRoutes() {
Expand All @@ -62,7 +62,7 @@ public function testUpdateFailsIfStartedWithoutAnyRoutes() {

public function testUseCanonicalRedirector() {
$router = new Router;
$router->addRoute("GET", "/{name}/{age}/?", new CallableResponder(function (Request $req) use (&$routeArgs) {
$router->addRoute("GET", "/{name}/{age}/?", new CallableRequestHandler(function (Request $req) use (&$routeArgs) {
$routeArgs = $req->getAttribute(Router::class);
return new Response;
}));
Expand All @@ -73,22 +73,22 @@ public function testUseCanonicalRedirector() {
// Test that response is redirection
$request = new Request($this->createMock(Client::class), "GET", Uri\Http::createFromString("/amphp/bob/19/"));
/** @var \Amp\Http\Server\Response $response */
$response = Promise\wait($router->respond($request));
$response = Promise\wait($router->handleRequest($request));

$this->assertEquals(Status::PERMANENT_REDIRECT, $response->getStatus());
$this->assertEquals("/amphp/bob/19", $response->getHeader("location"));

// Test that response is handled and no redirection
$request = new Request($this->createMock(Client::class), "GET", Uri\Http::createFromString("/amphp/bob/19"));
$response = Promise\wait($router->respond($request));
$response = Promise\wait($router->handleRequest($request));

$this->assertEquals(Status::OK, $response->getStatus());
$this->assertSame(["name" => "bob", "age" => "19"], $routeArgs);
}

public function testMultiplePrefixes() {
$router = new Router;
$router->addRoute("GET", "{name}", new CallableResponder(function (Request $req) use (&$routeArgs) {
$router->addRoute("GET", "{name}", new CallableRequestHandler(function (Request $req) use (&$routeArgs) {
$routeArgs = $req->getAttribute(Router::class);
return new Response;
}));
Expand All @@ -99,98 +99,100 @@ public function testMultiplePrefixes() {

$request = new Request($this->createMock(Client::class), "GET", Uri\Http::createFromString("/github/amphp/bob"));
/** @var \Amp\Http\Server\Response $response */
$response = Promise\wait($router->respond($request));
$response = Promise\wait($router->handleRequest($request));

$this->assertEquals(Status::OK, $response->getStatus());
$this->assertSame(["name" => "bob"], $routeArgs);
}

public function testStack() {
$router = new Router;
$router->addRoute("GET", "/", new CallableResponder(function (Request $req) {
$router->addRoute("GET", "/", new CallableRequestHandler(function (Request $req) {
return new Response(Status::OK, [], $req->getAttribute("stack"));
}));

$router->stack(new class implements Middleware {
public function process(Request $request, Responder $responder): Promise {
public function handleRequest(Request $request, RequestHandler $RequestHandler): Promise {
$request->setAttribute("stack", "a");
return $responder->respond($request);
return $RequestHandler->handleRequest($request);
}
}, new class implements Middleware {
public function process(Request $request, Responder $responder): Promise {
public function handleRequest(Request $request, RequestHandler $RequestHandler): Promise {
$request->setAttribute("stack", $request->getAttribute("stack") . "b");
return $responder->respond($request);
return $RequestHandler->handleRequest($request);
}
});

Promise\wait($router->onStart($this->mockServer()));

$request = new Request($this->createMock(Client::class), "GET", Uri\Http::createFromString("/"));
/** @var \Amp\Http\Server\Response $response */
$response = Promise\wait($router->respond($request));
$response = Promise\wait($router->handleRequest($request));

$this->assertEquals(Status::OK, $response->getStatus());
$this->assertSame("ab", Promise\wait(new Message($response->getBody())));
$payload = new Payload($response->getBody());
$this->assertSame("ab", Promise\wait($payload->buffer()));
}

public function testStackMultipleCalls() {
$router = new Router;
$router->addRoute("GET", "/", new CallableResponder(function (Request $req) {
$router->addRoute("GET", "/", new CallableRequestHandler(function (Request $req) {
return new Response(Status::OK, [], $req->getAttribute("stack"));
}));

$router->stack(new class implements Middleware {
public function process(Request $request, Responder $responder): Promise {
public function handleRequest(Request $request, RequestHandler $RequestHandler): Promise {
$request->setAttribute("stack", $request->getAttribute("stack") . "b");
return $responder->respond($request);
return $RequestHandler->handleRequest($request);
}
});

$router->stack(new class implements Middleware {
public function process(Request $request, Responder $responder): Promise {
public function handleRequest(Request $request, RequestHandler $RequestHandler): Promise {
$request->setAttribute("stack", "a");
return $responder->respond($request);
return $RequestHandler->handleRequest($request);
}
});

Promise\wait($router->onStart($this->mockServer()));

$request = new Request($this->createMock(Client::class), "GET", Uri\Http::createFromString("/"));
/** @var \Amp\Http\Server\Response $response */
$response = Promise\wait($router->respond($request));
$response = Promise\wait($router->handleRequest($request));

$this->assertEquals(Status::OK, $response->getStatus());
$this->assertSame("ab", Promise\wait(new Message($response->getBody())));
$payload = new Payload($response->getBody());
$this->assertSame("ab", Promise\wait($payload->buffer()));
}

public function testMerge() {
$responder = new CallableResponder(function (Request $req) {
$RequestHandler = new CallableRequestHandler(function (Request $req) {

This comment has been minimized.

Sign in to view

Copy link
@kelunik

kelunik Mar 19, 2018

Member

Should be $requestHandler.

This comment has been minimized.

Sign in to view

Copy link
@trowski

trowski Mar 19, 2018

Author Member

Yeah, find-and-replace fail.
return new Response(Status::OK, [], $req->getUri()->getPath());
});

$routerA = new Router;
$routerA->prefix("a");
$routerA->addRoute("GET", "{name}", $responder);
$routerA->addRoute("GET", "{name}", $RequestHandler);

$routerB = new Router;
$routerB->prefix("b");
$routerB->addRoute("GET", "{name}", $responder);
$routerB->addRoute("GET", "{name}", $RequestHandler);

$routerA->merge($routerB);

Promise\wait($routerA->onStart($this->mockServer()));

/** @var \Amp\Http\Server\Response $response */
$request = new Request($this->createMock(Client::class), "GET", Uri\Http::createFromString("/a/bob"));
$response = Promise\wait($routerA->respond($request));
$response = Promise\wait($routerA->handleRequest($request));
$this->assertEquals(Status::OK, $response->getStatus());

$request = new Request($this->createMock(Client::class), "GET", Uri\Http::createFromString("/a/b/bob"));
$response = Promise\wait($routerA->respond($request));
$response = Promise\wait($routerA->handleRequest($request));
$this->assertEquals(Status::OK, $response->getStatus());

$request = new Request($this->createMock(Client::class), "GET", Uri\Http::createFromString("/b/bob"));
$response = Promise\wait($routerA->respond($request));
$response = Promise\wait($routerA->handleRequest($request));
$this->assertEquals(Status::NOT_FOUND, $response->getStatus());
}
}

0 comments on commit 3fd3e2e

Please sign in to comment.