Make WordPress Core

Opened 3 years ago

Last modified 7 months ago

#42061 assigned enhancement

Add new utility function for checking if the current request is a REST API request.

Reported by: lots.0.logs Owned by: flixos90
Milestone: Future Release Priority: normal
Severity: normal Version: 4.9
Component: Bootstrap/Load Keywords: has-patch dev-feedback
Focuses: rest-api Cc:

Description (last modified by joehoyle)

This patch adds a new function: wp_doing_rest() and filter by the same name that work exactly like the current wp_doing_ajax/cron() functions but for Rest API requests.

Attachments (2)

42061-add-wp-doing-rest-filterable-function.patch (1.7 KB) - added by lots.0.logs 3 years ago.
42061.diff (3.1 KB) - added by flixos90 2 years ago.

Download all attachments as: .zip

Change History (17)

#1 @lots.0.logs
3 years ago

  • Keywords has-patch added

#2 @joehoyle
3 years ago

  • Description modified (diff)
  • Milestone Awaiting Review deleted
  • Resolution set to wontfix
  • Status changed from new to closed

See https://github.com/WP-API/WP-API/issues/926 for the previous story on this.

Though we want to discourage use of global state functions like these, there _is_ a REST_REQUEST constant for this purpose. As mentioned in that ticket, the idea of "the current request is a REST one" breaks down for things like _embed requests, and internal rest_do_request calls, therefore we don't want to encourage the use of "is the current request a REST one" as it's not always in that context.

@lots.0.logs perhaps you could define why you need such a function? wp_doing_ajax/cron() are inherently process-level functions, but the REST API callback methods are not intended to scoped to the whole "page load" request.

#3 @flixos90
2 years ago

  • Resolution wontfix deleted
  • Status changed from closed to reopened

I think this needs to be revisited, for the same purpose we have functions like wp_doing_ajax() etc. Every function in WordPress can be misused, but a wp_doing_rest() or wp_is_rest_request() would allow fixing weird hacks necessary when working with unit tests. See #43751 for an example.

#4 @SergeyBiryukov
2 years ago

  • Milestone set to Awaiting Review

#5 @swissspidy
2 years ago

  • Milestone changed from Awaiting Review to 5.0

+1 for consistency.

See #25669, #39591.

2 years ago

#6 @lots.0.logs
2 years ago

I see you changed the name to wp_is_rest_request(). That is inconsistent with existing similar utility functions. IMO, wp_doing_rest() is the most appropriate name for this function.

#7 follow-up: @flixos90
2 years ago

  • Keywords dev-feedback added
  • Owner set to flixos90
  • Status changed from reopened to assigned

As usual, this is more complicated than it may seem at first glance. Other than AJAX and Cron, which are controlled by constants, REST is controlled by a query variable, meaning it relies on the current request to be parsed. The REST_REQUEST constant is only set for an easy check, but is not available from the beginning of the request's lifecycle.

It is therefore impossible to detect a REST request before the parse_request action, at least with the means currently in place. That action is triggered rather late, after init, wp_loaded etc.

There is actually a bug in core which doesn't take the above into account: in wp_debug_mode(), it is checked whether the current request is a REST request in order to not display errors in such a case. However, this check will always result in false, since the function is executed even before most of WordPress files are even loaded. Therefore, currently REST API responses will still be messed up by debugging messages if enabled. This could be fixed in a separate issue though, depending on what we decide here.

I see two possibilities:

  • We could embrace the current functionality, relying on the query variable and introduce the function in a way that is triggers _doing_it_wrong() if called too early. In that case, it should be named differently from the existing wp_doing_ajax() and wp_doing_cron(), to indicate its different behavior (I suggest wp_is_rest_request() because in WordPress terms that connects it to the request which is associated with parsing the current request).
  • We could find a way to detect a REST request through another way and aim for making the process similar to AJAX and Cron so that it can be detected really early. In that case, the function could also be named accordingly, wp_doing_rest().

I'm in favor of the first alternative, and this is what I implemented in 42061.diff:

  • It introduces wp_is_rest_request() in wp-includes/functions.php. No point to put it in wp-includes/load.php under these circumstances.
  • If parse_request has not been executed yet and REST_REQUEST is not set (this check would allow custom setups to still set it early), but the function is called already, a _doing_it_wrong() is triggered.
  • The check for a REST request in wp_debug_mode() has been removed, since it doesn't do anything. Since I adjusted that line anyway, I also fixed a direct check of WP_INSTALLING, which should instead rely on wp_installing().
  • Other checks of REST_REQUEST has been adjusted as well, and the additional check in wp-admin/includes/ms.php is something I detected during #43751 (it could currently simply kill REST requests).

#8 @flixos90
2 years ago

  • Milestone changed from 5.0 to 5.1

#9 @swissspidy
2 years ago

Worth noting that #44534 introduced wp_is_json_request()

#10 @pento
21 months ago

wp_is_json_request() replaced some of the use cases for this function.

@flixos90: Do you think wp_doing_rest() is still needed?

#11 @ocean90
21 months ago

#45875 was marked as a duplicate.

#12 @pento
21 months ago

  • Milestone changed from 5.1 to Future Release

Punting this, as it requires further consideration.

#14 @mikejolley
20 months ago

@pento We could use this in WooCommerce. We do things such as load the cart after wp_loaded action, but we're unable to disable this based on REST_REQUEST due to that constant not being set until parse_request.

A function, available earlier, would be very useful.

ref https://github.com/woocommerce/woocommerce/issues/20937

#15 in reply to: ↑ 7 @iandunn
7 months ago

Replying to flixos90:

REST is controlled by a query variable...
It is therefore impossible to detect a REST request before the parse_request action, at least with the means currently in place...
We could find a way to detect a REST request through another way and aim for making the process similar to AJAX and Cron so that it can be detected really early.

I think that'd be ideal. One benefit would be that WP_USE_THEMES could be set to false for REST API requests, and wp_using_themes() could be used to detect the front end before template_redirect executes.

One way to do that might be something like the solution that Woo experiment with. I haven't looked deeply to see if there are cases where that wouldn't work, though.

Note: See TracTickets for help on using tickets.