Make WordPress Core

Opened 12 years ago

Closed 11 years ago

#25711 closed defect (bug) (fixed)

The "{$taxonomy}_children" cache can become stale

Reported by: dd32's profile dd32 Owned by:
Milestone: 3.9 Priority: normal
Severity: normal Version: 3.8
Component: Taxonomy Keywords:
Focuses: Cc:

Description

When you add multiple hierarchical terms on a single page load, "{$taxonomy}_children" can become stale as only the first call to clean_term_cache() will invalidate that cache.

On subsequent calls, it, along with the all_ids and get taxonomy caches will not be cleared, which leads to anything which relies upon hierarchical checks to fail.
Many things rely upon the hierarchical cache, retrieving children of terms being the primary one.

This can be traced to the static $cleaned added in [9102].

I've attached a basic unit test that covers "{$taxonomy}_children" via _get_term_hierarchy() in 25711.diff

Attachments (1)

25711.diff (1.8 KB) - added by dd32 12 years ago.

Download all attachments as: .zip

Change History (7)

@dd32
12 years ago

#1 @nacin
12 years ago

Related ticket, referring to {$taxonomy}_relationships stale cache: #24189.

This ticket was mentioned in IRC in #wordpress-dev by helen. View the logs.


11 years ago

#3 @wonderboymusic
11 years ago

  • Milestone changed from Awaiting Review to 3.9
  • Resolution set to fixed
  • Status changed from new to closed

Fixed via [27102].

#4 @bpetty
11 years ago

  • Keywords needs-unit-tests added
  • Resolution fixed deleted
  • Status changed from closed to reopened

I'm not sure exactly what values should be tested for here, so I'm not going to attempt a patch, but this does need some eyes on the unit tests again at least.

1) Tests_Term_Cache::test_category_children_cache
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
 Array (
-    75 => Array (...)
 )

/vagrant/wordpress/tests/phpunit/tests/term/cache.php:22

Here's the assertion:

$this->assertEquals( array( $term_id1 => array( $term_id1_child ) ), $hierarchy );

I've only see this fail on PHP 5.2, however, what I'd like to specifically point out is that this is comparing the result fetched from the factory, which should contain row IDs, and likely a bunch more data that is both irrelevant to this test, and is also volatile (can easily change between runs just like the ID does depending on previous tests run - try running full tests compared to just --group taxonomy).

This really needs to be rewritten with more more specific values used for assertion checks. Try to avoid using arrays, they can frequently fail when just out of order (but still remain functional and correct).

Version 0, edited 11 years ago by bpetty (next)

#6 @SergeyBiryukov
11 years ago

  • Keywords needs-unit-tests removed
  • Resolution set to fixed
  • Status changed from reopened to closed

The changes suggested in 25711.diff were committed in [27163] and [27103].

Unit test failure is not directly related to those commits, see ticket:14485:95. Let's fix it in #14485.

Note: See TracTickets for help on using tickets.