Make WordPress Core

Opened 19 months ago

Last modified 19 months ago

#57455 new enhancement

respond_to_request: store matched handlers across other methods, saving a call to get_routes().

Reported by: mreishus's profile mreishus Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: REST API Keywords: has-patch dev-feedback
Focuses: Cc:

Description

Current Behavior

Out of the box, API requests (example: GET http://localhost:8889/wp-json/wp/v2/posts) will make two calls to WP_REST_Server::get_routes().

Call 1: WP_REST_Server::dispatch() -> WP_REST_Server::match_request_to_handler() -> WP_REST_Server::get_routes().

This is the main path used when serving the request. It gets the routes, matches to the request, then determines which handler will be used for the request. Later, WP_REST_Server::respond_to_request() saves the matched route and matched handler.

Call 2: rest_send_allow_header() -> WP_REST_Server::get_routes().

This is used to set the allow header of the response (example: "Allow: GET, POST, HEAD").
This header shows all HTTP methods that are allowed to be sent on the same route. This information was already found in the "Call 1" pathway above, but it was discarded.

To find the allowed headers, rest_send_allow_header() calls WP_REST_Server::get_routes() which rebuilds information for all existing routes (but we only need information for one route).

Objective

To reduce the number of calls to WP_REST_Server::get_routes() per single request from 2 to 1.
While this only saves 0.1ms on a fresh site, on sites with large custom APIs, this could save 10ms or more.

Patch

When WP_REST_Server::match_request_to_handler() -> WP_REST_Server::get_routes() finds the matching route,
it will now not only remembers the exact matching handler, but all other handlers matching the route with different http methods.

For example, imagine an "/a/b/c" route exists with GET and POST handlers.
Previous to the patch, match_request_to_handler() would find the GET /a/b/c handler and remember it, setting it on $response->matched_handler.
After the patch, a new variable $response->all_methods_matched_handlers is set containing the array: [ (GET /a/b/c handler), (POST /a/b/c handler) ],

rest_send_allow_header() now looks for $response->all_methods_matched_handlers and uses it to set the Allow header if possible, saving a call to WP_REST_Server::get_routes().

This was my way to save the information found in call 1 and to pass it along to call 2, but I definitely open to other ideas of accomplishing the same thing.

Basic Testing

Add a log message when get_routes is called:

  • src/wp-includes/rest-api/class-wp-rest-server.php

    a b class WP_REST_Server { 
    862862        *               `'/path/regex' => array( array( $callback, $bitmask ), ...)`.
    863863        */
    864864       public function get_routes( $route_namespace = '' ) {
     865
    865866               $endpoints = $this->endpoints;
    866867
    867868               if ( $route_namespace ) {

Query an endpoint:
curl http://localhost:8889/wp-json/wp/v2/posts

Before the patch: See two calls to get_routes
After the patch: See one call to get_routes

Alternative Approaches

#39473 adds a per-request cache to get_routes. I'd be happy to rework this if this is a better method.

Attachments (1)

patchfile57455.patch (5.2 KB) - added by mreishus 19 months ago.

Download all attachments as: .zip

Change History (3)

#1 @mreishus
19 months ago

For the alternative approach: #39473 adds a per-request cache to get_routes: I don't know how to resolve the caching behavior with the new $namespace parameter added to get_routes().

In my example request of GET http://localhost:8889/wp-json/wp/v2/posts, the two calls to get_routes() have different parameters, since only one of the two applies a namespace optimization:

  • WP_REST_Server::match_request_to_handler() -> WP_REST_Server::get_routes( 'wp_v2' )
  • rest_send_allow_header() -> WP_REST_Server::get_routes( '' )

A straight forward cache of $namespace -> built $endpoints wouldn't work without other changes.

#2 @obenland
19 months ago

  • Keywords has-patch dev-feedback added
Note: See TracTickets for help on using tickets.