Add pre_wp_unique_post_slug to override post slug handling

[coffee2code]

As originally ​proposed in the comments of #20480 (and in fact further back in a ​comment for #14111), a pre_wp_unique_post_slug filter early in wp_unique_post_slug()could be useful and justified for some use cases rather than relying on the existing and late-firing wp_unique_post_slug filter for a couple reasons:

• wp_unique_post_slug() (and thus this new filter) is not high use, so won't impact performance
• wp_unique_post_slug() performs database queries (perhaps repeatedly until a unique slug is found), fires other filters, etc, all of which may be rendered moot if a hooking function's intent is to fully implement its own slug generation/handling logic. The pre slug could short-circuit that unnecessary effort.

The attached patch introduces the new filter in addition to adding related phpDocs.

[coffee2code]

Refreshed the 21112.diff patch. See associated comment for details.

[coffee2code]

Added 21112.2.diff​ as a refresh of 21112.diff​ in order to:

• Add inline docs for filter
• Add unit test
• Add braces
• Remove unnecessary additions to function's docblock

[johnbillion]

Out of interest, can you give a use case for this filter?

[coffee2code]

Well, likely most uses for the existing wp_unique_post_slug filter would be better served using the proposed pre_wp_unique_post_slug filter for the simple reason that if you are mucking around with unique slug detection (and regeneration due to a conflict), you're abandoning the built-in work being performed by wp_unique_post_slug().

Being early-firing (pre_wp_unique_post_slug) versus late-firing (wp_unique_post_slug) means you can skip the queries, filters firing, and code execution that you know will always be disregarded.

Some examples (geared towards custom-crafting the slugs the site uses) include:

• changing of certain terms when appearing in slugs:
  e.g. "wp-is-fun" => "wordpress-is-fun", "wp-cant-be-shuttered" => "wp-cannot-be-shuttered"
• removal of certain terms/phrases from appearing in slugs:
  e.g. "the-one-where-i-go-camping" => "where-i-go-camping"
• force prepending of word(s) to all, or contextually to some, slugs:
  e.g. "wp-is-fun" => "mysite-wp-is-fun"
• custom slug truncation length
  (Core has a 200 char limit, and while the internally used _truncate_post_slug() supports customizing that length via an argument, it is not otherwise filterable or customizable)
• custom duplicate slug suffix
  (core attempts "-N" where N is a number tried until a unique slug is found)
  e.g. "the-weekly-report-15" => "the-weekly-report-fifteen"
• custom slug rules per post type

In the end, yes, all this can be achieved with the existing wp_unique_post_slug filter, but it's a waste to run the 1(+n) queries and code execution in situations where custom handling will negate it all.

[chriscct7]

The unit test functions need doc blocs

[javorszky]

Just to add a bit more emphasis on this: a library we're using creates posts to keep track of scheduled events (wp's cron is not good enough). By default the post_name is not populated, but even when it is, because WordPress would do it on ajax, there's no authenticated user, and the post is pending. wp_insert_post strips post_name even if one was passed through here.

Because there's no way to bypass this, on a large site when there are 60,000+ entries with the same empty post_name, the server just comes to a grinding halt. It's also not great with hosts like WPEngine, that have a hard limit for script execution. It's 60 seconds in WPEngine's case.

Currently the only way for us to do this is to essentially copy the entire wp_insert_post function, and remove all the things that would hit wp_unique_post_slug and call our version of it from within the library. It's not very WordPress way to be honest.

Being able to short circuit the function would be AMAZING.

[CabGfx]

Chiming in here, to basically echo what @javorszky mentioned above, as this:

[…] on a large site when there are 60,000+ entries with the same empty post_name, the server just comes to a grinding halt. It's also not great with hosts like WPEngine, that have a hard limit for script execution. It's 60 seconds in WPEngine's case.

is exactly what we're experiencing at the moment :[

[johnjamesjacoby]

My thoughts:

• Needs seems genuine
• Patch seems reasonable
• Approach is similar to others we use in core
• This could go in as-is, or the patch could be refreshed to include phpdoc in the tests like @chriscct7 mentioned

Paging @johnbillion, who marked himself as owner. Able to see this through?

[negrusti]

Chiming in as well, 190000+ entries, 17 actions from scheduler generate ~190000 MySQL queries:

[javorszky]

I hotfixed this on a huge site with Subscriptions on. The result was that renewals went from around ~200 / hr to ~980 / hr.

Can we please get this into WP Core officially?

<puppy eyes>

[slaFFik]

Recently bumped into this as well.

@johnbillion, can you please consider to move this ticket forward?

[javorszky]

Attaching a refreshed diff. Test passes as of writing this comment.

[javorszky]

Refreshed state of 21112.2.diff

[javorszky]

Removing needs-docs, as test in 21112.2.diff has inline documentation.

[javorszky]

Rebased on latest master (50cd98012e), and changed @since tag to 5.0

[swissspidy]

Some notes on the patch and inline docs:

• Third-person singular verbs should be used for the summary, describing what is being filtered. This should be more like "Filters the unique post slug..." than "Filter to allow setting the unique post slug..." See the example at ​https://make.wordpress.org/core/handbook/best-practices/inline-documentation-standards/php/#language.
• Version should be using three digits, see ​https://make.wordpress.org/core/handbook/best-practices/inline-documentation-standards/php/#1-functions-class-methods. No need to change it right now because it can be adjusted before committing.
• Missing period after Post parent ID
• We usually use null for pre_ filters like in this case and then explicitly check whether the variable is null or something else.

[netweb]

Per ​https://make.wordpress.org/core/2018/10/08/wordpress-5-0-for-contributors-and-committers/

See also Slack discussion here ​https://wordpress.slack.com/archives/C02RQBWTW/p1538986854000100

The 21112.4.diff​ will also need refreshing to updatethe PHPDoc @since block to 5.1 and updating per @swissspidy's comment above

[iCaleb]

Refreshed the patch per the above comments, along with some additional documentation tweaks.

[swissspidy]

Looks good to me!

I don't see anything that would hold this back.

[netweb]

Reassigning to @swissspidy to move this along for 5.1

[pento]

In 44454:

Permalinks: Add a pre_wp_unique_post_slug filter.

Returning a non-null value on this fillter will cause wp_unique_post_slug() to return early with that value, skipping potentially expensive database queries on some sites.

Props coffee2code, javorszky, iCaleb.
Fixes #21112.

[ocean90]

#45855 was marked as a duplicate.

[lkraav]

Any chance of this getting backported to 4.9-branch?

[pento]

No, only security fixes are backported to branches older than the current major version.

[desrosj]

This was mentioned in a 5.1 dev note: ​https://make.wordpress.org/core/2019/01/23/miscellaneous-developer-focused-changes-in-5-1/