mirror of
https://github.com/Combodo/iTop.git
synced 2026-02-13 07:24:13 +01:00
N°5655 - Router: Improve robustness
* Change visibility from public to protected of methods for which it was not necessary * Change visibility from static to non-static of methods that could be used as such for easier testability * Cache available routes for better performances * Finish Router::GetRoutes() so we can use the available routes data to display them to the developers somewhere (ideally in the toolkit) * Improve unit tests
This commit is contained in:
Molkobain
@@ -10,6 +10,7 @@ use Combodo\iTop\Service\Router\Exception\RouteNotFoundException;
|
||||
use ReflectionClass;
|
||||
use ReflectionMethod;
|
||||
use utils;
|
||||
use SetupUtils;
|
||||
|
||||
/**
|
||||
* Class Router
|
||||
@@ -29,7 +30,7 @@ class Router
|
||||
/**
|
||||
* @return $this The singleton instance of the router
|
||||
*/
|
||||
public static function GetInstance()
|
||||
public static function GetInstance(): Router
|
||||
{
|
||||
if (null === static::$oSingleton) {
|
||||
static::$oSingleton = new static();
|
||||
@@ -38,44 +39,6 @@ class Router
|
||||
return static::$oSingleton;
|
||||
}
|
||||
|
||||
/**
|
||||
* @return array{0: string, 1: string} Array of available routes namespaces and their corresponding controllers (eg. ['object' => '\Combodo\iTop\Controller\Base\Layout\ObjectController', ...])
|
||||
*/
|
||||
public static function GetRoutesNamespaces(): array
|
||||
{
|
||||
$aRoutesNamespaces = [];
|
||||
foreach (utils::GetClassesForInterface('Combodo\iTop\Controller\iController', '', ['[\\\\/]lib[\\\\/]', '[\\\\/]node_modules[\\\\/]', '[\\\\/]test[\\\\/]']) as $sControllerFQCN) {
|
||||
$aRoutesNamespaces[$sControllerFQCN::ROUTE_NAMESPACE] = $sControllerFQCN;
|
||||
}
|
||||
|
||||
return $aRoutesNamespaces;
|
||||
}
|
||||
|
||||
/**
|
||||
* @return array{0: string, 1: string} Array of available routes and their corresponding controllers (eg. ['object.modify' => '\Combodo\iTop\Controller\Base\Layout\ObjectController::OperationModify', ...])
|
||||
* @throws \ReflectionException
|
||||
*/
|
||||
public static function GetRoutes(): array
|
||||
{
|
||||
$aRoutes = [];
|
||||
foreach (static::GetRoutesNamespaces() as $sRouteNamespace => $sRouteControllerFQCN) {
|
||||
$oReflectionClass = new ReflectionClass($sRouteControllerFQCN);
|
||||
foreach ($oReflectionClass->getMethods(ReflectionMethod::IS_PUBLIC) as $oReflectionMethod) {
|
||||
// Ignore non "operation" methods
|
||||
$sPrefix = 'Operation';
|
||||
$iPos = stripos($oReflectionMethod->name, $sPrefix);
|
||||
if ($iPos !== 0) {
|
||||
continue;
|
||||
}
|
||||
|
||||
$sOperationName = substr($oReflectionMethod->name, $iPos + strlen($sPrefix));
|
||||
$aRoutes[$sRouteNamespace.'.'.utils::ToSnakeCase($sOperationName)] = $sRouteControllerFQCN.'::'.$oReflectionMethod->name;
|
||||
}
|
||||
}
|
||||
|
||||
return $aRoutes;
|
||||
}
|
||||
|
||||
/**********************/
|
||||
/* Non-static methods */
|
||||
/**********************/
|
||||
@@ -85,7 +48,7 @@ class Router
|
||||
*
|
||||
* @return void
|
||||
*/
|
||||
private function __construct()
|
||||
protected function __construct()
|
||||
{
|
||||
// Don't do anything, we don't want to be initialized
|
||||
}
|
||||
@@ -149,31 +112,104 @@ class Router
|
||||
return $mResponse;
|
||||
}
|
||||
|
||||
/**
|
||||
* @return array{0: string, 1: array{
|
||||
* namespace: string,
|
||||
* operation: string,
|
||||
* controller: string,
|
||||
* description: string
|
||||
* }
|
||||
* } Array of available routes and their corresponding controllers (eg. [
|
||||
* 'object.modify' => [ // Complete route code
|
||||
* 'namespace' => 'object', // Route namespace
|
||||
* 'operation' => 'modify', // Route operation
|
||||
* 'controller' => '\Combodo\iTop\Controller\Base\Layout\ObjectController::OperationModify', // FQCN of the controller/method that handle the route
|
||||
* 'description' => 'Handles display of a modification form for a datamodel object' // Text description of the route
|
||||
* ],
|
||||
* ...
|
||||
* ])
|
||||
* @throws \ReflectionException
|
||||
*/
|
||||
public function GetRoutes(): array
|
||||
{
|
||||
$aRoutes = [];
|
||||
$bUseCache = false === utils::IsDevelopmentEnvironment();
|
||||
$sCacheFilePath = $this->GetCacheFileAbsPath();
|
||||
|
||||
// Try to read from cache
|
||||
if ($bUseCache) {
|
||||
if (is_file($sCacheFilePath)) {
|
||||
$aRoutes = include $sCacheFilePath;
|
||||
}
|
||||
}
|
||||
|
||||
// If no cache, force to re-scan for routes
|
||||
if (count($aRoutes) === 0) {
|
||||
foreach (utils::GetClassesForInterface('Combodo\iTop\Controller\iController', '', ['[\\\\/]lib[\\\\/]', '[\\\\/]node_modules[\\\\/]', '[\\\\/]test[\\\\/]']) as $sControllerFQCN) {
|
||||
$sRouteNamespace = $sControllerFQCN::ROUTE_NAMESPACE;
|
||||
// Ignore controller with no namespace
|
||||
if (is_null($sRouteNamespace)) {
|
||||
continue;
|
||||
}
|
||||
|
||||
$oReflectionClass = new ReflectionClass($sControllerFQCN);
|
||||
foreach ($oReflectionClass->getMethods(ReflectionMethod::IS_PUBLIC) as $oReflectionMethod) {
|
||||
// Ignore non "operation" methods
|
||||
$sPrefix = 'Operation';
|
||||
$iPos = stripos($oReflectionMethod->name, $sPrefix);
|
||||
if ($iPos !== 0) {
|
||||
continue;
|
||||
}
|
||||
|
||||
// eg. "OperationDoSomething"
|
||||
$sMethodName = $oReflectionMethod->name;
|
||||
// eg. "do_something"
|
||||
$sRouteOperation = utils::ToSnakeCase(substr($oReflectionMethod->name, $iPos + strlen($sPrefix)));
|
||||
|
||||
$aRoutes[$sRouteNamespace . '.' . $sRouteOperation] = [
|
||||
'namespace' => $sRouteNamespace,
|
||||
'operation' => $sRouteOperation,
|
||||
'controller' => $sControllerFQCN . '::' . $sMethodName,
|
||||
'description' => $oReflectionMethod->getDocComment(),
|
||||
];
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Save to cache
|
||||
if ($bUseCache) {
|
||||
$sCacheContent = "<?php\n\nreturn ".var_export($aRoutes, true).";";
|
||||
SetupUtils::builddir(dirname($sCacheFilePath));
|
||||
file_put_contents($sCacheFilePath, $sCacheContent);
|
||||
}
|
||||
|
||||
return $aRoutes;
|
||||
}
|
||||
|
||||
/**
|
||||
* @param string $sRoute
|
||||
*
|
||||
* @return array{sControllerFQCN, sOperationMethodName}|null The FQCN controller and operation method matching $sRoute, null if no matching handler
|
||||
*/
|
||||
public function GetDispatchSpecsForRoute(string $sRoute)
|
||||
protected function GetDispatchSpecsForRoute(string $sRoute)
|
||||
{
|
||||
$aRouteParts = $this->GetRouteParts($sRoute);
|
||||
if (is_null($aRouteParts)) {
|
||||
return null;
|
||||
}
|
||||
|
||||
$sRouteNamespace = $aRouteParts['namespace'];
|
||||
$sRouteOperation = $aRouteParts['operation'];
|
||||
$sControllerFQCN = $this->FindControllerFromRouteNamespace($sRouteNamespace);
|
||||
if (utils::IsNullOrEmptyString($sControllerFQCN)) {
|
||||
$sRouteHandlerFQCN = $this->FindHandlerFromRoute($sRoute);
|
||||
if (utils::IsNullOrEmptyString($sRouteHandlerFQCN)) {
|
||||
return null;
|
||||
}
|
||||
|
||||
$sOperationMethodName = $this->MakeOperationMethodNameFromOperation($sRouteOperation);
|
||||
if (false === method_exists($sControllerFQCN, $sOperationMethodName)) {
|
||||
// Extract controller and method names
|
||||
$aParts = explode('::', $sRouteHandlerFQCN);
|
||||
if (count($aParts) !== 2) {
|
||||
return null;
|
||||
}
|
||||
|
||||
return [$sControllerFQCN, $sOperationMethodName];
|
||||
return [$aParts[0], $aParts[1]];
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -181,7 +217,7 @@ class Router
|
||||
*
|
||||
* @return array{namespace: string, operation: string}|null Route parts (namespace and operation) if route can be parsed, null otherwise
|
||||
*/
|
||||
public function GetRouteParts(string $sRoute)
|
||||
protected function GetRouteParts(string $sRoute)
|
||||
{
|
||||
if (utils::IsNullOrEmptyString($sRoute)) {
|
||||
return null;
|
||||
@@ -201,7 +237,7 @@ class Router
|
||||
*
|
||||
* @return string|null Namespace of the route (eg. "object" for "object.modify") if route can be parsed null otherwise
|
||||
*/
|
||||
public function GetRouteNamespace(string $sRoute): ?string
|
||||
protected function GetRouteNamespace(string $sRoute): ?string
|
||||
{
|
||||
$mSeparatorPos = strripos($sRoute, '.', -1);
|
||||
if (false === $mSeparatorPos) {
|
||||
@@ -216,7 +252,7 @@ class Router
|
||||
*
|
||||
* @return string|null Operation of the route (eg. "modify" for "object.modify") if route can be parsed null otherwise
|
||||
*/
|
||||
public function GetRouteOperation(string $sRoute): ?string
|
||||
protected function GetRouteOperation(string $sRoute): ?string
|
||||
{
|
||||
$mSeparatorPos = strripos($sRoute, '.', -1);
|
||||
if (false === $mSeparatorPos) {
|
||||
@@ -227,28 +263,23 @@ class Router
|
||||
}
|
||||
|
||||
/**
|
||||
* @param string $sRouteNamespace {@see static::$sRouteNamespace}
|
||||
* @param string $sRouteToFind Route (eg. 'object.modify') to find the matching controler for
|
||||
*
|
||||
* @return string|null The FQCN of the controller matching the $sRouteNamespace, null if none matching.
|
||||
* @return string|null The FQCN of the handler (controller class + operation, eg. "\Combodo\iTop\Controller\Base\Layout\ObjectController::OperationModify) matching $sRouteNamespace, null if none matching.
|
||||
*/
|
||||
protected function FindControllerFromRouteNamespace(string $sRouteNamespace): ?string
|
||||
protected function FindHandlerFromRoute(string $sRouteToFind): ?string
|
||||
{
|
||||
foreach (static::GetRoutesNamespaces() as $sControllerRouteNamespace => $sControllerFQCN) {
|
||||
if ($sControllerRouteNamespace === $sRouteNamespace) {
|
||||
return $sControllerFQCN;
|
||||
foreach ($this->GetRoutes() as $sRoute => $aRouteData) {
|
||||
if ($sRoute === $sRouteToFind) {
|
||||
return $aRouteData['controller'];
|
||||
}
|
||||
}
|
||||
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* @param string $sOperation
|
||||
*
|
||||
* @return string The method name for the $sOperation regarding the convention
|
||||
*/
|
||||
protected function MakeOperationMethodNameFromOperation(string $sOperation): string
|
||||
protected function GetCacheFileAbsPath(): string
|
||||
{
|
||||
return 'Operation'.utils::ToCamelCase($sOperation);
|
||||
return utils::GetCachePath().'router/available-routes.php';
|
||||
}
|
||||
}
|
||||
@@ -124,29 +124,13 @@ class RouterTest extends ItopTestCase
|
||||
*
|
||||
* @return void
|
||||
*/
|
||||
// public function testCanDispatchRoute(string $sRoute, $bExpectedResult): void
|
||||
// {
|
||||
// $oRouter = Router::GetInstance();
|
||||
// $bTestedResult = $oRouter->CanDispatchRoute($sRoute);
|
||||
//
|
||||
// $sRouteNamespace = $oRouter->GetRouteNamespace($sRoute);
|
||||
// $sRouteOperation = $oRouter->GetRouteOperation($sRoute);
|
||||
// $aRouteParts = $oRouter->GetRouteParts($sRoute);
|
||||
// $sControllerFQCN = $this->InvokeNonPublicMethod(get_class($oRouter), 'FindControllerFromRouteNamespace', $oRouter, ['object']);
|
||||
// $sMethodName = $this->InvokeNonPublicMethod(get_class($oRouter), 'MakeOperationMethodNameFromOperation', $oRouter, ['modify']);
|
||||
// $aDispatchSpecs = $oRouter->GetDispatchSpecsForRoute($sRoute);
|
||||
//
|
||||
//$this->debug($sRoute);
|
||||
//$this->debug($sRouteNamespace);
|
||||
//$this->debug($sRouteOperation);
|
||||
//$this->debug($aRouteParts);
|
||||
//$this->debug($sControllerFQCN);
|
||||
//$this->debug($sMethodName);
|
||||
//$this->debug(is_callable([$sControllerFQCN, $sMethodName]) ? 'true' : 'false');
|
||||
//$this->debug($aDispatchSpecs);
|
||||
//$this->debug($bTestedResult);
|
||||
// $this->assertEquals($bExpectedResult, $bTestedResult, "Dispatch capability for '$sRoute' was not the expected one. Got ".var_export($bTestedResult, true).", expected ".var_export($bExpectedResult, true));
|
||||
// }
|
||||
public function testCanDispatchRoute(string $sRoute, $bExpectedResult): void
|
||||
{
|
||||
$oRouter = Router::GetInstance();
|
||||
$bTestedResult = $oRouter->CanDispatchRoute($sRoute);
|
||||
|
||||
$this->assertEquals($bExpectedResult, $bTestedResult, "Dispatch capability for '$sRoute' was not the expected one. Got ".var_export($bTestedResult, true).", expected ".var_export($bExpectedResult, true));
|
||||
}
|
||||
|
||||
public function CanDispatchRouteProvider(): array
|
||||
{
|
||||
@@ -166,6 +150,84 @@ class RouterTest extends ItopTestCase
|
||||
];
|
||||
}
|
||||
|
||||
/**
|
||||
* @dataProvider GetRoutesProvider
|
||||
* @covers \Combodo\iTop\Service\Router\Router::GetRoutes
|
||||
*
|
||||
* @param string $sRoute
|
||||
* @param bool $bShouldBePresent
|
||||
*
|
||||
* @return void
|
||||
* @throws \ReflectionException
|
||||
*/
|
||||
public function testGetRoutes(string $sRoute, bool $bShouldBePresent): void
|
||||
{
|
||||
$oRouter = Router::GetInstance();
|
||||
$aTestedRoutes = $this->InvokeNonPublicMethod(Router::class, 'GetRoutes', $oRouter, []);
|
||||
|
||||
$bIsPresent = array_key_exists($sRoute, $aTestedRoutes);
|
||||
$this->assertEquals($bShouldBePresent, $bIsPresent, "Route '$sRoute' was not expected amongst the available routes.");
|
||||
}
|
||||
|
||||
public function GetRoutesProvider(): array
|
||||
{
|
||||
return [
|
||||
'Valid route' => [
|
||||
'object.modify',
|
||||
true,
|
||||
],
|
||||
// eg. a route from a controller that has no ROUTE_NAMESPACE
|
||||
'Invalid route' => [
|
||||
'.save_state',
|
||||
false,
|
||||
],
|
||||
];
|
||||
}
|
||||
|
||||
/**
|
||||
* @dataProvider GetRoutePartsProvider
|
||||
* @covers \Combodo\iTop\Service\Router\Router::GetRouteParts
|
||||
*
|
||||
* @param string $sRoute
|
||||
* @param array|null $aExpectedParts
|
||||
*
|
||||
* @return void
|
||||
*/
|
||||
public function testGetRouteParts(string $sRoute, ?array $aExpectedParts): void
|
||||
{
|
||||
$oRouter = Router::GetInstance();
|
||||
$aTestedParts = $this->InvokeNonPublicMethod(Router::class, 'GetRouteParts', $oRouter, [$sRoute]);
|
||||
|
||||
$this->assertEquals($aExpectedParts, $aTestedParts, "Parts found for '$sRoute' were not the expected ones. Got '".print_r($aTestedParts, true)."', expected '".print_r($aExpectedParts, true)."'.");
|
||||
}
|
||||
|
||||
public function GetRoutePartsProvider(): array
|
||||
{
|
||||
return [
|
||||
'Empty route' => [
|
||||
'',
|
||||
null,
|
||||
],
|
||||
// eg. controller implmenting iController but without the ROUTE_NAMESPACE content. This is for BC compatibility
|
||||
'Route with no namespace' => [
|
||||
'.some_operation',
|
||||
null,
|
||||
],
|
||||
'Route with no operation' => [
|
||||
'some_namespace.',
|
||||
null,
|
||||
],
|
||||
'Valid route' => [
|
||||
'some_namespace.some_operation',
|
||||
['namespace' => 'some_namespace', 'operation' => 'some_operation'],
|
||||
],
|
||||
'Valid route with deep namespace' => [
|
||||
'some.deep.namespace.some_operation',
|
||||
['namespace' => 'some.deep.namespace', 'operation' => 'some_operation'],
|
||||
],
|
||||
];
|
||||
}
|
||||
|
||||
/**
|
||||
* @dataProvider GetRouteNamespaceProvider
|
||||
* @covers \Combodo\iTop\Service\Router\Router::GetRouteNamespace
|
||||
@@ -178,7 +240,7 @@ class RouterTest extends ItopTestCase
|
||||
public function testGetRouteNamespace(string $sRoute, ?string $sExpectedNamespace): void
|
||||
{
|
||||
$oRouter = Router::GetInstance();
|
||||
$sTestedNamespace = $oRouter->GetRouteNamespace($sRoute);
|
||||
$sTestedNamespace = $this->InvokeNonPublicMethod(Router::class, 'GetRouteNamespace', $oRouter, [$sRoute]);
|
||||
|
||||
$this->assertEquals($sExpectedNamespace, $sTestedNamespace, "Namespace found for '$sRoute' was not the expected one. Got '$sTestedNamespace', expected '$sExpectedNamespace'.");
|
||||
}
|
||||
@@ -213,7 +275,7 @@ class RouterTest extends ItopTestCase
|
||||
public function testGetRouteOperation(string $sRoute, ?string $sExpectedOperation): void
|
||||
{
|
||||
$oRouter = Router::GetInstance();
|
||||
$sTestedOperation = $oRouter->GetRouteOperation($sRoute);
|
||||
$sTestedOperation = $this->InvokeNonPublicMethod(Router::class, 'GetRouteOperation', $oRouter, [$sRoute]);
|
||||
|
||||
$this->assertEquals($sExpectedOperation, $sTestedOperation, "Operation found for '$sRoute' was not the expected one. Got '$sTestedOperation', expected '$sExpectedOperation'.");
|
||||
}
|
||||
@@ -237,30 +299,28 @@ class RouterTest extends ItopTestCase
|
||||
}
|
||||
|
||||
/**
|
||||
* @dataProvider FindControllerFromRouteNamespaceProvider
|
||||
* @covers \Combodo\iTop\Service\Router\Router::FindControllerFromRouteNamespace
|
||||
* @dataProvider FindHandlerFromRouteProvider
|
||||
* @covers \Combodo\iTop\Service\Router\Router::FindHandlerFromRoute
|
||||
*
|
||||
* @param string $sRouteNamespace
|
||||
* @param string $sExpectedControllerFQCN
|
||||
* @param string $sExpectedHandlerFQCN
|
||||
*
|
||||
* @return void
|
||||
*/
|
||||
public function testFindControllerFromRouteNamespace(string $sRoute, ?string $sExpectedControllerFQCN): void
|
||||
public function testFindHandlerFromRoute(string $sRoute, ?string $sExpectedHandlerFQCN): void
|
||||
{
|
||||
$oRouter = Router::GetInstance();
|
||||
$sRouteNamespace = $oRouter->GetRouteNamespace($sRoute);
|
||||
$sTestedHandlerFQCN = $this->InvokeNonPublicMethod(Router::class, 'FindHandlerFromRoute', $oRouter, [$sRoute]);
|
||||
|
||||
$sTestedControllerFQCN = $this->InvokeNonPublicMethod(get_class($oRouter), 'FindControllerFromRouteNamespace', $oRouter, [$sRouteNamespace]);
|
||||
|
||||
$this->assertEquals($sExpectedControllerFQCN, $sTestedControllerFQCN, "Controller found for '$sRouteNamespace' was not the expected one. Got '$sTestedControllerFQCN', expected '$sExpectedControllerFQCN'.");
|
||||
$this->assertEquals($sExpectedHandlerFQCN, $sTestedHandlerFQCN, "Handler found for '$sRoute' was not the expected one. Got '$sTestedHandlerFQCN', expected '$sExpectedHandlerFQCN'.");
|
||||
}
|
||||
|
||||
public function FindControllerFromRouteNamespaceProvider(): array
|
||||
public function FindHandlerFromRouteProvider(): array
|
||||
{
|
||||
return [
|
||||
'Object controller' => [
|
||||
'object.modify',
|
||||
'Combodo\iTop\Controller\Base\Layout\ObjectController',
|
||||
'Combodo\iTop\Controller\Base\Layout\ObjectController::OperationModify',
|
||||
],
|
||||
'Unknown controller' => [
|
||||
'something_that_should_not_exist_in_the_default_package.foo',
|
||||
@@ -268,36 +328,4 @@ class RouterTest extends ItopTestCase
|
||||
],
|
||||
];
|
||||
}
|
||||
|
||||
/**
|
||||
* @dataProvider GetOperationMethodNameFromRouteOperationProvider
|
||||
* @covers \Combodo\iTop\Service\Router\Router::MakeOperationMethodNameFromOperation
|
||||
*
|
||||
* @param string $sRoute
|
||||
* @param string $sExpectedMethodName
|
||||
*
|
||||
* @return void
|
||||
*/
|
||||
public function testGetOperationMethodNameFromRouteOperation(string $sRoute, string $sExpectedMethodName): void
|
||||
{
|
||||
$oRouter = Router::GetInstance();
|
||||
$aRouteParts = $oRouter->GetRouteParts($sRoute);
|
||||
|
||||
$sTestedMethodName = $this->InvokeNonPublicMethod(get_class($oRouter), 'MakeOperationMethodNameFromOperation', $oRouter, [$aRouteParts['operation']]);
|
||||
$this->assertEquals($sExpectedMethodName, $sTestedMethodName, "Operation method name '".$aRouteParts['operation']."' was not matching the expected one. Got '$sTestedMethodName', expected '$sExpectedMethodName'.");
|
||||
}
|
||||
|
||||
public function GetOperationMethodNameFromRouteOperationProvider(): array
|
||||
{
|
||||
return [
|
||||
'Simple operation' => [
|
||||
'object.modify',
|
||||
'OperationModify',
|
||||
],
|
||||
'Operation with an underscore' => [
|
||||
'object.apply_modify',
|
||||
'OperationApplyModify',
|
||||
],
|
||||
];
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user