Short-circuit HEAD methods in Core controllers

[TimothyBlynJacobs]

The REST API has built-in support for responding to a HEAD request. If no callback is specifically registered for the method, the server will fallback to the associated GET handler. However, this means that the server is throwing away work when no response body is needed.

GET routes can be adapted to check if the WP_REST_Request::get_method is HEAD and if so skip preparing the response body. For single item routes, this is pretty straightforward.

It is a little more nebulous for collection routes. We still need to provide any headers like pagination which are currently prepared after the response body is generated.

I think we'd want to extract the pagination logic to it's own method. Then, before making the database query,
adjust the query to only return ids. This will give us enough information to build the pagination headers and allow us to send the HEAD response.

See ​https://github.com/WordPress/gutenberg/pull/43703

[antonvlasenko]

I’m working on a patch to explore implementing this for both single and collection routes. If it’s challenging to do both, I’ll start with single routes first, as this should provide better insight into how to implement it for collection routes later.

Update (19/8): I will be unable to work on this issue this week, but I will resume next week.

U̶p̶d̶a̶t̶e̶ (̶2̶8̶/8̶)̶:̶ r̶e̶s̶u̶m̶e̶d̶ w̶o̶r̶k̶i̶n̶g̶ o̶n̶ t̶h̶i̶s̶ i̶s̶s̶u̶e̶.
Update (2/9): resumed working on this issue.

[Mamaduka]

Thanks for the updates, @antonvlasenko!

Let me know if I can help with this in any way.

[antonvlasenko]

Let me know if I can help with this in any way.

@Mamaduka, that's very kind of you. Thanks!
I think you can help with testing and code reviews once the first PR is ready.
I'm currently working on the posts controller.

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

[antonvlasenko]

I've just submitted a first PR to address the issue for the WP_REST_Posts_Controller controller: ​https://github.com/WordPress/wordpress-develop/pull/7349.

I think we'd want to extract the pagination logic to it's own method.

For the WP_REST_Posts_Controller, the adjustments required for HEAD requests compared to standard GET requests are minimal, so I found it unnecessary to extract the pagination logic into a separate method.

As suggested by @TimothyBlynJacobs, the proposed PR optimizes performance by adjusting the query to return only ids. If the direction of my PR is considered correct, I am prepared to make similar adjustments for other controllers in future PRs.

My PR also adds several unit tests for the HEAD method to ensure that the WP_REST_Posts_Controller functions correctly.

Please note that I will be unavailable until Sep 23, but I can resume work on subsequent PRs upon my return.

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

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

[antonvlasenko]

I'd like to share the performance improvement data I measured after applying the three submitted PRs to the relevant controllers:

• ​Posts controller: 122ms improvement (100 posts per page);
• ​Comments controller: 179ms improvement (100 comments per page);
• ​Terms controller: 124ms improvement (100 categories per page);

@TimothyBlynJacobs, any feedback on these PRs would be greatly appreciated!

[TimothyBlynJacobs]

Thanks for sharing! That sounds very promising. Will take a deeper look this weekend.

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

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

[antonvlasenko]

I'd like to share the performance improvement data I measured after submitting two additional PRs and applying them to the relevant controllers:

• ​Users controller: 199 ms improvement (100 users);
• ​Taxonomies controller: 0.45 ms improvement.

[TimothyBlynJacobs]

Yeah, I think we can commit basically this. Can you roll it into one patch @antonvlasenko?

The only other thing, is I'd rather us pass the uppercase form of the methods to the is_method(). That's what we use everywhere in the REST API. I think it makes sense to allow someone to pass the lowercased named. But I think Core should be consistent.

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

[antonvlasenko]

@TimothyBlynJacobs I've consolidated all the PRs into one, updated the string cases passed to is_method() to uppercase, and added unit tests for the is_method() method.
Thank you for the review!

PR: ​https://github.com/WordPress/wordpress-develop/pull/7970

