REST API: Support nested _fields in revisions controller

[ellatrix]

https://core.trac.wordpress.org/ticket/64844

Use rest_is_field_included() instead of in_array() for content, title, excerpt, and guid fields in WP_REST_Revisions_Controller. This lets clients request specific sub-fields (e.g. _fields=content.raw) and skip expensive the_content rendering.

The REST API _fields documentation states that nested fields are supported using dot notation. However, the revisions controller uses in_array() which doesn't match nested keys like content.raw.

Several other controllers already use rest_is_field_included() for raw/rendered sub-fields:

• WP_REST_Posts_Controller — title.raw, title.rendered, content.raw, content.rendered
• WP_REST_Templates_Controller — content.raw, title.raw, title.rendered
• WP_REST_Menu_Items_Controller — title.raw, title.rendered
• WP_REST_Global_Styles_Controller — title.raw, title.rendered
• WP_REST_Themes_Controller — generic {field}.raw, {field}.rendered

Related Gutenberg PR: WordPress/gutenberg#76347

[github-actions]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props ellatrix, andrewserong, mukesh27.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[andrewserong]

This change seems sound to me! As mentioned in the linked Gutenberg issue, the performance benefits of skipping the rendered fields makes this very much worthwhile 👍

Totally optional as I don't think we're testing this specifically in the posts controller, but if you wanted you could possibly add a test for the nested properties case? E.g. something like:

  /**
   * Tests that nested _fields are supported for revision responses.
   *
   * @ticket 64844
   */
  public function test_prepare_item_with_nested_fields() {
      wp_set_current_user( self::$editor_id );

      $endpoint = new WP_REST_Revisions_Controller( 'post' );
      $request  = new WP_REST_Request( 'GET', '/wp/v2/posts/' . self::$post_id . '/revisions/' . $this->revision_id1 );

      $request->set_param( 'context', 'edit' );
      $request->set_param( '_fields', 'title.raw,content.raw' );

      $revision = get_post( $this->revision_id1 );
      $response = $endpoint->prepare_item_for_response( $revision, $request );

      $data = $response->get_data();

      // Only the requested sub-fields should be present.
      $this->assertArrayHasKey( 'raw', $data['title'] );
      $this->assertArrayNotHasKey( 'rendered', $data['title'] );
      $this->assertArrayHasKey( 'raw', $data['content'] );
      $this->assertArrayNotHasKey( 'rendered', $data['content'] );
  }

Or, update the existing test_prepare_item_limit_fields test to include the sub-properties:

wordpress-develop/tests/phpunit/tests/rest-api/rest-revisions-controller.php

Lines 508 to 523 in ed2f2ec

	public function test_prepare_item_limit_fields() {
	wp_set_current_user( self::$editor_id );
	$request = new WP_REST_Request( 'GET', '/wp/v2/posts/' . self::$post_id . '/revisions/' . $this->revision_id1 );
	$endpoint = new WP_REST_Revisions_Controller( 'post' );
	$request->set_param( 'context', 'edit' );
	$request->set_param( '_fields', 'id,slug' );
	$revision = get_post( $this->revision_id1 );
	$response = $endpoint->prepare_item_for_response( $revision, $request );
	$this->assertSame(
	array(
	'id',
	'slug',
	),
	array_keys( $response->get_data() )
	);
	}

In any case, this LGTM!

[mukeshpanchal27]

Previously, if guid was included, the response always contained both guid.rendered and guid.raw.
Now, if the we requests only guid (but not guid.raw or guid.rendered), they’ll get guid: [] (empty array).

Does this break BC?

[andrewserong]

It'll actually wind up including both, due to how rest_is_field_included works internally. Take a look here:

wordpress-develop/src/wp-includes/rest-api.php

Lines 1008 to 1023 in 1f333db

	foreach ( $fields as $accepted_field ) {
	/*
	* Check to see if $field is the parent of any item in $fields.
	* A field "parent" should be accepted if "parent.child" is accepted.
	*/
	if ( str_starts_with( $accepted_field, "$field." ) ) {
	return true;
	}
	/*
	* Conversely, if "parent" is accepted, all "parent.child" fields
	* should also be accepted.
	*/
	if ( str_starts_with( $field, "$accepted_field." ) ) {
	return true;
	}
	}

It's pretty neat — if a user just provides the parent guid then the sub-properties will also be matched.

[mukeshpanchal27]

Got it. Thanks!

[ellatrix]

Thanks for the review! I'm going to leave it as-is since we're close to Beta 5.

[github-actions]

A commit was made that fixes the Trac ticket referenced in the description of this pull request.

SVN changeset: 61987
GitHub commit: 4ad7d81

This PR will be closed, but please confirm the accuracy of this and reopen if there is more work to be done.