vip-go-mu-plugins icon indicating copy to clipboard operation
vip-go-mu-plugins copied to clipboard

Trigger activation hooks for plugins loaded via `wpcom_vip_load_plugin()`

Open felixarntz opened this issue 7 years ago • 4 comments
trafficstars

Currently, an issue with the wpcom_vip_load_plugin() function is that plugins loaded by it will not have their activation routines triggered. This was recently flagged as a problem with Yoast SEO, and I started looking into the issue.

We could fix it specifically on the Yoast side, but I think it would be more solid to find a solution in the loader function itself, as this would solve the feature for any other plugin, and now require each plugin to implement extra logic to support VIP Go. I've recently implemented an MU plugin that loads regular plugins "as MU plugins" manually, so it's close to how the wpcom_vip_load_plugin() function works. I implemented support for activation and deactivation. The latter is a bit more tricky though, so I think it would be fine to go with only activation for now, as this is the more critical part anyway.

Suggested Approach

  • Hook a new function on the shutdown action that persists the contents of the $vip_loaded_plugins global in an option.
  • After calling _wpcom_vip_include_plugin() from wpcom_vip_load_plugin(), call a new function wpcom_vip_maybe_activate_plugin( "{$plugin_type}/{$plugin}/{$file}" ).
  • In that new function, retrieve the value of the aforementioned option. If the plugin that has just been loaded is not part of that option (i.e. was not already VIP-loaded in the previous request), its activation hook needs to be triggered. Triggering the activation hook right after including the plugin main file is in line with how WordPress core does it, so it shouldn't cause any issues.
  • When triggering the activation hook, false should be passed in order to simulate that the plugin is being activated only for the current site, not network-wide. Since we'd store the VIP-loaded plugins in a sitewide option, this would play well together, and it would also account for use-cases where wpcom_vip_load_plugin() is called conditionally, for example only for certain sites. And even when it is called universally (which would be similar to a network-wide activation), handling everything on a per-site basis ensures that the plugin gets activated wherever it needs to be.

Here is what the new function wpcom_vip_maybe_activate_plugin() could look like:

function wpcom_vip_maybe_activate_plugin( $plugin ) {
    // Get the plugins that were previously loaded through VIP.
    $vip_loaded_plugins_prev = get_option( 'vip_go_loaded_plugins', array() );

    // Bail if the plugin was already loaded before.
    if ( in_array( $plugin, $vip_loaded_plugins_prev, true ) ) {
        return;
    }

    // Get the plugin basename (plugin directory plus main file).
    $plugin_basename = explode( '/', $plugin );
    array_shift( $plugin_basename );
    $plugin_basename = implode( '/', $plugin );

    // Pass false in order to simulate a site-wide activation.
    do_action( "activate_{$plugin_basename}", false );
}

Let me know your thoughts about this. I'm interested in discussing further as necessary to come up with a solid solution, and help with a PR too.

felixarntz avatar Jul 18 '18 09:07 felixarntz

Thanks for the ideas here @felixarntz; definitely some good options to work with here.

There are a couple things we need to be careful of:

  • Some of the sites we work with are at a scale where front-end writes (e.g. (add|update)_option) can be dangerous. Similarly, at scale, the suggested code which could result in many requests all triggering the upgrade routine (which could be fixed with a simple lock).
  • How well would this work if multiple plugins were added at once?
  • The options entry could get rather large as some sites have a large number of plugins.

Some possible alternatives (each has its own set of tradeoffs):

  • Show an admin notice and button when we find a plugin that didn't fire an activation routine to allow it to be run manually (predictable; but not super elegant and easy to miss/ignore).
  • Do this automatically, via a recurring cron job (would need to run at high frequency to catch plugins that were added).
  • Have our deploy system detect changes and queue a wp-cli command / one-time cron job / etc. to trigger the activation (very high complexity; can't replicate behaviour locally as easily).

For the single option, it might be better just to move to one option per-file, especially if we go with one of the alternatives (as we would no longer need to autoload it).

mjangda avatar Jul 31 '18 04:07 mjangda

Thanks for the feedback @mjangda!

A few follow-up comments / questions from my end:

Some of the sites we work with are at a scale where front-end writes (e.g. (add|update)_option) can be dangerous.

That's a good point. We could restrict that logic to only run in the admin, if we went with it.

How well would this work if multiple plugins were added at once?

I don't see any issues with that. It's the same as bulk activating plugins via the admin.

The options entry could get rather large as some sites have a large number of plugins.

Is this an actual problem? About how many plugins are we talking? Given that there are tons of plugins that use massive options, I'm curious how a large list of plugins would be a problem. FWIW, it would be similar to the active_plugins option from core in a way.

Out of the three alternative ideas you're listing, I like the cron job and WP-CLI ones the best. To be fair, the approach I outlined makes most sense for maximum compatibility and ease of use on any setup. For custom setups where we know the constraints, one of your ideas might be better suited. Some considerations:

  • In order to support many plugins being added at the same time, we need to ensure timeout and memory constraints are not exceeded. A WP-CLI command may be a good solution for that.
  • The approaches of showing an admin notice or running a cron job have the drawback that they are not reliable. My approach (and also your deploy system-related WP-CLI command if I understand correctly) would be reliable, as in: When a plugin is added, we can ensure its activation routine runs immediately and its functionality thus is available properly.

felixarntz avatar Jul 31 '18 09:07 felixarntz

This issue has been marked stale because it has been open for 60 days with no activity. If there is no activity within 7 days, it will be closed.

This is an automation to keep issues manageable and actionable and is not a comment on the quality of this issue nor on the work done so far. Closed issues are still valuable to the project and are available to be searched.

github-actions[bot] avatar Sep 27 '22 00:09 github-actions[bot]

I'd still like to look into this and finish the PoC I started that'll allow us to trigger the activation hook one time for plugins activated via code. Is still a painpoint for some plugins that register roles and whatnot upon install.

WPprodigy avatar Oct 05 '22 05:10 WPprodigy

This issue has been marked stale because it has been open for 60 days with no activity. If there is no activity within 7 days, it will be closed.

This is an automation to keep issues manageable and actionable and is not a comment on the quality of this issue nor on the work done so far. Closed issues are still valuable to the project and are available to be searched.

github-actions[bot] avatar Dec 05 '22 00:12 github-actions[bot]