Make WordPress Core

Opened 6 years ago

Closed 6 years ago

#45189 closed defect (bug) (fixed)

Improve preloading request code

Reported by: schlessera's profile schlessera Owned by: danielbachhuber's profile danielbachhuber
Milestone: 5.0 Priority: low
Severity: minor Version: 5.0
Component: REST API Keywords: has-patch fixed-5.0
Focuses: rest-api Cc:

Description

The attached patch removes the conditional branching based on the existence of the get_compact_response_links() method on the $server object, as that method was introduced with WordPress 4.5, and Gutenberg requires WordPress 4.9.8 or later.

Also, it turns the indirect and slow call through call_user_func() into a direct method call.

Note: The call would actually be more correct as a static call ($server::get_compact_response_links( $response)), but this notation is PHP 5.3+ only.

The patch is done against 5.0 branch.

Mirrors Gutenberg PR #11007 per request by @danielbachhuber.

Attachments (2)

45189.diff (743 bytes) - added by schlessera 6 years ago.
45189.2.diff (1.6 KB) - added by danielbachhuber 6 years ago.

Download all attachments as: .zip

Change History (7)

@schlessera
6 years ago

#1 follow-up: @TimothyBlynJacobs
6 years ago

If dropping the method exists check, I think it makes sense to drop it from WP_REST_Controller::prepare_response_for_collection() as well.

I wonder if this was included for BC because the server class is filterable, so theoretically someone could be using a server class without that method, though I think doing a completely custom class that doesn't extend WP_REST_Server is probably something that shouldn't/isn't supported anyways.

#2 in reply to: ↑ 1 @danielbachhuber
6 years ago

  • Focuses rest-api added

Replying to TimothyBlynJacobs:

If dropping the method exists check, I think it makes sense to drop it from WP_REST_Controller::prepare_response_for_collection() as well.

Yes, let's do.

I wonder if this was included for BC because the server class is filterable, so theoretically someone could be using a server class without that method, though I think doing a completely custom class that doesn't extend WP_REST_Server is probably something that shouldn't/isn't supported anyways.

There are a ton of WP_REST_Server methods in use so, if someone is filtering the server, they need to implement all methods that could be used. I'm not concerned about BC here.

#3 @danielbachhuber
6 years ago

  • Owner set to danielbachhuber
  • Resolution set to fixed
  • Status changed from new to closed

In 43834:

REST API: Improve performance by avoiding call_user_func().

The get_compact_response_links() method was introduced in WP 4.5, and this conditional is no longer necessary.

Props schlessera, timothyblynjacobs.
Fixes #45189.

#4 @danielbachhuber
6 years ago

  • Keywords fixed-5.0 added
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening for merge to trunk.

#5 @pento
6 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 44173:

REST API: Improve performance by avoiding call_user_func().

The get_compact_response_links() method was introduced in WP 4.5, and this conditional is no longer necessary.

Merges [43834] from the 5.0 branch to trunk.

Props schlessera, timothyblynjacobs.
Fixes #45189.

Note: See TracTickets for help on using tickets.