Split do_all_pings

[dshanske]

do_all_pings is a function that does pingbacks, trackbacks, enclosures, and updates. It is attached to a hook called do_pings.

Suggesting that the function be broken into individual pieces and those pieces attached in priority order to the same hook.

This would allow the specific removal/replacement of one of more services.

Splits do_all_pings into separate functions. Keeps the do_all_pings function for backcompat, but hooks individual functions on to the action.

Trac ticket: https://core.trac.wordpress.org/ticket/36576

[hellofromTonya]

New public function added. Needs a mention in the Misc Dev Note.

[dshanske]

@Mista-Flo I am a bit confused about what unit testing is required or what refresh. There is effectively no change. Three sets of code were in one function hooked into an action, they are now separate functions hooked into the same action, so they can be individually removed. The code in question was already being unit tested.

@hellofromTonya Where does one write a dev note? If I were to note it here, it would say, "Since WordPress 2.1, do_all_pings has, every time a post is published, queried for all pending pingbacks, trackbacks, and enclosures, regardless of whether you are actually using these features. It is hooked into the do_pings action. This change splits each query into its own function(do_pingbacks, do_enclosures, do_trackbacks), which are added to the do_pings action so you could remove one or more on systems not using them. The do_all_pings function is retained for backward compatibility and now calls the individual functions."

[Mista-Flo]

Hi @dshanske Yes, after looking at the code, I think it's safe to assume it does not need unit test, sorry for the wrong alert.

Nice catch about the unused global wpdb variable btw.

The code looks good, great job

[hellofromTonya]

@dshanske Here's a link in the handbook for "Writing developer notes" ​https://make.wordpress.org/core/handbook/tutorials/writing-developer-notes/. This ticket may only need a mention in the Misc Dev Note.

[dshanske]

@hellofromTonya I think it would be in a Misc one.

[hellofromTonya]

Thanks @dshanske. I think you're right. I edited my comment above to avoid any confusion when we're doing the Misc dev note.

[johnbillion]

Consideration: If a site has removed the do_all_pings action from the do_pings hook in order to disable all pinging, this change will break that and cause all pinging to be reactivated.

[dshanske]

@johnbillion That is a good point...how do we address it while allowing the increased granularity?

[johnbillion]

Ideas anyone?

[garrett-eclipse]

Could we leave a do_all_pings function and call action, and inside the do_all_pings function call the others seperately?
So if you remove do_all_pings all would be disabled. But you could also selectively disable from their own actions.

<?php
function do_all_pings() {
    do_action( 'doing_pings' );
}
add_action( 'do_pings', 'do_all_pings', 10, 0 );
add_action( 'doing_pings', 'do_pingbacks', 10, 0 );
add_action( 'doing_pings', 'do_enclosures', 10, 0 );
add_action( 'doing_pings', 'do_tracksbacks', 10, 0 );
add_action( 'doing_pings', 'generic_ping', 10, 0 );

This leaves the ability to remove all pings, but also introduces the ability to remove specific pings.

Thoughts?

[azaozz]

Yeah, seems this will work. Refactor do_all_pings() into a function that only triggers the action all "ping types" are attached to.

Of course all add_action() will go in /wp-includes/default-filters.php and the code from do_all_pings() will be separated similarly to
​https://github.com/WordPress/wordpress-develop/pull/494/commits/6a527c50c47cb4d0f534a8cccc270a5410e8837b.

[dshanske]

The interim measure could be just to split the do_all_pings functions into individual functions, have them still loaded onto the hook using do_all_pings, and allow for customization by removing it and adding the individual functions.

Does that make sense?

[garrett-eclipse]

Updated patch to introduce a perform_pings action inside of the do_all_pings function.

[garrett-eclipse]

I've put together an initial patch 36576.diff in this direction, please test and review.

In the patch I introduce a new action hook perform_pings used to hook in the other new functions. I did rename the new methods from do_ as there's already a do_trackbacks so wanted to avoid a duplicate use of the function name. And I removed the first comments inside the new functions as their function docblock already handles the description and they're mostly duplicated there.

[garrett-eclipse]

Minor fix as I'd left a action param from where I'd copied it from. There should be no params for the action.

[garrett-eclipse]

Replying to dshanske:

The interim measure could be just to split the do_all_pings functions into individual functions, have them still loaded onto the hook using do_all_pings, and allow for customization by removing it and adding the individual functions.

Does that make sense?

Sorry @dshanske didn't see your comment while I uploaded. Take a look at the patch and let me know if that's the direction that would make sense. Thanks

[dshanske]

@garrett-eclipse It looks good and I'm fine with doing it this way. The reason I asked the other question was because we'd be adding another hook to accomplish it this way.

But if everyone is fine with that, it solves the problem neatly.

[hellofromTonya]

Notes from today's scrub warrant changes:

• Prefix function with wp_. Helen suggests

I think it might be best to go with wp_do_trackbacks() etc. but I am not SUPER strongly opinionated on it

• Sergey noted a typo

There's also a typo in the second file, tracksbacks.

Beta 1 is tomorrow. But if we can get the above changes and then testing, there's a chance for it to land in 5.6.

[Mista-Flo]

Refreshed patch, fix typo

[Mista-Flo]

Hi there, I have uploaded a new patch, just a quick update to fix the typo and add a wp prefix to the functions. I am not confident to rename generic_ping for backward compatible reasons.

[SergeyBiryukov]

In 49211:

Pings/Trackbacks: Split do_all_pings() into several functions:

• do_all_pingbacks()
• do_all_enclosures()
• do_all_trackbacks()

This allows for the specific removal/replacement of one of more services.

Props dshanske, garrett-eclipse, Mista-Flo, azaozz, hellofromTonya.
Fixes #36576.

[SergeyBiryukov]

Went back and forth on function naming for a bit, decided to use the existing pattern for consistency :)

Merged into WP Core in https://core.trac.wordpress.org/changeset/49211

[daisyo]

Dev note published: ​https://make.wordpress.org/core/2020/11/20/miscellaneous-developer-focused-changes-in-wordpress-5-6/