Skip to content
This repository has been archived by the owner on Jun 1, 2023. It is now read-only.
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: hhvm/hack-router
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: main
Choose a base ref
...
head repository: azjezz/hack-router
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: master
Choose a head ref
Can’t automatically merge. Don’t worry, you can still create the pull request.
  • 1 commit
  • 3 files changed
  • 1 contributor

Commits on Nov 22, 2019

  1. Copy the full SHA
    c71cb58 View commit details
Showing with 103 additions and 66 deletions.
  1. +12 −0 src/http-exceptions/MethodNotAllowedException.php
  2. +35 −20 src/router/BaseRouter.php
  3. +56 −46 tests/RouterTest.php
12 changes: 12 additions & 0 deletions src/http-exceptions/MethodNotAllowedException.php
Original file line number Diff line number Diff line change
@@ -11,4 +11,16 @@
namespace Facebook\HackRouter;

class MethodNotAllowedException extends HttpException {
public function __construct(
protected keyset<HttpMethod> $allowed,
string $message = '',
int $code = 0,
?\Exception $previous = null,
) {
parent::__construct($message, $code, $previous);
}

public function getAllowedMethods(): keyset<HttpMethod> {
return $this->allowed;
}
}
55 changes: 35 additions & 20 deletions src/router/BaseRouter.php
Original file line number Diff line number Diff line change
@@ -10,7 +10,7 @@

namespace Facebook\HackRouter;

use namespace HH\Lib\Dict;
use namespace HH\Lib\{C, Dict};
use function Facebook\AutoloadMap\Generated\is_dev;

