From 3af5c3fe5f91800f582527da31b089c3bfbedb7d Mon Sep 17 00:00:00 2001 From: Tim Otten Date: Mon, 20 Apr 2020 19:53:35 -0700 Subject: [PATCH] dev/core#1713 - At end of upgrade, perform a system flush. Move menu-rebuild. Before ------ If/when an administrator opens the `civicrm/upgrade?reset=1` UI, then it rebuilds the menu (`CRM_Core_Menu::store()`). This runs in upgrade-mode, so the menu does not include any routes defined via hook. After ----- At the end of upgrade, after the general schema has reached canonical form, after most hooks are re-enabled for extensions, then it performs a full system flush. This incldues rebuilding the menu (and other caches). Comments -------- You might think that it serves some purpose to call `CRM_Core_Menu::store()` from `CRM_Upgrade_Page_Upgrade::runIntro()`. I can't figure one in the current regimen. Consider: * After loading a new source tree, do we need to rebuild the menu to access `civicrm/upgrade?reset=1`? Conceivably. But... if that's the case, then we wouldn't be able to visit `civicrm/upgrade?reset=1` at all. Moreover, the other upgrade routes (eg `civicrm/upgrade/queue*`) are hard-coded into the router specifically to resolve that chicken-egg issue. * Code in `CRM_Upgrade_Page_Upgrade::runIntro()` only runs if you use the web-based upgrader (not headless upgraders). So that line isn't a reliable part of upgrade logic. * When that old line runs, lots of hooks are disabled (incl `hook_xmlMenu` and `hook_alterMenu`), so it doesn't build a proper route table. * Consider subsystems like Afform and Data Processor - which allow admins to define new pages. These routes have to be stored somewhere and then loaded programmatically (eg `hook_alterMenu`). The logic which reads the list probably involves APIs, database tables, and/or settings -- i.e. things that are not be reliably functional with old schema. --- CRM/Upgrade/Form.php | 15 ++------------- CRM/Upgrade/Page/Upgrade.php | 1 - 2 files changed, 2 insertions(+), 14 deletions(-) diff --git a/CRM/Upgrade/Form.php b/CRM/Upgrade/Form.php index 3c65c5e31f..10c2b54253 100644 --- a/CRM/Upgrade/Form.php +++ b/CRM/Upgrade/Form.php @@ -779,17 +779,8 @@ SET version = '$version' // Seems extraneous in context, but we'll preserve old behavior $upgrade->setVersion($latestVer); - // TODO: Consider running a general system flush instead of piecemeal flushes. - // Historically, that would not have worked well because hooks are restricted - // in upgrade-mode. But the dispatch-policy for 'upgrade.finish' is probably - // more suitable. - - // Clear cached metadata. - Civi::service('settings_manager')->flush(); - - // cleanup caches CRM-8739 - $config = CRM_Core_Config::singleton(); - $config->cleanupCaches(1); + CRM_Core_Invoke::rebuildMenuAndCaches(FALSE, TRUE); + // NOTE: triggerRebuild is FALSE becaues it will run again in a moment (via fixSchemaDifferences). $versionCheck = new CRM_Utils_VersionCheck(); $versionCheck->flushCache(); @@ -797,8 +788,6 @@ SET version = '$version' // Rebuild all triggers and re-enable logging if needed $logging = new CRM_Logging_Schema(); $logging->fixSchemaDifferences(); - - CRM_Core_ManagedEntities::singleton(TRUE)->reconcile(TRUE); } /** diff --git a/CRM/Upgrade/Page/Upgrade.php b/CRM/Upgrade/Page/Upgrade.php index d1f9d26765..a8008f14b9 100644 --- a/CRM/Upgrade/Page/Upgrade.php +++ b/CRM/Upgrade/Page/Upgrade.php @@ -86,7 +86,6 @@ class CRM_Upgrade_Page_Upgrade extends CRM_Core_Page { // All cached content needs to be cleared because the civi codebase was just replaced CRM_Core_Resources::singleton()->flushStrings()->resetCacheCode(); - CRM_Core_Menu::store(); // cleanup only the templates_c directory $config->cleanup(1, FALSE); -- 2.25.1