From 762dc04dcf9b61d964d24b7177289c79e104922c Mon Sep 17 00:00:00 2001 From: Tim Otten Date: Fri, 3 Mar 2017 12:15:09 -0800 Subject: [PATCH] CRM-19813 - GenericHookEvent - Bridge between Symfony Events and hooks The GenericHookEvent is used to expose all traditional hooks to the Symfony EventDispatcher. The traditional notation for a hook is based on a function signature: function hook_civicrm_foo($bar, &$whiz, &$bang); Symfony Events are based on a class with properties and methods. This requires some kind of mapping. Symfony Events has two conventions which might be used to support that mapping. One might implement event classes for every hook, or one might use the `GenericEvent`. This design-decision comes with a basic trade-off between size (total #files, #classes, #SLOC) and IDE assistance (docs/autocomplete): * `GenericEvent` has smaller size and less boiler-plate, but it also provides little IDE assistance. * Custom event classes provide more IDE assistance, but they also inflate the size (with lots of boilerplate). This patch implements `GenericHookEvent`, which is conceptually similar to `GenericEvent`, but it has a few modifications: * The `__get()` function returns references, which makes it easier to alter data. * The `getHookValues()` function returns an ordered list of hook arguments. The approach of `GenericEvent` / `GenericHookEvent` seems like a reasonable balance -- it starts out with little boilerplate, but we can incrementally introduce subclasses. The subclasses can: * Use docblocks for IDE support * Use declared properties for IDE support (though you may need to customize the constructor, etal). * Add semantic/businessy functions. * Override the `__get()` / `__set()` functions to be provide different getter/setter behavior. --- Civi/Core/CiviEventDispatcher.php | 135 ++++++++++++ Civi/Core/Container.php | 7 +- Civi/Core/Event/GenericHookEvent.php | 205 ++++++++++++++++++ .../Civi/Core/Event/GenericHookEventTest.php | 108 +++++++++ 4 files changed, 453 insertions(+), 2 deletions(-) create mode 100644 Civi/Core/CiviEventDispatcher.php create mode 100644 Civi/Core/Event/GenericHookEvent.php create mode 100644 tests/phpunit/Civi/Core/Event/GenericHookEventTest.php diff --git a/Civi/Core/CiviEventDispatcher.php b/Civi/Core/CiviEventDispatcher.php new file mode 100644 index 0000000000..a10fa63fd6 --- /dev/null +++ b/Civi/Core/CiviEventDispatcher.php @@ -0,0 +1,135 @@ + trueish). + */ + private $autoListeners = array(); + + /** + * Determine whether $eventName should delegate to the CMS hook system. + * + * @param string $eventName + * Ex: 'civi.token.eval', 'hook_civicrm_post`. + * @return bool + */ + protected function isHookEvent($eventName) { + return (substr($eventName, 0, 5) === 'hook_') && (strpos($eventName, '::') === FALSE); + } + + /** + * @inheritDoc + */ + public function dispatch($eventName, Event $event = NULL) { + $this->bindPatterns($eventName); + return parent::dispatch($eventName, $event); + } + + /** + * @inheritDoc + */ + public function getListeners($eventName = NULL) { + $this->bindPatterns($eventName); + return parent::getListeners($eventName); + } + + /** + * @inheritDoc + */ + public function hasListeners($eventName = NULL) { + // All hook_* events have default listeners, so hasListeners(NULL) is a truism. + return ($eventName === NULL || $this->isHookEvent($eventName)) + ? TRUE : parent::hasListeners($eventName); + } + + /** + * Invoke hooks using an event object. + * + * @param \Civi\Core\Event\GenericHookEvent $event + * @param string $eventName + * Ex: 'hook_civicrm_dashboard'. + */ + public static function delegateToUF($event, $eventName) { + $hookName = substr($eventName, 5); + $hooks = \CRM_Utils_Hook::singleton(); + $params = $event->getHookValues(); + $count = count($params); + + switch ($count) { + case 0: + $fResult = $hooks->invokeViaUF($count, \CRM_Utils_Hook::$_nullObject, \CRM_Utils_Hook::$_nullObject, \CRM_Utils_Hook::$_nullObject, \CRM_Utils_Hook::$_nullObject, \CRM_Utils_Hook::$_nullObject, \CRM_Utils_Hook::$_nullObject, $hookName); + break; + + case 1: + $fResult = $hooks->invokeViaUF($count, $params[0], \CRM_Utils_Hook::$_nullObject, \CRM_Utils_Hook::$_nullObject, \CRM_Utils_Hook::$_nullObject, \CRM_Utils_Hook::$_nullObject, \CRM_Utils_Hook::$_nullObject, $hookName); + break; + + case 2: + $fResult = $hooks->invokeViaUF($count, $params[0], $params[1], \CRM_Utils_Hook::$_nullObject, \CRM_Utils_Hook::$_nullObject, \CRM_Utils_Hook::$_nullObject, \CRM_Utils_Hook::$_nullObject, $hookName); + break; + + case 3: + $fResult = $hooks->invokeViaUF($count, $params[0], $params[1], $params[2], \CRM_Utils_Hook::$_nullObject, \CRM_Utils_Hook::$_nullObject, \CRM_Utils_Hook::$_nullObject, $hookName); + break; + + case 4: + $fResult = $hooks->invokeViaUF($count, $params[0], $params[1], $params[2], $params[3], \CRM_Utils_Hook::$_nullObject, \CRM_Utils_Hook::$_nullObject, $hookName); + break; + + case 5: + $fResult = $hooks->invokeViaUF($count, $params[0], $params[1], $params[2], $params[3], $params[4], \CRM_Utils_Hook::$_nullObject, $hookName); + break; + + case 6: + $fResult = $hooks->invokeViaUF($count, $params[0], $params[1], $params[2], $params[3], $params[4], $params[5], $hookName); + break; + + default: + throw new \RuntimeException("hook_{$hookName} cannot support more than 6 parameters"); + } + + $event->addReturnValues($fResult); + } + + /** + * @param string $eventName + * Ex: 'civi.api.resolve' or 'hook_civicrm_dashboard'. + */ + protected function bindPatterns($eventName) { + if ($eventName !== NULL && !isset($this->autoListeners[$eventName])) { + $this->autoListeners[$eventName] = 1; + if ($this->isHookEvent($eventName)) { + // WISHLIST: For native extensions (and possibly D6/D7/D8/BD), enumerate + // the listeners and list them one-by-one. This would make it easier to + // inspect via "cv debug:event-dispatcher". + $this->addListener($eventName, array( + '\Civi\Core\CiviEventDispatcher', + 'delegateToUF', + ), self::DEFAULT_HOOK_PRIORITY); + } + } + } + +} diff --git a/Civi/Core/Container.php b/Civi/Core/Container.php index d95b62aac1..cf59e2c58a 100644 --- a/Civi/Core/Container.php +++ b/Civi/Core/Container.php @@ -106,6 +106,9 @@ class Container { $container->addObjectResource($this); $container->setParameter('civicrm_base_path', $civicrm_base_path); //$container->set(self::SELF, $this); + + $container->addResource(new \Symfony\Component\Config\Resource\FileResource(__FILE__)); + $container->setDefinition(self::SELF, new Definition( 'Civi\Core\Container', array() @@ -134,7 +137,7 @@ class Container { ->setFactoryService(self::SELF)->setFactoryMethod('createAngularManager'); $container->setDefinition('dispatcher', new Definition( - 'Symfony\Component\EventDispatcher\ContainerAwareEventDispatcher', + 'Civi\Core\CiviEventDispatcher', array(new Reference('service_container')) )) ->setFactoryService(self::SELF)->setFactoryMethod('createEventDispatcher'); @@ -238,7 +241,7 @@ class Container { * @return \Symfony\Component\EventDispatcher\ContainerAwareEventDispatcher */ public function createEventDispatcher($container) { - $dispatcher = new ContainerAwareEventDispatcher($container); + $dispatcher = new CiviEventDispatcher($container); $dispatcher->addListener(SystemInstallEvent::EVENT_NAME, array('\Civi\Core\InstallationCanary', 'check')); $dispatcher->addListener(SystemInstallEvent::EVENT_NAME, array('\Civi\Core\DatabaseInitializer', 'initialize')); $dispatcher->addListener('hook_civicrm_post::Activity', array('\Civi\CCase\Events', 'fireCaseChange')); diff --git a/Civi/Core/Event/GenericHookEvent.php b/Civi/Core/Event/GenericHookEvent.php new file mode 100644 index 0000000000..4345c0c8d8 --- /dev/null +++ b/Civi/Core/Event/GenericHookEvent.php @@ -0,0 +1,205 @@ + $readOnlyData, + * 'second' => &readWriteData, + * )); + * Civi::service('dispatcher')->dispatch('hook_civicrm_foo', $event); + * print_r($event->getReturnValues()); + * @endCode + */ +class GenericHookEvent extends \Symfony\Component\EventDispatcher\Event { + + /** + * @var array + * Ex: array('contactID' => &$contactID, 'contentPlacement' => &$contentPlacement). + */ + protected $hookValues; + + /** + * @var array + * Ex: array(0 => 'contactID', 1 => 'contentPlacement'). + */ + protected $hookFields; + + /** + * @var array + * Ex: array('contactID' => 0, 'contentPlacement' => 1). + */ + protected $hookFieldsFlip; + + /** + * Some legacy hooks expect listener-functions to return a value. + * OOP listeners may set the $returnValue. + * + * This field is not recommended for use in new hooks. The return-value + * convention is not portable across different implementations of the hook + * system. Instead, it's more portable to provide an alterable, named field. + * + * @var mixed + * @deprecated + */ + private $returnValues = array(); + + /** + * Create a GenericHookEvent using key-value pairs. + * + * @param array $params + * Ex: array('contactID' => &$contactID, 'contentPlacement' => &$contentPlacement). + * @return \Civi\Core\Event\GenericHookEvent + */ + public static function create($params) { + $e = new static(); + $e->hookValues = array_values($params); + $e->hookFields = array_keys($params); + $e->hookFieldsFlip = array_flip($e->hookFields); + return $e; + } + + /** + * Create a GenericHookEvent using ordered parameters. + * + * @param array $hookFields + * Ex: array(0 => 'contactID', 1 => 'contentPlacement'). + * @param array $hookValues + * Ex: array(0 => &$contactID, 1 => &$contentPlacement). + * @return \Civi\Core\Event\GenericHookEvent + */ + public static function createOrdered($hookFields, $hookValues) { + $e = new static(); + if (count($hookValues) > count($hookFields)) { + $hookValues = array_slice($hookValues, 0, count($hookFields)); + } + $e->hookValues = $hookValues; + $e->hookFields = $hookFields; + $e->hookFieldsFlip = array_flip($e->hookFields); + return $e; + } + + /** + * @return array + * Ex: array(0 => &$contactID, 1 => &$contentPlacement). + */ + public function getHookValues() { + return $this->hookValues; + } + + /** + * @return mixed + * @deprecated + */ + public function getReturnValues() { + return empty($this->returnValues) ? TRUE : $this->returnValues; + } + + /** + * @param mixed $fResult + * @return GenericHookEvent + * @deprecated + */ + public function addReturnValues($fResult) { + if (!empty($fResult) && is_array($fResult)) { + $this->returnValues = array_merge($this->returnValues, $fResult); + } + return $this; + } + + /** + * @inheritDoc + */ + public function &__get($name) { + if (isset($this->hookFieldsFlip[$name])) { + return $this->hookValues[$this->hookFieldsFlip[$name]]; + } + } + + /** + * @inheritDoc + */ + public function __set($name, $value) { + if (isset($this->hookFieldsFlip[$name])) { + $this->hookValues[$this->hookFieldsFlip[$name]] = $value; + } + } + + /** + * @inheritDoc + */ + public function __isset($name) { + return isset($this->hookFieldsFlip[$name]) + && isset($this->hookValues[$this->hookFieldsFlip[$name]]); + } + + /** + * @inheritDoc + */ + public function __unset($name) { + if (isset($this->hookFieldsFlip[$name])) { + // Unset while preserving order. + $this->hookValues[$this->hookFieldsFlip[$name]] = NULL; + } + } + + /** + * Determine whether the hook supports the given field. + * + * The field may or may not be empty. Use isset() or empty() to + * check that. + * + * @param string $name + * @return bool + */ + public function hasField($name) { + return isset($this->hookFieldsFlip[$name]); + } + +} diff --git a/tests/phpunit/Civi/Core/Event/GenericHookEventTest.php b/tests/phpunit/Civi/Core/Event/GenericHookEventTest.php new file mode 100644 index 0000000000..c6cb27b479 --- /dev/null +++ b/tests/phpunit/Civi/Core/Event/GenericHookEventTest.php @@ -0,0 +1,108 @@ +reset(); + parent::tearDown(); + } + + public function testConstructParams() { + $event = GenericHookEvent::create(array( + 'ab' => 123, + 'cd' => array('foo' => 'bar'), + 'nothingNull' => NULL, + 'nothingZero' => 0, + )); + $this->assertEquals(123, $event->ab); + $this->assertEquals('bar', $event->cd['foo']); + $this->assertTrue($event->hasField('ab')); + $this->assertTrue(isset($event->ab)); + $this->assertFalse($event->hasField('abc')); + $this->assertFalse(isset($event->abc)); + $this->assertTrue(!isset($event->nothingNull) && empty($event->nothingNull)); + $this->assertTrue(isset($event->nothingZero) && empty($event->nothingZero)); + } + + public function testConstructOrdered() { + $event = GenericHookEvent::createOrdered( + array('alpha', 'beta', 'nothingNull', 'nothingZero'), + array(456, array('whiz' => 'bang'), NULL, 0, \CRM_Utils_Hook::$_nullObject) + ); + $this->assertEquals(456, $event->alpha); + $this->assertEquals('bang', $event->beta['whiz']); + $this->assertTrue($event->hasField('alpha')); + $this->assertTrue(isset($event->alpha)); + $this->assertFalse($event->hasField('ab')); + $this->assertFalse(isset($event->ab)); + $this->assertTrue(!isset($event->nothingNull) && empty($event->nothingNull)); + $this->assertTrue(isset($event->nothingZero) && empty($event->nothingZero)); + $this->assertEquals(4, count($event->getHookValues())); + } + + public function testDispatch() { + \CRM_Utils_Hook::singleton()->setHook('civicrm_ghet', + array($this, 'hook_civicrm_ghet')); + \Civi::service('dispatcher')->addListener('hook_civicrm_ghet', + array($this, 'onGhet')); + + $roString = 'readonly'; + $rwString = 'readwrite'; + $roArray = array('readonly'); + $rwArray = array('readwrite'); + $plainObj = new \stdClass(); + $refObj = new \stdClass(); + + $returnValue = $this->hookStub($roString, $rwString, $roArray, $rwArray, $plainObj, $refObj); + + $this->assertEquals('readonly', $roString); + $this->assertEquals('readwrite added-string-via-event added-string-via-hook', $rwString); + $this->assertEquals(array('readonly'), $roArray); + $this->assertEquals(array('readwrite', 'added-to-array-via-event', 'added-to-array-via-hook'), $rwArray); + $this->assertEquals('added-to-object-via-hook', $plainObj->prop1); + $this->assertEquals('added-to-object-via-hook', $refObj->prop2); + $this->assertEquals(array('early-running-result', 'late-running-result'), $returnValue); + } + + /** + * Fire a hook. This stub follows the same coding convention as + * CRM_Utils_Hook::*(). This ensures that the coding convention is valid. + * + * @param $roString + * @param $rwString + * @param $roArray + * @param $rwArray + * @param $plainObj + * @param $refObj + * @return mixed + */ + public function hookStub($roString, &$rwString, $roArray, &$rwArray, $plainObj, &$refObj) { + return \CRM_Utils_Hook::singleton()->invoke( + array('roString', 'rwString', 'roArray', 'rwArray', 'plainObj', 'refObj'), + $roString, $rwString, $roArray, $rwArray, $plainObj, $refObj, + 'civicrm_ghet' + ); + } + + public function hook_civicrm_ghet(&$roString, &$rwString, &$roArray, &$rwArray, $plainObj, &$refObj) { + $roString .= 'changes should not propagate back'; + $rwString .= ' added-string-via-hook'; + $roArray[] = 'changes should not propagate back'; + $rwArray[] = 'added-to-array-via-hook'; + $plainObj->prop1 = 'added-to-object-via-hook'; + $refObj->prop2 = 'added-to-object-via-hook'; + return array('late-running-result'); + } + + public function onGhet(GenericHookEvent $e) { + $e->roString .= 'changes should not propagate back'; + $e->rwString .= ' added-string-via-event'; + $e->roArray[] = 'changes should not propagate back'; + $e->rwArray[] = 'added-to-array-via-event'; + $e->plainObj->prop1 = 'added-to-object-via-event'; + $e->refObj->prop2 = 'added-to-object-via-event'; + $e->addReturnValues(array('early-running-result')); + } + +} -- 2.25.1