[wp-trac] [WordPress Trac] #29589: term_exists() short-circuits and results in duplicate inserts.

[WordPress Trac]

#29589: term_exists() short-circuits and results in duplicate inserts.
-----------------------------------+---------------------------
 Reporter:  georgestephanis        |       Owner:  boonebgorges
     Type:  defect (bug)           |      Status:  assigned
 Priority:  normal                 |   Milestone:  4.1
Component:  Taxonomy               |     Version:  trunk
 Severity:  normal                 |  Resolution:
 Keywords:  has-patch 2nd-opinion  |     Focuses:
-----------------------------------+---------------------------
Changes (by boonebgorges):

 * keywords:  needs-patch => has-patch 2nd-opinion


Comment:

 Confirmed. Thanks for the report and for the unit tests, georgestephanis.

 Your second test is sorta redundant - since `wp_set_object_terms()`
 ultimately falls back onto `term_exists()`, we don't need it.

 > I'm not sure the ideal place to fix it, I suspect it's the conditional
 in term_exists, but we may want to just modify terms to always reject
 something that can't produce a slug.

 My thought is that we should ditch the special check for empty slugs
 altogether. It looks like the only purpose it serves is to save a database
 query in case a slug has sanitized down to an empty string. But the query
 is definitely not worth saving if it causes a short circuit like this.

 29589.1 is my proposed fix. Note that this will lead to a slight change in
 behavior: when you pass a slug that sanitizes to an empty string, the
 function returns `null` rather than `0`. Checking this kind of slug is
 already an edge case; the possibility that someone is doing a strictly
 typed check of this return value is an even bigger edge case. Plus,
 returning `null` is in keeping with the behavior of the rest of cases when
 terms are not found. So I think the change is a good one. I've updated the
 relevant existing unit test
 (`test_term_exists_term_trimmed_to_empty_string()`) to reflect the change.
 I've also updated the docblock, which was incorrect to begin with.

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