Make Router::getModuleClass throw exceptions

- Add new MethodNotAllowedModule
- Add new Module->determineClass catch blocks
- Update Module and Router tests

src/App/Module.php

@@ -7,7 +7,10 @@ use Friendica\BaseObject;
 use Friendica\Core;
 use Friendica\LegacyModule;
 use Friendica\Module\Home;
-use Friendica\Module\PageNotFound;
+use Friendica\Module\HTTPException\MethodNotAllowed;
+use Friendica\Module\HTTPException\PageNotFound;
+use Friendica\Network\HTTPException\MethodNotAllowedException;
+use Friendica\Network\HTTPException\NotFoundException;
 use Psr\Log\LoggerInterface;
 
 /**
@@ -144,16 +147,20 @@ class Module
 	{
 		$printNotAllowedAddon = false;
 
+		$module_class = null;
 		/**
 		 * ROUTING
 		 *
 		 * From the request URL, routing consists of obtaining the name of a BaseModule-extending class of which the
 		 * post() and/or content() static methods can be respectively called to produce a data change or an output.
 		 **/
+		try {
 			$module_class = $router->getModuleClass($args->getCommand());
-
+		} catch (MethodNotAllowedException $e) {
+			$module_class = MethodNotAllowed::class;
+		} catch (NotFoundException $e) {
 			// Then we try addon-provided modules that we wrap in the LegacyModule class
-		if (!$module_class && Core\Addon::isEnabled($this->module) && file_exists("addon/{$this->module}/{$this->module}.php")) {
+			if (Core\Addon::isEnabled($this->module) && file_exists("addon/{$this->module}/{$this->module}.php")) {
 				//Check if module is an app and if public access to apps is allowed or not
 				$privateapps = $config->get('config', 'private_addons', false);
 				if ((!local_user()) && Core\Hook::isAddonApp($this->module) && $privateapps) {
@@ -175,7 +182,8 @@ class Module
 				$module_class = LegacyModule::class;
 			}
 
-		$module_class = !isset($module_class) ? PageNotFound::class : $module_class;
+			$module_class = $module_class ?: PageNotFound::class;
+		}
 
 		return new Module($this->module, $module_class, $this->isBackend, $printNotAllowedAddon);
 	}

src/App/Router.php

@@ -8,7 +8,8 @@ use FastRoute\Dispatcher;
 use FastRoute\RouteCollector;
 use FastRoute\RouteParser\Std;
 use Friendica\Core\Hook;
-use Friendica\Network\HTTPException\InternalServerErrorException;
+use Friendica\Core\L10n;
+use Friendica\Network\HTTPException;
 
 /**
  * Wrapper for FastRoute\Router
@@ -57,7 +58,7 @@ class Router
 	 *
 	 * @return self The router instance with the loaded routes
 	 *
-	 * @throws InternalServerErrorException In case of invalid configs
+	 * @throws HTTPException\InternalServerErrorException In case of invalid configs
 	 */
 	public function addRoutes(array $routes)
 	{
@@ -71,7 +72,7 @@ class Router
 			} elseif ($this->isRoute($config)) {
 				$routeCollector->addRoute($config[1], $route, $config[0]);
 			} else {
-				throw new InternalServerErrorException("Wrong route config for route '" . print_r($route, true) . "'");
+				throw new HTTPException\InternalServerErrorException("Wrong route config for route '" . print_r($route, true) . "'");
 			}
 		}
 
@@ -96,7 +97,7 @@ class Router
 				} elseif ($this->isRoute($config)) {
 					$routeCollector->addRoute($config[1], $route, $config[0]);
 				}else {
-					throw new InternalServerErrorException("Wrong route config for route '" . print_r($route, true) . "'");
+					throw new HTTPException\InternalServerErrorException("Wrong route config for route '" . print_r($route, true) . "'");
 				}
 			}
 		});
@@ -155,7 +156,11 @@ class Router
 	 *
 	 * @param string $cmd The path component of the request URL without the query string
 	 *
