search

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

Enable the page cache to be disabled without turning off the plugin #209

Closed stevegrunwell closed 3 years ago

stevegrunwell commented 3 years ago

In situations where site owners might need to troubleshoot some caching issues but don't want to completely disable Cache Enabler, it's helpful to have the ability to pause the cache.

This PR adds a new "Cache Status" section to the top of the plugin settings screen, with a single option: "Enable the site cache". When checked (default behavior), it's assumed that the site owner wants the site cache enabled, and the plugin will proceed with checks around a missing advanced-cache.php file, the WP_CACHE constant, etc. However, when the page cache is disabled, these warnings are suppressed and both the constant and drop-in are removed. Re-enabling the page cache will then re-run Cache_Enabler_Disk::setup(), which puts these items back into place.

coreykn commented 3 years ago

Thanks for all of the work you've been putting into this plugin, Steve. It's really been great to see. I've actually intentionally avoided adding this type of behavior but am open to discussing its implementation. For cache troubleshooting I have thought the plugin can either be deactivated, which I see wouldn't work if Cache Enabler is used as a must-use plugin, WP_CACHE can be set to false, or a query string can be appended to the page URL (depending on the query strings that are excluded). For example, by default something like https://www.example.com/path/to/page/?nocache or https://www.example.com/path/to/page/?bypass_cache_enabler=1 could be used.

stevegrunwell commented 3 years ago

I see a few major benefits of having a setting over relying solely on the WP_CACHE constant, most of which come down to better understanding the site owner's intent:

  1. Site owners aren't forced to modify their wp-config.php files directly, as that's a common cause of issues for less-technical site owners.
  2. If the site owner has signaled that they want to disable the page cache, then Cache Enabler will no longer warn them on every page about the missing constant and/or advanced-cache.php file. Alternately, if they do have it enabled but the site cache isn't working, the alerts help guide them to the desired state.
  3. Cache Enabler can explicitly flush the cache when disabled, which will prevent situations where sites using the Cache Enabler rewrite rules continue to serve existing cache files despite a false-y value for WP_CACHE (since Apache would capture the request before it ever makes it down to PHP).
  4. This setting being present also gives developers something to look for when determining, for example, whether or not the aforementioned rewrite rules should be added or removed from a site's Htaccess file dynamically.

As you alluded to above, we're dealing with a situation where Cache Enabler will be running as an MU plugin, so deactivation would no longer be an option in our case. This seems like a reasonable way to let site owners opt-out of the functionality (whether troubleshooting or longer-term), while preventing them from running out-of-date versions of the plugin.

coreykn commented 3 years ago

Those points are great to know. I appreciate the detail as it allows me to understand the cases I'm unaware of.

Let me think about this change a little bit more and get back to this next week.

coreykn commented 3 years ago

That was a long week. 😉 Over the last few version branches we've been setting up Cache Enabler to be more server oriented as the performance is far better. This has been in preparation for the request made by your colleague in #176 in addition to other features that we want to bring to Cache Enabler.

You've made really great points that I believe makes adding this type of behavior important. We plan on implementing this in some way in the future before or at the time of #176, but it will be done slightly differently than what you've proposed. I do like "Cache Status" as it fits well. It also allows other status information to potentially be included, like data from the new cache iterator.

In case it helps for your current use case I thought I'd let you know in Cache_Enabler::get_default_settings() we consider the system settings to be defined by Cache Enabler and not the user. The only place this may cause an issue for you is when Cache_Enabler::update_backend() is called. That would cause the enabled system setting you've added to overwrite whatever was potentially defined in the database. I'm unsure if that matters in your case but I thought I'd let you know.