Civi::dispatcher() - Close loopholes that occur during early boot
authorTim Otten <totten@civicrm.org>
Tue, 14 Jul 2020 07:06:46 +0000 (00:06 -0700)
committerTim Otten <totten@civicrm.org>
Tue, 14 Jul 2020 10:54:32 +0000 (03:54 -0700)
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
Civi.php
Civi/Core/CiviEventDispatcher.php
Civi/Core/Container.php

index 38c1da128e0db51fdf0e1678591e21dea5858ec7..5a5e612c057dfaf8ebf43be286ce47d80e732a8a 100644 (file)
@@ -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.
index 95009e35c3fd3df36ba03e43dca7b844d78b9818..f916756883ed7ae8c40756847a70a471576490df 100644 (file)
--- 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');
   }
 
   /**
index 9f3da3215e4fcacc0659bf4836632f05e5313e05..ac104537d81bfe8968015ea37004a6447634aeb1 100644 (file)
@@ -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).");
 
index 15c7860a9d64ce522f1b46b2bc029d2f5cfad5f3..c8a107014b7365e3316cfc8edb148decc5c6a833 100644 (file)
@@ -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);