Make Router::getModuleClass throw exceptions

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

diff --git a/src/App/Module.php b/src/App/Module.php
index 9a24c55544199635cde4a956577070491ce6519f..33a9b2fc2f33d91d0a2754f38448f15999322072 100644 (file)
--- a/src/App/Module.php
+++ b/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,38 +147,43 @@ 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.
                 **/
-               $module_class = $router->getModuleClass($args->getCommand());
-
-               // 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")) {
-                       //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) {
-                               $printNotAllowedAddon = true;
-                       } else {
-                               include_once "addon/{$this->module}/{$this->module}.php";
-                               if (function_exists($this->module . '_module')) {
-                                       LegacyModule::setModuleFile("addon/{$this->module}/{$this->module}.php");
-                                       $module_class = LegacyModule::class;
+               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 (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) {
+                                       $printNotAllowedAddon = true;
+                               } else {
+                                       include_once "addon/{$this->module}/{$this->module}.php";
+                                       if (function_exists($this->module . '_module')) {
+                                               LegacyModule::setModuleFile("addon/{$this->module}/{$this->module}.php");
+                                               $module_class = LegacyModule::class;
+                                       }
                                }
                        }
-               }
 
-               /* Finally, we look for a 'standard' program module in the 'mod' directory
-                * We emulate a Module class through the LegacyModule class
-                */
-               if (!$module_class && file_exists("mod/{$this->module}.php")) {
-                       LegacyModule::setModuleFile("mod/{$this->module}.php");
-                       $module_class = LegacyModule::class;
-               }
+                       /* Finally, we look for a 'standard' program module in the 'mod' directory
+                        * We emulate a Module class through the LegacyModule class
+                        */
+                       if (!$module_class && file_exists("mod/{$this->module}.php")) {
+                               LegacyModule::setModuleFile("mod/{$this->module}.php");
+                               $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);
        }

diff --git a/src/App/Router.php b/src/App/Router.php
index 1bdfdcab1d0fac735d338c8fba7baa0e7def8dd6..f723321ac67cf883bfe8a2ab93c26c5e77c78b7c 100644 (file)
--- a/src/App/Router.php
+++ b/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;

diff --git a/src/Module/HTTPException/MethodNotAllowed.php b/src/Module/HTTPException/MethodNotAllowed.php
new file mode 100644 (file)
index 0000000..8d2d280
--- /dev/null
+++ b/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.'));
+       }
+}

diff --git a/src/Module/HTTPException/PageNotFound.php b/src/Module/HTTPException/PageNotFound.php
new file mode 100644 (file)
index 0000000..d848905
--- /dev/null
+++ b/src/Module/HTTPException/PageNotFound.php
@@ -0,0 +1,15 @@
+<?php
+
+namespace Friendica\Module\HTTPException;
+
+use Friendica\BaseModule;
+use Friendica\Core\L10n;
+use Friendica\Network\HTTPException;
+
+class PageNotFound extends BaseModule
+{
+       public static function content()
+       {
+               throw new HTTPException\NotFoundException(L10n::t('Page not found.'));
+       }
+}

diff --git a/src/Module/PageNotFound.php b/src/Module/PageNotFound.php
deleted file mode 100644 (file)
index 764903c..0000000
--- a/src/Module/PageNotFound.php
+++ /dev/null
@@ -1,15 +0,0 @@
-<?php
-
-namespace Friendica\Module;
-
-use Friendica\BaseModule;
-use Friendica\Core\L10n;
-use Friendica\Network\HTTPException;
-
-class PageNotFound extends BaseModule
-{
-       public static function content()
-       {
-               throw new HTTPException\NotFoundException(L10n::t('Page not found.'));
-       }
-}

diff --git a/tests/src/App/ModuleTest.php b/tests/src/App/ModuleTest.php
index e43f22d0ceb2d96e3b84990b8dfc82deeaa9b6cd..8327bc706dd479e8d8c4159966f7150c0be67636 100644 (file)
--- a/tests/src/App/ModuleTest.php
+++ b/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);
        }

diff --git a/tests/src/App/RouterTest.php b/tests/src/App/RouterTest.php
index 2ed8574c43dd23a0a342cc1ef3b37b4627702146..2fcc02ff517ef3874e5789d87bdb386f4d8dd5d6 100644 (file)
--- a/tests/src/App/RouterTest.php
+++ b/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'));
+       }
+
+       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');
 
-               $this->assertNull($router->getModuleClass('/unsupported'));
+               $router->getModuleClass('/test');
        }
 
        public function dataRoutes()