Skip to content

Commit

Permalink
Require HTTP method for matching
Browse files Browse the repository at this point in the history
  • Loading branch information
Robin de Graaf committed Apr 7, 2019
1 parent 332a3fd commit 4bdddf1
Show file tree
Hide file tree
Showing 3 changed files with 90 additions and 27 deletions.
11 changes: 11 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,16 @@
# Parable Routing

## 0.2.0

_Changes_

Due to how it's possible to add a route more than once, for multiple methods, storing them by url turned out to be a problem. As such, the following methods now require an HTTP method to be passed:
- `getRoutes(string $httpMethod): Route[]`
- `getRouteByName(string $httpMethod, string $name): ?Route`
- `buildRouteUrl(string $httpMethod, string $name, array $parameters = []): string`

The unintended but welcome benefit of this is that matching parametered routes is now significantly faster, since we can immediately ignore all irrelevant HTTP methods.

## 0.1.1

_Changes_
Expand Down
41 changes: 28 additions & 13 deletions src/Router.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,30 @@
class Router
{
/**
* @var Route[]
* @var Route[][]
*/
protected $routes = [];

/**
* @var string[]
* @var string[][]
*/
protected $routeNames = [];

public function addRoute(Route $route): void
{
$this->routes[$route->getUrl()] = $route;
$this->routeNames[$route->getName()] = $route->getUrl();
foreach ($route->getHttpMethods() as $httpMethod) {
$this->addRouteForHttpMethod($httpMethod, $route);
}
}

protected function addRouteForHttpMethod(string $httpMethod, Route $route): void
{
if (!array_key_exists($httpMethod, $this->routes)) {
$this->routes[$httpMethod] = [];
}

$this->routes[$httpMethod][$route->getUrl()] = $route;
$this->routeNames[$httpMethod][$route->getName()] = $route->getUrl();
}

public function addRoutes(Route ...$routes): void
Expand All @@ -29,25 +40,25 @@ public function addRoutes(Route ...$routes): void
}
}

public function getRoutes(): array
public function getRoutes(string $httpMethod): array
{
return $this->routes;
return $this->routes[$httpMethod] ?? [];
}

public function getRouteByName(string $name): ?Route
public function getRouteByName(string $httpMethod, string $name): ?Route
{
$routeUrl = $this->routeNames[$name] ?? null;
$routeUrl = $this->routeNames[$httpMethod][$name] ?? null;

if ($routeUrl === null) {
return null;
}

return $this->routes[$routeUrl] ?? null;
return $this->routes[$httpMethod][$routeUrl] ?? null;
}