Fantastic work, @anton-vlasenko!

Let me know how I can help to land this in WP 6.8. I think it would improve editors' loading performance.

Fantastic work, @anton-vlasenko!

Thank you, @Mamaduka.

Let me know how I can help to land this in WP 6.8. I think it would improve editors' loading performance.

Thank you for offering your help!
A code review and/or testing report could help with landing this in WordPress 6.8.
This ticket is likely on @TimothyBJacobs's radar, so hopefully, he'll be able to approve and commit it when he has the time, provided there’s no additional feedback from the code review.

Closed in favour of ​https://github.com/WordPress/wordpress-develop/pull/7970.
Context: https://core.trac.wordpress.org/ticket/56481#comment:14.

Closed in favour of ​https://github.com/WordPress/wordpress-develop/pull/7970.
Context: https://core.trac.wordpress.org/ticket/56481#comment:14.

Closed in favour of ​https://github.com/WordPress/wordpress-develop/pull/7970.
Context: https://core.trac.wordpress.org/ticket/56481#comment:14.

Closed in favour of ​https://github.com/WordPress/wordpress-develop/pull/7970.
Context: https://core.trac.wordpress.org/ticket/56481#comment:14.

Closed in favour of ​https://github.com/WordPress/wordpress-develop/pull/7970.
Context: https://core.trac.wordpress.org/ticket/56481#comment:14.

Yeah I'm planning on landing this this weekend.

I think we need to also disable any meta and term cache priming as well, as this results in database queries.

Also if there are changes to be made for taxononmy endpoint, those changes should also be made for post type endpoint. Otherwise this looks like great work.

Thank you for the review, @spacedmonkey.
Well spotted.
I've addresses these issues in 294584e3a25df251df985b2c28525ac264b9adff and b80fd51601bb95ea2ddc04ea98b7e4d79f68ac8c.

I think we have a BC issue with the single item route. Right now, if I use the rest_prepare_ filters, I can set headers on the returned response object. After these changes, this no longer happens.

I think we might want to move the is_method( 'HEAD' ) cehcks into the prepare_item_for_response methods, and prepare the empty response object there. That way, if someone adds a header, it is maintained. If they add a property, that's fine because it'll get omitted by the server.

I think we have a BC issue with the single item route. Right now, if I use the rest_prepare_ filters, I can set headers on the returned response object. After these changes, this no longer happens.

I think we might want to move the is_method( 'HEAD' ) cehcks into the prepare_item_for_response methods, and prepare the empty response object there. That way, if someone adds a header, it is maintained. If they add a property, that's fine because it'll get omitted by the server.

Thank you for the review, @TimothyBJacobs.
While I agree that this is a BC break, I tried searching for any plugins that add headers using the rest_prepare_ filters and couldn’t find any:

• ​https://www.wpdirectory.net/search/01JFAMD046JGJRWHWQ1QNMZAW9
• ​https://www.wpdirectory.net/search/01JFAMDVT9ZWJ0TS45ZG5B1D1E

It could theoretically happen, but I’m not aware of any such cases.

That said, I also agree that GET and HEAD requests should behave similarly, with the exception that HEAD requests don’t include a response body.

I’m working on a fix.

---

There are 42 files in the endpoints directory. Some of these are sub classes. But there are other endpoints that are not sub classes of the files edited in this PR. Is this applied all the endpoints that need this change?

No, it hasn’t been applied to all the classes that need editing, as this PR focuses only on the most commonly used REST controllers.
Do you think the other controllers can be addressed in a follow-up PR, or would you prefer to fix everything in a single PR?

There are 42 files in the endpoints directory. Some of these are sub classes. But there are other endpoints that are not sub classes of the files edited in this PR. Is this applied all the endpoints that need this change?

@jonnynews No, it hasn’t been applied to all the classes that need editing, as this PR focuses only on the most commonly used REST controllers. Do you think the other controllers can be addressed in a follow-up PR, or would you prefer to fix everything in a single PR?

