From a8620cbc70725f0759c9501b6fbb70f7cca30338 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Sessing=C3=B8?= Date: Thu, 20 Oct 2016 08:31:21 +0200 Subject: [PATCH] Updates - Simplified exception-handling (see demo project for examples). - Optimised sample-project. - Optimised and added further unit-tests. - Optimised and bugfixes. --- README.md | 66 +++-------- demo-project/README.md | 5 +- .../app/Controllers/DefaultController.php | 4 + .../app/Handlers/CustomExceptionHandler.php | 8 +- demo-project/app/Router.php | 49 ++------ demo-project/app/routes.php | 20 ++-- src/Pecee/SimpleRouter/RouterBase.php | 106 ++++++++++-------- src/Pecee/SimpleRouter/RouterEntry.php | 2 +- test/Dummy/DummyController.php | 4 + test/Dummy/Handler/ExceptionHandler.php | 8 ++ test/GroupTest.php | 7 -- test/MiddlewareTest.php | 20 ++-- test/RouterRouteTest.php | 27 ++++- 13 files changed, 151 insertions(+), 175 deletions(-) create mode 100644 test/Dummy/Handler/ExceptionHandler.php diff --git a/README.md b/README.md index 0abb041..a9dfc05 100644 --- a/README.md +++ b/README.md @@ -187,76 +187,36 @@ The framework has it's own ```Router``` class which inherits from the ```SimpleR namespace Demo; use Pecee\Exception\RouterException; -use Pecee\Handler\ExceptionHandler; use Pecee\Http\Middleware\IMiddleware; use Pecee\SimpleRouter\RouterBase; use Pecee\SimpleRouter\SimpleRouter; class Router extends SimpleRouter { - protected static $defaultExceptionHandler; protected static $defaultMiddlewares = array(); public static function start($defaultNamespace = null) { - // Debug information - Debug::getInstance()->add('Router initialised.'); + // change this to whatever makes sense in your project + require_once 'routes.php'; - // Load framework specific controllers - static::get('/js-wrap', 'ControllerJs@wrap', ['namespace' => '\Pecee\Controller'])->setAlias('pecee.js.wrap'); - static::get('/css-wrap', 'ControllerCss@wrap', ['namespace' => '\Pecee\Controller'])->setAlias('pecee.css.wrap'); - static::get('/captcha', 'ControllerCaptcha@show', ['namespace' => '\Pecee\Controller']); - // Load routes.php - $file = $_ENV['base_path'] . DIRECTORY_SEPARATOR . 'app' . DIRECTORY_SEPARATOR . 'routes.php'; - if(file_exists($file)) { - require_once $file; + if(count(static::$defaultMiddlewares)) { + /* @var $middleware \Pecee\Http\Middleware\IMiddleware */ + foreach(static::$defaultMiddlewares as $middleware) { + $middleware = new $middleware(); + if(!($middleware instanceof IMiddleware)) { + throw new RouterException('Middleware must be implement the IMiddleware interface.'); + } + $middleware->handle(RouterBase::getInstance()->getRequest()); + } } // Set default namespace - $defaultNamespace = '\\'.$_ENV['app_name'] . '\\Controller'; + $defaultNamespace = '\\Demo\\Controllers'; - // Handle exceptions - try { + parent::start($defaultNamespace); - if(count(static::$defaultMiddlewares)) { - /* @var $middleware \Pecee\Http\Middleware\IMiddleware */ - foreach(static::$defaultMiddlewares as $middleware) { - $middleware = new $middleware(); - if(!($middleware instanceof IMiddleware)) { - throw new RouterException('Middleware must be implement the IMiddleware interface.'); - } - $middleware->handle(RouterBase::getInstance()->getRequest()); - } - } - - parent::start($defaultNamespace); - } catch(\Exception $e) { - - $route = RouterBase::getInstance()->getLoadedRoute(); - - // Otherwise use the fallback default exceptions handler - if(static::$defaultExceptionHandler !== null) { - static::loadExceptionHandler(static::$defaultExceptionHandler, $route, $e); - } - - throw $e; - } - - } - - protected static function loadExceptionHandler($class, $route, $e) { - $class = new $class(); - - if(!($class instanceof ExceptionHandler)) { - throw new \ErrorException('Exception handler must be an instance of \Pecee\Handler\ExceptionHandler'); - } - - $class->handleError(RouterBase::getInstance()->getRequest(), $route, $e); - } - - public static function defaultExceptionHandler($handler) { - static::$defaultExceptionHandler = $handler; } /** diff --git a/demo-project/README.md b/demo-project/README.md index b95564e..ee0ec84 100644 --- a/demo-project/README.md +++ b/demo-project/README.md @@ -18,7 +18,7 @@ Please note that this demo-project only covers how to integrate the `simple-php- ## Installation -- Navigate to the `demo-project` folder in terminal and run `composer install`. +- Navigate to the `demo-project` folder in terminal and run `composer update` to install the latest version. - Point your webserver to `demo-project/public`. ### Setting up Nginx @@ -46,7 +46,8 @@ Nothing special is required for Apache to work. We've include the `.htaccess` fi ## Notes -The demo project has it's own `Router` class implementation which extends the `SimpleRouter` class with further functionality such as default exceptionhandlers and middlewares. This class can be useful adding additional functionality that are required before and after routing occurs or any extra functionality belonging to the router itself. +The demo project has it's own `Router` class implementation which extends the `SimpleRouter` class with further functionality. +This class can be useful adding additional functionality that are required before and after routing occurs or any extra functionality belonging to the router itself. In this project we also use our custom router-class to autoload the `routes.php` file from our custom location (`app/routes.php`). diff --git a/demo-project/app/Controllers/DefaultController.php b/demo-project/app/Controllers/DefaultController.php index 4d5856c..41afc10 100644 --- a/demo-project/app/Controllers/DefaultController.php +++ b/demo-project/app/Controllers/DefaultController.php @@ -22,4 +22,8 @@ class DefaultController { } + public function notFound() { + echo 'Page not found'; + } + } \ No newline at end of file diff --git a/demo-project/app/Handlers/CustomExceptionHandler.php b/demo-project/app/Handlers/CustomExceptionHandler.php index f59901f..019a1b2 100644 --- a/demo-project/app/Handlers/CustomExceptionHandler.php +++ b/demo-project/app/Handlers/CustomExceptionHandler.php @@ -21,8 +21,14 @@ class CustomExceptionHandler implements IExceptionHandler { // else we just throw the error if($error->getCode() == 404) { - die(sprintf('An error occurred (%s):
%s', $error->getCode(), $error->getMessage())); + + // Return 404 path + $request->setUri('/404'); + return $request; + } + + throw $error; } } \ No newline at end of file diff --git a/demo-project/app/Router.php b/demo-project/app/Router.php index ec7e027..374667e 100644 --- a/demo-project/app/Router.php +++ b/demo-project/app/Router.php @@ -8,14 +8,12 @@ namespace Demo; use Pecee\Exception\RouterException; -use Pecee\Handler\IExceptionHandler; use Pecee\Http\Middleware\IMiddleware; use Pecee\SimpleRouter\RouterBase; use Pecee\SimpleRouter\SimpleRouter; class Router extends SimpleRouter { - protected static $defaultExceptionHandler; protected static $defaultMiddlewares = array(); public static function start($defaultNamespace = null) { @@ -23,50 +21,23 @@ class Router extends SimpleRouter { // change this to whatever makes sense in your project require_once 'routes.php'; - // Handle exceptions - try { - if(count(static::$defaultMiddlewares)) { - /* @var $middleware \Pecee\Http\Middleware\IMiddleware */ - foreach(static::$defaultMiddlewares as $middleware) { - $middleware = new $middleware(); - if(!($middleware instanceof IMiddleware)) { - throw new RouterException('Middleware must be implement the IMiddleware interface.'); - } - $middleware->handle(RouterBase::getInstance()->getRequest()); + if(count(static::$defaultMiddlewares)) { + /* @var $middleware \Pecee\Http\Middleware\IMiddleware */ + foreach(static::$defaultMiddlewares as $middleware) { + $middleware = new $middleware(); + if(!($middleware instanceof IMiddleware)) { + throw new RouterException('Middleware must be implement the IMiddleware interface.'); } + $middleware->handle(RouterBase::getInstance()->getRequest()); } - - // Set default namespace - $defaultNamespace = '\\Demo\\Controllers'; - - parent::start($defaultNamespace); - } catch(\Exception $e) { - - $route = RouterBase::getInstance()->getLoadedRoute(); - - // Otherwise use the fallback default exceptions handler - if(static::$defaultExceptionHandler !== null) { - static::loadExceptionHandler(static::$defaultExceptionHandler, $route, $e); - } - - throw $e; } - } + // Set default namespace + $defaultNamespace = '\\Demo\\Controllers'; - protected static function loadExceptionHandler($class, $route, $e) { - $class = new $class(); + parent::start($defaultNamespace); - if(!($class instanceof IExceptionHandler)) { - throw new \ErrorException('Exception handler must be an instance of \Pecee\Handler\IExceptionHandler'); - } - - $class->handleError(RouterBase::getInstance()->getRequest(), $route, $e); - } - - public static function defaultExceptionHandler($handler) { - static::$defaultExceptionHandler = $handler; } /** diff --git a/demo-project/app/routes.php b/demo-project/app/routes.php index 0ed150d..6125596 100644 --- a/demo-project/app/routes.php +++ b/demo-project/app/routes.php @@ -6,14 +6,18 @@ use Demo\Router; Router::csrfVerifier(new \Demo\Middlewares\CsrfVerifier()); -Router::defaultExceptionHandler('\Demo\Handlers\CustomExceptionHandler'); -Router::get('/', 'DefaultController@index')->setAlias('home'); -Router::get('/contact', 'DefaultController@contact')->setAlias('contact'); -Router::basic('/companies', 'DefaultController@companies')->setAlias('companies'); -Router::basic('/companies/{id}', 'DefaultController@companies')->setAlias('companies'); +Router::group(['exceptionHandler' => 'Demo\Handlers\CustomExceptionHandler'], function() { + + Router::get('/', 'DefaultController@index')->setAlias('home'); + Router::get('/contact', 'DefaultController@contact')->setAlias('contact'); + Router::get('/404', 'DefaultController@notFound')->setAlias('404'); + Router::basic('/companies', 'DefaultController@companies')->setAlias('companies'); + Router::basic('/companies/{id}', 'DefaultController@companies')->setAlias('companies'); + + // Api + Router::group(['prefix' => '/api', 'middleware' => 'Demo\Middlewares\ApiVerification'], function() { + Router::resource('/demo', 'ApiController'); + }); -// Api -Router::group(['prefix' => '/api', 'middleware' => 'Demo\Middlewares\ApiVerification'], function() { - Router::resource('/demo', 'ApiController'); }); \ No newline at end of file diff --git a/src/Pecee/SimpleRouter/RouterBase.php b/src/Pecee/SimpleRouter/RouterBase.php index ba0612a..dea5400 100644 --- a/src/Pecee/SimpleRouter/RouterBase.php +++ b/src/Pecee/SimpleRouter/RouterBase.php @@ -109,64 +109,68 @@ class RouterBase { } } - public function routeRequest() { + public function routeRequest($original = true) { $originalUri = $this->request->getUri(); - // Initialize boot-managers - if(count($this->bootManagers)) { - /* @var $manager RouterBootManager */ - foreach($this->bootManagers as $manager) { - $this->request = $manager->boot($this->request); + try { - if(!($this->request instanceof Request)) { - throw new RouterException('Custom router bootmanager "'. get_class($manager) .'" must return instance of Request.'); + // Initialize boot-managers + if(count($this->bootManagers)) { + /* @var $manager RouterBootManager */ + foreach($this->bootManagers as $manager) { + $this->request = $manager->boot($this->request); + + if(!($this->request instanceof Request)) { + throw new RouterException('Custom router bootmanager "'. get_class($manager) .'" must return instance of Request.'); + } } } - } - // Verify csrf token for request - if($this->baseCsrfVerifier !== null) { - $this->baseCsrfVerifier->handle($this->request); - } + // Loop through each route-request + $this->processRoutes($this->routes); - // Loop through each route-request - $this->processRoutes($this->routes); - - $routeNotAllowed = false; - - $max = count($this->controllerUrlMap); - - /* @var $route RouterEntry */ - for($i = 0; $i < $max; $i++) { - - $route = $this->controllerUrlMap[$i]; - - $routeMatch = $route->matchRoute($this->request); - - if($routeMatch) { - - if(count($route->getRequestMethods()) && !in_array($this->request->getMethod(), $route->getRequestMethods())) { - $routeNotAllowed = true; - continue; + if($original === true) { + // Verify csrf token for request + if ($this->baseCsrfVerifier !== null) { + $this->baseCsrfVerifier->handle($this->request); } + } - $routeNotAllowed = false; + $routeNotAllowed = false; - $this->request->rewrite_uri = $this->request->uri; - $this->request->setUri($originalUri); + $max = count($this->controllerUrlMap); - $this->request->loadedRoute = $route; - $route->loadMiddleware($this->request); + /* @var $route RouterEntry */ + for ($i = 0; $i < $max; $i++) { + + $route = $this->controllerUrlMap[$i]; + + $routeMatch = $route->matchRoute($this->request); + + if ($routeMatch) { + + if (count($route->getRequestMethods()) && !in_array($this->request->getMethod(), $route->getRequestMethods())) { + $routeNotAllowed = true; + continue; + } + + $routeNotAllowed = false; + + $this->request->rewrite_uri = $this->request->uri; + $this->request->setUri($originalUri); + + $this->request->loadedRoute = $route; + $route->loadMiddleware($this->request); - try { $this->request->loadedRoute->renderRoute($this->request); - } catch(\Exception $e) { - $this->handleException($e); - } - break; + break; + } } + + } catch(\Exception $e) { + $this->handleException($e); } if($routeNotAllowed) { @@ -180,6 +184,8 @@ class RouterBase { protected function handleException(\Exception $e) { + $request = null; + /* @var $route RouterGroup */ foreach ($this->exceptionHandlers as $route) { $route->loadMiddleware($this->request); @@ -190,7 +196,13 @@ class RouterBase { throw new RouterException('Exception handler must implement the IExceptionHandler interface.'); } - $handler->handleError($this->request, $this->request->loadedRoute, $e); + $request = $handler->handleError($this->request, $this->request->loadedRoute, $e); + } + + if($request !== null) { + $this->request = $request; + $this->routeRequest(false); + return; } throw $e; @@ -457,14 +469,14 @@ class RouterBase { } public static function getInstance() { - if(self::$instance === null) { - self::$instance = new static(); + if(static::$instance === null) { + static::$instance = new static(); } - return self::$instance; + return static::$instance; } public static function reset() { - self::$instance = null; + static::$instance = null; } } \ No newline at end of file diff --git a/src/Pecee/SimpleRouter/RouterEntry.php b/src/Pecee/SimpleRouter/RouterEntry.php index 8e52521..39b2d69 100644 --- a/src/Pecee/SimpleRouter/RouterEntry.php +++ b/src/Pecee/SimpleRouter/RouterEntry.php @@ -310,7 +310,7 @@ abstract class RouterEntry { if($max) { for($i = 0; $i < $max; $i++) { $name = $parameterNames[$i]; - $parameterValue = (isset($parameterValues[$name['name']]) && !empty($parameterValues[$name['name']])) ? $parameterValues[$name['name']] : null; + $parameterValue = isset($parameterValues[$name['name']]) ? $parameterValues[$name['name']] : null; if($name['required'] && $parameterValue === null) { throw new RouterException('Missing required parameter ' . $name['name'], 404); diff --git a/test/Dummy/DummyController.php b/test/Dummy/DummyController.php index 019dd0d..a760d74 100644 --- a/test/Dummy/DummyController.php +++ b/test/Dummy/DummyController.php @@ -11,4 +11,8 @@ class DummyController { echo 'Params: ' . join(', ', $params); } + public function notFound() { + echo 'not found'; + } + } \ No newline at end of file diff --git a/test/Dummy/Handler/ExceptionHandler.php b/test/Dummy/Handler/ExceptionHandler.php new file mode 100644 index 0000000..5b415ef --- /dev/null +++ b/test/Dummy/Handler/ExceptionHandler.php @@ -0,0 +1,8 @@ +result = true; } diff --git a/test/MiddlewareTest.php b/test/MiddlewareTest.php index e0c44ba..1ec6b60 100644 --- a/test/MiddlewareTest.php +++ b/test/MiddlewareTest.php @@ -2,29 +2,25 @@ require_once 'Dummy/DummyMiddleware.php'; require_once 'Dummy/DummyController.php'; +require_once 'Dummy/Handler/ExceptionHandler.php'; class MiddlewareTest extends PHPUnit_Framework_TestCase { - public function __construct() { - // Initial setup - $_SERVER['HTTP_HOST'] = 'example.com'; - $_SERVER['REQUEST_URI'] = '/my/test/url'; - $_SERVER['REQUEST_METHOD'] = 'get'; - } - public function testMiddlewareFound() { - - \Pecee\Http\Request::getInstance()->setMethod('get'); - \Pecee\SimpleRouter\RouterBase::reset(); - \Pecee\SimpleRouter\SimpleRouter::get('/my/test/url', 'DummyController@start', ['middleware' => 'DummyMiddleware']); + \Pecee\Http\Request::getInstance()->setMethod('get'); + \Pecee\Http\Request::getInstance()->setUri('/my/test/url'); + + \Pecee\SimpleRouter\SimpleRouter::group(['exceptionHandler' => 'ExceptionHandler'], function() { + \Pecee\SimpleRouter\SimpleRouter::get('/my/test/url', 'DummyController@start', ['middleware' => 'DummyMiddleware']); + }); $found = false; try { \Pecee\SimpleRouter\SimpleRouter::start(); - }catch(Exception $e) { + }catch(\Exception $e) { $found = ($e instanceof MiddlewareLoadedException); } diff --git a/test/RouterRouteTest.php b/test/RouterRouteTest.php index bdac4aa..a98ffb3 100644 --- a/test/RouterRouteTest.php +++ b/test/RouterRouteTest.php @@ -2,14 +2,31 @@ require_once 'Dummy/DummyMiddleware.php'; require_once 'Dummy/DummyController.php'; +require_once 'Dummy/Handler/ExceptionHandler.php'; class RouterRouteTest extends PHPUnit_Framework_TestCase { + + + public function testNotFound() { + \Pecee\SimpleRouter\RouterBase::reset(); + + \Pecee\Http\Request::getInstance()->setMethod('get'); + \Pecee\Http\Request::getInstance()->setUri('/test-param1-param2'); + + \Pecee\SimpleRouter\SimpleRouter::group(['exceptionHandler' => 'ExceptionHandler'], function() { + \Pecee\SimpleRouter\SimpleRouter::get('/non-existing-path', 'DummyController@start'); + }); + + $found = false; + + try { + \Pecee\SimpleRouter\SimpleRouter::start(); + }catch(\Exception $e) { + $found = ($e instanceof \Pecee\Exception\RouterException && $e->getCode() == 404); + } + + $this->assertTrue($found); - public function __construct() { - // Initial setup - $_SERVER['HTTP_HOST'] = 'example.com'; - $_SERVER['REQUEST_URI'] = '/my/test/url'; - $_SERVER['REQUEST_METHOD'] = 'get'; } public function testGet() {