public function buildRouteUrl(string $name, array $parameters = []): string
public function buildRouteUrl(string $httpMethod, string $name, array $parameters = []): string
{
$route = $this->getRouteByName($name);
$route = $this->getRouteByName($httpMethod, $name);

if ($route === null) {
throw new Exception(sprintf("Route '%s' not found.", $name));
Expand Down Expand Up @@ -83,7 +94,7 @@ public function match(string $httpMethod, string $urlToMatch)

protected function matchDirect(string $httpMethod, string $urlToMatch): ?Route
{
$route = $this->routes[$urlToMatch] ?? null;
$route = $this->routes[$httpMethod][$urlToMatch] ?? null;

if ($route === null || !$route->supportsHttpMethod($httpMethod)) {
return null;
Expand All @@ -94,7 +105,11 @@ protected function matchDirect(string $httpMethod, string $urlToMatch): ?Route

protected function matchParametered(string $httpMethod, string $urlToMatch): ?Route
{
foreach ($this->routes as $routeUrl => $route) {
if (!array_key_exists($httpMethod, $this->routes)) {
return null;
}

foreach ($this->routes[$httpMethod] as $routeUrl => $route) {
if (strpos($routeUrl, '{') === false || !$route->supportsHttpMethod($httpMethod)) {
continue;
}
Expand Down
65 changes: 51 additions & 14 deletions tests/RouterTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public function setUp()

protected function setUpDefaultRoutesAndAssert()
{
self::assertCount(0, $this->router->getRoutes());
self::assertCount(0, $this->router->getRoutes('GET'));

$this->router->addRoutes(
new Route(
Expand All @@ -46,14 +46,15 @@ function ($parameter): string {
)
);

self::assertCount(3, $this->router->getRoutes());
self::assertCount(3, $this->router->getRoutes('GET'));
self::assertCount(1, $this->router->getRoutes('POST'));
}

public function testAddRouteAndGetRouteByName()
{
$this->setUpDefaultRoutesAndAssert();

$route = $this->router->getRouteByName('simple');
$route = $this->router->getRouteByName('GET', 'simple');

self::assertSame(['GET'], $route->getHttpMethods());
self::assertSame('/simple', $route->getUrl());
Expand All @@ -67,7 +68,7 @@ public function testAddRouteAndGetRouteByName()

public function testInvalidGetRouteByNameReturnsNull()
{
self::assertNull($this->router->getRouteByName('la-dee-dah'));
self::assertNull($this->router->getRouteByName('GET', 'la-dee-dah'));
}

public function testInvalidMatchReturnsNull()
Expand All @@ -77,6 +78,11 @@ public function testInvalidMatchReturnsNull()
self::assertNull($this->router->match('GET', 'la-dee-dah'));
}

public function testNoRoutesExistingReturnsNull()
{
self::assertNull((new Router())->match('GET', 'la-dee-dah'));
}

public function testInvalidMatchOnMethodReturnsNull()
{
$this->setUpDefaultRoutesAndAssert();
Expand Down Expand Up @@ -246,7 +252,7 @@ public function testBuildRouteUrl()
{
$this->setUpDefaultRoutesAndAssert();

$route = $this->router->buildRouteUrl('complex', ['id' => 2, 'name' => 'stuff']);
$route = $this->router->buildRouteUrl('GET', 'complex', ['id' => 2, 'name' => 'stuff']);
self::assertSame("/complex/2/stuff", $route);
}

Expand All @@ -255,7 +261,7 @@ public function testBuildRouteUrlThrowsOnUnknownName()
$this->expectException(Exception::class);
$this->expectExceptionMessage("Route 'nope' not found.");

$this->router->buildRouteUrl('nope', ['id' => 2, 'name' => 'stuff']);
$this->router->buildRouteUrl('GET', 'nope', ['id' => 2, 'name' => 'stuff']);
}

public function testBuildRouteUrlThrowsOnUrlWithWrongParameters()
Expand All @@ -265,7 +271,7 @@ public function testBuildRouteUrlThrowsOnUrlWithWrongParameters()
$this->expectException(Exception::class);
$this->expectExceptionMessage("Parameter 'id2' not found in url '/complex/{id}/{name}'.");

$this->router->buildRouteUrl('complex', ['id2' => 2, 'name2' => 'stuff']);
$this->router->buildRouteUrl('GET', 'complex', ['id2' => 2, 'name2' => 'stuff']);
}

public function testRouteReturnsNullOnNonExistingValueKey()
Expand All @@ -274,7 +280,7 @@ public function testRouteReturnsNullOnNonExistingValueKey()

$route = $this->router->match('GET', '/simple');

self::assertInstanceOf(\Parable\Routing\Route::class, $route);
self::assertInstanceOf(Route::class, $route);

self::assertNull($route->getParameterValue('stuff'));
}
Expand All @@ -283,29 +289,30 @@ public function testRouteBuildUrlWithOrWithoutParameters()
{
$this->setUpDefaultRoutesAndAssert();

self::assertSame('/simple', $this->router->buildRouteUrl('simple'));
self::assertSame('/simple', $this->router->buildRouteUrl('simple', []));
self::assertSame('/simple', $this->router->buildRouteUrl('simple', ['id2' => 2, 'name2' => 'stuff']));
self::assertSame('/simple', $this->router->buildRouteUrl('GET', 'simple'));
self::assertSame('/simple', $this->router->buildRouteUrl('GET', 'simple', []));
self::assertSame('/simple', $this->router->buildRouteUrl('GET', 'simple', ['id2' => 2, 'name2' => 'stuff']));
}

public function testGetRoutesReturnsCorrectNumberOfRoutes()
{
$this->setUpDefaultRoutesAndAssert();

self::assertCount(3, $this->router->getRoutes());
self::assertCount(3, $this->router->getRoutes('GET'));
self::assertCount(1, $this->router->getRoutes('POST'));
}

public function testAddMultipleRoutesDirectly()
{
self::assertCount(0, $this->router->getRoutes());
self::assertCount(0, $this->router->getRoutes('GET'));

$route1 = new Route(['GET'], 'route1', 'route1', function() {});
$route2 = new Route(['GET'], 'route2', 'route2', function() {});
$route3 = new Route(['GET'], 'route3', 'route3', function() {});

$this->router->addRoutes($route1, $route2, $route3);

self::assertCount(3, $this->router->getRoutes());
self::assertCount(3, $this->router->getRoutes('GET'));
}

public function testRandomHttpMethodsAllowed()
Expand All @@ -319,6 +326,36 @@ public function testRandomHttpMethodsAllowed()
self::assertSame($route, $routeMatched);
}

public function testSameUrlDifferentMethodIsMatchedCorrectly()
{
$routeGet = new Route(['GET'], 'traceroute-get', 'traceroute', function() {});
$routePost = new Route(['POST'], 'traceroute-post', 'traceroute', function() {});

$this->router->addRoutes($routeGet, $routePost);

$routeMatchedGet = $this->router->match('GET', 'traceroute');

self::assertSame($routeGet, $routeMatchedGet);

$routeMatchedPost = $this->router->match('POST', 'traceroute');

self::assertSame($routePost, $routeMatchedPost);
}

public function testSameUrlDifferentMethodIsMatchedEvenOnMultipleMethods()
{
$route = new Route(['GET', 'POST'], 'traceroute', 'traceroute', function() {});

$this->router->addRoute($route);

$routeMatchedGet = $this->router->match('GET', 'traceroute');
$routeMatchedPost = $this->router->match('POST', 'traceroute');

self::assertSame($route, $routeMatchedGet);
self::assertSame($route, $routeMatchedPost);
self::assertSame($routeMatchedGet, $routeMatchedPost);
}

/**
* @dataProvider dpSimpleUrls
*/
Expand Down

0 comments on commit 4bdddf1

Please sign in to comment.