From c09c9d3dc0f17cfba6242eed7e9f10660f9c4419 Mon Sep 17 00:00:00 2001 From: Tim Otten Date: Wed, 2 Mar 2022 21:34:31 -0800 Subject: [PATCH 1/6] CRM/Upgrade - Define queue weights. Put finalization step in the queue. --- CRM/Upgrade/Form.php | 50 +++++++++++++++++++++++++++++++++++++------- 1 file changed, 43 insertions(+), 7 deletions(-) diff --git a/CRM/Upgrade/Form.php b/CRM/Upgrade/Form.php index d69e9bd20b9d..bd8e609cc2f3 100644 --- a/CRM/Upgrade/Form.php +++ b/CRM/Upgrade/Form.php @@ -499,6 +499,14 @@ public function checkCurrentVersion($currentVer, $latestVer) { /** * Fill the queue with upgrade tasks. * + * The queue is a priority-queue (sorted by tuple weight+id). Here are some common weights: + * + * - `weight=0`: Add a typical upgrade step for revising core schema. + * - `weight=-1`: In the middle of the upgrade, add an extra step for immediate execution. + * - `weight=1000`: Add some general core upgrade-logic that runs after all schema have change.d + * - `weight=2000`: Add some post-upgrade logic. If a task absolutely requires full system services + * (eg enabling a new extension), then place it here. + * * @param string $currentVer * the original revision. * @param string $latestVer @@ -526,14 +534,14 @@ public static function buildQueue($currentVer, $latestVer, $postUpgradeMessageFi [$postUpgradeMessageFile], "Cleanup old files" ); - $queue->createItem($task); + $queue->createItem($task, ['weight' => 0]); $task = new CRM_Queue_Task( ['CRM_Upgrade_Form', 'disableOldExtensions'], [$postUpgradeMessageFile], "Checking extensions" ); - $queue->createItem($task); + $queue->createItem($task, ['weight' => 0]); $revisions = $upgrade->getRevisionSequence(); $maxRevision = empty($revisions) ? NULL : end($revisions); @@ -552,7 +560,7 @@ public static function buildQueue($currentVer, $latestVer, $postUpgradeMessageFi [$rev], "Begin Upgrade to $rev" ); - $queue->createItem($beginTask); + $queue->createItem($beginTask, ['weight' => 0]); $task = new CRM_Queue_Task( // callback @@ -561,7 +569,7 @@ public static function buildQueue($currentVer, $latestVer, $postUpgradeMessageFi [$rev, $currentVer, $latestVer, $postUpgradeMessageFile], "Upgrade DB to $rev" ); - $queue->createItem($task); + $queue->createItem($task, ['weight' => 0]); $task = new CRM_Queue_Task( // callback @@ -570,7 +578,7 @@ public static function buildQueue($currentVer, $latestVer, $postUpgradeMessageFi [$rev, $currentVer, $latestVer, $postUpgradeMessageFile], "Finish Upgrade DB to $rev" ); - $queue->createItem($task); + $queue->createItem($task, ['weight' => 0]); } } @@ -581,9 +589,16 @@ public static function buildQueue($currentVer, $latestVer, $postUpgradeMessageFi [$rev, $latestVer, $latestVer, $postUpgradeMessageFile], "Finish Upgrade DB to $latestVer" ); - $queue->createItem($task); + $queue->createItem($task, ['weight' => 0]); } + $task = new CRM_Queue_Task( + ['CRM_Upgrade_Form', 'doCoreFinish'], + [$rev, $latestVer, $latestVer, $postUpgradeMessageFile], + "Finish core DB updates $latestVer" + ); + $queue->createItem($task, ['weight' => 1000]); + return $queue; } @@ -780,7 +795,13 @@ public static function doIncrementalUpgradeFinish(CRM_Queue_TaskContext $ctx, $r return TRUE; } - public static function doFinish() { + /** + * Finalize the core upgrade. + * + * @return bool + * @throws \CRM_Core_Exception + */ + public static function doCoreFinish(): bool { Civi::dispatcher()->setDispatchPolicy(\CRM_Upgrade_DispatchPolicy::get('upgrade.finish')); $restore = \CRM_Utils_AutoClean::with(function() { Civi::dispatcher()->setDispatchPolicy(\CRM_Upgrade_DispatchPolicy::get('upgrade.main')); @@ -805,6 +826,21 @@ public static function doFinish() { $logging->fixSchemaDifferences(); // Force a rebuild of CiviCRM asset cache in case things have changed. \Civi::service('asset_builder')->clear(FALSE); + + return TRUE; + } + + /** + * After finishing the queue, the upgrade-runner calls `doFinish()`. + * + * This is called by all upgrade-runners (inside or outside of `civicrm-core.git`). + * Removing it would be a breaky-annoying process; it would foreclose future use; + * and it would produce no tangible benefits. + * + * @return bool + */ + public static function doFinish(): bool { + return TRUE; } /** From 6b5c19687a334cd32f3369be4971450510bd1619 Mon Sep 17 00:00:00 2001 From: Tim Otten Date: Wed, 2 Mar 2022 21:48:25 -0800 Subject: [PATCH 2/6] Upgrader - Add enable extension as a reusable upgrade task --- CRM/Upgrade/Incremental/Base.php | 42 ++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/CRM/Upgrade/Incremental/Base.php b/CRM/Upgrade/Incremental/Base.php index 49568dc51a0b..be5e72d90ffc 100644 --- a/CRM/Upgrade/Incremental/Base.php +++ b/CRM/Upgrade/Incremental/Base.php @@ -158,6 +158,48 @@ protected function addTask($title, $funcName) { $queue->createItem($task, ['weight' => -1]); } + /** + * Add a task to activate an extension. This task will run post-upgrade (after all + * changes to core DB are settled). + * + * @param string $title + * @param string[] $keys + * List of extensions to enable. + */ + protected function addExtensionTask(string $title, array $keys): void { + Civi::queue(CRM_Upgrade_Form::QUEUE_NAME)->createItem( + new CRM_Queue_Task([static::CLASS, 'enableExtension'], [$keys], $title), + ['weight' => 2000] + ); + } + + /** + * @param \CRM_Queue_TaskContext $ctx + * @param string[] $keys + * List of extensions to enable. + * @return bool + */ + public static function enableExtension(CRM_Queue_TaskContext $ctx, array $keys): bool { + // The `enableExtension` has a very high value of `weight`, so this runs after all + // core DB schema updates have been resolved. We can use high-level services. + + Civi::dispatcher()->setDispatchPolicy(\CRM_Upgrade_DispatchPolicy::get('upgrade.finish')); + $restore = \CRM_Utils_AutoClean::with(function() { + Civi::dispatcher()->setDispatchPolicy(\CRM_Upgrade_DispatchPolicy::get('upgrade.main')); + }); + + $manager = CRM_Extension_System::singleton()->getManager(); + $manager->enable($manager->findInstallRequirements($keys)); + + // Hrm, `enable()` normally does these things... but not during upgrade... + // Note: A good test-scenario is to install 5.45; enable logging and CiviGrant; disable searchkit+afform; then upgrade to 5.47. + $schema = new CRM_Logging_Schema(); + $schema->fixSchemaDifferences(); + CRM_Core_Invoke::rebuildMenuAndCaches(FALSE, TRUE); + + return TRUE; + } + /** * Remove a payment processor if not in use * From d4a9b736979b9a0b2ed7626b7ba60cf1922702dc Mon Sep 17 00:00:00 2001 From: Coleman Watts Date: Thu, 3 Mar 2022 09:35:43 -0500 Subject: [PATCH 3/6] CiviGrant - Ensure dependencies are installed Fixes dev/core#3093 --- CRM/Upgrade/Incremental/php/FiveFortySeven.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CRM/Upgrade/Incremental/php/FiveFortySeven.php b/CRM/Upgrade/Incremental/php/FiveFortySeven.php index 08135fdf24ab..6ce55145eccc 100644 --- a/CRM/Upgrade/Incremental/php/FiveFortySeven.php +++ b/CRM/Upgrade/Incremental/php/FiveFortySeven.php @@ -58,6 +58,9 @@ public function setPreUpgradeMessage(&$preUpgradeMessage, $rev, $currentVer = NU public function upgrade_5_47_alpha1($rev): void { $this->addTask(ts('Upgrade DB to %1: SQL', [1 => $rev]), 'runSql', $rev); $this->addTask('Migrate CiviGrant component to an extension', 'migrateCiviGrant'); + if (CRM_Core_Component::isEnabled('CiviGrant')) { + $this->addExtensionTask('Enable CiviGrant dependencies', ['org.civicrm.search_kit', 'org.civicrm.afform']); + } $this->addTask('Add created_date to civicrm_relationship', 'addColumn', 'civicrm_relationship', 'created_date', "timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP COMMENT 'Relationship created date'" ); From 221b40dfbae49adb552bc1f243921543ad5ad27f Mon Sep 17 00:00:00 2001 From: Tim Otten Date: Thu, 3 Mar 2022 16:29:34 -0800 Subject: [PATCH 4/6] ManagedEntities - Ignore `$ignoreUpgradeMode` This guard was added by 912511a359680eb72b15c595b5091fa03f8687f2 as part of a previous approach to managing hooks during upgrades. This general approach changed with https://github.com/civicrm/civicrm-core/pull/17126; so 17126 partially undid this... but it inadvertently had the effect of completely disabling `reconcile()` (because this guard was left). --- CRM/Core/ManagedEntities.php | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/CRM/Core/ManagedEntities.php b/CRM/Core/ManagedEntities.php index eeb78f947d60..d94d89e22f7f 100644 --- a/CRM/Core/ManagedEntities.php +++ b/CRM/Core/ManagedEntities.php @@ -116,14 +116,10 @@ public function get($moduleName, $name) { * existing entities, and remove orphaned (stale) entities. * * @param bool $ignoreUpgradeMode - * + * Unused. * @throws \CRM_Core_Exception */ public function reconcile($ignoreUpgradeMode = FALSE) { - // Do not reconcile whilst we are in upgrade mode - if (CRM_Core_Config::singleton()->isUpgradeMode() && !$ignoreUpgradeMode) { - return; - } $this->loadDeclarations(); if ($error = $this->validate($this->getDeclarations())) { throw new CRM_Core_Exception($error); From 458e2d8e2c1689a2bd73ad3d4ef6d21bdff189a4 Mon Sep 17 00:00:00 2001 From: Tim Otten Date: Thu, 3 Mar 2022 16:37:38 -0800 Subject: [PATCH 5/6] FiveFortySeven - Leave `civigrant` inactive during inconsistent period Before ------ `migrateCiviGrant()` migrates some metadata from core-ownership to core-extension-ownership... and it ALSO activates the extension. However, the extension depends on other (possibly-inactive) extensions. This creates an inconsistent state (where active parts of `civigrant` depend on inactive parts of `search_kit`). In this inconsistent state, `ManagedEntities::reconcile()` fails. After ----- `migrateCiviGrant()` still migrates metadata. However, it initially leaves the extension inactive. So `ManagedEntities::reconcile()` won't try to setup these records. After core schema is fully resolved, then it installs all necessary extensions using normal mechanisms (with normal ordering). --- CRM/Upgrade/Incremental/php/FiveFortySeven.php | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/CRM/Upgrade/Incremental/php/FiveFortySeven.php b/CRM/Upgrade/Incremental/php/FiveFortySeven.php index 6ce55145eccc..462650716889 100644 --- a/CRM/Upgrade/Incremental/php/FiveFortySeven.php +++ b/CRM/Upgrade/Incremental/php/FiveFortySeven.php @@ -59,7 +59,7 @@ public function upgrade_5_47_alpha1($rev): void { $this->addTask(ts('Upgrade DB to %1: SQL', [1 => $rev]), 'runSql', $rev); $this->addTask('Migrate CiviGrant component to an extension', 'migrateCiviGrant'); if (CRM_Core_Component::isEnabled('CiviGrant')) { - $this->addExtensionTask('Enable CiviGrant dependencies', ['org.civicrm.search_kit', 'org.civicrm.afform']); + $this->addExtensionTask('Enable CiviGrant extension', ['civigrant']); } $this->addTask('Add created_date to civicrm_relationship', 'addColumn', 'civicrm_relationship', 'created_date', "timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP COMMENT 'Relationship created date'" @@ -110,6 +110,9 @@ public static function migrateCiviGrant(CRM_Queue_TaskContext $ctx): bool { } CRM_Core_DAO::executeQuery("DELETE FROM civicrm_component WHERE name = 'CiviGrant'", [], TRUE, NULL, FALSE, FALSE); } + + // There are existing records which should be managed by `civigrant`. To assign ownership, we need + // placeholders in `civicrm_extension` and `civicrm_managed`. $ext = new CRM_Core_DAO_Extension(); $ext->full_name = 'civigrant'; if (!$ext->find(TRUE)) { @@ -117,8 +120,9 @@ public static function migrateCiviGrant(CRM_Queue_TaskContext $ctx): bool { $ext->name = 'CiviGrant'; $ext->label = ts('CiviGrant'); $ext->file = 'civigrant'; - $ext->is_active = (int) $civiGrantEnabled; + $ext->is_active = 0; /* Not active _yet_. If site uses CiviGrant, we will re-activate once the core-schema has been revised. */ $ext->save(); + CRM_Extension_System::singleton()->getManager()->refresh(); $managedItems = [ 'OptionGroup_advanced_search_options_OptionValue_CiviGrant' => [ From 448a357efa075cf4e623fd1c7c7bc66a31b6f428 Mon Sep 17 00:00:00 2001 From: Tim Otten Date: Thu, 3 Mar 2022 20:37:43 -0800 Subject: [PATCH 6/6] FiveFortySeven - Ensure that `civicrm_search_display` is up-to-date Note: This step was added to the codebase circa 5.47. The underlying schema change actually originated circa 5.46. The step should be idempotent. --- CRM/Upgrade/Incremental/php/FiveFortySeven.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CRM/Upgrade/Incremental/php/FiveFortySeven.php b/CRM/Upgrade/Incremental/php/FiveFortySeven.php index 462650716889..4cc9616442e9 100644 --- a/CRM/Upgrade/Incremental/php/FiveFortySeven.php +++ b/CRM/Upgrade/Incremental/php/FiveFortySeven.php @@ -75,6 +75,12 @@ public function upgrade_5_47_alpha1($rev): void { $this->addTask('core-issue#2122 - Set the timezone to the default for existing Events', 'setEventTZDefault'); $this->addTask('Drop CustomGroup UI_name_extends index', 'dropIndex', 'civicrm_custom_group', 'UI_name_extends'); $this->addTask('Add CustomGroup UI_name index', 'addIndex', 'civicrm_custom_group', ['name'], 'UI'); + if (CRM_Core_DAO::checkTableExists('civicrm_search_display')) { + $this->addTask('Add SearchDisplay.acl_bypass', 'addColumn', + 'civicrm_search_display', 'acl_bypass', + "tinyint DEFAULT 0 COMMENT 'Skip permission checks and ACLs when running this display.'" + ); + } } /**