add new cache clearing structure for term actions

[davelit]

This extends #129 to also support term updates. The term archive will be cleared from cache on term updates as well as parent and child archives and pages associated with the updated term. In addition to that, parent term archives will be cleared on post updates, since they often contain posts from child terms. Last but not least, the repeated queries for associated terms in Cache_Enabler::clear_taxonomies_archives_cache_by_post_id() were consolidated into a single query and updated to only consider taxonomies.

[coreykn]

Wow, this looks great! 🙂 This behavior will be a great enhancement and is definitely needed. Thank you for diving into this! I will be able to do an in-depth review here shortly. I have a change that I'll be adding for version 1.8 beforehand, but there are no overlaps with what you've done here so there shouldn't be any conflicts.

[coreykn]

@davelit, I know it's been a little while so it's not as fresh in your memory. I apologize for that. What you've started is really needed, but I believe we can simplify it. I have a question to help my understanding. Is there a reason why $taxonomy, the parameter containing the taxonomy slug, is used in the following methods you're proposing...

• Cache_Enabler::get_term()
• Cache_Enabler::clear_parent_term_cache()
• Cache_Enabler::clear_child_term_cache()
• Cache_Enabler::clear_page_cache_by_term()

...instead of just relying on the term ID? From what I can tell each term will have a unique ID, so getting the taxonomy term link only needs that term ID. I know many core functions use that taxonomy slug optionally, but that is to check if the taxonomy_exists() from what I can tell.

I believe you've used this to check if the taxonomy is public in Cache_Enabler::is_taxonomy_public(), which is_taxonomy_viewable() could be used for that purpose instead (would be checking $taxonomy->publicly_queryable instead of $taxonomy->public). I think that is why the $term_archive_url is being checked if it has a query string as the private taxonomies have that? I need to run more tests to confirm that. If that is the case we could remove that query string check.

[davelit]

@coreykn Thank you for considering my PR, no worries regarding the delay :-).

I agree with what you've said regarding the $taxonomy parameter. I have added it primarily because of these lines, which seem to indicate that the same $term_id can have different taxonomies (maybe over time?):

$_term = wp_cache_get( $term_id, 'terms' );

// If there isn't a cached version, hit the database.
if ( ! $_term || ( $taxonomy && $taxonomy !== $_term->taxonomy ) ) { 
     ...

As regards Cache_Enabler::is_taxonomy_public() I also agree that it can probably be replaced by is_taxonomy_viewable(). Cache_Enabler::is_taxonomy_public() checks whether the taxonomy was registered as public while is_taxonomy_viewable() returns true if it was registered as publicly_queryable, with publicly_queryable being a subset of public. To quote the docs:

• 'public' (bool) Whether a taxonomy is intended for use publicly either via the admin interface or by front-end users. The default settings of $publicly_queryable, $show_ui, and $show_in_nav_menus are inherited from $public.
• 'publicly_queryable' (bool) Whether the taxonomy is publicly queryable. If not set, the default is inherited from $public To give you an example for a public, but not publicly queryable taxnonomy: We have a taxonomy in our backend to assign posts to a "consumer" or a "business" category. We then inject a tracking varible into the frontend code, that helps us to filter by target audience in Google Analytics. However, we rarely update the terms in this taxonomy, so is_taxonomy_viewable() would work for our use case too.

Cache_Enabler::clear_term_archive_cache() is the term counterpart of Cache_Enabler::clear_page_cache_by_post_id() from where I copied the query string check. I'm not sure why it was there in the first place.

[coreykn]

Thanks for your understanding. Much appreciated!

I have added it primarily because of these lines, which seem to indicate that the same $term_id can have different taxonomies (maybe over time?)

Ah, you're absolutely right! That's really helpful to see and know. I wasn't aware of that before. That really assists in my review and I will follow up with any additional questions if I have any. You did a very thorough, well-thought out job and I'm excited to get this change merged. Some of it may be adjusted, but overall the base of it will all come from what you've came up with. After I'm done with my changes I'll let you know. At that point I'd love to get your eyes on it.

I also agree that it can probably be replaced by is_taxonomy_viewable().

Great, good to know.

Cache_Enabler::clear_term_archive_cache() is the term counterpart of Cache_Enabler::clear_page_cache_by_post_id() from where I copied the query string check. I'm not sure why it was there in the first place.

Cache_Enabler::clear_taxonomies_archives_cache_by_post_id() contained the query string check, which is where the Cache_Enabler::clear_term_archive_cache() method looks to have originated from (which makes a lot of sense to create it's own method given the changes made). If my memory servers me right, this was done to prevent sending clear cache requests from what I believe were private, or non-public, taxonomy term links. I'll make sure to check this thoroughly in case we need to remove it with the changes being brought in.

As for why a query string check is in Cache_Enabler::clear_page_cache_by_post_id(), that was added in #216 to prevent continuing with clearing the page cache when a guid link was retrieved (e.g. https://www.example.com/p=123), like after a page has been changed to a draft.

[coreykn]

@davelit, I think we're ready to merge this. When you have time, would you like to review the changes I've made?

If my memory serves me right, this was done to prevent sending clear cache requests from what I believe were private, or non-public, taxonomy term links.

That was correct. This has been removed from Cache_Enabler::clear_term_archive_cache() because we'll bail early if the term taxonomy is not publicly viewable.

If you have any questions just let me know. I'll be adding another PR after this that'll include some of the other changes briefly mentioned in my commits in this PR.

[davelit]

@coreykn I haven't had a chance to review your changes yet. I'll do that as soon as I can, but that shouldn't hold you back from merging this PR.

[coreykn]

Thanks for the update, @davelit. Take as much time as you need because there isn't a rush. I'll get this merged and feel free to follow up at any time should any feedback arise. Thanks for your large contribution in this PR, it's surely appreciated! 🙂