keycdn / cache-enabler

A lightweight caching plugin for WordPress that makes your website faster by generating static HTML files.
https://wordpress.org/plugins/cache-enabler/
123 stars 46 forks source link

$network_wide param for each_site() method is inaccurate #162

Closed nosilver4u closed 3 years ago

nosilver4u commented 3 years ago

I've noticed in several places that each_site() is called with the $network_wide parameter set to the value of is_multisite(). Unfortunately, the result of is_multsite() is not the same as whether or not the plugin is activated network-wide, which will cause the callback function to run on all sites when it should only be running on the active sites (or perhaps the current site).

It might be worth making a "wrapper" method like so:

function is_network_active() {
    if ( ! function_exists( 'is_plugin_active_for_network' ) && is_multisite() ) {
        require_once( ABSPATH . 'wp-admin/includes/plugin.php' );
    }
    if ( is_multisite() && is_plugin_active_for_network( CE_BASE ) ) {
        return true;
    }
    return false;
}
coreykn commented 3 years ago

Thanks for your feedback, @nosilver4u. That's actually intentional based on what's being performed as the Cache_Enabler::each_site() $network parameter focuses on whether or not to enter each site in the network (maybe some renaming of variables for more clarity is required). Methods that are using just is_multisite() and the reasoning behind it:

Hopefully that helps to show why this was done, but of course if there is a better way to handle this or if the handling above is not quite right we can make changes at any time.

nosilver4u commented 3 years ago

Ah, that does make sense! I can definitely see where you might end up with orphaned settings files and what not that you want to make sure to cleanup--even more so for the on_uninstall() method. And since the plugin checks is_plugin_active() within the update_backend() method, you won't have anything running on sites where it shouldn't.