From d04035dd3fc62b91ffd5f318e751ec0f095cb3b2 Mon Sep 17 00:00:00 2001 From: Taras Machyshyn Date: Mon, 13 Nov 2017 13:59:30 +0200 Subject: [PATCH] HookManager improvements --- application/Espo/Core/HookManager.php | 73 +++++--- tests/unit/Espo/Core/HookManagerTest.php | 203 +++++++++++++++++------ 2 files changed, 199 insertions(+), 77 deletions(-) diff --git a/application/Espo/Core/HookManager.php b/application/Espo/Core/HookManager.php index 7470a99bef..8be8f89a1f 100644 --- a/application/Espo/Core/HookManager.php +++ b/application/Espo/Core/HookManager.php @@ -37,6 +37,8 @@ class HookManager private $data; + private $hookListHash = array(); + private $hooks; protected $cacheFile = 'data/cache/application/hooks.php'; @@ -105,18 +107,10 @@ class HookManager $this->loadHooks(); } - $hookList = array(); + $hookList = $this->getHookList($scope, $hookName); - if (isset($this->data['Common'])) { - $hookList = $this->data['Common']; - } - - if (isset($this->data[$scope])) { - $hookList = $this->mergeHooks($hookList, $this->data[$scope]); - } - - if (!empty($hookList[$hookName])) { - foreach ($hookList[$hookName] as $className => $classOrder) { + if (!empty($hookList)) { + foreach ($hookList as $className) { if (empty($this->hooks[$className])) { $this->hooks[$className] = $this->createHookByClassName($className); if (empty($this->hooks[$className])) continue; @@ -174,8 +168,11 @@ class HookManager foreach($hookMethods as $hookType) { $entityHookData = isset($hookData[$normalizedScopeName][$hookType]) ? $hookData[$normalizedScopeName][$hookType] : array(); - if (!$this->isHookExists($className, $entityHookData)) { - $hookData[$normalizedScopeName][$hookType][$className] = $className::$order; + if (!$this->hookExists($className, $entityHookData)) { + $hookData[$normalizedScopeName][$hookType][] = array( + 'className' => $className, + 'order' => $className::$order + ); } } } @@ -198,7 +195,7 @@ class HookManager { foreach ($hooks as $scopeName => &$scopeHooks) { foreach ($scopeHooks as $hookName => &$hookList) { - asort($hookList); + usort($hookList, array($this, 'cmpHooks')); } } @@ -206,21 +203,38 @@ class HookManager } /** - * Merge hooks for two entities + * Get sorted hook list * - * @param array $hookList1 - * @param array $hookList2 + * @param string $scope + * @param string $hookName * * @return array */ - protected function mergeHooks(array $hookList1, array $hookList2) + protected function getHookList($scope, $hookName) { - $mergedHookList = array_merge_recursive($hookList1, $hookList2); - foreach ($mergedHookList as $hookType => &$hookList) { - asort($hookList); + $key = $scope . '_' . $hookName; + + if (!isset($this->hookListHash[$key])) { + $hookList = array(); + + if (isset($this->data['Common'][$hookName])) { + $hookList = $this->data['Common'][$hookName]; + } + + if (isset($this->data[$scope][$hookName])) { + $hookList = array_merge($hookList, $this->data[$scope][$hookName]); + usort($hookList, array($this, 'cmpHooks')); + } + + $normalizedList = array(); + foreach ($hookList as $hookData) { + $normalizedList[] = $hookData['className']; + } + + $this->hookListHash[$key] = $normalizedList; } - return $mergedHookList; + return $this->hookListHash[$key]; } /** @@ -231,16 +245,25 @@ class HookManager * * @return boolean */ - protected function isHookExists($className, array $hookData) + protected function hookExists($className, array $hookData) { $class = preg_replace('/^.*\\\(.*)$/', '$1', $className); - foreach ($hookData as $hookName => $hookOrder) { - if (preg_match('/\\\\'.$class.'$/', $hookName)) { + foreach ($hookData as $hookData) { + if (preg_match('/\\\\'.$class.'$/', $hookData['className'])) { return true; } } return false; } + + protected function cmpHooks($a, $b) + { + if ($a['order'] == $b['order']) { + return 0; + } + + return ($a['order'] < $b['order']) ? -1 : 1; + } } \ No newline at end of file diff --git a/tests/unit/Espo/Core/HookManagerTest.php b/tests/unit/Espo/Core/HookManagerTest.php index e9fb248fe9..d4262b37a1 100644 --- a/tests/unit/Espo/Core/HookManagerTest.php +++ b/tests/unit/Espo/Core/HookManagerTest.php @@ -76,11 +76,26 @@ class HookManagerTest extends \PHPUnit_Framework_TestCase '\\Espo\\Hooks\\Note\\Notifications' => 14, ); - $this->assertTrue( $this->reflection->invokeMethod('isHookExists', array('\\Espo\\Hooks\\Note\\Mentions', $data)) ); - $this->assertTrue( $this->reflection->invokeMethod('isHookExists', array('\\Espo\\Modules\\Crm\\Hooks\\Note\\Mentions', $data)) ); - $this->assertTrue( $this->reflection->invokeMethod('isHookExists', array('\\Espo\\Modules\\Test\\Hooks\\Note\\Mentions', $data)) ); - $this->assertTrue( $this->reflection->invokeMethod('isHookExists', array('\\Espo\\Modules\\Test\\Hooks\\Common\\Stream', $data)) ); - $this->assertFalse( $this->reflection->invokeMethod('isHookExists', array('\\Espo\\Hooks\\Note\\TestHook', $data)) ); + $data = array ( + array ( + 'className' => '\\Espo\\Hooks\\Note\\Stream', + 'order' => 8, + ), + array ( + 'className' => '\\Espo\\Hooks\\Note\\Mentions', + 'order' => 9, + ), + array ( + '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)) ); } public function testSortHooks() @@ -90,26 +105,50 @@ class HookManagerTest extends \PHPUnit_Framework_TestCase array ( 'afterSave' => array ( - '\\Espo\\Hooks\\Common\\AssignmentEmailNotification' => 9, - '\\Espo\\Hooks\\Common\\Notifications' => 10, - '\\Espo\\Hooks\\Common\\Stream' => 9, + array ( + 'className' => '\\Espo\\Hooks\\Common\\AssignmentEmailNotification', + 'order' => 9, + ), + array ( + 'className' => '\\Espo\\Hooks\\Common\\Notifications', + 'order' => 10, + ), + array ( + 'className' => '\\Espo\\Hooks\\Common\\Stream', + 'order' => 9, + ), ), 'beforeSave' => array ( - '\\Espo\\Hooks\\Common\\Formula' => 5, - '\\Espo\\Hooks\\Common\\NextNumber' => 10, - '\\Espo\\Hooks\\Common\\CurrencyConverted' => 1, + array ( + 'className' => '\\Espo\\Hooks\\Common\\Formula', + 'order' => 5, + ), + array ( + 'className' => '\\Espo\\Hooks\\Common\\NextNumber', + 'order' => 10, + ), + array ( + 'className' => '\\Espo\\Hooks\\Common\\CurrencyConverted', + 'order' => 1, + ), ), ), 'Note' => array ( 'beforeSave' => array ( - '\\Espo\\Hooks\\Note\\Mentions' => 9, + array ( + 'className' => '\\Espo\\Hooks\\Note\\Mentions', + 'order' => 9, + ), ), 'afterSave' => array ( - '\\Espo\\Hooks\\Note\\Notifications' => 14, + array ( + 'className' => '\\Espo\\Hooks\\Note\\Notifications', + 'order' => 14, + ), ), ), ); @@ -119,26 +158,50 @@ class HookManagerTest extends \PHPUnit_Framework_TestCase array ( 'afterSave' => array ( - '\\Espo\\Hooks\\Common\\AssignmentEmailNotification' => 9, - '\\Espo\\Hooks\\Common\\Stream' => 9, - '\\Espo\\Hooks\\Common\\Notifications' => 10, + 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 ( - '\\Espo\\Hooks\\Common\\CurrencyConverted' => 1, - '\\Espo\\Hooks\\Common\\Formula' => 5, - '\\Espo\\Hooks\\Common\\NextNumber' => 10, + 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 ( - '\\Espo\\Hooks\\Note\\Mentions' => 9, + array ( + 'className' => '\\Espo\\Hooks\\Note\\Mentions', + 'order' => 9, + ), ), 'afterSave' => array ( - '\\Espo\\Hooks\\Note\\Notifications' => 14, + array ( + 'className' => '\\Espo\\Hooks\\Note\\Notifications', + 'order' => 14, + ), ), ), ); @@ -174,7 +237,10 @@ class HookManagerTest extends \PHPUnit_Framework_TestCase array ( 'beforeSave' => array ( - '\\tests\\unit\\testData\\Hooks\\testCase1\\custom\\Espo\\Custom\\Hooks\\Note\\Mentions' => 7, + array ( + 'className' => '\\tests\\unit\\testData\\Hooks\\testCase1\\custom\\Espo\\Custom\\Hooks\\Note\\Mentions', + 'order' => 7, + ), ), ), ); @@ -210,7 +276,10 @@ class HookManagerTest extends \PHPUnit_Framework_TestCase array ( 'beforeSave' => array ( - '\\tests\\unit\\testData\\Hooks\\testCase2\\application\\Espo\\Modules\\Crm\\Hooks\\Note\\Mentions' => 9, + array ( + 'className' => '\\tests\\unit\\testData\\Hooks\\testCase2\\application\\Espo\\Modules\\Crm\\Hooks\\Note\\Mentions', + 'order' => 9, + ), ), ), ); @@ -246,7 +315,10 @@ class HookManagerTest extends \PHPUnit_Framework_TestCase array ( 'beforeSave' => array ( - '\\tests\\unit\\testData\\Hooks\\testCase2\\application\\Espo\\Modules\\Test\\Hooks\\Note\\Mentions' => 9, + array ( + 'className' => '\\tests\\unit\\testData\\Hooks\\testCase2\\application\\Espo\\Modules\\Test\\Hooks\\Note\\Mentions', + 'order' => 9, + ), ), ), ); @@ -280,7 +352,10 @@ class HookManagerTest extends \PHPUnit_Framework_TestCase array ( 'beforeSave' => array ( - '\\tests\\unit\\testData\\Hooks\\testCase3\\application\\Espo\\Hooks\\Note\\Mentions' => 9, + array ( + 'className' => '\\tests\\unit\\testData\\Hooks\\testCase3\\application\\Espo\\Hooks\\Note\\Mentions', + 'order' => 9, + ), ), ), ); @@ -288,54 +363,78 @@ class HookManagerTest extends \PHPUnit_Framework_TestCase $this->assertEquals($result, $this->reflection->getProperty('data')); } - public function testMergeHooks() + public function testGetHookList() { - $data = array ( + $this->reflection->setProperty('data', array ( 'Common' => array ( 'afterSave' => array ( - '\\Espo\\Hooks\\Common\\AssignmentEmailNotification' => 7, - '\\Espo\\Hooks\\Common\\Stream' => 9, - '\\Espo\\Hooks\\Common\\Notifications' => 10, + 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 ( - '\\Espo\\Hooks\\Common\\CurrencyConverted' => 1, - '\\Espo\\Hooks\\Common\\Formula' => 5, - '\\Espo\\Hooks\\Common\\NextNumber' => 10, + 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 ( - '\\Espo\\Hooks\\Note\\Mentions' => 9, + array ( + 'className' => '\\Espo\\Hooks\\Note\\Mentions', + 'order' => 9, + ), ), 'afterSave' => array ( - '\\Espo\\Hooks\\Note\\Notifications' => 8, + 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', ); - $result = array( - 'afterSave' => - array ( - '\\Espo\\Hooks\\Common\\AssignmentEmailNotification' => 7, - '\\Espo\\Hooks\\Note\\Notifications' => 8, - '\\Espo\\Hooks\\Common\\Stream' => 9, - '\\Espo\\Hooks\\Common\\Notifications' => 10, - ), - 'beforeSave' => - array ( - '\\Espo\\Hooks\\Common\\CurrencyConverted' => 1, - '\\Espo\\Hooks\\Common\\Formula' => 5, - '\\Espo\\Hooks\\Note\\Mentions' => 9, - '\\Espo\\Hooks\\Common\\NextNumber' => 10, - ), + $resultAfterSave = array( + '\\Espo\\Hooks\\Common\\AssignmentEmailNotification', + '\\Espo\\Hooks\\Note\\Btest', + '\\Espo\\Hooks\\Common\\Stream', + '\\Espo\\Hooks\\Common\\Notifications', + '\\Espo\\Hooks\\Note\\Notifications', ); - - $this->assertEquals($result, $this->reflection->invokeMethod('mergeHooks', array($data['Common'], $data['Note']))); } } \ No newline at end of file