diff --git a/src/http-exceptions/MethodNotAllowedException.php b/src/http-exceptions/MethodNotAllowedException.php index 342b516..45ce801 100644 --- a/src/http-exceptions/MethodNotAllowedException.php +++ b/src/http-exceptions/MethodNotAllowedException.php @@ -11,4 +11,16 @@ namespace Facebook\HackRouter; class MethodNotAllowedException extends HttpException { + public function __construct( + protected keyset $allowed, + string $message = '', + int $code = 0, + ?\Exception $previous = null, + ) { + parent::__construct($message, $code, $previous); + } + + public function getAllowedMethods(): keyset { + return $this->allowed; + } } diff --git a/src/router/BaseRouter.php b/src/router/BaseRouter.php index 8616e5c..650bab8 100644 --- a/src/router/BaseRouter.php +++ b/src/router/BaseRouter.php @@ -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) { $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 { + $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 $resolver = null; protected function getResolver(): IResolver { @@ -76,9 +92,8 @@ protected function getResolver(): IResolver { 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()) { diff --git a/tests/RouterTest.php b/tests/RouterTest.php index c75126d..de969ab 100644 --- a/tests/RouterTest.php +++ b/tests/RouterTest.php @@ -18,24 +18,23 @@ use type Facebook\Experimental\Http\Message\HTTPMethod; final class RouterTest extends \Facebook\HackTest\HackTest { - const keyset - 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 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)> { @@ -124,35 +123,45 @@ public function expectedMatchesWithResolvers( <> public function testMethodNotAllowedResponses( string $_name, - (function(dict>): IResolver) - $factory, + (function( + dict>, + ): IResolver) $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]); } <> @@ -161,8 +170,8 @@ public function testMatchesPattern( string $expected_responder, dict $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 $_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( <> public function testNotFound( string $_resolver_name, - (function(dict>): IResolver) - $factory, + (function( + dict>, + ): IResolver) $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 {