In core, we normally do these things one commit.

Some controllers might benefit from this work.

WP_REST_Block_Pattern_Categories_Controller
WP_REST_Block_Types_Controller
WP_REST_Font_Collections_Controller
WP_REST_Global_Styles_Revisions_Controller
WP_REST_Pattern_Directory_Controller
WP_REST_Search_Controller
WP_REST_Sidebars_Controller
WP_REST_Templates_Controller
WP_REST_Widget_Types_Controller
WP_REST_Widgets_Controller

To clear, as I am asking, I believe it just a simply copy and paste for most of these endpoints, but correct me if I am wrong.

Some controllers might benefit from this work.

WP_REST_Block_Pattern_Categories_Controller
WP_REST_Block_Types_Controller
WP_REST_Font_Collections_Controller
WP_REST_Global_Styles_Revisions_Controller
WP_REST_Pattern_Directory_Controller
WP_REST_Search_Controller
WP_REST_Sidebars_Controller
WP_REST_Templates_Controller
WP_REST_Widget_Types_Controller
WP_REST_Widgets_Controller

To clear, as I am asking, I believe it just a simply copy and paste for most of these endpoints, but correct me if I am wrong.

Yes, I believe it is. PHPUnit tests would need to be added as well (which can also be partially copy-pasted). I'll work on this tomorrow.

[spacedmonkey]

Approved PR. This is good to commit. @antonvlasenko @TimothyBlynJacobs

[antonvlasenko]

Approved PR. This is good to commit.

Thank you, @spacedmonkey.

I haven't fixed the following controllers from your list yet:

• WP_REST_Search_Controller
• WP_REST_Sidebars_Controller
• WP_REST_Templates_Controller
• WP_REST_Widget_Types_Controller
• WP_REST_Widgets_Controller

I should be able to fix them by the end of the day.

I believe this PR is ready for review.

Additionally, I've updated some REST controllers beyond the ones ​listed, including:

• WP_REST_Revisions_Controller;
• WP_REST_Template_Revisions_Controller
• WP_REST_Autosaves_Controller;
• WP_REST_Template_Autosaves_Controller.

[flixos90]

@TimothyBlynJacobs Just checking in to get a status update. Based on the feedback above, it looks like this is ready to commit. Are you planning to commit it, or are there any outstanding concerns from your end?

[TimothyBlynJacobs]

Thanks for the ping @flixos90. I'll take another look and plan on committing shortly.

I'm working on fixing the merge conflicts.

[spacedmonkey]

I have reviewed @TimothyBlynJacobs and the PR looks good to go now.

[swissspidy]

@TimothyBlynJacobs Is this one on your radar to commit before the 6.8 beta?

[TimothyBlynJacobs]

yep!

[audrasjb]

@TimothyBlynJacobs it looks like the last PR is ready for commit. Please commit it by Monday so it's not punted before we release beta 1. Thanks! :)

[TimothyBlynJacobs]

In 59899:

REST API: Improve performance for HEAD requests.

By default, the REST API responds to HEAD rqeuests by calling the GET handler and omitting the body from the response. While convenient, this ends up performing needless work that slows down the API response time.

This commit adjusts the Core controllers to specifically handle HEAD requests by not preparing the response body.

Fixes #56481.
Props antonvlasenko, janusdev, ironprogrammer, swissspidy, spacedmonkey, mukesh27, mamaduka, timothyblynjacobs.

[Mamaduka]

The Gutenberg e2e tests discovered a possible fatal error coming from rest_filter_response_fields - PHP Fatal error: Uncaught TypeError: array_intersect_key(): Argument #1 ($array) must be of type array, null given.

How to reproduce

• Make a HEAD request with the _fields argument. This triggers an error since response data is now a null.

I'm guessing that the usage in Gutenberg was related to previous behavior, and the client code was trying to limit the response size.

