diff --git a/application/Espo/Core/HookManager.php b/application/Espo/Core/HookManager.php index 2600548afb..9ed6b946c3 100644 --- a/application/Espo/Core/HookManager.php +++ b/application/Espo/Core/HookManager.php @@ -37,38 +37,34 @@ use Espo\Core\{ Utils\Util, Utils\DataCache, Utils\Log, + Utils\Module\PathProvider, }; /** - * Runs hooks. E.g. beforeSave, afterSave. Hooks can be located in a folder that matches a certain *entityType* or - * in Common folder. Common hooks will be applied to any *entityType*. + * Runs hooks. E.g. beforeSave, afterSave. Hooks can be located in a folder + * that matches a certain *entityType* or in the `Common` folder. + * Common hooks are applied to all entity types. */ class HookManager { - const DEFAULT_ORDER = 9; + private const DEFAULT_ORDER = 9; private $data; - protected $isDisabled; + private $isDisabled; private $hookListHash = []; private $hooks; - protected $cacheKey = 'hooks'; + private $cacheKey = 'hooks'; - protected $ignoredMethodList = [ + private $ignoredMethodList = [ '__construct', 'getDependencyList', 'inject', ]; - protected $paths = [ - 'corePath' => 'application/Espo/Hooks', - 'modulePath' => 'application/Espo/Modules/{*}/Hooks', - 'customPath' => 'custom/Espo/Custom/Hooks', - ]; - private $injectableFactory; private $fileManager; @@ -81,13 +77,16 @@ class HookManager private $log; + private $pathProvider; + public function __construct( InjectableFactory $injectableFactory, FileManager $fileManager, Metadata $metadata, Config $config, DataCache $dataCache, - Log $log + Log $log, + PathProvider $pathProvider ) { $this->injectableFactory = $injectableFactory; $this->fileManager = $fileManager; @@ -95,6 +94,7 @@ class HookManager $this->config = $config; $this->dataCache = $dataCache; $this->log = $log; + $this->pathProvider = $pathProvider; } public function process( @@ -146,7 +146,7 @@ class HookManager $this->isDisabled = false; } - protected function loadHooks(): void + private function loadHooks(): void { if ($this->config->get('useCache') && $this->dataCache->has($this->cacheKey)) { $this->data = $this->dataCache->get($this->cacheKey); @@ -156,15 +156,15 @@ class HookManager $metadata = $this->metadata; - $data = $this->getHookData($this->paths['customPath']); + $data = $this->readHookData($this->pathProvider->getCustom() . 'Hooks'); foreach ($metadata->getModuleList() as $moduleName) { - $modulePath = str_replace('{*}', $moduleName, $this->paths['modulePath']); + $modulePath = $this->pathProvider->getModule($moduleName) . 'Hooks'; - $data = $this->getHookData($modulePath, $data); + $data = $this->readHookData($modulePath, $data); } - $data = $this->getHookData($this->paths['corePath'], $data); + $data = $this->readHookData($this->pathProvider->getCore() . 'Hooks', $data); $this->data = $this->sortHooks($data); @@ -173,7 +173,7 @@ class HookManager } } - protected function createHookByClassName(string $className): object + private function createHookByClassName(string $className): object { if (!class_exists($className)) { $this->log->error("Hook class '{$className}' does not exist."); @@ -184,55 +184,45 @@ class HookManager return $obj; } - /** - * Get and merge hook data by checking the files exist in $hookDirs. - * - * @param $hookDirs - can be ['Espo/Hooks', 'Espo/Custom/Hooks', 'Espo/Modules/Crm/Hooks'] - */ - protected function getHookData($hookDirs, array $hookData = []): array + private function readHookData(string $hookDir, array $hookData = []): array { - if (is_string($hookDirs)) { - $hookDirs = (array) $hookDirs; + if (!$this->fileManager->exists($hookDir)) { + return $hookData; } - foreach ($hookDirs as $hookDir) { - if (!file_exists($hookDir)) { - continue; - } + $fileList = $this->fileManager->getFileList($hookDir, 1, '\.php$', true); - $fileList = $this->fileManager->getFileList($hookDir, 1, '\.php$', true); + foreach ($fileList as $scopeName => $hookFiles) { + $hookScopeDirPath = Util::concatPath($hookDir, $scopeName); + $normalizedScopeName = Util::normilizeScopeName($scopeName); - foreach ($fileList as $scopeName => $hookFiles) { - $hookScopeDirPath = Util::concatPath($hookDir, $scopeName); - $normalizedScopeName = Util::normilizeScopeName($scopeName); + foreach ($hookFiles as $hookFile) { + $hookFilePath = Util::concatPath($hookScopeDirPath, $hookFile); + $className = Util::getClassName($hookFilePath); - $scopeHooks = []; + $classMethods = get_class_methods($className); - foreach ($hookFiles as $hookFile) { - $hookFilePath = Util::concatPath($hookScopeDirPath, $hookFile); - $className = Util::getClassName($hookFilePath); + $hookMethods = array_diff($classMethods, $this->ignoredMethodList); - $classMethods = get_class_methods($className); - $hookMethods = array_diff($classMethods, $this->ignoredMethodList); - - $hookMethods = array_filter($hookMethods, function ($item) { - if (strpos($item, 'set') === 0) { - return false; - } - - return true; - }); - - foreach ($hookMethods as $hookType) { - $entityHookData = $hookData[$normalizedScopeName][$hookType] ?? []; - - if (!$this->hookExists($className, $entityHookData)) { - $hookData[$normalizedScopeName][$hookType][] = [ - 'className' => $className, - 'order' => $className::$order ?? self::DEFAULT_ORDER, - ]; - } + $hookMethods = array_filter($hookMethods, function ($item) { + if (strpos($item, 'set') === 0) { + return false; } + + return true; + }); + + foreach ($hookMethods as $hookType) { + $entityHookData = $hookData[$normalizedScopeName][$hookType] ?? []; + + if ($this->hookExists($className, $entityHookData)) { + continue; + } + + $hookData[$normalizedScopeName][$hookType][] = [ + 'className' => $className, + 'order' => $className::$order ?? self::DEFAULT_ORDER, + ]; } } } @@ -241,12 +231,12 @@ class HookManager } /** - * Sort hooks by the order param. + * Sort hooks by the order parameter. */ - protected function sortHooks(array $hooks): array + private function sortHooks(array $hooks): array { - foreach ($hooks as $scopeName => &$scopeHooks) { - foreach ($scopeHooks as $hookName => &$hookList) { + foreach ($hooks as &$scopeHooks) { + foreach ($scopeHooks as &$hookList) { usort($hookList, [$this, 'cmpHooks']); } } @@ -257,12 +247,12 @@ class HookManager /** * Get sorted hook list. */ - protected function getHookList(string $scope, string $hookName): array + private function getHookList(string $scope, string $hookName): array { $key = $scope . '_' . $hookName; if (!isset($this->hookListHash[$key])) { - $hookList = array(); + $hookList = []; if (isset($this->data['Common'][$hookName])) { $hookList = $this->data['Common'][$hookName]; @@ -270,6 +260,7 @@ class HookManager if (isset($this->data[$scope][$hookName])) { $hookList = array_merge($hookList, $this->data[$scope][$hookName]); + usort($hookList, array($this, 'cmpHooks')); } @@ -288,7 +279,7 @@ class HookManager /** * Check if hook exists in the list. */ - protected function hookExists(string $className, array $hookData): bool + private function hookExists(string $className, array $hookData): bool { $class = preg_replace('/^.*\\\(.*)$/', '$1', $className); @@ -301,7 +292,7 @@ class HookManager return false; } - protected function cmpHooks($a, $b): int + private function cmpHooks($a, $b): int { if ($a['order'] == $b['order']) { return 0; diff --git a/tests/unit/Espo/Core/HookManagerTest.php b/tests/unit/Espo/Core/HookManagerTest.php index f7fe2d239c..d4fce282a4 100644 --- a/tests/unit/Espo/Core/HookManagerTest.php +++ b/tests/unit/Espo/Core/HookManagerTest.php @@ -39,52 +39,69 @@ use Espo\Core\{ Utils\File\Manager as FileManager, Utils\DataCache, Utils\Log, + Utils\Module\PathProvider, }; class HookManagerTest extends \PHPUnit\Framework\TestCase { - protected $object; + private $hookManager; - protected $objects; + private $filesPath = 'tests/unit/testData/Hooks'; - protected $filesPath = 'tests/unit/testData/Hooks'; - - protected function setUp() : void + protected function setUp(): void { - - $this->metadata = + $this->metadata = $this->createMock(Metadata::class); $this->getMockBuilder(Metadata::class)->disableOriginalConstructor()->getMock(); - $this->config = - $this->getMockBuilder(Config::class)->disableOriginalConstructor()->getMock(); + $this->config = $this->createMock(Config::class); - $this->injectableFactory = - $this->getMockBuilder(InjectableFactory::class)->disableOriginalConstructor()->getMock(); + $this->injectableFactory = $this->createMock(InjectableFactory::class); - $this->dataCache = - $this->getMockBuilder(DataCache::class)->disableOriginalConstructor()->getMock(); + $this->dataCache = $this->createMock(DataCache::class); $this->fileManager = new FileManager(); - $this->object = new HookManager( + $this->pathProvider = $this->createMock(PathProvider::class); + + $this->hookManager = new HookManager( $this->injectableFactory, $this->fileManager, $this->metadata, $this->config, $this->dataCache, - $this->createMock(Log::class) + $this->createMock(Log::class), + $this->pathProvider ); - $this->reflection = new ReflectionHelper($this->object); + $this->reflection = new ReflectionHelper($this->hookManager); } - protected function tearDown() : void + private function initPathProvider(string $folder): void { - $this->object = NULL; - $this->reflection = NULL; + $this->pathProvider + ->method('getCustom') + ->willReturn($this->filesPath . '/' . $folder . '/custom/Espo/Custom/'); + + $this->pathProvider + ->method('getCore') + ->willReturn($this->filesPath . '/' . $folder . '/application/Espo/'); + + $this->pathProvider + ->method('getModule') + ->willReturnCallback( + function (?string $moduleName) use ($folder): string { + $path = $this->filesPath . '/' . $folder . '/application/Espo/Modules/{*}/'; + + if ($moduleName === null) { + return $path; + } + + return str_replace('{*}', $moduleName, $path); + } + ); } - public function testIsHookExists() + public function testHookExists(): void { $data = array ( 'Espo\\Hooks\\Note\\Stream' => 8, @@ -237,11 +254,7 @@ class HookManagerTest extends \PHPUnit\Framework\TestCase public function testCase1CustomHook() { - $this->reflection->setProperty('paths', array( - 'corePath' => $this->filesPath . '/testCase1/application/Espo/Hooks', - 'modulePath' => $this->filesPath . '/testCase1/application/Espo/Modules/{*}/Hooks', - 'customPath' => $this->filesPath . '/testCase1/custom/Espo/Custom/Hooks', - )); + $this->initPathProvider('testCase1'); $this->config ->expects($this->exactly(2)) @@ -262,25 +275,21 @@ class HookManagerTest extends \PHPUnit\Framework\TestCase 'Note' => array ( 'beforeSave' => - array ( - array ( + [ + [ 'className' => 'tests\\unit\\testData\\Hooks\\testCase1\\custom\\Espo\\Custom\\Hooks\\Note\\Mentions', 'order' => 7, - ), - ), + ], + ], ), ); $this->assertEquals($result, $this->reflection->getProperty('data')); } - public function testCase2ModuleHook() + public function testCase2ModuleHook1() { - $this->reflection->setProperty('paths', array( - 'corePath' => $this->filesPath . '/testCase2/application/Espo/Hooks', - 'modulePath' => $this->filesPath . '/testCase2/application/Espo/Modules/{*}/Hooks', - 'customPath' => $this->filesPath . '/testCase2/custom/Espo/Custom/Hooks', - )); + $this->initPathProvider('testCase2'); $this->config ->expects($this->exactly(2)) @@ -316,11 +325,7 @@ class HookManagerTest extends \PHPUnit\Framework\TestCase public function testCase2ModuleHookReverseModuleOrder() { - $this->reflection->setProperty('paths', array( - 'corePath' => $this->filesPath . '/testCase2/application/Espo/Hooks', - 'modulePath' => $this->filesPath . '/testCase2/application/Espo/Modules/{*}/Hooks', - 'customPath' => $this->filesPath . '/testCase2/custom/Espo/Custom/Hooks', - )); + $this->initPathProvider('testCase2'); $this->config ->expects($this->exactly(2)) @@ -356,11 +361,7 @@ class HookManagerTest extends \PHPUnit\Framework\TestCase public function testCase3CoreHook() { - $this->reflection->setProperty('paths', array( - 'corePath' => $this->filesPath . '/testCase3/application/Espo/Hooks', - 'modulePath' => $this->filesPath . '/testCase3/application/Espo/Modules/{*}/Hooks', - 'customPath' => $this->filesPath . '/testCase3/custom/Espo/Custom/Hooks', - )); + $this->initPathProvider('testCase3'); $this->config ->expects($this->exactly(2)) @@ -390,79 +391,4 @@ class HookManagerTest extends \PHPUnit\Framework\TestCase $this->assertEquals($result, $this->reflection->getProperty('data')); } - - public function noTestGetHookList() - { - $this->reflection->setProperty('data', array ( - 'Common' => - array ( - 'afterSave' => - array ( - array ( - 'className' => 'Espo\\Hooks\\Common\\AssignmentEmailNotification', - 'order' => 9, - ), - array ( - 'className' => 'Espo\\Hooks\\Common\\Stream', - 'order' => 9, - ), - array ( - 'className' => 'Espo\\Hooks\\Common\\Notifications', - 'order' => 10, - ), - ), - 'beforeSave' => - array ( - array ( - 'className' => 'Espo\\Hooks\\Common\\CurrencyConverted', - 'order' => 1, - ), - array ( - 'className' => 'Espo\\Hooks\\Common\\Formula', - 'order' => 5, - ), - array ( - 'className' => 'Espo\\Hooks\\Common\\NextNumber', - 'order' => 10, - ), - ), - ), - 'Note' => - array ( - 'beforeSave' => - array ( - array ( - 'className' => 'Espo\\Hooks\\Note\\Mentions', - 'order' => 9, - ), - ), - 'afterSave' => - array ( - array ( - 'className' => 'Espo\\Hooks\\Note\\Btest', - 'order' => 9, - ), - array ( - 'className' => 'Espo\\Hooks\\Note\\Notifications', - 'order' => 14, - ), - ), - ), - )); - - $resultBeforeSave = array( - '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', - ); - } }