Caching problems when using 'alias_of' in wp_update_term() and wp_insert_term()

[boonebgorges]

See https://core.trac.wordpress.org/browser/tags/4.0/src/wp-includes/taxonomy.php?annotate=blame#L2448

When creating a new term using wp_insert_term() and passing the alias_of parameter, the term_group of the aliased term is updated (see line 2467). However, the cache for this item is not cleared.

We should use wp_update_term() here instead of a direct database query. This takes care of caching for us. It also allows us to remove these duplicate 'edit_terms' and 'edited_terms' hooks.

Patch attached.

[boonebgorges]

There are actually a few bugs here, so let's do something more comprehensive. 29848.2.patch​ does the following:

• Give the same treatment to wp_insert_term() and wp_update_term()
• Use get_term_by( 'slug' ) instead of a direct DB query. Better because it's prettier, it'll hit the cache, it'll hit relevant filters, etc
• Use wp_update_term() instead of a direct database query, so that we purge the cache
• Remove the now redundant 'edit_terms' and 'edited_terms' hooks - they appear only in wp_update_term()
• Unit tests for using 'alias_of' in both functions

[boonebgorges]

Oh, almost forgot this important bullet point:

• Fix a PHP notice and incorrect term_group assignment when the term referenced in the 'alias_of' param does not exist. Currently, if the direct query turns up a $null alias, a new term group is created and the primary term is added to it (though not the aliased term, since there is no aliased term)

[DrewAPicture]

The only thing I'd be concerned about with removing the duplicate hooks is that it's possible somebody could be relying on that behavior. It's pretty edge and bordering on misuse, but worth considering.

[boonebgorges]

Replying to DrewAPicture:

The only thing I'd be concerned about with removing the duplicate hooks is that it's possible somebody could be relying on that behavior. It's pretty edge and bordering on misuse, but worth considering.

The hooks still fire in 29848.2.patch - they just fire in wp_update_term(). I think they were added before and after the direct db calls in order to provide parity with wp_update_term(), but using the API function means that this duplication is no longer necessary.

[DrewAPicture]

Replying to boonebgorges:

Replying to DrewAPicture:

The only thing I'd be concerned about with removing the duplicate hooks is that it's possible somebody could be relying on that behavior. It's pretty edge and bordering on misuse, but worth considering.

The hooks still fire in 29848.2.patch - they just fire in wp_update_term(). I think they were added before and after the direct db calls in order to provide parity with wp_update_term(), but using the API function means that this duplication is no longer necessary.

Sorry if I wasn't clear. What I meant was that it's possible people have workarounds in place to handle the duplicate filters.

Also, the hook doc that was converted to a comment in wp-includes/taxonomy.php should use /* to open instead of /** so it isn't mistakenly picked up as a docblock in the parser. The same goes for the new comment above this line further down:

$term_group = $wpdb->get_var("SELECT MAX(term_group) FROM $wpdb->terms") + 1;

The [syntax] for multi-line comments is simply:

/*
 * first line
 * second line
 */

[boonebgorges]

Sorry if I wasn't clear. What I meant was that it's possible people have workarounds in place to handle the duplicate filters.

Ah, yes. Like you say, this is very edge. And I'd assume that the "handling" would mainly involves "making sure that stuff isn't run twice", which will be moot after the duplicates are removed. I'm having a hard time thinking of a legitimate way to use the double hooks, but maybe the point is that those uses wouldn't be legitimate :)

The [syntax] for multi-line comments

My bad. Fixed in 29848.3.patch​ - at least for the multi-line comments. The hook documentation that I moved is unchanged; it begins with /**. I don't think I converted any hook docs to comments, but I might be misunderstanding your comment.

[DrewAPicture]

Replying to boonebgorges:

The [syntax] for multi-line comments

My bad. Fixed in 29848.3.patch​ - at least for the multi-line comments. The hook documentation that I moved is unchanged; it begins with /**. I don't think I converted any hook docs to comments, but I might be misunderstanding your comment.

It just appeared to be a "replacement" due to the changed-lines placement in the patch: ​http://cl.ly/image/3Q3R0y2C1d17 :-)

[boonebgorges]

Foiled again by the Trac algorithms!

[boonebgorges]

In 29862:

Improve 'alias_of' handling in wp_insert_term() and wp_update_term().

Using get_term_by() rather than direct SQL queries to fetch the alias term
fixes a number of issues:

• Object cache for aliased term is properly cleared after update.
• If the aliased term is in the object cache, it's served from there, saving a database query.
• Duplicate 'edit_terms' and 'edited_terms' hooks can be removed.
• Fix a PHP notice when the 'alias_of' term is not found.
• Prevent the incorrect creation of a new term group for the primary term when the 'alias_of' term is not found.

Adds unit tests for 'alias_of' functionality in both functions.

Fixes #29848.