WordPress.org

Make WordPress Core

Opened 8 months ago

Last modified 8 months ago

#46238 new enhancement

REST API: Allow conditional field registration

Reported by: dmsnell Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: REST API Keywords:
Focuses: Cc:
PR Number:

Description

register_rest_field() allows us to add additional fields to API call responses but it doesn't allow us to do so conditionally. in prior art we add a field and then remove it later when it's not needed.

why would inclusion of a field be conditional? what is wrong with returning null?

we came across this when wanting to add metadata to video files, which are post types and conflated with posts and other media attachments. we'd like to be able to just add fields to video file media requests and not pollute the responses for all of the other "subtypes." we'd also like to do it without resorting to surprising behaviors like adding and later removing them.

in this ticket I'd like to propose two approaches to accomplishing this goal. the first is a bit clever but should make it possible to extend existing behaviors without breaking any code and without introducing further confusion on how to add fields. it uses a "sentinel value" passed into the get_callback() function which can be returned to indicate that the field should not exist.

the second approach creates a new function register_conditional_rest_field() and provides a wrapped response format to indicate whether the field should be added. this approach is less idiomatic in my opinion as we're not used to returning wrapped values.

---

I thought for sure I'd seen a discussion around this before but I couldn't find any tickets with my search. that being said I'm curious why this doesn't already exist as it seems like it could be a common need, though I'm not sure how the fact that media attachments are a bit odd in comparison to custom post types comes into play.

Attachments (1)

46238a.diff (1.9 KB) - added by dmsnell 8 months ago.
add-the-sentinel approach

Download all attachments as: .zip

Change History (4)

@dmsnell
8 months ago

add-the-sentinel approach

#1 @mmtr86
8 months ago

Thanks for opening this ticket @dmsnell!

we came across this when wanting to add metadata to video files, which are post types and conflated with posts and other media attachments

For context, this is the GitHub PR where we adding this behavior. I'd like to clarify that we were actually adding the new REST field only to the media attachments with register_rest_field( 'attachment', ... ), so the posts were not affected. But indeed, this is adding the field to all the attachments and we only want it on video attachments.

it uses a "sentinel value" passed into the get_callback() function which can be returned to indicate that the field should not exist.

I like how this approach is backwards compatible with the existing behavior, and your suggested implementation looks elegant and simple to me.

I wonder however what implications this may cause to the schema. Since we cannot conditionally add the field to the schema, it could present an inconsistency:

  • When the field is declared on the schema but not present on the response.
  • When the field is undeclared on the schema but present on the response.

On the previous PR, we opted for declaring the field on the schema. This doesn't cause any failure to the responses not containing the field, but the inconsistency between schema and response is there.

I'm not familiar with API schemas, but maybe there is a way to define a field as optional so we make the schema to always match the response.

#2 @dmsnell
8 months ago

thanks for the feedback @mmtr86

I wonder however what implications this may cause to the schema.

I did some checking to see if I could explore your prompt more. in the process I noticed an oddity and a salient point about schema. I also noted that it was difficult for me to find clear documentation on the schema side of the API - particularly to understand how this schema fits in with the overall shape of the response

for one, I learned that I should be able to query the schema with a GET call to the API base…

curl 'http://localhost/wp-json/wp/v2/' | jq '.routes | ."/wp/v2/media/(?P<id>[\\\\d]+)" | .endpoints[0]'
{
  "methods": [
    "GET"
  ],
  "args": {
    "id": {
      "required": false,
      "description": "Unique identifier for the object.",
      "type": "integer"
    },
    "context": {
      "required": false,
      "default": "view",
      "enum": [
        "view",
        "embed",
        "edit"
      ],
      "description": "Scope under which the request is made; determines fields present in response.",
      "type": "string"
    }
  }
}

…but the schema for my fields didn't come though. interestingly enough they come through for the update endpoints.

curl 'http://localhost/wp-json/wp/v2/' | jq '.routes | ."/wp/v2/media/(?P<id>[\\\\d]+)" | .endpoints[1]'
{
  "methods": [
    "POST",
    "PUT",
    "PATCH"
  ],
  "args": {"vp_guid": {
      "required": false,
      "description": "Some GUID",
      "type": "string"
    },
    …
  }
}

It took me a while to figure out how to query the schema. This we do with an OPTIONS request.

curl -X OPTIONS 'http://localhost/wp-json/wp/v2/media/5' | jq '.schema .properties'
{"vp_guid": {
    "description": "Some GUID",
    "type": "string",
    "context": [
      "view",
      "edit"
    ],
    "required": false
  },
  …
}

If I interpret the entire response schema as a JSON-Schema then I think it means that we're covered. Without that explicit required => false indicator it won't show up as not required, but at the same time my understanding of JSON-Schema is that unless a field is explicitly indicated as required then it's not required.

In other words I don't think this carries any implications on the schema unless we've marked the field as required => true. If I take into consideration that it's less obvious how to even get the GET schema it seems to mark us even more safe. I don't understand the implications of the update endpoints though.

#3 @mmtr86
8 months ago

Wonderful, thanks for confirming @dmsnell!

Note: See TracTickets for help on using tickets.