#36814 closed defect (bug) (fixed)
get_the_terms() caches term count property
Reported by: | ocean90 | Owned by: | boonebgorges |
---|---|---|---|
Milestone: | 4.6 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Taxonomy | Keywords: | has-patch needs-testing |
Focuses: | Cc: |
Description
Noticed this on w.org see attached screenshot.
Call stack is get_the_tags( $id, 'post_tag' );
-> get_the_terms( $id, 'post_tag' )
.
get_the_terms()
caches the assigned terms which includes the count property too. But it seems like the cache gets only refreshed on post updates.
Attachments (2)
Change History (11)
#4
@
8 years ago
- Keywords has-patch needs-testing added; needs-patch needs-unit-tests removed
Invalidate term cache when a post is updated that has a term_relationship with that term.
On further review, more invalidation does *not* appear to be the right answer. Here's the problem. Term data (classes with properties term_id
, taxonomy
, count
, etc) are currently cached in *two* places:
wp_cache_set( $term_id, 'terms' )
(the single-term cache)wp_cache_set( $object_id, $terms, "{$taxonomy}_relationships" )
(the object-term-relationships cache -$terms
is an array of term objects)
When a term's count
changes, cache 1 is invalidated correctly. Cache 2 is not, which is what causes the current bug. But to invalidate it properly would mean busting the object relationship cache for *every single object* associated with the term. In other words: A category foo
is associated with 100 posts (count=100
). It's added to a new post. Fixing the current bug by invalidating cache 2 means that *all 100 posts* must have their relationship caches dumped. This is a lousy system.
There is a better way! The {$taxonomy}_relationships
cache should be an array of term IDs, rather than an array of term objects. Term objects can then be pulled from the single-term cache. See 36814.diff.
A couple back compat concerns. First, to maintain the same return value for get_object_term_cache()
- an array of term objects - I had to move a bunch of logic into there. In some rare cases, this may mean that calling get_object_term_cache()
results in a database query, in order to prime the cache for the individual terms. It's very rare, because the query only takes places when the {$taxonomy}_relationships
cache is primed, but one or more of the matching single-term caches is not. But this hardly ever happens, because get_object_term_cache()
is usually called in conjunction with wp_get_object_terms()
or another function that sets all of these caches. I don't think this is actually a concern, but it's worth mentioning for the record.
Second, the change means that the value stored in {$taxonomy}_relationships
is of a different format. Anyone directly reading that value and expecting it to be term objects will have their stuff break. I've done a search of the plugin repo and have found a very, very small number of cases that would ever be affected by this. To be extra safe, I've maintained support for plugins that *update* the {$taxonomy}_relationships
cache with term objects - this happens a bit more frequently in the wild.
Anyone see problems with the approach I'm suggesting? I think it's far more elegant than what we've currently got. (And it will make the cache schema more compatible with a split term query, if we ever get around to that; see #35381.)
I was hopeful that we'd be able to solve this by switching to ID-only queries in #35381, but I've been working on that ticket, and it looks like it won't be possible to make that change at this time. So the bug described here will have to be addressed separately.
Two possible strategies:
count
. There's some prior art here - inwp_get_object_terms()
, term cache is handled in such a way that theobject_id
property is uncached.term_relationship
with that term.I think the second technique is going to be a bit easier. Let me see what I can do about a patch.