WordPress.org

Make WordPress Core

Opened 8 hours ago

Last modified 8 hours ago

#49799 new defect (bug)

get_the_terms() object term cache check too strict?

Reported by: dingo_d Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 5.4
Component: Taxonomy Keywords: dev-feedback 2nd-opinion has-patch
Focuses: Cc:

Description

I ran into a weird issue when doing integration tests on one of a client's project.

I was setting up custom post and custom taxonomy from the factory (for posts I used the default one, and for custom taxonomy I have created my own factory that extends WP_UnitTest_Factory_For_Term), set everything up, but the part of the plugins code that used get_the_terms returned nothing. So after racking my brains, I tried wp_get_post_terms, and lo and behold, this worked (the test posts had assigned terms for them, I checked before invoking my function I needed to test using get_the_terms).

So something was up with caching. I started to dig and it turned out that

$terms = get_object_term_cache( $post->ID, $taxonomy );

inside wp-includes/category-template.php

returned an empty array.

Now, since the line 1246 says

if ( false === $terms ) {

The empty array, while being a falsy value, it's not a false value (https://3v4l.org/RVuG7), so the above check will return false and the returned value from the get_the_terms function will be false.

So when I 'loosened' the condition in my test suite WP, the $terms = wp_get_object_terms( $post->ID, $taxonomy );
works, and I get my terms inside the tests.

If the condition is loosened, nothing seems to be lost in terms of checking the emptiness of the terms from the term object cache (if it's full, it's false all the times https://3v4l.org/34UQk).

Now, on the face value, this could be the oddity of the test suite (which in my case is set up using wpcli scaffold command), but I'm thinking that changing this requirement cannot hurt (from what I can tell) in the long run.

Attachments (1)

49799.diff (546 bytes) - added by dingo_d 8 hours ago.
Patch with the fix

Download all attachments as: .zip

Change History (3)

This ticket was mentioned in PR #211 on WordPress/wordpress-develop by dingo-d.


8 hours ago

Loosened the condition to check the terms fetched fro mthe object term cache. The cache can return an empty array, which is a falsy value, so the get_the_terms will return false value.

Trac ticket: https://core.trac.wordpress.org/ticket/49799

@dingo_d
8 hours ago

Patch with the fix

#2 @prbot
8 hours ago

dingo-d commented on PR #211:

Also, maybe a good thing to note is that I'm assigning the terms in my tests using wp_set_object_terms(); function. The same thing happened when I tried wp_set_post_terms(), so it could be something related to that (test env could mess with the object caching?)

Note: See TracTickets for help on using tickets.