[wp-trac] [WordPress Trac] #14485: Taxonomy hierarchy cache isn't properly refreshed

[WordPress Trac]

#14485: Taxonomy hierarchy cache isn't properly refreshed
------------------------------+-----------------------------
 Reporter:  thealien          |       Owner:  wonderboymusic
     Type:  defect (bug)      |      Status:  reopened
 Priority:  high              |   Milestone:  3.9
Component:  Taxonomy          |     Version:  2.7
 Severity:  normal            |  Resolution:
 Keywords:  has-patch commit  |     Focuses:
------------------------------+-----------------------------
Changes (by SergeyBiryukov):

 * keywords:   => has-patch commit


Comment:

 So, [27115] introduced a race condition here.

 After adding a new term, it's important to clear the
 `{$taxonomy}_children` option in order for `get_terms()` to work properly
 with hierarchical taxonomies (as `_get_term_hierarchy()` relies on that
 option).

 Normally, this happens via `clean_term_cache()` call in
 `wp_insert_term()`:
 [source:tags/3.8.1/src/wp-includes/taxonomy.php#L2223]

 However, when rapidly creating new terms and calling `get_terms()` right
 away, `_get_term_hierarchy()` sometimes returns and caches stale data.

 Since [23401], we use `microtime()` for `last_changed` cache value. Before
 [27115], it used to be a string, now it's a float.

 Let's see what `last_changed` looks like when calling `get_terms()` in
 `test_get_terms_grandparent_zero()`. I've added logs to lines 1312 and
 1214: [source:tags/3.8.1/src/wp-includes/taxonomy.php#L1312].

 Before [27115]:
 {{{
 last_changed set: 0.66727500 1392952644
 last_changed get: 0.66862600 1392952644
 last_changed get: 0.67553500 1392952644
 last_changed get: 0.70352200 1392952644
 }}}
 After [27115]:
 {{{
 last_changed set: 1392953072.29
 last_changed get: 1392953072.29
 last_changed get: 1392953072.29
 last_changed get: 1392953072.32
 }}}
 As you can see, there are no identical values in the first set, which
 means the term cache is properly refreshed after creating each new term.

 In the second set, however, there are three identical values, which means
 `get_terms()` returns stale data from cache, and the test fails. With only
 two decimal digits, the chances of a collision have increased.

 According to [comment:ticket:27000:6], [27115] was introduced as a fix for
 an old `object-cache.php` drop-in, so we can probably revert it.

 If we still want to keep it, we need to call `clean_term_cache()` manually
 before `get_terms()` in our tests. `delete_option( "{$taxonomy}_children"
 )` would work too, but `clean_term_cache()` seems less cryptic.

 See [attachment:14485.3.diff].

 Tests for non-hierarchical taxonomies appear to be unaffected, as they
 don't involve `_get_term_hierarchy()`.

 Replying to [comment:94 kovshenin]:
 > I did get this test to fail consistently though:
 >
 > {{{
 > 1) Tests_Term::test_update_shared_term
 > Failed asserting that '61' is not equal to 61.
 > }}}

 That one should be skipped, #5809 is not fixed yet :)

--
Ticket URL: <https://core.trac.wordpress.org/ticket/14485#comment:95>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform