Add MethodNotAllowedException::getAllowedMethods()

hhvm · Nov 19, 2019 · 5fc0109 · 5fc0109
1 parent d02bd48
commit 5fc0109
Show file tree

Hide file tree

Showing 3 changed files with 73 additions and 21 deletions.
diff --git 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<HttpMethod> $allowed,
+    string $message = '',
+    int $code = 0,
+    ?\Exception $previous = null,
+  ) {
+    parent::__construct($message, $code, $previous);
+  }
+
+  public function getAllowedMethods(): keyset<HttpMethod> {
+    return $this->allowed;
+  }
 }
diff --git 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 (0 === C\count($allowed)) {
+        throw $e;
       }
-      throw $e;
+
+      if (
+        HttpMethod::HEAD === $method && keyset[HttpMethod::GET] === $allowed
+      ) {
+        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()) {

diff --git a/tests/RouterTest.php b/tests/RouterTest.php
@@ -11,7 +11,7 @@
 namespace Facebook\HackRouter;
 
 use function Facebook\FBExpect\expect;
-use namespace HH\Lib\Dict;
+use namespace HH\Lib\{Dict, Str};
 use type Facebook\HackRouter\Tests\TestRouter;
 use type Facebook\HackTest\DataProvider;
 use type Usox\HackTTP\{ServerRequestFactory, UriFactory};
@@ -147,12 +147,37 @@ public function testMethodNotAllowedResponses(
     expect(() ==> $router->routeMethodAndPath(HttpMethod::GET, 'headonly'))->toThrow(
       MethodNotAllowedException::class,
     );
+    try {
+      $router->routeMethodAndPath(HttpMethod::GET, 'headonly');
+      static::fail(
+        Str\format('Failed asserting that %s was thrown.', MethodNotAllowedException::class)
+      );
+    } catch(MethodNotAllowedException $e) {
+      expect($e->getAllowedMethods())->toBeSame(keyset[HttpMethod::HEAD]);
+    }
     expect(() ==> $router->routeMethodAndPath(HttpMethod::HEAD, 'postonly'))->toThrow(
       MethodNotAllowedException::class,
     );
+    try {
+      $router->routeMethodAndPath(HttpMethod::HEAD, 'postonly');
+      static::fail(
+        Str\format('Failed asserting that %s was thrown.', MethodNotAllowedException::class)
+      );
+    } catch(MethodNotAllowedException $e) {
+      expect($e->getAllowedMethods())->toBeSame(keyset[HttpMethod::POST]);
+    }
     expect(() ==> $router->routeMethodAndPath(HttpMethod::GET, 'postonly'))->toThrow(
       MethodNotAllowedException::class,
     );
+    try {
+      $router->routeMethodAndPath(HttpMethod::GET, 'postonly');
+      static::fail(
+        Str\format('Failed asserting that %s was thrown.', MethodNotAllowedException::class)
+      );
+    } catch(MethodNotAllowedException $e) {
+      expect($e->getAllowedMethods())->toContain(HttpMethod::POST);
+    }
+
   }
 
   <<DataProvider('expectedMatches')>>