Skip to content

Commit

Permalink
Allow router to start without routes, require logger
Browse files Browse the repository at this point in the history 
Fixes #7.
  • Loading branch information
@kelunik
kelunik committed Aug 5, 2023
1 parent 565eeb2 commit 2aa4f17
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 33 deletions.
3 changes: 2 additions & 1 deletion composer.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@
"amphp/php-cs-fixer-config": "^2",
"league/uri": "^6",
"phpunit/phpunit": "^9",
"psalm/phar": "^5.6"
"psalm/phar": "^5.6",
"colinodell/psr-testlogger": "^1.2"
},
"minimum-stability": "beta",
"prefer-stable": true,
Expand Down
2 changes: 1 addition & 1 deletion examples/hello-world.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@

$errorHandler = new DefaultErrorHandler();

$router = new Router($server, $errorHandler);
$router = new Router($server, $logger, $errorHandler);
$router->addRoute('GET', '/', new ClosureRequestHandler(function () {
return new Response(HttpStatus::OK, ['content-type' => 'text/plain'], 'Hello, world!');
}));
Expand Down
23 changes: 17 additions & 6 deletions src/Router.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
use Amp\Http\Server\RequestHandler\ClosureRequestHandler;
use FastRoute\Dispatcher;
use FastRoute\RouteCollector;
use Psr\Log\LoggerInterface;
use function Amp\Http\Server\Middleware\stackMiddleware;
use function FastRoute\simpleDispatcher;

