YahnisElsts / plugin-update-checker

A custom update checker for WordPress plugins. Useful if you don't want to host your project in the official WP repository, but would still like it to support automatic updates. Despite the name, it also works with themes.
MIT License
2.22k stars 403 forks source link

network deactivation keeps the cron in the child sites #517

Closed liedekef closed 1 year ago

liedekef commented 1 year ago

When network activating, the plugin creates a cron in each subsite (puc_cron_check_updates-events-made-easy), but upon network deactivation, the cron is not removed from all the child sites. I can take care of removing this cron, but should the code not do this directly? Based on a quick code-check, the plugin update checker is not taking multisite into account, correct?

YahnisElsts commented 1 year ago

Yes, that part of the code currently is not designed to be Multisite-aware.

Did you have something in mind for removing the cron jobs in Multisite? The first thing that I thought of was to switch to every individual site and remove the cron job there, but that seems like it could be incredibly slow in large networks.

liedekef commented 1 year ago

The way I currently do this:

register_deactivation_hook( __FILE__, 'my_deactivation_function' );
function my_deactivation_function($networkwide) {
       global $wpdb;
        if ( function_exists( 'is_multisite' ) && is_multisite() ) {
                // check if it is a network activation - if so, run the activation function for each blog id
                if ( $networkwide ) {
                        $old_blog = $wpdb->blogid;
                        // Get all blog ids
                        $blogids = $wpdb->get_col( 'SELECT blog_id FROM ' . $wpdb->blogs );
                        foreach ( $blogids as $blog_id ) {
                                switch_to_blog( $blog_id );
                                _my_normal_deactivation_code();
                        }
                        switch_to_blog( $old_blog );
                        return;
                }
        }
        // executed if no network activation
        _my_normal_deactivation_code();
}

You already call removeUpdaterCron upon deactivation, so it would be a matter of changing that function to be multisite-compliant.

YahnisElsts commented 1 year ago

Right, that's what I meant by switching to every site. It would work in most cases, but I'm concerned about what would happen on networks with hundreds of sites. That's hundreds of DB queries, and on a large enough network with low enough limits, you might even be at risk of hitting the PHP execution time limit.

liedekef commented 1 year ago

While I understand the problem (the php execution time etc ...), but there's no other workaround for multisite (at least not that I found). All pluginq activating in multisite do something like this and need to go through the same steps upon deactivation.

YahnisElsts commented 1 year ago

Might be nice if there was a way to set a cron job at the network level. The update checker doesn't actually need a separate cron for each site, checking updates once for the entire network would be enough. However, I'm not aware of any way to do that with the current API (except if the plugin is network activated - then it could schedule one event just on the main site).

Anyway, that's just commentary. I'll leave this issue open until I get around to implementing some kind of a fix.

liedekef commented 1 year ago

Each site in a multisite is independent of the others. While you could detect running on a certain instance, all crons are individual. Doing it on network level only would prevent normal admins to see an update is present, doing it in one site alone would prevent admins from other sites to see the update. So there's no good way around it :-) Playing with network-wide options might help a bit, but it would create an ugly solution. People with hundreds of sites in a multisite setup should already realise that activating a plugin network-wide might cause problems, but I think it is not up to a plugin developer to take this into account (it is a wordpress issue in fact, not a plugin issue). I even posted a suggestion to improve on cron (not use options, but a separate table with one entry per cron+interval so people don't need to start creating "schedules" themselves but can just define the interval in minutes), but the wordpress team is more busy with guttenberg than with regular wordpress improvements. Currently everyone would need to create their own schedule (like "5 minutes" + define what "5 minutes" means) and only then be able to run their cron on that schedule, while it is much more logical the linux cron-way (the command + the time interval itself)

liedekef commented 1 year ago

For the moment I'm going to remove the puc-cron when uninstalling my plugin, so for me it is covered like that :-)