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

Automatically clear expired caches via WP-Cron #223

Closed stevegrunwell closed 3 years ago

stevegrunwell commented 3 years ago

Currently, sites that use the Apache rewrites to bypass PHP will not clear out expired cached files, as noted here: https://www.keycdn.com/support/wordpress-cache-enabler-plugin#will-cached-page-expiration-still-work-if-the-advanced-configuration-is-used

This PR registers an hourly cron job that iterates through the disk-based cache and removes cache files that have passed their expiration time.

coreykn commented 3 years ago

Having this type of behavior available would be a great addition. However, this way won't work properly because the site objects retrieved with Cache_Enabler_Disk::get_site_objects() can be both files and directories (I should make that more specific in the docs). That would cause an error when trying to use unlink() on a directory. If a check was done beforehand, like with is_file() or if the warning was suppressed, it could leave an empty directory. I'll have more time here shortly to take a deeper look at this and share my approach, but feel free to make any changes on how you'd do it.

stevegrunwell commented 3 years ago

Ah, great to know. I've updated the PR with an is_file() check, but also happy to see what further suggestions you might have!

coreykn commented 3 years ago

I apologize for the delay while I had other tasks to complete. When thinking about this enhancement in more detail I think it would be best to add this behavior to the updated cache clearing handling that I want to introduce now that the cache file handling was updated in 1.7.0. Something I missed in my previous reply is that this PR is actually only checking the home page as getting the site objects isn't recursive because it was not needed at the time. (The new way I'm considering would allow directory objects to be retrieved recursively, allow the directory path to be prepended so this doesn't have to be handled later, and should perform better than scandir().)

The idea I have is to allow arguments to be passed to Cache_Enabler_Disk::clear_cache(), either by query string or a parameter, and allow this method to handle all of the cache clearing instead of sending it to Cache_Enabler_Disk::clear_dir(). (If both were to be provided the query string parameters would take precedence.) This removes the need for two properties (Cache_Enabler::$fire_page_cache_cleared_hook and Cache_Enabler_Disk::$dir_cleared) and allows adding new clear cache arguments in the future without the need for another parameter. For example, something like https://www.example.com?subpages=1&expired=1. This would bring control to what is actually being cleared, like if a page is expired or only clearing a specific cached version, such as only cached pages that are https and webp through something like https://www.example.com/about/?versions[]=https,webp or https://www.example.com/about/?keys[]=https,webp. This would also bring complete cache clearing control to any current or future method and hooks, as well as to the command line. (If query string caching was ever introduced we would likely need to begin creating an actual cache key.)

That would then allow the public cache clearing methods to be created in Cache_Enabler with whatever needs to occur, such as clearing the expired cache. That way by default the cache cleared hooks are still fired in the event Cache Enabler is clearing any cached pages, even if only expired, while still passing enough information in the cache cleared action hook to know what took place. (The clear cache arguments would allow the firing of the cache cleared hooks to be controlled.)

Hopefully that makes sense. 🙂 If it's not all the way clear yet I will share what I create and that should help. Any feedback or suggestions you have are always welcome.

coreykn commented 3 years ago

Closing in favor of #237.