Auto-purging bug when Editor Role publishes a post

[raamdev]

Bug Summary

When users with the Editor role publish new posts, the cache files for all associated archive views (e.g., categories and tags) may not be purged as expected. This only occurs when Publishing a post (Updating works fine) and when both a Category and a Tag are associated with the new post (if only a Category is associated, everything works as expected).

Steps to reproduce

1. Enable Quick Cache Pro and create a user with the Editor role
2. Visit the a category archive view (e.g., /category/Uncategorized/) and a tag archive view (e.g., /tag/example/) to generate a cache for those two pages
3. Login as the Editor user you created in the first step
4. Create a new post (Dashboard → Posts → Add New), assign it to the category and tag whose archive view you cached (e.g., category Uncategorized and Tag example); Note: you MUST assign both a Category and a Tag to reproduce this; see notes below.
5. Publish the post; Note: you MUST Publish the post to reproduce this; Updating existing posts works fine; see notes below.
6. As a logged-out user (or from another browser that's not logged in), visit the archive views you cached earlier (e.g., /category/Uncategorized/) and a tag archive view (e.g., /tag/example/). You'll see that both pages are loading the old cache file and have not been purged as they should have been when publishing a new post with that category and tag.

Notes

• The Home Page cache is purged as expected
• When "Updating" an existing post as an Editor, all relevant cache files do get purged as expected (including associated category and tag archive views)
• If you assign only a Category but no Tags, then the category archive view cache is cleared as expected. It's only when you assign both a category and a tag that neither archive view is purged.

[jaswrks]

I can reproduce this also, thanks for the steps!

[jaswrks]

A guess (and that's all it is at this point) is that auto_purge_post_terms_cache() is getting called upon before the terms are added to the post. The second time it runs, once the terms are added, the $static property is already set, and so it doesn't run again when it needs to.

[jaswrks]

I ran a couple of tests. It seems under this scenario that the first hook to auto_purge_post_terms_cache() is coming through with a post status of draft, even though the post is being published. Since the first call is with a draft status, any later calls are void due to the $static property having already been set; i.e. QC ignores future hooks because it already handled this post ID.

Not sure why this is happening yet, or what we can do to fix it. Just sharing my findings.

[jaswrks]

I see the default status is draft for a new post. In some cases (in the WP core), the post terms are added before the final call to wp_update_post(); where the status is then updated to publish.

Since we have auto_purge_post_terms_cache() connected to the added_term_relationship hook, it is possible for auto_purge_post_terms_cache() to be called before the post status is set to publish. This causes auto_purge_post_terms_cache() to bail on the first call, and since the $static property is set then, it's not triggered later either, once the status is updated to publish.

Possible Solution...

I see that both of these hooks; i.e. added_term_relationship and delete_term_relationships include a list of term/taxnomy IDs. I think it could be helpful to read this array of IDs on those hooks, instead of being dependent upon the post having an association with the IDs in this particular scenario. That's how it is now, but we could add a new method to better deal with these two hooks where a connection to a published post may not exist just yet.

Although, I suppose it is important for us to look at the post status before we do anything here. What I mean is, we really DO need to know what the status is (or is going to be) before we do anything here. Hmm. Maybe we could look for $_POST['publish'] being present. That would indicate the status is going to become publish. Assuming the current_user_can('publish_posts').

[jaswrks]

My last commit here should resolve the issue.

I think it would be a good idea to create a second issue that references this one. Just something to remind us that in the future we should probably refactor the auto_purge_post_terms_cache() method so that this will be a little cleaner. Not really sure (at the moment) what would work best there.

[raamdev]

Thanks for fixing this, @jaswsinc! :) Pull Requests have been merged.

[raamdev]

Next release changelog:

• Bug Fix: Fixes an edge-case where the proper cache files do not get purged when a user with the Editor role publishes a new post with both a category and a tag associated with the post. See #313.

[raamdev]

@jaswsinc writes...

I think it would be a good idea to create a second issue that references this one. Just something to remind us that in the future we should probably refactor the auto_purge_post_terms_cache() method so that this will be a little cleaner. Not really sure (at the moment) what would work best there.

Hmm, I'm not really sure what would work best their either. What you have now (i.e., what you added to fix this issue) seems like all we really can do. Do you foresee a better way to optimize this?

I'm just wondering if we should bother opening another issue to refactor auto_purge_post_terms_cache(), or if there's a bigger issue or bigger refactor that needs to happen.

[jaswrks]

I'm just wondering if we should bother opening another issue to refactor auto_purge_post_terms_cache(), or if there's a bigger issue or bigger refactor that needs to happen.

What came to my mind the other day when I working on this, was that perhaps we could work on queuing these auto-purging routines instead of processing them in real-time; i.e. letting the hooks run as they do now, but instead of acting on these events, we hold-off until right before a script's shutdown phase; once WordPress is done with the tasks that it has been asked to do.

I see a few advantages to this...

• We can run through everything that needs to occur once all of the hooks are fired, and work to optimize the amount of directory scanning that really is needed to accomplish what we need to do as we purge cache files. In short, optimizing QC even further.
• It may allow us to better formulate (and consolidate) the Dashboard notices associated with automatic cache purging. Instead of these purging routines being scattered about in the codebase, they could theoretically be consolidated; and where they all occur together, as much as possible.
• This practice, in and of itself, would defer the execution of certain routines that Quick Cache runs, until such time as WordPress is finished with the tasks it was assigned. Therefore, things like the status of a post being changed while a hook is running, would not be nearly as messy for us to deal with.
• It might allow for a load-balanced approach to come in the future, where the automatic purging routines could perhaps run in phases on very large sites. That's another issue/discussion I think. But this refactoring might set the stage for something like this to occur later.

[raamdev]

we could work on queuing these auto-purging routines instead of processing them in real-time

Ah, I see where you're going with this now. Thanks! Fantastic idea. I love the idea of a queuing system--that would solve a lot of the funky issues we have to deal with right now. I'll open a new issue for this so we can work on it.