Props to @abcd95 for discovering the initial connection - ​https://github.com/WordPress/gutenberg/pull/69378#issuecomment-2693912904.

[wildworks]

I've summarized the issues here: ​https://github.com/WordPress/gutenberg/issues/69392

[antonvlasenko]

I’ll look into it today, @Mamaduka @wildworks. Hopefully, we won’t have to revert the patch.

[Mamaduka]

@antonvlasenko, adding an extra check for response data to the rest_filter_response_fields also fixes the issue.

[TimothyBlynJacobs]

Nice catch, thanks @Mamaduka! I think the most correct fix here would be to set the data to an empty array when we create the new WP_REST_Response object.

It occurs because the first argument is null.

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

The tests are failing but that is expected. I'll fix them in a couple of hours (need to go now).

[flixos90]

Marking this as "task (blessed)", just to make it obvious that this can continue post Beta, since it's only about a follow up bug to what was already committed prior to Beta.

[antonvlasenko]

The pull request at ​https://github.com/WordPress/wordpress-develop/pull/8440 presumably fixes the fatal errors introduced in ​https://github.com/WordPress/wordpress-develop/pull/7970.
It’s ready for review.

Props to @Mamaduka, @wildworks, @TimothyBlynJacobs , and @antonvlasenko.

Thanks @anton-vlasenko! This looks right to me.

Could we add a unit test that specifically covers a HEAD request with ?_fields present?

Thanks @anton-vlasenko! This looks right to me.

Could we add a unit test that specifically covers a HEAD request with ?_fields present?

Awesome! I'll add them today and let you know once they're ready.

@TimothyBJacobs I've added more tests in 0b08957b7e13aa22b4ba2d4e3b18b1035a79071b. This took longer than I anticipated, but I've double checked they cover the regression.

Sweet. Thank you!

Thanks for working on this, @anton-vlasenko!

@TimothyBJacobs, do you have time to commit this before Beta 2?

[mehdi01]

Thank You @TimothyBlynJacobs

This makes sense for single-item routes. For collection routes, would returning only ids affect any existing filters or query modifications developers might be using? Also, would this change require adjustments in custom REST endpoints?

[TimothyBlynJacobs]

In 59970:

REST API: Fix fatal error when making HEAD requests with _fields filter.

In [59889] the REST API controllers were adjusted to perform less work when responding to HEAD requests. The WP_REST_Response body would now be null, which caused issues with filters that expected the response body to be an array.

This commit sets the response body to be an empty array when preparing the response instead. The body will still be discarded, but this provides better backward comppatibility with code that assumes an array will be used.

See #56481.
Props antonvlasenko, timothyblynjacobs, mamaduka, wildworks.

[Mamaduka]

I was reminded that the HEAD request still needs to adhere to user permissions, which can be a roadblock to adopting this for the core data canUser selector.

For example, HEAD /wp/v2/settings will return 401 for low-capability users.

[TimothyBlynJacobs]

I think for probably all core endpoints, that is just indicative of the user not having permissions for that route, right?

[Mamaduka]

that is just indicative of the user not having permissions for that route, right?

That would be a correct assumption.

[jorbin]

In 60044:

Coding Standards: Fix alignment in WP_REST_Widgets_Controller::prepare_item_for_response.

Follow-up to [59899].

See #56481, 62279.

[desrosj]

While looking through the Hosting Test reports prior to RC1, I noticed that there is at least one host where a few tests are failing after [59899].

I'm reopening this for tracking purposes so this can be properly investigated prior to 6.8 being released on April 15.

[antonvlasenko]

Replying to desrosj:

While looking through the Hosting Test reports prior to RC1, I noticed that there is at least one host where a few tests are failing after [59899].

I'm reopening this for tracking purposes so this can be properly investigated prior to 6.8 being released on April 15.

