WordPress.org

Make WordPress Core

Opened 21 months ago

Last modified 3 weeks ago

#42094 assigned enhancement

REST API: extend _fields parameter to selectively include nested fields in response JSON

Reported by: kadamwhite Owned by: kadamwhite
Milestone: 5.3 Priority: normal
Severity: normal Version: 4.9
Component: REST API Keywords: has-patch dev-feedback needs-testing has-unit-tests
Focuses: Cc:

Description

This enhancement has been broken out from #38131, which introduced the _fields query parameter to whitelist select fields for inclusion in the API response object.

In the comments of that ticket @TimothyBlynJacobs uploaded a patch with support for nested fields, such as returning title.raw while omitting title.rendered:

[patch 31131.3.diff allows] specifying title.raw for just the raw title or title to retrieve both the raw and rendered title. This could also be used by controllers to only calculate the fields as necessary. I'm not sure whether this should be used to limit the fields queried ( I think that can be done in WP_User_Query but I'm not sure about the others ). I imagine some of the performance benefits here, besides response size, would be not filtering the_content if it wasn't a requested field, for instance.

There's been discussion of how feasible it is to use these parameters to hint that certain types of data processing are unnecessary, but on its own the base use-case of "give me the raw API response without its rendered counterpart" in edit mode could significantly speed up initial load of editor interfaces on slow connections.

This nesting feature has also been proven out in the plugin repository in the rest-api-filter-fields and wp-rest-mespath, the former through the same basefield.nestedfield syntax and the latter through JMESPath.

Attachments (2)

42094.1.diff (1.1 KB) - added by kadamwhite 21 months ago.
Basic unit test exploring the proposed dot-nested ?_fields syntax
42094.2.diff (16.0 KB) - added by kadamwhite 4 weeks ago.
Propose an implementation for nested _fields handling

Download all attachments as: .zip

Change History (16)

@kadamwhite
21 months ago

Basic unit test exploring the proposed dot-nested ?_fields syntax

#1 @kadamwhite
21 months ago

Added a patch with a basic unit test exloring the dot-nested syntax proposed above. I propose that the dot-separated solution is preferable to JMESPath for core usage, as it will be a straightforward enhancement over the initial syntax from #38131.

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


5 months ago

#3 @kadamwhite
5 months ago

Updates from REST API chat discussion: This has renewed relevance given discussions within the Gutenberg repo around how to signal that we can skip the expensive generation of content.rendered when the filtered value is not used. Implementing that in Gutenberg is non-trivial, but this _fields improvement is a good first step and would yield immediate performance gains for client-side consumers even without any special handling within our controllers.

In Slack @danielbachhuber proposes ?_fields=content[raw] as an alternative to either of the syntaxes discussed above.

#4 @kadamwhite
5 months ago

  • Milestone changed from Future Release to 5.2
  • Owner set to kadamwhite
  • Status changed from new to assigned

#5 @kadamwhite
5 months ago

  • Keywords needs-patch added

#6 @danielbachhuber
5 months ago

I'm amenable to dot syntax too (i.e. ?_fields=content.raw). My original suggestion for ?_fields=content[raw] was to follow PHP's existing pattern for transforming ?bar[]=1&bar[]=2&bar[]=3 into $_GET['bar'] = array(1, 2, 3).

We could support both if it's not too difficult. Otherwise, I'm fine with dot syntax if that's what everyone else prefers.

One minor backwards compatibility concern: a field could theoretically be named 'content.raw'. In this scenario, ?_fields=content.raw should return the 'content.raw' field, not a 'content' field with sub-field 'raw'.

#7 @desrosj
3 months ago

  • Keywords has-patch needs-refresh added; needs-patch removed

This still needs some work. Going to punt it. @kadamwhite if you have the time the next two days before 5.2 beta 1 and feel confident in this, feel free to add it back.

#8 @desrosj
3 months ago

  • Milestone changed from 5.2 to 5.3

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


2 months ago

#10 @kadamwhite
4 weeks ago

  • Keywords has-patch removed

@kadamwhite
4 weeks ago

Propose an implementation for nested _fields handling

#11 @kadamwhite
4 weeks ago

  • Keywords has-patch dev-feedback needs-testing has-unit-tests added; needs-refresh removed

I have uploaded a patch with a proposed solution to this problem. It works in three parts:

  • The initial generation of the accepted fields list is modified to be nested.field-aware,
  • The posts controller is modified to use a smarter helper method to determine if a specific property should be included, rather than a simple in_array call, and
  • The filter which strips invalid properties out of the response is modified to do a recursive check against a tree generated from the _fields input string. (Thank you @rmccue for helping with the code here.)

I would love to get some critique on this patch.

My biggest dissatisfaction with it is the code organization: the recursive key merge method feels like the simplest solution to this problem, but making that a rest_-prefixed global function feels inappropriate. However, introducing an arbitrary wp_ helper method also feels like an overstep. I'd welcome input, @danielbachhuber @desrosj @timothybjacobs

One additional unit test I'd like to add would be a check to ensure the content filters are not triggered if content.rendered is not requested in the fields list.

Once we agree on the mechanics of the patch, I can update the remaining controllers to use this new method when determining what fields to compute.

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


4 weeks ago

#13 @TimothyBlynJacobs
4 weeks ago

FWIW, I think extracting those as generic array utilities would be immensely helpful. I've had to implement similar things in other contexts, and having core functions for that would be helpful.

Perhaps the filtering by dotted fields could be a part of WP_List_Util? And then extract is_field_included into a standalone function?

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


3 weeks ago

Note: See TracTickets for help on using tickets.