Load hooks during upgrade mode
authoreileen <emcnaughton@wikimedia.org>
Wed, 6 Feb 2019 21:48:02 +0000 (10:48 +1300)
committereileen <emcnaughton@wikimedia.org>
Tue, 9 Apr 2019 09:18:59 +0000 (21:18 +1200)
For unknown, svn, reasons extension hooks are not loaded during upgrade
(this doesn't apply to drupal modules) - this causes some fairly serious problems
1) settings are re-loaded & cached with settings from extensions being lost
2) trigger alter hooks are lost this means
 - the summary fields triggers are frequently lost on upgrade
 - hooks that unset various tables to prevent them from being logged can fail, resulting in those log tables being created
 - hooks that specify the table should be innodb can fail to run, resulting in archive format.

I can't think WHY we do this? Presumably there was some problem that would have been better solved another
way but which was solved this way?

Fix "Load hooks during upgrade mode" (45312e1e64dd6af0281fe5fb7f96dbd8be39e524)

In my testing, the commit doesn't do what it says because the symbols are wrong.

CRM/Extension/Mapper.php
CRM/Utils/Hook.php

index 358d3a9103233a37b5d8494f561de53f0298efb7..b9cc7ed18210c0062dcf7f066770d2f83db54c3a 100644 (file)
@@ -275,9 +275,9 @@ class CRM_Extension_Mapper {
    *   array(array('prefix' => $, 'file' => $))
    */
   public function getActiveModuleFiles($fresh = FALSE) {
-    $config = CRM_Core_Config::singleton();
-    if ($config->isUpgradeMode() || !defined('CIVICRM_DSN')) {
-      return []; // hmm, ok
+    if (!defined('CIVICRM_DSN')) {
+      // hmm, ok
+      return [];
     }
 
     $moduleExtensions = NULL;
index 8282a89c7ca20d5e04987b56402c8a18d0dc4995..a47f3076d7f7fb8b10009033570498125f853c82 100644 (file)
@@ -161,6 +161,17 @@ abstract class CRM_Utils_Hook {
     &$arg1, &$arg2, &$arg3, &$arg4, &$arg5, &$arg6,
     $fnSuffix
   ) {
+    // Per https://github.com/civicrm/civicrm-core/pull/13551 we have had ongoing significant problems where hooks from modules are
+    // invoked during upgrade but not those from extensions. The issues are both that an incorrect module list & settings are cached and that
+    // some hooks REALLY need to run during upgrade - the loss of triggers during upgrade causes significant loss of data
+    // whereas loss of logTable hooks means that log tables are created for tables specifically excluded - e.g due to large
+    // quantities of low-importance data or the table not having an id field, which could cause a fatal error.
+    // Instead of not calling any hooks we only call those we know to be frequently important - if a particular extension wanted
+    // to avoid this they could do an early return on CRM_Core_Config::singleton()->isUpgradeMode
+    $upgradeFriendlyHooks = ['civicrm_alterSettingsFolders', 'civicrm_alterSettingsMetaData', 'civicrm_triggerInfo', 'civicrm_alterLogTables', 'civicrm_container'];
+    if (CRM_Core_Config::singleton()->isUpgradeMode() && !in_array($fnSuffix, $upgradeFriendlyHooks)) {
+      return;
+    }
     if (is_array($names) && !defined('CIVICRM_FORCE_LEGACY_HOOK') && \Civi\Core\Container::isContainerBooted()) {
       $event = \Civi\Core\Event\GenericHookEvent::createOrdered(
         $names,