From d7476c1de8ff1cf66bbce09dc894d4e5693bf6d1 Mon Sep 17 00:00:00 2001 From: Yuri Kuznetsov Date: Sat, 19 Sep 2020 16:43:07 +0300 Subject: [PATCH] fix class parser --- application/Espo/Core/Utils/ClassFinder.php | 6 ++ .../Espo/Core/Utils/File/ClassParser.php | 43 ++++---- application/Espo/Core/Utils/Util.php | 2 +- tests/unit/Espo/Core/HookManagerTest.php | 100 +++++++++--------- .../Espo/Core/Utils/File/ClassParserTest.php | 30 +++--- tests/unit/Espo/Core/Utils/UtilTest.php | 4 +- 6 files changed, 94 insertions(+), 91 deletions(-) diff --git a/application/Espo/Core/Utils/ClassFinder.php b/application/Espo/Core/Utils/ClassFinder.php index b2686f049b..2124dba8df 100644 --- a/application/Espo/Core/Utils/ClassFinder.php +++ b/application/Espo/Core/Utils/ClassFinder.php @@ -58,6 +58,7 @@ class ClassFinder { $map = $this->getMap($category, $subDirs); $className = $map[$name] ?? null; + return $className; } @@ -69,6 +70,7 @@ class ClassFinder if (!array_key_exists($category, $this->dataHash)) { $this->load($category, $subDirs); } + return $this->dataHash[$category] ?? []; } @@ -76,21 +78,25 @@ class ClassFinder { $path = $this->buildPaths($category); $cacheFile = $this->buildCacheFilePath($category); + $this->dataHash[$category] = $this->classParser->getData($path, $cacheFile, null, $subDirs); } protected function buildPaths(string $category) : array { $paths = []; + foreach ($this->pathsTemplate as $key => $value) { $path[$key] = str_replace('{category}', $category, $value); } + return $path; } protected function buildCacheFilePath(string $category) : string { $path = 'data/cache/application/classmap_' . str_replace('/', '_', strtolower($category)) . '.php'; + return $path; } } diff --git a/application/Espo/Core/Utils/File/ClassParser.php b/application/Espo/Core/Utils/File/ClassParser.php index 6b8bac1969..a0efb950b7 100644 --- a/application/Espo/Core/Utils/File/ClassParser.php +++ b/application/Espo/Core/Utils/File/ClassParser.php @@ -36,6 +36,8 @@ use Espo\Core\Utils\Metadata; use Espo\Core\Exceptions\Error; +use ReflectionClass; + class ClassParser { private $fileManager; @@ -51,21 +53,6 @@ class ClassParser $this->metadata = $metadata; } - protected function getFileManager() - { - return $this->fileManager; - } - - protected function getConfig() - { - return $this->config; - } - - protected function getMetadata() - { - return $this->metadata; - } - /** * Return paths to class files. * @@ -87,8 +74,8 @@ class ClassParser ]; } - if ($cacheFile && file_exists($cacheFile) && $this->getConfig()->get('useCache')) { - $data = $this->getFileManager()->getPhpContents($cacheFile); + if ($cacheFile && file_exists($cacheFile) && $this->config->get('useCache')) { + $data = $this->fileManager->getPhpContents($cacheFile); if (!is_array($data)) { $GLOBALS['log']->error("ClassParser: Non-array value stored in {$cacheFile}."); @@ -99,7 +86,7 @@ class ClassParser $data = $this->getClassNameHash($paths['corePath'], $allowedMethods, $subDirs); if (isset($paths['modulePath'])) { - foreach ($this->getMetadata()->getModuleList() as $moduleName) { + foreach ($this->metadata->getModuleList() as $moduleName) { $path = str_replace('{*}', $moduleName, $paths['modulePath']); $data = array_merge($data, $this->getClassNameHash($path, $allowedMethods, $subDirs)); @@ -110,8 +97,9 @@ class ClassParser $data = array_merge($data, $this->getClassNameHash($paths['customPath'], $allowedMethods, $subDirs)); } - if ($cacheFile && $this->getConfig()->get('useCache')) { - $result = $this->getFileManager()->putPhpContents($cacheFile, $data); + if ($cacheFile && $this->config->get('useCache')) { + $result = $this->fileManager->putPhpContents($cacheFile, $data); + if ($result == false) { throw new Error("ClassParser: Could not save file {$cacheFile}."); } @@ -128,9 +116,10 @@ class ClassParser } $data = []; + foreach ($dirs as $dir) { if (file_exists($dir)) { - $fileList = $this->getFileManager()->getFileList($dir, $subDirs, '\.php$', true); + $fileList = $this->fileManager->getFileList($dir, $subDirs, '\.php$', true); $this->fillHashFromFileList($fileList, $dir, $allowedMethods, $data); } @@ -147,15 +136,20 @@ class ClassParser if (is_array($file)) { $this->fillHashFromFileList($file, $dir . '/'. $key, $allowedMethods, $data, $category . $key . '\\'); } + continue; } $filePath = Util::concatPath($dir, $file); $className = Util::getClassName($filePath); - $fileName = $this->getFileManager()->getFileName($filePath); - $class = new \ReflectionClass($className); - if (!$class->isInstantiable()) continue; + $fileName = $this->fileManager->getFileName($filePath); + + $class = new ReflectionClass($className); + + if (!$class->isInstantiable()) { + continue; + } $name = Util::normilizeScopeName(ucfirst($fileName)); @@ -163,6 +157,7 @@ class ClassParser if (empty($allowedMethods)) { $data[$name] = $className; + continue; } diff --git a/application/Espo/Core/Utils/Util.php b/application/Espo/Core/Utils/Util.php index ca979ff94d..cb417d41b7 100644 --- a/application/Espo/Core/Utils/Util.php +++ b/application/Espo/Core/Utils/Util.php @@ -459,7 +459,7 @@ class Util { $className = preg_replace('/\.php$/i', '', $filePath); $className = preg_replace('/^(application|custom)(\/|\\\)/i', '', $className); - $className = '\\'.static::toFormat($className, '\\'); + $className = static::toFormat($className, '\\'); return $className; } diff --git a/tests/unit/Espo/Core/HookManagerTest.php b/tests/unit/Espo/Core/HookManagerTest.php index 5e5398c9a9..6eec614a62 100644 --- a/tests/unit/Espo/Core/HookManagerTest.php +++ b/tests/unit/Espo/Core/HookManagerTest.php @@ -43,13 +43,13 @@ class HookManagerTest extends \PHPUnit\Framework\TestCase { $this->objects['metadata'] = - $this->getMockBuilder('\\Espo\\Core\\Utils\\Metadata')->disableOriginalConstructor()->getMock(); + $this->getMockBuilder('Espo\\Core\\Utils\\Metadata')->disableOriginalConstructor()->getMock(); $this->objects['config'] = - $this->getMockBuilder('\\Espo\\Core\\Utils\\Config')->disableOriginalConstructor()->getMock(); + $this->getMockBuilder('Espo\\Core\\Utils\\Config')->disableOriginalConstructor()->getMock(); $this->objects['injectableFactory'] = - $this->getMockBuilder('\\Espo\\Core\\InjectableFactory')->disableOriginalConstructor()->getMock(); + $this->getMockBuilder('Espo\\Core\\InjectableFactory')->disableOriginalConstructor()->getMock(); $this->objects['fileManager'] = new \Espo\Core\Utils\File\Manager(); @@ -80,24 +80,24 @@ class HookManagerTest extends \PHPUnit\Framework\TestCase $data = array ( array ( - 'className' => '\\Espo\\Hooks\\Note\\Stream', + 'className' => 'Espo\\Hooks\\Note\\Stream', 'order' => 8, ), array ( - 'className' => '\\Espo\\Hooks\\Note\\Mentions', + 'className' => 'Espo\\Hooks\\Note\\Mentions', 'order' => 9, ), array ( - 'className' => '\\Espo\\Hooks\\Note\\Notifications', + 'className' => 'Espo\\Hooks\\Note\\Notifications', 'order' => 14, ), ); - $this->assertTrue( $this->reflection->invokeMethod('hookExists', array('\\Espo\\Hooks\\Note\\Mentions', $data)) ); - $this->assertTrue( $this->reflection->invokeMethod('hookExists', array('\\Espo\\Modules\\Crm\\Hooks\\Note\\Mentions', $data)) ); - $this->assertTrue( $this->reflection->invokeMethod('hookExists', array('\\Espo\\Modules\\Test\\Hooks\\Note\\Mentions', $data)) ); - $this->assertTrue( $this->reflection->invokeMethod('hookExists', array('\\Espo\\Modules\\Test\\Hooks\\Common\\Stream', $data)) ); - $this->assertFalse( $this->reflection->invokeMethod('hookExists', array('\\Espo\\Hooks\\Note\\TestHook', $data)) ); + $this->assertTrue( $this->reflection->invokeMethod('hookExists', array('Espo\\Hooks\\Note\\Mentions', $data)) ); + $this->assertTrue( $this->reflection->invokeMethod('hookExists', array('Espo\\Modules\\Crm\\Hooks\\Note\\Mentions', $data)) ); + $this->assertTrue( $this->reflection->invokeMethod('hookExists', array('Espo\\Modules\\Test\\Hooks\\Note\\Mentions', $data)) ); + $this->assertTrue( $this->reflection->invokeMethod('hookExists', array('Espo\\Modules\\Test\\Hooks\\Common\\Stream', $data)) ); + $this->assertFalse( $this->reflection->invokeMethod('hookExists', array('Espo\\Hooks\\Note\\TestHook', $data)) ); } public function testSortHooks() @@ -108,30 +108,30 @@ class HookManagerTest extends \PHPUnit\Framework\TestCase 'afterSave' => array ( array ( - 'className' => '\\Espo\\Hooks\\Common\\AssignmentEmailNotification', + 'className' => 'Espo\\Hooks\\Common\\AssignmentEmailNotification', 'order' => 9, ), array ( - 'className' => '\\Espo\\Hooks\\Common\\Notifications', + 'className' => 'Espo\\Hooks\\Common\\Notifications', 'order' => 10, ), array ( - 'className' => '\\Espo\\Hooks\\Common\\Stream', + 'className' => 'Espo\\Hooks\\Common\\Stream', 'order' => 9, ), ), 'beforeSave' => array ( array ( - 'className' => '\\Espo\\Hooks\\Common\\Formula', + 'className' => 'Espo\\Hooks\\Common\\Formula', 'order' => 5, ), array ( - 'className' => '\\Espo\\Hooks\\Common\\NextNumber', + 'className' => 'Espo\\Hooks\\Common\\NextNumber', 'order' => 10, ), array ( - 'className' => '\\Espo\\Hooks\\Common\\CurrencyConverted', + 'className' => 'Espo\\Hooks\\Common\\CurrencyConverted', 'order' => 1, ), ), @@ -141,14 +141,14 @@ class HookManagerTest extends \PHPUnit\Framework\TestCase 'beforeSave' => array ( array ( - 'className' => '\\Espo\\Hooks\\Note\\Mentions', + 'className' => 'Espo\\Hooks\\Note\\Mentions', 'order' => 9, ), ), 'afterSave' => array ( array ( - 'className' => '\\Espo\\Hooks\\Note\\Notifications', + 'className' => 'Espo\\Hooks\\Note\\Notifications', 'order' => 14, ), ), @@ -161,30 +161,30 @@ class HookManagerTest extends \PHPUnit\Framework\TestCase 'afterSave' => array ( array ( - 'className' => '\\Espo\\Hooks\\Common\\AssignmentEmailNotification', + 'className' => 'Espo\\Hooks\\Common\\AssignmentEmailNotification', 'order' => 9, ), array ( - 'className' => '\\Espo\\Hooks\\Common\\Stream', + 'className' => 'Espo\\Hooks\\Common\\Stream', 'order' => 9, ), array ( - 'className' => '\\Espo\\Hooks\\Common\\Notifications', + 'className' => 'Espo\\Hooks\\Common\\Notifications', 'order' => 10, ), ), 'beforeSave' => array ( array ( - 'className' => '\\Espo\\Hooks\\Common\\CurrencyConverted', + 'className' => 'Espo\\Hooks\\Common\\CurrencyConverted', 'order' => 1, ), array ( - 'className' => '\\Espo\\Hooks\\Common\\Formula', + 'className' => 'Espo\\Hooks\\Common\\Formula', 'order' => 5, ), array ( - 'className' => '\\Espo\\Hooks\\Common\\NextNumber', + 'className' => 'Espo\\Hooks\\Common\\NextNumber', 'order' => 10, ), ), @@ -194,14 +194,14 @@ class HookManagerTest extends \PHPUnit\Framework\TestCase 'beforeSave' => array ( array ( - 'className' => '\\Espo\\Hooks\\Note\\Mentions', + 'className' => 'Espo\\Hooks\\Note\\Mentions', 'order' => 9, ), ), 'afterSave' => array ( array ( - 'className' => '\\Espo\\Hooks\\Note\\Notifications', + 'className' => 'Espo\\Hooks\\Note\\Notifications', 'order' => 14, ), ), @@ -240,7 +240,7 @@ class HookManagerTest extends \PHPUnit\Framework\TestCase 'beforeSave' => array ( array ( - 'className' => '\\tests\\unit\\testData\\Hooks\\testCase1\\custom\\Espo\\Custom\\Hooks\\Note\\Mentions', + 'className' => 'tests\\unit\\testData\\Hooks\\testCase1\\custom\\Espo\\Custom\\Hooks\\Note\\Mentions', 'order' => 7, ), ), @@ -279,7 +279,8 @@ class HookManagerTest extends \PHPUnit\Framework\TestCase 'beforeSave' => array ( array ( - 'className' => '\\tests\\unit\\testData\\Hooks\\testCase2\\application\\Espo\\Modules\\Crm\\Hooks\\Note\\Mentions', + 'className' => + 'tests\\unit\\testData\\Hooks\\testCase2\\application\\Espo\\Modules\\Crm\\Hooks\\Note\\Mentions', 'order' => 9, ), ), @@ -318,7 +319,8 @@ class HookManagerTest extends \PHPUnit\Framework\TestCase 'beforeSave' => array ( array ( - 'className' => '\\tests\\unit\\testData\\Hooks\\testCase2\\application\\Espo\\Modules\\Test\\Hooks\\Note\\Mentions', + 'className' => + 'tests\\unit\\testData\\Hooks\\testCase2\\application\\Espo\\Modules\\Test\\Hooks\\Note\\Mentions', 'order' => 9, ), ), @@ -355,7 +357,7 @@ class HookManagerTest extends \PHPUnit\Framework\TestCase 'beforeSave' => array ( array ( - 'className' => '\\tests\\unit\\testData\\Hooks\\testCase3\\application\\Espo\\Hooks\\Note\\Mentions', + 'className' => 'tests\\unit\\testData\\Hooks\\testCase3\\application\\Espo\\Hooks\\Note\\Mentions', 'order' => 9, ), ), @@ -373,30 +375,30 @@ class HookManagerTest extends \PHPUnit\Framework\TestCase 'afterSave' => array ( array ( - 'className' => '\\Espo\\Hooks\\Common\\AssignmentEmailNotification', + 'className' => 'Espo\\Hooks\\Common\\AssignmentEmailNotification', 'order' => 9, ), array ( - 'className' => '\\Espo\\Hooks\\Common\\Stream', + 'className' => 'Espo\\Hooks\\Common\\Stream', 'order' => 9, ), array ( - 'className' => '\\Espo\\Hooks\\Common\\Notifications', + 'className' => 'Espo\\Hooks\\Common\\Notifications', 'order' => 10, ), ), 'beforeSave' => array ( array ( - 'className' => '\\Espo\\Hooks\\Common\\CurrencyConverted', + 'className' => 'Espo\\Hooks\\Common\\CurrencyConverted', 'order' => 1, ), array ( - 'className' => '\\Espo\\Hooks\\Common\\Formula', + 'className' => 'Espo\\Hooks\\Common\\Formula', 'order' => 5, ), array ( - 'className' => '\\Espo\\Hooks\\Common\\NextNumber', + 'className' => 'Espo\\Hooks\\Common\\NextNumber', 'order' => 10, ), ), @@ -406,18 +408,18 @@ class HookManagerTest extends \PHPUnit\Framework\TestCase 'beforeSave' => array ( array ( - 'className' => '\\Espo\\Hooks\\Note\\Mentions', + 'className' => 'Espo\\Hooks\\Note\\Mentions', 'order' => 9, ), ), 'afterSave' => array ( array ( - 'className' => '\\Espo\\Hooks\\Note\\Btest', + 'className' => 'Espo\\Hooks\\Note\\Btest', 'order' => 9, ), array ( - 'className' => '\\Espo\\Hooks\\Note\\Notifications', + 'className' => 'Espo\\Hooks\\Note\\Notifications', 'order' => 14, ), ), @@ -425,18 +427,18 @@ class HookManagerTest extends \PHPUnit\Framework\TestCase )); $resultBeforeSave = array( - '\\Espo\\Hooks\\Common\\CurrencyConverted', - '\\Espo\\Hooks\\Common\\Formula', - '\\Espo\\Hooks\\Note\\Mentions', - '\\Espo\\Hooks\\Common\\NextNumber', + 'Espo\\Hooks\\Common\\CurrencyConverted', + 'Espo\\Hooks\\Common\\Formula', + 'Espo\\Hooks\\Note\\Mentions', + 'Espo\\Hooks\\Common\\NextNumber', ); $resultAfterSave = array( - '\\Espo\\Hooks\\Common\\AssignmentEmailNotification', - '\\Espo\\Hooks\\Note\\Btest', - '\\Espo\\Hooks\\Common\\Stream', - '\\Espo\\Hooks\\Common\\Notifications', - '\\Espo\\Hooks\\Note\\Notifications', + 'Espo\\Hooks\\Common\\AssignmentEmailNotification', + 'Espo\\Hooks\\Note\\Btest', + 'Espo\\Hooks\\Common\\Stream', + 'Espo\\Hooks\\Common\\Notifications', + 'Espo\\Hooks\\Note\\Notifications', ); } } diff --git a/tests/unit/Espo/Core/Utils/File/ClassParserTest.php b/tests/unit/Espo/Core/Utils/File/ClassParserTest.php index a9913cf6c6..0c529485be 100644 --- a/tests/unit/Espo/Core/Utils/File/ClassParserTest.php +++ b/tests/unit/Espo/Core/Utils/File/ClassParserTest.php @@ -42,8 +42,8 @@ class ClassParserTest extends \PHPUnit\Framework\TestCase protected function setUp() : void { $this->objects['fileManager'] = new \Espo\Core\Utils\File\Manager(); - $this->objects['config'] = $this->getMockBuilder('\Espo\Core\Utils\Config')->disableOriginalConstructor()->getMock(); - $this->objects['metadata'] = $this->getMockBuilder('\Espo\Core\Utils\Metadata')->disableOriginalConstructor()->getMock(); + $this->objects['config'] = $this->getMockBuilder('Espo\Core\Utils\Config')->disableOriginalConstructor()->getMock(); + $this->objects['metadata'] = $this->getMockBuilder('Espo\Core\Utils\Metadata')->disableOriginalConstructor()->getMock(); $this->object = new \Espo\Core\Utils\File\ClassParser( $this->objects['fileManager'], $this->objects['config'], $this->objects['metadata']); @@ -64,9 +64,9 @@ class ClassParserTest extends \PHPUnit\Framework\TestCase ]; $result = [ - 'Download' => '\tests\unit\testData\EntryPoints\Espo\EntryPoints\Download', - 'Test' => '\tests\unit\testData\EntryPoints\Espo\EntryPoints\Test', - 'InModule' => '\tests\unit\testData\EntryPoints\Espo\Modules\Crm\EntryPoints\InModule' + 'Download' => 'tests\unit\testData\EntryPoints\Espo\EntryPoints\Download', + 'Test' => 'tests\unit\testData\EntryPoints\Espo\EntryPoints\Test', + 'InModule' => 'tests\unit\testData\EntryPoints\Espo\Modules\Crm\EntryPoints\InModule' ]; $this->assertEquals($result, $this->reflection->invokeMethod('getClassNameHash', [$paths, ['run']])); } @@ -80,9 +80,9 @@ class ClassParserTest extends \PHPUnit\Framework\TestCase $cacheFile = 'tests/unit/testData/EntryPoints/cache/entryPoints.php'; $paths = [ - 'corePath' => 'tests/unit/testData/EntryPoints/Espo/EntryPoints', - 'modulePath' => 'tests/unit/testData/EntryPoints/Espo/Modules/{*}/EntryPoints', - 'customPath' => 'tests/unit/testData/EntryPoints/Espo/Custom/EntryPoints', + 'corePath' => '/tests/unit/testData/EntryPoints/Espo/EntryPoints', + 'modulePath' => '/tests/unit/testData/EntryPoints/Espo/Modules/{*}/EntryPoints', + 'customPath' => '/tests/unit/testData/EntryPoints/Espo/Custom/EntryPoints', ]; $result = [ @@ -116,9 +116,9 @@ class ClassParserTest extends \PHPUnit\Framework\TestCase ]; $result = [ - 'Download' => '\tests\unit\testData\EntryPoints\Espo\EntryPoints\Download', - 'Test' => '\tests\unit\testData\EntryPoints\Espo\EntryPoints\Test', - 'InModule' => '\tests\unit\testData\EntryPoints\Espo\Modules\Crm\EntryPoints\InModule' + 'Download' => 'tests\unit\testData\EntryPoints\Espo\EntryPoints\Download', + 'Test' => 'tests\unit\testData\EntryPoints\Espo\EntryPoints\Test', + 'InModule' => 'tests\unit\testData\EntryPoints\Espo\Modules\Crm\EntryPoints\InModule' ]; $this->assertEquals($result, $this->reflection->invokeMethod('getData', [$paths, $cacheFile, ['run']])); @@ -144,8 +144,8 @@ class ClassParserTest extends \PHPUnit\Framework\TestCase $path = 'tests/unit/testData/EntryPoints/Espo/EntryPoints'; $result = [ - 'Download' => '\tests\unit\testData\EntryPoints\Espo\EntryPoints\Download', - 'Test' => '\tests\unit\testData\EntryPoints\Espo\EntryPoints\Test', + 'Download' => 'tests\unit\testData\EntryPoints\Espo\EntryPoints\Download', + 'Test' => 'tests\unit\testData\EntryPoints\Espo\EntryPoints\Test', ]; $this->assertEquals($result, $this->reflection->invokeMethod('getData', [$path, $cacheFile, ['run']])); @@ -173,8 +173,8 @@ class ClassParserTest extends \PHPUnit\Framework\TestCase ]; $result = [ - 'Download' => '\tests\unit\testData\EntryPoints\Espo\EntryPoints\Download', - 'Test' => '\tests\unit\testData\EntryPoints\Espo\EntryPoints\Test', + 'Download' => 'tests\unit\testData\EntryPoints\Espo\EntryPoints\Download', + 'Test' => 'tests\unit\testData\EntryPoints\Espo\EntryPoints\Test', ]; $this->assertEquals($result, $this->reflection->invokeMethod('getData', [$paths, null, ['run']])); diff --git a/tests/unit/Espo/Core/Utils/UtilTest.php b/tests/unit/Espo/Core/Utils/UtilTest.php index dd24d92c4b..619c85bf48 100644 --- a/tests/unit/Espo/Core/Utils/UtilTest.php +++ b/tests/unit/Espo/Core/Utils/UtilTest.php @@ -1047,7 +1047,7 @@ class UtilTest extends \PHPUnit\Framework\TestCase $this->assertEquals('\Espo\Core\Utils', Util::toFormat('/Espo/Core/Utils', '\\')); $this->assertEquals('/Espo/Core/Utils', Util::toFormat('\Espo\Core\Utils', '/')); - $this->assertEquals('\Espo\Core\Utils', Util::toFormat('\Espo\Core\Utils', '\\')); + $this->assertEquals('Espo\Core\Utils', Util::toFormat('Espo\Core\Utils', '\\')); } public function testConcatPath() @@ -1157,7 +1157,7 @@ class UtilTest extends \PHPUnit\Framework\TestCase /** * @dataProvider dp_classNames */ - public function testGetClassName($path, $expectedClassName = '\Espo\EntryPoints\Donwload') + public function testGetClassName($path, $expectedClassName = 'Espo\EntryPoints\Donwload') { $this->assertEquals($expectedClassName, Util::getClassName($path)); }