Simplify handling of invalid paths and methods

lib/REST/Exception404.php

@@ -1,10 +0,0 @@
-<?php
-/** @license MIT
- * Copyright 2017 J. King, Dustin Wilson et al.
- * See LICENSE and AUTHORS files for details */
-
-declare(strict_types=1);
-namespace JKingWeb\Arsse\REST;
-
-class Exception404 extends \Exception {
-}

lib/REST/Exception405.php

@@ -1,10 +0,0 @@
-<?php
-/** @license MIT
- * Copyright 2017 J. King, Dustin Wilson et al.
- * See LICENSE and AUTHORS files for details */
-
-declare(strict_types=1);
-namespace JKingWeb\Arsse\REST;
-
-class Exception405 extends \Exception {
-}

lib/REST/Miniflux/V1.php

@@ -12,9 +12,6 @@ use JKingWeb\Arsse\Db\ExceptionInput;
 use JKingWeb\Arsse\Misc\HTTP;
 use JKingWeb\Arsse\Misc\ValueInfo;
 use JKingWeb\Arsse\REST\Exception;
-use JKingWeb\Arsse\REST\Exception404;
-use JKingWeb\Arsse\REST\Exception405;
-use JKingWeb\Arsse\REST\Exception501;
 use JKingWeb\Arsse\User\ExceptionConflict as UserException;
 use Psr\Http\Message\ServerRequestInterface;
 use Psr\Http\Message\ResponseInterface;
@@ -86,6 +83,9 @@ class V1 extends \JKingWeb\Arsse\REST\AbstractHandler {
             return $this->handleHTTPOptions($target);
         }
         $func = $this->chooseCall($target, $method);