abstract class BaseRouter<+TResponder> {
@@ -27,22 +27,20 @@ final public function routeMethodAndPath(
$data = Dict\map($data, $value ==> \urldecode($value));
return tuple($responder, new ImmMap($data));
} catch (NotFoundException $e) {
foreach (HttpMethod::getValues() as $next) {
if ($next === $method) {
continue;
}
try {
list($responder, $data) = $resolver->resolve($next, $path);
if ($method === HttpMethod::HEAD && $next === HttpMethod::GET) {
$data = Dict\map($data, $value ==> \urldecode($value));
return tuple($responder, new ImmMap($data));
}
throw new MethodNotAllowedException();
} catch (NotFoundException $_) {
continue;
}
$allowed = $this->getAllowedMethods($path);
if (C\is_empty($allowed)) {
throw $e;
}
throw $e;

if (
$method === HttpMethod::HEAD && $allowed === keyset[HttpMethod::GET]
) {
list($responder, $data) = $resolver->resolve(HttpMethod::GET, $path);
$data = Dict\map($data, $value ==> \urldecode($value));
return tuple($responder, new ImmMap($data));
}

throw new MethodNotAllowedException($allowed);
}
}

@@ -51,11 +49,29 @@ final public function routeRequest(
): (TResponder, ImmMap<string, string>) {
$method = HttpMethod::coerce($request->getMethod());
if ($method === null) {
throw new MethodNotAllowedException();
throw new MethodNotAllowedException(
$this->getAllowedMethods($request->getUri()->getPath()),
);
}

return $this->routeMethodAndPath($method, $request->getUri()->getPath());
}

private function getAllowedMethods(string $path): keyset<HttpMethod> {
$resolver = $this->getResolver();
$allowed = keyset[];
foreach (HttpMethod::getValues() as $method) {
try {
list($_responder, $_data) = $resolver->resolve($method, $path);
$allowed[] = $method;
} catch (NotFoundException $_) {
continue;
}
}

return $allowed;
}

private ?IResolver<TResponder> $resolver = null;

protected function getResolver(): IResolver<TResponder> {
@@ -76,9 +92,8 @@ protected function getResolver(): IResolver<TResponder> {
if ($routes === null) {
$routes = Dict\map(
$this->getRoutes(),
$method_routes ==> PrefixMatching\PrefixMap::fromFlatMap(
dict($method_routes),
),
$method_routes ==>
PrefixMatching\PrefixMap::fromFlatMap(dict($method_routes)),
);

if (!is_dev()) {
102 changes: 56 additions & 46 deletions tests/RouterTest.php
Original file line number Diff line number Diff line change
@@ -18,24 +18,23 @@
use type Facebook\Experimental\Http\Message\HTTPMethod;

final class RouterTest extends \Facebook\HackTest\HackTest {
const keyset<string>
MAP = keyset[
'/foo',
'/foo/',
'/foo/bar',
'/foo/bar/{baz}',
'/foo/{bar}',
'/foo/{bar}/baz',
'/foo/{bar}{baz:.+}',
'/food/{noms}',
'/bar/{herp:\\d+}',
'/bar/{herp}',
'/unique/{foo}/bar',
'/optional_suffix_[foo]',
'/optional_suffix[/]',
'/optional_suffixes/[herp[/derp]]',
'/manual/en/{LegacyID}.php',
];
const keyset<string> MAP = keyset[
'/foo',
'/foo/',
'/foo/bar',
'/foo/bar/{baz}',
'/foo/{bar}',
'/foo/{bar}/baz',
'/foo/{bar}{baz:.+}',
'/food/{noms}',
'/bar/{herp:\\d+}',
'/bar/{herp}',
'/unique/{foo}/bar',
'/optional_suffix_[foo]',
'/optional_suffix[/]',
'/optional_suffixes/[herp[/derp]]',
'/manual/en/{LegacyID}.php',
];

public function expectedMatches(
): varray<(string, string, dict<string, string>)> {
@@ -124,35 +123,45 @@ public function expectedMatchesWithResolvers(
<<DataProvider('getAllResolvers')>>
public function testMethodNotAllowedResponses(
string $_name,
(function(dict<HttpMethod, dict<string, string>>): IResolver<string>)
$factory,
(function(
dict<HttpMethod, dict<string, string>>,
): IResolver<string>) $factory,
): void {
$map = dict[
HttpMethod::GET => dict[
'getonly' => 'getonly',
'/get' => 'get',
],
HttpMethod::HEAD => dict[
'headonly' => 'headonly',
'/head' => 'head',
],
HttpMethod::POST => dict[
'postonly' => 'postonly',
'/post' => 'post',
],
];

$router = $this->getRouter()->setResolver($factory($map));

list($responder, $_data) =
$router->routeMethodAndPath(HttpMethod::HEAD, 'getonly');
expect($responder)->toBeSame('getonly');
expect(() ==> $router->routeMethodAndPath(HttpMethod::GET, 'headonly'))->toThrow(
MethodNotAllowedException::class,
);
expect(() ==> $router->routeMethodAndPath(HttpMethod::HEAD, 'postonly'))->toThrow(
MethodNotAllowedException::class,
);
expect(() ==> $router->routeMethodAndPath(HttpMethod::GET, 'postonly'))->toThrow(
MethodNotAllowedException::class,
// HEAD -> GET ( re-routing )
list($responder, $_data) = $router->routeMethodAndPath(
HttpMethod::HEAD,
'/get',
);
expect($responder)->toBeSame('get');

// GET -> HEAD
$e = expect(() ==> $router->routeMethodAndPath(HttpMethod::GET, '/head'))
->toThrow(MethodNotAllowedException::class);
expect($e->getAllowedMethods())->toBeSame(keyset[HttpMethod::HEAD]);

// HEAD -> POST
$e = expect(() ==> $router->routeMethodAndPath(HttpMethod::HEAD, '/post'))
->toThrow(MethodNotAllowedException::class);
expect($e->getAllowedMethods())->toBeSame(keyset[HttpMethod::POST]);

// GET -> POST
$e = expect(() ==> $router->routeMethodAndPath(HttpMethod::GET, '/post'))
->toThrow(MethodNotAllowedException::class);
expect($e->getAllowedMethods())->toEqual(keyset[HttpMethod::POST]);
}

<<DataProvider('expectedMatches')>>
@@ -161,8 +170,8 @@ public function testMatchesPattern(
string $expected_responder,
dict<string, string> $expected_data,
): void {
list($actual_responder, $actual_data) =
$this->getRouter()->routeMethodAndPath(HttpMethod::GET, $in);
list($actual_responder, $actual_data) = $this->getRouter()
->routeMethodAndPath(HttpMethod::GET, $in);
expect($actual_responder)->toBeSame($expected_responder);
expect(dict($actual_data))->toBeSame($expected_data);
}
@@ -199,8 +208,10 @@ public function testRequestResponseInterfacesSupport(
dict<string, string> $_expected_data,
): void {
$router = $this->getRouter();
list($direct_responder, $direct_data) =
$router->routeMethodAndPath(HttpMethod::GET, $path);
list($direct_responder, $direct_data) = $router->routeMethodAndPath(
HttpMethod::GET,
$path,
);

expect($path[0])->toBeSame('/');

@@ -217,21 +228,20 @@ public function testRequestResponseInterfacesSupport(
<<DataProvider('getAllResolvers')>>
public function testNotFound(
string $_resolver_name,
(function(dict<HttpMethod, dict<string, string>>): IResolver<string>)
$factory,
(function(
dict<HttpMethod, dict<string, string>>,
): IResolver<string>) $factory,
): void {
$router = $this->getRouter()->setResolver($factory(dict[]));
expect(() ==> $router->routeMethodAndPath(HttpMethod::GET, '/__404'))->toThrow(
NotFoundException::class,
);
expect(() ==> $router->routeMethodAndPath(HttpMethod::GET, '/__404'))
->toThrow(NotFoundException::class);

$router = $this->getRouter()
->setResolver($factory(dict[
HttpMethod::GET => dict['/foo' => '/foo'],
]));
expect(() ==> $router->routeMethodAndPath(HttpMethod::GET, '/__404'))->toThrow(
NotFoundException::class,
);
expect(() ==> $router->routeMethodAndPath(HttpMethod::GET, '/__404'))
->toThrow(NotFoundException::class);
}

public function testMethodNotAllowed(): void {