From 73ee4521bc627dd81eda57db08a514ecf4d996e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Sessing=C3=B8?= Date: Thu, 17 Nov 2016 03:50:33 +0100 Subject: [PATCH] Bugfixes - Array arguments are now longer automaticially merged. - Added domain-route parameter unit-test. --- src/Pecee/SimpleRouter/LoadableRoute.php | 10 +- src/Pecee/SimpleRouter/RouterBase.php | 34 +- src/Pecee/SimpleRouter/RouterController.php | 2 +- src/Pecee/SimpleRouter/RouterEntry.php | 370 +++++++++++--------- src/Pecee/SimpleRouter/RouterGroup.php | 120 ++----- src/Pecee/SimpleRouter/RouterResource.php | 4 +- src/Pecee/SimpleRouter/RouterRoute.php | 8 +- src/Pecee/SimpleRouter/SimpleRouter.php | 18 +- test/GroupTest.php | 2 +- test/RouterRouteTest.php | 22 ++ 10 files changed, 303 insertions(+), 287 deletions(-) diff --git a/src/Pecee/SimpleRouter/LoadableRoute.php b/src/Pecee/SimpleRouter/LoadableRoute.php index 0dd1d39..130a0bb 100644 --- a/src/Pecee/SimpleRouter/LoadableRoute.php +++ b/src/Pecee/SimpleRouter/LoadableRoute.php @@ -6,6 +6,7 @@ abstract class LoadableRoute extends RouterEntry implements ILoadableRoute { const PARAMETERS_REGEX_MATCH = '{([A-Za-z\-\_]*?)\?{0,1}}'; protected $url; + protected $alias; public function getUrl() { return $this->url; @@ -45,7 +46,7 @@ abstract class LoadableRoute extends RouterEntry implements ILoadableRoute { * @return string|array */ public function getAlias(){ - return $this->getSetting('alias'); + return $this->alias; } /** @@ -75,19 +76,18 @@ abstract class LoadableRoute extends RouterEntry implements ILoadableRoute { * @return static */ public function setAlias($alias){ - $this->settings['alias'] = $alias; + $this->alias = $alias; return $this; } - public function addSettings(array $settings) { + public function setData(array $settings) { // Change as to alias if(isset($settings['as'])) { $this->setAlias($settings['as']); - unset($settings['as']); } - return parent::addSettings($settings); + return parent::setData($settings); } } \ No newline at end of file diff --git a/src/Pecee/SimpleRouter/RouterBase.php b/src/Pecee/SimpleRouter/RouterBase.php index d08c962..453d3bc 100644 --- a/src/Pecee/SimpleRouter/RouterBase.php +++ b/src/Pecee/SimpleRouter/RouterBase.php @@ -126,7 +126,7 @@ class RouterBase { $route = $routes[$i]; if(count($settings)) { - $route->addSettings($settings); + $route->setData($settings); } if($parent !== null) { @@ -135,15 +135,6 @@ class RouterBase { if ($parent->getPrefix() !== null && trim($parent->getPrefix(), '/') !== '') { $prefixes[] = trim($parent->getPrefix(), '/'); } - - if ($route->matchRoute($this->request)) { - $mergedSettings = array_merge($settings, $parent->getMergeableSettings()); - - // Add ExceptionHandler - if (count($parent->getExceptionHandlers())) { - $this->exceptionHandlers = array_merge($this->exceptionHandlers, $parent->getExceptionHandler()); - } - } } $route->setParent($parent); @@ -161,11 +152,20 @@ class RouterBase { if($route instanceof ILoadableRoute) { $route->setUrl( trim(join('/', $prefixes) . $route->getUrl(), '/') ); $this->controllerUrlMap[] = $route; - } else { + } elseif($route instanceof RouterGroup) { if ($route->getCallback() !== null && is_callable($route->getCallback())) { $this->processingRoute = true; $route->renderRoute($this->request); $this->processingRoute = false; + + if ($route->matchRoute($this->request)) { + $mergedSettings = array_merge($settings, $route->getMergeableData()); + + // Add ExceptionHandler + if (count($route->getExceptionHandlers())) { + $this->exceptionHandlers = array_merge($this->exceptionHandlers, $route->getExceptionHandlers()); + } + } } } @@ -384,15 +384,9 @@ class RouterBase { $parent = $route->getParent(); - if($parent !== null && ($parent instanceof RouterGroup) && $parent->getDomain() !== null) { - if(is_array($parent->getDomain())) { - $domains = $parent->getDomain(); - $domain = array_shift($domains); - } else { - $domain = $parent->getDomain(); - } - - $domain = '//' . $domain; + if($parent !== null && $parent instanceof RouterGroup && count($parent->getDomains())) { + $domain = $parent->getDomains(); + $domain = '//' . $domain[0]; } $url = $domain . '/' . trim($route->getUrl(), '/'); diff --git a/src/Pecee/SimpleRouter/RouterController.php b/src/Pecee/SimpleRouter/RouterController.php index 60a8781..6dd5a28 100644 --- a/src/Pecee/SimpleRouter/RouterController.php +++ b/src/Pecee/SimpleRouter/RouterController.php @@ -57,7 +57,7 @@ class RouterController extends LoadableRoute implements IControllerRoute { $this->method = $method; array_shift($path); - $this->settings['parameters'] = $path; + $this->parameters = $path; // Set callback $this->setCallback($this->controller . '@' . $this->method); diff --git a/src/Pecee/SimpleRouter/RouterEntry.php b/src/Pecee/SimpleRouter/RouterEntry.php index 8ed83e5..40b8570 100644 --- a/src/Pecee/SimpleRouter/RouterEntry.php +++ b/src/Pecee/SimpleRouter/RouterEntry.php @@ -8,175 +8,31 @@ use Pecee\Http\Request; abstract class RouterEntry { - const REQUEST_TYPE_POST = 'post'; const REQUEST_TYPE_GET = 'get'; + const REQUEST_TYPE_POST = 'post'; const REQUEST_TYPE_PUT = 'put'; const REQUEST_TYPE_PATCH = 'patch'; + const REQUEST_TYPE_OPTIONS = 'options'; const REQUEST_TYPE_DELETE = 'delete'; public static $allowedRequestTypes = [ - self::REQUEST_TYPE_DELETE, self::REQUEST_TYPE_GET, self::REQUEST_TYPE_POST, self::REQUEST_TYPE_PUT, self::REQUEST_TYPE_PATCH, + self::REQUEST_TYPE_OPTIONS, + self::REQUEST_TYPE_DELETE, ]; protected $parent; protected $callback; - protected $settings = [ - 'requestMethods' => array(), - 'where' => array(), - 'parameters' => array(), - 'middleware' => array(), - ]; - - /** - * @param string $callback - * @return static - */ - public function setCallback($callback) { - $this->callback = $callback; - return $this; - } - - /** - * @return mixed - */ - public function getCallback() { - return $this->callback; - } - - public function getMethod() { - if(strpos($this->callback, '@') !== false) { - $tmp = explode('@', $this->callback); - return $tmp[1]; - } - return null; - } - - public function getClass() { - if(strpos($this->callback, '@') !== false) { - $tmp = explode('@', $this->callback); - return $tmp[0]; - } - return null; - } - - public function setMethod($method) { - $this->callback = sprintf('%s@%s', $this->getClass(), $method); - return $this; - } - - public function setClass($class) { - $this->callback = sprintf('%s@%s', $class, $this->getMethod()); - return $this; - } - - /** - * @param string $middleware - * @return static - */ - public function setMiddleware($middleware) { - $this->settings['middleware'][] = $middleware; - return $this; - } - - /** - * @param string $namespace - * @return static - */ - public function setNamespace($namespace) { - $this->settings['namespace'] = $namespace; - return $this; - } - - /** - * @return string|array - */ - public function getMiddleware() { - return $this->getSetting('middleware'); - } - - /** - * @return string - */ - public function getNamespace() { - return $this->getSetting('namespace'); - } - - /** - * @return array - */ - public function getSettings() { - return $this->settings; - } - - /** - * @return array - */ - public function getParameters(){ - return $this->getSetting('parameters', array()); - } - - /** - * @param mixed $parameters - * @return static - */ - public function setParameters($parameters) { - $this->settings['parameters'] = $parameters; - return $this; - } - - /** - * Add regular expression parameter match - * - * @param array $options - * @return static - */ - public function where(array $options) { - $this->settings['where'] = array_merge($this->settings['where'], $options); - return $this; - } - - /** - * Add regular expression match for url - * - * @param string $regex - * @return static - */ - public function match($regex) { - $this->settings['regexMatch'] = $regex; - return $this; - } - - /** - * Get settings that are allowed to be inherited by child routes. - * - * @return array - */ - public function getMergeableSettings() { - return $this->settings; - } - - /** - * @param array $settings - * @return static - */ - public function addSettings(array $settings) { - $this->settings = array_merge($this->settings, $settings); - return $this; - } - - /** - * @param array $settings - * @return static - */ - public function setSettings($settings) { - $this->settings = $settings; - return $this; - } + protected $namespace; + protected $regex; + protected $requestMethods = array(); + protected $where = array(); + protected $parameters = array(); + protected $middlewares = array(); protected function loadClass($name) { if(!class_exists($name)) { @@ -210,8 +66,8 @@ abstract class RouterEntry { // Check for optional parameter // Use custom parameter regex if it exists - if(is_array($this->getSetting('where')) && isset($this->settings['where'][$parameter])) { - $parameterRegex = $this->settings['where'][$parameter]; + if(is_array($this->where) && isset($this->where[$parameter])) { + $parameterRegex = $this->where[$parameter]; } if($lastCharacter === '?') { @@ -271,8 +127,8 @@ abstract class RouterEntry { } public function loadMiddleware(Request $request, RouterEntry &$route) { - if(count($this->getMiddleware())) { - foreach($this->getMiddleware() as $middleware) { + if(count($this->getMiddlewares())) { + foreach($this->getMiddlewares() as $middleware) { $middleware = $this->loadClass($middleware); if (!($middleware instanceof IMiddleware)) { throw new RouterException($middleware . ' must be instance of Middleware'); @@ -319,7 +175,7 @@ abstract class RouterEntry { * @return static $this */ public function setRequestMethods(array $methods) { - $this->settings['requestMethods'] = $methods; + $this->requestMethods = $methods; return $this; } @@ -329,7 +185,7 @@ abstract class RouterEntry { * @return array */ public function getRequestMethods() { - return $this->getSetting('requestMethods'); + return $this->requestMethods; } /** @@ -339,13 +195,203 @@ abstract class RouterEntry { return $this->parent; } + /** + * Set parent route + * @param RouterEntry $parent + * @return static $this + */ public function setParent(RouterEntry $parent) { $this->parent = $parent; return $this; } - protected function getSetting($name, $defaultValue = null) { - return isset($this->settings[$name]) ? $this->settings[$name] : $defaultValue; + /** + * @param string $callback + * @return static + */ + public function setCallback($callback) { + $this->callback = $callback; + return $this; + } + + /** + * @return mixed + */ + public function getCallback() { + return $this->callback; + } + + public function getMethod() { + if(strpos($this->callback, '@') !== false) { + $tmp = explode('@', $this->callback); + return $tmp[1]; + } + return null; + } + + public function getClass() { + if(strpos($this->callback, '@') !== false) { + $tmp = explode('@', $this->callback); + return $tmp[0]; + } + return null; + } + + public function setMethod($method) { + $this->callback = sprintf('%s@%s', $this->getClass(), $method); + return $this; + } + + public function setClass($class) { + $this->callback = sprintf('%s@%s', $class, $this->getMethod()); + return $this; + } + + /** + * @param string $middleware + * @return static + */ + public function setMiddleware($middleware) { + $this->middlewares[] = $middleware; + return $this; + } + + public function setMiddlewares(array $middlewares) { + $this->middlewares = $middlewares; + return $this; + } + + /** + * @param string $namespace + * @return static + */ + public function setNamespace($namespace) { + $this->namespace = $namespace; + return $this; + } + + /** + * @return string|array + */ + public function getMiddlewares() { + return $this->middlewares; + } + + /** + * @return string + */ + public function getNamespace() { + return $this->namespace; + } + + /** + * @return array + */ + public function getParameters(){ + return $this->parameters; + } + + /** + * @param mixed $parameters + * @return static + */ + public function setParameters($parameters) { + $this->parameters = $parameters; + return $this; + } + + /** + * Add regular expression parameter match + * + * @param array $options + * @return static + */ + public function where(array $options) { + $this->where = $options; + return $this; + } + + /** + * Add regular expression match for url + * + * @param string $regex + * @return static + */ + public function match($regex) { + $this->regex = $regex; + return $this; + } + + /** + * Get arguments that can be inherited by child routes. + * + * @return array + */ + public function getMergeableData() { + + $output = [ + 'namespace' => $this->namespace, + ]; + + if(count($this->middlewares)) { + $output['middleware'] = $this->middlewares; + } + + if(count($this->where)) { + $output['where'] = $this->where; + } + + if(count($this->requestMethods)) { + $output['method'] = $this->requestMethods; + } + + if(count($this->parameters)) { + $output['parameters'] = $this->parameters; + } + + return $output; + } + + /** + * Set arguments/data by array + * + * @param array $settings + * @return static + */ + public function setData(array $settings) { + + if (isset($settings['namespace'])) { + $this->setNamespace($settings['namespace']); + } + + // Push middleware if multiple + if (isset($settings['middleware'])) { + + if (!is_array($settings['middleware'])) { + $settings['middleware'] = array_merge($this->middlewares, array($settings['middleware'])); + } else { + $settings['middleware'][] = $this->middlewares; + } + + $middlewares = is_array($settings['middleware']) ? $settings['middleware'] : array($settings['middleware']); + $this->middlewares = array_reverse(array_merge($this->middlewares, $middlewares)); + + } + + if(isset($settings['method'])) { + $requestMethods = is_array($settings['method']) ? $settings['method'] : array($settings['method']); + $this->setRequestMethods($requestMethods); + } + + if(isset($settings['where'])) { + $this->where($settings['where']); + } + + if(isset($settings['parameters'])) { + $this->setParameters($settings['parameters']); + } + + return $this; } abstract function matchRoute(Request $request); diff --git a/src/Pecee/SimpleRouter/RouterGroup.php b/src/Pecee/SimpleRouter/RouterGroup.php index 23d149e..50a385d 100644 --- a/src/Pecee/SimpleRouter/RouterGroup.php +++ b/src/Pecee/SimpleRouter/RouterGroup.php @@ -2,41 +2,25 @@ namespace Pecee\SimpleRouter; -use Pecee\Exception\RouterException; use Pecee\Http\Request; class RouterGroup extends RouterEntry { - public function __construct() { - $this->settings = array_merge($this->settings, [ - 'exceptionHandlers' => array() - ]); - } + protected $prefix; + protected $domains = array(); + protected $exceptionHandlers = array(); public function matchDomain(Request $request) { - if($this->getSetting('domain') !== null) { + if(count($this->domains)) { + for($i = 0; $i < count($this->domains); $i++) { + $domain = $this->domains[$i]; - if(is_array($this->getSetting('domain'))) { + $parameters = $this->parseParameters($domain, $request->getHost(), '.*'); - for($i = 0; $i < count($this->getSetting('domain')); $i++) { - $domain = $this->settings['domain'][$i]; - - $parameters = $this->parseParameters($domain, $request->getHost(), '[^.]*'); - - if($parameters !== null) { - $this->settings['parameters'] = $parameters; - return true; - } + if($parameters !== null) { + $this->parameters = $parameters; + return true; } - - return false; - } - - $parameters = $this->parseParameters($this->getSetting('domain'), $request->getHost(), '[^.]*'); - - if ($parameters !== null) { - $this->settings['parameters'] = $parameters; - return true; } return false; @@ -45,47 +29,31 @@ class RouterGroup extends RouterEntry { return true; } - public function renderRoute(Request $request) { - // Check if request method is allowed - $hasAccess = true; - - if($this->getSetting('method') !== null) { - if(is_array($this->getSetting('method'))) { - $hasAccess = (in_array($request->getMethod(), $this->getRequestMethods())); - } else { - $hasAccess = strtolower($this->getRequestMethods()) == strtolower($request->getMethod()); - } - } - - if(!$hasAccess) { - throw new RouterException('Method not allowed'); - } - - $this->matchDomain($request); - - return parent::renderRoute($request); - } - public function matchRoute(Request $request) { // Skip if prefix doesn't match - if($this->getPrefix() !== null && stripos($request->getUri(), $this->getPrefix()) === false) { + if($this->prefix !== null && stripos($request->getUri(), $this->prefix) === false) { return false; } return $this->matchDomain($request); } - public function setExceptionHandlers($class) { - $this->settings['exceptionHandlers'][] = $class; + public function setExceptionHandlers(array $handlers) { + $this->exceptionHandlers = $handlers; return $this; } public function getExceptionHandlers() { - return $this->getSetting('exceptionHandlers'); + return $this->exceptionHandlers; } - public function getDomain() { - return $this->getSetting('domain'); + public function getDomains() { + return $this->domains; + } + + public function setDomains(array $domains) { + $this->domains = $domains; + return $this; } /** @@ -93,7 +61,7 @@ class RouterGroup extends RouterEntry { * @return static */ public function setPrefix($prefix) { - $this->settings['prefix'] = '/' . trim($prefix, '/'); + $this->prefix = '/' . trim($prefix, '/'); return $this; } @@ -101,48 +69,26 @@ class RouterGroup extends RouterEntry { * @return string */ public function getPrefix() { - return $this->getSetting('prefix'); + return $this->prefix; } - /** - * @param array $settings - * @return static - */ - public function addSettings(array $settings) { - - if ($this->getNamespace() !== null && isset($settings['namespace'])) { - unset($settings['namespace']); - } - - // Push middleware if multiple - if ($this->getMiddleware() !== null && isset($settings['middleware'])) { - - if (!is_array($settings['middleware'])) { - $settings['middleware'] = array_merge($this->getMiddleware(), array($settings['middleware'])); - } else { - $settings['middleware'][] = $this->getMiddleware(); - } - - $settings['middleware'] = array_unique(array_reverse($settings['middleware'])); - } + public function setData(array $settings) { if(isset($settings['prefix'])) { $this->setPrefix($settings['prefix']); - unset($settings['prefix']); } - $this->settings = array_merge($this->settings, $settings); - return $this; - } - - public function getMergeableSettings() { - $settings = $this->settings; - - if(isset($settings['prefix'])) { - unset($settings['prefix']); + if(isset($settings['exceptionHandler'])) { + $handlers = is_array($settings['exceptionHandler']) ? $settings['exceptionHandler'] : array($settings['exceptionHandler']); + $this->setExceptionHandlers($handlers); } - return $settings; + if(isset($settings['domain'])) { + $domains = is_array($settings['domain']) ? $settings['domain'] : array($settings['domain']); + $this->setDomains($domains); + } + + return parent::setData($settings); } } \ No newline at end of file diff --git a/src/Pecee/SimpleRouter/RouterResource.php b/src/Pecee/SimpleRouter/RouterResource.php index 9b2aba6..a0f596a 100644 --- a/src/Pecee/SimpleRouter/RouterResource.php +++ b/src/Pecee/SimpleRouter/RouterResource.php @@ -38,7 +38,7 @@ class RouterResource extends LoadableRoute implements IControllerRoute { protected function call($method, $parameters) { $this->setCallback($this->controller . '@' . $method); - $this->settings['parameters'] = $parameters; + $this->parameters = $parameters; return true; } @@ -53,7 +53,7 @@ class RouterResource extends LoadableRoute implements IControllerRoute { if($parameters !== null) { if(is_array($parameters)) { - $parameters = array_merge($this->settings['parameters'], $parameters); + $parameters = array_merge($this->parameters, $parameters); } $action = isset($parameters['action']) ? $parameters['action'] : null; diff --git a/src/Pecee/SimpleRouter/RouterRoute.php b/src/Pecee/SimpleRouter/RouterRoute.php index 74bf74c..22a17af 100644 --- a/src/Pecee/SimpleRouter/RouterRoute.php +++ b/src/Pecee/SimpleRouter/RouterRoute.php @@ -17,10 +17,10 @@ class RouterRoute extends LoadableRoute { $url = rtrim($url, '/') . '/'; // Match on custom defined regular expression - if($this->getSetting('regexMatch') !== null) { + if($this->regex !== null) { $parameters = array(); - if(preg_match('/(' . $this->getSetting('regexMatch') . ')/is', $request->getHost() . $url, $parameters)) { - $this->settings['parameters'] = (!is_array($parameters[0]) ? array($parameters[0]) : $parameters[0]); + if(preg_match('/(' . $this->regex . ')/is', $request->getHost() . $url, $parameters)) { + $this->parameters = (!is_array($parameters[0]) ? array($parameters[0]) : $parameters[0]); return true; } return null; @@ -32,7 +32,7 @@ class RouterRoute extends LoadableRoute { $parameters = $this->parseParameters($route, $url); if($parameters !== null) { - $this->settings['parameters'] = array_merge($this->getSetting('parameters'), $parameters); + $this->parameters = array_merge($this->parameters, $parameters); return true; } diff --git a/src/Pecee/SimpleRouter/SimpleRouter.php b/src/Pecee/SimpleRouter/SimpleRouter.php index 352e69d..c2760c4 100644 --- a/src/Pecee/SimpleRouter/SimpleRouter.php +++ b/src/Pecee/SimpleRouter/SimpleRouter.php @@ -53,6 +53,14 @@ class SimpleRouter { return static::match(['put'], $url, $callback, $settings); } + public static function patch($url, $callback, array $settings = null) { + return static::match(['patch'], $url, $callback, $settings); + } + + public static function options($url, $callback, array $settings = null) { + return static::match(['options'], $url, $callback, $settings); + } + public static function delete($url, $callback, array $settings = null) { return static::match(['delete'], $url, $callback, $settings); } @@ -62,7 +70,7 @@ class SimpleRouter { $group->setCallback($callback); if($settings !== null && is_array($settings)) { - $group->setSettings($settings); + $group->setData($settings); } RouterBase::getInstance()->addRoute($group); @@ -87,7 +95,7 @@ class SimpleRouter { $route->setRequestMethods($requestMethods); if($settings !== null) { - $route->addSettings($settings); + $route->setData($settings); } RouterBase::getInstance()->addRoute($route); @@ -99,7 +107,7 @@ class SimpleRouter { $route = new RouterRoute($url, $callback); if($settings !== null) { - $route->addSettings($settings); + $route->setData($settings); } RouterBase::getInstance()->addRoute($route); @@ -111,7 +119,7 @@ class SimpleRouter { $route = new RouterController($url, $controller); if($settings !== null) { - $route->addSettings($settings); + $route->setData($settings); } RouterBase::getInstance()->addRoute($route); @@ -123,7 +131,7 @@ class SimpleRouter { $route = new RouterResource($url, $controller); if($settings !== null) { - $route->addSettings($settings); + $route->setData($settings); } static::router()->addRoute($route); diff --git a/test/GroupTest.php b/test/GroupTest.php index 9b3001e..3093c57 100644 --- a/test/GroupTest.php +++ b/test/GroupTest.php @@ -18,7 +18,7 @@ class GroupTest extends PHPUnit_Framework_TestCase { try { \Pecee\SimpleRouter\SimpleRouter::start(); } catch(Exception $e) { - echo $e->getMessage(); + // ignore RouteNotFound exception } $this->assertTrue($this->result); diff --git a/test/RouterRouteTest.php b/test/RouterRouteTest.php index e0ae29d..a8dbf96 100644 --- a/test/RouterRouteTest.php +++ b/test/RouterRouteTest.php @@ -6,6 +6,7 @@ require_once 'Dummy/Handler/ExceptionHandler.php'; class RouterRouteTest extends PHPUnit_Framework_TestCase { + protected $result = false; public function testNotFound() { \Pecee\SimpleRouter\RouterBase::getInstance()->reset(); @@ -113,4 +114,25 @@ class RouterRouteTest extends PHPUnit_Framework_TestCase { } + public function testDomainRoute() { + + \Pecee\SimpleRouter\RouterBase::getInstance()->reset(); + \Pecee\SimpleRouter\SimpleRouter::request()->setMethod('get'); + \Pecee\SimpleRouter\SimpleRouter::request()->setUri('/test'); + \Pecee\SimpleRouter\SimpleRouter::request()->setHost('hello.world.com'); + + $this->result = false; + + \Pecee\SimpleRouter\SimpleRouter::group(['domain' => '{subdomain}.world.com'], function() { + \Pecee\SimpleRouter\SimpleRouter::get('test', function($subdomain) { + $this->result = ($subdomain === 'hello'); + }); + }); + + \Pecee\SimpleRouter\SimpleRouter::start(); + + $this->assertTrue($this->result); + + } + } \ No newline at end of file