-	 * @return string|null A Friendica\BaseModule-extending class name if a route rule matched
+	 * @return string A Friendica\BaseModule-extending class name if a route rule matched
+	 *
+	 * @throws HTTPException\InternalServerErrorException
+	 * @throws HTTPException\MethodNotAllowedException    If a rule matched but the method didn't
+	 * @throws HTTPException\NotFoundException            If no rule matched
 	 */
 	public function getModuleClass($cmd)
 	{
@@ -171,6 +176,10 @@ class Router
 		$routeInfo  = $dispatcher->dispatch($this->httpMethod, $cmd);
 		if ($routeInfo[0] === Dispatcher::FOUND) {
 			$moduleClass = $routeInfo[1];
+		} elseif ($routeInfo[0] === Dispatcher::METHOD_NOT_ALLOWED) {
+			throw new HTTPException\MethodNotAllowedException(L10n::t('Method not allowed for this module. Allowed method(s): %s', implode(', ', $routeInfo[1])));
+		} else {
+			throw new HTTPException\NotFoundException(L10n::t('Page not found.'));
 		}
 
 		return $moduleClass;

src/Module/HTTPException/MethodNotAllowed.php

@@ -0,0 +1,15 @@
+<?php
+
+namespace Friendica\Module\HTTPException;
+
+use Friendica\BaseModule;
+use Friendica\Core\L10n;
+use Friendica\Network\HTTPException;
+
+class MethodNotAllowed extends BaseModule
+{
+	public static function content()
+	{
+		throw new HTTPException\MethodNotAllowedException(L10n::t('Method Not Allowed.'));
+	}
+}

src/Module/HTTPException/PageNotFound.php

@@ -1,6 +1,6 @@
 <?php
 
-namespace Friendica\Module;
+namespace Friendica\Module\HTTPException;
 
 use Friendica\BaseModule;
 use Friendica\Core\L10n;

tests/src/App/ModuleTest.php

@@ -5,7 +5,7 @@ namespace Friendica\Test\src\App;
 use Friendica\App;
 use Friendica\Core\Config\Configuration;
 use Friendica\LegacyModule;
-use Friendica\Module\PageNotFound;
+use Friendica\Module\HTTPException\PageNotFound;
 use Friendica\Module\WellKnown\HostMeta;
 use Friendica\Test\DatabaseTest;
 
@@ -166,7 +166,7 @@ class ModuleTest extends DatabaseTest
 	{
 		$module = new App\Module();
 
-		$moduleNew = $module->determineModule(new App\Arguments(), []);
+		$moduleNew = $module->determineModule(new App\Arguments());
 
 		$this->assertNotSame($moduleNew, $module);
 	}

tests/src/App/RouterTest.php

@@ -4,13 +4,15 @@ namespace Friendica\Test\src\App;
 
 use Friendica\App\Router;
 use Friendica\Module;
+use Friendica\Network\HTTPException\MethodNotAllowedException;
+use Friendica\Network\HTTPException\NotFoundException;
 use PHPUnit\Framework\TestCase;
 
 class RouterTest extends TestCase
 {
 	public function testGetModuleClass()
 	{
-		$router = new Router(['GET']);
+		$router = new Router(['REQUEST_METHOD' => 'GET']);
 
 		$routeCollector = $router->getRouteCollector();
 		$routeCollector->addRoute(['GET'], '/', 'IndexModuleClassName');
@@ -22,23 +24,70 @@ class RouterTest extends TestCase
 		$routeCollector->addRoute(['POST', 'PUT', 'PATCH', 'DELETE', 'HEAD'], '/unsupported', 'UnsupportedMethodModuleClassName');
 
 		$this->assertEquals('IndexModuleClassName', $router->getModuleClass('/'));
-
 		$this->assertEquals('TestModuleClassName', $router->getModuleClass('/test'));
-		$this->assertNull($router->getModuleClass('/tes'));
-
 		$this->assertEquals('TestSubModuleClassName', $router->getModuleClass('/test/sub'));
-
 		$this->assertEquals('OptionalModuleClassName', $router->getModuleClass('/optional'));
 		$this->assertEquals('OptionalModuleClassName', $router->getModuleClass('/optional/option'));
-		$this->assertNull($router->getModuleClass('/optional/opt'));
-
 		$this->assertEquals('VariableModuleClassName', $router->getModuleClass('/variable/123abc'));
-		$this->assertNull($router->getModuleClass('/variable'));
-
 		$this->assertEquals('OptionalVariableModuleClassName', $router->getModuleClass('/optionalvariable'));
 		$this->assertEquals('OptionalVariableModuleClassName', $router->getModuleClass('/optionalvariable/123abc'));
+	}
 
-		$this->assertNull($router->getModuleClass('/unsupported'));
+	public function testGetModuleClassNotFound()
+	{
+		$this->expectException(NotFoundException::class);
+
+		$router = new Router(['REQUEST_METHOD' => 'GET']);
+
+		$router->getModuleClass('/unsupported');
+	}
+
+	public function testGetModuleClassNotFoundTypo()
+	{
+		$this->expectException(NotFoundException::class);
+
+		$router = new Router(['REQUEST_METHOD' => 'GET']);
+
+		$routeCollector = $router->getRouteCollector();
+		$routeCollector->addRoute(['GET'], '/test', 'TestModuleClassName');
+
+		$router->getModuleClass('/tes');
+	}
+
+	public function testGetModuleClassNotFoundOptional()
+	{
+		$this->expectException(NotFoundException::class);
+
+		$router = new Router(['REQUEST_METHOD' => 'GET']);
+
+		$routeCollector = $router->getRouteCollector();
+		$routeCollector->addRoute(['GET'], '/optional[/option]', 'OptionalModuleClassName');
+
+		$router->getModuleClass('/optional/opt');
+	}
+
+	public function testGetModuleClassNotFoundVariable()
+	{
+		$this->expectException(NotFoundException::class);
+
+		$router = new Router(['REQUEST_METHOD' => 'GET']);
+
+		$routeCollector = $router->getRouteCollector();
+		$routeCollector->addRoute(['GET'], '/variable/{var}', 'VariableModuleClassName');
+
+		$router->getModuleClass('/variable');
+	}
+
+	public function testGetModuleClassMethodNotAllowed()
+	{
+		$this->expectException(MethodNotAllowedException::class);
+
+		$router = new Router(['REQUEST_METHOD' => 'POST']);
+
+		$routeCollector = $router->getRouteCollector();
+		$routeCollector->addRoute(['GET'], '/test', 'TestModuleClassName');
+
+		$router->getModuleClass('/test');
 	}
 
 	public function dataRoutes()