Thanks for the report, @desrosj. From what I can see, this is likely due to a hardcoded DB table name that was overlooked in one of the tests, while the correct table name was used elsewhere. I’ll submit a PR shortly.

[devsahadat]

Thanks for submitting the patch to improve HEAD request handling. It successfully skips unnecessary body generation, improving performance, especially for collection routes. The approach of querying only IDs for pagination headers is effective.

Suggestions:

Extend this fix to other controllers (e.g., WP_REST_Search_Controller).

Ensure compatibility with custom endpoints or plugins using rest_prepare_ filters.

Looking forward to further improvements!

This PR fixes failing unit test because of the hardcoded table name in WP_Test_REST_Users_Controller::test_get_items_only_fetches_ids_for_head_requests.

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

[antonvlasenko]

I've reviewed both Hosting Test Reports (​1, ​2), and the related test failures appear to be the following:

1. ​https://make.wordpress.org/hosting/test-results/r59970/wpmudevbot-r59970-8-1-mysql-ver-15-1-distrib-10-6-18-mariadb-for-debian-linux-gnu-x86_64-using-editline-wrapper/
2. ​https://make.wordpress.org/hosting/test-results/r59970/wpmudevbot-r59970-8-2-mysql-ver-15-1-distrib-10-6-18-mariadb-for-debian-linux-gnu-x86_64-using-editline-wrapper/
3. ​https://make.wordpress.org/hosting/test-results/r59970/wpmudevbot-r59970-8-3-mysql-ver-15-1-distrib-10-6-18-mariadb-for-debian-linux-gnu-x86_64-using-editline-wrapper/
4. ​https://make.wordpress.org/hosting/test-results/r60044/xserverbot-r60044-7-4-mysql-ver-15-1-distrib-10-5-22-mariadb-for-linux-x86_64-using-editline-wrapper/
5. ​https://make.wordpress.org/hosting/test-results/r60044/xserver2bot-r60044-8-2-mysql-ver-15-1-distrib-10-5-22-mariadb-for-linux-x86_64-using-editline-wrapper/
6. ​https://make.wordpress.org/hosting/test-results/r60044/wpmudevbot-r60044-8-3-mysql-ver-15-1-distrib-10-6-18-mariadb-for-debian-linux-gnu-x86_64-using-editline-wrapper/
7. ​https://make.wordpress.org/hosting/test-results/r60044/wpmudevbot-r60044-8-3-mysql-ver-15-1-distrib-10-6-18-mariadb-for-debian-linux-gnu-x86_64-using-editline-wrapper/
8. ​https://make.wordpress.org/hosting/test-results/r60044/wpmudevbot-r60044-8-4-mysql-ver-15-1-distrib-10-6-18-mariadb-for-debian-linux-gnu-x86_64-using-editline-wrapper/.

All of them are caused by the hardcoded database table name in the WP_Test_REST_Users_Controller::test_get_items_only_fetches_ids_for_head_requests method.

​The proposed PR addresses this issue and is ready for review.

[audrasjb]

Self assigning for commit before 6.8 RC3.

[audrasjb]

In 60141:

REST API: Fix hardcoded table name in WP_Test_REST_Users_Controller.

This changeset fixes failing unit tests on the Hosting test suite because of the hardcoded table name in WP_Test_REST_Users_Controller::test_get_items_only_fetches_ids_for_head_requests.

Props antonvlasenko, desrosj.
Fixes #56481.

[audrasjb]

Reopening for backport to branch 6.8.
(double committer sign-off already posted in the PR)

[audrasjb]

In 60143:

REST API: Fix hardcoded table name in WP_Test_REST_Users_Controller.

This changeset fixes failing unit tests on the Hosting test suite because of the hardcoded table name in WP_Test_REST_Users_Controller::test_get_items_only_fetches_ids_for_head_requests.

Reviewed by audrasjb, spacedmonkey.
Merges [60141] to the 6.8 branch.
Props antonvlasenko, desrosj.
Fixes #56481.