From 8612be582c571f31e3c4f6a5ee2c3180d9f27f25 Mon Sep 17 00:00:00 2001 From: Coleman Watts Date: Thu, 23 Mar 2023 08:40:21 -0400 Subject: [PATCH] Code cleanup on aisle CRM_Utils_Hook --- CRM/Utils/Hook.php | 51 +++++++++---------- tests/phpunit/CRM/Utils/HookTest.php | 9 ++-- .../Civi/Core/Event/GenericHookEventTest.php | 3 +- tests/phpunit/E2E/Core/HookTest.php | 3 +- 4 files changed, 33 insertions(+), 33 deletions(-) diff --git a/CRM/Utils/Hook.php b/CRM/Utils/Hook.php index a92fa79352..724a7fe42d 100644 --- a/CRM/Utils/Hook.php +++ b/CRM/Utils/Hook.php @@ -50,7 +50,7 @@ abstract class CRM_Utils_Hook { * * @var CRM_Utils_Hook */ - static private $_singleton = NULL; + static private $_singleton; /** * @var bool @@ -75,7 +75,7 @@ abstract class CRM_Utils_Hook { * @return CRM_Utils_Hook * An instance of $config->userHookClass */ - public static function singleton($fresh = FALSE) { + public static function singleton($fresh = FALSE): CRM_Utils_Hook { if (self::$_singleton == NULL || $fresh) { $config = CRM_Core_Config::singleton(); $class = $config->userHookClass; @@ -87,7 +87,7 @@ abstract class CRM_Utils_Hook { /** * CRM_Utils_Hook constructor. * - * @throws \CRM_Core_Exception + * @throws CRM_Core_Exception */ public function __construct() { $this->cache = CRM_Utils_Cache::create([ @@ -165,7 +165,7 @@ abstract class CRM_Utils_Hook { $names, [&$arg1, &$arg2, &$arg3, &$arg4, &$arg5, &$arg6] ); - \Civi::dispatcher()->dispatch('hook_' . $fnSuffix, $event); + Civi::dispatcher()->dispatch('hook_' . $fnSuffix, $event); return $event->getReturnValues(); } @@ -181,6 +181,7 @@ abstract class CRM_Utils_Hook { * @param $fnPrefix * * @return array|bool + * @throws CRM_Core_Exception */ public function commonInvoke( $numParams, @@ -262,7 +263,6 @@ abstract class CRM_Utils_Hook { } foreach ($fnNames as $fnName) { - $fResult = []; switch ($numParams) { case 0: $fResult = $fnName(); @@ -296,9 +296,7 @@ abstract class CRM_Utils_Hook { throw new CRM_Core_Exception(ts('Invalid hook invocation')); } - if (!empty($fResult) && - is_array($fResult) - ) { + if (!empty($fResult) && is_array($fResult)) { $result = array_merge($result, $fResult); } } @@ -346,7 +344,7 @@ abstract class CRM_Utils_Hook { */ public static function pre($op, $objectName, $id, &$params = []) { $event = new \Civi\Core\Event\PreEvent($op, $objectName, $id, $params); - \Civi::dispatcher()->dispatch('hook_civicrm_pre', $event); + Civi::dispatcher()->dispatch('hook_civicrm_pre', $event); return $event->getReturnValues(); } @@ -368,7 +366,7 @@ abstract class CRM_Utils_Hook { */ public static function post($op, $objectName, $objectId, &$objectRef = NULL) { $event = new \Civi\Core\Event\PostEvent($op, $objectName, $objectId, $objectRef); - \Civi::dispatcher()->dispatch('hook_civicrm_post', $event); + Civi::dispatcher()->dispatch('hook_civicrm_post', $event); return $event->getReturnValues(); } @@ -398,7 +396,7 @@ abstract class CRM_Utils_Hook { */ public static function postCommit($op, $objectName, $objectId, $objectRef = NULL) { $event = new \Civi\Core\Event\PostEvent($op, $objectName, $objectId, $objectRef); - \Civi::dispatcher()->dispatch('hook_civicrm_postCommit', $event); + Civi::dispatcher()->dispatch('hook_civicrm_postCommit', $event); return $event->getReturnValues(); } @@ -423,7 +421,6 @@ abstract class CRM_Utils_Hook { * the return value is ignored */ public static function links($op, $objectName, &$objectId, &$links, &$mask = NULL, &$values = []) { - $null = NULL; return self::singleton()->invoke(['op', 'objectName', 'objectId', 'links', 'mask', 'values'], $op, $objectName, $objectId, $links, $mask, $values, 'civicrm_links'); } @@ -1032,13 +1029,10 @@ abstract class CRM_Utils_Hook { * Only add/edit/remove the specific field options you intend to affect. * @param bool $detailedFormat * If true, the options are in an ID => array ( 'id' => ID, 'label' => label, 'value' => value ) format - * @param array $selectAttributes - * Contain select attribute(s) if any. * * @return mixed */ - public static function customFieldOptions($customFieldID, &$options, $detailedFormat = FALSE, $selectAttributes = []) { - // Weird: $selectAttributes is inputted but not outputted. + public static function customFieldOptions($customFieldID, &$options, $detailedFormat = FALSE) { $null = NULL; return self::singleton()->invoke(['customFieldID', 'options', 'detailedFormat'], $customFieldID, $options, $detailedFormat, $null, $null, $null, @@ -1964,7 +1958,7 @@ abstract class CRM_Utils_Hook { */ public static function install() { // Actually invoke via CRM_Extension_Manager_Module::callHook - throw new \RuntimeException(sprintf("The method %s::%s is just a documentation stub and should not be invoked directly.", __CLASS__, __FUNCTION__)); + throw new RuntimeException(sprintf("The method %s::%s is just a documentation stub and should not be invoked directly.", __CLASS__, __FUNCTION__)); } /** @@ -1982,7 +1976,7 @@ abstract class CRM_Utils_Hook { */ public static function postInstall() { // Actually invoke via CRM_Extension_Manager_Module::callHook - throw new \RuntimeException(sprintf("The method %s::%s is just a documentation stub and should not be invoked directly.", __CLASS__, __FUNCTION__)); + throw new RuntimeException(sprintf("The method %s::%s is just a documentation stub and should not be invoked directly.", __CLASS__, __FUNCTION__)); } /** @@ -1992,7 +1986,7 @@ abstract class CRM_Utils_Hook { */ public static function uninstall() { // Actually invoke via CRM_Extension_Manager_Module::callHook - throw new \RuntimeException(sprintf("The method %s::%s is just a documentation stub and should not be invoked directly.", __CLASS__, __FUNCTION__)); + throw new RuntimeException(sprintf("The method %s::%s is just a documentation stub and should not be invoked directly.", __CLASS__, __FUNCTION__)); } /** @@ -2002,7 +1996,7 @@ abstract class CRM_Utils_Hook { */ public static function enable() { // Actually invoke via CRM_Extension_Manager_Module::callHook - throw new \RuntimeException(sprintf("The method %s::%s is just a documentation stub and should not be invoked directly.", __CLASS__, __FUNCTION__)); + throw new RuntimeException(sprintf("The method %s::%s is just a documentation stub and should not be invoked directly.", __CLASS__, __FUNCTION__)); } /** @@ -2012,7 +2006,7 @@ abstract class CRM_Utils_Hook { */ public static function disable() { // Actually invoke via CRM_Extension_Manager_Module::callHook - throw new \RuntimeException(sprintf("The method %s::%s is just a documentation stub and should not be invoked directly.", __CLASS__, __FUNCTION__)); + throw new RuntimeException(sprintf("The method %s::%s is just a documentation stub and should not be invoked directly.", __CLASS__, __FUNCTION__)); } /** @@ -2057,7 +2051,7 @@ abstract class CRM_Utils_Hook { * * @param string $op * The type of operation being performed; 'check' or 'enqueue'. - * @param CRM_Queue_Queue $queue + * @param CRM_Queue_Queue|null $queue * (for 'enqueue') the modifiable list of pending up upgrade tasks. * * @return bool|null @@ -2288,13 +2282,11 @@ abstract class CRM_Utils_Hook { /** * @param CRM_Core_Exception $exception - * @param mixed $request - * Reserved for future use. */ - public static function unhandledException($exception, $request = NULL) { + public static function unhandledException($exception) { $null = NULL; $event = new \Civi\Core\Event\UnhandledExceptionEvent($exception, $null); - \Civi::dispatcher()->dispatch('hook_civicrm_unhandled_exception', $event); + Civi::dispatcher()->dispatch('hook_civicrm_unhandled_exception', $event); } /** @@ -2672,7 +2664,7 @@ abstract class CRM_Utils_Hook { */ public static function caseChange(\Civi\CCase\Analyzer $analyzer) { $event = new \Civi\CCase\Event\CaseChangeEvent($analyzer); - \Civi::dispatcher()->dispatch('hook_civicrm_caseChange', $event); + Civi::dispatcher()->dispatch('hook_civicrm_caseChange', $event); } /** @@ -2892,6 +2884,8 @@ abstract class CRM_Utils_Hook { * @param array $outcomes * The outcomes of each task. One of 'ok', 'retry', 'fail'. * Keys should match the keys in $items. + * @return mixed + * @throws CRM_Core_Exception */ public static function queueRun(CRM_Queue_Queue $queue, array $items, &$outcomes) { $runner = $queue->getSpec('runner'); @@ -2941,6 +2935,7 @@ abstract class CRM_Utils_Hook { * The default outcome for task-errors is determined by the queue settings (`civicrm_queue.error`). * @param \Throwable|null $exception * If the task failed, this is the cause of the failure. + * @return mixed */ public static function queueTaskError(CRM_Queue_Queue $queue, $item, &$outcome, ?Throwable $exception) { $null = NULL; @@ -3007,7 +3002,7 @@ abstract class CRM_Utils_Hook { } /** - * ALlow Extensions to custom process IPN hook data such as sending Google Analyitcs information based on the IPN + * Allow Extensions to custom process IPN hook data such as sending Google Analytics information based on the IPN * @param array $IPNData - Array of IPN Data * @return mixed */ diff --git a/tests/phpunit/CRM/Utils/HookTest.php b/tests/phpunit/CRM/Utils/HookTest.php index a74e543bfc..882db79ed2 100644 --- a/tests/phpunit/CRM/Utils/HookTest.php +++ b/tests/phpunit/CRM/Utils/HookTest.php @@ -52,7 +52,8 @@ class CRM_Utils_HookTest extends CiviUnitTestCase { */ public function testRunHooks_reentrancy() { $arg1 = 'whatever'; - $this->hook->runHooks($this->fakeModules, 'civicrm_testRunHooks_outer', 1, $arg1, CRM_Utils_Hook::$_nullObject, CRM_Utils_Hook::$_nullObject, CRM_Utils_Hook::$_nullObject, CRM_Utils_Hook::$_nullObject, CRM_Utils_Hook::$_nullObject); + $null = NULL; + $this->hook->runHooks($this->fakeModules, 'civicrm_testRunHooks_outer', 1, $arg1, $null, $null, $null, $null, $null); $this->assertEquals( [ 'a-outer', @@ -70,7 +71,8 @@ class CRM_Utils_HookTest extends CiviUnitTestCase { * Verify that the results of runHooks() are correctly merged */ public function testRunHooks_merge() { - $result = $this->hook->runHooks($this->fakeModules, 'civicrm_testRunHooks_merge', 0, CRM_Utils_Hook::$_nullObject, CRM_Utils_Hook::$_nullObject, CRM_Utils_Hook::$_nullObject, CRM_Utils_Hook::$_nullObject, CRM_Utils_Hook::$_nullObject, CRM_Utils_Hook::$_nullObject); + $null = NULL; + $result = $this->hook->runHooks($this->fakeModules, 'civicrm_testRunHooks_merge', 0, $null, $null, $null, $null, $null, $null); $this->assertEquals( [ 'from-module-a1', @@ -96,7 +98,8 @@ function hooktesta_civicrm_testRunHooks_outer() { function hooktestb_civicrm_testRunHooks_outer() { $test = CRM_Utils_HookTest::$activeTest; $test->log[] = 'b-outer-1'; - $test->hook->runHooks($test->fakeModules, 'civicrm_testRunHooks_inner', 0, CRM_Utils_Hook::$_nullObject, CRM_Utils_Hook::$_nullObject, CRM_Utils_Hook::$_nullObject, CRM_Utils_Hook::$_nullObject, CRM_Utils_Hook::$_nullObject, CRM_Utils_Hook::$_nullObject); + $null = NULL; + $test->hook->runHooks($test->fakeModules, 'civicrm_testRunHooks_inner', 0, $null, $null, $null, $null, $null, $null); $test->log[] = 'b-outer-2'; } diff --git a/tests/phpunit/Civi/Core/Event/GenericHookEventTest.php b/tests/phpunit/Civi/Core/Event/GenericHookEventTest.php index 3ae516c552..85971846ae 100644 --- a/tests/phpunit/Civi/Core/Event/GenericHookEventTest.php +++ b/tests/phpunit/Civi/Core/Event/GenericHookEventTest.php @@ -26,9 +26,10 @@ class GenericHookEventTest extends \CiviUnitTestCase { } public function testConstructOrdered() { + $null = NULL; $event = GenericHookEvent::createOrdered( ['alpha', 'beta', 'nothingNull', 'nothingZero'], - [456, ['whiz' => 'bang'], NULL, 0, \CRM_Utils_Hook::$_nullObject] + [456, ['whiz' => 'bang'], NULL, 0, $null] ); $this->assertEquals(456, $event->alpha); $this->assertEquals('bang', $event->beta['whiz']); diff --git a/tests/phpunit/E2E/Core/HookTest.php b/tests/phpunit/E2E/Core/HookTest.php index bfadfe0838..c159003c17 100644 --- a/tests/phpunit/E2E/Core/HookTest.php +++ b/tests/phpunit/E2E/Core/HookTest.php @@ -45,8 +45,9 @@ class HookTest extends \CiviEndToEndTestCase { * @return mixed */ private function hookStub($names, &$a, $b) { + $null = NULL; return \CRM_Utils_Hook::singleton() - ->invoke($names, $a, $b, \CRM_Utils_Hook::$_nullObject, \CRM_Utils_Hook::$_nullObject, \CRM_Utils_Hook::$_nullObject, \CRM_Utils_Hook::$_nullObject, + ->invoke($names, $a, $b, $null, $null, $null, $null, 'civicrm_e2eHookExample'); } -- 2.25.1