[wp-trac] [WordPress Trac] #34533: Term splitting in `get_term()` can have unexpected effects

[WordPress Trac]

#34533: Term splitting in `get_term()` can have unexpected effects
--------------------------+--------------------
 Reporter:  dlh           |       Owner:
     Type:  defect (bug)  |      Status:  new
 Priority:  high          |   Milestone:  4.4
Component:  Taxonomy      |     Version:  trunk
 Severity:  normal        |  Resolution:
 Keywords:  has-patch     |     Focuses:
--------------------------+--------------------
Changes (by boonebgorges):

 * keywords:   => has-patch
 * priority:  normal => high
 * milestone:  Awaiting Review => 4.4


Comment:

 @dlh and I had a number of helpful talks about this issue at WordCamp NYC
 last weekend.

 In a nutshell: I'd originally introduced the term-splitting fallback in
 [34997] as a sort of trick. I figured, if a term is still shared at this
 point, we may as well clean up the mess and split it. But, as noted in the
 ticket description, this ends up resulting in some funny behavior. Even if
 we were to make the suggested switch, so that we split the term from the
 requested taxonomy, we still have problems if someone has a `$term_id`,
 uses it to `get_term()`, and then attempts to do something else with
 `$term_id`, since it may have changed in the process of fetching the term.

 Given these problems, I'd like to remove the term-splitting fallback.
 Here's the logic that I think should be in its place. Assume that
 `$term_id` is shared between `tax1` and `tax2`.

 - `get_term( $term_id, 'tax1' )` should return the matching term
 - `get_term( $term_id, 'tax3' )` (invalid tax) should return `null`, for
 legacy reasons
 - `get_term( $term_id )` should return a `WP_Error` explaining that
 `$term_id` is ambiguous.

 See [attachment:34533.diff].

 Two notes about the suggested implementation:

 1. Terms are now cached according to `$term_id` only. As such, when a term
 is shared, we must refrain from caching it. That is, even if `get_term(
 $term_id, 'tax1' )` returns the expected term, it should never load it
 into the cache.
 2. `get_term()` can now return a `WP_Error` object, which it never did in
 the past. However, it only does it when `$taxonomy` is omitted. Prior to
 4.4, `$taxonomy` was a required parameter. So there should be no backward
 compatibility concerns with the new return type.

 It'd be great to get another set of eyes on this - especially those of
 @dlh :-D

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