[wp-trac] [WordPress Trac] #49799: get_the_terms() object term cache check too strict?

[WordPress Trac]

#49799: get_the_terms() object term cache check too strict?
------------------------------------------------+--------------------------
 Reporter:  dingo_d                             |       Owner:  (none)
     Type:  defect (bug)                        |      Status:  new
 Priority:  normal                              |   Milestone:  Awaiting
                                                |  Review
Component:  Taxonomy                            |     Version:  2.3
 Severity:  normal                              |  Resolution:
 Keywords:  reporter-feedback needs-unit-tests  |     Focuses:
------------------------------------------------+--------------------------
Changes (by SergeyBiryukov):

 * keywords:  dev-feedback 2nd-opinion has-patch => reporter-feedback needs-
     unit-tests
 * version:  5.4 => 2.3


Comment:

 Thanks for the ticket!

 Replying to [comment:4 joyously]:
 > Is this another fallout from #48965 (similar to #49853)?

 Doesn't seem like that. This line was added in [5598] / #4189 for
 WordPress 2.3, moved to the newly created `get_the_terms()` function in
 [7520] / #6357, and has not changed much since then.

 {{{
 $terms = get_object_term_cache( $post_id, $taxonomy );

 if ( false === $terms ) {
         $terms = wp_get_object_terms( $post_id, $taxonomy, $args );
         ...
 }
 }}}

 This pattern is not specific to `get_the_terms()`, it's used in quite a
 few other places in core:

 * `get_attachment_fields_to_edit()`
 * `get_compat_media_markup()`
 * `get_terms_to_edit()`
 * `get_inline_data()`
 * `wp_queue_posts_for_term_meta_lazyload()`
 * `get_the_taxonomies()`
 * `is_object_in_term()`

 So while there indeed might be an issue here, this doesn't appear to be a
 regression in 5.4.

 The `false === ...` check is specifically used to ensure the cache is
 empty. Making the check less strict would go against the coding standards
 and doesn't seem like the correct way to fix this.

 Instead, a check for an empty array should be added if necessary. Before
 making any changes though, it would be helpful to have some unit tests to
 reproduce the issue.

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