WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 4 years ago

Last modified 4 years ago

#35329 closed enhancement (fixed)

Permit rest_do_request() to be used in non-REST API contexts

Reported by: danielbachhuber Owned by: rmccue
Milestone: 4.5 Priority: normal
Severity: normal Version: 4.4
Component: REST API Keywords: has-patch commit has-unit-tests
Focuses: Cc:

Description

rest_do_request() currently requires $wp_rest_server to be properly instantiated:

function rest_do_request( $request ) {
    global $wp_rest_server;
    $request = rest_ensure_request( $request );
    return $wp_rest_server->dispatch( $request );
}

We should improve it such that it initializes $wp_rest_server if it isn't already loaded, such that you could use rest_do_request() in a non-REST API context.

@joehoyle wrote an equivalent function we can crib from.

Attachments (4)

35329.diff (2.9 KB) - added by swissspidy 5 years ago.
35329.2.diff (2.9 KB) - added by rmccue 5 years ago.
Patch without rest_internal_request
35329.3.diff (4.0 KB) - added by danielbachhuber 5 years ago.
Patch uses rest_get_server() when initializing REST server tests
35329.4.diff (4.0 KB) - added by danielbachhuber 4 years ago.
Use corrected @since tags

Download all attachments as: .zip

Change History (18)

#1 @westonruter
5 years ago

+1

In my looong career with the REST API (*wink*), I've already had to write an equivalent to this rest_internal_request() function a couple times to ensure the $wp_rest_server is constructed for internal requests.

#2 follow-up: @TimothyBlynJacobs
5 years ago

Would it be worth it to have a rest_get_server() function?

#3 in reply to: ↑ 2 @rmccue
5 years ago

Replying to TimothyBlynJacobs:

Would it be worth it to have a rest_get_server() function?

I think this is a good idea.

@swissspidy
5 years ago

#4 @swissspidy
5 years ago

  • Keywords has-patch added; needs-patch removed

35329.diff adds the rest_internal_request() function created by joehoyle and a new rest_get_server() function to get (and maybe set) the server instance. Not sure if the former is needed, but rest_get_server() feels like a great idea. Just think of all the places where global $wp_rest_server wouldn't be needed anymore.

#5 @danielbachhuber
5 years ago

I think we should add the logic of rest_internal_request() to rest_do_request(), given rest_do_request() already exists in core.

#6 @rmccue
5 years ago

  • Owner set to rmccue
  • Status changed from new to accepted

New patch added. I dropped rest_internal_request() (I don't think we need this given the other pieces) and replaced the rest_api_loaded() internals to avoid duplication of the API initialisation.

I don't think we need unit tests for this version (now that we don't have rest_internal_request())?

@rmccue
5 years ago

Patch without rest_internal_request

@danielbachhuber
5 years ago

Patch uses rest_get_server() when initializing REST server tests

#7 @swissspidy
5 years ago

Just that it's not forgotten, the version used in the @since docs for the hooks of course shouldn't change.

@danielbachhuber
4 years ago

Use corrected @since tags

This ticket was mentioned in Slack in #core-restapi by danielbachhuber. View the logs.


4 years ago

#9 @rachelbaker
4 years ago

  • Keywords commit added

@rmccue @joehoyle @swissspidy The latest patch here looks good to me, but I understand the use-case well enough to be comfortable committing it. Can you help get this in 4.5?

#10 @rachelbaker
4 years ago

  • Keywords has-unit-tests added; needs-unit-tests removed

This ticket was mentioned in Slack in #core-restapi by danielbachhuber. View the logs.


4 years ago

#12 @rmccue
4 years ago

  • Resolution set to fixed
  • Status changed from accepted to closed
  • Version set to 4.4

Fixed in r36529.

#13 @rmccue
4 years ago

In 36531:

REST API: Fix tests from r36529.

See #35329.

#14 @DrewAPicture
4 years ago

In 36947:

Docs: Improve syntax in the DocBlock for rest_get_server(), introduced in [36529].

See #35329. See #35986.

Note: See TracTickets for help on using tickets.