Expand Down Expand Up @@ -41,6 +42,7 @@ final class Router implements RequestHandler
*/
public function __construct(
HttpServer $httpServer,
private readonly LoggerInterface $logger,
private readonly ErrorHandler $errorHandler,
int $cacheSize = self::DEFAULT_CACHE_SIZE,
) {
Expand All @@ -60,10 +62,18 @@ public function __construct(
*/
public function handleRequest(Request $request): Response
{
if (!$this->routeDispatcher) {
if (!$this->running) {
throw new \Error('HTTP server has not been started so the router has not been built');
}

if (!$this->routeDispatcher) {
if ($this->fallback !== null) {
return $this->fallback->handleRequest($request);
}

return $this->notFound($request);
}

$method = $request->getMethod();
$path = \rawurldecode($request->getUri()->getPath());

Expand Down Expand Up @@ -248,12 +258,13 @@ private function onStart(): void
throw new \Error("Router already started");
}

if (empty($this->routes)) {
throw new \Error("Router start failure: no routes registered");
}

$this->running = true;

if (!$this->routes) {
$this->logger->notice("No routes registered");
return;
}

$this->routeDispatcher = simpleDispatcher(function (RouteCollector $rc): void {
$redirectHandler = new ClosureRequestHandler(static function (Request $request): Response {
$uri = $request->getUri();
Expand Down Expand Up @@ -295,7 +306,7 @@ private function onStart(): void

private function onStop(): void
{
unset($this->routeDispatcher);
$this->routeDispatcher = null;
$this->running = false;
}
}
50 changes: 25 additions & 25 deletions test/RouterTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,31 +3,32 @@
namespace Amp\Http\Server;

use Amp\ByteStream\Payload;
use Amp\CompositeException;
use Amp\Http\HttpStatus;
use Amp\Http\Server\Driver\Client;
use Amp\Http\Server\RequestHandler\ClosureRequestHandler;
use Amp\Socket\InternetAddress;
use ColinODell\PsrTestLogger\TestLogger;
use League\Uri;
use PHPUnit\Framework\TestCase;
use Psr\Log\NullLogger;

class RouterTest extends TestCase
{
private SocketHttpServer $server;
private ErrorHandler $errorHandler;
private TestLogger $testLogger;

protected function setUp(): void
{
parent::setUp();

$this->testLogger = new TestLogger();
$this->server = $this->createMockServer();
$this->errorHandler = $this->createErrorHandler();
}

protected function createMockServer(): SocketHttpServer
{
$server = SocketHttpServer::createForDirectAccess(new NullLogger);
$server = SocketHttpServer::createForDirectAccess($this->testLogger);
$server->expose(new InternetAddress('127.0.0.1', 0));
return $server;
}
Expand All @@ -41,32 +42,31 @@ public function testThrowsOnInvalidCacheSize(): void
{
$this->expectException(\Error::class);

new Router($this->createMockServer(), $this->createErrorHandler(), 0);
new Router($this->server, $this->testLogger, $this->errorHandler, 0);
}

public function testRouteThrowsOnEmptyMethodString(): void
{
$this->expectException(\Error::class);
$this->expectExceptionMessage('Amp\Http\Server\Router::addRoute() requires a non-empty string HTTP method at Argument 1');

$router = new Router($this->createMockServer(), $this->createErrorHandler());
$router = new Router($this->server, $this->testLogger, $this->errorHandler);
$router->addRoute("", "/uri", new ClosureRequestHandler(function () {
}));
}

public function testUpdateFailsIfStartedWithoutAnyRoutes(): void
public function testLogsMessageWithoutAnyRoutes(): void
{
$router = new Router($this->server, $this->errorHandler);

$this->expectException(CompositeException::class);
$this->expectExceptionMessage("Router start failure: no routes registered");
$router = new Router($this->server, $this->testLogger, $this->errorHandler);

$this->server->start($router, $this->errorHandler);

self::assertTrue($this->testLogger->hasNotice('No routes registered'));
}

public function testUseCanonicalRedirector(): void
{
$router = new Router($this->server, $this->errorHandler);
$router = new Router($this->server, $this->testLogger, $this->errorHandler);
$router->addRoute("GET", "/{name}/{age}/?", new ClosureRequestHandler(function (Request $req) use (&$routeArgs) {
$routeArgs = $req->getAttribute(Router::class);
return new Response;
Expand All @@ -93,7 +93,7 @@ public function testUseCanonicalRedirector(): void

public function testMultiplePrefixes(): void
{
$router = new Router($this->server, $this->errorHandler);
$router = new Router($this->server, $this->testLogger, $this->errorHandler);
$router->addRoute("GET", "{name}", new ClosureRequestHandler(function (Request $req) use (&$routeArgs) {
$routeArgs = $req->getAttribute(Router::class);
return new Response;
Expand All @@ -112,7 +112,7 @@ public function testMultiplePrefixes(): void

public function testStack(): void
{
$router = new Router($this->server, $this->errorHandler);
$router = new Router($this->server, $this->testLogger, $this->errorHandler);
$router->addRoute("GET", "/", new ClosureRequestHandler(function (Request $req) {
return new Response(HttpStatus::OK, [], $req->getAttribute("stack"));
}));
Expand Down Expand Up @@ -143,7 +143,7 @@ public function handleRequest(Request $request, RequestHandler $requestHandler):

public function testStackMultipleCalls(): void
{
$router = new Router($this->server, $this->errorHandler);
$router = new Router($this->server, $this->testLogger, $this->errorHandler);
$router->addRoute("GET", "/", new ClosureRequestHandler(function (Request $req) {
return new Response(HttpStatus::OK, [], $req->getAttribute("stack"));
}));
Expand Down Expand Up @@ -180,11 +180,11 @@ public function testMerge(): void
return new Response(HttpStatus::OK, [], $req->getUri()->getPath());
});

$routerA = new Router($this->server, $this->createErrorHandler());
$routerA = new Router($this->server, $this->testLogger, $this->createErrorHandler());
$routerA->prefix("a");
$routerA->addRoute("GET", "{name}", $requestHandler);

$routerB = new Router($this->server, $this->createErrorHandler());
$routerB = new Router($this->server, $this->testLogger, $this->createErrorHandler());
$routerB->prefix("b");
$routerB->addRoute("GET", "{name}", $requestHandler);

Expand All @@ -211,7 +211,7 @@ public function testPathIsMatchedDecoded(): void
return new Response(HttpStatus::OK);
});

$router = new Router($this->server, $this->errorHandler);
$router = new Router($this->server, $this->testLogger, $this->errorHandler);
$router->addRoute("GET", "/fo+ö", $requestHandler);

$this->server->start($router, $this->errorHandler);
Expand All @@ -233,7 +233,7 @@ public function testFallbackInvokedOnNotFoundRoute(): void
return new Response(HttpStatus::NO_CONTENT);
});

$router = new Router($this->server, $this->errorHandler);
$router = new Router($this->server, $this->testLogger, $this->errorHandler);
$router->addRoute("GET", "/foo/{name}", $requestHandler);
$router->setFallback($fallback);

Expand All @@ -250,7 +250,7 @@ public function testNonAllowedMethod(): void
return new Response(HttpStatus::OK);
});

$router = new Router($this->server, $this->errorHandler);
$router = new Router($this->server, $this->testLogger, $this->errorHandler);
$router->addRoute("GET", "/foo/{name}", $requestHandler);
$router->addRoute("DELETE", "/foo/{name}", $requestHandler);

Expand All @@ -270,14 +270,14 @@ public function testMergeAfterStart(): void

$mockServer = $this->createMockServer();

$router = new Router($this->server, $this->errorHandler);
$router = new Router($this->server, $this->testLogger, $this->errorHandler);
$router->addRoute("GET", "/foo/{name}", $requestHandler);

$this->server->start($router, $this->errorHandler);

$this->expectException(\Error::class);
$this->expectExceptionMessage('Cannot merge routers after');
$router->merge(new Router($mockServer, $this->createErrorHandler()));
$router->merge(new Router($mockServer, $this->testLogger, $this->createErrorHandler()));
}

public function testPrefixAfterStart(): void
Expand All @@ -286,7 +286,7 @@ public function testPrefixAfterStart(): void
return new Response(HttpStatus::OK);
});

$router = new Router($this->server, $this->errorHandler);
$router = new Router($this->server, $this->testLogger, $this->errorHandler);
$router->addRoute("GET", "/foo/{name}", $requestHandler);

$this->server->start($router, $this->errorHandler);
Expand All @@ -302,7 +302,7 @@ public function testAddRouteAfterStart(): void
return new Response(HttpStatus::OK);
});

$router = new Router($this->server, $this->errorHandler);
$router = new Router($this->server, $this->testLogger, $this->errorHandler);
$router->addRoute("GET", "/foo/{name}", $requestHandler);

$this->server->start($router, $this->errorHandler);
Expand All @@ -318,7 +318,7 @@ public function testStackAfterStart(): void
return new Response(HttpStatus::OK);
});

$router = new Router($this->server, $this->errorHandler);
$router = new Router($this->server, $this->testLogger, $this->errorHandler);
$router->addRoute("GET", "/foo/{name}", $requestHandler);

$this->server->start($router, $this->errorHandler);
Expand All @@ -334,7 +334,7 @@ public function testSetFallbackAfterStart(): void
return new Response(HttpStatus::OK);
});

$router = new Router($this->server, $this->errorHandler);
$router = new Router($this->server, $this->testLogger, $this->errorHandler);
$router->addRoute("GET", "/foo/{name}", $requestHandler);

$this->server->start($router, $this->errorHandler);
Expand Down

0 comments on commit 2aa4f17

Please sign in to comment.