dev/core#2547 - Add base-upgrader to core

[totten]

Overview

Many extensions include an "upgrader" class, a "base upgrader" class, and some wiring to support them. This patch migrates a lot of that boilerplate to core and does some cleanup in the process.

As discussed in https://lab.civicrm.org/dev/core/-/issues/2547, this addresses a theme shared by a few other issues -- the old way of wiring-up lifecycle-hooks is problematic, and making a direct change is also problematic. We can resolve those with new wiring for the upgrader.

This change means that (going forward):

• It will be easier to control the order in which upgrade-tasks run.
• There will be less duplicate / boilerplate code.
• It will be easier to change the boilerplate/baseline code.

Before

Most extensions which define schema rely on civix's upgrader template. It defines these four elements:

• myext.php: This has stubs for hook_install, hook_uninstall, hook_upgrade, etc -- each delegates to myext.civix.php.
• myext.civix.php: This has stubs for hook_install, hook_uninstall, hook_upgrade, etc -- each delegates to CRM/Myext/Upgrader.php.
• CRM/Myext/Upgrader.php: This is the customizable list of install/uninstall/upgrade steps.
• CRM/Myext/Upgrader/Base.php: This is a large boilerplate file. It has a library of helpers+adapters.

After

Existing extensions may still work the same way.

Going forward, core extensions (and probably future civix-based extensions) should only define two elements:

• info.xml: Declare <upgrader>CRM_Myext_Upgrader</upgrader>.
• CRM/Myext/Upgrader.php: As before, this is the customizable list of install/uninstall/upgrade steps. It implements an interface (CRM_Extension_Upgrader_Interface). For convenience, it can extend the base implementation (CRM_Extension_Upgrader_Base) which matches the old boilerplate.

For example:

<!-- FILE:- info.xml -->
<extension key='test.extension.manager.moduleupgtest' type='module'>
   ...
  <classloader>
    <psr0 prefix="CRM_" path="" />
  </classloader>
  <upgrader>CRM_Moduleupgtest_Upgrader</upgrader>
</extension>

And:

// FILE: CRM/Moduleupgtest/Upgrader.php
class CRM_Moduleupgtest_Upgrader extends CRM_Extension_Upgrader_Base {
  public function install() { ... }
  public function upgrade_0001() {...}
  public function upgrade_0002() {...}
}

(There is an example of this in tests/extensions/test.extension.manager.moduleupgtest.)

Comments

#19865 also aims to reduce civix boilerplate. However, it addresses a different aspect. #19865 provides a way to dedupe boilerplate for normal hooks (like hook_xmlMenu). This PR deals with the abnormal hooks (like hook_install).

I think a primary question for this PR is: what kind of QA tasks are appropriate before merging?

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[totten]

This piece was fairly easy to extract: #20091

[seamuslee001]

@totten can you rebase now?

[totten]

Yep, thanks @seamuslee001. Rebased.

[totten]

... a primary question for this PR is: what kind of QA tasks are appropriate before merging?

• r-maint: For the existing style of lifecycle hooks, there's a unit-test CRM_Extension_Manager_ModuleTest which mocks-up an extension and then checks the hooks that run in various edge-cases. Since the PR basically moves these hooks to a dedicated class (the "upgrader"), I made a variant of the test -- it mocks up a fake extension but uses an upgrader-class instead.
• r-run: While working on the patch, I used a core extension as a test case. Process:
  • Install a prior version (IIRC, ~5.34?), enable a core ext (oauth-client or search), and make a DB snapshot. Note the revisions (select file, schema_version from civicrm_extension).
  • Switch to master. Apply the new patches. Run ./bin/setup.sh -Dg.
  • Upgrade core-DB (eg cv upgrade:db). Upgrade ext-DB (eg cv ext:upgrade-db)
  • Observe: The extension schema is updated as expected.

@seamuslee001 @eileenmcnaughton @colemanw - In terms of scoping this PR, I included the patches for ext/search and ext/oauth-client because they were part of the testing. This does increase the #commits, but I figure it makes it easier to r-run.

(An alternative way to r-run is to hand-code a test-extension ... but using a coupel core-exts as a demonstration seems easier for all.)

[colemanw]

I did an r-run and confirmed that the search extension's db upgrades execute with this patch.
Seems like an additional cleanup step would be to remove the CRM_Search_Upgrader_Base class entirely, but that's not a blocker to merge this.