From 62f662b002e4f8b6cfbdbd2b9d0c03712f2441fa Mon Sep 17 00:00:00 2001 From: eileen Date: Thu, 7 Feb 2019 10:48:02 +1300 Subject: [PATCH] Load hooks during upgrade mode 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 | 6 +++--- CRM/Utils/Hook.php | 11 +++++++++++ 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/CRM/Extension/Mapper.php b/CRM/Extension/Mapper.php index 358d3a9103..b9cc7ed182 100644 --- a/CRM/Extension/Mapper.php +++ b/CRM/Extension/Mapper.php @@ -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; diff --git a/CRM/Utils/Hook.php b/CRM/Utils/Hook.php index 8282a89c7c..a47f3076d7 100644 --- a/CRM/Utils/Hook.php +++ b/CRM/Utils/Hook.php @@ -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, -- 2.25.1