From 42ccedc7c6bdba553c36462d923f887aeb632e18 Mon Sep 17 00:00:00 2001 From: Tim Otten Date: Tue, 14 Jul 2020 00:06:46 -0700 Subject: [PATCH] Civi::dispatcher() - Close loopholes that occur during early boot Before ------ * The event-dispatcher is fully instantiated as a container service (`Container::createEventDispatcher()`). * It is not possible to add listeners to `Civi::dispatcher()` before the container is instantiated. * It is not possible to add listeners to `Civi::dispatcher()` in the top-level logic of an extension file (`myext.php`) * There is a distinction between "pre-boot" hooks (`hook_civicrm_container` or `hook_civicrm_entityType`) and normal hooks. Pre-boot hooks do not work with `Civi::dispatcher()`. After ----- * The event-dispatcher is instantiated as boot service (ie very early during bootstrap). * To preserve compatibility (with `RegisterListenersPass`, with any downstream modifications of `createEventDispatcher`, and with any dependency-injection) the event-dispatcher will still use `createEventDispatcher()` for extra intialization. * It is possible to add listeners to `Civi::dispatcher()` in several more places, e.g. at the top-level of an extension's `.php` file and in `CRM_Utils_System_*::initialize()` * There is no distinction between "preboot" hooks and normal hooks. `Civi::dispatcher()` can register listeners for all hooks. * To ensure that we do not accidentally re-create "preboot" hooks, this patch enables a "dispatch policy" => if any hook fires too early, it will throw an exception. --- CRM/Utils/Hook.php | 14 -------------- Civi.php | 9 ++++++++- Civi/Core/CiviEventDispatcher.php | 3 +++ Civi/Core/Container.php | 19 ++++++++++++------- 4 files changed, 23 insertions(+), 22 deletions(-) diff --git a/CRM/Utils/Hook.php b/CRM/Utils/Hook.php index 38c1da128e..5a5e612c05 100644 --- a/CRM/Utils/Hook.php +++ b/CRM/Utils/Hook.php @@ -152,20 +152,6 @@ abstract class CRM_Utils_Hook { &$arg1, &$arg2, &$arg3, &$arg4, &$arg5, &$arg6, $fnSuffix ) { - if (!\Civi\Core\Container::isContainerBooted()) { - $prebootHooks = ['civicrm_container', 'civicrm_entityTypes']; - // 'civicrm_config' ? - if (in_array($fnSuffix, $prebootHooks)) { - $count = is_array($names) ? count($names) : $names; - return $this->invokeViaUF($count, $arg1, $arg2, $arg3, $arg4, $arg5, $arg6, $fnSuffix); - } - else { - // TODO: Emit a warning, eg - // error_log("Warning: hook_$fnSuffix fired prematurely. Dropped."); - return; - } - } - if (!is_array($names)) { // We were called with the old contract wherein $names is actually an int. // Symfony dispatcher requires some kind of name. diff --git a/Civi.php b/Civi.php index 95009e35c3..f916756883 100644 --- a/Civi.php +++ b/Civi.php @@ -64,7 +64,14 @@ class Civi { * @return \Symfony\Component\EventDispatcher\EventDispatcherInterface */ public static function dispatcher() { - return Civi\Core\Container::singleton()->get('dispatcher'); + // NOTE: The dispatcher object is initially created as a boot service + // (ie `dispatcher.boot`). For compatibility with the container (eg + // `RegisterListenersPass` and `createEventDispatcher` addons), + // it is also available as the `dispatcher` service. + // + // The 'dispatcher.boot' and 'dispatcher' services are the same object, + // but 'dispatcher.boot' is resolvable earlier during bootstrap. + return Civi\Core\Container::getBootService('dispatcher.boot'); } /** diff --git a/Civi/Core/CiviEventDispatcher.php b/Civi/Core/CiviEventDispatcher.php index 9f3da3215e..ac104537d8 100644 --- a/Civi/Core/CiviEventDispatcher.php +++ b/Civi/Core/CiviEventDispatcher.php @@ -117,6 +117,9 @@ class CiviEventDispatcher extends EventDispatcher { case 'fail': throw new \RuntimeException("The dispatch policy prohibits event \"$eventName\"."); + case 'not-ready': + throw new \RuntimeException("CiviCRM has not bootstrapped sufficiently to fire event \"$eventName\"."); + default: throw new \RuntimeException("The dispatch policy for \"$eventName\" is unrecognized ($mode)."); diff --git a/Civi/Core/Container.php b/Civi/Core/Container.php index 15c7860a9d..c8a107014b 100644 --- a/Civi/Core/Container.php +++ b/Civi/Core/Container.php @@ -125,7 +125,7 @@ class Container { $container->setDefinition('dispatcher', new Definition( 'Civi\Core\CiviEventDispatcher', - [new Reference('service_container')] + [] )) ->setFactory([new Reference(self::SELF), 'createEventDispatcher'])->setPublic(TRUE); @@ -324,14 +324,11 @@ class Container { } /** - * @param \Symfony\Component\DependencyInjection\ContainerInterface $container * @return \Symfony\Component\EventDispatcher\EventDispatcher */ - public function createEventDispatcher($container) { - $dispatcher = new CiviEventDispatcher(); - if (\CRM_Core_Config::isUpgradeMode()) { - $dispatcher->setDispatchPolicy(\CRM_Upgrade_DispatchPolicy::get('upgrade.main')); - } + public function createEventDispatcher() { + // Continue building on the original dispatcher created during bootstrap. + $dispatcher = static::getBootService('dispatcher.boot'); $dispatcher->addListener('civi.core.install', ['\Civi\Core\InstallationCanary', 'check']); $dispatcher->addListener('civi.core.install', ['\Civi\Core\DatabaseInitializer', 'initialize']); @@ -501,6 +498,12 @@ class Container { $bootServices['paths'] = new \Civi\Core\Paths(); + $bootServices['dispatcher.boot'] = new CiviEventDispatcher(); + $bootServices['dispatcher.boot']->setDispatchPolicy([ + // Quality control: during pre-boot, we can register hook listeners - but not dispatch them. + '/./' => 'not-ready', + ]); + $class = $runtime->userFrameworkClass; $bootServices['userSystem'] = $userSystem = new $class(); $userSystem->initialize(); @@ -517,6 +520,8 @@ class Container { $bootServices['lockManager'] = self::createLockManager(); + $bootServices['dispatcher.boot']->setDispatchPolicy(\CRM_Core_Config::isUpgradeMode() ? \CRM_Upgrade_DispatchPolicy::get('upgrade.main') : NULL); + if ($loadFromDB && $runtime->dsn) { \CRM_Core_DAO::init($runtime->dsn); \CRM_Utils_Hook::singleton(TRUE); -- 2.25.1