+        if ($func instanceof ResponseInterface) {
+            return $func;
+        }
         if ($func === "opmlImport") {
             if (!HTTP::matchType($req, "", ...[self::ACCEPTED_TYPES_OPML])) {
                 return new ErrorResponse("", 415, ['Accept' => implode(", ", self::ACCEPTED_TYPES_OPML)]);
@@ -149,7 +149,7 @@ class V1 extends \JKingWeb\Arsse\REST\AbstractHandler {
         }
     }
 
-    protected function chooseCall(string $url, string $method): string {
+    protected function chooseCall(string $url, string $method) {
         // // normalize the URL path: change any IDs to 1 for easier comparison
         $url = $this->normalizePathIds($url);
         // normalize the HTTP method to uppercase
@@ -160,18 +160,15 @@ class V1 extends \JKingWeb\Arsse\REST\AbstractHandler {
             // if the path is supported, make sure the method is allowed
             if (isset($this->paths[$url][$method])) {
                 // if it is allowed, return the object method to run, assuming the method exists
-                if (method_exists($this, $this->paths[$url][$method])) {
+                assert(method_exists($this, $this->paths[$url][$method]), new \Exception("Method is not implemented"));
-                    return $this->paths[$url][$method];
+                return $this->paths[$url][$method];
-                } else {
-                    throw new Exception501(); // @codeCoverageIgnore
-                }
             } else {
                 // otherwise return 405
-                throw new Exception405(implode(", ", array_keys($this->paths[$url])));
+                return new EmptyResponse(405, ['Allow' => implode(", ", array_keys($this->paths[$url]))]);
             }
         } else {
             // if the path is not supported, return 404
-            throw new Exception404();
+            return new EmptyResponse(404);
         }
     }
 

lib/REST/NextcloudNews/V1_2.php

@@ -15,9 +15,6 @@ use JKingWeb\Arsse\Db\ExceptionInput;
 use JKingWeb\Arsse\Feed\Exception as FeedException;
 use JKingWeb\Arsse\Misc\HTTP;
 use JKingWeb\Arsse\REST\Exception;
-use JKingWeb\Arsse\REST\Exception404;
-use JKingWeb\Arsse\REST\Exception405;
-use JKingWeb\Arsse\REST\Exception501;
 use Psr\Http\Message\ServerRequestInterface;
 use Psr\Http\Message\ResponseInterface;
 use Laminas\Diactoros\Response\JsonResponse as Response;
@@ -110,15 +107,14 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler {
         // merge GET and POST data, and normalize it. POST parameters are preferred over GET parameters
         $data = $this->normalizeInput(array_merge($req->getQueryParams(), $data), $this->validInput, "unix");
         // check to make sure the requested function is implemented
+        $func = $this->chooseCall($target, $req->getMethod());
+        if ($func instanceof ResponseInterface) {
+            return $func;
+        }
         // dispatch
         try {
-            $func = $this->chooseCall($target, $req->getMethod());
             $path = explode("/", ltrim($target, "/"));
             return $this->$func($path, $data);
-        } catch (Exception404 $e) {
-            return new EmptyResponse(404);
-        } catch (Exception405 $e) {
-            return new EmptyResponse(405, ['Allow' => $e->getMessage()]);
             // @codeCoverageIgnoreStart
         } catch (Exception $e) {
             // if there was a REST exception return 400
@@ -141,7 +137,7 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler {
         return implode("/", $path);
     }
 
-    protected function chooseCall(string $url, string $method): string {
+    protected function chooseCall(string $url, string $method) {
         // // normalize the URL path: change any IDs to 1 for easier comparison
         $url = $this->normalizePathIds($url);
         // normalize the HTTP method to uppercase
@@ -152,18 +148,15 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler {
             // if the path is supported, make sure the method is allowed
             if (isset($this->paths[$url][$method])) {
                 // if it is allowed, return the object method to run, assuming the method exists
-                if (method_exists($this, $this->paths[$url][$method])) {
+                assert(method_exists($this, $this->paths[$url][$method]), new \Exception("Method is not implemented"));
-                    return $this->paths[$url][$method];
+                return $this->paths[$url][$method];
-                } else {
-                    throw new Exception501(); // @codeCoverageIgnore
-                }
             } else {
                 // otherwise return 405
-                throw new Exception405(implode(", ", array_keys($this->paths[$url])));
+                return new EmptyResponse(405, ['Allow' => implode(", ", array_keys($this->paths[$url]))]);
             }
         } else {
             // if the path is not supported, return 404
-            throw new Exception404();
+            return new EmptyResponse(404);
         }
     }
 

tests/cases/REST/Miniflux/TestV1.php

@@ -11,7 +11,6 @@ use JKingWeb\Arsse\User;
 use JKingWeb\Arsse\Database;
 use JKingWeb\Arsse\Db\Transaction;
 use JKingWeb\Arsse\Db\ExceptionInput;
-use JKingWeb\Arsse\REST\Exception404;
 use JKingWeb\Arsse\REST\Miniflux\V1;
 use JKingWeb\Arsse\REST\Miniflux\ErrorResponse;
 use Psr\Http\Message\ResponseInterface;
@@ -61,7 +60,7 @@ class TestV1 extends \JKingWeb\Arsse\Test\AbstractTest {
 
     /** @dataProvider provideAuthResponses */
     public function testAuthenticateAUser($token, bool $auth, bool $success): void {
-        $exp = new ErrorResponse("401", 401);
+        $exp = $success ? new EmptyResponse(404) : new ErrorResponse("401", 401);
         $user = "john.doe@example.com";
         if ($token !== null) {
             $headers = ['X-Auth-Token' => $token];
@@ -71,17 +70,8 @@ class TestV1 extends \JKingWeb\Arsse\Test\AbstractTest {
         Arsse::$user->id = null;
         \Phake::when(Arsse::$db)->tokenLookup->thenThrow(new ExceptionInput("subjectMissing"));
         \Phake::when(Arsse::$db)->tokenLookup("miniflux.login", $this->token)->thenReturn(['user' => $user]);
-        if ($success) {
+        $this->assertMessage($exp, $this->req("GET", "/", "", $headers, $auth));
-            $this->expectExceptionObject(new Exception404);
+        $this->assertSame($success ? $user : null, Arsse::$user->id);
-            try {
-                $this->req("GET", "/", "", $headers, $auth);
-            } finally {
-                $this->assertSame($user, Arsse::$user->id);
-            }
-        } else {
-            $this->assertMessage($exp, $this->req("GET", "/", "", $headers, $auth));
-            $this->assertNull(Arsse::$user->id);
-        }
     }
 
     public function provideAuthResponses(): iterable {
@@ -100,7 +90,7 @@ class TestV1 extends \JKingWeb\Arsse\Test\AbstractTest {
     }
 
     /** @dataProvider provideInvalidPaths */
-    public function xtestRespondToInvalidPaths($path, $method, $code, $allow = null): void {
+    public function testRespondToInvalidPaths($path, $method, $code, $allow = null): void {
         $exp = new EmptyResponse($code, $allow ? ['Allow' => $allow] : []);
         $this->assertMessage($exp, $this->req($method, $path));
     }
@@ -108,7 +98,7 @@ class TestV1 extends \JKingWeb\Arsse\Test\AbstractTest {
     public function provideInvalidPaths(): array {
         return [
             ["/",                  "GET",     404],
-            ["/version",           "POST",    405, "GET"],
+            ["/me",                "POST",    405, "GET"],
         ];
     }