Restrict doing plugin upgrade routine when not in admin

[westonruter]

Summary

The changes here in effect restrict the upgrade routine to happen at admin_init. This was discussed in depth at #4177. This implements #3284 (comment):

The DB upgrade routines don't happen on the frontend either, as you recall the upgrade DB message is only presented once trying to access the admin.

Fixes #3284

Checklist

• My pull request is addressing an open issue (please create one otherwise).
• My code is tested and passes existing tests.
• My code follows the Engineering Guidelines (updates are often made to the guidelines, check it out periodically).

[schlessera]

Before doing this, we'll need to audit what will happen if a new version of the plugin runs with an old DB version, as the upgrade routine will now not run unconditionally anymore and it might be hours, days or weeks between updating the plugin files and updating the DB.

This can lead to unexpected behavior and breakage if the new code doesn't expect the old DB schema.

[westonruter]

@schlessera There are only two instances of the amp_plugin_update hook being used at present:

amp-wp/includes/validation/class-amp-validated-url-post-type.php

Lines 147 to 173 in cfd87a2

	/**
	* Handle update to plugin.
	*
	* @param string $old_version Old version.
	*/
	public static function handle_plugin_update( $old_version ) {
	// Update the old post type slug from amp_validated_url to amp_validated_url.
	if ( '1.0-' === substr( $old_version, 0, 4 ) || version_compare( $old_version, '1.0', '<' ) ) {
	global $wpdb;
	$post_ids = get_posts(
	[
	'post_type' => 'amp_invalid_url',
	'fields' => 'ids',
	'posts_per_page' => -1,
	]
	);
	foreach ( $post_ids as $post_id ) {
	$wpdb->update(
	$wpdb->posts,
	[ 'post_type' => self::POST_TYPE_SLUG ],
	[ 'ID' => $post_id ]
	);
	clean_post_cache( $post_id );
	}
	}
	}

amp-wp/src/BackgroundTask/MonitorCssTransientCaching.php

Lines 158 to 168 in cfd87a2

	/**
	* Handle update to plugin.
	*
	* @param string $old_version Old version.
	*/
	public function handle_plugin_update( $old_version ) {
	// Reset the disabling of the CSS caching subsystem when updating from versions 1.5.0 or 1.5.1.
	if ( version_compare( $old_version, '1.5.0', '>=' ) && version_compare( $old_version, '1.5.2', '<' ) ) {
	AMP_Options_Manager::update_option( Option::DISABLE_CSS_TRANSIENT_CACHING, false );
	}
	}

I don't see how either of these will cause problems if they are deferred to when the admin user logs in. For the first, it's only for sites running the AMP plugin on versions before 1.0 stable. And it is just renaming a post type for the validated URLs. There's no impact on that being deferred.

For the second, it's just making sure the transient caching gets re-enabled when upgrading from 1.5.0+ to 1.5.2. This also can wait until the admin user logs in.

So in both cases I see no issue to move forward.

[schlessera]

Looks good then. We just need to keep this in mind when we use this hook. It changes from enforcing consistency after upgrades to hopefully end up with eventual consistency instead. So we cannot rely on anything that happens during an update event, it will either need to be optional or trigger user feedback in some way.

[pierlon]

Verified in QA. The upgrade routine is only run when an admin logs in that has the permission of managing options.